From d8805b132443a211d755fa52ef08dddec6153d43 Mon Sep 17 00:00:00 2001 From: Ryuya Date: Fri, 2 Oct 2026 03:20:08 -0700 Subject: [PATCH 1/2] fix: retain diagnostics for failed updates --- .github/workflows/typescript-update.yml | 57 +++++-- .github/workflows/weekly-update.yml | 49 ++++-- docs/OPERATIONS.md | 10 ++ lib/update-diagnostics.mjs | 192 ++++++++++++++++++++++++ scripts/update-diagnostics.mjs | 27 ++++ test/update-diagnostics.test.mjs | 162 ++++++++++++++++++++ 6 files changed, 472 insertions(+), 25 deletions(-) create mode 100644 lib/update-diagnostics.mjs create mode 100644 scripts/update-diagnostics.mjs create mode 100644 test/update-diagnostics.test.mjs diff --git a/.github/workflows/typescript-update.yml b/.github/workflows/typescript-update.yml index d410e6f..b674883 100644 --- a/.github/workflows/typescript-update.yml +++ b/.github/workflows/typescript-update.yml @@ -24,45 +24,67 @@ jobs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - - uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 + - id: checkout + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + - id: setup-node + uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 with: node-version: "24" cache: "npm" - - run: npm ci + - id: install + run: node scripts/update-diagnostics.mjs run install -- npm ci - name: Run static JS checks - run: npm run lint + id: lint + run: node scripts/update-diagnostics.mjs run lint -- npm run lint - name: Pin latest TypeScript toolchain - run: npm run update:typescript-toolchain + id: update-toolchain + run: node scripts/update-diagnostics.mjs run update-toolchain -- npm run update:typescript-toolchain - name: Verify pinned web-features dataset - run: npm run verify:dataset + id: verify-dataset + run: node scripts/update-diagnostics.mjs run verify-dataset -- npm run verify:dataset - name: Regenerate baseline - run: npm run generate + id: generate + run: node scripts/update-diagnostics.mjs run generate -- npm run generate - name: Run standalone validation - run: npm test + id: test + run: node scripts/update-diagnostics.mjs run test -- npm test - name: Checkout updated TypeScript source - run: npm run checkout:typescript-source -- --out .tmp/TypeScript --force + id: checkout-typescript + run: node scripts/update-diagnostics.mjs run checkout-typescript -- npm run checkout:typescript-source -- --out .tmp/TypeScript --force - name: Read required Go version id: go-version run: echo "version=$(awk '/^go / { print $2; exit }' .tmp/TypeScript/tsc/go.mod)" >> "$GITHUB_OUTPUT" - - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6.5.0 + - id: setup-go + uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6.5.0 with: go-version: "${{ steps.go-version.outputs.version }}" cache-dependency-path: .tmp/TypeScript/tsc/go.sum - name: Test updated TypeScript source - run: npm run test:typescript:gate -- --typescript-dir ./.tmp/TypeScript + id: integration + run: node scripts/update-diagnostics.mjs run integration -- npm run test:typescript:gate -- --typescript-dir ./.tmp/TypeScript + + - name: Check latest TypeScript major against the peer range + id: peer-range + run: node scripts/update-diagnostics.mjs run peer-range -- npm run check:typescript-peer-latest - name: Prepare pull request body - run: npm run write:update-pr-body -- --out .tmp/typescript-toolchain-update-pr.md --summary-out .tmp/typescript-toolchain-update-summary.json + id: pr-body + run: node scripts/update-diagnostics.mjs run pr-body -- npm run write:update-pr-body -- --out .tmp/typescript-toolchain-update-pr.md --summary-out .tmp/typescript-toolchain-update-summary.json + + - name: Write update diagnostics + if: always() + env: + UPDATE_WORKFLOW_STEPS: ${{ toJSON(steps) }} + run: node scripts/update-diagnostics.mjs report # By GitHub design, PRs opened with the default GITHUB_TOKEN don't trigger # any pull_request workflow (validate / test-typescript / package-dry-run). @@ -80,6 +102,7 @@ jobs: fi - name: Create pull request + id: pull-request uses: peter-evans/create-pull-request@5f6978faf089d4d20b00c7766989d076bb2fc7f1 # v8.1.1 with: token: ${{ secrets.AUTOMATION_TOKEN || github.token }} @@ -89,5 +112,11 @@ jobs: title: "chore: update TypeScript toolchain snapshot" body-path: .tmp/typescript-toolchain-update-pr.md - - name: Check latest TypeScript major against the peer range - run: npm run check:typescript-peer-latest + - name: Upload update diagnostics + if: always() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: update-diagnostics + path: .tmp/update-diagnostics + include-hidden-files: true + if-no-files-found: error diff --git a/.github/workflows/weekly-update.yml b/.github/workflows/weekly-update.yml index c7feb5b..e4c0b83 100644 --- a/.github/workflows/weekly-update.yml +++ b/.github/workflows/weekly-update.yml @@ -24,36 +24,53 @@ jobs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - - uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 + - id: checkout + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + - id: setup-node + uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 with: node-version: "24" cache: "npm" - - run: npm ci + - id: install + run: node scripts/update-diagnostics.mjs run install -- npm ci - name: Run static JS checks - run: npm run lint + id: lint + run: node scripts/update-diagnostics.mjs run lint -- npm run lint - name: Pin latest web-features release - run: npm install --save-exact "web-features@$(npm view web-features dist-tags.latest)" + id: pin-web-features + run: node scripts/update-diagnostics.mjs run pin-web-features -- bash -e -o pipefail -c 'npm install --save-exact "web-features@$(npm view web-features dist-tags.latest)"' - name: Update manifest and dataset - run: npm run update:web-features -- --snapshot-date "$(date -u +%F)" + id: update-dataset + run: node scripts/update-diagnostics.mjs run update-dataset -- npm run update:web-features -- --snapshot-date "$(date -u +%F)" - name: Validate compat-management schema - run: npm run validate:registry + id: validate-registry + run: node scripts/update-diagnostics.mjs run validate-registry -- npm run validate:registry - name: Verify pinned web-features dataset - run: npm run verify:dataset + id: verify-dataset + run: node scripts/update-diagnostics.mjs run verify-dataset -- npm run verify:dataset - name: Regenerate baseline - run: npm run generate + id: generate + run: node scripts/update-diagnostics.mjs run generate -- npm run generate - name: Run standalone validation - run: npm test + id: test + run: node scripts/update-diagnostics.mjs run test -- npm test - name: Prepare pull request body - run: npm run write:update-pr-body -- --out .tmp/baseline-lib-update-pr.md --summary-out .tmp/baseline-lib-update-summary.json + id: pr-body + run: node scripts/update-diagnostics.mjs run pr-body -- npm run write:update-pr-body -- --out .tmp/baseline-lib-update-pr.md --summary-out .tmp/baseline-lib-update-summary.json + + - name: Write update diagnostics + if: always() + env: + UPDATE_WORKFLOW_STEPS: ${{ toJSON(steps) }} + run: node scripts/update-diagnostics.mjs report # By GitHub design, PRs opened with the default GITHUB_TOKEN don't trigger # any pull_request workflow (validate / test-typescript / package-dry-run). @@ -71,6 +88,7 @@ jobs: fi - name: Create pull request + id: pull-request uses: peter-evans/create-pull-request@5f6978faf089d4d20b00c7766989d076bb2fc7f1 # v8.1.1 with: token: ${{ secrets.AUTOMATION_TOKEN || github.token }} @@ -79,3 +97,12 @@ jobs: commit-message: "chore: update web-features baseline snapshot" title: "chore: update web-features baseline snapshot" body-path: .tmp/baseline-lib-update-pr.md + + - name: Upload update diagnostics + if: always() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: update-diagnostics + path: .tmp/update-diagnostics + include-hidden-files: true + if-no-files-found: error diff --git a/docs/OPERATIONS.md b/docs/OPERATIONS.md index 7afe6ed..eb2668b 100644 --- a/docs/OPERATIONS.md +++ b/docs/OPERATIONS.md @@ -35,3 +35,13 @@ Identical inputs produce byte-identical artifacts on each supported operating sy - `libSource` records the platform package and one content hash. The update requires identical files on Linux, macOS, and Windows. - `typescriptSource` records the exact `microsoft/TypeScript` `main` commit used by proposal preparation and integration tests. Proposal artifacts are generated from that checkout's declaration corpus. + +## Failed updates + +Weekly data and TypeScript toolchain updates save an `update-diagnostics` workflow artifact, including command logs, `summary.md`, and `summary.json`. The job summary shows the failed step, candidate input pins, and added, removed, or changed compat keys. Reports use the input dataset even if generation fails; existing derived files are not evidence that the candidate passed. + +For registry drift, inspect each named compat key and its upstream declaration before changing `registry/compat-management.json`. Keep missing declarations separate from newly implemented declarations. Do not relax the checks just to make an update pass. + +For a TypeScript layout change, update the integration adapter and its fixtures. Keep proposal generation pinned to the recorded commit. Run `npm run validate`, then run `npm run test:typescript:gate` against that commit before retrying the toolchain workflow. + +Failed update jobs do not create pull requests. Repair the cause and rerun the workflow. Diagnostic collection does not change the release gates or publish a package. diff --git a/lib/update-diagnostics.mjs b/lib/update-diagnostics.mjs new file mode 100644 index 0000000..c5cc89c --- /dev/null +++ b/lib/update-diagnostics.mjs @@ -0,0 +1,192 @@ +// @ts-check + +import { execFileSync, spawn } from "node:child_process"; +import fs from "node:fs"; +import path from "node:path"; +import { getCompatFeatureMemberships } from "./compat-rows.mjs"; +import { compareStringsCaseSensitive } from "./shared.mjs"; + +const manifestPath = "manifests/baseline-js.json"; +const datasetPath = "datasets/web-features-js-compat.json"; + +/** + * Keep the command's failure blocking while saving its output for the update report. + * @param {{ cwd: string; outputDirectory: string; step: string; command: string; args: string[]; }} options + */ +export async function runRecordedUpdateStep(options) { + if (!/^[a-z][a-z0-9-]*$/u.test(options.step)) { + throw new Error(`Invalid update step name: ${options.step}`); + } + fs.mkdirSync(options.outputDirectory, { recursive: true }); + const logPath = path.join(options.outputDirectory, `${options.step}.log`); + const resultPath = path.join(options.outputDirectory, `${options.step}.json`); + fs.writeFileSync(logPath, ""); + const startedAt = new Date().toISOString(); + /** @type {{ code: number | null; signal: NodeJS.Signals | null; error?: string; }} */ + const result = await new Promise(resolve => { + const child = spawn(options.command, options.args, { cwd: options.cwd, stdio: ["ignore", "pipe", "pipe"] }); + /** @param {Buffer} chunk */ + const record = chunk => { + fs.appendFileSync(logPath, chunk); + process.stdout.write(chunk); + }; + child.stdout.on("data", record); + child.stderr.on("data", record); + child.on("error", error => { + record(Buffer.from(`${error.message}\n`)); + resolve({ code: null, signal: null, error: error.message }); + }); + child.on("close", (code, signal) => resolve({ code, signal })); + }); + const exitCode = result.code ?? 1; + fs.writeFileSync(resultPath, `${JSON.stringify({ + step: options.step, + command: [options.command, ...options.args], + startedAt, + finishedAt: new Date().toISOString(), + exitCode, + signal: result.signal, + error: result.error, + logFile: path.basename(logPath), + }, undefined, 2)}\n`); + return exitCode; +} + +/** + * Read inputs independently of generation: derived files may still describe HEAD + * after a failed update, so they are deliberately absent from this report. + * @param {{ cwd: string; outputDirectory: string; steps?: Record; }} options + */ +export function writeUpdateDiagnostics(options) { + fs.mkdirSync(options.outputDirectory, { recursive: true }); + /** @type {string[]} */ + const readErrors = []; + /** @param {string} relativePath @param {boolean} previous */ + const readInput = (relativePath, previous) => { + try { + const text = previous + ? execFileSync("git", ["show", `HEAD:${relativePath}`], { cwd: options.cwd, encoding: "utf8", stdio: ["ignore", "pipe", "pipe"], maxBuffer: 16 * 1024 * 1024 }) + : fs.readFileSync(path.join(options.cwd, relativePath), "utf8"); + return JSON.parse(text); + } + catch (error) { + readErrors.push(`${previous ? "HEAD" : "Working tree"} ${relativePath}: ${error instanceof Error ? error.message : String(error)}`); + return undefined; + } + }; + const previousManifest = readInput(manifestPath, true); + const currentManifest = readInput(manifestPath, false); + const previousPackage = readInput("package.json", true); + const currentPackage = readInput("package.json", false); + const previousDataset = readInput(datasetPath, true); + const currentDataset = readInput(datasetPath, false); + let dataset; + try { + dataset = compareDatasetInputs(previousDataset, currentDataset); + } + catch (error) { + readErrors.push(`Dataset comparison: ${error instanceof Error ? error.message : String(error)}`); + } + const commands = fs.readdirSync(options.outputDirectory) + .filter(name => name.endsWith(".json") && name !== "summary.json") + .map(name => JSON.parse(fs.readFileSync(path.join(options.outputDirectory, name), "utf8"))) + .sort((left, right) => left.startedAt.localeCompare(right.startedAt)); + const failedSteps = Object.entries(options.steps ?? {}) + .filter(([, step]) => step.outcome === "failure" || step.outcome === "cancelled") + .map(([step, value]) => ({ step, outcome: value.outcome })); + const summary = { + status: failedSteps.length || commands.some(command => command.exitCode !== 0) + ? "failed" + : readErrors.length || !commands.length ? "incomplete" : "passed", + failedSteps, + commands, + inputs: { + previous: { dependencies: previousPackage?.dependencies, snapshot: previousManifest?.snapshot, typescriptSource: previousManifest?.typescriptSource }, + candidate: { dependencies: currentPackage?.dependencies, snapshot: currentManifest?.snapshot, typescriptSource: currentManifest?.typescriptSource }, + }, + dataset, + readErrors, + }; + const lines = [ + "# Update diagnostics", + "", + `Update checks: ${summary.status}.`, + "", + "These are candidate inputs. A failed update does not validate or publish generated declarations.", + "", + ...failedSteps.map(step => `- Workflow step \`${step.step}\`: ${step.outcome}.`), + ...commands.map(command => `- \`${command.step}\`: exit ${command.exitCode}; log: \`${command.logFile}\`.`), + "", + "## Input pins", + "", + "```json", + JSON.stringify(summary.inputs, undefined, 2), + "```", + "", + "## Dataset changes", + "", + summary.dataset + ? `${summary.dataset.added.length} added, ${summary.dataset.removed.length} removed, ${summary.dataset.changed.length} changed compat keys.` + : "Dataset comparison unavailable. See the input read errors below.", + "", + ...(summary.dataset ? [ + ...summary.dataset.added.map(key => `- Added: \`${key}\`.`), + ...summary.dataset.removed.map(key => `- Removed: \`${key}\`.`), + ...summary.dataset.changed.map(key => `- Changed: \`${key}\`.`), + ] : []), + ]; + for (const command of commands.filter(command => command.exitCode !== 0)) { + const log = fs.readFileSync(path.join(options.outputDirectory, command.logFile), "utf8"); + lines.push("", `## ${command.step} failure`, "", "```text", log.trimEnd().split(/\r?\n/u).slice(-120).join("\n").replaceAll("```", "'''"), "```"); + } + if (readErrors.length) { + lines.push("", "## Input read errors", "", ...readErrors.map(error => `- ${error}`)); + } + if (summary.status === "failed") { + lines.push("", "## Recovery", "", + "Inspect the failed step's log in this artifact. For registry drift, review the named compat keys and their upstream declarations before editing registry/compat-management.json. For an upstream layout error, update the TypeScript adapter and its fixtures.", + "", + "Run npm run validate after the repair. TypeScript source updates also require npm run test:typescript:gate against the candidate commit. Rerun the update workflow before merging its pull request."); + } + const markdown = `${lines.join("\n")}\n`; + fs.writeFileSync(path.join(options.outputDirectory, "summary.json"), `${JSON.stringify(summary, undefined, 2)}\n`); + fs.writeFileSync(path.join(options.outputDirectory, "summary.md"), markdown); + return { summary, markdown }; +} + +/** @param {any} previous @param {any} current */ +function compareDatasetInputs(previous, current) { + if (!Array.isArray(previous?.compatRows) || !Array.isArray(current?.compatRows)) { + return undefined; + } + /** @param {any[]} rows */ + const index = rows => { + /** @type {Map} */ + const grouped = new Map(); + for (const row of rows) { + const values = grouped.get(row.compatKey) ?? []; + values.push(JSON.stringify({ + baselineStatus: row.baselineStatus, + baselineLowDate: row.baselineLowDate, + baselineHighDate: row.baselineHighDate, + featureMemberships: getCompatFeatureMemberships(row) + .map(membership => ({ + featureId: membership.featureId, + featureName: membership.featureName, + snapshot: [...membership.snapshot].sort(compareStringsCaseSensitive), + group: [...membership.group].sort(compareStringsCaseSensitive), + })) + .sort((left, right) => compareStringsCaseSensitive(left.featureId, right.featureId)), + })); + grouped.set(row.compatKey, values); + } + return new Map([...grouped].map(([key, values]) => [key, JSON.stringify(values.sort())])); + }; + const before = index(previous.compatRows); + const after = index(current.compatRows); + return { + added: [...after.keys()].filter(key => !before.has(key)).sort(), + removed: [...before.keys()].filter(key => !after.has(key)).sort(), + changed: [...after.keys()].filter(key => before.has(key) && before.get(key) !== after.get(key)).sort(), + }; +} diff --git a/scripts/update-diagnostics.mjs b/scripts/update-diagnostics.mjs new file mode 100644 index 0000000..6ec9091 --- /dev/null +++ b/scripts/update-diagnostics.mjs @@ -0,0 +1,27 @@ +// @ts-check + +import fs from "node:fs"; +import path from "node:path"; +import { runRecordedUpdateStep, writeUpdateDiagnostics } from "../lib/update-diagnostics.mjs"; + +const cwd = process.cwd(); +const outputDirectory = path.join(cwd, ".tmp", "update-diagnostics"); +const [mode, step, separator, command, ...args] = process.argv.slice(2); + +if (mode === "run" && step && separator === "--" && command) { + process.exitCode = await runRecordedUpdateStep({ cwd, outputDirectory, step, command, args }); +} +else if (mode === "report" && !step) { + const { markdown } = writeUpdateDiagnostics({ + cwd, + outputDirectory, + steps: JSON.parse(process.env.UPDATE_WORKFLOW_STEPS ?? "{}"), + }); + if (process.env.GITHUB_STEP_SUMMARY) { + fs.appendFileSync(process.env.GITHUB_STEP_SUMMARY, markdown); + } + console.log(`Wrote ${path.relative(cwd, outputDirectory)}/summary.md`); +} +else { + throw new Error("Usage: node scripts/update-diagnostics.mjs run -- [args...] | report"); +} diff --git a/test/update-diagnostics.test.mjs b/test/update-diagnostics.test.mjs new file mode 100644 index 0000000..5ad6529 --- /dev/null +++ b/test/update-diagnostics.test.mjs @@ -0,0 +1,162 @@ +// @ts-check + +import assert from "node:assert/strict"; +import { execFileSync, spawnSync } from "node:child_process"; +import fs from "node:fs"; +import path from "node:path"; +import test from "node:test"; +import { parse } from "yaml"; +import { writeUpdateDiagnostics } from "../lib/update-diagnostics.mjs"; +import { cleanupTempDirectories, createTempDirectory, repoRoot, writeJsonFile } from "./helpers.mjs"; + +/** @type {string[]} */ +const tempDirectories = []; +test.afterEach(() => cleanupTempDirectories(tempDirectories)); + +test("a failed update keeps its exit code and reports candidate inputs without reading stale generated reports", () => { + const cwd = createTempDirectory(tempDirectories); + const outputDirectory = path.join(cwd, ".tmp", "update-diagnostics"); + const manifestPath = path.join(cwd, "manifests", "baseline-js.json"); + const datasetPath = path.join(cwd, "datasets", "web-features-js-compat.json"); + writeJsonFile(path.join(cwd, "package.json"), { dependencies: { "web-features": "1.0.0" } }); + writeJsonFile(manifestPath, { snapshot: { webFeaturesPackageVersion: "1.0.0" } }); + writeJsonFile(datasetPath, { compatRows: [ + { compatKey: "javascript.builtins.Known", featureId: "known", baselineStatus: "low" }, + { compatKey: "javascript.builtins.Removed", featureId: "removed", baselineStatus: false }, + ] }); + commitInputs(cwd); + writeJsonFile(manifestPath, { snapshot: { webFeaturesPackageVersion: "2.0.0" } }); + writeJsonFile(path.join(cwd, "package.json"), { dependencies: { "web-features": "2.0.0" } }); + writeJsonFile(datasetPath, { compatRows: [ + { compatKey: "javascript.builtins.Known", featureId: "known", baselineStatus: "high" }, + { compatKey: "javascript.builtins.Known", featureId: "other-feature", baselineStatus: "high" }, + { compatKey: "javascript.builtins.New", featureId: "new", baselineStatus: false }, + ] }); + const derivedDirectory = path.join(cwd, "derived", "current"); + fs.mkdirSync(derivedDirectory, { recursive: true }); + fs.writeFileSync(path.join(derivedDirectory, "generation.json"), "not a current generation report"); + + const failure = runDiagnostics(cwd, ["run", "generate", "--", process.execPath, "-e", "console.error('Special compat keys missing registry metadata: javascript.builtins.New'); process.exit(17)"]); + assert.equal(failure.status, 17); + const { summary, markdown } = writeUpdateDiagnostics({ cwd, outputDirectory, steps: { generate: { outcome: "failure" }, test: { outcome: "skipped" } } }); + assert.equal(summary.status, "failed"); + assert.equal(summary.commands[0].exitCode, 17); + assert.equal(summary.inputs.previous.snapshot.webFeaturesPackageVersion, "1.0.0"); + assert.equal(summary.inputs.candidate.snapshot.webFeaturesPackageVersion, "2.0.0"); + assert.equal(summary.inputs.candidate.dependencies["web-features"], "2.0.0"); + assert.deepEqual(summary.dataset, { + added: ["javascript.builtins.New"], + removed: ["javascript.builtins.Removed"], + changed: ["javascript.builtins.Known"], + }); + assert.deepEqual(summary.readErrors, []); + assert.match(markdown, /Special compat keys missing registry metadata/u); + assert.match(markdown, /registry\/compat-management\.json/u); + assert.match(markdown, /does not validate or publish/u); + assert.ok(fs.existsSync(path.join(outputDirectory, "summary.json"))); + assert.ok(fs.existsSync(path.join(outputDirectory, "generate.log"))); +}); + +test("an unavailable executable still leaves a failed command record", () => { + const cwd = createTempDirectory(tempDirectories); + const failure = runDiagnostics(cwd, ["run", "install", "--", path.join(cwd, "missing-command")]); + assert.equal(failure.status, 1); + const result = JSON.parse(fs.readFileSync(path.join(cwd, ".tmp", "update-diagnostics", "install.json"), "utf8")); + assert.equal(result.exitCode, 1); + assert.match(result.error, /ENOENT/u); +}); + +test("setup failures produce a report even when no command ran and candidate inputs are missing", () => { + const cwd = createTempDirectory(tempDirectories); + execFileSync("git", ["init", "--quiet"], { cwd }); + const summaryPath = path.join(cwd, "github-summary.md"); + const result = runDiagnostics(cwd, ["report"], { + UPDATE_WORKFLOW_STEPS: JSON.stringify({ "setup-node": { outcome: "failure" } }), + GITHUB_STEP_SUMMARY: summaryPath, + }); + assert.equal(result.status, 0, result.stderr); + const summary = JSON.parse(fs.readFileSync(path.join(cwd, ".tmp", "update-diagnostics", "summary.json"), "utf8")); + assert.equal(summary.status, "failed"); + assert.deepEqual(summary.failedSteps, [{ step: "setup-node", outcome: "failure" }]); + assert.equal(summary.readErrors.length, 6); + assert.match(fs.readFileSync(summaryPath, "utf8"), /Dataset comparison unavailable/u); +}); + +test("diagnostics track secondary feature memberships and retain failure logs for malformed candidates", () => { + const cwd = createTempDirectory(tempDirectories); + const outputDirectory = path.join(cwd, ".tmp", "update-diagnostics"); + const datasetPath = path.join(cwd, "datasets", "web-features-js-compat.json"); + const first = { featureId: "first", featureName: "First", snapshot: ["es2020"], group: ["group-a", "group-b"] }; + const second = { featureId: "second", featureName: "Second", snapshot: ["es2021"], group: ["group-c"] }; + const third = { ...second, featureId: "third", featureName: "Third" }; + const row = { + compatKey: "javascript.builtins.Known", + ...first, + baselineStatus: "high", + featureMemberships: [first, second], + }; + writeJsonFile(path.join(cwd, "package.json"), { dependencies: {} }); + writeJsonFile(path.join(cwd, "manifests", "baseline-js.json"), { snapshot: {} }); + writeJsonFile(datasetPath, { compatRows: [row] }); + commitInputs(cwd); + + for (const featureMemberships of [ + [first, second, third], + [first, third], + [first, { ...second, featureName: "Renamed" }], + [first, { ...second, snapshot: ["es2022"] }], + [first, { ...second, group: ["other-group"] }], + undefined, + ]) { + writeJsonFile(datasetPath, { compatRows: [{ ...row, featureMemberships }] }); + const { summary } = writeUpdateDiagnostics({ cwd, outputDirectory }); + assert.deepEqual(summary.dataset?.changed, [row.compatKey]); + } + writeJsonFile(datasetPath, { compatRows: [{ ...row, featureMemberships: [second, { ...first, group: [...first.group].reverse() }] }] }); + assert.deepEqual(writeUpdateDiagnostics({ cwd, outputDirectory }).summary.dataset?.changed, []); + + writeJsonFile(datasetPath, { compatRows: [{ ...row, featureMemberships: [first, { ...second, snapshot: null }] }] }); + assert.equal(runDiagnostics(cwd, ["run", "generate", "--", process.execPath, "-e", "console.error('candidate rejected'); process.exit(2)"]).status, 2); + const { summary, markdown } = writeUpdateDiagnostics({ cwd, outputDirectory }); + assert.equal(summary.status, "failed"); + assert.equal(summary.dataset, undefined); + assert.match(summary.readErrors.join("\n"), /feature membership with invalid snapshot/u); + assert.match(markdown, /candidate rejected/u); +}); + +test("update workflows keep checks blocking and collect candidate diagnostics before creating a pull request", () => { + for (const name of ["weekly-update.yml", "typescript-update.yml"]) { + const workflow = parse(fs.readFileSync(path.join(repoRoot, ".github", "workflows", name), "utf8")); + /** @type {Array>} */ + const steps = workflow.jobs.update.steps; + const reportIndex = steps.findIndex(step => step.run === "node scripts/update-diagnostics.mjs report"); + const pullRequestIndex = steps.findIndex(step => step.id === "pull-request"); + const upload = steps.find(step => step.uses?.startsWith("actions/upload-artifact@")); + assert.ok(reportIndex >= 0 && pullRequestIndex > reportIndex); + assert.equal(steps[reportIndex].if, "always()"); + assert.equal(upload?.if, "always()"); + assert.equal(upload?.with["path"], ".tmp/update-diagnostics"); + assert.equal(upload?.with["include-hidden-files"], true); + assert.equal(steps[pullRequestIndex].if, undefined); + for (const step of steps.filter(step => step.run?.includes("update-diagnostics.mjs run"))) { + assert.ok(steps.indexOf(step) < reportIndex, `${name}: ${step.id} runs after diagnostics`); + assert.equal(step["continue-on-error"], undefined); + } + } +}); + +/** @param {string} cwd @param {string[]} args @param {Record} [environment] */ +function runDiagnostics(cwd, args, environment = {}) { + return spawnSync(process.execPath, [path.join(repoRoot, "scripts", "update-diagnostics.mjs"), ...args], { + cwd, + encoding: "utf8", + env: { ...process.env, ...environment }, + }); +} + +/** @param {string} cwd */ +function commitInputs(cwd) { + execFileSync("git", ["init", "--quiet"], { cwd }); + execFileSync("git", ["add", "manifests", "datasets", "package.json"], { cwd }); + execFileSync("git", ["-c", "user.name=Test", "-c", "user.email=test@example.com", "commit", "--quiet", "-m", "fixture"], { cwd }); +} From ad95db6cd9e65dcd86351fa1cc842e1a54498868 Mon Sep 17 00:00:00 2001 From: Ryuya Date: Sun, 4 Oct 2026 01:31:19 -0700 Subject: [PATCH 2/2] fix: repair update reports and command capture --- lib/update-diagnostics.mjs | 173 +++++++++++++++++++++++++------ scripts/update-diagnostics.mjs | 4 + scripts/write-update-pr-body.mjs | 83 ++++++++++----- test/update-diagnostics.test.mjs | 134 ++++++++++++++++++++++-- test/update-pr-body.test.mjs | 95 +++++++++++++++++ 5 files changed, 422 insertions(+), 67 deletions(-) create mode 100644 test/update-pr-body.test.mjs diff --git a/lib/update-diagnostics.mjs b/lib/update-diagnostics.mjs index c5cc89c..3a89edd 100644 --- a/lib/update-diagnostics.mjs +++ b/lib/update-diagnostics.mjs @@ -3,10 +3,12 @@ import { execFileSync, spawn } from "node:child_process"; import fs from "node:fs"; import path from "node:path"; -import { getCompatFeatureMemberships } from "./compat-rows.mjs"; +import { finished } from "node:stream/promises"; +import { getCompatFeatureMemberships, parseCompatRow } from "./compat-rows.mjs"; import { compareStringsCaseSensitive } from "./shared.mjs"; const manifestPath = "manifests/baseline-js.json"; + const datasetPath = "datasets/web-features-js-compat.json"; /** @@ -17,28 +19,46 @@ export async function runRecordedUpdateStep(options) { if (!/^[a-z][a-z0-9-]*$/u.test(options.step)) { throw new Error(`Invalid update step name: ${options.step}`); } + fs.mkdirSync(options.outputDirectory, { recursive: true }); const logPath = path.join(options.outputDirectory, `${options.step}.log`); const resultPath = path.join(options.outputDirectory, `${options.step}.json`); - fs.writeFileSync(logPath, ""); + const log = fs.createWriteStream(logPath); const startedAt = new Date().toISOString(); - /** @type {{ code: number | null; signal: NodeJS.Signals | null; error?: string; }} */ - const result = await new Promise(resolve => { - const child = spawn(options.command, options.args, { cwd: options.cwd, stdio: ["ignore", "pipe", "pipe"] }); - /** @param {Buffer} chunk */ - const record = chunk => { - fs.appendFileSync(logPath, chunk); - process.stdout.write(chunk); - }; - child.stdout.on("data", record); - child.stderr.on("data", record); + const child = spawn(options.command, options.args, { cwd: options.cwd, stdio: ["ignore", "pipe", "pipe"] }); + + for (const stream of [child.stdout, child.stderr]) { + stream.pipe(log, { end: false }); + stream.pipe(process.stdout, { end: false }); + } + + /** @type {Promise<{ code: number | null; signal: NodeJS.Signals | null; error?: string; }>} */ + const completion = new Promise(resolve => { + /** @type {string | undefined} */ + let startError; child.on("error", error => { - record(Buffer.from(`${error.message}\n`)); - resolve({ code: null, signal: null, error: error.message }); + startError = error.stack ?? error.message; + log.write(`${startError}\n`); + process.stdout.write(`${startError}\n`); + }); + child.on("close", (code, signal) => { + log.end(); + resolve({ code, signal, error: startError }); }); - child.on("close", (code, signal) => resolve({ code, signal })); }); - const exitCode = result.code ?? 1; + + const result = await Promise.all([completion, finished(log)]) + .then(([result]) => result) + .catch(async error => { + child.kill(); + child.stdout.destroy(); + child.stderr.destroy(); + const result = await completion; + + return { ...result, code: 1, error: error instanceof Error ? error.stack ?? error.message : String(error) }; + }); + + const exitCode = result.error ? 1 : result.code ?? 1; fs.writeFileSync(resultPath, `${JSON.stringify({ step: options.step, command: [options.command, ...options.args], @@ -49,6 +69,7 @@ export async function runRecordedUpdateStep(options) { error: result.error, logFile: path.basename(logPath), }, undefined, 2)}\n`); + return exitCode; } @@ -61,19 +82,23 @@ export function writeUpdateDiagnostics(options) { fs.mkdirSync(options.outputDirectory, { recursive: true }); /** @type {string[]} */ const readErrors = []; + /** @param {string} relativePath @param {boolean} previous */ const readInput = (relativePath, previous) => { try { const text = previous ? execFileSync("git", ["show", `HEAD:${relativePath}`], { cwd: options.cwd, encoding: "utf8", stdio: ["ignore", "pipe", "pipe"], maxBuffer: 16 * 1024 * 1024 }) : fs.readFileSync(path.join(options.cwd, relativePath), "utf8"); + return JSON.parse(text); } catch (error) { readErrors.push(`${previous ? "HEAD" : "Working tree"} ${relativePath}: ${error instanceof Error ? error.message : String(error)}`); + return undefined; } }; + const previousManifest = readInput(manifestPath, true); const currentManifest = readInput(manifestPath, false); const previousPackage = readInput("package.json", true); @@ -81,19 +106,35 @@ export function writeUpdateDiagnostics(options) { const previousDataset = readInput(datasetPath, true); const currentDataset = readInput(datasetPath, false); let dataset; + try { dataset = compareDatasetInputs(previousDataset, currentDataset); } catch (error) { readErrors.push(`Dataset comparison: ${error instanceof Error ? error.message : String(error)}`); } - const commands = fs.readdirSync(options.outputDirectory) - .filter(name => name.endsWith(".json") && name !== "summary.json") - .map(name => JSON.parse(fs.readFileSync(path.join(options.outputDirectory, name), "utf8"))) - .sort((left, right) => left.startedAt.localeCompare(right.startedAt)); + + const commands = []; + + for (const name of fs.readdirSync(options.outputDirectory)) { + if (!name.endsWith(".json") || name === "summary.json") { + continue; + } + + try { + commands.push(parseCommandRecord(JSON.parse(fs.readFileSync(path.join(options.outputDirectory, name), "utf8")))); + } + catch (error) { + readErrors.push(`Command record ${name}: ${error instanceof Error ? error.message : String(error)}`); + } + } + + commands.sort((left, right) => compareStringsCaseSensitive(left.startedAt, right.startedAt)); + const failedSteps = Object.entries(options.steps ?? {}) .filter(([, step]) => step.outcome === "failure" || step.outcome === "cancelled") .map(([step, value]) => ({ step, outcome: value.outcome })); + const summary = { status: failedSteps.length || commands.some(command => command.exitCode !== 0) ? "failed" @@ -107,6 +148,7 @@ export function writeUpdateDiagnostics(options) { dataset, readErrors, }; + const lines = [ "# Update diagnostics", "", @@ -135,35 +177,105 @@ export function writeUpdateDiagnostics(options) { ...summary.dataset.changed.map(key => `- Changed: \`${key}\`.`), ] : []), ]; + for (const command of commands.filter(command => command.exitCode !== 0)) { - const log = fs.readFileSync(path.join(options.outputDirectory, command.logFile), "utf8"); - lines.push("", `## ${command.step} failure`, "", "```text", log.trimEnd().split(/\r?\n/u).slice(-120).join("\n").replaceAll("```", "'''"), "```"); + try { + const log = readLogTail(path.join(options.outputDirectory, command.logFile)); + lines.push("", `## ${command.step} failure`, "", "```text", log.replaceAll("```", "'''"), "```"); + } + catch (error) { + readErrors.push(`Command log ${command.logFile}: ${error instanceof Error ? error.message : String(error)}`); + } } + if (readErrors.length) { lines.push("", "## Input read errors", "", ...readErrors.map(error => `- ${error}`)); } + if (summary.status === "failed") { lines.push("", "## Recovery", "", "Inspect the failed step's log in this artifact. For registry drift, review the named compat keys and their upstream declarations before editing registry/compat-management.json. For an upstream layout error, update the TypeScript adapter and its fixtures.", "", "Run npm run validate after the repair. TypeScript source updates also require npm run test:typescript:gate against the candidate commit. Rerun the update workflow before merging its pull request."); } + const markdown = `${lines.join("\n")}\n`; fs.writeFileSync(path.join(options.outputDirectory, "summary.json"), `${JSON.stringify(summary, undefined, 2)}\n`); fs.writeFileSync(path.join(options.outputDirectory, "summary.md"), markdown); + return { summary, markdown }; } -/** @param {any} previous @param {any} current */ +/** @param {unknown} value */ +function parseCommandRecord(value) { + if (!value || typeof value !== "object" + || !("step" in value) || !("command" in value) || !("startedAt" in value) + || !("finishedAt" in value) || !("exitCode" in value) || !("signal" in value) + || !("logFile" in value)) { + throw new Error("Invalid command record"); + } + + const { step, command, startedAt, finishedAt, exitCode, signal, logFile } = value; + const error = "error" in value ? value.error : undefined; + + if (typeof step !== "string" || !/^[a-z][a-z0-9-]*$/u.test(step) + || !Array.isArray(command) || !command.length + || typeof startedAt !== "string" || !Number.isFinite(Date.parse(startedAt)) + || typeof finishedAt !== "string" || !Number.isFinite(Date.parse(finishedAt)) + || typeof exitCode !== "number" || !Number.isInteger(exitCode) || exitCode < 0 + || (signal !== null && typeof signal !== "string") + || (error !== undefined && typeof error !== "string") + || typeof logFile !== "string" || logFile !== `${step}.log`) { + throw new Error("Invalid command record fields"); + } + + const parsedCommand = []; + + for (const argument of command) { + if (typeof argument !== "string") { + throw new Error("Invalid command argument"); + } + + parsedCommand.push(argument); + } + + return { step, command: parsedCommand, startedAt, finishedAt, exitCode, signal, error, logFile }; +} + +/** @param {string} logPath */ +function readLogTail(logPath) { + const descriptor = fs.openSync(logPath, "r"); + + try { + const size = fs.fstatSync(descriptor).size; + const buffer = Buffer.alloc(Math.min(size, 64 * 1024)); + const bytesRead = fs.readSync(descriptor, buffer, 0, buffer.length, size - buffer.length); + const text = buffer.toString("utf8", 0, bytesRead).trimEnd().split(/\r?\n/u).slice(-120).join("\n"); + + return size > buffer.length ? `[Log truncated to the last 64 KiB.]\n${text}` : text; + } + finally { + fs.closeSync(descriptor); + } +} + +/** @param {unknown} previous @param {unknown} current */ function compareDatasetInputs(previous, current) { - if (!Array.isArray(previous?.compatRows) || !Array.isArray(current?.compatRows)) { + if (previous === undefined || current === undefined) { return undefined; } - /** @param {any[]} rows */ - const index = rows => { + + /** @param {unknown} dataset */ + const index = dataset => { + if (!dataset || typeof dataset !== "object" || !("compatRows" in dataset) || !Array.isArray(dataset.compatRows)) { + throw new Error("Dataset must contain a compatRows array"); + } + /** @type {Map} */ const grouped = new Map(); - for (const row of rows) { + + for (const value of dataset.compatRows) { + const row = parseCompatRow(value); const values = grouped.get(row.compatKey) ?? []; values.push(JSON.stringify({ baselineStatus: row.baselineStatus, @@ -180,10 +292,13 @@ function compareDatasetInputs(previous, current) { })); grouped.set(row.compatKey, values); } + return new Map([...grouped].map(([key, values]) => [key, JSON.stringify(values.sort())])); }; - const before = index(previous.compatRows); - const after = index(current.compatRows); + + const before = index(previous); + const after = index(current); + return { added: [...after.keys()].filter(key => !before.has(key)).sort(), removed: [...before.keys()].filter(key => !after.has(key)).sort(), diff --git a/scripts/update-diagnostics.mjs b/scripts/update-diagnostics.mjs index 6ec9091..01e926f 100644 --- a/scripts/update-diagnostics.mjs +++ b/scripts/update-diagnostics.mjs @@ -5,7 +5,9 @@ import path from "node:path"; import { runRecordedUpdateStep, writeUpdateDiagnostics } from "../lib/update-diagnostics.mjs"; const cwd = process.cwd(); + const outputDirectory = path.join(cwd, ".tmp", "update-diagnostics"); + const [mode, step, separator, command, ...args] = process.argv.slice(2); if (mode === "run" && step && separator === "--" && command) { @@ -17,9 +19,11 @@ else if (mode === "report" && !step) { outputDirectory, steps: JSON.parse(process.env.UPDATE_WORKFLOW_STEPS ?? "{}"), }); + if (process.env.GITHUB_STEP_SUMMARY) { fs.appendFileSync(process.env.GITHUB_STEP_SUMMARY, markdown); } + console.log(`Wrote ${path.relative(cwd, outputDirectory)}/summary.md`); } else { diff --git a/scripts/write-update-pr-body.mjs b/scripts/write-update-pr-body.mjs index f6c4a5e..c136a22 100644 --- a/scripts/write-update-pr-body.mjs +++ b/scripts/write-update-pr-body.mjs @@ -8,12 +8,17 @@ import { } from "node:fs/promises"; import path from "node:path"; import { fileURLToPath } from "node:url"; +import { resolveManagedOutputPath } from "../lib/shared.mjs"; import { compareYearContracts } from "../lib/year-contracts.mjs"; const scriptPath = fileURLToPath(import.meta.url); + const scriptDirectory = path.dirname(scriptPath); + const repoRoot = path.resolve(scriptDirectory, ".."); + const defaultManifestPath = path.join(repoRoot, "manifests", "baseline-js.json"); + const defaultOutputPath = path.join(repoRoot, ".tmp", "baseline-lib-update-pr.md"); if (process.argv[1] && path.resolve(process.argv[1]) === scriptPath) { @@ -27,10 +32,15 @@ async function main(args) { const manifestPath = path.resolve(args.manifest ?? defaultManifestPath); const outputPath = path.resolve(args.out ?? defaultOutputPath); const summaryOutputPath = args.summaryOut ? path.resolve(args.summaryOut) : undefined; - const baseRef = args.baseRef ?? "HEAD"; + const baseRef = readFromGit(["rev-parse", "--verify", `${args.baseRef ?? "HEAD"}^{commit}`]).trim(); const currentManifest = await readJsonFile(manifestPath); const currentState = await readCurrentState(currentManifest, manifestPath); - const previousManifest = readJsonFileFromGit(baseRef, path.relative(repoRoot, manifestPath)); + const manifestRelativePath = path.relative(repoRoot, manifestPath); + + const previousManifest = readFromGit(["ls-tree", "--name-only", baseRef, "--", manifestRelativePath]).trim() + ? readJsonFileFromGit(baseRef, manifestRelativePath) + : undefined; + const previousState = previousManifest ? readStateFromGit(previousManifest, manifestPath, baseRef) : undefined; @@ -62,6 +72,7 @@ function parseArgs(argv) { for (let index = 0; index < argv.length; index++) { const current = argv[index]; + switch (current) { case "--manifest": args.manifest = requireArgValue(argv[++index], current); @@ -95,6 +106,7 @@ function requireArgValue(value, flagName) { if (!value) { throw new Error(`Missing value for ${flagName}`); } + return value; } @@ -115,9 +127,9 @@ Examples: */ async function readCurrentState(manifest, manifestPath) { return { - classification: await readJsonFile(resolveManifestRelativePath(manifestPath, manifest.classificationOutput)), - generation: await readJsonFile(resolveManifestRelativePath(manifestPath, manifest.generationOutput)), - compatManagement: await readJsonFile(resolveManifestRelativePath(manifestPath, manifest.compatManagementOutput)), + classification: await readJsonFile(resolveReportPath(manifest.classificationOutput, manifestPath, "classificationOutput")), + generation: await readJsonFile(resolveReportPath(manifest.generationOutput, manifestPath, "generationOutput")), + compatManagement: await readJsonFile(resolveReportPath(manifest.compatManagementOutput, manifestPath, "compatManagementOutput")), }; } @@ -128,9 +140,9 @@ async function readCurrentState(manifest, manifestPath) { */ function readStateFromGit(manifest, manifestPath, baseRef) { return { - classification: readJsonFileFromGit(baseRef, path.relative(repoRoot, resolveManifestRelativePath(manifestPath, manifest.classificationOutput))), - generation: readJsonFileFromGit(baseRef, path.relative(repoRoot, resolveManifestRelativePath(manifestPath, manifest.generationOutput))), - compatManagement: readJsonFileFromGit(baseRef, path.relative(repoRoot, resolveManifestRelativePath(manifestPath, manifest.compatManagementOutput))), + classification: readJsonFileFromGit(baseRef, path.relative(repoRoot, resolveReportPath(manifest.classificationOutput, manifestPath, "classificationOutput"))), + generation: readJsonFileFromGit(baseRef, path.relative(repoRoot, resolveReportPath(manifest.generationOutput, manifestPath, "generationOutput"))), + compatManagement: readJsonFileFromGit(baseRef, path.relative(repoRoot, resolveReportPath(manifest.compatManagementOutput, manifestPath, "compatManagementOutput"))), }; } @@ -159,6 +171,7 @@ export function buildUpdateSummary(options) { const currentAllowEntries = currentState.generation.allowEntries ?? []; const previousAllowEntries = previousState?.generation?.allowEntries ?? []; const allowEntryChanges = compareAllowEntries(previousAllowEntries, currentAllowEntries); + const yearContractComparison = compareYearContracts( JSON.stringify(previousState?.generation ?? {}), JSON.stringify(currentState.generation), @@ -166,21 +179,27 @@ export function buildUpdateSummary(options) { /** @type {string[]} */ const reviewFlags = []; + if (!previousCompatRegistry || previousCompatRegistry.sourceHash !== currentState.compatManagement.registry.sourceHash) { reviewFlags.push("`registry/compat-management.json` changed. Verify every edited group still has the right category, upstream action, and primary-source URLs."); } + if ((delta(currentClassificationSummary.notModeledUpstreamCount, previousClassificationSummary?.notModeledUpstreamCount) ?? 0) !== 0) { reviewFlags.push("`not-modeled-upstream` count changed. Inspect `derived/current/classification.json` and `derived/current/compat-management-report.json` before merging."); } + if ((delta(currentCompatSummary.managedUpstreamStateCounts.actionable, previousState?.compatManagement?.summary?.managedUpstreamStateCounts?.actionable) ?? 0) !== 0) { reviewFlags.push("Actionable upstream-gap count changed. Confirm whether a new or updated `microsoft/TypeScript` or `web-features` action item is needed."); } + if (allowEntryChanges.length) { reviewFlags.push("Allow entry state or compat contract changed. Verify the polyfill contract and generated declaration diff before merging."); } + if (yearContractComparison.changes.length) { reviewFlags.push("Baseline year contract changed. Inspect every year declaration diff before selecting the release bump."); } + if (!reviewFlags.length) { reviewFlags.push("No special review flags beyond the normal generated diff review."); } @@ -307,23 +326,29 @@ function compareAllowEntries(previousEntries, currentEntries) { for (const name of names) { const previous = previousByName.get(name); const current = currentByName.get(name); + if (!previous) { changes.push(`added allow/${name} (${current?.kind ?? "unknown"})`); continue; } + if (!current) { changes.push(`removed allow/${name}`); continue; } + if (previous.kind !== current.kind) { changes.push(`allow/${name}: ${previous.kind ?? "unknown"} -> ${current.kind ?? "unknown"}`); } + const previousCompatKeys = [...(previous.compatKeys ?? [])].sort(); const currentCompatKeys = [...(current.compatKeys ?? [])].sort(); + if (JSON.stringify(previousCompatKeys) !== JSON.stringify(currentCompatKeys)) { changes.push(`allow/${name}: compat contract changed`); } } + return changes; } @@ -339,33 +364,31 @@ async function readJsonFile(filePath) { * @param {string} repoRelativePath */ function readJsonFileFromGit(gitRef, repoRelativePath) { - const sourceText = readTextFileFromGit(gitRef, repoRelativePath); - return sourceText ? JSON.parse(sourceText) : undefined; + return JSON.parse(readFromGit(["show", `${gitRef}:${repoRelativePath}`])); } /** - * @param {string} gitRef - * @param {string} repoRelativePath + * @param {string[]} args */ -function readTextFileFromGit(gitRef, repoRelativePath) { - try { - return execFileSync("git", ["show", `${gitRef}:${repoRelativePath}`], { - cwd: repoRoot, - encoding: "utf8", - stdio: ["ignore", "pipe", "ignore"], - }); - } - catch { - return undefined; - } +function readFromGit(args) { + return execFileSync("git", args, { + cwd: repoRoot, + encoding: "utf8", + stdio: ["ignore", "pipe", "pipe"], + maxBuffer: 16 * 1024 * 1024, + }); } /** - * @param {string} manifestFilePath * @param {string} relativePath + * @param {string} manifestPath + * @param {string} propertyName */ -function resolveManifestRelativePath(manifestFilePath, relativePath) { - return path.resolve(path.dirname(manifestFilePath), relativePath); +function resolveReportPath(relativePath, manifestPath, propertyName) { + return resolveManagedOutputPath(relativePath, repoRoot, manifestPath, propertyName, [ + path.join(repoRoot, "derived"), + path.join(repoRoot, ".tmp"), + ]); } /** @@ -373,9 +396,10 @@ function resolveManifestRelativePath(manifestFilePath, relativePath) { * @param {number | undefined | null} previous */ function delta(current, previous) { - if (typeof current !== "number" || typeof previous !== "number") { + if (current == null || previous == null) { return undefined; } + return current - previous; } @@ -384,9 +408,10 @@ function delta(current, previous) { * @param {number | undefined | null} countDelta */ function formatCountWithDelta(count, countDelta) { - if (typeof countDelta !== "number" || countDelta === 0) { + if (countDelta == null || countDelta === 0) { return `${count}`; } + return `${count} (${countDelta > 0 ? "+" : ""}${countDelta})`; } @@ -412,8 +437,10 @@ function formatTransition(previous, current) { if (!previous) { return current ?? "n/a"; } + if (previous === current) { return current ?? previous; } + return `${previous} -> ${current ?? "n/a"}`; } diff --git a/test/update-diagnostics.test.mjs b/test/update-diagnostics.test.mjs index 5ad6529..e054405 100644 --- a/test/update-diagnostics.test.mjs +++ b/test/update-diagnostics.test.mjs @@ -1,7 +1,8 @@ // @ts-check import assert from "node:assert/strict"; -import { execFileSync, spawnSync } from "node:child_process"; +import { execFileSync, spawn, spawnSync } from "node:child_process"; +import { once } from "node:events"; import fs from "node:fs"; import path from "node:path"; import test from "node:test"; @@ -11,6 +12,7 @@ import { cleanupTempDirectories, createTempDirectory, repoRoot, writeJsonFile } /** @type {string[]} */ const tempDirectories = []; + test.afterEach(() => cleanupTempDirectories(tempDirectories)); test("a failed update keeps its exit code and reports candidate inputs without reading stale generated reports", () => { @@ -21,16 +23,16 @@ test("a failed update keeps its exit code and reports candidate inputs without r writeJsonFile(path.join(cwd, "package.json"), { dependencies: { "web-features": "1.0.0" } }); writeJsonFile(manifestPath, { snapshot: { webFeaturesPackageVersion: "1.0.0" } }); writeJsonFile(datasetPath, { compatRows: [ - { compatKey: "javascript.builtins.Known", featureId: "known", baselineStatus: "low" }, - { compatKey: "javascript.builtins.Removed", featureId: "removed", baselineStatus: false }, + { compatKey: "javascript.builtins.Known", featureId: "known", featureName: "Known", baselineStatus: "low" }, + { compatKey: "javascript.builtins.Removed", featureId: "removed", featureName: "Removed", baselineStatus: false }, ] }); commitInputs(cwd); writeJsonFile(manifestPath, { snapshot: { webFeaturesPackageVersion: "2.0.0" } }); writeJsonFile(path.join(cwd, "package.json"), { dependencies: { "web-features": "2.0.0" } }); writeJsonFile(datasetPath, { compatRows: [ - { compatKey: "javascript.builtins.Known", featureId: "known", baselineStatus: "high" }, - { compatKey: "javascript.builtins.Known", featureId: "other-feature", baselineStatus: "high" }, - { compatKey: "javascript.builtins.New", featureId: "new", baselineStatus: false }, + { compatKey: "javascript.builtins.Known", featureId: "known", featureName: "Known", baselineStatus: "high" }, + { compatKey: "javascript.builtins.Known", featureId: "other-feature", featureName: "Other", baselineStatus: "high" }, + { compatKey: "javascript.builtins.New", featureId: "new", featureName: "New", baselineStatus: false }, ] }); const derivedDirectory = path.join(cwd, "derived", "current"); fs.mkdirSync(derivedDirectory, { recursive: true }); @@ -66,14 +68,98 @@ test("an unavailable executable still leaves a failed command record", () => { assert.match(result.error, /ENOENT/u); }); +test("command capture applies backpressure and preserves all output before recording exit", { timeout: 10_000 }, async () => { + const cwd = createTempDirectory(tempDirectories); + const markerPath = path.join(cwd, "output-finished"); + + const child = spawn(process.execPath, [path.join(repoRoot, "scripts/update-diagnostics.mjs"), "run", "large-output", "--", process.execPath, "-e", ` + const fs = require('node:fs'); + const chunk = Buffer.alloc(64 * 1024, 'x'); + let remaining = 64; + function write() { + while (remaining > 0) { + remaining--; + if (!process.stdout.write(chunk)) { + process.stdout.once('drain', write); + return; + } + } + fs.writeFileSync(${JSON.stringify(markerPath)}, 'done'); + process.exitCode = 23; + } + write(); + `], { cwd, stdio: ["ignore", "pipe", "pipe"] }); + + const completion = once(child, "close"); + + try { + await once(child.stdout, "readable"); + await new Promise(resolve => setTimeout(resolve, 300)); + assert.equal(fs.existsSync(markerPath), false, "producer must wait while the console consumer is paused"); + let outputBytes = 0; + child.stdout.on("data", chunk => { outputBytes += chunk.length; }); + child.stdout.resume(); + const [exitCode] = await completion; + assert.equal(exitCode, 23); + assert.equal(outputBytes, 4 * 1024 * 1024); + const outputDirectory = path.join(cwd, ".tmp/update-diagnostics"); + assert.equal(fs.statSync(path.join(outputDirectory, "large-output.log")).size, outputBytes); + const record = JSON.parse(fs.readFileSync(path.join(outputDirectory, "large-output.json"), "utf8")); + assert.equal(record.exitCode, 23); + } + finally { + child.kill(); + child.stdout.resume(); + child.stderr.resume(); + await completion; + } +}); + +test("a log write failure stops the command and remains a blocking result", () => { + const cwd = createTempDirectory(tempDirectories); + const outputDirectory = path.join(cwd, ".tmp/update-diagnostics"); + fs.mkdirSync(path.join(outputDirectory, "generate.log"), { recursive: true }); + const failure = runDiagnostics(cwd, ["run", "generate", "--", process.execPath, "-e", "console.log('starting'); setTimeout(() => { process.exitCode = 17; }, 1000)"]); + assert.equal(failure.status, 1); + const record = JSON.parse(fs.readFileSync(path.join(outputDirectory, "generate.json"), "utf8")); + assert.equal(record.exitCode, 1); + assert.match(record.error, /EISDIR/u); +}); + +test("incomplete command artifacts retain the other failure logs and a bounded log tail", () => { + const cwd = createTempDirectory(tempDirectories); + const outputDirectory = path.join(cwd, ".tmp/update-diagnostics"); + const failure = runDiagnostics(cwd, ["run", "generate", "--", process.execPath, "-e", "process.exitCode = 17"]); + assert.equal(failure.status, 17); + const command = JSON.parse(fs.readFileSync(path.join(outputDirectory, "generate.json"), "utf8")); + fs.writeFileSync(path.join(outputDirectory, "generate.log"), `${"x".repeat(2 * 1024 * 1024)}\n${Array.from({ length: 200 }, (_, index) => `Failure line ${index}`).join("\n")}\n`); + fs.writeFileSync(path.join(outputDirectory, "interrupted.json"), "{"); + writeJsonFile(path.join(outputDirectory, "missing.json"), { ...command, step: "missing", logFile: "missing.log" }); + writeJsonFile(path.join(outputDirectory, "invalid.json"), { ...command, logFile: "../outside.log" }); + + const { summary, markdown } = writeUpdateDiagnostics({ cwd, outputDirectory }); + assert.equal(summary.status, "failed"); + assert.equal(summary.commands.length, 2); + assert.ok(summary.readErrors.some(error => error.startsWith("Command record interrupted.json:"))); + assert.ok(summary.readErrors.some(error => error.startsWith("Command record invalid.json:"))); + assert.ok(summary.readErrors.some(error => error.startsWith("Command log missing.log:"))); + assert.match(markdown, /Log truncated to the last 64 KiB/u); + assert.match(markdown, /Failure line 199/u); + assert.doesNotMatch(markdown, /Failure line 79\n/u); + assert.ok(markdown.length < 70 * 1024); + assert.ok(fs.existsSync(path.join(outputDirectory, "summary.json"))); +}); + test("setup failures produce a report even when no command ran and candidate inputs are missing", () => { const cwd = createTempDirectory(tempDirectories); execFileSync("git", ["init", "--quiet"], { cwd }); const summaryPath = path.join(cwd, "github-summary.md"); + const result = runDiagnostics(cwd, ["report"], { UPDATE_WORKFLOW_STEPS: JSON.stringify({ "setup-node": { outcome: "failure" } }), GITHUB_STEP_SUMMARY: summaryPath, }); + assert.equal(result.status, 0, result.stderr); const summary = JSON.parse(fs.readFileSync(path.join(cwd, ".tmp", "update-diagnostics", "summary.json"), "utf8")); assert.equal(summary.status, "failed"); @@ -89,12 +175,14 @@ test("diagnostics track secondary feature memberships and retain failure logs fo const first = { featureId: "first", featureName: "First", snapshot: ["es2020"], group: ["group-a", "group-b"] }; const second = { featureId: "second", featureName: "Second", snapshot: ["es2021"], group: ["group-c"] }; const third = { ...second, featureId: "third", featureName: "Third" }; + const row = { compatKey: "javascript.builtins.Known", ...first, baselineStatus: "high", featureMemberships: [first, second], }; + writeJsonFile(path.join(cwd, "package.json"), { dependencies: {} }); writeJsonFile(path.join(cwd, "manifests", "baseline-js.json"), { snapshot: {} }); writeJsonFile(datasetPath, { compatRows: [row] }); @@ -112,6 +200,7 @@ test("diagnostics track secondary feature memberships and retain failure logs fo const { summary } = writeUpdateDiagnostics({ cwd, outputDirectory }); assert.deepEqual(summary.dataset?.changed, [row.compatKey]); } + writeJsonFile(datasetPath, { compatRows: [{ ...row, featureMemberships: [second, { ...first, group: [...first.group].reverse() }] }] }); assert.deepEqual(writeUpdateDiagnostics({ cwd, outputDirectory }).summary.dataset?.changed, []); @@ -120,14 +209,38 @@ test("diagnostics track secondary feature memberships and retain failure logs fo const { summary, markdown } = writeUpdateDiagnostics({ cwd, outputDirectory }); assert.equal(summary.status, "failed"); assert.equal(summary.dataset, undefined); - assert.match(summary.readErrors.join("\n"), /feature membership with invalid snapshot/u); + assert.match(summary.readErrors.join("\n"), /feature membership snapshot must be an array of strings/u); assert.match(markdown, /candidate rejected/u); }); +test("malformed dataset inputs make a successful command report incomplete", () => { + const cwd = createTempDirectory(tempDirectories); + const outputDirectory = path.join(cwd, ".tmp/update-diagnostics"); + const datasetPath = path.join(cwd, "datasets/web-features-js-compat.json"); + writeJsonFile(path.join(cwd, "package.json"), { dependencies: {} }); + writeJsonFile(path.join(cwd, "manifests/baseline-js.json"), { snapshot: {} }); + writeJsonFile(datasetPath, { compatRows: [] }); + commitInputs(cwd); + assert.equal(runDiagnostics(cwd, ["run", "install", "--", process.execPath, "-e", "process.exitCode = 0"]).status, 0); + + for (const candidate of [ + null, + {}, + { compatRows: [{ compatKey: "javascript.builtins.Known", featureId: "known", baselineStatus: "high" }] }, + ]) { + writeJsonFile(datasetPath, candidate); + const { summary } = writeUpdateDiagnostics({ cwd, outputDirectory }); + assert.equal(summary.status, "incomplete"); + assert.equal(summary.dataset, undefined); + assert.equal(summary.readErrors.length, 1); + assert.match(summary.readErrors[0], /^Dataset comparison:/u); + } +}); + test("update workflows keep checks blocking and collect candidate diagnostics before creating a pull request", () => { for (const name of ["weekly-update.yml", "typescript-update.yml"]) { const workflow = parse(fs.readFileSync(path.join(repoRoot, ".github", "workflows", name), "utf8")); - /** @type {Array>} */ + /** @type {Array<{ id?: string; run?: string; if?: string; uses?: string; with?: { path?: string; "include-hidden-files"?: boolean; }; "continue-on-error"?: boolean; }>} */ const steps = workflow.jobs.update.steps; const reportIndex = steps.findIndex(step => step.run === "node scripts/update-diagnostics.mjs report"); const pullRequestIndex = steps.findIndex(step => step.id === "pull-request"); @@ -135,9 +248,10 @@ test("update workflows keep checks blocking and collect candidate diagnostics be assert.ok(reportIndex >= 0 && pullRequestIndex > reportIndex); assert.equal(steps[reportIndex].if, "always()"); assert.equal(upload?.if, "always()"); - assert.equal(upload?.with["path"], ".tmp/update-diagnostics"); - assert.equal(upload?.with["include-hidden-files"], true); + assert.equal(upload?.with?.path, ".tmp/update-diagnostics"); + assert.equal(upload?.with?.["include-hidden-files"], true); assert.equal(steps[pullRequestIndex].if, undefined); + for (const step of steps.filter(step => step.run?.includes("update-diagnostics.mjs run"))) { assert.ok(steps.indexOf(step) < reportIndex, `${name}: ${step.id} runs after diagnostics`); assert.equal(step["continue-on-error"], undefined); diff --git a/test/update-pr-body.test.mjs b/test/update-pr-body.test.mjs new file mode 100644 index 0000000..479ab10 --- /dev/null +++ b/test/update-pr-body.test.mjs @@ -0,0 +1,95 @@ +// @ts-check + +import assert from "node:assert/strict"; +import { execFileSync, spawnSync } from "node:child_process"; +import fs from "node:fs"; +import path from "node:path"; +import test from "node:test"; +import { cleanupTempDirectories, createTempDirectory, repoRoot, writeJsonFile } from "./helpers.mjs"; + +/** @type {string[]} */ +const tempDirectories = []; + +test.afterEach(() => cleanupTempDirectories(tempDirectories)); + +test("the update PR CLI reads repo-relative reports and retains large previous reports", () => { + const cwd = createUpdateRepository(); + const classificationPath = path.join(cwd, "derived/current/classification.json"); + const classification = JSON.parse(fs.readFileSync(classificationPath, "utf8")); + classification.padding = "x".repeat(1024 * 1024); + writeJsonFile(classificationPath, classification); + commitInputs(cwd); + classification.summary.includedCompatCount += 1; + writeJsonFile(classificationPath, classification); + + const result = runUpdateBody(cwd); + assert.equal(result.status, 0, result.stderr); + const summary = JSON.parse(fs.readFileSync(path.join(cwd, ".tmp/update.json"), "utf8")); + assert.equal(summary.deltas.includedCompatCount, 1); + assert.equal(summary.previousState.classification.padding.length, 1024 * 1024); + assert.match(fs.readFileSync(path.join(cwd, ".tmp/update.md"), "utf8"), /Included high rows: \d+ \(\+1\)/u); +}); + +test("the update PR CLI rejects invalid refs and missing reports instead of inventing an initial snapshot", () => { + const cwd = createUpdateRepository(); + commitInputs(cwd); + const invalidRef = runUpdateBody(cwd, ["--base-ref", "missing-update-base"]); + assert.notEqual(invalidRef.status, 0); + assert.match(invalidRef.stderr, /missing-update-base/u); + assert.equal(fs.existsSync(path.join(cwd, ".tmp/update.json")), false); + + execFileSync("git", ["rm", "--cached", "derived/current/generation.json"], { cwd }); + execFileSync("git", ["-c", "user.name=Test", "-c", "user.email=test@example.com", "commit", "--quiet", "-m", "Remove previous report"], { cwd }); + const missingReport = runUpdateBody(cwd); + assert.notEqual(missingReport.status, 0); + assert.match(missingReport.stderr, /derived\/current\/generation\.json/u); + assert.equal(fs.existsSync(path.join(cwd, ".tmp/update.json")), false); +}); + +test("a valid base commit without a manifest is an initial snapshot", () => { + const cwd = createUpdateRepository(); + execFileSync("git", ["-c", "user.name=Test", "-c", "user.email=test@example.com", "commit", "--allow-empty", "--quiet", "-m", "Initial commit"], { cwd }); + const result = runUpdateBody(cwd); + assert.equal(result.status, 0, result.stderr); + const summary = JSON.parse(fs.readFileSync(path.join(cwd, ".tmp/update.json"), "utf8")); + assert.equal(summary.previousManifest, undefined); + assert.equal(summary.deltas.includedCompatCount, undefined); +}); + +function createUpdateRepository() { + const cwd = createTempDirectory(tempDirectories); + + for (const relativePath of [ + "scripts/write-update-pr-body.mjs", + "lib/year-contracts.mjs", + "lib/shared.mjs", + "manifests/baseline-js.json", + "derived/current/classification.json", + "derived/current/generation.json", + "derived/current/compat-management-report.json", + ]) { + const destination = path.join(cwd, relativePath); + fs.mkdirSync(path.dirname(destination), { recursive: true }); + fs.copyFileSync(path.join(repoRoot, relativePath), destination); + } + + execFileSync("git", ["init", "--quiet"], { cwd }); + + return cwd; +} + +/** @param {string} cwd */ +function commitInputs(cwd) { + execFileSync("git", ["add", "."], { cwd }); + execFileSync("git", ["-c", "user.name=Test", "-c", "user.email=test@example.com", "commit", "--quiet", "-m", "Snapshot"], { cwd }); +} + +/** @param {string} cwd @param {string[]} args */ +function runUpdateBody(cwd, args = []) { + return spawnSync(process.execPath, [ + path.join(cwd, "scripts/write-update-pr-body.mjs"), + "--out", path.join(cwd, ".tmp/update.md"), + "--summary-out", path.join(cwd, ".tmp/update.json"), + ...args, + ], { cwd, encoding: "utf8" }); +}