stack/docs/plans/reviews/2026-09-10_m20-refresh-final-verdict.md

M20-I2-FINAL-FILBERT-1 (#1500) — final whole-increment verdict

Reviewer: Filbert (independent; prior stage reviewer — this verdict relies on fresh whole-increment evidence). Frozen candidate /tmp/m20-refresh-final-review-xn34lpeb at manifest SHA-256 11255dd4d76adb1762af0b5dd0d9eac114f48d5775594dd29ba554e4a96d82e6, 21 files verified before and after; snapshot unchanged. All execution in a disposable baseline-plus-overlay tree. No real Pi, network, credentials, Docker, services, timers, commits, or shared-log edits.

Verdict: APPROVED for the fixture-only increment

Independent receipts

• Combined suites: node --test packages/mosaic/tests/ scripts/test-darkwing-launch.mjs scripts/test-rocko-launch.mjs — 74 tests, 74 pass, 0 fail, exit 0, reproducing the author's /tmp/m20-refresh-integrated.log claim. The failure-mode child's FIXTURE_PRIVATE_DIAGNOSTIC stderr appears nowhere in the output.
• My probes: rotated output carries fresh markers and expiry with caller input untouched; near-expiry OAuth auto-refreshes inside the generation transaction without an explicit option; the rotation commits to the in-memory store (visible to a later transaction); a failed refresh refuses with refresh-failed and leaves the store's prior rotation intact. Zero /tmp/mosaic-*-fixture-* residue after the full run and the probes.
• Delta discipline: versus the stage-23 snapshot, only materialize-fixture.mjs (refresh integration, after-publish fault), materialize.test.mjs (seven new tests), README.md (fixture/refresh documentation and limits), and the new refresh-fixture.mjs/refresh.test.mjs differ; execution.mjs, fixture-store.mjs, index.mjs, and the package manifest are byte-identical to the stage-reviewed bytes. The refresh runner is internal — not exported from the package index.

Findings against the requested attention points

1. Fixed-only runner: the child is process.execPath with a fixed embedded program and fixed argv (auth check --provider <p>), shell:false; executable/env injection attempts refuse before spawning and before burning the execution claim.
2. Isolation: isolated PI_CODING_AGENT_DIR/HOME/cwd per invocation from an env literal of exactly three variables — no inherited environment, no PATH, so no user-executable resolution; the fake self-checks its env purity and exits on any extra variable. This matches Rocko's corrected static findings (hash 6943eef5… verified): PI_CODING_AGENT_DIR is the sole credential isolation lever, auth dispatch precedes cwd/settings/extension/trust setup, and the lock stays inside the agent dir.
3. Output/error privacy: child stdio discarded, never buffered or parsed; the parent validates only the rewritten auth.json under O_NOFOLLOW, uid/mode/4 KiB bounds, fixed invalid-refresh-output code with no parser excerpts; all diagnostics are fixed codes in the marker language.
4. Child cleanup/timeout: SIGKILL on timeout with close-event resolution and root removal in finally; refresh-timeout/refresh-failed refusals; no orphaned temp trees observed.
5. Lock scope and rollback: refresh runs inside the store transaction; rotation commits only on successful generation (my R3/R4); same-account contention refuses credential-busy while an independent account's generation proceeds; failed refresh preserves prior generation bytes and store state, and the same execution ID cannot retry.
6. Uncertainty regression (added after my early review): after-publish fault records an uncertain receipt, retains the complete published generation, empties pending, rolls back the in-memory draft, and refuses same-ID replay. This closes the gap I flagged; the README states the residual honestly (store commit and publication are not crash-atomic together; no external issuer exists to reconcile in this simulator).
7. Fork/enrollment/native-ceiling semantics: unchanged stage-reviewed execution.mjs; its tests all still pass in the combined run.
8. README claims: accurate and deliberately bounded — in-process locks, same-UID/hardlink residual, static-only 0.85.1 evidence, and the explicit warning that the model-catalog network flag does not suppress OAuth refresh networking and that real Pi must never be substituted, even with synthetic OAuth.
9. Rocko corrected evidence: the correction receipt is honest (stale pin claim, placeholder path, over-broad no-network claim — each fixed with the original preserved); the corrected bounded claims are the ones this increment relies on.

Limits and conditions

• This approves the fixture-only increment exactly as frozen. Committing/pushing the reviewed bytes is the authorized next step; owner acceptance follows per charter.
• Nothing here is a production refresh protocol: a real backend needs a separate charter covering reconciliation, distributed locking, and crash-atomicity. Never substitute real Pi in these tests.
• Residuals already on record carry forward: in-process locks, same-UID tampering/hardlink provenance, and the correction verdict's design-only items.