fix: use net diff size (additions − deletions) for push memory patch size enforcement

[Copilot]

max-patch-size enforcement for repo-memory pushes was measuring raw + line bytes from git diff --cached. When a memory file (e.g. JSON) is fully regenerated each run, git emits the entire old content as - lines and entire new content as + lines — causing patchSizeBytes to equal the full file size regardless of how small the logical change was.

Changes

• repo_memory_patch_size.cjs — replace getAddedPatchSizeBytesFromDiff (sums + lines) with getPatchDiffSizeBytes (net: max(0, additions_bytes − deletions_bytes)); rename export to getStagedPatchDiffSizeBytes
• push_repo_memory.cjs / safe_outputs_handlers.cjs — update import + call site; update error/debug messages to reflect "diff" vs "additions"
• repo_memory_patch_size.test.cjs (new) — unit tests for append-only, complete rewrite (key regression), shrink/clamp, multi-file, header line exclusion

// Before: 50 KB JSON rewritten with a 100-byte change → reported 50 KB
additions += Buffer.byteLength(line + "\n", "utf8"); // only + lines

// After: same scenario → reports ~0 KB net growth
return Math.max(0, additions - deletions);

---

run: https://github.com/github/gh-aw/actions/runs/30787664739

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 11.6 AIC · ⌖ 7.08 AIC · ⊞ 8.3K · ◷
_{Comment /souschef to run again}

---

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 6.91 AIC · ⌖ 6.54 AIC · ⊞ 8.3K · ◷
_{Comment /souschef to run again}

[github-actions]

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

[github-actions]

✅ PR Code Quality Reviewer completed the code quality review.

[github-actions]

✅ Design Decision Gate 🏗️ completed the design decision gate check.

No ADR enforcement needed: PR #49894 does not have the 'implementation' label and has 0 new lines of code in business logic directories (threshold: 100).

[github-actions]

✅ Test Quality Sentinel completed test quality analysis.

Test Quality Sentinel completed analysis: 83/100 score, all 9 tests are design-level behavioral contracts with 0% implementation tests. Ready for approval.

[Copilot]

Pull request overview

Updates repo-memory patch enforcement to measure net byte growth rather than raw additions.

Changes:

• Calculates additions minus deletions, clamped to zero.
• Updates repo-memory validation messages and call sites.
• Adds unit coverage for diff-size calculation.

Show a summary per file

File	Description
actions/setup/js/repo_memory_patch_size.cjs	Implements net diff-size calculation.
actions/setup/js/repo_memory_patch_size.test.cjs	Tests calculation behavior and Git invocation.
actions/setup/js/push_repo_memory.cjs	Uses and reports net diff size.
actions/setup/js/safe_outputs_handlers.cjs	Updates safe-output validation and messages.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Suppressed comments (1)

actions/setup/js/safe_outputs_handlers.cjs:1748

• The corresponding success-path test at safe_outputs_handlers.test.cjs:2723 still asserts that this message contains "patch additions". This wording change therefore makes the existing test fail; update the assertion to "patch diff".

            message: `Memory validation passed: ${files.length} file(s), ${totalSizeKb} KB total content, ` + `${patchSizeKb} KB patch diff (${patchSizeBytes} bytes) (limit: ${effectiveMaxKb} KB / ${effectiveMaxPatchSize} bytes).`,

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

[pelikhan]

@copilot patch size is computed in other JavaScript files. Find and reuse helper.

[github-actions]

Skills-Based Review 🧠

Applied /tdd and /diagnosing-bugs — requesting changes on one critical test-file defect.

📋 Key Themes & Highlights

Key Issues

• Broken test file: repo_memory_patch_size.test.cjs uses ES module import syntax in a CommonJS (.cjs) file. Node.js will throw a parse error and none of the tests will run, leaving the regression guard empty.

Positive Highlights

• ✅ Root cause correctly identified and fixed: net diff size (additions − deletions, clamped to 0) prevents false positives on full rewrites
• ✅ Good JSDoc explaining the new algorithm and its tradeoffs
• ✅ Test cases cover the key scenarios: new file, append, rewrite, shrink, multi-file, and header-line exclusion
• ✅ Clean rename (getStagedPatchAdditionsSizeBytes → getStagedPatchDiffSizeBytes) applied consistently across all three call sites

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 34 AIC · ⌖ 10.7 AIC · ⊞ 7.1K
_{Comment /matt to run again}

[github-actions]

The fix correctly changes raw additions counting to net diff bytes (additions − deletions, clamped to 0), which avoids false over-reporting when a file is completely regenerated with similar-sized content.

Code is clean, the logic is sound, and the test suite covers the key regression scenario (same-size rewrite), append-only, shrink/clamp-to-zero, multi-file, and header-line exclusion. Test pattern is consistent with the rest of the test suite.

🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 30 AIC · ⌖ 7.46 AIC · ⊞ 5.4K

[github-actions]

🧪 Test Quality Sentinel Report

✅ Test Quality Score: 83/100 — Excellent

Analyzed 9 test(s): 9 design, 0 implementation, 0 violation(s).

