diff --git a/.github/workflows/dead-code-remover.lock.yml b/.github/workflows/dead-code-remover.lock.yml index ed143843bbc..4fa3729322a 100644 --- a/.github/workflows/dead-code-remover.lock.yml +++ b/.github/workflows/dead-code-remover.lock.yml @@ -231,24 +231,24 @@ jobs: run: | bash "${RUNNER_TEMP}/gh-aw/actions/create_prompt_first.sh" { - cat << 'GH_AW_PROMPT_18306a05ecb4d558_EOF' + cat << 'GH_AW_PROMPT_78a1d3ec5c7dcc5e_EOF' - GH_AW_PROMPT_18306a05ecb4d558_EOF + GH_AW_PROMPT_78a1d3ec5c7dcc5e_EOF cat "${RUNNER_TEMP}/gh-aw/prompts/xpia.md" cat "${RUNNER_TEMP}/gh-aw/prompts/temp_folder_prompt.md" cat "${RUNNER_TEMP}/gh-aw/prompts/markdown.md" cat "${RUNNER_TEMP}/gh-aw/prompts/cache_memory_prompt.md" cat "${RUNNER_TEMP}/gh-aw/prompts/safe_outputs_prompt.md" - cat << 'GH_AW_PROMPT_18306a05ecb4d558_EOF' + cat << 'GH_AW_PROMPT_78a1d3ec5c7dcc5e_EOF' Tools: create_pull_request, missing_tool, missing_data, noop - GH_AW_PROMPT_18306a05ecb4d558_EOF + GH_AW_PROMPT_78a1d3ec5c7dcc5e_EOF cat "${RUNNER_TEMP}/gh-aw/prompts/safe_outputs_create_pull_request.md" - cat << 'GH_AW_PROMPT_18306a05ecb4d558_EOF' + cat << 'GH_AW_PROMPT_78a1d3ec5c7dcc5e_EOF' - GH_AW_PROMPT_18306a05ecb4d558_EOF + GH_AW_PROMPT_78a1d3ec5c7dcc5e_EOF cat "${RUNNER_TEMP}/gh-aw/prompts/mcp_cli_tools_prompt.md" - cat << 'GH_AW_PROMPT_18306a05ecb4d558_EOF' + cat << 'GH_AW_PROMPT_78a1d3ec5c7dcc5e_EOF' The following GitHub context information is available for this workflow: {{#if github.actor}} @@ -277,15 +277,15 @@ jobs: {{/if}} - GH_AW_PROMPT_18306a05ecb4d558_EOF + GH_AW_PROMPT_78a1d3ec5c7dcc5e_EOF cat "${RUNNER_TEMP}/gh-aw/prompts/cli_proxy_with_safeoutputs_prompt.md" - cat << 'GH_AW_PROMPT_18306a05ecb4d558_EOF' + cat << 'GH_AW_PROMPT_78a1d3ec5c7dcc5e_EOF' {{#runtime-import .github/workflows/shared/otlp.md}} {{#runtime-import .github/workflows/shared/activation-app.md}} {{#runtime-import .github/workflows/shared/reporting.md}} {{#runtime-import .github/workflows/dead-code-remover.md}} - GH_AW_PROMPT_18306a05ecb4d558_EOF + GH_AW_PROMPT_78a1d3ec5c7dcc5e_EOF } > "$GH_AW_PROMPT" - name: Interpolate variables and render templates uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 @@ -548,9 +548,9 @@ jobs: mkdir -p "${RUNNER_TEMP}/gh-aw/safeoutputs" mkdir -p /tmp/gh-aw/safeoutputs mkdir -p /tmp/gh-aw/mcp-logs/safeoutputs - cat > "${RUNNER_TEMP}/gh-aw/safeoutputs/config.json" << 'GH_AW_SAFE_OUTPUTS_CONFIG_c200878e6f494fe3_EOF' + cat > "${RUNNER_TEMP}/gh-aw/safeoutputs/config.json" << 'GH_AW_SAFE_OUTPUTS_CONFIG_6a962a6b758a044f_EOF' {"create_pull_request":{"expires":72,"labels":["chore","dead-code"],"max":1,"max_patch_files":100,"max_patch_size":1024,"protect_top_level_dot_folders":true,"protected_files":["package.json","bun.lockb","bunfig.toml","deno.json","deno.jsonc","deno.lock","global.json","NuGet.Config","Directory.Packages.props","mix.exs","mix.lock","go.mod","go.sum","stack.yaml","stack.yaml.lock","pom.xml","build.gradle","build.gradle.kts","settings.gradle","settings.gradle.kts","gradle.properties","package-lock.json","yarn.lock","pnpm-lock.yaml","npm-shrinkwrap.json","requirements.txt","Pipfile","Pipfile.lock","pyproject.toml","setup.py","setup.cfg","Gemfile","Gemfile.lock","uv.lock","CODEOWNERS","DESIGN.md","README.md","CONTRIBUTING.md","CHANGELOG.md","SECURITY.md","CODE_OF_CONDUCT.md","AGENTS.md","CLAUDE.md","GEMINI.md"],"protected_files_policy":"request_review","reviewers":["copilot"],"title_prefix":"[dead-code] "},"create_report_incomplete_issue":{},"missing_data":{},"missing_tool":{},"noop":{"max":1,"report-as-issue":"true"},"report_incomplete":{}} - GH_AW_SAFE_OUTPUTS_CONFIG_c200878e6f494fe3_EOF + GH_AW_SAFE_OUTPUTS_CONFIG_6a962a6b758a044f_EOF - name: Generate Safe Outputs Tools env: GH_AW_TOOLS_META_JSON: | @@ -762,7 +762,7 @@ jobs: mkdir -p /home/runner/.copilot GH_AW_NODE=$(which node 2>/dev/null || command -v node 2>/dev/null || echo node) - cat << GH_AW_MCP_CONFIG_7d54735b2e88781a_EOF | "$GH_AW_NODE" "${RUNNER_TEMP}/gh-aw/actions/start_mcp_gateway.cjs" + cat << GH_AW_MCP_CONFIG_433f84c29b1f57ad_EOF | "$GH_AW_NODE" "${RUNNER_TEMP}/gh-aw/actions/start_mcp_gateway.cjs" { "mcpServers": { "safeoutputs": { @@ -792,7 +792,7 @@ jobs: } } } - GH_AW_MCP_CONFIG_7d54735b2e88781a_EOF + GH_AW_MCP_CONFIG_433f84c29b1f57ad_EOF - name: Mount MCP servers as CLIs id: mount-mcp-clis continue-on-error: true @@ -1744,3 +1744,4 @@ jobs: with: key: memory-none-nopolicy-${{ env.GH_AW_WORKFLOW_ID_SANITIZED }}-${{ github.run_id }} path: /tmp/gh-aw/cache-memory + diff --git a/actions/setup/js/resolve_mentions.cjs b/actions/setup/js/resolve_mentions.cjs index 3b201d8bdf2..81317aaa93c 100644 --- a/actions/setup/js/resolve_mentions.cjs +++ b/actions/setup/js/resolve_mentions.cjs @@ -2,6 +2,20 @@ /// const { getErrorMessage } = require("./error_helpers.cjs"); +/** @type {Map>} */ +const recentCollaboratorsCache = new Map(); +/** @type {Map>} */ +const userPermissionCache = new Map(); + +/** + * Build a stable cache key for repository-scoped lookups. + * @param {string} owner + * @param {string} repo + * @returns {string} + */ +function getRepoCacheKey(owner, repo) { + return `${owner}/${repo}`.toLowerCase(); +} /** * @typedef {Object} MentionResolutionResult @@ -58,6 +72,11 @@ function isPayloadUserBot(user) { * @returns {Promise>} Map of username (lowercase) to whether they're allowed (any collaborator, not bot) */ async function getRecentCollaborators(owner, repo, github, core) { + const repoCacheKey = getRepoCacheKey(owner, repo); + const cachedCollaborators = recentCollaboratorsCache.get(repoCacheKey); + if (cachedCollaborators) { + return cachedCollaborators; + } try { // Fetch only first page (30 collaborators) for optimistic resolution const collaborators = await github.rest.repos.listCollaborators({ @@ -75,6 +94,7 @@ async function getRecentCollaborators(owner, repo, github, core) { allowedMap.set(lowercaseLogin, isAllowed); } + recentCollaboratorsCache.set(repoCacheKey, allowedMap); return allowedMap; } catch (error) { core.warning(`Failed to fetch recent collaborators: ${getErrorMessage(error)}`); @@ -92,6 +112,17 @@ async function getRecentCollaborators(owner, repo, github, core) { * @returns {Promise} True if user is allowed (any collaborator, not bot) */ async function checkUserPermission(username, owner, repo, github, core) { + const repoCacheKey = getRepoCacheKey(owner, repo); + const usernameKey = username.toLowerCase(); + const cachedPermissions = userPermissionCache.get(repoCacheKey); + if (cachedPermissions && cachedPermissions.has(usernameKey)) { + // TypeScript doesn't recognize Map.has() as a type guard for Map.get() + // @ts-expect-error - .has() guarantees .get() returns boolean, not undefined + return cachedPermissions.get(usernameKey); + } + + /** @type {Map} */ + const permissionsForRepo = cachedPermissions || new Map(); try { // First check if user exists and is not a bot const { data: user } = await github.rest.users.getByUsername({ @@ -99,6 +130,8 @@ async function checkUserPermission(username, owner, repo, github, core) { }); if (user.type === "Bot") { + permissionsForRepo.set(usernameKey, false); + userPermissionCache.set(repoCacheKey, permissionsForRepo); return false; } @@ -110,9 +143,14 @@ async function checkUserPermission(username, owner, repo, github, core) { }); // Allow any permission level (read, triage, write, maintain, admin) - return permissionData.permission !== "none"; + const isAllowed = permissionData.permission !== "none"; + permissionsForRepo.set(usernameKey, isAllowed); + userPermissionCache.set(repoCacheKey, permissionsForRepo); + return isAllowed; } catch (error) { // User doesn't exist, not a collaborator, or API error - deny + permissionsForRepo.set(usernameKey, false); + userPermissionCache.set(repoCacheKey, permissionsForRepo); return false; } } @@ -142,6 +180,16 @@ async function resolveMentionsLazily(text, knownAuthors, owner, repo, github, co core.warning(`Mention limit exceeded: ${totalMentions} mentions found, processing only first 50`); } + if (mentionsToProcess.length === 0) { + core.info("No mentions found in text"); + return { + allowedMentions: [], + totalMentions, + resolvedCount: 0, + limitExceeded, + }; + } + // Build set of known allowed authors (case-insensitive) const knownAuthorsLowercase = new Set(knownAuthors.filter(a => a).map(a => a.toLowerCase())); @@ -189,10 +237,16 @@ async function resolveMentionsLazily(text, knownAuthors, owner, repo, github, co }; } +function resetMentionResolutionCaches() { + recentCollaboratorsCache.clear(); + userPermissionCache.clear(); +} + module.exports = { extractMentions, isPayloadUserBot, getRecentCollaborators, checkUserPermission, resolveMentionsLazily, + resetMentionResolutionCaches, }; diff --git a/actions/setup/js/resolve_mentions.test.cjs b/actions/setup/js/resolve_mentions.test.cjs index a4fecff3934..7b106b277f4 100644 --- a/actions/setup/js/resolve_mentions.test.cjs +++ b/actions/setup/js/resolve_mentions.test.cjs @@ -2,10 +2,11 @@ import { describe, it, expect, beforeEach, vi } from "vitest"; const mockCore = { info: vi.fn(), warning: vi.fn() }, mockGithub = { rest: { repos: { listCollaborators: vi.fn(), getCollaboratorPermissionLevel: vi.fn() }, users: { getByUsername: vi.fn() } } }; ((global.core = mockCore), (global.github = mockGithub)); -const { extractMentions, isPayloadUserBot, getRecentCollaborators, checkUserPermission, resolveMentionsLazily } = require("./resolve_mentions.cjs"); +const { extractMentions, isPayloadUserBot, getRecentCollaborators, checkUserPermission, resolveMentionsLazily, resetMentionResolutionCaches } = require("./resolve_mentions.cjs"); describe("resolve_mentions.cjs", () => { (beforeEach(() => { vi.clearAllMocks(); + resetMentionResolutionCaches(); }), describe("extractMentions", () => { (it("should extract single mention", () => { @@ -157,6 +158,21 @@ describe("resolve_mentions.cjs", () => { (await resolveMentionsLazily("Hello @author1 @maintainer1", ["author1"], "owner", "repo", mockGithub, mockCore), expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("Found 2 unique mentions")), expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("Total allowed mentions"))); + }), + it("should skip collaborator lookups when text has no mentions", async () => { + const result = await resolveMentionsLazily("Hello world", [], "owner", "repo", mockGithub, mockCore); + expect(result).toMatchObject({ allowedMentions: [], totalMentions: 0, resolvedCount: 0, limitExceeded: false }); + expect(mockGithub.rest.repos.listCollaborators).not.toHaveBeenCalled(); + expect(mockGithub.rest.users.getByUsername).not.toHaveBeenCalled(); + expect(mockGithub.rest.repos.getCollaboratorPermissionLevel).not.toHaveBeenCalled(); + }), + it("should reuse cached collaborator list across repeated resolution calls", async () => { + mockGithub.rest.repos.listCollaborators.mockResolvedValue({ data: [{ login: "maintainer1", type: "User", permissions: { maintain: true, admin: false, push: false } }] }); + + await resolveMentionsLazily("Hello @maintainer1", [], "owner", "repo", mockGithub, mockCore); + await resolveMentionsLazily("Hello @maintainer1", [], "owner", "repo", mockGithub, mockCore); + + expect(mockGithub.rest.repos.listCollaborators).toHaveBeenCalledTimes(1); })); })); });