Skip to content

aggregator: concurrent writes to the JSON stores share one temp path — most fail with ENOENT (returned as '400 invalid'), and persisted records don't match the calls that succeeded #1509

Closed

Description

@dezren39

Summary

The aggregator's JSON stores persist through a temp file named only by process id — `${file}.tmp-${process.pid}` — and none of them serializes writes. Two writes that overlap in the same process share the one temp path: the first rename wins, the rest fail with ENOENT, and which records actually reach disk stops matching which callers were told succeeded.

Three sites share the pattern:

  • pm-stack/aggregator/src/comments.ts:121 — CommentsStore.persist() (used by add, markTriaged, markGithubResult)
  • pm-stack/aggregator/src/actions.ts:441 — ActionsStore.persist() (used by add, markResult)
  • pm-stack/aggregator/src/sources/live.ts:594 — the session-registry merge write (same shared temp name; its try/catch swallows the failure as "best-effort", so registry updates are silently lost instead)

Verified evidence (this run, main 931f177)

Probe: the verbatim pm-stack/aggregator/src tree (comments.ts sha256 5120ae7d…, actions.ts ca10a65f…, model.ts f801cde7…, identical on both machines) under Bun 1.4.2 on the laptop (~/git/fleet/probe-2026-10-27-laneA-persist there; probe + log in the goal's hidden_files/probe-2026-10-27-laneA-1435/), firing concurrent add() calls at one store, then reloading through a fresh store instance (what survives a restart):

COMMENTS n=2  fulfilled=1  rejected=1   firstErr=ENOENT rename '…comments-2.json.tmp-2579588'  fileCount=1  reloaded=1
COMMENTS n=5  fulfilled=1  rejected=4   firstErr=ENOENT rename …                              fileCount=1  reloaded=1
COMMENTS n=20 fulfilled=1  rejected=19  firstErr=ENOENT rename …                              fileCount=1  reloaded=1
COMMENTS n=20 fulfilled=2  rejected=18  firstErr=ENOENT rename …                              fileCount=19 reloaded=19
ACTIONS  n=20 fulfilled=2  rejected=18  firstErr=ENOENT rename …                              fileCount=1  reloaded=1
ACTIONS  n=20 fulfilled=3  rejected=17  firstErr=ENOENT rename …                              fileCount=19 reloaded=19

Two distinct failures, both from the shared temp path + unserialized persists:

  1. Most concurrent writes error out. The HTTP layer maps the failure to a misleading client error — POST /api/comments returns 400 {"error":"invalid comment: ENOENT: no such file or directory, rename …"} (server.ts:324-332) and POST /api/actions likewise 400 "invalid action: ENOENT…" — for a comment/action that was perfectly valid.
  2. Success and durability disagree in both directions. Because each persist() stringifies the live in-memory array whenever its writes happen to land, a run where only 2 of 20 adds fulfilled still left 19 records on disk: callers told "400, not saved" had their comment/action silently persisted, so a client that retries creates a duplicate (comments have no dedupe token at all; actions are protected only when the client reuses its clientToken). The mirror image also occurs — in-memory state and the file diverge until some later, unrelated write happens to persist the whole array again.

Why it matters

These stores are the dashboard's write path, and concurrency is the normal case, not an edge: a human clicking card actions in two tabs, the orchestrator posting /api/comments/triage and /api/actions/result while Drew posts a comment, and the live source's registry merges on its own schedule all hit the same processes. Today that yields spurious 400s on valid input, duplicate comments on retry, and silently dropped registry/result transitions — in the system whose whole job is to be the fleet's reliable triage surface.

Acceptance criteria

  • Concurrent add / markTriaged / markGithubResult / markResult calls on one store all fulfill, and a fresh reload returns exactly the records the fulfilled calls wrote — no ENOENT, no extras, no losses.
  • Temp files are unique per write (or persists are serialized per store through a write queue/mutex) at all three sites: comments.ts, actions.ts, sources/live.ts.
  • A persistence I/O failure is no longer reported to HTTP clients as 400 invalid comment/action (a 5xx, or an explicit persistence-error body, is fine).
  • A regression test fires ≥10 concurrent adds at a store and asserts fulfilled count == reloaded count (the probe above is a ready-made shape).
  • Sequential behaviour is unchanged: same file format, same atomic rename publish, same dedupe-by-clientToken semantics for actions.

Activity

  1. developing-today commented on Oct 27, 2026

    @developing-today
    Contributor
  2. added
    kind/bugSomething isn't working (prefixed form of the classic bug label)
    queue/queuedHas a queued task in the Supabase agent queue
    source/creatorFiled by the new-issue-creator job
    task/triageQueue packet kind: verify, reproduce, and route this item
    priority/normalOrdinary priority (the default)
    app/aggregatorPackage/app: aggregator (pm-stack in this repo)
    effort/sSmall: expected under 2 hours of work
    on Oct 27, 2026
  3. dezren39 commented on Oct 27, 2026

    @dezren39
    MemberAuthor

    🤖 Agent note · filed

    • Agent: new-issue-creator-code-a (scheduled creator lane A)
    • Session / ID: cron job new-issue-creator-code-a, run 2026-10-27 14:35 CDT
    • Found via: filed this run
    • Task: Triage the shared-temp-path concurrency defect in the aggregator JSON stores (comments / actions / live registry): concurrent writes fail with ENOENT (surfaced as 400 "invalid") and persisted records diverge from fulfilled calls.
    • Expectation: effort/s — unique temp name per write and/or serialized persists at all three sites, honest 5xx on persistence failure, plus a concurrent-add regression test.
    • Links: verified with the verbatim aggregator tree under Bun on the laptop (probe log in the goal backlog records); queue packet packet-1509.json (repo-health-code-pilot)
  4. added
    work/assignedQueue worker lifecycle state
    queue/claimedQueue worker lifecycle state
    and removed
    queue/queuedHas a queued task in the Supabase agent queue
    on Oct 28, 2026
  5. 26 remaining items

  6. added
    queue/claimedQueue worker lifecycle state
    and removed
    queue/claimedQueue worker lifecycle state
    status/parkedDeliberately paused with a recorded resume condition
    queue/parkedQueue task is parked (waiting on CI, time, or an event)
    on Oct 28, 2026
  7. dezren39 commented on Oct 28, 2026

    @dezren39
    MemberAuthor

    🤖 Agent note · picked up

  8. dezren39 commented on Oct 28, 2026

    @dezren39
    MemberAuthor

    🤖 Agent note · done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

app/aggregatorPackage/app: aggregator (pm-stack in this repo)effort/sSmall: expected under 2 hours of workkind/bugSomething isn't working (prefixed form of the classic bug label)priority/normalOrdinary priority (the default)source/creatorFiled by the new-issue-creator jobstatus:donetask/small-fixQueue packet kind: bounded fix with reproduction and targeted teststask/triageQueue packet kind: verify, reproduce, and route this item

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions