A write waits for its own indexing and never for anyone else's

[habdelra]

A write waits for its own indexing, and never for anyone else's

That is the whole of it. Reading your own write matters — a card save answers its response out of the index, so it has to be settled by a pass that ran after its bytes landed. Being made to read someone else's write does not matter at all: serving another user a slightly stale version of the card you are updating is correct behaviour, not a compromise, and nothing in the system is owed a globally current view.

Three places did not honour that. Each made a writer wait on, or be answered by, indexing it had no stake in.

1. The file locks were held across the index wait

The locks over the files a write touches exist to order writers of those files against each other — two writers that both read a card's stored bytes, merge independently and write must not interleave, or the second silently drops the first's changes. That ordering is settled the moment the bytes are durable. Everything after it is indexing.

Held across the index wait, the second person editing a card waited out the first person's indexing before they could so much as stage. The commit now announces its durable boundary and the lock helper ends its critical section there. Both coordination layers honour it: the pg_advisory_xact_lock transaction commits, and the in-process queue in front of it advances, so the next writer is not let through by postgres only to be held in memory instead. A section that never releases behaves exactly as before.

A removal is the exception, and keeps its files to the end of its pass. Not because it waits for anyone else — it still waits only for its own — but because its ordering is not settled by durability the way a write's is. An index pass resolves a removal and a write of one url as the removal whichever reached it first: the queue's merge and the visit loop's operations map both give delete precedence, and the visit is then skipped without asking whether the file came back. So a writer that recreated the path while the removal's pass was still pending would be folded into it, and the row dropped for a file that is on disk. Releasing at the durable enqueue would not close that either — the absorption lasts for as long as that job sits unclaimed.

The deeper asymmetry, that both merge points assume a removal is always last, is pre-existing and left alone.

2. A write could be answered by a pass that predated it

A claimed index pass reads each file it visits once, and it was claimed before a later publish existed — so it may already have read the bytes that publish supersedes. Attaching to it as a late waiter answers the writer from a version of their own card that predates the write they just made. That is the one thing the principle does not permit: it is not someone else's staleness being tolerated, it is your own write going missing from your own response.

So a publish whose caller reads the index for its own urls inserts its own pass instead, and the next one behind it merges into that pending job. A burst of saves to one card costs the two passes read-your-writes actually requires — the pass in flight, then the one carrying your bytes — rather than one-with-a-stale-answer, and rather than one per save.

Which callers those are is named by the caller, not inferred from the shape of the call: awaiting a pass and reading one back are different things. The commit's own pass is flagged. The file watcher announcing a change made elsewhere, and a removal answering 204, are not — they have no stake in the result, they are exactly who the in-flight join exists to serve, and flagging them would add a pass to the realm's serial lane for every externally-observed change landing mid-pass.

3. No gate could tell one writer's pass from another's

The writer's identity already reaches the enqueue, and already scopes the read path's drain. It lives on a map entry in one replica's memory, so any gate built on the jobs table sees every writer's pass rather than its own caller's — which is the realm-wide wait this whole principle is trying to remove, reappearing as soon as there is more than one replica.

jobs.initiated_by carries it into shared state, and awaitRealmIndexSettled takes an initiatedBy scope beside its existing job-type one.

Nothing passes that scope yet — its consumer is the read path's read-your-writes drain, which today asks the same question against one replica's memory and can now ask it of the lane. The write path deliberately does not use it: a write waits on passes that touched an executable module, because a module changes what every writer resolves, and on nothing else. Waiting on its own earlier passes as well would reinstate exactly the serialization this change removes, for the one workload anyone has measured — several saves to one card come from one person, so a writer scope matches every pass in that shape.

A set rather than one name, because passes coalesce: a publish that merges into a pending job joins the callers already on it, and recording only whoever got there first would make an absorbed writer invisible to its own gate. A jsonb array rather than a postgres array, since no table here uses one — the query builder binds an object-valued parameter as JSON, so an array column would need a binding path of its own.

One option carrying both the writer and the realm owner, not two, because the owner is what decides an untagged pass: a scope naming only the writer compares against NULL and lets every one of them through — precisely the file-watcher and GC passes that arm exists to cover. Both are full matrix ids, as the column records them.

A pass no HTTP write produced records nobody and reads as the realm owner's rather than as nobody's, so a file-watcher echo or a GC sweep still gates somebody. Worth saying plainly: the owner pays for those and no other writer does.

