Conversation
There was a problem hiding this comment.
Findings
- [Major] Inherited-model Cursor sessions reject auto-steer and manual Steer after the active prompt is rehashed —
cli/src/cursor/cursorAcpRemoteLauncher.ts:625. Details and suggested fix in the inline comment.
Summary
Review mode: initial
One Major finding across the full 16-file diff: active and incoming Cursor messages can hash different representations of the same inherited model.
Testing
Not run (automation; PR code not executed). Add an inherited-model regression with the production hasher, fresh incoming mode objects, and assertions for both automatic and manual steering. GitHub's test check was still running; integration and drift-gate had completed successfully.
HAPI Bot
There was a problem hiding this comment.
Findings
- [Major] New Cursor regression test fails typechecking —
cli/src/cursor/cursorAcpRemoteLauncher.test.ts:380. Normalize the nullable model with?? undefined; fix snippet in the inline comment. - [Major] Codex Reserve turns reject steering because active and incoming hashes use different model aliases —
cli/src/codex/codexRemoteLauncher.ts:4306. Normalize both sides throughreserve.turnMode(); fix snippet in the inline comment.
Summary
Review mode: follow-up after new commits
Two Major findings across the full 16-file diff. The prior Cursor inherited-model synchronization issue is addressed; its updated test introduces the type error above.
Testing
Not run (automation; PR code not executed). Inspected current-head CI logs: test fails at typecheck with TS2322; integration and drift-gate completed successfully. After the fixes, rerun typecheck and the steering suites; add Reserve-active automatic/manual steering coverage.
HAPI Bot
|
Addressed the inherited Cursor model Major in b64f7f0: sync |
There was a problem hiding this comment.
Findings
No reportable issues found in the full 16-file diff.
Summary
Review mode: follow-up after new commits
The previous Cursor nullable-model and Codex Reserve hash findings are addressed. Residual testing gap: the new arrival-hook tests cover successful injection; rejection, abort, and uncertain-delivery coverage primarily exercises the shared manual-steer path. Real-backend peer delivery was not validated in this review.
Testing
Not run (automation; PR code not executed). Current-head CI: typecheck, selected browser tests, integration, Windows MCP, and drift-gate passed; the unit-test step remains in progress.
HAPI Bot
09de973 to
e800899
Compare
There was a problem hiding this comment.
Findings
No reportable issues found in the full 18-file diff.
Summary
Review mode: follow-up after new commits
No confirmed regressions found. Residual coverage gap: the new shared Codex tests exercise successful fresh-message steering (cli/src/codex/shared/root.test.ts:125, cli/src/codex/shared/queue.test.ts:130), but do not directly exercise fresh-message rejection falling back to the native queue or indeterminate delivery preventing replay. Real-backend peer delivery was not validated.
Testing
Not run (automation; PR code not executed). Current-head CI: integration, Windows Codex MCP, and drift-gate passed; the test job remains in progress. Suggested regressions: fresh-message rejection, indeterminate dispatch, and turn completion racing auto-steer.
HAPI Bot
e800899 to
e24bb05
Compare
There was a problem hiding this comment.
Review mode: follow-up after new commits
Requirement — Pass
Automatically delivering peer nudges during active turns addresses a supported coordination use case while retaining queued delivery as fallback.
Evidence
- The snapshot description and predecessor PR #1708 describe peer nudges waiting until turn completion.
- Base cli/src/codex/shared/root.ts:119-122 unconditionally queues ordinary incoming messages; shared/src/modes.ts:193-203 documents existing steering capabilities.
Approach — Pass
Reusing existing delivery metadata, session capability checks, and native steering paths is appropriate. The cancellation defect below is fixable within this approach.
Evidence
- cli/src/modules/pingPeer/pingPeer.ts:358-361 supplies a localId and steer intent.
- hub/src/sync/messageService.ts:118-133 gates steering by session capability.
- cli/src/codex/shared/root.ts:127-132 and cli/src/cursor/cursorAcpRemoteLauncher.ts:585-591 connect arrivals to existing steering mechanisms.
Code — Reviewed
Reviewed the full 18-file merge-base diff. Found one cancellation race in the shared Codex fallback.
- [Major] Preserve cancellation when auto-steer falls back to the queue —
cli/src/codex/shared/root.ts:132
Testing
Not run (automation; PR code execution is prohibited).
- No PR code, tests, builds, or scripts were executed.
- Fixed-head CI: integration, windows-codex-mcp, and drift-gate passed; test remained in progress.
- Added shared Codex tests cover successful fresh-message steering, but not rejection racing cancellation or indeterminate fresh-message delivery.
- Diff whitespace checks passed. The previous review commit was unavailable locally, so incremental comparison was unavailable; the full fixed-head merge-base diff was reviewed.
HAPI Bot
There was a problem hiding this comment.
Review mode: follow-up after new commits
Requirement — Pass
Automatically delivering peer nudges during active turns supports timely agent coordination, with queued delivery as fallback.
Evidence
- The snapshot description and predecessor PR #1708 identify peer nudges waiting until turn completion.
- Base cli/src/codex/shared/root.ts:119-122 queues incoming prompts; shared/src/modes.ts:193-203 documents existing steering capabilities.
Approach — Pass
Reusing delivery metadata, session capability checks, and existing steering mechanisms is appropriate. The retry regression below is fixable within this approach.
Evidence
- cli/src/modules/pingPeer/pingPeer.ts:358-361 supplies a localId and steer intent.
- hub/src/sync/messageService.ts:118-133 gates steering by session capability.
- cli/src/codex/shared/root.ts:127-134 and cli/src/cursor/cursorAcpRemoteLauncher.ts:585-591 connect arrivals to existing steering paths.
Code — Reviewed
Reviewed the full 19-file fixed-head merge-base diff. The previous cancellation race is addressed, but making cancellation terminal also breaks explicit retry.
- [Major] Preserve explicit retry after its cancellation handshake —
cli/src/codex/shared/queue.ts:171
Testing
Not run (automation; PR code execution is prohibited).
- Fixed-head CI passed: test, integration, windows-codex-mcp, and drift-gate.
- No PR code, tests, builds, or scripts were executed. Diff whitespace checks passed.
- Added coverage checks fresh-message steering, inherited-model matching, and cancellation beating fallback; it does not cover explicit retry through the cancellation handshake.
HAPI Bot
There was a problem hiding this comment.
Review mode: follow-up after new commits
Requirement — Pass
Delivering peer nudges during active turns supports timely agent coordination, with queued delivery as fallback.
Evidence
- The snapshot description and predecessor PR #1708 identify peer messages waiting until turn completion.
- Base cli/src/codex/shared/root.ts:119-122 queues incoming prompts; shared/src/modes.ts:193-203 documents existing steering capabilities.
Approach — Pass
Reusing delivery metadata, session capability checks, and existing steering paths addresses the requirement. Serializing auto-steer with fallback preserves cancellation ordering while allowing explicit retry.
Evidence
- cli/src/modules/pingPeer/pingPeer.ts:358-361 supplies localId and steer intent; hub/src/sync/messageService.ts:132 gates it by session capability.
- cli/src/codex/shared/root.ts:127-134 and cli/src/cursor/cursorAcpRemoteLauncher.ts:585-591 route arrivals into steering.
- cli/src/codex/shared/queue.ts:236-247 serializes steering and fallback; lines 170-175 permit enqueue after explicit cancellation.
Code — Reviewed
Reviewed the full 19-file merge-base diff at head 2890f9a. The previous retry finding is addressed, and no new actionable defects were found. Runtime behavior was assessed through source and test inspection only.
No reportable code issues found.
Testing
Not run (automation; PR code execution is prohibited).
- Fixed-head CI: integration, windows-codex-mcp, and drift-gate passed; test remained in progress at the last check.
- Added regressions cover fresh-message steering, inherited-model matching, cancellation ordering, and enqueue after cancellation for explicit retry.
- Diff whitespace checks passed. No PR code, tests, builds, or scripts were executed.
HAPI Bot
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
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.
Address bot Majors on tiann#1708: Cursor ACP now consumes deliveryMode steer on arrival (hub gate + inbound hint + soft-steer hook), and Codex rehashes the active turn after model resolution so peer nudges during the first inherited-model turn can auto-steer. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Rehashing the active prompt with currentBackendModel without updating session.getModel() made peer nudges and manual Steer hash differently and refuse mid-turn inject. Pin the session model via previousSetModel and cover with a production-hasher arrival test. Co-authored-by: Cursor <cursoragent@cursor.com>
Normalize steer comparisons through lunaReserve.turnMode so session-facing Luna models still match Reserve turns, and coerce getModel() with ?? undefined for the Cursor auto-steer regression typecheck. Co-authored-by: Cursor <cursoragent@cursor.com>
After the concurrent Codex refactor, production traffic no longer hits codexRemoteLauncher. Honor deliveryMode steer in SharedCodexRoot via queue.steer(freshInput) so mid-turn peer nudges still inject. Co-authored-by: Cursor <cursoragent@cursor.com>
When turn/steer rejects a fresh peer nudge, cancel can mark the ledger canceled before the ordinary enqueue fallback runs. Treat canceled as terminal so successful cancellation cannot be followed by thread/queue/add. Co-authored-by: Cursor <cursoragent@cursor.com>
Keep canceled→enqueue for the explicit retry handshake. Peer auto-steer fallback uses steerThenEnqueue so cancel cannot ACK between turn/steer rejection and thread/queue/add. Co-authored-by: Cursor <cursoragent@cursor.com>
2890f9a to
1873d55
Compare
There was a problem hiding this comment.
Review mode: follow-up after new commits
Requirement — Pass
Automatically delivering peer nudges during active turns supports timely agent coordination, with queued delivery when steering is unavailable.
Evidence
- The snapshot description and predecessor PR #1708 identify peer messages waiting until turn completion.
- Base cli/src/codex/shared/root.ts:119-122 queues incoming prompts; shared/src/modes.ts:193-203 documents existing steering capabilities.
Approach — Pass
Reusing delivery metadata, session capability checks, and existing steering paths addresses the problem. Shared Codex serializes steering and fallback to preserve cancellation ordering.
Evidence
- cli/src/modules/pingPeer/pingPeer.ts:358-359 supplies a localId and steer intent; hub/src/sync/messageService.ts:131 gates it by session capability.
- cli/src/codex/shared/root.ts:127-134 and cli/src/cursor/cursorAcpRemoteLauncher.ts:585-591 route arrivals into steering.
- cli/src/codex/shared/queue.ts:242-246 keeps steering and queued fallback within one serialized operation.
Code — Reviewed
Reviewed the entire 19-file merge-base diff at 1873d55. No actionable defects were found. Assessment used source and test inspection only. The previous review was available, but its commit was unavailable locally for incremental comparison.
No reportable code issues found.
Testing
Not run (automation; PR code execution is prohibited).
- Fixed-head CI: integration and drift-gate passed; test and windows-codex-mcp remained in progress at the last check.
- Added regressions cover fresh-message steering, inherited-model matching, hint propagation, cancellation ordering, and explicit retry after cancellation.
- Diff whitespace checks passed. No PR code, tests, builds, or scripts were executed.
HAPI Bot
Summary
Takeover / continuation of @KevinFun's #1708 (
feat/ping-peer-auto-steer): peerping_peernudges mid-turn are auto-steered into the active turn instead of silently waiting for turn end.Original work (rebased onto current
main):ping_peer/ MCP sendlocalId+deliveryMode: 'steer'steerQueuedByLocalId(turn/steer)MessageQueue2carries optionalsteerHintThis takeover fixes the remaining bot Majors from #1708:
isSteeringSupportedForSession); inbound path forwards the hint; ACP launcher soft-steers on arrival (same path as the manual Steer button).activeMessageso peer nudges hashed with the resolved model match the active turn instead of falling back to the queue.Test plan
bun typecheckbun test src/sync/messageService.test.ts(Cursor ACP keepssteer; Claude downgrades)cursorUserMessageQueue,cursorAcpRemoteLauncher,codexRemoteLauncherunit suites (includes new auto-steer / inherited-model regressions)test+integration+drift-gate+ bot review on this PRCredits: original implementation by @KevinFun in #1708.