Skip to content

fix(worker): use fallback-to-issue for protected files - #1747

Merged
bradygaster merged 2 commits into
devfrom
bradygaster-1746-protected-files-fallback-to-issue
Aug 20, 2026
Merged

fix(worker): use fallback-to-issue for protected files#1747
bradygaster merged 2 commits into
devfrom
bradygaster-1746-protected-files-fallback-to-issue

Conversation

@bradygaster

Copy link
Copy Markdown
Owner

Closes #1746

What

Replaces protected-files: request_review with the object form in workflows/squad-implement-worker.md:

protected-files:
  policy: fallback-to-issue
  exclude:
    - README.md

Why

request_review is unusable for signed create-pull-request writes. Verified against gh-aw v0.86.2 source:

  1. create_pull_request.cjs calls checkFileProtection(...), classifies request_review as a soft action, and logs that it will open the PR with a caution and a request-changes review.
  2. The same handler then calls pushSignedCommits(...).
  3. push_signed_commits.cjs synthesizes the GraphQL createCommitOnBranch payload and calls checkFileProtectionPostApply(...), which throws on anything other than allow.

So the run announces soft handling and then hard-refuses with Signed-commit payload violates file-protection policy. This blocked the live end-to-end continuation test in bradygaster/aspiregregator-squad-test (finding F).

fallback-to-issue takes a different route — the signed push still rejects, but the PR handler has manifestProtectionFallback set, catches the failure, and opens a protected-file review issue instead.

Why README.md is excluded

gh-aw's default protected list (runtime_definitions.go:206-214) includes README.md, matched by basename at any depthmanifest_file_helpers.cjs:301-305 does file.split("/").pop(). That's a different matcher from allowed-files globs, where * does not cross /.

Under plain fallback-to-issue, every README task would produce a review issue rather than a PR. README updates are one of the most common task classes there is, and the worker's output is always a reviewable PR — which is the actual safety mechanism. Protecting every README.md at every depth is inherited from gh-aw's generic manifest-safety model, not from Squad's product model.

Everything genuinely load-bearing stays protected: dependency manifests and lockfiles, CODEOWNERS, SECURITY.md, CONTRIBUTING.md, CHANGELOG.md, and top-level dot folders. CHANGELOG.md specifically stays protected because in adopting repos it is often generated, is release provenance, and may satisfy a release gate — a false entry can misstate shipped behavior.

Not protected-files: allowed, which would let the worker rewrite protected manifests and security docs outright.

Scope

excluded-files is unchanged. It strips .github/workflows/, .github/agents/, .github/aw/, and .squad/ from the patch before protected-file evaluation, so it solves a different problem and is independent of this change.

Validation

  • node scripts/check-workflow-input-interpolation.mjs — passed, 5 prompt files scanned
  • npx vitest run test/gh-aw-quality.test.ts — 81 passed / 13 skipped, matching the dev baseline
  • No packages/*/src/ changes, so no changeset required
  • Single file, no mirrors (workflows/squad-implement-worker.md is not duplicated into the template directories)

gh aw compile cannot validate this file from the repo root — workflows/*.md are source that install into consuming repos, so dispatch-workflow can't resolve squad in .github/workflows/. That failure is structural and pre-existing.

Follow-ups (not this PR)

  • Document fallback-to-issue as a deliberate safety handoff rather than a failed worker. When a task legitimately needs to touch a manifest ("add the Serilog package"), the user gets an issue instead of a PR — defensible as a supply-chain decision, but it needs to read as intentional.
  • Surface protected-file policy as an adoption-time repo configuration choice, alongside the finding E preflight for Settings → Actions → Allow GitHub Actions to create and approve pull requests. Both are adoption-time repo safety choices and should share one surface.
  • Any consuming/test repo needs its installed .lock.yml recompiled before this takes effect there.

`protected-files: request_review` is incompatible with signed
create-pull-request writes. The PR handler classifies it as a soft
action and logs it that way, then the signed-push path re-validates the
synthesized GraphQL payload and rejects anything but `allow`, failing
with "Signed-commit payload violates file-protection policy". The run
announces soft handling and then hard-fails, which blocked the live
end-to-end continuation test.

Switch to the object form so a protected write routes to a review issue
instead, and exclude README.md from gh-aw's inherited default list.
README.md is high-frequency, low-control-plane work that ordinary PR
review already covers; leaving it protected would turn every docs task
into an issue rather than a PR. Everything load-bearing stays protected:
dependency manifests, lockfiles, CODEOWNERS, SECURITY.md,
CONTRIBUTING.md, CHANGELOG.md, and top-level dot folders.

Note that gh-aw matches protected files by basename at any depth
(manifest_file_helpers.cjs does file.split("/").pop()), which is a
different matcher from allowed-files globs where `*` does not cross `/`.

Existing excluded-files rules are unchanged — they strip paths from the
patch before protected-file evaluation and remain independent.

Closes #1746

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 26891f18-3f78-4cda-801b-ee668fecae33
Copilot AI lite review requested due to automatic review settings August 19, 2026 20:36
@github-actions

github-actions Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

🟡 Impact Analysis — PR #1747

Risk tier: 🟡 MEDIUM

📊 Summary

Metric Count
Files changed 2
Files added 0
Files modified 2
Files deleted 0
Modules touched 2

🎯 Risk Factors

  • 2 files changed (≤5 → LOW)
  • 2 modules touched (2-4 → MEDIUM)

📦 Modules Affected

root (1 file)
  • workflows/squad-implement-worker.md
tests (1 file)
  • test/gh-aw-implement-workflow.test.ts

This report is generated automatically for every PR. See #733 for details.

@github-actions

github-actions Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

🛫 PR Readiness Check

ℹ️ This comment updates on each push. Last checked: commit 9ad363d

PR Scope: 🔧 Infrastructure

⚠️ 3 item(s) to address before review

Status Check Details
Single commit 2 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 1 unresolved Copilot thread(s) — fix and resolve before merging
CI passing All checks passing

Files Changed (2 files, +103 −7)

File +/−
test/gh-aw-implement-workflow.test.ts +91 −6
workflows/squad-implement-worker.md +12 −1

Total: +103 −7


This check runs automatically on every push. Fix any ❌ items and push again.
See CONTRIBUTING.md and PR Requirements for details.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the Squad implementation worker’s safe-outputs configuration to use the protected-files object form with fallback-to-issue, preserving protection for high-risk files while exempting README.md to avoid routing routine docs edits into review issues.

Changes:

  • Replaces protected-files: request_review with protected-files.policy: fallback-to-issue.
  • Excludes README.md from the default protected-file set to keep common docs updates flowing through PRs.
Show a summary per file
File Description
workflows/squad-implement-worker.md Switches protected-file handling to fallback-to-issue with an explicit README.md exclusion.

Review details

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment on lines +178 to +181
protected-files:
policy: fallback-to-issue
exclude:
- README.md
Closes #1746

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Worker protected-files: request_review is incompatible with signed commits (finding F)

2 participants