Where the principle is not true yet

The pre-staging drain still waits on every index pass in flight in the realm, whoever produced it. So the second writer of one card now takes the lock immediately and parks there instead. Net latency is about the same; the stage it is charged to changes.

That is also why a measurement of eight saves to one card reads the drain at zero while the lock holds two thirds of the handler — the lock hold is what kept the drain clear. Take the hold away and that is where the seconds go. A before/after on drain should be read as the same wait renamed, not as a regression.

So this is the prerequisite, not the payout: it removes the structural obstacle and gives a cross-replica gate something to scope by. Narrowing that drain — to the passes that touched an executable module, and to nothing else — is separate work in flight. Please do not read this as a latency fix with a number behind it.

One other thing the early release gives up: the realm's shared guard ends with the file keys, since one transaction holds them all. A section that releases excludes a realm-lifecycle caller up to its durable write rather than to its own end, which is now stated where the lock is taken. A section whose remaining work depends on the realm surviving it should hold to the end; making lifecycle callers drain indexing before taking the key is a change to those callers rather than to this primitive, and is not folded in here.

One assertion is deliberately still absent: that two saves to one card do not serialize. That needs both halves — without the release the second writer blocks on the lock and never reaches the drain, and without the narrowing it clears the lock and parks at the drain instead — so it belongs with whichever of the two lands second. What the tests here pin is the locks, which is why each holds its index pass open: with the drain live the second writer parks there and no assertion could say which gate held it.

Test plan

Every claim is covered at the layer it is made, and each new test was run against the defect it names before being kept.

The lock primitive (realm-advisory-locks-test, 37 passing)

• a second writer of one file runs while the first is still working, once the first releases — exercising both the advisory lock and the in-process queue, since the second writer must clear both
• a released section still reports its own value, and its own failure, and does not strand the next writer of those files behind it
• a section that fails before releasing still excludes the next writer until it unwinds, so the release is the only thing that ends a section early

Negative control: with the early release disabled, exactly the first of these fails; the other two stay green, which is what they are for.

The coordinator (card-operations-batch-test, 375 passing across the stub suites)

• the files are held across the read-merge-write and open again while the write is indexed, asserted as the lock depth observed at the commit's two boundaries rather than as a duration
• a batch that removes a card keeps its files shut while the removal is indexed, and opens them again when it returns
• the section ends at the commit rather than early on the way in, and nothing the batch acts on is read with the files open to others

Negative control: with the release not passed to the commit, the first fails with indexWait:held=1 against the expected held=0, and nothing else moves.

The lane gate (await-realm-index-settled-test)

• another writer's pass does not hold this writer; the writer's own pass does
• a coalesced pass holds every writer it carries, from either direction
• an untagged pass gates the realm owner alone
• the writer scope ANDs with the job-type scope rather than replacing it

Negative control: with the scope clause removed, the two tests that assert a writer is let through fail and the rest stay green.

The queue (queue-test)

• the row records every writer whose work a coalesced pass carries, and a publish naming nobody leaves those already there alone
• a publish naming no writer leaves the row null, not an empty set — the two read differently at the gate
• a publish that reads its own write does not attach to a running pass, while a bystander publish with the same change set still does, and the one behind it merges into the pending job rather than inserting a second

Negative control: with the incoming writer dropped from the union, the coalesced-row test fails and nothing else does.

Against a real realm (card-save-concurrent-test), where the contrast is what gives either half its meaning — same harness, same observation window, different answer:

• an update releases at the durable write, so a second save reaches its own index pass while the first is still parked on one
• a removal does not, so a recreate stays queued on the files with nothing written, and takes them only once the removal's pass has run

End to end, through a browser (concurrent-save-read-your-writes): eight saves fired together at one card, each carrying a higher revision, and every response answered with at least the revision that save sent — read-your-writes as monotonicity. Nothing below that suite can see this. The host's tests run on SQLite and have no jobs table, so the coalescing decision does not exist there; a realm-server test either runs a worker it cannot control or stubs the updater and stops exercising the thing that decides. Here the realm server, its worker, its postgres queue and a real session are all present, and the decision is made where production makes it. How much of the burst overlapped is reported as an annotation rather than asserted — a floor would be asserting the machine was busy enough, which is a different claim and a flaky one.

Migration applied, reversed and re-applied against local postgres; the SQLite schema file is regenerated (content-identical — jobs is a postgres-only table and does not appear in it, so the change is the required filename bump).

