fix(sel): warm singleton at startup; drop per-site audit hops (#8608)

[bolichen97]

Problem / Motivation

SecurityEventLog._init_locked runs on whatever thread first calls sel() (blocking file I/O: trust-dir creation, HMAC key load/create, a tail read of the live log). On a fresh gateway, whichever handler audits first paid that init on the event loop — stalling every session's turn and the liveness heartbeat. The per-site fix, await asyncio.to_thread(lambda: _sel().log_api_access(...)), was replicated 17 times across the dashboard (four of them added by PR #8604), while 250+ other log_api_access call sites in src/kiro_crew remained candidate first-touch stalls.

Why it matters

Every new audit site is a latent loop stall unless its author remembers the wrapper — a class that grows with the codebase. The wrappers themselves add a suspension point and a worker dispatch per audit (some on deny paths where the surrounding code explicitly must not yield), and the members.py AST guard existed only to police the workaround.

What changed (motivation → approach → change)

Symptom → cause: the per-site hops only dodge the singleton's first touch; after init a non-critical log_api_access is an enqueue. So pay the first touch once, at startup, off the loop:

• sel.warm_sel_singleton() (new): await asyncio.to_thread(sel), best-effort. Awaited by BOTH start_dashboard and start_api_server right after await warm_auth_singletons() — before the middleware chain is built and before the socket binds, so no handler or middleware can be the first touch. Same shape and position as the auth-singleton warm precedent.
• 17 non-critical wrappers deleted (_shared, autonudge, connections ×5, cron ×2, members ×4, messaging, secrets, session_control handlers, server._audit_denied) — replaced with direct calls. Pre-existing per-site try/except guards are KEPT: construction can still raise after a FAILED warm (e.g. a trust root too short to sign the chain), and an unguarded audit would turn a 403 denial into a 500.
• 7 critical=True sites deliberately keep their to_thread wrappers (autonudge_authz ×2, taskrunner, cron ×3, md_notebook): a critical audit writes SYNCHRONOUSLY on its caller's thread by design (audit-or-deny), so their offload carries the write, not just the init. This is the issue's own "verify no site was relying on the wrapper for more than warm-up cost" caveat — the issue's "delete all 18" over-counted.
• log() writer-unavailable fallback made loop-aware (pre-push GPT review finding): the synchronous _flush_batch fallback was reachable ON the loop once the hops were gone. On the loop a non-critical event is now dropped with a warning (best-effort by contract; a frozen loop is not survivable); off the loop it still writes synchronously. Either way the failed enqueue's pending credit is returned so flush() cannot wait on a count nothing will decrement.
• Docs: docs/system-specs/modules/sel.md gains the startup-warm paragraph (including the failed-warm caveat).

Boot-path justification (AUTOSDE no-new-work-on-gateway-boot-path): the warm is one awaited asyncio.to_thread before readiness — the rule's explicitly allowed shape for init that must precede serving traffic. Worst-case bound on a large profile: one key-file read (or create), one backward tail read of the live log — a single 4 KiB chunk on a healthy log, worst case a full backward scan bounded by rotation's _SEGMENT_MAX_BYTES (32 MiB) when the tail holds no parseable record — and a stat. No cross-process chain lock is taken at init. A failed warm logs and never blocks readiness; the first later touch then retries init on its caller's thread (as every call site did before the per-site hops existed), and critical=True audits still fail closed at their own sites.

Pre-push review dispositions (GPT gpt-5.6-sol + Opus claude-opus-5):

• GPT "unbounded warm can hang boot on an unresponsive filesystem": rebutted — both start paths already await the unguarded, timeout-less warm_auth_singletons() (three to_thread hops reading token_signing.key under the same config_dir() data home) before this warm, so a hung data-home filesystem wedges boot at the existing precedent first; this warm adds no new failure mode, and the proposed dual-path mitigation would reintroduce the per-site complexity this PR exists to delete.
• GPT "sync fallback reachable on loop": real — fixed at root (loop-aware fallback above), mutation-verified.
• GPT "failed warm restores the first-touch stall": accepted residual, deliberately: it only arises when SEL storage is already broken, the retry cost is one bounded init attempt, and the alternative (warm-state machine gating every call site) is disproportionate. Documented at the helper, the doc, and the guards.
• Opus (PASS, 3 rationale-accuracy findings): all fixed — worst-case bound stated honestly (was healthy-case), sel.md absolute softened with the failed-warm caveat, "only enqueues" qualified with the writer's one-time start.

Tests

• test/test_sel_startup_warm.py (new): real SecurityEventLog init runs off-loop through the warm (migrated members.py denial audits initialize SEL synchronously on the event loop #8523 first-touch property); a failed warm is swallowed and logged; a source guard pins both start paths awaiting the warm before state.ready = True and before the middleware chain; the writer-unavailable fallback drops on-loop, writes off-loop, and leaks no pending credit.
• test/test_members_dm_thread.py: thread-ident tests flipped to pin the inline enqueue; the members AST guard is inverted (no log_api_access may hide inside a to_thread lambda).
• test/test_api_health.py: _audit_denied pins updated (direct call, still best-effort; boundary audit asserted on MainThread).
• Mutation-verified — each of these is caught by a distinct test: bare sel() in the warm, dropped except, warm removed from either start path, warm moved after readiness, a re-added per-site wrapper, the fallback's loop-guard dropped, the fallback made drop-unconditional.

Manual verification

N/A — unit coverage sufficient: the startup ordering, off-loop init, failure swallow, and fallback semantics are each pinned by an automated test; no UI or external service is involved.

Related Issues

Closes #8608
Refs #8523, PR #8604 (the four wrappers + AST guard + test this migrates)

Pattern harvest

Rule candidate: review-prompt
Pattern: "per-call-site thread hop to dodge a singleton's first-touch init — warm the singleton once at startup instead, and check whether any site's offload carries a synchronous write (critical=True) rather than just init cost"

Checklist

• At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable)
• No secrets, credentials, or internal references in the diff

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of 91ae2c8782f8fff175ed92111c6930788813b955 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

The design here is sound: this replaces 17 per-site symptom patches with a single root-cause fix at the correct layer, following an existing precedent in the same file.

Design-Verdict: PASS

Root-cause fix at the right seam: one startup warm retires a growing class of per-site thread hops, with critical-write offloads correctly preserved.

The failed-warm residual (first later touch retries blocking init on the caller's thread, potentially the loop) is disclosed, bounded to already-broken SEL storage, and the rejected alternative (warm-state gating every call site) would reintroduce exactly the per-site complexity being deleted. The newly reachable sync fallback was made loop-aware rather than left latent, and the seven critical=True sites keeping their offloads shows the deletion was audited per-site, not mechanical.

[DESIGN-REVIEWED] 91ae2c8

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 91ae2c8782f8fff175ed92111c6930788813b955 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 91ae2c8

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 91ae2c8782f8fff175ed92111c6930788813b955: <one-sentence reason>

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 91ae2c8782f8fff175ed92111c6930788813b955 — this comment is updated in place on each push.

Review details

Both candidates hinge on failure modes that do not occur in normal operation — Candidate 1 requires the SEL writer thread failing to start (FD/thread exhaustion), Candidate 2 requires the startup warm itself raising. Neither supplies a concrete input that occurs in practice; both describe documented, deliberate best-effort tradeoffs, and Candidate 2's exposure pre-existed at 250+ unwrapped sites. The pending-credit accounting (_decr_pending on the failed-enqueue path) and the _on_event_loop() drop branch are correct and pinned by new tests. Nothing survives falsification at the 80 bar, and I found no additional grounded defect.

No findings.

[OPUS-REVIEWED] 91ae2c8

Verdict parsed from the review's SHA-scoped output markers for commit 91ae2c8782f8fff175ed92111c6930788813b955.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 91ae2c8782f8fff175ed92111c6930788813b955: <one-sentence reason>

[github-actions]

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of 91ae2c8782f8fff175ed92111c6930788813b955 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All claims verified. I have everything needed to produce the review.

First-Principles-Verdict: PASS

Cause-level fix for a counted class (18+ per-site hops, 1070 latent sites): warm once at startup, delete the workarounds; every item earns its place.

What this change ships

Intent: stop the SEL singleton's first-touch file I/O from ever landing on the event loop, without a per-call-site workaround. FIX.

1. Gateway startup now initializes the audit log once, off the loop, before serving — justified (cause level; 2 consumers, both start paths)
2. 17 deny/audit sites lose their per-call thread hop and audit inline — justified subtraction
3. Writer-unavailable fallback now drops a non-critical event on the loop instead of writing inline — declared; a hazard item 2 made reachable, fixed at its mechanism
4. A failed enqueue returns its pending credit so flush() cannot hang — declared, rides along inside item 3, harmless
5. A failed warm degrades to the old retry-on-first-touch behavior — declared residual, proportionate
6. sel.md startup-warm paragraph — mandated (same-commit spec rule)

Watch

The description's "7 critical=True sites deliberately keep their to_thread wrappers" over-claims by one: md_notebook/server.py:1428 (the auto-sync disabled audit) keeps its wrapper with no critical=True. Sweep run: to_thread within 4 lines of log_api_access across src/kiro_crew — survivors are cron.py ×3, autonudge_authz.py ×2, taskrunner.py, md_notebook ×2; all carry critical=True except that one.

Subtractions

• Drop the asyncio.to_thread at src/kiro_crew/apps/builtins/md_notebook/server.py:1428 and call sel().log_api_access(...) directly — it is the same non-critical hop this PR deletes 17 of (verify first that the builtin runs in the warmed gateway process; if it does not, the warm does not cover it and the hop stays with a comment saying why).

[FIRST-PRINCIPLES-REVIEWED] 91ae2c8