fix(docs): reject ambiguous tab reads#801
Conversation
|
Codex review: needs maintainer review before merge. Reviewed June 14, 2026, 12:03 AM ET / 04:03 UTC. Summary Reproducibility: yes. Source inspection of current main shows CLI and MCP paths accept both selectors independently, and the PR body includes after-fix live CLI output for the new failure path. Review metrics: 2 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the focused validation if maintainers accept fail-fast behavior for unsupported ambiguous inputs; otherwise preserve and document the existing permissive precedence. Do we have a high-confidence way to reproduce the issue? Yes. Source inspection of current main shows CLI and MCP paths accept both selectors independently, and the PR body includes after-fix live CLI output for the new failure path. Is this the best way to solve the issue? Yes, if maintainers accept the compatibility break. Early validation in the CLI command and MCP argument builder is the narrow repair, with preserving the current permissive behavior as the safer alternative if compatibility wins. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against c57fb9dab6d3. Label changesLabel justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
|
Landed with maintainer hardening in 9cf00cb:
Verification:
Thanks @kiranmagic7. |
Summary
docs cat --tab ... --all-tabsbefore contacting the Docs APIdocs catreadsdocs_getMCP inputs when bothtabandall_tabsare providedBehavior proof
The built CLI exits with a usage error when both tab selectors are provided; there is no auth prompt or API response:
Exit status: 2.
Raw reads hit the same validation:
Exit status: 2.
Verification
go test ./internal/cmd -run "TestDocsCat_RejectsTabWithAllTabs|TestMCPDocsGetRejectsTabWithAllTabs" -count=1go test ./...go vet ./...make buildmake testgit diff --checkNo changelog entry; this is a small validation fix for an ambiguous flag combination.