14 KiB
Alias implementation review remediation
Status: in progress; completed issues are marked individually below.
Review scope and evidence
Reviewed commit a54a9354b1a563849c31dfda906c602950016b02 against the accepted
alias roadmap, implementation plan,
ADR 0013, and applicable architecture,
testing, and documentation policies. The implementation range is
d7cc12a..a54a935.
The root module's default suite passed independently with GOWORK=off.
Both integration default suites, including their maintained examples, passed
in the existing three-module workspace against the current local core.
These checks used Go 1.26.8 with network resolution disabled. Passing suites
do not cover the reproductions below: those failures were confirmed separately
through the public API using a disposable consumer outside the repository.
No production code or tracked tests were changed during review.
Unless stated otherwise, reproduction commands below assume an application
named demo, ordinary string operand conversion, and local success callbacks.
They describe fixtures to construct through the public API, not commands
already installed by the example applications. Use semantic assertions and
supported causes when adding regression tests; do not freeze diagnostic prose
or private representations unnecessarily.
R-1: Protect prefixes of canonical operand space during installation
Status: complete. Priority: P1. Category: canonical command reachability.
Evidence: traceCanonicalPath uses acceptsOperandCount, which requires the source's entire remaining token count to meet the command's minimum. This misses an alias source that becomes a prefix of a valid invocation when more operands follow.
With canonical run ARG{2..2} and a separate runnable inspect accepting an
optional repeated operand, installation accepts:
commandkit.Alias("run first", "inspect")
demo run first second then runs inspect, passing second as its operand;
the original valid run invocation is no longer reachable with that input.
The public probe returned status 0 from the wrong handler.
Required work: Check whether a source can intercept a valid canonical
invocation, including prefixes completed by later operands, after normal
canonical child selection. Do not require that the alias source already
satisfy the command's minimum operand count. Apply this against the complete
candidate in both registration orders and preserve failed-batch atomicity.
Keep the selected distinction between canonical operand space and explicit
longer aliases beneath alternate spellings such as r → run.
Completion evidence: Reject the example at construction and when either side is added later; preserve the preexisting registry on rejection. Include multi-required and repeated operands and a valid distinct child/shortcut case so the correction does not reserve every route beneath any runnable ancestor.
Completed: Installation now protects operand prefixes that can be completed by later arguments without requiring the minimum count immediately. New regressions failed before the correction and pass for fixed/repeated operands, construction, both later registration orders, and rollback. Existing child and longer-shortcut cases and the complete offline root suite passed on Go 1.26.8.
R-2: Select canonical routing context before validating operand counts
Status: complete. Priority: P2. Category: unintended parsing restriction and identity.
Evidence: resolveEnteredRoute also uses the complete
operand-count predicate from R-1, but it examines only tokens before the first
flag. This is not enough information to determine execution arity when flags
and operands are interspersed. A known canonical command can therefore become
routeUnknown, disabling its aliases and discarding its identity.
With canonical run ARG{2..2}, r → run, and scoped
--fmt → --format:
| Invocation | Observed result |
|---|---|
demo r first --fmt=yaml second |
Status 2: unknown --fmt, although both operands are supplied. |
demo r first --format=yaml second |
Handler succeeds but sees EnteredPath() == "r first" and CanonicalPath() == "". |
demo r first --help |
Status 0 with application-root help instead of run help. |
This introduces an unrequested prerequisite: enough operands must appear before the first option for routing metadata and flag aliases to work. It also contradicts the selected exclusion of operands from invocation paths.
Required work: Separate structural canonical selection from execution operand-cardinality validation and from installation's operand-interception check. Retain the selected command, its spelling context, and the actual entered command span even when execution operands are incomplete or invalid. Let the existing parser/input-preparation boundary validate operands after all tokens are interpreted. Help must continue to skip missing execution-required operands without constructing application inputs.
Completion evidence: The valid interspersed invocation succeeds with
entered path r and canonical path run; partial-operand help describes that
command and recognizes its applicable aliases. Test canonical and shortcut
forms, and ensure genuinely invalid execution arity still produces usage errors.
Completed: Canonical tracing now selects runnable commands structurally; installation alone applies the operand-prefix capacity check. Runtime routing keeps command identity and applicable flag aliases for partial or excess operands, while input preparation retains arity validation. Regressions cover interspersed canonical/alias flags, incomplete help, invalid arity, path metadata, and discovery beneath a zero-operand runnable command. The excess-operand regression failed before this change; the full offline root suite and the additional discovery check passed on Go 1.26.8.
R-3: Preserve exact target lookup for the explicit help command
Status: open. Priority: P2. Category: help grammar regression.
Evidence: renderHelpTargets reuses execution lookup
and accepts routeCanonical results even when remaining target tokens are
ordinary operands. It then renders the canonical view and drops those tokens.
For the run fixture above:
demo help r first second
returns status 0 and the help for run. first second are not registered
command components. This contradicts the exact help-path contract
and conflates execution operands with a help target's command path.
Required work: Use an exact-target mode or explicit consumed-token result
for help lookup. After alias resolution, all target tokens must identify a
canonical command/group or a supported alias-prefix discovery view. Do not
silently accept execution operands as part of an explicit help path. Preserve
ordinary r first second --help behavior and help for commands whose execution
requires operands.
Completion evidence: Canonical and aliased operand-bearing explicit help targets fail with usage status 2; exact aliases, group shortcuts, and discovery prefixes still render help without application initialization.
R-4: Resolve discovery-help contexts through group shortcuts
Status: open. Priority: P2. Category: incomplete alias composition.
Evidence: Discovery contexts are indexed by declared source prefixes in discoverFlagContexts, while resolveEnteredRoute searches them using the original entered path only. A group shortcut can reach a valid alias-only prefix whose entered spelling has no separate context entry.
With canonical group box, canonical inspect, and:
pk → box
box admin pick → inspect
demo box admin --help succeeds and lists pick → inspect, but both
demo pk admin --help and demo help pk admin fail with status 2.
The complete pk admin pick route remains available through composition.
Required work: Select discovery metadata through resolved group-prefix composition while retaining the entered spelling for presentation. Reuse the same rule precedence as execution, including more-specific mappings. Keep discovery contexts separate from canonical command nodes and restrict their flags to core help plus applicable global aliases.
Completion evidence: Both help forms work through the group shortcut,
show pk admin in usage, and list the routes actually reachable there.
Cover the global help alias and a more-specific route override; do not add
canonical groups or inherit descendant flags to make discovery work.
R-5: Include alias routes in help for groups without canonical children
Status: open. Priority: P2. Category: help discovery gap.
Evidence: routeHelpAliases returns immediately when the selected canonical node has no canonical children. Alias records are separate from those children, so this suppresses valid routes beneath an otherwise empty group.
With Group("box", "Box group."), canonical inspect, and
box pick → inspect, demo box pick is available, but demo box --help
lists no pick route. The same problem affects help through pk → box.
Required work: Determine relevant help aliases independently of whether the canonical group has children. Preserve the separate display of canonical commands and routing records; do not install fake children to fill the view.
Completion evidence: Canonical and shortcut help for an empty group list its explicit alias routes and their resolved destinations, while groups with no such routes retain their ordinary help behavior.
R-6: Keep the flag interpretation pass consistent with value consumption
Status: open. Priority: P2. Category: canonical/alias parsing divergence.
Evidence: interpretFlagTokens refuses to
consume a following -- as the value of a value-taking flag in both the long
and short branches. It subsequently treats that token as the option terminator
and stops resolving aliases. The backend still consumes -- as the pending
flag's value, so subsequent tokens are interpreted differently by the two passes.
For value flag --format and alias --fmt → --format:
demo show --format -- --fmt red # status 2: unknown --fmt
demo show --format -- --format red # status 0
The canonical and aliased second occurrences should feed the same binding.
Required work: Preserve value-taking arity consistently between alias
interpretation and backend parsing, including when the consumed value is --.
Treat a token as an option terminator only when it occupies that grammatical
role. Do not address this by banning a previously accepted flag value or by
introducing a different operand/option convention.
Completion evidence: The paired invocations have equivalent outcomes and
values. Cover canonical and aliased long/short spellings, and preserve ordinary
termination when -- occurs after a completed value or boolean flag.
R-7: Retain entered flag spellings in parser-level usage failures
Status: open. Priority: P3. Category: incomplete diagnostic adaptation.
Evidence: Typed flag conversion uses occurrence provenance, but
Session.Parse returns normalized backend
failure text directly, and rawValue.lastBool
hardcodes --help. Verified examples:
- Through
r → run, a missing value for--fmt → --formatreports onlyflag needs an argument: --format, omitting the entered flag spelling. - With
--hlep → --help,demo show --hlep=badreports an invalid value for--help, omitting--hlep; canonical short-h=badlikewise loses-h.
The roadmap and CLI reference require usage diagnostics to retain the entered spelling, not only for the later typed-conversion path.
Required work: Carry the relevant occurrence context through missing-value and built-in-help conversion failures. Retain the entered spelling, optionally identifying the canonical target, using neutral adapter records rather than exposing backend error types or making brittle whole-message substitutions. Keep the existing usage classification and delivery behavior.
Completion evidence: Check entered spellings for missing long/short alias values and invalid help values, including an earlier invalid occurrence followed by a valid overwrite. Assert the identifying fragments and existing causes where supported; no new exact diagnostic format is required.
R-8: Reconcile the completed feature roadmap's historical descriptions
Status: open. Priority: P3. Category: documentation drift.
Evidence: aliases.md now declares implementation complete
but still has a Current baseline section saying the public model has no alias
layer, calls the implemented declarations planned APIs, and describes the
old -c= behavior as current. The consumer guide also calls the temporary
roadmap the routing-contract record despite linking the maintained CLI reference
next to it.
Required work: Label the preimplementation baseline and old behavior as historical, distinguish delivered implementation from this open review work, and describe the now-implemented APIs accurately. Point consumer contract navigation to GoDoc/CLI owners; retain the roadmap as planning/history, not a second contract owner. Keep completed stage evidence historical and do not restart completed implementation stages or rewrite published release notes.
Completion evidence: Roadmap status, baseline, and API wording agree; consumer navigation identifies the canonical owners and the open remediation work is discoverable without claiming that these findings are already fixed.