US-45.7: thread page with WebAuthn browser login

[mkreyman]

Story US-45.7 (Epic 45 change threads, PRD §6.1). The tenant's human reads a story's thread at /threads/:story_id and writes message and finding entries on it, after signing in at /login with the WebAuthn credential enrolled at signup. The review gate has not run yet; it runs next.

What it does

Login (AC-45.7.1). Loopctl.WebAuthn.BrowserLogin uses Reauth's ceremony under the purpose browser_login. The challenge is stored and single-use, its allow-list is the tenant's enrolled RootAuthenticators, the sign counter is checked, and every failure fails closed.

LoginLive runs navigator.credentials.get() through the WebAuthnLogin hook. It fills a form with the tenant_id and challenge_id the server holds, never values the client sent. It then triggers a POST to BrowserSessionController, so the request goes through :browser's CSRF check and the controller can write the cookie.

The session:

• is renewed on login, and the CSRF token is rotated;
• is bound to {tenant_id, authenticator_id, authenticated_at};
• gets a live_socket_id, so logout disconnects any open LiveViews;
• lasts at most 8 hours from the assertion, whatever the activity.

BrowserLogin.validate/2 checks the session:

• on every HTTP request (RequireBrowserSession plug);
• on every LiveView mount (on_mount on the :browser_threads live_session);
• before every write;
• every minute while a page is open.

The session ends at the next check once the authenticator is revoked (revocation deletes its row) or the tenant is no longer active.

Rate limits fail closed:

• challenge issuance: per client IP (in the LiveView) and per tenant;
• verification: per client IP (in the controller) and per tenant.

Page (AC-45.7.2). Threads.page/3 reads the story, its thread and the project's repository in one Repo.with_tenant transaction, under the tenant's RLS and never through AdminRepo. The page shows:

• checkpoints: kind, claim epoch, merge SHA, and the gate_evidence["ci"] summary and jobs;
• entries: in a <pre> as escaped text, each marked untrusted, with no Markdown and no raw HTML;
• findings.

"show diff" calls Threads.checkpoint_diff/3 inside start_async. That function reads the checkpoint and its parent in a short RLS transaction and closes it before calling the new forge callback PullRequestSource.checkpoint_diff/3:

• The callback asks GitHub for /compare/parent...head, or /commits/head for a first checkpoint, using the diff media type.
• The response is capped at 512 KB and 10 s of wall clock, and marked truncated when either cap cuts it.
• The diff is never stored.
• A slow or down forge shows its state in that checkpoint's panel only.

Writes (AC-45.7.3). Each form carries a nonce minted when it is rendered, and the nonce is the entry's idempotency key. The key is read from the submitted form, not the socket. A double submit, a resubmit after a lost reply, or a reconnect therefore replays the same key and produces one entry.

Entries are written as human:webauthn with an empty lineage. That is the label the audit chain already gives WebAuthn-authenticated human acts, and no API key can produce it.

• Messages go through the existing Threads.record_entry/4.
• Findings go through the new Threads.record_human_finding/3: under the story lock, secret-screened, and refused tenant_halted during a halt. A resend is still answered from its row during a halt.

Issue link (AC-45.7.4). When a thread records its first checkpoint, the same transaction writes a thread_issue_links row, unique per story. That is when the page first has work on it, and every thread passes through it exactly once.

ThreadIssueLinkWorker runs on cron every 2 minutes. It posts one comment through the existing comment_issue path and GITHUB_TOKEN, using a compare-and-set claim and holding nothing across the forge call.

Design decisions to check

1. A human finding needed a schema change. The thread_entries_judgement_shape CHECK required review_id on every finding. It now also admits author_principal = 'human:webauthn', and no other author. A test proves an agent author is still refused.
2. "Same standing as an agent's" (PRD §6) is enforced, not just stored. A human finding counts in round N when it is on round N's checkpoint and was written after round N-1's verdict and before round N's (Reviews.round_findings/4). That window stops one finding counting in two rounds, and stops a finding written after a verdict from reopening that verdict. Within it, a human finding:
   • counts toward the ceiling escalation;
   • counts toward the round-3 decision;
   • can be named by a fix (answers_findings/4);
   • follows the reviewer's introduced_by rule for the round in progress.
3. Tenant halt. Findings are refused, as judgements are. Messages are allowed, as on the API, where thread entries are not in CustodySurface. The page shows a halt banner.
4. A duplicate comment is possible. If a node dies after GitHub accepted the comment and before mark_commented, the comment is re-posted. IssueCloser documents and accepts the same cost for its own comment.
5. No MCP tool. These are browser routes: a cookie session and a WebAuthn ceremony. They are declared browser in route_coverage.test.js, and the route snapshot is regenerated.

SOUL rule 9

