Comments: keep only those one rule allows, outside the files #11 changes

scripts/e2e/host-lifecycle-check.mjs

@@ -1,24 +1,4 @@
 #!/usr/bin/env node
-/**
- * The one automated check on {@link ../e2e/fake-host.mjs}'s own behaviour.
- *
- * `npm test` covers `src/`; the end-to-end host is a script and nothing in the
- * suite would catch a regression in it. Three review rounds found three
- * separate ways it reported a green run over a server that had failed — a
- * refusal taken for a success (round 2), a crash between commands (round 3
- * finding 3), and a server that crashed *during* its own shutdown (round 3
- * finding 2) — so the lifecycle it has to get right is checked here instead.
- *
- * It needs no credentials and no network: each case spawns the host against a
- * tiny stub server written to a temp directory, which answers the MCPL
- * handshake and then behaves badly in one specific way. What is asserted is
- * the host's **exit status** and the verdict on its `child-exit` line, because
- * those are what a scenario run is read by.
- *
- *   node scripts/e2e/host-lifecycle-check.mjs
- *
- * Exits 0 when every case holds, 1 on the first that does not, naming it.
- */
 
 import { spawn } from 'node:child_process';
 import { mkdtempSync, rmSync, writeFileSync } from 'node:fs';
@@ -27,15 +7,8 @@ import * as path from 'node:path';
 
 const HOST = path.join(import.meta.dirname, 'fake-host.mjs');
 const workspace = mkdtempSync(path.join(tmpdir(), 'github-mcpl-host-check-'));
+const IDLE_BEFORE_INPUT_MS = 400;
 
