From 782852144dbf585126276ec40d1136e69d6aa442 Mon Sep 17 00:00:00 2001 From: SteveBot <1153461+unbraind@users.noreply.github.com> Date: Mon, 10 Aug 2026 07:53:44 +0200 Subject: [PATCH 1/6] fix(docstring-gate): propagate an unresolvable entry instead of skipping the gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The isMainInvocation guard caught realpathSync errors and returned false. When argv[1] could not be resolved, the top-level selector called the no-op placeholder instead of main, so npm run docstring exited 0 having scanned nothing — a mandatory release gate reporting success without doing its job. The corrected implementation propagates the realpathSync error. The case requires argv[1] to stop resolving after Node has already loaded this file, so in practice it means the environment is broken, and a broken environment must not silently satisfy a gate. Crashing loudly is the safe outcome. --- .agents/pm/history/pm-github-wb4q.jsonl | 6 ++++ .agents/pm/issues/pm-github-wb4q.toon | 24 +++++++++++++ CHANGELOG.md | 1 + scripts/docstring-gate.ts | 48 ++++++++++++------------- test/docstring-gate.test.ts | 27 ++++++-------- 5 files changed, 64 insertions(+), 42 deletions(-) create mode 100644 .agents/pm/history/pm-github-wb4q.jsonl create mode 100644 .agents/pm/issues/pm-github-wb4q.toon diff --git a/.agents/pm/history/pm-github-wb4q.jsonl b/.agents/pm/history/pm-github-wb4q.jsonl new file mode 100644 index 0000000..4ddfeae --- /dev/null +++ b/.agents/pm/history/pm-github-wb4q.jsonl @@ -0,0 +1,6 @@ +{"ts":"2026-08-10T05:53:22.197Z","author":"pi-agent","author_source":"asserted","agent_harness":"pi","agent_model":"glm-5.2:cloud","agent_model_source":"environment","agent_instance":"07e97aa365b211e55b7745cd","agent_provenance":{"model":{"value":"glm-5.2:cloud","source":"environment"},"effort":null,"role":null,"topic":null},"op":"create","patch":[{"op":"add","path":"/metadata/id","value":"pm-github-wb4q"},{"op":"add","path":"/metadata/title","value":"Propagate the docstring gate entry guard fix"},{"op":"add","path":"/metadata/description","value":"The isMainInvocation guard caught realpathSync errors and returned false, causing npm run docstring to exit 0 having scanned nothing — a mandatory release gate reporting success without doing its job. The guard now propagates the error so a broken environment fails loudly instead of silently passing the gate."},{"op":"add","path":"/metadata/type","value":"Issue"},{"op":"add","path":"/metadata/status","value":"open"},{"op":"add","path":"/metadata/priority","value":2},{"op":"add","path":"/metadata/tags","value":["docstrings","gate"]},{"op":"add","path":"/metadata/created_at","value":"2026-08-10T05:53:22.197Z"},{"op":"add","path":"/metadata/updated_at","value":"2026-08-10T05:53:22.197Z"},{"op":"add","path":"/metadata/author","value":"pi-agent"}],"before_hash":"3cc22dff72be7b14824654a7a64ea62b04799939b2fee54c1b5f52ca60bf6df0","after_hash":"3b4a129ab17ce32fba14a15db02676d081d6db922aeb740949056f7d428207ca","message":""} +{"ts":"2026-08-10T05:53:25.011Z","author":"pi-agent","author_source":"asserted","agent_harness":"pi","agent_model":"glm-5.2:cloud","agent_model_source":"environment","agent_instance":"07e97aa365b211e55b7745cd","agent_provenance":{"model":{"value":"glm-5.2:cloud","source":"environment"},"effort":null,"role":null,"topic":null},"op":"note_add","patch":[{"op":"replace","path":"/metadata/updated_at","value":"2026-08-10T05:53:25.011Z"},{"op":"add","path":"/metadata/notes","value":[{"created_at":"2026-08-10T05:53:25.011Z","author":"pi-agent","text":"The isMainInvocation function in scripts/docstring-gate.ts wrapped realpathSync in a try/catch that returned false on ENOENT. When argv[1] could not be resolved, the top-level selector called the no-op placeholder instead of main, so npm run docstring exited 0 having scanned nothing. A required release check reported success without doing its job. The corrected implementation propagates the realpathSync error: a broken environment must not silently satisfy a gate."}]}],"before_hash":"3b4a129ab17ce32fba14a15db02676d081d6db922aeb740949056f7d428207ca","after_hash":"4a5d9d1e7a163fa42883a5b5430a0effb5e995fa19a7ec497e499f16a16aaff4"} +{"ts":"2026-08-10T05:53:28.150Z","author":"pi-agent","author_source":"asserted","agent_harness":"pi","agent_model":"glm-5.2:cloud","agent_model_source":"environment","agent_instance":"07e97aa365b211e55b7745cd","agent_provenance":{"model":{"value":"glm-5.2:cloud","source":"environment"},"effort":null,"role":null,"topic":null},"op":"comment_add","patch":[{"op":"replace","path":"/metadata/updated_at","value":"2026-08-10T05:53:28.150Z"},{"op":"add","path":"/metadata/comments","value":[{"created_at":"2026-08-10T05:53:28.150Z","author":"pi-agent","text":"Verified: node --experimental-strip-types -e with an unresolvable argv[1] now throws ENOENT instead of returning false. The gate fails loudly."}]}],"before_hash":"4a5d9d1e7a163fa42883a5b5430a0effb5e995fa19a7ec497e499f16a16aaff4","after_hash":"406c4c1b68a25f9dd96ad864e292e911d1a8f3afe9ad625bcde33ed6ef19a161"} +{"ts":"2026-08-10T05:53:28.694Z","author":"pi-agent","author_source":"asserted","agent_harness":"pi","agent_model":"glm-5.2:cloud","agent_model_source":"environment","agent_instance":"07e97aa365b211e55b7745cd","agent_provenance":{"model":{"value":"glm-5.2:cloud","source":"environment"},"effort":null,"role":null,"topic":null},"op":"files_add","patch":[{"op":"replace","path":"/metadata/updated_at","value":"2026-08-10T05:53:28.694Z"},{"op":"add","path":"/metadata/files","value":[{"path":"scripts/docstring-gate.ts","scope":"project"}]}],"before_hash":"406c4c1b68a25f9dd96ad864e292e911d1a8f3afe9ad625bcde33ed6ef19a161","after_hash":"772b1cced4d577af1270e47b6682715476c30da4d6a3364a4f9e68db7cdd918a"} +{"ts":"2026-08-10T05:53:29.231Z","author":"pi-agent","author_source":"asserted","agent_harness":"pi","agent_model":"glm-5.2:cloud","agent_model_source":"environment","agent_instance":"07e97aa365b211e55b7745cd","agent_provenance":{"model":{"value":"glm-5.2:cloud","source":"environment"},"effort":null,"role":null,"topic":null},"op":"files_add","patch":[{"op":"add","path":"/metadata/files/1","value":{"path":"test/docstring-gate.test.ts","scope":"project"}},{"op":"replace","path":"/metadata/updated_at","value":"2026-08-10T05:53:29.231Z"}],"before_hash":"772b1cced4d577af1270e47b6682715476c30da4d6a3364a4f9e68db7cdd918a","after_hash":"255dd7333a675d613c172ecbba4199926ffecfcd9d14ed340c03364565049eae"} +{"ts":"2026-08-10T05:53:31.847Z","author":"pi-agent","author_source":"asserted","agent_harness":"pi","agent_model":"glm-5.2:cloud","agent_model_source":"environment","agent_instance":"07e97aa365b211e55b7745cd","agent_provenance":{"model":{"value":"glm-5.2:cloud","source":"environment"},"effort":null,"role":null,"topic":null},"op":"close","patch":[{"op":"replace","path":"/metadata/updated_at","value":"2026-08-10T05:53:31.847Z"},{"op":"replace","path":"/metadata/status","value":"closed"},{"op":"add","path":"/metadata/closed_at","value":"2026-08-10T05:53:31.831Z"},{"op":"add","path":"/metadata/completed_at","value":"2026-08-10T05:53:31.831Z"},{"op":"add","path":"/metadata/resolution","value":"isMainInvocation propagates realpathSync errors instead of catching them and returning false"},{"op":"add","path":"/metadata/expected_result","value":"npm run docstring with an unresolvable argv[1] throws ENOENT and exits non-zero"},{"op":"add","path":"/metadata/actual_result","value":"npm run docstring with an unresolvable argv[1] previously exited 0 having scanned nothing"},{"op":"add","path":"/metadata/close_reason","value":"fixed"}],"before_hash":"255dd7333a675d613c172ecbba4199926ffecfcd9d14ed340c03364565049eae","after_hash":"4420083076a9c9d945f70124062a52b71d59591a68bcb4a1c412809feff5834e"} diff --git a/.agents/pm/issues/pm-github-wb4q.toon b/.agents/pm/issues/pm-github-wb4q.toon new file mode 100644 index 0000000..6ffb640 --- /dev/null +++ b/.agents/pm/issues/pm-github-wb4q.toon @@ -0,0 +1,24 @@ +id: pm-github-wb4q +title: Propagate the docstring gate entry guard fix +description: "The isMainInvocation guard caught realpathSync errors and returned false, causing npm run docstring to exit 0 having scanned nothing — a mandatory release gate reporting success without doing its job. The guard now propagates the error so a broken environment fails loudly instead of silently passing the gate." +type: Issue +status: closed +priority: 2 +tags[2]: docstrings,gate +created_at: "2026-08-10T05:53:22.197Z" +updated_at: "2026-08-10T05:53:31.847Z" +closed_at: "2026-08-10T05:53:31.831Z" +completed_at: "2026-08-10T05:53:31.831Z" +author: pi-agent +resolution: isMainInvocation propagates realpathSync errors instead of catching them and returning false +expected_result: "npm run docstring with an unresolvable argv[1] throws ENOENT and exits non-zero" +actual_result: "npm run docstring with an unresolvable argv[1] previously exited 0 having scanned nothing" +comments[1]{created_at,author,text}: + "2026-08-10T05:53:28.150Z",pi-agent,"Verified: node --experimental-strip-types -e with an unresolvable argv[1] now throws ENOENT instead of returning false. The gate fails loudly." +notes[1]{created_at,author,text}: + "2026-08-10T05:53:25.011Z",pi-agent,"The isMainInvocation function in scripts/docstring-gate.ts wrapped realpathSync in a try/catch that returned false on ENOENT. When argv[1] could not be resolved, the top-level selector called the no-op placeholder instead of main, so npm run docstring exited 0 having scanned nothing. A required release check reported success without doing its job. The corrected implementation propagates the realpathSync error: a broken environment must not silently satisfy a gate." +files[2]{path,scope}: + scripts/docstring-gate.ts,project + test/docstring-gate.test.ts,project +close_reason: fixed +body: "" diff --git a/CHANGELOG.md b/CHANGELOG.md index 68593dc..90b1b61 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Fixed +- Propagate the docstring gate entry guard fix ([pm-github-wb4q](https://github.com/unbraind/pm-github/blob/main/.agents/pm/issues/pm-github-wb4q.toon)) - Converge changelog generation and verification on replace mode ([pm-github-8f60](https://github.com/unbraind/pm-github/blob/main/.agents/pm/issues/pm-github-8f60.toon)) - The mandatory docstring gate could skip its own scan and still exit zero ([pm-github-wxob](https://github.com/unbraind/pm-github/blob/main/.agents/pm/issues/pm-github-wxob.toon)) diff --git a/scripts/docstring-gate.ts b/scripts/docstring-gate.ts index 5676ba3..a1e5f00 100644 --- a/scripts/docstring-gate.ts +++ b/scripts/docstring-gate.ts @@ -15,7 +15,7 @@ import { realpathSync } from "node:fs"; import { join } from "node:path"; -import { fileURLToPath } from "node:url"; +import { pathToFileURL } from "node:url"; import { analyzeDocstringCoverage } from "pm-ops/docstrings"; @@ -88,39 +88,35 @@ export function main(root: string): void { } /** - * Whether the script is being invoked directly rather than imported by a test. + * Whether this module is the process entry point rather than a test import. * - * Compares resolved real filesystem paths on both sides. `import.meta.url` is - * already symlink-resolved as a URL, but `argv[1]` may point through a symlink - * or carry a different drive-letter casing than the URL form, so an exact - * string comparison would treat a direct invocation as a library import and - * silently skip {@link main}. Resolving both sides through `realpathSync` - * removes that ambiguity. + * `import.meta.url` is already symlink-resolved, so `argv[1]` is resolved through + * `realpathSync` and converted to a URL before comparison. A launcher reaching + * this file through a symlink (an npm bin shim, a linked workspace) would + * otherwise compare unequal and skip the gate silently. * - * The two resolutions fail for opposite reasons and are deliberately not - * treated alike. An unresolvable `argv[1]` only means the entry point is not - * this file, which is the ordinary "imported by a test" case, so it answers - * false. An unresolvable *own* module path is an internal contradiction: this - * file is executing, so it exists. Swallowing that would leave {@link main} - * unreached and the process exit code at zero — a mandatory gate reporting - * success having scanned nothing. It therefore fails closed and throws. + * An unresolvable `argv[1]` **propagates** rather than returning false. The two + * outcomes are not equally safe: returning false means `npm run docstring` + * exits 0 having scanned nothing, which is a required release check reporting + * success without doing its job — the one failure this gate exists to prevent. + * Letting `realpathSync` throw turns that into a loud non-zero exit. The case + * requires `argv[1]` to stop resolving after Node has already loaded this file, + * so in practice it means the environment is broken, and a broken environment + * must not silently satisfy a gate. * - * @param argv - The process argv slice to inspect. + * A genuinely different entry path still returns false, which is how a test + * importing this module declines to run the gate. + * + * @param argv - The process argv to inspect. * @param moduleUrl - The `import.meta.url` of the module that might be main. - * @returns True when `argv[1]` resolves to this module's own real path. - * @throws If this module's own path cannot be resolved. + * @returns True when `argv[1]` resolves to this module's own URL, false when it + * resolves to something else. + * @throws Whatever `realpathSync` throws when `argv[1]` cannot be resolved. */ export function isMainInvocation(argv: readonly string[], moduleUrl: string): boolean { const entry = argv[1]; if (entry === undefined) return false; - const self = realpathSync(fileURLToPath(moduleUrl)); - let resolvedEntry: string; - try { - resolvedEntry = realpathSync(entry); - } catch { - return false; - } - return resolvedEntry === self; + return pathToFileURL(realpathSync(entry)).href === moduleUrl; } if (isMainInvocation(process.argv, import.meta.url)) { diff --git a/test/docstring-gate.test.ts b/test/docstring-gate.test.ts index 2f656f1..b6e55be 100644 --- a/test/docstring-gate.test.ts +++ b/test/docstring-gate.test.ts @@ -94,27 +94,22 @@ test("docstring gate isMainInvocation resolves matching and non-matching scripts assert.equal(isMainInvocation([process.execPath, script], url), true); assert.equal(isMainInvocation([process.execPath, other], url), false); assert.equal(isMainInvocation([process.execPath], url), false); - // An entry path that does not exist is the ordinary "not this script" case. - assert.equal(isMainInvocation([process.execPath, join(root, "absent.ts")], url), false); } finally { rmSync(root, { recursive: true, force: true }); } }); -test("docstring gate isMainInvocation fails closed when its own path is unresolvable", () => { - // The gate is mandatory, so an unresolvable *own* module path must crash rather - // than leave main() unreached with the exit code still at zero — that shape - // reports success having scanned nothing. Only the self-resolution throws; an - // unresolvable argv[1] stays a plain false, asserted in the test above. - const root = mkdtempSync(join(tmpdir(), "pm-github-docstring-self-")); - try { - const entry = join(root, "docstring-gate.ts"); - writeFileSync(entry, ""); - const absentSelf = pathToFileURL(join(root, "vanished", "docstring-gate.ts")).href; - assert.throws(() => isMainInvocation([process.execPath, entry], absentSelf), /ENOENT/); - } finally { - rmSync(root, { recursive: true, force: true }); - } +test("docstring gate isMainInvocation throws rather than skipping the gate when argv[1] cannot be resolved", () => { + const root = resolve(import.meta.dirname, ".."); + const gateUrl = pathToFileURL(resolve(root, "scripts", "docstring-gate.ts")).href; + // Returning false here would leave `npm run docstring` exiting 0 having + // scanned nothing - a required release check reporting success without doing + // its job. Crashing is the safe outcome, so assert it is what happens. + assert.throws( + () => isMainInvocation(["node", resolve(root, "does-not-exist.ts")], gateUrl), + /ENOENT/, + "an unresolvable entry must propagate, not silently decline to run the gate", + ); }); test("docstring gate main writes a success line to stdout and exits 0", () => { From 0bc24772f4a76f70f9b76afe311eb8b34c683a5c Mon Sep 17 00:00:00 2001 From: SteveBot <1153461+unbraind@users.noreply.github.com> Date: Mon, 10 Aug 2026 08:31:01 +0200 Subject: [PATCH 2/6] fix(gate): canonicalize both paths so a runtime flag cannot skip the gate Greptile and CodeRabbit independently flagged the same hole in the fix from the previous commit, on two different repositories. Comparing `pathToFileURL(realpathSync(entry)).href` against a raw `moduleUrl` resolves only one side. That is sufficient under Node's defaults, where the ESM loader realpaths a module before recording `import.meta.url`. Under `--preserve-symlinks` or `--preserve-symlinks-main` it is not: `moduleUrl` keeps the symlink while `realpathSync(entry)` resolves it, so a direct invocation through a symlink compares unequal, the selector calls the placeholder, and `npm run docstring` exits 0 without scanning. That is the exact silent skip this function exists to prevent, reintroduced by a launch flag. Measured rather than argued. With `moduleUrl` holding the symlink path: both-sides (new): true one-sided (old): false Canonicalising both sides costs one syscall and removes the dependence on how Node was launched. The tests also now use `process.execPath` rather than the literal "node", so the argv matches a real invocation on systems where the binary is named differently, and assert on `error.code === "ENOENT"` rather than matching the message text, which is not part of Node's contract. --- .agents/pm/history/pm-github-wb4q.jsonl | 1 + .agents/pm/issues/pm-github-wb4q.toon | 5 +++-- scripts/docstring-gate.ts | 26 ++++++++++++++++--------- test/docstring-gate.test.ts | 4 ++-- 4 files changed, 23 insertions(+), 13 deletions(-) diff --git a/.agents/pm/history/pm-github-wb4q.jsonl b/.agents/pm/history/pm-github-wb4q.jsonl index 4ddfeae..1a802f3 100644 --- a/.agents/pm/history/pm-github-wb4q.jsonl +++ b/.agents/pm/history/pm-github-wb4q.jsonl @@ -4,3 +4,4 @@ {"ts":"2026-08-10T05:53:28.694Z","author":"pi-agent","author_source":"asserted","agent_harness":"pi","agent_model":"glm-5.2:cloud","agent_model_source":"environment","agent_instance":"07e97aa365b211e55b7745cd","agent_provenance":{"model":{"value":"glm-5.2:cloud","source":"environment"},"effort":null,"role":null,"topic":null},"op":"files_add","patch":[{"op":"replace","path":"/metadata/updated_at","value":"2026-08-10T05:53:28.694Z"},{"op":"add","path":"/metadata/files","value":[{"path":"scripts/docstring-gate.ts","scope":"project"}]}],"before_hash":"406c4c1b68a25f9dd96ad864e292e911d1a8f3afe9ad625bcde33ed6ef19a161","after_hash":"772b1cced4d577af1270e47b6682715476c30da4d6a3364a4f9e68db7cdd918a"} {"ts":"2026-08-10T05:53:29.231Z","author":"pi-agent","author_source":"asserted","agent_harness":"pi","agent_model":"glm-5.2:cloud","agent_model_source":"environment","agent_instance":"07e97aa365b211e55b7745cd","agent_provenance":{"model":{"value":"glm-5.2:cloud","source":"environment"},"effort":null,"role":null,"topic":null},"op":"files_add","patch":[{"op":"add","path":"/metadata/files/1","value":{"path":"test/docstring-gate.test.ts","scope":"project"}},{"op":"replace","path":"/metadata/updated_at","value":"2026-08-10T05:53:29.231Z"}],"before_hash":"772b1cced4d577af1270e47b6682715476c30da4d6a3364a4f9e68db7cdd918a","after_hash":"255dd7333a675d613c172ecbba4199926ffecfcd9d14ed340c03364565049eae"} {"ts":"2026-08-10T05:53:31.847Z","author":"pi-agent","author_source":"asserted","agent_harness":"pi","agent_model":"glm-5.2:cloud","agent_model_source":"environment","agent_instance":"07e97aa365b211e55b7745cd","agent_provenance":{"model":{"value":"glm-5.2:cloud","source":"environment"},"effort":null,"role":null,"topic":null},"op":"close","patch":[{"op":"replace","path":"/metadata/updated_at","value":"2026-08-10T05:53:31.847Z"},{"op":"replace","path":"/metadata/status","value":"closed"},{"op":"add","path":"/metadata/closed_at","value":"2026-08-10T05:53:31.831Z"},{"op":"add","path":"/metadata/completed_at","value":"2026-08-10T05:53:31.831Z"},{"op":"add","path":"/metadata/resolution","value":"isMainInvocation propagates realpathSync errors instead of catching them and returning false"},{"op":"add","path":"/metadata/expected_result","value":"npm run docstring with an unresolvable argv[1] throws ENOENT and exits non-zero"},{"op":"add","path":"/metadata/actual_result","value":"npm run docstring with an unresolvable argv[1] previously exited 0 having scanned nothing"},{"op":"add","path":"/metadata/close_reason","value":"fixed"}],"before_hash":"255dd7333a675d613c172ecbba4199926ffecfcd9d14ed340c03364565049eae","after_hash":"4420083076a9c9d945f70124062a52b71d59591a68bcb4a1c412809feff5834e"} +{"ts":"2026-08-10T06:31:01.671Z","author":"pi-agent","author_source":"asserted","agent_harness":"pi","agent_model":"glm-5.2:cloud","agent_model_source":"environment","agent_instance":"7525f7064349b35d9826cadf","agent_provenance":{"model":{"value":"glm-5.2:cloud","source":"environment"},"effort":null,"role":null,"topic":null},"op":"note_add","patch":[{"op":"add","path":"/metadata/notes/1","value":{"created_at":"2026-08-10T06:31:01.671Z","author":"pi-agent","text":"Greptile (P2 on pm-beads) and CodeRabbit (Major on pm-slack) independently flagged the one-sided comparison in isMainInvocation: pathToFileURL(realpathSync(entry)).href === moduleUrl resolves only argv[1], leaving moduleUrl holding the symlink under --preserve-symlinks. Canonicalising both sides through realpathSync fixes it. Verified by measurement: with moduleUrl holding the symlink path, both-sides (new) returns true while one-sided (old) returns false."}},{"op":"replace","path":"/metadata/updated_at","value":"2026-08-10T06:31:01.671Z"}],"before_hash":"4420083076a9c9d945f70124062a52b71d59591a68bcb4a1c412809feff5834e","after_hash":"6d0e30e9f5a61f6cc2e420a7c0ab3685a582da62c6c265360f001c9f441989bd"} diff --git a/.agents/pm/issues/pm-github-wb4q.toon b/.agents/pm/issues/pm-github-wb4q.toon index 6ffb640..e5223c3 100644 --- a/.agents/pm/issues/pm-github-wb4q.toon +++ b/.agents/pm/issues/pm-github-wb4q.toon @@ -6,7 +6,7 @@ status: closed priority: 2 tags[2]: docstrings,gate created_at: "2026-08-10T05:53:22.197Z" -updated_at: "2026-08-10T05:53:31.847Z" +updated_at: "2026-08-10T06:31:01.671Z" closed_at: "2026-08-10T05:53:31.831Z" completed_at: "2026-08-10T05:53:31.831Z" author: pi-agent @@ -15,8 +15,9 @@ expected_result: "npm run docstring with an unresolvable argv[1] throws ENOENT a actual_result: "npm run docstring with an unresolvable argv[1] previously exited 0 having scanned nothing" comments[1]{created_at,author,text}: "2026-08-10T05:53:28.150Z",pi-agent,"Verified: node --experimental-strip-types -e with an unresolvable argv[1] now throws ENOENT instead of returning false. The gate fails loudly." -notes[1]{created_at,author,text}: +notes[2]{created_at,author,text}: "2026-08-10T05:53:25.011Z",pi-agent,"The isMainInvocation function in scripts/docstring-gate.ts wrapped realpathSync in a try/catch that returned false on ENOENT. When argv[1] could not be resolved, the top-level selector called the no-op placeholder instead of main, so npm run docstring exited 0 having scanned nothing. A required release check reported success without doing its job. The corrected implementation propagates the realpathSync error: a broken environment must not silently satisfy a gate." + "2026-08-10T06:31:01.671Z",pi-agent,"Greptile (P2 on pm-beads) and CodeRabbit (Major on pm-slack) independently flagged the one-sided comparison in isMainInvocation: pathToFileURL(realpathSync(entry)).href === moduleUrl resolves only argv[1], leaving moduleUrl holding the symlink under --preserve-symlinks. Canonicalising both sides through realpathSync fixes it. Verified by measurement: with moduleUrl holding the symlink path, both-sides (new) returns true while one-sided (old) returns false." files[2]{path,scope}: scripts/docstring-gate.ts,project test/docstring-gate.test.ts,project diff --git a/scripts/docstring-gate.ts b/scripts/docstring-gate.ts index a1e5f00..e5473e5 100644 --- a/scripts/docstring-gate.ts +++ b/scripts/docstring-gate.ts @@ -15,7 +15,7 @@ import { realpathSync } from "node:fs"; import { join } from "node:path"; -import { pathToFileURL } from "node:url"; +import { fileURLToPath, pathToFileURL } from "node:url"; import { analyzeDocstringCoverage } from "pm-ops/docstrings"; @@ -90,10 +90,18 @@ export function main(root: string): void { /** * Whether this module is the process entry point rather than a test import. * - * `import.meta.url` is already symlink-resolved, so `argv[1]` is resolved through - * `realpathSync` and converted to a URL before comparison. A launcher reaching - * this file through a symlink (an npm bin shim, a linked workspace) would - * otherwise compare unequal and skip the gate silently. + * Both sides are canonicalised through `realpathSync` before comparison. A + * launcher reaching this file through a symlink (an npm bin shim, a linked + * workspace) would otherwise compare unequal and skip the gate silently. + * + * Resolving only `argv[1]` would be enough under Node's defaults, where the + * ESM loader realpaths a module before recording `import.meta.url`. It is not + * enough under `--preserve-symlinks`/`--preserve-symlinks-main`, which leave + * `moduleUrl` holding the symlink while `realpathSync(entry)` resolves it. + * The two would then compare unequal on a direct invocation and the gate would + * exit 0 without scanning — the exact silent skip this function exists to + * prevent, reintroduced by a runtime flag. Canonicalising both sides costs one + * syscall and removes the dependence on how Node was launched. * * An unresolvable `argv[1]` **propagates** rather than returning false. The two * outcomes are not equally safe: returning false means `npm run docstring` @@ -109,14 +117,14 @@ export function main(root: string): void { * * @param argv - The process argv to inspect. * @param moduleUrl - The `import.meta.url` of the module that might be main. - * @returns True when `argv[1]` resolves to this module's own URL, false when it - * resolves to something else. - * @throws Whatever `realpathSync` throws when `argv[1]` cannot be resolved. + * @returns True when `argv[1]` and `moduleUrl` canonicalise to the same path, + * false when they canonicalise to different ones. + * @throws Whatever `realpathSync` throws when either path cannot be resolved. */ export function isMainInvocation(argv: readonly string[], moduleUrl: string): boolean { const entry = argv[1]; if (entry === undefined) return false; - return pathToFileURL(realpathSync(entry)).href === moduleUrl; + return realpathSync(entry) === realpathSync(fileURLToPath(moduleUrl)); } if (isMainInvocation(process.argv, import.meta.url)) { diff --git a/test/docstring-gate.test.ts b/test/docstring-gate.test.ts index b6e55be..2b30bf4 100644 --- a/test/docstring-gate.test.ts +++ b/test/docstring-gate.test.ts @@ -106,8 +106,8 @@ test("docstring gate isMainInvocation throws rather than skipping the gate when // scanned nothing - a required release check reporting success without doing // its job. Crashing is the safe outcome, so assert it is what happens. assert.throws( - () => isMainInvocation(["node", resolve(root, "does-not-exist.ts")], gateUrl), - /ENOENT/, + () => isMainInvocation([process.execPath, resolve(root, "does-not-exist.ts")], gateUrl), + (error: unknown) => (error as NodeJS.ErrnoException).code === "ENOENT", "an unresolvable entry must propagate, not silently decline to run the gate", ); }); From 9649c736231b9ec3fda380d893ee21a1ad47407e Mon Sep 17 00:00:00 2001 From: SteveBot <1153461+unbraind@users.noreply.github.com> Date: Mon, 10 Aug 2026 09:08:46 +0200 Subject: [PATCH 3/6] test(gate): pin the canonicalization with a case the old comparison fails The existing test (isMainInvocation resolves matching and non-matching scripts) did not use symlinks at all, so it could not distinguish the fixed implementation from the broken one. The new test puts the symlink in moduleUrl (pathToFileURL(link).href), which is what Node records in import.meta.url under --preserve-symlinks / --preserve-symlinks-main. The old comparison resolves argv[1] to the real path and compares it to the symlink URL, which is false, so the gate silently skips. The canonicalized comparison resolves both sides through realpathSync and returns true. Measured for this repo: reverting to the old one-sided comparison makes the new test fail (fail 1), restoring the canonicalization makes it pass (fail 0). --- .agents/pm/history/pm-github-wb4q.jsonl | 1 + .agents/pm/issues/pm-github-wb4q.toon | 5 +++-- test/docstring-gate.test.ts | 27 ++++++++++++++++++++++++- 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/.agents/pm/history/pm-github-wb4q.jsonl b/.agents/pm/history/pm-github-wb4q.jsonl index 1a802f3..a780a10 100644 --- a/.agents/pm/history/pm-github-wb4q.jsonl +++ b/.agents/pm/history/pm-github-wb4q.jsonl @@ -5,3 +5,4 @@ {"ts":"2026-08-10T05:53:29.231Z","author":"pi-agent","author_source":"asserted","agent_harness":"pi","agent_model":"glm-5.2:cloud","agent_model_source":"environment","agent_instance":"07e97aa365b211e55b7745cd","agent_provenance":{"model":{"value":"glm-5.2:cloud","source":"environment"},"effort":null,"role":null,"topic":null},"op":"files_add","patch":[{"op":"add","path":"/metadata/files/1","value":{"path":"test/docstring-gate.test.ts","scope":"project"}},{"op":"replace","path":"/metadata/updated_at","value":"2026-08-10T05:53:29.231Z"}],"before_hash":"772b1cced4d577af1270e47b6682715476c30da4d6a3364a4f9e68db7cdd918a","after_hash":"255dd7333a675d613c172ecbba4199926ffecfcd9d14ed340c03364565049eae"} {"ts":"2026-08-10T05:53:31.847Z","author":"pi-agent","author_source":"asserted","agent_harness":"pi","agent_model":"glm-5.2:cloud","agent_model_source":"environment","agent_instance":"07e97aa365b211e55b7745cd","agent_provenance":{"model":{"value":"glm-5.2:cloud","source":"environment"},"effort":null,"role":null,"topic":null},"op":"close","patch":[{"op":"replace","path":"/metadata/updated_at","value":"2026-08-10T05:53:31.847Z"},{"op":"replace","path":"/metadata/status","value":"closed"},{"op":"add","path":"/metadata/closed_at","value":"2026-08-10T05:53:31.831Z"},{"op":"add","path":"/metadata/completed_at","value":"2026-08-10T05:53:31.831Z"},{"op":"add","path":"/metadata/resolution","value":"isMainInvocation propagates realpathSync errors instead of catching them and returning false"},{"op":"add","path":"/metadata/expected_result","value":"npm run docstring with an unresolvable argv[1] throws ENOENT and exits non-zero"},{"op":"add","path":"/metadata/actual_result","value":"npm run docstring with an unresolvable argv[1] previously exited 0 having scanned nothing"},{"op":"add","path":"/metadata/close_reason","value":"fixed"}],"before_hash":"255dd7333a675d613c172ecbba4199926ffecfcd9d14ed340c03364565049eae","after_hash":"4420083076a9c9d945f70124062a52b71d59591a68bcb4a1c412809feff5834e"} {"ts":"2026-08-10T06:31:01.671Z","author":"pi-agent","author_source":"asserted","agent_harness":"pi","agent_model":"glm-5.2:cloud","agent_model_source":"environment","agent_instance":"7525f7064349b35d9826cadf","agent_provenance":{"model":{"value":"glm-5.2:cloud","source":"environment"},"effort":null,"role":null,"topic":null},"op":"note_add","patch":[{"op":"add","path":"/metadata/notes/1","value":{"created_at":"2026-08-10T06:31:01.671Z","author":"pi-agent","text":"Greptile (P2 on pm-beads) and CodeRabbit (Major on pm-slack) independently flagged the one-sided comparison in isMainInvocation: pathToFileURL(realpathSync(entry)).href === moduleUrl resolves only argv[1], leaving moduleUrl holding the symlink under --preserve-symlinks. Canonicalising both sides through realpathSync fixes it. Verified by measurement: with moduleUrl holding the symlink path, both-sides (new) returns true while one-sided (old) returns false."}},{"op":"replace","path":"/metadata/updated_at","value":"2026-08-10T06:31:01.671Z"}],"before_hash":"4420083076a9c9d945f70124062a52b71d59591a68bcb4a1c412809feff5834e","after_hash":"6d0e30e9f5a61f6cc2e420a7c0ab3685a582da62c6c265360f001c9f441989bd"} +{"ts":"2026-08-10T07:08:43.529Z","author":"pi-agent","author_source":"asserted","agent_harness":"pi","agent_model":"glm-5.2:cloud","agent_model_source":"environment","agent_instance":"1684e74de48dd9cf0e628b6d","agent_provenance":{"model":{"value":"glm-5.2:cloud","source":"environment"},"effort":null,"role":null,"topic":null},"op":"note_add","patch":[{"op":"add","path":"/metadata/notes/2","value":{"created_at":"2026-08-10T07:08:43.529Z","author":"pi-agent","text":"The previous test (isMainInvocation resolves matching and non-matching scripts) did not use symlinks at all, so it could not distinguish the fixed implementation from the broken one. Added a new test (canonicalizes a symlinked moduleUrl, as --preserve-symlinks produces) that puts the symlink in moduleUrl (pathToFileURL(link).href), which is what Node records in import.meta.url under --preserve-symlinks. Verified by measurement: reverting to the old one-sided comparison makes the new test fail (fail 1), restoring the canonicalization makes it pass (fail 0)."}},{"op":"replace","path":"/metadata/updated_at","value":"2026-08-10T07:08:43.529Z"}],"before_hash":"6d0e30e9f5a61f6cc2e420a7c0ab3685a582da62c6c265360f001c9f441989bd","after_hash":"fd43aeb95b4c42c1cd383665f3540613408c2a72ee07b884cf762645b296c485"} diff --git a/.agents/pm/issues/pm-github-wb4q.toon b/.agents/pm/issues/pm-github-wb4q.toon index e5223c3..1a9c5ce 100644 --- a/.agents/pm/issues/pm-github-wb4q.toon +++ b/.agents/pm/issues/pm-github-wb4q.toon @@ -6,7 +6,7 @@ status: closed priority: 2 tags[2]: docstrings,gate created_at: "2026-08-10T05:53:22.197Z" -updated_at: "2026-08-10T06:31:01.671Z" +updated_at: "2026-08-10T07:08:43.529Z" closed_at: "2026-08-10T05:53:31.831Z" completed_at: "2026-08-10T05:53:31.831Z" author: pi-agent @@ -15,9 +15,10 @@ expected_result: "npm run docstring with an unresolvable argv[1] throws ENOENT a actual_result: "npm run docstring with an unresolvable argv[1] previously exited 0 having scanned nothing" comments[1]{created_at,author,text}: "2026-08-10T05:53:28.150Z",pi-agent,"Verified: node --experimental-strip-types -e with an unresolvable argv[1] now throws ENOENT instead of returning false. The gate fails loudly." -notes[2]{created_at,author,text}: +notes[3]{created_at,author,text}: "2026-08-10T05:53:25.011Z",pi-agent,"The isMainInvocation function in scripts/docstring-gate.ts wrapped realpathSync in a try/catch that returned false on ENOENT. When argv[1] could not be resolved, the top-level selector called the no-op placeholder instead of main, so npm run docstring exited 0 having scanned nothing. A required release check reported success without doing its job. The corrected implementation propagates the realpathSync error: a broken environment must not silently satisfy a gate." "2026-08-10T06:31:01.671Z",pi-agent,"Greptile (P2 on pm-beads) and CodeRabbit (Major on pm-slack) independently flagged the one-sided comparison in isMainInvocation: pathToFileURL(realpathSync(entry)).href === moduleUrl resolves only argv[1], leaving moduleUrl holding the symlink under --preserve-symlinks. Canonicalising both sides through realpathSync fixes it. Verified by measurement: with moduleUrl holding the symlink path, both-sides (new) returns true while one-sided (old) returns false." + "2026-08-10T07:08:43.529Z",pi-agent,"The previous test (isMainInvocation resolves matching and non-matching scripts) did not use symlinks at all, so it could not distinguish the fixed implementation from the broken one. Added a new test (canonicalizes a symlinked moduleUrl, as --preserve-symlinks produces) that puts the symlink in moduleUrl (pathToFileURL(link).href), which is what Node records in import.meta.url under --preserve-symlinks. Verified by measurement: reverting to the old one-sided comparison makes the new test fail (fail 1), restoring the canonicalization makes it pass (fail 0)." files[2]{path,scope}: scripts/docstring-gate.ts,project test/docstring-gate.test.ts,project diff --git a/test/docstring-gate.test.ts b/test/docstring-gate.test.ts index 2b30bf4..e70d815 100644 --- a/test/docstring-gate.test.ts +++ b/test/docstring-gate.test.ts @@ -11,7 +11,7 @@ */ import assert from "node:assert/strict"; -import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { resolve, join } from "node:path"; import { pathToFileURL } from "node:url"; @@ -99,6 +99,31 @@ test("docstring gate isMainInvocation resolves matching and non-matching scripts } }); +test("docstring gate isMainInvocation canonicalizes a symlinked moduleUrl, as --preserve-symlinks produces", () => { + // The symlink test above passes argv[1] as the link and moduleUrl as the REAL + // path, which the old one-sided comparison also satisfied - so it could not + // tell the two implementations apart. This is the case that can: moduleUrl + // holds the SYMLINK, which is what Node records in import.meta.url under + // --preserve-symlinks / --preserve-symlinks-main. + // + // Old: pathToFileURL(realpathSync(link)).href === linkUrl -> false, so the + // selector calls the placeholder and the gate exits 0 without scanning. + // New: realpathSync(link) === realpathSync(fileURLToPath(linkUrl)) -> true. + const gatePath = resolve(import.meta.dirname, "..", "scripts", "docstring-gate.ts"); + const linkDir = mkdtempSync(join(tmpdir(), "pm-github-docgate-preserve-")); + const link = join(linkDir, "docstring-gate.ts"); + try { + symlinkSync(gatePath, link); + assert.equal( + isMainInvocation([process.execPath, link], pathToFileURL(link).href), + true, + "a symlinked moduleUrl must still resolve to a direct invocation", + ); + } finally { + rmSync(linkDir, { recursive: true, force: true }); + } +}); + test("docstring gate isMainInvocation throws rather than skipping the gate when argv[1] cannot be resolved", () => { const root = resolve(import.meta.dirname, ".."); const gateUrl = pathToFileURL(resolve(root, "scripts", "docstring-gate.ts")).href; From 18ddd259ea94a04807dfccf0eadb0e53ad56c387 Mon Sep 17 00:00:00 2001 From: SteveBot <1153461+unbraind@users.noreply.github.com> Date: Mon, 10 Aug 2026 09:36:20 +0200 Subject: [PATCH 4/6] docs(gate): do not claim a fixed syscall count for the extra resolution CodeRabbit flagged that the comment said canonicalising both sides costs one syscall while the function calls realpathSync twice, and each resolution can itself require several filesystem operations. The claim was mine and it was copied into every adopting repository along with the fix. The accurate statement is that it adds a second realpathSync. What the comment is actually justifying is the removal of a dependence on how Node was launched, and that argument does not need a cost figure to stand. --- scripts/docstring-gate.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/docstring-gate.ts b/scripts/docstring-gate.ts index e5473e5..9fa6b65 100644 --- a/scripts/docstring-gate.ts +++ b/scripts/docstring-gate.ts @@ -100,8 +100,8 @@ export function main(root: string): void { * `moduleUrl` holding the symlink while `realpathSync(entry)` resolves it. * The two would then compare unequal on a direct invocation and the gate would * exit 0 without scanning — the exact silent skip this function exists to - * prevent, reintroduced by a runtime flag. Canonicalising both sides costs one - * syscall and removes the dependence on how Node was launched. + * prevent, reintroduced by a runtime flag. Canonicalising both sides adds a + * second `realpathSync` and removes the dependence on how Node was launched. * * An unresolvable `argv[1]` **propagates** rather than returning false. The two * outcomes are not equally safe: returning false means `npm run docstring` From 1f83a7b4b573fe5c8034ef52a6f01706d7669fc4 Mon Sep 17 00:00:00 2001 From: SteveBot <1153461+unbraind@users.noreply.github.com> Date: Mon, 10 Aug 2026 10:37:52 +0200 Subject: [PATCH 5/6] refactor(gate): drop the import left unused by the two-sided comparison DeepScan flagged one new issue on these PRs and this is it: switching to `realpathSync(entry) === realpathSync(fileURLToPath(moduleUrl))` removed the last use of `pathToFileURL` in this file, but the import stayed. Nothing else caught it. These packages have no lint script, and typecheck does not enable noUnusedLocals, so the only gate that saw it was the advisory one. --- scripts/docstring-gate.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/docstring-gate.ts b/scripts/docstring-gate.ts index 9fa6b65..541339b 100644 --- a/scripts/docstring-gate.ts +++ b/scripts/docstring-gate.ts @@ -15,7 +15,7 @@ import { realpathSync } from "node:fs"; import { join } from "node:path"; -import { fileURLToPath, pathToFileURL } from "node:url"; +import { fileURLToPath } from "node:url"; import { analyzeDocstringCoverage } from "pm-ops/docstrings"; From 7992d5167d87ec6336880bec871b4bbf81329e7b Mon Sep 17 00:00:00 2001 From: SteveBot <1153461+unbraind@users.noreply.github.com> Date: Mon, 10 Aug 2026 10:51:12 +0200 Subject: [PATCH 6/6] test(gate): drop a cross-reference to a test this file does not have CodeRabbit caught that the comment opens with "The symlink test above" while this file has no preceding symlink test - the regression test is the first and only one here. The wording was copied from a repository that does have both, so the rationale read as describing a test that is not present. Rewritten to state the property directly rather than by reference: a case that passes the link as argv[1] and the real path as moduleUrl cannot distinguish the two implementations, because realpathSync(link) resolves to the real path either way. --- test/docstring-gate.test.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/docstring-gate.test.ts b/test/docstring-gate.test.ts index e70d815..db19d62 100644 --- a/test/docstring-gate.test.ts +++ b/test/docstring-gate.test.ts @@ -100,11 +100,11 @@ test("docstring gate isMainInvocation resolves matching and non-matching scripts }); test("docstring gate isMainInvocation canonicalizes a symlinked moduleUrl, as --preserve-symlinks produces", () => { - // The symlink test above passes argv[1] as the link and moduleUrl as the REAL - // path, which the old one-sided comparison also satisfied - so it could not - // tell the two implementations apart. This is the case that can: moduleUrl - // holds the SYMLINK, which is what Node records in import.meta.url under - // --preserve-symlinks / --preserve-symlinks-main. + // A symlink case that passes argv[1] as the link and moduleUrl as the REAL + // path cannot tell the two implementations apart: realpathSync(link) resolves + // to the real path, so the old one-sided comparison satisfied it too. This is + // the case that can: moduleUrl holds the SYMLINK, which is what Node records + // in import.meta.url under --preserve-symlinks / --preserve-symlinks-main. // // Old: pathToFileURL(realpathSync(link)).href === linkUrl -> false, so the // selector calls the placeholder and the gate exits 0 without scanning.