S2: add Wave 1 protected-files.exclude to squad-deps-worker (#1748) - #1883
Merged
Conversation
Wave 1 basenames (npm/yarn/pnpm manifests + lockfiles, NuGet CPM, Go modules) are now excluded from protected-files in the dependency worker 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 unchanged. Tests (18/18) validate authored and compiled contracts, mutation detection, and structural vendored-content exclusions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
🛫 PR Readiness Check
PR Scope: 🔧 Infrastructure
|
| Status | Check | Details |
|---|---|---|
| ❌ | Single commit | 5 commits — consider squashing before review |
| ✅ | Not in draft | Ready for review |
| ✅ | Branch up to date | Up to date with dev |
| ❌ | Copilot review | No Copilot review yet — it may still be processing |
| ✅ | Changeset present | No source files changed — changeset not required |
| ✅ | Scope clean | No .squad/ or docs/proposals/ files |
| ✅ | No merge conflicts | No merge conflicts |
| ✅ | Copilot threads resolved | 0 active Copilot thread(s) resolved (4 outdated skipped) |
| ❌ | CI passing | 1 check(s) failing: test |
Files Changed (2 files, +404 −165)
| File | +/− |
|---|---|
test/gh-aw-deps-worker-workflow.test.ts |
+370 −146 |
workflows/squad-deps-worker.md |
+34 −19 |
Total: +404 −165
This check runs automatically on every push. Fix any ❌ items and push again.
See CONTRIBUTING.md and PR Requirements for details.
Contributor
🟡 Impact Analysis — PR #1883Risk tier: 🟡 MEDIUM 📊 Summary
🎯 Risk Factors
📦 Modules Affectedroot (1 file)
tests (1 file)
This report is generated automatically for every PR. See #733 for details. |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The updated test file introduces an unused import and a few comment/test-description mismatches that can cause lint/TS failures and make the contract tests harder to interpret.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR advances issue #1748 (S2 slice) by enabling the dependency worker to open signed PRs for a Wave 1 set of dependency manifests/lockfiles via protected-files.exclude, while keeping the general implement worker’s protection model unchanged.
Changes:
- Added Wave 1
protected-files.excludebasenames toworkflows/squad-deps-worker.md(npm/yarn/pnpm manifests+lockfiles,Directory.Packages.props,go.mod,go.sum). - Updated/expanded workflow contract tests to validate both authored YAML and compiled
.lock.ymlsafe-output behavior, plus mutation-detection guards.
File summaries
| File | Description |
|---|---|
| workflows/squad-deps-worker.md | Adds Wave 1 protected-files.exclude entries so the deps worker can PR approved manifest/lockfile basenames while preserving always-protected files. |
| test/gh-aw-deps-worker-workflow.test.ts | Updates tests to assert authored + compiled safe-output contract for the deps worker and ensure the general worker remains unaffected; adds mutation tests. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Fix two reviewer-flagged defects in gh-aw-deps-worker-workflow.test.ts: 1. Policy mutation test was a tautology: the validator re-applied the mutation to depsWorkerFrontmatter independently, so it always saw the expected 'allowed' value regardless of whether applyMutationAndValidate passed the pipeline-produced output. Thread the mutated frontmatter through to the validator signature so the assertion inspects the actual pipeline-produced frontmatter; the test now fails if the mutation is not applied. 2. T1 'exact Wave-1 exclude list' checked subset membership + negatives but not cardinality, so a ninth unrelated exclusion would go undetected. Add an exact length assertion against WAVE_1_MANIFEST_BASENAMES.length. All 18 tests pass. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
- Remove unused writeFileSync import (nit 1) - Fix compileDepsWorker doc: returns create_pull_request config, not full compiled config (nit 2) - Fix applyMutationAndValidate doc: returns void/asserts internally, not true/false (nit 3) - Rename T7-compiled test: 'Wave-1 basenames remain compiled protected' (nit 4) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Finding A: replace **/bin/Debug/** and **/bin/Release/** with **/bin/** in squad-deps-worker excluded-files so that arbitrary nested bin configurations such as src/App/bin/Staging/package.json are structurally excluded. Top-level bin/** is preserved. T8 arrayContaining updated to assert both bin/** and **/bin/**. T8b focused test added to prove nested arbitrary bin paths are covered and narrower patterns absent. Finding B: replace the tautological general-worker authority mutation test (which asserted against a hand-built local array without touching any workflow source) with a genuine compile-based mutation test. The new test mutates squad-implement-worker.md on disk, runs gh aw compile --strict through the real pipeline, parses the compiled .lock.yml safe-output config, and wraps the T7-compiled contract assertion to prove the guard turns red when package.json is injected into the general worker's protected-files.exclude list. Closes #1748 (S2 formal-review findings only) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…clusion T8b previously only checked that the literal strings 'bin/**' and '**/bin/**' exist in the authored excluded-files array (tautological). Strengthen by extracting the bin-specific patterns from the configured excluded-files and evaluating representative paths against them using Node 22's built-in path.matchesGlob — a production-equivalent runtime implementation with semantics appropriate for workflow glob patterns. No new dependencies; matchesGlob is added to the existing node:path import already present in the file. Positive assertions (must be excluded by bin patterns): bin/Staging/package.json src/App/bin/Staging/package.json a/b/c/bin/x86/Release/out.dll Negative assertions (must NOT be excluded by bin patterns alone): package.json src/package.json The negative assertions isolate only the bin patterns (filtered from excluded-files by /\bbin\b/) so they are not confounded by unrelated entries (node_modules, vendor, .squad, etc.). All 19 tests pass. Part of #1748 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This was referenced Aug 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #1748
Scope
This is S2 only -- Wave 1
protected-files.excludeentries for the dependency worker. Deferred out of scope:squad-deps-workernot yet wired into the dispatcher)dependency-changelabel, PR body rules)No dependency manifest or lockfile was changed by this PR.
What changed
workflows/squad-deps-worker.md: addedprotected-files.excludewith all Wave 1 basenames (npm/yarn/pnpm manifests + lockfiles, NuGet CPMDirectory.Packages.props, Gogo.mod/go.sum). Registry/install config (.npmrc,NuGet.Config,bunfig.toml), SDK/tool pins (global.json), and governance docs remain protected. Theexcluded-fileslist structurally strips vendored/generated content.test/gh-aw-deps-worker-workflow.test.ts: 19 tests covering authored + compiled safe-output contract, mutation detection (T9), always-protected invariants, general-worker isolation, and T8b path-evaluation coverage usingpath.matchesGlob(Node 22.5+ built-in) to prove representative nested bin paths are excluded.Validation evidence
Tests:
npx vitest run test/gh-aw-deps-worker-workflow.test.ts-- 19/19 passed (7.99 s)T8b matcher:
path.matchesGlob(Node 22.5.0+ built-in,node:path). No new dependencies. Proven results against extractedexcluded-filesbin patterns:bin/Staging/package.json→ excluded ✓ (matchesbin/**)src/App/bin/Staging/package.json→ excluded ✓ (matches**/bin/**)a/b/c/bin/x86/Release/out.dll→ excluded ✓ (matches**/bin/**)package.json→ NOT excluded by bin patterns ✓src/package.json→ NOT excluded by bin patterns ✓Build:
npm run build-- fails with 6 pre-existing TypeScript errors insquad-cli(health.ts,cast.ts,nap.ts,upgrade.ts) that are identical on the base branch with my changes stashed. Not caused by this PR (S2 touches only workflow markdown and test files).Compiled safe-output observations
gh aw compile squad-deps-worker --strict --no-check-updatein isolated workspace:protected_files_policy:fallback-to-issueconfirmedprotected_files:package.json,package-lock.json,yarn.lock,pnpm-lock.yaml,npm-shrinkwrap.json,Directory.Packages.props,go.mod,go.sum-- all confirmed absentprotected_files:bunfig.toml,NuGet.Config,global.json,CODEOWNERS,SECURITY.md,CONTRIBUTING.md,CHANGELOG.md,CODE_OF_CONDUCT.md,AGENTS.md-- all confirmed presentallowed_files: all 8 Wave-1 manifest/lockfile globs presentexcluded_files:node_modules/**,vendor/**,.squad/**,.github/workflows/**,bin/**,**/bin/**, etc.protect_top_level_dot_folders:true.lock.ymlfiles left inworkflows/gh aw compile squad-implement-worker --strict --no-check-updatein isolated workspace:protected_files(no leakage confirmed)protected_files_policy:fallback-to-issueFresh fixture evidence
Fresh clone of
octodemo/aspiregregator-squad-e2e(local path:%TEMP%/fixture-aspiregregator-1497811405, cleaned up after evidence capture). Copied current S2squad-deps-worker.mdinto the fixture's.github/workflows/, rangh aw compile squad-deps-worker --strict --no-check-updatefrom inside the fixture repo:.lock.ymlconfirmed: all Wave-1 basenames absent fromprotected_files, all always-protected present,policy: fallback-to-issue,allowed_filescomplete,excluded_filescorrectsquad-deps-workerinto the dispatcher) is intentionally absent in this slice. The/squad depsslash command does not exist yet -- that boundary is by design and documented in S3 scope.Generated lock behavior
The generated
.lock.ymlbakes the safe-output config at compile time and is structurally immutable at runtime -- theprotected-files.excludelist cannot be expanded by the agent, only further restricted. This is the correct security boundary for a dependency-only worker.Security review
New secrets in compiled output:
SQUAD_GITHUB_APP_PRIVATE_KEYandSQUAD_GITHUB_TOKEN-- standard Squad workflow secrets already present in the existingsquad-implement-workerandsquad.mdcompiled workflows. The workflowpermissions:block is minimal:contents: read,copilot-requests: write,issues: read,pull-requests: read. No elevated permissions.Architectural review
.squad/config changes in this feature PR