From 5177db0611a647e1bd02e5df1f373481eab4a406 Mon Sep 17 00:00:00 2001
From: Eric Rakestraw <eric@rakestraw.us>
Date: Wed, 4 Nov 2026 15:29:58 +0000
Subject: [PATCH] Record API and interface contract audit findings

---
 docs/roadmap/audit/09-interfaces.md | 199 ++++++++++++++++++++++++++++
 1 file changed, 199 insertions(+)
 create mode 100644 docs/roadmap/audit/09-interfaces.md

diff --git a/docs/roadmap/audit/09-interfaces.md b/docs/roadmap/audit/09-interfaces.md
new file mode 100644
index 0000000..054ef93
--- /dev/null
+++ b/docs/roadmap/audit/09-interfaces.md
@@ -0,0 +1,199 @@
+# API And Interface Contract Audit
+
+Reviewed 2026-11-04 against commit `31f1e6a` and the current worktree. Existing
+audit-document moves were preserved. No production code or repository tests
+were changed.
+
+## Scope And Assessment
+
+The public root package remains a facade over configuration, execution,
+generation, capacity, rendering, validation, and observation components.
+ConfigKit owns composition before the final configuration handoff; PromptKit
+validates that handoff and captures its runtime indexes and templates. Public
+adapters translate owned request/result values and errors. Namespace views
+share execution ownership and capacity, while separately constructed engines
+remain independent.
+
+The review compared exported GoDoc, configuration and integration contracts,
+internal collaborator interfaces, call paths, and existing behavioral tests.
+It covered preparation/preflight/execution parity, prepared-handle lifecycle,
+optional values and numeric presence, configuration inspection and listing,
+injected collaborator ownership, output repair, diagnostics, observations,
+serialization, cancellation, and public error categories. It did not treat
+different intentionally documented workflows as defects or infer additional
+admission, filesystem, provider, or semantic-validation guarantees.
+
+Four findings warrant remediation. The configuration behaviors in findings 2
+and 3 are consistent with the current format reference and implementation;
+their older contradictory descriptions should be corrected rather than used
+to change those behaviors.
+
+## 1. ArtifactReader Permits Buffer Reuse Before PromptKit Can Copy It
+
+**Priority:** P2. A documented ownership permission permits data races and
+inconsistent input snapshots.
+
+**Relevant interfaces/code:** [ArtifactReader GoDoc](../../../types.go:294),
+[public reader adapter](../../../artifact_reader.go:19), and
+[copyBytes](../../../convert.go:490).
+
+**Contract defect:** The interface says that `Read` may be called concurrently
+and that the reader may reuse or mutate the returned artifact and body after
+`Read` returns. The adapter can only copy those values after that return.
+There is no synchronization or acknowledgement between return and completion
+of the copy. “Immediately copies” does not make concurrent reuse safe.
+
+**Circumstances and consequence:** A reader serializes its own `Read` calls
+with a mutex and returns a reusable buffer. Another invocation can overwrite
+that buffer as soon as the first call returns, while the first adapter is still
+copying it. Both calls follow the published reuse permission, yet preparation
+can race, mix contents, or associate metadata with the wrong bytes. This is
+not a demonstrated defect in the default file reader or in readers that
+return independent snapshots.
+
+**Evidence:** A temporary, offline public-API diagnostic used two concurrent
+`Prepare` calls and a mutex-protected reader returning a shared 1 MiB buffer.
+`go run -race` reported a write in the next `Read` racing with `copyBytes`
+through `publicArtifactReaderAdapter.Read`. The adapter's existing copy test
+establishes detachment after copying, not safety during that copy.
+
+**Potential remediation:** Resolve the return-value ownership contract before
+v1.0. The straightforward approach is for a successful `Read` to return a
+per-invocation snapshot that remains stable while PromptKit consumes it, with
+reuse permitted after the enclosing preparation/execution method returns.
+Returning fresh owned data is the simplest implementation for concurrent
+readers. If reuse immediately after `Read` itself returns must remain
+supported, it requires an explicit borrowing/release arrangement; another
+unsynchronized copy cannot provide that permission. Add focused coverage for
+the selected lifetime, without globally serializing unrelated engine work.
+
+## 2. Profile Inspection Misstates Required-Credential Semantics
+
+**Priority:** P2. The public inspection contract describes valid results as
+impossible and incorrectly limits the sources that can satisfy a requirement.
+
+**Relevant interfaces/code:** [Engine.InspectProfile](../../../engine.go:313),
+[ProfileInspection.APIKeyRequired](../../../types.go:372),
+[profile-inspection consumer example](../../consumers/pkg-promptkit.md#inspect-a-profile-before-prompt-work),
+[effective-target construction](../../../internal/configuration/targets.go:99),
+and [credential availability check](../../../internal/usecase/runner.go:611).
+
+**Contract defect:** Both method and field GoDoc claim that `APIKeyRequired`
+is mutually exclusive with a nonblank `EffectiveModelParams.APIKeyEnv`. They
+also describe the flag as requiring a direct credential or a request-level
+environment override. In fact, `api_key_required: true` suppresses a
+backend-derived environment source only when no explicit profile source was
+provided. An explicit profile environment source remains usable, as the
+[credential format reference](../../formats.md#credentials) already states.
+Inspection can therefore legitimately return both the flag and a nonblank
+environment name. The consumer example also calls every reported environment
+source optional, regardless of the flag.
+
+**Circumstances and consequence:** A profile sets `api_key_required: true` and
+`api_key_env: PROMPTKIT_INTERFACE_AUDIT_KEY`. Inspection returns `true` and
+that name; preparation succeeds using its populated environment variable
+without a direct key or request override. Consumers following the GoDoc may
+reject this valid combination or report credential readiness incorrectly.
+
+**Evidence:** A temporary public-API diagnostic demonstrated that combination
+and successful preparation. The existing
+[effective-target test](../../../internal/configuration/targets_test.go:88)
+also includes a required profile with an explicit environment source.
+
+**Potential remediation:** Describe the flag as a local credential-availability
+requirement, distinguish source selection from required availability, and
+remove the mutual-exclusion/direct-only claims. Align the consumer explanation
+with the format reference and retain inspection's intentional lack of an
+environment-value lookup. Do not reject the valid configuration or change
+the public target's stable JSON shape to make the old wording true.
+
+## 3. The Wire Contract Incorrectly Says Configured Provider Maps Replace
+
+**Priority:** P2. Conflicting published contracts predict different outbound
+provider parameters.
+
+**Relevant interfaces/code:** [integration request-body contract](../../integrations/openai-compatible-chat.md#request-body),
+[profile defaulting reference](../../formats.md#defaults-and-overrides),
+[overlayProviderMap](../../../internal/configuration/targets.go:133), and
+[runtime override merge](../../../internal/usecase/runner.go:562).
+
+**Contract defect:** The integration document says backend, profile, and request
+extra-parameter maps all use whole-map replacement. Actual configured profile
+maps recursively overlay backend defaults, including nested maps. A nonempty
+runtime `ExecutionTargetOverride.ExtraParams` instead replaces the entire
+configured map. The format reference and runtime-override GoDoc describe
+these distinct behaviors correctly.
+
+**Circumstances and consequence:** A backend supplies `backend_only: true` and
+`nested: {backend: true}`; its profile supplies `profile_only: true` and
+`nested: {profile: true}`. All entries survive into the effective target and
+built-in HTTP body. A reader of the integration contract would expect the
+backend entries to disappear, potentially leading to surprising provider
+options or ineffective attempts to clear defaults. A runtime map containing
+only `request_only: true` does remove the configured entries.
+
+**Evidence:** A temporary public-API diagnostic observed both forms through an
+injected client and confirmed the configured merged fields through a controlled
+built-in HTTP transport. No external request was made. The existing
+[effective-target test](../../../internal/configuration/targets_test.go:70)
+also exercises recursive configured overlay.
+
+**Potential remediation:** Replace the stale configuration-precedence claim
+with a short link to the format reference. Keep the integration contract
+focused on flattening the resulting effective map into the HTTP body and on
+reserved request fields. Preserve the distinct runtime replacement behavior;
+do not add another merge policy or duplicate its full configuration reference.
+
+## 4. Identity GoDoc Still Describes Removed Normalization And Version Fields
+
+**Priority:** P3. Small but concrete public-contract inconsistencies.
+
+**Relevant interfaces/code:** [ProfileInspection.ProfileID](../../../types.go:364),
+[ListPrompts GoDoc](../../../listing.go:90), and
+[literal profile identity test](../../../profile_inspection_contract_test.go:13).
+
+**Contract defect:** `ProfileInspection.ProfileID` is described as trimmed,
+but configured IDs and lookups preserve meaningful whitespace exactly.
+`ListPrompts` promises ownership of each `Version` pointer even though
+`PromptIdentity` contains only `ID`; collection-level `ReleaseVersion` belongs
+to the separate collection inventory.
+
+**Circumstances and consequence:** Inspecting the valid profile `" padded "`
+returns that exact ID, while looking up `"padded"` fails. Callers expecting the
+documented trimmed identity can fail later lookups. The nonexistent version
+field creates an erroneous expectation about available prompt metadata.
+
+**Evidence:** The existing literal-identity test explicitly protects preserved
+whitespace and failure of the trimmed lookup; a temporary public-API diagnostic
+confirmed it. The current `PromptIdentity` declaration and facade conversion
+have no version field.
+
+**Potential remediation:** State that the profile ID is the exact selected
+identity after any namespace qualification, and remove the nonexistent version
+pointer language. Keep literal identity behavior and collection metadata
+unchanged; no new normalization, version field, or compatibility guarantee is
+needed.
+
+## Existing Findings And Validation
+
+The earlier error-handling audit already records loss of owning public error
+categories for nested injected causes, loss of successful-body I/O causes, and
+pre-dispatch smoke failures incorrectly becoming concluded reports. Those are
+also interface-contract concerns, not new duplicate findings here. The hidden
+`ValidationPreparer` requirement behind the runner's `Validator` parameter is
+already recorded in the [complexity audit](03-complexity.md). No additional
+independent defect was established in the reviewed prepared-handle lifecycle,
+optional-value handling, inspection ownership, or observation/result mapping.
+
+Focused existing tests passed with the local Go 1.27.0 toolchain:
+
+```sh
+go test -race -count=1 -run 'Test(InspectProfile|ListIdentities|ScopedIdentityListings|PublicArtifactReaderAdapter|PublicLLMClientAdapter|ConfigurationInspection|LoadBuildsEffectiveTargetsAfterInheritance)' . ./internal/configuration
+```
+
+The separate temporary reader-reuse diagnostic intentionally produced a race
+report and exited unsuccessfully, substantiating finding 1 despite the green
+existing tests. Temporary programs and outputs remain outside the repository.
+All diagnostics were offline with synthetic configuration and credentials;
+no paid provider calls, new repository tests, or production changes were used.
+These checks are review evidence, not additional release gates.