oficina / home Public

Explicit configured claude-fable-5 code review APPROVE, no blockers, frozen SHA 667dc8218dd5d04a0de72eea11badfee0eebd8fd. Actual Amadan CI green on that exact SHA (vet/fmt/build/tests/race including roster); local make test passed; six compiled security mutations killed. Evidence /tmp/oficina-roster-code-review/{final.txt,review.ndjson,ci.json}, /tmp/oficina-roster-module-gate.log and /tmp/oficina-roster-mutations/results.json. No runtime deployment or operator seeding. Canonical module resolution follows this review gate. Full final review:

APPROVE

I reviewed the frozen code at 667dc82 against the approved design and my prior design review, under the requested lenses. No blockers within the declared scope (shared protocol/record verification, consumer cache, nested-module CI inclusion). The gated posture holds: nothing here mints a producer, a route, an operator transport, or a live seed, and the Makefile now runs vet/build/test/race over the roster module as first-class gates.

Why each lens passes

• Record replay/generation persistence. VerifiedRecord is constructible only through VerifyProvisioning; the digest is over the re-marshaled canonical record, so whitespace variants of one operation hash identically. Durable receipts refuse operation reuse across sequences; equal sequence requires equal digest; lower sequence refuses — and TestRecordOperationAndRestartRollback proves an older signed record on restart is refused, which is the durable floor doing its job. Generation change requires a non-empty owner-confirmation reference; the residual "rollback after total durable loss" threat is stated in the VerifiedRecord comment exactly as my advisory 9 asked.
• Epoch/digest floors, including empty snapshots. The Snapshot flag correctly distinguishes "provisioned floor only" from "observed empty roster": a fetched members=[] at epoch E pins digest-of-empty durably, and TestEmptyRosterAndRestartEpochFloor proves both same-epoch mutation and restart refuse a conflicting body. Lower epoch refuses at publish against the durable floor, not just the record minimum.
• Staging atomicity. Pages accumulate in flight-local state; publish holds the mutex, re-checks ctx/version/record-digest/elapsed-budget, persists sealed evidence before publishing, and a save failure changes neither bytes nor lease (tested, and the partial-publish and renew-failure mutations died). A record update mid-flight cancels and version-fences; the blocked-goroutine test proves old staging cannot land.
• Challenge binding and wire shape. Fresh 32-byte challenge per refresh, verified on every page with scope (directory/consumer/team/audience), request_cursor_hash pinned to SHA256("") for page 0 by exact test vector (advisory 1 done), signature proven to cover decoded payload bytes and not the base64 text (advisory 2 done). Strict json v2 parsing rejects unknown members, duplicates, case-folding and 1.0-style numerics — all in the refusal matrices. HTTP status must agree with signed type; 409 cannot publish; a signed 409 with a foreign generation still fences (tested).
• Lease expiry under failures/restarts/clock. 30/60/5 with transit inside the 60 (Remaining()==56s test), age never renewed by any of 17 refusal variants, hard ceiling measured with the deterministic clock and the real transport deadline measured separately. The Round(0) wall-vs-monotonic comparison is the correct way to strip Go's monotonic reading — routine slew inside 2s tolerated, rollback beyond it fences, both tested at the exact boundary (advisory 3 done). Restart starts Unknown; no persisted timestamp exists to become a lease.
• Cancellation, races, limits, privacy. Single flight under 24 concurrent checks; caller cancellation abandons the wait, not the shared flight; retry capped at one start per 5s. Bounds are enforced everywhere they matter (64KiB body, 100 pages, 10000 members with the 10001 case tested, 4096-byte URL, 1MiB sealed evidence, 1024 receipts). Every error surfaced is a fixed sentinel — transport errors that could carry URLs or tokens never propagate (asserted), durable bytes are sealed and grepped for token/ref/role, and the module imports no logger. Redirects return unfollowed, so the bearer cannot cross hosts.

Advisories (non-blocking)

1. Unknown kid reaches family.Verify with a nil key. keys[e.KeyID] for an unfamiliar kid passes nil into Verify, and rastrillo's nil-key behavior is not in the inline evidence. Only the pinned HTTPS origin (or its compromiser) can exercise it, and the worst case is a fail-closed crash, not forgery — but add an explicit key, ok := guard and an unknown-kid case to the refusal matrix.
2. Blocked is un-clearable within a generation. A higher-sequence record for the same generation keeps Blocked=true, so recovering from a spurious generation fence (directory restored back to the original generation) requires a full generation change with owner confirmation. Fail-closed and defensible — but decide it on purpose and write it down.
3. Receipts cap is a lifetime brick. After 1024 accepted records, nextEvidence refuses all future updates permanently. It is a fixed bound (correctly not a slowest-consumer rule), but name it as an operational ceiling.
4. The refusal matrix covers team/directory/audience binding but not the consumer field, and the deliberate _ = c.save(...) in the generation-fence path deserves a comment so nobody "fixes" it into a return before the fence. Similarly, publish can advance the durable floor and then refuse publication on the second deadline check — safe (floors only refuse), worth a sentence.
5. Carried forward to pieces 2–3: producer-side ValidChallenge on inbound fetch, gate-1/2 corpus seeded with bearer tokens and cursor plaintext when the directory tables land, grant-only mode as one scope constructor, the SSE re-arm test (the Lease comment already pins re-arm-from-new-decision), and the §5.3 divergence note, which I could not verify from the inline evidence.

The two absolutes hold in the code as they held in the design: a removed member's standing dies within the measured 60-second ceiling even when every refresh, save and clock fails, and no unauthenticated, replayed, partial or unchanged byte extends it. Merge remains conditional on actual CI green per the shipping cadence, and consumer pins and operator delivery remain audited integration gates as scoped.

Canonical module publication verified after review and CI: amadan.net/oficina/home/roster v0.0.0-20260908115900-667dc8218dd5, h1:MQElRkuUPK46BWA3V3CVgJC9robI8X7LR0N67Jb3EZI=. Remote go list + download succeeded; separate /tmp/oficina-roster-consumer-smoke compiled and ran with GOWORK=off and no replace. Producer author has permanent pin. Nonblocking operational limitations remain explicit: a generation fence requires a genuinely changed, owner-confirmed generation; 1024 operator record receipts is a bounded lifetime ceiling; complete durable-state loss requires latest audited operator delivery. No runtime route or deployment.