mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-10-20 13:15:00 +00:00
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.
This commit is contained in:
parent
32a25efa6e
commit
f71a4e797e
8 changed files with 252 additions and 34 deletions
1
.github/workflows/.size-baseline
vendored
1
.github/workflows/.size-baseline
vendored
|
|
@ -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
|
||||
|
|
|
|||
83
.github/workflows/pnpm-lock-freshness.yml
vendored
Normal file
83
.github/workflows/pnpm-lock-freshness.yml
vendored
Normal file
|
|
@ -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
|
||||
|
|
@ -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: {
|
||||
|
|
|
|||
13
pnpm-lock.yaml
generated
13
pnpm-lock.yaml
generated
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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');
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue