Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .agents/pm/history/pm-github-wb4q.jsonl
Original file line number Diff line number Diff line change
@@ -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"}
26 changes: 26 additions & 0 deletions .agents/pm/issues/pm-github-wb4q.toon
Original file line number Diff line number Diff line change
@@ -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: ""
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Expand Down
54 changes: 29 additions & 25 deletions scripts/docstring-gate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
48 changes: 34 additions & 14 deletions test/docstring-gate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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 });
}
});
Comment thread
unbraind marked this conversation as resolved.

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",
);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
});

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
Expand Down