Repository navigation
aggregator(confirm-charges-surfaced): chargesAttention elements are never validated — one malformed element crashes confirmation after delivery, so delivered charges re-alert forever #3681
Description
Problem
pm-stack/aggregator/scripts/confirm-charges-surfaced.ts — the post-delivery confirmation step of the fleet-feed push (its whole job, per #3122, is to mark the charges in the successfully delivered feed as surfaced, exactly once) — validates only that chargesAttention is an array (line 13). It never validates the array's elements, and it never validates the feed's top-level shape beyond property access. It then calls unmatchedKey(charge) (src/charges-collect.ts:119-121) on each raw element:
return `${u.normalized}|${u.amountUsd.toFixed(2)}|${u.at}`;That dereferences normalized, calls .toFixed(2) on amountUsd, and interpolates at, with no check that any of them exist or have the right type. The render path got this exact protection in #2439 (closed): loadFeed now sanitizes chargesAttention element-by-element with isUnmatchedCharge (src/feed.ts:502-506). The confirm script bypasses loadFeed/isFleetFeed entirely — it does its own JSON.parse of the delivered file — so the fix never covered it.
Evidence (verified this run)
Probe on the laptop with the verbatim script and its real import chain at main c6bf9337 (sha256-matched: confirm script 49675ccf…, charges-collect.ts 20e3c5b4…; probe dir ~/git/fleet/probe-2026-11-01-laneB-confirmcharges/, log probe.log):
- Control — one well-formed element (
{merchant, normalized:"acmeai", amountUsd:12.5, at:"2026-10-30", source}): exit 0, state file gains keyacmeai|12.50|2026-10-30. ✔ chargesAttention: [null]: exit 1 —TypeError: null is not an object (evaluating 'u.normalized'); no state file written.- String amount (
amountUsd: "12.50"): exit 1 —TypeError: u.amountUsd.toFixed is not a function; no state written. - Mixed
[valid, null]: exit 1 and the valid charge is not confirmed either — the key loop (line 24) runs before the atomic write (lines 25-30), so one bad element discards the whole feed's confirmations. - Top-level feed
null: exit 1 —TypeError: null is not an object (evaluating 'feed.chargesAttention')at line 13/14. - Top-level feed
[]: exit 0, silently —chargesAttentionisundefined, attention is empty, nothing is confirmed and nothing says so. - Element missing
normalized({merchant, amountUsd:12.5, at, source}— a shape the house gateisUnmatchedChargeinsrc/feed.ts:350-353explicitly accepts, since it does not requirenormalized): exit 0, but the persisted surfaced key is the bogusundefined|12.50|2026-10-30, which can never match the key the collector computes for the real charge (filterNewUnmatched, charges-collect.ts:130-144) — the real charge is never marked surfaced, and the bogus key stays in the state file forever.
Why it matters
scripts/push-fleet-feed.sh runs under set -euo pipefail on a ~5-minute systemd cadence, and the confirm script is its last line (line 209), run after the feed has already been scp'd and mv'd into place on the laptop. So one malformed element (a hand-edited feed, an older/newer producer shape, a partially written file that still parses):
- makes the whole push unit exit non-zero even though delivery succeeded — a phantom push failure every 5 minutes, indistinguishable from a real transport failure; and
- persists none of the confirmations, so every charge in that feed re-surfaces as a fresh attention flag on the next cycle, forever, until the element is removed — the exact duplicate-alert failure the surfaced-once state exists to prevent (fix(aggregator): charges surfaced keys are persisted before feed delivery, so a failed transfer permanently suppresses the flags (PR #2149) #3122).
The silent variants are worse in the other direction: a top-level array feed, or any element missing normalized, exits 0 while confirming nothing (or confirming a bogus undefined|… key), so the push looks healthy while charges re-alert indefinitely.
Producers today emit well-formed UnmatchedCharges (collect-fleet-feed.ts:520-539,766), so this is a trust-boundary gap, not a live producer bug — the same class as #2439/#2445/#2446 for the render path. The confirm script is the one consumer of the feed file that still trusts it completely.
Acceptance criteria
- The confirm script validates each
chargesAttentionelement before keying it (reuseisUnmatchedChargefromsrc/feed.ts, extended if needed so a keyable element must also carry a stringnormalized— or validate equivalently inline): malformed elements are dropped with a loud stderr note naming the dropped count, never keyed, and never abort the confirmation of the well-formed elements. - A non-object top-level feed (
null, array, string, number) fails with a clearinvalid feederror naming the shape problem — never a rawTypeError, and never a silent exit 0. - A persisted surfaced key is never built from missing/coerced fields: no
undefined|…keys can enter the state file. - Regression tests (or a scripted probe in the fix PR) cover: control, null element, string amount, mixed valid+invalid (valid still confirmed), top-level null, top-level array, missing-
normalizedelement. -
push-fleet-feed.shbehaviour is unchanged for well-formed feeds: exit 0 and the same keys persisted as today.
Distinct from: #2439 (closed — the render path gate in feed.ts; this is the push/confirm path, which does not use that gate), #3122 (closed — moved confirmation after delivery; this script is that fix's implementation), #3458 (open — isChargeRow/isUnmatchedCharge accept negative amounts and non-date at on the render path; if that lands first, this fix should reuse the tightened gate).
Activity
developing-today commentedon Nov 1, 2026
CODE-1500
dezren39 commentedon Nov 1, 2026
🤖 Agent note · filed
4e4c9f7bbefore filing — confirm script lines 13/24,unmatchedKey(charges-collect.ts:119-121), and theisUnmatchedChargegate (nonormalizedrequirement) are unchanged from the probe basec6bf9337. Enqueued arepo-task/v1triage packet torepo-health-code-pilot.undefined|…keys).packets/packet-3681-triage.json3 remaining items
dezren39 commentedon Nov 2, 2026
Triage verification (task 5573)
Revalidated against clean laptop main
cc819feb86679e6e429645bb9781c6e7b40b572f; the issue is still open. No open PR matchedconfirm-charges-surfaced, and no open PR matched #3458.Ran the real
pm-stack/aggregator/scripts/confirm-charges-surfaced.tsfrom a dedicated worktree with an isolatedCHARGES_SURFACED_STATE_PATH(probe artifacts:/tmp/opencode/repo-5573.XykvbQ/). Current-main results: control succeeds and persistsacmeai|12.50|2026-10-30;[null], string amount, and mixed[valid, null]exit 1 with raw TypeErrors and persist nothing (the valid sibling is lost); top-levelnullexits 1 with a raw TypeError; top-level[]exits 0 silently; missingnormalizedexits 0 and persistsundefined|12.50|2026-10-30.This confirms the trust-boundary gap. The fix should validate the object feed shape, drop/count malformed elements while processing valid siblings, and require a string
normalizedbefore key creation. Reuse/tightenisUnmatchedChargeas appropriate; keep #3458’s render-path scope separate.dezren39 commentedon Nov 2, 2026
🤖 Agent note · picked up
dezren39 commentedon Nov 2, 2026
🤖 Agent note · handed off
dezren39 commentedon Nov 2, 2026
🤖 Agent note · parked
Merge pull request #4299 from developing-today/queue/6168-3681-confir…
dezren39 commentedon Nov 2, 2026
🤖 Agent note · done
679536250e0cea139e84ed0384c9bb59f769057e, verified as origin/main head with the validation present in the merged script. Issue aggregator(confirm-charges-surfaced): chargesAttention elements are never validated — one malformed element crashes confirmation after delivery, so delivered charges re-alert forever #3681 auto-closed by the merge.a769f672c(no findings); CodeRabbit rate-limited (no review produced); CI checks were queued-only at merge (standing runner-backlog pattern, merge per standing authority). Local verification on the laptop: new regression test 7/7 pass, adjacent charges/feed suites 63/63 pass, no tsc errors in touched files.