Skip to content

Version issue-creation REST calls in handle_agent_failure #38017

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 3 commits into from
Jun 9, 2026
+53 −1
Merged

Version issue-creation REST calls in handle_agent_failure #38017

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fff214a
Add API version header to failure issue create calls
Copilot Jun 9, 2026
15be720
Test API-version header on failure issue creation
Copilot Jun 9, 2026
6c00eee
Add full API version header coverage
Copilot Jun 9, 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: 5 additions & 0 deletions actions/setup/js/handle_agent_failure.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const FAILURE_ISSUE_DEDUP_WINDOW_HOURS = 24;
const FAILURE_ISSUE_CATEGORY_DAILY_CAP = 50;
const FAILURE_ISSUE_WINDOW_MS = FAILURE_ISSUE_DEDUP_WINDOW_HOURS * 60 * 60 * 1000;
const DEFAULT_OTEL_JSONL_PATH = "/tmp/gh-aw/otel.jsonl";
const GITHUB_API_VERSION = "2022-11-28";
const COPILOT_SESSION_STATE_DIR = path.join(os.tmpdir(), "gh-aw", "sandbox", "agent", "logs", "copilot-session-state");
// Engine-side 429/rate-limit signatures:
// - HTTP 429 accompanied by "too many requests"/"rate limit" phrasing
Expand Down Expand Up @@ -571,6 +572,7 @@ gh aw audit <run-id>
title: parentTitle,
body: parentBody,
labels: [parentLabel],
headers: { "X-GitHub-Api-Version": GITHUB_API_VERSION },

@github-actions github-actions Bot Jun 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ensureParentIssue create path has zero header test coverage: the header is added here, but no test ever reaches this branch — every existing test mocks search.issuesAndPullRequests to return an existing parent issue, which causes ensureParentIssue to short-circuit before reaching issues.create. The header change at this callsite is functionally unverified.

💡 Suggested fix

Add a test where the search mock returns an empty result for the parent-issue query, forcing ensureParentIssue to hit the create path, and assert:

expect(createIssueMock).toHaveBeenCalledWith(
  expect.objectContaining({
    headers: { "X-GitHub-Api-Version": "2022-11-28" },
  })
);

A minimal setup is a search mock that returns { total_count: 0, items: [] } only when the query contains "[aw] Failed runs".

Copilot AI Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in 6c00eee. Added a test that enables grouped reports, forces the [aw] Failed runs create path, and verifies the version header on that parent issue creation.

});

core.info(`✓ Created parent issue #${newIssue.data.number}: ${newIssue.data.html_url}`);
Expand Down Expand Up @@ -2133,6 +2135,7 @@ async function findOrCreateDailyCapRollupIssue(owner, repo) {
title: DAILY_CAP_ROLLUP_TITLE,
body,
labels: ["agentic-workflows", DAILY_CAP_ROLLUP_LABEL],
headers: { "X-GitHub-Api-Version": GITHUB_API_VERSION },
});
core.info(`✓ Created daily cap rollup issue #${newIssue.data.number}: ${newIssue.data.html_url}`);
return { number: newIssue.data.number, html_url: newIssue.data.html_url };
Expand Down Expand Up @@ -2216,6 +2219,7 @@ async function detectAndHandleFailureCascade(owner, repo, triggeringIssueNumber)
title: CASCADE_ROLLUP_TITLE,
body: rollupBody,
labels: ["agentic-workflows", CASCADE_ROLLUP_LABEL],
headers: { "X-GitHub-Api-Version": GITHUB_API_VERSION },
});
core.info(`✓ Created cascade rollup issue #${newRollup.data.number}: ${newRollup.data.html_url}`);
}
Expand Down Expand Up @@ -3051,6 +3055,7 @@ async function main() {
title: issueTitle,
body: issueBody,
labels: ["agentic-workflows"],
headers: { "X-GitHub-Api-Version": GITHUB_API_VERSION },
});

