Skip to content

fix(gateway): preserve-and-prompt on restart instead of silently skipping interrupted work - #136

Merged
Kyzcreig merged 10 commits into
mainfrom
feat/gateway-preserve-prompt-restart
Jul 1, 2026
Merged

Kyzcreig merged 10 commits into
mainfrom
feat/gateway-preserve-prompt-restart

Conversation

@Kyzcreig

@Kyzcreig Kyzcreig commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

What & why

Today when the gateway restarts mid-turn (deploy, OOM, safe-restart.py, crash), the resume note tells the model to "skip any unfinished work from the conversation history" (gateway/run.py) — silently dropping an interrupted long task. The user's hour-long turn just vanishes.

This changes the behavior to preserve-and-prompt: on reconnect the agent surfaces exactly where it stopped — what it had completed, what was in-flight — and asks whether to pick it up or redirect, then waits. It does not auto-continue an arbitrary in-flight turn unattended (that path was explicitly descoped — it carried an outage-class self-restart-loop risk for near-zero marginal value when the human is present).

Design (super-passed: 4 Opus review passes BLOCK→BLOCK→AWC→APPROVE + a code-review BLOCK→fix→verify)

  • (d) Surface-and-ask resume note. In the _is_resume_pending arm, when the reloaded tail is a synthetic interrupt-close turn, emit a "summarize completed + in-flight, then ask go-or-redirect" note instead of "skip unfinished work". The B14 backstop clause ("any restart/shutdown command already ran — do NOT re-execute or verify it") is retained in all branches. Note placement is unchanged (prompt cache safe); only the text differs.
  • Structural discriminator (not content-equality). The synthetic close turn is flagged via finish_reason="interrupt_close" — an existing SQLite column that round-trips — so a model that merely emits the literal text "Operation interrupted." can never mis-trigger the branch. (An in-memory _interrupt_close bool is also set for the pre-persist path.)
  • Boundary hardening. close_interrupted_tool_sequence now makes every interrupt shape API-valid (tool tail, partial-text tail, dangling tool_calls tail) and flagged, so the reload is valid and the summary accurate.
  • INV-D7 deterministic interlock. Because the resume turn is internal (no human message), the executor deterministically blocks forward/mutating tool calls on that turn until the next genuine human message — so "never auto-continue" is enforced, not just prompted. Pure reads are allowed; a blocked forward call emits RESUME_AUTOCONTINUE_VIOLATION. Turn-scoped (holds across model rounds), cleared only on the next human turn.

Tests (17 focused, all green)

  • tests/gateway/test_resume_flag_roundtrip.py — the load-bearing gate: persists a real interrupted turn to a temp HERMES_HOME via the actual SessionDB, reloads via get_messages, and asserts the discriminator survives SQLite (a plain "Operation interrupted." with finish_reason=None reloads as NOT an interrupt-close — collision-proof).
  • test_resume_surface_and_ask.py — note text: surface-and-ask branch has no "skip unfinished work" / no "continue"; idle branch unchanged; collision control; new-message case.
  • test_resume_interlock.py — the interlock blocks a mock model that ignores the note, is turn-scoped (round 2 still blocked), and resets on the next human turn.
  • test_interrupt_close_flag.py, test_resume_summary_only_reset.py, test_resume_note_cache_safe.py.

Full-suite failures on this branch were verified pre-existing (baseline fork/main produces the same transport/credential-test failures under a hermetic sandbox; this branch is +net-passing with zero regressions attributable to the diff).

Scope: Phase 3 (cross-path restart coalescing) is intentionally a separate PR.

@github-actions

github-actions Bot commented Jul 1, 2026 •

Copy link
Copy Markdown

🔎 Lint report: feat/gateway-preserve-prompt-restart 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: 12263 on HEAD, 12243 on base (🆕 +20)

🆕 New issues (13):

