From 31f5327255f4d8aa3bfe634ba1a2c7c9a6b27da4 Mon Sep 17 00:00:00 2001
From: Eric Rakestraw <eric@rakestraw.us>
Date: Sun, 1 Nov 2026 20:25:21 +0000
Subject: [PATCH] Record API and interface contract audit findings

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

diff --git a/docs/roadmap/interfaces.md b/docs/roadmap/interfaces.md
new file mode 100644
index 0000000..29eed4e
--- /dev/null
+++ b/docs/roadmap/interfaces.md
@@ -0,0 +1,234 @@
+# API and interface contract audit
+
+## Status and scope
+
+Audit completed against source commit `3f00ad2` on 2026-11-01. The findings below
+remain open for consideration. This audit changes documentation only.
+
+The review covered exported declarations and conversions in both public
+facades; candidate capture, validation, callbacks, and publication outcomes;
+module selection and dependency preparation; build results and CLI receipts;
+shared artifact and delivery contracts; Gitea observations and write evidence;
+and subprocess, blob-reader, and output ownership. GoDoc, current consumer and
+integration documentation, accepted architecture decisions, and existing tests
+provided the contract evidence.
+
+The review used the code graph for discovery and inspected implementation and
+tests directly, including the build package excluded by the graph index.
+
+## Current architecture
+
+- The root `releasekit` facade owns consumer request/result types, conversions,
+  backend selection, and public validation and preflight errors. It exposes
+  checking, publication, builds, preparation, and hosted delivery.
+- The separate `buildinfo` facade exposes runtime version access through a small
+  internal implementation. The builder and runtime implementation share the
+  version-injection contract.
+- Candidate and validation packages own captured-source checks and callback
+  sequencing. Publication adds authoritative Git endpoint guards, annotated
+  tag creation, exact-object pushes, and reconciliation.
+- Release identity resolves module selection and derives module paths, tags,
+  and note locations. Build and artifact packages own compilation, source
+  observations, output declarations, digests, and manifests.
+- Shared delivery owns page matching, asset inspection, additive uploads, and
+  recovery outcomes. It consumes `internal/forge.Backend`; the Gitea adapter
+  owns HTTP operations, opaque handles, and protocol failure conversion.
+- The CLI owns profiles, credential loading, local build receipts, CI
+  preparation, presentation, and exit statuses.
+
+These boundaries are consistent with the
+[architecture policy](../policy/architecture.md) and
+[ADR 0006](../adr/0006-neutral-forge-delivery-and-preflight.md). The findings
+concern behavior and descriptions within these boundaries; they do not require
+an architectural redesign.
+
+## Findings
+
+### 1. Build source inspection loses cancellation identity
+
+**Priority:** medium. **Status:** open.
+
+**Relevant code:** `cleanSource`, `gitCommit`, and `checkVersion` in
+[internal/build/build.go](../../internal/build/build.go), reached through public
+`Build` and managed delivery builds; the process error contract in
+[internal/process/process.go](../../internal/process/process.go).
+
+`cleanSource` wraps the error returned by `exec.CommandContext(...).Output()`
+without checking the context. Cancellation while `git status` runs therefore
+returns a killed-process error rather than an error matching
+`context.Canceled` or `context.DeadlineExceeded`. The neighboring `gitCommit`
+and `checkVersion` helpers translate interrupted commands to the context error,
+and the shared process runner does the same. Existing
+`TestBuildCancellationReturnsCompletedArtifacts` establishes identifiable
+cancellation during compilation, but does not exercise this source-inspection
+path.
+
+A temporary public-API probe blocked the first build-time `git status` with a
+local Git wrapper, then canceled the context. `Build` returned
+`inspect build source: signal: killed`; `errors.Is(err, context.Canceled)` was
+false. No Go compilation was needed. The same helper runs after compilation,
+so this discrepancy also affects callers receiving completed artifacts.
+
+**Consequences:** callers cannot reliably distinguish an interrupted build
+from a Git/source-inspection failure using the error chain. Managed delivery
+inherits that discrepancy when its builder reaches this path.
+
+**Potential remediation:** preserve the context error in `cleanSource`, using
+its neighboring helpers or the shared process boundary as the model. A focused
+regression check should interrupt source inspection and verify cancellation
+identity while retaining any partial build facts. This does not require a new
+error type or cancellation policy.
+
+### 2. A release lookup can produce an unusable asset handle without an error
+
+**Priority:** low. **Status:** open.
+
+**Relevant interfaces:** `forge.Backend.FindRelease` and `ListAssets` in
+[internal/forge/forge.go](../../internal/forge/forge.go); Gitea `FindRelease`,
+`ListAssets`, and `CreateRelease` in
+[internal/gitea/backend.go](../../internal/gitea/backend.go); `listAssets` in
+[internal/gitea/assets.go](../../internal/gitea/assets.go).
+
+Gitea `FindRelease` returns success and a non-nil opaque handle when the decoded
+release ID is missing, zero, or negative. Passing that handle to `ListAssets`
+then fails locally with `invalid_request` because the ID is not positive.
+The handle came directly from a successful backend operation; the missing
+identity originated in the service response. `CreateRelease` already treats
+an absent positive ID as a malformed acknowledgment.
+
+A local HTTP probe returned the correct tag and note body with no release ID.
+Page-only delivery completed. Binary delivery using the same page failed with
+`invalid_request: gitea list release assets: invalid_request: release ID must be
+positive`, without making an asset-list request.
+
+**Consequences:** binary delivery receives an unusable successful observation
+and attributes a service-response defect to the request. This weakens the
+failure distinction at the neutral backend boundary and gives callers
+misleading diagnostics.
+
+**Potential remediation:** classify the missing service identity when an asset
+operation needs it, distinguishing an incomplete backend-produced handle from
+an invalid handle supplied to the adapter. Alternatively, validate the ID at
+lookup if a deliberate decision makes it required for every successful lookup.
+The narrower approach preserves the existing minimal page-only lookup behavior:
+`TestBackendFindReleaseDoesNotRequireWriteResponseFields` explicitly accepts a
+response without an ID. This finding does not establish a new requirement for
+page URLs or write-response fields on page-only reads.
+
+### 3. Dependency preparation uses a different repository anchor
+
+**Priority:** low; contract clarification needed. **Status:** open.
+
+**Relevant API:** `DependencyPreparationRequest` and `PrepareBuildDependencies`
+in [preparation.go](../../preparation.go), compared with `Build`, `Check`, and
+`Deliver`, and `ResolveSelection` in
+[internal/releaseidentity/identity.go](../../internal/releaseidentity/identity.go).
+
+The preparation request calls `ModuleDir` repository-relative and says its
+empty value selects the repository root. Its implementation uses the supplied
+`RepositoryDir` directly as the selection anchor. Other orchestration requests
+accept a directory within a checkout and first resolve the containing repository
+root. The preparation request does not explain that its anchor differs or
+explicitly require an already resolved root.
+
+In a two-module local checkout, a public-API probe supplied the child directory
+as `RepositoryDir` and omitted `ModuleDir` for both operations. `Build` selected
+the root module. `PrepareBuildDependencies`, observed with a local Go wrapper,
+ran in the child module. A nonempty repository-relative selection would also
+be interpreted beneath that child directory.
+
+**Consequences:** consumers reusing one repository location across operations
+can prepare dependencies for a different module, or fail to locate the requested
+module. The CLI avoids this discrepancy by resolving the root before calling
+the preparation facade.
+
+**Potential remediation:** make the preparation anchor explicit in GoDoc and
+consumer guidance, or align checkout-root resolution with the other facades.
+Existing preparation tests also support supplied directories outside Git
+repositories; alignment should account for that behavior rather than silently
+introducing a Git-checkout prerequisite. Requiring a pre-resolved root is a
+viable documented alternative, with additional normalization work for API
+callers. The current evidence establishes the inconsistency, not a requirement
+to remove standalone directory preparation.
+
+### 4. Empty-prebuilt GoDoc omits the required source identity
+
+**Priority:** low. **Status:** open.
+
+**Relevant API:** `DeliveryRequest` in [delivery.go](../../delivery.go),
+`validateBuildIdentity` in
+[internal/delivery/delivery.go](../../internal/delivery/delivery.go), and the
+[Go API delivery guidance](../consumers/delivery.md#go-api).
+
+The request GoDoc says that an empty `Prebuilt` result performs page-only
+delivery. In practice, `Prebuilt: &BuildResult{}` fails before preflight with
+`build result does not identify the selected tagged module`. Identity checking
+is unconditional for non-nil prebuilt results; only the binary-source and
+artifact requirements depend on selected targets.
+
+The consumer guide more precisely says a *matching* empty prebuilt result.
+The temporary public-API probe confirmed the zero-result rejection.
+
+**Consequences:** a caller following the exported request documentation can
+reasonably supply a zero result as the advertised empty selection and receive
+an unexpected validation error.
+
+**Potential remediation:** clarify the GoDoc to say that a prebuilt result with
+no targets or artifacts still needs matching repository, module, tag, commit,
+and version identity. Recommend nil `Build` and nil `Prebuilt` for ordinary
+page-only delivery. This aligns documentation with the existing implementation
+and consumer guide without adding a zero-result compatibility path.
+
+### 5. Prebuilt delivery guidance promises hashing before every hosted request
+
+**Priority:** low. **Status:** open.
+
+**Relevant documentation:** the prebuilt paragraph in the
+[Go API delivery guidance](../consumers/delivery.md#go-api), compared with
+`Deliver` GoDoc in [delivery.go](../../delivery.go) and
+[ADR 0006](../adr/0006-neutral-forge-delivery-and-preflight.md).
+
+The guide says current prebuilt files are checked against recorded digests
+"before any hosted request." The implementation and accepted preflight decision
+place read-only credential verification before full local artifact hashing.
+Other sections of the same guide correctly describe that order.
+
+**Consequences:** readers receive conflicting descriptions of when delivery can
+contact the service, especially when a local prebuilt file is corrupt or
+unreadable. They cannot use the quoted statement to predict an entirely local
+failure before network access.
+
+**Potential remediation:** change the statement to "before any hosted write"
+and retain the documented preflight-first order. Existing
+`TestAutomaticPreflightFailurePrecedesPrebuiltArtifactHashing` covers that
+selected behavior. No production ordering change is recommended.
+
+## Validation and interpretation
+
+The following completed successfully with Go 1.26.10 on Linux/amd64 and
+`GOTOOLCHAIN=local GOPROXY=off GOSUMDB=off`:
+
+- `go test ./...`
+- `go run ./internal/apicheck`
+- `go test ./internal/process ./internal/gotool ./internal/gitea ./internal/delivery ./internal/releaseidentity -count=1`
+
+Temporary probes used public APIs, disposable local Git repositories, local
+command wrappers, synthetic credentials, and a local HTTP server. They confirmed
+the observations described above without modifying production code or adding
+repository tests. Passing existing tests does not cover the newly identified
+source-inspection cancellation case or resolve the documented ambiguities.
+
+No additional actionable contract defect was established in facade type
+ownership and conversions, callback sequencing, validation input selection,
+receipt identity binding, source publication reconciliation, runtime version
+lookup, or stream ownership. This is an implementation and local-fixture audit;
+it does not establish live-forge or live-CI behavior. Successful credential
+preflight continues to verify the available access evidence, and
+`SourceVerified` continues to describe source observations rather than an
+attestation.
+
+Address the cancellation error chain and asset-handle failure classification
+first, then align the preparation and delivery descriptions with the selected
+behavior. The findings do not call for new public extension interfaces,
+compatibility shims, stricter page-only response requirements, or a new release
+procedure.