From 111cf40ae041386fb3d0517294360a8e3a5fc78b Mon Sep 17 00:00:00 2001 From: Eason Liang Date: Fri, 4 Sep 2026 15:14:59 +0800 Subject: [PATCH] feat(task): persist task thinking effort on history items (DTE series 2/5) - historyItemSchema: optional thinkingEffort/thinkingEffortSource - taskMetadata: carries both keys (always present, even while undefined) so clearing the override propagates through the TaskHistoryStore merge - Task: constructor restore via setRuntimeThinkingEffort; saveClineMessages writes the active effort; abortTask snapshots the effort before dispose() clears it - dispose: task-end override reset + DTE JSDoc (boundary group split out of the DTE 2b PR per plan 2.6) - spec: taskMetadata key-presence contract + history round-trip / abort snapshot cases Part of #35 (DTE-v2 ship plan, unit 4/5). --- packages/types/src/history.ts | 7 ++ .../__tests__/taskMetadata.spec.ts | 81 +++++++++++++ src/core/task-persistence/taskMetadata.ts | 14 ++- src/core/task/Task.ts | 38 +++++- .../Task.runtime-thinking-effort.test.ts | 113 +++++++++++++++++- 5 files changed, 248 insertions(+), 5 deletions(-) create mode 100644 src/core/task-persistence/__tests__/taskMetadata.spec.ts diff --git a/packages/types/src/history.ts b/packages/types/src/history.ts index 5d9671842e..918ee04221 100644 --- a/packages/types/src/history.ts +++ b/packages/types/src/history.ts @@ -1,5 +1,6 @@ import { z } from "zod" +import { reasoningEffortExtendedSchema } from "./model.js" import { todoItemSchema } from "./todo.js" /** @@ -48,6 +49,12 @@ export const historyItemSchema = z.object({ awaitingChildId: z.string().optional(), // Child currently awaited (set when delegated) completedByChildId: z.string().optional(), // Child that completed and resumed this parent completionResultSummary: z.string().optional(), // Summary from completed child + // DTE series 2/5: task-local thinking effort override persisted with the history + // item so a task reopened from history keeps the effort it had (user-set or + // model/parent-chosen) instead of falling back to the settings value. + thinkingEffort: reasoningEffortExtendedSchema.optional(), + // Provenance of the persisted effort (e.g. "you", "model", "parent"). + thinkingEffortSource: z.string().optional(), pendingAction: pendingTaskActionSchema.optional(), }) diff --git a/src/core/task-persistence/__tests__/taskMetadata.spec.ts b/src/core/task-persistence/__tests__/taskMetadata.spec.ts new file mode 100644 index 0000000000..2fccc03521 --- /dev/null +++ b/src/core/task-persistence/__tests__/taskMetadata.spec.ts @@ -0,0 +1,81 @@ +// cd src && npx vitest run core/task-persistence/__tests__/taskMetadata.spec.ts +// +// DTE series 2/5: taskMetadata() persists the active task-local thinking effort +// (and its provenance) on the history item so that reopening the task from +// history restores it. +// +// The keys are always present on the returned history item — even while +// undefined — so that clearing the override propagates through the +// TaskHistoryStore merge (an absent key would leave the stale disk value in +// place; see buildDelta/mergeWithDisk, which only propagate keys present in +// the incoming item). These tests drive the real taskMetadata() with both +// truthy and falsy effort values to pin that contract. +import { describe, it, expect, vi, beforeEach } from "vitest" +import * as os from "os" +import * as path from "path" +import * as fs from "fs/promises" + +import type { ClineMessage, ReasoningEffortExtended } from "@roo-code/types" + +vi.mock("get-folder-size", () => ({ + __esModule: true, + default: { loose: vi.fn().mockResolvedValue(0) }, +})) +vi.mock("../../../utils/storage", () => ({ + getTaskDirectoryPath: vi + .fn() + .mockImplementation((globalStoragePath, taskId) => Promise.resolve(`${globalStoragePath}/tasks/${taskId}`)), +})) + +// Import after mocks +import { taskMetadata } from "../taskMetadata" + +let tmpBaseDir: string + +beforeEach(async () => { + // Unique writable temp dir as the global storage path (mirrors taskMessages.spec.ts). + tmpBaseDir = await fs.mkdtemp(path.join(os.tmpdir(), "roo-taskmetadata-")) +}) + +function taskSayMessage(text: string): ClineMessage { + return { + ts: 1_700_000_000_000, + type: "say", + say: "task", + text, + } +} + +async function runMetadata(overrides: { thinkingEffort?: ReasoningEffortExtended; thinkingEffortSource?: string }) { + return taskMetadata({ + taskId: "task-meta-1", + taskNumber: 7, + messages: [taskSayMessage("Do the thing")], + globalStoragePath: tmpBaseDir, + workspace: "workspace", + ...overrides, + }) +} + +describe("taskMetadata thinkingEffort persistence", () => { + it("clears the effort fields with explicit keys when not provided", async () => { + const { historyItem } = await runMetadata({}) + + expect(historyItem.thinkingEffort).toBeUndefined() + expect(historyItem.thinkingEffortSource).toBeUndefined() + // The keys must still be PRESENT (with undefined) so the history-store + // merge propagates the clear and drops any stale disk value. + expect("thinkingEffort" in historyItem).toBe(true) + expect("thinkingEffortSource" in historyItem).toBe(true) + // The rest of the history item is still written. + expect(historyItem.id).toBe("task-meta-1") + expect(historyItem.task).toBe("Do the thing") + }) + + it("persists the effort and its provenance on the history item when provided", async () => { + const { historyItem } = await runMetadata({ thinkingEffort: "low", thinkingEffortSource: "you" }) + + expect(historyItem.thinkingEffort).toBe("low") + expect(historyItem.thinkingEffortSource).toBe("you") + }) +}) diff --git a/src/core/task-persistence/taskMetadata.ts b/src/core/task-persistence/taskMetadata.ts index ec2e6cceeb..897289d965 100644 --- a/src/core/task-persistence/taskMetadata.ts +++ b/src/core/task-persistence/taskMetadata.ts @@ -1,7 +1,7 @@ import NodeCache from "node-cache" import getFolderSize from "get-folder-size" -import type { ClineMessage, HistoryItem } from "@roo-code/types" +import type { ClineMessage, HistoryItem, ReasoningEffortExtended } from "@roo-code/types" import { combineApiRequests } from "../../shared/combineApiRequests" import { combineCommandSequences } from "../../shared/combineCommandSequences" @@ -25,6 +25,10 @@ export type TaskMetadataOptions = { apiConfigName?: string /** Initial status for the task (e.g., "active" for child tasks) */ initialStatus?: "active" | "delegated" | "completed" | "interrupted" + /** DTE series 2/5: active task-local thinking effort override to persist on the history item. */ + thinkingEffort?: ReasoningEffortExtended + /** DTE series 2/5: provenance of the persisted effort (e.g. "you", "model", "parent"). */ + thinkingEffortSource?: string } export async function taskMetadata({ @@ -38,6 +42,8 @@ export async function taskMetadata({ mode, apiConfigName, initialStatus, + thinkingEffort, + thinkingEffortSource, }: TaskMetadataOptions) { const taskDir = await getTaskDirectoryPath(globalStoragePath, id) @@ -112,6 +118,12 @@ export async function taskMetadata({ mode, ...(typeof apiConfigName === "string" && apiConfigName.length > 0 ? { apiConfigName } : {}), ...(initialStatus && { status: initialStatus }), + // DTE series 2/5: persist the active task-local effort (and its provenance) so + // reopening this task from history restores it. The keys are always present + // (even while undefined) so that clearing the override propagates through the + // history-store merge — an absent key would leave the stale disk value in place. + thinkingEffort, + thinkingEffortSource, } return { historyItem, tokenUsage } diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index d5e4a08b79..fbd5427e25 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -588,6 +588,12 @@ export class Task extends EventEmitter implements TaskLike { if (historyItem) { this._taskMode = historyItem.mode || defaultModeSlug this._taskApiConfigName = historyItem.apiConfigName + // DTE series 2/5: restore the task-local thinking effort persisted with the + // history item so a reopened task keeps the effort it had instead of + // silently falling back to the settings value. + if (historyItem.thinkingEffort) { + this.setRuntimeThinkingEffort(historyItem.thinkingEffort, historyItem.thinkingEffortSource) + } this.taskModeReady = Promise.resolve() this.taskApiConfigReady = Promise.resolve() TelemetryService.instance.captureTaskRestarted(this.taskId) @@ -1220,7 +1226,12 @@ export class Task extends EventEmitter implements TaskLike { } } - private async saveClineMessages(): Promise { + private async saveClineMessages( + // DTE series 2/5: abortTask() snapshots the effort state before dispose() clears + // it and passes it here so the final history save still records it. Other + // callers pass nothing and the live state is read. + effortSnapshot?: { effort?: ReasoningEffortExtended; source?: string }, + ): Promise { try { await saveTaskMessages({ messages: structuredClone(this.clineMessages), @@ -1232,6 +1243,10 @@ export class Task extends EventEmitter implements TaskLike { await this.taskApiConfigReady } + // DTE series 2/5: the abort path passes a pre-dispose snapshot because + // dispose() has already cleared the live state by the time the final save runs. + const runtimeEffort = effortSnapshot ?? this.getRuntimeThinkingEffort() + const { historyItem, tokenUsage } = await taskMetadata({ taskId: this.taskId, rootTaskId: this.rootTaskId, @@ -1243,6 +1258,10 @@ export class Task extends EventEmitter implements TaskLike { mode: this._taskMode || defaultModeSlug, // Use the task's own mode, not the current provider mode. apiConfigName: this._taskApiConfigName, // Use the task's own provider profile, not the current provider profile. initialStatus: this.initialStatus, + // DTE series 2/5: persist the active task-local effort override so it + // survives reopening this task from history (undefined while inactive). + thinkingEffort: runtimeEffort.effort, + thinkingEffortSource: runtimeEffort.source, }) // Emit token/tool usage updates using debounced function @@ -2577,6 +2596,12 @@ export class Task extends EventEmitter implements TaskLike { this.emit(RooCodeEventName.TaskAborted) + // DTE series 2/5: snapshot the transient effort state before dispose() clears + // it, so the final history save below still records the effort the task was + // using (otherwise an aborted task's history item loses its effort and the + // history-restore path cannot recover it). + const effortAtAbort = this.getRuntimeThinkingEffort() + try { this.dispose() // Call the centralized dispose method } catch (error) { @@ -2593,7 +2618,7 @@ export class Task extends EventEmitter implements TaskLike { return } try { - await this.saveClineMessages() + await this.saveClineMessages(effortAtAbort) } catch (error) { console.error(`Error saving messages during abort for task ${this.taskId}.${this.instanceId}:`, error) } @@ -2602,10 +2627,19 @@ export class Task extends EventEmitter implements TaskLike { /** * Centralized task teardown: releases task resources and resets transient * task-local state. + * + * DTE series 2/5: also clears the task-local thinking effort override (the + * `setRuntimeThinkingEffort` state) — the override never outlives the task. */ public dispose(): void { console.log(`[Task#dispose] disposing task ${this.taskId}.${this.instanceId}`) + // DTE series 2/5: the task-local effort override is transient — clear it on + // task end so a disposed task never carries it forward. + this.runtimeThinkingEffort = undefined + this.runtimeThinkingEffortSource = undefined + this.preOverrideReasoningEffort = undefined + // Stop the idle telemetry check and report any unflushed activity as a // shutdown installment, so a task torn down mid-work (panel closed, task // switched, extension deactivated) isn't invisible to telemetry. diff --git a/src/core/task/__tests__/Task.runtime-thinking-effort.test.ts b/src/core/task/__tests__/Task.runtime-thinking-effort.test.ts index 0800fadbd9..3d1fd864bd 100644 --- a/src/core/task/__tests__/Task.runtime-thinking-effort.test.ts +++ b/src/core/task/__tests__/Task.runtime-thinking-effort.test.ts @@ -2,14 +2,15 @@ // // DTE series 2/5 — task-local thinking effort state on Task: // setRuntimeThinkingEffort / getRuntimeThinkingEffort, the in-memory -// apiConfiguration merge + restore. +// apiConfiguration merge + restore, and the task-end reset in dispose(). -import { ProviderSettings, type ReasoningEffortExtended } from "@roo-code/types" +import { ProviderSettings, type HistoryItem, type ReasoningEffortExtended } from "@roo-code/types" import { providerIdentifiers } from "@roo-code/types/provider-identifiers" import { Task } from "../Task" import { ClineProvider } from "../../webview/ClineProvider" import { buildApiHandler } from "../../../api" +import { taskMetadata } from "../../task-persistence" // Mock dependencies (same lightweight set as Task.throttle.test.ts) vi.mock("../../webview/ClineProvider") @@ -76,6 +77,7 @@ type RuntimeThinkingEffortAccess = { runtimeThinkingEffortSource?: string preOverrideReasoningEffort?: ProviderSettings["reasoningEffort"] getRuntimeThinkingEffortMetadata: () => { reasoningEffort?: ReasoningEffortExtended } + saveClineMessages: () => Promise } function getPrivateAccess(task: Task): RuntimeThinkingEffortAccess { @@ -298,4 +300,111 @@ describe("Task runtime thinking effort (DTE series 2/5)", () => { expect(getPrivateAccess(task).getRuntimeThinkingEffortMetadata()).not.toHaveProperty("reasoningEffort") }) }) + + describe("dispose", () => { + it("clears the task-local override at task end", () => { + task.setRuntimeThinkingEffort("xhigh", "source") + task.dispose() + + expect(task.getRuntimeThinkingEffort()).toEqual({ effort: undefined, source: undefined }) + const access = getPrivateAccess(task) + expect(access.runtimeThinkingEffort).toBeUndefined() + expect(access.runtimeThinkingEffortSource).toBeUndefined() + expect(access.preOverrideReasoningEffort).toBeUndefined() + }) + }) + + describe("history persistence round-trip", () => { + const baseHistoryItem: HistoryItem = { + id: "hist-task-id", + number: 2, + task: "Task from history", + ts: Date.now(), + totalCost: 0.01, + tokensIn: 10, + tokensOut: 5, + } + + function makeHistoryTask(historyItem: Partial): Task { + return new Task({ + provider: mockProvider as unknown as ClineProvider, + apiConfiguration: mockApiConfiguration, + startTask: false, + historyItem: { ...baseHistoryItem, ...historyItem }, + }) + } + + it("restores the persisted task-local effort when constructed from a history item", () => { + const histTask = makeHistoryTask({ thinkingEffort: "xhigh", thinkingEffortSource: "you" }) + + expect(histTask.getRuntimeThinkingEffort()).toEqual({ effort: "xhigh", source: "you" }) + // The in-memory copy carries the restored effort, so the rebuilt handler uses it. + expect(histTask.apiConfiguration).toEqual(expect.objectContaining({ reasoningEffort: "xhigh" })) + const lastCall = vi.mocked(buildApiHandler).mock.calls.at(-1) + expect(lastCall?.[0]).toEqual(expect.objectContaining({ reasoningEffort: "xhigh" })) + histTask.dispose() + }) + + it("leaves the override inactive for history items without a persisted effort", () => { + const histTask = makeHistoryTask({}) + + expect(histTask.getRuntimeThinkingEffort()).toEqual({ effort: undefined, source: undefined }) + expect(histTask.apiConfiguration.reasoningEffort).toBe(SETTINGS_EFFORT) + histTask.dispose() + }) + + it("never calls the restore path when the history item has no persisted effort", () => { + // Kills the if-test → true mutant on the constructor restore guard: + // a restored (undefined, undefined) would be silently dropped by + // the setter's already-inactive early return, so only a call-count + // assertion on the guard is observable. + const restoreSpy = vi.spyOn(Task.prototype, "setRuntimeThinkingEffort") + + makeHistoryTask({}) + + expect(restoreSpy).not.toHaveBeenCalled() + restoreSpy.mockRestore() + }) + + it("carries the active task-local effort onto the taskMetadata payload in saveClineMessages", async () => { + task.setRuntimeThinkingEffort("max", "you") + + await getPrivateAccess(task).saveClineMessages() + + expect(vi.mocked(taskMetadata)).toHaveBeenCalledWith( + expect.objectContaining({ thinkingEffort: "max", thinkingEffortSource: "you" }), + ) + }) + + it("omits the effort values from the taskMetadata payload while inactive", async () => { + await getPrivateAccess(task).saveClineMessages() + + expect(vi.mocked(taskMetadata)).toHaveBeenCalledWith( + expect.objectContaining({ thinkingEffort: undefined, thinkingEffortSource: undefined }), + ) + }) + }) + + describe("abortTask final save (DTE series 2/5)", () => { + it("records the active task-local effort on the final history save despite dispose() clearing it", async () => { + task.setRuntimeThinkingEffort("high", "you") + + await task.abortTask() + + // dispose() has already cleared the live state... + expect(task.getRuntimeThinkingEffort()).toEqual({ effort: undefined, source: undefined }) + // ...but the final save still recorded the pre-dispose snapshot. + expect(vi.mocked(taskMetadata)).toHaveBeenCalledWith( + expect.objectContaining({ thinkingEffort: "high", thinkingEffortSource: "you" }), + ) + }) + + it("saves undefined effort fields on the final history save while inactive", async () => { + await task.abortTask() + + expect(vi.mocked(taskMetadata)).toHaveBeenCalledWith( + expect.objectContaining({ thinkingEffort: undefined, thinkingEffortSource: undefined }), + ) + }) + }) })