fix(connections): isolate warm mint agent specs per process

[pepmach]

Problem / Motivation

Connections warm mint modes were written into the user's global agent directory. That exposed internal process-only modes in the user agent roster and left global artifacts behind when a helper or gateway crashed.

Why it matters

Internal warm modes are runtime implementation details, not user-selectable agents. Publishing them globally pollutes discovery and allows stale files to outlive the process that owns their OAuth sessions, making the visible roster and the actual runtime lifecycle disagree.

What changed (motivation → approach → change)

The warm helper now gives every ACP process a random, owner-only generation root under the protected Connections run tree and writes both warm modes to that root's project-local .kiro/agents scope. The generation root is also the runtime working directory, so the initial --agent spawn and later session mode selection resolve the same private specs without publishing either mode globally.

Each generation carries an ownership marker with gateway/runtime PIDs and process-start identities. Confirmed process teardown releases only that generation; parked, failed-to-kill, and otherwise ambiguous generations are retained. Startup schedules a tracked off-loop scavenger without delaying listener binding, and removes a private generation only when both recorded identities are provably dead. Graceful cleanup retires live and parked generations in both full-dashboard and headless API gateways.

Startup also removes sentinel-owned legacy global warm specs. Unsentinelled legacy files remain untouched and are documented for manual inspection because their names alone cannot prove ownership. Internal generation scopes remain excluded from discovery and agent pickers.

Tests

• Added/updated warm-engine tests for private project scope, initial/later mode resolution, per-generation ownership, confirmed-kill cleanup, failed-kill retention, fail-closed scavenging, legacy cleanup, and discovery exclusion.
• Added dashboard lifecycle coverage proving both gateway modes schedule scavenging without blocking startup and retire the warm singleton on cleanup.
• Final affected backend slice: 383 passed, 2 skipped.
• Full frontend Vitest: 28,472 passed across 1,805 files; scoped cross-surface guard: 5,682 passed across 208 files.
• Electron: 1,587 passed, 1 skipped. Production/analyze builds, TypeScript, ESLint, i18n, phantom-class, duplication, and bundle-size gates passed.
• Full isort, flake8, mypy (1,282 source files), baseline-aware Black, docs, encoding, SDK-boundary, async-I/O, harness-parity, feature-map, vendor, scrub, and related repository gates passed.
• The full backend suite collected 84,378 tests. Its remaining 21 failures reproduced unchanged on clean current main in the same synthetic HOME/temp environment, so they are not PR-11 regressions.

Manual verification

N/A — this is backend/runtime lifecycle behavior with no visual surface. The automated tests exercise filesystem placement, process ownership, cleanup, startup scheduling, and both gateway registrations directly.

Related Issues

no linked issue: this is PR-11 in the pre-G2 Connections hardening sequence.

Pattern harvest

Rule candidate: review-prompt
Pattern: Process-only agent specs must use a process-owned project scope, never a globally discovered user-agent scope.

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 9cf5b620e1c179ac65be08fc8ac7d02bc7d9a646 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

Moves process-private specs to a process-owned scope with fail-closed, PID-reuse-proof lifecycle — root-cause fix, proportionate machinery, reversible, boot-path and injection fences preserved.

[DESIGN-REVIEWED] 9cf5b62

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of 9cf5b620e1c179ac65be08fc8ac7d02bc7d9a646 — 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 checks complete. Producing the review.

First-Principles-Verdict: CONCERNS

Cause-level fix for warm modes, but the cold mint engine still publishes process-only specs into the same global roster this PR calls pollution.

What this change ships

Intent: stop internal warm-mint modes appearing in the user's agent roster and outliving their process — a FIX.

1. Warm modes vanish from the user's global agent directory — justified (cause-level: removes the shared namespace that forced the ownership-defense pile)
2. Each warm process runs from a random owner-only folder under the protected run tree — justified (documented run/ sensitivity fence)
3. Folder carries gateway+runtime PID/start-time ownership record — justified (PID-reuse guard is the documented platform_compat pattern)
4. Folder deleted only on proven process death; parked/unkillable keep theirs — justified, declared
5. Startup scavenges dead leftovers in background, strictly post-bind — justified (no-new-work-on-gateway-boot-path)
6. Gateway shutdown now retires the warm process in both gateway modes — declared, justified (new dirs would otherwise persist to next boot)
7. Startup auto-removes sentinel-owned legacy global specs, keeps unsentinelled ones — justified migration, declared
8. New bounded reader for private-scope specs (discovery reader refuses run/ by design) — justified
9. Design note updated in the same commit — mandated by spec-management invariant

