feat(mcp): fork existing conversations#8696

mcp/ConversationTransferMcpService.ts:45

New service modules should declare their interface inline in the Context.Service call rather than keeping a standalone ...ServiceShape type. Nothing else references ConversationTransferMcpServiceShape, so it can be inlined; consumers that need the type can use ConversationTransferMcpService["Service"].

No longer relevant as of 99b0ed269816569abcf9033ab6083a44642122a5

🟡 Medium mcp/ConversationTransferMcpService.ts:171

Projection/database failures from getProjectThread are returned to MCP clients as thread_not_found, so transient server errors are treated as missing threads. isThreadManagementError(error) matches both ThreadManagementThreadNotFoundError and ThreadManagementProjectionLoadError; narrow this branch to ThreadManagementThreadNotFoundError so projection failures reach the orchestration_error fallback.

✅ Resolved in 99b0ed269816569abcf9033ab6083a44642122a5

🟠 High mcp/ConversationTransferMcpService.ts:206

A fork can be created with a runtime or interaction mode above the caller's current ceiling if a thread.runtime-mode.set or thread.interaction-mode.set runs after these checks. threads.dispatch reloads the source projection and ThreadForkService copies that newer configuration into the target, so the earlier parent/source snapshots are a TOCTOU check; enforce the ceilings atomically during dispatch (or in dispatchThreadFork) using the current projections.

✅ Resolved in 99b0ed269816569abcf9033ab6083a44642122a5

🟡 Medium mcp/ConversationTransferMcpService.ts:205

Retrying a committed fork with the same required clientRequestId fails instead of returning the original receipt and transfer when the source is deleted or its runtime/interaction mode changes. loadScoped and the ceiling checks run before constructing and dispatching the stable commandId, so mutable preflight prevents idempotent replay; resolve an existing committed command by commandId before these checks, or otherwise bypass preflight for an already-committed request.

✅ Resolved in 99b0ed269816569abcf9033ab6083a44642122a5

Fork source resolution diverges from orchestrator

Medium Severity

sourceRun does not match orchestrator runForSourcePoint. latest_stable accepts any completed run, while dispatch requires a completed run with a checkpointId. A later uncheckpointed run can pass the pre-check, then either fail dispatch or fork an older run while nativeEligibility is computed against the wrong one. checkpoint looks up run.checkpointId instead of projection.checkpoints, so historical checkpoints are rejected even though thread.fork accepts them.

^{Reviewed by Cursor Bugbot for commit b26872873df04cf9c8c7ae9cde030f86eaf5220e. Configure here.}

Thread load errors reported as missing

Low Severity

loadScoped maps every ThreadManagementError from getProjectThread to thread_not_found, including ThreadManagementProjectionLoadError. A transient projection failure is reported as a missing thread, unlike OrchestratorMcpService.threadManagementFailure, which maps load errors to orchestration_error.

^{Reviewed by Cursor Bugbot for commit b26872873df04cf9c8c7ae9cde030f86eaf5220e. Configure here.}

Fork thread IDs collide with create

Medium Severity

Fork target IDs use thread:mcp:{session}:fork:{key}, which occupies the same four-segment shape as create_threads IDs (thread:mcp:{session}:{requestKey}:{index}). A create_threads call with clientRequestId fork and a later t3_thread_fork with a numeric key such as 0 resolve to the same ThreadId. The create path then dispatches thread.fork onto an existing ordinary thread, which can overwrite that thread as a fork.

^{Reviewed by Cursor Bugbot for commit 99b0ed269816569abcf9033ab6083a44642122a5. Configure here.}

Fixed in 125913a4f2. Fork targets now use a structurally disjoint conversation-fork:target namespace, and the serialized fork decision rejects any pre-existing target before planning. Integration coverage reproduces create_threads(key=fork,index=0) plus t3_thread_fork(key=0), verifies distinct threads, and proves an occupied target remains unchanged.

🟡 Medium mcp/ConversationTransferMcpService.ts:123

nativeEligibility returns "eligible" for a completed fork source with a strong native thread reference but no providerTurnRef, even when session.capabilities.threads.canForkFromTurn is false. The target's first turn resolves that source run as fromSpecificTurn and falls back to portable context, so the advertised native eligibility is incorrect; check canForkFromTurn for every resolvable source run.

Fixed in 125913a4f2. Every resolved durable source run is now classified as a specific-turn fork, matching the first-target-turn policy decision, so canForkFromTurn is always required for native eligibility. Focused coverage includes a strong native source with no providerTurnRef and canForkFromTurn=false.

orchestration-v2/Orchestrator.ts:2072

Prefer Effect.catchTags({ ... }) for statically known tagged failures, even when handling a single tag — catchTag isn't the convention here (the rest of the repo uses catchTags, e.g. GitVcsDriverCore.ts).

No longer relevant as of ef0aa127e9a5c23370bd00ef99ef0f71b0217524

Shared MCP key schema has description

Medium Severity

WellFormedRequestKey is a reusable MCP input schema that carries its own description plus filter/isMaxLength annotations. Used as clientRequestId on t3_thread_fork, that composition can emit multiple described allOf members and cause some providers to reject the tool schema on every turn.

^{Triggered by learned rule: MCP tool input schemas: only one description per allOf branch in Effect Schema compositions}

^{Reviewed by Cursor Bugbot for commit 7577ec0bb0925bc1aa823f98b433f7a7d0fc7a8c. Configure here.}

Verified against the exact production serializer used by McpHttpServer: Tool.getJsonSchema for t3_thread_fork emits clientRequestId as { "type": "string", "allOf": [{ "maxLength": 256 }] }. It contains zero description members, and the overall input remains a root object. The source annotation is not duplicated into described allOf branches by this Effect version, so the reported provider-rejection shape is not present. No schema change is warranted.

Retry probe swallows projection errors

Medium Severity

Effect.option treats every getThreadProjection failure as a missing target, so a retry can miss an existing fork transfer and fall through into the first-create permission checks. After the caller ceiling has been lowered, that path denies a retry that should have replayed the original receipt.

^{Reviewed by Cursor Bugbot for commit 7577ec0bb0925bc1aa823f98b433f7a7d0fc7a8c. Configure here.}

Fixed in 9fb708321bbe4717656aba85f98a93e43bfde159. The target probe now treats only an underlying ProjectionStoreThreadNotFoundError as absence; other projection failures return the safe, retryable orchestration_error before source planning, policy checks, or dispatch. The focused regression injects an operational projection failure and asserts that dispatch is never called. Accepted-retry behavior remains unchanged when projection observation is available.

Triplicated rank functions risk inconsistent permission checks

Medium Severity

runtimeModeRank and interactionModeRank now exist in three separate files: ConversationTransferMcpService.ts, OrchestratorMcpService.ts, and Orchestrator.ts. Two of these three copies are newly added in this PR. These identical functions drive permission-ceiling enforcement at different layers — a future mode addition or ranking change updated in only one or two copies would silently produce inconsistent escalation checks. Extracting them into a shared utility would prevent that drift.

^{Reviewed by Cursor Bugbot for commit 6f3e62696d120c88207290321ec42dde4d75a488. Configure here.}

Verified against 6f3e62696d120c88207290321ec42dde4d75a488: all three rank tables are currently identical and exhaustive for the closed RuntimeMode and ProviderInteractionMode unions, so there is no inconsistent permission decision in this head. The copies sit at separate MCP and durable command boundaries intentionally; extracting a new cross-layer utility here would be speculative architecture churn rather than a correctness fix. We will keep this scoped and update all boundaries together if either contract gains a mode.