fix(server): PR discovery sweeps skip hidden subagent threads#26983

merged3

main

fix/issue-26970

View on GitHubTwo-way sync enabledPR last imported 5d ago

Problem

The one-minute PR discovery sweep (ThreadPullRequestServiceV2.synchronize) does a git.branchPullRequest lookup for every unsettled thread with a branch. That includes subagent and delegate children, which the sidebar hides and which only settle by inactivity after 3 days. Servers that run many agents end up with hundreds of hidden children in every sweep, which pins a core and drops remote clients.

Reproduced on main 20497e4890 with a fresh dev server (T3CODE_DEV_SEED=0) and one web client connected. The project's origin was github.com, with one parent thread that had no branch, three delegate_task children in their own worktrees, then a worktree parent with one shared-checkout child and one more worktree child. Each minute sweep made 5 lookups: one for the parent's group and one per hidden worktree child. When the cache expired, a single sweep ran 42 git commands.

Change

When synchronize runs as a sweep (threadId === null, so the startup backfill and the minute sweep), it drops threads with lineage.relationshipToParent === "subagent" before doing anything else. This covers both T3 delegate children and provider-native subagents, and it keeps children out of pendingBackfill. A child's own refresh is unchanged: the per-thread refresh enqueued after its run, checkpoint, metadata update, unsettle or unarchive still looks up its PR. Forks are still swept.

Scope and approval

Fixes pingdotgg/t3code#26970

Triaged via the automated issue flow.

Verification

New test ThreadPullRequestServiceV2 reads > leaves delegated child threads to their own refresh instead of the sweep in ThreadPullRequestService.test.ts. It drives the startup backfill sweep, one minute sweep via TestClock, and a checkpoint.captured event for the child.
- On main it fails: expected [ 'feature/parent', 'feature/parent', 't3/reply-ok', 't3/reply-ok' ] to deeply equal [ 'feature/parent', 'feature/parent' ].
- With the fix, all 7 tests in the file pass.
vp run check:changed: format, lint, knip, graphql, typecheck (3 packages) and tests (2 files) all pass.
Runtime transcript from the real dev server, with the same threads and database on both sides. Server-only fix, no visible surface, so no captures.

`branchPullRequest` lookups	Before (main 20497e4890)	After (5e9fc867cf)
startup backfill sweep	5	1
each minute sweep	5 (42 git commands on a cold cache)	1
wt-child's own refresh after its run	—	1 per refresh (still runs)

before-transcript.txt · after-transcript.txt · repro-driver.md · persweep.py

Not checked: a large real install with hundreds of children; the counts above come from a small synthetic setup.

Model: Claude Opus 5.5 (lead, test, implementation, review B) and GPT-6.1-Sol (review A). Harness: Claude Code and Codex in T3 Code.

Takeover

No code changes; evidence and verification added below. Earlier evidence measured 5e9fc867cf; both sides are re-measured here at this head.

Evidence

Server-only change, so each capture is an ordered runtime transcript from the real dev server, not a screenshot. Before 37fbefd (merge-base and main), after 45c1162. Both sides used the same driver on fresh databases (T3CODE_DEV_SEED=0). Children are genuine delegate_task children (shared checkout and isolation: worktree) plus one genuine provider-native subagent. Forks, settle, archive and metadata go through public RPC. Counts are completed branchPullRequest calls (L) and descendant git subprocesses (G), attributed by span ancestry under ThreadPullRequestServiceV2.synchronize.

`branchPullRequest` calls	Before 37fbefd	After 45c1162	Bundle
Startup sweep and each of six minute sweeps: root, fork of a child, three worktree delegates	5	2	A
Shared-checkout child, parent active (one group either way)	1	1	B
Shared-checkout child, parent settled after startup (app-owned and native, separately)	1	0	B
Settled records missing a PR ref, startup backfill; next minute	3; 0	2; 0	C
Settled records with a saved PR ref; archived records	0	0	C
Each child event: created, metadata, unarchive, unsettle, checkpoint, completed run	1	1	D
Existing PR, startup discovery + validation, root / child	2 / 2	2 / 0	D
No foreground client: startup; minute sweep; child event	2; gated; 1	1; gated; 1	E
Failing settled backfill (fault-injected `git remote`), attempts root / child	5 / 5	5 / 0	F

Environment: fs-18, Linux x86_64, Node v26.12.1, OpenCode 1.18.18 with anthropic/claude-haiku-5-5 as the provider. Before ran on port 19684 and after on 19685, each with its own home. Real one-minute scheduler, and a scripted WebSocket client reporting foreground activity. No browser.

What else changed:

A, minute 5 (five-minute cache expiry): git subprocesses 10 → 40. Before, that sweep only re-read git config. After, root and fork each did a full provider lookup because their no-open-PR result cache (PR_LOOKUP_NO_OPEN_PR_CACHE_TTL, 5 min) expired in that minute. The cache is keyed per cwd and branch, so child groups cannot affect it. This is cache-phase alignment between the two runs. Warm minutes are 0 G on both sides, and startup is 11 → 10 G.
E: a child's first own refresh is 1 L on both sides but 0 → 20 G. Before, the sweep had already warmed the child's caches. After, the child's own refresh does that work.
D: after the fix, the first child event for a known PR does discovery plus validation (1 → 2 L), because the sweep no longer found it first. Setup and control requests vary by a few git subprocesses with one lookup each. Every non-equal request is listed in count-differences.json.
Lookup counts per sweep go down. Total git work, CPU use and client disconnects at scale were not measured.

R12's failed, cancelled and interrupted selectors are replayed through the real service worker (labelled harness). F's fault is a scratch git wrapper (labelled harness). Everything else is genuine app behaviour.

evidence index (its sheet-D link predates the R11/R12 count correction; use the D link above) · raw traces, transcripts, scripts · reproduce.md · seed-all.py · driver.py · counter.py · fixture matrix

Verification log

Mode evidence-only: no code changes, no review rounds this round.
Proof audit pass 1 derived 19 rows (R01–R15 required, S16–S19 optional) in six bundles A–F: SUFFICIENT.
Capture covered R01–R15 on both SHAs; 69 internal consistency checks passed.
Proof audit pass 2 recomputed all 324 synchronize records from the raw trace (bundle A delta 0) and checked all 246 manifest hashes. It confirmed the cache-expiry and E explanations above. It found R11/R12 git counts swapped in sheet D (checkpoint 19 G, completed run 18 G). The sheet is corrected (linked above); the counts are equal on both sides and need no recapture.
Unverified: optional S16–S19 harness rows (guards, summary replacement, races, private pending-map transitions), optional R05 linked-environment destination, R06 field-only settlement, and R15 externally created PR variants. No hundreds-of-threads scale run.
CI on 45c1162: all required checks green.
Model: Claude Opus 5.5 (lead) and GPT-6.1-Sol (proof audit, capture). Harness: Claude Code and Codex in T3 Code.

🤖 Generated with Claude Code

Checks

23 successful · 17 skipped

All checks passed

Added 3 commits5d ago

test(server): reproduce #26970, PR sweep looks up delegated child threads1471407
Merge remote-tracking branch 'origin/main' into fix/issue-2697045c1162
fix(server): PR sweeps skip delegated child threads5e9fc86

MmacroscopeappBotapproved these changes5d ago

MmacroscopeappBot5d ago

Approvability

Verdict: Approved at 45c1162

Macroscope's review found this PR approvable — PR-discovery sweeps skip hidden subagent threads to reduce redundant Git/GitHub lookups. Child threads still refresh through their own events, and ordinary-thread sweeps remain unchanged, keeping the impact limited.

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

Ggithub-actionsBot5d 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.3 KiB	1.3 KiB	0 B (0.0%)	2.0 KiB	✅
Codex	Live turn WebSocket decoded	21.6 KiB	21.6 KiB	0 B (0.0%)	24.4 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.3 KiB	1.3 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	2	2	0 (0.0%)	8	✅

Baseline: 37fbefd · PR result: 45c1162 · 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.

CcoderabbitaiBot5d 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-26970
File: .coderabbit.config.ts
Commit: 45c1162
Layers: Repository TS config
Review profile: CHILL
Plan: Team
Run ID: 48aacb15-548a-4b20-af14-53c87ac5c872

External image Commits

Reviewing files that changed from the base of the PR and between 37fbefd147197fab9d88f9c120560c3f21b52cfc and 45c1162b91872e5e7a7ddd7843b24364caca0f4d.

External image Files selected for processing: 2

apps/server/src/orchestration-v2/ThreadPullRequestService.test.ts
apps/server/src/orchestration-v2/ThreadPullRequestService.ts

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

Walkthrough

Full pull-request sweeps now exclude subagent threads from backfill initialization, visible-thread cleanup, and thread selection. Targeted requests retain the unfiltered snapshot. A test covers parent sweeps and a child lookup after its checkpoint event.

Changes: Pull-request discovery

