Skip to content

feat(engine,client): session fork with lazy provider re-binding - #524

Open
Zerlight wants to merge 10 commits into
ruocheng/code-638from
ruocheng/code-639
Open

feat(engine,client): session fork with lazy provider re-binding#524
Zerlight wants to merge 10 commits into
ruocheng/code-638from
ruocheng/code-639

Conversation

@Zerlight

@Zerlight Zerlight commented Sep 8, 2026

Copy link
Copy Markdown
Member

Summary

Phase 5 of CODE-627 — Conversation turn graph & immutable attachment store. Linear: https://linear.app/arcbox/issue/CODE-639/featengine-session-fork-with-lazy-provider-re-binding

Stack: #517this PR (ruocheng/code-639, base ruocheng/code-638) ← #525. Merge bottom-up; this PR's diff is only its own commits.

session.fork { sourceSessionId, throughTurnId, operationId, expectedGraphRevision } forks a session through a completed turn onto a provider-native copy: the child holds the chosen prefix (the selected turn included, its suffix excluded), shares current file state, and never re-executes history. The saga mirrors turn.submit: replay by operation id → admit under the source's critical section (typed busy while a turn runs or another operation is open; the through turn must be completed with a usable checkpoint; the graph revision must match) and persist the open operation → provider fork and child adapter start outside it under the launch budget, the child record held provisionally so its session-ref and status bind before it exists durably → one transaction commits the child record, its copied prefix (new turn ids, shared prompt and attachment references) and the operation. The source is never stopped or switched. Copied turns start with no child-history binding; one cold read re-derives bindings only when the user-row count matches exactly and per-position prompt fingerprints agree — any mismatch leaves all bindings absent, because a wrong binding forks at a wrong cut. The copied prefix renders from the source history while it exists (claude's forkSession re-stamps the copied rows) and from the child's own copy once the source is gone. Adapters: claude announces the child session-ref at fork and copies subagents/; pi announces its resumed or branched id at start. Client: forkSession behind a wire-version gate, "Fork a new thread from here" on completed turns (withheld while a fork is in flight), dev mock parity.

Commits

  • feat(schema): add the session.fork wire frames and the fork operation kind
  • feat(engine,daemon): persist fork commits and hold provisional session records
  • feat(engine): fork a session through a completed turn onto a provider-native copy
  • feat(agent-adapter): announce a fork child before its first prompt and carry claude subagent transcripts
  • feat(client-core,sdk): expose session.fork behind a wire-version gate
  • feat(workbench,ui): fork a new thread from an agent turn
  • fix(engine): render a forked session's copied prefix from its source history while the source exists
  • fix(engine): resolve a fork child under its own id, allow its hidden prefix, and keep it out of notifications until it commits
  • fix(workbench,client-core): hold the fork affordance while a fork is in flight and carry the child's MCP warnings

Verification

