Merge branch 'main' into fix/rename-roo-to-zoo-user-strings

Author: rrewll

apps/vscode-e2e/src/fixtures/subtasks.ts

@@ -32,6 +32,14 @@ export const SUBTASK_API_HANG_RESUME_MESSAGE = "Continue after provider hang."
 export const SUBTASK_API_HANG_CHILD_RESULT = "Hung child completed"
 export const SUBTASK_API_HANG_PARENT_RESULT = "API hang parent resumed"
 
+// Abandon-subtask scenario (#559) — separate markers to avoid sequenceIndex collisions with the
+// interrupted-child-resumes tests above, which exhaust the sequence count for INTERRUPT markers.
+const SUBTASK_ABANDON_PARENT_MARKER = "SUBTASK_PARENT_ABANDON_SEVER"
+const SUBTASK_ABANDON_CHILD_MARKER = "SUBTASK_CHILD_ABANDON_SEVER"
+const SUBTASK_ABANDON_CHILD_PROMPT = `${SUBTASK_ABANDON_CHILD_MARKER}: Ask the user exactly this follow-up question: What is the square root of 81? After the user answers, complete with only the answer.`
+export const SUBTASK_ABANDON_PARENT_PROMPT = `${SUBTASK_ABANDON_PARENT_MARKER}: Use the new_task tool exactly once. Create an ask-mode subtask with this exact message: "${SUBTASK_ABANDON_CHILD_PROMPT}" Do not answer directly.`
+export const SUBTASK_ABANDON_CHILD_FOLLOWUP_ANSWER = "9"
+
 const SUBTASK_XPROFILE_SAME_CHILD_PROMPT = `${SUBTASK_XPROFILE_SAME_CHILD_MARKER}: Complete immediately with the exact result "Same-profile child completed".`
 const SUBTASK_XPROFILE_DIFFERENT_CHILD_PROMPT = `${SUBTASK_XPROFILE_DIFFERENT_CHILD_MARKER}: Complete immediately with the exact result "Different-profile child completed".`
 export const SUBTASK_XPROFILE_PARENT_PROMPT = `${SUBTASK_XPROFILE_PARENT_MARKER}: First use new_task to create a code-mode subtask with this exact message: "${SUBTASK_XPROFILE_SAME_CHILD_PROMPT}" After it returns, create an ask-mode subtask with the next instructions you receive.`
@@ -437,4 +445,67 @@ export function addSubtaskFixtures(mock: InstanceType<typeof LLMock>) {
 			],
 		},
 	})
+
+	// Abandon-subtask scenario (#559)
+	mock.addFixture({
+		match: {
+			userMessage: new RegExp(SUBTASK_ABANDON_PARENT_MARKER),
+			sequenceIndex: 0,
+		},
+		response: {
+			toolCalls: [
+				{
+					name: "new_task",
+					arguments: JSON.stringify({
+						mode: "ask",
+						message: SUBTASK_ABANDON_CHILD_PROMPT,
+					}),
+					id: "call_abandon_parent_new_task_001",
+				},
+			],
+		},
+	})
+
+	mock.addFixture({
+		match: {
+			predicate: (req: ChatCompletionRequest) =>
+				requestContains(req, [SUBTASK_ABANDON_CHILD_MARKER]) &&
+				!requestContains(req, [SUBTASK_ABANDON_PARENT_MARKER]) &&
+				!requestContains(req, ["call_abandon_child_followup_001"]) &&
+				!requestContains(req, [`<user_message>\\n${SUBTASK_ABANDON_CHILD_FOLLOWUP_ANSWER}\\n</user_message>`]),
+		},
+		response: {
+			toolCalls: [
+				{
+					name: "ask_followup_question",
+					arguments: JSON.stringify({
+						question: "What is the square root of 81?",
+						follow_up: [{ text: SUBTASK_ABANDON_CHILD_FOLLOWUP_ANSWER }],
+					}),
+					id: "call_abandon_child_followup_001",
+				},
+			],
+		},
+	})
+
+	mock.addFixture({
+		match: {
+			predicate: (req: ChatCompletionRequest) =>
+				toolResultContains(req, "call_abandon_child_followup_001", [SUBTASK_ABANDON_CHILD_FOLLOWUP_ANSWER]) ||
+				requestContains(req, ["call_abandon_child_followup_001", SUBTASK_ABANDON_CHILD_FOLLOWUP_ANSWER]) ||
+				requestContains(req, [
+					SUBTASK_ABANDON_CHILD_MARKER,
+					`<user_message>\\n${SUBTASK_ABANDON_CHILD_FOLLOWUP_ANSWER}\\n</user_message>`,
+				]),
+		},
+		response: {
+			toolCalls: [
+				{
+					name: "attempt_completion",
+					arguments: JSON.stringify({ result: SUBTASK_ABANDON_CHILD_FOLLOWUP_ANSWER }),
+					id: "call_abandon_child_completion_002",
+				},
+			],
+		},
+	})
 }

