Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approach Review: Approach looks good. Adding copilot as a new entry in the existing HARNESS_REGISTRY is the natural extension point — the registry pattern was designed for exactly this kind of addition. No alternative approach to suggest.
Approach review by Claude (opus)
There was a problem hiding this comment.
Approach Review: approach looks good. Modeling Copilot as a first-class harness is consistent with the existing registry-based design for tool-specific global targets, and Copilot CLI’s user-level instructions use a distinct official path (~/.copilot/copilot-instructions.md), so this does not look like unnecessary complexity or a missed simpler abstraction.
Approach review by Codex CLI (gpt-5.4)
Jercik
left a comment
Jercik
left a comment
There was a problem hiding this comment.
Approach Review: approach looks good
Approach review by Copilot (gpt-5.4)
| gemini: { target: "~/.gemini/AGENTS.md" }, | ||
| opencode: { target: "~/.config/opencode/AGENTS.md" }, | ||
| codex: { target: "~/.codex/AGENTS.md" }, | ||
| copilot: { target: "~/.copilot/copilot-instructions.md" }, |
There was a problem hiding this comment.
ℹ️ Info: Verified that ~/.copilot/copilot-instructions.md is the correct path for Copilot CLI user-level instructions per GitHub docs. Good.
|
|
||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| vi.mocked(fsPromises.lstat).mockRejectedValue( |
There was a problem hiding this comment.
ℹ️ Info: This beforeEach lstat mock reset is a good defensive addition. vi.clearAllMocks() clears call history but technically preserves mock implementations — however, re-establishing the default explicitly here makes the test setup self-documenting and guards against any mockResolvedValueOnce queue leakage between tests. Nice housekeeping bundled with this PR.
| const paths = actionsArgument.map((action) => action.path); | ||
| expect(paths.some((p) => p.endsWith("/.claude/CLAUDE.md"))).toBe(true); | ||
| expect(paths.some((p) => p.endsWith("/.codex/AGENTS.md"))).toBe(true); | ||
| expect( |
There was a problem hiding this comment.
ℹ️ Info: Good addition. The test now verifies that all five harnesses (including copilot) receive write actions when global patterns are configured. This catches any future regression where a new harness is added to the registry but accidentally excluded from the sync loop.
|
🎉 This PR is included in version 5.11.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 56a488cbec
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| gemini: { target: "~/.gemini/AGENTS.md" }, | ||
| opencode: { target: "~/.config/opencode/AGENTS.md" }, | ||
| codex: { target: "~/.codex/AGENTS.md" }, | ||
| copilot: { target: "~/.copilot/copilot-instructions.md" }, |
There was a problem hiding this comment.
The new Copilot harness hard-codes ~/.copilot/copilot-instructions.md, so global sync writes to the wrong location whenever users run Copilot CLI with a non-default config directory (for example by setting COPILOT_HOME, which GitHub’s CLI docs describe as overriding ~/.copilot). In that setup, synced instructions are silently ignored by Copilot CLI even though sync reports the Copilot target was handled.
Useful? React with 👍 / 👎.
1 participant
Summary
copilotas a first-class harness forglobalOverrides~/.copilot/copilot-instructions.mdWhy
Copilot support needed to be modeled explicitly instead of relying on implicit compatibility. The implementation now follows the Copilot CLI instruction loading documented in the adjacent research docs.
Validation
pnpm testpnpm typecheckpnpm lint