Auto Lane Chat Recovery#650
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
@copilot review but do not make fixes |
📝 WalkthroughWalkthroughThe PR adds optional project-binding pins across desktop bridge and launch flows, updates draft-launch and PTY session handling to use those pins, and replaces lane auto-name generation on desktop and iOS with deterministic prompt-derived naming. Related tests and prompt instructions were updated. ChangesPinned desktop runtime flows
Deterministic lane naming
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 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 docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/desktop/src/renderer/components/lanes/useLaneWorkSessions.ts`:
- Around line 698-700: `pty.create` already receives `args.pin`, but the same
pin is not being carried into `closePtySession` and `continueCliSession` in
`useLaneWorkSessions`, which causes pinned/remote PTYs to be resolved against
the default project root. Update the hook to store the pin alongside the created
session and pass it through the `pty.dispose` call in `closePtySession` and the
session-resume path in `continueCliSession`, using the existing
`args.pin`/`createArgs` flow as the source of truth.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 49c1eced-7834-4f8d-a3b4-7240ec8b2b41
⛔ Files ignored due to path filters (3)
docs/ARCHITECTURE.mdis excluded by!docs/**docs/features/chat/README.mdis excluded by!docs/**docs/features/chat/composer-and-ui.mdis excluded by!docs/**
📒 Files selected for processing (15)
apps/desktop/src/main/services/chat/agentChatService.tsapps/desktop/src/preload/global.d.tsapps/desktop/src/preload/orchestrationBridge.tsapps/desktop/src/preload/preload.tsapps/desktop/src/renderer/components/chat/AgentChatPane.test.tsxapps/desktop/src/renderer/components/chat/AgentChatPane.tsxapps/desktop/src/renderer/components/lanes/useLaneWorkSessions.tsapps/desktop/src/renderer/components/terminals/cliLaunch.tsapps/desktop/src/renderer/components/terminals/useWorkSessions.tsapps/desktop/src/renderer/lib/draftLaunchJobs.tsapps/desktop/src/renderer/lib/projectConfigCache.tsapps/desktop/src/shared/laneNameFallback.test.tsapps/desktop/src/shared/laneNameFallback.tsapps/ios/ADE/Views/Work/WorkNewChatScreen.swiftapps/ios/ADETests/ADETests.swift
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Prevent auto-opening after a project switch
When a foreground draft launch is pinned and the user switches projects before it completes, shouldAutoOpen still stays true because project switches no longer abort the launch. Warm project surfaces remain mounted, so this stale pane can still call openLaunchedDraftSession and navigate the currently active project to a lane/session that belongs to the previous project; gate the auto-open on the active binding still matching launchBinding and leave the job as a ready/restorable launch otherwise.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ea28dfc415
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b808354dd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/desktop/src/renderer/components/terminals/cliLaunch.ts (1)
44-68: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value
workPtyLaunchPinsByIdcan accumulate stale entries.Pins are only removed via
forgetWorkPtyLaunchPinon the dispose paths. Two leak vectors exist: (1) sessions that end without going throughclosePtySession/stopRuntime(e.g. externalonExit) never clear their entry, and (2)continueCliSessionresume returns a newptyId, sorememberWorkPtyLaunchPin(result, pin)writes the new key while the previousptyIdkey lingers. Entries are small, so this is a slow leak rather than a correctness bug, but a reusedptyIdcould theoretically resolve a stale pin.Consider forgetting the prior key on resume, or pruning on
pty.onExit.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/components/terminals/cliLaunch.ts` around lines 44 - 68, `workPtyLaunchPinsById` is retaining stale pin entries across terminal lifecycle changes. Update `rememberWorkPtyLaunchPin`, `workPtyLaunchPinFor`, and `forgetWorkPtyLaunchPin` so a resumed session from `continueCliSession` clears any previous `ptyId`/session mapping before storing the new one, and add cleanup on `pty.onExit` for paths that do not reach `closePtySession` or `stopRuntime`. Keep the map keyed consistently by `WorkPtyPinLookup` identifiers and ensure the old key is removed whenever a new pin is remembered for the same session.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/desktop/src/renderer/components/lanes/useLaneWorkSessions.ts`:
- Around line 701-704: The lane PTY launch flow in useLaneWorkSessions currently
updates UI state after create without checking whether the pinned project is
still current. Add the same canMutatePinnedProjectUi guard used in
useWorkSessions around the post-launch side effects in the pinned PTY path, so
selectLane and focusSession only run when the project context has not gone
stale. Use the launch/result handling in useLaneWorkSessions as the place to
insert the guard before mutating lane state.
---
Nitpick comments:
In `@apps/desktop/src/renderer/components/terminals/cliLaunch.ts`:
- Around line 44-68: `workPtyLaunchPinsById` is retaining stale pin entries
across terminal lifecycle changes. Update `rememberWorkPtyLaunchPin`,
`workPtyLaunchPinFor`, and `forgetWorkPtyLaunchPin` so a resumed session from
`continueCliSession` clears any previous `ptyId`/session mapping before storing
the new one, and add cleanup on `pty.onExit` for paths that do not reach
`closePtySession` or `stopRuntime`. Keep the map keyed consistently by
`WorkPtyPinLookup` identifiers and ensure the old key is removed whenever a new
pin is remembered for the same session.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f2abd951-7289-4315-b51c-02a29b4bfc3c
📒 Files selected for processing (17)
apps/desktop/src/main/services/chat/agentChatService.tsapps/desktop/src/preload/global.d.tsapps/desktop/src/preload/preload.test.tsapps/desktop/src/preload/preload.tsapps/desktop/src/renderer/components/chat/AgentChatPane.test.tsxapps/desktop/src/renderer/components/chat/AgentChatPane.tsxapps/desktop/src/renderer/components/lanes/useLaneWorkSessions.test.tsapps/desktop/src/renderer/components/lanes/useLaneWorkSessions.tsapps/desktop/src/renderer/components/terminals/cliLaunch.tsapps/desktop/src/renderer/components/terminals/useWorkSessions.test.tsapps/desktop/src/renderer/components/terminals/useWorkSessions.tsapps/desktop/src/renderer/lib/projectConfigCache.test.tsapps/desktop/src/renderer/lib/projectConfigCache.tsapps/desktop/src/shared/laneNameFallback.test.tsapps/desktop/src/shared/laneNameFallback.tsapps/ios/ADE/Views/Work/WorkNewChatScreen.swiftapps/ios/ADETests/ADETests.swift
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/ios/ADETests/ADETests.swift
- apps/desktop/src/shared/laneNameFallback.ts
- apps/desktop/src/preload/global.d.ts
- apps/desktop/src/renderer/components/chat/AgentChatPane.test.tsx
- apps/ios/ADE/Views/Work/WorkNewChatScreen.swift
- apps/desktop/src/renderer/components/chat/AgentChatPane.tsx
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cc397fe431
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
ADE/apps/desktop/src/renderer/components/chat/AgentChatPane.tsx
Lines 7503 to 7504 in b31a21a
Gate launch errors by the pinned project
When a pinned draft launch fails after the user has switched projects, this path now avoids refreshing the old project but still calls setError as long as the originating job exists. In the Work tab the pane can remain mounted across projectBinding changes, so a failure from the old project can show an error banner in the newly active project with no matching Restore job. Apply the same canRefreshPinnedProject(launchBinding)/current-project guard before mutating the pane error state so failures stay scoped to the originating draft job.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
1 participant
Summary
Describe the change.
What Changed
Key files and behaviors.
Validation
How you tested.
Risks
Anything to watch.
Summary by CodeRabbit
Greptile Summary
This PR changes the mid-launch project-switch handling from "abort and surface a Restorable failure" to "keep running against the originating project's runtime via a captured
OpenProjectBindingpin." Thepinis threaded through all mutating IPC calls (lane create/rename, chat create/send, orchestration bundle allocation, git fetch/listBranches, PTY create/sendToSession/dispose, and project-config get), withcallPinnedRuntimeActionrouting to the correct runtime regardless of which project is currently active in the UI. UI mutations (session refresh, lane refresh, autoOpen) are gated oncanRefreshPinnedProjectso they only fire when the active project still matches the originating one.priorityNamingWords/workPriorityLaneNamingWordsto produce deterministic slugs for the "claude auth login button" pattern. The guards require(auth OR login) AND (login OR UI control), butwordsalways starts with[provider, \"auth\"]regardless of whethermentionsAuthis true, injecting "auth" into names for prompts that only mention "login" (e.g. "Fix the claude login page" →\"claude-auth-login\"instead of something reflecting the actual words).workPtyLaunchPinsByIdmap incliLaunch.tsstores the binding across component unmounts socontinueCliSessionandclosePtySessioncan route through the correct runtime; entries are cleaned up viaforgetWorkPtyLaunchPinafter dispose.restoreDraftLaunchJob— Extracts the restore+dismiss+clearError logic into a callback and adds a "Restore" button to the error banner when the banner's error matches a failed launch job, providing a second restore entry point alongside the job strip button.Confidence Score: 4/5
Safe to merge after fixing the lane-name auth injection; the pinning and project-routing changes are well-tested and the logic is sound.
The
priorityNamingWordsfunction in both TypeScript and Swift startswordswith[provider, "auth"]regardless of whether the prompt actually mentions an auth-related word. A prompt like "Fix the claude login page" has no auth mention, passes both guards (becausementionsLogin=true), and currently produces"claude-auth-login"— silently inserting a word the user never typed. This affects the deterministic lane name that shows immediately in the UI before the AI rename completes, and the Swift port carries the same issue, so the bug ships on both platforms.apps/desktop/src/shared/laneNameFallback.ts and apps/ios/ADE/Views/Work/WorkNewChatScreen.swift need the words initialisation fixed to conditionally include auth; the corresponding tests should add a case that catches the login-without-auth scenario.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[User submits draft launch] --> B{projectBinding present?} B -- yes --> C[Capture launchBinding = projectBinding] B -- no --> D[launchProjectRoot only] C --> E[Start detached launch chain] D --> E E --> F[assertLaunchActive: timeout?] F -- launchTimedOut --> G[Abort: timeout error] F -- ok, launchBinding present --> H[Pass pin to all IPC calls] F -- ok, no binding --> I{Project root drifted?} I -- yes --> J[Abort: LAUNCH_PROJECT_CHANGED_MESSAGE] I -- no --> H H --> K[git.fetch / listBranches with pin] K --> L[lanes.create with pin] L --> M[agentChat.create with pin] M --> N[orchestration.runCreate with pin] N --> O[agentChat.send with pin] O --> P{canRefreshPinnedProject?} P -- pin.key === current binding --> Q[Refresh UI, autoOpen, notify] P -- project switched away --> R[Skip UI mutations, set autoOpen=false] Q --> S[Job status: ready] R --> S S --> T[User opens from job strip or error banner Restore]%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% flowchart TD A[User submits draft launch] --> B{projectBinding present?} B -- yes --> C[Capture launchBinding = projectBinding] B -- no --> D[launchProjectRoot only] C --> E[Start detached launch chain] D --> E E --> F[assertLaunchActive: timeout?] F -- launchTimedOut --> G[Abort: timeout error] F -- ok, launchBinding present --> H[Pass pin to all IPC calls] F -- ok, no binding --> I{Project root drifted?} I -- yes --> J[Abort: LAUNCH_PROJECT_CHANGED_MESSAGE] I -- no --> H H --> K[git.fetch / listBranches with pin] K --> L[lanes.create with pin] L --> M[agentChat.create with pin] M --> N[orchestration.runCreate with pin] N --> O[agentChat.send with pin] O --> P{canRefreshPinnedProject?} P -- pin.key === current binding --> Q[Refresh UI, autoOpen, notify] P -- project switched away --> R[Skip UI mutations, set autoOpen=false] Q --> S[Job status: ready] R --> S S --> T[User opens from job strip or error banner Restore]Comments Outside Diff (1)
apps/ios/ADE/Views/Work/WorkNewChatScreen.swift, line 1370-1383 (link)workRegexContainsandworkRegexMatchesboth calltry? NSRegularExpression(pattern:)on every invocation.workPriorityLaneNamingWordscallsworkRegexContainsup to five times per naming run (one provider scan per candidate, then auth, login, and button checks), andworkCleanPromptForNaminginvokesworkReplacingRegexMatcheswhich also compiles fresh. SinceNSRegularExpressioncompilation is non-trivial, these should be compiled once at file scope (or lazily via a static dictionary) and reused.Prompt To Fix With AI
Reviews (6): Last reviewed commit: "Guard stale pinned chat notifications" | Re-trigger Greptile