Refuse a card write whose If-Match names a card that moved

[habdelra]

PATCH and DELETE on a card+json URL now honour If-Match, so a client can say "only apply this if the card is still the one I saw" and be refused rather than silently overwrite a card that moved underneath it. A card that no longer matches answers 412, with nothing staged, nothing enqueued and no event broadcast. A request carrying no If-Match behaves exactly as it did.

The change is additive: three files, no deletions.

What it compares against

The card's ETag — the validator a GET hands out and an If-None-Match is checked against. It is the only fingerprint a read gives a client, so it is the only one a client can hold.

It is the broader of the realm's two fingerprints and the later of them. Broader, because it moves whenever the served document may differ — a linked card's re-index included — where the stored file's hash moves only when this card's own bytes do. Later, because it is built from indexed_at, which moves after the bytes rather than with them.

Where the check runs is what makes it mean anything, so it runs inside the commit's write lock. CommitBatchOptions gains a precondition that commitBatch invokes inside core.withWriteLock, after its drain and before anything stages. The realm keeps what the check is — an If-Match compares an ETag built from index and realm-info state the coordinator has no business assembling — and the coordinator owns only when it runs. A refusal travels as the operation failure it throws, so the 412 reaches the caller by the path every other card-write refusal takes.

Both halves of that placement are load-bearing:

• After the drain, because a commit records a file's hash before it indexes and a skip-index-wait write defers indexing to a worker. Read without draining, the check consults a row that still spells a validator the caller has already been overtaken by, and accepts the write this header exists to refuse — with no concurrency in the request at all.
• Inside the lock, because the lock is acquired within commitBatch. A check made before that call can pass and the request then queue behind another writer's entire write, so it holds only while the realm is uncontended — which inverts what a conditional write is for.

And it refuses when it cannot decide. The realm's own view of in-flight indexing is an in-memory map of the jobs this process enqueued, so a peer replica's pending job is invisible to it. The check therefore also queries the realm's indexing lane in the shared jobs table. It asks about the whole lane rather than the job types a write races: a realm republish swaps files under the write lock and enqueues a from-scratch-index before releasing, so that job type is one of the strongest reasons the index cannot be believed, not one to exclude. If the lane will not settle within its budget the write is refused rather than answered: an unsettled lane is exactly the state in which the row still describes the pre-write card and a validator the caller has been overtaken by would match, so answering from it would turn congestion into a silent accept. It logs, because a lane that never drains otherwise looks from here like a realm with nothing to refuse.

That refusal is a 503, not a 412 — nothing about the caller's request is wrong and repeating it is the remedy — under a new precondition-unverifiable code, so the wire says "could not decide" rather than implying a conflict. Its budget is deliberately shorter than a readiness probe's, because this one waits with the write lock held: every other writer is queued behind it, so a long wait spends their latency to answer one request.

The check belongs to the precondition rather than to the shared drain, which is untouched. Riding the drain would have charged every card write for a guarantee only a conditional one asks for, made a removal wait on an undeadlined in-memory gate while holding the lock, and — since the drain discards its answer — reintroduced the silent accept above.

Three decisions worth a look

• * falls through rather than answering for itself. It asks only that a card be there, which these handlers already settle — a DELETE from the stored file inside the write lock, a PATCH from the commit. So * against a path with nothing at it reaches the 404 it always would, which says more than a 412 does.
• A concrete validator needs one to match. A card the realm can offer no validator for — never indexed, or an ETag suppressed because the document depends on another realm — fails the precondition, because "this is the card you saw" is a claim the realm cannot make about it.
• Either link-shape variant matches. A card+json read picks its link shape per request, and the shapes take different variants of one validator so a client holding either is never 304'd to the other. That distinction is about representations; a conditional write asks about the card, so the gate accepts either variant at the card's current indexed_at. Refusing over the shape a preceding read happened to answer in would refuse on a server setting rather than on anything the caller did — and that setting is on its way to varying per request with load.

Comparison reuses ifNoneMatchMatches: *, comma lists, and the W/ prefix ignored on both sides. RFC 9110 §13.1.1 asks for strong comparison where §13.1.2 allows weak, but the realm emits no weak validators, so the two rules differ only over a validator no response of ours produced.

The precondition is read after the body is validated, so a payload the realm would have refused anyway still gets the 400 naming what is wrong with it, and a 412 means only that the card moved.

meta.version is not here

The ticket also specified meta.version — the stored file's content hash — on card+json GET, POST and PATCH bodies. It is not in this PR, deliberately.

Nothing in the repo reads it. Every occurrence would have been a producer or a strip; there is no consumer in the host, in base, or in boxel-cli. Meanwhile adding a key to the served read representation moves none of the ETag's inputs, so CARD_JSON_ETAG_VARIANT has to be bumped — which makes every client holding a cached card body revalidate and refetch, fleet-wide, to receive a key nothing reads.

The consumers are the index-event and envelope surfaces, and that work now specifies version itself rather than depending on this ticket for it, including why the read path is left alone. If-Match remains the only way to refuse a write on a stale base, which is what this PR delivers.

Tests

packages/realm-server/tests/card-conditional-write-test.ts — kept out of the characterization file.

The refusal test wedges the indexing lane the way one actually wedges: a job claimed under a far-future reservation, which nothing runs. A bare unfulfilled row does not do it — the worker claims that, fails on the empty args, and poisons the realm's next index job, so the test passes while its premise was never established and the damage surfaces as a neighbouring write's 500.

The "no event" test is worth a look: its positive control writes a different card, because a control writing the same card with the same body would be a no-op if the refused write had landed — and a no-op writes nothing and queues nothing, so the event counts would agree either way and the test could not fail for the regression it names. It also counts incremental-index-initiation, which is broadcast per written file before indexing runs and is the earliest signal that a refusal staged anything.

Two of the tests are in card-operations-batch-test.ts rather than with the others, because placement is not observable from a response. A PATCH answers the same status and body whether the precondition ran inside the lock, outside it, or not at all — so every endpoint assertion here passes on the broken arrangement too. Those two read the stub's lockDepth() and drainCount() from inside the precondition and assert lock depth 1, drain count 1, and nothing staged at that moment. Verified able to fail: moving the hook outside the lock turns both readings to 0 and reddens exactly that test, while the other 81 in the file stay green.

🤖 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-17T09:03:43.742247Z	08c001c	Manual request

ℹ️ 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: 68a3852fbd

ℹ️ 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 42m 14s ⏱️
4 885 tests 4 871 ✅ 14 💤 0 ❌
4 900 runs  4 886 ✅ 14 💤 0 ❌

Results for commit 824d02f.

Realm Server Test Results

    1 files    242 suites   1h 25m 38s ⏱️
3 526 tests 3 526 ✅ 0 💤 0 ❌
3 574 runs  3 574 ✅ 0 💤 0 ❌