apps/vscode-e2e/src/suite/subtasks.test.ts

@@ -5,8 +5,10 @@ import { RooCodeEventName, type ClineMessage } from "@roo-code/types"
 import { setDefaultSuiteTimeout } from "./test-utils"
 import { sleep, waitFor, waitUntilCompleted } from "./utils"
 import {
-	SUBTASK_API_HANG_CHILD_RESULT,
+	SUBTASK_ABANDON_CHILD_FOLLOWUP_ANSWER,
+	SUBTASK_ABANDON_PARENT_PROMPT,
 	SUBTASK_API_HANG_CHILD_MARKER,
+	SUBTASK_API_HANG_CHILD_RESULT,
 	SUBTASK_API_HANG_PARENT_MARKER,
 	SUBTASK_API_HANG_PARENT_PROMPT,
 	SUBTASK_API_HANG_PARENT_RESULT,
@@ -271,7 +273,10 @@ suite("Roo Code Subtasks", function () {
 
 			const parent = await api.getTaskHistoryItem(parentTaskId)
 			assert.ok(parent, "Parent history item should exist")
-			assert.strictEqual(parent.status, "active", "Parent status should be 'active' after child completes")
+			assert.ok(
+				parent.status === "active" || parent.status === "completed",
+				`Parent status should be 'active' or 'completed' after child completes (got '${parent.status}')`,
+			)
 			assert.strictEqual(parent.awaitingChildId, undefined, "Parent awaitingChildId should be cleared")
 			assert.strictEqual(parent.delegatedToId, undefined, "Parent delegatedToId should be cleared")
 			assert.strictEqual(parent.completedByChildId, childTaskId, "Parent completedByChildId should be the child")
@@ -769,4 +774,165 @@ suite("Roo Code Subtasks", function () {
 			await waitFor(() => api.getCurrentTaskStack().length === 0).catch(() => {})
 		}
 	})
+
+	// Issue #559: explicit "Abandon subtask" action. Unlike cancellation alone (which leaves
+	// the child "interrupted" and the parent "delegated" so the child can still resume and
+	// report back), abandoning severs the link outright: the parent goes back to "active" and
+	// the child's parentTaskId/rootTaskId are cleared so a later resume can never reattach it.
+	test("abandoning an interrupted subtask severs the parent-child link", async () => {
+		const api = globalThis.api
+		const asks: Record<string, ClineMessage[]> = {}
+		const says: Record<string, ClineMessage[]> = {}
+
+		const messageHandler = ({ taskId, message }: { taskId: string; message: ClineMessage }) => {
+			if (message.type === "ask") {
+				asks[taskId] = asks[taskId] || []
+				asks[taskId].push(message)
+			}
+			if (message.type === "say" && message.partial === false) {
+				says[taskId] = says[taskId] || []
+				says[taskId].push(message)
+			}
+		}
+
+		api.on(RooCodeEventName.Message, messageHandler)
+
+		try {
+			const parentTaskId = await api.startNewTask({
+				configuration: {
+					mode: "ask",
+					alwaysAllowModeSwitch: true,
+					alwaysAllowSubtasks: true,
+					autoApprovalEnabled: true,
+					enableCheckpoints: false,
+				},
+				text: SUBTASK_ABANDON_PARENT_PROMPT,
+			})
+
+			let childTaskId: string | undefined
+			await waitFor(() => {
+				const stack = api.getCurrentTaskStack()
+				const current = stack[stack.length - 1]
+				if (current && current !== parentTaskId) {
+					childTaskId = current
+					return true
+				}
+				return false
+			})
+
+			await waitFor(() => asks[childTaskId!]?.some(({ ask }) => ask === "followup") ?? false)
+			await waitFor(async () => (await api.getTaskApiConversationHistoryLength(childTaskId!)) > 0)
+
+			// Cancel the child — marked "interrupted", parent stays "delegated".
+			await api.cancelCurrentTask()
+
+			await waitFor(() => api.getCurrentTaskStack().at(-1) === childTaskId)
+			await waitFor(
+				() => asks[childTaskId!]?.some(({ type, ask }) => type === "ask" && ask === "resume_task") ?? false,
+			)
+
+			const interruptedChild = await api.getTaskHistoryItem(childTaskId!)
+			assert.strictEqual(interruptedChild?.status, "interrupted", "Child should be marked interrupted")
+
+			const delegatedParent = await api.getTaskHistoryItem(parentTaskId)
+			assert.strictEqual(delegatedParent?.status, "delegated", "Parent should still be delegated before abandon")
+			assert.strictEqual(
+				delegatedParent?.awaitingChildId,
+				childTaskId,
+				"Parent should await the interrupted child",
+			)
+
+			// The interrupted child is the live/open task at this point (cancelTask rehydrates
+			// it onto the stack). Abandon must close that live instance before severing the
+			// persisted link — otherwise a later save on the still-open child would rebuild
+			// parentTaskId/rootTaskId from its live (readonly) fields and silently reattach it.
+			const abandoned = await api.abandonSubtask(childTaskId!)
+			assert.strictEqual(abandoned, true, "abandonSubtask should report the link was severed")
+
+			await waitFor(() => api.getCurrentTaskStack().at(-1) !== childTaskId)
+
+			const parentAfterAbandon = await api.getTaskHistoryItem(parentTaskId)
+			assert.strictEqual(parentAfterAbandon?.status, "active", "Parent should return to active after abandon")
+			assert.strictEqual(
+				parentAfterAbandon?.awaitingChildId,
+				undefined,
+				"Parent awaitingChildId should be cleared",
+			)
+			assert.strictEqual(parentAfterAbandon?.delegatedToId, undefined, "Parent delegatedToId should be cleared")
+
+			const childAfterAbandon = await api.getTaskHistoryItem(childTaskId!)
+			// The child's own status is left untouched (VALID_TRANSITIONS only allows interrupted → completed);
+			// only its parent/root links are cleared so it can never reattach to the parent again.
+			assert.strictEqual(childAfterAbandon?.status, "interrupted", "Child status stays interrupted")
+			assert.strictEqual(childAfterAbandon?.parentTaskId, undefined, "Child parentTaskId should be cleared")
+			assert.strictEqual(childAfterAbandon?.rootTaskId, undefined, "Child rootTaskId should be cleared")
+
+			// A second abandon call is a no-op since the parent is no longer delegated to this child.
+			const secondAbandon = await api.abandonSubtask(childTaskId!)
+			assert.strictEqual(secondAbandon, false, "Second abandonSubtask call should be a no-op")
+
+			// Resume and complete the abandoned child — it must NOT reopen or reattach to the
+			// parent. Before the abandon fix, a subsequent save on the still-live child could
+			// silently rewrite its persisted parentTaskId back to the parent; this proves the
+			// link stays severed all the way through a real resume/save/complete cycle.
+			// api.resumeTask() re-instantiates the child from history, which re-raises its own
+			// "resume_task" ask; answering it with the follow-up answer (same pattern the sibling
+			// "cancelled child completes and reopens parent" test above uses) both resumes the
+			// task and supplies the answer the re-asked follow-up question is waiting for.
+			// asks[childTaskId] already holds the earlier resume_task ask from the pre-abandon
+			// cancellation, so the wait below must look for a NEW one, not just any occurrence.
+			const askCountBeforeResume = asks[childTaskId!]?.length ?? 0
+			await api.resumeTask(childTaskId!)
+			await waitFor(() =>
+				(asks[childTaskId!] ?? [])
+					.slice(askCountBeforeResume)
+					.some(({ type, ask }) => type === "ask" && ask === "resume_task"),
+			)
+
+			const completedChildTaskId = await waitUntilCompleted({
+				api,
+				start: async () => {
+					await api.sendMessage(SUBTASK_ABANDON_CHILD_FOLLOWUP_ANSWER)
+					return childTaskId!
+				},
+			})
+
+			assert.strictEqual(
+				completedChildTaskId,
+				childTaskId,
+				"The abandoned child itself should be the task that completes, not the parent",
+			)
+			assert.strictEqual(
+				says[parentTaskId]?.find(({ say }) => say === "completion_result"),
+				undefined,
+				"Parent must never complete/reopen after its abandoned child resumes and completes",
+			)
+
+			const parentAfterChildCompletes = await api.getTaskHistoryItem(parentTaskId)
+			assert.strictEqual(
+				parentAfterChildCompletes?.status,
+				"active",
+				"Parent status must remain untouched by the abandoned child's completion",
+			)
+			assert.strictEqual(
+				parentAfterChildCompletes?.awaitingChildId,
+				undefined,
+				"Parent must not start awaiting the abandoned child again",
+			)
+
+			const childAfterCompletion = await api.getTaskHistoryItem(childTaskId!)
+			assert.strictEqual(
+				childAfterCompletion?.parentTaskId,
+				undefined,
+				"Child parentTaskId must still be cleared after it completes on its own — " +
+					"proves the live-instance save did not resurrect the old link",
+			)
+		} finally {
+			api.off(RooCodeEventName.Message, messageHandler)
+			while (api.getCurrentTaskStack().length > 0) {
+				await api.clearCurrentTask()
+			}
+			await waitFor(() => api.getCurrentTaskStack().length === 0).catch(() => {})
+		}
+	})
 })

