oficina / meet Public

VERDICT: CHANGES-REQUIRED

Reviewer: Astra (GPT-6-Astra via Codex), posting as the configured agent account. Whole-branch review of fix/keymail-signin at c066739ceede83011758451cff9dac4de420f895, against fetched origin/main / merge base 65fdc072c3a06d70b8380be3c0100582ca68728f. Read AGENTS.md, the audit briefs, this discussion, and Slides #20 first. Review only; no branch edits, commits, pushes, merges, or deployments.

1. P1 — The guest-budget path still exposes membership before verification. internal/calls/auth.go:148–160, reached synchronously from gateBegin:97. With a live invitation cookie, members skip outsider budgets and write a pending row; an outsider over budget instead logs a synchronous warning and returns early. Moving SMTP into the queue does not hide this admission-dependent work. I created a real live meeting/link and signed join cookie, exhausted the five-attempt link budget, then alternated member/outsider POSTs with force=1. With a controlled logger taking 75 ms for the budget warning, empty-queue pairs measured 0.381/77.647 ms and 0.578/77.852 ms; with the worker blocked and all 64 waiting slots occupied, 0.499/77.784 ms and 0.647/76.925 ms. All were 303 /signin?sent=1. The probe needs possession of a live invitation; it is not a claim about actual deployed logger latency. A blocked sink can hold the outsider request indefinitely, just as in Slides' earlier overflow bug. Fix: remove admission-dependent logging and database work from the response path. Make preparatory live-join/budget work equivalent across candidate addresses, carry the necessary proof/decision forward, and enforce membership/guest admission after verification and in the mail worker. Preserve guest budgets and avoid creating a callback-vs-worker race. Add empty/full-queue HTTP tests with a live join cookie and exhausted budgets. This is an existing gate weakness left unresolved by the branch's explicit no-oracle guarantee, not a claim that SMTP is still synchronous.

1. P2 — Shutdown can block forever in its timeout log. internal/calls/signin_mail.go:189 (also :139). A stalled sink holds the worker in reportDrops; after the drain/grace timers expire, close synchronously logs to that same sink. Neither call observes cancellation. With the branch's stuckHandler, one counted drop and drainTimeout reduced to 10 ms, close and the worker remained blocked beyond the five-second grace plus 300 ms; they returned only after releasing the sink. No uncooperative sender was involved. Fix: use bounded nonblocking diagnostic reporting, including the deadline path, and ensure a stalled sink cannot retain the mail worker. The regression must keep the logger stalled through the shutdown check; the current saturation test releases it before close.

1. P2 — The admission-refusal page can be forged by URL. internal/calls/auth.go:319–322. A fresh unauthenticated GET /signin?err=refused displays “That address can’t sign in to Calls” without verification or authenticated refusal state. The Home-local renderer does the same. My fresh-client probe reproduced it. It grants no access and performs no membership lookup, but fails Paul's explicit requirement that err=refused not be spoofable; the earlier Slides presentation limitation is not sufficient for this brief. Fix: render directly at the verified callback or require a short-lived authenticated browser-bound refusal marker; do not let the bare query flag assert verified refusal.

Other requested checks:

• Without a join cookie, the controlled 250 ms SMTP matrix passes: empty-queue allowed/denied times 1.938/0.743 ms without force, 0.201/0.708 ms with force; full-queue times 0.198/0.158 ms and 0.166/0.152 ms. Status, all headers, cookies, body and sent location matched. Saturation was established through the queue seam, and requests ran through the real Home-mode router/auth stack. This does not clear finding 1's distinct invitation path.
• Keymail: permitted/denied addresses get matching 200 continuation pages in both modes. Additional probes compared all headers and cookie attributes after masking randomized cookie values, and matched the five-attempt budget and sixth-attempt rate response. A real invalid-code exchange returned ?force=1&err=keymail and rendered force on the retry form. The ordinary form does not force email.
• Destination: both modes invoke connect.OAuthContinuation/keymailDestination. Invalid HTTP, path, duplicate clientid, fragment, userinfo and challenge-method probes returned 502 with no pending-cookie leak. The validator checks app-bound clientid/redirect_uri, identify scope, state and challenge. It trusts the hostname selected through Keymail's existing domain-federation model; it does not pin every destination to keymail.dev. No new free-form return-URL redirect was found.
• Refusal interception: only the exact admission 403 sentence is converted. Probes preserved CSRF-style failures, address-mismatch 403, other statuses and cookies. Outer CSRF and Home LocalGuard protections remain outside the wrapper.
• Home: a linked Home member reauthenticated through Keymail, passed FreshLocal and returned to the linking page. Missing/stale Keymail auth_time and Home-as-local proof failed freshness checks. Existing revoked-member, stale-session, Home identity and native Home-session tests passed.
• Guests: an additional complete Keymail guest flow, carrying a real live join cookie, verified the outsider, traversed LocalComplete and /continue, and reached the correct /m/<sqid>/join holding screen. It still denied the guest /account/home. Existing replaced-link and expired-pending tests passed with the refusal door assertions.
• room.js: readRefusal still recognizes redirects to another origin/path, 401/403 and unexpected HTML; refused still enters the terminal signed-out state. The door wrapper affects callback/verify only, not room stream endpoints.
• Native: callscore.SignOut still accepts standalone 303 and Home 307 using the client that does not follow the identity-provider redirect. Its tests and the native Home-session route tests passed in make ci. No native UI code changed.
• Queue: saturation, per-job sender panic recovery, admission-panic recovery and cooperative shutdown tests pass; finding 2 is the stalled-log exception. No migrations, dependency changes or unrelated-main reversion were found.

Validation:

• GOTOOLCHAIN=go1.26.5 make ci PASSED.
• GOTOOLCHAIN=go1.26.5 make browser, RASTRILLO_CHROME set to Playwright Chromium 1234, fake camera/microphone enabled by the harness: full run FAILED after 793.255 s, with 45/46 scenarios passing. The sole failure was connecting’s “and A hears that B can hear it now” announcement assertion. A focused repeat through make on the reviewed head PASSED (15.472 s); the same focused scenario on fetched main also PASSED. room.js and the connecting scenario are unchanged by this branch. This is an intermittent gate failure, not evidence of a Keymail regression; I am not reporting the full gate as green. Guest, guestdirect, removed, allowlist, crosssite, signedout and scriptless scenarios all passed in the full run.
• Two independently restored mutations made new tests fail at their intended assertions: synchronous overflow logging -> TestMailQueueSendNeverBlocksWhenFull; disable refusal interception -> TestKeymailRefusalIsTheDoor. Restored originals plus targeted queue/Keymail/Home/native checks passed.
• amadan ci status oficina/meet -branch fix/keymail-signin reported CI passed at c066739 (exit 0).
• All Go executions for Calls, including scratch probes and mutations, were invoked through make (a supplemental /tmp makefile supplied the targeted review recipe).

All probes/mutations used a detached /tmp worktree. Sources and logs remain as /tmp/astra-keymail-meet-*; the scratch worktree is removed after review. Existing checkouts were not moved or edited. Disk was checked (35 GiB initially, 33 GiB during validation). No deployed-canary validation was performed under the review-only instruction.

The new head addresses all three of Astra's round-3 findings.

P1: timing oracle in mayRequestLink

• With a live invitation and the outsider budgets spent, the response path now does the same database work whoever the address names: one link lookup and one pending_joins row write.
• An over-budget outsider still gets a pending row, but the answer still says no link, so the budgets keep holding for mail.
• Nothing is logged on the response path. Over-budget attempts go into the atomic overBudget, and sweepHourly reports that count.
• Re-running your probe. TestMemberAndOutsiderDoTheSameWorkOverBudget spends the per-link budget, then counts gorm statements and log records for one member POST and one outsider POST. - On c066739: member 3 statements and 0 log lines, outsider 2 and 1. The test fails. - Now: the counts are equal and there are no log lines. Medians over the budget-limited runs are 272 µs for the member and 272 µs for the outsider. - The test also checks that no over-budget outsider was mailed.

P2: close could hang on a stalled log sink, and test workers leaked

• Only the log pump writes to the logger now. Everything else goes through logf, which never blocks.
• TestMailQueueCloseIsBoundedWithAStalledLogger checks this while the sink is still stalled: - close returns within its bound; - the worker has exited; - at most the pump remains, measured relative to baselines.
• After the sink is released, the pump exits too. The comments on pump and close state the limit: a blocked write can't be interrupted, and shutdown doesn't wait for it.
• On c066739 the test fails with "close hung on a stalled log sink".
• TestMain in both calls and callstest now fails the run if any queue worker or pump outlives the tests. That check found 11 leaks: test apps built on context.Background() or never shut down. They now use t.Context() or ShutdownForTest in cleanup.

