diff --git a/packages/core/src/core/coreToolScheduler.test.ts b/packages/core/src/core/coreToolScheduler.test.ts index 04dfdb378..b2bfe4335 100644 --- a/packages/core/src/core/coreToolScheduler.test.ts +++ b/packages/core/src/core/coreToolScheduler.test.ts @@ -4391,11 +4391,34 @@ describe('extractToolFilePaths', () => { expect(extractToolFilePaths('glob', { pattern: 'src/**/*.tsx' })).toEqual([ 'src/**/*.tsx', ]); - // Both `path` (search root) and `pattern` (selector) when both - // present. + }); + + it('joins glob.path + glob.pattern into the effective selector', () => { + // Regression: glob({ path: 'src', pattern: '**/*.ts' }) actually + // searches src/**/*.ts. Emitting them as separate candidates + // ('src', '**/*.ts') would NOT activate a skill keyed on + // `paths: ['src/**/*.ts']`, because neither component matches the + // skill glob in isolation. Join them with path.join so the + // effective-selector candidate reflects what the tool really + // touched. (The standalone `path` candidate is still emitted by the + // generic block above so a broad skill keyed on `paths: ['src/**']` + // still matches.) expect( extractToolFilePaths('glob', { path: 'src', pattern: '**/*.ts' }), - ).toEqual(['src', '**/*.ts']); + ).toEqual(['src', 'src/**/*.ts']); + }); + + it('joins absolute glob.path with pattern (registry guard rejects downstream)', () => { + // glob({ path: '/tmp/external', pattern: '**/*.ts' }) joins to an + // absolute path. SkillActivationRegistry's project-root guard + // rejects it; the test pins the joined shape so absolute roots + // stay distinguishable from project-relative ones. + expect( + extractToolFilePaths('glob', { + path: '/tmp/external', + pattern: '**/*.ts', + }), + ).toEqual(['/tmp/external', '/tmp/external/**/*.ts']); }); it('does not extract pattern for non-glob tools', () => { diff --git a/packages/core/src/core/coreToolScheduler.ts b/packages/core/src/core/coreToolScheduler.ts index 3e00231cc..faf7a64d2 100644 --- a/packages/core/src/core/coreToolScheduler.ts +++ b/packages/core/src/core/coreToolScheduler.ts @@ -47,6 +47,7 @@ import type { Part, PartListUnion, } from '@google/genai'; +import * as path from 'node:path'; import { ToolNames } from '../tools/tool-names.js'; import { CONCURRENCY_SAFE_KINDS } from '../tools/tools.js'; import { isShellCommandReadOnly } from '../utils/shellReadOnlyChecker.js'; @@ -247,10 +248,24 @@ export function extractToolFilePaths( for (const p of pathsField) push(p); } if (toolName === ToolNames.GLOB) { - // Glob-only: include `pattern` as a path-shaped selector. Don't add - // it for grep_search even though grep also has a `pattern` field — - // grep's pattern is a regex, not a path glob, and would false-match. - push(obj['pattern']); + // Glob-only: derive the effective selector from `path` + `pattern`. + // The previous version pushed `pattern` standalone, but the glob + // call actually searches `/` — so a skill keyed on + // `paths: ['src/**/*.ts']` would not activate from + // `glob({ path: 'src', pattern: '**/*.ts' })` because the candidate + // was just `**/*.ts`. Joining matches what the tool really touched. + // (Don't add `pattern` for grep_search even though it also has a + // `pattern` field — grep's pattern is a regex, not a path glob, + // and would false-match.) + const pathField = obj['path']; + const patternField = obj['pattern']; + if (typeof patternField === 'string' && patternField.length > 0) { + if (typeof pathField === 'string' && pathField.length > 0) { + push(path.join(pathField, patternField)); + } else { + push(patternField); + } + } } return out; } diff --git a/packages/core/src/skills/skill-activation.test.ts b/packages/core/src/skills/skill-activation.test.ts index 71cd81870..899ca78cf 100644 --- a/packages/core/src/skills/skill-activation.test.ts +++ b/packages/core/src/skills/skill-activation.test.ts @@ -201,3 +201,50 @@ describe('resolveProjectRelativePath', () => { ).toBe('src/App.tsx'); }); }); + +describe('extractToolFilePaths → SkillActivationRegistry integration', () => { + // Regression: feed the real candidate output for a `glob` call into + // the registry and assert end-to-end activation. The earlier per-field + // extraction (path + pattern as separate candidates) silently failed + // to activate skills keyed on the joined effective selector — there + // was no test exercising the path that mattered. + it('activates a skill keyed on src/**/*.ts from glob({ path: "src", pattern: "**/*.ts" })', async () => { + const { extractToolFilePaths } = await import( + '../core/coreToolScheduler.js' + ); + const candidates = extractToolFilePaths('glob', { + path: 'src', + pattern: '**/*.ts', + }); + const reg = new SkillActivationRegistry( + [makeSkill({ name: 'tsx-helper', paths: ['src/**/*.ts'] })], + '/project', + ); + // Hand each candidate the registry the way coreToolScheduler does; + // collect the union. + const activated = new Set(); + for (const c of candidates) { + for (const n of reg.matchAndConsume(c)) activated.add(n); + } + expect(Array.from(activated)).toEqual(['tsx-helper']); + }); + + it('does NOT activate from external glob.path (project-root guard wins)', async () => { + const { extractToolFilePaths } = await import( + '../core/coreToolScheduler.js' + ); + const candidates = extractToolFilePaths('glob', { + path: '/tmp/external', + pattern: '**/*.ts', + }); + const reg = new SkillActivationRegistry( + [makeSkill({ name: 'broad', paths: ['**/*.ts'] })], + '/project', + ); + const activated = new Set(); + for (const c of candidates) { + for (const n of reg.matchAndConsume(c)) activated.add(n); + } + expect(activated.size).toBe(0); + }); +});