Fix: Delete Worktree Crash + PR Comments + Dev Server UX Improvements#792
Conversation
📝 WalkthroughWalkthroughRefactors PR review comment fetching with cursor-based GraphQL pagination and bot detection; scopes sessions and agent/session UI to worktrees via a new Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(30,144,255,0.5)
participant Caller
end
rect rgba(60,179,113,0.5)
participant PRService
end
rect rgba(255,99,71,0.5)
participant GitHubAPI
end
Caller->>PRService: fetchReviewThreadResolvedStatus(prNumber)
PRService->>GitHubAPI: executeGraphQL(query, cursor=null)
GitHubAPI-->>PRService: threads + pageInfo(endCursor, hasNextPage)
alt hasNextPage and under MAX_PAGINATION_PAGES
PRService->>GitHubAPI: executeGraphQL(query, cursor=endCursor)
end
alt reached MAX_PAGINATION_PAGES
PRService-->>Caller: emit pagination cap warning
end
PRService-->>Caller: aggregated resolved status per comment
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
Summary of ChangesHello @gsxdsm, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the application's stability and user experience by addressing a critical crash related to worktree deletion and introducing several quality-of-life improvements. It refines the handling of PR review comments by improving data fetching and UI presentation, introduces worktree-specific session management for the AI agent, and streamlines the developer server interaction with better feedback and log visibility. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This is a substantial pull request that delivers several valuable bug fixes and quality-of-life improvements. The fix for the worktree deletion crash is a critical stability improvement. The enhancements to PR review comments, especially pagination and bot support, are excellent additions. Scoping agent sessions per worktree and the various dev server UX improvements will significantly enhance the developer experience. The code is well-structured, and the changes are clearly explained.
I've identified one critical issue related to fetching paginated GitHub comments that could cause failures. Please see the specific comment for details.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/server/src/services/pr-review-comments.service.ts (2)
263-299:⚠️ Potential issue | 🟡 MinorMark GitHub App comments as bots for accurate badges.
Whenperformed_via_github_appis present anduseris null,isBotstays false, so app comments won’t show the Bot badge.🐛 Suggested fix
- isBot: c.user?.type === 'Bot', + isBot: c.user?.type === 'Bot' || !!c.performed_via_github_app,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/server/src/services/pr-review-comments.service.ts` around lines 263 - 299, In the regularComments mapping inside pr-review-comments.service.ts update the isBot logic so GitHub App comments are treated as bots: replace isBot: c.user?.type === 'Bot' with a conditional that marks the comment as a bot when c.user is present and c.user.type === 'Bot', or when c.user is null/undefined but c.performed_via_github_app is truthy (e.g. isBot: c.user ? c.user.type === 'Bot' : !!c.performed_via_github_app); this ensures comments with performed_via_github_app show the Bot badge.
250-257:⚠️ Potential issue | 🟠 MajorEmit events for PR review comment fetching in the route handler.
The
fetchPRReviewComments()service function performs remote API calls, but neither the service nor the route handler emit any events viacreateEventEmitter()fromlib/events.ts. Per the coding guidelines, all server operations should emit events that stream to the frontend via WebSocket. Add start/success/failure event emissions increateListPRReviewCommentsHandler()to allow the frontend to observe fetch progress.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/server/src/services/pr-review-comments.service.ts` around lines 250 - 257, The route handler createListPRReviewCommentsHandler currently calls fetchPRReviewComments but does not emit progress events; update createListPRReviewCommentsHandler to use createEventEmitter() to emit a "pr:comments:fetch:start" event before calling fetchPRReviewComments(projectPath, owner, repo, prNumber), emit "pr:comments:fetch:success" with the returned PRReviewComment[] on successful completion, and emit "pr:comments:fetch:failure" with the error details if fetchPRReviewComments throws; ensure the emitter is created/closed per request and include contextual identifiers (projectPath, owner, repo, prNumber) in the event payloads so the frontend can correlate events.apps/ui/src/components/session-manager.tsx (1)
182-227:⚠️ Potential issue | 🟠 MajorWrap
handleQuickCreateSessioninuseCallbackto capture current worktree directory.
onQuickCreateRefis updated only whenprojectPathchanges, buteffectiveWorkingDirectorycan change independently via the optionalworkingDirectoryprop. When switching worktrees without changingprojectPath, the ref points to a stale closure with the previous worktree path, causing quick-create to use the wrong directory.Suggested fix
- const handleQuickCreateSession = async () => { + const handleQuickCreateSession = useCallback(async () => { const api = getElectronAPI(); if (!api?.sessions) return; const sessionName = generateRandomSessionName(); const result = await api.sessions.create(sessionName, projectPath, effectiveWorkingDirectory); if (result.success && result.session?.id) { await invalidateSessions(); onSelectSession(result.session.id); } - }; + }, [effectiveWorkingDirectory, projectPath, invalidateSessions, onSelectSession]); // Expose the quick create function via ref for keyboard shortcuts useEffect(() => { if (onQuickCreateRef) { onQuickCreateRef.current = handleQuickCreateSession; } return () => { if (onQuickCreateRef) { onQuickCreateRef.current = null; } }; - }, [onQuickCreateRef, projectPath]); + }, [onQuickCreateRef, handleQuickCreateSession]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ui/src/components/session-manager.tsx` around lines 182 - 227, The quick-create handler currently closes over effectiveWorkingDirectory and becomes stale; wrap handleQuickCreateSession in useCallback and include effectiveWorkingDirectory (or workingDirectory and projectPath) in its dependency array so it always captures the current worktree, then change the useEffect to assign onQuickCreateRef.current = handleQuickCreateSession and depend on handleQuickCreateSession instead of projectPath; ensure cleanup sets current = null as before.
🧹 Nitpick comments (1)
apps/ui/src/components/views/agent-view/components/agent-header.tsx (1)
37-48: Consider truncating long branch names in the badge.
Long branch names can stretch the header; a max-width + truncate keeps layout stable.🧩 Suggested tweak
- {worktreeBranch && ( - <span className="inline-flex items-center gap-1 text-xs bg-muted/50 px-2 py-0.5 rounded-full border border-border"> - <GitBranch className="w-3 h-3" /> - {worktreeBranch} - </span> - )} + {worktreeBranch && ( + <span className="inline-flex items-center gap-1 text-xs bg-muted/50 px-2 py-0.5 rounded-full border border-border"> + <GitBranch className="w-3 h-3" /> + <span className="max-w-[180px] truncate">{worktreeBranch}</span> + </span> + )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ui/src/components/views/agent-view/components/agent-header.tsx` around lines 37 - 48, The worktreeBranch badge can overflow; update the span that renders worktreeBranch (the element inside the conditional that includes GitBranch and {worktreeBranch}) to constrain long branch names by adding a max-width and text truncation classes/styles (e.g., a utility class like max-w-[120px] plus truncate or equivalent CSS) so the badge keeps its rounded layout; ensure the GitBranch icon remains visible and the branch text uses overflow-ellipsis/white-space-nowrap to show a single-line truncated name.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@apps/server/src/services/pr-review-comments.service.ts`:
- Around line 263-299: In the regularComments mapping inside
pr-review-comments.service.ts update the isBot logic so GitHub App comments are
treated as bots: replace isBot: c.user?.type === 'Bot' with a conditional that
marks the comment as a bot when c.user is present and c.user.type === 'Bot', or
when c.user is null/undefined but c.performed_via_github_app is truthy (e.g.
isBot: c.user ? c.user.type === 'Bot' : !!c.performed_via_github_app); this
ensures comments with performed_via_github_app show the Bot badge.
- Around line 250-257: The route handler createListPRReviewCommentsHandler
currently calls fetchPRReviewComments but does not emit progress events; update
createListPRReviewCommentsHandler to use createEventEmitter() to emit a
"pr:comments:fetch:start" event before calling
fetchPRReviewComments(projectPath, owner, repo, prNumber), emit
"pr:comments:fetch:success" with the returned PRReviewComment[] on successful
completion, and emit "pr:comments:fetch:failure" with the error details if
fetchPRReviewComments throws; ensure the emitter is created/closed per request
and include contextual identifiers (projectPath, owner, repo, prNumber) in the
event payloads so the frontend can correlate events.
In `@apps/ui/src/components/session-manager.tsx`:
- Around line 182-227: The quick-create handler currently closes over
effectiveWorkingDirectory and becomes stale; wrap handleQuickCreateSession in
useCallback and include effectiveWorkingDirectory (or workingDirectory and
projectPath) in its dependency array so it always captures the current worktree,
then change the useEffect to assign onQuickCreateRef.current =
handleQuickCreateSession and depend on handleQuickCreateSession instead of
projectPath; ensure cleanup sets current = null as before.
---
Nitpick comments:
In `@apps/ui/src/components/views/agent-view/components/agent-header.tsx`:
- Around line 37-48: The worktreeBranch badge can overflow; update the span that
renders worktreeBranch (the element inside the conditional that includes
GitBranch and {worktreeBranch}) to constrain long branch names by adding a
max-width and text truncation classes/styles (e.g., a utility class like
max-w-[120px] plus truncate or equivalent CSS) so the badge keeps its rounded
layout; ensure the GitBranch icon remains visible and the branch text uses
overflow-ellipsis/white-space-nowrap to show a single-line truncated name.
- Include GitHub app-initiated comments in bot detection - Wrap handleQuickCreateSession with useCallback to fix dependency issues - Truncate long branch names in agent header to prevent layout overflow
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/ui/src/components/session-manager.tsx (1)
300-310:⚠️ Potential issue | 🟠 MajorBug: fallback after session deletion uses unscoped
sessionsinstead ofscopedSessions.When the currently-selected session is deleted, the fallback on Line 303 filters the global
sessionslist. Since the rest of the component now scopes to the current worktree viascopedSessions, this can select a session belonging to a different worktree.🐛 Proposed fix
if (currentSessionId === sessionId) { // Switch to another session or create a new one - const activeSessionsList = sessions.filter((s) => !s.isArchived); + const activeSessionsList = scopedSessions.filter((s) => !s.isArchived && s.id !== sessionId); if (activeSessionsList.length > 0) { onSelectSession(activeSessionsList[0].id); + } else { + onSelectSession(null); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ui/src/components/session-manager.tsx` around lines 300 - 310, The deletion fallback currently uses the global sessions list which can pick a session from the wrong worktree; update the logic inside the delete handler (the block that calls invalidateSessions and checks if currentSessionId === sessionId) to use scopedSessions instead of sessions (i.e., replace sessions.filter(...) with scopedSessions.filter(...)) so the chosen active session comes from the same worktree; keep the rest of the flow (onSelectSession and setSessionToDelete(null)) unchanged.apps/server/src/services/pr-review-comments.service.ts (1)
348-356:⚠️ Potential issue | 🟡 MinorInconsistent bot detection:
performed_via_github_appnot checked for inline review comments or review body comments.Section 1 (issue comments, line 298) detects bots with
c.user?.type === 'Bot' || !!c.performed_via_github_app, but sections 2 and 3 only checkc.user?.type === 'Bot'. The GitHub REST API also returnsperformed_via_github_appon pull request review comments and reviews, so app-initiated review comments won't be flagged as bot here.♻️ Suggested fix for inline review comments (similar for review body)
- isBot: c.user?.type === 'Bot', + isBot: c.user?.type === 'Bot' || !!c.performed_via_github_app,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/server/src/services/pr-review-comments.service.ts` around lines 348 - 356, The isBot check for inline review comments (the object being built with properties isOutdated, isResolved, isBot, diffHunk, side, commitId) is missing the performed_via_github_app check; update the isBot assignment (currently c.user?.type === 'Bot') to match the issue-comments logic by using c.user?.type === 'Bot' || !!c.performed_via_github_app so app-created review comments and review body comments are flagged as bots as well.
🧹 Nitpick comments (2)
apps/ui/src/components/views/agent-view/components/agent-header.tsx (1)
42-47: Add atitleattribute so truncated branch names are visible on hover.With
max-w-[180px] truncate, long branch names get clipped. Atitleon the outer<span>lets users hover to see the full name.💡 Suggested improvement
{worktreeBranch && ( - <span className="inline-flex items-center gap-1 text-xs bg-muted/50 px-2 py-0.5 rounded-full border border-border"> + <span + className="inline-flex items-center gap-1 text-xs bg-muted/50 px-2 py-0.5 rounded-full border border-border" + title={worktreeBranch} + > <GitBranch className="w-3 h-3 shrink-0" /> <span className="max-w-[180px] truncate">{worktreeBranch}</span> </span>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ui/src/components/views/agent-view/components/agent-header.tsx` around lines 42 - 47, The branch name span in the AgentHeader component clips long names due to max-w-[180px] truncate; update the outer conditional span (the one wrapping GitBranch and the {worktreeBranch} text) to include a title attribute set to worktreeBranch so the full branch name is visible on hover (refer to the render that checks worktreeBranch in agent-header.tsx and the span containing GitBranch and the truncated {worktreeBranch}).apps/ui/src/components/session-manager.tsx (1)
182-192: Consider guardingeffectiveWorkingDirectoryagainst empty strings.If both
workingDirectoryandprojectPathare empty or undefined (e.g., during initialization),effectiveWorkingDirectorywould be""(orundefined), causingscopedSessionsto behave unpredictably and thecreatecall to receive an invalid path. A defensive check may be worthwhile.💡 Suggested guard
const effectiveWorkingDirectory = workingDirectory || projectPath; + + // Safety: if neither path is available, bail out of session operations + if (!effectiveWorkingDirectory) { + logger.warn('No working directory or project path available'); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ui/src/components/session-manager.tsx` around lines 182 - 192, The computed effectiveWorkingDirectory used in handleCreateSession can end up as an empty string which breaks scopedSessions and api.sessions.create; update the logic in handleCreateSession (or where effectiveWorkingDirectory is defined) to defensively normalize/guard empty values — e.g., use trimmed checks on workingDirectory and projectPath, and set effectiveWorkingDirectory to undefined (or null) when both are empty so you never pass "" into api.sessions.create; ensure references in this change include effectiveWorkingDirectory, handleCreateSession, workingDirectory, projectPath and the call to api.sessions.create so reviewers can locate the fix.
🤖 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/server/src/services/pr-review-comments.service.ts`:
- Around line 106-151: The executeGraphQL function can crash if gh.stdin emits
an 'error' when the child is killed; add an 'error' listener on gh.stdin (e.g.
gh.stdin.on('error', (err) => {/* noop or optionally log */})) before calling
gh.stdin.write(requestBody) so the stream error is handled; ensure the listener
is attached early (after spawn and before write) and can be left as a no-op or
used to reject the enclosing Promise if you prefer to propagate the error.
---
Outside diff comments:
In `@apps/server/src/services/pr-review-comments.service.ts`:
- Around line 348-356: The isBot check for inline review comments (the object
being built with properties isOutdated, isResolved, isBot, diffHunk, side,
commitId) is missing the performed_via_github_app check; update the isBot
assignment (currently c.user?.type === 'Bot') to match the issue-comments logic
by using c.user?.type === 'Bot' || !!c.performed_via_github_app so app-created
review comments and review body comments are flagged as bots as well.
In `@apps/ui/src/components/session-manager.tsx`:
- Around line 300-310: The deletion fallback currently uses the global sessions
list which can pick a session from the wrong worktree; update the logic inside
the delete handler (the block that calls invalidateSessions and checks if
currentSessionId === sessionId) to use scopedSessions instead of sessions (i.e.,
replace sessions.filter(...) with scopedSessions.filter(...)) so the chosen
active session comes from the same worktree; keep the rest of the flow
(onSelectSession and setSessionToDelete(null)) unchanged.
---
Nitpick comments:
In `@apps/ui/src/components/session-manager.tsx`:
- Around line 182-192: The computed effectiveWorkingDirectory used in
handleCreateSession can end up as an empty string which breaks scopedSessions
and api.sessions.create; update the logic in handleCreateSession (or where
effectiveWorkingDirectory is defined) to defensively normalize/guard empty
values — e.g., use trimmed checks on workingDirectory and projectPath, and set
effectiveWorkingDirectory to undefined (or null) when both are empty so you
never pass "" into api.sessions.create; ensure references in this change include
effectiveWorkingDirectory, handleCreateSession, workingDirectory, projectPath
and the call to api.sessions.create so reviewers can locate the fix.
In `@apps/ui/src/components/views/agent-view/components/agent-header.tsx`:
- Around line 42-47: The branch name span in the AgentHeader component clips
long names due to max-w-[180px] truncate; update the outer conditional span (the
one wrapping GitBranch and the {worktreeBranch} text) to include a title
attribute set to worktreeBranch so the full branch name is visible on hover
(refer to the render that checks worktreeBranch in agent-header.tsx and the span
containing GitBranch and the truncated {worktreeBranch}).
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/ui/src/components/session-manager.tsx (1)
294-310:⚠️ Potential issue | 🟠 MajorStale closure + deleted session not excluded from next-session candidates.
confirmDeleteSessioncapturesscopedSessionsfrom the enclosing render. Afterawait invalidateSessions()(which awaits a full refetch), the component schedules a re-render, but the still-executing async closure holds the oldscopedSessions— one that still contains the just-deleted session. The filter at line 303 also does not excludesessionId, so if the deleted session is at index 0 (e.g., it was the only active session),onSelectSessionis called with the deleted session's ID.🐛 Proposed fix — exclude the deleted session from the candidate list
- const activeSessionsList = scopedSessions.filter((s) => !s.isArchived); + const activeSessionsList = scopedSessions.filter( + (s) => !s.isArchived && s.id !== sessionId + );Alternatively, use the fresh data returned from
invalidateSessionsto avoid the stale-closure issue entirely:const confirmDeleteSession = async (sessionId: string) => { const api = getElectronAPI(); if (!api?.sessions) return; const result = await api.sessions.delete(sessionId); if (result.success) { - await invalidateSessions(); + const freshSessions = await invalidateSessions(); if (currentSessionId === sessionId) { - const activeSessionsList = scopedSessions.filter((s) => !s.isArchived); + const freshScoped = (freshSessions ?? []).filter((s) => { + const dir = s.workingDirectory || s.projectPath; + return pathsEqual(dir, effectiveWorkingDirectory) && !s.isArchived && s.id !== sessionId; + }); - if (activeSessionsList.length > 0) { - onSelectSession(activeSessionsList[0].id); + if (freshScoped.length > 0) { + onSelectSession(freshScoped[0].id); } } } setSessionToDelete(null); };(requires
invalidateSessionsto return the refetched data)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ui/src/components/session-manager.tsx` around lines 294 - 310, confirmDeleteSession currently uses the stale scopedSessions captured by the async closure and its candidate filter doesn't exclude the deleted session; update confirmDeleteSession to use fresh session data after invalidation (e.g., await the refetch result returned from invalidateSessions or read the latest sessions state/ref after invalidateSessions completes) and when building the activeSessionsList filter exclude the deleted sessionId (use filter(s => !s.isArchived && s.id !== sessionId)) before calling onSelectSession; keep setSessionToDelete(null) at the end.
🧹 Nitpick comments (1)
apps/server/src/services/pr-review-comments.service.ts (1)
266-310: Consider extracting the repeated REST fetch-parse-map pattern into a shared helper.All three comment-fetching blocks (regular issue comments, inline review comments, review body comments) follow an identical structure:
- Build
gh api <endpoint> --paginateargsexecFileAsyncwith the samecwd,env,maxBuffer,timeoutJSON.parsethe output- Guard with
Array.isArray(...) ? ... : [].filter/.maptoPRReviewCommentpushintoallCommentscatch→logErrorA small helper like
fetchGitHubRestPages<T>(projectPath, endpoint): Promise<T[]>would eliminate ~30 lines of duplication and make it easier to adjust buffer sizes or timeouts uniformly in the future.♻️ Sketch of a potential helper
async function fetchGitHubRestPages<T>(projectPath: string, endpoint: string): Promise<T[]> { const { stdout } = await execFileAsync( 'gh', ['api', endpoint, '--paginate'], { cwd: projectPath, env: execEnv, maxBuffer: 1024 * 1024 * 10, timeout: GITHUB_API_TIMEOUT_MS, } ); const data = JSON.parse(stdout); return Array.isArray(data) ? data : []; }Each call site would then just do:
const raw = await fetchGitHubRestPages<IssueComment>(projectPath, `repos/${owner}/${repo}/issues/${prNumber}/comments`); const regularComments = raw.map(c => ({ /* mapping */ }));Also applies to: 312-366, 368-415
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/server/src/services/pr-review-comments.service.ts` around lines 266 - 310, Extract the repeated REST fetch/parse logic into a helper like fetchGitHubRestPages<T>(projectPath: string, endpoint: string): Promise<T[]> that calls execFileAsync('gh', ['api', endpoint, '--paginate'], { cwd: projectPath, env: execEnv, maxBuffer: 1024*1024*10, timeout: GITHUB_API_TIMEOUT_MS }), JSON.parses stdout and returns Array.isArray(data) ? data : []; then replace the three duplicated blocks that build endpoints and call execFileAsync (the regular issue comments, inline review comments, and review body comments) to instead call fetchGitHubRestPages with the same endpoint and map the returned items to your PRReviewComment shape before pushing into allComments; keep the existing try/catch semantics at each call site (use logError on failure) so error handling and context remain identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@apps/ui/src/components/session-manager.tsx`:
- Around line 294-310: confirmDeleteSession currently uses the stale
scopedSessions captured by the async closure and its candidate filter doesn't
exclude the deleted session; update confirmDeleteSession to use fresh session
data after invalidation (e.g., await the refetch result returned from
invalidateSessions or read the latest sessions state/ref after
invalidateSessions completes) and when building the activeSessionsList filter
exclude the deleted sessionId (use filter(s => !s.isArchived && s.id !==
sessionId)) before calling onSelectSession; keep setSessionToDelete(null) at the
end.
---
Nitpick comments:
In `@apps/server/src/services/pr-review-comments.service.ts`:
- Around line 266-310: Extract the repeated REST fetch/parse logic into a helper
like fetchGitHubRestPages<T>(projectPath: string, endpoint: string):
Promise<T[]> that calls execFileAsync('gh', ['api', endpoint, '--paginate'], {
cwd: projectPath, env: execEnv, maxBuffer: 1024*1024*10, timeout:
GITHUB_API_TIMEOUT_MS }), JSON.parses stdout and returns Array.isArray(data) ?
data : []; then replace the three duplicated blocks that build endpoints and
call execFileAsync (the regular issue comments, inline review comments, and
review body comments) to instead call fetchGitHubRestPages with the same
endpoint and map the returned items to your PRReviewComment shape before pushing
into allComments; keep the existing try/catch semantics at each call site (use
logError on failure) so error handling and context remain identical.
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 `@apps/ui/src/components/session-manager.tsx`:
- Around line 301-308: The next-session selection after deleting the current
session uses unscoped freshSessions from invalidateSessions(), which can pick
sessions from other worktrees; update the logic in the block using
currentSessionId/sessionId to first filter freshSessions by the current worktree
the same way scopedSessions does (use pathsEqual to compare session.path against
the current worktree path), then compute activeSessionsList from that scoped set
(exclude isArchived and the deleted session), and if none remain call
onSelectSession(null) so the selection is cleared instead of leaving a stale
selection; ensure you reference/integrate the existing pathsEqual helper and the
onSelectSession/invalidateSessions variables in the change.
| const refetchResult = await invalidateSessions(); | ||
| if (currentSessionId === sessionId) { | ||
| // Switch to another session or create a new one | ||
| const activeSessionsList = sessions.filter((s) => !s.isArchived); | ||
| // Switch to another session using fresh data, excluding the deleted session | ||
| const freshSessions = refetchResult?.data ?? []; | ||
| const activeSessionsList = freshSessions.filter((s) => !s.isArchived && s.id !== sessionId); | ||
| if (activeSessionsList.length > 0) { | ||
| onSelectSession(activeSessionsList[0].id); | ||
| } |
There was a problem hiding this comment.
Next-session selection after delete ignores worktree scope.
After deleting the currently selected session, freshSessions is the unscoped list from the query cache, so activeSessionsList may contain sessions from other worktrees. The user could be auto-switched to a session that doesn't even appear in their current (worktree-scoped) session list.
Apply the same pathsEqual filtering used for scopedSessions:
🐛 Proposed fix — scope next-session selection to the current worktree
const refetchResult = await invalidateSessions();
if (currentSessionId === sessionId) {
// Switch to another session using fresh data, excluding the deleted session
const freshSessions = refetchResult?.data ?? [];
- const activeSessionsList = freshSessions.filter((s) => !s.isArchived && s.id !== sessionId);
+ const activeSessionsList = freshSessions.filter((s) => {
+ if (s.isArchived || s.id === sessionId) return false;
+ const sessionDir = s.workingDirectory || s.projectPath;
+ return pathsEqual(sessionDir, effectiveWorkingDirectory);
+ });
if (activeSessionsList.length > 0) {
onSelectSession(activeSessionsList[0].id);
+ } else {
+ onSelectSession(null);
}
}Also note that the current code silently does nothing when no active sessions remain (falls through without calling onSelectSession(null)), leaving a stale selection. The else branch above handles that.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const refetchResult = await invalidateSessions(); | |
| if (currentSessionId === sessionId) { | |
| // Switch to another session or create a new one | |
| const activeSessionsList = sessions.filter((s) => !s.isArchived); | |
| // Switch to another session using fresh data, excluding the deleted session | |
| const freshSessions = refetchResult?.data ?? []; | |
| const activeSessionsList = freshSessions.filter((s) => !s.isArchived && s.id !== sessionId); | |
| if (activeSessionsList.length > 0) { | |
| onSelectSession(activeSessionsList[0].id); | |
| } | |
| const refetchResult = await invalidateSessions(); | |
| if (currentSessionId === sessionId) { | |
| // Switch to another session using fresh data, excluding the deleted session | |
| const freshSessions = refetchResult?.data ?? []; | |
| const activeSessionsList = freshSessions.filter((s) => { | |
| if (s.isArchived || s.id === sessionId) return false; | |
| const sessionDir = s.workingDirectory || s.projectPath; | |
| return pathsEqual(sessionDir, effectiveWorkingDirectory); | |
| }); | |
| if (activeSessionsList.length > 0) { | |
| onSelectSession(activeSessionsList[0].id); | |
| } else { | |
| onSelectSession(null); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/ui/src/components/session-manager.tsx` around lines 301 - 308, The
next-session selection after deleting the current session uses unscoped
freshSessions from invalidateSessions(), which can pick sessions from other
worktrees; update the logic in the block using currentSessionId/sessionId to
first filter freshSessions by the current worktree the same way scopedSessions
does (use pathsEqual to compare session.path against the current worktree path),
then compute activeSessionsList from that scoped set (exclude isArchived and the
deleted session), and if none remain call onSelectSession(null) so the selection
is cleared instead of leaving a stale selection; ensure you reference/integrate
the existing pathsEqual helper and the onSelectSession/invalidateSessions
variables in the change.
…AutoMaker-Org#792) * Changes from fix/delete-worktree-hotifx * fix: Improve bot detection and prevent UI overflow issues - Include GitHub app-initiated comments in bot detection - Wrap handleQuickCreateSession with useCallback to fix dependency issues - Truncate long branch names in agent header to prevent layout overflow * feat: Support GitHub App comments in PR review and fix session filtering * feat: Return invalidation result from delete session handler
* Changes from fix/docker-compose-cors-error * Update apps/server/src/index.ts Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * Fix: Delete Worktree Crash + PR Comments + Dev Server UX Improvements (#792) * Changes from fix/delete-worktree-hotifx * fix: Improve bot detection and prevent UI overflow issues - Include GitHub app-initiated comments in bot detection - Wrap handleQuickCreateSession with useCallback to fix dependency issues - Truncate long branch names in agent header to prevent layout overflow * feat: Support GitHub App comments in PR review and fix session filtering * feat: Return invalidation result from delete session handler * fix: Improve CORS origin validation to handle wildcard correctly * fix: Correct IPv6 localhost parsing and improve responsive UI layouts * Changes from fix/pwa-cache-fix (#794) * fix: Add type checking to prevent crashes from malformed cache entries --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
1 participant
Overview
This PR fixes a crash when deleting the currently-selected worktree and delivers several quality-of-life improvements across PR review comments, session scoping, and dev server UX.
Bug Fixes
Worktree Deletion Crash (Primary Fix)
board-view.tsx: When the active worktree is deleted, immediately reset the selected worktree tomainto prevent the UI from trying to render a view for a non-existent worktreeboard-view.tsx: Added a fallback inactiveWorktreeresolution — if the selected worktree path no longer exists (e.g. just deleted), fall back to the main worktree instead of returningundefinedFeature Duplication Regression
use-board-actions.ts: StripinitialStatusandworkModetransient fields when duplicating features, preventing duplicates from incorrectly appearing as "In Progress" instead of "Backlog"PR Review Comments Improvements
Full Pagination Support (
pr-review-comments.service.ts)executeGraphQL()helper to eliminate duplicatedghsubprocess/timeout/JSON-parse logicMAX_PAGINATION_PAGES = 20safety limit (~2,000 threads max) to prevent infinite loopsendCursorto GraphQL response typesBot Comment Support
gh pr view --json comments(GraphQL, drops bot authors) togh api repos/.../issues/.../comments(REST, preserves bot/app author info)isBotfield toPRReviewCommenttype — propagated toelectron.tsand rendered as a purple "Bot" badge in the dialogunknownReview Body Comments (New)
UI Polish (
pr-comment-resolution-dialog.tsx)cursor-defaultfor short comments)pl-5) from comment body and date rowspr-10)Session Scoping per Worktree
libs/types/src/session.ts&electron.d.ts: AddedworkingDirectoryfield toAgentSession/SessionListItemto track which worktree a session belongs touse-agent-session.ts: Session persistence key is now the worktree path (not the project path), so each worktree remembers its own last-selected session. Switching worktrees now resets the current session and restores the last session for that worktreesession-manager.tsx: Sessions are filtered to only show those matching the current worktree'sworkingDirectory; new sessions are created with the worktree path as the working directoryagent-view.tsx: PasseseffectiveWorkingDirectory(worktree path or project path) down to session management and agent executionagent-header.tsx: Displays the current worktree branch name as a pill badge next to the project nameDev Server UX Improvements
worktree-panel.tsx: Starting a dev server now automatically opens the logs panel so output (including errors) is immediately visibleworktree-actions-dropdown.tsx: "Start/Stop Dev Server" and "View Logs" are now a split button — primary action is start/stop, with a chevron submenu revealing "View Dev Server Logs"use-dev-servers.ts: Toast notifications for dev server start/failure now include descriptive subtitles pointing users to the logs panelSummary by CodeRabbit
New Features
Improvements
Bug Fixes