From 51ab772b34aa248d8a1e8091e9335d0bd32d0dad Mon Sep 17 00:00:00 2001 From: Yi-111-a <153097222+Yi-111-a@users.noreply.github.com> Date: Sun, 4 Oct 2026 04:43:11 +0800 Subject: [PATCH] fix(scripts): read Pi patch hashes from pnpm's virtual store lockfile check-pi-dependencies and check-pi-patches decided whether a Pi package was installed as pnpm's patched instance by looking for a `patch_hash=` segment in the resolved symlink. pnpm shortens the `.pnpm` entry directory when a path gets too long, which drops that segment, so a correctly patched install was reported as unpatched on Windows. Read the hashes from `node_modules/.pnpm/lock.yaml` instead. pnpm keys every patched snapshot as `name@version(patch_hash=)`, so the lockfile keeps the full hash regardless of how the store directory is named. Both checks now also fail with a distinct message when nothing patched is recorded at all, rather than only when the hash is missing from pnpm-lock.yaml. The fork test in native-pi-session.test.ts derived the publication filename with `foreignPath.split("/")`, which cannot split a Windows path, so it compared a whole `D:\...` path against a bare readdir entry. `path.basename` gives the same answer on POSIX and fixes the comparison on Windows. Regression coverage builds a workspace whose `.pnpm` entry names are shortened the way a long Windows path forces, and asserts both scripts accept it and still reject an unpatched install or a hash pnpm-lock.yaml does not record. fixes #1361 --- .github/workflows/ci.yml | 3 + .../src/native-pi-session.test.ts | 4 +- scripts/check-pi-dependencies.mjs | 26 +- scripts/check-pi-patches.mjs | 44 +++- scripts/pi-patch-hash.mjs | 36 +++ scripts/pi-patch-hash.test.mjs | 226 ++++++++++++++++++ 6 files changed, 329 insertions(+), 10 deletions(-) create mode 100644 scripts/pi-patch-hash.mjs create mode 100644 scripts/pi-patch-hash.test.mjs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f3216a4d31..0e8ab1d7c9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -53,6 +53,9 @@ jobs: ARCHITECTURE_BASE: ${{ github.event.pull_request.base.sha || github.event.before || 'HEAD^' }} run: node scripts/check-architecture.mjs + - name: Unit-test the Pi patch checks + run: node --test scripts/pi-patch-hash.test.mjs + - name: Unit tests run: pnpm -r --if-present test diff --git a/packages/agent-runtime/src/native-pi-session.test.ts b/packages/agent-runtime/src/native-pi-session.test.ts index d9f4b34366..e7cec08e3a 100644 --- a/packages/agent-runtime/src/native-pi-session.test.ts +++ b/packages/agent-runtime/src/native-pi-session.test.ts @@ -1,6 +1,6 @@ import { existsSync, mkdtempSync, mkdirSync, readFileSync, readdirSync, realpathSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; -import { join } from "node:path"; +import { basename, join } from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { AgentSession, ModelRuntime, SessionManager } from "@earendil-works/pi-coding-agent"; import { createAssistantMessageEventStream, type AssistantMessage } from "@earendil-works/pi-ai"; @@ -781,7 +781,7 @@ describe("native fork children", () => { expect(failure?.message).not.toContain(f.group); expect(existsSync(foreignPath)).toBe(true); expect(readFileSync(foreignPath, "utf8")).toContain("collision"); - expect(groupEntries(f.group).sort()).toEqual(before.concat([foreignPath.split("/").at(-1)!]).sort()); + expect(groupEntries(f.group).sort()).toEqual(before.concat([basename(foreignPath)]).sort()); expect(readFileSync(f.file, "utf8")).toBe(parentBytes); } finally { service.disposeAll(); } } finally { diff --git a/scripts/check-pi-dependencies.mjs b/scripts/check-pi-dependencies.mjs index c31cc6acf5..0f03ac6aed 100644 --- a/scripts/check-pi-dependencies.mjs +++ b/scripts/check-pi-dependencies.mjs @@ -1,8 +1,30 @@ +#!/usr/bin/env node +/** + * Require every pinned Pi package to be installed at the target version, and + * the ones we patch to be installed as pnpm's patched instance. + * + * Usage: + * node scripts/check-pi-dependencies.mjs + * node scripts/check-pi-dependencies.mjs --root + * pnpm check:pi-dependencies + */ import { existsSync, readFileSync, realpathSync } from "node:fs"; import { dirname, join, resolve } from "node:path"; import { fileURLToPath } from "node:url"; +import { installedPatchHashes } from "./pi-patch-hash.mjs"; -const root = resolve(dirname(fileURLToPath(import.meta.url)), ".."); +function parseArgs(argv) { + let out = resolve(dirname(fileURLToPath(import.meta.url)), ".."); + for (let i = 0; i < argv.length; i += 1) { + if (argv[i] === "--root" && argv[i + 1]) { + out = resolve(argv[i + 1]); + i += 1; + } + } + return out; +} + +const root = parseArgs(process.argv.slice(2)); const targetVersion = "1.0.1"; function readJson(path) { @@ -25,7 +47,7 @@ function assertInstalled(packagePath, expectedName, { patched = false } = {}) { if (manifest.name !== expectedName || manifest.version !== targetVersion) { throw new Error(`${packagePath} resolves to ${manifest.name}@${manifest.version}, expected ${expectedName}@${targetVersion}`); } - if (patched && !resolved.includes("patch_hash=")) { + if (patched && installedPatchHashes(root, expectedName, targetVersion).length === 0) { throw new Error(`${packagePath} does not resolve to pnpm's patched package instance`); } } diff --git a/scripts/check-pi-patches.mjs b/scripts/check-pi-patches.mjs index 2164f4f812..6e56f3c71d 100644 --- a/scripts/check-pi-patches.mjs +++ b/scripts/check-pi-patches.mjs @@ -1,8 +1,30 @@ -import { existsSync, readFileSync, realpathSync } from "node:fs"; +#!/usr/bin/env node +/** + * Require every Pi patch to be mapped in pnpm-workspace.yaml, installed by + * pnpm, recorded in pnpm-lock.yaml, and to still carry the audited contracts. + * + * Usage: + * node scripts/check-pi-patches.mjs + * node scripts/check-pi-patches.mjs --root + * pnpm check:pi-patches + */ +import { existsSync, readFileSync } from "node:fs"; import { dirname, join, resolve } from "node:path"; import { fileURLToPath } from "node:url"; +import { installedPatchHashes } from "./pi-patch-hash.mjs"; -const root = resolve(dirname(fileURLToPath(import.meta.url)), ".."); +function parseArgs(argv) { + let out = resolve(dirname(fileURLToPath(import.meta.url)), ".."); + for (let i = 0; i < argv.length; i += 1) { + if (argv[i] === "--root" && argv[i + 1]) { + out = resolve(argv[i + 1]); + i += 1; + } + } + return out; +} + +const root = parseArgs(process.argv.slice(2)); const targetVersion = "1.0.1"; const entries = [ { @@ -39,10 +61,20 @@ for (const entry of entries) { const workspaceMapping = `'${entry.name}@${targetVersion}': ${entry.patch}`; if (!workspace.includes(workspaceMapping)) throw new Error(`pnpm-workspace.yaml does not map ${entry.name} to ${entry.patch}`); const packagePath = join(root, entry.packagePath); - const resolved = realpathSync(packagePath); - const patchHash = resolved.match(/patch_hash=([a-f0-9]+)/)?.[1]; - if (!patchHash || !lockfile.includes(`${entry.name}@${targetVersion}(patch_hash=${patchHash}`)) { - throw new Error(`${entry.name}@${targetVersion} installed patch hash is absent from pnpm-lock.yaml`); + if (!existsSync(packagePath)) { + throw new Error(`Install dependencies before this check; missing ${entry.packagePath}`); + } + const patchHashes = installedPatchHashes(root, entry.name, targetVersion); + if (patchHashes.length === 0) { + throw new Error(`${entry.name}@${targetVersion} is not installed as a patched instance`); + } + const unlocked = patchHashes.filter( + (patchHash) => !lockfile.includes(`${entry.name}@${targetVersion}(patch_hash=${patchHash}`), + ); + if (unlocked.length > 0) { + throw new Error( + `${entry.name}@${targetVersion} installed patch hash is absent from pnpm-lock.yaml: ${unlocked.join(", ")}`, + ); } } diff --git a/scripts/pi-patch-hash.mjs b/scripts/pi-patch-hash.mjs new file mode 100644 index 0000000000..5a18bfbc5a --- /dev/null +++ b/scripts/pi-patch-hash.mjs @@ -0,0 +1,36 @@ +import { readFileSync } from "node:fs"; +import { join } from "node:path"; + +/** + * pnpm copies the lockfile it actually installed from into the virtual + * store, next to the store entries it describes. + */ +export const VIRTUAL_STORE_LOCKFILE = join("node_modules", ".pnpm", "lock.yaml"); + +function escapeForRegExp(value) { + return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); +} + +/** + * Return the patch hashes pnpm recorded for `name@version` in a lockfile. + * + * Every patched snapshot is keyed `'name@version(patch_hash=)...` + * (single-quoted for scoped names, bare otherwise), so the snapshot keys + * carry the full hash even when the matching `.pnpm` directory name has + * been shortened. + */ +export function patchHashesIn(lockText, name, version) { + const snapshotKey = new RegExp( + `'?${escapeForRegExp(name)}@${escapeForRegExp(version)}\\(patch_hash=([a-f0-9]+)\\)`, + "g", + ); + return [...new Set([...lockText.matchAll(snapshotKey)].map((match) => match[1]))].sort(); +} + +/** + * Patch hashes of the instances pnpm actually installed, read from the + * virtual store's own lockfile. + */ +export function installedPatchHashes(root, name, version) { + return patchHashesIn(readFileSync(join(root, VIRTUAL_STORE_LOCKFILE), "utf8"), name, version); +} \ No newline at end of file diff --git a/scripts/pi-patch-hash.test.mjs b/scripts/pi-patch-hash.test.mjs new file mode 100644 index 0000000000..60dda0cc82 --- /dev/null +++ b/scripts/pi-patch-hash.test.mjs @@ -0,0 +1,226 @@ +import assert from "node:assert/strict"; +import { execFileSync } from "node:child_process"; +import { mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { dirname, join, relative } from "node:path"; +import test from "node:test"; +import { fileURLToPath } from "node:url"; +import { patchHashesIn } from "./pi-patch-hash.mjs"; + +const here = dirname(fileURLToPath(import.meta.url)); +const patchDependencyCheck = join(here, "check-pi-dependencies.mjs"); +const patchCheck = join(here, "check-pi-patches.mjs"); + +const TARGET = "1.0.1"; +const PATCHED = [ + { + name: "@earendil-works/pi-agent-core", + markers: ["hosted_search_update", "localRequestErrorDetails"], + extra: "", + }, + { + name: "@earendil-works/pi-ai", + markers: ["hostedSearch", "withLocalRequestErrors", "AnthropicOAuthTokenError", "Retry-After"], + extra: [ + '+{"openai-completions":{"chat:deepseek-flash":{"baseUrl":"https://api.deepseek.com","compat":{"supportsMidConvoSystemMessages":true}}}}', + "diff --git a/dist/index.d.ts b/dist/index.d.ts", + "diff --git a/dist/types.d.ts b/dist/types.d.ts", + "diff --git a/dist/utils/assistant-message-frame.d.ts b/dist/utils/assistant-message-frame.d.ts", + "diff --git a/dist/utils/estimate.d.ts b/dist/utils/estimate.d.ts", + "diff --git a/dist/utils/hosted-search.d.ts b/dist/utils/hosted-search.d.ts", + "diff --git a/dist/utils/local-request-error.d.ts b/dist/utils/local-request-error.d.ts", + "diff --git a/dist/utils/local-request-stream.d.ts b/dist/utils/local-request-stream.d.ts", + ].join("\n"), + }, + { + name: "@earendil-works/pi-coding-agent", + markers: ["hostedSearchReplayProjection", "estimateProjectedContextTokens"], + extra: "+export declare function estimateProjectedContextTokens(", + }, +]; +const RELEASE_AGE_EXCLUDE = [ + "chord", + "pi-agent-core", + "pi-ai", + "pi-codemode", + "pi-coding-agent", + "pi-mcp", + "pi-telemetry", + "pi-tui", +].map((name) => `@earendil-works/${name}@${TARGET}`); +const HASH = `${"0".repeat(63)}1`; + +function patchPath(name) { + return `patches/${name.replace("@earendil-works/", "@earendil-works__")}@${TARGET}.patch`; +} + +/** + * A `.pnpm` entry name as pnpm writes it once it has to shorten the virtual + * store directory on a long path: the `patch_hash=` segment is gone even + * though the install really is patched. Both checks used to read the hash out + * of the resolved symlink, so they called this install unpatched. + */ +function storeEntryDir(name, hash, shorten) { + const bare = name.replace("@earendil-works/", ""); + return shorten + ? `node_modules/.pnpm/${bare}@${TARGET}_${hash.slice(0, 12)}` + : `node_modules/.pnpm/${bare}@${TARGET}_patch_hash=${hash}`; +} + +function run(script, root) { + try { + const stdout = execFileSync("node", [script, "--root", root], { + encoding: "utf8", + stdio: ["ignore", "pipe", "pipe"], + }); + return { code: 0, stdout, stderr: "" }; + } catch (error) { + return { + code: error.status ?? 1, + stdout: error.stdout?.toString() ?? "", + stderr: error.stderr?.toString() ?? "", + }; + } +} + +function write(root, path, contents) { + const file = join(root, path); + mkdirSync(dirname(file), { recursive: true }); + writeFileSync(file, contents); +} + +function snapshotSection(root, names, hash) { + return `lockfileVersion: '9.0'\n\nsnapshots:\n${ + names.map((name) => ` '${name}@${TARGET}(patch_hash=${hash})': {}`).join("\n") + }\n`; +} + +/** + * Build a workspace that satisfies both checks: correct pins, every patch + * mapped and locked, and the Pi packages installed as patched instances. + */ +function workspace({ + shorten = true, + storeNames = PATCHED.map((entry) => entry.name), + storeHash = HASH, + lockHash = HASH, +} = {}) { + const dir = mkdtempSync(join(tmpdir(), "pi-desktop-patches-")); + + write( + dir, + "packages/agent-runtime/package.json", + JSON.stringify({ + dependencies: Object.fromEntries([...PATCHED.map((e) => e.name), "@earendil-works/pi-mcp"].map((n) => [n, TARGET])), + }), + ); + write( + dir, + "apps/desktop/package.json", + JSON.stringify({ + devDependencies: { "@earendil-works/pi-ai": TARGET, "@earendil-works/pi-mcp": TARGET }, + }), + ); + write(dir, "pnpm-lock.yaml", snapshotSection(dir, PATCHED.map((entry) => entry.name), lockHash)); + write(dir, "node_modules/.pnpm/lock.yaml", snapshotSection(dir, storeNames, storeHash)); + + for (const entry of PATCHED) { + write(dir, patchPath(entry.name), [entry.markers.join("\n"), entry.extra].filter(Boolean).join("\n")); + } + // pnpm-workspace.yaml maps each patch, which is what check-pi-patches reads. + const workspaceYaml = PATCHED.map( + (entry) => `'${entry.name}@${TARGET}': ${patchPath(entry.name)}`, + ).join("\n"); + write( + dir, + "pnpm-workspace.yaml", + `${workspaceYaml}\nminimumReleaseAgeExclude:\n${RELEASE_AGE_EXCLUDE.map((name) => ` - '${name}'`).join("\n")}\n`, + ); + + const link = (from, to) => { + mkdirSync(dirname(from), { recursive: true }); + symlinkSync(relative(dirname(from), to), from, "dir"); + }; + for (const name of PATCHED.map((entry) => entry.name)) { + const entryDir = join(dir, storeEntryDir(name, storeHash, shorten), "node_modules", name); + write(dir, relative(dir, join(entryDir, "package.json")), JSON.stringify({ name, version: TARGET })); + link(join(dir, "packages/agent-runtime/node_modules", name), entryDir); + } + for (const [scope, name] of [ + ["apps/desktop", "@earendil-works/pi-ai"], + ["apps/desktop", "@earendil-works/pi-mcp"], + ]) { + write(dir, `${scope}/node_modules/${name}/package.json`, JSON.stringify({ name, version: TARGET })); + } + return dir; +} + +test("patchHashesIn reads scoped and unscoped snapshot keys, and ignores other versions", () => { + const lock = [ + " '@scope/pkg@2.0.0(patch_hash=abc123)': {}", + " bare@2.0.0(patch_hash=def456)': {}", + " '@scope/pkg@1.0.0(patch_hash=aaa111)(peer@1)': {}", + " '@scope/pkg@1.0.0(patch_hash=aaa111)(peer@2)': {}", + " '@scope/other@1.0.0(patch_hash=bbb222)': {}", + ].join("\n"); + assert.deepEqual(patchHashesIn(lock, "@scope/pkg", "1.0.0"), ["aaa111"]); + assert.deepEqual(patchHashesIn(lock, "bare", "2.0.0"), ["def456"]); + assert.deepEqual(patchHashesIn(lock, "@scope/pkg", "9.9.9"), []); +}); + +test("patchHashesIn treats the version literally", () => { + assert.deepEqual(patchHashesIn(" 'pkg@1x0.0(patch_hash=aaa111)': {}", "pkg", "1.0.0"), []); +}); + +test("check-pi-dependencies accepts a shortened virtual store entry name", () => { + const dir = workspace({ shorten: true }); + try { + const result = run(patchDependencyCheck, dir); + assert.equal(result.code, 0, result.stderr); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +test("check-pi-patches accepts a shortened virtual store entry name", () => { + const dir = workspace({ shorten: true }); + try { + const result = run(patchCheck, dir); + assert.equal(result.code, 0, result.stderr); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +test("check-pi-dependencies still rejects an install with no patched snapshot", () => { + const dir = workspace({ storeNames: [] }); + try { + const result = run(patchDependencyCheck, dir); + assert.equal(result.code, 1); + assert.match(result.stderr, /does not resolve to pnpm's patched package instance/); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +test("check-pi-patches rejects an installed patch hash that pnpm-lock.yaml does not record", () => { + const dir = workspace({ storeHash: HASH, lockHash: "f".repeat(64) }); + try { + const result = run(patchCheck, dir); + assert.equal(result.code, 1); + assert.match(result.stderr, /absent from pnpm-lock\.yaml/); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +test("check-pi-patches rejects a package that is not installed as a patched instance", () => { + const dir = workspace({ storeNames: [] }); + try { + const result = run(patchCheck, dir); + assert.equal(result.code, 1); + assert.match(result.stderr, /is not installed as a patched instance/); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); \ No newline at end of file