Add Pi package compatibility layer#811
Conversation
Add source parsing, manifest normalization, known profiles, doctor reports, import aliases, CLI shim generation, and safe legacy path bridge helpers. Export the new pi-compat API from the coding-agent package for integration points and tests.
Add omp pi commands and --compat-pi plugin install/doctor flows for npm, git, and local Pi package sources. Resolve Pi skills, prompts, and themes from plugins, and activate scoped shims/env before extension and tool loading.
Exercise Pi source parsing, manifest normalization, resource resolution, shim generation, path bridge safety, known profiles, and local doctor classification.
Describe tiers, install grammar, resource support, scoped shims, bridge modes, doctor output, and known package profiles. Refs can1357#433.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 25044de15e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| return execCommand(command, args, options?.cwd ?? this.cwd, { | ||
| ...options, | ||
| env: buildPiCompatEnv({ baseEnv: process.env }), | ||
| }); |
There was a problem hiding this comment.
ConcreteExtensionAPI.exec now unconditionally sets env from buildPiCompatEnv({ baseEnv: process.env }), which discards options.env from the caller. Any extension that relies on per-invocation env overrides (for example API tokens, feature flags, or PATH changes) will silently lose those values after this change and execute commands with the wrong environment. Merge options.env into the base env (or layer it after building compat env) to keep the existing exec contract.
Useful? React with 👍 / 👎.
| if (options.compatPi) { | ||
| await activatePiCompatEnvironment({ packageName: source.packageNameHint, bridgeMode: "profile" }); | ||
| } | ||
|
|
||
| if (options.dryRun) { |
There was a problem hiding this comment.
The compat setup runs before the dry-run guard, so install(..., { compatPi: true, dryRun: true }) still performs side effects. activatePiCompatEnvironment() writes shim/home files and mutates process.env, which violates the --dry-run expectation of previewing changes without applying them. This affects pi install --dry-run and plugin install --compat-pi --dry-run flows; the activation should happen only after the dry-run early return.
Useful? React with 👍 / 👎.
Resolve shim targets to the installed OMP runtime instead of relying on plugin-local node_modules resolution. Expand Pi extension directory manifest entries to concrete index files, and add compatibility shims for common legacy exports used by tested Pi packages.
HabibPro1999
force-pushed
the
feat/pi-package-compat
branch
from
2e93f9d to
06d1705
Compare
|
Sandbox follow-up on real Pi packages after the compatibility fixes:
This surfaced and fixed two compatibility gaps before merge:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 06d1705d0b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| function isLocalSource(spec: string): boolean { | ||
| return ( | ||
| spec.startsWith("./") || | ||
| spec.startsWith("../") || | ||
| spec.startsWith("/") || | ||
| spec.startsWith("~/") || |
There was a problem hiding this comment.
parsePiInstallSource() only treats ./, ../, /, ~/, ., .., and file: as local sources, so Windows drive-letter paths like C:/plugins/foo are classified as npm specs. In PluginManager.install(), that path then goes through npm package-name validation and fails, which blocks local plugin installs on Windows for plugin install/pi install. Add drive-letter path detection (and corresponding safe handling) in the local-source check.
Useful? React with 👍 / 👎.
| const skillPath = path.join(skillDir, "SKILL.md"); | ||
| const content = await readFile(skillPath); |
There was a problem hiding this comment.
Skill resource paths are always treated as directories, and loadDirectSkill() unconditionally appends SKILL.md. If a plugin manifest provides a direct skill-file entry (for example skills: ["skills/my-skill/SKILL.md"]), discovery looks for <that-file>/SKILL.md and drops the skill. Add a file-path branch (similar to prompt loading) so direct skill-file entries are loaded correctly.
Useful? React with 👍 / 👎.
Preserve per-call extension exec env, keep compat setup out of dry-run installs, treat Windows drive paths as local sources, and load direct SKILL.md plugin resources.
|
Addressed the latest Codex review feedback in
Verification:
|
Keep the user-facing Pi compatibility note under the mutable Unreleased section per repository changelog rules.
|
Contribution-guideline audit complete. I found one changelog placement issue after the rebase: the Pi compatibility entry had landed under released |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 085ba0b0d3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const runtime = new ExtensionRuntime(); | ||
| if (paths.length > 0) { | ||
| await ensurePiCompatImportShims(); | ||
| await activatePiCompatEnvironment({ bridgeMode: "env" }); |
There was a problem hiding this comment.
loadExtensions unconditionally calls activatePiCompatEnvironment({ bridgeMode: "env" }), which overwrites any preselected OMP_PI_COMPAT_BRIDGE mode and forces all extension subprocesses back to env-only bridging. This breaks Tier 3 compatibility scenarios (e.g., child-home) because the shim only rewrites HOME when the mode remains child-home, so packages with hardcoded ~/.pi paths can still touch the real home directory. The same forced mode is also present in custom-tools/loader.ts, so both extension and tool execution paths ignore the user/profile-selected bridge mode.
Useful? React with 👍 / 👎.
|
Addressed the latest bridge-mode review in
Added regression coverage for both extension and custom-tool loading paths. Verification:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5527d5c120
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // Run npm install | ||
| const proc = Bun.spawn(["bun", "install", spec.packageName], { | ||
| if (options.compatPi) { | ||
| await activatePiCompatEnvironment({ packageName: source.packageNameHint, bridgeMode: "profile" }); |
There was a problem hiding this comment.
PluginManager.install() forwards source.packageNameHint directly into activatePiCompatEnvironment(), but npm-style specs keep the version suffix (for example @tmustier/pi-agent-teams@0.5.4). Profile lookup is exact-name based, so version-pinned installs miss their compatibility profile and skip profile env injection (such as PI_TEAMS_ROOT_DIR), which undermines the documented --compat-pi install path.
Useful? React with 👍 / 👎.
| if (mode === "child-home") { | ||
| env.HOME = getPiCompatHomeDir(); |
There was a problem hiding this comment.
In buildPiCompatEnv(), child-home mode only sets HOME, so Windows subprocesses that resolve the home directory via USERPROFILE/HOMEDRIVE/HOMEPATH still point at the real user profile. Under OMP_PI_COMPAT_BRIDGE=child-home, this leaves %USERPROFILE%\.pi unisolated and defeats the bridge for Windows plugin processes.
Useful? React with 👍 / 👎.
|
Addressed the latest two Codex comments in
Added regression coverage for both cases. Verification:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 94c7c26e6b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (!manifest[key]?.length && conventional[key]?.length) { | ||
| manifest[key] = conventional[key]; |
There was a problem hiding this comment.
applyConventionalFallbacks() currently treats an explicit empty array the same as a missing key because it checks !manifest[key]?.length. That means a plugin that intentionally sets pi.skills: [] (or prompts/themes/extensions) will still get those directories auto-added if they exist on disk, enabling resources the manifest explicitly disabled. This changes capability loading behavior unexpectedly; fallback should run only when the key is absent, not when it is present-but-empty.
Useful? React with 👍 / 👎.
| const valueToCheck = isWindowsDrivePath(spec) ? spec.replace(/\\/g, "/") : spec; | ||
| if (SHELL_METACHARS.test(valueToCheck)) { |
There was a problem hiding this comment.
Windows-style relative local specs (for example .\plugin or ..\plugin) are rejected as unsafe because backslashes are only normalized for drive-letter paths before the metacharacter check. In parsePiInstallSource(), this throws Invalid characters in package source before local-path resolution, so local Pi installs on Windows require non-native path rewriting. Treat Windows relative separators as local path syntax before applying the shell-metacharacter denylist.
Useful? React with 👍 / 👎.
|
Addressed the latest Codex comments in
Added regression coverage for both cases. Verification:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c2c0f83342
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const runtime = new ExtensionRuntime(); | ||
| if (paths.length > 0) { | ||
| await ensurePiCompatImportShims(); | ||
| await activatePiCompatEnvironment(); |
There was a problem hiding this comment.
loadExtensions enables Pi compatibility by calling activatePiCompatEnvironment() without a package name, so profile mode cannot inject package-specific variables from getPiCompatProfileEnv (for example PI_TEAMS_ROOT_DIR for @tmustier/pi-agent-teams). In a normal runtime (separate from install), extensions that rely on profile-provided env can start without those variables, so --compat-pi profile behavior silently degrades for affected packages.
Useful? React with 👍 / 👎.
| const changed = Object.entries(deps).filter(([name, version]) => previousDeps[name] !== version); | ||
| if (changed.length === 1) return changed[0][0]; | ||
| if (fallbackName) { | ||
| const actualName = extractPackageName(fallbackName); |
There was a problem hiding this comment.
#resolveInstalledPackageName() assumes either exactly one dependency entry changed or that fallbackName maps directly to node_modules/<name>. For git installs where repo basename differs from package.json.name, reinstalling the same spec leaves dependencies unchanged (changed.length === 0) and the fallback lookup misses, causing a throw even though the package is already installed. This breaks idempotent plugin install git:... flows for those repositories.
Useful? React with 👍 / 👎.
|
Checked the latest Codex comments for conflicts/loops: they are new edge cases on top of the existing compatibility work, not contradictory with the prior fixes. Addressed both in
Added regression coverage for profile env during plugin extension load. Verification:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 70fdd5fe02
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (paths.length > 0) { | ||
| await ensurePiCompatImportShims(); | ||
| await activatePiCompatEnvironment(); |
There was a problem hiding this comment.
loadExtensions now unconditionally runs Pi compat setup before loading any module, but both helpers perform filesystem writes under the OMP config directory and can throw on read-only or restricted environments. Because these awaits are not guarded, one shim/home setup failure aborts loadExtensions() and prevents all extensions (including non-Pi extensions) from loading instead of returning per-extension errors. This should be best-effort or limited to paths that actually need Pi compatibility.
Useful? React with 👍 / 👎.
| if (pathsWithSources.length > 0) { | ||
| await ensurePiCompatImportShims(); | ||
| await activatePiCompatEnvironment(); |
There was a problem hiding this comment.
loadCustomTools now blocks on Pi shim/environment setup whenever any tool path exists, and those setup calls can throw if ~/.omp is not writable. Since this happens before tool modules are loaded and without error isolation, a bootstrap failure prevents every custom tool (including ordinary non-Pi tools) from loading. Handle setup failures as warnings or restrict this path to Pi-compat tool sources so unrelated tools still work.
Useful? React with 👍 / 👎.
# Conflicts: # packages/coding-agent/CHANGELOG.md
|
Merged latest GitHub now reports:
Verification:
Note: I also tried |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 87fc6d7f1c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| { | ||
| name: "typebox", | ||
| files: { | ||
| "index.ts": typeboxShimContent(), |
There was a problem hiding this comment.
ensurePiCompatImportShims() always rewrites node_modules/typebox because this shim entry lacks skipIfPackageExists, unlike the adjacent @sinclair/typebox shim. In any environment where a plugin installs a real typebox dependency, loading extensions/tools will silently replace that package with the compat shim and can break runtime imports or behavior for that plugin.
Useful? React with 👍 / 👎.
|
Fantastic idea! I've been missing being able to use the Pi extensions since moving over to OMP. |
|
@can1357 can you review this please |
# Conflicts: # docs/plugin-manager-installer-plumbing.md # packages/coding-agent/CHANGELOG.md
|
Resolved the merge conflicts with latest Conflicts resolved:
Local verification before pushing:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08536910b6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| } else if (arg === "-l" || arg === "--local") { | ||
| } else if (arg === "--compat-pi") { | ||
| result.flags.compatPi = true; | ||
| result.flags.local = true; |
There was a problem hiding this comment.
The new flag parsing block leaves the -l/--local branch empty and sets result.flags.local = true under --compat-pi, so consumers of parsePluginArgs can no longer request local mode with -l alone while --compat-pi now unexpectedly forces local scope. In contexts that still use this parser, that changes which plugin registry is targeted (project-local vs user/global) and can run install/doctor operations against the wrong location.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 01fcdd7: -l/--local now sets flags.local = true, while --compat-pi only sets flags.compatPi = true.
Also added regression coverage for --local, -l, --compat-pi, and combined --local --compat-pi parsing.
Verification:
bunx biome check packages/coding-agent/src/cli/plugin-cli.ts packages/coding-agent/test/plugin-command.test.ts— passgit diff --check— pass
Note: local bun --cwd=packages/coding-agent test test/plugin-command.test.ts is blocked in this fresh checkout because the native addon is not built, and local native build cannot run because cargo is not installed on this machine.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 01fcdd7d5a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| ) { | ||
| if (pathsWithSources.length > 0) { | ||
| await ensurePiCompatImportShims(); | ||
| await activatePiCompatEnvironment(); |
There was a problem hiding this comment.
loadCustomTools enables Pi compatibility globally but never passes a package context, so profile-specific env injection is skipped for plugin tools. buildPiCompatEnv only applies profile vars via getPiCompatProfileEnv(options.packageName), which means tools from profiled packages (for example @tmustier/pi-agent-teams) can load/execute without required vars like PI_TEAMS_ROOT_DIR, breaking runtime behavior that depends on profile mode.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7e61ca2: plugin custom tools now derive their installed package name from the managed plugin node_modules path, activate Pi compat with that packageName before importing the tool module, and use a per-plugin CustomToolAPI whose exec() env includes package profile vars.
Added regression coverage using an @tmustier/pi-agent-teams plugin tool to verify both load-time process.env.PI_TEAMS_ROOT_DIR and api.exec() child env receive the profile value.
Verification:
bunx biome check packages/coding-agent/src/extensibility/custom-tools/loader.ts packages/coding-agent/test/pi-compat.test.ts— passbun --cwd=packages/coding-agent run check:types— passgit diff --check— pass
Note: local bun --cwd=packages/coding-agent test test/pi-compat.test.ts is still blocked in this checkout by missing pi_natives.darwin-arm64.node; building it locally is blocked because cargo is not installed. GitHub CI should run the full test job.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e61ca2361
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| for (const [key, value] of Object.entries(env)) { | ||
| process.env[key] = value; |
There was a problem hiding this comment.
activatePiCompatEnvironment() mutates process.env by only assigning keys from the newly built compat env, but it never removes profile-specific variables from previous activations. If a profiled package (for example one that sets PI_TEAMS_ROOT_DIR) is activated first, then a different package is loaded afterward, stale profile vars remain in the global process environment and can be inherited by later extension/tool subprocesses, causing them to run with the wrong package context.
Useful? React with 👍 / 👎.
| exec(command: string, args: string[], options?: ExecOptions) { | ||
| return execCommand(command, args, options?.cwd ?? this.cwd, options); | ||
| return execCommand(command, args, options?.cwd ?? this.cwd, { | ||
| ...options, | ||
| env: buildPiCompatEnv({ baseEnv: { ...process.env, ...options?.env }, packageName: this.packageName }), | ||
| }); |
There was a problem hiding this comment.
ConcreteExtensionAPI.exec() now always wraps subprocesses with buildPiCompatEnv(...), even when the extension is not from a Pi-compatible plugin (packageName is undefined). This changes subprocess semantics for all extensions by prepending the Pi shim path and setting Pi compatibility vars, so non-Pi/user extensions can unexpectedly resolve pi to the shim and observe compatibility-only env state they never requested.
Useful? React with 👍 / 👎.
2 participants
What
Adds a Pi compatibility layer for OMP plugin packages:
omp picommand group fordoctor,install,shim, and path bridge planning/creationnpm:,git:, GitHub URL, and local package specspackage.json.piand conventional Pi resource normalization for extensions, skills, prompts, and themes@mariozechner/*andtypeboxpackage namespiCLI shim and safe.pibridge modes for legacy packages@tmustier/pi-agent-teams,pi-teams, andpi-messengerWhy
Allows OMP to install, diagnose, and load upstream Pi-style packages without renaming OMP globally or writing to a real
~/.piby default.Fixes #433.
Testing
bun run checkbun --cwd=packages/coding-agent test test/plugin-command.test.ts test/plugin-extensions-discovery.test.ts test/pi-compat.test.tsbun packages/coding-agent/src/cli.ts pi doctor npm:pi-teams@0.9.14 --jsonHOME=/tmp/omp-pi-compat-sandbox/homewith a fake Pi plugin coveringpackage.json.pi,@mariozechner/*import aliases,pisubprocess detection,~/.pidetection, resource discovery, shim creation, and symlink bridge dry-run/createbun checkpasses