Files
releasekit/docs/roadmap/interfaces.md

251 lines
13 KiB
Markdown
Raw Normal View History

# API and interface contract audit
## Status and scope
Audit completed against source commit `3f00ad2` on 2026-11-01. The findings below
are resolved as recorded in their completion markers. The audit itself changed
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:** complete.
**Completion evidence:** `28a4144`: synchronized cancellation, deadline,
partial-artifact, and race checks pass.
**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:** complete.
**Completion evidence:** `9fa4407`: backend and public page-only/prebuilt
regressions pass with malformed-handle classification.
**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:** complete.
**Completion evidence:** `b47202a`, `5aba4c6`, `abbab3c`: checkout discovery,
standalone preparation, and build-composition regressions pass.
**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:** complete.
**Completion evidence:** `d368bb3`: GoDoc and consumer guidance describe
matching empty results; page-only regressions pass.
**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:** complete.
**Completion evidence:** `d368bb3`: consumer guidance matches preflight-first
order; the existing preflight-before-hashing regression passes.
**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.
The remediation addresses the cancellation error chain and asset-handle failure
classification and aligns preparation and delivery descriptions with the selected
behavior. The findings did not call for new public extension interfaces,
compatibility shims, stricter page-only response requirements, or a new release
procedure.