diff --git a/docs/releases/v0.6.0.md b/docs/releases/v0.6.0.md index 7ae122e..f2a0425 100644 --- a/docs/releases/v0.6.0.md +++ b/docs/releases/v0.6.0.md @@ -2,11 +2,23 @@ ## Summary -ReleaseKit adds neutral delivery coordination behind an explicit Gitea backend -selection and introduces authenticated, read-only credential preflight. +ReleaseKit adds file-based delivery credentials, optional checks for forbidden +committed files, neutral delivery coordination with explicit Gitea selection, and +authenticated, read-only credential preflight. It also corrects dependency +preparation, publication recovery, and build cancellation handling. ## Changes +- Delivery profiles can select a plain token file through `token_file`. + Relative paths resolve from the checkout root; a nonblank token environment + value takes precedence. The CLI trims file contents and reports unreadable + or blank files as credential-loading failures. Library callers continue to + supply the token explicitly through the destination. +- `ValidationSpec.ForbiddenFiles` and `validation.forbidden_files` select + literal repository-relative paths that must be absent from the committed + candidate. This optional check can protect a local token-file path. It + inspects committed path metadata independently of module/shared-file inputs; + it does not inspect file contents or history. - `releasekit.DeliveryRequest.Destination` now uses the facade-owned `releasekit.DeliveryDestination`. Set `Forge: releasekit.ForgeGitea` for Gitea delivery. The removed `GiteaDestination` type has no compatibility @@ -36,8 +48,50 @@ selection and introduces authenticated, read-only credential preflight. full local artifact size and digest verification; those checks still finish before hosted writes. +## Correctness and efficiency + +- Dependency preparation now resolves the containing checkout root, including + when called from a subdirectory or filesystem alias. `ModuleDir` is relative + to that root, and empty selection prepares the root module. Standalone use + retains the supplied directory as its base, including Go-only use when Git + is unavailable. Damaged checkout metadata and operational discovery failures + remain errors before dependency downloads. +- Preparation overrides alternate module-file selection from inherited or + persistent `GOFLAGS`, while retaining other effective Go settings. Downloads + therefore use the selected module's ordinary metadata. +- Candidate checking rejects a derived release tag that Git cannot create + before running validation. CI delivery also validates the selected module's + complete tag before dependency preparation. +- Publication recovery treats a peeled-only tag observation as unresolved, + retains the original push failure, and reports `PublicationUnknown` with CLI + status 3. Complete tag observations retain their existing recovery behavior. +- Cancellation and deadline expiration during build source inspection remain + identifiable through `errors.Is`; partial results retain completed artifacts. + Local artifact inspection errors also retain their underlying causes through + delivery. +- Gitea asset operations classify a missing or nonpositive service release ID + as a malformed observation before making an asset request. Page-only release + lookup retains its existing minimal response requirements. +- Asset delivery uses linear pending-asset bookkeeping and sorts only missing + assets once, reducing repeated work during uploads and reconciliation. + +## Compatibility + +This pre-1.0 minor release changes the public destination type and requires +explicit forge selection for delivery. Gitea is the supported backend. +Preparation callers that previously used a checkout subdirectory as their +implicit module selection must now select that module with `ModuleDir`. +Ambient `GOFLAGS` no longer redirects preparation to an alternate module file. + +The module identifier remains `go.maximumdirect.net/releasekit`, and the Go +consumer minimum remains 1.26.0. Receipt schemas and the separate source +publication and hosted-delivery outcomes retain their existing definitions. + ## Upgrade guidance +Select v0.6.0 for library and CLI dependencies, then update integrations as +follows: + 1. Replace `releasekit.GiteaDestination` with `releasekit.DeliveryDestination` and set `Forge: releasekit.ForgeGitea` in each API request. @@ -52,6 +106,15 @@ selection and introduces authenticated, read-only credential preflight. not a guarantee that a later write will be accepted. Source publication continues to use Git authentication and does not require hosted delivery credentials. +5. For dependency preparation of a nested module, supply its repository-relative + path through `DependencyPreparationRequest.ModuleDir`, even when + `RepositoryDir` already points inside that module. Root-module consumers can + keep `ModuleDir` empty. +6. To adopt file-based credentials, store the token in an ignored local file + and configure `delivery.token_file`. Optionally add that same + repository-relative path to `validation.forbidden_files`. Credential loading + and candidate file validation are independent selections; configuring the + file alone does not enable the validator. See the [delivery guide](../consumers/delivery.md), [configuration reference](../config.md), [CLI contract](../cli.md), and [Gitea integration guide](../integrations/gitea.md) diff --git a/docs/roadmap/correctness.md b/docs/roadmap/correctness.md deleted file mode 100644 index 4b1dccb..0000000 --- a/docs/roadmap/correctness.md +++ /dev/null @@ -1,225 +0,0 @@ -# Functional correctness audit - -## Status and scope - -Audit completed against source commit `31f5327` on 2026-11-01. The three findings -below are resolved as recorded in their completion markers. The audit itself -changed documentation only. - -The review covered candidate capture and rechecks, module/tag derivation, -committed-file validation and reference traversal, builds and runtime version -injection, dependency and checkout preparation, CLI adaptation and receipts, -source publication guards and recovery, and hosted page/asset delivery. It -examined ordinary paths, empty selections, error and cancellation paths, and -boundary interactions against the current code, GoDoc, consumer guides, -accepted decisions, and existing tests. - -The earlier [interface audit](interfaces.md) also recorded a build-source -cancellation defect; this document records -additional functional findings rather than duplicating that issue or its -contract-clarity findings. No production code or repository tests were changed. - -## Current architecture - -The root `releasekit` facade owns consumer types and coordinates checking, -building, publication, preparation, and delivery through internal packages. -The separate `buildinfo` facade exposes the runtime version accessor through a -small independent implementation. - -Candidate capture, shared validation, release identity, builds, artifacts, and -Git operations have separate owners. Publication coordinates exact annotated -tag publication and recovery. Shared delivery coordinates page and asset -matching, writes, and reconciliation through the neutral `internal/forge` -contract; `internal/gitea` implements its HTTP operations. The CLI owns -profiles, token loading, receipts, CI sequencing, output, and exit statuses. - -This structure remains consistent with the -[architecture policy](../policy/architecture.md). The findings require focused -behavioral corrections within those boundaries. - -## Findings - -### 1. Candidate checking can approve a tag that Git cannot create - -**Priority:** medium. **Status:** complete. - -**Completion evidence:** `b91a0d1`: public invalid-derived-tag regressions and -Git/candidate package tests pass. - -**Relevant code:** `Derive` in -[internal/releaseidentity/identity.go](../../internal/releaseidentity/identity.go), -and `Capture`, `Recheck`, and `hasLocalTag` in -[internal/candidate/check.go](../../internal/candidate/check.go). Compare the -existing Git ref-validation boundary in -[internal/git/remote.go](../../internal/git/remote.go). - -**Circumstances:** a selected nested module has a physical directory name that -produces an invalid Git ref component. For example, `module space` and -`module.lock` are usable filesystem directories but cannot occur as components -of the derived release tag. - -`Derive` constructs the complete tag from the physical module path and version. -Candidate capture and rechecking use `git show-ref --verify --quiet` to test -whether that tag exists. Git exits with status 1 for these invalid names as well -as for valid absent refs. `hasLocalTag` interprets every such exit as absence, -so neither candidate guard establishes that the derived name is a usable tag. - -**Expected behavior:** candidate checking should report an invalid derived tag -as a local candidate failure. An invalid ref cannot satisfy the selected Git -release-tag convention, independently of remote availability. - -**Actual behavior and evidence:** a temporary public-API probe created clean -committed modules with valid declarations, Go source, and module-local notes. -With version `v1.2.3`, public `Check` returned nil errors for both -`module space/v1.2.3` and `module.lock/v1.2.3`. Independent -`git check-ref-format refs/tags/` calls rejected both. The same fixture's -ordinary `normal/v1.2.3` selection passed both checks. - -**Consequences:** callers receive successful local candidate evidence for a -release identity that cannot be published. `Publish` subsequently rejects it -through its Git ref checks, so this finding is a false-positive check result, -not evidence of an invalid tag reaching the remote. - -**Potential remediation:** validate the complete derived ref at the candidate -capture boundary before treating it as an absent local tag, reusing Git's -existing ref-validation rules. Keep existence and name-validity checks distinct. -Cover the public `Check` result with invalid derived names and an ordinary -nested-module control. This does not require inventing filesystem directory -restrictions or changing standalone local-build behavior. - -### 2. Ambient Go flags can redirect dependency preparation to another module file - -**Priority:** medium. **Status:** complete. - -**Completion evidence:** `05f1b47`, `abbab3c`: local-proxy inherited and -persistent flag regressions verify selected graphs and build identity. - -**Relevant code:** public `PrepareBuildDependencies` in -[preparation.go](../../preparation.go), its preparation bridge, and -`PrepareBuildDependencies` in -[internal/gotool/gotool.go](../../internal/gotool/gotool.go). Binary CI delivery -calls this path from [cmd/releasekit/main.go](../../cmd/releasekit/main.go). - -**Circumstances:** effective `GOFLAGS` contains `-modfile` pointing to alternate -module metadata. The value can come from the process environment or Go's -persistent environment file. This finding applies even when the caller supplies -the correct repository root and selected module, independently of the anchor -ambiguity recorded in the interface audit. - -The preparation command owns `GOWORK=off` and `GOTOOLCHAIN=local` but leaves -module-selection flags inherited. Go therefore runs in the requested directory -while downloading dependencies from the alternate module file. Later validation -and artifact compilation use ReleaseKit's controlled Go flags and the selected -module's actual metadata. - -**Expected behavior:** successful preparation should prepare the dependencies -of the selected module described by the request and consumed by the subsequent -ordinary build. Caller proxy, private-module, authentication, checksum, and -cache settings should remain available as documented. - -**Actual behavior and evidence:** a temporary public-API probe selected a module -requiring `example.invalid/needed v1.0.0`, supplied an alternate dependency-free -module file through `GOFLAGS=-modfile=...`, and disabled module networks. -`PrepareBuildDependencies` returned nil. Removing that flag and preparing the -same selection failed with `module lookup disabled by GOPROXY=off`, confirming -that the requested graph had not been prepared. The probe used a disposable -`GOMODCACHE` for every synthetic-module download attempt. - -An additional local Go probe confirmed that setting `GOFLAGS` to an empty string -does not suppress a value in a persistent Go environment file on Go 1.26.10. -A fix that only clears the process value would leave that case unresolved. - -**Consequences:** preparation can report success while the selected binary build -still lacks dependencies. It can also use or update the alternate metadata's -checksum file rather than the selected module's `go.sum`, contrary to the -operator's selected preparation scope. - -**Potential remediation:** give dependency preparation a controlled, -nonredirecting Go-flag baseline while preserving its network-capable environment -and effective operational settings. For example, a local Go 1.26.10 probe -confirmed that an explicit nonempty `GOFLAGS=-mod=mod` overrides a persistent -`-modfile` value and is accepted by `go mod download`; the exact baseline should -be assessed against the supported preparation behavior. Verify both inherited -and persistent flag sources, actual selected-module preparation, and retention -of the existing proxy/private-module/cache settings. Avoid globally disabling -persistent Go configuration as a shortcut, since that also discards supported -caller settings. - -### 3. Incomplete tag reconciliation is reported as a known publication conflict - -**Priority:** medium. **Status:** complete. - -**Completion evidence:** `0632aa5`: both Git query paths and public/CLI -lost-acknowledgment recovery regressions pass. - -**Relevant code:** `mergeRef` in -[internal/git/remote.go](../../internal/git/remote.go), `Endpoint.QueryTag` in -[internal/git/tag.go](../../internal/git/tag.go), and `reconcilePush` in -[internal/publish/publish.go](../../internal/publish/publish.go). - -**Circumstances:** after a failed push acknowledgment, the authoritative tag -query succeeds as a command but supplies only the peeled -`refs/tags/^{}` record, with no exact `refs/tags/` object record. This -is an incomplete query result, not the normal output of a healthy Git remote. -Malformed and inconsistent reconciliation results already fall within the -implemented recovery scope. - -`mergeRef` marks the peeled observation `RefPresent` and stores `Peeled`, while -leaving `Object` empty. `QueryTag` returns that state without final completeness -validation. `reconcilePush` does not recognize it as unreadable; it falls through -to the different-object case and reports `PublicationConflict` with a blank -observed object ID. - -**Expected behavior:** the result should remain `PublicationUnknown`, preserving -the push failure and explaining that the query did not establish the exact tag -object. A peeled commit alone cannot establish equality or inequality with the -intended annotated tag object. - -**Actual behavior and evidence:** a temporary public-API probe published to a -local bare repository through a Git wrapper. The wrapper performed the real -push, reported a simulated lost acknowledgment, and returned only the peeled -record during recovery. `Publish` returned `state=conflict` and -`phase=reconciliation`, with a diagnostic claiming that the remote identifies -an empty object instead of the intended tag object. An independent normal Git -query confirmed that the remote actually held the intended exact tag object. - -**Consequences:** an unresolved observation is presented as a known remote -conflict. The CLI returns its known-failure status rather than the status for -an uncertain publication, and callers receive misleading recovery evidence -even though the push may have succeeded. - -**Potential remediation:** finalize ref observations after parsing the complete -response and classify a peeled-only result as inconsistent or unreadable. -Preserve valid responses where a peeled record precedes its exact object record. -Keep recovery unknown whenever the exact object cannot be determined. A focused -recovery regression should retain the original push cause, verify the public -unknown state, and preserve ordinary exact-match, absence, and real-conflict -outcomes. - -## Validation and limits - -The ordinary and race-enabled offline suites and compiled API/dependency-boundary -check passed with Go 1.26.10 on Linux/amd64: - -```sh -GOTOOLCHAIN=local GOPROXY=off GOSUMDB=off go test ./... -GOTOOLCHAIN=local GOPROXY=off GOSUMDB=off go test -race ./... -GOTOOLCHAIN=local GOPROXY=off GOSUMDB=off go run ./internal/apicheck -``` - -The probes used temporary programs outside the repository, disposable local -Git repositories, command wrappers, and offline Go invocations. They exercised -public APIs and verified the relevant external facts independently: Git ref -validity, dependency preparation against actual module metadata, and the exact -object present on a local remote. No live forge, CI runner, or real credentials -were involved. - -The audit also reviewed committed-file input boundaries, local-reference and -symlink traversal, validation ordering, build result/receipt binding, version -injection, and additive delivery with read-only preflight and write recovery. -No additional functional finding was substantiated in those paths beyond the -issues already recorded in [the interface audit](interfaces.md). The tests -available at audit time did not cover the three demonstrated cases above. - -The agreed remediation corrected these behaviors without new public interfaces -or a broader release model. diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md deleted file mode 100644 index ef413bc..0000000 --- a/docs/roadmap/implementation.md +++ /dev/null @@ -1,470 +0,0 @@ -# Audit remediation implementation plan - -## Baseline and execution - -This plan targeted `59bb074` on 2026-11-01. The worktree was clean when planning -began. At that baseline, production code contained the findings recorded in -[correctness.md](correctness.md) and [interfaces.md](interfaces.md). -[remediation.md](remediation.md) defines the agreed behavior and scope; this -plan defines how to reach it. - -Luna should execute Stage 1 through Stage 10 in order in one end-to-end sprint. -The stage boundaries divide implementation prompts. Intermediate stages may -leave dependent code or tests incomplete; compilation at every boundary is not -a requirement. Record deferred checks and carry them forward. Stage 10 requires -the complete, coherent result. There are no intermediate release, live-service, -operator-action, or human-review gates. - -Read [development.md](../development.md) and the applicable policies before -work. Use the repository's code graph for code discovery, with targeted source -reads where the graph is incomplete; the build package may be excluded from -its index. Follow the standing commit policy for completed reviewable units. -Update stage status and evidence here as work progresses; a deferred regression -check remains outstanding until it has run successfully. - -## Implementation decisions - -- Keep both public facades, public signatures, receipt schema, and package - dependency direction. New helpers belong under the existing internal owners. -- Resolve a checkout root automatically for preparation; retain standalone - supplied-directory preparation. Git discovery supplies the root, while the - existing selection resolver supplies relative-path, symlink, and containment - behavior. -- Override only alternate module-file selection with the explicit argument - `-modfile=` on dependency downloads. Preserve other effective Go settings. - Local Go 1.26.10 probes verified precedence over inherited and persistent - `GOFLAGS` without preventing normal upward module discovery. -- Represent a peeled-only remote ref as `RefConflict` after parsing the whole - response. Existing publication reconciliation already maps that inconsistent - observation to `PublicationUnknown`. -- Classify a Gitea handle of the expected private type with a nonpositive ID as - `forge.FailureMalformed` at asset use. Preserve minimal page-only lookup fields - and wrong-type handle classification. -- Keep empty-prebuilt identity checking and the existing preflight-first order. - Their remediation is documentation, rather than a new request interpretation. - -These decisions cover the reported cases. Preserve existing recovery bounds, -write evidence, source observations, and preflight evidence limits. No new -public options, directory-name policy, response-field requirements, or build -reproducibility guarantees are part of this work. - -## Coverage map - -| Finding | Implementation stages | -| --- | --- | -| Correctness 3: incomplete remote tag reconciliation | 1 | -| Interfaces 3: preparation path anchoring | 2, 3, 5 | -| Correctness 2: ambient module-file redirection | 4, 5 | -| Correctness 1: invalid derived candidate tag | 6 | -| Interfaces 1: source-inspection cancellation | 7 | -| Interfaces 2: incomplete Gitea release handle | 8 | -| Interfaces 4: empty-prebuilt documentation | 9 | -| Interfaces 5: documented hashing order | 9 | -| Complete integration and evidence | 10 | - -## Stage 1: Finalize remote ref observations and verify recovery - -**Status:** complete. The Git query, public publication, and CLI publication -regressions pass with the offline environment; the root `TestPublish` tests and -the full `internal/git` package also pass. - -**Files:** `internal/git/remote.go`, `internal/git/tag.go`, their tests, -`publish_test.go`, and a focused CLI publication test under `cmd/releasekit/`. - -1. Add a small ref-finalization helper beside `mergeRef`. After parsing a whole - response, a `RefPresent` value with an empty `Object` and a populated `Peeled` - becomes `RefConflict`. Retain its available fields for diagnostics. -2. Apply finalization to the tag observation returned by both `Endpoint.Query` - and `Endpoint.QueryTag`. Do not finalize after each record: a peeled record - followed by its exact object is valid. Preserve existing duplicate-record and - malformed-line handling. -3. Keep `internal/publish.reconcilePush`'s existing treatment of `RefConflict`: - unknown outcome, reconciliation phase, and the original push error retained. - Its existing path should suffice once the observation is classified correctly; - improve a diagnostic only if necessary to explain incomplete evidence. -4. Extend the existing local Git wrapper/fixture in `publish_test.go`, starting - with `TestPublishUninterpretableReconciliationRemainsUnknown` and - `installGitWrapperWithMutationMode`. Add a mode that performs the real push, - returns a failed acknowledgment, and supplies only the selected peeled tag - record on the recovery query. Keep endpoint URL inspection functional and - alter only the recovery response, not the pre-publication guards. -5. Verify the public result is `PublicationUnknown` in - `PublicationReconciliation`, retains `TagObject` and the push cause, and - leaves the local tag intact. Independently inspect the local bare remote to - prove the intended exact object was actually published. -6. Exercise the CLI's `run` publication path with the same failure shape in a - package-local fixture: status 3, unknown-state diagnostic, and no success - message. Retain the current mapping; do not add a new exit category or shared - test framework solely to share fixtures across packages. - -**Verification:** exercise both query entry points and the following response -shapes: empty response, exact object only, exact object plus peeled commit in -both orders, peeled record only, and the already covered duplicate/conflicting -records. Preserve successful recovery, confirmed absence, actual different-object -conflict, and the existing single recovery query. - -**Focused checks:** `go test ./internal/git`; the `TestPublish` tests at the root -and CLI publication tests. Use the offline environment specified in Stage 10. - -## Stage 2: Add optional checkout-root discovery with precise failure handling - -**Status:** complete. `go test ./internal/git` passes offline, including real -checkout, worktree, alias, standalone, missing-Git, diagnostic, and failure -cases. The public Go-only preparation composition check passed in Stage 3. - -**Files:** `internal/git/source.go` or a focused companion file, Git tests, and -`internal/process/process.go` only if an existing invocation capability proves -insufficient. Its current stdout/stderr capture and environment overlay should -already suffice. - -1. Add an internal Git function such as - `DiscoverRepositoryRoot(ctx, directory) (root string, found bool, err error)`. - Success returns a canonical containing checkout root and `found=true`. - Confirmed standalone use returns `found=false` and nil error. Operational - failures return their wrapped cause. -2. Use Git's own `rev-parse --show-toplevel` discovery, preserving normal Git - environment and worktree behavior. Capture stdout and stderr separately - through `internal/process`. Apply `LC_ALL=C` to this child only so the known - no-repository diagnostics can be recognized without depending on operator - locale. Do not change process-global locale or configuration. -3. Git uses exit status 128 for both absence and other failures. Consider absence - only when that status accompanies its recognized no-parent-repository - diagnostic: `fatal: not a git repository (or any of the parent directories): .git`, - or the form beginning `fatal: not a git repository (or any parent up to mount point ` - and ending `)`, followed by - `Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).`. - Match the fixed text around the variable mount path. Keep the - classifier small, private, and covered by diagnostic samples. Do not accept - every message starting with `not a git repository` as standalone: an invalid - explicit `GIT_DIR` has a different diagnostic. - Confirm the absence of `.git` entries in the directories Git could search - before accepting standalone use: damaged directory metadata can produce the - same recognized diagnostic. Respect configured discovery ceilings and any - reported filesystem boundary. A relevant marker preserves the Git failure; - filesystem inspection errors also remain failures. -4. Treat cancellation, inaccessible working directories, broken Git metadata, - ownership errors, bare-repository worktree errors, and unrecognized - diagnostics as errors. Preserve `errors.Is`/`errors.As` causes and useful - stderr. An arbitrary error is never permission to choose another base. -5. Reuse the existing successful-root normalization with `filepath.EvalSymlinks`. - Keep strict `RepositoryRoot`'s current public-to-internal behavior: its - callers still require a checkout. Share private mechanics where useful while - retaining the original error cause for that strict operation. -6. Preserve the existing Go-only standalone use when the Git executable is - unavailable. Only for `exec.ErrNotFound`, check the supplied physical - directory and its ancestors for `.git` entries with standard filesystem - operations. With no such entry and no explicit `GIT_DIR` or `GIT_WORK_TREE`, - return confirmed standalone use. If a marker or explicit Git context exists, - return the missing-Git error; checkout discovery is unavailable. Propagate - filesystem inspection failures instead of treating them as absence. This - limited presence check establishes standalone use only; Git remains the - authority for selecting an actual checkout root. Do not apply this fallback - to the strict `RepositoryRoot` function. - -**Verification:** real temporary repositories for root/subdirectory discovery, -filesystem aliases, and an independent worktree with a `.git` file; real -standalone directories for absence. Use controlled Git-process failures for -permission/ownership errors and cancellation, rather than relying on host user -privileges. Test both no-repository diagnostic forms and a status-128 failure -that must remain an error. A broken `.git` file or a `.git` directory missing -`HEAD` must not trigger standalone fallback. Cover discovery boundaries so an -excluded ancestor's marker does not change standalone selection. Existing -strict-root tests continue to pass. -Also cover a Go-only executable path in a marker-free standalone directory: -preparation can still run there. With that same executable path inside a -checkout, discovery fails with the missing-Git cause before Go runs. - -**Focused check:** `go test ./internal/git`. - -## Stage 3: Apply checkout-root selection to dependency preparation - -**Status:** complete. Public preparation and CLI `TestCIDeliver` checks pass -offline, including checkout-relative, symlink, standalone, and missing-Git -selection. **Depends on:** Stage 2. - -**Files:** `preparation.go`, `preparation_test.go`, and -`docs/consumers/delivery.md`'s preparation guidance. - -1. Retain the existing rejection of an empty/blank `RepositoryDir`. -2. Discover the optional checkout root from the supplied directory. If found, - pass that root and the request's unchanged `ModuleDir` into - `releaseidentity.ResolveSelection`. Otherwise pass the supplied directory - and `ModuleDir` into that same resolver. Propagate discovery failures before - invoking Go. -3. Continue calling `internal/preparation.PrepareBuildDependencies` with the - selected absolute directory. Preserve the current selection resolver's - handling of symlinks before parent traversal and its containment checks. -4. Leave CLI root normalization and `runCIDelivery`'s preflight/preparation order - in place. They already supply the containing root; no new CLI flag or profile - field is needed. -5. Update request/function GoDoc and the preparation guide: checkout-relative - selection inside Git, supplied-base selection for standalone use, and the - changed interpretation of calls from checkout subdirectories. - -**Verification:** extend `TestPrepareBuildDependenciesUsesSelectedModuleDirectory` -and related public tests. Include checkout root and child-directory inputs with -empty and explicit nested `ModuleDir`, standalone selection, symlink/`link/..` -selection, and discovery failure before any Go invocation. Use the existing Go -recording shim to observe the chosen working directory. Keep the current -standalone tests, which intentionally have no Git repository; do not introduce -an immediate-go.mod requirement just to simplify fixtures. Stage 5 verifies -composition with ordinary build identity and ambient flags. - -**Focused checks:** public `TestPrepareBuildDependencies` tests and existing CLI -`ci-deliver` preparation tests. - -## Stage 4: Override alternate module-file selection during downloads - -**Status:** complete. `go test ./internal/gotool` passes offline. Actual module -graph selection evidence passed in Stage 5. - -**Files:** `internal/gotool/gotool.go`, `internal/gotool/gotool_test.go`, -`preparation.go` GoDoc, and `docs/internal/gotool.md`. - -1. Change the download argument vector to - `go mod download -modfile=`. The explicit empty flag overrides the alternate - module file selected through `GOFLAGS` and restores ordinary discovery. -2. Keep the existing `GOWORK=off` and `GOTOOLCHAIN=local` overlays. Leave other - effective flags, proxy/private-module/checksum/authentication/cache settings, - and persistent Go configuration available to this network-capable operation. -3. Preserve combined diagnostics, wrapped command/cancellation errors, and - permission for the Go command to update its normal `go.sum`. -4. Update `TestPrepareBuildDependenciesPreservesPreparationEnvironment`'s - recording fixture for the intentional argument change and retain its - environment assertions. Keep command failure and cancellation tests. -5. Document only this preparation-specific flag override. Build/validation - environments and caller-controlled bootstrap commands keep their current - responsibilities. Actual graph-selection regression evidence follows in - Stage 5; argument recording alone is insufficient. - -**Focused check:** `go test ./internal/gotool`. - -## Stage 5: Verify selected-module preparation and build composition - -**Status:** complete. Local-proxy graph, negative-control, inherited and -persistent settings, standalone discovery, and public build identity checks -pass offline, as do the focused binary CI and credential-order checks. -**Depends on:** Stages 3 and 4. - -**Files:** `preparation_test.go` and small test-local helpers beside it. The -existing local module-proxy construction in `cmd/releasekit/woodpecker_test.go` -can inform fixture layout, but its package-local helpers cannot be imported. - -1. Create a real local repository with root and nested Go modules using distinct - synthetic dependency graphs, plus an alternate dependency-free module file. - Serve the synthetic dependency metadata and archives through a temporary - file proxy or `httptest` server. Use the standard library and a small fixture, - not a new general-purpose testing package. -2. For each preparation attempt, supply a fresh test-owned `GOMODCACHE` and - fixture-scoped proxy/checksum/private-module settings. Ensure that a synthetic - `example.invalid` module cannot escape to a public proxy or VCS lookup. -3. Cover `GOFLAGS=-modfile=...` from the inherited environment and from a - temporary `GOENV` file with no process override. Do not treat `GOFLAGS=""` - as sufficient isolation from persistent settings. The audit demonstrated - that Go can still use the persistent value. -4. Call public `PrepareBuildDependencies` from checkout subdirectories for both - default root selection and explicit nested selection. Prove that the chosen - module's dependency was downloaded into its owned cache and that the - alternate graph did not satisfy the request. Retain a local negative control - showing the ordinary uncorrected Go command would select the alternate file. -5. Call public `Build` with the same `RepositoryDir` and `ModuleDir`, using an - empty target list, and compare its repository/module identity with the graph - prepared above. This checks path composition without adding compiler or - buildinfo dependencies to the synthetic proxy. Existing binary build and CI - tests remain the owners of actual compilation/version-injection behavior. -6. Add a standalone package-subdirectory case proving Go still discovers its - parent declaration, and retain an unrelated effective flag such as `-x` in - a representative case to prove the fix did not replace all caller flags. - Keep changed checksum files within the disposable fixture. - -**Verification:** observed dependency metadata/cache files and public build -identity are the assertions. Avoid only checking command counts or argument -spelling. Inspect fixture subprocess settings to ensure every synthetic module -consumption uses an owned cache. - -**Focused checks:** the new root preparation tests and existing CLI binary -`ci-deliver` tests. Preserve their credential-before-download sequencing. - -## Stage 6: Reject invalid derived candidate tags before validation - -**Status:** complete. Git and candidate package tests and public invalid-tag -and publication regressions pass offline; malformed derived tags stop before -Go validation and callbacks while retaining candidate identity. - -**Files:** `internal/git/remote.go` and its current tag-helper callers/tests, -`internal/candidate/check.go`, public candidate tests, and candidate guidance -only where its explanation needs adjustment. - -1. Make the existing internal Git `checkTagRef` helper available to the candidate - package as `git.CheckTagRef`, updating its internal callers and GoDoc. Retain - the existing `git check-ref-format refs/tags/` behavior. This is an - internal-package export, not a new facade API. -2. In `candidate.Capture`, validate the fully derived tag before accepting its - absence through `hasLocalTag`. Return a contextual local-candidate error - while retaining the captured module/tag identity. A malformed derived ref is - not an `ErrInvalidVersion` syntax failure and needs no new public error type. -3. Keep name validity separate from `show-ref`'s existence result. The captured - tag is then immutable across ordinary rechecks; reuse the shared validator - if another entry path needs it, without adding a separate character parser. -4. Keep working-tree identity derivation and standalone `Build` behavior intact. - Do not reject module directories globally or invent alternate tag encodings. - -**Verification:** public `Check` rejects the reported `module space` and -`module.lock` cases before Go validation and additional/build callbacks. Keep -ordinary root/nested/major-version and slash-containing tags accepted when -otherwise valid. A Go marker shim can prove validation did not start. Existing -publication guard and local-tag conflict tests continue to cover those paths. - -**Focused checks:** `go test ./internal/git ./internal/candidate` and the public -candidate regression tests. - -## Stage 7: Preserve build source-inspection cancellation causes - -**Status:** complete. Synchronized initial, final, and deadline interruption -checks, ordinary Git failure coverage, and existing build and managed-delivery -cancellation tests pass; the cancellation paths also pass with `-race`. - -**Files:** `internal/build/build.go` and focused public tests in `build_test.go` -or a companion root test file. - -1. In `cleanSource`, when the Git command fails, inspect `ctx.Err()` as neighboring - `gitCommit` and `checkVersion` already do. Wrap the context error when present; - otherwise retain the Git command failure. Keep the existing diagnostic - context and partial result behavior. No new public error type is needed. -2. Reuse `newBuildConsumer` and the existing cancellation fixture style. Add a - Git wrapper that delegates ordinary operations and signals when the selected - build-time `status` invocation is entered before blocking. Use an explicit - readiness signal to cancel; do not rely on a guessed startup sleep. -3. Cover cancellation in the initial inspection, which needs no compilation, - and in the final inspection after a host-target binary has been built and - smoke-checked. Verify `errors.Is(err, context.Canceled)` and retention of - completed artifact paths/digests for the latter case. -4. Add one focused deadline-expiration variant with synchronized readiness and - a bounded wait. Trigger a test-controlled expiration context after Git signals - readiness, so repository/module setup cannot consume the inspection deadline. - Verify `context.DeadlineExceeded`, rather than asserting an exact killed-process - error string. Join test goroutines and reap children. Check the deadline case - with delayed unrelated setup as well as normal and race-enabled execution. - -**Verification:** ordinary source-inspection Git errors retain their causes; -`SourceVerified` is not reported true after an interrupted build. Retain -`TestBuildCancellationReturnsCompletedArtifacts` and managed-delivery temporary -cleanup coverage. Add another delivery test only if a distinct propagation or -cleanup risk remains uncovered. - -**Focused checks:** root build/cancellation tests and affected managed-delivery -cancellation tests; run these with the race detector after they are coherent. - -## Stage 8: Classify missing Gitea release IDs at asset use - -**Status:** complete. Offline Gitea and delivery package checks and public -page-only and prebuilt regressions pass, including missing, nonpositive, valid, -and wrong-type handles and not-attempted write evidence. - -**Files:** `internal/gitea/backend.go`, its tests, a focused root delivery test, -and `docs/integrations/gitea.md`. - -1. Split handle validation in `ListAssets` and `UploadAsset`: a failed private - `releaseHandle` type assertion remains `forge.FailureInvalid`; a correctly - typed handle with `id <= 0` becomes `forge.FailureMalformed` with a useful - missing-release-identity diagnostic. A small private validation helper is - sufficient if it avoids duplicating this distinction. -2. Perform these checks before making an asset HTTP request. For a locally - rejected upload, return `Attempted=false` and `WriteNotAttempted` exactly as - other local upload failures do. Do not pass a bad ID into the lower client - and reclassify its invalid-request error afterward. -3. Keep `FindRelease`'s minimal read response and `CreateRelease`'s existing - positive-ID acknowledgment check. No extra lookup, retry, public failure - type, handle-provenance framework, or mandatory page URL is needed. -4. Drive lookup responses with missing, zero, and negative IDs through the real - backend into both asset operations. Assert malformed neutral failures and - absence of asset requests. Test wrong-type handles and valid IDs as controls. -5. Retain `TestBackendFindReleaseDoesNotRequireWriteResponseFields`. Add a paired - public delivery regression using the existing root fixture: the same matching - existing page without an ID completes page-only delivery, while a request - with a prebuilt artifact fails asset use with `DeliveryNotAttempted` and no - new hosted writes. Preserve existing page creation progress and write-recovery - behavior. - -**Focused checks:** `go test ./internal/gitea ./internal/delivery` and affected -root delivery tests. Describe the corrected observation classification in the -integration guide without strengthening page-only read requirements. - -## Stage 9: Align delivery documentation with existing behavior - -**Status:** complete. The preflight-before-hashing and page-only regressions -pass offline. GoDoc and consumer guidance now describe matching empty results, -preparation selection, and verification before hosted writes. - -**Files:** `delivery.go` GoDoc and `docs/consumers/delivery.md`; review related -current descriptions for consistency. This stage changes documentation only. - -1. Change `DeliveryRequest` GoDoc to distinguish a matching prebuilt result with - no targets/artifacts from the zero `BuildResult`. A supplied result still - needs matching repository, module directory/path, tag, commit, and version. - Binary-only source/artifact requirements remain conditional on binary work. -2. Present nil `Build` and nil `Prebuilt` as the ordinary page-only API use. - Preserve matching-empty-result behavior, current identity validation, and - mutual exclusion of managed and prebuilt sources. Add no compatibility shim. -3. In the prebuilt consumer paragraph, replace verification before every hosted - request with verification before hosted writes. Describe the current order: - cheap local/source checks, read-only preflight, full local verification, - then hosted writes. Keep available-access-evidence limits clear. -4. Review preparation wording delivered in Stages 3 and 4, plus current CLI and - integration descriptions, for agreement. Update only canonical owners that - actually need correction. Accepted ADR decision content remains intact. - -**Verification:** compare documentation with `validateBuildIdentity`, automatic -preflight, and existing page-only/prebuilt tests. Reuse -`TestAutomaticPreflightFailurePrecedesPrebuiltArtifactHashing`. These wording -fixes require no new tests that merely repeat existing implementation behavior. -Validate affected local links/fragments and prose whitespace. - -## Stage 10: Verify the complete sprint and close the findings - -**Status:** complete. The complete offline `go test ./...` and -`go test -race ./...` suites and compiled `go run ./internal/apicheck` check -pass with `GOTOOLCHAIN=local GOPROXY=off GOSUMDB=off`. Additional discovery -failure and synchronized cancellation checks pass with `-race`. Changed local -Markdown links and fragments and whitespace checks pass. Public types and -receipt schema remain unchanged; the finding records contain completion evidence. -**Depends on:** all preceding stages. - -1. Resolve every deferred check, format changed Go files, and inspect the final - diff for changes outside the agreed remediation scope. Verify all eight - findings against the coverage map and target behavior in `remediation.md`. -2. Run the complete offline suite and compiled API/dependency-boundary check: - - ```sh - GOTOOLCHAIN=local GOPROXY=off GOSUMDB=off go test ./... - GOTOOLCHAIN=local GOPROXY=off GOSUMDB=off go run ./internal/apicheck - ``` - -3. Run race-enabled tests for the changed Git/subprocess, Go preparation, - Gitea/delivery, and public cancellation paths. A single - `GOTOOLCHAIN=local GOPROXY=off GOSUMDB=off go test -race ./...` is an acceptable - combined check; targeted race runs are sufficient when they cover the affected - paths. Repeat tests only when changes, failures, or unresolved concerns - justify it. -4. Confirm public types and receipt schema are unchanged; strict root lookup - still requires a checkout; standalone preparation still works; default and - explicit module selections agree across preparation/build; and publication - uncertainty and write evidence retain their selected meanings. -5. Review fixture isolation, cancellation readiness/cleanup, useful error chains, - and CLI status assertions. Keep synthetic module caches test-owned and live - infrastructure out of default tests. -6. Validate changed Markdown links/fragments and run the documented whitespace - checks. Review current consumer/GoDoc/integration owners together so the - delivered behavior has one consistent description. -7. Mark verified stage statuses complete here and add completion markers with - relevant test/commit evidence to the corresponding findings in - `correctness.md` and `interfaces.md`. Retain their audit evidence. Set - `remediation.md`'s status to complete only after every finding is addressed. -8. Follow the repository's normal commit discipline for completed work and - report the implementation changes, validation, remaining limitations, and - worktree status. No tag, push, hosted release, or pilot is part of this plan. - -Completion means the final repository implements the agreed target, the -regressions and existing relevant checks pass, and every finding has verified -completion evidence. No unresolved product or policy question remains from the -planning review. diff --git a/docs/roadmap/interfaces.md b/docs/roadmap/interfaces.md deleted file mode 100644 index 66ff4f8..0000000 --- a/docs/roadmap/interfaces.md +++ /dev/null @@ -1,250 +0,0 @@ -# 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. diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md deleted file mode 100644 index fd4ef1e..0000000 --- a/docs/roadmap/remediation.md +++ /dev/null @@ -1,249 +0,0 @@ -# Audit remediation roadmap - -## Status and scope - -The planned remediation was implemented against baseline `59bb074`. A -post-implementation review of `6796310` found two follow-up issues below. -Both are complete as recorded in their completion markers. The implementation -plan records the original stage and validation evidence. - -This roadmap defines the desired behavior and scope for all three findings in -the [functional correctness audit](correctness.md) and all five findings in the -[interface audit](interfaces.md). Those documents retain their reproduction -circumstances and evidence. The [implementation plan](implementation.md) owns -staging, code changes, regression checks, and completion tracking. - -## Purpose and architectural scope - -ReleaseKit owns routine release mechanics so consumers supply product facts and -product-specific checks with little release-related code. As recorded in -[ADR 0002](../adr/0002-routine-releases.md), internal complexity is justified -when it reduces caller-owned release work. In particular, callers can reuse a -repository location across preparation and subsequent orchestration without -normalization code or another configuration option. - -Preparation owns checkout-root resolution when the supplied location is inside -a checkout. Ordinary root-module callers keep the same request shape; callers -selecting a nested module supply its repository-relative `ModuleDir`. -Standalone preparation retains the supplied directory as its base. This is the -selected approach to the preparation-anchor question. - -Apply the same ownership principle to the other corrections: ReleaseKit -validates derived tags, prepares the selected module's dependencies, preserves -cancellation and uncertain publication outcomes, and classifies backend -observations. Consumers continue to use the existing operations and outcomes. -The empty-prebuilt and verification-order findings call for documentation that -accurately explains existing behavior. Evaluate each correction by whether an -ordinary consumer can use it without additional release code or configuration. - -The existing public orchestration and runtime facades and internal package -boundaries remain appropriate. These corrections use the existing public types, -receipt schema, module/tag conventions, and publication/delivery model. - -## Desired behavior - -### Incomplete publication observations - -After a failed push acknowledgment, a query containing only a peeled tag record -provides insufficient evidence about the exact annotated tag object. -Publication remains unknown and retains the original push cause. A complete -response works in either record order. Existing exact-object matches, genuinely -absent tags, different objects, and exact-object-only responses retain their -current outcomes. - -### Dependency preparation and ambient module files - -Preparation uses ordinary Go module discovery from its selected directory even -when inherited or persistent `GOFLAGS` selects an alternate module file. It -retains other effective flags and existing proxy, private-module, -authentication, checksum, cache, workspace, and local-toolchain behavior. -Go retains its existing checksum-update behavior, including upward declaration -discovery for standalone package-directory preparation. - -### Valid candidate tags - -Candidate checking establishes that the complete derived release tag is a valid -Git ref before accepting it as unused. Git's own ref rules remain the authority. -The correction concerns release candidate validity; standalone local builds -retain their existing filesystem choices and version rules. - -### Identifiable build cancellation - -Cancellation or deadline expiration during build source inspection remains -identifiable through the error chain. Ordinary Git failures retain useful -diagnostics, and partial results retain already completed artifacts. - -### Gitea release identity at asset use - -A backend-produced handle with a missing or nonpositive service release ID is a -malformed observation when an asset operation needs that ID. A handle of the -wrong type remains an invalid request. Locally rejected uploads retain -not-attempted write evidence. - -Existing page-only reads retain their current minimal response requirements. -This correction adds no universal positive-ID or page-URL requirement to a -successful release lookup and preserves existing delivery progress semantics. - -### Consistent preparation paths - -Within a Git checkout, preparation resolves its containing root automatically -and interprets `ModuleDir` relative to that root. Empty selection means the -root module, consistently with `Check`, `Build`, and `Deliver`. This includes -calls made from checkout subdirectories and filesystem aliases. - -Outside a Git checkout, preparation retains the supplied directory as its base -and the existing relative module selection. Standalone use remains supported. -It retains its existing Go-only use when Git is unavailable and the directory -can be identified as outside a checkout. -Discovery distinguishes this case from cancellation, permission failures, and -other Git errors, which remain failures. Existing symlink and containment rules -and the CLI's root normalization continue to apply. - -### Empty prebuilt results - -Page-only callers ordinarily omit both `Build` and `Prebuilt`. A supplied -prebuilt result with no targets or artifacts still identifies the matching -repository, module, tag, commit, and version. Documentation states that -requirement clearly. A zero `BuildResult` gains no new compatibility behavior. - -### Artifact-verification order - -Documentation consistently describes cheap local/source checks, read-only -credential preflight, full local artifact verification, and then hosted writes. -Digest verification completes before hosted writes. Credential preflight can -precede that verification and confirms only the access evidence exposed by the -backend. - -## Scope boundaries - -The corrections retain the current source-verification observations, bounded -publication recovery, explicit forge selection, and additive hosted delivery -policy. They introduce no new public configuration knobs, compatibility shims, -release procedure, pilot release, or intermediate human-review requirement. -The targeted sprint reaches this complete end state through local repository -work and automated validation. - -## Post-implementation review findings - -The review compared implementation commits `0632aa5` through `6796310` with the -agreed target and the plan at `730e638`. It examined production changes, new -regressions, current documentation, and the final package boundaries. The -following findings required follow-up; the other planned corrections were present. - -### 1. Damaged checkout metadata can silently select standalone preparation - -**Priority:** medium. **Status:** complete. - -**Completion evidence:** `TestPrepareBuildDependenciesRejectsDamagedCheckout` -verifies healthy root selection, preservation of the damaged checkout's Git -failure with no Go invocation, and genuine standalone preparation. Git discovery -regressions cover ceiling directories, physical and literal aliases, and the -filesystem boundary. Focused preparation and discovery checks pass offline with -the race detector. The implementation plan now requires marker inspection before -accepting the recognized no-repository diagnostic. - -**Code:** `DiscoverRepositoryRoot` in -[internal/git/source.go](../../internal/git/source.go), particularly the -recognized no-repository branch, and public `PrepareBuildDependencies` in -[preparation.go](../../preparation.go). - -The discovery code treats exit status 128 and the recognized no-repository -diagnostic as conclusive evidence of standalone use. Git emits that same -diagnostic when a checkout's `.git` directory remains present but its `HEAD` -file is missing. The marker inspection used when Git is unavailable does not -run on this branch. A broken `.git` file has coverage, but this damaged-directory -case does not. - -**Reproduction:** a temporary public-API probe created and committed root and -child modules, then called preparation from the child with empty `ModuleDir`. -With healthy metadata it selected the root. After moving `.git/HEAD` out of the -disposable checkout, the same request returned nil and ran Go in the child. -A local Go recording shim observed the directory without downloading anything. - -**Expected:** damaged checkout discovery fails before dependency preparation; -the request does not silently acquire a different selection base. This follows -the agreed treatment of broken metadata and other discovery failures. - -**Actual consequence:** in a multi-module repository, metadata damage can make -an unchanged request prepare another module's graph and report success. This -is a remaining gap in checkout selection and failure handling. - -**Correction:** establish that the no-repository result represents standalone -use before accepting it as a fallback. Account for relevant existing checkout -metadata and preserve the Git failure when that metadata is damaged. Keep Git's -normal discovery boundaries and genuine standalone/Go-only use; do not turn -this into a general directory-name restriction or a new public option. Add a -real damaged-`.git`-directory regression through the public preparation facade, -asserting an error and no Go invocation, alongside healthy and standalone -controls. The original plan's diagnostic-only classification also needs this -refinement; reproducing its algorithm alone is insufficient completion evidence. - -### 2. The deadline regression starts its timeout before inspection readiness - -**Priority:** low. **Status:** complete. - -**Completion evidence:** the deadline variant now uses a test-controlled -expiration context triggered after the blocked Git inspection signals readiness. -The public initial/final cancellation cases, completed-artifact assertions, and -ordinary Git error-chain check remain in place; the result wait shares the -bounded watchdog. Focused source-inspection tests pass with the race detector. -The deadline case also passes normally and with `-race` under the same -four-second repository-discovery delay that reproduced the original failure. - -**Code:** `TestBuildSourceInspectionCancellation/deadline` in -[build_source_cancellation_test.go](../../build_source_cancellation_test.go). - -The deadline context starts with a three-second timeout before public `Build` -resolves the repository and module and reaches `cleanSource`. The test waits -for the inspection wrapper's readiness marker afterward, but that marker does -not control when expiration begins. The separate 45-second readiness watchdog -cannot prevent this earlier deadline from expiring during unrelated setup. - -**Reproduction:** running only this subtest with a temporary delegating Git -wrapper that delays `rev-parse --show-toplevel` by four seconds fails with -`build ended before inspection` and a repository-resolution deadline error. -The test never exercises source inspection. The ordinary suite passes without -the injected startup delay. - -**Expected:** the deadline regression exercises expiration during the blocked -source-inspection command, as required by the plan's synchronized-readiness -case. Unrelated setup latency does not determine whether that path is tested. - -**Actual consequence:** a slower runner can fail the default suite for timing -outside the behavior being tested. This is a test reliability gap; the probe -does not demonstrate another production cancellation defect. - -**Correction:** make the expiration case readiness-controlled, or isolate it at -the source-inspection helper boundary so earlier repository/module work cannot -consume its deadline. Retain public cancellation and partial-artifact coverage, -bounded cleanup, and the `errors.Is(context.DeadlineExceeded)` assertion. -Increasing the whole-operation timeout alone does not establish the intended -inspection-specific synchronization. Verify the revised case under delayed -unrelated setup as well as normal and race-enabled execution. - -## Review validation and scope assessment - -The ordinary offline `go test ./...` suite and compiled -`go run ./internal/apicheck` check passed on the reviewed code with -`GOTOOLCHAIN=local GOPROXY=off GOSUMDB=off`. Race-enabled Git, Gitea, Go-tool, and -candidate package checks also passed. The temporary delayed-startup test above -failed as expected and substantiated finding 2; it is separate from the ordinary -passing suite. Focused public preparation, publication, cancellation, delivery, -and CLI regressions also passed with the race detector. Probes used disposable -local repositories and command wrappers, with no production changes or new -committed tests. - -The root orchestration facade, separate runtime facade, neutral forge boundary, -and Gitea adapter retain their responsibilities. Ref finalization, candidate tag -validation, cancellation error propagation, module-file override, and asset-use -failure classification follow the selected approach. The review found no -additional public types, configuration knobs, receipt changes, stronger page-only -response requirements, or broader source/preflight guarantees introduced by the -implementation. The two findings concern incomplete realization of the agreed -behavior and regression reliability rather than an intentional scope expansion. - -After the follow-up corrections, the complete offline `go test ./...` suite and -compiled `go run ./internal/apicheck` check pass. Focused discovery, dependency -preparation, and source-inspection regressions pass with the race detector. -Roadmap local links and whitespace checks pass. Both follow-up findings are -resolved within the existing public interfaces and package boundaries.