diff --git a/README.md b/README.md index 5e481ff..fc34337 100644 --- a/README.md +++ b/README.md @@ -125,7 +125,7 @@ my-plugin/ ├── commands/ # Slash commands (Claude, OpenCode) ├── hooks/ # Lifecycle hooks (Claude, Factory, Copilot, Codex) ├── .codex-plugin/ # Codex plugin manifest and explicit hook paths -├── .github/ # Copilot/VSCode overrides +├── .github/ # Copilot/VSCode project overrides └── .mcp.json # MCP server configs ``` @@ -135,6 +135,12 @@ that file. Codex plugins can declare hooks in `.codex-plugin/plugin.json` with a `hooks` path, path array, inline object, or inline object array; otherwise AllAgents falls back to `hooks/hooks.json`. +For Copilot, root `hooks/` can sync to either project or user hook directories. +Repository hooks under `.github/hooks/` remain project-scoped and are never +promoted into the user-global `~/.copilot/hooks/` directory. Potential copies +from older versions are left untouched and reported for manual review because +their ownership was not tracked. + ## Documentation Full documentation at [allagents.dev](https://allagents.dev): diff --git a/docs/src/content/docs/docs/guides/plugins.mdx b/docs/src/content/docs/docs/guides/plugins.mdx index f4f9c54..729eda5 100644 --- a/docs/src/content/docs/docs/guides/plugins.mdx +++ b/docs/src/content/docs/docs/guides/plugins.mdx @@ -13,7 +13,7 @@ my-plugin/ ├── agents/ # Agent definitions (Claude, Copilot, Factory) ├── hooks/ # Hook definitions (Claude, Copilot, Factory) ├── commands/ # Commands (Claude, OpenCode) -├── .github/ # GitHub overrides (Copilot, VSCode) +├── .github/ # Project-scoped GitHub overrides (Copilot, VSCode) │ ├── copilot-instructions.md │ ├── instructions/ # Pattern-based instructions │ ├── prompts/ # Prompt files @@ -22,6 +22,13 @@ my-plugin/ └── AGENTS.md ``` +:::note +At user scope, root `hooks/` entries sync to the client's user hook directory. +Repository hooks under `.github/hooks/` stay project-scoped and are not copied +to `~/.copilot/hooks/`. Potential copies from older versions are left untouched +and reported for manual review because their ownership was not tracked. +::: + ## Duplicate Skill Handling When multiple plugins define skills with the same folder name, AllAgents automatically resolves naming conflicts: diff --git a/docs/src/content/docs/docs/reference/clients.mdx b/docs/src/content/docs/docs/reference/clients.mdx index 62df43e..78d1172 100644 --- a/docs/src/content/docs/docs/reference/clients.mdx +++ b/docs/src/content/docs/docs/reference/clients.mdx @@ -43,7 +43,7 @@ These clients use their own skills directory: | Kiro | `.kiro/skills/` | `AGENTS.md` | No | No | :::note -Skills are the cross-client way to share reusable prompts. GitHub overrides (`.github/prompts/`, `.github/agents/`, `.github/hooks/`, `copilot-instructions.md`) are copied to the workspace's `.github/` folder for Copilot/VSCode. Root `agents/` and `hooks/` also map to `.github/agents/` and `.github/hooks/` for Copilot. +Skills are the cross-client way to share reusable prompts. GitHub overrides (`.github/prompts/`, `.github/agents/`, `.github/hooks/`, `copilot-instructions.md`) are copied to the workspace's `.github/` folder for Copilot/VSCode. Root `agents/` and `hooks/` also map to `.github/agents/` and `.github/hooks/` for Copilot. At user scope, root `hooks/` maps to `~/.copilot/hooks/`, while repository `.github/hooks/` remains project-scoped. ::: ### VSCode diff --git a/src/core/sync.ts b/src/core/sync.ts index bbf2d19..8bda624 100644 --- a/src/core/sync.ts +++ b/src/core/sync.ts @@ -39,6 +39,7 @@ import { copyWorkspaceFiles, collectPluginSkills, type CopyResult, + findRelocatedGitHubHooks, } from './transform.js'; import { updateAgentFiles } from './workspace-repo.js'; import { @@ -2579,6 +2580,29 @@ export async function syncUserWorkspace( await sw.measure('selective-purge', () => selectivePurgeWorkspace(homeDir, previousState, syncClients), ); + + const relocatedHooks = await sw.measure( + 'legacy-copilot-hook-scan', + () => + findRelocatedGitHubHooks( + validPlugins + .filter((plugin) => plugin.clients.includes('copilot')) + .map((plugin) => ({ + pluginPath: plugin.resolved, + ...(plugin.exclude && { exclude: plugin.exclude }), + })), + homeDir, + 'copilot', + { clientMappings: USER_CLIENT_MAPPINGS }, + ), + ); + + for (const filePath of relocatedHooks.found) { + const displayPath = relative(homeDir, filePath).replace(/\\/g, '/'); + warnings.push( + `Copilot user hook '${displayPath}' shares a path with a repository .github/hooks artifact. Repository hooks are no longer synced at user scope; review this file manually if an older AllAgents version installed it. A root hooks/ artifact may still manage the same path.`, + ); + } } // Two-pass skill name resolution (excluding disabled/non-enabled skills) diff --git a/src/core/transform.ts b/src/core/transform.ts index 39bb373..fd3fd7e 100644 --- a/src/core/transform.ts +++ b/src/core/transform.ts @@ -1,5 +1,12 @@ -import { existsSync } from 'node:fs'; -import { cp, mkdir, readFile, readdir, writeFile } from 'node:fs/promises'; +import { existsSync, type Dirent } from 'node:fs'; +import { + access, + cp, + mkdir, + readFile, + readdir, + writeFile, +} from 'node:fs/promises'; import { basename, dirname, join, relative } from 'node:path'; import micromatch from 'micromatch'; import { @@ -730,6 +737,107 @@ export interface GitHubCopyOptions extends CopyOptions { skillNameMap?: Map; } +function relocatesGitHubContent(mapping: ClientMapping): boolean { + return mapping.githubPath !== '.github/'; +} + +function githubContentExcludes( + mapping: ClientMapping, + exclude?: string[], +): string[] | undefined { + if (!relocatesGitHubContent(mapping)) return exclude; + + // .github/hooks is repository-owned. Root hooks/ remains the portable + // plugin artifact that can be installed into a client's user hook path. + return [...(exclude ?? []), '.github/hooks']; +} + +export interface RelocatedGitHubHooksSource { + pluginPath: string; + exclude?: string[]; +} + +export interface RelocatedGitHubHooksResult { + found: string[]; +} + +function isNotFoundError(error: unknown): boolean { + return ( + typeof error === 'object' && + error !== null && + 'code' in error && + error.code === 'ENOENT' + ); +} + +/** + * Find files that older versions may have mirrored from repository + * .github/hooks into a relocated user directory. Historical sync state did not + * record ownership of these files, so callers must leave them in place and + * report them for manual review. + */ +export async function findRelocatedGitHubHooks( + sources: RelocatedGitHubHooksSource[], + workspacePath: string, + client: ClientType, + options: Pick = {}, +): Promise { + const emptyResult: RelocatedGitHubHooksResult = { found: [] }; + const mapping = getMapping(client, options); + if ( + options.dryRun || + !mapping.githubPath || + !relocatesGitHubContent(mapping) + ) { + return emptyResult; + } + + const destDir = join(workspacePath, mapping.githubPath, 'hooks'); + const candidates = new Set(); + await Promise.all( + sources.map(async ({ pluginPath, exclude }) => { + const sourceRoot = join(pluginPath, '.github', 'hooks'); + + async function collect(sourceDir: string): Promise { + let entries: Dirent[]; + try { + entries = await readdir(sourceDir, { withFileTypes: true }); + } catch (error) { + if (isNotFoundError(error)) return; + throw error; + } + + await Promise.all( + entries.map(async (entry) => { + const sourcePath = join(sourceDir, entry.name); + if (isExcluded(pluginPath, sourcePath, exclude)) return; + if (entry.isDirectory()) return collect(sourcePath); + + const relativePath = relative(sourceRoot, sourcePath); + candidates.add(relativePath); + }), + ); + } + + await collect(sourceRoot); + }), + ); + + const found = await Promise.all( + [...candidates].sort().map(async (relativePath) => { + const destPath = join(destDir, relativePath); + try { + await access(destPath); + return destPath; + } catch (error) { + return isNotFoundError(error) ? undefined : destPath; + } + }), + ); + + return { found: found.filter((path) => path !== undefined) }; +} + /** * Recursively process a directory, copying files and adjusting links in markdown. * Single-pass approach: read source → transform if markdown → write to dest. @@ -819,6 +927,7 @@ export async function copyGitHubContent( } const destDir = join(workspacePath, mapping.githubPath); + const effectiveExclude = githubContentExcludes(mapping, options.exclude); if (dryRun) { results.push({ source: sourceDir, destination: destDir, action: 'copied' }); @@ -827,7 +936,10 @@ export async function copyGitHubContent( try { // Single-pass: copy files and adjust markdown links in one traversal - if (mapping.skillsPath || (options.exclude && options.exclude.length > 0)) { + if ( + mapping.skillsPath || + (effectiveExclude && effectiveExclude.length > 0) + ) { await copyAndAdjustDirectory( sourceDir, destDir, @@ -835,7 +947,7 @@ export async function copyGitHubContent( pluginPath, mapping.skillsPath ?? '', skillNameMap, - options.exclude, + effectiveExclude, ); } else { // No skills path and no excludes - just copy without adjustment diff --git a/tests/e2e/plugin-skills.test.ts b/tests/e2e/plugin-skills.test.ts index cb72493..959d7a6 100644 --- a/tests/e2e/plugin-skills.test.ts +++ b/tests/e2e/plugin-skills.test.ts @@ -190,14 +190,14 @@ description: Test skill }); it('promoted GitHub skill sources stay coherent for listing and sync', async () => { - const originalHome = process.env.HOME; + const originalTestHome = process.env.ALLAGENTS_TEST_HOME; const fakeHome = join(tmpDir, 'home'); const cacheDir = join( fakeHome, '.allagents/plugins/marketplaces/NousResearch-hermes-agent@main/skills/research', ); - process.env.HOME = fakeHome; + process.env.ALLAGENTS_TEST_HOME = fakeHome; resetFetchCache(); try { @@ -246,7 +246,11 @@ description: Blog watcher expect(existsSync(join(tmpDir, '.claude/skills/blogwatcher'))).toBe(true); } finally { resetFetchCache(); - process.env.HOME = originalHome; + if (originalTestHome === undefined) { + delete process.env.ALLAGENTS_TEST_HOME; + } else { + process.env.ALLAGENTS_TEST_HOME = originalTestHome; + } } }); }); diff --git a/tests/unit/core/sync-user.test.ts b/tests/unit/core/sync-user.test.ts index ba29d3a..81fab25 100644 --- a/tests/unit/core/sync-user.test.ts +++ b/tests/unit/core/sync-user.test.ts @@ -276,4 +276,122 @@ describe('syncUserWorkspace', () => { expect(stateContent.files.copilot).toContain('.copilot/skills/my-skill/'); expect(stateContent.files.codex).toContain('.codex/skills/my-skill/'); }); + + it('keeps repository hooks project-scoped during user Copilot sync', async () => { + const pluginDir = join(testDir, 'plugins', 'copilot-hooks'); + await mkdir(join(pluginDir, 'hooks'), { recursive: true }); + await mkdir(join(pluginDir, '.github', 'hooks', 'scripts'), { + recursive: true, + }); + await mkdir(join(pluginDir, '.github', 'prompts'), { recursive: true }); + await writeFile( + join(pluginDir, 'hooks', 'global.json'), + '{"hooks":{}}', + ); + await writeFile( + join(pluginDir, '.github', 'hooks', 'repository.json'), + '{"hooks":{}}', + ); + await writeFile( + join(pluginDir, '.github', 'hooks', 'scripts', 'repository.mjs'), + 'console.log("repository hook")', + ); + await writeFile( + join(pluginDir, '.github', 'prompts', 'review.prompt.md'), + '# Review', + ); + + await writeUserConfig({ + repositories: [], + plugins: [pluginDir], + clients: ['copilot'], + syncMode: 'copy', + }); + + // Simulate an upgrade from a version that promoted .github/hooks into + // the user-global Copilot directory. Ownership was not tracked, so the + // next sync must leave the files in place and warn instead of deleting + // potentially user-owned hooks. + const globalHooksDir = join(testDir, '.copilot', 'hooks'); + await mkdir(join(globalHooksDir, 'scripts'), { recursive: true }); + await writeFile( + join(globalHooksDir, 'repository.json'), + '{"hooks":{}}', + ); + await writeFile( + join(globalHooksDir, 'scripts', 'repository.mjs'), + 'console.log("repository hook")', + ); + await writeFile( + join(globalHooksDir, 'user-owned.json'), + '{"hooks":{"UserPromptSubmit":[]}}', + ); + await writeFile( + join(testDir, '.allagents', 'sync-state.json'), + JSON.stringify({ + version: 1, + lastSync: new Date().toISOString(), + files: { copilot: [] }, + }), + ); + + const result = await syncUserWorkspace(); + + expect(result.success).toBe(true); + expect(existsSync(join(globalHooksDir, 'global.json'))).toBe(true); + expect(existsSync(join(globalHooksDir, 'repository.json'))).toBe(true); + expect( + existsSync(join(globalHooksDir, 'scripts', 'repository.mjs')), + ).toBe(true); + expect(existsSync(join(globalHooksDir, 'user-owned.json'))).toBe(true); + expect( + existsSync( + join(testDir, '.copilot', 'prompts', 'review.prompt.md'), + ), + ).toBe(true); + expect(result.warnings).toContain( + "Copilot user hook '.copilot/hooks/repository.json' shares a path with a repository .github/hooks artifact. Repository hooks are no longer synced at user scope; review this file manually if an older AllAgents version installed it. A root hooks/ artifact may still manage the same path.", + ); + expect(result.warnings).toContain( + "Copilot user hook '.copilot/hooks/scripts/repository.mjs' shares a path with a repository .github/hooks artifact. Repository hooks are no longer synced at user scope; review this file manually if an older AllAgents version installed it. A root hooks/ artifact may still manage the same path.", + ); + }); + + it('reports a legacy path accurately when a root hook overwrites it', async () => { + const pluginDir = join(testDir, 'plugins', 'copilot-hooks'); + await mkdir(join(pluginDir, 'hooks'), { recursive: true }); + await mkdir(join(pluginDir, '.github', 'hooks'), { recursive: true }); + await writeFile( + join(pluginDir, 'hooks', 'repository.json'), + '{"hooks":{"global":true}}', + ); + await writeFile( + join(pluginDir, '.github', 'hooks', 'repository.json'), + '{"hooks":{"source":true}}', + ); + await writeUserConfig({ + repositories: [], + plugins: [pluginDir], + clients: ['copilot'], + syncMode: 'copy', + }); + + const legacyHookPath = join( + testDir, + '.copilot', + 'hooks', + 'repository.json', + ); + await mkdir(join(testDir, '.copilot', 'hooks'), { recursive: true }); + await writeFile(legacyHookPath, '{"hooks":{"userModified":true}}'); + + const result = await syncUserWorkspace(); + + expect(await readFile(legacyHookPath, 'utf-8')).toBe( + '{"hooks":{"global":true}}', + ); + expect(result.warnings).toContain( + "Copilot user hook '.copilot/hooks/repository.json' shares a path with a repository .github/hooks artifact. Repository hooks are no longer synced at user scope; review this file manually if an older AllAgents version installed it. A root hooks/ artifact may still manage the same path.", + ); + }); });