Failure	What happens
Replayed submit	The form nonce is the idempotency key, so the resend is answered from its row and there is one entry (TC-45.7.3, message and finding).
Expired or reused challenge	The challenge is stored, single-use and TTL-bound. Replay and expiry are refused (tests).
Authenticator revoked mid-session	The next request, write or timer check ends the session (a test for each).
Tenant halt	Reads stay open and a banner shows. Findings are refused and messages allowed (test).
Slow or down forge	The diff loads asynchronously within its bounds, the ledger renders regardless, and the error shows in the diff panel (the test holds the forge mid-call and asserts the ledger is rendered).

Tests

New test files:

• test/loopctl/web_authn/browser_login_test.exs
• test/loopctl_web/live/login_live_test.exs: TC-45.7.1, including the CSRF refusal through Endpoint.call
• test/loopctl_web/live/thread_live_test.exs: TC-45.7.2, TC-45.7.3, the session guard, tenant isolation
• test/loopctl/threads/human_findings_test.exs
• test/loopctl/threads/issue_links_test.exs
• test/loopctl/workers/thread_issue_link_worker_test.exs: TC-45.7.4 end to end on committed data, plus the cron wiring

The adapter's checkpoint_diff tests are in github_pull_request_source_test.exs. Every new context function has a tenant isolation case. The commit hook ran the full gate: 11910 tests, 0 failures.

Mutation table

Each row ran ~/workspace/claude-config/bin/mutate.sh <file> --no-baseline --old '<text>' --new '<broken>' -- mix test <file>, one at a time, after the check had passed unmutated. Exit 0 means the check went red under the mutation.

