Skip to content

fix(desktop): stop status-stack remounts from re-arming a dead-runtime poll storm (#98434) - #98455

Closed
chelsealong wants to merge 1 commit into
NousResearch:mainfrom
chelsealong:fix/98434-status-stack-remount-clears-gone-latch
Closed

chelsealong wants to merge 1 commit into
NousResearch:mainfrom
chelsealong:fix/98434-status-stack-remount-clears-gone-latch

Conversation

@chelsealong

Copy link
Copy Markdown
Contributor

Fixes the client-side half of #98434 ("Desktop: boot-restored chat stays bound to dead runtime id — status-stack polls it every 5s forever (composer flicker + focus loss)").

What's wrong

apps/desktop/src/store/composer-status.ts already has a "gone-session" latch: once process.list comes back 4001 session not found for a runtime id, that id is added to goneSessions and every later poll against it short-circuits — this is exactly what stops the storm the issue describes. But ComposerStatusStack's mount effect (apps/desktop/src/app/chat/composer/status-stack/index.tsx) called resetBackgroundPollingGuard(sessionId) on every mount, clearing that latch unconditionally:

useEffect(() => {
  if (sessionId) {
    resetBackgroundPollingGuard(sessionId)      // clears the latch
    void refreshBackgroundProcesses(sessionId)  // → process.list
    void refreshSessionGoal(sessionId)          // → slash.exec 'goal status'
  }
}, [sessionId])

A component mount/remount is not proof of a fresh runtime binding — the issue's own repro shows a boot-restored tile that stays bound to the same dead runtime id across repeated remounts. Each remount re-cleared the latch and re-fired both RPCs against the phantom id, so the storm never actually stopped; it just restarted itself every cycle. This is the exact failure the AI reviewer on the original PR that added this line (#94950) flagged before merge: "The rebind-reset may not fire on runtime re-mint for the same id... consider wiring the reset into whatever store/event tracks runtime-binding changes." That hardening landed separately, at the actual rebind seams — resetBackgroundPollingGuard() (no-arg, full clear) is called from the gateway reconnect path in apps/desktop/src/app/gateway/hooks/use-gateway-boot.ts:378 and the runtime re-mint path in apps/desktop/src/store/gateway.ts:484 — but the redundant per-mount reset in the status stack itself was never removed, so it kept defeating the latch on its own.

Fix

Remove the resetBackgroundPollingGuard(sessionId) call from the mount effect. Real rebinds (gateway reconnect, runtime re-mint) already clear the latch at their own seams; the status stack no longer needs — and must not perform — its own reset on mount.

Test

Added apps/desktop/src/app/chat/composer/status-stack/polling-guard.test.tsx, which renders ComposerStatusStack bound to a session id whose process.list call always rejects with "session not found", unmounts it, and remounts it — asserting process.list is called exactly once total (the latch must survive the remount).

Confirmed it fails without the fix (git checkout HEAD~1 -- apps/desktop/src/app/chat/composer/status-stack/index.tsx, i.e. the pre-fix version with the reset call restored):

AssertionError: expected 2 to be 1 // Object.is equality
- Expected: 1
+ Received: 2

Restored the fix and reran — passes:

 Test Files  6 passed (6)
      Tests  36 passed (36)

(src/app/chat/composer/status-stack + src/store/composer-status.test.ts)

Full suite:

npx eslint src/app/chat/composer/status-stack/index.tsx src/app/chat/composer/status-stack/polling-guard.test.tsx
# clean, 0 errors 0 warnings

npm run typecheck
# clean (tsc -p . / tsconfig.electron.json / tsconfig.e2e.json)

npm run test:ui
# full apps/desktop vitest UI suite — exit code 0

Scope note

This addresses the second of the issue's two "cooperating defects" (the gone-latch being defeated by remount) — the fix that's minimal, mechanical, and provably correct from the code alone. The first defect (boot-restore re-binding the view to the runtime id instead of resolving and resuming the stored id) is a separate, larger client-boot-restore change and isn't attempted here.

Disclosure

This PR was prepared with AI assistance (an autonomous Claude Code session), with the diff, test, and repro reviewed before submission.

…e poll storm (NousResearch#98434)

A boot-restored chat can stay bound to a dead runtime id and remount its
composer status stack repeatedly with no genuine rebind ever occurring.
The stack's mount effect cleared the gone-polling latch on every mount, so
each remount re-armed process.list + slash.exec('goal status') against
the same phantom id forever, churning the composer every ~5s.

Real rebinds already reset the latch at the runtime-mint seams
(use-gateway-boot.ts, store/gateway.ts). Drop the redundant per-mount
reset so the latch actually holds across a remount.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/desktop Electron desktop app (apps/desktop/*) sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Aug 30, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference; please use your judgment.

98455 — Stop clearing background-polling latch on ComposerStatusStack mount

  • apps/desktop/src/app/chat/composer/status-stack/index.tsx:21-24 removes resetBackgroundPollingGuard import and the useEffect that cleared the gone-polling latch on every mount. Comment clarifies: mount/remount is not proof of fresh runtime binding — a boot-restored tile can remount repeatedly while still bound to dead runtime id, re-arming 4001 storm forever. Latch reset now at real rebind seams (use-gateway-boot.ts and store/gateway.ts).
  • New test apps/desktop/.../polling-guard.test.tsx:75-117 reproduces Desktop: boot-restored chat stays bound to dead runtime id — status-stack polls it every 5s forever (composer flicker + focus loss) #98434: renders ComposerStatusStack with mocked $gateway.request that throws session not found for process.list, asserts first mount calls process.list once, unmount/remount still only 1 call (before fix latch cleared → second call). Uses ResizeObserver stub, MemoryRouter, I18nProvider.

Non-blocking:

  • Guard reset removal is safe only if use-gateway-boot.ts/store/gateway.ts indeed reset on gateway reconnect and runtime re-mint — spot-check those call sites. Non-blocking.
  • Test mocks $gateway with vi.fn(async (method) => ...) and checks mock.calls filtering — robust, but ensure beforeEach/afterEach cleanup resets resetBackgroundPollingGuard global state. Non-blocking.

Verdict: Targeted fix for endless 4001 polling with faithful reproduction test.


Non-blocking — please use your judgment.

@OutThisLife

Copy link
Copy Markdown
Contributor

Superseded by #99664.

Same poll-loop class — status-stack remounts re-arming a dead runtime. The latch on main covers it. Closing so we don't double-merge.

@OutThisLife

Copy link
Copy Markdown
Contributor

Superseded by #99891.

The remount latch (resetBackgroundPollingGuard) is the first commit there, with your authorship preserved. The rest of the cluster is stacked on that.

Thanks for isolating the remount clear.

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

Labels

comp/desktop Electron desktop app (apps/desktop/*) P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants