Orchestrator smoke cleanup: simplify, test, docs#362
Conversation
|
Warning Review limit reached
More reviews will be available in 40 minutes. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (24)
✨ 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
@copilot review but do not make fixes |
|
Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Review
Scope: 28 file(s), +759 / −401
Verdict: Looks good
This PR refactors orchestrator smoke follow-up work: unifies Work draft launch paths for chat and CLI (including background disposition), consolidates PTY launch typing via WorkPtyLaunchArgs, simplifies nested control flow in CLI/desktop services, and adds targeted tests plus feature-doc updates. The behavioral changes are intentional and covered by new/updated unit tests.
Notes
- Draft launch structure (
AgentChatPane.tsx): SplittingstartDraftChatLaunch,startDraftCliLaunch, and sharedlaunchDraftSessionmakes foreground vs background handling explicit. Foreground CLI correctly relies onlaunchPtySessionfocus/tab wiring; background CLI uses the dismissible notice withdraftKindso "Open" restores the right Work surface — matches the new tests. - Lane cleanup on failure now keys off
targetLane.autoCreatedrather than the auto-create UI flag alone, which is tighter and avoids deleting a pre-existing lane if resolution ever diverges. - PTY launch (
useWorkSessions.ts,useLaneWorkSessions.ts):disposition: "background"skippingselectLane/focusSession/openSessionTabis a clear fix for focus-stealing; passingstartupDelayMsthrough the lane hook aligns it with the Work tab path. - CLI runtime info parsing (
cli.ts,connection.ts,multiProjectRpcServer.ts): Dropping redundant.trim() || nullafterasString/ shared helpers is behavior-preserving, not a regression. projectConfigServiceagent-session coercion: Refactor is equivalent to main for empty-session filtering (Object.keys(session).lengthguard was already present).- Most ade-cli hunks are readability-only (nested ternaries →
if/else, removed no-op wrappers, dropped redundant type assertions where inference suffices). No new trust-boundary or injection surfaces identified.
Sent by Cursor Automation: BUGBOT in Versic
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…rk session tests Simplify nested ternaries, inline one-use helpers, consolidate duplicated guards, restore precise types, and update feature docs after the large missions-removal and Cursor SDK cleanup merges. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
arul28
force-pushed
the
ade/orch-smoke-481aaba0
branch
from
565353b to
0952d79
Compare
|
@copilot review but do not make fixes |
1 participant
Summary
Test plan
🤖 Generated with Claude Code
Greptile Summary
This PR refactors the orchestrator layer after two large merges (missions-removal #359, Cursor SDK cleanup #360): it unifies chat and CLI draft-launch paths into a single
launchDraftSessioncallback, introduces theWorkPtyLaunchArgs/WorkPtyLaunchResulttype pair to replace several inline object literals, and replaces nested-ternary chains with switch statements across 14 files.AgentChatPane.tsxextractsstartDraftChatLaunch,startDraftCliLaunch, andclearDraftLaunchComposerfrom the old monolithiclaunchDraftChatcallback, wires them through a unifiedlaunchDraftSession, and extends draft launch controls to CLI sessions (background launch support, lane auto-create for CLI).useWorkSessions.ts/useLaneWorkSessions.tsadd adispositionfield tolaunchPtySessionso background launches skipselectLane/focusSession/openSessionTab, andstartupDelayMsis now correctly forwarded inuseLaneWorkSessions.Confidence Score: 5/5
Safe to merge; the refactor is mechanical and the new CLI-background launch path is covered by integration tests.
The changes are primarily a refactor of an existing launch flow into smaller, well-named helpers, plus new background-disposition support for CLI sessions. Draft restore on failure is preserved via restoreDraftLaunchSnapshot in the catch block, PTY-layer disposition plumbing is validated by new unit tests, and the startupDelayMs omission in useLaneWorkSessions is fixed. All observations are style-level.
No files require special attention beyond what was already flagged in earlier review threads.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD Submit["handleSubmit / onSubmitInBackground"] --> CliCheck{isWorkCliLaunchDraft?} CliCheck -->|yes| LaunchCli["launchDraftCliSession(mode)"] CliCheck -->|no| ChatCheck{draftLaunchTargetIsAutoCreate\nor forceDraft?} ChatCheck -->|yes| LaunchChat["launchDraftChat(mode)"] ChatCheck -->|no| RegularSend["regular agentChat.send()"] LaunchCli --> DS["launchDraftSession('cli', mode)"] LaunchChat --> DS2["launchDraftSession('chat', mode)"] DS --> ResolveLane["resolveDraftLaunchLane(snapshot)"] DS2 --> ResolveLane ResolveLane -->|auto-create| CreateLane["window.ade.lanes.create()"] ResolveLane -->|existing| LaneTarget["DraftLaunchLaneTarget\n{laneId, laneName, worktreePath, autoCreated}"] CreateLane --> LaneTarget LaneTarget --> ClearComposer["clearDraftLaunchComposer(snapshot)"] ClearComposer --> KindSwitch{kind?} KindSwitch -->|chat| StartChat["startDraftChatLaunch(prepared, lane)"] KindSwitch -->|cli| StartCli["startDraftCliLaunch(prepared, lane, mode)"] StartChat --> ChatAPI["window.ade.agentChat.send()"] StartChat --> SessionId["StartedDraftLaunch {sessionId, draftKind:'chat'}"] StartCli --> PtyLaunch["onLaunchCliSession() → launchPtySession()"] StartCli --> CliId["StartedDraftLaunch {sessionId, draftKind:'cli'}"] SessionId --> ModeCheck{mode?} CliId --> ModeCheck ModeCheck -->|foreground + chat| OpenSession["openLaunchedDraftSession()\nsetWorkViewState + navigate"] ModeCheck -->|foreground + cli| PtyFocus["PTY layer: selectLane + focusSession + openSessionTab"] ModeCheck -->|background| BgNotice["setBackgroundLaunchNotice(launch)"] BgNotice --> OpenBtn["'Open' button → openLaunchedDraftSession()"]Prompt To Fix All With AI
Reviews (2): Last reviewed commit: "ship: orchestrator smoke cleanup — simpl..." | Re-trigger Greptile