fix(cli): accept the documented argument order on every command - #112
Conversation
`mir share <machine> --ttl 1h` — the order the command's own usage string shows — was refused. Go's flag.FlagSet stops at the first positional, so `--ttl 1h` landed in fs.Args() and tripped the arity check; only the flags-first spelling parsed. The same trap hit `mir attach <name> --dir X` (#45), and G1d had fixed it for `machine revoke` alone with a bespoke argument peel. One mechanism now, not one per command: parseArgs (cli/shared.go) parses flags and positionals in any order and every command that takes both goes through it — attach, share, share revoke, join, pair, machine rename, machine revoke. `mir run` keeps flags-first on purpose: everything after <machine> is the remote command, so `mir run box ls -la` must hand `-la` to ls; its usage line now says so. Usage strings print the running binary instead of a hardcoded "mir", so the mir-agent shim reads right. Consent gates are untouched: `share --write` still makes you type the machine name, `share` still refuses without a TTY, `machine revoke` still asks or wants --yes. Copy: the two attach banners described the detach gesture differently and each omitted what the other said. They now share an opening — client.AttachHint, "attached to X — Ctrl-C goes to the shell" — and each adds only the way out it really has: closing the client for a bare `mir attach`, Ctrl-O then d from the overview, which is the one place that gesture exists. Tests: a table over the real dispatch runs both orders for every command touched, so this class cannot regress a third time. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KeiotDVE94wEzvc7wcvm1y
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 10c566d731
ℹ️ 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".
| if a == "--" { | ||
| args, literal = args[:i], args[i+1:] | ||
| break |
There was a problem hiding this comment.
-- when it is a flag's argument
When a non-boolean option legitimately has the literal value --—for example, mir share --session -- box or mir attach --dir -- box—Go's documented -flag x syntax consumes that token as the option value. This unconditional pre-scan instead removes it as a terminator before FlagSet.Parse can interpret the surrounding flag, leaving the option without an argument and exiting with a parsing error; detect the terminator while respecting tokens consumed as flag values so these previously valid flags-first invocations continue to work.
Useful? React with 👍 / 👎.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
1 participant
mir share <machine> --ttl 1h— the exact order the command's own usage string documents — did not work. Go'sflag.FlagSet.Parsestops at the first positional, so--ttl 1hlanded infs.Args()and tripped the arity check; onlymir share --ttl 1h <machine>parsed. A user following our own help text got a refusal.This is the third outing for the same trap: #45 hit it on
mir attach <name> --dir X, and G1d solved it formachine revokealone with a bespoke argument peel. So the fix is one mechanism, applied everywhere, plus a test that walks every command.The mechanism
parseArgs(go/internal/cli/shared.go) parses a FlagSet against args and returns the positionals, accepting flags and positionals in any order — it re-parses after each positional until both sides of every positional are consumed. A literal--still ends flag parsing. Every command that takes positionals AND flags now goes through it, so the bug class cannot come back one command at a time.Audit — every subcommand that mixes positionals and flags
mir attach <machine…> [flags]attach box --dir X--dirmir run [flags] <machine> <command…>run --dir X box ls -lamir share <machine> [--ttl …]share box --ttl 1hmir share revoke <id> [flags]share revoke abc123 --dir Xmir share ls [flags]share ls --dir Xmir join <code> [flags]join CODE --dir Xmir pair [<code>] [flags]pair CODE --dir Xmir machine rename <name> <new> [flags]machine rename box newbox --dir Xmir machine revoke <name> [--yes]machine revoke box --yesmachine revoke --yes box --dir Xstill failedmir up,list/ls,doctor,keygen,add-machine,enroll,pair-dev,update,identity …,wallet …mir-signalmir runstays flags-first on purpose: everything after<machine>is the remote command, somir run box ls -lamust hand-latols, not to mir. Its usage line now says exactly that, and a test pins it.Usage strings
Corrected where they were wrong or hardcoded:
runnow documents flags-first and that the tail is the remote command;attach,join,machine …, andshareprint the running binary (a.binary) instead of a literalmir, so themir-agentshim reads right.Detach copy
The two attach entry paths described the gesture differently, and each dropped what the other said. They now share an opening —
client.AttachHint:attached to <name> — Ctrl-C goes to the shell— and each adds only the way out it actually has:mir attach box:[mir] attached to box — Ctrl-C goes to the shell; close the client to detach (the session keeps running)miroverview:[mir] attached to box — Ctrl-C goes to the shell; Ctrl-O then d comes back to your machinesThe overview's gesture exists only while the overview is running, so a bare attach does not claim it. The guide's
Ctrl-O dis now spelledCtrl-O then d, matching the banner.Consent gates unchanged
share --writestill makes you type the machine name;sharestill refuses without a TTY;machine revokestill asks, or wants--yes. Two tests pin the share refusals in both argument orders.Tests
go/internal/cli/argorder_test.go— a table over the real dispatch runs the documented order AND Go's native flags-first order for every command touched, asserting each reaches the same honest refusal and never a usage line. Verified the new rows fail on the pre-fix code (7 documented-order rows plus the share/TTY test) and pass after. Plus a unit table onparseArgs(flags before/after/between positionals,--terminator) and a copy guard onAttachHint.cd go && go test ./...green,gofmt -l .empty,go vet ./...clean,cd web && npm test167/167. Notestdata/vector changes.🤖 Generated with Claude Code
https://claude.ai/code/session_01KeiotDVE94wEzvc7wcvm1y
Note
Low Risk
CLI parsing and user-facing help only; consent gates and
run's remote-command semantics are explicitly preserved and regression-tested.Overview
Fixes invocations like
mir share box --ttl 1hthat matched help text but failed because Go'sflagpackage stops parsing at the first positional. A sharedparseArgshelper re-parses after each positional so flags can appear before, after, or between arguments (with--still forcing the rest to stay positional).parseArgsis wired into attach, share, share revoke, join, pair, and machine rename/revoke (replacing ad-hoc peeling on revoke).mir runstays flags-first on purpose so remote command flags are not swallowed; its usage line documents that. Several usage strings now usea.binaryinstead of a hardcodedmir.Attach stderr copy is split:
client.AttachHintcovers machine name and Ctrl-C; bare attach says to close the client to detach, overview attach adds Ctrl-O then d, and the guide uses the same wording.argorder_test.gotables real dispatch for documented vs flags-first argv (must reach real errors, neverusage:), unit-testsparseArgs, and checks share TTY consent andAttachHintcopy.Reviewed by Cursor Bugbot for commit 10c566d. Bugbot is set up for automated code reviews on this repo. Configure here.