diff --git a/apps/cli/src/commands/fleet.ts b/apps/cli/src/commands/fleet.ts index c290b49..830881e 100644 --- a/apps/cli/src/commands/fleet.ts +++ b/apps/cli/src/commands/fleet.ts @@ -296,6 +296,9 @@ type FleetInvocation = { timeoutSeconds: number commandId: string | null targetFallback: readonly string[] + /** From a saved command when it came from one; a flag always wins. */ + concurrency?: number + onFailure?: 'continue' | 'stop' /** * The caller already asked about this script's hazards in terms specific to * what it does. `fleet upgrade --reboot` names the servers it will restart, @@ -339,8 +342,8 @@ async function execute( output: Output, ): Promise { const targets = await resolveTargets(parsed, store, invocation.targetFallback) - const concurrency = numberFlag(parsed, '--concurrency') ?? FLEET_DEFAULT_CONCURRENCY - const onFailure = hasFlag(parsed, '--stop-on-error') ? 'stop' : 'continue' + const concurrency = numberFlag(parsed, '--concurrency') ?? invocation.concurrency ?? FLEET_DEFAULT_CONCURRENCY + const onFailure = hasFlag(parsed, '--stop-on-error') ? 'stop' : (invocation.onFailure ?? 'continue') const env = envFromFlags(parsed) const assumeYes = hasFlag(parsed, '--yes') @@ -550,6 +553,8 @@ async function fleetRun(parsed: ParsedArgv, store: DiskPushStore, output: Output sudo: command.sudo || hasFlag(parsed, '--sudo'), workingDirectory: flagValue(parsed, '--cwd') ?? command.workingDirectory, timeoutSeconds: numberFlag(parsed, '--timeout') ?? command.timeoutSeconds, + concurrency: numberFlag(parsed, '--concurrency') ?? command.concurrency, + onFailure: hasFlag(parsed, '--stop-on-error') ? 'stop' : command.onFailure, commandId: command.builtin ? null : command.id, targetFallback: command.targets, }, @@ -841,6 +846,11 @@ async function fleetCommands(parsed: ParsedArgv, store: DiskPushStore, output: O sudo: hasFlag(parsed, '--sudo'), workingDirectory: flags.workingDirectory, timeoutSeconds: flags.timeoutSeconds, + // Pacing is part of the command, not of the invocation: a saved command + // that forgets it was meant to run two at a time is a saved command that + // still gets run wrong. + concurrency: numberFlag(parsed, '--concurrency') ?? FLEET_DEFAULT_CONCURRENCY, + onFailure: hasFlag(parsed, '--stop-on-error') ? 'stop' : 'continue', targets: flagValues(parsed, '--on'), tags: flagValues(parsed, '--tag'), }) diff --git a/apps/desktop/electron/main/ipc.ts b/apps/desktop/electron/main/ipc.ts index 71b7530..373f84f 100644 --- a/apps/desktop/electron/main/ipc.ts +++ b/apps/desktop/electron/main/ipc.ts @@ -10,6 +10,7 @@ import { DeleteEntryRequestSchema, ExternalUrlSchema, FleetCheckRequestSchema, + FleetCommandSaveSchema, FleetListRenameSchema, FleetListSaveSchema, FleetRequestSchema, @@ -29,6 +30,8 @@ import { fleetCommands, fleetLists, fleetRunDetail, + removeFleetCommand, + saveFleetCommand, fleetServers, removeFleetList, renameFleetList, @@ -331,6 +334,12 @@ export function registerIpc(): void { handle(IPC.fleetRunDetail, z.object({ runId: FleetRunIdSchema }), async ({ runId }) => fleetRunDetail(runId)) + handle(IPC.fleetCommandSave, FleetCommandSaveSchema, async (input) => saveFleetCommand(input)) + + handle(IPC.fleetCommandRemove, z.object({ name: z.string().min(1).max(128) }), async ({ name }) => + removeFleetCommand(name), + ) + handle(IPC.fleetLists, z.undefined(), async () => fleetLists()) handle(IPC.fleetListSave, FleetListSaveSchema, async (input) => saveFleetList(input)) diff --git a/apps/desktop/electron/main/services/fleet.ts b/apps/desktop/electron/main/services/fleet.ts index b3045a2..6f006c2 100644 --- a/apps/desktop/electron/main/services/fleet.ts +++ b/apps/desktop/electron/main/services/fleet.ts @@ -173,6 +173,36 @@ export async function startFleet(request: FleetRequest, sender: WebContents): Pr } } +// --- saved commands --------------------------------------------------------- + +/** + * Saves a command from the Fleet view. + * + * `builtin` is not settable here: the store forces it false, so a saved + * command can shadow a shipped recipe by name but can never claim to be one. + */ +export async function saveFleetCommand(input: { + name: string + description: string + script: string + interpreter: 'sh' | 'bash' | 'raw' + sudo: boolean + workingDirectory: string | null + timeoutSeconds: number + concurrency: number + onFailure: 'continue' | 'stop' + targets: string[] +}): Promise { + return (await store()).saveFleetCommand({ ...input, tags: [] }) +} + +export async function removeFleetCommand(name: string): Promise { + if (BUILTIN_RECIPES.some((recipe) => recipe.name === name)) { + throw new Error(`${name} is a recipe DiskPush ships and cannot be deleted. Save a copy under another name instead.`) + } + return (await store()).deleteFleetCommand(name) +} + // --- saved lists ------------------------------------------------------------ export async function fleetLists(): Promise { diff --git a/apps/desktop/electron/preload/index.ts b/apps/desktop/electron/preload/index.ts index fcfae96..70e3b4c 100644 --- a/apps/desktop/electron/preload/index.ts +++ b/apps/desktop/electron/preload/index.ts @@ -58,6 +58,8 @@ const api = { check: (connectionIds: string[], concurrency = 4, timeoutSeconds = 180) => call(IPC.fleetCheck, { connectionIds, concurrency, timeoutSeconds }), runs: (limit = 25) => call(IPC.fleetRuns, { limit }), + saveCommand: (input: unknown) => call(IPC.fleetCommandSave, input), + removeCommand: (name: string) => call(IPC.fleetCommandRemove, { name }), lists: () => call(IPC.fleetLists), saveList: (name: string, connectionIds: string[], description = '') => call(IPC.fleetListSave, { name, connectionIds, description }), diff --git a/apps/desktop/electron/shared/contract.ts b/apps/desktop/electron/shared/contract.ts index 9ae8706..afae0c9 100644 --- a/apps/desktop/electron/shared/contract.ts +++ b/apps/desktop/electron/shared/contract.ts @@ -45,6 +45,8 @@ export const IPC = { fleetCancel: 'fleet:cancel', fleetCheck: 'fleet:check', fleetRuns: 'fleet:runs', + fleetCommandSave: 'fleet:command-save', + fleetCommandRemove: 'fleet:command-remove', fleetLists: 'fleet:lists', fleetListSave: 'fleet:list-save', fleetListRename: 'fleet:list-rename', @@ -244,6 +246,29 @@ export const FleetListNameSchema = z.string().min(1).max(128).refine((name) => n message: 'A name cannot begin or end with a space.', }) +/** + * Saving a command from the Fleet view. + * + * The same shape the run request takes, minus the servers and the password: + * a saved command is the *settings*, and which servers to point them at is a + * separate choice made at run time (or remembered as `targets`). + */ +export const FleetCommandSaveSchema = z.object({ + name: z.string().min(1).max(128).refine((value) => value.trim() === value, { + message: 'A name cannot begin or end with a space.', + }), + description: z.string().max(500).default(''), + script: z.string().min(1).max(256 * 1024), + interpreter: z.enum(['sh', 'bash', 'raw']).default('sh'), + sudo: z.boolean().default(false), + workingDirectory: PathSchema.nullable().default(null), + timeoutSeconds: z.number().int().min(1).max(86400).default(900), + concurrency: z.number().int().min(1).max(64).default(4), + onFailure: z.enum(['continue', 'stop']).default('continue'), + /** Remembered so a saved command can carry the servers it is usually for. */ + targets: z.array(z.string().min(1).max(128)).max(500).default([]), +}) + export const FleetListSaveSchema = z.object({ name: FleetListNameSchema, description: z.string().max(500).default(''), diff --git a/apps/desktop/src/components/fleet-view.tsx b/apps/desktop/src/components/fleet-view.tsx index 9a0c986..c32a4a4 100644 --- a/apps/desktop/src/components/fleet-view.tsx +++ b/apps/desktop/src/components/fleet-view.tsx @@ -71,6 +71,8 @@ export function FleetView({ onAddServer }: { onAddServer: () => void }) { const [lists, setLists] = useState([]) const [savingList, setSavingList] = useState(false) const [newListName, setNewListName] = useState('') + const [savingCommand, setSavingCommand] = useState(false) + const [newCommandName, setNewCommandName] = useState('') const [selected, setSelected] = useState>(new Set()) const [tagFilter, setTagFilter] = useState(null) @@ -221,9 +223,51 @@ export function FleetView({ onAddServer }: { onAddServer: () => void }) { setInterpreter(command.interpreter) setSudo(command.sudo) setTimeoutSeconds(command.timeoutSeconds) + // The pacing is part of the command: "reload nginx" and "upgrade the + // database tier" want very different answers, and leaving whatever was + // last on screen is how a saved command still gets run wrong. + setConcurrency(command.concurrency) + setStopOnError(command.onFailure === 'stop') setHazards([]) } + /** Saves everything on screen except the servers, which are chosen per run. */ + const saveCommand = useCallback(async () => { + const name = newCommandName.trim() + if (!name || !script.trim()) return + setError(null) + try { + await unwrap( + api()?.fleet.saveCommand({ + name, + script, + interpreter, + sudo, + workingDirectory: null, + timeoutSeconds, + concurrency, + onFailure: stopOnError ? 'stop' : 'continue', + }), + ) + setNewCommandName('') + setSavingCommand(false) + setLabel(name) + setCommands(await unwrap(api()?.fleet.commands())) + } catch (caught) { + setError(caught instanceof Error ? caught.message : String(caught)) + } + }, [newCommandName, script, interpreter, sudo, timeoutSeconds, concurrency, stopOnError]) + + const removeCommand = useCallback(async (name: string) => { + setError(null) + try { + await unwrap(api()?.fleet.removeCommand(name)) + setCommands(await unwrap(api()?.fleet.commands())) + } catch (caught) { + setError(caught instanceof Error ? caught.message : String(caught)) + } + }, []) + const requestBody = useCallback( (hazardsConfirmed: boolean) => ({ connectionIds: [...selected], @@ -457,19 +501,77 @@ export function FleetView({ onAddServer }: { onAddServer: () => void }) {
- Recipes + Commands {commands.map((command) => ( - + {/* + Only what someone saved can be deleted. A shipped recipe is + copied, not edited, so upgrading DiskPush never silently + changes a command anyone relies on. + */} + {command.builtin ? null : ( + + )} + + ))} + + {script.trim() && !savingCommand ? ( + - ))} + ) : null} + + {savingCommand ? ( + + setNewCommandName(event.target.value)} + onKeyDown={(event) => { + if (event.key === 'Enter') void saveCommand() + if (event.key === 'Escape') { + setSavingCommand(false) + setNewCommandName('') + } + }} + placeholder="command name" + className="h-6 w-[150px] text-[11.5px]" + /> + + + ) : null}
{/* diff --git a/apps/desktop/src/lib/api.ts b/apps/desktop/src/lib/api.ts index c3b2092..c0740cd 100644 --- a/apps/desktop/src/lib/api.ts +++ b/apps/desktop/src/lib/api.ts @@ -104,11 +104,27 @@ export type FleetCommand = { sudo: boolean workingDirectory: string | null timeoutSeconds: number + concurrency: number + onFailure: 'continue' | 'stop' targets: string[] tags: string[] builtin: boolean } +/** Everything a saved command remembers. The servers are chosen at run time. */ +export type FleetCommandSave = { + name: string + description?: string + script: string + interpreter: 'sh' | 'bash' | 'raw' + sudo: boolean + workingDirectory: string | null + timeoutSeconds: number + concurrency: number + onFailure: 'continue' | 'stop' + targets?: string[] +} + export type Hazard = { kind: string; explanation: string; line: string; lineNumber: number } export type FleetListMember = { connectionId: string; connectionName: string } @@ -209,6 +225,8 @@ type Api = { check(connectionIds: string[], concurrency?: number, timeoutSeconds?: number): Promise> runs(limit?: number): Promise> runDetail(runId: string): Promise> + saveCommand(input: FleetCommandSave): Promise> + removeCommand(name: string): Promise> lists(): Promise> saveList(name: string, connectionIds: string[], description?: string): Promise> renameList(from: string, to: string): Promise> diff --git a/docs/desktop.md b/docs/desktop.md index dd9920c..daa845c 100644 --- a/docs/desktop.md +++ b/docs/desktop.md @@ -95,6 +95,13 @@ script editor, and the results — and **the action bar is pinned outside all of them**. Run is on screen at the 960×600 minimum window size just as it is maximised. +The command strip above the editor holds the shipped recipes and anything you +have saved. Editing the script or the settings offers **Save these settings**, +which stores the script, the interpreter, sudo, the timeout, how many servers +at a time, and whether a failure stops the rest — picking it again restores all +of it. A saved command carries an × to delete it; a shipped recipe does not, +because it is copied rather than edited. + Saved **lists** sit at the top of the sidebar, above the tag chips. Clicking one ticks exactly its members; ticking servers by hand offers **Save these 3**; the × on a chip deletes the list and leaves the servers alone. A member whose diff --git a/docs/fleet.md b/docs/fleet.md index 5f94b2c..3109e72 100644 --- a/docs/fleet.md +++ b/docs/fleet.md @@ -173,9 +173,27 @@ diskpush fleet commands copy upgrade my-upgrade # built-ins are copied, not e diskpush fleet run --command reload-nginx # --on comes from the saved default ``` -A built-in cannot be edited or deleted, only copied, so upgrading DiskPush -never silently changes a command you rely on. A saved command with the same -name as a built-in shadows it. +A saved command remembers **the settings, not just the script**: the +interpreter, whether it runs through sudo, the timeout, how many servers at a +time, and whether a failure stops the rest. Picking one restores all of it. +Those last two matter — "reload nginx" and "upgrade the database tier" want +very different answers, and re-choosing them on every run is how a saved +command still gets run wrong. + +```bash +diskpush fleet commands save careful "systemctl reload nginx" \ + --sudo --concurrency 2 --stop-on-error --timeout 45 --on tag:web + +diskpush fleet run --command careful # 2 at a time, stops on failure +diskpush fleet run --command careful --concurrency 8 # a flag still wins +``` + +In the desktop app the same commands are the strip above the editor. Editing +anything offers **Save these settings**; a saved command carries an × to +delete it. A shipped recipe has no × — it is copied, not edited, so upgrading +DiskPush never silently changes a command you rely on. + +A saved command with the same name as a built-in shadows it. ### `fleet runs` and `fleet show` diff --git a/packages/database/src/migrations.ts b/packages/database/src/migrations.ts index 722e9a3..85251bc 100644 --- a/packages/database/src/migrations.ts +++ b/packages/database/src/migrations.ts @@ -173,4 +173,20 @@ export const MIGRATIONS: Migration[] = [ )`, ], }, + { + name: '004-fleet-command-pacing', + statements: [ + /* + * A saved command captured what to run and where, but not how fast. + * "reload nginx" and "upgrade the database tier" want very different + * answers to those two, and re-choosing them on every run is how a + * saved command still gets run wrong. + * + * Defaults match the schema's, so every existing row reads back exactly + * as it behaved before. + */ + `ALTER TABLE fleet_commands ADD COLUMN concurrency INTEGER NOT NULL DEFAULT 4`, + `ALTER TABLE fleet_commands ADD COLUMN on_failure TEXT NOT NULL DEFAULT 'continue'`, + ], + }, ] diff --git a/packages/database/src/store.ts b/packages/database/src/store.ts index b92da69..59fbb65 100644 --- a/packages/database/src/store.ts +++ b/packages/database/src/store.ts @@ -333,12 +333,13 @@ export class DiskPushStore { await this.client.execute({ sql: `INSERT INTO fleet_commands ( id, name, description, script, interpreter, sudo, working_directory, - timeout_seconds, targets, tags, created_at, updated_at - ) VALUES (?,?,?,?,?,?,?,?,?,?,?,?) + timeout_seconds, concurrency, on_failure, targets, tags, created_at, updated_at + ) VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?) ON CONFLICT(id) DO UPDATE SET name=excluded.name, description=excluded.description, script=excluded.script, interpreter=excluded.interpreter, sudo=excluded.sudo, working_directory=excluded.working_directory, timeout_seconds=excluded.timeout_seconds, + concurrency=excluded.concurrency, on_failure=excluded.on_failure, targets=excluded.targets, tags=excluded.tags, updated_at=excluded.updated_at`, args: [ command.id, @@ -349,6 +350,8 @@ export class DiskPushStore { command.sudo ? 1 : 0, command.workingDirectory, command.timeoutSeconds, + command.concurrency, + command.onFailure, JSON.stringify(command.targets), JSON.stringify(command.tags), command.createdAt, @@ -588,6 +591,8 @@ function rowToFleetCommand(row: Row): FleetCommand { sudo: Number(row.sudo) === 1, workingDirectory: row.working_directory === null ? null : String(row.working_directory), timeoutSeconds: Number(row.timeout_seconds), + concurrency: Number(row.concurrency), + onFailure: String(row.on_failure), targets: JSON.parse(String(row.targets)), tags: JSON.parse(String(row.tags)), builtin: false, diff --git a/packages/fleet-core/src/recipes.test.ts b/packages/fleet-core/src/recipes.test.ts index aa4deec..4d82620 100644 --- a/packages/fleet-core/src/recipes.test.ts +++ b/packages/fleet-core/src/recipes.test.ts @@ -40,6 +40,19 @@ describe('the recipes DiskPush ships', () => { } }) + it('state every field, since nothing parses them through the schema', () => { + // These are plain literals typed as FleetCommand. A field the schema + // merely defaults is `undefined` here, and reached the desktop as + // `setConcurrency(undefined)`. + for (const entry of BUILTIN_RECIPES) { + expect(entry.concurrency, entry.name).toBeGreaterThan(0) + expect(['continue', 'stop'], entry.name).toContain(entry.onFailure) + expect(entry.timeoutSeconds, entry.name).toBeGreaterThan(0) + expect(entry.workingDirectory, entry.name).toBeNull() + expect(Array.isArray(entry.targets), entry.name).toBe(true) + } + }) + it('have unique names, so one cannot shadow another', () => { const names = BUILTIN_RECIPES.map((recipe) => recipe.name) expect(new Set(names).size).toBe(names.length) @@ -68,6 +81,10 @@ describe('copyRecipe', () => { expect(copy.builtin).toBe(false) expect(copy.tags).not.toContain('builtin') expect(copy.script).toBe(findRecipe('upgrade')!.script) + // A copy that lost the pacing would run the same script at a different + // speed, which is the one thing a copy must not do. + expect(copy.concurrency).toBe(findRecipe('upgrade')!.concurrency) + expect(copy.onFailure).toBe(findRecipe('upgrade')!.onFailure) }) }) diff --git a/packages/fleet-core/src/recipes.ts b/packages/fleet-core/src/recipes.ts index f190194..ece9ec6 100644 --- a/packages/fleet-core/src/recipes.ts +++ b/packages/fleet-core/src/recipes.ts @@ -1,4 +1,4 @@ -import { FLEET_DEFAULT_TIMEOUT_SECONDS, type FleetCommand } from '@diskpush/schemas' +import { FLEET_DEFAULT_CONCURRENCY, FLEET_DEFAULT_TIMEOUT_SECONDS, type FleetCommand } from '@diskpush/schemas' import { buildUpgradeScript, CHECK_SCRIPT } from './upgrade.js' /** @@ -25,6 +25,14 @@ function recipe( sudo: false, workingDirectory: null, timeoutSeconds: FLEET_DEFAULT_TIMEOUT_SECONDS, + /* + * Stated, not inherited. These objects are plain literals typed as + * FleetCommand — nothing parses them through the schema, so a field the + * schema defaults is simply `undefined` here. That reached the desktop as + * `setConcurrency(undefined)` and broke the input. + */ + concurrency: FLEET_DEFAULT_CONCURRENCY, + onFailure: 'continue', targets: [], tags: ['builtin'], builtin: true, @@ -134,6 +142,10 @@ export function copyRecipe(recipeToCopy: FleetCommand, name: string): Omit tag !== 'builtin'), builtin: false, diff --git a/packages/schemas/src/fleet.ts b/packages/schemas/src/fleet.ts index dbbb5e4..3d2da31 100644 --- a/packages/schemas/src/fleet.ts +++ b/packages/schemas/src/fleet.ts @@ -61,6 +61,13 @@ export const FleetCommandSchema = z.object({ * Default target selector, in the same syntax `--on` takes. Saved with the * command so `diskpush fleet run deploy-reload` needs no `--on` at all. */ + /** + * How the run is paced. Stored with the command because "reload nginx" and + * "upgrade the database tier" want very different answers, and re-choosing + * them every time is how a saved command still gets run wrong. + */ + concurrency: z.number().int().positive().default(FLEET_DEFAULT_CONCURRENCY), + onFailure: FleetFailureModeSchema.default('continue'), targets: z.array(z.string().min(1)).default([]), tags: z.array(z.string()).default([]), /**