Watch

• The PR's own harvest rule — "Process-only agent specs must use a process-owned project scope, never a globally discovered user-agent scope" — has 1 counted unfixed sibling: cold mint still writes kirocrew-mint-<alias>-<pid>-<hex>.json into _agent.kiro_agents_dir_path() (mint.py:392), and no filter for that prefix exists outside connections/ (grepped kirocrew-mint|MINT_AGENT_PREFIX across src/, website/src/, dashboard: 0 hits), so those specs are roster-visible for a mint's lifetime. Larger fix (manifest + per-flow lifecycle) — accepted-and-deferred, but the sequence should name it.

Subtractions

• Drop _WARM_GENERATION_MARKER_VERSION — one value ever written/checked (writer _warm_generation_owner, reader _read_warm_generation_owner, warm.py); _WARM_GENERATION_SENTINEL already keys the format and a future format can change the sentinel string.

[FIRST-PRINCIPLES-REVIEWED] 9cf5b62

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 9cf5b620e1c179ac65be08fc8ac7d02bc7d9a646 — this comment is updated in place on each push.

Review details

No findings block the merge.

FINDING — src/kiro_crew/connections/warm.py:1516 — When runtime.spawn() times out/raises/is cancelled after the runtime is constructed but before _bind_warm_generation, the owner marker still carries the provisional "runtime_pid": 0; _abandon_spawn_locked then calls _release_runtime_generation(runtime), whose _recorded_runtime_is_dead returns False for pid <= 0 (warm.py:514), so the tree is not removed even though _kill_quietly just confirmed the kill — and _scavenge_warm_generation_dirs computes runtime_live = _process_identity_live(0, "") = None, so runtime_live is not False keeps the directory at every subsequent gateway start, contradicting the docstring's "clutter until the next gateway start" and leaking a generations/generation-*/ tree per spawn failure → Fix: in _abandon_spawn_locked's confirmed-kill branch (or _release_runtime_generation) remove the directory directly when the marker is still provisional and the kill has just been confirmed, since ownership was proven by construction in this call.

[OPUS-REVIEWED] 9cf5b62

Verdict parsed from the review's SHA-scoped output markers for commit 9cf5b620e1c179ac65be08fc8ac7d02bc7d9a646.

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

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 9cf5b620e1c179ac65be08fc8ac7d02bc7d9a646 and found no blocking issues.

This comment is updated in place on each push.

Review details

FINDING -- src/kiro_crew/connections/warm.py:1516 -- discarded _release_runtime_generation results here and at lines 1754/1792 leave a surviving helper protected but untracked until gateway exit -> Fix: retain the runtime/pending entry whenever release returns false.
[GPT-REVIEWED] 9cf5b62

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

[pepmach]

• Fixed — the gateway token was written in a format the reader can never compare equal to (span=0e5c8da823a6)

"gateway_started": platform_compat.own_process_start_time() writes the gateway token in a different format than the reader compares it in

Correct, and the divergence is total rather than intermittent. own_process_start_time() resolves to _own_identity_token, which returns f"{ticks}:{boot}" on Linux and a proc_pidinfo microtime on macOS, while _process_identity_live recomputes process_start_time(pid) — bare /proc field 22, and a 1-second ps -o lstart= string, respectively. Those values cannot be equal on either platform. They coincide only on Windows, where _own_identity_token delegates to process_start_time itself, which is why the defect is invisible on that one platform.

What made this worth fixing rather than noting is that it inverts a safety property instead of merely losing a signal. _scavenge_warm_generation_dirs removes a generation only when gateway_live is False and runtime_live is False. A gateway that cannot prove its own liveness therefore forfeits its half of that guard, so as soon as the short-lived runtime exits, the directory the still-running gateway owns becomes eligible for deletion underneath it. The degraded read is unaffected and stays safe: an unreadable token is stored as "", which _process_identity_live reports as unprovable, and an unprovable identity keeps the directory.

_warm_generation_owner now records process_start_time(os.getpid()) — the same helper the reader recomputes — and its docstring states why the neighbouring helper is not interchangeable at this call site, so the two identities in that record are no longer taken from different sources.

Pinned red-first; both tests fail on the prior head and pass on this one:

test_recorded_gateway_identity_reads_back_as_live asserts the token the writer stores reads back as live for this very process, and failed with assert False is True. test_scavenging_keeps_a_generation_whose_gateway_is_still_alive asserts a live gateway vetoes removal once its runtime is dead, and failed with assert 1 == 0 — one directory belonging to a live gateway was actually deleted.

