Repository navigation
aggregator(snapshots): append-usage-snapshots --by with no value silently discards every derived record — exit 0, 'no records derived' #3295
Description
aggregator(snapshots): append-usage-snapshots --by with no value silently discards every derived record — exit 0, "no records derived"
Evidence (verified this run, main fdfb377c)
pm-stack/aggregator/scripts/append-usage-snapshots.ts:54-62 reads the observer name with the bare index-of pattern, in both modes:
const byArg = process.argv.indexOf("--by");
const observedBy = byArg >= 0 ? process.argv[byArg + 1] : "check-usage"; // --from-provider-usage mode (:57)
const observedBy = byArg >= 0 ? process.argv[byArg + 1] : "stewardship-cycle"; // default mode (:61)When --by is the last argument, process.argv[byArg + 1] is undefined; with --by "" it is the empty string. There is no validation of the value. Every derived record then carries that observedBy, and the script's own filter drops all of them: isSnapshotRecord requires a non-empty string observer (pm-stack/aggregator/src/snapshots.ts:310, !isNonEmptyString(r.observedBy) → false). The script then reaches the empty-result branch (append-usage-snapshots.ts:71-80) and prints the misleading no records derived; outbox untouched — and exits 0.
Probe (laptop, real script run from the laptop clone; script sha256 e2612f0f… and src/snapshots.ts c6685035… identical on both clones; fixture provider-usage.json = {"generated_at":"2026-10-31T12:00:00.000Z","sessions":[{},{}]}, which derives exactly one record; probe dir ~/git/fleet/probe-2026-10-31-laneA-appendby/):
| run | result |
|---|---|
control: --from-provider-usage --by tester |
appended 1 record(s); outbox holds the sessions-listed record with "observedBy":"tester"; exit 0 |
--from-provider-usage --by (trailing, no value) |
no records derived; outbox untouched; no outbox file created; exit 0 |
--from-provider-usage --by "" |
same: no records derived; outbox untouched; no outbox file; exit 0 |
Why it matters
This script is a snapshot writer (hub design §4): stewardship / check-usage runs append the dated quota observations that become the fleet's measured usage history. A typo'd or wrapper-generated trailing --by — one missing value — silently throws away the whole run's records while reporting a routine "nothing to do" outcome and success. Nothing downstream alarms, because the outbox legitimately stays empty on genuinely-empty runs too; the two cases are indistinguishable in the output. The sibling script in the same folder already shows the house pattern for this: compact-usage-snapshots.ts:28-31 validates its --months value and exits 2 naming the bad value.
Acceptance criteria
append-usage-snapshots.ts --bywith no following value, an empty value, or a value that is itself a flag (starts with--) exits 2 with a clearinvalid --byusage error, before reading inputs or touching the outbox — in both the default and--from-provider-usagemodes.--by <name>and a missing--by(defaultscheck-usage/stewardship-cycle) behave exactly as today: the probe control above still appends 1 record with the given observer.- The "no records derived" message is only reachable when records were actually derived-and-filtered for content reasons; a test (or script-level check) pins the trailing-
--byrefusal so the silent-discard path cannot return. - Existing aggregator snapshot tests still pass.
Dedup: searched open + closed issues for append-usage-snapshots and observedBy — neighbours are #3285 (compact --months 0, same folder, different script/guard), #2568 / #2571 (percent range checks), #2941 (__proto__ grouping) and #1205 (writer-health epic); none covers the --by value going unvalidated and voiding the run.
Activity
dezren39 commentedon Nov 1, 2026
🤖 Agent note · filed
--byby bare index-of with no value validation (scripts/append-usage-snapshots.ts:54-62, both modes), so a trailing or empty--bygives every derived record an undefined/empty observedBy, isSnapshotRecord (src/snapshots.ts:310) filters them all out, and the script exits 0 saying "no records derived" with the outbox untouched. Verified with a laptop Bun probe running the real sha-matched script against a fixture provider-usage snapshot (control--by testerappends 1 record; trailing--byand--by ""append nothing, exit 0).--bycase against main, then the fix validates the--byvalue (missing/empty/flag-like → exit 2, mirroring compact-usage-snapshots'--monthsguard) and pins it with a test.--monthsguard pattern), Usage snapshots: move ingestion out of the aggregator read path and report writer health #1205 (writer-health epic)dezren39 commentedon Nov 1, 2026
Adopted as Phase 2 of design #3352 (corollary 3: present-but-valueless is not absent — an empty observer value is a usage error, not a silently discarded run). 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).
6 remaining items
dezren39 commentedon Nov 1, 2026
🤖 Agent note · done (triaged)
origin/main33aefa7.--byis read without validation in both modes; emptyobservedByis rejected, producing a successful no-op. No matching open PR found. Earlier real-script probe is cited on this issue; this pass did not rerun it.repo-health:v1:fix:developing-today/code:issue:3295; labels now reflect queued follow-up.dezren39 commentedon Nov 1, 2026
🤖 Agent note · picked up
--bysilent-discard defect inappend-usage-snapshots— validate missing/empty/flag-like--byvalues before any snapshot write, in both modes.origin/main33aefa7.Merge pull request #3920 from developing-today/queue/5873-3295-by-val…
dezren39 commentedon Nov 1, 2026
🤖 Agent note · done
fixed_mergedappend-usage-snapshotsnow validates--bybefore any file read/write in both modes: missing, empty/whitespace, or flag-like values exit 2 with a clear usage error and leave the outbox untouched; valid names and the absent---bydefaults are unchanged.test/append-usage-snapshots-by-3295.test.ts; on the laptopbun testover the new test + snapshot-writers + snapshot-retention — 30 pass, 0 fail. Codex review clean at head b57f6c2 (no findings).