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
47 changes: 29 additions & 18 deletions actions/setup/js/assign_milestone.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -67,34 +67,41 @@ async function main(config = {}) {
// Track how many items we've processed for max limit
let processedCount = 0;

// Cached results from paginated title searches
/** @type {Map<string, Object>} */
// Cached results from paginated title searches, scoped by "owner/repo" to prevent
// cross-repo cache pollution in multi-repo configurations.
/** @type {Map<string, Object>} key: "owner/repo::title" */
const milestoneByTitle = new Map();
/** @type {Array<Object>} All milestones fetched so far (for error messages) */
let allFetchedMilestones = [];
let milestonesExhausted = false;
/** @type {Set<string>} entries: "owner/repo" — repos whose milestones have been fully fetched */
const exhaustedRepos = new Set();
/** @type {Map<string, Array<Object>>} key: "owner/repo" → milestones fetched for that repo */
const fetchedMilestonesByRepo = new Map();

/**
* Find a milestone by title using lazy paginated search with early exit.
* Results are cached so repeated lookups don't re-paginate.
* Results are cached per-repo so repeated lookups don't re-paginate, and
* cache entries from one repo never affect lookups for another repo.
* @param {string} title
* @param {string} owner
* @param {string} repo
* @returns {Promise<Object|null>}
*/
async function findMilestoneByTitle(title, owner, repo) {
if (milestoneByTitle.has(title)) {
return milestoneByTitle.get(title);
const repoKey = `${owner}/${repo}`;
const cacheKey = `${repoKey}::${title}`;
if (milestoneByTitle.has(cacheKey)) {
return milestoneByTitle.get(cacheKey);
}
if (milestonesExhausted) {
if (exhaustedRepos.has(repoKey)) {
return null;
}
const repoMilestones = fetchedMilestonesByRepo.get(repoKey) || [];
let found = false;
await githubClient.paginate(githubClient.rest.issues.listMilestones, { owner, repo, state: "all", per_page: 100 }, (response, done) => {
for (const m of response.data) {
if (!milestoneByTitle.has(m.title)) {
milestoneByTitle.set(m.title, m);
allFetchedMilestones.push(m);
const key = `${repoKey}::${m.title}`;
if (!milestoneByTitle.has(key)) {
milestoneByTitle.set(key, m);
repoMilestones.push(m);
}
if (m.title === title) {
found = true;
Expand All @@ -103,11 +110,12 @@ async function main(config = {}) {
}
}
});
fetchedMilestonesByRepo.set(repoKey, repoMilestones);
if (!found) {
milestonesExhausted = true;
exhaustedRepos.add(repoKey);
}
core.info(`Searched ${allFetchedMilestones.length} milestones (exhausted=${milestonesExhausted})`);
return milestoneByTitle.get(title) || null;
core.info(`Searched ${repoMilestones.length} milestones for ${repoKey} (exhausted=${exhaustedRepos.has(repoKey)})`);
return milestoneByTitle.get(cacheKey) || null;
}

/**
Expand Down Expand Up @@ -202,11 +210,14 @@ async function main(config = {}) {
title: milestoneTitle,
});
milestoneNumber = created.data.number;
milestoneByTitle.set(created.data.title, created.data);
allFetchedMilestones.push(created.data);
const repoKey = `${milestoneOwner}/${milestoneRepo}`;
milestoneByTitle.set(`${repoKey}::${created.data.title}`, created.data);
const repoMilestones = fetchedMilestonesByRepo.get(repoKey) || [];
repoMilestones.push(created.data);
fetchedMilestonesByRepo.set(repoKey, repoMilestones);
core.info(`Auto-created milestone "${milestoneTitle}" as #${milestoneNumber}`);
} else {
const available = formatAvailableMilestones(allFetchedMilestones);
const available = formatAvailableMilestones(fetchedMilestonesByRepo.get(`${milestoneOwner}/${milestoneRepo}`) || []);
core.warning(`Milestone "${milestoneTitle}" not found in repository. Available: ${available}. Set auto_create: true to create it automatically.`);
return {
success: false,
Expand Down
92 changes: 92 additions & 0 deletions actions/setup/js/assign_milestone.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -441,4 +441,96 @@ describe("assign_milestone (Handler Factory Architecture)", () => {
expect(result.error).toContain("not in the allowed-repos list");
});
});

describe("cross-repo milestone cache isolation", () => {
it("should not return cached milestone from repo1 when querying repo2 (cache collision)", async () => {
const { main } = require("./assign_milestone.cjs");
const crossRepoHandler = await main({
max: 20,
allowed_repos: ["org/repo1", "org/repo2"],
});

// repo1: v1.0 = #5; repo2: v1.0 = #3 — same title, different numbers
mockGithub.paginate.mockImplementation(async (_method, params, callback) => {
const data = params.repo === "repo1" ? [{ title: "v1.0", number: 5 }] : [{ title: "v1.0", number: 3 }];
if (callback) {
const done = vi.fn();
callback({ data }, done);
}
return data;
});
mockGithub.rest.issues.update.mockResolvedValue({});

// First call: repo1
const result1 = await crossRepoHandler({ type: "assign_milestone", issue_number: 10, milestone_title: "v1.0", repo: "org/repo1" }, {});
expect(result1.success).toBe(true);
expect(result1.milestone_number).toBe(5);

// Second call: repo2 — must NOT reuse repo1's cached #5
const result2 = await crossRepoHandler({ type: "assign_milestone", issue_number: 20, milestone_title: "v1.0", repo: "org/repo2" }, {});
expect(result2.success).toBe(true);
expect(result2.milestone_number).toBe(3);

// The last issues.update call must target repo2 with milestone #3
expect(mockGithub.rest.issues.update).toHaveBeenLastCalledWith(expect.objectContaining({ owner: "org", repo: "repo2", milestone: 3 }));
});

it("should not skip repo2 search after repo1 is exhausted (false exhaustion)", async () => {
const { main } = require("./assign_milestone.cjs");
const crossRepoHandler = await main({
max: 20,
allowed_repos: ["org/repo1", "org/repo2"],
});

// repo1: no milestones; repo2: sprint-42 = #7
mockGithub.paginate.mockImplementation(async (_method, params, callback) => {
const data = params.repo === "repo1" ? [] : [{ title: "sprint-42", number: 7 }];
if (callback) {
const done = vi.fn();
callback({ data }, done);
}
return data;
});
mockGithub.rest.issues.update.mockResolvedValue({});

// First call: repo1 — exhausts repo1, milestone not found
const result1 = await crossRepoHandler({ type: "assign_milestone", issue_number: 10, milestone_title: "sprint-42", repo: "org/repo1" }, {});
expect(result1.success).toBe(false);

// Second call: repo2 — must still search even though repo1 was exhausted
const result2 = await crossRepoHandler({ type: "assign_milestone", issue_number: 20, milestone_title: "sprint-42", repo: "org/repo2" }, {});
expect(result2.success).toBe(true);
expect(result2.milestone_number).toBe(7);
});

it("should use repo-scoped milestones in error message when title not found", async () => {
const { main } = require("./assign_milestone.cjs");
const crossRepoHandler = await main({
max: 20,
allowed_repos: ["org/repo1", "org/repo2"],
});

// repo1: only has "v1.0"; repo2: only has "v2.0"
mockGithub.paginate.mockImplementation(async (_method, params, callback) => {
const data = params.repo === "repo1" ? [{ title: "v1.0", number: 5 }] : [{ title: "v2.0", number: 3 }];
if (callback) {
const done = vi.fn();
callback({ data }, done);
}
return data;
});

// Search for "missing" in repo2 — error message should list "v2.0", not "v1.0" from repo1
await crossRepoHandler({ type: "assign_milestone", issue_number: 10, milestone_title: "v1.0", repo: "org/repo1" }, {});
mockGithub.rest.issues.update.mockResolvedValue({});

const result = await crossRepoHandler({ type: "assign_milestone", issue_number: 20, milestone_title: "missing", repo: "org/repo2" }, {});
expect(result.success).toBe(false);
// Warning should list repo2's milestones ("v2.0"), not repo1's ("v1.0")
const warnCall = mockCore.warning.mock.calls.find(c => c[0].includes("not found in repository"));
expect(warnCall).toBeDefined();
expect(warnCall[0]).toContain("v2.0");
expect(warnCall[0]).not.toContain("v1.0");
});
});
});
Loading