Fix wrong session token total after a tab reload

[jurgenwerk]

Bug: the session token total shows a wrong sum after a tab reload — $1.32 of actual usage displayed as $0.62.

Why: content.data is a JSON string on the wire. We parse it for live events, but not for the edit bundled inside reloaded events — so data.usage reads off a string and comes back undefined. Token counts live on edits, so reloaded turns lose them. (Same field carries agentId, which gates tool auto-run.)

Fix: parse the bundled edit's data too. One conditional + a test.

Related to this display:

🤖 Generated with Claude Code

[github-actions]

Preview deployments

• Host staging preview

• Host production preview

Host Test Results

    1 files  ±0      1 suites  ±0   2h 18m 6s ⏱️ - 2m 47s
4 369 tests ±0  4 355 ✅ ±0  14 💤 ±0  0 ❌ ±0 
4 388 runs  ±0  4 374 ✅ ±0  14 💤 ±0  0 ❌ ±0 

Results for commit 98c3279. ± Comparison against earlier commit 6b004e7.

Realm Server Test Results

    1 files  ±0      1 suites  ±0   16m 7s ⏱️ +2s
2 206 tests ±0  2 205 ✅  - 1  0 💤 ±0  1 ❌ +1 
2 289 runs  ±0  2 288 ✅  - 1  0 💤 ±0  1 ❌ +1 

Results for commit 98c3279. ± Comparison against earlier commit 6b004e7.

For more details on these errors, see this check.

[Copilot]

Pull request overview

Fixes a host-side Matrix room rebuild bug where token usage (and context.agentId) could be lost after a tab reload because backfilled events’ bundled edit (unsigned['m.relations']['m.replace']) kept content.data wire-encoded as a JSON string.

Changes:

• Decode content.data not only on the event itself, but also on the bundled m.replace aggregation payload in Room.addEvent.
• Add a unit test asserting both the event’s own data and the bundled replacement’s data are decoded.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
packages/host/app/lib/matrix-classes/room.ts	Decode content.data for unsigned['m.relations']['m.replace'] so rebuilt messages retain usage/context.
packages/host/tests/unit/room-test.ts	New unit test covering decoding for both the base event and its bundled replacement.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[habdelra]

[Claude Code 🤖] Reviewed this as a correctness-of-mechanism change: does the diagnosis hold on the path a reload actually takes, does the decode land at the only place those events funnel through, and is anything else read off the substituted event still being lost. Traced it end to end and it holds.

Bottom line: no blocking issues — the fix is correct and minimal. The three non-blocking asks below are all about pinning the invariant the fix establishes, not about the fix itself.

Verified, for the record:

• The reload path is the filtered timeline in loadAllTimelineEvents, which excludes m.replace via 'org.matrix.msc3874.not_rel_types' — so on backfill the bundle genuinely is the only carrier of the latest edit. The diagnosis in the description is exact.
• Room.addEvent is the single funnel for AI-room timeline events; the other addRoomEvent callers are the optimistic-send and membership paths, neither of which carries bundles. Decoding here covers every reader.
• The in-place mutation is safe: buildEventForProcessing cloneDeeps before processDecryptedEvent, so the SDK's own event objects aren't touched, and nothing re-parses data afterwards — the only JSON.stringify of data in the host is on the send path in matrix-service.ts.
• data is the whole blast radius, not a sample of it: sendMatrixEvent in runtime-common/ai/matrix-utils.ts stringifies content.data and no other key, so usage and context.agentId are exactly what a string data costs.
• Dropping the duplicated eventId = eventId ?? stateKey in the second commit is a true no-op — 34b6998 carried that identical assignment twice, ten lines apart, and the earlier one survives. Worth stating because the diff renders it as a plain deletion of the fallback next to the guard that throws on a missing id.

Recommendations, most to least worth doing:

1. Add a room-resource-level test for the bundle path — nothing in the host suite exercises an aggregation bundle today, so this regression can return green. File-level comment on the new test.
2. Pin the decode/substitute ordering, or record the dependency at RoomResource.getAggregatedReplacement. The host's ordering is the inverse of constructHistory's, and that inversion is what made this bug possible. Inline on the new bundledReplace block.
3. Decide whether the two bare JSON.parse calls should be tolerant; the server-side twin try/catches, and an uncaught throw here aborts the rest of a room's backfill. Same thread.
4. Reword the code comment's past-tense bug narration, and disambiguate getAggregatedReplacement — two functions share that name. Inline on the comment block.

On CI: Realm Server Tests (3, 6) is red at 1 fail / 2 205 pass. The diff is host-only — packages/host/app/lib/matrix-classes/room.ts is imported from nowhere outside packages/host, and the realm-server suite only boots the host dist for prerendering, which never instantiates the matrix service — so nothing in this change can reach that suite. I couldn't extract the failing test name (the job-log API returns only the tail, which is teardown), so a re-run to confirm is the cheap next step rather than a code change.

Adjacent, out of scope: addEvent's new-event branch returns before this.emitter.emit('event.added'), so waitForNextEvent() resolves only on a replacement of an existing event. packages/host/app/tools/get-events-from-room.ts awaits it with no timeout when there's nothing new since sinceEventId, so that tool waits on a promise a genuinely new event won't settle. Pre-existing and not this PR's to fix, but it's in the method you're editing.

---

Generated by Claude Code

[habdelra]

[Claude Code 🤖] The decode is correct, but it establishes an invariant — "the event MessageBuilder sees always has an object data" — that holds only because two files, in two packages, happen to agree. Nothing enforces it, and the field that goes missing when they stop agreeing goes missing silently. Non-blocking, but worth pinning one way or the other.

Background — why the ordering matters. Two subsystems apply the same server-side aggregation, in opposite orders. constructHistory in packages/runtime-common/ai/history.ts maps getAggregatedReplacement over the whole event list first, then calls parseContentData(rawEvent) on the substituted event. Decoding after substitution means that path structurally cannot lose data, however many nesting levels the homeserver bundles. The host inverts it: Room.addEvent decodes at ingest, and RoomResource.getAggregatedReplacement (packages/host/app/resources/room.ts) substitutes later, in loadRoomMessage. That inversion is the bug's actual cause; this hunk compensates for it by pre-decoding the one nesting level the substitution can reach.

Verified against the real path. loadAllTimelineEvents (matrix-service.ts) builds its backfill timeline with 'org.matrix.msc3874.not_rel_types': ['m.replace'], so the edit events themselves never come back on a reload and the bundle under unsigned['m.relations']['m.replace'] is the only carrier of the latest edit — the diagnosis is exact. Those events flow timelineQueue → drainTimeline → buildEventForProcessing (a cloneDeep, so the in-place mutation here can't touch SDK state) → processDecryptedEvent → addRoomEvent → here, which is the single funnel for AI-room timeline events. Downstream, message-builder.ts reads (this.event.content as CardMessageContent)?.data?.usage at two sites on the substituted event, and components/matrix/room.gts sums message.usage into the conversation total — so a string data drops the counts for every pre-reload turn.

The ask. Either match the server's ordering — export parseContentData from runtime-common/ai/history.ts and call it in RoomResource.getAggregatedReplacement right after finalRawEvent = replacedRawEvent, keeping the ingest decode here for non-aggregated events — or, if you'd rather leave the fix where it is, add a line at getAggregatedReplacement recording that its substituted event is only safe because addEvent pre-decodes the bundle. As it stands, whoever next moves this decode or adds a nesting level reproduces the same silent field loss with neither a comment nor a test in the way (see the file-level comment on the new test).

Second, smaller thing on the same lines: neither parse is tolerant. parseContentData wraps its parse in try/catch, logs, and throws a typed HistoryConstructionError; both parses here are bare. A throw out of addEvent propagates through drainTimeline, whose loop is try/finally with no catch and which has already emptied timelineQueue — so one unparseable data drops every remaining event in that batch and rejects the await this.drainTimeline() in loadAllTimelineEvents, i.e. the rest of the room's backfill. The trigger is unlikely, since sendMatrixEvent produces these strings itself, and it's pre-existing for the event's own content.data — but this hunk adds a second site, so it's worth a decision rather than inheritance. One try/catch around both that logs and leaves the string in place degrades to "this message loses its usage" instead of "the timeline stops loading".

Scope: the first point is a missing guard on an otherwise-correct fix (regression class, non-blocking); the second is pre-existing and widened by this hunk (non-blocking).

---

Generated by Claude Code

[habdelra]

[Claude Code 🤖] Two rewording asks on this comment — non-blocking, but both cost a future reader real time.

The parenthetical narrates the bug in past tense. "the session token total visibly shrank after a reload because pre-reload turns' usage vanished this way" describes the behaviour before this commit. A reader two years out has no anchor for when "shrank" was true and will read it as a live symptom, then go hunting for a bug that isn't there. evergreen-comments asks for the contract stated timelessly; the mechanism in the first three lines already carries the value, so the parenthetical only needs to name what depends on the decode.

getAggregatedReplacement names two different functions in this repo — packages/host/app/resources/room.ts (the one this comment means) and packages/runtime-common/ai/history.ts (which orders the decode the other way round, per the comment below). The reader has to open one of them to see why the ordering matters here, so it's worth saying which.

Something like:

    // Backfilled events carry their latest edit as a server-side aggregation
    // bundle, and getAggregatedReplacement in app/resources/room.ts
    // substitutes that bundle's content for the event's own — so its data
    // needs the same decoding. Everything read off `data` on rebuild depends
    // on it: token usage, and context.agentId, which gates tool auto-run.

Scope: prose only, no behaviour change.

---

Generated by Claude Code

[habdelra]

[Claude Code 🤖] This test pins the decode but not the contract its own comment claims, and nothing else in the host suite covers aggregation bundles — so the regression it guards can come back green. Non-blocking, but the coverage ask is the one I'd most like to see land in this PR.

What the suite covers today. grep -rn "m.relations" packages/host/tests returns one unrelated line in query-fields-test.gts and nothing else; the only aggregation-bundle fixture anywhere in the repo is packages/ai-bot/tests/debug-test.ts. So the host's entire bundle path — RoomResource.getAggregatedReplacement → MessageBuilder → message.usage → the conversation total in components/matrix/room.gts — has no test at all.

What this test does and doesn't reach. It asserts that JSON.parse ran on two objects, and it does fail without the fix, which is the important half. But the comment above it claims the consequences — "token usage, context.agentId, and with it the agent gate for tool auto-execution" — and no assertion here reaches any of them. A change that keeps these two parses while altering the substitution site, or that moves the decode to the substitution site as suggested in the comment on room.ts, leaves this test green while the displayed total goes wrong again. That's the failure mode this PR exists to prevent.

The ask. Add a case one layer up: feed an original plus its bundled edit through the room resource and assert the rebuilt message's usage (and context.agentId, if it's cheap from there). packages/host/tests/unit/message-builder-test.ts is the closest precedent for the setup. Keeping this unit test alongside it is fine — it localises the failure — but on its own it doesn't pin the thing that broke.

Two smaller things. room.addEvent(event as any) means nothing checks that the fixture resembles a real timeline event, which also erases the status: null as EventStatus | null annotation and makes the EventStatus import decorative. The module under test already exports TempEvent — does the fixture type as that? I couldn't check locally (workspace deps aren't installed in my checkout), so treat it as a question rather than a claim; if it does, dropping the cast makes fixture drift a compile error. Separately, the module name is Unit | matrix | room but the file sits at tests/unit/room-test.ts, while tests/unit/matrix/ already exists and holds login-error-text-test.ts under the matching Unit | matrix | … prefix.

Scope: test coverage — the room-resource test is a follow-up at worst, the placement and typing are trivial.

---

Generated by Claude Code