Neither test stubs _process_identity_live. The pre-existing scavenging test replaces that function outright, which is exactly why the suite could not see a wrongly formatted token reaching it.

Verified on f4bf21348: 203 tests across the warm, handoff and dashboard-server suites, full mypy (1282 files), black, isort, flake8 7.1.0, and docs lint. The rebase onto 5df44de2f was content-preserving (identical git patch-id).

[pepmach]

• Fixed — stored process_start_time(os.getpid()), the remedy as prescribed (span=b23252488e85)

own_process_start_time() is verified using the incompatible process_start_time(pid) format, deterministically classifying live Linux/macOS gateways as dead -> Fix: store process_start_time(os.getpid()).

Adopted exactly as written. _warm_generation_owner now records platform_compat.process_start_time(os.getpid()), so the gateway identity is produced by the same helper _process_identity_live recomputes on read, and it matches how the sibling runtime_started field was already being recorded in that same dictionary.

"Deterministically" is the right word, and the platform split is worth stating precisely: own_process_start_time() answers f"{ticks}:{boot}" on Linux and a proc_pidinfo microtime on macOS, against bare /proc field 22 and a 1-second ps -o lstart= string from the reader. Equality is impossible on both. Windows is the exception — there _own_identity_token delegates to process_start_time, so the same code path happens to be correct, which is why no existing platform test caught it.

The severity comes from which direction the misclassification falls. _scavenge_warm_generation_dirs deletes only when gateway_live is False and runtime_live is False, so a live gateway reading as dead does not make cleanup too timid — it removes the gateway's veto, and the generation directory still in use is deleted once its short-lived runtime exits. An unreadable token remains safe, being stored as "" and read as unprovable, which keeps the directory.

Pinned red-first; both tests fail on the prior head and pass on this one:

test_recorded_gateway_identity_reads_back_as_live asserts the stored token reads back live for this process, and failed with assert False is True. test_scavenging_keeps_a_generation_whose_gateway_is_still_alive asserts a live gateway with a dead runtime keeps its directory, and failed with assert 1 == 0 — the live gateway's directory was deleted. Neither stubs _process_identity_live, unlike the pre-existing scavenging test, which replaces it and so cannot observe the token format at all.

Verified on f4bf21348: 203 tests across the warm, handoff and dashboard-server suites, full mypy (1282 files), black, isort, flake8 7.1.0, and docs lint. The rebase onto 5df44de2f was content-preserving (identical git patch-id).

[pepmach]

• Fixed — the post-kill release now requires positive proof the runtime is dead (span=49467a2ab02f)

A surviving runtime loses its private generation. Signal-delivery failure -> AcpRuntime.kill() logs the surviving PID but returns -> _kill_quietly() reports success -> the live runtime's cwd/specs are deleted.
Fix: Require the marker's runtime identity to be provably dead before removing the direct[ory]

Correct, and AcpRuntime._kill_inner documents the exact step the chain turns on. After the SIGTERM/SIGKILL escalation it checks pid_exists(pid) and, for a survivor, logs a warning and returns normally — with the comment that both kill_process_tree calls swallow OSError by design, so "a signal-delivery failure (EPERM through a launcher wrapper, pgid drift) look[s] identical to success", and that the PID must stay tracked for a sweep. Nothing raises, so _kill_quietly reports True, _kill_generation treats that as a confirmed kill, and _release_runtime_generation reaches shutil.rmtree on the cwd and private .kiro/agents scope of a process still reading them. The detection already existed one layer down; this module simply never consulted it.

_release_runtime_generation now gates on the ownership marker's own runtime identity through a new _recorded_runtime_is_dead, which mirrors the scavenger's is not False test: only positive proof of death releases a tree, so both "still running" and "cannot tell" keep it. Returning False leaves the tree attached, and startup scavenging reclaims it once both recorded identities are dead — so an unkillable child costs clutter until the next gateway start instead of costing a live process its specs.

Placement is deliberate. Of the three _remove_warm_generation_dir call sites, the scavenger already requires both identities dead, and the spawn-abandon path reaches it only in its runtime is None branch, where no runtime exists to protect and a gate would leak every abandoned directory. Only the post-kill path needed the proof, and both callers that confirm a kill route through it.

Pinned by two tests, deliberately paired:

test_release_keeps_the_tree_when_the_killed_runtime_survived fails on the prior head with assert True is False — release reported success and would have deleted the tree. test_release_removes_the_tree_once_the_runtime_is_provably_dead passes on both heads by design: it guards the opposite error, since a gate refusing whenever liveness was merely unproven would trade this deletion for a directory accumulating on every activation.

