releasekit/docs/roadmap/remediation.md

# Remediation: forge boundary and credential preflight review

Status: remediation in progress; completion is recorded per issue below.

## Review baseline and scope

Reviewed `cbdba90`, including the implementation commits after `9d6952a`,
against the [feature roadmap](forge-boundary.md),
[implementation plan](implementation.md), and the user's selected scope.
This document records work identified by the review; it does not implement it.

A follow-up integration audit reviewed `72de187`. Its production source is
unchanged from `cbdba90`. That audit traced the new boundary and preflight
through existing CI preparation, module/tag selection, build receipts, token
loading, source publication, and retry/error handling. It added finding 5 and
expanded finding 3 with a later authorization-failure reproduction.

An algorithmic audit reviewed `370fe3a`, whose production source is still
unchanged from `cbdba90`. It added findings 6–8. These opportunities and defects
were retained when policy moved from the former Gitea implementation into
shared delivery.

The intended production boundary is present. The root facade owns neutral
destination selection and public preflight errors; `internal/forge` supplies
the backend contract; `internal/delivery` owns page/asset policy and recovery;
`internal/gitea` owns protocol operations; and `internal/artifact` owns local
declarations and content verification. Gitea remains the implemented backend.
Automatic and standalone preflight use read-only repository requests, and binary
CI checks access before dependency preparation. Source publication retains its
separate Git authentication and completion semantics.

## 1. Complete local destination preflight error handling — P2

Status: complete.

**Completed:** Added the facade-owned `DeliveryPreflightInvalidDestination`
classification, preserved the adapter's validation cause, and mapped it to CLI
status 2 for ordinary and CI delivery. Updated consumer/configuration guidance.
Focused standalone, automatic, and CLI regressions cover invalid URLs, owners,
and repository names, early termination, and absence of hosted/build work.
Focused preflight tests and the compiled API/dependency check passed.

**Evidence:** [Gitea preflight](../../internal/gitea/backend.go),
[public error conversion](../../delivery.go), and
[CLI error handling](../../cmd/releasekit/main.go).

`Backend.Preflight` returns `destinationAPI` validation errors directly. Those
errors are neutral `forge.Failure` values with `FailureInvalid`, rather than
classified preflight failures. `publicPreflightError` consequently passes them
through. Both CLI delivery paths treat the resulting unclassified error as
status 1.

A temporary consumer with an otherwise valid tagged source and a nonblank
synthetic token reproduced this with `base_url: "ssh://forge.example"`:
`PreflightDelivery` returned `*forge.Failure`, `errors.As` could not obtain a
`*DeliveryPreflightError`, and `deliver` exited 1. A malformed owner/repository
segment follows the same local validation path. No server request is needed to
identify these configuration problems.

This leaves the public preflight error description incomplete and conflicts
with the documented status 2 for profile/configuration failures. Operators
cannot reliably distinguish a locally invalid destination from an operational
verification failure through the new boundary.

**Required work:**

- Make deterministic destination-setting failures recognizable through the
  facade and classify them as CLI configuration failures with status 2 in both
  ordinary and CI delivery. Keep backend-specific validation in its adapter;
  share its classification instead of duplicating Gitea rules in the CLI.
- Make GoDoc and the selected public error representation agree about local
  configuration failures. Preserve useful causes and identify the invalid
  setting; avoid labeling a malformed URL as a rejected token or an unavailable
  service. Keep real service/transport failures at status 1.
- Add focused coverage for an invalid URL and invalid repository addressing,
  including standalone preflight and the CLI mapping. Verify early failure,
  `not_attempted` delivery state, and absence of hosted requests/build work.

This correction concerns existing destination rules and failure classification;
it adds no token-format restriction, remote permission requirement, or probe.

## 2. Protect the runtime boundary from the new forge contract — P2

Status: complete.

**Completed:** Added `forge` to the runtime dependency exclusions and covered
both direct and transitive forbidden dependencies with focused graph fixtures.
The complete checker test package and the actual compiled API/dependency check
passed; the current runtime dependency graph remains accepted.

