Repository navigation
feat: index public-realm deploys and system reindexes without realm events - #6914
Conversation
Asynchronous indexing that a user's own session didn't ask for must not reset that session's loader. A deploy of a public realm's content, the post-deploy boxel-ui reindex, Grafana reindexes, the nightly OpenRouter sync and the boot reindex of bootstrap realms now index silently; open tabs pick the result up on their next load. - from-scratch args carry `realmEvents: false` (OR-merged on coalesce; an announcing pass never rides a silent in-flight one), which turns off the render job's prerender_html announcement and follow-up incremental events - the full-reindex job and enqueueReindexRealmJob thread it through - startReindex skips its invalidation and `full` broadcasts when silent, and a .boxelignore write sent with X-Boxel-Realm-Events: none starts a silent reindex - /_grafana-reindex, /_grafana-full-reindex and /_post-deployment enqueue silently - cardService.executeAtomicOperations takes `realmEvents: false`; the OpenRouter sync tool passes it - `boxel file write --no-realm-events` Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
habdelra
left a comment
habdelra
left a comment
There was a problem hiding this comment.
[Claude Code 🤖] This review covers this PR's own diff, not the header mechanics in its base branch. It checked three things: that every system-initiated index path goes silent, that no user-initiated path does, and that the coalescing keeps the two apart. It doesn't cover performance. I didn't run the realm-server or boxel-cli suites, because the test lane is shared, so every claim about a test below comes from reading the code.
No blocking issues. One design gap undoes the goal for a short window: a full-reindex sweep merges its realm flag across every realm in the union, so a single unarchive during a deploy makes the whole post-deploy sweep announce. The new atomic-endpoints test can't observe the worker-sent events it claims to cover.
- Merge
full-reindexsweeps only when theirrealmEventsflags match. See the thread onchooseFullReindexCoalesceDecision. - Pin the render job's
announce: falseand the silent follow-up args. The test worker has noreportRealmEvent, so the ignore-file test can't seeprerender_html. See the thread inatomic-endpoints-test.ts. - The boot-path
realmEventsexpression reduces toisNewIndex, and the "during or after" caveat beside it is out of date. See the thread inrealm.ts. - Update the comment on
#reindexForIgnoreRules(suggestion inline). - The
--no-realm-eventsCLI test passes whether or not the flag reacheswrite(). See the thread infile-write.test.ts. - Update the comment on the
fullbranch ofhandleInvalidationsinpackages/host/app/services/store.ts. It says a realm "reindexing at startup announces itself with this event alone", and after this change a bootstrap realm reindexing an existing index announces nothing. The behaviour of awaiting-index cards doesn't change: before this change, the bootfullevent for an existing index went out before the unawaited pass finished, so it never woke them. Non-blocking. - No host test checks that
executeAtomicOperations(…, { realmEvents: false })sends the header.sync-openrouter-models-testcould catch the_atomicrequest and assert it. Non-blocking.
Adjacent, not for this PR: the prerender-html-reconcile cron repairs stale rows with an announcing prerender_html job, because it never sets announce. If a silent from-scratch render fails or times out on a deploy, prerender_html events still reach that realm's tabs on the cron's next tick. The cron is system-initiated and its behaviour isn't new. It would make a good follow-up, carrying announce: false for repairs.
| realmUrls: merged, | ||
| } as JSONTypes.Object; | ||
| // One sweep that announces is enough for the merged sweep to announce. | ||
| let announces = announcesSweep(twin.args) || announcesSweep(incoming.args); |
There was a problem hiding this comment.
[Claude Code 🤖] This OR-merge runs over the union of both sweeps' realm lists, so one realm's announcing sweep makes every realm's pass announce. handle-unarchive-realm.ts publishes a full-reindex for a single realm with no realmEvents. If that lands while the post-deploy sweep is still pending, the merged sweep announces for every realm on the server, which is the storm this PR is meant to stop. The same happens in the other order. The pending window is real: the sweep waits at systemInitiatedPriority behind the bootstrap realms' boot reindexes, which is exactly when a deploy runs.
The per-realm from-scratch coalesce already OR-merges, and it does so at the right granularity, so this merge doesn't have to. Join only a twin whose flag matches, and insert otherwise:
let twin = candidates.find(
(candidate) =>
candidate.jobType === incoming.jobType &&
announcesSweep(candidate.args) === announcesSweep(incoming.args),
);full-reindex-group runs the two sweeps one after the other. The second sweep's pass for the unarchived realm then OR-merges into that realm's pending pass and nowhere else. The queue-test case "a sweep that announces makes the merged sweep announce" would change to expect two pending sweeps.
Regression in this PR. Non-blocking, but I'd fix it here, because it reopens the path the PR closes.
There was a problem hiding this comment.
[Claude Code 🤖] Fixed in 5525720. A full-reindex sweep now merges only with a pending sweep that makes the same choice about announcing; otherwise it inserts its own row. An unarchive's announcing sweep and a pending silent post-deploy sweep stay separate, and each realm's passes keep their own choice. The queue-test case checks that two silent sweeps merge and an announcing one doesn't join them.
| test('an ignore-file write sent with X-Boxel-Realm-Events: none reindexes the realm without any realm event', async function (assert) { | ||
| let auth = `Bearer ${createJWT(testRealm, 'user', ['read', 'write'])}`; | ||
| let writeIgnoreFile = async ( | ||
| content: string, | ||
| headers: Record<string, string>, | ||
| ) => { | ||
| let pending = request | ||
| .post('/.boxelignore') | ||
| .set('Accept', SupportedMimeType.CardSource) | ||
| .set('Content-Type', 'text/plain;charset=UTF-8') | ||
| .set('Authorization', auth); | ||
| for (let [name, value] of Object.entries(headers)) { | ||
| pending = pending.set(name, value); | ||
| } | ||
| let response = await pending.send(content); | ||
| assert.strictEqual(response.status, 204); | ||
| }; | ||
|
|
||
| let events = recordRealmEvents(); | ||
| await writeIgnoreFile('quiet-ignored.json\n', { | ||
| 'X-Boxel-Realm-Events': 'none', | ||
| }); | ||
| // The write's own pass, then the from-scratch pass it starts. | ||
| await testRealm.indexing(); | ||
| await testRealm.indexing(); | ||
| // The reindex announces right after its pass settles. | ||
| await new Promise((resolve) => setTimeout(resolve, 250)); | ||
| assert.deepEqual( | ||
| eventKinds(events), | ||
| [], | ||
| 'neither the write nor the reindex it started sent a realm event', | ||
| ); |
There was a problem hiding this comment.
[Claude Code 🤖] This test can't see the two broadcasts a silent from-scratch pass is supposed to withhold. The worker sends prerender_html and the follow-up incremental event through reportRealmEvent. The test worker that tests/helpers/index.ts builds (new Worker({...})) has no reportRealmEvent, so those events are dropped whatever the args say. The only gate the test pins is startReindex's own full/invalidation broadcast. If you revert announce: fromScratchAnnounces(args) or the followUpIncrementalArgs(…, fromScratchAnnounces(args)) change in tasks/indexer.ts, this test stays green. That follows from how the worker is built; I haven't run it, because the test lane is shared.
The cheapest way to pin the render job: once the silent reindex settles, read the prerender_html row it spawned for this realm from jobs and assert args.announce === false, then assert true for the write without the header. full-reindex-test reads realmEvents off a job row the same way. For the follow-up args you need a commit-validation scenario with a worker that records events, and index-commit-validation-test.ts already builds one. The description's test plan says this test covers "no prerender_html event"; please correct that too.
Coverage gap in this PR. Non-blocking.
There was a problem hiding this comment.
[Claude Code 🤖] Right, the test worker has no reportRealmEvent. Fixed in 5525720. The test now reads the jobs the silent write spawned and asserts that the from-scratch-index job carries realmEvents: false and every spawned prerender_html job carries announce: false. Those are what the worker's broadcasts read. I'll correct the description's test plan too.
| // tabs open on the realm pick the new content up on their next load | ||
| // rather than reset their loader mid-session (see | ||
| // `fromScratchAnnounces`). A brand-new index still announces. | ||
| let realmEvents = isNewIndex || !this.#fullIndexOnStartup; |
There was a problem hiding this comment.
[Claude Code 🤖] Inside this branch, isNewIndex || !this.#fullIndexOnStartup always equals isNewIndex, because the branch is only entered when one of the two holds. The second operand suggests a case that never reaches this line, a realm with an existing index that announces at boot. The caveat under if (realmEvents) that the event "may happen during or after the full index" is also out of date: the event now fires only on the new-index path, where the pass is awaited first, so it always comes after the pass. Please simplify this line and drop that caveat.
| let realmEvents = isNewIndex || !this.#fullIndexOnStartup; | |
| let realmEvents = isNewIndex; |
Non-blocking.
There was a problem hiding this comment.
[Claude Code 🤖] Simplified in 5525720. The boot path is now realmEvents = isNewIndex, and the full event is sent inside the isNewIndex block after the awaited pass, so the 'during or after' caveat is gone.
| // A changed root ignore file: the from-scratch pass rereads the rules and | ||
| // broadcasts what it hid or revealed, and the cache wipe after it tells the | ||
| // peer replicas to reread the rules too. |
There was a problem hiding this comment.
[Claude Code 🤖] This comment still says the pass always broadcasts what it hid or revealed. With opts threaded through, it broadcasts only if the write that changed the rules did.
| // A changed root ignore file: the from-scratch pass rereads the rules and | |
| // broadcasts what it hid or revealed, and the cache wipe after it tells the | |
| // peer replicas to reread the rules too. | |
| // A changed root ignore file: the from-scratch pass rereads the rules and, | |
| // unless the write that changed them asked for no realm events, broadcasts | |
| // what it hid or revealed. The cache wipe after it tells the peer replicas | |
| // to reread the rules too. |
Non-blocking.
There was a problem hiding this comment.
[Claude Code 🤖] Reworded in 5525720. The pass broadcasts what it hid or revealed only when the write that changed the rules announces itself, and the cache wipe for peer replicas happens either way.
| it('--no-realm-events writes the file', async () => { | ||
| let res = await runBoxel( | ||
| [ | ||
| 'file', | ||
| 'write', | ||
| 'flagged.gts', | ||
| '--realm', | ||
| realmUrl, | ||
| '--no-realm-events', | ||
| ], | ||
| { home, input: 'export const flagged = 1;' }, | ||
| ); | ||
| expect(res.ok, res.stderr).toBe(true); | ||
| expect(await (await readBack('flagged.gts')).text()).toContain('flagged'); | ||
| }); |
There was a problem hiding this comment.
[Claude Code 🤖] This test still passes if realmEvents: opts.realmEvents is removed from registerWriteCommand, so nothing covers the flag reaching write(). The test only shows that commander accepts --no-realm-events. runBoxel runs a subprocess, so the spy from the test above can't reach it. Two options: move the CLI-options-to-WriteCommandOptions mapping into a small function and test it in-process, or rename this test to say what it actually checks.
Coverage gap. Non-blocking.
There was a problem hiding this comment.
[Claude Code 🤖] Renamed in 5525720 to 'accepts --no-realm-events and writes the file', with a note that the header is checked in-process by the test above it. That in-process test covers write() with realmEvents: false for text and bytes. The commander action's one-line mapping is left to the type checker.
…oadcasts-one-realm-event-per-file-so' into cs-14191-silent-public-realm-deploys
Preview deploymentsHost Test Results 1 files ± 0 1 suites ±0 1h 48m 12s ⏱️ - 2m 10s Results for commit 064db81. ± Comparison against earlier commit 1bc0a02. Realm Server Test Results 1 files ±0 361 suites ±0 2h 1m 2s ⏱️ + 16m 3s Results for commit 064db81. ± Comparison against earlier commit 1bc0a02. |
- full-reindex sweeps merge only with sweeps that make the same choice about announcing, so an unarchive's announcing sweep can't make a pending post-deploy sweep announce for every realm - the boot index announces only a brand-new index (and only after it lands); comments on the ignore-file reindex and the host's `full` handling say when they announce - the .boxelignore test checks that the spawned reindex and render jobs carry no announcement, since the test worker reports no realm events - the CLI flag test is named for what it checks Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
[Claude Code 🤖] On the review body: the |
…oadcasts-one-realm-event-per-file-so' into cs-14191-silent-public-realm-deploys
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…oadcasts-one-realm-event-per-file-so' into cs-14191-silent-public-realm-deploys
|
[Claude Code 🤖] Reopening. This PR doesn't depend on the shared realm event room that #6906 proposed, which was closed under ADR 011 (#6921). It only makes system-initiated indexing (public-realm deploys and syncs, the post-deploy boxel-ui reindex, Grafana reindexes, the OpenRouter sync) run without realm events, so open tabs aren't reset by indexing their users didn't ask for. That holds whichever way public-realm events are delivered, and it cuts the load on Synapse under any of the alternatives. Hassan wants it to land, and it was already approved. Reducing fan-out for public realms is being redesigned separately. |
Important
Merge order: #6903 (
X-Boxel-Realm-Events: none,realm push --no-realm-events) has merged; this PR targetsmain.What this changes
Indexing that a user's own session didn't ask for must not reset that session's loader. When a realm event reaches an open tab, the tab reloads what changed, so a deploy of a public realm, or an operator's reindex of every realm, makes every tab with that realm open reset under its user. This PR makes those system-initiated paths index silently. The index, the byte caches and the peer replicas are updated exactly as before; only the Matrix broadcast to hosts is withheld, and open tabs pick the new content up on their next load. Indexing a user starts — their writes, a publish, a
/_reindexthey call — still announces.boxel realm push,boxel file write)realm push --no-realm-eventsalready sends the header;file write --no-realm-eventsnow does too. The workflows adopt the flags separately..boxelignore/.gitignorewrite sent with the headerfullevent, noprerender_htmlevent.cardService.executeAtomicOperations(…, { realmEvents: false })sends the header; the sync tool always passes it./_post-deploymentboxel-ui full reindexfull-reindexsweep withrealmEvents: false./_grafana-full-reindex) and per-realm/_grafana-reindexMechanics
fromScratchAnnounces(args)reads a from-scratch job'srealmEventskey. The key is carried only when false (FromScratchArgshas a JSON index signature, so it can't declare an optional field), and absent reads as announcing. A silent pass enqueues its render job withannounce: falseand its follow-up incremental passes withrealmEvents: false; those mechanisms come from the base branch.full-reindexsweep merges only with a sweep that makes the same choice, so one realm's announcing sweep (an unarchive, say) never makes every realm in a silent sweep announce.enqueueReindexRealmJob,RealmIndexUpdater.publishFullIndex/fullIndexandRealm.startReindextakerealmEvents.startReindexalso skips its own invalidation andfullbroadcasts when it's false.realmEventstoonIgnoreRulesChanged, so the reindex it starts follows the write.Follow-ups
prerender-html-reconcilecron announces the rows a silent pass left stale.Test plan
Run locally against the shared test stack:
queue-test,full-reindex-test,atomic-endpoints-testandmaintenance-endpoints-test, 146/146.queue-test: a silent from-scratch pass doesn't let an announcing one join it while running; pending passes OR-merge;full-reindexsweeps merge only when their choice matches.full-reindex-test: a silent sweep enqueues silent per-realm passes.atomic-endpoints-test› realm events: a.boxelignorewrite with the header reindexes the realm with no realm event, and the reindex and render jobs it spawned carryrealmEvents: false/announce: false. Without the header, the reindex announcesfull.maintenance-endpoints-test:/_grafana-reindex,/_grafana-full-reindexand/_post-deploymentenqueuerealmEvents: false.file-write.test.ts(CI):write(…, { realmEvents: false })sendsX-Boxel-Realm-Events: nonefor text and binary content, a plain write sends none, and--no-realm-eventswrites the file.lint:typesfor realm-server, host and boxel-cli; eslint on the changed files.🤖 Generated with Claude Code