lint:types and lint:js clean for runtime-common, postgres, realm-server, host, bot-runner and ai-bot.

🤖 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:07:50.913641Z	926980a	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: 926980a818

ℹ️ 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      1 suites   2h 38m 54s ⏱️
4 914 tests 4 900 ✅ 14 💤 0 ❌
4 929 runs  4 915 ✅ 14 💤 0 ❌

Results for commit 95ddbfa.

Realm Server Test Results

    1 files    245 suites   1h 20m 20s ⏱️
3 633 tests 3 633 ✅ 0 💤 0 ❌
3 684 runs  3 684 ✅ 0 💤 0 ❌

Results for commit 95ddbfa.

[habdelra]

[Claude Code 🤖] This review went at the two coordination changes — where the file-write critical section now ends, and which existing index pass a publish is allowed to be satisfied by — plus the new jobs.initiated_by column end to end: insert, coalesce join, and the gate's SQL. It did not run the suites; every claim below is traced through the tree at this head.

As a lock primitive the early release holds up — the rollback arm, the pool-client hand-back and the in-process queue all advance where the comments say they do. What I'd want fixed before this merges is that three of the four call sites let the locks go before the index job is durably enqueued, which reorders a removal against a recreate of the same file.

1. onDurable fires ahead of the enqueue at the batch commit and at both delete primitives, and mergeIncrementalChanges gives delete precedence whichever way the coalesce arrives — thread on options?.onDurable?.() in runtime-common/realm.ts. Blocking.
2. updateChanges flags every one of its callers readsOwnWrite, including the file-watcher echo the in-flight join was kept for — thread on updateChanges in runtime-common/realm-index-updater.ts.
3. initiatedBy and realmOwner are separable options and separating them fails open — thread on the options object in runtime-common/jobs/indexing.ts.
4. An in-flight join structurally cannot carry an initiatedBy update, so a deferred write's writer never reaches the row — thread on the in-flight branch in runtime-common/tasks/indexer.ts.
5. argsReadOwnWrite's doc comment is attached to mergeInitiators, and the reason it gives does not distinguish it from the parsed shape — same file.
6. The release drops the shared realm guard together with the file keys, and realm deletion removes pending job rows out from under a parked waiter — thread on the lock-helper comment in postgres/pg-adapter.ts.

Adjacent, and not asked of this PR: Realm.handleAtomicOperations calls withFileWriteLocks and ignores the releaseLocks it is now handed, so POST /_atomic?waitForIndex=true still holds the file locks across the entire index wait — the one write path the change does not reach. Nothing in that section reads the files after the commit, so threading it looks mechanical; say if you would rather it stay as it is.

[habdelra]

[Claude Code 🤖] The release lands before the index job is enqueued, so a removal can be reordered against a recreate of the same file.

onDurable() fires here, ahead of changes being assembled and published — and the same is true at _deleteUnlocked and _deleteAllUnlocked. On main the enqueue sits inside the locks on all three.

mergeIncrementalChanges resolves a (url, delete) against a (url, update) as delete whichever arrived first, and the pending-candidate join in chooseIncrementalCoalesceDecision carries no readsOwnWrite guard. So with the locks already gone at durability, the next writer of that path can take them, recreate the file, publish {url, update}, and have that publish absorbed into the removal's still-pending job — the pass then drops the index row for a file that is on disk, and the recreating writer's own read-your-writes returns without it. The window is as long as the removal's job sits unclaimed.

Suggested shape: fire onDurable at the durable enqueue rather than before the branch. The awaited arm already names that moment (onEnqueued, which updateIndexAndCollectInvalidations uses for mark('enqueue')); the deferred arm has it where enqueueIndexUpdateAndCollectInvalidations returns. The changes.length === 0 arm then needs its own call, or that section keeps its locks to the end.

Worth a test with the fix: two writers of one path, the first removing it and the second recreating it, asserting the recreate does not end up in a pass whose change for that url is delete.

Regression for the synchronous write and delete paths. The deferred delete path can already lose this ordering to a coalesce on main, so that half is pre-existing — this change is what opens it on the paths that await their own pass. Blocking. The automated review names one of the three sites; the other two have the same window.

[habdelra]

[Claude Code 🤖] Right on both counts, and the fix in 011176d172 goes further than the comment asks: a change set carrying a removal keeps its files to the very end of its pass rather than to the enqueue.

