Skip to content

fix: recover stale chat sends and drain one-shot CLI exits - #354

Merged
devSejung merged 2 commits into
mainfrom
fix/chat-history-send-sync
Oct 22, 2026
Merged

devSejung merged 2 commits into
mainfrom
fix/chat-history-send-sync

Conversation

@devSejung

@devSejung devSejung commented Oct 22, 2026 •

Copy link
Copy Markdown
Owner

What Problem This Solves

Fixes false “The thread switched branches — review and resend” failures when Control UI sends a follow-up after the preceding history refresh failed. Related: #210, which covered history still in flight and same-transcript ancestor admission.

Also fixes the CLI exit lifecycle exposed by the first CI run: the dotenv/import-error child completed output callbacks and entered process.reallyExit, then remained alive until its 240-second deadlock guard.

Why This Change Was Made

The pane's history-request lifecycle now records an owned failed full-history read. Foreground send recovers through the existing history loader and checks connection, selected session/agent, and transcript identity after awaiting it. Failed recovery preserves the draft and attachments. Contextual actions retain their exact-leaf constraint. Gateway production admission is unchanged; copied stale leaves and empty pre-switch cursors remain rejected.

CLI one-shot finalization previously treated empty stream-write callbacks as permission to force native shutdown. Those callbacks do not establish that background native work has drained. The finalizer now records the exit code, restores terminal state, and lets Node drain naturally. It removes the synthetic stream-write/pending-stream path and preserves the existing five-second fallback for retained handles, using the latest exit code. Commands retaining handles may therefore take up to five seconds to terminate. Long-running commands without a one-shot completion request keep their existing lifecycle.

Production delta: +52/-25, net +27 lines; tests: net +212 lines. The positive production delta belongs to recording failed history reconciliation at its owner and fencing submissions across recovery; the CLI repair removes 13 net production lines. No new protocol, configuration, schema, dependency, V8 runtime flag, test retry, or increased timeout.

User Impact

A follow-up refreshes history before send instead of failing with a false branch-switch error. If history remains unavailable, the user sees the error and keeps the composed message and attachments. Navigation, reconnection, agent selection, or transcript replacement during recovery cannot send the draft into a replacement context.

Completed one-shot commands preserve output, terminal restoration, and exit codes while allowing pending native work to finish before shutdown.

Evidence

AI-assisted. Validated in local Docker Desktop Linux; independent autoreview found no actionable P0–P2 findings. Rebased onto main at 5109dcad941e461593b6a64b5588bcab029b66de.

Final hosted verification on aadc3dd8d130d4780eea1bd7e268ab3f5ca6eb83: 65 checks passed, 11 skipped, zero failures or pending checks. OpenClaw CI run passed, including the previously failing checks-node-compact-small-6, all four browser E2E shards, lint, production/test types, and Docker smoke (separate workflow).

  • Chat: two regression cases fail before repair; full history/send suites pass 333 tests, with 12 affected cases rechecked after the final chat edits. Real Chromium against a production UI bundle/mock Gateway fails before (expectedLeafEntryId: null) and passes after (first-response). Focused Gateway protection cases: 3 passed.
  • Native CLI proof on the exact failing CI runtime, Node 24.22.0: the original finalizer hangs in a deterministic fault-injection fixture. GDB shows the main thread in WorkerThreadsTaskRunner::Shutdown / DefaultProcessExitHandlerInternal, with a V8 worker waiting in CollectionBarrier::AwaitCollectionBackground / StressConcurrentAllocationTask. The same fixture after repair finishes background work and exits with code 7 without invoking forced exit. The regression's stress flag is fault injection only.
  • Dependency contract checked directly: Node natural-exit documentation, src/api/environment.cc at v24.22.0, and src/node_platform.cc. Relevant upstream native defect: nodejs/node#64274, pending fix #66171.
  • CLI regression coverage includes native-work draining, 1 MiB piped output, dual-TTY JSON/terminal routing, fatal exit codes, system-CA gates, long-running commands, and unchanged fallback timing. Real children run in the existing isolated CLI-process project.
  • Final Linux owner checks on Node 24.22.0: 37 CLI-process tests passed (26 help, 4 hooks, 7 one-shot), including the original failing dotenv case in 1.65 seconds; 215 CLI tests passed (plugin installation, dry-run state, one-shot gates, MCP probe exits). Runtime/entry siblings: 15 passed. Test-project routing: 262 passed. The routing environment was a real Git source snapshot with consistent Git trust across isolated test homes.
  • Core production/test type checks, UI production/test type checks, targeted lint and formatting passed. UI production build, compressed sidecars, and performance gates passed.
  • Max-lines ratchet passed in Linux; final platformclaw-check --changed --base origin/main --head HEAD --overlay-only passed without quick mode. Shared checks run in hosted OpenClaw CI.
  • A packaged-plugin test fixture now uses an isolated temporary root instead of colliding with a real Docker /app source checkout; existing npm-selection and persistence assertions are unchanged.

Company VM deployment/live reproduction and native macOS execution were unavailable. Linux/browser evidence does not claim either. The retained-handle fallback still uses Node's native forced-exit path; this repair does not claim to solve every upstream Node shutdown defect.

Named follow-up: Audit reset lifecycle fencing for non-null ancestor admission (src/gateway/session-reset-service.ts:1434, src/config/sessions/session-accessor.sqlite-active-events.ts:208). Reset preserves transcript identity; this PR preserves the existing Gateway contract.

@devSejung
devSejung marked this pull request as ready for review October 22, 2026 04:51
@devSejung
devSejung force-pushed the fix/chat-history-send-sync branch from 1576adc to aadc3dd Compare October 22, 2026 06:47
@devSejung devSejung changed the title fix: follow-up chat fails after history refresh errors fix: recover stale chat sends and drain one-shot CLI exits Oct 22, 2026
@devSejung
devSejung merged commit c77f037 into main Oct 22, 2026
78 checks passed
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