From 7faf87628a1ad6175975bc595344337ce14a7db4 Mon Sep 17 00:00:00 2001 From: Eric Fennis Date: Fri, 23 Oct 2026 18:50:46 +0200 Subject: [PATCH] fix(ci): prevent broken and duplicate metadata suggestions (#5065) * fix(ci): store metadata suggestions in the review instead of actions/cache pull_request_target runs can't write to the Actions cache ("cache write denied: token has no writable scopes"), so the cache was never saved and the model was asked again on every push. That produced new suggestions anchored on the same lines as earlier ones. Store the model's suggestions per icon name as hidden JSON in the review body and read them back from the bot's earlier reviews, so the API is only called for icons without stored suggestions. This drops the gh api and actions/cache steps. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01TSdUq9uFpjuQsoobCBqz4j * fix(ci): handle earlier metadata suggestion comments on older PRs - Read values from the oldest comment format too, which doesn't start with "Suggested `field`" and was posted without a closing fence; the field is taken from the suggestion's `"field": [` line. - Skip a field while an earlier suggestion for it is still applicable (not outdated), so two suggestions never replace overlapping lines. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01TSdUq9uFpjuQsoobCBqz4j * fix(ci): read icon metadata at the reviewed commit The review comments are anchored to the commit that triggered the run, but the icon JSON was read through the PR files' raw_url, which always points at the latest head. When a commit was pushed while the action was running, suggestions were anchored to the wrong lines, e.g. a use-cases block replacing the "tags": [ line, producing invalid JSON. Read each file at the reviewed commit instead, default that commit to the PR's head SHA, and skip files that don't exist at it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01TSdUq9uFpjuQsoobCBqz4j --------- Co-authored-by: Claude --- .github/actions/suggest-metadata/.env.example | 4 +- .github/actions/suggest-metadata/action.yml | 21 ----- .../suggest-metadata/suggestMetaData.mts | 92 +++++++++++++------ 3 files changed, 67 insertions(+), 50 deletions(-) diff --git a/.github/actions/suggest-metadata/.env.example b/.github/actions/suggest-metadata/.env.example index a04ab87ff..0161d5ac7 100644 --- a/.github/actions/suggest-metadata/.env.example +++ b/.github/actions/suggest-metadata/.env.example @@ -12,11 +12,9 @@ PULL_REQUEST_NUMBER= # Optional # OpenAI model to use (default: gpt-5-mini) # OPENAI_API_MODEL=gpt-5-mini -# Commit the review comments are anchored to (default: HEAD) +# Commit the review comments are anchored to (default: the pull request's head commit) # COMMIT_SHA= # Reviewer login used to skip PRs that were already reviewed (default: github-actions[bot]) # REVIEWER= # Read icon JSON from the local checkout instead of the PR on GitHub (default: false) # USE_FILE_SYSTEM=true -# Cache model suggestions per icon name in this file, so re-runs skip the API (default: no cache) -# SUGGESTIONS_CACHE_FILE=.cache/suggestions.json diff --git a/.github/actions/suggest-metadata/action.yml b/.github/actions/suggest-metadata/action.yml index 74caaffc5..c59b7d111 100644 --- a/.github/actions/suggest-metadata/action.yml +++ b/.github/actions/suggest-metadata/action.yml @@ -22,26 +22,6 @@ inputs: runs: using: composite steps: - # Cache the suggestions by the pull request's icon names, so the API is only - # called again when icons are added, removed or renamed. - - name: Get changed icon names - id: icons - shell: bash - run: | - hash=$(gh api "repos/$GITHUB_REPOSITORY/pulls/$PULL_REQUEST_NUMBER/files" --paginate \ - --jq '.[].filename | select(startswith("icons/") and endswith(".json"))' \ - | sort | sha256sum | cut -d ' ' -f 1) - echo "hash=$hash" >> "$GITHUB_OUTPUT" - env: - GH_TOKEN: ${{ inputs.github-token }} - PULL_REQUEST_NUMBER: ${{ inputs.pull-request-number }} - - - name: Cache suggestions - uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 - with: - path: ${{ runner.temp }}/suggest-metadata/suggestions.json - key: suggest-metadata-pr-${{ inputs.pull-request-number }}-${{ steps.icons.outputs.hash }} - - name: Generate metadata suggestions shell: bash run: node "$GITHUB_ACTION_PATH/suggestMetaData.mts" @@ -51,4 +31,3 @@ runs: PULL_REQUEST_NUMBER: ${{ inputs.pull-request-number }} COMMIT_SHA: ${{ inputs.commit-sha }} OPENAI_API_MODEL: ${{ inputs.model }} - SUGGESTIONS_CACHE_FILE: ${{ runner.temp }}/suggest-metadata/suggestions.json diff --git a/.github/actions/suggest-metadata/suggestMetaData.mts b/.github/actions/suggest-metadata/suggestMetaData.mts index 665188577..73948304d 100644 --- a/.github/actions/suggest-metadata/suggestMetaData.mts +++ b/.github/actions/suggest-metadata/suggestMetaData.mts @@ -34,9 +34,7 @@ const { const octokit = new Octokit({ auth: process.env.GITHUB_TOKEN }); const pullRequestNumber = Number(process.env.PULL_REQUEST_NUMBER); const username = process.env.REVIEWER ?? 'github-actions[bot]'; -const commitSha = process.env.COMMIT_SHA ?? "HEAD"; const useFileSystem = process.env.USE_FILE_SYSTEM === 'true' || false; -const cacheFile = process.env.SUGGESTIONS_CACHE_FILE; const owner = 'lucide-icons'; const repo = 'lucide'; @@ -129,11 +127,28 @@ const metadataSchema = z.object({ type MetadataSuggestion = z.infer; -// The action caches the suggestions under a key based on the pull request's -// icon names, so a restored cache means the API doesn't need to be called again. -const suggestionsCache: Record = cacheFile - ? await fs.readFile(cacheFile, 'utf-8').then(JSON.parse, () => ({})) - : {}; +// The model's suggestions per icon name are stored as hidden JSON in the body +// of each review, so later runs on the pull request only call the API for icons +// that don't have suggestions yet (e.g. new or renamed icons). +const CACHE_MARKER = //; + +const suggestionsCache: Record = {}; +const previousReviews = await octokit.paginate(octokit.pulls.listReviews, { + owner, + repo, + pull_number: pullRequestNumber, + per_page: 100, +}); +for (const { user, body } of previousReviews) { + const cache = user?.login === username ? body?.match(CACHE_MARKER)?.[1] : undefined; + if (!cache) continue; + + try { + Object.assign(suggestionsCache, JSON.parse(cache)); + } catch { + // Ignore a malformed cache. + } +} const { data: files } = await octokit.pulls.listFiles({ owner, @@ -151,6 +166,10 @@ const { data: pullRequest } = await octokit.pulls.get({ const prDescription = (pullRequest.body || '').slice(0, 4000); +// The commit the review comments are anchored to. Icon files are read at this +// commit too, so line numbers match even if new commits are pushed mid-run. +const commitSha = process.env.COMMIT_SHA || pullRequest.head.sha; + // The action runs on every update of the pull request. Collect the values that // were already suggested per file and field from the suggestion blocks of // earlier comments, so they are not suggested again. Comparing values rather @@ -166,9 +185,16 @@ const previousComments = ( ).filter((comment) => comment.user?.login === username); const previouslySuggestedValues = new Map(); -for (const { path: filePath, body } of previousComments) { - const field = body.match(/^Suggested `([^`]+)`/)?.[1]; - const suggestion = body.match(/```suggestion\r?\n([\s\S]*?)\r?\n```/)?.[1]; +// Files and fields that still have a suggestion that can be applied. A second +// one would replace overlapping lines, and applying both would break the JSON. +const openSuggestions = new Set(); +for (const { path: filePath, body, line: commentLine } of previousComments) { + // Comments of the oldest format were posted without a closing fence, and + // don't start with "Suggested `field`". Their suggestion starts with the + // field's own line (e.g. `"tags": [`), so the field is read from there. + const suggestion = body.match(/```suggestion\r?\n([\s\S]*?)(?:\r?\n```|$)/)?.[1]; + const field = + body.match(/^Suggested `([^`]+)`/)?.[1] ?? suggestion?.match(/^\s*"([^"]+)":/)?.[1]; if (!field || !suggestion) continue; const values = suggestion.split('\n').flatMap((line) => { @@ -178,6 +204,9 @@ for (const { path: filePath, body } of previousComments) { const key = `${filePath}:${field}`; previouslySuggestedValues.set(key, [...(previouslySuggestedValues.get(key) ?? []), ...values]); + + // GitHub sets `line` to null once the commented lines have changed (outdated). + if (commentLine != null) openSuggestions.add(key); } const changedFiles = files.filter( @@ -333,22 +362,32 @@ async function getSuggestion( return suggested; } -const suggestionsByFile = await Promise.all( - changedFiles.map(async ({ filename, raw_url }) => { +const suggestionsByFile = (await Promise.all( + changedFiles.map(async ({ filename }) => { const iconName = path.basename(filename, '.json'); - // Read the icon's current metadata (the PR head version) first, so we can + // Read the icon's current metadata at the reviewed commit first, so we can // both give it to the model as context and dedupe suggestions against it. let fileContent: string if (useFileSystem) { fileContent = await fs.readFile(path.join(repoRoot, filename), 'utf-8'); } else { - const fileGithubRequest = await octokit.request(`GET ${raw_url}`, { - headers: { - 'Accept': 'application/vnd.github.v3.raw', - }, - }); - fileContent = fileGithubRequest.data + try { + const { data } = await octokit.repos.getContent({ + owner, + repo, + path: filename, + ref: commitSha, + mediaType: { format: 'raw' }, + }); + fileContent = data as unknown as string; + } catch (error) { + // The file list is for the latest head, so a file can be missing at + // the reviewed commit (added later, or removed). A later run covers it. + if ((error as { status?: number }).status !== 404) throw error; + console.log(`${filename} does not exist at ${commitSha}. Skipping...`); + return null; + } } const metadata = JSON.parse(fileContent) @@ -366,13 +405,7 @@ const suggestionsByFile = await Promise.all( return { filename, iconName, fileContent, currentMetadata, suggested }; }), -); - -// Write the cache before any early exit below, so the action can save it. -if (cacheFile) { - await fs.mkdir(path.dirname(cacheFile), { recursive: true }); - await fs.writeFile(cacheFile, JSON.stringify(suggestionsCache, null, 2)); -} +)).filter((file) => file !== null); const comments = suggestionsByFile.flatMap( ({ filename, iconName, fileContent, currentMetadata, suggested }) => { @@ -397,6 +430,11 @@ const comments = suggestionsByFile.flatMap( return []; } + if (openSuggestions.has(`${filename}:${field}`)) { + console.log(`An earlier ${field} suggestion for ${iconName} is still open. Skipping...`); + return []; + } + const block = findFieldBlock(lines, field); if (!block) { console.log(`Could not locate "${field}" in ${filename}. Skipping...`); @@ -443,6 +481,8 @@ if (comments.length === 0) { const reviewBody = `### 🤖 Metadata suggestions ✨ I've asked ChatGPT for some suggestions for \`tags\`, \`categories\` and \`use-cases\`. Please review them and apply any that you find useful. + + `; const review = {