docs(autosde): forbid new work on the gateway boot path - #2472
Conversation
Everything between process start and the dashboard socket accepting requests runs once, in order, on one thread. Every statement added there is paid by every user on every launch, and a startup that holds the loop long enough for the stall watchdog to fire turns a latency bug into a crash loop. The rule names the boot path precisely (GatewayOrchestrator.run up to KIROCREW_READY, start_dashboard up to _start_site, the _init_* and setup_*_routes reached from those, and cli.py before asyncio.run) and rejects six shapes, each drawn from a defect found in this codebase: a new awaited step before the socket binds, subsystem construction as a side effect of route registration, work that scales with user data, unconditional maintenance sweeps, eager import of a disabled subsystem, and reading a lazy-init accessor from boot code. It also states the accepted alternatives, so a reviewer can point at one instead of only rejecting: a memoized first-use accessor, a background task, an off-loop thread hop, or letting dependent routes 503 until ready.
Opus 4.8 Review — ✅ no blocking findingsReviewed Review detailsThe entire PR adds a single blocking AUTOSDE rule to No findings. [OPUS-REVIEWED] e36dc23 Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
GPT 5.6 Review — ✅ no blocking findingsGPT 5.6 completed its review of This comment is updated in place on each push. Review detailsNo findings. False positive or not applicable? A repository writer can comment: |
Design Review (Fable 5) — ✅ PASSAdvisory design-level review of Design-Verdict: PASS A real, incident-shaped risk (boot latency compounding into watchdog crash loops), addressed at the right layer with concrete anchors and an explicit escape hatch. Suggestions
[DESIGN-REVIEWED] e36dc23 |
What
Adds a blocking AutoSDE rule,
no-new-work-on-gateway-boot-path, toAUTOSDE.yaml.Why
Gateway startup on a real profile has grown to roughly 20s, and none of it is a
regression in the boot code — the boot statements are materially unchanged since
v0.1.2. What changed is that the data those statements traverse becamenon-empty. A cold boot on an empty
KIROCREW_HOMEstill reaches HTTP 200 in2.34s; the same code against a populated knowledge corpus spends ~4.55s on the
event loop before the socket binds, and three of its scans return zero rows.
That shape — a boot step that is free until it isn't, with no scaling guard and
no reviewer prompt to notice — is what this rule is for. The cost also compounds:
startup that holds the loop long enough for the stall watchdog to fire gets the
process killed and respawned into the same startup, converting a latency bug
into a crash loop.
The rule
It names the boot path precisely, so a reviewer can tell whether a diff is on it:
GatewayOrchestrator.run()up to theKIROCREW_READYprintstart_dashboard()up toawait _start_site(site, port)_init_*()orsetup_*_routes()reached from thosecli.py's gateway command beforeasyncio.run()Six rejected shapes, each drawn from a defect actually found in this codebase
rather than from principle:
setup_*_routes()that also builds a pipeline or opens a store drags thatinit onto the boot path where nobody reading the route file will find it.
because the measurement was taken on an empty profile.
or a migration that scans instead of checking a version.
Gate the import, not just the handler.
the lazy-init contract its author wrote.
It also lists the accepted alternatives — memoized first-use accessor,
asyncio.create_task,await asyncio.to_thread, or letting dependent routes503 until ready — so a reviewer can point at a fix rather than only object. If a
step genuinely must run at boot, the rule requires the PR to say why it cannot
be deferred and to state its worst-case bound on a large profile.
Scope
Judgment tier only, following the two-tier split this repo already documents in
test/test_no_blocking_call_on_loop.py: deterministic cases become a hard gate,judgment cases become an AutoSDE rule. "Does this step need to be on the
critical path?" cannot be decided statically, so it belongs here. A deterministic
companion is possible — AST-count the pre-bind statements in
run()/start_dashboard()and pin them in a baseline, in the style ofconfig-baseline.json— and is deliberately left for a follow-up.Testing
AUTOSDE.yamlparses; 5 rules present, the new one carries 4 file-patterns.vitest are unaffected. There is no schema test over
AUTOSDE.yaml— the onlytest-tree references to rule ids are
# loop-ok:suppression comments.Note on when it takes effect
All four review workflows extract rules from the base commit rather than the PR
HEAD (
git show "$BASE_SHA:AUTOSDE.yaml"), so that a PR cannot weaken the rulesit is reviewed against. This rule therefore does not gate its own PR; it starts
applying to PRs opened after it merges.
AUTOSDE.yamlis also in the:(exclude)list incode-review.yml, so the deterministic job skips it.