From 8075d1baa58bb0618b810e728fbe2b5c994390f6 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Tue, 6 Oct 2026 23:35:48 +0900 Subject: [PATCH] Skip codegen locks in workspace checks The workspace protocol walk can see the vocabulary codegen lock just before codegen removes it, causing intermittent NotFound failures in mise check. Exclude this temporary directory from the walk. Extract the manifest walk for deterministic regression tests covering existing and disappearing locks while preserving real manifest discovery and the checker's command-line behavior. Changelog: none Assisted-by: Codex:gpt-6.1-sol --- scripts/check_workspace_protocol.test.ts | 69 +++++++++++++++++ scripts/check_workspace_protocol.ts | 99 +++++++++++++----------- 2 files changed, 122 insertions(+), 46 deletions(-) create mode 100644 scripts/check_workspace_protocol.test.ts diff --git a/scripts/check_workspace_protocol.test.ts b/scripts/check_workspace_protocol.test.ts new file mode 100644 index 000000000..4e7897bbd --- /dev/null +++ b/scripts/check_workspace_protocol.test.ts @@ -0,0 +1,69 @@ +import { deepStrictEqual } from "node:assert/strict"; +import { it, mock } from "node:test"; +import { join } from "@std/path"; +import { walkPackageManifests } from "./check_workspace_protocol.ts"; + +async function withProject( + run: (root: string, parent: string, lock: string) => Promise, +) { + const root = await Deno.makeTempDir(); + const parent = join(root, "packages", "vocab"); + const lock = join(parent, ".vocab-codegen.lock"); + try { + await Deno.mkdir(lock, { recursive: true }); + await Deno.writeTextFile( + join(parent, "package.json"), + JSON.stringify({ dependencies: { "@fedify/fedify": "workspace:" } }), + ); + await run(root, parent, lock); + } finally { + mock.restoreAll(); + await Deno.remove(root, { recursive: true }); + } +} + +it("skips codegen locks while still yielding real package manifests", async () => { + await withProject(async (root, parent, lock) => { + await Deno.writeTextFile(join(lock, "package.json"), "{}"); + const readDir = Deno.readDir; + mock.method(Deno, "readDir", (path: string | URL) => { + if (path === lock) throw new Error("Lock must not be traversed"); + return readDir(path); + }); + const entries = await Array.fromAsync(walkPackageManifests(root)); + deepStrictEqual(entries.map((entry) => entry.path), [ + join(parent, "package.json"), + ]); + }); +}); + +it("continues checking manifests after codegen releases its lock", async () => { + await withProject(async (root, parent, lock) => { + const readDir = Deno.readDir; + let released = false; + mock.method(Deno, "readDir", async function* (path: string | URL) { + if (path !== parent) { + yield* readDir(path); + return; + } + const entries = await Array.fromAsync(readDir(path)); + // Visit the disappearing lock before the real manifest deterministically. + entries.sort((a, b) => + Number(b.name === ".vocab-codegen.lock") - + Number(a.name === ".vocab-codegen.lock") + ); + for (const entry of entries) { + if (entry.name === ".vocab-codegen.lock") { + await Deno.remove(lock, { recursive: true }); + released = true; + } + yield entry; + } + }); + const entries = await Array.fromAsync(walkPackageManifests(root)); + deepStrictEqual(released, true); + deepStrictEqual(entries.map((entry) => entry.path), [ + join(parent, "package.json"), + ]); + }); +}); diff --git a/scripts/check_workspace_protocol.ts b/scripts/check_workspace_protocol.ts index 28b3f9aae..2b5c895df 100644 --- a/scripts/check_workspace_protocol.ts +++ b/scripts/check_workspace_protocol.ts @@ -18,11 +18,9 @@ const DEPENDENCY_FIELDS = [ "optionalDependencies", ] as const; -const projectRoot = resolve(dirname(fromFileUrl(import.meta.url)), ".."); - -let found = false; -for await ( - const entry of walk(projectRoot, { +/** Walk package manifests without entering temporary codegen locks. */ +export function walkPackageManifests(projectRoot: string) { + return walk(projectRoot, { includeDirs: false, // Match the path separator with a character class so these patterns stay // valid on Windows too, where @std/path's SEPARATOR is a backslash and @@ -31,55 +29,64 @@ for await ( skip: [ /(?:^|[/\\])node_modules(?:[/\\]|$)/, /(?:^|[/\\])\.git(?:[/\\]|$)/, + // Codegen removes its lock on completion, even when generation is skipped. + /(?:^|[/\\])\.vocab-codegen\.lock(?:[/\\]|$)/, ], - }) -) { - let manifest: Record; - try { - const parsed = JSON.parse(await Deno.readTextFile(entry.path)); - // A package.json could be `null`, a string, a number, or an array; skip - // anything that is not a plain object so the field lookups below are safe. - if ( - parsed === null || typeof parsed !== "object" || Array.isArray(parsed) - ) { + }); +} + +if (import.meta.main) { + const projectRoot = resolve(dirname(fromFileUrl(import.meta.url)), ".."); + + let found = false; + for await (const entry of walkPackageManifests(projectRoot)) { + let manifest: Record; + try { + const parsed = JSON.parse(await Deno.readTextFile(entry.path)); + // A package.json could be `null`, a string, a number, or an array; skip + // anything that is not a plain object so the field lookups below are safe. + if ( + parsed === null || typeof parsed !== "object" || Array.isArray(parsed) + ) { + continue; + } + manifest = parsed as Record; + } catch { continue; } - manifest = parsed as Record; - } catch { - continue; - } - const invalid: string[] = []; - for (const field of DEPENDENCY_FIELDS) { - const deps = manifest[field]; - // typeof [] is "object", so exclude arrays explicitly before iterating. - if (deps == null || typeof deps !== "object" || Array.isArray(deps)) { - continue; + const invalid: string[] = []; + for (const field of DEPENDENCY_FIELDS) { + const deps = manifest[field]; + // typeof [] is "object", so exclude arrays explicitly before iterating. + if (deps == null || typeof deps !== "object" || Array.isArray(deps)) { + continue; + } + for ( + const [name, spec] of Object.entries(deps as Record) + ) { + if (spec === "workspace:") invalid.push(name); + } } - for ( - const [name, spec] of Object.entries(deps as Record) - ) { - if (spec === "workspace:") invalid.push(name); + + if (invalid.length > 0) { + if (!found) { + console.error( + "Error: Found invalid workspace: specifiers (missing *, ^, or ~):", + ); + console.error(""); + found = true; + } + console.error(`${relative(projectRoot, entry.path)}:`); + for (const name of invalid) console.error(` ${name}`); } } - if (invalid.length > 0) { - if (!found) { - console.error( - "Error: Found invalid workspace: specifiers (missing *, ^, or ~):", - ); - console.error(""); - found = true; - } - console.error(`${relative(projectRoot, entry.path)}:`); - for (const name of invalid) console.error(` ${name}`); + if (found) { + console.error(""); + console.error("Valid formats: workspace:*, workspace:^, workspace:~"); + Deno.exit(1); } -} -if (found) { - console.error(""); - console.error("Valid formats: workspace:*, workspace:^, workspace:~"); - Deno.exit(1); + console.log("All workspace: specifiers are valid"); } - -console.log("All workspace: specifiers are valid");