Linear Lane Launch Fixes#499
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Owner
Author
|
@copilot review but do not make fixes |
Copilot stopped work on behalf of
arul28 due to an error
June 1, 2026 16:24
📝 WalkthroughWalkthroughThis PR refactors the Linear issue picker UI by extracting display utilities and creating a dedicated modal component, enhances the PTY service to support configurable initial input timeouts for agent chat CLI launches, and adds test environment cleanup for ADE CLI tests. ChangesAgent Chat CLI Initial Input Timeout
Linear Issue Picker UI Refactoring
ADE CLI Test Environment Management
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/components/lanes/CreateLaneDialog.tsx (1)
668-673:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDisable the disconnect action while create/setup is locked.
The "Change issue" button is already disabled for
busy || laneCreated, but theXbutton still clearsselectedLinearIssue. That lets the UI drift away from the in-flight create request, which has already captured the old issue.Suggested fix
<SelectedLinearIssueCard issue={selectedLinearIssue} branchName={selectedLinearBranchName} branchConflict={selectedLinearBranchConflict} onClear={() => setSelectedLinearIssue(null)} + disabled={busy || laneCreated} />function SelectedLinearIssueCard({ issue, branchName, branchConflict, onClear, + disabled = false, }: { issue: LaneLinearIssue; branchName: string; branchConflict: boolean; onClear: () => void; + disabled?: boolean; }) { return ( @@ <button type="button" className="rounded-md p-1 text-muted-fg/60 transition-colors hover:bg-white/[0.06] hover:text-fg" onClick={onClear} aria-label="Disconnect Linear issue" + disabled={disabled} > <X size={14} /> </button>As per coding guidelines,
apps/desktop/src/**: Electron desktop app — check for IPC security, proper main/renderer process separation, and React best practices.Also applies to: 675-688, 891-897
🤖 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/lanes/CreateLaneDialog.tsx` around lines 668 - 673, The X (disconnect) action currently calls SelectedLinearIssueCard's onClear which clears selectedLinearIssue even when a create/setup is in-flight; modify the onClear handler passed to SelectedLinearIssueCard (and the other instances noted) so it is a no-op while the UI is locked (i.e., when busy || laneCreated) — for example, change onClear={() => setSelectedLinearIssue(null)} to onClear={busy || laneCreated ? undefined /* or () => {} */ : () => setSelectedLinearIssue(null)} (or equivalent guard) so that the disconnect cannot run during the create/setup lock; update all occurrences (the other SelectedLinearIssueCard usages) to use the same guarded handler.
🧹 Nitpick comments (1)
apps/desktop/src/shared/types/sessions.ts (1)
141-141: ⚡ Quick winConsider adding a JSDoc comment for clarity.
The adjacent
awaitInitialInputfield (line 143) includes documentation. Adding a brief comment here would help clarify the purpose and valid range of this timeout parameter.📝 Suggested documentation
+ /** Timeout in milliseconds to wait for agent CLI input readiness before delivering initialInput. */ initialInputReadyTimeoutMs?: number;🤖 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/shared/types/sessions.ts` at line 141, Add a short JSDoc comment above the initialInputReadyTimeoutMs property explaining that it sets the maximum wait time (in milliseconds) for initial input readiness and mention typical/allowed range or default behavior; reference the related awaitInitialInput field for context so readers understand this timeout controls how long awaitInitialInput will wait before timing out.
🤖 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/app/LinearIssueSelectModal.tsx`:
- Around line 57-74: The modal embeds LinearIssueBrowser but only returns a
single LaneLinearIssue via onSelectIssue, yet the browser still shows checkboxes
and “Select all”; update the LinearIssueSelectModal usage of LinearIssueBrowser
to disable multi-select affordances by passing a single-selection prop (e.g.,
multiSelect={false} or selectionMode="single") and/or explicit flags to hide
checkboxes and the select-all control (e.g., showRowCheckboxes={false},
showSelectAll={false}); if LinearIssueBrowser lacks such props, add a prop like
singleSelect:boolean to LinearIssueBrowser and implement it there to hide row
checkboxes and the select-all UI, then pass singleSelect={true} from
LinearIssueSelectModal.
In `@apps/desktop/src/renderer/components/chat/AgentChatComposer.tsx`:
- Around line 2882-2886: The code passes selectedIssue by indexing
contextAttachments[0], which fails if the first attachment isn't a Linear issue;
update AgentChatComposer.tsx to locate the Linear attachment by scanning the
full array (e.g., use Array.prototype.find on contextAttachments for item.type
=== "linear_issue") and pass that attachment.issue (or null) into the
selectedIssue prop so the picker retains the correct issue regardless of
attachment order.
---
Outside diff comments:
In `@apps/desktop/src/renderer/components/lanes/CreateLaneDialog.tsx`:
- Around line 668-673: The X (disconnect) action currently calls
SelectedLinearIssueCard's onClear which clears selectedLinearIssue even when a
create/setup is in-flight; modify the onClear handler passed to
SelectedLinearIssueCard (and the other instances noted) so it is a no-op while
the UI is locked (i.e., when busy || laneCreated) — for example, change
onClear={() => setSelectedLinearIssue(null)} to onClear={busy || laneCreated ?
undefined /* or () => {} */ : () => setSelectedLinearIssue(null)} (or equivalent
guard) so that the disconnect cannot run during the create/setup lock; update
all occurrences (the other SelectedLinearIssueCard usages) to use the same
guarded handler.
---
Nitpick comments:
In `@apps/desktop/src/shared/types/sessions.ts`:
- Line 141: Add a short JSDoc comment above the initialInputReadyTimeoutMs
property explaining that it sets the maximum wait time (in milliseconds) for
initial input readiness and mention typical/allowed range or default behavior;
reference the related awaitInitialInput field for context so readers understand
this timeout controls how long awaitInitialInput will wait before timing out.
🪄 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: b293eb84-2292-4773-9826-99def4d597e5
📒 Files selected for processing (14)
apps/ade-cli/src/adeRpcServer.test.tsapps/ade-cli/src/cli.test.tsapps/desktop/src/main/services/chat/agentChatCliLaunch.test.tsapps/desktop/src/main/services/chat/agentChatCliLaunch.tsapps/desktop/src/main/services/pty/ptyService.tsapps/desktop/src/renderer/components/app/LinearIssueBrowser.tsxapps/desktop/src/renderer/components/app/LinearIssueSelectModal.tsxapps/desktop/src/renderer/components/chat/AgentChatComposer.tsxapps/desktop/src/renderer/components/lanes/CreateLaneDialog.test.tsxapps/desktop/src/renderer/components/lanes/CreateLaneDialog.tsxapps/desktop/src/renderer/components/lanes/LanesPage.tsxapps/desktop/src/renderer/components/lanes/LinearIssuePicker.tsxapps/desktop/src/renderer/components/lanes/linearIssueDisplay.tsapps/desktop/src/shared/types/sessions.ts
💤 Files with no reviewable changes (1)
- apps/desktop/src/renderer/components/lanes/LinearIssuePicker.tsx
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
1 participant
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
Describe the change.
What Changed
Key files and behaviors.
Validation
How you tested.
Risks
Anything to watch.
Summary by CodeRabbit
New Features
Refactor
Tests
Greptile Summary
This PR refactors the Linear issue picker out of the
CreateLaneDialoginline pane and theAgentChatComposerinline component into a sharedLinearIssueSelectModal, and adds configurable timeout support for initial-input delivery in the PTY agent CLI flow.LinearIssueSelectModal.tsx): New reusable modal wrappingLinearIssueBrowserin single-select mode, consumed by bothCreateLaneDialog(now a separate dialog instead of an embedded view) andAgentChatComposer. Utility display helpers are extracted from the deletedLinearIssuePicker.tsxintolinearIssueDisplay.ts.agentChatCliLaunch.ts,ptyService.ts,sessions.ts): WheninitialInputis present,awaitInitialInput: trueand a 120 sinitialInputReadyTimeoutMsare forwarded to the PTY service; the service clamps caller-supplied values to[AGENT_CLI_READY_TIMEOUT_MS, 300 000]and warns when clamping occurs.adeRpcServer.test.ts,cli.test.tsuses the real CLI version, andCreateLaneDialog.test.tsxgains amatchMediamock and a modal-flow integration test.Confidence Score: 5/5
Safe to merge — the changes are well-scoped refactors and additive fixes with good test coverage.
The Linear picker refactor introduces no logic changes to the underlying browser component — it purely reshapes how the modal is surfaced. The kickoff-timeout addition is narrow and defended by a new test. Env isolation improvements in the test suite are strictly additive. No data paths are at risk.
No files require special attention.
Important Files Changed
Sequence Diagram
sequenceDiagram participant U as User participant CLD as CreateLaneDialog participant LDS as LaneDialogShell participant LISM as LinearIssueSelectModal participant LIB as LinearIssueBrowser U->>CLD: click "Connect a Linear issue" CLD->>LDS: "open={false} (hide lane dialog)" CLD->>LISM: "open={true}" LISM->>LIB: "singleSelect=true" U->>LIB: click issue → click "Connect issue" LIB->>LISM: onIssueAction(issue) LISM->>CLD: onSelectIssue(laneIssue) LISM->>LISM: onOpenChange(false) CLD->>LDS: "open={true} (restore lane dialog)" note over CLD,LDS: AgentChatComposer follows same pattern via LinearIssueSelectModalComments Outside Diff (1)
apps/desktop/src/renderer/components/lanes/CreateLaneDialog.test.tsx, line 426-453 (link)window.adenot restored after the testThe test sets
window.adewithObject.definePropertybut never deletes or resets it in anafterEach. Because the property isconfigurable: true, a later test that renders a component relying onwindow.adebeingundefined(or in a different state) will silently inherit the mock, making failures hard to diagnose. Consider adding a scoped cleanup or wrapping the property definition in its ownbeforeEach/afterEachblock.Prompt To Fix With AI
Reviews (2): Last reviewed commit: "Address PR review feedback" | Re-trigger Greptile