Skip to content

Commit f64b61c

Browse files
fix(wire-shape): close 4 review gaps from #182 + #184 follow-up
CI-internal hardenings; no user-visible behaviour change. - refresh.js: `--sha` with no value used to be silently consumed as the positional `specsDir`, masking the missing argument and routing the script through the gitHeadSHA fallback. Now exits 2 with a clear message. - validate.js: an empty `registered_types` paired with non-empty cross_spec / intra_file / per_type_drift entries used to silently disable Gate 4 (rename-escape). The validator now treats this combination as a baseline corruption and fails the run. A genuinely fresh baseline (every section empty) still skips the gate as before, since the first-pin run has nothing to guard. - lib.js writeBaseline: a crash between writeFileSync(tmp, ...) and renameSync(tmp, BASELINE_PATH) used to leave a `.tmp.<pid>` sidecar, which a future writer with the recycled PID would collide on. Now wraps in try/catch + unlinkSync on any throw. - lib.js extractInterfaceProps: an `interface X extends Y` would silently under-report fields because we walk only iface.members. No such interface ships in the SDK today; the gate now warns loudly the first time one appears so coverage gets resolved before drift can hide. Full heritage walking is the proper fix if this becomes a routine pattern. Verifications: - refresh.js with `--sha` as last token exits 2; with `--sha foo` works. - validate.js with corrupted baseline (empty registered_types but populated drift/cross_spec) exits 1 with explanatory message. - writeBaseline temp-file is cleaned up on synthetic write error. - Real baseline regenerates byte-identical to origin/main.
1 parent 27d6fa5 commit f64b61c

3 files changed

Lines changed: 69 additions & 4 deletions

File tree

scripts/wire-shape/lib.js

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,31 @@ function hasExportModifier(node) {
220220
}
221221

222222
function extractInterfaceProps(iface) {
223+
// A `heritageClauses` (e.g. `interface Foo extends Bar`) means the
224+
// interface inherits members from its parents that we won't see by
225+
// walking iface.members alone. The wire-shape gate would then
226+
// under-report fields and silently miss drift on inherited
227+
// properties. The TS SDK has no such interfaces today; flag it
228+
// loudly the first time one appears so coverage gets resolved
229+
// before it ships, instead of slipping past as invisible
230+
// under-counting.
231+
if (iface.heritageClauses && iface.heritageClauses.length > 0) {
232+
const parents = [];
233+
for (const clause of iface.heritageClauses) {
234+
for (const t of clause.types || []) {
235+
const expr = t.expression;
236+
if (expr && ts.isIdentifier(expr)) {
237+
parents.push(expr.text);
238+
}
239+
}
240+
}
241+
console.warn(
242+
`wire-shape: interface ${iface.name.text} extends [${parents.join(', ')}]; ` +
243+
'inherited fields are NOT walked. Add explicit field flattening to ' +
244+
'scripts/wire-shape/lib.js::extractInterfaceProps before merging an ' +
245+
'extending wire interface, or wire-shape coverage will be incomplete.',
246+
);
247+
}
223248
const names = [];
224249
for (const member of iface.members) {
225250
const name = propertyName(member);
@@ -290,8 +315,19 @@ function writeBaseline(baseline) {
290315
fs.mkdirSync(dir, { recursive: true });
291316
}
292317
const tmp = `${BASELINE_PATH}.tmp.${process.pid}`;
293-
fs.writeFileSync(tmp, JSON.stringify(baseline, null, 2) + '\n');
294-
fs.renameSync(tmp, BASELINE_PATH);
318+
try {
319+
fs.writeFileSync(tmp, JSON.stringify(baseline, null, 2) + '\n');
320+
fs.renameSync(tmp, BASELINE_PATH);
321+
} catch (e) {
322+
// Don't leave a `.tmp.<pid>` sidecar behind — the next run from
323+
// the same PID would collide and confuse a future writer.
324+
try {
325+
fs.unlinkSync(tmp);
326+
} catch {
327+
/* tmp may not exist yet; ignore */
328+
}
329+
throw e;
330+
}
295331
}
296332

297333
function difference(a, b) {

scripts/wire-shape/refresh.js

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,14 @@ function main() {
4444
let specsDir = null;
4545
let explicitSha = null;
4646
for (let i = 0; i < args.length; i += 1) {
47-
if (args[i] === '--sha' && i + 1 < args.length) {
47+
if (args[i] === '--sha') {
48+
if (i + 1 >= args.length) {
49+
// `--sha` as the last token used to be silently consumed as
50+
// a positional `specsDir`, masking the missing value. Fail
51+
// loudly instead.
52+
console.error('error: --sha requires a value (e.g. --sha bf1ca22…)');
53+
process.exit(2);
54+
}
4855
explicitSha = args[i + 1];
4956
i += 1;
5057
} else if (!specsDir) {

scripts/wire-shape/validate.js

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,29 @@ function main() {
223223
}
224224

225225
// Gate 4: registered-type coverage.
226-
if (baseline.registered_types.length > 0) {
226+
// After the first introduction, the baseline always has registered
227+
// types — emptying the list would silently disable the rename-
228+
// escape guard. Fail loudly on an empty list unless the baseline
229+
// is itself empty (genuine first-pin / fresh-repo case where every
230+
// baseline section is empty).
231+
const baselineIsFresh =
232+
baseline.registered_types.length === 0 &&
233+
Object.keys(baseline.per_type_drift).length === 0 &&
234+
Object.keys(baseline.cross_spec_duplicates).length === 0 &&
235+
Object.keys(baseline.intra_file_duplicates).length === 0;
236+
if (baseline.registered_types.length === 0 && !baselineIsFresh) {
237+
console.error(
238+
'baseline.registered_types is empty but other baseline sections are not.',
239+
);
240+
console.error(
241+
'This combination silently disables the rename-escape gate. Regenerate',
242+
);
243+
console.error(
244+
'tests/fixtures/wire-shape-baseline.json via scripts/wire-shape/refresh.js,',
245+
);
246+
console.error('or remove the file entirely if introducing a fresh baseline.');
247+
errors += 1;
248+
} else if (baseline.registered_types.length > 0) {
227249
const missingSDK = [];
228250
const missingSpec = [];
229251
for (const name of baseline.registered_types) {

0 commit comments

Comments
 (0)