Record the concurrency audit

docs/roadmap/audit/04-concurrency.md

@@ -0,0 +1,178 @@
+# Concurrency audit
+
+Status: three relevant P2 findings: one new cancellation defect and two
+reconfirmed findings from the functional correctness audit. Reviewed source:
+`2ea26afcc5ae691152e80d2fa5ee4d572d390a95`.
+
+## Architecture, contracts, and scope
+
+The [internal overview](../../internal/overview.md) inventories the implemented
+owners. Core captures declarations and routing metadata, builds fresh backend
+parse state per invocation, prepares typed inputs, and coordinates invocation
+reporting and outcomes. ckconfig owns configuration scope/comparison coordination;
+ckprompt owns PromptKit request/inspection coordination and sequential aggregate
+smoke tests. Applications own loaders, builders, runtime dependencies, contexts,
+and executable lifecycle. Upstream libraries own their domain operations.
+
+The [reporting contract](../../consumers/guide.md#invocation-reporting) and
+[Reporter GoDoc](../../../reporter.go) explicitly permit concurrent operations on
+one invocation's reporter. One reporting-state mutex protects visibility,
+deferred messages, destination observations, and writes. Managed output writes
+acquire it; rendering callbacks use a coordinated writer while the rendering
+operation holds it. Rendering callbacks must not reenter that reporter, and
+their supplied writer is valid only while the callback runs. Concurrent message
+submission order is unspecified. Reporter handles close before Execute returns;
+caller-owned writers are not closed.
+
+Concurrent Execute/Register access to one App is not established by the current
+contracts; [concurrent dispatch remains separate future work](../future.md#other-future-proposals).
+The audit did not infer that guarantee from independent invocation state or
+declaration reuse. Application callback objects and factory results retain their
+own ownership and synchronization responsibilities under the
+[architecture policy](../../policy/architecture.md#handler-inputs-and-invocation-state).
+
+Static review covered locks and state access, reporter acceptance/finalization,
+callback and writer lifetimes, declaration/snapshot ownership, integration input
+and shared-state lifetimes, cancellation checkpoints, comparison/aggregate
+sequencing, and maintained examples and release launchers. CommandKit's runtime
+code starts no goroutines and owns no channels or worker pools. Integration
+callbacks run synchronously; aggregate scopes, comparison sides, and smoke kinds
+are coordinated sequentially. Release launchers use exec to delegate to ReleaseKit.
+Application/upstream workers and subprocess internals remain with their owners.
+
+## Validation evidence
+
+Codebase-memory-mcp symbol/source searches and call tracing were supplemented
+with repository-wide searches for synchronization, goroutines, channels, and
+cancellation. The indexed Go source matches this revision. Validation used Go
+1.27.0, cached dependencies, and no live services or credentials:
+
+- The aligned current-source workspace suite passed with race detection and
+  fresh test execution:
+  `GOTOOLCHAIN=local go test -race -count=1 -mod=readonly ./... ./integrations/ckconfig/... ./integrations/ckprompt/...`.
+  This includes all three modules and their maintained examples. The optional
+  interactive shell checks remain opt-in and were not enabled in this pass.
+- A temporary external consumer run with `go run -race` reproduced N1 through
+  the public ckprompt/core APIs. It canceled the context from the progress
+  destination's writer without reentering the reporter.
+- Existing temporary external probes using local replacements for all three
+  CommandKit modules reproduced N2 and N3 in three consecutive runs each with
+  `go test -race -count=3`. Their functional assertions failed as described
+  below. No data-race report was emitted.
+
+No production files or repository tests were changed. These failures concern
+ordering and cancellation accounting; memory-race detection alone does not
+establish that an accepted queue item is delivered or a stopped aggregate fails.
+
+## N1 — P2: Cancellation can stop aggregate smoke testing with a success outcome
+
+**Code:** [smoke_command.go](../../../integrations/ckprompt/smoke_command.go),
+runSmokeAggregate lines 159–176 and final outcome selection at 183–187;
+queuePromptDiagnostics in [command.go](../../../integrations/ckprompt/command.go),
+lines 586–603; core's Progress delivery in [reporter.go](../../../reporter.go).
+
+**Circumstances:** a smoke callback returns a report without error. The first
+post-callback context check sees no cancellation. Cancellation becomes visible
+while diagnostics are presented, or asynchronously before the later checkpoint
+at line 175. That checkpoint stops the loop, but does not retain its error.
+Progress diagnostics are delivered immediately, so this window can be exercised
+deterministically with a valid progress writer that cancels the context.
+
+**Observed behavior:** a temporary consumer invoked
+`promptkit test all --progress --format=json`. Its first callback returned a
+completed reachability report and a progress diagnostic. The diagnostic writer
+canceled the context and accepted all bytes. Only one callback ran; the JSON
+report contained one successful entry with no error. Execute nevertheless
+returned Kind success, status 0, and nil Cause. Rendering and writing errors
+were nil. The other three requested kinds were never attempted.
+A second probe returned a provider error from that callback: status was 1, but
+errors.Is(outcome.Cause, context.Canceled) was still false after the same stop.
+
+**Expected behavior and consequences:** the
+[aggregate smoke contract](../../cli.md#promptkit-smoke-testing) requires all four
+callbacks to return without error for status 0 and preserves cancellation causes
+when cancellation stops the sequence. A partial aggregate stopped by cancellation
+must not report successful completion. Automation can otherwise accept an
+incomplete aggregate, and callers cannot detect the cancellation through
+errors.Is on the returned cause. If an operation error already exists, the same
+window can omit cancellation from its joined causes.
+
+**Potential remediation:** when the later checkpoint uses cancellation to omit
+remaining kinds, retain that error in the aggregate cause before leaving the
+loop. Deduplicate an already retained cancellation as appropriate. Preserve
+completed reports, prior operation errors/explanations, and attempted-kind order;
+do not invent a report for an unattempted kind. Moving only the earlier check
+leaves another interval in which cancellation can be observed and discarded.
+This does not require treating every cancellation after completed report delivery
+as a new failure. Coverage should distinguish cancellation in the provider,
+during immediate progress presentation, and between aggregate operations.
+
+## N2 — P2: Deferred-message acceptance remains open after the final batch detaches
+
+This is the same issue as
+[C2 in the correctness audit](01-correctness.md#c2--p2-finalization-can-discard-a-successfully-queued-deferred-message),
+reconfirmed here rather than a separate remediation item.
+
+**Code:** [reporter.go](../../../reporter.go), DeferredReporter.enqueue lines
+254–279 and Reporter.finalize lines 282–325.
+
+**Circumstances:** finalization detaches state.deferred and unlocks between
+deliveries. Another goroutine can acquire the mutex during that flush, append a
+visible message to the now-empty queue, and receive nil from its submission.
+The reporter has not yet set closed. The finalizer drains only the detached
+batch, then clears the new queue when closing.
+
+**Consequences and evidence:** the accepted message silently disappears, while
+Execute can return success and mark diagnostic delivery complete. The probe
+used one invocation, a briefly pausing diagnostic destination, and a goroutine
+submitting AtEnd().Warn during the flush. All three race-enabled runs accepted
+and lost the message. There was no unsynchronized memory access or callback
+reentry; the defect is the queue's acceptance/finalization ordering.
+
+**Potential remediation:** atomically close deferred acceptance when detaching
+the final batch, rejecting later submissions explicitly, or drain every accepted
+item before closure. Keep finalizer-owned delivery possible after acceptance
+closes; setting the reporter's existing closed flag too early would also reject
+its internal delivery. Preserve message visibility, ordering, and error evidence.
+Goroutines created by an application remain application-owned; this finding
+concerns an operation that already returned successful acceptance.
+
+## N3 — P2: Buffered configuration-show cancellation becomes a presentation failure
+
+This is the same issue as
+[C4 in the correctness audit](01-correctness.md#c4--p2-show-preparation-labels-ordinary-cancellation-as-a-rendering-error),
+reconfirmed here rather than a separate remediation item.
+
+**Code:** [show.go](../../../integrations/ckconfig/show.go), runConfigShow lines
+69–95, interacting with core Reporter.RenderOutput in
+[reporter.go](../../../reporter.go).
+
+**Circumstances:** cancellation becomes visible between buffered effective
+document conversions. The checkpoint inside RenderOutput returns ctx.Err as
+the rendering callback's error. Core records it as Output.RenderError before
+ckconfig returns cancellation as the operation cause.
+
+**Consequences and evidence:** normal cancellation is reported as both an
+operation failure and an incomplete output/rendering failure. A two-scope probe
+acquired both views; the first native writer emitted valid safe YAML, canceled
+the context, and returned nil. The second writer was skipped and no report
+bytes were delivered. All three race-enabled runs retained context.Canceled as
+Cause but also incorrectly recorded it as Output.RenderError, with no writer
+error. This can mislead callers distinguishing interrupted preparation from
+encoder or output failures.
+
+**Potential remediation:** retain preparation cancellation separately from the
+rendering callback's error, following renderPreparedConfigReport in
+[operations.go](../../../integrations/ckconfig/operations.go). Preserve real
+native-writing, conversion, and destination errors independently, and keep an
+incomplete aggregate report from being delivered. Retain the selected distinction
+between cancellation before delivery and cancellation after delivery has started.
+
+## Assessment
+
+N1 is new. N2 and N3 remain open in the correctness audit and should be tracked
+as the same underlying issues. No data race, supported-path deadlock,
+goroutine leak, or channel-lifecycle defect was established in the reviewed
+CommandKit code. The passing race suite supports the exercised synchronization
+paths; the three findings demonstrate why lifecycle and cancellation reasoning
+are also necessary.