Skip to content

Commit c95f406

Browse files
fix(ci): check-adr-0087-registration 的 CLI 派发加入入口守卫,import 不再跑门禁 (#6566) (#6695)
`scripts/check-adr-0087-registration.mjs` 在模块顶层读 `process.argv` 并直接 派发(`--self-test` / `--list` / `--audit-stock` / 门禁本体),没有比较 `import.meta.url` 与 `process.argv[1]` 的入口守卫。后果是:任何一次 `import` 都会真的把门禁跑在**调用方的仓库**上,判红时六个 `process.exit(1)` 里的一个还会直接掐死调用方进程 —— 调用方自己的代码一行都执行不到。 改为 `objectui-changeset-digest.mjs` 已有的写法:仅当本文件是进程入口时才 派发。`argv` / `readFlag` 只被派发用到,一并收进守卫块内;模块顶层此后只剩 纯常量计算(`REPO_ROOT` 由 `__dirname` 推出,不外调 git),import 无副作用。 判定逻辑、词表、分支行为、`--self-test` 的位置一概未动。CLI 输出与退出码 逐字节比对 origin/main 基线:`--list`、`--audit-stock`、门禁本体、 `--base <不存在的 ref>` 四条路径 stdout/stderr 全等,退出码全等。 自测新增 I1/I2 两组共 7 条断言(100 → 107): - I1:在一个**判红**的临时仓库里 `import` 本脚本的纯导出,必须不跑门禁、 不打印判词、不改退出码; - I2:同一个 fixture 下把同一份文件当入口跑,门禁仍判红并 exit 1,`--list` 仍打表并 exit 0。 反向验证(两个方向都实测):去掉守卫恢复顶层派发 → I1 三条全红、I2 不动; 把 CLI 块从被拷贝的脚本里删掉 → I2 四条里红三条、I1 不动。第四条 (`--list` 退出码为 0)在后一种消融下仍为绿 —— 没有 CLI 的脚本同样退出 0, 退出码分不清「分支跑了且成功」和「什么都没跑」,故每条退出码断言都配了一 条输出断言。 顺带修正 `objectui-changeset-digest.mjs` 里一处**注释**:它原文断言本脚本 「CLI dispatch runs at TOP LEVEL (it has no `import.meta.url === argv[1]` guard)」,本 PR 之后该陈述为假。只改注释,不动代码。 Claude-Session: https://claude.ai/code/session_01AZgRyPVwi1jLb1mNNuUQ9o Co-authored-by: Claude <noreply@anthropic.com>
1 parent 179f0ae commit c95f406

2 files changed

Lines changed: 153 additions & 66 deletions

File tree

scripts/check-adr-0087-registration.mjs

Lines changed: 146 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -153,8 +153,8 @@
153153
// `pnpm install` -- the same constraint its neighbours in the Check Changeset job
154154
// carry.
155155

156-
import { execFileSync } from 'node:child_process';
157-
import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from 'node:fs';
156+
import { execFileSync, spawnSync } from 'node:child_process';
157+
import { mkdtempSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
158158
import { tmpdir } from 'node:os';
159159
import { dirname, join, resolve } from 'node:path';
160160
import { fileURLToPath } from 'node:url';
@@ -2032,6 +2032,79 @@ function selfTest() {
20322032
assert(!breakingDeclaration(parseChangeset(CS({ bumps: [['a', 'patch']], body: 'plain\n' }))).breaking, 'P7: a plain patch is not');
20332033
assert(extractIds(" id: 'object-titleFormat-to-nameField',\n").length === 1, 'P8: an id with a capital letter must be extracted');
20342034

2035+
// ---- I1 (#6566): a bare `import` of this module must NOT run the gate -----
2036+
// The CLI dispatch at the bottom of this file is entry-guarded; importing the
2037+
// module for its pure exports (readDisposition, extractIds, ...) must be
2038+
// side-effect-free. The fixture is a RED-verdict repo (a declared-breaking
2039+
// changeset, no marker) holding a copy of this very script, and the importer
2040+
// is a child process whose only code borrows one pure function. Delete the
2041+
// entry guard and all three assertions go red: the imported copy judges the
2042+
// fixture repo, prints its verdict, `process.exit(1)`s, and the importer's
2043+
// own line never runs -- the behaviour measured in #6566, which forced
2044+
// PR #6556 into a subprocess fixture instead of an import.
2045+
{
2046+
const { dir, base } = mk({
2047+
files: { '.changeset/unanswered-breaking.md': CS({ body: '**BREAKING** x\n\nno marker here\n' }) },
2048+
});
2049+
// Park `main` back at BASE with the breaking changeset ahead of it on `work`:
2050+
// an unguarded import default-resolves base to `main` and must judge the
2051+
// changeset RED -- the caller-killing shape #6566 measured. With both commits
2052+
// on `main` the ablated gate would see an empty diff and merely print a green
2053+
// verdict, pinning the output leak but not the exit(1).
2054+
git(['checkout', '-q', '-b', 'work'], dir);
2055+
git(['branch', '-f', 'main', base], dir);
2056+
const w = (rel, text) => {
2057+
mkdirSync(dirname(join(dir, rel)), { recursive: true });
2058+
writeFileSync(join(dir, rel), text);
2059+
};
2060+
const copy = 'scripts/check-adr-0087-registration.mjs';
2061+
w(copy, readFileSync(fileURLToPath(import.meta.url), 'utf8'));
2062+
w(
2063+
'importer.mjs',
2064+
"import { readDisposition } from './scripts/check-adr-0087-registration.mjs';\n" +
2065+
"console.log('IMPORTER_OWN_LINE', JSON.stringify(readDisposition('nothing here')));\n",
2066+
);
2067+
const r = spawnSync(process.execPath, [join(dir, 'importer.mjs')], { cwd: dir, encoding: 'utf8' });
2068+
const all = `${r.stdout}\n${r.stderr}`;
2069+
assert(r.status === 0, `I1: a bare import must not adopt the gate's exit code (got ${r.status})\n${all}`);
2070+
assert(
2071+
r.stdout.includes('IMPORTER_OWN_LINE {"ok":false'),
2072+
`I1: the importer's own code must run and receive the export\n${all}`,
2073+
);
2074+
assert(!all.includes('check-adr-0087-registration:'), `I1: importing must not print a gate verdict\n${all}`);
2075+
2076+
// ---- I2 (#6566): the SAME file, run as the entry point, still dispatches --
2077+
// I1 alone is satisfied by deleting the CLI block outright -- it only ever
2078+
// asserts that nothing happens. I2 is its other half over the very same
2079+
// fixture: the guard must let the entry point through, on both a branch that
2080+
// exits non-zero and one that does not. Measured both ways: restore the
2081+
// top-level dispatch and I1's three go red with I2 untouched; strip the CLI
2082+
// block from the copied script and three of I2's four go red with I1
2083+
// untouched.
2084+
//
2085+
// Why each exit-code assertion is PAIRED with an output one: the fourth,
2086+
// `--list` exiting 0, stayed green under that second ablation -- a script
2087+
// with no CLI at all also exits 0. An exit code cannot distinguish "the
2088+
// branch ran and succeeded" from "nothing ran"; only the printed table can.
2089+
const cli = (...args) => spawnSync(process.execPath, [join(dir, copy), ...args], { cwd: dir, encoding: 'utf8' });
2090+
2091+
const gate = cli();
2092+
assert(gate.status === 1, `I2: the entry point must still run the gate and exit 1 on red (got ${gate.status})\n${gate.stdout}\n${gate.stderr}`);
2093+
assert(
2094+
gate.stderr.includes('unanswered-breaking.md') && gate.stderr.includes('no `adr-0087:` disposition marker'),
2095+
// The EXACT verdict, not merely "it is red": an entry point that died for
2096+
// an unrelated reason also exits 1, and would pin nothing about dispatch.
2097+
`I2: the entry point must report the real verdict\n${gate.stdout}\n${gate.stderr}`,
2098+
);
2099+
2100+
const listed = cli('--list');
2101+
assert(listed.status === 0, `I2: --list must still dispatch and exit 0 (got ${listed.status})\n${listed.stdout}\n${listed.stderr}`);
2102+
assert(
2103+
listed.stdout.includes('declared-breaking changeset(s) in stock'),
2104+
`I2: --list must still print its table\n${listed.stdout}\n${listed.stderr}`,
2105+
);
2106+
}
2107+
20352108
for (const d of cleanup) rmSync(d, { recursive: true, force: true });
20362109

20372110
if (failures.length) {
@@ -2045,72 +2118,84 @@ function selfTest() {
20452118
// ---------------------------------------------------------------------------
20462119
// CLI
20472120
// ---------------------------------------------------------------------------
2121+
//
2122+
// Entry-guarded (#6566): the dispatch below runs ONLY when this file is the
2123+
// process entry point (the `objectui-changeset-digest.mjs` pattern). No branch
2124+
// is a no-op and four of them `process.exit(1)` on red, so an unguarded top
2125+
// level made `import` mean "run the gate against the IMPORTER's repo and adopt
2126+
// its verdict as my exit code" -- PR #6556 paid a ~40-line subprocess fixture
2127+
// to avoid exactly that. The exported pure functions (readDisposition,
2128+
// extractIds, ...) are the module's import surface; the CLI is this block, and
2129+
// the I1/I2 self-test assertions pin BOTH halves of the separation -- silent as
2130+
// an import, unchanged as an entry point.
2131+
2132+
if (resolve(process.argv[1] ?? '') === resolve(fileURLToPath(import.meta.url))) {
2133+
const argv = process.argv.slice(2);
2134+
const readFlag = (name) => {
2135+
const i = argv.indexOf(name);
2136+
return i >= 0 ? argv[i + 1] : undefined;
2137+
};
20482138

2049-
const argv = process.argv.slice(2);
2050-
const readFlag = (name) => {
2051-
const i = argv.indexOf(name);
2052-
return i >= 0 ? argv[i + 1] : undefined;
2053-
};
2054-
2055-
if (argv.includes('--self-test')) {
2056-
selfTest();
2057-
} else if (argv.includes('--list')) {
2058-
list(REPO_ROOT, 'HEAD');
2059-
} else if (argv.includes('--audit-stock')) {
2060-
auditStock(REPO_ROOT, readFlag('--head') ?? 'HEAD');
2061-
} else {
2062-
const head = readFlag('--head') ?? 'HEAD';
2063-
const requested = readFlag('--base');
2064-
let base = requested;
2065-
if (base) {
2066-
if (!resolveCommit(base, REPO_ROOT)) {
2067-
console.error(`⛔ check-adr-0087-registration: --base '${base}' does not resolve to a commit.`);
2068-
console.error(' A base that cannot be resolved is a failure, never a pass (#4690).');
2069-
process.exit(1);
2070-
}
2139+
if (argv.includes('--self-test')) {
2140+
selfTest();
2141+
} else if (argv.includes('--list')) {
2142+
list(REPO_ROOT, 'HEAD');
2143+
} else if (argv.includes('--audit-stock')) {
2144+
auditStock(REPO_ROOT, readFlag('--head') ?? 'HEAD');
20712145
} else {
2072-
base = ['origin/main', 'main'].find((r) => resolveCommit(r, REPO_ROOT));
2073-
if (!base) {
2074-
console.error('⛔ check-adr-0087-registration: neither origin/main nor main resolves in this checkout.');
2075-
console.error(' Pass one explicitly: --base <ref-or-sha>. Missing input is a failure, never a pass (#4690).');
2076-
process.exit(1);
2146+
const head = readFlag('--head') ?? 'HEAD';
2147+
const requested = readFlag('--base');
2148+
let base = requested;
2149+
if (base) {
2150+
if (!resolveCommit(base, REPO_ROOT)) {
2151+
console.error(`⛔ check-adr-0087-registration: --base '${base}' does not resolve to a commit.`);
2152+
console.error(' A base that cannot be resolved is a failure, never a pass (#4690).');
2153+
process.exit(1);
2154+
}
2155+
} else {
2156+
base = ['origin/main', 'main'].find((r) => resolveCommit(r, REPO_ROOT));
2157+
if (!base) {
2158+
console.error('⛔ check-adr-0087-registration: neither origin/main nor main resolves in this checkout.');
2159+
console.error(' Pass one explicitly: --base <ref-or-sha>. Missing input is a failure, never a pass (#4690).');
2160+
process.exit(1);
2161+
}
20772162
}
2078-
}
20792163

2080-
const inputProblems = assertInputs({ cwd: REPO_ROOT, head });
2081-
if (inputProblems.length > 0) {
2082-
console.error(`\n✗ check-adr-0087-registration: ${inputProblems.length} input problem(s) -- refusing to report a verdict.\n`);
2083-
for (const p of inputProblems) console.error(` • ${p}\n`);
2084-
console.error(' A gate that cannot find its input and exits 0 is worse than no gate (#4690).');
2085-
process.exit(1);
2086-
}
2164+
const inputProblems = assertInputs({ cwd: REPO_ROOT, head });
2165+
if (inputProblems.length > 0) {
2166+
console.error(`\n✗ check-adr-0087-registration: ${inputProblems.length} input problem(s) -- refusing to report a verdict.\n`);
2167+
for (const p of inputProblems) console.error(` • ${p}\n`);
2168+
console.error(' A gate that cannot find its input and exits 0 is worse than no gate (#4690).');
2169+
process.exit(1);
2170+
}
20872171

2088-
let result;
2089-
try {
2090-
result = scan({ cwd: REPO_ROOT, base, head });
2091-
} catch (e) {
2092-
console.error(`⛔ check-adr-0087-registration: ${e.message}`);
2093-
process.exit(1);
2094-
}
2172+
let result;
2173+
try {
2174+
result = scan({ cwd: REPO_ROOT, base, head });
2175+
} catch (e) {
2176+
console.error(`⛔ check-adr-0087-registration: ${e.message}`);
2177+
process.exit(1);
2178+
}
20952179

2096-
if (result.problems.length > 0) {
2097-
report(result.problems);
2098-
process.exit(1);
2099-
}
2180+
if (result.problems.length > 0) {
2181+
report(result.problems);
2182+
process.exit(1);
2183+
}
21002184

2101-
const n = result.judged.length;
2102-
if (n === 0) {
2103-
console.log(`✓ check-adr-0087-registration: this PR adds no declared-breaking changeset (${result.skipped.length} non-breaking changeset(s) seen).`);
2104-
} else {
2105-
console.log(`✓ check-adr-0087-registration: ${n} declared-breaking changeset(s), each carrying an ADR-0087 disposition.`);
2106-
for (const j of result.judged) {
2107-
const what = j.verdict === 'registered' ? `registered ${j.ids.join(', ')} (new here: ${j.fresh.join(', ')})` : `not-required (${j.category})`;
2108-
console.log(` ${j.file} [${j.signals.join('+')}] ${what}`);
2109-
if (j.why) {
2110-
// Every exemption is printed AND annotated, on every run: an exemption
2111-
// nobody re-reads is the allow-list failure mode this gate exists to avoid.
2112-
console.log(` reason: ${j.why}`);
2113-
console.log(`::notice file=${j.file}::ADR-0087 exemption (${j.category}): ${j.why}`);
2185+
const n = result.judged.length;
2186+
if (n === 0) {
2187+
console.log(`✓ check-adr-0087-registration: this PR adds no declared-breaking changeset (${result.skipped.length} non-breaking changeset(s) seen).`);
2188+
} else {
2189+
console.log(`✓ check-adr-0087-registration: ${n} declared-breaking changeset(s), each carrying an ADR-0087 disposition.`);
2190+
for (const j of result.judged) {
2191+
const what = j.verdict === 'registered' ? `registered ${j.ids.join(', ')} (new here: ${j.fresh.join(', ')})` : `not-required (${j.category})`;
2192+
console.log(` ${j.file} [${j.signals.join('+')}] ${what}`);
2193+
if (j.why) {
2194+
// Every exemption is printed AND annotated, on every run: an exemption
2195+
// nobody re-reads is the allow-list failure mode this gate exists to avoid.
2196+
console.log(` reason: ${j.why}`);
2197+
console.log(`::notice file=${j.file}::ADR-0087 exemption (${j.category}): ${j.why}`);
2198+
}
21142199
}
21152200
}
21162201
}

scripts/objectui-changeset-digest.mjs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -419,11 +419,13 @@ export const ADR_0087_SCAFFOLD =
419419
* `/\*\*BREAKING/` matches it. Reading the artifact closes that by construction
420420
* instead of by agreement between two predicates.
421421
*
422-
* Why the gate's own function is not imported: that module's CLI dispatch runs
423-
* at TOP LEVEL (it has no `import.meta.url === argv[1]` guard), so importing it
424-
* would execute the gate; and it would put the release-critical bump path one
425-
* rename away from failing. The agreement is pinned in the self-test instead,
426-
* which runs the real gate as a child process over a real temp repository.
422+
* Why the gate's own function is not imported: it would put the release-critical
423+
* bump path one rename away from failing, and the claim being pinned is about
424+
* ANOTHER script's verdict -- only that script's own run settles it. (The gate's
425+
* former top-level CLI dispatch, which made any import execute the gate, was
426+
* entry-guarded in #6566; the coupling reason stands on its own.) The agreement
427+
* is pinned in the self-test instead, which runs the real gate as a child
428+
* process over a real temp repository.
427429
*
428430
* @param {{ bump: string, body: string }} artifact
429431
*/

0 commit comments

Comments
 (0)