fix(acp): retry a transient shared runtime spawn

[Premshay]

Problem / Motivation

A Kiro CLI self-update can briefly replace the executable while AcpRuntime starts. The shared-runtime path treats that process-creation OSError as terminal.

Why it matters

The shared runtime serves interactive, scheduled, subagent, background, and review work. A short replacement race should not fail startup.

What changed

Retry one process-creation OSError after two seconds while retaining the cold-start permit. The retry occurs before runtime state is recorded, so partial initialization is never replayed. Authentication, protocol, and later initialization failures remain terminal.

Pattern harvest

Rule candidate: retry shared startup only at an atomic pre-state mutation boundary; holding admission does not make replaying partial initialization safe.

Tests

python -m pytest -q test/test_acp_runtime.py test/test_acp_spawn_offload.py — 323 passed.

What changed (motivation → approach → change)

N/A — covered by the existing ## What changed section.

Manual verification

N/A — focused automated coverage is sufficient.

Related Issues

N/A.

Checklist

• Existing tests pass and regression coverage is included.
• Self-review completed; code follows project style guidelines.
• Documentation updated where applicable.
• No secrets, credentials, or internal references in the diff.

Contribution License Agreement

N/A — template placeholder; no CLA wording is supplied.

[github-actions]

GPT 5.6 Review (fork) — ✅ no blocking findings

Reviewed b344bfbb7046bbd2f1ff29ef5232997d3f69f829 via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] b344bfb

[github-actions]

First Principles Review (Fable 5, fork) — 🟡 CONCERNS

Premise-level review of b344bfbb7046bbd2f1ff29ef5232997d3f69f829 via the fork AI-review pipeline — 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.

The review contract requires my final message to be the structured review itself, with nothing after the marker. All evidence gathered: the fix is real, the base confirms the added return in spawn() is behaviorally inert (the try/finally is the function's last statement), no existing generic spawn-retry mechanism exists, and I counted the sibling kiro-cli spawn sites the same race still hits.

First-Principles-Verdict: CONCERNS

An inert return rides along undeclared in spawn(), and the same replacement race stays terminal at the sibling AcpClient._spawn path.

What this change ships

Intent: keep a shared ACP runtime start from failing when a kiro-cli self-update briefly replaces the executable — a FIX.

1. Shared-runtime start survives one process-creation failure, retrying after 2s — justified
2. A permanently failing spawn now reports ~2s later (one extra attempt) — rides along, inherent to item 1
3. return added after outcome = "ready" in spawn() — undeclared, rides along, behaviorally inert
4. Private backoff constant _ACP_RUNTIME_RESPAWN_BACKOFF_S — justified (named constant, not a knob)
5. acp-client spec paragraph updated in the same commit — justified (AGENTS.md mandate)
6. Regression test for the retry — justified

Watch

• Point patch, partially declared: the root cause (in-place executable replacement) hits every kiro-cli spawn. Grepped create_subprocess_limited + _resolve_kiro_bin_for_spawn: 3 unretried sibling spawn sites — src/kiro_crew/acp/client.py:4576 (AcpClient._spawn, same startup semantics, same terminal OSError), plus low-stakes background scrapes at src/kiro_crew/dashboard/handlers/sessions.py:836 and src/kiro_crew/dashboard/handlers/agents.py:2114. The description scopes to "the shared-runtime path" but never says the client path is left unfixed.

Subtractions

• Delete the return added after outcome = "ready" in AcpRuntime.spawn() (src/kiro_crew/acp/runtime.py): the try/finally is the function's last statement, so the line changes nothing and is unrelated to the retry.

[FIRST-PRINCIPLES-REVIEWED] b344bfb

[github-actions]

Design Review (Fable 5, fork) — ✅ PASS

Design-level review of b344bfbb7046bbd2f1ff29ef5232997d3f69f829 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

Bounded, single-retry recovery placed at the one point before any runtime state is recorded — the right boundary, documented in the same commit.

Suggestions

• The return added after outcome = "ready" in spawn() is a no-op (nothing follows the finally); drop it or state what future code it guards.

[DESIGN-REVIEWED] b344bfb

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

Reviewed b344bfbb7046bbd2f1ff29ef5232997d3f69f829 via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] b344bfb