| 1 | # Astra re-review r3: settings-5-surfaces |
| 2 | |
| 3 | Range 40e7782e..51790645. |
| 4 | |
| 5 | Verdict: ACCEPT |
| 6 | |
| 7 | Reviewed `40e7782e..51790645` against rev 11 and rulings R22–R23 by inspection. No files modified. **Go cannot run here; I did not run tests or builds.** I inspected the tests and implementer’s reported evidence. |
| 8 | |
| 9 | ## Round-2 finding |
| 10 | |
| 11 | **N1 — ADDRESSED within the ruled scope.** |
| 12 | |
| 13 | - **Persisted attribution:** [buildstate.go:113](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/settings-5-surfaces/internal/release/buildstate.go:113) adds optional Account/App fields without changing schema validation or legacy decoding. `DecidedFor` requires both identities to be non-empty and match the route. |
| 14 | - **Every production write:** [updatetarget.go:274](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/settings-5-surfaces/internal/release/updatetarget.go:274) stamps identity after copying any previous record. Both production publication paths—`adopt.go:2197` and `adopt.go:2229`—use this constructor; the fence preserves those fields. |
| 15 | - **Updates report:** [updatesreport.go:61](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/settings-5-surfaces/internal/host/updatesreport.go:61) rejects legacy or mismatched attribution. Its early exit suppresses Desired, Reason, Detail, Baking, Completed, CompletedFailed and Pending, including the build-state watermark fallback. Major remains sourced solely from the current route’s settings, as R22 permits. |
| 16 | - **Control socket:** [updatesreport.go:130](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/settings-5-surfaces/internal/host/updatesreport.go:130) applies the same identity check before adding Desired/Reason, satisfying R23. |
| 17 | - **Regression coverage:** [updatesreport_test.go:211](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/settings-5-surfaces/internal/host/updatesreport_test.go:211) exercises reassignment, legacy attribution, matching attribution and partial/mismatched identities. [adoptsettings_test.go:951](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/settings-5-surfaces/internal/release/adoptsettings_test.go:951) exercises both root write paths. |
| 18 | |
| 19 | ## New findings in the fix diff |
| 20 | |
| 21 | None at [P1], [P2] or [P3]. |
| 22 | |
| 23 | Root’s install decisions and request bookkeeping are unchanged. No platform-specific dependencies or build constraints were introduced; `release` already depends on `registry`. Inspection found no cross-platform build regression. The implementer reports successful Darwin, Windows and Linux-arm64 builds; I did not independently execute them. |
| 24 | |
| 25 | ## Out of scope |
| 26 | |
| 27 | The parked plan-4 notes are **correctly outside this fix’s scope**: |
| 28 | |
| 29 | - Root and the held-wake logic still trust previous-tenant completion/resolution data. `NextConsumed` can retain that bookkeeping when stamping the new identity. This remains a substantive follow-up, explicitly excluded by R22; acceptance does not certify its correctness. |
| 30 | - In-place registry Upsert preserves empty settings/build-state columns at [registry.go:1162](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/settings-5-surfaces/internal/registry/registry.go:1162). That behavior predates this diff. |
| 31 | - Copy approval and remaining execution gates, including sandbox-blocked socket tests, remain separate from this inspection verdict. |
amadan