Rule Count
unresolved-attribute 9
invalid-assignment 3
invalid-argument-type 1
First entries
tests/agent/test_resume_interlock.py:52: [unresolved-attribute] unresolved-attribute: Unresolved attribute `tool_delay` on type `AIAgent`
tests/agent/test_resume_interlock.py:53: [unresolved-attribute] unresolved-attribute: Unresolved attribute `compression_enabled` on type `AIAgent`
tests/agent/test_resume_interlock.py:50: [unresolved-attribute] unresolved-attribute: Unresolved attribute `_cached_system_prompt` on type `AIAgent`
tests/agent/test_resume_interlock.py:109: [unresolved-attribute] unresolved-attribute: Unresolved attribute `tool_progress_callback` on type `AIAgent`
tests/agent/test_resume_interlock.py:128: [unresolved-attribute] unresolved-attribute: Unresolved attribute `_resume_summary_only` on type `AIAgent`
tests/gateway/test_resume_flag_roundtrip.py:65: [invalid-argument-type] invalid-argument-type: Argument to bound method `SessionDB.append_message` is incorrect: Expected `str`, found `None`
tests/run_agent/test_credits_notices_toggle.py:76: [invalid-assignment] invalid-assignment: Object of type `None` is not assignable to attribute `_credits_session_start_micros` of type `int`
tests/agent/test_resume_interlock.py:54: [unresolved-attribute] unresolved-attribute: Unresolved attribute `save_trajectories` on type `AIAgent`
gateway/run.py:17963: [invalid-assignment] invalid-assignment: Object of type `Literal[True]` is not assignable to attribute `_resume_summary_only` on type `(Any & ~None) | AIAgent`
tests/agent/test_resume_interlock.py:123: [unresolved-attribute] unresolved-attribute: Object of type `AIAgent` has no attribute `tool_progress_callback`
tests/agent/test_resume_interlock.py:103: [unresolved-attribute] unresolved-attribute: Object of type `AIAgent` has no attribute `_resume_summary_only`
gateway/run.py:17983: [invalid-assignment] invalid-assignment: Object of type `Literal[False]` is not assignable to attribute `_resume_summary_only` on type `(Any & ~None) | AIAgent`
tests/agent/test_resume_interlock.py:51: [unresolved-attribute] unresolved-attribute: Unresolved attribute `_use_prompt_caching` on type `AIAgent`

✅ Fixed issues (2):

Rule Count
unresolved-attribute 2
First entries
run_agent.py:3089: [unresolved-attribute] unresolved-attribute: Object of type `Self@get_credits_spent_micros` has no attribute `_credits_session_start_micros`
tests/run_agent/test_credits_notices_toggle.py:76: [unresolved-attribute] unresolved-attribute: Unresolved attribute `_credits_session_start_micros` on type `AIAgent`

Unchanged: 6414 pre-existing issues carried over.

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

@greptile-apps

greptile-apps Bot commented Jul 1, 2026 •

Copy link
Copy Markdown

Greptile Summary

