diff --git a/.github/workflows/dist-diff-comment.yml b/.github/workflows/dist-diff-comment.yml new file mode 100644 index 00000000..c119740c --- /dev/null +++ b/.github/workflows/dist-diff-comment.yml @@ -0,0 +1,85 @@ +name: dist-diff comment + +# Posts the dist-diff report as a single sticky PR comment, edited in place on +# every push. Split from dist-diff.yml (workflow_run) so the report job stays +# unprivileged and fork PRs get the same comment as internal ones. +# +# This is the only privileged part of dist-diff. It treats the artifact as +# data: the PR number is verified against the run's head SHA before posting, +# and the report body is only ever posted as comment text. + +on: + workflow_run: + workflows: [dist-diff] + types: [completed] + +permissions: + pull-requests: write + +jobs: + comment: + # No conclusion gate: a failed self-check still uploads a (warning-stamped) + # report, and the PR should get its comment either way. Runs where the + # report step itself died leave no artifact — the download step's outcome + # check below skips the comment quietly in that case. + if: github.event.workflow_run.event == 'pull_request' + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - name: Download the report artifact + id: download + continue-on-error: true + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + with: + name: dist-diff-report + path: dist-diff-report + run-id: ${{ github.event.workflow_run.id }} + github-token: ${{ secrets.GITHUB_TOKEN }} + + - name: Upsert the sticky comment + if: steps.download.outcome == 'success' + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + with: + script: | + const fs = require('fs'); + const marker = ''; + + const prNumber = parseInt(fs.readFileSync('dist-diff-report/pr-number', 'utf8').trim(), 10); + if (!Number.isInteger(prNumber)) throw new Error('invalid PR number in artifact'); + + // The artifact comes from an unprivileged run of untrusted code: + // only post to the PR whose head produced this exact run. + const { data: pr } = await github.rest.pulls.get({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: prNumber, + }); + if (pr.head.sha !== context.payload.workflow_run.head_sha) { + throw new Error('PR number in artifact does not match the run head SHA'); + } + + let body = `${marker}\n${fs.readFileSync('dist-diff-report/comment.md', 'utf8')}`; + if (body.length > 65000) body = `${body.slice(0, 65000)}\n…truncated — see the workflow summary.`; + + const comments = await github.paginate(github.rest.issues.listComments, { + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: prNumber, + per_page: 100, + }); + const existing = comments.find(c => c.body?.startsWith(marker)); + if (existing) { + await github.rest.issues.updateComment({ + owner: context.repo.owner, + repo: context.repo.repo, + comment_id: existing.id, + body, + }); + } else { + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: prNumber, + body, + }); + } diff --git a/.github/workflows/dist-diff.yml b/.github/workflows/dist-diff.yml new file mode 100644 index 00000000..9697af4d --- /dev/null +++ b/.github/workflows/dist-diff.yml @@ -0,0 +1,112 @@ +name: dist-diff + +# Shows reviewers what a PR changes in the BUILT artifacts (dist/) — the +# skill menu the wizard reads, the marketplace plugins, the MCP manifest — +# which a source diff of configs/markdown can't show. Runs unprivileged; +# the sticky PR comment is posted by dist-diff-comment.yml (workflow_run), +# so fork PRs get the same report as internal ones. +# +# Normalization guarantee: dist-diff reports are only trustworthy if two +# builds of the same ref produce an empty normalized diff. That self-check +# runs on every push to main (catches environment drift) and on PRs that +# touch the build pipeline or dependencies (the only changes that can +# introduce new nondeterminism) — content PRs can never go red here. +# A failed self-check still renders and uploads the report (stamped with a +# warning) before failing the job, so the PR never silently loses its comment. + +on: + pull_request: + branches: [main] + push: + branches: [main] + +permissions: + contents: read + +jobs: + dist-diff: + runs-on: ubuntu-latest + timeout-minutes: 15 + steps: + - name: Checkout repository + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 + with: + fetch-depth: 0 + + - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 + with: + node-version: lts/* + + - uses: pnpm/action-setup@b906affcce14559ad1aafd4ab0e942779e9f58b1 # v4.3.0 + with: + version: latest + + - name: Install dependencies + run: pnpm install + + - name: Scope the run + id: scope + run: | + if [ "${{ github.event_name }}" = "push" ]; then + echo "self-check=true" >> "$GITHUB_OUTPUT" + else + MERGE_BASE=$(git merge-base "origin/${{ github.base_ref }}" HEAD) + echo "merge-base=$MERGE_BASE" >> "$GITHUB_OUTPUT" + if git diff --name-only "$MERGE_BASE"...HEAD | grep -qE '^(scripts/|package\.json$|pnpm-lock\.yaml$|\.github/workflows/dist-diff)'; then + echo "self-check=true" >> "$GITHUB_OUTPUT" + else + echo "self-check=false" >> "$GITHUB_OUTPUT" + fi + fi + + - name: Build head + # Also primes .docs-cache, which every later build in this job shares — + # so upstream doc changes between builds cannot leak into the diff. + run: pnpm build + + - name: Self-check — two builds of the same ref must normalize identically + id: selfcheck + if: steps.scope.outputs.self-check == 'true' + # Deferred failure: the report steps below still run, then the last + # step turns a failed self-check into a red job. + continue-on-error: true + run: | + mv dist dist-head-first + pnpm build + pnpm --silent diff --exit-code dist-head-first dist + rm -rf dist-head-first + + - name: Build merge-base and render the report + if: github.event_name == 'pull_request' + run: | + git worktree add /tmp/base "${{ steps.scope.outputs.merge-base }}" + ln -s "$PWD/node_modules" /tmp/base/node_modules + mkdir -p .docs-cache + ln -s "$PWD/.docs-cache" /tmp/base/.docs-cache + (cd /tmp/base && pnpm build) + + RUN_URL="${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}" + WARN="" + if [ "${{ steps.selfcheck.outcome }}" = "failure" ]; then + WARN="> ⚠️ **Self-check failed on this run** — two builds of the same ref differ after normalization, so this report may contain noise. See the job log." + fi + + mkdir -p dist-diff-report + { [ -n "$WARN" ] && printf '%s\n\n' "$WARN"; \ + pnpm --silent diff /tmp/base/dist dist --format comment --summary-url "$RUN_URL"; } > dist-diff-report/comment.md + { [ -n "$WARN" ] && printf '%s\n\n' "$WARN"; \ + pnpm --silent diff /tmp/base/dist dist --format full; } >> "$GITHUB_STEP_SUMMARY" + echo "${{ github.event.pull_request.number }}" > dist-diff-report/pr-number + + - name: Upload report for the comment workflow + if: github.event_name == 'pull_request' + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: dist-diff-report + path: dist-diff-report/ + + - name: Fail the job on a failed self-check + if: steps.selfcheck.outcome == 'failure' + run: | + echo "::error::dist-diff self-check failed: the build is nondeterministic. Add a normalization rule in scripts/lib/dist-diff.js for the file(s) named above." + exit 1 diff --git a/package.json b/package.json index 6fe272a0..7503aaf3 100644 --- a/package.json +++ b/package.json @@ -7,6 +7,7 @@ "scripts": { "build": "node scripts/build.js", "dev": "node scripts/dev-server.js", + "diff": "node scripts/diff-dist.js", "visual-dags": "node scripts/visual-dags.js", "test:plugins": "vitest run scripts/plugins/tests", "test:plugins:watch": "vitest scripts/plugins/tests", @@ -19,6 +20,8 @@ "devDependencies": { "archiver": "^7.0.1", "chokidar": "^4.0.3", + "diff": "9.0.0", + "fflate": "0.8.3", "vitest": "^2.0.0" }, "engines": { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 7bb3d4d5..0785b136 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -27,6 +27,12 @@ importers: chokidar: specifier: ^4.0.3 version: 4.0.3 + diff: + specifier: 9.0.0 + version: 9.0.0 + fflate: + specifier: 0.8.3 + version: 0.8.3 vitest: specifier: ^2.0.0 version: 2.1.9 @@ -233,66 +239,79 @@ packages: resolution: {integrity: sha512-E8jKK87uOvLrrLN28jnAAAChNq5LeCd2mGgZF+fGF5D507WlG/Noct3lP/QzQ6MrqJ5BCKNwI9ipADB6jyiq2A==} cpu: [arm] os: [linux] + libc: [glibc] '@rollup/rollup-linux-arm-musleabihf@4.56.0': resolution: {integrity: sha512-jQosa5FMYF5Z6prEpTCCmzCXz6eKr/tCBssSmQGEeozA9tkRUty/5Vx06ibaOP9RCrW1Pvb8yp3gvZhHwTDsJw==} cpu: [arm] os: [linux] + libc: [musl] '@rollup/rollup-linux-arm64-gnu@4.56.0': resolution: {integrity: sha512-uQVoKkrC1KGEV6udrdVahASIsaF8h7iLG0U0W+Xn14ucFwi6uS539PsAr24IEF9/FoDtzMeeJXJIBo5RkbNWvQ==} cpu: [arm64] os: [linux] + libc: [glibc] '@rollup/rollup-linux-arm64-musl@4.56.0': resolution: {integrity: sha512-vLZ1yJKLxhQLFKTs42RwTwa6zkGln+bnXc8ueFGMYmBTLfNu58sl5/eXyxRa2RarTkJbXl8TKPgfS6V5ijNqEA==} cpu: [arm64] os: [linux] + libc: [musl] '@rollup/rollup-linux-loong64-gnu@4.56.0': resolution: {integrity: sha512-FWfHOCub564kSE3xJQLLIC/hbKqHSVxy8vY75/YHHzWvbJL7aYJkdgwD/xGfUlL5UV2SB7otapLrcCj2xnF1dg==} cpu: [loong64] os: [linux] + libc: [glibc] '@rollup/rollup-linux-loong64-musl@4.56.0': resolution: {integrity: sha512-z1EkujxIh7nbrKL1lmIpqFTc/sr0u8Uk0zK/qIEFldbt6EDKWFk/pxFq3gYj4Bjn3aa9eEhYRlL3H8ZbPT1xvA==} cpu: [loong64] os: [linux] + libc: [musl] '@rollup/rollup-linux-ppc64-gnu@4.56.0': resolution: {integrity: sha512-iNFTluqgdoQC7AIE8Q34R3AuPrJGJirj5wMUErxj22deOcY7XwZRaqYmB6ZKFHoVGqRcRd0mqO+845jAibKCkw==} cpu: [ppc64] os: [linux] + libc: [glibc] '@rollup/rollup-linux-ppc64-musl@4.56.0': resolution: {integrity: sha512-MtMeFVlD2LIKjp2sE2xM2slq3Zxf9zwVuw0jemsxvh1QOpHSsSzfNOTH9uYW9i1MXFxUSMmLpeVeUzoNOKBaWg==} cpu: [ppc64] os: [linux] + libc: [musl] '@rollup/rollup-linux-riscv64-gnu@4.56.0': resolution: {integrity: sha512-in+v6wiHdzzVhYKXIk5U74dEZHdKN9KH0Q4ANHOTvyXPG41bajYRsy7a8TPKbYPl34hU7PP7hMVHRvv/5aCSew==} cpu: [riscv64] os: [linux] + libc: [glibc] '@rollup/rollup-linux-riscv64-musl@4.56.0': resolution: {integrity: sha512-yni2raKHB8m9NQpI9fPVwN754mn6dHQSbDTwxdr9SE0ks38DTjLMMBjrwvB5+mXrX+C0npX0CVeCUcvvvD8CNQ==} cpu: [riscv64] os: [linux] + libc: [musl] '@rollup/rollup-linux-s390x-gnu@4.56.0': resolution: {integrity: sha512-zhLLJx9nQPu7wezbxt2ut+CI4YlXi68ndEve16tPc/iwoylWS9B3FxpLS2PkmfYgDQtosah07Mj9E0khc3Y+vQ==} cpu: [s390x] os: [linux] + libc: [glibc] '@rollup/rollup-linux-x64-gnu@4.56.0': resolution: {integrity: sha512-MVC6UDp16ZSH7x4rtuJPAEoE1RwS8N4oK9DLHy3FTEdFoUTCFVzMfJl/BVJ330C+hx8FfprA5Wqx4FhZXkj2Kw==} cpu: [x64] os: [linux] + libc: [glibc] '@rollup/rollup-linux-x64-musl@4.56.0': resolution: {integrity: sha512-ZhGH1eA4Qv0lxaV00azCIS1ChedK0V32952Md3FtnxSqZTBTd6tgil4nZT5cU8B+SIw3PFYkvyR4FKo2oyZIHA==} cpu: [x64] os: [linux] + libc: [musl] '@rollup/rollup-openbsd-x64@4.56.0': resolution: {integrity: sha512-O16XcmyDeFI9879pEcmtWvD/2nyxR9mF7Gs44lf1vGGx8Vg2DRNx11aVXBEqOQhWb92WN4z7fW/q4+2NYzCbBA==} @@ -491,6 +510,10 @@ packages: resolution: {integrity: sha512-h5k/5U50IJJFpzfL6nO9jaaumfjO/f2NjK/oYB2Djzm4p9L+3T9qWpZqZ2hAbLPuuYq9wrU08WQyBTL5GbPk5Q==} engines: {node: '>=6'} + diff@9.0.0: + resolution: {integrity: sha512-svtcdpS8CgJyqAjEQIXdb3OjhFVVYjzGAPO8WGCmRbrml64SPw/jJD4GoE98aR7r25A0XcgrK3F02yw9R/vhQw==} + engines: {node: '>=0.3.1'} + eastasianwidth@0.2.0: resolution: {integrity: sha512-I88TYZWc9XiYHRQ4/3c5rjjfgkjhLyW2luGIheGERbNQ6OY7yTybanSpDXZa8y7VUP9YmDcYa+eyq4ca7iLqWA==} @@ -541,6 +564,9 @@ packages: fast-sha256@1.3.0: resolution: {integrity: sha512-n11RGP/lrWEFI/bWdygLxhI+pVeo1ZYIVwvvPkW7azl/rOy+F3HYRZ2K5zeE9mmkhQppyv9sQFx0JM9UabnpPQ==} + fflate@0.8.3: + resolution: {integrity: sha512-tbZNuJrLwGUp3zshBtdy4W+ORxZuIh8a5ilyIEQDC5rY1f3U20JMry0Ll3WBzU58EZKsEuJFXhb5gwv8CsPvgA==} + foreground-child@3.3.1: resolution: {integrity: sha512-gIXjKqtFuWEgzFRJA9WCQeSJLZDjgJUOMCMzxtvFq/37KojM1BFGufqsCy0r4qSQmYLsZYMeyRqzIWOMup03sw==} engines: {node: '>=14'} @@ -552,6 +578,7 @@ packages: glob@10.5.0: resolution: {integrity: sha512-DfXN8DfhJ7NH3Oe7cFmu3NCu1wKbkReJ8TorzSAFbSKrlNaQSKfIzqYqVY8zlbs2NLBbWpRiU52GX2PbaBVNkg==} + deprecated: Old versions of glob are not supported, and contain widely publicized security vulnerabilities, which have been fixed in the current version. Please update. Support for old versions may be purchased (at exorbitant rates) by contacting i@izs.me hasBin: true graceful-fs@4.2.11: @@ -1212,6 +1239,8 @@ snapshots: deep-eql@5.0.2: {} + diff@9.0.0: {} + eastasianwidth@0.2.0: {} emoji-regex@8.0.0: {} @@ -1272,6 +1301,8 @@ snapshots: fast-sha256@1.3.0: {} + fflate@0.8.3: {} + foreground-child@3.3.1: dependencies: cross-spawn: 7.0.6 diff --git a/scripts/diff-dist.js b/scripts/diff-dist.js new file mode 100644 index 00000000..b72f9716 --- /dev/null +++ b/scripts/diff-dist.js @@ -0,0 +1,90 @@ +#!/usr/bin/env node +/** + * dist-diff CLI — show what a change does to the built artifacts in dist/. + * + * Usage: + * node scripts/diff-dist.js [options] + * node scripts/diff-dist.js --against [options] + * + * Options: + * --format comment|full comment: sticky-comment report (default); full: complete listing + * --summary-url URL append a "Full report" link to the comment format + * --exit-code exit 1 when the normalized diff is non-empty (like `git diff --exit-code`); + * CI uses this to assert two builds of the same ref are identical + * + * --against builds the given ref in a temporary worktree and rebuilds the + * local dist/, both sharing the current .docs-cache — so upstream doc drift + * cannot appear in the diff (same guarantee the CI job gets by running both + * builds in one job). + */ +import { execFileSync } from 'child_process'; +import { existsSync, mkdtempSync, rmSync, symlinkSync } from 'fs'; +import { join, resolve } from 'path'; +import { tmpdir } from 'os'; +import { diffDistTrees, renderComment, renderFull } from './lib/dist-diff.js'; + +const options = { positional: [] }; +const argv = process.argv.slice(2); +for (let i = 0; i < argv.length; i++) { + switch (argv[i]) { + case '--against': options.against = argv[++i]; break; + case '--format': options.format = argv[++i]; break; + case '--summary-url': options.summaryUrl = argv[++i]; break; + case '--exit-code': options.exitCode = true; break; + default: options.positional.push(argv[i]); + } +} + +const repoRoot = resolve(import.meta.dirname, '..'); + +function buildRefDist(ref) { + const worktree = mkdtempSync(join(tmpdir(), 'dist-diff-base-')); + execFileSync('git', ['worktree', 'add', '--detach', worktree, ref], { cwd: repoRoot, stdio: 'inherit' }); + try { + symlinkSync(join(repoRoot, 'node_modules'), join(worktree, 'node_modules')); + // Share the docs cache so both sides see identical upstream doc bytes. + if (existsSync(join(repoRoot, '.docs-cache'))) { + symlinkSync(join(repoRoot, '.docs-cache'), join(worktree, '.docs-cache')); + } + execFileSync('node', ['scripts/build.js'], { cwd: worktree, stdio: 'inherit' }); + return { worktree, dist: join(worktree, 'dist') }; + } catch (err) { + rmSync(worktree, { recursive: true, force: true }); + execFileSync('git', ['worktree', 'prune'], { cwd: repoRoot }); + throw err; + } +} + +let beforeDir, afterDir, cleanup; +if (options.against) { + console.error(`Building ${options.against} in a temporary worktree...`); + const base = buildRefDist(options.against); + beforeDir = base.dist; + afterDir = join(repoRoot, 'dist'); + cleanup = () => { + rmSync(base.worktree, { recursive: true, force: true }); + execFileSync('git', ['worktree', 'prune'], { cwd: repoRoot }); + }; + // Always rebuild — a dist/ left over from another ref would silently + // poison the comparison. The base build above primed .docs-cache. + console.error('Rebuilding local dist/...'); + execFileSync('node', ['scripts/build.js'], { cwd: repoRoot, stdio: 'inherit' }); +} else { + [beforeDir, afterDir] = options.positional; + if (!beforeDir || !afterDir) { + console.error('Usage: diff-dist.js | --against [--format comment|full] [--summary-url URL] [--exit-code]'); + process.exit(2); + } +} + +try { + const model = await diffDistTrees(beforeDir, afterDir); + if (options.format === 'full') { + console.log(renderFull(model)); + } else { + console.log(renderComment(model, { fullReportUrl: options.summaryUrl })); + } + if (options.exitCode && model.changes.length) process.exitCode = 1; +} finally { + cleanup?.(); +} diff --git a/scripts/lib/dist-diff.js b/scripts/lib/dist-diff.js new file mode 100644 index 00000000..39f2f7e5 --- /dev/null +++ b/scripts/lib/dist-diff.js @@ -0,0 +1,633 @@ +/** + * dist-diff — normalized differ over two built dist/ trees. + * + * Design invariants (see the dist-diff PR/issue for the full rationale): + * - Totality: every file under both trees is compared. Unknown file types + * degrade to byte comparison — a future artifact type can at worst add a + * noisy line to the report, never be silently skipped. + * - Normalization is a small set of NAMED rules, nothing broader: + * ZIPs are compared by entry contents (the archive format embeds mtimes, + * which differ on every build), and JSON files are compared by parsed + * value with known build-stamp fields removed. + * - Determinism guard: `npm run diff` with `--exit-code` (git-diff + * convention) lets CI assert two builds of the same ref produce an empty + * normalized diff, so any new nondeterminism in the build fails loudly + * instead of eroding the report. + */ +import { readdirSync, readFileSync } from 'fs'; +import { join } from 'path'; +import { createHash } from 'crypto'; +import { unzipSync } from 'fflate'; +import { structuredPatch } from 'diff'; + +/** + * Read a ZIP buffer into { entryName: contentBuffer }. fflate is the same + * library the PostHog MCP server uses to unzip these archives. Entry mtimes + * and attributes are dropped here on purpose: they are exactly the noise this + * tool normalizes away. + */ +export function readZipEntries(buf) { + const entries = {}; + for (const [name, data] of Object.entries(unzipSync(buf))) { + if (!name.endsWith('/')) entries[name] = Buffer.from(data.buffer, data.byteOffset, data.byteLength); + } + return entries; +} + +/** + * True when two JSON buffers are equal after removing build-stamp fields. + * buildTimestamp (build-phases.js writeManifestAndMenu) is the build's only + * per-run stamp today; add fields here if another one appears. + */ +function jsonEqualIgnoringStamps(a, b) { + let pa, pb; + try { + pa = JSON.parse(a.toString('utf8')); + pb = JSON.parse(b.toString('utf8')); + } catch { + return false; // unparseable JSON falls back to the byte comparison verdict + } + if (pa && typeof pa === 'object') delete pa.buildTimestamp; + if (pb && typeof pb === 'object') delete pb.buildTimestamp; + return JSON.stringify(pa) === JSON.stringify(pb); +} + +/** + * True when two buffers are equal after normalization for their file type. + * Applies recursively inside zips, so aggregate archives (zips of zips plus a + * stamped manifest) normalize the same way the tree does. + */ +function normalizedEqual(name, a, b) { + if (a.equals(b)) return true; + if (name.endsWith('.json')) return jsonEqualIgnoringStamps(a, b); + if (name.endsWith('.zip')) return zipInnerChanges(a, b).length === 0; + return false; +} + +/** + * Fingerprint of a zip's inner delta: same changed entry names with the same + * before/after bytes → same hash. Lets the report group fan-out (one shared + * doc rebuilt into dozens of variant zips) into a single line. + */ +// Length-framed so contents can't collide with each other or with absence. +function hashFrame(hash, buf) { + hash.update(String(buf ? buf.length : -1)).update('\0').update(buf ?? '').update('\0'); +} + +function zipDeltaHash(a, b, innerChanges) { + const ea = readZipEntries(a); + const eb = readZipEntries(b); + const hash = createHash('sha256'); + for (const name of innerChanges) { + hash.update(name).update('\0'); + hashFrame(hash, ea[name]); + hashFrame(hash, eb[name]); + } + return hash.digest('hex'); +} + +/** Fingerprint of a plain file's before→after delta, for fan-out grouping. */ +function fileDeltaHash(a, b) { + const hash = createHash('sha256'); + hashFrame(hash, a); + hashFrame(hash, b); + return hash.digest('hex'); +} + +/** Inner entry names that differ (normalized) between two zip buffers. */ +function zipInnerChanges(a, b) { + const ea = readZipEntries(a); + const eb = readZipEntries(b); + const names = [...new Set([...Object.keys(ea), ...Object.keys(eb)])].sort(); + return names.filter(n => !ea[n] || !eb[n] || !normalizedEqual(n, ea[n], eb[n])); +} + +/** Recursively list files under dir as tree-relative paths. */ +function listFiles(dir, prefix = '') { + const out = []; + for (const entry of readdirSync(dir, { withFileTypes: true })) { + const rel = prefix ? `${prefix}/${entry.name}` : entry.name; + if (entry.isDirectory()) out.push(...listFiles(join(dir, entry.name), rel)); + else out.push(rel); + } + return out; +} + +/** + * Diff two dist trees. Returns { changes: [{ path, kind }] } where kind is + * 'added' | 'removed' | 'changed'. + */ +export async function diffDistTrees(beforeDir, afterDir) { + const beforeFiles = new Set(listFiles(beforeDir)); + const afterFiles = new Set(listFiles(afterDir)); + const changes = []; + + for (const path of [...new Set([...beforeFiles, ...afterFiles])].sort()) { + if (!beforeFiles.has(path)) { + changes.push({ path, kind: 'added' }); + } else if (!afterFiles.has(path)) { + changes.push({ path, kind: 'removed' }); + } else { + const a = readFileSync(join(beforeDir, path)); + const b = readFileSync(join(afterDir, path)); + try { + if (normalizedEqual(path, a, b)) continue; + } catch (err) { + // A file our own build just wrote but can't be read back (e.g. + // a truncated zip) is a broken build — fail loudly, naming it. + throw new Error(`dist-diff failed reading ${path}: ${err.message}`, { cause: err }); + } + if (path.endsWith('.zip')) { + const innerChanges = zipInnerChanges(a, b); + changes.push({ path, kind: 'changed', innerChanges, deltaHash: zipDeltaHash(a, b, innerChanges) }); + } else { + changes.push({ path, kind: 'changed', deltaHash: fileDeltaHash(a, b) }); + } + } + } + return { beforeDir, afterDir, changes }; +} + +/** Review surfaces, ordered by user impact. classify() returns the first match. */ +const SURFACES = [ + { key: 'wizard', title: 'Wizard surface', match: p => p === 'skills/skill-menu.json' }, + { + key: 'skill-content', + title: 'Skill content', + match: p => p.startsWith('skills/') && p !== 'skills/manifest.json', + }, + { key: 'marketplace', title: 'Marketplace', match: p => p.startsWith('marketplace/') }, + { key: 'agents', title: 'Agents', match: p => p.startsWith('agents/') }, + { + key: 'mcp', + title: 'MCP manifest', + match: p => p === 'skills/manifest.json' || p === 'skills-mcp-resources.zip', + }, + { key: 'mirror', title: 'Skills-repo mirror', match: p => p === 'push-manifest.json' }, + { key: 'other', title: 'Other artifacts', match: () => true }, +]; + +function classify(path) { + return SURFACES.find(s => s.match(path)).key; +} + +function bucketBySurface(model) { + const bySurface = new Map(SURFACES.map(s => [s.key, []])); + for (const change of model.changes) bySurface.get(classify(change.path)).push(change); + return bySurface; +} + +// Report bodies are ```diff fenced blocks: a leading '+' renders green, +// '-' renders red, and any other first character (we use '~' and ' ') gray. +const STATUS = { added: '+', removed: '-', changed: '~' }; +const MAX_INLINE_INNER = 5; + +/** Path prefix implied by a surface's title, stripped inside its block. */ +const SURFACE_STRIP = { 'skill-content': 'skills/', marketplace: 'marketplace/', agents: 'agents/' }; + +function stripSurfacePrefix(surfaceKey, path) { + const prefix = SURFACE_STRIP[surfaceKey]; + return prefix && path.startsWith(prefix) ? path.slice(prefix.length) : path; +} + +/** + * skills-mcp-resources.zip is DERIVED: it bundles manifest.json plus every + * skill zip/bundle, so its diff can never carry new information — unless the + * bundling step broke. Verify each inner change maps to a reported + * constituent change (inner `x.zip` ↔ `skills/x.zip`); consistent means the + * one-line summary is a checked claim, not an assumption. + */ +const AGGREGATE_PATH = 'skills-mcp-resources.zip'; + +function aggregateConsistency(model, change) { + const changedPaths = new Set(model.changes.map(c => c.path)); + const inner = change.innerChanges ?? []; + const unexplained = inner.filter(name => !changedPaths.has(`skills/${name}`)); + const zips = inner.filter(name => name.endsWith('.zip')).length; + const summary = [zips && `${zips} zip(s)`, inner.length - zips && `${inner.length - zips} other file(s)`] + .filter(Boolean).join(', '); + return { consistent: unexplained.length === 0, unexplained, summary }; +} + +/** Grouping key for a changed artifact: same key ⇔ identical delta. */ +function deltaKey(change) { + return change.innerChanges + ? `zip:${change.innerChanges.join('|')}#${change.deltaHash}` + : `file:#${change.deltaHash}`; +} + +/** " — inner1, inner2" suffix for a zip change, optionally capped. */ +function innerSuffix(change, capInner) { + if (!change.innerChanges?.length) return ''; + return ` — ${capInner ? capList(change.innerChanges) : change.innerChanges.join(', ')}`; +} + +/** + * Render changes as an indented directory tree (single-child directory chains + * collapsed, `tree`-style). A directory whose descendants all share one status + * carries that status char itself, so a freshly added subtree is solid green. + */ +function treeLines(items, { capInner = true } = {}) { + const root = { dirs: new Map(), files: [] }; + for (const { rel, change } of items) { + const segs = rel.split('/'); + let node = root; + for (const seg of segs.slice(0, -1)) { + if (!node.dirs.has(seg)) node.dirs.set(seg, { dirs: new Map(), files: [] }); + node = node.dirs.get(seg); + } + node.files.push({ name: segs.at(-1), change }); + } + const kindsOf = node => [ + ...node.files.map(f => f.change.kind), + ...[...node.dirs.values()].flatMap(kindsOf), + ]; + const lines = []; + const emit = (node, depth) => { + for (const [name, start] of [...node.dirs].sort(([a], [b]) => a.localeCompare(b))) { + let label = name; + let child = start; + while (child.files.length === 0 && child.dirs.size === 1) { + const [[next, sub]] = child.dirs; + label = `${label}/${next}`; + child = sub; + } + const kinds = new Set(kindsOf(child)); + const status = kinds.size === 1 ? STATUS[[...kinds][0]] : ' '; + // A directory holding exactly one file collapses onto one line. + if (child.dirs.size === 0 && child.files.length === 1) { + const f = child.files[0]; + lines.push(`${STATUS[f.change.kind]} ${' '.repeat(depth)}${label}/${f.name}${innerSuffix(f.change, capInner)}`); + continue; + } + lines.push(`${status} ${' '.repeat(depth)}${label}/`); + emit(child, depth + 1); + } + for (const f of [...node.files].sort((a, b) => a.name.localeCompare(b.name))) { + lines.push(`${STATUS[f.change.kind]} ${' '.repeat(depth)}${f.name}${innerSuffix(f.change, capInner)}`); + } + }; + emit(root, 0); + return lines; +} + +/** Added/removed cliEntries between the two skill menus. */ +function cliEntryDelta(model) { + const read = dir => { + try { + return JSON.parse(readFileSync(join(dir, 'skills/skill-menu.json'), 'utf8')).cliEntries ?? []; + } catch { + return []; + } + }; + const key = e => [e.skillId, e.parentCommand, e.command].join('|'); + const beforeEntries = new Map(read(model.beforeDir).map(e => [key(e), e])); + const afterEntries = new Map(read(model.afterDir).map(e => [key(e), e])); + return { + added: [...afterEntries.values()].filter(e => !beforeEntries.has(key(e))), + removed: [...beforeEntries.values()].filter(e => !afterEntries.has(key(e))), + modified: [...afterEntries.values()].filter(e => { + const prev = beforeEntries.get(key(e)); + return prev && JSON.stringify(prev) !== JSON.stringify(e); + }), + }; +} + +function describeEntry(e) { + const cmd = e.command ? `, command: ${e.parentCommand ? `${e.parentCommand} ` : ''}${e.command}` : ''; + return `${e.skillId} (role: ${e.role}${cmd})`; +} + +/** Diff-block lines for the wizard surface: semantic cliEntry delta. */ +function wizardBlock(model) { + const { added, removed, modified } = cliEntryDelta(model); + const block = [ + ...added.map(e => `+ cliEntry ${describeEntry(e)}`), + ...removed.map(e => `- cliEntry ${describeEntry(e)}`), + ...modified.map(e => `~ cliEntry ${e.skillId} updated`), + ]; + return block.length ? block : ['~ skill-menu.json changed (no cliEntry changes)']; +} + +/** + * Render the sticky-comment report: surfaces ordered by user impact, explicit + * "unchanged" assertions, hard line budget. + */ +export function renderComment(model, { fullReportUrl } = {}) { + const bySurface = bucketBySurface(model); + + // Build per-surface segments first, so the budget can trim inside diff + // blocks while the surface headers and "✓ unchanged" assertions — the + // report's core guarantee — always survive truncation. + const segments = []; + for (const surface of SURFACES) { + const changes = bySurface.get(surface.key); + if (surface.key === 'other' && changes.length === 0) continue; + if (changes.length === 0) { + segments.push({ head: [`**${surface.title}** ✓ unchanged`], block: null, after: [] }); + continue; + } + const { block, after } = surface.key === 'wizard' + ? { block: wizardBlock(model), after: [] } + : surfaceBlock(model, changes, surface.key); + segments.push({ head: [`**${surface.title}**`], block, after }); + } + + const BUDGET = 40; + const fixedCost = 1 + (fullReportUrl ? 2 : 0) + + segments.reduce((n, s) => n + 1 + s.head.length + (s.block ? 2 : 0) + s.after.length, 0); + let overflow = segments.reduce((n, s) => n + (s.block?.length ?? 0), 0) - (BUDGET - fixedCost); + if (overflow > 0) { + // Trim the largest blocks first; each trimmed block keeps its first + // lines plus a gray pointer at the full report. + for (const s of [...segments].sort((a, b) => (b.block?.length ?? 0) - (a.block?.length ?? 0))) { + if (overflow <= 0) break; + const len = s.block?.length ?? 0; + if (len < 4) continue; + const cut = Math.min(overflow + 1, len - 2); + s.block.length = len - cut; + s.block.push(`~ …${cut} more line(s) — see the full report in the workflow summary`); + overflow -= cut - 1; + } + } + + const lines = [`## dist-diff — ${model.changes.length} artifact change(s)`]; + for (const s of segments) { + // Blank line between blocks — without it, GFM folds a line that + // follows a fenced block or list into the preceding element. + lines.push('', ...s.head); + if (s.block) lines.push('```diff', ...s.block, '```', ...s.after); + } + if (fullReportUrl) lines.push('', `[Full report](${fullReportUrl})`); + return lines.join('\n'); +} + +// Content-hunk limits for the full report: the step summary allows ~1MB, but +// a diff nobody can scan is worse than a truncated one. +const MAX_HUNK_LINES = 120; +const MAX_LINE_CHARS = 300; +const MAX_DIFF_BYTES = 300 * 1024; + +function isText(buf) { + return !buf.subarray(0, 8192).includes(0); +} + +function hunkLines(oldStr, newStr) { + const patch = structuredPatch('a', 'b', oldStr, newStr, '', '', { context: 3 }); + const lines = []; + for (const h of patch.hunks) { + lines.push(`@@ -${h.oldStart},${h.oldLines} +${h.newStart},${h.newLines} @@`); + lines.push(...h.lines); + } + return lines.map(l => (l.length > MAX_LINE_CHARS ? `${l.slice(0, MAX_LINE_CHARS)}…` : l)); +} + +function capHunks(lines) { + if (lines.length <= MAX_HUNK_LINES) return lines; + return [...lines.slice(0, MAX_HUNK_LINES), `… ${lines.length - MAX_HUNK_LINES} more diff line(s) omitted`]; +} + +/** Unified-diff lines for one artifact (added/removed diff against empty). */ +function changeHunks(model, change) { + const tooLarge = size => `(too large to diff: ${Math.round(size / 1024)} KB)`; + const a = change.kind === 'added' ? Buffer.alloc(0) : readFileSync(join(model.beforeDir, change.path)); + const b = change.kind === 'removed' ? Buffer.alloc(0) : readFileSync(join(model.afterDir, change.path)); + if (change.path.endsWith('.zip')) { + const ea = a.length ? readZipEntries(a) : {}; + const eb = b.length ? readZipEntries(b) : {}; + const names = change.innerChanges + ?? [...new Set([...Object.keys(ea), ...Object.keys(eb)])].sort(); + const lines = []; + for (const name of names) { + lines.push(`# ${name}`); + if (name.endsWith('.zip')) { + lines.push('(nested archive — its contents are diffed as individual skill zips)'); + continue; + } + const ia = ea[name] ?? Buffer.alloc(0); + const ib = eb[name] ?? Buffer.alloc(0); + // Gate on DECOMPRESSED size — a tiny archive can hold a huge entry. + if (ia.length > MAX_DIFF_BYTES || ib.length > MAX_DIFF_BYTES) { + lines.push(tooLarge(Math.max(ia.length, ib.length))); + continue; + } + if (!isText(ia) || !isText(ib)) { + lines.push('(binary entry)'); + continue; + } + lines.push(...hunkLines(ia.toString('utf8'), ib.toString('utf8'))); + } + return capHunks(lines); + } + if (a.length > MAX_DIFF_BYTES || b.length > MAX_DIFF_BYTES) { + return [tooLarge(Math.max(a.length, b.length))]; + } + if (!isText(a) || !isText(b)) return ['(binary file)']; + return capHunks(hunkLines(a.toString('utf8'), b.toString('utf8'))); +} + +/** + * Sections for the full report, one per unique delta. An identical-delta + * fan-out is titled by WHAT changed (the shared inner file), shows its hunk + * once, and collapses the member list — the zip names are the noise, the + * content change is the signal. + */ +function fullSections(model, changes) { + const groups = new Map(); + for (const change of changes) { + const key = change.kind === 'changed' && change.deltaHash ? deltaKey(change) : `single:${change.path}`; + if (!groups.has(key)) groups.set(key, []); + groups.get(key).push(change); + } + return [...groups.values()].map(members => { + const rep = members[0]; + if (rep.path === AGGREGATE_PATH && rep.kind === 'changed') { + const { consistent, unexplained, summary } = aggregateConsistency(model, rep); + if (consistent) { + return { + title: `\`${AGGREGATE_PATH}\` — aggregate`, + note: `✓ consistent with its constituents — ${summary}, diffed in the sections above`, + members: null, + }; + } + return { + title: `⚠️ \`${AGGREGATE_PATH}\` — aggregate diverges from its constituents`, + hunks: [`unexplained inner change(s): ${unexplained.join(', ')}`, ...changeHunks(model, rep)], + members: null, + }; + } + const isZip = rep.path.endsWith('.zip'); + let title; + if (members.length > 1) { + const what = rep.innerChanges + ? capList(rep.innerChanges.map(n => `\`${n}\``)) + : `\`${commonPathSuffix(members.map(m => m.path)) || rep.path}\``; + title = `${what} — changed identically in ${members.length} ${isZip ? 'zips' : 'files'}`; + } else { + const kindNote = rep.kind === 'changed' ? '' : ` (${rep.kind})`; + title = `\`${rep.path}\`${kindNote}`; + } + return { + title, + hunks: changeHunks(model, rep), + members: members.length > 1 + ? `
${members.length} ${isZip ? 'archives' : 'files'}${members.map(m => m.path).join(', ')}
` + : null, + }; + }); +} + +/** Render the full report: every change listed; content hunks size-budgeted. */ +export function renderFull(model) { + const bySurface = bucketBySurface(model); + const lines = [`# dist-diff full report — ${model.changes.length} artifact change(s)`, '']; + let hunkBudget = 4000; + for (const surface of SURFACES) { + const changes = bySurface.get(surface.key); + if (surface.key === 'other' && changes.length === 0) continue; + lines.push(`## ${surface.title}`, ''); + if (changes.length === 0) { + lines.push('✓ unchanged', ''); + continue; + } + if (surface.key === 'wizard') { + lines.push('```diff', ...wizardBlock(model), '```', ''); + } + // One section per unique delta: hunk inline (the signal), member list + // collapsed (the noise). Report-wide hunk budget — the step summary + // rejects anything over 1MB. + let omitted = 0; + for (const { title, hunks, note, members } of fullSections(model, changes)) { + if (note) { + lines.push(`### ${title}`, '', note, ''); + continue; + } + if (hunkBudget <= 0) { omitted++; continue; } + hunkBudget -= hunks.length; + lines.push(`### ${title}`, '', '```diff', ...hunks, '```', ''); + if (members) lines.push(members, ''); + } + if (omitted) { + lines.push(`_${omitted} more content diff(s) omitted for size — run \`npm run diff\` locally for the complete set._`, ''); + } + } + return lines.join('\n'); +} + +/** + * One surface's diff-block content plus its
member lists (which + * are HTML and must live outside the code fence). Fan-out collapses in three + * ways: + * - changed files/zips with an IDENTICAL delta → one "identical delta" line + * - ≥4 changed items touching the same file name / inner-file set with + * differing contents (a skill rebuilt across variants) → one line + * - ≥4 files added/removed in the same directory (a new variant) → one line + * Everything below the thresholds is rendered as a directory tree. + */ +const GROUP_MIN = 4; + +function surfaceBlock(model, changes, surfaceKey) { + const block = []; + const after = []; + const singles = []; + const exactGroups = new Map(); + const dirBuckets = new Map(); + for (const change of changes) { + if (change.path === AGGREGATE_PATH && change.kind === 'changed') { + const { consistent, unexplained, summary } = aggregateConsistency(model, change); + if (consistent) { + block.push(`~ ${AGGREGATE_PATH} — aggregate, consistent with its constituents (${summary}, reported above)`); + } else { + block.push(`~ ⚠️ ${AGGREGATE_PATH} — ${unexplained.length} inner change(s) no constituent explains: ${capList(unexplained)}`); + singles.push(change); + } + continue; + } + if (change.kind === 'changed' && change.deltaHash) { + const key = deltaKey(change); + if (!exactGroups.has(key)) exactGroups.set(key, []); + exactGroups.get(key).push(change); + } else { + const dir = change.path.slice(0, change.path.lastIndexOf('/') + 1); + const key = `${change.kind}:${dir}`; + if (!dirBuckets.has(key)) dirBuckets.set(key, []); + dirBuckets.get(key).push(change); + } + } + + // A new variant lands as one directory tree; fold each bucket into the + // shallowest same-kind bucket whose directory contains it. The root + // bucket (empty dir, key "kind:") is a string-prefix of every same-kind + // key and has no meaningful label — it never folds or groups. + const isRootKey = k => k.endsWith(':'); + const dirKeys = [...dirBuckets.keys()].sort((a, b) => a.length - b.length); + for (const key of dirKeys) { + if (!dirBuckets.has(key)) continue; + const ancestor = dirKeys.find(k => k !== key && !isRootKey(k) && dirBuckets.has(k) && key.startsWith(k)); + if (ancestor) { + dirBuckets.get(ancestor).push(...dirBuckets.get(key)); + dirBuckets.delete(key); + } + } + for (const [key, members] of dirBuckets) { + if (members.length >= GROUP_MIN && !isRootKey(key)) { + const dir = stripSurfacePrefix(surfaceKey, key.slice(key.indexOf(':') + 1)); + block.push(`${STATUS[members[0].kind]} ${dir} — ${members.length} files ${members[0].kind}`); + after.push(`
${dir} (${members.length} files)${members.map(m => m.path).join(', ')}
`); + } else { + singles.push(...members); + } + } + + // Merge exact-delta groups that share a signature (same inner-file set for + // zips, same file name for plain files) — a rebuild fans out over dozens + // of variants whose contents differ per variant. + const signatureBuckets = new Map(); + for (const members of exactGroups.values()) { + const first = members[0]; + const sig = first.innerChanges ? `zip:${first.innerChanges.join('|')}` : `file:${first.path.split('/').pop()}`; + if (!signatureBuckets.has(sig)) signatureBuckets.set(sig, []); + signatureBuckets.get(sig).push(members); + } + for (const groups of signatureBuckets.values()) { + const total = groups.reduce((n, g) => n + g.length, 0); + const first = groups[0][0]; + let label = null; + if (groups.length === 1 && total > 1) { + label = first.innerChanges + ? `${total} zips changed — identical delta in ${capList(first.innerChanges)}` + : `${total} files changed — identical delta (${commonPathSuffix(groups[0].map(m => m.path)) || 'no common name'})`; + } else if (groups.length > 1 && total >= GROUP_MIN) { + label = first.innerChanges + ? `${total} zips changed — same files touched (${capList(first.innerChanges)}), contents differ per archive` + : `${total} files changed — same file name (${first.path.split('/').pop()}), contents differ`; + } + if (label) { + block.push(`~ ${label}`); + after.push(`
${first.innerChanges ? 'archives' : 'files'} (${total})${groups.flat().map(m => m.path).join(', ')}
`); + } else { + singles.push(...groups.flat()); + } + } + + block.push(...treeLines(singles.map(c => ({ rel: stripSurfacePrefix(surfaceKey, c.path), change: c })))); + return { block, after }; +} + +/** Comma-joined list, capped at MAX_INLINE_INNER with a "+N more" tail. */ +function capList(names) { + if (names.length <= MAX_INLINE_INNER) return names.join(', '); + return [...names.slice(0, MAX_INLINE_INNER), `…+${names.length - MAX_INLINE_INNER} more`].join(', '); +} + +/** Longest common trailing path segments, e.g. shared "references/4-conclude.md". */ +function commonPathSuffix(paths) { + const parts = paths.map(p => p.split('/')); + const suffix = []; + for (let i = 1; ; i++) { + const seg = parts[0][parts[0].length - i]; + if (seg === undefined || !parts.every(p => p[p.length - i] === seg)) break; + suffix.unshift(seg); + } + return suffix.join('/'); +} diff --git a/scripts/lib/tests/dist-diff.test.js b/scripts/lib/tests/dist-diff.test.js new file mode 100644 index 00000000..83b5ddf4 --- /dev/null +++ b/scripts/lib/tests/dist-diff.test.js @@ -0,0 +1,447 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { mkdirSync, writeFileSync, mkdtempSync, rmSync } from 'fs'; +import { join, dirname } from 'path'; +import { tmpdir } from 'os'; +import archiver from 'archiver'; + +import { diffDistTrees, renderComment, renderFull } from '../dist-diff.js'; + +/** Write a file tree: { 'a/b.txt': 'content' } */ +function writeTree(baseDir, files) { + for (const [rel, content] of Object.entries(files)) { + const full = join(baseDir, rel); + mkdirSync(dirname(full), { recursive: true }); + writeFileSync(full, content); + } +} + +/** Zip entries { name: content } into a buffer via archiver (the build's writer). */ +function zipBuffer(entries, { date } = {}) { + return new Promise((resolve, reject) => { + const chunks = []; + const archive = archiver('zip', { zlib: { level: 9 } }); + archive.on('data', c => chunks.push(c)); + archive.on('end', () => resolve(Buffer.concat(chunks))); + archive.on('error', reject); + for (const [name, content] of Object.entries(entries)) { + archive.append(content, { name, date: date ?? new Date('2020-01-01T00:00:00Z') }); + } + archive.finalize(); + }); +} + +let before, after; + +beforeEach(() => { + before = mkdtempSync(join(tmpdir(), 'distdiff-before-')); + after = mkdtempSync(join(tmpdir(), 'distdiff-after-')); +}); + +afterEach(() => { + rmSync(before, { recursive: true, force: true }); + rmSync(after, { recursive: true, force: true }); +}); + +describe('diffDistTrees', () => { + it('reports no changes for identical trees', async () => { + const files = { + 'skills/skill-menu.json': '{"cliEntries":[]}', + 'skills/web-analytics.zip': 'placeholder', + }; + writeTree(before, files); + writeTree(after, files); + + const model = await diffDistTrees(before, after); + expect(model.changes).toEqual([]); + }); + + it('filters zip archives whose bytes differ only by entry mtimes', async () => { + const entries = { 'SKILL.md': '# Web analytics', 'references/setup.md': 'steps' }; + writeTree(before, { + 'skills/web-analytics.zip': await zipBuffer(entries, { date: new Date('2020-01-01T00:00:00Z') }), + }); + writeTree(after, { + 'skills/web-analytics.zip': await zipBuffer(entries, { date: new Date('2026-08-09T12:00:00Z') }), + }); + + const model = await diffDistTrees(before, after); + expect(model.changes).toEqual([]); + }); + + it('reports a zip whose entry content changed, naming the inner files', async () => { + writeTree(before, { + 'skills/web-analytics.zip': await zipBuffer({ 'SKILL.md': 'v1', 'references/setup.md': 'same' }), + }); + writeTree(after, { + 'skills/web-analytics.zip': await zipBuffer({ 'SKILL.md': 'v2', 'references/setup.md': 'same' }), + }); + + const model = await diffDistTrees(before, after); + expect(model.changes).toEqual([ + { path: 'skills/web-analytics.zip', kind: 'changed', innerChanges: ['SKILL.md'], deltaHash: expect.any(String) }, + ]); + }); + + it('filters a manifest whose only difference is buildTimestamp, but reports real manifest changes', async () => { + const resource = { id: 'web-analytics', uri: 'posthog://skills/web/web-analytics' }; + writeTree(before, { + 'skills/manifest.json': JSON.stringify({ buildTimestamp: '2026-08-01T00:00:00Z', resources: [resource] }), + }); + writeTree(after, { + 'skills/manifest.json': JSON.stringify({ buildTimestamp: '2026-08-09T00:00:00Z', resources: [resource] }), + }); + expect((await diffDistTrees(before, after)).changes).toEqual([]); + + writeTree(after, { + 'skills/manifest.json': JSON.stringify({ + buildTimestamp: '2026-08-09T00:00:00Z', + resources: [resource, { id: 'new-skill', uri: 'posthog://skills/new/new-skill' }], + }), + }); + expect((await diffDistTrees(before, after)).changes).toEqual([ + { path: 'skills/manifest.json', kind: 'changed', deltaHash: expect.any(String) }, + ]); + }); + + it('reports unknown future artifact types instead of skipping them (totality)', async () => { + writeTree(before, { 'desktop/blob.bin': 'v1' }); + writeTree(after, { 'desktop/blob.bin': 'v2', 'desktop/extra.dat': 'new' }); + + expect((await diffDistTrees(before, after)).changes).toEqual([ + { path: 'desktop/blob.bin', kind: 'changed', deltaHash: expect.any(String) }, + { path: 'desktop/extra.dat', kind: 'added' }, + ]); + }); + + it('normalizes recursively inside aggregate zips (inner manifest stamps, inner zip mtimes)', async () => { + const innerSkill = { 'SKILL.md': 'stable' }; + writeTree(before, { + 'skills-mcp-resources.zip': await zipBuffer({ + 'manifest.json': JSON.stringify({ buildTimestamp: '2026-08-01T00:00:00Z', resources: [] }), + 'web-analytics.zip': await zipBuffer(innerSkill, { date: new Date('2020-01-01T00:00:00Z') }), + }), + }); + writeTree(after, { + 'skills-mcp-resources.zip': await zipBuffer({ + 'manifest.json': JSON.stringify({ buildTimestamp: '2026-08-09T00:00:00Z', resources: [] }), + 'web-analytics.zip': await zipBuffer(innerSkill, { date: new Date('2026-08-09T12:00:00Z') }), + }), + }); + + expect((await diffDistTrees(before, after)).changes).toEqual([]); + }); +}); + +describe('renderComment', () => { + it('renders a cliEntry addition under the wizard surface, with unchanged assertions for the rest', async () => { + const menu = entries => JSON.stringify({ cliEntries: entries }); + const base = [{ skillId: 'audit-events', role: 'command', parentCommand: 'audit', command: 'events' }]; + writeTree(before, { 'skills/skill-menu.json': menu(base) }); + writeTree(after, { + 'skills/skill-menu.json': menu([ + ...base, + { skillId: 'creating-product-tours', role: 'skill' }, + ]), + }); + + const model = await diffDistTrees(before, after); + const comment = renderComment(model); + + expect(comment).toContain('creating-product-tours'); + expect(comment).toMatch(/\+.*creating-product-tours.*skill/); + expect(comment).toMatch(/Skill content.*✓/i); + expect(comment).toMatch(/Marketplace.*✓/i); + expect(comment.split('\n').length).toBeLessThanOrEqual(40); + }); + + it('groups zips sharing an identical inner delta and keeps the comment within budget', async () => { + const beforeFiles = {}; + const afterFiles = {}; + // 45 variants all pick up the same shared-doc change... + for (let i = 0; i < 45; i++) { + const id = `ai-observability-variant-${i}`; + beforeFiles[`skills/${id}.zip`] = await zipBuffer({ 'SKILL.md': `# ${id}`, 'references/auth.md': 'old auth' }); + afterFiles[`skills/${id}.zip`] = await zipBuffer({ 'SKILL.md': `# ${id}`, 'references/auth.md': 'new auth' }); + } + // ...and one unrelated zip changes differently. + beforeFiles['skills/web-analytics.zip'] = await zipBuffer({ 'SKILL.md': 'wa v1' }); + afterFiles['skills/web-analytics.zip'] = await zipBuffer({ 'SKILL.md': 'wa v2' }); + writeTree(before, beforeFiles); + writeTree(after, afterFiles); + + const comment = renderComment(await diffDistTrees(before, after)); + + expect(comment).toMatch(/45 zips.*identical.*references\/auth\.md/i); + expect(comment).toContain('web-analytics.zip'); + expect(comment.split('\n').length).toBeLessThanOrEqual(40); + + const full = renderFull(await diffDistTrees(before, after)); + for (let i = 0; i < 45; i++) expect(full).toContain(`ai-observability-variant-${i}.zip`); + }); + + it('does not group a zip entry changed to the literal "absent" with a removed entry', async () => { + writeTree(before, { + 'skills/a.zip': await zipBuffer({ 'SKILL.md': 'keep', 'doc.md': 'old' }), + 'skills/b.zip': await zipBuffer({ 'SKILL.md': 'keep', 'doc.md': 'old' }), + }); + writeTree(after, { + 'skills/a.zip': await zipBuffer({ 'SKILL.md': 'keep', 'doc.md': 'absent' }), // content change + 'skills/b.zip': await zipBuffer({ 'SKILL.md': 'keep' }), // entry removed + }); + + const model = await diffDistTrees(before, after); + const [a, b] = model.changes; + expect(a.deltaHash).not.toEqual(b.deltaHash); + expect(renderComment(model)).not.toMatch(/2 zips.*identical/i); + }); + + it('groups many zips touching the same inner files with differing contents (skill rebuild)', async () => { + const beforeFiles = {}, afterFiles = {}; + for (const id of ['anthropic', 'openai', 'mistral', 'groq', 'cohere']) { + beforeFiles[`skills/ai-observability-${id}.zip`] = await zipBuffer({ 'SKILL.md': `old ${id}`, 'references/setup.md': `old setup ${id}` }); + afterFiles[`skills/ai-observability-${id}.zip`] = await zipBuffer({ 'SKILL.md': `new ${id}`, 'references/setup.md': `new setup ${id}` }); + } + // Two unrelated zips with the same inner-file signature must stay itemized (below threshold). + for (const id of ['quack', 'omnibus']) { + beforeFiles[`skills/${id}.zip`] = await zipBuffer({ 'SKILL.md': `old ${id}` }); + afterFiles[`skills/${id}.zip`] = await zipBuffer({ 'SKILL.md': `new ${id}` }); + } + writeTree(before, beforeFiles); + writeTree(after, afterFiles); + + const comment = renderComment(await diffDistTrees(before, after)); + + expect(comment).toMatch(/5 zips changed — same files touched.*SKILL\.md, references\/setup\.md/i); + expect(comment).toMatch(/^~ quack\.zip/m); + expect(comment).toMatch(/^~ omnibus\.zip/m); + }); + + it('collapses the new-variant shape: added files group by directory, small identical-delta pairs merge by file name', async () => { + const beforeFiles = {}, afterFiles = {}; + // 5 files added in one new plugin directory... + for (const f of ['SKILL.md', 'references/cli.md', 'references/go.md', 'references/upload.md', 'references/COMMANDMENTS.md']) { + afterFiles[`marketplace/plugins/posthog-uploads/skills/go/${f}`] = `new ${f}`; + } + // ...and 5 variants whose SKILL.md changed identically in their two plugin copies (5 pairs, distinct per variant). + for (const id of ['android', 'ios', 'react', 'node', 'python']) { + for (const loc of ['posthog-all/skills', 'posthog-uploads/skills']) { + beforeFiles[`marketplace/plugins/${loc}/${id}/SKILL.md`] = `old ${id}`; + afterFiles[`marketplace/plugins/${loc}/${id}/SKILL.md`] = `new ${id}`; + } + } + writeTree(before, beforeFiles); + writeTree(after, { ...afterFiles }); + + const comment = renderComment(await diffDistTrees(before, after)); + + expect(comment).toMatch(/^\+ plugins\/posthog-uploads\/skills\/go\/ — 5 files added/im); + expect(comment).toMatch(/10 files changed — same file name \(SKILL\.md\)/i); + expect(comment.split('\n').length).toBeLessThanOrEqual(22); + }); + + it('groups plain files sharing an identical content delta (marketplace fan-out)', async () => { + const beforeFiles = {}, afterFiles = {}; + for (const id of ['android', 'angular', 'django']) { + beforeFiles[`marketplace/plugins/posthog-all/skills/integration-${id}/references/4-conclude.md`] = 'old conclusion'; + afterFiles[`marketplace/plugins/posthog-all/skills/integration-${id}/references/4-conclude.md`] = 'new conclusion'; + } + beforeFiles['marketplace/plugins/posthog-all/skills/quack/SKILL.md'] = 'unrelated old'; + afterFiles['marketplace/plugins/posthog-all/skills/quack/SKILL.md'] = 'unrelated new'; + writeTree(before, beforeFiles); + writeTree(after, afterFiles); + + const comment = renderComment(await diffDistTrees(before, after)); + + expect(comment).toMatch(/3 files changed — identical delta.*4-conclude\.md/i); + expect(comment).toMatch(/^~ plugins\/posthog-all\/skills\/quack\/SKILL\.md/m); // single-file dir collapses + // Members live only inside the collapsed
, never as diff-block lines. + expect(comment).not.toMatch(/^[-+~ ] .*integration-angular/m); + }); + + it('reports a modified cliEntry (same identity, changed fields)', async () => { + const menu = entries => JSON.stringify({ cliEntries: entries }); + writeTree(before, { + 'skills/skill-menu.json': menu([{ skillId: 'audit-events', role: 'command', parentCommand: 'audit', command: 'events', default: false }]), + }); + writeTree(after, { + 'skills/skill-menu.json': menu([{ skillId: 'audit-events', role: 'command', parentCommand: 'audit', command: 'events', default: true }]), + }); + + const comment = renderComment(await diffDistTrees(before, after)); + expect(comment).toMatch(/^~ cliEntry audit-events updated/m); + }); + + it('full report names identical-delta groups by the changed file, hunk inline, members collapsed', async () => { + const beforeFiles = {}, afterFiles = {}; + for (const id of ['anthropic', 'openai', 'mistral', 'groq', 'cohere']) { + beforeFiles[`skills/ai-observability-${id}.zip`] = await zipBuffer({ 'SKILL.md': `keep ${id}`, 'references/setup.md': 'old setup' }); + afterFiles[`skills/ai-observability-${id}.zip`] = await zipBuffer({ 'SKILL.md': `keep ${id}`, 'references/setup.md': 'new setup' }); + } + writeTree(before, beforeFiles); + writeTree(after, afterFiles); + + const full = renderFull(await diffDistTrees(before, after)); + + // The group is titled by WHAT changed, not by 68 repeating zip names... + expect(full).toMatch(/`references\/setup\.md` — changed identically in 5 zips/); + // ...the hunk appears once, inline (not inside
)... + expect(full.match(/-old setup/g)).toHaveLength(1); + expect(full.indexOf('-old setup')).toBeLessThan(full.indexOf('
')); + // ...and the member list is what's collapsed. + expect(full).toMatch(/
5 archives<\/summary>skills\/ai-observability-anthropic\.zip/); + }); + + it('full report shows content-level hunks for changed files, once per identical-delta group', async () => { + const beforeFiles = {}, afterFiles = {}; + for (const id of ['android', 'angular', 'django']) { + beforeFiles[`marketplace/plugins/posthog-all/skills/integration-${id}/references/4-conclude.md`] = 'old conclusion\nshared line'; + afterFiles[`marketplace/plugins/posthog-all/skills/integration-${id}/references/4-conclude.md`] = 'new conclusion\nshared line'; + } + beforeFiles['skills/web-analytics.zip'] = await zipBuffer({ 'SKILL.md': 'zip v1' }); + afterFiles['skills/web-analytics.zip'] = await zipBuffer({ 'SKILL.md': 'zip v2' }); + writeTree(before, beforeFiles); + writeTree(after, afterFiles); + + const full = renderFull(await diffDistTrees(before, after)); + + expect(full).toContain('-old conclusion'); + expect(full).toContain('+new conclusion'); + // Identical delta across 3 files → the hunk is shown exactly once. + expect(full.match(/-old conclusion/g)).toHaveLength(1); + // Zip inner content is diffed too. + expect(full).toContain('-zip v1'); + expect(full).toContain('+zip v2'); + }); + + it('does not fold root-level files into directory buckets (empty-dir key is a prefix of every key)', async () => { + const afterFiles = {}; + for (let i = 0; i < 3; i++) afterFiles[`root-${i}.bin`] = `r${i}`; + for (let i = 0; i < 4; i++) afterFiles[`deep/nested/f${i}.bin`] = `n${i}`; + writeTree(before, { 'keep.txt': 'x' }); + writeTree(after, { 'keep.txt': 'x', ...afterFiles }); + + const comment = renderComment(await diffDistTrees(before, after)); + + expect(comment).toMatch(/deep\/nested\/ — 4 files added/); + expect(comment).not.toMatch(/^\+ {2}— \d+ files/m); // no nameless bucket + expect(comment).toMatch(/^\+ root-0\.bin/m); // root files itemized + }); + + it('keeps every surface assertion visible when the budget forces truncation', async () => { + const beforeFiles = {}, afterFiles = {}; + // Distinct inner-file names per zip, so no grouping tier can collapse them. + for (let i = 0; i < 60; i++) { + beforeFiles[`skills/skill-${i}.zip`] = await zipBuffer({ [`ref-${i}.md`]: `old ${i}` }); + afterFiles[`skills/skill-${i}.zip`] = await zipBuffer({ [`ref-${i}.md`]: `new ${i}` }); + } + writeTree(before, beforeFiles); + writeTree(after, afterFiles); + + const comment = renderComment(await diffDistTrees(before, after), { fullReportUrl: 'https://x' }); + const lines = comment.split('\n'); + + expect(lines.length).toBeLessThanOrEqual(40); + // Truncation must spend block content, never the surface assertions. + for (const title of ['Wizard surface', 'Marketplace', 'Agents', 'MCP manifest', 'Skills-repo mirror']) { + expect(comment).toContain(`**${title}** ✓ unchanged`); + } + expect(comment).toMatch(/more .*full report/i); + expect(lines.filter(l => l.startsWith('```')).length % 2).toBe(0); // fences balanced + }); + + it('caps content hunks by decompressed inner-entry size, not archive size', async () => { + const big = 'line\n'.repeat(80_000); // ~400KB decompressed, tiny compressed + writeTree(before, { 'skills/a.zip': await zipBuffer({ 'huge.md': big }) }); + writeTree(after, { 'skills/a.zip': await zipBuffer({ 'huge.md': `${big}tail\n` }) }); + + const full = renderFull(await diffDistTrees(before, after)); + expect(full).toMatch(/too large to diff/i); + }); + + it('full report shows content for added and removed files', async () => { + writeTree(before, { 'skills/old-note.md': 'goodbye content' }); + writeTree(after, { 'skills/new-note.md': 'hello content' }); + + const full = renderFull(await diffDistTrees(before, after)); + expect(full).toContain('+hello content'); + expect(full).toContain('-goodbye content'); + }); + + it('summarizes the aggregate bundle as one verified line when its changes match its constituents', async () => { + const oldSkill = { 'SKILL.md': 'v1' }; + const newSkill = { 'SKILL.md': 'v2' }; + writeTree(before, { + 'skills/web-analytics.zip': await zipBuffer(oldSkill), + 'skills-mcp-resources.zip': await zipBuffer({ + 'manifest.json': JSON.stringify({ resources: [] }), + 'web-analytics.zip': await zipBuffer(oldSkill), + }), + }); + writeTree(after, { + 'skills/web-analytics.zip': await zipBuffer(newSkill), + 'skills-mcp-resources.zip': await zipBuffer({ + 'manifest.json': JSON.stringify({ resources: [] }), + 'web-analytics.zip': await zipBuffer(newSkill), + }), + }); + + const model = await diffDistTrees(before, after); + const comment = renderComment(model); + const full = renderFull(model); + + expect(comment).toMatch(/skills-mcp-resources\.zip — aggregate, consistent with/i); + expect(full).toMatch(/^✓ consistent with its constituents/m); // plaintext line, not a code block + expect(full).not.toMatch(/```diff\n\(consistent/); + // The constituent's hunk appears once (its own section), not again for the aggregate. + expect(full.match(/-v1/g)).toHaveLength(1); + }); + + it('goes loud when the aggregate contains a change no constituent explains', async () => { + writeTree(before, { + 'skills-mcp-resources.zip': await zipBuffer({ 'phantom.zip': await zipBuffer({ 'SKILL.md': 'v1' }) }), + }); + writeTree(after, { + 'skills-mcp-resources.zip': await zipBuffer({ 'phantom.zip': await zipBuffer({ 'SKILL.md': 'v2' }) }), + }); + + const model = await diffDistTrees(before, after); + expect(renderComment(model)).toMatch(/⚠️.*phantom\.zip/); + expect(renderFull(model)).toMatch(/⚠️.*diverges/i); + }); + + it('golden: the PR #330 shape — menu/marketplace/mirror changes surface, stamp and mtime noise does not', async () => { + const menu = entries => JSON.stringify({ cliEntries: entries }); + const baseEntries = [{ skillId: 'audit-events', role: 'command', parentCommand: 'audit', command: 'events' }]; + const zipContent = { 'SKILL.md': '# stable skill' }; + + writeTree(before, { + 'skills/skill-menu.json': menu(baseEntries), + 'skills/manifest.json': JSON.stringify({ buildTimestamp: '2026-08-01T00:00:00Z', resources: [] }), + 'push-manifest.json': JSON.stringify({ plugins: [] }), + 'marketplace/.claude-plugin/marketplace.json': JSON.stringify({ plugins: [] }), + 'skills/web-analytics.zip': await zipBuffer(zipContent, { date: new Date('2020-01-01T00:00:00Z') }), + }); + writeTree(after, { + 'skills/skill-menu.json': menu([...baseEntries, { skillId: 'creating-product-tours', role: 'skill' }]), + 'skills/manifest.json': JSON.stringify({ buildTimestamp: '2026-08-09T00:00:00Z', resources: [] }), + 'push-manifest.json': JSON.stringify({ plugins: [{ name: 'posthog-product-tours' }] }), + 'marketplace/.claude-plugin/marketplace.json': JSON.stringify({ plugins: [{ name: 'posthog-product-tours' }] }), + 'marketplace/plugins/posthog-product-tours/SKILL.md': '# product tours', + 'skills/web-analytics.zip': await zipBuffer(zipContent, { date: new Date('2026-08-09T12:00:00Z') }), + }); + + const comment = renderComment(await diffDistTrees(before, after)); + + expect(comment).toMatch(/^\+ cliEntry creating-product-tours \(role: skill\)/m); + expect(comment).toMatch(/^~ \.claude-plugin\/marketplace\.json/m); // marketplace index, gray + expect(comment).toMatch(/^\+ plugins\/posthog-product-tours\/SKILL\.md/m); // new plugin file, green, chain-collapsed + expect(comment).toContain('push-manifest.json'); + expect(comment).toMatch(/Skill content.*✓ unchanged/i); // mtime-only zip filtered + expect(comment).toMatch(/MCP manifest.*✓ unchanged/i); // buildTimestamp-only filtered + + const linked = renderComment(await diffDistTrees(before, after), { + fullReportUrl: 'https://github.com/PostHog/context-mill/actions/runs/123', + }); + expect(linked).toContain('[Full report](https://github.com/PostHog/context-mill/actions/runs/123)'); + }); +});