diff --git a/CHANGELOG.md b/CHANGELOG.md index 666d4b6..4c2cbaa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,38 @@ Versioning follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Fixed + +- `checkLeaks()` filtered `collectedUnclosed` by `types`, so the leak this tool + calls the most definitive one there is could be dropped from the verdict. + Found while dogfooding: a run ending with 47 live `VideoDecoder`s and ten + collected without `close()` printed "No leaked WebCodecs objects." The shim + saw all of it and warned on the console; the assertion API filtered it out. + A GC'd-unclosed object of any tracked type now fails the check regardless of + `types`, which can turn a previously passing suite red — correctly. +- `checkLeaks()` and `summarize()` no longer print an unqualified all-clear + while an unenforced type holds live objects. The message now names them: + `No leaks in VideoFrame, AudioData, ImageBitmap — but VideoDecoder=47 still + live and not enforced.` +- The version stamp. `VERSION` in the core and the version the MCP server + announces were both hardcoded `'0.1.0'`, and `scripts/version.mjs` rewrote + neither — so every census payload from 0.2.0 and 0.2.1 carried a version two + releases stale. The release script now carries both, and fails loudly rather + than silently if either stops matching its pattern. +- `webcodecs_leak_sites` attributed only the default frame types, so an agent + asking which line is leaking got nothing back for a codec leak. Attribution + is not a verdict — it now covers every type unless one is named. + +### Added + +- `types: 'all'` on `checkLeaks()` / `expectNoLeaks()`, so enforcing the codecs + does not mean spelling out all seven type names. +- `LeakReport.unenforcedLive` and `LeakReport.enforced`: what was live but out + of scope, and what the filter resolved to. +- `test/assert.test.mjs`, which pins the verdict layer against synthetic + censuses — including a `VideoDecoder` collected without `close()`, which no + browser test can produce on demand. + ## [0.2.1] - 2026-08-14 ### Added diff --git a/README.md b/README.md index 4bd2a21..bf2157a 100644 --- a/README.md +++ b/README.md @@ -166,6 +166,13 @@ test('the editor releases every frame it decodes', async () => { `checkLeaks()` returns the same information without throwing. `minAgeMs` ignores objects that may still legitimately be in flight. +`types` decides what counts as live too long, and defaults to the frame-like types — a long-lived decoder is normal, a long-lived frame almost never is. Pass `types: 'all'` to hold the codecs to the same standard. Whatever you pass, an object the GC collected while it was still open fails the check, and a type left out of `types` is named in the message rather than quietly reported clean: + +``` +No leaks in VideoFrame, AudioData, ImageBitmap — but VideoDecoder=47 still +live and not enforced. Pass types: 'all' to check those too. +``` + ## The timeline, and why a snapshot lies A static count answers "how many are live". It cannot answer "was the pipeline busy when playback stalled" — and that difference matters. In the app this was built against, live decoder count did **not** predict failure: the highest count succeeded and lower counts stalled. A snapshot would have sent you after a resource-exhaustion bug that wasn't there. diff --git a/packages/core/README.md b/packages/core/README.md index 19959f3..fe7afb1 100644 --- a/packages/core/README.md +++ b/packages/core/README.md @@ -68,6 +68,25 @@ test('the editor releases every frame it decodes', async () => { `checkLeaks()` returns the same information without throwing. `minAgeMs` ignores objects that may still legitimately be in flight. +`types` decides what counts as live too long. It defaults to the frame-like +types, because a long-lived decoder is normal and a long-lived frame almost +never is. Pass `types: 'all'` to hold the codecs to the same standard: + +```js +expectNoLeaks([localCensus()], { types: 'all' }); +``` + +Two things `types` deliberately does not do. It never hides an object the GC +collected while it was still open — that is the definitive leak, and it fails +the check whatever its type. And it never lets the report claim a clean bill of +health for a type it did not look at: an unenforced type with live objects is +named in the message. + +``` +No leaks in VideoFrame, AudioData, ImageBitmap — but VideoDecoder=47 still +live and not enforced. Pass types: 'all' to check those too. +``` + ## What it counts, and why that is not obvious Tracked: `VideoDecoder`, `VideoEncoder`, `AudioDecoder`, `AudioEncoder`, diff --git a/packages/core/src/assert.ts b/packages/core/src/assert.ts index 7ab4265..2c93993 100644 --- a/packages/core/src/assert.ts +++ b/packages/core/src/assert.ts @@ -3,14 +3,22 @@ * rather than something a human has to notice in a panel. */ +import { TRACKED } from './types'; import type { ContextCensus, LeakSite, TrackedType } from './types'; export interface LeakReport { ok: boolean; - /** Live objects, summed across contexts, by type. */ + /** Live objects of the enforced types, summed across contexts. */ live: Partial>; - /** GC'd without close(), summed across contexts. Always a genuine leak. */ + /** Live objects of the types `types` left out. Reported, never failed on. */ + unenforcedLive: Partial>; + /** + * GC'd without close(), summed across contexts and across every tracked + * type. `types` cannot filter this one away — see `checkLeaks`. + */ collectedUnclosed: Partial>; + /** The types `types` resolved to. */ + enforced: TrackedType[]; /** Allocation sites holding live objects, worst first. */ sites: (LeakSite & { context: string })[]; message: string; @@ -18,10 +26,11 @@ export interface LeakReport { export interface LeakOptions { /** - * Types to enforce. Defaults to the frame-like types, because a long-lived - * decoder is normal and a long-lived frame almost never is. + * Types to enforce, or `'all'` for every tracked type. Defaults to the + * frame-like types, because a long-lived decoder is normal and a long-lived + * frame almost never is. */ - types?: TrackedType[]; + types?: TrackedType[] | 'all'; /** Tolerated live count per type. A steady-state pipeline holds a few. */ allow?: Partial>; /** Ignore live objects younger than this — they may be legitimately in flight. */ @@ -34,48 +43,86 @@ export function totalLive(censuses: ContextCensus[], type: TrackedType): number return censuses.reduce((sum, c) => sum + (c.live[type] ?? 0), 0); } -/** Build a report without throwing. `checkLeaks(...).ok` is the boolean form. */ +const counts = (m: Partial>) => + Object.entries(m) + .filter(([, n]) => n) + .map(([t, n]) => `${t}=${n}`) + .join(' '); + +/** + * Build a report without throwing. `checkLeaks(...).ok` is the boolean form. + * + * `types` narrows what counts as *live too long*. It deliberately does not + * narrow objects the GC collected while they were still open: that is the + * definitive leak, and a filter aimed at live frames must not hide a decoder + * that was dropped on the floor. + */ export function checkLeaks(censuses: ContextCensus[], options: LeakOptions = {}): LeakReport { - const types = options.types ?? DEFAULT_TYPES; + const enforced = options.types === 'all' ? [...TRACKED] : options.types ?? DEFAULT_TYPES; const allow = options.allow ?? {}; const minAgeMs = options.minAgeMs ?? 0; const live: Partial> = {}; + const unenforcedLive: Partial> = {}; const collectedUnclosed: Partial> = {}; const sites: (LeakSite & { context: string })[] = []; for (const c of censuses) { - for (const t of types) { - if (c.live[t]) live[t] = (live[t] ?? 0) + c.live[t]!; + for (const t of TRACKED) { + const n = c.live[t] ?? 0; + if (n) { + const bucket = enforced.includes(t) ? live : unenforcedLive; + bucket[t] = (bucket[t] ?? 0) + n; + } if (c.collectedUnclosed[t]) { collectedUnclosed[t] = (collectedUnclosed[t] ?? 0) + c.collectedUnclosed[t]!; } } for (const s of c.leakSites) { - if (types.includes(s.type) && s.oldestAgeMs >= minAgeMs) { + if (enforced.includes(s.type) && s.oldestAgeMs >= minAgeMs) { sites.push({ ...s, context: c.context }); } } } sites.sort((a, b) => b.count - a.count); - const over = types.filter((t) => (live[t] ?? 0) > (allow[t] ?? 0)); - const collected = types.filter((t) => (collectedUnclosed[t] ?? 0) > 0); + const over = enforced.filter((t) => (live[t] ?? 0) > (allow[t] ?? 0)); + const collected = TRACKED.filter((t) => (collectedUnclosed[t] ?? 0) > 0); const ok = over.length === 0 && collected.length === 0; - return { ok, live, collectedUnclosed, sites, message: describe(ok, over, collected, live, collectedUnclosed, sites, allow) }; + return { + ok, + live, + unenforcedLive, + collectedUnclosed, + enforced, + sites, + message: describe(ok, over, collected, enforced, live, unenforcedLive, collectedUnclosed, sites, allow), + }; } function describe( ok: boolean, over: TrackedType[], collected: TrackedType[], + enforced: TrackedType[], live: Partial>, + unenforcedLive: Partial>, collectedUnclosed: Partial>, sites: (LeakSite & { context: string })[], allow: Partial>, ): string { - if (ok) return 'No leaked WebCodecs objects.'; + const unenforced = counts(unenforcedLive); + + if (ok) { + if (!unenforced) return 'No leaked WebCodecs objects.'; + // An unqualified all-clear next to 47 live decoders is how this tool + // reported clean on the exact leak it was pointed at. + return ( + `No leaks in ${enforced.join(', ')} — but ${unenforced} still live and not enforced. ` + + `Pass types: 'all' to check those too.` + ); + } const lines: string[] = []; for (const t of collected) { @@ -84,6 +131,9 @@ function describe( for (const t of over) { lines.push(`${live[t]} ${t} still live (allowed ${allow[t] ?? 0}).`); } + if (unenforced) { + lines.push(`Not enforced, and still live: ${unenforced}. Pass types: 'all' to check those too.`); + } if (sites.length) { lines.push('', 'Held by:'); for (const s of sites.slice(0, 5)) { @@ -112,7 +162,7 @@ export function expectNoLeakedFrames(censuses: ContextCensus[], options: LeakOpt * full census is large and mostly stacks. */ export function summarize(censuses: ContextCensus[]): string { - const report = checkLeaks(censuses, { types: ['VideoFrame', 'AudioData', 'ImageBitmap'] }); + const report = checkLeaks(censuses); const lines = [`${censuses.length} context(s): ${censuses.map((c) => c.context).join(', ')}`]; for (const c of censuses) { diff --git a/packages/core/src/census.ts b/packages/core/src/census.ts index d171a19..1128509 100644 --- a/packages/core/src/census.ts +++ b/packages/core/src/census.ts @@ -659,4 +659,4 @@ function safely(what: string, fn: () => void): void { } } -export const VERSION = '0.1.0'; +export const VERSION = '0.2.1'; diff --git a/packages/mcp/src/index.ts b/packages/mcp/src/index.ts index 8c3fe60..535dcb3 100644 --- a/packages/mcp/src/index.ts +++ b/packages/mcp/src/index.ts @@ -17,7 +17,7 @@ import { } from '@modelcontextprotocol/sdk/types.js'; import { attach, launchChrome, type CensusSession } from '@motionvector/webcodecs-census-cdp'; -import { checkLeaks, summarize } from '@motionvector/webcodecs-census'; +import { checkLeaks, summarize, type TrackedType } from '@motionvector/webcodecs-census'; let session: CensusSession | null = null; let chrome: Awaited> | null = null; @@ -117,7 +117,7 @@ const TOOLS = [ ]; const server = new Server( - { name: 'webcodecs-census', version: '0.1.0' }, + { name: 'webcodecs-census', version: '0.2.1' }, { capabilities: { tools: {} } }, ); @@ -179,8 +179,10 @@ server.setRequestHandler(CallToolRequestSchema, async (request) => { case 'webcodecs_leak_sites': { const s = requireSession(); const censuses = await s.census(); + // Attribution, not a verdict: never hide a type the caller did not ask + // about, or "which line is leaking" answers nothing for a codec leak. const report = checkLeaks(censuses, { - types: args.type ? [args.type] : undefined, + types: args.type ? [args.type as TrackedType] : 'all', minAgeMs: args.minAgeMs, }); const sites = report.sites.slice(0, args.limit ?? 10); diff --git a/scripts/version.mjs b/scripts/version.mjs index 1ae96a4..cabbb02 100644 --- a/scripts/version.mjs +++ b/scripts/version.mjs @@ -66,6 +66,25 @@ for (const dir of PACKAGES) { console.log(` ${pkg.name} -> ${next}`); } +// Two version strings live in source rather than package.json: the census +// stamps every payload with one, and the MCP server announces the other on the +// wire. Nothing rewrote them here, so both said 0.1.0 for two releases. +const STAMPS = [ + { path: 'packages/core/src/census.ts', re: /(export const VERSION = ')[^']+(')/ }, + { path: 'packages/mcp/src/index.ts', re: /(name: 'webcodecs-census', version: ')[^']+(')/ }, +]; + +for (const { path, re } of STAMPS) { + const src = readFileSync(path, 'utf8'); + // Loudly, not silently: a pattern that stops matching is how they went stale. + if (!re.test(src)) { + console.error(`\n ${path} no longer matches its version pattern.\n Fix scripts/version.mjs before releasing, or the stamp ships wrong.\n`); + process.exit(1); + } + writeFileSync(path, src.replace(re, `$1${next}$2`)); + console.log(` ${path} -> ${next}`); +} + // Promote the Unreleased section rather than inventing notes: the release body // is generated from this file, so an empty section is a release with no notes. const CHANGELOG = 'CHANGELOG.md'; diff --git a/test/assert.test.mjs b/test/assert.test.mjs new file mode 100644 index 0000000..fb655c3 --- /dev/null +++ b/test/assert.test.mjs @@ -0,0 +1,120 @@ +/** + * The verdict layer, on synthetic censuses. No browser: the point is what + * `checkLeaks` does with numbers, and a GC'd-unclosed decoder is not something + * a test can make Chrome produce on demand. + * + * Found by dogfooding, 2026-08-17: a run that ended with 47 live VideoDecoders + * and ~10 collected without close() printed "No leaked WebCodecs objects." + * The shim saw all of it. The default `types` filter threw it away. + */ + +import { test, describe } from 'node:test'; +import assert from 'node:assert/strict'; + +import { checkLeaks, expectNoLeaks, summarize, TRACKED } from '../packages/core/dist/index.js'; + +/** A census with only the fields the verdict layer reads. */ +function census({ context = 'main', live = {}, collectedUnclosed = {}, leakSites = [] } = {}) { + return { + context, + uptimeMs: 30_000, + entered: {}, + left: {}, + live, + collectedUnclosed, + closedUnseen: 0, + leakSites, + oldestLive: [], + mediaElements: { total: 0, stalled: 0, byReadyState: {} }, + timeline: [], + problems: [], + }; +} + +const site = (type, count) => ({ + type, + origin: 'constructed', + stack: ` at leakOne (app.js:1:1)`, + count, + oldestAgeMs: 5_000, +}); + +describe('a leaking VideoDecoder under the default types', () => { + const leaked = [ + census({ + live: { VideoDecoder: 47 }, + collectedUnclosed: { VideoDecoder: 10 }, + leakSites: [site('VideoDecoder', 47)], + }), + ]; + + test('a decoder collected without close() fails whatever types says', () => { + const report = checkLeaks(leaked); + assert.equal(report.ok, false, `verdict was clean:\n${report.message}`); + assert.equal(report.collectedUnclosed.VideoDecoder, 10); + assert.match(report.message, /10 VideoDecoder garbage collected without close\(\)/); + }); + + test('the same holds for a type nobody asked about at all', () => { + const report = checkLeaks([census({ collectedUnclosed: { AudioEncoder: 3 } })], { + types: ['VideoFrame'], + }); + assert.equal(report.ok, false, `verdict was clean:\n${report.message}`); + assert.throws(() => expectNoLeaks([census({ collectedUnclosed: { AudioEncoder: 3 } })]), /AudioEncoder/); + }); + + test('live decoders are still not enforced by default — but never reported as clean', () => { + const live = [census({ live: { VideoDecoder: 47 }, leakSites: [site('VideoDecoder', 47)] })]; + const report = checkLeaks(live); + + assert.equal(report.ok, true, 'a live decoder is normal; the default must not fail on it'); + assert.equal(report.unenforcedLive.VideoDecoder, 47); + assert.doesNotMatch( + report.message, + /^No leaked WebCodecs objects\.$/, + 'an unqualified all-clear next to 47 live decoders is the bug', + ); + assert.match(report.message, /VideoDecoder=47/); + assert.match(summarize(live), /VideoDecoder=47/); + }); + + test("types: 'all' enforces every tracked type", () => { + const report = checkLeaks([census({ live: { VideoDecoder: 47 } })], { types: 'all' }); + assert.equal(report.ok, false); + assert.deepEqual(report.enforced, [...TRACKED]); + assert.equal(report.live.VideoDecoder, 47); + assert.match(report.message, /47 VideoDecoder still live/); + }); + + test("types: 'all' surfaces the codec's allocation site", () => { + const report = checkLeaks( + [census({ live: { VideoDecoder: 47 }, leakSites: [site('VideoDecoder', 47)] })], + { types: 'all' }, + ); + assert.equal(report.sites.length, 1); + assert.equal(report.sites[0].type, 'VideoDecoder'); + }); +}); + +describe('the clean case stays clean', () => { + test('nothing live, nothing collected', () => { + const report = checkLeaks([census()]); + assert.equal(report.ok, true); + assert.equal(report.message, 'No leaked WebCodecs objects.'); + }); + + test('frames within their allowance', () => { + const report = checkLeaks([census({ live: { VideoFrame: 2 } })], { allow: { VideoFrame: 2 } }); + assert.equal(report.ok, true); + assert.equal(report.message, 'No leaked WebCodecs objects.'); + }); + + test('leaked frames still fail, and still say where', () => { + const report = checkLeaks([ + census({ live: { VideoFrame: 5 }, leakSites: [site('VideoFrame', 5)] }), + ]); + assert.equal(report.ok, false); + assert.match(report.message, /5 VideoFrame still live \(allowed 0\)/); + assert.match(report.message, /Held by:/); + }); +}); diff --git a/test/version.test.mjs b/test/version.test.mjs new file mode 100644 index 0000000..9d718b8 --- /dev/null +++ b/test/version.test.mjs @@ -0,0 +1,39 @@ +/** + * Two version strings live in source rather than package.json, and both said + * 0.1.0 through the 0.2.0 and 0.2.1 releases: every census payload carried a + * wrong stamp, and the MCP server announced a wrong version on the wire. + * + * `scripts/version.mjs` rewrites them now. This is what catches it if that + * rewrite ever silently stops matching. + */ + +import { test, describe } from 'node:test'; +import assert from 'node:assert/strict'; +import { readFileSync } from 'node:fs'; + +import { VERSION } from '../packages/core/dist/index.js'; + +const read = (p) => readFileSync(new URL(p, import.meta.url), 'utf8'); +const version = (p) => JSON.parse(read(p)).version; + +describe('the version stamps track the packages', () => { + test('the census stamps payloads with the core version', () => { + assert.equal(VERSION, version('../packages/core/package.json')); + }); + + test('the MCP server announces its own version', () => { + const src = read('../packages/mcp/src/index.ts'); + const found = src.match(/name: 'webcodecs-census', version: '([^']+)'/)?.[1]; + assert.ok(found, 'the MCP server no longer declares a version the way version.mjs rewrites it'); + assert.equal(found, version('../packages/mcp/package.json')); + }); + + test('all three packages are on one version', () => { + const [core, cdp, mcp] = ['core', 'cdp', 'mcp'].map((p) => + version(`../packages/${p}/package.json`), + ); + // -cdp and -mcp depend on an exact core version; out of step is unpublishable. + assert.equal(cdp, core); + assert.equal(mcp, core); + }); +});