This PR replaces the gateway's silent "skip unfinished work" resume behaviour with a preserve-and-prompt flow: on reconnect after a restart, the agent surfaces what it completed and what was in-flight, then waits for the user to decide rather than silently dropping the interrupted turn.

  • Structural discriminator (finish_reason=\"interrupt_close\") is set on synthetic close turns and survives the SQLite round-trip, so the resume branch keys on a DB column rather than fragile content-equality. An in-place mutation path (update_message_finish_reason) re-persists the flag for already-flushed assistant-text tails.
  • _block_resume_summary_only_tools interlock (INV-D7) enforces "no auto-continue" at the executor level for the entire resume turn, not just the first tool round; cleared only on the next genuine human message.
  • Boundary hardening in close_interrupted_tool_sequence makes all three interrupt shapes (tool-tail, dangling assistant(tool_calls), partial-text tail) API-valid and flagged before persist.

Confidence Score: 4/5

Safe to merge with one targeted fix: the in-place re-persist path in run_agent.py silently skips update_message_finish_reason when append_message returns a non-integer row ID, with no log warning to surface the failure.

The in-place assistant-text-tail re-persist path silently skips update_message_finish_reason when append_message returns a non-integer row ID, with no log warning. If this ever occurs, the surface-and-ask branch invisibly reverts to the old skip-unfinished-work behaviour with no observable signal.

run_agent.py around the isinstance(_row_id, int) guard in _flush_messages_to_session_db, and hermes_state.py update_message_finish_reason for the missing rowcount check.

Important Files Changed

Filename Overview
gateway/run.py Adds _is_interrupt_close_tail, _build_resume_pending_message, and _clear_resume_summary_only_for_human_turn helpers; replaces the old "skip unfinished work" resume note with the new conditional surface-and-ask note; sets/clears the _resume_summary_only interlock flag across all message-handling branches.
run_agent.py Extends _flush_messages_to_session_db to track SQLite row IDs per flushed message dict and re-persist finish_reason="interrupt_close" via update_message_finish_reason when a message is mutated in-place; the re-persist path silently skips if append_message returns a non-integer, with no log warning.
agent/message_sanitization.py Extends close_interrupted_tool_sequence to handle three interrupt shapes (tool-tail, dangling assistant(tool_calls), partial-text assistant tail); sets both _interrupt_close: True (in-memory) and finish_reason="interrupt_close" (SQLite-durable discriminator) on all flagged closes.
agent/tool_executor.py Adds _block_resume_summary_only_tools interlock that blocks all tool calls while _resume_summary_only is True, emitting RESUME_AUTOCONTINUE_VIOLATION only for non-read-only tools; wired into both execute_tool_calls_concurrent and execute_tool_calls_sequential as an early return.
hermes_state.py Adds update_message_finish_reason targeted UPDATE for re-persisting the finish_reason column on an already-written row; does not verify rowcount, so a missed row update is a silent no-op.
tests/gateway/test_resume_flag_roundtrip.py Load-bearing acceptance gate: uses real SessionDB to verify the finish_reason="interrupt_close" discriminator survives a full append-reload cycle (including the in-place mutation / re-persist path) and that a plain "Operation interrupted." with no flag does not trigger the surface-and-ask branch.
tests/agent/test_resume_interlock.py Covers forward-tool blocking, turn-scoped persistence of the interlock across multiple rounds, read-only tool suppression of VIOLATION logs, and normal-turn pass-through; uses mock handle_function_call to assert zero real invocations.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant GW as Gateway
    participant DB as SessionDB
    participant Exec as tool_executor
    participant Model as LLM

    Note over GW,Model: Mid-turn gateway restart
    GW->>DB: "append_message finish_reason=interrupt_close"
    Note over DB: Discriminator survives SQLite round-trip

    Note over GW,Model: Next connection / resume
    GW->>DB: get_messages(session_id)
    DB-->>GW: "history tail has finish_reason=interrupt_close"
    GW->>GW: _is_interrupt_close_tail returns True
    GW->>GW: "_build_resume_pending_message returns surface_and_ask=True"
    GW->>GW: "agent._resume_summary_only = True"
    GW->>Model: System note - tell user what completed, ask go-or-redirect

    Model-->>Exec: tool_call emitted ignoring note
    Exec->>Exec: _block_resume_summary_only_tools blocks call
    Note over Exec: RESUME_AUTOCONTINUE_VIOLATION logged
    Exec-->>Model: synthetic error result summarize-only await user go

    Model-->>GW: Text summary asking user

    Note over GW,Model: Next human turn with go message
    GW->>GW: _clear_resume_summary_only_for_human_turn sets False
    GW->>Model: Normal turn tools unblocked
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 GW as Gateway
    participant DB as SessionDB
    participant Exec as tool_executor
    participant Model as LLM

    Note over GW,Model: Mid-turn gateway restart
    GW->>DB: "append_message finish_reason=interrupt_close"
    Note over DB: Discriminator survives SQLite round-trip

    Note over GW,Model: Next connection / resume
    GW->>DB: get_messages(session_id)
    DB-->>GW: "history tail has finish_reason=interrupt_close"
    GW->>GW: _is_interrupt_close_tail returns True
    GW->>GW: "_build_resume_pending_message returns surface_and_ask=True"
    GW->>GW: "agent._resume_summary_only = True"
    GW->>Model: System note - tell user what completed, ask go-or-redirect

    Model-->>Exec: tool_call emitted ignoring note
    Exec->>Exec: _block_resume_summary_only_tools blocks call
    Note over Exec: RESUME_AUTOCONTINUE_VIOLATION logged
    Exec-->>Model: synthetic error result summarize-only await user go

    Model-->>GW: Text summary asking user

    Note over GW,Model: Next human turn with go message
    GW->>GW: _clear_resume_summary_only_for_human_turn sets False
    GW->>Model: Normal turn tools unblocked
Loading

Reviews (3): Last reviewed commit: "fix(gateway): persist in-place interrupt..." | Re-trigger Greptile

Comment thread agent/tool_executor.py
Comment thread gateway/run.py
Apollo added 2 commits June 30, 2026 19:11
…th test) + gitleaks-allow test fixture key
…-2 C1)

