Skip to content

Align repo-memory max-patch-size pre-check with push diff semantics - #48106

Merged
pelikhan merged 5 commits into
mainfrom
copilot/fix-pre-check-measure-push-diff
Jul 26, 2026
Merged

Align repo-memory max-patch-size pre-check with push diff semantics#48106
pelikhan merged 5 commits into
mainfrom
copilot/fix-pre-check-measure-push-diff

Conversation

Copilot AI commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

repo-memory documents max-patch-size as a per-push diff limit, but MCP pre-check logic enforced it as total folder size. This caused small incremental updates to fail once memory accumulated.

  • Use diff-based patch sizing in both enforcement points

    • Extracted shared patch-size logic into actions/setup/js/repo_memory_patch_size.cjs.
    • push_repo_memory.cjs now calls the shared helper instead of duplicating diff parsing.
  • Fix MCP pre-check behavior

    • safe_outputs_handlers.cjs now computes staged additions size from git diff --cached in the memory repo and compares that against max_patch_size * 1.2 (same effective limit model as push gate).
    • Removed total-folder-size enforcement as the deciding factor for max-patch-size.
    • Updated messages to report patch additions size rather than folder size.
  • Regression coverage for the documented contract

    • Updated safe_outputs_handlers.test.cjs to validate:
      • large folder + tiny staged diff passes
      • oversized staged diff fails
    • Test setup now initializes a local git repo for repo-memory pre-check scenarios.
// shared logic now used by both paths
const patchSizeBytes = getStagedPatchAdditionsSizeBytes({
  execGitSyncFn: execGitSync,
  cwd: memoryDir, // optional in push path
});

if (patchSizeBytes > effectiveMaxPatchSize) {
  // fail on diff size, not accumulated folder size
}

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 14.5 AIC · ⌖ 8.46 AIC · ⊞ 7.1K ·
Comment /souschef to run again


Generated by 👨‍🍳 PR Sous Chef · gpt54 · 17.5 AIC · ⌖ 8.92 AIC · ⊞ 7.1K ·
Comment /souschef to run again

Copilot AI and others added 2 commits July 26, 2026 03:43
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix repo-memory max-patch-size pre-check to measure push diff Align repo-memory max-patch-size pre-check with push diff semantics Jul 26, 2026
Copilot AI requested a review from pelikhan July 26, 2026 03:52
@pelikhan
pelikhan marked this pull request as ready for review July 26, 2026 03:53
Copilot AI review requested due to automatic review settings July 26, 2026 03:53

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

Aligns repo-memory pre-checks with per-push diff semantics.

Changes:

  • Extracts shared staged-additions sizing.
  • Replaces total-folder enforcement with staged-diff validation.
  • Adds regression coverage and release-workflow guidance.
Show a summary per file
File Description
safe_outputs_handlers.test.cjs Tests diff-based validation.
safe_outputs_handlers.cjs Validates staged additions.
repo_memory_patch_size.cjs Adds shared sizing helper.
push_repo_memory.cjs Uses shared helper.
SKILL.md Adds release workflow reference.

Review details

Tip

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

Comments suppressed due to low confidence (1)

actions/setup/js/repo_memory_patch_size.cjs:27

  • The default diff suppresses binary contents as Binary files ... differ, so this helper reports zero additions for binary files. Since repo-memory allows all extensions by default, multiple binary files can now bypass the patch limit (while remaining under the per-file/count limits). Force a textual diff, and add a binary regression case, so binary additions contribute to the gate.
  const patchContent = execGitSyncFn(["diff", "--cached"], { stdio: "pipe", cwd });
  • Files reviewed: 5/5 changed files
  • Comments generated: 1
  • Review effort level: Medium

Comment on lines +11 to +16
function getAddedPatchSizeBytesFromDiff(patchContent) {
return patchContent
.split("\n")
.filter(line => line.startsWith("+") && !line.startsWith("+++"))
.reduce((sum, line) => sum + Buffer.byteLength(line + "\n", "utf8"), 0);
}
@github-actions

github-actions Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Test Quality Sentinel completed test quality analysis.

@github-actions

github-actions Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

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

@github-actions

github-actions Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

