fix: fall back to repo default_branch instead of hardcoded "main" (#1143)
* fix: fall back to repo default_branch instead of hardcoded "main" When no explicit base_branch input is provided, the action previously fell back to a hardcoded "main", which fails on repositories whose default branch is named differently (e.g. "master", "develop"). This reads repository.default_branch from the GitHub event payload (populated once in parseGitHubContext) and uses it as the fallback in all three callsites: agent/index.ts, run.ts, and update-comment-link.ts. Explicit env/input precedence is preserved; "main" remains only as a last-resort defensive fallback if the payload somehow lacks the field. * test: drop unused BASE_BRANCH env handling from default_branch test agent/index.ts no longer reads process.env.BASE_BRANCH directly (it now goes through context.inputs.baseBranch which is set on the mock context), so saving/clearing/restoring that env var in the regression test is dead code.
This commit is contained in:
parent
408a40e7c2
commit
c281e17d7f
@ -229,8 +229,8 @@ async function run() {
|
|||||||
// Restore them from the base branch before the CLI reads them.
|
// Restore them from the base branch before the CLI reads them.
|
||||||
//
|
//
|
||||||
// We read pull_request.base.ref from the payload directly because agent
|
// We read pull_request.base.ref from the payload directly because agent
|
||||||
// mode's branchInfo.baseBranch defaults to "main" rather than the PR's
|
// mode's branchInfo.baseBranch defaults to the repo's default branch rather
|
||||||
// actual target (agent/index.ts). For issue_comment on a PR the payload
|
// than the PR's actual target (agent/index.ts). For issue_comment on a PR the payload
|
||||||
// lacks base.ref, so we fall back to the mode-provided value — tag mode
|
// lacks base.ref, so we fall back to the mode-provided value — tag mode
|
||||||
// fetches it from GraphQL; agent mode on issue_comment is an edge case
|
// fetches it from GraphQL; agent mode on issue_comment is an edge case
|
||||||
// that at worst restores from the wrong trusted branch (still secure).
|
// that at worst restores from the wrong trusted branch (still secure).
|
||||||
@ -312,7 +312,7 @@ async function run() {
|
|||||||
commentId,
|
commentId,
|
||||||
githubToken,
|
githubToken,
|
||||||
claudeBranch,
|
claudeBranch,
|
||||||
baseBranch: baseBranch || "main",
|
baseBranch: baseBranch || context.repository.default_branch || "main",
|
||||||
triggerUsername: context.actor,
|
triggerUsername: context.actor,
|
||||||
context,
|
context,
|
||||||
octokit,
|
octokit,
|
||||||
|
|||||||
@ -253,7 +253,8 @@ async function run() {
|
|||||||
commentId: parseInt(process.env.CLAUDE_COMMENT_ID!),
|
commentId: parseInt(process.env.CLAUDE_COMMENT_ID!),
|
||||||
githubToken,
|
githubToken,
|
||||||
claudeBranch: process.env.CLAUDE_BRANCH,
|
claudeBranch: process.env.CLAUDE_BRANCH,
|
||||||
baseBranch: process.env.BASE_BRANCH || "main",
|
baseBranch:
|
||||||
|
process.env.BASE_BRANCH || context.repository.default_branch || "main",
|
||||||
triggerUsername: process.env.TRIGGER_USERNAME,
|
triggerUsername: process.env.TRIGGER_USERNAME,
|
||||||
context,
|
context,
|
||||||
octokit,
|
octokit,
|
||||||
|
|||||||
@ -79,6 +79,7 @@ type BaseContext = {
|
|||||||
owner: string;
|
owner: string;
|
||||||
repo: string;
|
repo: string;
|
||||||
full_name: string;
|
full_name: string;
|
||||||
|
default_branch?: string;
|
||||||
};
|
};
|
||||||
actor: string;
|
actor: string;
|
||||||
inputs: {
|
inputs: {
|
||||||
@ -140,6 +141,7 @@ export function parseGitHubContext(): GitHubContext {
|
|||||||
owner: context.repo.owner,
|
owner: context.repo.owner,
|
||||||
repo: context.repo.repo,
|
repo: context.repo.repo,
|
||||||
full_name: `${context.repo.owner}/${context.repo.repo}`,
|
full_name: `${context.repo.owner}/${context.repo.repo}`,
|
||||||
|
default_branch: context.payload.repository?.default_branch,
|
||||||
},
|
},
|
||||||
actor: context.actor,
|
actor: context.actor,
|
||||||
inputs: {
|
inputs: {
|
||||||
|
|||||||
@ -85,15 +85,15 @@ export async function prepareAgentMode({
|
|||||||
|
|
||||||
// Check for branch info from environment variables (useful for auto-fix workflows)
|
// Check for branch info from environment variables (useful for auto-fix workflows)
|
||||||
const claudeBranch = process.env.CLAUDE_BRANCH || undefined;
|
const claudeBranch = process.env.CLAUDE_BRANCH || undefined;
|
||||||
const baseBranch =
|
const defaultBranch = context.repository.default_branch || "main";
|
||||||
process.env.BASE_BRANCH || context.inputs.baseBranch || "main";
|
const baseBranch = context.inputs.baseBranch || defaultBranch;
|
||||||
|
|
||||||
// Detect current branch from GitHub environment
|
// Detect current branch from GitHub environment
|
||||||
const currentBranch =
|
const currentBranch =
|
||||||
claudeBranch ||
|
claudeBranch ||
|
||||||
process.env.GITHUB_HEAD_REF ||
|
process.env.GITHUB_HEAD_REF ||
|
||||||
process.env.GITHUB_REF_NAME ||
|
process.env.GITHUB_REF_NAME ||
|
||||||
"main";
|
defaultBranch;
|
||||||
|
|
||||||
// Get our GitHub MCP servers config
|
// Get our GitHub MCP servers config
|
||||||
const ourMcpConfig = await prepareMcpConfig({
|
const ourMcpConfig = await prepareMcpConfig({
|
||||||
|
|||||||
@ -36,6 +36,7 @@ const defaultRepository = {
|
|||||||
owner: "test-owner",
|
owner: "test-owner",
|
||||||
repo: "test-repo",
|
repo: "test-repo",
|
||||||
full_name: "test-owner/test-repo",
|
full_name: "test-owner/test-repo",
|
||||||
|
default_branch: "main",
|
||||||
};
|
};
|
||||||
|
|
||||||
type MockContextOverrides = Omit<Partial<ParsedGitHubContext>, "inputs"> & {
|
type MockContextOverrides = Omit<Partial<ParsedGitHubContext>, "inputs"> & {
|
||||||
|
|||||||
@ -88,7 +88,7 @@ describe("Agent Mode", () => {
|
|||||||
expect(result.claudeArgs).toBe("--model claude-sonnet-4 --max-turns 10");
|
expect(result.claudeArgs).toBe("--model claude-sonnet-4 --max-turns 10");
|
||||||
expect(result.claudeArgs).not.toContain("--mcp-config");
|
expect(result.claudeArgs).not.toContain("--mcp-config");
|
||||||
|
|
||||||
// Verify return structure - should use "main" as fallback when no env vars set
|
// Verify return structure - should fall back to repository.default_branch when no env vars set
|
||||||
expect(result).toEqual({
|
expect(result).toEqual({
|
||||||
commentId: undefined,
|
commentId: undefined,
|
||||||
branchInfo: {
|
branchInfo: {
|
||||||
@ -108,6 +108,60 @@ describe("Agent Mode", () => {
|
|||||||
process.env.GITHUB_REF_NAME = originalRefName;
|
process.env.GITHUB_REF_NAME = originalRefName;
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("prepare falls back to repository.default_branch when not 'main'", async () => {
|
||||||
|
const contextWithDevelop = createMockAutomationContext({
|
||||||
|
eventName: "workflow_dispatch",
|
||||||
|
repository: {
|
||||||
|
owner: "test-owner",
|
||||||
|
repo: "test-repo",
|
||||||
|
full_name: "test-owner/test-repo",
|
||||||
|
default_branch: "develop",
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
// Save and clear env vars that would otherwise override the fallback
|
||||||
|
const originalClaudeBranch = process.env.CLAUDE_BRANCH;
|
||||||
|
const originalHeadRef = process.env.GITHUB_HEAD_REF;
|
||||||
|
const originalRefName = process.env.GITHUB_REF_NAME;
|
||||||
|
delete process.env.CLAUDE_BRANCH;
|
||||||
|
delete process.env.GITHUB_HEAD_REF;
|
||||||
|
delete process.env.GITHUB_REF_NAME;
|
||||||
|
|
||||||
|
const mockOctokit = {
|
||||||
|
rest: {
|
||||||
|
users: {
|
||||||
|
getAuthenticated: mock(() =>
|
||||||
|
Promise.resolve({
|
||||||
|
data: { login: "test-user", id: 12345, type: "User" },
|
||||||
|
}),
|
||||||
|
),
|
||||||
|
getByUsername: mock(() =>
|
||||||
|
Promise.resolve({
|
||||||
|
data: { login: "test-user", id: 12345, type: "User" },
|
||||||
|
}),
|
||||||
|
),
|
||||||
|
},
|
||||||
|
},
|
||||||
|
} as any;
|
||||||
|
|
||||||
|
const result = await prepareAgentMode({
|
||||||
|
context: contextWithDevelop,
|
||||||
|
octokit: mockOctokit,
|
||||||
|
githubToken: "test-token",
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(result.branchInfo.baseBranch).toBe("develop");
|
||||||
|
expect(result.branchInfo.currentBranch).toBe("develop");
|
||||||
|
|
||||||
|
// Restore env vars
|
||||||
|
if (originalClaudeBranch !== undefined)
|
||||||
|
process.env.CLAUDE_BRANCH = originalClaudeBranch;
|
||||||
|
if (originalHeadRef !== undefined)
|
||||||
|
process.env.GITHUB_HEAD_REF = originalHeadRef;
|
||||||
|
if (originalRefName !== undefined)
|
||||||
|
process.env.GITHUB_REF_NAME = originalRefName;
|
||||||
|
});
|
||||||
|
|
||||||
test("prepare rejects bot actors without allowed_bots", async () => {
|
test("prepare rejects bot actors without allowed_bots", async () => {
|
||||||
const contextWithPrompts = createMockAutomationContext({
|
const contextWithPrompts = createMockAutomationContext({
|
||||||
eventName: "workflow_dispatch",
|
eventName: "workflow_dispatch",
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user