[Kyzcreig]

What & why

Closes the one residual the cascade fix (#70) left behind — and which was originally WONTFIX'd on a false premise. F2 (the restart-loop circuit-breaker) defined "forward progress" as "a clean turn completed," and cleared the breaker on any clean turn. So a session that completes a clean turn AND re-fires a restart every cycle (an autonomous self-restart / cron-resume shape) reset the breaker each cycle and looped forever — undetected by:

• F1 — only tags request_restart//restart, not the dominant safe-restart skill path.
• F2 — reset every cycle by the clean completion.
• F3 — single-flight only catches concurrent watchers; this loop is sequential.

Grounding the "F1+F3 catch it" claim against the code disproved it (F1's _session_initiated_restart is set only by request_restart, not the skill's Popen of the watcher), so this ships a real fix rather than a documented limitation.

The fix (A′ + C1)

A′ — record-mark-at-gate. Extracted the post-clean-turn logic into _apply_post_turn_resume_gate(). If the turn itself initiated a restart (_session_initiated_restart, popped one-shot), record a replay-mark — the same _record_restart_replay_mark call / threshold / suspend+alert the drain path uses — instead of clearing the breaker. A restart-only "clean" turn is loop progress, not work progress, so it accrues and F2 suspends at threshold. Real-work turns still clear (the parent anti-false-trip behavior is byte-preserved in the else branch).

C1 — skill-path observability. The safe-restart skill runs as a terminal tool call in-process; progress_callback now sets _session_initiated_restart when it observes a terminal command that invokes safe-restart.py — via a new _command_invokes_safe_restart matcher that excludes inspection verbs (cat/grep/vim/echo/…) so merely reading the script can't trip it. Both restart paths now converge on the same in-process flag. No sentinel file, no new model tool (purely internal inspection of a tool event the gateway already receives).

Threshold/window reuse the parent fix's restart_loop_threshold (3) / restart_loop_window_secs (300) — same failure class.

Tests — all RED-proven (tests/gateway/test_restart_cascade.py, +7 → 20 total)

• test_self_completing_restart_loop_is_suspended — the loop is suspended at threshold (RED: revert A′ → loops forever).
• test_single_self_restart_then_real_work_not_suspended / test_real_work_turns_never_accrue_marks — feature + parent anti-false-trip preserved (INV-2/INV-3).
• test_initiated_flag_is_one_shot_per_turn — flag popped per turn, no bleed.
• test_skill_path_progress_callback_sets_initiated_flag — C1 trips on invoke, not on inspect.
• test_command_invokes_safe_restart_matcher — direct matcher unit coverage (exec vs inspect vs pipeline), drives the real function.
• test_c1_detection_present_in_real_progress_callback — source-guard that the live C1 branch exists (the behavioral C1 test uses a mirror because the real callback is a closure; this ties the mirror to reality, RED: delete the branch → fails).

Full blast radius green: test_restart_cascade (20) + test_restart_resume_pending/test_restart_drain/test_resume_command/test_clean_shutdown_marker/test_restart_notification (163 total).

Review

Spec reviewed (APPROVE-WITH-CHANGES → R1 "record, don't skip-clear" + R2 "C1 over C2 sentinel" folded into v2). Diff reviewed by two independent passes (APPROVE + APPROVE-WITH-CHANGES, no BLOCK); the one substantive finding — the cat safe-restart.py false-positive — is fixed by the inspection-verb exclusion + tested. Cache/alternation/single-writer invariants untouched; no new core tool, no new env-for-config.

Spec: ~/.hermes/plans/2026-06-21_f2-self-completing-restart-loop-fix.md. Builds on #70.

[github-actions]

🔎 Lint report: fix/f2-self-restart-loop vs origin/main

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 11252 on HEAD, 11252 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 5877 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

[greptile-apps]

Greptile Summary

This PR closes the one residual loop-detection gap left by the cascade fix (#70): a session that completes a clean turn and immediately re-fires a restart every cycle was invisible to F1/F2/F3 because F2 cleared its breaker on any clean turn. Two additions fix this: A′ extracts post-turn logic into _apply_post_turn_resume_gate, which records a replay-mark instead of clearing the breaker when the turn itself initiated a restart, letting marks accumulate until suspension; C1 sets _session_initiated_restart from the progress_callback when it observes a terminal tool start invoking safe-restart.py, so the dominant skill path converges on the same in-process flag as request_restart().

• A′ gate (_apply_post_turn_resume_gate): restart-only turns accrue marks, real-work turns clear them; the parent anti-false-trip invariant is byte-preserved in the else branch.
• C1 skill detection (_command_invokes_safe_restart): tokenises terminal commands per segment, skips a curated list of inspection verbs so cat safe-restart.py is excluded, and the real function is exercised by the test suite—not a mirror.
• Tests (+7 → 20 total): each new test is described as RED-proven against the relevant revert, including a source-guard that ties CI to the live branch's existence.

Confidence Score: 5/5

Safe to merge; the gate and C1 detection logic is correct, the seven new tests are tied directly to the real implementation functions, and the anti-false-trip invariants from the parent fix are preserved.

The A′ gate correctly routes restart-initiated turns to mark accumulation and real-work turns to mark clearing, with the one-shot pop preventing bleed-through. C1 skill detection is accurately scoped to tool.started events for terminal commands that name the script without being inspection verbs. All new tests exercise the real functions, and the source-guard test ties CI to the live branch. The only new finding is a gap in the inspection-verb list that could produce spurious marks in unusual agent turns but does not affect the nominal restart-loop or legitimate-deploy paths.

gateway/run.py — specifically the _SAFE_RESTART_INSPECTION_VERBS tuple and the _apply_post_turn_resume_gate logic; the concerns raised in earlier review rounds about the missing _resumed_this_boot guard and tool.started timing are worth re-evaluating before the next change to this area.

Important Files Changed

Filename	Overview
gateway/run.py	Two additions: _command_invokes_safe_restart module-level matcher (with inspection-verb exclusion list) and _apply_post_turn_resume_gate instance method that records marks instead of clearing them when a turn-initiated restart is detected; C1 branch wired into the existing progress_callback closure. Logic is correct; previous-review concerns about the missing _resumed_this_boot guard and tool.started-before-completion timing remain open.
tests/gateway/test_restart_cascade.py	Seven new tests cover the core loop suspension, one-shot flag invariant, anti-false-trip preservation, C1 callback detection (invoke vs. inspect), direct matcher coverage, and a source-guard asserting the live branch exists. All clearly structured and tied to the real implementation functions.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Boot as Gateway Boot
    participant Runner as GatewayRunner
    participant CB as progress_callback (C1)
    participant Gate as _apply_post_turn_resume_gate (A')
    participant Marks as replay_marks store
    participant Drain as _mark_resume_pending_for_shutdown

    Boot->>Runner: "auto-resume session (resume_pending=True)"
    Runner->>Runner: _resumed_this_boot.add(session_key)
    Note over Runner: agent turn begins

    Runner->>CB: "event=tool.started, tool=terminal"
    CB->>Runner: "_session_initiated_restart[sk] = True"

    Note over Runner: agent turn completes cleanly

    Runner->>Gate: _apply_post_turn_resume_gate(sk)
    Gate->>Gate: "pop _session_initiated_restart[sk] -> True"
    Gate->>Marks: _record_restart_replay_mark(sk)
    Marks-->>Gate: "marks=[t1], count=1 (below threshold)"
    Gate->>Runner: session_store.clear_resume_pending(sk)

    Runner->>Drain: "stop() -> _mark_resume_pending_for_shutdown(sk)"
    Drain->>Runner: session_store.mark_resume_pending(sk)
    Note over Boot: gateway restarts, session re-resumed

    loop Each restart-only cycle
        Runner->>Gate: _apply_post_turn_resume_gate(sk)
        Gate->>Marks: _record_restart_replay_mark(sk)
        Marks-->>Gate: marks grow
    end

    Marks-->>Runner: "marks >= restart_loop_threshold(3)"
    Runner->>Runner: "session_entry.suspended = True"
    Note over Runner: F2 circuit-breaker trips - loop halted

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Boot as Gateway Boot
    participant Runner as GatewayRunner
    participant CB as progress_callback (C1)
    participant Gate as _apply_post_turn_resume_gate (A')
    participant Marks as replay_marks store
    participant Drain as _mark_resume_pending_for_shutdown

    Boot->>Runner: "auto-resume session (resume_pending=True)"
    Runner->>Runner: _resumed_this_boot.add(session_key)
    Note over Runner: agent turn begins

    Runner->>CB: "event=tool.started, tool=terminal"
    CB->>Runner: "_session_initiated_restart[sk] = True"

    Note over Runner: agent turn completes cleanly

    Runner->>Gate: _apply_post_turn_resume_gate(sk)
    Gate->>Gate: "pop _session_initiated_restart[sk] -> True"
    Gate->>Marks: _record_restart_replay_mark(sk)
    Marks-->>Gate: "marks=[t1], count=1 (below threshold)"
    Gate->>Runner: session_store.clear_resume_pending(sk)

    Runner->>Drain: "stop() -> _mark_resume_pending_for_shutdown(sk)"
    Drain->>Runner: session_store.mark_resume_pending(sk)
    Note over Boot: gateway restarts, session re-resumed

    loop Each restart-only cycle
        Runner->>Gate: _apply_post_turn_resume_gate(sk)
        Gate->>Marks: _record_restart_replay_mark(sk)
        Marks-->>Gate: marks grow
    end

    Marks-->>Runner: "marks >= restart_loop_threshold(3)"
    Runner->>Runner: "session_entry.suspended = True"
    Note over Runner: F2 circuit-breaker trips - loop halted

Loading

_{Reviews (3): Last reviewed commit: "Merge branch 'main' into fix/f2-self-res..." | Re-trigger Greptile}

[greptile-apps]

The import shlex statement is placed inside the function body rather than at the module level. Python caches imports in sys.modules so there is no runtime penalty, but the unconventional placement can confuse readers who expect all stdlib imports at the top of the file. The rest of the module follows the top-of-file import convention.

Suggested change

	if not cmd or "safe-restart.py" not in cmd:
	return False
	# Tokenize loosely; the first meaningful token decides intent. A pipeline or
	# &&-chain that contains an execution elsewhere still counts (we scan segments).
	import shlex
	for segment in cmd.replace("&&", "\n").replace("|", "\n").replace(";", "\n").splitlines():
	if not cmd or "safe-restart.py" not in cmd:
	return False
	# Tokenize loosely; the first meaningful token decides intent. A pipeline or
	# &&-chain that contains an execution elsewhere still counts (we scan segments).
	for segment in cmd.replace("&&", "\n").replace("|", "\n").replace(";", "\n").splitlines():

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[greptile-apps]

Gate records marks for non-resumed sessions

_apply_post_turn_resume_gate records a replay-mark whenever _session_initiated_restart is set, regardless of whether the session is in _resumed_this_boot. The drain path has an explicit session_key in _resumed_this_boot guard before calling _record_restart_replay_mark. Without that guard here, a session on its very first run (never auto-resumed) can start accruing marks. In practice this means a session that genuinely uses the safe-restart skill three times within the window (e.g., three rolling deploys in under five minutes) would be suspended before completing a full resume cycle, even though no loop has occurred. The test test_skill_path_progress_callback_sets_initiated_flag explicitly exercises this path without adding the session to _resumed_this_boot, confirming the broader scope is intentional — but the PR description frames the fix narrowly as a resume-loop guard, so the widened scope is worth a conscious decision rather than an implicit consequence.

[greptile-apps]

Marks accrue on tool.started, before execution outcome is known

C1 sets _session_initiated_restart on the tool.started event, before the terminal tool returns a result. If safe-restart.py exits non-zero (e.g., the handoff target isn't ready), no gateway restart occurs, but the flag is already set. When _apply_post_turn_resume_gate runs at turn end, it pops the flag as True and records a mark even though no loop iteration happened. Three such failed invocations within the window would suspend the session. For the nominal success path this is a non-issue (the old gateway shuts down before another turn runs), but a session repeatedly attempting a failed restart could be suspended via the breaker rather than the failure-count path. Whether treating failed invocations as "loop progress" is acceptable is a product call; the current behaviour is at least deterministic and auditable via logger.debug.