oficina / home Public

Distinguish handoff failures and log fixed diagnostic stages open

On branch fix/handoff-diagnostics

User encounters generic magic-link expiry message when launching Docs; underlying stage is not yet identified. Add bounded fixed stage/reason logs for Home credential assertion and directory handoff failures, never raw errors or request/identity data, and a separate generic handoff failure page. Preserve statuses, validation, cookies, sessions, CSP and magic-link behavior. This improves diagnosis only; it does not claim to fix unknown failure. User waived copy review. Root owns Fable and deployment.

Candidate fde9070e5588f2f6ba18f078fce2c8c89937e215 pushed. New separate handoff error page; fixed stage/reason logging for credential authorization/assertion and directory start/callback/finish. No raw errors, request values, URLs, cookies, addresses, subjects or tokens logged. Tests cover poisoned request input, unchanged true magic-link message, and a real directory current-subject conflict: refuses, logs directory_finish/refused, retains old session, no identity switch. Focused tests passed; full make test and exact CI running. This diagnoses the unknown user failure rather than claiming it fixed.

Review bound to exact fde9070e5588f2f6ba18f078fce2c8c89937e215 by prompt and frozen git archive. Root observed terminal CLI exit0.

Verdict: APPROVE (no blocking findings). Reviewer: Claude Fable 5.1, model ID claude-fable-5-1. Read-only review of the frozen archive and diff; no tests run by me, no live auth or email. Write tool is disabled here, so the plan file could not be written; the full verdict follows.

Delta reviewed

  • internal/home/exchange.go: 11 signinFailed sites become failAssertion(w, code).
  • internal/home/directory.go:50: consumer Fail hook now calls failDirectoryHandoff.
  • internal/home/handoff_failure.go (new), templates/handoff_failed.html (new), handoff_failure_test.go (new).
  • Related files read: handoff/http.go, handoff/store.go, signin.go, app.go, directory.go, directory_runtime.go, render.go, security.go, origins.go, assertion_intent.go, layout and failure templates, exchange_test.go, testapp_test.go, logs_test.go, logtest, noscript_test.go, markup_test.go, chooser fixtures, AGENTS.md, Makefile, .amadan/ci.d/50-race, ../gate.log.

Checks

| Check | Result | |---|---| | Token/email/subject/nonce/query/raw error in logs | None. Both Warn calls log only stage and reason, each a compile-time literal picked by switch (handoff_failure.go:26-72). r.URL.Path is compared to constants and the constant is logged. No err reaches a log. | | Disclosure in page body | None. Static text plus HomeURL, which is the validated directory origin (origins.go:64-85), html/template-escaped in href context. Test asserts the secret is absent. | | Status preservation | Assertion paths stay 400. Directory consumer passes status through unmodified: 404/405 from guard, 400 for ErrRefused, 503 for storage/clock/full (handoff/http.go:70-103). Allow header survives since render only adds Content-Type. | | Cookies / sessions / permissions | No cookie attributes touched. sessions.Require, middleware, CurrentSubject, Complete unchanged. Subject-conflict refusal in Store.Finish (store.go:351) unchanged; new test asserts 400, no session minted, prior session still 200. Refusal is not converted into an identity switch. | | CSRF / CSP | Router groups unchanged; callback remains the sole route outside csrf.Protect. SecurityPolicy() untouched. New page has no script, form, or inline style; the link is navigation, not form-action. | | Rendering context | Buffered render, no-store set before write. Page renders on both origins; neither renders account-authored content. | | Magic-link UI separation | signinFailed and signin_failed.html retained for handleVerify and completeSignIn, still logging nothing (oracle avoidance preserved). Test pins body text and absence of the new log line. | | Gate 9 / template sweeps | New lines carry no address, team, URL, actor (AGENTS.md:249-252). TestNoContentInLogs drives /authorize routes with canaries through a.logger. No rst- classes, no script tags. |

Informational notes (non-blocking)

  1. Race gate incomplete. ../gate.log shows vet, fmt (silent on success), build, and go test green for every package, then the go test -race line with no result. Local gate is not terminal yet; CI pending.
  2. Unauthenticated log volume. Any bad GET /authorize?... emits one Warn. Same class as existing session logging. Low.
  3. assertionSession reason is unreachable because /authorize/resume sits behind sessions.Require. Harmless.
  4. Evidence limitation on the user report. If the user actually saw signin_failed.html via POST /signin/verify or completeSignIn, this delta adds no diagnostic on that path by design. It only separates handoff refusals from magic-link refusals; the existing logger.Error calls at signin.go:205,302,328,337 remain the only signal there.
  5. Single-log-line assertion in the test couples it to rastrillo sessions/handoff emitting nothing on these paths at the pinned version. Intended tripwire, but an upstream bump could trip it.

Not verified by reviewer: tests, vet, gofmt, race, CI, canary. go.mod/go.sum untouched, so rastrillo internals were treated as unchanged. result.json in the review directory is empty and was not written.

Severity: blocking none; low notes 1 and 2; informational notes 3 to 5.

Authenticated browser investigation on deployed launcher source822a49b: actual Go credential/directory handlers through local TLS/CONNECT tunnel, real browser cookies, CSP and assertion POST, synthetic accounts only. Chromium/WebKit JS on/off: ordinary handoff4/4 passes; retained DB handler restart4/4 passes; different credential/directory subjects4/4 refuse and reproduce the original screenshot; competing sign-in tabs4/4 refuse. Explicit recovery4/4 passes: directory root retains its old valid session, explicit Sign out POST, then new Home sign-in with current credential session. No automatic identity switch. Evidence /home/paulca/.cache/oficina-home-app-launcher/stateful-822a49/verified.json. These are plausible causes, not a confirmed diagnosis of the user's live failure; URL/address still unprovided. Expired-intent browser case not run. All fixtures stopped.

Final candidate fde9070e5588f2f6ba18f078fce2c8c89937e215: full local make test terminal0 and exact candidate CI SUCCESS, including both ARM64 commands and all race suites. Receipt /home/paulca/.cache/oficina-handoff-diagnostics/evidence.json binds gate.log and ci.json. No changes after candidate. Root owns merge and release; this agent made no deployment.

Fable approved exact fde9070e5588f2f6ba18f078fce2c8c89937e215; full make test terminal0 and exact CI success. Built ARM64 from frozen git archive. Both preview Home body+edge versions now fde9070. Live invalid authorize probe rendered new handoff-specific400/no-store page; captured only fixed log stage=authorize reason=invalid_request. Initial deployment watch timed out during directory convergence; retained logs and final exact version proof in /home/paulca/.cache/oficina-handoff-diagnostics/. No user account/session inspection or email. No config or production release changes. This diagnostic/UI correction does not establish or fix the underlying user's failure; pending user retry can identify live stage. Prior browser experiments establish identity conflict and competing tabs as plausible causes, with explicit sign-out recovery verified. Ready to merge same candidate.

User confirmed the Home-to-Docs flow is working after retry. Underlying historical cause remains unproven; diagnostics preserved. Home55 closed on user-confirmed resolution, not a claimed root-cause finding.