FsFolderTabularStorage.putBulk races on duplicate primary keys, so last-write-wins is not honored

[sroussey]

Summary

FsFolderTabularStorage.putBulk fans its rows out with Promise.all, so two entries sharing a primary key race on the same file and the winner is whichever write settles last — not the last one in the array. The generic tabular contract asserts last-write-wins for that case, so the suite fails intermittently.

// packages/storage/src/tabular/FsFolderTabularStorage.ts:200
async putBulk(entities: InsertType[]): Promise<Entity[]> {
  await this.setupDirectory();
  return await Promise.all(entities.map(async (entity) => this.put(entity)));
}

Both put calls resolve the same path via getFilePath(key) and both writeFile it. Nothing orders them.

The contract it breaks

genericTabularStorageTests.ts (~line 286), run against every tabular backend:

it("should apply last-write-wins for duplicate primary keys within one putBulk batch", async () => {
  const entities = [
    { name: "dup",   type: "k", option: "first", success: true },
    { name: "other", type: "k", option: "kept",  success: true },
    { name: "dup",   type: "k", option: "last",  success: false },
  ];
  await repository.putBulk(entities);
  const stored = await repository.get({ name: "dup", type: "k" });
  expect(stored?.option).toEqual("last");

Evidence that it is a race, not a deterministic break

Observed on the CI run for #808, in test-vitest-integration:

FAIL src/test/storage-tabular/FsFolderTabularStorage.integration.test.ts
  > FsFolderTabularStorage > basic functionality > with compound primary keys
  > should apply last-write-wins for duplicate primary keys within one putBulk batch
AssertionError: expected 'first' to deeply equal 'last'

Re-running the same commit with no code change made it pass. It had also passed on the immediately preceding commit of the same branch. The branch that surfaced it touches no storage code at all — it is a @workglow/task-graph change.

Only FsFolderTabularStorage fails. Vitest prints the single failure twice (the summary reads Failed Tests 1), which is easy to misread as two backends failing.

Why the other backends are unaffected

• InMemoryTabularStorage.putBulk → atomicPutBulk, which writes serially over a snapshot and rolls back on a mid-batch throw.
• PostgresTabularStorage stores the batch inside a single BEGIN/COMMIT on one connection; its own comment names reduced pool churn "vs. a Promise.all fan-out" as a reason for that design.

So FsFolder is the outlier, and the contract test is right — the implementation is what disagrees.

Suggested fix

Either is sufficient for the failing assertion:

1. De-duplicate by primary key before writing, keeping the last occurrence. Cheapest, preserves the parallel write for the common all-distinct case, and makes the ordering explicit rather than emergent.
2. Write serially, matching InMemoryTabularStorage.

Option 1 is probably preferable — a bulk write of thousands of distinct rows should not lose its concurrency to fix a duplicate-key edge case.

Worth deciding alongside

putBulk's atomicity is not uniform across backends. InMemoryTabularStorage documents all-or-nothing semantics and emits a rollback event carrying the PKs that committed before a mid-batch failure; FsFolderTabularStorage.putBulk has no equivalent, so a mid-batch failure leaves a partially written directory. That is a separate question from this bug and may be intended, but if ITabularStorage.putBulk is meant to promise atomicity the way it promises last-write-wins, FsFolder does not currently provide it.

Impact

Low severity in production (duplicate PKs within one batch are unusual), but it makes test-vitest-integration flaky, which is corrosive: a red check that clears on re-run trains people to re-run rather than read.