Skip to content

fix(coding-agent): cancel the final kernel snapshot before disposal cleanup - #1708

Closed
snimu wants to merge 13 commits into
snimu/tui-queue-single-sourcefrom
snimu/kernel-snapshot-dispose-timeout
Closed

fix(coding-agent): cancel the final kernel snapshot before disposal cleanup#1708
snimu wants to merge 13 commits into
snimu/tui-queue-single-sourcefrom
snimu/kernel-snapshot-dispose-timeout

Conversation

@snimu

@snimu snimu commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

What was wrong

When a kernel was being disposed, its final state snapshot was raced against a 5-second guard that couldn't cancel anything: if the snapshot was slow, the guard "won", disposal proceeded to close the ZMQ sockets, and the still-running snapshot then failed against dead sockets.

The fix

The final snapshot now runs through the kernel's existing cancellable execution path (interrupt, then force-abort after a grace period). Disposal waits for the snapshot to settle — completed or cleanly aborted — before any socket cleanup. The separate dispose-timeout constant and the uncancellable race are deleted; there is one timeout policy for snapshot execution, applied in one place. Both teardown paths (dispose and shutdown-with-snapshot) share the fix.

Net −13 lines.

How it's verified

The existing timeout test was updated to exercise the real flow: dispose() directly, the timeout starting only after queued work, and cleanup deferred until the aborted snapshot settles. tsgo/biome/suite green, re-validated after each stack merge. Two-model implement/review loop, approved first pass.

Stacked on #1705 (test the whole stack at the leaf; merge base-first).

Note: intentionally no Linear ticket for this cleanup stack, so that check stays red.


Note

Medium Risk
Changes kernel teardown ordering and snapshot behavior on dispose/shutdown; mistakes could hang disposal or skip needed state persistence, but scope is bounded to ReplKernelManager graceful cleanup.

Overview
Fixes a race where graceful kernel disposal could close ZMQ sockets while a final namespace snapshot was still running, leaving that work failing against dead sockets.

flushSnapshotForDispose no longer races an uncancellable snapshotState() against a separate dispose guard. On dispose/shutdown-with-snapshot it interrupts any active execution, waits for the executionQueue to settle within SNAPSHOT_EXECUTION_TIMEOUT_MS, then runs captureSnapshot through the normal cancellable execution path (or skips the final snapshot if the queue never settles). SNAPSHOT_DISPOSE_TIMEOUT_MS is removed so snapshot timing uses one policy in one place.

Tests in repl-kernel-abort.test.ts cover dispose waiting until a timed-out snapshot aborts before cleanupResources, and tearing down when a hung execution blocks the queue without starting a snapshot.

Reviewed by Cursor Bugbot for commit 35d4433. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Cancel hung final kernel snapshot before disposal cleanup in ReplKernelManager

Rewrites ReplKernelManager.flushSnapshotForDispose to interrupt any active execution and wait for the executionQueue to settle within SNAPSHOT_EXECUTION_TIMEOUT_MS before taking a final snapshot. If the queue does not settle in time, the snapshot is skipped so teardown proceeds without hanging.

  • Removes the now-unused SNAPSHOT_DISPOSE_TIMEOUT_MS constant from shared.ts
  • Adds tests in repl-kernel-abort.test.ts covering both the interrupted-snapshot path and the permanently-hung execution path
  • Behavioral Change: during dispose(), if an active execution exists it is interrupted immediately, and the final snapshot is only captured when the queue settles within the timeout; previously the snapshot was raced against a separate dispose guard timeout

Macroscope summarized 35d4433.

Linear: ENG-5650

Comment thread packages/coding-agent/src/core/kernel/index.ts Outdated
Comment thread packages/coding-agent/src/core/kernel/index.ts Outdated

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 35d4433. Configure here.

]);
if (timeout) globalThis.clearTimeout(timeout);
if (!queueSettled) return;
await this.captureSnapshot({ executionTimeoutMs: SNAPSHOT_EXECUTION_TIMEOUT_MS });

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dispose snapshot wait can hang

Medium Severity

flushSnapshotForDispose only bounds the wait for the queue tail captured at entry, then awaits captureSnapshot with no outer deadline. A second execute (or other enqueue) that joins during that wait becomes prev for the snapshot and has no timeout, so dispose/shutdown can block indefinitely and delay /reload and signal teardown.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 35d4433. Configure here.

@snimu

snimu commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #1816: GitHub's stack lock prevents retargeting stacked PRs, so this PR was recreated as a plain PR against main (same branch, same content, all review threads here are resolved). Please review/merge #1816 instead.

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