Skip to content
Draft
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
141 changes: 141 additions & 0 deletions .github/alloy/CompletionPersistence.als
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
module CompletionPersistence

abstract sig CompletionPolicy {}
one sig CurrentPolicy, DurableFirstPolicy extends CompletionPolicy {}

one sig Config {
policy: one CompletionPolicy
}

one sig Marker {}

one sig Lifecycle {
var historyWriteStarted: lone Marker,
var historyDurable: lone Marker,
var completionAccepted: lone Marker,
var completionEmitted: lone Marker,
var hostStopped: lone Marker
}

pred init {
no Lifecycle.historyWriteStarted
no Lifecycle.historyDurable
no Lifecycle.completionAccepted
no Lifecycle.completionEmitted
no Lifecycle.hostStopped
}

pred startHistoryWrite {
no Lifecycle.historyWriteStarted
no Lifecycle.hostStopped
Lifecycle.historyWriteStarted' = Marker
Lifecycle.historyDurable' = Lifecycle.historyDurable
Lifecycle.completionAccepted' = Lifecycle.completionAccepted
Lifecycle.completionEmitted' = Lifecycle.completionEmitted
Lifecycle.hostStopped' = Lifecycle.hostStopped
}

pred finishHistoryWrite {
some Lifecycle.historyWriteStarted
no Lifecycle.historyDurable
no Lifecycle.hostStopped
Lifecycle.historyWriteStarted' = Lifecycle.historyWriteStarted
Lifecycle.historyDurable' = Marker
Lifecycle.completionAccepted' = Lifecycle.completionAccepted
Lifecycle.completionEmitted' = Lifecycle.completionEmitted
Lifecycle.hostStopped' = Lifecycle.hostStopped
}

pred acceptCompletion {
no Lifecycle.completionAccepted
no Lifecycle.hostStopped
Lifecycle.historyWriteStarted' = Lifecycle.historyWriteStarted
Lifecycle.historyDurable' = Lifecycle.historyDurable
Lifecycle.completionAccepted' = Marker
Lifecycle.completionEmitted' = Lifecycle.completionEmitted
Lifecycle.hostStopped' = Lifecycle.hostStopped
}

pred emitCompletion {
some Lifecycle.historyWriteStarted
some Lifecycle.completionAccepted
no Lifecycle.completionEmitted
no Lifecycle.hostStopped
Config.policy = DurableFirstPolicy implies some Lifecycle.historyDurable
Lifecycle.historyWriteStarted' = Lifecycle.historyWriteStarted
Lifecycle.historyDurable' = Lifecycle.historyDurable
Lifecycle.completionAccepted' = Lifecycle.completionAccepted
Lifecycle.completionEmitted' = Marker
Lifecycle.hostStopped' = Lifecycle.hostStopped
}

pred stopHost {
some Lifecycle.completionEmitted
no Lifecycle.hostStopped
Lifecycle.historyWriteStarted' = Lifecycle.historyWriteStarted
Lifecycle.historyDurable' = Lifecycle.historyDurable
Lifecycle.completionAccepted' = Lifecycle.completionAccepted
Lifecycle.completionEmitted' = Lifecycle.completionEmitted
Lifecycle.hostStopped' = Marker
}

pred stutter {
Lifecycle.historyWriteStarted' = Lifecycle.historyWriteStarted
Lifecycle.historyDurable' = Lifecycle.historyDurable
Lifecycle.completionAccepted' = Lifecycle.completionAccepted
Lifecycle.completionEmitted' = Lifecycle.completionEmitted
Lifecycle.hostStopped' = Lifecycle.hostStopped
}

fact traces {
init
always (
startHistoryWrite or
finishHistoryWrite or
acceptCompletion or
emitCompletion or
stopHost or
stutter
)
}

pred DurableFirstHappyPath {
Config.policy = DurableFirstPolicy
eventually (
some Lifecycle.hostStopped and
some Lifecycle.completionEmitted and
some Lifecycle.historyDurable
)
}

assert CurrentCompletionIsDurable {
Config.policy = CurrentPolicy implies
always (some Lifecycle.completionEmitted implies some Lifecycle.historyDurable)
}

assert CurrentShutdownPreservesHistory {
Config.policy = CurrentPolicy implies
always (
some Lifecycle.hostStopped and some Lifecycle.completionEmitted implies
some Lifecycle.historyDurable
)
}

assert DurableFirstCompletionIsDurable {
Config.policy = DurableFirstPolicy implies
always (some Lifecycle.completionEmitted implies some Lifecycle.historyDurable)
}

assert DurableFirstShutdownPreservesHistory {
Config.policy = DurableFirstPolicy implies
always (
some Lifecycle.hostStopped and some Lifecycle.completionEmitted implies
some Lifecycle.historyDurable
)
}

check CurrentCompletionIsDurable for 6 but 6 steps
check CurrentShutdownPreservesHistory for 6 but 6 steps
run DurableFirstHappyPath for 6 but 6 steps
check DurableFirstCompletionIsDurable for 6 but 8 steps
check DurableFirstShutdownPreservesHistory for 6 but 8 steps
57 changes: 57 additions & 0 deletions .github/alloy/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
# Completion persistence model

`CompletionPersistence.als` models the narrow lifecycle behind the restart-persistence E2E failure:

- the streamed assistant history write starts;
- completion is accepted and `TaskCompleted` is emitted;
- the history write becomes durable;
- the extension host stops after observing completion.

The model compares two event contracts:

- `CurrentPolicy` permits `TaskCompleted` once completion is accepted and a history write has started;
- `DurableFirstPolicy` additionally requires the history write to be durable before completion is emitted.

The current-policy assertions search for a hypothesized, contract-permitted bad shape: the host sees completion and stops while API history is still not durable. Here, durable means that the required history version is visible to a fresh extension host; the model does not claim power-loss durability or filesystem `fsync` semantics. The durability-gated assertions check that completion and shutdown cannot expose that state.

The model is intentionally small. It establishes the missing ordering invariant but does not prove that the CI failure followed this exact trace or that every concrete runtime path maps to the abstract current-policy transition. Unrestricted stuttering also means this is a bounded safety model: it does not guarantee write completion, retries, or eventual task completion when persistence keeps failing.

## Deterministic production regression

`src/core/task/__tests__/Task.persistence.spec.ts` blocks the real `saveApiMessages` boundary on a deferred promise and accepts completion on the same `Task`. It confirms that `TaskCompleted` remains pending while the write is unresolved, then emits after the write succeeds. A second case exhausts the bounded persistence retries and confirms that the failure is reported without emitting `TaskCompleted`.

The test maps to the model as follows:

- the captured `saveApiMessages` call for the assistant `attempt_completion` turn is `startHistoryWrite`;
- the unresolved deferred save is `not historyDurable`;
- accepting the matching completion call is `acceptCompletion`;
- resolving the deferred is `finishHistoryWrite`;
- observing `TaskCompleted` afterward is `emitCompletion`.

An indefinitely delayed write keeps completion pending rather than weakening the public event contract. A failed initial write is retried with the existing bounded retry policy; if all retries fail, the completion handler reports the persistence error and does not emit `TaskCompleted`.

## Code mapping

- `startHistoryWrite` and `finishHistoryWrite` represent `Task.saveApiConversationHistory()` entering and completing its durable file write.
- `acceptCompletion` and `emitCompletion` represent completion approval followed by `AttemptCompletionTool.emitPublicTaskCompleted()`.
- `stopHost` represents the restart E2E (or a real extension shutdown) acting on the public completion event.
- `DurableFirstPolicy` represents the production contract: the public completion boundary is not crossed until the required API history write succeeds.

## Run Alloy 6

Download the pinned Alloy release, verify it, and execute all commands:

```bash
cd .github/alloy
curl -fsSL https://github.com/AlloyTools/org.alloytools.alloy/releases/download/v6.2.0/org.alloytools.alloy.dist.jar -o alloy.jar
printf '%s %s\n' '6b8c1cb5bc93bedfc7c61435c4e1ab6e688a242dc702a394628d9a9801edb78d' alloy.jar | sha256sum --check
java -jar alloy.jar exec -c '*' -t text -o - CompletionPersistence.als
```

Expected results:

- both `Current...` checks produce counterexamples where completion precedes durable history, including a trace that stops the host in that state;
- `DurableFirstHappyPath` is satisfiable, so the stronger guard does not prevent completion;
- both `DurableFirst...` assertions have no counterexample within the configured bounds.

The JAR is a local analysis tool and must not be committed.
19 changes: 11 additions & 8 deletions apps/vscode-e2e/src/suite/restart-persistence.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,6 @@ async function runCreate(api: RooCodeAPI): Promise<void> {
})
await waitUntilCompleted({ api, taskId })
assert.strictEqual(sawMarker, true, `Completion should include ${MARKER}`)
const historyItem = await api.getTaskHistoryItem(taskId)
assert.ok(historyItem, "Completed task should have a history item")
assert.ok(historyItem.task.includes("RESTART_PERSISTENCE_SMOKE"), "History title should include the marker")
const conversationLength = await api.getTaskApiConversationHistoryLength(taskId)
assert.ok(conversationLength > 0, "Completed task should persist API conversation history")

const result: PhaseResult = {
version: PHASE_RESULT_VERSION,
Expand Down Expand Up @@ -84,14 +79,22 @@ async function runVerify(api: RooCodeAPI): Promise<void> {
const historyItem = await api.getTaskHistoryItem(taskId)
assert.ok(historyItem, "Task history item should be available after restart")
assert.ok(historyItem.task.includes("RESTART_PERSISTENCE_SMOKE"), "History title should persist after restart")
const conversationLength = await api.getTaskApiConversationHistoryLength(taskId)
assert.ok(conversationLength > 0, "API conversation history should be available after restart")
const restoredCompletion = await api.hasTaskApiConversationHistorySequence(taskId, {
userText: "RESTART_PERSISTENCE_SMOKE",
assistantToolName: "attempt_completion",
assistantToolInputText: MARKER,
})
assert.strictEqual(
restoredCompletion,
true,
"Fresh-host history should restore the marked user turn followed by its assistant completion",
)

await writePhaseResult(getResultsDir(), {
version: PHASE_RESULT_VERSION,
phase: "verify",
status: "passed",
values: { taskId, conversationLength: String(conversationLength) },
values: { taskId },
})
await quitGracefully()
} catch (error) {
Expand Down
16 changes: 16 additions & 0 deletions packages/types/src/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@ import type { WebviewThemeFixture } from "./vscode-extension-host.js"

export type RooCodeAPIEvents = RooCodeEvents

export interface TaskApiConversationHistorySequence {
userText: string
assistantToolName: string
assistantToolInputText: string
}

export interface RooCodeAPI extends EventEmitter<RooCodeAPIEvents> {
/**
* Starts a new task with an optional initial message and images.
Expand Down Expand Up @@ -52,6 +58,16 @@ export interface RooCodeAPI extends EventEmitter<RooCodeAPIEvents> {
* @returns The number of persisted API conversation history entries, or 0 if unavailable.
*/
getTaskApiConversationHistoryLength(taskId: string): Promise<number>
/**
* Checks for an ordered user turn and assistant tool call in persisted API history.
* @param taskId The ID of the task.
* @param sequence The expected user text and assistant tool-call markers.
* @returns True when the expected turns exist in order, or false if unavailable.
*/
hasTaskApiConversationHistorySequence(
taskId: string,
sequence: TaskApiConversationHistorySequence,
): Promise<boolean>
/**
* Returns the current task stack.
* @returns An array of task IDs.
Expand Down
1 change: 1 addition & 0 deletions packages/types/src/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export enum RooCodeEventName {

// Task Lifecycle
TaskStarted = "taskStarted",
/** Emitted after the accepted completion turn is persisted and visible to a fresh extension host. */
TaskCompleted = "taskCompleted",
TaskAborted = "taskAborted",
TaskFocused = "taskFocused",
Expand Down
1 change: 1 addition & 0 deletions src/__tests__/history-resume-delegation.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1379,6 +1379,7 @@ describe("History resume delegation - parent metadata transitions", () => {
consecutiveMistakeCount: 0,
emitFinalTokenUsageUpdate: vi.fn(),
flushTelemetryInstallment: vi.fn(),
waitForCurrentAssistantMessagePersistence: vi.fn().mockResolvedValue(undefined),
} as unknown as import("../core/task/Task").Task

const block = {
Expand Down
2 changes: 2 additions & 0 deletions src/__tests__/nested-delegation-resume.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ describe("Nested delegation resume (A → B → C)", () => {
consecutiveMistakeCount: 0,
emitFinalTokenUsageUpdate: vi.fn(),
flushTelemetryInstallment: vi.fn(),
waitForCurrentAssistantMessagePersistence: vi.fn().mockResolvedValue(undefined),
} as unknown as Task

const blockC = {
Expand Down Expand Up @@ -252,6 +253,7 @@ describe("Nested delegation resume (A → B → C)", () => {
consecutiveMistakeCount: 0,
emitFinalTokenUsageUpdate: vi.fn(),
flushTelemetryInstallment: vi.fn(),
waitForCurrentAssistantMessagePersistence: vi.fn().mockResolvedValue(undefined),
} as unknown as Task

const blockB = {
Expand Down
45 changes: 42 additions & 3 deletions src/core/task/Task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -402,9 +402,12 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
* appear BEFORE the assistant message with tool_uses, causing API errors.
*
* Reset to `false` at the start of each API request.
* Set to `true` after the assistant message is saved in `recursivelyMakeClineRequests`.
* Set to `true` only after the assistant message is durably saved.
*/
assistantMessageSavedToHistory = false
private assistantMessagePersistencePromise!: Promise<boolean>
private resolveAssistantMessagePersistence!: (saved: boolean) => void
private completionPersistenceReadyPromise?: Promise<void>

/**
* Fire-and-forget wrapper around `presentAssistantMessage` that swallows the
Expand Down Expand Up @@ -511,6 +514,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
diffFuzzyThreshold,
}: TaskOptions) {
super()
this.resetAssistantMessagePersistence()

if (startTask && !task && !images && !historyItem) {
throw new Error("Either historyItem or task/images must be provided")
Expand Down Expand Up @@ -979,7 +983,8 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
return readApiMessages({ taskId: this.taskId, globalStoragePath: this.globalStoragePath })
}

private async addToApiConversationHistory(message: Anthropic.MessageParam, reasoning?: string) {
/** Appends an API turn and records whether an assistant turn reached persistent storage. */
private async addToApiConversationHistory(message: Anthropic.MessageParam, reasoning?: string): Promise<void> {
const resolvesPendingAction =
this.pendingAction &&
message.role === "user" &&
Expand Down Expand Up @@ -1011,6 +1016,40 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
)
}
}
if (message.role === "assistant") {
this.assistantMessageSavedToHistory = saved
this.resolveAssistantMessagePersistence(saved)
}
}

/** Creates the persistence boundary for the next streamed assistant turn. */
private resetAssistantMessagePersistence(): void {
this.assistantMessagePersistencePromise = new Promise<boolean>((resolve) => {
this.resolveAssistantMessagePersistence = resolve
})
this.completionPersistenceReadyPromise = undefined
}

/**
* Waits until the current assistant turn is visible to a fresh extension host.
* A public completion event must not be emitted before this boundary succeeds.
*/
public waitForCurrentAssistantMessagePersistence(): Promise<void> {
if (!this.completionPersistenceReadyPromise) {
const currentPersistence = this.assistantMessagePersistencePromise
this.completionPersistenceReadyPromise = (async () => {
const saved = await currentPersistence
if (saved) return

const retrySucceeded = await this.retrySaveApiConversationHistory()
if (!retrySucceeded) {
throw new Error("Failed to persist API conversation history before task completion")
}
this.assistantMessageSavedToHistory = true
})()
}

return this.completionPersistenceReadyPromise
}

// NOTE: We intentionally do NOT mutate stored messages to merge consecutive user turns.
Expand Down Expand Up @@ -2991,6 +3030,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
this.didRejectTool = false
this.didAlreadyUseTool = false
this.assistantMessageSavedToHistory = false
this.resetAssistantMessagePersistence()
// Reset tool failure flag for each new assistant turn - this ensures that tool failures
// only prevent attempt_completion within the same assistant message, not across turns
// (e.g., if a tool fails, then user sends a message saying "just complete anyway")
Expand Down Expand Up @@ -3800,7 +3840,6 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
{ role: "assistant", content: assistantContent },
reasoningMessage || undefined,
)
this.assistantMessageSavedToHistory = true

this.messageCounts.assistant++
}
Expand Down
Loading
Loading