Rectify: Codex L2 Liveness Policy#4217
Conversation
Trecek
left a comment
Trecek
left a comment
There was a problem hiding this comment.
AutoSkillit PR Review — Verdict: changes_requested (own-PR guard applied: posted as COMMENT event). 1 critical + 14 warning + 2 decision-required findings posted as inline comments; 11 info findings omitted from inline posts (no line-rendered bug; surface in summary). Resolve and re-request review.
Trecek
left a comment
Trecek
left a comment
There was a problem hiding this comment.
AutoSkillit PR Review — Verdict: changes_requested (own-PR guard applied: posted as COMMENT event). 1 critical + 14 warning + 2 decision-required findings posted as inline comments; 11 info findings omitted from inline posts (no line-rendered bug; surface in summary). Resolve and re-request review.
Trecek
left a comment
Trecek
left a comment
There was a problem hiding this comment.
AutoSkillit PR Review — Verdict: approved_with_comments
| if explicit_idle_disabled: | ||
| return None, True | ||
| if value is not None and value > 0: | ||
| return float(value), False |
There was a problem hiding this comment.
[warning] bugs: _normalize_caller_idle with value=None, default=0 returns (None, False). The is_idle_disabled property returns True because stdout_idle_timeout_sec is None, which is correct — but explicit_idle_disabled=False is misleading. The field semantics say 'explicit caller disabled' but a None config default causes the same is_idle_disabled=True outcome without setting the explicit flag. Data model conflates two conditions.
There was a problem hiding this comment.
Valid observation — flagged for design decision.
| pytestmark = [pytest.mark.layer("contracts"), pytest.mark.small] | ||
|
|
||
|
|
||
| def _resolver_inputs_from_config( |
There was a problem hiding this comment.
[warning] cohesion: Production/test wiring duplication — resolver_inputs_from_config re-declares the same ResolverInputs(run_skill, fleet_, enable_deadline_extension, caller_idle_output_timeout, caller_session_id, is_food_truck) wiring that resolve_session_liveness_spec builds from cfg. The two copies will drift the moment a new ResolverInputs field is added. The contract test should call a single ResolverInputs.from_config(cfg, *, is_food_truck, caller_idle_output_timeout, ...) factory.
There was a problem hiding this comment.
Valid observation — flagged for design decision.
|
|
||
|
|
||
| @pytest.mark.anyio | ||
| async def test_operation_deadline_clamps_child_activity_extension(monkeypatch) -> None: |
There was a problem hiding this comment.
[warning] tests: test_operation_deadline_clamps_child_activity_extension — the 10ms slack on 500ms deadline is workable but the 'unclamped_deadline' assertion is heuristic (relies on probe_times[-1] + poll_interval * 2). Under xdist the recorded probe_times[-1] may not represent the latest probe. Tighten by checking the supervisor floor exactly.
There was a problem hiding this comment.
Valid observation — flagged for design decision.
Trecek
left a comment
Trecek
left a comment
There was a problem hiding this comment.
AutoSkillit review: warning-only findings detected. See inline comments — no blocking changes required.
⚠️ Outside Diff Range
These findings target lines not in the diff and could not be posted as inline comments:
tests/execution/test_liveness_supervisor.py
- L0 [warning/tests]: tests/execution/process/test_liveness_supervisor.py — missing coverage: test_publish_event_failed_drops_operation and te
tests/execution/test_process_session_log_monitor_dispatch_marker.py
- L0 [info/bugs]: test_supervisor_suppresses_stale_before_dispatch_marker_check expects STALE — but the test name says 'suppresses stale b
…ex builders" This reverts commit bb2b03d.
1 participant
Summary
Codex L2 food trucks can be killed by the raw stdout byte-idle watchdog while doing legitimate long MCP work. The immediate failure is not just a low timeout; it is an architectural mismatch between independent timeout scalars and actual backend liveness.
Today, Codex builders copy
run_skill.stream_idle_timeout_msintoCmdSpec.process_idle_timeout_ms, and_execute_claude_headless()treats that value as a lower cap on the outer AutoSkillit process idle watchdog. For Codex, that collapses default L2 food-truck idle from fleet's intended1800sto600s, whilerun_skill.mcp_tool_timeout_secand fleet session deadlines allow much longer blocking calls. Codexitem.started/item.updated status=in_progressMCP records are ignored as liveness, so a healthy in-flight MCP call can look identical to a hung stdout stream.The immunity plan is to replace raw timeout-field propagation with one resolved session liveness contract plus a process-level liveness supervisor. The contract must be resolved once per backend and session kind, then consumed by every watcher. Codex operation lifecycle events must become first-class liveness inputs, bounded by MCP/session deadlines rather than by an arbitrary marker-suppression cap.
Implementation Plan
Plan file:
/home/talon/projects/autoskillit-runs/remediation-20260708-205019-355728/.autoskillit/temp/rectify/rectify_codex_l2_liveness_policy_2026-07-08_211800.mdCloses #4202
🤖 Generated with Claude Code via AutoSkillit