Skip to content

fix(cli): classify invalid option values as usage errors - #615

Merged
ScriptedAlchemy merged 7 commits into
mainfrom
fix/cli-invalid-arguments-598
Sep 5, 2026
Merged

fix(cli): classify invalid option values as usage errors#615
ScriptedAlchemy merged 7 commits into
mainfrom
fix/cli-invalid-arguments-598

Conversation

@ScriptedAlchemy

@ScriptedAlchemy ScriptedAlchemy commented Sep 5, 2026

Copy link
Copy Markdown
Owner

What

  • Throw Commander's InvalidArgumentError for invalid --port, --trials, dev --install-host, and install/uninstall <host>, --mode, and --scope values.
  • Cover all five validators plus the shared positional host path with exit-code and Commander-message assertions.
  • Move those values from exit code 1 to exit code 2 in both CLI reference locales while retaining malformed --input / --input-file documents under exit code 1.

Why

Invalid CLI argument values are usage errors and should follow the same Commander path as invalid serve-app --profile, serve-app --allow, and doctor --host values instead of becoming generic AB5000 diagnostics.

Closes #598

Self-review

Reviewer: Claude Fable 5.1 Thinking High (claude-fable-5-1-thinking-high), concrete merge risks only against origin/main.

First pass:

  • No blocking findings.
  • Low: no direct test for the positional install / uninstall <host> parser. Fixed by asserting install windsurf exits 2 with Commander's command-argument message; uninstall shares the same parser.
  • Low: changeset could imply generated installer bins changed. Fixed by qualifying the summary as the agent-bundle CLI.

Second pass after fixes:

  • Verified both first-pass dispositions.
  • Low: changeset omitted the positional install / uninstall <host> argument. Fixed by naming <host> explicitly.
  • No other bugs, breaking changes, missed tests, documentation gaps, or changeset gaps found.

Verification

After merging origin/main at 62b69c068f9cccf4fcad3700287760b7fe44842d (#578):

  • pnpm build
  • pnpm typecheck
  • pnpm lint
  • pnpm test:unit (3,951 tests; 3,945 passed, 6 skipped)
  • pnpm docs:site:build
  • focused cli.test.ts integration run (24/24 passed before the added positional-host assertion; 24/24 passed after it)

@changeset-bot

changeset-bot Bot commented Sep 5, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 9d6c83a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
agent-bundle Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new

pkg-pr-new Bot commented Sep 5, 2026

Copy link
Copy Markdown
npm i https://pkg.pr.new/ScriptedAlchemy/agent-bundle@615
npm i https://pkg.pr.new/ScriptedAlchemy/agent-bundle/create-agent-bundle@615
npm i https://pkg.pr.new/ScriptedAlchemy/agent-bundle/rsc-markdown-stream@615
npm i https://pkg.pr.new/ScriptedAlchemy/agent-bundle/@agent-bundle/runtime@615

commit: 9d6c83a

@ScriptedAlchemy
ScriptedAlchemy marked this pull request as ready for review September 5, 2026 08:13
@ScriptedAlchemy
ScriptedAlchemy enabled auto-merge (squash) September 5, 2026 08:13
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 5, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-05T08:15:47.783677Z c01c611 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c01c611724

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .changeset/quiet-chairs-report.md Outdated

@ScriptedAlchemy ScriptedAlchemy left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks merge-ready after the existing changeset comment is fixed. The behavior change is correct: invalid option/argument values are parser usage errors and should exit 2 rather than surface as AB5000 runtime failures. No architecture concerns from #592/#600.

Only required follow-up I see is the open review thread asking the changeset to explicitly name the AB5000 behavior change; after that, re-run the focused CLI integration test if the head moves.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cli: five option validators throw TypeError (AB5000, exit 1) where Commander's InvalidArgumentError (exit 2) is used elsewhere

1 participant