Skip to content

fix(server): release completed pull request sweep data - #25177

Merged
juliusmarminge merged 3 commits into
mainfrom
fix/issue-25172
Oct 28, 2026
Merged

juliusmarminge merged 3 commits into
mainfrom
fix/issue-25172

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Oct 26, 2026 •

Copy link
Copy Markdown
Member

Problem

Every completed one-minute pull-request sweep can remain reachable through Effect stack frames retained by a cached lookup fiber. On an isolated headless server with 31 stopped thread projections linked to one open PR, successive completed sweeps retained one then two groups Maps after full GC.

Change

Use untraced function wrappers with explicit named spans for the two helpers defined inside each sweep. This releases sweep inputs while preserving the existing tracing span names and polling behavior. Add a collection regression that keeps a lookup fiber cached after the sweep finishes.

Scope and approval

Fixes #25172

Triaged bug via the automated issue flow: #25172. Focused server retention fix; no client, contract, authentication, or persisted-state changes.

Verification

  • Regression on the baseline implementation: expected zero surviving link objects, actual 30. Exact red command and failure.
  • Final head: all 49 reactor tests pass, including the collection regression. Command and output.
  • vp run check:changed passed format, lint, both knip checks, GraphQL, affected typechecks, and focused tests. Focused tests passed again after merging current main.
  • Two independent reviews returned MERGE without blocking findings. Final proof audit returned SUFFICIENT.
Real headless server measurement Before After
Retained sweep Maps after completed sweep 1 / full GC 1 0
Retained sweep Maps after completed sweep 2 / full GC 2 0
Helper tracing span names exported Present Preserved

Evidence index, runtime transcript, counters, provenance, and fixture script. Server-only change: visual fixture matrix is empty. Baseline runtime was captured at 0ad399c6c5; fixed runtime at 15b7291404, whose changed server files are byte-identical at final head 57a7bd0a8d. Upstream changes merged afterward were client-only.

Real-data seeding hit a VM database migration-history mismatch, so runtime proof uses synthetic stopped threads. The baseline contained 30 synthetic threads plus the bootstrap thread; after used 31 synthetic threads. This verifies retention and release, not the reporter's full 18,736-link workload or exact growth rate. Raw heaps are not published because they may contain credentials. CI and hosted review gates remain pending; no bugfix label will be added before they pass.

Models: Claude Opus 5.5 and GPT-6.1-sol, through T3 Code's Claude and Codex harnesses.

juliusmarminge and others added 3 commits October 26, 2026 00:54
…rom cache fibers

Regression for #25172. A cache lookup forked during the sweep keeps the
Effect.fn stack frames of syncGroup/syncEntry, whose closures hold the
sweep's groups, so every sweep's links outlive it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
syncGroup and syncEntry were Effect.fn closures defined inside each sweep.
Effect.fn's per-call stack frame closes over the call's arguments and body,
and a cache lookup fiber forked during the read keeps that frame for good,
so every one-minute sweep's groups and linked threads stayed reachable.
Both are now fnUntraced with an Effect.withSpan pipeable, which keeps their
span names without capturing the sweep in a stack-frame closure.

Fixes #25172

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Oct 26, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 57a7bd0

Macroscope's review found this PR approvable — This is a narrowly scoped server memory-retention fix that changes only Effect wrapper instrumentation, preserving existing spans and pull-request synchronization behavior. A focused regression test verifies that completed sweep data can be collected when a cache retains the lookup fiber.

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

@github-actions

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 6.8 KiB ✅
Codex Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Codex Live turn WebSocket wire 1.2 KiB 1.2 KiB 0 B (0.0%) 2.0 KiB ✅
Codex Live turn WebSocket decoded 20.9 KiB 20.9 KiB 0 B (0.0%) 29.3 KiB ✅
Codex Live turn messages 2 2 0 (0.0%) 8 ✅
Claude Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 6.8 KiB ✅
Claude Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Claude Live turn WebSocket wire 1.2 KiB 1.2 KiB 0 B (0.0%) 2.0 KiB ✅
Claude Live turn WebSocket decoded 21.2 KiB 21.2 KiB 0 B (0.0%) 29.3 KiB ✅
Claude Live turn messages 2 2 0 (0.0%) 8 ✅

Baseline: f4e1407 · PR result: 57a7bd0 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@coderabbitai

coderabbitai Bot commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between f4e1407 and 57a7bd0.

Files selected for processing: 2
  • apps/server/src/orchestration-v2/PullRequestSyncReactor.test.ts
  • apps/server/src/orchestration-v2/PullRequestSyncReactor.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.

Walkthrough

The pull-request sync helpers now use untraced Effect functions with explicit spans. A new test runs a cache-backed sweep, clears the snapshot, and checks that no SweptLink objects remain.

Changes: Sweep retention

Layer / File(s) Summary
Span handling and retention test
apps/server/src/orchestration-v2/PullRequestSyncReactor.ts, apps/server/src/orchestration-v2/PullRequestSyncReactor.test.ts
syncEntry and syncGroup use Effect.fnUntraced with explicit spans. The test runs a cache-backed sweep and checks for remaining SweptLink objects after clearing the snapshot.

Priority: Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested labels: size:S

Suggested reviewers: t3dotgg, maria-rcks

Merge Risk: Minimal · up to 57a7b

No actionable merge-blocking risk was identified; the change is ready for normal checks.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title is concise, specific, and accurately describes the server fix that releases completed pull-request sweep data.
Description check Passed The description includes the required Problem, Change, Scope and approval, and Verification sections. It explains the retention issue, links the triaged issue, documents focused tests and measurements…
Linked Issues check Passed Issue [#25172] requires completed PullRequestSyncReactor.sweep runs to release the groups map and its thread and link data. The change replaces the sweep-local Effect.fn closures for syncEntry…
Out of Scope Changes check Passed The reviewed changes are limited to PullRequestSyncReactor.ts and its regression test. The implementation changes Effect tracing and closure retention for the server pull-request sweep. The test val…
Full details: Description check

Explanation

The description includes the required Problem, Change, Scope and approval, and Verification sections. It explains the retention issue, links the triaged issue, documents focused tests and measurements, and states verification limits and pending gates.

Full details: Linked Issues check

Explanation

Issue [#25172] requires completed PullRequestSyncReactor.sweep runs to release the groups map and its thread and link data. The change replaces the sweep-local Effect.fn closures for syncEntry and syncGroup with Effect.fnUntraced wrappers and explicit Effect.withSpan calls. The existing span names remain PullRequestSyncReactor.syncEntry and PullRequestSyncReactor.syncGroup. The new regression keeps a cache lookup fiber alive and verifies that 30 swept link objects become collectible. The reported reactor and focused checks also pass.

Full details: Out of Scope Changes check

Explanation

The reviewed changes are limited to PullRequestSyncReactor.ts and its regression test. The implementation changes Effect tracing and closure retention for the server pull-request sweep. The test validates collection behavior under a retained cache fiber. These changes directly support issue [#25172]. No unrelated client, contract, authentication, or persisted-state changes are present in the reviewed diff.

Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@juliusmarminge
juliusmarminge merged commit 125b619 into main Oct 28, 2026
29 of 30 checks passed
@juliusmarminge
juliusmarminge deleted the fix/issue-25172 branch October 28, 2026 16:28
github-actions Bot added a commit to omarcresp/t3code-flake that referenced this pull request Oct 28, 2026
## What's Changed
* ci(release): desktop builds stop hitting the npm registry for staged installs by @t3dotgg in pingdotgg/t3code#26458
* feat(web): boring sidebar can show folder threads on two lines by @t3dotgg in pingdotgg/t3code#26463
* fix(server): release completed pull request sweep data by @juliusmarminge in pingdotgg/t3code#25177
* test(evals): device manager cases for mcp-tool-use by @gabrielelpidio in pingdotgg/t3code#25850
* fix(server): agents drive simulators through the device tools instead of ssh by @gabrielelpidio in pingdotgg/t3code#25661
* fix(mobile): overlay sidebar header blurs content scrolling under it by @juliusmarminge in pingdotgg/t3code#26580
* fix(web): numbered composer lists nest and lift correctly by @michal-billtech in pingdotgg/t3code#17138
* feat(mobile): add dynamic workflows to the Agents sheet by @Bil0000 in pingdotgg/t3code#18642
* fix(ci): Android merges stop cutting a new Play Store build every time by @juliusmarminge in pingdotgg/t3code#26596

## New Contributors
* @michal-billtech made their first contribution in pingdotgg/t3code#17138

**Full Changelog**: pingdotgg/t3code@v0.1.4-nightly.20261028.3824...v0.1.4-nightly.20261028.3838

Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.1.4-nightly.20261028.3838
Pipyakas pushed a commit to Pipyakas/t3code that referenced this pull request Oct 28, 2026
aorwall added a commit to aorwall/t3code that referenced this pull request Oct 29, 2026
Merges upstream `pingdotgg/t3code` `fe219c4de1` (95 commits, base
`49a9e80e0b`) into the fork. Branched from `main`; no open merge PR to
stack on.

## Resolution

15 conflicts, each resolved by the verdict `preflight.mjs` printed for
it. The full record is in `docs/fork/upstream-merge-log.md`
(2026-10-29). The main points:

- **`LegacySidebar.tsx` deleted upstream** (pingdotgg#21039). The deletion is
taken. Its fork deltas (touch long-press, Add project gate) already live
in `Sidebar.tsx` / `SidebarThreadRow.tsx`. The file's paths are removed
from three inventory entries, and the helper only it used is dropped
from `Sidebar.logic.ts`.
- **Composer access mode.** Upstream's simpler composer (pingdotgg#25869) shows
the Access control in three places: the Permissions chip, the
compact-menu section, and the effort-menu section in
`ComposerControlsBar.tsx`. All three are gated on `FEATURES.accessMode`.
`ComposerControlsBar.tsx` is added to `chat-composer-gates`.
- **Sidebar.** Upstream's boring-sidebar project folders are combined
with the fork's browse/closed/subtask filter. Upstream moved the row's
meta line into a `metaLine` variable, and the fork's owner name and
parent/child task links now live inside it.
- **Settings.** The boring-sidebar Labs rows are client settings and are
not gated. Simlock hosts join the `serverBehaviorSettings` rows, and the
`simlock-hosts` search id is mapped to that flag. The new Fallback model
row is hidden together with the default model row
(`defaultModelSetting`).
- **`AGENTS.md`.** Upstream now has agents check user-visible changes in
a real client without asking first. The fork's version says the same,
still pointing at the fork's skills.
- `agentContextBudget.json` uses upstream's numbers, plus the fork's 108
bytes for `ProjectScript.port`. The value `46015` was measured by
running the budget test.
- `pnpm-lock.yaml` was re-derived from upstream's copy.

**Merge stats:** 595 files landed vs 593 in the upstream range (the gap
is explained in the tracker entry). Fork delta: 766 files, unchanged.

## Verification

`verify.mjs` was run in full. These checks pass: duplicate-adds,
tripwires, resolution-check, unsupported-methods, lockfile, fmt, lint,
typecheck, build. In the test step, `web` and `client-runtime` pass. The
same failures as the 2026-10-27 and 2026-10-28 merges remain, each
confirmed failing alone:
- `t3`: `RepositoryIdentityResolver`, `GitVcsDriver`,
`CodexChatGptAuth`, `CodexManagedRuntime` (the known `/tmp` sticky-bit
gap)
- `provider-opencode`: `OpenCodeServerLedger`

## Usable as-is
- Boring sidebar project folders, two-line folder rows, and hiding empty
projects (pingdotgg#21039, pingdotgg#26387, pingdotgg#26391, pingdotgg#26463)
- Simpler composer with grouped controls and effort words (pingdotgg#25869). The
access-mode parts stay gated.
- Pending approvals on their timeline row (pingdotgg#26255); tool call diffs open
in the diff panel (pingdotgg#26304); compaction row timer (3ef1610)
- Many web fixes: chat tables, inline code double-click, numbered
composer lists, image chips, scroll-to-end, tool groups after a
usage-limit pause, long-thread timeline perf (pingdotgg#26727)
- New Claude threads default to Opus 5.5 (pingdotgg#26358)
- PR files: mark viewed with shift-click, and the diff follows the file
tree (pingdotgg#26349, pingdotgg#26730). These are reachable only where the PR surface is
(`pullRequestSurface`).

## Unsupported in Moatless / needs implementation
- **GitHub sign-in through the T3 Code for GitHub app** (pingdotgg#26245):
`sourceControl.signInStart`, `signInWait`, `signInCancel`, `appAccess`.
New `UnsupportedMethodError` entries.
- **Simlock device pools** (pingdotgg#25259, pingdotgg#26706): `device.simlockIdentity`
(new union entry) and the `simlockDeviceHostsEnabled` server setting.
Part of the device hub gap.
- **Fallback model when the default provider is out of usage** (pingdotgg#26164):
the `fallbackModelSelection` server setting cannot be saved, because
`server.updateSettings` is refused. The row is hidden.
- Managing GitHub maintainer edits (pingdotgg#25588): in the pull-request write
family Moatless does not dispatch.
- T3 Connect tunnel usage and T3 Chat account push (pingdotgg#26369, pingdotgg#25931):
relay/cloud, decided out.
- Mobile dynamic workflows in the Agents sheet (pingdotgg#18642) and the mobile
app fixes: mobile against Moatless is unverified.

## Backend behavior to consider reproducing in Moatless
These are listed under _Runtime fixes upstream made to its own server_
in `docs/fork/gaps.md`:
- Falling back to another provider when the default is out of usage
(pingdotgg#26164), and refreshing usage when a turn hits its limit (pingdotgg#26234); no
replay of stale Codex usage limits (pingdotgg#26331)
- Settling a thread at once when its watched PR merges (pingdotgg#26395).
Moatless reads no PR for settlement today, so this is a decision rather
than a fix.
- Loading long thread history in bounded pages (pingdotgg#26240); publishing
local git status before the remote lookup (pingdotgg#21046); releasing completed
PR sweep data (pingdotgg#25177)
- Thread links that outside MCP clients can open (pingdotgg#26642); listing
snoozed threads across projects in one MCP call (pingdotgg#26442)
- Provider approval and subagent fixes: Claude command approvals
(pingdotgg#20145), ACP ID-only approvals (pingdotgg#26647), Cursor subagents (pingdotgg#26592),
OpenCode subagent runtime mode (pingdotgg#26302), Codex shadow-home locks
(c4606e5); shorter remote reconnect and command delays (pingdotgg#25754)

## After merging
- Disable the new upstream workflow: `gh workflow disable
mobile-fingerprint-label.yml --repo soaplabs/t3code`

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

---
Moatless task:
https://moatless.soaplabstest.com/tasks/b2df0ac3-40ce-4c78-a67d-b561b3d8772d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S 10-29 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Server memory grows ~1.7 GB/hour: every one-minute pull-request sync sweep stays in memory

1 participant