From 66b8558b5012af21307e3b814eb5712978d86e1b Mon Sep 17 00:00:00 2001 From: xin zhuang <65798732+1052326311@users.noreply.github.com> Date: Sat, 3 Oct 2026 00:09:50 +0800 Subject: [PATCH 1/9] fix(agent): reconcile plan progress and refresh resumed session instructions --- apps/web/src/app/editor/[projectId]/page.tsx | 177 +++--- .../src/modules/editor/chat/chat-panel.tsx | 10 +- .../editor/chat/plan/plan-progress.tsx | 35 +- .../editor/chat/plan/plan-state.test.ts | 84 +++ .../modules/editor/chat/plan/plan-state.ts | 43 ++ .../modules/editor/chat/plan/plan-step.tsx | 8 + .../src/modules/editor/hooks/use-chat-sse.ts | 14 +- apps/web/src/modules/editor/hooks/use-chat.ts | 3 +- .../src/modules/editor/hooks/use-plan-sync.ts | 67 +++ docs/plan-progress.md | 52 ++ packages/shared/src/types/ai.ts | 3 +- services/api/src/ai/context/injector.ts | 2 +- services/api/src/ai/modes/plan.ts | 97 +-- services/api/src/ai/plan-parser.ts | 28 +- services/api/src/ai/plan-state.test.ts | 87 +++ services/api/src/ai/plan-state.ts | 50 ++ .../api/src/ai/plan-store.integration.test.ts | 34 ++ services/api/src/ai/plan-store.ts | 179 ++++++ services/api/src/ai/plan-tool-policy.ts | 24 + services/api/src/ai/provider-error.ts | 53 ++ .../providers/copilot-engine.rebind.test.ts | 28 + .../api/src/ai/providers/copilot-engine.ts | 32 +- .../api/src/ai/providers/copilot-tools.ts | 66 ++- services/api/src/ai/session-prompt.test.ts | 107 ++++ services/api/src/ai/session-prompt.ts | 29 + services/api/src/ai/sse-mapper.ts | 68 ++- services/api/src/ai/tool-messages.ts | 30 +- services/api/src/ai/tool-outcome.ts | 32 + services/api/src/ai/tools/index.ts | 3 + services/api/src/ai/tools/plan-tools.ts | 34 +- services/api/src/ai/trace-factory.ts | 2 +- services/api/src/ai/trace-types.ts | 3 + .../src/db/migrations/103_plan_revision.sql | 7 + services/api/src/routes/admin-tools.ts | 6 +- .../api/src/routes/chat/event-processor.ts | 89 +-- .../src/routes/chat/execution-state.test.ts | 388 ++++++++++++ .../api/src/routes/chat/execution-state.ts | 165 ++++++ .../chat/final-response.integration.test.ts | 207 +++++++ .../src/routes/chat/final-response.test.ts | 73 +++ .../api/src/routes/chat/final-response.ts | 27 + services/api/src/routes/chat/fix-error.ts | 2 +- services/api/src/routes/chat/misc-routes.ts | 8 +- .../api/src/routes/chat/post-processing.ts | 44 +- .../src/routes/chat/sdk-tool-ledger.test.ts | 92 +++ services/api/src/routes/chat/send-handler.ts | 97 +-- services/api/src/routes/chat/send-helpers.ts | 18 +- .../api/src/routes/chat/session-manager.ts | 65 ++- .../api/src/routes/chat/session-state.test.ts | 22 + services/api/src/routes/chat/session-state.ts | 9 +- .../stream-recovery.supabase-gate.test.ts | 4 +- .../api/src/routes/chat/stream-recovery.ts | 552 +++++++----------- .../api/src/routes/chat/system-prompts.ts | 12 +- .../api/src/routes/chat/tool-callbacks.ts | 110 +--- .../src/routes/chat/tool-event-bookkeeping.ts | 34 +- services/api/src/routes/chat/types.ts | 7 + services/api/src/routes/plan.ts | 175 +----- .../api/src/templates/definitions/blank.ts | 2 +- .../src/templates/definitions/nextjs-blank.ts | 2 +- services/api/src/thumbnails/capture.ts | 4 +- 59 files changed, 2603 insertions(+), 1102 deletions(-) create mode 100644 apps/web/src/modules/editor/chat/plan/plan-state.test.ts create mode 100644 apps/web/src/modules/editor/chat/plan/plan-state.ts create mode 100644 apps/web/src/modules/editor/hooks/use-plan-sync.ts create mode 100644 docs/plan-progress.md create mode 100644 services/api/src/ai/plan-state.test.ts create mode 100644 services/api/src/ai/plan-state.ts create mode 100644 services/api/src/ai/plan-store.integration.test.ts create mode 100644 services/api/src/ai/plan-store.ts create mode 100644 services/api/src/ai/plan-tool-policy.ts create mode 100644 services/api/src/ai/provider-error.ts create mode 100644 services/api/src/ai/providers/copilot-engine.rebind.test.ts create mode 100644 services/api/src/ai/session-prompt.test.ts create mode 100644 services/api/src/ai/session-prompt.ts create mode 100644 services/api/src/ai/tool-outcome.ts create mode 100644 services/api/src/db/migrations/103_plan_revision.sql create mode 100644 services/api/src/routes/chat/execution-state.test.ts create mode 100644 services/api/src/routes/chat/execution-state.ts create mode 100644 services/api/src/routes/chat/final-response.integration.test.ts create mode 100644 services/api/src/routes/chat/final-response.test.ts create mode 100644 services/api/src/routes/chat/final-response.ts create mode 100644 services/api/src/routes/chat/sdk-tool-ledger.test.ts create mode 100644 services/api/src/routes/chat/session-state.test.ts diff --git a/apps/web/src/app/editor/[projectId]/page.tsx b/apps/web/src/app/editor/[projectId]/page.tsx index 1194af13..467eb8d1 100644 --- a/apps/web/src/app/editor/[projectId]/page.tsx +++ b/apps/web/src/app/editor/[projectId]/page.tsx @@ -1,5 +1,6 @@ "use client"; - +import {usePlanSync} from "@/modules/editor/hooks/use-plan-sync"; +import {acceptPlanSnapshot} from "@/modules/editor/chat/plan/plan-state"; import { useState, useRef, useCallback, useEffect, memo, Suspense } from "react"; import { useParams, useRouter, useSearchParams } from "next/navigation"; import dynamic from "next/dynamic"; @@ -185,12 +186,13 @@ type DeviceMode = "desktop" | "tablet" | "mobile"; interface ToolAction { id: string; + callId?: string; toolName: string; description: string; isExpanded: boolean; isBookmarked?: boolean; filePath?: string; - status?: "running" | "completed" | "failed"; + status?: "running" | "completed" | "failed" | "unknown"; } interface ChatMsg { @@ -430,8 +432,8 @@ async function streamChat( onChunk: (text: string) => void, onDone: () => void, onError: (error: string) => void, - onToolCompleted?: (toolName: string, args: Record) => void, - onToolStarted?: (toolName: string, args: Record) => void, + onToolCompleted?: (toolName: string, args: Record, success?: boolean, callId?: string) => void, + onToolStarted?: (toolName: string, args: Record, callId?: string) => void, signal?: AbortSignal, onThinking?: (text: string) => void, onStatusChange?: (status: string, phase?: string) => void, @@ -576,7 +578,7 @@ async function streamChat( // Handle tool_call events — show "in progress" card immediately if (parsed.type === "tool_call" && onToolStarted) { const d = parsed.data as Record | undefined; - const toolName = (d?.name as string) ?? (d?.toolName as string) ?? ""; + const toolName = (d?.name as string) ?? (d?.toolName as string) ?? parsed.name ?? ""; let toolArgs: Record = {}; const rawArgs = d?.arguments ?? d?.args; if (typeof rawArgs === "string" && rawArgs.trim()) { @@ -590,14 +592,14 @@ async function streamChat( } if (toolName) { pendingToolNames.push(toolName); - onToolStarted(toolName, toolArgs); + onToolStarted(toolName, toolArgs, d?.toolCallId as string | undefined); } } // Handle tool_executing events — tool arguments are fully available before completing if (parsed.type === "tool_executing" && onToolStarted) { const d = parsed.data as Record | undefined; - const toolName = (d?.name as string) ?? (d?.toolName as string) ?? ""; + const toolName = (d?.name as string) ?? (d?.toolName as string) ?? parsed.name ?? ""; let toolArgs: Record = {}; const rawArgs = d?.arguments ?? d?.args; if (typeof rawArgs === "string" && rawArgs.trim()) { @@ -606,25 +608,21 @@ async function streamChat( toolArgs = rawArgs as Record; } if (toolName) { - onToolStarted(toolName, toolArgs); + onToolStarted(toolName, toolArgs, d?.toolCallId as string | undefined); } } // Handle tool completion events — triggers file tree / content refresh - if (parsed.type === "tool.completed" && onToolCompleted) { - const toolName = parsed.name ?? (typeof parsed.data === "object" && parsed.data !== null ? (parsed.data as Record).name as string : ""); - const toolArgs = parsed.args ?? (typeof parsed.data === "object" && parsed.data !== null ? (parsed.data as Record).args as Record : {}); - onToolCompleted(toolName ?? "", toolArgs ?? {}); - } + // Handle tool_result events — tool finished executing, update card to completed if ((parsed.type === "tool_result" || parsed.type === "tool.completed") && onToolCompleted) { const d = parsed.data as Record | undefined; - let toolName = (d?.name as string) ?? (d?.toolName as string) ?? ""; + let toolName = (d?.name as string) ?? (d?.toolName as string) ?? parsed.name ?? ""; let toolArgs: Record = {}; // Prefer the request args (so file-name extraction works) and fall // back to the result payload only if args are missing. - const rawArgs = d?.arguments ?? d?.args ?? d?.result; + const rawArgs = d?.arguments ?? d?.args ?? parsed.args ?? d?.result; if (typeof rawArgs === "string" && rawArgs.trim()) { try { toolArgs = JSON.parse(rawArgs); @@ -647,7 +645,7 @@ async function streamChat( pendingToolNames.shift(); } if (toolName) { - onToolCompleted(toolName, toolArgs); + onToolCompleted(toolName, toolArgs, typeof d?.success === "boolean" ? d.success : undefined, d?.toolCallId as string | undefined); } // Inline artifacts attached to tool_result (resilient // alternative to standalone artifact_ready / mcp_ui_resource @@ -673,7 +671,7 @@ async function streamChat( const filePath = (d?.filePath as string) ?? ""; const action = (d?.action as string) ?? "edit"; if (filePath) { - onToolCompleted(`${action}_file`, { path: filePath }); + onToolCompleted(`${action}_file`, { path: filePath }, true); } } @@ -696,9 +694,10 @@ async function streamChat( if (parsed.type === "plan_step_update" && onPlanStepUpdate) { const d = parsed.data as Record | undefined; + if (d?.plan && onPlan) onPlan(d.plan as Plan); const stepId = d?.stepId as string | undefined; const status = d?.status as string | undefined; - if (stepId && status) { + if (!d?.plan && stepId && status) { onPlanStepUpdate(stepId, status); } } @@ -900,8 +899,8 @@ interface BridgeCallbacks { onDone: () => void; onError: (error: string) => void; onUserInputRequest?: (req: UserInputRequestPayload) => void; - onToolCompleted?: (toolName: string, args: Record) => void; - onToolStarted?: (toolName: string, args: Record) => void; + onToolCompleted?: (toolName: string, args: Record, success?: boolean, callId?: string) => void; + onToolStarted?: (toolName: string, args: Record, callId?: string) => void; onThinking?: (text: string) => void; onStatusChange?: (status: string, phase?: string) => void; onClarification?: (questions: ClarificationQuestion[]) => void; @@ -935,23 +934,19 @@ function processOneSSEPayload( if (parsed.type === "tool_call" && cb.onToolStarted) { const d = parsed.data as Record | undefined; - const toolName = (d?.name as string) ?? (d?.toolName as string) ?? ""; + const toolName = (d?.name as string) ?? (d?.toolName as string) ?? parsed.name ?? ""; const toolArgs = (d?.arguments as Record) ?? {}; if (toolName) { pendingToolNames.push(toolName); - cb.onToolStarted(toolName, toolArgs); + cb.onToolStarted(toolName, toolArgs, d?.toolCallId as string | undefined); } } - if (parsed.type === "tool.completed" && cb.onToolCompleted) { - const toolName = parsed.name ?? (typeof parsed.data === "object" && parsed.data !== null ? (parsed.data as Record).name as string : ""); - const toolArgs = parsed.args ?? (typeof parsed.data === "object" && parsed.data !== null ? (parsed.data as Record).args as Record : {}); - cb.onToolCompleted(toolName ?? "", toolArgs ?? {}); - } + if ((parsed.type === "tool_result" || parsed.type === "tool.completed") && cb.onToolCompleted) { const d = parsed.data as Record | undefined; - let toolName = (d?.name as string) ?? (d?.toolName as string) ?? ""; + let toolName = (d?.name as string) ?? (d?.toolName as string) ?? parsed.name ?? ""; // Prefer request args so the file name is visible on the card. let toolArgs = ((d?.arguments as Record) ?? (d?.args as Record) ?? (d?.result as Record)) ?? {}; if (typeof d?.path === "string" && !(toolArgs as Record).path) { @@ -962,7 +957,7 @@ function processOneSSEPayload( } else if (toolName && pendingToolNames.length > 0 && pendingToolNames[0] === toolName) { pendingToolNames.shift(); } - if (toolName) cb.onToolCompleted(toolName, toolArgs); + if (toolName) cb.onToolCompleted(toolName, toolArgs, typeof d?.success === "boolean" ? d.success : undefined, d?.toolCallId as string | undefined); if (Array.isArray(d?.artifacts) && cb.onArtifactReady) { for (const a of d!.artifacts as Array>) { if (typeof a?.url === "string" && typeof a?.fileName === "string" && typeof a?.mimeType === "string") { @@ -982,7 +977,7 @@ function processOneSSEPayload( const d = parsed.data as Record | undefined; const filePath = (d?.filePath as string) ?? ""; const action = (d?.action as string) ?? "edit"; - if (filePath) cb.onToolCompleted(`${action}_file`, { path: filePath }); + if (filePath) cb.onToolCompleted(`${action}_file`, { path: filePath }, true); } if (parsed.type === "clarification" && cb.onClarification) { @@ -1014,9 +1009,10 @@ function processOneSSEPayload( if (parsed.type === "plan_step_update" && cb.onPlanStepUpdate) { const d = parsed.data as Record | undefined; + if(d?.plan && cb.onPlan) cb.onPlan(d.plan as Plan); const stepId = d?.stepId as string | undefined; const status = d?.status as string | undefined; - if (stepId && status) cb.onPlanStepUpdate(stepId, status); + if (!d?.plan && stepId && status) cb.onPlanStepUpdate(stepId, status); } if (parsed.type === "provision_supabase_required" && cb.onProvisionSupabase) { @@ -1975,6 +1971,15 @@ function EditorPageInner() { }); const [inputValue, setInputValue] = useState(""); const [isStreaming, setIsStreaming] = useState(false); + const [planError, setPlanError] = useState(null); + const receivePlanSnapshot = useCallback((plan:Plan|null)=>{ + setActivePlan(prev=>acceptPlanSnapshot(prev,plan,resolvedProjectId)); + },[resolvedProjectId]); + useEffect(() => { + if (activePlan) setPlanPhase(activePlan.status === "draft" && chatMode === "plan" ? "reviewing" : activePlan.status !== "draft" ? "building" : "idle"); + }, [activePlan, chatMode]); + usePlanSync(resolvedProjectId,isStreaming,receivePlanSnapshot); + const [keystrokeSignal, setKeystrokeSignal] = useState(0); // Skill manifest for / picker button @@ -3033,7 +3038,7 @@ function EditorPageInner() { ...(persistedAttachments ? { attachments: persistedAttachments } : {}), thinkingContent, toolActions: m.tool_actions || (Array.isArray(m.tool_calls) && m.tool_calls.length > 0 - ? m.tool_calls.map((tc: { name?: string; arguments?: Record }, i: number) => { + ? m.tool_calls.map((tc: { name?: string; arguments?: Record; status?: string }, i: number) => { // Some legacy rows store args double-wrapped under .arguments.arguments. const rawArgs = tc.arguments ?? {}; const args = (rawArgs.arguments && typeof rawArgs.arguments === "object" @@ -3046,7 +3051,7 @@ function EditorPageInner() { isExpanded: false, isBookmarked: false, filePath: (args.path ?? args.filePath ?? args.file) as string | undefined, - status: "completed" as const, + status: tc.status === "completed" ? "completed" as const : tc.status === "failed" ? "failed" as const : "unknown" as const, }; }) : undefined), @@ -3159,31 +3164,6 @@ function EditorPageInner() { useEffect(() => { loadFromApi(); - // Restore active plan state on mount (e.g., after refresh). Only - // restore DRAFT plans when the user's current chat mode is "plan" - // — otherwise a stale draft from a previous plan-mode session - // hijacks the chat UI into PlanCard review state and blocks the - // user who has since switched to build mode. `approved` / - // `in_progress` plans (the AI is actively executing them) always - // restore regardless of mode so a refresh doesn't drop the build - // in flight. - (async () => { - try { - const planRes = await apiFetch<{ data: any }>(`/projects/${resolvedProjectId}/plan`); - if ( - planRes.data && - planRes.data.status === "draft" && - chatMode === "plan" - ) { - setActivePlan(planRes.data); - setPlanPhase("reviewing"); - } else if (planRes.data && (planRes.data.status === "approved" || planRes.data.status === "in_progress")) { - setActivePlan(planRes.data); - setPlanPhase("building"); - } - } catch { /* no active plan */ } - })(); - // Check if AI is still actively working (e.g., user refreshed mid-build). // Strategy: // 1. Call /chat/status to see if there's an active stream and get its messageId. @@ -3365,8 +3345,7 @@ function EditorPageInner() { setPlanPhase("clarifying"); }, onPlan: (plan) => { - setActivePlan(plan); - setPlanPhase("reviewing"); + receivePlanSnapshot(plan); }, onPlanStepUpdate: (stepId, status) => { setActivePlan((prev) => { @@ -3636,8 +3615,7 @@ function EditorPageInner() { setPlanPhase("clarifying"); }, onPlan: (plan) => { - setActivePlan(plan); - setPlanPhase("reviewing"); + receivePlanSnapshot(plan); }, onPlanStepUpdate: (stepId, status) => { setActivePlan((prev) => { @@ -3785,7 +3763,7 @@ function EditorPageInner() { // ─── Handle tool started — add "running" card + update live status ── const handleToolStarted = useCallback( - (toolName: string, _args: Record) => { + (toolName: string, _args: Record, callId?: string) => { // Update live status with human-friendly description const description = describeToolAction(toolName, _args); setLiveStatus(description); @@ -3802,13 +3780,13 @@ function EditorPageInner() { const existing = lastAssistant.toolActions ?? []; // Find existing running action for this tool - const runningIdx = existing.findIndex((a) => a.status === "running" && a.toolName === toolName && (!a.filePath || a.filePath === filePath)); + const runningIdx = existing.findIndex((a) => callId && a.callId === callId || a.status === "running" && (!callId || !a.callId || a.callId.startsWith("hook-")) && a.toolName === toolName && (!a.filePath || a.filePath === filePath)); if (runningIdx !== -1) { // If we got a new filePath or better description, update it! - if ((filePath && !existing[runningIdx]!.filePath) || description !== existing[runningIdx]!.description) { + if ((callId && existing[runningIdx]!.callId !== callId) || (filePath && !existing[runningIdx]!.filePath) || description !== existing[runningIdx]!.description) { const updated = [...existing]; - updated[runningIdx] = { ...updated[runningIdx]!, filePath: filePath ?? updated[runningIdx]!.filePath, description }; + updated[runningIdx] = { ...updated[runningIdx]!, callId: callId ?? updated[runningIdx]!.callId, filePath: filePath ?? updated[runningIdx]!.filePath, description }; return prev.map((m) => m.id === lastAssistant.id ? { ...m, toolActions: updated } : m); } return prev; @@ -3816,6 +3794,7 @@ function EditorPageInner() { const action: ToolAction = { id: `tool-${Date.now()}-${Math.random().toString(36).slice(2, 6)}`, + callId, toolName, description, isExpanded: false, @@ -3835,7 +3814,8 @@ function EditorPageInner() { // ─── Handle tool completion — refresh files + update card ─ const handleToolCompleted = useCallback( - (toolName: string, _args: Record) => { + (toolName: string, _args: Record, success?: boolean, callId?: string) => { + const status = success === true ? "completed" as const : success === false ? "failed" as const : "unknown" as const; // Update the running tool action card to "completed", or add a new completed card setMessages((prev) => { const lastAssistant = [...prev].reverse().find((m) => m.role === "assistant"); @@ -3843,7 +3823,7 @@ function EditorPageInner() { // Try to find a running action with this tool name to mark as completed const runningAction = lastAssistant.toolActions?.find( - (a) => a.toolName === toolName && a.status === "running" + (a) => callId ? a.callId === callId : a.toolName === toolName && a.status === "running" ); const filePath = typeof (_args?.path ?? _args?.filePath ?? _args?.file) === "string" @@ -3863,7 +3843,7 @@ function EditorPageInner() { ...m, toolActions: m.toolActions?.map((a) => a.id === runningAction.id - ? { ...a, status: "completed" as const, description: keepExistingDesc ? a.description : finalDescription, filePath: filePath ?? a.filePath } + ? { ...a, status: a.status === "failed" ? "failed" : status === "unknown" && a.status === "completed" ? "completed" : status, description: keepExistingDesc ? a.description : finalDescription, filePath: filePath ?? a.filePath } : a ), } @@ -3874,12 +3854,13 @@ function EditorPageInner() { // No running card found — add a new completed card (fallback) const action: ToolAction = { id: `tool-${Date.now()}-${Math.random().toString(36).slice(2, 6)}`, + callId, toolName, description: finalDescription, isExpanded: false, isBookmarked: false, filePath, - status: "completed", + status, }; return prev.map((m) => m.id === lastAssistant.id @@ -4054,7 +4035,7 @@ function EditorPageInner() { isStreaming: false, // Mark any remaining "running" tool actions as completed toolActions: m.toolActions?.map((a) => - a.status === "running" ? { ...a, status: "completed" as const } : a + a.status === "running" ? { ...a, status: "unknown" as const } : a ), } : m @@ -4218,8 +4199,7 @@ function EditorPageInner() { setPlanPhase("clarifying"); }, (plan) => { - setActivePlan(plan); - setPlanPhase("reviewing"); + receivePlanSnapshot(plan); }, (stepId, status) => { setActivePlan(prev => { @@ -5621,7 +5601,7 @@ function EditorPageInner() { {/* Plan progress tracker during build */} {planPhase === "building" && activePlan && (
- +
)} @@ -5898,14 +5878,14 @@ function EditorPageInner() {
- ) : ( + ) : allActions.every(a => a.status === "completed") ? ( - )} + ) : }

{msg.isStreaming ? (liveStatus || "Building...") - : `${allActions.length} ${(allActions.length === 1) ? "change" : "changes"} applied`} + : `${allActions.every(a => a.status === "completed") ? "Completed" : "Recorded"} ${allActions.length} actions`}

