Avoid duplicate HEY skills in Codex - #385
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The full setup wizard can short-circuit before removing a managed legacy Codex copy, leaving the duplicate integration in place.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Moves Codex integration to the shared Agent Skills path and safely migrates managed legacy copies.
Changes:
- Uses
~/.agents/skills/heyfor Codex setup and health checks. - Removes managed legacy Codex copies during installation or refresh.
- Updates tests and documentation for the shared-path behavior.
[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or rungh pr ready --undo.
Click "Ready for review" or rungh pr readyto reengage.
File summaries
| File | Description |
|---|---|
README.md |
Documents Codex shared-skill discovery and migration. |
AGENTS.md |
Updates repository integration guidance. |
internal/harness/harness.go |
Adds the canonical shared skill path. |
internal/harness/codex.go |
Moves Codex health checks to the shared path. |
internal/harness/codex_test.go |
Tests shared-path health behavior. |
internal/cmd/skill_refresh.go |
Removes managed legacy copies during refresh. |
internal/cmd/skill_refresh_test.go |
Covers refresh migration and preservation rules. |
internal/cmd/skill_install.go |
Stops Codex-specific copying and adds migration. |
internal/cmd/skill_install_test.go |
Tests installation and legacy-copy safety. |
internal/cmd/setup_test.go |
Updates wizard behavior coverage. |
internal/cmd/setup_agents_test.go |
Updates agent setup and removal coverage. |
internal/cmd/setup_agents_remove.go |
Removes the legacy path during uninstall. |
internal/cmd/setup_agent.go |
Connects Codex through the shared skill. |
Review details
- Files reviewed: 13/13 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 13 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Contributor
Author
|
Addressed all current review findings in 0cfb6ae:
Added regression coverage for each path. Verified with |
jeremy
force-pushed
the
fix/codex-skill-duplicate
branch
from
September 9, 2026 23:15
0cfb6ae to
56c73fa
Compare
Member
|
Rebased onto main and pushed to the PR branch ( One conflict, in README.md: main rewrote the README and moved the "AI agent integration" section verbatim into docs/agents.md, so the README took main's version and this PR's two hunks (the |
jeremy
approved these changes
Sep 10, 2026
jeremy
enabled auto-merge (squash)
September 10, 2026 19:07
jeremy
pushed a commit
to shawnyeager/hey-cli
that referenced
this pull request
Sep 10, 2026
Register Grok in the agent harness, add hey setup grok, and include Grok in skill install, doctor, setup agents, and --remove. No Grok plugin yet. Grok, like Codex since basecamp#385, discovers the shared ~/.agents/skills/hey skill directly (xAI's docs list ~/.agents/skills/ among the user-level skill locations), so hey setup grok installs the shared skill and confirms it is healthy rather than copying it into $GROK_HOME/skills. Detection uses $GROK_HOME or ~/.grok, or a grok binary on PATH, ~/.local/bin, or $GROK_HOME/bin, where Grok Build's installers put it.
jeremy
added a commit
to shawnyeager/hey-cli
that referenced
this pull request
Sep 10, 2026
Grok arrived as a copy of Codex: a second harness file and a second block in every command that touches a shared-skill agent (setup handlers, hey skill install, refresh, --remove, the HEY_SETUP_AGENT values), differing only in name, id, home env var, home directory and binary. Two copies of one shape drift — the binary-only detection test existed for Grok but not for Codex, the ~/<home>/bin lookup existed for Grok but not for Codex, and the legacy-copy migration basecamp#385 added existed for Codex but not for Grok. Say it once. harness.SkillAgent carries those five fields; Codex and Grok are two values in a table that init() registers, and Detect, Home, FindBinary, LegacySkillPath and CheckSkill are its methods. The cmd package loops over harness.SkillAgents() wherever it used to name each agent, so a third shared-skill agent is a new row and the prose lists in the help topic, installers and docs — nothing else. AgentInfo gains FindBinary so setup's "binary not found" remediation reads the registry rather than a switch on ids. FindBinary looks on PATH, then ~/.local/bin, then the agent's own home's bin for every row: Grok Build's installers write ~/.grok/bin/grok ($GROK_HOME/bin/grok for the npm package), and a uniform rule costs Codex one stat. Legacy migration is uniform too: a hey-cli-marked copy in any row's own skills directory is a duplicate the agent would list twice, and only hey-cli could have written a marked one, so doctor flags it, and setup, hey skill install and refresh remove it once the shared skill is healthy, for Grok as for Codex. The twin tests fold the same way: one table-driven test per behavior in harness and cmd, run once per row, so a behavior one agent has and the other lacks is a failing test rather than a gap.
jeremy
added a commit
to shawnyeager/hey-cli
that referenced
this pull request
Sep 10, 2026
Grok arrived as a copy of Codex: a second harness file and a second block in every command that touches a shared-skill agent (setup handlers, hey skill install, refresh, --remove, the HEY_SETUP_AGENT values), differing only in name, id, home env var, home directory and binary. Two copies of one shape drift — the binary-only detection test existed for Grok but not for Codex, the ~/<home>/bin lookup existed for Grok but not for Codex, and the legacy-copy migration basecamp#385 added existed for Codex but not for Grok. Say it once. harness.SkillAgent carries those five fields; Codex and Grok are two values in a table that init() registers, and Detect, Home, FindBinary, LegacySkillPath and CheckSkill are its methods. The cmd package loops over harness.SkillAgents() wherever it used to name each agent, so a third shared-skill agent is a new row and the prose lists in the help topic, installers and docs — nothing else. AgentInfo gains FindBinary so setup's "binary not found" remediation reads the registry rather than a switch on ids. FindBinary looks on PATH, then ~/.local/bin, then the agent's own home's bin for every row: Grok Build's installers write ~/.grok/bin/grok ($GROK_HOME/bin/grok for the npm package), and a uniform rule costs Codex one stat. Legacy migration is uniform too: a hey-cli-marked copy in any row's own skills directory is a duplicate the agent would list twice, and only hey-cli could have written a marked one, so doctor flags it, and setup, hey skill install and refresh remove it once the shared skill is healthy, for Grok as for Codex. The twin tests fold the same way: one table-driven test per behavior in harness and cmd, run once per row, so a behavior one agent has and the other lacks is a failing test rather than a gap.
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
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.
Closes #287.
Problem
Current Codex discovers personal skills from
~/.agents/skills, so copying the same HEY skill into~/.codex/skillscreates two indistinguishable entries.Change
The existing
hey setup agents --removepath remains the supported uninstall flow. This PR does not change skill invocation policy; HEY remains available for normal implicit selection.Verification
TMPDIR=/tmp go test ./internal/...TMPDIR=/tmp go test ./internal/cmd ./internal/harness ./skillsgo vet ./...The short
TMPDIRavoids unrelated Unix-domain socket path-length failures in macOS TUI tests.Summary by cubic
Stops Codex from seeing duplicate HEY skills by making the shared
~/.agents/skillslocation the single source. Setup,hey skill install, health checks, and version refresh no longer copy the skill into~/.codex/skills; Codex discovers the shared skill directly.Migration
~/.codex/skills/heycopy only during explicit setup or once-per-version refresh, and only when the ownership marker proves hey-cli created it and the shared replacement is healthy.CODEX_HOMEaliases~/.agents, so the shared skill is never deleted as its own legacy copy.hey setup codex.hey setup agents --removeremains the supported uninstall flow.Written for commit 56c73fa. Summary will update on new commits.