[juliusmarminge]

Flaky test, not a product bug. The Check failure only aggregates the Test Server 2 failure: PullRequestReadCache.test.ts 'does not keep a caller's data alive after its reads and invalidations' (expected 2 to equal 0). Run once at a time it passed 20/20; repeated 300x in one process it failed about 10% of the time. Heap snapshots of three failing runs show the surviving sweep data held only by V8's weak_refs_keep_during_job list: the test's own new WeakRef(groups) keeps the target alive until the current job ends, and the Effect.yieldNow before gc() does not reliably get past that point. The cache itself retained nothing. The fix drops WeakRefs: sweep data is a per-test marker class (SweepData extends Map) and the test asserts v8.queryObjects(SweepData, { format: 'count' }) === 0, which runs a full GC first and creates no WeakRef. The --expose-gc/vm gc setup and the yieldNow are removed. A copy that leaks the data into a module-level array still fails (expected 5 to equal 0), so the test still catches real retention. Node prints an ExperimentalWarning for v8.queryObjects on stderr; it does not fail the run.

Reproduction on main

Run the failing check on main.

Evidence

The checks on this pull request are the proof: it changes what main's checks run, not what the app does.

Tests

Before the fix: vp test run src/pullRequest/PullRequestReadCache.test.ts -t 'does not keep a caller' 20x passed 20/20; the case repeated 300x in one process failed 32/300 (and ~10% under CPU load). After the fix: the 300x in-process repeat passed 300/300 under CPU load and 300/300 without load; a deliberately leaking copy failed as expected; vp test run src/pullRequest/PullRequestReadCache.test.ts run 20x in a row: 20/20 passed (12 tests each); vp run check:changed passed (format, lint, knip, graphql, typecheck of 3 packages, tests).

Not in this change

• Node prints an ExperimentalWarning for v8.queryObjects() on stderr; it does not affect the result.

Who did the work

• Written by Claude Opus 5.5, in Claude Code
• Reviewed by GPT-6.1 Sol, in Codex, and by Claude Opus 5.5, each on its own

Run by the t3code factory in T3 Code.

[github-actions]

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	21.5 KiB	21.5 KiB	0 B (0.0%)	24.4 KiB	✅
Codex	Live turn messages	1	1	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.9 KiB	21.9 KiB	0 B (0.0%)	24.4 KiB	✅
Claude	Live turn messages	1	1	0 (0.0%)	8	✅

Other scenarios

All 64 scenario measurements match main.

All 64 scenario measurements