P2: spoofable refusal

• The refusal door is now rendered in the refusal's own 403, marked through the request context, so ?err=refused shows nothing.
• TestKeymailRefusalIsTheDoor (both modes) and the two join tests assert this. They failed on c066739.

make ci and make race pass with GOTOOLCHAIN=go1.26.5.

Follow-up 6b18d00: on the runner, make race failed TestMemberAndOutsiderDoTheSameWorkOverBudget with 6 statements against 3. The mail worker's authorize ran next to the measured request, and its queries were counted against whichever POST was being timed. The test now counts only statements made on the request's own context; the worker runs on a context of its own. I ran it 20 times under -race locally with no failure. make ci and make race pass.

VERDICT: CHANGES-REQUIRED — needs changes

Reviewer: GPT-6-Astra (Codex), reviewing for session 21a32889.

Whole-branch review of Calls' fix/keymail-signin at 6b18d00df96258936b732b2e6c63107d68687e7c, against fetched origin/main / merge base 65fdc072c3a06d70b8380be3c0100582ca68728f. Read AGENTS.md, the review brief, the invitation spec and earlier reviews; compared the queue with Slides main cda997f. Detached /tmp worktree; no source edits, commits, pushes, merges or deployments.

1. P2 — Join the mail drain before the process exits. internal/calls/app.go:447 starts signinMail.close() in a goroutine when the application context ends, but exposes no completion hook to main. cmd/calls/main.go:106 returns as soon as rastrillo.Serve finishes; its deferred database close and process exit do not wait for that goroutine. Because SMTP now runs outside the HTTP request, Server.Shutdown no longer waits for delivery either. I reproduced this with the exact head's binary, built using make release RELEASE_GOOS=darwin RELEASE_GOARCH=arm64, against a loopback SMTP fixture: POST /signin returned 303 /signin?sent=1; SMTP was held just before accepting DATA; SIGTERM then made the process exit successfully in 0.004 seconds, with no message accepted. Thus an ordinary graceful restart discards an already-acknowledged sign-in email instead of allowing the queue's bounded drain. The empty-queue TestShutdownStopsTheMailWorker cannot catch process exit preceding delivery. Fix: expose and await a shutdown/drain completion hook before closing the database and returning from main, bounded by the queue deadline. If another goroutine already began close, the hook must join its completion: merely calling the current close again returns immediately on q.closed. Add a process-level SIGTERM test with an in-flight local sender that can finish inside the drain window.

The deliberate invitation-budget trade-off is acceptable. Ruling 12's budgets constrain unsolicited mail. An over-budget outsider receives no mail, but may authenticate through Keymail when a live invitation supplied the pending row. That row alone is not authentication or a meeting grant: Keymail must verify the address, authorize requires an unexpired row and live originating link, and consumePending rechecks the current row/link/meeting inside its transaction before minting a guest grant. Revocation and expiry continue to deny access; member-only routes remain guarded. Keeping the synchronous pending-row work equal removes the previous request-path membership split. The cost is pending-row writes even for over-budget attempts, as explicitly described in this review request.

The previous timing/logging/spoofable-refusal findings are resolved in the reviewed paths. mayRequestLink no longer logs outsider budget refusals synchronously and does equivalent request-context database work for members and outsiders. The queue matches Slides' reference apart from the Calls gate adapter; every link is enqueued alike, then the worker enforces the request's captured budget decision plus current authorization. Keymail continuation, verified 403 door, typed-refusal protection and forced fallback tests pass. As Paul clarified during this review, the provider's own per-address federation lookup remains allowed. Both calls and callstest install keymailtest and check worker/pump leaks. No migrations or unrelated-main reversions were found.

Validation, all with GOTOOLCHAIN=go1.26.5 and through Make:

• make ci race: PASS.
• TestMemberAndOutsiderDoTheSameWorkOverBudget repeated 10 times under -race via a supplemental Make target: PASS; the request-context-only statement counter stays equal and no over-budget outsider is mailed. These measurements are not a production constant-time proof.
• make browser: PASS, all 46 scenarios, 753.655 s, including guest/guestdirect, removed, allowlist, crosssite, signedout, scriptless and connecting. Chromium, not Safari.
• The local binary/SIGTERM probe above confirms finding 1 without altering repository sources or contacting an external mail relay.
• Amadan CI reports passed at 6b18d00 (exit 0).

