2026-10-12 23:27:27 +00:00
|
|
|
|
# PromptKit request validation review remediation
|
|
|
|
|
|
|
2026-10-12 23:42:18 +00:00
|
|
|
|
Status: one open finding; R-1 complete. No production-code correction or
|
2026-10-12 23:27:27 +00:00
|
|
|
|
architectural redesign was identified as necessary.
|
|
|
|
|
|
|
|
|
|
|
|
## Review scope and evidence
|
|
|
|
|
|
|
|
|
|
|
|
Reviewed implementation commits after planning baseline `3218222`, through
|
|
|
|
|
|
`420483b2d4659862626422b0577e39bc0a4c1a2d`, against the
|
|
|
|
|
|
[accepted feature roadmap](promptkit-validation.md),
|
|
|
|
|
|
[implementation plan](implementation.md), 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
|
|
|
|
|
|
|
2026-10-12 23:42:18 +00:00
|
|
|
|
Status: complete. Severity: low. Category: unintended lifecycle guarantee in a test.
|
2026-10-12 23:27:27 +00:00
|
|
|
|
|
|
|
|
|
|
**Location:**
|
|
|
|
|
|
[validation_command_test.go](../../integrations/ckprompt/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](../adr/0011-captured-declarations-and-installation.md#input-and-shared-state-lifetimes)
|
|
|
|
|
|
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.
|
|
|
|
|
|
|
2026-10-12 23:42:18 +00:00
|
|
|
|
**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.
|
|
|
|
|
|
|
2026-10-12 23:27:27 +00:00
|
|
|
|
## R-2: Replace the complete provider-failure stderr snapshot
|
|
|
|
|
|
|
|
|
|
|
|
Status: open. Severity: low. Category: unnecessary presentation constraint in a test.
|
|
|
|
|
|
|
|
|
|
|
|
**Location:**
|
|
|
|
|
|
[validation_command_test.go](../../integrations/ckprompt/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](../adr/0005-invocation-outcomes.md) selects diagnostic ownership,
|
|
|
|
|
|
failure classification, supported error identity, and delivery handling. It
|
|
|
|
|
|
does not select this complete text snapshot. The
|
|
|
|
|
|
[testing policy](../policy/testing.md#go-specific-guidance) 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.
|