CS-11167: HTTP DELETE card-source returns 204 without awaiting indexing#4857
Merged
Conversation
The DELETE handler for application/vnd.card+source was awaiting the incremental index job's settle before returning its 204. Under concurrent realm load, that job could sit in the worker queue for seconds — long enough that a caller with a normal 30s timeout would give up before the response arrived. POST card-source already returns without awaiting indexing (waitForIndex:false); DELETE now mirrors that contract. Realm.delete gains an optional waitForIndex flag (default true, preserving every existing internal/test caller). The HTTP handler passes waitForIndex:false; on that path the durable enqueue is still awaited inline (so a DB partial outage surfaces to the client), but the worker settle is fire-and-forget, with the post-worker broadcast routed through enqueueIndexUpdateAndCollectInvalidations' onSettled so realm.incrementalIndexing() doesn't resolve before the broadcast. Source-content-type callers don't depend on indexed state — if they did they'd use application/vnd.card+json. Subscribers that care about the deletion landing in the index still see the existing incremental-invalidation realm event once the worker settles. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 79cafc42cb
ℹ️ 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".
When DELETE card-source returns 204 without awaiting the index update, emit an info-level line once the worker settles with the elapsed time from enqueue. Pairs the existing error log on rejection — gives an operational signal that the async work ran and how long it took, so indexer back-pressure shows up in the logs at a glance instead of having to be inferred from request-trace gaps. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aligns the DELETE handler for application/vnd.card+source with the existing POST behavior by returning the HTTP 204 response without waiting for incremental indexing to complete, avoiding client timeouts when the indexing worker queue is backed up.
Changes:
- Add an optional
{ waitForIndex?: boolean }parameter toRealm.delete()(defaulting totrueto preserve current non-HTTP callers’ behavior). - For
waitForIndex:false, await only the durable enqueue of the incremental index job and run the invalidation broadcast via the deferredonSettledhook (fire-and-forget the worker settle). - Update the HTTP
removeCardSourcehandler to calldelete(..., { waitForIndex: false })so it can return 204 promptly.
Comments suppressed due to low confidence (1)
packages/runtime-common/realm.ts:3499
- This comment says the DELETE +source handler returns "as soon as the file is gone from disk", but the implementation still awaits additional work (e.g. removeFileMeta and durable index-job enqueue) before returning. Consider rewording to reflect the actual contract (e.g. returns once the delete is durable and indexing is enqueued, not once the worker finishes).
this.#logRequestPerformance(request, start, 'cache miss');
}
}
private async removeCardSource(
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address review feedback on PR #4857: * Log message now reports `url.href` rather than concatenating `this.url` + `path`, so file paths with characters that would be URL-encoded (or future changes to the realm URL shape) cannot produce a misleading line. The error branch now includes the Error's stack when present. * New host integration test asserts the HTTP DELETE `application/vnd.card+source` contract: the 204 response and disk removal land synchronously, while `realm.incrementalIndexing()` remains non-undefined until the deferred work drains, and the index reflects the deletion after the drain. Paired with a default `realm.delete()` test that confirms the inline `waitForIndex:true` semantics still hold for non-HTTP callers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Preview deploymentsHost Test Results 1 files 1 suites 3h 29m 16s ⏱️ Results for commit fe56917. Realm Server Test Results 1 files ± 0 1 suites +1 8m 32s ⏱️ + 8m 32s Results for commit fe56917. ± Comparison against earlier commit 1dedb0a. |
lukemelia
approved these changes
May 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
DELETEhandler forapplication/vnd.card+sourcewas awaiting the incremental index job'ssettledbefore returning its 204. Under concurrent realm load the job could sit in the worker queue for tens of seconds, long enough that callers with a normal 30s timeout would give up before the response arrived — the proximate cause of CS-11134 (flakyre-pushes a file that was deleted on the realm out-of-bandtest).POSTcard-source already returns immediately viawaitForIndex:false;DELETEnow mirrors that contract.Realm.deletegains an optionalwaitForIndexflag (defaulttrue, preserving every existing direct caller — two indexing-test cases and the unpublish path). The HTTPremoveCardSourcehandler passeswaitForIndex:false.waitForIndex:falsebranch, the durable enqueue is still awaited inline so DB-side failures surface to the HTTP client; only the worker settle is fire-and-forget. The post-worker broadcast is routed throughenqueueIndexUpdateAndCollectInvalidations'sonSettled, sorealm.incrementalIndexing()does not resolve before the broadcast (same shape as the write path).Test plan
packages/boxel-cliintegration tests pass, including the previously-flakyre-pushes a file that was deleted on the realm out-of-band(3 consecutive runs, ~2s for that test vs. 30s timeout before).--filter "deleted"— 10/10 pass, includingcan incrementally index deleted instance/can incrementally index instance that depends on deleted card source(directrealm.delete()callers that still use the defaultwaitForIndex:true) and the JSON-API card DELETE.Closes CS-11167. Supersedes CS-11134 (cancelled as duplicate).
🤖 Generated with Claude Code