Count unique files in create_pull_request patch limit and add max-patch-files config#28472
Count unique files in create_pull_request patch limit and add max-patch-files config#28472
create_pull_request patch limit and add max-patch-files config#28472Conversation
…config Agent-Logs-Url: https://github.com/github/gh-aw/sessions/7645659c-1121-402b-bbc0-b962f97f544d Co-authored-by: mrjf <180956+mrjf@users.noreply.github.com>
create_pull_request patch limit and add max-patch-files config
There was a problem hiding this comment.
Pull request overview
Fixes create_pull_request patch file-limit enforcement to count unique files across multi-commit git format-patch output, and adds a new top-level safe-outputs config knob (max-patch-files, default 100) that flows through compilation and schema.
Changes:
- Update JS PR creation handler to count unique patch file paths (with configurable
max_patch_files) instead of rawdiff --githeader count. - Add
max-patch-filesto safe-outputs Go config parsing/types, thread into compiled handler config, and extend JSON schema. - Add Go/JS tests and regenerate compiled
.lock.ymlworkflow outputs to includemax_patch_files: 100.
Show a summary per file
| File | Description |
|---|---|
actions/setup/js/create_pull_request.cjs |
Implements unique-file counting for patches and enforces a configurable max patch files limit. |
actions/setup/js/create_pull_request.test.cjs |
Adds regression and override tests for unique-file counting and max-files enforcement. |
pkg/workflow/safe_outputs_config.go |
Parses new safe-outputs.max-patch-files and defaults it to 100. |
pkg/workflow/compiler_types.go |
Adds MaximumPatchFiles to SafeOutputsConfig. |
pkg/workflow/compiler_safe_outputs_handlers.go |
Threads MaximumPatchFiles into compiled create_pull_request handler config as max_patch_files. |
pkg/workflow/compiler_safe_outputs_config_test.go |
Tests config propagation to handler config and parsing of max-patch-files. |
pkg/parser/schemas/main_workflow_schema.json |
Adds schema entry for safe-outputs.max-patch-files with default/minimum. |
.changeset/patch-create-pr-max-files-config.md |
Documents the behavior change and new configuration option for release notes. |
.github/workflows/weekly-safe-outputs-spec-review.lock.yml |
Regenerated compiled safe-outputs config to include max_patch_files: 100. |
.github/workflows/weekly-editors-health-check.lock.yml |
Regenerated compiled safe-outputs config to include max_patch_files: 100. |
.github/workflows/weekly-blog-post-writer.lock.yml |
Regenerated compiled safe-outputs config to include max_patch_files: 100. |
.github/workflows/update-astro.lock.yml |
Regenerated compiled safe-outputs config to include max_patch_files: 100. |
.github/workflows/unbloat-docs.lock.yml |
Regenerated compiled safe-outputs config to include max_patch_files: 100. |
.github/workflows/ubuntu-image-analyzer.lock.yml |
Regenerated compiled safe-outputs config to include max_patch_files: 100. |
.github/workflows/tidy.lock.yml |
Regenerated compiled safe-outputs config to include max_patch_files: 100. |
.github/workflows/test-create-pr-error-handling.lock.yml |
Regenerated compiled safe-outputs config to include max_patch_files: 100. |
.github/workflows/technical-doc-writer.lock.yml |
Regenerated compiled safe-outputs config to include max_patch_files: 100. |
.github/workflows/spec-extractor.lock.yml |
Regenerated compiled safe-outputs config to include max_patch_files: 100. |
.github/workflows/spec-enforcer.lock.yml |
Regenerated compiled safe-outputs config to include max_patch_files: 100. |
.github/workflows/smoke-project.lock.yml |
Regenerated compiled safe-outputs config to include max_patch_files: 100. |
.github/workflows/smoke-multi-pr.lock.yml |
Regenerated compiled safe-outputs config to include max_patch_files: 100. |
.github/workflows/smoke-create-cross-repo-pr.lock.yml |
Regenerated compiled safe-outputs config to include max_patch_files: 100. |
.github/workflows/slide-deck-maintainer.lock.yml |
Regenerated compiled safe-outputs config to include max_patch_files: 100. |
.github/workflows/schema-feature-coverage.lock.yml |
Regenerated compiled safe-outputs config to include max_patch_files: 100. |
.github/workflows/refiner.lock.yml |
Regenerated compiled safe-outputs config to include max_patch_files: 100. |
.github/workflows/q.lock.yml |
Regenerated compiled safe-outputs config to include max_patch_files: 100. |
.github/workflows/poem-bot.lock.yml |
Regenerated compiled safe-outputs config to include max_patch_files: 100. |
.github/workflows/layout-spec-maintainer.lock.yml |
Regenerated compiled safe-outputs config to include max_patch_files: 100. |
.github/workflows/jsweep.lock.yml |
Regenerated compiled safe-outputs config to include max_patch_files: 100. |
.github/workflows/instructions-janitor.lock.yml |
Regenerated compiled safe-outputs config to include max_patch_files: 100. |
.github/workflows/hourly-ci-cleaner.lock.yml |
Regenerated compiled safe-outputs config to include max_patch_files: 100. |
.github/workflows/go-logger.lock.yml |
Regenerated compiled safe-outputs config to include max_patch_files: 100. |
.github/workflows/glossary-maintainer.lock.yml |
Regenerated compiled safe-outputs config to include max_patch_files: 100. |
.github/workflows/github-mcp-tools-report.lock.yml |
Regenerated compiled safe-outputs config to include max_patch_files: 100. |
.github/workflows/functional-pragmatist.lock.yml |
Regenerated compiled safe-outputs config to include max_patch_files: 100. |
.github/workflows/dictation-prompt.lock.yml |
Regenerated compiled safe-outputs config to include max_patch_files: 100. |
.github/workflows/developer-docs-consolidator.lock.yml |
Regenerated compiled safe-outputs config to include max_patch_files: 100. |
.github/workflows/dead-code-remover.lock.yml |
Regenerated compiled safe-outputs config to include max_patch_files: 100. |
.github/workflows/daily-workflow-updater.lock.yml |
Regenerated compiled safe-outputs config to include max_patch_files: 100. |
.github/workflows/daily-safe-output-integrator.lock.yml |
Regenerated compiled safe-outputs config to include max_patch_files: 100. |
.github/workflows/daily-rendering-scripts-verifier.lock.yml |
Regenerated compiled safe-outputs config to include max_patch_files: 100. |
.github/workflows/daily-doc-updater.lock.yml |
Regenerated compiled safe-outputs config to include max_patch_files: 100. |
.github/workflows/daily-doc-healer.lock.yml |
Regenerated compiled safe-outputs config to include max_patch_files: 100. |
.github/workflows/daily-community-attribution.lock.yml |
Regenerated compiled safe-outputs config to include max_patch_files: 100. |
.github/workflows/daily-astrostylelite-markdown-spellcheck.lock.yml |
Regenerated compiled safe-outputs config to include max_patch_files: 100. |
.github/workflows/daily-architecture-diagram.lock.yml |
Regenerated compiled safe-outputs config to include max_patch_files: 100. |
.github/workflows/code-simplifier.lock.yml |
Regenerated compiled safe-outputs config to include max_patch_files: 100. |
.github/workflows/code-scanning-fixer.lock.yml |
Regenerated compiled safe-outputs config to include max_patch_files: 100. |
.github/workflows/cloclo.lock.yml |
Regenerated compiled safe-outputs config to include max_patch_files: 100. |
.github/workflows/ci-coach.lock.yml |
Regenerated compiled safe-outputs config to include max_patch_files: 100. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 50/50 changed files
- Comments generated: 2
| // Match: diff --git a/<path> b/<path> | ||
| // Paths may be quoted when they contain unusual characters; we capture both | ||
| // forms and prefer the "b/" path. The non-greedy capture for the a-path is | ||
| // bounded by " b/" to handle paths that contain spaces. | ||
| const re = /^diff --git "?a\/(.+?)"? "?b\/(.+?)"?$/gm; | ||
| let match; | ||
| while ((match = re.exec(patchContent)) !== null) { | ||
| const bPath = match[2] || match[1]; | ||
| if (bPath) { | ||
| files.add(bPath); | ||
| } | ||
| } | ||
| // Fallback: if the structured regex matched nothing (unexpected patch | ||
| // shape) but the patch contains diff headers, count those headers so we | ||
| // never silently skip the limit check. | ||
| if (files.size === 0) { | ||
| const fallback = patchContent.match(/^diff --git /gm); | ||
| return fallback ? fallback.length : 0; |
There was a problem hiding this comment.
countUniquePatchFiles() can undercount (and potentially let patches bypass the file limit) when git emits quoted paths that contain escaped quotes/backslashes (e.g. diff --git "a/foo\"bar" "b/foo\"bar"). The current regex does not handle C-style escapes, and the fallback only triggers when no headers match, so a single unparseable header can be silently dropped.
Consider either using a quoted-string aware pattern (handling \" and \\), or detecting any diff --git headers that didn't match and counting them conservatively so the safety limit can’t be evaded. Adding a regression test for an escaped-quote filename would help prevent reintroductions.
| if maxPatchFiles, exists := outputMap["max-patch-files"]; exists { | ||
| switch v := maxPatchFiles.(type) { | ||
| case int: | ||
| if v >= 1 { | ||
| config.MaximumPatchFiles = v | ||
| } | ||
| case int64: | ||
| if v >= 1 { | ||
| config.MaximumPatchFiles = int(v) | ||
| } | ||
| case uint64: | ||
| if v >= 1 { | ||
| config.MaximumPatchFiles = int(v) | ||
| } | ||
| case float64: | ||
| intVal := int(v) | ||
| if v != float64(intVal) { | ||
| safeOutputsConfigLog.Printf("max-patch-files: float value %.2f truncated to integer %d", v, intVal) | ||
| } | ||
| if intVal >= 1 { | ||
| config.MaximumPatchFiles = intVal | ||
| } | ||
| } |
There was a problem hiding this comment.
When parsing max-patch-files, the int64/uint64/float64 cases cast to int without any bounds checking. Very large values can overflow/wrap and end up as an unexpected (possibly negative) MaximumPatchFiles, which then falls back to the default later and makes the config appear to be ignored.
Suggestion: clamp to math.MaxInt (or reject with a log warning) before converting, and for float64 also guard against values outside the representable int range.
|
@copilot review all comments |
…nversions Agent-Logs-Url: https://github.com/github/gh-aw/sessions/273d508d-3274-453f-9899-2c3b088b20ce Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Addressed both reviewer comments in 46f994f:
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot merge main fix conflicts recompile |
…est-file-limit # Conflicts: # .github/workflows/schema-feature-coverage.lock.yml Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Merged Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
The
create_pull_request100-file limit counted everydiff --githeader in the generated patch. Sincegit format-patch base..branchemits one header per (commit, file), long-running autoloop branches that mergemainbefore committing trip the limit (E003: ...received 1768) even when the iteration only touches 1–3 files. There was also no escape hatch — onlymax-patch-sizewas configurable.Changes
Count unique files (
actions/setup/js/create_pull_request.cjs)parseDiffGitHeader()helper performs a proper C-style quoted-token tokenization ofdiff --githeaders, correctly handling quoted paths with embedded\"and\\escapes.countUniquePatchFiles()walks every header, dedupes by post-image path, and conservatively counts each unparseable header as a synthetic unique entry (keyed by byte offset) so a malformed/quoted-with-escapes header line can never silently bypass the limit.enforcePullRequestLimits(patch, maxFiles)now takes a configurable limit and uses unique counting.New
max-patch-filessafe-outputs option (mirrorsmax-patch-size)SafeOutputsConfig.MaximumPatchFiles(default 100), parsed insafe_outputs_config.gofor int / int64 / uint64 / float64.int:int64/uint64clamp tomath.MaxIntwith a log warning, and out-of-range floats /NaN/±Infare rejected with a log warning instead of producing an undefined narrowing.create_pull_requesthandler config asmax_patch_files, then read by the JS handler.safe-outputsinmain_workflow_schema.json.Tests
TestHandlerConfigPatchFiles,TestParseSafeOutputsMaxPatchFiles(incl.MaxUint64clamp, huge floats,NaN,±Inf).Recompiled all 202
.lock.ymlfiles to surfacemax_patch_files: 100in compiled handler configs.Usage