277 lines
21 KiB
Markdown
277 lines
21 KiB
Markdown
# Audit remediation roadmap
|
||
|
||
Status: source remediation complete; the [implementation plan](implementation.md)
|
||
records the sprint and verification. Release and dependency alignment remain
|
||
separate work in [future work](future.md#pending-release-and-integration-work).
|
||
|
||
Baseline: `d131999c7c474010a4814bb81601cd32920e6c83`. The
|
||
[catalog](audit/catalog.md) records 27 distinct findings, five P2 and 22 P3,
|
||
from seven audits. Its accepted decisions and the
|
||
[remediation analysis](audit/remediation-analysis.md) supply the scope and
|
||
rationale for this roadmap. The audited state was
|
||
`db732e3a1de55c844f77870e1c6db683c2685d5d`.
|
||
|
||
This roadmap owns the target, scope, and completion criteria. The implementation
|
||
plan owns ordered stages, dependencies, concrete changes, and verification steps.
|
||
The catalog owns finding status and accepted remediation decisions;
|
||
the original audit entries own detailed evidence. This is separate from the
|
||
older, completed [shared-reporting remediation](https://gitea.maximumdirect.net/eric/CommandKit/src/commit/264b82bd5759f052a3646d57828945a87b7586d3/docs/roadmap/remediation.md), whose findings
|
||
are not reopened here. Unselected capabilities in [future work](future.md)
|
||
remain outside this effort.
|
||
|
||
## Current baseline and targeted end state
|
||
|
||
CommandKit has three modules. Core owns declarations, routing, typed inputs,
|
||
completion, invocation reporting, and outcomes, with Cobra/pflag behind private
|
||
adapters. ckconfig exposes configuration inspection through application-supplied
|
||
scopes. ckprompt exposes PromptKit inspection, preparation, requirements, and
|
||
smoke testing and composes configuration inspection through ckconfig.
|
||
Applications retain loading, runtime construction, resources, and executable
|
||
lifecycle; upstream libraries retain domain semantics.
|
||
|
||
The current manifests select ConfigKit v1.1.0 and PromptKit v0.20.0. Both
|
||
integrations select core v0.4.0, and ckprompt selects ckconfig v0.4.0. The ignored
|
||
workspace can align the three local source modules. Core declares Go 1.26.0;
|
||
the integrations declare Go 1.27.0. This remediation selects no upstream upgrade
|
||
or consumer toolchain change.
|
||
|
||
The target keeps that architecture and existing command suite while delivering:
|
||
|
||
- Accurate outcomes and retained evidence when cancellation, provider mistakes,
|
||
or presentation failures occur, and isolation of CommandKit-parsed JSON
|
||
between the identified request destinations.
|
||
- An explicit deferred-message acceptance cutoff, release of unnecessary
|
||
borrowed-writer references after finalization, and one implementation of
|
||
shared detail-rendering bookkeeping.
|
||
- Working maintained examples and completion validation, with tests exercising
|
||
current production paths instead of obsolete private entry points.
|
||
- Narrower provider responsibilities and shared ConfigKit diagnostic adaptation
|
||
serving both integrations' CLI reporting.
|
||
- Shared internal flag-consumption mechanics, reuse of the existing alias tree,
|
||
removal of avoidable repeated work, and bounded private extractions.
|
||
- Public documentation that accurately describes implemented provider and flag
|
||
contracts, with current guidance separated from historical records.
|
||
|
||
CommandKit is pre-release with no consumers. Compatibility scaffolding and
|
||
consumer migration work are unnecessary for the selected API changes. This does
|
||
not alter the handling of published tags or historical release contents.
|
||
|
||
## Scope and implementation boundaries
|
||
|
||
Apply the [agent](../policy/agents.md), [architecture](../policy/architecture.md),
|
||
[testing](../policy/testing.md), and [documentation](../policy/documentation.md)
|
||
policies. The [CLI reference](../cli.md) and exported GoDoc remain the current
|
||
behavior owners; update them when their corresponding remedy lands.
|
||
|
||
The following limits keep the remedies within their accepted scope:
|
||
|
||
- F01 retains cancellation when it stops remaining smoke work; it does not make
|
||
every cancellation after completed work a failure.
|
||
- F02 copies the known CLI JSON maps and arrays, preserving presence and numeric
|
||
values. It does not promise arbitrary deep copies of application objects or
|
||
repair application-created sharing.
|
||
- F03 closes deferred acceptance atomically when finalization begins, presents
|
||
the accepted batch under existing visibility and failure rules, and rejects
|
||
later deferred submissions with `ErrReporterClosed`. It does not promise
|
||
successful delivery when rendering or writing fails, or require waiting for
|
||
application producers or owning their goroutines.
|
||
- F04 rejects a missing prepared request in that command. Core can still render
|
||
a legitimate null report. F05 keeps operation causes distinct from rendering
|
||
and writing failures.
|
||
- F07 releases direct writer references after final presentation, without
|
||
closing or flushing borrowed writers or promising immediate garbage collection.
|
||
Retained errors may legitimately retain their own referenced objects.
|
||
- F10 documents the agreed current naming rules, including Unicode lowercase
|
||
letters and digits, reserved names, and existing short spellings. It adds no
|
||
ASCII-only or leading-letter restriction or automatic name normalization.
|
||
- F13–F15 are justified by the repeated-work analysis. Performance testing is
|
||
not a prerequisite and no timing, allocation, or whole-installation complexity
|
||
guarantee is introduced.
|
||
- F16–F20 remain internal simplifications. Keep meaningful caller differences,
|
||
existing types, and necessary coordination; do not introduce a public parser,
|
||
generic pipeline, replay framework, or new concurrency contract.
|
||
- F21 serves CLI presentation. It does not load or validate configuration,
|
||
choose operation outcomes, or add general application-internal convenience
|
||
interfaces. Core remains independent of ConfigKit and PromptKit.
|
||
|
||
Preserve native safe evidence, selected report content, error identity, explicit
|
||
presence, and the established independent comparison selections through these
|
||
changes. Do not replace upstream domain behavior or expand validation merely to
|
||
simplify implementation. Private helper names and exact signatures are ordinary
|
||
implementation choices unless explicitly selected as public API.
|
||
|
||
## Target capabilities and completion evidence
|
||
|
||
The areas below cover every catalog finding and define the desired end state,
|
||
without specifying implementation order. Each finding remains accountable for
|
||
its own remedy and evidence; a nearby refactor or passing broad suite alone does
|
||
not establish that a defect is fixed.
|
||
|
||
### Correct outcomes, request ownership, and reporting lifecycle
|
||
|
||
| Finding | Required work | Evidence of the intended result |
|
||
| --- | --- | --- |
|
||
| F01 | Retain cancellation at the aggregate smoke checkpoint that omits remaining kinds, preserving previous errors and completed reports. | Cancellation during progress delivery or between operations produces a failed partial outcome with the cancellation cause; prior errors survive and unattempted kinds receive no invented report. |
|
||
| F02 | Copy parsed extra-parameter objects and nested arrays/maps at binding application and each aggregate seed boundary. | Mutating one destination does not alter a later smoke request or an independent captured-binding application; empty/nil and numeric distinctions remain intact. |
|
||
| F03 | Make deferred acceptance closure and batch detachment atomic while retaining the finalizer's delivery path. Document the cutoff. | A controlled concurrent submission is either accepted for the final batch or rejected as closed; successful acceptance cannot disappear into an abandoned queue. |
|
||
| F04 | Reject nil prepared results returned without an error, after the existing acquisition/cancellation handling. | The prepared-preview command fails operationally and retains supplied diagnostics/explanation; valid previews and core null reports retain their existing treatment. |
|
||
| F05 | Carry buffered configuration-show cancellation separately from renderer errors. | Interrupted preparation emits no partial aggregate report, retains cancellation as the operation cause, and does not invent a rendering failure; genuine rendering/writing failures remain observable. |
|
||
| F07 | Clear direct destination references under the existing reporting lock after final presentation. | Closed handles still reject late operations, returned presentation evidence survives, and CommandKit no longer retains writers through those direct fields or closes them. |
|
||
|
||
F03 and F07 touch the same finalization boundary but fix different problems.
|
||
Keep them separately explainable. F20's bookkeeping extraction must not
|
||
be used as a substitute for either correction.
|
||
|
||
### Working examples and completion validation
|
||
|
||
| Finding | Required work | Evidence of the intended result |
|
||
| --- | --- | --- |
|
||
| F06 | Remove the completion fixture's collision with core's reserved `format` flag. Use core's flag when testing report selection, or rename the fixture's custom choice flag when testing application completion. Adjust corresponding expectations. | The fixture constructs and generates scripts, and available opt-in shell checks exercise completion rather than failing during installation. |
|
||
| F08 | Assign the example's optional scenario argument only when supplied. | Bare `report` runs its `demo` default; an explicitly supplied value still reaches ordinary validation. |
|
||
|
||
The existing completion harness remains the validation owner; its prerequisites
|
||
and commands belong to the [developer guide](../development.md#shell-completion-validation).
|
||
Unavailable shell checks remain distinct from successful checks. This does not
|
||
select a new platform support matrix.
|
||
|
||
### Clear and narrow integration boundaries
|
||
|
||
| Finding | Required work | Evidence of the intended result |
|
||
| --- | --- | --- |
|
||
| F09 | Document `EffectiveWriter` as supplying safe YAML for `DecodeYAMLReport`, link the conversion contract, and correct `View`'s report description. | GoDoc describes the actual single-document conversion boundary and does not imply native formatting passthrough or broader YAML support. |
|
||
| F10 | Document the accepted naming/reservation rules once in `FlagMeta`; link relevant rename and alias helpers. | Public guidance matches installation behavior without changing the validator or flag-value semantics. |
|
||
| F11 | Remove `WriteExplanation` from `ExplanationReader`; update GoDoc and obsolete stubs/assertions. | Custom explanation providers need only acquisition, and standard handlers continue rendering acquired evidence through core. Native objects may keep their own writer methods. |
|
||
| F21 | Expose one narrow ckconfig diagnostic adaptation function and use it in both integrations, replacing the duplicated adaptation. | Both callers retain the selected snapshots, severity treatment, source labels, ordering, and native formatting; command-specific context, causes, outcomes, and scheduling remain with callers. |
|
||
| F23 | Make concise-diff token decoding accept native `configkit.Path` and remove unreachable arbitrary-string validation. | Literal key tokens, including `~01`, empty keys, slash, tilde, and root paths, remain correct; snapshot failures and actual operator-input validation remain intact. |
|
||
|
||
For F21, the accepted input/output shape is a caller-supplied heading, native
|
||
source records, and native diagnostic records returning existing core diagnostics
|
||
and an explanation. The illustrative `AdaptDiagnostics` name in the analysis is
|
||
not a separate required spelling. Reuse the existing private helper's work;
|
||
keep the smaller scheduling switches with their callers. No new module is needed.
|
||
|
||
For F23, ConfigKit's opaque path is already valid, but the concise report still
|
||
needs key-token decoding. Remove revalidation, not that necessary translation.
|
||
Use bounded single-pass escape decoding so `~01` becomes `~1`, not `/`.
|
||
|
||
### Cohesive private responsibilities
|
||
|
||
| Finding | Required work | Evidence of the intended result |
|
||
| --- | --- | --- |
|
||
| F12 | Remove unused execution-resource fields and parser options and redundant normalization. Keep invocation resources with their real owners. | Parsing and buffered help retain their behavior without application input/context/streams threaded through unused backend fields; internal discard destinations still prevent accidental process output. |
|
||
| F24 | Move tests to real selection, finalization, routed parsing, and occurrence-aware preparation paths; remove obsolete production wrappers. | Useful assertions remain, superseded paths have no production test support burden, and any small setup helpers live in test files. |
|
||
| F16 | Share flag identity, attached/missing values, next-argument consumption, and short-cluster position through small neutral internal helpers. | Execution and completion agree on shared grammar while preserving entered spellings, explicit empty values, and their distinct treatment of unfinished input. |
|
||
| F20 | Extract common detail-rendering/error bookkeeping under the existing reporting lock. | Ordinary and terminal reporting retain distinct headings/visibility/timing while sharing rendering-versus-writing classification, callback order, and error retention. |
|
||
| F18 | Separate operation checks, scope/capability selection, and two-sided comparison preparation using private functions. | Help and preflight remain metadata/input-only where required; input checks precede state/acquisition, and comparison sides remain independent. |
|
||
| F17 | Separate cohesive scope capture/validation and selector attachment work; make operation bindings and discovery inventory construction explicit. | Captured ownership, retained errors, binding identity, discovery, and construction without provider acquisition remain intact. |
|
||
| F19 | Extract conversion-error attribution and operand preparation modestly, keeping flag preparation and occurrence ordering together. | Entered spelling, supplied-state distinctions, error identity, application order, and captured replay remain correct without replacing dense reconstruction or the existing merge algorithm. |
|
||
|
||
F24 covers all identified obsolete entry points. Extractions retain related
|
||
loops and existing prepared types; F19 remains conservative. Tests protect
|
||
behavior rather than helper counts, incidental private call sequences, or
|
||
complexity scores.
|
||
|
||
For F16, retain execution normalization and failure handling with the parser,
|
||
and partial-word handling and suggestions with completion. Keep caller-specific
|
||
branches where unification requires extensive modes or special cases. This
|
||
sharing introduces no new alias-completion behavior.
|
||
|
||
### Efficient alias and diagnostic-path traversal
|
||
|
||
| Finding | Required work | Evidence of the intended result |
|
||
| --- | --- | --- |
|
||
| F13 | Resolve each context's effective flag-alias graph with visitation states and memoized terminal results, including inactive endpoints. | Each shared suffix is resolved once per context; scoped precedence, canonical endpoints, missing-target handling, and cycle diagnostics retain their meaning. |
|
||
| F14 | Enumerate distinct nonempty proper source prefixes through the existing resolver tree and classify each once. | Shared prefixes avoid repeated construction/classification; canonical paths, exact aliases, and valid canonical operand space retain their exclusions. |
|
||
| F15 | Query strict descendant sources through that same tree for each existing intermediate resolution path. | Help avoids full-catalog scans for unrelated sources while preserving authoritative resolution, longer overrides, canonical-target checks, suffix deduplication, and output order. |
|
||
| F22 | Use a scratch path during declaration validation and format full paths only for emitted diagnostics. | Successful validation avoids eager full-path formatting, while retained issues, error identity, child order, and absent/root-only declaration handling remain correct. |
|
||
|
||
F14 and F15 share internal read-only operations on the existing resolver tree,
|
||
with private nodes, correctly owned captured paths, and the existing
|
||
routing-snapshot lifetime. Root help may still use the complete catalog when
|
||
all entries are needed. No second index, persistent result cache, new alias
|
||
limits, or alias-completion features are selected.
|
||
|
||
Source-level review should establish removal of the identified repeated work;
|
||
behavioral checks establish preserved results. No performance threshold or
|
||
benchmark suite is required to justify these remedies. Other routing,
|
||
termination-analysis, formatting, and installation costs remain outside those
|
||
specific complexity improvements.
|
||
|
||
### Consistent current guidance and historical navigation
|
||
|
||
| Finding | Required work | Evidence of the intended result |
|
||
| --- | --- | --- |
|
||
| F25 | Replace stale planning/status directions in reporting ADR 0006 with current GoDoc/CLI references, preserving its accepted decision. | The ADR no longer presents implemented reporting as a pending prerequisite; no replacement ADR is introduced for unchanged architecture. |
|
||
| F26 | Keep a brief module/status orientation in README and link the consumer guide for exact compatibility and installation details. | Volatile compatibility guidance has one canonical owner. |
|
||
| F27 | Add a useful forward-only correction identifying the correct pinned consumer-guide section for the historical ckprompt note. | The correction points to the actual section without modifying the published tag, historical note, or hosted release contents. |
|
||
|
||
GoDoc, CLI documentation, examples, and focused internal documentation must
|
||
describe the implemented boundaries, with valid links and semantic claims.
|
||
Existing audit evidence remains available; the catalog records each finding's
|
||
implementation/verification result. A selected remedy alone does not close it.
|
||
|
||
## Verification and module alignment
|
||
|
||
Verification establishes CommandKit's adaptation and coordination, without
|
||
independently testing upstream domain correctness or turning private mechanics
|
||
into public guarantees. Consequential defects have focused behavioral protection;
|
||
useful contract coverage survives deletion of obsolete test machinery. The
|
||
implementation plan defines proportionate checks and when to run them.
|
||
|
||
For F03, acceptance/finalization evidence must establish queue ordering; a race
|
||
detector alone cannot do so. F07 does not establish a garbage collection timing
|
||
promise. Ordinary suites remain deterministic, offline, and independent of
|
||
credentials or live model services.
|
||
|
||
The [developer guide](../development.md#documentation-validation) owns module and
|
||
workspace test commands. The root suite does not traverse nested modules.
|
||
Evidence covers combined local source, representative maintained consumers,
|
||
and available repaired completion checks using the already selected Go 1.27.0.
|
||
|
||
The independent-module check has a concrete dependency constraint: ckprompt's
|
||
new F21 call will not exist in its currently selected ckconfig v0.4.0 dependency.
|
||
Workspace success therefore cannot be reported as proof that unchanged
|
||
independent module requirements are ready. Preserve the ignored workspace files;
|
||
do not commit sibling replacements or fabricate dependency checksums.
|
||
|
||
During source development, record workspace evidence separately. Temporary
|
||
external-consumer modules or alternate module files with explicit local
|
||
replacements may provide additional boundary evidence, but are not validation
|
||
of the published dependency selection. Once the required CommandKit module
|
||
versions are available, align requirements/checksums and run each independent
|
||
module suite with the workspace disabled. Until then, report the independent
|
||
dependency limitation explicitly rather than marking that check complete.
|
||
|
||
The [release procedure](../release.md) owns dependency-order publication and
|
||
candidate validation. Release version selection, notes, and publication remain
|
||
separate work; this roadmap neither authorizes publication nor makes it a
|
||
prerequisite for source refactoring. When preparing a release, reuse that
|
||
procedure rather than inventing additional gates. The absence of consumers
|
||
removes migration work, not the need for coherent module requirements.
|
||
|
||
## Completion criteria
|
||
|
||
- [x] Every finding F01–F27 has its selected remediation implemented and recorded
|
||
with relevant evidence; related findings remain individually accountable.
|
||
- [x] The five P2 defects have focused behavioral protection, including error
|
||
identity, retained evidence, and the deferred acceptance boundary.
|
||
- [x] F06 and F08's maintained consumers work, with actual completion checks
|
||
distinguished from unavailable or skipped shell checks.
|
||
- [x] F11 and F21's selected API changes and F09/F10's clarified contracts are
|
||
reflected in canonical GoDoc and affected integration guidance.
|
||
- [x] Obsolete production wrappers/plumbing and duplicate adaptation/bookkeeping
|
||
are removed at the agreed boundaries, with useful tests retained.
|
||
- [x] F13–F15 and F22 remove the identified repeated work while preserving
|
||
behavior, without a new performance guarantee or measurement prerequisite.
|
||
- [x] F17–F20 improve responsibility separation within their bounded scope;
|
||
public behavior and necessary ordering remain protected.
|
||
- [x] Documentation corrections F25–F27 are complete and local links resolve.
|
||
- [x] Relevant module and aligned-workspace results are recorded. Independent
|
||
results identify the exact dependency selection; any release-alignment work
|
||
still pending is explicitly distinguished from completed source work.
|
||
|
||
The significant choices have been resolved in the catalog and remediation
|
||
analysis. Routine implementation details do not require an Open Decisions
|
||
section. Revisit a selected direction only if concrete implementation evidence
|
||
reveals a conflict or materially different tradeoff.
|