Add configurable patch size limit for repo-memory safe output updates#18144
Add configurable patch size limit for repo-memory safe output updates#18144
Conversation
|
Hey That said, the PR is currently a skeleton — the single "Initial plan" commit carries no file changes, so a few things are needed before this can be reviewed:
If you'd like the agent to pick this up and drive it to completion, here's a ready-to-use prompt:
|
- Add MaxPatchSize field to RepoMemoryEntry with 10KB default - Parse max-patch-size config option with bounds validation - Pass MAX_PATCH_SIZE env var to push step - Validate patch size after staging via git diff --cached - Add tests for default, bounds validation, and array notation - Update scratchpad/repo-memory.md spec with new validation rule - Update docs with max-patch-size option and troubleshooting Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
pkg/workflow/repo_memory.go
Outdated
| entry.MaxPatchSize = int(sizeUint64) | ||
| } | ||
| // Validate max-patch-size bounds | ||
| if err := validateIntRange(entry.MaxPatchSize, 1, 104857600, "max-patch-size"); err != nil { |
There was a problem hiding this comment.
@copilot the maximum max-patch-size for repo memory should be 100kb.
There was a problem hiding this comment.
Changed the max-patch-size upper bound from 100MB to 100KB (102400 bytes) in f6cccfb. Updated both validation calls in repo_memory.go, the tests, and the spec/docs.
|
@copilot update the safe-output specific file in docs for repo memory: Enforce max patch size |
There was a problem hiding this comment.
Pull request overview
This PR adds a patch size limit feature to the repo-memory tool to prevent excessively large updates. The feature introduces a new max-patch-size configuration option that validates the total size of git changes before committing.
Changes:
- Added
max-patch-sizeconfiguration field with 10KB default and validation (1 byte to 100MB range) - Implemented patch size checking in JavaScript layer using
git diff --cachedbefore committing - Updated specification and user documentation with patch size limit details
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/repo_memory.go | Added MaxPatchSize field to struct, parsing logic for both config formats, and environment variable passing with default 10KB |
| pkg/workflow/repo_memory_test.go | Added comprehensive tests for default value, boundary validation, and array notation support |
| actions/setup/js/push_repo_memory.cjs | Implemented patch size computation after staging and validation before committing with error messages |
| scratchpad/repo-memory.md | Updated specification with patch size limit section including computation method and validation rules |
| docs/src/content/docs/reference/repo-memory.md | Added max-patch-size option documentation and troubleshooting guidance |
| .github/workflows/*.lock.yml | Generated workflow files with MAX_PATCH_SIZE environment variable set to 10240 |
Comments suppressed due to low confidence (1)
pkg/workflow/repo_memory.go:134
- The default MaxPatchSize of 10240 bytes (10KB) is inconsistent with the default MaxFileSize. A git diff patch for a file is larger than the file itself due to diff format overhead (headers, context lines, etc.). With MaxFileSize at 10240 bytes, a file approaching that size would produce a patch exceeding 10KB, causing validation failures. The default MaxPatchSize should be significantly larger than MaxFileSize, or at least equal to it with additional headroom for diff overhead. Consider setting the default to at least 20480 bytes (20KB) or making it proportional to MaxFileSize.
MaxPatchSize: 10240, // 10KB
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Parse max-patch-size | ||
| if maxPatchSize, exists := memoryMap["max-patch-size"]; exists { | ||
| if sizeInt, ok := maxPatchSize.(int); ok { | ||
| entry.MaxPatchSize = sizeInt | ||
| } else if sizeFloat, ok := maxPatchSize.(float64); ok { | ||
| entry.MaxPatchSize = int(sizeFloat) | ||
| } else if sizeUint64, ok := maxPatchSize.(uint64); ok { | ||
| entry.MaxPatchSize = int(sizeUint64) | ||
| } | ||
| // Validate max-patch-size bounds | ||
| if err := validateIntRange(entry.MaxPatchSize, 1, 104857600, "max-patch-size"); err != nil { | ||
| return nil, err | ||
| } | ||
| } |
There was a problem hiding this comment.
There is no validation to ensure max-patch-size is compatible with max-file-size. Since a git diff patch for a file includes the file content plus diff format overhead (headers, context lines, etc.), the patch size will always be larger than the file size. At minimum, max-patch-size should be greater than max-file-size to avoid runtime failures. Consider adding validation that enforces max-patch-size >= max-file-size * 1.2 (allowing for ~20% diff overhead), or warning users when max-patch-size is less than max-file-size.
This issue also appears on line 134 of the same file.
| const maxPatchSizeKb = Math.ceil(maxPatchSize / 1024); | ||
| core.info(`Patch size: ${patchSizeKb} KB (maximum allowed: ${maxPatchSizeKb} KB)`); | ||
| if (patchSizeBytes > maxPatchSize) { | ||
| core.setFailed(`Patch size (${patchSizeKb} KB) exceeds maximum allowed size (${maxPatchSizeKb} KB). Reduce the number or size of changes, or increase max-patch-size.`); |
There was a problem hiding this comment.
Using Math.ceil() for both the actual patch size and the maximum allowed size can produce confusing error messages. For example, if the patch is 10241 bytes and the limit is 10240 bytes, both would display as "10 KB" in the error message ("Patch size (10 KB) exceeds maximum allowed size (10 KB)"), which would confuse users. Consider using Math.floor() for the limit or showing exact byte counts in the error message for clarity.
| const maxPatchSizeKb = Math.ceil(maxPatchSize / 1024); | |
| core.info(`Patch size: ${patchSizeKb} KB (maximum allowed: ${maxPatchSizeKb} KB)`); | |
| if (patchSizeBytes > maxPatchSize) { | |
| core.setFailed(`Patch size (${patchSizeKb} KB) exceeds maximum allowed size (${maxPatchSizeKb} KB). Reduce the number or size of changes, or increase max-patch-size.`); | |
| const maxPatchSizeKb = Math.floor(maxPatchSize / 1024); | |
| core.info(`Patch size: ${patchSizeKb} KB (${patchSizeBytes} bytes) (maximum allowed: ${maxPatchSizeKb} KB (${maxPatchSize} bytes))`); | |
| if (patchSizeBytes > maxPatchSize) { | |
| core.setFailed(`Patch size (${patchSizeKb} KB, ${patchSizeBytes} bytes) exceeds maximum allowed size (${maxPatchSizeKb} KB, ${maxPatchSize} bytes). Reduce the number or size of changes, or increase max-patch-size.`); |
There was a problem hiding this comment.
Applied in 84197c2 — Math.floor() for the limit display and byte counts included in both the info log and error message.
| file-glob: ["memory/custom-agent-for-aw/*.md", "memory/custom-agent-for-aw/*.json"] | ||
| max-file-size: 1048576 # 1MB (default 10KB) | ||
| max-file-count: 50 # default 100 | ||
| max-patch-size: 51200 # 50KB (default 10KB) |
There was a problem hiding this comment.
The documentation example shows max-file-size: 1048576 (1MB) with max-patch-size: 51200 (50KB), which is inconsistent. A git diff patch for a 1MB file would be at least 1MB in size (plus diff format overhead), far exceeding the 50KB patch size limit. This would make it impossible to commit files near the maximum file size. The example should either use a much larger max-patch-size value (e.g., at least 2097152 bytes = 2MB to account for diff overhead) or reduce the max-file-size to be compatible with the 50KB patch limit.
| max-patch-size: 51200 # 50KB (default 10KB) | |
| max-patch-size: 2097152 # 2MB (default 10KB) |
There was a problem hiding this comment.
Fixed in 84197c2 — the docs example now uses max-patch-size: 102400 (100KB, the maximum), which is consistent with the 1MB max-file-size.
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
…ML fix for max-patch-size Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in e341e2f:
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:
|
Repo-memory push jobs had no bound on total diff size — a single update could write arbitrarily large patches to git. This adds a configurable
max-patch-sizelimit (default 10KB, max 100KB) enforced both at config-parse time and at staging before commit.Changes
Go layer (
pkg/workflow/repo_memory.go)MaxPatchSize intfield toRepoMemoryEntry(default:10240)max-patch-sizefrontmatter in both object and array notation with bounds[1, 102400](1 byte to 100KB)defaultRepoMemoryMaxPatchSize = 10240andmaxRepoMemoryPatchSize = 102400as named constants; all hardcoded magic numbers replacedMAX_PATCH_SIZEenv var into generated push step YAMLJavaScript layer (
actions/setup/js/push_repo_memory.cjs)git add ., computesgit diff --cachedbyte sizeeffectiveMaxPatchSize = Math.floor(maxPatchSize * 1.2)) to account for git diff format overhead (headers, context lines)Math.floor()to avoid ambiguous "10 KB vs 10 KB" messagesAgent prompt (
pkg/workflow/repo_memory_prompt.go)buildRepoMemoryPromptSectionnow includes a**Max Patch Size**line in the constraints block, using themaxRepoMemoryPatchSizeconstant for the maximum display valueYAML generation fix (
pkg/workflow/compiler_yaml_helpers.go)generatePlaceholderSubstitutionStepnow emits YAML double-quoted scalars with\nescape sequences for static env var values containing newlines (e.g.GH_AW_MEMORY_CONSTRAINTS), replacing the previous single-quoted multi-line strings whose newlines were folded to spaces by the YAML parserConfiguration example
Docs / spec
docs/src/content/docs/reference/repo-memory.md— documentsmax-patch-size, troubleshooting, and security notes; fixes inconsistent example (patch size now set to 100KB to match 1MB file size limit)docs/src/content/docs/reference/safe-outputs-specification.md— corrects patch size note to: default 10 KB, range 1–100 KBscratchpad/repo-memory.md— adds validation rule §5 with computation semantics and updated error format including byte countsWarning
Custom agent used: w3c-specification-writer
AI technical specification writer following W3C conventions and best practices for formal specifications
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.