Firing at the durable enqueue would not have closed it. The absorption runs for as long as the removal's job sits unclaimed — an update publish arriving then still coalesces into the pending row and still resolves as delete. Only holding until the pass has run keeps the two ordered, which is what the locks did before.

Confirmed the visit half as well: index-runner.ts builds its own operations map with the same precedence and skipReason returns 'delete' without stat-ing the file, so the recreated bytes go unnoticed all the way down.

card-operations-batch-test now pins the coordinator carrying whatever the commit decides rather than announcing a boundary of its own. The realm-level carve-out — that the commit withholds it when deleteURLs is non-empty — has no dedicated test yet: it needs a real realm and the machine-wide lane is held by other work. The existing realm-server delete coverage exercises the restored behaviour but would pass either way, so I am not claiming it as a guard.

[habdelra]

[Claude Code 🤖] readsOwnWrite: true here reaches the file-watcher echo, which the coalesce comment says it does not.

The echo's only route into indexing is Realm.drainUpdates -> updateIndexAndCollectInvalidations -> this method, and readsOwnWrite is not in the Pick above, so no caller can opt out. The comment on the in-flight branch in tasks/indexer.ts — "the file-watcher echo this branch was built for has no such stake ... and is unaffected" — therefore describes a caller that does not reach that branch: an echo inserts a pass of its own rather than attaching to the running one. The reindex handler is in the same position; it answers 204 and reads nothing back.

Where the watcher is enabled that is one extra pass per externally-observed change landing while a pass runs, on the realm's serial indexing lane.

Which way do you want it — thread the flag down from the callers that do read the index back, leaving drainUpdates and the reindex handler unflagged, or keep the blanket true and correct the comment in chooseIncrementalCoalesceDecision (and the description) to match?

Regression; blocking only to the extent that the prose currently states the opposite of the behaviour.

[habdelra]

[Claude Code 🤖] Correct — traced the same chain and fixed it in 011176d172 by taking the first of your two options.

readsOwnWrite now comes from the caller that reads the index for its own urls rather than from the shape of the call. updateChanges carries it through its Pick and sets nothing itself; the commit's own pass names it, because that is the pass a card write answers its response from. The file watcher via drainUpdates and the reindex handler both go unflagged and can attach to a running pass again.

Awaiting a pass and reading one back are separate things, which is what the blanket true conflated — and the cost of conflating them was one extra pass on the realm's serial lane per externally-observed change landing mid-pass, exactly as you describe.

[habdelra]

[Claude Code 🤖] Make the pairing structural rather than a sentence.

initiatedBy and realmOwner are independent optionals, and the comment directly above is the only thing holding them together. Passing initiatedBy alone renders AND (initiated_by @> to_jsonb($n::text) OR (initiated_by IS NULL AND $n = NULL)) — the second arm is NULL, so every untagged row is let through and the gate reports settled for exactly the file-watcher and GC passes it is meant to cover. I checked the shape against postgres: the @> containment and the parameter type resolution are both fine; it is the NULL comparison that fails open. No case in await-realm-index-settled-test passes one option without the other.

Nothing calls this with a scope yet, so the first caller is the one that gets it wrong silently. A single option — initiatedBy?: { user: string; realmOwner: string } — makes that unrepresentable.

While in here: realmOwner's spelling is undocumented and Realm exposes two that differ — getRealmOwnerUserId() gives the full matrix id, getRealmOwnerUsername() strips the sigil and the server. Only the first can ever match an initiated_by entry, and picking the other fails open the same way. Worth stating on the field.

Regression (new API), non-blocking.

[habdelra]

[Claude Code 🤖] Taken as written — 011176d172 makes it one option, initiatedBy?: { user: string; realmOwner: string }, so the fail-open combination is unrepresentable rather than warned against.

The NULL analysis is right: with only the user bound, the second arm is $n = NULL, which is NULL rather than false, so every untagged row is let through — the file-watcher and GC passes that arm exists to cover. A comment was the wrong thing to be holding that together.

The spelling note is now on the field: both are full matrix ids as initiated_by records them, getRealmOwnerUserId() answers with one, and getRealmOwnerUsername() strips the sigil and the server and can never match an entry.

[habdelra]

[Claude Code 🤖] A publish that takes this branch cannot record its writer, so that identity never reaches shared state at all.

