Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .changeset/patch-suppress-repo-memory-errors.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

39 changes: 24 additions & 15 deletions actions/setup/js/git_helpers.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,18 @@ const { ERR_SYSTEM } = require("./error_codes.cjs");
/**
* Safely execute git command using spawnSync with args array to prevent shell injection
* @param {string[]} args - Git command arguments
* @param {Object} options - Spawn options
* @param {Object} options - Spawn options; set suppressLogs: true to avoid core.error annotations for expected failures
* @returns {string} Command output
* @throws {Error} If command fails
*/
function execGitSync(args, options = {}) {
// Extract suppressLogs before spreading into spawnSync options.
// suppressLogs is a custom control flag (not a valid spawnSync option) that
// routes failure details to core.debug instead of core.error, preventing
// spurious GitHub Actions error annotations for expected failures (e.g., when
// a branch does not yet exist).
const { suppressLogs = false, ...spawnOptions } = options;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice pattern — destructuring suppressLogs before spreading ...spawnOptions ensures the custom flag never leaks into spawnSync options. Clean and safe.

// Log the git command being executed for debugging (but redact credentials)
const gitCommand = `git ${args
.map(arg => {
Expand All @@ -23,25 +30,29 @@ function execGitSync(args, options = {}) {
})
.join(" ")}`;

if (typeof core !== "undefined" && core.debug) {
core.debug(`Executing git command: ${gitCommand}`);
}
core.debug(`Executing git command: ${gitCommand}`);

const result = spawnSync("git", args, {
encoding: "utf8",
...options,
...spawnOptions,
});

if (result.error) {
if (typeof core !== "undefined" && core.error) {
core.error(`Git command failed with error: ${result.error.message}`);
}
// Spawn-level errors (e.g. ENOENT, EACCES) are always unexpected — log
// via core.error regardless of suppressLogs.
core.error(`Git command failed with error: ${result.error.message}`);
throw result.error;
}

if (result.status !== 0) {
const errorMsg = `${ERR_SYSTEM}: ${result.stderr || `Git command failed with status ${result.status}`}`;
if (typeof core !== "undefined" && core.error) {
if (suppressLogs) {
core.debug(`Git command failed (expected): ${gitCommand}`);
core.debug(`Exit status: ${result.status}`);
if (result.stderr) {
core.debug(`Stderr: ${result.stderr}`);
}
} else {
core.error(`Git command failed: ${gitCommand}`);
core.error(`Exit status: ${result.status}`);
if (result.stderr) {
Expand All @@ -51,12 +62,10 @@ function execGitSync(args, options = {}) {
throw new Error(errorMsg);
}

if (typeof core !== "undefined" && core.debug) {
if (result.stdout) {
core.debug(`Git command output: ${result.stdout.substring(0, 200)}${result.stdout.length > 200 ? "..." : ""}`);
} else {
core.debug("Git command completed successfully with no output");
}
if (result.stdout) {
core.debug(`Git command output: ${result.stdout.substring(0, 200)}${result.stdout.length > 200 ? "..." : ""}`);
} else {
core.debug("Git command completed successfully with no output");
}

return result.stdout;
Expand Down
76 changes: 75 additions & 1 deletion actions/setup/js/git_helpers.test.cjs
Original file line number Diff line number Diff line change
@@ -1,6 +1,27 @@
import { describe, it, expect } from "vitest";
import { describe, it, expect, beforeEach, afterEach } from "vitest";

describe("git_helpers.cjs", () => {
let originalCore;

beforeEach(() => {
// Save existing core and provide a minimal no-op stub if not already set,
// matching the guarantee that shim.cjs provides in production.
originalCore = global.core;
if (!global.core) {
global.core = {
debug: () => {},
info: () => {},
warning: () => {},
error: () => {},
setFailed: () => {},
};
}
});

afterEach(() => {
global.core = originalCore;
});

describe("execGitSync", () => {
it("should export execGitSync function", async () => {
const { execGitSync } = await import("./git_helpers.cjs");
Expand Down Expand Up @@ -68,6 +89,59 @@ describe("git_helpers.cjs", () => {
expect(result).toContain("git version");
});

it("should not call core.error when suppressLogs is true", async () => {
const { execGitSync } = await import("./git_helpers.cjs");

const errorLogs = [];
const debugLogs = [];
const originalCore = global.core;
global.core = {
debug: msg => debugLogs.push(msg),
error: msg => errorLogs.push(msg),
};

try {
// Use an invalid git command that will fail
try {
execGitSync(["rev-parse", "nonexistent-branch-that-does-not-exist"], { suppressLogs: true });
} catch (e) {
// Expected to fail
}

// core.error should NOT have been called
expect(errorLogs).toHaveLength(0);
// core.debug should have captured the failure details including exit status
expect(debugLogs.some(log => log.includes("Git command failed (expected)"))).toBe(true);
expect(debugLogs.some(log => log.includes("Exit status:"))).toBe(true);
} finally {
global.core = originalCore;
}
});

it("should call core.error when suppressLogs is false (default)", async () => {
const { execGitSync } = await import("./git_helpers.cjs");

const errorLogs = [];
const originalCore = global.core;
global.core = {
debug: () => {},
error: msg => errorLogs.push(msg),
};

try {
try {
execGitSync(["rev-parse", "nonexistent-branch-that-does-not-exist"]);
} catch (e) {
// Expected to fail
}

// core.error should have been called
expect(errorLogs.length).toBeGreaterThan(0);
} finally {
global.core = originalCore;
}
});

it("should redact credentials from logged commands", async () => {
const { execGitSync } = await import("./git_helpers.cjs");

Expand Down
8 changes: 4 additions & 4 deletions actions/setup/js/push_repo_memory.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ async function main() {
// This is necessary because checkout was configured with sparse-checkout
core.info(`Disabling sparse checkout...`);
try {
execGitSync(["sparse-checkout", "disable"], { stdio: "pipe" });
execGitSync(["sparse-checkout", "disable"], { stdio: "pipe", suppressLogs: true });
} catch (error) {
// Ignore if sparse checkout wasn't enabled
core.info("Sparse checkout was not enabled or already disabled");
Expand All @@ -140,7 +140,7 @@ async function main() {

// Try to fetch the branch
try {
execGitSync(["fetch", repoUrl, `${branchName}:${branchName}`], { stdio: "pipe" });
execGitSync(["fetch", repoUrl, `${branchName}:${branchName}`], { stdio: "pipe", suppressLogs: true });
execGitSync(["checkout", branchName], { stdio: "inherit" });
core.info(`Checked out existing branch: ${branchName}`);
} catch (fetchError) {
Expand Down Expand Up @@ -377,10 +377,10 @@ async function main() {
// Pull with merge strategy (ours wins on conflicts)
core.info(`Pulling latest changes from ${branchName}...`);
try {
execGitSync(["pull", "--no-rebase", "-X", "ours", repoUrl, branchName], { stdio: "inherit" });
execGitSync(["pull", "--no-rebase", "-X", "ours", repoUrl, branchName], { stdio: "inherit", suppressLogs: true });
} catch (error) {
// Pull might fail if branch doesn't exist yet or on conflicts - this is acceptable
core.warning(`Pull failed (this may be expected): ${getErrorMessage(error)}`);
core.info(`Pull failed (this is expected when branch does not exist yet): ${getErrorMessage(error)}`);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Downgrading from core.warning to core.info here is correct — a pull failing on first run (no branch yet) is expected behavior, not a warning. This prevents unnecessary orange annotations in the Actions UI.

}

// Push changes
Expand Down