Conversation
* simplify workspace controls * fix current workspace remove tooltip
…ompt (superset-sh#3662) * refactor(desktop): extract ToolStatusBadge from AskUserQuestionToolCall Pull the inline QuestionStatusDescription component and its QUESTION_STATUS_CONFIG lookup into a shared ToolStatusBadge with a variant prop (default / success / danger) so other tool call rows can reuse consistent badge styling without duplicating the pattern. * fix(chat,host-service): prevent stale ask_user question from shadowing sandbox access prompt The harness sets displayState.pendingQuestion when ask_user fires but only clears it on agent_end — never when the user answers. If an agent asks a question and then immediately calls request_access, the already- answered question stays in displayState.pendingQuestion and shadows the new sandbox prompt. The agent then waits for the sandbox prompt to be answered, but the UI never shows it, causing a deadlock. Fix: track answeredQuestionIds per session. In getDisplayState, filter out any harness pendingQuestion whose ID has already been answered this turn so the sandbox prompt can surface correctly. Clear the set on agent_start/agent_end. Also threads the sandbox request reason through as a top-level description on the pending question object so UIs can display it as context beneath the question header. * feat(desktop): custom UI for request_access tool call Replace the generic INPUT/OUTPUT block for request_access with a dedicated RequestSandboxAccessToolCall component that matches the question tool's visual language: - FolderLock icon with "Request Access" title - Status badge in the hint slot: AWAITING RESPONSE / ACCESS GRANTED / ACCESS DENIED / CANCELLED / ERROR (red only on error) - Not expandable/clickable while awaiting a response - After resolution: path and reason as muted context lines, then the access decision as the primary answer line Also surfaces the request reason as context below the question text in QuestionInputOverlay, using a new optional description field on the pending question shape. Alias request_sandbox_access → request_access so either tool name routes to the same component. * fix(desktop): handle empty path/reason and plain-string result in request_access UI - Use || for hasContext so empty string args.path doesn't hide reason - Fall back to result.text when result.content is absent, since getResult wraps non-JSON string outputs as { text } - Update normalizeToolName test to match the request_sandbox_access → request_access alias * fix(chat): preserve question prompt on response failure * fix(chat): rollback question state on response failure * lint * fix(chat): dedupe concurrent question responses --------- Co-authored-by: Kiet Ho <hoakiet98@gmail.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 48 minutes and 46 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (23)
📝 Walkthrough概要このプルリクエストは、GitHub PR/Issue検索にクローズ状態フィルタを追加し、ターミナルセッション管理を複数インスタンスに対応させ、チャット質問応答フローを最適化し、マークダウンエディタにTableKit拡張を統合します。また、ペインライフサイクル管理を強化し、ファイルオープンアクション（外部エディタ開く）を分離し、複数の新規UIコンポーネント（ToolStatusBadge、ToolCallRow、TerminalSessionDropdown等）を導入します。 変更内容
推定コードレビュー工数🎯 4 (複雑) | ⏱️ ~60 分 関連する可能性のあるプルリクエスト
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b71fbbb97f
ℹ️ 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".
|
|
||
| function cleanupWorkspacePaneRuntimes(rows: PaneLifecycleRow[]): void { | ||
| for (const terminalId of extractPaneIds(rows, getTerminalRuntimeId)) { | ||
| terminalRuntimeRegistry.release(terminalId); |
There was a problem hiding this comment.
cleanupWorkspacePaneRuntimes now calls terminalRuntimeRegistry.release, which explicitly only drops renderer state and leaves the PTY alive. Because this commit also changed lifecycle detection to skip removals when the owner workspace row disappears (getRemovedPaneLocations), removeWorkspaceFromSidebar/removeProjectFromSidebar can delete rows without ever hitting the old global dispose path, leaving active terminal processes orphaned in host-service after a workspace is removed from the sidebar.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (17)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceInitializingView/WorkspaceInitializingView.tsx (1)
331-331: 初期化中状態のworkspaceName表示にもクランプ／tooltip 適用を検討してください。「Setup incomplete」「Workspace setup failed」状態ではクランプと
titleを追加していますが、通常の初期化中状態（Line 331）の<p className="text-sm text-muted-foreground">{workspaceName}</p>は変更されていません。長いワークスペース名の場合、初期化進行中もレイアウトが崩れる可能性があり、3 箇所で挙動が一貫しません。意図的でなければ同等のクラスとtitle属性の付与を検討してください。♻️ 提案する修正
- <p className="text-sm text-muted-foreground">{workspaceName}</p> + <p + className="line-clamp-3 max-w-full break-words text-sm text-muted-foreground [overflow-wrap:anywhere]" + title={workspaceName} + > + {workspaceName} + </p>クラス文字列が 3 箇所で重複しているため、ローカル定数に抽出するのも一案です:
+const WORKSPACE_NAME_CLAMP_CLASS = + "line-clamp-3 max-w-full break-words text-sm text-muted-foreground [overflow-wrap:anywhere]";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceInitializingView/WorkspaceInitializingView.tsx` at line 331, The initializing view currently renders workspaceName with <p className="text-sm text-muted-foreground">{workspaceName}</p> (in WorkspaceInitializingView) but does not apply the same truncation/clamp and title used in the "Setup incomplete"/"Workspace setup failed" cases; update that <p> to use the same clamp/truncate utility classes and add a title attribute set to workspaceName so long names are truncated consistently, and to avoid duplication extract the repeated class string into a local constant (e.g., clampClass) used by all three places in WorkspaceInitializingView.apps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/AskUserQuestionToolCall/AskUserQuestionToolCall.tsx (1)
235-259: 条件分岐の重複を解消できます（任意）235-246行目と248-259行目で同一のJSXを返却しています。条件を統合することでコードの重複を削減できます。
♻️ リファクタリング案
- if (hasOutput || !onAnswer || optimisticAnswers) { - return ( - <div className="space-y-2"> - {messageBlock} - <SupersetToolCall - part={part} - toolName="Question" - icon={MessageCircleQuestionIcon} - /> - </div> - ); - } - - if (!canRespond) { + if (hasOutput || !onAnswer || optimisticAnswers || !canRespond) { return ( <div className="space-y-2"> {messageBlock} <SupersetToolCall part={part} toolName="Question" icon={MessageCircleQuestionIcon} /> </div> ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/AskUserQuestionToolCall/AskUserQuestionToolCall.tsx` around lines 235 - 259, The two early-return branches return identical JSX and can be merged: combine the conditions (hasOutput || !onAnswer || optimisticAnswers) and (!canRespond) into a single condition so the component returns the same block once when any of those are true; update the if to check (hasOutput || !onAnswer || optimisticAnswers || !canRespond) and keep the returned JSX (messageBlock plus the SupersetToolCall with part and MessageCircleQuestionIcon) unchanged to remove duplication.apps/desktop/src/renderer/components/Chat/ChatInterface/components/IssueLinkCommand/IssueLinkCommand.tsx (1)
90-130: フィルタ計算の重複を一本化できます。
showClosedに応じた可視タスク計算 (lines 93-99 と 114-118) が完全に同一ロジックで二重化しています。visibleTasksをuseMemoで先に計算し、Fuseの入力と非検索時の slice 元の双方で再利用すると DRY になり、フィルタ条件を変更する際の整合性も保ちやすくなります。♻️ 提案 diff
+ const visibleTasks = useMemo(() => { + if (!allTasks?.length) return []; + if (showClosed) return allTasks; + return allTasks.filter((task) => { + const status = task.statusId ? statusMap.get(task.statusId) : undefined; + return !isClosedStatus(status?.type); + }); + }, [allTasks, showClosed, statusMap]); + const taskFuse = useMemo( () => - new Fuse( - (allTasks ?? []).filter((task) => { - if (showClosed) return true; - const status = task.statusId - ? statusMap.get(task.statusId) - : undefined; - return !isClosedStatus(status?.type); - }), - { - keys: [ - { name: "slug", weight: 3 }, - { name: "title", weight: 2 }, - ], - threshold: 0.4, - ignoreLocation: true, - }, - ), - [allTasks, showClosed, statusMap], + new Fuse(visibleTasks, { + keys: [ + { name: "slug", weight: 3 }, + { name: "title", weight: 2 }, + ], + threshold: 0.4, + ignoreLocation: true, + }), + [visibleTasks], ); const filteredTasks = useMemo(() => { - if (!allTasks?.length) return []; - const visibleTasks = allTasks.filter((task) => { - if (showClosed) return true; - const status = task.statusId ? statusMap.get(task.statusId) : undefined; - return !isClosedStatus(status?.type); - }); if (!searchQuery) { return visibleTasks .sort( (a, b) => new Date(b.updatedAt).getTime() - new Date(a.updatedAt).getTime(), ) .slice(0, MAX_RESULTS); } return taskFuse .search(searchQuery, { limit: MAX_RESULTS }) .map((r) => r.item); - }, [allTasks, searchQuery, showClosed, statusMap, taskFuse]); + }, [visibleTasks, searchQuery, taskFuse]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/Chat/ChatInterface/components/IssueLinkCommand/IssueLinkCommand.tsx` around lines 90 - 130, Compute the "visibleTasks" once using useMemo (based on allTasks, showClosed, statusMap and isClosedStatus) and then feed that memoized visibleTasks into the Fuse constructor (taskFuse) and reuse it in filteredTasks for the non-search path (sorting + slice(MAX_RESULTS)), removing the duplicated filter logic; update taskFuse to new Fuse(visibleTasks, ...) and change filteredTasks to read from visibleTasks instead of re-filtering allTasks.apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/components/GitHubIssueLinkCommand/GitHubIssueLinkCommand.tsx (1)
155-170:debouncedTrimmed分岐内のメッセージが同一で冗長です。
debouncedTrimmedの有無で内側の分岐が同じ文言 ("No issues found." / "No open issues found.") を返しているため、外側の三項演算は実質ノーオペになっています。可読性向上のためshowClosedのみで分岐する形に縮約できます。検索時とブラウズ時で文言を変えたい意図があるならその差分を入れる必要があります。♻️ 提案 diff
{isLoading ? debouncedTrimmed ? "Searching..." : "Loading..." : repoMismatch ? `Issue URL must match ${repoMismatch}.` - : debouncedTrimmed - ? showClosed - ? "No issues found." - : "No open issues found." - : showClosed - ? "No issues found." - : "No open issues found."} + : showClosed + ? "No issues found." + : "No open issues found."}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/components/GitHubIssueLinkCommand/GitHubIssueLinkCommand.tsx` around lines 155 - 170, The nested conditional that checks debouncedTrimmed is redundant because both branches return the same messages; in GitHubIssueLinkCommand.tsx simplify the CommandEmpty rendering: keep the isLoading branch (still showing "Searching..." when debouncedTrimmed is true, otherwise "Loading..."), keep the repoMismatch branch as-is, and for the final fallback remove the debouncedTrimmed check and collapse to a single showClosed-based branch that returns showClosed ? "No issues found." : "No open issues found."; update the JSX inside the CommandEmpty (referencing searchResults, isLoading, repoMismatch, debouncedTrimmed, showClosed) to reflect this simplified logic.apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/components/PRLinkCommand/PRLinkCommand.tsx (1)
151-167: 空状態メッセージの分岐に冗長性と文言の揺れがあります。
showClosed=trueの場合、debouncedTrimmedの有無に関わらず"No pull requests found."で同一のため、内側の三項演算は不要です。showClosed=falseの場合のみ、検索時"No open pull requests found."と非検索時"No open pull requests."で末尾foundの有無が揺れています。意図的な差分でなければ揃えた方が UX として一貫します。♻️ 提案 diff（揃える例）
{isLoading ? debouncedTrimmed ? "Searching..." : "Loading..." : repoMismatch ? `PR URL must match ${repoMismatch}.` - : debouncedTrimmed - ? showClosed - ? "No pull requests found." - : "No open pull requests found." - : showClosed - ? "No pull requests found." - : "No open pull requests."} + : showClosed + ? "No pull requests found." + : "No open pull requests found."}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/components/PRLinkCommand/PRLinkCommand.tsx` around lines 151 - 167, The empty-state ternary is overly redundant and produces inconsistent wording; simplify the logic inside the CommandEmpty render so that: keep the existing loading branch (isLoading -> debouncedTrimmed ? "Searching..." : "Loading..."), keep the repoMismatch branch, then for the pullRequests empty case use a single branch for showClosed (showClosed ? "No pull requests found." : "No open pull requests found.") — remove the inner debouncedTrimmed-based duplication and unify the "open" message to always include "found". Apply this change to the component rendering CommandEmpty in PRLinkCommand so references to pullRequests, isLoading, debouncedTrimmed, repoMismatch and showClosed are updated accordingly.apps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/RequestSandboxAccessToolCall/RequestSandboxAccessToolCall.tsx (1)
89-96:ToolCallRowのisPending/isErrorをハードコードしている点の確認
statusがpending/errorの状態でもisPending={false}/isError={false}が固定で渡されています。ToolCallRow側でisPendingのときBrailleSpinnerを表示する仕様（PRサマリ参照）とトレードオフになるため、ステータスバッジに集約する意図であれば現状で問題ありませんが、UX 上スピナーが望ましい場合はisPending={status === "pending"}への切替を検討してください。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/RequestSandboxAccessToolCall/RequestSandboxAccessToolCall.tsx` around lines 89 - 96, The component is hardcoding ToolCallRow props isPending and isError to false which ignores the local status; update RequestSandboxAccessToolCall so ToolCallRow receives dynamic flags (e.g., set isPending to status === "pending" and isError to status === "error") instead of false, keeping title/description (statusBadge) as-is so the spinner behavior in ToolCallRow will activate when appropriate.packages/ui/src/components/ai-elements/tool-call-row.tsx (1)
144-155:TooltipProviderはインスタンスごとではなく上位ツリーへ持ち上げる方が望ましい。
ToolCallRowが描画されるたびにTooltipProviderを新規生成しており、各行ごとにプロバイダーを重ねる構造になっています。Radixでは通常アプリ/ページのルート付近で1つだけTooltipProviderを置く方法が推奨されており、複数のTooltip間でhover delay等を共有できます。アプリ全体ですでにルートでTooltipProviderが用意されている場合、このローカルProviderは不要です。機能的な不具合ではないため任意対応で構いません。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/ai-elements/tool-call-row.tsx` around lines 144 - 155, ToolCallRow currently creates a new TooltipProvider for every row (wrapping Tooltip/TooltipTrigger/TooltipContent), which is unnecessary and can prevent shared tooltip behavior; remove the local TooltipProvider wrapper in ToolCallRow and rely on the app/page-level TooltipProvider instead (or lift TooltipProvider up to a parent component if one doesn't exist) so keep Tooltip, TooltipTrigger, TooltipContent usage intact but eliminate per-row TooltipProvider creation.packages/host-service/src/runtime/chat/chat.ts (2)
107-150:respondToQuestionWithOptimisticStateがpackages/chat/src/server/trpc/service.tsと重複しています。ロジックはほぼ完全に同一で、
RuntimeSessionの形だけが異なる構造です。questionId/pendingQuestionResponses/pendingSandboxQuestion/answeredQuestionIdsというインターフェースを共有していれば動作するため、共通ユーティリティ（例:packages/chat/src/server/trpc/utils/runtime/runtime.ts直下、もしくはpackages/host-serviceから参照できる場所）に抽出して両方から利用するのが望ましいです。今後 dedup/ロールバックの仕様変更があった際の片側追従漏れを防げます。なお、実装ロジック自体（
===ガードによるagent_start/agent_end後のスタイル安全な扱い、!wasAlreadyAnswered条件付きロールバック）は妥当です。♻️ 共通化の方向性（イメージ）
// 共通モジュールに移動（RuntimeSession の最小構造に依存させる） export interface OptimisticQuestionRuntime<TResult> { harness: { respondToQuestion(payload: { questionId: string; answer: string }): Promise<TResult> }; answeredQuestionIds: Set<string>; pendingQuestionResponses: Map<string, Promise<TResult>>; pendingSandboxQuestion: { questionId: string } | null; } export function respondToQuestionWithOptimisticState<TResult>( runtime: OptimisticQuestionRuntime<TResult>, payload: { questionId: string; answer: string }, ): Promise<TResult> { // ...同一ロジック... }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/host-service/src/runtime/chat/chat.ts` around lines 107 - 150, The function respondToQuestionWithOptimisticState duplicates logic found elsewhere; extract it into a shared utility module as a generic function that depends only on a minimal runtime interface (e.g. OptimisticQuestionRuntime with harness.respondToQuestion, answeredQuestionIds, pendingQuestionResponses, pendingSandboxQuestion) and change the local respondToQuestionWithOptimisticState in this file to import and call that shared implementation, keeping the existing RuntimeSession usage by satisfying the interface; ensure the exported function is generic over TResult/payload shape and update both call sites to import the shared function so future changes stay in one place.
495-520: display state の harness pendingQuestion フィルタもservice.tsと重複しています。
harnessPendingQuestion ?? sandboxPendingQuestionの決定ロジックと、answeredQuestionIdsを用いた抑制条件はpackages/chat/src/server/trpc/service.tsのgetDisplayStateと完全に同じ意図です。仕様（特に「同じターンでquestionIdが回答済みなら抑制」というコメント記載のルール）は両者で必ず一致している必要があるので、こちらも共通ヘルパー（例:composeDisplayPendingQuestion(runtime, harnessPendingQuestion)）に抽出することで仕様ドリフトを防げます。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/host-service/src/runtime/chat/chat.ts` around lines 495 - 520, The harness pendingQuestion filtering logic (the harnessPendingQuestion variable and the pendingQuestion selection using answeredQuestionIds) is duplicated and must be extracted into a shared helper to avoid spec drift; create a helper (e.g., composeDisplayPendingQuestion(runtime, harnessPendingQuestion)) that encapsulates the suppression rule (suppress if runtime.answeredQuestionIds contains the questionId) and the fallback to sandboxPendingQuestion, replace the inline logic here and the identical logic in getDisplayState in service.ts to call that helper, and ensure the helper returns the final pendingQuestion shape used by the display state.packages/chat/src/server/trpc/service.ts (2)
41-88:respondToQuestionWithOptimisticStateはpackages/host-service/src/runtime/chat/chat.tsと実装が重複しています。
chat.ts側に同じ趣旨のコメントを書きましたが、こちらが共通モジュールの自然な置き場（例えば./utils/runtime/runtime.ts）になりそうです。RuntimeSession型がパッケージ内で定義されているので、エクスポートしてpackages/host-serviceから参照する、あるいは構造的型に依存するジェネリック関数として切り出すのが扱いやすいと思います。実装そのものについては問題ありません:
runtime.pendingQuestionResponses.get(questionId) === responsePromiseのガードでagent_start/agent_endによるクリア後に古いリクエストが状態を巻き戻すのを防げています。wasAlreadyAnsweredを考慮したロールバックで、二重 dedup されたケースで誤ってansweredQuestionIdsから削除しない設計になっています。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/chat/src/server/trpc/service.ts` around lines 41 - 88, The function respondToQuestionWithOptimisticState is duplicated between this module and the implementation in chat.ts; extract it into a shared utility so both packages reuse the single implementation. Create a new common module (e.g., utils/runtime) and move respondToQuestionWithOptimisticState there, parameterized to accept the RuntimeSession shape (or export RuntimeSession from the package and import it) and ensure it operates on the same properties used in the logic (pendingQuestionResponses, pendingSandboxQuestion, answeredQuestionIds, and harness.respondToQuestion). Update the callers in both places to import the shared function and remove the duplicate implementation.
63-86: Optional:let自己参照は IIFE でconst化できます（任意）。現状の
let responsePromise; responsePromise = Promise.resolve().then(...).catch(...).finally(...)は意図通り動作しますが、IIFE +constのほうがresponsePromiseが再代入不可なことを型・読み手の双方に伝えやすいです。プロジェクトのスタイル次第なので必須ではありません。♻️ 例（任意）
- let responsePromise: Promise<RuntimeQuestionResponse>; - responsePromise = Promise.resolve() - .then(() => runtime.harness.respondToQuestion(payload)) - .catch((error) => { - if ( - runtime.pendingQuestionResponses.get(questionId) === responsePromise - ) { - if (!wasAlreadyAnswered) { - runtime.answeredQuestionIds.delete(questionId); - } - if (clearsSandboxQuestion && runtime.pendingSandboxQuestion === null) { - runtime.pendingSandboxQuestion = previousSandboxQuestion; - } - } - throw error; - }) - .finally(() => { - if ( - runtime.pendingQuestionResponses.get(questionId) === responsePromise - ) { - runtime.pendingQuestionResponses.delete(questionId); - } - }); + const responsePromise: Promise<RuntimeQuestionResponse> = (async () => { + try { + return await runtime.harness.respondToQuestion(payload); + } catch (error) { + if (runtime.pendingQuestionResponses.get(questionId) === responsePromise) { + if (!wasAlreadyAnswered) runtime.answeredQuestionIds.delete(questionId); + if (clearsSandboxQuestion && runtime.pendingSandboxQuestion === null) { + runtime.pendingSandboxQuestion = previousSandboxQuestion; + } + } + throw error; + } finally { + if (runtime.pendingQuestionResponses.get(questionId) === responsePromise) { + runtime.pendingQuestionResponses.delete(questionId); + } + } + })();注:
responsePromiseを IIFE 内で参照する場合、awaitポイントを必ず通るため TDZ 違反にはなりません（同期で参照されないため）。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/chat/src/server/trpc/service.ts` around lines 63 - 86, Replace the mutable let responsePromise / later assignment pattern with a const initialized by an IIFE so responsePromise is not reassignable; wrap the existing Promise.resolve().then(...).catch(...).finally(...) chain in an immediately-invoked function expression (or an async IIFE) and assign it to const responsePromise so the subsequent runtime.pendingQuestionResponses.set(questionId, responsePromise) still refers to the same Promise; keep the existing catch behavior that references responsePromise, and ensure the IIFE introduces an await point (if using async) so TDZ is not violated when responsePromise is referenced in the catch/finally logic around runtime.harness.respondToQuestion, runtime.pendingQuestionResponses, runtime.pendingSandboxQuestion, previousSandboxQuestion, wasAlreadyAnswered, and runtime.answeredQuestionIds.apps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.tsx (2)
261-280:TableKit設定の重複:createMarkdownExtensions.tsと共通化を検討
createMarkdownExtensions.ts側のTableKit.configure({...})とこの箇所の設定がほぼ同一（resizable: false/cellMinWidth: 192/ 各HTMLAttributesの class 名まで一致）です。今後クラス名やcellMinWidthを変更する際に片方だけ更新されると、エディタとレンダラで挙動が乖離します。共通のcreateTableKit()ヘルパや設定オブジェクトに切り出すと安全です。♻️ 共通化の例
// e.g. apps/desktop/src/renderer/components/MarkdownRenderer/.../tableKitConfig.ts export const MARKDOWN_TABLE_KIT_OPTIONS = { table: { resizable: false, cellMinWidth: 192, HTMLAttributes: { class: "markdown-table my-4 min-w-full border-collapse", }, }, tableHeader: { HTMLAttributes: { class: "bg-muted px-4 py-2 text-left text-sm font-semibold align-top", }, }, tableCell: { HTMLAttributes: { class: "border-t border-border px-4 py-2 text-sm align-top", }, }, } as const;-TableKit.configure({ - table: { ... }, - tableHeader: { ... }, - tableCell: { ... }, -}), +TableKit.configure(MARKDOWN_TABLE_KIT_OPTIONS),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.tsx` around lines 261 - 280, This file duplicates the TableKit.configure(...) options already defined in createMarkdownExtensions.ts; extract the shared config into a single exported constant or factory (e.g., MARKDOWN_TABLE_KIT_OPTIONS or createTableKit()) and import and use it here and in createMarkdownExtensions.ts so both use the same table, tableHeader, and tableCell settings (resizable, cellMinWidth, and HTMLAttributes class names) to avoid drift.
185-185:editorRef.current = editorの代入位置レンダー本体での ref への代入はレンダリング自体には影響しないため致命的ではありませんが、React 18 の Strict Mode などでレンダーが二度実行されるケースを想定すると、
useEffect（またはuseLayoutEffect）内で同期するほうが意図が明確です。handlePasteから最新のeditorを参照したいだけなら、第一引数のviewから(view as any).editorを取らずに済むよう、現状の ref 経由でも実用上は十分ですが、副作用としての更新であることを示す形に整えると保守しやすくなります。♻️ useEffect で同期する例
- editorRef.current = editor; + useEffect(() => { + editorRef.current = editor; + }, [editor]);Also applies to: 356-356
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.tsx` at line 185, The component currently assigns editorRef.current = editor during render; move this assignment into a useEffect or useLayoutEffect that runs when the local editor instance changes so the ref update becomes an explicit side effect (e.g., useEffect(() => { editorRef.current = editor; }, [editor])). Remove any direct in-render writes to editorRef and ensure handlers like handlePaste read the instance from editorRef.current (and fall back safely if null) instead of extracting (view as any).editor; apply the same change for the other occurrence that updates editorRef at line ~356.apps/desktop/src/renderer/components/MarkdownEditor/markdown-editor.css (1)
89-111:min-width: 12remと TipTap 側のcellMinWidth: 192の一貫性に関する注意点
th/tdのmin-width: 12rem（=192px）は、createMarkdownExtensions.tsおよびMarkdownEditor.tsxのTableKit設定に含まれるcellMinWidth: 192と暗黙的に対応しています。片方だけ将来変更されるとレンダリングと TipTap のリサイズ/最小幅ロジックが乖離するため、コメントで対応関係を明示するか、共通の定数（CSS 変数 or TS 定数）に切り出すと安全です。♻️ コメントで対応関係を明示する例
.markdown-table th, .markdown-table td { border: 1px solid hsl(var(--border)); + /* Keep in sync with TableKit `cellMinWidth: 192` in MarkdownEditor / createMarkdownExtensions */ min-width: 12rem; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/MarkdownEditor/markdown-editor.css` around lines 89 - 111, The CSS rule setting th/td min-width: 12rem (192px) must be explicitly kept in sync with the TipTap TableKit setting cellMinWidth: 192 (found in createMarkdownExtensions.ts and MarkdownEditor.tsx); either add a clear comment in markdown-editor.css referencing those symbols and explaining the mapping, or extract the value to a shared constant (a CSS variable referenced in markdown-editor.css and a TS constant imported into createMarkdownExtensions.ts/MarkdownEditor.tsx or vice versa) so both the stylesheet and the TableKit config use the same single source of truth.apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsx (1)
211-225: Kill ミューテーションがフック単位で 1 つのため、複数ターミナルで disabled が共有されます。
useMutationをフックのトップレベルで一度だけ呼び出しているので、isKillingTerminalSessionはそのフック呼び出しに紐づく単一フラグです。あるターミナルで Kill 実行中に、別ターミナルの「Kill Terminal Session」メニューもdisabledになります。Kill は希少操作なので致命的ではありませんが、誤解を招くため、対象terminalIdが一致するときのみ無効化する、もしくは disabled を外して onSelect 側で多重実行を弾く方が直感的です。♻️ 例: 確認ダイアログのみで多重実行を抑止する
- const killAction: ContextMenuActionConfig<PaneViewerData> = { - key: "kill-terminal-session", - label: "Kill Terminal Session", - icon: <LuPower />, - variant: "destructive", - disabled: isKillingTerminalSession, + const killAction: ContextMenuActionConfig<PaneViewerData> = { + key: "kill-terminal-session", + label: "Kill Terminal Session", + icon: <LuPower />, + variant: "destructive",Also applies to: 412-417
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsx around lines 211 - 225, The current kill mutation (workspaceTrpc.terminal.killSession.useMutation) exposes a single isKillingTerminalSession flag shared across all terminals, causing every "Kill Terminal Session" control to be disabled when any kill is in progress; change this so the disabled state is per-terminal by either (A) moving the useMutation call into the per-terminal render/hook so each terminal gets its own mutate/isPending, or (B) keep a single mutation but track the active terminalId (e.g., currentKillingTerminalId) and only set disabled when currentKillingTerminalId === terminalId; additionally ensure onSelect checks and short-circuits duplicate kills by comparing terminalId before calling killTerminalSession. Use the symbols killTerminalSession, isKillingTerminalSession, workspaceTrpc.terminal.killSession.useMutation and the onSelect handler to implement the fix.packages/host-service/src/terminal/terminal.ts (2)
101-146:listTerminalSessions内でソケットを副作用的にprune している点は許容範囲ですが、責務分離の余地はあります
pruneAndCountOpenSocketsは読み取り系のlistTerminalSessionsから呼ばれている一方でsession.socketsを破壊的に変更します。Node.jsはシングルスレッドなので競合の心配は無く、また閉じたソケットを早期に取り除くハウスキーピングとして合理的ですが、将来listTerminalSessionsが頻繁な内部呼び出し対象になった場合に副作用が見えづらくなります。任意ですが、broadcastMessage/onClose/onErrorの経路に prune 責任を寄せ、listing側は単にreadyState === SOCKET_OPENをカウントするだけにするとより素直です。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/host-service/src/terminal/terminal.ts` around lines 101 - 146, pruneAndCountOpenSockets currently mutates session.sockets while being called from the read-only listTerminalSessions; change listing to only count sockets with readyState === SOCKET_OPEN and remove the pruning side-effect from pruneAndCountOpenSockets (or remove that helper). Move the responsibility for removing closed sockets into the socket lifecycle handlers (e.g., broadcastMessage, onClose, onError) so those places call session.sockets.delete(socket) when a socket transitions to CLOSING/CLOSED; keep references to pruneAndCountOpenSockets, listTerminalSessions, broadcastMessage, onClose, onError and session.sockets to locate the changes.
46-54:SOCKET_OPEN/CLOSING/CLOSED定数にWebSocket仕様準拠であることのコメントを添えると親切です数値リテラル
1/2/3はWebSocket仕様のreadyState定数と一致していますが、@hono/node-wsではWebSocketグローバルから直接定数を取り出せないケースがあります。意図が伝わるように一行コメントを追加するか、単一のSOCKET_READY_STATEオブジェクトにまとめると、後の読み手の混乱を防げます。♻️ 提案差分
-const SOCKET_OPEN = 1; -const SOCKET_CLOSING = 2; -const SOCKET_CLOSED = 3; +// WebSocket spec readyState values. node-ws does not expose the WebSocket +// global with these constants, so we mirror them locally. +const SOCKET_OPEN = 1; +const SOCKET_CLOSING = 2; +const SOCKET_CLOSED = 3;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/host-service/src/terminal/terminal.ts` around lines 46 - 54, The constants SOCKET_OPEN, SOCKET_CLOSING, and SOCKET_CLOSED currently use numeric literals (1/2/3) that match the WebSocket readyState values but lack clarification; update the declaration near TerminalSocket to either add a one-line comment stating these map to the WebSocket readyState spec (link to MDN optional) or replace the three separate constants with a single object like SOCKET_READY_STATE = { OPEN: 1, CLOSING: 2, CLOSED: 3 } and update usages to reference the object (e.g., SOCKET_READY_STATE.OPEN) so intent is explicit and future readers won’t be confused by raw numbers.
🤖 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/renderer/components/MarkdownEditor/MarkdownEditor.tsx`:
- Around line 155-167: The current isMarkdownTable function enforces "2+ columns
and 3+ dashes" because the regex uses (\|\s*:?-{3,}:?\s*)+ and -{3,}; to fix,
relax the detection to match GFM-legal tables by changing the regex in
isMarkdownTable to allow a single column and allow one-or-more dashes (e.g. use
^\|?\s*:?-{1,}:?\s*(\|\s*:?-{1,}:?\s*)*\|?$), or alternatively add a clear
comment above isMarkdownTable explaining the intentional stricter constraint if
you want to keep the current behavior. Ensure you update only the regex and/or
add the explanatory comment in MarkdownEditor.tsx.
- Around line 316-331: The handlePaste handler currently calls
event.preventDefault() then returns the boolean result of
currentEditor.commands.insertContentAt(...), which can cause paste to be lost if
insertContentAt returns false; change the logic in handlePaste so that after
event.preventDefault() you still call
currentEditor.commands.insertContentAt(...) but always return true to signal the
event was handled (e.g., keep using currentEditor and insertContentAt but ignore
its boolean result and return true).
- Around line 316-331: The paste handler (handlePaste) currently calls
currentEditor.commands.insertContentAt(...) with raw Markdown text causing
Tiptap to interpret it as JSON; update the call in the handlePaste block (using
editorRef.current and isMarkdownTable) to pass the Markdown parse option by
adding the third argument { contentType: 'markdown' } to insertContentAt so
pasted pipe-table Markdown is parsed into a table node; keep the existing
event.preventDefault and selection range logic unchanged and verify both edit
and read-only views render the pasted table correctly.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/TerminalSessionDropdown/TerminalSessionDropdown.tsx:
- Around line 153-182: The lifecycle hook double-kills because closing panes
triggers cleanupRemovedTerminal which calls terminal.killSession if the terminal
isn't marked for background; to prevent this, ensure the terminal is marked for
background before panes are closed or before the explicit kill: call
markTerminalForBackground(targetTerminalId) (await if it returns a promise)
prior to invoking closePanesForTerminal or before calling removeTerminalSession,
so that useGlobalTerminalLifecycle's cleanupRemovedTerminal takes the release()
path and does not invoke terminal.killSession a second time. Update the flow in
handleRemoveTerminal/removeTerminalSession to mark first, then proceed with
killing and pane closing.
In
`@apps/desktop/src/renderer/routes/_authenticated/hooks/useDashboardSidebarState/useDashboardSidebarState.ts`:
- Around line 89-107: cleanupWorkspacePaneRuntimes is calling
terminalRuntimeRegistry.release(terminalId) without the per-pane instance id, so
when multiple panes share the same terminalId all instances get destroyed;
update cleanup to release per-pane instances instead of by-terminal: use
extractPaneIds (or similar iteration) to obtain each Pane's terminalInstanceId
(from getTerminalRuntimeId or by reading pane.data.terminalInstanceId tied to
TerminalPane/terminalRuntimeRegistry.mount) and call
terminalRuntimeRegistry.release(terminalId, terminalInstanceId) for each pane,
while keeping browserRuntimeRegistry.destroy(browserId) as-is.
In `@packages/ui/src/components/ai-elements/tool-call-row.tsx`:
- Around line 28-33: The JSDoc for the title prop claims string titles are
wrapped in a ShimmerLabel while isPending is true, but the implementation in
tool-call-row.tsx currently wraps titles with a plain <span> and does not
import/use ShimmerLabel, causing a doc/behavior mismatch; fix by either (A)
implementing the shimmer behavior: import ShimmerLabel and, inside the component
that renders title (the prop on ToolCallRow / function rendering the
header/title and used by callers like RequestSandboxAccessToolCall), detect
typeof title === "string" and render <ShimmerLabel
isPending={isPending}>{title}</ShimmerLabel> instead of the plain span, or (B)
update the JSDoc comment for title to match current behavior (explain that a
plain span is used and that only the left icon changes to BrailleSpinner when
pending); choose one approach and apply consistently and ensure the import for
ShimmerLabel is added/removed accordingly.
- Around line 118-135: The chevron visibility currently depends only on mouse
hover (setIsHovered/isHovered), so keyboard focus users and touch users don't
see the expand indicator; update the button to also track focus (add
onFocus/onBlur handlers to setIsHovered or a new isFocused state) and include
aria-expanded tied to isOpen, or instead rely on Radix Collapsible's data-state
attribute in the render/CSS to show ChevronDownIcon/ChevronRightIcon when
expanded/collapsed; adjust the conditional around
Icon/ChevronDownIcon/ChevronRightIcon (referencing isHovered, isFocused or
data-state, hasDetails, isOpen, isPending) so focus-visible shows the chevron
the same way hover does and ensure aria-expanded is present on the toggling
element.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/components/Chat/ChatInterface/components/IssueLinkCommand/IssueLinkCommand.tsx`:
- Around line 90-130: Compute the "visibleTasks" once using useMemo (based on
allTasks, showClosed, statusMap and isClosedStatus) and then feed that memoized
visibleTasks into the Fuse constructor (taskFuse) and reuse it in filteredTasks
for the non-search path (sorting + slice(MAX_RESULTS)), removing the duplicated
filter logic; update taskFuse to new Fuse(visibleTasks, ...) and change
filteredTasks to read from visibleTasks instead of re-filtering allTasks.
In
`@apps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/AskUserQuestionToolCall/AskUserQuestionToolCall.tsx`:
- Around line 235-259: The two early-return branches return identical JSX and
can be merged: combine the conditions (hasOutput || !onAnswer ||
optimisticAnswers) and (!canRespond) into a single condition so the component
returns the same block once when any of those are true; update the if to check
(hasOutput || !onAnswer || optimisticAnswers || !canRespond) and keep the
returned JSX (messageBlock plus the SupersetToolCall with part and
MessageCircleQuestionIcon) unchanged to remove duplication.
In
`@apps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/RequestSandboxAccessToolCall/RequestSandboxAccessToolCall.tsx`:
- Around line 89-96: The component is hardcoding ToolCallRow props isPending and
isError to false which ignores the local status; update
RequestSandboxAccessToolCall so ToolCallRow receives dynamic flags (e.g., set
isPending to status === "pending" and isError to status === "error") instead of
false, keeping title/description (statusBadge) as-is so the spinner behavior in
ToolCallRow will activate when appropriate.
In `@apps/desktop/src/renderer/components/MarkdownEditor/markdown-editor.css`:
- Around line 89-111: The CSS rule setting th/td min-width: 12rem (192px) must
be explicitly kept in sync with the TipTap TableKit setting cellMinWidth: 192
(found in createMarkdownExtensions.ts and MarkdownEditor.tsx); either add a
clear comment in markdown-editor.css referencing those symbols and explaining
the mapping, or extract the value to a shared constant (a CSS variable
referenced in markdown-editor.css and a TS constant imported into
createMarkdownExtensions.ts/MarkdownEditor.tsx or vice versa) so both the
stylesheet and the TableKit config use the same single source of truth.
In `@apps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.tsx`:
- Around line 261-280: This file duplicates the TableKit.configure(...) options
already defined in createMarkdownExtensions.ts; extract the shared config into a
single exported constant or factory (e.g., MARKDOWN_TABLE_KIT_OPTIONS or
createTableKit()) and import and use it here and in createMarkdownExtensions.ts
so both use the same table, tableHeader, and tableCell settings (resizable,
cellMinWidth, and HTMLAttributes class names) to avoid drift.
- Line 185: The component currently assigns editorRef.current = editor during
render; move this assignment into a useEffect or useLayoutEffect that runs when
the local editor instance changes so the ref update becomes an explicit side
effect (e.g., useEffect(() => { editorRef.current = editor; }, [editor])).
Remove any direct in-render writes to editorRef and ensure handlers like
handlePaste read the instance from editorRef.current (and fall back safely if
null) instead of extracting (view as any).editor; apply the same change for the
other occurrence that updates editorRef at line ~356.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsx:
- Around line 211-225: The current kill mutation
(workspaceTrpc.terminal.killSession.useMutation) exposes a single
isKillingTerminalSession flag shared across all terminals, causing every "Kill
Terminal Session" control to be disabled when any kill is in progress; change
this so the disabled state is per-terminal by either (A) moving the useMutation
call into the per-terminal render/hook so each terminal gets its own
mutate/isPending, or (B) keep a single mutation but track the active terminalId
(e.g., currentKillingTerminalId) and only set disabled when
currentKillingTerminalId === terminalId; additionally ensure onSelect checks and
short-circuits duplicate kills by comparing terminalId before calling
killTerminalSession. Use the symbols killTerminalSession,
isKillingTerminalSession, workspaceTrpc.terminal.killSession.useMutation and the
onSelect handler to implement the fix.
In
`@apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/components/GitHubIssueLinkCommand/GitHubIssueLinkCommand.tsx`:
- Around line 155-170: The nested conditional that checks debouncedTrimmed is
redundant because both branches return the same messages; in
GitHubIssueLinkCommand.tsx simplify the CommandEmpty rendering: keep the
isLoading branch (still showing "Searching..." when debouncedTrimmed is true,
otherwise "Loading..."), keep the repoMismatch branch as-is, and for the final
fallback remove the debouncedTrimmed check and collapse to a single
showClosed-based branch that returns showClosed ? "No issues found." : "No open
issues found."; update the JSX inside the CommandEmpty (referencing
searchResults, isLoading, repoMismatch, debouncedTrimmed, showClosed) to reflect
this simplified logic.
In
`@apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/components/PRLinkCommand/PRLinkCommand.tsx`:
- Around line 151-167: The empty-state ternary is overly redundant and produces
inconsistent wording; simplify the logic inside the CommandEmpty render so that:
keep the existing loading branch (isLoading -> debouncedTrimmed ? "Searching..."
: "Loading..."), keep the repoMismatch branch, then for the pullRequests empty
case use a single branch for showClosed (showClosed ? "No pull requests found."
: "No open pull requests found.") — remove the inner debouncedTrimmed-based
duplication and unify the "open" message to always include "found". Apply this
change to the component rendering CommandEmpty in PRLinkCommand so references to
pullRequests, isLoading, debouncedTrimmed, repoMismatch and showClosed are
updated accordingly.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceInitializingView/WorkspaceInitializingView.tsx`:
- Line 331: The initializing view currently renders workspaceName with <p
className="text-sm text-muted-foreground">{workspaceName}</p> (in
WorkspaceInitializingView) but does not apply the same truncation/clamp and
title used in the "Setup incomplete"/"Workspace setup failed" cases; update that
<p> to use the same clamp/truncate utility classes and add a title attribute set
to workspaceName so long names are truncated consistently, and to avoid
duplication extract the repeated class string into a local constant (e.g.,
clampClass) used by all three places in WorkspaceInitializingView.
In `@packages/chat/src/server/trpc/service.ts`:
- Around line 41-88: The function respondToQuestionWithOptimisticState is
duplicated between this module and the implementation in chat.ts; extract it
into a shared utility so both packages reuse the single implementation. Create a
new common module (e.g., utils/runtime) and move
respondToQuestionWithOptimisticState there, parameterized to accept the
RuntimeSession shape (or export RuntimeSession from the package and import it)
and ensure it operates on the same properties used in the logic
(pendingQuestionResponses, pendingSandboxQuestion, answeredQuestionIds, and
harness.respondToQuestion). Update the callers in both places to import the
shared function and remove the duplicate implementation.
- Around line 63-86: Replace the mutable let responsePromise / later assignment
pattern with a const initialized by an IIFE so responsePromise is not
reassignable; wrap the existing
Promise.resolve().then(...).catch(...).finally(...) chain in an
immediately-invoked function expression (or an async IIFE) and assign it to
const responsePromise so the subsequent
runtime.pendingQuestionResponses.set(questionId, responsePromise) still refers
to the same Promise; keep the existing catch behavior that references
responsePromise, and ensure the IIFE introduces an await point (if using async)
so TDZ is not violated when responsePromise is referenced in the catch/finally
logic around runtime.harness.respondToQuestion,
runtime.pendingQuestionResponses, runtime.pendingSandboxQuestion,
previousSandboxQuestion, wasAlreadyAnswered, and runtime.answeredQuestionIds.
In `@packages/host-service/src/runtime/chat/chat.ts`:
- Around line 107-150: The function respondToQuestionWithOptimisticState
duplicates logic found elsewhere; extract it into a shared utility module as a
generic function that depends only on a minimal runtime interface (e.g.
OptimisticQuestionRuntime with harness.respondToQuestion, answeredQuestionIds,
pendingQuestionResponses, pendingSandboxQuestion) and change the local
respondToQuestionWithOptimisticState in this file to import and call that shared
implementation, keeping the existing RuntimeSession usage by satisfying the
interface; ensure the exported function is generic over TResult/payload shape
and update both call sites to import the shared function so future changes stay
in one place.
- Around line 495-520: The harness pendingQuestion filtering logic (the
harnessPendingQuestion variable and the pendingQuestion selection using
answeredQuestionIds) is duplicated and must be extracted into a shared helper to
avoid spec drift; create a helper (e.g., composeDisplayPendingQuestion(runtime,
harnessPendingQuestion)) that encapsulates the suppression rule (suppress if
runtime.answeredQuestionIds contains the questionId) and the fallback to
sandboxPendingQuestion, replace the inline logic here and the identical logic in
getDisplayState in service.ts to call that helper, and ensure the helper returns
the final pendingQuestion shape used by the display state.
In `@packages/host-service/src/terminal/terminal.ts`:
- Around line 101-146: pruneAndCountOpenSockets currently mutates
session.sockets while being called from the read-only listTerminalSessions;
change listing to only count sockets with readyState === SOCKET_OPEN and remove
the pruning side-effect from pruneAndCountOpenSockets (or remove that helper).
Move the responsibility for removing closed sockets into the socket lifecycle
handlers (e.g., broadcastMessage, onClose, onError) so those places call
session.sockets.delete(socket) when a socket transitions to CLOSING/CLOSED; keep
references to pruneAndCountOpenSockets, listTerminalSessions, broadcastMessage,
onClose, onError and session.sockets to locate the changes.
- Around line 46-54: The constants SOCKET_OPEN, SOCKET_CLOSING, and
SOCKET_CLOSED currently use numeric literals (1/2/3) that match the WebSocket
readyState values but lack clarification; update the declaration near
TerminalSocket to either add a one-line comment stating these map to the
WebSocket readyState spec (link to MDN optional) or replace the three separate
constants with a single object like SOCKET_READY_STATE = { OPEN: 1, CLOSING: 2,
CLOSED: 3 } and update usages to reference the object (e.g.,
SOCKET_READY_STATE.OPEN) so intent is explicit and future readers won’t be
confused by raw numbers.
In `@packages/ui/src/components/ai-elements/tool-call-row.tsx`:
- Around line 144-155: ToolCallRow currently creates a new TooltipProvider for
every row (wrapping Tooltip/TooltipTrigger/TooltipContent), which is unnecessary
and can prevent shared tooltip behavior; remove the local TooltipProvider
wrapper in ToolCallRow and rely on the app/page-level TooltipProvider instead
(or lift TooltipProvider up to a parent component if one doesn't exist) so keep
Tooltip, TooltipTrigger, TooltipContent usage intact but eliminate per-row
TooltipProvider creation.
🪄 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: 9e1077d3-a6b4-4115-aa8a-2dfe0319b035
📒 Files selected for processing (58)
apps/desktop/src/lib/trpc/routers/projects/projects.tsapps/desktop/src/renderer/components/Chat/ChatInterface/components/ChatInputFooter/ChatInputFooter.tsxapps/desktop/src/renderer/components/Chat/ChatInterface/components/ChatInputFooter/components/QuestionInputOverlay/QuestionInputOverlay.tsxapps/desktop/src/renderer/components/Chat/ChatInterface/components/IssueLinkCommand/IssueLinkCommand.tsxapps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/ToolCallBlock.tsxapps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/AskUserQuestionToolCall/AskUserQuestionToolCall.tsxapps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/RequestSandboxAccessToolCall/RequestSandboxAccessToolCall.tsxapps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/RequestSandboxAccessToolCall/index.tsapps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/ToolStatusBadge/ToolStatusBadge.tsxapps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/ToolStatusBadge/index.tsapps/desktop/src/renderer/components/Chat/ChatInterface/utils/tool-helpers.test.tsapps/desktop/src/renderer/components/Chat/ChatInterface/utils/tool-helpers.tsapps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.tsxapps/desktop/src/renderer/components/MarkdownEditor/markdown-editor.cssapps/desktop/src/renderer/components/MarkdownRenderer/components/TipTapMarkdownRenderer/createMarkdownExtensions.tsapps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/components/GitHubIssueLinkCommand/GitHubIssueLinkCommand.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/components/PRLinkCommand/PRLinkCommand.tsxapps/desktop/src/renderer/lib/terminal/terminal-background-intents.tsapps/desktop/src/renderer/lib/terminal/terminal-runtime-registry.tsapps/desktop/src/renderer/lib/terminal/terminal-runtime.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useConsumePendingLaunch/useConsumePendingLaunch.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/DiffPane.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/DiffFileEntry/DiffFileEntry.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/DiffFileHeader/DiffFileHeader.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/WorkspaceDiff/WorkspaceDiff.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/TerminalPane.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/TerminalHeaderExtras/TerminalHeaderExtras.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/TerminalHeaderExtras/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/TerminalSessionDropdown/TerminalSessionDropdown.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/TerminalSessionDropdown/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/V2WorkspacesList.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/components/V2WorkspaceRow/V2WorkspaceRow.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/constants.tsapps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/components/GitHubIssueLinkCommand/GitHubIssueLinkCommand.tsxapps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/components/PRLinkCommand/PRLinkCommand.tsxapps/desktop/src/renderer/routes/_authenticated/components/GlobalBrowserLifecycle/hooks/useGlobalBrowserLifecycle/useGlobalBrowserLifecycle.tsapps/desktop/src/renderer/routes/_authenticated/components/GlobalTerminalLifecycle/hooks/useGlobalTerminalLifecycle/useGlobalTerminalLifecycle.tsapps/desktop/src/renderer/routes/_authenticated/components/utils/paneLifecycleRows/index.tsapps/desktop/src/renderer/routes/_authenticated/components/utils/paneLifecycleRows/paneLifecycleRows.test.tsapps/desktop/src/renderer/routes/_authenticated/components/utils/paneLifecycleRows/paneLifecycleRows.tsapps/desktop/src/renderer/routes/_authenticated/hooks/useDashboardSidebarState/useDashboardSidebarState.tsapps/desktop/src/renderer/routes/_authenticated/layout.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceInitializingView/WorkspaceInitializingView.tsxpackages/chat/src/server/trpc/service.test.tspackages/chat/src/server/trpc/service.tspackages/chat/src/server/trpc/utils/runtime/index.tspackages/chat/src/server/trpc/utils/runtime/runtime.test.tspackages/chat/src/server/trpc/utils/runtime/runtime.tspackages/host-service/src/runtime/chat/chat.tspackages/host-service/src/runtime/teardown/teardown.tspackages/host-service/src/terminal/terminal.tspackages/host-service/src/trpc/router/terminal/terminal.tspackages/host-service/src/trpc/router/workspace-creation/procedures/search-github-issues.tspackages/host-service/src/trpc/router/workspace-creation/procedures/search-pull-requests.tspackages/host-service/src/trpc/router/workspace-creation/schemas.tspackages/ui/src/components/ai-elements/braille-spinner.tsxpackages/ui/src/components/ai-elements/tool-call-row.tsx
💤 Files with no reviewable changes (1)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useConsumePendingLaunch/useConsumePendingLaunch.ts
Comment on lines
+155
to
+167
| function isMarkdownTable(text: string): boolean { | ||
| const lines = text | ||
| .trim() | ||
| .split(/\r?\n/) | ||
| .map((line) => line.trim()) | ||
| .filter(Boolean); | ||
|
|
||
| if (lines.length < 2 || !lines[0]?.includes("|")) { | ||
| return false; | ||
| } | ||
|
|
||
| return /^\|?\s*:?-{3,}:?\s*(\|\s*:?-{3,}:?\s*)+\|?$/.test(lines[1]); | ||
| } |
There was a problem hiding this comment.
isMarkdownTable の検出条件は「区切り行に2列以上 + 3本以上のダッシュ」が必須となる点に注意
正規表現 ^\|?\s*:?-{3,}:?\s*(\|\s*:?-{3,}:?\s*)+\|?$ は (...)+ のため2 列以上が必須で、また各列は 3 本以上のダッシュが必要です。GFM 仕様上は 1 列／ダッシュ 1 本でも合法なため、たとえば | --- |（1 列）や |-|-|（ダッシュ 1 本）といったテーブルは検出から漏れ、通常のテキスト貼り付け扱いになります。実用上はほぼ問題ない想定だと思いますが、意図した制約であることをコメントで残すか、許容範囲を広げるかをご検討ください。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.tsx`
around lines 155 - 167, The current isMarkdownTable function enforces "2+
columns and 3+ dashes" because the regex uses (\|\s*:?-{3,}:?\s*)+ and -{3,}; to
fix, relax the detection to match GFM-legal tables by changing the regex in
isMarkdownTable to allow a single column and allow one-or-more dashes (e.g. use
^\|?\s*:?-{1,}:?\s*(\|\s*:?-{1,}:?\s*)*\|?$), or alternatively add a clear
comment above isMarkdownTable explaining the intentional stricter constraint if
you want to keep the current behavior. Ensure you update only the regex and/or
add the explanatory comment in MarkdownEditor.tsx.
Comment on lines
+316
to
+331
| handlePaste: (_, event) => { | ||
| const text = event.clipboardData?.getData("text/plain") ?? ""; | ||
| const currentEditor = editorRef.current; | ||
| if (!currentEditor || !isMarkdownTable(text)) { | ||
| return false; | ||
| } | ||
|
|
||
| event.preventDefault(); | ||
| return currentEditor.commands.insertContentAt( | ||
| { | ||
| from: currentEditor.state.selection.from, | ||
| to: currentEditor.state.selection.to, | ||
| }, | ||
| text, | ||
| ); | ||
| }, |
There was a problem hiding this comment.
handlePaste の戻り値: preventDefault 後は明示的に true を返す方が安全
event.preventDefault() を呼んだ後で currentEditor.commands.insertContentAt(...) の戻り値（コマンド成功可否の boolean）をそのまま返しています。万一 insertContentAt が false を返した場合、ProseMirror 側はハンドラ未処理とみなしますが、ブラウザの既定動作は止まっているため貼り付けが消失してしまいます。preventDefault を呼んだ時点で「このハンドラで処理した」と宣言しているので、戻り値は true 固定にする方が意図が明確で挙動も安定します。
🐛 提案する修正
handlePaste: (_, event) => {
const text = event.clipboardData?.getData("text/plain") ?? "";
const currentEditor = editorRef.current;
if (!currentEditor || !isMarkdownTable(text)) {
return false;
}
event.preventDefault();
- return currentEditor.commands.insertContentAt(
+ currentEditor.commands.insertContentAt(
{
from: currentEditor.state.selection.from,
to: currentEditor.state.selection.to,
},
text,
);
+ return true;
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| handlePaste: (_, event) => { | |
| const text = event.clipboardData?.getData("text/plain") ?? ""; | |
| const currentEditor = editorRef.current; | |
| if (!currentEditor || !isMarkdownTable(text)) { | |
| return false; | |
| } | |
| event.preventDefault(); | |
| return currentEditor.commands.insertContentAt( | |
| { | |
| from: currentEditor.state.selection.from, | |
| to: currentEditor.state.selection.to, | |
| }, | |
| text, | |
| ); | |
| }, | |
| handlePaste: (_, event) => { | |
| const text = event.clipboardData?.getData("text/plain") ?? ""; | |
| const currentEditor = editorRef.current; | |
| if (!currentEditor || !isMarkdownTable(text)) { | |
| return false; | |
| } | |
| event.preventDefault(); | |
| currentEditor.commands.insertContentAt( | |
| { | |
| from: currentEditor.state.selection.from, | |
| to: currentEditor.state.selection.to, | |
| }, | |
| text, | |
| ); | |
| return true; | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.tsx`
around lines 316 - 331, The handlePaste handler currently calls
event.preventDefault() then returns the boolean result of
currentEditor.commands.insertContentAt(...), which can cause paste to be lost if
insertContentAt returns false; change the logic in handlePaste so that after
event.preventDefault() you still call
currentEditor.commands.insertContentAt(...) but always return true to signal the
event was handled (e.g., keep using currentEditor and insertContentAt but ignore
its boolean result and return true).
🧩 Analysis chain
🌐 Web query:
tiptap-markdown insertContent insertContentAt parse markdown string
💡 Result:
To insert or parse a Markdown string in Tiptap, use the official @tiptap/markdown extension (released in Tiptap 3.7.0+). Add Markdown to your editor extensions along with StarterKit or other base extensions. ## Setup import { Editor } from '@tiptap/core' import StarterKit from '@tiptap/starter-kit' import Markdown from '@tiptap/markdown' const editor = new Editor({ extensions: [StarterKit, Markdown], content: '# Initial Markdown', // Parses automatically if contentType: 'markdown' specified }) ## Insert Markdown at Cursor (insertContent) editor.commands.insertContent('Bold text', { contentType: 'markdown' }) Parses the Markdown string and inserts at current selection. Supports parseOptions and updateSelection. ## Insert Markdown at Position (insertContentAt) editor.commands.insertContentAt(12, '## Heading', { contentType: 'markdown' }) Or replace a range: editor.commands.insertContentAt({ from: 12, to: 20 }, 'Replacement', { contentType: 'markdown' }) position can be number or { from, to }. Autodetects content type if unspecified (defaults to 'json'). ## Full Content Replacement (setContent) editor.commands.setContent('# Full Markdown Document', { contentType: 'markdown' }) The Markdown extension overrides core content commands to support contentType: 'markdown', using MarkedJS for parsing into Tiptap JSON. Export with editor.getMarkdown. Note: Older community tiptap-markdown exists but official is preferred and more feature-complete as of 2026.
Citations:
- 1: https://tiptap.dev/docs/editor/api/commands/content/insert-content
- 2: https://tiptap.dev/docs/editor/api/commands/content/insert-content-at
- 3: https://tiptap.dev/docs/editor/markdown/api
- 4: https://www.tiptap.dev/docs/editor/markdown/getting-started/basic-usage
- 5: https://tiptap.dev/docs/editor/markdown
🏁 Script executed:
cat -n apps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.tsx | head -350 | tail -100Repository: MocA-Love/superset
Length of output: 3691
🏁 Script executed:
rg "insertContent" apps/desktop/src --type ts --type tsx -A 2 -B 2Repository: MocA-Love/superset
Length of output: 90
🏁 Script executed:
rg "insertContent" apps/desktop/src -A 2 -B 2Repository: MocA-Love/superset
Length of output: 3193
🏁 Script executed:
rg "isMarkdownTable" apps/desktop/src -B 3 -A 5Repository: MocA-Love/superset
Length of output: 1845
🏁 Script executed:
rg "isMarkdownTable" apps/desktop/src -A 15Repository: MocA-Love/superset
Length of output: 3150
🏁 Script executed:
rg "contentType.*markdown" apps/desktop/srcRepository: MocA-Love/superset
Length of output: 44
🏁 Script executed:
cat -n apps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.tsx | sed -n '292,296p'Repository: MocA-Love/superset
Length of output: 206
insertContentAt に Markdown パースオプションを追加
insertContentAt(range, text) を生の Markdown 文字列で呼び出す場合、Tiptap の公式 @tiptap/markdown 拡張では { contentType: 'markdown' } オプションを明示的に渡す必要があります。このオプション無しではデフォルトで JSON として解釈されるため、パイプ記法のテーブルがプレーンテキストとして挿入されます。現在のコードでは以下のように修正してください：
insertContentAt(
{
from: currentEditor.state.selection.from,
to: currentEditor.state.selection.to,
},
text,
{ contentType: 'markdown' }
)
修正後、編集ビューと読み取り専用ビューの両方で、Markdown テーブルをペーストするとテーブルノードとして展開されることを動作確認してください。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.tsx`
around lines 316 - 331, The paste handler (handlePaste) currently calls
currentEditor.commands.insertContentAt(...) with raw Markdown text causing
Tiptap to interpret it as JSON; update the call in the handlePaste block (using
editorRef.current and isMarkdownTable) to pass the Markdown parse option by
adding the third argument { contentType: 'markdown' } to insertContentAt so
pasted pipe-table Markdown is parsed into a table node; keep the existing
event.preventDefault and selection range logic unchanged and verify both edit
and read-only views render the pasted table correctly.
Comment on lines
+28
to
+33
| /** | ||
| * Header title. A plain string is wrapped in a ShimmerLabel that pulses while | ||
| * `isPending` is true. Any other ReactNode is rendered as-is (useful when the | ||
| * title contains interactive elements like clickable file paths). | ||
| */ | ||
| title: ReactNode; |
There was a problem hiding this comment.
JSDocと実装の不一致: ShimmerLabelが使われていません。
titleのJSDocには「文字列の場合はShimmerLabelでラップされ、isPendingがtrueの間パルスする」と記載されていますが、Lines 94-99の実装は単なる<span className="shrink-0 text-xs text-foreground">でラップしているだけで、シマー/パルス効果はありません。ShimmerLabelのimportもありません。
呼び出し側（RequestSandboxAccessToolCall等）はこのドキュメントを読んで「pending時に文字列タイトルがアニメーションする」と期待する可能性が高いため、以下のいずれかの対応をご検討ください:
- 意図がpending時のシマー表示なら、
ShimmerLabelを実装/import して適用する - 意図しないなら、JSDocの記述を実装に合わせて修正する（左アイコンが
BrailleSpinnerに切り替わるだけ、と明記）
📝 ドキュメント修正案（実装に合わせる場合）
/**
- * Header title. A plain string is wrapped in a ShimmerLabel that pulses while
- * `isPending` is true. Any other ReactNode is rendered as-is (useful when the
- * title contains interactive elements like clickable file paths).
+ * Header title. A plain string is rendered in a muted xs span. Any other
+ * ReactNode is rendered as-is (useful when the title contains interactive
+ * elements like clickable file paths). When `isPending` is true, the leading
+ * icon is replaced by a BrailleSpinner; the title itself is not animated.
*/
title: ReactNode;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui/src/components/ai-elements/tool-call-row.tsx` around lines 28 -
33, The JSDoc for the title prop claims string titles are wrapped in a
ShimmerLabel while isPending is true, but the implementation in
tool-call-row.tsx currently wraps titles with a plain <span> and does not
import/use ShimmerLabel, causing a doc/behavior mismatch; fix by either (A)
implementing the shimmer behavior: import ShimmerLabel and, inside the component
that renders title (the prop on ToolCallRow / function rendering the
header/title and used by callers like RequestSandboxAccessToolCall), detect
typeof title === "string" and render <ShimmerLabel
isPending={isPending}>{title}</ShimmerLabel> instead of the plain span, or (B)
update the JSDoc comment for title to match current behavior (explain that a
plain span is used and that only the left icon changes to BrailleSpinner when
pending); choose one approach and apply consistently and ensure the import for
ShimmerLabel is added/removed accordingly.
Comment on lines
+118
to
+135
| onMouseEnter={() => setIsHovered(true)} | ||
| onMouseLeave={() => setIsHovered(false)} | ||
| type="button" | ||
| > | ||
| <div className="flex min-w-0 flex-1 items-center gap-1.5 text-xs"> | ||
| {isHovered && hasDetails ? ( | ||
| isOpen ? ( | ||
| <ChevronDownIcon className="h-3 w-3 shrink-0 text-muted-foreground" /> | ||
| ) : ( | ||
| <ChevronRightIcon className="h-3 w-3 shrink-0 text-muted-foreground" /> | ||
| ) | ||
| ) : isPending ? ( | ||
| <span className="flex h-3 w-3 shrink-0 items-center justify-center overflow-hidden"> | ||
| <BrailleSpinner /> | ||
| </span> | ||
| ) : ( | ||
| <Icon className="h-3 w-3 shrink-0 text-muted-foreground" /> | ||
| )} |
There was a problem hiding this comment.
キーボードフォーカス時にシェブロンが表示されません（アクセシビリティ）。
展開可能性のインジケータ（ChevronDown/Right）がisHovered（マウスイベント）にのみ依存しているため、キーボードでフォーカスした利用者には「展開可能」であることが視覚的に伝わりません。focus-visible:ringはありますが、ホバー時と異なりアイコンは元のIconのまま、かつaria-expanded等の属性も付与されていません。
可能であれば、onFocus/onBlurでフォーカス状態も加味する、またはRadix Collapsibleが付与するdata-stateを利用してCSSで切り替える方が、ホバーとフォーカスで挙動が揃います。タッチデバイスでも同様の問題があります。
♻️ 提案: フォーカス状態もインジケータに反映
- const [isHovered, setIsHovered] = useState(false);
+ const [isFocusedOrHovered, setIsFocusedOrHovered] = useState(false);
@@
<CollapsibleTrigger asChild>
<button
...
- onMouseEnter={() => setIsHovered(true)}
- onMouseLeave={() => setIsHovered(false)}
+ onMouseEnter={() => setIsFocusedOrHovered(true)}
+ onMouseLeave={() => setIsFocusedOrHovered(false)}
+ onFocus={() => setIsFocusedOrHovered(true)}
+ onBlur={() => setIsFocusedOrHovered(false)}
type="button"
>
<div className="flex min-w-0 flex-1 items-center gap-1.5 text-xs">
- {isHovered && hasDetails ? (
+ {isFocusedOrHovered && hasDetails ? (📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| onMouseEnter={() => setIsHovered(true)} | |
| onMouseLeave={() => setIsHovered(false)} | |
| type="button" | |
| > | |
| <div className="flex min-w-0 flex-1 items-center gap-1.5 text-xs"> | |
| {isHovered && hasDetails ? ( | |
| isOpen ? ( | |
| <ChevronDownIcon className="h-3 w-3 shrink-0 text-muted-foreground" /> | |
| ) : ( | |
| <ChevronRightIcon className="h-3 w-3 shrink-0 text-muted-foreground" /> | |
| ) | |
| ) : isPending ? ( | |
| <span className="flex h-3 w-3 shrink-0 items-center justify-center overflow-hidden"> | |
| <BrailleSpinner /> | |
| </span> | |
| ) : ( | |
| <Icon className="h-3 w-3 shrink-0 text-muted-foreground" /> | |
| )} | |
| onMouseEnter={() => setIsFocusedOrHovered(true)} | |
| onMouseLeave={() => setIsFocusedOrHovered(false)} | |
| onFocus={() => setIsFocusedOrHovered(true)} | |
| onBlur={() => setIsFocusedOrHovered(false)} | |
| type="button" | |
| > | |
| <div className="flex min-w-0 flex-1 items-center gap-1.5 text-xs"> | |
| {isFocusedOrHovered && hasDetails ? ( | |
| isOpen ? ( | |
| <ChevronDownIcon className="h-3 w-3 shrink-0 text-muted-foreground" /> | |
| ) : ( | |
| <ChevronRightIcon className="h-3 w-3 shrink-0 text-muted-foreground" /> | |
| ) | |
| ) : isPending ? ( | |
| <span className="flex h-3 w-3 shrink-0 items-center justify-center overflow-hidden"> | |
| <BrailleSpinner /> | |
| </span> | |
| ) : ( | |
| <Icon className="h-3 w-3 shrink-0 text-muted-foreground" /> | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui/src/components/ai-elements/tool-call-row.tsx` around lines 118 -
135, The chevron visibility currently depends only on mouse hover
(setIsHovered/isHovered), so keyboard focus users and touch users don't see the
expand indicator; update the button to also track focus (add onFocus/onBlur
handlers to setIsHovered or a new isFocused state) and include aria-expanded
tied to isOpen, or instead rely on Radix Collapsible's data-state attribute in
the render/CSS to show ChevronDownIcon/ChevronRightIcon when expanded/collapsed;
adjust the conditional around Icon/ChevronDownIcon/ChevronRightIcon (referencing
isHovered, isFocused or data-state, hasDetails, isOpen, isPending) so
focus-visible shows the chevron the same way hover does and ensure aria-expanded
is present on the toggling element.
* fix v2 terminal lifecycle after sleep * add v2 terminal session dropdown * make terminal close kill sessions explicitly * show terminal session create time * refine terminal session dropdown behavior * connect multiple panes to terminal sessions * address terminal PR review comments * refresh terminal pane locations in dropdown actions
MocA-Love
force-pushed
the
upstream/batch-2026-04-25
branch
from
b71fbbb to
c3bfed0
Compare
Merged
10 tasks
MocA-Love
added a commit
that referenced
this pull request
Apr 25, 2026
upstream の 10 commits は #426 と #427 で fork 固有の差分を保ちながら個別に cherry-pick 取り込み済み。本 merge は ours strategy で **記録だけ** マージ済みに することで behind=0 を達成し、git 履歴上の追跡を正しくする。 Cherry-pick 取り込み済 (PR #426): - 5aab22a fix closed picker filters (superset-sh#3702) → cdb52f9 - 99db5be [codex] simplify workspace controls (superset-sh#3714) → f079606 - 186078a fix(chat): prevent ask_user question from shadowing sandbox access prompt (superset-sh#3662) → 09d6b57 - 47893c2 fix desktop workspace creation title clamp (superset-sh#3718) → 6a8c4ae - 09323ff Add diff pane file viewer action (superset-sh#3715) → 817ed8d - a5891c6 remove pending launch log (superset-sh#3721) → 0764d03 - c83de0c Add Tiptap table support (superset-sh#3719) → e67a885 - 486b621 [codex] Fix v2 terminal lifecycle after sleep (superset-sh#3711) → b71fbbb (+ #426 内 review fixups) Cherry-pick 取り込み済 (PR #427): - e07aef6 feat(desktop): play v2 notification hooks client-side (superset-sh#3675) → 27ac18a - eae6008 [codex] Port v2 terminal hotkeys to v1 (superset-sh#3724) → 05a77b8 (+ #427 内 Windows .ps1 v2 化) Fork 固有領域は変更ゼロで保持: 19 tRPC procedures (workspaces.githubExtended)、 AudioScheduler / Aivis TTS / notification-manager、terminal suggestion handler (新 terminalKeyboardHandler.ts に移植)、TERMINAL_OPTIONS、SUPERSET_WORKSPACE_NAME、 MainWindowEffects、INCEPTION_AUTH_PROVIDER_ID、v1MigrationState、TiptapPromptEditor、 electron-builder.ts (dmg.size="4g", fileAssociations)、Service Status Dashboard、 Linux daemon systemd、Worktree auto-sync、Windows support、DnD scratch route 他。
7 tasks
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
4 participants
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
upstream (
superset-sh/superset) maineae600874まで 10 commits ahead だったうち、fork 固有領域に大きく触れない 8 commits を本 PR で取り込む。残り 2 件 (v2 notification hooks / v2 terminal hotkeys) は fork の AudioScheduler/Aivis や terminal suggestion 拡張との統合作業が必要なため別 PR に分離。ベース: fork main
d6296a39b(vibrancy fix 系まで)取り込み先:
upstream/batch-2026-04-25→ main取り込み内容
5aab22a22cdb52f9ddis:open条件化99db5be26f07960630186078ae109d6b5785request_sandbox_accessをrequest_accessとして正規化、専用 ToolCall UI 追加47893c21d6a8c4aeaa09323ff56817ed8deea5891c68d0764d0310c83de0c16e67a8855d486b62114b71fbbb97別 PR 送り (本 PR では取り込まない)
e07aef637 feat(desktop): play v2 notification hooks client-side (#3675)— fork の AudioScheduler/Aivis (PR feat(desktop): 通知音と Aivis 音声の重なりを防ぐ AudioScheduler 導入 #405) との統合確認が必要eae600874 [codex] Port v2 terminal hotkeys to v1 (#3724)— fork の terminal suggestion handler を新terminalKeyboardHandler.tsへ移植する手作業が必要Fork 側のコンフリクト解決
99db5be26(superset-sh#3714) —V2WorkspacesList.tsxupstream は
SortableHeaderを使ったcolumnHeader行を導入したが、fork のV2WorkspacesListはsortField/sortDirection/handleSorthook を持たないためそのまま取り込めない。constants.tsの grid 列変更とV2WorkspaceRowの sidebar 操作位置変更は取り込み済み、列ヘッダーは fork 側に hook を足してから戻す方針で別 PR 化。empty判定は fork の!hasAnyMatches(フィルタ込み) を維持。FORK NOTE コメント追加。186078ae1(superset-sh#3662) —AskUserQuestionToolCall.tsx,service.ts,ToolCallBlock.tsxfork は AskUserQuestion 表示に
buildQuestionMarkdown+UserQuestionToolという独自実装を採用済み。upstream の専用 UI コンポーネント化は素直に取り込まず、fork 表示を維持しつつRequestSandboxAccessToolCall経路は新規取り込み。ToolCallBlock.tsxにはisInterruptedprops を追加してRequestSandboxAccessToolCallに propagate。817ed8dee(superset-sh#3715) —page.tsxupstream は
openFilePane内でworktreePathとfilePathを結合してabsoluteFilePathを計算してから pane を開く。fork のopenSidebarFilePaneには対応する変換ロジックがないため、3 箇所のabsoluteFilePath参照をfilePathに戻し。サイドバー path 絶対化は別 PR で fork 側にも導入予定。b71fbbb97(superset-sh#3711) —terminal-runtime-registry.ts,TerminalPane.tsx,terminal.ts,page.tsx,tool-call-row.tsx,braille-spinner.tsx,ToolCallBlock.tsx複数 fork integration:
terminal-runtime-registry.ts:mountにinstanceId引数追加 + fork debug log 維持、disposeEntryprivate 化を取り込みTerminalPane.tsx:terminalInstanceId = ctx.pane.id追加、リンクオープン時mode: "generic"をBrowserPaneDataに補完terminal.ts: 旧 fork のsession.socket?.readyStateを upstream のbroadcastMessageパターンに置き換え (multi-socket session 対応)packages/ui/src/components/ai-elements/tool-call-row.tsx+braille-spinner.tsx: upstream で新規追加された依存ファイルを fork の packages/ui に追加ToolCallBlock.tsx: agent が衝突解消で誤って追加したisStreamingprops を削除 (upstream はisInterruptedのみを追加)Fork 固有機能ヘルスチェック
workspaces.githubExtended)apps/desktop/src/main/lib/notifications/)drizzle migration
10 commits すべて
packages/db/drizzle//packages/local-db/drizzle/への変更なし → idx 衝突 / migration ファイル競合なし。Codex 事前調査・最終レビュー
/tmp/upstream-batch-2026-04-25/codex-preanalysis.md— 全 commitgit apply --check --3wayclean 判定 (実際は cherry-pick で 3 件 conflict 発生、本 PR で fork 寄り解消)Test plan
bun install --frozen --ignore-scripts— cleanbun run lint— exit 0 (rg not found warning は無視)bunx turbo typecheck --filter=@superset/desktop --filter=@superset/host-service --filter=@superset/chat— 3/3 passrequest_access(旧request_sandbox_access) UI 表示次の PR
e07aef637+eae600874を AudioScheduler/Aivis 統合 + terminal suggestion handler 移植して取り込む follow-up PRSummary by CodeRabbit
リリースノート
新機能
改善