base-layer/commandkit
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Retire completed roadmaps and preserve deferred work

Browse Source
This commit is contained in:
Eric Rakestraw
2026-10-09 04:04:24 +00:00
parent 6bc37e7ba6
commit 54750dd957
15 changed files with 117 additions and 2841 deletions
Download Patch File Download Diff File Expand all files Collapse all files

138
docs/roadmap/remediation.md
View File

@@ -1,138 +0,0 @@
# Performance implementation review findings
Status: R-01 complete; all recorded remediation work is addressed.
Date: 2026-10-09.
## Review scope and authority
Reviewed the performance implementation at `18ff0d6`, covering the changes after
`1274ae7` against the [efficiency target](performance.md), selected OD-1/OD-2
approaches, [implementation record](implementation.md), accepted
[ADRs](../adr/), and applicable architecture, agent, and testing policies.
The completed ReleaseKit review findings remain available in
[Git history](https://gitea.maximumdirect.net/eric/CommandKit/src/commit/18ff0d6/docs/roadmap/remediation.md).
Release tooling is outside this implementation review.
## Architecture assessment
The selected structure is present:
- The root facade owns declarations, registration, typed inputs, standard
handlers, and invocation outcomes. Cobra and pflag remain inside the internal
adapter; the changes add no exported API declarations.
- Each `App` lazily retains immutable neutral command metadata. Successful
nonempty registration invalidates it; failed and empty registrations leave
it intact. Pending declarations are validated and cloned before installation
without copying the previously installed registry.
- Each invocation prepares a fresh mutable adapter session, reusing it for
parsing, help, and usage. Presentation can refresh metadata after callback
registration without rerunning input callbacks. Backend commands map directly
to declarations; parent links reconstruct the selected path.
- Configuration registration prepares ordered scopes, name lookup, installed
operations, and discovery metadata. Invocation contexts and acquisition remain
live callbacks. PromptKit builds the requested report format, and aggregate
configuration framing scans only the YAML preamble while retaining the
selected full native and framed buffers.
These changes implement PF-01–PF-06. The review identified an ownership
regression in the session integration, corrected under R-01 below. The
[internal overview](../internal/overview.md) owns the maintained package inventory.
## R-01: restrict built-in help interception to the root command
**Priority:** high. **Category:** dispatch correctness, application ownership,
and unintended restriction.
**Location:** [execute.go](../../execute.go), `App.Execute`, line 45 at the
reviewed revision. Introduced by `a7960e5`.
The dispatcher selects a leaf declaration and then checks `command.name ==
"help"` to identify the built-in help operation. That matches an application
command named `help` at any depth. Its ordinary typed input is then asserted to
be CommandKit's private `*helpInput`, producing an operational failure instead
of invoking the registered application handler.
A public API reproduction created a runnable `help` command with `struct{}`
input, composed it beneath the application-owned `session` group, and passed the
group to `New`. Construction and registration succeeded. Execution produced:
```text
args=[session help] kind=operational status=1 cause=built-in help input has an unexpected type handlerCalls=0
args=[help session] kind=success status=0 cause=<nil> handlerCalls=0
args=[session help --help] kind=success status=0 cause=<nil> handlerCalls=0
```
The first invocation must reach the application handler. The remaining two are
metadata help requests and appropriately leave that handler uninvoked.
[ADR 0001](../adr/0001-command-dispatch-and-inputs.md#ownership-and-namespace-structure)
and the [architecture policy](../policy/architecture.md#command-and-application-ownership)
leave custom names and semantics with applications outside standard families.
[ADR 0006](../adr/0006-command-grammar.md) also encourages consistent verbs,
including `help`. The selected session integration specifically concerns the
reserved **top-level** help command. Matching only the leaf name effectively
introduces a global reservation of that name, although registration still
accepts it. No such additional restriction was selected.
Required work:
- [x] Identify the root built-in help command by its complete selected path
or an equivalent explicit reserved-command identity. A root-path check such
as `len(parsed.Path) == 1 && parsed.Path[0] == "help"` is sufficient for the
current registry. Other runnable declarations must continue to their handler.
- [x] Add a public behavior regression for an application-owned nested `help`
command. Verify it receives its typed input and that its handler outcome is
preserved. Keep local `--help` and `help session help` as metadata operations.
- [x] Retain existing root help, invalid-target, usage, fresh-invocation,
registration-refresh, and diagnostic behavior. Use existing coverage where
sufficient; verify the relevant regression and default suite after the fix.
Do not remediate this by rejecting nested `help` names or imposing new command
naming rules. Keep the session optimization and established ownership boundary.
Mark this item complete only after the correction and its evidence are recorded.
### Completion evidence
**Completed:** `App.Execute` now recognizes the built-in help command only when
its selected path is the root `help` command. Nested application commands keep
their existing handler path and naming freedom; session reuse is retained.
The public regression `TestExecuteRunsApplicationOwnedNestedHelp` verifies a
required typed operand reaches the nested handler and preserves its domain,
status, and cause. Local `session help --help` and explicit `help session help`
render the command's metadata without invoking that handler or requiring its
execution operand.
The regression first failed against the original dispatcher with the reported
unexpected-input-type failure and passed after the correction. Focused validation
and the full default suite passed with readonly, offline module access:
```sh
GOWORK=off GOTOOLCHAIN=local GOPROXY=off GONOPROXY=none GOSUMDB=off go test -mod=readonly -count=1 . -run '^TestExecuteRunsApplicationOwnedNestedHelp$'
GOWORK=off GOTOOLCHAIN=local GOPROXY=off GONOPROXY=none GOSUMDB=off go test -mod=readonly -count=1 ./...
```
Existing root help, invalid-target, usage, fresh-invocation, registration-refresh,
and diagnostic tests passed. Changed Go files were formatted; document links
and whitespace checks passed. No new public contract or command-name restriction
was introduced.
## Original review evidence and limits
- Reviewed the production diffs, changed behavioral tests, benchmark fixtures,
architecture documentation, and completion/measurement records. Traced cache
invalidation, session refresh, input preparation, scope lookup, acquisition,
YAML framing, and requested-format delivery.
- Executed the public consumer reproduction above from a temporary file outside
the repository with readonly, offline module access. No repository test or
production code was added or changed during this review.
- The implementation record reports a successful default suite at `62b4a48`,
the last production-code revision. This review did not rerun that suite or the
benchmarks. Allocation improvements and registration tradeoffs are documented
there; its final cross-run timing comparison is explicitly inconclusive under
differing host load. This review does not establish a latency guarantee.
- No other issue requiring remediation was identified in the reviewed scope.
The changes do not establish concurrent dispatch, application startup rules,
provider-result caching, new release gates, or additional public API promises.