commandkit/docs/roadmap/audit/catalog.md

Consolidated audit catalog

Status: 27 distinct open findings: five P2 and 22 P3, consolidated from 30 numbered entries in seven audit documents. Reviewed audit state: db732e3a1de55c844f77870e1c6db683c2685d5d.

This catalog owns grouping, recommended review order, and subsequent remediation decisions. The linked original entries retain code locations, evidence, contract analysis, and detailed remediation options. Consolidation alone does not resolve a finding or select a proposed contract change. Explicit decisions recorded below supersede the original entries' alternatives; all findings remain open until remediated. Original priorities are retained. Within those priorities, incorrect outcomes and request isolation lead; blocked validation, resource retention, and consumer contracts precede scaling and maintainability work; editorial and historical navigation issues follow.

The remediation analysis tracks unresolved choices and recommendations for sequential discussion. Accepted conclusions are recorded in this catalog; analysis alone does not select a remedy.

Three pairs describe the same underlying issue: correctness C2/concurrency N2, correctness C4/concurrency N3, and complexity M6/responsibility R1. Other related findings remain distinct where they concern different failure modes or changes. Documentation cross-references and the responsibility review's closing assessment are supporting discussion, not additional numbered findings.

The internal overview inventories the architecture: core owns declarations, routing, typed input preparation, dispatch, completion, and reporting; neutral private packages isolate the Cobra/pflag adapter. ckconfig and ckprompt adapt native upstream capabilities through separate optional modules. Applications retain loaders, builders, runtime resources, and executable lifecycle; release launchers delegate to ReleaseKit. The findings call for specific fixes and simplifications within these boundaries.

F01 — P2: Aggregate smoke testing can stop on cancellation and return success

In ckprompt's runSmokeAggregate, cancellation triggered during progress delivery can stop the remaining smoke kinds without entering the operation cause. A partial aggregate can consequently return status 0; cancellation can also be omitted when another error already exists.

Record cancellation at the checkpoint that actually omits remaining work, preserving acquired reports and prior errors. This does not require treating cancellation after all requested work has completed as a failure.

Original: 04-concurrency N1. Also noted in the documentation cross-references.

F02 — P2: CLI provider parameters share mutable state across requests

ckprompt assigns parsed ExtraParams maps directly to request destinations, and smoke seeds share those nested maps. Mutation by one callback can change later smoke requests or another destination receiving the same captured bindings.

Clone the CLI-owned JSON maps and arrays at binding application and seed creation. Preserve presence and numeric semantics, and application builders' authority; a general deep-copy framework is unnecessary.

Original: 01-correctness C1. Also noted in the documentation cross-references.

F03 — P2: Finalization can discard an accepted deferred message

Core reporter finalization detaches the deferred batch before closing acceptance. A concurrent submission can return success while presentation is in progress, then be discarded when finalization clears the queue. The returned delivery evidence can still indicate success.

Decision: atomically close deferred-message acceptance when finalization begins, deliver the already accepted batch under the existing presentation rules, and return ErrReporterClosed for later deferred submissions. Preserve the finalizer's own delivery path and document the acceptance cutoff. Repeatedly draining newly arriving submissions is not selected; completion must not depend on application producers eventually stopping. This does not add responsibility for application goroutines or change ordinary message visibility.

The two source entries describe the same logical ordering defect, rather than a data race.

Originals: 01-correctness C2; 04-concurrency N2. Also noted in the documentation cross-references.

F04 — P2: Prepared-request preview can succeed without a prepared result

The ckprompt prepared-request handler accepts an empty Acquired value with a nil error from PrepareRequest. It renders null and reports success despite having no prepared request.

Reject the missing result as an operational failure in this handler while preserving diagnostics and explanations. Keep core's general ability to render a legitimate null report.

Original: 01-correctness C3. Also noted in the documentation cross-references.

F05 — P2: Configuration-show cancellation is also recorded as a rendering failure

