Skip to content

fix(server): recover side-chat fork history from its parent - #28849

Open
ScottN-PV wants to merge 1 commit into
pingdotgg:mainfrom
ScottN-PV:fix/28809-side-chat-recovery-thread
Open

ScottN-PV wants to merge 1 commit into
pingdotgg:mainfrom
ScottN-PV:fix/28809-side-chat-recovery-thread

Conversation

@ScottN-PV

Copy link
Copy Markdown
Contributor

Problem

When a side chat starts through portable context, its handoff tells the agent to recover omitted history with t3_thread_read on the side chat itself. A side chat excludes parent timeline rows, so a recovery hint that names the side chat cannot reach the parent history it lists.

Change

Newly prepared portable side-chat handoffs now name the parent thread in their history coverage and recovery hint. The side chat still owns the handoff and its target run. Regular portable forks keep naming the fork, because a fork shows the rows it inherits.

Scope and approval

Refs #28809. This is a small, focused fix for an obvious bug under CONTRIBUTING's exception for fixes without prior triage. The cause does not depend on the operating system.

The change covers recovery instructions only. The same-model label, transfer-limit wording, #27072 cursor cost, compact overflow fallback and already-persisted handoffs are unchanged.

Verification

Server-only change. The client renders the handoff data the server stores.

  • vp test run on apps/server/src/orchestration-v2/ContextHandoffService.test.ts, Orchestrator.control-reads.test.ts, Orchestrator.migration.test.ts and ThreadFork.execution.test.ts: 34 of 34 pass.
  • With ContextHandoffService.ts and Orchestrator.ts reverted to 57d764ea1a, one test fails with an assertion error: "portable fork recovery uses the readable history thread (sideChat=true)". The sideChat=false case passes before and after.
  • vp fmt --check and vp lint report no errors and no new warnings on the 3 changed files. vp run --filter t3 typecheck passes.

Live run on Linux with real Codex: the parent finished one turn and was running a second, so the side chat took the portable path on its own. On main 57d764ea1a, the stored recovery hint names the side chat. All 3 listed history items exist in the parent and none exist in the side chat. On 633a0b6ed7, the hint names the parent, which holds all 3 items, and the side chat still owns the handoff. Both runs omitted zero items, so this shows where recovery points, not a full omitted-item recovery.

Live Codex run, before and after

Environment: Linux, Codex GPT-6.1-Sol, real signed-in Codex 0.166.0. Isolated T3 servers, no forced provider capabilities. No Windows/UI rendering claim. Thread and item identifiers below use role aliases for readability.

Setup on both builds

  1. Start an isolated server from the pinned checkout with node scripts/dev-runner.ts dev:server --home-dir <isolated-home> --no-seed.
  2. Create a project and launch a real Codex parent thread. Ask it to reply PARENT_HISTORY_28809 and wait for its first run to complete.
  3. Dispatch a second parent message asking it to run sleep 120.
  4. While that second run is active, fork latest_stable with sideChat: true and send the first child message. This naturally selects portable context.
  5. Read the stored full-thread handoff from the isolated database read-only; compare its listed history item ids with parent/child visibleTurnItems from the same projection used by the MCP activity reader.

Before: main 57d764e

Parent runs at observation: first completed; second running. Child first run completed.

Stored recovery instruction (thread id replaced with alias):

Recover omitted history using t3_thread_read({threadId:"<child>",view:"activity",limit:20,maxCharsPerItem:4000}); paginate with afterPosition=nextPosition.
  • Stored handoff owner: child.
  • Listed historical item count: 3.
  • Parent projection contains all 3 listed item ids.
  • Child projection contains none of those ids.
  • History omitted count: 0; the emitted recovery hint still names the wrong thread for the source item range.

After: 633a0b6

Fresh isolated state and new real Codex parent/child; same setup. Parent first run completed and second running.

Stored recovery instruction (thread id replaced with alias):

Recover omitted history using t3_thread_read({threadId:"<parent>",view:"activity",limit:20,maxCharsPerItem:4000}); paginate with afterPosition=nextPosition.
  • Stored handoff owner: child; its target run belongs to the child.
  • Listed historical item count: 3.
  • Parent projection contains all 3 listed item ids.
  • Child projection contains none of those ids.
  • History omitted count: 0.

The regular portable-fork scope control is covered by the focused execution test. Same-model labels, transfer limits, overflow fallback, pagination cost and existing handoffs were not changed.

Model: GPT-6.1-Sol, Claude Opus 5.5 (review), GPT-6-Astra (review). Harness: Codex, Claude Code.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Nov 1, 2026
@macroscopeapp

macroscopeapp Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 633a0b6

Macroscope's review found this PR approvable — Portable side-chat handoffs now point history recovery at the parent thread while remaining owned by the side chat. Regular forks and existing handoffs are unchanged, keeping the change narrowly scoped.

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

@coderabbitai

coderabbitai Bot commented Nov 1, 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 57d764e and 633a0b6.

Files selected for processing: 3
  • apps/server/src/orchestration-v2/ContextHandoffService.ts
  • apps/server/src/orchestration-v2/Orchestrator.ts
  • apps/server/src/orchestration-v2/ThreadFork.execution.test.ts

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

Walkthrough

Provider handoffs can now use a separate thread for history coverage while remaining owned by the target thread. Side-chat dispatch passes the source thread for coverage, and fork recovery tests check side-chat and regular-fork behavior.

Changes: Provider handoff history

Layer / File(s) Summary
Separate history coverage from handoff ownership
apps/server/src/orchestration-v2/ContextHandoffService.ts
prepareProviderHandoff accepts an optional historyThreadId. Coverage uses it when supplied, but the handoff remains owned by threadId.
Side-chat wiring and fork recovery test
apps/server/src/orchestration-v2/Orchestrator.ts, apps/server/src/orchestration-v2/ThreadFork.execution.test.ts
Side-chat dispatch passes the source thread as historyThreadId. Tests check recovery handoffs for side chats and regular forks, including ownership and history coverage.

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

Change: Bug fix

Possibly related issues

  • pingdotgg/t3code#28809 — Reports side-chat recovery using the child thread when the history belongs to the parent. This change passes the source thread for side-chat handoff history coverage.

Suggested labels: bugfix, size:S

Suggested reviewers: t3dotgg, macroscopeapp

Merge Risk: Minimal · up to 633a0

New side-chat handoffs point recovery to the parent in normal delivery. The existing compact fallback remains unchanged.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title "fix(server): recover side-chat fork history from its parent" matches the main change. The PR makes newly prepared portable side-chat handoffs name the parent thread in their history coverag…
Description check Passed The description follows the template's main sections: Problem, Change, Scope and approval, and Verification. The Problem and Change sections clearly explain the bug and the fix. Scope and approval giv…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title "fix(server): recover side-chat fork history from its parent" matches the main change. The PR makes newly prepared portable side-chat handoffs name the parent thread in their history coverage and recovery hint, and the objectives describe the same change. It uses a conventional commit format and is specific to the server component, so it passes.

Full details: Description check

Explanation

The description follows the template's main sections: Problem, Change, Scope and approval, and Verification. The Problem and Change sections clearly explain the bug and the fix. Scope and approval gives a focused-fix rationale and references issue #28809, but it does not include a triaged issue link or maintainer approval comment, and it relies on the CONTRIBUTING exception for obvious bugs. Verification is detailed: it lists the focused test files and results, the revert-based failing-test check, lint and typecheck results, and a live Codex run before and after. It also states what was not checked (Windows and UI rendering). The template's agent-model footer is present. The description is mostly complete, so it passes.

Finishing Touches
Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS 0-9 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.

1 participant