Every commit passed pnpm check:ci and pnpm test at its own tip; the tip (27ac29b9) is at pnpm check:ci 0 errors, pnpm test 3463 passed / 1 skipped. Adversarial review pair (engine/store/saga axis and client/adapter axis, isolated read-only worktrees): both ACCEPT-WITH-FINDINGS, no P1; each finding was reproduced with a failing test before its fix — the record is on CODE-639. Real development daemon with live Claude Code, no paid turn for the fork itself: a wire probe forked an existing thread through its second turn in 1.7 s, and a headless-Chrome probe of the webview drove the same fork from the UI; a later run on CODE-640 forked a live source on a managed worktree against the real provider (distinct claude history, copied prefix rendered from the source while it existed and from the child's copy after the source was deleted). Not exercised on a real provider: editing a copied-prefix turn after re-binding; a fork while the source is mid-turn is refused typed busy by design. Deferred, recorded on the issue: opencode tip-fork stays unsupported until CODE-633 verifies it; codex binding copy; provider-native lineage metadata; mobile.

Checklist

  • pnpm check:ci and pnpm test both pass (no Rust changes)
  • I ran the affected surface and observed the change working — the webview against the development daemon and live Claude Code (fork from the UI; the child conversation renders the copied prefix)
  • Wire: bumped 81 → 82 (additive session.fork / session.forked frames and the session.fork operation kind); floor unchanged at 76
  • New code and assets are my own work
  • Docs and comments are updated where behavior changed (packages/host/agent-adapter/AGENTS.md and module docs in this branch)

@linear-code

linear-code Bot commented Sep 8, 2026

Copy link
Copy Markdown

CODE-639

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ Minor suggestions inline.

Reviewed changes — the full 48-file diff at 27ac29b, plus the surrounding source for every claim below: fork-service.ts end to end, session-record-registry.ts, checkpoint-service.ts, lineage-attribution.ts, lineage.ts, use-workbench-sessions.ts, both adapter changes, and the installed pi SDK .d.ts/.js to check the session-ref emission. Scoped gates run green: vitest run packages/host/engine packages/host/agent-adapter/src/__tests__ packages/presentation/ui/src/chat packages/client/workbench packages/foundation/schema → 223 files, 2058 tests, 0 failures.

The saga holds up

The fork critical section is the part most likely to go wrong, and it doesn't. operationId idempotency, the per-session semaphore, expectedGraphRevision optimistic concurrency, and the Effect.onExitabandon compensation together mean a failed fork leaves no listed session and no persisted row. Provisional records being in-memory, unlisted, unannounced and unpersisted until the fork transaction commits is the right shape for that — the child only becomes real when the graph copy does.

Two things I specifically checked rather than assumed:

  • The all-or-nothing backfill gate in checkpoint-service.ts:124-131 is sound. copiedPrefix only suppresses durable binding writes; attribution is still returned, so rendering is unaffected and a partially-attributed copied prefix never gets half its turns permanently bound to the wrong provider messages.
  • The provisional record identity makes the orphan log resolve. registerProvisional stores the same object reference bindHistoryId later mutates in place, so child.runs[0]?.historyId in abandon reads the bound id rather than undefined.

Wire 82 is correctly additive

WIRE_PROTOCOL_VERSION 81 → 82 with MIN_COMPATIBLE_WIRE_VERSION held at 76 is right: session.fork is a new variant, nothing was removed, renamed, or re-meaned. The client gates on SESSION_FORK_WIRE_VERSION rather than assuming, so an older daemon degrades to "no fork affordance" instead of a dropped frame.

Things I chased and cleared

Recording these so they don't get re-litigated on the next pass:

  • The fork affordance is enabled, not dead. lineage.ts:157-161 writes a lineageVersions entry for every path turn, with state: null for completed ones, so the gate resolves true on a normal agent reply. Where lineage is undefined, onForkTurn is also absent, so both gates fail together — consistent, not a half-wired button.
  • Fork errors do surface. useMutation(forkSession, { onError }) matches every sibling mutation in the file, and the doc comment ("Rejections propagate to the caller … and reach onError") is accurate. The .catch(noop) suppresses only the already-handled secondary rejection, not the primary one.
  • The pi session-ref emission change is required. Widening if (this.resumeFrom) to if (manager) looks like it might announce on fresh sessions, but manager is truthy only on the branch or resume paths. The old code emitted nothing on the branch path, which violated the AGENTS.md contract that pi "announce the resumed/branched id at start". AgentSession.sessionId delegates to sessionManager.getSessionId(), which preserves the file's header id on open and assigns a new one on branch — so resumes still announce the same id and branches announce the new one.

Scope and follow-ups

This is stacked on ruocheng/code-638, so the diff is this PR's commits only and merge order matters. Deferred work that I'd want tracked rather than forgotten: opencode forkAfterTurn (CODE-633), the codex binding copy, provider-native lineage metadata, and mobile. One behavioral consequence worth naming in the PR description — an open fork operation blocks submit and stop on the source session via hasOpenOperation for up to LAUNCH_TIMEOUT_MS (300000 ms / 5 min) if the child launch hangs. That's the correct trade for consistency, but it is a five-minute user-visible freeze in the bad case.

The one coverage gap I noticed: nothing exercises editing a turn inside a copied prefix after re-binding against a real provider. The unit tests cover the binding logic and the fork saga separately, but not that specific interaction.

Nitpicks

  • engine-session-fork.test.ts re-declares startedHarness, twoCheckpointedTurns, ForkingAdapter, cursorRow, assistantRow, and Harness, all of which already exist in engine-turn-submit.test.ts or engine-conversation-read.test.ts. A shared test-helper module would keep the three suites from drifting apart.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread packages/host/agent-adapter/src/native/claude-code.ts
Comment thread packages/host/engine/src/session/fork-service.ts
Copilot AI lite review requested due to automatic review settings September 10, 2026 08:14
@Zerlight

Copy link
Copy Markdown
Member Author

Review round (2026-09-10). Both inline threads addressed in c604564 (fix(agent-adapter,engine)): the subagent-transcript copy now throws when a transcript cannot be located — concretely reachable with CLAUDE_CONFIG_DIR, which the SDK honours and the helper's ~/.claude/projects lookup did not — so the existing catch reports it as fork_subagents_not_copied; the abandon warning logs the child's own id under sessionId and the source under sourceSessionId. Gates at c604564: pnpm check:ci 0 errors, pnpm test 3459 passed.

Recorded on CODE-639 rather than changed here: the source freeze while a child launch hangs (an open fork operation blocks submit and stop on the source for up to LAUNCH_TIMEOUT_MS, 5 min — the consistency trade), the copied-prefix edit against a real provider (already the deferred paid check), the helper duplication across the three engine fork/submit/read suites, and the hardcoded projects path.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes — the delta since the prior review at 27ac29b is the single commit c604564, touching three files; both inline threads from that review are addressed and resolved.

  • Made a skipped subagent copy loudcopyClaudeSubagentTranscripts now throws when either transcript directory cannot be located or an id is not a path segment, and reserves false for the one benign case (the source has no subagents/). The caller's existing catch turns every real miss into the recoverable fork_subagents_not_copied event that AGENTS.md already promised.
  • Named the abandoned fork child — the orphan warning in SessionForkService.abandon now logs sessionId: child.sessionId with the source under sourceSessionId, matching the sibling logError twelve lines above. Confirmed in the test run's own output: sessionId: 'sess-…-2', sourceSessionId: 'sess-…-1'.
  • Tightened the test to the new contractclaude-code-fork-subagents.test.ts pins each rejection message exactly instead of the false production used to discard. These assertions fail if any of the three throws is removed.

I specifically checked the one thing that could have made the new throw a regression: whether forkSession can resolve before the child's <sessionId>.jsonl is on disk, which would turn an ordinary fork into an error card under the default configuration. It cannot — the installed @anthropic-ai/claude-agent-sdk 0.3.215 copies the transcript in-process through a createWriteStream writer and awaits its finish event before resolving (sdk.d.ts:667-699 describes the copy; the awaited write lives in the sdk.mjs bundle). copySubagentTranscripts runs strictly after that await, so the child lookup is not racy.

Scoped gates green: vitest run over claude-code-fork-subagents.test.ts, claude-code-checkpoint.test.ts, and engine-session-fork.test.ts → 3 files, 25 tests, 0 failures.

ℹ️ Until the projects-dir follow-up lands, a relocated CLAUDE_CONFIG_DIR turns every claude fork into an error card

This is the deliberate consequence of the fix rather than a defect in it, and the hardcoded path is already recorded on CODE-639 — flagging only so the interim shape is a decision rather than a surprise. With CLAUDE_CONFIG_DIR set, the SDK writes both transcripts under $CLAUDE_CONFIG_DIR/projects while copySubagentTranscripts looks under homedir()/.claude/projects, so both lookups miss and the throw fires on every fork — including forks whose source had no subagents at all, where nothing was actually lost. Previously that configuration was silent.

Technical details
# `CLAUDE_CONFIG_DIR` makes the new throw fire unconditionally

## Affected sites
- `packages/host/agent-adapter/src/native/claude-code.ts:713``copySubagentTranscripts` hardcodes `path.join(homedir(), '.claude', 'projects')`.
- `packages/host/agent-adapter/src/native/claude-code.ts:2041-2044` — throws when `sourceDir` is undefined, which is the "wrong projects root" case, not the "child lost its subagents" case the prior review named.
- `packages/host/agent-adapter/src/native/claude-code.ts:1991``readClaudeProjectText` shares the same hardcoded root, so compaction supplements and cold subagent reads already degrade silently in this configuration.

## Required outcome
- No change required for merge. The interim behavior is a conscious trade: a false-positive error card in a non-default configuration, in exchange for never silently dropping real subagent transcripts.

## Open questions for the human
- Is the error card acceptable for the window before the follow-up? If not, the narrow interim option is to throw only when `sourceDir` was found but `childDir` was not — the source being absent means the root is wrong or the source is gone, and in both cases there is nothing this fork can recover.
- When the follow-up does land, note that reading `process.env.CLAUDE_CONFIG_DIR` in the daemon is not sufficient: per `AGENTS.md`, claude runs under the user's resolved login-shell environment (plus `direnv`), so the variable can reach the CLI without ever appearing in the daemon's own environment. The resolved session environment is the correct source.

Pullfrog  | View workflow run | Using Claude Opus𝕏

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Two moderate replay-validation findings remain unresolved.

Pull request overview

Adds provider-native session forking through completed turns across the schema, engine, adapters, client, and UI.

Changes:

  • Adds session.fork wire/API support and the fork saga.
  • Implements copied-prefix projection and lazy history rebinding.
  • Adds adapter, SDK, workbench, UI, translation, test, and mock support.
File summaries
File Reviewed change / final comment
packages/presentation/ui/src/shell/conversation-surface.tsx Connects lineage-aware fork handling.
packages/presentation/ui/src/chat/types.ts Defines fork-related chat types.
packages/presentation/ui/src/chat/turn-segment-view.tsx Enables fork actions for eligible turns.
packages/presentation/ui/src/chat/turn-actions.tsx Renders fork actions.
packages/presentation/ui/src/chat/conversation-view.tsx Propagates fork callbacks.
packages/presentation/ui/src/chat/__tests__/turn-actions.test.tsx Tests fork action states.
packages/presentation/i18n/src/locales/zh-cn.ts Adds Chinese fork strings.
packages/presentation/i18n/src/locales/en.ts Adds English fork strings.
packages/host/engine/src/wire/request-router.ts Routes fork requests.
packages/host/engine/src/session/session-record-registry.ts Maintains provisional child records.
packages/host/engine/src/session/session-event-processor.ts Processes session lifecycle events.
packages/host/engine/src/session/request-handler.ts Handles session requests and admission.
packages/host/engine/src/session/lifecycle-service.ts Manages operation lifecycle and replay.
packages/host/engine/src/session/fork-service.ts Coordinates the fork saga. Moderate (1 vote): replay must validate the operation kind, with symmetric validation in submitTurn.
packages/host/engine/src/index.ts Exports engine fork support.
packages/host/engine/src/engine.ts Integrates fork behavior into the engine.
packages/host/engine/src/conversation/turn-service.ts Coordinates turn and conversation operations.
packages/host/engine/src/conversation/projection-service.ts Projects copied fork prefixes.
packages/host/engine/src/conversation/lineage-attribution.ts Attributes copied-turn lineage.
packages/host/engine/src/conversation/conversation-store.ts Stores forked conversation state.
packages/host/engine/src/conversation/checkpoint-service.ts Resolves checkpoints and lazy bindings.
packages/host/engine/src/__tests__/session-record-registry.test.ts Tests provisional session records.
packages/host/engine/src/__tests__/engine-session-fork.test.ts Tests engine session forking.
packages/host/engine/src/__tests__/conversation-store.test.ts Tests conversation-store persistence.
packages/host/agent-adapter/src/native/pi/adapter.ts Announces resumed or branched Pi IDs.
packages/host/agent-adapter/src/native/claude-code.ts Implements Claude fork and subagent copying.
packages/host/agent-adapter/src/__tests__/claude-code-fork-subagents.test.ts Tests Claude subagent copying.
packages/host/agent-adapter/src/__tests__/claude-code-checkpoint.test.ts Tests Claude checkpoint handling.
packages/host/agent-adapter/AGENTS.md Documents adapter behavior.
packages/foundation/schema/tests/contract/wire/session.test.ts Tests session wire contracts.
packages/foundation/schema/src/wire/session.ts Defines session fork wire frames.
packages/foundation/schema/src/wire/message.ts Updates wire message definitions.
packages/foundation/schema/src/wire/index.ts Exports wire types.
packages/foundation/schema/src/model/conversation.ts Defines conversation fork model data.
packages/client/workbench/tests/integration/dev-mock-lineage.test.ts Tests mock lineage behavior.
packages/client/workbench/src/surface/workbench.tsx Dispatches workbench fork actions.
packages/client/workbench/src/surface/use-workbench-sessions.ts Tracks workbench fork session state.
packages/client/workbench/src/mock/dev-mock-host.ts Mirrors fork behavior in the mock. Moderate (1 vote): replay entries must retain and validate the source session before returning a child.
packages/client/workbench/AGENTS.md Documents workbench behavior.
packages/client/sdk/src/operations.ts Exposes the fork operation in the SDK.
packages/client/sdk/src/client.ts Provides the SDK fork API.
packages/client/core/tests/integration/conversation-client.test.ts Tests client fork integration.
packages/client/core/src/client/pending-registry.ts Tracks pending fork requests.
packages/client/core/src/client/control-channel.ts Correlates fork control frames.
packages/client/core/src/client.ts Integrates fork support into client core.
apps/daemon/src/session-store.ts Persists daemon session records.
apps/daemon/src/conversation-store.ts Persists daemon conversation fork state.
apps/daemon/src/__tests__/conversation-store.test.ts Tests daemon conversation persistence.
Review details

Suppressed comments (2)

packages/client/workbench/src/mock/dev-mock-host.ts:1301

  • The mock's replay journal stores only the child id, so reusing an operation id with a different source session returns the previous session instead of the engine's invalid_request result. Keep the original source session with each replay entry and validate it before returning the child to preserve the idempotency contract.
    const replayed = this.forkOperations.get(p.operationId);
    if (replayed !== undefined) {
      this.send({ kind: 'session.forked', replyTo: p.clientReqId, sessionId: replayed });
      return;

packages/host/engine/src/session/fork-service.ts:89

  • The operation journal is shared by turn.submit and session.fork, but this replay guard only checks the session id. A colliding submit operation can enter the fork path and have its terminal result interpreted as a fork; validate existing.kind === 'session.fork' here and symmetrically in submitTurn before replaying.
  • Files reviewed: 48/48 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants