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
3 changes: 3 additions & 0 deletions .github/workflows/security-guard.lock.yml

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

7 changes: 7 additions & 0 deletions actions/setup/js/checkout_pr_branch.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ async function main() {

if (!pullRequest) {
core.info("No pull request context available, skipping checkout");
core.setOutput("checkout_pr_success", "true");
return;
}

Expand All @@ -41,9 +42,15 @@ async function main() {

core.info(`✅ Successfully checked out PR #${prNumber}`);
}

// Set output to indicate successful checkout
core.setOutput("checkout_pr_success", "true");
} catch (error) {
const errorMsg = getErrorMessage(error);

// Set output to indicate checkout failure
core.setOutput("checkout_pr_success", "false");

// Load and render step summary template
const templatePath = "/opt/gh-aw/prompts/pr_checkout_failure.md";
const template = fs.readFileSync(templatePath, "utf8");
Expand Down
37 changes: 37 additions & 0 deletions actions/setup/js/checkout_pr_branch.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ describe("checkout_pr_branch.cjs", () => {
mockCore = {
info: vi.fn(),
setFailed: vi.fn(),
setOutput: vi.fn(),
summary: {
addRaw: vi.fn().mockReturnThis(),
write: vi.fn().mockResolvedValue(undefined),
Expand Down Expand Up @@ -327,4 +328,40 @@ If the pull request is still open, verify that:
expect(mockExec.exec).toHaveBeenCalledWith("git", ["fetch", "origin", longBranchName]);
});
});

describe("checkout output", () => {
it("should set output to true on successful checkout (pull_request event)", async () => {
await runScript();

expect(mockCore.setOutput).toHaveBeenCalledWith("checkout_pr_success", "true");
expect(mockCore.setFailed).not.toHaveBeenCalled();
});

it("should set output to true on successful checkout (comment event)", async () => {
mockContext.eventName = "issue_comment";

await runScript();

expect(mockCore.setOutput).toHaveBeenCalledWith("checkout_pr_success", "true");
expect(mockCore.setFailed).not.toHaveBeenCalled();
});

it("should set output to false on checkout failure", async () => {
mockExec.exec.mockRejectedValueOnce(new Error("checkout failed"));

await runScript();

expect(mockCore.setOutput).toHaveBeenCalledWith("checkout_pr_success", "false");
expect(mockCore.setFailed).toHaveBeenCalledWith("Failed to checkout PR branch: checkout failed");
});

it("should set output to true when no PR context", async () => {
mockContext.payload.pull_request = null;

await runScript();

expect(mockCore.setOutput).toHaveBeenCalledWith("checkout_pr_success", "true");
expect(mockCore.setFailed).not.toHaveBeenCalled();
});
});
});
9 changes: 9 additions & 0 deletions actions/setup/js/handle_agent_failure.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -252,11 +252,13 @@ async function main() {
const secretVerificationResult = process.env.GH_AW_SECRET_VERIFICATION_RESULT || "";
const assignmentErrors = process.env.GH_AW_ASSIGNMENT_ERRORS || "";
const assignmentErrorCount = process.env.GH_AW_ASSIGNMENT_ERROR_COUNT || "0";
const checkoutPRSuccess = process.env.GH_AW_CHECKOUT_PR_SUCCESS || "";

core.info(`Agent conclusion: ${agentConclusion}`);
core.info(`Workflow name: ${workflowName}`);
core.info(`Secret verification result: ${secretVerificationResult}`);
core.info(`Assignment error count: ${assignmentErrorCount}`);
core.info(`Checkout PR success: ${checkoutPRSuccess}`);

// Check if there are assignment errors (regardless of agent job status)
const hasAssignmentErrors = parseInt(assignmentErrorCount, 10) > 0;
Expand All @@ -279,6 +281,13 @@ async function main() {
return;
}

// Check if the failure was due to PR checkout (e.g., PR was merged and branch deleted)
// If checkout_pr_success is "false", skip creating an issue as this is expected behavior
if (agentConclusion === "failure" && checkoutPRSuccess === "false") {
core.info("Skipping failure handling - failure was due to PR checkout (likely PR merged)");
Comment on lines +285 to +287
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

The condition should also check that there are no assignment errors or missing safe outputs before skipping issue creation. If a PR checkout fails but there are also assignment errors or missing safe outputs, an issue should still be created to report those problems. The condition should be: if (agentConclusion === "failure" && checkoutPRSuccess === "false" && !hasAssignmentErrors && !hasMissingSafeOutputs)

Suggested change
// If checkout_pr_success is "false", skip creating an issue as this is expected behavior
if (agentConclusion === "failure" && checkoutPRSuccess === "false") {
core.info("Skipping failure handling - failure was due to PR checkout (likely PR merged)");
// If checkout_pr_success is "false" and there are no assignment errors or missing safe outputs,
// skip creating an issue as this is expected behavior
if (agentConclusion === "failure" && checkoutPRSuccess === "false" && !hasAssignmentErrors && !hasMissingSafeOutputs) {
core.info("Skipping failure handling - failure was due to PR checkout (likely PR merged) and no additional issues were detected");

Copilot uses AI. Check for mistakes.
return;
}

const { owner, repo } = context.repo;

// Try to find a pull request for the current branch
Expand Down
92 changes: 92 additions & 0 deletions actions/setup/js/handle_agent_failure.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -1111,4 +1111,96 @@ When prompted, instruct the agent to debug this workflow failure.`;
expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("Found existing parent issue #5"));
});
});

describe("checkout PR failure via output", () => {
it("should skip issue creation when checkout_pr_success is false", async () => {
// Set the checkout PR failure environment variable
process.env.GH_AW_CHECKOUT_PR_SUCCESS = "false";

await main();

// Verify that no issue was created
expect(mockGithub.rest.issues.create).not.toHaveBeenCalled();
expect(mockGithub.rest.issues.createComment).not.toHaveBeenCalled();
expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("Skipping failure handling - failure was due to PR checkout"));
});

it("should create issue when checkout_pr_success is true", async () => {
// Set the checkout PR success environment variable
process.env.GH_AW_CHECKOUT_PR_SUCCESS = "true";

mockGithub.rest.search.issuesAndPullRequests
.mockResolvedValueOnce({
// First search: PR search (no PR found)
data: { total_count: 0, items: [] },
})
.mockResolvedValueOnce({
// Second search: parent issue
data: { total_count: 0, items: [] },
})
.mockResolvedValueOnce({
// Third search: failure issue
data: { total_count: 0, items: [] },
});

mockGithub.rest.issues.create
.mockResolvedValueOnce({
data: { number: 1, html_url: "https://example.com/1", node_id: "I_1" },
})
.mockResolvedValueOnce({
data: { number: 2, html_url: "https://example.com/2", node_id: "I_2" },
});

mockGithub.graphql = vi.fn().mockResolvedValue({
addSubIssue: {
issue: { id: "I_1", number: 1 },
subIssue: { id: "I_2", number: 2 },
},
});

await main();

// Verify issue was created
expect(mockGithub.rest.issues.create).toHaveBeenCalled();
});

it("should create issue when checkout_pr_success is not set", async () => {
// Don't set GH_AW_CHECKOUT_PR_SUCCESS (workflow without PR checkout)
delete process.env.GH_AW_CHECKOUT_PR_SUCCESS;

mockGithub.rest.search.issuesAndPullRequests
.mockResolvedValueOnce({
// First search: PR search (no PR found)
data: { total_count: 0, items: [] },
})
.mockResolvedValueOnce({
// Second search: parent issue
data: { total_count: 0, items: [] },
})
.mockResolvedValueOnce({
// Third search: failure issue
data: { total_count: 0, items: [] },
});

mockGithub.rest.issues.create
.mockResolvedValueOnce({
data: { number: 1, html_url: "https://example.com/1", node_id: "I_1" },
})
.mockResolvedValueOnce({
data: { number: 2, html_url: "https://example.com/2", node_id: "I_2" },
});

mockGithub.graphql = vi.fn().mockResolvedValue({
addSubIssue: {
issue: { id: "I_1", number: 1 },
subIssue: { id: "I_2", number: 2 },
},
});

await main();

// Verify issue was created (normal failure handling)
expect(mockGithub.rest.issues.create).toHaveBeenCalled();
});
});
Comment on lines +1115 to +1205
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

Missing test coverage for the edge case where both checkout fails (checkout_pr_success is false) AND there are assignment errors or missing safe outputs. In this scenario, an issue should still be created to report the assignment errors/missing outputs, but there's no test verifying this behavior.

Copilot uses AI. Check for mistakes.
});
6 changes: 6 additions & 0 deletions pkg/workflow/compiler_activation_jobs.go
Original file line number Diff line number Diff line change
Expand Up @@ -734,6 +734,12 @@ func (c *Compiler) buildMainJob(data *WorkflowData, activationJobCreated bool) (
outputs["has_patch"] = "${{ steps.collect_output.outputs.has_patch }}"
}

// Add checkout_pr_success output to track PR checkout status
// This is used by the conclusion job to skip failure handling when checkout fails
// (e.g., when PR is merged and branch is deleted)
outputs["checkout_pr_success"] = "${{ steps.checkout-pr.outputs.checkout_pr_success || 'true' }}"
compilerActivationJobsLog.Print("Added checkout_pr_success output")

// Build job-level environment variables for safe outputs
var env map[string]string
if data.SafeOutputs != nil {
Expand Down
3 changes: 3 additions & 0 deletions pkg/workflow/notify_comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,9 @@ func (c *Compiler) buildConclusionJob(data *WorkflowData, mainJobName string, sa
agentFailureEnvVars = append(agentFailureEnvVars, fmt.Sprintf(" GH_AW_SECRET_VERIFICATION_RESULT: ${{ needs.%s.outputs.secret_verification_result }}\n", mainJobName))
}

// Add checkout_pr_success to detect PR checkout failures (e.g., PR merged and branch deleted)
agentFailureEnvVars = append(agentFailureEnvVars, fmt.Sprintf(" GH_AW_CHECKOUT_PR_SUCCESS: ${{ needs.%s.outputs.checkout_pr_success }}\n", mainJobName))

// Pass assignment error outputs from safe_outputs job if assign-to-agent is configured
if data.SafeOutputs != nil && data.SafeOutputs.AssignToAgent != nil {
agentFailureEnvVars = append(agentFailureEnvVars, " GH_AW_ASSIGNMENT_ERRORS: ${{ needs.safe_outputs.outputs.assign_to_agent_assignment_errors }}\n")
Expand Down
1 change: 1 addition & 0 deletions pkg/workflow/pr.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ func (c *Compiler) generatePRReadyForReviewCheckout(yaml *strings.Builder, data

// Always add the step with a condition that checks if PR context is available
yaml.WriteString(" - name: Checkout PR branch\n")
yaml.WriteString(" id: checkout-pr\n")

// Build condition that checks if github.event.pull_request exists
// This will be true for pull_request events and comment events on PRs
Expand Down
Loading