ckconfig checks cancellation during buffered show preparation inside the RenderOutput callback. Returning that cancellation from the renderer records both the correct operation cause and a false presentation failure, conflating two established outcome dimensions.

Carry preparation cancellation separately from renderer errors while retaining genuine rendering and writing failures and avoiding partial aggregate output. Both audits identify the same classification defect.

Originals: 01-correctness C4; 04-concurrency N3. Also noted in the documentation cross-references.

F06 — P3: The optional interactive completion consumer fails before shell validation

The completion consumer fixture declares the reserved format flag, so App construction fails before it can generate scripts or exercise interactive completion. Default test suites skip this optional path.

Use core's format flag or rename the fixture's custom flag, update its expectations, and rerun available shell checks. The fixture failure does not establish a defect in generated completion scripts.

Original: 01-correctness C6. Also noted in the documentation cross-references.

F07 — P3: Closed reporting handles retain both borrowed destinations

A retained closed Reporter, Output, or DeferredReporter keeps the shared reporting state and both destination writers reachable. Applications retaining a closed handle can therefore retain large buffers or resource-bearing writer objects after invocation completion.

Clear direct borrowed-writer references after final presentation under the reporting lock, preserving closed-handle behavior and returned error evidence. Do not close or flush application-owned writers. This is conditional retention through a reachable handle, not an unreachable heap leak.

Original: 05-resources RM1.

F08 — P3: The core example overwrites its optional argument default

The example's scenario setter replaces the factory's "demo" default with an unsupplied empty value. The ordinary report invocation consequently fails with a usage error.

Assign the optional argument only when its supplied state is set. Fix the consumer without changing core's optional-binding semantics.

Original: 01-correctness C5.

F09 — P3: The configuration provider contract omits its required YAML representation

EffectiveWriter GoDoc promises a native representation but omits that config show converts one supported YAML document through DecodeYAMLReport. Custom providers can supply output that appears compatible with the description yet fails conversion; native formatting is not passed through.

Document that boundary with the public interface and conversion helper, and update View's report description. This clarification is independent of the unused explanation-writer requirement in F11.

Original: 13-documentation D1.

F10 — P3: Public flag metadata omits enforced naming and reserved-spelling rules

FlagMeta describes names without leading dashes and an optional short spelling, while installation enforces additional name grammar and reserved spellings. Consumers must discover those constraints through errors or private implementation.

Document the implemented rules once with FlagMeta and link related public helpers. Preserve accepted Unicode lowercase letters and digits; changing validation would require a separate decision.

The F10 analysis reviews the naming convention's basis and recommends retaining the existing rules; agreement on that precise contract is pending.

Original: 13-documentation D2.

F11 — P3: Explanation providers must implement an unused text writer

ExplanationReader still requires WriteExplanation even though production show/explain handlers acquire evidence with Explain and core renders it. This burdens custom providers and suggests a presentation responsibility that standard commands no longer use.

Decision: remove WriteExplanation from ExplanationReader, leaving the acquisition responsibility, and update GoDoc, writer-only stubs, and assertions. Native provider objects may retain their own rendering methods. Do not add compatibility scaffolding or restore provider-owned rendering to justify the unused requirement. Both audits identify the same obsolete requirement.

The interface change removes the ability to invoke that method through View.Explanation. CommandKit is still pre-release with no consumers, so no consumer compatibility or migration work is needed.

Originals: 03-complexity M6; A1-responsibility R1.

F12 — P3: Parser sessions carry unused execution resources

Core and the Cobra adapter duplicate stream normalization and retain context and streams through parser options and backend nodes, although the adapter parses and renders buffered help rather than executing callbacks or reading application input. The execution object also retains an unread streams field.

Remove unused resource plumbing while preserving real invocation resources and explicit internal discard destinations/local help buffers. This simplifies ownership; it is separate from the closed-reporting-handle retention in F07.