The resume discriminator finish_reason=interrupt_close is set in place on an
existing plain-text assistant tail by close_interrupted_tool_sequence. If that
message was already flushed to state.db mid-turn, _flush_messages_to_session_db
skipped it by identity -> flag lost on reload -> resume silently fell to the
'skip unfinished work' branch (the INV-D2 defect, on the assistant-text-tail
shape instead of the tool-tail shape).

Fix: track each flushed message's DB row id (_flushed_db_row_ids) and, when an
already-flushed message now carries interrupt_close, issue a targeted
SessionDB.update_message_finish_reason UPDATE (once, idempotent via
_interrupt_close_repersisted_ids). Path-independent: fixes the whole bug class
across all 4 close_interrupted_tool_sequence call sites, not a reachability
argument.

Proven by test_interrupt_close_flag_survives_INPLACE_assistant_tail_roundtrip
(drives the real flush path for the initial persist, mutates in place,
re-flushes, asserts finish_reason survives + no duplicate row). 193 persistence
tests green, no regression.
Comment thread run_agent.py
@Kyzcreig
Kyzcreig merged commit c63fc0d into main Jul 1, 2026
60 of 62 checks passed
@Kyzcreig
Kyzcreig deleted the feat/gateway-preserve-prompt-restart branch July 1, 2026 02:42
Kyzcreig added a commit that referenced this pull request Jul 1, 2026
…bservability + reboot-parity proof (#142)

* fix(gateway): auto-surface a self-initiated restart that was drain-interrupted (restart_consumed_interrupted)

A turn whose last act fires safe-restart.py sets _session_initiated_restart, so
on shutdown the F1/F2 cascade-breaker stamps restart_consumed — deliberately
EXCLUDED from _AUTO_RESUME_REASONS to break the restart->resume->restart cascade.
But if that same turn was ALSO still running when the 180s drain timed out
(genuinely interrupted in-flight work), it fell in a seam: the cascade guard
suppressed the auto-wake AND #136 preserve-and-prompt never got to surface it,
so the session stayed silent until the next inbound message (observed live
2026-06-30 20:30 — Apollo's Discord turn preserved but not surfaced).

Fix (restart-only; reboot is unaffected — proven): a new resume_reason
restart_consumed_interrupted, stamped when a self-initiated restart is marked at
the POST-DRAIN-TIMEOUT site (interrupted=True) rather than the pre-drain
speculative site. It IS in _AUTO_RESUME_REASONS (so startup auto-resume
proactively surfaces the interrupted work, surface-and-wait, no tool replay),
but still records the F2 replay-mark, so a genuine restart->resume->restart loop
is still bounded and suspended at threshold within the 300s window.

- _resume_reason_for_shutdown_mark gains interrupted kwarg; _self_initiated
  helper also treats an entry already carrying restart_consumed[_interrupted] as
  self-initiated.
- Only the post-timeout mark site passes interrupted=True (the pre-drain
  speculative mark, which is cleared on graceful finish, stays interrupted=False).
- _resume_reason_phrase maps the new reason to 'a gateway restart'.

Reboot parity (Task C, proof-not-code): a self-initiated REBOOT never sets
_session_initiated_restart (the detector matches literal safe-restart.py, not
safe-reboot.py; reboot-watcher writes no breadcrumb), so it marks shutdown_timeout
(already auto-resumes). Reboot inherits #136/#137 preservation via the shared
SIGTERM spine unchanged.

Tests: 7 cascade unit tests + 1 integration call-site gate (real stop() through
the drain-timeout branch) + 2 reboot-parity tests. The integration test is
MUTATION-PROVEN: flipping the post-timeout site to interrupted=False turns it RED.
Change-detector allow-list test converted to an additive-invariant assertion.
Spec: plans/2026-07-01_restart-reboot-continuity-observability-SPEC.md (pass-3
APPROVE, all RCs folded).

* feat(gateway): PHASE= observability for the shutdown mark + boot-resume decisions

Task B (spec 2026-07-01 restart-reboot-continuity). The gateway shutdown spine
already logs phase TIMINGS ('Shutdown phase: drain done at +Xs', #140's
'Executor drain:', 'Auto-resume skipped') — that existing 'Auto-resume skipped'
line is what let us diagnose the 20:30 seam. This adds the two per-session
DECISION points that were invisible:

- PHASE=shutdown_mark key=... reason=... interrupted=... replay_marked=... marked=...
  at _mark_resume_pending_for_shutdown — makes the restart_consumed vs
  restart_consumed_interrupted vs shutdown_timeout choice observable per session.
- PHASE=boot_resume_scheduled key=... reason=... platform=...
  when a session is proactively resumed at boot (surface-and-wait).

Both use the module logger at INFO (INV-3/D-8: no fsync, no rollover in the hot
path — logging must never eat the 180s drain budget) and carry session key +
flags ONLY, never transcript content (INV-6).

Tests: PHASE-line-emitted, no-content-leak (INV-6), and a slow/blocking-handler
latency gate (D-8 — proves the mark path doesn't fan out or block). Watcher-side
PHASE= markers (safe-restart watcher.py, safe-reboot reboot-watcher.py) ship in
the skills tree.

* fix(gateway): address Opus code-review — real INV-6/INV-3 gates, no overclaim

Opus code-review BLOCKed the first cut: two invariant tests passed vacuously.
Fixed both to be genuine gates, plus the required-changes:

- INV-6 (no transcript content): the test was a TAUTOLOGY — it declared a SECRET
  marker but never seeded it where the PHASE logger could reach it, so the
  assertion was true by construction. Now seeds the sentinel into the transcript
  (append_to_transcript), _pending_messages, and the running-agent object, then
  asserts it never appears in any PHASE= line. MUTATION-PROVEN: adding msg=%s to
  the log line turns it RED.
- INV-3 (fail-open / non-blocking): the old test over-claimed 'non-blocking' with
  a bound its own docstring admitted didn't prove it. Replaced with an honest
  single-emit + FAIL-OPEN gate, and made the CODE genuinely fail-open (both PHASE
  logs wrapped in try/except so a raising/broken sink can't abort the resume-mark
  — losing the mark = losing the interrupted work). Dropped the overclaiming code
  comment (handler blocking is a gateway-logging-config concern, not this site's).
- RC#3: added a direct reason-UPGRADE test (mark clean restart_consumed → mark
  interrupted → upgraded to restart_consumed_interrupted) so the overwrite can't
  silently regress if mark_resume_pending ever gains idempotency.
- Residual #31 (reason drift): added a test proving a self-restart reason does NOT
  leak into a later unrelated shutdown (clear_resume_pending nulls resume_reason +
  the post-turn gate pops _session_initiated_restart → back to shutdown_timeout).
- Test hygiene: the logger-level test now restores the prior level in finally.
- Maintainability: promoted the 4-site magic literal to a named module constant
  _REASON_RESTART_CONSUMED_INTERRUPTED.

199 passed / 1 skipped.

* fix(gateway): code-review-2 follow-ups — WARNING-level audit breadcrumb + restart_consumed constant

Code-review pass-2 = APPROVE WITH CHANGES (0 blockers; both prior fake-gate
BLOCKERs certified genuinely resolved). Two non-blocking RCs landed:

- (SRE) PHASE breadcrumbs were logger.info → lost when a deployment runs
  gateway.run at WARNING+, exactly while diagnosing a bad restart. Made the mark
  log LEVEL-AWARE: an INTERRUPTED or replay-marked mark (the diagnostic case) and
  the boot_resume_scheduled breadcrumb now log at WARNING (survive a raised
  threshold, matching the sibling drain-timeout warning); a routine clean mark
  stays INFO so normal shutdowns don't spam WARNING.
- (Maintainability) promoted the clean-sibling literal 'restart_consumed' to a
  named constant _REASON_RESTART_CONSUMED alongside its twin
  _REASON_RESTART_CONSUMED_INTERRUPTED, so the cascade-guard pair moves together.

Residual #31 (reason drift) verified against the PRODUCTION path:
_apply_post_turn_resume_gate (called post-turn at the run loop) pops
_session_initiated_restart AND calls clear_resume_pending (nulls resume_reason) —
so the drift the test proves absent cannot ship.

Tests: WARNING-level-survives + clean-stays-INFO + caplog-propagation-pin.
209 passed / 1 skipped.

---------

Co-authored-by: Apollo <apollo@ang.ventures>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant