fix(cli): keep driven agents receiving relay messages - #1368
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthrough
ChangesDrive delivery behavior
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant DriveCommand
participant runDriveSession
participant DeliveryModeAPI
participant WorkerWebSocket
DriveCommand->>runDriveSession: start drive attach
runDriveSession->>DeliveryModeAPI: set auto_inject
runDriveSession->>WorkerWebSocket: receive delivery events
runDriveSession->>DeliveryModeAPI: toggle mode on Ctrl+]
runDriveSession->>DeliveryModeAPI: restore mode with CAS guard
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3b9e9b7652
ℹ️ 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
🤖 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 `@CHANGELOG.md`:
- Line 8: Update the unreleased changelog heading from “## [Unreleased - Minor]”
to the standard “## [Unreleased]” format, leaving the release classification for
the eventual versioned entry.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 83f142e3-0f01-4068-b5ed-6c67a54a746d
📒 Files selected for processing (3)
CHANGELOG.mdpackages/cli/src/cli/lib/attach-drive.test.tspackages/cli/src/cli/lib/attach-drive.ts
Attaching with `drive` put the agent in `manual_flush` for the whole session, so every inbound relay message parked until Ctrl+] or detach. Watching a team coordinate therefore stopped it: three driven agents exchanged 8 messages in seven minutes where the same unattended run exchanged 21 in three. `drive` now asserts `auto_inject` on attach. Ctrl+] holds delivery for when the screen must stay still while typing, and a second press drains the queue and returns to live delivery. Detach still restores an explicit pre-attach hold. The status line also counted every `delivery_queued` frame, including the ones harness runtimes emit for their own injection queues. With live delivery that count climbed on ordinary traffic and never came back down, so it now counts only the frames the inbound hold actually parked. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
3b9e9b7 to
3d12838
Compare
Behavior
node agent attach --mode driveassertsauto_inject, so a driven agent goes on receiving relay messages while you watch it.Ctrl+]holds delivery for when the screen must stay still while typing; a second press drains the parked queue and returns to live delivery. Detach still restores an explicit pre-attach hold via the same compare-and-set.The status line's
pendingcounter reports only messages the inbound hold actually parked.Why
drivepreviously flipped the agent intomanual_flushfor the whole session, so every inbound relay message parked untilCtrl+]or detach. Watching a team therefore stopped it: three PTY agents playing tic-tac-toe with a drive session attached to each exchanged 8 messages in seven minutes and never finished, where the same unattended run exchanged 21 and finished in three. Agents made progress only by pollingcheck_inbox— the push path was frozen for the session's lifetime.Two layers emit
delivery_queued: the inbound hold, which means "parked", and the harness runtimes, which emit it for every delivery they queue for injection — ordinary traffic on its way to the agent. The counter added both. Undermanual_flushthat was mostly hidden; with live delivery it would climb on every message and never come back down, since nothing drains a queue the message was never in. It now counts only frames carrying the hold's reason.Verification
vitest run packages/cli— 782 passed, 9 skippedtsc --noEmit— cleanagent new --mode drive, each attached in a real PTY, played a full tic-tac-toe game to a draw in 3m15s over 21 relay messages, with every status line readingdelivery=auto_inject | pending=0.Relationship to #1364 and #1366
Those fix the status line tearing the pane and are independent of this change. This one is the delivery half: #1364 attributed the "agents never used the relay protocol" symptom to operator error, and the measurements above show the session-long
manual_flushwas starving them. This PR does not touchrenderStatusLine, so it composes with either rendering fix.