mirror of
https://github.com/4gray/iptvnator.git
synced 2026-10-16 06:06:18 -08:00
fix(release): act on the third Codex pass
Four findings, each with a regression test: - An empty `.changes/` directory was reported as an internal-only release. The realistic cause is this step running after `--consume` deleted the notes, which is the one ordering mistake the pipeline exists to prevent — calling it "internal-only" hid it and shipped no cards at all. It is now a hard failure naming that cause. - A release that turned internal-only after cards were generated for the same version returned before the cleanup pass, leaving obsolete images in the output directory. Cleanup is now a named function both paths use, and `--dry-run` still deletes nothing. - Nothing enforces the lowercase-slug filename convention on notes, so `Player_New-UI.md` produced `card-player_new-ui.png`, which `isOwnedCardFile` cannot match and no later run could ever remove. Card slugs are normalized to the character set the predicate accepts, with collisions from that normalization broken by a numeric suffix. - The pipeline doc's surface table still named the old minor-scoped card directory while the prose below it named the new per-version one. 185 tests passing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
1417632f73
commit
8c3aebecac
4 files changed
+244
-29
No files matched your search
@@ -27,7 +27,7 @@ chosen deliberately by bumping `package.json`.
|
||||
| GitHub release body | (tag build) | via `extract-changelog-section.mjs --public` |
|
||||
| Telegram announcement | `release:notes:telegram` | stdout |
|
||||
| Reddit announcement | `release:notes:reddit` | stdout |
|
||||
| Highlight cards | `release:cards:generate` | `dist/release-highlight-cards/<vX-Y>/` |
|
||||
| Highlight cards | `release:cards:generate` | `dist/release-highlight-cards/v<version>/` |
|
||||
| Screenshots | `release:screenshots` | `apps/website/public/blog/<vX-Y>/screenshots/` |
|
||||
|
||||
### The ordering constraint that matters
|
||||
@@ -100,7 +100,11 @@ card into the website tree is a deliberate manual act.
|
||||
|
||||
A release with no `highlight:` notes is not an error: the hero card is still
|
||||
rendered and the run exits 0. An internal-only release has nothing public to
|
||||
put on a card and exits 0 having written nothing.
|
||||
put on a card and exits 0, first clearing any cards an earlier run of the same
|
||||
version left behind. An **empty** `.changes/` directory is a different thing
|
||||
and does fail: it almost always means this step ran after `--consume`, and
|
||||
reporting that as "internal-only" would hide the one ordering mistake the
|
||||
pipeline is built to prevent.
|
||||
|
||||
## Draft verification
|
||||
|
||||
|
||||
@@ -140,6 +140,29 @@ async function prepareShotOverlay(screenshotPath) {
|
||||
return { input: cropped, left: SHOT_LEFT, top: SHOT_TOP };
|
||||
}
|
||||
|
||||
/**
|
||||
* Removes the cards a previous run of THIS generator left in `outputDir`, so a
|
||||
* renamed, dropped, or newly-internal highlight cannot leave a stale image
|
||||
* sitting there waiting to be published. Only files matching what this tool
|
||||
* writes are touched; anything else in the directory is left alone.
|
||||
*
|
||||
* @param {string} outputDir
|
||||
* @returns {string[]} removed filenames
|
||||
*/
|
||||
export function removeStaleCards(outputDir) {
|
||||
if (!existsSync(outputDir)) {
|
||||
return [];
|
||||
}
|
||||
|
||||
const stale = readdirSync(outputDir).filter(isOwnedCardFile);
|
||||
|
||||
for (const entry of stale) {
|
||||
rmSync(path.join(outputDir, entry));
|
||||
}
|
||||
|
||||
return stale;
|
||||
}
|
||||
|
||||
/**
|
||||
* @param {{ feature: object[], hero: object }} plan
|
||||
* @param {string} version
|
||||
@@ -148,16 +171,7 @@ async function prepareShotOverlay(screenshotPath) {
|
||||
*/
|
||||
export async function renderCards(plan, version, outputDir) {
|
||||
mkdirSync(outputDir, { recursive: true });
|
||||
|
||||
// Remove the cards a previous run of THIS generator left behind, so a
|
||||
// renamed or dropped highlight cannot leave a stale image sitting beside
|
||||
// the current set waiting to be published. Only files matching what this
|
||||
// tool writes are touched; anything else in the directory is left alone.
|
||||
for (const entry of readdirSync(outputDir)) {
|
||||
if (isOwnedCardFile(entry)) {
|
||||
rmSync(path.join(outputDir, entry));
|
||||
}
|
||||
}
|
||||
removeStaleCards(outputDir);
|
||||
|
||||
const written = [];
|
||||
|
||||
@@ -210,6 +224,22 @@ async function main() {
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
// An empty directory is not an internal-only release: it is almost always
|
||||
// this step running AFTER `--consume` deleted the notes, which is the one
|
||||
// ordering mistake the whole pipeline warns about. Reporting it as
|
||||
// "internal-only" would hide the mistake and quietly ship no cards at all.
|
||||
if (notes.length === 0) {
|
||||
console.error(
|
||||
[
|
||||
`No notes found in ${path.relative(workspaceRoot, notesDir)}/.`,
|
||||
'Cards are built from `highlight:`, which exists only in the note',
|
||||
'files — if `build-release-notes.mjs --consume` already ran, that',
|
||||
'metadata is gone. Render cards before consuming.',
|
||||
].join(' ')
|
||||
);
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
const slug = releaseSlug(options.version);
|
||||
const screenshotsDir = path.join(
|
||||
workspaceRoot,
|
||||
@@ -223,6 +253,14 @@ async function main() {
|
||||
screenshotsDir,
|
||||
theme: options.theme,
|
||||
});
|
||||
// Keyed by the exact version, not the minor slug: 0.24.0 and 0.24.1 share
|
||||
// a blog post but not a card set, and mixing them in one directory invites
|
||||
// publishing the previous patch's card.
|
||||
const outputDir = path.resolve(
|
||||
workspaceRoot,
|
||||
options.out ??
|
||||
path.join('dist/release-highlight-cards', `v${options.version}`)
|
||||
);
|
||||
|
||||
// Neither shape below is an error: an internal-only release is legal, and
|
||||
// a release nobody marked a highlight on still deserves its hero card.
|
||||
@@ -232,6 +270,19 @@ async function main() {
|
||||
'Internal-only release: no public change to put on a card.'
|
||||
);
|
||||
|
||||
// A release that turned internal-only after cards were already made
|
||||
// for this exact version must not leave them behind. `--dry-run`
|
||||
// deletes nothing.
|
||||
if (options.mode === 'generate') {
|
||||
const removed = removeStaleCards(outputDir);
|
||||
|
||||
if (removed.length > 0) {
|
||||
console.log(
|
||||
`Removed ${removed.length} card(s) left by an earlier run of v${options.version}.`
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -256,15 +307,6 @@ async function main() {
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
// Keyed by the exact version, not the minor slug: 0.24.0 and 0.24.1 share
|
||||
// a blog post but not a card set, and mixing them in one directory invites
|
||||
// publishing the previous patch's card.
|
||||
const outputDir = path.resolve(
|
||||
workspaceRoot,
|
||||
options.out ??
|
||||
path.join('dist/release-highlight-cards', `v${options.version}`)
|
||||
);
|
||||
|
||||
console.log(`${plan.feature.length} highlight card(s) + hero for v${options.version}:`);
|
||||
for (const job of plan.feature) {
|
||||
const shot = job.screenshotPath
|
||||
@@ -280,14 +322,14 @@ async function main() {
|
||||
return;
|
||||
}
|
||||
|
||||
if (existsSync(outputDir)) {
|
||||
const stale = readdirSync(outputDir).filter(isOwnedCardFile);
|
||||
const stale = existsSync(outputDir)
|
||||
? readdirSync(outputDir).filter(isOwnedCardFile)
|
||||
: [];
|
||||
|
||||
if (stale.length > 0) {
|
||||
console.log(
|
||||
`Replacing ${stale.length} card(s) from a previous run in the same directory.`
|
||||
);
|
||||
}
|
||||
if (stale.length > 0) {
|
||||
console.log(
|
||||
`Replacing ${stale.length} card(s) from a previous run in the same directory.`
|
||||
);
|
||||
}
|
||||
|
||||
const written = await renderCards(plan, options.version, outputDir);
|
||||
|
||||
@@ -119,12 +119,23 @@ export function planHighlightCards(notes, options) {
|
||||
.flatMap((group) => group.notes);
|
||||
const highlights = ordered.filter((note) => note.highlight);
|
||||
|
||||
const takenSlugs = new Set();
|
||||
|
||||
const feature = highlights.map((note) => {
|
||||
// Named after the note file, never the screenshot slug: filenames are
|
||||
// unique within `.changes/`, while two highlights may legitimately
|
||||
// point at the same manifest shot — naming cards after it would let
|
||||
// one silently overwrite the other.
|
||||
const slug = path.basename(note.sourcePath, '.md').toLowerCase();
|
||||
const base = cardSlug(path.basename(note.sourcePath, '.md'));
|
||||
// Normalizing can collapse two distinct names onto one, so uniqueness
|
||||
// is re-established here rather than assumed.
|
||||
let slug = base;
|
||||
|
||||
for (let suffix = 2; takenSlugs.has(slug); suffix += 1) {
|
||||
slug = `${base}-${suffix}`;
|
||||
}
|
||||
|
||||
takenSlugs.add(slug);
|
||||
|
||||
return {
|
||||
slug,
|
||||
@@ -169,6 +180,24 @@ export function isOwnedCardFile(fileName) {
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Every emitted filename must satisfy isOwnedCardFile(), or a later run cannot
|
||||
* reclaim the card it wrote. Note filenames are conventionally lowercase slugs
|
||||
* but nothing enforces it, so normalize rather than trust: `player_new-ui.md`
|
||||
* would otherwise produce a card no cleanup pass can ever remove.
|
||||
*
|
||||
* @param {string} noteBaseName note filename without its `.md` extension
|
||||
* @returns {string}
|
||||
*/
|
||||
export function cardSlug(noteBaseName) {
|
||||
const normalized = noteBaseName
|
||||
.toLowerCase()
|
||||
.replace(/[^a-z0-9]+/g, '-')
|
||||
.replace(/^-+|-+$/g, '');
|
||||
|
||||
return normalized || 'note';
|
||||
}
|
||||
|
||||
function backgroundDefs() {
|
||||
return [
|
||||
'<defs>',
|
||||
|
||||
@@ -9,6 +9,8 @@ import {
|
||||
} from 'node:fs';
|
||||
import { tmpdir } from 'node:os';
|
||||
import path from 'node:path';
|
||||
import { spawnSync } from 'node:child_process';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
import { after, describe, it } from 'node:test';
|
||||
import sharp from 'sharp';
|
||||
|
||||
@@ -26,6 +28,7 @@ import {
|
||||
} from './highlight-cards.mjs';
|
||||
import {
|
||||
parseCardArguments,
|
||||
removeStaleCards,
|
||||
renderCards,
|
||||
} from './generate-highlight-cards.mjs';
|
||||
|
||||
@@ -178,6 +181,32 @@ describe('planHighlightCards', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('emits only filenames the ownership predicate can reclaim', () => {
|
||||
// Nothing enforces the lowercase-slug filename convention, and a card
|
||||
// the cleanup pass cannot recognize is a card that lives forever.
|
||||
const plan = planHighlightCards(
|
||||
[
|
||||
note({
|
||||
highlight: 'New UI',
|
||||
sourcePath: '.changes/Player_New-UI.md',
|
||||
}),
|
||||
note({
|
||||
highlight: 'Other',
|
||||
sourcePath: '.changes/player.new.ui.md',
|
||||
}),
|
||||
],
|
||||
planOptions
|
||||
);
|
||||
|
||||
assert.deepEqual(
|
||||
plan.feature.map((job) => job.fileName),
|
||||
['card-player-new-ui.png', 'card-player-new-ui-2.png']
|
||||
);
|
||||
for (const job of plan.feature) {
|
||||
assert.equal(isOwnedCardFile(job.fileName), true, job.fileName);
|
||||
}
|
||||
});
|
||||
|
||||
it('claims only the files it writes', () => {
|
||||
for (const owned of ['card-playback-up-next.png', 'hero.png', 'hero.jpg']) {
|
||||
assert.equal(isOwnedCardFile(owned), true, owned);
|
||||
@@ -400,6 +429,23 @@ describe('renderCards', () => {
|
||||
}
|
||||
});
|
||||
|
||||
it('removeStaleCards tolerates a missing directory and reports what it took', () => {
|
||||
const outputDir = path.join(makeTempDir(), 'never-created');
|
||||
|
||||
assert.deepEqual(removeStaleCards(outputDir), []);
|
||||
|
||||
mkdirSync(outputDir, { recursive: true });
|
||||
writeFileSync(path.join(outputDir, 'card-gone.png'), 'x');
|
||||
writeFileSync(path.join(outputDir, 'hero.jpg'), 'x');
|
||||
writeFileSync(path.join(outputDir, 'keep.txt'), 'x');
|
||||
|
||||
assert.deepEqual(removeStaleCards(outputDir).sort(), [
|
||||
'card-gone.png',
|
||||
'hero.jpg',
|
||||
]);
|
||||
assert.deepEqual(readdirSync(outputDir), ['keep.txt']);
|
||||
});
|
||||
|
||||
it('removes its own stale cards but leaves other files alone', async () => {
|
||||
const outputDir = path.join(makeTempDir(), 'cards');
|
||||
|
||||
@@ -424,3 +470,97 @@ describe('renderCards', () => {
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('generate-highlight-cards CLI', () => {
|
||||
const CLI = path.resolve(
|
||||
path.dirname(fileURLToPath(import.meta.url)),
|
||||
'generate-highlight-cards.mjs'
|
||||
);
|
||||
|
||||
function runCli(args) {
|
||||
const result = spawnSync(process.execPath, [CLI, ...args], {
|
||||
encoding: 'utf8',
|
||||
stdio: ['ignore', 'pipe', 'pipe'],
|
||||
});
|
||||
|
||||
return {
|
||||
status: result.status ?? 1,
|
||||
stdout: result.stdout ?? '',
|
||||
stderr: result.stderr ?? '',
|
||||
};
|
||||
}
|
||||
|
||||
it('fails on an empty notes directory instead of calling it internal-only', () => {
|
||||
// The realistic cause is running this step after `--consume`, which
|
||||
// deleted the notes; silently reporting "internal-only" would hide it.
|
||||
const emptyDir = makeTempDir();
|
||||
const result = runCli([
|
||||
'--generate',
|
||||
'--version',
|
||||
'0.24.0',
|
||||
'--dir',
|
||||
emptyDir,
|
||||
'--out',
|
||||
path.join(makeTempDir(), 'cards'),
|
||||
]);
|
||||
|
||||
assert.equal(result.status, 1);
|
||||
assert.match(result.stderr, /No notes found/);
|
||||
assert.match(result.stderr, /Render cards before consuming/);
|
||||
assert.doesNotMatch(result.stderr, /Internal-only/);
|
||||
});
|
||||
|
||||
it('clears stale cards when a release becomes internal-only', () => {
|
||||
const notesDir = makeTempDir();
|
||||
const outputDir = path.join(makeTempDir(), 'cards');
|
||||
|
||||
writeFileSync(
|
||||
path.join(notesDir, 'deps-bump.md'),
|
||||
'---\ntype: internal\narea: deps\n---\n\nBumped a parser.\n',
|
||||
'utf8'
|
||||
);
|
||||
mkdirSync(outputDir, { recursive: true });
|
||||
writeFileSync(path.join(outputDir, 'card-old-feature.png'), 'stale');
|
||||
writeFileSync(path.join(outputDir, 'keep.txt'), 'mine');
|
||||
|
||||
const result = runCli([
|
||||
'--generate',
|
||||
'--version',
|
||||
'0.24.0',
|
||||
'--dir',
|
||||
notesDir,
|
||||
'--out',
|
||||
outputDir,
|
||||
]);
|
||||
|
||||
assert.equal(result.status, 0);
|
||||
assert.match(result.stderr, /Internal-only release/);
|
||||
assert.deepEqual(readdirSync(outputDir), ['keep.txt']);
|
||||
});
|
||||
|
||||
it('leaves stale cards untouched in dry-run mode', () => {
|
||||
const notesDir = makeTempDir();
|
||||
const outputDir = path.join(makeTempDir(), 'cards');
|
||||
|
||||
writeFileSync(
|
||||
path.join(notesDir, 'deps-bump.md'),
|
||||
'---\ntype: internal\narea: deps\n---\n\nBumped a parser.\n',
|
||||
'utf8'
|
||||
);
|
||||
mkdirSync(outputDir, { recursive: true });
|
||||
writeFileSync(path.join(outputDir, 'card-old-feature.png'), 'stale');
|
||||
|
||||
const result = runCli([
|
||||
'--dry-run',
|
||||
'--version',
|
||||
'0.24.0',
|
||||
'--dir',
|
||||
notesDir,
|
||||
'--out',
|
||||
outputDir,
|
||||
]);
|
||||
|
||||
assert.equal(result.status, 0);
|
||||
assert.deepEqual(readdirSync(outputDir), ['card-old-feature.png']);
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user