diff --git a/.c8rc.json b/.c8rc.json index f0583ad..c047c62 100644 --- a/.c8rc.json +++ b/.c8rc.json @@ -3,6 +3,7 @@ "include": [ "scripts/cli-args.mjs", "scripts/build-diff-data.mjs", + "scripts/agent-note-output.mjs", "scripts/generate-summaries.mjs", "scripts/present.mjs", "scripts/serve-built.mjs" diff --git a/.github/workflows/product-gate.yml b/.github/workflows/product-gate.yml index be62284..bc72217 100644 --- a/.github/workflows/product-gate.yml +++ b/.github/workflows/product-gate.yml @@ -8,12 +8,8 @@ permissions: jobs: product-gate: - name: Product gate (Node ${{ matrix.node-version }}) + name: Product gate (Node 24.20.0) runs-on: ubuntu-latest - strategy: - fail-fast: false - matrix: - node-version: [22.13.0, 24.x] steps: - name: Check out repository uses: actions/checkout@v6 @@ -22,7 +18,7 @@ jobs: - name: Set up Node uses: actions/setup-node@v6 with: - node-version: ${{ matrix.node-version }} + node-version: '24.20.0' cache: pnpm cache-dependency-path: pnpm-lock.yaml - name: Install lockfile dependencies diff --git a/.github/workflows/test-lanes.yml b/.github/workflows/test-lanes.yml index ea53dc1..69c6648 100644 --- a/.github/workflows/test-lanes.yml +++ b/.github/workflows/test-lanes.yml @@ -49,26 +49,7 @@ jobs: cache-dependency-path: pnpm-lock.yaml - name: Install dependencies run: pnpm install --frozen-lockfile - - name: Run Git and server tests - run: pnpm run test:integration - - coverage: - if: github.event_name != 'schedule' - runs-on: ubuntu-latest - steps: - - name: Check out repository - uses: actions/checkout@v6 - - name: Set up pnpm - uses: pnpm/action-setup@v4 - - name: Set up Node - uses: actions/setup-node@v6 - with: - node-version: 22.13.0 - cache: pnpm - cache-dependency-path: pnpm-lock.yaml - - name: Install dependencies - run: pnpm install --frozen-lockfile - - name: Check core coverage + - name: Run core tests with coverage run: pnpm run test:coverage - name: Upload coverage report if: always() diff --git a/docs/content/development.mdx b/docs/content/development.mdx index 3657da9..394166b 100644 --- a/docs/content/development.mdx +++ b/docs/content/development.mdx @@ -56,6 +56,10 @@ lint, builds the app, runs the Node unit and integration tests, checks and builds the docs, then installs the packed package in a temporary folder and runs its CLI. Each stage names its failure before the command stops. +Npm release verification skips docs and saves the tarball from that same +package smoke test. Publishing uses that verified tarball without rebuilding +it. PR checks and site deployment still check and build the docs. + Install Chromium once after setup, and again when Playwright changes: ```sh @@ -85,19 +89,23 @@ corepack pnpm run test:platform | Command | What it checks | | --- | --- | | `pnpm run test:unit` | Fast parser, provider, doctor, release, config, and path checks. It does not build the app. | -| `pnpm run test:integration` | The production build plus local Git, timing, agent note, presenter, and server checks. | -| `pnpm run test:coverage` | Per-file coverage for the CLI parser, target builder, agent note writer, and presenter. | +| `pnpm run test:integration` | The production build plus local Git, agent note, presenter, and server checks. | +| `pnpm run test:coverage` | Unit and integration tests once, with per-file coverage checks. | | `pnpm run test:browser` | The review journey in Chromium. It does not download a browser. | | `pnpm run test:platform` | Host command lookup and npm launcher rules with local fake tools. | -| `pnpm test` | All five lanes in order. | +| `pnpm test` | Unit and integration tests with coverage, then browser and platform tests. | + +Coverage wraps `test:core`, which runs the unit and integration commands. It +does not repeat the integration tests. The fast unit job also runs on its own +in CI to report failures early. The coverage lane writes text, JSON summary, and LCOV reports under `coverage/`. Each covered file must keep at least 80% statement and line coverage, 60% branch coverage, and 90% function coverage. The check applies the floor to each file. -Pull request jobs run the unit, integration, coverage, and browser lanes from -a clean Ubuntu checkout. They use local Git repos, fake coding agents, and +Pull request jobs run a fast unit check, the combined core coverage check, +and browser tests from a clean Ubuntu checkout. They use local Git repos, fake coding agents, and checked-in browser data. A weekly job runs the platform lane on macOS 15 and Windows 2025. You can also start the **Test lanes** workflow by hand. @@ -284,6 +292,19 @@ same bounded canary from an authenticated checkout with: pnpm run benchmark:canary -- --runs 1 --reasoning minimal ``` +## Measure live updates + +Run the live-update benchmark by hand when changing event delivery or polling: + +```sh +pnpm run benchmark:live-update -- --mode events +pnpm run benchmark:live-update -- --mode poll +``` + +These commands report timing samples. They do not enforce a speed threshold +and are not part of the test suite. Server and browser tests check live-update +behavior and recovery. + ## Publish a release Use the GitHub Actions `Release` workflow for normal releases. Run it from @@ -355,7 +376,7 @@ corepack pnpm run release:verify ``` This command requires a clean tree and a `v` tag on `HEAD`. It runs the -full product gate, installs and tests the exact tarball in a temporary consumer, +product gate without docs, installs and tests the exact tarball in a temporary consumer, and records the commit and tarball hash in `.cache/`. Sign in to npm when needed, then publish by naming the exact version again: diff --git a/fallow-baselines/health.json b/fallow-baselines/health.json index 72e2dbb..8baeac9 100644 --- a/fallow-baselines/health.json +++ b/fallow-baselines/health.json @@ -25,6 +25,14 @@ "count": 1 } }, + "scripts/agent-note-output.mjs": { + "crap_high": { + "count": 1 + }, + "crap_moderate": { + "count": 3 + } + }, "scripts/build-diff-data.mjs": { "complexity_critical": { "count": 1 @@ -80,10 +88,10 @@ "count": 2 }, "crap_high": { - "count": 4 + "count": 3 }, "crap_moderate": { - "count": 4 + "count": 1 } }, "scripts/present.mjs": { @@ -161,4 +169,4 @@ "scripts/build-diff-data.mjs:complexity", "app/page.tsx:complexity" ] -} \ No newline at end of file +} diff --git a/package.json b/package.json index 9ecdb0e..d194038 100644 --- a/package.json +++ b/package.json @@ -30,6 +30,7 @@ "scripts/access-token.mjs", "scripts/agent-config.mjs", "scripts/agent-exclusions.mjs", + "scripts/agent-note-output.mjs", "scripts/agent-review.mjs", "scripts/agent-usage.mjs", "scripts/build-diff-data.mjs", @@ -83,16 +84,18 @@ "prepack": "pnpm run build", "setup": "pnpm install --frozen-lockfile", "setup:smoke": "node scripts/setup-smoke.mjs", - "test": "pnpm run test:unit && pnpm run test:integration && pnpm run test:coverage && pnpm run test:browser && pnpm run test:platform", - "test:unit": "node --test tests/access-token.test.mjs tests/agent-config.test.mjs tests/agent-exclusions.test.mjs tests/agent-review.test.mjs tests/agent-usage.test.mjs tests/automation-trust.test.mjs tests/cache.test.mjs tests/cli-args.test.mjs tests/cli-docs.test.mjs tests/coding-agents.test.mjs tests/data-contract.test.mjs tests/doctor.test.mjs tests/landing-demo.test.mjs tests/package-manifest.test.mjs tests/plan-ledger.test.mjs tests/presenter-runtime.test.mjs tests/product-gate.test.mjs tests/release-workflow.test.mjs tests/release.test.mjs tests/review-chat.test.mjs tests/summary-path.test.mjs tests/support-record.test.mjs tests/test-lanes.test.mjs tests/tool-profiles.test.mjs", - "test:integration": "pnpm run build && node --test --test-concurrency=1 tests/dev.test.mjs tests/generate-summaries.test.mjs tests/live-update-speed.test.mjs tests/performance-gate.test.mjs tests/present-agent.test.mjs tests/present-help.test.mjs tests/present-instances.test.mjs tests/presenter-recovery.test.mjs tests/remote-targets.test.mjs tests/rendered-html.test.mjs tests/serve-built.test.mjs tests/setup-environments.test.mjs", - "test:coverage": "pnpm run build && c8 node --test --test-concurrency=1 tests/cli-args.test.mjs tests/generate-summaries.test.mjs tests/present-agent.test.mjs tests/present-help.test.mjs tests/present-instances.test.mjs tests/remote-targets.test.mjs tests/serve-built.test.mjs", + "test": "pnpm run test:coverage && pnpm run test:browser && pnpm run test:platform", + "test:unit": "node --test tests/access-token.test.mjs tests/agent-config.test.mjs tests/agent-exclusions.test.mjs tests/agent-note-output.test.mjs tests/agent-review.test.mjs tests/agent-usage.test.mjs tests/automation-trust.test.mjs tests/cache.test.mjs tests/cli-args.test.mjs tests/cli-docs.test.mjs tests/coding-agents.test.mjs tests/data-contract.test.mjs tests/doctor.test.mjs tests/landing-demo.test.mjs tests/package-manifest.test.mjs tests/plan-ledger.test.mjs tests/presenter-runtime.test.mjs tests/product-gate.test.mjs tests/release-workflow.test.mjs tests/release.test.mjs tests/review-chat.test.mjs tests/summary-path.test.mjs tests/support-record.test.mjs tests/test-lanes.test.mjs tests/tool-profiles.test.mjs", + "test:integration": "pnpm run build && node --test --test-concurrency=1 tests/dev.test.mjs tests/generate-summaries.test.mjs tests/performance-gate.test.mjs tests/present-agent.test.mjs tests/present-help.test.mjs tests/present-instances.test.mjs tests/presenter-recovery.test.mjs tests/remote-targets.test.mjs tests/rendered-html.test.mjs tests/serve-built.test.mjs tests/setup-environments.test.mjs", + "test:coverage": "c8 pnpm run test:core", "test:browser": "pnpm run build && node --test tests/browser/*.test.mjs", "test:browser:install": "playwright install chromium", "test:cloud": "node --test tests/generate-summaries.test.mjs tests/present-agent.test.mjs", "test:platform": "node --test tests/platform-shell.test.mjs", "test:run": "node --test tests/*.test.mjs", - "lint": "tsc --noEmit && eslint ." + "lint": "tsc --noEmit && eslint .", + "test:core": "pnpm run test:unit && pnpm run test:integration", + "benchmark:live-update": "node benchmarks/live-update-speed.mjs" }, "devDependencies": { "@axe-core/playwright": "4.12.1", diff --git a/scripts/agent-note-output.mjs b/scripts/agent-note-output.mjs new file mode 100644 index 0000000..0ab705a --- /dev/null +++ b/scripts/agent-note-output.mjs @@ -0,0 +1,295 @@ +const titleCodePointLimit = 160; +const proseCodePointLimit = 1_200; +const detailItemLimit = 4; +const riskItemLimit = 3; +const listItemCodePointLimit = 500; + +function textSchema(maxLength) { + return { type: 'string', minLength: 1, maxLength }; +} + +function listSchema(maxItems) { + return { + type: 'array', + maxItems, + items: textSchema(listItemCodePointLimit), + }; +} + +const title = textSchema(titleCodePointLimit); +const prose = textSchema(proseCodePointLimit); +const details = listSchema(detailItemLimit); +const risks = listSchema(riskItemLimit); +const fileNote = { + type: 'object', + properties: { + title, + what: prose, + why: prose, + details, + risks, + }, + required: ['title', 'what', 'why', 'details', 'risks'], + additionalProperties: false, +}; + +export function outputSchema(paths, { includeChange = true } = {}) { + const properties = {}; + if (includeChange) { + properties.change = { + type: 'object', + properties: { + title, + summary: prose, + why: prose, + highlights: details, + risks, + }, + required: ['title', 'summary', 'why', 'highlights', 'risks'], + additionalProperties: false, + }; + } + if (paths.length) { + properties.files = { + type: 'array', + items: { + type: 'object', + properties: { + path: { type: 'string', enum: paths }, + ...fileNote.properties, + }, + required: ['path', ...fileNote.required], + additionalProperties: false, + }, + }; + } + return { + type: 'object', + properties, + required: [ + ...(includeChange ? ['change'] : []), + ...(paths.length ? ['files'] : []), + ], + additionalProperties: false, + }; +} + +function normalizedText(value, field, limit) { + if (typeof value !== 'string' || !value.trim()) { + throw new Error(`${field} must be a non-empty string`); + } + const text = value.trim(); + if (Array.from(text).length > limit) { + throw new Error(`${field} must be at most ${limit} Unicode code points`); + } + return text; +} + +// fallow-ignore-next-line complexity +function normalizedList(value, field, maxItems) { + if (!Array.isArray(value) || value.some((item) => typeof item !== 'string')) { + throw new Error(`${field} must be a list of strings`); + } + if (value.length > maxItems) { + throw new Error(`${field} must contain at most ${maxItems} items`); + } + const list = value.map((item) => item.trim()).filter(Boolean); + for (const item of list) { + if (Array.from(item).length > listItemCodePointLimit) { + throw new Error( + `${field} items must be at most ${listItemCodePointLimit} Unicode code points`, + ); + } + } + return list; +} + +function exactFields(value, fields, label) { + if (!value || typeof value !== 'object' || Array.isArray(value)) { + throw new Error(`${label} must be an object`); + } + const actual = Object.keys(value).sort(); + const expected = [...fields].sort(); + if (JSON.stringify(actual) !== JSON.stringify(expected)) { + const missing = expected.filter((field) => !actual.includes(field)); + const extra = actual.filter((field) => !expected.includes(field)); + const detail = [ + missing.length ? `missing: ${missing.join(', ')}` : '', + extra.length ? `extra: ${extra.join(', ')}` : '', + ] + .filter(Boolean) + .join('; '); + throw new Error(`${label} has ${detail}`); + } +} + +function isObject(value) { + return Boolean(value) && + typeof value === 'object' && + !Array.isArray(value); +} + +function responseObject(value) { + if (!isObject(value)) { + throw new Error('Agent response must be an object'); + } + return value; +} + +function unsupportedResponseFields(value) { + return Object.keys(value).filter( + (field) => !['change', 'files'].includes(field), + ); +} + +export function normalizeChangeResponse(value) { + const response = responseObject(value); + if (unsupportedResponseFields(response).length) { + throw new Error('Agent response has unsupported fields'); + } + exactFields( + response.change, + ['title', 'summary', 'why', 'highlights', 'risks'], + 'Change note', + ); + return { + title: normalizedText(response.change.title, 'change.title', titleCodePointLimit), + summary: normalizedText(response.change.summary, 'change.summary', proseCodePointLimit), + why: normalizedText(response.change.why, 'change.why', proseCodePointLimit), + highlights: normalizedList( + response.change.highlights, + 'change.highlights', detailItemLimit, + ), + risks: normalizedList(response.change.risks, 'change.risks', riskItemLimit), + }; +} + +function arrayFileEntry(note) { + if (!isObject(note)) { + return { error: 'Agent response has a malformed file note' }; + } + const path = typeof note.path === 'string' ? note.path.trim() : ''; + return path + ? { entry: { path, note, arrayForm: true } } + : { error: 'Agent response has a file note without a path' }; +} + +function arrayFileEntries(notes) { + const result = { entries: [], errors: [] }; + for (const note of notes) { + const item = arrayFileEntry(note); + if (item.entry) result.entries.push(item.entry); + if (item.error) result.errors.push(item.error); + } + return result; +} + +function fileResponseEntries(value) { + const response = responseObject(value); + const errors = unsupportedResponseFields(response).map( + (field) => `Agent response has unsupported field: ${field}`, + ); + if (Array.isArray(response.files)) { + const arrayResult = arrayFileEntries(response.files); + return { + entries: arrayResult.entries, + errors: [...errors, ...arrayResult.errors], + }; + } + if (!isObject(response.files)) { + throw new Error('Agent response has no file notes object'); + } + return { + entries: Object.entries(response.files).map(([path, note]) => ({ + path, + note, + arrayForm: false, + })), + errors, + }; +} + +function normalizeFileNote(note, path, arrayForm) { + exactFields( + note, + [ + ...(arrayForm ? ['path'] : []), + 'title', + 'what', + 'why', + 'details', + 'risks', + ], + path, + ); + return { + title: normalizedText(note.title, `${path}.title`, titleCodePointLimit), + what: normalizedText(note.what, `${path}.what`, proseCodePointLimit), + why: normalizedText(note.why, `${path}.why`, proseCodePointLimit), + details: normalizedList(note.details, `${path}.details`, detailItemLimit), + risks: normalizedList(note.risks, `${path}.risks`, riskItemLimit), + }; +} + +function indexFileEntries(entries, expected) { + const byPath = new Map(); + const failedFiles = []; + for (const entry of entries) { + if (!expected.has(entry.path)) { + failedFiles.push({ + path: entry.path, + reason: 'Agent output included a file outside this batch.', + }); + continue; + } + const values = byPath.get(entry.path) || []; + byPath.set(entry.path, [...values, entry]); + } + return { byPath, failedFiles }; +} + +function normalizeFileEntry(path, values) { + if (values.length !== 1) { + return { + failure: { + path, + reason: values.length + ? 'Agent output repeated this file.' + : 'Agent output omitted this file.', + }, + }; + } + try { + return { + note: normalizeFileNote( + values[0].note, + path, + values[0].arrayForm, + ), + }; + } catch (error) { + return { + failure: { + path, + reason: error instanceof Error ? error.message : String(error), + }, + }; + } +} + +export function normalizeFileResponse(value, paths) { + const expected = new Set(paths); + const files = {}; + const { entries, errors } = fileResponseEntries(value); + const indexed = indexFileEntries(entries, expected); + for (const path of paths) { + const result = normalizeFileEntry( + path, + indexed.byPath.get(path) || [], + ); + if (result.note) files[path] = result.note; + if (result.failure) indexed.failedFiles.push(result.failure); + } + return { files, failedFiles: indexed.failedFiles, errors }; +} + diff --git a/scripts/check.mjs b/scripts/check.mjs index aed8881..246fd60 100644 --- a/scripts/check.mjs +++ b/scripts/check.mjs @@ -60,6 +60,7 @@ const executeStage = proofMode } : (_id, run) => run(); const packageOnly = process.argv.includes('--package-only'); +const skipDocs = process.argv.includes('--skip-docs'); const releaseTarballIndex = process.argv.indexOf('--release-tarball'); const releaseTarball = releaseTarballIndex === -1 @@ -73,6 +74,7 @@ const requiredPackageFiles = [ 'scripts/agent-usage.mjs', 'scripts/agent-config.mjs', 'scripts/agent-exclusions.mjs', + 'scripts/agent-note-output.mjs', 'scripts/agent-review.mjs', 'scripts/build-diff-data.mjs', 'scripts/cache.mjs', @@ -93,7 +95,7 @@ const requiredPackageFiles = [ 'scripts/summary-path.mjs', 'scripts/support-record.mjs', ]; -const allowedPackageFile = /^(README(?:\.md)?|LICENSE(?:\.md)?|NOTICE(?:\.md)?|package\.json|dist\/.+|scripts\/(?:access-token|agent-config|agent-exclusions|agent-review|agent-usage|build-diff-data|cache|cli-args|coding-agents|dev|doctor|generate-summaries|local-target|mock-agent|present|presenter-runtime|review-chat(?:-context|-controller|-provider)?|serve-built|summary-path|support-record)\.mjs)$/; +const allowedPackageFile = /^(README(?:\.md)?|LICENSE(?:\.md)?|NOTICE(?:\.md)?|package\.json|dist\/.+|scripts\/(?:access-token|agent-config|agent-exclusions|agent-note-output|agent-review|agent-usage|build-diff-data|cache|cli-args|coding-agents|dev|doctor|generate-summaries|local-target|mock-agent|present|presenter-runtime|review-chat(?:-context|-controller|-provider)?|serve-built|summary-path|support-record)\.mjs)$/; const privatePackageFile = /(^|\/)(?:\.env|\.npmrc|\.git|\.github|\.agents|\.codex)(?:\/|$)|\.(?:pem|key)$/i; export function validatePackageManifest(pack) { @@ -219,11 +221,6 @@ async function smokeTestPackage() { ? pack.filename : join(packageRoot, pack.filename); validatePackageManifest(pack); - if (releaseTarball) { - await mkdir(dirname(releaseTarball), { recursive: true }); - await copyFile(tarball, releaseTarball); - } - await mkdir(consumerRoot); await writeFile( join(consumerRoot, 'package.json'), @@ -256,6 +253,10 @@ async function smokeTestPackage() { }); const runtime = await makeSmokeRuntimeFixture(consumerRoot); verifySmokeResults({ packageJson, version, help, doctor, runtime }); + if (releaseTarball) { + await mkdir(dirname(releaseTarball), { recursive: true }); + await copyFile(tarball, releaseTarball); + } } finally { await rm(packageRoot, { force: true, recursive: true }); } @@ -270,9 +271,9 @@ const stages = [ ]; export async function runCheck() { - const selectedStages = packageOnly - ? stages.filter(([id]) => id === 'build') - : stages; + const selectedStages = stages.filter(([id]) => + packageOnly ? id === 'build' : !(skipDocs && id === 'docs'), + ); for (const [id, name, run] of selectedStages) { await runStage(id, name, run); } diff --git a/scripts/generate-summaries.mjs b/scripts/generate-summaries.mjs index 06b0e13..2522550 100644 --- a/scripts/generate-summaries.mjs +++ b/scripts/generate-summaries.mjs @@ -1,4 +1,9 @@ #!/usr/bin/env node +import { + normalizeChangeResponse, + normalizeFileResponse, + outputSchema, +} from './agent-note-output.mjs'; import { spawn, spawnSync } from 'node:child_process'; import { createHash } from 'node:crypto'; @@ -208,11 +213,6 @@ const batchByteLimit = 180_000; const softFileByteLimit = 180_000; const hardInputByteLimit = 2_000_000; const priorNoteByteLimit = 250_000; -const titleCodePointLimit = 160; -const proseCodePointLimit = 1_200; -const detailItemLimit = 4; -const riskItemLimit = 3; -const listItemCodePointLimit = 500; const fileNoteAttemptLimit = 3; const jobsValue = option('--jobs') || '3'; if (!/^[1-9]\d*$/.test(jobsValue) || Number(jobsValue) > 8) { @@ -601,76 +601,6 @@ function batchInput(snapshot, rawSnapshot, paths, existingFiles) { : baseInput; } -function textSchema(maxLength) { - return { type: 'string', minLength: 1, maxLength }; -} - -function listSchema(maxItems) { - return { - type: 'array', - maxItems, - items: textSchema(listItemCodePointLimit), - }; -} - -const title = textSchema(titleCodePointLimit); -const prose = textSchema(proseCodePointLimit); -const details = listSchema(detailItemLimit); -const risks = listSchema(riskItemLimit); -const fileNote = { - type: 'object', - properties: { - title, - what: prose, - why: prose, - details, - risks, - }, - required: ['title', 'what', 'why', 'details', 'risks'], - additionalProperties: false, -}; - -function outputSchema(paths, { includeChange = true } = {}) { - const properties = {}; - if (includeChange) { - properties.change = { - type: 'object', - properties: { - title, - summary: prose, - why: prose, - highlights: details, - risks, - }, - required: ['title', 'summary', 'why', 'highlights', 'risks'], - additionalProperties: false, - }; - } - if (paths.length) { - properties.files = { - type: 'array', - items: { - type: 'object', - properties: { - path: { type: 'string', enum: paths }, - ...fileNote.properties, - }, - required: ['path', ...fileNote.required], - additionalProperties: false, - }, - }; - } - return { - type: 'object', - properties, - required: [ - ...(includeChange ? ['change'] : []), - ...(paths.length ? ['files'] : []), - ], - additionalProperties: false, - }; -} - function promptFor(paths, { accessMode, includeChange = true } = {}) { const responseInstruction = paths.length ? `Return only the file notes required by the output schema. Include one note @@ -703,225 +633,12 @@ is no useful detail or risk. For binary files, describe only the change shown by the metadata.`; } -function normalizedText(value, field, limit) { - if (typeof value !== 'string' || !value.trim()) { - throw new Error(`${field} must be a non-empty string`); - } - const text = value.trim(); - if (Array.from(text).length > limit) { - throw new Error(`${field} must be at most ${limit} Unicode code points`); - } - return text; -} - -// fallow-ignore-next-line complexity -function normalizedList(value, field, maxItems) { - if (!Array.isArray(value) || value.some((item) => typeof item !== 'string')) { - throw new Error(`${field} must be a list of strings`); - } - if (value.length > maxItems) { - throw new Error(`${field} must contain at most ${maxItems} items`); - } - const list = value.map((item) => item.trim()).filter(Boolean); - for (const item of list) { - if (Array.from(item).length > listItemCodePointLimit) { - throw new Error( - `${field} items must be at most ${listItemCodePointLimit} Unicode code points`, - ); - } - } - return list; -} - -function exactFields(value, fields, label) { - if (!value || typeof value !== 'object' || Array.isArray(value)) { - throw new Error(`${label} must be an object`); - } - const actual = Object.keys(value).sort(); - const expected = [...fields].sort(); - if (JSON.stringify(actual) !== JSON.stringify(expected)) { - const missing = expected.filter((field) => !actual.includes(field)); - const extra = actual.filter((field) => !expected.includes(field)); - const detail = [ - missing.length ? `missing: ${missing.join(', ')}` : '', - extra.length ? `extra: ${extra.join(', ')}` : '', - ] - .filter(Boolean) - .join('; '); - throw new Error(`${label} has ${detail}`); - } -} - function isObject(value) { return Boolean(value) && typeof value === 'object' && !Array.isArray(value); } -function responseObject(value) { - if (!isObject(value)) { - throw new Error('Agent response must be an object'); - } - return value; -} - -function unsupportedResponseFields(value) { - return Object.keys(value).filter( - (field) => !['change', 'files'].includes(field), - ); -} - -function normalizeChangeResponse(value) { - const response = responseObject(value); - if (unsupportedResponseFields(response).length) { - throw new Error('Agent response has unsupported fields'); - } - exactFields( - response.change, - ['title', 'summary', 'why', 'highlights', 'risks'], - 'Change note', - ); - return { - title: normalizedText(response.change.title, 'change.title', titleCodePointLimit), - summary: normalizedText(response.change.summary, 'change.summary', proseCodePointLimit), - why: normalizedText(response.change.why, 'change.why', proseCodePointLimit), - highlights: normalizedList( - response.change.highlights, - 'change.highlights', detailItemLimit, - ), - risks: normalizedList(response.change.risks, 'change.risks', riskItemLimit), - }; -} - -function arrayFileEntry(note) { - if (!isObject(note)) { - return { error: 'Agent response has a malformed file note' }; - } - const path = typeof note.path === 'string' ? note.path.trim() : ''; - return path - ? { entry: { path, note, arrayForm: true } } - : { error: 'Agent response has a file note without a path' }; -} - -function arrayFileEntries(notes) { - const result = { entries: [], errors: [] }; - for (const note of notes) { - const item = arrayFileEntry(note); - if (item.entry) result.entries.push(item.entry); - if (item.error) result.errors.push(item.error); - } - return result; -} - -function fileResponseEntries(value) { - const response = responseObject(value); - const errors = unsupportedResponseFields(response).map( - (field) => `Agent response has unsupported field: ${field}`, - ); - if (Array.isArray(response.files)) { - const arrayResult = arrayFileEntries(response.files); - return { - entries: arrayResult.entries, - errors: [...errors, ...arrayResult.errors], - }; - } - if (!isObject(response.files)) { - throw new Error('Agent response has no file notes object'); - } - return { - entries: Object.entries(response.files).map(([path, note]) => ({ - path, - note, - arrayForm: false, - })), - errors, - }; -} - -function normalizeFileNote(note, path, arrayForm) { - exactFields( - note, - [ - ...(arrayForm ? ['path'] : []), - 'title', - 'what', - 'why', - 'details', - 'risks', - ], - path, - ); - return { - title: normalizedText(note.title, `${path}.title`, titleCodePointLimit), - what: normalizedText(note.what, `${path}.what`, proseCodePointLimit), - why: normalizedText(note.why, `${path}.why`, proseCodePointLimit), - details: normalizedList(note.details, `${path}.details`, detailItemLimit), - risks: normalizedList(note.risks, `${path}.risks`, riskItemLimit), - }; -} - -function indexFileEntries(entries, expected) { - const byPath = new Map(); - const failedFiles = []; - for (const entry of entries) { - if (!expected.has(entry.path)) { - failedFiles.push({ - path: entry.path, - reason: 'Agent output included a file outside this batch.', - }); - continue; - } - const values = byPath.get(entry.path) || []; - byPath.set(entry.path, [...values, entry]); - } - return { byPath, failedFiles }; -} - -function normalizeFileEntry(path, values) { - if (values.length !== 1) { - return { - failure: { - path, - reason: values.length - ? 'Agent output repeated this file.' - : 'Agent output omitted this file.', - }, - }; - } - try { - return { - note: normalizeFileNote( - values[0].note, - path, - values[0].arrayForm, - ), - }; - } catch (error) { - return { - failure: { - path, - reason: error instanceof Error ? error.message : String(error), - }, - }; - } -} - -function normalizeFileResponse(value, paths) { - const expected = new Set(paths); - const files = {}; - const { entries, errors } = fileResponseEntries(value); - const indexed = indexFileEntries(entries, expected); - for (const path of paths) { - const result = normalizeFileEntry( - path, - indexed.byPath.get(path) || [], - ); - if (result.note) files[path] = result.note; - if (result.failure) indexed.failedFiles.push(result.failure); - } - return { files, failedFiles: indexed.failedFiles, errors }; -} - function writeJsonAtomic(file, value, options) { publishLeaseFile( ownership, diff --git a/scripts/release.mjs b/scripts/release.mjs index 97f3a40..04c0435 100644 --- a/scripts/release.mjs +++ b/scripts/release.mjs @@ -224,11 +224,11 @@ export async function verifyRelease(overrides = {}) { const initialState = deps.readState(pkg.version); validateReleaseState(initialState); - deps.runPnpm(['run', 'check']); deps.runPnpm([ 'run', - 'package:verify', + 'check', '--', + '--skip-docs', '--release-tarball', verifiedTarball, ]); diff --git a/tests/agent-note-output.test.mjs b/tests/agent-note-output.test.mjs new file mode 100644 index 0000000..4f3431e --- /dev/null +++ b/tests/agent-note-output.test.mjs @@ -0,0 +1,96 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; +import { + normalizeChangeResponse, + normalizeFileResponse, + outputSchema, +} from '../scripts/agent-note-output.mjs'; + +const fileNote = { + title: 'Add text', + what: 'Adds text.', + why: 'Tests note limits.', + details: [], + risks: [], +}; + +const invalidFields = [ + ['title', 'x'.repeat(161), /title.*160/], + ['title', '😀'.repeat(161), /title.*160/], + ['what', 'x'.repeat(1201), /what.*1200/], + ['why', 'x'.repeat(1201), /why.*1200/], + ['details', Array(5).fill('detail'), /details.*4 items/], + ['risks', Array(4).fill('risk'), /risks.*3 items/], + ['details', ['x'.repeat(501)], /details items.*500/], + ['risks', ['x'.repeat(501)], /risks items.*500/], +]; + +for (const [index, [field, value, error]] of invalidFields.entries()) { + test(`rejects file note ${field} exceeding its limit (case ${index + 1})`, () => { + const result = normalizeFileResponse({ + files: { + 'added.txt': { ...fileNote, [field]: value }, + 'changed.txt': fileNote, + }, + }, ['added.txt', 'changed.txt']); + assert.deepEqual(result.files, { 'changed.txt': fileNote }); + assert.equal(result.failedFiles.length, 1); + assert.equal(result.failedFiles[0].path, 'added.txt'); + assert.match(result.failedFiles[0].reason, error); + assert.deepEqual(result.errors, []); + }); +} + +const boundary = { + title: '😀'.repeat(160), + what: 'w'.repeat(1200), + why: 'y'.repeat(1200), + details: Array(4).fill('😀'.repeat(500)), + risks: Array(3).fill('r'.repeat(500)), +}; + +test('accepts file note boundaries in object and array responses', () => { + for (const files of [ + { 'added.txt': boundary }, + [{ path: 'added.txt', ...boundary }], + ]) { + assert.deepEqual(normalizeFileResponse({ files }, ['added.txt']), { + files: { 'added.txt': boundary }, + failedFiles: [], + errors: [], + }); + } +}); + +function changeNote(note) { + const { what, details, ...fields } = note; + return { ...fields, summary: what, highlights: details }; +} + +for (const [index, [field, value, error]] of invalidFields.entries()) { + test(`rejects change note ${field} exceeding its limit (case ${index + 1})`, () => { + const change = changeNote({ ...fileNote, [field]: value }); + const changeError = new RegExp(error.source + .replace('what', 'summary').replace('details', 'highlights')); + assert.throws(() => normalizeChangeResponse({ change }), changeError); + }); +} + +test('accepts change note boundaries', () => { + const change = changeNote(boundary); + assert.deepEqual(normalizeChangeResponse({ change }), change); +}); + +test('scopes output schemas to the requested notes and file paths', () => { + const combined = outputSchema(['added.txt', 'changed.txt']); + assert.deepEqual(combined.required, ['change', 'files']); + assert.deepEqual(combined.properties.files.items.properties.path.enum, [ + 'added.txt', 'changed.txt', + ]); + const files = outputSchema(['added.txt'], { includeChange: false }); + assert.deepEqual(files.required, ['files']); + assert.ok(!Object.hasOwn(files.properties, 'change')); + const change = outputSchema([]); + assert.deepEqual(change.required, ['change']); + assert.ok(!Object.hasOwn(change.properties, 'files')); +}); diff --git a/tests/generate-summaries.test.mjs b/tests/generate-summaries.test.mjs index 9649f7f..39e4c1d 100644 --- a/tests/generate-summaries.test.mjs +++ b/tests/generate-summaries.test.mjs @@ -1110,54 +1110,46 @@ test("states supported purpose directly and names missing evidence", async () => } }); -test("enforces note output limits in schemas and saved file notes", async () => { - const cases = [ - ["title", "x".repeat(161), /added\.txt\.title.*160/], - ["what", "x".repeat(1201), /added\.txt\.what.*1200/], - ["details", Array(5).fill("detail"), /added\.txt\.details.*4 items/], - ["risks", Array(4).fill("risk"), /added\.txt\.risks.*3 items/], - ["details", ["x".repeat(501)], /added\.txt\.details items.*500/], - ["title", "😀".repeat(161), /added\.txt\.title.*160/], - ]; - for (const [field, value, error] of cases) { - const repo = await makeRepo(); - const summaries = join(repo, "notes.json"); - const output = join(repo, "diff-data.json"); - try { - const response = notes({ - "added.txt": { - title: "Add text", - what: "Adds text.", - why: "Tests note limits.", - details: [], - risks: [], - [field]: value, - }, - "changed.txt": { - title: "Change text", - what: "Changes text.", - why: "Keeps one valid note.", - details: [], - risks: [], - }, - }); - const codex = await fakeCodex(repo, response); - const result = run(repo, [ - "--range", "HEAD~1..HEAD", "--codex-bin", codex.bin, - "--summaries", summaries, "--output", output, - ]); - assert.equal(result.status, 1, result.stderr); - const written = JSON.parse(await readFile(summaries, "utf8")); - assert.ok(!Object.hasOwn(written.files, "added.txt")); - assert.ok(Object.hasOwn(written.files, "changed.txt")); - assert.match( - written.meta.failedFiles.find((failure) => failure.path === "added.txt").reason, - error, - ); - } finally { - await rm(repo, { recursive: true, force: true }); - } +test("saves valid notes and reports an invalid note output limit", async () => { + const repo = await makeRepo(); + const summaries = join(repo, "notes.json"); + const output = join(repo, "diff-data.json"); + try { + const response = notes({ + "added.txt": { + what: "Adds text.", + why: "Tests note limits.", + details: [], + risks: [], + title: "😀".repeat(161), + }, + "changed.txt": { + title: "Change text", + what: "Changes text.", + why: "Keeps one valid note.", + details: [], + risks: [], + }, + }); + const codex = await fakeCodex(repo, response); + const result = run(repo, [ + "--range", "HEAD~1..HEAD", "--codex-bin", codex.bin, + "--summaries", summaries, "--output", output, + ]); + assert.equal(result.status, 1, result.stderr); + const written = JSON.parse(await readFile(summaries, "utf8")); + assert.ok(!Object.hasOwn(written.files, "added.txt")); + assert.ok(Object.hasOwn(written.files, "changed.txt")); + assert.match( + written.meta.failedFiles.find((failure) => failure.path === "added.txt").reason, + /added\.txt\.title.*160/, + ); + } finally { + await rm(repo, { recursive: true, force: true }); } +}); + +test("enforces note output limits in schemas and accepts boundary notes", async () => { const repo = await makeRepo(); const summaries = join(repo, "notes.json"); diff --git a/tests/live-update-speed.test.mjs b/tests/live-update-speed.test.mjs deleted file mode 100644 index f22aa27..0000000 --- a/tests/live-update-speed.test.mjs +++ /dev/null @@ -1,30 +0,0 @@ -import assert from 'node:assert/strict'; -import { execFile } from 'node:child_process'; -import { promisify } from 'node:util'; -import test from 'node:test'; - -const run = promisify(execFile); -const benchmark = new URL( - '../benchmarks/live-update-speed.mjs', - import.meta.url, -).pathname; - -for (const [mode, sampleCount] of [ - ['events', 9], - ['poll', 7], -]) { - test(`runs the protected ${mode} live-update benchmark`, async () => { - const { stdout } = await run(process.execPath, [ - benchmark, - '--mode', - mode, - ], { - timeout: 30_000, - }); - const result = JSON.parse(stdout); - - assert.equal(result.mode, mode); - assert.equal(result.samplesMs.length, sampleCount); - assert.ok(result.samplesMs.every((sample) => Number.isFinite(sample))); - }); -} diff --git a/tests/package-manifest.test.mjs b/tests/package-manifest.test.mjs index e14c16d..f4cf093 100644 --- a/tests/package-manifest.test.mjs +++ b/tests/package-manifest.test.mjs @@ -10,6 +10,7 @@ const requiredFiles = [ 'scripts/agent-usage.mjs', 'scripts/agent-config.mjs', 'scripts/agent-exclusions.mjs', + 'scripts/agent-note-output.mjs', 'scripts/agent-review.mjs', 'scripts/build-diff-data.mjs', 'scripts/cache.mjs', diff --git a/tests/product-gate.test.mjs b/tests/product-gate.test.mjs index 83e1570..2ed79e4 100644 --- a/tests/product-gate.test.mjs +++ b/tests/product-gate.test.mjs @@ -6,14 +6,16 @@ import test from 'node:test'; const root = new URL('..', import.meta.url); const check = new URL('../scripts/check.mjs', import.meta.url).pathname; -test('runs the product gate from the lockfile on supported Node lines', async () => { +test('runs the product gate once from the lockfile on the pinned Node LTS', async () => { const [workflow, fallow] = await Promise.all([ readFile(new URL('../.github/workflows/product-gate.yml', import.meta.url), 'utf8'), readFile(new URL('../.github/workflows/fallow.yml', import.meta.url), 'utf8'), ]); assert.match(workflow, /pull_request:/); - assert.match(workflow, /node-version: \[22\.13\.0, 24\.x\]/); + assert.match(workflow, /node-version: '24\.20\.0'/); + assert.doesNotMatch(workflow, /check-latest: true/); + assert.doesNotMatch(workflow, /matrix:/); assert.match(workflow, /uses: pnpm\/action-setup@v4/); assert.match(workflow, /run: pnpm install --frozen-lockfile/); assert.match(workflow, /run: pnpm run check/); @@ -37,6 +39,29 @@ for (const stage of ['test', 'lint', 'docs', 'build', 'package']) { }); } +test('release checks skip docs and still run each package check once', () => { + const result = spawnSync(process.execPath, [check, '--skip-docs'], { + cwd: root, + encoding: 'utf8', + env: { + ...process.env, + DIFFSPLAIN_CHECK_PROOF_MODE: '1', + DIFFSPLAIN_CHECK_PROOF_FAIL_STAGE: 'docs', + }, + }); + + assert.equal(result.status, 0, result.stderr); + assert.deepEqual( + result.stdout.split('\n').filter((line) => line.startsWith('✓ ')), + [ + '✓ React and TypeScript lint', + '✓ Production app build', + '✓ Unit and integration tests', + '✓ Packed-package smoke test', + ], + ); +}); + test('builds fresh assets before standalone package verification', () => { const result = spawnSync(process.execPath, [check, '--package-only'], { cwd: root, diff --git a/tests/release.test.mjs b/tests/release.test.mjs index 840f2b0..4dc495d 100644 --- a/tests/release.test.mjs +++ b/tests/release.test.mjs @@ -43,11 +43,11 @@ test('verifies a tagged release and records the exact tarball', async () => { }); assert.deepEqual(calls, [ - ['run', 'check'], [ 'run', - 'package:verify', + 'check', '--', + '--skip-docs', '--release-tarball', '.cache/diffsplain-release.tgz', ], @@ -71,6 +71,34 @@ test('rejects the wrong branch, a dirty tree, and a mismatched tag', () => { ); }); +test('does not record verification after a failed check or changed commit', async () => { + const common = { + readPackage: async () => ({ name: 'diffsplain', version: '1.2.3' }), + sha256: async () => assert.fail('failed verification must not hash a tarball'), + writeReceipt: async () => assert.fail('failed verification must not write a receipt'), + }; + await assert.rejects( + verifyRelease({ + ...common, + readState: () => cleanState, + runPnpm: () => { throw new Error('smoke test failed'); }, + }), + /smoke test failed/, + ); + + let reads = 0; + await assert.rejects( + verifyRelease({ + ...common, + runPnpm: () => {}, + readState: () => reads++ === 0 + ? cleanState + : { ...cleanState, commit: 'def456', tagCommit: 'def456' }, + }), + /checked-out commit changed/, + ); +}); + test('rejects a stale verification receipt', () => { assert.throws( () => validateReceipt(receipt, { ...receipt, sha256: 'changed-hash' }), diff --git a/tests/rendered-html.test.mjs b/tests/rendered-html.test.mjs index 87520c8..8702ca9 100644 --- a/tests/rendered-html.test.mjs +++ b/tests/rendered-html.test.mjs @@ -104,35 +104,6 @@ test("keeps live review data out of built assets", async () => { ); }); -test("makes the landing-page demo interactive", async () => { - const [html, script, styles] = await Promise.all([ - readFile(new URL("../site/index.html", import.meta.url), "utf8"), - readFile(new URL("../site/script.js", import.meta.url), "utf8"), - readFile(new URL("../site/styles.css", import.meta.url), "utf8"), - ]); - - assert.match(html, /