Results for commit 824d02f.

[habdelra]

[Claude Code 🤖] Read for whether the two validators can be served as a mismatched pair, whether any write path reaches a commit without passing the gate, and what the new suite can actually fail on. I did not run the realm-server suite — the lane was held elsewhere — so everything here is static reading or measured out of process; CI is the check on the new suite itself.

One blocking item: the card+json ETag variant is not bumped, so meta.version never reaches a client that already has the card cached. The rest are non-blocking — one new test that cannot fail for the regression it names, one narrowing that degrades silently, and two questions.

On the two open threads, both confirmed with a mechanism; replies are in-thread. The conditional gate reads the index, which this same handler treats as lagging the stored file by design, so during a write's indexing window a stale ETag matches and the write is accepted — a wrong answer rather than only a race. And the version/document skew has a no-race form: commitUnlocked runs persistFileMeta before performIndex, and only the writer's own reads drain, so another reader assembles the pre-write document and reads the post-write hash. The skew is one-directional — version runs ahead of the body, never behind, which is the worse direction for the reconcile it exists for.

Recommendations:

1. Bump CARD_JSON_ETAG_VARIANT — thread on the meta.version assignment in #assembleCardJson.
2. Point the no-event test's control at a different card — thread on card-conditional-write-test.ts.
3. Narrow with the discriminant plus a cast instead of isSingleCardDocument — thread in #assembleCardJson.
4. Say what exempts the card path from the size check the byte path applies to content_hash — thread on storedFileMetaFor.
5. Decide whether the host instance should hold version — thread on the editor test.
6. If the two threads above are deferred, narrow the claims that rest on them: resource-types.ts states flatly that version is "the base an optimistic client reconciles against", and #conditionalWriteRefusal rests on the ETag being "the stricter of the two".

