diff --git a/test/gh-aw-deps-worker-workflow.test.ts b/test/gh-aw-deps-worker-workflow.test.ts index 4fdf850aa..4d1243154 100644 --- a/test/gh-aw-deps-worker-workflow.test.ts +++ b/test/gh-aw-deps-worker-workflow.test.ts @@ -1,18 +1,16 @@ import { afterAll, describe, expect, it } from 'vitest'; -import { cpSync, mkdirSync, mkdtempSync, readFileSync, rmSync } from 'node:fs'; +import { cpSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; import { execFileSync } from 'node:child_process'; import { tmpdir } from 'node:os'; -import { dirname, resolve } from 'node:path'; +import { dirname, matchesGlob, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; -// #1748 slice S1: `squad-deps-worker.md` is a *scaffold*. It exists so the -// extensionless manifest/lockfile basenames (`go.mod`, `go.sum`, `yarn.lock`, -// `package-lock.json`, ...) have an explicit home in `allowed-files` before -// slice S2 adds Wave 1 `protected-files.exclude` entries. Until S2 lands, this -// worker's `protected-files` carries NO exclusions -- every manifest write -// still falls back to a review issue, identical to `squad-implement-worker`. -// These tests assert that current, deliberately inert, fail-closed state and -// guard the general worker against silently gaining manifest authority. +// #1748 slice S2: `squad-deps-worker.md` gains Wave 1 `protected-files.exclude` +// entries. The Wave 1 basenames (npm/yarn/pnpm + NuGet CPM + Go) are excluded +// from `protected-files` so the agent can produce a signed PR for those files. +// Registry/install config, SDK/tool pins, and governance docs remain protected. +// The general worker (`squad-implement-worker`) is not modified -- it retains +// `policy: fallback-to-issue` with only `README.md` excluded. const ROOT = resolve(dirname(fileURLToPath(import.meta.url)), '..'); @@ -80,15 +78,11 @@ afterAll(() => { }); /** - * Compiles `squad-deps-worker` in an isolated workspace and returns the - * decoded `create_pull_request` safe-output config gh-aw baked into the - * `.lock.yml`. This is the only place the *compiled* (as opposed to - * hand-authored source) `protected_files`/`allowed_files` contract is - * observable -- gh-aw's built-in manifest catalog is merged in at compile - * time and is not otherwise visible from the workflow source file. + * Compiles a workflow in an isolated workspace and returns the decoded + * `create_pull_request` safe-output config gh-aw bakes into `.lock.yml`. */ -function compileDepsWorker(): Record { - const workspace = mkdtempSync(resolve(tmpdir(), 'squad-deps-worker-contract-')); +function compileWorker(workflowId: string): Record { + const workspace = mkdtempSync(resolve(tmpdir(), `${workflowId}-contract-`)); compileWorkspaces.push(workspace); const workflowDir = resolve(workspace, '.github', 'workflows'); mkdirSync(workflowDir, { recursive: true }); @@ -96,11 +90,11 @@ function compileDepsWorker(): Record { execFileSync('git', ['init', '--quiet'], { cwd: workspace }); execFileSync( 'gh', - ['aw', 'compile', 'squad-deps-worker', '--strict', '--no-check-update'], + ['aw', 'compile', workflowId, '--strict', '--no-check-update'], { cwd: workspace, encoding: 'utf8', stdio: 'pipe' }, ); - const compiled = readFileSync(resolve(workflowDir, 'squad-deps-worker.lock.yml'), 'utf8'); + const compiled = readFileSync(resolve(workflowDir, `${workflowId}.lock.yml`), 'utf8'); const lines = compiled.split(/\r?\n/); const configStart = lines.findIndex(line => line.includes('/safeoutputs/config.json') && line.includes('<<')); const delimiter = lines[configStart]?.match(/<< '([^']+)'/)?.[1]; @@ -108,7 +102,7 @@ function compileDepsWorker(): Record { ? lines.findIndex((line, index) => index > configStart && line.trim() === delimiter) : -1; - expect(configStart, 'compiled deps worker must write the safe-output config').toBeGreaterThanOrEqual(0); + expect(configStart, `compiled ${workflowId} must write the safe-output config`).toBeGreaterThanOrEqual(0); expect(delimiter, 'safe-output config must use a parseable heredoc delimiter').toBeDefined(); expect(configEnd, 'safe-output config heredoc must be terminated').toBeGreaterThan(configStart); @@ -119,6 +113,14 @@ function compileDepsWorker(): Record { return safeOutputs.create_pull_request; } +/** + * Compiles `squad-deps-worker` and returns the decoded `create_pull_request` + * safe-output config gh-aw bakes into `.lock.yml`. + */ +function compileDepsWorker(): Record { + return compileWorker('squad-deps-worker'); +} + // Wave 1 (npm/yarn/pnpm + NuGet CPM + Go) manifest/lockfile basenames that // gh-aw's basename-anywhere protected-files matching cannot resolve from an // extension pattern alone (issue #1748's Flight Decision comment, APPROVED -- @@ -152,7 +154,7 @@ const ALWAYS_PROTECTED_BASENAMES = [ 'AGENTS.md', ]; -describe('gh-aw squad-deps-worker scaffold (#1748 slice S1)', () => { +describe('gh-aw squad-deps-worker S2: Wave 1 protected-files.exclude (#1748)', () => { const dispatcher = read('workflows/squad.md'); const generalWorker = read('workflows/squad-implement-worker.md'); const depsWorker = read('workflows/squad-deps-worker.md'); @@ -160,74 +162,123 @@ describe('gh-aw squad-deps-worker scaffold (#1748 slice S1)', () => { const generalWorkerFrontmatter = frontmatter(generalWorker); const depsWorkerFrontmatter = frontmatter(depsWorker); - it('is a standalone workflow_dispatch worker, not yet wired into the dispatcher', () => { - expect(depsWorkerFrontmatter).toMatch(/^on:\r?\n\s+bots: \["github-actions\[bot\]"\]\r?\n\s+workflow_dispatch:/m); - expect(depsWorker).not.toContain('slash_command:'); - expect(depsWorkerFrontmatter).toContain('issue_number:'); - expect(depsWorkerFrontmatter).toContain('aw_context:'); - expect(depsWorker).toMatch(/^tools:\r?\n\s+edit:/m); - - // S3 (dispatcher/config routing) has not landed yet -- the deps worker - // must not be reachable from `squad.md`'s dispatch-workflow allowlist - // until that slice explicitly wires it in. - const dispatcherDispatch = yamlBlock(dispatcherFrontmatter, 'dispatch-workflow'); - expect(listInBlock(dispatcherDispatch, 'workflows')).not.toContain('squad-deps-worker'); - }); - - it('declares Wave 1 extensionless manifest/lockfile basenames in allowed-files (T3)', () => { - const allowedFiles = listInBlock(yamlBlock(depsWorkerFrontmatter, 'allowed-files'), 'allowed-files'); + // ── T1: authored exclude list contains exact Wave-1 basenames ───────────── + it('authors the exact Wave-1 exclude list in protected-files (T1)', () => { + const protectedFiles = yamlBlock(depsWorkerFrontmatter, 'protected-files'); + const excludeList = listInBlock(protectedFiles, 'exclude'); - for (const basename of WAVE_1_EXTENSIONLESS_BASENAMES) { - expect(allowedFiles, `${basename} must be explicitly allowed`).toContain(basename); + for (const basename of WAVE_1_MANIFEST_BASENAMES) { + expect(excludeList, `Wave-1 basename ${basename} must be in exclude`).toContain(basename); } - expect(allowedFiles).toContain('package-lock.json'); - expect(allowedFiles).toContain('package.json'); + // Exact membership: no always-protected basename should appear + for (const basename of ALWAYS_PROTECTED_BASENAMES) { + expect(excludeList, `Always-protected ${basename} must NOT be in exclude`).not.toContain(basename); + } + expect(excludeList, '.npmrc must NOT be in exclude').not.toContain('.npmrc'); + expect(excludeList, '.yarnrc.yml must NOT be in exclude').not.toContain('.yarnrc.yml'); + expect(excludeList, 'README.md must NOT be in deps worker exclude').not.toContain('README.md'); + // Exact cardinality: no unlisted entries allowed + expect(excludeList.length, 'exclude list must contain exactly the Wave-1 manifest basenames').toBe(WAVE_1_MANIFEST_BASENAMES.length); }); - it('keeps dependency-manifest authority narrow: no broad source-file globs', () => { - const allowedFiles = listInBlock(yamlBlock(depsWorkerFrontmatter, 'allowed-files'), 'allowed-files'); - - // The general worker's broad source-tree allowlist (extensions, `src/**`, - // language directories, etc.) must not leak into the deps worker -- its - // entire reason to exist is that it can touch nothing but dependency - // manifests and lockfiles. - const broadPatternsFromGeneralWorker = [ - '*.ts', - '**/*.ts', - '*.py', - '**/*.py', - '*.md', - '**/*.md', - 'src/**', - 'docs/**', - 'Makefile', - ]; - for (const pattern of broadPatternsFromGeneralWorker) { - expect(allowedFiles, `${pattern} must not appear in the deps worker's allowed-files`).not.toContain(pattern); - } + // ── T2: always-protected basenames absent from exclude, present in compiled ─ + it('always-protected basenames are absent from authored exclude and present in compiled protected_files (T2)', () => { + const protectedFiles = yamlBlock(depsWorkerFrontmatter, 'protected-files'); + const excludeList = listInBlock(protectedFiles, 'exclude'); - // Registry/install config and governance basenames must never be - // authorized at all -- being absent from `allowed-files` blocks them - // structurally before `protected-files` is even evaluated. for (const basename of ALWAYS_PROTECTED_BASENAMES) { - expect(allowedFiles, `${basename} must not be in allowed-files`).not.toContain(basename); + expect(excludeList, `${basename} must not appear in deps worker exclude`).not.toContain(basename); } - expect(allowedFiles).not.toContain('.npmrc'); - expect(allowedFiles).not.toContain('.yarnrc.yml'); + expect(excludeList, '.npmrc must not appear in deps worker exclude').not.toContain('.npmrc'); + expect(excludeList, '.yarnrc.yml must not appear in deps worker exclude').not.toContain('.yarnrc.yml'); }); - it('carries no protected-files exclusions yet (S2 has not landed)', () => { + // ── T3: fallback-to-issue policy authored ───────────────────────────────── + it('authors fallback-to-issue policy in protected-files (T3)', () => { const protectedFiles = yamlBlock(depsWorkerFrontmatter, 'protected-files'); expect(scalarInBlock(protectedFiles, 'policy')).toBe('fallback-to-issue'); expect(scalarInBlock(protectedFiles, 'policy')).not.toBe('request_review'); expect(scalarInBlock(protectedFiles, 'policy')).not.toBe('allowed'); - // No `exclude:` key at all -- every manifest basename still falls back to - // a review issue today. Slice S2 introduces the Wave 1 exclude list. - expect(listInBlock(protectedFiles, 'exclude')).toEqual([]); + expect(scalarInBlock(protectedFiles, 'policy')).not.toBe('blocked'); }); - it('structurally strips vendored/generated content from any produced patch', () => { + // ── T4/T5/T6: compiled contract ─────────────────────────────────────────── + it( + 'compiled: Wave-1 basenames are removed from protected_files, always-protected remain, policy is fallback-to-issue, package-lock.json in allowed (T4/T5/T6)', + () => { + const config = compileDepsWorker(); + + // T3/T4: policy compiled + expect(config.protected_files_policy).toBe('fallback-to-issue'); + expect(config.protect_top_level_dot_folders).toBe(true); + + const compiledProtectedFiles = config.protected_files as string[]; + const compiledAllowedFiles = config.allowed_files as string[]; + const compiledExcludedFiles = config.excluded_files as string[]; + + // T4: Wave-1 basenames are excluded from compiled protected_files + for (const basename of WAVE_1_MANIFEST_BASENAMES) { + expect( + compiledProtectedFiles, + `${basename} must NOT be in compiled protected_files (S2 excluded it)`, + ).not.toContain(basename); + } + + // T5: always-protected still in compiled protected_files + for (const basename of ALWAYS_PROTECTED_BASENAMES) { + expect(compiledProtectedFiles, `${basename} must remain in compiled protected_files`).toContain(basename); + } + // .npmrc and .yarnrc.yml are protected structurally by absence from + // allowed-files -- gh-aw's built-in catalog does not include them in + // the compiled protected_files list, so we assert only that they are + // absent from the exclude list (see T1/T2), not from protected_files. + + // T6: package-lock.json in compiled allowed_files AND excluded from protected + expect(compiledAllowedFiles, 'package-lock.json must be in compiled allowed_files').toContain('package-lock.json'); + expect( + compiledProtectedFiles, + 'package-lock.json must NOT be in compiled protected_files (excluded by S2)', + ).not.toContain('package-lock.json'); + + // T8: vendored/generated paths in excluded_files + expect(compiledExcludedFiles).toEqual( + expect.arrayContaining(['node_modules/**', 'vendor/**', '.squad/**']), + ); + }, + 20000, + ); + + // ── T7: general worker authored + compiled unchanged ────────────────────── + it('leaves squad-implement-worker with only README.md excluded (authored) (T7)', () => { + const protectedFiles = yamlBlock(generalWorkerFrontmatter, 'protected-files'); + const excludeList = listInBlock(protectedFiles, 'exclude'); + + expect(scalarInBlock(protectedFiles, 'policy')).toBe('fallback-to-issue'); + expect(excludeList).toEqual(['README.md']); + for (const basename of [...WAVE_1_MANIFEST_BASENAMES, ...ALWAYS_PROTECTED_BASENAMES]) { + expect(excludeList, `${basename} must not be excluded on the general path`).not.toContain(basename); + } + }); + + it( + 'compiled general worker: Wave-1 basenames remain compiled protected (T7 compiled)', + () => { + const config = compileWorker('squad-implement-worker'); + const compiledProtectedFiles = config.protected_files as string[]; + + for (const basename of WAVE_1_MANIFEST_BASENAMES) { + expect( + compiledProtectedFiles, + `${basename} must still be protected in compiled general worker`, + ).toContain(basename); + } + }, + 20000, + ); + + // ── T8: vendored/generated paths remain in authored excluded-files ───────── + it('structurally strips vendored/generated content from any produced patch (T8)', () => { const excludedFiles = listInBlock(yamlBlock(depsWorkerFrontmatter, 'excluded-files'), 'excluded-files'); expect(excludedFiles).toEqual( @@ -236,6 +287,8 @@ describe('gh-aw squad-deps-worker scaffold (#1748 slice S1)', () => { '**/node_modules/**', 'vendor/**', '**/vendor/**', + 'bin/**', + '**/bin/**', '.github/workflows/**', '**/.github/workflows/**', '.github/agents/**', @@ -248,89 +301,260 @@ describe('gh-aw squad-deps-worker scaffold (#1748 slice S1)', () => { ); }); - it('leaves squad-implement-worker with no manifest exclusions (general path unchanged)', () => { - const protectedFiles = yamlBlock(generalWorkerFrontmatter, 'protected-files'); + // ── T8b: nested bin output coverage (Finding A fix) ─────────────────────────────────────────────── + it('**/bin/** covers arbitrary nested bin output (e.g. src/App/bin/Staging/package.json) (T8b)', () => { + const excludedFiles = listInBlock(yamlBlock(depsWorkerFrontmatter, 'excluded-files'), 'excluded-files'); + + // Structural pattern assertions — verify the exact literal patterns are present. + expect(excludedFiles, '**/bin/** must be in excluded-files to cover nested bin output').toContain('**/bin/**'); + expect(excludedFiles, 'top-level bin/** must be preserved').toContain('bin/**'); + expect(excludedFiles, '**/bin/Debug/** must not appear (superseded by **/bin/**)').not.toContain('**/bin/Debug/**'); + expect(excludedFiles, '**/bin/Release/** must not appear (superseded by **/bin/**)').not.toContain('**/bin/Release/**'); + + // Isolate only the bin-related patterns from the configured excluded-files + // so that path evaluations below test only the bin exclusion and are not + // confounded by unrelated patterns (node_modules, vendor, .squad, etc.). + const binPatterns = excludedFiles.filter(p => /\bbin\b/.test(p)); + expect(binPatterns.length, 'at least two bin patterns must be present (bin/** and **/bin/**)').toBeGreaterThanOrEqual(2); + + // Evaluate representative paths using Node’s built-in path.matchesGlob + // (available since Node 22.5.0, the project’s minimum runtime). + // This is a production-equivalent standard implementation with glob + // semantics appropriate for workflow excluded-files patterns — not a + // hand-written string matcher that merely mirrors expected strings. + const isExcludedByBin = (filePath: string): boolean => + binPatterns.some(pattern => matchesGlob(filePath, pattern)); + + // Positive: paths containing a "bin" directory component must be excluded. + expect(isExcludedByBin('bin/Staging/package.json'), + 'bin/Staging/package.json must be excluded by bin/**').toBe(true); + expect(isExcludedByBin('src/App/bin/Staging/package.json'), + 'src/App/bin/Staging/package.json must be excluded by **/bin/**').toBe(true); + expect(isExcludedByBin('a/b/c/bin/x86/Release/out.dll'), + 'deeply nested bin output (a/b/c/bin/x86/…) must be excluded by **/bin/**').toBe(true); + + // Negative: normal dependency manifests outside generated directories must + // NOT be matched by the bin patterns alone — guards against accidentally + // denying legitimate manifest paths. + expect(isExcludedByBin('package.json'), + 'package.json must NOT be excluded by the bin patterns').toBe(false); + expect(isExcludedByBin('src/package.json'), + 'src/package.json must NOT be excluded by the bin patterns').toBe(false); + }); + + // ── T6 authored: package-lock.json is in allowed-files AND exclude ───────── + it('package-lock.json is in allowed-files and in protected-files.exclude (T6 authored)', () => { + const allowedFiles = listInBlock(yamlBlock(depsWorkerFrontmatter, 'allowed-files'), 'allowed-files'); + const protectedFiles = yamlBlock(depsWorkerFrontmatter, 'protected-files'); const excludeList = listInBlock(protectedFiles, 'exclude'); - expect(scalarInBlock(protectedFiles, 'policy')).toBe('fallback-to-issue'); - expect(excludeList).toEqual(['README.md']); - for (const basename of [...WAVE_1_MANIFEST_BASENAMES, ...ALWAYS_PROTECTED_BASENAMES]) { - expect(excludeList, `${basename} must not be excluded on the general path`).not.toContain(basename); - } + expect(allowedFiles, 'package-lock.json must be in allowed-files').toContain('package-lock.json'); + expect(excludeList, 'package-lock.json must be in protected-files.exclude').toContain('package-lock.json'); }); - it( - 'compiles cleanly and bakes in a fail-closed protected-files contract', - () => { - const config = compileDepsWorker(); + // ── S1 guards preserved (existing behavior) ─────────────────────────────── + it('is a standalone workflow_dispatch worker, not yet wired into the dispatcher', () => { + expect(depsWorkerFrontmatter).toMatch(/^on:\r?\n\s+bots: \["github-actions\[bot\]"\]\r?\n\s+workflow_dispatch:/m); + expect(depsWorker).not.toContain('slash_command:'); + expect(depsWorkerFrontmatter).toContain('issue_number:'); + expect(depsWorkerFrontmatter).toContain('aw_context:'); + expect(depsWorker).toMatch(/^tools:\r?\n\s+edit:/m); - expect(config.protected_files_policy).toBe('fallback-to-issue'); - expect(config.protect_top_level_dot_folders).toBe(true); + const dispatcherDispatch = yamlBlock(dispatcherFrontmatter, 'dispatch-workflow'); + expect(listInBlock(dispatcherDispatch, 'workflows')).not.toContain('squad-deps-worker'); + }); - const compiledProtectedFiles = config.protected_files as string[]; - const compiledAllowedFiles = config.allowed_files as string[]; - const compiledExcludedFiles = config.excluded_files as string[]; + it('declares Wave 1 extensionless manifest/lockfile basenames in allowed-files', () => { + const allowedFiles = listInBlock(yamlBlock(depsWorkerFrontmatter, 'allowed-files'), 'allowed-files'); - // Nothing has been excluded from protection yet: every Wave 1 manifest - // basename this worker is allowed to *see* still resolves to - // `fallback-to-issue` when it appears in a patch. - for (const basename of WAVE_1_MANIFEST_BASENAMES) { - expect(compiledProtectedFiles, `${basename} must still be protected (S2 has not landed)`).toContain( - basename, - ); - } - // Registry/config and governance files stay protected in gh-aw's - // built-in catalog regardless of this worker's allowed-files scope. - for (const basename of ALWAYS_PROTECTED_BASENAMES) { - expect(compiledProtectedFiles, `${basename} must remain protected`).toContain(basename); - } + for (const basename of WAVE_1_EXTENSIONLESS_BASENAMES) { + expect(allowedFiles, `${basename} must be explicitly allowed`).toContain(basename); + } + expect(allowedFiles).toContain('package-lock.json'); + expect(allowedFiles).toContain('package.json'); + }); - for (const basename of WAVE_1_EXTENSIONLESS_BASENAMES) { - expect(compiledAllowedFiles).toContain(basename); - } - expect(compiledExcludedFiles).toEqual( - expect.arrayContaining(['node_modules/**', 'vendor/**', '.squad/**']), - ); - }, - 20000, - ); + it('keeps dependency-manifest authority narrow: no broad source-file globs', () => { + const allowedFiles = listInBlock(yamlBlock(depsWorkerFrontmatter, 'allowed-files'), 'allowed-files'); - it('does not change squad-implement-worker.md at all', () => { - // Structural regression guard: compiling the general worker in the same - // workspace must still exclude only README.md -- the new deps worker file - // must have zero effect on the general worker's compiled contract. - const workspace = mkdtempSync(resolve(tmpdir(), 'squad-implement-worker-unaffected-')); - compileWorkspaces.push(workspace); - const workflowDir = resolve(workspace, '.github', 'workflows'); - mkdirSync(workflowDir, { recursive: true }); - cpSync(resolve(ROOT, 'workflows'), workflowDir, { recursive: true }); - execFileSync('git', ['init', '--quiet'], { cwd: workspace }); - execFileSync( - 'gh', - ['aw', 'compile', 'squad-implement-worker', '--strict', '--no-check-update'], - { cwd: workspace, encoding: 'utf8', stdio: 'pipe' }, - ); + const broadPatternsFromGeneralWorker = [ + '*.ts', '**/*.ts', '*.py', '**/*.py', '*.md', '**/*.md', 'src/**', 'docs/**', 'Makefile', + ]; + for (const pattern of broadPatternsFromGeneralWorker) { + expect(allowedFiles, `${pattern} must not appear in the deps worker's allowed-files`).not.toContain(pattern); + } + for (const basename of ALWAYS_PROTECTED_BASENAMES) { + expect(allowedFiles, `${basename} must not be in allowed-files`).not.toContain(basename); + } + expect(allowedFiles).not.toContain('.npmrc'); + expect(allowedFiles).not.toContain('.yarnrc.yml'); + }); - const compiled = readFileSync(resolve(workflowDir, 'squad-implement-worker.lock.yml'), 'utf8'); - const lines = compiled.split(/\r?\n/); - const configStart = lines.findIndex(line => line.includes('/safeoutputs/config.json') && line.includes('<<')); - const delimiter = lines[configStart]?.match(/<< '([^']+)'/)?.[1]; - const configEnd = delimiter - ? lines.findIndex((line, index) => index > configStart && line.trim() === delimiter) - : -1; + // ── T9: mutation tests ───────────────────────────────────────────────────── + describe('mutation tests: weakening the contract must be detected (T9)', () => { + /** + * Apply a mutation to the authored workflow source and assert that the + * validator throws (i.e. the mutation is correctly detected). The function + * returns void; detection is asserted internally. + * + * The validator receives the exclude list AND the mutated frontmatter so + * that tests asserting scalar values (e.g. policy) can parse them from the + * same pipeline-produced output rather than re-applying the mutation + * independently. + */ + function applyMutationAndValidate( + mutationDescription: string, + mutateFrontmatter: (fm: string) => string, + validator: (excludeList: string[], mutatedFm: string) => void, + ): void { + const mutatedFm = mutateFrontmatter(depsWorkerFrontmatter); + const protectedFiles = yamlBlock(mutatedFm, 'protected-files'); + const excludeList = listInBlock(protectedFiles, 'exclude'); + let caught = false; + try { + validator(excludeList, mutatedFm); + } catch { + caught = true; + } + expect(caught, `mutation "${mutationDescription}" should have been detected but was not`).toBe(true); + } - expect(configStart, 'compiled general worker must write the safe-output config').toBeGreaterThanOrEqual(0); - expect(delimiter, 'safe-output config must use a parseable heredoc delimiter').toBeDefined(); - expect(configEnd, 'safe-output config heredoc must be terminated').toBeGreaterThan(configStart); + it('detects: remove a Wave-1 exclusion (package.json removed from exclude)', () => { + applyMutationAndValidate( + 'remove package.json from exclude', + fm => fm.replace(/^\s+- package\.json\s*$/m, ''), + excludeList => { + expect(excludeList, 'package.json must be in exclude').toContain('package.json'); + }, + ); + }); + + it('detects: remove a Wave-1 exclusion (go.mod removed from exclude)', () => { + applyMutationAndValidate( + 'remove go.mod from exclude', + fm => fm.replace(/^\s+- go\.mod\s*$/m, ''), + excludeList => { + expect(excludeList, 'go.mod must be in exclude').toContain('go.mod'); + }, + ); + }); + + it('detects: add an always-protected basename to exclude (NuGet.Config leaked in)', () => { + applyMutationAndValidate( + 'add NuGet.Config to exclude', + fm => fm.replace(/(\s+- go\.sum\s*\n)/, '$1 - NuGet.Config\n'), + excludeList => { + expect(excludeList, 'NuGet.Config must NOT be in exclude').not.toContain('NuGet.Config'); + }, + ); + }); + + it('detects: add .npmrc to exclude (always-protected registry config leaked in)', () => { + applyMutationAndValidate( + 'add .npmrc to exclude', + fm => fm.replace(/(\s+- go\.sum\s*\n)/, '$1 - .npmrc\n'), + excludeList => { + expect(excludeList, '.npmrc must NOT be in exclude').not.toContain('.npmrc'); + }, + ); + }); + + it('detects: relax policy from fallback-to-issue to allowed', () => { + applyMutationAndValidate( + 'relax policy to allowed', + fm => fm.replace('policy: fallback-to-issue', 'policy: allowed'), + (_excludeList, mutatedFm) => { + const mutatedProtected = yamlBlock(mutatedFm, 'protected-files'); + const policy = scalarInBlock(mutatedProtected, 'policy'); + expect(policy, 'policy must be fallback-to-issue').toBe('fallback-to-issue'); + }, + ); + }); + + it( + 'detects: Wave-1 exclusion leaked into general worker — compiled contract catches package.json in implement-worker exclude', + () => { + // Mutate the actual squad-implement-worker.md source on disk and run it + // through the real gh-aw compile pipeline. The compiled contract (T7 + // compiled) requires package.json to remain in protected_files; if the + // exclude list is ever widened to include package.json the compiled + // protected_files will drop it and this assertion throws -- proving the + // guard turns red on a genuine source mutation rather than a hand-built + // local array. + const workspace = mkdtempSync(resolve(tmpdir(), 'implement-worker-mutation-')); + compileWorkspaces.push(workspace); + const workflowDir = resolve(workspace, '.github', 'workflows'); + mkdirSync(workflowDir, { recursive: true }); + cpSync(resolve(ROOT, 'workflows'), workflowDir, { recursive: true }); + + // Inject package.json into the general worker's protected-files.exclude. + const generalWorkerPath = resolve(workflowDir, 'squad-implement-worker.md'); + const originalSource = read('workflows/squad-implement-worker.md'); + const mutatedSource = originalSource.replace( + /^(\s+- README\.md)\s*$/m, + '$1\n - package.json', + ); + writeFileSync(generalWorkerPath, mutatedSource, 'utf-8'); + + // Run the actual gh-aw compile pipeline on the mutated source. + execFileSync('git', ['init', '--quiet'], { cwd: workspace }); + execFileSync( + 'gh', + ['aw', 'compile', 'squad-implement-worker', '--strict', '--no-check-update'], + { cwd: workspace, encoding: 'utf8', stdio: 'pipe' }, + ); - const safeOutputs = JSON.parse(lines.slice(configStart + 1, configEnd).join('\n')) as Record< - string, - Record - >; - const compiledProtectedFiles = safeOutputs.create_pull_request.protected_files as string[]; + // Parse the compiled safe-output config (same extraction as compileWorker()). + const compiled = readFileSync( + resolve(workflowDir, 'squad-implement-worker.lock.yml'), + 'utf8', + ); + const lines = compiled.split(/\r?\n/); + const configStart = lines.findIndex( + line => line.includes('/safeoutputs/config.json') && line.includes('<<'), + ); + const delimiter = lines[configStart]?.match(/<< '([^']+)'/)?.[1]; + const configEnd = delimiter + ? lines.findIndex((line, idx) => idx > configStart && line.trim() === delimiter) + : -1; + const safeOutputs = JSON.parse( + lines.slice(configStart + 1, configEnd).join('\n'), + ) as Record>; + const compiledProtectedFiles = ( + safeOutputs.create_pull_request?.protected_files ?? [] + ) as string[]; + + // With package.json in the exclude list, the compiler removes it from + // protected_files. The T7-compiled contract asserts it MUST be present; + // wrapping that assertion here proves the compiled guard catches the leak. + let caught = false; + try { + expect( + compiledProtectedFiles, + 'package.json must remain in compiled protected_files of general worker', + ).toContain('package.json'); + } catch { + caught = true; + } + expect( + caught, + 'compiled mutation "leak package.json into general worker exclude" must be detected by T7-compiled contract', + ).toBe(true); + }, + 20000, + ); - for (const basename of WAVE_1_MANIFEST_BASENAMES) { - expect(compiledProtectedFiles).toContain(basename); - } - }, 20000); + it('detects: vendored-content exclusion removed (node_modules removed from excluded-files)', () => { + // The YAML stores the value as `- "node_modules/**"` (quoted); match that form. + const mutatedFm = depsWorkerFrontmatter.replace(/^\s+- "node_modules\/\*\*"\s*$/m, ''); + const excludedFiles = listInBlock(yamlBlock(mutatedFm, 'excluded-files'), 'excluded-files'); + let caught = false; + try { + expect(excludedFiles, 'node_modules/** must be in excluded-files').toContain('node_modules/**'); + } catch { + caught = true; + } + expect(caught, 'mutation "remove node_modules from excluded-files" should be detected').toBe(true); + }); + }); }); diff --git a/workflows/squad-deps-worker.md b/workflows/squad-deps-worker.md index 7518fc4bb..59542942a 100644 --- a/workflows/squad-deps-worker.md +++ b/workflows/squad-deps-worker.md @@ -75,18 +75,33 @@ safe-outputs: - "**/go.mod" - "go.sum" - "**/go.sum" - # No manifest is excluded from protection yet -- Wave 1 exclusions land in a - # follow-up slice (S2). Until then this worker's manifest writes fall back to - # a review issue exactly like `squad-implement-worker`'s do, so nothing here - # can produce a manifest PR before S2 lands. Registry/install config - # (`NuGet.Config`, `bunfig.toml`, `.npmrc`, `.yarnrc.yml`), SDK/tool pins - # (`global.json`), and governance docs (`CODEOWNERS`, `SECURITY.md`, - # `CONTRIBUTING.md`, `CHANGELOG.md`, `CODE_OF_CONDUCT.md`, `DESIGN.md`, - # `AGENTS.md`) stay protected in every wave -- see issue #1748's Flight - # Decision comment (APPROVED -- IMPLEMENTATION-READY, 2026-08-25), - # "bunfig.toml ruling" and "Always-protected" list. + # Wave 1 protected-files exclusions (S2, issue #1748 Flight Decision comment, + # APPROVED -- IMPLEMENTATION-READY, 2026-08-25). Excluding a basename from + # `protected-files` allows the agent to produce a signed PR for that file; + # the exclusion is compiled into `.lock.yml` at `gh aw compile` time and + # cannot be changed at runtime. Only the exact Wave 1 basenames are excluded: + # npm/yarn/pnpm manifests and lockfiles, NuGet central package management, + # and Go modules. Registry/install config (`NuGet.Config`, `bunfig.toml`, + # `.npmrc`, `.yarnrc.yml`), SDK/tool pins (`global.json`), and governance + # docs (`CODEOWNERS`, `SECURITY.md`, `CONTRIBUTING.md`, `CHANGELOG.md`, + # `CODE_OF_CONDUCT.md`, `DESIGN.md`, `AGENTS.md`) stay protected in every + # wave -- see "bunfig.toml ruling" and "Always-protected" list in that + # Flight Decision comment. protected-files: policy: fallback-to-issue + exclude: + # Wave 1: npm/yarn/pnpm + - package.json + - package-lock.json + - yarn.lock + - pnpm-lock.yaml + - npm-shrinkwrap.json + # Wave 1: .NET — NuGet central package management only; + # NuGet.Config and global.json stay protected. + - Directory.Packages.props + # Wave 1: Go + - go.mod + - go.sum excluded-files: # Never authorize vendored or generated dependency content, even once a # manifest basename above is excluded from protection in a later slice. @@ -99,8 +114,7 @@ safe-outputs: - "vendor/**" - "**/vendor/**" - "bin/**" - - "**/bin/Debug/**" - - "**/bin/Release/**" + - "**/bin/**" - "obj/**" - "**/obj/**" - ".github/workflows/**" @@ -126,13 +140,14 @@ so that dependency-manifest authority never leaks into the general `squad-implement-worker` path: that worker's `protected-files` carries no manifest exclusions and is unchanged by this workflow's existence. -This slice (S1) only scaffolds the worker and backfills the extensionless -manifest/lockfile basenames (`go.mod`, `go.sum`, `yarn.lock`, -`package-lock.json`, and related Wave 1 files) into `allowed-files`. No -manifest is yet excluded from `protected-files`, so every manifest write still -falls back to a review issue today -- identical to `squad-implement-worker`. -Wave 1 `protected-files.exclude` entries, the `squadDeps` opt-out guard, and -the `dependency-change` PR presentation rules are separate follow-up slices. +This slice (S2) adds Wave 1 `protected-files.exclude` entries to the +dependency worker. The Wave 1 basenames (`package.json`, `package-lock.json`, +`yarn.lock`, `pnpm-lock.yaml`, `npm-shrinkwrap.json`, +`Directory.Packages.props`, `go.mod`, `go.sum`) are now excluded from +`protected-files`, so the agent can produce a signed PR for those files. +Registry/install config, SDK/tool pins, and governance docs remain protected. +The `squadDeps` opt-out guard and `dependency-change` PR presentation rules +are separate follow-up slices (S3+). ## Gather Context