oficina / memoria Public

Whole-branch review — Fable 5.1 (dispatched as claude-fable-5-1), 2026-09-23

Diffed origin/main...origin/fix/r1-signin-outcome (bfca708, merge base 29247af) as one change, against AGENTS.md, PLAN.md B5 and polish-audit/memoria.md §B.1. Scratch worktree; the branch was not edited.

VERDICT: APPROVE-WITH-NITS

What I verified

• Root cause and fix. rastrillo v0.23.0 auth/handlers.go writes ?sent=1, ?err=rate|address|1, ?err=expired (Verify and both Callback failures), ?force=1&err=keymail, and RequireFreshSession writes ?reauth=1; memoria's beginGuard writes ?sent=1. Read off the module source, not the discussion. signinOutcome reads exactly those words. sent is checked before err, so force=1&err=keymail lands on the keymail sentence; an unknown err code says something rather than nothing.
• Home mode is covered too. With Home configured /signin is a.signin -> connect.Routes.Signin, and that handler (home/connect@5b8d8410, routes.go:106) delegates to LocalSignin whenever sent, err or force is present. So the fix reaches both sign-in modes.
• Tone mapping. render.go tone() maps good->positive, warn->warning, default->negative; the template writes rst-tone="{{.FlashKind | tone}}", and the test asserts on the rendered rst-tone="positive|negative" attribute — the state, not the copy.
• The test discriminates. Ran it green, then three mutations of my own choosing (not the author's): (A) sent returns an empty sentence — red; (B) misspell the expired arm so it falls to the generic sentence — red on both the magic-link and keymail-callback subtests; (C) rate-limit sentence with tone good — red. Reverted each. followToSignin also refuses a non-redirect and pins the Location query verbatim, so a rastrillo vocabulary change fails by name.
• Removal of "That address is not on this instance". Correct on both counts. Nothing on main or this branch writes outcome=denied (grep of internal/, cmd/, tools/: only the new comments mention it); beginGuard deliberately answers a stranger with ?sent=1 (gate 33), and rastrillo's own Begin comment says distinguishing "would be an enumeration oracle". A sentence that only a distinguishable path could reach is exactly the owners-list oracle the guard exists to remove. Deleting it is the right call, and the comment records why.
• No new log lines, no address or query text in any string. make ci targets (vet, fmt, test) green on the branch locally; amadan ci status passed at bfca708.

Findings (none blocking)

1. [nit] handlers.go:1132 — the reauth=1 arm is unreachable in this app. Nothing in memoria uses RequireFreshSession, and in Home mode connect.Routes.Signin only delegates to the local page on sent|err|force, so ?reauth=1 would go to the Home handoff anyway. AGENTS.md's rule on defences nothing can reach applies in spirit: either drop the arm (and its unit-test row) or leave a one-line note that it is rastrillo vocabulary held for the day step-up is wired. Not worth a re-spin on its own.
2. [nit] signinoutcome_test.go (memoriatest) — "a plain visit says nothing" asserts on the absence of the string rst-callout. It is the presence half of the six cases above, so it is fine as written; if the page ever gains a static callout the test will say so loudly, which is the right direction.
3. [note, no change] ForceEmail only when Home is enabled (the author's "noticed, not changed"): after ?force=1&err=keymail on a non-Home instance the next submission re-dials keymail. Pre-existing; the new sentence at least tells the person what happened. Worth a follow-up bug, not this branch.

Mutations run and reverted: 3, all red. Live defects found: 0.