diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index 29fb380..65ddd47 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -9,6 +9,12 @@ 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. + 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; @@ -96,7 +102,7 @@ small; the gap is in the planned regression protection. This enforces the already selected separation from orchestration. It does not require a new public package restriction or a broader dependency policy. -## 3. Qualify cancellation exit-status documentation — P3 +## 3. Qualify phase-specific CLI exit-status documentation — P3 Status: open. @@ -119,6 +125,14 @@ A temporary shared-delivery test supplied an attempted uncertain upload with 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 @@ -126,6 +140,11 @@ requested preflight behavior and conflicts with preserved write recovery. 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. @@ -148,6 +167,64 @@ 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: open. + +**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. + ## Verification performed Using Go 1.26.10 on linux/amd64 with module/toolchain downloads disabled: @@ -164,3 +241,23 @@ Using Go 1.26.10 on linux/amd64 with module/toolchain downloads disabled: 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.