core.info(`✓ Created new issue #${newIssue.data.number}: ${newIssue.data.html_url}`);
Expand Down
6 changes: 6 additions & 0 deletions actions/setup/js/handle_agent_failure.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,11 @@ describe("handle_agent_failure", () => {

expect(createCommentMock).not.toHaveBeenCalled();
expect(createIssueMock).toHaveBeenCalledOnce();
expect(createIssueMock).toHaveBeenCalledWith(
expect.objectContaining({
headers: { "X-GitHub-Api-Version": "2022-11-28" },

@github-actions github-actions Bot Jun 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Missing header assertion for the findOrCreateDailyCapRollupIssue create path: the existing daily-cap test (around line 743) exercises findOrCreateDailyCapRollupIssue and asserts on the issue title, but does not check headers. A regression that removes the version header from that callsite would go undetected.

💡 Suggested addition

In the daily-cap test block (the one that asserts title: "[aw] Daily failure issue cap exceeded"), add:

expect(createIssueMock).toHaveBeenCalledWith(
  expect.objectContaining({
    headers: { "X-GitHub-Api-Version": "2022-11-28" },
  })
);

This mirrors the pattern already used at line 424 for the main issue creation path.

Copilot AI Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in 6c00eee. The daily-cap rollup test now asserts the X-GitHub-Api-Version header on the create call.

})
);

@github-actions github-actions Bot Jun 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[/tdd] This assertion covers one of the four patched call sites, leaving two paths — parent issue creation (findOrCreateParentIssue, impl line ~575) and daily-cap rollup (findOrCreateDailyCapRollupIssue, impl line ~2138) — without header assertions. If either path accidentally drops the X-GitHub-Api-Version header in a future refactor, no test will catch it.

💡 Suggested additions

In the test for the parent issue creation path:

expect(createIssueMock).toHaveBeenCalledWith(
  expect.objectContaining({
    headers: { "X-GitHub-Api-Version": "2022-11-28" },
  })
);

And similarly in the findOrCreateDailyCapRollupIssue test block. The PR description says coverage was added for "representative" paths, but since the fix touches four distinct call sites, all four warrant explicit assertions to fully close the regression surface.

Copilot AI Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in 6c00eee. Added explicit header coverage for the missing parent-issue and daily-cap create paths in actions/setup/js/handle_agent_failure.test.cjs.

expect(searchMock).toHaveBeenCalledWith(expect.objectContaining({ q: expect.stringContaining('"gh-aw-agentic-workflow:"') }));
expect(searchMock).toHaveBeenCalledWith(expect.objectContaining({ q: expect.stringContaining('"workflow_id: test-workflow" in:body') }));
});
Expand Down Expand Up @@ -3160,6 +3165,7 @@ describe("handle_agent_failure", () => {
expect(createCall.title).toBe(CASCADE_ROLLUP_TITLE);
expect(createCall.labels).toContain(CASCADE_ROLLUP_LABEL);
expect(createCall.labels).toContain("agentic-workflows");
expect(createCall.headers).toEqual({ "X-GitHub-Api-Version": "2022-11-28" });

@github-actions github-actions Bot Jun 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[/tdd] Minor style inconsistency: this assertion uses direct property access (createCall.headers), while the companion assertion at line 428 uses toHaveBeenCalledWith(expect.objectContaining(...)). Both are functionally correct, but aligning on one pattern across all four call-site tests would improve readability and make the intent — verify header presence on every issues.create invocation — more scannable.

💡 Consistent pattern

If the test already has access to the call arguments via createCall, the direct style used here (expect(createCall.headers).toEqual(...)) is actually more readable. Consider applying it to the test at line 428 too, replacing the toHaveBeenCalledWith(expect.objectContaining(...)) form with the same destructured-call pattern for uniformity.

Copilot AI Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in 6c00eee. The header assertions in actions/setup/js/handle_agent_failure.test.cjs now use the same direct-call pattern for readability.


// All 10 issues labeled
expect(addLabelsMock).toHaveBeenCalledTimes(10);
Expand Down
Loading