feat: add full Oh My Pi (OMP) support - #256
Conversation
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:
GitHub currently shows only Greptile and Socket checks pending; no check is failing. |
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.
2 issues 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/dashboard/src-tauri/src/pi_sessions.rs">
<violation number="1" location="packages/dashboard/src-tauri/src/pi_sessions.rs:85">
P2: Dashboard session discovery fails for OMP installations whose configured environment path contains non-UTF-8 bytes, because `trimmed_env_path` lossy-converts the `OsString` before building the root. Preserve the native `OsString`/`PathBuf` for non-UTF-8 values and apply trimming only when a valid string representation is available.</violation>
</file>
<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
| let trimmed = value?.to_string_lossy().trim().to_string(); | ||
| (!trimmed.is_empty()).then(|| PathBuf::from(trimmed)) |
There was a problem hiding this comment.
P2: Dashboard session discovery fails for OMP installations whose configured environment path contains non-UTF-8 bytes, because trimmed_env_path lossy-converts the OsString before building the root. Preserve the native OsString/PathBuf for non-UTF-8 values and apply trimming only when a valid string representation is available.
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 85:
<comment>Dashboard session discovery fails for OMP installations whose configured environment path contains non-UTF-8 bytes, because `trimmed_env_path` lossy-converts the `OsString` before building the root. Preserve the native `OsString`/`PathBuf` for non-UTF-8 values and apply trimming only when a valid string representation is available.</comment>
<file context>
@@ -81,6 +81,11 @@ fn test_root() -> &'static RwLock<Option<PathBuf>> {
}
+fn trimmed_env_path(value: Option<std::ffi::OsString>) -> Option<PathBuf> {
+ let trimmed = value?.to_string_lossy().trim().to_string();
+ (!trimmed.is_empty()).then(|| PathBuf::from(trimmed))
+}
</file context>
| let trimmed = value?.to_string_lossy().trim().to_string(); | |
| (!trimmed.is_empty()).then(|| PathBuf::from(trimmed)) | |
| let value = value?; | |
| if let Some(text) = value.to_str() { | |
| let trimmed = text.trim(); | |
| return (!trimmed.is_empty()).then(|| PathBuf::from(trimmed)); | |
| } | |
| (!value.is_empty()).then(|| PathBuf::from(value)) |
| 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) : ""; |
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 Oh My Pi (OMP) support across the CLI, Pi-compatible extension, and dashboard. You can install, health‑check, and migrate OpenCode sessions to OMP; models and sessions show in the dashboard with a shared runtime, config, and database. Also normalizes OMP doctor paths and trims OMP env paths for reliable discovery.
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 and runs on both Pi and OMP; marks Pi peer deps as optional; resolves relative allowlist entries fromPI_CODING_AGENT_DIR.harness='pi'for session rows; project memories and config are shared across OpenCode, Pi, and OMP.doctor migrate --from opencode --to ompconverts an OpenCode session to Pi/OMP JSONL and writes it to the active OMP sessions root.Bug Fixes
Written for commit c683ddb. Summary will update on new commits.
Greptile Summary
Adds first-class OMP integration across the CLI, shared Pi-compatible runtime, dashboard session/model discovery, and documentation.
Confidence Score: 5/5
The PR appears safe to merge.
No blocking failure remains.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart LR CLI[Magic Context CLI] --> Detect[Detect OpenCode, Pi, or OMP] Detect --> Setup[Setup and Doctor] Setup --> OMP[OMP plugin manager and configuration] OMP --> Runtime[Pi-compatible Magic Context runtime] Runtime --> DB[(Shared CortexKit database)] OMP --> Sessions[OMP profile and XDG session roots] Sessions --> Dedup[Deduplicate logical sessions] Dedup --> Dashboard[Dashboard] OMP --> Models[omp models --json] Models --> DashboardReviews (5): Last reviewed commit: "fix(dashboard): 🧹 trim OMP environment ..." | Re-trigger Greptile