One pre-existing test needed its premise stated rather than weakened. test_a_spec_scope_is_retained_while_an_unkilled_child_still_needs_it assigns its fake runtime PID 6060 and stubs process_start_time for every PID, so with a real liveness gate the outcome depended on whether the host happens to be running a process at that PID — it is live on this machine (chronicled), which is what surfaced it. It now declares its child dead once the kill has actually taken, which removes the host dependency in both directions. _recorded_runtime_is_dead was also added to the module's pinned filesystem-helper set, since it reads the marker.

Verified on 2fb074a31664: 205 tests across the warm, handoff and dashboard-server suites, full mypy (1282 files), black, isort, flake8 7.1.0, docs lint. Rebase was content-preserving (identical git patch-id).

[pepmach]

• Fixed — both symbols now import at module scope instead of inside the hooks (span=c7137bad99df)

function-local from kiro_crew.connections.warm import ... violates top-level-imports -> Fix: resolve both symbols lazily without function-local imports.

Accepted, and the deferral turned out to buy nothing worth keeping. The rule's exception list covers TYPE_CHECKING, a documented circular import, and an optional dependency; my reason was none of those — it was avoiding the import side effect of pulling the mint table in when something merely imports the dashboard server. That benefit is narrower than it looked: _register_connections_warm_lifecycle is called unconditionally from both start_dashboard and start_api_server, and its on_startup hook fires immediately, so every real gateway boot imports connections.warm regardless. Only a process that imports the module without starting a server ever avoided it.

Hoisting is also safe rather than merely tolerable, which is the part I checked rather than assumed. connections/warm.py imports nothing from kiro_crew.dashboard, so there is no import-time cycle. The reverse direction does have one to be careful of — connections/ownership.py reaches dashboard.handlers.mcp through function-local imports — but warm does not import ownership, so this edge does not participate.

One test moved with the change, and it landed on the exact hazard the rule names third. TestConnectionsWarmLifecycle patched connections.warm.scavenge_warm_mint_artifacts; with the names bound in this module's globals at import time, that patch rebinds something nothing here reads, and the doubles were never called (assert [] == ['scavenge', 'shutdown']). The patches now target the dashboard module that performs the lookup, with a comment saying why, so the test fails loudly if the import ever moves back.

I took this on despite it being advisory and non-blocking because the PR is not green yet, so it costs no extra review round — and because the alternative was arguing for an exception the rule does not grant.

Verified on 5564b3a85b17: 205 tests across the warm, handoff and dashboard-server suites, full mypy (1282 files), isort, flake8 7.1.0, docs lint, and the black gate. test/test_dashboard_server_coverage.py is line 610 of .github/black-baseline.txt, so its pre-existing formatting is left for its own commit rather than mixed in here.

[pepmach]

• Accepted as follow-up — provisional-marker generations survive scavenging (span=0e5c8da823a6, warm.py:1446, advisory, non-blocking)

a runtime.spawn() failure leaves the marker provisional (runtime_pid: 0), and _process_identity_live(0, "") answers None, so scavenging retains the directory forever; repeated spawn failures accumulate generation dirs.

The mechanism is real and the reading matches the code: an unprovable identity keeps the directory, by the fail-closed rule this PR deliberately adopts (unreadable or ambiguous ownership must never delete). The consequence is bounded clutter under run/connections-warm/generations, not a credential or correctness risk. The prescribed fix (the confirmed-kill abandon path removes a never-bound provisional directory directly, as the runtime-is-None branch already does) is sound and small, but this head is otherwise green with zero blocking findings across every lane, so per the repository babysit convention the head is frozen and the fix lands in the next Connections slice (warm-rearm), which edits the same file.

[pepmach]

• Fixed — every generation directory level is now fail-loud owner-only via restrict_dir_to_owner (span=49467a2ab02f)

ACL failures leave injectable agent specs

Accepted, and the escalation from the earlier advisory framing is warranted: the loose directory does not merely weaken a guarantee, it holds the agent SPECS a warm runtime mounts — writable specs in a world-accessible directory are an injection point into the agent, which is the exact isolation this PR exists to provide. make_owner_only_dir's tighten step is best-effort by contract, so it alone could not carry that guarantee.

_create_warm_generation_dir now re-tightens all four levels — the generations root, the generation dir, .kiro, and .kiro/agents — with the fail-loud platform_compat.restrict_dir_to_owner after creation. A refused ACL raises; the existing BaseException cleanup removes the loose directory; the caller gets an error instead of a false private scope. The red-first test (test_generation_creation_refuses_a_dir_whose_acl_tighten_failed) patches restrict_dir_to_owner to raise and asserts creation raises AND nothing survives on disk; it failed DID NOT RAISE against the prior code.

