Conversation
…rset-sh#3348) * feat(desktop): port v2 hide-attach xterm pattern to v1 terminal renderer Instead of destroying and recreating xterm instances on every tab switch, keep them alive in a module-level cache and detach/reattach the DOM wrapper. This eliminates the visible flash and scrollback restoration delay when switching between terminal tabs. - New v1-terminal-cache.ts: singleton Map<paneId, CachedTerminal> that stores xterm instances across React mount/unmount cycles - Extract createTerminalInWrapper from helpers.ts: opens xterm into a detached wrapper div instead of directly into the container - Modify useTerminalLifecycle mount: use cache getOrCreate + attachToContainer instead of createTerminalInstance; skip render gate on reattach - Modify useTerminalLifecycle unmount: detachFromContainer (keep alive) for tab switches, dispose (full cleanup) for pane destruction - Add reattach fast-path in createOrAttach onSuccess: skip scrollback writing since buffer is already in xterm memory * chore(desktop): add instrumentation to v1 terminal cache for testing Adds performance.now() timing and buffer state logging behind the existing DEBUG_TERMINAL flag to help verify the hide-attach behavior: - Cache: creation time, reattach count, cache size on each operation - attachToContainer: timing, cols/rows, renderer kind, buffer lines, scrollY - detachFromContainer: buffer state snapshot before detach - dispose: remaining cache size and lifetime stats - createOrAttach: timing breakdown (mount→attach start, attach duration) - Fresh vs reattach comparison logs Enable via: localStorage.setItem('SUPERSET_TERMINAL_DEBUG', '1') * chore(desktop): remove instrumentation from v1 terminal cache * feat(desktop): keep stream subscription alive in v1 terminal cache Move the tRPC stream subscription from the React component into the v1 terminal cache so data continues flowing to xterm even while the tab is hidden. On reattach, skip createOrAttach entirely since the backend session and stream are still alive. On unmount, skip the backend detach — only detach the DOM wrapper. * fix(desktop): stabilize layoutPaneIds to prevent terminal remount on resize extractPaneIdsFromLayout always returns a new array reference even when only splitPercentage values change (during split drag). This cascaded through tabPanes → renderPane → Mosaic, causing all terminal components to unmount/remount on every resize pixel. Stabilize the reference with a shallow comparison so it only changes when the actual set of pane IDs changes. * Revert "fix(desktop): stabilize layoutPaneIds to prevent terminal remount on resize" This reverts commit 918e8f0. * fix(desktop): remove v1 reattach recovery system The reattach recovery (clearTextureAtlas + fit + refresh + forced backend resize on every window focus/visibility change) was built for the old CSS-hidden terminal pattern. With the cache-based DOM detach/reattach approach, it's unnecessary — the ResizeObserver handles resize and attachToContainer does a single refresh on reattach. V2 has none of this machinery. * chore(desktop): clean up instrumentation from v1 terminal cache Gate all debug logs behind DEBUG_TERMINAL flag instead of always-on console.log calls. * Move resize * fix(desktop): align v1 terminal resize and renderer with v2 behavior - Move ResizeObserver into v1-terminal-cache (attach/detach lifecycle) instead of managing it in React component lifecycle - Remove duplicate fitAddon.fit() calls: reattach rAF, first-mount onSuccess rAF, and restore scheduleFitAndScroll - Remove setupResizeHandlers (debounced ResizeObserver + window resize listener) — replaced by undebounced ResizeObserver in cache - Remove RESIZE_DEBOUNCE_MS constant - Track lastCols/lastRows to skip no-op resize backend IPCs - Add hostIsVisible guard before initial fit (matches v2) - Remove clearTextureAtlas call on attach — v2 never does this - Replace v1's custom loadRenderer/TerminalRendererRef/localStorage renderer preference with v2's simple WebGL addon loading pattern - Remove dead createTerminalInstance convenience wrapper - Centralize all fit() calls in v1-terminal-cache (attach, resize, updateAppearance) — no fit calls outside the cache - Move font-change fit from Terminal.tsx into cache updateAppearance - Wrap Terminal component in React.memo to prevent re-renders during Mosaic split resize cascade - Remove restore fit from useTerminalRestore (keep only scrollToBottom) * fix(desktop): remove resizeRef.current from font effect deps Biome's lint:fix incorrectly added resizeRef.current to the useEffect dependency array. Since resizeRef is a MutableRefObject whose .current changes identity on every render, this caused the effect to fire repeatedly instead of only when fontSettings changed. * fix(desktop): address PR review findings in v1 terminal cache - Null out subscription on stream error so startStream() can reconnect - Fix resize guard to skip fit() when either dimension is zero (not both) - Notify backend PTY on reattach when dimensions changed while hidden - Move handler registration after useTerminalLifecycle and gate on xtermInstance so it runs after the cache entry exists on cold mount
) * feat(desktop): respect executionMode in V2 preset execution and wire preset hotkeys V2 presets previously ignored executionMode, always creating a single tab with commands joined by &&. Now split-pane, new-tab, and new-tab-split-pane modes work correctly, each command gets its own pane with initialCommand, and OPEN_PRESET_1-9 hotkeys are functional. * fix(desktop): address PR review — remove unused workspaceId param, dead code branch - Remove unused `workspaceId` from `UseV2PresetExecutionArgs` and call site - Remove unreachable `commands.length === 0` guard in `new-tab-per-command` case - Fix plan doc wording to match actual data flow
…#3356) * Gaps * Link support * feat(host-service,desktop): add PR URL parsing and cross-repo validation to V2 workspace modal Move GitHub PR URL detection, `#123` shorthand normalization, and cross-repo validation into the host-service `searchPullRequests` endpoint so the V2 client stays thin. The client sends raw user input and reacts to a `repoMismatch` field in the response. Also adds debounce gap handling to avoid empty-state flash while typing. * fix(host-service): use direct PR lookup for URL paste and #123 shorthand When query is a PR number (from URL parsing or `#` shorthand), use `octokit.pulls.get()` instead of `in:title` text search. The text search only matched if the number appeared in the PR title. * fix(host-service): direct PR lookup for numbers, broader text search, and tests - Extract normalizePullRequestQuery into its own module for testability - Use octokit.pulls.get() for bare numbers (e.g. "3130") not just #shorthand - Remove `in:title` from text search so it also matches PR body - Add 36 test cases covering: URL tabs (/files, /changes, /commits, /checks), query params, hash fragments, www prefix, http, cross-repo mismatch, #shorthand, bare numbers, non-PR URLs, GitHub Enterprise * feat(host-service,desktop): unify GitHub query normalization for PRs and issues Generalize normalizePullRequestQuery into normalizeGitHubQuery with a `kind` parameter ("pull" | "issue"). Single regex matches both /pull/:number and /issues/:number URLs. - Wire into searchGitHubIssues: URL paste, #N shorthand, bare number direct lookup via octokit.issues.get(), cross-repo validation - Filter out PRs from issues.get() (GitHub returns PRs as issues) - Remove redundant `in:title,body` from issue text search - Update GitHubIssueLinkCommand client: debounce gap handling, repoMismatch display - Cross-entity fallback: wrong URL kind falls through to text search - 50 tests covering PRs, issues, cross-entity, and edge cases * fix: address PR review feedback (items 1-4) 1. Delay ctx.github() until after repoMismatch short-circuit in both searchPullRequests and searchGitHubIssues — avoids auth errors when the query is a cross-repo URL 2. Lowercase urlPath before comparing entity kind — fixes case-sensitive mismatch when regex captures "PULL" or "Issues" from uppercase URLs 3. Use isFetching instead of isLoading from useQuery in both client components — correctly reflects background refetch state 4. Use debouncedTrimmed for issue list heading instead of raw searchQuery — prevents "Results" label on whitespace-only input * fix(desktop): match V1 issue search UX with client-side Fuse.js filtering V2 was sending every keystroke to the GitHub search API which was slow and couldn't match issue numbers reliably. V1 pre-fetches all open issues once and does instant client-side fuzzy search with Fuse.js (issue number weighted 3x, title weighted 2x). Now V2 does the same: pre-fetch all open issues on popover open, Fuse.js for text/number filtering, and only hits the server for URL paste and #N shorthand (which need cross-repo validation and direct lookup). * Revert "fix(desktop): match V1 issue search UX with client-side Fuse.js filtering" This reverts commit 86c6151. * fix(host-service): use search API for issue listing to avoid PR contamination The no-query path used octokit.issues.listForRepo() which returns PRs mixed with issues. With per_page: 30, most slots were consumed by PRs that got filtered out client-side, leaving very few actual issues. Switch to octokit.search.issuesAndPullRequests() with `is:issue is:open` so GitHub filters server-side and the full page is real issues. * refactor(host-service): collapse duplicate issue search paths into one query Both the text search and no-query paths were doing the same search API call with `is:issue is:open`. Merged into a single path that appends the effective query when present. * refactor(host-service): single search path for PRs, drop is:open from issues Collapse PR text search + no-query list into one search API call (same pattern as issues). Drop `is:open` from issue search so closed issues are findable — useful when linking context for workspace creation. Both endpoints now use one query: `repo:owner/name is:<type> <query>`. * fix(desktop): align PR and issue link command labels and limits - Drop stale "open" from empty states — search is no longer open-only - Issue limit 20 → 30 to match PRs - Issue heading shows result count like PRs - Both default headings say "Recent" instead of implying open-only - Consistent "Loading..." text * Lint
…perset-sh#3361) * Workspace creation * feat(host-service): add targeted fetch before worktree creation When resolveStartPoint resolves to an origin/* ref, fetch just that single branch before creating the worktree to ensure we branch from the latest remote state. Fails gracefully if offline or auth expired. Also updates the design doc with cross-product comparison (VS Code, T3Code, GitHub Desktop) and documents the fetch strategy. * Lint * test(host-service): co-locate resolve-start-point test + add HEAD fallback case Moves the test next to its source per AGENTS.md co-location convention and adds a missing test for the cold-start HEAD fallback path (no baseBranch provided, symbolic-ref fails, no default branch exists).
* Shell readiness
* feat(host-service): host-side initial command delivery via ensureSession
Add initialCommand support to createTerminalSessionInternal and
ensureSession tRPC. Commands are queued behind shellReadyPromise
(OSC 133;A detection from Phase 1) and written to the PTY only after
the shell is ready.
Update v2 preset execution to create sessions with commands on the
host-service before adding panes. Remove initialCommand from
TerminalPaneData and delete the renderer-side WebSocket delivery
effect — command delivery is now entirely host-service-side.
* feat(desktop): create setup terminal during workspace creation
During v2 workspace creation, if .superset/setup.sh exists and
runSetupScript is enabled, the host-service now creates a terminal
session with the setup command via createTerminalSessionInternal.
The command runs after shell readiness (OSC 133;A), and output
buffers until the renderer connects.
Returns terminals array [{id, role, label}] instead of initialCommands.
The pending page pre-populates the workspace pane layout with terminal
panes referencing the host-provided terminalIds before navigating, so
TerminalPane mounts and attaches to already-running sessions.
* refactor: extract OSC 133 scanner into @superset/shared, update v1
Extract the shell readiness scanner (character-by-character OSC 133;A
matching) into packages/shared/src/shell-ready-scanner.ts. Both v1
desktop (session.ts) and v2 host-service (terminal.ts) now import
from the shared module.
V1 switches from the custom OSC 777 marker to the standard OSC 133
protocol, matching the shell wrappers updated in the first commit.
All 19 v1 shell-ready tests pass with the new marker.
* Lint
* chore: remove dead SHELL_READY_MARKER constant, clean up stale comment
* Lint
* chore: remove dead initialCommands field from pendingWorkspaceSchema
* refactor(shell-wrappers): emit only OSC 133;A, drop C/D
We only consume 133;A for shell readiness detection. Emitting 133;C
and 133;D via DEBUG trap (bash) and preexec hooks (zsh/fish) brought
complexity we don't need — the bash DEBUG trap was also fundamentally
buggy, firing within PROMPT_COMMAND and producing spurious 133;C
emissions inside precmd.
Simplify to a single precmd hook per shell that emits 133;A. Matches
kitty's bash approach. If we ever need command-finished tracking, we
can add bash-preexec or PS0 (Bash 4.4+) at that point.
* fix: surface setup terminal errors and re-test marker on scanner mismatch
- Setup terminal creation failures now surface via the `warnings` array
instead of being silently dropped.
- Scanner re-tests the current character after a mismatch — a valid
OSC 133;A marker that immediately follows a stale ESC byte is no
longer lost.
* Lint
* fix(preset): show toast on executePreset failure
executePreset is async and can reject if ensureSession fails (network,
host down). Callers invoke as fire-and-forget — without a try/catch,
errors become unhandled promise rejections and the user sees the
preset silently not open. Wrap the body in try/catch with a toast.
…#3367) * feat(desktop): custom dev/canary dock icons with per-build-type resolution Replaces the workspace-name-colored border overlay with explicit icon files per build type. Resolution order: icon-dev.png → icon.png (dev), icon-canary.png → icon.png (canary), icon.png (stable). * feat(desktop): custom DMG installer background Adds support for a custom background.tiff in the DMG installer window. Falls back to electron-builder's default if the file is absent. * Update macOS tray icon asset * Update favicon-192.png asset for marketing application * Update title.svg with new updated logo design * Update superset-empty-state-wordmark.svg with new logo design and dimensions * feat(desktop): workspace-colored corner fold on dev dock icon Overlays a hash-colored triangle in the top-right corner of the dock icon in dev mode so multiple workspaces are visually distinguishable at a glance. The fill is clipped to the icon's alpha so it hugs the rounded corner. Replaces the previous rounded-rect border implementation. * lint --------- Co-authored-by: Michal Kopanski <info@michalkopanski.com>
* Port link detection * Fix pathing * Respect per-project editor preference for terminal file links
- Move 3 stray plan docs to repo-root `plans/` (V2 PR link design, normalize-github-query plan, voice agent plan) - Move 3 shipped plan docs from `apps/desktop/docs/` into `apps/desktop/plans/done/` - Mark Gap 7 resolved in V2_WORKSPACE_MODAL_GAPS.md (PR superset-sh#3356) - Add AGENTS.md rule #7 documenting plan vs doc placement
…h#3387) * fix(desktop): accept legacy OSC 777 marker in v1 terminal The persistent terminal-host daemon in v1 only restarts on protocol mismatch, so after the 133;A marker refactor, users can end up with a running daemon (or on-disk wrappers) that still emits the legacy `\x1b]777;superset-shell-ready\x07` while the new scanner only matches `\x1b]133;A\x07`. When the two halves disagree, shellReadyState sticks at `pending` and stdin stays buffered until the 15s timeout — the terminal looks ready but typing does nothing. Run a local legacy scanner alongside the shared OSC 133;A scanner in Session; either match resolves readiness. Scoped to v1 — the shared scanner and v2 host-service stay on 133;A only. * fix(desktop): shorten terminal shell-ready timeout to 5s 15s left the terminal feeling frozen when the marker was missing or the scanner disagreed with the wrapper. Slow shell startups (direnv, nix) can still race us past 5s, but unblocking sooner is less jarring than a 15s stall — worst case the user re-types. * Lint
…eamReady (superset-sh#3389) * Revert "fix(desktop): accept legacy OSC 777 marker in v1 terminal (superset-sh#3387)" This reverts commit ef67d8a. * fix(desktop): emit legacy OSC 777 marker alongside 133;A in shell wrappers The v1 terminal-host daemon is detached and survives app updates, only restarting on PROTOCOL_VERSION bumps. Shell wrappers, by contrast, are rewritten by setupAgentHooks() on every main-process start. After the 133;A marker refactor, prod users end up with fresh wrappers emitting 133;A while an old v1 daemon still scans only for OSC 777 — readiness never resolves and stdin stays buffered until the timeout. Emit both markers from zsh/bash/fish wrappers. Any daemon version finds its expected marker without needing a restart, so the fix reaches users immediately on the next app launch. Unknown OSC sequences are dropped silently by the emulator, so the extra marker is invisible in output. * fix(desktop): v1 terminal reattach gate must check streamReady, not cache existence When many tabs are created rapidly, React can remount a pane before its initial createOrAttach finishes. The unmount cancels the in-flight attach but intentionally preserves the v1 terminal cache entry so visible-tab switches reuse the xterm instance. On remount, the gate at line 237 saw a cache entry and took the reattach fast path — skipping createOrAttach and stream subscription setup — which left the pane permanently disconnected: no daemon session, no subscription, writes routed to a session that doesn't exist. Gate on the cache entry's streamReady flag instead. A cache entry with streamReady=false means the prior mount unmounted mid-attach, so the remount should take the normal attach path and re-issue createOrAttach with a fresh requestId. streamReady is only ever set by a successful attach (v1-terminal-cache.ts:setStreamReady) and never reset until dispose, so the predicate cleanly distinguishes "cache ready from a prior successful attach" from "cache exists but attach never completed". v1-only — v2 uses in-process host-service with a different lifecycle. * test(desktop): session resolves readiness on combined 777+133;A wrapper output Addresses review feedback on superset-sh#3389. The three removed legacy-scanner tests weren't replaced by a session-level test for the new wrapper output format (both markers printf'd back-to-back). Adds one. Guards against wrapper regressions: dropping 133;A (breaks current scanner), or any future change to the combined marker sequence that would prevent readiness detection. * Lint
superset-sh#3390) v1's keyboard handler returned false for every ctrl/meta combo, starving TUIs like Claude Code of Ctrl+P/Ctrl+O and shell readline shortcuts like Ctrl+R/W/U. Mirror v2's pattern (terminal-runtime.ts:21): use resolveHotkeyFromEvent to only bubble chords actually registered as app hotkeys, letting everything else reach the PTY. Fixes superset-sh#3385.
* feat(desktop): v2 diff viewer pane
Ports the diff viewer pane from saddlepaddle/diff-viewer onto the
current v2 workspace structure. Adds a new "diff" pane kind that opens
when a row in the Changes sidebar is clicked, renders a scrollable
multi-file diff via pierre's MultiFileDiff inside a Virtualizer, and
shares a pierre worker pool singleton across the workspace.
* feat(desktop): v2 diff viewer perf, header actions, and sidebar polish
Rewrites the diff pane data flow for perf on large changesets and adds
GitHub-style header actions. The sidebar Changes list is flattened and
picks up persistent "viewed" state shared with the diff pane.
- useChangeset + path-aware git:changed events replace useDiffPaneData;
per-file diff queries stay warm and invalidate only on worktree
changes that touch their path.
- DiffFileEntry lazy-mounts via IntersectionObserver with a sticky
"once near" ref; large diffs (>250 lines) render a GitHub-style
placeholder until explicitly expanded.
- DiffFileHeader adds Viewed / expand-unchanged / copy / revert. Copy
pulls newFile.contents from the cached diff query; revert routes
through electronTrpc.changes.discardChanges and is gated to
category="unstaged". Chevron collapses the entry body.
- Viewed state persists per workspace in workspaceLocalStateSchema.
Toggling Viewed also snaps collapsed to match (imperative sync at
the click site, not an effect).
- Sidebar Changes list is now a flat list with muted dir + bold
basename truncation, a Viewed checkbox per row, and viewed files
partitioned to the bottom.
* feat(desktop): diff header polish and sidebar co-location
- Open-in-editor chevron wired through electronTrpcClient (bypasses
the workspaceTrpc provider hijack); deleted files show a toast
instead of a disabled pill, other failures also toast.
- DiffFileHeader path pill now has border, rounded corners, hover
state, tooltip (300ms delay) "Open <path>", and a leading
StatusIndicator showing added/modified/deleted/renamed.
- StatusIndicator promoted to the workspace-level components/ so it's
shared between the sidebar Changes list and the diff header.
- ChangesFileList broken up per AGENTS.md: FileRow, StatusIndicator,
and partitionByViewed each live in their own folder. Shared
ChangedFile type comes from the existing useChangesTab/types.ts.
- useInView co-located under DiffFileEntry (only consumer).
* chore(desktop): clean up v2 diff viewer dead code and review nits
- Remove WorkerPoolDebug from PierreWorkerPoolProvider (was unconditionally
mounted, logging every stat change in prod).
- Drop pierre worker diagnostic console logs.
- Remove persisted-but-unread `expandUnchanged` from settings store.
- Remove unused `scrollTop` field on DiffPaneData.
- Remove unused ThreadAnnotation type + `annotations` prop from WorkspaceDiff.
- Trim useViewedFiles API to the two values that are actually consumed.
- useChangeset: forward `enabled` to `useWorkspaceEvent` so a disabled
subscription doesn't churn the query cache.
- git-watcher: when a worktree batch has no usable paths, emit a broad
invalidation event instead of `{ paths: [] }` which downstream could
read as a path-scoped no-op.
- ScrollToFile: only mark a path as handled after the scrollTo succeeds,
so a first-frame miss can still retry.
- DiffFileHeader: icon buttons now carry aria-labels; expand-unchanged
disables when no handler is passed (large-diff placeholder).
- LargeDiffPlaceholder passes undefined for onToggleExpandUnchanged
instead of a noop, matching the "dead control" fix above.
- WorkspaceDiff: discardChanges failures now surface as a toast.
* refactor(desktop): unify v2 changeset hook, fix commit/range + per-file category + baseBranch
Fixes three bugs from the PR review by collapsing the sidebar and diff
pane onto one file-list source.
Bugs fixed:
- Commit/range sidebar filters used to fall through to "against-base",
so clicking a file in "Commit abc123" showed the working-tree diff.
- Uncommitted list hard-coded fileCategory="unstaged" for every row,
so clicking a staged file fetched the wrong bucket (and the same
path appearing in both staged+unstaged caused a React key collision).
- useChangeset never forwarded sidebarState.baseBranch to getStatus,
so the against-base file list drifted from the sidebar's selection.
Structural cleanup folded in:
- useChangeset now takes a DiffRef discriminated union and branches
between git.getStatus and git.getCommitFiles internally. Each
returned file carries its own DiffFileSource, so downstream getDiff
calls always target the right bucket (uncommitted mode tags staged
vs unstaged per row; "All changes" mode tags dirty files with their
working-tree source and purely committed files with against-base).
- New useSidebarDiffRef hook is the single source of truth mapping
sidebarState.changesFilter + baseBranch -> DiffRef; consumed by both
DiffPane and useChangesTab, deleting the old useSidebarDiffCategory
binary ternary and the duplicate filteredFiles/commitFiles plumbing
in useChangesTab.
- DiffPaneData drops its dead `category` field (was re-derived from
sidebar on every render). openDiffPane loses its category arg.
- WorkspaceDiff takes a DiffFileSource prop and builds the getDiff
input from it (including commitHash/fromHash for commit-scoped
diffs).
- git.getDiff gains a "commit" category with optional commitHash /
fromHash (server branch mirrors getCommitFiles at the git.show level).
- Narrow ChangesetFile.status to the FileStatus union.
- Drop dead onClick stopPropagation from FileRow checkbox.
- Drop useChangeset `enabled` param (unused).
- Delete ChangeCategory type (replaced by DiffFileSource).
The pane now fully reflects current sidebar state. An open pane
showing foo.ts will re-render whenever the user switches the filter,
with the matching git refs on both sides of the diff.
* feat(desktop): unify deleted-file diff placeholder with large-diff placeholder
Deleted files now show the same "Show diff" affordance as large diffs,
gated on a single shared showFullDiff state. Renames LargeDiffPlaceholder
to DeferredDiffPlaceholder and switches its copy on a deferReason
("large" | "deleted"). Deleted placeholder uses a shorter (160px) frame
and drops the line-count subtitle.
* refactor(desktop): hoist Pierre worker pool to authenticated layout, bump size, memoize ref
- Move WorkerPoolContextProvider into the authenticated layout's
provider stack so the pool boots once per session and survives
every workspace switch (was per-workspaceId, paying ~1s cold start
on every switch).
- Delete the now-redundant PierreWorkerPoolProvider wrapper folder.
- Relocate createPierreWorker into _authenticated/lib/pierreWorker
next to its only consumer.
- Bump pool size 4 → 8 to better saturate modern multi-core machines
during initial paint of large multi-file diffs.
- Memoize useSidebarDiffRef on primitive deps so repeated renders
return a stable DiffRef object — defeats the previous identity
churn that was busting useChangeset's useMemo on every render.
…through host-service (superset-sh#3394) * fix(desktop): route font-settings query via imperative electron tRPC client @trpc/react-query falls back to a module-level singleton React context when createTRPCReact is called without an explicit context. In the v2 workspace, nesting <workspaceTrpc.Provider> overrides that singleton and silently routes electronTrpc.*.useQuery calls through the workspace HTTP link, producing "No procedure found on path settings.getFontSettings" against host-service. Switch the three v2 render paths (useTerminalAppearance, WorkspaceDiff, and v1 CodeEditor which is reused in v2's FilePane renderers) to use electronTrpcClient.settings.getFontSettings.query() wrapped in a plain @tanstack/react-query useQuery, matching the rest of v2. * chore(desktop): drop explanatory comments from font-settings workaround
…perset-sh#3393) * fix(desktop): v2 Open In editor is per-project, not per-workspace The v2 top-bar "Open In <editor>" dropdown was storing the choice in the per-workspace localStorage collection, so switching between workspaces in the same project reset the picker. Move it to the per-project v2SidebarProjects collection so the choice is shared across every workspace in a project. Also drops the unused workspaceId prop and no longer passes projectId to openInApp, since persistence now happens entirely client-side on the react-db collection. * fix(desktop): use ensureProjectInSidebar helper in V2OpenInMenuButton Avoid a phantom v2SidebarProjects row with hard-coded tabOrder: 0 when persisting the default Open In app. Delegate record creation to the canonical sidebar state helper, which computes tabOrder correctly.
…uperset-sh#3392) * fix(desktop): enable modal focus trap on v1 + v2 workspace dialogs Applies the same fix as superset-sh#3276 to both v1 (NewWorkspaceModal) and v2 (DashboardNewWorkspaceModal) Dialogs so alt-tabbing back restores focus to the modal. * Lint
superset-sh#3398) * feat(desktop): modifier+click in file tree opens external editor / new tab - Meta/Ctrl+click: open in external editor (works on files and folders) - Shift+click: open file in a new internal tab - Plain click: unchanged (preview pane / toggle folder) Adds the behavior to both v1 (FileTreeItem, FileSearchResultItem) and v2 (WorkspaceFilesTreeItem) file trees. v2 guards the external-editor path with an isLocalWorkspace check so remote hosts toast instead of silently calling local electron IPC against a path that does not exist. * fix(desktop): v2 file-tree meta+click reads defaultOpenInApp from v2SidebarProjects Align with the per-project v2 "Open In" pattern established in superset-sh#3393: resolve the editor from collections.v2SidebarProjects.defaultOpenInApp (falling back to finder) and call electronTrpc.external.openInApp — instead of openFileInEditor, which does v1-era backend resolution off the projects.defaultApp column that v2 doesn't write to. * fix(desktop): address PR review on v2 file-tree meta+click - Gate the isLocal check on the host live query actually resolving, so a meta+click before the collections query settles silently no-ops instead of toasting a misleading "only supported on local workspaces" error (cubic/greptile/coderabbit). - Use a separator-safe split when deriving the new-tab title from a file path so Windows host paths (backslash-separated) don't produce a full path as the tab title (cubic).
… tRPC batch (superset-sh#3400) The workspace tRPC client used httpBatchLink, which returns a batched response only when every procedure in the batch resolves. On workspace switch, FilesTab's filesystem.listDirectory (ms) was batched with WorkspaceSidebar's git.getStatus (5-10s on a large monorepo), so the file tree stalled behind git for every switch. Swap to httpBatchStreamLink so responses stream as each procedure completes, allow the trpc-accept header through CORS on the host service, and drop a now-unnecessary staleTime:0 override on listDirectory.fetch.
…3401) * feat(desktop): focus neighbor workspace after v2 delete When the active v2 workspace is deleted, navigate to the previous workspace in visual order, falling back to the next, then to / if no neighbors exist. Mirrors v1 behavior (without the wrap-around). Also switches the delete flow to toast.promise for feedback. * fix(desktop): address PR feedback on v2 delete focus - Isolate navigation from toast.promise so a router rejection after a successful delete doesn't surface a misleading "Failed to delete" toast (greptile). - Break sort ties in getFlattenedV2WorkspaceIds with section-before- workspace to match the sidebar's ordering when tabOrder collides (cubic).
…on, terminal override respect (superset-sh#3391) * attempt fix * Test and doc * fix(desktop): terminal & migration respect hotkey overrides - Rebuild the hotkey reverse index on override store changes so the terminal forwards the user's current bindings instead of frozen defaults. Fixes swallowed-keystroke on rebound-away defaults and dead-binding on new overrides. - Sanitize migrated overrides: canonicalize and drop malformed strings (`ctrl+control`, `ctrl+shift+@`, `meta+[`) that the pre-fix recorder could produce. - Document the meta-on-non-Mac policy (Windows OS intercept, Linux WM ownership). * feat(desktop): allow meta (Win/Super) bindings on non-Mac with OS warning Drop the blanket reject and extend OS_RESERVED for Windows shell intercepts (meta+d/e/l/r/tab) so users get a warning instead of a silent block. Linux WM configs vary too much to predict — trust the user and let them rebind if a chord doesn't fire. * refactor(desktop): unify event↔chord matching via shared helpers Audit against react-hotkeys-hook and internal usages found two more consumers comparing via event.key, which breaks the same punctuation / layout / rebind cases fixed in the recorder: - utils/utils.ts isTerminalReservedEvent had its own event.key-based TERMINAL_RESERVED set with ctrl+\\. - Terminal/helpers.ts matchesKey used event.key to check the CLEAR_TERMINAL rebind — silently wrong for any punctuation rebind. Consolidation: - Export eventToChord and new matchesChord(event, chord) as the single canonical event↔chord matcher. - Export TERMINAL_RESERVED_CHORDS as the single source of truth. - Rewrite isTerminalReservedEvent around them. - Replace matchesKey() with matchesChord(). - Remove duplicated TERMINAL_RESERVED from useRecordHotkeys.ts. Also adds tests for eventToChord, matchesChord, and isTerminalReservedEvent parity. Updates plan doc with the cleanup. * docs(desktop): rewrite hotkey fix plan for concise review * chore(desktop/hotkeys): deslop — tighten comments, remove dead wrappers - Merge canonicalizeChord / normalizeChord into one exported function (the wrapper was pointless indirection). - Drop section-banner comments (`// Helpers`, `// Hook`) and comments that restated code (`// Must include ctrl or meta…`). - Tighten JSDocs to convey intent in one line where possible. - display.ts: drop duplicate short arrow entries (`up: "↑"`) — the normalizeToken call aliases them to canonical names already. Extract isModifier type guard to kill the cast repetition. Net -35 LOC, same behavior, same 62 tests passing. * fix(desktop/hotkeys): canonicalize reserved-chord tables + review cleanups Addresses PR review feedback: - Bug: OS_RESERVED[\"windows\"] had \"ctrl+alt+delete\" which never matched because canonicalization sorts modifiers alphabetically. Wrap both OS_RESERVED and TERMINAL_RESERVED_CHORDS in .map(canonicalizeChord) at build time so multi-modifier entries can't silently miss the reserved warning. OS_RESERVED values switched from array to Set. - Extract sanitizeOverride into utils/sanitizeOverride.ts so the test imports the real implementation instead of a near-copy. - Test stub: preserve explicit \`code: undefined\` so the synthetic-event guard in captureHotkeyFromEvent is actually exercised (not the empty-string branch). - Resolver tests: resolve a sample hotkey once at describe scope and throw if HOTKEYS has no bound defaults, instead of each test silently no-op-ing via \`if (!def) return\`. - Add regression test asserting canonicalizeChord(\"ctrl+alt+delete\") sorts to \"alt+ctrl+delete\". * fix(desktop/hotkeys): re-sanitize localStorage for users who migrated pre-fix The original guard short-circuited whenever \`hotkey-overrides\` existed in localStorage, which meant any user who ran the migration before the Bug 5 sanitizer shipped would keep their corrupt entries (\`ctrl+control\`, \`ctrl+shift+@\`, \`meta+[\`) forever. Gate migration/sanitization on a separate \`hotkey-overrides-sanitized-v1\` marker instead: - No marker + no store → tRPC migration with sanitizer. - No marker + has store → in-place re-sanitization of existing entries. - Marker present → done, skip. The marker is only set once per user; new bindings written by the fixed recorder can't be corrupt so no further passes are needed. * fix(desktop/hotkeys): bump migration marker key so pre-sanitizer users re-migrate Use a dedicated `hotkey-overrides-migrated-v2` marker separate from the Zustand persist key. Users who ran the migration on the pre-sanitizer build (~1 day window) will re-run once and get their corrupt entries (`ctrl+control`, `ctrl+shift+@`, `meta+[`) dropped. Mark set only on success paths (or when there's nothing to migrate), not in the catch branch — transient tRPC failures at boot retry next launch instead of silently giving up on the user's legacy bindings.
- Move 3 stray plan docs to repo-root `plans/` (V2 PR link design, normalize-github-query plan, voice agent plan) - Move 3 shipped plan docs from `apps/desktop/docs/` into `apps/desktop/plans/done/` - Mark Gap 7 resolved in V2_WORKSPACE_MODAL_GAPS.md (PR superset-sh#3356) - Add AGENTS.md rule #7 documenting plan vs doc placement
…rset-sh#3348) * feat(desktop): port v2 hide-attach xterm pattern to v1 terminal renderer Instead of destroying and recreating xterm instances on every tab switch, keep them alive in a module-level cache and detach/reattach the DOM wrapper. This eliminates the visible flash and scrollback restoration delay when switching between terminal tabs. - New v1-terminal-cache.ts: singleton Map<paneId, CachedTerminal> that stores xterm instances across React mount/unmount cycles - Extract createTerminalInWrapper from helpers.ts: opens xterm into a detached wrapper div instead of directly into the container - Modify useTerminalLifecycle mount: use cache getOrCreate + attachToContainer instead of createTerminalInstance; skip render gate on reattach - Modify useTerminalLifecycle unmount: detachFromContainer (keep alive) for tab switches, dispose (full cleanup) for pane destruction - Add reattach fast-path in createOrAttach onSuccess: skip scrollback writing since buffer is already in xterm memory * chore(desktop): add instrumentation to v1 terminal cache for testing Adds performance.now() timing and buffer state logging behind the existing DEBUG_TERMINAL flag to help verify the hide-attach behavior: - Cache: creation time, reattach count, cache size on each operation - attachToContainer: timing, cols/rows, renderer kind, buffer lines, scrollY - detachFromContainer: buffer state snapshot before detach - dispose: remaining cache size and lifetime stats - createOrAttach: timing breakdown (mount→attach start, attach duration) - Fresh vs reattach comparison logs Enable via: localStorage.setItem('SUPERSET_TERMINAL_DEBUG', '1') * chore(desktop): remove instrumentation from v1 terminal cache * feat(desktop): keep stream subscription alive in v1 terminal cache Move the tRPC stream subscription from the React component into the v1 terminal cache so data continues flowing to xterm even while the tab is hidden. On reattach, skip createOrAttach entirely since the backend session and stream are still alive. On unmount, skip the backend detach — only detach the DOM wrapper. * fix(desktop): stabilize layoutPaneIds to prevent terminal remount on resize extractPaneIdsFromLayout always returns a new array reference even when only splitPercentage values change (during split drag). This cascaded through tabPanes → renderPane → Mosaic, causing all terminal components to unmount/remount on every resize pixel. Stabilize the reference with a shallow comparison so it only changes when the actual set of pane IDs changes. * Revert "fix(desktop): stabilize layoutPaneIds to prevent terminal remount on resize" This reverts commit 918e8f0. * fix(desktop): remove v1 reattach recovery system The reattach recovery (clearTextureAtlas + fit + refresh + forced backend resize on every window focus/visibility change) was built for the old CSS-hidden terminal pattern. With the cache-based DOM detach/reattach approach, it's unnecessary — the ResizeObserver handles resize and attachToContainer does a single refresh on reattach. V2 has none of this machinery. * chore(desktop): clean up instrumentation from v1 terminal cache Gate all debug logs behind DEBUG_TERMINAL flag instead of always-on console.log calls. * Move resize * fix(desktop): align v1 terminal resize and renderer with v2 behavior - Move ResizeObserver into v1-terminal-cache (attach/detach lifecycle) instead of managing it in React component lifecycle - Remove duplicate fitAddon.fit() calls: reattach rAF, first-mount onSuccess rAF, and restore scheduleFitAndScroll - Remove setupResizeHandlers (debounced ResizeObserver + window resize listener) — replaced by undebounced ResizeObserver in cache - Remove RESIZE_DEBOUNCE_MS constant - Track lastCols/lastRows to skip no-op resize backend IPCs - Add hostIsVisible guard before initial fit (matches v2) - Remove clearTextureAtlas call on attach — v2 never does this - Replace v1's custom loadRenderer/TerminalRendererRef/localStorage renderer preference with v2's simple WebGL addon loading pattern - Remove dead createTerminalInstance convenience wrapper - Centralize all fit() calls in v1-terminal-cache (attach, resize, updateAppearance) — no fit calls outside the cache - Move font-change fit from Terminal.tsx into cache updateAppearance - Wrap Terminal component in React.memo to prevent re-renders during Mosaic split resize cascade - Remove restore fit from useTerminalRestore (keep only scrollToBottom) * fix(desktop): remove resizeRef.current from font effect deps Biome's lint:fix incorrectly added resizeRef.current to the useEffect dependency array. Since resizeRef is a MutableRefObject whose .current changes identity on every render, this caused the effect to fire repeatedly instead of only when fontSettings changed. * fix(desktop): address PR review findings in v1 terminal cache - Null out subscription on stream error so startStream() can reconnect - Fix resize guard to skip fit() when either dimension is zero (not both) - Notify backend PTY on reattach when dimensions changed while hidden - Move handler registration after useTerminalLifecycle and gate on xtermInstance so it runs after the cache entry exists on cold mount
…#3356) * Gaps * Link support * feat(host-service,desktop): add PR URL parsing and cross-repo validation to V2 workspace modal Move GitHub PR URL detection, `#123` shorthand normalization, and cross-repo validation into the host-service `searchPullRequests` endpoint so the V2 client stays thin. The client sends raw user input and reacts to a `repoMismatch` field in the response. Also adds debounce gap handling to avoid empty-state flash while typing. * fix(host-service): use direct PR lookup for URL paste and #123 shorthand When query is a PR number (from URL parsing or `#` shorthand), use `octokit.pulls.get()` instead of `in:title` text search. The text search only matched if the number appeared in the PR title. * fix(host-service): direct PR lookup for numbers, broader text search, and tests - Extract normalizePullRequestQuery into its own module for testability - Use octokit.pulls.get() for bare numbers (e.g. "3130") not just #shorthand - Remove `in:title` from text search so it also matches PR body - Add 36 test cases covering: URL tabs (/files, /changes, /commits, /checks), query params, hash fragments, www prefix, http, cross-repo mismatch, #shorthand, bare numbers, non-PR URLs, GitHub Enterprise * feat(host-service,desktop): unify GitHub query normalization for PRs and issues Generalize normalizePullRequestQuery into normalizeGitHubQuery with a `kind` parameter ("pull" | "issue"). Single regex matches both /pull/:number and /issues/:number URLs. - Wire into searchGitHubIssues: URL paste, #N shorthand, bare number direct lookup via octokit.issues.get(), cross-repo validation - Filter out PRs from issues.get() (GitHub returns PRs as issues) - Remove redundant `in:title,body` from issue text search - Update GitHubIssueLinkCommand client: debounce gap handling, repoMismatch display - Cross-entity fallback: wrong URL kind falls through to text search - 50 tests covering PRs, issues, cross-entity, and edge cases * fix: address PR review feedback (items 1-4) 1. Delay ctx.github() until after repoMismatch short-circuit in both searchPullRequests and searchGitHubIssues — avoids auth errors when the query is a cross-repo URL 2. Lowercase urlPath before comparing entity kind — fixes case-sensitive mismatch when regex captures "PULL" or "Issues" from uppercase URLs 3. Use isFetching instead of isLoading from useQuery in both client components — correctly reflects background refetch state 4. Use debouncedTrimmed for issue list heading instead of raw searchQuery — prevents "Results" label on whitespace-only input * fix(desktop): match V1 issue search UX with client-side Fuse.js filtering V2 was sending every keystroke to the GitHub search API which was slow and couldn't match issue numbers reliably. V1 pre-fetches all open issues once and does instant client-side fuzzy search with Fuse.js (issue number weighted 3x, title weighted 2x). Now V2 does the same: pre-fetch all open issues on popover open, Fuse.js for text/number filtering, and only hits the server for URL paste and #N shorthand (which need cross-repo validation and direct lookup). * Revert "fix(desktop): match V1 issue search UX with client-side Fuse.js filtering" This reverts commit 86c6151. * fix(host-service): use search API for issue listing to avoid PR contamination The no-query path used octokit.issues.listForRepo() which returns PRs mixed with issues. With per_page: 30, most slots were consumed by PRs that got filtered out client-side, leaving very few actual issues. Switch to octokit.search.issuesAndPullRequests() with `is:issue is:open` so GitHub filters server-side and the full page is real issues. * refactor(host-service): collapse duplicate issue search paths into one query Both the text search and no-query paths were doing the same search API call with `is:issue is:open`. Merged into a single path that appends the effective query when present. * refactor(host-service): single search path for PRs, drop is:open from issues Collapse PR text search + no-query list into one search API call (same pattern as issues). Drop `is:open` from issue search so closed issues are findable — useful when linking context for workspace creation. Both endpoints now use one query: `repo:owner/name is:<type> <query>`. * fix(desktop): align PR and issue link command labels and limits - Drop stale "open" from empty states — search is no longer open-only - Issue limit 20 → 30 to match PRs - Issue heading shows result count like PRs - Both default headings say "Recent" instead of implying open-only - Consistent "Loading..." text * Lint
…perset-sh#3361) * Workspace creation * feat(host-service): add targeted fetch before worktree creation When resolveStartPoint resolves to an origin/* ref, fetch just that single branch before creating the worktree to ensure we branch from the latest remote state. Fails gracefully if offline or auth expired. Also updates the design doc with cross-product comparison (VS Code, T3Code, GitHub Desktop) and documents the fetch strategy. * Lint * test(host-service): co-locate resolve-start-point test + add HEAD fallback case Moves the test next to its source per AGENTS.md co-location convention and adds a missing test for the cold-start HEAD fallback path (no baseBranch provided, symbolic-ref fails, no default branch exists).
…#3367) * feat(desktop): custom dev/canary dock icons with per-build-type resolution Replaces the workspace-name-colored border overlay with explicit icon files per build type. Resolution order: icon-dev.png → icon.png (dev), icon-canary.png → icon.png (canary), icon.png (stable). * feat(desktop): custom DMG installer background Adds support for a custom background.tiff in the DMG installer window. Falls back to electron-builder's default if the file is absent. * Update macOS tray icon asset * Update favicon-192.png asset for marketing application * Update title.svg with new updated logo design * Update superset-empty-state-wordmark.svg with new logo design and dimensions * feat(desktop): workspace-colored corner fold on dev dock icon Overlays a hash-colored triangle in the top-right corner of the dock icon in dev mode so multiple workspaces are visually distinguishable at a glance. The fill is clipped to the icon's alpha so it hugs the rounded corner. Replaces the previous rounded-rect border implementation. * lint --------- Co-authored-by: Michal Kopanski <info@michalkopanski.com>
* Port link detection * Fix pathing * Respect per-project editor preference for terminal file links
…perset-sh#3393) * fix(desktop): v2 Open In editor is per-project, not per-workspace The v2 top-bar "Open In <editor>" dropdown was storing the choice in the per-workspace localStorage collection, so switching between workspaces in the same project reset the picker. Move it to the per-project v2SidebarProjects collection so the choice is shared across every workspace in a project. Also drops the unused workspaceId prop and no longer passes projectId to openInApp, since persistence now happens entirely client-side on the react-db collection. * fix(desktop): use ensureProjectInSidebar helper in V2OpenInMenuButton Avoid a phantom v2SidebarProjects row with hard-coded tabOrder: 0 when persisting the default Open In app. Delegate record creation to the canonical sidebar state helper, which computes tabOrder correctly.
superset-sh#3398) * feat(desktop): modifier+click in file tree opens external editor / new tab - Meta/Ctrl+click: open in external editor (works on files and folders) - Shift+click: open file in a new internal tab - Plain click: unchanged (preview pane / toggle folder) Adds the behavior to both v1 (FileTreeItem, FileSearchResultItem) and v2 (WorkspaceFilesTreeItem) file trees. v2 guards the external-editor path with an isLocalWorkspace check so remote hosts toast instead of silently calling local electron IPC against a path that does not exist. * fix(desktop): v2 file-tree meta+click reads defaultOpenInApp from v2SidebarProjects Align with the per-project v2 "Open In" pattern established in superset-sh#3393: resolve the editor from collections.v2SidebarProjects.defaultOpenInApp (falling back to finder) and call electronTrpc.external.openInApp — instead of openFileInEditor, which does v1-era backend resolution off the projects.defaultApp column that v2 doesn't write to. * fix(desktop): address PR review on v2 file-tree meta+click - Gate the isLocal check on the host live query actually resolving, so a meta+click before the collections query settles silently no-ops instead of toasting a misleading "only supported on local workspaces" error (cubic/greptile/coderabbit). - Use a separator-safe split when deriving the new-tab title from a file path so Windows host paths (backslash-separated) don't produce a full path as the tab title (cubic).
…on, terminal override respect (superset-sh#3391) * attempt fix * Test and doc * fix(desktop): terminal & migration respect hotkey overrides - Rebuild the hotkey reverse index on override store changes so the terminal forwards the user's current bindings instead of frozen defaults. Fixes swallowed-keystroke on rebound-away defaults and dead-binding on new overrides. - Sanitize migrated overrides: canonicalize and drop malformed strings (`ctrl+control`, `ctrl+shift+@`, `meta+[`) that the pre-fix recorder could produce. - Document the meta-on-non-Mac policy (Windows OS intercept, Linux WM ownership). * feat(desktop): allow meta (Win/Super) bindings on non-Mac with OS warning Drop the blanket reject and extend OS_RESERVED for Windows shell intercepts (meta+d/e/l/r/tab) so users get a warning instead of a silent block. Linux WM configs vary too much to predict — trust the user and let them rebind if a chord doesn't fire. * refactor(desktop): unify event↔chord matching via shared helpers Audit against react-hotkeys-hook and internal usages found two more consumers comparing via event.key, which breaks the same punctuation / layout / rebind cases fixed in the recorder: - utils/utils.ts isTerminalReservedEvent had its own event.key-based TERMINAL_RESERVED set with ctrl+\\. - Terminal/helpers.ts matchesKey used event.key to check the CLEAR_TERMINAL rebind — silently wrong for any punctuation rebind. Consolidation: - Export eventToChord and new matchesChord(event, chord) as the single canonical event↔chord matcher. - Export TERMINAL_RESERVED_CHORDS as the single source of truth. - Rewrite isTerminalReservedEvent around them. - Replace matchesKey() with matchesChord(). - Remove duplicated TERMINAL_RESERVED from useRecordHotkeys.ts. Also adds tests for eventToChord, matchesChord, and isTerminalReservedEvent parity. Updates plan doc with the cleanup. * docs(desktop): rewrite hotkey fix plan for concise review * chore(desktop/hotkeys): deslop — tighten comments, remove dead wrappers - Merge canonicalizeChord / normalizeChord into one exported function (the wrapper was pointless indirection). - Drop section-banner comments (`// Helpers`, `// Hook`) and comments that restated code (`// Must include ctrl or meta…`). - Tighten JSDocs to convey intent in one line where possible. - display.ts: drop duplicate short arrow entries (`up: "↑"`) — the normalizeToken call aliases them to canonical names already. Extract isModifier type guard to kill the cast repetition. Net -35 LOC, same behavior, same 62 tests passing. * fix(desktop/hotkeys): canonicalize reserved-chord tables + review cleanups Addresses PR review feedback: - Bug: OS_RESERVED[\"windows\"] had \"ctrl+alt+delete\" which never matched because canonicalization sorts modifiers alphabetically. Wrap both OS_RESERVED and TERMINAL_RESERVED_CHORDS in .map(canonicalizeChord) at build time so multi-modifier entries can't silently miss the reserved warning. OS_RESERVED values switched from array to Set. - Extract sanitizeOverride into utils/sanitizeOverride.ts so the test imports the real implementation instead of a near-copy. - Test stub: preserve explicit \`code: undefined\` so the synthetic-event guard in captureHotkeyFromEvent is actually exercised (not the empty-string branch). - Resolver tests: resolve a sample hotkey once at describe scope and throw if HOTKEYS has no bound defaults, instead of each test silently no-op-ing via \`if (!def) return\`. - Add regression test asserting canonicalizeChord(\"ctrl+alt+delete\") sorts to \"alt+ctrl+delete\". * fix(desktop/hotkeys): re-sanitize localStorage for users who migrated pre-fix The original guard short-circuited whenever \`hotkey-overrides\` existed in localStorage, which meant any user who ran the migration before the Bug 5 sanitizer shipped would keep their corrupt entries (\`ctrl+control\`, \`ctrl+shift+@\`, \`meta+[\`) forever. Gate migration/sanitization on a separate \`hotkey-overrides-sanitized-v1\` marker instead: - No marker + no store → tRPC migration with sanitizer. - No marker + has store → in-place re-sanitization of existing entries. - Marker present → done, skip. The marker is only set once per user; new bindings written by the fixed recorder can't be corrupt so no further passes are needed. * fix(desktop/hotkeys): bump migration marker key so pre-sanitizer users re-migrate Use a dedicated `hotkey-overrides-migrated-v2` marker separate from the Zustand persist key. Users who ran the migration on the pre-sanitizer build (~1 day window) will re-run once and get their corrupt entries (`ctrl+control`, `ctrl+shift+@`, `meta+[`) dropped. Mark set only on success paths (or when there's nothing to migrate), not in the catch branch — transient tRPC failures at boot retry next launch instead of silently giving up on the user's legacy bindings.
|
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 22 minutes and 10 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 (4)
📝 WalkthroughWalkthroughこのPRは、ホットキーシステムの統合、V2ワークスペース機能の拡張（プリセット実行・差分ビューア）、V1ターミナルキャッシュの導入、GitHub PR/Issue検索の正規化、シェルプロンプト検出（OSC 133）への切り替え、ワークスペース作成フローの更新を含む包括的なデスクトップ機能拡張です。 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Client as クライアント<br/>(V2Workspace)
participant PresetHook as useV2PresetExecution
participant Store as WorkspaceStore
participant API as TRPC API
participant Terminal as ターミナル
User->>Client: プリセット1キー押下
Client->>PresetHook: executePreset(preset)
PresetHook->>API: ensureSession(initialCommand)
API->>Terminal: セッション作成
Terminal-->>API: sessionId返却
API-->>PresetHook: sessionId返却
PresetHook->>Store: addPane/addTab (ペーンレイアウト)
Store-->>Client: UI更新
Client->>Terminal: ターミナル接続
Terminal-->>Client: 初期コマンド実行
sequenceDiagram
participant User
participant UI as DiffFileEntry
participant Cache as v1-terminal-cache
participant Stream as WebSocket
participant PTY as PTYサブプロセス
User->>UI: ファイル変更を表示
UI->>Cache: getOrCreate(paneId)
alt キャッシュに存在
Cache-->>UI: 既存xterm インスタンス
else 新規作成
Cache->>Cache: createTerminalInWrapper()
Cache->>Stream: 初期化
end
UI->>Cache: attachToContainer(paneId)
Cache->>Cache: ResizeObserver開始
Cache->>Stream: startStream()
Stream->>PTY: ターミナルストリーム開始
PTY-->>Stream: 出力データ
Stream-->>UI: ターミナル表示
sequenceDiagram
participant User
participant GitHubLinkCommand as GitHubLinkCommand
participant Normalizer as normalizeGitHubQuery
participant API as TRPC API
participant GitHub as GitHub API
User->>GitHubLinkCommand: PR URLを貼り付け
GitHubLinkCommand->>Normalizer: normalizeGitHubQuery(url, repo, "pull")
alt リポジトリ一致
Normalizer-->>GitHubLinkCommand: {query: "123", repoMismatch: false}
GitHubLinkCommand->>API: searchPullRequests(query: "123")
else リポジトリミスマッチ
Normalizer-->>GitHubLinkCommand: {query: "", repoMismatch: true}
GitHubLinkCommand->>User: "PR URLはowner/repoと一致する必要があります"
end
API->>GitHub: GitHub検索リクエスト
GitHub-->>API: PR結果
API-->>GitHubLinkCommand: PR詳細
GitHubLinkCommand->>User: PR一覧表示
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![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: 8475e4cea1
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/hooks/useTerminalRestore.ts (1)
131-163:⚠️ Potential issue | 🟠 Major新規セッションにも復元スナップショットを流し込んでしまっています。
result.isNewのスキップが消えたことで、fresh な attach でもsnapshotAnsi/snapshot.cwd/ mode flags が適用されます。CreateOrAttachResultではisNewとisColdRestoreは別フラグですし、useTerminalColdRestore.tsも cold restore だけを復元扱いにしているので、ここは新規セッションでは snapshot 由来の状態を無視しないと、空のはずの端末に古い scrollback や CWD が出ます。💡 修正例
try { + if (result.isNew && !result.isColdRestore) { + isAlternateScreenRef.current = false; + isBracketedPasteRef.current = false; + modeScanBufferRef.current = ""; + isStreamReadyRef.current = true; + flushPendingEvents(); + return; + } + const scheduleScrollToBottom = () => { requestAnimationFrame(() => { if (xtermRef.current !== xterm) return;Also applies to: 225-229
🤖 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/ContentView/TabsContent/Terminal/hooks/useTerminalRestore.ts` around lines 131 - 163, The code is applying snapshot data for fresh attaches; guard snapshot-based restores so they only run for non-new sessions: wrap the snapshot parsing and assignments (the initialAnsi selection, the mode/ref assignments like isAlternateScreenRef.current, isBracketedPasteRef.current, modeScanBufferRef.current, and the isAltScreenReattach computation which reads result.snapshot?.modes.alternateScreen) with a check that result.isNew is false (or early-return when result.isNew), so new sessions ignore snapshotAnsi/snapshot.cwd/mode flags; apply the same guard in the other occurrence noted (around the block at the later occurrence ~225-229) to match useTerminalColdRestore behavior.apps/desktop/src/renderer/hotkeys/utils/resolveHotkeyFromEvent.ts (1)
55-67:⚠️ Potential issue | 🟠 Major
canonicalizeChordがMetaLeft/ControlRightを正規化できていません。ここで先に
toLowerCase()しているせいで、CODE_ALIASESのMetaLeft/ControlLeftなどが一度もヒットしません。event.code由来の文字列を保存・移行した override はmeta/ctrlに潰れず、eventToChord()と一致しなくなります。💡 修正例
export function canonicalizeChord(chord: string): string { - const parts = chord.toLowerCase().split("+").map(normalizeToken); + const parts = chord.split("+").map(normalizeToken); const mods: string[] = []; const keys: string[] = [];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/hotkeys/utils/resolveHotkeyFromEvent.ts` around lines 55 - 67, canonicalizeChord lowercases the entire chord before alias lookup which prevents CODE_ALIASES like "MetaLeft"/"ControlRight" from matching; change it to split the chord by "+" and call normalizeToken on each raw token (do not toLowerCase the token before normalizeToken), then use a lowercased version only for modifier detection (e.g. const lower = token.toLowerCase(); if (MODIFIERS.has(lower)) { mods.push(lower === "control" ? "ctrl" : lower) } else { keys.push(normalizedToken) }). Keep references to canonicalizeChord, normalizeToken, and MODIFIERS and ensure the resulting ordering/joining matches eventToChord.apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/components/PRLinkCommand/PRLinkCommand.tsx (1)
130-149:⚠️ Potential issue | 🟠 Majorデバウンス中は旧クエリの候補を隠してください。
isLoadingは空状態の文言にしか効いていないので、前回の検索結果がある状態で入力を続けると、その候補が 300ms の間そのまま選択可能です。Enter で別の PR を誤リンクできるため、isPendingDebounce || isFetchingの間は結果一覧を描画しないか無効化した方が安全です。💡 修正案
- {pullRequests.length === 0 && ( + {(isLoading || pullRequests.length === 0) && ( <CommandEmpty> {isLoading ? debouncedTrimmed ? "Searching..." : "Loading..." : repoMismatch ? `PR URL must match ${repoMismatch}.` : debouncedTrimmed ? "No pull requests found." : "No pull requests found."} </CommandEmpty> )} - {pullRequests.length > 0 && ( + {!isLoading && pullRequests.length > 0 && ( <CommandGroup🤖 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 130 - 149, The current UI still renders previous pullRequests while typing because isLoading only controls the empty-state text; update the render logic around pullRequests/CommandGroup/CommandEmpty so that when isPendingDebounce || isFetching is true you do not render (or render a disabled/placeholder) the previous results: treat isPendingDebounce || isFetching the same as an empty result by using that combined flag in the conditional that shows CommandGroup and CommandEmpty (refer to pullRequests, isLoading, isPendingDebounce, isFetching, CommandGroup, CommandEmpty) so previous suggestions are hidden/disabled during debounce/fetch.
🟡 Minor comments (8)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/V2OpenInMenuButton/V2OpenInMenuButton.tsx-48-49 (1)
48-49:⚠️ Potential issue | 🟡 Minor永続化値は検証してから
resolvedAppに採用した方が安全です。
defaultOpenInAppを無検証で使うと、古い値や未知の値が残っている端末でcurrentAppはnullになる一方、OPEN_IN_APPホットキー経由ではその無効値をopenInAppに流せます。既知の app 一覧で一度検証して、未知値は安全な既定値に落としてください。🤖 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/components/TopBar/components/V2OpenInMenuButton/V2OpenInMenuButton.tsx` around lines 48 - 49, persisted の defaultOpenInApp を無検証で採用している resolvedApp の初期化は危険なので、sidebarProjectRows[0]?.defaultOpenInApp を currentApp/OPEN_IN_APP 経路で使える既知の ExternalApp 一覧で検証してから採用し、未知の値は安全な既定値 "finder" にフォールバックするよう修正してください；具体的には resolvedApp の設定箇所で既存の ExternalApp 列挙（あるいは isValidExternalApp チェック関数）に対して sidebarProjectRows[0]?.defaultOpenInApp を照合し、合致すれば採用、合致しなければ "finder" を代入して openInApp や currentApp に無効値が流れないようにしてください。apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/components/FileTreeItem/FileTreeItem.tsx-92-101 (1)
92-101:⚠️ Potential issue | 🟡 MinorShift+click がフォルダでは発火しません。
Line 94 で
isFolderを先に判定しているため、フォルダ項目ではe.shiftKeyが真でも Line 100-101 に到達せず、常に展開/折りたたみに吸われます。modifier 挙動をフォルダにも適用したいなら、shiftKey判定をこの分岐より前に置く必要があります。修正案
const handleClick = (e: React.MouseEvent) => { e.stopPropagation(); if (e.metaKey || e.ctrlKey) { onOpenInEditor(entry); + } else if (e.shiftKey) { + onActivate(entry, true); } else if (isFolder) { if (isExpanded) { item.collapse(); } else { item.expand(); } - } else if (e.shiftKey) { - onActivate(entry, true); } else { onActivate(entry); } };🤖 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/RightSidebar/FilesView/components/FileTreeItem/FileTreeItem.tsx` around lines 92 - 101, The current click handler in FileTreeItem.tsx checks isFolder before e.shiftKey, so shift+click on folders never triggers onActivate; change the branching so the e.shiftKey check occurs before the isFolder/isExpanded logic (or explicitly handle e.shiftKey for both files and folders) — i.e., inspect e.shiftKey first and call onActivate(entry, true) when set, otherwise fall through to the existing metaKey, folder (isFolder/isExpanded -> item.expand()/item.collapse()) and default behaviors to ensure shift-modifier applies to folder items too.packages/host-service/WORKSPACE_CREATION_FALLBACK.md-45-45 (1)
45-45:⚠️ Potential issue | 🟡 Minorコードフェンスに言語指定を付けてください。
ここは
markdownlintのMD040で落ちます。ts/diff/textなど、各ブロックに合う言語を付けておくと CI を素通りできます。Also applies to: 62-62, 69-69, 88-88, 122-122, 241-241
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/host-service/WORKSPACE_CREATION_FALLBACK.md` at line 45, The markdown file contains unlabeled triple-backtick code fences that trigger markdownlint MD040; update each code fence in WORKSPACE_CREATION_FALLBACK.md so they include an appropriate language identifier (e.g., ts, diff, text) matching the block contents to satisfy the linter; scan the file for all ``` fences and add the correct language tags consistently across the document.apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsx-279-285 (1)
279-285:⚠️ Potential issue | 🟡 Minor差分ペイン再利用時に選択ファイルを展開し直してください。
既存の
collapsedFilesをそのまま引き継ぐと、さっき折りたたんだファイルをサイドバーから選び直しても閉じたままになり、クリック後に内容が見えないことがあります。再利用時は少なくとも選択したfilePathだけはcollapsedFilesから外した方がよいです。💡 修正案
state.setPaneData({ paneId: pane.id, data: { ...prev, path: filePath, + collapsedFiles: prev.collapsedFiles.filter( + (path) => path !== filePath, + ), } as PaneViewerData, });🤖 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/page.tsx around lines 279 - 285, 既存の pane データを再利用する際に collapsedFiles をそのまま引き継ぐと、選択した filePath が折りたたまれたまま表示されない問題があるので、state.setPaneData 呼び出し（pane.id / PaneViewerData を更新する箇所）で data を作るときに、必ず選択中の filePath を collapsedFiles から除外して返してください（例: collapsedFiles を prev.collapsedFiles.filter(f => f !== filePath) のように更新して型を保つ）。apps/desktop/V2_WORKSPACE_MODAL_GAPS.md-90-97 (1)
90-97:⚠️ Potential issue | 🟡 MinorGap 7 の記述は現状コードとずれています。
packages/host-service/src/trpc/router/workspace-creation/normalize-github-query.tsとpackages/host-service/src/trpc/router/workspace-creation/workspace-creation.tsで、PR URL /#123の正規化とrepoMismatchによる cross-repo validation はすでに host 側で処理されています。ここを未解決 gap として残すと、同じ責務をクライアント側に再実装する方向へ誤誘導します。Also applies to: 109-117
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/V2_WORKSPACE_MODAL_GAPS.md` around lines 90 - 97, Gap 7 is outdated: remove or rewrite the note that PR URL parsing and cross-repo validation are client responsibilities and instead state that normalization and repoMismatch validation are implemented server-side in packages/host-service via normalize-github-query (in workspace-creation/normalize-github-query.ts) and the workspace-creation router (workspace-creation.ts); update the V2 PRLinkCommand entry to reflect that the host-service handles PR URL/#123 normalization and repoMismatch validation so the client should not reimplement that logic.apps/desktop/V2_WORKSPACE_MODAL_GAPS.md-47-49 (1)
47-49:⚠️ Potential issue | 🟡 Minorコードフェンスに language を付けたいです。
ここだけ未指定なので MD040 が出ます。
textかregexを付けておくと lint が静かになります。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/V2_WORKSPACE_MODAL_GAPS.md` around lines 47 - 49, The markdown code fence that contains the snippet describing the submit behavior (the block mentioning electronTrpc.workspaces.generateBranchName.mutateAsync(), the 30-second timeout, fallback to random name, and the "generating-branch" pending status) is missing a language tag and triggers MD040; update that fenced code block to include a language identifier such as text or regex (e.g., ```text) so the linter is satisfied.apps/desktop/V2_PR_LINK_COMMAND_DESIGN.md-7-8 (1)
7-8:⚠️ Potential issue | 🟡 Minor文書内で方針が衝突しています。
冒頭では「frontend-only / backend work 不要」とありますが、最終案と実装は host 側の
normalizeGitHubQuery/searchPullRequestsに寄せています。さらに helper 名と配置もnormalizePullRequestQuery/workspace-creation.tsではなくnormalizeGitHubQuery/normalize-github-query.tsです。ここがズレたままだと、この文書だけ読んだ人が逆方向に実装します。Also applies to: 111-125, 138-143
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/V2_PR_LINK_COMMAND_DESIGN.md` around lines 7 - 8, The document conflicts with the implementation: update the doc to match the codebase (or vice versa) by removing the "frontend-only / backend work unnecessary" claim and explicitly stating that V2's PRLinkCommand integrates with the host-side normalizeGitHubQuery and searchPullRequests endpoints; correct helper names and locations to normalizeGitHubQuery and normalize-github-query.ts (not normalizePullRequestQuery/workspace-creation.ts), and make the same corrections in the other referenced sections (lines ~111-125, 138-143) so readers won't implement the opposite approach.apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.ts-13-13 (1)
13-13:⚠️ Potential issue | 🟡 Minor
defaultOpenInAppは任意文字列ではなく許可値で検証したいです。ここが
z.string()だと、壊れた persisted 値がFilesTab.tsxでExternalAppに cast され、そのままexternal.openInAppに流れます。schema 側で enum / union にしておくほうが安全です。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.ts` at line 13, The schema field defaultOpenInApp currently uses a permissive z.string() which allows arbitrary persisted values to flow into FilesTab.tsx and be cast to ExternalApp then passed to external.openInApp; change defaultOpenInApp to validate against the exact allowed values (use z.enum([...]) or z.union of z.literal(...) matching the ExternalApp possible values) and keep .nullable().default(null) so invalid persisted strings are rejected at parse time; update the enum literal list to mirror the ExternalApp type/values used in FilesTab.tsx to ensure runtime safety.
🧹 Nitpick comments (7)
apps/desktop/src/renderer/routes/_authenticated/layout.tsx (1)
212-214:poolOptionsとhighlighterOptionsはメモ化しておいた方が安全です。Line 213-214 のオブジェクトが毎 render で新しくなるので、
WorkerPoolContextProviderが参照変化を依存にしている実装だとワーカープールを再初期化する可能性があります。ここはuseMemoで固定して、不要な worker churn を避けたいです。♻️ 変更案
-import { useEffect, useRef } from "react"; +import { useEffect, useMemo, useRef } from "react";+ const workerPoolOptions = useMemo( + () => ({ workerFactory: createPierreWorker, poolSize: 8 }), + [], + ); + const highlighterOptions = useMemo( + () => ({ preferredHighlighter: "shiki-wasm" as const }), + [], + ); + return ( <DndProvider manager={dragDropManager}> <CollectionsProvider> <GlobalTerminalLifecycle /> <LocalHostServiceProvider> <LanguageServicesProvider /> <WorkerPoolContextProvider - poolOptions={{ workerFactory: createPierreWorker, poolSize: 8 }} - highlighterOptions={{ preferredHighlighter: "shiki-wasm" }} + poolOptions={workerPoolOptions} + highlighterOptions={highlighterOptions} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/layout.tsx` around lines 212 - 214, The objects passed to WorkerPoolContextProvider (poolOptions and highlighterOptions) are recreated every render; wrap them with React's useMemo so they are stable references—memoize poolOptions (including workerFactory: createPierreWorker and poolSize: 8) and highlighterOptions (preferredHighlighter: "shiki-wasm") and pass the memoized values into WorkerPoolContextProvider to avoid reinitializing the worker pool; use appropriate dependency arrays (e.g., include createPierreWorker if it can change) when creating the useMemo calls.apps/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/CodeEditor.tsx (1)
561-565: フォント設定クエリは共通 Hook 化しておくと保守しやすいです。Line 561 のクエリ定義が他コンポーネントと重複しているため、
useFontSettingsなどへ抽出するとキーやstaleTimeのズレを防げます。🤖 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/components/CodeEditor/CodeEditor.tsx` around lines 561 - 565, 抽出してください: 現在 CodeEditor.tsx の useQuery 呼び出し（queryKey: ["electron","settings","getFontSettings"], queryFn: () => electronTrpcClient.settings.getFontSettings.query(), staleTime: 30_000）を共通フック useFontSettings に移し、他コンポーネントと同じキー／staleTime／queryFn を再利用するように統一してください。具体的には新しいフック useFontSettings を作成して内部で上記の queryKey／queryFn／staleTime を設定し、CodeEditor の該当箇所は useFontSettings() を呼ぶだけに置き換えて重複を排除してください。apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/components/FileSearchResultItem/FileSearchResultItem.tsx (1)
100-121: 修飾キー挙動をクリックとキーボードで共通化してください。現在、クリックでは
Meta/Ctrlが「エディタで開く」ですが、EnterではMeta/Ctrlが「新規タブで開く」になっており、操作体系が分岐しています。判定を1か所にまとめると将来の差分事故を防げます。差分案
+type OpenAction = "activate" | "activate-new-tab" | "open-in-editor"; + +function resolveOpenAction(e: { shiftKey: boolean; metaKey: boolean; ctrlKey: boolean }): OpenAction { + if (e.shiftKey) return "activate-new-tab"; + if (e.metaKey || e.ctrlKey) return "open-in-editor"; + return "activate"; +} + const handleClick = (e: React.MouseEvent) => { if (!entry.isDirectory) { - if (e.shiftKey) { - onActivate(entry, true); - } else if (e.metaKey || e.ctrlKey) { - onOpenInEditor(entry); - } else { - onActivate(entry); - } + const action = resolveOpenAction(e); + if (action === "activate-new-tab") onActivate(entry, true); + else if (action === "open-in-editor") onOpenInEditor(entry); + else onActivate(entry); } }; @@ const handleKeyDown = (e: React.KeyboardEvent) => { if (e.key === "Enter") { e.preventDefault(); if (!entry.isDirectory) { - onActivate(entry, e.metaKey || e.ctrlKey ? true : undefined); + const action = resolveOpenAction(e); + if (action === "activate-new-tab") onActivate(entry, true); + else if (action === "open-in-editor") onOpenInEditor(entry); + else onActivate(entry); } } };🤖 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/RightSidebar/FilesView/components/FileSearchResultItem/FileSearchResultItem.tsx` around lines 100 - 121, handleClick and handleKeyDown currently interpret modifier keys differently (click uses Meta/Ctrl to open in editor while Enter uses Meta/Ctrl to mean “open in new tab” via onActivate), so centralize the modifier handling into a single helper used by all input handlers: add a function (e.g., resolveActivationMode or getActivationAction) that takes the event/modifier state and the entry and returns the intended action (call onOpenInEditor(entry) vs onActivate(entry, true/undefined)), then refactor handleClick, handleDoubleClick, and handleKeyDown to call that helper (use e.metaKey || e.ctrlKey and e.shiftKey inside the helper) so click and Enter behave identically for Meta/Ctrl/Shift, preserving existing behavior for directories (no activation/open).apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/CommentThread/CommentThread.tsx (1)
17-65: 新規 UI なので inline style ではなく Tailwind / shadcn 側に寄せたいです。背景色・境界線・余白・文字サイズまで style object に閉じているので、テーマ追従と再利用がかなりしづらくなります。少なくとも色と spacing は
className/ デザイントークンへ寄せておくと、この diff pane 周りの見た目を後から揃えやすいです。Based on learnings: Use React + TailwindCSS v4 + shadcn/ui for all UI development.
🤖 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/components/DiffPane/components/CommentThread/CommentThread.tsx around lines 17 - 65, The inline style block in the CommentThread rendering uses hard-coded styles (background, border, padding, margin, fontSize, opacity) making theme and reuse hard; update the JSX in the CommentThread component to replace style objects with Tailwind (and shadcn tokens where appropriate) via className strings for the outer container, the "Resolved" label, and each comment item, and use conditional classNames for resolved vs open states (referencing isResolved), the comments.map rendering and per-comment elements (use comment.id, comment.avatarUrl, comment.authorLogin, comment.body to locate the nodes); ensure colors, spacing and typography use design tokens or Tailwind utility classes instead of inlined CSS and preserve the same visual behavior (border-top between comments when i>0, avatar sizing, rounded avatar, pre-wrap for body) via classNames.apps/desktop/plans/20260412-keyboard-recorder-ctrl-binding-fix.md (1)
153-168: コードフェンスに言語を付けてください。このブロックは markdownlint の MD040 に引っかかるので、
textなどを付けておくと lint ノイズを減らせます。💡 修正案
-``` +```text apps/desktop/plans/20260412-keyboard-recorder-ctrl-binding-fix.md (this doc) apps/desktop/src/renderer/hotkeys/ display.ts @@ apps/desktop/src/renderer/screens/.../Terminal/helpers.ts</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@apps/desktop/plans/20260412-keyboard-recorder-ctrl-binding-fix.mdaround
lines 153 - 168, Add a language identifier to the fenced code block(s) in the
markdown doc apps/desktop/plans/20260412-keyboard-recorder-ctrl-binding-fix.md
to satisfy markdownlint MD040; locate the triple-backtick blocks (the large file
listing block shown) and change the opening fence fromtotext (or
another appropriate language) so the linter no longer flags it—ensure every
fenced block in that file uses a language tag.</details> </blockquote></details> <details> <summary>apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/StatusIndicator/StatusIndicator.tsx (1)</summary><blockquote> `49-60`: **`status` を `string` で受けると未知値を静かに見逃します。** ここで `string` を `FileStatus` にキャストしているため、新しい状態値が混ざってもコンパイルで検出できず、UI では色もアイコンも出ないまま通ってしまいます。呼び出し側が `FileStatus` を持てるなら props もその型に揃えるか、未知値を明示的にフォールバックした方が安全です。 <details> <summary>💡 修正案</summary> ```diff export function StatusIndicator({ status, className, }: { - status: string; + status: FileStatus; className?: string; }) { return ( <span - className={`flex shrink-0 items-center ${STATUS_COLORS[status as FileStatus] ?? ""} ${className ?? ""}`} + className={`flex shrink-0 items-center ${STATUS_COLORS[status] ?? ""} ${className ?? ""}`} > - {getStatusIcon(status as FileStatus)} + {getStatusIcon(status)} </span> ); } ``` </details> As per coding guidelines "Avoid using `any` type in TypeScript - prefer explicit type safety". <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/StatusIndicator/StatusIndicator.tsx around lines 49 - 60, The StatusIndicator component accepts status: string which lets unknown values slip through; change the prop type to status: FileStatus so TypeScript enforces valid values (update the function signature for StatusIndicator) and remove the unsafe casts, and/or add an explicit runtime fallback: validate status against the keys of STATUS_COLORS (and/or allowed FileStatus values) before using it and pass a default status to getStatusIcon/STATUS_COLORS when invalid; ensure you reference StatusIndicator, FileStatus, STATUS_COLORS, and getStatusIcon when making the edits. ``` </details> </blockquote></details> <details> <summary>apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useChangeset/types.ts (1)</summary><blockquote> `1-1`: **この model は UI component ではなく shared type に依存させたいです。** `hooks/useChangeset/types.ts` が `../../components/StatusIndicator` から `FileStatus` を取ると、表示用 component の型変更が diff/sidebar のデータ契約に波及します。`apps/desktop/src/shared/changes-types.ts` を source of truth に寄せたほうが drift を防げます。 <details> <summary>🤖 Prompt for AI Agents</summary> ``` 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/useChangeset/types.ts at line 1, Replace the UI-component import of FileStatus with the shared model: in hooks/useChangeset/types.ts stop importing FileStatus from the StatusIndicator component and instead import the shared type exported from changes-types (the central shared changes model). Update any local type aliases or re-exports in useChangeset/types.ts to reference that shared FileStatus symbol so the changeset types align with the shared source-of-truth rather than the UI component. ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Inline comments:
In@apps/desktop/plans/20260411-v2-preset-parity.md:
- Around line 80-81: 現在の実装は useHotkey("OPEN_PRESET_N", () =>
executePreset(matchedPresets[N-1])) を常時登録すると matchedPresets に該当インデックスがない場合に
executePreset(undefined) を呼んでしまう問題があるため、ハンドラ内で実行前に存在チェックを行するか（例：if
(matchedPresets[N-1]) executePreset(matchedPresets[N-1])）、あるいは
matchedPresets[N-1] が存在する時だけ useHotkey を登録するように修正してください；対象の識別子は
matchedPresets、executePreset、useHotkey、OPEN_PRESET_1..OPEN_PRESET_9 です。In
@apps/desktop/src/lib/trpc/routers/external/index.ts:
- Around line 328-352: statPath currently falls back to resolving relative paths
against process.cwd() when getWorkspace/getWorkspacePath fails, which can
mis-resolve workspace-scoped paths; in the statPath mutation (function symbol:
statPath) check if input.workspaceId is provided and either
getWorkspace(input.workspaceId) or getWorkspacePath(...) returns no usable path,
and in that case return null immediately instead of calling resolvePath;
otherwise continue to call resolvePath(input.path, cwd) and fs.promises.stat as
before—this ensures relative paths with an explicit workspaceId don't
incorrectly resolve to the Electron process cwd.In
@apps/desktop/src/renderer/hooks/host-service/useFileTree/useFileTree.ts:
- Around line 218-221: force=true のときに React Query
のデフォルトキャッシュが優先され古い結果が返る可能性があるため、loadDirectory(absolutePath, true) や
refreshPath()/refreshAll() から utils.filesystem.listDirectory.fetch(...)
を呼ぶ前に該当クエリを明示的に無効化してください。具体的には utils.filesystem.listDirectory のクエリキー/ユーティリティを使って
workspaceId と absolutePath に対応するクエリを invalidate（または
utils.filesystem.listDirectory.invalidate({...}) を呼ぶ）してから、改めて
utils.filesystem.listDirectory.fetch({workspaceId, absolutePath})
を実行するように修正してください。これで force フラグ時に必ず最新データを取得できます。In
@apps/desktop/src/renderer/hotkeys/migrate.ts:
- Around line 22-49: The migration unconditionally rebuilds "hotkey-overrides"
from the legacy electronTrpcClient.uiState.hotkeys.get.query() whenever
MIGRATION_MARKER_KEY changed to "-v2", which can silently overwrite newer user
changes; update the logic in migrate.ts to first check localStorage for an
existing "hotkey-overrides", parse and sanitize its overrides (using
sanitizeOverride), then fetch the oldState via
electronTrpcClient.uiState.hotkeys.get.query(), sanitize those too, and merge
them by preferring existing localStorage entries over legacy ones for the same
id; finally write the merged sanitized overrides back to localStorage (under
"hotkey-overrides") and set MIGRATION_MARKER_KEY so re-runs do not clobber local
changes.In
@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/V2OpenInMenuButton/V2OpenInMenuButton.tsx:
- Around line 40-46: 既存ユーザーの defaultOpenInApp がリセットされる問題: locate useLiveQuery
usage and the ensureSidebarProjectRecord function that writes to
v2SidebarProjects (and the old v2WorkspaceLocalState) and implement a
migration/backfill or set defaultOpenInApp explicitly when creating records;
specifically, when ensureSidebarProjectRecord creates a new v2SidebarProjects
entry, read existing value from v2WorkspaceLocalState (or apply the runtime
fallback) and write that value into the new record’s defaultOpenInApp (instead
of relying on schema default null), or add a one-time migration that copies
defaultOpenInApp from v2WorkspaceLocalState into v2SidebarProjects for existing
projects.In
@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/WorkspaceFilesTreeItem.tsx:
- Around line 93-103: 現在のクリックハンドラの分岐順が逆で、フォルダクリック時に修飾キーがあると誤動作するため、まず isFolder
を判定してフォルダなら修飾キーが無い場合のみ onToggleDirectory(node.absolutePath) を呼び、Meta/Ctrl/Shift
等の修飾キーがある場合はファイル操作（onOpenInEditor/onSelectFile）を行わないようにしてください；具体的には onClick
の分岐順を入れ替え、最初に if (isFolder) { if (!e.metaKey && !e.ctrlKey && !e.shiftKey)
onToggleDirectory(node.absolutePath); } else { if (e.metaKey || e.ctrlKey)
onOpenInEditor(node.absolutePath); else if (e.shiftKey)
onSelectFile(node.absolutePath, true); else onSelectFile(node.absolutePath); }
というロジックに書き換えてください（参照: onOpenInEditor, onToggleDirectory, onSelectFile, isFolder,
node.absolutePath）。In
@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useChangeset/useChangeset.ts:
- Around line 84-102: The current ref.kind === "uncommitted" branch concatenates
status.staged and status.unstaged causing duplicate entries when the same path
exists in both; change this to merge by path using status.staged and
status.unstaged: build a map keyed by file.path, insert staged entries first
(source.kind = "staged"), then upsert unstaged data — if a path already exists
append or toggle source to indicate both (e.g., source.kind = "staged+unstaged")
or merge additions/deletions as appropriate; ensure the result is an array of
unique ChangesetFile objects (preserving path, oldPath, status, additions,
deletions and a discriminating source) instead of naively spreading both arrays.In
@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/DiffFileEntry/hooks/useInView/useInView.ts:
- Around line 16-31: The effect that creates the IntersectionObserver can miss a
late-mounted element because it depends on the RefObject (ref) instead of the
actual DOM node; change the effect to read const element = ref.current before
creating the observer, include that element (e.g. const observed = ref.current)
in the dependency array instead of ref so the effect re-runs when ref.current
changes, call observer.observe(observed) only if observed is non-null, and in
the cleanup unobserve the same observed node (or disconnect) to avoid stale
observers; keep root, rootMargin, threshold and setInView as dependencies.In
@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/WorkspaceDiff/WorkspaceDiff.tsx:
- Around line 109-110: The diffQuery for workspaceTrpc.git.getDiff is being
cached with staleTime: Number.POSITIVE_INFINITY so after handleDiscard succeeds
the UI still shows discarded diffs; update handleDiscard's success handler to
invalidate or refetch the getDiff query (use
workspaceTrpc.git.getDiff.invalidate/refetch via your TRPC query utils or the
queryClient) using the same diffInput so the diffQuery (diffQuery /
workspaceTrpc.git.getDiff) is refreshed and the pane reflects the real worktree
state.In
@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/DiffPane.tsx:
- Around line 83-99: The current setCollapsed builds a new data object from the
stale snapshot in dataRef.current and calls updateData with that copy, risking
lost concurrent updates; change setCollapsed to compute next collapsedFiles but
call updateData with an updater that receives the latest state (e.g.
updateData(prev => ({ ...prev, collapsedFiles: next }))) rather than spreading
dataRef.current so other fields (like path) won't be overwritten; update
references to dataRef.current in setCollapsed accordingly and keep dataRef for
stable reads only.In
@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useSidebarDiffRef/useSidebarDiffRef.ts:
- Around line 29-44: The hook currently returns DiffRef objects with empty
string commitHash (via commitHash ?? "") which causes downstream code to try
resolving invalid git refs; update the useMemo in useSidebarDiffRef so the
"commit" and "range" branches validate commitHash (and for "range" also validate
fromHash if needed) and, if the required hash(es) are missing/empty, return the
fallback { kind: "against-base", baseBranch } instead of returning an object
containing an empty commitHash; adjust the switch so "commit" only returns {
kind: "commit", commitHash } when commitHash is non-empty and "range" only
returns the commit-kind with both hashes when present, otherwise fallback to
against-base.In
@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2PresetExecution/useV2PresetExecution.ts:
- Around line 73-78: When preset.commands is empty we currently still create a
random terminalId pane without ever calling ensureSession, producing a
disconnected terminal; update useV2PresetExecution so that before calling
getPresetLaunchPlan or adding any pane/terminalId you check if
preset.commands.length === 0, show a user toast/error and return early (do not
add panes or call ensureSession), and apply the same guard to the other similar
blocks that add random terminal panes (the branches that build a plan and call
ensureSession/append pane using terminalId); reference getPresetLaunchPlan,
preset.commands, ensureSession and terminalId to locate where to insert the
early-return + toast.- Around line 93-96: The multi-session creation using Promise.all in the
"new-tab-multi-pane" branch (and similarly at the other noted locations) can
orphan already-created sessions if a later create fails; change
createSessionWithCommand usage to perform sequential creation (await each
createSessionWithCommand/ensureSession call), track successful terminal/session
IDs, and if any creation fails immediately dispose/cleanup all previously
created sessions (call the same dispose/ensureSession cleanup path or
terminal.dispose by terminalId) before rethrowing or showing the toast;
alternatively use Promise.allSettled and if any promise failed iterate the
fulfilled results to dispose their terminalIds and then handle the error—ensure
you reference createSessionWithCommand, ensureSession, and the terminalId/
dispose APIs when implementing the rollback.In
@apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/components/GitHubIssueLinkCommand/GitHubIssueLinkCommand.tsx:
- Around line 86-94: When debouncing, prevent the UI from showing or allowing
selection of stale searchResults while debouncedTrimmed differs from
trimmedQuery: update the visibility/enable logic (in GitHubIssueLinkCommand.tsx)
to treat results as unavailable when isPendingDebounce or trimmedQuery !==
debouncedTrimmed (e.g., compute a new flag like resultsAvailable =
!isPendingDebounce && trimmedQuery === debouncedTrimmed && searchResults.length0 and use that instead of searchResults.length > 0), and apply the same change
to the second occurrence around the 129-149 block so results are hidden/disabled
until the debounced query matches the current trimmedQuery.In
@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts:
- Around line 143-149: When handling WebGL context loss in the
webglAddon.onContextLoss callback, also set the terminal's suggested renderer to
"dom" so future terminal instances won't keep retrying WebGL; inside the
onContextLoss callback (where webglAddon.dispose() and webglAddon = null are
done for the webglAddon created and loaded into xterm via xterm.loadAddon), call
xterm.setOption('suggestedRendererType', 'dom') (or set the equivalent
options.suggestedRendererType) before refreshing rows to prevent repeated WebGL
load attempts.In
@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/v1-terminal-cache.ts:
- Around line 230-237: The error handler currently nulls entry.subscription but
leaves entry.streamReady true so the cache looks reattachable; update the
onError block (in v1-terminal-cache.ts where onError is defined alongside
entry.subscription, entry.subscriptionErrorHandler, DEBUG_TERMINAL) to also set
entry.streamReady = false (and any related "ready/attached" flags you use) when
a stream error occurs so that subsequent mounts will take the slow path and call
createOrAttach / startStream again rather than incorrectly reusing a dead
subscription.In
@docs/V2_WORKSPACE_SETUP_SCRIPTS.md:
- Around line 115-124: The initialCommand currently injects the raw
setupScriptPath into a shell (initialCommand: \bash "${setupScriptPath}"`` in
createTerminalSessionInternal call), which can be interpreted by the PTY and
lead to command injection; change createTerminalSessionInternal call to pass a
shell-safe invocation instead: either construct initialCommand using a
shell-escaped print approach (e.g., use printf '%s\n' with the path) or,
preferably, pass the script path via argv-style execution so bash receives it as
a positional parameter (e.g., run bash -c with exec "$1" "$@" -- and supply
setupScriptPath as an argument), ensuring the code path that writes to the PTY
(session.shellReadyPromise → pty.write in terminal.ts) never receives unescaped
user-controlled chars.In
@packages/host-service/src/terminal/terminal.ts:
- Around line 125-145: In resolveShellReady, the currently held OSC bytes in
session.scanState.heldBytes are only buffered via bufferOutput, which never
sends live data to connected clients; modify resolveShellReady to, after
clearing the timeout and before resetting heldBytes and matchPos, check
session.socket?.readyState === 1 and if true send the held bytes immediately
over the socket as a { type: "data", data: ... } message to the client,
otherwise fall back to bufferOutput(session, session.scanState.heldBytes); then
set session.scanState.heldBytes = "" and proceed to reset
session.scanState.matchPos and call session.shellReadyResolve as before.In
@packages/shared/src/shell-ready-scanner.ts:
- Around line 11-15: 現在の実装だけが OSC_133_A を識別しているため旧仕様の OSC 777
を受け取るとマッチせず初回入力が遅延する問題があります。OSC_133_A と並んで legacy マーカー（例: OSC_777 =
"\x1b]777;A"）を定義し、マーカー判定処理（ファイル内のマッチングロジック、コメントで示された 46-86 範囲の関数／分岐）を拡張して
startsWith/regex チェックで OSC_133_A または OSC_777 を検出したら matched/consumed
として扱い、該当バイト列が出力に漏れないよう消費処理に含めるよう修正してください。注意点として SHELLS_WITH_READY_MARKER
の扱いはそのままにし、既存の条件分岐に OSC_777 を追加するだけで互換性を保ってください。
Outside diff comments:
In@apps/desktop/src/renderer/hotkeys/utils/resolveHotkeyFromEvent.ts:
- Around line 55-67: canonicalizeChord lowercases the entire chord before alias
lookup which prevents CODE_ALIASES like "MetaLeft"/"ControlRight" from matching;
change it to split the chord by "+" and call normalizeToken on each raw token
(do not toLowerCase the token before normalizeToken), then use a lowercased
version only for modifier detection (e.g. const lower = token.toLowerCase(); if
(MODIFIERS.has(lower)) { mods.push(lower === "control" ? "ctrl" : lower) } else
{ keys.push(normalizedToken) }). Keep references to canonicalizeChord,
normalizeToken, and MODIFIERS and ensure the resulting ordering/joining matches
eventToChord.In
@apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/components/PRLinkCommand/PRLinkCommand.tsx:
- Around line 130-149: The current UI still renders previous pullRequests while
typing because isLoading only controls the empty-state text; update the render
logic around pullRequests/CommandGroup/CommandEmpty so that when
isPendingDebounce || isFetching is true you do not render (or render a
disabled/placeholder) the previous results: treat isPendingDebounce ||
isFetching the same as an empty result by using that combined flag in the
conditional that shows CommandGroup and CommandEmpty (refer to pullRequests,
isLoading, isPendingDebounce, isFetching, CommandGroup, CommandEmpty) so
previous suggestions are hidden/disabled during debounce/fetch.In
@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/hooks/useTerminalRestore.ts:
- Around line 131-163: The code is applying snapshot data for fresh attaches;
guard snapshot-based restores so they only run for non-new sessions: wrap the
snapshot parsing and assignments (the initialAnsi selection, the mode/ref
assignments like isAlternateScreenRef.current, isBracketedPasteRef.current,
modeScanBufferRef.current, and the isAltScreenReattach computation which reads
result.snapshot?.modes.alternateScreen) with a check that result.isNew is false
(or early-return when result.isNew), so new sessions ignore
snapshotAnsi/snapshot.cwd/mode flags; apply the same guard in the other
occurrence noted (around the block at the later occurrence ~225-229) to match
useTerminalColdRestore behavior.
Minor comments:
In
@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/V2OpenInMenuButton/V2OpenInMenuButton.tsx:
- Around line 48-49: persisted の defaultOpenInApp を無検証で採用している resolvedApp
の初期化は危険なので、sidebarProjectRows[0]?.defaultOpenInApp を currentApp/OPEN_IN_APP
経路で使える既知の ExternalApp 一覧で検証してから採用し、未知の値は安全な既定値 "finder"
にフォールバックするよう修正してください；具体的には resolvedApp の設定箇所で既存の ExternalApp 列挙（あるいは
isValidExternalApp チェック関数）に対して sidebarProjectRows[0]?.defaultOpenInApp
を照合し、合致すれば採用、合致しなければ "finder" を代入して openInApp や currentApp に無効値が流れないようにしてください。In
@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsx:
- Around line 279-285: 既存の pane データを再利用する際に collapsedFiles をそのまま引き継ぐと、選択した
filePath が折りたたまれたまま表示されない問題があるので、state.setPaneData 呼び出し（pane.id / PaneViewerData
を更新する箇所）で data を作るときに、必ず選択中の filePath を collapsedFiles から除外して返してください（例:
collapsedFiles を prev.collapsedFiles.filter(f => f !== filePath) のように更新して型を保つ）。In
@apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.ts:
- Line 13: The schema field defaultOpenInApp currently uses a permissive
z.string() which allows arbitrary persisted values to flow into FilesTab.tsx and
be cast to ExternalApp then passed to external.openInApp; change
defaultOpenInApp to validate against the exact allowed values (use z.enum([...])
or z.union of z.literal(...) matching the ExternalApp possible values) and keep
.nullable().default(null) so invalid persisted strings are rejected at parse
time; update the enum literal list to mirror the ExternalApp type/values used in
FilesTab.tsx to ensure runtime safety.In
@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/components/FileTreeItem/FileTreeItem.tsx:
- Around line 92-101: The current click handler in FileTreeItem.tsx checks
isFolder before e.shiftKey, so shift+click on folders never triggers onActivate;
change the branching so the e.shiftKey check occurs before the
isFolder/isExpanded logic (or explicitly handle e.shiftKey for both files and
folders) — i.e., inspect e.shiftKey first and call onActivate(entry, true) when
set, otherwise fall through to the existing metaKey, folder (isFolder/isExpanded
-> item.expand()/item.collapse()) and default behaviors to ensure shift-modifier
applies to folder items too.In
@apps/desktop/V2_PR_LINK_COMMAND_DESIGN.md:
- Around line 7-8: The document conflicts with the implementation: update the
doc to match the codebase (or vice versa) by removing the "frontend-only /
backend work unnecessary" claim and explicitly stating that V2's PRLinkCommand
integrates with the host-side normalizeGitHubQuery and searchPullRequests
endpoints; correct helper names and locations to normalizeGitHubQuery and
normalize-github-query.ts (not normalizePullRequestQuery/workspace-creation.ts),
and make the same corrections in the other referenced sections (lines ~111-125,
138-143) so readers won't implement the opposite approach.In
@apps/desktop/V2_WORKSPACE_MODAL_GAPS.md:
- Around line 90-97: Gap 7 is outdated: remove or rewrite the note that PR URL
parsing and cross-repo validation are client responsibilities and instead state
that normalization and repoMismatch validation are implemented server-side in
packages/host-service via normalize-github-query (in
workspace-creation/normalize-github-query.ts) and the workspace-creation router
(workspace-creation.ts); update the V2 PRLinkCommand entry to reflect that the
host-service handles PR URL/#123 normalization and repoMismatch validation so
the client should not reimplement that logic.- Around line 47-49: The markdown code fence that contains the snippet
describing the submit behavior (the block mentioning
electronTrpc.workspaces.generateBranchName.mutateAsync(), the 30-second timeout,
fallback to random name, and the "generating-branch" pending status) is missing
a language tag and triggers MD040; update that fenced code block to include a
language identifier such as text or regex (e.g., ```text) so the linter is
satisfied.In
@packages/host-service/WORKSPACE_CREATION_FALLBACK.md:
- Line 45: The markdown file contains unlabeled triple-backtick code fences that
trigger markdownlint MD040; update each code fence in
WORKSPACE_CREATION_FALLBACK.md so they include an appropriate language
identifier (e.g., ts, diff, text) matching the block contents to satisfy the
linter; scan the file for all ``` fences and add the correct language tags
consistently across the document.
Nitpick comments:
In@apps/desktop/plans/20260412-keyboard-recorder-ctrl-binding-fix.md:
- Around line 153-168: Add a language identifier to the fenced code block(s) in
the markdown doc
apps/desktop/plans/20260412-keyboard-recorder-ctrl-binding-fix.md to satisfy
markdownlint MD040; locate the triple-backtick blocks (the large file listing
block shown) and change the opening fence fromtotext (or another
appropriate language) so the linter no longer flags it—ensure every fenced block
in that file uses a language tag.In
@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/StatusIndicator/StatusIndicator.tsx:
- Around line 49-60: The StatusIndicator component accepts status: string which
lets unknown values slip through; change the prop type to status: FileStatus so
TypeScript enforces valid values (update the function signature for
StatusIndicator) and remove the unsafe casts, and/or add an explicit runtime
fallback: validate status against the keys of STATUS_COLORS (and/or allowed
FileStatus values) before using it and pass a default status to
getStatusIcon/STATUS_COLORS when invalid; ensure you reference StatusIndicator,
FileStatus, STATUS_COLORS, and getStatusIcon when making the edits.In
@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useChangeset/types.ts:
- Line 1: Replace the UI-component import of FileStatus with the shared model:
in hooks/useChangeset/types.ts stop importing FileStatus from the
StatusIndicator component and instead import the shared type exported from
changes-types (the central shared changes model). Update any local type aliases
or re-exports in useChangeset/types.ts to reference that shared FileStatus
symbol so the changeset types align with the shared source-of-truth rather than
the UI component.In
@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/CommentThread/CommentThread.tsx:
- Around line 17-65: The inline style block in the CommentThread rendering uses
hard-coded styles (background, border, padding, margin, fontSize, opacity)
making theme and reuse hard; update the JSX in the CommentThread component to
replace style objects with Tailwind (and shadcn tokens where appropriate) via
className strings for the outer container, the "Resolved" label, and each
comment item, and use conditional classNames for resolved vs open states
(referencing isResolved), the comments.map rendering and per-comment elements
(use comment.id, comment.avatarUrl, comment.authorLogin, comment.body to locate
the nodes); ensure colors, spacing and typography use design tokens or Tailwind
utility classes instead of inlined CSS and preserve the same visual behavior
(border-top between comments when i>0, avatar sizing, rounded avatar, pre-wrap
for body) via classNames.In
@apps/desktop/src/renderer/routes/_authenticated/layout.tsx:
- Around line 212-214: The objects passed to WorkerPoolContextProvider
(poolOptions and highlighterOptions) are recreated every render; wrap them with
React's useMemo so they are stable references—memoize poolOptions (including
workerFactory: createPierreWorker and poolSize: 8) and highlighterOptions
(preferredHighlighter: "shiki-wasm") and pass the memoized values into
WorkerPoolContextProvider to avoid reinitializing the worker pool; use
appropriate dependency arrays (e.g., include createPierreWorker if it can
change) when creating the useMemo calls.In
@apps/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/CodeEditor.tsx:
- Around line 561-565: 抽出してください: 現在 CodeEditor.tsx の useQuery 呼び出し（queryKey:
["electron","settings","getFontSettings"], queryFn: () =>
electronTrpcClient.settings.getFontSettings.query(), staleTime: 30_000）を共通フック
useFontSettings に移し、他コンポーネントと同じキー／staleTime／queryFn
を再利用するように統一してください。具体的には新しいフック useFontSettings を作成して内部で上記の
queryKey／queryFn／staleTime を設定し、CodeEditor の該当箇所は useFontSettings()
を呼ぶだけに置き換えて重複を排除してください。In
@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/components/FileSearchResultItem/FileSearchResultItem.tsx:
- Around line 100-121: handleClick and handleKeyDown currently interpret
modifier keys differently (click uses Meta/Ctrl to open in editor while Enter
uses Meta/Ctrl to mean “open in new tab” via onActivate), so centralize the
modifier handling into a single helper used by all input handlers: add a
function (e.g., resolveActivationMode or getActivationAction) that takes the
event/modifier state and the entry and returns the intended action (call
onOpenInEditor(entry) vs onActivate(entry, true/undefined)), then refactor
handleClick, handleDoubleClick, and handleKeyDown to call that helper (use
e.metaKey || e.ctrlKey and e.shiftKey inside the helper) so click and Enter
behave identically for Meta/Ctrl/Shift, preserving existing behavior for
directories (no activation/open).</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
MocA-Love
changed the title
- Fix OSC marker mismatch: update useTerminalModes to detect OSC 133;A (FinalTerm standard) in addition to legacy OSC 777, matching the marker change in shell-wrappers.ts (superset-sh#3389) - Restore isAlternateScreenRef guard on commandBufferRef accumulation to prevent suggestion buffer pollution during vim/less usage - Add shouldSkipSnapshot guard for new sessions in useTerminalRestore to prevent potential prompt double-display - Move NORMALIZE_GITHUB_QUERY_PLAN.md from src/ to plans/ per repo rules
- loader.ts: biome-ignore for Function type in Module internals (noBannedTypes) - vscode-api.ts: biome-ignore for non-null assertions in overloaded ctors, static-only class stubs (noNonNullAssertion, noStaticOnlyClass) - AddTabMenu.tsx: add missing DropdownMenuSeparator and DropdownMenuCheckboxItem imports - page.tsx (v2-workspace): add electronTrpc showPresetsBar query and setShowPresetsBar mutation - ModelsSettings.tsx: fix tuple type inference for NextEditUsageBucket map entries - small-model.ts: correct Codex small model ID to gpt-5.1-codex-mini (matches test expectation) - runtime.test.ts: add missing setSubagentModelId mock to harness stub
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/hooks/useTerminalLifecycle.ts (1)
839-844:fitAddonRefのクリアが欠落している可能性があります。
searchAddonRefはnullに設定されていますが、fitAddonRefはクリアされていません。キャッシュがアドオンのライフサイクルを管理しているため意図的かもしれませんが、両方の ref を一貫して扱うべきか確認してください。♻️ 一貫性のための提案
// Do NOT dispose xterm or reset stream state — the cache owns // both the xterm lifecycle and the stream subscription. xtermRef.current = null; + fitAddonRef.current = null; searchAddonRef.current = null; setXtermInstance(null);🤖 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/ContentView/TabsContent/Terminal/hooks/useTerminalLifecycle.ts` around lines 839 - 844, The cleanup currently sets xtermRef.current = null, searchAddonRef.current = null and calls setXtermInstance(null) but misses clearing fitAddonRef; update the same teardown in useTerminalLifecycle to also set fitAddonRef.current = null (or explicitly document why it must be retained by the cache) so refs are handled consistently with xtermRef and searchAddonRef.
🤖 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/lib/vscode-shim/loader.ts`:
- Around line 134-135: Replace the non-null assertion on manifest.main with an
explicit guard: in the function that computes mainPath (the block using
info.manifest.main and assigning mainPath), check that info.manifest &&
info.manifest.main is a non-empty string and throw a clear, early Error
(including the extension id or info.extensionPath) if it's missing or invalid,
then compute mainPath with path.resolve only after the guard; remove or avoid
relying on the biome-ignore lint exemption and ensure the thrown error message
is descriptive for later debugging.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/page.tsx:
- Around line 201-216: The titleOverride value derived from
filePath.split(/[/\\]/).pop() can be undefined for empty paths or trailing
separators; update the openInNewTab branch before calling state.addTab to
compute a safe string (e.g., const title = filePath.split(/[/\\]/).pop() ?? ''
or a fallback like 'Untitled' or the full filePath) and pass that title variable
to titleOverride so it is never undefined; ensure any required typing/casting
for FilePaneData remains correct when substituting the fallback.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/PaneToolbarActions/PaneToolbarActions.tsx`:
- Around line 86-89: The split-opposite button in PaneToolbarActions (the
element using onSplitPaneOpposite in PaneToolbarActions.tsx) lacks an accessible
name; add an explicit accessible label by adding an aria-label (or
aria-labelledby referencing a visually-hidden text element) that describes the
action (e.g., "Split pane opposite" or the localized equivalent) so screen
readers can announce its purpose while preserving the existing class, onClick
handler, and visual layout.
In `@plans/normalize-github-query-plan.md`:
- Around line 12-13: The document currently contradicts itself about using
"in:title,body" (kept at lines 12-13 but removed at 149-150); pick one policy
and make it consistent: either keep "in:title,body" everywhere or remove it
everywhere, and update the Step 3 sample query to match that decision; ensure
all references to the search syntax (the phrase "in:title,body", the Step 3
example query, and the statements at the earlier and later sections) use the
same wording and intent so the implementation guidance is unambiguous.
- Around line 116-136: When handling normalized.isDirectLookup and calling
octokit.issues.get, wrap the call in a try/catch around the octokit.issues.get
invocation (the direct lookup branch that builds issueNumber and queries
octokit) and if the caught error has a 404 status, return { issues: [] } (keep
the existing PR-filtering behavior for non-404 results); for any other errors
rethrow or propagate/log them so genuine failures are not swallowed. Ensure you
reference the normalized.isDirectLookup branch and the octokit.issues.get call
when making the change.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/hooks/useTerminalLifecycle.ts`:
- Around line 839-844: The cleanup currently sets xtermRef.current = null,
searchAddonRef.current = null and calls setXtermInstance(null) but misses
clearing fitAddonRef; update the same teardown in useTerminalLifecycle to also
set fitAddonRef.current = null (or explicitly document why it must be retained
by the cache) so refs are handled consistently with xtermRef and searchAddonRef.
🪄 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: 2373b0c2-0b69-49ed-8214-60173058a5d1
📒 Files selected for processing (18)
apps/desktop/src/main/lib/vscode-shim/loader.tsapps/desktop/src/main/lib/vscode-shim/vscode-api.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/AddTabMenu/AddTabMenu.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/models/components/ModelsSettings/ModelsSettings.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/DatabaseExplorerPane/DatabaseExplorerPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/FileViewerPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/FileViewerToolbar/FileViewerToolbar.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/TabPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/BasePaneWindow/BasePaneWindow.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/PaneToolbarActions/PaneToolbarActions.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/hooks/useTerminalLifecycle.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/hooks/useTerminalModes.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/hooks/useTerminalRestore.tspackages/chat/src/server/desktop/small-model/small-model.tspackages/chat/src/server/trpc/utils/runtime/runtime.test.tsplans/normalize-github-query-plan.md
✅ Files skipped from review due to trivial changes (2)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/AddTabMenu/AddTabMenu.tsx
- apps/desktop/src/main/lib/vscode-shim/vscode-api.ts
- merge-pull-request.test.ts: add --repo superset-sh/superset to expected gh CLI args (impl now passes --repo from PR URL) - cache.test.ts: advance Date.now by 30_001ms instead of 10_001ms (TTL changed to 30_000ms) - useOrderedSections.test.tsx: add renderHook helper using React 19 internal dispatcher so useMemo works outside component context
- Remove unused biome-ignore comment (was on wrong line) - Import OrderedSection type and cast result.current for proper typing
Owner
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f79aa35dcb
ℹ️ 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".
Merge origin/main (conflict resolution): - vscode-shim/loader.ts: adopt type-safe ResolveFilename pattern from main - vscode-shim/vscode-api.ts: adopt type guards + fuller NotebookEdit from main - ModelsSettings.tsx: adopt object-array style from main - page.tsx: remove duplicate showPresetsBar declaration introduced by merge Review comment fixes: - hotkeys/migrate.ts: skip write when localStorage overrides already exist to prevent v2 re-migration from overwriting user's current settings - WorkspaceDiff.tsx: invalidate getDiff + getStatus after successful discard - useChangeset.ts: deduplicate staged+unstaged files by path (unstaged wins) - useSidebarDiffRef.ts: fall back to against-base when commitHash is empty - WorkspaceFilesTreeItem.tsx: check isFolder before modifier keys in onClick - external/index.ts: return null early when workspaceId given but not found - useTerminalLifecycle.ts: require active subscription for reattach fast path
Owner
Author
|
@codex review |
- v1-terminal-cache.ts: reset streamReady=false on stream error so remount does not take the reattach fast path with a dead subscription - useV2PresetExecution.ts: early return with toast when preset has no commands - GitHubIssueLinkCommand.tsx: hide stale search results during debounce pending - shell-ready-scanner.ts: support legacy OSC 777 marker alongside OSC 133;A - terminal.ts: send flushed held bytes to connected socket instead of buffer - PaneToolbarActions.tsx: add aria-label to opposite-split button - page.tsx: add fallback "File" for titleOverride when pop() returns undefined
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 24f2879c36
ℹ️ 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: 2
♻️ Duplicate comments (4)
apps/desktop/src/lib/trpc/routers/external/index.ts (1)
342-345:⚠️ Potential issue | 🟠 Major
workspacePath未取得時に相対パスがprocess.cwd()基準へフォールバックします。Line 345 で
cwdがundefinedのままresolvePathすると、workspaceIdを渡していても相対パスがワークスペース外に解決される可能性があります。workspaceが存在してもgetWorkspacePathが取れない場合を早期nullにしてください。💡 修正案
+import nodePath from "node:path"; import fs from "node:fs"; @@ const cwd = workspace ? (getWorkspacePath(workspace) ?? undefined) : undefined; + if (input.workspaceId && !cwd && !nodePath.isAbsolute(input.path)) { + return null; + } const resolved = resolvePath(input.path, cwd);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/external/index.ts` around lines 342 - 345, 現在のコード sets cwd to undefined when workspace is present but getWorkspacePath(workspace) returns falsy, which allows resolvePath(input.path, cwd) to fall back to process.cwd(); change logic so that if workspace is provided but getWorkspacePath(workspace) returns null/undefined, you pass an explicit null (or bail) instead of undefined; update the assignment of cwd (the variable computed from workspace and getWorkspacePath) used by resolvePath so resolvePath can detect a missing workspace root (e.g., return early or pass null) and avoid resolving relative paths against process.cwd(); ensure you only alter the code near the cwd calculation and the subsequent call to resolvePath.apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2PresetExecution/useV2PresetExecution.ts (1)
99-101:⚠️ Potential issue | 🟠 Major
Promise.allの部分失敗で成功済みセッションが孤児化する可能性があります。Line [99]-[101] / [117]-[119] / [148]-[150] は並列作成の途中で 1 件失敗すると reject しますが、Line [174]-[182] の catch では通知のみで成功済みセッションの回収がありません。
逐次作成 + 失敗時ロールバック、またはPromise.allSettledで成功分を明示的に dispose する形にしてください。Also applies to: 117-119, 148-150, 174-182
🤖 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/useV2PresetExecution/useV2PresetExecution.ts around lines 99 - 101, The parallel creation using Promise.all over preset.commands with createSessionWithCommand can leave successfully created sessions orphaned if one creation fails; replace this by either (a) sequentially creating sessions (for...of over preset.commands calling createSessionWithCommand) and on any failure run a rollback loop to dispose already-created sessions, or (b) use Promise.allSettled for preset.commands, detect any rejected results and call the session cleanup function (e.g., disposeSession/destroySession) for every fulfilled result before rethrowing the error; update the catch block that currently only notifies to also perform that cleanup of successful sessions.apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/WorkspaceFilesTreeItem.tsx (1)
93-107:⚠️ Potential issue | 🟠 Majorフォルダの修飾クリックをファイル操作に流さないでください。
Line 94-103 の分岐だと、フォルダでも
Meta/CtrlクリックでonOpenInEditor(node.absolutePath)が実行され、ShiftクリックはonToggleDirectoryに吸われます。フォルダは通常クリックだけで展開/折りたたみにして、少なくともこのハンドラで特別扱いしているMeta/Ctrl/Shift付きは早期returnで止めた方が安全です。💡 修正例
onClick={(e) => { - if (isFolder) { - if (e.metaKey || e.ctrlKey) { - onOpenInEditor(node.absolutePath); - } else { - onToggleDirectory(node.absolutePath); - } - } else if (e.metaKey || e.ctrlKey) { + if (isFolder) { + if (!e.metaKey && !e.ctrlKey && !e.shiftKey) { + onToggleDirectory(node.absolutePath); + } + return; + } + if (e.metaKey || e.ctrlKey) { onOpenInEditor(node.absolutePath); } else if (e.shiftKey) { onSelectFile(node.absolutePath, true); } else { onSelectFile(node.absolutePath); } }}🤖 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/components/WorkspaceSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/WorkspaceFilesTreeItem.tsx around lines 93 - 107, The click handler lets modifier-clicks on folders fall through to file actions; change the logic so folders only act on plain clicks: inside the onClick for WorkspaceFilesTreeItem, update the isFolder branch (using node.absolutePath, onToggleDirectory, onOpenInEditor, onSelectFile) to early-return when e.metaKey || e.ctrlKey || e.shiftKey (i.e., if isFolder and any modifier is pressed, do nothing/return), and otherwise call onToggleDirectory(node.absolutePath); keep the existing file branches for modifier/plain clicks unchanged.apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useChangeset/useChangeset.ts (1)
84-108:⚠️ Potential issue | 🟠 Major複合ソースの変更を最後の 1 件に潰すと差分が欠落します。
Map#setの上書きでstaged + unstagedやagainst-base + dirtyのファイルが最後のsourceだけに置き換わるため、部分 stage 済みファイルでは staged 側の hunks/件数が、against-base では base との差分が一覧から消えます。重複表示は解消できますが、このままだとサイドバーから開ける diff が実際の全変更と一致しません。sourceに複合状態を持たせるか、一覧用のadditions/deletionsを統合したほうが安全です。Also applies to: 111-146
🤖 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/useChangeset/useChangeset.ts around lines 84 - 108, The merge logic that builds byPath currently overwrites staged entries with unstaged (and vice versa) causing loss of hunks/counts; in the block handling ref.kind === "uncommitted" (and the similar block for "against-base"/"dirty"), detect when byPath already has an entry for file.path and instead of replacing it combine metadata: sum additions/deletions (or merge hunk lists if present), preserve oldPath/status appropriately, and set source to a composite marker (e.g., { kind: "composite", staged: true, unstaged: true } or include both sources) so the UI diff reflects both staged and unstaged changes rather than only the last writer.
🧹 Nitpick comments (3)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/v1-terminal-cache.ts (1)
295-301:unregisterHandlersでsubscriptionErrorHandlerも null にしていますが、ストリームエラー時のログ出力が失われる可能性があります。コンポーネントがアンマウントされた状態でストリームエラーが発生した場合、
DEBUG_TERMINALが false だとエラーが完全に無視されます。これは意図的な設計かもしれませんが、デバッグ時に問題を見逃す可能性があります。💡 エラーハンドラをnullにせず、デフォルトのログ出力を維持する案
export function unregisterHandlers(paneId: string): void { const entry = cache.get(paneId); if (!entry) return; entry.eventHandler = null; - entry.subscriptionErrorHandler = null; + // Keep subscriptionErrorHandler so errors while unmounted are still + // forwarded when the component remounts (or logged if DEBUG_TERMINAL). }🤖 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/ContentView/TabsContent/Terminal/v1-terminal-cache.ts` around lines 295 - 301, The current unregisterHandlers function clears both eventHandler and subscriptionErrorHandler which can drop stream error logs when a component is unmounted; update unregisterHandlers so it does NOT overwrite subscriptionErrorHandler with null (or if you must replace it, set it to a safe default logger) — modify the function handling in v1-terminal-cache.ts (function unregisterHandlers) to either leave entry.subscriptionErrorHandler intact or assign a fallback that logs (respecting the DEBUG_TERMINAL flag if needed), while still clearing entry.eventHandler as before.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/hooks/useTerminalLifecycle.ts (1)
844-849: コメントで「xtermを破棄しない」と説明されていますが、xtermRef.current = nullは設定されています。これは意図的な設計です（キャッシュがxtermを所有しているため、refのみをクリアする）が、将来の開発者が混乱する可能性があります。コメントを少し明確にすることを検討してください。
📝 コメントをより明確にする案
- // Do NOT dispose xterm or reset stream state — the cache owns - // both the xterm lifecycle and the stream subscription. + // Do NOT dispose xterm or reset stream state — the cache owns + // both the xterm lifecycle and the stream subscription. + // We only clear the local refs; the cache retains the instances. xtermRef.current = null;🤖 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/ContentView/TabsContent/Terminal/hooks/useTerminalLifecycle.ts` around lines 844 - 849, The comment claiming "Do NOT dispose xterm or reset stream state — the cache owns both the xterm lifecycle and the stream subscription." is ambiguous because the code still clears refs (xtermRef.current = null, searchAddonRef.current = null, setXtermInstance(null)); update the comment near that block (around xtermRef, searchAddonRef, setXtermInstance) to explicitly state that we only clear local references for GC/cleanup but intentionally do not call xterm.dispose() or unsubscribe the stream because the shared cache manages lifecycle and subscriptions; mention that clearing refs avoids dangling pointers in this component while ownership remains elsewhere.packages/shared/src/shell-ready-scanner.ts (1)
37-38:matchTargetに未定義の sentinel を入れない方が安全です。
"pending" as "osc133"と(state.matchTarget as string)で union を迂回しているので、この状態遷移が型検査に乗りません。"pending"を型に含めるか、共有 prefix 解決中フラグを別 field に分けた方が保守しやすいです。♻️ 修正イメージ
export interface ShellReadyScanState { matchPos: number; heldBytes: string; /** Which marker we're currently tracking after the shared ESC-] prefix. */ - matchTarget: "osc133" | "osc777" | null; + matchTarget: "pending" | "osc133" | "osc777" | null; } @@ - state.matchTarget = "pending" as "osc133"; // temporary sentinel + state.matchTarget = "pending"; @@ - if ((state.matchTarget as string) === "pending") { + if (state.matchTarget === "pending") {As per coding guidelines,
**/*.{ts,tsx}: Avoid usinganytype in TypeScript code; ensure type safety unless absolutely necessary.Also applies to: 83-99
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/shared/src/shell-ready-scanner.ts` around lines 37 - 38, The matchTarget field currently uses the union type "osc133" | "osc777" | null but the code circumvents that with hacks like "pending" as "osc133" and (state.matchTarget as string); fix by either adding "pending" to the matchTarget union type or, preferably, introducing a separate boolean/enum flag (e.g., isResolvingPrefix or matchResolutionState) to represent the intermediate "pending" state and keep matchTarget strictly "osc133" | "osc777" | null; update all usages and state transitions that reference state.matchTarget and any casts so they use the new flag or the expanded union consistently (remove any "pending" casts and (state.matchTarget as string) usages) and update functions/methods that read matchTarget to respect the new two-field model.
🤖 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/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/useChangeset/useChangeset.ts:
- Around line 35-47: The code treats the response from
workspaceTrpc.git.getCommitFiles (commitQuery) as an object with a files
property, but GitTaskResultMap.getCommitFiles returns a ChangedFile[] directly,
so replace all uses of commitQuery.data?.files with commitQuery.data (including
the memoized selectors using useMemo) and make the useMemo dependency arrays
reference the same value (commitQuery.data) so commits toggle correctly; update
the other similar spots (the other occurrences around the same hook where
commitQuery.data?.files or dependent useMemo entries are used) to the same
pattern.
In `@packages/host-service/src/terminal/terminal.ts`:
- Around line 268-270: The shell name extraction in resolveLaunchShell()
currently uses split("/").pop() which returns names like "bash.exe" on
MSYS2/Cygwin and fails the SHELLS_WITH_READY_MARKER check; replace that logic by
reusing the existing getShellName() from shell-launch.ts (or mimic its behavior)
which uses node:path.basename() and strips a trailing ".exe" before comparing,
then compute shellSupportsReady =
SHELLS_WITH_READY_MARKER.has(getShellName(shell)); ensure you import or call
getShellName() and remove the old split("/").pop() usage so ready scanner and
initialCommand waiting work correctly.
---
Duplicate comments:
In `@apps/desktop/src/lib/trpc/routers/external/index.ts`:
- Around line 342-345: 現在のコード sets cwd to undefined when workspace is present
but getWorkspacePath(workspace) returns falsy, which allows
resolvePath(input.path, cwd) to fall back to process.cwd(); change logic so that
if workspace is provided but getWorkspacePath(workspace) returns null/undefined,
you pass an explicit null (or bail) instead of undefined; update the assignment
of cwd (the variable computed from workspace and getWorkspacePath) used by
resolvePath so resolvePath can detect a missing workspace root (e.g., return
early or pass null) and avoid resolving relative paths against process.cwd();
ensure you only alter the code near the cwd calculation and the subsequent call
to resolvePath.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/WorkspaceFilesTreeItem.tsx:
- Around line 93-107: The click handler lets modifier-clicks on folders fall
through to file actions; change the logic so folders only act on plain clicks:
inside the onClick for WorkspaceFilesTreeItem, update the isFolder branch (using
node.absolutePath, onToggleDirectory, onOpenInEditor, onSelectFile) to
early-return when e.metaKey || e.ctrlKey || e.shiftKey (i.e., if isFolder and
any modifier is pressed, do nothing/return), and otherwise call
onToggleDirectory(node.absolutePath); keep the existing file branches for
modifier/plain clicks unchanged.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/useChangeset/useChangeset.ts:
- Around line 84-108: The merge logic that builds byPath currently overwrites
staged entries with unstaged (and vice versa) causing loss of hunks/counts; in
the block handling ref.kind === "uncommitted" (and the similar block for
"against-base"/"dirty"), detect when byPath already has an entry for file.path
and instead of replacing it combine metadata: sum additions/deletions (or merge
hunk lists if present), preserve oldPath/status appropriately, and set source to
a composite marker (e.g., { kind: "composite", staged: true, unstaged: true } or
include both sources) so the UI diff reflects both staged and unstaged changes
rather than only the last writer.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/useV2PresetExecution/useV2PresetExecution.ts:
- Around line 99-101: The parallel creation using Promise.all over
preset.commands with createSessionWithCommand can leave successfully created
sessions orphaned if one creation fails; replace this by either (a) sequentially
creating sessions (for...of over preset.commands calling
createSessionWithCommand) and on any failure run a rollback loop to dispose
already-created sessions, or (b) use Promise.allSettled for preset.commands,
detect any rejected results and call the session cleanup function (e.g.,
disposeSession/destroySession) for every fulfilled result before rethrowing the
error; update the catch block that currently only notifies to also perform that
cleanup of successful sessions.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/hooks/useTerminalLifecycle.ts`:
- Around line 844-849: The comment claiming "Do NOT dispose xterm or reset
stream state — the cache owns both the xterm lifecycle and the stream
subscription." is ambiguous because the code still clears refs (xtermRef.current
= null, searchAddonRef.current = null, setXtermInstance(null)); update the
comment near that block (around xtermRef, searchAddonRef, setXtermInstance) to
explicitly state that we only clear local references for GC/cleanup but
intentionally do not call xterm.dispose() or unsubscribe the stream because the
shared cache manages lifecycle and subscriptions; mention that clearing refs
avoids dangling pointers in this component while ownership remains elsewhere.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/v1-terminal-cache.ts`:
- Around line 295-301: The current unregisterHandlers function clears both
eventHandler and subscriptionErrorHandler which can drop stream error logs when
a component is unmounted; update unregisterHandlers so it does NOT overwrite
subscriptionErrorHandler with null (or if you must replace it, set it to a safe
default logger) — modify the function handling in v1-terminal-cache.ts (function
unregisterHandlers) to either leave entry.subscriptionErrorHandler intact or
assign a fallback that logs (respecting the DEBUG_TERMINAL flag if needed),
while still clearing entry.eventHandler as before.
In `@packages/shared/src/shell-ready-scanner.ts`:
- Around line 37-38: The matchTarget field currently uses the union type
"osc133" | "osc777" | null but the code circumvents that with hacks like
"pending" as "osc133" and (state.matchTarget as string); fix by either adding
"pending" to the matchTarget union type or, preferably, introducing a separate
boolean/enum flag (e.g., isResolvingPrefix or matchResolutionState) to represent
the intermediate "pending" state and keep matchTarget strictly "osc133" |
"osc777" | null; update all usages and state transitions that reference
state.matchTarget and any casts so they use the new flag or the expanded union
consistently (remove any "pending" casts and (state.matchTarget as string)
usages) and update functions/methods that read matchTarget to respect the new
two-field model.
🪄 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: a4e1f300-4e5e-4a52-937e-4caf684e5c59
📒 Files selected for processing (19)
apps/desktop/src/lib/trpc/routers/changes/utils/merge-pull-request.test.tsapps/desktop/src/lib/trpc/routers/external/index.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/github/cache.test.tsapps/desktop/src/renderer/hotkeys/migrate.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/WorkspaceFilesTreeItem.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useChangeset/useChangeset.tsapps/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/useSidebarDiffRef/useSidebarDiffRef.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2PresetExecution/useV2PresetExecution.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsxapps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/components/GitHubIssueLinkCommand/GitHubIssueLinkCommand.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/FileViewerPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/PaneToolbarActions/PaneToolbarActions.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/hooks/useTerminalLifecycle.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/v1-terminal-cache.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/hooks/useOrderedSections/useOrderedSections.test.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/CodeEditor.tsxpackages/host-service/src/terminal/terminal.tspackages/shared/src/shell-ready-scanner.ts
✅ Files skipped from review due to trivial changes (2)
- apps/desktop/src/lib/trpc/routers/changes/utils/merge-pull-request.test.ts
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/FileViewerPane.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/CodeEditor.tsx
- apps/desktop/src/renderer/hotkeys/migrate.ts
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/PaneToolbarActions/PaneToolbarActions.tsx
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsx
- apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/components/GitHubIssueLinkCommand/GitHubIssueLinkCommand.tsx
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useSidebarDiffRef/useSidebarDiffRef.ts
This was referenced Apr 14, 2026
This was referenced Apr 22, 2026
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
3 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.
概要
upstream (superset-sh/superset) の 2026-04-13 時点の変更を取り込み、フォーク固有の修正を追加。加えて、タブツールバーに逆方向の分割ボタンを新規追加した。
upstream 取り込み内容
フォーク独自追加: タブツールバーに縦/横分割ボタンを追加
auto分割ボタン（⌘E）の隣に、逆方向の分割ボタンを追加。auto が horizontal を選択する場合は vertical ボタンを、vertical を選択する場合は horizontal ボタンを表示する。
変更内容
PaneToolbarActions.tsx:onSplitPaneOppositeオプション prop を追加し、逆方向分割ボタンを追加。ホットキーツールチップも対応（SPLIT_DOWN / SPLIT_RIGHT）BasePaneWindow.tsx:splitPaneHorizontal/splitPaneVerticalをオプション props として追加し、splitOrientationに基づいて逆方向ハンドラを自動計算してPaneHandlers.onSplitPaneOppositeに設定FileViewerToolbar.tsx:onSplitPaneOppositeprop を追加してPaneToolbarActionsに渡すよう対応TabPane.tsx、ChatPane.tsx、DatabaseExplorerPane.tsx、FileViewerPane.tsx:splitPaneHorizontal/splitPaneVerticalをBasePaneWindowに渡し、onSplitPaneOppositeを toolbar に接続テスト方法
Summary by CodeRabbit
新機能
改善