Repository navigation
aggregator(comments+actions): a store record that passes the load filter but lacks a string createdAt crashes every comments/actions read — and persist() re-writes it forever #2347
Description
aggregator(comments+actions): one store record that passes the load filter but lacks a string createdAt crashes every comments/actions read — and persist() re-writes the poisoned record forever
Scope note: this covers BOTH dashboard JSON stores, which share the identical load-filter/sort code shape — CommentsStore (comments.ts) and ActionsStore (actions.ts:434-463, same id-only filter at :438-440, same createdAt.localeCompare sort at :463, same persist re-write). Both probes below were run against both stores with identical results. One fix, one issue.
Evidence (verified this run, main 5a34fa7)
pm-stack/aggregator/src/comments.ts:
CommentsStore.doLoad()(comments.ts:114-124) filters the on-disk records with a single check —typeof c.id === "string"(comments.ts:118-120). Every other field a record needs is accepted unchecked.all()(comments.ts:149) then unconditionally callsa.createdAt.localeCompare(b.createdAt)on every loaded record. A record withcreatedAtmissing or non-string throwsTypeErroron the first comparison.- Every read path goes through
all():forObject,byObject,counts,pendingCounts(comments.ts:153-196), and the server's/api/comments.jsonhandler (server.ts:436/440).counts()/byObject()are also awaited while rendering the main views (server.ts:270, 288-289), so the throw is not confined to the comments endpoints. - The poisoned record is never quarantined:
add()loads, pushes, andpersist()re-serializes the whole in-memory array including the bad record (comments.ts:131-145, 170-186), so the breakage survives restarts and new writes until someone hand-edits the JSON file.
Probe (sandbox Bun, real CommentsStore imported from main, temp files):
- control (one well-formed record):
all()OK, counts correct. - one well-formed record + one record identical except
createdAtdeleted:all()THROWSTypeError: undefined is not an object (evaluating 'a.createdAt.localeCompare'). createdAt: 123(number):all()THROWSTypeError: a.createdAt.localeCompare is not a function.- a record with only
{id, createdAt}also passes the filter and produces acounts()entry keyednull(missingobjectRef), andpendingCommentCountssilently treats its missingkindas neither pending class.
Why it matters
The comments file is a hand-inspectable JSON store in the aggregator's data directory, and the load filter exists precisely to tolerate records the current build didn't write (older/newer writers, partial hand edits, a restore from backup — cf. the #1509 load-race history in the same file). Today that tolerance is an illusion: the one field the filter doesn't check is the one field every read dereferences, so a single degraded record takes down the whole comments surface (queue input for the orchestrator, per-card comment boxes, 💬 badges, health pending counts) instead of being dropped or reported like the id-less records the filter already discards. Sibling stores in this codebase validate loaded records field-by-field; this one validates the one field reads don't sort by.
Acceptance criteria
- Load-time validation checks the fields reads actually use (at minimum
id,objectRef,kind,createdAtas an ISO string; author/body per the DashComment shape), and records failing it are excluded from the in-memory set (dropped or quarantined with a logged count — triage pins which), matching the existing id-filter precedent. -
all()/counts()/byObject()/pendingCounts()never throw on a store file containing such a record; a regression test loads a file mixing one valid record with records missingcreatedAt, with numericcreatedAt, and missingobjectRef/kind, and asserts the valid record is served and counted under its real ref. - After any subsequent
add()/persist(), the poisoned record does not silently return to disk in a form that re-breaks the next process start (or the issue documents why it is retained harmlessly). - Well-formed stores are byte-for-byte unaffected: existing comments tests (api-health-comments, store-concurrency) pass unchanged.
Dedup: #950 (githubUrl href validation), #1686 (empty github-result marks failed), #1509 (closed; temp-path/load race), #1210 (triage queue view) are adjacent comments-store issues; none covers load-filter field validation or the createdAt sort crash.
Activity
developing-today commentedon Oct 29, 2026
CODE-1192
dezren39 commentedon Oct 29, 2026
🤖 Agent note · filed
id, but all() (comments.ts:149) calls createdAt.localeCompare on every record: one record missing/with a non-string createdAt crashes all()/forObject/byObject/counts/pendingCounts and /api/comments.json, and persist() re-writes the poisoned record forever. Verified with a sandbox Bun probe of the real CommentsStore (main 5a34fa7): missing createdAt -> TypeError, numeric createdAt -> TypeError, control OK.[-]aggregator(comments): a store record that passes the load filter but lacks a string createdAt crashes every comments read — and persist() re-writes it forever[/-][+]aggregator(comments+actions): a store record that passes the load filter but lacks a string createdAt crashes every comments/actions read — and persist() re-writes it forever[/+]dezren39 commentedon Oct 29, 2026
🤖 Agent note · scope extension (same run)
createdAt.localeComparesort, same persist re-write): the identical probe against the real ActionsStore throws the identical TypeError on a record missingcreatedAt, control OK. Same fix shape, so it is folded into this one issue rather than filed separately. Triage packet packet-2347-triage.json names comments; treat actions.ts as in-scope for the same small_fix.51 remaining items
dezren39 commentedon Nov 2, 2026
🤖 Agent note · parked
queue/5246-2347-quarantine-malformed-records; metadata-only, nothing inspected or modified beyond labels/notes.dezren39 commentedon Nov 2, 2026
🤖 Agent note · correction to the park above
dezren39alone does not establish Drew-ownership — every fleet PR carries that author. Positive fleet-ownership evidence here: PR fix(aggregator): quarantine malformed comments and actions records #4151 is on this task's own branchqueue/5246-2347-quarantine-malformed-records, and the fleet worktree~/git/fleet/5246exists on the laptop at the PR head 46c5e6d; the PR body is a fleet-style summary ("Closes aggregator(comments+actions): a store record that passes the load filter but lacks a string createdAt crashes every comments/actions read — and persist() re-writes it forever #2347") with evidence and test sections.held/user/owned/do-not-merge/do-not-auto-editlabels or explicit Drew direction). If fleet-owned: verify CI/Codex on PR fix(aggregator): quarantine malformed comments and actions records #4151, disposition findings, and merge when verified, closing aggregator(comments+actions): a store record that passes the load filter but lacks a string createdAt crashes every comments/actions read — and persist() re-writes it forever #2347.dezren39 commentedon Nov 2, 2026
🤖 Agent note · picked up
queue/5246-2347-quarantine-malformed-records, head 46c5e6d) is this task's own canonical PR — the earlier Drew-owned park was the shared-account false positive (dezren39 is the fleet account; judged by queue/ branch + PR body task refs). Revalidating fresh: Codex review reports no findings; CI failures (slice-3 routes, hub renders) reproduce identically on clean main — pre-existing, unrelated to the aggregator store diff.dezren39 commentedon Nov 2, 2026
🤖 Agent note · done