feat(oh-my-pi): Cotal connector for oh-my-pi — headless peer + interactive extension - #13
Open
seal-agent wants to merge 15 commits into
Open
feat(oh-my-pi): Cotal connector for oh-my-pi — headless peer + interactive extension#13seal-agent wants to merge 15 commits into
seal-agent wants to merge 15 commits into
Conversation
…ctive extension Adds `@cotal-ai/oh-my-pi`, the Cotal connector for [oh-my-pi](https://github.com/can1357/oh-my-pi). It ships two entry points onto one mesh runtime (`MeshAgent` + the shared `cotal_*` tools from `@cotal-ai/connector-core`), mirroring the existing `@cotal-ai/pi` and `@cotal-ai/connector-opencode` adapters so the tool surface can't drift. ### Headless native-embed peer (`runOmpPeer` / `connector`) Embeds a Cotal endpoint inside an oh-my-pi process and answers mesh traffic through the agent's own loop, driven by the shared `InboxTurn` embed loop — `steer()` folds a same-scope message into an in-flight turn. This is the path a Cotal manager uses to spawn and supervise an oh-my-pi worker (`connector.buildLaunch`). ### Interactive session extension (`src/extension.ts`) A `pi --extension` that joins a human- or Compass-launched oh-my-pi session to the mesh — the interactive sibling of the opencode plugin. It holds a `MeshAgent`, registers the `cotal_*` tools via `pi.registerTool`, maps the session event stream to presence, and delivers inbound mesh traffic with `pi.sendMessage(..., { deliverAs })` (waking an idle session, steering a live one, never interrupting a running turn; acks on turn end so a crash redelivers). Identity comes from `COTAL_*` env — no identity, no join, so a plain `omp` never joins as a stray peer. `pnpm build` bundles it to `dist/extension.bundle.js` (esbuild). ### Fork divergences handled here oh-my-pi is a fork of Pi, so this targets `@oh-my-pi/pi-coding-agent`: retries surface as session `auto_retry_*` events (not an `agent_end.willRetry` flag), and imports use the package subpath entrypoints while the published root type barrel is fixed upstream. Spec-impact: none. Refs Cotal-AI#181 Co-authored-by: Matt Wilkinson <matt@sealedsecurity.com>
bin/cotal.ts imports @cotal-ai/delivery (added with the Plane-3 delivery daemon) but it was never in the root package.json deps, so a clean checkout can't run `cotal` (or `pnpm cotal`) — ERR_MODULE_NOT_FOUND on @cotal-ai/delivery. Co-Authored-By: seal <noreply@sealedsecurity.com>
A task/print/RPC subagent inherits the parent's COTAL_* env, so gating on hasIdentity() alone made every subagent a stray same-named mesh peer (roster pollution + ambiguous DMs, and a subagent could receive traffic meant for the main session). Defer the mesh-join to session_start and start only when ctx.hasUI is true; non-interactive sessions stay off the mesh. Smoke covers both branches. Co-Authored-By: seal <noreply@sealedsecurity.com>
…the host TUI A mid-session mesh drop made MeshAgent log every endpoint reconnect-failure (TIMEOUT) to stderr on a fixed 3s retry, which in the in-process oh-my-pi extension flooded and corrupted the live terminal. Log the drop/recover edges once (suppressing the retry churn), inject a logger so oh-my-pi routes through pi.logger (a file, not the shared terminal), and back the endpoint's reconnect retries off exponentially (3s->30s). Co-Authored-By: seal <noreply@sealedsecurity.com>
…uilds standalone loop.ts imports InboxTurn/InboxSource from @cotal-ai/connector-core, but the connector-core half (inbox-turn.ts + the ackInbox source method + the index export) was dropped when this branch was rebased onto upstream/main, leaving a dangling import that fails tsc. Roll those pieces in from the upstream feat/connector-pi work so the branch builds independently of that open PR while staying rebasable on upstream/main. Co-Authored-By: seal <noreply@sealedsecurity.com>
pi-coding-agent 16.3.7 changed the tool-registry typing so the legacy TypeBox
`defineTool`/`Type` shim no longer infers params (execute's `params` fell to
`unknown`; the result narrowed to `ToolDefinition<ArkSchema, {}>` and broke
`customTools` variance), and `registerTool` began recursing into
`Static<TParams>` on inline zod literals (TS2589 excessively-deep).
Move the peer's cotal_roster/cotal_status off the retired shim to zod schemas
(the SDK's canonical param format — `Static` infers `z.infer` first), and pin
`registerTool<ReturnType<typeof z.object>>` in the extension so the registry
generic no longer deep-infers. Drop the `details: {}` literals that narrowed
TDetails. Bump the dep to ^16.3.12 (latest); the whole build + all connector
smokes are green.
Co-Authored-By: seal <noreply@sealedsecurity.com>
Fix two P1 `loop.ts` delivery bugs (a declined `prompt()` no longer wedges the peer; a rejected `steer()` un-surfaces its id via new `InboxTurn.unsurface` so it redelivers instead of being falsely acked), make the connect-retry backoff and peer shutdown cancellable/idempotent, `await session.dispose()`, and wire the three hermetic oh-my-pi smokes into `pnpm -r test`. Adds regression tests 7-8 for the P1s. Co-Authored-By: seal <noreply@sealedsecurity.com>
The review of the prior commit surfaced real races in it: a folded `steer()` that rejects after `agent_end` acked the turn still lost the message; a `prompt()` settling during shutdown drove a disposed session; and awaiting `dispose()` let its failure skip the caller's `mesh.stop()`. Track pending steers and un-surface any unconfirmed at `agent_end`, guard the prompt callback with a `stopped` flag, and `dispose().catch(log)`. Peer smoke gates the async callbacks on a macrotask drain so the assertions aren't hop-count fragile. Co-Authored-By: seal <noreply@sealedsecurity.com>
`smoke:inbox` and `smoke:reconnect-log` (both added in this PR) ran only by hand. Add a package `test` aggregate so `pnpm -r test` in the unit lane executes them, matching the oh-my-pi connector. Co-Authored-By: seal <noreply@sealedsecurity.com>
The fix-review of c09b21e found two races the finding-level tests missed. Add regression tests that fail on c09b21e and pass on the race fix: - test 9: a rejecting same-scope fold with agent_end emitted before the rejection settles must not be acked (c09b21e acks it → message lost; the fix un-surfaces pending steers at agent_end before commit). - test 10: a rejecting session.dispose() must not reject loop.shutdown() (c09b21e's bare await rejects → peer.ts skips mesh.stop → ghost peer; the fix awaits dispose().catch(log)). Both proven red on c09b21e (tests 9 line 368, 10 line 400), green on head. Co-Authored-By: seal <noreply@sealedsecurity.com>
…rdown The fix-review of the race fix found three deeper races in agent_end's "un-surface every pending fold before commit". All three are real (the mesh dedups only ACKED ids, so an un-acked redelivery re-surfaces to the model): - Accepted-steer redeliver (greptile): a fold whose steer() RESOLVED but whose .then hadn't flushed when agent_end fired was un-surfaced → not acked → redelivered though the model already received it. Now agent_end defers the commit until each fold's steer settles (Promise.race against a one-macrotask boundary so a never-settling steer can't hang the turn): an accepted fold stays acked, only a rejected or stranded one redelivers. - Cross-turn mutation (cubic P1): the reject callback called turn.unsurface() with no turn identity, so a steer settling after its turn committed could strip an id a LATER turn had re-surfaced. A monotonic generation captured at fold time makes a late settle a no-op on any turn but its own. - Post-shutdown dispatch (cubic P2): pump()/foldSameScope()/the session handler now early-return once stopped, so a mesh event landing mid-teardown can't drive a disposed session. The common one-message turn still commits synchronously (no fold → no defer). Tests (red on the pre-fix 6f82f76, green here): accepted-steer-acked, late-settle-no-ops-later-turn, shutdown-blocks-dispatch, strand-safety-no-hang. Co-Authored-By: seal <noreply@sealedsecurity.com>
…l runs The sibling of the dispose-teardown fix: shutdown() awaited session.abort() bare, so a rejected abort() rejected shutdown() → skipped dispose() AND the caller's mesh.stop() → ghost peer on the mesh. Handle it like dispose(): await session.abort().catch(log). Regression test (red on the pre-fix commit, green here): a rejecting abort still resolves shutdown() and dispose() still runs (disposed === 1). Co-Authored-By: seal <noreply@sealedsecurity.com>
Two review findings on the deferred-commit + shutdown hardening: - Slow-accept redeliver: the deferred commit raced pending fold steers against a setTimeout(0) macrotask boundary. A steer accepting after that 0ms tick was treated as undelivered and redelivered though the model already had it. Replace the 0ms boundary with an injectable steerSettleTimeoutMs (default 5s): a healthy steer settles in <=1 microtask so allSettled wins by orders of magnitude and the timeout never fires on the happy path; it only bounds a genuinely stuck steer so the turn can't wedge. Correct by a real margin, not by microtask-vs-macrotask ordering. - Shutdown teardown coupling: abort() and dispose() now sit in independent try/catch blocks, so a failed OR synchronously-thrown abort() still runs dispose() and still resolves shutdown() — peer.ts's mesh.stop() always runs, no ghost peer. (A single wrapping try/catch would let an abort failure skip dispose.) Tests (red on the pre-fix boundary/shutdown, green here): slow-accept steer is acked under the human-scale timeout; a sync-throwing abort still resolves shutdown and runs dispose. Timeout is injectable so tests don't wait the real delay. Co-Authored-By: seal <noreply@sealedsecurity.com>
The human-scale steerSettleTimeoutMs timer added to bound the deferred commit was never cleared on the common path (allSettled wins before it fires). An uncleared setTimeout stays ref'd on the Node event loop, so every folded turn delayed process/CLI exit by up to the timeout — negligible at the old 0ms boundary, but up to 5s per turn at the new human-scale default. Clear it in a finally so the happy path leaves no live handle; the timeout still bounds a genuinely-stuck steer. Test 18 (red on an uncleared-timer loop, green here): after a fold commits via allSettled, the active Timeout-handle count is unchanged (no leak). Test 16 now exercises the production default (5s) rather than an injected 50ms — safe only because the timer is cleared, and it makes the suite exit promptly instead of hanging on a leaked handle. Co-Authored-By: seal <noreply@sealedsecurity.com>
Rebased the connector stack onto upstream/main; regenerate pnpm-lock.yaml so it reflects @cotal-ai/oh-my-pi and its @oh-my-pi/pi-coding-agent + zod deps against the current dependency graph. Co-Authored-By: seal <noreply@sealedsecurity.com>
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.
Adds
@cotal-ai/oh-my-pi, the Cotal connector for oh-my-pi. It ships two entry points onto one mesh runtime (MeshAgent+ the sharedcotal_*tools from@cotal-ai/connector-core), mirroring the existing@cotal-ai/piand@cotal-ai/connector-opencodeadapters so the tool surface can't drift.Headless native-embed peer (
runOmpPeer/connector)Embeds a Cotal endpoint inside an oh-my-pi process and answers mesh traffic through the agent's own loop, driven by the shared
InboxTurnembed loop —steer()folds a same-scope message into an in-flight turn. This is the path a Cotal manager uses to spawn and supervise an oh-my-pi worker (connector.buildLaunch).Known limitation — the
connector.buildLaunchspawn path does not currently load; see Open Question 2. The in-process embed logic (runPeerLoop, and theInboxTurnfold/steer/settle/shutdown handling) is complete and unit-covered; the gap is only the process launcher.Interactive session extension (
src/extension.ts)A
pi --extensionthat joins a human- or Compass-launched oh-my-pi session to the mesh — the interactive sibling of the opencode plugin. It holds aMeshAgent, registers thecotal_*tools viapi.registerTool, maps the session event stream to presence, and delivers inbound mesh traffic withpi.sendMessage(..., { deliverAs })(waking an idle session, steering a live one, never interrupting a running turn; acks on turn end so a crash redelivers). Identity comes fromCOTAL_*env — no identity, no join, so a plainompnever joins as a stray peer.pnpm buildbundles it todist/extension.bundle.js(esbuild).Fork divergences handled here
oh-my-pi is a fork of Pi, so this targets
@oh-my-pi/pi-coding-agent: retries surface as sessionauto_retry_*events (not anagent_end.willRetryflag), and imports use the package subpath entrypoints while the published root type barrel is fixed upstream.Open questions for review (parked)
1. Base staleness. This PR is based on
sealedsecurity/Cotalmain(d55c6adb), which mirrors an older Cotal-AI upstream (~PR Cotal-AI#179 + the sealed cotal-mint commits), whereas the connector was originally authored against a more recent upstream (PR Cotal-AI#218). The rebase onto the current fork base is clean and the connector type-checks and bundles green against it (@cotal-ai/connector-coretscand@cotal-ai/oh-my-pitsc --noEmit+ esbuild all pass), andpnpm install --frozen-lockfilepasses — so landing on the fork base as-is is viable. The alternative is to first syncsealedsecurity/Cotalmainup to current Cotal-AI upstream (shrinking the delta and landing the connector on newer substrate). That's amain-moving decision outside this lane; flagging it here for the maintainer to choose at merge time.2. Headless spawn path fails at module load. Grounded in the clone at head
c75e85cf:connector.tslaunches the peer withcommand: TSX(node, via tsx).main.tsvalue-importspeer.ts, which value-imports@oh-my-pi/pi-coding-agent/sdk; that runtime transitively importsbun(@oh-my-pi/pi-utils/src/frontmatter.ts), which node cannot resolve.loop.tsdocuments exactly this ("the oh-my-pi runtime pulls inbun, which is unloadable off-Bun") and is deliberately type-only to stay node-safe — but the launcher still spawns under tsx andmain->peerpulls the runtime in anyway.packages/core/src/agent-file.tsimportsyaml, which@cotal-ai/core'spackage.jsondoes not declare (onlyimplementations/cliandimplementations/managerdo). Under pnpm isolated linking both node and bun fail there first.tsx main.tsdies on the missingbunpackage; withyamlresolved and the same entrypoint run under Bun, the peer loads its full runtime and reaches mesh-connect. So a working fix = declareyamlinpackages/core+ switch the launcher tsx->bun + add a load-path smoke that imports the SDK under the target interpreter (so this regresses loudly, not at first manager spawn).--external:@oh-my-pi/*, runs in the Bun host); the three smokes stub the SDK, so a green CI does not exercise this path.Options: (A, recommended) land interactive-first — soften the headless section and file a follow-up for the spawn-path fix — keeping this PR single-purpose and off shared
packages/core(theyamlphantom dep is a pre-existing shared-core bug that shouldn't ride a connector PR). (B) fix headless in this PR — declareyamlinpackages/core, switch the launcher tsx->bun, add the load-path smoke — delivering the full capability but widening into shared substrate and changing the launch-interpreter contract. Maintainer's call.3. Incidental workspace dependency. This PR's root
package.jsonadds@cotal-ai/delivery: workspace:*, carried in from the connector branch's rebase base. It is benign — a workspace link backing the pre-existing rootsmoke:delivery-*scripts, withpnpm install --frozen-lockfilepassing — but unrelated to the connector. Keep it as an incidental workspace-graph fix, or drop it; nothing the connector adds needs it.4. Dead sibling-adapter references. The README and
loop.tspoint at a@cotal-ai/piadapter (../pi), which exists in Cotal-AI upstream but not on this fork base (extensions/piis absent atd55c6adb), so the relative doc links 404 here. The real in-tree structural sibling isconnector-opencode(the interactive loop mirrors itsplugin.ts). Repoint to the in-tree sibling, or keep the upstream reference explicit and drop the../pirelative links. Tied to OQ1 (resolves itself if the fork base syncs forward).Spec-impact: none.
Refs Cotal-AI#181
Co-authored-by: Matt Wilkinson matt@sealedsecurity.com