Skip to content

feat(#85): add OMP (Oh My Pi) as a 4th delivery format - #86

Merged
jsirish merged 2 commits into
mainfrom
claude/throughline-omp-compat-105ad7
Sep 19, 2026
Merged

jsirish merged 2 commits into
mainfrom
claude/throughline-omp-compat-105ad7

Conversation

@jsirish

@jsirish jsirish commented Sep 19, 2026

Copy link
Copy Markdown
Member

Summary

  • Adds .omp-plugin/ as a 4th throughline delivery format, targeting OMP (Oh My Pi) - a TypeScript coding-agent harness with its own plugin/hook/skill/MCP system.
  • Follows the Codex reuse pattern, not OpenCode's full port: OMP's hooks are in-process pi.on() event handlers, not subprocess hooks, so hooks/post/throughline.ts is a thin shim that translates OMP's own event shapes into the JSON the existing hooks/*.sh scripts already jq-parse, then shells out to them. No capture/redaction logic is duplicated in TypeScript.
  • Skills are auto-discovered via .omp-plugin/skills, a symlink to the shared skills/ directory - same pattern as .omp-plugin/hooks -> ../hooks. Zero extra files, zero drift risk.
  • Install: git clone + omp plugin link .omp-plugin (a live checkout, not a package install - git pull to update).

How this differs from the initial plan

The plan (session notes, not committed) assumed OMP's hook API surface based on docs alone. Installing the real package locally (npm install @oh-my-pi/pi-coding-agent) and reading its actual TypeScript source corrected several assumptions:

  • Hooks discovered under hooks/pre|post/ are bound through OMP's Extensions runtime (ExtensionAPI), not the separate, unused-at-runtime HookAPI module the docs and initial research implied.
  • pi.exec() has no stdin option, so the shim calls node:child_process directly.
  • omp plugin link <path> (not omp plugin install <git-url>) is the mechanism that installs a subdirectory of a monorepo - this is what makes .omp-plugin/ viable at all without a standalone npm package.
  • OMP's own tool schemas (bash/write/grep use command/path/pattern, confirmed against src/tools/*.ts) needed translating into Claude Code's tool-shape (Bash/Edit/Write/Grep with tool_input.*) for session-capture.sh to parse correctly.

What's verified vs. still open

Verified (against real OMP source + a bun-based integration test suite that fires synthetic events at the real shim and asserts on real hooks/*.sh output, not a mocked child_process):

  • All 6 registered events fire the right hook script with the right JSON shape.
  • session_start sends the onboarding markdown via pi.sendMessage.
  • tool_result correctly captures bash/edit/write/grep/mcp__*, skips read/glob (matching Claude Code's own exclusion), and drops unrecognized non-MCP tools rather than guessing a shape.
  • session_before_compact/session_compact stamp the compaction boundary and re-inject the buffer tail (OMP splits Claude Code's single re-fired SessionStart(source=compact) into two events).
  • session_shutdown stamps the buffer as ended.
  • All 186 existing shell hook tests still pass - no regression to the Claude Code/Codex path.

Still open, tracked in #85 (needs a real OMP session, not just static/simulated verification):

  • Exact field names for web_search/task tool inputs (best-effort mapping in place; falls back to empty fields rather than crashing if wrong).
  • session_shutdown reliability on a clean exit (/quit/EOF) vs. only SIGINT/SIGTERM.
  • Confirming pi.sendMessage() at session_start actually reaches the model on turn 1, not just the TUI.

I don't have a configured OMP+model setup in this environment to complete that last-mile verification myself - #85 stays open for it.

Test plan

  • bun test in .omp-plugin/ - 9/9 pass (integration tests against the real shim + real hook scripts)
  • bun run typecheck in .omp-plugin/ - clean
  • sh tests/run.sh - 186/186 pass, no regression
  • local-ci.sh --strict - all checks PASS (shellcheck, manifest validation, plugin version agreement, hook tests, typography, opencode plugin, omp plugin)
  • Live verification against a real OMP session (see "still open" above) - needs a maintainer with OMP + a configured model to complete

🤖 Generated with Claude Code

jsirish and others added 2 commits September 19, 2026 15:13
.omp-plugin/ mirrors the Codex reuse pattern rather than OpenCode's full
TypeScript port: OMP's hooks are in-process pi.on() event handlers, so
hooks/post/throughline.ts is a thin shim that translates OMP's own event
shapes into the JSON the existing hooks/*.sh scripts already jq-parse, then
shells out to them. No capture/redaction logic is duplicated.

Skills are auto-discovered via .omp-plugin/skills, a symlink to the shared
skills/ directory - zero extra step, same as .omp-plugin/hooks -> ../hooks.

Verified against OMP's actual TypeScript source (installed locally via
`npm install @oh-my-pi/pi-coding-agent`), not just its docs: the real
pi.on() event names/payload shapes, the omp-plugins/claude-plugins discovery
providers, and the omp plugin link mechanism for installing a monorepo
subdirectory. A bun-based integration test suite (.omp-plugin/) fires
synthetic OMP events at the real shim and asserts on the resulting
hooks/*.sh output, rather than mocking child_process - the same principle
tests/run.sh already applies to the Claude Code hooks, and the layer that
would have caught the two silent bugs .opencode-plugin's full port shipped.

Live verification against a real OMP session (tool_result field-shape
confirmation for web_search/task, session_shutdown reliability on a clean
exit, sendMessage reaching the model on turn 1) is still open - tracked in
issue #85, not closed by this commit.

Also updates CI (.github/workflows/ci.yml, .local-ci.json) with a 4th
manifest in the version-agreement check and a new omp-plugin job, and
documents OMP as a delivery format in README.md, docs/INSTALL.md, and
docs/index.html.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Review of PR #86 (pr-review-toolkit:code-reviewer) found:

- Critical: child.stdin had no 'error' listener. Every hooks/*.sh script
  guard-clause exits 0 before reading stdin when inactive/disabled/no-jq,
  and a payload large enough to exceed the pipe buffer then hit EPIPE on
  the write - an uncaught error event that killed the host OMP process.
  Reproduced live on Bun 1.3.8 with a 50KB prompt under
  THROUGHLINE_DISABLE=1. Fixed with a no-op error listener before the
  write; added a regression test with a 5MB prompt under the same
  condition.
- Important: the task branch's `description ?? input.prompt` used `??`,
  which only falls through on null/undefined - an empty-string description
  suppressed the prompt fallback and silently dropped the delegated intent.
  session-capture.sh's own jq filter guards against exactly this
  (empty-aware `select`), so the shim needs the same `||` semantics. Fixed;
  added a regression test.
- Also wrapped every registered handler in a swallow-and-continue wrapper
  (matching hooks/*.sh's own "always exit 0, never block" contract - an
  unexpected event shape from a future OMP version must not crash the
  session either), hardened the mcp__ prefix check against a
  missing/non-string toolName, and corrected a comment overclaiming that
  the `edit` tool's field names were confirmed against source (only
  bash/write/grep were).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@jsirish

jsirish commented Sep 19, 2026

Copy link
Copy Markdown
Member Author

Live-verified against a real OMP session (brew-installed `omp` 18.2.6, Jason's Dynamic LLM gateway model). All previously-open items from #85 now confirmed working except web_search/task field names (untested, low risk, degrades gracefully):

  • `session_start` -> `sendMessage()` reaches the model on turn 1 (model referenced the real injected HANDOFF/buffer content in its first reply)
  • `before_agent_start` prompt capture
  • `tool_result` capture for bash/grep with correct field extraction
  • `session_shutdown` fires on a clean `--print`-mode exit
  • Skills auto-discovery (all 4 skills visible to the model)
  • Git-worktree data-dir sharing resolves correctly to the main checkout

Details in #85. Ready to merge from my side.

@jsirish
jsirish merged commit 8253925 into main Sep 19, 2026
5 checks passed
@jsirish
jsirish deleted the claude/throughline-omp-compat-105ad7 branch September 19, 2026 20:53
jsirish added a commit that referenced this pull request Sep 19, 2026
…egression

Code review on this PR found that the existing CI/local-ci manifest checks
only assert `.name and .version` on .omp-plugin/package.json - they stayed
green through both #85 and #86 while the plugin silently had zero working
hooks/skills for exactly the reason this PR fixes. Extend both checks to
require `.omp` as well, so dropping that field again fails CI instead of
shipping silently broken.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
jsirish added a commit that referenced this pull request Sep 19, 2026
A plain omp plugin link .omp-plugin followed by an ordinary omp session
(no --plugin-dir override) never fired any throughline hook. Every "live
verified" test in #85/#86 had gone through --plugin-dir, which bypasses
OMP's enabled-plugins resolution entirely and was masking this.

Root cause (extensibility/plugins/loader.ts::collectPluginsAtRoot in
@oh-my-pi/pi-coding-agent): a linked/installed plugin is silently excluded
from getEnabledPlugins() - and therefore from every hooks/skills discovery
surface - unless its package.json has an omp or pi field. omp plugin
doctor's "No omp/pi manifest (not an omp plugin)" warning is that exact
functional gate, not cosmetic.

Added a minimal omp: { name, description } field. Re-verified live: a
plain omp --print session (no CLI override) against a real model now
correctly captures a prompt and a bash tool call in the shared buffer.

Also hardens the CI/local-ci manifest-validation checks to require the
omp key (they previously only checked name/version and would have stayed
green through this exact regression).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.

1 participant