fix: add omp manifest field so package.json isn't silently skipped - #87
Merged
Merged
Conversation
Discovered live-testing PR #86's shipped install path on a second pass: plain `omp plugin link .omp-plugin` followed by an ordinary `omp` session (no --plugin-dir override) never fired any throughline hook - the earlier "live verified" testing in #85/#86 had all gone through --plugin-dir, which bypasses the enabled-plugins resolution entirely and was masking this. Root cause, confirmed by reading extensibility/plugins/loader.ts's collectPluginsAtRoot(): 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 - a plain `if (!manifest) continue`. `omp plugin doctor`'s "No omp/pi manifest (not an omp plugin)" warning is that exact gate, not the cosmetic label it was assumed to be. 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 the prompt and a bash tool call in the shared buffer. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes a real gap found while re-testing PR #86's shipped install path: a plain
omp plugin link .omp-pluginfollowed by an ordinaryompsession (no--plugin-diroverride) 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): a linked/installed plugin is silently excluded fromgetEnabledPlugins()- and therefore from every hooks/skills discovery surface - unless itspackage.jsonhas anomporpifield (if (!manifest) continue).omp plugin doctor's "No omp/pi manifest (not an omp plugin)" warning is that exact functional gate, not the cosmetic label the original PR's memory notes assumed.Fix
Added a minimal
omp: { name, description }field to.omp-plugin/package.json.Verification
omp plugin doctornow shows the plugin healthy (no manifest warning).omp --printsession (no CLI override), against a real model, correctly captured both a prompt and a bash tool call in the shared buffer.local-ci --strict: all checks pass.🤖 Generated with Claude Code