fix(scripts): a failing README write must surface, not read as "missing"

applyRelease()'s `try` spanned both the catalog README read AND its write, so a
real EACCES/ENOSPC on the write was swallowed and reported as readme:'missing'
("no catalog README to update") with verdict WROTE and exit 0 — a bumped ref
with a stale label, announced as success. Pre-existing form, not a regression.

The `try` now covers the READ only: a catalog without a README stays a tolerated
state, a README that cannot be written throws.

Known narrow guarantee: the throw propagates out of main() AFTER marketplace.json
is written, so the working tree is left half-applied (bumped ref, stale label).
That is the same hazard ac7ad42 closed for gate ordering — but strictly better
than today's silent exit 0, and widening the fix is not in scope here.

Test is path-selective on purpose: a fake that throws for every path dies on the
marketplace write above the try (in both the old and the new code) and would go
green against the unfixed file. Verified red before the fix.

Tests 25 -> 26 (suite total 131 -> 132).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A839v1MGa3Bm8icX8dTHYi
This commit is contained in:
Kjell Tore Guttormsen 2026-08-10 21:00:40 +02:00
commit 06ede0ebe9
2 changed files with 31 additions and 2 deletions

View file

@ -152,9 +152,20 @@ export function applyRelease({ plan, catalogDir, mktPath, readmePath }, io) {
writes.push(mktPath);
// Keep the human-facing catalog README label in lock-step with the ref (gated by check-versions).
// The `try` covers the READ only: a catalog without a README is a tolerated state, but a README
// that cannot be WRITTEN is a real failure and must surface. The wider try reported EACCES/ENOSPC
// on the write as readme:'missing' ("no catalog README to update") with verdict WROTE and exit 0 —
// a bumped ref with a stale label, announced as success.
let readme;
let readmeText;
try {
const newReadme = reconcileReadmeLabel(io.readFileSync(readmePath, 'utf8'), plan.name, plan.newRef);
readmeText = io.readFileSync(readmePath, 'utf8');
} catch { readmeText = null; }
if (readmeText === null) {
readme = 'missing';
} else {
const newReadme = reconcileReadmeLabel(readmeText, plan.name, plan.newRef);
if (newReadme !== null) {
io.writeFileSync(readmePath, newReadme, 'utf8');
writes.push(readmePath);
@ -162,7 +173,7 @@ export function applyRelease({ plan, catalogDir, mktPath, readmePath }, io) {
} else {
readme = 'unchanged';
}
} catch { readme = 'missing'; }
}
return { verdict: 'WROTE', preflightErrors: [], writes, readme };
}

View file

@ -211,6 +211,24 @@ test('a missing catalog README does not abort the ref bump', () => {
assert.deepEqual(io.writes, [paths.mktPath]);
});
test('a FAILING README write surfaces — it must NOT be misreported as a missing README', () => {
// The `try` used to span the README read AND the README write, so a real EACCES/ENOSPC
// on the write came back as readme:'missing' ("no catalog README to update") with verdict
// WROTE and exit 0 — a bumped ref with a stale label, reported as success.
// Path-selective on purpose: a fake that throws for EVERY path dies on the marketplace
// write at the top of applyRelease (outside the try, before and after the fix), which
// would make this test green against the unfixed file.
const plan = planRelease({ marketplace: marketplace(), name: 'alpha', observed: observed() });
const io = fakeIo(gateResult({ alpha: 'OK' }));
const record = io.writeFileSync;
io.writeFileSync = (p, ...rest) => {
if (p === paths.readmePath) throw new Error('EACCES: permission denied');
return record(p, ...rest);
};
assert.throws(() => applyRelease({ plan, ...paths }, io), /EACCES/);
assert.deepEqual(io.writes, [paths.mktPath], 'the ref write happened; the README write is what failed');
});
// --- shouldCreateTag: --create-tag is a WRITE, so it must obey --write --------
//
// `--create-tag` mints AND PUSHES a tag to a public remote — the one genuinely