diff --git a/.agents/pm/history/pm-github-wb4q.jsonl b/.agents/pm/history/pm-github-wb4q.jsonl new file mode 100644 index 00000000..a780a109 --- /dev/null +++ b/.agents/pm/history/pm-github-wb4q.jsonl @@ -0,0 +1,8 @@ +{"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"} +{"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 new file mode 100644 index 00000000..1a9c5ce1 --- /dev/null +++ b/.agents/pm/issues/pm-github-wb4q.toon @@ -0,0 +1,26 @@ +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-10T07:08:43.529Z" +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[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 +close_reason: fixed +body: "" diff --git a/CHANGELOG.md b/CHANGELOG.md index 68593dcd..90b1b61d 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 5676ba37..541339bc 100644 --- a/scripts/docstring-gate.ts +++ b/scripts/docstring-gate.ts @@ -88,39 +88,43 @@ 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. + * 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. * - * 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. + * 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 adds a + * second `realpathSync` and removes the dependence on how Node was launched. * - * @param argv - The process argv slice to inspect. + * 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. + * + * 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]` 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; - const self = realpathSync(fileURLToPath(moduleUrl)); - let resolvedEntry: string; - try { - resolvedEntry = realpathSync(entry); - } catch { - return false; - } - return resolvedEntry === self; + 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 2f656f1f..db19d62c 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"; @@ -94,29 +94,49 @@ 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-")); +test("docstring gate isMainInvocation canonicalizes a symlinked moduleUrl, as --preserve-symlinks produces", () => { + // 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. + // 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 { - 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/); + 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(root, { recursive: true, force: true }); + 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; + // 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([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", + ); +}); + test("docstring gate main writes a success line to stdout and exits 0", () => { // The real repository is fully documented, so main() takes the success path: // non-empty stdout is terminated with a newline and exitCode stays 0. This