Skip to content

Report agent assignment failures without blocking safe_outputs job #13719

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 2 commits into from
Feb 4, 2026
Merged

Report agent assignment failures without blocking safe_outputs job #13719

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
89f3991
Initial plan
Copilot Feb 4, 2026
7981b3b
Fix assignment failure error handling - report but don't fail safe_ou…
Copilot Feb 4, 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
5 changes: 3 additions & 2 deletions actions/setup/js/assign_to_agent.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -426,9 +426,10 @@ async function main() {
core.setOutput("assignment_errors", assignmentErrors);
core.setOutput("assignment_error_count", failureCount.toString());

// Fail if any assignments failed (but not if they were skipped)
// Log assignment failures but don't fail the job
// The conclusion job will report these failures in the agent failure issue/comment
if (failureCount > 0) {
core.setFailed(`Failed to assign ${failureCount} agent(s)`);
core.warning(`Failed to assign ${failureCount} agent(s) - errors will be reported in conclusion job`);
}
}

Expand Down
14 changes: 7 additions & 7 deletions actions/setup/js/assign_to_agent.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ describe("assign_to_agent", () => {
await eval(`(async () => { ${assignToAgentScript}; await main(); })()`);

expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining('Agent "unsupported-agent" is not supported'));
expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("Failed to assign 1 agent(s)"));
expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("Failed to assign 1 agent(s)"));
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

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

Given the PR’s goal is to stop failing the safe_outputs job on agent-assignment errors, this test now only asserts that a warning is logged but no longer checks that core.setFailed is not called. To lock in the new behavior and prevent regressions where assignment failures start failing the job again, consider adding an assertion like expect(mockCore.setFailed).not.toHaveBeenCalled() here (and/or in at least one other failure-path test).

Copilot uses AI. Check for mistakes.
});

it("should handle invalid issue numbers", async () => {
Expand Down Expand Up @@ -350,7 +350,7 @@ describe("assign_to_agent", () => {
await eval(`(async () => { ${assignToAgentScript}; await main(); })()`);

expect(mockCore.error).toHaveBeenCalledWith(expect.stringContaining("Failed to assign agent"));
expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("Failed to assign 1 agent(s)"));
expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("Failed to assign 1 agent(s)"));
});

it("should handle 502 errors as success", async () => {
Expand Down Expand Up @@ -650,7 +650,7 @@ describe("assign_to_agent", () => {
await eval(`(async () => { ${assignToAgentScript}; await main(); })()`);

expect(mockCore.error).toHaveBeenCalledWith("Cannot specify both issue_number and pull_number in the same assign_to_agent item");
expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("Failed to assign 1 agent(s)"));
expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("Failed to assign 1 agent(s)"));
});

it("should auto-resolve issue number from context when not provided (triggering target)", async () => {
Expand Down Expand Up @@ -752,7 +752,7 @@ describe("assign_to_agent", () => {

// Should fail because target "*" requires explicit issue_number or pull_number
expect(mockCore.error).toHaveBeenCalled();
expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("Failed to assign 1 agent(s)"));
expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("Failed to assign 1 agent(s)"));
});

it("should accept agent when in allowed list", async () => {
Expand Down Expand Up @@ -816,7 +816,7 @@ describe("assign_to_agent", () => {

expect(mockCore.info).toHaveBeenCalledWith("Allowed agents: other-agent");
expect(mockCore.error).toHaveBeenCalledWith(expect.stringContaining('Agent "copilot" is not in the allowed list'));
expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("Failed to assign 1 agent(s)"));
expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("Failed to assign 1 agent(s)"));

// Should not have made any GraphQL calls since validation failed early
expect(mockGithub.graphql).not.toHaveBeenCalled();
Expand Down Expand Up @@ -923,7 +923,7 @@ describe("assign_to_agent", () => {

// Should error and fail
expect(mockCore.error).toHaveBeenCalledWith(expect.stringContaining("Failed to assign agent"));
expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("Failed to assign 1 agent(s)"));
expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("Failed to assign 1 agent(s)"));
});

it("should handle ignore-if-error when 'Resource not accessible' error", async () => {
Expand Down Expand Up @@ -971,6 +971,6 @@ describe("assign_to_agent", () => {

// Should error and fail (not skipped because it's not an auth error)
expect(mockCore.error).toHaveBeenCalledWith(expect.stringContaining("Failed to assign agent"));
expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("Failed to assign 1 agent(s)"));
expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("Failed to assign 1 agent(s)"));
});
});
Loading