fix(tui): drop stale stream events after ctrl-c interrupt#16706
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens the TUI turn streaming pipeline so that after a Ctrl‑C interrupt, late-arriving gateway stream events (reasoning/tool/message deltas) no longer mutate the current turn’s UI state, and ensures interruption doesn’t suppress the next fresh turn.
Changes:
- Add
this.interruptedearly-exits to additional stream-side recorders (reasoning + tool lifecycle + deltas). - Reset
this.interrupted = falseinstartMessage()to avoid muting the next turn whenmessage.completenever arrives. - Add a regression test covering late events after interrupt and verifying the next turn proceeds normally.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| ui-tui/src/app/turnController.ts | Adds interruption guards across streaming recorders and resets interrupted at message start. |
| ui-tui/src/tests/createGatewayEventHandler.test.ts | Adds a regression test that simulates interrupt + late events and asserts stream state remains empty. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Once interruptTurn() flips this.interrupted, only recordMessageDelta short-circuited. recordReasoningDelta/Available, recordToolStart/ Progress/Complete, and recordInlineDiffToolComplete kept populating turnState until the python loop reached its next _interrupt_requested check (~1s on busy turns), making it look like ctrl-c was ignored while late "thinking" + tool calls kept landing in the UI. Add the same interrupted guard to every stream-side recorder, and clear the flag at startMessage() so the next turn isn't suppressed if the previous turn never delivered message.complete.
…ers in test Copilot review on PR #16706: 1. `recordToolStart` is interruption-guarded, but `tool.start` handler also calls `recordTodos(payload.todos)` first — so a late tool.start carrying todos could still mutate `turnState.todos` after Ctrl-C, leaving ghost rows in the panel. Adds the same `if (this.interrupted) return` early-exit to `recordTodos` so *all* tool.start side-effects are dropped post-interrupt. 2. The interrupt test was leaking a real `setTimeout` (interrupt cooldown) across test files, which could fire later and mutate uiStore from the wrong test context. Wraps the test in `vi.useFakeTimers()` + `vi.runAllTimers()` and restores real timers in finally. 3. Extends the same test with a todos payload on the post-interrupt tool.start so we have explicit regression coverage for #1.
1458c78 to
cb9e106
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Round 2 Copilot review on PR #16706: 1. `tool.generating` events route through `pushTrail`, which was not interruption-guarded — late events could still write 'drafting …' into `turnTrail` after Ctrl-C, leaving a stale shimmer in the UI. Adds the same `if (this.interrupted) return` early-exit. 2. Test cleanup moved `vi.runAllTimers()` into `finally` (before `vi.useRealTimers()`) so a mid-test assertion failure can't leak the interrupt-cooldown setTimeout across other test files. 3. Replaced the misleading 'pre-interrupt todos … expected to be cleared by the interrupt cycle' comment with an accurate one reflecting current behaviour (interrupt does NOT clear todos). 4. Added an explicit assertion that a post-interrupt `tool.generating` event does not extend `turnTrail` — regression coverage for #1.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ulasbilgen
pushed a commit
to ulasbilgen/hermes-adhd-agent
that referenced
this pull request
May 1, 2026
…ch#16706) * fix(tui): drop stale stream events after ctrl-c interrupt Once interruptTurn() flips this.interrupted, only recordMessageDelta short-circuited. recordReasoningDelta/Available, recordToolStart/ Progress/Complete, and recordInlineDiffToolComplete kept populating turnState until the python loop reached its next _interrupt_requested check (~1s on busy turns), making it look like ctrl-c was ignored while late "thinking" + tool calls kept landing in the UI. Add the same interrupted guard to every stream-side recorder, and clear the flag at startMessage() so the next turn isn't suppressed if the previous turn never delivered message.complete. * fix(tui): guard recordTodos against post-interrupt mutation; fake-timers in test Copilot review on PR NousResearch#16706: 1. `recordToolStart` is interruption-guarded, but `tool.start` handler also calls `recordTodos(payload.todos)` first — so a late tool.start carrying todos could still mutate `turnState.todos` after Ctrl-C, leaving ghost rows in the panel. Adds the same `if (this.interrupted) return` early-exit to `recordTodos` so *all* tool.start side-effects are dropped post-interrupt. 2. The interrupt test was leaking a real `setTimeout` (interrupt cooldown) across test files, which could fire later and mutate uiStore from the wrong test context. Wraps the test in `vi.useFakeTimers()` + `vi.runAllTimers()` and restores real timers in finally. 3. Extends the same test with a todos payload on the post-interrupt tool.start so we have explicit regression coverage for NousResearch#1. * fix(tui): guard pushTrail post-interrupt; harden interrupt-test cleanup Round 2 Copilot review on PR NousResearch#16706: 1. `tool.generating` events route through `pushTrail`, which was not interruption-guarded — late events could still write 'drafting …' into `turnTrail` after Ctrl-C, leaving a stale shimmer in the UI. Adds the same `if (this.interrupted) return` early-exit. 2. Test cleanup moved `vi.runAllTimers()` into `finally` (before `vi.useRealTimers()`) so a mid-test assertion failure can't leak the interrupt-cooldown setTimeout across other test files. 3. Replaced the misleading 'pre-interrupt todos … expected to be cleared by the interrupt cycle' comment with an accurate one reflecting current behaviour (interrupt does NOT clear todos). 4. Added an explicit assertion that a post-interrupt `tool.generating` event does not extend `turnTrail` — regression coverage for NousResearch#1.
02356abc
pushed a commit
to 02356abc/hermes-agent
that referenced
this pull request
May 14, 2026
…ch#16706) * fix(tui): drop stale stream events after ctrl-c interrupt Once interruptTurn() flips this.interrupted, only recordMessageDelta short-circuited. recordReasoningDelta/Available, recordToolStart/ Progress/Complete, and recordInlineDiffToolComplete kept populating turnState until the python loop reached its next _interrupt_requested check (~1s on busy turns), making it look like ctrl-c was ignored while late "thinking" + tool calls kept landing in the UI. Add the same interrupted guard to every stream-side recorder, and clear the flag at startMessage() so the next turn isn't suppressed if the previous turn never delivered message.complete. * fix(tui): guard recordTodos against post-interrupt mutation; fake-timers in test Copilot review on PR NousResearch#16706: 1. `recordToolStart` is interruption-guarded, but `tool.start` handler also calls `recordTodos(payload.todos)` first — so a late tool.start carrying todos could still mutate `turnState.todos` after Ctrl-C, leaving ghost rows in the panel. Adds the same `if (this.interrupted) return` early-exit to `recordTodos` so *all* tool.start side-effects are dropped post-interrupt. 2. The interrupt test was leaking a real `setTimeout` (interrupt cooldown) across test files, which could fire later and mutate uiStore from the wrong test context. Wraps the test in `vi.useFakeTimers()` + `vi.runAllTimers()` and restores real timers in finally. 3. Extends the same test with a todos payload on the post-interrupt tool.start so we have explicit regression coverage for NousResearch#1. * fix(tui): guard pushTrail post-interrupt; harden interrupt-test cleanup Round 2 Copilot review on PR NousResearch#16706: 1. `tool.generating` events route through `pushTrail`, which was not interruption-guarded — late events could still write 'drafting …' into `turnTrail` after Ctrl-C, leaving a stale shimmer in the UI. Adds the same `if (this.interrupted) return` early-exit. 2. Test cleanup moved `vi.runAllTimers()` into `finally` (before `vi.useRealTimers()`) so a mid-test assertion failure can't leak the interrupt-cooldown setTimeout across other test files. 3. Replaced the misleading 'pre-interrupt todos … expected to be cleared by the interrupt cycle' comment with an accurate one reflecting current behaviour (interrupt does NOT clear todos). 4. Added an explicit assertion that a post-interrupt `tool.generating` event does not extend `turnTrail` — regression coverage for NousResearch#1.
dannyJ848
pushed a commit
to dannyJ848/hermes-agent
that referenced
this pull request
May 17, 2026
…ch#16706) * fix(tui): drop stale stream events after ctrl-c interrupt Once interruptTurn() flips this.interrupted, only recordMessageDelta short-circuited. recordReasoningDelta/Available, recordToolStart/ Progress/Complete, and recordInlineDiffToolComplete kept populating turnState until the python loop reached its next _interrupt_requested check (~1s on busy turns), making it look like ctrl-c was ignored while late "thinking" + tool calls kept landing in the UI. Add the same interrupted guard to every stream-side recorder, and clear the flag at startMessage() so the next turn isn't suppressed if the previous turn never delivered message.complete. * fix(tui): guard recordTodos against post-interrupt mutation; fake-timers in test Copilot review on PR NousResearch#16706: 1. `recordToolStart` is interruption-guarded, but `tool.start` handler also calls `recordTodos(payload.todos)` first — so a late tool.start carrying todos could still mutate `turnState.todos` after Ctrl-C, leaving ghost rows in the panel. Adds the same `if (this.interrupted) return` early-exit to `recordTodos` so *all* tool.start side-effects are dropped post-interrupt. 2. The interrupt test was leaking a real `setTimeout` (interrupt cooldown) across test files, which could fire later and mutate uiStore from the wrong test context. Wraps the test in `vi.useFakeTimers()` + `vi.runAllTimers()` and restores real timers in finally. 3. Extends the same test with a todos payload on the post-interrupt tool.start so we have explicit regression coverage for NousResearch#1. * fix(tui): guard pushTrail post-interrupt; harden interrupt-test cleanup Round 2 Copilot review on PR NousResearch#16706: 1. `tool.generating` events route through `pushTrail`, which was not interruption-guarded — late events could still write 'drafting …' into `turnTrail` after Ctrl-C, leaving a stale shimmer in the UI. Adds the same `if (this.interrupted) return` early-exit. 2. Test cleanup moved `vi.runAllTimers()` into `finally` (before `vi.useRealTimers()`) so a mid-test assertion failure can't leak the interrupt-cooldown setTimeout across other test files. 3. Replaced the misleading 'pre-interrupt todos … expected to be cleared by the interrupt cycle' comment with an accurate one reflecting current behaviour (interrupt does NOT clear todos). 4. Added an explicit assertion that a post-interrupt `tool.generating` event does not extend `turnTrail` — regression coverage for NousResearch#1.
gweeteve
pushed a commit
to gweeteve/hermes-agent
that referenced
this pull request
Jun 2, 2026
…ch#16706) * fix(tui): drop stale stream events after ctrl-c interrupt Once interruptTurn() flips this.interrupted, only recordMessageDelta short-circuited. recordReasoningDelta/Available, recordToolStart/ Progress/Complete, and recordInlineDiffToolComplete kept populating turnState until the python loop reached its next _interrupt_requested check (~1s on busy turns), making it look like ctrl-c was ignored while late "thinking" + tool calls kept landing in the UI. Add the same interrupted guard to every stream-side recorder, and clear the flag at startMessage() so the next turn isn't suppressed if the previous turn never delivered message.complete. * fix(tui): guard recordTodos against post-interrupt mutation; fake-timers in test Copilot review on PR NousResearch#16706: 1. `recordToolStart` is interruption-guarded, but `tool.start` handler also calls `recordTodos(payload.todos)` first — so a late tool.start carrying todos could still mutate `turnState.todos` after Ctrl-C, leaving ghost rows in the panel. Adds the same `if (this.interrupted) return` early-exit to `recordTodos` so *all* tool.start side-effects are dropped post-interrupt. 2. The interrupt test was leaking a real `setTimeout` (interrupt cooldown) across test files, which could fire later and mutate uiStore from the wrong test context. Wraps the test in `vi.useFakeTimers()` + `vi.runAllTimers()` and restores real timers in finally. 3. Extends the same test with a todos payload on the post-interrupt tool.start so we have explicit regression coverage for NousResearch#1. * fix(tui): guard pushTrail post-interrupt; harden interrupt-test cleanup Round 2 Copilot review on PR NousResearch#16706: 1. `tool.generating` events route through `pushTrail`, which was not interruption-guarded — late events could still write 'drafting …' into `turnTrail` after Ctrl-C, leaving a stale shimmer in the UI. Adds the same `if (this.interrupted) return` early-exit. 2. Test cleanup moved `vi.runAllTimers()` into `finally` (before `vi.useRealTimers()`) so a mid-test assertion failure can't leak the interrupt-cooldown setTimeout across other test files. 3. Replaced the misleading 'pre-interrupt todos … expected to be cleared by the interrupt cycle' comment with an accurate one reflecting current behaviour (interrupt does NOT clear todos). 4. Added an explicit assertion that a post-interrupt `tool.generating` event does not extend `turnTrail` — regression coverage for NousResearch#1.
Egavasyug
pushed a commit
to Egavasyug/hermes-agent
that referenced
this pull request
Jun 10, 2026
…ch#16706) * fix(tui): drop stale stream events after ctrl-c interrupt Once interruptTurn() flips this.interrupted, only recordMessageDelta short-circuited. recordReasoningDelta/Available, recordToolStart/ Progress/Complete, and recordInlineDiffToolComplete kept populating turnState until the python loop reached its next _interrupt_requested check (~1s on busy turns), making it look like ctrl-c was ignored while late "thinking" + tool calls kept landing in the UI. Add the same interrupted guard to every stream-side recorder, and clear the flag at startMessage() so the next turn isn't suppressed if the previous turn never delivered message.complete. * fix(tui): guard recordTodos against post-interrupt mutation; fake-timers in test Copilot review on PR NousResearch#16706: 1. `recordToolStart` is interruption-guarded, but `tool.start` handler also calls `recordTodos(payload.todos)` first — so a late tool.start carrying todos could still mutate `turnState.todos` after Ctrl-C, leaving ghost rows in the panel. Adds the same `if (this.interrupted) return` early-exit to `recordTodos` so *all* tool.start side-effects are dropped post-interrupt. 2. The interrupt test was leaking a real `setTimeout` (interrupt cooldown) across test files, which could fire later and mutate uiStore from the wrong test context. Wraps the test in `vi.useFakeTimers()` + `vi.runAllTimers()` and restores real timers in finally. 3. Extends the same test with a todos payload on the post-interrupt tool.start so we have explicit regression coverage for NousResearch#1. * fix(tui): guard pushTrail post-interrupt; harden interrupt-test cleanup Round 2 Copilot review on PR NousResearch#16706: 1. `tool.generating` events route through `pushTrail`, which was not interruption-guarded — late events could still write 'drafting …' into `turnTrail` after Ctrl-C, leaving a stale shimmer in the UI. Adds the same `if (this.interrupted) return` early-exit. 2. Test cleanup moved `vi.runAllTimers()` into `finally` (before `vi.useRealTimers()`) so a mid-test assertion failure can't leak the interrupt-cooldown setTimeout across other test files. 3. Replaced the misleading 'pre-interrupt todos … expected to be cleared by the interrupt cycle' comment with an accurate one reflecting current behaviour (interrupt does NOT clear todos). 4. Added an explicit assertion that a post-interrupt `tool.generating` event does not extend `turnTrail` — regression coverage for NousResearch#1.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Once `interruptTurn()` sets `this.interrupted = true`, only
`recordMessageDelta` short-circuited. `recordReasoningDelta` /
`recordReasoningAvailable` / `recordToolStart` / `recordToolProgress` /
`recordToolComplete` / `recordInlineDiffToolComplete` all kept
mutating `turnState` until python's next `_interrupt_requested`
check (~1s on busy turns).
User-visible symptom (per discord report): "ctrl-c kinda broke in TUI
— now if i ctrl-c it interrupts, but then keeps going a second later,
and i keep getting thinking and tool calls."
Fix
stream-side recorder.
isn't accidentally muted if the previous turn never delivered
`message.complete`.
Test
`ui-tui/src/tests/createGatewayEventHandler.test.ts` — drives the
event sequence `message.start → tool.start → interruptTurn →
{reasoning,tool}.* → message.delta` and asserts that all stream state
(`reasoningText`, `bufRef`, `streamSegments`, `streamPendingTools`,
`tools`) stays empty. Then sends a fresh `message.start` +
`reasoning.delta` and confirms the new turn is not suppressed.
Verified the test fails on `origin/main` before the fix and passes
after. All 385 ui-tui tests + tsc clean.