fix: count fresh eager-spawned sessions against the live-population cap - #8835
Conversation
The armed-prefetch registry and its evict-oldest population cap (_RESUME_PREFETCH_MAX_LIVE) only registered speculatively RESUMED sessions, so fresh eager spawns - slot create, agent reset, relaunch, project set - accumulated one live-but-unclaimed agent process per slot, each with its own MCP servers, bounded only by the 30-minute idle sweep. The spawn semaphore does not help: it gates concurrent handshakes, not accumulated live processes, and sequential slot signals pass it trivially. Register every successful speculative registration in the existing registry so arming beyond the cap evicts the oldest unclaimed session via the conditional remove_if_unclaimed. The TTL stays resume-only: fresh sessions hold no native per-session lock. A lost same-key race (is_new=False) still never registers - that session belongs to a real creator.
GPT 5.6 Review (fork) — ✅ no blocking findingsReviewed Review detailsNo findings. |
Design Review (Fable 5, fork) — ✅ PASSDesign-level review of Design-Verdict: PASS Wires fresh spawns into the already-proven registry instead of duplicating machinery; conditional evict keeps claimed sessions safe, and the invariant (unclaimed ≤ cap) holds. I verified in the base [DESIGN-REVIEWED] ef303b8 |
First Principles Review (Fable 5, fork) — ✅ PASSPremise-level review of All evidence verified against the base tree. Every speculative session flows through the single First-Principles-Verdict: PASS Closes the one un-wired arm of an already-shipped population cap by reusing the existing registry: no new surface, no new constants, cause-level. What this change shipsIntent: stop idle-but-unclaimed pre-warmed chat sessions from stacking one agent process (plus its MCP servers) per tab. This is a FIX.
Verified mechanics: base [FIRST-PRINCIPLES-REVIEWED] ef303b8 |
Opus 4.8 Review (fork) — ✅ no blocking findingsReviewed |
iamwhatever
left a comment
There was a problem hiding this comment.
Tier 1 auto-approve: fix (3 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: fresh eager-spawned sessions were never registered in the live-population cap registry, so sequential slot signals (create, agent/project set) stacked one unclaimed agent process per slot until the idle sweep; the fix lifts the _cap_armed_prefetches call out of the resume-only branch so fresh and resumed sessions are capped alike. Spec files changed as a ride-along (a minority of the diff on both file count and changed lines), not reviewed as a design decision: docs/system-specs/modules/learn-cron-dashboard.md. CodeQL is not applicable on this fork PR (default-setup emits no check-run); SAST coverage is Semgrep only, latest run success with 0 annotations.
Problem / Motivation
session.eager_spawn(on by default) speculatively creates a slot's sessionahead of its first message, from five trigger sites: slot create, agent reset,
relaunch, project set (
chat_handlers.py), and slot focus (ws.py, the onlyresume path).
The eager-spawn machinery bounds two resource dimensions but not the third:
_EAGER_SPAWN_MAX_CONCURRENT = 2) — its owncomment says it bounds the burst, i.e. in-flight handshakes only.
(
_RESUME_PREFETCH_MAX_LIVE = 3, evict-oldest via the_armed_prefetchesregistry) and TTL'd (600s).
_eager_spawnregisterspopulation accounting only inside
if allow_resume and resumed:, so afresh session is reaped by nothing but the 30-minute idle sweep.
A user who creates or reconfigures K slots within that window accumulates K
live-but-unclaimed agent processes. The concurrency semaphore does not help:
sequential slot signals pass it trivially. And each unclaimed session is not
one process — each session spawns its own full set of M configured MCP
servers (#3259 measured 6), so K idle tabs cost roughly K x (1 + M)
processes doing nothing.
Why it matters
Baseline memory pressure is a live user complaint (#5033: 5+ GB around one
chat on a 16 GB machine; #3259: CPU/RAM saturation from per-session MCP
process fan-out). Unclaimed speculative sessions add to exactly that baseline
without the user ever sending a message. The repo already decided a bounded
unclaimed population is the right shape — this change closes the gap between
that decision and the fresh-spawn path.
What changed (motivation → approach → change)
Observed symptom: create several chat tabs (or re-point a few tabs' agent or
project) and idle — one full kiro-cli process per tab stays alive for up to
30 minutes, none of them claimed by any turn. The registry that exists to
bound exactly this population stays empty (reproducer output below).
Root cause:
_cap_armed_prefetchesis wired only into the resumed-prefetcharm of
_eager_spawn's tail, so fresh speculative sessions never enter thelive-population accounting.
Change: register EVERY successful speculative registration in the existing
registry, keeping the TTL resume-only (fresh sessions hold no native
per-session lock, so the idle sweep remains their backstop — the cap just
bounds how many can pile up):
Eviction stays conditional (
remove_if_unclaimed): a claimed session isnever touched and lazily falls out of the accounting — semantics already
proven by the shipped resume-prefetch cap and its tests. A lost same-key
race (
is_new=False) still never registers; that session belongs to a realcreator and must not occupy unclaimed accounting.
Alternatives considered and rejected:
two constants drift independently; the existing registry's semantics
(insertion-ordered, evict-oldest-unclaimed, conditional remove) already fit.
fresh sessions hold no prior transcript's lock, which is what motivated the
TTL; the idle sweep already reaps them.
_RESUME_PREFETCH_MAX_LIVE: the name is now slightly narrow, butit is pinned by existing tests in two files; the comment block documents
the widened scope instead, keeping the diff minimal.
Tests
New
TestFreshSpawnPopulationCapintest/test_eager_spawn.py(3 tests),plus a module-level autouse fixture isolating the module-global registry
around every test so registrations from one test can never trigger a
spurious over-cap eviction in another.
Fails before (at
origin/main, fix reverted, tests kept):Passes after:
Full file and neighbours:
Local gates: mypy clean on the changed source file, flake8 clean, isort
clean,
scripts/check_black_formatting.pypassed,scripts/docs-lint.shpassed.
Manual verification
Not run against a live gateway; the mechanism is fully exercised by the
suite's existing mocked-session conventions (same approach as the shipped
resume-prefetch cap tests). The eviction path reuses
remove_if_unclaimed,whose live semantics are unchanged by this PR.
Related Issues
fan-out — the amplifier that makes unclaimed sessions expensive). Neither
is fully fixed by this change; it removes one unbounded contributor.
function (offloading the config load off the event loop); the hunks are
disjoint.
Checklist
feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)Pattern harvest
The pattern is a speculative-resource population where only in-flight concurrency is
bounded, not the accumulated live set. Harvested across the eager-spawn seam: all five
spawn triggers (slot create, agent reset, relaunch, project set, slot focus) funnel
through
_eager_spawn, so the fix lands once at the registration tail rather thanper-trigger. The resumed path already had population accounting
(
_RESUME_PREFETCH_MAX_LIVE+_armed_prefetches); the fresh path was the oneuncovered instance of the class at this seam. One adjacent candidate of the same
class exists on the warm-cache path (
chat_done-driven refreshes are notpopulation-capped); left out deliberately per one-topic-per-PR — happy to file it
separately if maintainers agree it is worth bounding.
Rule candidate: any speculative-resource spawn path must register in a bounded live-set
population cap (evict-oldest-unclaimed), not just an in-flight concurrency limit — audit
every trigger that funnels into the spawn seam, not only the one that surfaced the leak.