From e44d86357966f79f603bea521c749cd9d22ad61c Mon Sep 17 00:00:00 2001 From: delta456 Date: Mon, 24 Aug 2026 13:22:54 +0530 Subject: [PATCH] fix(agent): repair broken Anthropic goal execution and missing result output --- .../src/ui/ink/components/FinalSummary.tsx | 33 +++++--- packages/core/src/agent/loop.ts | 68 +++++++++++++-- packages/core/src/device/android-power.ts | 45 ++++++++++ packages/core/src/llm/provider.ts | 46 ++++++++++ packages/core/src/mcp/keyboard.ts | 2 +- tests/sdk/llm-thinking.test.ts | 83 +++++++++++++++++++ tests/ui/ink-ui.test.tsx | 59 ++++++++++++- 7 files changed, 312 insertions(+), 24 deletions(-) create mode 100644 packages/core/src/device/android-power.ts create mode 100644 tests/sdk/llm-thinking.test.ts diff --git a/packages/cli/src/ui/ink/components/FinalSummary.tsx b/packages/cli/src/ui/ink/components/FinalSummary.tsx index f6a3c85..73dc1e5 100644 --- a/packages/cli/src/ui/ink/components/FinalSummary.tsx +++ b/packages/cli/src/ui/ink/components/FinalSummary.tsx @@ -53,20 +53,27 @@ export function FinalSummary({ data }: { data: JourneySummaryData }) { {data.subGoals.map((sg, i) => { const sgOk = sg.status === 'completed'; return ( - - - - {sgOk ? symbols.check : symbols.cross} - - - - {sg.goal} - - - - {sgOk ? 'pass' : 'FAIL'} - + + + + + {sgOk ? symbols.check : symbols.cross} + + + + {sg.goal} + + + + {sgOk ? 'pass' : 'FAIL'} + + + {sg.result ? ( + + {sg.result} + + ) : null} ); })} diff --git a/packages/core/src/agent/loop.ts b/packages/core/src/agent/loop.ts index ef701bf..f09359a 100644 --- a/packages/core/src/agent/loop.ts +++ b/packages/core/src/agent/loop.ts @@ -15,6 +15,7 @@ import { typeViaKeyboard, detectDeviceUdid, pressEnterKey } from '../mcp/keyboar import type { LLMProvider, AgentContext, ToolCallDecision } from '../llm/provider.js'; import type { ActionResult } from '../llm/schemas.js'; import { getScreenState } from '../perception/screen.js'; +import { detectScreenOff } from '../device/android-power.js'; import { diffScreen, computePerceptionHash } from '../perception/screen-diff.js'; import { createStuckDetector } from './stuck.js'; import { shouldPreferVisionLocateTap } from './vision-tap-policy.js'; @@ -135,6 +136,22 @@ export interface StepRecord { const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms)); +/** + * Describe a screen-capture failure, appending a device-state hint when a + * sleeping display is the cause. Only probes on timeouts — every other failure + * mode already reports itself, and the probe costs an adb round-trip. + */ +async function describeScreenFailure( + err: unknown, + platform: 'android' | 'ios', + deviceUdid?: string | null +): Promise { + const msg = err instanceof Error ? err.message : String(err); + if (platform !== 'android' || !/timed out|timeout/i.test(msg)) return msg; + const hint = await detectScreenOff(deviceUdid ?? undefined); + return hint ? `${msg} — ${hint}` : msg; +} + export async function runAgent(options: AgentOptions): Promise { let { goal } = options; const { @@ -298,8 +315,9 @@ export async function runAgent(options: AgentOptions): Promise { ); } catch (err) { ui.stopSpinner(); - ui.printError(`Step ${step + 1}: Failed to get screen state`, String(err)); - lastResult = `Screen capture failed: ${err}`; + const detail = await describeScreenFailure(err, detectedPlatform, deviceUdid); + ui.printError(`Step ${step + 1}: Failed to get screen state`, detail); + lastResult = `Screen capture failed: ${detail}`; cachedPostScreen = undefined; // Invalidate cache on failure continue; } @@ -343,6 +361,19 @@ export async function runAgent(options: AgentOptions): Promise { modelName, totalCachedTokens ); + + // Second early-return path with the same gap as the LLM "done" + // branch above: screenEvaluator can conclude the goal is already + // satisfied from a screen diff alone, without an LLM tool call — + // still needs an onStep so the answer reaches the logger/SDK/JSON. + onStep?.({ + step, + decision: { toolName: 'done', args: { reason: evaluation.reason } }, + result: { success: true, message: evaluation.reason }, + elementsCount: screen.elementCount, + screenshot: postActionScreenshot, + }); + return { success: true, reason: evaluation.reason, @@ -574,6 +605,8 @@ export async function runAgent(options: AgentOptions): Promise { } catch (err: any) { const errName = err?.name ?? ''; const errMsg = err?.message ?? ''; + const status = err?.statusCode ?? err?.status; + const apiErrorType = err?.data?.error?.type ?? err?.responseBody?.error?.type; if ( errName.includes('UnsupportedModel') || errName.includes('AuthenticationError') || @@ -581,23 +614,27 @@ export async function runAgent(options: AgentOptions): Promise { errMsg.includes('API key') || errMsg.includes('is not found') || errMsg.includes('NOT_FOUND') || - err?.statusCode === 401 || - err?.statusCode === 404 + // A malformed request never succeeds on retry — a bad thinking/tool + // config would otherwise burn the entire step budget on identical 400s. + status === 400 || + status === 401 || + status === 404 || + apiErrorType === 'invalid_request_error' ) { ui.stopStreaming(); ui.stopSpinner(); - ui.printError('Fatal LLM error', err.message ?? String(err)); + ui.printError('Fatal LLM error', errMsg || String(err)); return { success: false, - reason: `Fatal LLM error: ${err.message ?? err}`, + reason: `Fatal LLM error: ${errMsg || err}`, stepsUsed: step + 1, history, }; } ui.stopStreaming(); ui.stopSpinner(); - ui.printError(`Step ${step + 1}: LLM error`, String(err)); - lastResult = `LLM call failed: ${err}`; + ui.printError(`Step ${step + 1}: LLM error`, errMsg || String(err)); + lastResult = `LLM call failed: ${errMsg || err}`; continue; } @@ -708,7 +745,7 @@ export async function runAgent(options: AgentOptions): Promise { } catch (err) { // Verification failed to execute — log but accept the done to avoid blocking indefinitely ui.printWarning( - `Done verification failed: ${err instanceof Error ? err.message : String(err)}` + `Done verification failed: ${await describeScreenFailure(err, detectedPlatform, deviceUdid)}` ); } @@ -722,6 +759,19 @@ export async function runAgent(options: AgentOptions): Promise { const cost = (totalInputTokens / 1_000_000) * pricing[0] + (totalOutputTokens / 1_000_000) * pricing[1]; ui.printTokenSummary(totalInputTokens, totalOutputTokens, cost, modelName, totalCachedTokens); + + // The "done" branch returns here rather than falling through to the + // bottom-of-loop onStep() call below — without this, the step carrying + // the LLM's actual answer (reason) never reaches the session logger, + // the SDK's onStep consumers, or the JSON event stream. + onStep?.({ + step, + decision, + result: { success: true, message: reason }, + elementsCount: screen.elementCount, + screenshot: postActionScreenshot, + }); + return { success: true, reason, diff --git a/packages/core/src/device/android-power.ts b/packages/core/src/device/android-power.ts new file mode 100644 index 0000000..b55f44c --- /dev/null +++ b/packages/core/src/device/android-power.ts @@ -0,0 +1,45 @@ +/** + * Android display-power probe. + * + * UiAutomator2 builds a page source by walking the focused window. While the + * display is asleep there is no focused window, so the dump never returns and + * the call burns the entire MCP request budget (MCP_TIMEOUT_MS, default 120 s) + * before failing with a bare "Request timed out" — which says nothing about the + * actual cause. Probing the power state turns that into an actionable message. + */ + +import { exec } from 'child_process'; +import { promisify } from 'util'; +import { getADBPath } from '../mcp/keyboard.js'; + +const execAsync = promisify(exec); + +/** Appended to screen-capture timeouts that a sleeping display explains. */ +export const SCREEN_OFF_HINT = + 'device screen is off — UiAutomator cannot read the screen while the display sleeps. ' + + 'Wake it with `adb shell input keyevent KEYCODE_WAKEUP`; ' + + 'for longer runs `adb shell svc power stayon true` keeps it awake while charging.'; + +/** + * Resolve SCREEN_OFF_HINT when the Android display is asleep, else undefined. + * + * Never throws and never blocks for long — a diagnostic must not mask or delay + * the failure it is explaining. Returns undefined when adb is missing, the + * device is gone, or the probe itself times out. + */ +export async function detectScreenOff(deviceUdid?: string): Promise { + const deviceFlag = deviceUdid ? `-s ${deviceUdid}` : ''; + try { + // Filter on-device: a full `dumpsys power` dump runs to megabytes and blows + // past exec's 1 MB maxBuffer, which would fail the probe before it ever + // reads the field. Grepping on the device returns a couple of lines. + const { stdout } = await execAsync( + `${getADBPath()} ${deviceFlag} shell "dumpsys power | grep mWakefulness"`, + { timeout: 5000 } + ); + // Asleep = display off. Dozing = ambient/always-on display, equally unreadable. + return /mWakefulness=(Asleep|Dozing)/.test(stdout) ? SCREEN_OFF_HINT : undefined; + } catch { + return undefined; + } +} diff --git a/packages/core/src/llm/provider.ts b/packages/core/src/llm/provider.ts index 998fd12..845e964 100644 --- a/packages/core/src/llm/provider.ts +++ b/packages/core/src/llm/provider.ts @@ -381,6 +381,20 @@ function isGemini25Family(modelId: string): boolean { return /gemini-2\.5/i.test(modelId); } +/** + * Adaptive thinking landed with Claude 4.6 and is the only thinking mode the + * 5 family accepts. Pre-4.6 models (4.5 and older) support only the budgeted + * `{type:'enabled', budgetTokens}` form. + */ +function supportsAdaptiveThinking(modelId: string): boolean { + const id = modelId.toLowerCase(); + // 5 family: claude-opus-5, claude-sonnet-5, claude-fable-5, claude-mythos-5 + if (/^claude-[a-z]+-5\b/.test(id)) return true; + // 4.x: adaptive from 4-6 onward (4-6, 4-7, 4-8, …) + const minor = id.match(/^claude-[a-z]+-4-(\d+)/); + return minor ? Number(minor[1]) >= 6 : false; +} + export function buildThinkingOptions(config: AppClawConfig): Record | undefined { if (config.LLM_THINKING !== 'on') return undefined; if (!THINKING_PROVIDERS.has(config.LLM_PROVIDER)) return undefined; @@ -394,6 +408,23 @@ export function buildThinkingOptions(config: AppClawConfig): Record switch (config.LLM_PROVIDER) { case 'anthropic': + // Anthropic rejects `{type:'enabled'}` when tool_choice forces tool use + // ("Thinking may not be enabled when tool_choice forces tool use"), and + // getDecision always sends toolChoice:'required'. Adaptive is the only + // thinking mode the API permits alongside forced tools — and the + // documented replacement for budgetTokens on 4.6+ regardless. + // `display:'summarized'` is required for reasoning to reach the UI; the + // API default omits it and streams empty thinking blocks. + if (supportsAdaptiveThinking(modelId)) { + return { + anthropic: { + thinking: { type: 'adaptive', display: 'summarized' }, + }, + }; + } + // Pre-4.6 models have no adaptive mode. The budgeted form still cannot + // coexist with forced tool use; the agent loop reports that 400 as fatal + // rather than retrying it. return { anthropic: { thinking: { type: 'enabled', budgetTokens: budget }, @@ -534,7 +565,15 @@ export function createLLMProvider(config: AppClawConfig, mcpTools: MCPToolInfo[] // textStream omits reasoning — use fullStream so goal-based runs show thinking. let reasoningText = ''; let streamingStarted = false; + let streamError: unknown; for await (const part of stream.fullStream) { + // Provider failures arrive as `error` parts. The promises awaited + // below then throw a generic NoOutputGeneratedError that carries no + // `cause`, so the actionable message is lost unless captured here. + if (part.type === 'error') { + streamError = (part as any).error; + continue; + } const chunk = part.type === 'reasoning-delta' ? part.text @@ -551,6 +590,13 @@ export function createLLMProvider(config: AppClawConfig, mcpTools: MCPToolInfo[] } if (streamingStarted) callbacks.onDone?.(); + // Rethrow the provider's own error rather than letting the awaits below + // surface it as "No output generated. Check the stream for errors." + if (streamError) { + clearTimeout(abortTimer); + throw streamError; + } + // Await final results after stream completes const [streamUsage, streamTotalUsage, toolCalls, text, providerMeta, response] = await Promise.all([ diff --git a/packages/core/src/mcp/keyboard.ts b/packages/core/src/mcp/keyboard.ts index c656e17..f9dc018 100644 --- a/packages/core/src/mcp/keyboard.ts +++ b/packages/core/src/mcp/keyboard.ts @@ -118,7 +118,7 @@ export async function typeViaSetValue(mcp: MCPClient, text: string): Promise { + test.each([ + ['claude-sonnet-4-6', true], + ['claude-opus-5', true], + ['claude-sonnet-5', true], + ['claude-fable-5', true], + ['claude-opus-4-8', true], + ['claude-opus-4-7', true], + ['claude-sonnet-4-5', false], + ['claude-haiku-4-5', false], + ['claude-3-5-sonnet', false], + ])('%s → adaptive: %s', (model, expectAdaptive) => { + const config = loadConfig({ LLM_PROVIDER: 'anthropic', LLM_MODEL: model, LLM_THINKING: 'on' }); + const options = buildThinkingOptions(config); + expect(options?.anthropic?.thinking?.type).toBe(expectAdaptive ? 'adaptive' : 'enabled'); + }); + + test('adaptive thinking requests display:summarized so reasoning reaches the UI', () => { + const config = loadConfig({ + LLM_PROVIDER: 'anthropic', + LLM_MODEL: 'claude-sonnet-4-6', + LLM_THINKING: 'on', + }); + const options = buildThinkingOptions(config); + // The API default for `display` is 'omitted', which streams empty + // thinking blocks — 'summarized' is required for the terminal's + // "Reasoning…" panel to actually show anything. + expect(options?.anthropic?.thinking?.display).toBe('summarized'); + }); + + test('pre-4.6 models still carry a budgetTokens value (no adaptive support)', () => { + const config = loadConfig({ + LLM_PROVIDER: 'anthropic', + LLM_MODEL: 'claude-sonnet-4-5', + LLM_THINKING: 'on', + }); + const options = buildThinkingOptions(config); + expect(options?.anthropic?.thinking).toMatchObject({ type: 'enabled' }); + expect(typeof options?.anthropic?.thinking?.budgetTokens).toBe('number'); + }); + + test('LLM_THINKING=off disables thinking regardless of model', () => { + const config = loadConfig({ + LLM_PROVIDER: 'anthropic', + LLM_MODEL: 'claude-sonnet-4-6', + LLM_THINKING: 'off', + }); + expect(buildThinkingOptions(config)).toBeUndefined(); + }); + + test('Gemini routing is unaffected by the Anthropic adaptive-thinking change', () => { + const gemini25 = loadConfig({ + LLM_PROVIDER: 'gemini', + LLM_MODEL: 'gemini-2.5-flash', + LLM_THINKING: 'on', + }); + expect(buildThinkingOptions(gemini25)).toMatchObject({ + google: { thinkingConfig: { thinkingBudget: expect.any(Number) } }, + }); + + const gemini3 = loadConfig({ + LLM_PROVIDER: 'gemini', + LLM_MODEL: 'gemini-3.1-flash-lite', + LLM_THINKING: 'on', + }); + expect(buildThinkingOptions(gemini3)).toMatchObject({ + google: { thinkingConfig: { thinkingLevel: expect.any(String) } }, + }); + }); +}); diff --git a/tests/ui/ink-ui.test.tsx b/tests/ui/ink-ui.test.tsx index 780f31c..d26f80b 100644 --- a/tests/ui/ink-ui.test.tsx +++ b/tests/ui/ink-ui.test.tsx @@ -7,11 +7,12 @@ import React from 'react'; import { describe, test, expect } from 'vitest'; import { render } from 'ink-testing-library'; import { RunScreen, tailSlice } from '@appclaw/cli/ui/ink/RunScreen'; -import type { TimelineEntry } from '@appclaw/cli/ui/ink/store'; +import type { TimelineEntry, JourneySummaryData } from '@appclaw/cli/ui/ink/store'; import { store } from '@appclaw/cli/ui/ink/store'; import { askUserViaInk } from '@appclaw/cli/ui/ink/InkRenderer'; import { PlaygroundApp } from '@appclaw/cli/ui/ink/PlaygroundApp'; import { pgStore } from '@appclaw/cli/ui/ink/playground-store'; +import { FinalSummary } from '@appclaw/cli/ui/ink/components/FinalSummary'; const tick = (ms = 30) => new Promise((r) => setTimeout(r, ms)); @@ -278,3 +279,59 @@ describe('Ink PlaygroundApp', () => { unmount(); }); }); + +describe('Ink FinalSummary', () => { + const journeyData = ( + overrides: Partial = {}, + subGoals: JourneySummaryData['subGoals'] = [] + ): JourneySummaryData => ({ + success: true, + overallGoal: 'open settings and get the wifi name', + subGoals, + totalSteps: 3, + durationMs: 58400, + tokens: { input: 17425, output: 86, cost: 0.0536, model: 'claude-sonnet-4-6' }, + ...overrides, + }); + + // Regression test for the bug where a sub-goal's `done` reason — e.g. the + // Wi-Fi name the agent was asked to find — reached this component but was + // never rendered, only its goal text and pass/fail were. + test('renders each sub-goal result text, not just goal + pass/fail', () => { + const data = journeyData({}, [ + { goal: 'Open the Settings app', status: 'completed', result: 'Opened Settings' }, + { + goal: 'Navigate to Network & internet → Internet and identify the Wi-Fi name', + status: 'completed', + result: "The connected Wi-Fi network is 'AndroidWifi'", + }, + ]); + const { lastFrame, unmount } = render(); + const out = lastFrame() ?? ''; + expect(out).toContain('AndroidWifi'); + expect(out).toContain('Opened Settings'); + unmount(); + }); + + test('renders a failed sub-goal result alongside FAIL', () => { + const data = journeyData({ success: false }, [ + { + goal: "Tap on 'Network & internet'", + status: 'failed', + result: 'UiAutomator2 driver crashed', + }, + ]); + const { lastFrame, unmount } = render(); + const out = lastFrame() ?? ''; + expect(out).toContain('FAIL'); + expect(out).toContain('UiAutomator2 driver crashed'); + unmount(); + }); + + test('omits the result line when a sub-goal has no result text', () => { + const data = journeyData({}, [{ goal: 'Open the Settings app', status: 'completed' }]); + const { lastFrame, unmount } = render(); + expect(lastFrame()).toContain('Open the Settings app'); + unmount(); + }); +});