fix(server): release completed pull request sweep data #25177 · pingdotgg/t3code

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

MmacroscopeappBotapproved these changes4d ago

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.}

Ggithub-actionsBot4d ago

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.

CcoderabbitaiBot4d ago

Review in Change Stack →

No actionable comments were generated in the recent review.

External image Recent review info

External image Run configuration

Source mode: Auto
Repository: pingdotgg/t3code
Branch: fix/issue-25172
File: .coderabbit.config.ts
Commit: 57a7bd0
Layers: Repository TS config
Review profile: CHILL
Plan: Team
Run ID: 0cfa97c0-7f88-4a7c-b95a-a13f487c3002

External image Commits

Reviewing files that changed from the base of the PR and between f4e140764fba579ccb77a3dabe2dd0bcee98d037 and 57a7bd0a8dda42029eab327ecdac478dce334cae.

External image 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: External image 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: External image 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.}

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

Problem

Change

Scope and approval

Verification

Checks

Approvability

Thread transfer impact