diff --git a/docs/roadmap/forge-boundary.md b/docs/roadmap/forge-boundary.md deleted file mode 100644 index 22d5285..0000000 --- a/docs/roadmap/forge-boundary.md +++ /dev/null @@ -1,180 +0,0 @@ -# Roadmap: forge delivery boundary and credential preflight - -Status: implementation complete; review corrections are tracked in -[remediation](remediation.md). - -The [implementation plan](implementation.md) owns stages, concrete design -choices, task sequencing, and verification. - -## Objective and scope - -Introduce an internal boundary between shared hosted-release delivery and the -Gitea implementation. The result should support adding another forge through -an adapter while reusing ReleaseKit's release and asset-delivery policy. -Add operation-specific credential preflight through that boundary so delivery -stops on credential problems before expensive preparation or builds. - -ReleaseKit's `Publish` operation validates and publishes source through Git. -Its `Deliver` operation verifies tagged source and delivers release notes and -declared assets to a hosting service. This refactor concerns the latter operation -and its composition through the existing public facade and CLI. - -Gitea is the concrete backend for this work. Forgejo, GitHub, GitLab, and other -backends remain future implementation work. The boundary should express the -operations used by the current delivery workflow and allow refinement when a -second backend supplies concrete requirements. - -## Historical planning baseline - -The original source baseline was `ac78e7d`, following the published v0.5.0 -module migration and the subsequent token-file and forbidden-file changes. -The following describes that revision, before implementation of this roadmap. - -- [delivery.go](../../delivery.go) exposes `GiteaDestination` through - `DeliveryRequest.Destination` and converts it into `gitea.Destination`. -- [internal/delivery](../../internal/delivery/delivery.go) resolves source - identity, checks tagged source, reads notes, coordinates builds, and directly - calls `gitea.Client` for release and asset operations. Its result conversion - and failure classification use Gitea-owned internal types. -- [release.go](../../internal/gitea/release.go) combines Gitea HTTP operations - with hosted-tag verification, matching-page acceptance, release creation, - and reconciliation of ambiguous creates. -- [assets.go](../../internal/gitea/assets.go) and - [upload.go](../../internal/gitea/upload.go) combine HTTP operations with - manifests, local-file verification, remote-content comparison, asset - observations, upload sequencing, and retry bookkeeping. -- CLI credential resolution supplies an explicit token from the configured - environment variable or token file. The profile describes a Gitea repository; - it has no forge selector. -- CLI loading checks token availability, but forge API authentication first occurs - through delivery requests after application dependency preparation and managed - builds. Library delivery also checks a blank token only at the later Gitea - request boundary. There is no dedicated authenticated credential preflight. - -The [Gitea integration guide](../integrations/gitea.md) and public Go declarations -now describe the implemented contracts. Behavioral tests provide regression -coverage for the selected behavior. The -[architecture policy](../policy/architecture.md) governs facade ownership and -dependency direction. - -## Targeted end state - -### Neutral public destination and explicit forge selection - -Replace `GiteaDestination` with a facade-owned neutral destination type and -require explicit forge selection in the public API and delivery configuration. -Gitea is the sole supported selection initially. -An omitted, blank, or unsupported selection produces a validation error; Gitea -is selected explicitly rather than inferred from a default or destination URL. - -The destination represents the addressing and credentials needed for current -delivery through neutral public names. Concrete backend construction uses the -selected forge. Additional backends may extend that model when their addressing -and protocol requirements are implemented. - -### Shared delivery responsibilities - -Shared internal code owns: - -- Automatic credential preflight coordination before expensive delivery work. -- Captured source identity, local tag/commit checks, committed release notes, - managed or prebuilt artifact coordination, and local artifact verification. -- The existing logical rules for accepting matching hosted content, reporting - conflicts, creating missing content, and reconciling uncertain writes. -- Manifest generation, content comparison, and asset observations used to decide - which declared assets require delivery. -- Aggregation into the existing public delivery outcomes and asset categories. - -These operations consume backend-neutral internal values and operations. The -generic delivery workflow has no dependency on `internal/gitea`. The facade or -private composition code constructs the concrete backend and supplies it to -shared delivery. - -### Credential preflight - -Delivery automatically checks the credentials required for its selected backend. -Local checks establish availability and valid destination settings; a read-only -backend probe establishes authentication and access to the configured repository. -Reliable evidence of insufficient required permissions stops the operation. -Unexposed permissions remain unverified; successful reads establish no blanket -guarantee that later writes will succeed. Preflight performs no test writes and -preserves ordinary delivery authorization and recovery handling. - -Library and ordinary CLI delivery perform preflight before managed builds and -full artifact verification. CI loads the verified tagged profile first, then -performs preflight before application dependency preparation. A callable facade -operation lets consumers check a destination before their own preparation, -without requiring a release tag or note to exist. - -Failures distinguish missing or rejected credentials, insufficient permissions, -an inaccessible destination, and unavailable or unverifiable service responses. -Delivery reports `not_attempted` for these pre-write failures. Diagnostics retain -useful causes while excluding tokens. Each check describes access at that time; -later operations retain their normal checks. - -Required credentials follow operation scope. Candidate checking and standalone -builds remain credential-free. Source publication continues using Git's native -SSH/HTTPS authentication and existing remote guards, independently of hosted -delivery credentials. This feature adds no Gitea-token requirement to `Publish`. - -### Backend responsibilities - -The Gitea adapter owns destination validation specific to Gitea, API routes, -authentication headers, request and response formats, pagination, multipart -uploads, attachment downloads, and HTTP-specific error interpretation. -It also owns authenticated credential probes and interpretation of available -permission evidence, using repository-scoped checks without requiring unrelated -account or administration scopes. - -The internal contract supplies the logical operations needed to preflight -credentials, inspect a hosted tag, find or create a release, enumerate assets, -read asset content, and upload an asset. Release and asset references carry -backend-owned handles; shared orchestration works with logical identity and content rather than -interpreting Gitea's positive integer IDs. - -The contract also carries the evidence needed for existing recovery behavior: -whether a write was attempted, whether its outcome is established or uncertain, -and actionable failure information. Generic code interprets delivery outcomes; -the adapter interprets protocol responses. A logical backend operation may -involve multiple HTTP requests. - -The operations and values stay internal under the existing facade policy. -Current Gitea behavior is the concrete compatibility target. This work does -not establish compatibility guarantees for unimplemented forges or a public -plugin protocol. - -## Behavioral scope and compatibility - -The refactor preserves the existing Gitea delivery contracts for complete note -bodies, retained existing titles, stable release state, content conflicts, -unrelated attachments, additive retries, and source-only or binary delivery. -Read-only inspection failures and unresolved writes retain their distinct -outcomes. Recovery retains write history, including confirmed progress after -an ambiguous create. The current integration guide owns the concrete protocol -and follow-up lookup behavior. - -Local verification reuse, streaming, cancellation, stream closure, temporary -build ownership, and upload cleanup remain part of the affected workflow. -Read-only credential preflight precedes expensive local artifact work; local -content verification still completes before hosted release or asset writes. -Credential environment/file selection and forbidden-file validation retain -their existing responsibilities. Backend-specific address validation stays -with that backend. - -The public destination replacement, required forge selection, and earlier -credential failures are deliberate changes for the next minor release. -Migration guidance describes consumer edits under the project's direct-cutover -approach. -Current contract documents and examples change with implementation; historical -release notes retain their original contents. - -Completion means shared delivery and asset policy have independent internal -ownership, Gitea implements the backend boundary, existing Gitea contracts work -through it, and the neutral public destination and explicit selection are -consistent across the API, configuration, examples, and documentation. -Credential preflight operates through the backend boundary in both library and -CLI delivery, with accurate failure distinctions and documented permission limits. - -Additional forge implementations and compatibility claims belong to later work. -Release preparation and publication continue under the -[existing procedure](../release.md). diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md deleted file mode 100644 index 916dad8..0000000 --- a/docs/roadmap/implementation.md +++ /dev/null @@ -1,570 +0,0 @@ -# Implementation plan: neutral forge delivery and credential preflight - -Status: implementation complete; Stages 1–10 are complete. -Follow-up review corrections are tracked in [remediation](remediation.md). - -## Purpose and execution - -Implement the target in the [feature roadmap](forge-boundary.md). That document -owns scope and user choices; this plan owns concrete design and ordered work. -The historical planning baseline was `44823ce`, whose source implementation -was unchanged from `ac78e7d`. It included Gitea delivery, token files, and -`forbidden_files`. The neutral destination and credential preflight were the -planned additions. The sections below retain the original implementation plan; -the completed stages record its execution. - -Execute Stage 1 through Stage 10 in order as one end-to-end sprint. Each stage -is a focused Luna implementation task. Intermediate stages may leave callers -or tests uncompilable while responsibilities move. Continue without an -intermediate release, pilot, external action, or human review. Run focused -checks when the affected packages compile; Stage 10 owns final assembled -verification. Record completion here and follow the -[agent policy](../policy/agents.md) for commits and validation reporting. - -Read the [development guide](../development.md) and applicable policies. Use -the [architecture policy](../policy/architecture.md), -[testing policy](../policy/testing.md), and -[documentation policy](../policy/documentation.md) throughout. Existing GoDoc, -[configuration](../config.md), [CLI](../cli.md), and -[Gitea integration](../integrations/gitea.md) documents define current contracts. -Tests supply regression evidence; assess incidental implementation assertions -against those contracts before preserving them. - -This sprint implements the Gitea backend behind a neutral boundary. Git source -publication, module/tag derivation, build targets, buildinfo, receipts, token -precedence, and optional candidate validators retain their responsibilities. -Additional backends, public backend registration, new transport retry policies, -and release publication are outside this sprint. - -## Selected design - -### Public API and configuration - -Replace the root facade's `GiteaDestination` with these facade-owned declarations: - -```go -type Forge string - -const ForgeGitea Forge = "gitea" - -type DeliveryDestination struct { - Forge Forge - BaseURL string - Owner string - Repository string - Token string -} -``` - -`DeliveryRequest.Destination` becomes `DeliveryDestination`; its other fields, -the public result shape, state values, and asset categories remain as defined -today. Use explicit conversions into internal values. The facade owns backend -selection and construction through a private helper in `delivery.go`. - -Add `delivery.forge` to the existing flat JSON delivery object. Its supported -value is `"gitea"`, corresponding to `ForgeGitea`. Omitted, empty, whitespace-only, -and unsupported selections are errors when delivery is requested. Selection is -explicit in both interfaces; neither URL inspection nor an omitted field selects -a backend. Keep `base_url`, `owner`, `repository`, `token_env`, and `token_file` -in their current locations. The owner/repository fields express the current -backend's addressing needs; future adapters can extend the neutral model when -needed without promising every forge the same address structure now. - -Keep semantic delivery validation at the delivery entry points. Strict JSON -decoding still applies to all commands, but `check`, `publish`, and `build` do -not acquire a new requirement for a delivery object, valid forge selection, or -credentials. `ci-deliver` first prepares/verifies the tagged checkout and loads -its profile as today, then checks the forge before token loading and application -dependency preparation. Ordinary `deliver` checks it before loading receipts, -building, or making hosted requests. Library selection failure returns -`DeliveryNotAttempted` with the requested version and an error before invoking -the generic workflow. Credential preflight failures use the classification -described below. - -After explicit Gitea selection, retain `RELEASEKIT_GITEA_TOKEN` as the CLI's -existing default token environment name. Preserve configured names, environment -over file precedence, trimming and file path resolution, and status 2 for -configuration/credential errors. The library receives `Token` explicitly. -Gitea's remaining destination validation stays in the adapter. Consumer -migration replaces the public type, supplies `Forge: releasekit.ForgeGitea`, -and adds `"forge": "gitea"` to each delivery profile. Use a direct API cutover -without a legacy `GiteaDestination` alias or implicit-selection compatibility -path. - -### Credential preflight contract and ordering - -Add `PreflightDelivery(ctx context.Context, destination DeliveryDestination) error` -to the root facade. It checks the selected destination without requiring source, -version, tag, note, or artifact inputs. `Deliver` automatically uses the same -internal preflight implementation; callers do not need to invoke the standalone -operation for ordinary delivery. - -Use a facade-owned `DeliveryPreflightError` with `Kind` and `Err` fields, -`Error`, and `Unwrap`. Its named string kind, `DeliveryPreflightFailureKind`, -has these constants: - -| Constant | Value | -| --- | --- | -| `DeliveryPreflightMissingCredential` | `missing_credential` | -| `DeliveryPreflightInvalidCredential` | `invalid_credential` | -| `DeliveryPreflightInsufficientPermissions` | `insufficient_permissions` | -| `DeliveryPreflightInaccessibleDestination` | `inaccessible_destination` | -| `DeliveryPreflightUnavailable` | `unavailable` | - -Convert internal classifications explicitly and retain causes and cancellation -identity without exposing internal types in -exported signatures. The unavailable category includes network/service failures -and malformed responses that prevent verification. A 404 can mean hidden or -missing repository access; diagnostics must preserve that ambiguity. A 403 -establishes denied access, not necessarily an expired or invalid token. - -Preflight checks local destination rules and nonblank credentials, then invokes -the backend's read-only authenticated repository probe. It fails on rejected -credentials, inaccessible target access, and reliably established deficiencies -in permissions needed for delivery. Scope checks to release/asset delivery; -code-push permission alone is not an equivalent release permission. Unexposed -token scopes or write permissions remain unverified and do not independently -fail preflight. A nil error establishes authentication and repository access -plus no known delivery-permission deficiency; it does not certify every future -write. Preserve ordinary authorization and write recovery. Do not create test -releases, upload test assets, or request extra user/admin token scopes. - -Library delivery performs cheap request/source/note checks, then credential -preflight before managed builds or full artifact size/digest verification. -Ordinary CLI delivery resolves credentials as today and relies on this automatic -library preflight. Reading a prebuilt receipt can remain a cheap earlier step. -After preflight succeeds, full local content verification still precedes every -hosted release/asset write. This deliberately permits the earlier read-only -credential probe before local artifact verification. - -For binary `ci-deliver`, load the verified tagged profile and resolve credentials, -invoke the standalone facade preflight, then prepare application dependencies. -Subsequent `Deliver` performs its normal preflight before building; that later -check is fresh after network-capable dependency preparation. Page-only CI has -no application dependency preparation and can rely on `Deliver` alone. No -long-lived validation cache or public preflight-bypass setting is needed. - -Automatic preflight failure leaves delivery `not_attempted`, including service -unavailability; unknown-write outcomes remain reserved for unresolved writes. -CLI missing/invalid credentials, established insufficient permissions, and -inaccessible destinations return status 2. Unavailable verification and -cancellation return status 1 with their actual causes. Existing local profile -errors keep status 2. Document these earlier failures and classifications as -part of the next minor release's operational compatibility impact. - -`check` and standalone `build` acquire no credentials or online preflight. -`publish` retains Git's native SSH/HTTPS authentication and remote guards; it -does not invoke hosted delivery preflight or require a forge token. Consumers -can explicitly call `PreflightDelivery` before their own preparation or source -publication without making that a new mandatory publication gate. Application -dependency credentials stay with the existing Go/Git preparation mechanisms. - -### Internal ownership and dependencies - -| Owner | Responsibility after the sprint | -| --- | --- | -| Root facade | Public destination and callable preflight, explicit selection, concrete backend construction, request/result/error conversion. | -| `internal/forge` (new) | Backend interface, preflight classifications, observed release/asset values, opaque handles, neutral failures, and write evidence. | -| `internal/artifact` | Existing build identity helpers plus expected local assets, manifests, declaration validation, and local size/digest verification. | -| `internal/delivery` | Automatic preflight coordination, local source/build coordination, hosted tag comparison, page matching/creation recovery, remote content comparison, upload sequencing/recovery, observations, and final outcomes. | -| `internal/gitea` | Destination rules, authenticated preflight probes and permission evidence, HTTP operations, codecs, protocol acknowledgments, authentication and redirects, multipart streaming, and protocol-to-neutral failure conversion. | -| CLI | Profile adaptation, token loading, receipts, preparation, presentation, and exit statuses. | - -Both delivery and Gitea depend on `internal/forge`; that contract imports neither -of them. Artifact mechanics stay independent of Gitea and delivery. The generic -workflow receives a configured backend rather than destination credentials. -The facade can import Gitea to assemble the operation. The runtime facade keeps -its existing small dependency graph. - -Use one internal `Backend` interface with these logical operations. The names -below define the intended seam; small private naming adjustments are routine -implementation choices. - -| Operation | Inputs and outputs | -| --- | --- | -| `Preflight` | Context; checks the backend's configured credentials/repository through read-only requests and returns a classified preflight error or nil. | -| `TagCommit` | Context and full tag; returns observed commit or a classified error. | -| `FindRelease` | Context and full tag; returns logical release data or a classified not-found/error. | -| `CreateRelease` | Context and requested tag, commit, title, body, draft/prerelease values; returns observed release, write evidence, and error. | -| `ListAssets` | Context and opaque release handle; returns logical remote assets or error. | -| `OpenAsset` | Context and opaque asset handle; returns `io.ReadCloser` or error. | -| `UploadAsset` | Context, opaque release handle, filename, and `io.Reader`; returns write evidence and error. | - -Logical release data contains the URL, full tag, title, body, draft/prerelease -flags, and an opaque handle. Remote asset data contains filename, reported size, -and an opaque handle. Use internal `any` handle fields with private concrete -Gitea handle values. Only the creating adapter interprets them. Keep numeric IDs -and download URLs in those private values; the shared workflow performs no -integer-ID checks or URL construction. - -Write evidence carries an `Attempted` flag and a disposition: not attempted, -accepted, rejected, conflict, or uncertain. Accepted means the adapter received -and decoded the required protocol acknowledgment; shared release-content checks -still apply. A known rejection differs from an uncertain outcome. Carry this -evidence even with an error, so successful reconciliation can retain earlier -write history. Record an attempted write when it is handed to the transport, -consistent with the existing possibly-successful-request handling; this is not -a claim that the server received it. - -Neutral failure kinds cover the existing invalid, not-found, unauthorized, -conflict, malformed, unverifiable, transport, rejected, and unknown distinctions. -Retain underlying causes through wrapping/`Unwrap` or `errors.Join`. Gitea HTTP -status and operation details belong to the adapter's diagnostic cause. Shared -recovery uses the neutral kind and write evidence, with no HTTP status branches -or Gitea error assertions. Credentials stay out of errors and results. - -The shared caller closes streams returned by `OpenAsset`, including on read -failure. For uploads it opens/closes the local file or supplies an inline -manifest reader; the adapter borrows that reader for the call and finishes its -multipart writer/pipe work before returning. Retain cancellation propagation -and cleanup rather than introducing detached recovery contexts, timeouts, or -buffering of complete binaries. - -## Stage 1: Establish the internal backend contract - -Status: complete. - -1. Add `internal/forge` with the interface and logical values specified above. - Define preflight classifications, neutral failures, and write-evidence types - with comments about not-found, accepted acknowledgments, uncertain writes, - opaque handles, and - stream ownership. Use ordinary Go values and error wrapping. -2. Keep this package concerned with the delivery seam; production backend - selection belongs to the facade. Avoid a registry, capability negotiation, - public interfaces, or additional forge implementations in this work. -3. Identify the existing Gitea policy types replaced by later stages: - `Request`, `Result`, `ExpectedAsset`, `Inspection`, asset identities/states, - upload results, and failure assertions in `internal/delivery`. Assign their - replacement ownership as above rather than reproducing the whole Gitea - model under new names. - -Result: an internal contract implemented independently of the public facade and -delivery policy. Type-check the new package when available. Test nontrivial -error behavior where useful; recovery is tested through the shared workflow. - -## Stage 2: Move local asset and manifest mechanics - -Status: complete. - -1. Move `ExpectedAsset` and the generic work in `BuildManifest`, - `validateExpectedAssets`, and local verification from `internal/gitea` to - `internal/artifact`. Keep the filename, SHA-256, size, path, and inline-content - representation and existing accepted inputs. -2. Retain `artifact.ManifestFilename`, sorted complete manifest generation, its - byte format, duplicate/name/digest validation, and the empty-set result that - produces no manifest. -3. Adapt `internal/delivery/assets.go` and build validation to shared types. - Keep target/name correspondence, captured module/tag/commit binding, - `SourceVerified`, regular-file checks, and prebuilt output ownership. Keep - build results and the private receipt format unchanged. -4. Move/consolidate manifest and local-content tests with these responsibilities. - Preserve rejection of same-size local tampering. - -Result: local assets are declared and verified independently of Gitea. Retain -one full local size/digest preflight before hosted writes and reuse it for -uploads; subsequent transport reads stream content without a second full -preflight. This preserves existing efficiency/integrity choices rather than -adding a guarantee against concurrent file replacement. - -## Stage 3: Extract Gitea read operations - -Status: complete. - -1. Adapt the Gitea client into a backend configured with its private destination - and optional HTTP client. Implement `TagCommit`, `FindRelease`, `ListAssets`, - and `OpenAsset` using the existing HTTP helpers and routes. -2. Keep Gitea address/token rules, full-tag escaping, JSON handling, attachment - metadata validation, and HTTP status interpretation here. Map a release - lookup's 404 to neutral not-found; a missing hosted tag remains a failure to - verify source. Preserve operation-specific checks rather than applying - create-response requirements to every read. -3. Split `downloadHash` so the adapter opens the download stream and generic - delivery hashes it. Keep service-origin token forwarding, cross-origin - downloads without that token, redirect traversal, and response cleanup with - Gitea. Preserve current listing behavior; extracting the boundary does not - introduce pagination or a new response-completeness guarantee. -4. Retain local HTTP tests for routes, authentication, escaped tags, metadata - decoding, malformed lists, redirects, and unreadable assets. Adapt tests to - the read operations; shared comparison tests move in Stage 7. - -Result: backend reads expose observations and streams. Content equality, tag -comparison, page acceptance, and delivery outcomes belong to shared policy. - -## Stage 4: Implement read-only Gitea credential preflight - -Status: complete. - -1. Implement backend `Preflight` with local destination/token validation and - authenticated repository-scoped reads. Perform no source/tag checks here so - the same probe works before a release tag exists. -2. Resolve the configured repository with `GET /repos/{owner}/{repo}`, then - use its ID with the authenticated `GET /repositories/{id}` route. Validate - the required repository identity and successful response shape. The second - route requires authentication and repository scope, so public anonymous - repository access alone cannot satisfy preflight. Keep headers, IDs, and - route details within Gitea. See the upstream - [route definitions](https://github.com/go-gitea/gitea/blob/release/v1.26/routers/api/v1/api.go). -3. Interpret reliable release/asset permission evidence when available. Gitea's - basic `permissions.push` describes code pushes, and a successful repository - GET does not establish `write:repository` token scope. Treat unexposed - release-write/token-scope evidence as unverified. Do not equate absent - metadata with denial or add `/user` probes requiring unrelated user scope. - The upstream [repository model](https://github.com/go-gitea/gitea/blob/release/v1.26/modules/structs/repo.go) - and [scope documentation](https://docs.gitea.com/development/api-usage/) - describe those distinctions. Keep later write authorization authoritative. -4. Classify missing/blank credentials locally, server-rejected authentication, - permission denial, inaccessible/hidden destinations, and unavailable or - malformed verification responses. Preserve cancellation and useful HTTP - causes without including the token. Unsupported/malformed destination - settings retain actionable local configuration errors. -5. Use a local HTTP fixture to cover accepted repository-scoped credentials, - invalid tokens, denied/missing repositories, service/decoding failure, and - cancellation. Demonstrate that anonymous public access cannot pass the - authenticated step and that unexposed write scopes are not claimed as - verified or treated as a new failure. Assert read-only behavior at this seam. - -Result: the backend checks authentication and repository access before costly -delivery work and accurately represents the limits of available permission -evidence. It uses no extra account/admin credentials or destructive probe. - -## Stage 5: Extract Gitea write operations and evidence - -Status: complete. - -1. Implement `CreateRelease` from the current release POST and `UploadAsset` - from multipart upload. Keep explicit target commit, stable flags, full note, - query encoding, attachment field, and token header behavior. Protocol codecs - and private IDs remain here. -2. Populate attempted-write evidence on each path. Translate the existing - `shouldReconcileWrite` triggers into neutral dispositions: HTTP conflict is - conflict; server errors, transport interruption, cancellation/deadline, and - malformed acknowledgments are uncertain; definitive other rejections remain - rejected. Preserve causes and HTTP details. Local failures before transport - are not attempted. Shared policy decides how to recover. -3. Validate Gitea acknowledgments here, including positive IDs and the uploaded - asset's selected filename. Leave requested release tag/title/body/stability - comparison to Stage 6. A decoded acknowledgment that fails shared comparison - can still require reconciliation. -4. Accept a borrowed content reader for upload. Move file/inline-reader selection - to shared code and preserve multipart streaming, pipe closure, writer - completion, response cleanup, and context cancellation without an extra - content pass. -5. Keep HTTP tests for request bodies, uploads, malformed acknowledgments, known - rejections, ambiguous writes, and cancellation at this boundary. Recovery - orchestration tests move with policy in Stages 6 and 7. - -Result: individual writes return evidence. Follow-up lookups, content comparison, -and continuation decisions belong to shared delivery. Remove the adapter's old -orchestration as its callers move rather than retaining a parallel policy path. - -## Stage 6: Move hosted page policy and recovery - -Status: complete. - -1. Add shared page coordination in `internal/delivery` using `forge.Backend`. - After credential preflight and local source/build verification, obtain the - hosted full tag's commit and compare it with the captured commit before - page writes. Find the - release, accepting the same full tag/body/stable non-draft content and - retaining an existing title. Only a missing release triggers create. -2. Supply the full note and H1 title to `CreateRelease`. Apply shared comparison - to a successful create response, including its requested title. Reconcile - an inconsistent response as the existing client does. -3. Move create recovery out of `EnsureRelease`. Preserve the current single - follow-up lookup for a conflict or uncertain create: - - | Follow-up observation | Result | - | --- | --- | - | Matching existing page | Accept; retain the earlier write attempt. | - | Different page | Conflict. | - | Missing after conflict | Conflict. | - | Missing after uncertain write | Unknown. | - | Result cannot be established | Unknown; retain write and inspection causes. | - -4. Preserve known/read-only failures as distinct from unknown writes. Retain - resolved source identity, confirmed URL, and write history. A later asset - inspection failure after confirmed page-write progress reports partial - progress. Page-only delivery succeeds without listing/uploading assets or - creating a manifest. -5. Move page policy cases into shared tests using a small controlled backend: - matching content, conflicts, missing/wrong tag, ambiguous create recovery, - and recovered-create history. Keep wire assertions in adapter tests. - -Result: page behavior runs through the seam. Current Gitea recovery remains -unchanged; this does not promise a fixed HTTP request count for future logical -backend methods. - -## Stage 7: Move remote asset comparison and upload recovery - -Status: complete. - -1. Move inspections, asset states/identities, upload aggregation, ordering, - and recovery into `internal/delivery`. Use `ListAssets` and `OpenAsset` for - streamed SHA-256 comparison. Retain filename grouping, duplicate-name - conflicts, digest comparison, unrelated asset preservation, and unverifiable - content classification. Reported size alone remains insufficient for equality. -2. Retain complete-manifest comparison, acceptance of a matching manifest when - binaries are missing, and rejection of a subset/different manifest. Add no - independent remote-size acceptance rule. -3. Preflight declarations before uploads. Matching assets stay matching; - conflicts stop with conflict; unverifiable reads stop with not-attempted - asset work, upgraded to partial on confirmed page-write progress. Whole-list - read failures retain existing uncertain asset observations without becoming - unknown-write outcomes. -4. Upload only missing assets in filename order with `SHA256SUMS` last. Open - content through the artifact representation and close it after the borrowed - upload call. Reuse Stage 2 verification. Acknowledged uploads move from pending - to uploaded. -5. On conflict or uncertain upload, perform the existing single list/content - reconciliation for that asset. Matching bytes become matching; differing - bytes become conflicting; missing after conflict remains conflict; missing - after uncertain write or an unreadable result becomes unknown/uncertain. - Stop on unresolved/conflicting outcomes and retain later known-missing assets - as pending. Keep reconciliation free of repeated POSTs. -6. Preserve disjoint current observations, earlier successful assets, and public - states. Definitive upload failure retains partial progress; early failures - can leave declarations unreported. A new invocation re-inspects remote state - and resumes only missing work. -7. Move comparison, manifest, partial retry, and category tests here. Use a small - backend double for controlled failures, with representative assembled HTTP - coverage instead of duplicating every policy case at both layers. - -Result: shared code owns asset policy/recovery. Remove the former Gitea -`InspectAssets`, `UploadMissing`, and `UploadVerifiedMissing` entry points when -their callers/tests move; keep transport helpers needed by the backend. These -internal entry points have no public compatibility obligation. - -## Stage 8: Connect the neutral facade and CLI - -Status: complete. - -1. Implement the public declarations/conversions in Selected design. Replace - current `GiteaDestination` uses. The private facade factory switches on the - explicit forge and constructs Gitea; invalid selection returns the described - early result. -2. Implement `PreflightDelivery` and `DeliveryPreflightError` conversions using - that same backend factory and preflight implementation. Support destination - checking without a repository checkout or published tag. Preserve causes - with `Unwrap` and keep all exported named types facade-owned. -3. Change the internal delivery request to receive the backend. Replace Gitea - asset/error references in request/result conversion and build validation. - Pass delivery's single captured module/source identity through its existing - workflow. Retain local source rechecks, temporary output cleanup, and - prebuilt output ownership. Invoke backend preflight after cheap local checks - and before managed builds/full artifact hashing. Convert its failures to - the facade error and retain `not_attempted` delivery state. -4. Add `Forge` to CLI delivery settings with JSON key `forge`. Validate selection - during delivery adaptation before credentials or subsequent delivery work, - returning status 2 for invalid selection. Map the facade's supported value; - preserve strict decoding and non-delivery behavior. -5. Update `deliveryDestination`, `deliverWithDestination`, ordinary delivery, - and CI delivery. Retain tagged profile loading, module/tag checks, dependency - preparation, receipt binding, and output/exit classification. Keep token - resolution in the CLI with current precedence and Gitea environment default. - For binary CI, invoke `PreflightDelivery` after tagged profile/credential - loading and before dependency preparation. Ordinary and page-only delivery - rely on automatic library preflight; subsequent binary CI delivery rechecks - after dependency preparation. Apply the preflight exit classifications above. -6. Add `"forge": "gitea"` to ReleaseKit's profile, maintained delivery profiles, - examples, and active test fixtures. Check/build-only profiles can keep - omitting delivery. Keep token names, file paths, and Woodpecker secret mappings. - -Result: both interfaces select Gitea explicitly through a neutral destination -and share generic orchestration and automatic credential preflight. CI checkout -preparation retains its earlier position because it establishes which tagged -profile is authoritative; credential verification precedes application downloads. - -## Stage 9: Complete regression coverage and dependency checks - -Status: complete. - -1. Finish adapting facade, CLI, HTTP fixture, and benchmark callers. Retain - root/nested/major-module identities, page-only delivery, managed/prebuilt - binaries, tagged retries, source checks, partial/conflict/unknown reporting, - and cleanup coverage. -2. Add focused API/CLI selection cases: explicit Gitea succeeds; omitted, blank, - and unsupported values fail; the URL does not infer a backend. For CLI errors, - assert status/semantic diagnostics and that token access, dependency - preparation, and hosted work were not reached. Reuse existing non-delivery - tests to show those commands still need neither selection nor credentials. -3. Cover automatic and standalone preflight with controlled failures: library - and ordinary CLI failures precede builds/full artifact hashing and writes; - binary CI failures precede dependency downloads; page-only CI still checks - credentials; standalone checks need no checkout or tag. Assert typed error - kinds, causes, CLI status, and `not_attempted` even on unavailable service. - Verify unavailable permission metadata alone is not failure and no preflight - path performs test writes. Update affected "before hosted requests" fixtures - to permit credential reads while retaining assertions that local verification - precedes writes. Keep `check`, `build`, and source `publish` independent of - hosted credential preflight and preserve the existing native Git guards. -4. Use controlled backend tests to demonstrate shared page/asset policy without - Gitea imports. Consolidate obsolete client-policy tests rather than retaining - wrappers. Keep representative real-Gitea HTTP assembly tests for page - creation and binary delivery/retry, exercising conversions and opaque handles. -5. Extend `internal/apicheck` with a focused check that shared delivery, forge - contracts, and artifact mechanics have no production dependency path to - `internal/gitea`. Preserve export ownership/runtime checks, keeping new - delivery-related packages out of the runtime dependency graph through the - existing checking mechanism. Add a small violation fixture following current - apicheck tests. -6. Review stream/error paths for cancellation identity, reader closure, - multipart worker completion, and causes. Reuse cancellation tests; add a - narrow regression for a gap introduced by moved ownership. Keep the default - suite offline and credential-free. - -Result: selected requirements have meaningful coverage and clear test owners. -Run relevant package tests once assembled; Stage 10 owns the final full checks. - -## Stage 10: Document the cutover and verify the sprint - -Status: complete. - -1. Add the next sequential ADR (currently `0006`) using the documentation - policy's lightweight Nygard format. Record the shared-policy/backend boundary, - neutral facade-owned destination, explicit selection, credential preflight - ownership and evidence limits, alternatives, and consequences. Keep progress - in these roadmaps. -2. Update GoDoc, `docs/config.md`, `docs/cli.md`, and - `docs/consumers/delivery.md` for the neutral destination, selection, automatic - preflight, callable preflight, typed failures, and CLI exit classifications. - Update current README/runbook/example snippets where affected. Keep Gitea - wire/authentication details in `docs/integrations/gitea.md`; describe shared - coordination in internal documentation linked from `docs/internal/overview.md`. - Align architecture policy wording with the implemented boundary while - preserving its broader constraints. - Document the authenticated repository probe and unverified write scopes in - the Gitea guide. Describe standalone preflight as a caller option; preserve - existing source-publication completion and credential requirements. -3. Write `docs/releases/v0.6.0.md`, the next minor release note, explaining the - features, compatibility impact, and public type/profile migration. Follow - the [release policy](../policy/release.md) and - [runbook](../release.md) for note/footer requirements. Explain the unchanged - Gitea token default and profile updates before tagging: CI reads the tagged profile. - Include fail-fast credential checks, additional read-only requests, failure - classifications, and the limited meaning of successful preflight. Explain - that local artifact verification now follows credential reads and still - precedes hosted writes. - Preserve historical notes. Prepare documentation here; tagging, pushing, - and publication remain separate release work. -4. Check maintained JSON/Go examples, profile-generating fixtures, and Woodpecker - tests for explicit selection. Search active code/docs for obsolete public - type uses and implicit-selection instructions, distinguishing history and - migration examples. Verify documentation links, fragments, and whitespace. -5. Run `go test ./...` and `go run ./internal/apicheck` from the repository root - with prepared local dependencies and offline module/toolchain access. These - include representative CLI/consumer workflows. Reuse successful checks for - unchanged content; fix failures and rerun affected checks. No live forge, - real token, CI run, or consumer pilot is required for sprint completion. -6. Review the final diff and dependency flow against every feature target and - stage. Verify generic policy has no Gitea dependency; only the adapter - interprets its handles/statuses; the assembled workflow retains recovery - outcomes; current delivery profiles explicitly select Gitea; migration - instructions match the API/configuration; credential failures stop before - costly delivery work; source publication remains independent; local content - verification is reused before writes; and comparison/transport remain streamed. -7. Mark stages complete as verified. Commit completed work under the agent - policy, report validation and the final commit, and leave a clean workspace. - Retain accurate completed plans for later release-preparation cleanup. - -Result: the initial refactor, migration guidance, and offline verification are -complete. All stages proceed to this end state within the sprint. diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md deleted file mode 100644 index b9519f7..0000000 --- a/docs/roadmap/remediation.md +++ /dev/null @@ -1,465 +0,0 @@ -# Remediation: forge boundary and credential preflight review - -Status: complete; all eight issues are addressed, with completion recorded 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 review findings and their remediation. Findings describe -the reviewed revisions; completion notes record their corrections. - -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: complete. - -**Completed:** Replaced repeated slice removals with pending-map updates and a -single deferred compaction pass on every return path. Category bookkeeping is -now expected O(`n`) with O(`n`) metadata, preserving pending observation order. -Added an early-return regression with out-of-order observations for definitive -and uncertain failures. Delivery/Gitea tests and focused facade/CLI recovery -tests passed. - -**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: complete. - -**Completed:** Declaration validation now receives an unsorted collection. -Classification collects only missing declarations, and that owned worklist is -sorted in place once after the no-upload exits. Scheduling now takes -O(`n + k log k`) metadata work while retaining filename order and the manifest -last. Existing delivery/artifact tests passed, including manifest matching, -upload ordering, conflict/unverifiable stops, and partial recovery. - -**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: complete. - -**Completed:** Joined per-asset inspection causes with filename context in -unverifiable and conflict summaries, retaining conflict precedence and existing -states/categories. Added focused multiple-cause coverage and public API -regressions for cancellation and truncated downloads. Both regressions detect -the original defect and pass with the fix. Full offline tests, focused race -tests, and the API/dependency check passed; final verification is recorded below. - -**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. - -### Remediation verification - -All eight issues were addressed sequentially and marked complete with their -implementation or documentation changes. Final checks used Go 1.26.10 on -linux/amd64 with `GOTOOLCHAIN=local`, `GOPROXY=off`, and `GOSUMDB=off`: - -- `go test ./...` passed. -- `go run ./internal/apicheck` passed. -- `go test -race ./internal/delivery ./internal/gitea` passed. -- Focused public API and CLI regressions also passed with the race detector: - invalid destinations, CI module/tag mismatch, phase-specific statuses, and - asset-inspection cause preservation. -- A temporary source overlay verified that the new aggregation/public API - cause-preservation regressions fail against the previous implementation. -- Local links and heading fragments across changed Markdown files resolved; - whitespace checks passed. No benchmark measurements were needed.