feat: add full Oh My Pi (OMP) support - #256
Conversation
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>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
11 issues found and verified against the latest diff
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/docs/src/content/docs/index.mdx">
<violation number="1" location="packages/docs/src/content/docs/index.mdx:3">
P3: The description now includes "Oh My Pi" in the supported-agents list, but the "Works with" section below still lists only OpenCode and Pi, and the sentence says "shared across both harnesses." Add OMP to the list and update "both" → "all" so the page is internally consistent.</violation>
</file>
<file name="packages/cli/src/commands/setup-pi.ts">
<violation number="1" location="packages/cli/src/commands/setup-pi.ts:60">
P2: DEFAULT_HOST doesn't implement rollbackPluginEntry, so for Pi (unlike OMP) a failure in writeMagicContextConfig after the plugin entry was successfully added leaves the settings file referencing the package with no valid Magic Context config written. Consider adding a rollbackPluginEntry for the Pi host (e.g. remove the added package entry) to match the new rollback contract.</violation>
<violation number="2" location="packages/cli/src/commands/setup-pi.ts:285">
P3: The generalized 'not found' message no longer mentions the home-directory fallback paths that detectPiBinary/detectOmpBinary actually check, which is a minor UX regression compared to the previous Pi-specific message.</violation>
</file>
<file name="packages/cli/src/commands/doctor-omp.ts">
<violation number="1" location="packages/cli/src/commands/doctor-omp.ts:120">
P3: The catch-all message "gh CLI unavailable" is shown even when `gh` is installed but the user isn't authenticated (since `gh auth status` exits non-zero in that case too); consider distinguishing "not installed" from "not authenticated" for a clearer message.</violation>
<violation number="2" location="packages/cli/src/commands/doctor-omp.ts:300">
P2: `repair()` bails out entirely with `if (!omp) return 0` before checking `plan.writeUserConfig`, so `magic-context omp doctor --force` will never create the default Magic Context config when the OMP binary isn't detected — even though that repair action has nothing to do with OMP being installed. Consider moving the `writeUserConfig` handling above the `!omp` early return (or handling it separately) so it always runs when needed.</violation>
</file>
<file name="packages/dashboard/src-tauri/src/commands.rs">
<violation number="1" location="packages/dashboard/src-tauri/src/commands.rs:1191">
P2: Inserting the OMP fallback before the pi login-shell fallback means a real Pi installation that's only resolvable via the login shell (e.g. an exotic PATH setup not covered by the static candidate list) will never be tried if OMP is also installed with a non-empty model list — get_available_pi_models will return OMP models under the 'pi' models cache instead. Consider moving the OMP fallback after the pi login-shell attempt (or making the two probes run independently) so a working Pi install is never masked by an unrelated OMP install.</violation>
</file>
<file name="packages/cli/src/commands/setup-omp.test.ts">
<violation number="1" location="packages/cli/src/commands/setup-omp.test.ts:67">
P2: Mock binary shebang `#!/usr/bin/node` breaks tests on machines where node lives outside /usr/bin (common for nvm, fnm, asdf, Homebrew). Use `#!/usr/bin/env node` instead so the binary resolves through PATH.</violation>
</file>
<file name="packages/dashboard/src-tauri/src/pi_sessions.rs">
<violation number="1" location="packages/dashboard/src-tauri/src/pi_sessions.rs:96">
P2: normalize_omp_profile() doesn't validate the profile name format like the CLI's resolveOmpPaths() does (regex-restricted to lowercase alnum/._- , max 64 chars). Without that guard, a crafted OMP_PROFILE/PI_PROFILE value containing '..' or path separators can make the profile-derived path escape the intended `.omp`/XDG root when later joined and scanned.</violation>
<violation number="2" location="packages/dashboard/src-tauri/src/pi_sessions.rs:184">
P3: scan_pi_session_dir()/scan_pi_cache_session_dir() no longer call pi_sessions_root(), and no other file references it — it's now unused dead code left over from the refactor and can be removed.</violation>
</file>
<file name="packages/cli/src/lib/omp-helpers.ts">
<violation number="1" location="packages/cli/src/lib/omp-helpers.ts:43">
P3: `runOmpCommand`'s spawnSync call has no explicit `maxBuffer`, so a large model catalog or plugin list JSON payload could exceed Node's default 1MB buffer and fail the call (surfaced as a generic failure) instead of being parsed; consider setting a higher `maxBuffer` for the JSON-producing subcommands.</violation>
<violation number="2" location="packages/cli/src/lib/omp-helpers.ts:51">
P2: When the OMP command times out (or otherwise fails after spawning with no stderr output), `result.stderr` is an empty string rather than null/undefined, so the `??` fallback to `result.error?.message` never triggers and the real error (e.g. `ETIMEDOUT`) is silently dropped. Doctor/setup flows that display `result.stderr` to the user (adapters/omp.ts, doctor-omp.ts, setup-omp.ts) will show a blank or generic failure message instead of the actionable cause.</violation>
</file>
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
| dryRun: boolean; | ||
| configureHost: boolean; | ||
| }) => Promise<SetupRollback | false>; | ||
| rollbackPluginEntry?: (registration: PluginEntryResult) => Promise<void>; |
There was a problem hiding this comment.
P2: DEFAULT_HOST doesn't implement rollbackPluginEntry, so for Pi (unlike OMP) a failure in writeMagicContextConfig after the plugin entry was successfully added leaves the settings file referencing the package with no valid Magic Context config written. Consider adding a rollbackPluginEntry for the Pi host (e.g. remove the added package entry) to match the new rollback contract.
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 60:
<comment>DEFAULT_HOST doesn't implement rollbackPluginEntry, so for Pi (unlike OMP) a failure in writeMagicContextConfig after the plugin entry was successfully added leaves the settings file referencing the package with no valid Magic Context config written. Consider adding a rollbackPluginEntry for the Pi host (e.g. remove the added package entry) to match the new rollback contract.</comment>
<file context>
@@ -40,9 +41,29 @@ export interface SetupEnvironment {
+ dryRun: boolean;
+ configureHost: boolean;
+ }) => Promise<SetupRollback | false>;
+ rollbackPluginEntry?: (registration: PluginEntryResult) => Promise<void>;
+}
+
</file context>
| } | ||
|
|
||
| async function repair(plan: RepairPlan, deps: DoctorDeps, prompts: PromptIO): Promise<number> { | ||
| const omp = deps.detectOmpBinary(); |
There was a problem hiding this comment.
P2: repair() bails out entirely with if (!omp) return 0 before checking plan.writeUserConfig, so magic-context omp doctor --force will never create the default Magic Context config when the OMP binary isn't detected — even though that repair action has nothing to do with OMP being installed. Consider moving the writeUserConfig handling above the !omp early return (or handling it separately) so it always runs when needed.
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 300:
<comment>`repair()` bails out entirely with `if (!omp) return 0` before checking `plan.writeUserConfig`, so `magic-context omp doctor --force` will never create the default Magic Context config when the OMP binary isn't detected — even though that repair action has nothing to do with OMP being installed. Consider moving the `writeUserConfig` handling above the `!omp` early return (or handling it separately) so it always runs when needed.</comment>
<file context>
@@ -0,0 +1,411 @@
+}
+
+async function repair(plan: RepairPlan, deps: DoctorDeps, prompts: PromptIO): Promise<number> {
+ const omp = deps.detectOmpBinary();
+ if (!omp) return 0;
+ let fixed = 0;
</file context>
| } | ||
| } | ||
|
|
||
| let omp_models = get_available_omp_models().await; |
There was a problem hiding this comment.
P2: Inserting the OMP fallback before the pi login-shell fallback means a real Pi installation that's only resolvable via the login shell (e.g. an exotic PATH setup not covered by the static candidate list) will never be tried if OMP is also installed with a non-empty model list — get_available_pi_models will return OMP models under the 'pi' models cache instead. Consider moving the OMP fallback after the pi login-shell attempt (or making the two probes run independently) so a working Pi install is never masked by an unrelated OMP install.
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/commands.rs, line 1191:
<comment>Inserting the OMP fallback before the pi login-shell fallback means a real Pi installation that's only resolvable via the login shell (e.g. an exotic PATH setup not covered by the static candidate list) will never be tried if OMP is also installed with a non-empty model list — get_available_pi_models will return OMP models under the 'pi' models cache instead. Consider moving the OMP fallback after the pi login-shell attempt (or making the two probes run independently) so a working Pi install is never masked by an unrelated OMP install.</comment>
<file context>
@@ -1108,6 +1188,10 @@ pub async fn get_available_pi_models() -> Vec<String> {
}
}
+ let omp_models = get_available_omp_models().await;
+ if !omp_models.is_empty() {
+ return omp_models;
</file context>
| return { | ||
| ok: result.status === 0 && !result.error, | ||
| stdout: result.stdout?.trim() ?? "", | ||
| stderr: result.stderr?.trim() ?? result.error?.message ?? "", |
There was a problem hiding this comment.
P2: When the OMP command times out (or otherwise fails after spawning with no stderr output), result.stderr is an empty string rather than null/undefined, so the ?? fallback to result.error?.message never triggers and the real error (e.g. ETIMEDOUT) is silently dropped. Doctor/setup flows that display result.stderr to the user (adapters/omp.ts, doctor-omp.ts, setup-omp.ts) will show a blank or generic failure message instead of the actionable cause.
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 51:
<comment>When the OMP command times out (or otherwise fails after spawning with no stderr output), `result.stderr` is an empty string rather than null/undefined, so the `??` fallback to `result.error?.message` never triggers and the real error (e.g. `ETIMEDOUT`) is silently dropped. Doctor/setup flows that display `result.stderr` to the user (adapters/omp.ts, doctor-omp.ts, setup-omp.ts) will show a blank or generic failure message instead of the actionable cause.</comment>
<file context>
@@ -0,0 +1,143 @@
+ return {
+ ok: result.status === 0 && !result.error,
+ stdout: result.stdout?.trim() ?? "",
+ stderr: result.stderr?.trim() ?? result.error?.message ?? "",
+ };
+ } catch (error) {
</file context>
| stderr: result.stderr?.trim() ?? result.error?.message ?? "", | |
| stderr: result.stderr?.trim() || result.error?.message || "", |
| --- | ||
| title: Magic Context | ||
| description: Persistent memory and self-managing context for OpenCode and Pi coding agents. | ||
| description: Persistent memory and self-managing context for OpenCode, Pi, and Oh My Pi coding agents. |
There was a problem hiding this comment.
P3: The description now includes "Oh My Pi" in the supported-agents list, but the "Works with" section below still lists only OpenCode and Pi, and the sentence says "shared across both harnesses." Add OMP to the list and update "both" → "all" so the page is internally consistent.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/docs/src/content/docs/index.mdx, line 3:
<comment>The description now includes "Oh My Pi" in the supported-agents list, but the "Works with" section below still lists only OpenCode and Pi, and the sentence says "shared across both harnesses." Add OMP to the list and update "both" → "all" so the page is internally consistent.</comment>
<file context>
@@ -1,6 +1,6 @@
---
title: Magic Context
-description: Persistent memory and self-managing context for OpenCode and Pi coding agents.
+description: Persistent memory and self-managing context for OpenCode, Pi, and Oh My Pi coding agents.
template: splash
hero:
</file context>
| const binary = env.detectPiBinary(); | ||
| if (!binary) { | ||
| spinner.stop(`${host.displayName} not found`); | ||
| prompts.log.warn(`Could not find \`${host.cliName}\` on PATH.`); |
There was a problem hiding this comment.
P3: The generalized 'not found' message no longer mentions the home-directory fallback paths that detectPiBinary/detectOmpBinary actually check, which is a minor UX regression compared to the previous Pi-specific message.
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 285:
<comment>The generalized 'not found' message no longer mentions the home-directory fallback paths that detectPiBinary/detectOmpBinary actually check, which is a minor UX regression compared to the previous Pi-specific message.</comment>
<file context>
@@ -231,45 +278,40 @@ export async function runSetup(options: RunSetupOptions = {}): Promise<number> {
+ const binary = env.detectPiBinary();
+ if (!binary) {
+ spinner.stop(`${host.displayName} not found`);
+ prompts.log.warn(`Could not find \`${host.cliName}\` on PATH.`);
+ prompts.log.message(`Install ${host.displayName} first, then rerun setup.`);
+ prompts.outro(`Setup stopped — install ${host.displayName} and try again`);
</file context>
| try { | ||
| const value = req(path) as { version?: unknown }; | ||
| if (typeof value.version === "string") return value.version; | ||
| } catch { |
There was a problem hiding this comment.
P3: The catch-all message "gh CLI unavailable" is shown even when gh is installed but the user isn't authenticated (since gh auth status exits non-zero in that case too); consider distinguishing "not installed" from "not authenticated" for a clearer message.
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 120:
<comment>The catch-all message "gh CLI unavailable" is shown even when `gh` is installed but the user isn't authenticated (since `gh auth status` exits non-zero in that case too); consider distinguishing "not installed" from "not authenticated" for a clearer message.</comment>
<file context>
@@ -0,0 +1,411 @@
+ try {
+ const value = req(path) as { version?: unknown };
+ if (typeof value.version === "string") return value.version;
+ } catch {
+ // Try the source/published alternate layout.
+ }
</file context>
| const result = spawnSync(invocation.command, invocation.args, { | ||
| encoding: "utf-8", | ||
| timeout, | ||
| stdio: ["ignore", "pipe", "pipe"], | ||
| }); |
There was a problem hiding this comment.
P3: runOmpCommand's spawnSync call has no explicit maxBuffer, so a large model catalog or plugin list JSON payload could exceed Node's default 1MB buffer and fail the call (surfaced as a generic failure) instead of being parsed; consider setting a higher maxBuffer for the JSON-producing subcommands.
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 43:
<comment>`runOmpCommand`'s spawnSync call has no explicit `maxBuffer`, so a large model catalog or plugin list JSON payload could exceed Node's default 1MB buffer and fail the call (surfaced as a generic failure) instead of being parsed; consider setting a higher `maxBuffer` for the JSON-producing subcommands.</comment>
<file context>
@@ -0,0 +1,143 @@
+export function runOmpCommand(ompPath: string, args: string[], timeout = 30_000): OmpCommandResult {
+ try {
+ const invocation = getPiCommandInvocation(ompPath, args);
+ const result = spawnSync(invocation.command, invocation.args, {
+ encoding: "utf-8",
+ timeout,
</file context>
| const result = spawnSync(invocation.command, invocation.args, { | |
| encoding: "utf-8", | |
| timeout, | |
| stdio: ["ignore", "pipe", "pipe"], | |
| }); | |
| const result = spawnSync(invocation.command, invocation.args, { | |
| encoding: "utf-8", | |
| timeout, | |
| maxBuffer: 10 * 1024 * 1024, | |
| stdio: ["ignore", "pipe", "pipe"], | |
| }); |
| roots | ||
| } | ||
|
|
||
| pub fn scan_pi_session_dir() -> Vec<PiSessionMeta> { |
There was a problem hiding this comment.
P3: scan_pi_session_dir()/scan_pi_cache_session_dir() no longer call pi_sessions_root(), and no other file references it — it's now unused dead code left over from the refactor and can be removed.
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 184:
<comment>scan_pi_session_dir()/scan_pi_cache_session_dir() no longer call pi_sessions_root(), and no other file references it — it's now unused dead code left over from the refactor and can be removed.</comment>
<file context>
@@ -90,18 +90,109 @@ pub fn pi_sessions_root() -> Option<PathBuf> {
+ roots
+}
+
+pub fn scan_pi_session_dir() -> Vec<PiSessionMeta> {
+ pi_compatible_session_roots()
+ .into_iter()
</file context>
Co-authored-by: Wine Fox <fox@ling.plus>
|
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.
2 issues found across 42 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/dashboard/src-tauri/src/pi_sessions.rs">
<violation number="1" location="packages/dashboard/src-tauri/src/pi_sessions.rs:193">
P2: When two duplicate session entries have identical `modified` timestamps, the tie-break falls back to lexical comparison of `jsonl_path`, which has no semantic relationship to which root should take priority (e.g. OMP vs legacy Pi vs XDG). Consider breaking ties using an explicit root-priority index captured during root traversal instead of raw path bytes, so dedup resolution is deterministic and matches intended precedence.</violation>
<violation number="2" location="packages/dashboard/src-tauri/src/pi_sessions.rs:198">
P3: Deduplication now performs `fs::canonicalize` per session on every scan across all discovered roots, which is extra I/O on top of the existing meta cache and could add latency as the number of OMP profiles/roots grows. Consider caching canonicalized paths or only canonicalizing when a path collision by non-canonical form is first detected.</violation>
</file>
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
| right | ||
| .modified | ||
| .cmp(&left.modified) | ||
| .then_with(|| left.jsonl_path.cmp(&right.jsonl_path)) |
There was a problem hiding this comment.
P2: When two duplicate session entries have identical modified timestamps, the tie-break falls back to lexical comparison of jsonl_path, which has no semantic relationship to which root should take priority (e.g. OMP vs legacy Pi vs XDG). Consider breaking ties using an explicit root-priority index captured during root traversal instead of raw path bytes, so dedup resolution is deterministic and matches intended precedence.
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>When two duplicate session entries have identical `modified` timestamps, the tie-break falls back to lexical comparison of `jsonl_path`, which has no semantic relationship to which root should take priority (e.g. OMP vs legacy Pi vs XDG). Consider breaking ties using an explicit root-priority index captured during root traversal instead of raw path bytes, so dedup resolution is deterministic and matches intended precedence.</comment>
<file context>
@@ -90,18 +90,137 @@ pub fn pi_sessions_root() -> Option<PathBuf> {
+ right
+ .modified
+ .cmp(&left.modified)
+ .then_with(|| left.jsonl_path.cmp(&right.jsonl_path))
+ });
+ let mut seen_paths = HashSet::new();
</file context>
| let mut seen_paths = HashSet::new(); | ||
| let mut seen_ids = HashSet::new(); | ||
| sessions.retain(|session| { | ||
| let canonical_path = |
There was a problem hiding this comment.
P3: Deduplication now performs fs::canonicalize per session on every scan across all discovered roots, which is extra I/O on top of the existing meta cache and could add latency as the number of OMP profiles/roots grows. Consider caching canonicalized paths or only canonicalizing when a path collision by non-canonical form is first detected.
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 198:
<comment>Deduplication now performs `fs::canonicalize` per session on every scan across all discovered roots, which is extra I/O on top of the existing meta cache and could add latency as the number of OMP profiles/roots grows. Consider caching canonicalized paths or only canonicalizing when a path collision by non-canonical form is first detected.</comment>
<file context>
@@ -90,18 +90,137 @@ pub fn pi_sessions_root() -> Option<PathBuf> {
+ let mut seen_paths = HashSet::new();
+ let mut seen_ids = HashSet::new();
+ sessions.retain(|session| {
+ let canonical_path =
+ fs::canonicalize(&session.jsonl_path).unwrap_or_else(|_| session.jsonl_path.clone());
+ if !seen_paths.insert(canonical_path) {
</file context>
1 participant
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
Add first-class Oh My Pi (OMP) support across the CLI, extension, and dashboard. You can install, health-check, and migrate sessions to OMP, with models and sessions visible in the dashboard; the runtime, config, and database are shared with Pi. The dashboard now deduplicates multi-root sessions.
New Features
ompharness adapter withsetup --harness ompanddoctor --harness omp; auto-detects the OMP binary, plugin entry, and version; reads OMP profiles and XDG paths; discovers models viaomp models --json.doctor --harness omp --force.@cortexkit/pi-magic-contextdeclares an OMP manifest and runs on both Pi and OMP; resolves relative allowlist entries fromPI_CODING_AGENT_DIR.harness='pi'for session rows; project memories and config are shared across OpenCode, Pi, and OMP.Migration
doctor migrate --from opencode --to ompconverts an OpenCode session to Pi/OMP JSONL and writes it to the active OMP sessions root.Written for commit f979800. Summary will update on new commits.
Greptile Summary
Adds first-class Oh My Pi support across the CLI, Pi-compatible extension, dashboard, 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 / OMP] Detect --> Setup[Setup and doctor] Setup --> OMP[OMP plugin manager and settings] OMP --> Runtime[Pi-compatible Magic Context runtime] Runtime --> DB[(Shared CortexKit database)] PiRoots[Pi session roots] --> Dashboard[Dashboard session discovery] OMPRoots[OMP default and profile roots] --> Dashboard Dashboard --> Dedup[Deduplicate logical sessions] Dedup --> SessionUI[Session list and details]Reviews (3): Last reviewed commit: "fix(dashboard): 🐛 deduplicate multi-roo..." | Re-trigger Greptile