Every write path in scope does pass the gate — I traced the skip-index-wait / prerender echo, the !changed reserialize retry, and the browser-test fallback, all of which sit after it — and the x-created read genuinely gains a column rather than a round-trip, getFileMeta being one query on the same primary key getCreatedTime used. The 23 added strips all sit on responses that do carry a version: the eleven in card-endpoints-test.ts are GETs of cards written through the API earlier in the same test (the description's table calls them POST/PATCH echoes), the ten in realm-test.gts are write responses.

CI was still running when this was written; nothing red to act on yet.

Adjacent, out of scope: POST and DELETE on the card+source mime reach upsertCardSource / removeCardSource, which write and unlink the same <card>.json without passing the gate — so the conditional-write guarantee is card+json-facade-only, not per card. Worth knowing before a client treats If-Match as protection against every writer.

[habdelra]

[Claude Code 🤖] CARD_JSON_ETAG_VARIANT needs a bump, or meta.version never reaches a client that already has the card cached.

This adds a key to the served card+json representation without moving any input to the validator — indexed_at, the realm-info hash and the screenshot fingerprint are all untouched, and the constant is still card-rri on this branch. card+json goes out max-age=0, must-revalidate with an ETag, so a client that has seen the card sends If-None-Match, gets 304 against the identical validator, and keeps a stored body that predates version. Since the median row is never re-indexed, that is not a short window — it lasts until the card is next written or the realm info changes.

It also bites inside a single rolling deploy: an old-revision replica hands out ETag E with a version-less body, a new-revision replica answers 304 to the same E.

The constant's own comment is the ask — "bump this variant whenever the served card-JSON representation changes so caches revalidate instead of 304'ing a client to a stale body".

Regression, blocking.

[habdelra]

[Claude Code 🤖] Right, and the bump is no longer needed — because the key that required it is gone.

meta.version is no longer added to any card+json response. Nothing in the tree consumed it: every occurrence was a producer or a strip, with no reader in the host, in base, or in boxel-cli. So the served read representation is byte-identical to before, CARD_JSON_ETAG_VARIANT stays card-rri, and no cached body is invalidated.

Your reasoning is what made the cut obvious, so it is worth stating where it landed rather than just closing this: paying a fleet-wide revalidation to deliver a key with no reader is the trade that could not be justified. That reasoning is now recorded on the ticket that owns emitting version, so whoever adds it to the read path decides the bump deliberately.

Worth knowing for that future change: the grep this would normally be caught by does not work. Four assertions across card-endpoints-test.ts and card-html-endpoints-test.ts pin the validator by regex (/^"\d+(?:-[0-9a-f]+)?:card-rri"$/), and none of them contains the constant's name.

Fixed in 782ba8b.

[habdelra]

[Claude Code 🤖] The control sends the same body to the same card as the refused write, so neither event assertion can tell the two outcomes apart — including the exact regression this test names.

Suppose the precondition answered 412 but still staged and enqueued. The refused write moves person-1 to Paper and broadcasts one incremental event. The control then sends the identical body, stageUpdate finds the file already holding those bytes, and the entry reports changed: false — which, per stageTransform's comment on the same branch, "writes nothing, leaves the modification time alone and queues nothing for indexing". So it broadcasts none. events.length is 1 either way and invalidations is [person-1] either way. The only assertion that moves is refused.status, which the two tests above already pin.

Point the control at a different card and assert its invalidations do not name person-1; a leaked event from the refused write is then a second event.

One more gap while you are in here: incrementalIndexEvents filters to indexType === 'incremental', so incremental-index-initiation — emitted per written file before indexing runs, and the earliest signal a refused write had staged anything — is not counted at all by a test named "broadcasts no event".

Regression (a new test that cannot fail for the regression it names). Non-blocking, but it ships with the feature.

[habdelra]

[Claude Code 🤖] Correct, and the reasoning is exact — a byte-identical control over the same card reports changed: false, writes nothing and queues nothing, so both counts agree whether or not the refusal leaked.

Fixed in 782ba8b. The control now writes a different card, and the assertions check that the surviving event names that card and does not name the refused one — so a leaked event is a second event rather than an indistinguishable first.

Also took the second half: it now counts incremental-index-initiation as well as incremental, since that one is broadcast per written file before indexing runs and is the earliest point a staged refusal would show.

[habdelra]

[Claude Code 🤖] Please narrow with the discriminant and an explicit cast rather than the deep validator — a card document that fails this walk degrades silently instead of erring.

isSingleCardDocument validates data and every entry of included[]: each resource's relationships, and its whole meta.fields tree. Anything that fails — a foreign-realm resource fetched by loadLinks with no adoptsFrom, an included entry missing both id and lid — does not throw. It falls into the file-meta outcome and is served as a 200 card+json with no ETag, no response-cache entry, no x-created and no version. Nothing distinguishes that from the answer for a path that actually holds bytes.

The narrowing itself is load-bearing, so this is not just style: document.data.type === 'file-meta' does not narrow SingleCardDocument | SingleFileMetaDocument (nested discriminant), and reverting the line alone gives realm.ts: error TS2339: Property 'version' does not exist on type 'CardResourceMeta | FileMetaResourceResourceMeta'. But let card = document as SingleCardDocument after the discriminant test buys the same thing in O(1) and keeps the failure mode loud.

Secondary: the walk runs about 15% of the cost of the JSON.stringify it precedes on the same document — 0.43 ms vs 3.3 ms on a 529 KB doc with 50 included resources, 1.68 ms vs 11.0 ms at 2 MB / 200 included.

Regression, non-blocking.

[habdelra]

[Claude Code 🤖] Agreed, and this was not latent — it was the CI failure on the pushed commit.

media-cache-dsl-test.ts > the card+json validator rotates when the manifest changes went red with 200 and no validator. The cause is narrower than a link-target walk: seedInstanceRow seeds pristine_doc: { attributes: {} }, and isCardResource requires meta to be present, so the deep validator rejected the primary resource. The document fell into the file-meta outcome and was served without an ETag — which accounts for all three assertions, including the third, where notStrictEqual(null, null) fails because there was never a validator to rotate.

Reverted in 782ba8b: the line is back to document.data.type === 'file-meta', exactly as on main. The cast is not needed either, because the narrowing was only load-bearing for the version assignment, and that is gone. git diff origin/main -- packages/runtime-common/realm.ts now has zero deleted lines.

[habdelra]

[Claude Code 🤖] What makes the card path exempt from the check the byte path applies to this same column?

#operationStoredFileMeta, a few thousand lines up in this file, reads realm_file_meta.content_hash for the byte facade and refuses to trust it unless the row's content_size matches the handle's stat — on the stated grounds that persistFileMeta "is reached from the realm's own write path and nowhere else, so a file overwritten out of band (a deploy rsync, an operator editing the volume) keeps a row describing bytes that are gone", and that "handing that hash back would be worse than computing one". storedFileMetaFor reports the same column as the card's version with no such check.

Out-of-band overwrite is a path the server itself takes: the realm publish swap copies a source directory over the target with copy + move and touches no realm_file_meta row.

If the card path is exempt because version is advisory here rather than a validator, that belongs next to this reader — as it stands the file holds two readers of one column with opposite trust rules and only one of them explains itself.

Question. Non-blocking.

[habdelra]

[Claude Code 🤖] Fair question, and the reader is gone in 782ba8b — no card path reads content_hash any more, so the file no longer holds two readers of one column with opposite trust rules.

The question survives the removal, though, so it is recorded rather than dropped: whoever adds a read-side version inherits it, and the ticket that owns that work now says so. Your publish-swap observation is the sharp end of it — copy + move over a realm's files touches no realm_file_meta row, so the recorded hash can describe bytes that are gone, which is precisely why #operationStoredFileMeta validates against the handle's stat before trusting it.

[habdelra]

[Claude Code 🤖] Is the client meant to carry version back out, or should the host drop it where it serializes for a write?

This strip is on the onSave payload — the body the host actually PATCHes — so needing it here says version is on the wire in every interactive save. createFromSerialized puts the whole served meta on the instance, and the save serializes it back out. That contradicts the description's "host-side serializeCard() output are unaffected"; the server's stageUpdate strip is what keeps it from persisting, not the client.

Worth settling now rather than after a client starts reconciling on it: if the instance is supposed to hold version, say so; if not, drop it host-side next to generation and this hunk goes away.

Question. Non-blocking.

[habdelra]

[Claude Code 🤖] Settled by removal in 782ba8b — version is not on any card+json response, so nothing puts it on the instance and nothing echoes it back.

The question was the right one to ask and it is not lost: the ticket that adds version now carries it explicitly, including that the host round-trips a served meta into the body it PATCHes, so the choice between dropping it host-side next to generation and letting the instance hold it is made deliberately there rather than discovered by a test needing a strip.

[habdelra]

[Claude Code 🤖] @codex review

Requesting a fresh pass because the last one reviewed 68a3852, and a push does not re-trigger. The branch has changed substantially since:

• Both P1s from that review are addressed. The If-Match precondition now drains in-flight indexing before reading the index row, so it no longer rests on a source that lags the bytes — exempting prerender and skip-index-wait callers, for whom that drain would deadlock on a job needing the render slot they hold.
• meta.version is gone from card+json responses entirely, which removes the second P1's subject rather than fixing it. Nothing consumed the key, and adding it to the served read representation would have forced an ETag variant bump.
• The diff is now three files and additions only: the gate in realm.ts, a preconditionFailed helper in error.ts, and one new test file.

The area most worth a fresh look is the remaining gate-to-commit window, and whether the drain's placement is right.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9543c99fe6

ℹ️ 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".

[chatgpt-codex-connector]

Check If-Match under the commit lock

Fresh evidence in this revision is that the new drain and index lookup still finish before either handler calls commitBatch, which acquires the write lock separately. If another request updates the card after this match succeeds but before this request obtains that lock, PATCH merges onto the newer stored bytes and DELETE removes them while returning success for the stale ETag; the drain performed by commitBatch does not re-evaluate the precondition. Evaluate the validator while holding the same write lock as staging and committing the mutation.

Useful? React with 👍 / 👎.

[habdelra]

[Claude Code 🤖] Confirmed and fixed in 77826a6 — you were right, and my reply to your earlier finding under-rated this exact window as "the narrow race". It isn't narrow: because commitBatch is where the write lock is taken, a request whose check passed can queue behind another writer's entire write and then stage against state that moved. The check held only while the realm was uncontended, which is the inverse of what a conditional write is for.

CommitBatchOptions now carries a precondition that the coordinator invokes inside core.withWriteLock, after its drain and before anything stages. The realm still owns what is compared — an If-Match names an ETag built from index and realm-info state the coordinator has no business assembling — and the coordinator owns only when it runs. The refusal travels as the OperationFailure it throws, so the 412 reaches the caller by the path every other card-write refusal already takes, and a refused batch stages nothing, enqueues nothing and broadcasts nothing.

Two things worth recording beyond the fix.

The drain placement is a separate requirement from the lock. persistFileMeta runs before performIndex, so without draining first the row still spells the pre-write validator and a stale If-Match matches — a wrong accept with no concurrency in the request at all. Inside-the-lock and after-the-drain are both needed, for different reasons.

No endpoint test could have caught this. A PATCH answers the same status and the same body whether the precondition runs inside the lock, outside it, or never, so all 13 tests in the new suite passed on the broken arrangement. The two tests added in card-operations-batch-test.ts read the stub's lock depth and drain count from inside the precondition closure and assert on those instead. I verified they can fail rather than assuming it: moving the hook back outside the lock turns both readings to 0 and reddens exactly that test, while the other 81 in the file stay green.

[habdelra]

[Claude Code 🤖] @codex review

Re-requesting because a push does not re-trigger, and the fix for your last P1 is substantial enough to want a fresh pass rather than my own say-so.

Since 9543c99:

• The precondition no longer runs before commitBatch. CommitBatchOptions carries a precondition that the coordinator invokes inside core.withWriteLock, after its drain and before staging, so the compare and the write share one critical section. The realm supplies the closure, so the coordinator never assembles a card ETag.
• The refusal travels as an OperationFailure, which is how the 412 reaches the caller.
• Two tests in card-operations-batch-test.ts pin the placement by reading lock depth and drain count from inside the precondition, since a response cannot distinguish the three arrangements.

Most worth a fresh look: whether anything can still reach a stage or a commit without passing the hook, and whether throwing from inside the lock leaves the lock or any partial state behind.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 77826a65ed

ℹ️ 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".

[chatgpt-codex-connector]

Validate against cross-replica deferred writes

Fresh evidence in this revision is that the precondition still derives the validator from the index after only core.drainIndexing(), while RealmIndexUpdater.incrementalIndexing() tracks an in-memory deferred map local to one replica. If a skip-index-wait writer updates the file on replica A and a conditional PATCH or DELETE acquires the advisory lock on replica B before A's index job lands, B sees no job to drain, this query returns the old ETag, and the stale If-Match succeeds against the newer stored bytes. The comparison needs a shared freshness barrier or a validator derived from state protected by the cross-replica write lock.

Useful? React with 👍 / 👎.

[habdelra]

[Claude Code 🤖] Fixed, and apologies for the late reply — this thread went unanswered while I was working through the findings it prompted.

The precondition no longer trusts the realm's local view of in-flight indexing, which as you say is a per-process map and therefore blind to a peer replica's pending job. It queries the realm's indexing lane in the shared jobs table, which every replica writes to, and refuses rather than proceeds when that lane will not settle inside its budget.

Two corrections to the shape since, both from later review. The check lives in the precondition's own closure rather than in the shared write-path drain — riding the drain charged every card write for a guarantee only a conditional one asks for, and the drain discards the answer, which turned an unsettled lane back into the silent accept you were describing. And the lane query is scoped to the job types that can leave a row describing bytes the realm no longer stores, because asking whether the lane was occupied at all refused conditional writes for the length of any fleet-wide reindex.

[chatgpt-codex-connector]

Drain indexing before DELETE preconditions

Fresh evidence in this revision is that entries.some(stagesContent) is always false for the single DELETE submitted by removeCard, so its precondition runs without even the local indexing drain. After a same-replica x-boxel-skip-index-wait PATCH has changed the file while its index job remains pending, the old ETag still matches, and commitUnlocked drains only after this check before deleting the newer file and returning 204 instead of 412. A precondition that reads indexed state must force the drain before it runs, independently of whether the batch stages content.

Useful? React with 👍 / 👎.

[habdelra]

[Claude Code 🤖] Fixed, and likewise late — thread missed while the findings it led to were in flight.

You were right that entries.some(stagesContent) is false for the single delete a removal submits, so its precondition ran with no drain at all. It is resolved by removing the dependency rather than widening the gate: the freshness check no longer rides the shared drain, so it does not matter what the batch stages. The coordinator's drain condition is back to exactly what it was.

A removal's precondition now does its own deadlined query against the realm's indexing lane, and a stub-level test asserts that a removal runs its precondition inside the write lock with nothing committed at that point.

[habdelra]

[Claude Code 🤖] This pass covers only 9543c99..7572ee5 — the precondition's move inside commitBatch's lock, the drain's new gate, the cross-replica index gate, and the three coordinator tests. I ran card-operations-batch-test.ts standalone (it is pure-stub); the endpoint-level 412 path is read rather than executed, since the realm-server suite needs the shared test database and the suite on this head was still running.

Moving the precondition inside the lock is the right fix and the coordinator tests do pin the placement. What is not settled is what the new cross-replica gate does when it times out: as written the precondition silently stops refusing, which is the failure it exists to prevent. That, and the lock-held drain a conditional removal now takes, are what I would settle before merge.

On the two open bot threads from the previous head: both are addressed by 7572ee5. The cross-replica one — a skip-index-wait write on one replica while a conditional write takes the lock on another — is closed by the jobs-table half of drainIndexing, but only for as long as that half's budget holds; past the budget the hole reopens with no signal, which is recommendation 1. The DELETE-drain one is closed outright: I reverted the || opts.precondition !== undefined clause and only the delete-drain test went red, so it is not decoration.

1. Decide what drainIndexing does when the cross-replica gate reports false — fail closed, or proceed with a log and a comment saying the gate is advisory. See the thread on drainIndexing in realm.ts.
2. Bound, or justify, the drain a precondition forces on a batch that stages nothing — its first half has no deadline and runs with the write lock held. See the thread on the readsIndexedState gate.
3. Pin "before anything was staged" on something staging moves; the current assertion passes with the hook sitting after all of it. See the thread on the placement test.
4. Make the two index-job-type scopes one definition, or stop claiming they are. See the thread on the drainIndexing comment.
5. Guard jobTypes against an empty array. See the thread on awaitRealmIndexSettled.

Adjacent, not asked of this change: removeCard passes no waitForIndex, so a removal already awaits its own index job with the lock held and the delete path is not prerender-safe independently of anything here. Whoever makes deletes answer from an echo inherits that.

[habdelra]

[Claude Code 🤖] The gate's answer is discarded, so a lane that stays busy turns the precondition into a silent accept.

awaitRealmIndexSettled returns false when its budget (10s by default) runs out with the lane still holding unfulfilled work. This call throws that away. The realm's other caller of it — the readiness gate — reads false as "the index is knowably behind its source" and refuses to answer ready. Here the batch proceeds instead: the precondition reads the row that is still behind, builds the pre-write ETag from its indexed_at, and the client holding the validator it has already been overtaken by matches. That is the accept this header exists to refuse.

It needs no concurrency inside the request — any backlog on indexing:<realm> longer than the budget does it: a bulk import, a claim hold on the group, a worker that died with a job claimed. And nothing on this path logs it; awaitRealmIndexSettled has no logger, so a fleet that has quietly stopped refusing looks identical to one that has nothing to refuse.

Please decide the failure mode rather than inheriting it. Refusing when the gate reports false — a 503 the client retries — fails closed, which is what a conditional write asks for. If proceeding is the intended answer, it needs at minimum a warn naming the realm, and the constraint belongs in the comment above so the next reader knows the gate is advisory.

Regression, and the one I'd settle before merge.

[habdelra]

[Claude Code 🤖] Agreed, and the whole check moved as a result — fixed in 16ae09f.

Your framing is what settled it: the realm's other caller reads false as "knowably behind" and refuses, and mine threw it away, so congestion turned into exactly the accept the header exists to refuse. That it is silent made it worse than a wrong answer.

Rather than read the boolean inside the shared drain, the check left the drain entirely. It now runs in the precondition's own closure with its own budget and refuses when the lane will not settle. That fixes this and the removal-under-the-lock finding together, and leaves the shared drain untouched — riding it was charging every card write for a guarantee only a conditional one asks for.

The refusal is a 503 under a new precondition-unverifiable code rather than a 412: nothing about the caller's request is wrong, and repeating it is the remedy. It logs a warn naming the realm and the lane, for the reason you gave — a fleet that has quietly stopped refusing is otherwise indistinguishable from one with nothing to refuse.

The budget is shorter than the readiness probe's, because this one waits holding the write lock, so every other writer is queued behind it.

[habdelra]

[Claude Code 🤖] "defined against one list" is not what the code does, and the next reader will act on it.

WRITE_RACING_INDEX_JOB_TYPES is read here and nowhere else. The in-memory scope is not defined from it at all — it is whatever inserts into RealmIndexUpdater's #incrementalIndexingDeferreds, which today is enqueueChanges and copy. The two agree by coincidence of maintenance, not by construction: a third job type published into indexing:<realm>, or a new insert into that map, moves one scope and leaves the other, and this half of the drain then reports settled on a lane that is not. The constant's own declaration says "must stay in step", which is the accurate statement.

Either wire them together — have the two enqueue sites key off the constant, or assert the agreement somewhere that fails — or drop the claim here and keep the declaration's wording.

Regression in new prose, non-blocking.

[habdelra]

[Claude Code 🤖] Right, and the claim is gone with the code it described — 16ae09f reverted the drain change entirely, so there is no longer a second scope here to be in step with.

The constant survives as the precondition's own scope and its declaration keeps the "must stay in step" wording, which as you say is the accurate statement: the in-memory scope is whatever inserts into #incrementalIndexingDeferreds, and nothing enforces the agreement. Worth someone wiring the enqueue sites to the constant eventually; it is not this PR's to do now that only one caller reads it.

[habdelra]

[Claude Code 🤖] This puts an undeadlined wait inside the write lock on the one verb the paragraph directly above exempts for exactly that reason.

drainIndexing awaits incrementalIndexing() first, and that gate takes no deadline: it resolves only when every deferred this replica is holding settles, bounded per job by INCREMENTAL_INDEX_JOB_TIMEOUT_SEC — ten minutes. A conditional removal that arrives while a bulk import is draining now parks there with the realm's write lock held and every other writer to the realm queued behind it. That is the cost the paragraph above spells out as the reason a removal skips this, and it is paid inside the lock rather than in front of it.

Worth choosing one: carry a deadline into the drain a precondition forces, or force only the cross-replica half (which at least has a budget, though see the comment on drainIndexing about what it does with the answer), or state why a removal may hold the lock for the whole lane.

Separately, those two paragraphs now contradict each other a line apart — the first still says the drain is skipped for a batch that stages nothing, full stop. Worth folding into one.

Regression.

[habdelra]

[Claude Code 🤖] Fixed in 16ae09f, by removing the reason rather than bounding it.

You are right that this was the worst placement available: an undeadlined wait, inside the lock, on the one verb the paragraph above exempts for exactly that reason. incrementalIndexing() resolves only when every deferred settles, bounded per job at ten minutes, so a conditional removal arriving during a bulk import parks there with the realm's write lock held.

The forced drain is gone — the coordinator's gate is back to entries.some(stagesContent) — and the freshness a removal's precondition needs comes from its own deadlined query instead. That also resolves the contradiction you flagged between the two paragraphs: there is only the original one again.

[habdelra]

[Claude Code 🤖] This pins "before the commit", not "before anything was staged" — verified, the weaker placement passes.

The stub only touches commits inside commitUnlocked, so everything between the hook and the commit is invisible to it: readPreState, every stageEntry, the compose, and the four assert* passes. I moved opts.precondition?.() to sit immediately before commitStaged and ran this file — all 83 tests passed, this one included.

That gap matters because "a refusal costs nothing but the lock it already holds" is the property the hook's own comment claims, and it is the one a future reshuffle would break without turning anything red. The cheapest discriminator is already half-built: readPreState calls core.readSourceFile for an update's target, so a total read counter on the stub (alongside readsOutsideLock) asserted at 0 inside the precondition would fail the moved placement.

Regression in new coverage, non-blocking.

[habdelra]

[Claude Code 🤖] Confirmed and fixed in 16ae09f — and thank you for running the moved placement rather than reasoning about it, because the assertion looked adequate and was not.

The stub now counts every source read, and the test asserts that count is zero inside the precondition. readPreState is the first thing staging does, so the placement you moved it to now fails. I verified that the same way you disproved the old one: moved opts.precondition?.() to immediately before commitStaged and the run goes red on and before anything had been read to stage from, with the rest of the file green.

The delete-drain test is gone with the forced drain it covered. In its place is one asserting a removal's precondition runs inside the lock — a removal stages nothing and takes no drain, so the lock is the only thing its placement can rest on.

[habdelra]

[Claude Code 🤖] An empty jobTypes compiles to IN (), which Postgres rejects — and it throws rather than degrading, since the first hasSettled() is the one call not wrapped by recheck's catch. The option is exported, so it is a caller away.

Suggested change

	if (jobTypes) {
	if (jobTypes?.length) {

Follow-up, non-blocking.

[habdelra]

[Claude Code 🤖] Taken as suggested in 16ae09f — jobTypes?.length.

Worth noting for anyone reading later why it was worth a comment rather than a shrug: the first hasSettled() is the one call outside recheck's catch, so an empty array would not have degraded to a poll, it would have thrown out of the gate.

[habdelra]

[Claude Code 🤖] @codex review

Re-requesting, since a push does not re-trigger and both of your last findings were fixed by a restructure rather than a patch.

Since 77826a65:

• Cross-replica validation. The precondition now queries the realm's indexing lane in the shared jobs table, scoped to the job types a write races, so a peer replica's pending index job is visible to it. It refuses when the lane will not settle within its budget rather than proceeding — a 503 under a new precondition-unverifiable code, because the caller's request is not what is wrong.
• Drain before DELETE preconditions. Resolved by removing the dependency: the check no longer rides the shared drain at all, so it does not matter that a removal stages nothing. The coordinator's drain gate is back to its original form.
• The check runs in the precondition's own closure, inside the commit's lock, with a budget shorter than the readiness probe's because it waits with that lock held.

Most worth a fresh look: whether refusing on an unsettled lane can starve a realm under sustained indexing load, whether the 503 is reachable by any caller that did not send If-Match, and whether the lane query's scoping can miss a job type that does race a write.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 08c001c501

ℹ️ 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".

[habdelra]

[Claude Code 🤖] @codex review

Re-requesting, since a push does not re-trigger and the last pass's findings are now resolved on the branch.

Since 08c001c5:

• Publish reindexes. Confirmed your premise against handle-publish-realm.ts (it takes withWriteLock and enqueues from-scratch-index at line 628) and fixed it by removing the job-type filter rather than extending it. The precondition asks about the whole indexing:<realm> lane now, so it also cannot fall out of step with a job type added later. WRITE_RACING_INDEX_JOB_TYPES and the jobTypes option are gone; jobs/indexing.ts is byte-identical to main and the branch is five files.
• Failed write-index jobs. Not fixed, deliberately, with the reasoning on that thread: counting rejected as unsettled has no exit, so one terminally failed job would refuse every conditional write on the realm indefinitely. The scoped form needs the index row to record which bytes produced it, which is out of scope here and noted as the fix for the class.

Verified with a negative control: disabling the freshness gate reddens exactly one test and leaves the other thirteen green.

Most worth a fresh look: whether asking the whole lane can starve conditional writes on a realm that indexes continuously, and whether anything reaches the precondition without holding the write lock.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[habdelra]

[Claude Code 🤖] Re-review of 7572ee57c7..d654a1ad7b only — the freshness check's move into the precondition's own closure, its budget, the restored drain gate, the wedge fixture, and the two batch-test assertions. Earlier work on the branch is treated as settled, and the merge from main is out of scope, so a quiet area here means unexamined, not cleared.

Bottom line: one decision blocks. Asking the whole indexing lane makes conditional writes unavailable on a realm for as long as any job occupies it, and two scheduled fleet-wide events — the post-deployment reindex sweep and the daily scoped-css-gc cron — park one there on every realm. Everything else is non-blocking. The restructure itself checks out: jobs/indexing.ts is byte-identical to main, the coordinator is additive-only, and on Postgres the lane query does subsume the local drain the removal path lost (a locally-enqueued index job is a row in that lane until job-finalize runs, so lane-clear implies incrementalIndexing() would have resolved).

1. Scope of the lane question — decide whether a refusal window the length of a fleet reindex is acceptable, and keep scoped-css-gc out of it either way. Detail on the // The whole lane, not the job types a write races block in realm.ts.
2. Budget and poll cost — shorten CONDITIONAL_WRITE_INDEX_SETTLE_BUDGET_MS and give it a matching pollIntervalMs; withWriteLock's own pool caveat is the thing this change moves. Detail on the awaitRealmIndexSettled call.
3. precondition-unverifiable never reaches the wire — #cardWriteRefusal keeps only status/title/detail. Detail on the OperationFailure.
4. The removal's precondition test — one added line makes it pin ordering, not just the lock; mutation-verified. Detail in card-operations-batch-test.ts.
5. The wedge fixture — a cleanup that survives a partial insert, and a positive control that the 503 was the wedge's doing. Detail in card-conditional-write-test.ts.

On the open thread about a rejected index job: the "no exit" half holds and I verified it. The "no bounded form is answerable" half is where I'd push back — replied on that thread.

The PR description is now stale in the paragraph a reviewer most needs. It still says the lane query is "scoped to the job types a write races — from-scratch-index is excluded, since waiting on one would hold this lock for as long as a system-wide reindex takes", which is the opposite of what ships and describes away the exact window recommendation 1 is about.

CI is mid-run on the head commit and the realm-server shards have not reported; the sticky results comment is still for an earlier commit, so the 503 test has never run in CI. Nothing red to act on yet.

Adjacent, out of scope: awaitRealmIndexSettled returns true for a non-pg adapter, and the coordinator skips its drain for a batch that stages nothing, so on a SQLite-backed realm a conditional removal has no freshness check at all. Only reachable in the in-browser realm, where nothing sends If-Match today — noted for whoever gets there first, not asked of this change.

[habdelra]

[Claude Code 🤖] Asking the whole lane makes conditional writes unavailable on a realm for as long as anything occupies indexing:<realm> — and two routine, fleet-wide events park a job there for minutes to hours. Please decide whether that window is the trade you want; as it stands it is not an edge case but a scheduled one.

The window is the job's whole life, not its queue wait. hasSettled() counts status = 'unfulfilled', and claiming a job does not move the status — pg-queue.ts inserts a job_reservations row and leaves status alone, and job-finalize.ts is what finally sets it. So a running pass keeps the lane unsettled for its entire duration. (This is the same property wedgeIndexingLane relies on.)

Two ways that happens on a schedule:

• handle-post-deployment.ts publishes a full-reindex whenever the boxel-ui checksum moves, and tasks/full-reindex.ts loops enqueueReindexRealmJob over every eligible realm — one from-scratch-index into each realm's indexing: group. After a UI-affecting deploy, therefore, every realm has one queued or running, and every conditional write on every realm takes the realm write lock, waits the budget, and 503s until the sweep's tail clears. jobs/indexing.ts's own note on systemInitiatedIndexPriority puts that tail at over an hour.
• scoped-css-gc shares the group too — scripts/scoped-css-gc.ts publishes with concurrencyGroup: indexingConcurrencyGroup(realm_url), one job per realm, from a daily cron. It deletes unreferenced scoped_css rows and cannot move any card's indexed_at. So "Any job in the lane means the same thing here — that something is on its way to changing what the index says" is not true of it, and a GC sweep refuses conditional writes realm-wide for no freshness reason at all.

The way out. If what this closes is the republish hazard, the question that answers it is "is there a job here that can move an index row", which is narrower than "is the lane occupied". The fail-closed property you wanted from dropping the filter survives if the scope is expressed as a deny-list of job types that provably cannot move an index row (scoped-css-gc today) rather than an allow-list of ones that can — a type added later then still refuses. That leaves the from-scratch window, which is the decision: either accept a refusal window the length of a fleet reindex, or find a marker for the republish case specifically (the swap happens under the write lock, so what a conditional write can interleave with is only the gap between lock release and that realm's reindex completing).

Class: regression, introduced by d654a1ad. Blocking as a decision rather than necessarily as a code change.

[habdelra]

[Claude Code 🤖] Resolved in 3af0cbba, and you were right that it was not an edge case — the full-reindex fan-out alone made it a scheduled realm-wide outage for conditional writes.

The fix is not a deny-list, because checking the premise removed the need for one. The republish hazard that made me widen the scope is unreachable. handle-publish-realm.ts creates a published realm with [newUserId]: ['read','realm-owner'], [ownerUserId]: ['read','realm-owner'], '*': ['read'] — write to nobody — and RealmPermissionChecker.can is a plain includes(action), so realm-owner does not imply write. No conditional PATCH or DELETE can reach a published realm at all; it is refused at the permission check long before a precondition runs. The write check at line 157 of that handler is on the source realm, which is what you need write on in order to publish.

So from-scratch-index can be excluded without reopening anything, and scoped-css-gc falls out for free.

What I changed beyond reverting: the list is now derived rather than inherited, which is the mistake underneath both of my wrong answers here. It is CONTENT_MOVING_INDEX_JOB_TYPES, and its definition is "what can leave a row describing bytes the realm no longer stores" — only a write moves a card's file, and a write's indexing is incremental or copy. A from-scratch pass re-derives rows from files nobody changed; it moves indexed_at without moving content. The first time I took this list from the drain, which answers a different question ("should I wait"), and that is exactly how it came to disagree with itself on from-scratch.

Also took the budget point from the neighbouring comment: 3s → 1s with a matching 250ms poll, since the wait is held under the write lock and each poll takes a pool client while the lock already pins one.

[habdelra]

[Claude Code 🤖] Please shorten this budget and pass a pollIntervalMs to match it — 3s of lock hold buys almost nothing, and the polling inside it costs pool clients the lock holder is already competing for.

The pool. PgAdapter.withWriteLock pins one client for the whole critical section, and its own note names the condition this change moves: "if the pool ceiling is less than realistic write concurrency, callbacks that need additional pool clients could deadlock waiting on the pool. For current scope (low realistic write concurrency, pool size >= concurrent writers + headroom) this is acceptable." That was written for a callback that does a handful of quick queries. A refused write now takes: the initial hasSettled(), three setInterval ticks at the 1000ms default inside a 3000ms budget, the recheck fired when subscribe resolves — and one more per NOTIFY jobs_finished, which is a global unpayloaded channel (job-finalize.ts, pg-queue.ts, lib/mark-failed-job.ts all fire it), so that last count tracks fleet-wide job throughput rather than this realm's. Every one of those is a pool checkout taken while this caller already pins one.

The value. 3s only catches a job that was already about to finish: INCREMENTAL_INDEX_JOB_TIMEOUT_SEC is 10 minutes and a from-scratch pass is longer, and the NOTIFY subscription already makes the wakeup prompt rather than poll-bound. A few hundred ms catches nearly the same set of "about to finish" cases at a fraction of the hold. As written the interval is also doing 1s-granularity work inside a 3s window, which is three rechecks to resolve something the subscription would have delivered.

The amplification. The 503 invites a retry, and the retry re-takes the realm write lock for another full budget. During an unsettled window a client retrying in a loop starves the realm's unconditional writers — which inverts the reason the drain was carefully skipped for batches that stage nothing a few lines up in coordinator.ts.

Class: pre-existing caveat in withWriteLock, now load-bearing. Non-blocking.

[habdelra]

[Claude Code 🤖] Taken in 3af0cbba — 3s → 1s, with pollIntervalMs at 250ms passed explicitly rather than left at the 1000ms default.

Your value argument is the one that decided it: the jobs this waits on are bounded in minutes, so anything that would finish inside a wait of this size was finishing regardless, and the jobs_finished subscription is what delivers the wakeup. The poll is a backstop and should be sized to the budget rather than firing three times inside it.

The retry-amplification point is now much smaller too, since the gate is scoped to jobs that move bytes — a realm has to be actively taking writes for it to trigger at all, rather than any indexing activity anywhere on the box.

I have left the withWriteLock caveat itself alone. It was accurate before this change and is accurate after; what this PR does is make one more caller of it wait, which is the thing to keep an eye on rather than something to fix here.

[habdelra]

[Claude Code 🤖] This code never reaches a client, so either surface it or drop the claim that it does.

#cardWriteRefusal destructures { status, title, detail } and nothing else. A 503 takes its status >= 500 branch into systemError, whose options carry no code — nor do CardError's. The card+json PATCH and DELETE handlers are the only two places this precondition is installed, so precondition-unverifiable is unobservable on the wire: a caller sees a 503 and a sentence, indistinguishable from any other realm 5xx, which is exactly the distinction the new code exists to make. The description says the wire says "could not decide" rather than implying a conflict — as it stands it says neither.

Either plumb code through #cardWriteRefusal onto the response body, or keep the code as the internal taxonomy it actually is and say so in types.ts and the description. Worth settling now rather than after a client is written against a field that isn't there.

Class: regression. Non-blocking.

[habdelra]

[Claude Code 🤖] You are right that it does not reach the wire, and I have taken the second option — documented it as the internal taxonomy it actually is, in types.ts, and corrected the description.

#cardWriteRefusal carries status, title and detail; a 503 takes the status >= 500 branch into systemError, and neither that nor CardError has a code. So on the card verbs a caller distinguishes this refusal by its detail, which names the card and says the realm could not establish that its index is current. The code is observable where an operation result carries its own error, which is the envelope.

Plumbing code through systemError and CardError would touch the shared error path for every realm 5xx, which is more than this PR should carry to surface a field no client reads yet. Better done by whichever ticket first has a client that wants to branch on it.

[habdelra]

[Claude Code 🤖] Please also record s.commits.length here and assert it is 0 — as written this test pins that the removal's precondition is inside the lock, but not that it runs before the removal lands, and nothing else in the file covers that for a removal.

Checked by mutation against the stub-only suite (83 tests / 191 assertions, no DB):

• hook moved outside withWriteLock → this test reddens (lock depth 0). Not vacuous for the lock.
• hook moved past commitStaged, still inside the lock → this test stays green. The test that does catch it, a precondition that refuses writes nothing and commits nothing, drives an update entry, so the removal path has no equivalent.

That second mutation is the one that matters for a removal: a precondition evaluated after the file is gone refuses a delete that already happened. Adding

commitsWhenChecked = s.commits.length;

in the closure and assert.strictEqual(commitsWhenChecked, 0, …) below reddens it — I ran that exact addition against the post-commit mutation and it fails as intended, and the tree is back as I found it.

Class: coverage gap in a test added here. Non-blocking.

[habdelra]

[Claude Code 🤖] Fixed in 3af0cbba — the removal's precondition now also records s.commits.length and asserts it is 0.

Your mutation is the right one and I had not run it: a removal reads nothing to stage from, so the source-read counter that discriminates the update case does nothing here, and lock depth alone is satisfied by a hook anywhere inside the lock including after the file is gone. The commit count is the only thing that places it ahead of the removal itself.

[habdelra]

[Claude Code 🤖] Two asks on the fixture.

Move this call inside the try. wedgeIndexingLane does two inserts and only returns the cleanup after the second. A throw between them — a constraint, a disconnect — leaves behind exactly the bare unfulfilled job with '{}' args that the fixture's own comment identifies as the thing the worker claims, fails on, and poisons the realm's next index job with. Nothing then removes it, and every later If-Match test in this file waits the full budget and 503s, with the failure surfacing somewhere other than here. Registering the cleanup as soon as the jobs row exists (or wrapping the whole fixture call) makes the cleanup total rather than dependent on both inserts landing.

Add a positive control. Nothing here shows the 503 came from the wedge rather than from the realm happening to be indexing anyway — the assertion holds either way, which is the shape that passes while the premise was never established. Re-issuing the same conditional PATCH after unwedge() and asserting it succeeds pins both halves at once: that the wedge is what caused the refusal, and that the cleanup restored the lane for the tests that follow. The ETag is still current at that point, since the refused write changed nothing.

I could not run this file — the realm-server lane is held elsewhere on this machine, so the 503 test is unverified here, and CI has not reported on the head commit yet.

Class: test robustness. Non-blocking.

[habdelra]

[Claude Code 🤖] Both taken in 3af0cbba.

wedgeIndexingLane now builds its own cleanup before the reservation insert and calls it from a catch if that insert throws, so the half-built wedge cannot survive. That mattered more than a normal leak: what is left behind is exactly the bare unfulfilled job the fixture exists to avoid — the one a worker claims and dies on — so every later conditional write in the file would have waited out its budget and 503'd, which reads as a product failure rather than a leaked fixture.

The positive control is in: after unwedge(), the same conditional PATCH is re-issued unchanged and asserted to succeed. Without it a 503 from any other cause would have read as this one's.

[backspace]

[Claude Code 🤖] Convergence pass over the head commit: the five threads open on the previous head, plus the two commits since — the lane-scope narrowing and the validator-shape enumeration. I re-derived the narrowing's premises rather than checking them off; the earlier history of the branch is treated as settled, so a quiet area there means unexamined.

No blocking issues. The narrowing holds — its three load-bearing premises check out — and what is left is a stale description and one missing test.

Dispositions:

1. Lane scope — resolved, and the premises hold. The two names in CONTENT_MOVING_INDEX_JOB_TYPES are the strings the enqueue sites actually publish (realm-index-updater.ts publishes incremental-index and copy-index; reindex-realm.ts publishes from-scratch-index), so the filter matches rather than silently never matching. Excluding a from-scratch pass is safe for the republish case it was widened for: a published realm's rows are ['read','realm-owner'] per user and '*': ['read'], and RealmPermissionChecker.can is a plain includes, so realm-owner carries no write and no conditional write reaches one. And the added job_type IN (…) still rides jobs_unfulfilled_concurrency_group_idx — it filters live-queue rows rather than reintroducing a scan under the write lock.
2. Budget and poll cost — resolved; 1 s with a matching 250 ms poll.
3. precondition-unverifiable never reaches the wire — resolved in the code comment; the description half is recommendation 1.
4. The removal's placement test — resolved; the commit count is what places the hook ahead of the removal.
5. The wedge fixture — resolved; the cleanup is built before the reservation insert and the control re-issues the same request after unwedge(). The fixture still inserts an incremental-index row, so it exercises the narrowed filter rather than falling outside it.

Recommendations, none blocking:

1. The description is stale in four places, and one of them argues for the design that was reverted. It still says the check "asks about the whole lane rather than the job types a write races", and reasons from the republish case that from-scratch-index is "one of the strongest reasons the index cannot be believed, not one to exclude". The code excludes it, and a reader who restores what the description specifies reintroduces the fleet-wide refusal window the narrowing removed. Three smaller ones alongside it: "three files, no deletions" (six files, and jobs/indexing.ts and realm.ts both carry modified lines); "under a new precondition-unverifiable code, so the wire says 'could not decide'" (it does not, on the card verbs); and "Either link-shape variant matches" (three shapes now, times the assembly split).
2. Nothing pins the write-echo validator — thread on the links-only test in card-conditional-write-test.ts.
3. The enumeration covers the shape but not the budget number — thread on issued in realm.ts.

CI is green on the head commit, and the new suite is not silently skipped: shard assignment is filesystem-derived and puts card-conditional-write-test.ts on realm-server shard 3, which passed.

Adjacent, not asked of this change: because the realm issues no validator for an error row or a document with foreign-realm deps (peekEtag is built only for type === 'instance'), a client that always sends If-Match has no way to repair either kind of card — it has to drop the header to get the write through. Worth a sentence wherever the client contract for this eventually gets written.

[backspace]

[Claude Code 🤖] Please add the write-echo twin of this test: take the ETag off a PATCH response and send it back as If-Match on the next one.

Every validator in this file comes from a GET — each site is read.get('etag') — so nothing here exercises the shape a write response hands out, which is the one a client doing consecutive conditional edits holds, the echo being the last thing it read. card-endpoints-test.ts pins that echo's spelling, not that the gate accepts it, so the two files together still leave the path uncovered, and the comparison's coverage of that shape rests on reading the enumeration rather than on anything red.

Four lines, in the shape this module already uses: PATCH with no If-Match, response.get('etag'), PATCH again carrying it, assert 200.

Class: coverage gap for a change made in this PR. Non-blocking.

[habdelra]

[Claude Code 🤖] Added, in the shape you described — PATCH with no If-Match, take response.get('etag'), PATCH again carrying it, assert 200 and that the second edit landed.

You are right that the gap was structural rather than incidental: every validator in this file came from a GET, so the shape a client doing consecutive conditional edits actually holds — the echo from its own last write — was the one shape with no test behind it. And it was the shape most recently added to the comparison, so its coverage rested entirely on reading the enumeration. card-endpoints-test.ts pinning the echo's spelling is not the same claim as the gate accepting it.

[backspace]

[Claude Code 🤖] The enumeration covers the full shape's exemption flag but not the number folded into its bounded variant: cardJsonEtagVariant() interpolates assembledLinkResourceBudget(), so only the current budget's validator is built here.

That value is read from the environment at module load, so today it is uniform across a deployment and this is reachable only across a rolling deploy that changes it — a caller whose read landed on an old replica is refused by a new one. The description says the setting is on its way to varying per request with load, and at that point every conditional write is deciding partly on a server setting, which is the failure the comment above says the enumeration avoids.

So either narrow the claim where it says a dimension added to the builder is covered here the day it ships — a value dimension is not — or keep the number out of the validator when the budget starts varying. A decision rather than a fix.

Class: follow-up. Non-blocking.

[habdelra]

[Claude Code 🤖] Correct, and the claim is narrowed rather than the code changed — because this one cannot be enumerated.

cardJsonEtagVariant() interpolates assembledLinkResourceBudget(), so the bounded full spelling carries a number, and the comparison builds only the number this process is running with. Enumerating the builder's arguments covers a further enumerable argument; it cannot cover a value, because the set of values it could have taken is not knowable from here.

The comment now says that explicitly: what the product covers, that the budget is a value folded into a variant and therefore outside it, that it is uniform across a deployment today because the value is read at module load, and that the reachable case is a rolling deploy that retunes it — a validator issued by an old replica refused by a new one. It also records the consequence you name: if that budget starts varying per request, the number has to come out of the validator, or every conditional write begins deciding partly on a server setting.

Leaving it as a stated limit rather than a fix, since the fix belongs with whoever makes the budget vary — it is their change that turns this from a deploy-window edge into a per-request one.