Layer / File(s)	Summary
Filter full sweeps and verify targeted lookups `apps/server/src/orchestration-v2/ThreadPullRequestService.ts`, `apps/server/src/orchestration-v2/ThreadPullRequestService.test.ts`	Full sweeps exclude subagent threads from backfill initialization, visible-thread cleanup, and selection. Targeted requests remain unfiltered. The test checks startup and periodic parent sweeps, then a refreshed child lookup after its checkpoint event.

Priority: External image Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: t3dotgg, macroscopeapp

Merge Risk: External image Minimal · up to 45c11

Full sweeps skip subagent threads while targeted refreshes remain available. No material merge risk was identified.

Pre-merge checks: ^Passed 4

Check name	Status	Explanation
Title check	Passed	The title clearly and concisely describes the primary change: skipping hidden subagent threads during PR discovery sweeps.
Linked Issues check	Passed	Issue `#26970` asks that the per-minute PR discovery sweep skip subagent and delegate child threads, or settle them with their parent. The change skips them. In ThreadPullRequestService.ts, `synchronize…
Out of Scope Changes check	Passed	The whole-PR diff against merge base 37fbefd1 touches two files. ThreadPullRequestService.ts adds one filter (`snapshotThreads`) and replaces `threadSnapshot.threads` with it in the backfill, visible-…
Description check	Passed	The description includes all required sections. It clearly explains the problem, implementation, scope, linked issue, focused regression test, verification results, limitations, runtime evidence, and …

Full details: Linked Issues check

Explanation

Issue #26970 asks that the per-minute PR discovery sweep skip subagent and delegate child threads, or settle them with their parent. The change skips them. In ThreadPullRequestService.ts, synchronize builds snapshotThreads by filtering out threads where lineage.relationshipToParent === "subagent" when request.threadId === null (startup backfill and minute sweep). The filtered list feeds the backfill loop, visibleThreadIds, and thread selection, so these children never enter pendingBackfill or branchPullRequest lookups during sweeps. The contract type OrchestrationV2AppThreadLineage.relationshipToParent is NullOr(Literals(["fork","subagent"])), so the comparison is type-valid and root threads (null) remain swept. Per-thread requests (threadId !== null) keep the unfiltered snapshot, so the child's own refresh after a run, checkpoint, or unsettle still looks up its PR. The new test leaves delegated child threads to their own refresh instead of the sweep checks both behaviors: two parent lookups across startup and one minute sweep, then one refresh: true child lookup after checkpoint.captured. Fork threads are unchanged. Residual uncertainty: the issue groups "subagent / delegate" children. This review did not inspect the delegate_task creation path to confirm that delegate children are stored with relationshipToParent "subagent". That mapping rests on the PR description and its runtime transcripts, which were not readable from this review.

Full details: Out of Scope Changes check

Explanation

The whole-PR diff against merge base 37fbefd1 touches two files. ThreadPullRequestService.ts adds one filter (snapshotThreads) and replaces threadSnapshot.threads with it in the backfill, visible-thread, and selection paths. Each replacement serves the same goal: exclude subagent children from sweeps. ThreadPullRequestService.test.ts adds one regression test for that behavior. The PR description's claimed checks (7 tests passing, check:changed) were not rerun in this review. No unrelated code, configuration, or refactoring appears in the reviewed changes.

Full details: Description check

Explanation

The description includes all required sections. It clearly explains the problem, implementation, scope, linked issue, focused regression test, verification results, limitations, runtime evidence, and agent details. The scope section identifies automated triage but does not include an explicit maintainer approval comment; this is a minor omission for an otherwise complete description.

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

JJulius Marminge5d ago

Taking over this one

Aaravhawkreviewed5d ago

A question about children that share their parent's checkout.

The subagent filter runs before Map.groupBy, so a child with the same [projectId, worktreePath, branch] as its parent is now dropped from the parent's group too. Before this change, that child rode along on the parent's single branchPullRequest lookup, so it cost nothing extra (your table counts one lookup for the parent's group plus one per worktree child), but it did pick up the PR link once the branch had one. Now it only gets the link on its own next run, checkpoint or metadata update. The startup backfill also no longer adds settled children to pendingBackfill.

The case I'd worry about is a child that pushes the branch and finishes, after which the parent or the user opens the PR. The child's branchPullRequest stays empty until it runs again, which may be never. Is that intended because nothing reads a hidden child's link? Or would it be worth keeping children whose group key matches a non-subagent thread in the sweep, and only dropping the ones that would need a lookup of their own? That would keep the 5 to 1 drop in your repro.

Related: #26704 lets delegated children watch their own PR. Does that path depend on the child's link being populated?