Consolidate and prioritize the audit findings

This commit is contained in:
2026-11-07 13:48:22 +00:00
parent db732e3a1d
commit f00fe1909d

View File

@@ -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).