feedback1 integrate: fetch-assets refuses to shrink data/assets.json without --allow-shrink or --prune (pointed out in PR #7)

A rebuilt manifest leaves out every entry whose files are missing on that machine, so a run
with failed downloads silently dropped entries from the tracked file (PR #7 carried one 42
audio entries short; all 42 still resolve upstream). The run now keeps the current manifest,
prints the entries it would drop (also in the report) and exits 1. The tool runs main only as
a script so the guard and parseArgs can be tested; ASSETS.md documents the flag.

docs/ASSETS.md

@@ -20,9 +20,19 @@ npm run assets       # = node tools/vendor.mjs && node tools/fetch-assets.mjs
 | `--offline` | No network. Re-runs post-processing (atlas fixes, skeleton parsing, WOFF2) on what is already on disk, then rebuilds `data/assets.json`. |
 | `--dry-run` | Print the plan (file and model counts, alias notes) and exit. |
 | `--refresh-index` | Re-download the two upstream indexes: `audio_data.json` and `models_data.json`. |
-| `--prune` | Delete files under `public/assets/` that the manifest no longer references, for example after a mapping change. Without this flag they are only listed in the report. `public/assets/local/` (written by `tools/local-extract`) is never pruned. |
+| `--prune` | Delete files under `public/assets/` that the manifest no longer references, for example after a mapping change. Without this flag they are only listed in the report. `public/assets/local/` (written by `tools/local-extract`) is never pruned. Implies `--allow-shrink`. |
+| `--allow-shrink` | Write `data/assets.json` even when it loses entries the current one has (see "The manifest never shrinks by accident" below). |
 | `--local-spines` | Rewrite `tools/assets/local-enemy-spines.json` (the metadata of the enemy models only the local client has, see "Enemy aliases") from the models `tools/local-extract/extract.py` extracted to `public/assets/local/spine/enemy/`. Run it after a game update changed them; without it the committed file is used and a differing extraction only gets a warning. |
 
+**The manifest never shrinks by accident.** An entry whose files are missing on this machine is left out of a rebuilt
+manifest, so a run where some downloads failed (or whose upstream audio / model index lost them) would drop entries that
+every other install still has from the committed `data/assets.json` (a pull request once carried such a manifest, 42
+audio entries short — all 42 still resolve upstream; PR #7). When the rebuilt manifest lacks an entry of the current
+one, the run keeps the current file, prints the entries it would drop (also in the report: `droppedEntries`,
+`manifestWritten: false`) and exits 1. Re-run to retry the downloads, or pass `--allow-shrink` (or `--prune`) when the
+smaller manifest is intended, for example after a mapping change. Build fields (`version`, `hash`, `generator`,
+`stats`), new entries and a changed value are never a drop (`tools/assets/manifest.mjs droppedEntries`).
+
 The script is **idempotent**. A file on disk is kept, not re-downloaded, when any one of these holds:
 - its size matches the ledger entry from a previous download (`.cache/assets-ledger.json`);
 - its size matches the byte count recorded in research;

test/fetch-assets-shrink.test.js

@@ -0,0 +1,79 @@
+// tools/fetch-assets.mjs never shrinks the committed data/assets.json by accident. A rebuilt manifest leaves out every
+// entry whose files are missing on this machine, so a run where some downloads failed (or whose upstream index lost
+// them) dropped entries every other install still has — a pull request (PR #7) carried such a manifest, 42 audio
+// entries short. The run now keeps the current manifest, lists the entries it would drop and exits 1, unless
+// --allow-shrink (or --prune) is passed. DESIGN §21, docs/ASSETS.md.
+import { test } from 'node:test';
+import assert from 'node:assert/strict';
+import { readFileSync, statSync } from 'node:fs';
+import { spawnSync } from 'node:child_process';
+import { join, dirname } from 'node:path';
+import { fileURLToPath } from 'node:url';
+import { droppedEntries } from '../tools/assets/manifest.mjs';
+
+const ROOT = join(dirname(fileURLToPath(import.meta.url)), '..');
+const MANIFEST = join(ROOT, 'data', 'assets.json');
+
+test('droppedEntries: leaves of the old manifest the new one lacks; build fields, additions and shape changes are no drops', () => {
+  const prev = {
+    version: 1, hash: 'a', generator: 'x', stats: { files: 3 },
+    audio: { sfx: { ui: { timer: '/t.mp3', click: '/c.mp3' }, units: { u1: { hit: '/h.mp3', skills: { 0: '/s.mp3' } } } } },
+    chars: { c1: { avatar: '/a.png', spine: { textures: ['/x.png'] } }, c2: { avatar: '/b.png' } },
+    ui: { gone: {} },
+  };
+  const next = {
+    version: 2, hash: 'b', generator: 'y', stats: { files: 9 },
+    audio: { sfx: { ui: { click: '/c2.mp3', extra: '/e.mp3' }, units: {} } },
+    chars: { c1: { avatar: { lo: '/a.png' }, spine: { textures: [] } }, c3: { avatar: '/z.png' } },
+  };
+  assert.deepEqual(droppedEntries(prev, next), [
+    'audio.sfx.ui.timer', 'audio.sfx.units.u1.hit', 'audio.sfx.units.u1.skills.0', 'chars.c2.avatar',
+  ]);
+  assert.deepEqual(droppedEntries(next, prev), ['audio.sfx.ui.extra', 'chars.c3.avatar'], 'the other way round');
+  assert.deepEqual(droppedEntries(prev, prev), []);
+  assert.deepEqual(droppedEntries(null, next), [], 'no current manifest: nothing to lose');
+});
+
+test('fetch-assets shrink guard: the committed manifest minus some audio entries (PR #7) is refused unless --allow-shrink / --prune', async () => {
+  const { shrinkGuard, parseArgs } = await import('../tools/fetch-assets.mjs');
+  const before = statSync(MANIFEST).mtimeMs;
+  const prev = JSON.parse(readFileSync(MANIFEST, 'utf8'));
+  const next = JSON.parse(JSON.stringify(prev));
+  const gone = [];
+  const drop = (path) => {
+    const keys = path.split('.');
+    const parent = keys.slice(0, -1).reduce((o, k) => o?.[k], next);
+    if (parent && Object.hasOwn(parent, keys.at(-1))) { delete parent[keys.at(-1)]; gone.push(path); }
+  };
+  // what PR #7's manifest lacked: UI sounds, boss BGM intros, per-unit sounds
+  for (const k of ['timer', 'draft', 'battleStart', 'artPlace', 'killBossAll']) drop(`audio.sfx.ui.${k}`);
+  for (const b of ['boss_8', 'boss_9', 'boss_10']) drop(`audio.bossBgm.${b}.intro`);
+  const unit = Object.entries(prev.audio.sfx.units).find(([, v]) => v && typeof v.hit === 'string');
+  drop(`audio.sfx.units.${unit[0]}.hit`);
+  assert.ok(gone.length >= 6, `the fixture drops real entries (${gone})`);
+  next.stats = { ...next.stats, files: next.stats.files - gone.length };
+  next.hash = 'changed';
+  next.chars = { ...next.chars, char_new_test: { avatar: '/assets/char/avatar/new.png' } };
+
+  const refused = shrinkGuard(prev, next, parseArgs([]));
+  assert.equal(refused.write, false, 'a smaller manifest is not written by default');
+  assert.deepEqual(refused.dropped, gone.slice().sort(), 'and the run names exactly the entries it would drop');
+  assert.equal(shrinkGuard(prev, next, parseArgs(['--allow-shrink'])).write, true, '--allow-shrink writes it');
+  assert.equal(shrinkGuard(prev, next, parseArgs(['--prune'])).write, true, '--prune writes it');
+  assert.equal(shrinkGuard(prev, next, parseArgs(['--offline'])).write, false, '--offline is guarded too');
+  const grown = JSON.parse(JSON.stringify(prev));
+  grown.chars.char_new_test = { avatar: '/x.png' };
+  grown.stats.files += 1;
+  assert.deepEqual(shrinkGuard(prev, grown, parseArgs([])), { dropped: [], write: true }, 'a manifest that only grows is written');
+  assert.deepEqual(shrinkGuard(null, next, parseArgs([])), { dropped: [], write: true }, 'no current manifest: written');
+  assert.equal(parseArgs(['--allow-shrink']).allowShrink, true);
+  assert.throws(() => parseArgs(['--allow-shrinks']), /unknown option/);
+  assert.equal(statSync(MANIFEST).mtimeMs, before, 'importing the tool runs nothing');
+});
+
+test('fetch-assets still runs as a script: --help lists --allow-shrink', () => {
+  const r = spawnSync(process.execPath, [join(ROOT, 'tools', 'fetch-assets.mjs'), '--help'], { encoding: 'utf8', timeout: 30000 });
+  assert.equal(r.status, 0, r.stderr);
+  assert.match(r.stdout, /--allow-shrink/);
+  assert.match(r.stdout, /--prune .*\n.*implies --allow-shrink/);
+});

tools/assets/manifest.mjs

@@ -146,3 +146,35 @@ export function totalBytes(root, rels) {
 export function contentHash(value) {
   return createHash('sha1').update(JSON.stringify(value)).digest('hex').slice(0, 12);
 }
+
+/** Top-level manifest fields that describe the build, not assets: never counted as dropped entries. */
+const BUILD_FIELDS = new Set(['version', 'hash', 'generator', 'stats']);
+
+/**
+ * Entries of the manifest `prev` that `next` no longer has, as sorted dotted paths. An entry is a leaf value (a URL
+ * string, a number, a flag, null or an array — an array is one entry); an object entry missing from `next` contributes
+ * all its leaves. A path that changed shape (a leaf became an object or the reverse) is still there, not dropped.
+ * The build fields (version, hash, generator, stats) are left out.
+ * @param {object} prev the current data/assets.json
+ * @param {object} next the manifest about to replace it
+ * @returns {string[]}
+ */
+export function droppedEntries(prev, next) {
+  const out = [];
+  const isObj = (v) => !!v && typeof v === 'object' && !Array.isArray(v);
+  const leaves = (v, path) => {
+    if (!isObj(v)) { out.push(path); return; }
+    for (const [k, x] of Object.entries(v)) leaves(x, `${path}.${k}`);
+  };
+  const walk = (p, n, path) => {
+    if (n === undefined) { leaves(p, path); return; }
+    if (!isObj(p) || !isObj(n)) return;
+    for (const [k, v] of Object.entries(p)) walk(v, Object.hasOwn(n, k) ? n[k] : undefined, `${path}.${k}`);
+  };
+  if (!isObj(prev)) return out;
+  const nx = isObj(next) ? next : {};
+  for (const [k, v] of Object.entries(prev)) {
+    if (!BUILD_FIELDS.has(k)) walk(v, Object.hasOwn(nx, k) ? nx[k] : undefined, k);
+  }
+  return out.sort();
+}

tools/fetch-assets.mjs

@@ -17,20 +17,27 @@
 // jsDelivr mirror fallback. Spine atlases get `size:` (and `pma: true` for
 // enemies); every skeleton is parsed to resolve animation roles.
 //
+// The committed data/assets.json never shrinks by accident: an entry whose files
+// are missing here is left out of a rebuilt manifest, so a run on a machine where
+// some downloads failed (or whose upstream index lost them) would drop entries
+// every other install still has. The run then keeps the current manifest, lists
+// the entries it would drop and exits 1; --allow-shrink (or --prune) writes the
+// smaller manifest.
+//
 // Usage: node tools/fetch-assets.mjs [--concurrency=16] [--force] [--offline]
 //                                    [--dry-run] [--refresh-index] [--prune]
-//                                    [--local-spines] [--help]
+//                                    [--allow-shrink] [--local-spines] [--help]
 
 import { readFile, writeFile, mkdir, rename, readdir, unlink } from 'node:fs/promises';
-import { existsSync } from 'node:fs';
+import { existsSync, realpathSync } from 'node:fs';
 import { dirname, join, relative, sep } from 'node:path';
-import { fileURLToPath } from 'node:url';
+import { fileURLToPath, pathToFileURL } from 'node:url';
 import { Downloader } from './assets/downloader.mjs';
 import { loadIndexes } from './assets/cache.mjs';
 import { indexAudio } from './assets/audio.mjs';
 import { buildPlan } from './assets/plan.mjs';
 import { processModels, findLocalEnemyModels, localEnemySpineMeta, loadLocalEnemySpines, LOCAL_ENEMY_SPINES_FILE } from './assets/spine.mjs';
-import { collectLeaves, downloadLeaves, resolveTemplate, totalBytes, contentHash, MANIFEST_VERSION } from './assets/manifest.mjs';
+import { collectLeaves, downloadLeaves, resolveTemplate, totalBytes, contentHash, droppedEntries, MANIFEST_VERSION } from './assets/manifest.mjs';
 import { fontJobs, buildFonts } from './assets/fonts.mjs';
 import { skelParserAvailable } from './assets/skel.mjs';
 
@@ -49,7 +56,9 @@ const HELP = `Usage: node tools/fetch-assets.mjs [options]
   --dry-run         print the plan and exit
   --refresh-index   re-download audio_data.json / models_data.json indexes
   --prune           delete files under public/assets that the manifest no longer references
-                    (public/assets/local/** of tools/local-extract is never deleted)
+                    (public/assets/local/** of tools/local-extract is never deleted); implies --allow-shrink
+  --allow-shrink    write data/assets.json even when it loses entries the current one has
+                    (without it such a run keeps the current manifest, lists the entries and exits 1)
   --local-spines    rewrite ${LOCAL_ENEMY_SPINES_FILE} from the enemy models extracted
                     by tools/local-extract/extract.py (public/assets/local/spine/enemy/)
   --help            this text`;
@@ -57,10 +66,10 @@ const HELP = `Usage: node tools/fetch-assets.mjs [options]
 /**
  * Parse CLI flags.
  * @param {string[]} argv
- * @returns {{concurrency:number, force:boolean, offline:boolean, dryRun:boolean, refreshIndex:boolean, prune:boolean, localSpines:boolean, help:boolean}}
+ * @returns {{concurrency:number, force:boolean, offline:boolean, dryRun:boolean, refreshIndex:boolean, prune:boolean, allowShrink:boolean, localSpines:boolean, help:boolean}}
  */
-function parseArgs(argv) {
-  const o = { concurrency: 16, force: false, offline: false, dryRun: false, refreshIndex: false, prune: false, localSpines: false, help: false };
+export function parseArgs(argv) {
+  const o = { concurrency: 16, force: false, offline: false, dryRun: false, refreshIndex: false, prune: false, allowShrink: false, localSpines: false, help: false };
   for (const a of argv) {
     const [k, v] = a.split('=');
     if (k === '--concurrency') o.concurrency = Math.max(1, Math.min(64, parseInt(v, 10) || 16));
@@ -69,6 +78,7 @@ function parseArgs(argv) {
     else if (k === '--dry-run') o.dryRun = true;
     else if (k === '--refresh-index') o.refreshIndex = true;
     else if (k === '--prune') o.prune = true;
+    else if (k === '--allow-shrink') o.allowShrink = true;
     else if (k === '--local-spines') o.localSpines = true;
     else if (k === '--help' || k === '-h') o.help = true;
     else throw new Error(`unknown option ${a}\n${HELP}`);
@@ -76,6 +86,20 @@ function parseArgs(argv) {
   return o;
 }
 
+/**
+ * The shrink guard of data/assets.json (header): the entries of the current manifest `prev` that `next` would drop,
+ * and whether `next` may be written — always when nothing is dropped (or there is no current manifest), otherwise
+ * only with --allow-shrink or --prune.
+ * @param {object|null} prev
+ * @param {object} next
+ * @param {{allowShrink?:boolean, prune?:boolean}} opts
+ * @returns {{dropped:string[], write:boolean}}
+ */
+export function shrinkGuard(prev, next, { allowShrink = false, prune = false } = {}) {
+  const dropped = prev ? droppedEntries(prev, next) : [];
+  return { dropped, write: dropped.length === 0 || !!allowShrink || !!prune };
+}
+
 const mb = (n) => `${(n / 1048576).toFixed(1)} MB`;
 const log = (m) => console.log(m);
 
@@ -259,7 +283,12 @@ async function main() {
     stats: countStats(body, bytes, resolved.files.size),
     ...body,
   };
-  await writeJsonAtomic(MANIFEST, manifest);
+  let current = null;
+  if (existsSync(MANIFEST)) {
+    try { current = JSON.parse(await readFile(MANIFEST, 'utf8')); } catch (e) { log(`[manifest] the current ${relative(ROOT, MANIFEST)} is unreadable (${e.message}): replaced`); }
+  }
+  const guard = shrinkGuard(current, manifest, opts);
+  if (guard.write) await writeJsonAtomic(MANIFEST, manifest);
 
   // Orphans: files on disk that the manifest does not reference (e.g. after a mapping change). public/assets/local/**
   // belongs to tools/local-extract (data/local-assets.json) and is never an orphan: --prune used to delete all of it.
@@ -280,6 +309,8 @@ async function main() {
     fontErrors,
     orphans: opts.prune ? [] : orphans,
     pruned: opts.prune ? orphans : [],
+    manifestWritten: guard.write,
+    droppedEntries: guard.dropped, // entries of the previous data/assets.json the rebuilt one lacks
     notes: plan.notes,
   };
   await writeJsonAtomic(REPORT, report, 1);
@@ -304,15 +335,26 @@ async function main() {
   if (spine.problems.length) { log(`spine notes (${spine.problems.length}):`); for (const p of spine.problems.slice(0, 20)) log(`  ${p}`); }
   for (const e of fontErrors) log(`font error: ${e}`);
   if (orphans.length) log(opts.prune ? `pruned ${orphans.length} unreferenced files` : `${orphans.length} unreferenced files on disk (run with --prune to delete)`);
-  log(`manifest: ${MANIFEST} · report: ${REPORT} · ${((Date.now() - t0) / 1000).toFixed(0)} s`);
+  if (guard.dropped.length) {
+    log(guard.write
+      ? `data/assets.json lost ${guard.dropped.length} entries (${opts.prune ? '--prune' : '--allow-shrink'}):`
+      : `ERROR: data/assets.json NOT written — it would lose ${guard.dropped.length} entries the current one has (their files are missing here):`);
+    for (const k of guard.dropped) log(`  ${k}`);
+    if (!guard.write) log('  re-run to retry the downloads (--refresh-index for the audio/model indexes), or pass --allow-shrink (or --prune) to write the smaller manifest');
+  }
+  log(`manifest: ${MANIFEST}${guard.write ? '' : ' (kept)'} · report: ${REPORT} · ${((Date.now() - t0) / 1000).toFixed(0)} s`);
   if (required.length) {
     log(`ERROR: ${required.length} required assets missing: ${required.slice(0, 10).join(', ')}`);
     return 1;
   }
-  return 0;
+  return guard.write ? 0 : 1;
 }
 
-main().then((code) => { process.exitCode = code; }, (e) => {
-  console.error(`[assets] FAILED: ${process.env.DEBUG ? e?.stack || e : e?.message || e}`);
-  process.exitCode = 1;
-});
+// run only as a script (tests import parseArgs / shrinkGuard)
+const invoked = (() => { try { return pathToFileURL(realpathSync(process.argv[1] || '')).href; } catch { return null; } })();
+if (invoked === import.meta.url) {
+  main().then((code) => { process.exitCode = code; }, (e) => {
+    console.error(`[assets] FAILED: ${process.env.DEBUG ? e?.stack || e : e?.message || e}`);
+    process.exitCode = 1;
+  });
+}