feat(cli): auto-steer ping_peer messages into an active turn - #1708
feat(cli): auto-steer ping_peer messages into an active turn#1708KevinFun wants to merge 2 commits into
Conversation
ping_peer messages to a busy peer waited silently for the next turn boundary. Tag them with deliveryMode 'steer' and a generated localId so the peer CLI can inject them into the running turn (codex turn/steer, pi native steer), falling back to the ordinary queue when the peer is idle or its flavor cannot steer. - ping_peer/MCP: send localId + deliveryMode 'steer'; document steer semantics in the tool description - hub: persist steer provenance for every steering-capable flavor (isSteeringSupportedForSession) instead of pi only - codex: extract the SteerQueuedMessage RPC handler and reuse it from a queue-arrival hook; every refusal path restores the queued row - queue: push() carries an optional steerHint and passes the item to the onMessage handler
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Findings
- [Major] Cursor ACP peer nudges are classified as auto-steerable but never auto-steered — the new hub gate preserves
deliveryMode: "steer"for Cursor ACP, and the tool description promises mid-turn injection, but Cursor’s inbound path dropsmessage.meta.deliveryModeand callsmessageQueue.push(...)without an arrival hint or hook (cli/src/cursor/runCursor.ts:100,cli/src/cursor/cursorUserMessageQueue.ts:20). During an active Cursor turn,ping_peertherefore remains queued until the turn finishes. Evidence:hub/src/sync/messageService.ts:130.
Suggested fix:Keep Cursor downgraded (and remove it from the new tool-description claim) until its receive path carries the steer hint and invokes the existing Cursor ACP steer routine on arrival.const flavor = isObject(metadata) ? metadata.flavor : null return flavor === "pi" || flavor === "codex" ? "steer" : "queue"
Summary
Review mode: initial. One Major finding: automatic peer steering is advertised and persisted for Cursor ACP, but only Codex and Pi currently consume the live steer intent.
Testing
Not run locally (automation; PR code was not executed). GitHub CI integration check is failing in cli/src/runner/runner.integration.test.ts; relation to this diff is not established. The main test job was still running at review time. Add a Cursor ACP busy-turn test if Cursor remains in the capability gate, plus a Codex launcher-level test proving a steer-tagged arrival calls the extracted steer path and falls back to the queue on refusal.
HAPI Bot
| // Steer provenance is only meaningful for flavors whose CLI can act on it | ||
| // (codex turn/steer, pi native steer, cursor ACP soft-send). Everything | ||
| // else (claude, unknown flavors) stores an ordinary queue row. | ||
| return isObject(metadata) && isSteeringSupportedForSession(metadata as Parameters<typeof isSteeringSupportedForSession>[0]) |
There was a problem hiding this comment.
[MAJOR] Cursor ACP is included here, but its inbound path never auto-steers this intent
isSteeringSupportedForSession() returns true for Cursor ACP, so this preserves deliveryMode: "steer". However, cli/src/cursor/runCursor.ts:100-108 ignores message.meta.deliveryMode, enqueueCursorUserMessage() calls messageQueue.push(...) without a steer hint, and the Cursor launcher only steers when the explicit SteerQueuedMessage RPC is invoked. A ping_peer message sent during an active Cursor turn therefore waits for turn completion despite the new tool description promising immediate injection.
Suggested fix (until Cursor gets equivalent arrival-hook plumbing):
const flavor = isObject(metadata) ? metadata.flavor : null
return flavor === "pi" || flavor === "codex" ? "steer" : "queue"Also remove Cursor from PING_PEER_TOOL_DESCRIPTION; alternatively, pass the delivery mode through Cursor’s queue and reuse its existing soft-steer handler from an arrival hook.
Cursor ACP's inbound path does not consume the steer hint yet, so keep its rows queued and drop it from the ping_peer tool description until its receive path is wired; codex and pi remain auto-steerable.
|
Thanks for the review. Addressed the Major finding in 73630bb:
On the launcher-level test suggestion: Re: the failing |
|
Additional data point on the |
There was a problem hiding this comment.
Findings
- [Major] First-turn auto-steer is rejected when Codex resolves an inherited model — the new arrival hook delegates to a mode hash captured before model resolution; see the inline comment at
cli/src/codex/codexRemoteLauncher.ts:2253.
Summary
Review mode: follow-up after new commits. The prior Cursor ACP finding is no longer present in the latest head. One Major finding remains: on a common default/inherited-model launch, peer nudges sent during the initial active Codex turn fall back to the queue instead of being auto-steered.
Testing
Not run (automation; PR code was not executed). At review time, drift-gate passed, integration was failing, and the main test job was pending; failure logs were unavailable while the workflow was still running, so relation to this diff is not established. Add a launcher-level regression test with an initial mode whose model is omitted, a mocked thread/start response that resolves the model, and a steer-tagged arrival during that active turn.
HAPI Bot
| session.queue.setOnMessage((_message, _mode, item) => { | ||
| if (!item.steerHint || !item.localId) return; | ||
| if (!turnInFlight || this.abortInProgress || this.shouldExit) return; | ||
| void steerQueuedByLocalId(item.localId).catch((error) => { |
There was a problem hiding this comment.
[MAJOR] Inherited-model turns reject this auto-steer path
On a normal default-model launch, the first queued prompt is hashed with model: undefined in cli/src/codex/runCodex.ts:273-289. Starting/resuming the thread then calls applyResolvedModel() (cli/src/codex/codexRemoteLauncher.ts:441-448), so syncCurrentConfigFromSession() builds this arriving peer nudge with the resolved model. The delegated handler compares that new hash against the original activeMessage.hash at cli/src/codex/codexRemoteLauncher.ts:2122-2124, treats the modes as different, and restores the row. Therefore peer nudges during the initial active turn silently wait for the next turn instead of auto-steering.
Suggested fix: normalize the active message to the effective mode used for turn/start, while that mode is captured, so later genuine mode changes still remain blocked.
const mode = {
...message.mode,
model: session.getModel() ?? message.mode.model
};
activeMessage = {
...message,
mode,
hash: session.queue.modeHasher(mode)
};Add a regression test where the initial message omits model, startThread resolves it, and a steerHint arrival with that resolved model must call turn/steer and emit messages-consumed { steered: true }.
1 participant
Summary
ping_peermessages to a peer that is mid-turn were stored and forwarded immediately, but the peer CLI silently held them until the current turn ended — the nudge looked "sent" yet was ignored by the working agent. This PR delivers peer messages into a running turn automatically (no manual Steer click), while keeping the ordinary queue as the fallback everywhere else.Changes
ping_peer(MCP tool +hapi ping-peer): sends a generatedlocalId(row becomes queue-shaped: visible in the peer's waiting bar, steerable, cancellable) anddeliveryMode: 'steer'— both are existing protocol fields ofSendMessageRequestSchema. Tool description documents the steer semantics.messageService.getNormalizedDeliveryMode): persists steer provenance for every steering-capable flavor via the sharedisSteeringSupportedForSessionpredicate (previously pi-only). Claude and unknown flavors still downgrade to an ordinary queue row.codexRemoteLauncher): theSteerQueuedMessageRPC handler is extracted verbatim intosteerQueuedByLocalId(the web Steer button now delegates to it), and a queue-arrival hook injects a steer-tagged arrival into the active turn via app-serverturn/steer. Every refusal path restores the queued row, so a refused auto-steer simply delivers at the next turn boundary.MessageQueue2):push()accepts an optionalsteerHintand theonMessagehandler now receives the queued item.Behavior
messages-consumed { steered: true }). Pi gets this for free: its CLI already honorsmeta.deliveryMode === 'steer'.Testing
bun typecheck(cli / hub / web / relay)bun run test(cli + hub + web + shared + relay)ping_peerrequest body carrieslocalId(ping-peer-<uuid>) +deliveryMode: 'steer'MessageQueue2passessteerHintthrough to the arrival handler; isolated/clear pushes stay unhinted