This supersedes my earlier same-mechanism advisory record (span b23252488e85, "accepted as follow-up"): the re-roll's blocking severity is the correct reading, so the fix landed now rather than in the follow-up slice. The follow-up still carries the Opus abandon-path advisory (span 0e5c8da823a6).

Verified on 9cf5b620e1c1: 207 tests across the warm, handoff and dashboard-server suites (one new), full mypy (1288 files), isort, flake8, black gate. Single commit, rebased onto f59b865bbd4e.

[buluoray]

Reviewed against source at 9cf5b62, with the guards mutation-tested and all 209 deletions individually accounted for. 0 blocking findings.

The sharing was real. Pre-fix (git show f59b865bbd:…/warm.py), both warm modes wrote specs to _agent.kiro_agents_dir_path() — the user's global ~/.kiro/agents, shared by every process and scanned by agent discovery. It polluted the user's agent roster and left global files outliving the process that owned their OAuth sessions.

The isolation key is the right one. Each ACP process gets run/connections-warm/generations/generation-<uuid4hex>/ as its cwd, with specs in that dir's project-local .kiro/agents, which kiro-cli resolves ahead of the global scope for both the initial --agent spawn and later session/set_mode. Deliberately not a PID, so OS PID reuse cannot collide dir names; PID reuse is instead handled in the ownership marker via platform_compat.process_start_time identity plus the pid_liveness tri-state, so a reused PID reads as dead rather than live.

Credentials and platform, the two things I most wanted to check: the generation dir holds agent specs only (model / allowedTools / mcpServers), not tokens — the OAuth cache is kiro-cli's separate store. It is owner-only via fail-loud platform_compat.restrict_dir_to_owner re-tightening all four levels after the best-effort make_owner_only_dir, with BaseException cleanup, so an ACL-write failure raises rather than leaving world-readable and injectable specs. Every process/lock/identity/ACL call routes through platform_compat; the diff adds no bare os.chmod, fcntl, os.kill or resource use.

Cleanup is three-layered: removal once the runtime's death is proven, a graceful shutdown_warm_mint hook wired into both start_dashboard and start_api_server, and a post-bind boot scavenge that deletes only generations whose gateway and runtime identities are both provably dead. The scavenge is scheduled strictly after _start_site with the connections.warm import deferred into the worker thread, so the no-new-work-on-boot-path rule holds.

Mutation results:

Mutation	Result
_recorded_runtime_is_dead: pid<=0 → return True	RED
Scavenge drops the gateway-liveness veto	RED
_warm_generation_agents_dir → global kiro_agents_dir_path()	3 RED (private-scope, discovery-exclusion, both-resolve)
Delete all restrict_dir_to_owner calls	RED (DID NOT RAISE)

Deletion accounting — every one is a move or a strengthening, none drops a guard. The global _write_warm_mint_specs / _unowned_plan_specs / _remove_warm_mint_specs become the same functions taking an explicit agents_dir, fed the private scope with ownership/sentinel gating preserved. The retire-time global spec sweeps in _abandon_spawn_locked, _kill_generation and shutdown become per-generation _release_runtime_generation, which is strictly stronger: it proves runtime death before removing, where the old path swept globally and unconditionally. The "never strand a still-running parked process" rule is preserved and tightened. No changed-signature symbol has a stale caller.

Non-blocking:

1. The residual leak the blocking bots found and you accepted as a follow-up is real, and I concur it is non-blocking: on a spawn failure where the runtime dies before _bind_warm_generation (warm.py:921, the if pid <= 0: return False branch) the marker keeps runtime_pid: 0, so _recorded_runtime_is_dead returns "unproved" and _process_identity_live(0, "") returns None at every future boot — the dir persists indefinitely. It is error-path-only (per spawn failure, not per operation), the artifacts are small owner-only dirs, and there is no credential or correctness exposure. It is the one thing keeping this from pristine.
2. Cold mint still writes kirocrew-mint-<alias>-<pid>-<hex>.json into the global agents dir (mint.py:392) with no roster filter — the same pollution class, pre-existing, correctly out of scope here.

Not verified: no test spawns two real OS processes racing on the generations root — all 26 tests run in one pytest process and simulate the second via marker files with synthetic PIDs and start-times. That is the practical way to test it and the marker-identity logic is exercised directly and proven load-bearing by mutation, but the genuine concurrent-process case rests on platform_compat's own tests. Linux only; Windows/macOS ACL and start-time behavior untested here.