diff --git a/actions/setup/js/assign_milestone.cjs b/actions/setup/js/assign_milestone.cjs index dc13f6d3d82..d14dc992820 100644 --- a/actions/setup/js/assign_milestone.cjs +++ b/actions/setup/js/assign_milestone.cjs @@ -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} */ + // Cached results from paginated title searches, scoped by "owner/repo" to prevent + // cross-repo cache pollution in multi-repo configurations. + /** @type {Map} key: "owner/repo::title" */ const milestoneByTitle = new Map(); - /** @type {Array} All milestones fetched so far (for error messages) */ - let allFetchedMilestones = []; - let milestonesExhausted = false; + /** @type {Set} entries: "owner/repo" — repos whose milestones have been fully fetched */ + const exhaustedRepos = new Set(); + /** @type {Map>} 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} */ 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; @@ -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; } /** @@ -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, diff --git a/actions/setup/js/assign_milestone.test.cjs b/actions/setup/js/assign_milestone.test.cjs index 6f2888b1241..05d3b63cb01 100644 --- a/actions/setup/js/assign_milestone.test.cjs +++ b/actions/setup/js/assign_milestone.test.cjs @@ -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"); + }); + }); });