Skip to content

create_pull_request: pushed commit can contain files that allowed-files/excluded-files should have kept out #48934

Description

@heiskr

👋 Copilot here, on behalf of @heiskr.

Two independent bugs in the create_pull_request push path, hit in two different workflows. Both let a file reach the pushed commit that the workflow config said should not be there.

  1. A shallow checkout sends the push down a rewrite path that reverts unrelated base-branch drift and attributes it to the agent.
  2. excluded-files filters the .patch artifact but not the .bundle artifact, and the bundle is what gets pushed. No rewrite involved.

They share a root shape: validation and push do not operate on the same object, so a passing validation does not tell you what will land.

Both were found in private repos, so paths, SHAs and run links are genericized below. Everything structural is preserved: file counts, byte sizes, commit counts, and gh-aw's own log strings are verbatim.


Bug 1: shallow checkout makes the rewrite revert base-branch drift

create_pull_request synthesized a commit that reverted 13 unrelated files on the base branch, including files explicitly listed in excluded-files, and deleted a file that had been added to the base after the agent's checkout. The max-patch-size gate is the only thing that stopped the PR from being created.

Version: gh-aw v0.82.14, github/gh-aw-actions/setup@v0.82.14.

Setup

The agent edits 2 files in a large monorepo checked out under a path. Relevant frontmatter shape:

checkout:
  - repository: <org>/<monorepo>
    path: monorepo
safe-outputs:
  create-pull-request:
    allowed-repos: [<org>/<monorepo>]
    allowed-files:
      - "src/lib/agent-target.ts"
      - "src/tests/agent-target.ts"
    excluded-files:
      - "src/generated-a/**"
      - "src/generated-b/**"

Two properties of the target repo matter: it has ~61k commits on main, and main moves during the run (the run took about an hour).

What the log shows

The agent's actual patch is fine:

Patch size: 7 KB (maximum allowed: 4096 KB)
Patch size validation passed
Patch content validation passed

Then:

Repository is shallow; fetching 1 bundle prerequisite commit(s) directly from origin by SHA
Using --filter=blob:none for prerequisite SHA fetch (shallow or sparse checkout detected)

The checkout is shallow (the default fetch-depth: 1), so origin/main has no traversable ancestry and git rev-list --parents --topo-order --reverse <origin/main>..HEAD cannot exclude anything:

pushSignedCommits: replaying 61008 commit(s) via GraphQL createCommitOnBranch

The repo has 61,062 commits on main. The range is essentially the entire repository history. A range that size contains merge commits, which trips the rewrite path:

pushSignedCommits: merge commit <sha> detected, refusing unsigned push fallback
Signed push rejected merge commit topology from bundle; rewriting branch and retrying signed push
Rewriting bundled commits to a single linear commit for signed push compatibility (base: origin/main)

The rewrite then stages 15 files instead of 2:

[command]/usr/bin/git reset --soft origin/main
[command]/usr/bin/git diff --cached --name-only
src/lib/agent-target.ts            <- the agent's
src/tests/agent-target.ts          <- the agent's
docs/a.md                          <- base drift
docs/b.md                          <- base drift
docs/c.md                          <- base drift
data/features/new-thing.yml        <- base drift, added to main after checkout
src/generated-a/v1/map.json        <- base drift, matches excluded-files
src/generated-a/v1/changelog.json  <- base drift, matches excluded-files
src/generated-a/v1/projects.json   <- base drift, matches excluded-files
src/generated-a/v1/schema.graphql  <- base drift, matches excluded-files
src/generated-a/v2/map.json        <- base drift, matches excluded-files
src/generated-a/v2/projects.json   <- base drift, matches excluded-files
src/generated-a/v2/schema.graphql  <- base drift, matches excluded-files
src/generated-b/v1/patterns.yml    <- base drift, matches excluded-files
src/generated-b/v2/patterns.yml    <- base drift, matches excluded-files

 15 files changed, 106 insertions(+), 1359 deletions(-)
 delete mode 100644 data/features/new-thing.yml

Only 2 of those 15 are the agent's. The other 13 are the base branch moving forward during the run. The synthesized commit reverts them, and deletes a file that was added to main after the agent's checkout. Nine of the staged paths match this workflow's excluded-files globs.

The push was rejected, but only incidentally, because the reverted generated files happen to be large:

E003: Signed-commit payload exceeds max-patch-size (4096 KB). Synthesized payload additions total 5436 KB

