From f71a4e797e290030a5feb70dca2457409752a96d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=98=93=E8=89=AF?= <1204183885@qq.com> Date: Mon, 14 Sep 2026 04:45:19 +0000 Subject: [PATCH] chore(pnpm): scope build approvals, pin the ACP zod peer, add a freshness lane (#11797) Follow-ups left open when #11625 merged: its three unresolved review threads, the deferred finding in #11681, and #10524. - allowBuilds decisions now honour the whole key the way pnpm does: a bare name covers every version, an exact `name@version` (or a `||` union) covers only those, and a range or source-like key decides nothing. Reducing a key to its name let an approval scoped to one version cover the next (R1-10). - Pin `@agentclientprotocol/sdk>zod` to 3.25.76 so a pnpm worktree runs the zod the npm tree installs. Under zod 4 the SDK's own `instanceof z.ZodError` test stops matching and a malformed request answers -32602 instead of -32603; both versions are npm-locked, so the agreement gate cannot see the split (R1-1). - Cover web-shell's shipped client sources with the undeclared-import rule; it is the one published package whose sources are not under src/. Measured cost today: zero violations (R2-6). - Add a path-filtered workflow that regenerates pnpm-lock.yaml with `corepack pnpm import` and fails when the committed file differs, which is the freshness half set membership cannot cover (#11681). - setup-worktree: accept only an executable file on PATH, so a corepack left without its exec bit reports the actionable error instead of EACCES (#10524). - Name the esbuild divergence an accepted parity gap, and record what `fsevents: false` costs a macOS worktree. --- .github/workflows/.size-baseline | 1 + .github/workflows/pnpm-lock-freshness.yml | 83 +++++++++++++++++++++++ eslint.config.js | 3 + pnpm-lock.yaml | 13 ++-- pnpm-workspace.yaml | 15 +++- scripts/check-lockfile.js | 64 +++++++++++------ scripts/setup-worktree.js | 27 +++++++- scripts/tests/package-scripts.test.js | 80 +++++++++++++++++++++- 8 files changed, 252 insertions(+), 34 deletions(-) create mode 100644 .github/workflows/pnpm-lock-freshness.yml diff --git a/.github/workflows/.size-baseline b/.github/workflows/.size-baseline index 1a35ddf2c3..a2205f961f 100644 --- a/.github/workflows/.size-baseline +++ b/.github/workflows/.size-baseline @@ -32,6 +32,7 @@ 5950 live-host.yml 7642 main-ci-failure-issue.yml 1686 npm-cache.yml +2709 pnpm-lock-freshness.yml 2489 pnpm-worktree-smoke.yml 7299 pr-force-push-reminder.yml 6495 pr-self-report-label.yml diff --git a/.github/workflows/pnpm-lock-freshness.yml b/.github/workflows/pnpm-lock-freshness.yml new file mode 100644 index 0000000000..ee3857b886 --- /dev/null +++ b/.github/workflows/pnpm-lock-freshness.yml @@ -0,0 +1,83 @@ +name: 'pnpm Lock Freshness' + +# The version agreement gate in scripts/check-lockfile.js is set membership: +# it catches a pnpm resolution npm never locked, but not a stale one whose +# superseded version still exists elsewhere in npm's tree. Only regenerating +# the lockfile proves it is current, and that needs registry metadata — so it +# runs here, on dependency changes only, instead of in the full-profile lane +# every non-docs PR pays for. + +on: + pull_request: + branches: + - 'main' + - 'release/**' + paths: + - '.github/workflows/pnpm-lock-freshness.yml' + - '.npmrc' + - '.pnpmfile.mjs' + - 'package.json' + - 'package-lock.json' + - 'packages/*/package.json' + - '!packages/desktop-shell/package.json' + - '!packages/live-host/package.json' + - 'packages/channels/*/package.json' + - 'integrations/*/package.json' + - 'pnpm-lock.yaml' + - 'pnpm-workspace.yaml' + push: + branches: + - 'main' + paths: + - '.github/workflows/pnpm-lock-freshness.yml' + - '.npmrc' + - '.pnpmfile.mjs' + - 'package.json' + - 'package-lock.json' + - 'packages/*/package.json' + - '!packages/desktop-shell/package.json' + - '!packages/live-host/package.json' + - 'packages/channels/*/package.json' + - 'integrations/*/package.json' + - 'pnpm-lock.yaml' + - 'pnpm-workspace.yaml' + workflow_dispatch: + +concurrency: + group: '${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}' + cancel-in-progress: "${{ github.event_name == 'pull_request' }}" + +permissions: + contents: 'read' + +jobs: + import: + name: 'Lockfile freshness' + runs-on: 'ubuntu-latest' + timeout-minutes: 15 + steps: + - name: 'Checkout' + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 + + - name: 'Set up Node.js 22.x' + uses: 'actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e' # v6.4.0 + with: + node-version: '22.x' + + - name: 'Regenerate the pnpm lockfile from package-lock.json' + env: + COREPACK_ENABLE_DOWNLOAD_PROMPT: '0' + run: 'corepack pnpm import' + + - name: 'Compare with the committed lockfile' + shell: 'bash' + run: | + if git diff --quiet -- pnpm-lock.yaml; then + echo 'pnpm-lock.yaml is what corepack pnpm import produces.' + exit 0 + fi + echo 'pnpm-lock.yaml is stale: it is not what `corepack pnpm import`' + echo 'produces from the committed package-lock.json. Update the npm' + echo 'lockfile first, then run `corepack pnpm import` and commit both.' + git diff --stat -- pnpm-lock.yaml + exit 1 diff --git a/eslint.config.js b/eslint.config.js index 56d9b0d0a1..4f7edd2bcb 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -245,6 +245,9 @@ export default tseslint.config( files: [ 'packages/**/src/**/*.{ts,tsx}', 'integrations/**/src/**/*.{ts,tsx}', + // web-shell is published and keeps its shipped sources in client/, not + // src/, so the globs above reach none of it. + 'packages/web-shell/client/**/*.{ts,tsx}', ], ignores: extraneousDependencyTestFiles, rules: { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 1711ed80be..c61b0fa9c9 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -18,6 +18,7 @@ overrides: '@types/react-dom': ^19.2.0 axios: ^1.19.0 react-markdown: ^9.0.0 + '@agentclientprotocol/sdk>zod': 3.25.76 pnpmfileChecksum: sha256-YMmpx+dQrBqa8zfJspP6zukr7YJJ5XNi4HD4BYo9SzM= @@ -246,7 +247,7 @@ importers: dependencies: '@agentclientprotocol/sdk': specifier: ^0.14.1 - version: 0.14.1(zod@4.4.3) + version: 0.14.1(zod@3.25.76) '@qwen-code/qwen-code-core': specifier: workspace:* version: link:../core @@ -284,7 +285,7 @@ importers: dependencies: '@agentclientprotocol/sdk': specifier: ^0.14.1 - version: 0.14.1(zod@4.4.3) + version: 0.14.1(zod@3.25.76) devDependencies: typescript: specifier: 5.8.3 @@ -1132,7 +1133,7 @@ importers: dependencies: '@agentclientprotocol/sdk': specifier: ^0.14.1 - version: 0.14.1(zod@4.4.3) + version: 0.14.1(zod@3.25.76) '@node-rs/jieba': specifier: 2.0.2 version: 2.0.2 @@ -1512,7 +1513,7 @@ packages: '@agentclientprotocol/sdk@0.14.1': resolution: {integrity: sha512-b6r3PS3Nly+Wyw9U+0nOr47bV8tfS476EgyEMhoKvJCZLbgqoDFN7DJwkxL88RR0aiOqOYV1ZnESHqb+RmdH8w==} peerDependencies: - zod: ^3.25.0 || ^4.0.0 + zod: 3.25.76 '@alcalzone/ansi-tokenize@0.3.0': resolution: {integrity: sha512-p+CMKJ93HFmLkjXKlXiVGlMQEuRb6H0MokBSwUsX+S6BRX8eV5naFZpQJFfJHjRZY0Hmnqy1/r6UWl3x+19zYA==} @@ -10244,10 +10245,6 @@ snapshots: dependencies: zod: 3.25.76 - '@agentclientprotocol/sdk@0.14.1(zod@4.4.3)': - dependencies: - zod: 4.4.3 - '@alcalzone/ansi-tokenize@0.3.0': dependencies: ansi-styles: 6.2.3 diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index 0b9346ce98..5082c32f27 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -47,6 +47,15 @@ overrides: '@types/react-dom': '^19.2.0' axios: '^1.19.0' react-markdown: '^9.0.0' + # packages/acp-bridge declares no zod, so pnpm is free to satisfy the SDK's + # `^3.25.0 || ^4.0.0` peer with the highest locked zod while npm serves it + # 3.25.76. Both are npm-locked, so the version agreement gate cannot see the + # split — and it is not cosmetic: the SDK imports `zod` and `zod/v4` from + # different modules, so under zod 4 its own `instanceof z.ZodError` test + # stops matching and a malformed request answers -32602 instead of npm's + # -32603. Pin it until npm moves, so every ACP consumer behaves the way CI + # verifies. + '@agentclientprotocol/sdk>zod': '3.25.76' allowBuilds: '@google/genai': true @@ -55,7 +64,11 @@ allowBuilds: esbuild: true # Darwin-only optional dependency whose install script is only # `node-gyp rebuild`; the macOS smoke install reports no ignored build for - # it, so false records the behaviour pnpm already has. + # it, so false records the behaviour pnpm already has. The consequence is + # that a macOS pnpm worktree runs the prebuilt binding shipped in the + # tarball, and where that is missing, vite, tsx, rollup, playwright and + # tailwindcss's nested chokidar fall back to fs.watch. All of them are + # build and test tooling, never shipped runtime. fsevents: false keytar: true msw: true diff --git a/scripts/check-lockfile.js b/scripts/check-lockfile.js index f7c9811d82..a69bdb2de5 100644 --- a/scripts/check-lockfile.js +++ b/scripts/check-lockfile.js @@ -289,11 +289,35 @@ function npmPackageName(location, details) { return location.slice(location.lastIndexOf(marker) + marker.length); } -// pnpm keys are `name@version` (allowBuilds also accepts `name@spec`); a -// scoped name starts with its own `@`. -function pnpmPackageName(key) { - const at = key.indexOf('@', 1); - return at === -1 ? key : key.slice(0, at); +// pnpm decides a build from the whole allowBuilds key, not from its name +// alone: a bare name covers every version, `name@1.2.3` — or a `||` union of +// exact versions — covers only those, and a source-like key such as +// `name@file:packages/core` is an exact-instance rule no registry dependency +// can match. A range is not a scope at all: pnpm files `esbuild@^0.25.0` +// under a package literally named that, so it decides nothing. Reducing a key +// to its name would let an approval scoped to one version silently cover the +// next version npm installs. +function allowBuildDecisions(allowBuilds) { + const names = new Set(); + const versions = new Set(); + for (const [key, decision] of Object.entries(allowBuilds ?? {})) { + // pnpm records an undecided entry as the string 'set this to true or + // false'; only a boolean runs or skips a build. + if (typeof decision !== 'boolean') continue; + const at = key.indexOf('@', 1); + if (at === -1) { + names.add(key); + continue; + } + const name = key.slice(0, at); + for (const spec of key.slice(at + 1).split('||')) { + const version = spec.trim(); + if (/^\d+\.\d+\.\d+[\w.+-]*$/.test(version)) { + versions.add(`${name}@${version}`); + } + } + } + return { names, versions }; } console.log('Checking pnpm lockfile against package-lock.json...'); @@ -326,10 +350,16 @@ for (const [location, details] of Object.entries(packages)) { // 1.52.0 or form-data moves off mime-types@2. const knownNpmLockGaps = new Set(['mime-db@1.52.0']); -// pnpm dedupes where npm keeps nested copies (npm locks esbuild 0.25.6 at the -// root and 0.25.12 nested; pnpm uses 0.25.12 for both), so the two graphs are -// never equal. The direction that matters is this one: a pnpm worktree must -// not run a dependency version that CI's npm install has not locked. +// The two graphs are never equal, because pnpm dedupes where npm keeps nested +// copies: npm locks esbuild 0.25.6 at the root and 0.25.12 nested, and pnpm +// uses 0.25.12 for both. Read that as an accepted parity gap rather than +// harmless deduplication — a pnpm worktree bundles with a different esbuild +// than CI does, bounded only by npm staying authoritative for build, +// packaging and release. What this gate enforces is the direction that stays +// safe under that gap: a pnpm worktree must not run a dependency version that +// CI's npm install has never locked. Closing the gap itself is what the +// hoisted-parity measurement in docs/verification/pnpm-stage2-evidence/ +// exists to settle. const pnpmVersions = Object.keys(pnpmLockfile?.packages ?? {}); if (pnpmVersions.length === 0) { console.error( @@ -379,13 +409,7 @@ try { // npm runs every dependency install script; pnpm runs one only when // allowBuilds approves it. Requiring an entry for each script npm runs keeps // that difference a reviewed decision instead of a silent one. -const decidedBuilds = new Set( - Object.entries(pnpmWorkspace?.allowBuilds ?? {}) - // pnpm itself records an undecided entry as the string 'set this to true - // or false'; only a boolean runs or skips a build. - .filter(([, decision]) => typeof decision === 'boolean') - .map(([key]) => pnpmPackageName(key)), -); +const decidedBuilds = allowBuildDecisions(pnpmWorkspace?.allowBuilds); const undecidedBuilds = new Set(); for (const [location, details] of Object.entries(packages)) { if ( @@ -395,14 +419,14 @@ for (const [location, details] of Object.entries(packages)) { continue; } const name = npmPackageName(location, details); - if (!decidedBuilds.has(name)) { - undecidedBuilds.add(name); - } + if (decidedBuilds.names.has(name)) continue; + if (decidedBuilds.versions.has(`${name}@${details.version}`)) continue; + undecidedBuilds.add(`${name}@${details.version}`); } if (undecidedBuilds.size > 0) { console.error( - '\nError: these dependencies have install scripts but no allowBuilds entry in pnpm-workspace.yaml; add each with true (run it) or false (skip it):', + '\nError: these dependencies have install scripts but no allowBuilds entry in pnpm-workspace.yaml covering the version npm locks; add each with true (run it) or false (skip it):', ); [...undecidedBuilds].sort().forEach((name) => console.error(`- ${name}`)); process.exitCode = 1; diff --git a/scripts/setup-worktree.js b/scripts/setup-worktree.js index 2fc6a4d494..a54fda6a1b 100644 --- a/scripts/setup-worktree.js +++ b/scripts/setup-worktree.js @@ -5,7 +5,13 @@ */ import { spawnSync } from 'node:child_process'; -import { existsSync, readFileSync } from 'node:fs'; +import { + accessSync, + constants as fsConstants, + existsSync, + readFileSync, + statSync, +} from 'node:fs'; import { constants as osConstants } from 'node:os'; import { delimiter, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; @@ -38,11 +44,28 @@ function pathValue() { return envValue('PATH') ?? ''; } +// A PATH entry only counts when it can actually be executed. `existsSync` +// alone also accepts a directory named `corepack`, or the file a partially +// removed Node toolchain leaves without its exec bit: the spawn below then +// fails with EACCES and the caller loses the actionable message this script +// prints for a missing Corepack. Windows decides executability by extension +// rather than a mode bit, so there the file check is the whole test. +function isExecutableFile(candidate) { + try { + if (!statSync(candidate).isFile()) return false; + if (process.platform === 'win32') return true; + accessSync(candidate, fsConstants.X_OK); + return true; + } catch { + return false; + } +} + function findOnPath(command) { for (const entry of pathValue().split(delimiter)) { const directory = entry.replace(/^"(.*)"$/, '$1'); const candidate = resolve(directory || '.', command); - if (existsSync(candidate)) return candidate; + if (isExecutableFile(candidate)) return candidate; } return undefined; diff --git a/scripts/tests/package-scripts.test.js b/scripts/tests/package-scripts.test.js index 18fb9a1801..02fb53e41a 100644 --- a/scripts/tests/package-scripts.test.js +++ b/scripts/tests/package-scripts.test.js @@ -354,11 +354,11 @@ describe('package scripts', () => { ); }); - it('counts a spec-scoped allowBuilds key as a decision for the package', () => { + it('accepts an allowBuilds key scoped to the version npm locks', () => { const result = runCheckLockfile((fixtureRoot) => mutatePnpmWorkspace(fixtureRoot, (workspace) => { - delete workspace.allowBuilds.esbuild; - workspace.allowBuilds['esbuild@^0.25.0'] = true; + delete workspace.allowBuilds.keytar; + workspace.allowBuilds['keytar@7.9.0'] = true; }), ); @@ -368,6 +368,36 @@ describe('package scripts', () => { ); }); + it('rejects an allowBuilds key scoped to another version', () => { + // pnpm matches a version-scoped key exactly, so an approval written for + // one version says nothing about the one npm actually locks. + const result = runCheckLockfile((fixtureRoot) => + mutatePnpmWorkspace(fixtureRoot, (workspace) => { + delete workspace.allowBuilds.keytar; + workspace.allowBuilds['keytar@7.0.0'] = true; + }), + ); + + expect(result.status).toBe(1); + expect(result.stderr).toContain('no allowBuilds entry'); + expect(result.stderr).toContain('- keytar@7.9.0'); + }); + + it('ignores a range-scoped allowBuilds key, which decides nothing', () => { + // pnpm's isDepPathAllowBuildKey rejects a range, so `esbuild@^0.25.0` + // is filed under a package literally named that and approves no build. + const result = runCheckLockfile((fixtureRoot) => + mutatePnpmWorkspace(fixtureRoot, (workspace) => { + delete workspace.allowBuilds.esbuild; + workspace.allowBuilds['esbuild@^0.25.0'] = true; + }), + ); + + expect(result.status).toBe(1); + expect(result.stderr).toContain('no allowBuilds entry'); + expect(result.stderr).toContain('- esbuild@'); + }); + it('ignores an allowBuilds placeholder that decides nothing', () => { const result = runCheckLockfile((fixtureRoot) => mutatePnpmWorkspace(fixtureRoot, (workspace) => { @@ -382,6 +412,23 @@ describe('package scripts', () => { }); }); + it('checks undeclared imports in web-shell shipped sources', () => { + // web-shell is published and keeps its shipped sources in client/, so the + // src/ globs reach none of it. Pinning the glob inside this block keeps a + // published package from silently dropping out of the check. + const config = readFileSync(path.join(root, 'eslint.config.js'), 'utf8'); + const start = config.indexOf( + 'A package must declare what its own sources import', + ); + const end = config.indexOf('export-html and insight carry', start); + + expect(start).toBeGreaterThan(-1); + expect(end).toBeGreaterThan(start); + expect(config.slice(start, end)).toContain( + "'packages/web-shell/client/**/*.{ts,tsx}'", + ); + }); + it('keeps the internal release-age exception independent of the version', () => { const workspace = parse( readFileSync(path.join(root, 'pnpm-workspace.yaml'), 'utf8'), @@ -826,6 +873,33 @@ describe('package scripts', () => { } }); + it.skipIf(process.platform === 'win32')( + 'ignores a PATH entry that cannot be executed', + () => { + // A `corepack` left without its exec bit by a half-removed toolchain + // used to be chosen anyway, and the spawn then died with EACCES instead + // of the actionable message below. + const binDir = mkdtempSync(path.join(tmpdir(), 'qwen-worktree-noexec-')); + const env = { ...process.env, PATH: binDir }; + + try { + writeFileSync(path.join(binDir, 'corepack'), '#!/bin/sh\nexit 0\n'); + chmodSync(path.join(binDir, 'corepack'), 0o644); + + const result = spawnSync( + process.execPath, + [path.join(root, 'scripts/setup-worktree.js')], + { cwd: root, encoding: 'utf8', env }, + ); + + expect(result.status).toBe(1); + expect(result.stderr).toContain('Corepack is required'); + } finally { + rmSync(binDir, { recursive: true, force: true }); + } + }, + ); + it('falls back to registry access when the pnpm store is incomplete', () => { const binDir = mkdtempSync(path.join(tmpdir(), 'qwen-worktree-fallback-')); const logFile = path.join(binDir, 'corepack.log');