eric/promptkit
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Record engine operation audit findings

Browse Source
This commit is contained in:
Eric Rakestraw
2026-08-11 14:43:20 +00:00
parent 3b4ea21208
commit 0678d242b9
1 changed files with 161 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

161
audit.md
View File

@@ -583,3 +583,164 @@ S03-F01. The temporary source and generated file were removed.
repositories, validators, provider transport, and capacity scheduling
respectively. This stage established only that `NewEngine` selects and wires
their boundaries consistently.
## Stage 4: Engine Operations And Root Contract Coverage
### Scope Reviewed
The review covered the public operation portion of `engine.go`
(`InspectPrompt`, `InspectProfile`, `Prepare`, `PrepareExecution`, `Run`, and
`RunPrepared`) and the directly relevant root tests in `engine_test.go`,
`public_contract_test.go`, `prepared_execution_contract_test.go`, and
`errors_internal_test.go`. `prepared_execution.go` and conversion helpers were
consulted only to confirm the operation boundary established in Stage 1.
Internal runner tests were consulted only to identify test ownership; runner,
transport, validation, capacity, and repository mechanics were treated as
black boxes.
### Accepted Findings
#### S04-F01: Ordinary-run cancellation identity is protected only below the public boundary
- **Category:** testing
- **Severity:** medium
- **Confidence:** high
- **Status:** accepted
- **Affected code:** `engine.go` (`Engine.Run`), `errors.go`
(`mapPublicError`), `engine_test.go`
(`TestEngineRunPropagatesCallerCancellation`),
`errors_internal_test.go`
(`TestMapPublicErrorPreservesGenerationCancellation`), and
`internal/usecase/runner_test.go`
(`TestRunnerRunCancellationPreservesGenerationCategory`)
- **Contract at issue:** `Engine.Run` passes the caller's context through the
execution boundary and preserves the active collaborator's cancellation
identity while adding the public operation category. Cancellation and
injected dependency failures are consequential public behaviors that should
be asserted through the consumer-visible boundary.
- **Evidence:** `Engine.Run` currently passes `ctx` unchanged to the runner and
maps its returned error without dropping wrapped identities. The external-
package cancellation test drives a real request context through the built-in
HTTP transport but asserts only `errors.Is(err, ErrLLMGenerate)` after
cancellation. The `context.Canceled` identity is asserted separately only
against the unexported `mapPublicError` helper and internal runner. A search
of the root operation tests found no other ordinary-run assertion for that
identity. Prepared execution does assert both identities at the public
boundary, but it exercises a different entry point.
- **Failure mode:** A facade or ordinary-run composition change could replace
the caller context, stop wrapping the collaborator cancellation, or discard
it during public error mapping. The internal tests and existing public test
could all remain green while consumers lose the ability to distinguish
caller cancellation with `errors.Is(err, context.Canceled)`.
- **Recommended direction:** Extend the existing external-package
`TestEngineRunPropagatesCallerCancellation` assertion to require both
`ErrLLMGenerate` and `context.Canceled`. Retain the focused internal tests
only for the distinct internal translation and runner responsibilities they
protect; do not add a parallel end-to-end cancellation test.
- **Required verification:** Run the focused public cancellation test normally
and with the race detector. Confirm that deliberately removing caller-context
propagation or cancellation wrapping at the facade boundary makes that test
fail.
### Unresolved Observations
None. The absence of package-level operation convenience functions was
confirmed and is not a consistency defect; the reviewed public API exposes
these operations only as `Engine` methods.
### Coverage Ledger
- **Facade shape and request translation:** All six methods reject a nil or
uninitialized engine before delegation. `Prepare`, `PrepareExecution`, and
`Run` use the same field-complete, defensive request conversion and classify
conversion failures as `ErrInvalidRequest`; inspections pass their scalar
selectors with the documented prompt/profile normalization behavior.
`RunPrepared` unwraps only the opaque handle reference. Each successful
facade method delegates once and converts the returned domain snapshot.
- **Context propagation:** Every method passes the supplied context directly
to its matching runner operation. Public tests protect cancellation before
inspection source work, active ordinary generation, and prepared generation,
and prove that a completed preparation is independent of later cancellation
of its preparation context. The missing consumer-boundary assertion for the
ordinary-run cancellation identity is recorded as S04-F01.
- **Inspection operations:** Prompt inspection loads declared metadata and
referenced content without profile, artifact, schema, capacity, or provider
work. Profile inspection resolves the effective target and credential state
without prompt or generation work. External-package tests protect nil and
blank inputs, not-found versus load identities, cancellation, point-in-time
behavior, agreement with preparation, and deep ownership of returned nested
values.
- **Preparation and ordinary execution:** `Prepare` returns a caller-owned,
credential-redacted prepared snapshot and performs no model generation.
`Run` returns a caller-owned result after one execution path; content
validation failure remains a successful result, while operational failures
return no partial result. Root tests protect representative translation,
prepared/generated metadata agreement, injected artifact and LLM behavior,
validation-result semantics, and the documented public error categories.
- **Prepared execution boundary:** `PrepareExecution` publishes one opaque,
engine-bound handle whose details are independent copies of frozen
preparation state. `RunPrepared` preserves owner binding, atomic single-use
claim behavior, independent execution context, no-result-on-error semantics,
collaborator identities, credential revalidation, capacity rejection, and
execution-only timing. External-package tests also protect concurrent claim
and run/discard behavior. The internal claim, admission, validation, and
release mechanisms remain assigned to later component stages.
- **Public error mapping:** Every runner error is routed through one facade
mapping point. Ordinary public identities and injected collaborator errors
remain discoverable with `errors.Is`; capacity failures become public
`CapacityError` values without leaking the internal type; successful paths
do not invent errors. Internal mapping tests appropriately own internal-type
containment, while public operation tests own consumer-visible categories
and collaborator identities except for S04-F01.
- **Ownership:** Request conversion freezes caller maps, slices, pointers, and
JSON-compatible values before internal use. Inspection, prepared, details,
and result conversions return fresh mutable values. The prepared-execution
contract tests demonstrate that later caller and source mutations do not
change frozen execution and that mutating one returned snapshot does not
change engine-owned state.
- **Convenience-function consistency:** The graph and source search found no
package-level `Prepare`, `PrepareExecution`, `Run`, `RunPrepared`,
`InspectPrompt`, or `InspectProfile` functions. There is therefore no second
operation surface whose translation, errors, or ownership can drift from
the methods.
- **Test ownership and duplication:** Root external-package tests protect the
exported facade and representative assembled workflows. Focused internal
tests own runner coordination and public-error translation mechanics. Some
lifecycle and error categories necessarily appear at both levels, but the
assertions address different stable boundaries; no removable semantic
duplication was found. S04-F01 is the one important identity currently
asserted only below the applicable public operation boundary.
- **Clarity and cost:** The operation facade is a uniform sequence of guard,
translation where needed, one delegation, public error mapping, and outward
conversion. No duplicated orchestration policy, repeated I/O, avoidable
copying, or operation-layer complexity was found. Costs inside preparation,
generation, validation, transport, and capacity remain assigned to their
owning later stages.
### Verification Performed
The code knowledge graph was used to find every public engine operation,
confirm their runner and conversion edges, inventory directly relevant root
tests, locate the internal cancellation assertions, and verify that no
package-level operation convenience functions exist. Important context,
error, ownership, and no-partial-result conclusions were confirmed against
source.
The following focused commands passed:
```sh
go test . -run 'Test(PrepareWorksWithFrameworkContractCorpus|RunSucceedsWithInjectedLLMClient|RunPassesPreparedRequestToInjectedLLMClient|EngineRunPropagatesCallerCancellation|ArtifactReaderFailuresPreserveArtifactLoadErrors|RunAddsLLMGenerateToCollaboratorPublicError|PrepareWithoutProfileMatchesSpecificPublicError|RunValidationFailureReturnsResult|PublicErrorsSupportErrorsIs|InspectProfileResolvesCredentialStatesWithoutPromptOrGeneration|InspectProfilePreservesPublicErrorIdentities|InspectProfileReturnsIndependentTargetMatchingPreparation|InspectPromptReturnsDeclaredMetadataWithoutExecutionWork|InspectPromptPreservesPublicErrorIdentities|InspectPromptReturnsIndependentMetadataMatchingPreparation|PreparedExecutionFreezesSourcesAndReturnsIndependentDetails|PreparedExecutionLifecycleAndEngineBinding|PreparedExecutionConcurrentClaimAllowsOneGeneration|PreparedExecutionRunAndDiscardRaceHasOneWinner|PreparedExecutionDiscardAndFormattingDoNotExposePrivateState|PreparedExecutionCredentialCapacityAndTimingBoundaries)$'
go test -race . -run 'Test(EngineRunPropagatesCallerCancellation|InspectProfilePreservesPublicErrorIdentities|InspectPromptPreservesPublicErrorIdentities|PreparedExecutionFreezesSourcesAndReturnsIndependentDetails|PreparedExecutionLifecycleAndEngineBinding|PreparedExecutionConcurrentClaimAllowsOneGeneration|PreparedExecutionRunAndDiscardRaceHasOneWinner)$' -count=3
go test ./internal/usecase -run 'TestRunnerRunCancellationPreservesGenerationCategory'
go test . -run 'Test(MapPublicErrorPreservesGenerationCancellation|MapPublicErrorTranslatesCapacityError)$'
```
### Handoff
- The Stage 0 baseline remains absent and was not backfilled during this
operation-boundary review.
- Stages 7, 8, 10, 11, 13, 14, and 15 own prompt loading, profile loading,
validation, runner preparation/execution, prepared-handle lifecycle,
provider transport, and capacity mechanics respectively. Those stages
should treat the facade behavior recorded here as their outward contract
rather than repeat this public-boundary audit.