Skip to content

Commit a585374

Browse files
hotlongclaude
andauthored
fix(scripts): digest 的回落读取不再刷一屏 git fatal:,改说一句可读事实 (#6175) (#6335)
`readAt()` 在 `to` 端点读不到 changeset 时会回落到「添加它的那个 commit」, 这条路径本身是对的 —— 范围内的 release commit 消费掉了当时挂着的全部 changeset,回落正是把它们留在发布记录里的机制。但 `execFileSync` 默认把子进程 stderr 直接接到我们自己的 stderr 上,于是第一次 `git show` 失败时 git 的 `fatal: path … does not exist in …` 在 `catch` 跑起来之前就已经打出去了: 真实 pin bump `f995a452d2ca..7dfbeb704e1e`(#6159 / PR #6173)上是九行 `fatal:` 之后接一行「全部成功」。 这正是本文件已经写死的那条原则的镜像面。#4731 写的是「a degraded list and a complete one must never look alike」,这里发生的是**一个完整的列表长得像一次 失败**,而且它指向错误的处置:下一个读者的第一反应是「digest 挂了,改用 `--no-changeset` 手写」—— 那条路才会真的丢掉 9 条 releasing changeset。 - B:第一次尝试改为**捕获** stderr(`stdio: ['ignore','pipe','pipe']`)而不是 继承;两次都失败时把两次的诊断按尝试点名重新打出来并继续抛错 —— 安静是 「回落成功」挣来的,永不外借给真正的失败。 - C:发生过回落时,在 accounting 行旁打一句事实(`absentAtTo`),0 次回落时 不打 —— 不制造新噪声。这句落在 stderr 而不是 changeset 正文:正文记录这个 区间发布了什么,而「工具怎么读到这个文件」不是发布事实。 self-test 新增 8 条断言独立成组(#4731/#6099 既有 fixture 的精确计数一字未动), 其中三条通过**子进程**跑真实 CLI —— git 的 `fatal:` 落在真实 stderr 上, 进程内断言看不见它。 Claude-Session: https://claude.ai/code/session_01BDmDsu2575gDxeMCxXhDE3 Co-authored-by: Claude <noreply@anthropic.com>
1 parent 00fc5eb commit a585374

1 file changed

Lines changed: 263 additions & 20 deletions

File tree

scripts/objectui-changeset-digest.mjs

Lines changed: 263 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -69,12 +69,45 @@
6969
//
7070
// This criterion changes the ANNOTATION and the COUNT only. The collected set is
7171
// untouched: nothing is admitted or dropped because of it.
72-
73-
import { execFileSync } from 'node:child_process';
72+
//
73+
// WHY THE FALLBACK IS QUIET, AND WHY IT STILL SAYS SO (#6175)
74+
// ----------------------------------------------------------
75+
// `readAt` reads each changeset at `to` and falls back to the commit that ADDED
76+
// it, because a changeset consumed by a release INSIDE the range is gone at `to`
77+
// (the reason `collectAddedChangesets` walks the log instead of diffing the
78+
// endpoints). The fallback works, and the artifact it produces is complete.
79+
//
80+
// It used to ANNOUNCE ITSELF AS A FAILURE anyway. `execFileSync` sends the
81+
// child's stderr to ours unless `stdio` says otherwise, so the first `git show`
82+
// printed git's own `fatal: path … does not exist in …` before `catch` could run
83+
// — one line per changeset, then one line saying everything succeeded. Measured
84+
// on the real pin bump `f995a452d2ca..7dfbeb704e1e` (#6159 / PR #6173): 9
85+
// `fatal:` lines, 9 complete entries, exit 0.
86+
//
87+
// That is the MIRROR of the rule this file already enforces. #4731 wrote "a
88+
// degraded list and a complete one must never look alike"; here a COMPLETE list
89+
// looked like a failure. The two failure modes cost the same, because the next
90+
// reader's first instinct is "the digest is broken, write the changeset by hand
91+
// with `--no-changeset`" — and that hand-written path really does drop all 9
92+
// releasing entries. Noise that points at the wrong remedy is not merely noise.
93+
//
94+
// So the first attempt CAPTURES its stderr instead of inheriting it, and the
95+
// fallback is reported once, as a fact, beside the accounting line
96+
// (`absentAtTo`). Two things this deliberately does not do:
97+
//
98+
// * It does not go silent. Silence would trade a misleading line for no line,
99+
// and "this range crossed a release" is worth exactly one sentence — the
100+
// same reason the cap and the degraded list announce themselves (#4731).
101+
// * It does not mute FAILURE. When BOTH reads fail nothing was read and the
102+
// entry would vanish, so both captured diagnostics are re-emitted, named by
103+
// attempt, and the error still propagates. Quiet is earned by the fallback
104+
// that worked; it is never extended to the one that did not.
105+
106+
import { execFileSync, spawnSync } from 'node:child_process';
74107
import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
75108
import { tmpdir } from 'node:os';
76109
import { dirname, join, resolve } from 'node:path';
77-
import { fileURLToPath } from 'node:url';
110+
import { fileURLToPath, pathToFileURL } from 'node:url';
78111

79112
const __dirname = dirname(fileURLToPath(import.meta.url));
80113
const REPO_ROOT = resolve(__dirname, '..');
@@ -84,11 +117,20 @@ export const DEFAULT_MAX_ENTRIES = 100;
84117

85118
const LEVEL_RANK = { patch: 1, minor: 2, major: 3 };
86119

87-
/** @param {string} cwd @param {string[]} args */
88-
function git(cwd, args) {
120+
/**
121+
* @param {string} cwd
122+
* @param {string[]} args
123+
* @param {{ captureStderr?: boolean }} [options] `captureStderr` routes the
124+
* child's stderr into the thrown error instead of ours. Leave it OFF by
125+
* default: an unset `stdio` inherits our stderr (`execFileSync`'s documented
126+
* behaviour), which is what a call whose failure is a real failure should do.
127+
* Turn it on only where a failure is EXPECTED and retried — `readAt` (#6175).
128+
*/
129+
function git(cwd, args, { captureStderr = false } = {}) {
89130
return execFileSync('git', ['-C', cwd, ...args], {
90131
encoding: 'utf8',
91132
maxBuffer: 64 * 1024 * 1024,
133+
...(captureStderr ? { stdio: ['ignore', 'pipe', 'pipe'] } : {}),
92134
});
93135
}
94136

@@ -309,12 +351,54 @@ export function collectAddedChangesets(objectuiRoot, from, to) {
309351
};
310352
}
311353

312-
/** Read a changeset's content at `to`, falling back to the commit that added it. */
313-
function readAt(objectuiRoot, to, sha, path) {
354+
/** A failed `execFileSync` rendered as the child's own diagnostic, indented. */
355+
function gitDiagnostic(err) {
356+
const captured = typeof err?.stderr === 'string' ? err.stderr : '';
357+
const text = captured.trim() || err?.message || String(err);
358+
return text
359+
.split('\n')
360+
.map((line) => ` ${line}`)
361+
.join('\n');
362+
}
363+
364+
/**
365+
* Read a changeset's content at `to`, falling back to the commit that added it.
366+
*
367+
* `fellBack` is the caller's only handle on "this range crossed a release".
368+
* git's own `fatal:` used to be that handle by accident — which made a complete
369+
* result read as a failed one, the defect #6175 records; see the header note.
370+
*
371+
* Exported for the self-test: the both-reads-failed branch cannot be reached
372+
* through `classifyRange`, whose `sha` always comes from `--diff-filter=A` and
373+
* therefore always has the path. An error path no test can enter is an error
374+
* path that quietly rots into silence, which is the one outcome #6175 forbids.
375+
*
376+
* @returns {{ text: string, fellBack: boolean }}
377+
*/
378+
export function readAt(objectuiRoot, to, sha, path) {
314379
try {
315-
return git(objectuiRoot, ['show', `${to}:${path}`]);
316-
} catch {
317-
return git(objectuiRoot, ['show', `${sha}:${path}`]);
380+
return {
381+
text: git(objectuiRoot, ['show', `${to}:${path}`], { captureStderr: true }),
382+
fellBack: false,
383+
};
384+
} catch (atTo) {
385+
try {
386+
return {
387+
text: git(objectuiRoot, ['show', `${sha}:${path}`], { captureStderr: true }),
388+
fellBack: true,
389+
};
390+
} catch (atSha) {
391+
// BOTH reads failed: nothing was read, and this entry is about to be lost
392+
// from the release record. Say so with everything git told us, twice over.
393+
console.error(
394+
`✗ objectui-changeset-digest: cannot read ${path} — neither at \`to\` nor at the commit that added it.`,
395+
);
396+
console.error(` at \`to\` (${to}):`);
397+
console.error(gitDiagnostic(atTo));
398+
console.error(` at the commit that added it (${sha}):`);
399+
console.error(gitDiagnostic(atSha));
400+
throw atSha;
401+
}
318402
}
319403
}
320404

@@ -366,7 +450,11 @@ export function inPreMode(frameworkRoot) {
366450
* Nothing here reads a commit type. Grouping output BY type is presentation and
367451
* belongs to the caller; it must never become a filter again.
368452
*
369-
* @returns {{ releasing: Array<object>, releaseNothingEntries: Array<object>, noChangesetCommits: Array<object>, releaseNothing: number, noChangeset: number, changesetsAdded: number, totalCommits: number }}
453+
* `absentAtTo` counts the changesets that were gone at `to` and had to be read
454+
* from the commit that added them — the readable form of what used to arrive as
455+
* a screenful of git `fatal:` lines (#6175).
456+
*
457+
* @returns {{ releasing: Array<object>, releaseNothingEntries: Array<object>, noChangesetCommits: Array<object>, releaseNothing: number, noChangeset: number, changesetsAdded: number, absentAtTo: number, totalCommits: number }}
370458
*/
371459
export function classifyRange({ objectuiRoot, from, to }) {
372460
const { entries, commits, totalCommits, commitsWithChangesetShas } = collectAddedChangesets(
@@ -377,10 +465,11 @@ export function classifyRange({ objectuiRoot, from, to }) {
377465

378466
const releasing = [];
379467
const releaseNothingEntries = [];
468+
let absentAtTo = 0;
380469
for (const entry of entries) {
381-
const { packages, summary, body } = parseChangeset(
382-
readAt(objectuiRoot, to, entry.sha, entry.path),
383-
);
470+
const read = readAt(objectuiRoot, to, entry.sha, entry.path);
471+
if (read.fellBack) absentAtTo++;
472+
const { packages, summary, body } = parseChangeset(read.text);
384473
const level = highestLevel(packages);
385474
if (!level) {
386475
releaseNothingEntries.push({ ...entry, summary: summary || entry.subject });
@@ -414,14 +503,15 @@ export function classifyRange({ objectuiRoot, from, to }) {
414503
releaseNothing: releaseNothingEntries.length,
415504
noChangeset: noChangesetCommits.length,
416505
changesetsAdded: entries.length,
506+
absentAtTo,
417507
totalCommits,
418508
};
419509
}
420510

421511
/**
422512
* Build the digest for a range.
423513
*
424-
* @returns {{ bump: string, declaredLevel: string|null, breaking: number, breakingByLevel: number, breakingByAnnotation: number, releasing: Array<object>, releaseNothing: number, noChangeset: number, totalCommits: number, downgradedMajor: boolean, body: string }}
514+
* @returns {{ bump: string, declaredLevel: string|null, breaking: number, breakingByLevel: number, breakingByAnnotation: number, releasing: Array<object>, releaseNothing: number, noChangeset: number, changesetsAdded: number, absentAtTo: number, totalCommits: number, downgradedMajor: boolean, body: string }}
425515
*/
426516
export function buildDigest({
427517
objectuiRoot,
@@ -431,11 +521,12 @@ export function buildDigest({
431521
max = DEFAULT_MAX_ENTRIES,
432522
bumpOverride = '',
433523
}) {
434-
const { releasing, releaseNothing, noChangeset, changesetsAdded, totalCommits } = classifyRange({
435-
objectuiRoot,
436-
from,
437-
to,
438-
});
524+
const { releasing, releaseNothing, noChangeset, changesetsAdded, absentAtTo, totalCommits } =
525+
classifyRange({
526+
objectuiRoot,
527+
from,
528+
to,
529+
});
439530

440531
const declaredLevel = releasing.length
441532
? releasing.reduce(
@@ -533,6 +624,8 @@ export function buildDigest({
533624
releasing,
534625
releaseNothing,
535626
noChangeset,
627+
changesetsAdded,
628+
absentAtTo,
536629
totalCommits,
537630
downgradedMajor,
538631
body,
@@ -612,6 +705,22 @@ function main(argv) {
612705
(digest.downgradedMajor ? ' — declared major recorded as minor (launch window)' : ''),
613706
);
614707

708+
// #6175: the one sentence that replaces the screenful of git `fatal:` lines.
709+
// It lands HERE, beside the accounting line on stderr, and NOT in the
710+
// changeset body: the body records what the range released, and how the tool
711+
// had to read a file is not a fact about the release. The reader who needs it
712+
// is the operator watching this run — the same reader the `fatal:` lines used
713+
// to mislead. Printed only when it fires, so a range that crossed no release
714+
// gains no new line (silence here means "nothing to explain", never "nothing
715+
// happened" — the absence itself is not load-bearing).
716+
if (digest.absentAtTo > 0) {
717+
console.error(
718+
`→ ${digest.absentAtTo} of ${digest.changesetsAdded} changeset(s) added in this range ` +
719+
`no longer exist at ${short} — a release consumes the changesets it ships; ` +
720+
`each was read from the commit that added it, so the account above is complete.`,
721+
);
722+
}
723+
615724
if (out) {
616725
mkdirSync(dirname(out), { recursive: true });
617726
writeFileSync(out, file);
@@ -1039,6 +1148,140 @@ function selfTest() {
10391148
degraded.includes('NOT a\ncomplete account'),
10401149
degraded,
10411150
);
1151+
1152+
// --- #6175: a range whose `to` endpoint is a RELEASE COMMIT -------------
1153+
// The shape that made a COMPLETE result look like a failure. A release
1154+
// consumes the changesets it ships, so every changeset added in the range
1155+
// is gone at `to` and every read falls back. Its own repo on purpose: the
1156+
// #4731 / #6099 fixtures above pin exact counts and must not shift under it.
1157+
const ui3 = join(tmp, 'objectui-released');
1158+
mkdirSync(join(ui3, '.changeset'), { recursive: true });
1159+
const g3 = (...args) => git(ui3, args);
1160+
g3('init', '-q', '-b', 'main');
1161+
g3('config', 'user.email', 'selftest@objectstack.ai');
1162+
g3('config', 'user.name', 'self test');
1163+
g3('config', 'commit.gpgsign', 'false');
1164+
const commit3 = (subject, files, removals = []) => {
1165+
for (const [path, content] of Object.entries(files)) {
1166+
mkdirSync(dirname(join(ui3, path)), { recursive: true });
1167+
writeFileSync(join(ui3, path), content);
1168+
}
1169+
for (const path of removals) rmSync(join(ui3, path), { force: true });
1170+
g3('add', '-A');
1171+
g3('commit', '-q', '-m', subject);
1172+
return g3('rev-parse', 'HEAD').trim();
1173+
};
1174+
1175+
const base3 = commit3('chore: base', { 'README.md': 'base\n' });
1176+
commit3('feat(grid): column pinning survives a layout reload (#3401)', {
1177+
'.changeset/grid-column-pinning-survives-reload.md':
1178+
'---\n"@object-ui/plugin-grid": minor\n---\n\nGrid column pinning survives a layout reload.\n',
1179+
'src/grid.ts': 'a\n',
1180+
});
1181+
commit3('fix(fields): the lookup picker keeps the authored value (#3402)', {
1182+
'.changeset/lookup-picker-keeps-authored-value.md':
1183+
'---\n"@object-ui/fields": patch\n---\n\nThe lookup picker keeps the authored value.\n',
1184+
'src/fields.ts': 'b\n',
1185+
});
1186+
const release3 = commit3(
1187+
'chore: release packages (#3403)',
1188+
{ 'package.json': '{"version":"17.1.0"}\n' },
1189+
[
1190+
'.changeset/grid-column-pinning-survives-reload.md',
1191+
'.changeset/lookup-picker-keeps-authored-value.md',
1192+
],
1193+
);
1194+
1195+
const released = buildDigest({
1196+
objectuiRoot: ui3,
1197+
frameworkRoot: fwPlain,
1198+
from: base3,
1199+
to: release3,
1200+
});
1201+
check(
1202+
'#6175 a range crossing a release still collects every changeset',
1203+
released.releasing.length === 2 && released.changesetsAdded === 2,
1204+
`releasing=${released.releasing.length} added=${released.changesetsAdded}`,
1205+
);
1206+
check(
1207+
'#6175 the fallback is COUNTED, not merely survived',
1208+
released.absentAtTo === 2,
1209+
`got ${released.absentAtTo}`,
1210+
);
1211+
1212+
// The CLI runs as a CHILD so its real stderr can be read: that stream is
1213+
// where git's `fatal:` used to land, and no in-process assertion can see it.
1214+
const selfPath = fileURLToPath(import.meta.url);
1215+
const runCli = (root, from, to, label) =>
1216+
spawnSync(
1217+
process.execPath,
1218+
[
1219+
selfPath,
1220+
'--objectui-root', root,
1221+
'--framework-root', fwPlain,
1222+
'--from', from,
1223+
'--to', to,
1224+
'--out', join(tmp, `cli-${label}.md`),
1225+
],
1226+
{ encoding: 'utf8' },
1227+
);
1228+
1229+
const fellBack = runCli(ui3, base3, release3, 'released');
1230+
check(
1231+
'#6175 the fallback no longer leaks git `fatal:` onto stderr',
1232+
fellBack.status === 0 && !fellBack.stderr.includes('fatal:'),
1233+
fellBack.stderr,
1234+
);
1235+
check(
1236+
'#6175 the fallback SAYS SO — once, with the real count',
1237+
/^ 2 of 2 changeset\(s\) added in this range no longer exist at [0-9a-f]{12} /m.test(
1238+
fellBack.stderr,
1239+
) && fellBack.stderr.split('no longer exist at').length === 2,
1240+
fellBack.stderr,
1241+
);
1242+
const quietArtifact = readFileSync(join(tmp, 'cli-released.md'), 'utf8');
1243+
check(
1244+
'#6175 the artifact stays COMPLETE — the quiet read is the whole read',
1245+
quietArtifact.includes('Grid column pinning survives') &&
1246+
quietArtifact.includes('lookup picker keeps the authored value'),
1247+
quietArtifact,
1248+
);
1249+
1250+
const noFallback = runCli(ui, base, head, 'plain');
1251+
check(
1252+
'#6175 a range that crossed no release gains NO new line',
1253+
noFallback.status === 0 &&
1254+
!noFallback.stderr.includes('no longer exist at') &&
1255+
!noFallback.stderr.includes('fatal:'),
1256+
noFallback.stderr,
1257+
);
1258+
1259+
// Both reads failing is a REAL failure — nothing was read and the entry
1260+
// would vanish from the record. Quiet is earned by the fallback that
1261+
// worked; it is never extended to this. Driven through the exported
1262+
// `readAt` because `classifyRange` cannot reach the branch: its `sha`
1263+
// comes from `--diff-filter=A` and therefore always has the path.
1264+
const probe = join(tmp, 'probe-both-reads-fail.mjs');
1265+
writeFileSync(
1266+
probe,
1267+
`import { readAt } from ${JSON.stringify(pathToFileURL(selfPath).href)};\n` +
1268+
`readAt(${JSON.stringify(ui3)}, ${JSON.stringify(release3)}, ${JSON.stringify(base3)}, ` +
1269+
`'.changeset/never-existed.md');\n`,
1270+
);
1271+
const bothFail = spawnSync(process.execPath, [probe], { encoding: 'utf8' });
1272+
check(
1273+
'#6175 when BOTH reads fail the failure is LOUD, naming both attempts',
1274+
bothFail.status !== 0 &&
1275+
bothFail.stderr.includes('cannot read .changeset/never-existed.md') &&
1276+
bothFail.stderr.includes(release3) &&
1277+
bothFail.stderr.includes(base3),
1278+
bothFail.stderr,
1279+
);
1280+
check(
1281+
"#6175 both attempts' git diagnostics are re-emitted, not swallowed",
1282+
(bothFail.stderr.match(/fatal:/g) ?? []).length >= 2,
1283+
bothFail.stderr,
1284+
);
10421285
} finally {
10431286
rmSync(tmp, { recursive: true, force: true });
10441287
}

0 commit comments

Comments
 (0)