📊 Metrics (9 tests)

Metric	Value
Analyzed	9 (Go: 0, JS: 9)
✅ Design	9 (100%)
⚠️ Implementation	0 (0%)
Edge/error coverage	7 (77.8%)
Duplicate clusters	0
Inflation	Yes (3.5:1)
🚨 Violations	0

Test	File	Classification	Notes
returns 0 for an empty diff	repo_memory_patch_size.test.cjs:10	Design (edge case)	Empty input boundary
counts only addition bytes for a new file	repo_memory_patch_size.test.cjs:14	Design (core)	New file scenario (first push)
returns a small net value when file is appended	repo_memory_patch_size.test.cjs:22	Design (core)	Append scenario (incremental)
returns near-zero for complete rewrite with same size	repo_memory_patch_size.test.cjs:31	Design (bug fix)	The key fix: net additions − deletions vs. raw additions
returns 0 when deletions exceed additions	repo_memory_patch_size.test.cjs:48	Design (edge case)	Content shrinks (clamped to 0)
handles multiple files in one diff	repo_memory_patch_size.test.cjs:65	Design (multi-file)	Aggregation across files
does not count +++ file header lines	repo_memory_patch_size.test.cjs:85	Design (edge case)	Header exclusion rule
calls git diff --cached and returns net patch diff size	repo_memory_patch_size.test.cjs:102	Design (integration)	Mocks external git call
passes the cwd option to execGitSyncFn	repo_memory_patch_size.test.cjs:116	Design (contract)	Argument passing contract

Key Observations

✅ All tests are behavioral contract tests — each verifies a user-facing guarantee:

• Net diff size calculation (not raw additions)
• Edge cases (empty, shrinking, rewrites)
• Integration with git's --cached flag
• Option propagation to underlying functions

✅ Comprehensive coverage of the bug fix: Test #4 explicitly validates the core issue — that a complete file rewrite with the same size should contribute ~0 bytes to the patch budget, not the entire new content.

⚠️ Test inflation (3.5:1) — 128 test lines for 37 production lines. This is acceptable because all tests are legitimate and necessary:

• The production code is a pure utility function (two small functions)
• Diff parsing has multiple edge cases (empty files, rewrites, header lines, multi-file diffs)
• Each test case is independent and non-redundant
• This reflects the complexity of the contract, not over-testing

Verdict

✅ Passed. 0% implementation tests (threshold: 30%). Test Quality Score: 83/100.

All 9 tests are design-level behavioral contracts. The bug fix is well-defended by edge-case and regression tests. The test inflation ratio reflects legitimate edge-case coverage, not artificial test bloat.

🧪 Test quality analysis by Test Quality Sentinel · haiku45 · 21.6 AIC · ⌖ 6.48 AIC · ⊞ 8.5K · ◷
_{Comment /review to run again}

[github-actions]

✅ Test Quality Sentinel: 83/100 (Excellent). All 9 tests are design-level behavioral contracts with 0% implementation tests (threshold: 30%). See detailed report in comment above.

[pelikhan]

@copilot run pr-finisher skill

[github-actions]

Verdict: Request changes — the net-diff redesign reopens the size guard to bypasses it was meant to prevent.

💡 Themes

• Cross-file masking: the net byte total is accumulated globally across the whole staged diff rather than per file, so an unrelated deletion in one file can offset a large addition in another, letting the reported patch size collapse toward zero while substantial new content is pushed.
• Same-size rewrite bypass: the net formula (additions - deletions, clamped at 0) treats any equal-size full-file rewrite as ~0 bytes regardless of how much or how different the new content is, which is a much larger regression than the original false-positive it fixes (over-counting regenerated files).
• Test coverage gap: the new unit tests validate single-file same-size rewrites well, but do not cover the multi-file masking scenario that is the main risk introduced by moving from per-line addition counting to a diff-wide net total.

The intent of the fix (avoid over-counting a benign same-size rewrite) is reasonable, but the implementation trades a false-positive for a false-negative that undermines the guard's actual security purpose (bounding pushed content size). Recommend computing net size per-file (flushing accumulators per diff --git boundary) and/or using additions directly for full-file replacements before merging.

🔎 Code quality review by PR Code Quality Reviewer · auto · 82.3 AIC · ⌖ 3.81 AIC · ⊞ 7.9K
_{Comment /review to run again}

[Copilot]

@copilot patch size is computed in other JavaScript files. Find and reuse helper.

Done in commit refactor: move getPatchDiffSizeBytes/getStagedPatchDiffSizeBytes into git_patch_utils.cjs.

Moved both getPatchDiffSizeBytes and getStagedPatchDiffSizeBytes from the purpose-specific repo_memory_patch_size.cjs into git_patch_utils.cjs (the shared module for patch/git utilities). Removed repo_memory_patch_size.cjs, updated imports in push_repo_memory.cjs and safe_outputs_handlers.cjs, moved the unit tests into git_patch_utils.test.cjs, and fixed the stale "patch additions" assertions in safe_outputs_handlers.test.cjs.

done

[pelikhan]

