Skip to content

Commit e315dbb

Browse files
Trecekclaude
andauthored
Rectify: Dispatch Envelope Shape Discrimination Immunity (#4118)
## Summary The `_fmt_dispatch_food_truck` formatter conflates two structurally distinct `success=False` response shapes — `DispatchRejected` (subprocess never started) and `DispatchCompleted(success=False)` (subprocess ran and failed) — under a single early-return branch. This strips critical fields (`dispatched_session_id`, `dispatch_status`, `reason`, `token_usage`, `elapsed_seconds`) from failed dispatches that actually ran, causing the orchestrator to believe no session ever existed. The architectural solution introduces a typed `kind` discriminator field at the envelope layer, adds the field to the existing `DispatchEnvelopeResult` TypedDict, and adds per-shape formatter contract tests that structurally prevent any formatter from silently dropping fields for a given envelope shape. ## Implementation Plan Plan file: `/home/talon/projects/autoskillit-runs/remediation-20260626-152751-009727/.autoskillit/temp/rectify/rectify_dispatch_envelope_shape_discrimination_2026-06-26_153500.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 --> Closes #4111 ## Token Usage Summary | Step | Model | count | uncached | output | cache_read | peak_ctx | turns | cache_write | time | |------|-------|-------|----------|--------|------------|----------|-------|-------------|------| | rectify* | opus[1m] | 1 | 4.9k | 16.0k | 1.3M | 109.9k | 52 | 90.8k | 13m 50s | | review_approach* | sonnet | 1 | 60 | 8.3k | 257.1k | 51.2k | 17 | 37.9k | 6m 15s | | dry_walkthrough* | opus | 1 | 46 | 9.5k | 1.3M | 86.7k | 52 | 87.5k | 9m 8s | | implement* | MiniMax-M3 | 1 | 108.6k | 21.8k | 3.0M | 0 | 93 | 0 | 5m 48s | | assess* | opus[1m] | 1 | 48 | 4.6k | 1.0M | 58.7k | 43 | 39.6k | 5m 9s | | audit_impl* | sonnet | 1 | 539 | 11.2k | 284.0k | 47.9k | 20 | 41.0k | 6m 21s | | prepare_pr* | MiniMax-M3 | 1 | 43.4k | 2.3k | 152.5k | 0 | 14 | 0 | 30s | | compose_pr* | MiniMax-M3 | 1 | 36.8k | 1.4k | 171.8k | 0 | 11 | 0 | 31s | | review_pr* | sonnet | 3 | 510 | 112.4k | 4.0M | 99.9k | 152 | 225.2k | 28m 58s | | ci_conflict_fix* | opus[1m] | 1 | 26 | 2.1k | 306.4k | 45.6k | 19 | 26.6k | 1m 14s | | diagnose_ci* | MiniMax-M3 | 1 | 54.4k | 2.3k | 297.6k | 0 | 20 | 0 | 53s | | resolve_ci* | opus[1m] | 1 | 54 | 6.1k | 1.7M | 77.7k | 49 | 58.5k | 4m 29s | | **Total** | | | 249.4k | 198.0k | 13.8M | 109.9k | | 607.0k | 1h 23m | \* *Step used a non-Anthropic provider; caching behavior may differ.* ## Token Efficiency | Step | LoC Changed | cache_read/LoC | cache_write/LoC | output/LoC | |------|-------------|----------------|-----------------|------------| | rectify | 0 | — | — | — | | review_approach | 0 | — | — | — | | dry_walkthrough | 0 | — | — | — | | implement | 288 | 10507.4 | 0.0 | 75.7 | | assess | 3 | 344689.7 | 13199.0 | 1535.7 | | audit_impl | 0 | — | — | — | | prepare_pr | 0 | — | — | — | | compose_pr | 0 | — | — | — | | review_pr | 0 | — | — | — | | ci_conflict_fix | 381 | 804.2 | 69.8 | 5.5 | | diagnose_ci | 0 | — | — | — | | resolve_ci | 1 | 1706644.0 | 58532.0 | 6130.0 | | **Total** | **673** | 20433.8 | 902.0 | 294.2 | ## Model Usage Breakdown | Model | steps | uncached | output | cache_read | cache_write | time | |-------|-------|----------|--------|------------|-------------|------| | opus[1m] | 4 | 5.0k | 28.8k | 4.3M | 215.5k | 24m 43s | | sonnet | 3 | 1.1k | 131.8k | 4.5M | 304.0k | 41m 36s | | opus | 1 | 46 | 9.5k | 1.3M | 87.5k | 9m 8s | | MiniMax-M3 | 4 | 243.2k | 27.8k | 3.6M | 0 | 7m 42s | --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 339fa3a commit e315dbb

7 files changed

Lines changed: 291 additions & 3 deletions

File tree

src/autoskillit/fleet/state_types.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -374,6 +374,7 @@ class DispatchRejected:
374374

375375
def to_envelope(self) -> str:
376376
d: dict[str, Any] = {
377+
"kind": "rejected",
377378
"success": False,
378379
"error": self.error_code,
379380
"user_visible_message": self.message,
@@ -406,6 +407,7 @@ class DispatchCompleted:
406407

407408
def to_envelope(self) -> str:
408409
d: dict[str, Any] = {
410+
"kind": "completed",
409411
"success": self.success,
410412
"dispatch_status": self.dispatch_status.value,
411413
"dispatch_id": self.dispatch_id,

src/autoskillit/hooks/formatters/_fmt_dispatch.py

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,18 +23,19 @@ def _fmt_dispatch_food_truck(data: dict, _pipeline: bool) -> str:
2323
"""Format dispatch_food_truck result as Markdown-KV.
2424
2525
Renders both success (DispatchCompleted) and error (DispatchRejected /
26-
fleet_error) response shapes — discriminates on ``data.get("success")``.
26+
fleet_error) response shapes — discriminates on ``data.get("kind")``.
2727
Nested structured fields (l3_payload, health_report, token_usage,
2828
resume_checkpoint) are rendered in full without size-based truncation
2929
to preserve dispatch_plan visibility for downstream orchestrators.
3030
"""
3131
success = data.get("success", False)
3232
mark = _CHECK_MARK if success else _CROSS_MARK
3333
status = "OK" if success else "FAIL"
34+
kind = data.get("kind")
3435

3536
lines = [f"## dispatch_food_truck {mark} {status}", ""]
3637

37-
if not success:
38+
if kind == "rejected" or (not success and kind is None):
3839
# Error path — DispatchRejected / fleet_error shape
3940
error = data.get("error", "unknown")
4041
lines.append(f"error: {error}")
@@ -154,4 +155,19 @@ def _fmt_dispatch_food_truck(data: dict, _pipeline: bool) -> str:
154155
"details",
155156
}
156157
)
157-
_FMT_DISPATCH_FOOD_TRUCK_SUPPRESSED: frozenset[str] = frozenset()
158+
_FMT_DISPATCH_FOOD_TRUCK_SUPPRESSED: frozenset[str] = frozenset({"kind"})
159+
160+
_FMT_DISPATCH_COMPLETED_REQUIRED: frozenset[str] = frozenset(
161+
{
162+
"dispatch_status",
163+
"dispatch_id",
164+
"dispatched_session_id",
165+
"reason",
166+
}
167+
)
168+
_FMT_DISPATCH_REJECTED_REQUIRED: frozenset[str] = frozenset(
169+
{
170+
"error",
171+
"user_visible_message",
172+
}
173+
)

src/autoskillit/server/tools/_types.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,7 @@ class DispatchEnvelopeResult(TypedDict, total=False):
176176
"""
177177

178178
success: bool
179+
kind: str
179180
dispatch_status: str
180181
dispatch_id: str
181182
dispatched_session_id: str

tests/fleet/test_dispatch_envelope_fields.py

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,70 @@ async def dispatch_food_truck(self, orchestrator_prompt, cwd, *, on_spawn=None,
136136
assert result["dispatch_status"] == "resumable"
137137

138138

139+
class TestKindDiscriminatorEnvelopeField:
140+
def test_kind_discriminator_present_in_both_envelopes(self):
141+
"""Both DispatchCompleted and DispatchRejected must carry a 'kind' discriminator
142+
in their envelope. This explicit discriminator is what allows the formatter to
143+
distinguish them when both have success=False.
144+
"""
145+
from autoskillit.core import FleetErrorCode
146+
from autoskillit.fleet import DispatchCompleted, DispatchRejected, DispatchStatus
147+
148+
completed = DispatchCompleted(
149+
success=True,
150+
dispatch_status=DispatchStatus.SUCCESS,
151+
dispatch_id="d-c",
152+
dispatched_session_id="s-c",
153+
reason="ok",
154+
)
155+
completed_env = json.loads(completed.to_envelope())
156+
assert completed_env.get("kind") == "completed", (
157+
f"DispatchCompleted.to_envelope() must emit 'kind': 'completed': {completed_env}"
158+
)
159+
160+
rejected = DispatchRejected(
161+
error_code=FleetErrorCode.FLEET_QUOTA_EXHAUSTED,
162+
message="quota hit",
163+
dispatch_id="d-r",
164+
)
165+
rejected_env = json.loads(rejected.to_envelope())
166+
assert rejected_env.get("kind") == "rejected", (
167+
f"DispatchRejected.to_envelope() must emit 'kind': 'rejected': {rejected_env}"
168+
)
169+
170+
def test_rejected_envelope_never_emits_completed_exclusive_fields(self):
171+
"""DispatchRejected.to_envelope() must never emit DispatchCompleted-exclusive
172+
fields. This is a structural exclusion guard that protects the discriminator
173+
invariant: if a future change adds dispatch_status to DispatchRejected, this
174+
test fails immediately.
175+
"""
176+
from autoskillit.core import FleetErrorCode
177+
from autoskillit.fleet import DispatchRejected
178+
179+
rejected = DispatchRejected(
180+
error_code=FleetErrorCode.FLEET_QUOTA_EXHAUSTED,
181+
message="quota hit",
182+
dispatch_id="d-r",
183+
)
184+
env = json.loads(rejected.to_envelope())
185+
excluded_keys = {
186+
"dispatch_status",
187+
"dispatched_session_id",
188+
"reason",
189+
"token_usage",
190+
"l3_payload",
191+
"l3_parse_source",
192+
"lifespan_started",
193+
"stderr",
194+
"elapsed_seconds",
195+
}
196+
leaked = excluded_keys & set(env.keys())
197+
assert not leaked, (
198+
f"DispatchRejected.to_envelope() leaked DispatchCompleted-exclusive fields: "
199+
f"{sorted(leaked)}. The 'kind' discriminator invariant is broken."
200+
)
201+
202+
139203
class TestHealthReportEnvelopeField:
140204
def test_envelope_includes_health_report_when_present(self):
141205
from autoskillit.fleet import DispatchCompleted, DispatchStatus

tests/infra/conftest.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,13 @@ def _dispatch_food_truck_json_producer() -> dict:
5959
resume_checkpoint={"step": 1, "completed_items": ["a"]},
6060
health_report={"status": "healthy", "findings": []},
6161
)
62+
completed_failure = DispatchCompleted(
63+
success=False,
64+
dispatch_status=DispatchStatus.FAILURE,
65+
dispatch_id="d3",
66+
dispatched_session_id="s3",
67+
reason="fleet_l3_no_result_block",
68+
)
6269
rejected = DispatchRejected(
6370
error_code=FleetErrorCode.FLEET_QUOTA_EXHAUSTED,
6471
message="quota limit hit",
@@ -70,6 +77,7 @@ def _dispatch_food_truck_json_producer() -> dict:
7077
for envelope_str in (
7178
base.to_envelope(),
7279
with_optionals.to_envelope(),
80+
completed_failure.to_envelope(),
7381
rejected.to_envelope(),
7482
error_str,
7583
):

tests/infra/test_pretty_output_formatters.py

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -866,6 +866,7 @@ def test_dispatch_food_truck_six_group_plan_preserved():
866866
from tests.infra._pretty_output_helpers import _wrap_for_claude_code
867867

868868
payload = {
869+
"kind": "completed",
869870
"success": True,
870871
"dispatch_status": "completed_clean",
871872
"dispatch_id": "d-test-001",
@@ -912,6 +913,7 @@ def test_dispatch_food_truck_error_envelope_preserves_diagnostics():
912913
from tests.infra._pretty_output_helpers import _wrap_for_claude_code
913914

914915
payload = {
916+
"kind": "rejected",
915917
"success": False,
916918
"error": "fleet_quota_exhausted",
917919
"user_visible_message": "quota limit hit for this session",
@@ -927,3 +929,82 @@ def test_dispatch_food_truck_error_envelope_preserves_diagnostics():
927929
assert "fleet_quota_exhausted" in text
928930
assert "quota limit hit" in text
929931
assert "dispatch_id: d-rejected-001" in text
932+
933+
934+
# PHK-FMT-DISPATCH-COMPLETED-FAIL: DispatchCompleted(success=False) must not be conflated
935+
def test_completed_failure_through_formatter_preserves_all_fields():
936+
"""A DispatchCompleted(success=False) envelope carries critical fields that
937+
distinguish it from DispatchRejected: dispatched_session_id, dispatch_status,
938+
reason, token_usage, elapsed_seconds, lifespan_started. These fields must ALL
939+
survive formatting — they prove the subprocess actually ran. The bug: the
940+
formatter's `if not success:` branch strips these fields because it conflates
941+
DispatchCompleted(success=False) with DispatchRejected.
942+
"""
943+
from tests.infra._pretty_output_helpers import _wrap_for_claude_code
944+
945+
payload = {
946+
"kind": "completed",
947+
"success": False,
948+
"dispatch_status": "failure",
949+
"dispatch_id": "d-completed-fail-001",
950+
"dispatched_session_id": "sess-xyz",
951+
"reason": "fleet_l3_no_result_block",
952+
"token_usage": {"input": 1250, "output": 82381},
953+
"lifespan_started": True,
954+
"elapsed_seconds": 9774.74,
955+
}
956+
event = {
957+
"tool_name": "mcp__plugin_autoskillit_autoskillit__dispatch_food_truck",
958+
"tool_response": _wrap_for_claude_code(payload),
959+
}
960+
out, _ = _run_hook(event=event)
961+
text = json.loads(out)["hookSpecificOutput"]["updatedMCPToolOutput"]
962+
963+
assert "FAIL" in text, f"Header must show FAIL on failure: {text!r}"
964+
assert "dispatch_status: failure" in text, (
965+
f"dispatch_status must appear in formatted output: {text!r}"
966+
)
967+
assert "dispatched_session_id: sess-xyz" in text, (
968+
f"dispatched_session_id must appear in formatted output: {text!r}"
969+
)
970+
assert "reason: fleet_l3_no_result_block" in text, (
971+
f"reason must appear in formatted output: {text!r}"
972+
)
973+
assert "token_usage:" in text, f"token_usage section must appear in formatted output: {text!r}"
974+
assert "elapsed_seconds: 9774.74" in text, (
975+
f"elapsed_seconds must appear in formatted output: {text!r}"
976+
)
977+
assert "lifespan_started: True" in text, (
978+
f"lifespan_started must appear in formatted output: {text!r}"
979+
)
980+
assert "dispatch_id: d-completed-fail-001" in text, (
981+
f"dispatch_id must appear in formatted output: {text!r}"
982+
)
983+
984+
985+
# PHK-FMT-DISPATCH-FLEET-ERROR: fleet_error() envelope has no kind field — fallback path required
986+
def test_fleet_error_through_formatter_renders_error_fields():
987+
"""fleet_error() produces envelopes without a kind field. The formatter must
988+
handle these via a fallback path (kind is None + not success → rejection rendering).
989+
This protects backward compatibility for the legacy error envelope shape.
990+
"""
991+
from tests.infra._pretty_output_helpers import _wrap_for_claude_code
992+
993+
payload = {
994+
"success": False,
995+
"error": "fleet_quota_exhausted",
996+
"user_visible_message": "quota limit hit",
997+
"details": None,
998+
}
999+
event = {
1000+
"tool_name": "mcp__plugin_autoskillit_autoskillit__dispatch_food_truck",
1001+
"tool_response": _wrap_for_claude_code(payload),
1002+
}
1003+
out, _ = _run_hook(event=event)
1004+
text = json.loads(out)["hookSpecificOutput"]["updatedMCPToolOutput"]
1005+
assert "fleet_quota_exhausted" in text, (
1006+
f"fleet_error() error code must appear in output: {text!r}"
1007+
)
1008+
assert "quota limit hit" in text, (
1009+
f"fleet_error() user_visible_message must appear in output: {text!r}"
1010+
)

tests/infra/test_pretty_output_hook_infra.py

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,3 +368,119 @@ def test_conftest_import_does_not_load_server():
368368
assert result.returncode == 0, (
369369
f"Importing tests.infra.conftest triggered autoskillit.server import:\n{result.stderr}"
370370
)
371+
372+
373+
# ---------------------------------------------------------------------------
374+
# PHK-FMT-DISPATCH-SHAPE: Per-shape field obligation tests
375+
# ---------------------------------------------------------------------------
376+
377+
378+
def test_shape_field_obligations_per_kind():
379+
"""Per-shape required-field sets must be enforced for dispatch_food_truck.
380+
381+
The _FMT_DISPATCH_FOOD_TRUCK_RENDERED frozenset is a flat union of all 18 fields
382+
and cannot express per-shape obligations. This test defines required-field sets
383+
per shape ('completed' vs 'rejected') and asserts the formatter renders them
384+
when given an envelope with the matching 'kind' discriminator.
385+
386+
This is the shape-aware coverage contract that protects against future
387+
regressions where a formatter silently drops fields for a specific shape.
388+
"""
389+
from autoskillit.hooks.formatters._fmt_dispatch import (
390+
_FMT_DISPATCH_COMPLETED_REQUIRED,
391+
_FMT_DISPATCH_REJECTED_REQUIRED,
392+
)
393+
from tests.infra._pretty_output_helpers import _wrap_for_claude_code
394+
395+
completed_required = _FMT_DISPATCH_COMPLETED_REQUIRED
396+
rejected_required = _FMT_DISPATCH_REJECTED_REQUIRED
397+
398+
completed_payload = {
399+
"kind": "completed",
400+
"success": True,
401+
"dispatch_status": "success",
402+
"dispatch_id": "d-shape-c",
403+
"dispatched_session_id": "s-shape-c",
404+
"reason": "ok",
405+
"token_usage": {},
406+
"l3_payload": None,
407+
"l3_parse_source": "",
408+
"lifespan_started": False,
409+
"stderr": "",
410+
"elapsed_seconds": 1.0,
411+
}
412+
event_c = {
413+
"tool_name": "mcp__plugin_autoskillit_autoskillit__dispatch_food_truck",
414+
"tool_response": _wrap_for_claude_code(completed_payload),
415+
}
416+
out_c, _ = _run_hook(event=event_c)
417+
text_c = json.loads(out_c)["hookSpecificOutput"]["updatedMCPToolOutput"]
418+
completed_expected = {
419+
"dispatch_status": "dispatch_status: success",
420+
"dispatch_id": "dispatch_id: d-shape-c",
421+
"dispatched_session_id": "dispatched_session_id: s-shape-c",
422+
"reason": "reason: ok",
423+
}
424+
for field in completed_required:
425+
assert field in completed_expected, (
426+
f"Field {field!r} from _FMT_DISPATCH_COMPLETED_REQUIRED has no expected value — "
427+
f"add it to completed_expected and completed_payload"
428+
)
429+
assert completed_expected[field] in text_c, (
430+
f"Completed shape missing {field!r}: expected {completed_expected[field]!r} in output"
431+
)
432+
433+
rejected_payload = {
434+
"kind": "rejected",
435+
"success": False,
436+
"error": "fleet_quota_exhausted",
437+
"user_visible_message": "quota hit",
438+
"details": None,
439+
}
440+
event_r = {
441+
"tool_name": "mcp__plugin_autoskillit_autoskillit__dispatch_food_truck",
442+
"tool_response": _wrap_for_claude_code(rejected_payload),
443+
}
444+
out_r, _ = _run_hook(event=event_r)
445+
text_r = json.loads(out_r)["hookSpecificOutput"]["updatedMCPToolOutput"]
446+
rejected_expected = {
447+
"error": "fleet_quota_exhausted",
448+
"user_visible_message": "quota hit",
449+
}
450+
for field in rejected_required:
451+
assert field in rejected_expected, (
452+
f"Field {field!r} from _FMT_DISPATCH_REJECTED_REQUIRED has no expected value — "
453+
f"add it to rejected_expected and rejected_payload"
454+
)
455+
assert rejected_expected[field] in text_r, (
456+
f"Rejected shape missing {field!r}: expected {rejected_expected[field]!r} in output"
457+
)
458+
459+
460+
def test_json_producer_includes_completed_failure():
461+
"""_dispatch_food_truck_json_producer must include a DispatchCompleted(success=False)
462+
shape so the coverage contract exercises both success states and the union of keys
463+
includes 'kind'.
464+
465+
Asserts on the individual completed_failure envelope (not the merged dict)
466+
to catch regressions where this specific shape drops 'kind'.
467+
"""
468+
from autoskillit.fleet.state_types import DispatchCompleted, DispatchStatus
469+
from tests.infra.conftest import _dispatch_food_truck_json_producer
470+
471+
keys = _dispatch_food_truck_json_producer().keys()
472+
assert "kind" in keys, (
473+
f"_dispatch_food_truck_json_producer must emit 'kind' field: {sorted(keys)}"
474+
)
475+
failure = DispatchCompleted(
476+
success=False,
477+
dispatch_status=DispatchStatus.FAILURE,
478+
dispatch_id="d-verify",
479+
dispatched_session_id="s-verify",
480+
reason="verify_kind",
481+
)
482+
failure_env = json.loads(failure.to_envelope())
483+
assert "kind" in failure_env, (
484+
f"DispatchCompleted(success=False).to_envelope() must emit 'kind': "
485+
f"{sorted(failure_env.keys())}"
486+
)

0 commit comments

Comments
 (0)