| 1 | # Astra re-review r2: settings-5-surfaces |
| 2 | |
| 3 | Range 561c7dfb..40e7782e. N1 fixed in 51790645 (wave 2). |
| 4 | |
| 5 | Verdict: REJECT |
| 6 | |
| 7 | Reviewed `561c7dfb..40e7782e` against rev 11 and R18–R21 by inspection. No files modified. **Go cannot run here; I did not run tests.** I inspected the regression tests and implementer’s reported results. |
| 8 | |
| 9 | ## Round-1 findings |
| 10 | |
| 11 | 1. **F1 — NOT ADDRESSED.** The original stale-snapshot join is fixed, but the new producer attribution can mislabel an older tenant’s decision; see N1 below. The console correctly strips tenantless reports to `Enforced` and rejects mismatches at [apiinstances.go:387](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/settings-5-surfaces/internal/console/apiinstances.go:387). The additive fields survive the private snapshot copy at [snapshot.go:1076](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/settings-5-surfaces/internal/status/snapshot.go:1076); public fleet output excludes them. Platform precedence remains intact. An `Enforced`-only view exposes no update details through the list, CLI reason, or page. |
| 12 | |
| 13 | 2. **F2 — ADDRESSED.** The fast-track owner 404 now precedes `allowWrite` at [browserupdates.go:254](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/settings-5-surfaces/internal/console/browserupdates.go:254). `requireFreshAuth` remains last at line 302. The exhausted-budget regression compares the refusal with an unknown host’s response and checks for no install request. |
| 14 | |
| 15 | 3. **F3 — ADDRESSED.** [browserupdates.go:99](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/settings-5-surfaces/internal/console/browserupdates.go:99) preserves the explicit override while calculating the inherited note separately. [app_instance.html:48](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/settings-5-surfaces/internal/console/templates/app_instance.html:48) retains both input and `was_major` semantics. Coverage includes clearing an override with enforcement unavailable. |
| 16 | |
| 17 | 4. **F4 — ADDRESSED.** The named-unit check precedes kind exclusion at [browserupdates.go:82](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/settings-5-surfaces/internal/console/browserupdates.go:82). The service-kind regression checks the explanation and absence of forms; ordinary services remain excluded. |
| 18 | |
| 19 | 5. **F5 — ADDRESSED.** The lead-in enters the draft inventory at [words_surfaces.go:77](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/settings-5-surfaces/internal/updatewords/words_surfaces.go:77), is pinned by [words_docs_test.go:19](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/settings-5-surfaces/internal/updatewords/words_docs_test.go:19), and maps to “Docs and roll notes.” |
| 20 | |
| 21 | 6. **F6 — ADDRESSED.** Both commands use the shared, em-dash-free draft at [instances_updates.go:106](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/settings-5-surfaces/instances_updates.go:106) and line 208. Export mapping and help-output regression coverage are present; the separately pinned app-set literal is unchanged. |
| 22 | |
| 23 | ## New findings in the fix diff |
| 24 | |
| 25 | ### N1. [P1] Current route attribution can disclose a previous tenant’s stored decision |
| 26 | |
| 27 | **Where:** [internal/host/updatesreport.go:66](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/settings-5-surfaces/internal/host/updatesreport.go:66), consumed by [internal/console/apiinstances.go:643](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/settings-5-surfaces/internal/console/apiinstances.go:643). |
| 28 | |
| 29 | **Scenario:** A hostname moves from tenant A to tenant B on the same box. Before root replaces its previous decision, `updatesReport` reads A’s hostname-keyed consumed file, labels the entry with B’s current route account/app, and copies A’s desired build, reason, detail, baking build and completion information into it. |
| 30 | |
| 31 | [Consumed has no account/app identity](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/settings-5-surfaces/internal/release/buildstate.go:87); `ReadConsumed` validates only schema and hostname. With claims disabled, [route retirement deletes the registry row](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/settings-5-surfaces/internal/provision/claimassembly.go:40), leaving that decision file. |
| 32 | |
| 33 | The console consequently accepts A’s details as B’s. This creates a new disclosure path for rollout-free fleet reports, which the pre-fix join refused. The regression fixtures assume attribution is already correct and do not exercise this transition. |
| 34 | |
| 35 | **Fix:** Bind persisted decisions to their account/app when written, and suppress decision-derived report fields unless that identity matches the route. Treat legacy unattributed decisions conservatively. Add a regression that retains A’s consumed file, replaces the route with B’s, generates the actual report, and checks list/page disclosure. |
| 36 | |
| 37 | ## Out of scope |
| 38 | |
| 39 | - Paul’s plan-5 copy approval remains pending. The missing-record-only `TestApprovedCopyIsApplied` failure is expected and does not affect this verdict. |
| 40 | - Full execution gates and the reported sandbox-blocked host socket tests remain outstanding; this review does not independently validate their results. |
amadan