From b8974d59e8fc7530273a9e07bfa7317049b38135 Mon Sep 17 00:00:00 2001 From: Howard Braham Date: Sun, 25 Oct 2026 15:23:24 -0700 Subject: [PATCH] feat(release/auto-changelog): add clean-room changelog proofreader Adds a script that proofreads the generated `## [version]` changelog section with a model, given only each PR's title and body as evidence, and writes the result back only if it passes validation. A model failure, a rejected key, or a response that fails validation leaves the changelog untouched and is recorded in a report. The section helpers shared with `merge-previous-release-changelog.mts` live in `release-section.mts`. The tsconfig allows `.ts`/`.mts` import extensions, which Node's native TypeScript support requires, and adds the DOM lib for `fetch` and `AbortController`. The DOM lib makes `Document` a global, so `segment-schema-draft-pr/apply-changes.ts` imports yaml's `Document` as `YamlDocument` to avoid shadowing it. `.github/scripts/**/*.mts` are CLI scripts, so they may read `process.env`. --- .eslintrc.js | 5 +- .github/prompts/auto-changelog-clean-room.md | 118 ++ .github/scripts/clean-release-changelog.mts | 973 ++++++++++++++++ .../merge-previous-release-changelog.mts | 70 +- .github/scripts/release-section.mts | 192 ++++ .github/scripts/shared/environment.mts | 14 + .github/scripts/shared/report.mts | 16 + .github/scripts/shared/retry.mts | 71 ++ .../tests/clean-release-changelog.test.mts | 1012 +++++++++++++++++ .../scripts/tests/git-test-environment.mts | 41 + .../tests/merge-previous-releases.test.mts | 27 +- .../scripts/tests/release-section.test.mts | 59 + .github/scripts/tests/retry.test.mts | 67 ++ src/segment-schema-draft-pr/apply-changes.ts | 18 +- tsconfig.json | 3 +- 15 files changed, 2597 insertions(+), 89 deletions(-) create mode 100644 .github/prompts/auto-changelog-clean-room.md create mode 100644 .github/scripts/clean-release-changelog.mts create mode 100644 .github/scripts/release-section.mts create mode 100644 .github/scripts/shared/environment.mts create mode 100644 .github/scripts/shared/report.mts create mode 100644 .github/scripts/shared/retry.mts create mode 100644 .github/scripts/tests/clean-release-changelog.test.mts create mode 100644 .github/scripts/tests/git-test-environment.mts create mode 100644 .github/scripts/tests/release-section.test.mts create mode 100644 .github/scripts/tests/retry.test.mts diff --git a/.eslintrc.js b/.eslintrc.js index 7c44adad..5f0bb93c 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -39,13 +39,16 @@ module.exports = { parserOptions: { project: './tsconfig.json', }, + // These are command-line scripts that read their inputs from the environment. + rules: { + 'n/no-process-env': 'off', + }, }, { files: ['.github/scripts/**/*.test.mts'], rules: { '@typescript-eslint/no-floating-promises': 'off', - 'n/no-process-env': 'off', }, }, ], diff --git a/.github/prompts/auto-changelog-clean-room.md b/.github/prompts/auto-changelog-clean-room.md new file mode 100644 index 00000000..4c91fdd1 --- /dev/null +++ b/.github/prompts/auto-changelog-clean-room.md @@ -0,0 +1,118 @@ +You are reviewing a generated MetaMask changelog section before release. + +Your task is to rewrite and recategorize only the release section provided +below. Use the provided PR evidence when needed to understand an entry. Return +the complete corrected release section in Markdown, including its `## [VERSION]` +heading and all `###` category headings. Do not include analysis, explanations, +a diff, or any text outside the corrected Markdown. + +## Goals + +1. Remove the `### Uncategorized` section entirely. The other category + headings in the input are the generator's guesses; keep an entry where it + is when its category is right and move it when it is wrong. +2. Put every retained entry in exactly one appropriate Keep a Changelog + category. Include a category heading only when it contains at least one + entry: + - Added: new user-visible functionality + - Changed: changed behavior, UI, flows, dependencies, or configuration + - Deprecated: functionality marked deprecated + - Removed: functionality removed + - Fixed: user-facing bug fixes + - Security: security fixes or improvements +3. Use concise, user-facing, past-tense wording. +4. Preserve every distinct input PR number exactly once, in the existing + `(#12345)` form. If a generator defect repeats the same PR reference, + retain it once. A consolidated entry may contain multiple trailing PR + references. Every entry must end with its PR reference. +5. Do not add, remove, or invent distinct PR numbers. Do not split a PR into + multiple entries. +6. Do not add information not supported by the entry text, its matching PR, + or the relevant repository changes. +7. Preserve existing valid entries unless a correction is required. +8. Before returning, perform a PR-reference inventory audit. Every distinct + input `(#12345)` reference must appear exactly once in the output. Compare + the unique reference sets to ensure none is omitted or invented, then count + output occurrences to ensure no reference was duplicated while moving or + consolidating entries. A consolidated entry must retain every source PR + reference. Do not omit a reference because its authored entry is `null`, + because it is a dependency or internal change, or because it is paired with + a release-automation cherry-pick. This is a blocking completion check: do + not return output until every input reference has a destination entry. + +## Writing rules + +- Prefer simple past tense: `Added`, `Updated`, `Changed`, `Removed`, + `Fixed`, `Improved`, `Enabled`. +- Use `non-EVM`, not `non EVM` or `Non-EVM`, unless it starts a proper name. +- Use American English: `behavior`, not `behaviour`. +- Do not expose implementation-only identifiers, constants, controller names, + package names, internal flags, or internal APIs unless needed for clarity. + Examples to rewrite: + - `PASSWORD_MIN_LENGTH` -> `the confirmation password is long enough` + - `QuoteMetadata` -> `quote metadata`, unless the type name is essential + - `TextFieldSearch` -> `the updated search field`, unless it is + user-facing terminology +- Do not use vague fragments such as `UI updates`, `Assets unify balance and +traces`, `Local bridge activity label`, or `Bump controller`. +- Treat the supplied PR title as evidence for the matching trailing PR + reference. It can clarify a vague generated entry, but it is not permission + to infer functionality beyond what the title states. +- Do not turn a changelog entry into a guess. If the entry and PR title are + both too vague to safely improve, retain their meaning with minimal + grammatical cleanup. +- When rewriting an entry, retain concrete user-relevant behavior from the + source, including conditions, affected surfaces, and observable outcomes. + Simplify wording, but do not remove specific details merely for brevity + unless the PR or repository evidence shows they are inaccurate or redundant. +- A dependency bump that fixes a user-visible issue should be described by + the user-visible result where that result is clear. +- Do not claim a user-visible result from a dependency bump, feature-flag + plumbing, or internal refactor unless the PR description or relevant diff + explicitly establishes that result. Otherwise, describe the scoped update + conservatively. +- Preserve the distinction between enabling a rollout capability and enabling + the rollout itself. Do not say that a feature was enabled, retired, or + removed when the evidence only adds a feature flag or support for doing so. +- Correct obvious typographical errors, but do not expand a vague entry into a + specific user-facing claim without supporting evidence. +- Keep internal analytics, telemetry, and test-only changes only when they are + already present in the generated section. Put them in `Changed` unless + they clearly belong in another category. Describe them accurately and + conservatively; do not present them as a product improvement or as a bug + fix unless the PR establishes a user-facing defect. +- Remove internal rollout, experiment-treatment, implementation, and service + names when the supported behavior can be stated without them. For example, + write `Reduced unnecessary requests during wallet unlock`, not `Reduced +requests for the treatment discovery layout`. Retain a rollout distinction + only when it is necessary to avoid incorrectly claiming that a feature was + enabled for users. +- Do not include Markdown links inside entry descriptions. Preserve only the + trailing PR reference. +- When several entries have the same generated description and their PR + titles show they are coordinated variants of one release-wide change, + consolidate them into one concise user-facing entry. Include every affected + PR reference in one trailing parenthesized list, in ascending numeric order + (for example, `(#12345, #12346)`). +- Do not consolidate entries merely because their wording is similar. Keep + distinct user-facing changes separate. +- A raw release-automation subject such as `Cherry-picking commits ... for PR +#12345 (#12345)` is still a distinct input PR reference. Resolve its + original PR to improve the wording, but retain that exact trailing PR + reference in the output; do not substitute a different PR merely because it + is also a release-automation or related change. + +## Research rules + +- For an unclear, overly technical, generic, or potentially duplicate entry, + use the matching evidence for its trailing `(#12345)` reference before + rewriting it. Mainline squash commits provide local title/body evidence; + direct-release and cherry-pick entries provide GitHub PR title/body evidence. + Do not infer implementation details when the available evidence is vague. +- Use the PR evidence only to clarify the entry, assign its category, or + identify a coordinated release-wide change. Do not turn a minor + implementation change into an unsupported feature claim. +- Do not include research notes, source paths, PR titles, links, or commit + hashes in the result. Output only the corrected changelog section. +- The `pr_evidence` block is all the research material available. If it does + not answer a question about an entry, keep the entry's existing wording. diff --git a/.github/scripts/clean-release-changelog.mts b/.github/scripts/clean-release-changelog.mts new file mode 100644 index 00000000..6e9daad9 --- /dev/null +++ b/.github/scripts/clean-release-changelog.mts @@ -0,0 +1,973 @@ +import { execFile } from 'node:child_process'; +import { readFileSync, realpathSync, writeFileSync } from 'node:fs'; +import { resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { promisify } from 'node:util'; + +import { + CHANGE_CATEGORIES, + extractReleaseSection, + getEntryPrNumbers, + getPrNumbers, + releaseHeading, + replaceReleaseSection, + splitLines, +} from './release-section.mts'; +import { getEnvironmentWithoutAiKey } from './shared/environment.mts'; +import { writeReport } from './shared/report.mts'; +import { retry } from './shared/retry.mts'; + +const MODEL = 'gpt-5.6-terra'; +const DEFAULT_BASE_URL = 'https://litellm.consensys.info'; +const DEFAULT_TIMEOUT_MS = 90_000; +const MAX_ATTEMPTS = 2; +const MAX_VALIDATION_ATTEMPTS = 3; +const GH_API_MAX_ATTEMPTS = 3; +const GH_API_RETRY_DELAY_MS = 1_000; +const MAX_PR_BODY_LENGTH = 4_000; +const MAX_ENTRIES_PER_CLEAN_ROOM_REQUEST = 20; +// `setTimeout` treats a larger delay as 1 ms. +const MAX_TIMEOUT_MS = 2_147_483_647; + +const evidenceEnvironment = getEnvironmentWithoutAiKey(); +const execFileAsync = promisify(execFile); +const EVIDENCE_MAX_BUFFER = 10 * 1024 * 1024; + +type GhApiRequest = (argumentsList: string[]) => Promise; + +type LiteLlmResponse = { + choices?: { message?: { content?: unknown } }[]; +}; + +/** Thrown when LiteLLM rejects the API key. Retrying cannot help. */ +export class LiteLlmAuthenticationError extends Error { + /** + * Creates the error for a rejected credential. + * + * @param status - The HTTP status LiteLLM returned. + */ + constructor(status: number) { + super(`LiteLLM rejected the credential with HTTP ${String(status)}`); + this.name = 'LiteLlmAuthenticationError'; + } +} + +/** + * Checks whether an error is a rejected-credential error. + * + * @param error - The error to check. + * @returns True for `LiteLlmAuthenticationError`. + */ +export function isLiteLlmAuthenticationError( + error: unknown, +): error is LiteLlmAuthenticationError { + return error instanceof LiteLlmAuthenticationError; +} + +/** + * Reads the per-attempt model timeout from the environment. + * + * @returns The timeout in milliseconds. + * @throws If `CHANGELOG_AI_TIMEOUT_MS` is set but is not a positive integer. + */ +function getTimeoutMs(): number { + const configured = process.env.CHANGELOG_AI_TIMEOUT_MS; + if (configured === undefined) { + return DEFAULT_TIMEOUT_MS; + } + const timeoutMs = Number(configured); + if ( + !Number.isInteger(timeoutMs) || + timeoutMs < 1 || + timeoutMs > MAX_TIMEOUT_MS + ) { + throw new Error( + `CHANGELOG_AI_TIMEOUT_MS must be a positive integer up to ${MAX_TIMEOUT_MS}, got "${configured}"`, + ); + } + return timeoutMs; +} + +/** + * Reads the value following a `--name` command-line flag. + * + * @param name - The flag, including dashes. + * @returns The value, or undefined when the flag is absent. + */ +function getArgument(name: string): string | undefined { + const index = process.argv.indexOf(name); + return index === -1 ? undefined : process.argv[index + 1]; +} + +/** + * Records a failure in the report and fails the process. + * + * @param reportPath - Where to write the report. + * @param stage - The stage that failed. + * @param error - The error message. + */ +function fail(reportPath: string, stage: string, error: string): void { + writeReport(reportPath, { status: 'failed', stage, error }); + console.error(`Clean-room proofread failed during ${stage}: ${error}`); + process.exitCode = 1; +} + +type CleanRoomChunk = { + section: string; + prNumbers: string[]; +}; + +type CategorizedEntry = { + category: string; + text: string; +}; + +/** + * Splits a generated release section into chunks small enough for one model + * request each. Every chunk keeps its entries under the generator's + * categories, which the model may correct. + * + * @param section - The generated release section. + * @param version - The release version. + * @param maxEntries - The most entries per chunk. + * @returns The chunks and the PR numbers each one covers. + * @throws If an entry has no PR reference, partly overlaps an earlier entry's + * PRs, differs from an earlier entry for the same PRs, or the section has no + * entries. An identical repeat is skipped with a warning. + */ +export function splitReleaseSectionIntoChunks( + section: string, + version: string, + maxEntries = MAX_ENTRIES_PER_CLEAN_ROOM_REQUEST, +): CleanRoomChunk[] { + if (!Number.isInteger(maxEntries) || maxEntries < 1) { + throw new Error('maxEntries must be a positive integer'); + } + + const lines = splitLines(section.trimEnd()); + const heading = releaseHeading(version); + if (lines[0] !== heading) { + throw new Error(`Expected section to start with ${heading}`); + } + + const entries: CategorizedEntry[] = []; + let category: string | undefined; + let currentEntry: string[] = []; + const endEntry = () => { + if (category && currentEntry.length > 0) { + entries.push({ category, text: currentEntry.join('\n').trimEnd() }); + } + currentEntry = []; + }; + for (const line of lines.slice(1)) { + if (line.startsWith('- ') && category) { + endEntry(); + currentEntry = [line]; + } else if (line.startsWith('### ')) { + endEntry(); + category = line.slice('### '.length).trim(); + } else if (currentEntry.length > 0) { + currentEntry.push(line); + } else if (line.trim() !== '') { + throw new Error(`Unexpected release-section content: ${line}`); + } + } + endEntry(); + + const chunks: CleanRoomChunk[] = []; + const entryByPrNumber = new Map(); + let chunkEntries = new Map(); + let chunkEntryCount = 0; + let chunkPrNumbers: string[] = []; + const addChunk = () => { + if (chunkEntryCount === 0) { + return; + } + const categories = [...chunkEntries].map( + ([name, texts]) => `### ${name}\n\n${texts.join('\n\n')}`, + ); + chunks.push({ + section: `${heading}\n\n${categories.join('\n\n')}`, + prNumbers: chunkPrNumbers, + }); + chunkEntries = new Map(); + chunkEntryCount = 0; + chunkPrNumbers = []; + }; + + for (const { category: entryCategory, text: entry } of entries) { + const entryPrNumbers = getEntryPrNumbers(entry); + if (entryPrNumbers.length === 0) { + throw new Error( + `Generated changelog entry has no PR reference: ${entry}`, + ); + } + const uniqueEntryPrNumbers = [...new Set(entryPrNumbers)]; + const alreadySeen = uniqueEntryPrNumbers.filter((prNumber) => + entryByPrNumber.has(prNumber), + ); + if (alreadySeen.length === uniqueEntryPrNumbers.length) { + const earlierEntry = entryByPrNumber.get(alreadySeen[0] ?? ''); + if (earlierEntry !== entry) { + throw new Error( + `Generated section has different entries for PR ${alreadySeen.join(', ')}: ${earlierEntry ?? ''} / ${entry}`, + ); + } + console.warn( + `Skipping duplicate entry for PR ${alreadySeen.join(', ')}: ${entry}`, + ); + continue; + } + if (alreadySeen.length > 0) { + throw new Error( + `Generated changelog entry overlaps an earlier PR reference: ${alreadySeen.join(', ')}`, + ); + } + if (chunkEntryCount === maxEntries) { + addChunk(); + } + chunkEntries.set(entryCategory, [ + ...(chunkEntries.get(entryCategory) ?? []), + entry, + ]); + chunkEntryCount += 1; + chunkPrNumbers.push(...uniqueEntryPrNumbers); + uniqueEntryPrNumbers.forEach((prNumber) => + entryByPrNumber.set(prNumber, entry), + ); + } + addChunk(); + + if (chunks.length === 0) { + throw new Error(`Generated ## [${version}] section contains no entries`); + } + return chunks; +} + +const CATEGORY_HEADING = new RegExp( + `^### (${CHANGE_CATEGORIES.join('|')})$`, + 'u', +); + +// Text, then the PR reference at the end with no trailing period, which is +// the form auto-changelog writes and `auto-changelog validate` accepts. +const CANONICAL_ENTRY = /^- .*\S \(#\d+(?:, #\d+)*\)$/u; + +// Two entries the model ran together on one line, as in `(#101) - Next`. +const ADJACENT_LIST_ITEMS = /\(#\d+(?:, #\d+)*\)\.?\s*\+?-\s/u; + +/** + * Rejects a line that holds more than one list item. + * + * @param line - A list line from a model response. + * @throws If the line runs two entries together. + */ +function assertNotAdjacentListItems(line: string): void { + if (ADJACENT_LIST_ITEMS.test(line)) { + throw new Error(`Response contains adjacent list items: ${line}`); + } +} + +/** + * Groups a model response's entries by category, rejecting stray Markdown. + * It is deliberately strict because the input is untrusted model output. + * + * @param section - One chunk's cleaned section. + * @param version - The release version. + * @returns The entry lines for each category. + * @throws If the section has content other than categories and entries. + */ +function getCategorizedBlocks( + section: string, + version: string, +): Map { + const lines = splitLines(section.trimEnd()); + const heading = releaseHeading(version); + if (lines[0] !== heading) { + throw new Error(`Expected section to start with ${heading}`); + } + + const blocks = new Map(); + let category: string | undefined; + let categoryEntries: string[] = []; + const addCategory = () => { + if (!category) { + return; + } + if (categoryEntries.length > 0) { + blocks.set(category, [ + ...(blocks.get(category) ?? []), + ...categoryEntries, + ]); + } + }; + + for (const line of lines.slice(1)) { + const categoryMatch = /^### (.+)$/u.exec(line); + if (categoryMatch) { + addCategory(); + category = categoryMatch[1]; + categoryEntries = []; + } else if (line === '') { + continue; + } else if (category && line.startsWith('- ')) { + assertNotAdjacentListItems(line); + categoryEntries.push(line); + } else if (category) { + throw new Error(`Response contains unexpected Markdown: ${line}`); + } else if (line.trim() !== '') { + throw new Error(`Unexpected release-section content: ${line}`); + } + } + addCategory(); + return blocks; +} + +/** + * Combines the cleaned chunks into one release section, with the categories in + * auto-changelog's order. + * + * @param sections - The cleaned chunk sections. + * @param version - The release version. + * @returns The merged release section. + */ +export function mergeCleanRoomSections( + sections: readonly string[], + version: string, +): string { + const blocksByCategory = new Map(); + for (const section of sections) { + const categorizedBlocks = getCategorizedBlocks(section, version); + for (const [category, blocks] of categorizedBlocks) { + blocksByCategory.set(category, [ + ...(blocksByCategory.get(category) ?? []), + ...blocks, + ]); + } + } + + const categories = CHANGE_CATEGORIES.flatMap((category) => { + const blocks = blocksByCategory.get(category) ?? []; + return blocks.length > 0 ? [`### ${category}\n\n${blocks.join('\n')}`] : []; + }); + return `${releaseHeading(version)}\n\n${categories.join('\n\n')}`; +} + +/** + * Checks that a model's output is a well-formed release section holding + * exactly the expected PRs, so a bad response is never written to the file. + * + * @param section - The section to validate. + * @param version - The release version. + * @param expectedPrNumbers - The PRs the section must reference, once each. + * @throws If the section is malformed or its PR references differ. + */ +export function validateReplacement( + section: string, + version: string, + expectedPrNumbers: readonly string[], +): void { + const heading = releaseHeading(version); + if (!section.startsWith(`${heading}\n`)) { + throw new Error(`Response must start with ${heading}`); + } + if (/^## \[/mu.test(section.slice(heading.length))) { + throw new Error('Response must contain only one release section'); + } + if (/^### Uncategorized$/mu.test(section)) { + throw new Error('Response must not contain Uncategorized'); + } + + const categoryNames = new Set(); + let category: string | undefined; + let categoryEntryCount = 0; + const endCategory = () => { + if (category && categoryEntryCount === 0) { + throw new Error(`Category ${category} must not be empty`); + } + }; + const releaseBody = section.slice(heading.length); + for (const line of splitLines(releaseBody)) { + const categoryName = CATEGORY_HEADING.exec(line)?.[1]; + if (line === '') { + continue; + } else if (categoryName) { + endCategory(); + if (categoryNames.has(categoryName)) { + throw new Error( + `Response contains duplicate ${categoryName} categories`, + ); + } + categoryNames.add(categoryName); + category = categoryName; + categoryEntryCount = 0; + } else if (category && line.startsWith('- ')) { + assertNotAdjacentListItems(line); + if (!CANONICAL_ENTRY.test(line)) { + throw new Error( + `Response entry must be text followed by its PR reference: ${line}`, + ); + } + categoryEntryCount += 1; + } else { + throw new Error(`Response contains unexpected Markdown: ${line}`); + } + } + endCategory(); + if (categoryNames.size === 0) { + throw new Error('Response must contain at least one valid category'); + } + if (/\[[^\]]+\]\([^)]/u.test(releaseBody)) { + throw new Error( + 'Response must not contain Markdown links in entry descriptions', + ); + } + + const actualPrNumbers = getPrNumbers(section); + const expected = new Set(expectedPrNumbers); + const actual = new Set(actualPrNumbers); + if ( + expected.size !== actual.size || + [...expected].some((prNumber) => !actual.has(prNumber)) + ) { + throw new Error( + 'Response PR references do not match the generated section', + ); + } + if (actualPrNumbers.length !== actual.size) { + throw new Error('Response contains duplicate PR references'); + } +} + +type PullRequestMetadata = { + title?: unknown; + body?: unknown; +}; + +type UnavailablePrEvidence = { + prNumber: string; + error: string; +}; + +/** + * Fails when evidence for any PR could not be collected, because the model + * must not rewrite an entry it has no evidence for. + * + * @param unavailablePrEvidence - The PRs whose evidence is missing. + * @throws If the list is not empty. + */ +export function assertPrEvidenceAvailable( + unavailablePrEvidence: readonly UnavailablePrEvidence[], +): void { + if (unavailablePrEvidence.length > 0) { + throw new Error( + `PR evidence unavailable: ${unavailablePrEvidence + .map(({ prNumber, error }) => `#${prNumber} (${error})`) + .join(', ')}`, + ); + } +} + +/** + * Describes an error, including a child process's stderr and stdout. + * + * @param error - The error. + * @returns The details as text. + */ +function getErrorDetails(error: unknown): string { + if (!(error instanceof Error)) { + return String(error); + } + const extendedError = error as Error & { stderr?: unknown; stdout?: unknown }; + return [error.message, extendedError.stderr, extendedError.stdout] + .filter((value): value is string => typeof value === 'string') + .join('\n'); +} + +/** + * Checks whether a `gh api` failure looks transient. + * + * @param error - The failure. + * @returns True for rate limits, 5xx responses, and network errors. + */ +export function isRetryableGhApiFailure(error: unknown): boolean { + return /\bHTTP (?:429|500|502|503|504)\b|rate limit|ECONNRESET|ECONNREFUSED|EAI_AGAIN|ENETUNREACH|ETIMEDOUT|socket hang up/iu.test( + getErrorDetails(error), + ); +} + +/** + * Runs `gh api` without the LiteLLM credential in its environment. + * + * @param argumentsList - The arguments after `gh api`. + * @returns The response body. + */ +async function executeGhApi(argumentsList: string[]): Promise { + const { stdout } = await execFileAsync('gh', ['api', ...argumentsList], { + encoding: 'utf8', + env: evidenceEnvironment, + maxBuffer: EVIDENCE_MAX_BUFFER, + }); + return stdout; +} + +/** + * Runs a `gh api` request, retrying transient failures. + * + * @param argumentsList - The arguments after `gh api`. + * @param request - Runs the request; replaced in tests. + * @param sleep - Waits between attempts; replaced in tests. + * @returns The response body. + */ +export async function requestGhApiWithRetry( + argumentsList: string[], + request: GhApiRequest = executeGhApi, + sleep?: (milliseconds: number) => Promise, +): Promise { + return await retry(async () => request(argumentsList), { + delayMilliseconds: GH_API_RETRY_DELAY_MS, + maxAttempts: GH_API_MAX_ATTEMPTS, + onRetry: ({ attempt, delayMilliseconds }) => { + console.warn( + `Retrying GitHub evidence request after transient failure (attempt ${attempt + 1}/${GH_API_MAX_ATTEMPTS} in ${delayMilliseconds}ms).`, + ); + }, + shouldRetry: isRetryableGhApiFailure, + sleep, + }); +} + +/** + * Builds the `git log` arguments that find the squash commits for PRs. + * + * @param prNumbers - The PR numbers to find. + * @returns The arguments, or undefined when there are no PRs. + */ +export function getLocalEvidenceLogArguments( + prNumbers: readonly string[], +): string[] | undefined { + const uniquePrNumbers = [...new Set(prNumbers)]; + if (uniquePrNumbers.length === 0) { + return undefined; + } + + return [ + 'log', + 'HEAD', + '--extended-regexp', + `--grep=\\(#(${uniquePrNumbers.join('|')})\\)`, + '--format=%H%x1f%s%x1f%b%x1e', + ]; +} + +/** + * Checks whether a commit is reachable from `origin/main`. + * + * @param commitHash - The commit. + * @returns True when it is on main. + */ +async function isCommitOnMain(commitHash: string): Promise { + try { + await execFileAsync( + 'git', + ['merge-base', '--is-ancestor', commitHash, 'origin/main'], + { + env: evidenceEnvironment, + }, + ); + return true; + } catch { + return false; + } +} + +/** + * Collects evidence for PRs from their squash commits already on `main`, + * skipping cherry-picks, so those PRs need no GitHub request. + * + * @param prNumbers - The PR numbers to look up. + * @returns The evidence text by PR number, for the PRs that were found. + */ +export async function getLocalPrEvidence( + prNumbers: readonly string[], +): Promise> { + const argumentsList = getLocalEvidenceLogArguments(prNumbers); + if (!argumentsList) { + return new Map(); + } + + let stdout: string; + try { + ({ stdout } = await execFileAsync('git', argumentsList, { + encoding: 'utf8', + env: evidenceEnvironment, + maxBuffer: EVIDENCE_MAX_BUFFER, + })); + } catch (error) { + console.warn( + `Local PR evidence is unavailable; using GitHub instead: ${getErrorDetails(error)}`, + ); + return new Map(); + } + const wantedPrNumbers = new Set(prNumbers); + const evidence = new Map(); + + for (const record of stdout.split('\x1e')) { + const [hash, subject, body] = record.trim().split('\x1f'); + if ( + !hash || + !subject || + /\bcherry-pick(?:ing)?\b/iu.test(`${subject}\n${body ?? ''}`) + ) { + continue; + } + // A squash commit's own PR is the last one; earlier ones are quoted, as in + // a revert's subject. + const number = [...subject.matchAll(/\(#(\d+)\)/gu)].at(-1)?.[1]; + if (!number || !wantedPrNumbers.has(number) || evidence.has(number)) { + continue; + } + // On a release branch the newest match is often a cherry-pick that is not + // on main, so the mainline commit has to be found behind it. + if (await isCommitOnMain(hash)) { + evidence.set( + number, + `#${number}: ${subject}\nLocal squash commit body:\n${(body ?? '').slice(0, MAX_PR_BODY_LENGTH) || ''}`, + ); + } + } + return evidence; +} + +/** + * Collects each PR's title and body as evidence for the model, preferring + * local evidence and fetching the rest from GitHub. + * + * @param repository - The GitHub repository URL. + * @param prNumbers - The PR numbers to collect. + * @param request - Runs a `gh api` request; replaced in tests. + * @param localEvidence - Evidence already found locally, by PR number. + * @returns The evidence, and the PRs whose evidence could not be fetched. + * @throws If the repository URL cannot be parsed. + */ +export async function getPrEvidence( + repository: string, + prNumbers: readonly string[], + request: GhApiRequest = executeGhApi, + localEvidence: ReadonlyMap = new Map(), +): Promise<{ + evidence: string[]; + unavailablePrEvidence: UnavailablePrEvidence[]; +}> { + const match = /github\.com[/:]([^/]+)\/([^/]+)$/u.exec( + repository.replace(/\.git$/u, ''), + ); + const [, owner, repo] = match ?? []; + if (!owner || !repo) { + throw new Error(`Cannot parse GitHub repository URL: ${repository}`); + } + const evidence: string[] = []; + const unavailablePrEvidence: UnavailablePrEvidence[] = []; + const uniquePrNumbers = [...new Set(prNumbers)]; + console.log( + `Collecting evidence for ${uniquePrNumbers.length} changelog PRs.`, + ); + for (const [index, prNumber] of uniquePrNumbers.entries()) { + const localPrEvidence = localEvidence.get(prNumber); + if (localPrEvidence) { + evidence.push(localPrEvidence); + continue; + } + try { + console.log( + `Collecting evidence for PR ${index + 1}/${uniquePrNumbers.length}.`, + ); + const pullRequest = JSON.parse( + await requestGhApiWithRetry( + [`repos/${owner}/${repo}/pulls/${prNumber}`], + request, + ), + ) as PullRequestMetadata; + const title = + typeof pullRequest.title === 'string' ? pullRequest.title : undefined; + if (!title) { + throw new Error('PR title is unavailable'); + } + const body = + typeof pullRequest.body === 'string' + ? pullRequest.body.slice(0, MAX_PR_BODY_LENGTH) + : ''; + evidence.push(`#${prNumber}: ${title}\nBody:\n${body}`); + } catch (error) { + unavailablePrEvidence.push({ + prNumber, + error: getErrorDetails(error), + }); + } + } + return { evidence, unavailablePrEvidence }; +} + +/** + * Asks the model to rewrite one chunk of entries from the PR evidence. + * + * @param options - Options. + * @param options.prompt - The clean-room instructions. + * @param options.section - The chunk to rewrite. + * @param options.prEvidence - The evidence for the chunk's PRs. + * @param options.apiKey - The LiteLLM API key. + * @param options.fetchImpl - The fetch to use; replaced in tests. + * @param options.sleep - Waits between attempts; replaced in tests. + * @param options.timeoutMs - How long to wait for each attempt. + * @returns The model's Markdown. + * @throws `LiteLlmAuthenticationError` when the key is rejected, otherwise + * the last failure once attempts run out. + */ +export async function requestCleanRoomRewrite({ + prompt, + section, + prEvidence, + apiKey, + fetchImpl = fetch, + sleep, + timeoutMs = getTimeoutMs(), +}: { + prompt: string; + section: string; + prEvidence: readonly string[]; + apiKey: string; + fetchImpl?: typeof fetch; + sleep?: (milliseconds: number) => Promise; + timeoutMs?: number; +}): Promise { + const baseUrl = (process.env.LITELLM_BASE_URL ?? DEFAULT_BASE_URL).replace( + /\/$/u, + '', + ); + // Built through a variable because lint rejects the API's snake_case key. + const maxCompletionTokensField = 'max_completion_tokens'; + const requestBody: Record = { + model: MODEL, + temperature: 0, + [maxCompletionTokensField]: 16_000, + messages: [ + { + role: 'system', + content: 'Return only the requested Markdown release section.', + }, + { + role: 'user', + content: `${prompt}\n\n## Input\n\n\n${section}\n\n\n\n${prEvidence.join('\n\n')}\n`, + }, + ], + }; + + return retry( + async () => { + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), timeoutMs); + try { + const response = await fetchImpl(`${baseUrl}/chat/completions`, { + method: 'POST', + headers: { + Authorization: `Bearer ${apiKey}`, + 'Content-Type': 'application/json', + }, + body: JSON.stringify(requestBody), + signal: controller.signal, + }); + if (!response.ok) { + if (response.status === 401 || response.status === 403) { + throw new LiteLlmAuthenticationError(response.status); + } + throw new Error(`LiteLLM returned HTTP ${String(response.status)}`); + } + + const payload = (await response.json()) as LiteLlmResponse; + const content = payload.choices?.[0]?.message?.content; + if (typeof content !== 'string' || content.trim() === '') { + throw new Error('LiteLLM returned no message content'); + } + return content.trim(); + } finally { + clearTimeout(timeout); + } + }, + { + maxAttempts: MAX_ATTEMPTS, + sleep, + // A rejected credential never succeeds on retry. + shouldRetry: (error) => !isLiteLlmAuthenticationError(error), + onRetry: ({ attempt, delayMilliseconds }) => { + console.warn( + `Retrying clean-room model request (attempt ${attempt + 1}/${MAX_ATTEMPTS} in ${delayMilliseconds}ms).`, + ); + }, + }, + ); +} + +/** Thrown when a model response fails validation. A new response may pass. */ +class RewriteValidationError extends Error { + /** + * Wraps the validation failure, keeping its message. + * + * @param cause - The validation failure. + */ + constructor(cause: unknown) { + super(cause instanceof Error ? cause.message : String(cause), { cause }); + this.name = 'RewriteValidationError'; + } +} + +/** + * Asks the model to rewrite one chunk and validates the response. The model is + * not deterministic even at temperature 0, so a response that fails validation + * is requested again. + * + * @param options - The request, as for `requestCleanRoomRewrite`. + * @param version - The release version. + * @param prNumbers - The PRs the response must reference, once each. + * @param maxAttempts - How many responses to try. + * @returns The first response that passes validation. + * @throws The last validation failure, or any request failure. + */ +export async function requestValidatedRewrite( + options: Parameters[0], + version: string, + prNumbers: readonly string[], + maxAttempts = MAX_VALIDATION_ATTEMPTS, +): Promise { + return retry( + async () => { + const section = await requestCleanRoomRewrite(options); + try { + validateReplacement(section, version, prNumbers); + } catch (error) { + throw new RewriteValidationError(error); + } + return section; + }, + { + delayMilliseconds: 0, + maxAttempts, + shouldRetry: (error) => error instanceof RewriteValidationError, + onRetry: ({ attempt, error }) => { + console.warn( + `Response ${attempt}/${maxAttempts} failed validation, requesting another: ${error instanceof Error ? error.message : String(error)}`, + ); + }, + }, + ); +} + +/** + * Proofreads a release section in place and writes a report. Any failure is + * recorded in the report and leaves the changelog untouched. + */ +async function main(): Promise { + const changelogPath = getArgument('--changelog'); + const version = getArgument('--version'); + const repository = getArgument('--repository'); + const promptPath = getArgument('--prompt'); + const reportPath = + getArgument('--report') ?? '.tmp/release-changelog-ai/report.json'; + + if (!changelogPath || !version || !repository || !promptPath) { + fail( + reportPath, + 'arguments', + 'Required arguments: --changelog, --version, --repository, --prompt', + ); + return; + } + + const apiKey = process.env.AI_ANALYZER_LITELLM_KEY; + if (!apiKey) { + fail(reportPath, 'authentication', 'AI_ANALYZER_LITELLM_KEY is not set'); + return; + } + + try { + const absoluteChangelogPath = resolve(changelogPath); + const changelogContent = readFileSync(absoluteChangelogPath, 'utf8'); + const { section } = extractReleaseSection(changelogContent, version); + const prompt = readFileSync(promptPath, 'utf8'); + const chunks = splitReleaseSectionIntoChunks(section, version); + const sourcePrNumbers = chunks.flatMap(({ prNumbers }) => prNumbers); + console.log(`Clean-room proofreading started with ${MODEL}.`); + const localPrEvidence = await getLocalPrEvidence(sourcePrNumbers); + console.log( + `Using local mainline squash evidence for ${localPrEvidence.size} changelog PRs.`, + ); + const prEvidence = await getPrEvidence( + repository, + sourcePrNumbers, + executeGhApi, + localPrEvidence, + ); + assertPrEvidenceAvailable(prEvidence.unavailablePrEvidence); + console.log( + `Collected evidence for ${prEvidence.evidence.length} changelog PRs.`, + ); + // Each evidence string starts with `#123:`, which keys it by PR. + const evidenceByPrNumber = new Map( + prEvidence.evidence.flatMap((evidence) => { + const match = /^#(\d+):/u.exec(evidence); + return match?.[1] ? [[match[1], evidence]] : []; + }), + ); + const cleanRoomSections: string[] = []; + for (const [index, chunk] of chunks.entries()) { + const chunkEvidence = chunk.prNumbers.map((prNumber) => { + const evidence = evidenceByPrNumber.get(prNumber); + if (!evidence) { + throw new Error(`PR evidence is unavailable for #${prNumber}`); + } + return evidence; + }); + console.log( + `Requesting clean-room rewrite for chunk ${index + 1}/${chunks.length} (${chunk.prNumbers.length} PRs).`, + ); + const cleanRoomSection = await requestValidatedRewrite( + { + prompt, + section: chunk.section, + prEvidence: chunkEvidence, + apiKey, + }, + version, + chunk.prNumbers, + ); + cleanRoomSections.push(cleanRoomSection); + console.log(`Validated clean-room chunk ${index + 1}/${chunks.length}.`); + } + const replacement = mergeCleanRoomSections(cleanRoomSections, version); + validateReplacement(replacement, version, sourcePrNumbers); + console.log('Validated merged clean-room release section.'); + + // Nothing is written until every chunk and the merged result validate. + writeFileSync( + absoluteChangelogPath, + replaceReleaseSection(changelogContent, version, replacement), + ); + writeReport(reportPath, { status: 'succeeded', model: MODEL }); + console.log(`Clean-room proofreading completed with ${MODEL}.`); + } catch (error) { + const stage = isLiteLlmAuthenticationError(error) + ? 'authentication' + : 'cleaning'; + fail( + reportPath, + stage, + error instanceof Error ? error.message : String(error), + ); + } +} + +if ( + process.argv[1] && + realpathSync(process.argv[1]) === fileURLToPath(import.meta.url) +) { + await main(); +} diff --git a/.github/scripts/merge-previous-release-changelog.mts b/.github/scripts/merge-previous-release-changelog.mts index e830eb25..bf547fa8 100644 --- a/.github/scripts/merge-previous-release-changelog.mts +++ b/.github/scripts/merge-previous-release-changelog.mts @@ -18,10 +18,13 @@ import { execFileSync } from 'node:child_process'; import { readFileSync, writeFileSync } from 'node:fs'; -// Any `[label]: url` line, including `[Unreleased]: url`. -const LINK_REFERENCE = /^\[[^\]]+\]: /mu; -// A release section ends at the next heading or at the link references. -const SECTION_END = /^(?:## \[|\[[^\]]+\]: )/mu; +import { + detectLineEnding, + extractReleaseSection, + getReleaseVersions, + hasSingleReleaseHeading, + LINK_REFERENCE, +} from './release-section.mts'; /** * Compares two `major.minor.patch` versions. @@ -42,55 +45,6 @@ function compareVersions(left: string, right: string): number { return 0; } -/** - * Escapes a string for literal use inside a regular expression. - * @param value - The string to escape. - * @returns The escaped string. - */ -function escapeRegex(value: string): string { - return value.replace(/[.*+?^${}()|[\]\\]/gu, '\\$&'); -} - -/** - * Finds every `## [version]` heading for a version. - * @param changelog - The changelog contents. - * @param version - The release version. - * @returns All matching headings. - */ -function getReleaseHeadingMatches( - changelog: string, - version: string, -): RegExpMatchArray[] { - const heading = `## [${version}]`; - return [ - ...changelog.matchAll(new RegExp(`^${escapeRegex(heading)}$`, 'gmu')), - ]; -} - -/** - * Extracts the section for a release, if it appears exactly once. - * @param changelog - The changelog contents. - * @param version - The release version. - * @returns The section text, or undefined when missing or ambiguous. - */ -function getReleaseSection( - changelog: string, - version: string, -): string | undefined { - const matches = getReleaseHeadingMatches(changelog, version); - // A duplicated heading means we can't tell which copy is the real one. - if (matches.length !== 1 || matches[0]?.index === undefined) { - return undefined; - } - - const start = matches[0].index; - const bodyStart = start + matches[0][0].length; - const bodyEnd = changelog.slice(bodyStart).search(SECTION_END); - return changelog - .slice(start, bodyEnd === -1 ? undefined : bodyStart + bodyEnd) - .trimEnd(); -} - /** * Finds the `[version]: url` link reference for a release. * @param changelog - The changelog contents. @@ -103,7 +57,7 @@ function getReleaseReference( ): string | undefined { const matches = [ ...changelog.matchAll( - new RegExp(`^\\[${escapeRegex(version)}\\]: .+$`, 'gmu'), + new RegExp(`^\\[${version.replaceAll('.', '\\.')}\\]: .+$`, 'gmu'), ), ]; return matches.length === 1 ? matches[0]?.[0] : undefined; @@ -217,7 +171,9 @@ function main(): void { encoding: 'utf8', maxBuffer: 64 * 1024 * 1024, }); - const section = getReleaseSection(source, version); + const section = hasSingleReleaseHeading(source, version) + ? extractReleaseSection(source, version).section + : undefined; // Only a finalized release has a versioned section; an unreleased or // duplicated one is not safe to copy. @@ -229,7 +185,7 @@ function main(): void { } // The destination already records this release, so there is nothing to // restore and inserting would duplicate the heading. - if (getReleaseHeadingMatches(destination, version).length > 0) { + if (getReleaseVersions(destination).includes(version)) { console.warn( `Destination already has a ${version} section; preserving destination changelog.`, ); @@ -237,7 +193,7 @@ function main(): void { } // Match the destination so the diff doesn't rewrite every line ending. - const lineEnding = destination.includes('\r\n') ? '\r\n' : '\n'; + const lineEnding = detectLineEnding(destination); const withSection = insertReleaseSection( destination, section, diff --git a/.github/scripts/release-section.mts b/.github/scripts/release-section.mts new file mode 100644 index 00000000..c7f362b2 --- /dev/null +++ b/.github/scripts/release-section.mts @@ -0,0 +1,192 @@ +// Pure helpers for reading and editing a `## [VERSION]` changelog section. +// Nothing here touches the filesystem, Git, or the network. + +export const CHANGE_CATEGORIES = [ + 'Added', + 'Changed', + 'Deprecated', + 'Removed', + 'Fixed', + 'Security', +] as const; + +export type ReleaseSection = { + section: string; + start: number; + end: number; +}; + +// Any `[label]: url` line, including `[Unreleased]: url`. +export const LINK_REFERENCE = /^\[[^\]]+\]: /mu; + +// A release section ends at the next heading or at the link references. +const SECTION_END = /^(?:## \[|\[[^\]]+\]: )/mu; + +/** + * Builds the `## [version]` heading line. + * + * @param version - The release version. + * @returns The heading. + */ +export function releaseHeading(version: string): string { + return `## [${version}]`; +} + +/** + * Builds a global regex matching a release's heading line. + * + * @param version - The release version. + * @returns The pattern. + */ +function releaseHeadingPattern(version: string): RegExp { + return new RegExp(`^## \\[${version.replaceAll('.', '\\.')}\\]$`, 'gmu'); +} + +/** + * Checks that a release's heading appears exactly once. + * + * @param content - The changelog contents. + * @param version - The release version. + * @returns True when there is exactly one matching heading. + */ +export function hasSingleReleaseHeading( + content: string, + version: string, +): boolean { + return content.match(releaseHeadingPattern(version))?.length === 1; +} + +/** + * Reads the version out of a release heading line. + * + * @param line - A single line. + * @returns The version, or undefined when the line is not a release heading. + */ +export function parseReleaseHeadingVersion(line: string): string | undefined { + return /^## \[(\d+\.\d+\.\d+)\]$/u.exec(line)?.[1]; +} + +/** + * Lists every released version heading, in file order. + * + * @param content - The changelog contents. + * @returns The versions. `Unreleased` is not included. + */ +export function getReleaseVersions(content: string): string[] { + return splitLines(content).flatMap((line) => { + const version = parseReleaseHeadingVersion(line); + return version ? [version] : []; + }); +} + +/** + * Detects whether text uses CRLF or LF line endings. + * + * @param text - The text to inspect. + * @returns CRLF if the text contains any, otherwise LF. + */ +export function detectLineEnding(text: string): '\r\n' | '\n' { + return text.includes('\r\n') ? '\r\n' : '\n'; +} + +/** + * Splits text into lines regardless of line ending. + * + * @param text - The text to split. + * @returns The lines, without line endings. + */ +export function splitLines(text: string): string[] { + return text.split(/\r?\n/u); +} + +/** + * Finds a release's section and where it sits in the changelog. + * + * @param content - The changelog contents. + * @param version - The release version. + * @returns The section text and its start and end offsets. + * @throws If the release heading is missing or duplicated. + */ +export function extractReleaseSection( + content: string, + version: string, +): ReleaseSection { + const heading = releaseHeading(version); + const matches = [...content.matchAll(releaseHeadingPattern(version))]; + const start = matches[0]?.index; + if (matches.length !== 1 || start === undefined) { + throw new Error(`Expected exactly one release heading for ${version}`); + } + + const bodyStart = start + heading.length; + const bodyEnd = content.slice(bodyStart).search(SECTION_END); + const section = content + .slice(start, bodyEnd === -1 ? undefined : bodyStart + bodyEnd) + .trimEnd(); + + return { + section, + start, + end: start + section.length, + }; +} + +/** + * Replaces a release's section in place. + * + * @param content - The changelog contents. + * @param version - The release version. + * @param replacement - The new section text, with any line endings. + * @returns The updated changelog, with the replacement in its line endings. + */ +export function replaceReleaseSection( + content: string, + version: string, + replacement: string, +): string { + const { start, end } = extractReleaseSection(content, version); + const normalized = replacement.replace(/\r?\n/gu, detectLineEnding(content)); + return `${content.slice(0, start)}${normalized}${content.slice(end)}`; +} + +// Mirrors auto-changelog's parser, which reads these groups from an entry's +// first line: `(#123)`, `(#123, #124)`, and the legacy `(#123), (#124)`. +const PR_REFERENCE_GROUPS = /\s+(?:\(#\d+(?:\s*,\s*#\d+)*\)\s*,?\s*)+/gu; + +/** + * Reads the PR numbers in an entry the way auto-changelog does. Other `#123` + * text, such as an issue number, is not a PR reference. + * + * @param entry - One changelog entry. + * @returns The PR numbers, in order, or none when the first line has no reference group. + */ +export function getEntryPrNumbers(entry: string): string[] { + const firstLine = splitLines(entry)[0] ?? ''; + return [...firstLine.matchAll(PR_REFERENCE_GROUPS)].flatMap(([group]) => + [...group.matchAll(/#(\d+)/gu)].flatMap(([, prNumber]) => + prNumber ? [prNumber] : [], + ), + ); +} + +/** + * Collects the PR numbers of every entry in a section. + * + * @param section - The section text. + * @returns The PR numbers, as strings, in order of appearance. + */ +export function getPrNumbers(section: string): string[] { + const entries: string[][] = []; + let currentEntry: string[] | undefined; + for (const line of splitLines(section)) { + if (line.startsWith('- ')) { + currentEntry = [line]; + entries.push(currentEntry); + } else if (line.startsWith('## ') || line.startsWith('### ')) { + currentEntry = undefined; + } else if (currentEntry && line !== '') { + currentEntry.push(line); + } + } + return entries.flatMap((entry) => getEntryPrNumbers(entry.join('\n'))); +} diff --git a/.github/scripts/shared/environment.mts b/.github/scripts/shared/environment.mts new file mode 100644 index 00000000..91a893e4 --- /dev/null +++ b/.github/scripts/shared/environment.mts @@ -0,0 +1,14 @@ +/** + * Copies an environment without the LiteLLM credential. Child processes other + * than the clean-room cleaner must never see it. + * + * @param environment - The environment to copy. Defaults to the current one. + * @returns The copy, without `AI_ANALYZER_LITELLM_KEY`. + */ +export function getEnvironmentWithoutAiKey( + environment: NodeJS.ProcessEnv = process.env, +): NodeJS.ProcessEnv { + const scrubbedEnvironment = { ...environment }; + delete scrubbedEnvironment.AI_ANALYZER_LITELLM_KEY; + return scrubbedEnvironment; +} diff --git a/.github/scripts/shared/report.mts b/.github/scripts/shared/report.mts new file mode 100644 index 00000000..3fecfdb0 --- /dev/null +++ b/.github/scripts/shared/report.mts @@ -0,0 +1,16 @@ +import { mkdirSync, writeFileSync } from 'node:fs'; +import { dirname } from 'node:path'; + +/** + * Writes a JSON report, creating its directory if needed. + * + * @param reportPath - Where to write the report. + * @param report - The report fields. + */ +export function writeReport( + reportPath: string, + report: Record, +): void { + mkdirSync(dirname(reportPath), { recursive: true }); + writeFileSync(reportPath, `${JSON.stringify(report, null, 2)}\n`); +} diff --git a/.github/scripts/shared/retry.mts b/.github/scripts/shared/retry.mts new file mode 100644 index 00000000..23d1a30c --- /dev/null +++ b/.github/scripts/shared/retry.mts @@ -0,0 +1,71 @@ +/** Options for `retry`. */ +export type RetryOptions = { + delayMilliseconds?: number; + maxAttempts?: number; + onRetry?: (details: { + attempt: number; + delayMilliseconds: number; + error: unknown; + }) => void; + shouldRetry?: (error: unknown) => boolean; + sleep?: ((milliseconds: number) => Promise) | undefined; +}; + +/** + * Waits for a number of milliseconds. + * + * @param milliseconds - How long to wait. + */ +async function defaultSleep(milliseconds: number): Promise { + await new Promise((resolveDelay) => setTimeout(resolveDelay, milliseconds)); +} + +/** + * Runs an operation, retrying failures with a linearly growing delay. + * + * @param operation - The operation to run. + * @param options - Retry behavior. + * @param options.delayMilliseconds - The base delay, multiplied by the attempt. + * @param options.maxAttempts - The total number of attempts. + * @param options.onRetry - Called before each delay. + * @param options.shouldRetry - Returns false to rethrow without retrying. + * @param options.sleep - Waits between attempts; replaced in tests. + * @returns The operation's result. + * @throws The last error, once attempts run out or `shouldRetry` declines. + */ +export async function retry( + operation: () => Promise, + { + delayMilliseconds = 1_000, + maxAttempts = 3, + onRetry, + shouldRetry = () => true, + sleep = defaultSleep, + }: RetryOptions = {}, +): Promise { + if (!Number.isInteger(maxAttempts) || maxAttempts < 1) { + throw new Error('maxAttempts must be a positive integer'); + } + if (!Number.isFinite(delayMilliseconds) || delayMilliseconds < 0) { + throw new Error('delayMilliseconds must be a non-negative number'); + } + + for (let attempt = 1; attempt <= maxAttempts; attempt += 1) { + try { + return await operation(); + } catch (error) { + if (attempt === maxAttempts || !shouldRetry(error)) { + throw error; + } + const retryDelayMilliseconds = delayMilliseconds * attempt; + onRetry?.({ + attempt, + delayMilliseconds: retryDelayMilliseconds, + error, + }); + await sleep(retryDelayMilliseconds); + } + } + + throw new Error('Retry attempts exhausted'); +} diff --git a/.github/scripts/tests/clean-release-changelog.test.mts b/.github/scripts/tests/clean-release-changelog.test.mts new file mode 100644 index 00000000..325c2360 --- /dev/null +++ b/.github/scripts/tests/clean-release-changelog.test.mts @@ -0,0 +1,1012 @@ +import assert from 'node:assert/strict'; +import { execFile, execFileSync } from 'node:child_process'; +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { createServer } from 'node:http'; +import type { ServerResponse } from 'node:http'; +import type { AddressInfo } from 'node:net'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import test from 'node:test'; +import { fileURLToPath } from 'node:url'; +import { promisify } from 'node:util'; + +import { getGitTestEnvironment } from './git-test-environment.mts'; +import { + assertPrEvidenceAvailable, + getLocalEvidenceLogArguments, + getLocalPrEvidence, + getPrEvidence, + isRetryableGhApiFailure, + LiteLlmAuthenticationError, + isLiteLlmAuthenticationError, + mergeCleanRoomSections, + requestCleanRoomRewrite, + requestGhApiWithRetry, + requestValidatedRewrite, + splitReleaseSectionIntoChunks, + validateReplacement, +} from '../clean-release-changelog.mts'; +import { + extractReleaseSection, + getEntryPrNumbers, + getPrNumbers, + replaceReleaseSection, +} from '../release-section.mts'; +import { getEnvironmentWithoutAiKey } from '../shared/environment.mts'; + +const generatedSection = `## [13.51.0] + +### Uncategorized + +- Added a new flow (#101) +- Fixed an issue (#102)`; + +test('removes LiteLLM credentials from evidence subprocesses', () => { + assert.deepEqual( + getEnvironmentWithoutAiKey({ + GH_TOKEN: 'github-token', + AI_ANALYZER_LITELLM_KEY: 'litellm-key', + }), + { GH_TOKEN: 'github-token' }, + ); +}); + +test('retries transient GitHub API failures before returning evidence', async () => { + let attempts = 0; + const retryDelays: number[] = []; + const response = await requestGhApiWithRetry( + ['repos/MetaMask/metamask-extension/pulls/1'], + async () => { + attempts += 1; + if (attempts < 3) { + throw new Error('HTTP 502: Bad Gateway'); + } + return '{"title":"Recovered"}'; + }, + async (milliseconds) => { + retryDelays.push(milliseconds); + }, + ); + + assert.equal(response, '{"title":"Recovered"}'); + assert.equal(attempts, 3); + assert.deepEqual(retryDelays, [1_000, 2_000]); +}); + +test('does not retry permanent GitHub API failures', async () => { + let attempts = 0; + await assert.rejects( + async () => + requestGhApiWithRetry( + ['repos/MetaMask/metamask-extension/pulls/1'], + async () => { + attempts += 1; + throw new Error('HTTP 404: Not Found'); + }, + async () => { + throw new Error('Unexpected retry'); + }, + ), + /HTTP 404/u, + ); + assert.equal(attempts, 1); + assert.equal( + isRetryableGhApiFailure(new Error('HTTP 404: Not Found')), + false, + ); + // The command line names the PR, which is not an HTTP status. + assert.equal( + isRetryableGhApiFailure( + new Error( + 'Command failed: gh api repos/o/r/pulls/503\ngh: Not Found (HTTP 404)', + ), + ), + false, + ); +}); + +test('uses PR title and body when local evidence is unavailable', async () => { + const requests: string[] = []; + const evidence = await getPrEvidence( + 'https://github.com/MetaMask/metamask-mobile', + ['37469'], + async (argumentsList) => { + requests.push(argumentsList[0] ?? ''); + if (argumentsList[0]?.endsWith('/pulls/37469')) { + return JSON.stringify({ + title: 'fix: match the recurring every sheet to expires after', + body: 'The recurring interval picker now matches the expiration sheet.', + }); + } + throw new Error('Unexpected GitHub request'); + }, + ); + + assert.deepEqual(evidence.unavailablePrEvidence, []); + assert.match( + evidence.evidence[0] ?? '', + /#37469: fix: match the recurring every sheet/u, + ); + assert.equal(requests.length, 1); +}); + +test('collects local evidence only from mainline squash commits', async () => { + const directory = mkdtempSync(join(tmpdir(), 'local-evidence-')); + const originalDirectory = process.cwd(); + const git = (...argumentsList: string[]) => + execFileSync('git', argumentsList, { + cwd: directory, + env: getGitTestEnvironment(), + }); + const commit = (message: string) => + git('commit', '--allow-empty', '-m', message); + + try { + git('init', '--initial-branch', 'main'); + commit('feat: add thing (#1)\n\nBody one'); + commit('feat: real thing (#4)\n\nReal body'); + commit('Revert "feat: real thing (#4)" (#5)\n\nRevert body'); + commit('chore: backport (#2)\n\nThis is a cherry-pick of another commit.'); + // `origin/main` is what marks a commit as mainline. + git('update-ref', 'refs/remotes/origin/main', 'HEAD'); + commit('feat: side branch only (#3)'); + // A newer copy of #1 that is not on main must not hide the mainline one. + commit('feat: add thing for the release (#1)\n\nRelease copy'); + + process.chdir(directory); + const evidence = await getLocalPrEvidence(['1', '2', '3', '4']); + + // A revert quotes the PR it undoes but is not evidence for that PR. + assert.deepEqual([...evidence].sort(), [ + ['1', '#1: feat: add thing (#1)\nLocal squash commit body:\nBody one'], + ['4', '#4: feat: real thing (#4)\nLocal squash commit body:\nReal body'], + ]); + } finally { + process.chdir(originalDirectory); + rmSync(directory, { force: true, recursive: true }); + } +}); + +test('falls back to GitHub when local history cannot be read', async (context) => { + const directory = mkdtempSync(join(tmpdir(), 'no-repo-')); + const originalDirectory = process.cwd(); + const warn = context.mock.method(console, 'warn', () => undefined); + + try { + process.chdir(directory); + assert.deepEqual([...(await getLocalPrEvidence(['1']))], []); + assert.equal(warn.mock.callCount(), 1); + } finally { + process.chdir(originalDirectory); + rmSync(directory, { force: true, recursive: true }); + } +}); + +test('does not query GitHub when local mainline evidence exists', async () => { + const evidence = await getPrEvidence( + 'https://github.com/MetaMask/metamask-mobile', + ['37469'], + async () => { + throw new Error('GitHub evidence was not expected'); + }, + new Map([ + [ + '37469', + '#37469: fix: match the recurring every sheet to expires after\nLocal squash commit body:\nThe interval picker now matches.', + ], + ]), + ); + + assert.deepEqual(evidence.unavailablePrEvidence, []); + assert.deepEqual(evidence.evidence, [ + '#37469: fix: match the recurring every sheet to expires after\nLocal squash commit body:\nThe interval picker now matches.', + ]); +}); + +test('limits local evidence history to referenced PR numbers', () => { + assert.deepEqual(getLocalEvidenceLogArguments([]), undefined); + assert.deepEqual(getLocalEvidenceLogArguments(['37469', '37522', '37469']), [ + 'log', + 'HEAD', + '--extended-regexp', + '--grep=\\(#(37469|37522)\\)', + '--format=%H%x1f%s%x1f%b%x1e', + ]); +}); + +test('identifies LiteLLM credential rejections', () => { + const error = new LiteLlmAuthenticationError(401); + assert.equal(error.name, 'LiteLlmAuthenticationError'); + assert.match(error.message, /HTTP 401/u); + assert.equal(isLiteLlmAuthenticationError(error), true); + assert.equal(isLiteLlmAuthenticationError(new Error('HTTP 401')), false); +}); + +test('cleans large release sections in reference-preserving chunks', () => { + const section = `## [13.51.0] + +### Uncategorized + +- Added a new flow (#101) +- Fixed an issue (#102) + +### Fixed + +- Improved a flow (#103)`; + assert.deepEqual(splitReleaseSectionIntoChunks(section, '13.51.0', 2), [ + { + section: `## [13.51.0] + +### Uncategorized + +- Added a new flow (#101) + +- Fixed an issue (#102)`, + prNumbers: ['101', '102'], + }, + { + section: `## [13.51.0] + +### Fixed + +- Improved a flow (#103)`, + prNumbers: ['103'], + }, + ]); + const merged = mergeCleanRoomSections( + [ + `## [13.51.0] + +### Added + +- Added a new flow (#101) + +### Fixed + +- Fixed an issue (#102)`, + `## [13.51.0] + +### Fixed + +- Improved a flow (#103)`, + ], + '13.51.0', + ); + assert.doesNotThrow(() => + validateReplacement(merged, '13.51.0', ['101', '102', '103']), + ); + assert.doesNotMatch(merged, /\)\n\n- /u); +}); + +test('warns about and skips an identical repeat of an entry', (context) => { + const warn = context.mock.method(console, 'warn', () => undefined); + const chunks = splitReleaseSectionIntoChunks( + `## [13.51.0] + +### Fixed + +- Fixed an issue (#101) + +### Changed + +- Fixed an issue (#101)`, + '13.51.0', + ); + + assert.deepEqual(chunks[0]?.prNumbers, ['101']); + assert.equal(warn.mock.callCount(), 1); + assert.match(String(warn.mock.calls[0]?.arguments[0]), /PR 101/u); +}); + +test('rejects different entries for the same PR instead of dropping one', () => { + assert.throws( + () => + splitReleaseSectionIntoChunks( + `## [13.51.0] + +### Fixed + +- Fixed an issue (#101) +- Fixed another issue (#101)`, + '13.51.0', + ), + /different entries for PR 101/u, + ); +}); + +test('extracts exactly one release section', () => { + const changelog = `# Changelog + +${generatedSection} + +## [13.50.3] + +### Fixed + +- An earlier fix (#100) +`; + + assert.equal( + extractReleaseSection(changelog, '13.51.0').section, + generatedSection, + ); + assert.deepEqual(getPrNumbers(generatedSection), ['101', '102']); +}); + +test('counts the PR groups auto-changelog counts', () => { + assert.deepEqual( + getPrNumbers(`## [1.0.0] + +### Fixed + +- Fixed issue #4521 in the send flow (#101, #102). +- Reverted "Fixed it (#67)" (#69) +- Added a flow with no reference`), + ['101', '102', '67', '69'], + ); + // auto-changelog reads the first line, wherever the group sits on it. + assert.deepEqual( + getEntryPrNumbers('- Fixed it (#7)\n - nested detail (#8)'), + ['7'], + ); + assert.deepEqual(getEntryPrNumbers('- Fixed it (#7), (#8)'), ['7', '8']); + assert.deepEqual(getEntryPrNumbers('- Fixed swap from (#46000) (#101)'), [ + '46000', + '101', + ]); + assert.deepEqual(getEntryPrNumbers('- (#101)'), ['101']); +}); + +test('rejects a duplicate release heading', () => { + assert.throws( + () => + extractReleaseSection( + `${generatedSection}\n\n${generatedSection}`, + '13.51.0', + ), + /Expected exactly one release heading/u, + ); +}); + +test('replaces only the release section without changing its surrounding whitespace', () => { + const changelog = `# Changelog + +${generatedSection} + +## [13.50.3] + +### Fixed + +- An earlier fix (#100) +`; + const replacement = `## [13.51.0] + +### Added + +- Added a clean flow (#101) + +### Fixed + +- Fixed an issue (#102)`; + + assert.equal( + replaceReleaseSection(changelog, '13.51.0', replacement), + changelog.replace(generatedSection, replacement), + ); +}); + +test('accepts a categorized replacement that preserves every PR once', () => { + assert.doesNotThrow(() => + validateReplacement( + `## [13.51.0] + +### Added + +- Added a new flow (#101) + +### Fixed + +- Fixed an issue (#102)`, + '13.51.0', + ['101', '102'], + ), + ); +}); + +test('rejects uncategorized, omitted, invented, and duplicate PR references', () => { + assert.throws( + () => validateReplacement(generatedSection, '13.51.0', ['101', '102']), + /Uncategorized/u, + ); + assert.throws( + () => + validateReplacement( + `## [13.51.0] + +### Fixed + +- Fixed an issue (#101)`, + '13.51.0', + ['101', '102'], + ), + /references do not match/u, + ); + assert.throws( + () => + validateReplacement( + `## [13.51.0] + +### Fixed + +- Fixed an issue (#101, #102, #103)`, + '13.51.0', + ['101', '102'], + ), + /references do not match/u, + ); + assert.throws( + () => + validateReplacement( + `## [13.51.0] + +### Fixed + +- Fixed an issue (#101) +- Fixed it again (#101) +- Fixed another issue (#102)`, + '13.51.0', + ['101', '102'], + ), + /duplicate PR references/u, + ); + assert.throws( + () => + validateReplacement( + `## [13.51.0] + +Here is the cleaned section: + +### Fixed + +- Fixed an issue (#101, #102)`, + '13.51.0', + ['101', '102'], + ), + /unexpected Markdown/u, + ); + assert.throws( + () => + validateReplacement( + `## [13.51.0] + +### Fixed + +- Fixed an issue (#101)- Fixed another issue (#102)`, + '13.51.0', + ['101', '102'], + ), + /adjacent list items/u, + ); +}); + +test('rejects an entry without a trailing PR reference', () => { + assert.throws( + () => + validateReplacement( + `## [13.51.0] + +### Fixed + +- Fixed an issue (#101) +- Improved performance across the app`, + '13.51.0', + ['101'], + ), + /must be text followed by its PR reference/u, + ); +}); + +test('rejects entries that are not canonical or hide a PR in the text', () => { + const entry = (line: string) => `## [13.51.0]\n\n### Fixed\n\n${line}`; + assert.throws( + () => validateReplacement(entry('- Fixed X (#101).'), '13.51.0', ['101']), + /must be text followed by its PR reference/u, + ); + assert.throws( + () => validateReplacement(entry('- (#101)'), '13.51.0', ['101']), + /must be text followed by its PR reference/u, + ); + assert.throws( + () => + validateReplacement( + entry('- Fixed swap from (#46000) (#101)'), + '13.51.0', + ['101'], + ), + /references do not match/u, + ); +}); + +test('accepts dashes and parentheses inside an entry', () => { + assert.doesNotThrow(() => + validateReplacement( + `## [13.51.0] + +### Fixed + +- Updated fees (beta) - now shown in fiat (#101)`, + '13.51.0', + ['101'], + ), + ); +}); + +type FetchResponse = Awaited>; +type RecordedRequest = { url: string; init: RequestInit }; + +/** + * Builds a minimal fetch response. + * + * @param status - The HTTP status. + * @param body - The JSON body. + * @returns The response. + */ +function fakeResponse(status: number, body: unknown = {}): FetchResponse { + return { + ok: status >= 200 && status < 300, + status, + json: async () => body, + } as FetchResponse; +} + +/** + * Builds a fake fetch that returns or throws the given results in order. + * + * @param responses - The results, consumed one per request. + * @param requests - Collects each request made. + * @returns The fake fetch. + */ +function createFetch( + responses: (FetchResponse | Error)[], + requests: RecordedRequest[] = [], +): typeof fetch { + return (async (url: Parameters[0], init?: RequestInit) => { + requests.push({ url: String(url), init: init ?? {} }); + const next = responses.shift(); + if (!next) { + throw new Error('Unexpected request'); + } + if (next instanceof Error) { + throw next; + } + return next; + }) as typeof fetch; +} + +/** + * Builds a successful chat-completions response. + * + * @param content - The message content. + * @returns The response. + */ +function completion(content: unknown): FetchResponse { + return fakeResponse(200, { choices: [{ message: { content } }] }); +} + +const rewriteInput = { + prompt: 'Prompt', + section: generatedSection, + prEvidence: ['#101: Title'], + apiKey: 'litellm-key', +}; + +/** + * Puts an environment variable back to its earlier value. + * + * @param name - The variable. + * @param value - Its earlier value, or undefined if it was unset. + */ +function restoreEnvironmentVariable( + name: string, + value: string | undefined, +): void { + if (value === undefined) { + delete process.env[name]; + } else { + process.env[name] = value; + } +} + +for (const invalidTimeout of ['soon', '0', '3000000000']) { + test(`rejects the timeout "${invalidTimeout}" in the environment`, async () => { + const originalTimeout = process.env.CHANGELOG_AI_TIMEOUT_MS; + restoreEnvironmentVariable('CHANGELOG_AI_TIMEOUT_MS', invalidTimeout); + try { + await assert.rejects( + async () => + requestCleanRoomRewrite({ + ...rewriteInput, + fetchImpl: createFetch([]), + }), + /CHANGELOG_AI_TIMEOUT_MS must be a positive integer/u, + ); + } finally { + restoreEnvironmentVariable('CHANGELOG_AI_TIMEOUT_MS', originalTimeout); + } + }); +} + +test('sends the pinned model with the bearer credential to chat completions', async () => { + const requests: RecordedRequest[] = []; + const result = await requestCleanRoomRewrite({ + ...rewriteInput, + fetchImpl: createFetch([completion(' ## [13.51.0] ')], requests), + sleep: async () => undefined, + timeoutMs: 1_000, + }); + + assert.equal(result, '## [13.51.0]'); + assert.equal(requests.length, 1); + assert.match(requests[0]?.url ?? '', /\/chat\/completions$/u); + assert.equal(requests[0]?.init.method, 'POST'); + const headers = requests[0]?.init.headers as Record; + assert.equal(headers.Authorization, 'Bearer litellm-key'); + const body = JSON.parse(String(requests[0]?.init.body)) as { + model: string; + temperature: number; + messages: { content: string }[]; + }; + assert.equal(body.model, 'gpt-5.6-terra'); + assert.equal(body.temperature, 0); + assert.match(body.messages[1]?.content ?? '', //u); + assert.doesNotMatch(JSON.stringify(body), /litellm-key/u); +}); + +test('does not retry a rejected LiteLLM credential', async () => { + for (const status of [401, 403]) { + const requests: RecordedRequest[] = []; + const sleeps: number[] = []; + await assert.rejects( + async () => + requestCleanRoomRewrite({ + ...rewriteInput, + fetchImpl: createFetch([fakeResponse(status)], requests), + sleep: async (milliseconds) => { + sleeps.push(milliseconds); + }, + timeoutMs: 1_000, + }), + (error: unknown) => + isLiteLlmAuthenticationError(error) && + error.message.includes(`HTTP ${String(status)}`), + ); + assert.equal(requests.length, 1); + assert.deepEqual(sleeps, []); + } +}); + +test('retries one transient LiteLLM failure and then succeeds', async () => { + const requests: RecordedRequest[] = []; + const sleeps: number[] = []; + const result = await requestCleanRoomRewrite({ + ...rewriteInput, + fetchImpl: createFetch( + [fakeResponse(503), completion('## [13.51.0]')], + requests, + ), + sleep: async (milliseconds) => { + sleeps.push(milliseconds); + }, + timeoutMs: 1_000, + }); + + assert.equal(result, '## [13.51.0]'); + assert.equal(requests.length, 2); + assert.deepEqual(sleeps, [1_000]); +}); + +test('stops after the retry budget for persistent LiteLLM failures', async () => { + const requests: RecordedRequest[] = []; + await assert.rejects( + async () => + requestCleanRoomRewrite({ + ...rewriteInput, + fetchImpl: createFetch( + [fakeResponse(500), fakeResponse(500)], + requests, + ), + sleep: async () => undefined, + timeoutMs: 1_000, + }), + /LiteLLM returned HTTP 500/u, + ); + assert.equal(requests.length, 2); +}); + +test('rejects an empty LiteLLM message after the retry budget', async () => { + await assert.rejects( + async () => + requestCleanRoomRewrite({ + ...rewriteInput, + fetchImpl: createFetch([completion(''), completion(null)]), + sleep: async () => undefined, + timeoutMs: 1_000, + }), + /no message content/u, + ); +}); + +test('requests another response when one fails validation', async (context) => { + context.mock.method(console, 'warn', () => undefined); + const requests: RecordedRequest[] = []; + const valid = `## [13.51.0] + +### Added + +- Added a new flow (#101) + +### Fixed + +- Fixed an issue (#102)`; + const glued = valid.replace( + '(#101)\n\n### Fixed\n\n- Fixed', + '(#101)- Fixed', + ); + + const section = await requestValidatedRewrite( + { + ...rewriteInput, + fetchImpl: createFetch([completion(glued), completion(valid)], requests), + }, + '13.51.0', + ['101', '102'], + ); + + assert.equal(section, valid); + assert.equal(requests.length, 2); +}); + +test('gives up after the last response fails validation', async (context) => { + context.mock.method(console, 'warn', () => undefined); + const requests: RecordedRequest[] = []; + const invalid = '## [13.51.0]\n\n### Fixed\n\n- Fixed an issue (#101)'; + + await assert.rejects( + async () => + requestValidatedRewrite( + { + ...rewriteInput, + fetchImpl: createFetch( + [completion(invalid), completion(invalid), completion(invalid)], + requests, + ), + }, + '13.51.0', + ['101', '102'], + ), + /references do not match/u, + ); + assert.equal(requests.length, 3); +}); + +test('does not request again after a rejected key', async () => { + const requests: RecordedRequest[] = []; + + await assert.rejects( + async () => + requestValidatedRewrite( + { + ...rewriteInput, + fetchImpl: createFetch([fakeResponse(401)], requests), + }, + '13.51.0', + ['101', '102'], + ), + LiteLlmAuthenticationError, + ); + assert.equal(requests.length, 1); +}); + +test('aborts a LiteLLM request that exceeds the timeout', async () => { + const signals: (AbortSignal | null | undefined)[] = []; + const hangingFetch = (async (_url: unknown, init?: RequestInit) => { + signals.push(init?.signal); + return new Promise((_resolve, reject) => { + init?.signal?.addEventListener('abort', () => { + reject(new Error('aborted by timeout')); + }); + }); + }) as typeof fetch; + + await assert.rejects( + async () => + requestCleanRoomRewrite({ + ...rewriteInput, + fetchImpl: hangingFetch, + sleep: async () => undefined, + timeoutMs: 5, + }), + /aborted by timeout/u, + ); + assert.equal(signals.length, 2); + assert.equal( + signals.every((signal) => signal?.aborted), + true, + ); +}); + +test('rejects a proofread when any source PR lacks evidence', () => { + assert.throws( + () => + assertPrEvidenceAvailable([ + { prNumber: '101', error: 'HTTP 404: Not Found' }, + { prNumber: '102', error: 'HTTP 500: Internal Server Error' }, + ]), + /PR evidence unavailable: #101 \(HTTP 404: Not Found\), #102 \(HTTP 500: Internal Server Error\)/u, + ); + assert.doesNotThrow(() => assertPrEvidenceAvailable([])); +}); + +const execFileAsync = promisify(execFile); +const scriptPath = fileURLToPath( + new URL('../clean-release-changelog.mts', import.meta.url), +); + +type ProofreadResult = { + exitCode: number; + original: string; + changelog: string; + report: Record; +}; + +/** + * Runs the script as a subprocess against a throwaway repository and a local + * stand-in for LiteLLM. Evidence comes from the repository's mainline commits, + * so `gh` is never called. + * + * @param respond - Writes the stand-in's response to the model request. + * @returns The exit code, the changelog before and after, and the report. + */ +async function runProofreader( + respond: (response: ServerResponse) => void, +): Promise { + const directory = mkdtempSync(join(tmpdir(), 'proofread-')); + const server = createServer((_request, response) => { + respond(response); + }); + const git = (...argumentsList: string[]) => + execFileSync('git', argumentsList, { + cwd: directory, + env: getGitTestEnvironment(), + }); + + try { + git('init', '--initial-branch', 'main'); + git('commit', '--allow-empty', '-m', 'feat: add a flow (#101)\n\nBody one'); + git( + 'commit', + '--allow-empty', + '-m', + 'fix: fix an issue (#102)\n\nBody two', + ); + git('update-ref', 'refs/remotes/origin/main', 'HEAD'); + + const changelogPath = join(directory, 'CHANGELOG.md'); + const original = `# Changelog\n\n${generatedSection}\n\n## [13.50.3]\n\n### Fixed\n\n- An earlier fix (#100)\n`; + writeFileSync(changelogPath, original); + const promptPath = join(directory, 'prompt.md'); + writeFileSync(promptPath, 'Prompt'); + const reportPath = join(directory, 'report.json'); + + await new Promise((resolve) => { + server.listen(0, '127.0.0.1', resolve); + }); + const { port } = server.address() as AddressInfo; + + let exitCode = 0; + try { + await execFileAsync( + process.execPath, + [ + scriptPath, + '--changelog', + changelogPath, + '--version', + '13.51.0', + '--repository', + 'https://github.com/MetaMask/example', + '--prompt', + promptPath, + '--report', + reportPath, + ], + { + cwd: directory, + env: { + ...process.env, + AI_ANALYZER_LITELLM_KEY: 'litellm-key', + LITELLM_BASE_URL: `http://127.0.0.1:${port}`, + }, + }, + ); + } catch (error) { + exitCode = (error as { code?: number }).code ?? 1; + } + + return { + exitCode, + original, + changelog: readFileSync(changelogPath, 'utf8'), + report: JSON.parse(readFileSync(reportPath, 'utf8')), + }; + } finally { + server.closeAllConnections(); + server.close(); + rmSync(directory, { force: true, recursive: true }); + } +} + +/** + * Replies with a chat-completions body. + * + * @param content - The message content. + * @returns A responder for `runProofreader`. + */ +function respondWithContent(content: string) { + return (response: ServerResponse) => { + response.setHeader('Content-Type', 'application/json'); + response.end(JSON.stringify({ choices: [{ message: { content } }] })); + }; +} + +test('writes the validated section and reports success', async () => { + const section = `## [13.51.0] + +### Added + +- Added a new flow (#101) + +### Fixed + +- Fixed an issue (#102)`; + const result = await runProofreader(respondWithContent(section)); + + assert.equal(result.exitCode, 0); + assert.equal(result.report.status, 'succeeded'); + assert.equal( + result.changelog, + result.original.replace(generatedSection, section), + ); +}); + +test('leaves the changelog untouched when the model invents an entry', async () => { + const result = await runProofreader( + respondWithContent(`## [13.51.0] + +### Added + +- Added a new flow (#101) +- Improved performance across the app + +### Fixed + +- Fixed an issue (#102)`), + ); + + assert.equal(result.exitCode, 1); + assert.equal(result.report.status, 'failed'); + assert.equal(result.report.stage, 'cleaning'); + assert.match(String(result.report.error), /must be text followed by its PR/u); + assert.equal(result.changelog, result.original); +}); + +test('leaves the changelog untouched when LiteLLM rejects the key', async () => { + const result = await runProofreader((response) => { + response.statusCode = 401; + response.end('{}'); + }); + + assert.equal(result.exitCode, 1); + assert.equal(result.report.stage, 'authentication'); + assert.equal(result.changelog, result.original); +}); diff --git a/.github/scripts/tests/git-test-environment.mts b/.github/scripts/tests/git-test-environment.mts new file mode 100644 index 00000000..0e478557 --- /dev/null +++ b/.github/scripts/tests/git-test-environment.mts @@ -0,0 +1,41 @@ +import { execFileSync } from 'node:child_process'; + +/** + * Builds the environment for the throwaway repositories the tests create, so + * the developer's global git config and any inherited `GIT_*` variables + * cannot affect them. CI runners have no global git identity. + * + * @returns The environment. + */ +export function getGitTestEnvironment(): NodeJS.ProcessEnv { + const inherited = Object.fromEntries( + Object.entries(process.env).filter(([name]) => !name.startsWith('GIT_')), + ); + return { + ...inherited, + GIT_AUTHOR_NAME: 'Test User', + GIT_AUTHOR_EMAIL: 'test@example.com', + GIT_COMMITTER_NAME: 'Test User', + GIT_COMMITTER_EMAIL: 'test@example.com', + GIT_CONFIG_NOSYSTEM: '1', + GIT_CONFIG_COUNT: '1', + GIT_CONFIG_KEY_0: 'commit.gpgsign', + GIT_CONFIG_VALUE_0: 'false', + }; +} + +/** + * Runs Git in a directory with the isolated test environment. + * + * @param cwd - The directory to run in. + * @param args - The Git arguments. + * @returns The trimmed standard output. + */ +export function git(cwd: string, ...args: string[]): string { + return execFileSync('git', args, { + cwd, + encoding: 'utf8', + env: getGitTestEnvironment(), + stdio: ['ignore', 'pipe', 'pipe'], + }).trim(); +} diff --git a/.github/scripts/tests/merge-previous-releases.test.mts b/.github/scripts/tests/merge-previous-releases.test.mts index 7e37d4dc..75a81525 100644 --- a/.github/scripts/tests/merge-previous-releases.test.mts +++ b/.github/scripts/tests/merge-previous-releases.test.mts @@ -12,6 +12,8 @@ import { tmpdir } from 'node:os'; import { delimiter, join } from 'node:path'; import test, { after } from 'node:test'; +import { getGitTestEnvironment, git } from './git-test-environment.mts'; + const SCRIPT_PATH = join( process.cwd(), '.github', @@ -24,28 +26,6 @@ const TEMPORARY_DIRECTORY = mkdtempSync( ); after(() => rmSync(TEMPORARY_DIRECTORY, { force: true, recursive: true })); -// CI runners have no global git identity, and the script commits. -const GIT_IDENTITY = { - GIT_AUTHOR_NAME: 'Test User', - GIT_AUTHOR_EMAIL: 'test@example.com', - GIT_COMMITTER_NAME: 'Test User', - GIT_COMMITTER_EMAIL: 'test@example.com', -}; - -/** - * Runs a git command and returns its trimmed output. - * @param directory - The working directory. - * @param argumentsList - The git arguments. - * @returns The trimmed stdout. - */ -function git(directory: string, ...argumentsList: string[]): string { - return execFileSync('git', argumentsList, { - cwd: directory, - encoding: 'utf8', - env: { ...process.env, ...GIT_IDENTITY }, - }).trim(); -} - /** * Creates an origin repository with a `main`, an older active release, and a * new release branch, plus a clone and a fake `gh` for running the script. @@ -112,8 +92,7 @@ function createReleaseMergeFixture({ execFileSync('bash', [SCRIPT_PATH], { cwd: workDirectory, env: { - ...process.env, - ...GIT_IDENTITY, + ...getGitTestEnvironment(), NEW_RELEASE_BRANCH: 'release/13.55.0', GITHUB_TOKEN: 'test-token', PATH: `${binDirectory}${delimiter}${process.env.PATH ?? ''}`, diff --git a/.github/scripts/tests/release-section.test.mts b/.github/scripts/tests/release-section.test.mts new file mode 100644 index 00000000..35b245c3 --- /dev/null +++ b/.github/scripts/tests/release-section.test.mts @@ -0,0 +1,59 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; + +import { + extractReleaseSection, + replaceReleaseSection, +} from '../release-section.mts'; + +test('finds the heading line even when its text appears earlier', () => { + const changelog = + '\n\n## [1.0.0]\n\n- Real entry (#1)\n\n## [0.9.0]\n'; + + assert.equal( + extractReleaseSection(changelog, '1.0.0').section, + '## [1.0.0]\n\n- Real entry (#1)', + ); +}); + +test('writes a replacement in the changelog line endings', () => { + const changelog = '## [1.0.0]\r\n\r\n- Old (#1)\r\n\r\n## [0.9.0]\r\n'; + + assert.equal( + replaceReleaseSection(changelog, '1.0.0', '## [1.0.0]\n\n- New (#1)'), + '## [1.0.0]\r\n\r\n- New (#1)\r\n\r\n## [0.9.0]\r\n', + ); +}); + +test('ends the oldest release section before the link references', () => { + const changelog = [ + '# Changelog', + '', + '## [Unreleased]', + '', + '## [13.51.0]', + '', + '### Fixed', + '', + '- Fixed an issue (#101)', + '', + '[Unreleased]: https://example.com/HEAD', + '[13.51.0]: https://example.com/13.51.0', + '', + ].join('\n'); + + assert.equal( + extractReleaseSection(changelog, '13.51.0').section, + '## [13.51.0]\n\n### Fixed\n\n- Fixed an issue (#101)', + ); + assert.equal( + replaceReleaseSection( + changelog, + '13.51.0', + '## [13.51.0]\n\n### Added\n\n- Added a feature (#102)', + ), + changelog + .replace('### Fixed', '### Added') + .replace('Fixed an issue (#101)', 'Added a feature (#102)'), + ); +}); diff --git a/.github/scripts/tests/retry.test.mts b/.github/scripts/tests/retry.test.mts new file mode 100644 index 00000000..75077519 --- /dev/null +++ b/.github/scripts/tests/retry.test.mts @@ -0,0 +1,67 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; + +import { retry } from '../shared/retry.mts'; + +test('retries an operation with increasing delays', async () => { + let attempts = 0; + const delays: number[] = []; + const result = await retry( + async () => { + attempts += 1; + if (attempts < 3) { + throw new Error('Transient failure'); + } + return 'recovered'; + }, + { + delayMilliseconds: 100, + sleep: async (milliseconds) => { + delays.push(milliseconds); + }, + }, + ); + + assert.equal(result, 'recovered'); + assert.equal(attempts, 3); + assert.deepEqual(delays, [100, 200]); +}); + +test('does not retry a rejected error category', async () => { + let attempts = 0; + await assert.rejects( + async () => + retry( + async () => { + attempts += 1; + throw new Error('Permanent failure'); + }, + { shouldRetry: () => false }, + ), + /Permanent failure/u, + ); + + assert.equal(attempts, 1); +}); + +test('reports retry details before delaying', async () => { + const retries: { attempt: number; delayMilliseconds: number }[] = []; + await assert.rejects( + async () => + retry( + async () => { + throw new Error('Transient failure'); + }, + { + maxAttempts: 2, + onRetry: ({ attempt, delayMilliseconds }) => { + retries.push({ attempt, delayMilliseconds }); + }, + sleep: async () => undefined, + }, + ), + /Transient failure/u, + ); + + assert.deepEqual(retries, [{ attempt: 1, delayMilliseconds: 1_000 }]); +}); diff --git a/src/segment-schema-draft-pr/apply-changes.ts b/src/segment-schema-draft-pr/apply-changes.ts index 7abd35ec..dca253c2 100644 --- a/src/segment-schema-draft-pr/apply-changes.ts +++ b/src/segment-schema-draft-pr/apply-changes.ts @@ -1,7 +1,7 @@ import fs from 'fs/promises'; import path from 'path'; import { parseDocument, YAMLMap, Scalar } from 'yaml'; -import type { Document } from 'yaml'; +import type { Document as YamlDocument } from 'yaml'; import { TODO_DESCRIPTION } from './constants'; import { uniqueKebabSlug } from './names'; @@ -226,7 +226,7 @@ function buildNewEventDocument( eventName: string, config: PlatformConfig, properties: PropertyChange[], -): Document { +): YamlDocument { const doc = parseDocument( [ 'name: ""', @@ -251,7 +251,10 @@ function buildNewEventDocument( * @param doc - Event YAML document. * @param properties - Properties to add. */ -function appendProperties(doc: Document, properties: PropertyChange[]): void { +function appendProperties( + doc: YamlDocument, + properties: PropertyChange[], +): void { const map = ensurePropertiesMap(doc); for (const property of properties) { if (map.has(property.key)) { @@ -283,7 +286,7 @@ function quotedScalar(value: string): Scalar { * @param doc - Event YAML document. * @returns Properties map. */ -function ensurePropertiesMap(doc: Document): YAMLMap { +function ensurePropertiesMap(doc: YamlDocument): YAMLMap { const current = doc.get('properties'); if (current instanceof YAMLMap) { current.flow = false; @@ -317,7 +320,7 @@ async function preserveAndWrite( absolute: string, previousDir: string | undefined, relative: string, - doc: Document, + doc: YamlDocument, ): Promise { if (previousDir) { const previousPath = path.join(previousDir, relative); @@ -342,7 +345,10 @@ async function preserveAndWrite( * @param next - Newly generated document. * @param previous - Document from the last bot branch. */ -function overlayPreservedFields(next: Document, previous: Document): void { +function overlayPreservedFields( + next: YamlDocument, + previous: YamlDocument, +): void { if (yamlKey(previous.get('name')) !== yamlKey(next.get('name'))) { return; } diff --git a/tsconfig.json b/tsconfig.json index 032d9536..8b9ea8f2 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -1,9 +1,10 @@ { "compilerOptions": { + "allowImportingTsExtensions": true, "esModuleInterop": true, "exactOptionalPropertyTypes": true, "forceConsistentCasingInFileNames": true, - "lib": ["ES2022"], + "lib": ["ES2022", "DOM"], "module": "Node16", "moduleResolution": "Node16", "noEmit": true,