ADE-94: Linear multi-agent launch can create untracked CLI sessions#510
Merged
arul28 merged 2 commits intoJun 2, 2026
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
📝 WalkthroughWalkthroughThis PR removes the timeout-based kickoff-readiness wait from agent CLI launches and refactors PTY failure handling. When initial input is provided, the launch no longer waits for readiness; the PTY service now explicitly terminates and cleans up sessions when initial-input await times out. Batch launch test coverage validates concurrent CLI session tracking. ChangesAgent CLI Initial Input Handling and Batch Launch
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 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 |
![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.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/lib/linearBatchLaunch.test.ts (1)
351-354: 💤 Low valueMinor: wait loop could be fragile on slow systems.
The loop yields up to 5 times waiting for
launchClito be called 3 times. In heavily loaded test environments, this might not be enough iterations. Consider usingvi.waitFor()for more robust async waiting.However, this is acceptable for a unit test and will fail with a clear assertion error if timing is an issue.
♻️ Alternative using vi.waitFor (optional)
- for (let i = 0; i < 5 && launchCli.mock.calls.length < 3; i += 1) { - await Promise.resolve(); - } - expect(launchCli).toHaveBeenCalledTimes(3); + await vi.waitFor(() => { + expect(launchCli).toHaveBeenCalledTimes(3); + });🤖 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/lib/linearBatchLaunch.test.ts` around lines 351 - 354, Replace the fragile manual yield loop that spins up to 5 ticks waiting for launchCli to be called 3 times with a robust async wait using vi.waitFor: remove the for-loop and instead await vi.waitFor until the launchCli mock has been called 3 times (e.g. check launchCli.mock.calls.length === 3 or use an assertion inside vi.waitFor), so the test reliably waits on slow/loaded CI without flaky timing assumptions.
🤖 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.
Nitpick comments:
In `@apps/desktop/src/renderer/lib/linearBatchLaunch.test.ts`:
- Around line 351-354: Replace the fragile manual yield loop that spins up to 5
ticks waiting for launchCli to be called 3 times with a robust async wait using
vi.waitFor: remove the for-loop and instead await vi.waitFor until the launchCli
mock has been called 3 times (e.g. check launchCli.mock.calls.length === 3 or
use an assertion inside vi.waitFor), so the test reliably waits on slow/loaded
CI without flaky timing assumptions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 234ff4c4-a414-4e20-8918-e2b68b322ecc
📒 Files selected for processing (5)
apps/desktop/src/main/services/chat/agentChatCliLaunch.test.tsapps/desktop/src/main/services/chat/agentChatCliLaunch.tsapps/desktop/src/main/services/pty/ptyService.test.tsapps/desktop/src/main/services/pty/ptyService.tsapps/desktop/src/renderer/lib/linearBatchLaunch.test.ts
💤 Files with no reviewable changes (1)
- apps/desktop/src/main/services/chat/agentChatCliLaunch.ts
Owner
Author
|
@copilot review but do not make fixes |
Copilot stopped work on behalf of
arul28 due to an error
June 2, 2026 07:02
arul28
deleted the
ade-94-linear-multi-agent-launch-can-create-untracked-cli-sessions
branch
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
awaitInitialInputfailures terminate the PTY and end the session as failed.Verification
origin/main(0f2e5518c2d67de8c3a51ad2b846f9f901e79638) and on this lane before patching./audit; fixed the discovered explicitawaitInitialInputcleanup gap./automate: docs parity updated; iOS, ADE CLI, and TUI parity required no product changes. ADE CLI typecheck/tests passed; focused TUI tests passed.npm --prefix apps/desktop run test -- src/main/services/chat/agentChatCliLaunch.test.tsnpm --prefix apps/desktop run test -- src/main/services/pty/ptyService.test.tsnpm --prefix apps/desktop run test -- src/renderer/lib/linearBatchLaunch.test.ts/finalizelocal gates passed:npm installinapps/desktop,apps/ade-cli, andapps/webnpm --prefix apps/desktop run typechecknpm --prefix apps/ade-cli run typechecknpm --prefix apps/web run typechecknpm --prefix apps/desktop run lint(0 errors; existing warnings only)apps/ade-cli npm test; shard 4 had a Vitest IPC timeout while runninglinearSync.test.ts, and the targeted rerun passed (46tests)npm --prefix apps/desktop run buildnpm --prefix apps/ade-cli run buildnpm --prefix apps/web run buildnode scripts/validate-docs.mjsSource file mapsections andPRD.mdrelative linksgit diff --checkGreptile Summary
This PR fixes a race condition in Linear multi-agent batch launches where
launchClicould start an untracked PTY session because the caller waited for CLI input-readiness before returning the durable session record. The fix decouples session creation from CLI readiness by removingawaitInitialInputfrom theagentChatCliLaunchpath (making kickoff prompt injection fully asynchronous) and adds explicit teardown — SIGTERM +closeEntrywithstatus: "failed"— for any caller that does explicitly opt in toawaitInitialInputand encounters a timeout.agentChatCliLaunch.ts: DropsawaitInitialInput: trueand theAGENT_CHAT_CLI_KICKOFF_READY_TIMEOUT_MSconstant soptyService.createresolves as soon as the tracked PTY record is committed, not when the Codex CLI signals readiness.ptyService.ts: TheawaitInitialInputcatch block now callsterminatePtyProcessTree+closeEntry(ptyId, 1)before re-throwing, ensuring a hidden live process is never left behind when an explicitly-awaited prompt injection fails.sessionService.end(failed)calls.Confidence Score: 5/5
Safe to merge — the change is narrowly scoped to decoupling session-record creation from CLI readiness, with no regressions to the non-awaited path.
The removal of awaitInitialInput in agentChatCliLaunch is the exact minimal fix for the untracked-session bug: the durable record is committed before the caller returns, so batch launch state is always consistent. The new teardown path in ptyService is gated behind the awaitInitialInput flag and protected by the existing entry.disposed guard in closeEntry, preventing any double-end of a session.
No files require special attention.
Important Files Changed
Sequence Diagram
sequenceDiagram participant BL as linearBatchLaunch participant LC as launchAgentChatCli participant PS as ptyService.create participant CLI as Codex CLI BL->>LC: launchCli(args) LC->>PS: create with initialInput and initialInputDelayMs Note over PS: Creates durable PTY/session record PS-->>LC: ptyId, sessionId, pid LC-->>BL: sessionId returned immediately Note over BL: Session is tracked and batch state updated PS->>CLI: async fire-and-forget writeInitialInput alt CLI becomes ready in time CLI-->>PS: ready signal PS->>CLI: write kickoff prompt else awaitInitialInput true and timeout PS->>PS: terminatePtyProcessTree SIGTERM PS->>PS: closeEntry exitCode 1 status failed PS-->>LC: throws error else awaitInitialInput false and timeout PS->>PS: log warning and preserve session endReviews (2): Last reviewed commit: "test: finalize Linear CLI launch coverag..." | Re-trigger Greptile