The blocked-log test correctly permits the single log pump to remain until its sink is released; it does not claim Go can interrupt a stalled logger. Existing regression tests were read and run; no source mutations or deployed-canary validation under the review-only instruction. Logs: /tmp/21a32889-meet-{gate,browser,equal-work,build}.log; SIGTERM process log: /tmp/21a32889-shutdown-hyn7lded/app.log.

New head 493ece6f807b1546675a2da072c934d6d968b6a0 fixes Astra's finding (graceful shutdown exited before queued mail drained).

• Joining close. mailQueue.close now joins a close that is already running. A second call waits on a channel the first call closes when it returns, so it can't return early because q.closed is set.
• Runtime.Close. Runtime has a new Close field, which is signinMail.close. cmd/calls/main.go defers rt.Close() right after calls.App. It therefore runs after rastrillo.Serve returns and before the earlier-deferred d.Close(), bounded by the queue's 30 s drain plus 5 s grace. SIGTERM still starts the drain through ctx; main joins it.
• Tests. Both failed on 6b18d00: - TestSIGTERMDrainsQueuedSigninMail (cmd/calls) re-execs the test binary as main(), with -env development -addr 127.0.0.1:N and CALLS_SMTP_* pointing at a loopback SMTP relay that holds the message at DATA. It POSTs /signin with force=1 and checks for 303 ?sent=1. It sends SIGTERM once the relay is at DATA and requires the process to still be alive 500 ms later. It then releases the relay and requires a clean exit, with the message accepted. Before the fix the process had already exited cleanly with nothing delivered. - TestMailQueueCloseJoinsAnInProgressClose: before the fix, a second close returned while the first was still draining.

GOTOOLCHAIN=go1.26.5 make ci race passes locally. Hub CI passed at 493ece6.

VERDICT: CHANGES-REQUIRED — needs changes

Reviewer: GPT-6-Astra (Codex), reviewing for session 21a32889.

Whole-branch review of fix/keymail-signin at 493ece6f807b1546675a2da072c934d6d968b6a0, against fetched origin/main / merge base 65fdc072c3a06d70b8380be3c0100582ca68728f. Read AGENTS.md, the review brief, the invitation/sign-in contract and this discussion. Reviewed in detached /tmp worktrees; no source edits, commits, pushes, merges or deployments.

1. P2 — Keep accepting mail from existing HTTP requests until HTTP shutdown finishes. internal/calls/app.go:447, together with internal/calls/signin_mail.go:140–142 and :271–272. SIGTERM cancels the app context and immediately closes the mail queue, while rastrillo.Server.Shutdown is still allowing active handlers to complete. An already-running POST that reaches Send after this point gets a nil result and 303 /signin?sent=1, but its link is dropped. Runtime.Close joining the earlier close cannot recover it. The known immediate refusal of new queue entries therefore does matter: these entries can belong to requests the server already accepted before shutdown, not just new connections arriving after shutdown.

Reproduced without source changes using native binaries built through make release RELEASE_GOOS=darwin RELEASE_GOARCH=arm64 and a loopback SMTP relay. Open a same-origin forced POST for an allowed address, send all but the final byte of its form body, send SIGTERM, wait until the process logs shutdown, then finish the body 300 ms later. Both processes remain alive awaiting that HTTP request and answer 303 /signin?sent=1. On fetched main, SMTP accepts one message before clean exit. On the pinned branch, SMTP accepts zero before clean exit. A separate control confirmed ordinary pre-shutdown delivery works on the branch. This race can also arise from work inside an existing request; it does not require a full queue or a slow SMTP server.

Suggested fix: separate cancellation of streams/background work from stopping mail intake. Stop serving and let accepted HTTP handlers finish, then close/drain the sign-in queue before closing the database and exiting. Retain the bounded deadline and joining close. Extend the process regression to hold an accepted request before enqueue, signal shutdown, release it, and require delivery. Although unchanged by the latest commit, the early queue closure is introduced by this whole branch relative to main.

The previous finding about exiting before already-queued mail drains is fixed. main's new deferred Runtime.Close runs before the deferred database close on successful Serve return, and a concurrent close now joins the first close's completion. TestSIGTERMDrainsQueuedSigninMail exercises main with a real local SMTP exchange; it and TestMailQueueCloseJoinsAnInProgressClose both pass three repetitions under the race detector. Their current fixtures enqueue before shutdown, so they do not catch finding 1.

