| author |  Theo Browne <me@t3.gg> 2026-10-07 22:20:16-07:00 |
| committer |  GitHub <noreply@github.com> 2026-10-07 22:20:16-07:00 |
| log | c28cd905572e2a28934a6229d88bcf2975963032 |
| tree | a27e5fc6d9824129a9f0eff6fcc3f1c2b81c4774 |
| parent | 0bfd9dd19d0c9418a936190525ec8f4c4fdae09b |
| signature | Signed by PGP key B5690EEEBB952194 |
fix(server): a message sent during a rollback no longer undoes it (#17079)
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>5 files changed, 95 insertions(+), 61 deletions(-)
apps
apps/server/src/orchestration-v2/CheckpointRestoreSafety.test.ts+2
| ... | @@ -17,6 +17,7 @@ import * as Option from "effect/Option"; | ... | @@ -17,6 +17,7 @@ import * as Option from "effect/Option"; |
| 17 | import * as Path from "effect/Path"; | 17 | import * as Path from "effect/Path"; |
| 18 | import { symlinksSupported } from "@t3tools/shared/testing/symlinks"; | 18 | import { symlinksSupported } from "@t3tools/shared/testing/symlinks"; |
| 19 | import * as CheckpointRollbackService from "./CheckpointRollbackService.ts"; | 19 | import * as CheckpointRollbackService from "./CheckpointRollbackService.ts"; |
| 20 | import * as ThreadCommandExecutor from "./ThreadCommandExecutor.ts"; | ||
| 20 | import { CheckpointServiceV2 } from "./CheckpointService.ts"; | 21 | import { CheckpointServiceV2 } from "./CheckpointService.ts"; |
| 21 | import { EventSinkV2 } from "./EventSink.ts"; | 22 | import { EventSinkV2 } from "./EventSink.ts"; |
| 22 | import * as IdAllocator from "./IdAllocator.ts"; | 23 | import * as IdAllocator from "./IdAllocator.ts"; |
| ... | @@ -102,6 +103,7 @@ it.effect.each([ | ... | @@ -102,6 +103,7 @@ it.effect.each([ |
| 102 | Layer.provide( | 103 | Layer.provide( |
| 103 | Layer.mergeAll( | 104 | Layer.mergeAll( |
| 104 | NodeServices.layer, | 105 | NodeServices.layer, |
| 106 | ThreadCommandExecutor.layer, | ||
| 105 | IdAllocator.layer, | 107 | IdAllocator.layer, |
| 106 | Layer.mock(ProjectStore.ProjectStoreV2)({ | 108 | Layer.mock(ProjectStore.ProjectStoreV2)({ |
| 107 | get: () => Effect.succeed(Option.some({ workspaceRoot: parent } as never)), | 109 | get: () => Effect.succeed(Option.some({ workspaceRoot: parent } as never)), |
apps/server/src/orchestration-v2/CheckpointRollbackService.test.ts+20-1
| ... | @@ -20,6 +20,8 @@ import { resolveCodexRollbackTurnCount } from "./Adapters/CodexAdapterV2.ts"; | ... | @@ -20,6 +20,8 @@ import { resolveCodexRollbackTurnCount } from "./Adapters/CodexAdapterV2.ts"; |
| 20 | import { isCheckpointRestoreIsolated } from "./CheckpointRestoreSafety.ts"; | 20 | import { isCheckpointRestoreIsolated } from "./CheckpointRestoreSafety.ts"; |
| 21 | import * as CheckpointService from "./CheckpointService.ts"; | 21 | import * as CheckpointService from "./CheckpointService.ts"; |
| 22 | import * as CheckpointRollbackService from "./CheckpointRollbackService.ts"; | 22 | import * as CheckpointRollbackService from "./CheckpointRollbackService.ts"; |
| 23 | import * as ThreadCommandExecutor from "./ThreadCommandExecutor.ts"; | ||
| 24 | import * as KeyedLock from "@t3tools/shared/KeyedLock"; | ||
| 23 | import * as EventSink from "./EventSink.ts"; | 25 | import * as EventSink from "./EventSink.ts"; |
| 24 | import * as IdAllocator from "./IdAllocator.ts"; | 26 | import * as IdAllocator from "./IdAllocator.ts"; |
| 25 | import * as ProjectionStore from "./ProjectionStore.ts"; | 27 | import * as ProjectionStore from "./ProjectionStore.ts"; |
| ... | @@ -64,6 +66,7 @@ it.effect("rejects a non-ready checkpoint before opening a session or restoring | ... | @@ -64,6 +66,7 @@ it.effect("rejects a non-ready checkpoint before opening a session or restoring |
| 64 | const layerTest = layerCheckpointRollbackService.pipe( | 66 | const layerTest = layerCheckpointRollbackService.pipe( |
| 65 | Layer.provide( | 67 | Layer.provide( |
| 66 | Layer.mergeAll( | 68 | Layer.mergeAll( |
| 69 | ThreadCommandExecutor.layer, | ||
| 67 | Layer.mock(CheckpointService.CheckpointServiceV2)({ restore }), | 70 | Layer.mock(CheckpointService.CheckpointServiceV2)({ restore }), |
| 68 | Layer.mock(EventSink.EventSinkV2)({}), | 71 | Layer.mock(EventSink.EventSinkV2)({}), |
| 69 | IdAllocator.layer, | 72 | IdAllocator.layer, |
| ... | @@ -141,6 +144,7 @@ it.effect("rejects a rollback when another provider thread became active", () => | ... | @@ -141,6 +144,7 @@ it.effect("rejects a rollback when another provider thread became active", () => |
| 141 | const layerTest = layerCheckpointRollbackService.pipe( | 144 | const layerTest = layerCheckpointRollbackService.pipe( |
| 142 | Layer.provide( | 145 | Layer.provide( |
| 143 | Layer.mergeAll( | 146 | Layer.mergeAll( |
| 147 | ThreadCommandExecutor.layer, | ||
| 144 | Layer.mock(CheckpointService.CheckpointServiceV2)({ restore }), | 148 | Layer.mock(CheckpointService.CheckpointServiceV2)({ restore }), |
| 145 | Layer.mock(EventSink.EventSinkV2)({}), | 149 | Layer.mock(EventSink.EventSinkV2)({}), |
| 146 | IdAllocator.layer, | 150 | IdAllocator.layer, |
| ... | @@ -221,6 +225,7 @@ it.effect("rejects a rollback when provider selection changed before execution", | ... | @@ -221,6 +225,7 @@ it.effect("rejects a rollback when provider selection changed before execution", |
| 221 | const layerTest = layerCheckpointRollbackService.pipe( | 225 | const layerTest = layerCheckpointRollbackService.pipe( |
| 222 | Layer.provide( | 226 | Layer.provide( |
| 223 | Layer.mergeAll( | 227 | Layer.mergeAll( |
| 228 | ThreadCommandExecutor.layer, | ||
| 224 | Layer.mock(CheckpointService.CheckpointServiceV2)({ restore }), | 229 | Layer.mock(CheckpointService.CheckpointServiceV2)({ restore }), |
| 225 | Layer.mock(EventSink.EventSinkV2)({}), | 230 | Layer.mock(EventSink.EventSinkV2)({}), |
| 226 | IdAllocator.layer, | 231 | IdAllocator.layer, |
| ... | @@ -292,6 +297,7 @@ it.effect("reports a missing provider turn as a structured rollback failure", () | ... | @@ -292,6 +297,7 @@ it.effect("reports a missing provider turn as a structured rollback failure", () |
| 292 | const layerTest = layerCheckpointRollbackService.pipe( | 297 | const layerTest = layerCheckpointRollbackService.pipe( |
| 293 | Layer.provide( | 298 | Layer.provide( |
| 294 | Layer.mergeAll( | 299 | Layer.mergeAll( |
| 300 | ThreadCommandExecutor.layer, | ||
| 295 | Layer.mock(CheckpointService.CheckpointServiceV2)({ restore }), | 301 | Layer.mock(CheckpointService.CheckpointServiceV2)({ restore }), |
| 296 | Layer.mock(EventSink.EventSinkV2)({}), | 302 | Layer.mock(EventSink.EventSinkV2)({}), |
| 297 | IdAllocator.layer, | 303 | IdAllocator.layer, |
| ... | @@ -352,6 +358,15 @@ it.effect.each([ | ... | @@ -352,6 +358,15 @@ it.effect.each([ |
| 352 | const checkpointId = CheckpointId.make("rewind-start"); | 358 | const checkpointId = CheckpointId.make("rewind-start"); |
| 353 | const scopeId = CheckpointScopeId.make("rewind-scope"); | 359 | const scopeId = CheckpointScopeId.make("rewind-scope"); |
| 354 | const calls: string[] = []; | 360 | const calls: string[] = []; |
| 361 | let lockedThreads = Effect.succeed<ReadonlyArray<ThreadId>>([]); | ||
| 362 | const layerThreadCommands = Layer.effect( | ||
| 363 | ThreadCommandExecutor.ThreadCommandExecutor, | ||
| 364 | Effect.tap(KeyedLock.make<ThreadId>(), (lock) => | ||
| 365 | Effect.sync(() => { | ||
| 366 | lockedThreads = lock.activeKeys; | ||
| 367 | }), | ||
| 368 | ), | ||
| 369 | ); | ||
| 355 | const providerThread = { | 370 | const providerThread = { |
| 356 | id: providerThreadId, | 371 | id: providerThreadId, |
| 357 | providerSessionId, | 372 | providerSessionId, |
| ... | @@ -390,6 +405,7 @@ it.effect.each([ | ... | @@ -390,6 +405,7 @@ it.effect.each([ |
| 390 | const layerTest = layerCheckpointRollbackService.pipe( | 405 | const layerTest = layerCheckpointRollbackService.pipe( |
| 391 | Layer.provide( | 406 | Layer.provide( |
| 392 | Layer.mergeAll( | 407 | Layer.mergeAll( |
| 408 | layerThreadCommands, | ||
| 393 | Layer.mock(CheckpointService.CheckpointServiceV2)({ | 409 | Layer.mock(CheckpointService.CheckpointServiceV2)({ |
| 394 | restore: () => | 410 | restore: () => |
| 395 | Effect.sync(() => { | 411 | Effect.sync(() => { |
| ... | @@ -398,7 +414,10 @@ it.effect.each([ | ... | @@ -398,7 +414,10 @@ it.effect.each([ |
| 398 | }), | 414 | }), |
| 399 | Layer.mock(EventSink.EventSinkV2)({ | 415 | Layer.mock(EventSink.EventSinkV2)({ |
| 400 | write: ({ events }) => | 416 | write: ({ events }) => |
| 401 | Effect.sync(() => { | 417 | Effect.map(lockedThreads, (locked) => { |
| 418 | // A thread command that planned against the old provider thread | ||
| 419 | // must not commit after this and undo the rollback. | ||
| 420 | assert.include(locked, threadId); | ||
| 402 | assert.ok( | 421 | assert.ok( |
| 403 | events.some( | 422 | events.some( |
| 404 | (event) => event.type === "run.updated" && event.payload.status === "rolled_back", | 423 | (event) => event.type === "run.updated" && event.payload.status === "rolled_back", |
apps/server/src/orchestration-v2/CheckpointRollbackService.ts+71-60
| ... | @@ -26,6 +26,7 @@ import * as ProjectStore from "./ProjectStore.ts"; | ... | @@ -26,6 +26,7 @@ import * as ProjectStore from "./ProjectStore.ts"; |
| 26 | import type { ProviderAdapterV2RollbackTarget } from "./ProviderAdapter.ts"; | 26 | import type { ProviderAdapterV2RollbackTarget } from "./ProviderAdapter.ts"; |
| 27 | import { ProviderSessionManagerV2 } from "./ProviderSessionManager.ts"; | 27 | import { ProviderSessionManagerV2 } from "./ProviderSessionManager.ts"; |
| 28 | import { RuntimePolicyV2 } from "./RuntimePolicy.ts"; | 28 | import { RuntimePolicyV2 } from "./RuntimePolicy.ts"; |
| 29 | import * as ThreadCommandExecutor from "./ThreadCommandExecutor.ts"; | ||
| 29 | 30 | ||
| 30 | export const ROLLBACK_FAILED_MESSAGE = | 31 | export const ROLLBACK_FAILED_MESSAGE = |
| 31 | "The provider could not roll back this conversation. Try again; if it keeps failing, check the provider and server logs."; | 32 | "The provider could not roll back this conversation. Try again; if it keeps failing, check the provider and server logs."; |
| ... | @@ -91,6 +92,7 @@ export const layer: Layer.Layer< | ... | @@ -91,6 +92,7 @@ export const layer: Layer.Layer< |
| 91 | | FileSystem.FileSystem | 92 | | FileSystem.FileSystem |
| 92 | | Path.Path | 93 | | Path.Path |
| 93 | | ProjectStore.ProjectStoreV2 | 94 | | ProjectStore.ProjectStoreV2 |
| 95 | | ThreadCommandExecutor.ThreadCommandExecutor | ||
| 94 | > = Layer.effect( | 96 | > = Layer.effect( |
| 95 | CheckpointRollbackServiceV2, | 97 | CheckpointRollbackServiceV2, |
| 96 | Effect.gen(function* () { | 98 | Effect.gen(function* () { |
| ... | @@ -103,6 +105,7 @@ export const layer: Layer.Layer< | ... | @@ -103,6 +105,7 @@ export const layer: Layer.Layer< |
| 103 | const fileSystem = yield* FileSystem.FileSystem; | 105 | const fileSystem = yield* FileSystem.FileSystem; |
| 104 | const projects = yield* ProjectStore.ProjectStoreV2; | 106 | const projects = yield* ProjectStore.ProjectStoreV2; |
| 105 | const path = yield* Path.Path; | 107 | const path = yield* Path.Path; |
| 108 | const threadCommands = yield* ThreadCommandExecutor.ThreadCommandExecutor; | ||
| 106 | 109 | ||
| 107 | const execute = Effect.fn("orchestrationV2.checkpointRollback.execute")(function* (input: { | 110 | const execute = Effect.fn("orchestrationV2.checkpointRollback.execute")(function* (input: { |
| 108 | readonly threadId: ThreadId; | 111 | readonly threadId: ThreadId; |
| ... | @@ -274,72 +277,80 @@ export const layer: Layer.Layer< | ... | @@ -274,72 +277,80 @@ export const layer: Layer.Layer< |
| 274 | yield* checkpoints.deleteStaleRefs({ scope, checkpoints: staleCheckpoints }); | 277 | yield* checkpoints.deleteStaleRefs({ scope, checkpoints: staleCheckpoints }); |
| 275 | } | 278 | } |
| 276 | 279 | ||
| 277 | const now = yield* DateTime.now; | 280 | // Thread commands write full provider thread rows under this lock. Without |
| 278 | const makeEvent = <Event extends OrchestrationV2DomainEvent>(event: Omit<Event, "id">) => | 281 | // it, a message sent during the rollback can plan against the old native |
| 279 | Effect.map( | 282 | // session and commit after this, undoing the provider's fork. |
| 280 | ids.allocate.event({ threadId: event.threadId }), | 283 | yield* threadCommands.withLock( |
| 281 | (id) => | 284 | input.threadId, |
| 282 | ({ | 285 | Effect.gen(function* () { |
| 283 | ...event, | 286 | const now = yield* DateTime.now; |
| 284 | id, | 287 | const makeEvent = <Event extends OrchestrationV2DomainEvent>(event: Omit<Event, "id">) => |
| 285 | }) as Event, | 288 | Effect.map( |
| 286 | ); | 289 | ids.allocate.event({ threadId: event.threadId }), |
| 287 | const events: Array<OrchestrationV2DomainEvent> = []; | 290 | (id) => |
| 288 | events.push( | 291 | ({ |
| 289 | yield* makeEvent({ | 292 | ...event, |
| 290 | type: "provider-thread.updated", | 293 | id, |
| 291 | threadId: input.threadId, | 294 | }) as Event, |
| 292 | driver: providerThread.driver, | 295 | ); |
| 293 | providerInstanceId: providerThread.providerInstanceId, | 296 | const events: Array<OrchestrationV2DomainEvent> = []; |
| 294 | occurredAt: now, | ||
| 295 | payload: { | ||
| 296 | ...snapshot.providerThread, | ||
| 297 | lastRunOrdinal: targetOrdinal === 0 ? null : targetOrdinal, | ||
| 298 | updatedAt: now, | ||
| 299 | }, | ||
| 300 | }), | ||
| 301 | ); | ||
| 302 | for (const staleCheckpoint of staleCheckpoints) { | ||
| 303 | events.push( | ||
| 304 | yield* makeEvent({ | ||
| 305 | type: "checkpoint.captured", | ||
| 306 | threadId: input.threadId, | ||
| 307 | ...(staleCheckpoint.runId === null ? {} : { runId: staleCheckpoint.runId }), | ||
| 308 | nodeId: staleCheckpoint.nodeId, | ||
| 309 | providerInstanceId: providerThread.providerInstanceId, | ||
| 310 | occurredAt: now, | ||
| 311 | payload: { ...staleCheckpoint, status: "stale" }, | ||
| 312 | }), | ||
| 313 | ); | ||
| 314 | } | ||
| 315 | for (const run of runsToRollback) { | ||
| 316 | const rootNode = projection.nodes.find((candidate) => candidate.id === run.rootNodeId); | ||
| 317 | events.push( | ||
| 318 | yield* makeEvent({ | ||
| 319 | type: "run.updated", | ||
| 320 | threadId: input.threadId, | ||
| 321 | runId: run.id, | ||
| 322 | ...(rootNode === undefined ? {} : { nodeId: rootNode.id }), | ||
| 323 | providerInstanceId: run.providerInstanceId, | ||
| 324 | occurredAt: now, | ||
| 325 | payload: { ...run, status: "rolled_back", completedAt: now }, | ||
| 326 | }), | ||
| 327 | ); | ||
| 328 | if (rootNode !== undefined) { | ||
| 329 | events.push( | 297 | events.push( |
| 330 | yield* makeEvent({ | 298 | yield* makeEvent({ |
| 331 | type: "node.updated", | 299 | type: "provider-thread.updated", |
| 332 | threadId: input.threadId, | 300 | threadId: input.threadId, |
| 333 | runId: run.id, | 301 | driver: providerThread.driver, |
| 334 | nodeId: rootNode.id, | 302 | providerInstanceId: providerThread.providerInstanceId, |
| 335 | providerInstanceId: run.providerInstanceId, | ||
| 336 | occurredAt: now, | 303 | occurredAt: now, |
| 337 | payload: { ...rootNode, status: "rolled_back", completedAt: now }, | 304 | payload: { |
| 305 | ...snapshot.providerThread, | ||
| 306 | lastRunOrdinal: targetOrdinal === 0 ? null : targetOrdinal, | ||
| 307 | updatedAt: now, | ||
| 308 | }, | ||
| 338 | }), | 309 | }), |
| 339 | ); | 310 | ); |
| 340 | } | 311 | for (const staleCheckpoint of staleCheckpoints) { |
| 341 | } | 312 | events.push( |
| 342 | yield* eventSink.write({ events }); | 313 | yield* makeEvent({ |
| 314 | type: "checkpoint.captured", | ||
| 315 | threadId: input.threadId, | ||
| 316 | ...(staleCheckpoint.runId === null ? {} : { runId: staleCheckpoint.runId }), | ||
| 317 | nodeId: staleCheckpoint.nodeId, | ||
| 318 | providerInstanceId: providerThread.providerInstanceId, | ||
| 319 | occurredAt: now, | ||
| 320 | payload: { ...staleCheckpoint, status: "stale" }, | ||
| 321 | }), | ||
| 322 | ); | ||
| 323 | } | ||
| 324 | for (const run of runsToRollback) { | ||
| 325 | const rootNode = projection.nodes.find((candidate) => candidate.id === run.rootNodeId); | ||
| 326 | events.push( | ||
| 327 | yield* makeEvent({ | ||
| 328 | type: "run.updated", | ||
| 329 | threadId: input.threadId, | ||
| 330 | runId: run.id, | ||
| 331 | ...(rootNode === undefined ? {} : { nodeId: rootNode.id }), | ||
| 332 | providerInstanceId: run.providerInstanceId, | ||
| 333 | occurredAt: now, | ||
| 334 | payload: { ...run, status: "rolled_back", completedAt: now }, | ||
| 335 | }), | ||
| 336 | ); | ||
| 337 | if (rootNode !== undefined) { | ||
| 338 | events.push( | ||
| 339 | yield* makeEvent({ | ||
| 340 | type: "node.updated", | ||
| 341 | threadId: input.threadId, | ||
| 342 | runId: run.id, | ||
| 343 | nodeId: rootNode.id, | ||
| 344 | providerInstanceId: run.providerInstanceId, | ||
| 345 | occurredAt: now, | ||
| 346 | payload: { ...rootNode, status: "rolled_back", completedAt: now }, | ||
| 347 | }), | ||
| 348 | ); | ||
| 349 | } | ||
| 350 | } | ||
| 351 | yield* eventSink.write({ events }); | ||
| 352 | }), | ||
| 353 | ); | ||
| 343 | }); | 354 | }); |
| 344 | 355 | ||
| 345 | return CheckpointRollbackServiceV2.of({ | 356 | return CheckpointRollbackServiceV2.of({ |
apps/server/src/orchestration-v2/runtimeLayer.ts+1
| ... | @@ -182,6 +182,7 @@ const layerCheckpointRollbackServiceProvided = CheckpointRollbackService.layer.p | ... | @@ -182,6 +182,7 @@ const layerCheckpointRollbackServiceProvided = CheckpointRollbackService.layer.p |
| 182 | IdAllocator.layer, | 182 | IdAllocator.layer, |
| 183 | ProjectionStore.layer, | 183 | ProjectionStore.layer, |
| 184 | layerProviderSessionManagerProvided, | 184 | layerProviderSessionManagerProvided, |
| 185 | ThreadCommandExecutor.layer, | ||
| 185 | layerRuntimePolicyProvided, | 186 | layerRuntimePolicyProvided, |
| 186 | ), | 187 | ), |
| 187 | ), | 188 | ), |
apps/server/src/orchestration-v2/testkit/ProviderReplayHarness.ts+1
| ... | @@ -395,6 +395,7 @@ export function layerWithRegistry<Error>( | ... | @@ -395,6 +395,7 @@ export function layerWithRegistry<Error>( |
| 395 | IdAllocator.layer, | 395 | IdAllocator.layer, |
| 396 | layerStores, | 396 | layerStores, |
| 397 | layerProviderSessionManagerProvided, | 397 | layerProviderSessionManagerProvided, |
| 398 | ThreadCommandExecutor.layer, | ||
| 398 | layerRuntime, | 399 | layerRuntime, |
| 399 | ), | 400 | ), |
| 400 | ), | 401 | ), |