Adds support for codex

[ssheftel]

This makes the skill usable with codex

[harshal2802]

Review — PR #39: Adds support for codex

Thanks for putting this together, Sam! Codex support is a great addition. I've gone through the diff and have a few thoughts — mostly around scope and consistency. Happy to discuss any of these if you see it differently.

---

1. Formatting changes mixed in with the Codex work

There are quite a few changes in the README that aren't directly related to Codex — em-dashes replaced with hyphens, Unicode arrows swapped for ASCII, tree characters changed, etc. They're all reasonable on their own, but bundled together they make the diff harder to review. Would you be open to splitting those into a separate PR so this one stays focused on the Codex additions?

2. <repository-url> placeholder vs. the real URL

I see the reasoning behind making the install docs fork-safe, and I appreciate the thought. My concern is that replacing the real URL with <repository-url> makes the README less copy-pasteable for most users. There's also an inconsistency now — the Quick Install section still references the actual repo via the plugin marketplace command. Maybe we could keep the real URL in the main instructions and add a small note for fork users? Open to other ideas here too.

3. Learn More section — removed links to existing docs

The Learn More section went from three links down to one, but docs/efficiency-tips.md and docs/migration.md still exist in the repo. Removing the links without removing the files means those docs become harder to discover. Would it make sense to either restore the links or remove the files in the same PR?

4. consistency.sh doesn't reflect the new checklist item

The CLAUDE.md checklist now includes a check for agents/openai.yaml matching SKILL.md, which is great — but tests/consistency.sh doesn't have a corresponding automated check yet. It'd be nice to keep those in sync so CI catches the same things the manual checklist does. Happy to help with that if useful.

5. SKILL.md path reference in consistency.sh

This might predate your PR, but line 53 of tests/consistency.sh references skills/pdd/SKILL.md, while this PR (and the README/CLAUDE.md changes) treat SKILL.md as living at the repo root. Worth a quick check to make sure the test is pointing at the right place.

6. Minor — SKILL.md wording change

The "Claude" → "the assistant" change makes sense for vendor-neutrality. Just flagging it since the PR description doesn't mention it — might be worth a quick note so reviewers know it's intentional.

---

What I like

• The agents/openai.yaml is clean and minimal — nice job keeping it simple.
• Adding the Codex checklist item to CLAUDE.md is the right call.
• The Codex installation section in the README reads well.

Overall this is heading in a good direction. Let me know what you think about the suggestions above — happy to chat through any of them!

[harshal2802]

Review — PR #39: Adds support for codex

Thanks for putting this together, Sam! Codex support is a great addition. I've gone through the diff and have a few thoughts — mostly around scope and consistency. Happy to discuss any of these if you see it differently.

1. Formatting changes mixed in with the Codex work

There are quite a few changes in the README that aren't directly related to Codex — em-dashes replaced with hyphens, Unicode arrows swapped for ASCII, tree characters changed, etc. They're all reasonable on their own, but bundled together they make the diff harder to review. Would you be open to splitting those into a separate PR so this one stays focused on the Codex additions?

2. <repository-url> placeholder vs. the real URL

I see the reasoning behind making the install docs fork-safe, and I appreciate the thought. My concern is that replacing the real URL with <repository-url> makes the README less copy-pasteable for most users. There's also an inconsistency now — the Quick Install section still references the actual repo via the plugin marketplace command. Maybe we could keep the real URL in the main instructions and add a small note for fork users? Open to other ideas here too.

3. Learn More section — removed links to existing docs

The Learn More section went from three links down to one, but docs/efficiency-tips.md and docs/migration.md still exist in the repo. Removing the links without removing the files means those docs become harder to discover. Would it make sense to either restore the links or remove the files in the same PR?

4. consistency.sh doesn't reflect the new checklist item

The CLAUDE.md checklist now includes a check for agents/openai.yaml matching SKILL.md, which is great — but tests/consistency.sh doesn't have a corresponding automated check yet. It'd be nice to keep those in sync so CI catches the same things the manual checklist does. Happy to help with that if useful.

5. SKILL.md path reference in consistency.sh

This might predate your PR, but line 53 of tests/consistency.sh references skills/pdd/SKILL.md, while this PR (and the README/CLAUDE.md changes) treat SKILL.md as living at the repo root. Worth a quick check to make sure the test is pointing at the right place.

6. Minor — SKILL.md wording change

The "Claude" → "the assistant" change makes sense for vendor-neutrality. Just flagging it since the PR description doesn't mention it — might be worth a quick note so reviewers know it's intentional.

What I like

• The agents/openai.yaml is clean and minimal — nice job keeping it simple.
• Adding the Codex checklist item to CLAUDE.md is the right call.
• The Codex installation section in the README reads well.

Overall this is heading in a good direction. Let me know what you think about the suggestions above — happy to chat through any of them!

this is opus generated review. lets chat and find out a way to resolve the conflicts