feat(omp): add native oh-my-pi harness support - #1323
Conversation
- detect omp (~/.omp, project .omp/) so `--to all` reports it as a native plugin install instead of attempting conversion; no --to omp converter, writer, or manifest (per CONCEPTS.md native-plugin-surface) - docs/specs/omp.md target spec verified against omp 17.2.7; README gains omp install/local-dev/limitations coverage - skills: omp built-in `ask` in blocking-question lists, `task` in closed subagent enumerations, and `/skill:<name>` user-facing invocation form across the rendering-rule copies, AGENTS.md, and the authoring guide - ce-compound session history: discover omp sessions under ~/.omp/agent/sessions and named-profile roots (honoring PI_CODING_AGENT_SESSION_DIR/PI_CODING_AGENT_DIR/PI_CONFIG_DIR), detect omp JSONL via its physical title-slot line before the pi-shaped header, and reuse pi extraction for omp platform identity - tests: detection, --to all note, session fixture + discovery/extract coverage, invocation-rendering parity, and a guarded omp-native-install smoke test Closes EveryInc#1224
![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: 8bf0cb1a91
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- deduplicate all-platform discovery output: Pi and omp share the PI_CODING_AGENT_SESSION_DIR flat override, so both discoverers emitted the same files (Codex review P2) - glob omp's sanitized bucket basename ([^a-zA-Z0-9._-]+ -> "-", edge dashes stripped, capped at the last 80 chars, empty -> "project", mirroring session-paths.ts getDefaultSessionDirName) so repos whose basename omp normalizes (e.g. spaces) are still discovered - SKILL.md: include omp sessions in the mandatory keyword rerun clause — omp session headers carry cwd but no git branch, same as Codex/Pi
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd24249c82
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…pport # Conflicts: # skills/ce-doc-review/references/walkthrough.md
Address PR EveryInc#1323 review feedback by updating the historian prompt's platform enum, cwd metadata, cross-tool synthesis guidance, and provenance header. Add a contract regression guard.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a7794e6b21
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
omp 17.2.9 restored the legacy raw bucket naming (-<home-rel>, -tmp-<rel>, --<abs>--) and removed automatic migration (#7646), so both raw and hashed bucket shapes exist in the wild. Discovery globbed only the sanitized basename, missing raw buckets whose basename contains characters the hashed scheme normalizes (e.g. "my repo" in --Users-test-Code-my repo--). Add an exact raw-bucket probe for --cwd (canonicalized via physical paths like omp's resolveEquivalentPath), a raw-basename glob alongside the sanitized one, and awk dedup for buckets hit by both probe and glob. Update docs/specs/omp.md for the restored naming scheme.
|
Thanks for this. The boundary you picked (no converter, no writer, no Before reviewing file by file, I went and read the omp source to check the install and update semantics, and I found one thing that changes what I want the README to recommend. Our marketplace catalog has no I have written up the full findings and a proposal in #1224 and asked @lustrousgorilla some questions there, since it affects the shape of the install story rather than any single line in this diff: #1224 (comment) Short version of what would change here:
The Let's settle the questions in #1224 first, then I will do a proper review pass here. |
# Conflicts: # skills/ce-commit/SKILL.md
Split out per maintainer request (EveryInc#1224): omp's session-bucket naming is still moving (17.2.9 restored the raw scheme), so the install and prose work here should not be blocked behind it.
omp's update checker skips catalog entries without a plugin-entry version, so CE was invisible to omp plugin upgrade, the 24h catalog refresh, and marketplace.autoUpdate. Ship .omp-plugin/marketplace.json (omp's preferred catalog path, read ahead of the Claude copy when both exist) with a version wired to the root release component via extra-files jsonpath $.plugins[0].version, mirroring the existing kimi/grok/devin native-catalog pattern. release:validate gains an omp arm: plugin-list parity with the Claude catalog, version presence and drift detection, and a co-located source check. README now leads with the marketplace flow plus autoUpdate guidance (notify mode only writes to the debug log), demoting omp install <git-url> to pin-style and omp plugin link to contributor dev. The target spec's causal claim is corrected: skills load via provider scanning of the root skills/ directory; the pi manifest is the discovery gate and an install-time validation risk, not the loader.
|
Reworked per the #1224 discussion (#1224 (comment)):
Full suite green: 2907 pass, 0 fail; |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d21b20c4c7
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ".cursor-plugin/marketplace.json", | ||
| ".grok-plugin/marketplace.json" | ||
| ".grok-plugin/marketplace.json", | ||
| ".omp-plugin/marketplace.json" |
There was a problem hiding this comment.
When a future PR changes only the omp catalog (for example, fixing the marketplace source or update metadata), release-please will ignore it for the root component because this path is excluded and there is no separate .omp-plugin package; that means the root extra-file version at $.plugins[0].version will not bump, so existing omp marketplace installs keep seeing the old plugin-entry version and will not receive the catalog-only fix. Keep this catalog in a releasable component, or add a dedicated component, rather than excluding it.
Useful? React with 👍 / 👎.
I said in my reply "Let's settle the questions in #1224 first, then I will do a proper review pass here." :) . Is your agent just working autonomously? |
2 participants
Summary
Implements the boundary settled in #1224 (#1224 (comment)): native oh-my-pi (
omp) support with no converter, no writer, and no separate release component. omp installs this repository natively; the gaps were CE's missing omp marketplace catalog (without one, omp's update checker never sees CE) and CE's own harness-specific instructions. This PR closes both.The
ce-compoundsession-history work was split into #1333 per maintainer request, so this install/prose work is not blocked behind omp's still-moving session-bucket naming.Closes #1224.
What changed
omp marketplace catalog — the auto-update fix
.omp-plugin/marketplace.json, omp's preferred catalog path (read ahead of.claude-plugin/marketplace.jsonwhen both exist; verified empirically on 17.2.9 — discovery output and the cached catalog both come from the omp copy, Claude Code is unaffected).version, wired to the root release component in.github/release-please-config.jsonviaextra-files(jsonpath: "$.plugins[0].version"), with the file added to rootexclude-pathslike the other native catalogs. Without this field omp'scheckForUpdates()skips CE permanently: reproduced empirically — version-less catalog + changed plugin →omp plugin upgradereports "up to date"; with the field, the same change upgrades0.0.1 -> 0.0.2into a new version-keyed cache dir and repoints thenode_modulessymlink.release:validategains an omp arm (src/release/metadata.ts): plugin-list parity with the Claude catalog, version presence (missing = structural error, since it disables upgrade detection) and drift detection (release-please owns the write; detect-only here), and a co-located"./"source check. Mirrored arms intests/release-metadata.test.ts.CLI — detection, not conversion
src/utils/detect-tools.ts: detection-onlyompentry (~/.omp, project.omp/), same class as copilot/droid/qwen.--to allprints- omp — native plugin install; skippedinstead of silently ignoring the host.install/convert"no installable tools" guidance names oh-my-pi (omp).--to omprejects as an unknown target, exactly like the other native-install-only hosts.Docs
docs/specs/omp.mdtarget spec, re-verified 2026-08-05 against omp 17.2.9. Corrects the original causal claim per maintainer review: skills load through omp's provider scan of the rootskills/directory — the bundled.pi/extensions/compound-engineering.tsis a no-op on omp (resources_discoverhas noAgentSessioncallsites); thepimanifest is the discovery gate and an install-time validation risk (install rolls back if anextensionsentry fails to import), not the skill loader. Documents the update mechanics (catalogversion→checkForUpdates()→marketplace.autoUpdate), both session-bucket schemes, and the profile/env-override chain.README.md: the omp subsection now leads with the marketplace flow plusomp config set marketplace.autoUpdate auto(the defaultnotifymode only writes to the debug log — verified), demotingomp install <git-url>to pin-style andomp plugin link "$PWD"to contributor dev in a collapsed block.Skill prose — the compatibility pass
ask(adjacent to the Piask_useritem; the negative pin ince-code-reviewincludes it too).ce-workexecution-engines matrix,ce-plan,ce-simplify-code) now name omp's built-intask. Open "or the equivalent" phrasings were deliberately left alone — they already cover omp./skill:<name>(not/skill-name, not$skill-name). The centralized rule inAGENTS.md, the authoring guide, README's invocation note, and all per-skill rendering-rule copies carry the new clause; the parity tests were generalized from a two-form to a three-form contract.Tests
tests/cli.test.ts:--to allprints the omp native-skip line;--to omprejects like other native-only targets.tests/detect-tools.test.ts: tool count 7 → 8; omp detection asserted.tests/release-metadata.test.ts: omp catalog fixture + four arm tests (missing catalog, version drift detect-only, missing version error, plugin-list parity).tests/omp-native-install.test.ts: guarded on the omp binary — dry-run install recognizes this repo (omp install --dry-run --json) and everyskills/<name>/SKILL.mdcarries thename+descriptionfrontmatter omp requires for discovery. Skips cleanly when omp is absent.Verification
bun run test(full suite, same as CI): 2907 pass, 0 fail on the committed tree.bun run release:validate: in sync (0 agents, 32 skills, 0 MCP servers — counts unchanged), including the new omp arm.HOME): dual-catalog precedence, version-less upgrade invisibility, and clean0.0.1 -> 0.0.2upgrade with"source": "./"all confirmed — reported in Proposal: add native oh-my-pi (omp) support #1224 (comment).omp config get marketplace.autoUpdate→notify(default);omp config set marketplace.autoUpdate autoconfirmed working.Deliberately not done (per the #1224 boundary)
--to ompconverter or pi-writer reuse; no separate release component (the catalog version rides the root component'sextra-files, same as the kimi/grok/devin manifests).Security Disclosure
No security-relevant changes. The new
.omp-plugin/marketplace.jsonis static metadata; therelease:validatearm only reads it. No shell, path-handling, permission, network, credential, or dependency changes beyond what the detection-only CLI entry already did (unchanged in this revision).Agent Disclosure
Oh My Pi (omp) · kimi-code/k3Research, empirical verification (sandboxed marketplace install/upgrade runs against omp 17.2.9), decomposition, and implementation by the agent; earlier slices of this PR were executed by delegated subagents under the same harness, and their diffs were reviewed before commit.