Make the bloom-automation scripts safe to ask for help (BL-16799)

.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.`);
     }
 

.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 <path>    The worktree to judge instances against (default: this checkout).
+  --http-port <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;

.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 <port> [--wait] [--timeout-ms <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 <port> [--wait] [--timeout-ms <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) {

.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 <port>      The Bloom whose server answers on this port (default: 8092).
+  --cdp-port <port>       The debugging port to attach to (default: --http-port plus 2).
+  --match <text>          Part of the src of the image to edit (default: ai-image).
+  --shot <path>           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]);
 }

.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 <path>      The worktree to judge instances against (default: this checkout).
+  --http-port <port>      Kill the instance whose server answers on this port.
+  --pid <pid>             Kill this process and the Bloom processes in its chain.
+  --watch-pid <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;
 

.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 <action> [options]
+
+  actions: ${actionNames.join(", ")}
+  options: --json, --wait-ready, --repo-root <path>, --timeout-ms <n>
+           --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(