diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md new file mode 100644 index 0000000..6df7fb1 --- /dev/null +++ b/docs/roadmap/remediation.md @@ -0,0 +1,140 @@ +# Shared reporting and PromptKit adoption remediation + +Status: three open findings from review of `f83866e44a8a9066db52cbd2aa5cccde0d69a839`. + +## Review basis and boundaries + +The review covers the implementation since `105bb74`, against the final +[feature roadmap](https://forge.maximumdirect.net/base-layer/commandkit/src/commit/2f43f78f76439f5d11ef313cd379bec036543504/docs/roadmap/promptkit-v0.20.0.md) +and [implementation plan](https://forge.maximumdirect.net/base-layer/commandkit/src/commit/2f43f78f76439f5d11ef313cd379bec036543504/docs/roadmap/implementation.md), +now retired into Git history. The [architecture policy](../policy/architecture.md), +[CLI reference](../cli.md), and public GoDoc govern current boundaries. + +Core supplies command mechanics, typed input composition, CLI JSON parsing, +shared report representation/rendering, and the existing invocation reporting +and outcome lifecycle. ckconfig supplies declared scopes, independent comparison +inputs, opt-in overrides, and configuration report content. ckprompt adapts +released PromptKit v0.20.0 resources, preparation, namespaces, requirements, +smoke evidence, and configuration inspection through those shared contracts. +Applications retain loading and execution; upstream libraries retain domain +semantics. Module requirements select released core and ckconfig v0.4.0, +ConfigKit v1.1.0, and PromptKit v0.20.0. + +All three independent module suites and all three aligned workspace suites +passed with Go 1.27.0. Temporary external-consumer probes using local module +replacements reproduced the findings below. Those probes did not modify the +repository's source or tests. No additional strengthened invariant, prohibition, +or guarantee was identified; the findings concern incomplete report/evidence +adaptation, not a request for stronger domain validation or new guarantees. + +## R1 — P2: Explain still uses a separate provider text renderer + +**Location:** [explain.go](../../integrations/ckconfig/explain.go), +`runConfigExplanation`, particularly lines 160–169. + +The handler prepares an explanationReport, but its text template is only a +`{{render .}}` thunk. That function calls the acquired provider's +WriteExplanation again and appends explicit/null lines. YAML/JSON serialize the +prepared report instead. This preserves the old parallel rendering path inside +a template instead of completing the shared handoff selected for the output +inventory and Stages 7/23. Formatting still depends on the provider after the +report content has been selected. + +**Observed consequence:** an external provider returning a valid Explain result +and an error from WriteExplanation succeeds with `--format json` but fails with +status 1 under text/default formatting. The error comes from the separate +provider writer, not the selected report data. The maintained simple consumer +also demonstrates different selected evidence: JSON includes scope and metadata +history/previous-origin records, while text calls the native writer and appends +only current explicit/null metadata. Native text can introduce source labels +that are absent from the handed-off projection. + +**Required change:** render text from the same acquired projection as the other +formats. Prepare a reusable ordinary template and project any selected source +labels or metadata needed by it before handoff. Stop invoking WriteExplanation +from the template. Keep upstream safe evidence and meaning; no second loading +or explanation algorithm, custom template language, or per-format report tree. + +**Verification:** use an explanation provider whose Explain result is valid and +whose WriteExplanation records or rejects calls. Confirm the standard handler +uses its selected report value in text, YAML, and JSON without calling that +writer. Check selected scope, current/historical metadata, and available +provenance semantically across formats, plus ordinary presentation failures. +Do not freeze incidental text layout. + +## R2 — P2: PromptKit configuration cancellation discards captured evidence + +**Location:** [adapters.go](../../integrations/ckprompt/adapters.go), +loadPromptKitEngine lines 487–495 and ConfigScope.Acquire lines 276–287. + +After the loader returns an engine, loadPromptKitEngine obtains a native +ConfigurationInspection. If the context is then canceled, it returns a zero +inspection together with the cancellation error. ConfigScope's error branch +only supplies structured facets for a ConfigKit LoadError, so the inspection's +already-captured source inventory and diagnostics are lost. + +**Observed consequence:** a loader that returns a valid engine and cancels the +invocation before returning yields context.Canceled with Sources and Findings +both nil, even though direct InspectConfiguration on that engine supplies a +source inventory. The probe retained one native source before adaptation and +none afterward. Detailed validation therefore reports evidence as unavailable +instead of the available native state; source listing cannot report the acquired +inventory. This conflicts with OD-6's preservation of reached-scope evidence +when cancellation stops acquisition. The validation handler's post-cancellation +submission cannot recover data already discarded by its adapter. + +**Required change:** retain an explicitly available native inspection through +the acquisition error path and expose its safe Sources/Findings to ConfigScope. +Keep cancellation as the operation failure, and keep effective configuration, +metadata, and ordinary resource/request work unavailable after the failed +acquisition. Preserve the distinction between unavailable and available-empty +evidence. Do not resume canceled acquisition or add a blanket reporting guard. + +**Verification:** return an engine from a loader that cancels the context after +successful loading. Compare the adapter's retained sources/diagnostics with the +native inspection, and exercise source reporting and detailed aggregate +validation. Confirm context.Canceled identity, no unvisited-scope success, and +separate presentation failure. Existing ordinary inspection cancellation behavior +should continue to stop domain work. + +## R3 — P2: A native ConfigKit result accompanying an error loses evidence + +**Location:** [configkit_adapter.go](../../integrations/ckconfig/configkit_adapter.go), +FromConfigKit.Acquire lines 35–47. + +The adapter now handles err before examining result. When an application loader +returns an acquired native Result plus a non-LoadError failure (for example, a +post-load application check), it returns an empty View and drops the result's +captured sources and diagnostics. The previous adapter inspected a nonnil result +before handling the error. Restricting effective/metadata availability after +failure should not also discard the safe evidence already supplied by a native +result. + +**Observed consequence:** a loader returning a successfully loaded native result +with one source plus a sentinel application error preserves the error identity, +but Sources and Findings are nil. Consequently `config sources` says inventory +is unavailable and detailed validation cannot expose the available findings. +This falls short of the selected failed-acquisition evidence behavior without +requiring any additional loading or upstream inference. + +**Required change:** when a nonnil native result accompanies such a failure, +retain its source/diagnostic evidence and associated ordinary messages while +returning the original error. Do not expose effective configuration or metadata +as successful acquisition facets. A matching ConfigKit LoadError should continue +to supply its authoritative failure snapshot without duplicating it with result +evidence. Nil result plus a generic error may legitimately have no evidence. + +**Verification:** cover result-plus-application-error and result-plus-cancellation +with source reporting and detailed validation; compare retained facts with that +result's native inspection data. Preserve original causes, available-empty versus +unavailable findings, ordinary warnings, and separate presentation errors. +Retain a matched LoadError case to prevent duplicate inventories/diagnostics. + +## Applying the fixes + +Keep fixes and regression coverage at the CommandKit adaptation/reporting +boundaries under the [testing policy](../policy/testing.md). These findings do +not call for reimplementation or independent verification of upstream domain +algorithms. Follow the [release procedure](../release.md) and existing +immutability policy for any subsequent released dependency alignment; do not +rewrite published core or ckconfig v0.4.0 contents.