Had the drift been smaller, the PR would have opened and quietly reverted the base branch. The size cap was the only thing standing in the way, which means raising max-patch-size would have shipped it.

Root cause

actions/setup/js/git_helpers.cjs, linearizeRangeAsCommit:

await execApi.exec("git", ["reset", "--soft", baseRef], ...execArgs);
const { stdout: stagedFilesOut } = await execApi.getExecOutput("git", ["diff", "--cached", "--name-only"], ...execArgs);
if (!stagedFilesOut.trim()) {
  throw new Error(`No staged changes found after soft reset to ${baseRef}. ...`);
}
await execApi.exec("git", ["commit", ...commitFlags, "-m", commitMessage], ...execArgs);

baseRef is origin/<base> (set at create_pull_request.cjs:333, used at :346). Soft-resetting to the moving base tip leaves the index at the old tree, so the staged diff becomes diff(base_tip_tree, agent_head_tree), which includes the inverse of all base drift. The intent is diff(agent_base_tree, agent_head_tree).

The staged-changes check at :636 only verifies the set is non-empty, so it cannot catch this.

Suggested fixes

  1. Linearize against the agent's own base, not the branch tip. Reset soft to the bundle's base commit (the prerequisite SHA already extracted by extractBundlePrerequisiteCommits) rather than origin/<base>, or squash with git commit-tree against that base. If the result needs to sit on the current tip, rebase it rather than absorbing the difference.
  2. Warn on an implausible commit range in a shallow checkout. 61,008 commits for a one-commit branch is a clear signal the range is meaningless, and detecting it there is far cheaper than after synthesizing a 5 MB payload.

Workaround

Give the base traversable ancestry:

checkout:
  - repository: <org>/<monorepo>
    path: monorepo
    fetch-depth: 100

The bogus commit range never forms, so merge-commit detection never fires and the rewrite path never runs.


Bug 2: excluded-files never applies to the bundle

Independent of bug 1. No rewrite is involved, the checkout is full depth, and the patch validates clean.

Setup

An agent fixes broken links across Markdown files. One of the files it touched was a nested README.md. README.md is in gh-aw's default protected set and is matched by basename, so the workflow had set excluded-files to keep it out:

safe-outputs:
  create-pull-request:
    allowed-files: ["docs/**/*.md", "data/**/*.md"]
    excluded-files:
      - "**/README.md"
    protected-files:
      policy: blocked

What the log shows

The branch rebased cleanly to a single commit and validation passed:

Patch size: 58 KB
Patch content validation passed

The push then failed:

Signed-commit payload violates file-protection policy (deny): docs/README.md

Evidence

Comparing the two artifacts from that same run:

Artifact Files nested README.md present
.patch 24 no
.bundle 25 yes

excluded-files filtered the patch. gh-aw validated the patch and passed it. Then it pushed the unfiltered bundle, and the push-time protected-files check rejected the whole thing, discarding 24 valid unrelated changes.

Two things worth separating

  1. The bundle is not filtered by excluded-files at all. checkFileProtection runs on the patch, checkFileProtectionPostApply runs on the synthesized payload, and the two see different file sets. A passing validation cannot be trusted to predict the push.
  2. excluded_files and protected_files use different matchers. excluded_files: "**/README.md" is glob-matched at synthesis; protected_files: "README.md" is basename-matched at push. A pattern a user reasonably expects to exempt a file from the protected list does not.

Suggested fix

Apply excluded-files, and re-apply allowed-files, to the object that actually gets pushed, not only to the synthesized patch. As it stands, any code path that rebuilds the commit can reintroduce excluded paths. This also covers bug 1: a post-rewrite check on git diff --cached --name-only would have caught that one and every future variant of it.

What is and is not workaroundable today

Worth being precise, because these look like the same need and are not:

  • "I want the agent to be able to write this file." Solved by protected-files.exclude, which works correctly at both check sites, since checkFileProtection and checkFileProtectionPostApply both read config.protected_files. Verified by running checkFileProtectionPostApply against the compiled lock config.
  • "I want this file kept out of the PR entirely, and a stray edit to it must not kill the run." Not solved. That is what excluded-files reads like it does, and it only does half of it.

So excluded-files currently presents as a working mitigation for protected-file collisions and is not one. That is the part worth fixing.

Metadata

Metadata

Labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions