-
Notifications
You must be signed in to change notification settings - Fork 432
fix: improve agent assignment failure issue reports #41392
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -243,6 +243,7 @@ function buildFailureMatchCategories(options) { | |
| * @param {boolean} options.hasDailyAICExceeded | ||
| * @param {boolean} options.aiCreditsRateLimitError | ||
| * @param {boolean} options.maxAICreditsExceeded | ||
| * @param {boolean} options.hasAssignmentErrors | ||
| * @returns {string} | ||
| */ | ||
| function buildFailureIssueTitle(options) { | ||
|
|
@@ -260,6 +261,7 @@ function buildFailureIssueTitle(options) { | |
| if (options.hasMissingSafeOutputs) return `[aw] ${workflowName} produced no safe outputs`; | ||
| if (options.hasMissingTool) return `[aw] ${workflowName} is missing required tool`; | ||
| if (options.hasMissingData) return `[aw] ${workflowName} is missing required data`; | ||
| if (options.hasAssignmentErrors) return `[aw] ${workflowName} failed to assign agent`; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
💡 Impact and suggested fixIn Concrete scenario: the job times out while GitHub is retrying agent assignment (bad token, rate limit, service blip). Both
...instead of:
The user debugs phantom timeouts instead of checking their Suggested fix — move the check to before function buildFailureIssueTitle(options) {
const { workflowName } = options;
if (options.hasAssignmentErrors) return `[aw] ${workflowName} failed to assign agent`; // check root causes first
if (options.hasDailyAICExceeded) return `[aw] ${workflowName} exceeded daily AI credits budget`;
// ... rest unchanged
}Also add a test case that sets both |
||
| return `[aw] ${workflowName} failed`; | ||
| } | ||
|
|
||
|
|
@@ -2058,9 +2060,9 @@ function buildAssignmentErrorsContext(assignmentErrors) { | |
| } | ||
|
|
||
| context += "\nTo resolve this, verify the agent token and Copilot access configuration:\n"; | ||
| context += "- Configure a valid `GH_AW_AGENT_TOKEN` with `issues: write` and `pull-requests: write` plus active Copilot entitlement\n"; | ||
| context += "- If your org supports it, add `permissions: { copilot-requests: write }` to use org inference without a personal token\n"; | ||
| context += "- Docs: https://github.github.com/gh-aw/reference/engines/#github-copilot-default\n\n"; | ||
| context += "- Configure a valid `GH_AW_AGENT_TOKEN` as a fine-grained PAT with **Agent tasks: read and write** permission (GitHub App installation tokens are not supported)\n"; | ||
| context += "- Ensure Copilot coding agent is enabled for this repository and a Copilot Business or Enterprise subscription is active\n"; | ||
| context += "- Docs: https://github.github.com/gh-aw/reference/copilot-cloud-agent/#authentication\n\n"; | ||
|
|
||
| return context; | ||
| } | ||
|
|
@@ -2990,6 +2992,7 @@ async function main() { | |
| hasDailyAICExceeded, | ||
| aiCreditsRateLimitError, | ||
| maxAICreditsExceeded, | ||
| hasAssignmentErrors, | ||
| }); | ||
| const failureCategories = buildFailureMatchCategories({ | ||
| agentConclusion, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -87,6 +87,7 @@ describe("handle_agent_failure", () => { | |
| hasDailyAICExceeded: false, | ||
| aiCreditsRateLimitError: false, | ||
| maxAICreditsExceeded: false, | ||
| hasAssignmentErrors: false, | ||
| }; | ||
|
|
||
| const cases = [ | ||
|
|
@@ -103,6 +104,7 @@ describe("handle_agent_failure", () => { | |
| { flag: "hasMissingSafeOutputs", expected: "[aw] Test Workflow produced no safe outputs" }, | ||
| { flag: "hasMissingTool", expected: "[aw] Test Workflow is missing required tool" }, | ||
| { flag: "hasMissingData", expected: "[aw] Test Workflow is missing required data" }, | ||
| { flag: "hasAssignmentErrors", expected: "[aw] Test Workflow failed to assign agent" }, | ||
| ]; | ||
|
|
||
| it.each(cases)("returns expected title for isolated $flag", ({ flag, expected }) => { | ||
|
|
@@ -1339,8 +1341,9 @@ describe("handle_agent_failure", () => { | |
| expect(result).toContain("Issue #42 (agent: copilot): Bad credentials"); | ||
| expect(result).toContain("PR #7 (agent: copilot): copilot coding agent is not available for this repository"); | ||
| expect(result).toContain("GH_AW_AGENT_TOKEN"); | ||
| expect(result).toContain("copilot-requests: write"); | ||
| expect(result).toContain("https://github.github.com/gh-aw/reference/engines/#github-copilot-default"); | ||
| expect(result).toContain("Agent tasks: read and write"); | ||
| expect(result).not.toContain("copilot-requests: write"); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [/tdd] Only The implementation also deleted 💡 Suggested additionsexpect(result).not.toContain('copilot-requests: write');
expect(result).not.toContain('issues: write');
expect(result).not.toContain('pull-requests: write');All three strings were removed from the implementation; all three deserve a guard. |
||
| expect(result).toContain("https://github.github.com/gh-aw/reference/copilot-cloud-agent/#authentication"); | ||
| }); | ||
| }); | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[/diagnose]
hasAssignmentErrorsis placed last inbuildFailureIssueTitle— just before the generic fallback — even thoughbuildFailureMatchCategoriestreats it as a high-priority category (second, afterisTimedOut).In practice this is safe because
hasMissingSafeOutputsis only set whenagentConclusion === "success", and assignment errors driveagentConclusionto"failure". But the ordering relies on a non-obvious invariant worth documenting inline.💡 Suggested comment
This makes the ordering explicit and guards against future flags that might share the
"failure"conclusion slipping in ahead of this check.