Record algorithmic delivery audit findings

docs/roadmap/remediation.md

@@ -15,6 +15,11 @@ 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;
@@ -225,6 +230,99 @@ validation behavior across the two supported CI paths.
   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:
@@ -261,3 +359,37 @@ 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.