Repository navigation
aggregator(backfill): trailing --dir crashes with a raw TypeError, and a flag-like value (--dir --print) silently writes the rebuilt history into a directory named '--print' #3330
Description
Bug
backfill-usage-snapshots (the one-shot usage-snapshot backfill, pm-stack/aggregator/scripts/backfill-usage-snapshots.ts) reads its only flag with a bare index lookup and no validation:
// backfill-usage-snapshots.ts:32-33
const dirArg = process.argv.indexOf("--dir");
const storeDir = dirArg >= 0 ? process.argv[dirArg + 1] : (process.env.PM_AGG_SNAPSHOTS_DIR ?? join(PKG, "snapshots"));Two failure modes, both verified this run:
- Trailing
--dircrashes with a rawTypeError, not a usage error.process.argv[dirArg + 1]isundefined, sostoreDirisundefined, and the run dies insideSnapshotStore.append→withSnapshotStoreLockatmkdir(dir)(src/snapshots.ts:347):TypeError: path must be a string or TypedArraywith a Bun stack trace, exit 1. Nothing names--diror its missing value. - A flag-like next token is silently taken as the directory.
--dir --printappends the derived records into a newly created directory literally named--printin the current working directory and exits 0 reportingappended to --print— the real snapshot store is untouched while the report claims success. Any mistyped following token does the same.
Evidence (verified this run)
Probe on the laptop (~/git/fleet/probe-2026-10-31-laneA-backfilldir/): script run verbatim from main 6e7f8fcf (backfill-usage-snapshots.ts sha256 f55386364648275ac53f35752dee2302006f23545c68a5c409ca6d47c22d8da7 on both laptop and sandbox clones), Bun 1.4.2, fixture QUOTA_VIEW_PATH carrying one DeepSeek ¥44.63 / 2026-10-25 line (exactly 1 derived record; the 31 unmatched-pattern warnings are the script's normal fail-soft behaviour):
=== CONTROL: --dir <valid temp dir> ===
backfill: 1 record(s) appended to .../store-control (0 already present, 31 skipped) exit=0
store-control/usage-2026-10.jsonl written
=== BUG 1: --dir (trailing, no value) ===
TypeError: path must be a string or TypedArray
at mkdir (node:fs/promises)
at withSnapshotStoreLock (src/snapshots.ts:347)
at append (src/snapshots.ts:606)
at main (scripts/backfill-usage-snapshots.ts:431) exit=1
=== BUG 2: --dir --print (flag-like value) ===
backfill: 1 record(s) appended to --print (0 already present, 31 skipped) exit=0
# a directory named `--print` was created in the CWD (the aggregator package dir)
# containing usage-2026-10.jsonl; the real store never saw the record.
# (Probe cleanup: the stray `--print` directory was removed from the laptop clone.)
Contrast with the sibling scripts in the same package: compact-usage-snapshots.ts:28-31 validates its --months value and exits 2 with invalid --months '<value>', and append-usage-snapshots.ts was already filed for its --by variant of this shape (#3295). This script has no guard at all.
Why it matters
The backfill is the recovery/seed path for the usage snapshot store — it is run by hand, rarely, exactly when history needs rebuilding. A forgotten --dir value currently either crashes with a stack trace that points into the store lock code (suggesting store corruption, not a typo) or, worse, silently writes the rebuilt history into a stray flag-named directory in the working tree and reports success, so the operator believes the store was seeded when nothing was. Because re-running is documented as safe (dedupe), the silent case can go unnoticed until the missing history is queried.
Acceptance criteria
backfill-usage-snapshots.ts --dirwith no following value, or with a following token that starts with--, prints a usage/error message naming--dirand its missing value to stderr and exits 2 — no stack trace, no directory created, no records written.--dir <valid path>behaves exactly as today (records appended to that store, exit 0, same summary line).- The no-flag default (env
PM_AGG_SNAPSHOTS_DIR, else the packagesnapshots/directory) is unchanged. - A regression test (or script-level test harness case) covers the trailing-
--dirand flag-like-value cases.
Distinct from: #3295 (append-usage-snapshots --by voiding observedBy), #3285 (compact --months 0 guard gap — compact does validate non-numeric values), #3312 (docs-history trailing flag panic, Go tool). Same family, different script and different failure: this one both crashes and silently misdirects the write.
Activity
dezren39 commentedon Nov 1, 2026
🤖 Agent note · filed
dezren39 commentedon Nov 1, 2026
Adopted as Phase 2 of design #3352 (corollary 3: present-but-valueless is not absent — neither default nor flag-as-value nor downstream crash). The design records the contract this issue's fix must follow: every argument is accounted for before any side effect; unknown/typo'd arguments are a usage error naming the argument (exit non-zero, 2 where no usage code is established) before any subprocess/network/file/lock/deploy step. Triage: do NOT re-file, re-scope or split this issue — record the phase structure and verdict, and keep this issue's own acceptance criteria as the phase's acceptance. Design doc: files/designs/code-operator-argument-contract.md (keep-the-four-repos-healthy goal workspace).
22 remaining items
dezren39 commentedon Nov 2, 2026
🤖 Agent note · state: parked
0accb1a860c993948c16f1062c537995a5d0b2a4; latest fresh checksaggregator + projectorandtestare both QUEUED. Targeted Bun regression tests were recorded passing 4/4.Merge pull request #4393 from developing-today/queue/5929-3330-backfi…
dezren39 commentedon Nov 2, 2026
agent-note:v1 — done (verified on main, closing)
git merge-base --is-ancestorconfirms the PR head is in origin/main; the --dir guard (--dir requires a directory value) is present at origin/main:pm-stack/aggregator/scripts/backfill-usage-snapshots.ts:32-36, and the regression test test/backfill-usage-snapshots-dir-3330.test.ts is on main.web-pushpackage in node_modules (declared dep, not installed in the worktree) via src/push.ts imported by test/alerts.test.ts — pre-existing environment gap, unrelated to this change (diff touches only the backfill script + the new test).