From 235def35525449348e3a8b6e6c5903b426124e29 Mon Sep 17 00:00:00 2001 From: Drew Stone Date: Tue, 28 Jul 2026 13:04:56 -0600 Subject: [PATCH] fix(lock): load proper-lockfile lazily so a Worker can import this package MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `proper-lockfile` pulls in `graceful-fs`, which patches Node's `fs` at module scope (`fs.close = ...`, `fs.closeSync = ...`). workerd exposes those as getter-only accessors, so a static import made the assignment throw while Cloudflare ran startup validation on an uploaded Worker: Uncaught TypeError: Cannot set property close of # which has only a getter [code: 10021] The whole Worker is rejected — no request frame, nothing the app can catch — even for bundles that never take a lock. Every downstream consumer inherits it: `@tangle-network/agent-runtime` pins this package and re-exports the knowledge subsystem from its ROOT barrel, which is where a product's chat turn imports `runToolLoop` from. Confirmed by uploading a Worker whose only import is that root barrel. Both call sites are already async, so deferring the module scope to the first lock is behaviour-preserving. `LockOptions` stays a type import. `verify:package` now walks the packed dist and fails on any static import of a module that patches a Node builtin. `wrangler deploy --dry-run` only bundles — it never executes module scope — so it is structurally blind to this class, which is why the regression shipped green. --- CHANGELOG.md | 9 +++++++ package.json | 2 +- scripts/verify-package.mjs | 48 ++++++++++++++++++++++++++++++++++++++ src/mutation-lock.ts | 19 ++++++++++++++- 4 files changed, 76 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 76a2dd8..bbab128 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,14 @@ # Changelog +## 6.1.9 + +### Fixed + +- Load `proper-lockfile` with a dynamic import inside the functions that take a lock, instead of at module scope. + It pulls in `graceful-fs`, which patches Node's `fs` at import time (`fs.close = ...`). + workerd exposes those as getter-only accessors, so the assignment threw while Cloudflare validated an uploaded Worker (`Cannot set property close of # which has only a getter [code: 10021]`), rejecting the whole Worker — including consumers that never take a lock. + `verify:package` now fails on any static import of a module that patches a Node builtin, because `wrangler deploy --dry-run` bundles without executing and cannot see this class of failure. + ## 6.1.8 ### Changed diff --git a/package.json b/package.json index 3c8c7cf..f5dee9a 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@tangle-network/agent-knowledge", - "version": "6.1.8", + "version": "6.1.9", "description": "Build, search, evaluate, and improve source-backed knowledge bases.", "homepage": "https://github.com/tangle-network/agent-knowledge#readme", "repository": { diff --git a/scripts/verify-package.mjs b/scripts/verify-package.mjs index b005666..5ca7821 100644 --- a/scripts/verify-package.mjs +++ b/scripts/verify-package.mjs @@ -6,6 +6,7 @@ import { readFileSync, readdirSync, rmSync, + statSync, writeFileSync, } from 'node:fs' import { tmpdir } from 'node:os' @@ -44,6 +45,18 @@ const forbiddenDeclarationNames = [ 'TwoAgentResearchLoopResult', 'TwoAgentResearchRound', ] +// Packages that patch a Node builtin at MODULE scope. `graceful-fs` assigns +// `fs.close` / `fs.closeSync`; workerd exposes those as getter-only accessors, +// so the assignment throws while Cloudflare validates an uploaded Worker — +// `Cannot set property close of # which has only a getter [code: +// 10021]` — rejecting the entire Worker with no request frame and no way for +// the app to catch it. A STATIC import of any of these anywhere in the shipped +// bundle poisons every downstream Cloudflare consumer, whether or not it ever +// takes a lock; a dynamic `import()` does not, because the module scope only +// runs when the lock is actually needed. `wrangler deploy --dry-run` bundles +// without executing, so it is structurally blind to this class — this check is +// what stands in for it. +const edgeUnsafeStaticImports = ['proper-lockfile', 'graceful-fs'] const requiredMemoryExports = ['runAgentMemoryImprovement'] const requiredAgentEvalExports = ['gepaOptimizationMethod', 'skillOptOptimizationMethod'] const repoRoot = resolve(dirname(fileURLToPath(import.meta.url)), '..') @@ -116,6 +129,7 @@ try { throw new Error(`published declarations include obsolete export: ${name}`) } } + assertNoEdgeUnsafeStaticImports(join(installedPackageDir, 'dist')) const installedAgentEval = JSON.parse( readFileSync( join(appDir, 'node_modules', '@tangle-network', 'agent-eval', 'package.json'), @@ -209,6 +223,40 @@ try { } finally { rmSync(tempRoot, { recursive: true, force: true }) } +function assertNoEdgeUnsafeStaticImports(distDir) { + const offenders = [] + for (const file of javascriptFiles(distDir)) { + const source = readFileSync(file, 'utf8') + for (const specifier of edgeUnsafeStaticImports) { + const quoted = `["']${specifier.replaceAll('/', '\\/')}["']` + // A static ESM import, a re-export, or a CJS require. `import(...)` is + // deliberately excluded: deferring the module scope is the whole fix. + const statik = new RegExp(`(?:from\\s*${quoted}|require\\(\\s*${quoted}\\s*\\))`) + if (statik.test(source)) offenders.push(`${file.slice(distDir.length + 1)} -> ${specifier}`) + } + } + if (offenders.length > 0) { + throw new Error( + [ + 'published bundle statically imports a package that patches a Node builtin at module scope.', + 'Cloudflare rejects the whole Worker on upload with code 10021, and a dry run cannot see it.', + 'Load it with a dynamic import() inside the function that needs it.', + ...offenders.map((offender) => ` - ${offender}`), + ].join('\n'), + ) + } +} + +function javascriptFiles(directory) { + const files = [] + for (const entry of readdirSync(directory)) { + const path = join(directory, entry) + if (statSync(path).isDirectory()) files.push(...javascriptFiles(path)) + else if (/\.(?:js|mjs|cjs)$/.test(entry)) files.push(path) + } + return files +} + function onlyTarball(directory) { const tarballs = readdirSync(directory).filter((name) => name.endsWith('.tgz')) if (tarballs.length !== 1) { diff --git a/src/mutation-lock.ts b/src/mutation-lock.ts index 5593ad8..5132a50 100644 --- a/src/mutation-lock.ts +++ b/src/mutation-lock.ts @@ -1,7 +1,7 @@ import { AsyncLocalStorage } from 'node:async_hooks' import { mkdir, readFile } from 'node:fs/promises' import { dirname, join, resolve } from 'node:path' -import { check, type LockOptions, lock } from 'proper-lockfile' +import type { LockOptions } from 'proper-lockfile' import { isMissingFile, withSafeDescendant, @@ -19,6 +19,21 @@ const DEFAULT_STALE_MS = 15 * 60 * 1000 const DEFAULT_READ_RETRIES = 100 const DEFAULT_READ_WAIT_MS = 25 +// `proper-lockfile` pulls in `graceful-fs`, which patches Node's `fs` at MODULE +// scope (`fs.close = ...`, `fs.closeSync = ...`). workerd exposes those as +// getter-only accessors, so a STATIC import makes the assignment throw while +// Cloudflare runs startup validation on upload — `Cannot set property close of +// # which has only a getter [code: 10021]` — rejecting the whole Worker +// before any request frame exists, uncatchably, even for bundles that never +// take a lock. Loading it on first use keeps this module importable at the +// edge. Every consumer below is already async, so nothing else changes. +let lockfileModule: Promise | undefined + +function loadLockfile(): Promise { + lockfileModule ??= import('proper-lockfile') + return lockfileModule +} + interface KnowledgeMutationScope { active: boolean lock: KnowledgeMutationLock @@ -289,6 +304,7 @@ export async function acquireDurableFileLock( const update = Math.max(1_000, Math.floor(stale / 3)) let compromised: Error | undefined let released = false + const { lock } = await loadLockfile() await mkdir(dirname(options.lockfilePath), { recursive: true }) const release = await lock(target, { lockfilePath: options.lockfilePath, @@ -429,6 +445,7 @@ async function hasActiveMutationLock( options: Pick, ): Promise { try { + const { check } = await loadLockfile() return await withSafeDirectory(root, '.agent-knowledge', false, (cacheDir) => check(root, { lockfilePath: join(cacheDir, 'mutation.lock.durable'),