feat(cli): agent-relay drive <name> interactive take-over client (#864 sub-3)#887
Merged
Conversation
sub-3) Adds the read-write attach verb from issue #864's verb taxonomy. `drive` flips the worker into `human` session mode so the broker parks new inbound relay messages in a per-worker FIFO queue, captures the agent's current screen, opens the WS event stream, and forwards your keystrokes to the worker's PTY via `POST /api/input/{name}`. Keybinds: Ctrl+G to flush the queue, Ctrl+B D (or Ctrl+C) to detach. On any exit path the client best-effort restores the worker's prior session mode so the queue doesn't fill up indefinitely. Factors broker-connection discovery (flag → env → connection.json) into `src/cli/lib/broker-connection.ts` so `drive`, `view`, and future `relay`/`new`/`run` verbs share a single resolver. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds an interactive CLI verb ChangesDrive CLI Command
Sequence DiagramsequenceDiagram
participant User
participant CLI as drive command
participant Broker as Broker HTTP API
participant Agent as Agent WebSocket
User->>CLI: agent-relay drive <name>
CLI->>Broker: GET /api/spawned/{name}/mode
CLI->>Broker: PUT /api/spawned/{name}/mode = human
CLI->>Broker: GET /api/spawned/{name}/snapshot
CLI->>Broker: GET /api/spawned/{name}/pending
CLI->>Agent: WebSocket /ws
Agent-->>CLI: stream chunks/events
CLI-->>User: render + status bar
User->>CLI: keystrokes (raw)
CLI->>Broker: POST /api/input/{name}
CLI->>Broker: POST /api/spawned/{name}/flush (Ctrl+G)
CLI->>Broker: POST /api/resize/{name} (on attach/resize)
User->>CLI: Ctrl+C (detach)
CLI->>Broker: PUT /api/spawned/{name}/mode = prior
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 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 unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 @.trajectories/active/traj_v87cyrs8dke9.json:
- Line 26: Locate the JSON objects with the "content" field that contain a
duplicated sentence separated by a colon (e.g., "Factored broker-connection
discovery into src/cli/lib/broker-connection.ts: Factored broker-connection
discovery into src/cli/lib/broker-connection.ts") and replace the value with the
single non-duplicated sentence (remove the trailing ": <same text>" repetition).
Apply the same fix to the other occurrence noted (the second "content" entry at
the other occurrence) so each "content" string reads once without the "A: A"
duplication.
In @.trajectories/index.json:
- Line 417: The entry uses a machine-specific absolute path value for the JSON
"path" key ("/Users/will/.../traj_v87cyrs8dke9.json"); replace it with a
repo-relative path (e.g. ".trajectories/active/traj_v87cyrs8dke9.json") in
.trajectories/index.json so it matches the other entries. Locate the JSON object
containing the "path" key referencing traj_v87cyrs8dke9.json, edit the value to
the relative path, then stage and recommit the change (or amend the commit) so
no local filesystem/user info remains in the repo.
In `@web/content/docs/reference-cli.mdx`:
- Line 107: Update the detach key documentation entry to list all supported
variants: replace the current "`Ctrl+B` then `D` | Detach cleanly..." line with
a description that includes "D (or d or Ctrl+D)" so it reads something like
"`Ctrl+B` then `D` (or `d` or `Ctrl+D`) | Detach cleanly. Restores the worker's
prior session mode, closes the WebSocket, exits 0." Ensure the table row in
web/content/docs/reference-cli.mdx reflecting the detach behavior is updated
accordingly.
🪄 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: 09151df4-bb12-406f-b5fd-4e03d19c05f9
📒 Files selected for processing (8)
.trajectories/active/traj_v87cyrs8dke9.json.trajectories/index.jsonsrc/cli/bootstrap.test.tssrc/cli/bootstrap.tssrc/cli/commands/drive.test.tssrc/cli/commands/drive.tssrc/cli/lib/broker-connection.tsweb/content/docs/reference-cli.mdx
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![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
Here are some automated review suggestions for this pull request.
Reviewed commit: b73642ef0c
ℹ️ 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".
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 8 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/cli/lib/broker-connection.ts">
<violation number="1" location="src/cli/lib/broker-connection.ts:81">
P2: Using `??` here treats blank trimmed values as valid and prevents fallback to lower-priority URL sources.</violation>
<violation number="2" location="src/cli/lib/broker-connection.ts:87">
P2: `??` allows empty trimmed API key values to override valid fallback keys from lower-priority sources.</violation>
</file>
<file name="src/cli/commands/drive.ts">
<violation number="1" location="src/cli/commands/drive.ts:537">
P2: `stdin` bytes are encoded with `'binary'` before `/api/input`, which can corrupt non-ASCII keystrokes. Use UTF-8 decoding so terminal input round-trips correctly through the JSON API.</violation>
</file>
<file name=".trajectories/index.json">
<violation number="1" location=".trajectories/index.json:417">
P2: Store trajectory paths as repository-relative paths instead of absolute local paths to avoid leaking machine-specific filesystem details and breaking portability.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Fix all with cubic | Re-trigger cubic
Reviewer pushback on the original sub-PR 3 punt: terminal resize
forwarding isn't a nice-to-have for a take-over UX, it's table
stakes. Every comparable tool (tmux, screen, ssh, mosh) forwards
resizes; without it, a TUI in the agent renders into whatever
24×80 box the PTY was spawned with and ignores the human's actual
viewport. Bad enough that the previous "out of scope, follow-up
issue" answer doesn't hold up.
Wires resize forwarding into the existing drive session lifecycle:
- New `DriveTerminal` dep: `getSize()` reads local rows/cols
from `process.stdout` (or `null` when stdout isn't a TTY);
`onResize(handler)` translates Node's automatic
`process.stdout` `'resize'` event (which Node fires on
SIGWINCH for TTY stdouts) into a subscription with an
unsubscribe handle.
- New `resizeWorker()` HTTP helper: `POST /api/resize/{name}`
body `{rows, cols}`. Matches the shape of `sendInput` /
`setSessionMode` so it slots into the existing pattern.
- On attach: after snapshot + before WS open, sync the agent's
PTY to the local terminal. Best-effort — a failure logs but
doesn't abort (the human can still type, output just renders
at the old size). Skipped when stdout isn't a TTY.
- On the same `socket.on('open')` callback that takes over
stdin: subscribe to local resize events. Forwards every
change to the broker and repaints the status line at the new
bottom-row index.
- On teardown (clean detach, signal, abnormal close): unsub
the resize handler alongside the stdin handler. Same
lifecycle = same teardown path.
- Status-line row source switched from `snapshot.rows` (the
agent's PTY rows at snapshot time) to the local terminal's
current rows. Falls back to snapshot value if local size is
unavailable, then to the renderer's 24-row default. Without
this, the status line would land in the wrong place after
the first resize.
Tests (5 new, 38 total in drive.test.ts):
- Initial-attach size forwarded with correct body
- Subsequent SIGWINCH events forward every change in order
- Detach unsubscribes the resize listener (`listenerCount() == 0`)
- Non-TTY stdout skips resize forwarding entirely (0 calls)
- Initial resize failure logs a warning but lets the session continue
New `FakeTerminal` helper in the test file models the DI surface
the same way `FakeStdin` / `FakeWebSocket` do — fully under test
control, never touches a real TTY.
Docs (`web/content/docs/reference-cli.mdx`) updated: added a 7th
step to the on-attach sequence describing the resize sync, and
mentions SIGWINCH forwarding alongside it.
Touched-suite test status: 80 passing across drive / view / attach
/ bootstrap (was 75 before; +5 new resize tests).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/cli/commands/drive.ts (1)
756-758: 💤 Low valueWebSocket message data handling may not correctly convert
ArrayBufferorBuffer[].The
WebSocket.RawDatatype includesArrayBuffer | Buffer[]in addition toBuffer. For these types,String(data)produces"[object ArrayBuffer]"or similar, which fails JSON parsing and returns{ kind: 'other' }. This is safe but could log confusing "other" events if the broker ever sends non-Buffer data.If this matters for future compatibility:
Proposed fix for broader message type support
socket.on('message', (data) => { - const text = - typeof data === 'string' ? data : Buffer.isBuffer(data) ? data.toString('utf-8') : String(data); + let text: string; + if (typeof data === 'string') { + text = data; + } else if (Buffer.isBuffer(data)) { + text = data.toString('utf-8'); + } else if (Array.isArray(data)) { + text = Buffer.concat(data).toString('utf-8'); + } else if (data instanceof ArrayBuffer) { + text = Buffer.from(data).toString('utf-8'); + } else { + text = String(data); + } const event = classifyWsEvent(text, agentName);🤖 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 `@src/cli/commands/drive.ts` around lines 756 - 758, The WebSocket message handler in the socket.on('message', ...) callback currently only handles string and Buffer, causing ArrayBuffer or Buffer[] (WebSocket.RawData) to stringify as "[object ArrayBuffer]" and break JSON parsing; update the conversion for the message payload (the local variable text) to detect ArrayBuffer (convert via new Uint8Array(data) then text decoder or Buffer.from) and Buffer[] (concat into a single Buffer) so all RawData variants produce a proper UTF-8 string before downstream JSON.parse or event handling.
🤖 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 `@src/cli/commands/drive.ts`:
- Around line 756-758: The WebSocket message handler in the socket.on('message',
...) callback currently only handles string and Buffer, causing ArrayBuffer or
Buffer[] (WebSocket.RawData) to stringify as "[object ArrayBuffer]" and break
JSON parsing; update the conversion for the message payload (the local variable
text) to detect ArrayBuffer (convert via new Uint8Array(data) then text decoder
or Buffer.from) and Buffer[] (concat into a single Buffer) so all RawData
variants produce a proper UTF-8 string before downstream JSON.parse or event
handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2b58bcb0-639b-4374-95c5-26a40adac722
📒 Files selected for processing (3)
src/cli/commands/drive.test.tssrc/cli/commands/drive.tsweb/content/docs/reference-cli.mdx
✅ Files skipped from review due to trivial changes (1)
- web/content/docs/reference-cli.mdx
Review feedback on PR #887: (1) UTF-8 stdin encoding (devin / codex / cubic P2) `outcome.forward.toString('binary')` encoded raw stdin bytes as latin1 before JSON transport. The broker deserializes `data` as a Rust UTF-8 String, so bytes ≥ 0x80 got re-encoded — a typed `é` (0xC3 0xA9) arrived at the agent as `é` (4 bytes). Switching to `'utf-8'` matches Node's default and round-trips correctly. (2) Empty-trim falls through in broker-connection (cubic P2 ×2) `?.trim() ?? envValue` is wrong: `''.trim()` is `''`, not nullish, so a blank `--broker-url` flag would silently override a real `RELAY_BROKER_URL`. New `trimOrUndefined()` helper filters empty results to undefined so `??` chains correctly fall through to lower-priority sources. Four regression tests in the new `broker-connection.test.ts` lock the behaviour for both url and api-key, across both override layers. (3) Trajectory absolute paths leak local user (coderabbit Major / cubic P2) `.trajectories/index.json:417` stored `/Users/will/Projects/AgentWorkforce/relay/.claude/worktrees/...` — the trail tool didn't normalize to repo-relative when it ran in a worktree. Replaced with the repo-relative form `.trajectories/active/traj_v87cyrs8dke9.json` matching every other entry. (4) Duplicated decision content (coderabbit Minor) `.trajectories/active/traj_v87cyrs8dke9.json` had two entries shaped like `"X: X"` (formatter bug — duplicated the headline around a colon). De-duped both. (5) Docs underdocumented detach (coderabbit Minor) The `KeybindParser` accepts `D`, `d`, or `Ctrl+D` after the `Ctrl+B` prefix. Docs only listed `D`. Updated the keybinds row. Touched-suite test status: 87 passing across drive / view / attach / bootstrap / broker-connection (was 80; +7 from the new broker-connection test file). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Member
Author
|
Pushed @devin-ai-integration (🟡) / @chatgpt-codex-connector (P2) / @cubic-dev-ai (P2) —
@cubic-dev-ai (P2 ×2) —
@coderabbitai (🟠 Major) / @cubic-dev-ai (P2) —
@coderabbitai (🟡 Minor) —
@coderabbitai (🟡 Minor) — Docs only listed
Test status: 87 passing across drive / view / attach / bootstrap / the new broker-connection (was 80; +7 — 4 trim-fallthrough regression tests + 3 reorganization). All 8 inline review threads marked as resolved on the GitHub UI. |
|
To use Codex here, create an environment for this repo. |
Contributor
|
Test count at 87 with the new regression coverage looks solid. Happy to approve. (´• ω •`) 🐇✨ ✏️ Learnings added
✅ Actions performedComments resolved. Approval is disabled; enable |
Member
Author
|
@copilot resolve the merge conflicts in this pull request |
Co-authored-by: willwashburn <957608+willwashburn@users.noreply.github.com>
Co-authored-by: willwashburn <957608+willwashburn@users.noreply.github.com>
Contributor
Resolved. I merged |
Copilot stopped work on behalf of
willwashburn due to an error
May 18, 2026 05:36
6 tasks
This was referenced May 18, 2026
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
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
Adds the read-write attach verb from the #864 verb taxonomy.
agent-relay drive <name>attaches to a running agent, flips it intohumansession mode so the broker parks new inbound relay messages in a per-worker FIFO queue, forwards your keystrokes to the worker's PTY, lets you drain the queue on demand, and detaches cleanly without killing the agent. Built on top of the four session-mode routes added in #884 and the snapshot-on-attach helper from #880.Sequence on attach
--broker-url→RELAY_BROKER_URL→.agent-relay/connection.json).GET /api/spawned/{name}/mode— remember the previous mode so we can restore on detach.PUT /api/spawned/{name}/modewith{"mode":"human"}— start queueing inbound relay messages.captureAndRenderSnapshot— repaint the agent's current visible screen so the user isn't staring at a blank terminal.GET /api/spawned/{name}/pending— seed the status-line counter./ws, subscribe toworker_stream/delivery_queued/agent_pending_drainedevents for this worker.POST /api/input/{name}.On clean or abnormal detach, best-effort
PUT /api/spawned/{name}/moderestores the worker's prior session mode so the queue doesn't fill up indefinitely. Raw mode is always restored intry/finallystyle via ateardownStdinhelper.Keybinds
Ctrl+GPOST /api/spawned/{name}/flush).Ctrl+BthenD(ordor Ctrl+D)Ctrl+Bthen?Ctrl+Cdrivenever kills the agent.Ctrl+Bthen any other byteCtrl+Baren't deprived.Status line is painted on the bottom row using ANSI save/restore-cursor escapes so the agent's output isn't disturbed:
What's in this PR
src/cli/commands/drive.ts— new command. Includes the session runner, HTTP helpers,KeybindParserstate machine,renderStatusLinehelper, andclassifyWsEventfilter — all exported and unit-tested in isolation.src/cli/commands/drive.test.ts— 33 tests covering: classify filter, keybind parser (single-byte +Ctrl+Bprefix + pass-through), status-line ANSI, happy-path attach + detach, mode-flip failure, snapshotnot_found/no_pty/unavailable, pending-count tracking, keystroke forwarding, flush keybind, abnormal-close mode restore, re-attach-when-already-human, SIGINT handling, and missing-broker.src/cli/lib/broker-connection.ts— new. Factors the--broker-url→ env →connection.jsondiscovery chain thatviewwas carrying inline.drivereuses it; futurerelay/new/runverbs can too.src/cli/bootstrap.ts— registerdrive.src/cli/bootstrap.test.ts— add'drive'toexpectedLeafCommands(per the regression feat(cli):agent-relay view <name>read-only stream client (#864 sub-1) #880 hit on first try).web/content/docs/reference-cli.mdx— new### drivesection with flag table, keybinds table, status line example, and worked example.What's out (deliberately)
agent-relay relay <name>— the auto-inject sibling. Sub-PR 4.agent-relay new/agent-relay run— spawn-time verbs. Sub-PR 4.Test plan
npx vitest run src/cli/commands/drive.test.ts src/cli/commands/view.test.ts src/cli/lib/attach.test.ts— 72 passed.npx vitest run src/cli/bootstrap.test.ts— 3 passed (the newdriveleaf is in the hardcoded list).npx tsc --noEmitclean for touched scope.npm run lint— 0 errors. One complexity warning (19) onrunDriveSession, consistent with the patterns inattach.ts(18) and lots of other CLI commands (20–68).agent-relay drive <name>, type into it, send relay messages from another window, watchpendingcount go up, hitCtrl+G, watch it drain, hitCtrl+B D, confirm the agent's session mode is back torelay.Part of #864.
🤖 Generated with Claude Code