diff --git a/.github/skills/bloom-automation/bloomProcessCommon.mjs b/.github/skills/bloom-automation/bloomProcessCommon.mjs index 21d2784acc66..f0fde3c2e00d 100644 --- a/.github/skills/bloom-automation/bloomProcessCommon.mjs +++ b/.github/skills/bloom-automation/bloomProcessCommon.mjs @@ -42,9 +42,40 @@ export const requireTcpPortOption = (optionName, value) => { return port; }; +/** + * A process id given on the command line, as a positive integer. + * + * This throws rather than returning undefined, because the caller that wants a process id wants + * to act on exactly that process. A killer script that read a malformed id as "no id given" would + * fall through to whatever its no-target default is, and killBloomProcess.mjs's default is to kill + * every Bloom the worktree owns. + */ +export const requireProcessIdOption = (optionName, value) => { + const normalized = value === undefined ? "" : String(value).trim(); + const processId = /^\d+$/.test(normalized) + ? toPositiveInteger(normalized) + : undefined; + if (!processId) { + throw new Error( + `${optionName} must be a positive integer process id. Received: ${value}`, + ); + } + + return processId; +}; + +// Whether the caller asked for the usage, wherever the request sits on the command line. Every +// script checks this before it parses anything else: `--repo-root -h` used to store "-h" as +// the path, name no target, and reach the default that kills every Bloom on the machine. +export const asksForHelp = (args) => + args.some((arg) => arg === "--help" || arg === "-h"); + export const requireOptionValue = (args, index, optionName) => { const value = args[index + 1]; - if (!value || value.startsWith("--")) { + // A leading "-" of any length means the next flag, not this option's value. Every value + // these scripts take is a path, a TCP port or a process id, and none of those starts + // with "-", so there is nothing legitimate to reject here. + if (!value || value.startsWith("-")) { throw new Error(`${optionName} requires a value.`); } diff --git a/.github/skills/bloom-automation/bloomProcessStatus.mjs b/.github/skills/bloom-automation/bloomProcessStatus.mjs index cc3df6e285e3..0a072df964ba 100644 --- a/.github/skills/bloom-automation/bloomProcessStatus.mjs +++ b/.github/skills/bloom-automation/bloomProcessStatus.mjs @@ -1,4 +1,5 @@ import { + asksForHelp, buildProcessChain, classifyProcesses, fetchBloomInstanceInfo, @@ -13,8 +14,24 @@ import { toWorkspaceTabsEndpoint, } from "./bloomProcessCommon.mjs"; +const usage = `Report the Bloom and launcher processes this machine is running. + + node bloomProcessStatus.mjs [options] + + --help, -h Print this and exit. + --json Report as JSON. + --running-bloom Also probe each running Bloom's own HTTP server. + --repo-root The worktree to judge instances against (default: this checkout). + --http-port Report the instance whose server answers on this port. + +This script only reads; it never kills anything.`; + const parseArgs = () => { const args = process.argv.slice(2); + if (asksForHelp(args)) { + console.log(usage); + process.exit(0); + } const options = { json: false, runningBloom: false, @@ -25,6 +42,11 @@ const parseArgs = () => { for (let i = 0; i < args.length; i++) { const arg = args[i]; + if (arg === "--help" || arg === "-h") { + console.log(usage); + process.exit(0); + } + if (arg === "--json") { options.json = true; continue; @@ -36,7 +58,11 @@ const parseArgs = () => { } if (arg === "--repo-root") { - options.repoRoot = args[i + 1] || options.repoRoot; + // A required value, checked the same way as every other option's. Taking + // `args[i + 1]` and falling back to the default would swallow the next flag as + // this option's value. killBloomProcess.mjs has the same parser, where that + // mistake is destructive. + options.repoRoot = requireOptionValue(args, i, "--repo-root"); i++; continue; } @@ -57,6 +83,10 @@ const parseArgs = () => { ); continue; } + + // An unknown flag is a mistake, and silently ignoring it hides it. + console.error(`Unknown option ${arg}.\n\n${usage}`); + process.exit(2); } return options; diff --git a/.github/skills/bloom-automation/dismissProblemDialog.mjs b/.github/skills/bloom-automation/dismissProblemDialog.mjs index 4e37b73febc0..fbb46ff52dc9 100644 --- a/.github/skills/bloom-automation/dismissProblemDialog.mjs +++ b/.github/skills/bloom-automation/dismissProblemDialog.mjs @@ -23,6 +23,7 @@ import { createRequire } from "node:module"; import path from "node:path"; import { + asksForHelp, fetchBloomInstanceInfo, getDefaultRepoRoot, normalizeBloomInstanceInfo, @@ -32,8 +33,15 @@ import { toLocalOrigin, } from "./bloomProcessCommon.mjs"; +const usage = + "Usage: node .github/skills/bloom-automation/dismissProblemDialog.mjs --http-port [--wait] [--timeout-ms ] [--json]"; + const parseArgs = () => { const args = process.argv.slice(2); + if (asksForHelp(args)) { + console.log(usage); + process.exit(0); + } const options = { httpPort: undefined, wait: false, @@ -79,12 +87,12 @@ const parseArgs = () => { continue; } - if (arg === "--help") { - console.log( - "Usage: node .github/skills/bloom-automation/dismissProblemDialog.mjs --http-port [--wait] [--timeout-ms ] [--json]", - ); - process.exit(0); - } + // A typo must not be ignored: an option this script does not know is a request it + // cannot carry out, so say so rather than do something else. + console.error(`Unknown option ${arg}. + +${usage}`); + process.exit(2); } if (!options.httpPort) { diff --git a/.github/skills/bloom-automation/driveAiImageEditor.mjs b/.github/skills/bloom-automation/driveAiImageEditor.mjs index 4743bd4de0e6..a9695c4d4cba 100644 --- a/.github/skills/bloom-automation/driveAiImageEditor.mjs +++ b/.github/skills/bloom-automation/driveAiImageEditor.mjs @@ -35,6 +35,29 @@ const { chromium } = createRequire(path.join(componentTester, "package.json"))( ); const args = process.argv.slice(2); + +const usage = `Drive the "Edit with AI…" image editor of a running Bloom over CDP. + + node driveAiImageEditor.mjs [options] [command] + + frames List every frame of the Edit tab (the default command). + images Report the images of the current page. + credits Report each book image's credits, read from the file metadata. + dummy-edit Open the editor, edit with the Local Dummy model, and commit. + + --help, -h Print this and exit. + --http-port The Bloom whose server answers on this port (default: 8092). + --cdp-port The debugging port to attach to (default: --http-port plus 2). + --match Part of the src of the image to edit (default: ai-image). + --shot Where to write the screenshot of a dummy-edit run.`; + +// The usage counts wherever the request sits, and this script attaches to a running Bloom, so +// answer it before anything reaches that Bloom. +if (args.some((arg) => arg === "--help" || arg === "-h")) { + console.log(usage); + process.exit(0); +} + const opt = (name, def) => { const i = args.indexOf(name); return i >= 0 && args[i + 1] ? args[i + 1] : def; @@ -44,8 +67,16 @@ const valueFlags = new Set(["--http-port", "--cdp-port", "--match", "--shot"]); const positional = []; for (let i = 0; i < args.length; i++) { if (args[i].startsWith("--")) { - if (valueFlags.has(args[i])) i++; // skip its value - continue; + if (valueFlags.has(args[i])) { + i++; // skip its value + continue; + } + // A typo must not be ignored: this script attaches to a running Bloom and does things + // to the book being edited, so an option it does not know stops the run. + console.error(`Unknown option ${args[i]}. + +${usage}`); + process.exit(2); } positional.push(args[i]); } diff --git a/.github/skills/bloom-automation/killBloomProcess.mjs b/.github/skills/bloom-automation/killBloomProcess.mjs index 7c97dde452bc..c48b40a9897a 100644 --- a/.github/skills/bloom-automation/killBloomProcess.mjs +++ b/.github/skills/bloom-automation/killBloomProcess.mjs @@ -6,12 +6,42 @@ import { getWindowsProcessSnapshot, killProcessIds, normalizeBloomInstanceInfo, + asksForHelp, requireOptionValue, + requireProcessIdOption, requireTcpPortOption, } from "./bloomProcessCommon.mjs"; +const usage = `Kill the Bloom.exe (and dotnet.exe BloomExe.csproj) processes of this worktree. + + node killBloomProcess.mjs [options] + + --help, -h Print this and exit without killing anything. + --json Report what was killed as JSON. + --only-mismatched Kill only instances whose repo root is not this worktree. + --repo-root The worktree to judge instances against (default: this checkout). + --http-port Kill the instance whose server answers on this port. + --pid Kill this process and the Bloom processes in its chain. + --watch-pid Kill this launcher/watch process and its Bloom processes. + +With no --http-port, --pid or --watch-pid, this kills EVERY Bloom this worktree owns.`; + +// Print the usage and exit 0 without killing anything, or reject an unknown flag with a non-zero +// exit. Reading the usage first must never be the dangerous move: this script used to ignore +// --help and go straight to its destructive default (AUTOMATION-DEBT.md, "Automation helper +// scripts run destructive defaults on unknown flags"). +const exitWithUsage = (unknownArgument) => { + if (unknownArgument) { + console.error(`Unknown option ${unknownArgument}.\n\n${usage}`); + process.exit(2); + } + console.log(usage); + process.exit(0); +}; + const parseArgs = () => { const args = process.argv.slice(2); + if (asksForHelp(args)) exitWithUsage(); const options = { json: false, onlyMismatched: false, @@ -24,6 +54,10 @@ const parseArgs = () => { for (let i = 0; i < args.length; i++) { const arg = args[i]; + if (arg === "--help" || arg === "-h") { + exitWithUsage(); + } + if (arg === "--json") { options.json = true; continue; @@ -35,7 +69,11 @@ const parseArgs = () => { } if (arg === "--repo-root") { - options.repoRoot = args[i + 1] || options.repoRoot; + // A required value, checked the same way as every other option's. Taking + // `args[i + 1]` and falling back to the default would swallow the NEXT FLAG as + // this option's value, so `--repo-root --pid 123` would name no target at all and + // reach the default that kills every Bloom this worktree owns. + options.repoRoot = requireOptionValue(args, i, "--repo-root"); i++; continue; } @@ -58,25 +96,40 @@ const parseArgs = () => { } if (arg === "--pid") { - options.pid = Number(args[i + 1]); + options.pid = requireProcessIdOption( + "--pid", + requireOptionValue(args, i, "--pid"), + ); i++; continue; } if (arg.startsWith("--pid=")) { - options.pid = Number(arg.slice("--pid=".length)); + options.pid = requireProcessIdOption( + "--pid", + arg.slice("--pid=".length), + ); continue; } if (arg === "--watch-pid") { - options.watchPid = Number(args[i + 1]); + options.watchPid = requireProcessIdOption( + "--watch-pid", + requireOptionValue(args, i, "--watch-pid"), + ); i++; continue; } if (arg.startsWith("--watch-pid=")) { - options.watchPid = Number(arg.slice("--watch-pid=".length)); + options.watchPid = requireProcessIdOption( + "--watch-pid", + arg.slice("--watch-pid=".length), + ); + continue; } + + exitWithUsage(arg); } return options; @@ -85,8 +138,14 @@ const parseArgs = () => { const options = parseArgs(); const processState = classifyProcesses(options.repoRoot); const processIds = new Set(); +// Whether the caller named a target, not whether the value we parsed from it is usable. The +// two are the same now that every target option is validated at parse time, and this says the +// intended thing: a caller who asked for one process must never reach the default that kills +// every Bloom this worktree owns. const exactTargetRequested = - !!options.httpPort || !!options.pid || !!options.watchPid; + options.httpPort !== undefined || + options.pid !== undefined || + options.watchPid !== undefined; let targetedInstance; let exactTargetResolutionError; diff --git a/.github/skills/bloom-automation/launcherControl.mjs b/.github/skills/bloom-automation/launcherControl.mjs index c614f481079a..a2d17ac9baee 100644 --- a/.github/skills/bloom-automation/launcherControl.mjs +++ b/.github/skills/bloom-automation/launcherControl.mjs @@ -26,6 +26,7 @@ import { } from "node:fs"; import path from "node:path"; import { + asksForHelp, getDefaultRepoRoot, requireOptionValue, } from "./bloomProcessCommon.mjs"; @@ -43,8 +44,25 @@ const actionNames = [ "--ensure-running", ]; +const usage = `Command the go.sh launcher for this worktree. + + node launcherControl.mjs [options] + + actions: ${actionNames.join(", ")} + options: --json, --wait-ready, --repo-root , --timeout-ms + --help, -h Print this and exit without commanding anything. + + --restart rebuilds and relaunches; --quit-bloom stops Bloom and leaves the launcher; + --shutdown stops Bloom, the launcher, and Vite.`; + const parseArgs = () => { const args = process.argv.slice(2); + // Asking a destructive script how to use it must never be the destructive move, and the + // request counts wherever it sits: see asksForHelp. + if (asksForHelp(args)) { + console.log(usage); + process.exit(0); + } const options = { action: undefined, json: false, @@ -56,6 +74,13 @@ const parseArgs = () => { for (let i = 0; i < args.length; i++) { const arg = args[i]; + // Print the usage and stop, before any action can run. Asking a destructive script how to + // use it must never be the destructive move. + if (arg === "--help" || arg === "-h") { + console.log(usage); + process.exit(0); + } + if (actionNames.includes(arg)) { if (options.action) { throw new Error( diff --git a/.github/skills/bloom-automation/switchWorkspaceTab.mjs b/.github/skills/bloom-automation/switchWorkspaceTab.mjs index 5ef4a7d92b94..0e8c97244bc1 100644 --- a/.github/skills/bloom-automation/switchWorkspaceTab.mjs +++ b/.github/skills/bloom-automation/switchWorkspaceTab.mjs @@ -1,6 +1,7 @@ import { createRequire } from "node:module"; import path from "node:path"; import { + asksForHelp, fetchBloomInstanceInfo, findRunningStandardBloomInstance, getDefaultRepoRoot, @@ -11,8 +12,15 @@ import { toWorkspaceTabsEndpoint, } from "./bloomProcessCommon.mjs"; +const usage = + "Usage: node .github/skills/bloom-automation/switchWorkspaceTab.mjs (--running-bloom | --http-port ) --tab [--json] [--timeout-ms ]"; + const parseArgs = () => { const args = process.argv.slice(2); + if (asksForHelp(args)) { + printHelp(); + process.exit(0); + } const options = { runningBloom: false, httpPort: undefined, @@ -68,19 +76,19 @@ const parseArgs = () => { continue; } - if (arg === "--help") { - printHelp(); - process.exit(0); - } + // A typo must not be ignored: an option this script does not know is a request it + // cannot carry out, so say so rather than do something else. + console.error(`Unknown option ${arg}. + +${usage}`); + process.exit(2); } return options; }; const printHelp = () => { - console.log( - "Usage: node .github/skills/bloom-automation/switchWorkspaceTab.mjs (--running-bloom | --http-port ) --tab [--json] [--timeout-ms ]", - ); + console.log(usage); }; const normalizeTab = (tab) => { diff --git a/.github/skills/bloom-automation/webview2Targets.mjs b/.github/skills/bloom-automation/webview2Targets.mjs index ee6b62fbe3fe..40a8acc49bd9 100644 --- a/.github/skills/bloom-automation/webview2Targets.mjs +++ b/.github/skills/bloom-automation/webview2Targets.mjs @@ -1,4 +1,5 @@ import { + asksForHelp, fetchBloomInstanceInfo, findRunningStandardBloomInstance, normalizeBloomInstanceInfo, @@ -7,8 +8,26 @@ import { toLocalOrigin, } from "./bloomProcessCommon.mjs"; +const usage = `List the WebView2 debugging targets of a running Bloom. + + node webview2Targets.mjs [options] + + --help, -h Print this and exit. + --json Report the targets as JSON. + --all List every target, not only Bloom's own documents. + --running-bloom Find the Bloom that is running, rather than naming a port. + --http-port The Bloom whose server answers on this port. + --host The host to ask for targets (default: localhost). + --port The CDP port to ask directly. + --wait Wait for a target to appear. + --timeout-ms How long to wait (default: 15000).`; + const parseArgs = () => { const args = process.argv.slice(2); + if (asksForHelp(args)) { + console.log(usage); + process.exit(0); + } const options = { host: "localhost", port: undefined, @@ -85,7 +104,15 @@ const parseArgs = () => { if (arg === "--timeout-ms") { options.timeoutMs = Number(args[i + 1] || options.timeoutMs); i++; + continue; } + + // A typo must not be ignored: an option this script does not know is a request it + // cannot carry out, so say so rather than do something else. + console.error(`Unknown option ${arg}. + +${usage}`); + process.exit(2); } return options; diff --git a/src/BloomE2E/AUTOMATION-DEBT.md b/src/BloomE2E/AUTOMATION-DEBT.md index 5eab6dcfb40f..5d04ebc16292 100644 --- a/src/BloomE2E/AUTOMATION-DEBT.md +++ b/src/BloomE2E/AUTOMATION-DEBT.md @@ -21,7 +21,6 @@ and ask its author. | Pull request | Branch | What it pays down | | --- | --- | --- | -| #8290 | `BL-16799-automation-scripts` | The `bloom-automation` scripts answer `--help` without killing anything, and reject an unknown flag or a malformed process id. | | #8291 | `BL-16799-vr-collect-failures` | A visual-regression case collects every failed comparison and fails once at the end. | | #8292 | `BL-16799-component-tests-in-ci` | The component-tester Playwright suites get a nightly job. | | #8293 | `BL-16799-vite-port` | `BLOOM_E2E_VITE_PORT` makes a run test the working tree's front end. Adds a new entry for what remains. | @@ -211,18 +210,6 @@ Known WebView2/CDP behaviors that every ad-hoc script rediscovers the hard way; (Promoted from PAPERCUTS 2026-07-22.) -## Automation helper scripts run destructive defaults on unknown flags - -`node .github/skills/bloom-automation/killBloomProcess.mjs --help` killed the running -Bloom: unknown flags are ignored and the destructive default runs, so "read the usage -first" is itself the dangerous move; sibling scripts may share the shape. Fix -direction: recognize `--help`/`-h` and reject unknown flags in every script that kills -processes — and fold these helpers' jobs into the library's audited launch/teardown -fixture over time. (Promoted from PAPERCUTS 2026-07-24.) - -being fixed on `BL-16799-automation-scripts` (#8290), for the `--help`, unknown-flag and -malformed-process-id part. Folding the helpers into the fixture is not part of it. - ## Adding a page needs the Add Page dialog, which offers nothing to automate against A book made from a template starts with front and back matter only, because every page