diff --git a/docs/roadmap/audit/catalog.md b/docs/roadmap/audit/catalog.md new file mode 100644 index 0000000..dbd47ed --- /dev/null +++ b/docs/roadmap/audit/catalog.md @@ -0,0 +1,382 @@ +# 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 and recommended review order. The linked original +entries retain code locations, evidence, contract analysis, and detailed +remediation options. Consolidation does not resolve a finding or select a +proposed contract change. 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. + +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](../../internal/overview.md) 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](04-concurrency.md#n1--p2-cancellation-can-stop-aggregate-smoke-testing-with-a-success-outcome). +Also noted in the [documentation cross-references](13-documentation.md#previously-recorded-discrepancies-affecting-documented-behavior). + +## 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](01-correctness.md#c1--p2-cli-provider-parameter-objects-leak-mutations-between-requests). +Also noted in the [documentation cross-references](13-documentation.md#previously-recorded-discrepancies-affecting-documented-behavior). + +## 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. + +Make the transition out of deferred acceptance atomic, or drain every accepted +message before closing. Preserve the finalizer's own delivery path. The two +source entries describe the same logical ordering defect, rather than a data +race. + +**Originals:** [01-correctness C2](01-correctness.md#c2--p2-finalization-can-discard-a-successfully-queued-deferred-message); +[04-concurrency N2](04-concurrency.md#n2--p2-deferred-message-acceptance-remains-open-after-the-final-batch-detaches). +Also noted in the [documentation cross-references](13-documentation.md#previously-recorded-discrepancies-affecting-documented-behavior). + +## 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](01-correctness.md#c3--p2-prepared-request-inspection-reports-success-without-a-result). +Also noted in the [documentation cross-references](13-documentation.md#previously-recorded-discrepancies-affecting-documented-behavior). + +## 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](01-correctness.md#c4--p2-show-preparation-labels-ordinary-cancellation-as-a-rendering-error); +[04-concurrency N3](04-concurrency.md#n3--p2-buffered-configuration-show-cancellation-becomes-a-presentation-failure). +Also noted in the [documentation cross-references](13-documentation.md#previously-recorded-discrepancies-affecting-documented-behavior). + +## 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](01-correctness.md#c6--p3-the-interactive-completion-consumer-cannot-construct-its-app). +Also noted in the [documentation cross-references](13-documentation.md#previously-recorded-discrepancies-affecting-documented-behavior). + +## 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](05-resources.md#rm1--p3-closed-reporting-handles-retain-both-destination-objects). + +## 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](01-correctness.md#c5--p3-the-core-example-overwrites-its-optional-scenario-default). + +## 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](13-documentation.md#d1--p3-configuration-provider-godoc-omits-the-effective-output-contract). + +## 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. + +**Original:** [13-documentation D2](13-documentation.md#d2--p3-public-flag-metadata-omits-enforced-declaration-constraints). + +## 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. + +Consider narrowing the interface to acquisition and updating writer-only stubs +and assertions. This needs a deliberate public compatibility decision: callers +invoking the method through `View.Explanation` would stop compiling. Both audits +identify the same obsolete requirement. + +**Originals:** [03-complexity M6](03-complexity.md#m6--p3-explanation-providers-must-implement-an-unused-rendering-capability); +[A1-responsibility R1](A1-responsibility.md#r1--p3-explanation-providers-still-owe-an-unused-text-writer). + +## 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](A1-responsibility.md#r2--p3-parser-sessions-carry-application-execution-resources-they-do-not-use). + +## 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. + +**Original:** [02-efficiency E1](02-efficiency.md#e1--p3-flag-alias-compilation-repeatedly-resolves-shared-chain-suffixes). + +## 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²). An index can be shared with F15, but +the repeated preparation and help-time scan are distinct issues. + +**Original:** [02-efficiency E3](02-efficiency.md#e3--p3-discovery-context-preparation-rebuilds-shared-prefixes-repeatedly). + +## 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. + +**Original:** [02-efficiency E2](02-efficiency.md#e2--p3-alias-help-scans-every-source-for-every-intermediate-route). + +## 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](03-complexity.md#m3--p3-parsing-and-completion-maintain-parallel-flag-consumption-logic). + +## 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](03-complexity.md#m1--p3-scope-capture-combines-several-compilation-phases). + +## 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](03-complexity.md#m2--p3-configuration-preflight-mixes-shared-and-operation-specific-rules). + +## 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](03-complexity.md#m4--p3-typed-preparation-combines-conversion-provenance-and-replay-assembly). + +## 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](03-complexity.md#m7--p3-terminal-and-ordinary-messages-duplicate-detail-rendering-mechanics). + +## 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. + +**Original:** [03-complexity M5](03-complexity.md#m5--p3-configuration-diagnostic-adaptation-is-maintained-in-both-integrations). +Also discussed in the [responsibility assessment](A1-responsibility.md#machinery-worth-retaining-under-the-selected-contracts). + +## 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](02-efficiency.md#e4--p3-successful-declaration-validation-eagerly-constructs-every-full-path). + +## 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](A1-responsibility.md#r3--p3-concise-diff-rendering-revalidates-native-configkit-paths). + +## 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](A1-responsibility.md#r4--p3-private-production-entry-points-exist-only-to-sustain-older-tests). + +## 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](13-documentation.md#d3--p3-reporting-adr-still-directs-readers-to-a-planned-implementation). + +## 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](13-documentation.md#d5--p3-the-root-readme-duplicates-the-compatibility-matrix). + +## 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](13-documentation.md#d4--p3-a-historical-release-note-links-to-a-nonexistent-section).