fix(parser): reject ambiguous semantic signatures

[steipete]

Summary

• reject duplicate semantic labels independently for arguments, options, and flags after option-group flattening
• reject required positional arguments that follow optional positionals, which otherwise cannot be expressed unambiguously
• preserve aliases as multiple spellings on a single validated definition and retain full nested command-path errors
• document the four new typed CommanderError cases as part of the existing 0.3.0 breaking train

Proof

• focused validation tests: 13
• full Debug and Release suites: 64/64 each
• external-module exhaustive public API fixture
• Debug build-tests and DocC archive
• SwiftFormat lint and strict SwiftLint: zero violations
• source-blind release CLI validation: 5/5 clauses plus 4/4 anti-cheat probes
• final P0-P2 review clean at 0.94

Downstream exhaustive switches must add the four documented cases. CommanderProgramError is unchanged.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed August 13, 2026, 4:27 AM ET / 08:27 UTC.

ClawSweeper review

What this changes

The PR rejects duplicate argument, option, and flag labels plus required positional arguments declared after optional ones, and documents four new typed validation errors.

Merge readiness

⚠️ Ready for maintainer review - 3 items remain

Keep open for normal owner review. No patch defect was found; merging intentionally expands the public CommanderError enum as part of the stated 0.3.0 breaking train.

Priority: P2
Reviewed head: b7a536f1446d217624c87d32a68156a23ac14612
Owner decision: Required. See Decision needed.

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	The patch is narrowly implemented with focused coverage; release compatibility is the remaining maintainer choice.
Proof confidence	🌊 off-meta tidepool	Not applicable: This owner-authored PR is outside the external-contributor real-behavior-proof gate; its body reports focused and full-suite validation.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Not applicable	Not applicable: This owner-authored PR is outside the external-contributor real-behavior-proof gate; its body reports focused and full-suite validation.
Evidence reviewed	4 items	Shared validation boundary: The PR validates arguments before building option and flag lookups, rejecting duplicate semantic labels and required-after-optional positionals at the common parser boundary. Both entry paths are covered: Current main flattens signatures in CommandParser, while Program validates each descriptor signature before route resolution; the new checks therefore apply consistently. Feature provenance: Current signature preflight validation was introduced on main by the immediately preceding parser-validation commit, which git blame attributes throughout the validation file to Peter Steinberger.
Findings	None	None.
Security	None	None.

How this fits together

Commander flattens command and option-group definitions into a signature used by both direct parsing and program-level routing. Validation runs before tokens are consumed, returning typed errors for invalid definitions.

flowchart LR
A[Command definitions] --> B[Flattened signature]
B --> C[Signature validation]
C --> D{Definition valid?}
D -->|Yes| E[Parser or program routing]
D -->|No| F[Typed validation error]

Loading

Decision needed

Question	Recommendation
Should the four new public CommanderError cases ship in the planned 0.3.0 breaking release?	Ship in 0.3.0: Accept the documented breaking error expansion and require downstream exhaustive switches to add the four cases.

Why: The implementation is coherent, but accepting a public enum expansion is an API-versioning choice that automated review cannot make.

Before merge

• Resolve merge risk (P1) - Existing downstream exhaustive switches over the public CommanderError enum will need source updates for the four added cases.
• Complete next step (P2) - A release owner must accept the documented public enum expansion before merge; no mechanical repair is needed.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Diff scope	8 files affected; +290 / -11 lines	The change is focused on parser validation, public errors, documentation, and regression coverage.
Production versus test growth	production +46, tests +228	Most added code is focused coverage for the new validation contract.

Merge-risk options

Maintainer options:

1. Confirm the 0.3.0 API break (recommended)
   Merge with the documented public error expansion once the release owner accepts the required downstream switch updates.
2. Keep source compatibility
   Pause this PR and revise its public error representation if the planned release cannot take a source-breaking enum change.

Technical review

Best possible solution:

Land this as the documented 0.3.0 breaking API change, with downstream consumers updating exhaustive CommanderError switches.

Do we have a high-confidence way to reproduce the issue?

Yes—current parsing stores options by label, so differently spelled definitions sharing a label converge on one output key, and optional-before-required positionals cannot be bound unambiguously by position.

Is this the best way to solve the issue?

Yes for the validation defect: placing the checks in the flattened shared signature index covers direct parsing and Program routing. The remaining question is acceptance of the public 0.3.0 error-enum expansion.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 756cfce9a127.

Labels

Label changes:

• add P2: The change affects invalid command-definition handling and public parser errors, with a bounded downstream impact.
• add merge-risk: 🚨 compatibility: Four new public enum cases can break downstream exhaustive CommanderError switches during upgrade.
• add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
• add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: This owner-authored PR is outside the external-contributor real-behavior-proof gate; its body reports focused and full-suite validation.

Label justifications:

• P2: The change affects invalid command-definition handling and public parser errors, with a bounded downstream impact.
• merge-risk: 🚨 compatibility: Four new public enum cases can break downstream exhaustive CommanderError switches during upgrade.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: This owner-authored PR is outside the external-contributor real-behavior-proof gate; its body reports focused and full-suite validation.

Evidence

What I checked:

• Shared validation boundary: The PR validates arguments before building option and flag lookups, rejecting duplicate semantic labels and required-after-optional positionals at the common parser boundary. (Sources/Commander/CommandSignatureValidation.swift:20, b7a536f1446d)
• Both entry paths are covered: Current main flattens signatures in CommandParser, while Program validates each descriptor signature before route resolution; the new checks therefore apply consistently. (Sources/Commander/Parser/CommandParser.swift:21, 756cfce9a127)
• Feature provenance: Current signature preflight validation was introduced on main by the immediately preceding parser-validation commit, which git blame attributes throughout the validation file to Peter Steinberger. (Sources/Commander/CommandSignatureValidation.swift:20, 756cfce9a127)
• Release baseline: The current main validation work is newer than the latest shipped v0.2.4 release; no tag contains the current main commit, so this PR targets the unreleased breaking train. (CHANGELOG.md:1, 756cfce9a127)

Likely related people:

• steipete: Current signature validation and the adjacent parser preflight work on main are attributed to Peter Steinberger; this PR extends that same boundary. (role: introduced current validation boundary; confidence: high; commits: 756cfce9a127, ba5d67fb49bc; files: Sources/Commander/CommandSignatureValidation.swift, Sources/Commander/Program.swift, Sources/Commander/Errors.swift)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Confirm that the planned 0.3.0 release accepts the public CommanderError expansion.

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.