From 431f1f1291f7b4bce116f4518a67d19f3274fece Mon Sep 17 00:00:00 2001 From: Kyle Brown Date: Mon, 31 Aug 2026 21:31:59 +0000 Subject: [PATCH 1/2] refactor(ci): port source and test report checks to JavaScript --- .../check_plugin_source_compatibility.mjs | 163 ++++++++++++ .../check_plugin_source_compatibility.py | 152 ----------- ...test_check_plugin_source_compatibility.mjs | 171 +++++++++++++ .../test_check_plugin_source_compatibility.py | 159 ------------ .github/workflows/node-ci.yml | 28 +- .github/workflows/test-quality.yml | 14 +- AGENTS.md | 7 +- sdk/typescript/package.json | 2 + sdk/typescript/pnpm-lock.yaml | 19 ++ .../scripts/compare-test-reports.mjs | 239 ++++++++++++++++++ .../scripts/compare-test-reports.py | 84 ------ .../tests-ts/release-automation.test.ts | 6 +- sdk/typescript/tests-ts/test-reports.test.ts | 148 ++++++++++- 13 files changed, 768 insertions(+), 424 deletions(-) create mode 100644 .github/scripts/check_plugin_source_compatibility.mjs delete mode 100644 .github/scripts/check_plugin_source_compatibility.py create mode 100644 .github/scripts/test_check_plugin_source_compatibility.mjs delete mode 100644 .github/scripts/test_check_plugin_source_compatibility.py create mode 100644 sdk/typescript/scripts/compare-test-reports.mjs delete mode 100644 sdk/typescript/scripts/compare-test-reports.py diff --git a/.github/scripts/check_plugin_source_compatibility.mjs b/.github/scripts/check_plugin_source_compatibility.mjs new file mode 100644 index 000000000..0b7b9cefe --- /dev/null +++ b/.github/scripts/check_plugin_source_compatibility.mjs @@ -0,0 +1,163 @@ +#!/usr/bin/env node +// Check that tracked plugin source stays portable across repository imports. + +import { spawnSync } from "node:child_process"; +import { lstatSync, readFileSync } from "node:fs"; +import { basename, extname, join, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; +import { parseArgs } from "node:util"; + +const MAX_SOURCE_FILE_BYTES = 150_000; +const MAX_DEPENDENCY_LOCK_BYTES = 2_000_000; +const DEPENDENCY_LOCK_NAMES = new Set([ + "Cargo.lock", + "package-lock.json", + "pnpm-lock.yaml", + "requirements.txt", + "uv.lock", + "yarn.lock", +]); +const LIST_ITEM = /^\s*(?:[-*+]|\d+[.)])\s+/u; +const HTML_BLOCK = /^\s*<\/?[A-Za-z][^>]*>\s*$/u; +const NATURAL_LINE_ENDINGS = new Set("\\.?!:;。!?:;)]}'\"`>"); +const utf8 = new TextDecoder("utf-8", { fatal: true, ignoreBOM: true }); + +function trackedFiles(pluginRoot) { + const result = spawnSync( + "git", + ["-C", pluginRoot, "ls-files", "-z", "--", "."], + { + maxBuffer: Infinity, + }, + ); + if (result.error) throw result.error; + if (result.status !== 0) { + throw new Error(result.stderr.toString().trim() || "git ls-files failed"); + } + return utf8.decode(result.stdout).split("\0").filter(Boolean); +} + +function isMarkdownStructure(line) { + const stripped = line.trim(); + return ( + !stripped || + ["#", ">", "|", "\n::directive\n
\n code\n\tmore code\n", + "A list\n- first\n1. second\n2) third\n", + "Complete.\nQuestion?\nBang!\nColon:\nSemicolon;\n。\n!\n?\n:\n;\nParen)\nBracket]\nBrace}\nQuote'\nDouble\"\nCode`\nAngle>\nBackslash\\\nBreak \nhttps://example.invalid/url\nlast\n", + ].map((content, index) => [`example-${index}.md`, content]), + ), + ); + const result = runChecker("--plugin-root", root); + assert.equal(result.status, 0, result.stderr); +}); + +test("rejects hard wraps after closed front matter and fences, including CRLF", () => { + const root = fixture({ + "README.MD": + "---\r\ntitle: Example\r\n---\r\n```\r\ncode\r\n```\r\nThis continues\r\non another line.\r\n", + }); + const result = runChecker("--plugin-root", root); + assert.equal(result.status, 1); + assert.match(result.stderr, /README\.MD:7: prose is hard-wrapped/u); +}); + +test("rejects dependency lock files above two megabytes", () => { + const root = fixture({ "pnpm-lock.yaml": "x".repeat(2_000_001) }); + const result = runChecker("--plugin-root", root); + assert.equal(result.status, 1); + assert.equal( + result.stderr, + "pnpm-lock.yaml: file is 2000001 bytes; maximum is 2000000 bytes\n", + ); +}); + +test( + "does not follow tracked symlinks outside the plugin", + { skip: process.platform === "win32" }, + () => { + const outside = fixture({ + "outside.md": "This outside prose continues\nonto another source line.\n", + }); + const root = fixture(); + symlinkSync(join(outside, "outside.md"), join(root, "linked.md")); + git(root, "add", "--", "linked.md"); + const result = runChecker("--plugin-root", root); + assert.equal(result.status, 0, result.stderr); + }, +); + +test("reports Git, missing tracked files, and invalid UTF-8 as check failures", () => { + const root = fixture({ "README.md": Buffer.from([0xff]) }); + for (const action of [ + () => {}, + () => unlinkSync(join(root, "README.md")), + () => rmSync(join(root, ".git"), { recursive: true }), + ]) { + action(); + const result = runChecker("--plugin-root", root); + assert.equal(result.status, 2); + assert.match(result.stderr, /^source compatibility check failed:/u); + } +}); + +test("help describes the source contract and invalid arguments fail", () => { + const result = runChecker("--help"); + assert.equal(result.status, 0, result.stderr); + assert.match(result.stdout, /tracked plugin source/u); + assert.equal(runChecker("--plugin-root").status, 2); + assert.equal(runChecker("--unknown").status, 2); +}); diff --git a/.github/scripts/test_check_plugin_source_compatibility.py b/.github/scripts/test_check_plugin_source_compatibility.py deleted file mode 100644 index f49a80420..000000000 --- a/.github/scripts/test_check_plugin_source_compatibility.py +++ /dev/null @@ -1,159 +0,0 @@ -from __future__ import annotations - -import os -import subprocess -import sys -from pathlib import Path - -import pytest - -CHECKER = Path(__file__).with_name("check_plugin_source_compatibility.py") - - -def initialize_repository(root: Path) -> None: - subprocess.run(["git", "init", "--quiet", str(root)], check=True) - - -def track(root: Path, *paths: str) -> None: - subprocess.run(["git", "-C", str(root), "add", "--", *paths], check=True) - - -def run_checker(root: Path, *args: str) -> subprocess.CompletedProcess[str]: - return subprocess.run( - [sys.executable, str(CHECKER), "--plugin-root", str(root), *args], - check=False, - capture_output=True, - text=True, - ) - - -def test_reports_tracked_source_violations_in_stable_order(tmp_path: Path) -> None: - (tmp_path / "notes.md").write_text( - "This prose continues in the middle of a sentence\nonto another source line.\n", - encoding="utf-8", - ) - (tmp_path / "oversized.py").write_bytes(b"x" * 150_001) - initialize_repository(tmp_path) - track(tmp_path, "oversized.py", "notes.md") - - result = run_checker(tmp_path) - - assert result.returncode == 1 - assert result.stdout == "" - assert result.stderr.splitlines() == [ - "notes.md:1: prose is hard-wrapped mid-sentence; use a natural Markdown line", - "oversized.py: file is 150001 bytes; maximum is 150000 bytes", - ] - - -def test_accepts_valid_source_and_ignores_untracked_files(tmp_path: Path) -> None: - (tmp_path / "README.md").write_text("A complete sentence.\n", encoding="utf-8") - (tmp_path / "package-lock.json").write_bytes(b"x" * 150_001) - (tmp_path / "untracked.md").write_text( - "This untracked prose continues\nonto another source line.\n", - encoding="utf-8", - ) - initialize_repository(tmp_path) - track(tmp_path, "README.md", "package-lock.json") - - result = run_checker(tmp_path) - - assert result.returncode == 0, result.stderr - assert result.stdout == "Plugin source compatibility checks passed.\n" - assert result.stderr == "" - - -def test_python_checkout_preserves_source_size_with_autocrlf(tmp_path: Path) -> None: - attributes = CHECKER.parents[2] / ".gitattributes" - (tmp_path / ".gitattributes").write_bytes(attributes.read_bytes()) - source = tmp_path / "module.py" - content = b"pass\n" * 30_000 - source.write_bytes(content) - initialize_repository(tmp_path) - subprocess.run( - ["git", "-C", str(tmp_path), "config", "--local", "core.autocrlf", "true"], - check=True, - ) - track(tmp_path, ".gitattributes", "module.py") - source.unlink() - subprocess.run( - ["git", "-C", str(tmp_path), "checkout-index", "--", "module.py"], - check=True, - ) - - result = run_checker(tmp_path) - - assert result.returncode == 0, result.stderr - assert source.read_bytes() == content - - -def test_accepts_prose_after_an_opening_thematic_break(tmp_path: Path) -> None: - (tmp_path / "README.md").write_text( - """--- -This prose continues -onto another source line. -""", - encoding="utf-8", - ) - initialize_repository(tmp_path) - track(tmp_path, "README.md") - - result = run_checker(tmp_path) - - assert result.returncode == 0, result.stderr - - -@pytest.mark.parametrize( - "content", - [ - "First clause,\ncontinues here.\n", - "First clause\n**continues** here.\n", - ], -) -def test_accepts_wraps_adjacent_to_inline_markup(tmp_path: Path, content: str) -> None: - (tmp_path / "README.md").write_text(content, encoding="utf-8") - initialize_repository(tmp_path) - track(tmp_path, "README.md") - - result = run_checker(tmp_path) - - assert result.returncode == 0, result.stderr - - -def test_rejects_dependency_lock_files_above_two_megabytes(tmp_path: Path) -> None: - (tmp_path / "pnpm-lock.yaml").write_bytes(b"x" * 2_000_001) - initialize_repository(tmp_path) - track(tmp_path, "pnpm-lock.yaml") - - result = run_checker(tmp_path) - - assert result.returncode == 1 - assert result.stderr == ("pnpm-lock.yaml: file is 2000001 bytes; maximum is 2000000 bytes\n") - - -@pytest.mark.skipif(os.name == "nt", reason="creating symlinks requires elevated Windows access") -def test_does_not_follow_tracked_symlinks_outside_the_plugin(tmp_path: Path) -> None: - outside = tmp_path.parent / f"{tmp_path.name}-outside.md" - outside.write_text( - "This outside prose continues\nonto another source line.\n", - encoding="utf-8", - ) - (tmp_path / "linked.md").symlink_to(outside) - initialize_repository(tmp_path) - track(tmp_path, "linked.md") - - result = run_checker(tmp_path) - - assert result.returncode == 0, result.stderr - - -def test_help_describes_the_source_contract() -> None: - result = subprocess.run( - [sys.executable, str(CHECKER), "--help"], - check=False, - capture_output=True, - text=True, - ) - - assert result.returncode == 0, result.stderr - assert "tracked plugin source" in result.stdout diff --git a/.github/workflows/node-ci.yml b/.github/workflows/node-ci.yml index 07108c904..93385bf03 100644 --- a/.github/workflows/node-ci.yml +++ b/.github/workflows/node-ci.yml @@ -82,11 +82,6 @@ jobs: exit 1 fi - - name: Check plugin source compatibility - if: steps.scope.outputs.ci-mode == 'markdown' - run: | - python .github/scripts/check_plugin_source_compatibility.py - - name: Set up pnpm if: steps.scope.outputs.check-markdown == 'true' uses: pnpm/action-setup@0ebf47130e4866e96fce0953f49152a61190b271 # v6.0.9 @@ -101,6 +96,11 @@ jobs: with: node-version: "22.13.0" + - name: Check plugin source compatibility + if: steps.scope.outputs.ci-mode == 'markdown' + run: | + node .github/scripts/check_plugin_source_compatibility.mjs + - name: Install dependencies if: steps.scope.outputs.check-markdown == 'true' run: pnpm --dir sdk/typescript install --frozen-lockfile @@ -362,6 +362,10 @@ jobs: uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: persist-credentials: false + - name: Set up Node.js + uses: actions/setup-node@53b83947a5a98c8d113130e565377fae1a50d02f # v6 + with: + node-version: "22.13.0" - name: Set up Python uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # v6 with: @@ -376,11 +380,13 @@ jobs: sudo apt-get install --yes ripgrep - name: Check plugin source compatibility run: | - python -m ruff check --config plugins/codex-security/pyproject.toml .github/scripts/check_plugin_source_compatibility.py .github/scripts/test_check_plugin_source_compatibility.py plugins/codex-security - python -m ruff format --check --config plugins/codex-security/pyproject.toml .github/scripts/check_plugin_source_compatibility.py .github/scripts/test_check_plugin_source_compatibility.py plugins/codex-security - python .github/scripts/check_plugin_source_compatibility.py + python -m ruff check --config plugins/codex-security/pyproject.toml plugins/codex-security + python -m ruff format --check --config plugins/codex-security/pyproject.toml plugins/codex-security + node .github/scripts/check_plugin_source_compatibility.mjs + - name: Test source compatibility checker + run: node --test .github/scripts/test_check_plugin_source_compatibility.mjs - name: Test Python source contracts - run: python -m pytest .github/scripts/test_check_plugin_source_compatibility.py plugins/codex-security/tests -q -n 4 --dist worksteal --max-worker-restart 0 --durations=30 --junitxml=reports/python.xml + run: python -m pytest plugins/codex-security/tests -q -n 4 --dist worksteal --max-worker-restart 0 --durations=30 --junitxml=reports/python.xml - name: Upload Python test reports if: always() continue-on-error: true @@ -391,10 +397,6 @@ jobs: path: reports/python.xml if-no-files-found: warn retention-days: 14 - - name: Set up Node.js - uses: actions/setup-node@53b83947a5a98c8d113130e565377fae1a50d02f # v6 - with: - node-version: "22.13.0" - name: Test deterministic triage eval contracts working-directory: plugins/codex-security/skills/triage-finding/evals run: node --run test:deterministic diff --git a/.github/workflows/test-quality.yml b/.github/workflows/test-quality.yml index 1400b02bc..0520fc1ec 100644 --- a/.github/workflows/test-quality.yml +++ b/.github/workflows/test-quality.yml @@ -113,6 +113,16 @@ jobs: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: persist-credentials: false + - uses: actions/setup-node@53b83947a5a98c8d113130e565377fae1a50d02f # v6 + with: + node-version: "22.13.0" + - uses: pnpm/action-setup@0ebf47130e4866e96fce0953f49152a61190b271 # v6.0.9 + with: + package_json_file: package.json + cache: true + cache_dependency_path: sdk/typescript/pnpm-lock.yaml + - name: Install comparison dependencies + run: pnpm --dir sdk/typescript install --frozen-lockfile - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 with: pattern: runner-* @@ -124,10 +134,10 @@ jobs: comparison_status=0 for os in ubuntu-latest windows-latest; do for mode in isolated parallel; do - python3 sdk/typescript/scripts/compare-test-reports.py "reports/runner-$os-baseline.xml" "reports/runner-$os-$mode.xml" >> "$GITHUB_STEP_SUMMARY" || comparison_status=1 + node sdk/typescript/scripts/compare-test-reports.mjs "reports/runner-$os-baseline.xml" "reports/runner-$os-$mode.xml" >> "$GITHUB_STEP_SUMMARY" || comparison_status=1 done done - python3 sdk/typescript/scripts/compare-test-reports.py reports/runner-windows-latest-baseline.xml 'reports/runner-windows-latest-shard-*.xml' >> "$GITHUB_STEP_SUMMARY" || comparison_status=1 + node sdk/typescript/scripts/compare-test-reports.mjs reports/runner-windows-latest-baseline.xml 'reports/runner-windows-latest-shard-*.xml' >> "$GITHUB_STEP_SUMMARY" || comparison_status=1 exit "$comparison_status" mutation: diff --git a/AGENTS.md b/AGENTS.md index a395c4c4a..0f3f83636 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -18,9 +18,10 @@ When changing `plugins/codex-security`, run its portable source checks before submitting the change: ```bash -python -m ruff check --config plugins/codex-security/pyproject.toml .github/scripts/check_plugin_source_compatibility.py .github/scripts/test_check_plugin_source_compatibility.py plugins/codex-security -python -m ruff format --check --config plugins/codex-security/pyproject.toml .github/scripts/check_plugin_source_compatibility.py .github/scripts/test_check_plugin_source_compatibility.py plugins/codex-security -python .github/scripts/check_plugin_source_compatibility.py +python -m ruff check --config plugins/codex-security/pyproject.toml plugins/codex-security +python -m ruff format --check --config plugins/codex-security/pyproject.toml plugins/codex-security +node .github/scripts/check_plugin_source_compatibility.mjs +node --test .github/scripts/test_check_plugin_source_compatibility.mjs ``` ## Avoid speculative defenses diff --git a/sdk/typescript/package.json b/sdk/typescript/package.json index 0b0889e7e..49c31c3a3 100644 --- a/sdk/typescript/package.json +++ b/sdk/typescript/package.json @@ -93,9 +93,11 @@ "fast-check": "4.9.0", "ink-testing-library": "4.0.0", "json-schema-to-typescript": "15.0.4", + "minimatch": "10.2.6", "postcss": "8.5.23", "prettier": "3.2.5", "react-dom": "19.2.4", + "saxes": "6.0.0", "tailwindcss": "4.3.3", "typescript": "5.7.3" } diff --git a/sdk/typescript/pnpm-lock.yaml b/sdk/typescript/pnpm-lock.yaml index eed94b772..b38d32fc6 100644 --- a/sdk/typescript/pnpm-lock.yaml +++ b/sdk/typescript/pnpm-lock.yaml @@ -99,6 +99,9 @@ importers: json-schema-to-typescript: specifier: 15.0.4 version: 15.0.4 + minimatch: + specifier: 10.2.6 + version: 10.2.6 postcss: specifier: 8.5.23 version: 8.5.23 @@ -108,6 +111,9 @@ importers: react-dom: specifier: 19.2.4 version: 19.2.4(react@19.2.4) + saxes: + specifier: 6.0.0 + version: 6.0.0 tailwindcss: specifier: 4.3.3 version: 4.3.3 @@ -2682,6 +2688,10 @@ packages: safer-buffer@2.1.2: resolution: {integrity: sha512-YZo3K82SD7Riyi0E1EQPojLz7kpepnSQI9IyPbHHg1XXXevb5dJI7tpyN2ADxGcQbHG7vcyRHk0cbwqcQriUtg==} + saxes@6.0.0: + resolution: {integrity: sha512-xAg7SOnEhrm5zI3puOOKyy1OMcMlIJZYNJY7xLBwSze0UjhPLnWfj2GF2EpT0jmzaJKIWKHLsaSSajf35bcYnA==} + engines: {node: '>=v12.22.7'} + scheduler@0.27.0: resolution: {integrity: sha512-eNv+WrVbKu1f3vbYJT/xtiF5syA5HPIMtf9IgY/nKg0sWqzAUEvqY/xm7OcZc/qafLx/iO9FgOmeSAp4v5ti/Q==} @@ -2946,6 +2956,9 @@ packages: utf-8-validate: optional: true + xmlchars@2.2.0: + resolution: {integrity: sha512-JZnDKK8B0RCDw84FNdDAIpZK+JuJw+s7Lz8nksI7SIuU3UXJJslUthsi+uWBUYOwPFwW7W7PRLRfUKpxjtjFCw==} + yallist@3.1.1: resolution: {integrity: sha512-a4UGQaWPH59mOXUYnAG2ewncQS4i4F43Tv3JoAM+s2VDAmS9NsK8GpDMLrCHPksFT7h3K6TOoUNn2pb7RoXx4g==} @@ -5897,6 +5910,10 @@ snapshots: safer-buffer@2.1.2: {} + saxes@6.0.0: + dependencies: + xmlchars: 2.2.0 + scheduler@0.27.0: {} semver@6.3.1: {} @@ -6145,6 +6162,8 @@ snapshots: ws@8.21.3: {} + xmlchars@2.2.0: {} + yallist@3.1.1: {} yaml@2.9.0: {} diff --git a/sdk/typescript/scripts/compare-test-reports.mjs b/sdk/typescript/scripts/compare-test-reports.mjs new file mode 100644 index 000000000..faa02ef4a --- /dev/null +++ b/sdk/typescript/scripts/compare-test-reports.mjs @@ -0,0 +1,239 @@ +// Compare Bun JUnit inventories before changing the required CI runner. +import { lstatSync, readFileSync, readdirSync, statSync } from "node:fs"; +import { basename, dirname, join, sep } from "node:path"; +import { parseArgs } from "node:util"; +import { minimatch } from "minimatch"; +import { SaxesParser } from "saxes"; + +function matchingReports(pattern) { + // Python glob treats ** as one component and does not expand braces/extglobs. + const directoriesOnly = + pattern.endsWith(sep) || + (process.platform === "win32" && pattern.endsWith("/")); + if (!/[*?[]/u.test(pattern)) { + try { + const stat = directoriesOnly ? statSync(pattern) : lstatSync(pattern); + return !directoriesOnly || stat.isDirectory() ? [pattern] : []; + } catch { + return []; + } + } + const parent = dirname(pattern); + const namePattern = basename(pattern); + const directories = /[*?[]/u.test(parent) + ? matchingReports(parent) + : [parent]; + return directories.flatMap((directory) => { + let names; + try { + names = readdirSync(directory); + } catch { + return []; + } + return names + .filter( + (name) => + (!name.startsWith(".") || namePattern.startsWith(".")) && + minimatch( + name, + namePattern.replaceAll("\\", "\\\\").replaceAll("[^", "[\\^"), + { + dot: true, + nobrace: true, + noext: true, + noglobstar: true, + nonegate: true, + nocomment: true, + nocase: process.platform === "win32", + }, + ), + ) + .map((name) => join(directory, name)) + .filter((path) => { + if (!directoriesOnly) return true; + try { + return statSync(path).isDirectory(); + } catch { + return false; + } + }); + }); +} + +function integer(value) { + if (!/^[+-]?\d(?:_?\d)*$/u.test(value.trim())) { + throw new Error(`invalid integer: ${value}`); + } + return BigInt(value.replaceAll("_", "").trim()); +} + +function seconds(value) { + const number = value.trim().replaceAll(/(?<=\d)_(?=\d)/gu, ""); + if ( + !/^[+-]?(?:(?:\d+(?:\.\d*)?|\.\d+)(?:e[+-]?\d+)?|inf(?:inity)?|nan)$/iu.test( + number, + ) + ) { + throw new Error(`invalid duration: ${value}`); + } + return Number(number.replace(/inf(?:inity)?/iu, "Infinity")); +} + +function readReport(path) { + const parser = new SaxesParser({ xmlns: true, fileName: path }); + let root; + const stack = []; + const records = []; + const suites = []; + parser.on("opentag", (node) => { + root ??= node; + const name = node.uri ? `{${node.uri}}${node.local}` : node.local; + const parent = stack.at(-1); + if (parent?.record) { + if (name === "failure" || name === "error") + parent.record.status = "failed"; + else if (name === "skipped" && parent.record.status === "passed") + parent.record.status = "skipped"; + } + let record; + if (name === "testcase") { + record = { attributes: node.attributes, status: "passed" }; + records.push(record); + } + if (name === "testsuite" || name === "testsuites") suites.push(node); + stack.push({ record }); + }); + parser.on("closetag", () => stack.pop()); + const content = new TextDecoder("utf-8", { fatal: true }).decode( + readFileSync(path), + ); + parser.write(content).close(); + + const cases = new Map(); + for (const { attributes, status } of records) { + const identity = [ + (attributes.file?.value ?? "") + .replaceAll("\\", "/") + .replace(/^\.\//u, ""), + attributes.classname?.value ?? "", + attributes.name?.value ?? "", + ]; + const key = JSON.stringify(identity); + if (cases.has(key)) + throw new Error( + `${path}: duplicate test identity: ${identity.join(" > ")}`, + ); + cases.set(key, status); + } + if (!cases.size) throw new Error(`${path}: no test cases`); + if ( + integer(root.attributes.tests?.value ?? String(cases.size)) !== + BigInt(cases.size) + ) { + throw new Error(`${path}: reported test count does not match test cases`); + } + const failed = + [...cases.values()].includes("failed") || + suites.some((node) => + ["failures", "errors"].some( + (field) => integer(node.attributes[field]?.value ?? "0") !== 0n, + ), + ); + if (failed) console.error(`${path}: test run failed`); + const duration = seconds(root.attributes.time?.value ?? "0"); + const skipped = [...cases.values()].filter( + (status) => status === "skipped", + ).length; + console.log( + `| ${basename(path)} | ${cases.size} | ${skipped} | ${duration.toFixed(2)} |`, + ); + return { + cases: new Map( + [...cases].map(([identity, status]) => [ + JSON.stringify([...JSON.parse(identity), status]), + 1, + ]), + ), + duration, + failed, + }; +} + +function main() { + let args; + try { + args = parseArgs({ + options: { help: { type: "boolean", short: "h" } }, + allowPositionals: true, + }); + if (!args.values.help && args.positionals.length < 2) + throw new Error( + "a baseline and at least one candidate report are required", + ); + } catch (error) { + console.error(error.message); + return 2; + } + if (args.values.help) { + console.log( + "Compare Bun JUnit inventories before changing the required CI runner.\n\nUsage: node compare-test-reports.mjs baseline candidates [candidates ...]\nCandidates are JUnit files or glob patterns.", + ); + return 0; + } + console.log("| Report | Cases | Skipped | Seconds |"); + console.log("| --- | ---: | ---: | ---: |"); + const baseline = readReport(args.positionals[0]); + let failed = baseline.failed; + const candidates = new Map(); + const durations = []; + for (const pattern of args.positionals.slice(1)) { + const paths = matchingReports(pattern).sort((a, b) => + Buffer.compare(Buffer.from(a), Buffer.from(b)), + ); + if (!paths.length) throw new Error(`No reports match ${pattern}`); + for (const path of paths) { + const report = readReport(path); + failed ||= report.failed; + for (const [identity, count] of report.cases) + candidates.set(identity, (candidates.get(identity) ?? 0) + count); + durations.push(report.duration); + } + } + for (const [label, left, right] of [ + ["Missing", baseline.cases, candidates], + ["Extra", candidates, baseline.cases], + ]) { + const differences = [...left] + .map(([identity, count]) => [ + JSON.parse(identity), + count - (right.get(identity) ?? 0), + ]) + .filter(([, count]) => count > 0) + .sort(([a], [b]) => { + for (let index = 0; index < a.length; index++) { + const order = Buffer.compare( + Buffer.from(a[index]), + Buffer.from(b[index]), + ); + if (order) return order; + } + return 0; + }); + for (const [identity, count] of differences) { + failed = true; + console.error(`${label} (${count}): ${identity.join(" > ")}`); + } + } + if (failed) return 1; + console.log( + `\nIdentical test inventory and outcomes. Slowest candidate: ${Math.max(...durations).toFixed(2)}s; combined test time: ${durations.reduce((sum, duration) => sum + duration, 0).toFixed(2)}s.\n`, + ); + return 0; +} + +try { + process.exitCode = main(); +} catch (error) { + console.error(error.message); + process.exitCode = 1; +} diff --git a/sdk/typescript/scripts/compare-test-reports.py b/sdk/typescript/scripts/compare-test-reports.py deleted file mode 100644 index 6f735eed4..000000000 --- a/sdk/typescript/scripts/compare-test-reports.py +++ /dev/null @@ -1,84 +0,0 @@ -"""Compare Bun JUnit inventories before changing the required CI runner.""" - -import argparse -from collections import Counter -from glob import glob -from pathlib import Path -import sys -import xml.etree.ElementTree as ET - - -def read_report(path: Path) -> tuple[Counter, float, bool]: - root = ET.parse(path).getroot() - cases = {} - for case in root.iter("testcase"): - status = "passed" - if case.find("skipped") is not None: - status = "skipped" - if case.find("failure") is not None or case.find("error") is not None: - status = "failed" - identity = ( - case.get("file", "").replace("\\", "/").removeprefix("./"), - case.get("classname", ""), - case.get("name", ""), - ) - if identity in cases: - raise ValueError(f"{path}: duplicate test identity: {' > '.join(identity)}") - cases[identity] = status - if not cases: - raise ValueError(f"{path}: no test cases") - if int(root.get("tests", str(len(cases)))) != len(cases): - raise ValueError(f"{path}: reported test count does not match test cases") - failed = "failed" in cases.values() or any( - int(node.get(field, "0")) - for node in root.iter() - if node.tag in ("testsuite", "testsuites") - for field in ("failures", "errors") - ) - if failed: - print(f"{path}: test run failed", file=sys.stderr) - seconds = float(root.get("time", "0")) - skipped = sum(status == "skipped" for status in cases.values()) - print(f"| {path.name} | {len(cases)} | {skipped} | {seconds:.2f} |") - return ( - Counter((*identity, status) for identity, status in cases.items()), - seconds, - failed, - ) - - -def main() -> int: - parser = argparse.ArgumentParser(description=__doc__) - parser.add_argument("baseline", type=Path) - parser.add_argument("candidates", nargs="+", help="JUnit files or glob patterns") - args = parser.parse_args() - print("| Report | Cases | Skipped | Seconds |") - print("| --- | ---: | ---: | ---: |") - baseline, _, failed = read_report(args.baseline) - candidates = Counter() - durations = [] - for pattern in args.candidates: - paths = sorted(glob(pattern)) - if not paths: - raise ValueError(f"No reports match {pattern}") - for path in paths: - cases, seconds, report_failed = read_report(Path(path)) - failed = failed or report_failed - candidates.update(cases) - durations.append(seconds) - missing, extra = baseline - candidates, candidates - baseline - if failed or missing or extra: - for label, difference in (("Missing", missing), ("Extra", extra)): - for identity, count in sorted(difference.items()): - print(f"{label} ({count}): {' > '.join(identity)}", file=sys.stderr) - return 1 - print(f"\nIdentical test inventory and outcomes. Slowest candidate: {max(durations):.2f}s; combined test time: {sum(durations):.2f}s.\n") - return 0 - - -if __name__ == "__main__": - try: - sys.exit(main()) - except (OSError, ValueError, ET.ParseError) as error: - print(error, file=sys.stderr) - sys.exit(1) diff --git a/sdk/typescript/tests-ts/release-automation.test.ts b/sdk/typescript/tests-ts/release-automation.test.ts index 2b6dde7ab..691076aeb 100644 --- a/sdk/typescript/tests-ts/release-automation.test.ts +++ b/sdk/typescript/tests-ts/release-automation.test.ts @@ -4173,10 +4173,10 @@ describe("GitHub release workflow safeguards", () => { mkdirSync(scripts, { recursive: true }); mkdirSync(pluginRoot, { recursive: true }); writeFileSync( - join(scripts, "check_plugin_source_compatibility.py"), + join(scripts, "check_plugin_source_compatibility.mjs"), readFileSync( new URL( - "../../../.github/scripts/check_plugin_source_compatibility.py", + "../../../.github/scripts/check_plugin_source_compatibility.mjs", import.meta.url, ), ), @@ -4188,7 +4188,7 @@ describe("GitHub release workflow safeguards", () => { workspace, "add", "--", - ".github/scripts/check_plugin_source_compatibility.py", + ".github/scripts/check_plugin_source_compatibility.mjs", "plugins/codex-security/README.md", ]); try { diff --git a/sdk/typescript/tests-ts/test-reports.test.ts b/sdk/typescript/tests-ts/test-reports.test.ts index d85fba7bf..746bf26e5 100644 --- a/sdk/typescript/tests-ts/test-reports.test.ts +++ b/sdk/typescript/tests-ts/test-reports.test.ts @@ -1,4 +1,4 @@ -import { mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { fileURLToPath } from "node:url"; @@ -41,15 +41,11 @@ function testcase(name: string, status = "") { } async function compare(baseline: string, ...candidates: string[]) { - const python = Bun.which("python3") ?? Bun.which("python") ?? Bun.which("py"); - if (python === null) throw new Error("A Python interpreter is required."); const child = Bun.spawn({ cmd: [ - python, - "-I", - "-B", + "node", fileURLToPath( - new URL("../scripts/compare-test-reports.py", import.meta.url), + new URL("../scripts/compare-test-reports.mjs", import.meta.url), ), baseline, ...candidates, @@ -88,7 +84,7 @@ describe("JUnit inventory comparison", () => { ), "reports/runner-windows-latest-shard-*.xml", ]; - const mock = `python3() { + const mock = `node() { printf '%s\\n' "$3" [[ "$3" != "$CODEX_SECURITY_TEST_FAIL_REPORT" ]] }`; @@ -128,9 +124,145 @@ describe("JUnit inventory comparison", () => { await fixture.report("shard-2.xml", [passed]); const result = await compare(baseline, join(fixture.root, "shard-*.xml")); expect(result.status, result.stderr).toBe(0); + expect(result.stderr).toBe(""); expect(result.stdout).toContain("combined test time: 2.50s"); }); + test("parses XML entities while ignoring comments and CDATA markup", async () => { + const fixture = await fixtures(); + const baseline = await fixture.report("baseline.xml", [ + testcase("a & " ' < >"), + ]); + const candidate = join(fixture.root, "candidate.xml"); + await writeFile( + candidate, + ` + + +]]> +${testcase("a & " ' < >")} +`, + ); + const result = await compare(baseline, candidate); + expect(result.status, result.stderr).toBe(0); + expect(result.stderr).toBe(""); + }); + + test("normalizes report paths and rejects duplicates across candidate files", async () => { + const fixture = await fixtures(); + const original = testcase("portable"); + const baseline = await fixture.report("baseline.xml", [original]); + const candidate = await fixture.report("candidate.xml", [ + original.replace( + "tests-ts/example.test.ts", + "./tests-ts\\example.test.ts", + ), + ]); + expect((await compare(baseline, candidate)).status).toBe(0); + const result = await compare(baseline, candidate, candidate); + expect(result.status).toBe(1); + expect(result.stderr).toContain( + "Extra (1): tests-ts/example.test.ts > example > portable > passed", + ); + }); + + test("preserves file, character-class, hidden-file, and nonrecursive glob matching", async () => { + const fixture = await fixtures(); + const cases = [testcase("portable")]; + const baseline = await fixture.report("baseline.xml", cases); + const directory = join(fixture.root, "reports with spaces"); + await mkdir(directory); + await fixture.report("reports with spaces/shard-a.xml", cases); + await fixture.report("reports with spaces/.hidden.xml", [ + testcase("hidden"), + ]); + await mkdir(join(directory, "nested")); + await fixture.report("reports with spaces/nested/shard-b.xml", [ + testcase("nested"), + ]); + for (const pattern of [ + "reports with spaces/shard-?.xml", + "reports with spaces/shard-[ab].xml", + "reports with spaces/shard-[!b].xml", + "reports with spaces/*.xml", + "**/shard-*.xml", + ]) { + const result = await compare(baseline, join(fixture.root, pattern)); + expect(result.status, `${pattern}: ${result.stderr}`).toBe(0); + } + const hidden = await fixture.report("hidden-baseline.xml", [ + testcase("hidden"), + ]); + expect((await compare(hidden, join(directory, ".*.xml"))).status).toBe(0); + for (const name of ["{a,b}.xml", "!report.xml", "@(shard).xml"]) { + const path = await fixture.report(name, cases); + expect((await compare(baseline, path)).status, name).toBe(0); + } + }); + + test("detects error status and nested summary failures with failure overriding skipped", async () => { + const fixture = await fixtures(); + const baseline = await fixture.report("baseline.xml", [ + testcase("example"), + ]); + for (const status of ["", ""]) { + const candidate = await fixture.report("error.xml", [ + testcase("example", status), + ]); + const result = await compare(baseline, candidate); + expect(result.status).toBe(1); + expect(result.stderr).toContain("test run failed"); + expect(result.stderr).toContain("example > example > failed"); + expect(result.stderr).not.toContain("example > example > skipped"); + } + const nested = join(fixture.root, "nested.xml"); + await writeFile( + nested, + `${testcase("example")}`, + ); + const result = await compare(baseline, nested); + expect(result.status).toBe(1); + expect(result.stderr).toContain("test run failed"); + }); + + test("rejects malformed XML and invalid report numbers", async () => { + const fixture = await fixtures(); + const baseline = await fixture.report("baseline.xml", [ + testcase("example"), + ]); + for (const xml of [ + "", + "", + '', + '', + "", + '', + '', + '', + ]) { + const candidate = join(fixture.root, "invalid.xml"); + await writeFile(candidate, xml); + const result = await compare(baseline, candidate); + expect(result.status, xml).toBe(1); + expect(result.stdout).not.toContain("Identical test inventory"); + } + }); + + test("uses count and timing defaults and only direct unqualified status children", async () => { + const fixture = await fixtures(); + const baseline = await fixture.report("baseline.xml", [ + testcase("example"), + ]); + const candidate = join(fixture.root, "defaults.xml"); + await writeFile( + candidate, + `${testcase("example", "")}`, + ); + const result = await compare(baseline, candidate); + expect(result.status, result.stderr).toBe(0); + expect(result.stdout).toContain("| defaults.xml | 1 | 0 | 0.00 |"); + }); + test("rejects ambiguous test identities even when totals match", async () => { const fixture = await fixtures(); const first = testcase("same parameterized name"); From e374e6543ce09adaebb78389762fa46c02fe0a39 Mon Sep 17 00:00:00 2001 From: Kyle Brown Date: Mon, 31 Aug 2026 23:16:47 +0000 Subject: [PATCH 2/2] refactor(ci): use TypeScript for migrated checks --- .gitattributes | 1 + ... => check_plugin_source_compatibility.mts} | 34 ++++---- ...est_check_plugin_source_compatibility.mts} | 46 +++++++---- .github/workflows/node-ci.yml | 6 +- .github/workflows/test-quality.yml | 4 +- AGENTS.md | 4 +- sdk/typescript/package.json | 2 +- ...t-reports.mjs => compare-test-reports.mts} | 77 +++++++++++-------- .../tests-ts/release-automation.test.ts | 6 +- sdk/typescript/tests-ts/test-reports.test.ts | 17 +++- sdk/typescript/tsconfig.json | 2 + 11 files changed, 120 insertions(+), 79 deletions(-) rename .github/scripts/{check_plugin_source_compatibility.mjs => check_plugin_source_compatibility.mts} (82%) rename .github/scripts/{test_check_plugin_source_compatibility.mjs => test_check_plugin_source_compatibility.mts} (83%) rename sdk/typescript/scripts/{compare-test-reports.mjs => compare-test-reports.mts} (76%) diff --git a/.gitattributes b/.gitattributes index a411474ea..e9a5ecc5b 100644 --- a/.gitattributes +++ b/.gitattributes @@ -3,6 +3,7 @@ *.json text eol=lf *.md text eol=lf *.mjs text eol=lf +*.mts text eol=lf *.py text eol=lf *.sh text eol=lf *.ts text eol=lf diff --git a/.github/scripts/check_plugin_source_compatibility.mjs b/.github/scripts/check_plugin_source_compatibility.mts similarity index 82% rename from .github/scripts/check_plugin_source_compatibility.mjs rename to .github/scripts/check_plugin_source_compatibility.mts index 0b7b9cefe..73f2a9fb7 100644 --- a/.github/scripts/check_plugin_source_compatibility.mjs +++ b/.github/scripts/check_plugin_source_compatibility.mts @@ -1,4 +1,4 @@ -#!/usr/bin/env node +#!/usr/bin/env -S node --experimental-strip-types --disable-warning=ExperimentalWarning // Check that tracked plugin source stays portable across repository imports. import { spawnSync } from "node:child_process"; @@ -22,7 +22,7 @@ const HTML_BLOCK = /^\s*<\/?[A-Za-z][^>]*>\s*$/u; const NATURAL_LINE_ENDINGS = new Set("\\.?!:;。!?:;)]}'\"`>"); const utf8 = new TextDecoder("utf-8", { fatal: true, ignoreBOM: true }); -function trackedFiles(pluginRoot) { +function trackedFiles(pluginRoot: string): string[] { const result = spawnSync( "git", ["-C", pluginRoot, "ls-files", "-z", "--", "."], @@ -37,7 +37,7 @@ function trackedFiles(pluginRoot) { return utf8.decode(result.stdout).split("\0").filter(Boolean); } -function isMarkdownStructure(line) { +function isMarkdownStructure(line: string): boolean { const stripped = line.trim(); return ( !stripped || @@ -51,24 +51,24 @@ function isMarkdownStructure(line) { ); } -function lineEndsNaturally(line) { +function lineEndsNaturally(line: string): boolean { const stripped = line.trimEnd(); return ( line.endsWith(" ") || - NATURAL_LINE_ENDINGS.has(stripped.at(-1)) || + NATURAL_LINE_ENDINGS.has(stripped.slice(-1)) || /https?:\/\/\S+$/u.test(stripped) ); } -function hardWrappedLines(content) { +function hardWrappedLines(content: string): number[] { const lines = content.split( /\r\n|[\n\r\v\f\u001c-\u001e\u0085\u2028\u2029]/u, ); - const offenders = []; + const offenders: number[] = []; let inFence = false; let inFrontmatter = content.startsWith("---\n"); for (let index = 0; index < lines.length - 1; index++) { - const line = lines[index]; + const line = lines[index]!; const stripped = line.trim(); if (stripped.startsWith("```") || stripped.startsWith("~~~")) { inFence = !inFence; @@ -79,7 +79,7 @@ function hardWrappedLines(content) { continue; } if (inFence || inFrontmatter) continue; - const followingLine = lines[index + 1]; + const followingLine = lines[index + 1]!; if (isMarkdownStructure(line) || isMarkdownStructure(followingLine)) continue; if (LIST_ITEM.test(followingLine) || lineEndsNaturally(line)) continue; @@ -90,8 +90,8 @@ function hardWrappedLines(content) { return offenders; } -function sourceCompatibilityErrors(pluginRoot) { - const errors = []; +function sourceCompatibilityErrors(pluginRoot: string): string[] { + const errors: string[] = []; for (const relativePath of trackedFiles(pluginRoot)) { const path = join(pluginRoot, relativePath); const stat = lstatSync(path); @@ -116,8 +116,8 @@ function sourceCompatibilityErrors(pluginRoot) { return errors.sort((a, b) => Buffer.compare(Buffer.from(a), Buffer.from(b))); } -function main() { - let values; +function main(): number { + let values: { "plugin-root": string; help?: boolean }; try { ({ values } = parseArgs({ options: { @@ -131,7 +131,7 @@ function main() { }, })); } catch (error) { - console.error(error.message); + console.error((error as Error).message); return 2; } if (values.help) { @@ -139,7 +139,7 @@ function main() { "Check tracked plugin source for deterministic import compatibility.\n", ); console.log( - "Usage: node check_plugin_source_compatibility.mjs [--plugin-root PATH]", + "Usage: node --experimental-strip-types --disable-warning=ExperimentalWarning check_plugin_source_compatibility.mts [--plugin-root PATH]", ); console.log( "\n--plugin-root PATH plugin source root (default: plugins/codex-security in this repository)", @@ -153,7 +153,9 @@ function main() { return 1; } } catch (error) { - console.error(`source compatibility check failed: ${error.message}`); + console.error( + `source compatibility check failed: ${(error as Error).message}`, + ); return 2; } console.log("Plugin source compatibility checks passed."); diff --git a/.github/scripts/test_check_plugin_source_compatibility.mjs b/.github/scripts/test_check_plugin_source_compatibility.mts similarity index 83% rename from .github/scripts/test_check_plugin_source_compatibility.mjs rename to .github/scripts/test_check_plugin_source_compatibility.mts index 09c6c0e9a..7ce1f3315 100644 --- a/.github/scripts/test_check_plugin_source_compatibility.mjs +++ b/.github/scripts/test_check_plugin_source_compatibility.mts @@ -1,5 +1,5 @@ import assert from "node:assert/strict"; -import { spawnSync } from "node:child_process"; +import { spawnSync, type SpawnSyncReturns } from "node:child_process"; import { copyFileSync, mkdtempSync, @@ -15,20 +15,20 @@ import { afterEach, test } from "node:test"; import { fileURLToPath } from "node:url"; const checker = fileURLToPath( - new URL("./check_plugin_source_compatibility.mjs", import.meta.url), + new URL("./check_plugin_source_compatibility.mts", import.meta.url), ); -const directories = []; +const directories: string[] = []; afterEach(() => { for (const root of directories.splice(0)) rmSync(root, { recursive: true, force: true }); }); -function git(root, ...args) { +function git(root: string, ...args: string[]): void { const result = spawnSync("git", ["-C", root, ...args], { encoding: "utf8" }); assert.equal(result.status, 0, result.stderr); } -function fixture(files = {}) { +function fixture(files: Record = {}): string { const root = mkdtempSync(join(tmpdir(), "plugin-source-check-")); directories.push(root); git(root, "init", "--quiet"); @@ -38,8 +38,17 @@ function fixture(files = {}) { return root; } -function runChecker(...args) { - return spawnSync(process.execPath, [checker, ...args], { encoding: "utf8" }); +function runChecker(...args: string[]): SpawnSyncReturns { + return spawnSync( + process.execPath, + [ + "--experimental-strip-types", + "--disable-warning=ExperimentalWarning", + checker, + ...args, + ], + { encoding: "utf8" }, + ); } test("reports tracked source violations in stable order", () => { @@ -81,16 +90,21 @@ test("checkout preserves source size with autocrlf", () => { new URL("../../.gitattributes", import.meta.url), join(root, ".gitattributes"), ); - const source = join(root, "module.py"); - const content = Buffer.from("pass\n".repeat(30_000)); - writeFileSync(source, content); git(root, "config", "--local", "core.autocrlf", "true"); - git(root, "add", "--", ".gitattributes", "module.py"); - unlinkSync(source); - git(root, "checkout-index", "--", "module.py"); - const result = runChecker("--plugin-root", root); - assert.equal(result.status, 0, result.stderr); - assert.deepEqual(readFileSync(source), content); + for (const [name, line] of [ + ["module.py", "pass\n"], + ["module.mts", "null\n"], + ] as const) { + const source = join(root, name); + const content = Buffer.from(line.repeat(30_000)); + writeFileSync(source, content); + git(root, "add", "--", ".gitattributes", name); + unlinkSync(source); + git(root, "checkout-index", "--", name); + const result = runChecker("--plugin-root", root); + assert.equal(result.status, 0, result.stderr); + assert.deepEqual(readFileSync(source), content); + } }); test("accepts Markdown structures, natural line endings, and inline markup", () => { diff --git a/.github/workflows/node-ci.yml b/.github/workflows/node-ci.yml index 93385bf03..44ed33f1d 100644 --- a/.github/workflows/node-ci.yml +++ b/.github/workflows/node-ci.yml @@ -99,7 +99,7 @@ jobs: - name: Check plugin source compatibility if: steps.scope.outputs.ci-mode == 'markdown' run: | - node .github/scripts/check_plugin_source_compatibility.mjs + node --experimental-strip-types --disable-warning=ExperimentalWarning .github/scripts/check_plugin_source_compatibility.mts - name: Install dependencies if: steps.scope.outputs.check-markdown == 'true' @@ -382,9 +382,9 @@ jobs: run: | python -m ruff check --config plugins/codex-security/pyproject.toml plugins/codex-security python -m ruff format --check --config plugins/codex-security/pyproject.toml plugins/codex-security - node .github/scripts/check_plugin_source_compatibility.mjs + node --experimental-strip-types --disable-warning=ExperimentalWarning .github/scripts/check_plugin_source_compatibility.mts - name: Test source compatibility checker - run: node --test .github/scripts/test_check_plugin_source_compatibility.mjs + run: node --experimental-strip-types --disable-warning=ExperimentalWarning --test .github/scripts/test_check_plugin_source_compatibility.mts - name: Test Python source contracts run: python -m pytest plugins/codex-security/tests -q -n 4 --dist worksteal --max-worker-restart 0 --durations=30 --junitxml=reports/python.xml - name: Upload Python test reports diff --git a/.github/workflows/test-quality.yml b/.github/workflows/test-quality.yml index 0520fc1ec..390e4672b 100644 --- a/.github/workflows/test-quality.yml +++ b/.github/workflows/test-quality.yml @@ -134,10 +134,10 @@ jobs: comparison_status=0 for os in ubuntu-latest windows-latest; do for mode in isolated parallel; do - node sdk/typescript/scripts/compare-test-reports.mjs "reports/runner-$os-baseline.xml" "reports/runner-$os-$mode.xml" >> "$GITHUB_STEP_SUMMARY" || comparison_status=1 + node --experimental-strip-types --disable-warning=ExperimentalWarning sdk/typescript/scripts/compare-test-reports.mts "reports/runner-$os-baseline.xml" "reports/runner-$os-$mode.xml" >> "$GITHUB_STEP_SUMMARY" || comparison_status=1 done done - node sdk/typescript/scripts/compare-test-reports.mjs reports/runner-windows-latest-baseline.xml 'reports/runner-windows-latest-shard-*.xml' >> "$GITHUB_STEP_SUMMARY" || comparison_status=1 + node --experimental-strip-types --disable-warning=ExperimentalWarning sdk/typescript/scripts/compare-test-reports.mts reports/runner-windows-latest-baseline.xml 'reports/runner-windows-latest-shard-*.xml' >> "$GITHUB_STEP_SUMMARY" || comparison_status=1 exit "$comparison_status" mutation: diff --git a/AGENTS.md b/AGENTS.md index 0f3f83636..8b1697919 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -20,8 +20,8 @@ submitting the change: ```bash python -m ruff check --config plugins/codex-security/pyproject.toml plugins/codex-security python -m ruff format --check --config plugins/codex-security/pyproject.toml plugins/codex-security -node .github/scripts/check_plugin_source_compatibility.mjs -node --test .github/scripts/test_check_plugin_source_compatibility.mjs +node --experimental-strip-types --disable-warning=ExperimentalWarning .github/scripts/check_plugin_source_compatibility.mts +node --experimental-strip-types --disable-warning=ExperimentalWarning --test .github/scripts/test_check_plugin_source_compatibility.mts ``` ## Avoid speculative defenses diff --git a/sdk/typescript/package.json b/sdk/typescript/package.json index 49c31c3a3..cfa53f29a 100644 --- a/sdk/typescript/package.json +++ b/sdk/typescript/package.json @@ -47,7 +47,7 @@ "build:plugin": "node scripts/build-plugin.mjs", "check:plugin-source": "node scripts/check-plugin-source.mjs", "check:package": "node scripts/check-package.mjs", - "format": "prettier --check --ignore-path .gitignore --ignore-path .prettierignore \"**/*.{cjs,mjs,js,ts,tsx,json,md}\"", + "format": "prettier --check --ignore-path .gitignore --ignore-path .prettierignore \"**/*.{cjs,mjs,js,ts,mts,tsx,json,md}\" \"../../.github/scripts/*.mts\"", "generate:models": "node scripts/generate-models.cjs", "generate:models:check": "node scripts/generate-models.cjs --check", "lint": "tsc --noEmit", diff --git a/sdk/typescript/scripts/compare-test-reports.mjs b/sdk/typescript/scripts/compare-test-reports.mts similarity index 76% rename from sdk/typescript/scripts/compare-test-reports.mjs rename to sdk/typescript/scripts/compare-test-reports.mts index faa02ef4a..e5eb505d5 100644 --- a/sdk/typescript/scripts/compare-test-reports.mjs +++ b/sdk/typescript/scripts/compare-test-reports.mts @@ -3,9 +3,22 @@ import { lstatSync, readFileSync, readdirSync, statSync } from "node:fs"; import { basename, dirname, join, sep } from "node:path"; import { parseArgs } from "node:util"; import { minimatch } from "minimatch"; -import { SaxesParser } from "saxes"; +import { SaxesParser, type SaxesTagNS } from "saxes"; -function matchingReports(pattern) { +type TestStatus = "passed" | "skipped" | "failed"; +type TestIdentity = [file: string, classname: string, name: string]; +type TestOutcome = [...TestIdentity, status: TestStatus]; +type TestRecord = { + attributes: SaxesTagNS["attributes"]; + status: TestStatus; +}; +type TestReport = { + cases: Map; + duration: number; + failed: boolean; +}; + +function matchingReports(pattern: string): string[] { // Python glob treats ** as one component and does not expand braces/extglobs. const directoriesOnly = pattern.endsWith(sep) || @@ -24,7 +37,7 @@ function matchingReports(pattern) { ? matchingReports(parent) : [parent]; return directories.flatMap((directory) => { - let names; + let names: string[]; try { names = readdirSync(directory); } catch { @@ -60,14 +73,14 @@ function matchingReports(pattern) { }); } -function integer(value) { +function integer(value: string): bigint { if (!/^[+-]?\d(?:_?\d)*$/u.test(value.trim())) { throw new Error(`invalid integer: ${value}`); } return BigInt(value.replaceAll("_", "").trim()); } -function seconds(value) { +function seconds(value: string): number { const number = value.trim().replaceAll(/(?<=\d)_(?=\d)/gu, ""); if ( !/^[+-]?(?:(?:\d+(?:\.\d*)?|\.\d+)(?:e[+-]?\d+)?|inf(?:inity)?|nan)$/iu.test( @@ -79,12 +92,12 @@ function seconds(value) { return Number(number.replace(/inf(?:inity)?/iu, "Infinity")); } -function readReport(path) { +function readReport(path: string): TestReport { const parser = new SaxesParser({ xmlns: true, fileName: path }); - let root; - const stack = []; - const records = []; - const suites = []; + let root: SaxesTagNS | undefined; + const stack: Array<{ record: TestRecord | undefined }> = []; + const records: TestRecord[] = []; + const suites: SaxesTagNS[] = []; parser.on("opentag", (node) => { root ??= node; const name = node.uri ? `{${node.uri}}${node.local}` : node.local; @@ -95,7 +108,7 @@ function readReport(path) { else if (name === "skipped" && parent.record.status === "passed") parent.record.status = "skipped"; } - let record; + let record: TestRecord | undefined; if (name === "testcase") { record = { attributes: node.attributes, status: "passed" }; records.push(record); @@ -109,14 +122,14 @@ function readReport(path) { ); parser.write(content).close(); - const cases = new Map(); + const cases = new Map(); for (const { attributes, status } of records) { - const identity = [ - (attributes.file?.value ?? "") + const identity: TestIdentity = [ + (attributes["file"]?.value ?? "") .replaceAll("\\", "/") .replace(/^\.\//u, ""), - attributes.classname?.value ?? "", - attributes.name?.value ?? "", + attributes["classname"]?.value ?? "", + attributes["name"]?.value ?? "", ]; const key = JSON.stringify(identity); if (cases.has(key)) @@ -127,7 +140,7 @@ function readReport(path) { } if (!cases.size) throw new Error(`${path}: no test cases`); if ( - integer(root.attributes.tests?.value ?? String(cases.size)) !== + integer(root!.attributes["tests"]?.value ?? String(cases.size)) !== BigInt(cases.size) ) { throw new Error(`${path}: reported test count does not match test cases`); @@ -140,7 +153,7 @@ function readReport(path) { ), ); if (failed) console.error(`${path}: test run failed`); - const duration = seconds(root.attributes.time?.value ?? "0"); + const duration = seconds(root!.attributes["time"]?.value ?? "0"); const skipped = [...cases.values()].filter( (status) => status === "skipped", ).length; @@ -150,7 +163,7 @@ function readReport(path) { return { cases: new Map( [...cases].map(([identity, status]) => [ - JSON.stringify([...JSON.parse(identity), status]), + JSON.stringify([...(JSON.parse(identity) as TestIdentity), status]), 1, ]), ), @@ -159,8 +172,8 @@ function readReport(path) { }; } -function main() { - let args; +function main(): number { + let args: { values: { help?: boolean }; positionals: string[] }; try { args = parseArgs({ options: { help: { type: "boolean", short: "h" } }, @@ -171,21 +184,21 @@ function main() { "a baseline and at least one candidate report are required", ); } catch (error) { - console.error(error.message); + console.error((error as Error).message); return 2; } if (args.values.help) { console.log( - "Compare Bun JUnit inventories before changing the required CI runner.\n\nUsage: node compare-test-reports.mjs baseline candidates [candidates ...]\nCandidates are JUnit files or glob patterns.", + "Compare Bun JUnit inventories before changing the required CI runner.\n\nUsage: node --experimental-strip-types --disable-warning=ExperimentalWarning compare-test-reports.mts baseline candidates [candidates ...]\nCandidates are JUnit files or glob patterns.", ); return 0; } console.log("| Report | Cases | Skipped | Seconds |"); console.log("| --- | ---: | ---: | ---: |"); - const baseline = readReport(args.positionals[0]); + const baseline = readReport(args.positionals[0]!); let failed = baseline.failed; - const candidates = new Map(); - const durations = []; + const candidates = new Map(); + const durations: number[] = []; for (const pattern of args.positionals.slice(1)) { const paths = matchingReports(pattern).sort((a, b) => Buffer.compare(Buffer.from(a), Buffer.from(b)), @@ -202,18 +215,18 @@ function main() { for (const [label, left, right] of [ ["Missing", baseline.cases, candidates], ["Extra", candidates, baseline.cases], - ]) { + ] as const) { const differences = [...left] - .map(([identity, count]) => [ - JSON.parse(identity), + .map(([identity, count]): [TestOutcome, number] => [ + JSON.parse(identity) as TestOutcome, count - (right.get(identity) ?? 0), ]) .filter(([, count]) => count > 0) .sort(([a], [b]) => { for (let index = 0; index < a.length; index++) { const order = Buffer.compare( - Buffer.from(a[index]), - Buffer.from(b[index]), + Buffer.from(a[index]!), + Buffer.from(b[index]!), ); if (order) return order; } @@ -234,6 +247,6 @@ function main() { try { process.exitCode = main(); } catch (error) { - console.error(error.message); + console.error((error as Error).message); process.exitCode = 1; } diff --git a/sdk/typescript/tests-ts/release-automation.test.ts b/sdk/typescript/tests-ts/release-automation.test.ts index 691076aeb..528c414f3 100644 --- a/sdk/typescript/tests-ts/release-automation.test.ts +++ b/sdk/typescript/tests-ts/release-automation.test.ts @@ -4173,10 +4173,10 @@ describe("GitHub release workflow safeguards", () => { mkdirSync(scripts, { recursive: true }); mkdirSync(pluginRoot, { recursive: true }); writeFileSync( - join(scripts, "check_plugin_source_compatibility.mjs"), + join(scripts, "check_plugin_source_compatibility.mts"), readFileSync( new URL( - "../../../.github/scripts/check_plugin_source_compatibility.mjs", + "../../../.github/scripts/check_plugin_source_compatibility.mts", import.meta.url, ), ), @@ -4188,7 +4188,7 @@ describe("GitHub release workflow safeguards", () => { workspace, "add", "--", - ".github/scripts/check_plugin_source_compatibility.mjs", + ".github/scripts/check_plugin_source_compatibility.mts", "plugins/codex-security/README.md", ]); try { diff --git a/sdk/typescript/tests-ts/test-reports.test.ts b/sdk/typescript/tests-ts/test-reports.test.ts index 746bf26e5..01e1adb59 100644 --- a/sdk/typescript/tests-ts/test-reports.test.ts +++ b/sdk/typescript/tests-ts/test-reports.test.ts @@ -44,8 +44,10 @@ async function compare(baseline: string, ...candidates: string[]) { const child = Bun.spawn({ cmd: [ "node", + "--experimental-strip-types", + "--disable-warning=ExperimentalWarning", fileURLToPath( - new URL("../scripts/compare-test-reports.mjs", import.meta.url), + new URL("../scripts/compare-test-reports.mts", import.meta.url), ), baseline, ...candidates, @@ -85,8 +87,8 @@ describe("JUnit inventory comparison", () => { "reports/runner-windows-latest-shard-*.xml", ]; const mock = `node() { - printf '%s\\n' "$3" - [[ "$3" != "$CODEX_SECURITY_TEST_FAIL_REPORT" ]] + printf '%s\\n' "$5" + [[ "$5" != "$CODEX_SECURITY_TEST_FAIL_REPORT" ]] }`; const summary = join(fixture.root, "summary.md"); for (const failedReport of ["", expected[0]!]) { @@ -125,7 +127,14 @@ describe("JUnit inventory comparison", () => { const result = await compare(baseline, join(fixture.root, "shard-*.xml")); expect(result.status, result.stderr).toBe(0); expect(result.stderr).toBe(""); - expect(result.stdout).toContain("combined test time: 2.50s"); + expect(result.stdout).toBe( + "| Report | Cases | Skipped | Seconds |\n" + + "| --- | ---: | ---: | ---: |\n" + + "| baseline.xml | 2 | 1 | 1.25 |\n" + + "| shard-1.xml | 1 | 1 | 1.25 |\n" + + "| shard-2.xml | 1 | 0 | 1.25 |\n" + + "\nIdentical test inventory and outcomes. Slowest candidate: 1.25s; combined test time: 2.50s.\n\n", + ); }); test("parses XML entities while ignoring comments and CDATA markup", async () => { diff --git a/sdk/typescript/tsconfig.json b/sdk/typescript/tsconfig.json index 18454c78e..6738b4dcf 100644 --- a/sdk/typescript/tsconfig.json +++ b/sdk/typescript/tsconfig.json @@ -5,6 +5,8 @@ "dashboard/**/*.ts", "dashboard/**/*.tsx", "tests-ts/**/*.ts", + "../../.github/scripts/*.mts", + "scripts/compare-test-reports.mts", "scripts/smoke-findings-service.ts", "scripts/fixtures/findings-service-sqlite.ts", "scripts/fixtures/prepare-runner-scan.ts"