fix(agent): expose Greenpill review skill to Codex#15
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a Codex Greenpill production-readiness review skill, its display metadata, and tests verifying file discovery, content, naming, canonical references, and separation from the built-in ChangesGreenpill review skill
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Oba-One
added
automated/codex
ChatGPT Codex Connector
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@scripts/repo-skill-discovery.test.ts`:
- Around line 17-19: Update the regular expressions in the test around the skill
content assertions to tolerate optional carriage returns before line feeds:
include \r? in both the frontmatter name match and the prohibited name check.
Preserve the existing multiline anchoring and assertion behavior.
- Around line 1-6: Update the rootDir initialization near the imports to use
fileURLToPath from node:url on import.meta.url before passing it to resolve,
replacing direct .pathname access while preserving the existing repository-root
resolution behavior.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e819996d-2d2b-4a81-aaed-ec3f3dc54450
📒 Files selected for processing (3)
.agents/skills/greenpill-review/SKILL.md.agents/skills/greenpill-review/agents/openai.yamlscripts/repo-skill-discovery.test.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Private map/member-node intake must never expose emails, raw notes, IP addresses, user agents, spam metadata, steward review notes, pending submissions, or raw upstream EAS/Green Goods work and media feedback.
Files:
scripts/repo-skill-discovery.test.ts
**/*.{ts,tsx,astro}
📄 CodeRabbit inference engine (AGENTS.md)
Keep implementation within the smallest applicable package boundary and do not expand beyond issue acceptance criteria.
Files:
scripts/repo-skill-discovery.test.ts
🪛 SkillSpector (2.3.11)
.agents/skills/greenpill-review/SKILL.md
[warning] 11: [AS3] Skill Enumeration: Skill enumerates or reads other installed skills. Access to other skills' SKILL.md files or the skills directory reveals prompt instructions, capabilities, and secrets that should be invisible to peer skills.
Remediation: Remove all code or instructions that list or read other skills' files or directories. Skills should operate independently; cross-skill access is a privilege escalation.
(Agent Snooping (AS3))
[warning] 11: [AS3] Skill Enumeration: Skill enumerates or reads other installed skills. Access to other skills' SKILL.md files or the skills directory reveals prompt instructions, capabilities, and secrets that should be invisible to peer skills.
Remediation: Remove all code or instructions that list or read other skills' files or directories. Skills should operate independently; cross-skill access is a privilege escalation.
(Agent Snooping (AS3))
🔇 Additional comments (2)
.agents/skills/greenpill-review/SKILL.md (1)
1-23: LGTM!.agents/skills/greenpill-review/agents/openai.yaml (1)
1-5: LGTM!
| import assert from 'node:assert/strict'; | ||
| import { access, readFile } from 'node:fs/promises'; | ||
| import { resolve } from 'node:path'; | ||
| import test from 'node:test'; | ||
|
|
||
| const rootDir = resolve(new URL('..', import.meta.url).pathname); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use fileURLToPath to support paths with spaces and Windows drives.
Extracting .pathname from import.meta.url leaves URI-encoded characters (like %20 for spaces) unresolved and retains leading slashes on Windows (e.g., /C:/). This will cause file resolution to fail if the repository is cloned into a directory with spaces. Use fileURLToPath for robust cross-platform compatibility.
💻 Proposed fix
import assert from 'node:assert/strict';
import { access, readFile } from 'node:fs/promises';
import { resolve } from 'node:path';
import test from 'node:test';
+import { fileURLToPath } from 'node:url';
-const rootDir = resolve(new URL('..', import.meta.url).pathname);
+const rootDir = fileURLToPath(new URL('..', import.meta.url));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import assert from 'node:assert/strict'; | |
| import { access, readFile } from 'node:fs/promises'; | |
| import { resolve } from 'node:path'; | |
| import test from 'node:test'; | |
| const rootDir = resolve(new URL('..', import.meta.url).pathname); | |
| import assert from 'node:assert/strict'; | |
| import { access, readFile } from 'node:fs/promises'; | |
| import { resolve } from 'node:path'; | |
| import test from 'node:test'; | |
| import { fileURLToPath } from 'node:url'; | |
| const rootDir = fileURLToPath(new URL('..', import.meta.url)); |
🤖 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 `@scripts/repo-skill-discovery.test.ts` around lines 1 - 6, Update the rootDir
initialization near the imports to use fileURLToPath from node:url on
import.meta.url before passing it to resolve, replacing direct .pathname access
while preserving the existing repository-root resolution behavior.
| const skill = await readFile(codexSkillPath, 'utf8'); | ||
| assert.match(skill, /^---\nname: greenpill-review\n/m); | ||
| assert.doesNotMatch(skill, /^name: review$/m); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make regex matching robust against Windows CRLF line endings.
If the repository is checked out with CRLF line endings on Windows, the hardcoded \n will fail to match lines ending with \r\n. Additionally, $ in JavaScript's /m mode matches before \n, meaning ^name: review$ would incorrectly fail to match name: review\r, leading to a false positive test for assert.doesNotMatch. Account for optional carriage returns with \r?.
💻 Proposed fix
const skill = await readFile(codexSkillPath, 'utf8');
- assert.match(skill, /^---\nname: greenpill-review\n/m);
- assert.doesNotMatch(skill, /^name: review$/m);
+ assert.match(skill, /^---\r?\nname: greenpill-review\r?\n/m);
+ assert.doesNotMatch(skill, /^name: review\r?$/m);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const skill = await readFile(codexSkillPath, 'utf8'); | |
| assert.match(skill, /^---\nname: greenpill-review\n/m); | |
| assert.doesNotMatch(skill, /^name: review$/m); | |
| const skill = await readFile(codexSkillPath, 'utf8'); | |
| assert.match(skill, /^---\r?\nname: greenpill-review\r?\n/m); | |
| assert.doesNotMatch(skill, /^name: review\r?$/m); |
🤖 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 `@scripts/repo-skill-discovery.test.ts` around lines 17 - 19, Update the
regular expressions in the test around the skill content assertions to tolerate
optional carriage returns before line feeds: include \r? in both the frontmatter
name match and the prohibited name check. Preserve the existing multiline
anchoring and assertion behavior.
1 participant
Summary
greenpill-reviewname so Codex's built-in/reviewcommand is not shadowedCodex discovers repository skills under
.agents/skills; the existing workflow remains canonical under.claude/skills/reviewand the new entrypoint delegates to it without duplication.References:
Validation
maingreenpill-reviewand does not collide with built-inreviewThe desktop sandbox protects the exact local
.agentspath, so this scoped branch was created directly from syncedmainthrough GitHub rather than through a worktree.