Skip to content

Commit 7be304f

Browse files
Trecekclaude
andauthored
[FIX] Rectify: Fleet Dispatch Resume — Centralized Precondition Chokepoint (#4214)
## Summary The new headless `fleet run --resume-session-id` CLI path (PR #4191) inherits the dispatch state machine but bypasses the campaign-level auto-reset precondition gate. When the prior dispatch is in `FAILURE`, `INTERRUPTED`, or `REFUSED`, the headless path reads `prior_session_chain` from the stale record, spawns a child subprocess, then `mark_dispatch_running` raises `ValueError` on the illegal `FAILURE→RUNNING` transition. The exception is swallowed in `_on_spawn` (`fleet/_api.py:153`), the child runs for ~44 minutes on top of a stale `FAILURE` record, and the eventual envelope reports `dispatch_status: "failure"` while real work was completed. Part A introduces a **single precondition chokepoint** (`prepare_resume`) that every resume entry point must call before spawning, **a fail-closed spawn path** that kills the child on transition failure, and **`MAX_CONSECUTIVE_RESUME_ATTEMPTS` enforcement on the headless path** so the existing campaign-level cap is not silently bypassed. Together these make the immediate bug class architecturally impossible rather than papering over the immediate crash. ## Implementation Plan Plan file: `/home/talon/projects/generic_automation_mcp/.autoskillit/temp/rectify/rectify_fleet-resume-precondition-chokepoint_2026-07-08_143000.md` Closes #4199 🤖 Generated with [Claude Code](https://claude.com/claude-code) via AutoSkillit <!-- autoskillit:pipeline-signature steps=prepare_pr,run_arch_lenses,compose_pr,annotate_pr_diff,review_pr --> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 7809e19 commit 7be304f

16 files changed

Lines changed: 1194 additions & 140 deletions

pyproject.toml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -365,6 +365,11 @@ ignore_imports = [
365365
# _dispatch_reaper calls kill_process_tree for orphan reaping; psutil is non-stdlib
366366
# so the reaper cannot live in core/. This mirrors the workspace path-utility exception.
367367
"autoskillit.fleet._dispatch_reaper -> autoskillit.execution",
368+
# _write_pid (the _on_spawn callback) calls kill_process_tree to fail-closed-spawn
369+
# when mark_dispatch_running raises an illegal transition. This is the IL-2 fleet-side
370+
# counterpart to the reaper's kill primitive; the import is deferred (function-body)
371+
# and deliberate. Do not expand _api's execution access beyond this single kill call.
372+
"autoskillit.fleet._api -> autoskillit.execution",
368373
# _run_dispatch enforces config-authoritative ingredient injection at dispatch time.
369374
# apply_config_authoritative_overrides lives in config/ to share resolve_ingredient_defaults
370375
# without duplication. The import is deferred (function-body) and deliberate — it is the

src/autoskillit/cli/fleet/_fleet_run.py

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,26 @@
1919
logger = get_logger(__name__)
2020

2121

22-
def _fleet_run_error(error: str, message: str, exit_code: int = 1) -> NoReturn:
23-
envelope = {"success": False, "error": error, "user_visible_message": message}
22+
def _fleet_run_error(
23+
error: str,
24+
message: str,
25+
exit_code: int = 1,
26+
*,
27+
dispatch_status: str | None = None,
28+
) -> NoReturn:
29+
"""Emit a CLI error envelope and exit.
30+
31+
Every envelope carries a ``dispatch_status`` field so downstream consumers
32+
can distinguish crash outcomes from real dispatch outcomes without parsing
33+
the message string. When ``dispatch_status`` is omitted, the default
34+
``"rejected"`` is used (no subprocess was launched).
35+
"""
36+
envelope: dict[str, object] = {
37+
"success": False,
38+
"error": error,
39+
"user_visible_message": message,
40+
"dispatch_status": "rejected" if dispatch_status is None else dispatch_status,
41+
}
2442
print(json.dumps(envelope))
2543
raise SystemExit(exit_code)
2644

src/autoskillit/fleet/__init__.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
``autoskillit.fleet``, not from sub-modules.
55
"""
66

7+
from ._api import DispatchSpawnFailed as DispatchSpawnFailed
78
from ._api import _build_capability_overrides as _build_capability_overrides
89
from ._api import _write_pid as _write_pid
910
from ._api import execute_dispatch
@@ -56,6 +57,7 @@
5657
DispatchStateHandle,
5758
DispatchStatus,
5859
GateRecordResult,
60+
ResumeCountExceeded,
5961
ResumeDecision,
6062
append_dispatch_record,
6163
build_protected_campaign_ids,
@@ -75,13 +77,20 @@
7577
write_captured_values,
7678
write_initial_state,
7779
)
80+
from .state_recovery import (
81+
MAX_CONSECUTIVE_RESUME_ATTEMPTS as MAX_CONSECUTIVE_RESUME_ATTEMPTS,
82+
)
83+
from .state_recovery import (
84+
ResumePreflight as ResumePreflight,
85+
)
7886
from .state_recovery import (
7987
classify_stale_dispatch,
8088
derive_orchestrator_resume_spec,
8189
find_completed_dispatch,
8290
find_dispatch_for_issue,
8391
has_blocking_dispatch,
8492
has_completed_dispatch,
93+
prepare_resume,
8594
resolve_stale_running,
8695
)
8796
from .state_types import (
@@ -138,9 +147,11 @@
138147
"DispatchRecord",
139148
"DispatchRejected",
140149
"DispatchResult",
150+
"DispatchSpawnFailed",
141151
"DispatchStateHandle",
142152
"DispatchStatus",
143153
"DispatchOutcome",
154+
"ResumeCountExceeded",
144155
"ResumeDecision",
145156
"GateRecordResult",
146157
"append_dispatch_record",
@@ -165,6 +176,9 @@
165176
"normalize_dispatch_token_usage",
166177
"classify_stale_dispatch",
167178
"FLEET_STATE_SCHEMA_VERSION",
179+
"MAX_CONSECUTIVE_RESUME_ATTEMPTS",
180+
"prepare_resume",
181+
"ResumePreflight",
168182
"derive_orchestrator_resume_spec",
169183
"find_dispatch_for_issue",
170184
"checkpoint_from_sidecar",

src/autoskillit/fleet/_api.py

Lines changed: 128 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
from autoskillit.fleet._outcome import _checkpoint_to_dict, classify_dispatch_outcome
2929
from autoskillit.fleet.result_parser import parse_l3_result_block
3030
from autoskillit.fleet.state import DispatchStatus
31+
from autoskillit.fleet.state_recovery import ResumePreflight, prepare_resume
3132
from autoskillit.fleet.state_types import (
3233
DispatchCompleted,
3334
DispatchRejected,
@@ -45,6 +46,20 @@
4546
ENVELOPE_STDERR_MAX = 2000
4647

4748

49+
class DispatchSpawnFailed(RuntimeError):
50+
"""Spawn-time failure signal for the ``execute_dispatch`` callback path.
51+
52+
Attributes:
53+
error_code: FleetErrorCode identifying the failure category.
54+
message: Human-readable description of the spawn failure.
55+
"""
56+
57+
def __init__(self, error_code: FleetErrorCode, message: str) -> None:
58+
super().__init__(message)
59+
self.error_code = error_code
60+
self.message = message
61+
62+
4863
@asynccontextmanager
4964
async def _dispatch_heartbeat(
5065
dispatches_dir: Path,
@@ -132,9 +147,28 @@ def _write_pid(
132147
dispatched_create_time: float = 0.0,
133148
identity_degraded: bool = False,
134149
issue_url: str = "",
135-
) -> None:
136-
"""on_spawn callback: atomically mark dispatch as running with dispatched_pid."""
150+
*,
151+
enforce_max_resume_attempts: bool = False,
152+
) -> str | None:
153+
"""on_spawn callback: atomically mark dispatch as running with dispatched_pid.
154+
155+
L2 — Fail-closed: if ``mark_dispatch_running`` raises (e.g. illegal state
156+
transition), the spawned child is killed via ``kill_process_tree`` (the
157+
canonical sync kill primitive used by ``_dispatch_reaper``) and the
158+
exception's message string is returned to the caller via closure-scoped
159+
state. Raising the exception from ``_on_spawn`` is NOT safe because
160+
``_execute_claude_headless`` catches runner exceptions and returns
161+
``SkillResult.crashed`` — the propagated exception would never reach the
162+
outer ``execute_dispatch`` wrapper. The caller therefore inspects the
163+
returned error string (or the closure-scoped ``_spawn_error`` list) and
164+
translates it into a ``FLEET_L3_STARTUP_OR_CRASH`` envelope.
165+
166+
Returns:
167+
None on success; the formatted error message string on failure (also
168+
recorded via the side-effect of having killed the child).
169+
"""
137170
from autoskillit.core import read_boot_id
171+
from autoskillit.execution import kill_process_tree
138172
from autoskillit.fleet import mark_dispatch_running
139173

140174
try:
@@ -149,9 +183,23 @@ def _write_pid(
149183
sidecar_path=sidecar_path,
150184
identity_degraded=identity_degraded,
151185
issue_url=issue_url,
186+
enforce_max_resume_attempts=enforce_max_resume_attempts,
152187
)
153-
except Exception:
154-
logger.warning("_write_pid: failed to mark dispatch running", exc_info=True)
188+
return None
189+
except Exception as exc:
190+
# Fail-closed: kill the child before the state record can diverge.
191+
if pid:
192+
try:
193+
kill_process_tree(pid, timeout=2.0)
194+
except Exception:
195+
logger.warning(
196+
"_write_pid: kill_process_tree failed for pid=%d",
197+
pid,
198+
exc_info=True,
199+
)
200+
cause = getattr(exc, "__cause__", None) or getattr(exc, "__context__", None)
201+
cause_str = f" caused by {type(cause).__name__}: {cause}" if cause is not None else ""
202+
return f"_on_spawn transition failed: {type(exc).__name__}: {exc}{cause_str}"
155203

156204

157205
def _post_dispatch_cleanup(
@@ -479,24 +527,60 @@ async def _run_dispatch(
479527
}
480528
prior_session_chain: list[str] = []
481529
prior_dispatched_session_id = ""
530+
# Hoisted: closure-captured by _on_spawn to derive is_resume_branch for
531+
# the enforce_max_resume_attempts kwarg on _write_pid. A non-None value
532+
# means the resume branch executed prepare_resume (vs. fresh dispatch path).
533+
preflight: ResumePreflight | None = None
482534
if resume_session_id and prior_dispatch_id:
483535
try:
484536
handle = DispatchStateHandle.open_continued(dispatches_dir, prior_dispatch_id)
485537
prior_state = read_state(handle.state_path)
486-
if prior_state:
487-
for d in prior_state.dispatches:
488-
if d.name == effective_name:
489-
if d.status == DispatchStatus.SUCCESS:
490-
logger.info(
491-
"resume_skipped_prior_success",
492-
dispatch_name=effective_name,
493-
prior_dispatch_id=prior_dispatch_id,
494-
)
495-
return _build_success_short_circuit(d, handle)
496-
prior_session_chain = list(d.session_chain)
497-
prior_dispatched_session_id = d.dispatched_session_id
498-
break
499-
except (OSError, ValueError, KeyError, TypeError):
538+
if prior_state is None:
539+
# Corrupt or missing prior state — degrade to fresh.
540+
handle = DispatchStateHandle.create_fresh(
541+
dispatches_dir,
542+
campaign_id,
543+
effective_name,
544+
"",
545+
[DispatchRecord(name=effective_name, caller_session_id=caller_session_id)],
546+
recipe_snapshot,
547+
)
548+
else:
549+
# Single precondition chokepoint. Funnels every resume entry
550+
# point through one function so the bug class becomes
551+
# structurally impossible.
552+
preflight = prepare_resume(
553+
handle.state_path,
554+
effective_name,
555+
continue_on_failure=True,
556+
)
557+
if preflight is not None:
558+
if preflight.short_circuit is not None:
559+
logger.info(
560+
"resume_skipped_prior_success",
561+
dispatch_name=effective_name,
562+
prior_dispatch_id=prior_dispatch_id,
563+
)
564+
return _build_success_short_circuit(preflight.short_circuit, handle)
565+
if preflight.halt:
566+
return DispatchResult(
567+
outcome=DispatchRejected(
568+
error_code=FleetErrorCode.FLEET_CAMPAIGN_HALTED,
569+
message=preflight.halted_reason
570+
or "Resume refused by precondition chokepoint",
571+
dispatch_id=handle.identity.dispatch_id,
572+
),
573+
per_dispatch_state_path=None,
574+
)
575+
prior_session_chain = preflight.prior_session_chain
576+
prior_dispatched_session_id = preflight.prior_dispatched_session_id
577+
except (OSError, KeyError, TypeError):
578+
# OSError covers FileNotFoundError from DispatchStateHandle.open_continued
579+
# when the prior state file is missing (fail-open to fresh dispatch) and
580+
# write failures from reset_blocking_dispatch. ValueError is intentionally
581+
# NOT caught so ResumeCountExceeded (a ValueError subclass raised by the
582+
# cap check in mark_dispatch_running) propagates instead of being silently
583+
# degraded to a create_fresh path.
500584
logger.warning("failed to read prior session chain from state", exc_info=True)
501585
handle = DispatchStateHandle.create_fresh(
502586
dispatches_dir,
@@ -668,6 +752,9 @@ def _reject_with_state(error_code: FleetErrorCode, message: str) -> DispatchResu
668752
_dispatched_create_time: list[float] = []
669753
_dispatched_boot_id: list[str] = []
670754
_dispatched_session_id: list[str] = []
755+
# Closure-scoped spawn error state (layer L2). See _write_pid docstring
756+
# for why raising from on_spawn would not propagate.
757+
_spawn_error: list[str] = []
671758

672759
# Collect prior dispatch_ids from attempt_history for defense-in-depth parsing
673760
prior_ids: list[str] = []
@@ -705,7 +792,16 @@ def _on_spawn(pid: int, ticks: int) -> None:
705792
_dispatched_ticks.append(ticks)
706793
_dispatched_create_time.append(create_time)
707794
_dispatched_boot_id.append(boot_id)
708-
_write_pid(
795+
# Resume branch iff preflight was returned by prepare_resume above.
796+
# Cap enforcement (MAX_CONSECUTIVE_RESUME_ATTEMPTS) lives one layer down
797+
# in mark_dispatch_running.
798+
is_resume_branch = preflight is not None
799+
# _write_pid returns None on success or an error string on failure.
800+
# We record the error in closure-scoped _spawn_error rather than
801+
# raising — raising from on_spawn would be swallowed by the executor
802+
# and converted to SkillResult.crashed, never reaching the outer
803+
# execute_dispatch wrapper.
804+
err = _write_pid(
709805
state_path,
710806
effective_name,
711807
dispatch_id,
@@ -715,7 +811,10 @@ def _on_spawn(pid: int, ticks: int) -> None:
715811
create_time,
716812
identity_degraded=(ticks == 0),
717813
issue_url=_issue_urls_raw,
814+
enforce_max_resume_attempts=is_resume_branch,
718815
)
816+
if err is not None:
817+
_spawn_error.append(err)
719818

720819
def _on_session_id(session_id: str) -> None:
721820
from autoskillit.fleet.state import mark_dispatch_session_identity
@@ -777,6 +876,16 @@ def _on_session_id(session_id: str) -> None:
777876
else None,
778877
)
779878

879+
# L2 fail-closed spawn gate: check closure-scoped error state.
880+
# If _on_spawn recorded a transition failure (and killed the child
881+
# via kill_process_tree), translate it to a structured envelope
882+
# instead of letting the dispatch proceed on a stale record.
883+
if _spawn_error:
884+
return _reject_with_state(
885+
FleetErrorCode.FLEET_L3_STARTUP_OR_CRASH,
886+
_spawn_error[0],
887+
)
888+
780889
ended_at = max(time.time(), started_at + 1e-6)
781890
_dispatch_completed_normally = True
782891
except asyncio.CancelledError:

src/autoskillit/fleet/state.py

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,16 @@ def _write_state(state_path: Path, state: CampaignState) -> None:
350350
write_versioned_json(state_path, payload, schema_version=FLEET_STATE_SCHEMA_VERSION)
351351

352352

353+
class ResumeCountExceeded(ValueError):
354+
"""Raised when ``mark_dispatch_running`` is invoked with ``enforce_max_resume_attempts=True``
355+
and the dispatch has already accumulated ``MAX_CONSECUTIVE_RESUME_ATTEMPTS``
356+
consecutive RESUMABLE entries in its ``attempt_history``.
357+
358+
Distinct from the ValueError raised by ``_validate_transition`` so callers
359+
can differentiate state-machine violations from cap exhaustion.
360+
"""
361+
362+
353363
def mark_dispatch_running(
354364
state_path: Path,
355365
dispatch_name: str,
@@ -362,8 +372,22 @@ def mark_dispatch_running(
362372
sidecar_path: str | None = None,
363373
identity_degraded: bool = False,
364374
issue_url: str = "",
375+
enforce_max_resume_attempts: bool = False,
365376
) -> None:
366-
"""Atomically mark a dispatch as running with its dispatch_id and dispatched_pid."""
377+
"""Atomically mark a dispatch as running with its dispatch_id and dispatched_pid.
378+
379+
L3 — Resume-count cap on the headless path: when ``enforce_max_resume_attempts``
380+
is True, the transition is gated by ``_count_consecutive_resumable_timeouts``
381+
over the dispatch's ``attempt_history``. The cap check is computed from the
382+
history (preserved across FAILURE→PENDING auto-resets by ``_RETRY_IDENTITY_FIELDS``),
383+
so the existing campaign-level cap semantics are extended to the headless
384+
path without skipping the cap on a reset-rewriting-the-status path.
385+
"""
386+
from autoskillit.fleet.state_recovery import ( # noqa: PLC0415
387+
MAX_CONSECUTIVE_RESUME_ATTEMPTS,
388+
_count_consecutive_resumable_timeouts,
389+
)
390+
367391
with CampaignStateMutator(state_path) as m:
368392
if m.state is None:
369393
raise FileNotFoundError(f"State file not found or corrupted: {state_path}")
@@ -372,6 +396,13 @@ def mark_dispatch_running(
372396
d.retry_reason = ""
373397
d.infra_exit_category = ""
374398
_validate_transition(d.status, DispatchStatus.RUNNING, d.name)
399+
if enforce_max_resume_attempts:
400+
consecutive_timeouts = _count_consecutive_resumable_timeouts(d.attempt_history)
401+
if consecutive_timeouts >= MAX_CONSECUTIVE_RESUME_ATTEMPTS:
402+
raise ResumeCountExceeded(
403+
f"Resume cap ({MAX_CONSECUTIVE_RESUME_ATTEMPTS}) "
404+
f"exhausted for {dispatch_name!r}"
405+
)
375406
was_resumable = d.status == DispatchStatus.RESUMABLE
376407
d.status = DispatchStatus.RUNNING
377408
if was_resumable:

0 commit comments

Comments
 (0)