Provider	Scenario	Metric	Main baseline	This PR	Impact	PR ceiling
Codex	short turn resume	WebSocket decoded	10.5 KiB	10.5 KiB	0 B (0.0%)	12.7 KiB	✅
Codex	short turn resume	WebSocket messages	2	2	0 (0.0%)	2	✅
Codex	long turn running, cold open	HTTP decoded	281.7 KiB	281.7 KiB	0 B (0.0%)	302.7 KiB	✅
Codex	long turn running, cold open	rows	75	75	0 (0.0%)	75	✅
Codex	long turn running, history page	HTTP decoded	92.4 KiB	92.4 KiB	0 B (0.0%)	107.4 KiB	✅
Codex	long turn running, history page	rows	75	75	0 (0.0%)	75	✅
Codex	long turn finished, cold open	HTTP decoded	381.5 KiB	381.5 KiB	0 B (0.0%)	410.2 KiB	✅
Codex	long turn finished, cold open	rows	151	151	0 (0.0%)	151	✅
Codex	long turn finished, socket cold open	WebSocket decoded	381.6 KiB	381.6 KiB	0 B (0.0%)	410.2 KiB	✅
Codex	long turn finished, socket cold open	WebSocket messages	2	2	0 (0.0%)	2	✅
Codex	long turn finished, socket cold open	rows	151	151	0 (0.0%)	151	✅
Codex	long turn finished, work page	HTTP decoded	186.2 KiB	186.2 KiB	0 B (0.0%)	200.2 KiB	✅
Codex	long turn finished, work page	rows	150	150	0 (0.0%)	150	✅
Codex	idle thread resume	WebSocket decoded	65 B	65 B	0 B (0.0%)	200 B	✅
Codex	idle thread resume	WebSocket messages	1	1	0 (0.0%)	1	✅
Codex	fleet shell snapshot	HTTP decoded	42.4 KiB	42.4 KiB	0 B (0.0%)	53.7 KiB	✅
Codex	fleet shell snapshot	rows	27	27	0 (0.0%)	27	✅
Codex	fleet shell subscribe	WebSocket decoded	65 B	65 B	0 B (0.0%)	200 B	✅
Codex	fleet shell subscribe	WebSocket messages	1	1	0 (0.0%)	1	✅
Codex	fleet shell resume check	HTTP decoded	18 B	18 B	0 B (0.0%)	100 B	✅
Codex	fleet shell resume check	rows	0	0	0	0	✅
Codex	stale shell reload	HTTP decoded	42.4 KiB	42.4 KiB	0 B (0.0%)	53.7 KiB	✅
Codex	stale shell reload	rows	27	27	0 (0.0%)	27	✅
Codex	stale shell resume	WebSocket decoded	140 B	140 B	0 B (0.0%)	400 B	✅
Codex	stale shell resume	WebSocket messages	2	2	0 (0.0%)	2	✅
Codex	stale todo list cold open	HTTP decoded	72.4 KiB	72.4 KiB	0 B (0.0%)	83.0 KiB	✅
Codex	stale todo list cold open	rows	75	75	0 (0.0%)	75	✅
Codex	stale todo list cold open	nodes	2	2	0 (0.0%)	4	✅
Codex	stale todo list cold open	subagents	2	2	0 (0.0%)	4	✅
Codex	stale todo list cold open	plans	1	1	0 (0.0%)	2	✅
Codex	stale todo list cold open	providerTurns	1	1	0 (0.0%)	2	✅
Codex	stale todo list cold open	checkpoints	1	1	0 (0.0%)	2	✅
Claude	short turn resume	WebSocket decoded	10.6 KiB	10.6 KiB	0 B (0.0%)	12.7 KiB	✅
Claude	short turn resume	WebSocket messages	2	2	0 (0.0%)	2	✅
Claude	long turn running, cold open	HTTP decoded	283.3 KiB	283.3 KiB	0 B (0.0%)	302.7 KiB	✅
Claude	long turn running, cold open	rows	75	75	0 (0.0%)	75	✅
Claude	long turn running, history page	HTTP decoded	93.4 KiB	93.4 KiB	0 B (0.0%)	107.4 KiB	✅
Claude	long turn running, history page	rows	75	75	0 (0.0%)	75	✅
Claude	long turn finished, cold open	HTTP decoded	384.2 KiB	384.2 KiB	0 B (0.0%)	410.2 KiB	✅
Claude	long turn finished, cold open	rows	151	151	0 (0.0%)	151	✅
Claude	long turn finished, socket cold open	WebSocket decoded	384.3 KiB	384.3 KiB	0 B (0.0%)	410.2 KiB	✅
Claude	long turn finished, socket cold open	WebSocket messages	2	2	0 (0.0%)	2	✅
Claude	long turn finished, socket cold open	rows	151	151	0 (0.0%)	151	✅
Claude	long turn finished, work page	HTTP decoded	188.1 KiB	188.1 KiB	0 B (0.0%)	200.2 KiB	✅
Claude	long turn finished, work page	rows	150	150	0 (0.0%)	150	✅
Claude	idle thread resume	WebSocket decoded	66 B	66 B	0 B (0.0%)	200 B	✅
Claude	idle thread resume	WebSocket messages	1	1	0 (0.0%)	1	✅
Claude	fleet shell snapshot	HTTP decoded	42.9 KiB	42.9 KiB	0 B (0.0%)	53.7 KiB	✅
Claude	fleet shell snapshot	rows	27	27	0 (0.0%)	27	✅
Claude	fleet shell subscribe	WebSocket decoded	66 B	66 B	0 B (0.0%)	200 B	✅
Claude	fleet shell subscribe	WebSocket messages	1	1	0 (0.0%)	1	✅
Claude	fleet shell resume check	HTTP decoded	18 B	18 B	0 B (0.0%)	100 B	✅
Claude	fleet shell resume check	rows	0	0	0	0	✅
Claude	stale shell reload	HTTP decoded	42.9 KiB	42.9 KiB	0 B (0.0%)	53.7 KiB	✅
Claude	stale shell reload	rows	27	27	0 (0.0%)	27	✅
Claude	stale shell resume	WebSocket decoded	140 B	140 B	0 B (0.0%)	400 B	✅
Claude	stale shell resume	WebSocket messages	2	2	0 (0.0%)	2	✅
Claude	stale todo list cold open	HTTP decoded	73.4 KiB	73.4 KiB	0 B (0.0%)	83.0 KiB	✅
Claude	stale todo list cold open	rows	75	75	0 (0.0%)	75	✅
Claude	stale todo list cold open	nodes	2	2	0 (0.0%)	4	✅
Claude	stale todo list cold open	subagents	2	2	0 (0.0%)	4	✅
Claude	stale todo list cold open	plans	1	1	0 (0.0%)	2	✅
Claude	stale todo list cold open	providerTurns	1	1	0 (0.0%)	2	✅
Claude	stale todo list cold open	checkpoints	1	1	0 (0.0%)	2	✅