@copilot run pr-finisher skill

[Copilot]

@copilot run pr-finisher skill

PR is ready to merge:

• ✅ Checks (CI) — all 8 jobs green on latest commit (fb07027): JS Tests (4/4 shards), impacted-js-tests, lint-js, js-typecheck, Artifact API Integration
• ✅ Mergeable — OPEN, not draft, mergeable_state: clean
• ✅ Reviews — all 5 review threads have substantive replies; the CHANGES_REQUESTED review from github-actions[bot] was on the old commit and its concerns (per-file clamping, missing cross-file masking test, setup.sh stale entry) were all addressed in commits 84b457d and fb07027

The 5 review threads still need to be dismissed/resolved by a maintainer (4 are outdated); I can't resolve them without gh auth in this environment.

[gh-aw-bot]

@copilot Maintainer-ready pass: please run the pr-finisher skill and confirm the remaining cleanup is complete.

Outstanding unresolved review threads (newest first):

• github-actions — multi-file masking coverage: fix: use net diff size (additions − deletions) for push memory patch size enforcement #49894 (comment)
• github-actions — same-size rewrite guard semantics: fix: use net diff size (additions − deletions) for push memory patch size enforcement #49894 (comment)
• github-actions — cross-file masking bug: fix: use net diff size (additions − deletions) for push memory patch size enforcement #49894 (comment)
• github-actions — CommonJS test-file syntax: fix: use net diff size (additions − deletions) for push memory patch size enforcement #49894 (comment)
• copilot-pull-request-reviewer — renamed error text assertion: fix: use net diff size (additions − deletions) for push memory patch size enforcement #49894 (comment)

All current checks on the latest commit are green. After confirming, a maintainer can resolve the threads and dismiss any stale github-actions review.

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 11.6 AIC · ⌖ 7.08 AIC · ⊞ 8.3K · ◷
_{Comment /souschef to run again}

[Copilot]

@copilot Maintainer-ready pass: please run the pr-finisher skill and confirm the remaining cleanup is complete.

Outstanding unresolved review threads (newest first):...

All 5 review threads are now resolved. CI is fully green on the latest commit (fb07027) — JS Tests (4/4 shards), impacted-js-tests, lint-js, js-typecheck, and Artifact API Integration all pass. The PR is OPEN, not draft, and mergeable_state: clean. Ready to merge.

[github-actions]

Hey @app/copilot-swe-agent 👋 — thanks for fixing the patch-size enforcement bug! This PR looks great and is ready for review.

What's working well:

• ✅ Clear focus — replaces the flawed "additions-only" logic with net diff size (additions − deletions), clamped per-file
• ✅ Comprehensive tests — new unit tests cover append-only, complete rewrites, shrinkage, multi-file scenarios, and per-file clamping
• ✅ Strong description — the PR body clearly explains the regression (50 KB JSON rewrite with 100-byte change falsely reported as 50 KB)
• ✅ Consistent refactoring — function renamed (getStagedPatchAdditionsSizeBytes → getStagedPatchDiffSizeBytes), module consolidated into git_patch_utils.cjs, all call sites updated
• ✅ Error messages updated — all user-facing messages reflect the new "diff" terminology

Why this matters: Previously, files that were completely regenerated with similar-sized content (e.g., regenerated JSON state) would report their entire file size toward the push limit, even if only a few bytes actually changed. This fix removes that false positive by using the net change instead.

Generated by ✅ Contribution Check · auto · 57.3 AIC · ⌖ 4.5 AIC · ⊞ 8.8K · ◷

[github-actions]

Triage Result

Category: bug | Risk: medium | Score: 78/100 (impact 40, urgency 22, quality 16)

Recommended action: fast_track

CI green (all checks success), clean mergeable state, sizable diff (251+/65-, 7 files) touching push-memory patch-size enforcement logic. Ready for expedited human review.

Generated by 🔧 PR Triage Agent · auto · 27.4 AIC · ⌖ 2.42 AIC · ⊞ 8K · ◷

[gh-aw-bot]

@copilot Please refresh this PR for maintainer review: update the branch if needed, rerun the pr-finisher skill, and summarize any remaining blockers.

Run: https://github.com/github/gh-aw/actions/runs/30806305280

Current status: latest checks are green and the PR appears mergeable, but please confirm the remaining cleanup is complete and summarize whether any maintainer action is still needed.

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 6.91 AIC · ⌖ 6.54 AIC · ⊞ 8.3K · ◷
_{Comment /souschef to run again}

[Copilot]

@copilot Please refresh this PR for maintainer review: update the branch if needed, rerun the pr-finisher skill, and summarize any remaining blockers.

Run: https://github.com/github/gh-aw/actions/r...

Refreshed now.

• Branch update: not needed (mergeable_state: clean, PR open, not draft).
• pr-finisher pass: complete.
• Review cleanup: all 5 review threads are already resolved.
• CI check refresh: run 30806305280 is success (8/8 jobs, 0 failed job logs).

Remaining blockers: none found from the agent side. Maintainer action is only the normal final approval/merge step.