11 KiB
Alias interaction audit remediation
Status: complete; R-1 through R-3 are addressed and marked individually below.
Review scope and evidence
Reviewed commit 08ad7427e2aefa54170c0fa50ef760fe187e3cc2, including the alias
implementation and its subsequent corrections, against CommandKit's existing
dispatch, help/version, binding, integration, and maintained-example behavior.
The CLI reference, public GoDoc, accepted
ADR 0013, and applicable policies
remain the contract owners. This review does not reopen completed implementation
stages or select additional public APIs.
Validation used Go 1.26.8 with network resolution disabled:
- The uncached root default suite passed with
GOWORK=off. - Both integration default suites and their maintained examples passed in the existing workspace against the local core. This is workspace composition evidence, not independent module-release validation. Their manifests still reference published core v0.2.0; the deliberate core-first publication sequence remains described in the prepared core release note.
- The core example's
ws greet --times=0 --tag=first --tag second, composed example'spk show prompt briefing --campaign spring --fmt yaml, and simple ConfigKit example's two-fileconfig diffcompleted successfully. - Disposable consumers outside the repository confirmed the findings below through the public API. Additional checks passed for aliases to built-in help and version, a scoped version-format alias, and the global help alias.
- The runnable-parent failure in R-3 was also reproduced against an exported
pre-alias baseline at
f3b8a129d99d04230f998d0327ffad05f6002d7a.
No production code or repository tests were changed during the audit. The reproductions below describe small fixtures to assemble through the public API; they are not commands already installed in the maintained examples. Assertions for remediation should cover CommandKit-owned routing, presentation, and callback behavior, without retesting upstream domain semantics or fixing whole diagnostic messages as contracts.
R-1: Include intermediate alias prefixes in help discovery
Status: complete. Priority: P2. Category: alias-chain composition.
Evidence: resolveEnteredRoute resolves the entered path to its final form, then looks for discovery contexts using only that final path and the original path. Intermediate prefixes are lost. routeHelpAliases similarly searches only the entered and canonical prefixes, and discoveryHelpAliases searches only the entered and selected discovery prefixes. Execution can select a longer mapping at an intermediate step that these help views never examine.
With canonical empty group box, canonical runnable inspect, and:
q → box
p → q
q pick → inspect
demo q --help lists pick → inspect; demo p --help omits it, even though
demo p pick successfully executes inspect.
Replace the last mapping with q admin pick → inspect. Then:
demo q admin --help # status 0; lists pick → inspect
demo p admin --help # status 2; unexpected group operand admin
demo help p admin # status 2; unknown help path
demo p admin pick # status 0; executes inspect
The existing single-shortcut regressions exercise aliases beneath the final canonical group. Execution-chain tests also pass. Neither covers help for routes declared beneath an intermediate shortcut.
Required work: Make help selection and route discovery account for relevant prefixes reached through alias composition. Display destinations reachable under actual longest-match precedence, including entered-prefix overrides. Preserve the original entered path and keep discovery separate from canonical command nodes. Intermediate rewrites need not become public invocation metadata, and help need not enumerate every possible composed spelling.
Completion evidence: Both help forms work for the chained discovery example; group help includes the explicitly declared route reachable through the chain. Cover an overriding mapping and applicable global help aliases while preserving descendant flag exclusion and metadata-only help. Execution and help must agree on the displayed destinations.
Completed: Discovery selection now checks intermediate rewrite paths when the entered and final paths do not supply a context. Canonical and discovery help share alias listing across the resolution chain and resolve each displayed suffix through actual execution precedence. New public-boundary regressions failed before the correction and now cover group/discovery help, expanded chains, entered overrides, both help forms, global help aliases, descendant flag exclusion, metadata-only help, and matching execution destinations. The complete offline root suite passed on Go 1.26.8. No canonical nodes or public invocation metadata were added.
R-2: Retain discovery-prefix context in usage diagnostics
Status: complete. Priority: P3. Category: presentation consistency.
Evidence: Discovery parsing uses the backend root with only trailing flag tokens, so parser failures have an empty canonical path. execution.present adapts usage only for canonical alias routes. For discovery routes, backend root usage is displayed instead; execution.usageLine also falls back to root usage when given a discovery path that is not a canonical node.
With canonical inspect, list things → inspect, and --hlep → --help:
demo list --help # status 0; help identifies demo list
demo list --missing # status 2; unknown flag, then Usage: demo [flags] [command]
demo list --hlep=bad # status 2; invalid --hlep, then root usage again
The latter two diagnostics contain no indication that the entered command path
was list. A bare demo list identifies list in its explanatory message but
still appends root usage. This differs from the entered-path diagnostic behavior
documented in the CLI contract.
Required work: Present usage from the selected discovery context, retaining the entered prefix for incomplete-prefix and parser failures. Do not assign a canonical command identity to a discovery view or add synthetic canonical nodes. Preserve usage status 2, entered flag spelling, and existing delivery handling.
Completion evidence: Bare-prefix, unknown-flag, and invalid-help-value cases
identify demo list in usage. Repeat through a composed group shortcut after
R-1. Check identifying fragments rather than exact formatting, and confirm that
ordinary root and canonical command diagnostics retain their existing behavior.
Completed: Usage presentation now uses the selected discovery prefix rather than asking the canonical backend for its usage. Regressions first failed for all three error forms at both direct and chained prefixes, and now retain the entered prefix and flag spelling without inventing a canonical identity. Usage status 2, completed diagnostic delivery, metadata-only failures, and ordinary root/canonical usage remain covered. The complete offline root suite passed on Go 1.26.8.
R-3: Preserve empty and dash operands before child-name tokens
Status: complete. Priority: P2. Category: inherited parser/routing inconsistency.
Evidence: Session.Parse gives the entire
leading non-option token sequence to Cobra's Find. Cobra's command search
filters out empty strings and the single - token while searching for child
commands. It can therefore look past the first operand and select a child whose
name appears later in the operand list. The subsequent exact-prefix check
rejects that selection instead of parsing the parent's operands.
Core's structural route selection stops at the first token that is not a canonical child and correctly identifies the runnable parent. The backend then independently makes a different selection. Alias metadata and flag context can describe the parent while parsing reports a child-path error.
Fixture: runnable run with repeatable string operand ARG accepting zero or
more values, a runnable zero-operand child run child, and shortcut r → run:
demo run first child # status 0; parent receives ["first", "child"]
demo run "" child # status 2; exact command-path error, child usage
demo r "" child # status 2; same parser error, parent alias usage
demo run - child # status 2; exact command-path error, child usage
demo r -- "" child # status 0; parent receives ["", "child"]
Empty strings and - are ordinary operands here, not options or registered
child names. They must establish the operand boundary just as first does.
The canonical failures also occur at the pre-alias baseline identified above;
this is not a newly introduced alias regression. It remains relevant because
the new routing layer and the existing parser disagree on the selected command,
and both canonical and aliased entry points reject valid parent invocations.
Existing runnable-parent tests use ordinary nonempty operands without this
later child-name token.
Required work: Keep backend selection consistent with the structural command
boundary and preserve operand tokens for ordinary typed preparation. Do not
strip or normalize these operands, require an extra -- to make them usable,
or relax exact command-name matching. Keep genuine child selection working when
the child name immediately follows the parent.
Completion evidence: Canonical and aliased parent invocations accept empty
and - operands followed by a child-name token, deliver the original ordered
values, and expose parent invocation paths. Cover parent-local flag aliases and
help on the same inputs, plus genuine child selection and ordinary terminator
behavior. Tests should establish CommandKit's boundary, not depend on a specific
replacement routing algorithm or backend type.
Completed: The adapter ends command search before empty or single-dash operand tokens, preserving them and subsequent tokens for the parent's ordinary flag/operand parser. All four canonical/alias regressions failed before the fix and now preserve the ordered operands, parent-local flag aliases, and invocation paths. The same cases cover metadata-only parent help and explicit terminators; immediate child selection remains intact. No syntax restriction or public API was added. Final uncached offline suites passed on Go 1.26.8: root with the workspace disabled, plus both integrations and their maintained examples in the existing workspace against the local core.