fix(connector): name the oh-my-pi session after COTAL_NAME at join - #6
Conversation
|
Warning Review limit reached
Next review available in: 16 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe OMP extension's session_start handler was changed to async, adding a detached best-effort session rename to config.name after mesh start, gated on interactivity and existing session name, with errors caught and logged. The corresponding smoke test was expanded to verify this behavior across several scenarios. ChangesSession Title Rename on session_start
Estimated code review effort: 2 (Simple) | ~12 minutes Sequence Diagram(s)sequenceDiagram
participant Pi
participant Extension
participant MeshAgent
participant SessionManager
Pi->>Extension: session_start(ctx)
alt ctx.hasUI === false
Extension-->>Pi: no-op, no rename
else interactive
Extension->>MeshAgent: start()
Extension->>SessionManager: getSessionName()
alt name is undefined
Extension->>Pi: setSessionName(config.name)
alt setSessionName rejects
Extension->>Extension: catch and log warning
end
else name already set
Note over Extension: skip rename
end
end
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
Greptile SummaryThis PR names interactive oh-my-pi mesh sessions after the configured Cotal identity.
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (3): Last reviewed commit: "fix(connector): never gate the mesh-join..." | Re-trigger Greptile |
There was a problem hiding this comment.
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
7214267 to
aacbea9
Compare
The launcher forwards COTAL_NAME but OMP had no agent-reachable way to set the session (pane/terminal) title, so every agent's title was wrong. The extension now calls `pi.setSessionName(config.name)` at session_start on the interactive branch, guarded on an unset name so a resumed or manually-renamed session is never clobbered, and best-effort so a title failure never breaks the mesh-join. Co-Authored-By: seal <noreply@sealedsecurity.com>
greptile + cubic both flagged that `await pi.setSessionName()` ran before agent.start(), so a getSessionName() throw (session manager not ready) or a setSessionName() promise that stalls instead of rejecting would leave the pane off the mesh. Start the join first (non-blocking background connect), then do the title work in a detached fire-and-forget IIFE with getSessionName() inside the try — a title failure of any kind can no longer block connect. Smoke: flush the detached title work before asserting it (cases b/c/d no longer race) and add case (e) — getSessionName() throws => agent.start still fires, no rename, handler resolves. Red-green demonstrated: (e) reddens on the pre-fix ordering, green after. Co-Authored-By: seal <noreply@sealedsecurity.com>
aacbea9 to
4222fdd
Compare
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>
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>
fix(connector): deliver peer messages as nextTurn, not steer
Names the oh-my-pi mesh session after
COTAL_NAMEso every agent's pane/terminal title reflects who it is, instead of a generic auto-title.Problem
The connector launcher forwards
COTAL_NAMEinto the spawned session's environment, but the extension had no agent-reachable way to set the session (pane/terminal) title — so every agent showed a generic auto-title regardless of its mesh identity. In a multi-pane wave that makes panes indistinguishable.Fix
In the
session_starthandler (extensions/connector-oh-my-pi/src/extension.ts), after thehasUI/identity guard andstarted = true, callpi.setSessionName(config.name)when a name is resolved. Properties:!ctx.sessionManager.getSessionName()), so a resumed or manually-/rename'd session is never clobbered.setSessionNamefailure is caught and logged, never breaking the mesh-join (the title is cosmetic; the join is not).hasUIgate, so subagent/print/RPC sessions stay off the mesh and untouched.Test
Extends
extensions/connector-oh-my-pi/oh-my-pi-extension.smoke.tswith session-name regression coverage (4 behaviors): title set when unset, no-clobber when already named, non-interactive skip, and best-effort onsetSessionNamerejection. Red-green demonstrated for the set + no-clobber cases.pnpm --filter @cotal-ai/oh-my-pi run smoke:extension→ green; connector typecheck + build green.Base
Stacked on
zheng-connector-upstream(PR #5, the zod-migrated SDK-16.3.12 connector that ownsextension.ts'ssession_starthandler). Retargets tosealed-forkwhen #5 merges.Co-Authored-By: seal noreply@sealedsecurity.com