fix(connector): deliver peer messages as nextTurn, not steer by sealedsecurity-bot · Pull Request #8 · sealedsecurity/Cotal

sealedsecurity-bot · 2026-07-09T01:20:11Z

Problem

Hitting ESC to interrupt a running OMP turn while a cotal message is waiting bled the message text into Matt's editable composer/text input, instead of holding it and redelivering it as a peer message on the next turn.

Root cause

The interactive connector delivered every inbound mesh message with deliverAs: "steer" (interactive-loop.ts). OMP routes a steer two ways (agent-session.ts sendCustomMessage, 7457-7499):

idle → #promptAgentInitiatedMessage — a fresh turn (the happy path).
streaming/unwinding → this.agent.steer() (7466), which folds into the live turn and, during an ESC-abort teardown, surfaces into the editable pending-message UI = the composer. ESC latches #advisorAutoResumeSuppressed = true (7766-7768) and isStreaming stays true through the unwind (it also counts #promptInFlightCount, 5211-5213).

The connector's no-interrupt gate (busy, flipped by the external agent_end notification) can race ahead of OMP's abort-unwind window, so a flushed message is steered in while OMP is still unwinding → composer bleed.

The connector cannot observe the interrupt to guard against it: AgentEndEvent carries no reason/abort field (shared-events.d.ts:152-155), and ExtensionContext exposes isIdle()/hasPendingMessages() but no aborting signal (extensions/types.d.ts:229-234). A "hold only when interrupted" guard would need an OMP-side signal (cross-repo). So the fix must be a delivery mode that is hidden-from-composer by contract, unconditionally.

Fix

Deliver with deliverAs: "nextTurn" instead of "steer" (the contract, types.d.ts:850-859; routing, agent-session.ts:7458-7479):

idle + triggerTurn → the same #promptAgentInitiatedMessage fresh turn as steer — happy path byte-identical.
still-unwinding after ESC → parked in the hidden #queueHiddenNextTurnMessage queue and redelivered on the next clean turn — never this.agent.steer(), never the composer.

The connector never intends a mid-turn fold (drive() early-returns while busy), so steer's only distinguishing behavior was exactly the misroute frame. nextTurn loses zero intended behavior and preserves ack-on-turn-end. Two-line behavioral change (interface option type + the single call site).

Regression test (red → green)

interactive-loop.smoke.ts test 9 models OMP's documented routing in FakeHost (steer-during-interrupt → composer; nextTurn → hidden held queue; cited to agent-session.ts:7458-7479 so the fake can't silently drift). It asserts the user-visible landing zone, not the envelope string:

A DM arrives mid-turn (buffered by the no-interrupt gate).
User ESCs; the turn-end hook flushes it while unwinding.
Assert: the DM text is HELD (not in the composer), stays on the inbox, and — the other half of the acceptance — is delivered + acked on the next clean turn-end (held-AND-delivered, not held-AND-dropped).

RED on the pre-fix steer envelope (FAIL: 9) message must NOT bleed into the composer (got [...📨 DM from Alice...])), GREEN after the flip. Tests 1-8 unchanged.

Verification

pnpm test (connector): peer 10/10, extension 4/4, interactive-loop 9/9 — all green.
pnpm typecheck: clean.

Stack

Stacked on #6 (session-title) — both are interactive-loop.ts-side connector fixes, disjoint from the headless loop.ts path. Kept as a separate PR (distinct fix, independent regression test, independent revertability).

Co-Authored-By: seal noreply@sealedsecurity.com

