Add Codex wrappers for Claude skills#1551
Conversation
meenchen
requested review from
chadvoegele,
cjluo-nv,
kaix-nv and
mxinO
and removed request for
chadvoegele and
kaix-nv
Signed-off-by: weimingc <17592131+meenchen@users.noreply.github.com>
meenchen
force-pushed
the
add-codex-claude-skill-wrappers
branch
from
72cbf54 to
9aa040c
Compare
📝 WalkthroughWalkthroughThis PR refactors skill documentation to use relative parent paths ( ChangesDocumentation Path Refactoring and Deployment Validation
🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
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 @.claude/skills/deployment/SKILL.md:
- Line 177: The "Check container registry auth" preflight line currently
requires credential verification for any SLURM container-image job; update the
text in SKILL.md so it only mandates auth checks for private or
access-restricted registries (or else add wording that public images do not
require auth), e.g., change the instruction to scope verification to images
hosted on private registries and reference ../common/slurm-setup.md section 6
for how to confirm auth, or explicitly document the rationale if you intend
stricter enforcement; ensure the step's title/phrase "Check container registry
auth" and its sentence about not submitting until auth is confirmed are adjusted
accordingly.
🪄 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: Enterprise
Run ID: 0c51caf2-68f7-4887-9a9b-e86c66737080
📒 Files selected for processing (4)
.claude/skills/deployment/SKILL.md.claude/skills/evaluation/SKILL.md.claude/skills/ptq/SKILL.md.codex/skills
| If a cluster config exists (`~/.config/modelopt/clusters.yaml` or `.claude/clusters.yaml`), or the user mentions running on a remote machine: | ||
|
|
||
| 0. **Check container registry auth** — before submitting any SLURM job with a container image, verify credentials exist on the cluster per `skills/common/slurm-setup.md` section 6. If credentials are missing for the image's registry, ask the user to fix auth or switch to an image on an authenticated registry (e.g., NGC). **Do not submit until auth is confirmed.** | ||
| 0. **Check container registry auth** — before submitting any SLURM job with a container image, verify credentials exist on the cluster per `../common/slurm-setup.md` section 6. If credentials are missing for the image's registry, ask the user to fix auth or switch to an image on an authenticated registry (e.g., NGC). **Do not submit until auth is confirmed.** |
There was a problem hiding this comment.
Align container-auth preflight policy with private/public image logic.
This step currently requires credential verification for any SLURM container-image job, which conflicts with the evaluation skill’s private-only check and can block valid public-image submissions. Please scope the preflight to private/access-restricted registries (or explicitly document why deployment must be stricter).
🤖 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 @.claude/skills/deployment/SKILL.md at line 177, The "Check container
registry auth" preflight line currently requires credential verification for any
SLURM container-image job; update the text in SKILL.md so it only mandates auth
checks for private or access-restricted registries (or else add wording that
public images do not require auth), e.g., change the instruction to scope
verification to images hosted on private registries and reference
../common/slurm-setup.md section 6 for how to confirm auth, or explicitly
document the rationale if you intend stricter enforcement; ensure the step's
title/phrase "Check container registry auth" and its sentence about not
submitting until auth is confirmed are adjusted accordingly.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1551 +/- ##
=======================================
Coverage 76.70% 76.70%
=======================================
Files 477 477
Lines 51977 51977
=======================================
+ Hits 39868 39869 +1
+ Misses 12109 12108 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I remember we have a PR for general agent support, renaming |
|
Related PR: #1362 |
3 participants
What does this PR do?
Type of change: enable skills for codex
Claude reads the skill at:
.claude/skills/ptq/SKILL.md
Codex sees it through:
.codex/skills/ptq/SKILL.md -> .claude/skills/ptq/SKILL.md
The old skill text referenced shared files as skills/common/.... That path only makes
sense to Claude if it treats .claude as a special root. From Codex’s symlinked view,
skills/common/... would not resolve cleanly.
So I changed references like:
skills/common/environment-setup.md
to:
../common/environment-setup.md
That is path-relative from the skill directory, so it works in both views:
I did not change the workflow content, just made internal references portable.
Usage
# Add a code snippet demonstrating how to use thisTesting
Run codex and verify skills are correctly loaded
Before your PR is "Ready for review"
Make sure you read and follow Contributor guidelines and your commits are signed (
git commit -s -S).Make sure you read and follow the Security Best Practices (e.g. avoiding hardcoded
trust_remote_code=True,torch.load(..., weights_only=False),pickle, etc.).CONTRIBUTING.md: ✅ / ❌ / N/AAdditional Information
Summary by CodeRabbit
Documentation
Chores