Increase Metrics Collector repo-memory patch cap to prevent push_repo_memory gate failures (#49970)

* Initial plan

* Increase Metrics Collector repo-memory patch cap

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

* Plan patch-size computation fix

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

* Scope repo-memory patch size to managed diff

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

* Apply remaining changes

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

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Co-authored-by: Peli de Halleux <pelikhan@users.noreply.github.com>

Author: 4 people

.github/workflows/metrics-collector.lock.yml

@@ -27,6 +27,7 @@ tools:
   repo-memory:
     branch-name: memory/meta-orchestrators
     file-glob: "metrics/**"
+    max-patch-size: 131072 # 128KB - handles large daily metrics snapshots without patch-size gate failures
 timeout-minutes: 15
 safe-outputs:
   noop:

.github/workflows/metrics-collector.md

@@ -498,10 +498,20 @@ async function main() {
     }
   }
 
-  // Check if we have any changes to commit
+  // Build literal pathspecs from the relative paths of files to copy.
+  // The :(literal) magic prefix tells Git to treat each entry as a plain string,
+  // preventing glob expansion or pathspec-magic interpretation (e.g. :(top),
+  // wildcards) even when a filename happens to contain those characters.
+  const literalPathspecs = Array.from(new Set(filesToCopy.map(file => `:(literal)${file.relativePath}`))).sort();
+
+  // Check if we have any changes to commit, scoped to managed memory files only.
   let changedFileCount = 0;
   try {
-    const status = execGitSync(["status", "--porcelain"]);
+    const statusArgs = ["status", "--porcelain"];
+    if (literalPathspecs.length > 0) {
+      statusArgs.push("--", ...literalPathspecs);
+    }
+    const status = execGitSync(statusArgs, { cwd: workspaceDir });
     const changedEntries = status
       .split("\n")
       .map(line => line.trim())
@@ -534,7 +544,13 @@ async function main() {
   // sparse-checkout, causing a plain "git add ." to silently skip or reject
   // files on the first run for a new memory branch.
   try {
-    execGitSync(["add", "--sparse", "."], { stdio: "inherit" });
+    const addArgs = ["add", "--sparse"];
+    if (literalPathspecs.length > 0) {
+      addArgs.push("--", ...literalPathspecs);
+    } else {
+      addArgs.push(".");
+    }
+    execGitSync(addArgs, { stdio: "inherit", cwd: workspaceDir });
   } catch (error) {
     core.setFailed(`Failed to stage changes: ${getErrorMessage(error)}`);
     return;
@@ -546,7 +562,7 @@ async function main() {
   // (e.g. a regenerated JSON object) from being counted as "entire source code size"
   // even though only a small portion of the data actually changed.
   try {
-    const patchSizeBytes = getStagedPatchDiffSizeBytes({ execGitSyncFn: execGitSync });
+    const patchSizeBytes = getStagedPatchDiffSizeBytes({ execGitSyncFn: execGitSync, cwd: workspaceDir });
     const patchSizeKb = Math.ceil(patchSizeBytes / 1024);
     const maxPatchSizeKb = Math.floor(maxPatchSize / 1024);
     // Allow 20% overhead to account for git diff format (headers, context lines, etc.)
@@ -559,7 +575,7 @@ async function main() {
       // Add per-file diff stats to diagnose what's causing the large patch
       // (e.g. a full rewrite of an accumulated history file shows old + new content in the diff)
       try {
-        const diffStat = execGitSync(["diff", "--cached", "--stat"], { stdio: "pipe" });
+        const diffStat = execGitSync(["diff", "--cached", "--stat"], { stdio: "pipe", cwd: workspaceDir });
         core.warning(`Patch content breakdown (git diff --stat):\n${diffStat}`);
       } catch (statError) {
         core.warning(`Could not retrieve diff stat: ${getErrorMessage(statError)}`);

actions/setup/js/push_repo_memory.cjs

@@ -1073,13 +1073,36 @@ describe("push_repo_memory.cjs - shell injection security tests", () => {
       const scriptPath = path.join(import.meta.dirname, "push_repo_memory.cjs");
       const scriptContent = fs.readFileSync(scriptPath, "utf8");
 
-      // Must use "git add --sparse ." to stage files regardless of sparse-checkout state.
-      expect(scriptContent).toContain('"add", "--sparse", "."');
+      // Must use git add --sparse with literal pathspec staging so only managed memory paths are included.
+      expect(scriptContent).toContain('"add", "--sparse"');
+      expect(scriptContent).toContain('addArgs.push("--", ...literalPathspecs)');
 
       // Must NOT use plain "git add ." which breaks under sparse-checkout.
       expect(scriptContent).not.toContain('"add", "."');
     });
 
+    it("should encode pathspecs as :(literal) to prevent Git glob/magic interpretation (source check)", () => {
+      // Regression test for: filenames with glob metacharacters or pathspec-magic prefixes
+      // (e.g. ":(top)foo", "metrics/*.json") being interpreted as Git pathspecs rather
+      // than literal paths, which could cause git status/add to match files outside the
+      // managed memory scope.
+      //
+      // Fix: all artifact-derived paths are wrapped with the :(literal) magic prefix so
+      // Git treats them as plain strings regardless of their content.
+
+      const fs = require("fs");
+      const path = require("path");
+
+      const scriptPath = path.join(import.meta.dirname, "push_repo_memory.cjs");
+      const scriptContent = fs.readFileSync(scriptPath, "utf8");
+
+      // Must build literalPathspecs using the :(literal) magic prefix.
+      expect(scriptContent).toContain("`:(literal)${");
+      // Must pass literalPathspecs (not raw paths) to git status and git add.
+      expect(scriptContent).toContain("literalPathspecs");
+      expect(scriptContent).not.toContain("changedPathspecs");
+    });
+
     it("should safely handle malicious branch names", () => {
       // Test that malicious branch names would be rejected by git, not executed as shell commands
       const maliciousBranchNames = [
@@ -1562,7 +1585,8 @@ describe("push_repo_memory.cjs - changed-file limit checks", () => {
     const scriptContent = nodeFs.readFileSync(scriptPath, "utf8");
 
     expect(scriptContent).toContain("changedFileCount");
-    expect(scriptContent).toContain('execGitSync(["status", "--porcelain"])');
+    expect(scriptContent).toContain('["status", "--porcelain"]');
+    expect(scriptContent).toContain('statusArgs.push("--", ...literalPathspecs)');
     expect(scriptContent).toContain("Too many changed files");
     expect(scriptContent).not.toContain("if (filesToCopy.length > maxFileCount)");
     expect(scriptContent).toContain("if (changedFileCount > maxFileCount)");