amadan
settings-1: Astra review of the main merge (ACCEPT at c385f2eb)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 file changed,
+25
−0
docs/superpowers/plans/CODEX-FINDINGS-SETTINGS-IMPL-1-MERGE.md+25 −0
CI passed — run details
diff --git a/docs/superpowers/plans/CODEX-FINDINGS-SETTINGS-IMPL-1-MERGE.md b/docs/superpowers/plans/CODEX-FINDINGS-SETTINGS-IMPL-1-MERGE.md| new file mode 100644 |
| index 000000000..9aa9a3bbf |
| --- /dev/null |
| +++ b/docs/superpowers/plans/CODEX-FINDINGS-SETTINGS-IMPL-1-MERGE.md |
| @@ -0,0 +1,25 @@ |
| +ACCEPT |
| + |
| +- **Critical:** None found. |
| +- **Important:** None found. |
| +- **Minor:** No new findings in this scoped merge review. |
| + |
| +The composition preserves both guarantees. `internal/release/adopt.go:1899` checks the canonical manifest before staging and passes that same manifest and raw bytes into `stageFrom`. At `:1833–1847`, mismatched copies are refused when referenced or when references are unknown. Unreferenced copies are downloaded and verified against the checked manifest. The no-digest path preserves main’s staging behavior. |
| + |
| +Both `WithDigestRatchet` and `WithPinSource` remain. The `verified` map’s change to artifact SHA values is correct: `adopt.go:1815` prevents a digest-bound host from trusting verification against another manifest while preserving main’s unbound reuse behavior. |
| + |
| +Verdicts on the five concerns: |
| + |
| +1. **Unreadable manifest with a digest — acceptable.** `internal/release/adopt.go:1899` fails closed before downloading or restaging. Adoption can stall during a bucket outage, but silently using an unchecked staged copy would weaken the required guarantee. No fix needed. |
| + |
| +2. **“Version name reused” refusal — acceptable.** `internal/release/adopt.go:1847` uses `ErrVersionReused` specifically after the canonical manifest passes its digest check and the referenced staged binary differs. A canonical-manifest mismatch still produces `ErrManifestDigest`. The distinction is appropriate; no fix needed. |
| + |
| +3. **Overwriting a readable sibling — acceptable.** `internal/release/adopt.go:1912` synchronizes it to the checked raw bytes after the binary matches. Readability alone cannot establish that a sibling contains the signed manifest, particularly after an older lossy re-marshal. Different signed manifests sharing one binary can replace this shared metadata, as reported; that does not defeat either reviewed guarantee. No merge fix needed. |
| + |
| +4. **Edge helper rename — correct.** `internal/edge/staticdigest_test.go:51` now defines `getBody`, avoiding the unrelated `body` at `internal/edge/genproxy_test.go:161`. The callers were updated consistently; no behavior change. |
| + |
| +5. **Held materialization remains digest-free — outside this merge’s pointer composition.** `internal/release/materializeheld.go:168` stages a move-pinned version without resolving a signed channel pointer. Its unchanged call through `ensureStaged` refuses mismatched staged copies because references are unknown. It does not establish digest-bound authorization for fresh held downloads; that remains separate work, not a regression introduced by this resolution. |
| + |
| +The report’s control-flow table matches the implementation. The new tests cover checked-manifest reuse, referenced refusal, unreferenced restaging, cross-manifest verification caching, and exact sibling bytes. |
| + |
| +Scoped `git diff --check` passes. Tests/builds were not rerun because this review is read-only; reported execution results remain independently unverified. `origin/main` has advanced to `f36718c5`, but its intervening changes do not touch the scoped files. No files changed. |
| \ No newline at end of file |