fix(cli): refuse extra prompt arguments instead of dropping them - #1436
Merged
Conversation
`go-code explain this repo` — the natural unquoted form — ran against
`-prompt explain` and silently discarded the rest, exit 0. A truncated
prompt often still produces plausible output, so the loss was invisible.
The prompt branch assigned `prompt="$1"` and never read `$2` onward,
while the CLI branch beside it captures the rest with `cli_args=("$@")`
and the flag branches die on anything unknown. Prompt mode was the only
path that accepted input and threw part of it away.
It now refuses and reconstructs the correct command:
unexpected extra argument: this. Quote the whole prompt as one
argument, e.g. go-code "explain this repo"
Joining the words was considered and rejected — it guesses at intent and
silently reinterprets a shell-splitting mistake, where refusing teaches
the rule once.
Found by the untrusted external reviewer reading only the script, then
confirmed by running it.
Closes #1435
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WJGxhoFhA8JjkwZFcLGdS5
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1435
The bug
and also thisis gone. No warning, exit 0. And the unquoted form — thenatural thing to type — loses almost everything:
Why it hid for so long
The run looks successful, and a truncated prompt often still produces plausible
output, so nothing signals the loss. No test covered multi-argument prompt mode.
Cause
The prompt branch of the argument
casetook only the first positional:The CLI branch immediately above captures the rest with
cli_args=("$@"), andthe flag branches
dieon anything unknown. Prompt mode was the only path thataccepted input and then threw part of it away.
Fix
Refuse, and name the fix:
The message reconstructs the correct command rather than only complaining.
Joining the arguments was considered and rejected. Joining guesses at intent
and silently reinterprets a shell-splitting mistake; refusing teaches the rule
once. It also matches the wrapper's existing style, which rejects unknown flags
rather than ignoring them.
Verification
Red first — the wrapper exited 0 with
harnesscliinvoked on the truncatedprompt:
The test asserts the user-visible consequence — either the whole prompt reaches
harnesscli, or the wrapper refuses and says why — not an internal variable. Itcarries a control asserting a single quoted prompt still passes through whole,
so a fix that refused everything would fail.
All wrapper tests green. The refusal was read in a real terminal, not just
asserted, since the wording is the deliverable here.
How it was found
By the untrusted external reviewer (
gpt-6-astravia the Surplus proxy) readingonly the script, with no tools — then confirmed by running the stub
reproduction rather than by reading.
Honest hit rate on that review: six findings on this file, of which two
confirmed (this, plus a narrow spawn-to-trap orphan window), three
legitimate low-severity hardening (PID reuse in
stop_server, predictable tmppaths, unvalidated
HARNESS_ADDR), and one rejected as speculative (a--serverownership race with no concrete failure, already covered bySTARTED_BY_US). The remaining five are tracked separately rather than bundledin here.
🤖 Generated with Claude Code
https://claude.ai/code/session_01WJGxhoFhA8JjkwZFcLGdS5