**Evidence:** [dependency checker](../../internal/apicheck/main.go), especially
`forbiddenRuntimeDependency`, and the runtime boundary in
[architecture policy](../policy/architecture.md#runtime-version-boundary).
Implementation Stage 9 also calls for keeping the new delivery packages out of
the runtime dependency graph through the existing checking mechanism.

The checker excludes named delivery implementations such as `gitea` and
`delivery`, but its runtime exclusion list omits the newly added `forge`
package. A temporary test overlay supplied this package graph:

```text
buildinfo -> internal/forge
```

`checkImportDirections` returned nil. The graph would make the runtime facade
depend on the release backend contract while the advertised boundary check
continued to pass. The actual current `buildinfo` graph remains appropriately
small; the gap is in the planned regression protection.

**Required work:**

- Extend the existing runtime check to exclude the new forge delivery contract.
  Retain the existing policy for legitimate runtime dependencies.
- Add a focused fixture showing rejection of a runtime dependency on
  `internal/forge`, including traversal through a small intermediary where
  needed to protect the existing transitive rule. Retain the passing actual
  repository check.

This enforces the already selected separation from orchestration. It does not
require a new public package restriction or a broader dependency policy.

## 3. Qualify phase-specific CLI exit-status documentation — P3

Status: complete.

**Completed:** Qualified the CLI reference and unreleased v0.6.0 note by phase:
local loading/classified preflight errors use status 2; later known failures
use 1; unresolved writes retain 3 even when canceled. Added a narrow CLI
regression for preflight versus later authorization failure and cancellation
after an attempted page write. Focused CLI preflight/recovery tests passed.

**Evidence:** [CLI exit statuses](../cli.md),
[v0.6.0 release note](../releases/v0.6.0.md),
[write evidence](../../internal/gitea/backend.go),
[asset reconciliation](../../internal/delivery/remote_assets.go), and
[CLI delivery mapping](../../cmd/releasekit/main.go).

The new CLI prose and release note broadly state that cancellation returns
status 1. That is correct for preflight and preparation cancellation. After a
possibly successful write, cancellation can prevent reconciliation and leave
delivery unknown; the CLI correctly returns status 3 in that case.

A temporary shared-delivery test supplied an attempted uncertain upload with
`context.Canceled` and an unavailable canceled reconciliation. It returned
`State == Unknown`, retained the cancellation cause, and placed the asset in
`Uncertain`. The CLI's unknown-state branch maps that result to 3.

The broad documentation introduces a stronger cancellation guarantee than the
requested preflight behavior and conflicts with preserved write recovery.

The same integration issue affects the unqualified statement that credential
failures return status 2. With valid tagged source, a synthetic 401 on the first
repository preflight read returned status 2. When both repository preflight
reads succeeded and the hosted-tag read then returned 401, ordinary delivery
returned status 1. Later provider failures retain the existing delivery error
path and are not `DeliveryPreflightError` values. Successful preflight cannot
guarantee that authorization will remain valid during later operations.

**Required work:**

- Scope status 1 wording to preflight/preparation cancellation and cancellation
  that leaves no unresolved write. State that an unresolved write retains
  status 3, including when cancellation prevented verification.
- Align the CLI reference and unreleased v0.6.0 note. Keep implementation
  behavior and unknown-write recovery intact.
- Qualify credential/access status 2 as applying to local credential loading
  and classified preflight failures. Describe later authorization failures
  through the existing delivery outcomes: known failure can return 1, while an
  unresolved write retains 3. Do not reclassify later failures as preflight
  failures or erase partial progress to satisfy overbroad prose.
- Reuse existing cancellation/recovery tests. If CLI coverage does not already
  distinguish these cases, add a narrow case protecting unknown-write status
  after cancellation; avoid an expanded cancellation matrix.

## 4. Align roadmap completion records — P3

Status: complete.

**Completed:** Marked the feature roadmap's implementation complete, labeled
both planning baselines as historical, and linked both plans to this
remediation record. Reviewed the status wording against the completed stages
and retained the plans for release-preparation cleanup.

**Evidence:** [feature roadmap](forge-boundary.md) and
[implementation plan](implementation.md).

The implementation plan marks all ten stages complete, while the feature
roadmap still says implementation has not started. The plan's introductory
baseline text also uses present-tense descriptions of the old implementation
and says the neutral destination and preflight are planned.

**Required work:** Mark the feature roadmap's implementation status accurately
and clearly label the implementation plan's original baseline as historical.
Keep these records consistent with the completed stages and remaining
remediation. Roadmap retirement can follow the existing release-preparation
cleanup; this finding does not require removing plans now.

## 5. Bind CI release identity before online preflight and preparation — P2

Status: complete.

**Completed:** CI now derives the selected module identity from the verified
event commit and checks the complete tag before credential loading, forge
requests, or dependency preparation. The optional launcher-module guard and
delivery's later source/preflight verification retain their responsibilities.
A focused page-only/binary regression covers a matching launcher selection
with a mismatched derived tag and verifies no forge or Go-tool work. That test
and existing valid CI/preflight/managed-delivery tests passed.

**Evidence:** [CI adapter](../../cmd/releasekit/main.go), especially
`runCIDelivery` and `checkExpectedModule`, and the existing identity/tag checks
in [shared delivery](../../internal/delivery/delivery.go).

Binary `ci-deliver` invokes standalone credential preflight and application
dependency preparation before `Deliver` derives the selected module's tag and
checks it against the CI event tag. `PrepareTaggedCheckout` proves that the
supplied Git tag and commit agree; it does not prove that this is the release
tag derived from the profile's module. The optional expected-module guard
compares selected directories and also does not establish that relationship.

A temporary CLI fixture had a clean tagged checkout at `v1.2.3`, a profile
selecting committed module `child`, and valid repository credentials. The
selected module's derived tag was `child/v1.2.3`. With `--prepared` and
`--expected-module child`, both profile variants eventually returned the same
tag-mismatch error, but their earlier effects differed:

| Profile | Forge requests | Dependency preparation |
| --- | --- | --- |
| Page-only | None | Skipped |
| Binary | Two credential-preflight reads | Invoked before the mismatch was reported |

Neither variant made a hosted release write; the existing late guard still
prevents delivery under the wrong module tag.

The dependency command was intercepted by a temporary local Go wrapper;
no application dependencies were fetched for the reproduction. A real binary
job can perform avoidable downloads or let a preparation failure obscure the
already determinable identity mismatch. A preflight credential failure can
likewise hide the source-selection error.

Dependency preparation already preceded this guard before the feature work.
The new standalone preflight adds hosted credential requests to that existing
ordering gap. Page-only delivery continues to reach the shared identity check
before any hosted requests, so the integration now has inconsistent early
validation behavior across the two supported CI paths.

**Required work:**

- After verifying the event checkout and loading its profile, derive the
  committed selected-module release identity and apply the existing full-tag
  comparison before the first forge probe or application dependency download.
  Reuse the existing identity derivation and keep the optional launcher-module
  comparison's responsibility separate.
- Preserve credential preflight before expensive dependency/build work, with
  normal delivery verification after preparation. This moves an existing
  acceptance check earlier; it adds no module map, source-publication gate,
  candidate test suite, or extra requirement to standalone `PreflightDelivery`.
- Add a focused CI regression for a valid Git tag/commit and matching
  `--expected-module` whose derived module tag differs. Cover page-only and
  binary profiles, asserting the identity diagnostic and absence of forge
  requests or dependency preparation. Retain the valid binary path's early
  credential check and subsequent delivery behavior.

## 6. Make pending-asset bookkeeping linear — P3

Status: open.

**Evidence:** `uploadMissingAssets`, `reconcileAssetUpload`, and
`removeIdentity` in [shared asset delivery](../../internal/delivery/remote_assets.go).

Every acknowledged upload removes its filename from `result.Pending` through
`removeIdentity`. That helper searches the slice and shifts its remaining tail.
For `k` initially missing assets uploaded successfully in their pending order,
the searches find the first element immediately, but the deletions still move
`(k-1) + (k-2) + ... + 1 = k(k-1)/2` identities. Bookkeeping alone is therefore
Theta(`k²`), independently of network and content processing. A different
pending order can also incur repeated linear searches. Reconciled uploads use
the same removal helper.

**Refactoring opportunity:** Track pending membership or per-filename state in
a map, update it in expected constant time per completed/reconciled upload,
and materialize `Pending` with one pass through the initial pending order when
returning. For `n` declarations, this permits O(`n`) total category bookkeeping
and O(`n`) metadata space. Keep the current category ordering during this
refactor; a final pass avoids repeated shifts without requiring swap deletion.
Apply finalization to every return path so partial, conflicting, and unknown
outcomes retain later missing assets and disjoint categories.

Existing progress/reconciliation tests should remain useful. Add a focused
case only if needed to protect pending results after early termination. The
usual small target sets limit practical impact; this finding establishes an
avoidable asymptotic cost without claiming measured release-time savings.

## 7. Order the missing upload set once — P3

Status: open.

**Evidence:** `orderedExpected`, `orderMissing`, and `uploadMissingAssets` in
[shared asset delivery](../../internal/delivery/remote_assets.go).

`orderedExpected` copies and sorts all `n` declarations before validation and
classification. This happens even when every asset matches, a conflict stops
delivery, or an unverifiable asset prevents uploads. If uploads proceed, the
coordinator copies and sorts the full list again, then skips entries outside
the missing set. The first full sort requires O(`n log n`) comparisons in the
worst case. The second sort receives already ordered input, so its actual cost
can be lower, but its full-list copy and ordering pass are still redundant.

**Refactoring opportunity:** Collect declarations without ordering, validate
and classify them in a linear pass, and return immediately when classification
prevents or eliminates uploads. Collect only the `k` missing declarations and
sort that worklist once, retaining filename order and `SHA256SUMS` last.
Declaration/classification/ordering work becomes O(`n + k log k`), with O(`n`)
metadata space, and O(`n`) work when no upload is needed. This also removes
redundant full-list copies. These bounds treat filename operations as unit
cost and exclude inline-content hashing, which remains linear in its bytes.

Keep complete sorted manifest generation and all validation. The plan selected
the upload ordering and manifest format; this opportunity concerns the size
and duplication of the scheduling work, not either selected behavior. The
existing ordering, matching-manifest, and no-write-on-conflict tests cover the
main behavior that the refactor should preserve. Combined with finding 6,
successful upload coordination can avoid its quadratic bookkeeping term.

## 8. Preserve asset-inspection failure causes during aggregation — P2

Status: open.

**Evidence:** `inspectRemoteAssets` and `uploadMissingAssets` in
[shared asset delivery](../../internal/delivery/remote_assets.go), and the
cause-preservation requirement in the
[implementation plan](implementation.md#internal-ownership-and-dependencies).

An asset-open or content-read failure is retained in `AssetInspection.Err` and
classified as `AssetUnverifiable`. `uploadMissingAssets` then discards those
errors and returns a fresh generic `FailureUnverifiable` message. `Deliver`
returns that aggregate error, so callers lose the underlying diagnostic and
cannot use `errors.Is` or `errors.As` to recognize its cause.

A temporary test overlay supplied one existing asset whose `OpenAsset` returned
`context.Canceled`, then repeated the case with a distinct read-failure
sentinel. Inspection retained each cause. Aggregation correctly reported
`NotAttempted`, one uncertain asset, and no uploads, but `errors.Is` could find
neither cause in its returned error. Whole-list failures already return their
original cause directly; this finding concerns per-asset failures folded into
an otherwise successful inspection.

**Required work:** Collect relevant non-nil observation errors during the
existing classification pass and wrap or join them with the aggregate summary.
Preserve actionable asset context, cancellation identity, and the existing
conflict precedence, asset categories, and delivery states. Read-only failures
remain distinct from unresolved writes. This takes O(`n`) traversal and O(`u`)
error references for `u` unverifiable observations, without a new public error
type or protocol operation. Retain a focused regression asserting cause
identity through the returned delivery error, including cancellation.

## Verification performed

Using Go 1.26.10 on linux/amd64 with module/toolchain downloads disabled:

- `go test ./...` passed.
- `go run ./internal/apicheck` passed for the current repository.
- `go test -race ./internal/delivery ./internal/gitea` passed.
- Temporary review fixtures reproduced findings 1 and 2, and confirmed the
  cancellation outcome underlying finding 3. The runtime-boundary reproduction
  intentionally failed its assertion because the prohibited graph was accepted.
- An additional temporary HTTP/1.1 and HTTP/2 check confirmed that a large upload
  rejected before its body was consumed returns promptly as a known rejection;
  that investigated path produced no finding.

Temporary fixtures and test overlays were kept outside the checkout. No
implementation files or permanent tests were changed during this review.

### Follow-up integration audit

The same Go 1.26.10 toolchain and offline environment were used. Existing full
suite, API-boundary, and focused race results above remain applicable because
the production source was unchanged. Additional checks comprised:

- A fresh CLI integration run covering tagged CI preparation, managed/prebuilt
  delivery and stale receipts, module-relative output, multi-module dispatch,
  Woodpecker workflows, and source-tool bootstrap; all selected tests passed.
- Focused token-file precedence, repository-root token-file loading,
  forbidden-file validation, and check/publication integration tests passed.
- Temporary CLI fixtures demonstrating finding 5 and the initial-versus-later
  401 exit-status difference in finding 3, using a local HTTP server and an
  intercepted dependency command.

The build, receipt, publication, candidate-validation, dependency-preparation,
and Go-tool implementation owners were also compared with the pre-feature
baseline. Their production code is unchanged; the confirmed new interaction
occurs in the CI adapter's composition and the phase-specific error contract.

### Algorithmic audit

The audit covered the implemented plan's facade/CLI adaptation, backend
contract and preflight, page coordination/recovery, local artifact mechanics,
remote inspection/upload bookkeeping, Gitea read/write transport, and dependency
checks. It compared the moved code with the pre-feature baseline. Bounds below
describe collection/content work with ordinary expected constant-time map
operations and unit-cost metadata comparisons; variable-length names and URLs
add their corresponding byte-processing costs.

| Reviewed operation | Assessment |
| --- | --- |
| Remote asset inspection | Expected O(`n + r + B`) time for `n` declarations, `r` listed assets, and `B` downloaded bytes; filename maps avoid nested list searches, and content hashing is streamed. |
| Artifact declaration and local verification | Map-based identity/duplicate checks are linear; hashing is linear in content bytes. Complete manifest sorting remains the selected O(`n log n`) algorithm. |
| Page matching and recovery | Note comparisons are linear in note bytes; shared coordination uses the selected bounded follow-up lookup. |
| Gitea adaptation and transport | Metadata conversion is linear in listed entries; uploads stream bytes. Redirect tracking uses a visited map rather than repeated scans. |
| Facade result conversion and dependency checks | Result conversion is linear in category entries. Dependency traversal uses visited sets from a fixed number of roots, O(`V + E`) in the reachable graph. |
| Upload scheduling and pending state | Findings 6 and 7 identify avoidable work beyond the selected ordering and recovery behavior. |

The audit preserved explicitly selected algorithms: complete sorted manifests,
filename upload ordering with the manifest last, full local verification before
writes, streamed remote comparisons, one follow-up reconciliation per ambiguous
write, current asset listing behavior, and fresh credential checks around CI
preparation. It proposes no batching, pagination, concurrency, validation cache,
or altered recovery policy.

Fresh offline tests for `internal/delivery`, `internal/artifact`, and
`internal/gitea` and the API/dependency-boundary check passed. A temporary
overlay intentionally failed two cause-preservation assertions to reproduce
finding 8; it was removed after the check. Findings 6 and 7 follow from
source-level operation counts. No benchmarks
or real-world performance measurements were run, and no production code or
permanent tests changed.