#	File	Mutation	Check	Exit
M01	router.ex	drop :browser_session from the thread scope (plug wiring)	thread_live_test	0
M02	router.ex	drop the live_session on_mount (mount wiring)	thread_live_test	0
M03	router.ex	drop plug :protect_from_forgery	login_live_test	0
M04	browser_auth.ex	drop clear_session() on login	login_live_test	0
M05	browser_auth.ex	on_mount returns :cont instead of :halt	thread_live_test	0
M06	browser_session_controller.ex	skip the per-IP verify throttle	login_live_test	0
M07	login_live.ex	skip the per-IP challenge budget	login_live_test	0 (1 on the first run; the test's limiter stub was fixed and it was re-run)
M08	login_live.ex	trust a tenant_id the client sent	login_live_test	0
M09	browser_login.ex	skip the per-tenant challenge budget	browser_login_test	0
M10	browser_login.ex	ignore authenticator revocation	browser_login_test	0
M11	browser_login.ex	ignore the lifetime	browser_login_test	0
M12	browser_login.ex	ignore an inactive tenant	browser_login_test	0
M13	thread_live.ex	skip the revalidation before a write	thread_live_test	0
M14	thread_live.ex	skip the revalidation on the timer	thread_live_test	0
M15	thread_live.ex	take the idempotency key from a fresh nonce instead of the form	thread_live_test	0
M16	thread_live.ex	write as api_key:page instead of the human principal	thread_live_test	0
M17	thread_live.ex	render entry bodies with raw/1	thread_live_test	0
M18	threads.ex	skip the halt check on a human finding	thread_live_test	0
M19	threads.ex	never record the issue link at the first checkpoint	issue_links_test	0
M20	threads.ex	fetch the diff with no parent	thread_live_test	0
M21	reviews.ex	accept any checkpoint for a human finding	human_findings_test	0
M22	reviews.ex	leave human findings out of the round	human_findings_test	0
M23	reviews.ex	drop the upper bound of the round window	human_findings_test	0
M24	reviews.ex	stop fixes from answering human findings	human_findings_test	0
M25	issue_links.ex	CAS claim without status == :pending	issue_links_test	0
M26	issue_links.ex	skip the revoked-source check	issue_links_test	0
M27	oban_config.ex	drop the cron entry	thread_issue_link_worker_test	0
M28	github_pull_request_source.ex	never truncate the diff	github_pull_request_source_test	0
M29	browser_login.ex	skip the per-tenant verify budget	browser_login_test	0 (1 on the first run; a test was added and it was re-run)
M30	migration	the CHECK admits any author (the check ran a rollback and migrate; the database was re-migrated afterwards)	human_findings_test	0

Not covered

• Nothing proves the one-minute timer is scheduled. Its handler is tested (M14); the Process.send_after call is not.
• Nothing asserts that logout's broadcast disconnects open LiveViews.
• The 10 s diff deadline has no test. The byte bound is covered (M28).
• The ceremony was not run end to end with an authenticator in a real browser. Chrome's CDP virtual authenticator cannot answer an allow-listed get() (see authenticator_enroll.js). The server side is covered with the mocked adapter.

Screenshots

Taken from a dev server running on its own port against an isolated dev database, with a session cookie minted for a seeded tenant, at 1440 px and 390 px wide:

• Desktop has two columns: the ledger on the left, and the findings and both forms on the right.
• At 390 px the page stacks cleanly with no horizontal overflow.
• An entry body containing <script> and **markdown** shows as literal text, and the entries contain no script elements.
• The CI line reads "2 passed · 0 pending" on one checkpoint and "1 passed · 1 pending" on the other.
• The diff panel for the fake acme/widgets repository reads "diff unavailable: the forge answered 404", and the ledger beside it is unaffected.
• The login page shows a single card.

Review round 1 (14 findings, fixed in 63ce0e4) — supersedes the design notes above where they differ

• Login cannot be locked out or used to enumerate tenants. Per-tenant budgets are removed; the challenge and verify steps are limited per client (RemoteIp.bucket_key/1). An unknown, inactive or unenrolled slug gets a decoy challenge with the same shape (random id and bytes, a credential id derived as an HMAC of the slug), the form carries only the challenge_id, and the tenant is resolved from the stored challenge; no tenant id is rendered. Residual timing difference (a real challenge costs a lookup and insert) is stated in the moduledoc.
• Sessions are server-side. A browser_sessions table (RLS) backs the cookie {tenant_id, session_id}; logout revokes the row; every request, mount, write and 60s tick validates it unrevoked, unexpired and its tenant active; deleting the authenticator cascades to its sessions.
• Human findings count in the round in progress (after round N-1's verdict, before round N's) on any checkpoint; a material one after the final verdict records a review_ceiling escalation and the ceiling worker moves the story.
• Diffs are always the placed base branch ...checkpoint (what the gate judges), fetched on demand with a retry control after a failure; the body is accumulated linearly.
• The page opens on the newest entries, pages older ones backwards, and a write inserts only the new entry.
• The issue link is derived by a sweep from durable state (threads with a checkpoint and no link row), which also covers threads that predate this change; the in-transaction trigger was removed.
• A crafted event on a page with no story is refused; both page test files are async: true.

Mutations: every cited one re-run at the new text (M09, M10 and M29 retired with the code they tested) plus N01-N18; all exit 0. Not covered by a test: the 10s diff deadline (only via a past-deadline unit check), that the 60s timer is scheduled, and logout's disconnect broadcast.

Review round 2 (10 findings, fixed in 8115f03 and 32dbca9) — supersedes the login notes above

• Login is usernameless with a discoverable credential and required user verification: no slug field and no decoy (both deleted). A tenant-less, single-use, short-lived challenge with empty allowCredentials; the authenticator is found by its credential id (new global unique index tenant_root_authenticators_credential_id_uidx) and the tenant comes from it; an unknown credential answers exactly like a bad signature. AC-45.7.1 reworded accordingly. The login challenge carries no purpose (a redundant filter my own mutation showed could not fail; removed).
• Issue links: written in the first checkpoint's transaction (skipping closed issues), plus a one-time backfill in the migration for in-flight stories whose issue is not closed; the worker only drains pending rows. No recurring history scan.
• Diffs compare each checkpoint against the placed base branch of its OWN claim (dispatch_route/3), falling back to the source's current base only when that claim has no ledger row, and saying so; one diff open at a time; only this page's checkpoints can be fetched.
• A late human finding distinguishes escalated from already_escalated and only the first moves the stage; a session insert racing a revoke refuses cleanly.

Before deploy: the global credential-id index fails the migration if two tenants share a credential id; the CHANGELOG carries the check query. A security key enrolled without a resident credential cannot log in at /login until a passkey is enrolled.

Mutations: cited rows re-run (N01, N02, N15 retired with the deleted code), R01-R22 new, all exit 0 except R23 (no re-enqueue on already_escalated), whose absence a test cannot observe because the enqueue is idempotent against an already-escalated stage.

Review round 3 (the ceiling; 9 findings, fixed in place in 504f339, no round 4)

Contained edges, fixed with mutation proof as #910 and #911 were.

1. Expired or used login challenges are purged by ReauthChallengeCleanupWorker.
2. The form nonce survives a reconnect: each form's phx-change handler adopts the client's nonce (shape-checked) from LiveView's form recovery; it rotates only after a recorded write. Tested by simulating the recovery event; not observed in a real browser.
3. A resent human finding answers exactly what its first delivery recorded (the escalation at seq+1, else nothing), never an escalation inferred from later state.
4. The finding form offers only the current claim's checkpoints, refreshed on the 60s tick and after every write, and is disabled with a reason when there are none.
5. "Load older" reads entries only (Threads.page_entries/3).
6. Loopctl.ForgeOutbox holds the outbox mechanics IssueClosures and IssueLinks share (IssueClosures' behaviour and tests unchanged).
7. One halt check (Runners.custody_halted? via not_halted/1) for every judgement and the page banner; the two halt tests run in their own async: false module on a committed tenant.
8. Loopctl.GitSha.valid?/1 is the one commit-SHA rule.
9. Migration doc points at the real test.

Mutations: cited rows re-run at the current text, T01-T17 new, all exit 0 except R23 (as before). T07 (a released claim offers no checkpoints) has no page test; the server refusal of that case is proven (M21). Session check: removing the login-challenge purge fails its test.