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
48 changes: 48 additions & 0 deletions actions/setup/js/assign_to_agent.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,8 @@ async function main() {
issue_number: item.issue_number || null,
pull_number: item.pull_number || null,
agent: agentName,
owner: null,
repo: null,
success: false,
error: repoResult.error,
});
Expand All @@ -248,6 +250,8 @@ async function main() {
issue_number: item.issue_number,
pull_number: item.pull_number ?? null,
agent: agentName,
owner: effectiveOwner,
repo: effectiveRepo,
success: false,
error: resolvedTarget.errorMessage || `Failed to resolve issue target: ${item.issue_number}`,
});
Expand Down Expand Up @@ -286,6 +290,8 @@ async function main() {
issue_number: item.issue_number || null,
pull_number: item.pull_number || null,
agent: agentName,
owner: effectiveOwner,
repo: effectiveRepo,
success: false,
error: pullRequestRepoValidation.error,
});
Expand All @@ -310,6 +316,8 @@ async function main() {
issue_number: item.issue_number || null,
pull_number: item.pull_number || null,
agent: agentName,
owner: effectiveOwner,
repo: effectiveRepo,
success: false,
error: `Failed to fetch pull request repository ID for ${itemPullRequestRepo}`,
});
Expand Down Expand Up @@ -338,6 +346,8 @@ async function main() {
issue_number: item.issue_number || null,
pull_number: item.pull_number || null,
agent: agentName,
owner: effectiveOwner,
repo: effectiveRepo,
success: false,
error: targetResult.error,
});
Expand All @@ -359,6 +369,8 @@ async function main() {
issue_number: issueNumber,
pull_number: pullNumber,
agent: agentName,
owner: effectiveOwner,
repo: effectiveRepo,
success: false,
error: `Invalid ${type} number: ${number}`,
});
Expand All @@ -372,6 +384,8 @@ async function main() {
issue_number: issueNumber,
pull_number: pullNumber,
agent: agentName,
owner: effectiveOwner,
repo: effectiveRepo,
success: false,
error: `Unsupported agent: ${agentName}`,
});
Expand All @@ -385,6 +399,8 @@ async function main() {
issue_number: issueNumber,
pull_number: pullNumber,
agent: agentName,
owner: effectiveOwner,
repo: effectiveRepo,
success: false,
error: `Agent not allowed: ${agentName}`,
});
Expand Down Expand Up @@ -438,6 +454,8 @@ async function main() {
issue_number: issueNumber,
pull_number: pullNumber,
agent: agentName,
owner: effectiveOwner,
repo: effectiveRepo,
success: true,
});
continue;
Expand Down Expand Up @@ -471,6 +489,8 @@ async function main() {
issue_number: issueNumber,
pull_number: pullNumber,
agent: agentName,
owner: effectiveOwner,
repo: effectiveRepo,
success: true,
});
} catch (error) {
Expand All @@ -487,6 +507,8 @@ async function main() {
issue_number: issueNumber,
pull_number: pullNumber,
agent: agentName,
owner: effectiveOwner,
repo: effectiveRepo,
success: true, // Treat as success when ignored
skipped: true,
});
Expand All @@ -509,6 +531,8 @@ async function main() {
issue_number: issueNumber,
pull_number: pullNumber,
agent: agentName,
owner: effectiveOwner,
repo: effectiveRepo,
success: false,
error: errorMessage,
});
Expand Down Expand Up @@ -573,6 +597,30 @@ async function main() {

await core.summary.addRaw(summaryContent).write();

// Post failure comments on each issue/PR that failed assignment.
// This is needed because the agent may have already posted an "assigned to agent" comment
// before the assignment step runs. If assignment fails, users need to see the actual failure
// status directly on their issue/PR, not just in the general failure tracking issue.
for (const r of results) {
if (r.success || r.skipped || !r.owner || !r.repo || (!r.issue_number && !r.pull_number)) {
continue;
Comment on lines +604 to +606
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

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

The failure-comment post-processing relies on each results entry having owner/repo, but not all results.push(...) paths add these fields (e.g., the early validation error when both issue_number and pull_number are set stores a result without owner/repo). This makes the results schema inconsistent and can silently skip posting the corrective failure comment. Consider normalizing the results object shape so every entry always includes owner and repo (set to null when unknown), or adjust the comment-posting loop to handle that specific case explicitly.

Copilot uses AI. Check for mistakes.
}
const failedNumber = r.issue_number || r.pull_number;
const failedType = r.issue_number ? "issue" : "pull request";
try {
await github.rest.issues.createComment({
owner: r.owner,
repo: r.repo,
issue_number: failedNumber,
body: `⚠️ **Assignment failed**: Failed to assign ${r.agent} coding agent to this ${failedType}.\n\nError: ${r.error}`,
});
core.info(`Posted failure comment on ${failedType} #${failedNumber} in ${r.owner}/${r.repo}`);
} catch (commentError) {
// Best-effort: log but don't fail the step if we can't post the comment
core.warning(`Failed to post failure comment on ${failedType} #${failedNumber}: ${getErrorMessage(commentError)}`);
}
}

// Set outputs
const assignedAgents = results
.filter(r => r.success && !r.skipped)
Expand Down
131 changes: 131 additions & 0 deletions actions/setup/js/assign_to_agent.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ const mockContext = {

const mockGithub = {
graphql: vi.fn(),
rest: {
issues: {
createComment: vi.fn().mockResolvedValue({ data: { id: 12345 } }),
},
},
};

global.core = mockCore;
Expand All @@ -47,6 +52,9 @@ describe("assign_to_agent", () => {
// Reset mockGithub.graphql to ensure no lingering mock implementations
mockGithub.graphql = vi.fn();

// Reset mockGithub.rest.issues.createComment
mockGithub.rest.issues.createComment = vi.fn().mockResolvedValue({ data: { id: 12345 } });

delete process.env.GH_AW_AGENT_OUTPUT;
delete process.env.GH_AW_SAFE_OUTPUTS_STAGED;
delete process.env.GH_AW_AGENT_DEFAULT;
Expand Down Expand Up @@ -929,6 +937,16 @@ describe("assign_to_agent", () => {
// Should error and fail
expect(mockCore.error).toHaveBeenCalledWith(expect.stringContaining("Failed to assign agent"));
expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("Failed to assign 1 agent(s)"));

// Should post a failure comment on the issue with all required properties
expect(mockGithub.rest.issues.createComment).toHaveBeenCalledWith(
expect.objectContaining({
owner: "test-owner",
repo: "test-repo",
issue_number: 42,
body: expect.stringMatching(/Assignment failed.*Bad credentials/s),
})
);
});

it("should handle ignore-if-error when 'Resource not accessible' error", async () => {
Expand Down Expand Up @@ -979,6 +997,119 @@ describe("assign_to_agent", () => {
expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("Failed to assign 1 agent(s)"));
});

it("should not post failure comment on success", async () => {
setAgentOutput({
items: [
{
type: "assign_to_agent",
issue_number: 42,
agent: "copilot",
},
],
errors: [],
});

mockGithub.graphql
.mockResolvedValueOnce({
repository: {
suggestedActors: {
nodes: [{ login: "copilot-swe-agent", id: "MDQ6VXNlcjE=" }],
},
},
})
.mockResolvedValueOnce({
repository: {
issue: {
id: "I_abc123",
assignees: { nodes: [] },
},
},
})
.mockResolvedValueOnce({
replaceActorsForAssignable: { __typename: "ReplaceActorsForAssignablePayload" },
});

await eval(`(async () => { ${assignToAgentScript}; await main(); })()`);

// Should NOT post a failure comment on success
expect(mockGithub.rest.issues.createComment).not.toHaveBeenCalled();
});

it("should post failure comment on single failed assignment", async () => {
setAgentOutput({
items: [{ type: "assign_to_agent", issue_number: 11, agent: "copilot" }],
errors: [],
});

// Fail all assignments with auth error
const authError = new Error("Bad credentials");
mockGithub.graphql.mockRejectedValue(authError);

await eval(`(async () => { ${assignToAgentScript}; await main(); })()`);

// Should post a failure comment for the failed issue with all required properties
expect(mockGithub.rest.issues.createComment).toHaveBeenCalledTimes(1);
expect(mockGithub.rest.issues.createComment).toHaveBeenCalledWith(
expect.objectContaining({
owner: "test-owner",
repo: "test-repo",
issue_number: 11,
body: expect.stringMatching(/Assignment failed.*Bad credentials/s),
})
);
});

it("should not post failure comment when ignore-if-error skips the assignment", async () => {
process.env.GH_AW_AGENT_IGNORE_IF_ERROR = "true";
setAgentOutput({
items: [
{
type: "assign_to_agent",
issue_number: 42,
agent: "copilot",
},
],
errors: [],
});

// Simulate authentication error (will be skipped by ignore-if-error)
const authError = new Error("Bad credentials");
mockGithub.graphql.mockRejectedValue(authError);

await eval(`(async () => { ${assignToAgentScript}; await main(); })()`);

// Should NOT post a failure comment since it was skipped
expect(mockGithub.rest.issues.createComment).not.toHaveBeenCalled();
});

it("should still set outputs and log warning when failure comment post fails", async () => {
setAgentOutput({
items: [
{
type: "assign_to_agent",
issue_number: 42,
agent: "copilot",
},
],
errors: [],
});

const authError = new Error("Bad credentials");
mockGithub.graphql.mockRejectedValue(authError);

// Simulate failure to post comment
mockGithub.rest.issues.createComment.mockRejectedValue(new Error("Could not post comment"));

await eval(`(async () => { ${assignToAgentScript}; await main(); })()`);

// Should still set the assignment_error outputs even if comment fails
expect(mockCore.setOutput).toHaveBeenCalledWith("assignment_error_count", "1");
expect(mockCore.setOutput).toHaveBeenCalledWith("assignment_errors", expect.stringContaining("Bad credentials"));

// Should warn about failure to post comment (best-effort)
expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("Failed to post failure comment"));
});

it.skip("should add 10-second delay between multiple agent assignments", async () => {
// Note: This test is skipped because testing actual delays with eval() is complex.
// The implementation has been manually verified to include the delay logic.
Expand Down
Loading