commandkit/docs/roadmap/remediation.md

PromptKit request validation review remediation

Status: complete; R-1 and R-2 addressed. No production-code correction or architectural redesign was identified as necessary.

Review scope and evidence

Reviewed implementation commits after planning baseline 3218222, through 420483b2d4659862626422b0577e39bc0a4c1a2d, against the accepted feature roadmap, implementation plan, applicable ADRs, and repository policy. The review covered production changes, new tests, the maintained consumer, dependency metadata/attribution, and updated documentation.

The targeted architecture is present:

• Core retains generic dispatch, binding, registration, and outcome ownership.
• ckconfig retains configuration adaptation without depending on PromptKit.
• ckprompt owns the independent validation capability, request envelope, suite-returning adapter, standard command, and YAML confirmation.
• The bundled adapter builds before loading, forwards the resulting native request without merging or independent identity checks, and delegates to Engine.ValidateRequest. Nil-builder forwarding and retained wiring issues are implemented.
• Applications retain typed input/state, defaults, artifact mapping, and runtime ownership. Requirements discovery remains separate and permits partial results.

Review verification passed:

• Fresh GOTOOLCHAIN=local go test -count=1 ./... runs from each of the three module directories, including the maintained composed consumer.
• Fresh isolated GOWORK=off, GOTOOLCHAIN=local, -mod=readonly suite runs for all three modules. Temporary manifests replaced only unpublished CommandKit prerequisites; their module/sum hashes stayed unchanged during verification.
• Isolated module and test-import listings confirm the core/ckconfig boundaries; ckprompt selects released PromptKit v0.15.0 without a PromptKit replacement.
• Changed prose whitespace and local documentation links/anchors were checked.

The findings below concern test constraints that would make otherwise valid refactoring fail. They do not indicate an incorrect runtime result. Address them without changing production behavior or expanding public contracts.

R-1: Remove the required factory call after missing-selector failure

Status: complete. Severity: low. Category: unintended lifecycle guarantee in a test.

Location: validation_command_test.go, TestValidationCommandUsageAndInputFailuresStopProviderWork, lines 276–283.

Finding: The missing-required-selector case sets wantInputCalls = 1 and requires that exact factory count. The comment explains the current private prepareSuiteInput order, rather than an accepted requirement. A refactor that detects the missing selector before allocating application input would return the same usage failure and correctly skip later work, but this test would fail.

ADR 0011 allows failures to stop later input factories. Suite GoDoc likewise says factories need not run after failure. The selected contract requires the usage classification and stopping input checks, state creation, and provider work; it does not require an input factory to run for this failed invocation.

Required work: Remove the exact input-factory count assertion for this case and the comment that treats the private ordering as a required lifecycle. Retain the appropriate usage status and assertions that CheckInput, NewState, and the provider remain uninvoked. Preserve zero-factory assertions for malformed syntax and unsupported-format preflight cases where that behavior is selected. Keep successful-invocation and help/version lifecycle coverage.

Completion evidence: The focused test and ckprompt suite pass; the missing-selector case accepts early failure before application input allocation without losing its guards against later work. No production change is required.

Completed: 2026-10-12. Removed the missing-selector factory-count requirement and private-ordering comment. Usage classification and checks preventing input checks, state creation, and provider work remain; syntax/format cases still require zero input-factory calls. The focused TestValidationCommandUsageAndInputFailuresStopProviderWork test and GOTOOLCHAIN=local go test -count=1 ./... passed from integrations/ckprompt, including the composed consumer. Production code is unchanged.

R-2: Replace the complete provider-failure stderr snapshot

Status: complete. Severity: low. Category: unnecessary presentation constraint in a test.

Location: validation_command_test.go, TestValidationCommandRetainsProviderDiagnosticsAndClassifiesFailures, line 394.

Finding: The provider-failure case requires the entire diagnostic stream to equal "validation warning\nvalidation provider failed\n". This freezes the unadorned error presentation and exact line layout. An otherwise valid shared diagnostic formatting change, such as adding an error label, would fail this validation-command test even while preserving cause identity, classification, warning delivery, and single presentation.

ADR 0005 selects diagnostic ownership, failure classification, supported error identity, and delivery handling. It does not select this complete text snapshot. The testing policy calls for minimal semantic error fragments unless exact wording is contractual.

Required work: Replace the complete failed-stderr equality with focused checks that the synthetic warning and provider error are delivered once. Retain operational classification/status, errors.Is cause identity, retained diagnostics, and applicable delivery-state checks. Controlled diagnostic-writer payload checks can remain; this finding does not require removing exact checks of bytes explicitly supplied by the test's own writer.

Completion evidence: The focused test and ckprompt suite pass and still detect lost or duplicate diagnostics without fixing incidental error decoration or line layout. Preserve current production delivery behavior and order.

Completed: 2026-10-12. Replaced the failed-stderr snapshot with independent single-occurrence checks for the synthetic warning and provider error. Retained operational classification, status, cause identity, and diagnostic payload checks, and verified complete diagnostic delivery. The focused TestValidationCommandRetainsProviderDiagnosticsAndClassifiesFailures test and GOTOOLCHAIN=local go test -count=1 ./... passed from integrations/ckprompt, including the composed consumer. Production formatting and delivery are unchanged.