feat(orchestrator): move a running thread between devices - #5544
Conversation
Thread handoff moves a thread from the environment that owns it to another connected environment: the conversation, the provider continuation, and the git working state the thread was left in. This lands the parts that are expensive to change once two servers have exchanged anything, ahead of the services that use them: - `OrchestrationV2HandoffBundleV1`, a manifest plus content-addressed parts. Structural state is inline; large payloads (git bundle, tracked patch, untracked tar, attachments) are fetched separately by digest, so the transport can change later without moving the manifest or the ids with it. - `ThreadHandoffId`, shared by both sides of a hop, which makes it both the idempotency key for applying a bundle and the link along a lineage. - A lineage that records the previous hop rather than a push/pull direction, so laptop -> server -> phone -> laptop is a walk instead of a special case. - The `threadHandoff` environment capability, following the same version-skew contract as `threadSettlement`: absent means unsupported, so a client lists such an environment as unavailable instead of starting a transfer that fails once bytes are already moving. No server advertises it yet. - `orchestration_v2_thread_handoffs`, which records what a receiving repository looked like before a bundle was applied so a partial apply is rolled back from a lookup rather than a reconstruction.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
There was a problem hiding this comment.
One finding on the new handoff error contract; the migration, its tests, and the bundle schemas otherwise follow the Effect conventions (subpath namespace imports, no service-instance injection, no ManagedRuntime/runPromise, no Layer.succeed shortcuts).
Posted via Macroscope — Effect Service Conventions
Adds the handoff link to the thread aggregate and the three commands that move it, so a thread can record that its work left for another environment and came back. - `thread.handoff.depart` locks the giving side before any bundle is applied anywhere. Locking first is what makes "exactly one side is live" an invariant: an interrupted transfer leaves a locked thread that `thread.handoff.abort` releases, never a second conversation to merge. - `thread.handoff.complete` records the peer's thread id once it confirms. - `message.dispatch` is refused while a thread is away, in the orchestrator rather than the client, naming the environment that owns it. The link rides in the thread payload, which the projection already stores as `payload_json`, so no projection migration is needed and older stored payloads decode unchanged. It is also carried on the thread shell so a sidebar can badge a thread without loading its detail. `ThreadHandoffGit` lands the safety core: a branch tip only ever moves to a descendant of itself, on either machine. `classifyIncomingTip` reduces that to advance / absorb / diverged / unrelated as a pure function, with the ref, tag and stash naming that make a refusal or a rollback legible afterwards.
`ThreadHandoffService` is the hop itself: prepare on the giving side, receive on the taking side, and a startup pass for anything left half-applied. Prepare stages the parts and writes nothing the user can see, so the counts a preflight shows come from the code path the transfer actually uses. It bundles the branch together with the checkpoint refs — checkpoints are hidden git refs, so the whole checkpoint timeline travels inside the object bundle with no payload of its own and revert keeps working on the far side. Tracked changes travel as a binary patch so images and lockfiles survive; untracked files travel as a null-delimited tar so paths with spaces do. The two payload ceilings are enforced here, before anything has been sent, so a refusal costs nothing on either machine. Receive verifies every part against its digest before touching anything, then classifies the incoming tip. A diverged or unrelated branch parks the sender's commits at `refs/handoff/<environment>/<branch>` and stops with both machines untouched. Otherwise the old tip is tagged and a dirty worktree stashed before any pointer moves, the patch is dry-run applied before it is really applied, and a patch that will not apply rolls the repository back to the tag and pops the stash rather than leaving it half-written. Hops are serialized per thread, and `applying` is the only state in which a repository can have been written to, so it is the only one startup recovery has to look at.
There was a problem hiding this comment.
Effect service conventions review of the two new service modules. Three findings, all in apps/server/src/orchestration-v2/. Earlier feedback on OrchestrationV2HandoffError.message still stands and is not repeated here.
Posted via Macroscope — Effect Service Conventions
| ...(input.maxOutputBytes === undefined ? {} : { maxOutputBytes: input.maxOutputBytes }), | ||
| }); | ||
|
|
||
| export interface ThreadHandoffGitShape { |
There was a problem hiding this comment.
Same here: ThreadHandoffGitShape should be inlined into the Context.Service<ThreadHandoffGit, { ... }>() declaration, with the local implementation bindings annotated as ThreadHandoffGit["Service"]["resolveTip"] and so on, instead of exporting a separate shape type.
Posted via Macroscope — Effect Service Conventions
| ThreadHandoffServiceShape | ||
| >()("t3/orchestration-v2/ThreadHandoffService") {} | ||
|
|
||
| const handoffError = (input: { |
There was a problem hiding this comment.
handoffError only forwards its arguments into new OrchestrationV2HandoffError({ ... }), so it adds an indirection without normalization. Consider constructing the error directly at each failure boundary (the optional-field spreads are only needed for handoffId/cause, which each call site already knows) and keeping asHandoffError as the one mapper, since that one does real work by passing through an existing handoff error.
Posted via Macroscope — Effect Service Conventions
| readonly preTag: string | null; | ||
| } | ||
|
|
||
| export interface ThreadHandoffServiceShape { |
There was a problem hiding this comment.
New service modules should define the service interface inline in the Context.Service declaration rather than keeping a standalone ThreadHandoffServiceShape. Consider moving this body into Context.Service<ThreadHandoffService, { ... }>() below and referring to members as ThreadHandoffService["Service"]["partPath"] etc. at the implementation sites (partPath, verifyStagedPart, prepare, receive, recoverInterrupted).
Posted via Macroscope — Effect Service Conventions
| const process = yield* VcsProcess; | ||
| const git = (input: GitInput) => runGit(process, input); | ||
|
|
||
| const resolveTip: ThreadHandoffGitShape["resolveTip"] = (input) => |
There was a problem hiding this comment.
🟠 High orchestration-v2/ThreadHandoffGit.ts:195
resolveTip maps any nonzero git rev-parse exit to null, so a corrupted or unreadable ref returns null instead of failing with VcsError. The caller then treats the branch as nonexistent, classifies the transfer as advance, skips the pre-tag and stash, and runs checkout -B — overwriting an existing branch without the rollback tag the safety model requires. Only the expected "ref not found" case should map to null; other nonzero exits should propagate as VcsError.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/ThreadHandoffGit.ts around line 195:
`resolveTip` maps any nonzero `git rev-parse` exit to `null`, so a corrupted or unreadable ref returns `null` instead of failing with `VcsError`. The caller then treats the branch as nonexistent, classifies the transfer as `advance`, skips the pre-tag and stash, and runs `checkout -B` — overwriting an existing branch without the rollback tag the safety model requires. Only the expected "ref not found" case should map to `null`; other nonzero exits should propagate as `VcsError`.
| if (sha256(contents) !== input.part.digest) { | ||
| return yield* handoffError({ | ||
| reason: "part_digest_mismatch", | ||
| message: `Handoff part ${input.part.kind} does not match the digest in the manifest.`, | ||
| handoffId: input.handoffId, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🟡 Medium orchestration-v2/ThreadHandoffService.ts:250
verifyStagedPart checks the SHA-256 digest but never compares contents.length against input.part.byteLength, so a manifest with a wrong declared byte length is accepted. Any receive-side size accounting that trusts the declared byteLength (e.g., payload ceilings or transfer-progress tracking) uses the incorrect value, and a peer can declare a smaller length to bypass size checks. Consider rejecting the part when contents.length !== input.part.byteLength.
if (sha256(contents) !== input.part.digest) {
return yield* handoffError({
reason: "part_digest_mismatch",
message: `Handoff part ${input.part.kind} does not match the digest in the manifest.`,
handoffId: input.handoffId,
});
}
+ if (contents.length !== input.part.byteLength) {
+ return yield* handoffError({
+ reason: "part_digest_mismatch",
+ message: `Handoff part ${input.part.kind} byte length does not match the manifest.`,
+ handoffId: input.handoffId,
+ });
+ }🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/ThreadHandoffService.ts around lines 250-256:
`verifyStagedPart` checks the SHA-256 digest but never compares `contents.length` against `input.part.byteLength`, so a manifest with a wrong declared byte length is accepted. Any receive-side size accounting that trusts the declared `byteLength` (e.g., payload ceilings or transfer-progress tracking) uses the incorrect value, and a peer can declare a smaller length to bypass size checks. Consider rejecting the part when `contents.length !== input.part.byteLength`.
| }); | ||
| } | ||
|
|
||
| yield* markHop({ handoffId: bundle.handoffId, state: "applying", lastError: null }); |
There was a problem hiding this comment.
🟠 High orchestration-v2/ThreadHandoffService.ts:693
When receive processes a first-time arrival, markHop is called with state: "applying" before recordHop has inserted a row for bundle.handoffId. Because markHop is only an UPDATE, it affects zero rows on a first arrival. If the server stops after repository writes begin, recoverInterrupted finds no row in the applying state and cannot mark the partially modified repository as failed — leaving the handoff unrecoverable by the advertised recovery path. Consider calling recordHop with state: "applying" before the first markHop, or making markHop upsert.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/ThreadHandoffService.ts around line 693:
When `receive` processes a first-time arrival, `markHop` is called with `state: "applying"` before `recordHop` has inserted a row for `bundle.handoffId`. Because `markHop` is only an `UPDATE`, it affects zero rows on a first arrival. If the server stops after repository writes begin, `recoverInterrupted` finds no row in the `applying` state and cannot mark the partially modified repository as failed — leaving the handoff unrecoverable by the advertised recovery path. Consider calling `recordHop` with `state: "applying"` before the first `markHop`, or making `markHop` upsert.
| serialize.withLock( | ||
| input.bundle.handoffId, | ||
| Effect.gen(function* () { | ||
| const { bundle } = input; |
There was a problem hiding this comment.
🟠 High orchestration-v2/ThreadHandoffService.ts:617
receive has no idempotency check: when called again with the same bundle.handoffId that already succeeded, it re-imports the git bundle, re-applies the tracked patch (which typically fails because the changes are already present), and creates a second thread with a new random threadId instead of returning the existing one. The duplicate recordHop row is upserted to arrived state, but writeArrival already emitted a second thread.created event with a different threadId. Consider looking up the handoff row by bundle.handoffId before processing and short-circuiting if its state is already arrived, returning the previously recorded thread_id and classification.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/ThreadHandoffService.ts around line 617:
`receive` has no idempotency check: when called again with the same `bundle.handoffId` that already succeeded, it re-imports the git bundle, re-applies the tracked patch (which typically fails because the changes are already present), and creates a second thread with a new random `threadId` instead of returning the existing one. The duplicate `recordHop` row is upserted to `arrived` state, but `writeArrival` already emitted a second `thread.created` event with a different `threadId`. Consider looking up the handoff row by `bundle.handoffId` before processing and short-circuiting if its state is already `arrived`, returning the previously recorded `thread_id` and classification.
| }), | ||
| ); | ||
|
|
||
| const recoverInterrupted: ThreadHandoffServiceShape["recoverInterrupted"] = () => |
There was a problem hiding this comment.
🟠 High orchestration-v2/ThreadHandoffService.ts:785
recoverInterrupted marks interrupted hops as failed but never reverts the partial working-tree changes. After a shutdown during receive, the repository is left with a half-applied checkout, patch, or untracked archive, and startup recovery only updates the database row to failed — it does not read the recorded pre_tag or stash_ref to run rollback. The user's worktree is permanently left in the partial state. Consider having recoverInterrupted load each row's pre_tag, stash_ref, and cwd and call rollback (or the equivalent git.resetHardTo / git.popStash) before marking the hop failed.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/ThreadHandoffService.ts around line 785:
`recoverInterrupted` marks interrupted hops as `failed` but never reverts the partial working-tree changes. After a shutdown during `receive`, the repository is left with a half-applied checkout, patch, or untracked archive, and startup recovery only updates the database row to `failed` — it does not read the recorded `pre_tag` or `stash_ref` to run `rollback`. The user's worktree is permanently left in the partial state. Consider having `recoverInterrupted` load each row's `pre_tag`, `stash_ref`, and `cwd` and call `rollback` (or the equivalent `git.resetHardTo` / `git.popStash`) before marking the hop `failed`.
|
|
||
| const receive: ThreadHandoffServiceShape["receive"] = (input) => | ||
| serialize.withLock( | ||
| input.bundle.handoffId, |
There was a problem hiding this comment.
🟠 High orchestration-v2/ThreadHandoffService.ts:615
receive serializes on bundle.handoffId, but prepare serializes on input.threadId. Two concurrent handoffs for the same thread acquire different locks, as does a concurrent prepare and receive on the same thread, so both can execute overlapping checkout, stash, patch, and archive operations in the same worktree simultaneously. This corrupts or cross-contaminates both handoffs' working-state application. Both prepare and receive should serialize on a key derived from the thread identity (e.g. the thread id or workspace root) rather than on the handoff id.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/ThreadHandoffService.ts around line 615:
`receive` serializes on `bundle.handoffId`, but `prepare` serializes on `input.threadId`. Two concurrent handoffs for the same thread acquire different locks, as does a concurrent `prepare` and `receive` on the same thread, so both can execute overlapping checkout, stash, patch, and archive operations in the same worktree simultaneously. This corrupts or cross-contaminates both handoffs' working-state application. Both `prepare` and `receive` should serialize on a key derived from the thread identity (e.g. the thread id or workspace root) rather than on the handoff id.
| handoffId: bundle.handoffId, | ||
| }); | ||
| } | ||
| yield* git |
There was a problem hiding this comment.
🟠 High orchestration-v2/ThreadHandoffService.ts:740
After the patch dry-run succeeds, any failure from the real git.applyPatch, git.extractArchive, writeArrival, recordHop, or markHop escapes the receive block without calling rollback. The branch may have already been moved to the incoming commit and the local worktree stashed, so the repository is left partially modified and the user's stashed changes are never restored. The rollback({ cwd, preTag, stashRef }) call is only wired into the dry-run-failure path; it is not used as a cleanup handler for the rest of the apply step.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/ThreadHandoffService.ts around line 740:
After the patch dry-run succeeds, any failure from the real `git.applyPatch`, `git.extractArchive`, `writeArrival`, `recordHop`, or `markHop` escapes the `receive` block without calling `rollback`. The branch may have already been moved to the incoming commit and the local worktree stashed, so the repository is left partially modified and the user's stashed changes are never restored. The `rollback({ cwd, preTag, stashRef })` call is only wired into the dry-run-failure path; it is not used as a cleanup handler for the rest of the apply step.
| verifyStagedPart({ handoffId: bundle.handoffId, part }), | ||
| ); | ||
|
|
||
| const cwd = yield* workspaceRootFor(input.projectId); |
There was a problem hiding this comment.
🟠 High orchestration-v2/ThreadHandoffService.ts:622
receive always applies git state to the project's root workspace regardless of the bundle's workspace.strategy. When the bundle specifies existing_worktree, the git import, branch update, stash, and patch application are all performed at the root workspace path, while the created/updated thread is assigned the sender's worktreePath. This means the handoff reports success even though the repository at worktreePath was never modified and the project root was modified instead. The cwd for git operations should resolve from the bundle's workspace strategy (or the existing thread's worktreePath for returning hops) rather than always using workspaceRootFor(input.projectId).
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/ThreadHandoffService.ts around line 622:
`receive` always applies git state to the project's root workspace regardless of the bundle's `workspace.strategy`. When the bundle specifies `existing_worktree`, the git import, branch update, stash, and patch application are all performed at the root workspace path, while the created/updated thread is assigned the sender's `worktreePath`. This means the handoff reports success even though the repository at `worktreePath` was never modified and the project root was modified instead. The `cwd` for git operations should resolve from the bundle's workspace strategy (or the existing thread's `worktreePath` for returning hops) rather than always using `workspaceRootFor(input.projectId)`.
| }, | ||
| ]), | ||
| ]; | ||
| yield* eventStore.append({ events }); |
There was a problem hiding this comment.
🟠 High orchestration-v2/ThreadHandoffService.ts:604
When receive completes a handoff, the thread is created or continued on the destination without any of its conversation history. writeArrival only appends the thread.created/thread.handoff-returned and thread.handoff-arrived lifecycle events, and never emits events for bundle.conversation.items or bundle.conversation.coveredRunOrdinals. As a result the receiving environment has a thread with no turn items or run ordinals, so the provider session has no prior context to continue from. Consider replaying the carried conversation items and run ordinals as domain events alongside the lifecycle events in writeArrival.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/ThreadHandoffService.ts around line 604:
When `receive` completes a handoff, the thread is created or continued on the destination without any of its conversation history. `writeArrival` only appends the `thread.created`/`thread.handoff-returned` and `thread.handoff-arrived` lifecycle events, and never emits events for `bundle.conversation.items` or `bundle.conversation.coveredRunOrdinals`. As a result the receiving environment has a thread with no turn items or run ordinals, so the provider session has no prior context to continue from. Consider replaying the carried conversation items and run ordinals as domain events alongside the lifecycle events in `writeArrival`.
Wires the handoff service into the running server: - `orchestration.prepareThreadHandoff` and `orchestration.receiveThreadHandoff` join the orchestration RPC group, both behind the operate scope. Preparing stages bytes and records a hop, so it is an operate even though nothing the user can see has changed yet. - The RPC results narrow to what a caller can act on: a payload over the hard ceiling fails with `payload_too_large` rather than returning a verdict nobody can do anything about, and a diverged or unrelated branch fails with `workspace_diverged` rather than returning a classification that wrote nothing. - `ThreadHandoffService` is layered with its git, project, repository-identity and provider-registry dependencies. - Startup fails any hop still marked `applying`, which is the only state in which a transfer that never finished can have written to this environment's repository.
Bil0000
changed the title
Adds the transport the manifest was designed for, and turns the capability on now that an environment can actually receive a hop. Part bytes move in bounded chunks rather than as one body. A part can be as large as the payload ceiling, so chunking keeps neither server holding a whole payload in memory, and it makes a transfer that dies partway resumable from the offset it reached instead of starting over. - `POST /api/orchestration/handoffs/:handoffId/parts/:kind/read` returns a window of a staged part plus whether it is the last one, so a caller stops asking without comparing offsets itself. Reading takes a payload rather than a query string: the offset is part of a resumable transfer, not a filter, and it keeps both directions on the same typed shape. - `POST .../:kind` appends a chunk, and rejects one that does not continue exactly where the staged bytes end. A chunk written at the wrong offset would leave a hole that only surfaces later as a digest mismatch or a corrupt bundle, so it fails at the seam instead. - Both are behind the orchestration operate scope. - `handoffChunkWindow` is the chunk math on its own, clamping rather than failing so a retry that asks past the end resolves instead of erroring. Servers now advertise `threadHandoff`, and the client projection applies the four handoff events, so a departed or arrived thread updates live rather than only after a reload.
`runThreadHandoffTransfer` is the hop from the client's side: it is the only component authenticated to both environments, so it is what moves the bytes. The order is the safety model — stage, lock the giving side, move the bytes, apply, record where the thread went. Locking before the bytes move guarantees the two sides can never both be live, and every failure after the lock releases it, so a transient network error leaves the thread usable where it started rather than stranded with nobody able to type in it. Parts copy a chunk at a time between the two servers. The receiver rejects a chunk that does not continue where the staged bytes end, so the reader's reported offset is the only thing deciding where a write lands and a retried chunk cannot punch a hole in a part. Progress is reported per phase, split by whether the receiving repository has been written to yet: everything through `upload` is free to cancel, `apply` onward is not.
`Run on` already answers "which machine is this thread on", so it is where moving belongs. Once a thread is locked to its environment the control stays interactive instead of collapsing to a label: the current device sits under "Running on", and the others under "Move thread to" — a separate group with its own verb rather than the same rows quietly meaning something different. The dialog shows what travels, then the steps as they run, split by whether the receiving repository has been written to yet. Cancel is offered through the upload and withdrawn at apply, because from there only the servers can put the other machine back. A failure surfaces the server's own message: a divergence names the ref the commits were parked at and a payload refusal names the size, both of which beat anything generic the dialog could say. The move command spans two environments, so it is a runtime command rather than a per-environment one, and it resolves both connections before locking anything — discovering halfway through that the destination was never reachable would mean pausing a thread for a transfer that could not work.
| handoffId: bundle.handoffId, | ||
| targetThreadId: application.threadId, | ||
| } satisfies ThreadHandoffTransferResult; | ||
| }).pipe( |
There was a problem hiding this comment.
🟠 High state/threadHandoffTransfer.ts:201
Effect.tapCause calls release (which sends thread.handoff.abort) on every failure, including failures that occur after receiveThreadHandoff has already applied the bundle on the target. If completeThreadHandoff fails, release unlocks the origin side even though the target is already live — leaving both copies of the thread usable and free to diverge. Cleanup after the apply boundary must not abort/unlock the giving side; consider splitting the error handler so only pre-apply failures trigger release, while post-apply failures surface without aborting.
🤖 Copy this AI Prompt to have your agent fix this:
In file @packages/client-runtime/src/state/threadHandoffTransfer.ts around line 201:
`Effect.tapCause` calls `release` (which sends `thread.handoff.abort`) on every failure, including failures that occur after `receiveThreadHandoff` has already applied the bundle on the target. If `completeThreadHandoff` fails, `release` unlocks the origin side even though the target is already live — leaving both copies of the thread usable and free to diverge. Cleanup after the apply boundary must not abort/unlock the giving side; consider splitting the error handler so only pre-apply failures trigger `release`, while post-apply failures surface without aborting.
| )} | ||
| </DialogPanel> | ||
| <div className="flex justify-end gap-2 px-4 pb-4"> | ||
| <Button |
There was a problem hiding this comment.
🟠 High components/ThreadHandoffDialog.tsx:194
When the user clicks Cancel during a safe-to-cancel phase (prepare, depart, or upload), the button calls onOpenChange(false) which closes the dialog but does not abort the in-flight move promise. The transfer keeps running, and on success onMoved is still invoked — so the thread is moved even though the user cancelled. The onOpenChange guard only blocks the backdrop/escape path, not the explicit Cancel button, and there is no mechanism wired to interrupt move. Consider having the Cancel button actually cancel the handoff (e.g. via an AbortSignal/abort command exposed by threadHandoff) rather than just closing the dialog.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/ThreadHandoffDialog.tsx around line 194:
When the user clicks `Cancel` during a safe-to-cancel phase (`prepare`, `depart`, or `upload`), the button calls `onOpenChange(false)` which closes the dialog but does not abort the in-flight `move` promise. The transfer keeps running, and on success `onMoved` is still invoked — so the thread is moved even though the user cancelled. The `onOpenChange` guard only blocks the backdrop/escape path, not the explicit Cancel button, and there is no mechanism wired to interrupt `move`. Consider having the Cancel button actually cancel the handoff (e.g. via an `AbortSignal`/abort command exposed by `threadHandoff`) rather than just closing the dialog.
| if (!exists) { | ||
| return yield* failEnvironmentNotFound("handoff_part_not_found"); | ||
| } | ||
| const contents = yield* fs |
There was a problem hiding this comment.
🟠 High orchestration-v2/http.ts:126
readHandoffPart calls fs.readFile(target) to load the entire staged part into memory before slicing out one fixed-size chunk. A handoff part can approach the 1 GiB payload limit, so every chunk request allocates the whole file, and repeated requests reread the full file for each chunk. Consider using a bounded positional read for the requested window instead of reading the entire file.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/http.ts around line 126:
`readHandoffPart` calls `fs.readFile(target)` to load the entire staged part into memory before slicing out one fixed-size chunk. A handoff part can approach the 1 GiB payload limit, so every chunk request allocates the whole file, and repeated requests reread the full file for each chunk. Consider using a bounded positional read for the requested window instead of reading the entire file.
| if (args.payload.offset !== staged.length) { | ||
| return yield* failEnvironmentInvalidRequest("handoff_part_offset_mismatch"); | ||
| } | ||
| const next = new Uint8Array(staged.length + args.payload.data.length); |
There was a problem hiding this comment.
🟠 High orchestration-v2/http.ts:167
In writeHandoffPart, the offset check and file write are not serialized. If two requests for the same handoff part arrive concurrently with the same offset (e.g., an overlapping retry), both read the same staged length, both pass the offset !== staged.length check, and both write their own next buffer — the second write silently overwrites the first, so one accepted chunk is lost even though both requests report success. The transfer ends up corrupted and only surfaces later as a digest mismatch. Consider serializing writes per handoff part (e.g., a lock keyed by handoffId + kind) so concurrent writes cannot interleave.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/http.ts around line 167:
In `writeHandoffPart`, the offset check and file write are not serialized. If two requests for the same handoff part arrive concurrently with the same offset (e.g., an overlapping retry), both read the same `staged` length, both pass the `offset !== staged.length` check, and both write their own `next` buffer — the second write silently overwrites the first, so one accepted chunk is lost even though both requests report success. The transfer ends up corrupted and only surfaces later as a digest mismatch. Consider serializing writes per handoff part (e.g., a lock keyed by `handoffId` + `kind`) so concurrent writes cannot interleave.
| */ | ||
| export const ENVIRONMENT_HANDOFF_PART_CHUNK_BYTES = 4 * 1024 * 1024; | ||
|
|
||
| const EnvironmentHandoffPartParams = Schema.Struct({ |
There was a problem hiding this comment.
🟠 High src/environmentHttp.ts:492
EnvironmentHandoffPartParams.handoffId accepts any trimmed non-empty string, including path traversal sequences like ../../etc. The readHandoffPart and writeHandoffPart endpoints interpolate handoffId directly into the request path /api/orchestration/handoffs/:handoffId/parts/:kind, and the downstream ThreadHandoffService.partPath joins it with config.handoffsDir via path.join. A client with orchestration:operate scope can supply a traversal payload as handoffId to read or overwrite files (e.g. git.bundle) in arbitrary directories reachable from handoffsDir. Consider restricting ThreadHandoffId to a path-safe identifier pattern, or validating that the resolved path remains beneath handoffsDir before any filesystem operation.
🤖 Copy this AI Prompt to have your agent fix this:
In file @packages/contracts/src/environmentHttp.ts around line 492:
`EnvironmentHandoffPartParams.handoffId` accepts any trimmed non-empty string, including path traversal sequences like `../../etc`. The `readHandoffPart` and `writeHandoffPart` endpoints interpolate `handoffId` directly into the request path `/api/orchestration/handoffs/:handoffId/parts/:kind`, and the downstream `ThreadHandoffService.partPath` joins it with `config.handoffsDir` via `path.join`. A client with `orchestration:operate` scope can supply a traversal payload as `handoffId` to read or overwrite files (e.g. `git.bundle`) in arbitrary directories reachable from `handoffsDir`. Consider restricting `ThreadHandoffId` to a path-safe identifier pattern, or validating that the resolved path remains beneath `handoffsDir` before any filesystem operation.
Wires the handoff into the thread view: - The `Run on` control offers the other devices once a thread is locked to its environment, and opens the move dialog for the one picked. It is only offered for a real thread that is not already away — a draft has nothing to move, and a thread running elsewhere is not this side's to send. - A departed thread shows a composer banner naming the environment that owns it. The server refuses the send either way; this is what keeps that refusal from being a mystery. - The client thread shell carries the handoff link, so the banner and the move affordance both read it without loading thread detail. Absent on servers that predate handoff, which is what stops a client offering a move an environment cannot perform.
There was a problem hiding this comment.
🟡 Medium
When layout === "panel", onMoveThread is never forwarded to BranchToolbarEnvironmentSelector — the panel branch returns at line 406, before the only forwarding site at line 472. As a result, a locked thread rendered in the thread-details panel cannot be moved to another environment even when the caller supplies onMoveThread: the environment selector collapses to a static label instead of offering move targets. The panel branch should forward onMoveThread to its own BranchToolbarEnvironmentSelector (or to the BranchToolbar that renders it), so the move-thread control stays interactive in the panel layout.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/BranchToolbar.tsx around line 407:
When `layout === "panel"`, `onMoveThread` is never forwarded to `BranchToolbarEnvironmentSelector` — the panel branch returns at line 406, before the only forwarding site at line 472. As a result, a locked thread rendered in the thread-details panel cannot be moved to another environment even when the caller supplies `onMoveThread`: the environment selector collapses to a static label instead of offering move targets. The panel branch should forward `onMoveThread` to its own `BranchToolbarEnvironmentSelector` (or to the `BranchToolbar` that renders it), so the move-thread control stays interactive in the panel layout.
1 participant
Stacked on #2829 (
t3code/codex-turn-mapping), notmain. Every primitive this builds on —orchestration-v2/,OrchestrationV2*contracts,ContextHandoffService, the turn-item model — exists only on that branch;mainand the orchestrator branch share no common ancestor, so a version built againstmainwould be rewritten wholesale when the orchestrator lands.What this does
Today
Run onpicks the machine a thread starts on and locks once the thread has a message, so a thread is born on one device and dies there. This makes that reversible: from an open thread you pick another connected environment, and the conversation, the provider continuation, and the git working state the thread was left in all move there.Open a thread, click
Run on, pick a device under Move thread to, confirm. The origin thread goes read-only with a banner naming where it went; the thread continues on the other machine with its history and its uncommitted work.The safety model
A branch tip only ever moves to a descendant of itself, on either machine.
classifyIncomingTipreduces that to advance / absorb / diverged / unrelated as a pure function. Diverged or unrelated parks the sender's commits atrefs/handoff/<environment>/<branch>and stops with both machines untouched.Exactly one side is live.
thread.handoff.departlocks the giving side before any bundle is applied anywhere. That ordering is the invariant: an interrupted transfer leaves a locked thread thatthread.handoff.abortreleases, never a second conversation to merge.message.dispatchis refused while a thread is away, in the orchestrator rather than the client.Before any pointer moves the old tip is tagged and a dirty worktree stashed with the base sha in the label. The patch is dry-run applied first; one that will not apply resets to the tag and pops the stash rather than leaving the tree half-written.
applyingis the only state in which a repository can have been written to, so it is the only state startup recovery looks at.What travels
Branch and unpushed commits as a git bundle; tracked changes as a binary patch so images and lockfiles survive; untracked files as a null-delimited tar so paths with spaces do; the conversation as turn items with their covered run ordinals; the workspace strategy so a worktree thread can be reprovisioned. Checkpoints are hidden git refs, so bundling them with the commits carries the whole checkpoint timeline with no payload of its own and revert keeps working on the far side.
How it moves
The manifest is inline; the large parts are content-addressed and fetched by digest, so the transport can change later without moving the manifest or the ids with it. The client brokers the copy because it is the only component authenticated to both environments.
Part bytes move in 4 MB chunks. Chunking keeps neither server holding a whole payload in memory and makes a transfer that dies partway resumable from the offset it reached. The receiver rejects a chunk that does not continue exactly where the staged bytes end — a chunk written at the wrong offset would leave a hole that only surfaces later as a digest mismatch or a corrupt bundle, so it fails at the seam instead.
Payload ceilings are enforced while preparing, before anything has been sent: warn at 200 MB, refuse at 1 GB.
Lineage, not direction
Each hop records the previous hop rather than a push/pull direction, so
laptop → server → phone → laptopis a walk and a return trip is an ordinary hop toward an environment already in the chain. One nullable column; retrofitting it later would rewrite every existing row.Version skew
threadHandofffollows the same contract asthreadSettlement: absent means unsupported, so a client lists such an environment as an unavailable destination instead of starting a transfer that fails once bytes are already moving.Files
Contracts: bundle, ids, capability, commands, events, RPC and HTTP surface.
Server:
ThreadHandoffService(prepare / receive / recover),ThreadHandoffGit(classification and git plumbing), orchestrator commands and guards, projection, migration 046, part endpoints, startup recovery.Client:
runThreadHandoffTransfer(the hop across two environments), chunked part copy, the move command, theRun onmove group, the dialog, the departed banner.Verification
packages/contracts: 264 pass.packages/client-runtime: 570 pass.apps/server: 732 pass acrossorchestration-v2,persistence,environment,wsand startup. New coverage: the full tip-classification table, ref/tag/stash naming, payload-ceiling boundaries, chunk-window clamping and completion, part-name collisions, digest behaviour, conversation coverage, and four migration tests including duplicate-hop rejection and mid-apply recovery lookup.apps/web: 1841 pass. One pre-existing failure,imageCompression.test.ts, which fails identically on a clean tree here (a 15s timeout on this box) and is unrelated to this branch.pnpm typecheckclean across the monorepo.vp lintclean.vp fmt --checkclean.Not yet exercised end to end against two live servers. The unit and integration coverage is real, but the round trip on two real environments with a dirty worktree has not been run.
Not in this PR
Attachment and terminal payloads (the part kind and the manifest field exist; nothing packs them yet), worktree reprovisioning and clone-from-bundle on arrival, mid-run departure via
interruptAndAwaitTerminal, the pull-back affordance in the UI, and mobile.Note for review
Migration id 046 is also taken by #5499 (session import) on a sibling branch off the same base. Whichever merges second renumbers.