Repository navigation
feat(chat-fork): provider-generated handoff pipeline with B/A/D fallback ladder - #499
Merged
Merged
Conversation
- Claude: clean fork, 180k char budget (45k tokens with headroom) - Cursor: mutating fork, 4k chars (known per-turn cap) - Codex + Copilot: unsupported fork, 16k chars (conservative until verified) Also fold in three code-review followups on the interface file: - Fix em-dash in maxInputCharactersPerTurn JSDoc - Clarify conservative placeholder values for Codex/Copilot - Hoist SessionForkBehavior to a named type alias for clarity Update test mocks to include the new required fields.
…ser-msg fork Moves fork mode from ChatView local useState into threadStore so it survives thread navigation. On user-message fork, pre-fills the lexical editor with the quoted text styled italic so it is visually distinct from fresh input. Adds queued-send: messages typed while the handoff context is still generating are held locally and fired automatically when the handoff lands.
…send time Guards handleSend against sending a /cmd that doesn't exist in the active provider's command set, surfacing an error toast and blocking the send. The palette already sourced from effectiveProviderId (composer state for new threads, thread.provider for existing ones).
Adds a boolean setting (default true) that controls whether a banner is shown when a fork's handoff was produced by the local deterministic path. Surfaced as a toggle in the Notifications settings section.
…tion Renders an amber banner above the message list when a child fork thread's handoff was produced by the local deterministic path (path D). Respects the chat.handoff.notifyOnLocalFallback setting. Regenerate button is a disabled v1 stub.
Registers the method in WS_METHODS with typed params/result and adds the
case handler in ws-router returning { status: "not-implemented" }. Live
regeneration via AI provider is deferred to a follow-on plan.
…get; use resolve The local IProviderRegistry interface shadowed the real contracts type and declared get() instead of resolve(). All call sites now go through a private tryResolveProvider() helper that wraps resolve() to return null on miss rather than throwing. Test mocks updated from get to resolve; the deps type is now Pick<IProviderRegistry, "resolve"> so future interface drift is caught at compile time.
…adata strip Adds an X dismiss button to HandoffFallbackBanner that clears the thread's handoff status to "ready" so the banner won't reappear on navigation. The View doc dialog is widened to max-w-4xl and capped at 90vh. The viewer now renders a metadata strip (ladderStep, mode, generatedBy, provider, chars, when) from artifact.meta and strips YAML frontmatter from the markdown body so the doc text renders cleanly via MarkdownContent.
…ontroller.signal is read-only)
… session is unresumable
… overflow for oversized docs - Restructures SECTIONS_FULL/SECTIONS_MINIMAL to quote verbatim parent message and concrete key facts instead of abstract summaries - Adds userFollowUpMessage field to HandoffPromptInput and HandoffRequest so the side-channel prompt bridges parent context to the child's first question - Rewrites anchor-role framing: user-fork = retry framing, assistant-fork = follow-up-about-reply framing - Makes applyBudgetGuard async; on overflow writes the full doc to OS temp dir and inlines a truncated head + file pointer - Updates all test fixtures (BASE_REQ in pipeline + e2e tests, buildHandoffPrompt baseInput) and adds new assertions for follow-up message inclusion and stronger framing language
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (38)
📝 WalkthroughWalkthroughAdds a complete chat fork handoff system: provider capability contracts and hooks, DB migration and message-repo internal flag, filesystem handoff storage, HandoffPipelineService implementing B→A→D ladder (side-channel, hidden turn, deterministic fallback), provider-side implementations (Claude, Cursor, others), AgentService/server wiring, WebSocket transport and client APIs, UI state/components for fork mode and fallback banners, tests, and docs. ChangesChat Fork Handoff Implementation
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
|
…ncement # Conflicts: # apps/server/drizzle/meta/_journal.json # apps/web/src/components/chat/Composer.tsx
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/stores/threadStore.ts (1)
1403-1436:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPrune newly added per-thread maps during thread cleanup.
clearThreadStateandclearThreadStateManydon’t removehandoffMeta,handoffStatus, orforkMode. Deleted threads can retain stale fork/handoff UI state in memory.Suggested fix
return { runningThreadIds: nextRunning, + handoffMeta: omitKey(state.handoffMeta, threadId), + handoffStatus: omitKey(state.handoffStatus, threadId), + forkMode: omitKey(state.forkMode, threadId), errorByThread: omitKey(state.errorByThread, threadId), streamingByThread: omitKey(state.streamingByThread, threadId), ... };return { runningThreadIds: nextRunning, + handoffMeta: pruneAll(state.handoffMeta), + handoffStatus: pruneAll(state.handoffStatus), + forkMode: pruneAll(state.forkMode), errorByThread: pruneAll(state.errorByThread), streamingByThread: pruneAll(state.streamingByThread), ... };Also applies to: 1495-1528
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/stores/threadStore.ts` around lines 1403 - 1436, The cleanup functions clearThreadState and clearThreadStateMany are missing removals for the new per-thread maps handoffMeta, handoffStatus, and forkMode, leaving stale UI state; update the returned state object in both clearThreadState and clearThreadStateMany to include handoffMeta: omitKey(state.handoffMeta, threadId), handoffStatus: omitKey(state.handoffStatus, threadId), and forkMode: omitKey(state.forkMode, threadId) (and when clearing many, apply the same omission logic for each threadId) so those per-thread entries are pruned during thread delete.
🧹 Nitpick comments (1)
apps/web/src/transport/ws-transport.ts (1)
751-781: ⚡ Quick winDe-duplicate the handoff response type to prevent schema drift.
This large inline type repeats the transport contract shape and is likely to diverge over time. Derive it from
McodeTransport["readLatestHandoff"]instead.♻️ Suggested refactor
+type ReadLatestHandoffResult = Awaited< + ReturnType<McodeTransport["readLatestHandoff"]> +>; + ... - readLatestHandoff: (threadId: string) => - rpc<{ - markdown: string; - meta: { - schemaVersion: 1; - parentThreadId: string; - forkedFromMessageId: string; - forkAnchorRole: "user" | "assistant"; - childThreadId: string; - generatedBy: "provider" | "deterministic"; - provider: string | null; - ladderStep: "B" | "A" | "D"; - mode: "full" | "minimal"; - generatedAt: string; - characterCount: number; - parentSdkSessionId: string | null; - providerErrorOnGenerate: "quota" | "auth" | "context-overflow" | "transient" | "fatal" | "clean" | null; - regenerationHistory: Array<{ - at: string; - ladderStep: "B" | "A" | "D"; - reason: "quota" | "auth" | "context-overflow" | "transient" | "fatal" | "clean" | "user-requested"; - }>; - attachments: Array<{ - id: string; - originalName: string; - sha256: string; - mime: string; - parentMessageId: string; - }>; - }; - } | null>("handoff.readLatest", { threadId }), + readLatestHandoff: (threadId: string) => + rpc<ReadLatestHandoffResult>("handoff.readLatest", { threadId }),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/transport/ws-transport.ts` around lines 751 - 781, The inline, verbose response type in the readLatestHandoff call is duplicating the transport contract and risks schema drift; change the generic type passed to rpc to derive from the canonical transport type (use McodeTransport["readLatestHandoff"] or the appropriate member) instead of the long inline shape so the call signature stays in sync with the shared contract (ensure you preserve the union with null if the transport type allows null), update the rpc invocation around readLatestHandoff to use that referenced type and remove the inline object type.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/server/src/providers/claude/claude-provider.ts`:
- Around line 446-449: The code is using literal defaults for Claude models in
multiple places (e.g., the parentSessionId lookup that sets model to
"claude-sonnet-4-5" and the doSendMessage() default "claude-sonnet-4-6"),
causing drift; extract a single shared constant like DEFAULT_CLAUDE_MODEL and
replace the hardcoded strings in the parentSessionId resolution (where model is
set via this.sessions.get(parentSessionId)?.model) and the doSendMessage()
default so both paths reference DEFAULT_CLAUDE_MODEL (also update the other
occurrence around lines 581-583) to ensure a single canonical default.
In `@apps/server/src/providers/cursor/cursor-provider.ts`:
- Around line 929-945: runRawPrompt currently assigns a normal activeTurnState
(via createCursorAcpTurnState) so deliverSessionUpdate still maps and emits
hidden-turn events; change the turn state used for this hidden handoff to a
suppressed variant and ensure deliverSessionUpdate skips mapping for it: update
runRawPrompt to set entry.activeTurnState to a turn state with a suppression
flag (e.g., turnState.suppressed = true) or to use a separate hiddenAccumulator
field (e.g., turnState.hiddenAccumulator) and then modify deliverSessionUpdate
to check that flag/field and avoid emitting textDelta/tool/completion events for
suppressed turns; keep resolveCursorAssistantMessageContent using the hidden
accumulator so the function returns the content but no ACP events are propagated
to the parent thread.
- Around line 857-919: runHiddenTurn currently bypasses the per-session
serialization (entry.turnChain) and doesn't check abortSignal between steps;
enqueue the entire hidden-turn bracket onto the session's turnChain so it runs
under the same mutex as sendMessage(): use entry.turnChain =
entry.turnChain.then(async () => { ... }) (or the project's equivalent
promise-chaining helper) and move the logic for openLogicalSession,
messageRepo.create calls, runRawPrompt calls, and final return inside that
chained callback; additionally, before each major step (before persisting each
hidden message and before each runRawPrompt) check abortSignal (throw or return
early if signaled) so a timeout/abort can't continue mutating the session, and
keep using
entry.activeTurnState/runRawPrompt/openLogicalSession/messageRepo.create as
currently named inside the chained block.
In `@apps/server/src/services/agent-service.ts`:
- Around line 1049-1055: Re-check the child thread existence inside the
catch/fallback path before executing the legacy fallback: call
threadRepo.findById(thread.id) (same as the happy-path guard) and if it returns
null or has deleted_at set, log an info/error (e.g., via logger.info) noting the
child thread vanished and abort the fallback flow instead of writing seq 1,
persisting fallback artifacts, or queuing sends; ensure the code in the legacy
fallback branch (the catch that writes seq 1, persists fallback artifacts and
emits thread.handoff) returns/throws early when the re-check fails so no FK
errors, stale files, or misleading fallback events are produced.
In `@apps/server/src/services/cleanup-worker.ts`:
- Line 23: reconcileOnStartup() and finalizeWorkspaceIfDone() currently only
call attachmentService.removeForThread(...), so add deletion of handoff
artifacts by invoking HandoffStorage.removeForThread(...) (or the appropriate
HandoffStorage API used elsewhere) in those hard-delete paths so forked direct
threads' handoffs are cleaned up too; also change the existing executeJob() call
sites that invoke finalizeWorkspaceIfDone(...) to await the promise (i.e., make
them await finalizeWorkspaceIfDone(...)) so cleanup completes before proceeding.
Ensure you reference the same HandoffStorage implementation imported at top and
modify reconcileOnStartup(), finalizeWorkspaceIfDone(), and executeJob() callers
accordingly.
In `@apps/server/src/services/handoff/__tests__/fork-flow.e2e.test.ts`:
- Around line 249-264: The test currently calls broadcastMock(...) directly and
then asserts that call, which makes the assertion meaningless; remove the direct
invocation and instead set up broadcast as a mocked spy (vi.mocked(broadcast)),
trigger the real code path that should emit the broadcast (e.g., invoke the
fork/fallback integration flow or AgentService entry point used in this test
harness—look for the function under test in fork-flow.e2e.test.ts such as the
fork handler or AgentService.run that processes BASE_REQ and artifact), then
assert broadcastMock was calledWith the expected payload (thread.handoff, {
threadId: BASE_REQ.childThreadId, status: "fallback", ladderStep: "D",
providerErrorOnGenerate: "quota" }). Ensure the test arranges
artifact.meta.ladderStep="D" and providerErrorOnGenerate="quota" before
triggering the real flow so the broadcast is emitted by the system under test
rather than invoked directly in the test.
In `@apps/server/src/services/handoff/__tests__/handoff-pipeline.test.ts`:
- Around line 108-118: The test mocks deps.providerRegistry.resolve to return a
fresh object each call, so the assertion on provider.runSideChannelQuery is
checking a different instance than the one used in
HandoffPipelineService.forTesting(). Fix by stubbing resolve to return the same
mock object reference (create a single mock provider with runSideChannelQuery =
vi.fn(...) and any other props, then have deps.providerRegistry.resolve =
vi.fn(() => mockProvider)), then call svc.orchestrate(BASE_REQ) and assert
mockProvider.runSideChannelQuery.not.toHaveBeenCalled(); this ensures the tested
provider instance matches the asserted mock.
- Around line 141-156: The test temporarily replaces globalThis.AbortController
(saving OriginalAbortController) before calling
HandoffPipelineService.forTesting(...).orchestrate(BASE_REQ); ensure the
restoration of globalThis.AbortController happens in a finally block so the
original AbortController is always restored even if svc.orchestrate or the
assertion throws; move the assignment globalThis.AbortController =
OriginalAbortController into a finally and keep the override and test logic
inside the try so cross-test leakage cannot occur.
In `@apps/server/src/services/handoff/handoff-pipeline.ts`:
- Around line 144-146: The parent thread is loaded entirely via
messageRepo.listIncludingInternal into messages and later reused for
replay/fallback, which can leak messages after the fork anchor; locate where
messages and forkedFromMessageId are used (the messages array, forkMsg lookup
using messages.find) and compute the fork index (e.g., const forkIndex =
messages.findIndex(m => m.id === req.forkedFromMessageId); if forkIndex < 0
throw) then slice the array to only include messages up to and including the
fork anchor (messages = messages.slice(0, forkIndex + 1)) and replace all
subsequent uses (replay/fallback history, references previously using the full
messages array) with this truncated array so no later parent messages are
included in the child handoff context.
In `@apps/server/src/services/handoff/handoff-prompt.ts`:
- Around line 46-59: Add a one-line TSDoc/JSDoc summary for the exported
interface HandoffPromptInput at its declaration to satisfy the coding guideline;
open the HandoffPromptInput interface and add a top-level comment (e.g. /**
One-line summary describing the purpose of HandoffPromptInput */) above the
export so the interface and its intent are documented for consumers.
In `@apps/server/src/services/handoff/handoff-storage.ts`:
- Around line 145-147: The current unanchored regex that builds body from
markdownBody can remove valid content; update the stripping logic so you only
remove frontmatter when it matches a known handoff signature: either (a) tighten
the regex to require a known key (e.g., require a line like "handoff:" or
"provider:" inside the frontmatter) when computing body (replace the current
/^---\n[\s\S]*?\n---\n/ with a pattern that only matches if those keys exist),
or (b) safer—parse the leading frontmatter with a frontmatter parser (or
js-yaml): extract the YAML block from markdownBody, parse it and only strip it
(assign to body) if the parsed object contains one of the expected keys (e.g.,
"handoff", "provider", "handoff_version"); otherwise leave markdownBody intact.
Apply this change where markdownBody is transformed into body in
handoff-storage.ts.
In `@apps/server/src/services/handoff/path-d-deterministic.ts`:
- Around line 14-22: Add a one-line TSDoc/JSDoc summary above the exported
interface PathDInput describing its purpose (e.g., represents the input data
needed to construct/record a deterministic "D" fork path including parentThread,
messagesUpToFork, forkedFromMessageId, forkAnchorRole, childThreadId, and
optional reason). Place the comment immediately above the PathDInput declaration
and ensure it follows project TSDoc style (/** ... */) so the exported interface
is documented for consumers and tooling.
In `@apps/web/src/components/chat/Composer.tsx`:
- Around line 1696-1709: The handoff queue currently saves only the trimmed
string (trimmed) which drops attachments, reply/context/provider overrides and
prevents attachment-only sends; change the queue to store a full message
payload/descriptor (including text, attachments, replyTo, providerOverride,
metadata) instead of just trimmed, update setQueuedSend usage and the auto-fire
path to call the existing send function with that stored message object (so
sends fire even when trimmed === "" for attachment-only) and preserve
branchFromMessageId/threadId/isNewThread semantics; apply the same fix to the
other occurrence (the block around the 1959-1979 region) so both enqueue and
dequeue paths use the complete message object from
useThreadStore.getState().handoffStatus and hasSeenHandoffTransition checks.
In `@apps/web/src/components/chat/HandoffFallbackBanner.tsx`:
- Around line 97-117: When useEffect re-runs for a new threadId the previous
ready/error state can persist; inside the effect before calling
getTransport().readLatestHandoff, immediately call setState({ phase: "loading"
}) (or the component's loading phase shape) so the UI resets while the new fetch
is in-flight; update the effect surrounding getTransport().readLatestHandoff in
the same block that uses cancelled, and keep the existing .then/.catch logic
that sets phase to "ready" or "error" based on the result.
In `@docs/guides/chat-fork-handoff.md`:
- Around line 39-42: Update the fenced code blocks in
docs/guides/chat-fork-handoff.md to include a language identifier (use "text")
to satisfy markdown linting: replace the opening backticks for the blocks
containing the paths
"<MCODE_DATA_DIR>/threads/<threadId>/handoffs/<ulid>/handoff.md" and
"<MCODE_DATA_DIR>/threads/<threadId>/handoffs/<ulid>/handoff.json" and the block
containing "<MCODE_DATA_DIR>/threads/<threadId>/attachments/<id>.<ext>" with
```text so each fenced code block begins with ```text and closes with ```.
In `@docs/guides/cursor-sdk-migration.md`:
- Line 29: Change the unhyphenated compound modifier "10 second" to "10-second"
at each occurrence in this document (notably the phrase in the sentence
containing "settle-wait poll and a disregard-turn cleanup" and the other
instances referenced around lines 68 and 211) so the compound numeric modifier
is consistently hyphenated as "10-second".
In `@packages/contracts/src/ws/channels.ts`:
- Around line 105-113: The "thread.handoff" Zod object is being constructed
eagerly; wrap its schema in lazySchema to defer construction (replace the direct
z.object({...}) with lazySchema(() => z.object({...}))). Ensure lazySchema is
imported into the module and retain the existing fields (threadId, status,
ladderStep, providerErrorOnGenerate) unchanged inside the returned object. This
makes the schema lazy-loaded while preserving the original shape and options.
In `@packages/contracts/src/ws/methods.ts`:
- Around line 682-684: The providerErrorOnGenerate enum in the WebSocket
contract (symbol: providerErrorOnGenerate in methods.ts) includes the value
"clean" which is not present on the push-channel contract (symbol:
WS_CHANNELS["thread.handoff"].providerErrorOnGenerate); update the enum so both
contracts match—either remove "clean" from providerErrorOnGenerate here or add
"clean" to WS_CHANNELS["thread.handoff"].providerErrorOnGenerate so they are
identical, keeping the .nullable() behavior intact.
---
Outside diff comments:
In `@apps/web/src/stores/threadStore.ts`:
- Around line 1403-1436: The cleanup functions clearThreadState and
clearThreadStateMany are missing removals for the new per-thread maps
handoffMeta, handoffStatus, and forkMode, leaving stale UI state; update the
returned state object in both clearThreadState and clearThreadStateMany to
include handoffMeta: omitKey(state.handoffMeta, threadId), handoffStatus:
omitKey(state.handoffStatus, threadId), and forkMode: omitKey(state.forkMode,
threadId) (and when clearing many, apply the same omission logic for each
threadId) so those per-thread entries are pruned during thread delete.
---
Nitpick comments:
In `@apps/web/src/transport/ws-transport.ts`:
- Around line 751-781: The inline, verbose response type in the
readLatestHandoff call is duplicating the transport contract and risks schema
drift; change the generic type passed to rpc to derive from the canonical
transport type (use McodeTransport["readLatestHandoff"] or the appropriate
member) instead of the long inline shape so the call signature stays in sync
with the shared contract (ensure you preserve the union with null if the
transport type allows null), update the rpc invocation around readLatestHandoff
to use that referenced type and remove the inline object type.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 02a8b8e6-6307-4360-9293-812a2bcd3691
📒 Files selected for processing (68)
AGENTS.mdCONTEXT.mdapps/desktop/scripts/dev-electron.mjsapps/server/drizzle/0008_brave_big_bertha.sqlapps/server/drizzle/meta/0008_snapshot.jsonapps/server/drizzle/meta/_journal.jsonapps/server/src/__tests__/agent-service-turn-started.test.tsapps/server/src/__tests__/thread-forking.test.tsapps/server/src/__tests__/utility-completion-service.test.tsapps/server/src/container.tsapps/server/src/index.tsapps/server/src/providers/claude/__tests__/side-channel-sessionless.test.tsapps/server/src/providers/claude/claude-provider.tsapps/server/src/providers/codex/codex-provider.tsapps/server/src/providers/copilot/copilot-provider.tsapps/server/src/providers/cursor/cursor-provider.tsapps/server/src/repositories/message-repo.tsapps/server/src/services/__tests__/agent-service-gate.test.tsapps/server/src/services/__tests__/agent-service-goal-command.test.tsapps/server/src/services/__tests__/agent-service-narrative-persist.test.tsapps/server/src/services/__tests__/agent-service-plan-marker.test.tsapps/server/src/services/__tests__/agent-service-snapshot-flag.test.tsapps/server/src/services/__tests__/agent-service-stack-fallback.test.tsapps/server/src/services/__tests__/agent-service-turn-cleanup.test.tsapps/server/src/services/__tests__/provider-availability-service.test.tsapps/server/src/services/agent-service.tsapps/server/src/services/cleanup-worker.tsapps/server/src/services/handoff/__tests__/error-classifier.test.tsapps/server/src/services/handoff/__tests__/fork-flow.e2e.test.tsapps/server/src/services/handoff/__tests__/handoff-pipeline.test.tsapps/server/src/services/handoff/__tests__/handoff-prompt.test.tsapps/server/src/services/handoff/__tests__/handoff-storage.test.tsapps/server/src/services/handoff/__tests__/path-d-deterministic.test.tsapps/server/src/services/handoff/error-classifier.tsapps/server/src/services/handoff/handoff-pipeline.tsapps/server/src/services/handoff/handoff-prompt.tsapps/server/src/services/handoff/handoff-storage.tsapps/server/src/services/handoff/handoff-types.tsapps/server/src/services/handoff/path-d-deterministic.tsapps/server/src/store/schema.tsapps/server/src/transport/ws-router.tsapps/web/src/__tests__/mocks/transport.tsapps/web/src/__tests__/workspace-behavior.test.tsapps/web/src/components/chat/ChatView.tsxapps/web/src/components/chat/Composer.tsxapps/web/src/components/chat/ComposerBranchBar.tsxapps/web/src/components/chat/HandoffCard.tsxapps/web/src/components/chat/HandoffFallbackBanner.tsxapps/web/src/components/chat/MessageBubble.tsxapps/web/src/components/chat/MessageList.tsxapps/web/src/components/chat/__tests__/composer-branch-bar.test.tsxapps/web/src/components/settings/sections/NotificationsSection.tsxapps/web/src/stores/threadStore.tsapps/web/src/transport/types.tsapps/web/src/transport/ws-events.tsapps/web/src/transport/ws-transport.tsdocs/guides/chat-fork-handoff.mddocs/guides/cursor-sdk-migration.mdpackages/contracts/src/index.tspackages/contracts/src/models/message.tspackages/contracts/src/models/settings.tspackages/contracts/src/providers/interfaces.tspackages/contracts/src/ws/channels.tspackages/contracts/src/ws/methods.tspackages/contracts/tsconfig.jsonpackages/shared/src/index.tspackages/shared/src/paths/__tests__/handoffs.test.tspackages/shared/src/paths/handoffs.ts
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
1 of 3 tasks
chuks-qua
added a commit
that referenced
this pull request
May 23, 2026
docs(cursor-handoff): defer SDK migration; drop shipped chat-fork plans Investigation on 2026-05-23 found the @cursor/sdk v1.0.13 migration is blocked: the SDK requires an API key (CURSOR_API_KEY env or apiKey arg) with no path to read the cursor-agent CLI's stored login. That conflicts with the project rule that mcode never requires users to set API keys. The ACP session/load upstream bug is also still unresolved (latest cursor-cli 2026.05.16-0338208 still affected), so we cannot flip Cursor to sessionForkOnResume "clean" without the SDK either. Prepend a Status: DEFERRED block to the handoff doc with the two un-defer triggers (Cursor fixes ACP, or SDK gains CLI-auth) and a 2026-08-23 recheck date with the exact commands to re-evaluate. Drop docs/plans/2026-05-21-chat-branch-handoff-pipeline.md and its prototype: the chat-fork handoff feature they described already shipped in #499 (commit fb4e712), and docs/plans/ is already gitignored. Fix the residual reference in CONTEXT.md.
This was referenced May 28, 2026
Merged
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Replaces the legacy transcript-replay branching with a provider-generated chat fork handoff system. When the user forks a chat at a specific message, the parent thread's provider summarizes the conversation into a structured markdown handoff doc. The doc is inlined into the child thread's first turn so the new agent picks up with real context, not raw transcripts. Three execution paths handle different provider capabilities and failure modes.
Why
The previous flow dumped the entire parent transcript into the child's first turn. New providers re-derived everything from scratch and were unable to reference specific prior decisions. The vendored
/handoffskill is exactly the right primitive: trust the LLM to summarize what matters, given the conversation it already has loaded. By running it side-channel on the parent's provider, we get a high-fidelity handoff without polluting the parent's UI or forcing the user to wait through provider-by-provider migrations.The feature is also a rename pass: chat "branch" became chat "fork" in all user-visible copy to disambiguate from git branches. The schema field
forkedFromMessageIdwas already aligned.Key Changes
Pipeline architecture (B / A / D ladder)
Storage
<MCODE_DATA_DIR>/threads/<id>/handoffs/<ulid>/handoff.md + handoff.json.Provider integration
sessionForkOnResume(clean / mutating / unsupported) andmaxInputCharactersPerTurncapabilities toIAgentProvider.unsupported(route to path D until verified).Object.assignhack threw on Node 24.UI
HandoffFallbackBannerwith trigger-aware copy and a "View doc" dialog showing metadata strip + frontmatter-stripped markdown.Schema
messages.is_internalflag for hidden turns. Filtered from every user-facing query.Robustness phase
Documentation
CONTEXT.mdglossary at repo root for the fork + handoff vocabulary.docs/guides/chat-fork-handoff.mduser-facing guide.docs/guides/cursor-sdk-migration.mdhandoff doc for the Cursor SDK migration that would unlock path B for Cursor and let us delete path A entirely.docs/plans/2026-05-21-chat-branch-handoff-pipeline.mddesign and implementation plan.docs/plans/prototypes/2026-05-21-chat-branch-handoff.htmlUX storyboard used to validate the design.Tests
Deferred items (documented in the plan, not in this PR)
docs/guides/cursor-sdk-migration.md.not-implemented).Need help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores