commandkit/docs/roadmap/remediation.md

Dependency upgrade and PromptKit smoke review remediation

Status: in progress; R-1 through R-3 complete; R-4 remains open.

Review scope and evidence

Reviewed 29582dd..730f930b47c929fb2c59a25a75cbe2924dfc75bb against the accepted upgrade roadmap, implementation plan, architecture policy, and testing ownership policy. The worktree was clean at review start.

The intended architecture is substantially present:

• Core owns generic declarations, binding, installation, dispatch, help/version, diagnostics, and outcomes; its Cobra/pflag adapter remains internal.
• ckconfig owns ConfigKit adaptation and the extended source reports.
• ckprompt depends on public core/ckconfig/ConfigKit/PromptKit APIs. Its eight bundled inspection/catalog callbacks and two optional validation/smoke additions preserve the selected ownership boundary.
• Native IDs, optional versions, filters, source evidence, and smoke reports pass through the owning integrations. Aggregate smoke testing coordinates concrete callbacks in the selected order and retains partial reports/errors.
• ReleaseKit remains a root maintainer tool. The adoption leaves the launcher, profiles, shared-file selection, and source-only publication model intact.

This was a source, test, documentation, and dependency review. Readonly workspace and independent module listings confirmed ReleaseKit v0.4.2, ConfigKit v0.8.0, and PromptKit v0.16.0. Independent package dependency listings, including test imports, confirmed no optional integration libraries in core and no PromptKit/ckprompt dependency in ckconfig. Temporary manifests replaced only unpublished CommandKit prerequisites; upstream dependencies remained released modules. No test suite was run or added during this review.

The completed implementation plan records earlier test runs; those completion markers are historical evidence, not fresh test results from this review. No production correction was applied during the review. Completion records below describe the subsequent remediation. The preceding completed request-validation review remains available in Git history.

R-1: Preserve aggregate error presence when its message is empty

Status: complete. Priority: P2. Category: report fidelity.

Location: smoke_report.go, smokeResultDTO and encodeSmokeResults, lines 101–125.

Finding: The DTO uses Error string with yaml:"error,omitempty". encodeSmokeResults assigns result.Err.Error() only when the error is non-nil. A non-nil error whose message is "" therefore produces the same empty DTO field as no error, and omitempty drops it.

For example, a concrete callback returning a native report and errors.New("") still causes an operational aggregate outcome, but its YAML entry has no error key. Report consumers cannot distinguish that entry from one whose callback returned no error. No nonempty-error-message requirement was selected for application callbacks.

Required work: Track error presence separately from its display text. A private *string DTO field with omitempty is sufficient: nil for no error, and a pointer to the returned message for a non-nil error, including an empty message. Preserve the existing native report, aggregate ordering, joined cause identity, and normal outcome classification. Do not replace empty messages, reject the error, or add another error taxonomy.

Completion evidence: Add a focused semantic YAML regression covering nil, empty-message, and ordinary nonempty errors. Only nil omits error; an empty message remains error: "". Confirm the assembled aggregate still returns operational failure and preserves the original error through errors.Is. The appropriate focused tests and ckprompt suite pass.

Completed: 2026-10-18. Changed the private aggregate error field to an optional string pointer, preserving non-nil empty messages. Extended the YAML projection and assembled aggregate regressions to distinguish nil, empty, and nonempty errors while retaining operational classification and typed cause identity. Both focused regressions failed before the fix and passed afterward. The complete ckprompt suite, including the composed consumer, passed with Go 1.26.8 and offline dependency resolution.

R-2: Remove assertions about PromptKit's private smoke-test execution

Status: complete. Priority: P3. Category: upstream testing ownership.

Location: adapters_test.go, TestWithSmokeTestingForwardsAuthoritativeNativeRequestAndRetainsReport, line 167, and TestWithSmokeTestingNilBuilderForwardsSeedAndBuilderFailureSkipsLoader, line 182.

Finding: The nil-builder test requires the native generated prompt to contain exactly one message. Both tests require exactly one call to the injected native LLM client. CommandKit forwards a SmokeTestRequest to Engine.SmokeTest; PromptKit constructs its private diagnostic prompt and owns its internal generation sequence.

A legitimate upstream change to the private prompt shape or diagnostic generation sequence could fail these tests without any incorrect CommandKit adaptation. The selected testing boundary permits observing the client to verify forwarding, but does not establish private native work as a CommandKit contract.

Required work: Remove the exact native message-count assertion and exact native generation-count expectations. Retain evidence that the intended native path is reached and that the seed or authoritative builder request controls the observed model/credential values. Keep report/error preservation, configuration selection, context/input/state forwarding, and builder-failure ordering checks. Where counting is useful, checking that native client use occurred is sufficient; do not fix its native internal call count.

This does not remove zero-call guards for help, metadata operations, or failures that must stop before acquisition. Those protect selected CommandKit behavior.

Completion evidence: Focused helper tests and the ckprompt suite pass. Assertions still detect request remerging, wrong forwarding, or dropped results, without inspecting the native private diagnostic message inventory or fixing the generation sequence. No production-code change is required.

Completed: 2026-10-18. Removed the private diagnostic message-count assertion and changed native generation-count checks to require only that the configured client was used. Request/model/credential forwarding, authoritative builder output, report/error handling, context/input/state checks, and the builder-failure guard against loading remain covered. Focused TestWithSmokeTesting tests and the complete ckprompt suite, including the composed consumer, passed with Go 1.26.8 and offline dependency resolution. Production code is unchanged.

R-3: Remove incidental loader-count and pointer-identity contracts

Status: complete. Priority: P3. Category: unnecessary implementation constraints.

Locations:

• smoke_lifecycle_test.go, TestSmokeTestBuilderReceivesConcreteAggregateKinds, lines 201–212.
• smoke_aggregate_test.go, TestSmokeAggregateCoordinatesConcreteRequestsAndRetainsErrors, lines 89–95 and its pointer-collection bookkeeping.

Finding: The builder test requires exactly four engine-loader calls. The accepted aggregate behavior requires per-kind requests/builders and allows engine/acquisition reuse where practical; it deliberately does not guarantee a loader-call count.

The aggregate test also requires pairwise distinct addresses for every execution override and selected scalar pointer. It already checks the meaningful behavior: mutating the first seed must not change the common CLI values seen by subsequent operations. Pairwise allocation identity adds a separate constraint on storage strategy that the operator and consumer contracts do not require.

The implementation plan describes a straightforward fresh-storage approach. That instruction does not independently establish public pointer identity or resource-management guarantees under the agent policy.

Required work: Remove the exact aggregate loader count and pairwise pointer inequality assertions, together with bookkeeping used only for those assertions. Retain concrete builder/callback order, per-kind builder inputs, shared invocation input/state, subsequent seed values after mutation, continuation, cancellation, and native error identity. Keep justified guards preventing loading after builder failure and during help/version. Do not add caching, pooling, or a public lifetime abstraction to resolve a test-only issue.

Completion evidence: Focused aggregate/lifecycle tests and the ckprompt suite pass. The tests continue to detect cross-test value contamination and missing operations, while accepting equivalent acquisition and storage implementations. No production-code change is required.

Completed: 2026-10-18. Removed the aggregate loader counter and pairwise pointer-address assertions/bookkeeping. The tests retain ordered concrete builder/callback inputs, shared input/state, mutation-isolated seed values, continuation, cancellation, reports, and error identity. Focused aggregate, builder, help/version, and builder-failure tests and the complete ckprompt suite, including the composed consumer, passed with Go 1.26.8 and offline dependency resolution. Production code is unchanged.

R-4: Reconcile the feature roadmap with completed implementation

Status: open. Priority: P3. Category: stale implementation status.

Location: upgrades.md, status at lines 3–6, current-implementation descriptions at lines 28–29 and 41–44, and the source DTO statement at line 124.

Finding: The roadmap still says implementation has not started and that the current source DTO omits all four new evidence fields. The code now exposes those fields, and the implementation plan marks Stages 1–18 complete. Other present-tense descriptions still refer to the planning baseline.

Stage 18 expressly called for updating the roadmap's implementation status. Leaving these claims in place makes implemented features appear absent and can cause a later agent to restart completed work.

Required work: Record the delivered implementation and link to the completed plan/current canonical documentation. Qualify old baseline descriptions and version tables as historical rather than rewriting the planning baseline. Remove or recast assertions that the current DTO/API lacks delivered behavior. Preserve accepted decisions, dependency boundaries, and the remaining review findings; feature delivery can be recorded as complete while these focused remediations remain open.

Completion evidence: The roadmap and implementation plan agree on delivery status, current-code claims match the inspected implementation, and local links/Markdown whitespace checks pass. No code or policy change is required.