Skip to content

fix(USE-001): use ERR_SYSTEM constant in generate_git_patch.cjs #18877

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 3 commits into from
Feb 28, 2026
Merged

fix(USE-001): use ERR_SYSTEM constant in generate_git_patch.cjs #18877

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2b89aa2
Initial plan
Copilot Feb 28, 2026
b28a205
fix: add E005 error code to generate_git_patch.cjs (USE-001)
Copilot Feb 28, 2026
3e56821
fix: use ERR_SYSTEM constant from error_codes.cjs in generate_git_pat…
Copilot Feb 28, 2026
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
3 changes: 2 additions & 1 deletion actions/setup/js/generate_git_patch.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const path = require("path");

const { getErrorMessage } = require("./error_helpers.cjs");
const { execGitSync } = require("./git_helpers.cjs");
const { ERR_SYSTEM } = require("./error_codes.cjs");

/**
* Debug logging helper - logs to stderr when DEBUG env var matches
Expand Down Expand Up @@ -163,7 +164,7 @@ async function generateGitPatch(branchName, baseBranch, options = {}) {
} else {
// No remote refs available - fall through to Strategy 2
debugLog(`Strategy 1 (full): No remote refs available, falling through to Strategy 2`);
throw new Error("No remote refs available for merge-base calculation");
throw new Error(`${ERR_SYSTEM}: No remote refs available for merge-base calculation`);
}
}
}
Expand Down
38 changes: 38 additions & 0 deletions actions/setup/js/generate_git_patch.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,44 @@ describe("sanitizeBranchNameForPatch", () => {
});
});

describe("generateGitPatch - standardized error codes", () => {
let originalEnv;

beforeEach(() => {
originalEnv = {
GITHUB_SHA: process.env.GITHUB_SHA,
GITHUB_WORKSPACE: process.env.GITHUB_WORKSPACE,
};
});

afterEach(() => {
Object.keys(originalEnv).forEach(key => {
if (originalEnv[key] !== undefined) {
process.env[key] = originalEnv[key];
} else {
delete process.env[key];
}
});
});

it("should fail gracefully and return a non-empty error string when no commits can be found", async () => {
const { generateGitPatch } = await import("./generate_git_patch.cjs");

process.env.GITHUB_WORKSPACE = "/tmp/nonexistent-repo";
process.env.GITHUB_SHA = "abc123";

const result = await generateGitPatch("feature-branch", "main");

Comment on lines +347 to +354
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

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

This new test doesn’t appear to exercise the specific Strategy 1 (full mode) branch where No remote refs available... is thrown: setting GITHUB_WORKSPACE to a nonexistent path causes git commands to fail earlier (similar to the existing cross-repo tests above), so it won’t cover the newly prefixed throw line. Consider creating a minimal temporary git repo with a local feature-branch but no origin/* refs (or mocking execGitSync) so the code reaches the intended fallback/throw path, or otherwise adjust the PR description/test name to reflect what’s actually being validated.

Copilot uses AI. Check for mistakes.
expect(result.success).toBe(false);
expect(result).toHaveProperty("error");
// Note: E005 is used as an internal control-flow signal in Strategy 1 (full mode)
// and is caught before reaching the final return value. The conformance check
// validates the E005 code at source level via check-safe-outputs-conformance.sh.
Comment on lines +357 to +359
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

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

The test comments reference “E005” as the internal control-flow signal, but the implementation uses the ERR_SYSTEM constant (which expands to the literal string ERR_SYSTEM, not an E### code). This makes the test documentation misleading; please update the comment to match the actual prefix being used (or remove the hard-coded code reference).

Suggested change
// Note: E005 is used as an internal control-flow signal in Strategy 1 (full mode)
// and is caught before reaching the final return value. The conformance check
// validates the E005 code at source level via check-safe-outputs-conformance.sh.
// Note: ERR_SYSTEM is used as an internal control-flow signal in Strategy 1 (full mode)
// and is caught before reaching the final return value. The conformance check
// validates the ERR_SYSTEM prefix at source level via check-safe-outputs-conformance.sh.

Copilot uses AI. Check for mistakes.
expect(typeof result.error).toBe("string");
expect(result.error.length).toBeGreaterThan(0);
});
});

describe("getPatchPath", () => {
it("should return correct path format", async () => {
const { getPatchPath } = await import("./generate_git_patch.cjs");
Expand Down
Loading