-/**
- * A stub server, parameterised by how it misbehaves.
- *
- * `onSigterm`: `'clean'` exits 0, `'fail'` exits 3, `'ignore'` does nothing at
- * all so the host has to escalate, and `'default'` installs no handler so the
- * signal itself ends the process. `crashWhenIdle` exits 3 shortly after the
- * grant, with no command in flight.
- */
 function stub(name, { onSigterm = 'clean', crashWhenIdle = false } = {}) {
   const file = path.join(workspace, `${name}.mjs`);
   writeFileSync(
@@ -63,19 +36,10 @@ setInterval(() => {}, 1000);
   return file;
 }
 
-/**
- * Run the host against `server`, closing its stdin the way `closing` says.
- *
- * `quit` sends the command; `eof` just closes the pipe. Either way the input
- * is delayed, so the stub has a window to misbehave while the host sits idle
- * on stdin with nothing in flight — which is the state round 3's finding 3
- * was about and which a command sent immediately would hide.
- */
 function run(server, closing, extra = []) {
   return new Promise((resolve, reject) => {
     const child = spawn(process.execPath, [HOST, '--server', server, '--shutdown-grace', '700', ...extra], {
       stdio: ['pipe', 'pipe', 'pipe'],
-      // No credentials: the stub needs none, and this check must not read any.
       env: { PATH: process.env.PATH },
     });
     let out = '';
@@ -86,7 +50,7 @@ function run(server, closing, extra = []) {
     setTimeout(() => {
       if (closing === 'quit') child.stdin.write('{"cmd":"quit"}\n');
       child.stdin.end();
-    }, 400);
+    }, IDLE_BEFORE_INPUT_MS);
     child.on('exit', (code) => {
       const lines = out
         .split('\n')
@@ -165,7 +129,6 @@ try {
     if (testCase.expected !== undefined && got.childExit?.expected !== testCase.expected) {
       problems.push(`child-exit.expected ${got.childExit?.expected}, wanted ${testCase.expected}`);
     }
-    // A failing run has to say why, or the status is the only evidence there is.
     if (testCase.exit === 1 && got.failures.length === 0) problems.push('exit 1 with no failed result reported');
     if (testCase.exit === 0 && got.failures.length > 0) {
       problems.push(`exit 0 but ${got.failures.length} failed result(s)`);

scripts/release-changelog.mjs

@@ -1,13 +1,6 @@
-// Runs as npm's `version` lifecycle hook (see package.json): at that point
-// package.json already carries the new version, and files staged here are
-// included in the release commit that `npm version` then creates and tags.
-//
-// Folds the pending fragments in changelog.d/ (one file per change,
-// `<slug>.<breaking|added|changed|fixed>.md`) together with anything filed
-// directly under the standing `## Unreleased` section into a new
-// `## X.Y.Z — YYYY-MM-DD` section, deletes the consumed fragments, and
-// leaves a fresh empty `## Unreleased` above it. Refuses to release when
-// there is nothing to release, or when the input's shape is ambiguous.
+// Runs as npm's `version` lifecycle hook: package.json already carries the
+// new version, and files staged here go into the release commit that
+// `npm version` then creates and tags.
 import {
   existsSync,
   readdirSync,
@@ -21,7 +14,6 @@ import { fileURLToPath } from "node:url";
 
 const CHANGELOG = "CHANGELOG.md";
 const FRAGMENT_DIR = "changelog.d";
-// Canonical subsection order; a fragment's category must be one of these.
 const CATEGORIES = ["breaking", "added", "changed", "fixed"];
 const HEADINGS = {
   breaking: "Breaking",
@@ -30,32 +22,22 @@ const HEADINGS = {
   fixed: "Fixed",
 };
 
-// Refusals throw and are reported at the entry point, which then lets the
-// process end on its own with exitCode 1. process.exit() would race the
-// stderr write when stderr is a pipe (asynchronous on macOS), leaving the
-// caller a bare failure status with no reason attached.
+// Refusals throw and set exitCode rather than calling process.exit(), which
+// can cut off a stderr write still pending: Node writes to a pipe
+// asynchronously on POSIX.
 export class ReleaseError extends Error {}
 const fail = (msg) => {
   throw new ReleaseError(msg);
 };
 
-// Fragment grammar: every non-blank line starts a bullet or continues one
-// (indented two or more spaces; nested bullets included). Headings and
-// thematic breaks are refused wherever they appear — at top level a '## '
-// line would splice a fake section boundary into the released changelog,
-// and as item content ('- ## x', '  ## x') they still render as headings.
+// A heading inside a list item still renders as a heading.
 const isBulletStart = (l) => /^[-*] /.test(l);
 const isContinuation = (l) => /^ {2,}\S/.test(l);
 const itemContent = (l) => l.replace(/^\s*(?:[-*]\s+)?/, "");
 const isHeading = (s) => /^#{1,6}(\s|$)/.test(s);
-// Thematic breaks may carry interior whitespace ('- - -', '* * *').
 const isRule = (s) => /^([-*_=])(\s*\1){2,}\s*$/.test(s);
 const isBlockConstruct = (l) => isRule(l.trim()) || isHeading(itemContent(l));
 
-// The directory is scanned fail-closed: anything that is not README.md or
-// a well-formed fragment file aborts the release, so an entry can never be
-// silently left out (e.g. a fragment created under a 'fix/' subdirectory
-// because the slug was taken verbatim from a branch name).
 function collectFragments(dir) {
   const fragments = [];
   if (!existsSync(join(dir, FRAGMENT_DIR))) return fragments;
@@ -100,11 +82,6 @@ function collectFragments(dir) {
   return fragments;
 }
 
-// Merge fragment bullets into the Unreleased body's subsection structure.
-// Directly-filed entries are kept; a fragment joins the first subsection
-// whose title starts with its category (so audience-qualified headings like
-// '### Breaking (module authors only)' still attract 'breaking' fragments),
-// or a new canonical subsection. Output is emitted in canonical order.
 function mergeFragments(body, frags) {
   const preamble = [];
   const parts = [];
@@ -147,9 +124,6 @@ export function release(dir) {
   const text = readFileSync(join(dir, CHANGELOG), "utf8");
   const fragments = collectFragments(dir);
 
-  // Exactly one Unreleased heading. A second one silently strands entries:
-  // only the first is ever cut, so anything filed under a later heading is
-  // never released and never reaches the GitHub release notes.
   const headings = [...text.matchAll(/^## Unreleased[ \t]*$/gm)];
   if (headings.length === 0) {
     fail(`no '## Unreleased' section in ${CHANGELOG} — add one before releasing.`);
@@ -181,10 +155,6 @@ export function release(dir) {
     );
   }
 
-  // Spliced by index rather than string-replaced: `text.replace("## Unreleased", …)`
-  // would hit the first *substring* occurrence, which is not necessarily the
-  // heading the regex matched (an inline mention of `## Unreleased` in prose
-  // comes first) and would inject the version heading into the wrong place.
   const date = new Date().toISOString().slice(0, 10);
   writeFileSync(
     join(dir, CHANGELOG),