Reconcile the store with the cards a batch mints

[habdelra]

Creating a card the browser is already holding

A card the user has made but not saved exists only as an object in the tab, known by a local id. Saving it through the store threads that local id end to end: it travels as the JSON:API lid, the realm names the new file after it, and the store pairs the object with the URL that comes back. A batch is a second way for such a card to come into existence, and this makes that way end in the same place.

let activity = await store.add(new Activity({ headline: 'Lab safety' }), {
  doNotPersist: true,
});

await operations(classroom).atomic((b) => {
  let created = b.create(activity);      // the card the tab is holding
  b.addActivity({ activity: created });
});

create takes either the class of a card to mint from data, or an unsaved instance to mint that card. The instance form is the whole of what the entry says — its field values are the card's and its local id is the card's name — so passing attributes beside it is refused rather than resolved in one of the two directions silently. When the answer comes back, the instance takes its id, joins the identity map under both ids, subscribes to its realm, starts autosaving, and has consumers in other realms re-save so their links stop pointing at a card only this tab could resolve. None of that is new code: it is the hook the store's own create path runs, reached from a second caller.

A batch that adopts a card also runs under that card's mutation lock, so an autosave that starts while the batch is in flight waits for the realm to name the card and then patches it, rather than creating a second one.

Naming the cards a batch mints

The realm names a created card's file after the local id its entry carried. A local id is therefore the tail of a URL the card keeps, not a token that only has to survive the request — and a per-batch counter had every batch in a realm claim the same names. The second batch to mint a card of a given type was refused for writing a file the first one had already written.

A batch now has a name of its own, and names the cards it mints under it: <uuid>_1, <uuid>_2. A card the caller is already holding keeps the name that caller knows it by instead, which is what makes the URL the realm mints end in the id the holder is already using.

One event, two cards, two answers

An index event named one request and every card the pass touched. A client recognizing the request as its own then had to choose between two wrong answers for the whole pass:

	re-read everything	re-read nothing
the card we described	loses an edit typed while the write was in flight	correct
the card the realm computed	correct	shows what it said before, permanently

Neither is right for a batch, because a batch does both at once — b.create(activity) carries content the client is holding, while the addActivity beside it produces state only the realm has.

So a write can now name the cards it wrote from content its caller supplied, and the event carries those names beside the request id. The store skips exactly those and re-reads the rest. The coordinator names the cards a batch minted under a caller-chosen local id; a card the realm named itself, and a card the batch changed rather than minted, hold state no caller has. A writer that names none says nothing about the pass, which is what every single-card write means by it — so the REST path is unchanged.

Testing

The client core (card-operations-client-test.ts, standalone, no realm) covers what a call emits: that two batches in one realm name their cards differently and that one batch names each of its own by position, that an adopted create carries the holder's name and field values and reports that name to the transport, that a create from data reports none, the three refusals, and the reader that turns a committed answer into lid → id pairs including from inside a group.

The coordinator (card-operations-batch-test.ts, standalone) covers which cards a commit claims: one minted under a caller's name, and not one the realm named itself or one the batch only changed.

The host (operations-store-reconciliation-test.ts) covers what the store is left holding — the promotion and both identities, the autosave race resolving to a single patch of the card the batch named, the data-only create staying store-opaque while the card it was linked into reloads and lazy-loads it, the foreign consumer re-saved with the real URL, and the conflicting pairing reported to the caller rather than thrown from an event handler.

Three of those deserve a note.

The per-card rule is tested by observing which cards the store went back to the realm for, with the re-read card and the skipped one asserted in the same test. A card's field values afterwards would not say which decision was made — and a linksToMany only materializes its targets when something renders them, which this suite does not do, so the data-only test stops at the report having been re-read and the realm holding the link rather than claiming anything about the field.

The realm's own broadcast is read in its own test rather than only hand-delivered events, because whether the names match is the one thing a hand-written event cannot check: the coordinator and the indexer arrive at that spelling separately.

And these tests hold a reference to the cards they read. A card the store merely read once is not retained and its realm is not subscribed, so a test asserting what an event caused would otherwise be asserting against a store that never heard one — with every "nothing happened" assertion in it passing for that reason instead of the one it names.

One existing test changes its subject rather than its expectations: a write's request id was asserted not to be registered, on the grounds that registering it would suppress the reload of a card the server computed. Registering it no longer means that, because the skip is now per card and a transform names none — so that test now asserts the id is registered and that the write claims no card.

Each fix carries a case confirmed to redden only itself when the fix is reverted — the batch-scoped name, the adopted name, and the commit's naming in both directions.

