From 58bb5ada319000ff613e1b1869de09bda2e53c5b Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 14:01:12 +0000 Subject: [PATCH] =?UTF-8?q?fix(scripts):=20digest=20=E7=9A=84=E5=9B=9E?= =?UTF-8?q?=E8=90=BD=E8=AF=BB=E5=8F=96=E4=B8=8D=E5=86=8D=E5=88=B7=E4=B8=80?= =?UTF-8?q?=E5=B1=8F=20git=20`fatal:`,=E6=94=B9=E8=AF=B4=E4=B8=80=E5=8F=A5?= =?UTF-8?q?=E5=8F=AF=E8=AF=BB=E4=BA=8B=E5=AE=9E=20(#6175)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 上, 进程内断言看不见它。 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01BDmDsu2575gDxeMCxXhDE3 --- scripts/objectui-changeset-digest.mjs | 283 ++++++++++++++++++++++++-- 1 file changed, 263 insertions(+), 20 deletions(-) diff --git a/scripts/objectui-changeset-digest.mjs b/scripts/objectui-changeset-digest.mjs index 5cd8e3a2e8..741d5d864e 100644 --- a/scripts/objectui-changeset-digest.mjs +++ b/scripts/objectui-changeset-digest.mjs @@ -69,12 +69,45 @@ // // This criterion changes the ANNOTATION and the COUNT only. The collected set is // untouched: nothing is admitted or dropped because of it. - -import { execFileSync } from 'node:child_process'; +// +// WHY THE FALLBACK IS QUIET, AND WHY IT STILL SAYS SO (#6175) +// ---------------------------------------------------------- +// `readAt` reads each changeset at `to` and falls back to the commit that ADDED +// it, because a changeset consumed by a release INSIDE the range is gone at `to` +// (the reason `collectAddedChangesets` walks the log instead of diffing the +// endpoints). The fallback works, and the artifact it produces is complete. +// +// It used to ANNOUNCE ITSELF AS A FAILURE anyway. `execFileSync` sends the +// child's stderr to ours unless `stdio` says otherwise, so the first `git show` +// printed git's own `fatal: path … does not exist in …` before `catch` could run +// — one line per changeset, then one line saying everything succeeded. Measured +// on the real pin bump `f995a452d2ca..7dfbeb704e1e` (#6159 / PR #6173): 9 +// `fatal:` lines, 9 complete entries, exit 0. +// +// That is the MIRROR of the rule this file already enforces. #4731 wrote "a +// degraded list and a complete one must never look alike"; here a COMPLETE list +// looked like a failure. The two failure modes cost the same, because the next +// reader's first instinct is "the digest is broken, write the changeset by hand +// with `--no-changeset`" — and that hand-written path really does drop all 9 +// releasing entries. Noise that points at the wrong remedy is not merely noise. +// +// So the first attempt CAPTURES its stderr instead of inheriting it, and the +// fallback is reported once, as a fact, beside the accounting line +// (`absentAtTo`). Two things this deliberately does not do: +// +// * It does not go silent. Silence would trade a misleading line for no line, +// and "this range crossed a release" is worth exactly one sentence — the +// same reason the cap and the degraded list announce themselves (#4731). +// * It does not mute FAILURE. When BOTH reads fail nothing was read and the +// entry would vanish, so both captured diagnostics are re-emitted, named by +// attempt, and the error still propagates. Quiet is earned by the fallback +// that worked; it is never extended to the one that did not. + +import { execFileSync, spawnSync } from 'node:child_process'; import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { dirname, join, resolve } from 'node:path'; -import { fileURLToPath } from 'node:url'; +import { fileURLToPath, pathToFileURL } from 'node:url'; const __dirname = dirname(fileURLToPath(import.meta.url)); const REPO_ROOT = resolve(__dirname, '..'); @@ -84,11 +117,20 @@ export const DEFAULT_MAX_ENTRIES = 100; const LEVEL_RANK = { patch: 1, minor: 2, major: 3 }; -/** @param {string} cwd @param {string[]} args */ -function git(cwd, args) { +/** + * @param {string} cwd + * @param {string[]} args + * @param {{ captureStderr?: boolean }} [options] `captureStderr` routes the + * child's stderr into the thrown error instead of ours. Leave it OFF by + * default: an unset `stdio` inherits our stderr (`execFileSync`'s documented + * behaviour), which is what a call whose failure is a real failure should do. + * Turn it on only where a failure is EXPECTED and retried — `readAt` (#6175). + */ +function git(cwd, args, { captureStderr = false } = {}) { return execFileSync('git', ['-C', cwd, ...args], { encoding: 'utf8', maxBuffer: 64 * 1024 * 1024, + ...(captureStderr ? { stdio: ['ignore', 'pipe', 'pipe'] } : {}), }); } @@ -309,12 +351,54 @@ export function collectAddedChangesets(objectuiRoot, from, to) { }; } -/** Read a changeset's content at `to`, falling back to the commit that added it. */ -function readAt(objectuiRoot, to, sha, path) { +/** A failed `execFileSync` rendered as the child's own diagnostic, indented. */ +function gitDiagnostic(err) { + const captured = typeof err?.stderr === 'string' ? err.stderr : ''; + const text = captured.trim() || err?.message || String(err); + return text + .split('\n') + .map((line) => ` ${line}`) + .join('\n'); +} + +/** + * Read a changeset's content at `to`, falling back to the commit that added it. + * + * `fellBack` is the caller's only handle on "this range crossed a release". + * git's own `fatal:` used to be that handle by accident — which made a complete + * result read as a failed one, the defect #6175 records; see the header note. + * + * Exported for the self-test: the both-reads-failed branch cannot be reached + * through `classifyRange`, whose `sha` always comes from `--diff-filter=A` and + * therefore always has the path. An error path no test can enter is an error + * path that quietly rots into silence, which is the one outcome #6175 forbids. + * + * @returns {{ text: string, fellBack: boolean }} + */ +export function readAt(objectuiRoot, to, sha, path) { try { - return git(objectuiRoot, ['show', `${to}:${path}`]); - } catch { - return git(objectuiRoot, ['show', `${sha}:${path}`]); + return { + text: git(objectuiRoot, ['show', `${to}:${path}`], { captureStderr: true }), + fellBack: false, + }; + } catch (atTo) { + try { + return { + text: git(objectuiRoot, ['show', `${sha}:${path}`], { captureStderr: true }), + fellBack: true, + }; + } catch (atSha) { + // BOTH reads failed: nothing was read, and this entry is about to be lost + // from the release record. Say so with everything git told us, twice over. + console.error( + `✗ objectui-changeset-digest: cannot read ${path} — neither at \`to\` nor at the commit that added it.`, + ); + console.error(` at \`to\` (${to}):`); + console.error(gitDiagnostic(atTo)); + console.error(` at the commit that added it (${sha}):`); + console.error(gitDiagnostic(atSha)); + throw atSha; + } } } @@ -366,7 +450,11 @@ export function inPreMode(frameworkRoot) { * Nothing here reads a commit type. Grouping output BY type is presentation and * belongs to the caller; it must never become a filter again. * - * @returns {{ releasing: Array, releaseNothingEntries: Array, noChangesetCommits: Array, releaseNothing: number, noChangeset: number, changesetsAdded: number, totalCommits: number }} + * `absentAtTo` counts the changesets that were gone at `to` and had to be read + * from the commit that added them — the readable form of what used to arrive as + * a screenful of git `fatal:` lines (#6175). + * + * @returns {{ releasing: Array, releaseNothingEntries: Array, noChangesetCommits: Array, releaseNothing: number, noChangeset: number, changesetsAdded: number, absentAtTo: number, totalCommits: number }} */ export function classifyRange({ objectuiRoot, from, to }) { const { entries, commits, totalCommits, commitsWithChangesetShas } = collectAddedChangesets( @@ -377,10 +465,11 @@ export function classifyRange({ objectuiRoot, from, to }) { const releasing = []; const releaseNothingEntries = []; + let absentAtTo = 0; for (const entry of entries) { - const { packages, summary, body } = parseChangeset( - readAt(objectuiRoot, to, entry.sha, entry.path), - ); + const read = readAt(objectuiRoot, to, entry.sha, entry.path); + if (read.fellBack) absentAtTo++; + const { packages, summary, body } = parseChangeset(read.text); const level = highestLevel(packages); if (!level) { releaseNothingEntries.push({ ...entry, summary: summary || entry.subject }); @@ -414,6 +503,7 @@ export function classifyRange({ objectuiRoot, from, to }) { releaseNothing: releaseNothingEntries.length, noChangeset: noChangesetCommits.length, changesetsAdded: entries.length, + absentAtTo, totalCommits, }; } @@ -421,7 +511,7 @@ export function classifyRange({ objectuiRoot, from, to }) { /** * Build the digest for a range. * - * @returns {{ bump: string, declaredLevel: string|null, breaking: number, breakingByLevel: number, breakingByAnnotation: number, releasing: Array, releaseNothing: number, noChangeset: number, totalCommits: number, downgradedMajor: boolean, body: string }} + * @returns {{ bump: string, declaredLevel: string|null, breaking: number, breakingByLevel: number, breakingByAnnotation: number, releasing: Array, releaseNothing: number, noChangeset: number, changesetsAdded: number, absentAtTo: number, totalCommits: number, downgradedMajor: boolean, body: string }} */ export function buildDigest({ objectuiRoot, @@ -431,11 +521,12 @@ export function buildDigest({ max = DEFAULT_MAX_ENTRIES, bumpOverride = '', }) { - const { releasing, releaseNothing, noChangeset, changesetsAdded, totalCommits } = classifyRange({ - objectuiRoot, - from, - to, - }); + const { releasing, releaseNothing, noChangeset, changesetsAdded, absentAtTo, totalCommits } = + classifyRange({ + objectuiRoot, + from, + to, + }); const declaredLevel = releasing.length ? releasing.reduce( @@ -533,6 +624,8 @@ export function buildDigest({ releasing, releaseNothing, noChangeset, + changesetsAdded, + absentAtTo, totalCommits, downgradedMajor, body, @@ -612,6 +705,22 @@ function main(argv) { (digest.downgradedMajor ? ' — declared major recorded as minor (launch window)' : ''), ); + // #6175: the one sentence that replaces the screenful of git `fatal:` lines. + // It lands HERE, beside the accounting line on stderr, and NOT in the + // changeset body: the body records what the range released, and how the tool + // had to read a file is not a fact about the release. The reader who needs it + // is the operator watching this run — the same reader the `fatal:` lines used + // to mislead. Printed only when it fires, so a range that crossed no release + // gains no new line (silence here means "nothing to explain", never "nothing + // happened" — the absence itself is not load-bearing). + if (digest.absentAtTo > 0) { + console.error( + `→ ${digest.absentAtTo} of ${digest.changesetsAdded} changeset(s) added in this range ` + + `no longer exist at ${short} — a release consumes the changesets it ships; ` + + `each was read from the commit that added it, so the account above is complete.`, + ); + } + if (out) { mkdirSync(dirname(out), { recursive: true }); writeFileSync(out, file); @@ -1039,6 +1148,140 @@ function selfTest() { degraded.includes('NOT a\ncomplete account'), degraded, ); + + // --- #6175: a range whose `to` endpoint is a RELEASE COMMIT ------------- + // The shape that made a COMPLETE result look like a failure. A release + // consumes the changesets it ships, so every changeset added in the range + // is gone at `to` and every read falls back. Its own repo on purpose: the + // #4731 / #6099 fixtures above pin exact counts and must not shift under it. + const ui3 = join(tmp, 'objectui-released'); + mkdirSync(join(ui3, '.changeset'), { recursive: true }); + const g3 = (...args) => git(ui3, args); + g3('init', '-q', '-b', 'main'); + g3('config', 'user.email', 'selftest@objectstack.ai'); + g3('config', 'user.name', 'self test'); + g3('config', 'commit.gpgsign', 'false'); + const commit3 = (subject, files, removals = []) => { + for (const [path, content] of Object.entries(files)) { + mkdirSync(dirname(join(ui3, path)), { recursive: true }); + writeFileSync(join(ui3, path), content); + } + for (const path of removals) rmSync(join(ui3, path), { force: true }); + g3('add', '-A'); + g3('commit', '-q', '-m', subject); + return g3('rev-parse', 'HEAD').trim(); + }; + + const base3 = commit3('chore: base', { 'README.md': 'base\n' }); + commit3('feat(grid): column pinning survives a layout reload (#3401)', { + '.changeset/grid-column-pinning-survives-reload.md': + '---\n"@object-ui/plugin-grid": minor\n---\n\nGrid column pinning survives a layout reload.\n', + 'src/grid.ts': 'a\n', + }); + commit3('fix(fields): the lookup picker keeps the authored value (#3402)', { + '.changeset/lookup-picker-keeps-authored-value.md': + '---\n"@object-ui/fields": patch\n---\n\nThe lookup picker keeps the authored value.\n', + 'src/fields.ts': 'b\n', + }); + const release3 = commit3( + 'chore: release packages (#3403)', + { 'package.json': '{"version":"17.1.0"}\n' }, + [ + '.changeset/grid-column-pinning-survives-reload.md', + '.changeset/lookup-picker-keeps-authored-value.md', + ], + ); + + const released = buildDigest({ + objectuiRoot: ui3, + frameworkRoot: fwPlain, + from: base3, + to: release3, + }); + check( + '#6175 a range crossing a release still collects every changeset', + released.releasing.length === 2 && released.changesetsAdded === 2, + `releasing=${released.releasing.length} added=${released.changesetsAdded}`, + ); + check( + '#6175 the fallback is COUNTED, not merely survived', + released.absentAtTo === 2, + `got ${released.absentAtTo}`, + ); + + // The CLI runs as a CHILD so its real stderr can be read: that stream is + // where git's `fatal:` used to land, and no in-process assertion can see it. + const selfPath = fileURLToPath(import.meta.url); + const runCli = (root, from, to, label) => + spawnSync( + process.execPath, + [ + selfPath, + '--objectui-root', root, + '--framework-root', fwPlain, + '--from', from, + '--to', to, + '--out', join(tmp, `cli-${label}.md`), + ], + { encoding: 'utf8' }, + ); + + const fellBack = runCli(ui3, base3, release3, 'released'); + check( + '#6175 the fallback no longer leaks git `fatal:` onto stderr', + fellBack.status === 0 && !fellBack.stderr.includes('fatal:'), + fellBack.stderr, + ); + check( + '#6175 the fallback SAYS SO — once, with the real count', + /^→ 2 of 2 changeset\(s\) added in this range no longer exist at [0-9a-f]{12} —/m.test( + fellBack.stderr, + ) && fellBack.stderr.split('no longer exist at').length === 2, + fellBack.stderr, + ); + const quietArtifact = readFileSync(join(tmp, 'cli-released.md'), 'utf8'); + check( + '#6175 the artifact stays COMPLETE — the quiet read is the whole read', + quietArtifact.includes('Grid column pinning survives') && + quietArtifact.includes('lookup picker keeps the authored value'), + quietArtifact, + ); + + const noFallback = runCli(ui, base, head, 'plain'); + check( + '#6175 a range that crossed no release gains NO new line', + noFallback.status === 0 && + !noFallback.stderr.includes('no longer exist at') && + !noFallback.stderr.includes('fatal:'), + noFallback.stderr, + ); + + // Both reads failing is a REAL failure — nothing was read and the entry + // would vanish from the record. Quiet is earned by the fallback that + // worked; it is never extended to this. Driven through the exported + // `readAt` because `classifyRange` cannot reach the branch: its `sha` + // comes from `--diff-filter=A` and therefore always has the path. + const probe = join(tmp, 'probe-both-reads-fail.mjs'); + writeFileSync( + probe, + `import { readAt } from ${JSON.stringify(pathToFileURL(selfPath).href)};\n` + + `readAt(${JSON.stringify(ui3)}, ${JSON.stringify(release3)}, ${JSON.stringify(base3)}, ` + + `'.changeset/never-existed.md');\n`, + ); + const bothFail = spawnSync(process.execPath, [probe], { encoding: 'utf8' }); + check( + '#6175 when BOTH reads fail the failure is LOUD, naming both attempts', + bothFail.status !== 0 && + bothFail.stderr.includes('cannot read .changeset/never-existed.md') && + bothFail.stderr.includes(release3) && + bothFail.stderr.includes(base3), + bothFail.stderr, + ); + check( + "#6175 both attempts' git diagnostics are re-emitted, not swallowed", + (bothFail.stderr.match(/fatal:/g) ?? []).length >= 2, + bothFail.stderr, + ); } finally { rmSync(tmp, { recursive: true, force: true }); }