Repository navigation
feat(omp): open OMP in the structured chat over ACP - #26401
Conversation
OMP joins Grok and OpenCode on the ACP lane: `omp acp`, with the user's environment passed through and the chat pinned to PI_CODING_AGENT_DIR (default ~/.omp/agent), the directory OMP's terminal chats read. A create and every launch ask `omp --version` of the binary the Command setting resolves to; stable releases from 17.0.5 run the chat, older ones keep the terminal-backed chat. Like every ACP agent it runs only with the updated structured native chat setting on.
OMP's ACP server sends a command's result as rawOutput {content, details}
with a non-zero exit at details.exitCode, and repeats it as content behind
a "$ <command>" echo. The shared reader takes content text first, so the
row's output began with the command and carried no exit code. The OMP
dialect drops the echo, moves the result text to stdout without OMP's
"Wall time" and "Command exited with code N" notice lines, and maps
details.exitCode. Recordings of omp acp 17.0.5 (exit 3, exit 0, Stop
mid-command) replay through the adapter.
|
Status: ready. With "Use updated structured native chat" on, OMP 17.0.5+ opens in Orca's chat view through
|
📝 WalkthroughWalkthroughThe change adds OMP as an ACP agent with a structured-launch version floor of 17.0.5. It adds normalization for OMP tool updates, including command-echo handling and output cleanup. Runtime tests cover launch configuration, environment handling, binary selection, and launch modes. Recorded-session tests cover successful and failed shell commands, plus cancellation during a running command. Priority: ➖ Normal Merge Risk: 🔵 Low · up to OMP structured chat is opt-in and works for the default setup. Users with OMP profiles or a custom config root may reopen chats with the wrong agent folder. Users with "always ask" approval cannot run commands in structured chat. These limitations are acceptable for merge with follow-up awareness. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 31.25% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 9 files. (3 skipped: 3 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
d582d951-3166-414e-ab72-87426ac744c7
📒 Files selected for processing (12)
src/main/acp/acp-dialects/omp-dialect.test.tssrc/main/acp/acp-dialects/omp-dialect.tssrc/main/acp/acp-launch-specs.tssrc/main/acp/acp-structured-launch-resolution-omp.test.tssrc/main/acp/acp-structured-omp-recordings.test.tssrc/main/acp/fixtures/omp-v17-shell-exit-3.jsonlsrc/main/acp/fixtures/omp-v17-shell-ok.jsonlsrc/main/acp/fixtures/omp-v17-stop-mid-tool.jsonlsrc/main/agent-cli-version-probe.test.tssrc/main/agent-cli-version-probe.tssrc/main/runtime/structured-agent-launch-support-probe-omp.test.tssrc/main/runtime/structured-agent-runtime-registrations-acp.test.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.
| account: directoryAccountBinding('PI_CODING_AGENT_DIR', (homePath) => | ||
| join(homePath, '.omp', 'agent') | ||
| ), |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Pin the directory OMP actually uses.
When PI_CONFIG_DIR changes the default config root, this binding still sets PI_CODING_AGENT_DIR to ~/.omp/agent. OMP then starts with a different agent directory from the one the user configured. Named profiles introduce a separate persistence failure: OMP ignores PI_CODING_AGENT_DIR for them, but this binding records the default directory. If the user changes OMP_PROFILE before reopening a chat, the launch can select another profile instead of the chat's original profile. Resolve the effective directory from OMP's environment and persist the profile selection needed to restore it. (github.com)
| // Stable releases from 17.0.5, the release verified to serve `omp acp`. | ||
| supportsVersion: (version) => isStableCliVersionFrom(version, '17.0.5') |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Do not select ACP when OMP cannot honor its approval mode.
The version gate admits an OMP installation configured with tools.approvalMode: always-ask. The PR reports that commands are denied in that mode under ACP, so an otherwise supported structured chat cannot complete command requests. Keep that configuration on the terminal path until ACP can complete its approval flow. Do not bypass the user's approval mode to make commands run. OMP documents that always-ask requires approval for execution tools. (github.com)
1 participant
ELI5
OMP ("Oh My Pi", the
ompCLI) only had its terminal chat in Orca. With Settings > Chat UI > "Use updated structured native chat" on, OMP 17.0.5 and later now open in Orca's own chat view, like OpenCode and Grok. With the setting off, nothing changes.What Changed
The problem. Orca draws its own chat view for Claude, Codex, Grok, OpenCode and Pi, but OMP users still got only the terminal chat. OMP already speaks the Agent Client Protocol (ACP), the open protocol Orca's Grok and OpenCode chats use, through
omp acp.What is different for you.
How.
src/main/acp/acp-launch-specs.ts:omp acp, a small OMP dialect (acp-dialects/omp-dialect.ts) that drops the$ commandecho OMP puts first in every shell tool message and reads the output and exit code from where OMP reports them, so the row shows them (a successful command shows its output only: OMP sends an exit code only when it isn't 0), and stable releases from 17.0.5 on any later line (isStableCliVersionFrom, a small helper added toagent-cli-version-probe.ts; the existingisStableCliVersionOnLinenow uses it).PI_CODING_AGENT_DIR, which defaults to~/.omp/agent. This is the same way Grok recordsGROK_HOME, and it is the folder Orca's terminal OMP support already treats as OMP's home. Everything else in your environment reaches OMP unchanged; nothing is refused because of what it contains.Why
The common pattern runs OMP as an ACP agent through
omp acp, so this reuses Orca's existing ACP chat lane instead of Pi's RPC lane.Differences from the common pattern:
omp acp). Older OMP keeps the terminal chat. Orca's terminal support suggestsacpmay be older, but no earlier release was verified.tools.approvalMode: always-ask) denies every command when OMP runs under ACP, because it has no dialog there. With OMP's default setting, approvals come through Orca's permission card. This is OMP's behaviour, not Orca's.OMP_PROFILE/PI_PROFILE), OMP uses the profile's own folder and ignores the recorded one. The chat still works, but the record names~/.omp/agent. Follow-up: record the profile's folder using Orca's existing OMP folder rules (src/main/ai-vault/omp-session-root.ts).Linked Issue
Follow-up to #25845 (OpenCode over ACP) and #25851 (Pi); no separate issue.
Visual Proof
Live QA in a hidden Orca build of
fd59b324085on macOS, with the installed OMP 17.0.5 selected through the Command setting. The QA machine's real~/.codex,~/.claude,~/.orca,~/.grokfiles and OMP'sconfig.ymlwere hashed before and after every check, and none of them changed.omp --version: 17.0.5 is supported; repliesOMP-OKprintf hi; exit 3onfd59b324085: the permission card shows OMP's own options (Allow once / Always allow / Reject / Always reject); after Allow once, the row said1 failedbut showed neither the outputhinor the exit coded3075eccf68(the fix): the row shows outputhiandexit 3d3075eccf68: a second message answers within seconds after a plain reply, after a failed command, and after a successful command;printf ok's row showsoksleep 30: "Interrupted", file never written, OMP process gone within 6 s, the next message runs on a new process and remembers the conversationOne earlier run saw a second message get no reply within 35 seconds after a failed command; the longer re-check above (3-minute waits, three variants) did not reproduce it, so its cause is unknown. Not verified live: an OMP profile, Windows, Linux, SSH and mobile.
Testing
I manually tested these changes locally
Automated tests added/updated, or explained why not below
New tests: versions admitted (17.0.5, 17.2.12, 18.4.5) and refused (17.0.4, 16.9.0, a pre-release, unreadable);
omp acpspawned with the recordedPI_CODING_AGENT_DIRand the user's variables; a record for another agent's folder refused; the version check asks exactly the binary, folder and environment it launches; OMP registered only with the setting on; stand-inompscripts for the Command setting, the per-agent PATH, an old OMP keeping the terminal chat, and an unrunnable Command (acp-structured-launch-resolution-omp.test.ts,structured-agent-launch-support-probe-omp.test.ts,structured-agent-runtime-registrations-acp.test.ts,agent-cli-version-probe.test.ts).CI on the final head
d3075eccf68: every check passed (18 passed, 17 skipped).Recordings: real
omp acp17.0.5 traffic (a failing command, a successful command, Stop mid-command), scrubbed, undersrc/main/acp/fixtures/omp-v17-*.jsonl, replayed byacp-structured-omp-recordings.test.ts;omp-dialect.test.tscovers tools the mapping must leave alone.Locally on
fd59b324085: both typechecks, lint, and the changed-code quality gate pass. 33 test files (279 tests) importing the changed modules, plus the renderer route and launch tests, pass.