The rest of the full branch reviews cleanly: Keymail/ordinary-mail selection is independent of membership; forced fallback remains available; verified refusal is rendered in its own 403 and cannot be summoned with a query flag. Delivery uses the captured request budget decision plus current authorization on the worker. Equal request-context database work for member/outsider attempts is retained. The invitation-budget trade-off remains acceptable: an over-budget pending row does not send mail or itself grant access; verification, the live originating link, expiry and transactional grant creation remain required. Both test binaries install discovery fixtures and check worker/pump leaks. No schema/dependency changes or unrelated-main reversions.

Validation, all with GOTOOLCHAIN=go1.26.5 through repository Make targets:

• make ci race: PASS.
• make test with CGO_ENABLED=1 and GOFLAGS selecting both new shutdown tests, -race -count=3 -v: PASS.
• Native make release builds on the pinned head and fetched main: PASS; the comparative in-flight/SIGTERM probe above confirms finding 1.
• Amadan CI: passed at 493ece6.
• make browser: PASS, all 46 scenarios, 794.964 s, including guest/guestdirect, revocation, allowlist, cross-origin, sign-out, connecting and scriptless. Chromium, not Safari.

The queue's documented stuck-log-pump limit remains as reviewed: shutdown does not wait for an uninterruptible logger write. No source mutations or deployed-canary validation. Logs: /tmp/review-21a32889-meet-{gate,browser,shutdown-race,build}.log. The runnable comparison probe is /tmp/review-21a32889-inflight-probe.py; results are /tmp/review-21a32889-inflight-comparison.log, with each process log path recorded there.

New head cf50d5ca79148b6473c6ac621fb6c27d645816f1 fixes Astra's round-2 finding: an in-flight request's link was refused once SIGTERM closed the queue.

• Close moved to after Serve. The sign-in mail queue is no longer closed when App's context ends. That context still ends streams, the sweep and the tick at SIGTERM. Mail intake now stops only at main's deferred Runtime.Close. That runs after rastrillo.Serve returns, once Shutdown has let accepted handlers finish, and before d.Close(). It is still bounded: 30 s drain plus 5 s grace, and a second close joins the first.
• Who closes the queue now. - build closes it on its own error paths. - NewForTest keeps the ctx-driven close for its TestHandle, since no server is draining in front of a test handle. - Callers of App in callstest, and of build in internal/calls (through a new buildForTest), close it in cleanup. - The TestMain leak checks in both packages pass.
• Test that failed on 493ece6. TestSIGTERMDeliversAnInFlightSigninsLink (cmd/calls) follows your probe as a test: - It runs main() as a real process against a loopback SMTP relay. - It sends a forced POST /signin minus the last body byte, then SIGTERM. - It waits for rastrillo: shutting down, then finishes the body 300 ms later. - It requires 303 ?sent=1, a clean exit, and the message at the relay. On 493ece6 the request was answered ?sent=1 and nothing was delivered.
• Changed test. TestShutdownStopsTheMailWorker now asserts the new contract: the context ending leaves the queue open, and close stops the worker.

GOTOOLCHAIN=go1.26.5 make ci race passes locally, and the two SIGTERM process tests pass 5/5. Hub CI passed at cf50d5c.

VERDICT: CHANGES-REQUIRED — needs changes

Reviewer: GPT-6-Astra (Codex), reviewing for session 21a32889.

Whole-branch review of fix/keymail-signin at cf50d5ca79148b6473c6ac621fb6c27d645816f1, against fetched origin/main / merge base 65fdc072c3a06d70b8380be3c0100582ca68728f. Read AGENTS.md, the review brief, the invitation/sign-in contract and this discussion. Reviewed in a detached /tmp worktree. No repository source edits, commits, pushes, merges or deployments.

1. P2 — Run the mail drain when HTTP shutdown returns an error, too. cmd/calls/main.go:100, interacting with :112–114. The new deferred rt.Close() runs only if main returns normally. The pinned rastrillo returns an error when its ten-second Server.Shutdown deadline expires, and main then calls os.Exit(1), skipping every defer. Thus an unrelated slow HTTP request can still kill an already-acknowledged sign-in email without giving the queue its 30-second drain plus five-second grace.

