Skip to content

Commit 75c72dd

Browse files
Trecekclaude
andauthored
eager session-id persistence via on_session_id_resolved callback (#4078)
The dispatch system has a structural weakness where runtime-discovered identity fields (`dispatched_session_id`, `session_chain`, `dispatched_session_log_dir`, `token_usage`, `resume_checkpoint`, `branch_name`) are only written to state on the normal completion path (`_api.py:927` via `upsert_dispatch_record_by_name`). On signal interruption (`CancelledError`), only `status`, `reason`, and `ended_at` are written via `mark_dispatch_interrupted()`. This leaves the manifest non-resumable because downstream consumers (state recovery, fleet session, label cleanup) rely on `dispatched_session_id` being non-empty. Part A addresses the core architectural immunity: eager session-identity persistence via an `on_session_id_resolved` callback fired from inside the live-execution task group (mirroring the existing `on_pid_resolved` pattern), `mark_dispatch_interrupted` signature extension for defense-in-depth identity capture, and test coverage that makes this bug class instantly caught. Part B (separate task) will cover R3 (`sessions.jsonl` fallback lookup), R4 (L3 orchestrator dispatch history visibility), and the remaining deferred-write fields (`token_usage`, `resume_checkpoint`, `branch_name`). ## Requirements `dispatched_session_id` is recorded as `""` (empty string) in the dispatch manifest when a signal interrupts the dispatch — even when the headless session completed all pipeline steps successfully. This makes the dispatch non-resumable and, combined with the stale `in-progress` GitHub label, prevents the orchestrator from resuming the interrupted session. Recovery requires manual `reset_dispatch(force=True)` followed by a fresh dispatch, losing the ability to continue from where the session left off. ### R1: Eager `dispatched_session_id` Write on Interruption Extend `mark_dispatch_interrupted()` to accept an optional `dispatched_session_id` parameter. In the `except asyncio.CancelledError` handler at `_api.py:731`: 1. Guard against `NameError`: `skill_result` is assigned inside the `try` block at line 698. If `CancelledError` fires BEFORE `dispatch_food_truck()` returns, `skill_result` is never assigned. The handler must use `skill_result = locals().get("skill_result")` or pre-initialize `skill_result = None` before the `try` block. 2. If `skill_result` is available and has a non-empty `session_id`, pass it to `mark_dispatch_interrupted()`. 3. Also capture `session_chain` — extend `mark_dispatch_interrupted()` to accept an optional `session_chain` parameter, or add a separate `set_dispatch_session_identity()` state mutation function. This follows the same eager-write pattern used for PID capture via `mark_dispatch_running()` and for `issue_url` capture added in PR #3828. ### R2: Intermediate Session ID Write at Process Layer When Channel B or stdout extraction succeeds during the race monitoring phase, write `dispatched_session_id` to the manifest immediately — before `SubprocessResult` is fully constructed. This covers the case where a signal arrives during the session (before `dispatch_food_truck()` returns) but the session ID is already known. **Architectural constraint:** The execution package is IL-1; fleet state is IL-2. Direct imports from `execution` → `fleet` would violate import layering. This must be implemented via a callback pattern: `_run_dispatch` passes a `on_session_id_resolved: Callable[[str], None]` callback down to the process runner, which invokes it when Channel B or stdout extraction succeeds. The callback closes over `state_path` and `effective_name` and calls a new state mutation function. This mirrors the existing `on_pid_resolved` callback pattern at `_api.py:657-679`. ### R3: `sessions.jsonl` Fallback Lookup When `dispatched_session_id` is empty on a resumable/interrupted dispatch that has a non-empty `dispatch_id`, search `sessions.jsonl` for entries matching that `dispatch_id` to recover the session_id. Defense-in-depth, not a substitute for R1/R2. ### R4: L3 Orchestrator Dispatch History Visibility Give the L3 fleet dispatch orchestrator read access to the history of dispatches it launched and their current status. Currently, the L3 has no mechanism to query dispatch state — it relies entirely on the return value of `dispatch_food_truck`, which is lost on interruption. The orchestrator should be able to: - Query dispatches by `caller_session_id` (its own session) or `campaign_id` - See the current status, `dispatched_session_id`, `dispatched_pid`, and `branch_name` for each dispatch - Use this information to make informed resume/reset/skip decisions when re-entering after a crash This would eliminate the class of bugs where the L3 orchestrator has no context about what happened during an interrupted dispatch and must rely on indirect signals (labels, sidecar files) that may themselves be stale. ### R5: Test Coverage - Add assertion on `dispatched_session_id` in `TestCancelledErrorRecordsInterruptedState` - Add test for post-completion race window: `CancelledError` after `dispatch_food_truck()` returns but before `upsert_dispatch_record_by_name()` - Add test verifying `session_chain` is also captured on the interruption path - Add test for the resume-blocked scenario: INTERRUPTED + empty `dispatched_session_id` + stale `in-progress` label → verify `reset_dispatch` recovery works Closes #4072 ## Implementation Plan Plan file: `/home/talon/projects/autoskillit-runs/remediation-20260611-155102-572761/.autoskillit/temp/rectify/rectify_dispatch_session_id_signal_persistence_2026-06-11_155102.md` 🤖 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 Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 16048e0 commit 75c72dd

17 files changed

Lines changed: 314 additions & 2 deletions

src/autoskillit/core/types/_type_protocols_execution.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ async def dispatch_food_truck(
109109
session_id: str | None = None,
110110
resume_message: str | None = None,
111111
backend_override: str | None = None,
112+
on_session_id_resolved: Callable[[str], None] | None = None,
112113
) -> SkillResult: ...
113114

114115

src/autoskillit/core/types/_type_subprocess.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,4 +175,5 @@ def __call__(
175175
session_record_types: frozenset[str] = frozenset({"assistant"}),
176176
inspector_callback: InspectorCallback | None = None,
177177
workload_basenames: frozenset[str] | None = None,
178+
on_session_id_resolved: Callable[[str], None] | None = None,
178179
) -> Awaitable[SubprocessResult]: ...

src/autoskillit/execution/headless/__init__.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -380,6 +380,7 @@ async def dispatch_food_truck(
380380
session_id: str | None = None,
381381
resume_message: str | None = None,
382382
backend_override: str | None = None,
383+
on_session_id_resolved: Callable[[str], None] | None = None,
383384
) -> SkillResult:
384385
if self._ctx.backend is not None and not self._ctx.backend.capabilities.food_truck_capable:
385386
raise RuntimeError(
@@ -483,4 +484,5 @@ async def dispatch_food_truck(
483484
marker_dir=effective_marker_dir,
484485
session_id=session_id,
485486
model_identity=model_identity,
487+
on_session_id_resolved=on_session_id_resolved,
486488
)

src/autoskillit/execution/headless/_headless_execute.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ async def _execute_claude_headless(
111111
model_identity: ModelIdentity = ModelIdentity.unknown(),
112112
inspector_eligible: bool = False,
113113
inspector_model: str = "",
114+
on_session_id_resolved: Callable[[str], None] | None = None,
114115
) -> SkillResult:
115116
"""Shared subprocess execution for headless Claude sessions.
116117
@@ -262,6 +263,7 @@ async def _execute_claude_headless(
262263
max_extension_seconds=max_extension_seconds,
263264
marker_dir=marker_dir,
264265
session_id=session_id,
266+
on_session_id_resolved=on_session_id_resolved,
265267
stream_parser=_stream_parser,
266268
completion_record_types=_step_backend.capabilities.completion_record_types,
267269
session_record_types=_step_backend.capabilities.session_record_types,

src/autoskillit/execution/process/__init__.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,7 @@ async def run_managed_async(
220220
stream_parser: StreamParser | None = None,
221221
inspector_callback: InspectorCallback | None = None,
222222
workload_basenames: frozenset[str] | None = None,
223+
on_session_id_resolved: Callable[[str], None] | None = None,
223224
) -> SubprocessResult:
224225
"""Async subprocess execution with temp file I/O and process tree cleanup.
225226
@@ -352,6 +353,7 @@ async def run_managed_async(
352353
functools.partial(
353354
_extract_stdout_session_id,
354355
stream_parser=stream_parser,
356+
on_session_id_resolved=on_session_id_resolved,
355357
),
356358
stdout_path,
357359
acc,
@@ -632,6 +634,7 @@ async def __call__(
632634
session_record_types: frozenset[str] = frozenset({"assistant"}),
633635
inspector_callback: InspectorCallback | None = None,
634636
workload_basenames: frozenset[str] | None = None,
637+
on_session_id_resolved: Callable[[str], None] | None = None,
635638
) -> SubprocessResult:
636639
return await run_managed_async(
637640
cmd,
@@ -657,4 +660,5 @@ async def __call__(
657660
session_record_types=session_record_types,
658661
inspector_callback=inspector_callback,
659662
workload_basenames=workload_basenames,
663+
on_session_id_resolved=on_session_id_resolved,
660664
)

src/autoskillit/execution/process/_process_race.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
from __future__ import annotations
44

5+
from collections.abc import Callable
56
from dataclasses import asdict, dataclass, field
67
from pathlib import Path
78
from typing import TYPE_CHECKING, assert_never
@@ -366,6 +367,8 @@ async def _extract_stdout_session_id(
366367
stream_parser: StreamParser | None = None,
367368
_poll_interval: float = 0.3,
368369
_timeout: float = 10.0,
370+
*,
371+
on_session_id_resolved: Callable[[str], None] | None = None,
369372
) -> None:
370373
"""Extract session ID from stdout type=system record and deposit on accumulator.
371374
@@ -413,6 +416,8 @@ async def _extract_stdout_session_id(
413416
if sid:
414417
acc.stdout_session_id = sid
415418
logger.debug("stdout_session_id_extracted", session_id=sid)
419+
if on_session_id_resolved is not None:
420+
on_session_id_resolved(sid)
416421
ready.set()
417422
return
418423
logger.debug("stdout_session_id_extraction_timeout", timeout=_timeout)

src/autoskillit/execution/recording.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ async def __call__(
140140
session_record_types: frozenset[str] = frozenset({"assistant"}),
141141
inspector_callback: InspectorCallback | None = None,
142142
workload_basenames: frozenset[str] | None = None,
143+
on_session_id_resolved: Callable[[str], None] | None = None,
143144
) -> SubprocessResult:
144145
step_name = (env or {}).get(SCENARIO_STEP_NAME_ENV, "")
145146

@@ -430,6 +431,7 @@ async def __call__(
430431
session_record_types: frozenset[str] = frozenset({"assistant"}),
431432
inspector_callback: InspectorCallback | None = None,
432433
workload_basenames: frozenset[str] | None = None,
434+
on_session_id_resolved: Callable[[str], None] | None = None,
433435
) -> SubprocessResult:
434436
step_name = (env or {}).get(SCENARIO_STEP_NAME_ENV, "")
435437

src/autoskillit/fleet/__init__.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@
6161
mark_dispatch_interrupted,
6262
mark_dispatch_resumable,
6363
mark_dispatch_running,
64+
mark_dispatch_session_identity,
6465
normalize_dispatch_token_usage,
6566
read_all_campaign_captures,
6667
read_state,
@@ -146,6 +147,7 @@
146147
"record_gate_outcome",
147148
"mark_dispatch_interrupted",
148149
"mark_dispatch_resumable",
150+
"mark_dispatch_session_identity",
149151
"mark_dispatch_running",
150152
"read_all_campaign_captures",
151153
"read_state",

src/autoskillit/fleet/_api.py

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -629,6 +629,7 @@ def _reject_with_state(error_code: FleetErrorCode, message: str) -> DispatchResu
629629
_dispatched_ticks: list[int] = []
630630
_dispatched_create_time: list[float] = []
631631
_dispatched_boot_id: list[str] = []
632+
_dispatched_session_id: list[str] = []
632633

633634
# Collect prior dispatch_ids from attempt_history for defense-in-depth parsing
634635
prior_ids: list[str] = []
@@ -678,6 +679,14 @@ def _on_spawn(pid: int, ticks: int) -> None:
678679
issue_url=_issue_urls_raw,
679680
)
680681

682+
def _on_session_id(session_id: str) -> None:
683+
from autoskillit.fleet.state import mark_dispatch_session_identity
684+
685+
_dispatched_session_id.append(session_id)
686+
mark_dispatch_session_identity(
687+
state_path, effective_name, dispatched_session_id=session_id
688+
)
689+
681690
marker_dir: Path | None = None
682691
if _locator is not None:
683692
try:
@@ -710,6 +719,7 @@ def _on_spawn(pid: int, ticks: int) -> None:
710719
project_dir=str(tool_ctx.project_dir),
711720
marker_dir=marker_dir,
712721
session_id=caller_session_id,
722+
on_session_id_resolved=_on_session_id,
713723
timeout=resolved_timeout,
714724
idle_output_timeout=float(idle_output_timeout)
715725
if idle_output_timeout is not None
@@ -733,11 +743,21 @@ def _on_spawn(pid: int, ticks: int) -> None:
733743
try:
734744
from autoskillit.fleet.state import mark_dispatch_interrupted # noqa: PLC0415
735745

746+
captured_session_id = _dispatched_session_id[0] if _dispatched_session_id else ""
747+
if not captured_session_id:
748+
sr = locals().get("skill_result")
749+
if sr is not None:
750+
captured_session_id = getattr(sr, "session_id", "") or ""
751+
736752
with anyio.CancelScope(shield=True):
737753
mark_dispatch_interrupted(
738754
state_path,
739755
effective_name,
740756
reason="signal_induced_cancellation",
757+
dispatched_session_id=captured_session_id,
758+
dispatched_session_log_dir=str(marker_dir)
759+
if marker_dir is not None
760+
else "",
741761
)
742762
except Exception:
743763
logger.warning(
@@ -894,7 +914,9 @@ def _on_spawn(pid: int, ticks: int) -> None:
894914
dispatch_id=dispatch_id,
895915
campaign_id=campaign_id,
896916
caller_session_id=caller_session_id,
897-
dispatched_session_id=skill_result.session_id,
917+
dispatched_session_id=_dispatched_session_id[0]
918+
if _dispatched_session_id
919+
else skill_result.session_id,
898920
session_chain=extended_chain,
899921
dispatched_session_log_dir=project_log_dir,
900922
dispatched_pid=_dispatched_pid[0] if _dispatched_pid else 0,

src/autoskillit/fleet/state.py

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@
8282
"read_state",
8383
"mark_dispatch_running",
8484
"mark_dispatch_interrupted",
85+
"mark_dispatch_session_identity",
8586
"mark_dispatch_resumable",
8687
"reset_blocking_dispatch",
8788
"append_dispatch_record",
@@ -395,8 +396,17 @@ def mark_dispatch_interrupted(
395396
dispatch_name: str,
396397
*,
397398
reason: str,
399+
dispatched_session_id: str = "",
400+
session_chain: list[str] | None = None,
401+
dispatched_session_log_dir: str = "",
398402
) -> None:
399-
"""Atomically mark a dispatch as interrupted with a reason."""
403+
"""Atomically mark a dispatch as interrupted with a reason.
404+
405+
Identity fields (dispatched_session_id, session_chain, dispatched_session_log_dir)
406+
are additive — they are only written if the corresponding field on the record is
407+
still empty, preventing the cancellation handler from overwriting values already
408+
eagerly persisted by the on_session_id_resolved callback.
409+
"""
400410
with CampaignStateMutator(state_path) as m:
401411
if m.state is None:
402412
raise FileNotFoundError(f"State file not found or corrupted: {state_path}")
@@ -406,12 +416,43 @@ def mark_dispatch_interrupted(
406416
d.status = DispatchStatus.INTERRUPTED
407417
d.reason = reason
408418
d.ended_at = time.time()
419+
if dispatched_session_id and not d.dispatched_session_id:
420+
d.dispatched_session_id = dispatched_session_id
421+
if session_chain is not None and not d.session_chain:
422+
d.session_chain = session_chain
423+
if dispatched_session_log_dir and not d.dispatched_session_log_dir:
424+
d.dispatched_session_log_dir = dispatched_session_log_dir
409425
m.mark_dirty()
410426
return
411427
else:
412428
raise ValueError(f"Dispatch '{dispatch_name}' not found in state")
413429

414430

431+
def mark_dispatch_session_identity(
432+
state_path: Path,
433+
dispatch_name: str,
434+
*,
435+
dispatched_session_id: str,
436+
) -> None:
437+
"""Eagerly persist the dispatched_session_id once it is discovered during execution.
438+
439+
Idempotent and status-guarded: only writes when the dispatch is RUNNING and
440+
dispatched_session_id is still empty. If the dispatch has already transitioned
441+
to INTERRUPTED (race condition), this is a no-op.
442+
"""
443+
with CampaignStateMutator(state_path) as m:
444+
if m.state is None:
445+
raise FileNotFoundError(f"State file not found or corrupted: {state_path}")
446+
for d in m.state.dispatches:
447+
if d.name == dispatch_name:
448+
if d.status == DispatchStatus.RUNNING and not d.dispatched_session_id:
449+
d.dispatched_session_id = dispatched_session_id
450+
m.mark_dirty()
451+
return
452+
else:
453+
raise ValueError(f"Dispatch '{dispatch_name}' not found in state")
454+
455+
415456
def mark_dispatch_resumable(
416457
state_path: Path,
417458
dispatch_name: str,

0 commit comments

Comments
 (0)