fix(acp): back off before retrying adapter startup

[Premshay]

Problem / Motivation

An ACP adapter self-update can briefly leave its executable missing or locked while it is replaced in place. The immediate startup retry repeats that race.

Why it matters

A recoverable adapter replacement can be reported as an unusable ACP backend even though waiting briefly permits the next spawn.

What changed

Retry one spawn-level OSError after a two-second delay. Existing timeout and ACP-error retry timing remains unchanged.

Tests

python -m pytest test/test_acp_client.py -n0 -q — 604 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.

Pattern harvest

Not generalizable: this is a bounded adapter-startup retry at a specific process-creation boundary.

[github-actions]

Design Review (Fable 5, fork) — ✅ PASS

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

The change is small and well-scoped; I've read the retry loop in ensure_ready and the surrounding code. One fidelity note worth surfacing, otherwise sound.

Design-Verdict: PASS

A real spawn race, mitigated in the retry loop that already owns recovery — proportionate, reversible, and contract-preserving (still one retry).

Suggestions

• The description says "spawn-level OSError," but the except spans the whole attempt — _discard_bound_workspace, _discard_claude_settings_seed, and _initialize_session OSErrors (e.g. pipe/disk failures) now also get the spawn-tuned 2s backoff-and-respawn; narrowing the catch (or errno check) to the _spawn() call would keep the fix as scoped as it is described.

[DESIGN-REVIEWED] 76bf952

[github-actions]

GPT 5.6 Review (fork) — ✅ no blocking findings

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

Review details

FINDING -- src/kiro_crew/acp/client.py:5532 -- "OSError" also delays and retries initialization failures, exceeding the stated spawn-only scope -> Fix: re-raise when the subprocess was successfully spawned.
[GPT-REVIEWED] 76bf952

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

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

Review details

No findings.

The sole candidate is the widened except (AcpTimeoutError, AcpError, OSError) with a 2s backoff. Its worst case — a permanently-missing binary (FileNotFoundError) taking ~2s longer to surface and spawning once more — is a bounded latency/UX cost, not an observable wrong result: the retry is capped at range(2), the sleep fires only on attempt == 0, and the failure still propagates on attempt 1. That is the change's intended behavior (retry the one permitted respawn past the self-update window), so (c) "observable wrong outcome" does not hold. No correctness, crash, data-loss, or security consequence. It falls below the 80 bar and into no blocking class.

[OPUS-REVIEWED] 76bf952

[github-actions]

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

Premise-level review of 76bf95256465ca8bd74ad0219d36da4d750c1e54 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.

All evidence is gathered. The change is a small, well-grounded fix; my findings are about breadth of the catch and one unfixed sibling.

First-Principles-Verdict: CONCERNS

The backoff earns its place, but the catch is wider than the "spawn-level OSError" the description sells, and the same race is left live in runtime.py.

What this change ships

Intent: recover adapter startup when a self-update briefly removes or locks the adapter executable — a FIX.

1. Startup that hit a briefly-missing/locked adapter now recovers instead of declaring the backend unusable — justified
2. That one retry waits two seconds instead of respawning immediately — justified
3. Any OSError in the whole startup attempt (session init, workspace teardown) is now retried, not just the spawn — undeclared
4. New private constant _ACP_RESPAWN_BACKOFF_S — justified (1 consumer, not generalized)

Watch

• Description says "Retry one spawn-level OSError", but the except (AcpTimeoutError, AcpError, OSError) wraps the entire attempt body — _initialize_session, _discard_bound_workspace, _discard_claude_settings_seed all sit inside it. The base file itself relied on the old narrowness: client.py:4620-4624 says "it only catches AcpTimeoutError / AcpError … an OSError from the executor or a wedged file lock sails straight past it" — now false and left standing.
• Unfixed sibling: the same adapter binary is spawned at acp/runtime.py:1336 (shared runtime / subagents) with no retry or backoff at all. Grepped create_subprocess_limited under src/kiro_crew/acp/: 2 spawn sites, 1 fixed. A general fix is larger than this change — accepted-and-deferred, but the fix's stated harm ("unusable ACP backend") still occurs on that path.

Subtractions

• Shrink the OSError catch to wrap only await self._spawn() (client.py:5506) — the described harm is spawn-level; the attempt-wide catch is breadth nobody declared or justified.

[FIRST-PRINCIPLES-REVIEWED] 76bf952

[iamwhatever]

Tier 1 auto-approve: fix (2 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: adapter startup raised OSError (FileNotFoundError) when a self-updating ACP binary was replaced mid-spawn; the single permitted retry now catches OSError and sleeps _ACP_RESPAWN_BACKOFF_S before respawning, plus one covering test. 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.