feat: add full Oh My Pi (OMP) support - #256
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
|
Addressed the multi-root session review in f979800. Dashboard discovery now canonicalizes paths, deduplicates by session ID, sorts duplicates by modified time descending with a stable path tie-break, and therefore keeps the newest continued copy rather than root-order priority. Added regression coverage for a newer duplicate in the second root; the production module now passes 9 isolated Rust tests. |
There was a problem hiding this comment.
All reported issues were addressed across 42 files
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
|
This is on our merge path — could you rebase onto current master? The v0.33.1 release wave landed conflicting changes in the areas this touches. Once it's green we'll do the full review pass. |
Co-authored-by: Wine Fox <fox@ling.plus>
Co-authored-by: Wine Fox <fox@ling.plus>
Co-authored-by: Wine Fox <fox@ling.plus>
Co-authored-by: Wine Fox <fox@ling.plus>
Co-authored-by: Wine Fox <fox@ling.plus>
Co-authored-by: Wine Fox <fox@ling.plus>
Co-authored-by: Wine Fox <fox@ling.plus>
Co-authored-by: Wine Fox <fox@ling.plus>
Co-authored-by: Wine Fox <fox@ling.plus>
Co-authored-by: Wine Fox <fox@ling.plus>
f979800 to
5754598
Compare
|
Rebased I also resolved the valid review findings while re-verifying the v0.33.1 contracts: Pi setup now rolls back a newly-added package entry on later failure; OMP doctor can repair shared config without an OMP binary; OMP subprocess errors preserve timeout details and use a 10 MiB buffer; dashboard Pi login-shell discovery precedes OMP fallback; OMP profile names are validated; multi-root dedup uses explicit root priority and canonicalizes roots rather than every session; the homepage support matrix now includes OMP. Post-rebase verification:
|
There was a problem hiding this comment.
Pull request overview
This PR adds first-class Oh My Pi (OMP) support across Magic Context’s CLI, Pi-compatible extension/runtime, dashboard session/model discovery, and documentation—while keeping Pi-compatible session semantics (harness='pi') and shared project memory/database behavior.
Changes:
- CLI: introduce an
ompharness with setup/doctor flows, OMP model discovery, OMP path resolution, and OpenCode → Pi/OMP migration targeting. - Dashboard: expand Pi-compatible session-root discovery to include OMP profile/XDG layouts and add OMP model selector parsing.
- Extension + docs: update Pi extension to support OMP env semantics (agent dir allowlist resolution) and document installation, conflicts, migration, and dashboard behavior.
Reviewed changes
Copilot reviewed 42 out of 43 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates top-level product messaging and setup/doctor guidance to include OMP. |
| packages/plugin/scripts/build-config-docs.ts | Updates generated config-doc preface to reflect shared config across OpenCode/Pi/OMP. |
| packages/pi-plugin/src/subagent-runner.ts | Resolves relative allowlisted extensions from PI_CODING_AGENT_DIR for Pi/OMP parity. |
| packages/pi-plugin/src/subagent-runner.test.ts | Adds test coverage for host agent-dir based allowlist resolution. |
| packages/pi-plugin/README.md | Updates extension docs for shared config paths and adds OMP installation/troubleshooting guidance. |
| packages/pi-plugin/package.json | Publishes explicit OMP extension manifest metadata and keywords/description updates. |
| packages/docs/src/content/docs/reference/dashboard.md | Documents dashboard behavior for Pi-compatible sessions including OMP. |
| packages/docs/src/content/docs/reference/configuration.md | Updates configuration reference header to include OMP. |
| packages/docs/src/content/docs/index.mdx | Updates docs landing page to include OMP and “three harnesses” messaging. |
| packages/docs/src/content/docs/help/troubleshooting.md | Adds OMP-specific restart guidance (/reload-plugins) and wording updates. |
| packages/docs/src/content/docs/help/faq.md | Updates FAQ for shared DB across OpenCode/Pi/OMP and migration to Pi/OMP. |
| packages/docs/src/content/docs/help/compatibility.md | Adds OMP compaction/memory conflict documentation and resolution steps. |
| packages/docs/src/content/docs/getting-started/migrating-between-harnesses.md | Extends migration guide to cover OpenCode → Pi/OMP. |
| packages/docs/src/content/docs/getting-started/introduction.md | Updates intro narrative to include OMP as a supported harness. |
| packages/docs/src/content/docs/getting-started/installation.md | Adds OMP as a first-class install target and documents OMP-specific setup actions. |
| packages/docs/src/content/docs/concepts/mural.md | Clarifies mural support across OpenCode and Pi-compatible hosts (Pi/OMP). |
| packages/docs/src/content/docs/concepts/memory.md | Updates memory-sharing explanation to include OMP. |
| packages/dashboard/src-tauri/src/pi_sessions.rs | Adds OMP profile/XDG root discovery and deduplication across multiple Pi-compatible roots. |
| packages/dashboard/src-tauri/src/commands.rs | Adds omp models --json parsing and OMP model discovery fallback for dashboard. |
| packages/cli/src/lib/v22-backfill-commands.ts | Updates schema-fence restart warning to include OMP. |
| packages/cli/src/lib/paths.ts | Adds OMP path resolution helpers (profiles, overrides, XDG data layout). |
| packages/cli/src/lib/paths-omp.test.ts | Adds tests for OMP path resolution behavior. |
| packages/cli/src/lib/omp-helpers.ts | Adds OMP binary detection, command helpers, settings access, and model/plugin parsing. |
| packages/cli/src/lib/omp-helpers.test.ts | Adds tests for OMP model parsing and command timeout/buffer behavior. |
| packages/cli/src/lib/harness-select.ts | Extends harness selection/validation to include --harness omp. |
| packages/cli/src/index.ts | Updates CLI help text to reflect OMP support and Pi/OMP migration. |
| packages/cli/src/commands/setup.ts | Dispatches setup to OMP setup flow and updates next-steps messaging. |
| packages/cli/src/commands/setup-pi.ts | Refactors setup into a Pi-compatible host abstraction and adds rollback support. |
| packages/cli/src/commands/setup-pi.test.ts | Adds rollback tests for Pi settings package registration. |
| packages/cli/src/commands/setup-omp.ts | Implements OMP setup via Pi-compatible setup host + transactional OMP conflict disabling. |
| packages/cli/src/commands/setup-omp.test.ts | Adds tests for OMP transactional rollback behavior. |
| packages/cli/src/commands/migrate.ts | Extends migration target to --to omp and routes output to OMP sessions root. |
| packages/cli/src/commands/migrate.test.ts | Adds CLI parsing test coverage for --to omp. |
| packages/cli/src/commands/doctor.ts | Dispatches doctor to OMP doctor flow. |
| packages/cli/src/commands/doctor-omp.ts | Adds OMP-specific doctor/repair/issue-report logic. |
| packages/cli/src/commands/doctor-omp.test.ts | Adds tests for healthy OMP doctor flow and config creation when OMP is missing. |
| packages/cli/src/adapters/types.ts | Extends adapter types to include `HarnessKind = ... |
| packages/cli/src/adapters/omp.ts | Adds an OMP adapter covering detection, plugin enablement, cache/log paths, and install hint. |
| packages/cli/src/adapters/omp.test.ts | Adds adapter tests for detecting an enabled plugin via omp plugin list --json. |
| packages/cli/src/adapters/index.ts | Registers OMP adapter in the global adapter list and exports it. |
| packages/cli/package.json | Updates CLI package metadata/keywords to include OMP. |
| CONFIGURATION.md | Updates shared-config + doctor documentation to include OMP behavior. |
| .gitignore | Ignores agent log directories (AgentLogs/, .agent-logs/). |
Suppressed comments (2)
packages/dashboard/src-tauri/src/pi_sessions.rs:154
PI_CONFIG_DIRisn’t trimmed before converting toPathBuf. If the env var contains leading/trailing whitespace it will resolve to a different directory than the CLI uses (which trims), causing session-root discovery to diverge across components.
let config_dir = std::env::var_os("PI_CONFIG_DIR")
.filter(|v| !v.is_empty())
.map(PathBuf::from)
.unwrap_or_else(|| PathBuf::from(".omp"));
packages/dashboard/src-tauri/src/pi_sessions.rs:169
XDG_DATA_HOMEis also consumed without trimming. Trimming avoids treating whitespace-only values as valid and keeps behavior aligned with the CLI path resolver (which trims).
if let Some(xdg_data) = std::env::var_os("XDG_DATA_HOME").filter(|v| !v.is_empty()) {
let app_root = PathBuf::from(xdg_data).join("omp");
append_omp_profile_roots(&mut roots, &app_root.join("profiles"), true);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const reportedAgentDir = options.deps.runOmpCommand(omp.path, ["config", "path"], 10_000); | ||
| if (!reportedAgentDir.ok) | ||
| add(results, "warn", "Could not verify OMP active agent directory"); | ||
| else if (reportedAgentDir.stdout === getOmpAgentDir()) { | ||
| add(results, "pass", `OMP agent directory resolved to ${getOmpAgentDir()}`); | ||
| } else { | ||
| add( | ||
| results, | ||
| "fail", | ||
| `OMP reports agent directory ${reportedAgentDir.stdout}, but Magic Context resolved ${getOmpAgentDir()}`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Fixed in 9730e23. Doctor now resolves both the reported and expected agent directories before comparison, and the healthy-install regression test returns an equivalent path with a trailing /./ segment.
| if let Some(agent_dir) = std::env::var_os("PI_CODING_AGENT_DIR").filter(|v| !v.is_empty()) { | ||
| roots.push(PathBuf::from(agent_dir).join("sessions")); | ||
| } |
There was a problem hiding this comment.
Fixed in c683ddb. PI_CODING_AGENT_DIR, PI_CONFIG_DIR, and XDG_DATA_HOME now share one trimming helper; blank values are ignored. Added direct regression coverage, and the production session module passes 11 isolated Rust tests.
Co-authored-by: Wine Fox <fox@ling.plus>
Co-authored-by: Wine Fox <fox@ling.plus>
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/cli/src/commands/doctor-omp.ts">
<violation number="1" location="packages/cli/src/commands/doctor-omp.ts:221">
P2: A successful `omp config path` with empty output can make this check pass incorrectly because `resolve("")` becomes the current working directory; preserve an empty result as invalid before normalizing it so the doctor cannot report a healthy agent-directory configuration without a path.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| if (!reportedAgentDir.ok) { | ||
| add(results, "warn", "Could not verify OMP active agent directory"); | ||
| } else { | ||
| const reportedPath = resolve(reportedAgentDir.stdout); |
There was a problem hiding this comment.
P2: A successful omp config path with empty output can make this check pass incorrectly because resolve("") becomes the current working directory; preserve an empty result as invalid before normalizing it so the doctor cannot report a healthy agent-directory configuration without a path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/commands/doctor-omp.ts, line 221:
<comment>A successful `omp config path` with empty output can make this check pass incorrectly because `resolve("")` becomes the current working directory; preserve an empty result as invalid before normalizing it so the doctor cannot report a healthy agent-directory configuration without a path.</comment>
<file context>
@@ -215,16 +215,20 @@ async function runHealthChecks(options: {
- "fail",
- `OMP reports agent directory ${reportedAgentDir.stdout}, but Magic Context resolved ${getOmpAgentDir()}`,
- );
+ const reportedPath = resolve(reportedAgentDir.stdout);
+ const expectedPath = resolve(getOmpAgentDir());
+ if (reportedPath === expectedPath) {
</file context>
| const reportedPath = resolve(reportedAgentDir.stdout); | |
| const reportedPath = reportedAgentDir.stdout ? resolve(reportedAgentDir.stdout) : ""; |
|
Thanks for the rebase — we did the full review. The direction is right and several pieces are exactly how we'd want them (probing the distinct
The remaining three: complete the OMP path/config contract ( Happy to re-review as soon as these land — the containment shape you already have (own adapter, own commands) makes them all tractable. |
There was a problem hiding this comment.
2 issues found across 43 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/cli/src/commands/doctor-omp.ts">
<violation number="1" location="packages/cli/src/commands/doctor-omp.ts:92">
P3: The new OMP doctor reimplements the same semver triple-parse/compare that `comparePiVersion` in `commands/setup-pi.ts` already provides for the same Pi-compatible flow. The two functions parse `X.Y.Z` with the identical regex and compare component-by-component, so a future change to how Pi/OMP versions are treated must be made in two places and can drift. Consider extracting a shared semver helper (e.g. in `lib`) and having both the Pi setup and the OMP doctor use it, matching the existing repo convention of shared CLI helpers.</violation>
<violation number="2" location="packages/cli/src/commands/doctor-omp.ts:221">
P2: The OMP path check compares `omp config path` with the agent directory instead of the OMP config path, which can make every healthy installation fail its doctor check. Compare against `getOmpConfigPath()` (and label the result as a config path) or invoke the command that actually reports the agent directory.</violation>
</file>
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
| if (!reportedAgentDir.ok) { | ||
| add(results, "warn", "Could not verify OMP active agent directory"); | ||
| } else { | ||
| const reportedPath = resolve(reportedAgentDir.stdout); |
There was a problem hiding this comment.
P2: The OMP path check compares omp config path with the agent directory instead of the OMP config path, which can make every healthy installation fail its doctor check. Compare against getOmpConfigPath() (and label the result as a config path) or invoke the command that actually reports the agent directory.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/commands/doctor-omp.ts, line 221:
<comment>The OMP path check compares `omp config path` with the agent directory instead of the OMP config path, which can make every healthy installation fail its doctor check. Compare against `getOmpConfigPath()` (and label the result as a config path) or invoke the command that actually reports the agent directory.</comment>
<file context>
@@ -0,0 +1,425 @@
+ if (!reportedAgentDir.ok) {
+ add(results, "warn", "Could not verify OMP active agent directory");
+ } else {
+ const reportedPath = resolve(reportedAgentDir.stdout);
+ const expectedPath = resolve(getOmpAgentDir());
+ if (reportedPath === expectedPath) {
</file context>
| return match ? [Number(match[1]), Number(match[2]), Number(match[3])] : null; | ||
| } | ||
|
|
||
| function isOlderThan(value: string | null, minimum: string): boolean { |
There was a problem hiding this comment.
P3: The new OMP doctor reimplements the same semver triple-parse/compare that comparePiVersion in commands/setup-pi.ts already provides for the same Pi-compatible flow. The two functions parse X.Y.Z with the identical regex and compare component-by-component, so a future change to how Pi/OMP versions are treated must be made in two places and can drift. Consider extracting a shared semver helper (e.g. in lib) and having both the Pi setup and the OMP doctor use it, matching the existing repo convention of shared CLI helpers.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/commands/doctor-omp.ts, line 92:
<comment>The new OMP doctor reimplements the same semver triple-parse/compare that `comparePiVersion` in `commands/setup-pi.ts` already provides for the same Pi-compatible flow. The two functions parse `X.Y.Z` with the identical regex and compare component-by-component, so a future change to how Pi/OMP versions are treated must be made in two places and can drift. Consider extracting a shared semver helper (e.g. in `lib`) and having both the Pi setup and the OMP doctor use it, matching the existing repo convention of shared CLI helpers.</comment>
<file context>
@@ -0,0 +1,425 @@
+ return match ? [Number(match[1]), Number(match[2]), Number(match[3])] : null;
+}
+
+function isOlderThan(value: string | null, minimum: string): boolean {
+ const left = parseSemver(value);
+ const right = parseSemver(minimum);
</file context>
|
Hey @Lynricsy, please address cubic findings also. Thanks! |
Co-authored-by: Wine Fox <fox@ling.plus>
Co-authored-by: Wine Fox <fox@ling.plus>
Co-authored-by: Wine Fox <fox@ling.plus>
|
@waltino Thanks for the detailed containment review. I addressed all five merge blockers and pushed three atomic commits:
Verification completed:
Two unrelated current-master gates remain visible but were not changed/suppressed here: Dashboard ConfigEditor schema parity has four pre-existing unclassified fields (36 pass / 2 fail), and root/Pi lint reports pre-existing unused-symbol diagnostics outside these touched paths. Rust 1.77 source checking is additionally blocked before project compilation by the current lockfile's The branch is pushed at |
Co-authored-by: Wine Fox <fox@ling.plus>
|
Follow-up documentation consistency fix pushed as |
There was a problem hiding this comment.
1 issue found across 20 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="tests/docker/Dockerfile.omp">
<violation number="1" location="tests/docker/Dockerfile.omp:69">
P3: The new OMP E2E image isn't wired into the local runner: `tests/docker/run-e2e.sh` only accepts `opencode|pi|all`, so `run-e2e.sh omp` fails with "Unknown target" and `all` never runs it. Add an `omp` arm to `run_target` so developers can reproduce the CI e2e-omp job locally, matching the existing pi/opencode targets.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| RUN chmod +x /test/test-omp-e2e.sh | ||
|
|
||
| WORKDIR /test/project | ||
| ENTRYPOINT ["/test/test-omp-e2e.sh"] |
There was a problem hiding this comment.
P3: The new OMP E2E image isn't wired into the local runner: tests/docker/run-e2e.sh only accepts opencode|pi|all, so run-e2e.sh omp fails with "Unknown target" and all never runs it. Add an omp arm to run_target so developers can reproduce the CI e2e-omp job locally, matching the existing pi/opencode targets.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/docker/Dockerfile.omp, line 69:
<comment>The new OMP E2E image isn't wired into the local runner: `tests/docker/run-e2e.sh` only accepts `opencode|pi|all`, so `run-e2e.sh omp` fails with "Unknown target" and `all` never runs it. Add an `omp` arm to `run_target` so developers can reproduce the CI e2e-omp job locally, matching the existing pi/opencode targets.</comment>
<file context>
@@ -0,0 +1,69 @@
+RUN chmod +x /test/test-omp-e2e.sh
+
+WORKDIR /test/project
+ENTRYPOINT ["/test/test-omp-e2e.sh"]
</file context>
Co-authored-by: Wine Fox <fox@ling.plus>
Co-authored-by: Wine Fox <fox@ling.plus>
Co-authored-by: Wine Fox <fox@ling.plus>
|
All nine findings from the latest cubic review were valid and are fixed in P1 — package-only OMP discovery produced an unusable binary P1 — active-profile sessions vanished under the XDG guard P2 — non-array P2 — prototype-member provider ids were rewritten P2 — non-executable file counted as an OMP install P2 — P3 — P3 — version floor not asserted P3 — whitespace-sensitive Verification after these changes:
|
There was a problem hiding this comment.
1 issue found across 9 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/dashboard/src-tauri/src/pi_sessions.rs">
<violation number="1" location="packages/dashboard/src-tauri/src/pi_sessions.rs:256">
P2: Named-profile sessions disappear from the dashboard whenever `PI_CODING_AGENT_DIR` is stale or custom, even though OMP's profile resolver correctly ignores that override and uses the initialized profile-specific XDG root. The XDG guard should treat an active named profile as authoritative, rather than rejecting XDG solely because `PI_CODING_AGENT_DIR` differs.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| } | ||
|
|
||
| let configured_agent = trimmed_env_path(std::env::var_os("PI_CODING_AGENT_DIR")); | ||
| let can_use_xdg = omp_xdg_allowed( |
There was a problem hiding this comment.
P2: Named-profile sessions disappear from the dashboard whenever PI_CODING_AGENT_DIR is stale or custom, even though OMP's profile resolver correctly ignores that override and uses the initialized profile-specific XDG root. The XDG guard should treat an active named profile as authoritative, rather than rejecting XDG solely because PI_CODING_AGENT_DIR differs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/dashboard/src-tauri/src/pi_sessions.rs, line 256:
<comment>Named-profile sessions disappear from the dashboard whenever `PI_CODING_AGENT_DIR` is stale or custom, even though OMP's profile resolver correctly ignores that override and uses the initialized profile-specific XDG root. The XDG guard should treat an active named profile as authoritative, rather than rejecting XDG solely because `PI_CODING_AGENT_DIR` differs.</comment>
<file context>
@@ -212,34 +234,37 @@ fn pi_compatible_session_roots_for_home(home: &Path, include_omp: bool) -> Vec<P
- && configured_agent
- .as_ref()
- .map_or(true, |agent| agent == &default_agent);
+ let can_use_xdg = omp_xdg_allowed(
+ configured_agent.as_deref(),
+ &expected_agent,
</file context>
Summary
Compatibility decisions
harness='pi'discriminator for session-scoped rowsomp models --jsonselectors so scoped and nested IDs are preservedVerification
omp plugin, launched OMP, and/ctx-statusreported Magic Context v0.33.0 with historian idleEnvironment note
The full Tauri Cargo suite could not link on this workstation because
pkg-config/DBus development metadata is absent; the failure occurs inlibdbus-sysbefore project code compiles.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Adds first-class OMP support across the CLI, Pi-compatible extension, and dashboard, including setup, doctor, and OpenCode→OMP migration. Models and sessions show up in the dashboard via the shared runtime, config, and database; discovery is robust with normalized paths, executable detection, and provider mapping.
New Features
ompharness withsetup --harness ompanddoctor --harness omp; supports--harness opencode|pi|omp.--forcerepairs.@cortexkit/pi-magic-contextdeclares an OMP manifest, runs on both Pi and OMP, marks Pi peers optional, resolves relative allowlist entries fromPI_CODING_AGENT_DIR.omp models --json.doctor migrate --from opencode --to ompwrites Pi/OMP JSONL to the active OMP sessions root.Bug Fixes
Written for commit 1f6d417. Summary will update on new commits.
Greptile Summary
The PR adds first-class Oh My Pi support across installation, diagnostics, migration, runtime integration, and dashboard discovery.
Confidence Score: 5/5
The PR appears safe to merge.
The previously reported multi-root duplication issue is addressed by canonical root deduplication and logical-session deduplication, with the selected JSONL path propagated to session detail lookup; no blocking failure remains.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart LR CLI[Magic Context CLI] -->|setup / doctor / migrate| OMP[Oh My Pi] OMP --> EXT[Pi-compatible extension] EXT --> DB[(Shared CortexKit database)] OMP --> JSONL[OMP session JSONL roots] JSONL --> DASH[Dashboard discovery] DB --> DASHReviews (7): Last reviewed commit: "test(omp): 🧪 tighten smoke and local ru..." | Re-trigger Greptile