coalesceAndGetCanonicalJobId throws when a join on an in-flight target carries an update, so this return { type: 'join', jobId: candidate.id } is obliged to drop initiatedBy. That is exactly the branch a deferred write takes: enqueueChanges sets initiatedBy and never sets readsOwnWrite, so a write whose urls a running pass already covers attaches here and the jobs table names nobody for it. A cross-replica gate scoped to that writer then reads settled while the pass carrying its bytes is still running — the case the column exists to close.

What should the gate that follows do about it: is the deferred path meant to sit outside writer-scoped gating, or does this branch need to stop being reachable for a publish that names a writer? Either answer deserves a line here, because the column is otherwise documented as carrying every writer a coalesced pass carries, and this is the one join where it cannot.

Regression in machinery nothing gates on yet, non-blocking — but it decides the shape of the follow-up gate.

[habdelra]

[Claude Code 🤖] Confirmed. 011176d172 states it where you asked rather than answering it, because the answer belongs to the gate.

The branch is documented as the one join where the row cannot be made to describe every writer waiting on the pass, and it names the decision the first gate has to make: whether a publish naming a writer may take it at all. Deciding that here would be guessing at a consumer that does not exist yet, and the wrong guess is invisible until one does.

For the record of what is reachable today: a publish that names a writer and does not read its own write is the deferred write path, and it is the only caller that can land here with an identity to lose. A publish that reads its own write already refuses this branch for a different reason.

[habdelra]

[Claude Code 🤖] This doc comment belongs to argsReadOwnWrite, and the reason it gives does not hold.

The first paragraph documents argsReadOwnWrite, the second documents mergeInitiators, and both sit above mergeInitiators — so the function the first paragraph is about is the one left undocumented. Please split them.

The claim needs a second look too: "read loosely, off the raw args rather than the parsed shape" implies the parsed shape is stricter, but parseIncrementalArgsForCoalesce already computes readsOwnWrite: readsOwnWrite === true, and incomingArgs.readsOwnWrite is in scope at the call site with the identical answer. As written this is one decision with two spellings that can drift. Either use incomingArgs.readsOwnWrite and drop the helper, or say what reading the raw args actually buys.

Regression, non-blocking.

[habdelra]

[Claude Code 🤖] Both points taken in 011176d172.

The helper is gone. You are right that it was one decision with two spellings: parseIncrementalArgsForCoalesce already computes readsOwnWrite: readsOwnWrite === true, and incomingArgs is in scope at the call site with the identical answer, so the branch now reads incomingArgs.readsOwnWrite and there is nothing left to drift.

The comments are split. mergeInitiators keeps its own, and the paragraph that had attached itself to the wrong function went with the helper.

[habdelra]

[Claude Code 🤖] The early release drops the realm guard along with the file keys — worth saying so here, and deciding whether the lifecycle callers now need a drain of their own.

The realm's own key is taken in the same transaction as the file keys, so the COMMIT that releaseLocks triggers releases it too. The lock-plan comment below still reads that a realm-lifecycle caller taking the realm key exclusively "still excludes every one of them", and that now holds only through durability rather than through the section.

The concrete case: realm deletion takes the realm key exclusively and, inside it, removeRealmDatabaseArtifacts force-cancels running jobs in the indexing group but DELETEs the pending ones. A deletion landing in the window this change opens therefore removes the very job row a synchronous write is parked on, and that waiter has nothing left to be notified from — the notification drain only reports rows it can still select.

Two asks: narrow the lock-plan comment so it stops claiming the guard spans the section, and say whether a realm-lifecycle caller should now drain indexing before taking the key.

Regression in scope — on main the guard spans the index wait on the awaited path — non-blocking.

[habdelra]

[Claude Code 🤖] Confirmed — one transaction holds the realm guard and the file keys, so the COMMIT that ends the section early ends the guard with it. 011176d172 does the first of your two asks: the lock-plan note no longer claims the guard spans the section, and the early-release note says what a section that outlives it is giving up.

On the second, I am stating the contract rather than changing the callers: a section whose remaining work depends on the realm surviving it should hold to the end. That is the honest boundary to draw from inside the lock, which cannot know what its caller is waiting on.

Your deletion case is real and worth keeping visible — the pending job rows a released section is parked on can be removed out from under it, and the notification drain only reports rows it can still select. Making a realm-lifecycle caller drain indexing before taking the key is the fix, and it is a change to those callers rather than to this primitive, so I would rather it were its own than folded in here. Say if you would rather see it in this one.