Remove completed alias interaction remediation document
This commit is contained in:
@@ -1,198 +0,0 @@
|
||||
# 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](../cli.md), public GoDoc, accepted
|
||||
[ADR 0013](../adr/0013-application-alias-routing.md), 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's `pk show prompt briefing --campaign spring --fmt yaml`, and simple
|
||||
ConfigKit example's two-file `config diff` completed 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](../../route.go#L25) 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](../../help_view.go#L62) similarly searches only the entered
|
||||
and canonical prefixes, and
|
||||
[discoveryHelpAliases](../../help_view.go#L33) 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:
|
||||
|
||||
```text
|
||||
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:
|
||||
|
||||
```text
|
||||
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](../../execute.go#L195) adapts usage only for canonical alias
|
||||
routes. For discovery routes, backend root usage is displayed instead;
|
||||
[execution.usageLine](../../execute.go#L182) 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`:
|
||||
|
||||
```text
|
||||
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](../cli.md#application-owned-aliases).
|
||||
|
||||
**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](../../internal/cobra/parse.go#L55) 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](../../route.go#L34) 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`:
|
||||
|
||||
```text
|
||||
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.
|
||||
Reference in New Issue
Block a user