packages/types/src/__tests__/kimi-code.test.ts

@@ -0,0 +1,29 @@
+import {
+	SECRET_STATE_KEYS,
+	dynamicProviders,
+	kimiCodeDefaultModelId,
+	providerSettingsSchema,
+	providerSettingsSchemaDiscriminated,
+} from "../index.js"
+
+describe("Kimi Code provider types", () => {
+	it("registers Kimi Code as a dynamic provider with a distinct secret", () => {
+		expect(dynamicProviders).toContain("kimi-code")
+		expect(SECRET_STATE_KEYS).toContain("kimiCodeApiKey")
+		expect(SECRET_STATE_KEYS).toContain("moonshotApiKey")
+	})
+
+	it("parses OAuth and API-key settings independently from Moonshot", () => {
+		expect(
+			providerSettingsSchemaDiscriminated.parse({
+				apiProvider: "kimi-code",
+				kimiCodeAuthMethod: "api-key",
+				kimiCodeApiKey: "kimi-key",
+				apiModelId: kimiCodeDefaultModelId,
+			}),
+		).toMatchObject({ kimiCodeApiKey: "kimi-key" })
+		expect(providerSettingsSchema.parse({ apiProvider: "kimi-code", kimiCodeAuthMethod: "oauth" })).toMatchObject({
+			kimiCodeAuthMethod: "oauth",
+		})
+	})
+})

