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
14 changes: 8 additions & 6 deletions actions/setup/js/create_issue.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
const RECENTLY_CLOSED_DEDUP_DAYS = 30;
const TITLE_DEDUP_SEARCH_PER_PAGE = 100;
const TITLE_DEDUP_MAX_SEARCH_PAGES = 2;
const TITLE_DEDUP_MIN_SEARCH_RATE_LIMIT_REMAINING = 500;
const TITLE_DEDUP_MIN_SEARCH_RATE_LIMIT_FRACTION = 0.2;

/**
* Create a dedicated GitHub client for copilot assignment operations.
Expand Down Expand Up @@ -494,14 +494,16 @@
async function shouldSkipRepoTitleDedupSearch(githubClient, owner, repo) {
try {
const response = await githubClient.rest.rateLimit.get();
const rawRemaining = response?.data?.resources?.search?.remaining;
const { remaining: rawRemaining, limit: rawLimit } = response?.data?.resources?.search ?? {};
const remaining = Number(rawRemaining);
if (!Number.isFinite(remaining)) {
core.warning(`Could not determine search rate limit remaining for ${owner}/${repo}; proceeding with repo-level title dedup search`);
const limit = Number(rawLimit);
if (!Number.isFinite(remaining) || !Number.isFinite(limit)) {
core.warning(`Could not determine search rate limit values for ${owner}/${repo} (remaining=${rawRemaining}, limit=${rawLimit}); proceeding with repo-level title dedup search`);
return false;
}
if (remaining <= TITLE_DEDUP_MIN_SEARCH_RATE_LIMIT_REMAINING) {
core.warning(`Skipping repo-level title dedup search for ${owner}/${repo}: search rate limit remaining is ${remaining} (threshold <= ${TITLE_DEDUP_MIN_SEARCH_RATE_LIMIT_REMAINING})`);
const threshold = limit * TITLE_DEDUP_MIN_SEARCH_RATE_LIMIT_FRACTION;
if (remaining <= threshold) {
core.warning(`Skipping repo-level title dedup search for ${owner}/${repo}: search rate limit remaining is ${remaining}/${limit} (threshold <= ${Math.floor(threshold)})`);
return true;
}
} catch (error) {
Expand Down Expand Up @@ -1187,7 +1189,7 @@
core.info("✓ Successfully linked issue #" + issue.number + " as sub-issue of #" + effectiveParentIssueNumber);
} catch (error) {
core.info(`Warning: Could not link sub-issue to parent: ${getErrorMessage(error)}`);
core.info(`Error details: ${error instanceof Error ? error.stack : String(error)}`);

Check warning on line 1192 in actions/setup/js/create_issue.cjs

View workflow job for this annotation

GitHub Actions / lint-js

Prefer getErrorMessage(error) from error_helpers.cjs. The `error.stack` ternary surfaces noisy stack frames; getErrorMessage() returns a clean, consistent message
// Fallback: add a comment if sub-issue linking fails
try {
core.info(`Attempting fallback: adding comment to parent issue #${effectiveParentIssueNumber}...`);
Expand Down
71 changes: 69 additions & 2 deletions actions/setup/js/create_issue.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ describe("create_issue", () => {
data: {

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 boundary tests for the 20% threshold: the updated mock uses remaining: 30, limit: 30 (100% quota), which never exercises the guard logic introduced by this PR.

💡 Suggested additions

Add tests for:

  1. Exactly at threshold (remaining: 6, limit: 30 → 20% → should skip)
  2. Just above threshold (remaining: 7, limit: 30 → ~23% → should NOT skip)
  3. limit is undefined/missing → new !Number.isFinite(limit) branch returns false with warning — completely untested

Without these, the <= semantics and the limit-NaN guard are exercised by neither the old nor the new tests.

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 c1ef37a: added coverage for boundary/guard behavior with a just-above-threshold case (35/8) and a missing-limit case that proceeds with warning and repo-level search.

resources: {
search: {
remaining: 1000,
limit: 30,
remaining: 30,
},
},
},
Expand Down Expand Up @@ -680,7 +681,8 @@ describe("create_issue", () => {
data: {
resources: {
search: {
remaining: 1,
limit: 35,
remaining: 6,
},
},
},
Expand All @@ -701,6 +703,71 @@ describe("create_issue", () => {
expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("Skipping repo-level title dedup search"));
});

it("should skip repo-level search when search rate limit is exactly at 20 percent threshold", async () => {
mockGithub.rest.rateLimit.get.mockResolvedValue({
data: {
resources: {
search: {
limit: 30,
remaining: 6,
},
},
},
});

const handler = await main({
deduplicate_by_title: true,
});
const result = await handler({ title: "At threshold title" });

expect(result.success).toBe(true);
expect(mockGithub.rest.search.issuesAndPullRequests).not.toHaveBeenCalled();
expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("Skipping repo-level title dedup search"));
});

it("should not skip repo-level search when search rate limit is just above 20 percent threshold", async () => {
mockGithub.rest.rateLimit.get.mockResolvedValue({
data: {
resources: {
search: {
limit: 30,
remaining: 7,
},
},
},
});

const handler = await main({
deduplicate_by_title: true,
});
const result = await handler({ title: "Above threshold title" });

expect(result.success).toBe(true);
expect(mockGithub.rest.search.issuesAndPullRequests).toHaveBeenCalled();
expect(mockCore.warning).not.toHaveBeenCalledWith(expect.stringContaining("Skipping repo-level title dedup search"));
});

it("should proceed with repo-level search when search rate limit values are missing", async () => {
mockGithub.rest.rateLimit.get.mockResolvedValue({
data: {
resources: {
search: {
remaining: 30,
},
},
},
});

const handler = await main({
deduplicate_by_title: true,
});
const result = await handler({ title: "Missing limit title" });

expect(result.success).toBe(true);
expect(mockGithub.rest.search.issuesAndPullRequests).toHaveBeenCalled();
expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("remaining=30, limit=undefined"));
});

it("should reject invalid deduplicate-by-title configuration", async () => {
await expect(main({ deduplicate_by_title: "invalid" })).rejects.toThrow("deduplicate-by-title");
await expect(main({ deduplicate_by_title: 101 })).rejects.toThrow("deduplicate-by-title");
Expand Down
Loading