fix(desktop): Codex 重さ調査ログ改善・xterm rAFバッファ・JSONL タイムアウト延長 (#293)#381
fix(desktop): Codex 重さ調査ログ改善・xterm rAFバッファ・JSONL タイムアウト延長 (#293)#381
Conversation
Sentry ログ調査(PR #341/#344)で得られた知見をもとに修正。 - debug-channel: 集計間隔を 1 秒 → 30 秒に延長 毎秒 Sentry にイベントが飛び続けていた問題を解消。 これ自体が CPU/メモリの追加負荷になっていた可能性がある。 - v1-terminal-cache: rAF ベースの write バッファを追加 xterm.write を 1 フレーム(16ms)単位でまとめて呼ぶようにし、 パーサ/レンダリングサイクルの回数を削減。 hidden 経路・マウント時経路の両方に適用。 - terminal-cleanup: killTerminalForPane に v1TerminalCache.dispose を追加 React unmount が遅延した場合でも xterm.js / WebGL リソースを ペイン削除時点で確実に解放する。 - pty-turn-runner: JSONL 発見タイムアウトを 15 秒 → 30 秒に延長 Claude Code の JSONL 生成が SessionStart hook 発火より遅れる場合に 15 秒以内で失敗していた問題を修正。 タイムアウト時のエラーメッセージに実在ファイル一覧を追記。
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
@codex review |
📝 WalkthroughWalkthroughこのPRはターミナルデータ処理を最適化し、エラー診断機能を強化します。リクエストアニメーションフレームベースのバッチ処理によるターミナル書き込み、タイムアウト・集約間隔の延長、ディレクトリ列挙による詳細エラーメッセージ、ターミナル破棄時の初期化ロジックが複数ファイルで実装されます。 Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d9d07460dd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/desktop/src/renderer/stores/tabs/utils/terminal-cleanup.ts (1)
3-3: 追加importはaliasに寄せてください。この深い相対パスは移動に弱いので、既存の
renderer/...aliasで参照する方が安全です。修正例
-import * as v1TerminalCache from "../../../screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/v1-terminal-cache"; +import * as v1TerminalCache from "renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/v1-terminal-cache";As per coding guidelines,
apps/desktop/**/*.{ts,tsx}should "Use aliases as defined intsconfig.jsonwhen possible".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/stores/tabs/utils/terminal-cleanup.ts` at line 3, この行で使っている深い相対パスをプロジェクトの tsconfig エイリアスに合わせて置き換えてください: import * as v1TerminalCache from "../../../screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/v1-terminal-cache"; — 対象モジュール識別子は v1TerminalCache なので、同じモジュールを指すエイリアス版(例: renderer/screens/... などプロジェクトで定義されているパス)に差し替えて、相対パスへの依存をなくしてください。
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/main/todo-daemon/pty-turn-runner.ts`:
- Around line 333-345: The current error builds foundStr by joining foundJsonls
and injects raw filenames into the returned error (see foundJsonls, foundStr and
the returned error object), which can expose other session IDs; change it to
avoid leaking full filenames by replacing the joined filename list with either a
count-only message (e.g. "projectDir に .jsonl が N 件見つかりました") or a masked sample
(e.g. show up to 2 filenames with their middle characters redacted like
"ab****yz.jsonl"); update the construction of foundStr and the error strings to
use the sanitized count/masked sample instead of the unmasked foundJsonls values
so the returned error never contains full session identifiers.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/v1-terminal-cache.ts`:
- Around line 57-59: When HMR restores an older CachedTerminal instance it may
lack the new rAF fields causing rafWriteBuffer to become "undefined..." or
rafWriteId checks to fail; update CachedTerminal initialization (e.g.,
constructor or any restore/revive path) to explicitly backfill rafWriteBuffer =
"" and rafWriteId = null, and ensure scheduleWrite uses these initialized fields
safely (referencing rafWriteBuffer, rafWriteId, scheduleWrite, and
CachedTerminal); apply the same backfill for the other restored instances where
rafWriteBuffer/rafWriteId were added (the corresponding CachedTerminal
restoration code paths).
- Around line 227-242: The delayed write via scheduleWrite can be lost if
lifecycle events (exit/error/disconnect) are handled in the same frame; add a
synchronous flush path that drains rafWriteBuffer to xterm before processing
non-data lifecycle events. Implement a helper (e.g., flushPendingWrite(paneId))
that looks up the cache entry, if rafWriteId exists
cancelAnimationFrame(rafWriteId), if rafWriteBuffer is non-empty call
e.xterm.write(e.rafWriteBuffer) and then clear rafWriteBuffer and set rafWriteId
to null; call this helper from the lifecycle handlers that process
exit/error/disconnect (and the code paths referenced around lines 257-278)
before queuing/removing the pane or logging the lifecycle event.
---
Nitpick comments:
In `@apps/desktop/src/renderer/stores/tabs/utils/terminal-cleanup.ts`:
- Line 3: この行で使っている深い相対パスをプロジェクトの tsconfig エイリアスに合わせて置き換えてください: import * as
v1TerminalCache from
"../../../screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/v1-terminal-cache";
— 対象モジュール識別子は v1TerminalCache なので、同じモジュールを指すエイリアス版(例: renderer/screens/...
などプロジェクトで定義されているパス)に差し替えて、相対パスへの依存をなくしてください。
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c3571002-7ca3-4ad9-bde7-fee177f6ee8a
📒 Files selected for processing (5)
apps/desktop/src/main/todo-daemon/pty-turn-runner.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/hooks/useTerminalStream.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/v1-terminal-cache.tsapps/desktop/src/renderer/stores/tabs/utils/terminal-cleanup.tsapps/desktop/src/shared/debug-channel.ts
- useTerminalStream: exit/disconnect/error イベント処理前に flushWrite を呼び、 trailing output が exitバナーより先に描画されるバグを修正 (Codex レビュー指摘) - v1-terminal-cache: flushWrite をエクスポート - terminal-cleanup: killTerminalForPane から dispose を除去し、 releaseTerminalCache を別関数として追加 - store: paneをstateから削除した直後に releaseTerminalCache を呼ぶよう変更 (React unmount と同バッチになるため、dispose前にコンポーネントが動くリスクを排除)
- HMR復元時に rafWriteBuffer/rafWriteId を ??= で初期化し、 旧モジュールから引き継いだエントリで undefined になるバグを修正 - routeEvent の hidden 経路で非dataイベントをキューに入れる前に flushWrite を呼び、data→exit の順序が保たれるよう修正
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: df0a5fa198
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
概要
Issue #293「Codexが重い」の Sentry ログ調査(PR #341/#344)で得られた知見をもとに修正。
変更一覧
shared/debug-channel.tsv1-terminal-cache.tsscheduleWriteで1フレーム分のデータを束ねて1回のxterm.writeに集約v1-terminal-cache.tsflushWriteをエクスポート。exit/disconnect/error イベント処理前と hidden経路のキュー前に呼び出し、trailing output の消失を防止v1-terminal-cache.tsrafWriteBuffer ??= ""/rafWriteId ??= nullを追加。ホットリロード後のフィールド欠落バグを修正useTerminalStream.tsxterm.writeをscheduleWrite経由に変更(#2 の適用)terminal-cleanup.tskillTerminalForPaneから dispose を除去し、releaseTerminalCacheを独立した関数として追加store.tsdelete newPanes[id]の直後)にreleaseTerminalCacheを呼び、xterm/WebGL リソースを確実に解放pty-turn-runner.tspty-turn-runner.ts根本原因の整理
Sentry 9エージェント並列調査から判明した原因と対応関係:
xterm.writeがスロットリングなしで毎秒30回呼ばれ、描画負荷が高かった残課題
allowTransparency: trueの WebGL 描画コスト(視覚確認後に判断)Closes 一部 #293(根本的な Codex TUI 負荷自体は継続調査)