Repository navigation
Conversation
Captures the planned approach for keeping the terminal pane's red error toast from appending duplicate lines indefinitely when the WebSocket connection drops. The spec covers four follow-up implementation tasks: bounded event table for the toast, single-clear on PTY connect, one-shot error on remote-runtime close, and regression coverage. Skipped intentionally: retry-cap (exhausted) state, per user direction. Moved from docs/superpowers/plans/ to docs/reference/ to comply with this repo's durable-docs allow-list.
…out effect on reset ref
…anner Apply the four action items surfaced by /review on the WS-disconnect toast stabilization work. 1. Rename TerminalErrorToast -> TerminalErrorBanner. STYLEGUIDE.md reserves 'toast' for messages that disappear; the component persists indefinitely and carries a 'Restart daemon' action, so it is functionally an inline banner, not a transient toast. Updated JSX site, import, and the five locale files. 2. Lift useTerminalErrorTable from pane-local React state into useAppStore, keyed by worktreeId via a new TerminalErrorsSlice. With the previous pane-local design, split panes subscribing to the same runtime surfaced N independent red toasts. The store-backed dedup domain collapses them to one logical banner per worktree. Slice registered in store/index.ts and wired into AppState. 3. Add an 'if (disposed) return' guard inside resetError() in pty-connection.ts. StrictMode dev builds let the first mount's spawn resolve after the binding is torn down; the guard prevents setState on a dead fiber (verified by Claude adversarial + Red-team RT-02). 4. Add @testing-library/react render tests for TerminalErrorBanner: count badge visibility, SSH vs non-SSH palette branches, daemon-restart button conditional, file-an-issue link, and multi-entry rendering. Total 11 tests in the file (3 pre-existing for shouldOfferDaemonRestart, 8 new). Test factories (store-test-helpers.ts, diffComments.test.ts) extended to include the new slice fields. Also: drop the now-unused 'clearTerminalError' dep from handleRestartCodexPane's useCallback deps array (the lift removed the only call site inside that callback's body). Add curly braces to the dispose guard per oxlint.
… memo Three follow-ups to the WS-disconnect toast stabilization review. A. Hoist <TerminalErrorBanner> out of TerminalPane and render once per workspace via a new TerminalErrorBannerOverlayLayer mounted alongside the other per-worktree overlays in Terminal.tsx. With useTerminalErrorTable already lifted to useAppStore keyed by worktreeId, the banner is shared state; rendering per-pane produced N visually redundant banners on split panes. The pane still consumes the hook for push/clear (dispatch handlers stay at the pane level), but the rendering surface is now a single sibling-of-pane overlay. B. Wrap TerminalErrorBanner in React.memo. Defensive against WS flap pushing a new errors array reference every ~100 ms during reconnect storms; the component re-renders only when the errors reference itself changes (the store guarantees referential equality for unchanged slices). C. Sync docs/reference/2026-06-30-terminal-ws-disconnect-toast- stabilization.md: remove the row that still referenced isTerminalZeroDimensionsDiagnostic (deleted in feeafb0) and add a brief note explaining why the visibility-driven zero- dimensions clear was removed in favor of onResetErrorRef.
Apply the auto-fix items + 2 ASK items from /review round 2. Slice hardening (terminal-errors.ts): - Clamp now in pushTerminalError: reject NaN/Infinity/negative, fallback to Date.now() so a buggy or hostile caller can't bypass the 30s dedup window by injecting a future or negative timestamp. - Reject empty-string worktreeId in both push and clear, so a partially- initialized store or a malformed IPC payload cannot pollute the empty-string namespace. - clearTerminalErrors early-returns on absent key (no map churn when dismissing for a worktree that never had errors). Hook split (use-terminal-error-table.ts): - useTerminalErrorActions(worktreeId, options?) returns push/clear only, no slice subscription. TerminalPane consumes this — it dispatches errors but doesn't render them, so subscribing would re-render every pane in the worktree on each push. - useTerminalErrorTable(worktreeId) returns TerminalErrorEntry[] via a shallow selector. TerminalErrorBannerOverlayLayer consumes this and the selector is intentionally shallow — referential equality is what makes React.memo on TerminalErrorBanner short-circuit on pushes for other worktrees. Overlay layer (TerminalErrorBannerOverlayLayer.tsx): - Stable onDismiss via useCallback, so React.memo on TerminalErrorBanner is no longer defeated by an inline arrow allocation per render. - Drop the redundant 'as TerminalErrorEntry[]' cast — the !errors guard already narrows the type. Pane wiring (TerminalPane.tsx): - Re-track onResetErrorRef.current via useLayoutEffect so the ref stays current when clearTerminalError identity changes (worktreeId switch). The disposed-guard inside resetError() in pty-connection.ts covers the StrictMode race; this addresses the cross-render stale-closure window. Design tokens (main.css + TerminalErrorBanner.tsx): - Replace inline rgba/hex literals with documented CSS-variable tokens per STYLEGUIDE.md and AGENTS.md 'don't invent new color values'. - Add --destructive-soft, --destructive-border, --destructive-fg and --color-warning-soft, --color-warning-border, --color-warning-fg in both :root and .dark via color-mix from existing --destructive. Stale references cleanup: - locale-ko-key-overrides.json: rename TerminalErrorToast.* i18n keys to TerminalErrorBanner.* so the override file actually matches the generated keys. - docs/reference/2026-06-30-...md: rename 15 stale TerminalErrorToast references to TerminalErrorBanner (file paths, JSX names, prop types, i18n keys in code blocks). Verified: vitest 2852/0/0, tsc clean.
Round-2 /review surfaced four CRITICAL test gaps. Address all four. 1. New TerminalErrorBannerOverlayLayer.test.tsx (5 tests): - renders null when no errors for the worktree - dismisses via the × button - isolates banners across worktrees - does not clear unrelated worktrees on dismiss - unrelated-worktree pushes don't render into the active overlay 2. New terminal-errors.test.ts (10 tests): - append, Date.now() fallback, dedup window, eviction - 5-row cap (drops oldest when full and new entry arrives) - defensive guards: negative now, NaN/Infinity, empty worktreeId - clearTerminalErrors preserves unrelated worktrees - clear on absent key is a no-op (no state churn) 3. Extend TerminalErrorBanner.test.tsx with React.memo coverage: - skipped re-render when errors array ref is stable - re-render when errors array ref changes (count incremented) 4. Rename TerminalPane.test.tsx -> use-terminal-error-table.test.tsx: the file name was misleading — it only tests the useTerminalErrorActions hook, never the TerminalPane component. Header comment now flags this scope explicitly. The renamed file's tests still pass via useAppStore.getState() reads since useTerminalErrorActions no longer returns 'errors'. Two new tests (test.tsx file) make the renamed hook signature explicit. Verified: vitest 2869/0/0 (17 new tests), tsc clean.
…#6893 4 real findings + 3 doc drifts from CodeRabbit's review of PR stablyai#6893 (stablyai#6893). Two additional CodeRabbit items (slice dedup window removal) were misreads; they will be addressed by reply-only comments since the window is the actual bug-fix mechanism. A. main.css — light-theme contrast: --destructive-fg (#7f1d1d) and --color-warning-fg (#78350f) in :root give WCAG-AA contrast against the soft backgrounds. Dark-mode pale-on-dark variants preserved. B. TerminalErrorBanner.tsx — SSH mode (L42): errors.some(isSshMessage) flipped the entire banner into SSH style when ANY row was SSH, suppressing recovery actions for non-SSH rows in mixed tables. Changed to errors.length > 0 && errors.every(...). C. TerminalErrorBanner.tsx — Restart daemon button (L118): hard-coded '#fecaca' replaced with var(--destructive-fg) so it switches with the light-mode fix above and aligns with the rest of the token migration done in commit 09eae6b. D. remote-runtime-pty-transport.ts — onTransportClose idempotency: clear connected/handle/remotePtyId after firing onDisconnect+onError (not before, to preserve the first-close behavior the existing tests pin at remote-runtime-pty-transport.test.ts:1188-1192). A second close short-circuits via the existing 'if (destroyed || !connected || !handle) return' guard. E. docs/reference/2026-06-30-...md — three doc drifts: React 18 → 19; renderHook import path corrected to @testing-library/react + unused createRef/RefObject removed; raw rgba/hex in banner snippet replaced with token refs to match the implemented CSS tokens. Tests: vitest 2881/0/0 (terminal-pane + store); tsc clean.
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.
Summary
Stabilize the terminal pane's red error toast when the WebSocket connection drops. Before this change the toast accumulated lines indefinitely (one per reconnect attempt). After this change the toast is bounded, deduplicated, and cleared on a successful PTY reconnect.
Bug fixed: WS断开 → 终端页面超时,红色字体不断刷新.
Architecture
TerminalErrorsSlicekeyed byworktreeId. 30s dedup window, 5-row cap. Identical messages within the window increment a counter instead of appending lines.onResetErrorRefcallback fired insideconnectPanePtyat 4 success paths (fresh spawn, sync attach, reattach, StrictMode reuse). Replaces the previous ad-hoc visibility-driven and codex-restart clears. Errors only clear when something actually succeeded.onTransportCloseno longer triggers an async resubscribe loop. WS-driven multiplex close surfaces a single 'Remote Orca runtime connection lost — waiting for runtime to come back.' message via the dedup table.TerminalErrorBanner(renamed fromTerminalErrorToastper STYLEGUIDE.md — it persists indefinitely, not transient) renders once per workspace viaTerminalErrorBannerOverlayLayer, eliminating per-pane duplication.--destructive-soft/-border/-fg,--color-warning-soft/-border/-fg).Files changed
src/renderer/src/store/slices/terminal-errors.ts(new)src/renderer/src/components/terminal-pane/use-terminal-error-table.ts— split intouseTerminalErrorActions+useTerminalErrorTablesrc/renderer/src/components/terminal-pane/TerminalErrorBanner.tsx(renamed)src/renderer/src/components/terminal-pane/TerminalErrorBannerOverlayLayer.tsx(new)src/renderer/src/components/Terminal.tsx— registers the new overlay layersrc/renderer/src/components/terminal-pane/TerminalPane.tsxsrc/renderer/src/components/terminal-pane/pty-connection.ts—resetErroraddsif (disposed) returnguardsrc/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts— removed resubscribe loopsrc/renderer/src/assets/main.css— new destructive + warning token familieslocale-ko-key-overrides.json+docs/reference/...mdTest Coverage
2869 tests in terminal-pane + store suites, 0 failures. New tests:
terminal-errors.test.ts(10): slice unit testsTerminalErrorBannerOverlayLayer.test.tsx(5): render-null, dismiss, isolationTerminalErrorBanner.test.tsx(8 + 2 new): React.memo skip/re-renderuse-terminal-error-table.test.tsx(renamed)pty-connection.test.ts:onResetErrorReffiringPre-Landing Review
Two rounds ran. Final outcome: APPROVED — ready to merge. PR Quality Score: 6.5/10.
Verification
pnpm exec vitest runon terminal-pane + store → 2881/0/0pnpm exec tsc→ No errors found🤖 Generated with Claude Code