Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions apps/server/src/checkpointing/Utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,18 @@ export function checkpointRefForThreadTurn(threadId: ThreadId, turnCount: number
);
}

/**
* Working-tree snapshot taken right before a revert so a failed provider
* rollback can be compensated back to the exact pre-revert state — prior
* checkpoints can miss uncommitted local edits. One per thread; the capture
* overwrites it on each revert attempt via `git update-ref`.
*/
export function checkpointRefForRevertBase(threadId: ThreadId): CheckpointRef {
return CheckpointRef.make(
`${CHECKPOINT_REFS_PREFIX}/${Encoding.encodeBase64Url(threadId)}/revert-base`,
);
}

export function resolveThreadWorkspaceCwd(input: {
readonly thread: {
readonly projectId: ProjectId;
Expand Down
97 changes: 96 additions & 1 deletion apps/server/src/orchestration/Layers/CheckpointReactor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ import {
ProviderService,
type ProviderServiceShape,
} from "../../provider/Services/ProviderService.ts";
import { ProviderAdapterRequestError, type ProviderServiceError } from "../../provider/Errors.ts";
import { checkpointRefForThreadTurn } from "../../checkpointing/Utils.ts";
import { ServerConfig } from "../../config.ts";
import * as WorkspaceEntries from "../../workspace/WorkspaceEntries.ts";
Expand Down Expand Up @@ -85,7 +86,10 @@ function createProviderServiceHarness(
const now = "2026-01-01T00:00:00.000Z";
const runtimeEventPubSub = Effect.runSync(PubSub.unbounded<ProviderRuntimeEvent>());
const rollbackConversation = vi.fn(
(_input: { readonly threadId: ThreadId; readonly numTurns: number }) => Effect.void,
(_input: {
readonly threadId: ThreadId;
readonly numTurns: number;
}): Effect.Effect<void, ProviderServiceError> => Effect.void,
);

const unsupported = <A>() =>
Expand Down Expand Up @@ -1083,6 +1087,97 @@ describe("CheckpointReactor", () => {
).toBe(false);
});

it("does not restore the workspace when the provider rejects the conversation rollback", async () => {
const harness = await createHarness();
const createdAt = "2026-01-01T00:00:00.000Z";

await Effect.runPromise(
harness.engine.dispatch({
type: "thread.session.set",
commandId: CommandId.make("cmd-session-set-rejected-rollback"),
threadId: ThreadId.make("thread-1"),
session: {
threadId: ThreadId.make("thread-1"),
status: "ready",
providerName: "codex",
runtimeMode: "approval-required",
activeTurnId: null,
lastError: null,
updatedAt: createdAt,
},
createdAt,
}),
);

await Effect.runPromise(
harness.engine.dispatch({
type: "thread.turn.diff.complete",
commandId: CommandId.make("cmd-diff-rejected-1"),
threadId: ThreadId.make("thread-1"),
turnId: asTurnId("turn-rejected-1"),
completedAt: createdAt,
checkpointRef: checkpointRefForThreadTurn(ThreadId.make("thread-1"), 1),
status: "ready",
files: [],
checkpointTurnCount: 1,
createdAt,
}),
);
await Effect.runPromise(
harness.engine.dispatch({
type: "thread.turn.diff.complete",
commandId: CommandId.make("cmd-diff-rejected-2"),
threadId: ThreadId.make("thread-1"),
turnId: asTurnId("turn-rejected-2"),
completedAt: createdAt,
checkpointRef: checkpointRefForThreadTurn(ThreadId.make("thread-1"), 2),
status: "ready",
files: [],
checkpointTurnCount: 2,
createdAt,
}),
);

harness.provider.rollbackConversation.mockImplementation(() =>
Effect.fail(
new ProviderAdapterRequestError({
provider: "codex",
method: "thread/rollback",
detail: "paginated threads do not support thread/rollback",
}),
),
);

// Git checkpoint restore can rewrite LF as CRLF under core.autocrlf, so
// compare line-ending-normalized content when asserting the workspace is
// back to its pre-revert state.
const normalize = (value: string): string => value.replace(/\r\n/g, "\n");
const readmeBeforeRevert = normalize(
NodeFS.readFileSync(NodePath.join(harness.cwd, "README.md"), "utf8"),
);

await Effect.runPromise(
harness.engine.dispatch({
type: "thread.checkpoint.revert",
commandId: CommandId.make("cmd-revert-request-rejected"),
threadId: ThreadId.make("thread-1"),
turnCount: 1,
createdAt,
}),
);

const thread = await waitForThread(harness.readModel, (entry) =>
entry.activities.some((activity) => activity.kind === "checkpoint.revert.failed"),
);
expect(harness.provider.rollbackConversation).toHaveBeenCalledTimes(1);
expect(
thread.activities.some((activity) => activity.kind === "checkpoint.revert.failed"),
).toBe(true);
expect(
normalize(NodeFS.readFileSync(NodePath.join(harness.cwd, "README.md"), "utf8")),
).toBe(readmeBeforeRevert);
});

it("executes provider revert and emits thread.reverted for claude sessions", async () => {
const harness = await createHarness({ providerName: ProviderDriverKind.make("claudeAgent") });
const createdAt = "2026-01-01T00:00:00.000Z";
Expand Down
48 changes: 43 additions & 5 deletions apps/server/src/orchestration/Layers/CheckpointReactor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,12 @@ import { isTemporaryWorktreeBranch } from "@t3tools/shared/git";

import { parseTurnDiffFilesFromUnifiedDiff } from "../../checkpointing/Diffs.ts";
import {
checkpointRefForRevertBase,
checkpointRefForThreadTurn,
resolveThreadWorkspaceCwd,
} from "../../checkpointing/Utils.ts";
import * as CheckpointStore from "../../checkpointing/CheckpointStore.ts";
import type { ProviderServiceError } from "../../provider/Errors.ts";
import { ProviderService } from "../../provider/Services/ProviderService.ts";
import { CheckpointReactor, type CheckpointReactorShape } from "../Services/CheckpointReactor.ts";
import { forkParked } from "../../serverActivation.ts";
Expand Down Expand Up @@ -755,6 +757,15 @@ const make = Effect.gen(function* () {
return;
}

// Snapshot the current worktree before touching anything so a failed
// provider rollback can be compensated back to the exact pre-revert
// state — prior checkpoints can miss uncommitted local edits.
const revertBaseRef = checkpointRefForRevertBase(event.payload.threadId);
yield* checkpointStore.captureCheckpoint({
cwd: sessionRuntime.value.cwd,
checkpointRef: revertBaseRef,
});

Comment thread
cursor[bot] marked this conversation as resolved.
const restored = yield* checkpointStore.restoreCheckpoint({
Comment thread
macroscopeapp[bot] marked this conversation as resolved.
cwd: sessionRuntime.value.cwd,
checkpointRef: targetCheckpointRef,
Expand All @@ -776,13 +787,40 @@ const make = Effect.gen(function* () {

const rolledBackTurns = Math.max(0, currentTurnCount - event.payload.turnCount);
if (rolledBackTurns > 0) {
yield* providerService.rollbackConversation({
threadId: sessionRuntime.value.threadId,
numTurns: rolledBackTurns,
});
const rollbackFailure = yield* providerService
.rollbackConversation({
threadId: sessionRuntime.value.threadId,
numTurns: rolledBackTurns,
})
.pipe(
Effect.map(() => Option.none<ProviderServiceError>()),
Effect.catch((error) => Effect.succeed(Option.some(error))),
);
if (Option.isSome(rollbackFailure)) {
// Compensate: the workspace moved but the conversation did not.
// Restore the pre-revert snapshot so both sides stay in their
// original state and retrying the revert starts from a clean slate.
yield* checkpointStore

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Medium Layers/CheckpointReactor.ts:803

When provider rollback fails, the compensation restores file contents but leaves any pre-existing staged edits unstaged, so it does not restore the claimed exact pre-revert state. captureCheckpoint stores only a tree commit, while restoreCheckpoint resets the index to HEAD after restoring with --staged --worktree; preserve the original index state as part of the revert-base snapshot and restore it here.

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration/Layers/CheckpointReactor.ts around line 803:

When provider rollback fails, the compensation restores file contents but leaves any pre-existing staged edits unstaged, so it does not restore the claimed exact pre-revert state. `captureCheckpoint` stores only a tree commit, while `restoreCheckpoint` resets the index to `HEAD` after restoring with `--staged --worktree`; preserve the original index state as part of the revert-base snapshot and restore it here.

.restoreCheckpoint({
cwd: sessionRuntime.value.cwd,
checkpointRef: revertBaseRef,
fallbackToHead: false,
})
.pipe(
Effect.andThen(workspaceEntries.refresh(sessionRuntime.value.cwd)),
Effect.catch(() => Effect.void),
);
yield* appendRevertFailureActivity({
threadId: event.payload.threadId,
turnCount: event.payload.turnCount,
detail: rollbackFailure.value.message,
createdAt: now,
}).pipe(Effect.catch(() => Effect.void));
return;
}
}

const staleCheckpointRefs: Array<CheckpointRef> = [];
const staleCheckpointRefs: Array<CheckpointRef> = [revertBaseRef];
for (const checkpoint of thread.checkpoints) {
if (checkpoint.checkpointTurnCount > event.payload.turnCount) {
staleCheckpointRefs.push(checkpoint.checkpointRef);
Expand Down
Loading