PR Code Quality Reviewer completed the code quality review.

@github-actions

github-actions Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

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

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot please run the pr-finisher skill, address the unresolved review comments and failed checks below, and rerun checks once the branch is up to date.

Unresolved review feedback:

Failed checks:

Generated by 👨🍳 PR Sous Chef

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 14.5 AIC · ⌖ 8.46 AIC · ⊞ 7.1K ·
Comment /souschef to run again

…d repo_memory_patch_size.cjs to SAFE_OUTPUTS_FILES

- Replace filter-based +++ exclusion with a state-machine that tracks
  whether parsing is inside a hunk. File headers (+++  before the first
  @@ of each file) are correctly ignored; content lines that begin with
  ++ (appearing as +++ inside a hunk) are now correctly counted.
- Add repo_memory_patch_size.cjs to SAFE_OUTPUTS_FILES in setup.sh so
  the transitive-dependency test passes (safe_outputs_handlers.cjs
  requires it at runtime).

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

Copilot AI commented Jul 26, 2026

Copy link
Copy Markdown
Contributor Author

@copilot please run the pr-finisher skill, address the unresolved review comments and failed checks below, and rerun checks once the branch is up to date.

Unresolved review feedback:...

Addressed both issues in the latest commit:

Review comment (hunk-state tracking): Replaced the filter-based !line.startsWith("+++") approach with a state machine in getAddedPatchSizeBytesFromDiff. The function now tracks an inHunk flag (set on @@ lines, reset on diff lines). File headers (+++ b/...) outside a hunk are excluded; content lines starting with ++ that appear as +++ inside a hunk are correctly counted.

CI failure (shard 4/4): Added repo_memory_patch_size.cjs to SAFE_OUTPUTS_FILES in setup.sh. The transitive-dependency test was failing because safe_outputs_handlers.cjs requires this new file at runtime.

Copilot AI requested a review from gh-aw-bot July 26, 2026 05:55
@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot please run the pr-finisher skill, address the remaining unresolved review feedback and failed checks, and rerun checks once the branch is up to date.

Unresolved review feedback:

Failed checks:

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

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 17.5 AIC · ⌖ 8.92 AIC · ⊞ 7.1K ·
Comment /souschef to run again

Copilot AI commented Jul 26, 2026

Copy link
Copy Markdown
Contributor Author

@copilot please run the pr-finisher skill, address the remaining unresolved review feedback and failed checks, and rerun checks once the branch is up to date.

Unresolved review feedback:...

Both issues were already addressed in commit 2820953 from the previous session:

  • Review feedback: getAddedPatchSizeBytesFromDiff now uses a hunk-state machine (inHunk flag set on @@, reset on diff ) instead of the !startsWith("+++") filter.
  • CI failure (shard 4/4): repo_memory_patch_size.cjs was added to SAFE_OUTPUTS_FILES in setup.sh.

The referenced failed runs (30186794434, 30186875359) predate that commit. The latest CJS run on the fix commit (30190167597) shows action_required with 0 jobs — it needs workflow approval rather than a code fix. All 193 local tests pass.

@github-actions

Copy link
Copy Markdown
Contributor

🤖 PR Triage — §30192186368

Field Value
Category bug
Risk 🟡 medium
Score 68 / 100 (impact 38 + urgency 18 + quality 12)
Action ⚡ fast_track

Why: Fixes repo-memory pre-check enforcing total folder size instead of diff (patch) size — causing incremental memory updates to fail once memory accumulates. Copilot check passing. Regression tests updated.

Next: Full CI confirmation, then fast-track merge.

Generated by 🔧 PR Triage Agent · sonnet46 · 48.8 AIC · ⌖ 8.75 AIC · ⊞ 5.7K ·

@pelikhan
pelikhan merged commit b5a7dcd into main Jul 26, 2026
9 checks passed
@pelikhan
pelikhan deleted the copilot/fix-pre-check-measure-push-diff branch July 26, 2026 08:34
@github-actions

Copy link
Copy Markdown
Contributor

🎉 This pull request is included in a new release.

Release: v0.83.4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make repo-memory max-patch-size pre-check measure the push diff, not the whole folder

4 participants