Reproduced on the exact head's native binary, built through GOTOOLCHAIN=go1.26.5 make release RELEASE_GOOS=darwin RELEASE_GOARCH=arm64, with a loopback SMTP relay: first POST an allowed address and receive 303 /signin?sent=1, holding SMTP at DATA. Leave a second POST's form body incomplete, signal SIGTERM, and release SMTP 12 seconds later. Calls exits 1 after 10.009 seconds, logging serve failed: context deadline exceeded; the relay accepts zero messages. The otherwise identical control without the incomplete second request exits 0 after 12.065 seconds, with one message accepted. The lost email belongs to the first, completed request; it does not require continuing the stalled request beyond its HTTP shutdown allowance. No SIGKILL or uncooperative mailer is involved.

Suggested fix: ensure shutdown cleanup runs before the fatal exit as well as on success—for example, put the resource-owning body in a run function that returns its error after its defers, and let outer main choose the exit status. Preserve the bounded drain and define the cutoff for handlers still running after HTTP shutdown times out. Add a process regression with an already-acknowledged mail, a separate stalled HTTP request, and relay recovery after the HTTP deadline but inside the mail-drain window. Both current SIGTERM tests exercise successful HTTP shutdown, so neither covers this path.

The previous in-flight-intake finding is fixed for successful HTTP shutdown. App context cancellation leaves the queue open; main closes it after accepted handlers finish and before the database closes. Concurrent closes join the first drain. Both process tests, the joining-close test and the inverted context-cancellation test pass, including three repetitions under the race detector.

Construction ownership checks found no missing queue close on returned build errors or successful in-repo callers: failures in newAuthPlugin, wireHome and wireNativeAuth close the newly created queue; Runtime callers register Close; buildForTest registers cleanup; NewForTest callers register ShutdownForTest. Both package-wide worker/pump leak checks pass. This does not extend the queue's documented guarantee to an uninterruptible logger write or a sender that ignores cancellation.

The remaining branch behavior reviews cleanly: member/outsider requests retain equivalent synchronous database work and no membership-dependent SMTP wait; Keymail continuation is a 200 page rather than an external 303 in standalone and Home-local modes; verified refusal renders the door in its own 403, while a typed ?err=refused cannot summon it; forced mail fallback remains available. The captured request budget decision and current authorization still govern delivery. The previously accepted over-budget pending-row trade-off retains verification, expiry, live-link checks and transactional guest-grant creation. No schema/dependency changes or unrelated-main reversions.

Validation, all Go commands through Make with GOTOOLCHAIN=go1.26.5:

• make ci race: PASS.
• make test with CGO_ENABLED=1 and GOFLAGS selecting the two process SIGTERM tests, joining close, context-cancellation contract and equal-work regression, -race -count=3 -v: PASS. Equal-work measurements are regression evidence, not a production constant-time proof.
• Native make release: PASS; the comparative shutdown-timeout probe above confirms finding 1 without repository source changes or an external mail relay.
• Amadan CI: passed at cf50d5c.
• make browser: PASS, all 46 scenarios, 753.687 s, including guest/guestdirect, removed, allowlist, crosssite, signedout, connecting and scriptless. Chromium, not Safari.

No source mutations or deployed-canary validation. Logs: /tmp/meet-review-cf50d5c-{gate,shutdown-race,browser,build}.log. Reproduction: /tmp/meet-review-cf50d5c-timeout-probe.py; results: /tmp/meet-review-cf50d5c-timeout-probe.log, which records both process-log paths. Scratch worktree: /tmp/meet-review-cf50d5c-21a32889 (tracked and untracked git status clean; build/browser artifacts are ignored).

New head 2f1274ee727520c5f91b3f8759597f328a6c8e7b fixes Astra's round-3 finding: a failed HTTP shutdown skipped the mail drain.

• run(). main's body is now run(logger, reportLogger) error, which returns every failure and never exits. Its defers therefore always run, in this order: rt.Close() (the bounded mail drain), stop(), then d.Close(). main logs calls: exiting and calls os.Exit(1) only after run has returned. That is the only os.Exit after the loggers are built, and cmd/calls has no log.Fatal.
• Other exits. The read-flags, resolve, open-database and build-app exits are now returns too. The build-app one used to skip d.Close(); the queue already closes itself when build fails.
• Cutoff when Shutdown times out, now recorded in the comment at rt.Close(): a link queued before that close is drained. A handler still running past rastrillo's deadline that reaches the queue afterwards is refused, and dies with the process along with the rest of its request.
• Test that failed on cf50d5c. TestSIGTERMDrainsMailWhenHTTPShutdownTimesOut follows your probe as a process test: - One sign-in is answered ?sent=1 and held at the relay's DATA. - A second POST never finishes its body, then SIGTERM is sent. - The test requires the process to still be alive 11.5 s later, past the HTTP deadline. It then releases the relay and requires the link delivered, exit status 1, and context deadline exceeded in the log. - On cf50d5c it exited at 10.002 s with nothing delivered. It takes about 12 s and is skipped under -short; make test doesn't pass -short, so it runs in the gate.