Locally: 8/8 the new host suite, 21/21 the existing operations suite, 116/116 the store suite, 88/88 the client core and 209/209 the coordinator, against a stack rebuilt from this branch.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-18T16:40:37.553719Z	0664f33	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0664f333cf

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[github-actions]

Preview deployments

• Host staging preview

• Host production preview

Host Test Results

    1 files  ±  0      1 suites  ±0   2h 30m 46s ⏱️ - 1m 20s
4 926 tests +309  4 912 ✅ +320  14 💤 ±0  0 ❌  -  1 
4 941 runs  +309  4 927 ✅ +330  14 💤 ±0  0 ❌  - 11 

Results for commit 708dbf6. ± Comparison against earlier commit 97cbe07.

Realm Server Test Results

    1 files  ±0    245 suites  ±0   1h 27m 44s ⏱️ + 6m 7s
3 663 tests +1  3 663 ✅ +15  0 💤 ±0  0 ❌  - 14 
3 714 runs  +1  3 714 ✅ +17  0 💤 ±0  0 ❌  - 16 

Results for commit 708dbf6. ± Comparison against earlier commit 97cbe07.

[habdelra]

[Claude Code 🤖] This pass traced the two host-side promotion paths this PR adds (the adopted-create lock/promote flow and the per-card clientAuthored echo suppression) against the store's pre-existing realm-event reconciliation, and checked the clientAuthored/invalidations naming contract end to end from the coordinator through the broadcast to the store's skip logic. It did not re-flag the two issues already fixed on this branch (the empty/absent clientAuthored collapse, and the duplicate-lid lock deadlock) — both read as complete and are covered by tests.

One correctness gap worth resolving before merge: a batch's conflict refusal leaves the conflicting card silently stale, with no test catching either half of that. The other two comments are a scoping question and a coverage gap, both non-blocking.

Recommendations:

1. adoptMintedIdentities's conflict throw discards every pairing in the batch, not just the conflicting one, and leaves the conflicting card's own resident instance permanently unreconciled because the write's clientAuthored still names it — see the thread on store.ts.
2. The clientAuthored filter in broadcastIncrementalInvalidationEvent silently drops any name not present in invalidations; I couldn't find a path where the two spellings diverge, but nothing pins that they can't, or that a divergence would be noticed — see the thread on realm.ts.
3. The duplicate-lid dedup in withMutationLocks has no test that actually calls it with a repeated local id — see the thread on store.ts.

CI is green on this commit; nothing to flag there.

Adjacent, out of scope: clientAuthored is populated from every staged create with a lid — including a named-operation create whose fields a fill template resolves server-side, not just the b.create(instance) adoption path. That's inert today because the store never holds a resident instance under such a local id unless it was adopted, but it's worth knowing the set is "every card this batch minted under a client-chosen name," not "every card an instance the client holds provided," if that assumption is leaned on later.

[habdelra]

[Claude Code 🤖] This narrows clientAuthored to names present in invalidations, dropping the rest silently. A dropped name flips that card from "skip reload" to "reload" for its owner — the exact edit-loss failure mode this member exists to prevent — with nothing surfaced when it happens.

