prep codex plugin for official directory submission#304
Conversation
|
Warning Review limit reached
Next review available in: 9 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe Codex plugin manifest now includes expanded metadata and a skills directory. Runtime commands and hooks use ChangesCodex plugin updates
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant SessionStart
participant autoUpdate
participant InstallDetector
participant UpdateProcess
SessionStart->>autoUpdate: pass Codex bundleDir
autoUpdate->>InstallDetector: detect managed plugin-cache path
InstallDetector-->>autoUpdate: managed or unmanaged result
autoUpdate->>UpdateProcess: spawn update for unmanaged installs
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@harnesses/codex/.codex-plugin/plugin.json`:
- Around line 3-11: Reconcile the manifest schema mismatch between plugin.json
and the expectations in codex-hooks.test.ts. Update the manifest to use the
expected array-based skills field and restore the required mcpServers and apps
fields, or consistently update the test and validation if the new schema is
intentional; ensure all contract checks pass.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f49c54ad-7b55-4f00-9e35-2bd3e18ce29a
📒 Files selected for processing (2)
harnesses/codex/.codex-plugin/plugin.jsonharnesses/codex/SUBMISSION.md
Codex injects PLUGIN_ROOT/CLAUDE_PLUGIN_ROOT into plugin hook commands, never CODEX_PLUGIN_ROOT. A directory (Codex-managed) install runs the static harness files verbatim, so the old $CODEX_PLUGIN_ROOT expanded to an empty string and every hook/command hit /bundle/... -> ENOENT. The npm installer bakes absolute paths and was unaffected, which masked this.
Coverage ReportScope: files changed in this PR. Enforced threshold: 90% per metric (per file via
File Coverage — 2 files changed
Generated for commit 6b7db25. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/codex/codex-hooks.test.ts (1)
60-68: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer parsing JSON and asserting on specific command fields over raw substring matching.
The test reads
hooks.jsonas raw text and usestoContain/not.toContainfor substring checks. Per path instructions, tests should assert on specific values rather than generic substrings. Parsing the JSON and iterating over each hook'scommandfield would be more precise — it verifies that every command actually starts with the expected prefix and avoids false positives from non-command text.As per coding guidelines,
tests/**files should prefer asserting on specific values (paths, messages) over generic substrings.♻️ Suggested refactor: parse JSON and assert on each command field
it("hook commands reference $PLUGIN_ROOT, never the unset $CODEX_PLUGIN_ROOT", () => { - // Codex injects PLUGIN_ROOT / CLAUDE_PLUGIN_ROOT into hook commands, NOT - // CODEX_PLUGIN_ROOT. A directory (Codex-managed) install runs this file - // verbatim, so referencing the unset var would expand to `/bundle/...` - // and every hook would ENOENT silently. Our npm installer bakes absolute - // paths and is unaffected — this guard protects the directory channel. - const raw = readFileSync(join(codexRoot, "hooks", "hooks.json"), "utf-8"); - expect(raw).not.toContain("CODEX_PLUGIN_ROOT"); - expect(raw).toContain("$PLUGIN_ROOT/bundle/"); + const raw = readFileSync(join(codexRoot, "hooks", "hooks.json"), "utf-8"); + const hooks = JSON.parse(raw).hooks; + + const commands: string[] = []; + for (const event of Object.values(hooks)) { + for (const entry of event as any[]) { + for (const hook of entry.hooks) { + commands.push(hook.command); + } + } + } + + expect(commands.length).toBeGreaterThan(0); + for (const cmd of commands) { + expect(cmd).not.toContain("CODEX_PLUGIN_ROOT"); + expect(cmd).toContain("$PLUGIN_ROOT/bundle/"); + } });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/codex/codex-hooks.test.ts` around lines 60 - 68, Refactor the test “hook commands reference $PLUGIN_ROOT...” to parse hooks.json with JSON.parse, extract every hook command field, and assert each command starts with “$PLUGIN_ROOT/bundle/” and contains no “CODEX_PLUGIN_ROOT”. Replace raw readFileSync substring assertions with these field-specific checks.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/codex/codex-hooks.test.ts`:
- Around line 60-68: Refactor the test “hook commands reference $PLUGIN_ROOT...”
to parse hooks.json with JSON.parse, extract every hook command field, and
assert each command starts with “$PLUGIN_ROOT/bundle/” and contains no
“CODEX_PLUGIN_ROOT”. Replace raw readFileSync substring assertions with these
field-specific checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3d146882-37d1-4d9e-99d6-3c709fb1b59f
📒 Files selected for processing (4)
harnesses/codex/commands/login.mdharnesses/codex/hooks/hooks.jsonharnesses/codex/skills/deeplake-memory/SKILL.mdtests/codex/codex-hooks.test.ts
✅ Files skipped from review due to trivial changes (3)
- harnesses/codex/hooks/hooks.json
- harnesses/codex/commands/login.md
- harnesses/codex/skills/deeplake-memory/SKILL.md
A directory (marketplace) install lives under ~/.codex/plugins/cache/... and is updated by Codex via snapshot refresh. Running our npm 'hivemind update' there fights Codex's file management. Guard autoUpdate on the running bundleDir: skip ONLY on positive ID of the managed cache path; every other layout (our ~/.codex/hivemind/bundle npm install, dev checkout, unknown) still updates. Conservative by design — a false 'managed' verdict would silently freeze updates for npm users, so the default is always 'update'.
…, false-skip regression)
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/codex/codex-session-start-setup-hook.test.ts`:
- Around line 170-178: Strengthen the assertions in the “invokes autoUpdate
exactly once…” test by replacing the generic type and non-empty checks for
opts.bundleDir with an assertion that it ends with the expected “hooks/codex”
path suffix, matching the directory derived from session-start-setup.ts while
remaining environment-independent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5c569975-427e-49fa-aa21-f417512490a9
📒 Files selected for processing (4)
src/hooks/codex/session-start-setup.tssrc/hooks/shared/autoupdate.tstests/claude-code/autoupdate.test.tstests/codex/codex-session-start-setup-hook.test.ts
1 participant
What
Makes the Codex plugin actually installable from the official OpenAI plugin directory (platform.openai.com/plugins) and prepares the submission. The plugin previously worked only via our
npmchannel (hivemind install), which masked several directory-channel blockers.Fixes (in order)
plugin.jsonhad"skills": [], so a directory install exported zero skills. Now"skills": "./skills/"(official schema is a path, not an array); dropped emptymcpServers/apps; addedauthor/repository/keywords.scripts/sync-versions.mjssynced every manifest exceptharnesses/codex/.codex-plugin/plugin.json(why it was stale at 0.6.7). Added it to the sync list; now trackspackage.json(0.7.123).$PLUGIN_ROOT(directory-channel blocker) — Codex injectsPLUGIN_ROOT/CLAUDE_PLUGIN_ROOTinto hook commands, neverCODEX_PLUGIN_ROOT. The static harness files used$CODEX_PLUGIN_ROOT→ on a directory install every hook expanded to/bundle/...→ ENOENT, silently dead. Swapped to$PLUGIN_ROOTinhooks.json,SKILL.md,login.md. The npm installer bakes absolute paths and was unaffected, which hid this.hivemind updatemust not run there.autoUpdatenow skips only on positive ID of the managed cache path (~/.codex/plugins/cache/...); our npm path, dev checkouts, and unknown layouts still update. Conservative by design: a false "managed" verdict would silently freeze updates for npm users.Tests
tests/codex/codex-hooks.test.ts— official manifest schema (skills path, no mcpServers/apps) + a guard rejecting any return ofCODEX_PLUGIN_ROOT.tests/claude-code/autoupdate.test.ts— managed-vs-npm detection incl. the explicit false-skip regression guard (npm path MUST update), macOS/Windows/CODEX_HOMEvariants, and the real on-disk paths from a live machine.tests/codex/codex-session-start-setup-hook.test.ts— assertsbundleDiris threaded toautoUpdate.tscOK, build OK, 263/263 across autoupdate + all codex + cursor session-start suites.Not in this PR (login/asset-bound — owner action)
Directory submission itself needs: OpenAI org identity verification + Apps Management: Write, listing URLs (support/privacy/terms) + logo, and the 5 positive / 3 negative test cases uploaded from
harnesses/codex/SUBMISSION.md.Session Context
Session transcript