Original: A1-responsibility R2.

F13 — P3: Flag-alias compilation repeatedly walks shared chain suffixes

For A chained effective flag mappings across C contexts, independent target walks can perform Θ(CA²) target examinations, including inactive chains. Each source repeats work already performed for another source's suffix.

Use context-local visitation states and memoized terminal results. Traversal can become expected O(A) per context, with existing deterministic sorting retaining O(A log A) total work for these steps. Preserve scoped precedence, canonical endpoints, missing-target behavior, and cycle diagnostics.

Decision for F13–F15: the repeated-work and scaling analysis sufficiently justifies remediation. Performance testing is not a prerequisite for selecting or implementing these changes. Validate correctness and preservation of the selected routing and discovery semantics.

Original: 02-efficiency E1.

F14 — P3: Discovery-context preparation repeatedly rebuilds shared prefixes

R alias sources sharing a depth-D proper prefix cause Θ(RD²) copying and key construction before result deduplication, with repeated classification as well. The source declarations contain only Θ(RD) tokens.

Traverse distinct source prefixes once and classify each once. For the audited shape, this phase can become O(RD + D²).

Decision: design F14 and F15 together around shared source-prefix processing, preferably reusing the existing alias source structure rather than adding parallel indexes. The repeated preparation and help-time scan remain distinct findings. The performance-testing decision under F13 applies to both.

Original: 02-efficiency E3.

F15 — P3: Alias help scans the entire source catalog for each intermediate route

Help discovery examines all A alias sources at each of P rewrite paths, doing PA examinations and repeated copies even when there are no descendants. A valid single-token alias chain produces Θ(A²) work to discover an empty alias section on every such help request.

Query an immutable source-prefix index for strict descendants. The identified discovery work becomes O(A) for that example; general cases still pay for matching candidates, resolution, and output sorting. Design this together with F14 under the decisions recorded there and in F13.

Original: 02-efficiency E2.

F16 — P3: Parsing and completion duplicate flag-consumption grammar

Execution parsing and completion separately implement short clusters, attached/next values, boolean assignments, terminators, and unknown spellings. Shared classification helpers cover only part of the grammar, leaving changes to cross the core/backend boundary.

Share a small neutral token-consumption primitive. Keep execution normalization, failure provenance, and partial-word completion policies in their existing owners rather than introducing a common public parser.

Original: 03-complexity M3.

F17 — P3: Configuration scope capture combines several compilation phases

ckconfig's captureSuite interleaves scope and selector capture, validation, comparison-binding renaming, binding attachment, operation unions, and discovery inventory. Several partially populated collections must be understood together.

Extract cohesive private phases over the existing prepared types, keeping related loops together. Preserve retained error order, declaration ownership, scope and binding identity, and metadata-only construction.

Original: 03-complexity M1.

F18 — P3: Configuration preflight interleaves shared and operation-specific rules

The configuration command builder and prepareConfigOperation mix flag combinations, help selection, capabilities, pointer parsing, plan assembly, ordinary inputs, and independent comparison inputs. Understanding one operation requires reasoning through irrelevant submodes.

Separate operation options, shared selection, and comparison preparation with small private functions. Preserve the ordering of checks, input factories, binding application, state construction, and acquisition; no generic operation engine is needed.

Original: 03-complexity M2.

F19 — P3: Typed input preparation mixes conversion, provenance, and replay assembly

prepareInputWithOccurrences handles shape validation, conversion-error spelling, occurrence identities, application order, required flags, operands, and prepared callbacks. Captured binding replay depends on those shared identities across another code path.

Extract cohesive error translation, flag/occurrence preparation, and argument preparation. Retain the selected dense reconstruction and heap merge, binding identity, conversion-before-factory ordering, and error identity.

Original: 03-complexity M4.

F20 — P3: Reporter message paths duplicate detail-rendering bookkeeping