Hitting ESC to interrupt a running OMP turn while a cotal message was waiting bled the message text into the editable composer instead of holding it for redelivery. The connector delivered every inbound mesh message with deliverAs:"steer". When idle that wakes a fresh turn, but a steer arriving while OMP is still tearing down a user-interrupted (ESC) turn folds via agent.steer() into the editable pending- message UI = the composer (agent-session.ts:7466; the ESC path latches #advisorAutoResumeSuppressed at 7766-7768 and isStreaming stays true through the unwind). The connector cannot observe the interrupt to guard against it — AgentEndEvent carries no reason and ExtensionContext exposes no aborting signal — so it must deliver in a mode that is hidden-from-composer by contract. deliverAs:"nextTurn" is that mode (agent-session.ts:7458-7479): idle+triggerTurn routes to the same #promptAgentInitiatedMessage fresh turn as steer, while the still-unwinding case is parked in the hidden #queueHiddenNextTurnMessage queue and redelivered on the next clean turn — never the composer. The connector never intends a mid-turn fold (drive() early-returns while busy), so steer's only distinguishing behavior was exactly the misroute frame; nextTurn loses zero intended behavior and preserves ack-on-turn-end. Regression: interactive-loop.smoke.ts test 9 models OMP's routing in FakeHost (steer-during-interrupt -> composer; nextTurn -> hidden held queue) and asserts a DM queued mid-turn lands HELD not composer, then is delivered + acked on the next clean turn-end. Fails on the pre-fix steer envelope, passes after. Co-Authored-By: seal <noreply@sealedsecurity.com>

coderabbitai · 2026-07-09T01:20:34Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 79be8e44-9b10-423a-9e4c-27ce9b53844c

📥 Commits

Reviewing files that changed from the base of the PR and between 905a6a4 and 8b98869.

📒 Files selected for processing (2)

extensions/connector-oh-my-pi/interactive-loop.smoke.ts
extensions/connector-oh-my-pi/src/interactive-loop.ts

🚧 Files skipped from review as they are similar to previous changes (2)

extensions/connector-oh-my-pi/src/interactive-loop.ts
extensions/connector-oh-my-pi/interactive-loop.smoke.ts

📝 Walkthrough

Summary by CodeRabbit

Bug Fixes
- Improved handling of interrupted turns so incoming messages are buffered and redelivered more reliably instead of appearing in the wrong place.
- Fixed ESC-interrupt behavior to ensure buffered messages stay available for retry and are properly acknowledged after the next clean turn.
- Updated smoke coverage for this delivery flow to prevent regressions.

Walkthrough

The PeerHost.sendMessage contract and its drive() call site now use deliverAs: "nextTurn" with triggerTurn: true. The smoke harness updates its fake host routing to separate turns, held, and composer content, and adds an ESC-interrupt scenario that exercises held redelivery.

Changes

nextTurn delivery mode

Layer / File(s)	Summary
PeerHost contract and drive call `extensions/connector-oh-my-pi/src/interactive-loop.ts`	`PeerHost.sendMessage` now accepts `deliverAs: "nextTurn"`, and `drive()` sends injected content with that mode and updated comments about ESC unwinding.
Smoke harness routing and ESC scenario `extensions/connector-oh-my-pi/interactive-loop.smoke.ts`	`SentCall` and envelope assertions now expect `nextTurn`; `FakeHost` routes content through `turns`, `held`, and `composer` with `interruptUnwinding`; a new "9. ESC-interrupt" test verifies held redelivery and inbox draining.

Estimated code review effort: 2 (Simple) | ~15 minutes

Poem

A rabbit hops on nextTurn tracks,
No steer, no slip, no sudden cracks.
Held hops wait, then leap in time,
The inbox hums in tidy rhyme.
🐇 Hop! The turn goes round and round.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately names the main behavior change from steer to nextTurn for peer message delivery.
Description check	✅ Passed	The description is directly about the same connector routing bug, fix, and regression test.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch cotal-connector-esc-composer

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

greptile-apps · 2026-07-09T01:22:24Z

Greptile Summary

This PR changes how the interactive OMP connector delivers peer messages.

Switches peer delivery from steer to nextTurn.
Updates the host interface to require nextTurn delivery.
Adds smoke coverage for ESC-interrupt delivery into the hidden next-turn queue.
Documents why acking is expected to happen after the deferred delivery completes.

Confidence Score: 5/5

This looks safe to merge.

No blocking issues found in the changed code.

Important Files Changed

Filename	Overview
extensions/connector-oh-my-pi/src/interactive-loop.ts	Changes interactive peer-message delivery to `nextTurn` and documents the turn-end ack ordering.
extensions/connector-oh-my-pi/interactive-loop.smoke.ts	Adds fake-host routing for hidden next-turn delivery and covers the ESC-interrupt message path.

_{Reviews (3): Last reviewed commit: "docs(connector): explain why ESC-interru..." | Re-trigger Greptile}

cubic-dev-ai

All reported issues were addressed across 2 files

_{Heads up: you’re close to your included review allowance. Set a flex budget so reviews don’t pause.

Fix all with cubic | Re-trigger cubic}

cubic P3 on #8: the ESC regression asserted the inbox drains on the next clean turn-end, but FakeHost never modeled the held nextTurn content actually being delivered into a turn — so it proved held-AND-acked, not held-AND-delivered (the drain came purely from the connector's surfaced bookkeeping). A future regression where the hidden queue never redelivers would still pass. Model OMP's deferred continuation (#promptQueuedHiddenNextTurnMessages, agent-session.ts:7323-7346) with FakeHost.flushHeld(): it drains the hidden next-turn queue into consumed turn content, exactly as a clean redelivery does. Test 9 now flushes the held message into a real turn and asserts it was consumed BEFORE the clean turn-end acks it — so the ack is proven to follow an actual delivery, closing cubic's gap and mirroring the real deliver-then-ack ordering. Refs #3. Co-Authored-By: seal <noreply@sealedsecurity.com>

…sage Capture the coalescing invariant at the ack site (greptile P1 on #8): a future reader touching ackSurfaced must see WHY a nextTurn message parked during an ESC unwind can't be acked before it's consumed. OMP holds the wire-level agent_end while #promptInFlightCount > 0 and lets a later one supersede it (agent-session.ts:2787-2799), so a wire-level subscriber sees one agent_end at the true settle — the interrupted turn and the deferred continuation collapse into a single post-consume event. Backstop: ackSurfaced drains only front-matching ids, so an unconsumed survivor redelivers (fails safe). Comment only, no behavior change. Refs #3. Co-Authored-By: seal <noreply@sealedsecurity.com>

CodeRabbit flagged step 4 vs task B1 naming the same call two ways (`pi.sendMessage` vs `host.sendMessage`), both attaching a `<CotalInjectionDetails>` type-arg to the non-generic `PeerHost` seam. Name the single send path `host.sendMessage` (host binds to `pi` at `extension.ts:99`) and state the real mechanism: widening the seam's `details` field type-checks the payload; the seam stays non-generic (no type-arg at the call), and OMP's generic `sendMessage<T>` keeps it sound end-to-end. Also correct source line anchors that had drifted to a divergent local checkout: on PR #8, `text = override` is `:74`, the mention-recall caller `:127`, the `formatInjection` call `:79`, and `host` binds to `pi` at `extension.ts:99`. Co-Authored-By: seal <noreply@sealedsecurity.com>

Structured per-item rendering of inbound peer messages (via sendMessage details + registerMessageRenderer) and renderCall/renderResult on the cotal_* tools to leave OMP's animated-spinner fallback. Two independent workstreams; implementation gated on #8's fork-base clearing, the design record is not. Co-Authored-By: seal <noreply@sealedsecurity.com>

CodeRabbit flagged step 4 vs task B1 naming the same call two ways (`pi.sendMessage` vs `host.sendMessage`), both attaching a `<CotalInjectionDetails>` type-arg to the non-generic `PeerHost` seam. Name the single send path `host.sendMessage` (host binds to `pi` at `extension.ts:99`) and state the real mechanism: widening the seam's `details` field type-checks the payload; the seam stays non-generic (no type-arg at the call), and OMP's generic `sendMessage<T>` keeps it sound end-to-end. Also correct source line anchors that had drifted to a divergent local checkout: on PR #8, `text = override` is `:74`, the mention-recall caller `:127`, the `formatInjection` call `:79`, and `host` binds to `pi` at `extension.ts:99`. Co-Authored-By: seal <noreply@sealedsecurity.com>

greptile-apps Bot reviewed Jul 9, 2026

View reviewed changes

Comment thread extensions/connector-oh-my-pi/src/interactive-loop.ts

coderabbitai Bot approved these changes Jul 9, 2026

View reviewed changes

cubic-dev-ai Bot reviewed Jul 9, 2026

View reviewed changes

Comment thread extensions/connector-oh-my-pi/interactive-loop.smoke.ts Outdated

mattwilkinsonn and others added 2 commits July 8, 2026 21:30

mattwilkinsonn mentioned this pull request Jul 10, 2026

docs(platform): design the native cotal connector renderer #10

Open

sealedsecurity-bot mentioned this pull request Jul 11, 2026

feat(connector-oh-my-pi): render cotal_* tool calls natively (no spinner fallback) #11

Closed

mattwilkinsonn merged commit 6a9af43 into cotal-connector-session-title Jul 25, 2026
18 checks passed

mattwilkinsonn deleted the cotal-connector-esc-composer branch July 25, 2026 22:07

Navigation Menu

Uh oh!

fix(connector): deliver peer messages as nextTurn, not steer - #8

fix(connector): deliver peer messages as nextTurn, not steer#8
mattwilkinsonn merged 3 commits into
cotal-connector-session-titlefrom
cotal-connector-esc-composer

sealedsecurity-bot commented Jul 9, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jul 9, 2026 •

edited

Loading

Summary by CodeRabbit

Walkthrough

Changes

Uh oh!

greptile-apps Bot commented Jul 9, 2026 •

edited

Loading

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

sealedsecurity-bot commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Root cause

Fix

Regression test (red → green)

Verification

Stack

Uh oh!

coderabbitai Bot commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Walkthrough

Changes

Uh oh!

greptile-apps Bot commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sealedsecurity-bot commented Jul 9, 2026 •

edited

Loading

coderabbitai Bot commented Jul 9, 2026 •

edited

Loading

greptile-apps Bot commented Jul 9, 2026 •

edited

Loading

cubic-dev-ai Bot left a comment •

edited

Loading