Repository navigation
feat: 优化右侧边栏的宽度切换速度; 左侧边栏展示时定位错误的问题 - #1680
Conversation
ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (8)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThis PR adds pretext-backed intrinsic sizing for chat messages, wires project selection from the desktop sidebar into existing page handlers, and introduces an xterm input fallback that recovers inserted text from textarea events. ChangesMessage List Intrinsic Layout
Sidebar Project Selection Wiring
Xterm Input Fallback
Estimated code review effort: 3 (Moderate) | ~30 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
frontend/src/features/tasks/components/chat/SourceReferences.tsxESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…2-111958 # Conflicts: # wework/src/components/layout/workspace-panels/EmbeddedLocalTerminal.tsx # wework/src/components/layout/workspace-panels/RemoteTerminal.tsx
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
wework/src/components/chat/messagePretextLayout.ts (1)
29-29: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSingle measurement failure permanently disables Pretext for the whole session.
Once
prepare()/layout()throws once,pretextMeasurementAvailableis flipped tofalseand never automatically retried — every subsequent message (regardless of font/text) falls back to the coarse arithmetic estimate for the remainder of the page's life, sinceclearMessagePretextLayoutCache()(the only place resetting the flag) doesn't appear to be called anywhere at runtime in the provided files.Consider retrying per-call (or with backoff) instead of a permanent kill switch, so a transient failure doesn't degrade accuracy indefinitely.
♻️ Possible approach
-let pretextMeasurementAvailable = true +let consecutivePretextFailures = 0 +const MAX_CONSECUTIVE_FAILURES = 5 function measurePretextHeight( text: string, font: string, width: number, lineHeight: number ): number { - if (!pretextMeasurementAvailable) { + if (consecutivePretextFailures >= MAX_CONSECUTIVE_FAILURES) { return estimateFallbackTextHeight(text, width, lineHeight) } try { const prepared = prepare(text, font, { whiteSpace: 'pre-wrap' }) - return layout(prepared, Math.max(MIN_TEXT_WIDTH, width), lineHeight).height + const height = layout(prepared, Math.max(MIN_TEXT_WIDTH, width), lineHeight).height + consecutivePretextFailures = 0 + return height } catch { - pretextMeasurementAvailable = false + consecutivePretextFailures += 1 return estimateFallbackTextHeight(text, width, lineHeight) } }Also applies to: 93-110
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wework/src/components/chat/messagePretextLayout.ts` at line 29, The pretext measurement fallback is being permanently disabled after a single `prepare()`/`layout()` failure, so update `messagePretextLayout.ts` to avoid using `pretextMeasurementAvailable` as a one-way kill switch. In the measurement path that calls `prepare()` and `layout()`, retry per call (optionally with simple backoff) or only suppress measurement for the current failure, and keep future calls attempting measurement again. Also review `clearMessagePretextLayoutCache()` and the `pretextMeasurementAvailable` flag handling so the `measurePretext`/layout flow can recover automatically from transient errors instead of staying on the arithmetic estimate forever.wework/src/components/layout/workspace-panels/xtermInputFallback.test.ts (1)
1-88: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing test coverage for the no-
textareafallback branch.
installXtermInputFallbackreturns no-opnoteData/disposewhenterminal.textareais falsy (source Lines 27-33), but no test exercises this path. Giventerminal: Pick<Terminal, 'textarea'>allowstextareato beundefined, this is a real, reachable branch worth covering.As per coding guidelines, "follow the project's testing principles: AAA, mock external services, test edge cases, and keep tests isolated."
✅ Suggested additional test
+ test('returns no-op controller when textarea is unavailable', () => { + const writeData = vi.fn() + const fallback = installXtermInputFallback({ + terminal: { textarea: null as unknown as HTMLTextAreaElement }, + writeData, + }) + + fallback.noteData('pwd') + fallback.dispose() + + expect(writeData).not.toHaveBeenCalled() + })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wework/src/components/layout/workspace-panels/xtermInputFallback.test.ts` around lines 1 - 88, Add a test in installXtermInputFallback.test.ts that covers the no-textarea branch by passing terminal with textarea set to undefined and verifying the returned fallback exposes no-op noteData and dispose behavior. Use the installXtermInputFallback symbol to locate the branch, keep the test isolated with a vi.fn writeData, and assert that calling noteData and dispose does not throw or invoke writeData.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@wework/src/components/chat/MessageList.tsx`:
- Around line 118-174: The ResizeObserver setup in MessageList never runs again
when the list container mounts after initially being null, so layoutWidth stays
stuck at its default. Update the MessageList ref handling and the
useLayoutEffect so it reacts when the container div actually appears, ideally by
using a state-backed callback ref or equivalent mount-tracking tied to listRef.
Make sure the observer setup/cleanup logic in the effect can re-run when the
element becomes available, preserving the existing debounced resize behavior.
---
Nitpick comments:
In `@wework/src/components/chat/messagePretextLayout.ts`:
- Line 29: The pretext measurement fallback is being permanently disabled after
a single `prepare()`/`layout()` failure, so update `messagePretextLayout.ts` to
avoid using `pretextMeasurementAvailable` as a one-way kill switch. In the
measurement path that calls `prepare()` and `layout()`, retry per call
(optionally with simple backoff) or only suppress measurement for the current
failure, and keep future calls attempting measurement again. Also review
`clearMessagePretextLayoutCache()` and the `pretextMeasurementAvailable` flag
handling so the `measurePretext`/layout flow can recover automatically from
transient errors instead of staying on the arithmetic estimate forever.
In `@wework/src/components/layout/workspace-panels/xtermInputFallback.test.ts`:
- Around line 1-88: Add a test in installXtermInputFallback.test.ts that covers
the no-textarea branch by passing terminal with textarea set to undefined and
verifying the returned fallback exposes no-op noteData and dispose behavior. Use
the installXtermInputFallback symbol to locate the branch, keep the test
isolated with a vi.fn writeData, and assert that calling noteData and dispose
does not throw or invoke writeData.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9a04206e-0825-496b-9a80-82d9108dc62e
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
wework/package.jsonwework/src/components/chat/MessageList.test.tsxwework/src/components/chat/MessageList.tsxwework/src/components/chat/messagePretextLayout.test.tswework/src/components/chat/messagePretextLayout.tswework/src/components/layout/DesktopSidebar.test.tsxwework/src/components/layout/DesktopSidebar.tsxwework/src/components/layout/DesktopWorkbenchLayout.test.tsxwework/src/components/layout/workspace-panels/EmbeddedLocalTerminal.tsxwework/src/components/layout/workspace-panels/RemoteTerminal.tsxwework/src/components/layout/workspace-panels/xtermInputFallback.test.tswework/src/components/layout/workspace-panels/xtermInputFallback.tswework/src/pages/PluginManagementPage.tsxwework/src/pages/PluginsPage.tsx
| const listRef = useRef<HTMLDivElement>(null) | ||
| const isPointerSelectingRef = useRef(false) | ||
| const layoutWidthUpdateTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null) | ||
| const [isTextSelectionActive, setIsTextSelectionActive] = useState(false) | ||
| const [layoutWidth, setLayoutWidth] = useState(0) | ||
| const visibleMessages = useMemo(() => messages.filter(shouldRenderMessage), [messages]) | ||
| const shouldShowWaitingIndicator = | ||
| isWaitingForAssistant && | ||
| !messages.some(message => message.role === 'assistant' && message.status === 'streaming') | ||
| const disableMessageContentVisibility = disableContentVisibility || isTextSelectionActive | ||
| const messageIntrinsicHeights = useMemo(() => { | ||
| return new Map( | ||
| visibleMessages.map(message => [ | ||
| message.id, | ||
| getMessagePretextIntrinsicHeight(message, layoutWidth), | ||
| ]) | ||
| ) | ||
| }, [layoutWidth, visibleMessages]) | ||
| const listLayoutClass = className | ||
| ? 'mx-auto flex min-w-0 flex-col gap-4 overflow-x-hidden pb-2 pt-8' | ||
| : 'mx-auto flex w-full min-w-0 max-w-3xl flex-col gap-4 overflow-x-hidden px-6 pb-2 pt-8' | ||
|
|
||
| useLayoutEffect(() => { | ||
| const element = listRef.current | ||
| if (!element) return | ||
|
|
||
| const updateLayoutWidth = () => { | ||
| setLayoutWidth(currentWidth => { | ||
| const nextWidth = element.clientWidth | ||
| return nextWidth === currentWidth ? currentWidth : nextWidth | ||
| }) | ||
| } | ||
|
|
||
| const scheduleLayoutWidthUpdate = () => { | ||
| if (layoutWidthUpdateTimerRef.current !== null) { | ||
| clearTimeout(layoutWidthUpdateTimerRef.current) | ||
| } | ||
|
|
||
| layoutWidthUpdateTimerRef.current = setTimeout(() => { | ||
| layoutWidthUpdateTimerRef.current = null | ||
| updateLayoutWidth() | ||
| }, MESSAGE_LAYOUT_RESIZE_SETTLE_MS) | ||
| } | ||
|
|
||
| updateLayoutWidth() | ||
| if (typeof ResizeObserver === 'undefined') return | ||
|
|
||
| const resizeObserver = new ResizeObserver(scheduleLayoutWidthUpdate) | ||
| resizeObserver.observe(element) | ||
| return () => { | ||
| resizeObserver.disconnect() | ||
| if (layoutWidthUpdateTimerRef.current !== null) { | ||
| clearTimeout(layoutWidthUpdateTimerRef.current) | ||
| layoutWidthUpdateTimerRef.current = null | ||
| } | ||
| } | ||
| }, []) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
ResizeObserver/width tracking never attaches if the list starts empty.
useLayoutEffect(() => {...}, []) captures listRef.current exactly once, at the first commit. If that first commit renders null (Line 229-231, e.g. a brand-new conversation with no messages yet and no waiting indicator), element is null and the effect's body exits early via if (!element) return. Because the dependency array is empty, this effect never runs again — so once messages arrive and the container div actually mounts, the ResizeObserver is never attached and layoutWidth is stuck at its initial 0, which normalizeLayoutWidth silently maps to DEFAULT_LAYOUT_WIDTH (720px) forever for that component instance. The debounced resize handling this PR introduces (and the sidebar-width-switching-speed goal from the PR title) effectively never activates for any conversation that starts empty.
Switch to a callback ref (state-backed) so the effect can react to the container actually mounting:
🐛 Proposed fix
- const listRef = useRef<HTMLDivElement>(null)
+ const [listElement, setListElement] = useState<HTMLDivElement | null>(null)
const isPointerSelectingRef = useRef(false)
const layoutWidthUpdateTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null)
@@
useLayoutEffect(() => {
- const element = listRef.current
+ const element = listElement
if (!element) return
@@
resizeObserver.observe(element)
return () => {
resizeObserver.disconnect()
if (layoutWidthUpdateTimerRef.current !== null) {
clearTimeout(layoutWidthUpdateTimerRef.current)
layoutWidthUpdateTimerRef.current = null
}
}
- }, [])
+ }, [listElement])
@@
const selection = document.getSelection?.()
- const root = listRef.current
+ const root = listElement
@@
- <div ref={listRef} className={cn(listLayoutClass, className)} onPointerDown={handlePointerDown}>
+ <div ref={setListElement} className={cn(listLayoutClass, className)} onPointerDown={handlePointerDown}>Also applies to: 229-231
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wework/src/components/chat/MessageList.tsx` around lines 118 - 174, The
ResizeObserver setup in MessageList never runs again when the list container
mounts after initially being null, so layoutWidth stays stuck at its default.
Update the MessageList ref handling and the useLayoutEffect so it reacts when
the container div actually appears, ideally by using a state-backed callback ref
or equivalent mount-tracking tied to listRef. Make sure the observer
setup/cleanup logic in the effect can re-run when the element becomes available,
preserving the existing debounced resize behavior.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
wework/src/components/layout/workspace-panels/RemoteTerminal.tsx (1)
107-130: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winResize-sync logic duplicated across the two effects.
syncTerminalSize(Lines 122-130) and the inline resize block in theactiveeffect (Lines 186-194) both implement the same "checklastSizeRef, update it, callclient.resize" logic. Consider extracting a shared helper (e.g., a module-level oruseCallbackfunction takingterminal,client,lastSizeRef) to avoid the two copies drifting out of sync.Also applies to: 168-200
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wework/src/components/layout/workspace-panels/RemoteTerminal.tsx` around lines 107 - 130, The resize-sync logic is duplicated between syncTerminalSize and the inline block in the active effect, so extract the repeated “check lastSizeRef, update it, call client.resize” flow into a shared helper or useCallback that both fitAndResize and the active effect can call. Keep the helper centered around the existing RemoteTerminal symbols (syncTerminalSize, fitAndResize, lastSizeRef, client, terminal) so there is one source of truth and the two effects cannot drift apart.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@wework/src/components/layout/workspace-panels/RemoteTerminal.tsx`:
- Around line 73-78: The pending terminal theme update scheduled by
createTerminalThemeScheduler can still fire after RemoteTerminal cleanup,
causing applyTerminalTheme to run on a disposed terminal. Update the scheduler
in xterm-theme to keep track of the requestAnimationFrame id and expose a
cancel/disposer, then call it from RemoteTerminal’s cleanup alongside
terminal.dispose() and unsubscribeOutput so no queued theme frame runs after
unmount.
---
Nitpick comments:
In `@wework/src/components/layout/workspace-panels/RemoteTerminal.tsx`:
- Around line 107-130: The resize-sync logic is duplicated between
syncTerminalSize and the inline block in the active effect, so extract the
repeated “check lastSizeRef, update it, call client.resize” flow into a shared
helper or useCallback that both fitAndResize and the active effect can call.
Keep the helper centered around the existing RemoteTerminal symbols
(syncTerminalSize, fitAndResize, lastSizeRef, client, terminal) so there is one
source of truth and the two effects cannot drift apart.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1b7a2663-0811-41f0-8ecc-06afe703df40
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
chat_shell/tests/test_knowledge_listing.pyfrontend/src/features/tasks/components/chat/SourceReferences.tsxwework/package.jsonwework/src/components/chat/MessageList.test.tsxwework/src/components/chat/MessageList.tsxwework/src/components/layout/DesktopWorkbenchLayout.test.tsxwework/src/components/layout/workspace-panels/EmbeddedLocalTerminal.tsxwework/src/components/layout/workspace-panels/RemoteTerminal.tsx
✅ Files skipped from review due to trivial changes (1)
- frontend/src/features/tasks/components/chat/SourceReferences.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- wework/package.json
- wework/src/components/layout/DesktopWorkbenchLayout.test.tsx
- wework/src/components/layout/workspace-panels/EmbeddedLocalTerminal.tsx
- wework/src/components/chat/MessageList.tsx
- wework/src/components/chat/MessageList.test.tsx
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🧹 Nitpick comments (1)
wework/src/components/layout/workspace-panels/RemoteTerminal.tsx (1)
107-130: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winResize-sync logic duplicated across the two effects.
syncTerminalSize(Lines 122-130) and the inline resize block in theactiveeffect (Lines 186-194) both implement the same "checklastSizeRef, update it, callclient.resize" logic. Consider extracting a shared helper (e.g., a module-level oruseCallbackfunction takingterminal,client,lastSizeRef) to avoid the two copies drifting out of sync.Also applies to: 168-200
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wework/src/components/layout/workspace-panels/RemoteTerminal.tsx` around lines 107 - 130, The resize-sync logic is duplicated between syncTerminalSize and the inline block in the active effect, so extract the repeated “check lastSizeRef, update it, call client.resize” flow into a shared helper or useCallback that both fitAndResize and the active effect can call. Keep the helper centered around the existing RemoteTerminal symbols (syncTerminalSize, fitAndResize, lastSizeRef, client, terminal) so there is one source of truth and the two effects cannot drift apart.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@wework/src/components/layout/workspace-panels/RemoteTerminal.tsx`:
- Around line 73-78: The pending terminal theme update scheduled by
createTerminalThemeScheduler can still fire after RemoteTerminal cleanup,
causing applyTerminalTheme to run on a disposed terminal. Update the scheduler
in xterm-theme to keep track of the requestAnimationFrame id and expose a
cancel/disposer, then call it from RemoteTerminal’s cleanup alongside
terminal.dispose() and unsubscribeOutput so no queued theme frame runs after
unmount.
---
Nitpick comments:
In `@wework/src/components/layout/workspace-panels/RemoteTerminal.tsx`:
- Around line 107-130: The resize-sync logic is duplicated between
syncTerminalSize and the inline block in the active effect, so extract the
repeated “check lastSizeRef, update it, call client.resize” flow into a shared
helper or useCallback that both fitAndResize and the active effect can call.
Keep the helper centered around the existing RemoteTerminal symbols
(syncTerminalSize, fitAndResize, lastSizeRef, client, terminal) so there is one
source of truth and the two effects cannot drift apart.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1b7a2663-0811-41f0-8ecc-06afe703df40
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
chat_shell/tests/test_knowledge_listing.pyfrontend/src/features/tasks/components/chat/SourceReferences.tsxwework/package.jsonwework/src/components/chat/MessageList.test.tsxwework/src/components/chat/MessageList.tsxwework/src/components/layout/DesktopWorkbenchLayout.test.tsxwework/src/components/layout/workspace-panels/EmbeddedLocalTerminal.tsxwework/src/components/layout/workspace-panels/RemoteTerminal.tsx
✅ Files skipped from review due to trivial changes (1)
- frontend/src/features/tasks/components/chat/SourceReferences.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- wework/package.json
- wework/src/components/layout/DesktopWorkbenchLayout.test.tsx
- wework/src/components/layout/workspace-panels/EmbeddedLocalTerminal.tsx
- wework/src/components/chat/MessageList.tsx
- wework/src/components/chat/MessageList.test.tsx
🛑 Comments failed to post (1)
wework/src/components/layout/workspace-panels/RemoteTerminal.tsx (1)
73-78: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
xterm.js Terminal dispose() calling options.theme setter after dispose behavior💡 Result:
In xterm.js, the Terminal.dispose method is designed as a terminal or "trapdoor" function, meaning that once it is called, the terminal object is considered destroyed and should not be used again [1][2][3]. When dispose is executed, the terminal internal state is marked as disposed (e.g., _isDisposed = true), and critical internal components—such as event listeners, the DOM renderer, and various service handlers—are detached or destroyed [1][4][3]. Attempting to interact with the terminal instance after this point, including calling setters like options.theme, will not produce the expected results and will likely fail silently or cause runtime errors because the necessary underlying services (such as the renderer or color manager) have been cleaned up [1][5][2]. If you are experiencing issues where you need to update a theme or other options, you should ensure that these configuration changes are performed before dispose is called, or if the terminal needs to be "reset," it is recommended to completely destroy the instance and create a new one, rather than attempting to reuse a disposed instance [1][2]. Regarding the behavior of the theme setter specifically, it is worth noting that even on an active (undisposed) terminal, setting options.theme requires the theme object to change identity (a new object reference) for the update to take effect [6]. The setter does not perform a deep comparison of the existing theme properties [6]. However, this behavior is independent of the disposal process; once dispose has been called, the terminal instance is effectively defunct [1][2].
Citations:
- 1: xtermjs/xterm.js#3939
- 2: xtermjs/xterm.js#4739
- 3: robert-harbison/xterm-for-react#13
- 4: xtermjs/xterm.js#5181
- 5: https://github.com/xtermjs/xterm.js/blob/d3fd3d0f2254a966238e575f3505d5cac31cff91/src/browser/Terminal.ts
- 6: xtermjs/xterm.js#4124
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== RemoteTerminal.tsx ==" wc -l wework/src/components/layout/workspace-panels/RemoteTerminal.tsx sed -n '1,260p' wework/src/components/layout/workspace-panels/RemoteTerminal.tsx | cat -n echo echo "== xterm-theme.ts ==" wc -l wework/src/lib/xterm-theme.ts sed -n '1,220p' wework/src/lib/xterm-theme.ts | cat -n echo echo "== References to createTerminalThemeScheduler ==" rg -n "createTerminalThemeScheduler|scheduleThemeSync|applyTerminalTheme|observeTerminalTheme|unobserveTheme" wework/src -SRepository: wecode-ai/Wegent
Length of output: 15458
Cancel the pending theme frame before disposing the terminal.
createTerminalThemeScheduler()queuesapplyTerminalTheme()withrequestAnimationFramebut exposes no cancel path, so an output event right before unmount can still run afterterminal.dispose(). Track the frame id and cancel it in cleanup, or return a disposer fromwework/src/lib/xterm-theme.ts:93-107.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wework/src/components/layout/workspace-panels/RemoteTerminal.tsx` around lines 73 - 78, The pending terminal theme update scheduled by createTerminalThemeScheduler can still fire after RemoteTerminal cleanup, causing applyTerminalTheme to run on a disposed terminal. Update the scheduler in xterm-theme to keep track of the requestAnimationFrame id and expose a cancel/disposer, then call it from RemoteTerminal’s cleanup alongside terminal.dispose() and unsubscribeOutput so no queued theme frame runs after unmount.
Summary by CodeRabbit
New Features
Bug Fixes