Baseline: 7d51718 · PR result: 146c6bd · 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.

[macroscopeapp]

Approvability

Verdict: Approved at 146c6bd

Macroscope's review found this PR approvable — This change is confined to an ignored server cache test. It does not change product defaults or introduce diagnostic suppressions, and no material production risk is identified.

Notes:

• No code objects were reviewed. Approvability was decided on eligibility alone.

^{You can add or adjust custom eligibility rules. Learn more.}

[coderabbitai]

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

Run configuration

• Source mode: Auto
• Repository: pingdotgg/t3code
• Branch: factory/main-eee5f6f8ca
• File: .coderabbit.config.ts
• Commit: 146c6bd
• Layers: Repository TS config
• Review profile: CHILL
• Plan: Team
• Run ID: 49e55320-daf2-4a85-afc6-cd9d2baac8ec

Commits

Reviewing files that changed from the base of the PR and between 93b5833 and 146c6bd.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• Trigger review

No actionable comments were generated in the recent review.

Walkthrough

The cache sweep test removes its explicit-GC setup and weak-reference tracking. It marks sweep data with a Map subclass and uses NodeV8.queryObjects to assert that no instances remain.

Changes: Cache sweep test

Layer / File(s)	Summary
Sweep data lifetime assertion apps/server/src/pullRequest/PullRequestReadCache.test.ts	The test removes the node:vm GC setup and weak-reference count. It uses a SweepData subclass of Map and checks the instance count with NodeV8.queryObjects.

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

Change: Other

Possibly related PRs

• pingdotgg/t3code#25285: Adds the earlier weak-reference sweep-data test and changes PullRequestReadCache.ts.
• pingdotgg/t3code#25267: Adds the weak-reference regression test that this change replaces.

Suggested reviewers: yuruzuu, macroscopeapp

Merge Risk: Minimal · previous review 93b58

This change only updates a test and does not affect product behavior. No actionable merge-blocking risk remains.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title names the main change: the PullRequestReadCache test now counts leaked caller data without WeakRefs. It follows the conventional commit format and matches the summary and objectives, which d…
Description check		The description covers the problem (a flaky test caused by the test's own WeakRef), the change (SweepData marker class and v8.queryObjects count), and verification with specific before/after results. …
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Title check

Explanation

The title names the main change: the PullRequestReadCache test now counts leaked caller data without WeakRefs. It follows the conventional commit format and matches the summary and objectives, which describe replacing the WeakRef-based check with a SweepData marker class and v8.queryObjects.

Full details: Description check

Explanation

The description covers the problem (a flaky test caused by the test's own WeakRef), the change (SweepData marker class and v8.queryObjects count), and verification with specific before/after results. It omits a Scope section linking an issue or stating why no issue is needed, and the Reproduction on main section is a bare instruction without steps. The Evidence section is an unusual addition. The Problem and Change sections are present as headings but are not labeled to match the template's sections exactly. Overall the description is mostly complete, so it passes.

Finishing Touches

Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Recent review info

Run configuration

• Source mode: Auto
• Repository: pingdotgg/t3code
• Branch: factory/main-eee5f6f8ca
• File: .coderabbit.config.ts
• Commit: 93b5833
• Layers: Repository TS config
• Review profile: CHILL
• Plan: Team
• Run ID: 68410838-bd18-45a0-ad8a-1e8d7eecc199

Commits

Reviewing files that changed from the base of the PR and between cc97303 and 93b5833.

Files selected for processing: 1

• apps/server/src/pullRequest/PullRequestReadCache.test.ts

Limit details: You’ve used all 10 included reviews currently available.

Dismissing prior approval to re-evaluate 146c6bd