Skip to content

fix(android): do not session.interrupt on idle/retarget watchdog - #668

Closed
ophirhan wants to merge 1 commit into
Codename-11:devfrom
ophirhan:fix/watchdog-no-interrupt-on-retarget
Closed

ophirhan wants to merge 1 commit into
Codename-11:devfrom
ophirhan:fix/watchdog-no-interrupt-on-retarget

Conversation

@ophirhan

@ophirhan ophirhan commented Oct 3, 2026

Copy link
Copy Markdown
Contributor

Summary

After a LAN⇄Tailscale route retarget mid-turn, the Android client armed a 30s settle watchdog that called session.interrupt on silence. That aborted live provider waits (e.g. waiting on the model with no tokens yet). The host then default-labeled the interrupt as user_stop / "explicit stop requested" even though the user never pressed Stop.

Fix

  • Post-retarget rejoin uses the full idle watchdog (no short settle that kills the server turn).
  • Idle watchdog fails the local stream only — does not call session.interrupt.
  • Real Stop still interrupts the gateway turn.
  • Diagnostics log entry on user Stop (chat_user_stop).
  • Log lines distinguish idle_watchdog vs user_stop.

Why this ownership model

  • Agent owns model run / retry / give-up.
  • Client owns UI reconnect + display; only a real Stop (or true server failure) should abort the server turn.

Tests

  • idle watchdog fires when events stop flowing — stream error, no session.interrupt
  • non compacting status keeps the ordinary watchdog — same
  • cancel sends session interrupt — still interrupts
  • active route retarget … preserves the live turn — still passes

Notes

Wire session.interrupt still forbids extra keys (extra=forbid), so interrupt reason is log-only on Android until a host contract change accepts reason/source.

After LAN⇄Tailscale retarget the client armed a 30s settle that called
session.interrupt on silence. That aborted live provider waits and the
host mislabeled them as user_stop / explicit stop requested.

- Post-retarget rejoin uses the full idle watchdog
- Idle watchdog fails the local stream only (no server interrupt)
- Real Stop still interrupts and is logged in Diagnostics
- Unit tests assert watchdog does not interrupt; cancel still does
@hermes-relay-triage hermes-relay-triage Bot added area:android Kotlin app needs-maintainer-review Automated triage exhausted; needs a human labels Oct 3, 2026
@hermes-relay-triage

Copy link
Copy Markdown

🤖 Hermes-Relay automated PR intake

Thanks for addressing the Android idle/retarget watchdog interruption behavior! This targets dev, stays within the Android surface, and describes the intended watchdog/Stop separation with named regression tests and an upstream wire-contract note.

Please fill in the remaining intake evidence using the PR template:

  • Under Verification, add exact test/lint commands and results (or explicitly say what was not run). Test names alone do not establish execution results. For reconnect behavior, also state relevant Gateway contract/lifecycle checks and device/emulator evidence, or what remains outstanding.
  • Restore the remaining template sections and checklist. The current Fix/Tests content can become Changes/Verification; include Screenshots (or “No visual change”), compatibility/risk, and lineage (N/A if original work).
  • Link any related issue/PR, if applicable, and state the changelog update or N/A rationale; no changelog change is listed.

A maintainer will review the implementation, compatibility, and verification evidence; this is intake only, not a code review.

@BrokeSkill

Copy link
Copy Markdown

Ran into this exact failure mode. Sharing what I saw:
Three turns in one session died the same way, at 240s, 241s and 313s of no stream events. The gateway logged each one as Interrupt requested (hard): explicit stop requested / reason=interrupted_during_api_call, which made it a pain to trace as nothing on the server side points at the watchdog, it reads like the user pressed Stop, even the agent itself sees it as that. The session row was never reaped (end_reason stayed NULL) and the app was polling the gateway every second the whole time, so it was purely the idle timer, not the socket.

Two things from that experience worth weighing before this merges:

This PR keeps failFromTransport("Gateway turn timed out") on expiry, so the client-side turn still dies after 180s of quiet. The server work continues, but the app shows the turn as failed and the user loses the view of it. For long prefill (a big prompt where the provider thinks for minutes before the first token) or a long quiet background run, the visible symptom survives the fix. Would you consider not failing the local stream either, and letting settleFromAuthoritativeSessionState or a real disconnect handle a genuinely dead gateway?

The per-event timeouts (watchdogTimeoutFor and the ASK_* / COMPACTING constants) only move the deadline, so they dont stop the kill. And since this PR removes the kill, they have nothing left to do, so they could go too.

+1 on the Server Ownership model. I tend to run overnight runs, and its really discouraging to see them getting interrupted :/

@ophirhan

ophirhan commented Oct 5, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for writing this up — and especially for the concrete timings (240s / 241s / 313s) and the host log shape. That matches the failure mode this PR is aimed at: the client idle path was effectively forging a user Stop (explicit stop requested / interrupted_during_api_call) even while the app kept polling and the session row never reaped. Really glad the Server Ownership framing resonates; overnight runs are exactly the case we don’t want the phone to abort.

On the two follow-ups:

1. Still failing the local stream on idle
You’re right that this PR only removes the server kill. failFromTransport("Gateway turn timed out") remains, so a long quiet prefill or a long background think can still look “dead” in the UI while the gateway keeps working. That’s a real remaining symptom.

The direction we want next is closer to what you suggest: on idle expiry, don’t invent failure — query authoritative session state (session.info / session.activate / exact session.active_list row; same family as ADR 64 settle-from-authoritative) and:

  • still running → keep waiting / rearm the idle timer (UI stays “working”)
  • running=false → settle successfully + bounded history reconcile
  • real disconnect / repeated probe failure → then fail the local stream

I’d rather land that as a small follow-up than expand this PR’s diff mid-review, so the interrupt vs Stop split stays easy to review. Happy to take a patch or issue link if you already have a sketch.

2. Per-event timeouts (watchdogTimeoutFor, ASK_*/COMPACTING)
Agreed they mostly only move the deadline once interrupt-on-idle is gone. They’re still useful as how long to wait before the status probe (asks/compacting legitimately go quiet longer than ordinary deltas), so I wouldn’t delete them in this PR — but if the follow-up makes “idle → probe” the only action, we can simplify that table so it’s clearly a probe cadence, not a kill switch.

Wire note (unchanged here): session.interrupt still forbids extra keys upstream, so interrupt reason stays log-only on Android until the host accepts reason/source. Your “nothing on the server points at the watchdog” pain is exactly why that contract change would help triage.

Thanks again for the field report and for +1’ing the ownership model — this kind of overnight dogfood signal is gold.

@ophirhan

Copy link
Copy Markdown
Contributor Author

Closing in favor of maintainer consolidation #694.

#694 already adapts the useful parts of this PR (cancellation-only session.interrupt diagnostics / co-authorship) and supersedes the local idle-timeout failure path with quiet authoritative recovery instead of failing the stream.

Core policy here:

  • idle / post-retarget quiet must not session.interrupt
  • only explicit Stop cancels the server turn

…is covered more completely by #694 once that lands. Leaving this open would only fight the same GatewayChatClient surface.

Not merging this branch on top of #694.

@ophirhan ophirhan closed this Oct 10, 2026
@Codename-11

Copy link
Copy Markdown
Owner

Apologies for the issues and for taking a while to get back to this, @ophirhan. #694 includes your cancellation logging with co-author credit, and keeps the local stream alive too. Appreciate your help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:android Kotlin app needs-maintainer-review Automated triage exhausted; needs a human

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants