Skip to content

feat: add full Oh My Pi (OMP) support — rebased, conflict-free - #298

Open
Qiiks wants to merge 19 commits into
cortexkit:masterfrom
Qiiks:feat/omp-support-rebased
Open

feat: add full Oh My Pi (OMP) support — rebased, conflict-free#298
Qiiks wants to merge 19 commits into
cortexkit:masterfrom
Qiiks:feat/omp-support-rebased

Conversation

@Qiiks

@Qiiks Qiiks commented Aug 10, 2026

Copy link
Copy Markdown

This PR carries the complete work of #256 (Lynricsy's feat/omp-support) rebased onto current master with all 16 merge conflicts resolved, verified.

What #256 adds (unchanged)

  • CLI: setup --harness omp / doctor --harness omp (auto-detect, min-version check, disables native compaction + automatic memory, atomic config writes, --force repair)
  • doctor migrate --from opencode --to omp (OpenCode DB → Pi/OMP JSONL + Magic Context compartment re-home)
  • Pi-compatible extension with explicit OMP manifest ("omp": {"extensions": [...]}), optional Pi peers
  • Dashboard: OMP model/session discovery incl. named profiles, dedup across aliased roots
  • CI: real OMP Docker E2E lane

Rebase work (this PR)

  • Rebased 19 commits onto master — conflicts in packages/cli/package.json, packages/pi-plugin/package.json, packages/pi-plugin/src/subagent-runner.test.ts, packages/plugin/scripts/build-config-docs.ts, and 3 docs files resolved (kept master's 0.35.1 versions + merged OMP semantics; docs wording merged to cover all three harnesses)
  • Verification on Windows: workspace typecheck clean; CLI suite 296 pass / 10 skip / 22 fail — the identical 22 tests fail on the untouched pre-rebase PR head (sh-fixture/ComSpec/chmod environment artifacts; CI's Linux lane is the authoritative gate)
  • Greptile (5/5 safe-to-merge), cubic, Socket, and the author's own test claims all previously green

Closes #256's blocker (mergeable state was CONFLICTING). Can be merged or used to unblock #256.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Adds full Oh My Pi (OMP) support across the CLI, Pi/OMP extension, and dashboard with real Docker E2E coverage. Enables OpenCode → OMP migration and keeps one shared config and database across OpenCode, Pi, and OMP.

  • New Features

    • CLI: --harness omp for setup and doctor; disables OMP native compaction and automatic memory; atomic config writes; auto-detect updated; migration supports doctor migrate --from opencode --to omp.
    • Extension: @cortexkit/pi-magic-context runs on Pi and OMP; adds an OMP manifest ("omp": {"extensions": [...]}); translates provider/model refs for OMP; subagent runner handles OMP model resolution and paths.
    • Dashboard: discovers OMP models and sessions (profiles supported), merges with Pi, and deduplicates across aliased roots.
    • CI: adds a real OMP Docker E2E lane and focused unit tests for OMP CLI/paths/provider mapping.
  • Bug Fixes

    • Hardened OMP discovery and precedence (require executable evidence), normalized/trimmed env paths, and isolated host config boundaries; safer CLI/provider lookups.
    • Deduplicated multi-root sessions and improved session/model gating for OMP.
    • Cleanup: ignore agent logs and refine doctor/setup path handling.

Written for commit 8302900. Summary will update on new commits.

Review in cubic

Greptile Summary

Adds Oh My Pi support throughout Magic Context.

  • Adds OMP setup, doctor, detection, path resolution, and OpenCode migration support to the CLI.
  • Adds OMP model and session discovery to the dashboard.
  • Makes the Pi-compatible extension available to OMP and adds a real OMP Docker E2E lane.

Confidence Score: 5/5

The PR appears safe to merge because no blocking failure remains within the scope of the available follow-up review threads.

No blocking failure remains.

Important Files Changed

Filename Overview
packages/cli/src/adapters/omp.ts Implements OMP detection, plugin registration, removal, cache inspection, and version discovery.
packages/cli/src/commands/setup-omp.ts Integrates OMP host configuration and rollback behavior with the shared Pi-compatible setup flow.
packages/cli/src/commands/doctor-omp.ts Adds OMP-specific installation, configuration, conflict, path, and database diagnostics and repairs.
packages/cli/src/commands/migrate.ts Extends OpenCode session migration to write Pi-compatible JSONL sessions under the resolved OMP session root.
packages/dashboard/src-tauri/src/pi_sessions.rs Extends Pi-compatible session discovery to OMP roots and profiles with alias deduplication.
packages/pi-plugin/src/subagent-runner.ts Adapts Pi-compatible subagent execution and provider/model handling for OMP.
packages/plugin/src/shared/harness-provider-map.ts Adds OMP-aware provider and model reference translation.
.github/workflows/ci.yml Adds a Docker-based OMP installation and session smoke-test job.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    CLI[Magic Context CLI] --> Detect[Detect OMP and resolve profile paths]
    Detect --> Setup[Install extension and configure OMP]
    Detect --> Doctor[Check and repair OMP integration]
    CLI --> Migration[Convert OpenCode sessions to OMP JSONL]
    Extension[Pi-compatible extension] --> OMP[Oh My Pi runtime]
    OMP --> Store[(Shared Magic Context database)]
    Migration --> Sessions[OMP session directory]
    Dashboard[Dashboard] --> Sessions
    Dashboard --> Store
Loading

Reviews (2): Last reviewed commit: "test(omp): 🧪 tighten smoke and local ru..." | Re-trigger Greptile

Copilot AI lite review requested due to automatic review settings August 10, 2026 12:30

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Lynricsy and others added 19 commits August 10, 2026 18:06
Co-authored-by: Wine Fox <fox@ling.plus>
Co-authored-by: Lynricsy <im@ling.plus>
Co-authored-by: Wine Fox <fox@ling.plus>
Co-authored-by: Lynricsy <im@ling.plus>
Co-authored-by: Wine Fox <fox@ling.plus>
Co-authored-by: Lynricsy <im@ling.plus>
Co-authored-by: Wine Fox <fox@ling.plus>
Co-authored-by: Lynricsy <im@ling.plus>
Co-authored-by: Wine Fox <fox@ling.plus>
Co-authored-by: Lynricsy <im@ling.plus>
Co-authored-by: Wine Fox <fox@ling.plus>
Co-authored-by: Lynricsy <im@ling.plus>
Co-authored-by: Wine Fox <fox@ling.plus>
Co-authored-by: Lynricsy <im@ling.plus>
Co-authored-by: Wine Fox <fox@ling.plus>
Co-authored-by: Lynricsy <im@ling.plus>
Co-authored-by: Wine Fox <fox@ling.plus>
Co-authored-by: Lynricsy <im@ling.plus>
Co-authored-by: Wine Fox <fox@ling.plus>
Co-authored-by: Lynricsy <im@ling.plus>
Co-authored-by: Wine Fox <fox@ling.plus>
Co-authored-by: Lynricsy <im@ling.plus>
Co-authored-by: Wine Fox <fox@ling.plus>
Co-authored-by: Lynricsy <im@ling.plus>
Co-authored-by: Wine Fox <fox@ling.plus>
Co-authored-by: Lynricsy <im@ling.plus>
Co-authored-by: Wine Fox <fox@ling.plus>
Co-authored-by: Lynricsy <im@ling.plus>
Co-authored-by: Wine Fox <fox@ling.plus>
Co-authored-by: Lynricsy <im@ling.plus>
Co-authored-by: Wine Fox <fox@ling.plus>
Co-authored-by: Lynricsy <im@ling.plus>
Co-authored-by: Wine Fox <fox@ling.plus>
Co-authored-by: Lynricsy <im@ling.plus>
Co-authored-by: Wine Fox <fox@ling.plus>
Co-authored-by: Lynricsy <im@ling.plus>
Co-authored-by: Wine Fox <fox@ling.plus>
Co-authored-by: Lynricsy <im@ling.plus>
@Qiiks
Qiiks force-pushed the feat/omp-support-rebased branch from fc0fd0c to 8302900 Compare August 10, 2026 12:37

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

12 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/cli/src/commands/doctor-omp.ts">

<violation number="1" location="packages/cli/src/commands/doctor-omp.ts:456">
P2: `doctor --harness omp --force` skips default-config repair when missing config is the only warning; return early only when `--force` is absent so the planned repair runs.</violation>
</file>

<file name="packages/pi-plugin/src/subagent-runner.ts">

<violation number="1" location="packages/pi-plugin/src/subagent-runner.ts:206">
P1: OMP hosts configured with `PI_PACKAGE_DIR=~/...` are misidentified as plain Pi because this lookup does not expand `~`. Expand OMP’s supported home-relative override before reading its manifest.</violation>

<violation number="2" location="packages/pi-plugin/src/subagent-runner.ts:233">
P1: Named-profile OMP children resolve relative extensions through `PI_CODING_AGENT_DIR` even when it is stale, instead of the active profile directory. Resolve the profile first and ignore the override whenever a named profile is selected.</violation>
</file>

<file name="tests/docker/run-e2e.sh">

<violation number="1" location="tests/docker/run-e2e.sh:58">
P3: The new comment says only Dockerfile.pi and Dockerfile.omp COPY packages/cli/dist, but Dockerfile.opencode also COPYs packages/cli/dist (line 75) for its `magic-context doctor --harness …` invocation. Since the cli build added here actually benefits all three images, the comment understates the dependency and could mislead later readers into thinking the CLI build only matters for pi/omp.</violation>
</file>

<file name="packages/dashboard/src-tauri/src/commands.rs">

<violation number="1" location="packages/dashboard/src-tauri/src/commands.rs:1087">
P2: OMP-only source/Nix/Guix installs with `PI_PACKAGE_DIR` set but no PATH shim produce an empty dashboard catalog. Mirror the CLI's package-root discovery and invoke `dist/cli.js` through Bun.</violation>
</file>

<file name="packages/cli/src/adapters/omp.ts">

<violation number="1" location="packages/cli/src/adapters/omp.ts:52">
P3: OMP install/enable/uninstall and rollback paths have no adapter coverage, so command or lockfile-state regressions ship undetected; add fake-OMP tests for install, enable, failed verification rollback, and uninstall.</violation>
</file>

<file name="packages/dashboard/src-tauri/src/pi_sessions.rs">

<violation number="1" location="packages/dashboard/src-tauri/src/pi_sessions.rs:235">
P2: OMP sessions disappear when `PI_CONFIG_DIR` begins with a path separator because this Rust join treats it as an absolute root, unlike repository OMP resolution. Normalize this value with the same home-relative OMP semantics before building session roots.</violation>
</file>

<file name="packages/docs/src/content/docs/getting-started/migrating-between-harnesses.md">

<violation number="1" location="packages/docs/src/content/docs/getting-started/migrating-between-harnesses.md:43">
P2: The new `--to omp` block tells users the file is written into OMP's active profile/XDG session directory, but the immediately-following "What happens" and "After migration" sections (unchanged in this PR) still describe only the Pi path (`~/.pi/agent/sessions/`) and tell the user to "restart Pi" and look in "Pi's session picker". A user following the OMP path now gets contradictory, Pi-only instructions. Please update those downstream sections to mention OMP's directory and restart/picker when `--to omp` is used (and likewise the /ctx-recomp note), so the OMP migration flow is accurate end to end.</violation>
</file>

<file name=".github/workflows/ci.yml">

<violation number="1" location=".github/workflows/ci.yml:218">
P3: The pipeline-shape comment at the top of this file still enumerates the Docker smoke layer as only "e2e-opencode, e2e-pi", and no longer reflects the newly added e2e-omp job. Update the diagram and the following layer descriptions so the documented architecture matches the actual workflow and future readers understand OMP is part of the gated Docker smoke tier.</violation>
</file>

<file name="packages/cli/src/adapters/omp.test.ts">

<violation number="1" location="packages/cli/src/adapters/omp.test.ts:30">
P2: This test mocks the `omp` binary as a shebang shell script named `omp` with no extension, which is POSIX-only. On Windows `detectOmpBinary` -> `findOnPath("omp")` only matches `omp.exe/.cmd/.bat/.com`, so it returns `null`, `isInstalled()` is `false`, and all three assertions fail even though the adapter logic is correct. Consider making the mock platform-aware (write a `omp.cmd` on win32) or skipping this test on Windows so it doesn't create a spurious per-platform failure.</violation>
</file>

<file name="packages/cli/src/lib/omp-helpers.test.ts">

<violation number="1" location="packages/cli/src/lib/omp-helpers.test.ts:60">
P3: This test intends to prove detectOmpBinary() returns null when no Bun runtime can run the package CLI, but on Windows it never isolates APPDATA. getOmpFallbackCandidates('win32', ...) still checks %APPDATA%\npm\omp.cmd, so on a developer machine with a global npm OMP install detectOmpBinary() returns that real path and the test fails. Set APPDATA (and XDG_DATA_HOME, for symmetry) to a temp dir in the test/makePackageRoot so fallback discovery is fully hermetic across platforms.</violation>
</file>

<file name="packages/cli/src/commands/setup-omp.test.ts">

<violation number="1" location="packages/cli/src/commands/setup-omp.test.ts:106">
P3: Tests 1 and 2 run against `process.cwd()` (the repo root) instead of an isolated temp dir, so the 'project/overlay config active' guard in beforeWrite depends on the ambient environment staying clean (no `.omp/config.yml` in the cwd and no `PI_CONFIG_FILES` set). If a developer or CI runs the suite with `PI_CONFIG_FILES` exported or with a local `.omp` config present, these two tests would spuriously fail. Use a per-test temp dir (as the other tests do) for `cwd` to make them hermetic.</violation>
</file>

Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.

Re-trigger cubic

Comment on lines +206 to +207
const packageOverride = process.env.PI_PACKAGE_DIR?.trim();
if (packageOverride && packageRootIsOmp(resolvePath(packageOverride)))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: OMP hosts configured with PI_PACKAGE_DIR=~/... are misidentified as plain Pi because this lookup does not expand ~. Expand OMP’s supported home-relative override before reading its manifest.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/pi-plugin/src/subagent-runner.ts, line 206:

<comment>OMP hosts configured with `PI_PACKAGE_DIR=~/...` are misidentified as plain Pi because this lookup does not expand `~`. Expand OMP’s supported home-relative override before reading its manifest.</comment>

<file context>
@@ -176,13 +184,75 @@ const TERMINAL_DRAIN_GRACE_MS = 2_000;
+	const execName = basename(process.execPath).toLowerCase();
+	if (/^omp(?:\.exe)?$/.test(execName)) return true;
+
+	const packageOverride = process.env.PI_PACKAGE_DIR?.trim();
+	if (packageOverride && packageRootIsOmp(resolvePath(packageOverride)))
+		return true;
</file context>
Suggested change
const packageOverride = process.env.PI_PACKAGE_DIR?.trim();
if (packageOverride && packageRootIsOmp(resolvePath(packageOverride)))
const packageOverride = process.env.PI_PACKAGE_DIR?.trim();
const resolvedPackageOverride =
packageOverride === "~"
? homedir()
: packageOverride?.startsWith("~/") || packageOverride?.startsWith("~\\")
? resolvePath(homedir(), packageOverride.slice(2))
: packageOverride && resolvePath(packageOverride);
if (resolvedPackageOverride && packageRootIsOmp(resolvedPackageOverride))
return true;

Comment on lines +233 to +243
const configured = process.env.PI_CODING_AGENT_DIR?.trim();
if (configured) return resolvePath(configured);
const configRoot = join(
homedir(),
process.env.PI_CONFIG_DIR?.trim() || ".omp",
);
const profile = normalizedOmpProfile();
return profile
? join(configRoot, "profiles", profile, "agent")
: join(configRoot, "agent");
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Named-profile OMP children resolve relative extensions through PI_CODING_AGENT_DIR even when it is stale, instead of the active profile directory. Resolve the profile first and ignore the override whenever a named profile is selected.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/pi-plugin/src/subagent-runner.ts, line 233:

<comment>Named-profile OMP children resolve relative extensions through `PI_CODING_AGENT_DIR` even when it is stale, instead of the active profile directory. Resolve the profile first and ignore the override whenever a named profile is selected.</comment>

<file context>
@@ -176,13 +184,75 @@ const TERMINAL_DRAIN_GRACE_MS = 2_000;
+// consume it without the positive host check or plain-Pi argv changes.
+function getHostAgentSettingsDir(): string {
+	if (!isOmpHostProcess()) return join(homedir(), ".pi", "agent");
+	const configured = process.env.PI_CODING_AGENT_DIR?.trim();
+	if (configured) return resolvePath(configured);
+	const configRoot = join(
</file context>
Suggested change
const configured = process.env.PI_CODING_AGENT_DIR?.trim();
if (configured) return resolvePath(configured);
const configRoot = join(
homedir(),
process.env.PI_CONFIG_DIR?.trim() || ".omp",
);
const profile = normalizedOmpProfile();
return profile
? join(configRoot, "profiles", profile, "agent")
: join(configRoot, "agent");
}
const configRoot = join(
homedir(),
process.env.PI_CONFIG_DIR?.trim() || ".omp",
);
const profile = normalizedOmpProfile();
if (profile) return join(configRoot, "profiles", profile, "agent");
const configured = process.env.PI_CODING_AGENT_DIR?.trim();
return configured ? resolvePath(configured) : join(configRoot, "agent");

prompts.intro("Magic Context for Oh My Pi (OMP) Doctor");
const first = await runHealthChecks({ cwd, prompts, deps });
prompts.log.message(`Summary: PASS ${first.pass} / WARN ${first.warn} / FAIL ${first.fail}`);
if (!options.force || first.fail === 0) return first.fail === 0 ? 0 : 1;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: doctor --harness omp --force skips default-config repair when missing config is the only warning; return early only when --force is absent so the planned repair runs.

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 456:

<comment>`doctor --harness omp --force` skips default-config repair when missing config is the only warning; return early only when `--force` is absent so the planned repair runs.</comment>

<file context>
@@ -0,0 +1,469 @@
+    prompts.intro("Magic Context for Oh My Pi (OMP) Doctor");
+    const first = await runHealthChecks({ cwd, prompts, deps });
+    prompts.log.message(`Summary: PASS ${first.pass} / WARN ${first.warn} / FAIL ${first.fail}`);
+    if (!options.force || first.fail === 0) return first.fail === 0 ? 0 : 1;
+    if (migrationRefused && first.repairPlan.writeUserConfig) {
+        first.repairPlan.writeUserConfig = false;
</file context>

list
} else {
let home = std::env::var("HOME").unwrap_or_default();
vec![

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: OMP-only source/Nix/Guix installs with PI_PACKAGE_DIR set but no PATH shim produce an empty dashboard catalog. Mirror the CLI's package-root discovery and invoke dist/cli.js through Bun.

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 1087:

<comment>OMP-only source/Nix/Guix installs with `PI_PACKAGE_DIR` set but no PATH shim produce an empty dashboard catalog. Mirror the CLI's package-root discovery and invoke `dist/cli.js` through Bun.</comment>

<file context>
@@ -1037,6 +1037,102 @@ pub fn parse_pi_models_output(text: &str) -> Vec<String> {
+        list
+    } else {
+        let home = std::env::var("HOME").unwrap_or_default();
+        vec![
+            format!("{}/.bun/bin/omp", home),
+            "omp".to_string(),
</file context>

if include_omp {
let config_dir = trimmed_env_path(std::env::var_os("PI_CONFIG_DIR"))
.unwrap_or_else(|| PathBuf::from(".omp"));
let config_root = home.join(&config_dir);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: OMP sessions disappear when PI_CONFIG_DIR begins with a path separator because this Rust join treats it as an absolute root, unlike repository OMP resolution. Normalize this value with the same home-relative OMP semantics before building session roots.

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>OMP sessions disappear when `PI_CONFIG_DIR` begins with a path separator because this Rust join treats it as an absolute root, unlike repository OMP resolution. Normalize this value with the same home-relative OMP semantics before building session roots.</comment>

<file context>
@@ -81,27 +81,259 @@ fn test_root() -> &'static RwLock<Option<PathBuf>> {
+    if include_omp {
+        let config_dir = trimmed_env_path(std::env::var_os("PI_CONFIG_DIR"))
+            .unwrap_or_else(|| PathBuf::from(".omp"));
+        let config_root = home.join(&config_dir);
+        let default_agent = config_root.join("agent");
+        let active_profile = active_omp_profile();
</file context>

Comment thread tests/docker/run-e2e.sh
Comment on lines +58 to +59
# Dockerfile.pi and Dockerfile.omp both COPY packages/cli/dist for their
# `magic-context doctor --harness …` invocations.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The new comment says only Dockerfile.pi and Dockerfile.omp COPY packages/cli/dist, but Dockerfile.opencode also COPYs packages/cli/dist (line 75) for its magic-context doctor --harness … invocation. Since the cli build added here actually benefits all three images, the comment understates the dependency and could mislead later readers into thinking the CLI build only matters for pi/omp.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/docker/run-e2e.sh, line 58:

<comment>The new comment says only Dockerfile.pi and Dockerfile.omp COPY packages/cli/dist, but Dockerfile.opencode also COPYs packages/cli/dist (line 75) for its `magic-context doctor --harness …` invocation. Since the cli build added here actually benefits all three images, the comment understates the dependency and could mislead later readers into thinking the CLI build only matters for pi/omp.</comment>

<file context>
@@ -54,6 +55,9 @@ run_target() {
 echo "Pre-building local dist artifacts..."
 bun run --cwd "$REPO_ROOT/packages/plugin" build
 bun run --cwd "$REPO_ROOT/packages/pi-plugin" build
+# Dockerfile.pi and Dockerfile.omp both COPY packages/cli/dist for their
+# `magic-context doctor --harness …` invocations.
+bun run --cwd "$REPO_ROOT/packages/cli" build
</file context>
Suggested change
# Dockerfile.pi and Dockerfile.omp both COPY packages/cli/dist for their
# `magic-context doctor --harness …` invocations.
# All three Dockerfiles (opencode, pi, omp) COPY packages/cli/dist for their
# `magic-context doctor --harness …` invocations.

};
}

async ensurePluginEntry(): Promise<PluginEntryResult> {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: OMP install/enable/uninstall and rollback paths have no adapter coverage, so command or lockfile-state regressions ship undetected; add fake-OMP tests for install, enable, failed verification rollback, and uninstall.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/adapters/omp.ts, line 52:

<comment>OMP install/enable/uninstall and rollback paths have no adapter coverage, so command or lockfile-state regressions ship undetected; add fake-OMP tests for install, enable, failed verification rollback, and uninstall.</comment>

<file context>
@@ -0,0 +1,193 @@
+        };
+    }
+
+    async ensurePluginEntry(): Promise<PluginEntryResult> {
+        const configPath = getOmpPluginsLockPath();
+        const omp = detectOmpBinary();
</file context>

Comment thread .github/workflows/ci.yml
@@ -215,6 +215,37 @@ jobs:
- name: Run E2E

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The pipeline-shape comment at the top of this file still enumerates the Docker smoke layer as only "e2e-opencode, e2e-pi", and no longer reflects the newly added e2e-omp job. Update the diagram and the following layer descriptions so the documented architecture matches the actual workflow and future readers understand OMP is part of the gated Docker smoke tier.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/ci.yml, line 218:

<comment>The pipeline-shape comment at the top of this file still enumerates the Docker smoke layer as only "e2e-opencode, e2e-pi", and no longer reflects the newly added e2e-omp job. Update the diagram and the following layer descriptions so the documented architecture matches the actual workflow and future readers understand OMP is part of the gated Docker smoke tier.</comment>

<file context>
@@ -215,6 +215,37 @@ jobs:
       - name: Run E2E
         run: docker run --rm --platform linux/amd64 mc-e2e-pi
 
+  e2e-omp:
+    name: E2E (Oh My Pi, real Docker)
+    runs-on: ubuntu-latest
</file context>


it("ignores the package root when no Bun runtime can execute the CLI script", () => {
const { root } = makePackageRoot();
process.env.PATH = join(root, "empty-bin");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: This test intends to prove detectOmpBinary() returns null when no Bun runtime can run the package CLI, but on Windows it never isolates APPDATA. getOmpFallbackCandidates('win32', ...) still checks %APPDATA%\npm\omp.cmd, so on a developer machine with a global npm OMP install detectOmpBinary() returns that real path and the test fails. Set APPDATA (and XDG_DATA_HOME, for symmetry) to a temp dir in the test/makePackageRoot so fallback discovery is fully hermetic across platforms.

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.test.ts, line 60:

<comment>This test intends to prove detectOmpBinary() returns null when no Bun runtime can run the package CLI, but on Windows it never isolates APPDATA. getOmpFallbackCandidates('win32', ...) still checks %APPDATA%\npm\omp.cmd, so on a developer machine with a global npm OMP install detectOmpBinary() returns that real path and the test fails. Set APPDATA (and XDG_DATA_HOME, for symmetry) to a temp dir in the test/makePackageRoot so fallback discovery is fully hermetic across platforms.</comment>

<file context>
@@ -0,0 +1,141 @@
+
+    it("ignores the package root when no Bun runtime can execute the CLI script", () => {
+        const { root } = makePackageRoot();
+        process.env.PATH = join(root, "empty-bin");
+
+        expect(detectOmpBinary()).toBeNull();
</file context>

const prompts = new MockPrompts([true, true]);
const rollback = await __test.OMP_HOST.beforeWrite?.({
binaryPath: binary,
cwd: process.cwd(),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Tests 1 and 2 run against process.cwd() (the repo root) instead of an isolated temp dir, so the 'project/overlay config active' guard in beforeWrite depends on the ambient environment staying clean (no .omp/config.yml in the cwd and no PI_CONFIG_FILES set). If a developer or CI runs the suite with PI_CONFIG_FILES exported or with a local .omp config present, these two tests would spuriously fail. Use a per-test temp dir (as the other tests do) for cwd to make them hermetic.

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-omp.test.ts, line 106:

<comment>Tests 1 and 2 run against `process.cwd()` (the repo root) instead of an isolated temp dir, so the 'project/overlay config active' guard in beforeWrite depends on the ambient environment staying clean (no `.omp/config.yml` in the cwd and no `PI_CONFIG_FILES` set). If a developer or CI runs the suite with `PI_CONFIG_FILES` exported or with a local `.omp` config present, these two tests would spuriously fail. Use a per-test temp dir (as the other tests do) for `cwd` to make them hermetic.</comment>

<file context>
@@ -0,0 +1,208 @@
+        const prompts = new MockPrompts([true, true]);
+        const rollback = await __test.OMP_HOST.beforeWrite?.({
+            binaryPath: binary,
+            cwd: process.cwd(),
+            prompts,
+            dryRun: false,
</file context>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants