Skip to content

fix(server): release consumed event replay pages - #10777

Merged
juliusmarminge merged 1 commit into
pingdotgg:mainfrom
Gigioxx:fix/10774-event-replay-memory
Sep 9, 2026
Merged

fix(server): release consumed event replay pages#10777
juliusmarminge merged 1 commit into
pingdotgg:mainfrom
Gigioxx:fix/10774-event-replay-memory

Conversation

@Gigioxx

@Gigioxx Gigioxx commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Nightly 1400 runs attachment cleanup over the full event history at startup. The event store recursively concatenated 500-row pages, keeping consumed pages alive until replay finished. Large histories exhausted the backend heap before cleanup could save progress.

Use Stream.paginate for both global and aggregate replay so only the cursor and remaining limit carry between pages. Full catch-up, ordering, limits, decoding errors, captured heads, and cleanup retries keep their existing behavior. No new dependencies, API changes, or data migration.

Reproduced both readers retaining a consumed page before the change. The new regression tests use V8's post-GC object counts rather than memory-size or timing thresholds. All 67 focused event-store, projection-pipeline, and orchestration-engine tests pass, as do server typecheck and scoped lint/format checks.

On an isolated copy of the affected 1,131,083-event database (3.44 GB of payload), cleanup completed in 40 seconds under a 1 GiB heap cap and advanced its cursor from zero to the head. A second bootstrap took under 1 ms; all 586 threads and all events remained present. This exercised the real startup projection pipeline, not the Electron UI. The installed app and live data were untouched.

Closes #10774.

Created with GPT-6 Astra in Codex.

Summary by CodeRabbit

  • Bug Fixes

    • Improved reliability when reading large event histories by releasing consumed replay pages promptly, helping reduce memory usage during extended processing.
    • Event streams can be safely collected more than once with consistent results.
    • Invalid negative limits now return an empty result instead of attempting to read events.
  • Performance

    • Stream pagination was streamlined while preserving existing ordering and stopping behavior.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Sep 8, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — The server change is a narrowly scoped replay-pagination fix with tests covering both affected paths and page release behavior. Human review is required because the test adds a line-level oxlint suppression directive.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Sep 8, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: bd782b86-b189-4752-9a49-394d27d10b0b

📥 Commits

Reviewing files that changed from the base of the PR and between 061543e and 04c157d.

📒 Files selected for processing (2)
  • apps/server/src/persistence/Layers/OrchestrationEventStore.test.ts
  • apps/server/src/persistence/Layers/OrchestrationEventStore.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The event store replaces recursive page concatenation with explicit pagination state. New tests verify reusable streams, negative-limit handling, and release of consumed pages during large replays.

Changes

Event pagination and retention

Layer / File(s) Summary
Explicit event pagination
apps/server/src/persistence/Layers/OrchestrationEventStore.ts
readFromSequence and readAggregateRange use Stream.paginate with cursor and remaining state. Existing page and sequence termination conditions remain explicit.
Pagination and page-retention tests
apps/server/src/persistence/Layers/OrchestrationEventStore.test.ts
Tests verify repeated collection, negative limits, and release of consumed pages after a 1,501-event replay.

Priority: ⬆️ High — Prioritize the event-store pagination fix because replay from a 1.13-million-event history can exhaust startup memory and block attachment cleanup.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Severity of issue fixed: High

Merge Risk: ⚪ Minimal · up to 04c15

Event replay now releases consumed pages during large catch-up scans without changing replay limits or ordering. The change is covered by pagination and retention tests and is ready to merge.

Suggested reviewers: t3dotgg

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the primary change: releasing consumed event replay pages in the server.
Description check ✅ Passed The description clearly explains the problem, implementation, preserved behavior, tests, and validation results. It does not use the template headings or include the checklist, but it contains the req…
Linked Issues check ✅ Passed The changes satisfy issue #10774 by replacing retaining recursive pagination with Stream.paginate for global and aggregate replay while preserving catch-up, limits, ordering, decoding errors, and retr…
Out of Scope Changes check ✅ Passed The changes are limited to the event-store pagination implementation and focused regression tests. No unrelated code, API, dependency, schema, or migration changes are present.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@juliusmarminge
juliusmarminge merged commit 08463e2 into pingdotgg:main Sep 9, 2026
21 checks passed
aorwall added a commit to aorwall/t3code that referenced this pull request Sep 9, 2026
Merges `pingdotgg/t3code` into the fork: upstream `2a3035353` from base
`a37c66406`, 53 commits. Landed as a merge commit, not a cherry-pick.

Tracker entry: `docs/fork/upstream-merge-log.md`. Gaps this merge opened
or
extended: `docs/fork/gaps.md`.

## Usable as-is

Fork can expose these with no Moatless backend or deployment work.

- **Project icons everywhere they belong** — favicon in the new-thread
project
picker (pingdotgg#10790), the project record passed to `ProjectFavicon` so icons
cannot
  drift (pingdotgg#10714), and the same icon in the command palette (pingdotgg#10712).
  `apps/web/src/components/ProjectFavicon.tsx`, `DraftHeroHeadline.tsx`.
- **Composer and sidebar layout settling** — footer held still while
thread data
loads (pingdotgg#10768), the bar under the composer no longer pops in (pingdotgg#10727),
chat
text no longer shows through a 1px gap under composer banners (pingdotgg#10635),
scroll-to-end button kept close to the composer (pingdotgg#10543), sidebar rows
no
longer flash and shift on click (pingdotgg#10713), settings sidebar no longer
shifts
  when switching pages (pingdotgg#10705).
- **Minimap turn navigation** (pingdotgg#8531) — previous/next turn controls,
  `apps/web/src/components/chat/` minimap surface. Pure client state.
- **Terminal** — copy selection with Ctrl+Insert (pingdotgg#8541), honor terminal
link
  browser overrides (pingdotgg#10060).
- **Usage panel** — account columns aligned across limit rows (pingdotgg#10690),
  email-bearing account labels hidden (pingdotgg#10668).
- **Setup wizards consolidated into shared components** (pingdotgg#10832).
- **File drops onto sidebar threads** (pingdotgg#7892) — rides the attachment
upload path
the fork already has; `useSidebarPendingFileDropStore` threaded through
  `ChatView.tsx` and `_chat.$environmentId.$threadId.tsx`.
- **Mobile** — drag handles to arrange threads (pingdotgg#10496), Android
wallpaper
colors (pingdotgg#10691), optional Material You layout (pingdotgg#10692), tolerate native
`Headers` without `getSetCookie` (pingdotgg#10851), respect notification
permission
  when tokens rotate (pingdotgg#10850).
- **Desktop** — macOS installer artwork (pingdotgg#10632, pingdotgg#10819, pingdotgg#10820), layout
control
hit targets (pingdotgg#10673), context menus in the browser (pingdotgg#10670), no
declarations
during bundling (pingdotgg#10679), keyring loading deferred until macOS cookie
import
(pingdotgg#10667). `electron-desktop` is kept in tree and is not a compliance
target.
- **Dependency and hygiene** — Effect `rc.112` and Alchemy `beta.76`
(pingdotgg#10652)
with reference syncs (pingdotgg#10653, pingdotgg#10654), and the knip export
classification
  sweep across server modules (pingdotgg#10274pingdotgg#10282).

## Unsupported in Moatless / needs implementation

- **Attach files to question answers** (#7220dfe2c, pingdotgg#9871). Upstream
added the
  wire capability `questionAttachments` in
  `packages/contracts/src/environment.ts` and threads
`supportsQuestionAttachments` through `ChatView.tsx` →
`ChatComposer.tsx`.
The fork takes upstream's plumbing verbatim; the capability is simply
absent
  from what Moatless reports, so the composer correctly offers nothing.
**To implement:** accept attachments on the answer-submission path and
report
  `capabilities.questionAttachments: true`. The sibling
  `ServerProvider.reportsContextWindow` flag lands in the same shape.
  Recorded in `gaps.md`.
- **Pull request merge defaults** (#7d9aaf6a7, pingdotgg#8088). Adds
`pullRequestMergeMethodOverrides` to
`packages/contracts/src/settings.ts` —
  a per-project merge method plus a last-used default, surfaced in
`ProjectSettingsPanel.tsx` and `PullRequestDetailPanel.tsx`. **To
implement:**
Moatless must persist these settings fields, and the panel that consumes
them
needs `pullRequests.detail`, which Moatless does not dispatch (it serves
only
  `pullRequests.summary`). Recorded in `gaps.md` under Pull requests.
- **Relay push-notification routing** (pingdotgg#10859, pingdotgg#10849, pingdotgg#10848) — current
APNs
registration routing for queued jobs, requeue checks for queued iOS
alerts,
  shared notification policy prioritizing waiting agents. These land in
`infra/relay/`, which belongs to the `cloud-relay-connect` concern the
fork has
decided out. No fork app code imports them; taken as upstream and left
inert.

## Backend behavior to consider reproducing in Moatless

Upstream server behavior the fork cannot use directly — `apps/server` is
not
what Moatless runs — but that would improve Moatless.

- **Give completed turns a full session idle window** (#430fbd1ff,
pingdotgg#10689).
Upstream's `ProviderSessionReaper` was measuring idle time from a point
that
cut a completed turn's window short, so provider sessions were reaped
earlier
than intended and the next turn paid a cold start. Worth checking
whatever
  Moatless uses to retire provider sessions against the same case.
- **Release consumed event replay pages** (#08463e2c4, pingdotgg#10777).
`OrchestrationEventStore` held every page it had produced while
replaying,
  so a long thread's replay grew without bound. Upstream moved it to
`Stream.paginate`. If Moatless replays orchestration events to
reconnecting
  clients, it has the same shape of exposure.
- **Stop Windows terminal processes when closing** (#47eed9fac, pingdotgg#10771)
—
terminal child processes outlived their session on Windows. Relevant to
  Moatless only if it hosts terminals on Windows runners.
- **Generate thread titles with the selected model across connections**
(#bc4b00666, pingdotgg#10526) — title generation was falling back to a default
model
  rather than the connection's selected one.
- **Keep preview snapshots usable by the agent and let it save them**
(#061543e9e, pingdotgg#10501) — adds a `save` argument to the MCP
`preview_snapshot`
  tool so the agent can persist a snapshot rather than only view it.
  Recorded in `gaps.md` under Preview automation.

## Conflicts

7 files. Full reasoning is in the tracker entry; the two worth reading
here:

- **`SettingsSidebarNav.tsx`** — upstream deleted the settings
sub-section nav
wholesale (`settingsSectionVisibility.ts` no longer exists upstream),
which
collided with the fork's admin/personal split. Kept upstream's removal
and
rebuilt the split on top of it, extracting `renderNavItem` so both nav
groups
  render identical rows.
- **`ChatComposer.tsx`** — both hunks resolved to upstream. The fork
comment
there documented `maxFileAttachmentBytes`, which upstream now owns
itself.

`pnpm-lock.yaml` was `theirs` then re-derived with `vp i`.
`apps/server/src/cli/pair.ts` is the one file in the upstream range that
did not
land — the fork deletes that surface deliberately, and the tripwire
confirms it
is still deleted.

## A rename that no conflict marked

Upstream's Effect bump renamed `Schema.TaggedErrorClass` to
`Schema.TaggedError`. Upstream renamed its own two occurrences, so those
merged
clean — but the fork's three (`UnsupportedMethodError` in
`packages/contracts/src/auth.ts`, `SandboxNotRunningError` in
`packages/contracts/src/sandbox.ts`, and one in
`apps/web/src/environments/primary/auth.ts`) have no upstream
counterpart, so
git carried them through untouched. Typecheck failed with one `TS2551`
and about
forty cascading `TS2740`s in `rpc.ts` behind it. All three renamed.

## Verification

`verify.mjs` is green on seven checks: duplicate-adds, tripwires,
resolution-check, unsupported-methods, `fmt:check`, lint, typecheck.

`test` is red on **`@t3tools/desktop` only**, and it is the machine
rather than
this merge: `scripts/browser-secret-native.test.mjs` shells out to
`pkg-config --cflags --libs libsecret-1`, which the sandbox image does
not
carry. The file is byte-identical to upstream and fails the same way on
a clean
tree; the standing entry is in `gaps.md`. 1289 tests pass, 1 suite fails
to
compile. Four packages did not finish under parallel load and all four
pass
alone — `@t3tools/web` 383 files, `t3` 293, `@t3tools/mobile` 155,
`t3code-relay` 28.

Unsupported methods: 1 ADD, 0 DROP, 2 KEEP, 4 known exceptions. The ADD
is
`sandbox.detail` and it is **pre-existing drift, not merge-introduced**
—
confirmed by re-running the derivation against `HEAD^1`. Applied anyway,
with
the union entry documenting that Moatless dispatches
`sandbox.subscribeDetail`
and not its one-shot sibling.

`duplicate-adds.mjs` reported `target="_blank"` in
`MessagesTimeline.tsx`. Both
parents have it once, on two unrelated anchors — the fork's
`MessageOriginIcon`
and upstream's new question-attachment link. The script now skips a bare
JSX
attribute on its own line, for the same reason it already skips
punctuation.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---
Moatless task:
https://moatless.soaplabstest.com/tasks/ff7d0df5-d989-4ec6-95c6-4e730cf3fd6f
github-actions Bot added a commit to omarcresp/t3code-flake that referenced this pull request Sep 9, 2026
## What's Changed
* fix(relay): use current APNs registration routing for queued jobs by @juliusmarminge in pingdotgg/t3code#10859
* fix(server): release consumed event replay pages by @Gigioxx in pingdotgg/t3code#10777
* feat(mobile): arrange threads with drag handles by @juliusmarminge in pingdotgg/t3code#10496
* feat(mobile): add Android agent notifications and ongoing activity by @ryanrhughes in pingdotgg/t3code#10416


**Full Changelog**: pingdotgg/t3code@v0.0.41-nightly.20260909.1426...v0.0.41-nightly.20260909.1439

Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.41-nightly.20260909.1439
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Nightly 1400 backend exhausts heap during attachment cleanup on startup

2 participants