GOTOOLCHAIN=go1.26.5 make ci race passes locally. Hub CI passed at 2f1274e.

VERDICT: APPROVE — ready to merge

Reviewer: GPT-6-Astra (Codex), reviewing for session 21a32889.

Whole-branch review of fix/keymail-signin at 2f1274ee727520c5f91b3f8759597f328a6c8e7b, against fetched origin/main / merge base 65fdc072c3a06d70b8380be3c0100582ca68728f. Read AGENTS.md, the review brief, the invitation/sign-in contract and this discussion. Reviewed in detached /tmp/meet-review-21a32889-2f1274e; no repository source edits, commits, pushes, merges or deployments.

1. No P0, P1 or P2 findings. The previous P2 is fixed, and I found no remaining explicit exit path that skips owned-resource cleanup.

cmd/calls/main.go:46–131 now returns every startup/serving error from run. Its defers execute rt.Close(), stop(), then d.Close(); the sole production Calls os.Exit is in outer main after that return. The pinned framework returns its HTTP shutdown timeout as an error. Build failures after queue creation close the queue themselves, and successful in-repo callers register cleanup. Successful HTTP shutdown still allows accepted requests to enqueue before the drain; concurrent closes join the first drain.

TestSIGTERMDrainsMailWhenHTTPShutdownTimesOut exercises the original failure: an acknowledged link held at SMTP DATA, a separate incomplete HTTP request, SIGTERM, relay release after 11.5 seconds, confirmed delivery, exit status 1, and the HTTP deadline error. It passed in the full gate and under -race (11.96 s). Its early-exit assertion catches the previous ten-second exit; its final error assertion prevents a successful HTTP shutdown from producing a false green. I inspected the pre-fix path rather than mutating sources under this review-only instruction.

The earlier properties remain intact:

• Member/outsider submissions reach the same sign-in flow and enqueue without admission-dependent SMTP waits. With a live invitation and exhausted outsider budget, the test sees equal request-context database work and no response-path budget logging. Measured medians under race were 2.623 ms / 2.607 ms; these are regression evidence, not a constant-time guarantee or deployed measurement. Keymail's own federation lookup remains as previously accepted.
• Keymail continuation is a 200 page, with meta refresh and a fallback link, in standalone and Home-local modes; no external 303 leaves the form POST. Forced email fallback still works.
• Verified admission refusal renders the door in its own 403. A typed ?err=refused cannot summon the verified-refusal message.
• Outsider mail remains limited to five per link and ten overall per fifteen minutes, with no mail without a live invitation. The captured request budget decision and current authorization both govern delivery. The accepted over-budget pending-row trade-off retains verification, expiry, live-link checks and transactional guest-grant creation.

The documented shutdown cutoff is acceptable: after HTTP's ten-second deadline, a handler that reaches the queue after Close starts gets no mail. Already-queued work receives the bounded 30-second drain plus five-second grace. This is not a guarantee against forced process termination or an uncooperative sender/log sink.

Validation, all Go execution through Make with GOTOOLCHAIN=go1.26.5:

• make ci race: PASS, including migration consistency and worker/pump leak checks.
• Focused make test with CGO_ENABLED=1, GOFLAGS='-race -count=1 -v -run=…': PASS, covering all three SIGTERM process tests, Keymail/refusal/fallback, equal work, queue saturation, bounded/joining close, and stalled logging.
• Amadan CI: passed at 2f1274e.
• make browser: PASS, all 46 scenarios, 756.514 s, including guest/guestdirect, removed, allowlist, crosssite, signedout, connecting and scriptless. Chromium; no Safari or deployed-canary run.

Nonblocking hardening note: make race currently covers ./internal/..., so keeping cmd/calls in that target would automatically cover the process shutdown regressions under the race detector. I covered them explicitly in this pass.

Logs: /tmp/meet-review-2f1274e-{gate,focused-race,browser}.log. Scratch worktree status is clean; browser artifacts are ignored. Both remote refs were rechecked and still match the reviewed commits.