Make the bloom-automation scripts safe to ask for help (BL-16799) - #8290
Conversation
`node killBloomProcess.mjs --help` killed the running Bloom. Unknown flags were ignored, so the destructive default ran, which made "read the usage first" the dangerous move. The sibling scripts shared the shape. Every script in .github/skills/bloom-automation now recognizes --help and -h, prints its usage, and exits without killing anything. An unknown option is rejected with the usage rather than ignored, and a --pid or --watch-pid that is not a positive integer is rejected instead of being passed to the kill code, where a NaN would have meant "every Bloom this worktree owns". A help request is answered wherever it sits on the command line, which took two more changes. `requireOptionValue` rejects any value starting with "-", because `killBloomProcess.mjs --repo-root -h` otherwise stored "-h" as the repository path, named no target, and reached that destructive default; every value these scripts take is a path, a TCP port or a process id, so nothing legitimate is lost. And `asksForHelp(args)` runs before each parse loop can consume the request as some option's value. `switchWorkspaceTab.mjs`, `dismissProblemDialog.mjs`, `webview2Targets.mjs` and `driveAiImageEditor.mjs` needed the help and the unknown-option check as well: two had no help at all, and two took `--help` only. So the claim above holds for all eight scripts. Retires the AUTOMATION-DEBT.md entry "Automation helper scripts run destructive defaults on unknown flags". Folding these helpers into the launch fixture, the other half of that entry's fix direction, is not part of this. Verified by running each script with `--nonsense`, with `-h`, and with `-h` in the second position, and by checking that a real option still parses. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
5f725ba to
7cf9a58
Compare
Master reworked the head of src/BloomE2E/AUTOMATION-DEBT.md while this branch was open: the table of the BL-16799 stack now gives each pull request number, it lists two branches that are not open yet, and the visual-regression baseline entry is gone. This branch, meanwhile, deletes its own row from that table and deletes the entry it pays off. The merge takes master's text and applies both of those deletions to it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BaWw4EHWrbFF2fNJvMJVC8
| // answer it before anything reaches that Bloom. | ||
| if (args.some((arg) => arg === "--help" || arg === "-h")) { | ||
| console.log(usage); | ||
| process.exit(0); |
There was a problem hiding this comment.
[Devin] Bug: Help requires installed Playwright
Without Playwright installed, --help fails during module loading before the new help check runs. No usage text is printed.
(.github/skills/bloom-automation/driveAiImageEditor.mjs:58)
| continue; | ||
| if (valueFlags.has(args[i])) { | ||
| i++; // skip its value | ||
| continue; |
There was a problem hiding this comment.
[Devin] Bug: Malformed options still commit edits
dummy-edit --shot --typo skips --typo as a value, while dummy-edit -x treats -x as positional. Both runs still commit the edit.
(.github/skills/bloom-automation/driveAiImageEditor.mjs:72)
| const normalized = value === undefined ? "" : String(value).trim(); | ||
| const processId = /^\d+$/.test(normalized) | ||
| ? toPositiveInteger(normalized) | ||
| : undefined; |
There was a problem hiding this comment.
[Devin] Investigate: Process IDs allow numeric rounding
Values above Number.MAX_SAFE_INTEGER pass validation but round before reaching taskkill. Bound process IDs to the platform range or safe integers.
(.github/skills/bloom-automation/bloomProcessCommon.mjs:57)
|
[Devin] Investigate: PR targets the disallowed branch The PR targets ( |
|
[Claude Opus 5 from Hatton's machine during devin-review] Consulted Devin on 2026-09-03 22:45 UTC up to commit |
1 participant
node killBloomProcess.mjs --helpkilled the running Bloom. Unknown flags wereignored, so the destructive default ran, which made "read the usage first" the
dangerous move. The sibling scripts shared the shape.
Every script in .github/skills/bloom-automation now recognizes --help and -h,
prints its usage, and exits without killing anything. An unknown option is
rejected with the usage rather than ignored, and a --pid or --watch-pid that is
not a positive integer is rejected instead of being passed to the kill code,
where a NaN would have meant "every Bloom this worktree owns".
Retires the AUTOMATION-DEBT.md entry "Automation helper scripts run destructive
defaults on unknown flags". Folding these helpers into the launch fixture, the
other half of that entry's fix direction, is not part of this.
This is one of eleven stacked pull requests (BL-16799)
Each one pays down one entry of
src/BloomE2E/AUTOMATION-DEBT.md, and each branches off the one before it. Base:master. Review only this pull request's own commit; the ones below it are reviewed in their own pull requests. The first six change test and tooling code only; the last five also change product code.BL-16799-automation-scripts— Make the bloom-automation scripts safe to ask for helpBL-16799-vr-collect-failures— Report every failed image comparison in a visual-regression case, not the firstBL-16799-component-tests-in-ci— Run the component-tester Playwright suites nightlyBL-16799-vite-port— Let an e2e run test the working tree's front endBL-16799-type-in-one-call— Type into a text box in one call, not one key press per characterBL-16799-page-screenshot— Capture a whole book page from a testBL-16799-toolbox-registration— Register the toolbox tools from one list both callers shareBL-16799-shell-document— Stop a test attaching to a shell document Bloom does not driveBL-16799-tab-test-ids— Click a workspace tab by a test id, not by its localized labelBL-16799-page-change— Refuse a page change the Edit tab cannot do, and wait before askingBL-16799-collection-languages— Set a collection's languages through an e2e hook, not by writing XMLReplaces #8276, which did all of this in one pull request.
Verification of the whole stack, at its tip: the C# suite passes (3338 passed, 13 skipped), the front-end vitest suite passes (781 passed, 5 skipped), and the
src/BloomE2Esuite passes against a Vite dev server on the working tree (36 passed, 0 skipped, 8.2 minutes). Each pull request also has its own type check and lint.🤖 Generated with Claude Code
Devin review
This change is