feat: add --worktree flag toggle for Claude Code#578
Draft
ashalliants wants to merge 3 commits into
Draft
Conversation
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds git worktree isolation support with a major sidebar redesign. It introduces worktree detection and project grouping by repository, new UI components for displaying branches and worktrees, a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as Chat/Settings UI
participant ComposerState as useChatComposerState
participant Shell as useShellConnection
participant Server as server/index.js
participant SDK as server/claude-sdk.js
participant Git as git commands
User->>UI: Enable worktree option & enter worktree name
UI->>ComposerState: updateUseWorktreeForSession(), setWorktreeName()
User->>UI: Submit new session
ComposerState->>ComposerState: Build claudeToolsSettings with useWorktree flag
ComposerState->>Shell: Send message with claudeSettings
Shell->>Server: WebSocket init with claudeSettings {useWorktree, skipPermissions}
Server->>SDK: mapCliOptionsToSDK(cliOptions, claudeSettings)
SDK->>SDK: Build sdkOptions.settings.worktree = {name: worktreeName}
Server->>Git: git worktree add || git checkout
Git-->>Server: Success
Server->>User: Session initialized in isolated git worktree
sequenceDiagram
participant Server as server/projects.js
participant Git as git commands
participant FS as File System
participant Sidebar as SidebarProjectList
Server->>Git: Discover projects in workspace
Server->>Git: getWorktreeInfo(projectPath)
Git-->>Server: {isWorktree, mainRepoRoot, branchName, worktreeRoot}
Server->>FS: fs.access() to check if stale
Server->>Server: Group by mainRepoRoot
Server->>Server: Synthesize parent "main" project if needed
Server->>Server: Annotate with repoGroup, repoGroupSize, isMainWorktree
Server-->>Sidebar: Projects with worktreeInfo & repo grouping
Sidebar->>Sidebar: groupProjectsByRepo(projects)
Sidebar->>Sidebar: Render RepoCard per group
Sidebar->>Sidebar: Show branch chips & worktree rows
Possibly Related PRs
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/i18n/locales/ru/settings.json (1)
335-338: Localize the new RU strings to avoid mixed-language UI.The key structure is correct, but this introduces English copy in the Russian locale and causes inconsistent UX in the settings panel.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/i18n/locales/ru/settings.json` around lines 335 - 338, The Russian locale contains English text for the "useWorktree" setting; update the "useWorktree.label" and "useWorktree.description" values in src/i18n/locales/ru/settings.json to proper Russian translations (replace the current English strings for the keys "useWorktree.label" and "useWorktree.description") so the settings panel no longer mixes languages and displays fully localized RU text.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/i18n/locales/ko/settings.json`:
- Around line 356-359: The Korean locale currently contains English placeholders
for the new useWorktree keys; update the useWorktree object by translating the
"label" and "description" into Korean (i.e., replace the English strings for
useWorktree.label and useWorktree.description with appropriate Korean
translations) so the UI is fully localized.
---
Nitpick comments:
In `@src/i18n/locales/ru/settings.json`:
- Around line 335-338: The Russian locale contains English text for the
"useWorktree" setting; update the "useWorktree.label" and
"useWorktree.description" values in src/i18n/locales/ru/settings.json to proper
Russian translations (replace the current English strings for the keys
"useWorktree.label" and "useWorktree.description") so the settings panel no
longer mixes languages and displays fully localized RU text.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 97ed8830-1a0f-448f-beb3-5dacd26d6b9e
📒 Files selected for processing (13)
server/claude-sdk.jssrc/components/chat/types/types.tssrc/components/chat/utils/chatStorage.tssrc/components/settings/hooks/useSettingsController.tssrc/components/settings/types/types.tssrc/components/settings/view/tabs/agents-settings/sections/AgentCategoryContentSection.tsxsrc/components/settings/view/tabs/agents-settings/sections/content/PermissionsContent.tsxsrc/i18n/locales/de/settings.jsonsrc/i18n/locales/en/settings.jsonsrc/i18n/locales/ja/settings.jsonsrc/i18n/locales/ko/settings.jsonsrc/i18n/locales/ru/settings.jsonsrc/i18n/locales/zh-CN/settings.json
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/components/chat/utils/__tests__/chatStorage.test.ts (1)
4-12: Consider makingclearconsistent with other mock methods.The
clearmethod is a plain function whilegetItem,setItem, andremoveItemusevi.fn(). This doesn't affect test functionality but could be made consistent if you want to assert onclearcalls in future tests.🔧 Optional: Make clear a vi.fn() for consistency
const localStorageMock = (() => { let store: Record<string, string> = {}; return { getItem: vi.fn((key: string) => store[key] ?? null), setItem: vi.fn((key: string, value: string) => { store[key] = value; }), removeItem: vi.fn((key: string) => { delete store[key]; }), - clear: () => { store = {}; }, + clear: vi.fn(() => { store = {}; }), }; })();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/chat/utils/__tests__/chatStorage.test.ts` around lines 4 - 12, The localStorageMock's clear method is a plain function while getItem/setItem/removeItem use vi.fn(); make clear consistent by replacing its implementation with a spy: change clear to vi.fn(() => { store = {}; }) while keeping the same closure over store so tests can now assert calls on localStorageMock.clear just like they do for localStorageMock.getItem, setItem, and removeItem.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/claude-sdk.js`:
- Around line 174-177: The current code sets sdkOptions.worktree = {} which is
not a valid ClaudeAgentOptions field; instead, when settings.useWorktree is
true, remove the invalid assignment and configure the agent to use isolation and
the worktree tools/events: set the agent option isolation: "worktree" (e.g.,
when constructing the agent options object), and register/use the built-in tools
EnterWorktree and ExitWorktree plus hooks/events WorktreeCreate and
WorktreeRemove for any custom VCS setup, so replace references to
sdkOptions.worktree with these correct mechanisms (check where sdkOptions is
assembled and where the agent is created to add isolation:"worktree" and
hook/tool registration).
In `@src/i18n/__tests__/locales.test.ts`:
- Line 5: The test uses the LOCALES_DIR constant which relies on __dirname but
__dirname isn't defined in ES modules; add the standard ES-module definition at
the top of this test file by importing fileURLToPath from 'url' and dirname from
'path', then define const __dirname = dirname(fileURLToPath(import.meta.url));
so LOCALES_DIR = path.resolve(__dirname, '../locales') works as expected; place
these imports/definition before the LOCALES_DIR declaration to mirror the
pattern used in other files.
---
Nitpick comments:
In `@src/components/chat/utils/__tests__/chatStorage.test.ts`:
- Around line 4-12: The localStorageMock's clear method is a plain function
while getItem/setItem/removeItem use vi.fn(); make clear consistent by replacing
its implementation with a spy: change clear to vi.fn(() => { store = {}; })
while keeping the same closure over store so tests can now assert calls on
localStorageMock.clear just like they do for localStorageMock.getItem, setItem,
and removeItem.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 18456a1e-3581-4510-bbe4-f932fcf4040c
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
package.jsonserver/__tests__/claude-sdk.test.jsserver/claude-sdk.jssrc/components/chat/utils/__tests__/chatStorage.test.tssrc/i18n/__tests__/locales.test.tsvitest.config.js
ashalliants
force-pushed
the
feat/worktree-toggle
branch
from
08278ed to
8707776
Compare
Author
|
@coderabbitai resume |
Contributor
✅ Actions performedReviews resumed. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
src/i18n/__tests__/locales.test.ts (1)
14-19: Expand locale key checks to cover nested fields used byPermissionsContent.Current assertions only validate object presence for
skipPermissions,allowedTools, andblockedTools. The UI reads deeper keys (e.g., labels/descriptions/placeholders), so missing translations can still ship undetected.Also applies to: 43-59
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/i18n/__tests__/locales.test.ts` around lines 14 - 19, The test currently only asserts presence of top-level keys in REQUIRED_PERMISSION_KEYS but must also verify nested translation fields that PermissionsContent reads (e.g., labels, descriptions, placeholders, and any enum/item labels under 'allowedTools'/'blockedTools'); update the test in src/i18n/__tests__/locales.test.ts to iterate each locale and for each key in REQUIRED_PERMISSION_KEYS assert the presence and non-empty string values for the deeper properties referenced by PermissionsContent (explicitly check label, description, placeholder and any per-tool display names), and apply the same deeper checks for the other assertions around lines where the current permission keys are validated (the block covering the other assertions referenced in the comment).server/index.js (1)
1800-1801: Consider using a more robust worktree path detection.The string check
projectPath.includes('/.claude/worktrees/')works for the standard Claude worktree location but could miss edge cases on Windows (backslashes) or non-standard configurations.🔧 Optional: Use path-based check for cross-platform support
-const isAlreadyInWorktree = projectPath.includes('/.claude/worktrees/'); +const isAlreadyInWorktree = projectPath.includes(`${path.sep}.claude${path.sep}worktrees${path.sep}`) || + projectPath.includes('/.claude/worktrees/');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/index.js` around lines 1800 - 1801, The current detection uses a raw string include on projectPath (isAlreadyInWorktree) which fails on Windows/backslashes or non-standard locations; update the check to normalize and inspect path segments instead: require('path'), use path.normalize(projectPath) or split path by path.sep and verify that the segment sequence ['.claude','worktrees'] appears, then derive isAlreadyInWorktree from that normalized/segment-based test and keep shouldCreateWorktree as claudeSettings.useWorktree && !hasSession && !isAlreadyInWorktree so the logic uses cross-platform path checks (referencing projectPath, isAlreadyInWorktree, shouldCreateWorktree, claudeSettings.useWorktree, hasSession).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/load-env.js`:
- Around line 11-16: The deletion of CLAUDECODE (delete process.env.CLAUDECODE)
is being undone by the .env loader; move the removal to after the environment is
loaded (or remove CLAUDECODE from the parsed .env object before merging) so the
variable cannot be reintroduced. Locate the delete process.env.CLAUDECODE
statement and ensure it runs after dotenv/config or whatever env-loading call is
used (also apply the same change for the other occurrence around the second
block at lines 23-25), or strip CLAUDECODE from the result of the env loader
before assigning to process.env.
In `@server/projects.js`:
- Around line 1271-1302: The cleanupWorktreeDirectory function currently calls
execFileAsync('git', ['worktree', 'remove', worktreePath, '--force'], ...) which
can discard uncommitted/untracked changes and swallows errors; update it to
first verify mainRepoRoot is a git repo (e.g. run execFileAsync('git',
['rev-parse', '--show-toplevel'], {cwd: mainRepoRoot}) and bail if that fails),
then attempt git worktree remove without '--force' and only fall back to
'--force' if the first remove fails and you log the specific error;
additionally, in both the outer catch (around execFileAsync removal) and the
inner catch (around fs.rm and prune), log detailed errors including worktreePath
and mainRepoRoot via console.error or processLogger (referencing
cleanupWorktreeDirectory, execFileAsync, fs.rm, and git worktree prune) instead
of silently ignoring them so failures are visible for debugging.
- Around line 698-705: When creating manual project entries, add the same
fallback worktree synthesis used for Claude-discovered projects whenever
getWorktreeInfo(actualProjectDir) returns null (i.e., after the try/catch that
sets project.worktreeInfo: if null, call the same synthesize/fallback routine
used for Claude-discovered projects to populate project.worktreeInfo so deleted
worktrees still group with their parent). Also initialize project.isStale on the
manual project object (set project.isStale = false or the same initial value
used for Claude-discovered projects) before pushing the project to projects to
ensure consistent behavior with Claude-discovered entries.
In `@src/i18n/__tests__/locales.test.ts`:
- Around line 24-29: The current loop that reads and JSON.parses locale files at
module load time (using SUPPORTED_LOCALES, LOCALES_DIR, and populating
localeData) causes the test suite to crash before assertions if a settings.json
is missing/invalid; move the file I/O into test runtime (e.g., within each test
case or a beforeEach) and wrap reads/parses in existence checks and try/catch so
the tests can assert presence/validity (for example, check
fs.existsSync(filePath) and then JSON.parse inside a try block) instead of
parsing during suite definition.
- Around line 45-47: The test dereferences nested properties directly
(localeData[locale].permissions and permissions[key]) which can throw TypeError
if a parent key is missing; before asserting about permissions[key] in the test
(variables: localeData, locale, permissions, key), first assert that
localeData[locale] is defined and that permissions (e.g., const permissions =
(localeData[locale] as ...).permissions) is defined and is an object, then
perform the existing assertions about permissions[key]; apply the same guarding
pattern to the similar block referenced around lines 52-58 so the tests fail
with clear assertion messages rather than throwing.
---
Nitpick comments:
In `@server/index.js`:
- Around line 1800-1801: The current detection uses a raw string include on
projectPath (isAlreadyInWorktree) which fails on Windows/backslashes or
non-standard locations; update the check to normalize and inspect path segments
instead: require('path'), use path.normalize(projectPath) or split path by
path.sep and verify that the segment sequence ['.claude','worktrees'] appears,
then derive isAlreadyInWorktree from that normalized/segment-based test and keep
shouldCreateWorktree as claudeSettings.useWorktree && !hasSession &&
!isAlreadyInWorktree so the logic uses cross-platform path checks (referencing
projectPath, isAlreadyInWorktree, shouldCreateWorktree,
claudeSettings.useWorktree, hasSession).
In `@src/i18n/__tests__/locales.test.ts`:
- Around line 14-19: The test currently only asserts presence of top-level keys
in REQUIRED_PERMISSION_KEYS but must also verify nested translation fields that
PermissionsContent reads (e.g., labels, descriptions, placeholders, and any
enum/item labels under 'allowedTools'/'blockedTools'); update the test in
src/i18n/__tests__/locales.test.ts to iterate each locale and for each key in
REQUIRED_PERMISSION_KEYS assert the presence and non-empty string values for the
deeper properties referenced by PermissionsContent (explicitly check label,
description, placeholder and any per-tool display names), and apply the same
deeper checks for the other assertions around lines where the current permission
keys are validated (the block covering the other assertions referenced in the
comment).
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2ffc08c0-1c67-4361-bfdd-a11361cc96fd
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (44)
package.jsonserver/__tests__/claude-sdk.test.jsserver/__tests__/worktree-grouping.test.jsserver/claude-sdk.jsserver/index.jsserver/load-env.jsserver/projects.jsserver/routes/git.jssrc/components/app/AppContent.tsxsrc/components/chat/types/types.tssrc/components/chat/utils/__tests__/chatStorage.test.tssrc/components/chat/utils/chatStorage.tssrc/components/git-panel/hooks/useGitPanelController.tssrc/components/settings/hooks/useSettingsController.tssrc/components/settings/types/types.tssrc/components/settings/view/tabs/agents-settings/sections/AgentCategoryContentSection.tsxsrc/components/settings/view/tabs/agents-settings/sections/content/PermissionsContent.tsxsrc/components/shell/hooks/useShellConnection.tssrc/components/shell/types/types.tssrc/components/sidebar/types/types.tssrc/components/sidebar/utils/utils.tssrc/components/sidebar/view/subcomponents/SidebarContent.tsxsrc/components/sidebar/view/subcomponents/SidebarModals.tsxsrc/components/sidebar/view/subcomponents/SidebarProjectItem.tsxsrc/components/sidebar/view/subcomponents/SidebarProjectList.tsxsrc/components/sidebar/view/subcomponents/SidebarProjectsState.tsxsrc/components/sidebar/view/subcomponents/SidebarRepoGroup.tsxsrc/hooks/useProjectsState.tssrc/i18n/__tests__/locales.test.tssrc/i18n/locales/de/settings.jsonsrc/i18n/locales/de/sidebar.jsonsrc/i18n/locales/en/settings.jsonsrc/i18n/locales/en/sidebar.jsonsrc/i18n/locales/ja/settings.jsonsrc/i18n/locales/ja/sidebar.jsonsrc/i18n/locales/ko/settings.jsonsrc/i18n/locales/ko/sidebar.jsonsrc/i18n/locales/ru/settings.jsonsrc/i18n/locales/ru/sidebar.jsonsrc/i18n/locales/zh-CN/settings.jsonsrc/i18n/locales/zh-CN/sidebar.jsonsrc/types/app.tssrc/types/global.d.tsvitest.config.js
✅ Files skipped from review due to trivial changes (18)
- src/types/global.d.ts
- package.json
- src/i18n/locales/de/settings.json
- src/i18n/locales/en/settings.json
- vitest.config.js
- src/components/shell/types/types.ts
- src/i18n/locales/de/sidebar.json
- src/components/settings/hooks/useSettingsController.ts
- src/i18n/locales/zh-CN/settings.json
- src/components/sidebar/types/types.ts
- src/i18n/locales/en/sidebar.json
- src/i18n/locales/ru/settings.json
- src/i18n/locales/ru/sidebar.json
- src/i18n/locales/ja/sidebar.json
- src/i18n/locales/ko/settings.json
- src/components/settings/view/tabs/agents-settings/sections/content/PermissionsContent.tsx
- src/i18n/locales/ja/settings.json
- src/components/chat/utils/chatStorage.ts
🚧 Files skipped from review as they are similar to previous changes (9)
- src/components/chat/types/types.ts
- src/components/settings/types/types.ts
- server/claude-sdk.js
- src/components/sidebar/view/subcomponents/SidebarProjectList.tsx
- src/i18n/locales/zh-CN/sidebar.json
- src/types/app.ts
- src/components/chat/utils/tests/chatStorage.test.ts
- src/i18n/locales/ko/sidebar.json
- server/tests/claude-sdk.test.js
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
server/projects.js (1)
1297-1308:⚠️ Potential issue | 🟠 MajorAvoid force-removing the worktree on the first delete attempt.
git worktree removerefuses dirty worktrees unless--forceis used. Calling it with--forceimmediately here, then falling back tofs.rm(), means deleting a project can discard uncommitted or untracked work without a second destructive opt-in. Please try a plain remove first and only escalate to a forced delete when the caller explicitly confirms that data loss is acceptable. (git-scm.com)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/projects.js` around lines 1297 - 1308, The current removal always calls execFileAsync('git', ['worktree', 'remove', worktreePath, '--force'], ...) which can discard local changes; change the logic in the block that uses execFileAsync, fs.rm, worktreePath and mainRepoRoot so it first attempts execFileAsync('git', ['worktree', 'remove', worktreePath'], ...) without --force, and only if that call fails with the specific “worktree dirty”/refusal error (or an explicit force flag passed into the surrounding function, e.g., a forceDelete parameter) retry with --force; preserve the existing fallback to fs.rm only when forced or when git is unavailable, and update the console.warn/process logs to clearly indicate when a forced destructive delete was used versus a safe non-forced removal.
🧹 Nitpick comments (1)
src/i18n/__tests__/locales.test.ts (1)
21-45: Consider usingbeforeAllfor cleaner test isolation.The current approach populates
localeData[locale]as a side effect within the second test (line 35). Subsequent tests depend on this test having run and succeeded. While the guards (expect(root).toBeDefined()) ensure clear failures, using abeforeAllhook would make tests more independent and allow selective test execution (e.g.,it.only) without breaking dependent tests.♻️ Optional refactor for test isolation
for (const locale of SUPPORTED_LOCALES) { describe(`locale: ${locale}`, () => { const filePath = path.join(LOCALES_DIR, locale, 'settings.json'); + let localeContent: Record<string, unknown> | undefined; + + beforeAll(() => { + if (fs.existsSync(filePath)) { + try { + const raw = fs.readFileSync(filePath, 'utf-8'); + localeContent = JSON.parse(raw); + localeData[locale] = localeContent; + } catch { + // Let tests assert the failure + } + } + }); it('has a settings.json file', () => { expect(fs.existsSync(filePath)).toBe(true); }); it('has valid settings.json content', () => { - expect(() => { - const raw = fs.readFileSync(filePath, 'utf-8'); - localeData[locale] = JSON.parse(raw); - }).not.toThrow(); + expect(localeContent).toBeDefined(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/i18n/__tests__/locales.test.ts` around lines 21 - 45, The tests populate localeData[locale] as a side-effect inside the `'has valid settings.json content'` it block, causing later tests (like the permissions check) to depend on that test having run; move the file read + JSON.parse for each locale into a beforeAll hook inside the describe(`locale: ${locale}`) block so localeData[locale] is populated before any it tests run (use the same filePath and JSON parsing logic currently in the validity test), keep the existing expect guards in the it blocks but read/parse in beforeAll to allow running individual it tests (e.g., it.only) without breaking them, and retain references to SUPPORTED_LOCALES, LOCALES_DIR, and settings.json when locating the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/projects.js`:
- Around line 394-407: The path parser is Windows-unsafe because it only looks
for '/.claude/worktrees/'; create a small helper (e.g., normalizePathForParsing)
that returns the input path with backslashes converted to forward slashes, use
that in synthesizeWorktreeInfoFromPath to find the marker '/.claude/worktrees/'
and compute markerIdx, afterMarker and worktreeName from the normalized path,
then map results back to the original string when building
worktreeRoot/mainRepoRoot/branchName if you need original separators; also
replace the same logic in the other helper that uses the same marker (the
cleanupWorktreeDirectory-related function around the other occurrence) so both
are separator-agnostic.
- Around line 417-430: The function fixWorktreeDisplayNames is incorrectly
treating an arbitrary grouped[0] (mainProject) that may itself be a worktree as
the repo's real main checkout, causing siblings with displayName === branchName
to be rewritten to that child's name; change the logic to detect whether
mainProject is actually a worktree (e.g. const mainIsWorktree =
!!mainProject?.worktreeInfo) and only perform the
branch-name-to-mainProject.displayName rewrite when mainIsWorktree is false
(i.e., when you have a real main checkout). Apply the same guard to the
equivalent branch-name rewrite at the other occurrence referenced in the
comment.
---
Duplicate comments:
In `@server/projects.js`:
- Around line 1297-1308: The current removal always calls execFileAsync('git',
['worktree', 'remove', worktreePath, '--force'], ...) which can discard local
changes; change the logic in the block that uses execFileAsync, fs.rm,
worktreePath and mainRepoRoot so it first attempts execFileAsync('git',
['worktree', 'remove', worktreePath'], ...) without --force, and only if that
call fails with the specific “worktree dirty”/refusal error (or an explicit
force flag passed into the surrounding function, e.g., a forceDelete parameter)
retry with --force; preserve the existing fallback to fs.rm only when forced or
when git is unavailable, and update the console.warn/process logs to clearly
indicate when a forced destructive delete was used versus a safe non-forced
removal.
---
Nitpick comments:
In `@src/i18n/__tests__/locales.test.ts`:
- Around line 21-45: The tests populate localeData[locale] as a side-effect
inside the `'has valid settings.json content'` it block, causing later tests
(like the permissions check) to depend on that test having run; move the file
read + JSON.parse for each locale into a beforeAll hook inside the
describe(`locale: ${locale}`) block so localeData[locale] is populated before
any it tests run (use the same filePath and JSON parsing logic currently in the
validity test), keep the existing expect guards in the it blocks but read/parse
in beforeAll to allow running individual it tests (e.g., it.only) without
breaking them, and retain references to SUPPORTED_LOCALES, LOCALES_DIR, and
settings.json when locating the file.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3a1671e0-06b7-4f12-8197-8695fe264a3e
📒 Files selected for processing (3)
server/load-env.jsserver/projects.jssrc/i18n/__tests__/locales.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- server/load-env.js
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/projects.js`:
- Around line 731-739: Normalize the repo-group key before bucketing: compute a
normalizedRepoRoot from project.worktreeInfo?.mainRepoRoot (use Node's
path.normalize to unify separators and, on Windows, lowercase the path for
case-insensitive matching) and use that normalizedRepoRoot when checking/setting
repoGroups instead of the raw mainRepoRoot; this ensures outputs of
getWorktreeInfo() and synthesizeWorktreeInfoFromPath() map to the same bucket.
Reference repoGroups and project.worktreeInfo?.mainRepoRoot when implementing
the change and add a path import if not present.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Merged
6 tasks
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (7)
server/projects.js (1)
299-310: Usepath.joinfor the synthesized worktree path.
${repoRoot}/.claude/worktrees/${worktreeName}hardcodes forward slashes. On Windows,repoRoot(resolved from the parent project) typically contains backslashes, so the constructed path mixes separators. Node FS tolerates this, butpath.joinis more idiomatic and consistent with the rest of the file (e.g., line 312, 79).♻️ Proposed fix
- const repoRoot = await extractProjectDirectory(parentProjectName); - const candidatePath = `${repoRoot}/.claude/worktrees/${worktreeName}`; + const repoRoot = await extractProjectDirectory(parentProjectName); + const candidatePath = path.join(repoRoot, '.claude', 'worktrees', worktreeName); projectDirectoryCache.set(projectName, candidatePath); return candidatePath;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/projects.js` around lines 299 - 310, The synthesized worktree path currently concatenates strings with forward slashes; update the construction to use path.join to produce a platform-safe path. In the block handling worktreeMarker (variables: worktreeMarker, markerIdx, parentProjectName, worktreeName) where repoRoot is obtained from extractProjectDirectory(parentProjectName), replace the literal `${repoRoot}/.claude/worktrees/${worktreeName}` with a path.join(repoRoot, '.claude', 'worktrees', worktreeName) result before setting projectDirectoryCache.set(projectName, ...) and returning it so paths are correct on Windows and consistent with other uses in this file.docs/superpowers/specs/2026-04-25-sidebar-worktree-redesign-design.md (1)
29-29: Add a language tag to the fenced code blocks (markdownlint MD040).Both ASCII-art blocks are tagged as plain fences. Adding
text(orplaintext) silences the linter and is a common convention for non-language content.📝 Proposed fix
-``` +```text ┌──────────────────────────────────────────────┐-``` +```text SidebarProjectList.tsxAlso applies to: 123-123
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/superpowers/specs/2026-04-25-sidebar-worktree-redesign-design.md` at line 29, The fenced code blocks containing ASCII-art and the "SidebarProjectList.tsx" snippet are currently untagged and trigger markdownlint MD040; update those fence openings to include a plaintext tag (e.g., ```text or ```plaintext) for the two ASCII-art blocks shown and any other similar plain fences (the other occurrence noted at the document). Locate the untagged triple-backtick blocks in the sidebar-worktree-redesign-design.md file and add the language tag to each opening fence so the linter recognizes them as non-code/plaintext.src/components/sidebar/view/subcomponents/BranchChip.tsx (1)
5-30: Tie palette length to the hash modulus to avoid silent drift.
branchChipColorIndexusesPALETTE_SIZE = 5inrepoAggregates.ts, and this file independently defines a 5-entryPALETTE. If one is changed, the chip will silently fall back toNEUTRALfor some branches. Either export the palette length fromrepoAggregates(and use it there) or assert the relationship at module load.Optional refactor
-import { branchChipColorIndex } from '../../utils/repoAggregates'; +import { branchChipColorIndex } from '../../utils/repoAggregates'; + +// Keep in sync with PALETTE_SIZE in repoAggregates.ts const PALETTE = [ ... ]; +if (process.env.NODE_ENV !== 'production') { + // Sanity: hash modulus and palette length must match. + // (Or export PALETTE_SIZE and derive both from it.) +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/sidebar/view/subcomponents/BranchChip.tsx` around lines 5 - 30, PALETTE in BranchChip.tsx is independently sized while branchChipColorIndex in repoAggregates.ts uses a hardcoded PALETTE_SIZE=5, which can cause silent fallback to NEUTRAL if they diverge; fix by either importing an exported palette size (e.g., export PALETTE_SIZE or getPaletteLength) from repoAggregates and use that to derive the palette or by adding a runtime assertion at module load in BranchChip.tsx that checks PALETTE.length === branchChipColorIndex's modulus constant (referencing PALETTE and branchChipColorIndex) and throw/log a clear error if they differ so the relationship cannot silently drift.src/components/sidebar/view/subcomponents/RepoCard.tsx (2)
158-177: Performance: per-worktreegetRepoSessions/getRepoSessionTotalruns on every render.For each
WorktreeRow, bothgetRepoSessionTotal([wt])(Line 163) andlastActivityLabelFor(wt, additionalSessions, currentTime, t)(Line 164) are computed inline on every render, andlastActivityLabelForitself callsgetRepoSessions([project], additionalSessions)which clones, annotates, and sorts that worktree's sessions. With many worktrees and acurrentTimeticking each second (typical for relative-time labels), this is wasted work on the hot path.Consider a single memo that produces
Map<name, { sessionCount, lastActivityLabel }>keyed off[projects, additionalSessions, currentTime]:Suggested refactor
+ const worktreeStats = useMemo(() => { + const map = new Map<string, { sessionCount: number; lastActivityLabel: string | null }>(); + for (const wt of linkedWorktrees) { + const sessions = getRepoSessions([wt], additionalSessions); + map.set(wt.name, { + sessionCount: getRepoSessionTotal([wt]), + lastActivityLabel: sessions.length === 0 + ? null + : labelForDate(new Date(sessions[0].lastActivity || sessions[0].createdAt || 0), currentTime, t), + }); + } + return map; + }, [linkedWorktrees, additionalSessions, currentTime, t]);…and consume
worktreeStats.get(wt.name)in both the active and archivedWorktreeRowmappings.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/sidebar/view/subcomponents/RepoCard.tsx` around lines 158 - 177, Rendering calls compute getRepoSessionTotal and lastActivityLabelFor (which itself calls getRepoSessions) for every WorktreeRow on each render, causing wasted work when currentTime updates; fix by computing a memoized Map keyed by [activeWorktrees, additionalSessions, currentTime] (e.g., worktreeStats: Map<name, { sessionCount, lastActivityLabel }>) using useMemo, populate it by iterating activeWorktrees and calling getRepoSessions/getRepoSessionTotal/lastActivityLabelFor once per worktree, then replace inline calls in the WorktreeRow props (sessionCount, lastActivityLabel) with lookups like worktreeStats.get(wt.name) so re-renders reuse the precomputed values.
112-122: ConditionaldefaultValuefor plurals is ineffective.Same pattern as in
WorktreeRow.tsx:t('projects.worktrees', { defaultValue: linkedWorktrees.length === 1 ? 'worktree' : 'worktrees', count: linkedWorktrees.length })—defaultValueis only used when the key is missing; withcount, i18next selectsworktrees_one/worktrees_otherfrom the bundle. Either define plural keys (worktrees_one/worktrees_other) and drop the conditional, or use distinctt()calls. The same applies toprojects.sessionsShorton Line 112.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/sidebar/view/subcomponents/RepoCard.tsx` around lines 112 - 122, The current conditional defaultValue on t() is ineffective for plurals; update the code to use i18next pluralization properly by adding plural keys (projects.worktrees_one / projects.worktrees_other and projects.sessionsShort_one / projects.sessionsShort_other) in your locale bundle and then call t('projects.worktrees', { count: linkedWorktrees.length }) and t('projects.sessionsShort', { count: sessionTotal }) in RepoCard (same pattern applies in WorktreeRow.tsx) so you can remove the linkedWorktrees.length === 1 conditional and let i18next pick the correct plural form.src/components/sidebar/view/subcomponents/RecentSessions.tsx (1)
122-134:formatRelativereturns "0m ago" for the 0–59 s window whenlastActivityis slightly in the past but rounds down.Line 128 already short-circuits
minutes < 1to "just now", so this is fine for the common path. But note the "1 minute ago" case will render as "1m ago" rather than the singulartime.oneMinuteAgokey that exists in the locale files (e.g.,de/sidebar.jsonhas"oneMinuteAgo": "vor 1 Min."). Consider routing 1m and 1h through the singular keys for higher-quality localized output.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/sidebar/view/subcomponents/RecentSessions.tsx` around lines 122 - 134, formatRelative currently returns pluralized minute/hour translations (e.g., "1m ago") because it only short-circuits for <1 minute; update formatRelative to detect the singular cases and call the singular i18n keys: when minutes === 1 use t('time.oneMinuteAgo', { defaultValue: '1m ago' }) and when hours === 1 use t('time.oneHourAgo', { defaultValue: '1h ago' }), keeping the existing branches for <1 minute, minutes <60, hours <24 and days; apply these checks using the same computed values (date from session.lastActivity || session.createdAt, minutes, hours) so localization uses the dedicated singular keys.src/components/sidebar/view/subcomponents/WorktreeRow.tsx (1)
32-39: Conditional defaultValue in i18next is ignored when the translation key exists.The bundles define
sessionsShort_oneandsessionsShort_otheracross all locales, so i18next usescountto select the correct plural form from the bundle rather than from the conditionaldefaultValue. The ternary expression doesn't control pluralization and only adds confusion.Suggested fix
- : `${sessionCount} ${t('projects.sessionsShort', { - defaultValue: sessionCount === 1 ? 'session' : 'sessions', - count: sessionCount, - })}${lastActivityLabel ? ` · ${lastActivityLabel}` : ''}`; + : `${sessionCount} ${t('projects.sessionsShort', { + defaultValue: 'sessions', + count: sessionCount, + })}${lastActivityLabel ? ` · ${lastActivityLabel}` : ''}`;This aligns with how the same pattern is already implemented in
RepoCard.tsx.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/sidebar/view/subcomponents/WorktreeRow.tsx` around lines 32 - 39, The metaText uses a ternary defaultValue to pick singular/plural for t('projects.sessionsShort') but i18next will ignore that when plural keys exist; replace the ternary + conditional defaultValue with the same pattern used in RepoCard.tsx: always call t('projects.sessionsShort', { count: sessionCount, defaultValue: sessionCount === 1 ? 'session' : 'sessions' }) (keeping the `${sessionCount} ${...}${lastActivityLabel ? \` · ${lastActivityLabel}\` : ''}` composition) so pluralization is driven by the count param and the code is simplified; update the metaText expression in WorktreeRow.tsx accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/projects.js`:
- Around line 800-805: The synthetic worktreeInfo sets branchName to null which
violates the WorktreeInfo.branchName: string contract; fix by changing the
object literal to use branchName: '' (empty string) instead of null in the
worktreeInfo construction (the object containing isWorktree, worktreeRoot,
mainRepoRoot, branchName), or alternatively update the WorktreeInfo type
declaration to accept string | null (i.e., change the branchName type to allow
null) so the runtime value matches the declared type.
In `@src/components/sidebar/utils/repoAggregates.ts`:
- Around line 33-46: getRepoSessionTotal currently skips adding
cursor/codex/gemini sessions when project.sessionMeta?.total is a number,
causing non-Claude sessions to be undercounted; fix by removing the early
continue and always summing Claude count plus other provider counts.
Specifically, in getRepoSessionTotal compute per-projectTotal =
(project.sessionMeta?.total ?? 0) + (project.sessions?.length ?? 0) +
(project.cursorSessions?.length ?? 0) + (project.codexSessions?.length ?? 0) +
(project.geminiSessions?.length ?? 0) and add that to total (replace the
existing if/continue branch with this combined sum).
In `@src/components/sidebar/view/subcomponents/RecentSessions.tsx`:
- Around line 33-117: The "Show all" button is one-way because showAll is only
set to true; change the toggle to flip showAll (use setShowAll(prev => !prev))
and update the button label to show either t('projects.showAll') with
sessions.length when showAll is false or a "Show less" label (e.g.,
t('projects.showLess', { defaultValue: 'Show less' })) when showAll is true;
keep the button rendered whenever expanded && sessions.length > DEFAULT_VISIBLE
(instead of hidden > 0) so users can collapse back to the compact view; adjust
any text/count shown to reflect the current state and keep visible/hidden
calculation based on showAll and DEFAULT_VISIBLE as before.
In `@src/components/sidebar/view/subcomponents/RepoCard.tsx`:
- Around line 228-245: The lastActivityLabelFor function currently returns "0m
ago" for events under one minute; update lastActivityLabelFor to short-circuit
when minutes < 1 and return t('time.justNow', { defaultValue: 'just now' })
(mirror the behavior in RecentSessions.tsx's formatRelative) before the minutes
< 60 branch—use the existing minutes calculation and keep the rest of the
branches intact.
In `@src/components/sidebar/view/subcomponents/WorktreeRow.tsx`:
- Line 60: The BranchChip fallback currently uses project.displayName which
mislabels non-branch worktrees; update the rendering in WorktreeRow so that if
project.worktreeInfo?.branchName exists you render <BranchChip
branchName={project.worktreeInfo.branchName} /> and otherwise render a plain
<span> (e.g., <span className="worktree-name">{project.displayName}</span>)
instead of BranchChip so the directory/display name is shown without the
Git/Home branch styling or icon; locate the conditional around BranchChip in
WorktreeRow and replace the fallback with a simple span.
In `@src/i18n/locales/de/sidebar.json`:
- Around line 66-67: The German locale contains untranslated tooltip strings:
the keys "newSessionInWorktree" and "deleteWorktree" currently have English
values; update their values to proper German translations (e.g., "Neue Sitzung
in diesem Worktree" for newSessionInWorktree and "Worktree löschen" for
deleteWorktree) so the de sidebar bundle is fully localized.
- Around line 24-35: Several keys in the German locale file are still English
and one key duplicates another with inconsistent casing; update the values for
"recent", "showAll", "emptyWorktree", "staleWorktree", "messages",
"sessionsShort_one", "sessionsShort_other", and "older" to their German
translations (e.g. "recent" -> "Zuletzt"/"Letzte", "showAll" -> "Alle anzeigen",
"emptyWorktree" -> "leer · zum Starten klicken", "staleWorktree" ->
"archiviert", "messages" -> "Nachrichten", "sessionsShort_one" -> "Sitzung",
"sessionsShort_other" -> "Sitzungen", "older" -> "Älter") and remove or
consolidate the duplicate "archived" vs "staleWorktree" so there's a single,
consistently cased source of truth; then audit the same keys in the
corresponding locale files for ja, ko, ru, and zh-CN and provide proper
translations or mark for translator review if missing.
In `@src/i18n/locales/ja/sidebar.json`:
- Around line 27-35: Several sidebar JSON keys remain in English; update the
Japanese translations for the keys "recent", "worktreesUpper", "showAll",
"emptyWorktree", "staleWorktree", "messages", "sessionsShort_one",
"sessionsShort_other", "older", plus "newSessionInWorktree" and "deleteWorktree"
referenced later, replacing the English values with appropriate Japanese strings
so the ja locale is fully localized (you can ignore "sessionsShort_one" for
plural behavior since i18next uses only "other" for ja). Locate those keys in
src/i18n/locales/ja/sidebar.json and provide natural Japanese translations for
each value.
In `@src/i18n/locales/ko/sidebar.json`:
- Around line 27-35: Several sidebar i18n keys in ko/sidebar.json remain in
English (e.g., "recent", "worktreesUpper", "showAll", "emptyWorktree",
"staleWorktree", "messages", "sessionsShort_one", "sessionsShort_other",
"older", plus "newSessionInWorktree" and "deleteWorktree") causing a
mixed-language UI; replace the English values with appropriate Korean
translations for those keys (keep "sessionsShort_one" if you prefer even though
i18next for ko only uses the "other" plural form) so the sidebar is fully
localized—update the values for the listed keys in the JSON (matching the exact
key names) to their Korean equivalents and ensure JSON syntax remains valid.
---
Nitpick comments:
In `@docs/superpowers/specs/2026-04-25-sidebar-worktree-redesign-design.md`:
- Line 29: The fenced code blocks containing ASCII-art and the
"SidebarProjectList.tsx" snippet are currently untagged and trigger markdownlint
MD040; update those fence openings to include a plaintext tag (e.g., ```text or
```plaintext) for the two ASCII-art blocks shown and any other similar plain
fences (the other occurrence noted at the document). Locate the untagged
triple-backtick blocks in the sidebar-worktree-redesign-design.md file and add
the language tag to each opening fence so the linter recognizes them as
non-code/plaintext.
In `@server/projects.js`:
- Around line 299-310: The synthesized worktree path currently concatenates
strings with forward slashes; update the construction to use path.join to
produce a platform-safe path. In the block handling worktreeMarker (variables:
worktreeMarker, markerIdx, parentProjectName, worktreeName) where repoRoot is
obtained from extractProjectDirectory(parentProjectName), replace the literal
`${repoRoot}/.claude/worktrees/${worktreeName}` with a path.join(repoRoot,
'.claude', 'worktrees', worktreeName) result before setting
projectDirectoryCache.set(projectName, ...) and returning it so paths are
correct on Windows and consistent with other uses in this file.
In `@src/components/sidebar/view/subcomponents/BranchChip.tsx`:
- Around line 5-30: PALETTE in BranchChip.tsx is independently sized while
branchChipColorIndex in repoAggregates.ts uses a hardcoded PALETTE_SIZE=5, which
can cause silent fallback to NEUTRAL if they diverge; fix by either importing an
exported palette size (e.g., export PALETTE_SIZE or getPaletteLength) from
repoAggregates and use that to derive the palette or by adding a runtime
assertion at module load in BranchChip.tsx that checks PALETTE.length ===
branchChipColorIndex's modulus constant (referencing PALETTE and
branchChipColorIndex) and throw/log a clear error if they differ so the
relationship cannot silently drift.
In `@src/components/sidebar/view/subcomponents/RecentSessions.tsx`:
- Around line 122-134: formatRelative currently returns pluralized minute/hour
translations (e.g., "1m ago") because it only short-circuits for <1 minute;
update formatRelative to detect the singular cases and call the singular i18n
keys: when minutes === 1 use t('time.oneMinuteAgo', { defaultValue: '1m ago' })
and when hours === 1 use t('time.oneHourAgo', { defaultValue: '1h ago' }),
keeping the existing branches for <1 minute, minutes <60, hours <24 and days;
apply these checks using the same computed values (date from
session.lastActivity || session.createdAt, minutes, hours) so localization uses
the dedicated singular keys.
In `@src/components/sidebar/view/subcomponents/RepoCard.tsx`:
- Around line 158-177: Rendering calls compute getRepoSessionTotal and
lastActivityLabelFor (which itself calls getRepoSessions) for every WorktreeRow
on each render, causing wasted work when currentTime updates; fix by computing a
memoized Map keyed by [activeWorktrees, additionalSessions, currentTime] (e.g.,
worktreeStats: Map<name, { sessionCount, lastActivityLabel }>) using useMemo,
populate it by iterating activeWorktrees and calling
getRepoSessions/getRepoSessionTotal/lastActivityLabelFor once per worktree, then
replace inline calls in the WorktreeRow props (sessionCount, lastActivityLabel)
with lookups like worktreeStats.get(wt.name) so re-renders reuse the precomputed
values.
- Around line 112-122: The current conditional defaultValue on t() is
ineffective for plurals; update the code to use i18next pluralization properly
by adding plural keys (projects.worktrees_one / projects.worktrees_other and
projects.sessionsShort_one / projects.sessionsShort_other) in your locale bundle
and then call t('projects.worktrees', { count: linkedWorktrees.length }) and
t('projects.sessionsShort', { count: sessionTotal }) in RepoCard (same pattern
applies in WorktreeRow.tsx) so you can remove the linkedWorktrees.length === 1
conditional and let i18next pick the correct plural form.
In `@src/components/sidebar/view/subcomponents/WorktreeRow.tsx`:
- Around line 32-39: The metaText uses a ternary defaultValue to pick
singular/plural for t('projects.sessionsShort') but i18next will ignore that
when plural keys exist; replace the ternary + conditional defaultValue with the
same pattern used in RepoCard.tsx: always call t('projects.sessionsShort', {
count: sessionCount, defaultValue: sessionCount === 1 ? 'session' : 'sessions'
}) (keeping the `${sessionCount} ${...}${lastActivityLabel ? \` ·
${lastActivityLabel}\` : ''}` composition) so pluralization is driven by the
count param and the code is simplified; update the metaText expression in
WorktreeRow.tsx accordingly.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: ae73fb08-2f74-4cef-a118-2fb935f5c2f0
📒 Files selected for processing (22)
docs/superpowers/plans/2026-04-25-sidebar-worktree-redesign.mddocs/superpowers/specs/2026-04-25-sidebar-worktree-redesign-design.mdserver/projects.jssrc/components/sidebar/utils/__tests__/repoAggregates.test.tssrc/components/sidebar/utils/repoAggregates.tssrc/components/sidebar/view/Sidebar.tsxsrc/components/sidebar/view/subcomponents/BranchChip.tsxsrc/components/sidebar/view/subcomponents/NewSessionRow.tsxsrc/components/sidebar/view/subcomponents/RecentSessions.tsxsrc/components/sidebar/view/subcomponents/RepoCard.tsxsrc/components/sidebar/view/subcomponents/SidebarProjectItem.tsxsrc/components/sidebar/view/subcomponents/SidebarProjectList.tsxsrc/components/sidebar/view/subcomponents/SidebarProjectSessions.tsxsrc/components/sidebar/view/subcomponents/SidebarSessionItem.tsxsrc/components/sidebar/view/subcomponents/WorktreeRow.tsxsrc/hooks/useProjectsState.tssrc/i18n/locales/de/sidebar.jsonsrc/i18n/locales/en/sidebar.jsonsrc/i18n/locales/ja/sidebar.jsonsrc/i18n/locales/ko/sidebar.jsonsrc/i18n/locales/ru/sidebar.jsonsrc/i18n/locales/zh-CN/sidebar.json
💤 Files with no reviewable changes (3)
- src/components/sidebar/view/subcomponents/SidebarSessionItem.tsx
- src/components/sidebar/view/subcomponents/SidebarProjectSessions.tsx
- src/components/sidebar/view/subcomponents/SidebarProjectItem.tsx
✅ Files skipped from review due to trivial changes (2)
- src/i18n/locales/ru/sidebar.json
- docs/superpowers/plans/2026-04-25-sidebar-worktree-redesign.md
🚧 Files skipped from review as they are similar to previous changes (2)
- src/i18n/locales/zh-CN/sidebar.json
- src/components/sidebar/view/subcomponents/SidebarProjectList.tsx
ashalliants
force-pushed
the
feat/worktree-toggle
branch
from
d097804 to
a64785c
Compare
ashalliants
force-pushed
the
feat/worktree-toggle
branch
3 times, most recently
from
6078b30 to
b09f081
Compare
Open
ashalliants
force-pushed
the
feat/worktree-toggle
branch
from
b09f081 to
d4a1b95
Compare
Member
|
hey @ashalliants, was gonna merge it now but it had merge conflicts. Can you update the PR and I'll make sure to review it. |
Introduces per-session git worktree control and a redesigned sidebar that surfaces worktrees as first-class citizens of their parent repo. Worktrees: - Per-session worktree toggle in the new-session flow - Server-side worktree detection via git rev-parse, falling back to the .claude/worktrees/<name> path convention - Repo-grouping logic that synthesizes a parent project for repos that have only worktrees discovered so far - Cleanup hook removes the git worktree directory on project delete - Fast-path resolves a worktree's directory from its encoded project name (preserving dashes in repo names like "developer-tracker"), bypassing cwd-vote heuristics that subagent cwds would otherwise poison Sidebar redesign (docs/superpowers/specs/2026-04-25-sidebar-worktree- redesign-design.md, /plans/2026-04-25-sidebar-worktree-redesign.md): - Repo header (RepoCard) with current-branch chip, replacing the three-level RepoGroup → ProjectItem → Sessions nesting - "+ New session" full-width primary CTA at top of expanded body - RECENT section: recency-sorted sessions across main + worktrees with branch chips; "Show all N" inline expansion - WORKTREES section: collapsible, with active worktrees plus a separate "Archived (N)" sub-toggle for stale ones - BranchChip primitive with deterministic per-branch color and Home/GitBranch icon variants for main vs worktree origin - Hover-revealed "+" (new session here) and trash (delete worktree) on each worktree row - Header click toggles expansion only — no implicit project select, so mobile users can browse without losing the sidebar - Single-worktree projects render with no WORKTREES section Tests: - src/components/sidebar/utils/__tests__/repoAggregates.test.ts - server/__tests__/worktree-grouping.test.js - server/__tests__/claude-sdk.test.js - src/components/chat/utils/__tests__/chatStorage.test.ts
- WorktreeRow: don't fall back to displayName for the branch chip so stale worktrees no longer render a non-branch label as a branch - RepoCard: add a just-now branch to lastActivityLabelFor so sub-minute events stop showing "0m ago" (parity with RecentSessions.formatRelative) - RecentSessions: add a "Show less" affordance to collapse an expanded session list back to the top 5 - i18n: translate the new sidebar strings (recent, showAll, emptyWorktree, staleWorktree, messages, sessionsShort, older, worktreesUpper) for de, ja, ko, ru and zh-CN; add showLess across all locales Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The /git/pull and /git/push catch blocks matched against error.message, but spawnAsync puts git's actual output on error.stderr — error.message is only "Command failed: git <cmd>". As a result none of the friendly branches (CONFLICT, rejected, divergent branches, etc.) ever fired, and the UI always showed a generic "Pull failed"/"Push failed" with an unhelpful detail string. - Add gitErrorText() to combine stderr/stdout/message for matching - Match against that combined output in both handlers - Default the detail to git's stderr so even unmatched errors are useful - Catch both "diverged" and "divergent" and give actionable guidance (rebase/merge) for divergent branches Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ashalliants
force-pushed
the
feat/worktree-toggle
branch
from
fbfd344 to
e4b3e10
Compare
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
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
--worktreeflag and SDKsettings.worktree)--worktree,--dangerously-skip-permissions) through to the shell/PTY provider, with safety guards to avoid creating duplicate worktrees on resumeCLAUDECODEenv varChanges
Worktree Toggle (Settings & SDK):
useWorktree: booleantoClaudePermissionsStateandClaudeSettingsinterfacesgetClaudeSettings()defaults and parsing to includeuseWorktreeuseWorktreeto SDKsettings: { worktree: {} }inmapCliOptionsToSDK()GitBranchicon (blue info styling)Sidebar Worktree Grouping:
/.claude/worktrees/) and groups them under their parent repoSidebarRepoGroupcomponent with expandable headers, branch badges, and group actionsShell/PTY CLI Flags:
claude --worktree --dangerously-skip-permissionsflag string fromclaudeSettings--worktreefor new sessions and when not already inside a worktree directorygit worktree removewhen deleting a worktree project, with confirmation dialogResizable Sidebar & UX:
SDK Nested-Session Fix:
CLAUDECODEenv var inserver/load-env.jsat startupi18n:
permissions.useWorktree.labelandpermissions.useWorktree.descriptionkeys for all 6 locales (en, de, ja, ko, ru, zh-CN)Testing:
mapCliOptionsToSDK(worktree mapping + existing SDK option behavior)getClaudeSettings(localStorage parsing includinguseWorktreedefaults)Summary by CodeRabbit
Release Notes
New Features
Improvements
Internationalization
Tests
Test plan