packages/types/src/__tests__/lite-llm.test.ts

@@ -12,6 +12,7 @@ describe("LiteLLM preserveReasoning model detection", () => {
 	})
 
 	it("matches provider-prefixed routed model names by their final segment", () => {
+		expect(isLiteLLMPreserveReasoningModel("kimi-k3")).toBe(true)
 		expect(isLiteLLMPreserveReasoningModel("deepseek/deepseek-reasoner")).toBe(true)
 		expect(isLiteLLMPreserveReasoningModel("bedrock/moonshot.kimi-k2-thinking")).toBe(true)
 		expect(isLiteLLMPreserveReasoningModel("fireworks_ai/accounts/fireworks/models/kimi-k2p7-code")).toBe(true)

packages/types/src/__tests__/moonshot.test.ts

@@ -0,0 +1,72 @@
+import type { ModelInfo } from "../model.js"
+import { MOONSHOT_DEFAULT_TEMPERATURE, moonshotDefaultModelId, moonshotModels } from "../providers/moonshot.js"
+
+const modelEntries: [string, ModelInfo][] = Object.entries(moonshotModels)
+const modelInfos: ModelInfo[] = Object.values(moonshotModels)
+
+describe("moonshot registry", () => {
+	describe("moonshotModels registry invariants", () => {
+		it("every entry has a positive maxTokens and contextWindow", () => {
+			for (const [id, info] of modelEntries) {
+				expect(info.maxTokens).toBeGreaterThan(0)
+				expect(info.contextWindow).toBeGreaterThan(0)
+				// Sanity: max output must not exceed the context window.
+				expect(info.maxTokens).toBeLessThanOrEqual(info.contextWindow)
+				void id
+			}
+		})
+
+		it("every entry declares supportsImages and supportsPromptCache", () => {
+			for (const info of modelInfos) {
+				expect(typeof info.supportsImages).toBe("boolean")
+				expect(typeof info.supportsPromptCache).toBe("boolean")
+			}
+		})
+
+		it("models with an array supportsReasoningEffort expose a non-empty allow-list", () => {
+			for (const info of modelInfos) {
+				if (Array.isArray(info.supportsReasoningEffort)) {
+					expect(info.supportsReasoningEffort.length).toBeGreaterThan(0)
+				}
+			}
+		})
+
+		it("every entry declares a reasoningEffort that is covered by its allow-list", () => {
+			for (const info of modelInfos) {
+				if (Array.isArray(info.supportsReasoningEffort) && info.reasoningEffort !== undefined) {
+					expect(info.supportsReasoningEffort).toContain(info.reasoningEffort)
+				}
+			}
+		})
+	})
+
+	describe("kimi-k3", () => {
+		it("exposes always-on reasoning with effort allow-list and reasoning preservation", () => {
+			const info = moonshotModels["kimi-k3"]
+			expect(info).toBeDefined()
+			expect(info.maxTokens).toBe(131_072)
+			expect(info.contextWindow).toBe(1_048_576)
+			expect(info.supportsImages).toBe(true)
+			expect(info.supportsPromptCache).toBe(true)
+			expect(info.supportsReasoningEffort).toEqual(["low", "high", "max"])
+			expect(info.reasoningEffort).toBe("max")
+			expect(info.preserveReasoning).toBe(true)
+			expect(info.defaultTemperature).toBe(1.0)
+			expect(info.inputPrice).toBe(3.0)
+			expect(info.outputPrice).toBe(15.0)
+			expect(info.cacheWritesPrice).toBe(0)
+			expect(info.cacheReadsPrice).toBe(0.3)
+		})
+	})
+
+	describe("defaults", () => {
+		it("the default model id is a curated registry entry", () => {
+			expect(moonshotDefaultModelId).toBe("kimi-k2-0905-preview")
+			expect(moonshotModels[moonshotDefaultModelId]).toBeDefined()
+		})
+
+		it("exposes a deterministic default temperature", () => {
+			expect(MOONSHOT_DEFAULT_TEMPERATURE).toBe(0.6)
+		})
+	})
+})