{allActions.length > 0 && (() => { @@ -5950,9 +5930,9 @@ function EditorPageInner() { ) : action.status === "failed" ? ( - ) : ( + ) : action.status === "completed" ? ( - )} + ) : ?} {formatDescription(action)} @@ -6231,42 +6211,23 @@ function EditorPageInner() { )} + {planError &&

{planError}

} {/* Plan Mode V2: Plan card for review */} {planPhase === "reviewing" && activePlan && (
{ - // Capture plan data before state changes + onApprove={async () => { + setPlanError(null); const plan = activePlan; - const summary = plan.summary; - const stepList = plan.steps.map((s) => `${s.order}. ${s.title}`).join("\n"); - - // Switch mode IMMEDIATELY — don't wait for API - setActivePlan(prev => prev ? { ...prev, status: "approved", approvedAt: new Date().toISOString() } : prev); - setPlanPhase("building"); - setChatMode("agent"); - - // Approve in DB (fire and forget — UI already switched) - const token = getStoredTokens().accessToken; - fetch(`${API_URL}/projects/${resolvedProjectId}/plan/approve`, { - method: "POST", - headers: { - "Content-Type": "application/json", - ...(token ? { Authorization: `Bearer ${token}` } : {}), - }, - body: JSON.stringify({ planId: plan.id }), - }).catch(() => {}); - - // Trigger the AI to start building — pass "agent" mode explicitly - setTimeout(() => { - sendMessage( - `Start building! Here's the approved plan:\n\n**${summary}**\n\n${stepList}\n\nBuild each step in order. The full plan details are in .doable/plan.md.`, - undefined, - "agent" - ); - }, 150); + const stepList = plan.steps.map(s=>`${s.order}. ${s.title} (Step ID: ${s.id})`).join("\n"); + try { + await apiFetch(`/projects/${resolvedProjectId}/plan/approve`,{method:"POST",body:JSON.stringify({planId:plan.id})}); + setActivePlan(prev=>prev?{...prev,status:"approved",revision:(prev.revision??0)+1}:prev); + setPlanPhase("building");setChatMode("agent"); + sendMessage(`Start building! Here's the approved plan:\n\n**${plan.summary}**\n\n${stepList}\n\nBuild each step in order. The full plan details are in .doable/plan.md.`,undefined,"agent"); + } catch(err) {setPlanError(err instanceof Error?err.message:"Could not approve plan. Please try again.");} }} onRefine={() => { sendMessage("Please refine the plan based on my feedback."); diff --git a/apps/web/src/modules/editor/chat/chat-panel.tsx b/apps/web/src/modules/editor/chat/chat-panel.tsx index 63b19e87..b7e96042 100644 --- a/apps/web/src/modules/editor/chat/chat-panel.tsx +++ b/apps/web/src/modules/editor/chat/chat-panel.tsx @@ -6,6 +6,8 @@ import { useChat } from "../hooks/use-chat"; import { useEditorStore } from "../hooks/use-editor-store"; import { ChatMessage } from "./chat-message"; import { ChatInput } from "./chat-input"; +import {usePlanSync} from "../hooks/use-plan-sync"; +import {acceptPlanSnapshot} from "./plan/plan-state"; import { ClarificationFlow, PlanCard, PlanProgress } from "./plan"; import { SupabaseProvisionDialog } from "@/modules/integrations/supabase-provision-dialog"; import { IntegrationConnectDialog } from "@/modules/integrations/integration-connect-dialog"; @@ -58,6 +60,12 @@ export function ChatPanel() { dismissSupabaseProvision, } = useChat(projectId); + const receivePlan = useCallback((plan:import("@doable/shared/types/ai").Plan|null)=> { + if(!projectId)return;const store=useEditorStore.getState(); + const next=acceptPlanSnapshot(store.activePlan,plan,projectId);store.setActivePlan(next); + if(next && next.status!=="draft")store.setPlanPhase("building"); + },[projectId]); + usePlanSync(projectId,isStreaming,receivePlan); const activePlan = useEditorStore((s) => s.activePlan); const planPhase = useEditorStore((s) => s.planPhase); const pendingQuestions = useEditorStore((s) => s.pendingQuestions); @@ -206,7 +214,7 @@ export function ChatPanel() { {/* Sticky plan progress during build */} {planPhase === "building" && activePlan && (
- +
)} diff --git a/apps/web/src/modules/editor/chat/plan/plan-progress.tsx b/apps/web/src/modules/editor/chat/plan/plan-progress.tsx index d1fbe950..e8a9ca05 100644 --- a/apps/web/src/modules/editor/chat/plan/plan-progress.tsx +++ b/apps/web/src/modules/editor/chat/plan/plan-progress.tsx @@ -9,10 +9,12 @@ import { SkipForward, XCircle, } from "lucide-react"; +import {planProgressState} from "./plan-state"; import type { Plan } from "@doable/shared/types/ai"; interface PlanProgressProps { plan: Plan; + running?: boolean; onPause?: () => void; onSkipStep?: (stepId: string) => void; /** When true, renders the compact horizontal pill strip for the sticky header */ @@ -20,8 +22,9 @@ interface PlanProgressProps { } // ─── Step icon ──────────────────────────────────────────────── -function StepIcon({ status }: { status: string }) { +function StepIcon({ status, running }: { status: string; running: boolean }) { if (status === "completed") return ; + if (status === "in_progress" && !running) return ; if (status === "in_progress") return ; if (status === "skipped") return ; if (status === "failed") return ; @@ -33,10 +36,12 @@ function StepPill({ status, title, isLast, + running, }: { status: string; title: string; isLast: boolean; + running: boolean; }) { const [justCompleted, setJustCompleted] = useState(false); const prevStatus = useRef(status); @@ -67,7 +72,7 @@ function StepPill({ : "ring-border bg-transparent" }`} > - +
{!isLast && (
[...plan.steps].sort((a, b) => a.order - b.order), [plan.steps] @@ -99,14 +105,15 @@ function CompactPlanProgress({ plan }: { plan: Plan }) { }; }, [plan.steps]); + const display = planProgressState(plan,running); const activeStep = sortedSteps.find((s) => s.status === "in_progress"); return (
{/* Spinner + label */}
- - Building + {display.spinning ? : display.finished ? : } + {display.label}
{/* Step pills */} @@ -117,6 +124,7 @@ function CompactPlanProgress({ plan }: { plan: Plan }) { status={step.status} title={step.title} isLast={idx === sortedSteps.length - 1} + running={running} /> ))}
@@ -142,6 +150,7 @@ export const PlanProgress = memo(function PlanProgress({ onPause, onSkipStep, compact = false, + running = false, }: PlanProgressProps) { const sortedSteps = useMemo( () => [...plan.steps].sort((a, b) => a.order - b.order), @@ -157,19 +166,20 @@ export const PlanProgress = memo(function PlanProgress({ return { completedCount: done, percentage: pct }; }, [plan.steps]); + const display = planProgressState(plan,running); + // Early return AFTER hooks to keep hook order stable across renders. - if (compact) return ; + if (compact) return ; return (
{/* Header */}
- - Building + {display.spinning ? : display.finished ? : } + {display.label} - {completedCount}/{plan.steps.length} steps - - {onPause && ( + {completedCount}/{plan.steps.length} {"steps"} + {onPause && running && (
+

{"Steps show reported results, not elapsed build time."}{!running && !display.finished && " " + "No active run. Unreported steps remain unconfirmed."}

{/* Progress bar */}
@@ -211,7 +222,7 @@ export const PlanProgress = memo(function PlanProgress({ isActive ? "bg-brand-500/5" : "" }`} > - + { + assert.equal(planProgressState(base, true).spinning, true); + const idle = planProgressState(base, false); + assert.equal(idle.spinning, false); + assert.equal(idle.percentage, 0); + assert.equal(idle.label, "Plan paused"); + assert.equal(idle.finished, false); +}); +test("late snapshots and unrelated projects cannot reverse or replace current progress", () => { + assert.equal( + acceptPlanSnapshot(base, { ...base, revision: 2 }, "project"), + base, + ); + assert.equal( + acceptPlanSnapshot( + base, + { ...base, projectId: "other", revision: 9 }, + "project", + ), + base, + ); + assert.equal( + acceptPlanSnapshot( + base, + { ...base, id: "older", createdAt: "2025-01-01T00:00:00Z" }, + "project", + ), + base, + ); + const next = { ...base, revision: 4 }; + assert.equal(acceptPlanSnapshot(base, next, "project"), next); +}); +test("out-of-order step completion measures results, never the current step index", () => { + const p = { + ...base, + steps: [ + base.steps[0]!, + { ...base.steps[1]!, status: "completed" as const }, + ], + }; + assert.equal(planProgressState(p, true).percentage, 50); +}); +test("failed steps remain unresolved; explicit skips and completed results finish a plan", () => { + assert.equal( + planProgressState( + { + ...base, + steps: [{ ...base.steps[0]!, status: "failed" }, base.steps[1]!], + }, + false, + ).label, + "Plan needs attention", + ); + const p = { + ...base, + steps: [ + { ...base.steps[0]!, status: "skipped" as const }, + { ...base.steps[1]!, status: "completed" as const }, + ], + }; + const display = planProgressState(p, true); + assert.equal(display.finished, true); + assert.equal(display.spinning, false); + assert.equal(display.completed, 1); + assert.equal(display.skipped, 1); + assert.equal(display.percentage, 100); +}); diff --git a/apps/web/src/modules/editor/chat/plan/plan-state.ts b/apps/web/src/modules/editor/chat/plan/plan-state.ts new file mode 100644 index 00000000..61af58b8 --- /dev/null +++ b/apps/web/src/modules/editor/chat/plan/plan-state.ts @@ -0,0 +1,43 @@ +import type { Plan } from "@doable/shared/types/ai"; +export function acceptPlanSnapshot( + current: Plan | null, + next: Plan | null, + projectId: string, +): Plan | null { + if (next && next.projectId !== projectId) + return current?.projectId === projectId ? current : null; + if (!next) return null; + if (current?.id === next.id && (current.revision ?? 0) > (next.revision ?? 0)) + return current; + if ( + current?.projectId === projectId && + current.id !== next.id && + Date.parse(current.createdAt) > Date.parse(next.createdAt) + ) + return current; + return next; +} +export function planProgressState(plan: Plan, running: boolean) { + const completed = plan.steps.filter((s) => s.status === "completed").length; + const skipped = plan.steps.filter((s) => s.status === "skipped").length; + const failed = plan.steps.some((s) => s.status === "failed"); + const finished = + plan.status === "completed" || + (plan.steps.length > 0 && completed + skipped === plan.steps.length); + return { + completed, + skipped, + percentage: plan.steps.length + ? Math.round(((completed + skipped) / plan.steps.length) * 100) + : 0, + finished, + label: finished + ? "Plan finished" + : running + ? "Executing plan" + : failed + ? "Plan needs attention" + : "Plan paused", + spinning: running && !finished, + }; +} diff --git a/apps/web/src/modules/editor/chat/plan/plan-step.tsx b/apps/web/src/modules/editor/chat/plan/plan-step.tsx index 458f5932..5aabd1a1 100644 --- a/apps/web/src/modules/editor/chat/plan/plan-step.tsx +++ b/apps/web/src/modules/editor/chat/plan/plan-step.tsx @@ -8,6 +8,7 @@ import { SkipForward, ChevronDown, X, + XCircle, GripVertical, } from "lucide-react"; import type { PlanStep as PlanStepType } from "@doable/shared/types/ai"; @@ -49,6 +50,13 @@ const STATUS_CONFIG = { iconColor: "text-green-500", badge: null, }, + failed: { + icon: , + borderColor: "border-red-500/40", + leftBorder: "bg-red-500", + iconColor: "text-red-500", + badge: "Failed", + }, skipped: { icon: , borderColor: "border-border/30", diff --git a/apps/web/src/modules/editor/hooks/use-chat-sse.ts b/apps/web/src/modules/editor/hooks/use-chat-sse.ts index 36a35694..49ce1dc1 100644 --- a/apps/web/src/modules/editor/hooks/use-chat-sse.ts +++ b/apps/web/src/modules/editor/hooks/use-chat-sse.ts @@ -1,3 +1,4 @@ +import {acceptPlanSnapshot} from "../chat/plan/plan-state"; import { useEditorStore, type ChatMessage } from "./use-editor-store"; import type { SupabaseProvisionRequest, PendingIntegrationRequest } from "./use-chat-types"; import { @@ -301,8 +302,10 @@ export function dispatchSSEEvent( if (parsed.type === "plan") { const plan = parsed.data?.plan; if (plan) { + const accepted = acceptPlanSnapshot(store.activePlan,plan,store.projectId ?? ""); + if(accepted !== plan)return {}; store.setActivePlan(plan); - store.setPlanPhase("reviewing"); + store.setPlanPhase(plan.status === "draft" ? "reviewing" : "building"); store.setActiveAgentProgress({ phase: "planning", message: "Plan ready for review" }); ctx.updateMessageFields(ctx.assistantId, { agentProgress: { phase: "planning", message: "Plan ready for review" }, @@ -313,7 +316,14 @@ export function dispatchSSEEvent( // ─── Plan step update ───────────────────────────────────── if (parsed.type === "plan_step_update") { - const { stepId, status, message } = parsed.data ?? {}; + const { stepId, status, message, planId, plan } = parsed.data ?? {}; + if(plan) { + const accepted=acceptPlanSnapshot(store.activePlan,plan,store.projectId ?? ""); + if(accepted !== plan)return {}; + store.setActivePlan(plan); + return {}; + } + if(planId && store.activePlan?.id !== planId)return {}; if (stepId && status) { store.updatePlanStep(stepId, { status }); diff --git a/apps/web/src/modules/editor/hooks/use-chat.ts b/apps/web/src/modules/editor/hooks/use-chat.ts index 7ceb0771..08a8cc35 100644 --- a/apps/web/src/modules/editor/hooks/use-chat.ts +++ b/apps/web/src/modules/editor/hooks/use-chat.ts @@ -462,7 +462,7 @@ export function useChat( try { const { getStoredTokens } = await import("@/lib/api"); const { accessToken } = getStoredTokens(); - await fetch(`${API_BASE}/projects/${projectId}/plan/approve`, { + const response = await fetch(`${API_BASE}/projects/${projectId}/plan/approve`, { method: "POST", headers: { "Content-Type": "application/json", @@ -470,6 +470,7 @@ export function useChat( }, body: JSON.stringify({ planId }), }); + if(!response.ok)throw new Error("Could not approve plan. Please try again."); useEditorStore.getState().approvePlan(); setTimeout(() => { sendMessage( diff --git a/apps/web/src/modules/editor/hooks/use-plan-sync.ts b/apps/web/src/modules/editor/hooks/use-plan-sync.ts new file mode 100644 index 00000000..7ea8a7da --- /dev/null +++ b/apps/web/src/modules/editor/hooks/use-plan-sync.ts @@ -0,0 +1,67 @@ +"use client"; +import { useEffect } from "react"; +import { apiFetch } from "@/lib/api"; +import type { Plan } from "@doable/shared/types/ai"; +/** Repair missed SSE events from an authoritative snapshot; never resubmit work. */ +export function usePlanSync( + projectId: string | null, + running: boolean, + onSnapshot: (plan: Plan | null) => void, +) { + useEffect(() => { + if (!projectId) return; + let cancelled = false, + busy = false; + let attempts = 0; + let retry: ReturnType | undefined; + let currentAbort: AbortController | null = null; + const refresh = async () => { + if (busy || cancelled) return; + if (retry) clearTimeout(retry); + busy = true; + const abort = new AbortController(); + currentAbort = abort; + const timeout = setTimeout(() => abort.abort(), 10000); + try { + const { data } = await apiFetch<{ data: Plan | null }>( + `/projects/${projectId}/plan`, + { signal: abort.signal }, + ); + attempts = 0; + if (!cancelled) onSnapshot(data); + } catch (err) { + if (cancelled) return; + if (attempts < 4) + retry = setTimeout(() => void refresh(), 500 * 2 ** attempts++); + console.warn( + "[Plan] Snapshot refresh failed", + err instanceof Error ? err.message : String(err), + ); + } finally { + clearTimeout(timeout); + currentAbort = null; + busy = false; + } + }; + void refresh(); + const onFocus = () => { + if (document.visibilityState === "visible") { + attempts = 0; + void refresh(); + } + }; + window.addEventListener("focus", onFocus); + window.addEventListener("online", onFocus); + document.addEventListener("visibilitychange", onFocus); + const timer = running ? setInterval(() => void refresh(), 8000) : undefined; + return () => { + cancelled = true; + currentAbort?.abort(); + if (retry) clearTimeout(retry); + if (timer) clearInterval(timer); + window.removeEventListener("focus", onFocus); + window.removeEventListener("online", onFocus); + document.removeEventListener("visibilitychange", onFocus); + }; + }, [projectId, running, onSnapshot]); +} diff --git a/docs/plan-progress.md b/docs/plan-progress.md new file mode 100644 index 00000000..27f32c1f --- /dev/null +++ b/docs/plan-progress.md @@ -0,0 +1,52 @@ +# Plan progress and execution lifecycle + +A plan tracks checked task results across chat turns. An active AI turn is a separate lifecycle. An approved plan must not display an endless build spinner just because some steps are still unconfirmed. A finished/interrupted turn also must not mark those steps complete. + +## State contract + +- Database plans/steps are authoritative; `.doable/plan.md` mirrors the latest snapshot for context. Every step has a stable ID, and each committed change advances the plan revision. +- `get_plan` supplies current IDs and results. Execution can report pending, in_progress, completed, skipped or failed through `mark_step_complete`; its omitted status retains the previous completed behavior. Planning does not expose progress mutation tools. +- Completion means the agent checked a result. Progress is the ratio of completed/skipped steps, not elapsed build time or a tool-call count. Skips remain distinguishable from completed results. +- `update_plan` changes order/scope without resetting surviving IDs/statuses. Repeated reports are idempotent; completed/skipped steps must be explicitly reopened to in_progress before changing a terminal result. +- Writers lock the parent plan and commit steps/status/revision atomically. Reports cannot update another project's plan or an unrelated step. HTTP reads/writes require project membership; viewers cannot approve/edit/abandon plans. Approval retries never reset progress. +- SSE updates carry whole persisted snapshots. Clients reject older revisions and snapshots for another project, and recover from the API on mount, focus, visibility/online changes and execution transitions; active runs also reconcile every eight seconds. Snapshot requests time out after ten seconds and retry transient failures with bounded backoff. Recovery never resubmits AI work. +- Without an active run, an unfinished plan displays paused/unconfirmed; failed steps need attention. Old plans with no recorded reports stay unconfirmed. No historical completion is guessed. + +## Scenarios to verify + +Initial approval and execution; work across multiple turns; finishing a later step first; reordering/renaming/adding/removing steps; failure and retry; repeated reports; competing writes; approval failure/retry; missed SSE/reload; stale revision; unrelated project; execution ending before all steps are checked. + +Run the pure tests in `services/api/src/ai/plan-state.test.ts` and `apps/web/src/modules/editor/chat/plan/plan-state.test.ts`. `plan-store.integration.test.ts` requires a disposable PostgreSQL database and isolated project directory; never run it on the production database. + +## Design references + +- [Codex App Server](https://developers.openai.com/codex/app-server): turn lifecycle events are distinct from `turn/plan/updated`; pending/inProgress/completed plans can change during a turn. +- [Claude Code interactive mode](https://code.claude.com/docs/en/interactive-mode) and [tools reference](https://code.claude.com/docs/en/tools-reference): persistent task lists and explicit TaskCreate/TaskUpdate, separate from background task processes. +- [OpenClaw agent loop](https://docs.openclaw.ai/concepts/agent-loop): stable run IDs and explicit lifecycle/settled state; a wait timeout does not imply cancellation. + +These references support separating result state from execution state, with recovery from durable data. They do not guarantee that every provider or transport failure can never delay progress. + +## Recovery contract (2026-10-01) + +The current request lifecycle is separate from persisted plan steps. `report_task_status` reports structured `intent` (read_only/change) and `status` (in_progress/completed/waiting_for_input) in execution modes. These fields are protocol enums, never translated. They do not grant permissions or complete plan steps. Task reports are model assertions; plan acceptance still requires per-step evidence. + +The API owns per-turn tool observations (call ID, cycle, arguments, result digest, status). SDK IDs deduplicate mirrored events; completed invocations must survive repeated calls with identical arguments. Recovery compares complete observations from each cycle, including changed read results. Tool names and successful file writes are not proof the whole request is complete. Legacy history with no result is unknown, not successful. + +Recovery removes sentence/word-count intent heuristics. Unknown clients get one bounded status reconciliation; missing reports become a visible stalled outcome. Two unchanged cycles stop recovery, with an absolute six-cycle ceiling. A stalled/error/aborted/waiting outcome is preserved through final cleanup. Cancellation prevents another recovery round; disconnecting the browser still permits the existing background run. + +No framework, dependency, database schema, or deployment topology change is needed. Existing JSON tool-call history accepts additional result fields. Rollback uses the previous API/Web images; old history remains readable. The tool is read-only metadata and remains available in execution tool manifests, including legacy allow lists. Model conformance and real-tool E2E must be checked before promotion; a passing deterministic replay alone does not qualify a release. + +Validation: `pnpm tsx --test services/api/src/routes/chat/execution-state.test.ts services/api/src/routes/chat/stream-recovery.supabase-gate.test.ts`; UI regressions: `pnpm tsx --tsconfig apps/web/tsconfig.tests.json --test apps/web/src/modules/editor/localization-regressions.test.tsx`. Tests cover distinct/repeated/changed reads, failed writes, database tasks, both languages, negations, filenames, missing reports, provider errors and waiting states. + + +### SDK and stream ownership + +SDK start/end events are the sole source of tool invocation records and cards, including external tool requests. Hook callbacks only handle supplementary plan, clarification, integration and artifact events. Hooks may omit arguments/IDs, so they must never create synthetic invocation rows. Cached sessions rebind their callbacks to the current HTTP stream before each turn, and post-tool callbacks are awaited before the SDK result consumes their artifact metadata. + +`report_task_status` is hidden from operation cards while retaining its structured trace. History cards are projected from saved tool observations; old display-only success labels cannot turn missing evidence into success. The regression suite includes a real hook-plus-SDK replay with empty hook parameters and a cached-callback rebinding check. + +### Refreshing reused session instructions + +A cached session now fingerprints the effective system prompt (including current project context). An unchanged prompt only rebinds the current turn callbacks. A changed or previously unknown prompt resumes the same SDK session ID with the current system message, preserving history. Persisted sessions also receive the current system message through both engine and SDK resume configuration. If refreshing a cached session fails, the request fails rather than silently using stale instructions. Mode/provider/context eviction clears the fingerprint alongside the session mapping. + +The targeted session, execution, ledger, final-response and parser regression run passes 39 tests. Session tests cover unchanged prompts, changed project files, language and protocol preservation, legacy cached sessions, resume failure, lost engine bindings, and project-scoped eviction. These unit tests do not assert that every provider honors resumed system messages; a real provider check remains a separate integration requirement. diff --git a/packages/shared/src/types/ai.ts b/packages/shared/src/types/ai.ts index f1f2418c..d8a9dd48 100644 --- a/packages/shared/src/types/ai.ts +++ b/packages/shared/src/types/ai.ts @@ -5,7 +5,7 @@ export type AiMode = "agent" | "plan" | "chat"; // ─── Plan Mode V2 ───────────────────────────────────────── export type PlanStatus = "draft" | "approved" | "in_progress" | "completed" | "abandoned"; -export type PlanStepStatus = "pending" | "in_progress" | "completed" | "skipped"; +export type PlanStepStatus = "pending" | "in_progress" | "completed" | "skipped" | "failed"; export type PlanComplexity = "simple" | "moderate" | "complex"; export type PlanPhase = "idle" | "clarifying" | "planning" | "reviewing" | "building"; @@ -40,6 +40,7 @@ export interface Plan { originalPrompt?: string; clarificationAnswers?: Record; createdAt: string; + revision?: number; approvedAt?: string; completedAt?: string; } diff --git a/services/api/src/ai/context/injector.ts b/services/api/src/ai/context/injector.ts index f4a769ce..710ecd6e 100644 --- a/services/api/src/ai/context/injector.ts +++ b/services/api/src/ai/context/injector.ts @@ -58,7 +58,7 @@ export function buildSystemPrompt( context.contextFiles["plan.md"] ) { sections.push( - wrapSection("ACTIVE PLAN", "There is an active development plan at `.doable/plan.md`. Read it with read_file before starting work. Follow the plan step by step."), + wrapSection("ACTIVE PLAN", "There is an active development plan at `.doable/plan.md`. Read it with read_file before starting work. Read current IDs with get_plan before continuing. Report checked results with mark_step_complete, and revise scope/order with update_plan while retaining existing IDs. Never infer step completion from the end of a chat turn."), ); } diff --git a/services/api/src/ai/modes/plan.ts b/services/api/src/ai/modes/plan.ts index 8af75461..af36156a 100644 --- a/services/api/src/ai/modes/plan.ts +++ b/services/api/src/ai/modes/plan.ts @@ -1,3 +1,4 @@ +import {extractPlanFromResponse as extractPlan} from "../plan-parser.js"; import { randomUUID } from "node:crypto"; import type { ConversationMessage, @@ -19,7 +20,7 @@ import { planEvent, } from "../streaming.js"; import { updateContextFile } from "../context/index.js"; -import { sql } from "../../db/index.js"; + // Tools allowed in plan mode: read-only + plan-specific const PLAN_MODE_TOOLS = new Set([ @@ -28,6 +29,7 @@ const PLAN_MODE_TOOLS = new Set([ "search_files", "ask_clarification", "create_plan", + "get_plan", ]); // ─── Plan Mode Handler ─────────────────────────────────── @@ -184,24 +186,6 @@ export async function* runPlanMode( if (result.metadata?.type === "plan") { const plan = result.metadata.plan as Plan; - // Save plan to database - try { - await savePlanToDb(plan, toolCtx.projectId); - } catch (err) { - const msg = err instanceof Error ? err.message : String(err); - yield errorEvent(`Failed to save plan to DB: ${msg}`, "DB_ERROR", true); - // Continue anyway — emit the plan event so the frontend gets it - } - - // Save plan to .doable/plan.md - try { - const markdown = planToMarkdown(plan); - await updateContextFile(toolCtx.projectId, "plan.md", markdown); - } catch (err) { - const msg = err instanceof Error ? err.message : String(err); - yield errorEvent(`Failed to save plan file: ${msg}`, "SAVE_ERROR", true); - } - yield planEvent(plan); return; } @@ -209,83 +193,10 @@ export async function* runPlanMode( } } -// ─── Save Plan to Database ─────────────────────────────── - -async function savePlanToDb(plan: Plan, projectId: string): Promise { - // Insert the plan record - await sql` - INSERT INTO plans (id, project_id, summary, complexity, status, original_prompt, clarification_answers, created_at) - VALUES ( - ${plan.id}, - ${projectId}, - ${plan.summary}, - ${plan.complexity}, - ${plan.status}, - ${plan.originalPrompt ?? null}, - ${plan.clarificationAnswers ? JSON.stringify(plan.clarificationAnswers) : null}, - ${plan.createdAt} - ) - `; - - // Insert all plan steps - for (const step of plan.steps) { - await sql` - INSERT INTO plan_steps (id, plan_id, "order", title, description, details, status, file_paths) - VALUES ( - ${step.id}, - ${plan.id}, - ${step.order}, - ${step.title}, - ${step.description}, - ${step.details ?? null}, - ${step.status}, - ${step.filePaths ?? null} - ) - `; - } -} - -// ─── Plan to Markdown ──────────────────────────────────── - -export function planToMarkdown(plan: Plan): string { - let md = `# Plan\n\n${plan.summary}\n\n**Complexity:** ${plan.complexity}\n\n`; - for (const step of plan.steps) { - md += `## ${step.order}. ${step.title}\n\n${step.description}\n\n`; - if (step.details) md += `**Details:** ${step.details}\n\n`; - if (step.filePaths?.length) md += `**Files:** ${step.filePaths.join(", ")}\n\n`; - } - return md; -} +export { planToMarkdown } from "../plan-state.js"; // ─── Plan Extraction (Fallback) ────────────────────────── -function extractPlan(text: string): string | null { - // Look for a markdown plan structure in the response - const planHeaderPattern = /^#\s+Plan/m; - - if (planHeaderPattern.test(text)) { - const match = text.match(planHeaderPattern); - if (match?.index !== undefined) { - return text.slice(match.index).trim(); - } - } - - // If the whole response looks like a plan, use it all - if ( - text.includes("##") && - (text.includes("Step") || text.includes("Task") || text.includes("Phase")) - ) { - return `# Plan\n\n${text.trim()}`; - } - - // Fallback: wrap the entire response as a plan if long enough - if (text.trim().length > 100) { - return `# Plan\n\n${text.trim()}`; - } - - return null; -} - // ─── Plan Prompt ────────────────────────────────────────── const PLAN_GENERATION_PROMPT = `You have two tools for planning: ask_clarification and create_plan. diff --git a/services/api/src/ai/plan-parser.ts b/services/api/src/ai/plan-parser.ts index 6579f1ee..5e351427 100644 --- a/services/api/src/ai/plan-parser.ts +++ b/services/api/src/ai/plan-parser.ts @@ -25,7 +25,7 @@ export function parsePlanSteps(planContent: string | null | undefined): Array<{ for (const line of lines) { // Match "## 1. Title", "## Step 1: Title", or "- [ ] Title" - const headingMatch = line.match(/^##\s+(?:(?:Step\s+)?\d+[\.:]\s*)?(.+)/i); + const headingMatch = line.match(/^#{2,3}\s+(?:(?:Step|Task|Phase)\s+\d+|(?:步骤|阶段|任务)\s*[一二三四五六七八九十百\d]+|\d+)[.::、))]?\s*(.+)/i); const checkboxMatch = line.match(/^-\s+\[[ x]\]\s+(.+)/i); const match = headingMatch || checkboxMatch; if (match) { @@ -61,29 +61,11 @@ export function parsePlanSteps(planContent: string | null | undefined): Array<{ * Looks for markdown plan structure and wraps it appropriately. */ export function extractPlanFromResponse(text: string): string | null { - // Look for a markdown plan header - const planHeaderPattern = /^#\s+Plan/m; - if (planHeaderPattern.test(text)) { - const match = text.match(planHeaderPattern); - if (match?.index !== undefined) { - return text.slice(match.index).trim(); - } - } - - // If the response looks like a structured plan, wrap it - if ( - text.includes("##") && - (text.includes("Step") || text.includes("Task") || text.includes("Phase")) - ) { - return `# Plan\n\n${text.trim()}`; - } + // Prose length and UI language are not evidence of a structured plan. + if (!parsePlanSteps(text).length) return null; + const header = /^#\s+(?:Plan\b|计划|实施计划|执行计划)[^\n]*/im.exec(text); + return header?.index !== undefined ? text.slice(header.index).trim() : `# Plan\n\n${text.trim()}`; - // Fallback: if substantial text, treat it all as a plan - if (text.trim().length > 200) { - return `# Plan\n\n${text.trim()}`; - } - - return null; } /** diff --git a/services/api/src/ai/plan-state.test.ts b/services/api/src/ai/plan-state.test.ts new file mode 100644 index 00000000..c4b46d09 --- /dev/null +++ b/services/api/src/ai/plan-state.test.ts @@ -0,0 +1,87 @@ +import assert from "node:assert/strict"; +import { test } from "node:test"; +import { planToMarkdown, revisedSteps, isStepStatus } from "./plan-state.js"; +import { selectModeTools } from "./plan-tool-policy.js"; +const old = [ + { + id: "a", + order: 1, + title: "A", + description: "A", + status: "completed" as const, + details: "Verified", + filePaths: ["src/A.tsx"], + }, + { + id: "b", + order: 2, + title: "B", + description: "B", + status: "in_progress" as const, + }, +]; +test("reordering/renaming retains stable identity, checked result and metadata", () => { + const next = revisedSteps(old, [ + { ...old[1]!, order: 1 }, + { id: "a", order: 2, title: "A revised", description: "A" }, + { id: "c", order: 3, title: "C", description: "C" }, + ]); + assert.deepEqual( + next.map((s) => [s.id, s.order, s.status]), + [ + ["b", 1, "in_progress"], + ["a", 2, "completed"], + ["c", 3, "pending"], + ], + ); + assert.equal(next[1]?.details, "Verified"); + assert.deepEqual(next[1]?.filePaths, ["src/A.tsx"]); +}); +test("duplicate IDs fail before any persistence", () => + assert.throws(() => revisedSteps(old, [old[0]!, old[0]!]), /Duplicate/)); +test("the model receives stable step IDs, revision and recovery/reporting rules", () => { + const md = planToMarkdown({ + id: "p", + projectId: "project", + summary: "Plan", + complexity: "simple", + status: "approved", + revision: 7, + createdAt: new Date().toISOString(), + steps: old, + }); + assert.match(md, /Plan ID: p/); + assert.match(md, /Step ID: a/); + assert.match(md, /Revision: 7/); + assert.match(md, /get_plan/); + assert.match(md, /finished chat turn does not complete/); +}); +test("execution exposes progress tools; plan mode never exposes progress writes, even with a legacy DB list", () => { + const tools = [ + "create_plan", + "ask_clarification", + "get_plan", + "mark_step_complete", + "update_plan", + "edit_file", + ].map((name) => ({ name })); + assert.deepEqual( + selectModeTools(tools, "agent").map((t) => t.name), + ["get_plan", "mark_step_complete", "update_plan", "edit_file"], + ); + assert.deepEqual( + selectModeTools(tools, "plan", new Set(tools.map((t) => t.name))).map( + (t) => t.name, + ), + ["create_plan", "ask_clarification", "get_plan", "edit_file"], + ); + assert.deepEqual( + selectModeTools(tools, "agent", new Set(["edit_file"])).map((t) => t.name), + ["edit_file"], + ); +}); +test("invalid statuses cannot pass the reporting boundary", () => { + assert.equal(isStepStatus("failed"), true); + assert.equal(isStepStatus("done"), false); + assert.equal(isStepStatus(undefined), false); +}); diff --git a/services/api/src/ai/plan-state.ts b/services/api/src/ai/plan-state.ts new file mode 100644 index 00000000..7ac3706b --- /dev/null +++ b/services/api/src/ai/plan-state.ts @@ -0,0 +1,50 @@ +import type { + Plan, + PlanStep, + PlanStepStatus, +} from "@doable/shared/types/ai.js"; + +export const stepStatuses = [ + "pending", + "in_progress", + "completed", + "skipped", + "failed", +] as const; +export function isStepStatus(value: unknown): value is PlanStepStatus { + return ( + typeof value === "string" && + (stepStatuses as readonly string[]).includes(value) + ); +} +export function planToMarkdown(plan: Plan): string { + let md = `# Plan\n\nPlan ID: ${plan.id}\nRevision: ${plan.revision ?? 0}\nStatus: ${plan.status}\n\n${plan.summary}\n\n**Complexity:** ${plan.complexity}\n\n`; + for (const step of [...plan.steps].sort((a, b) => a.order - b.order)) { + md += `## ${step.order}. ${step.title}\n\nStep ID: ${step.id}\nStatus: ${step.status}\n\n${step.description}\n\n`; + if (step.details) md += `**Details:** ${step.details}\n\n`; + if (step.filePaths?.length) + md += `**Files:** ${step.filePaths.join(", ")}\n\n`; + } + return ( + md + + `\nUse get_plan to read current IDs and status. Call mark_step_complete with status=in_progress when starting a step, completed only after checking its result, failed on a confirmed failure, or skipped with an explanation. Use update_plan to revise order/scope while keeping existing step IDs. A finished chat turn does not complete unverified steps.\n` + ); +} +export function revisedSteps( + previous: PlanStep[], + next: Array & { id: string }>, +): PlanStep[] { + const ids = new Set(); + return next.map((step, index) => { + if (ids.has(step.id)) throw new Error("Duplicate step ID"); + ids.add(step.id); + const prior = previous.find((item) => item.id === step.id); + return { + ...step, + details: step.details ?? prior?.details, + filePaths: step.filePaths ?? prior?.filePaths, + order: index + 1, + status: prior?.status ?? "pending", + }; + }); +} diff --git a/services/api/src/ai/plan-store.integration.test.ts b/services/api/src/ai/plan-store.integration.test.ts new file mode 100644 index 00000000..3fc4d4b3 --- /dev/null +++ b/services/api/src/ai/plan-store.integration.test.ts @@ -0,0 +1,34 @@ +/** Run only against a disposable PostgreSQL database (never production). */ +import assert from 'node:assert/strict'; +import {test,after} from 'node:test'; +import {randomUUID} from 'node:crypto'; +import {readFile} from 'node:fs/promises'; +import {sql,closeDb} from '../db/index.js'; +import {savePlan,getPlan,setPlanStep,revisePlan,approvePlan} from './plan-store.js'; +import type {Plan} from '@doable/shared/types/ai.js'; +if(!process.env.DOABLE_PLAN_TEST_DB)throw new Error('A disposable DOABLE_PLAN_TEST_DB is required'); +after(closeDb); +test('PostgreSQL transactions preserve progress, revision, scope and concurrent reports',async()=>{ + await sql.unsafe(await readFile(new URL('../db/migrations/025_plans.sql',import.meta.url),'utf8')); + await sql`CREATE TABLE mode_tool_config (mode text,allowed_tools text[],updated_at timestamptz)`; + await sql.unsafe(await readFile(new URL('../db/migrations/103_plan_revision.sql',import.meta.url),'utf8')); + const plan:Plan={id:randomUUID(),projectId:randomUUID(),summary:'Disposable test',status:'draft',complexity:'simple',createdAt:new Date().toISOString(),steps:[1,2,3].map(order=>({id:randomUUID(),order,title:`Step ${order}`,description:'Check a result',status:'pending'}))}; + await savePlan(plan);assert.equal((await getPlan(plan.projectId))?.revision,0); + await assert.rejects(setPlanStep(plan.projectId,plan.id,plan.steps[0]!.id,'completed'),/Approved/); + const approved=await approvePlan(plan.projectId,plan.id);assert.equal(approved.revision,1);assert.equal((await approvePlan(plan.projectId,plan.id)).revision,1); + await assert.rejects(setPlanStep('other-project',plan.id,plan.steps[0]!.id,'completed'),/not found/); + await assert.rejects(setPlanStep(plan.projectId,plan.id,'foreign-step','completed'),/not found/); + await Promise.all([setPlanStep(plan.projectId,plan.id,plan.steps[2]!.id,'completed'),setPlanStep(plan.projectId,plan.id,plan.steps[0]!.id,'completed')]); + const partial=(await getPlan(plan.projectId))!;assert.equal(partial.revision,3);assert.equal(partial.steps.filter(s=>s.status==='completed').length,2);assert.equal(partial.status,'in_progress'); + assert.equal((await setPlanStep(plan.projectId,plan.id,plan.steps[0]!.id,'completed')).revision,3); + await assert.rejects(setPlanStep(plan.projectId,plan.id,plan.steps[0]!.id,'pending'),/Reopen/); + const reordered=await revisePlan(plan.projectId,plan.id,[partial.steps[2]!,partial.steps[1]!,partial.steps[0]!]);assert.deepEqual(reordered.steps.map(s=>s.status),['completed','pending','completed']);assert.equal(reordered.revision,4); + await assert.rejects(revisePlan(plan.projectId,plan.id,[partial.steps[0]!,partial.steps[0]!]),/Duplicate/);assert.equal((await getPlan(plan.projectId))?.revision,4); + const failed=await setPlanStep(plan.projectId,plan.id,plan.steps[1]!.id,'failed');assert.equal(failed.status,'in_progress'); + const complete=await setPlanStep(plan.projectId,plan.id,plan.steps[1]!.id,'completed');assert.equal(complete.status,'completed');assert.equal(complete.revision,6); + const reopened=await setPlanStep(plan.projectId,plan.id,plan.steps[1]!.id,'in_progress');assert.equal(reopened.status,'in_progress');assert.equal(reopened.completedAt,undefined); + const mirror=await readFile(`${process.env.DOABLE_PROJECTS_DIR}/${plan.projectId}/.doable/plan.md`,'utf8');assert.match(mirror,/Revision: 7/); + const later={...plan,id:randomUUID(),createdAt:new Date().toISOString(),steps:plan.steps.map(s=>({...s,id:randomUUID()}))};await savePlan(later); + await sql`UPDATE plans SET status='abandoned',revision=revision+1 WHERE id=${later.id}`; + assert.equal(await getPlan(plan.projectId),null,'abandoning the latest plan must not resurrect an older plan'); +}); diff --git a/services/api/src/ai/plan-store.ts b/services/api/src/ai/plan-store.ts new file mode 100644 index 00000000..b8342c10 --- /dev/null +++ b/services/api/src/ai/plan-store.ts @@ -0,0 +1,179 @@ +import type { + Plan, + PlanStep, + PlanStepStatus, +} from "@doable/shared/types/ai.js"; +import { sql } from "../db/index.js"; +import { updateContextFile } from "./context/index.js"; +import { isStepStatus, planToMarkdown, revisedSteps } from "./plan-state.js"; + +async function readPlan( + db: typeof sql, + projectId: string, + planId?: string, + lock = false, +): Promise { + const rows = planId + ? await db`SELECT * FROM plans WHERE project_id=${projectId} AND id=${planId} ${lock ? db`FOR UPDATE` : db``}` + : await db`SELECT * FROM plans WHERE project_id=${projectId} ORDER BY created_at DESC LIMIT 1 ${lock ? db`FOR UPDATE` : db``}`; + const row = rows[0]; + if (!row || row.status === "abandoned") return null; + const steps = + await db`SELECT * FROM plan_steps WHERE plan_id=${row.id} ORDER BY "order"`; + return { + id: row.id, + projectId: row.project_id, + summary: row.summary, + complexity: row.complexity, + status: row.status, + revision: Number(row.revision ?? 0), + createdAt: row.created_at.toISOString(), + approvedAt: row.approved_at?.toISOString(), + completedAt: row.completed_at?.toISOString(), + originalPrompt: row.original_prompt ?? undefined, + clarificationAnswers: row.clarification_answers ?? undefined, + steps: steps.map( + (s): PlanStep => ({ + id: s.id, + order: s.order, + title: s.title, + description: s.description, + details: s.details ?? undefined, + filePaths: s.file_paths ?? undefined, + status: s.status, + }), + ), + }; +} +export async function getPlan( + projectId: string, + planId?: string, +): Promise { + return sql.begin(async (tx) => { + return readPlan(tx as unknown as typeof sql, projectId, planId, true); + }); +} +const mirrors = new Map>(); +async function mirror(plan: Plan) { + const previous = mirrors.get(plan.projectId) ?? Promise.resolve(); + const write = previous + .catch(() => {}) + .then(async () => { + const latest = await getPlan(plan.projectId); + if (latest) + await updateContextFile( + plan.projectId, + "plan.md", + planToMarkdown(latest), + ); + }) + .catch((err) => + console.warn( + "[Plan] Context mirror failed; DB is authoritative:", + err instanceof Error ? err.message : err, + ), + ); + mirrors.set(plan.projectId, write); + await write; + if (mirrors.get(plan.projectId) === write) mirrors.delete(plan.projectId); +} +export async function savePlan(plan: Plan): Promise { + await sql.begin(async (tx) => { + const db = tx as unknown as typeof sql; + await db`INSERT INTO plans (id,project_id,summary,complexity,status,created_at,revision,original_prompt,clarification_answers) VALUES (${plan.id},${plan.projectId},${plan.summary},${plan.complexity},${plan.status},${plan.createdAt},0,${plan.originalPrompt ?? null},${plan.clarificationAnswers ? db.json(plan.clarificationAnswers) : null})`; + for (const step of plan.steps) + await db`INSERT INTO plan_steps (id,plan_id,"order",title,description,details,status,file_paths) VALUES (${step.id},${plan.id},${step.order},${step.title},${step.description},${step.details ?? null},${step.status},${step.filePaths ?? null})`; + }); + const saved = (await getPlan(plan.projectId, plan.id))!; + await mirror(saved); + return saved; +} +export async function setPlanStep( + projectId: string, + planId: string, + stepId: string, + status: PlanStepStatus, +): Promise { + if (!isStepStatus(status)) throw new Error("Invalid step status"); + const updated = await sql.begin(async (tx) => { + const db = tx as unknown as typeof sql; + const plan = await readPlan(db, projectId, planId, true); + if ( + !plan || + !["approved", "in_progress", "completed"].includes(plan.status) + ) + throw new Error("Approved plan not found in this project"); + const step = plan.steps.find((s) => s.id === stepId); + if (!step) throw new Error("Step not found in this plan"); + // Replays are idempotent. Explicitly reopen a completed step before changing its result. + if (step.status === status) return plan; + if ( + ["completed", "skipped"].includes(step.status) && + status !== "in_progress" + ) + throw new Error("Reopen the step before changing a terminal result"); + await db`UPDATE plan_steps SET status=${status}, started_at=CASE WHEN ${status}='in_progress' THEN COALESCE(started_at,now()) ELSE started_at END, completed_at=CASE WHEN ${status} IN ('completed','skipped') THEN now() ELSE NULL END WHERE id=${stepId} AND plan_id=${planId}`; + const allDone = plan.steps.every((s) => + ["completed", "skipped"].includes(s.id === stepId ? status : s.status), + ); + await db`UPDATE plans SET status=${allDone ? "completed" : "in_progress"}, completed_at=CASE WHEN ${allDone} THEN now() ELSE NULL END, revision=revision+1 WHERE id=${planId} AND project_id=${projectId}`; + return (await readPlan(db, projectId, planId))!; + }); + await mirror(updated); + return updated; +} +export async function revisePlan( + projectId: string, + planId: string, + steps: Array>, +): Promise { + if (!steps.length) throw new Error("Plan must contain at least one step"); + const updated = await sql.begin(async (tx) => { + const db = tx as unknown as typeof sql; + const plan = await readPlan(db, projectId, planId, true); + if (!plan || plan.status === "abandoned") + throw new Error("Plan not found in this project"); + const next = revisedSteps(plan.steps, steps); + // Editing the plan does not erase progress on surviving IDs. + for (const step of next) { + if (plan.steps.some((s) => s.id === step.id)) { + await db`UPDATE plan_steps SET "order"=${step.order},title=${step.title},description=${step.description},details=${step.details ?? null},file_paths=${step.filePaths ?? null} WHERE id=${step.id} AND plan_id=${planId}`; + } else { + await db`INSERT INTO plan_steps (id,plan_id,"order",title,description,details,status,file_paths) VALUES (${step.id},${planId},${step.order},${step.title},${step.description},${step.details ?? null},'pending',${step.filePaths ?? null})`; + } + } + for (const step of plan.steps) + if (!next.some((s) => s.id === step.id)) + await db`DELETE FROM plan_steps WHERE id=${step.id} AND plan_id=${planId}`; + const finished = next.every((s) => + ["completed", "skipped"].includes(s.status), + ); + const status = + plan.status === "draft" + ? "draft" + : finished + ? "completed" + : "in_progress"; + await db`UPDATE plans SET status=${status},completed_at=CASE WHEN ${status}='completed' THEN now() ELSE NULL END,revision=revision+1 WHERE id=${planId} AND project_id=${projectId}`; + return (await readPlan(db, projectId, planId))!; + }); + await mirror(updated); + return updated; +} + +export async function approvePlan( + projectId: string, + planId: string, +): Promise { + const updated = await sql.begin(async (tx) => { + const db = tx as unknown as typeof sql; + const plan = await readPlan(db, projectId, planId, true); + if (!plan || plan.status === "abandoned") + throw new Error("Plan not found in this project"); + if (plan.status !== "draft") return plan; // Retrying an approval never resets progress. + await db`UPDATE plans SET status='approved', approved_at=now(),revision=revision+1 WHERE id=${planId} AND project_id=${projectId}`; + return (await readPlan(db, projectId, planId))!; + }); + await mirror(updated); + return updated; +} diff --git a/services/api/src/ai/plan-tool-policy.ts b/services/api/src/ai/plan-tool-policy.ts new file mode 100644 index 00000000..e869e96b --- /dev/null +++ b/services/api/src/ai/plan-tool-policy.ts @@ -0,0 +1,24 @@ +/** Plan mode is read-only; progress mutations belong to execution mode. */ +export function selectModeTools( + tools: T[], + mode: string, + allowed?: ReadonlySet | null, +): T[] { + const planning = new Set([ + "read_file", + "list_files", + "search_files", + "ask_clarification", + "create_plan", + "get_plan", + ]); + const planningOnly = new Set(["ask_clarification", "create_plan"]); + const progressWrites = new Set(["mark_step_complete", "update_plan"]); + return tools.filter((tool) => { + const name = tool.name ?? ""; + if(name === "report_task_status") return mode !== "plan"; + if (mode === "plan" && progressWrites.has(name)) return false; + if (allowed) return allowed.has(name); + return mode === "plan" ? planning.has(name) : !planningOnly.has(name); + }); +} diff --git a/services/api/src/ai/provider-error.ts b/services/api/src/ai/provider-error.ts new file mode 100644 index 00000000..dfd61845 --- /dev/null +++ b/services/api/src/ai/provider-error.ts @@ -0,0 +1,53 @@ +/** Preserve protocol error codes through policy decisions; localize only at display time. */ +export function classifyProviderError(error: unknown): string { + const data = + error && typeof error === "object" + ? (error as Record) + : {}; + const status = Number(data.statusCode ?? data.status); + const code = String(data.code ?? data.errorType ?? "").toLowerCase(); + const message = String( + data.message ?? data.error ?? error ?? "", + ).toLowerCase(); + if ( + status === 401 || + status === 403 || + /^(invalid_api_key|authentication_error|permission_denied)$/.test(code) + ) + return "AUTH"; + if (/^(insufficient_quota|quota_exceeded|insufficient_balance)$/.test(code)) + return "QUOTA"; + if (status === 429 || /^(rate_limit_exceeded|rate_limit_error)$/.test(code)) + return "RATE_LIMIT"; + if ( + status === 408 || + status === 504 || + /^(timeout|request_timeout)$/.test(code) + ) + return "TIMEOUT"; + if (status >= 500 && status <= 599) return "SERVER"; + if (status === 404) return "NOT_FOUND"; + // Compatibility with providers that omit structured metadata (not UI text). + if (/quota|insufficient.?balance|余额不足|配额|额度不足/.test(message)) + return "QUOTA"; + if ( + /rate.?limit|too many requests|\b429\b|请求过于频繁|限流|请求频率/.test( + message, + ) + ) + return "RATE_LIMIT"; + if ( + /unauthorized|forbidden|authentication|\b40[13]\b|认证失败|鉴权失败|无效.{0,8}密钥/.test( + message, + ) + ) + return "AUTH"; + if (/timeout|timed out|deadline|超时/.test(message)) return "TIMEOUT"; + if (/network|econn|enotfound|dns|socket|网络/.test(message)) return "NETWORK"; + if (/not found|\b404\b/.test(message)) return "NOT_FOUND"; + if ( + /\b50[0-9]\b|internal server|bad gateway|service unavailable/.test(message) + ) + return "SERVER"; + return "UNKNOWN"; +} diff --git a/services/api/src/ai/providers/copilot-engine.rebind.test.ts b/services/api/src/ai/providers/copilot-engine.rebind.test.ts new file mode 100644 index 00000000..f76352f3 --- /dev/null +++ b/services/api/src/ai/providers/copilot-engine.rebind.test.ts @@ -0,0 +1,28 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { CopilotEngine } from "./copilot-engine.js"; +import type { CopilotSessionConfig } from "../engine-types.js"; + +test("cached session callbacks target the current turn, not its original stream", async () => { + const engine = new CopilotEngine(); + const calls: string[] = []; + const config: Partial = { + toolProgress: { onToolEnd: () => { calls.push("old"); } }, + }; + // Represents the config captured by the SDK hook at create/resume time. + const hook = () => config.toolProgress?.onToolEnd?.("read_file", {}, {}); + const internals = engine as unknown as { + sessionConfigs: Map>; + engines: Map; + }; + internals.sessionConfigs.set("cached", config); + internals.engines.set("cached", {}); + assert.equal(engine.bindToolProgress("missing", {}), false); + assert.equal(engine.bindToolProgress("cached", { + onToolEnd: () => { calls.push("current"); }, + }), true); + await hook(); + assert.deepEqual(calls, ["current"]); + internals.engines.delete("cached"); + assert.equal(engine.bindToolProgress("cached", {}), false); +}); diff --git a/services/api/src/ai/providers/copilot-engine.ts b/services/api/src/ai/providers/copilot-engine.ts index 7b74db6c..1f21b052 100644 --- a/services/api/src/ai/providers/copilot-engine.ts +++ b/services/api/src/ai/providers/copilot-engine.ts @@ -7,6 +7,7 @@ */ import path from "node:path"; +import { systemMessageConfig } from "../session-prompt.js"; import { DoCorePool, DoCoreEngine } from "docore"; import type { SessionEvent, @@ -26,7 +27,7 @@ const PLAN_ALLOWED_TOOLS = new Set([ // Custom read-only tools "read_file", "list_files", "search_files", // Custom plan-specific tools - "ask_clarification", "create_plan", "mark_step_complete", + "ask_clarification", "create_plan", "get_plan", ]); // Tools whose first argument is a project file path that may need @@ -194,6 +195,8 @@ export class CopilotEngine { private abortedSessions = new Set(); private sessionWakeups = new Map void>(); private sessionModes = new Map(); + // Hooks outlive an HTTP stream. Rebind their callback target on every turn. + private sessionConfigs = new Map>(); constructor(config: CopilotEngineConfig = {}) { this.config = config; @@ -228,6 +231,7 @@ export class CopilotEngine { } this.engines.clear(); this.sessionModes.clear(); + this.sessionConfigs.clear(); await this.pool.stop(); this.pool = null; console.log("[CopilotEngine] Pool stopped"); @@ -269,9 +273,7 @@ export class CopilotEngine { ...(config.skillDirectories && config.skillDirectories.length > 0 ? { skillDirectories: config.skillDirectories } : {}), - ...(config.systemPrompt - ? { systemMessage: { mode: "replace" as const, content: config.systemPrompt } } - : {}), + ...systemMessageConfig(config.systemPrompt), hooks: { onPreToolUse: async (input: { toolName: string; toolArgs: unknown }) => { // Rewrite file paths BEFORE the SDK permission @@ -316,7 +318,7 @@ export class CopilotEngine { if (rewritten) return { modifiedArgs: rewritten }; }, onPostToolUse: async (input: { toolName: string; toolArgs: unknown; toolResult: unknown }) => { - config.toolProgress?.onToolEnd?.(input.toolName, input.toolArgs, input.toolResult); + await config.toolProgress?.onToolEnd?.(input.toolName, input.toolArgs, input.toolResult); }, onSessionEnd: async (input: { reason: string; error?: string }) => { config.toolProgress?.onSessionEnd?.(input.reason, input.error); @@ -335,11 +337,15 @@ export class CopilotEngine { const sessionId = engine.sessionId!; currentSessionId = sessionId; this.engines.set(sessionId, engine); + this.sessionConfigs.set(sessionId, config); return sessionId; } async resumeSession(sessionId: string, config?: Partial): Promise { this.ensurePool(); + // Release old hooks/connection while preserving the SDK's persisted history. + // This is also used to apply updated instructions to a cached conversation. + await this.disconnectSession(sessionId); // Mutable ref captured by hook closure — set after resume() let currentSessionId: string | undefined = sessionId; @@ -356,6 +362,7 @@ export class CopilotEngine { workingDirectory: config?.workingDirectory, onPermissionRequest: config?.onPermissionRequest, sessionConfig: { + ...systemMessageConfig(config?.systemPrompt), ...(config?.provider ? { provider: config.provider } : {}), ...(config?.tools ? { tools: config.tools } : {}), ...(config?.skillDirectories && config.skillDirectories.length > 0 @@ -398,7 +405,7 @@ export class CopilotEngine { } if (rewritten) return { modifiedArgs: rewritten }; }, - onPostToolUse: async (input: { toolName: string; toolArgs: unknown; toolResult: unknown }) => { config.toolProgress?.onToolEnd?.(input.toolName, input.toolArgs, input.toolResult); }, + onPostToolUse: async (input: { toolName: string; toolArgs: unknown; toolResult: unknown }) => { await config.toolProgress?.onToolEnd?.(input.toolName, input.toolArgs, input.toolResult); }, onSessionEnd: async (input: { reason: string; error?: string }) => { config.toolProgress?.onSessionEnd?.(input.reason, input.error); }, onErrorOccurred: async (input: { error: string; errorContext: string }) => { config.toolProgress?.onError?.(input.error, input.errorContext); }, }, @@ -406,6 +413,7 @@ export class CopilotEngine { }, }); await engine.resume(sessionId, { + ...systemMessageConfig(config?.systemPrompt), onPermissionRequest: config?.onPermissionRequest, streaming: true, workingDirectory: config?.workingDirectory, @@ -424,9 +432,18 @@ export class CopilotEngine { const newSessionId = engine.sessionId!; currentSessionId = newSessionId; this.engines.set(newSessionId, engine); + if (config) this.sessionConfigs.set(newSessionId, config); return newSessionId; } + /** Return false when the cached session was lost and must be resumed. */ + bindToolProgress(sessionId: string, toolProgress: CopilotSessionConfig["toolProgress"]): boolean { + const config = this.sessionConfigs.get(sessionId); + if (!config || !this.engines.has(sessionId)) return false; + config.toolProgress = toolProgress; + return true; + } + async setSessionMode(sessionId: string, mode: "interactive" | "plan" | "autopilot"): Promise { const engine = this.engines.get(sessionId); if (!engine) throw new Error(`Session ${sessionId} not found`); @@ -612,11 +629,12 @@ export class CopilotEngine { await engine.disconnectSession(); this.engines.delete(sessionId); this.sessionModes.delete(sessionId); + this.sessionConfigs.delete(sessionId); } async deleteSession(sessionId: string): Promise { const engine = this.engines.get(sessionId); - if (engine) { await engine.deleteSession(sessionId); await engine.disconnectSession(); this.engines.delete(sessionId); this.sessionModes.delete(sessionId); } + if (engine) { await engine.deleteSession(sessionId); await engine.disconnectSession(); this.engines.delete(sessionId); this.sessionModes.delete(sessionId); this.sessionConfigs.delete(sessionId); } } async getSessionMessages(sessionId: string): Promise { diff --git a/services/api/src/ai/providers/copilot-tools.ts b/services/api/src/ai/providers/copilot-tools.ts index 3f363a28..d6cd6888 100644 --- a/services/api/src/ai/providers/copilot-tools.ts +++ b/services/api/src/ai/providers/copilot-tools.ts @@ -1,3 +1,4 @@ +import {parseTaskReport} from "../../routes/chat/execution-state.js"; /** * Doable-specific tools for Copilot agent sessions. * @@ -8,6 +9,7 @@ import { defineTool, type Tool } from "@github/copilot-sdk"; import { existsSync } from "node:fs"; +import { randomUUID } from "node:crypto"; import path from "node:path"; import { readFile, @@ -23,6 +25,8 @@ import { tanStackHijackViolation, } from "../../projects/detect-tanstack-start.js"; import { sql } from "../../db/index.js"; +import { getPlan, savePlan, setPlanStep, revisePlan } from "../plan-store.js"; +import { isStepStatus } from "../plan-state.js"; import { createBashTool } from "../tools/bash.js"; import { validateFileSyntax } from "../tools/validate-syntax.js"; @@ -144,6 +148,14 @@ export interface DoableToolOptions { export function createDoableTools(projectId: string, userId?: string, workspaceId?: string, options?: DoableToolOptions): Tool[] { const hasSupabase = options?.hasSupabase ?? false; return ([ + defineTool("report_task_status", { + description: "Report the ORIGINAL request intent and lifecycle before acting and before ending. read_only means questions/reviews; change means explicitly requested changes. in_progress means requested work remains; completed means the request is addressed; waiting_for_input means a user decision is needed. Does not mark plan steps complete or authorize actions.", + parameters: {type:"object" as const, properties:{intent:{type:"string" as const,enum:["read_only","change"]},status:{type:"string" as const,enum:["in_progress","completed","waiting_for_input"]}},required:["intent","status"]}, + handler: async (args: Record) => { + const report=parseTaskReport(args); + return report ? {success:true,...report} : {success:false,error:"Invalid task report"}; + }, + }), defineTool("create_file", { description: "Create or overwrite a file in the project. Required fields are `path` (RELATIVE, e.g. 'index.html', 'src/App.tsx') and `content` (the full file body as a string). Do NOT use `file_text` — this tool uses `content`. Do NOT pass `command`. Do NOT use absolute paths or `/app/...` prefixes.", overridesBuiltInTool: true, @@ -431,20 +443,7 @@ export function createDoableTools(projectId: string, userId?: string, workspaceI })); const plan = { id: planId, projectId, summary: args.summary, complexity: args.complexity, steps, status: "draft" as const, createdAt: new Date().toISOString() }; - try { - const { writeFile: fsWrite, mkdir } = await import("node:fs/promises"); - const { join } = await import("node:path"); - const projectPath = getProjectPath(projectId); - await mkdir(join(projectPath, ".doable"), { recursive: true }); - let md = `# Plan\n\nPlan ID: ${planId}\n\n${args.summary}\n\n**Complexity:** ${args.complexity}\n\n`; - for (const step of steps) { - md += `## ${step.order}. ${step.title}\n\n${step.description}\n\n`; - if (step.details) md += `**Details:** ${step.details}\n\n`; - if (step.filePaths?.length) md += `**Files:** ${step.filePaths.join(", ")}\n\n`; - } - md += `\n---\nAfter each step, call mark_step_complete(stepId, planId).\n`; - await fsWrite(join(projectPath, ".doable", "plan.md"), md, "utf-8"); - } catch { /* non-fatal */ } + await savePlan(plan as import("@doable/shared/types/ai.js").Plan); emitToolEvent(projectId, "create_plan", "start", {}); emitToolEvent(projectId, "create_plan", "end", { output: JSON.stringify(plan) }); @@ -550,19 +549,46 @@ export function createDoableTools(projectId: string, userId?: string, workspaceI }, }), + defineTool("get_plan", { + description: "Read the current project plan, stable step IDs, revision and reported progress. Use before executing or resuming an approved plan.", + parameters: { type: "object" as const, properties: {} }, + handler: async () => ({ success: true, plan: await getPlan(projectId) }), + }), defineTool("mark_step_complete", { - description: "Mark a plan step as completed during build execution.", + description: "Report progress on an approved plan step. Set in_progress when starting, completed after checking the result, failed on failure, or skipped when explicitly omitted. Never infer completion from chat ending. IDs come from get_plan.", parameters: { type: "object" as const, properties: { - stepId: { type: "string" as const, description: "The step ID to mark complete" }, - planId: { type: "string" as const, description: "The plan ID" }, + stepId: { type: "string" as const }, planId: { type: "string" as const }, + status: { type: "string" as const, enum: ["pending", "in_progress", "completed", "skipped", "failed"] as const }, }, required: ["stepId", "planId"] as const, }, - handler: async (args: { stepId: string; planId: string }) => { - emitToolEvent(projectId, "mark_step_complete", "end", { stepId: args.stepId, planId: args.planId, status: "completed" }); - return { success: true, stepId: args.stepId, planId: args.planId, status: "completed" }; + handler: async (args: { stepId: string; planId: string; status?: string }) => { + const status = args.status ?? "completed"; + if (!isStepStatus(status)) return { success: false, error: "Invalid step status" }; + try { + const plan = await setPlanStep(projectId, args.planId, args.stepId, status); + emitToolEvent(projectId, "mark_step_complete", "end", { stepId: args.stepId, planId: args.planId, status, plan }); + return { success: true, plan, stepId: args.stepId, status }; + } catch (err) { return { success:false, error: err instanceof Error ? err.message : String(err) }; } + }, + }), + defineTool("update_plan", { + description: "Revise the current approved plan when the work changes or needs a different order. Keep IDs of existing steps so their progress survives; omit an ID only for a new step. Do not claim completion by rewriting the list.", + parameters: { + type: "object" as const, + properties: { + planId: {type:"string" as const}, + steps: {type:"array" as const, items:{type:"object" as const, properties:{id:{type:"string" as const},title:{type:"string" as const},description:{type:"string" as const}},required:["title","description"] as const}}, + }, required:["planId","steps"] as const, + }, + handler: async (args: {planId:string;steps:Array<{id?:string;title:string;description:string}>}) => { + try { + const plan = await revisePlan(projectId,args.planId,args.steps.map((step,i)=>({...step,id:step.id ?? randomUUID(),order:i+1}))); + emitToolEvent(projectId,"update_plan","end",{plan}); + return {success:true,plan}; + } catch(err) { return {success:false,error:err instanceof Error ? err.message : String(err)}; } }, }), diff --git a/services/api/src/ai/session-prompt.test.ts b/services/api/src/ai/session-prompt.test.ts new file mode 100644 index 00000000..463f9e20 --- /dev/null +++ b/services/api/src/ai/session-prompt.test.ts @@ -0,0 +1,107 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { fingerprintSystemPrompt, reuseSessionWithPrompt, systemMessageConfig } from "./session-prompt.js"; + +test("replacement preserves Chinese instructions and machine contracts verbatim", () => { + const prompt = "You are Doable's assistant. 默认中文;继续已批准计划。\n" + + "report_task_status: read_only | change; waiting_for_input\n" + + "import { db } from '@doable/data'; .doable/plan.md"; + assert.deepEqual(systemMessageConfig(prompt), { + systemMessage: { mode: "replace", content: prompt }, + }); + assert.notEqual(fingerprintSystemPrompt(prompt), fingerprintSystemPrompt(prompt.replace("默认中文", "English"))); + assert.equal(fingerprintSystemPrompt(prompt), fingerprintSystemPrompt(prompt)); +}); + +test("omitted prompt keeps SDK defaults; explicit empty prompt clears old instructions", () => { + assert.deepEqual(systemMessageConfig(undefined), {}); + assert.deepEqual(systemMessageConfig(""), { + systemMessage: { mode: "replace", content: "" }, + }); +}); + +test("unchanged cached prompt only binds callbacks for the current turn", async () => { + let bindings = 0; + const fingerprint = fingerprintSystemPrompt("Answer in the user's language."); + const id = await reuseSessionWithPrompt({ + sessionId: "existing-history", + previousFingerprint: fingerprint, + nextFingerprint: fingerprint, + bindCurrentTurn: () => { bindings++; return true; }, + resumeCurrentPrompt: async () => { assert.fail("unchanged prompt must not reconnect"); }, + }); + assert.equal(id, "existing-history"); + assert.equal(bindings, 1); +}); + +test("changed instructions resume the existing history with the new prompt", async () => { + const history = ["Build a staff directory", "Created src/App.tsx", "继续"]; + const sessions = new Map([["existing-history", history]]); + const next = "Use Simplified Chinese. Do not change existing data."; + let observedPrompt = ""; + const id = await reuseSessionWithPrompt({ + sessionId: "existing-history", + previousFingerprint: fingerprintSystemPrompt("Use English."), + nextFingerprint: fingerprintSystemPrompt(next), + bindCurrentTurn: () => { assert.fail("stale instructions must not be reused"); }, + resumeCurrentPrompt: async (sessionId) => { + assert.equal(sessions.get(sessionId), history); + observedPrompt = systemMessageConfig(next).systemMessage!.content; + return sessionId; + }, + }); + assert.equal(id, "existing-history"); + assert.equal(observedPrompt, next); + assert.deepEqual(sessions.get(id!), history); +}); + +test("file-list/context changes refresh the same session instead of discarding its history", async () => { + const seen: string[] = []; + let previous = "Current project files:\nsrc/App.tsx"; + for (const file of ["src/components/Employee.tsx", "src/components/Payroll.tsx"]) { + const next = `${previous}\n${file}`; + const id = await reuseSessionWithPrompt({ + sessionId: "existing-history", + previousFingerprint: fingerprintSystemPrompt(previous), + nextFingerprint: fingerprintSystemPrompt(next), + bindCurrentTurn: () => { assert.fail("new project context needs refresh"); }, + resumeCurrentPrompt: async (sessionId) => { seen.push(sessionId); return sessionId; }, + }); + assert.equal(id, "existing-history"); + previous = next; + } + assert.deepEqual(seen, ["existing-history", "existing-history"]); +}); + +test("a cached session without a recorded prompt is refreshed once", async () => { + let refreshed = false; + assert.equal(await reuseSessionWithPrompt({ + sessionId: "legacy-history", + previousFingerprint: undefined, + nextFingerprint: fingerprintSystemPrompt("Current instructions"), + bindCurrentTurn: () => { assert.fail("unknown prompt must not be trusted"); }, + resumeCurrentPrompt: async (id) => { refreshed = true; return id; }, + }), "legacy-history"); + assert.equal(refreshed, true); +}); + +test("refresh failure propagates without sending under stale instructions", async () => { + const failure = new Error("provider unavailable"); + await assert.rejects(reuseSessionWithPrompt({ + sessionId: "existing-history", + previousFingerprint: fingerprintSystemPrompt("Old instructions"), + nextFingerprint: fingerprintSystemPrompt("Current instructions"), + bindCurrentTurn: () => { assert.fail("must not bind stale session"); }, + resumeCurrentPrompt: async () => { throw failure; }, + }), (error) => error === failure); +}); + +test("a recycled engine with unchanged instructions falls through to persisted-session recovery", async () => { + assert.equal(await reuseSessionWithPrompt({ + sessionId: "existing-history", + previousFingerprint: "same", + nextFingerprint: "same", + bindCurrentTurn: () => false, + resumeCurrentPrompt: async () => { assert.fail("normal engine-loss recovery owns this path"); }, + }), undefined); +}); diff --git a/services/api/src/ai/session-prompt.ts b/services/api/src/ai/session-prompt.ts new file mode 100644 index 00000000..8c9facce --- /dev/null +++ b/services/api/src/ai/session-prompt.ts @@ -0,0 +1,29 @@ +import { createHash } from "node:crypto"; + +/** Compare effective prompts without retaining/logging project context. */ +export function fingerprintSystemPrompt(prompt: string): string { + return createHash("sha256").update(prompt).digest("hex"); +} + +/** Shared by create and resume: an empty prompt explicitly clears stale content. */ +export function systemMessageConfig(prompt: string | undefined) { + return prompt === undefined + ? {} + : { systemMessage: { mode: "replace" as const, content: prompt } }; +} + +/** Refresh in place by resuming the same SDK history, never creating a new one. + * A failed refresh must propagate: sending with stale instructions or silently + * starting an empty conversation would both violate the user's context. */ +export async function reuseSessionWithPrompt(options: { + sessionId: string; + previousFingerprint: string | undefined; + nextFingerprint: string; + bindCurrentTurn: () => boolean; + resumeCurrentPrompt: (sessionId: string) => Promise; +}): Promise { + if (options.previousFingerprint !== options.nextFingerprint) { + return options.resumeCurrentPrompt(options.sessionId); + } + return options.bindCurrentTurn() ? options.sessionId : undefined; +} diff --git a/services/api/src/ai/sse-mapper.ts b/services/api/src/ai/sse-mapper.ts index 219a5e52..4f298db6 100644 --- a/services/api/src/ai/sse-mapper.ts +++ b/services/api/src/ai/sse-mapper.ts @@ -1,3 +1,5 @@ +import {toolSucceeded} from "./tool-outcome.js"; +import {classifyProviderError} from "./provider-error.js"; /** * SSE event mapper — maps SDK session events to SSE events for the client. * Also exports ChannelTokenRouter for model thinking/reasoning tag parsing. @@ -173,7 +175,25 @@ export class ChannelTokenRouter { } -export function mapEventToSSE(event: Record): SSEEvent | null { +/** Decode SDK envelopes once so lifecycle bookkeeping and rendering agree. */ +export function extractToolArguments(data: Record): Record | undefined { + let value: unknown = data.arguments ?? data.args ?? data.input; + for (let depth = 0; depth < 4; depth++) { + if (typeof value === "string") { try { value = JSON.parse(value); } catch { return undefined; } } + if (!value || typeof value !== "object" || Array.isArray(value)) return undefined; + const record = value as Record; + if (record.arguments !== undefined) { value = record.arguments; continue; } + return record; + } + return undefined; +} + +/** requestId-only external completions acknowledge dispatch, not tool execution. */ +export function isExternalToolAcknowledgement(data: Record | undefined): boolean { + return !data?.toolCallId || (data.result === undefined && data.output === undefined && typeof data.success !== "boolean"); +} + +export function mapEventToSSE(event: Record, options: { preserveThinkingMarkers?: boolean } = {}): SSEEvent | null { const type = event.type as string; const data = event.data as Record | undefined; @@ -182,14 +202,14 @@ export function mapEventToSSE(event: Record): SSEEvent | null { case "assistant.message_delta": { const delta = (data?.deltaContent ?? "") as string; if (!delta) return null; - return { type: "text_delta", data: sanitizeText(delta) }; + return { type: "text_delta", data: sanitizeText(delta, options) }; } // ─── SDK v0.2.0 streaming delta (raw text chunks) ──── case "assistant.streaming_delta": { const streamDelta = (data?.deltaContent ?? data?.content ?? data?.delta ?? "") as string; if (!streamDelta) return null; - return { type: "text_delta", data: sanitizeText(streamDelta) }; + return { type: "text_delta", data: sanitizeText(streamDelta, options) }; } // ─── Final complete message (sent after streaming ends) ─ @@ -199,7 +219,7 @@ export function mapEventToSSE(event: Record): SSEEvent | null { // ─── Legacy / direct provider text events ───────────── case "text_delta": { const raw = (data?.content ?? data ?? "") as string; - return { type: "text_delta", data: sanitizeText(String(raw)) }; + return { type: "text_delta", data: sanitizeText(String(raw), options) }; } // ─── Streaming reasoning deltas (token-by-token thinking) ── @@ -223,18 +243,14 @@ export function mapEventToSSE(event: Record): SSEEvent | null { case "external_tool.requested": { const startToolName = (data?.toolName ?? data?.name) as string | undefined; if (!startToolName) return null; - // Unwrap SDK envelope { toolName, arguments: {...real args...}, toolCallId } - const rawStartArgs = (data?.arguments ?? data?.args ?? data?.input) as - | Record - | undefined; - const startArgs = (rawStartArgs && typeof (rawStartArgs as { arguments?: unknown }).arguments === "object") - ? (rawStartArgs as { arguments: Record }).arguments - : rawStartArgs; + if (startToolName === "report_task_status") return null; + const startArgs = extractToolArguments(data ?? {}); const startPath = (startArgs?.path ?? startArgs?.filePath ?? startArgs?.file ?? startArgs?.target) as string | undefined; return { type: "tool_call", data: { name: startToolName, + toolCallId: data?.toolCallId, ...(startArgs ? { arguments: startArgs } : {}), ...(startPath ? { path: startPath } : {}), }, @@ -243,25 +259,21 @@ export function mapEventToSSE(event: Record): SSEEvent | null { // ─── Tool results (completed) ───────────────────────── case "tool.completed": - case "tool.execution_complete": { + case "tool.execution_complete": + case "external_tool.completed": { + if (type === "external_tool.completed" && isExternalToolAcknowledgement(data)) return null; const resultToolName = (data?.toolName ?? data?.name) as string; const toolResult = data?.result as Record | undefined; - // Some SDK channels wrap the request args under .arguments - // ({ toolName, arguments: {...real args...}, toolCallId }); unwrap so - // the client sees the user-facing path/command fields. - const rawReqArgs = (data?.arguments ?? data?.args ?? data?.input) as - | Record - | undefined; - const reqArgs = (rawReqArgs && typeof (rawReqArgs as { arguments?: unknown }).arguments === "object") - ? (rawReqArgs as { arguments: Record }).arguments - : rawReqArgs; + if (resultToolName === "report_task_status") return null; + const reqArgs = extractToolArguments(data ?? {}); const reqPath = (reqArgs?.path ?? reqArgs?.filePath ?? reqArgs?.file ?? reqArgs?.target) as string | undefined; return { type: "tool_result", data: { name: resultToolName, - success: data?.success, - friendlyMessage: friendlyToolResult(resultToolName, data?.result, data?.success), + success: toolSucceeded(data?.result ?? data?.output,data?.success), + toolCallId: data?.toolCallId, + friendlyMessage: friendlyToolResult(resultToolName, data?.result ?? data?.output, toolSucceeded(data?.result ?? data?.output,data?.success)), // Pass through request args so the client can label cards with the // correct file name (BUG: "Reading file" instead of "Reading App.tsx"). ...(reqArgs ? { args: reqArgs } : {}), @@ -272,19 +284,17 @@ export function mapEventToSSE(event: Record): SSEEvent | null { }, }; } - case "external_tool.completed": - return null; // ─── Errors ─────────────────────────────────────────── case "session.error": { const rawMsg = String(data?.message ?? data?.errorType ?? "Unknown error"); - const statusCode = data?.statusCode as number | undefined; + const category=classifyProviderError(data ?? rawMsg); let userMsg: string; - if (statusCode === 404 || rawMsg.includes("404")) { + if (category === "NOT_FOUND") { userMsg = "The AI model is unavailable (404). Check your model ID and provider settings."; - } else if (statusCode === 401 || rawMsg.includes("unauthorized") || rawMsg.includes("not authorized")) { + } else if (category === "AUTH") { userMsg = "Authentication failed with the AI provider. Check your API key."; - } else if (statusCode === 429 || statusCode === 503 || rawMsg.includes("rate limit") || rawMsg.includes("rate_limit") || rawMsg.includes("quota")) { + } else if (category === "RATE_LIMIT" || category === "QUOTA") { userMsg = `⚠️ Rate limit exceeded — the AI provider is rejecting requests due to too many calls. Please wait a minute before trying again, or switch to a different model in AI Settings. (Provider error: ${rawMsg.slice(0, 200)})`; } else { userMsg = sanitizeText(rawMsg); diff --git a/services/api/src/ai/tool-messages.ts b/services/api/src/ai/tool-messages.ts index 1a90d49c..3c4a216d 100644 --- a/services/api/src/ai/tool-messages.ts +++ b/services/api/src/ai/tool-messages.ts @@ -40,6 +40,7 @@ export function friendlyToolMessage( // Internal SDK tools — give them human-friendly names if (toolName === "report_intent") return "Planning"; + if (toolName === "report_task_status") return "Checking task progress"; if (toolName === "create_plan") return "Creating plan"; if (toolName === "mark_step_complete") return "Tracking progress"; @@ -293,7 +294,7 @@ export function sanitizeCommand(cmd: string): string { return result; } -export function sanitizeText(text: string): string { +export function sanitizeText(text: string, options: { preserveThinkingMarkers?: boolean } = {}): string { if (!text) return text; let result = text; @@ -303,10 +304,12 @@ export function sanitizeText(text: string): string { // <|channel>thought, , <|channel|> — Gemma 4 // , — Claude (when prompted) // , — DeepSeek (post-thinking answer marker) - result = result.replace(/<\/?think>/gi, ""); - result = result.replace(/<\|?channel\|?>(?:thought)?/gi, ""); - result = result.replace(/<\/?rationale>/gi, ""); - result = result.replace(/<\/?answer>/gi, ""); + if (!options.preserveThinkingMarkers) { + result = result.replace(/<\/?think>/gi, ""); + result = result.replace(/<\|?channel\|?>(?:thought)?/gi, ""); + result = result.replace(/<\/?rationale>/gi, ""); + result = result.replace(/<\/?answer>/gi, ""); + } // 1. Strip absolute server paths result = stripServerPaths(result); @@ -325,10 +328,10 @@ export function sanitizeText(text: string): string { * history on reload. */ export function buildToolActionsFromCalls( - toolCalls: Array<{ name?: string; arguments?: Record | undefined }>, + toolCalls: Array<{ name?: string; arguments?: Record | undefined; status?: string }>, assistantMessageId: string, ): Array> { - return toolCalls.map((tc, i) => { + return toolCalls.filter(tc => tc.name !== "report_task_status").map((tc, i) => { const toolName = tc.name ?? "unknown"; const args = tc.arguments ?? {}; const filePath = @@ -339,7 +342,9 @@ export function buildToolActionsFromCalls( const lower = toolName.toLowerCase(); let description = toolName; - if ( + if (["create_plan","get_plan","update_plan","mark_step_complete","report_task_status"].includes(toolName)) { + description = friendlyToolMessage(toolName,args); + } else if ( lower.includes("bash") || lower.includes("shell") || lower.includes("powershell") || lower.includes("cmd") || lower.includes("exec") || lower.includes("run_command") || lower.includes("terminal") @@ -385,7 +390,14 @@ export function buildToolActionsFromCalls( isExpanded: false, isBookmarked: false, filePath, - status: "completed" as const, + status: tc.status === "completed" || tc.status === "failed" ? tc.status : "unknown", }; }); } + +/** Rebuild cached display cards from observations without rewriting legacy rows. */ +export function projectHistoryToolActions(message:T): T { + if (!Array.isArray(message.tool_calls) || message.tool_calls.length === 0) return message; + const calls=message.tool_calls.filter((call): call is {name?: string; arguments?: Record;status?:string} => !!call && typeof call === "object"); + return {...message,tool_actions:buildToolActionsFromCalls(calls,String(message.id ?? "history"))}; +} diff --git a/services/api/src/ai/tool-outcome.ts b/services/api/src/ai/tool-outcome.ts new file mode 100644 index 00000000..58491b5d --- /dev/null +++ b/services/api/src/ai/tool-outcome.ts @@ -0,0 +1,32 @@ +/** Normalize SDK and MCP result envelopes before any display or persistence. */ +export function toolSucceeded(result: unknown, explicit?: unknown): boolean { + return evaluateResult(result, explicit, true); +} + +function evaluateResult(result: unknown, explicit: unknown, sdkEnvelope: boolean): boolean { + if (explicit === false) return false; + if (result && typeof result === "object") { + const r = result as Record; + if (r.success === false || r.isError === true || r.error) return false; + for (const payload of [ + r.output, + r.textResultForLlm, + ...(sdkEnvelope ? [r.content, r.detailedContent] : []), + ...(Array.isArray(r.content) + ? r.content.map((item: any) => item?.text) + : []), + ]) { + if (typeof payload === "string") { + try { + const parsed = JSON.parse(payload); + if (parsed && typeof parsed === "object" && !evaluateResult(parsed, undefined, false)) + return false; + } catch { + /* Plain output is not a machine status. */ + } + } + } + if (r.exitCode !== undefined && r.exitCode !== 0) return false; + } + return true; +} diff --git a/services/api/src/ai/tools/index.ts b/services/api/src/ai/tools/index.ts index d5bd2097..56c02eb3 100644 --- a/services/api/src/ai/tools/index.ts +++ b/services/api/src/ai/tools/index.ts @@ -91,6 +91,7 @@ import { searchFilesTool } from "./search-files.js"; import { installPackageTool } from "./install-package.js"; import { askClarificationTool } from "./plan-tools.js"; import { createPlanTool } from "./plan-tools.js"; +import {getPlanTool, updatePlanTool} from "./plan-tools.js"; import { markStepCompleteTool } from "./plan-tools.js"; toolRegistry.register(createFileTool); @@ -104,3 +105,5 @@ toolRegistry.register(installPackageTool); toolRegistry.register(askClarificationTool); toolRegistry.register(createPlanTool); toolRegistry.register(markStepCompleteTool); +toolRegistry.register(getPlanTool); +toolRegistry.register(updatePlanTool); diff --git a/services/api/src/ai/tools/plan-tools.ts b/services/api/src/ai/tools/plan-tools.ts index 3c95d396..5ac68160 100644 --- a/services/api/src/ai/tools/plan-tools.ts +++ b/services/api/src/ai/tools/plan-tools.ts @@ -1,3 +1,5 @@ +import { getPlan, savePlan, setPlanStep, revisePlan } from "../plan-store.js"; +import { isStepStatus } from "../plan-state.js"; import { randomUUID } from "node:crypto"; import type { ClarificationQuestion, @@ -193,6 +195,7 @@ export const createPlanTool: Tool = { createdAt: now, }; + await savePlan(plan); return { success: true, output: JSON.stringify(plan), @@ -209,19 +212,20 @@ export const createPlanTool: Tool = { export const markStepCompleteTool: Tool = { name: "mark_step_complete", description: - "Mark a plan step as completed during build execution. Call this after you finish implementing a step from the active plan.", + "Report a plan step as in_progress, completed, failed or skipped using stable IDs from get_plan. completed means its result was checked.", parameters: { type: "object", required: ["stepId", "planId"], properties: { stepId: { type: "string" }, planId: { type: "string" }, + status: { type: "string", enum: ["pending", "in_progress", "completed", "skipped", "failed"] }, }, }, async execute( params: Record, - _ctx: ToolContext, + ctx: ToolContext, ): Promise { const stepId = params.stepId as string; const planId = params.planId as string; @@ -242,14 +246,22 @@ export const markStepCompleteTool: Tool = { }; } - return { - success: true, - output: JSON.stringify({ planId, stepId, status: "completed" }), - metadata: { - type: "plan_step_update", - planId, - stepId, - }, - }; + const status = params.status ?? "completed"; + if (!isStepStatus(status)) return {success:false,output:"",error:"Invalid step status"}; + const plan = await setPlanStep(ctx.projectId,planId,stepId,status); + return {success:true,output:JSON.stringify({planId,stepId,status,plan}),metadata:{type:"plan_step_update",planId,stepId,status,plan}}; + }, +}; +export const getPlanTool: Tool = { + name:"get_plan",description:"Read current project plan, step IDs and reported progress before continuing.",parameters:{type:"object",properties:{}}, + async execute(_params,ctx) {return {success:true,output:JSON.stringify(await getPlan(ctx.projectId))};}, +}; +export const updatePlanTool: Tool = { + name:"update_plan",description:"Revise plan order/scope retaining IDs and progress on surviving steps.",parameters:{type:"object",properties:{planId:{type:"string"},steps:{type:"array",items:{type:"object",properties:{id:{type:"string"},title:{type:"string"},description:{type:"string"}},required:["title","description"]}}},required:["planId","steps"]}, + async execute(params,ctx) { + if(typeof params.planId !== "string" || !Array.isArray(params.steps)) return {success:false,output:"",error:"planId and steps required"}; + const steps=(params.steps as Array<{id?:string;title:string;description:string}>).map((step,i)=>({...step,id:step.id??randomUUID(),order:i+1})); + const plan=await revisePlan(ctx.projectId,params.planId,steps); + return {success:true,output:JSON.stringify(plan),metadata:{type:"plan",plan}}; }, }; diff --git a/services/api/src/ai/trace-factory.ts b/services/api/src/ai/trace-factory.ts index c28c74e2..7f969087 100644 --- a/services/api/src/ai/trace-factory.ts +++ b/services/api/src/ai/trace-factory.ts @@ -360,7 +360,7 @@ export function createTraceCollector(ctx: TraceCollectorContext) { function setModel(model: string): void { ctx.model = model; } async function complete( - status: "completed" | "error" | "aborted" | "stalled", + status: "completed" | "error" | "aborted" | "stalled" | "waiting", usage?: TraceUsageSummary, ): Promise { const turnEndedAt = Date.now(); diff --git a/services/api/src/ai/trace-types.ts b/services/api/src/ai/trace-types.ts index 870ddde8..d13722c0 100644 --- a/services/api/src/ai/trace-types.ts +++ b/services/api/src/ai/trace-types.ts @@ -1,3 +1,4 @@ +import {classifyProviderError} from "./provider-error.js"; // ─── Trace Collector Types ───────────────────────────────── export interface TraceCollectorContext { @@ -34,6 +35,8 @@ export interface TraceUsageSummary { // ─── Error categorization ────────────────────────────────── export function categorizeError(message: string): string { + const classified=classifyProviderError(message); + if(classified!=="UNKNOWN") return classified; const m = message.toLowerCase(); if (m.includes("auth") || m.includes("unauthorized") || m.includes("forbidden") || m.includes("401") || m.includes("403")) return "AUTH"; if (m.includes("timeout") || m.includes("timed out") || m.includes("deadline")) return "TIMEOUT"; diff --git a/services/api/src/db/migrations/103_plan_revision.sql b/services/api/src/db/migrations/103_plan_revision.sql new file mode 100644 index 00000000..9ec01505 --- /dev/null +++ b/services/api/src/db/migrations/103_plan_revision.sql @@ -0,0 +1,7 @@ +ALTER TABLE plans ADD COLUMN IF NOT EXISTS revision BIGINT NOT NULL DEFAULT 0; + +-- Upgrade only the shipped legacy allowlists. Never overwrite a customized policy. +UPDATE mode_tool_config SET allowed_tools=ARRAY['read_file','list_files','search_files','ask_clarification','create_plan','get_plan','view','grep','glob','ask_user','report_intent'],updated_at=now() +WHERE mode='plan' AND allowed_tools=ARRAY['read_file','list_files','search_files','ask_clarification','create_plan','mark_step_complete','view','grep','glob','ask_user','report_intent']; +UPDATE mode_tool_config SET allowed_tools=array_append(array_append(allowed_tools,'get_plan'),'update_plan'),updated_at=now() +WHERE mode='build' AND allowed_tools=ARRAY['create_file','edit_file','read_file','list_files','install_package','deploy_preview','provision_supabase','request_integration','mark_step_complete','view','grep','glob','ask_user','report_intent','bash','edit']; diff --git a/services/api/src/routes/admin-tools.ts b/services/api/src/routes/admin-tools.ts index 1e8781ba..6c602add 100644 --- a/services/api/src/routes/admin-tools.ts +++ b/services/api/src/routes/admin-tools.ts @@ -22,6 +22,8 @@ const KNOWN_TOOLS = [ { name: "deploy_preview", category: "doable", description: "Deploy to preview URL" }, { name: "ask_clarification", category: "doable", description: "Ask user clarifying questions" }, { name: "create_plan", category: "doable", description: "Create step-by-step plan" }, + { name: "get_plan", category: "doable", description: "Read current project plan and stable step IDs" }, + { name: "update_plan", category: "doable", description: "Revise plan order and scope preserving step IDs" }, { name: "mark_step_complete", category: "doable", description: "Mark a plan step as completed" }, { name: "provision_supabase", category: "doable", description: "Provision Supabase project" }, { name: "request_integration", category: "doable", description: "Request third-party integration" }, @@ -43,10 +45,10 @@ const KNOWN_TOOLS = [ // override)" instead of acting like the admin has to wire tools from scratch // before chat works. The `agent` mode allows every known tool except the // three plan-only ones; `plan` mirrors PLAN_MODE_ALLOWED_DEFAULT. -const PLAN_ONLY_TOOL_NAMES = ["ask_clarification", "create_plan", "mark_step_complete"]; +const PLAN_ONLY_TOOL_NAMES = ["ask_clarification", "create_plan"]; const PLAN_DEFAULT_ALLOWED = [ "read_file", "list_files", "search_files", - "ask_clarification", "create_plan", "mark_step_complete", + "ask_clarification", "create_plan", "get_plan", ]; const DEFAULT_MODES = [ { diff --git a/services/api/src/routes/chat/event-processor.ts b/services/api/src/routes/chat/event-processor.ts index c2009f2a..11fb79cd 100644 --- a/services/api/src/routes/chat/event-processor.ts +++ b/services/api/src/routes/chat/event-processor.ts @@ -1,3 +1,4 @@ +import {classifyProviderError} from "../../ai/provider-error.js"; /** * processEvent callback factory and helpers for routing SDK events to SSE. */ @@ -62,7 +63,8 @@ export function createProcessEvent( // identical so auto-continue tool events still increment the trace // counter.) const toolEvt = recordToolEventForTrace(state, event as Record, recordAssistantToolCall); - if (toolEvt.handled && toolEvt.phase === "start" && toolEvt.toolName) { + if (toolEvt.suppressDisplay) return; + if (toolEvt.handled && toolEvt.phase === "start" && toolEvt.toolName && toolEvt.toolName !== "report_task_status") { state.pendingToolNames.push(toolEvt.toolName); state.lastToolName = toolEvt.toolName; state.friendlyLastTool = friendlyToolMessage(toolEvt.toolName, toolEvt.toolArgs ?? {}) ?? toolEvt.toolName; @@ -80,8 +82,11 @@ export function createProcessEvent( } state.lastCapturedMsgId = deltaMessageId; state.msgIdDeltaStart = state.assistantContent.length; + state.currentMessageTextLength = 0; state.lastMsgIdSepEmitted = true; } + const rawText = String(evtData?.deltaContent ?? evtData?.content ?? evtData?.delta ?? ""); + state.currentMessageTextLength += rawText.length; } // assistant.message catch-up (BUG-119) @@ -95,12 +100,14 @@ export function createProcessEvent( // later only if recovery fails (see send-handler.ts). // EXCEPTION: Rate limit errors are sent immediately — they are not // transient and the user needs to know why generation stopped. - const sseData = mapEventToSSE(event); + const sseData = mapEventToSSE(event, { preserveThinkingMarkers: true }); if (sseData) { if (evtType === "session.error" && sseData.type === "error") { const errMsg = typeof sseData.data === "string" ? sseData.data : "Unknown error"; - const isRateLimit = errMsg.toLowerCase().includes("rate limit") || errMsg.includes("429") || errMsg.toLowerCase().includes("quota"); + state.deferredErrorCode=classifyProviderError(evtData ?? errMsg); + const isRateLimit = ["RATE_LIMIT","QUOTA","AUTH"].includes(state.deferredErrorCode); if (isRateLimit) { + if(state.runOutcome!=="aborted") state.runOutcome="error"; // Rate limit errors are non-recoverable — surface immediately routeSseEvent(stream, state, channelRouter, sseData, evtData, projectId, userId, messageId); } else { @@ -145,55 +152,21 @@ function handleAssistantMessageCatchUp( } state.lastCapturedMsgId = msgId; state.msgIdDeltaStart = state.assistantContent.length; + state.currentMessageTextLength = 0; } // Reset the flag after catch-up so the next transition works fresh state.lastMsgIdSepEmitted = false; if (!content) return; - const sanitizedContent = sanitizeText(content); - const deltasSoFar = state.assistantContent.slice(state.msgIdDeltaStart); - // Account for text we classified as thinking via the leading-text buffer. - // The SDK's assistant.message includes ALL text (reasoning + content), but - // our delta handler split it into assistantContent and assistantThinking. - // Without this, the catch-up would see thinking text as "missing" and - // re-emit it as text_delta, leaking reasoning into the chat. - const totalProcessed = deltasSoFar.length + state.assistantThinking.length; - if (sanitizedContent.length > totalProcessed) { - const missing = sanitizedContent.slice(totalProcessed); - console.log(`[Chat][${projectId.slice(0, 8)}] catch-up: msg=${sanitizedContent.length} processed=${totalProcessed} (content=${deltasSoFar.length} thinking=${state.assistantThinking.length}) missing=${missing.length}`); - let visibleText = ""; - for (const chunk of channelRouter.process(missing)) { - if (!chunk.content) continue; - if (chunk.type === "text") { - visibleText += chunk.content; - stream.writeSSE({ data: JSON.stringify({ type: "text_delta", data: chunk.content }) }).catch(() => {}); - } else if (chunk.type === "thinking") { - state.assistantThinking += chunk.content; - stream.writeSSE({ data: JSON.stringify({ type: "thinking", data: stripServerPaths(chunk.content) }) }).catch(() => {}); - } else if (chunk.type === "tool") { - state.sawToolDelta = true; - stream.writeSSE({ data: JSON.stringify({ type: "tool_delta", data: chunk.content }) }).catch(() => {}); - } - } - if (visibleText) { - state.assistantContent = state.assistantContent.slice(0, state.msgIdDeltaStart) + deltasSoFar + visibleText; - } - } else if (!totalProcessed && !state.assistantContent) { - let visibleText = ""; - for (const chunk of channelRouter.process(sanitizedContent)) { - if (!chunk.content) continue; - if (chunk.type === "text") { - visibleText += chunk.content; - stream.writeSSE({ data: JSON.stringify({ type: "text_delta", data: chunk.content }) }).catch(() => {}); - } else if (chunk.type === "thinking") { - state.assistantThinking += chunk.content; - stream.writeSSE({ data: JSON.stringify({ type: "thinking", data: stripServerPaths(chunk.content) }) }).catch(() => {}); - } else if (chunk.type === "tool") { - state.sawToolDelta = true; - stream.writeSSE({ data: JSON.stringify({ type: "tool_delta", data: chunk.content }) }).catch(() => {}); - } - } - state.assistantContent = visibleText; + // Count only deltas from THIS message, including markers. Session-wide + // thinking length can hide a complete final answer from a non-streaming + // provider. Slice raw text BEFORE sanitizing: jargon replacements can change + // lengths differently for partial deltas and complete messages. + const missing = sanitizeText(content.slice(state.currentMessageTextLength), { preserveThinkingMarkers: true }); + if (missing) { + routeSseEvent(stream, state, channelRouter, { type: "text_delta", data: missing }, evtData, projectId, userId, messageId); + state.currentMessageTextLength = content.length; } + } function routeSseEvent( @@ -226,17 +199,17 @@ function routeSseEvent( sql`UPDATE ai_messages SET had_tool_calls = true WHERE id = ${state.assistantMessageId} AND had_tool_calls = false`.catch(() => {}); } const resultData = sseData.data as Record; - if (!resultData?.name) { - const tcId = evtData?.toolCallId as string | undefined; - const mappedName = tcId ? state.toolCallIdMap.get(tcId) : undefined; - if (mappedName) { - resultData.name = mappedName; - state.toolCallIdMap.delete(tcId!); - const idx = state.pendingToolNames.indexOf(mappedName); - if (idx !== -1) state.pendingToolNames.splice(idx, 1); - } else if (state.pendingToolNames.length > 0) { - resultData.name = state.pendingToolNames.shift(); - } + const tcId = evtData?.toolCallId as string | undefined; + resultData.name ??= tcId ? state.toolCallIdMap.get(tcId) : undefined; + if (!resultData.name) resultData.name = state.pendingToolNames[0]; + const pendingIndex = state.pendingToolNames.indexOf(String(resultData.name)); + if (pendingIndex !== -1) state.pendingToolNames.splice(pendingIndex, 1); + // Keep the ID/name mapping for duplicate terminal mirrors and history. + const observation = state.assistantToolCalls.find((r) => tcId && r.callId === tcId); + if (observation) { + resultData.toolCallId = observation.callId; + resultData.args ??= observation.arguments; + resultData.success = observation.status === "completed"; } // Merge any artifacts stashed by tool-callbacks.onToolEnd. CF Tunnel can // drop the dedicated `artifact` / `mcp_ui_resource` SSE events, so the diff --git a/services/api/src/routes/chat/execution-state.test.ts b/services/api/src/routes/chat/execution-state.test.ts new file mode 100644 index 00000000..527a3587 --- /dev/null +++ b/services/api/src/routes/chat/execution-state.test.ts @@ -0,0 +1,388 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { createInitialState } from "./types.js"; +import { + startTool, + finishTool, + cycleFingerprint, + runOutcome, +} from "./execution-state.js"; +import { + handleAutoContinue, + handleEmptyResponseRetry, +} from "./stream-recovery.js"; +import { recordToolEventForTrace } from "./tool-event-bookkeeping.js"; +import { + extractPlanFromResponse, + parsePlanSteps, +} from "../../ai/plan-parser.js"; +import { classifyProviderError } from "../../ai/provider-error.js"; +import { buildToolActionsFromCalls } from "../../ai/tool-messages.js"; +const stream = () => { + const events: any[] = []; + return { + events, + async writeSSE(e: any) { + events.push(JSON.parse(e.data)); + }, + }; +}; +function report( + s: ReturnType, + status: string, + intent = "change", +) { + startTool(s, "report_task_status", { intent, status }); + finishTool(s, "report_task_status", { intent, status }, { success: true }); +} +function read( + s: ReturnType, + path: string, + result = "same", +) { + startTool(s, "read_file", { path }); + finishTool(s, "read_file", { path }, { success: true, content: result }); +} +test("distinct invocations, arguments, cycles and results cannot collapse into tool names", () => { + const s = createInitialState(); + read(s, "a"); + const first = cycleFingerprint(s, 0); + s.recoveryCycle = 1; + read(s, "b"); + assert.notEqual(first, cycleFingerprint(s, 1)); + s.recoveryCycle = 2; + read(s, "a", "changed"); + assert.notEqual(first, cycleFingerprint(s, 2)); + assert.equal(s.assistantToolCalls.length, 3); +}); +test("SDK IDs deduplicate mirrored starts and correlate nameless ends", () => { + const s = createInitialState(); + const event = { + type: "tool.execution_start", + data: { + toolCallId: "one", + toolName: "read_file", + arguments: { path: "a" }, + }, + }; + recordToolEventForTrace(s, event, () => {}); + recordToolEventForTrace(s, event, () => {}); + recordToolEventForTrace( + s, + { + type: "tool.execution_complete", + data: { toolCallId: "one", success: false, result: { error: "bad" } }, + }, + () => {}, + ); + assert.equal(s.assistantToolCalls.length, 1); + assert.equal(s.assistantToolCalls[0].status, "failed"); +}); +test("Chinese, English, negations and filenames never determine recovery policy", async () => { + for (const text of [ + "继续", + "Continue", + "开始构建!以下是已确认的计划", + "Do not build; only explain", + "Read src/build.ts", + ]) { + const s = createInitialState(); + read(s, "a"); + report(s, "completed", "read_only"); + let calls = 0; + await handleAutoContinue( + stream() as any, + s, + { + async sendMessage() { + calls++; + }, + } as any, + "s", + "p", + "agent", + () => {}, + text, + ); + assert.equal(calls, 0, text); + } +}); +test("Chinese unfinished request continues over 21 distinct paths and ends on explicit report", async () => { + const s = createInitialState(); + for (let i = 0; i < 20; i++) read(s, `part${i}`); + report(s, "in_progress"); + let calls = 0; + await handleAutoContinue( + stream() as any, + s, + { + async sendMessage() { + calls++; + read(s, "new"); + report(s, "completed"); + }, + } as any, + "s", + "p", + "agent", + () => {}, + "继续", + ); + assert.equal(calls, 1); + assert.equal(runOutcome(s), "completed"); +}); +test("failed file write never suppresses unfinished-task recovery; database completion never forces file writes", async () => { + const s = createInitialState(); + startTool(s, "edit_file", { path: "a" }); + finishTool(s, "edit_file", { path: "a" }, { success: false }); + report(s, "in_progress"); + let calls = 0; + await handleAutoContinue( + stream() as any, + s, + { + async sendMessage() { + calls++; + report(s, "completed"); + }, + } as any, + "s", + "p", + "agent", + () => {}, + ); + assert.equal(calls, 1); + const d = createInitialState(); + startTool(d, "data_migrate", {}); + finishTool(d, "data_migrate", {}, { success: true }); + report(d, "completed"); + await handleAutoContinue( + stream() as any, + d, + { + async sendMessage() { + assert.fail("unwanted mutation nudge"); + }, + } as any, + "s", + "p", + "agent", + () => {}, + ); +}); +test("same observations stop after bounded repetitions; changed results are progress", async () => { + const s = createInitialState(); + read(s, "a"); + report(s, "in_progress"); + let calls = 0; + await handleAutoContinue( + stream() as any, + s, + { + async sendMessage() { + calls++; + read(s, "a"); + report(s, "in_progress"); + }, + } as any, + "s", + "p", + "agent", + () => {}, + ); + assert.equal(calls, 2); + assert.equal(runOutcome(s), "stalled"); +}); +test("waiting, provider failure and missing status remain distinct terminal outcomes", async () => { + for (const kind of ["waiting", "error", "missing"]) { + const s = createInitialState(); + read(s, "a"); + report(s, "in_progress"); + await handleAutoContinue( + stream() as any, + s, + { + async sendMessage() { + if (kind === "error") throw Error("offline"); + if (kind === "waiting") report(s, "waiting_for_input"); + }, + } as any, + "s", + "p", + "agent", + () => {}, + ); + assert.equal(runOutcome(s), kind === "missing" ? "stalled" : kind); + } +}); +test("provider codes precede translated text; Chinese rate limits do not retry", async () => { + assert.equal( + classifyProviderError({ statusCode: 503, message: "rate limit" }), + "SERVER", + ); + assert.equal( + classifyProviderError({ statusCode: 429, message: "错误" }), + "RATE_LIMIT", + ); + assert.equal(classifyProviderError("请求超时"), "TIMEOUT"); + for (const message of ["请求过于频繁", "余额不足", "rate limit exceeded"]) { + const s = createInitialState(); + s.deferredError = message; + await handleEmptyResponseRetry( + stream() as any, + s, + { + async sendMessage() { + assert.fail("retry must be suppressed"); + }, + } as any, + "s", + "p", + "x", + [], + ); + assert.equal(runOutcome(s), "error"); + } +}); +test("plan extraction is language independent; prose sections are not steps", () => { + for (const p of [ + "# 计划\n## 步骤一:新增页面", + "# Plan\n## Step 1: Add page", + ]) { + assert.ok(extractPlanFromResponse(p)); + assert.equal(parsePlanSteps(p).length, 1); + } + assert.deepEqual(parsePlanSteps("## 背景\n## 验收标准"), []); + assert.equal(extractPlanFromResponse("x".repeat(300)), null); +}); +test("history never invents completion and names plan operations correctly", () => { + const r = buildToolActionsFromCalls( + [ + { name: "create_plan" }, + { name: "edit_file", status: "failed" }, + { name: "edit_file", status: "completed" }, + ], + "m", + ); + assert.equal(r[0]!.description, "Creating plan"); + assert.equal(r[0]!.status, "unknown"); + assert.equal(r[1]!.status, "failed"); + assert.equal(r[2]!.status, "completed"); +}); + +test("recovery displays complete-only SDK messages and preserves split reasoning channels", async () => { + const s = createInitialState(); + read(s, "a"); + report(s, "in_progress"); + await handleAutoContinue( + stream() as any, + s, + { + async sendMessage(_id: any, _msg: any, _files: any, cb: any) { + cb({ type: "assistant.message", data: { content: "完成了" } }); + report(s, "completed"); + }, + } as any, + "s", + "p", + "agent", + () => {}, + ); + assert.equal(s.assistantContent, "完成了"); +}); +test("partial failed retries enter error, record tools, and cannot overwrite cancellation", async () => { + const s = createInitialState(); + await handleEmptyResponseRetry( + stream() as any, + s, + { + async sendMessage(_id: any, _msg: any, _files: any, cb: any) { + cb({ + type: "assistant.message_delta", + data: { deltaContent: "读取中" }, + }); + cb({ + type: "tool.execution_start", + data: { + toolName: "read_file", + toolCallId: "one", + arguments: { path: "a" }, + }, + }); + cb({ + type: "tool.execution_complete", + data: { + toolName: "read_file", + toolCallId: "one", + success: false, + result: { error: "Denied" }, + }, + }); + cb({ type: "session.error", data: { message: "连接断开" } }); + }, + } as any, + "s", + "p", + "继续", + [], + ); + assert.equal(runOutcome(s), "error"); + assert.equal(s.assistantToolCalls[0].status, "failed"); + const cancelled = createInitialState(); + read(cancelled, "a"); + report(cancelled, "in_progress"); + await handleAutoContinue( + stream() as any, + cancelled, + { + async sendMessage(_id: any, _msg: any, _files: any, cb: any) { + cancelled.runOutcome = "aborted"; + cb({ type: "session.error", data: { message: "Request aborted" } }); + }, + } as any, + "s", + "p", + "agent", + () => {}, + ); + assert.equal(runOutcome(cancelled), "aborted"); +}); +test("inner MCP failure agrees in live SSE and history; incomplete mirrors cannot erase it", async () => { + const { mapEventToSSE } = await import("../../ai/sse-mapper.js"); + const s = createInitialState(); + startTool(s, "edit_file", { path: "a" }, "id"); + const e = { + type: "tool.execution_complete", + data: { + toolName: "edit_file", + toolCallId: "id", + success: true, + result: { textResultForLlm: '{"success":false,"error":"Denied"}' }, + }, + }; + recordToolEventForTrace(s, e, () => {}); + assert.equal(s.assistantToolCalls[0].status, "failed"); + assert.equal((mapEventToSSE(e)?.data as any).success, false); + finishTool(s, "edit_file", undefined, undefined, true, "id"); + assert.equal(s.assistantToolCalls[0].status, "failed"); +}); + +test("automatic preview repair requires change intent and stops for waiting or cancellation", async () => { + const { mayAutoRepair } = await import("./execution-state.js"); + const s = createInitialState(); + report(s, "completed", "read_only"); + assert.equal(mayAutoRepair(s), false); + report(s, "completed", "change"); + assert.equal(mayAutoRepair(s), true); + s.awaitingIntegrationConnect = true; + assert.equal(mayAutoRepair(s), false); + s.awaitingIntegrationConnect = false; + s.runOutcome = "aborted"; + assert.equal(mayAutoRepair(s), false); +}); + +test("history projections replace old fabricated card status without modifying stored rows",async()=>{ + const {projectHistoryToolActions}=await import("../../ai/tool-messages.js"); + const row={id:"legacy",tool_calls:[{name:"create_plan"}],tool_actions:[{status:"completed",description:"Creating file"}]}; + const view=projectHistoryToolActions(row);assert.equal(view.tool_actions[0]!.status,"unknown");assert.equal(view.tool_actions[0]!.description,"Creating plan");assert.equal(row.tool_actions[0]!.status,"completed"); +}); diff --git a/services/api/src/routes/chat/execution-state.ts b/services/api/src/routes/chat/execution-state.ts new file mode 100644 index 00000000..1a776ac9 --- /dev/null +++ b/services/api/src/routes/chat/execution-state.ts @@ -0,0 +1,165 @@ +import { toolSucceeded } from "../../ai/tool-outcome.js"; +export { toolSucceeded } from "../../ai/tool-outcome.js"; +import { createHash, randomUUID } from "node:crypto"; +import type { ChatStreamState } from "./types.js"; + +export type RunOutcome = + | "completed" + | "stalled" + | "error" + | "aborted" + | "waiting"; +export type TaskReport = { + intent: "read_only" | "change"; + status: "in_progress" | "completed" | "waiting_for_input"; +}; +export interface ToolObservation { + name: string; + arguments?: Record; + callId?: string; + cycle: number; + status: "running" | "completed" | "failed" | "unknown"; + resultHash?: string; +} +function canonical(value: unknown): string { + if (Array.isArray(value)) return `[${value.map(canonical).join(",")}]`; + if (value && typeof value === "object") + return `{${Object.entries(value) + .sort(([a], [b]) => a.localeCompare(b)) + .map(([k, v]) => `${JSON.stringify(k)}:${canonical(v)}`) + .join(",")}}`; + return JSON.stringify(value) ?? "null"; +} +export function parseTaskReport( + value: Record, +): TaskReport | null { + if ( + !["read_only", "change"].includes(String(value.intent)) || + !["in_progress", "completed", "waiting_for_input"].includes( + String(value.status), + ) + ) + return null; + return { + intent: value.intent as TaskReport["intent"], + status: value.status as TaskReport["status"], + }; +} +/** SDK IDs are authoritative; hook mirrors without IDs merge only into a pending invocation. */ +export function startTool( + state: ChatStreamState, + name: string, + args?: unknown, + callId?: string, +): ToolObservation { + const records = (state.assistantToolCalls ??= []) as ToolObservation[]; + const argumentsValue = + args && typeof args === "object" + ? (args as Record) + : undefined; + let record = callId ? records.find((r) => r.callId === callId) : undefined; + record ??= records.find( + (r) => + r.status === "running" && + r.cycle === (state.recoveryCycle ?? 0) && + r.name === name && + canonical(r.arguments) === canonical(argumentsValue) && + (!callId || !r.callId || r.callId.startsWith("hook-")), + ); + if (!record) { + record = { + name, + arguments: argumentsValue, + callId: callId ?? `hook-${randomUUID()}`, + cycle: state.recoveryCycle ?? 0, + status: "running", + }; + records.push(record); + } else if (callId) record.callId = callId; + state.hadToolCalls = true; + if ( + name !== "report_task_status" && + state.taskReport?.status === "completed" + ) { + state.taskReport = { ...state.taskReport, status: "in_progress" }; + } + return record; +} +export function finishTool( + state: ChatStreamState, + name: string, + args: unknown, + result: unknown, + success?: boolean, + callId?: string, +) { + const records = (state.assistantToolCalls ??= []) as ToolObservation[]; + let record = callId ? records.find((r) => r.callId === callId) : undefined; + record ??= records.find( + (r) => + r.status === "running" && + r.name === name && + (args == null || canonical(r.arguments) === canonical(args)), + ); + // End events without an ID/args may mirror the hook result. Do not invent another invocation. + record ??= [...records] + .reverse() + .find((r) => r.name === name && r.cycle === (state.recoveryCycle ?? 0)); + record ??= startTool(state, name, args, callId); + // A later incomplete mirror cannot erase an observed failure. A real retry has a new call ID. + record.status = + record.status === "failed" || !toolSucceeded(result, success) + ? "failed" + : "completed"; + record.resultHash = createHash("sha256") + .update(canonical(result)) + .digest("hex"); + if (name === "report_task_status" && record.status === "completed") { + const report = parseTaskReport(record.arguments ?? {}); + if (report) state.taskReport = report; + } + return record; +} +/** Compare complete arguments AND observations from one SDK cycle, never a rolling historical window. */ +export function cycleFingerprint( + state: ChatStreamState, + cycle: number, +): string { + return (state.assistantToolCalls as ToolObservation[]) + .filter((r) => r.cycle === cycle && r.name !== "report_task_status") + .map((r) => + canonical({ + name: r.name, + args: r.arguments, + status: r.status, + result: r.resultHash, + }), + ) + .sort() + .join("\n"); +} +export function runOutcome(state: ChatStreamState): RunOutcome { + if (state.runOutcome) return state.runOutcome; + if (state.assistantToolCalls.some((r) => r.status === "running")) + return "stalled"; + if ( + state.awaitingMcpWidget || + state.awaitingSupabaseProvision || + state.awaitingIntegrationConnect || + state.taskReport?.status === "waiting_for_input" + ) + return "waiting"; + if (state.taskReport?.status === "in_progress") return "stalled"; + return "completed"; +} + +/** Automatic repair must never turn a read-only or waiting task into a write. */ +export function mayAutoRepair(state: ChatStreamState): boolean { + return ( + !state.runOutcome && + !state.deferredError && + state.taskReport?.intent === "change" && + state.taskReport.status === "completed" && + runOutcome(state) === "completed" + ); +} diff --git a/services/api/src/routes/chat/final-response.integration.test.ts b/services/api/src/routes/chat/final-response.integration.test.ts new file mode 100644 index 00000000..2852f767 --- /dev/null +++ b/services/api/src/routes/chat/final-response.integration.test.ts @@ -0,0 +1,207 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { createInitialState } from "./types.js"; +import { createProcessEvent } from "./event-processor.js"; +import { ChannelTokenRouter } from "../../ai/sse-mapper.js"; +import { finalizeLeadingResponse } from "./final-response.js"; + +function fixture() { + const state = createInitialState(); + const frames: Array<{ type: string; data: unknown }> = []; + const stream = { + writeSSE: async ({ data }: { data: string }) => { + frames.push(JSON.parse(data)); + }, + } as unknown as import("hono/streaming").SSEStreamingApi; + const process = createProcessEvent( + stream, + state, + new ChannelTokenRouter(), + "regression-test", + "test-user", + "test-message", + "build", + () => {}, + () => {}, + () => "test-session", + ); + const emit = (type: string, data: Record) => + process({ type, data } as Parameters[0]); + return { state, frames, emit }; +} + +test("SDK tool round-trip and final deltas preserve answer and native reasoning", () => { + const { state, frames, emit } = fixture(); + emit("assistant.reasoning_delta", { deltaContent: "Native reasoning." }); + emit("assistant.message_delta", { + messageId: "m1", + deltaContent: "I will inspect the files.", + }); + emit("tool.execution_start", { + toolName: "read_file", + toolCallId: "t1", + arguments: { path: "package.json" }, + }); + emit("tool.execution_complete", { + toolName: "read_file", + toolCallId: "t1", + success: true, + }); + emit("assistant.message_delta", { + messageId: "m2", + deltaContent: "这是一个 React 项目。", + }); + emit("assistant.message", { + messageId: "m2", + content: "这是一个 React 项目。", + }); + emit("session.idle", {}); + assert.equal(state.hadToolCalls, true); + assert.ok(frames.some((frame) => frame.type === "tool_result")); + assert.equal(finalizeLeadingResponse(state), "这是一个 React 项目。"); + assert.equal(state.assistantContent, "这是一个 React 项目。"); + assert.equal( + state.assistantThinking, + "Native reasoning.I will inspect the files.", + ); +}); + +test("tagged reasoning after a tool is not promoted with the final answer", () => { + const { state, emit } = fixture(); + emit("tool.execution_complete", { toolName: "read_file", success: true }); + emit("assistant.message_delta", { + messageId: "m1", + deltaContent: "Internal reasoning.Final answer.", + }); + emit("assistant.message", { + messageId: "m1", + content: "Internal reasoning.Final answer.", + }); + assert.equal(finalizeLeadingResponse(state), "Final answer."); + assert.equal(state.assistantThinking, "Internal reasoning."); +}); + +test("text followed by another tool call is not a final answer", () => { + const { state, emit } = fixture(); + emit("assistant.message_delta", { + messageId: "m1", + deltaContent: "I need another file.", + }); + emit("tool.execution_start", { toolName: "read_file", toolCallId: "t1" }); + emit("tool.execution_complete", { + toolName: "read_file", + toolCallId: "t1", + success: true, + }); + assert.equal(finalizeLeadingResponse(state), ""); + assert.equal(state.assistantContent, ""); + assert.equal(state.assistantThinking, "I need another file."); +}); + +test("complete-only final SDK message is not hidden by prior reasoning length", () => { + const { state, emit } = fixture(); + emit("assistant.reasoning_delta", { + deltaContent: "Long reasoning from an earlier tool round. ".repeat(20), + }); + emit("tool.execution_complete", { toolName: "read_file", success: true }); + emit("assistant.message", { messageId: "final", content: "Final answer." }); + assert.equal(finalizeLeadingResponse(state), "Final answer."); + assert.equal(state.assistantContent, "Final answer."); +}); + +test("thinking markers split across deltas stay out of the answer", () => { + const { state, emit } = fixture(); + for (const deltaContent of ["Reasoning", "Answer."]) { + emit("assistant.message_delta", { messageId: "final", deltaContent }); + } + emit("assistant.message", { + messageId: "final", + content: "ReasoningAnswer.", + }); + assert.equal(finalizeLeadingResponse(state), "Answer."); + assert.equal(state.assistantContent, "Answer."); + assert.equal(state.assistantThinking, "Reasoning"); +}); + +test("catch-up uses raw offsets when sanitization expands split jargon", () => { + const { state, emit } = fixture(); + const content = "Read package.json. Final answer."; + for (const deltaContent of ["Read pack", "age.json. Final answer."]) { + emit("assistant.message_delta", { messageId: "final", deltaContent }); + } + emit("assistant.message", { messageId: "final", content }); + assert.equal(finalizeLeadingResponse(state), content); +}); + +test("catch-up sanitizes only a genuinely missing raw suffix", () => { + const { state, emit } = fixture(); + emit("assistant.message_delta", { + messageId: "final", + deltaContent: "Read package.json.", + }); + emit("assistant.message", { + messageId: "final", + content: "Read package.json. Final answer.", + }); + assert.equal( + finalizeLeadingResponse(state), + "Read project configuration. Final answer.", + ); +}); + +test("named SDK results clear pending completion and preserve stable invocation IDs", () => { + const {state,frames,emit}=fixture(); + emit("tool.execution_start",{toolName:"read_file",toolCallId:"invocation",arguments:{path:"a"}}); + emit("tool.execution_complete",{toolName:"read_file",toolCallId:"invocation",success:true,result:{content:"ok"}}); + assert.deepEqual(state.pendingToolNames,[]); + const result=frames.find(f=>f.type==="tool_result")?.data as Record; + assert.equal(result.toolCallId,"invocation");assert.equal(result.success,true); +}); + +test("SDK and empty hook mirrors produce one completed invocation and no metadata card", async () => { + const { createToolProgressCallbacks } = await import("./tool-callbacks.js"); + const { runOutcome } = await import("./execution-state.js"); + const { state, frames, emit } = fixture(); + const stream = { writeSSE: async ({data}: {data: string}) => { frames.push(JSON.parse(data)); } } as unknown as import("hono/streaming").SSEStreamingApi; + const hooks = createToolProgressCallbacks(stream, state, null, () => {}); + for (const [name, args, id] of [ + ["read_file", {path: "package.json"}, "read"], + ["report_task_status", {intent: "read_only", status: "completed"}, "report"], + ] as const) { + emit("tool.execution_start", {toolName: name, arguments: args, toolCallId: id}); + hooks.onToolStart(name, {}); + await hooks.onToolEnd(name, {}, {success: true}); + emit("tool.execution_complete", {toolCallId: id, success: true, result: {success: true}}); + } + assert.equal(state.assistantToolCalls.length, 2); + assert.ok(state.assistantToolCalls.every(call => call.status === "completed")); + assert.equal(state.pendingToolNames.length, 0); + assert.equal(runOutcome(state), "completed"); + assert.equal(frames.filter(frame => frame.type === "tool_call").length, 1); + assert.equal(frames.filter(frame => frame.type === "tool_result").length, 1); +}); + +test("real external dispatch ACKs preserve final answer before hidden task report",()=>{ + const {state,frames,emit}=fixture();let count=0; + state.traceCollector={onSdkEvent(){},onToolStart(){count++;},onToolEnd(){},onSseEmit(){},onThinkingDelta(){},onTextDelta(){}} as any; + function call(id:string,name:string,args:Record){ + emit("tool.execution_start",{toolCallId:id,toolName:name,arguments:args}); + emit("external_tool.requested",{requestId:`request-${id}`,sessionId:"session",toolCallId:id,toolName:name,arguments:args}); + emit("external_tool.completed",{requestId:`request-${id}`}); + emit("tool.execution_complete",{toolCallId:id,success:true,result:{content:JSON.stringify({success:true,...args}),detailedContent:JSON.stringify({success:true,...args})}}); + } + call("report1","report_task_status",{intent:"read_only",status:"in_progress"}); + call("read1","read_file",{path:"package.json"}); + call("read2","read_file",{path:"src/App.tsx"}); + const answer="当前应用使用 React、TypeScript 和 Vite。入口为 src/App.tsx,验收标记 ACK-ANSWER-OK。"; + emit("assistant.message",{messageId:"answer359",content:answer,toolRequests:[{toolCallId:"report2",name:"report_task_status"}]}); + call("report2","report_task_status",{intent:"read_only",status:"completed"}); + emit("assistant.message",{messageId:"answer382",content:"只读检查已完成。"}); + emit("session.idle",{});finalizeLeadingResponse(state); + assert.ok(state.assistantContent.includes(answer));assert.ok(state.assistantContent.includes("ACK-ANSWER-OK")); + assert.equal(count,4);assert.equal(state.assistantToolCalls.length,4); + assert.equal(frames.filter(f=>f.type==="tool_call").length,2); + assert.equal(frames.filter(f=>f.type==="tool_result").length,2); + assert.equal(state.pendingToolNames.length,0); + assert.ok(frames.filter(f=>f.type==="tool_result").every(f=>(f.data as any).toolCallId)); +}); diff --git a/services/api/src/routes/chat/final-response.test.ts b/services/api/src/routes/chat/final-response.test.ts new file mode 100644 index 00000000..d31f2e97 --- /dev/null +++ b/services/api/src/routes/chat/final-response.test.ts @@ -0,0 +1,73 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { createInitialState } from "./types.js"; +import { finalizeLeadingResponse } from "./final-response.js"; + +const answer = "这是一个空白 React 项目。可以添加业务页面。没有修改文件。"; + +test("final answer after read-only tools becomes persisted content", () => { + const state = createInitialState(); + state.hadToolCalls = true; + state.assistantThinking = "Earlier tool planning.\n" + answer; + state.leadingTextBuffer = answer; + assert.equal(finalizeLeadingResponse(state), answer); + assert.equal(state.assistantContent, answer); + assert.equal(state.assistantThinking, "Earlier tool planning.\n"); + assert.equal(state.leadingTextBuffer, ""); + assert.equal( + finalizeLeadingResponse(state), + "", + "finalization is idempotent", + ); +}); + +test("direct answers still work without tools", () => { + const state = createInitialState(); + state.assistantThinking = state.leadingTextBuffer = answer; + assert.equal(finalizeLeadingResponse(state), answer); + assert.equal(state.assistantThinking, ""); +}); + +test("explicit reasoning without a plain-text final segment stays private", () => { + const state = createInitialState(); + state.hadToolCalls = true; + state.assistantThinking = "Native reasoning only"; + assert.equal(finalizeLeadingResponse(state), ""); + assert.equal(state.assistantContent, ""); + assert.equal(state.assistantThinking, "Native reasoning only"); +}); + +test("a deferred stream error does not promote an unfinished segment", () => { + const state = createInitialState(); + state.assistantThinking = state.leadingTextBuffer = "I still need to inspect"; + state.deferredError = "Request timed out"; + assert.equal(finalizeLeadingResponse(state), ""); + assert.equal(state.assistantContent, ""); + assert.equal(state.assistantThinking, "I still need to inspect"); +}); + +test("later native reasoning is preserved rather than removed as a suffix", () => { + const state = createInitialState(); + state.hadToolCalls = true; + state.leadingTextBuffer = answer; + state.assistantThinking = + "Earlier reasoning. " + answer + " Later reasoning."; + assert.equal(finalizeLeadingResponse(state), answer); + assert.equal(state.assistantThinking, "Earlier reasoning. Later reasoning."); +}); + +test("previous visible content is retained", () => { + const state = createInitialState(); + state.assistantContent = "Existing answer. "; + state.assistantThinking = state.leadingTextBuffer = answer; + assert.equal(finalizeLeadingResponse(state), answer); + assert.equal(state.assistantContent, "Existing answer. " + answer); +}); + +test("unmatched/interleaved reasoning is conservatively left untouched", () => { + const state = createInitialState(); + state.leadingTextBuffer = "Hello world"; + state.assistantThinking = "Hello native reasoning world"; + assert.equal(finalizeLeadingResponse(state), ""); + assert.equal(state.assistantContent, ""); +}); diff --git a/services/api/src/routes/chat/final-response.ts b/services/api/src/routes/chat/final-response.ts new file mode 100644 index 00000000..1669af3b --- /dev/null +++ b/services/api/src/routes/chat/final-response.ts @@ -0,0 +1,27 @@ +import type { ChatStreamState } from "./types.js"; + +/** + * Ordinary text is provisionally routed to thinking while waiting to see + * whether another tool call follows. A successfully completed final segment + * is the answer, even when earlier segments invoked tools. Explicit reasoning + * events and text confirmed by a subsequent tool call are never promoted. + */ +export function finalizeLeadingResponse(state: ChatStreamState): string { + const buffered = state.leadingTextBuffer; + state.leadingTextBuffer = ""; + state.leadingTextFlushed = true; + if (!buffered || state.deferredError) return ""; + + // Remove only the exact provisional segment, not an arbitrary suffix: a + // native reasoning event may have arrived after the ordinary text. + const index = state.assistantThinking.lastIndexOf(buffered); + if (index < 0) return ""; + const visible = buffered.replace(/[\s\S]*?<\/think>\s*/gi, "").trim(); + if (!visible) return ""; + + state.assistantThinking = + state.assistantThinking.slice(0, index) + + state.assistantThinking.slice(index + buffered.length); + state.assistantContent += visible; + return visible; +} diff --git a/services/api/src/routes/chat/fix-error.ts b/services/api/src/routes/chat/fix-error.ts index 7d41a89a..ffa8ea17 100644 --- a/services/api/src/routes/chat/fix-error.ts +++ b/services/api/src/routes/chat/fix-error.ts @@ -28,7 +28,7 @@ const fixErrorSchema = z.object({ context: z.string().max(4000).optional(), }); -const PLAN_ONLY_TOOLS = new Set(["ask_clarification", "create_plan", "mark_step_complete"]); +const PLAN_ONLY_TOOLS = new Set(["ask_clarification", "create_plan"]); export function registerFixErrorRoute(app: Hono) { app.post( diff --git a/services/api/src/routes/chat/misc-routes.ts b/services/api/src/routes/chat/misc-routes.ts index 7db168e4..6fe73f26 100644 --- a/services/api/src/routes/chat/misc-routes.ts +++ b/services/api/src/routes/chat/misc-routes.ts @@ -1,3 +1,4 @@ +import {projectHistoryToolActions} from "../../ai/tool-messages.js"; /** * Miscellaneous small chat routes: ai-status, traces/live, chat/status, * chat/history, clear chat, abort, models, auth-status. @@ -221,7 +222,7 @@ export function registerMiscRoutes(app: Hono) { FROM ai_messages WHERE session_id = ${dbSession.id} ORDER BY created_at ASC `; - return c.json({ data: messages, hasMore: false }); + return c.json({ data: messages.map(projectHistoryToolActions), hasMore: false }); } // Cursor-based: get newest N messages (or N before cursor) @@ -268,7 +269,7 @@ export function registerMiscRoutes(app: Hono) { hasMore = !!older; } - return c.json({ data: messages, hasMore }); + return c.json({ data: messages.map(projectHistoryToolActions), hasMore }); } catch (err) { console.warn("[Chat] Failed to load history from DB:", err); const sessionId = projectSessions.get(projectId); @@ -276,7 +277,7 @@ export function registerMiscRoutes(app: Hono) { try { const engine = await getCopilotEngine(); const messages = await engine.getSessionMessages(sessionId); - return c.json({ data: messages, hasMore: false }); + return c.json({ data: messages.map(projectHistoryToolActions), hasMore: false }); } catch { return c.json({ data: [], hasMore: false }); } @@ -306,6 +307,7 @@ export function registerMiscRoutes(app: Hono) { // ─── POST /projects/:id/chat/abort ── app.post("/projects/:id/chat/abort", async (c) => { const projectId = c.req.param("id"); + activeRequests.get(projectId)?.cancel?.(); const sessionId = projectSessions.get(projectId) ?? projectSessions.get(`${projectId}:visual-edit`); if (sessionId) { const engine = getCopilotManager().tryGetEngine(projectId); diff --git a/services/api/src/routes/chat/post-processing.ts b/services/api/src/routes/chat/post-processing.ts index 1146a245..7a816591 100644 --- a/services/api/src/routes/chat/post-processing.ts +++ b/services/api/src/routes/chat/post-processing.ts @@ -1,3 +1,4 @@ +import {runOutcome, mayAutoRepair} from "./execution-state.js"; /** * Post-processing after AI streaming completes: auto-fix preview errors, * version control, memory update, thumbnail capture, final DB save, @@ -14,10 +15,12 @@ import { contextManager } from "../../context/manager.js"; import { broadcastToRoom } from "../../ai/yjs-bridge.js"; import { detectPreviewError, buildAutoFixPrompt } from "../../ai/preview-errors.js"; import { extractPlanFromResponse } from "../../ai/plan-parser.js"; -import { mapEventToSSE } from "../../ai/sse-mapper.js"; +import type {RecoveryPipeline} from "./stream-recovery.js"; import { scheduleThumbnailCapture } from "../../ai/thumbnail.js"; import { getCopilotManager } from "../../ai/providers/copilot-manager.js"; import { finalSaveAssistantMessage } from "./message-persistence.js"; +import { getPlan } from "../../ai/plan-store.js"; +import { planToMarkdown } from "../../ai/plan-state.js"; import { activeRequests } from "./session-state.js"; const ctxManager = contextManager(sql); @@ -29,20 +32,24 @@ export async function handleAutoFixPreview( projectId: string, resolvedGithubToken: string | undefined, sessionId: string, + pipeline?: RecoveryPipeline, ): Promise { - if (!state.hadToolCalls || !isProjectScaffolded(projectId)) return; + if (!pipeline || !mayAutoRepair(state) || !isProjectScaffolded(projectId)) return; try { const MAX_FIX_ATTEMPTS = 3; let fixedSuccessfully = false; for (let attempt = 0; attempt < MAX_FIX_ATTEMPTS; attempt++) { + if (!mayAutoRepair(state)) return; await stream.writeSSE({ data: JSON.stringify({ type: "status", data: { phase: "checking", message: "Checking preview for errors..." } }), }); await new Promise((r) => setTimeout(r, 1500)); + if (!mayAutoRepair(state)) return; const previewError = await detectPreviewError(projectId); + if (!mayAutoRepair(state)) return; if (!previewError) { if (attempt > 0) { await stream.writeSSE({ data: JSON.stringify({ type: "status", data: { phase: "fixed", message: "Error fixed successfully" } }) }); @@ -62,16 +69,24 @@ export async function handleAutoFixPreview( try { const fixEngine = await getCopilotManager().getEngine(projectId, resolvedGithubToken); + if (!mayAutoRepair(state)) return; + state.recoveryCycle=(state.recoveryCycle ?? 0)+1; await fixEngine.sendMessage( sessionId, buildAutoFixPrompt(previewError.message), undefined, - (event: import("@github/copilot-sdk").SessionEvent) => { - const sseData = mapEventToSSE(event); - if (sseData) stream.writeSSE({ data: JSON.stringify(sseData) }).catch(() => {}); - }, + pipeline.process, ); + await pipeline.flush(); + if (state.deferredError) { + state.runOutcome ??= "error"; + await stream.writeSSE({data:JSON.stringify({type:"error",data:state.deferredError})}); + return; + } + if (!mayAutoRepair(state)) return; } catch (fixErr) { + state.runOutcome ??= "error"; + await stream.writeSSE({data:JSON.stringify({type:"error",data:fixErr instanceof Error ? fixErr.message : String(fixErr)})}); console.warn(`[Chat] Auto-fix attempt ${attempt + 1} failed:`, fixErr); break; } @@ -80,17 +95,21 @@ export async function handleAutoFixPreview( }); } - if (!fixedSuccessfully) { + if (!fixedSuccessfully && mayAutoRepair(state)) { await new Promise((r) => setTimeout(r, 1500)); + if (!mayAutoRepair(state)) return; const finalError = await detectPreviewError(projectId); + if (!mayAutoRepair(state)) return; if (!finalError) { await stream.writeSSE({ data: JSON.stringify({ type: "status", data: { phase: "fixed", message: "Error fixed successfully" } }) }); await stream.writeSSE({ data: JSON.stringify({ type: "auto_fix_complete", data: { success: true } }) }); } else { + state.runOutcome ??= "error"; await stream.writeSSE({ data: JSON.stringify({ type: "auto_fix_complete", data: { success: false, error: finalError.message } }) }); } } } catch (autoFixErr) { + state.runOutcome ??= "error"; console.warn("[Chat] Auto-fix system failed:", autoFixErr); } } @@ -106,7 +125,7 @@ export async function handleVersionAndMemory( ): Promise { state.traceCollector?.onSseEmit("post_processing_start", { phase: "version_control", hadToolCalls: state.hadToolCalls }); - if (state.hadToolCalls && isProjectScaffolded(projectId)) { + if (state.taskReport?.intent === "change" && isProjectScaffolded(projectId)) { try { const projectPath = getProjectPath(projectId); if (isGitRepo(projectPath)) { @@ -170,7 +189,8 @@ export async function handleFinalCleanup( // In plan mode, save assistant response as .doable/plan.md if (mode === "plan" && state.assistantContent) { try { - const planContent = extractPlanFromResponse(state.assistantContent); + const structuredPlan = await getPlan(projectId); + const planContent = structuredPlan ? planToMarkdown(structuredPlan) : extractPlanFromResponse(state.assistantContent); if (planContent) { await ctxManager.updateContextFile(projectId, "plan.md", planContent); } @@ -193,7 +213,7 @@ export async function handleFinalCleanup( // correlation. The SDK session lifecycle is managed by // projectSessions / ai_sessions, not by the trace context. if (state.assistantMessageId) state.traceCollector.setMessageId(state.assistantMessageId); - state.traceCollector.complete("completed", { + state.traceCollector.complete(runOutcome(state), { promptTokens: usage.promptTokens, completionTokens: usage.completionTokens, thinkingTokens: usage.thinkingTokens, @@ -205,12 +225,12 @@ export async function handleFinalCleanup( } else if (state.traceCollector) { // Same rationale as above — do not wipe session_id on the trace row. if (state.assistantMessageId) state.traceCollector.setMessageId(state.assistantMessageId); - state.traceCollector.complete("completed").catch(() => {}); + state.traceCollector.complete(runOutcome(state)).catch(() => {}); } // Done signal try { - await stream.writeSSE({ data: JSON.stringify({ type: "status", data: { phase: "complete", message: "Done" } }) }); + await stream.writeSSE({ data: JSON.stringify({ type: "status", data: { phase: runOutcome(state) === "completed" ? "complete" : runOutcome(state), message: runOutcome(state) === "completed" ? "Done" : runOutcome(state) === "waiting" ? "Waiting for your input" : "Task stopped before completion" } }) }); } catch { /* stream already closed */ } console.log(`[Chat] Sending [DONE] for ${projectId}`); state.traceCollector?.onStreamEnd("done", state.sseFrameCount); diff --git a/services/api/src/routes/chat/sdk-tool-ledger.test.ts b/services/api/src/routes/chat/sdk-tool-ledger.test.ts new file mode 100644 index 00000000..5f9041e8 --- /dev/null +++ b/services/api/src/routes/chat/sdk-tool-ledger.test.ts @@ -0,0 +1,92 @@ +import {test} from "node:test"; +import assert from "node:assert/strict"; +import {readFileSync} from "node:fs"; +import {createInitialState} from "./types.js"; +import {recordToolEventForTrace} from "./tool-event-bookkeeping.js"; +import {mapEventToSSE,extractToolArguments} from "../../ai/sse-mapper.js"; +import {buildToolActionsFromCalls} from "../../ai/tool-messages.js"; + +test("SDK argument aliases share one decoding contract",()=>{ + const expected={intent:"read_only",status:"completed"}; + for(const envelope of [{arguments:expected},{args:expected},{input:expected},{arguments:{arguments:expected}},{arguments:JSON.stringify(expected)}]) { + assert.deepEqual(extractToolArguments(envelope),expected); + const s=createInitialState(); + recordToolEventForTrace(s,{type:"tool.execution_start",data:{toolName:"report_task_status",toolCallId:"r",...envelope}},()=>{}); + recordToolEventForTrace(s,{type:"tool.execution_complete",data:{toolCallId:"r",success:true,result:{success:true}}},()=>{}); + assert.deepEqual(s.taskReport,expected); + assert.equal(s.assistantToolCalls.length,1); + } + assert.equal(extractToolArguments({toolName:"read_file",toolCallId:"a"}),undefined); +}); + +test("external SDK requests complete with a single ledger row and visible card",()=>{ + const s=createInitialState(); + const start={type:"external_tool.requested",data:{toolName:"mcp_search",toolCallId:"external-1",input:{query:"中文"}}}; + const end={type:"external_tool.completed",data:{toolCallId:"external-1",success:true,result:{content:[{type:"text",text:"found"}]}}}; + recordToolEventForTrace(s,start,()=>{});recordToolEventForTrace(s,end,()=>{}); + assert.equal(s.assistantToolCalls.length,1);assert.equal(s.assistantToolCalls[0].status,"completed"); + assert.equal(mapEventToSSE(start)?.type,"tool_call"); + assert.deepEqual((mapEventToSSE(end)?.data as any).name,"mcp_search"); + assert.equal((mapEventToSSE(end)?.data as any).success,true); +}); + +test("task reports remain structured state without live or historical tool cards",()=>{ + const s=createInitialState(); + const start={type:"tool.execution_start",data:{toolName:"report_task_status",toolCallId:"r",arguments:{intent:"change",status:"completed"}}}; + const end={type:"tool.execution_complete",data:{toolCallId:"r",success:true,result:{success:true}}}; + recordToolEventForTrace(s,start,()=>{});recordToolEventForTrace(s,end,()=>{}); + assert.equal(s.taskReport?.status,"completed");assert.equal(mapEventToSSE(start),null);assert.equal(mapEventToSSE(end),null); + assert.deepEqual(buildToolActionsFromCalls(s.assistantToolCalls,"m"),[]); +}); + +test("external failed output is not rendered as a success",()=>{ + const event={type:"external_tool.completed",data:{toolName:"external_write",toolCallId:"e",success:true,output:{success:false,error:"Denied"}}}; + assert.equal((mapEventToSSE(event)?.data as any).success,false); +}); + +test("supplemental hooks cannot create phantom invocations or duplicate cards",()=>{ + const source=readFileSync(new URL("./tool-callbacks.ts",import.meta.url),"utf8"); + const hooks=source.slice(source.indexOf("export function createToolProgressCallbacks")); + assert.doesNotMatch(hooks,/\b(?:startTool|finishTool|recordAssistantToolCall)\s*\(/); + assert.doesNotMatch(hooks,/traceCollector\?\.onTool(?:Start|End)\s*\(/); + assert.doesNotMatch(hooks,/type:\s*["']tool_(?:call|result)["']/); + for(const event of ["clarification","plan","integration_required","provision_supabase_required","mcp_ui_resource","artifact_ready"]) assert.ok(hooks.includes(`"${event}"`),event); + assert.ok(hooks.includes("persistViewerToProject("));assert.ok(hooks.includes("pushArtifacts(")); +}); + +test("dispatch mirrors count once; request-only acknowledgement is never a result",()=>{ + const s=createInitialState();let starts=0,ends=0; + s.traceCollector={onToolStart(){starts++;},onToolEnd(){ends++;}} as any; + const dispatch={type:"external_tool.requested",data:{requestId:"dispatch",sessionId:"session",toolCallId:"one",toolName:"read_file",arguments:{path:"src/App.tsx"}}}; + assert.equal(recordToolEventForTrace(s,dispatch,()=>{}).phase,"start"); + assert.equal(recordToolEventForTrace(s,{...dispatch,type:"tool.execution_start"},()=>{}).suppressDisplay,true); + const ack={type:"external_tool.completed",data:{requestId:"dispatch"}}; + assert.equal(recordToolEventForTrace(s,ack,()=>{}).suppressDisplay,true); + assert.equal(mapEventToSSE(ack),null);assert.equal(s.assistantToolCalls[0].status,"running"); + const end={type:"tool.execution_complete",data:{toolCallId:"one",success:true,result:{content:'{"success":true}'}}}; + assert.equal(recordToolEventForTrace(s,end,()=>{}).phase,"end"); + assert.equal(recordToolEventForTrace(s,end,()=>{}).suppressDisplay,true); + assert.equal(starts,1);assert.equal(ends,1);assert.equal(s.assistantToolCalls.length,1); +}); + +test("late duplicate read start cannot reopen an already completed task",()=>{ + const s=createInitialState(); + const read={type:"tool.execution_start",data:{toolCallId:"read-late",toolName:"read_file",arguments:{path:"package.json"}}}; + recordToolEventForTrace(s,read,()=>{}); + recordToolEventForTrace(s,{type:"tool.execution_complete",data:{toolCallId:"read-late",success:true,result:{content:"ok"}}},()=>{}); + recordToolEventForTrace(s,{type:"tool.execution_start",data:{toolCallId:"report-last",toolName:"report_task_status",arguments:{intent:"read_only",status:"completed"}}},()=>{}); + recordToolEventForTrace(s,{type:"tool.execution_complete",data:{toolCallId:"report-last",success:true,result:{success:true}}},()=>{}); + assert.equal(s.taskReport?.status,"completed"); + assert.equal(recordToolEventForTrace(s,{...read,type:"external_tool.requested"},()=>{}).suppressDisplay,true); + assert.equal(s.taskReport?.status,"completed");assert.equal(s.assistantToolCalls.length,2); +}); + + +test("SDK JSON-string results preserve failures without interpreting file content as status", async () => { + const {toolSucceeded} = await import("../../ai/tool-outcome.js"); + const failure = JSON.stringify({success:false,error:"Denied"}); + assert.equal(toolSucceeded({content:failure,detailedContent:failure},true),false); + assert.equal(toolSucceeded({detailedContent:failure},true),false); + const file = JSON.stringify({path:"fixture.json",content:failure}); + assert.equal(toolSucceeded({content:file,detailedContent:file},true),true); +}); diff --git a/services/api/src/routes/chat/send-handler.ts b/services/api/src/routes/chat/send-handler.ts index 13067c7d..334394b4 100644 --- a/services/api/src/routes/chat/send-handler.ts +++ b/services/api/src/routes/chat/send-handler.ts @@ -3,6 +3,8 @@ * Coordinates all chat stream phases: setup, session management, * message sending, recovery, and post-processing. */ +import { getPlan } from "../../ai/plan-store.js"; +import { planToMarkdown } from "../../ai/plan-state.js"; import { Hono } from "hono"; import { z } from "zod"; import { zValidator } from "@hono/zod-validator"; @@ -34,11 +36,12 @@ import { projectSessions, activeRequests } from "./session-state.js"; import { buildSystemPrompt } from "./system-prompts.js"; import { createRecordAssistantToolCall, createToolProgressCallbacks } from "./tool-callbacks.js"; import { createProcessEvent } from "./event-processor.js"; +import { finalizeLeadingResponse } from "./final-response.js"; import { popArtifacts } from "./artifact-stash.js"; import { scaffoldAndStartDev, emitConfigTraces, logToolManifest, handleToolEndEvent } from "./send-helpers.js"; import { checkAndEvictOnModeChange, checkAndEvictOnProviderChange, resolveSession, persistSessionToDb, filterToolsForMode, recreateSession } from "./session-manager.js"; import { resolveUserDisplay, saveUserMessage, preInsertAssistantMessage } from "./message-persistence.js"; -import { handleAutoContinue, handleEmptyResponseRetry } from "./stream-recovery.js"; +import { handleAutoContinue, handleEmptyResponseRetry, type RecoveryPipeline } from "./stream-recovery.js"; import { handleAutoFixPreview, handleVersionAndMemory, handleFinalCleanup, handleStreamError } from "./post-processing.js"; import { writeStreamBuffer, shouldBufferType, type BufferedEvent, type StreamBuffer } from "./stream-buffer.js"; import { getRateLimitState } from "../../ai/rate-limit-state.js"; @@ -426,6 +429,8 @@ export function registerSendHandler(app: Hono) { }; const state = createInitialState(); + const active = activeRequests.get(projectId); + if(active) active.cancel = () => { state.runOutcome="aborted"; }; // Register this stream as the channel for blocking "ask the user" // prompts (tools that pause mid-turn for a human decision, e.g. the @@ -477,6 +482,7 @@ export function registerSendHandler(app: Hono) { return; } thinkingLoopAborted = true; + state.runOutcome="stalled"; console.warn(`[Chat][${projectId.slice(0, 8)}] thinking_loop watchdog firing — realSilence=${realSilence}ms, no tools, no content`); state.traceCollector?.onError("thinking_loop", "STREAM", "thinking_loop_timeout"); try { @@ -622,6 +628,12 @@ export function registerSendHandler(app: Hono) { // Strip the /skill prefix from the message sent to AI augmentedContent = cleanMessage + (augmentedContent !== content ? augmentedContent.slice(content.length) : ""); } + // The DB snapshot is current even when an older session cached plan.md. + // It supplies stable IDs; finishing this turn is not proof of plan completion. + const currentPlan = await getPlan(projectId); + if (mode !== "plan" && currentPlan && currentPlan.status !== "draft") { + augmentedContent += `\n\n\n${planToMarkdown(currentPlan)}\n`; + } const [projectContext, allTools] = await Promise.all([ buildProjectContextForMode(projectId, mode, workspaceId, userId, { invokedSkillNames, @@ -688,6 +700,7 @@ export function registerSendHandler(app: Hono) { handleToolEndEvent(stream, toolName, args, projectId); }); const releaseTracker = getCopilotManager().trackRequest(projectId); + let recoveryPipeline: RecoveryPipeline | undefined; try { const manager = getCopilotManager(); @@ -736,38 +749,14 @@ export function registerSendHandler(app: Hono) { } } - // ── Finalize leading-text buffer at stream end ── - // Post-tool text stays as thinking — tool results (file changes, - // build cards, MCP UI resources) provide all the visible UI the - // user needs. Converting the buffer to content leaks internal - // reasoning (BUG-119: MiniMax emits untagged reasoning as text). - // However, if no tool calls occurred AND no content was emitted, - // the buffer is the actual response (e.g. simple chat greeting). - if (state.leadingTextBuffer) { - const bufLen = state.leadingTextBuffer.length; - if (!state.hadToolCalls && state.assistantContent.length === 0) { - // No tool calls, no content — this IS the response, not reasoning - const buffered = state.leadingTextBuffer; - state.leadingTextBuffer = ""; - state.leadingTextFlushed = true; - // Strip any ... blocks that the channel router - // didn't catch during streaming (token boundary issue) - const visibleContent = buffered.replace(/[\s\S]*?<\/think>\s*/gi, "").trim(); - if (visibleContent) { - // Move buffer from thinking to content (only the visible portion) - state.assistantThinking = state.assistantThinking.slice(0, state.assistantThinking.length - buffered.length); - state.assistantContent += visibleContent; - console.log(`[Chat][${projectId.slice(0, 8)}] Flushing ${visibleContent.length} chars as content (stripped from ${bufLen} buffer, no tools)`); - broadcastToRoom(projectId, { type: "ai:stream-chunk", chunk: visibleContent, messageId, isThinking: false }, userId).catch(() => {}); - await stream.writeSSE({ data: JSON.stringify({ type: "thinking_to_text", data: visibleContent }) }); - } else { - console.log(`[Chat][${projectId.slice(0, 8)}] Keeping ${bufLen} chars as thinking (all thinking, no visible content)`); - } - } else { - state.leadingTextBuffer = ""; - state.leadingTextFlushed = true; - console.log(`[Chat][${projectId.slice(0, 8)}] Keeping ${bufLen} chars as thinking (stream end, hadTools=${state.hadToolCalls})`); - } + // A tool call confirms preceding text as intermediate reasoning. + // The remaining ordinary-text segment at successful stream end is + // the final answer, regardless of tools in earlier turns. + const finalResponse = finalizeLeadingResponse(state); + if (finalResponse) { + state.traceCollector?.onTextDelta(finalResponse); + broadcastToRoom(projectId, { type: "ai:stream-chunk", chunk: finalResponse, messageId, isThinking: false }, userId).catch(() => {}); + await stream.writeSSE({ data: JSON.stringify({ type: "thinking_to_text", data: finalResponse }) }); } console.log(`[Chat][${projectId.slice(0, 8)}] stream done — content: ${state.assistantContent.length}, thinking: ${state.assistantThinking.length}, tools: ${state.hadToolCalls}`); @@ -782,16 +771,27 @@ export function registerSendHandler(app: Hono) { had_tool_calls: state.hadToolCalls, }); - // Save pre-recovery content length to detect if auto-continue added anything - const contentBeforeRecovery = state.assistantContent.length; - await tracePhase(state, "auto_continue", () => - handleAutoContinue(stream, state, currentEngine, sessionId!, projectId, mode, recordAssistantToolCall, content), - ); + recoveryPipeline = { + process: processEvent, + flush: async () => { + for (const chunk of channelRouter.flush()) { + if(chunk.type === "text") state.assistantContent += chunk.content; + else if(chunk.type === "thinking") state.assistantThinking += chunk.content; + await stream.writeSSE({data:JSON.stringify({type:chunk.type === "text" ? "text_delta" : chunk.type,data:chunk.content})}); + } + const response=finalizeLeadingResponse(state); + if(response) await stream.writeSSE({data:JSON.stringify({type:"thinking_to_text",data:response})}); + }, + }; await tracePhase(state, "empty_response_retry", () => - handleEmptyResponseRetry(stream, state, currentEngine, sessionId!, projectId, augmentedContent, fileAttachments), + handleEmptyResponseRetry(stream, state, currentEngine, sessionId!, projectId, augmentedContent, fileAttachments,recoveryPipeline), + ); + await tracePhase(state, "auto_continue", () => + handleAutoContinue(stream, state, currentEngine, sessionId!, projectId, mode, recordAssistantToolCall, content,recoveryPipeline), ); if (!state.hadToolCalls && state.sawToolDelta) { + state.runOutcome ??= "error"; await stream.writeSSE({ data: JSON.stringify({ type: "error", @@ -800,19 +800,19 @@ export function registerSendHandler(app: Hono) { }); } - // Emit deferred session.error only if auto-continue didn't produce new content - if (state.deferredError && state.assistantContent.length <= contentBeforeRecovery) { - console.log(`[Chat][${projectId.slice(0, 8)}] emitting deferred error (no recovery): ${state.deferredError.slice(0, 80)}`); + // Partial text is not evidence that a provider error was recovered. + if (state.deferredError) { + state.runOutcome ??= "error"; await stream.writeSSE({ data: JSON.stringify({ type: "error", data: state.deferredError }) }); - } else if (state.deferredError) { - console.log(`[Chat][${projectId.slice(0, 8)}] swallowed deferred error — auto-continue recovered (${state.assistantContent.length - contentBeforeRecovery} chars added)`); } state.deferredError = undefined; // Flush pending tool names - for (const pendingName of state.pendingToolNames) { + for (const pending of state.assistantToolCalls.filter(r => r.status === "running")) { + state.runOutcome ??= "stalled"; + const pendingName = pending.name; const arts = state.pendingArtifacts.get(pendingName) ?? popArtifacts(pendingName); - const data: Record = { name: pendingName, success: true, friendlyMessage: "Done" }; + const data: Record = { name: pendingName, toolCallId: pending.callId, friendlyMessage: "No completion result received" }; if (arts && arts.length > 0) { data.artifacts = arts; state.pendingArtifacts.delete(pendingName); @@ -832,6 +832,7 @@ export function registerSendHandler(app: Hono) { } state.pendingArtifacts.clear(); } catch (err) { + state.runOutcome ??= "error"; const msg = err instanceof Error ? err.message : String(err); if (msg.includes("not authorized") || msg.includes("policy") || msg.includes("unauthorized")) { const manager = getCopilotManager(); @@ -859,9 +860,9 @@ export function registerSendHandler(app: Hono) { // Skip post-processing entirely if the watchdog already aborted — // the stream is in an error state and there's no real assistant // content to fix/version/save. - if (!thinkingLoopAborted) { + if (!thinkingLoopAborted && !state.runOutcome) { await tracePhase(state, "auto_fix_preview", () => - handleAutoFixPreview(stream, state, projectId, resolvedGithubToken, sessionId!), + handleAutoFixPreview(stream, state, projectId, resolvedGithubToken, sessionId!, recoveryPipeline), ); await tracePhase(state, "version_and_memory", () => handleVersionAndMemory(stream, state, projectId, userId, content, messageId), diff --git a/services/api/src/routes/chat/send-helpers.ts b/services/api/src/routes/chat/send-helpers.ts index de5a5abe..078556e7 100644 --- a/services/api/src/routes/chat/send-helpers.ts +++ b/services/api/src/routes/chat/send-helpers.ts @@ -182,27 +182,19 @@ export function handleToolEndEvent(stream: SSEStreamingApi, toolName: string, ar if (toolName === "create_plan" && args.output) { try { const plan = JSON.parse(args.output as string); - (async () => { - try { - const planId = plan.id as string; - await sql`INSERT INTO plans (id, project_id, summary, complexity, status, created_at) VALUES (${planId}, ${projectId}, ${plan.summary}, ${plan.complexity}, 'draft', ${plan.createdAt}) ON CONFLICT (id) DO NOTHING`; - if (Array.isArray(plan.steps)) { - for (const step of plan.steps) { - await sql`INSERT INTO plan_steps (id, plan_id, "order", title, description, details, status, file_paths) VALUES (${step.id}, ${planId}, ${step.order}, ${step.title}, ${step.description}, ${step.details ?? null}, 'pending', ${step.filePaths ?? null}) ON CONFLICT (id) DO NOTHING`; - } - } - } catch (dbErr) { console.warn("[Chat] Failed to save plan to DB:", dbErr); } - })(); stream.writeSSE({ data: JSON.stringify({ type: "plan", data: { plan } }) }).catch(() => {}); } catch { /* parse error */ } } + if (toolName === "update_plan" && args.plan) { + stream.writeSSE({ data: JSON.stringify({ type: "plan", data: { plan: args.plan } }) }).catch(() => {}); + } if (toolName === "mark_step_complete") { - const { stepId, planId, status } = args as { stepId?: string; planId?: string; status?: string }; + const { stepId, planId, status, plan } = args as { stepId?: string; planId?: string; status?: string; plan?: unknown }; if (stepId && planId) { stream.writeSSE({ data: JSON.stringify({ type: "plan_step_update", - data: { stepId, planId, status: status ?? "completed" }, + data: { stepId, planId, status: status ?? "completed", plan }, }), }).catch(() => {}); } diff --git a/services/api/src/routes/chat/session-manager.ts b/services/api/src/routes/chat/session-manager.ts index b69856d3..8bdb843e 100644 --- a/services/api/src/routes/chat/session-manager.ts +++ b/services/api/src/routes/chat/session-manager.ts @@ -1,3 +1,4 @@ +import {selectModeTools} from "../../ai/plan-tool-policy.js"; /** * Session management: eviction, resume, creation, DB persistence, * and session recreation on engine loss during sendMessage. @@ -8,7 +9,8 @@ import { createAllTools, type ByokProviderConfig } from "../../ai/providers/copi import type { TraceCollector } from "../../ai/trace-collector.js"; import { createPermissionHandler } from "../../ai/docore-bridge.js"; import { createHash } from "node:crypto"; -import { projectSessions, projectSessionModes, projectSessionProviders } from "./session-state.js"; +import { projectSessions, projectSessionModes, projectSessionProviders, projectSessionPrompts } from "./session-state.js"; +import { fingerprintSystemPrompt, reuseSessionWithPrompt } from "../../ai/session-prompt.js"; import { modeToolQueries } from "@doable/db"; /** @@ -67,6 +69,7 @@ export async function checkAndEvictOnProviderChange( console.log(`[Chat] provider/model fingerprint changed ${prev} → ${next} for ${sessionKey} — evicting cached session`); projectSessions.delete(sessionKey); projectSessionModes.delete(sessionKey); + projectSessionPrompts.delete(sessionKey); projectSessionProviders.set(sessionKey, next); if (evictedSid) { traceCollector?.onSessionEvict(evictedSid, `provider_change:${prev}->${next}`); @@ -87,15 +90,6 @@ export async function checkAndEvictOnProviderChange( const modeTools = modeToolQueries(sql); -// Hardcoded fallbacks (used when no DB config exists) -const PLAN_MODE_ALLOWED_DEFAULT = new Set([ - "read_file", "list_files", "search_files", - "ask_clarification", "create_plan", "mark_step_complete", -]); -const PLAN_ONLY_TOOLS = new Set([ - "ask_clarification", "create_plan", "mark_step_complete", -]); - // In-memory cache for DB tool configs (refreshed every 60s) let _toolConfigCache: Map> | null = null; let _toolConfigCacheAt = 0; @@ -123,13 +117,7 @@ async function getToolConfigForMode(mode: string): Promise | null> { // eslint-disable-next-line @typescript-eslint/no-explicit-any export async function filterToolsForMode(allTools: any[], mode: string) { const dbAllowed = await getToolConfigForMode(mode); - if (dbAllowed) { - return allTools.filter((t: { name?: string }) => dbAllowed.has(t.name ?? "")); - } - // Fallback to hardcoded defaults - return mode === "plan" - ? allTools.filter((t: { name?: string }) => PLAN_MODE_ALLOWED_DEFAULT.has(t.name ?? "")) - : allTools.filter((t: { name?: string }) => !PLAN_ONLY_TOOLS.has(t.name ?? "")); + return selectModeTools(allTools,mode,dbAllowed); } /** Check if session mode changed and evict if needed. Returns true if mode changed. */ @@ -145,6 +133,7 @@ export function checkAndEvictOnModeChange( console.log(`[Chat] mode changed ${cachedMode} → ${mode} for ${sessionKey} — evicting cached session`); projectSessions.delete(sessionKey); projectSessionModes.delete(sessionKey); + projectSessionPrompts.delete(sessionKey); if (evictedSid) { traceCollector?.onSessionEvict(evictedSid, `mode_change:${cachedMode}->${mode}`); } @@ -172,8 +161,37 @@ export async function resolveSession( stream: import("hono/streaming").SSEStreamingApi, skillDirectories: string[] | undefined, ): Promise { + const promptFingerprint = fingerprintSystemPrompt(systemPrompt); + const resumeConfig = { + model: resolvedModel, + provider: resolvedProvider, + systemPrompt, + tools: sessionTools, + toolProgress, + workingDirectory: projectPath, + onPermissionRequest: createPermissionHandler(userId, projectPath), + skillDirectories, + }; let sessionId = projectSessions.get(sessionKey); - if (sessionId) return sessionId; + if (sessionId) { + const engine = await getCopilotManager().getEngine(projectId, resolvedGithubToken); + const cachedId = sessionId; + const reused = await reuseSessionWithPrompt({ + sessionId: cachedId, + previousFingerprint: projectSessionPrompts.get(sessionKey), + nextFingerprint: promptFingerprint, + bindCurrentTurn: () => engine.bindToolProgress(cachedId, toolProgress), + resumeCurrentPrompt: (id) => engine.resumeSession(id, resumeConfig), + }); + if (reused) { + projectSessions.set(sessionKey, reused); + projectSessionPrompts.set(sessionKey, promptFingerprint); + return reused; + } + projectSessions.delete(sessionKey); + projectSessionPrompts.delete(sessionKey); + sessionId = undefined; + } await stream.writeSSE({ data: JSON.stringify({ type: "status", data: { phase: "connecting", message: "Connecting to AI..." } }), @@ -197,18 +215,13 @@ export async function resolveSession( // BUG-RESUME-PROVIDER: resume must pass the SAME provider+model as // the create path, otherwise the resumed CLI session has no model // to call and the turn hangs until the thinking_loop watchdog fires. - model: resolvedModel, - provider: resolvedProvider, - tools: sessionTools, - toolProgress, - workingDirectory: projectPath, - onPermissionRequest: createPermissionHandler(userId, projectPath), - skillDirectories, + ...resumeConfig, }); }); projectSessions.set(sessionKey, sessionId!); projectSessionModes.set(sessionKey, mode); projectSessionProviders.set(sessionKey, computeProviderFingerprint(resolvedProvider, resolvedModel)); + projectSessionPrompts.set(sessionKey, promptFingerprint); resumed = true; console.log(`[Chat] Resumed SDK session ${dbRow.copilot_session_id.slice(0, 8)}… for ${projectId.slice(0, 8)}… (mode=${mode}, tools=${sessionTools.length})`); } @@ -237,6 +250,7 @@ export async function resolveSession( projectSessions.set(sessionKey, sessionId!); projectSessionModes.set(sessionKey, mode); projectSessionProviders.set(sessionKey, computeProviderFingerprint(resolvedProvider, resolvedModel)); + projectSessionPrompts.set(sessionKey, promptFingerprint); } return sessionId!; @@ -327,6 +341,7 @@ export async function recreateSession( projectSessions.set(sessionKey, sessionId); projectSessionModes.set(sessionKey, mode); projectSessionProviders.set(sessionKey, computeProviderFingerprint(resolvedProvider, resolvedModel)); + projectSessionPrompts.set(sessionKey, fingerprintSystemPrompt(systemPrompt)); if (mode === "plan" && sessionId) { try { await currentEngine.setSessionMode(sessionId, "plan"); diff --git a/services/api/src/routes/chat/session-state.test.ts b/services/api/src/routes/chat/session-state.test.ts new file mode 100644 index 00000000..c4a37546 --- /dev/null +++ b/services/api/src/routes/chat/session-state.test.ts @@ -0,0 +1,22 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { + evictProjectSessions, projectSessionModes, projectSessionPrompts, + projectSessionProviders, projectSessions, +} from "./session-state.js"; + +test("context invalidation clears prompt fingerprints for all project modes only", () => { + const keys = ["context-project", "context-project:visual-edit", "unrelated-project"]; + const maps = [projectSessions, projectSessionModes, projectSessionProviders, projectSessionPrompts]; + try { + for (const map of maps) for (const key of keys) map.set(key, "value"); + assert.equal(evictProjectSessions("context-project"), 2); + for (const map of maps) { + assert.equal(map.has("context-project"), false); + assert.equal(map.has("context-project:visual-edit"), false); + assert.equal(map.get("unrelated-project"), "value"); + } + } finally { + for (const map of maps) for (const key of keys) map.delete(key); + } +}); diff --git a/services/api/src/routes/chat/session-state.ts b/services/api/src/routes/chat/session-state.ts index e176f71f..034b4633 100644 --- a/services/api/src/routes/chat/session-state.ts +++ b/services/api/src/routes/chat/session-state.ts @@ -27,13 +27,17 @@ export const projectSessionModes = new Map(); // or trace events. (BUG-R9-CHAT-SESSION-STICKY-ON-OLD-PROVIDER.) export const projectSessionProviders = new Map(); +// Effective prompt applied to the SDK session, including scoped instructions. +// Store only its hash, never the potentially sensitive prompt itself. +export const projectSessionPrompts = new Map(); + // Track active streaming requests per project so /ai-status can report // whether the AI is still working (survives page refresh). -export const activeRequests = new Map(); +export const activeRequests = new Map void }>(); /** * Evict all cached chat sessions for a project so the next chat message - * creates a fresh session that picks up updated context (identity.md, + * resumes with updated context (identity.md, * soul.md, instructions.md, knowledge.md etc.). Without this, edits to * `.doable/*.md` only take effect after the user manually starts a new * session — the AI keeps using the old system prompt. @@ -45,6 +49,7 @@ export function evictProjectSessions(projectId: string): number { projectSessions.delete(key); projectSessionModes.delete(key); projectSessionProviders.delete(key); + projectSessionPrompts.delete(key); count++; } } diff --git a/services/api/src/routes/chat/stream-recovery.supabase-gate.test.ts b/services/api/src/routes/chat/stream-recovery.supabase-gate.test.ts index f02fdebe..3a857e95 100644 --- a/services/api/src/routes/chat/stream-recovery.supabase-gate.test.ts +++ b/services/api/src/routes/chat/stream-recovery.supabase-gate.test.ts @@ -116,8 +116,8 @@ test("control: auto-continue DOES fire for the same state when no provision is p ); assert.match( calls[0] ?? "", - /continue building/i, - "the nudge prompt should be the build-continue nudge", + /report_task_status/, + "recovery must request structured task reconciliation, not force file writes", ); }); diff --git a/services/api/src/routes/chat/stream-recovery.ts b/services/api/src/routes/chat/stream-recovery.ts index 33555178..77cfa939 100644 --- a/services/api/src/routes/chat/stream-recovery.ts +++ b/services/api/src/routes/chat/stream-recovery.ts @@ -1,3 +1,5 @@ +import { cycleFingerprint } from "./execution-state.js"; +import { classifyProviderError } from "../../ai/provider-error.js"; /** * Stream recovery: auto-continue (stall detection) and empty-response retry. */ @@ -5,96 +7,76 @@ import type { SSEStreamingApi } from "hono/streaming"; import type { ChatStreamState } from "./types.js"; import type { CopilotEngine } from "../../ai/providers/copilot.js"; import { mapEventToSSE, ChannelTokenRouter } from "../../ai/sse-mapper.js"; -import { stripServerPaths } from "../../ai/tool-messages.js"; import { recordToolEventForTrace } from "./tool-event-bookkeeping.js"; -const MAX_AUTO_CONTINUE = 6; -const MAX_READ_ONLY_CYCLES = 3; -// BUG-VISUAL-EDIT-001: Visual Edit prompts arrive with full selector -// context, so the model SHOULD edit immediately. But it sometimes does up -// to 3-4 reads (locate the JSX, peek related components) before committing -// — bumping the stall ceiling from 3 to 5 keeps the watchdog protective on -// truly stuck turns while not killing the legitimate visual-edit "explore -// then edit" pattern. -const MAX_READ_ONLY_CYCLES_VISUAL_EDIT = 5; -const FILE_WRITE_TOOLS = new Set(["create_file", "edit_file", "write_file", "create", "edit", "write"]); -const READ_TOOLS = new Set(["read_file", "list_files", "search_files", "read", "list", "search"]); - -/** - * Returns true when the user message is a Visual Edit turn (the editor - * auto-prefixes `[Visual Edit] For the element with class "..." - * (selector: ...): `). These turns ALWAYS want a build - * intent regardless of what verb the user used ("animate this text", - * "make it red", etc.) — the selector + element tag is implicit context - * that the request is to MUTATE the element. - */ -function isVisualEditPrompt(userMessage: string | undefined): boolean { - if (!userMessage) return false; - return /^\s*\[Visual Edit\]/i.test(userMessage); +export interface RecoveryPipeline { + process: (event: import("@github/copilot-sdk").SessionEvent) => void; + flush: () => Promise; } -/** - * MCP tools that produce a finished artifact (UI resource, download, etc.) - * — calling one of these IS the deliverable, so auto-continue must NOT - * pester the model to "continue building" afterwards. Match by suffix so - * connector-prefixed names (mcp__) are covered too. - */ -// Only RENDERERS belong here — not the "create_*" kick-off tools. The kick-off -// returns a status card that injects a BUILD_* prompt for the model's NEXT -// turn; the actual artifact (xlsx / pdf / slides / md) is produced by the -// matching build_* / render_*. Listing the kick-offs here made auto-continue -// declare "deliverable produced" after the status card alone, so the model -// never reached build_* and the project's index.html / App.tsx stayed at the -// default Vite scaffold (no SheetJS viewer, no .xlsx file, no preview). -const PRODUCTIVE_TOOL_SUFFIXES = [ - // presentation-builder - "build_deck", - "render_deck", - "render_pptx", - "render_web_slides", - "build_presentation", - // markdown-builder - "build_markdown", - // spreadsheet-builder - "build_spreadsheet", - // pdf-builder - "build_pdf", -]; -function isProductiveToolName(name: string | undefined): boolean { - if (!name) return false; - return PRODUCTIVE_TOOL_SUFFIXES.some((s) => name === s || name.endsWith(`_${s}`) || name.endsWith(`.${s}`)); -} - -/** - * Heuristically decide whether the user actually asked the AI to BUILD/MODIFY - * something. Auto-continue exists to nudge the model when it explored but - * forgot to write files for a build request — it must NOT fire when the user - * only asked a read-only / informational question ("read X", "what does Y do?", - * "show me Z"), because then the model is correctly done after one read. - */ -function userWantsBuild(userMessage: string | undefined): boolean { - if (!userMessage) return true; // unknown intent → preserve old behaviour - // BUG-VISUAL-EDIT-001: Visual Edit prompts ALWAYS want a build — the - // selector + element tag are implicit "mutate this" context regardless - // of which verb the user used ("animate this", "make it red", etc.). - if (isVisualEditPrompt(userMessage)) return true; - const m = userMessage.toLowerCase().trim(); - if (m.length === 0) return true; - // Strong informational/read-only signals — short messages dominated by these - // verbs are almost never build requests. - const readOnly = /\b(read|show|tell me|what is|what does|what's|explain|describe|summari[sz]e|list|find|search|check|inspect|look at|view|open|display|print|how does|why does|count)\b/; - const buildIntent = /\b(build|create|make|add|implement|fix|update|modify|change|edit|write|generate|scaffold|refactor|deploy|publish|install|setup|set up|configure|design|develop|extend|enhance|improve|delete|remove|rename|move)\b/; - const hasBuild = buildIntent.test(m); - const hasRead = readOnly.test(m); - if (hasBuild) return true; - if (hasRead) return false; - // No clear signal — be conservative: if the AI already invoked tools (MCP - // calls, search, etc.) and is otherwise "done", forcing a "continue building" - // turn is far worse than under-continuing. Only auto-continue when the user - // gave a long, descriptive product brief (>= 25 words) which implies a build. - return m.split(/\s+/).length >= 25; +/** Standalone adapter for callers without the main chat pipeline. Production + * supplies the SAME processor/flush used by the initial stream. */ +function recoveryPipeline( + stream: SSEStreamingApi, + state: ChatStreamState, + provided?: RecoveryPipeline, +): RecoveryPipeline { + if (provided) return provided; + const router = new ChannelTokenRouter(); + let rawLength = 0; + const emit = (type: string, data: unknown) => { + stream.writeSSE({ data: JSON.stringify({ type, data }) }).catch(() => {}); + }; + const chunks = (items: ReturnType) => { + for (const chunk of items) { + if (chunk.type === "text") state.assistantContent += chunk.content; + else if (chunk.type === "thinking") + state.assistantThinking += chunk.content; + emit(chunk.type === "text" ? "text_delta" : chunk.type, chunk.content); + } + }; + return { + process(event) { + state.usageCollector?.onUsageEvent(event); + state.traceCollector?.onSdkEvent(event as Record); + const toolEvent = recordToolEventForTrace( + state, + event as Record, + () => {}, + ); + if (toolEvent.suppressDisplay) return; + const e = event as unknown as { + type: string; + data?: Record; + }; + if (e.type === "assistant.message") { + const full = String(e.data?.content ?? ""); + chunks(router.process(full.slice(rawLength))); + rawLength = 0; + return; + } + const mapped = mapEventToSSE(event, { preserveThinkingMarkers: true }); + if (!mapped || mapped.type === "done") return; + if (mapped.type === "text_delta" && typeof mapped.data === "string") { + rawLength += mapped.data.length; + chunks(router.process(mapped.data)); + return; + } + if (mapped.type === "error") { + state.runOutcome ??= "error"; + state.traceCollector?.onError(String(mapped.data), "recovery_provider"); + } + if (mapped.type === "thinking" && typeof mapped.data === "string") + state.assistantThinking += mapped.data; + emit(mapped.type, mapped.data); + }, + async flush() { + chunks(router.flush()); + }, + }; } -/** Run auto-continue loops if AI explored but wrote 0 files. */ +/** Recovery never infers authorization from translated prose. The model reports + * intent through a validated tool; unknown clients get one reconciliation round. */ export async function handleAutoContinue( stream: SSEStreamingApi, state: ChatStreamState, @@ -103,196 +85,108 @@ export async function handleAutoContinue( projectId: string, mode: string, recordAssistantToolCall: (name?: string, args?: unknown) => void, - userMessage?: string, + _userMessage?: string, + providedPipeline?: RecoveryPipeline, ): Promise { - if (mode === "plan") return; - - // If the user's request was clearly informational ("read X", "what is Y?"), - // don't push the model to start writing files. This was the source of the - // "Stall: same files read in consecutive continues" error. - if (!userWantsBuild(userMessage)) { - console.log(`[Chat][${projectId.slice(0, 8)}] auto-continue skipped — read-only user intent`); + if ( + mode === "plan" || + mode === "chat" || + state.runOutcome || + state.deferredError + ) return; - } - - // If an MCP interactive widget was shown (e.g. format picker), the session - // is intentionally waiting for a user click — do NOT auto-continue, that - // would force the model to bypass the picker and start generating files. - if (state.awaitingMcpWidget) { - console.log(`[Chat][${projectId.slice(0, 8)}] auto-continue skipped — awaiting MCP widget click`); - return; - } - - // If `provision_supabase` opened the Connect-Supabase dialog this turn, the - // DB connection is not live until the user completes it. Auto-continuing here - // nudges the model to keep building while Supabase is still unconnected — at - // which point the race-guard blocks @supabase/supabase-js but NOT @doable/data, - // so the model falls back to the per-app PGlite DB and builds the whole app in - // the wrong database. End the turn and wait for the editor's "Supabase - // provisioning complete" re-prompt, which fires once the connection exists. - if (state.awaitingSupabaseProvision) { - console.log(`[Chat][${projectId.slice(0, 8)}] auto-continue skipped — awaiting Supabase provision dialog`); - return; - } - - // If `request_integration` opened the Connect-integration dialog this turn, - // the third-party credential is not live until the user completes the popup. - // Auto-continuing here nudges the model to keep building against an empty - // block — at which point it guesses actionNames - // (e.g. `text_to_speech` instead of `elevenlabs-text-to-speech`) and the - // generated app throws at runtime. End the turn; the fresh manifest is - // picked up on the next user message. - if (state.awaitingIntegrationConnect) { - console.log(`[Chat][${projectId.slice(0, 8)}] auto-continue skipped — awaiting integration connect dialog`); - return; - } - - // BUG-VISUAL-EDIT-001: a Visual Edit turn arrives with full selector - // context, so 3 read-only cycles is too tight a ceiling — the model often - // legitimately needs 3-4 reads to locate the JSX before committing the - // edit. Use the wider cap for these turns; everyone else keeps the old - // 3-cycle ceiling. - const isVisualEdit = isVisualEditPrompt(userMessage); - const readOnlyCycleCeiling = isVisualEdit ? MAX_READ_ONLY_CYCLES_VISUAL_EDIT : MAX_READ_ONLY_CYCLES; - - let autoContinueCount = 0; - let prevReadFingerprint = ""; - let consecutiveReadOnlyCycles = 0; - - while (autoContinueCount < MAX_AUTO_CONTINUE) { - const wroteFiles = state.assistantToolCalls.some( - (tc) => FILE_WRITE_TOOLS.has((tc as { name?: string }).name ?? ""), - ); - const producedArtifact = state.assistantToolCalls.some( - (tc) => isProductiveToolName((tc as { name?: string }).name), - ); - if (!state.hadToolCalls || wroteFiles || producedArtifact) break; - - // Stall detection: same-file fingerprinting - const toolCallsSinceLastContinue = autoContinueCount === 0 - ? state.assistantToolCalls - : state.assistantToolCalls.slice(-20); - const readFiles = toolCallsSinceLastContinue - .filter((tc) => READ_TOOLS.has((tc as { name?: string }).name ?? "")) - .map((tc) => { - const args = tc as Record; - return String(args.path ?? args.file_path ?? args.filePath ?? args.name ?? ""); - }) - .sort() - .join("|"); - - if (autoContinueCount > 0 && readFiles === prevReadFingerprint && readFiles !== "") { - state.traceCollector?.onError(`Stall: same files read in consecutive continues: ${readFiles.slice(0, 100)}`, "auto_continue_fingerprint"); - console.warn(`[Chat][${projectId.slice(0, 8)}] stall detected — same files read`); - await stream.writeSSE({ - data: JSON.stringify({ - type: "error", - data: "The AI appears to be stuck reading the same files without making progress. Try rephrasing your request or check the preview for errors.", - }), - }).catch(() => {}); - break; - } - prevReadFingerprint = readFiles; - - consecutiveReadOnlyCycles++; - if (consecutiveReadOnlyCycles >= readOnlyCycleCeiling) { - state.traceCollector?.onError(`Stall: ${consecutiveReadOnlyCycles} consecutive read-only continues (visualEdit=${isVisualEdit})`, "auto_continue_write_free"); - console.warn(`[Chat][${projectId.slice(0, 8)}] stall detected — ${consecutiveReadOnlyCycles} read-only continues`); - // BUG-VISUAL-EDIT-001: tailor the bail message — Visual Edit users - // already gave a precise selector, so "provide more guidance" is - // misleading; the real problem is the model's planning. - const bailMessage = isVisualEdit - ? "I couldn't apply the visual edit automatically. Try rephrasing — for example, name the exact change ('add a fade-in animation', 'change the color to red')." - : "The AI has been investigating without making changes. Please provide more specific guidance, or check the preview console for errors."; - await stream.writeSSE({ - data: JSON.stringify({ type: "error", data: bailMessage }), - }).catch(() => {}); - break; + if (!state.hadToolCalls) return; + const pipeline = recoveryPipeline(stream, state, providedPipeline); + let previous = cycleFingerprint(state, state.recoveryCycle ?? 0); + let unchanged = 0; + const fail = async (outcome: "stalled" | "error", message: string) => { + if (state.runOutcome === "aborted") return; + state.runOutcome = outcome; + state.traceCollector?.onError(message, `recovery_${outcome}`); + await stream + .writeSSE({ data: JSON.stringify({ type: "error", data: message }) }) + .catch(() => {}); + }; + for (let round = 1; round <= 6; round++) { + if ( + state.runOutcome || + state.awaitingMcpWidget || + state.awaitingSupabaseProvision || + state.awaitingIntegrationConnect + ) + return; + const report = state.taskReport; + if (report?.status === "waiting_for_input") { + state.runOutcome = "waiting"; + return; } - - autoContinueCount++; - state.traceCollector?.onAutoContinue(autoContinueCount, `read-only cycle ${autoContinueCount}/${MAX_AUTO_CONTINUE}`); - console.log(`[Chat][${projectId.slice(0, 8)}] auto-continue attempt ${autoContinueCount}/${MAX_AUTO_CONTINUE}`); - + if (report?.status === "completed") return; + state.recoveryCycle = (state.recoveryCycle ?? 0) + 1; + state.taskReport = undefined; // Never reuse the previous cycle's completion report. + state.traceCollector?.onAutoContinue( + round, + "structured_task_reconciliation", + ); + await stream.writeSSE({ + data: JSON.stringify({ + type: "status", + data: { phase: "continuing", message: "Checking task progress..." }, + }), + }); try { - await stream.writeSSE({ - data: JSON.stringify({ - type: "status", - data: { phase: "continuing", message: `Continuing to build\u2026 (step ${autoContinueCount})` }, - }), - }); - // BUG-VISUAL-EDIT-001: when a Visual Edit turn enters auto-continue, - // use a sharper, selector-aware nudge instead of the generic "create - // all the files" prompt. The Visual Edit user already specified the - // exact element; the model just needs to commit to an `edit_file` - // call now. - const continuePrompt = isVisualEdit - ? "You have the selector and the user's intent. Make the edit NOW with edit_file — modify the JSX/HTML for that exact element only. Do not read any more files. Apply the change in a single edit_file call. Reply with one short sentence describing what you changed." - : "You explored the project and installed packages but haven't created any files yet. Continue building NOW — create all the files the user asked for. Do NOT stop until the app is working in the preview."; await engine.sendMessage( sessionId, - continuePrompt, + "Reconcile the ORIGINAL user request and the actual tool results. Call report_task_status. If the request was read-only, do not make changes. If it is fulfilled, report completed and stop. If user input is needed, report waiting_for_input and ask. Only continue work explicitly requested and still unfinished, preserving existing results. For an approved plan use get_plan and report verified step outcomes. Never recreate completed work just to produce a file write.", undefined, - (evt: import("@github/copilot-sdk").SessionEvent) => { - if (state.usageCollector) state.usageCollector.onUsageEvent(evt); - state.traceCollector?.onSdkEvent(evt as Record); - // BUG-TRACE-001: route tool start/end through the same helper the - // main turn uses, so auto-continue's tool calls (read_file / - // edit_file etc.) increment `tool_call_count` and produce - // tool_start/tool_end trace events. Previously this inline - // callback only matched `tool.execution_start` / - // `tool.execution_complete` / `tool.completed` — missing - // `tool.running` and `external_tool.completed`, which silently - // dropped MCP-channel tool events from the trace. - recordToolEventForTrace(state, evt as Record, recordAssistantToolCall); - const sseData = mapEventToSSE(evt as Record); - if (!sseData) return; - state.lastRealEventAt = Date.now(); - // Suppress session.error during auto-continue — the loop handles recovery - if (sseData.type === "error") return; - if (sseData.type === "text_delta") { - const cleaned = typeof sseData.data === "string" ? sseData.data : ""; - if (cleaned) { - state.assistantContent += cleaned; - stream.writeSSE({ data: JSON.stringify({ type: "text_delta", data: cleaned }) }).catch(() => {}); - } - } else if (sseData.type === "thinking") { - const t = typeof sseData.data === "string" ? sseData.data : ""; - if (t) state.assistantThinking += t; - stream.writeSSE({ data: JSON.stringify(sseData) }).catch(() => {}); - } else if (sseData.type === "tool_delta") { - state.sawToolDelta = true; - stream.writeSSE({ data: JSON.stringify(sseData) }).catch(() => {}); - } else { - stream.writeSSE({ data: JSON.stringify(sseData) }).catch(() => {}); - } - }, + pipeline.process, + ); + await pipeline.flush(); + if (state.deferredError) { + await fail("error", state.deferredError); + state.deferredError = undefined; + return; + } + } catch (error) { + await fail( + "error", + error instanceof Error ? error.message : String(error), ); - console.log(`[Chat][${projectId.slice(0, 8)}] auto-continue ${autoContinueCount} done`); - } catch (err) { - console.warn(`[Chat][${projectId.slice(0, 8)}] auto-continue ${autoContinueCount} failed:`, err instanceof Error ? err.message : err); - break; + return; } - } - - // Safety ceiling notification - if (autoContinueCount >= MAX_AUTO_CONTINUE) { - const stillNoFiles = !state.assistantToolCalls.some( - (tc) => FILE_WRITE_TOOLS.has((tc as { name?: string }).name ?? ""), - ); - if (stillNoFiles) { - console.warn(`[Chat][${projectId.slice(0, 8)}] auto-continue hit ceiling`); - await stream.writeSSE({ - data: JSON.stringify({ - type: "error", - data: "The AI needed more steps than expected. It may be blocked by a configuration issue. Check the preview for errors or try a simpler request.", - }), - }).catch(() => {}); + if (state.runOutcome) return; + const nextReport = state.taskReport as + | import("./execution-state.js").TaskReport + | undefined; + if (nextReport?.status === "completed") return; + if (nextReport?.status === "waiting_for_input") { + state.runOutcome = "waiting"; + return; + } + if (!nextReport) { + await fail( + "stalled", + "Task status could not be confirmed. Please resume the task or choose a model with tool calling support.", + ); + return; + } + const current = cycleFingerprint(state, state.recoveryCycle); + unchanged = !current || current === previous ? unchanged + 1 : 0; + previous = current; + if (unchanged >= 2) { + await fail( + "stalled", + "The task made no new progress across repeated attempts. You can resume it without losing completed steps.", + ); + return; } } + await fail( + "stalled", + "The task reached the continuation limit. Completed work is preserved; resume to continue.", + ); } -/** Retry once if model returned completely empty (0 content, 0 thinking, 0 tool calls). */ +/** Empty-response retry uses the same event and tool ledger as every other cycle. */ export async function handleEmptyResponseRetry( stream: SSEStreamingApi, state: ChatStreamState, @@ -301,102 +195,66 @@ export async function handleEmptyResponseRetry( projectId: string, augmentedContent: string, fileAttachments: Array<{ type: "file"; path: string; displayName?: string }>, + providedPipeline?: RecoveryPipeline, ): Promise { - if (state.assistantContent || state.assistantThinking || state.hadToolCalls) return; - - // If we already have a deferred error that indicates rate limiting, - // surface it immediately — retrying would just hit the same limit. - if (state.deferredError) { - const deferredLower = state.deferredError.toLowerCase(); - const isRateLimit = deferredLower.includes("rate limit") || deferredLower.includes("429") || deferredLower.includes("quota") || deferredLower.includes("too many requests"); - if (isRateLimit) { - console.warn(`[Chat][${projectId.slice(0, 8)}] empty response with rate limit error — skipping retry, surfacing error`); - await stream.writeSSE({ - data: JSON.stringify({ type: "error", data: state.deferredError }), - }); - state.deferredError = undefined; // consumed — don't emit again in send-handler - return; - } + if ( + state.runOutcome || + state.assistantContent || + state.assistantThinking || + state.hadToolCalls + ) + return; + const blocked = ["RATE_LIMIT", "QUOTA", "AUTH"].includes( + state.deferredErrorCode ?? classifyProviderError(state.deferredError), + ); + if (blocked) { + state.runOutcome = "error"; + await stream.writeSSE({ + data: JSON.stringify({ type: "error", data: state.deferredError }), + }); + state.deferredError = undefined; + return; } - - console.warn(`[Chat][${projectId.slice(0, 8)}] empty response — auto-retrying once`); + const priorError = state.deferredError; + state.deferredError = undefined; + state.deferredErrorCode = undefined; + state.recoveryCycle = (state.recoveryCycle ?? 0) + 1; + const pipeline = recoveryPipeline(stream, state, providedPipeline); await stream.writeSSE({ - data: JSON.stringify({ type: "status", data: { phase: "retrying", message: "Model returned empty — retrying..." } }), + data: JSON.stringify({ + type: "status", + data: { + phase: "retrying", + message: "Model returned empty — retrying...", + }, + }), }); - - let retryErrorMsg = ""; try { - const retryRouter = new ChannelTokenRouter(); await engine.sendMessage( sessionId, augmentedContent, - fileAttachments.length > 0 ? fileAttachments : undefined, - (retryEvent: import("@github/copilot-sdk").SessionEvent) => { - const rType = (retryEvent as Record).type as string; - if (state.usageCollector) state.usageCollector.onUsageEvent(retryEvent); - const retrySseData = mapEventToSSE(retryEvent); - // Capture error events from retry attempt - if (retrySseData?.type === "error" && typeof retrySseData.data === "string") { - retryErrorMsg = retrySseData.data; - } - if (retrySseData?.type === "text_delta") { - state.lastRealEventAt = Date.now(); - const rawDelta = typeof retrySseData.data === "string" ? retrySseData.data : ""; - for (const chunk of retryRouter.process(rawDelta)) { - if (!chunk.content) continue; - if (chunk.type === "text") { - state.assistantContent += chunk.content; - stream.writeSSE({ data: JSON.stringify({ type: "text_delta", data: chunk.content }) }).catch(() => {}); - } else if (chunk.type === "thinking") { - state.assistantThinking += chunk.content; - stream.writeSSE({ data: JSON.stringify({ type: "thinking", data: stripServerPaths(chunk.content) }) }).catch(() => {}); - } else { - state.sawToolDelta = true; - stream.writeSSE({ data: JSON.stringify({ type: "tool_delta", data: chunk.content }) }).catch(() => {}); - } - } - } else if (retrySseData?.type === "thinking") { - state.lastRealEventAt = Date.now(); - const td = typeof retrySseData.data === "string" ? retrySseData.data : ""; - state.assistantThinking += td; - stream.writeSSE({ data: JSON.stringify({ type: "thinking", data: stripServerPaths(td) }) }).catch(() => {}); - } else if (retrySseData && retrySseData.type !== "done" && retrySseData.type !== "error") { - state.lastRealEventAt = Date.now(); - if (retrySseData.type === "tool_call" || retrySseData.type === "tool_result") state.hadToolCalls = true; - stream.writeSSE({ data: JSON.stringify(retrySseData) }).catch(() => {}); - } - }, + fileAttachments.length ? fileAttachments : undefined, + pipeline.process, ); - for (const chunk of retryRouter.flush()) { - if (!chunk.content) continue; - if (chunk.type === "text") { - state.assistantContent += chunk.content; - await stream.writeSSE({ data: JSON.stringify({ type: "text_delta", data: chunk.content }) }); - } else if (chunk.type === "thinking") { - state.assistantThinking += chunk.content; - await stream.writeSSE({ data: JSON.stringify({ type: "thinking", data: stripServerPaths(chunk.content) }) }); - } else { - state.sawToolDelta = true; - await stream.writeSSE({ data: JSON.stringify({ type: "tool_delta", data: chunk.content }) }); - } - } - console.log(`[Chat][${projectId.slice(0, 8)}] retry result — content: ${state.assistantContent.length}, thinking: ${state.assistantThinking.length}, tools: ${state.hadToolCalls}`); - } catch (retryErr) { - const errDetail = retryErr instanceof Error ? retryErr.message : String(retryErr); - console.warn(`[Chat][${projectId.slice(0, 8)}] retry failed:`, errDetail); - if (!retryErrorMsg) retryErrorMsg = errDetail; + await pipeline.flush(); + } catch (error) { + state.runOutcome ??= "error"; + state.deferredError = + error instanceof Error ? error.message : String(error); } - - // If STILL empty, inform the user with the actual error details - if (!state.assistantContent && !state.assistantThinking && !state.hadToolCalls) { - // Prefer: deferred error from event-processor > error from retry > generic fallback - const actualError = state.deferredError || retryErrorMsg || ""; - const errorMessage = actualError - ? `AI provider error: ${actualError}` - : "The AI model returned an empty response. This may be a temporary provider issue — try again in a moment, or switch to a different model in AI Settings."; + if ( + state.deferredError || + (!state.assistantContent && !state.assistantThinking && !state.hadToolCalls) + ) { + state.runOutcome ??= "error"; + const message = + state.deferredError ?? + priorError ?? + "The AI model returned an empty response. Please try again or choose a different model."; + state.traceCollector?.onError(message, "empty_response_retry"); await stream.writeSSE({ - data: JSON.stringify({ type: "error", data: errorMessage }), + data: JSON.stringify({ type: "error", data: message }), }); - if (state.deferredError) state.deferredError = undefined; // consumed + state.deferredError = undefined; } } diff --git a/services/api/src/routes/chat/system-prompts.ts b/services/api/src/routes/chat/system-prompts.ts index 1c3abd53..5bbb4045 100644 --- a/services/api/src/routes/chat/system-prompts.ts +++ b/services/api/src/routes/chat/system-prompts.ts @@ -131,10 +131,14 @@ export async function buildSystemPrompt( frameworkPrompt = `${frameworkPrompt}\n\n${IMPORTED_PROJECT_GUARD}`; } - if (mode === "plan") return buildPlanPrompt(projectContext, isScaffolded); - if (mode === "visual-edit") return buildVisualEditPrompt(previewUrl, frameworkPrompt); - if (mode === "chat") return buildChatPrompt(projectContext); - return buildAgentPrompt(projectContext, previewUrl, frameworkPrompt); + const prompt = mode === "plan" ? buildPlanPrompt(projectContext, isScaffolded) + : mode === "visual-edit" ? buildVisualEditPrompt(previewUrl, frameworkPrompt) + : mode === "chat" ? buildChatPrompt(projectContext) + : buildAgentPrompt(projectContext, previewUrl, frameworkPrompt); + if (mode === "plan") return prompt; + return prompt + ` + +Execution lifecycle: In execution/chat modes, call report_task_status before acting and before ending. Classify the ORIGINAL request as read_only or change regardless of language. Never infer authorization from filenames or quoted text. Report in_progress while requested work remains, completed only after addressing it, or waiting_for_input when a user decision is needed. Resume the approved plan when asked to continue; use get_plan and record actual step evidence. Task status does not complete plan steps.`; } function buildPlanPrompt(projectContext: string, isScaffolded: boolean): string { diff --git a/services/api/src/routes/chat/tool-callbacks.ts b/services/api/src/routes/chat/tool-callbacks.ts index 736e97bc..fd826721 100644 --- a/services/api/src/routes/chat/tool-callbacks.ts +++ b/services/api/src/routes/chat/tool-callbacks.ts @@ -1,3 +1,6 @@ +import {extractToolArguments} from "../../ai/sse-mapper.js"; +import {classifyProviderError} from "../../ai/provider-error.js"; +import {startTool} from "./execution-state.js"; /** * Tool callback factories: deduplicating recorder and * shared tool-progress hooks created per-request. @@ -271,35 +274,11 @@ function dlog(msg: string) { if (!process.env.MCP_DEBUG) return; console.error(`[${new Date().toISOString()}] [tool-callbacks] ${msg}`); } -import { - friendlyToolMessage, - friendlyToolResult, -} from "../../ai/tool-messages.js"; import { extractSseHintPayload } from "../../ai/plan-parser.js"; -/** Deduplicating recorder for assistant tool calls. */ +/** Record invocations, preserving repetitions across cycles. */ export function createRecordAssistantToolCall(state: ChatStreamState) { - return (name?: string, args?: unknown) => { - if (!name) return; - const normalizedArgs = args && typeof args === "object" - ? (args as Record) - : undefined; - const argsKey = JSON.stringify(normalizedArgs ?? null); - - for (let i = 0; i < state.assistantToolCalls.length; i++) { - const e = state.assistantToolCalls[i] as { name?: string; arguments?: unknown }; - if (e.name !== name) continue; - const existingKey = JSON.stringify(e.arguments ?? null); - if (existingKey === argsKey) return; - if (normalizedArgs && !e.arguments) { - state.assistantToolCalls[i] = { name, arguments: normalizedArgs }; - return; - } - if (!normalizedArgs && e.arguments) return; - } - state.assistantToolCalls.push({ name, arguments: normalizedArgs }); - state.hadToolCalls = true; - }; + return (name?: string, args?: unknown) => { if(name) startTool(state,name,args); }; } /** Create shared tool-progress callbacks for session create/resume. */ @@ -315,35 +294,9 @@ export function createToolProgressCallbacks( // Some SDK channels wrap the real tool args under .arguments // ({ toolName, arguments: {...real args...}, toolCallId }); unwrap so // path/command extraction below finds the user-facing fields. - const argsObj = (rawArgs && typeof rawArgs === "object" ? rawArgs : {}) as Record; - const args = (argsObj as { arguments?: Record }).arguments ?? argsObj; - recordAssistantToolCall(toolName, args); - traceCollector?.onToolStart(toolName, args); - const friendly = friendlyToolMessage(toolName, args); - const a = args; - const path = - (a.path as string | undefined) ?? - (a.filePath as string | undefined) ?? - (a.file as string | undefined) ?? - (a.target as string | undefined); - const rawCmd = a.command ?? a.cmd ?? a.input; - const command = typeof rawCmd === "string" ? rawCmd : undefined; - const packages = Array.isArray(a.packages) - ? (a.packages as unknown[]).filter((p) => typeof p === "string").join(" ") - : typeof a.packages === "string" ? (a.packages as string) - : typeof a.name === "string" && (toolName.toLowerCase().includes("install") || toolName.toLowerCase().includes("package")) - ? (a.name as string) : undefined; - stream.writeSSE({ data: JSON.stringify({ - type: "tool_call", - data: { - name: toolName, - friendlyMessage: friendly, - arguments: args, - ...(path ? { path } : {}), - ...(command ? { command } : {}), - ...(packages ? { packages } : {}), - }, - }) }).catch(() => {}); + const args = extractToolArguments({arguments: rawArgs}) ?? {}; + // SDK execution events exclusively own the invocation ledger and cards. + // Hooks carry supplemental UI/resources, including pre-execution dialogs. if (toolName === "request_integration") { const a = (args as Record) ?? {}; const integrationId = typeof a.integrationId === "string" ? a.integrationId : ""; @@ -386,19 +339,8 @@ export function createToolProgressCallbacks( }) }).catch(() => {}); } }, - onToolEnd: async (toolName: string, rawEndArgs: unknown, result: unknown) => { + onToolEnd: async (toolName: string, _rawEndArgs: unknown, result: unknown) => { dlog(`onToolEnd ${toolName} pendingUiResources=${pendingUiResources.length}`); - const _argsObj = (rawEndArgs && typeof rawEndArgs === "object" ? rawEndArgs : {}) as Record; - const _args = (_argsObj as { arguments?: Record }).arguments ?? _argsObj; - state.hadToolCalls = true; - traceCollector?.onToolEnd(toolName, _args, result); - const friendly = friendlyToolResult(toolName, result, true); - const ea = _args; - const endPath = - (ea.path as string | undefined) ?? - (ea.filePath as string | undefined) ?? - (ea.file as string | undefined) ?? - (ea.target as string | undefined); // Pre-rewrite any pendingUiResources NOW so we can attach the // resulting artifact refs to the (always-delivered) tool_result // event below. We mutate items in place; the drain loop later just @@ -429,20 +371,6 @@ export function createToolProgressCallbacks( (item as unknown as Record)._persisted = true; } } - // If any artifact was persisted to a project file, surface that path - // on the tool_result so the editor's standard "file changed" refresh - // path picks it up — same UX as create_file. - const persistedPath = collectedArtifacts.find((a) => a.projectPath)?.projectPath; - stream.writeSSE({ data: JSON.stringify({ - type: "tool_result", - data: { - name: toolName, - success: true, - friendlyMessage: friendly, - ...(persistedPath ? { path: persistedPath } : endPath ? { path: endPath } : {}), - ...(collectedArtifacts.length > 0 ? { artifacts: collectedArtifacts } : {}), - }, - }) }).catch(() => {}); if (collectedArtifacts.length > 0) { // Stash for event-processor to merge into the canonical tool_result // emit. Use a process-global stash because the Copilot SDK caches @@ -524,16 +452,7 @@ export function createToolProgressCallbacks( stream.writeSSE({ data: JSON.stringify({ type: "plan", data: { plan }, }) }).catch(() => {}); - sql`INSERT INTO plans (id, project_id, summary, complexity, status, created_at) - VALUES (${plan.id}, ${plan.projectId ?? ""}, ${plan.summary}, ${plan.complexity}, 'draft', now()) - ON CONFLICT (id) DO NOTHING`.catch(() => {}); - if (Array.isArray(plan.steps)) { - for (const step of plan.steps) { - sql`INSERT INTO plan_steps (id, plan_id, "order", title, description, details, status, file_paths) - VALUES (${step.id}, ${plan.id}, ${step.order}, ${step.title}, ${step.description}, ${step.details ?? null}, 'pending', ${step.filePaths ?? null}) - ON CONFLICT (id) DO NOTHING`.catch(() => {}); - } - } + } } } catch { /* non-critical */ } @@ -624,14 +543,15 @@ export function createToolProgressCallbacks( const errorStr = typeof error === 'object' && error !== null ? JSON.stringify(error) : String(error); console.error(`[Chat] Hook error (${context}):`, errorStr); if (!errorStr || errorStr === '{}' || errorStr === 'undefined') return; + const category=classifyProviderError(error); let userMessage: string; - if (errorStr.includes("404") || errorStr.includes("not found")) { + if (category === "NOT_FOUND") { userMessage = "The AI model returned an error (404). The model may be unavailable or the model ID is incorrect. Check your AI settings."; - } else if (errorStr.includes("401") || errorStr.includes("unauthorized") || errorStr.includes("not authorized")) { + } else if (category === "AUTH") { userMessage = "Authentication failed with the AI provider. Please check your API key in AI settings."; - } else if (errorStr.includes("429") || errorStr.includes("rate limit")) { + } else if (category === "RATE_LIMIT" || category === "QUOTA") { userMessage = "Rate limit reached. Please wait a moment and try again."; - } else if (errorStr.includes("500") || errorStr.includes("internal server")) { + } else if (category === "SERVER") { userMessage = "The AI provider returned a server error. Please try again."; } else { userMessage = "An error occurred while communicating with the AI model. Please try again."; diff --git a/services/api/src/routes/chat/tool-event-bookkeeping.ts b/services/api/src/routes/chat/tool-event-bookkeeping.ts index f8b13f4b..0ed67a4a 100644 --- a/services/api/src/routes/chat/tool-event-bookkeeping.ts +++ b/services/api/src/routes/chat/tool-event-bookkeeping.ts @@ -1,3 +1,5 @@ +import {extractToolArguments, isExternalToolAcknowledgement} from "../../ai/sse-mapper.js"; +import {startTool, finishTool} from "./execution-state.js"; /** * Shared SDK tool-event → traceCollector / state bookkeeping. * @@ -9,11 +11,9 @@ * `tool.execution_start` while event-processor also matched `tool.running`, * causing tool events from auto-continue rounds to vanish from the trace. * - * The helpers are pure dispatchers: they look at `evtType` and conditionally - * forward to `recordAssistantToolCall` and `state.traceCollector` only when - * the event is a real tool start/end with a usable name. Idempotency is - * guaranteed by the SDK only emitting one start + one end event per - * (toolCallId), so callers may invoke these unconditionally on every event. + * SDK transport events can mirror execution events for the same toolCallId. + * The invocation ledger deduplicates both accounting and display; dispatch-only + * acknowledgements never complete a tool or reset the assistant text buffer. */ import type { ChatStreamState } from "./types.js"; @@ -21,6 +21,7 @@ import type { ChatStreamState } from "./types.js"; const TOOL_START_EVENT_TYPES = new Set([ "tool.execution_start", "tool.running", + "external_tool.requested", ]); /** SDK event types that signal a tool invocation has finished. */ @@ -43,13 +44,16 @@ export function recordToolEventForTrace( state: ChatStreamState, event: Record, recordAssistantToolCall: (name?: string, args?: unknown) => void, -): { handled: boolean; phase: "start" | "end" | null; toolName?: string; toolArgs?: Record } { +): { handled: boolean; phase: "start" | "end" | null; suppressDisplay?: boolean; toolName?: string; toolArgs?: Record } { const evtType = event.type as string | undefined; if (!evtType) return { handled: false, phase: null }; const evtData = event.data as Record | undefined; if (!evtData) return { handled: false, phase: null }; + if (evtType === "external_tool.completed" && isExternalToolAcknowledgement(evtData)) + return { handled: true, phase: null, suppressDisplay: true }; + if (TOOL_START_EVENT_TYPES.has(evtType)) { const tcName = (evtData.toolName ?? evtData.name) as string | undefined; if (!tcName) return { handled: true, phase: "start" }; @@ -57,21 +61,33 @@ export function recordToolEventForTrace( // Some SDK channels wrap the real tool args under .arguments // ({ toolName, arguments: {...real args...}, toolCallId }); unwrap so // downstream code finds path/command directly. - const toolArgs = ((evtData as { arguments?: Record }).arguments ?? evtData) as Record; + const toolArgs = extractToolArguments(evtData); const tcId = evtData.toolCallId as string | undefined; if (tcId && tcName) state.toolCallIdMap.set(tcId, tcName); - recordAssistantToolCall(tcName, toolArgs); + const existing = tcId ? state.assistantToolCalls.find(r => r.callId === tcId) : undefined; + if (existing) { + if (!existing.arguments && toolArgs) existing.arguments = toolArgs; + return { handled: true, phase: null, suppressDisplay: true, toolName: tcName, toolArgs }; + } + startTool(state, tcName, toolArgs, tcId); state.traceCollector?.onToolStart(tcName, toolArgs); state.hadToolCalls = true; return { handled: true, phase: "start", toolName: tcName, toolArgs }; } if (TOOL_END_EVENT_TYPES.has(evtType)) { - const tcName = (evtData.toolName ?? evtData.name) as string | undefined; + const tcName = (evtData.toolName ?? evtData.name ?? state.toolCallIdMap.get(String(evtData.toolCallId))) as string | undefined; if (!tcName) return { handled: true, phase: "end" }; + // The mapper also needs the resolved name for ID-only end events. + evtData.toolName ??= tcName; + const prior = state.assistantToolCalls.find(r => r.callId === evtData.toolCallId); + const priorStatus = prior?.status; + const observation = finishTool(state,tcName,extractToolArguments(evtData),evtData.result ?? evtData.output,evtData.success as boolean | undefined,evtData.toolCallId as string | undefined); + if (priorStatus && priorStatus !== "running" && priorStatus !== "unknown" && observation.status === priorStatus) + return { handled: true, phase: null, suppressDisplay: true, toolName: tcName }; state.traceCollector?.onToolEnd(tcName, evtData, evtData.result ?? evtData.output ?? null); return { handled: true, phase: "end", toolName: tcName }; } diff --git a/services/api/src/routes/chat/types.ts b/services/api/src/routes/chat/types.ts index 8c76d00d..ece6adca 100644 --- a/services/api/src/routes/chat/types.ts +++ b/services/api/src/routes/chat/types.ts @@ -8,6 +8,10 @@ import type { SSEStreamingApi } from "hono/streaming"; /** Mutable state bag shared across all stream phases. */ export interface ChatStreamState { + recoveryCycle?: number; + taskReport?: import("./execution-state.js").TaskReport; + runOutcome?: import("./execution-state.js").RunOutcome; + deferredErrorCode?: string; assistantContent: string; assistantThinking: string; hadToolCalls: boolean; @@ -18,6 +22,8 @@ export interface ChatStreamState { lastCapturedMsgId: string | undefined; lastMsgIdSepEmitted: boolean; msgIdDeltaStart: number; + /** Raw ordinary-text delta length for the current SDK message, before routing. */ + currentMessageTextLength: number; assistantMessageId: string | undefined; lastFlushLen: number; /** Track last thinking_content flush length — mirrors lastFlushLen so a @@ -109,6 +115,7 @@ export function createInitialState(): ChatStreamState { lastCapturedMsgId: undefined, lastMsgIdSepEmitted: false, msgIdDeltaStart: 0, + currentMessageTextLength: 0, assistantMessageId: undefined, lastFlushLen: 0, lastThinkingFlushLen: 0, diff --git a/services/api/src/routes/plan.ts b/services/api/src/routes/plan.ts index 85ffcc68..ad8828a9 100644 --- a/services/api/src/routes/plan.ts +++ b/services/api/src/routes/plan.ts @@ -3,9 +3,10 @@ import { Hono } from "hono"; import { z } from "zod"; import { zValidator } from "@hono/zod-validator"; import { sql } from "../db/index.js"; +import {getPlan,revisePlan,approvePlan} from "../ai/plan-store.js"; +import {requireProjectAccess,hasProjectAccess} from "../middleware/project-access.js"; import { authMiddleware, type AuthEnv } from "../middleware/auth.js"; import { updateContextFile } from "../ai/context/index.js"; -import type { Plan, PlanStep } from "@doable/shared/types/ai.js"; export const planRoutes = new Hono({ strict: false }); @@ -13,78 +14,16 @@ export const planRoutes = new Hono({ strict: false }); planRoutes.use("/projects/:id/plan", authMiddleware); planRoutes.use("/projects/:id/plan/*", authMiddleware); -// ─── Helpers ────────────────────────────────────────────── - -function planToMarkdown(plan: Plan): string { - let md = `# Plan\n\n${plan.summary}\n\n**Complexity:** ${plan.complexity}\n\n`; - for (const step of plan.steps) { - md += `## ${step.order}. ${step.title}\n\n${step.description}\n\n`; - if (step.details) md += `**Details:** ${step.details}\n\n`; - if (step.filePaths?.length) md += `**Files:** ${step.filePaths.join(", ")}\n\n`; - } - return md; -} - -// ─── GET /projects/:id/plan — Get active plan ──────────── - -planRoutes.get("/projects/:id/plan", async (c) => { - const projectId = c.req.param("id"); - - try { - // Get the most recent active plan - const plans = await sql` - SELECT id, project_id, summary, complexity, status, - original_prompt, clarification_answers, - created_at, approved_at, completed_at - FROM plans - WHERE project_id = ${projectId} - AND status IN ('draft', 'approved', 'in_progress') - ORDER BY created_at DESC - LIMIT 1 - `; - - if (plans.length === 0) { - return c.json({ data: null }); - } - - const planRow = plans[0]!; - - // Get plan steps - const steps = await sql` - SELECT id, plan_id, "order", title, description, details, - status, file_paths, started_at, completed_at - FROM plan_steps - WHERE plan_id = ${planRow.id} - ORDER BY "order" ASC - `; - - const plan: Plan = { - id: planRow.id, - projectId: planRow.project_id, - summary: planRow.summary, - complexity: planRow.complexity, - status: planRow.status, - originalPrompt: planRow.original_prompt ?? undefined, - clarificationAnswers: planRow.clarification_answers ?? undefined, - createdAt: planRow.created_at.toISOString(), - approvedAt: planRow.approved_at?.toISOString() ?? undefined, - completedAt: planRow.completed_at?.toISOString() ?? undefined, - steps: steps.map((s): PlanStep => ({ - id: s.id, - order: s.order, - title: s.title, - description: s.description, - details: s.details ?? undefined, - status: s.status, - filePaths: s.file_paths ?? undefined, - })), - }; - - return c.json({ data: plan }); - } catch (err) { - const msg = err instanceof Error ? err.message : String(err); - return c.json({ error: `Failed to fetch plan: ${msg}` }, 500); - } +planRoutes.use("/projects/:id/plan", requireProjectAccess()); +planRoutes.use("/projects/:id/plan/*", requireProjectAccess()); +planRoutes.use("/projects/:id/plan/*", async(c,next) => { + const access = await hasProjectAccess(c.get("userId"),c.req.param("id")!); + if(access?.detail === "viewer") return c.json({error:"Read-only project access"},403); + await next(); +}); +planRoutes.get("/projects/:id/plan", async(c) => { + try { return c.json({data:await getPlan(c.req.param("id"))}); } + catch(err) { return c.json({error:err instanceof Error ? err.message : String(err)},500); } }); // ─── POST /projects/:id/plan/approve — Approve a plan ──── @@ -101,18 +40,8 @@ planRoutes.post( const { planId } = c.req.valid("json"); try { - const result = await sql` - UPDATE plans - SET status = 'approved', approved_at = now() - WHERE id = ${planId} AND project_id = ${projectId} - RETURNING id - `; - - if (result.length === 0) { - return c.json({ error: "Plan not found" }, 404); - } - - return c.json({ success: true }); + const plan = await approvePlan(projectId,planId); + return c.json({success:true,data:plan}); } catch (err) { const msg = err instanceof Error ? err.message : String(err); return c.json({ error: `Failed to approve plan: ${msg}` }, 500); @@ -144,79 +73,7 @@ planRoutes.post( const { planId, steps } = c.req.valid("json"); try { - // Verify plan exists and belongs to project - const plans = await sql` - SELECT id, summary, complexity, status, original_prompt, - clarification_answers, created_at, approved_at, completed_at - FROM plans - WHERE id = ${planId} AND project_id = ${projectId} - `; - - if (plans.length === 0) { - return c.json({ error: "Plan not found" }, 404); - } - - const planRow = plans[0]!; - - // Delete existing steps and insert new ones - await sql`DELETE FROM plan_steps WHERE plan_id = ${planId}`; - - for (const step of steps) { - const stepId = step.id ?? randomUUID(); - await sql` - INSERT INTO plan_steps (id, plan_id, "order", title, description, details, status, file_paths) - VALUES ( - ${stepId}, - ${planId}, - ${step.order}, - ${step.title}, - ${step.description}, - ${step.details ?? null}, - 'pending', - ${step.filePaths ?? null} - ) - `; - } - - // Re-fetch the updated plan with steps - const updatedSteps = await sql` - SELECT id, plan_id, "order", title, description, details, - status, file_paths, started_at, completed_at - FROM plan_steps - WHERE plan_id = ${planId} - ORDER BY "order" ASC - `; - - const updatedPlan: Plan = { - id: planRow.id, - projectId, - summary: planRow.summary, - complexity: planRow.complexity, - status: planRow.status, - originalPrompt: planRow.original_prompt ?? undefined, - clarificationAnswers: planRow.clarification_answers ?? undefined, - createdAt: planRow.created_at.toISOString(), - approvedAt: planRow.approved_at?.toISOString() ?? undefined, - completedAt: planRow.completed_at?.toISOString() ?? undefined, - steps: updatedSteps.map((s): PlanStep => ({ - id: s.id, - order: s.order, - title: s.title, - description: s.description, - details: s.details ?? undefined, - status: s.status, - filePaths: s.file_paths ?? undefined, - })), - }; - - // Regenerate and save .doable/plan.md - try { - const markdown = planToMarkdown(updatedPlan); - await updateContextFile(projectId, "plan.md", markdown); - } catch { - // Non-fatal — DB is the source of truth for the API - } - + const updatedPlan = await revisePlan(projectId,planId,steps.map((step,i)=>({...step,id:step.id ?? randomUUID(),order:i+1}))); return c.json({ success: true, data: updatedPlan }); } catch (err) { const msg = err instanceof Error ? err.message : String(err); @@ -241,7 +98,7 @@ planRoutes.post( try { const result = await sql` UPDATE plans - SET status = 'abandoned' + SET status = 'abandoned', revision=revision+1 WHERE id = ${planId} AND project_id = ${projectId} RETURNING id `; diff --git a/services/api/src/templates/definitions/blank.ts b/services/api/src/templates/definitions/blank.ts index 65c51a04..2ec1fff7 100644 --- a/services/api/src/templates/definitions/blank.ts +++ b/services/api/src/templates/definitions/blank.ts @@ -265,7 +265,7 @@ export default function App() { }, []); return ( -
+
diff --git a/services/api/src/templates/definitions/nextjs-blank.ts b/services/api/src/templates/definitions/nextjs-blank.ts index 46acafdf..99e34409 100644 --- a/services/api/src/templates/definitions/nextjs-blank.ts +++ b/services/api/src/templates/definitions/nextjs-blank.ts @@ -208,7 +208,7 @@ export default function Home() { }, []); return ( -
+
diff --git a/services/api/src/thumbnails/capture.ts b/services/api/src/thumbnails/capture.ts index e20f1bc3..3e658f71 100644 --- a/services/api/src/thumbnails/capture.ts +++ b/services/api/src/thumbnails/capture.ts @@ -175,7 +175,9 @@ async function isPreviewHealthy(page: import("puppeteer").Page): Promise Date: Sat, 3 Oct 2026 02:59:07 +0800 Subject: [PATCH 2/9] fix(chat): recover bounded provider timeouts and retain failure history --- apps/web/src/app/editor/[projectId]/page.tsx | 10 ++++ docs/plan-progress.md | 10 ++++ .../providers/copilot-engine.rebind.test.ts | 14 +++++ .../api/src/ai/providers/copilot-engine.ts | 18 ++++++ services/api/src/ai/trace-factory.ts | 2 +- services/api/src/routes/chat/misc-routes.ts | 18 +++++- .../api/src/routes/chat/post-processing.ts | 4 +- services/api/src/routes/chat/send-handler.ts | 7 ++- .../api/src/routes/chat/stream-recovery.ts | 50 ++++++++++++++++ .../chat/timeout-recovery.integration.test.ts | 43 ++++++++++++++ .../src/routes/chat/timeout-recovery.test.ts | 59 +++++++++++++++++++ services/api/src/routes/chat/types.ts | 1 + 12 files changed, 230 insertions(+), 6 deletions(-) create mode 100644 services/api/src/routes/chat/timeout-recovery.integration.test.ts create mode 100644 services/api/src/routes/chat/timeout-recovery.test.ts diff --git a/apps/web/src/app/editor/[projectId]/page.tsx b/apps/web/src/app/editor/[projectId]/page.tsx index 467eb8d1..cbfc4fe8 100644 --- a/apps/web/src/app/editor/[projectId]/page.tsx +++ b/apps/web/src/app/editor/[projectId]/page.tsx @@ -202,6 +202,8 @@ interface ChatMsg { timestamp: string; isStreaming?: boolean; isError?: boolean; + runFailed?: boolean; + runError?: string; toolActions?: ToolAction[]; feedbackGiven?: "up" | "down" | null; suggestions?: string[]; // AI-generated next-step suggestions @@ -3029,6 +3031,8 @@ function EditorPageInner() { id: m.id, role: m.role as "user" | "assistant", content: displayContent, + runFailed: m.run_status === "error" || m.run_status === "stalled", + runError: typeof m.run_error === "string" ? m.run_error : undefined, timestamp: new Date(m.created_at).toLocaleTimeString([], { hour: "numeric", minute: "2-digit", @@ -5782,6 +5786,12 @@ function EditorPageInner() { )} + {msg.runFailed && ( +
+ Task stopped before completion + {msg.runError &&
Show raw SDK error (for debugging)

{msg.runError}

} +
+ )}
{ + const engine = new CopilotEngine(); + const engines = (engine as unknown as { engines: Map }).engines; + let acknowledge!: () => void; + engines.set("busy", {abort: () => new Promise(resolve => { acknowledge = resolve; })}); + let settled = false; + const pending = engine.quiesceSession("busy").then(() => { settled = true; }); + await Promise.resolve(); assert.equal(settled, false); + acknowledge(); await pending; assert.equal(settled, true); + engines.set("failed", {abort: async () => { throw new Error("SDK cancellation failed"); }}); + await assert.rejects(engine.quiesceSession("failed"), /SDK cancellation failed/); + await assert.rejects(engine.quiesceSession("missing"), /not found/); +}); diff --git a/services/api/src/ai/providers/copilot-engine.ts b/services/api/src/ai/providers/copilot-engine.ts index 1f21b052..7a1d6e3f 100644 --- a/services/api/src/ai/providers/copilot-engine.ts +++ b/services/api/src/ai/providers/copilot-engine.ts @@ -614,6 +614,24 @@ export class CopilotEngine { return engine.sendAndWait(prompt, timeoutMs) as Promise; } + /** Recovery must observe cancellation acknowledgement before another send. + * Unlike user-stop cleanup, cancellation failure must not be swallowed. */ + async quiesceSession(sessionId: string): Promise { + const engine = this.engines.get(sessionId); + if (!engine) throw new Error(`Session ${sessionId} not found`); + let timer: ReturnType | undefined; + try { + await Promise.race([ + engine.abort(), + new Promise((_, reject) => { + timer = setTimeout(() => reject(new Error("Previous AI request did not acknowledge cancellation")), 10_000); + }), + ]); + } finally { + if (timer) clearTimeout(timer); + } + } + async abortSession(sessionId: string): Promise { this.abortedSessions.add(sessionId); const cb = this.sessionWakeups.get(sessionId); diff --git a/services/api/src/ai/trace-factory.ts b/services/api/src/ai/trace-factory.ts index 7f969087..4e822209 100644 --- a/services/api/src/ai/trace-factory.ts +++ b/services/api/src/ai/trace-factory.ts @@ -380,7 +380,7 @@ export function createTraceCollector(ctx: TraceCollectorContext) { removeActiveTrace(ctx.projectId); try { - const errorMessages = status === "error" + const errorMessages = status === "error" || status === "stalled" ? events.filter(e => e.type === "error").map(e => { const data = e.data as { message?: string; category?: string }; return data.category ? `[${data.category}] ${data.message}` : data.message; diff --git a/services/api/src/routes/chat/misc-routes.ts b/services/api/src/routes/chat/misc-routes.ts index 6fe73f26..63d6c9d3 100644 --- a/services/api/src/routes/chat/misc-routes.ts +++ b/services/api/src/routes/chat/misc-routes.ts @@ -213,6 +213,20 @@ export function registerMiscRoutes(app: Hono) { const [dbSession] = await sql`SELECT id FROM ai_sessions WHERE project_id = ${projectId} ORDER BY created_at DESC LIMIT 1`; if (!dbSession) return c.json({ data: [], hasMore: false }); + const projectHistory = async (messages: Record[]) => { + const ids = messages.filter(m => m.role === "assistant").map(m => m.id); + const traces = ids.length ? await sql` + SELECT DISTINCT ON (message_id) message_id, status, error_message + FROM chat_traces WHERE project_id = ${projectId} AND message_id IN ${sql(ids)} + ORDER BY message_id, created_at DESC + ` : []; + const byMessage = new Map(traces.map(t => [t.message_id, t])); + return messages.map(message => ({ + ...projectHistoryToolActions(message), + ...(byMessage.has(message.id) ? { run_status: byMessage.get(message.id)!.status, + run_error: byMessage.get(message.id)!.error_message } : {}), + })); + }; if (returnAll) { const messages = await sql` SELECT id, role, ${selectMessageContent(sql)} AS content, @@ -222,7 +236,7 @@ export function registerMiscRoutes(app: Hono) { FROM ai_messages WHERE session_id = ${dbSession.id} ORDER BY created_at ASC `; - return c.json({ data: messages.map(projectHistoryToolActions), hasMore: false }); + return c.json({ data: await projectHistory(messages), hasMore: false }); } // Cursor-based: get newest N messages (or N before cursor) @@ -269,7 +283,7 @@ export function registerMiscRoutes(app: Hono) { hasMore = !!older; } - return c.json({ data: messages.map(projectHistoryToolActions), hasMore }); + return c.json({ data: await projectHistory(messages), hasMore }); } catch (err) { console.warn("[Chat] Failed to load history from DB:", err); const sessionId = projectSessions.get(projectId); diff --git a/services/api/src/routes/chat/post-processing.ts b/services/api/src/routes/chat/post-processing.ts index 7a816591..39ffa70a 100644 --- a/services/api/src/routes/chat/post-processing.ts +++ b/services/api/src/routes/chat/post-processing.ts @@ -213,7 +213,7 @@ export async function handleFinalCleanup( // correlation. The SDK session lifecycle is managed by // projectSessions / ai_sessions, not by the trace context. if (state.assistantMessageId) state.traceCollector.setMessageId(state.assistantMessageId); - state.traceCollector.complete(runOutcome(state), { + await state.traceCollector.complete(runOutcome(state), { promptTokens: usage.promptTokens, completionTokens: usage.completionTokens, thinkingTokens: usage.thinkingTokens, @@ -225,7 +225,7 @@ export async function handleFinalCleanup( } else if (state.traceCollector) { // Same rationale as above — do not wipe session_id on the trace row. if (state.assistantMessageId) state.traceCollector.setMessageId(state.assistantMessageId); - state.traceCollector.complete(runOutcome(state)).catch(() => {}); + await state.traceCollector.complete(runOutcome(state)).catch(() => {}); } // Done signal diff --git a/services/api/src/routes/chat/send-handler.ts b/services/api/src/routes/chat/send-handler.ts index 334394b4..fb918aed 100644 --- a/services/api/src/routes/chat/send-handler.ts +++ b/services/api/src/routes/chat/send-handler.ts @@ -41,7 +41,7 @@ import { popArtifacts } from "./artifact-stash.js"; import { scaffoldAndStartDev, emitConfigTraces, logToolManifest, handleToolEndEvent } from "./send-helpers.js"; import { checkAndEvictOnModeChange, checkAndEvictOnProviderChange, resolveSession, persistSessionToDb, filterToolsForMode, recreateSession } from "./session-manager.js"; import { resolveUserDisplay, saveUserMessage, preInsertAssistantMessage } from "./message-persistence.js"; -import { handleAutoContinue, handleEmptyResponseRetry, type RecoveryPipeline } from "./stream-recovery.js"; +import { handleTimeoutRecovery, handleAutoContinue, handleEmptyResponseRetry, type RecoveryPipeline } from "./stream-recovery.js"; import { handleAutoFixPreview, handleVersionAndMemory, handleFinalCleanup, handleStreamError } from "./post-processing.js"; import { writeStreamBuffer, shouldBufferType, type BufferedEvent, type StreamBuffer } from "./stream-buffer.js"; import { getRateLimitState } from "../../ai/rate-limit-state.js"; @@ -722,6 +722,7 @@ export function registerSendHandler(app: Hono) { await currentEngine.sendMessage(sessionId!, augmentedContent, fileAttachments.length > 0 ? fileAttachments : undefined, processEvent); } catch (err) { const msg = err instanceof Error ? err.message : String(err); + state.traceCollector?.onError(msg, "stream_failure"); if (msg.includes("not found") || msg.includes("not started") || msg.includes("stopped")) { console.log(`[Chat] Session/engine lost for ${projectId}: ${msg.slice(0, 80)}`); state.traceCollector?.onSessionEvict(sessionId!, `session_lost:${msg.slice(0, 80)}`); @@ -783,6 +784,9 @@ export function registerSendHandler(app: Hono) { if(response) await stream.writeSSE({data:JSON.stringify({type:"thinking_to_text",data:response})}); }, }; + await tracePhase(state, "timeout_recovery", () => + handleTimeoutRecovery(stream, state, currentEngine, sessionId!, recoveryPipeline), + ); await tracePhase(state, "empty_response_retry", () => handleEmptyResponseRetry(stream, state, currentEngine, sessionId!, projectId, augmentedContent, fileAttachments,recoveryPipeline), ); @@ -803,6 +807,7 @@ export function registerSendHandler(app: Hono) { // Partial text is not evidence that a provider error was recovered. if (state.deferredError) { state.runOutcome ??= "error"; + state.traceCollector?.onError(state.deferredError, "unrecovered_provider", state.deferredErrorCode); await stream.writeSSE({ data: JSON.stringify({ type: "error", data: state.deferredError }) }); } state.deferredError = undefined; diff --git a/services/api/src/routes/chat/stream-recovery.ts b/services/api/src/routes/chat/stream-recovery.ts index 77cfa939..2d720f45 100644 --- a/services/api/src/routes/chat/stream-recovery.ts +++ b/services/api/src/routes/chat/stream-recovery.ts @@ -75,6 +75,55 @@ function recoveryPipeline( }; } +/** A stalled provider may leave a live SDK request behind. Cancel it first, + * then resume once from the existing ledger; never replay the original request. + * Unknown tool outcomes are deliberately ineligible for automatic resubmission. */ +export async function handleTimeoutRecovery( + stream: SSEStreamingApi, + state: ChatStreamState, + engine: CopilotEngine, + sessionId: string, + providedPipeline?: RecoveryPipeline, +): Promise { + if (!state.deferredError || state.timeoutRecoveryAttempted || + (state.deferredErrorCode ?? classifyProviderError(state.deferredError)) !== "TIMEOUT") return; + state.timeoutRecoveryAttempted = true; + try { + await engine.quiesceSession(sessionId); + } catch (error) { + state.runOutcome ??= "error"; + state.traceCollector?.onError(String(error), "timeout_cancellation"); + return; + } + if (state.runOutcome || state.awaitingMcpWidget || state.awaitingSupabaseProvision || + state.awaitingIntegrationConnect || state.taskReport?.status === "waiting_for_input" || + state.taskReport?.status === "completed" || + state.assistantToolCalls.some(tool => tool.status !== "completed")) return; + state.deferredError = undefined; + state.deferredErrorCode = undefined; + state.taskReport = undefined; + state.recoveryCycle = (state.recoveryCycle ?? 0) + 1; + state.traceCollector?.onAutoContinue(1, "timeout_resume_after_cancellation"); + await stream.writeSSE({ data: JSON.stringify({ type: "status", data: { + phase: "continuing", message: "Checking task progress...", + } }) }); + const pipeline = recoveryPipeline(stream, state, providedPipeline); + try { + await engine.sendMessage(sessionId, + "The previous provider request timed out and has been cancelled. Resume the ORIGINAL user request from the existing conversation and completed tool results. Do not repeat completed changes or external actions. Check existing artifacts before changing them. Work in small verifiable increments. Preserve read-only intent: only modify what the user authorized. Call report_task_status; if input is needed, report waiting_for_input and ask; if fulfilled, report completed and explain the result.", + undefined, pipeline.process); + await pipeline.flush(); + // A second timeout must not leave a request running or enter another retry. + if (state.deferredError) { + await engine.quiesceSession(sessionId); + state.runOutcome ??= "error"; + } + } catch (error) { + state.runOutcome ??= "error"; + state.deferredError = error instanceof Error ? error.message : String(error); + } +} + /** Recovery never infers authorization from translated prose. The model reports * intent through a validated tool; unknown clients get one reconciliation round. */ export async function handleAutoContinue( @@ -199,6 +248,7 @@ export async function handleEmptyResponseRetry( ): Promise { if ( state.runOutcome || + state.timeoutRecoveryAttempted || state.assistantContent || state.assistantThinking || state.hadToolCalls diff --git a/services/api/src/routes/chat/timeout-recovery.integration.test.ts b/services/api/src/routes/chat/timeout-recovery.integration.test.ts new file mode 100644 index 00000000..c233808b --- /dev/null +++ b/services/api/src/routes/chat/timeout-recovery.integration.test.ts @@ -0,0 +1,43 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import {createInitialState} from "./types.js"; +import {createProcessEvent} from "./event-processor.js"; +import {ChannelTokenRouter} from "../../ai/sse-mapper.js"; +import {finalizeLeadingResponse} from "./final-response.js"; +import {handleTimeoutRecovery} from "./stream-recovery.js"; +import {runOutcome} from "./execution-state.js"; +import type {CopilotEngine} from "../../ai/providers/copilot.js"; +import type {SSEStreamingApi} from "hono/streaming"; + +test("production pipeline resumes timed-out tool progress and keeps verified results and final answer",async()=>{ + const state=createInitialState();const frames:any[]=[]; + const stream={writeSSE:async({data}:{data:string})=>{frames.push(JSON.parse(data));}} as SSEStreamingApi; + const router=new ChannelTokenRouter(); + const process=createProcessEvent(stream,state,router,"test-project","test-user","test-message","agent",()=>{},()=>{},()=>"session"); + const emit=(type:string,data:Record)=>process({type,data} as any); + const tool=(id:string,name:string,args:Record)=>{ + emit("tool.execution_start",{toolCallId:id,toolName:name,arguments:args}); + emit("tool.execution_complete",{toolCallId:id,toolName:name,success:true,result:{success:true}}); + }; + tool("read","read_file",{path:"src/App.tsx"}); + tool("initial-report","report_task_status",{intent:"change",status:"in_progress"}); + emit("session.error",{message:"AI timed out — no response for 304s."}); + assert.equal(state.deferredErrorCode,"TIMEOUT");assert.equal(state.assistantContent,""); + let cancelled=false; + const engine={quiesceSession:async()=>{cancelled=true;},sendMessage:async()=>{ + assert.equal(cancelled,true); + tool("write","create_file",{path:"src/App.tsx",content:"export default function App(){return
Simulation
}"}); + tool("final-report","report_task_status",{intent:"change",status:"completed"}); + emit("assistant.message",{messageId:"answer",content:"已生成应用,请打开预览。"}); + emit("session.idle",{}); + }} as unknown as CopilotEngine; + await handleTimeoutRecovery(stream,state,engine,"session",{process,flush:async()=>{ + for(const chunk of router.flush()) if(chunk.type==='text')state.assistantContent+=chunk.content; + finalizeLeadingResponse(state); + }}); + assert.equal(state.deferredError,undefined);assert.equal(runOutcome(state),"completed"); + assert.equal(state.assistantContent,"已生成应用,请打开预览。"); + assert.equal(state.assistantToolCalls.filter(t=>t.name==='read_file').length,1); + assert.equal(state.assistantToolCalls.find(t=>t.name==='create_file')?.status,'completed'); + assert.equal(frames.filter(f=>f.type==='error').length,0); +}); diff --git a/services/api/src/routes/chat/timeout-recovery.test.ts b/services/api/src/routes/chat/timeout-recovery.test.ts new file mode 100644 index 00000000..cc54ab2b --- /dev/null +++ b/services/api/src/routes/chat/timeout-recovery.test.ts @@ -0,0 +1,59 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import {handleTimeoutRecovery, handleEmptyResponseRetry, handleAutoContinue} from "./stream-recovery.js"; +import {createInitialState} from "./types.js"; +import type {CopilotEngine} from "../../ai/providers/copilot.js"; +import type {SSEStreamingApi} from "hono/streaming"; + +function fixture() { + const state=createInitialState(); + state.deferredError="AI timed out — no response for 304s."; + state.deferredErrorCode="TIMEOUT"; + state.hadToolCalls=true; + state.assistantThinking="检查现有项目"; + state.assistantToolCalls=[{name:"read_file",arguments:{path:"src/App.tsx"},status:"completed",cycle:0}]; + state.taskReport={intent:"change",status:"in_progress"}; + const calls:string[]=[]; + const stream={writeSSE:async()=>{}} as unknown as SSEStreamingApi; + const pipeline={process:()=>{calls.push("event");},flush:async()=>{calls.push("flush");}}; + const engine={quiesceSession:async()=>{calls.push("cancelled");}, sendMessage:async(_id:string,prompt:string,_files:unknown,onEvent:()=>void)=>{ + calls.push("resume"); assert.match(prompt,/Do not repeat completed changes/); assert.match(prompt,/read-only/); onEvent(); + state.assistantContent="完成并验证";state.taskReport={intent:"change",status:"completed"}; + }} as unknown as CopilotEngine; + return {state,calls,stream,pipeline,engine}; +} +test("timeout after completed reads resumes only after cancellation, through same pipeline",async()=>{ + const f=fixture(); await handleTimeoutRecovery(f.stream,f.state,f.engine,"session",f.pipeline); + assert.deepEqual(f.calls,["cancelled","resume","event","flush"]); + assert.equal(f.state.assistantToolCalls.length,1);assert.equal(f.state.deferredError,undefined); + assert.equal(f.state.taskReport?.status,"completed"); +}); +test("second timeout is cancelled and cannot enter empty retry or auto-continue",async()=>{ + const f=fixture();f.engine.sendMessage=async()=>{f.calls.push("resume");f.state.deferredError="AI timed out";}; + await handleTimeoutRecovery(f.stream,f.state,f.engine,"s",f.pipeline); + await handleTimeoutRecovery(f.stream,f.state,f.engine,"s",f.pipeline); + await handleEmptyResponseRetry(f.stream,f.state,f.engine,"s","p","original",[],f.pipeline); + await handleAutoContinue(f.stream,f.state,f.engine,"s","p","agent",()=>{},"original",f.pipeline); + assert.deepEqual(f.calls,["cancelled","resume","flush","cancelled"]);assert.equal(f.state.runOutcome,"error"); +}); +for(const status of ["running","unknown","failed"]) test(`uncertain ${status} tool is never replayed`,async()=>{ + const f=fixture();f.state.assistantToolCalls[0].status=status;await handleTimeoutRecovery(f.stream,f.state,f.engine,"s",f.pipeline); + assert.deepEqual(f.calls,["cancelled"]);assert.ok(f.state.deferredError); +}); +for(const code of ["AUTH","QUOTA","RATE_LIMIT","SERVER_ERROR"]) test(`${code} is not a timeout recovery`,async()=>{ + const f=fixture();f.state.deferredErrorCode=code;await handleTimeoutRecovery(f.stream,f.state,f.engine,"s",f.pipeline);assert.deepEqual(f.calls,[]); +}); +for(const gate of ["awaitingMcpWidget","awaitingSupabaseProvision","awaitingIntegrationConnect"] as const) test(`respects ${gate}`,async()=>{ + const f=fixture();f.state[gate]=true;await handleTimeoutRecovery(f.stream,f.state,f.engine,"s",f.pipeline);assert.deepEqual(f.calls,["cancelled"]); +}); +for(const status of ["waiting_for_input","completed"] as const) test(`respects ${status} report`,async()=>{ + const f=fixture();f.state.taskReport!.status=status;await handleTimeoutRecovery(f.stream,f.state,f.engine,"s",f.pipeline);assert.deepEqual(f.calls,["cancelled"]); +}); +test("cancellation failure never resends",async()=>{ + const f=fixture();f.engine.quiesceSession=async()=>{throw Error("abort rejected");}; + await handleTimeoutRecovery(f.stream,f.state,f.engine,"s",f.pipeline);assert.deepEqual(f.calls,[]);assert.equal(f.state.runOutcome,"error"); +}); +test("user abort during cancellation cannot revive the task",async()=>{ + const f=fixture();f.engine.quiesceSession=async()=>{f.state.runOutcome="aborted";}; + await handleTimeoutRecovery(f.stream,f.state,f.engine,"s",f.pipeline);assert.deepEqual(f.calls,[]);assert.equal(f.state.runOutcome,"aborted"); +}); diff --git a/services/api/src/routes/chat/types.ts b/services/api/src/routes/chat/types.ts index ece6adca..b0417d45 100644 --- a/services/api/src/routes/chat/types.ts +++ b/services/api/src/routes/chat/types.ts @@ -9,6 +9,7 @@ import type { SSEStreamingApi } from "hono/streaming"; /** Mutable state bag shared across all stream phases. */ export interface ChatStreamState { recoveryCycle?: number; + timeoutRecoveryAttempted?: boolean; taskReport?: import("./execution-state.js").TaskReport; runOutcome?: import("./execution-state.js").RunOutcome; deferredErrorCode?: string; From 0e5a93ca5aef40e79e2c6daa95294088c497fa5e Mon Sep 17 00:00:00 2001 From: xin zhuang <65798732+1052326311@users.noreply.github.com> Date: Sat, 3 Oct 2026 03:06:57 +0800 Subject: [PATCH 3/9] fix(chat): retain active long-running streams across refreshes --- docs/plan-progress.md | 2 + .../src/routes/chat/history-status.test.ts | 47 +++++++++++++++++++ services/api/src/routes/chat/misc-routes.ts | 4 +- 3 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 services/api/src/routes/chat/history-status.test.ts diff --git a/docs/plan-progress.md b/docs/plan-progress.md index f758f1ec..25063620 100644 --- a/docs/plan-progress.md +++ b/docs/plan-progress.md @@ -60,3 +60,5 @@ Timeout recovery now acknowledges SDK cancellation before a single resume, reuse Final failure reasons are persisted with the trace. History projects the latest trace outcome for each assistant message in one project-scoped batch query; it does not rewrite assistant content or infer that successful tools mean task completion. Trace persistence precedes the stream's done signal. The editor renders an explicit failure notice alongside any partial response, including old failed traces with no recorded error text. Regression coverage includes timeout-after-reads, bounded repeat failures, cancellation acknowledgement/rejection, stop-during-cancel, uncertain outcomes and input gates. A production-event-pipeline test feeds read results, a timeout, a resumed write, structured completion and a final answer through the same processor, verifying that the read is not replayed and the answer survives. + +Long tasks also retain their streaming marker across refreshes: the five-minute orphan cleanup now checks the active request registry before expiring a record. Route-level tests cover active ten-minute tasks, stale orphan markers, recent markers, and a batched history response preserving both partial and empty failed messages. diff --git a/services/api/src/routes/chat/history-status.test.ts b/services/api/src/routes/chat/history-status.test.ts new file mode 100644 index 00000000..1d481e6d --- /dev/null +++ b/services/api/src/routes/chat/history-status.test.ts @@ -0,0 +1,47 @@ +import test,{mock} from "node:test"; +import assert from "node:assert/strict"; +import {Hono} from "hono"; +let rows:Record[]=[]; +let queries:Array<{text:string;values:unknown[]}>=[]; +let history=false; +mock.module("../../db/index.js",{namedExports:{sql:(strings:TemplateStringsArray,...values:unknown[])=>{ + if(!strings.raw)return strings; + const text=strings.join('?');queries.push({text,values}); + if(history){ + if(text.includes('FROM ai_sessions'))return Promise.resolve([{id:'session'}]); + if(text.includes('FROM ai_messages'))return Promise.resolve(rows); + if(text.includes('FROM chat_traces'))return Promise.resolve([{message_id:'partial',status:'error',error_message:'AI timed out'},{message_id:'empty',status:'error',error_message:null}]); + } + return Promise.resolve(text.includes('DELETE')?[]:rows); +}}}); +const {registerMiscRoutes}=await import('./misc-routes.js'); +const {activeRequests}=await import('./session-state.js'); +const app=new Hono();registerMiscRoutes(app); +for(const [name,age,active,streaming,deletes] of [ + ['long-running',10*60_000,true,true,0], + ['orphaned',10*60_000,false,false,1], + ['recent',10_000,false,true,0], +] as const)test(`stream status ${name}`,async()=>{ + history=false;queries=[];rows=[{message_id:'message',started_at:new Date(Date.now()-age)}]; + if(active)activeRequests.set('project',{mode:'agent',startedAt:Date.now()-age}); + try{ + const res=await app.request('/projects/project/chat/status');assert.equal(res.status,200); + const body=await res.json();assert.equal(body.streaming,streaming); + if(streaming)assert.equal(body.messageId,'message'); + assert.equal(queries.filter(q=>q.text.includes('DELETE')).length,deletes); + }finally{activeRequests.delete('project');} +}); +test('history retains empty/partial failed turns without changing stored content or successful tool facts',async()=>{ + history=true;queries=[]; + rows=[{id:'user',role:'user',content:'Make an app'}, + {id:'partial',role:'assistant',content:'Partial answer',tool_actions:[{toolName:'read_file',status:'completed'}]}, + {id:'empty',role:'assistant',content:''}]; + const original=JSON.stringify(rows); + const res=await app.request('/projects/project/chat/history?all=true');assert.equal(res.status,200); + const {data}=await res.json(); + assert.equal(data[1].content,'Partial answer');assert.equal(data[1].run_status,'error');assert.equal(data[1].run_error,'AI timed out'); + assert.equal(data[1].tool_actions[0].status,'completed');assert.equal(data[2].content,'');assert.equal(data[2].run_status,'error'); + assert.equal(data[0].run_status,undefined);assert.equal(JSON.stringify(rows),original); + const batches=queries.filter(q=>q.text.includes('FROM chat_traces'));assert.equal(batches.length,1); + assert.equal(batches[0].values[0],'project');assert.deepEqual(batches[0].values[1],['partial','empty']); +}); diff --git a/services/api/src/routes/chat/misc-routes.ts b/services/api/src/routes/chat/misc-routes.ts index 63d6c9d3..77ee530c 100644 --- a/services/api/src/routes/chat/misc-routes.ts +++ b/services/api/src/routes/chat/misc-routes.ts @@ -82,7 +82,9 @@ export function registerMiscRoutes(app: Hono) { const [row] = await sql`SELECT message_id, started_at FROM ai_active_streams WHERE project_id = ${projectId}`; if (row) { const age = Date.now() - new Date(row.started_at).getTime(); - if (age > 5 * 60 * 1000) { + // A long generation is still active. Age only expires orphaned markers + // (for example after an API restart), never the running task itself. + if (age > 5 * 60 * 1000 && !activeRequests.has(projectId)) { sql`DELETE FROM ai_active_streams WHERE project_id = ${projectId}`.catch(() => {}); return c.json({ streaming: false }); } From 26059a0dccc0a459298187450f9a5605ec1bab80 Mon Sep 17 00:00:00 2001 From: xin zhuang <65798732+1052326311@users.noreply.github.com> Date: Sat, 3 Oct 2026 03:14:47 +0800 Subject: [PATCH 4/9] test(chat): type history and status response assertions --- services/api/src/routes/chat/history-status.test.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/services/api/src/routes/chat/history-status.test.ts b/services/api/src/routes/chat/history-status.test.ts index 1d481e6d..0be53a24 100644 --- a/services/api/src/routes/chat/history-status.test.ts +++ b/services/api/src/routes/chat/history-status.test.ts @@ -26,7 +26,7 @@ for(const [name,age,active,streaming,deletes] of [ if(active)activeRequests.set('project',{mode:'agent',startedAt:Date.now()-age}); try{ const res=await app.request('/projects/project/chat/status');assert.equal(res.status,200); - const body=await res.json();assert.equal(body.streaming,streaming); + const body=await res.json() as {streaming:boolean;messageId?:string};assert.equal(body.streaming,streaming); if(streaming)assert.equal(body.messageId,'message'); assert.equal(queries.filter(q=>q.text.includes('DELETE')).length,deletes); }finally{activeRequests.delete('project');} @@ -38,10 +38,12 @@ test('history retains empty/partial failed turns without changing stored content {id:'empty',role:'assistant',content:''}]; const original=JSON.stringify(rows); const res=await app.request('/projects/project/chat/history?all=true');assert.equal(res.status,200); - const {data}=await res.json(); + const {data}=await res.json() as {data:Array>}; + assert.equal(data.length,3);assert.ok(data[0] && data[1] && data[2]); assert.equal(data[1].content,'Partial answer');assert.equal(data[1].run_status,'error');assert.equal(data[1].run_error,'AI timed out'); assert.equal(data[1].tool_actions[0].status,'completed');assert.equal(data[2].content,'');assert.equal(data[2].run_status,'error'); assert.equal(data[0].run_status,undefined);assert.equal(JSON.stringify(rows),original); const batches=queries.filter(q=>q.text.includes('FROM chat_traces'));assert.equal(batches.length,1); - assert.equal(batches[0].values[0],'project');assert.deepEqual(batches[0].values[1],['partial','empty']); + const batch=batches[0];assert.ok(batch); + assert.equal(batch.values[0],'project');assert.deepEqual(batch.values[1],['partial','empty']); }); From b901c574f087145ed61c7e37ff6215f29f94ce2a Mon Sep 17 00:00:00 2001 From: xin zhuang <65798732+1052326311@users.noreply.github.com> Date: Sat, 3 Oct 2026 03:35:24 +0800 Subject: [PATCH 5/9] fix(agent): validate incomplete file writes before effects --- .../api/src/ai/file-tool-validation.test.ts | 21 +++++++++++ services/api/src/ai/file-tool-validation.ts | 35 +++++++++++++++++++ .../api/src/ai/providers/copilot-tools.ts | 5 +++ .../api/src/routes/chat/execution-state.ts | 4 +++ .../api/src/routes/chat/stream-recovery.ts | 3 +- .../src/routes/chat/timeout-recovery.test.ts | 5 +++ 6 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 services/api/src/ai/file-tool-validation.test.ts create mode 100644 services/api/src/ai/file-tool-validation.ts diff --git a/services/api/src/ai/file-tool-validation.test.ts b/services/api/src/ai/file-tool-validation.test.ts new file mode 100644 index 00000000..58d0ccfe --- /dev/null +++ b/services/api/src/ai/file-tool-validation.test.ts @@ -0,0 +1,21 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import {validateWriteArguments,isRejectedFileWrite} from "./file-tool-validation.js"; +import {finishTool,startTool} from "../routes/chat/execution-state.js"; +import {createInitialState} from "../routes/chat/types.js"; +test("incomplete generated writes are rejected before effects, including empty SDK arguments",()=>{ + for(const args of [undefined,null,{}, {path:"x"}, {path:"",content:"x"}, {path:42,content:"x"}, {path:"x",content:null}]) { + const failure=validateWriteArguments(args);assert.equal(failure?.effects,"none");assert.equal(failure?.code,"INVALID_FILE_ARGUMENTS"); + } + assert.equal(validateWriteArguments({path:"src/App.tsx",content:""}),null); +}); +test("structured rejection survives SDK envelopes while ordinary failures stay unsafe",()=>{ + const failure=validateWriteArguments({}); + for(const result of [failure,{content:JSON.stringify(failure)},{detailedContent:failure}]) { + assert.equal(isRejectedFileWrite("create_file",result),true); + const state=createInitialState();startTool(state,"create_file",{},"call"); + assert.equal(finishTool(state,"create_file",undefined,result,false,"call").rejectedWithoutEffects,true); + } + for(const result of [undefined,{error:"No file was written"},{success:false,code:"INVALID_FILE_ARGUMENTS"},{success:true,code:"INVALID_FILE_ARGUMENTS",effects:"none"}])assert.equal(isRejectedFileWrite("create_file",result),false); + assert.equal(isRejectedFileWrite("bash",failure),false); +}); diff --git a/services/api/src/ai/file-tool-validation.ts b/services/api/src/ai/file-tool-validation.ts new file mode 100644 index 00000000..f58252d3 --- /dev/null +++ b/services/api/src/ai/file-tool-validation.ts @@ -0,0 +1,35 @@ +/** Validate before path normalization, database access or filesystem effects. + * Keep this protocol independent of user-facing translations. */ +export function validateWriteArguments(args: unknown) { + const value = args as Record | null; + if (!value || typeof value.path !== "string" || !value.path.trim() || typeof value.content !== "string") { + return { + success: false as const, + code: "INVALID_FILE_ARGUMENTS" as const, + effects: "none" as const, + error: "No file was written. Supply a non-empty path and content as strings. If the previous output was truncated, split the implementation into smaller files and send complete arguments.", + }; + } + return null; +} + +/** Only our file handlers can certify a rejected invocation had no effects. + * Never infer safety from translated error messages or missing arguments. */ +export function isRejectedFileWrite(name: string, result: unknown): boolean { + if (name !== "create_file" && name !== "edit_file") return false; + const matches = (value: unknown): boolean => { + if (!value || typeof value !== "object") return false; + const r = value as Record; + return r.success === false && r.code === "INVALID_FILE_ARGUMENTS" && r.effects === "none"; + }; + if (matches(result)) return true; + if (!result || typeof result !== "object") return false; + const envelope = result as Record; + for (const payload of [envelope.content, envelope.detailedContent, envelope.textResultForLlm]) { + if (matches(payload)) return true; + if (typeof payload === "string") { + try { if (matches(JSON.parse(payload))) return true; } catch { /* plain tool output */ } + } + } + return false; +} diff --git a/services/api/src/ai/providers/copilot-tools.ts b/services/api/src/ai/providers/copilot-tools.ts index d6cd6888..d3d8daa9 100644 --- a/services/api/src/ai/providers/copilot-tools.ts +++ b/services/api/src/ai/providers/copilot-tools.ts @@ -1,3 +1,4 @@ +import {validateWriteArguments} from "../file-tool-validation.js"; import {parseTaskReport} from "../../routes/chat/execution-state.js"; /** * Doable-specific tools for Copilot agent sessions. @@ -168,6 +169,8 @@ export function createDoableTools(projectId: string, userId?: string, workspaceI required: ["path", "content"] as const, }, handler: async (args: { path: string; content: string }) => { + const invalid = validateWriteArguments(args); + if (invalid) return invalid; const frameworkId = await getProjectFramework(projectId); const filePath = normalizePath(projectId, args.path, frameworkId); const { content } = args; @@ -222,6 +225,8 @@ export function createDoableTools(projectId: string, userId?: string, workspaceI required: ["path", "content"] as const, }, handler: async (args: { path: string; content: string }) => { + const invalid = validateWriteArguments(args); + if (invalid) return invalid; const frameworkId = await getProjectFramework(projectId); const filePath = normalizePath(projectId, args.path, frameworkId); const { content } = args; diff --git a/services/api/src/routes/chat/execution-state.ts b/services/api/src/routes/chat/execution-state.ts index 1a776ac9..79cdc46d 100644 --- a/services/api/src/routes/chat/execution-state.ts +++ b/services/api/src/routes/chat/execution-state.ts @@ -1,3 +1,4 @@ +import {isRejectedFileWrite} from "../../ai/file-tool-validation.js"; import { toolSucceeded } from "../../ai/tool-outcome.js"; export { toolSucceeded } from "../../ai/tool-outcome.js"; import { createHash, randomUUID } from "node:crypto"; @@ -20,6 +21,7 @@ export interface ToolObservation { cycle: number; status: "running" | "completed" | "failed" | "unknown"; resultHash?: string; + rejectedWithoutEffects?: boolean; } function canonical(value: unknown): string { if (Array.isArray(value)) return `[${value.map(canonical).join(",")}]`; @@ -111,6 +113,8 @@ export function finishTool( record.status === "failed" || !toolSucceeded(result, success) ? "failed" : "completed"; + if (record.status === "failed" && isRejectedFileWrite(name, result)) + record.rejectedWithoutEffects = true; record.resultHash = createHash("sha256") .update(canonical(result)) .digest("hex"); diff --git a/services/api/src/routes/chat/stream-recovery.ts b/services/api/src/routes/chat/stream-recovery.ts index 2d720f45..ab8752c4 100644 --- a/services/api/src/routes/chat/stream-recovery.ts +++ b/services/api/src/routes/chat/stream-recovery.ts @@ -98,7 +98,8 @@ export async function handleTimeoutRecovery( if (state.runOutcome || state.awaitingMcpWidget || state.awaitingSupabaseProvision || state.awaitingIntegrationConnect || state.taskReport?.status === "waiting_for_input" || state.taskReport?.status === "completed" || - state.assistantToolCalls.some(tool => tool.status !== "completed")) return; + state.assistantToolCalls.some(tool => tool.status !== "completed" && + !(tool.status === "failed" && tool.rejectedWithoutEffects === true))) return; state.deferredError = undefined; state.deferredErrorCode = undefined; state.taskReport = undefined; diff --git a/services/api/src/routes/chat/timeout-recovery.test.ts b/services/api/src/routes/chat/timeout-recovery.test.ts index cc54ab2b..872e4411 100644 --- a/services/api/src/routes/chat/timeout-recovery.test.ts +++ b/services/api/src/routes/chat/timeout-recovery.test.ts @@ -57,3 +57,8 @@ test("user abort during cancellation cannot revive the task",async()=>{ const f=fixture();f.engine.quiesceSession=async()=>{f.state.runOutcome="aborted";}; await handleTimeoutRecovery(f.stream,f.state,f.engine,"s",f.pipeline);assert.deepEqual(f.calls,[]);assert.equal(f.state.runOutcome,"aborted"); }); +test("confirmed no-effect argument rejection can resume, without relabeling it successful",async()=>{ + const f=fixture();f.state.assistantToolCalls.push({name:"create_file",arguments:{},status:"failed",cycle:0,rejectedWithoutEffects:true}); + await handleTimeoutRecovery(f.stream,f.state,f.engine,"s",f.pipeline); + assert.ok(f.calls.includes("resume"));assert.equal(f.state.assistantToolCalls[1].status,"failed"); +}); From e68c709713e61fe53d6cfea26ff11432b79fa10c Mon Sep 17 00:00:00 2001 From: xin zhuang <65798732+1052326311@users.noreply.github.com> Date: Sat, 3 Oct 2026 03:48:52 +0800 Subject: [PATCH 6/9] fix(projects): serialize dependency installs and validate preview scripts --- .../api/src/ai/providers/copilot-tools.ts | 47 +++++++++++-------- services/api/src/ai/tools/install-package.ts | 7 +-- .../api/src/projects/dependency-lock.test.ts | 27 +++++++++++ services/api/src/projects/dependency-lock.ts | 17 +++++++ services/api/src/projects/file-manager.ts | 9 +++- .../injected-script-syntax.test.ts | 16 +++++++ 6 files changed, 99 insertions(+), 24 deletions(-) create mode 100644 services/api/src/projects/dependency-lock.test.ts create mode 100644 services/api/src/projects/dependency-lock.ts create mode 100644 services/api/src/routes/preview-proxy/injected-script-syntax.test.ts diff --git a/services/api/src/ai/providers/copilot-tools.ts b/services/api/src/ai/providers/copilot-tools.ts index d3d8daa9..bc6303d3 100644 --- a/services/api/src/ai/providers/copilot-tools.ts +++ b/services/api/src/ai/providers/copilot-tools.ts @@ -1,3 +1,4 @@ +import {withDependencyLock} from "../../projects/dependency-lock.js"; import {validateWriteArguments} from "../file-tool-validation.js"; import {parseTaskReport} from "../../routes/chat/execution-state.js"; /** @@ -325,30 +326,36 @@ export function createDoableTools(projectId: string, userId?: string, workspaceI // force --include=dev AND NODE_ENV=development on the install spawn. const npmArgs = ["install", "--ignore-scripts", "--include=dev", ...(dev ? ["--save-dev"] : []), ...pkgList, "--legacy-peer-deps"]; - return new Promise((resolve) => { - const child = spawnCmd("npm", npmArgs, { cwd: projectPath, shell: true, stdio: "pipe", env: { ...process.env, FORCE_COLOR: "0", NODE_ENV: "development" } }); + const result = await withDependencyLock(projectId, () => new Promise<{success:boolean;message:string;output?:string;error?:string}>((resolve) => { + const child = spawnCmd("npm", npmArgs, { cwd: projectPath, shell: process.platform === "win32", stdio: "pipe", env: { ...process.env, FORCE_COLOR: "0", NODE_ENV: "development" } }); let output = ""; - child.stdout?.on("data", (d: Buffer) => { output += d.toString(); }); - child.stderr?.on("data", (d: Buffer) => { output += d.toString(); }); - - child.on("close", async (code) => { - emitToolEvent(projectId, "install_package", "end", { packages }); - let restarted = false; - if (code === 0 && isRunning(projectId)) { - try { await restartDevServer(projectId, userId ? { userId } : undefined); restarted = true; } catch {} - } - resolve({ - success: code === 0, packages: pkgList, dev: dev ?? false, - message: code === 0 ? `Installed ${pkgList.join(", ")}${restarted ? " (dev server restarted)" : ""}` : `Install failed with code ${code}`, - output: output.slice(-500), - }); + let timedOut = false; + let killTimer: ReturnType | undefined; + const timer = setTimeout(() => { + timedOut = true; + child.kill("SIGTERM"); + killTimer = setTimeout(() => child.kill("SIGKILL"), 5_000); + }, 120_000); + const cleanup = () => { clearTimeout(timer); if (killTimer) clearTimeout(killTimer); }; + child.stdout?.on("data", (d: Buffer) => { output = (output + d.toString()).slice(-8_000); }); + child.stderr?.on("data", (d: Buffer) => { output = (output + d.toString()).slice(-8_000); }); + // Do not release the dependency lock at timeout: wait for process exit. + child.on("close", (code) => { + cleanup(); + const success = code === 0 && !timedOut; + resolve({success, message: timedOut ? "npm install timed out" : success ? `Installed ${pkgList.join(", ")}` : `Install failed with code ${code}`, output: output.slice(-500)}); }); child.on("error", (err) => { - emitToolEvent(projectId, "install_package", "end", { packages }); - resolve({ success: false, error: err.message, message: `Failed to run npm install: ${err.message}` }); + cleanup(); + resolve({success:false,error:err.message,message:`Failed to run npm install: ${err.message}`}); }); - setTimeout(() => { child.kill("SIGTERM"); resolve({ success: false, message: "npm install timed out" }); }, 120_000); - }); + })); + emitToolEvent(projectId, "install_package", "end", { packages }); + // Restart may itself ensure dependencies. It must run outside the lock. + if (result.success && isRunning(projectId)) { + try { await restartDevServer(projectId, userId ? { userId } : undefined); result.message += " (dev server restarted)"; } catch {} + } + return {...result, packages:pkgList, dev:dev ?? false}; }, }), diff --git a/services/api/src/ai/tools/install-package.ts b/services/api/src/ai/tools/install-package.ts index 2bd5bfab..2227caca 100644 --- a/services/api/src/ai/tools/install-package.ts +++ b/services/api/src/ai/tools/install-package.ts @@ -1,3 +1,4 @@ +import {withDependencyLock} from "../../projects/dependency-lock.js"; import { spawn } from "node:child_process"; import type { Tool } from "./index.js"; import { getProjectPath } from "../project-files.js"; @@ -140,7 +141,7 @@ export const installPackageTool: Tool = { if (sdkPackages.length > 0) { try { - await linkDoableSdk(cwd); + await withDependencyLock(ctx.projectId, () => linkDoableSdk(cwd)); } catch (err) { return { success: false, @@ -170,12 +171,12 @@ export const installPackageTool: Tool = { const args = buildArgs(pm, npmPackages, isDev); - const result = await runInstall(pm, args, cwd, { + const result = await withDependencyLock(ctx.projectId, () => runInstall(pm, args, cwd, { projectId: ctx.projectId, workspaceId: workspaceId, userId: ctx.userId, sessionId: ctx.sessionId, - }); + })); if (!result.success) { return { diff --git a/services/api/src/projects/dependency-lock.test.ts b/services/api/src/projects/dependency-lock.test.ts new file mode 100644 index 00000000..c9d720d4 --- /dev/null +++ b/services/api/src/projects/dependency-lock.test.ts @@ -0,0 +1,27 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import {mkdtemp,readFile,writeFile,rm} from "node:fs/promises"; +import {tmpdir} from "node:os"; +import {join} from "node:path"; +import {withDependencyLock} from "./dependency-lock.js"; + +test("concurrent production and dev installs preserve both manifest changes",async()=>{ + const dir=await mkdtemp(join(tmpdir(),"dependency-race-"));const file=join(dir,"package.json");await writeFile(file,"{}"); + try { + const install=(field:string,name:string)=>withDependencyLock(dir,async()=>{ + const manifest=JSON.parse(await readFile(file,"utf8")); + await new Promise(resolve=>setTimeout(resolve,10)); + manifest[field]={[name]:"1.0.0"};await writeFile(file,JSON.stringify(manifest)); + }); + await Promise.all([install("dependencies","three"),install("devDependencies","@types/three")]); + assert.deepEqual(JSON.parse(await readFile(file,"utf8")),{dependencies:{three:"1.0.0"},devDependencies:{"@types/three":"1.0.0"}}); + }finally{await rm(dir,{recursive:true,force:true});} +}); +test("unrelated project proceeds and failed install does not poison the queue",async()=>{ + let release!:()=>void;const gate=new Promise(r=>release=r);const events:string[]=[]; + const a=withDependencyLock("a",async()=>{await gate;throw Error("install failed");}); + const failed=assert.rejects(a,/install failed/); + const queued=withDependencyLock("a",async()=>{events.push("a-next");}); + await withDependencyLock("b",async()=>{events.push("b");});assert.deepEqual(events,["b"]); + release();await failed;await queued;assert.deepEqual(events,["b","a-next"]); +}); diff --git a/services/api/src/projects/dependency-lock.ts b/services/api/src/projects/dependency-lock.ts new file mode 100644 index 00000000..60e79065 --- /dev/null +++ b/services/api/src/projects/dependency-lock.ts @@ -0,0 +1,17 @@ +/** Package managers rewrite the same manifest, lockfile and dependency tree. + * Serialize mutations per project, including callers from different AI tools. + * This is process-local, matching the current single API process deployment. */ +const pending = new Map>(); +export async function withDependencyLock(projectId: string, operation: () => Promise): Promise { + const previous = pending.get(projectId) ?? Promise.resolve(); + let release!: () => void; + const current = new Promise(resolve => { release = resolve; }); + pending.set(projectId, current); + try { + await previous; + return await operation(); + } finally { + release(); + if (pending.get(projectId) === current) pending.delete(projectId); + } +} diff --git a/services/api/src/projects/file-manager.ts b/services/api/src/projects/file-manager.ts index 3469cfba..8ebd9fae 100644 --- a/services/api/src/projects/file-manager.ts +++ b/services/api/src/projects/file-manager.ts @@ -1,3 +1,4 @@ +import {withDependencyLock} from "./dependency-lock.js"; /** * Project File Manager * @@ -243,7 +244,7 @@ async function doCreateProject( await chownProjectToApiUser(projectId, projectPath); let installResult; try { - installResult = await adapter.install(installCtx); + installResult = await withDependencyLock(projectId, () => adapter.install(installCtx)); } catch (err) { recordInstallFailure(projectId); throw err; @@ -321,6 +322,9 @@ export function hasNodeModules(projectId: string): boolean { * framework's required build tool (e.g. vite). */ export async function ensureDependencies(projectId: string): Promise { + return withDependencyLock(projectId, () => ensureDependenciesLocked(projectId)); +} +async function ensureDependenciesLocked(projectId: string): Promise { const projectPath = getProjectPath(projectId); // Python projects: check for requirements.txt without package.json @@ -432,6 +436,9 @@ export async function ensureDependencies(projectId: string): Promise { * only reliable cure. Callers MUST guard against repeated invocation. */ export async function forceReinstallDependencies(projectId: string): Promise { + return withDependencyLock(projectId, () => forceReinstallDependenciesLocked(projectId)); +} +async function forceReinstallDependenciesLocked(projectId: string): Promise { const projectPath = getProjectPath(projectId); // Node projects only — Python deps live in .venv, not node_modules. if (!existsSync(path.join(projectPath, "package.json"))) return; diff --git a/services/api/src/routes/preview-proxy/injected-script-syntax.test.ts b/services/api/src/routes/preview-proxy/injected-script-syntax.test.ts new file mode 100644 index 00000000..c498e284 --- /dev/null +++ b/services/api/src/routes/preview-proxy/injected-script-syntax.test.ts @@ -0,0 +1,16 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import {Script} from "node:vm"; +import {VISUAL_EDIT_BRIDGE_INLINE} from "../../visual-edit-bridge-inline.js"; +import {CONNECTOR_BRIDGE_SNIPPET,ERROR_CAPTURE_SNIPPET,RETRY_HTML,getReactRefreshPreambleSnippet,getStorageNamespaceSnippet} from "./injected-scripts.js"; + +// These are JS inside TS strings. TypeScript/build checks never parse them. +test("every classic preview bridge parses as the JavaScript delivered to a browser",()=>{ + assert.doesNotThrow(()=>new Script(VISUAL_EDIT_BRIDGE_INLINE,{filename:"visual-edit-bridge.js"})); + const snippets=[CONNECTOR_BRIDGE_SNIPPET,ERROR_CAPTURE_SNIPPET,RETRY_HTML,getReactRefreshPreambleSnippet("test-project"),getStorageNamespaceSnippet("test-project")]; + for(const html of snippets){ + const scripts=[...html.matchAll(/]*>([\s\S]*?)<\/script>/gi)]; + assert.ok(scripts.length>0); + for(const [index,match] of scripts.entries())assert.doesNotThrow(()=>new Script(match[1],{filename:`injected-${index}.js`})); + } +}); From 3342679baadbfa901fd02c8da41f9c11ec094f6e Mon Sep 17 00:00:00 2001 From: xin zhuang <65798732+1052326311@users.noreply.github.com> Date: Sat, 3 Oct 2026 03:50:59 +0800 Subject: [PATCH 7/9] test(preview): type captured script bodies --- .../api/src/routes/preview-proxy/injected-script-syntax.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/api/src/routes/preview-proxy/injected-script-syntax.test.ts b/services/api/src/routes/preview-proxy/injected-script-syntax.test.ts index c498e284..caa07b81 100644 --- a/services/api/src/routes/preview-proxy/injected-script-syntax.test.ts +++ b/services/api/src/routes/preview-proxy/injected-script-syntax.test.ts @@ -11,6 +11,6 @@ test("every classic preview bridge parses as the JavaScript delivered to a brows for(const html of snippets){ const scripts=[...html.matchAll(/]*>([\s\S]*?)<\/script>/gi)]; assert.ok(scripts.length>0); - for(const [index,match] of scripts.entries())assert.doesNotThrow(()=>new Script(match[1],{filename:`injected-${index}.js`})); + for(const [index,match] of scripts.entries())assert.doesNotThrow(()=>new Script(match[1]!, {filename:`injected-${index}.js`})); } }); From feac79bb2573957ddfbe58b0035c905a1b76c696 Mon Sep 17 00:00:00 2001 From: xin zhuang <65798732+1052326311@users.noreply.github.com> Date: Sat, 3 Oct 2026 04:01:54 +0800 Subject: [PATCH 8/9] fix(agent): honor explicit container isolation for command execution --- .../copilot-install.integration.test.ts | 28 +++++++++++++++++++ services/api/src/ai/tools/bash.ts | 13 +++++++-- .../src/ai/tools/container-command.test.ts | 24 ++++++++++++++++ .../api/src/ai/tools/container-command.ts | 28 +++++++++++++++++++ 4 files changed, 90 insertions(+), 3 deletions(-) create mode 100644 services/api/src/ai/providers/copilot-install.integration.test.ts create mode 100644 services/api/src/ai/tools/container-command.test.ts create mode 100644 services/api/src/ai/tools/container-command.ts diff --git a/services/api/src/ai/providers/copilot-install.integration.test.ts b/services/api/src/ai/providers/copilot-install.integration.test.ts new file mode 100644 index 00000000..6cef402d --- /dev/null +++ b/services/api/src/ai/providers/copilot-install.integration.test.ts @@ -0,0 +1,28 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import {mkdtemp,mkdir,writeFile,readFile,rm} from "node:fs/promises"; +import {tmpdir} from "node:os"; +import {join} from "node:path"; + +test("parallel SDK install handlers preserve both dependency sets",{skip:process.platform==="win32"},async()=>{ + const root=await mkdtemp(join(tmpdir(),"sdk-install-race-"));const before={root:process.env.DOABLE_PROJECTS_DIR,path:process.env.PATH}; + try { + await mkdir(join(root,"project"));await mkdir(join(root,"bin"));await writeFile(join(root,"project/package.json"),"{}"); + // Simulate npm's read-modify-write transaction without network or packages. + await writeFile(join(root,"bin/npm"),`#!/usr/bin/env node +const fs=require('node:fs'); const args=process.argv.slice(2);const data=JSON.parse(fs.readFileSync('package.json','utf8')); +setTimeout(()=>{const dev=args.includes('--save-dev'); data[dev?'devDependencies':'dependencies']={[dev?'types-package':'runtime-package']:'1.0.0'};fs.writeFileSync('package.json',JSON.stringify(data));},80); +`,{mode:0o755}); + process.env.DOABLE_PROJECTS_DIR=root;process.env.PATH=join(root,"bin")+":"+before.path; + const {createDoableTools}=await import("./copilot-tools.js"); + const tool=createDoableTools("project").find(t=>t.name==="install_package");assert.ok(tool); + const handler=(tool as unknown as {handler:(args:unknown)=>Promise<{success:boolean}>}).handler; + const result=await Promise.all([handler({packages:"runtime-package"}),handler({packages:"types-package",dev:true})]); + assert.ok(result.every(r=>r.success)); + assert.deepEqual(JSON.parse(await readFile(join(root,"project/package.json"),"utf8")),{dependencies:{"runtime-package":"1.0.0"},devDependencies:{"types-package":"1.0.0"}}); + } finally { + if(before.root===undefined)delete process.env.DOABLE_PROJECTS_DIR;else process.env.DOABLE_PROJECTS_DIR=before.root; + if(before.path===undefined)delete process.env.PATH;else process.env.PATH=before.path; + await rm(root,{recursive:true,force:true}); + } +}); diff --git a/services/api/src/ai/tools/bash.ts b/services/api/src/ai/tools/bash.ts index b8cab4ad..0941b6cc 100644 --- a/services/api/src/ai/tools/bash.ts +++ b/services/api/src/ai/tools/bash.ts @@ -1,3 +1,5 @@ +import {runContainerCommand,usesContainerIsolation} from "./container-command.js"; +import {getProjectPath} from "../project-files.js"; /** * Doable-owned `bash` tool — overrides the Copilot SDK's built-in bash. * @@ -41,7 +43,7 @@ export interface BashToolCtx { export function createBashTool(ctx: BashToolCtx): Tool { return defineTool("bash", { description: - "Execute a shell command in the project sandbox. All commands run inside the configured isolation backend (psroot / bubblewrap / systemd / sandbox-exec / dovault); the host filesystem and network are not directly accessible.", + "Execute a shell command in the project sandbox. Execution follows the operator isolation settings. With explicit off/off settings the API container is the isolation boundary; otherwise the configured process jail is required. This tool waits for the command to finish and returns exitCode/output; it does not create an interactive session for read_bash/write_bash.", overridesBuiltInTool: true, parameters: { type: "object" as const, @@ -83,7 +85,9 @@ export function createBashTool(ctx: BashToolCtx): Tool { }; try { - const result = await jailedSpawn( + const result = usesContainerIsolation(spawnCtx.hardening, process.env.DOABLE_HARDENING) + ? await runContainerCommand(command, getProjectPath(ctx.projectId)) + : await jailedSpawn( "/bin/sh", ["-c", command], spawnCtx, @@ -92,7 +96,7 @@ export function createBashTool(ctx: BashToolCtx): Tool { const stdout = truncateOutput(result.stdout); const stderr = truncateOutput(result.stderr); - const success = result.exitCode === 0 && !result.oomKilled; + const success = result.exitCode === 0 && !result.oomKilled && !result.timedOut; const output = [ stdout, @@ -105,10 +109,13 @@ export function createBashTool(ctx: BashToolCtx): Tool { exitCode: result.exitCode, durationMs: result.durationMs, oomKilled: result.oomKilled, + timedOut: result.timedOut, backendId: result.backendId, profileId: result.profileId, message: success ? `Command exited 0 in ${result.durationMs}ms (backend=${result.backendId})` + : result.timedOut + ? `Command timed out after ${result.durationMs}ms` : result.oomKilled ? `Command killed (OOM) after ${result.durationMs}ms` : `Command exited ${result.exitCode} in ${result.durationMs}ms`, diff --git a/services/api/src/ai/tools/container-command.test.ts b/services/api/src/ai/tools/container-command.test.ts new file mode 100644 index 00000000..83da0880 --- /dev/null +++ b/services/api/src/ai/tools/container-command.test.ts @@ -0,0 +1,24 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import {mkdtemp,rm,realpath} from "node:fs/promises"; +import {tmpdir} from "node:os"; +import {join} from "node:path"; +import {usesContainerIsolation,runContainerCommand} from "./container-command.js"; + +test("only explicit consistent off/off selects container mode; prod never falls back",()=>{ + assert.equal(usesContainerIsolation("off","off"),true); + for(const level of ["dev","staging","prod",""])assert.equal(usesContainerIsolation(level,"off"),false); + for(const config of [undefined,"full","relaxed","invalid",""])assert.equal(usesContainerIsolation("off",config),false); +}); +test("container command uses project cwd and filters server credentials",async()=>{ + const dir=await mkdtemp(join(tmpdir(),"container-command-")); + const key="DOABLE_TEST_SERVER_SECRET";const old=process.env[key];process.env[key]="test-only-secret"; + try { + const r=await runContainerCommand('node -e \'console.log(JSON.stringify({cwd:process.cwd(),leaked:!!process.env.DOABLE_TEST_SERVER_SECRET}))\'',dir); + assert.equal(r.exitCode,0);assert.equal(r.backendId,"container");assert.deepEqual(JSON.parse(r.stdout),{cwd:await realpath(dir),leaked:false}); + }finally{if(old===undefined)delete process.env[key];else process.env[key]=old;await rm(dir,{recursive:true,force:true});} +}); +test("container command preserves failure and supervises timeout",async()=>{ + const fail=await runContainerCommand("exit 7",tmpdir());assert.equal(fail.exitCode,7); + const timed=await runContainerCommand("sleep 30",tmpdir(),50);assert.equal(timed.timedOut,true);assert.notEqual(timed.exitCode,0);assert.ok(timed.durationMs<4000); +}); diff --git a/services/api/src/ai/tools/container-command.ts b/services/api/src/ai/tools/container-command.ts new file mode 100644 index 00000000..808de0af --- /dev/null +++ b/services/api/src/ai/tools/container-command.ts @@ -0,0 +1,28 @@ +import {spawn} from "node:child_process"; +import {buildSafeEnv} from "../../projects/safe-env.js"; + +/** Only the operator's explicit, consistent Docker single-tenant setting opts + * out of an additional process jail. Missing/unknown/mixed settings stay jailed. */ +export function usesContainerIsolation(hardening: string, configured: string | undefined): boolean { + return hardening === "off" && configured?.toLowerCase() === "off"; +} + +/** Execute within the existing API container for explicit off/off deployments. + * This is NOT a per-project filesystem jail; do not report it as one. */ +export async function runContainerCommand(command: string, cwd: string, timeoutMs = 60_000) { + const start = Date.now(); + let stdout = "", stderr = "", timedOut = false; + const result = await new Promise<{exitCode:number|null;signal:NodeJS.Signals|null}>((resolve,reject)=>{ + const group = process.platform !== "win32"; + const child=spawn("/bin/sh",["-c",command],{cwd,env:buildSafeEnv(undefined,{FORCE_COLOR:"0"}),stdio:["ignore","pipe","pipe"],detached:group}); + let force:ReturnType|undefined; + const kill=(signal:NodeJS.Signals)=>{try{if(group && child.pid)process.kill(-child.pid,signal);else child.kill(signal);}catch{/* already exited */}}; + const timer=setTimeout(()=>{timedOut=true;kill("SIGTERM");force=setTimeout(()=>kill("SIGKILL"),1000);},timeoutMs); + const clear=()=>{clearTimeout(timer);if(force)clearTimeout(force);}; + child.stdout.on("data",d=>{stdout=(stdout+d.toString()).slice(-1_048_576);}); + child.stderr.on("data",d=>{stderr=(stderr+d.toString()).slice(-1_048_576);}); + child.on("error",e=>{clear();reject(e);}); + child.on("close",(exitCode,signal)=>{clear();resolve({exitCode,signal});}); + }); + return {...result,stdout,stderr,durationMs:Date.now()-start,timedOut,oomKilled:false,backendId:"container",profileId:"ai-bash",composers:[]}; +} From abc9694d48453bae43e4966e56921a5fb9b5c3fa Mon Sep 17 00:00:00 2001 From: xin zhuang <65798732+1052326311@users.noreply.github.com> Date: Sat, 3 Oct 2026 04:11:56 +0800 Subject: [PATCH 9/9] docs(agent): describe recovery and execution validation boundaries --- docs/plan-progress.md | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/docs/plan-progress.md b/docs/plan-progress.md index 25063620..78748237 100644 --- a/docs/plan-progress.md +++ b/docs/plan-progress.md @@ -55,10 +55,22 @@ The targeted session, execution, ledger, final-response and parser regression ru A provider timeout after successful tools used to fall between both recovery paths: empty-response retry excludes tool activity, while structured continuation excludes deferred errors. The terminal error was then omitted from the saved trace, so history could show only successful tool cards and no explanation of the unfinished task. -Timeout recovery now acknowledges SDK cancellation before a single resume, reuses the existing conversation and tool ledger, and excludes pending/unknown/failed tool results, user stops, input/integration gates, completed task reports and non-timeout provider errors. A repeated timeout is stopped, not replayed. The recovery prompt preserves original authorization and read-only intent. Failed cancellation never permits another send. +Timeout recovery now acknowledges SDK cancellation before a single resume, reuses the existing conversation and tool ledger, and excludes pending/unknown/failed tool results (except validated file requests rejected before side effects), user stops, input/integration gates, completed task reports and non-timeout provider errors. A repeated timeout is stopped, not replayed. The recovery prompt preserves original authorization and read-only intent. Failed cancellation never permits another send. Final failure reasons are persisted with the trace. History projects the latest trace outcome for each assistant message in one project-scoped batch query; it does not rewrite assistant content or infer that successful tools mean task completion. Trace persistence precedes the stream's done signal. The editor renders an explicit failure notice alongside any partial response, including old failed traces with no recorded error text. Regression coverage includes timeout-after-reads, bounded repeat failures, cancellation acknowledgement/rejection, stop-during-cancel, uncertain outcomes and input gates. A production-event-pipeline test feeds read results, a timeout, a resumed write, structured completion and a final answer through the same processor, verifying that the read is not replayed and the answer survives. Long tasks also retain their streaming marker across refreshes: the five-minute orphan cleanup now checks the active request registry before expiring a record. Route-level tests cover active ten-minute tasks, stale orphan markers, recent markers, and a batched history response preserving both partial and empty failed messages. + +### File arguments, dependency transactions and command execution + +File creation/edit handlers validate a nonempty string path and string content before touching the database or filesystem. A rejected request returns stable `INVALID_FILE_ARGUMENTS` and `effects: "none"` metadata. Recovery recognizes only this structured result for the known file tools, including SDK result envelopes. Other failures remain uncertain and are never made retryable by matching translated error prose. + +Dependency mutations are serialized per project inside the API process: SDK installation, generic installation, scaffolding and dependency repair share the same queue. Preview restart happens after releasing the lock so dependency checks cannot deadlock. A timed-out installation waits for process exit before releasing ownership. This is not a distributed lock across multiple API replicas. Tests include concurrent manifest updates and a real SDK tool-handler integration using a local fake package-manager process, without network downloads. + +The AI command tool honors the explicit official Docker container-isolation configuration only when both the request hardening level and `DOABLE_HARDENING` equal `off`. It runs in the project directory with an allowlisted environment, bounded output and process-group timeout handling, and reports the selected backend. Missing/mixed/hardened configurations continue to require the sandbox; sandbox failure never triggers a fallback. Container mode does not isolate one project's filesystem from another and is suitable only for trusted internal use. No additional Linux capabilities or relaxed seccomp policy are introduced. + +Injected classic preview scripts are parsed as JavaScript in a regression test; successful TypeScript compilation alone does not check the syntax inside template-string scripts. + +Current runtime-compatible Linux validation passes 138 regression cases, one separate real SDK installation integration case, and 13 sandbox cases. An actual model turn generated a small 3D app, and the command tool subsequently completed its TypeScript/Vite production build through the explicit container backend. Historical failure UI and refreshed preview were checked in a browser. Existing full-tree type diagnostics remain outside this change. Long provider output truncation is not claimed to be fixed: the pinned SDK's OpenAI-compatible transport did not forward tested output-token overrides, and a large real generation still timed out. Smaller complete file operations succeeded. Automatic timeout recovery has failure-injection coverage; no claim is made that all upstream timeouts recover successfully.