Terminal and ordinary message paths repeat callback invocation, managed-writer setup, render-versus-write error classification, and first-error retention. Maintaining the same presentation policy in two branches increases the chance of drift.

Extract a private helper under the existing reporting lock. Keep visibility/discard selection, headings, delivery observations, callback ordering, and non-reentrancy explicit. This refactor does not itself fix deferred-message loss or writer retention.

Original: 03-complexity M7.

F21 — P3: Both integrations maintain the same ConfigKit diagnostic adaptation

ckconfig and ckprompt independently snapshot ConfigKit diagnostic evidence, partition severities, construct detail callbacks, label sources, and delegate native text formatting. Related Reporter scheduling switches are duplicated too.

Consider a narrow public ckconfig adaptation helper returning existing core diagnostic/explanation values. Any exported helper needs an API decision; preserve snapshot ownership, ordering, error identity, and diagnostic timing rather than introducing a universal framework.

Decision context: CommandKit is still pre-release with no consumers. Compatibility and consumer migration are therefore not constraints on this remediation. The remaining design question is whether a concrete, narrow CLI adaptation API simplifies both integrations enough to justify maintaining it. The helper's signature and scope remain to be selected; this clarification does not itself approve a new public API or a separate reporting-scheduler helper.

Original: 03-complexity M5. Also discussed in the responsibility assessment.

F22 — P3: Successful declaration validation eagerly formats every full path

collectDeclarationIssues copies and formats each ancestral path before knowing whether a diagnostic is needed. A valid depth-N declaration chain incurs Θ(N²) cumulative path work and allocation while producing no errors.

Use an append/truncate scratch path and format only emitted diagnostics. The successful validation walk can become O(N) with O(D) path storage. Other installation phases retain their own costs; this is not a claim that all App construction becomes linear.

Original: 02-efficiency E4.

F23 — P3: Concise diff rendering validates already-valid native paths

ckconfig's private pointer decoder validates arbitrary strings even though its sole production caller supplies the canonical string of an immutable ConfigKit Path. Tests sustain malformed-input branches that this boundary cannot reach.

Accept the native path directly and retain only the key-token decoding needed by the selected concise report. Decode escapes once so ~01 remains ~1; preserve snapshot errors and operator-input validation.

Original: A1-responsibility R3.

F24 — P3: Obsolete private production entry points remain for tests alone

Legacy reporter setters/close, App.parse, and typed-preparation wrappers have only test callers, while production uses current selection, finalization, routed-session, and occurrence-aware paths. Some tests therefore exercise superseded lifecycle entry points.

Move tests to the real private paths and remove obsolete production wrappers. Keep any concise setup helpers in _test.go files, retaining useful behavioral assertions without new exported test hooks.

Original: A1-responsibility R4.

F25 — P3: The reporting ADR mixes implemented status with stale planning language

ADR 0006 marks shared reporting implemented but still directs implementation status and format choices to a retired roadmap and describes the extension as planned. Readers can mistake completed work for a pending prerequisite.

Clarify historical wording and link current behavior to GoDoc and the CLI reference. Preserve the accepted architectural decision; an unchanged decision does not need a replacement ADR.

Original: 13-documentation D3.

F26 — P3: The README duplicates volatile module compatibility guidance

The root README repeats exact version combinations and candidate dependency details already maintained by the consumer guide. The facts currently agree, but two owners create policy and maintenance drift risk.

Keep a brief status/module overview in README and link the consumer guide for exact compatibility and installation choices. This is documentation consolidation, not a dependency change.

Original: 13-documentation D5.

F27 — P3: A historical release note points to a nonexistent section

The ckprompt v0.2.0 release note uses the wrong consumer-guide fragment at its immutable tag. The file exists, but the requested section does not.

Provide the correct pinned section link in a useful forward-only correction or erratum. Preserve the published tag, historical note, and hosted release contents; republishing is unnecessary.

Original: 13-documentation D4.