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.
9 issues 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="packages/cli/src/lib/omp-helpers.ts">
<violation number="1" location="packages/cli/src/lib/omp-helpers.ts:37">
P1: On Windows, package-only OMP discovery produces a detected binary that every `omp` command fails to execute. The returned `.js` path bypasses the existing Windows shim handling; resolving the generated `.cmd` shim or invoking the package through its Bun runtime would keep this detection usable.</violation>
</file>
<file name="packages/cli/src/commands/setup-pi.ts">
<violation number="1" location="packages/cli/src/commands/setup-pi.ts:104">
P2: A parseable Pi settings file with a non-array `packages` value is not restored when setup rolls back: `packagesFieldExisted` records only presence, so rollback preserves the field after setup has replaced its value with `[]`. Capturing/restoring the original value, or rejecting non-array `packages` before registration, would keep transactional setup from discarding user configuration.</violation>
</file>
<file name="packages/plugin/src/shared/harness-provider-map.ts">
<violation number="1" location="packages/plugin/src/shared/harness-provider-map.ts:79">
P2: OMP model references such as `constructor/model` or `toString/model` are rewritten into invalid provider strings rather than passed through unchanged. Use an own-property check when reading the mapping so arbitrary OMP providers remain identities.</violation>
</file>
<file name="packages/dashboard/src-tauri/src/pi_sessions.rs">
<violation number="1" location="packages/dashboard/src-tauri/src/pi_sessions.rs:179">
P2: Plain Pi can show unrelated OMP/profile sessions whenever `OMP_PROFILE` is present without an OMP executable or package. Host detection should require the same positive package/binary evidence used by the Pi runtime instead of treating the profile variable alone as proof of OMP.</violation>
<violation number="2" location="packages/dashboard/src-tauri/src/pi_sessions.rs:193">
P2: A non-executable file named `omp` can falsely classify OMP as installed and make plain Pi users see unrelated OMP roots. The PATH and fallback checks should use an executable-file test, including POSIX execute permission, rather than `is_file()` alone.</violation>
<violation number="3" location="packages/dashboard/src-tauri/src/pi_sessions.rs:235">
P1: Sessions for an active OMP profile disappear from the dashboard when OMP sets `PI_CODING_AGENT_DIR` to the profile agent directory, because this comparison rejects the profile-specific path and skips all XDG roots. The guard should compare against the effective profile agent directory or allow an active valid profile to use its initialized XDG root.</violation>
</file>
<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>
<file name="tests/docker/test-omp-e2e.sh">
<violation number="1" location="tests/docker/test-omp-e2e.sh:31">
P3: The section titled "Real OMP installation..." claims to verify a minimum version, but the only OMP version check is `test -n "$OMP_VERSION"` (non-empty). Since doctor-omp/setup-omp enforce a 17.1.7 floor, assert it here too (compare against 17.1.7) so an unexpectedly old floating OMP doesn't silently pass.</violation>
<violation number="2" location="tests/docker/test-omp-e2e.sh:103">
P3: The `agent_end` check is a literal whitespace-sensitive grep on JSON output. If OMP emits `"type": "agent_end"` (space) or a different terminal event name, the test spuriously FAILs despite a successful turn; make it whitespace-tolerant (`grep -qE '"type"\s*:\s*"agent_end"'`) or rely on the exit-code/output checks already present.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| }; | ||
| if (manifest.name !== "@oh-my-pi/pi-coding-agent") return null; | ||
| const cli = join(packageDir, "dist", "cli.js"); | ||
| return existsSync(cli) ? cli : null; |
There was a problem hiding this comment.
P1: On Windows, package-only OMP discovery produces a detected binary that every omp command fails to execute. The returned .js path bypasses the existing Windows shim handling; resolving the generated .cmd shim or invoking the package through its Bun runtime would keep this detection usable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/lib/omp-helpers.ts, line 37:
<comment>On Windows, package-only OMP discovery produces a detected binary that every `omp` command fails to execute. The returned `.js` path bypasses the existing Windows shim handling; resolving the generated `.cmd` shim or invoking the package through its Bun runtime would keep this detection usable.</comment>
<file context>
@@ -24,15 +25,46 @@ export interface OmpPluginInfo {
+ };
+ if (manifest.name !== "@oh-my-pi/pi-coding-agent") return null;
+ const cli = join(packageDir, "dist", "cli.js");
+ return existsSync(cli) ? cli : null;
+ } catch {
+ return null;
</file context>
| let can_use_xdg = (cfg!(target_os = "linux") || cfg!(target_os = "macos")) | ||
| && configured_agent | ||
| .as_ref() | ||
| .map_or(true, |agent| agent == &default_agent); |
There was a problem hiding this comment.
P1: Sessions for an active OMP profile disappear from the dashboard when OMP sets PI_CODING_AGENT_DIR to the profile agent directory, because this comparison rejects the profile-specific path and skips all XDG roots. The guard should compare against the effective profile agent directory or allow an active valid profile to use its initialized XDG root.
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 235:
<comment>Sessions for an active OMP profile disappear from the dashboard when OMP sets `PI_CODING_AGENT_DIR` to the profile agent directory, because this comparison rejects the profile-specific path and skips all XDG roots. The guard should compare against the effective profile agent directory or allow an active valid profile to use its initialized XDG root.</comment>
<file context>
@@ -137,45 +141,109 @@ fn append_omp_profile_roots(roots: &mut Vec<PathBuf>, profiles_dir: &Path, xdg:
+ let can_use_xdg = (cfg!(target_os = "linux") || cfg!(target_os = "macos"))
+ && configured_agent
+ .as_ref()
+ .map_or(true, |agent| agent == &default_agent);
+ if can_use_xdg {
+ if let Some(xdg_data) = trimmed_env_path(std::env::var_os("XDG_DATA_HOME")) {
</file context>
| .map_or(true, |agent| agent == &default_agent); | |
| .map_or(true, |agent| active_omp_profile().is_some() || agent == &default_agent); |
| `Run \`pi update --self\` (or \`npm install -g @earendil-works/pi-coding-agent@latest\`) before continuing.`, | ||
| ensurePluginEntry: async (settingsPath) => { | ||
| const settings = readJsoncConfigForUpdate(settingsPath); | ||
| const packagesFieldExisted = Object.hasOwn(settings, "packages"); |
There was a problem hiding this comment.
P2: A parseable Pi settings file with a non-array packages value is not restored when setup rolls back: packagesFieldExisted records only presence, so rollback preserves the field after setup has replaced its value with []. Capturing/restoring the original value, or rejecting non-array packages before registration, would keep transactional setup from discarding user configuration.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/commands/setup-pi.ts, line 104:
<comment>A parseable Pi settings file with a non-array `packages` value is not restored when setup rolls back: `packagesFieldExisted` records only presence, so rollback preserves the field after setup has replaced its value with `[]`. Capturing/restoring the original value, or rejecting non-array `packages` before registration, would keep transactional setup from discarding user configuration.</comment>
<file context>
@@ -97,6 +100,8 @@ const DEFAULT_HOST: PiCompatibleSetupHost = {
`Run \`pi update --self\` (or \`npm install -g @earendil-works/pi-coding-agent@latest\`) before continuing.`,
ensurePluginEntry: async (settingsPath) => {
+ const settings = readJsoncConfigForUpdate(settingsPath);
+ const packagesFieldExisted = Object.hasOwn(settings, "packages");
const added = writePiSettingsPackage(settingsPath);
return {
</file context>
|
|
||
| /** OMP-native selector -> canonical shared-config model reference. */ | ||
| export function ompModelRefToCanonical(ref: string): string { | ||
| return remapProviderPrefix(ref, OMP_TO_CANONICAL_PROVIDER); |
There was a problem hiding this comment.
P2: OMP model references such as constructor/model or toString/model are rewritten into invalid provider strings rather than passed through unchanged. Use an own-property check when reading the mapping so arbitrary OMP providers remain identities.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/plugin/src/shared/harness-provider-map.ts, line 79:
<comment>OMP model references such as `constructor/model` or `toString/model` are rewritten into invalid provider strings rather than passed through unchanged. Use an own-property check when reading the mapping so arbitrary OMP providers remain identities.</comment>
<file context>
@@ -64,3 +73,13 @@ export function piModelRefToCanonical(ref: string): string {
+
+/** OMP-native selector -> canonical shared-config model reference. */
+export function ompModelRefToCanonical(ref: string): string {
+ return remapProviderPrefix(ref, OMP_TO_CANONICAL_PROVIDER);
+}
+
</file context>
| &["omp"] | ||
| }; | ||
| if std::env::split_paths(&std::env::var_os("PATH").unwrap_or_default()) | ||
| .any(|dir| binary_names.iter().any(|name| dir.join(name).is_file())) |
There was a problem hiding this comment.
P2: A non-executable file named omp can falsely classify OMP as installed and make plain Pi users see unrelated OMP roots. The PATH and fallback checks should use an executable-file test, including POSIX execute permission, rather than is_file() alone.
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 193:
<comment>A non-executable file named `omp` can falsely classify OMP as installed and make plain Pi users see unrelated OMP roots. The PATH and fallback checks should use an executable-file test, including POSIX execute permission, rather than `is_file()` alone.</comment>
<file context>
@@ -137,45 +141,109 @@ fn append_omp_profile_roots(roots: &mut Vec<PathBuf>, profiles_dir: &Path, xdg:
+ &["omp"]
};
+ if std::env::split_paths(&std::env::var_os("PATH").unwrap_or_default())
+ .any(|dir| binary_names.iter().any(|name| dir.join(name).is_file()))
+ {
+ return true;
</file context>
| } | ||
|
|
||
| fn omp_installation_detected(home: &Path) -> bool { | ||
| if std::env::var_os("OMP_PROFILE").is_some() { |
There was a problem hiding this comment.
P2: Plain Pi can show unrelated OMP/profile sessions whenever OMP_PROFILE is present without an OMP executable or package. Host detection should require the same positive package/binary evidence used by the Pi runtime instead of treating the profile variable alone as proof of OMP.
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 179:
<comment>Plain Pi can show unrelated OMP/profile sessions whenever `OMP_PROFILE` is present without an OMP executable or package. Host detection should require the same positive package/binary evidence used by the Pi runtime instead of treating the profile variable alone as proof of OMP.</comment>
<file context>
@@ -137,45 +141,109 @@ fn append_omp_profile_roots(roots: &mut Vec<PathBuf>, profiles_dir: &Path, xdg:
- let Some(home) = dirs::home_dir() else {
- return Vec::new();
+fn omp_installation_detected(home: &Path) -> bool {
+ if std::env::var_os("OMP_PROFILE").is_some() {
+ return true;
+ }
</file context>
| 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>
| PLUGIN_LIST=$(omp plugin list --json 2>&1) | ||
| echo "OMP version: ${OMP_VERSION:-unknown}" | ||
| echo "$PLUGIN_LIST" | ||
| check "omp --version returns a value" "test -n \"$OMP_VERSION\"" |
There was a problem hiding this comment.
P3: The section titled "Real OMP installation..." claims to verify a minimum version, but the only OMP version check is test -n "$OMP_VERSION" (non-empty). Since doctor-omp/setup-omp enforce a 17.1.7 floor, assert it here too (compare against 17.1.7) so an unexpectedly old floating OMP doesn't silently pass.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/docker/test-omp-e2e.sh, line 31:
<comment>The section titled "Real OMP installation..." claims to verify a minimum version, but the only OMP version check is `test -n "$OMP_VERSION"` (non-empty). Since doctor-omp/setup-omp enforce a 17.1.7 floor, assert it here too (compare against 17.1.7) so an unexpectedly old floating OMP doesn't silently pass.</comment>
<file context>
@@ -0,0 +1,119 @@
+PLUGIN_LIST=$(omp plugin list --json 2>&1)
+echo "OMP version: ${OMP_VERSION:-unknown}"
+echo "$PLUGIN_LIST"
+check "omp --version returns a value" "test -n \"$OMP_VERSION\""
+check "OMP lists the linked Magic Context package" \
+ "echo \"\$PLUGIN_LIST\" | grep -q '@cortexkit/pi-magic-context'"
</file context>
| tail -20 /tmp/omp.log | ||
|
|
||
| check "OMP turn exits successfully" "test \"$OMP_EXIT\" -eq 0" | ||
| check "OMP emits a terminal agent_end protocol event" \ |
There was a problem hiding this comment.
P3: The agent_end check is a literal whitespace-sensitive grep on JSON output. If OMP emits "type": "agent_end" (space) or a different terminal event name, the test spuriously FAILs despite a successful turn; make it whitespace-tolerant (grep -qE '"type"\s*:\s*"agent_end"') or rely on the exit-code/output checks already present.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/docker/test-omp-e2e.sh, line 103:
<comment>The `agent_end` check is a literal whitespace-sensitive grep on JSON output. If OMP emits `"type": "agent_end"` (space) or a different terminal event name, the test spuriously FAILs despite a successful turn; make it whitespace-tolerant (`grep -qE '"type"\s*:\s*"agent_end"'`) or rely on the exit-code/output checks already present.</comment>
<file context>
@@ -0,0 +1,119 @@
+tail -20 /tmp/omp.log
+
+check "OMP turn exits successfully" "test \"$OMP_EXIT\" -eq 0"
+check "OMP emits a terminal agent_end protocol event" \
+ "grep -q '\"type\":\"agent_end\"' /tmp/omp.log"
+
</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, with install, doctor, and OpenCode→OMP migration. Models and sessions appear in the dashboard using a shared runtime, config, and database; discovery is reliable via normalized doctor paths and trimmed env paths.
New Features
ompharness withsetup --harness ompanddoctor --harness omp; supports--harness opencode|pi|omp.--forcerepair.omp models --jsonand preserves scoped/nested selectors.@cortexkit/pi-magic-contextdeclares an OMP manifest, runs on both Pi and OMP, marks Pi peers optional, and resolves relative allowlist entries fromPI_CODING_AGENT_DIR.harness='pi'for session rows.doctor migrate --from opencode --to ompwrites Pi/OMP JSONL to the active OMP sessions root.Bug Fixes
Written for commit d231001. Summary will update on new commits.
Greptile Summary
Adds first-class Oh My Pi support across the CLI, Pi-compatible extension, dashboard, documentation, and CI.
Confidence Score: 5/5
The PR appears safe to merge.
No blocking failure remains; the previously reported multi-root duplication and detail-selection mismatch are addressed by canonical root deduplication and consistent session-ID deduplication across list and detail paths.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart LR CLI[Magic Context CLI] --> Detect[Detect OMP and resolve profile/XDG paths] Detect --> Setup[Install and configure Pi-compatible extension] Setup --> OMP[Oh My Pi runtime] OMP --> SharedDB[(Shared Magic Context database)] OMP --> Sessions[OMP JSONL sessions] Sessions --> Dashboard[Dashboard session discovery] SharedDB --> Dashboard OpenCode[OpenCode sessions] --> Migration[Session migration] Migration --> SessionsReviews (6): Last reviewed commit: "docs(pi): 📝 clarify host-scoped extensi..." | Re-trigger Greptile