fix(sync): dedupe agent files sharing a name across .md and .agent.md - #471
Conversation
Plugins ship two files for one logical agent: a portable <name>.md (root agents/) and a GitHub Copilot native <name>.agent.md (.github/agents/). Both land in the same agentsPath directory for copilot/vscode because each copy phase matches on the destination filename, not on the agent's identity, so Copilot Chat builds that still load plain .md files there show the agent twice. Match by frontmatter name (not filename), keep .agent.md, remove the .md twin, and track the survivor in sync-state so future purges don't orphan it. Reported in sync messages. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 824b26bf65
ℹ️ 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".
|
|
||
| for (const file of group.plainMdFiles) { | ||
| if (!dryRun) { | ||
| await unlink(join(dir, file)); |
There was a problem hiding this comment.
When syncing any client with an agentsPath, this scan includes every existing file in that directory and unconditionally deletes matching plain .md files without checking copy results or prior ownership. For example, a Copilot workspace containing user-created foo.md and foo.agent.md with the same frontmatter name loses foo.md even when no plugin supplies either file; the survivor is then recorded as synced and can be deleted by the next selective purge. Only deduplicate variants known to have been produced or managed by AllAgents.
AGENTS.md reference: AGENTS.md:L7-L11
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 80f738b: dedupeAgentFilesByName no longer scans the whole destination directory. It now only considers filenames that the given plugins' own agents/ and .github/agents/ source directories ship this sync (same exclude/fileArtifacts gating as copyAgents/copyGitHubContent), reading frontmatter from those source files — never from an arbitrary file already sitting in the destination. Added a regression test ("never touches a file the given plugins do not ship, even if it collides on disk").
| const agentDedupeRecords = await dedupeAgentFilesForClients( | ||
| workspacePath, | ||
| syncClients, | ||
| resolvedMappings, | ||
| dryRun, |
There was a problem hiding this comment.
On a fresh workspace, dry-run plugin copies return planned results without materializing the destination files, but this call asks the dedupe helper to inspect only the current filesystem. A plugin providing both agents/foo.md and .github/agents/foo.agent.md therefore produces no Would dedupe message during --dry-run, while the equivalent real sync creates both and removes the plain file. Model the planned copies when calculating dry-run deduplication so the reported operations match the real command.
AGENTS.md reference: AGENTS.md:L7-L10
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 80f738b: frontmatter is now always read from the plugin's own source file (agents/.md or .github/agents/.agent.md), not the destination directory. Dry-run and a real sync now compute identical dedupe records regardless of whether the destination file has been materialized yet. Added a regression test ("reports what a fresh sync would dedupe even before any destination file exists").
Addresses Codex review on EntityProcess#471: - P1: dedupeAgentFilesByName scanned the entire destination agentsPath directory, so a user-created (or otherwise unmanaged) .md/.agent.md pair with matching frontmatter name would get deleted even though no plugin provided it, and the survivor would then be tracked as synced and eligible for a future selective purge. It now only considers filenames that the given plugins' own agents/ and .github/agents/ source directories actually ship this sync (respecting each plugin's exclude patterns and fileArtifacts gating, same as copyAgents/ copyGitHubContent), reading frontmatter from those source files. - P2: dry-run previously read frontmatter from the destination directory, which dry-run copies never materialize, so a fresh workspace's dry-run reported no dedup even though the following real sync would immediately remove a file. Reading from plugin sources instead means dry-run and a real sync now compute identical records. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Requesting changes based on reproduced file-copy sync regressions.
Terminology used in the inline comments:
| Term | Meaning here |
|---|---|
| Portable agent definition | agents/<name>.md, copied by copyAgents |
| GitHub-format agent definition | .github/agents/<name>.agent.md, copied by copyGitHubContent for Copilot |
Two blocking regressions are introduced by this PR: mixed Claude/Copilot sync deletes Claude's valid .md definition, and a failed GitHub-format file copy still causes the successfully copied portable definition to be deleted. I also found a narrower state-ownership gap after a plugin removes its portable duplicate, false deletion output during the path migration, and a non-blocking cross-plugin collision edge case. Please address the blocking findings and add syncWorkspace regression coverage; the ownership guarantee should be fixed or narrowed. For cross-plugin destination conflicts, use deterministic configuration order—first configured plugin wins, matching the existing MCP precedent—and use that same owner for copying and dedupe.
2 participants
Source
Reported here (internal, cited for reference only): https://teams.microsoft.com/l/message/19:d29d75b3931b4302b9bd9d968d55adcf@thread.skype/1787648455643?tenantId=8b493985-e1b4-4b95-ade6-98acafdbdb01&groupId=188ba793-8136-4b10-b984-10097d7bb40c&parentMessageId=1787648455643&teamName=Development%20Customs%20Team&channelName=Customs%20Specifications%20(Specs-as-Code)&createdTime=1787648455643
Problem
A synced workspace's
.github/agents/directory can contain two files for the same logical agent, for examplecw-reviewer.mdandcw-reviewer.agent.md, both declaringname: cw-reviewer.This happens when a plugin ships both:
agents/<name>.mddefinition; and.github/agents/<name>.agent.mddefinition.For Copilot these routes converge on
.github/agents/. VS Code versions that discover both Markdown files show the logical agent twice. The GitHub-format.agent.mdfile was also not tracked individually because.githubcopying previously returned one aggregate result.Fix
.md, while Copilot/compatible VS Code routes can receive both formats..githubcopy..mdand a GitHub-route.agent.mdwith the same frontmatternamein the same physical directory..mdto.agent.mdreplacement as one logical agent during deletion reporting; later removal reportsreviewer, notreviewer.agent.Scope
This dedupes representations within the same Copilot/VS Code agent directory. It does not collapse definitions across
.claude/agents/and.github/agents/; those are distinct client destinations.Verification
Automated:
bun run buildbun run typecheckbun run lintbun test --timeout 10000— 1527 passed, 6 skipped, 0 failedBuilt-CLI smoke test:
The fixture used one local plugin with
agents/reviewer.mdand.github/agents/reviewer.agent.md, targeting Claude and Copilot. Verified:.claude/agents/reviewer.mdremains present and tracked..github/agents/reviewer.mdis removed after successful dedupe..github/agents/reviewer.agent.mdremains present and tracked.Deleted: agent 'reviewer'.