I traced both spellings: StagedIdentity.id is url.href from paths.fileURL(...) with .json stripped (stagedIdentity in card-operations/executors.ts), and invalidations is built the same way — .json-stripped URL.href values from RealmPaths-rooted paths (realm-index-updater.ts's two onInvalidation call sites). I couldn't construct a case where the two disagree, and no test exercises a mismatch either way.

Is there a path where the spellings can diverge (an id-resolution route that skips RealmPaths, say)? If not, is it worth asserting or logging on a name that fails to match here instead of dropping it, since a divergence would silently reintroduce the exact bug this member exists to fix?

[habdelra]

[Claude Code 🤖]

Fair, and changed in 708dbf6 — a drop is now logged where it happens, naming the dropped URLs and saying their holders will re-read them.

Not turned into a refusal, because the narrowing itself is right: a name the event does not carry describes a card nobody can match it against, so emitting it would be worse than dropping it. What was wrong was doing that silently, since the failure it hides is the edit loss the member exists to prevent. A warning makes it findable without failing a write that has already committed.

Your trace matches mine — both sides spell a card as its realm href with .json stripped, and nothing is known to produce a divergence. The log is there for the case where that stops being true.

One thing your review surfaced that is worth stating plainly: this is not the only place the mechanism reached further than I thought. The card and source front doors call the same coordinator, so the naming was landing on every ordinary card write — including making a PATCH's empty list read as "re-read", which removes the autosave protection for every edit. That is fixed in the same commit by making authorship naming something a caller asks for, and the envelope is the only caller that does.

[habdelra]

[Claude Code 🤖] Two things happen when a pairing conflicts, not one.

The throw runs before pass two, so no pairing in minted gets promoted — not just the conflicting one. sendOperationsEnvelope's promise rejects, so the caller loses answer and has no way to read the ids of the batch's other, non-conflicting cards short of a fresh query. Is losing all of them the intended cost of one conflict?

The conflicting card is left stale, and nothing else will correct it. clientRequestId is already registered in cardService.clientRequestIds before this runs, and the coordinator's clientAuthored for this write already names this exact URL — computed server-side, with no knowledge of the client-side conflict. So when the realm's own event arrives, #reloadInvalidatedInstances reads the URL as one this client authored and skips reloading it: the instance already resident there, the one this error is about, keeps showing its pre-write content with nothing left to fix it.

The only test for this path (a pairing that contradicts one the store already made is reported to the caller) asserts just the rejection — it never delivers the follow-up event or checks the resident instance afterward, so neither effect is covered.

[habdelra]

[Claude Code 🤖]

Both parts were right, and both are changed in 708dbf6.

Losing the whole answer was not intended — it was a consequence of ordering the check as one pass over everything before any promotion, which I had done to avoid a partial promotion. Your framing is better: the batch committed, so every pairing the store can honor is one the realm already agrees with, and withholding them is not undoing a partial success, it is dropping the rest of the batch over a quarrel about a different card. Now every pairing is judged first, the honorable ones are all promoted, and only the conflicting card is held back — so which cards get promoted no longer depends on where in the list the conflict sat.

The second half is the one I would not have found. The write names that card as carrying this client's content, because for the local id the batch sent it does, so the realm's own follow-up event is read as one to skip — while the instance the store actually holds for that URL belongs to a different local id and is stale with nothing coming to refresh it. The conflicting card is now re-read explicitly, which is the only thing that closes it, guarded on the held instance having a URL to re-read at all.

Still throwing afterwards, and deliberately: the caller's model of identity is wrong, and a conflict is not something to absorb quietly. The message says the batch committed and that the card is being re-read, so the caller knows the write landed and what state the tab is in.

[habdelra]

[Claude Code 🤖] The dedup here ([...new Set(localIds)]) is what stops a batch naming the same card twice from deadlocking on its own nested lock — but nothing calls this with a repeated local id. The client-core suite (card-operations-client-test.ts, "one card named twice is reported once to the transport") only confirms the transport receives the duplicate name; it stubs the transport entirely and never reaches this method. Every b.create(...) call in operations-store-reconciliation-test.ts, the host suite that does reach it, adopts a single instance per batch.

Worth a case that adopts the same instance twice in one batch and asserts the call settles (resolving, or rejecting on the realm's own duplicate-lid refusal) rather than hangs — that's the scenario the dedup exists for, and it's currently only reasoned about, not exercised.

[habdelra]

[Claude Code 🤖]

Correct — the dedup had no test that exercised it, and the client-core case you name proves something else at a different layer. Added in 708dbf6.

The new case drives withMutationLocks directly with the same local id twice and asserts both that the work under the lock ran and that the call returned, raced against a timeout so a regression fails rather than hanging the suite. Driving the store's own lock rather than going through a batch is deliberate: a batch naming one card twice reaches this before anything else, and it has to get past it to receive the refusal the realm already has waiting for it.

[habdelra]

[Claude Code 🤖]

One change since the description was written, worth calling out because it moves where this touches.

The card and source front doors reach the same coordinator the _operations envelope does — commitBatch has five call sites in realm.ts and only one of them is the envelope. So computing a write's client-authored cards inside the commit put the new clientAuthored member on the index event of every ordinary card POST, PATCH and DELETE, and a PATCH's empty list read as "re-read this card" where the previous behavior was "leave it alone" — the autosave protection the request id exists for, removed for every ordinary edit.

Authorship naming is now something a caller asks for. The envelope asks, because it is the one front door that writes several cards whose state came from different places. The card and source routes do not, so their events are exactly what they were, and a test pins that they stay silent.

Verified locally against a stack built from this branch: 10/10 the store-reconciliation suite, 21/21 the operations suite, 116/116 the store suite, 212/212 the coordinator and 90/90 the client core.

Not yet verified locally: card-endpoints-test.ts, which is the suite that caught this. A synapse belonging to another local run holds port 8008, so the realm-server lane skips provisioning and every fixture test fails at matrix login — an environment failure, not a signal about this branch. CI covers it in the meantime and I will run it locally when the port frees.