From 23e9864bb4f3780667cb9c510b43c64bab8fd6ef Mon Sep 17 00:00:00 2001 From: Saul Moro Date: Tue, 22 Sep 2026 17:20:42 +0200 Subject: [PATCH] fix(doctor): probe the Copilot hooks file where inject writes it (#732) In a non-self project scope, resolveDoctorContext forces the hook paths to the hook scope ('user', per resolveHookScope) so settings-based hooks are probed where reconcileHooksToAllTools writes them. The standalone Copilot hooks file is written by reconcileTeamHooksForConfig at the config's own scope instead, so the doctor ended up joining the userScope relative path (hooks/teamai.json) onto and reported Copilot missing right after a successful `hooks inject`. buildHookChecks now takes both maps and picks per hook kind: a standalone `hooks` file from the config-scoped paths, `settings` from the hook-scoped ones. Same rule `hooks list` already applies. Hypothesis confirmed: scope mismatch between the two path maps, introduced when #695 moved the doctor's hook paths to the hook scope for Qoder CN. --- src/__tests__/doctor.test.ts | 37 ++++++++++++++++++++++++++++++++++++ src/doctor.ts | 14 +++++++++++--- 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/src/__tests__/doctor.test.ts b/src/__tests__/doctor.test.ts index a93e05cfc..cd1978a7d 100644 --- a/src/__tests__/doctor.test.ts +++ b/src/__tests__/doctor.test.ts @@ -286,6 +286,43 @@ describe('doctor — hook checks', () => { expect(allPassed).toBe(false); }); + // Non-self project scope: `hooks inject` writes copilot at + // /.github/hooks/teamai.json (the config's own scope), so doctor + // must probe that file — not userScope.hooks joined onto projectRoot. + it('checks project Copilot hooks where inject wrote them when userScope.hooks is set', async () => { + const projectRoot = '/tmp/teamai-doctor-copilot-project-userscope'; + const hookPath = path.join(projectRoot, '.github', 'hooks', 'teamai.json'); + mockedLoadLocalConfig.mockResolvedValue({ + ...mockLocalConfig, + scope: 'project', + projectRoot, + enabledAgents: ['copilot'], + }); + mockedLoadTeamConfig.mockResolvedValue({ + ...mockTeamConfig, + sharing: { env: { injectShellProfile: false } }, + toolPaths: { + copilot: { + hooks: '.github/hooks/teamai.json', + userScope: { hooks: 'hooks/teamai.json' }, + }, + }, + }); + mockedPathExists.mockImplementation(async (filePath: string) => ( + filePath === hookPath || filePath === path.dirname(hookPath) + )); + mockedReadFileSafe.mockImplementation(async (filePath: string) => ( + filePath === hookPath ? buildFullHooksContent() : null + )); + + await doctor({}); + const copilotLine = consoleSpy.mock.calls + .map((call) => String(call[0])) + .find((message) => message.includes('hooks in copilot')); + + expect(copilotLine).toContain('✔'); + }); + it('does not infer project Copilot installation from .github/hooks alone', async () => { const projectRoot = '/tmp/teamai-doctor-unselected-copilot'; const copilotHome = '/tmp/teamai-doctor-unselected-home'; diff --git a/src/doctor.ts b/src/doctor.ts index b403ca65a..91ae901c4 100644 --- a/src/doctor.ts +++ b/src/doctor.ts @@ -170,15 +170,23 @@ async function buildEnabledToolChecks(ctx: DoctorContext): Promise { */ async function buildHookChecks( toolPaths: TeamaiConfig['toolPaths'], + hookToolPaths: TeamaiConfig['toolPaths'], baseDir: string, localConfig: LocalConfig, ): Promise { const checks: Check[] = []; for (const [tool, paths] of Object.entries(toolPaths)) { + // A standalone hooks file (Copilot) is injected at the config's own scope + // (`reconcileTeamHooksForConfig` joins resolveToolBaseDir with the + // config-scoped `hooks`), so it is probed from `toolPaths`. Settings-based + // hooks follow resolveHookScope and are probed from `hookToolPaths`. + // Mixing the two — userScope `hooks/teamai.json` under — + // reported Copilot missing right after a successful `hooks inject` (#732). + const settings = hookToolPaths[tool]?.settings; const hookPath = paths.hooks ? path.join(resolveToolBaseDir(tool, localConfig), paths.hooks) - : paths.settings - ? path.join(baseDir, paths.settings) + : settings + ? path.join(baseDir, settings) : undefined; if (!hookPath) continue; const settingsPath = hookPath; @@ -375,7 +383,7 @@ export async function buildChecks(ctx: DoctorContext, stage: CheckStage = 'docto + 'can push to the team repo (run with --verbose to see the push error).', }, ...await buildEnabledToolChecks(ctx), - ...await buildHookChecks(hookToolPaths, baseDir, localConfig), + ...await buildHookChecks(toolPaths, hookToolPaths, baseDir, localConfig), ...await buildDeliveryChecks(ctx), // Built only for `doctor`: the work is in building these, not in running // them, so skipping them post-pull is what keeps the budget for the rest.