fix(extension): expose honest browser runner outcomes - #5681
Conversation
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| } catch (error) { | ||
| return { | ||
| // Attach denial issued no page code. A timeout or lost eval response cannot prove completion. | ||
| effectsUncertain: dispatched, |
There was a problem hiding this comment.
WARNING: Chrome CDP navigation errors are marked uncertain, so workflow click-recovery never runs
chrome.debugger.sendCommand rejects with {"code":-32000,"message":"Execution context was destroyed."} when a click navigates mid-eval. This catch treats every post-attach rejection as effectsUncertain: dispatched (true). runWorkflow now skips navigation recovery for uncertain results, and the new "legacy destroyed context" test confirms metadata-free destroyed-context errors no longer retry.
The recovery test only passes because it stubs effectsUncertain: false. The real Chrome producer will not emit that shape, so a successful in-page navigation will interrupt the workflow instead of re-running on the landed page.
Treat this known CDP navigation failure as a confirmed error (effectsUncertain: false) so isNavigationDestroyedEval can recover.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 1 Issue Found | Recommendation: Address before merge Fix these issues in Kilo Cloud Overview
Issue Details (click to expand)WARNING
Files Reviewed (30 files)
Reviewed by grok-4.6 · Input: 543.5K · Output: 39.6K · Cached: 1.7M Review guidance: REVIEW.md from base branch |
1 participant
Summary
LlmTurnOutcomereplacesPromise<void>with four statuses:succeeded,failed,cancelled, andinterrupted, plus a reason, summary, confirmedtoolResults, andeffectsUncertain.LlmTurnFailureReasondistinguishes model failure, retry exhaustion, context overflow, exhausted rounds, truncation, empty or incomplete answers, failure caps, and unsafe calls.succeededmeans a complete answer, not verified page changes; confirmed tool errors remain recoverable, and existing callers keep transcript updates.Files
apps/extension/src/shared/agent-llm-turn-runner-core.ts— Source, modified, 298 changed lines. Classifies terminal paths, explains empty failures, preserves partial replies, excludes stale retry summaries, and stops batches at the 25-failure cap. Checks authority before model requests, retries, tool calls, and automatic continuation, including after asynchronous tool preparation.apps/extension/src/shared/agent-llm-turn-runner-core.test.ts— Test, modified, 568 changed lines. Updates coverage for terminal outcomes, guards, unsafe calls, and recovery.ExecutionGuardandExecutionStoppedErrorcombine owner checks withAbortSignal, preserving cancellation and interruption reasons without claiming to undo issued actions.ToolExecutionOptions.onResultreports settledToolResultEventvalues immediately; requiredeffectsUncertainturns uncertain successes into failed events and stops later calls. Existing signal-only callers still receive settled results on Stop; the documented compatibility path remains until all guardless callers retire.Files
apps/extension/src/shared/agent-tool-results.ts— Source, modified, 116 changed lines. Adds guard normalization, typed stops, immediate callbacks, and uncertain-error events while preserving the existing signal argument.apps/extension/src/shared/agent-tool-results.test.ts— Test, modified, 170 changed lines. Updates batch-stop, uncertainty, and settled-result assertions.EvalTabResultandTabDebuggerResponseadd optionaleffectsUncertain, whileNormalizedEvalTabResultrequires it.normalizeEvalTabResulttreats older successes as confirmed and failures without metadata as uncertain;evalTabResultSchemaandtabDebuggerResponseSchemapreserve explicit metadata. Browser producers mark timeouts and lost results as uncertain, distinguish confirmed rejection or script errors, and never equate a scripting timeout with cancellation.Files
apps/extension/src/shared/tab-debugger.ts— Source, modified, 178 changed lines. Adds producer metadata and structured page-tool replies across evaluation, snapshots, screenshots, and discovery; distinguishes denied or settled failures from missing replies.apps/extension/src/shared/tab-debugger.test.ts— Test, modified, 69 changed lines. Updates browser-result and uncertainty assertions.KiloGatewayUnsupportedToolErrorseparates missing or unoffered tool names from retryable transport errors. The runner rejects unsafe, truncated, or unfinished call batches before dispatch, including partially accepted replies. A bad call invalidates the entire batch, so accepted calls cannot execute beside discarded calls or leave unmatched history events.Files
apps/extension/src/shared/kilo-gateway-chat-stream-client.ts— Source, modified, 10 changed lines. Throws the terminal error from the real stream parser instead of a retryableTypeError.apps/extension/src/shared/kilo-api-client.ts— Source, modified, 1 changed line. Re-exports the typed unsupported-tool error for runner consumers.apps/extension/src/shared/kilo-gateway-chat-stream-client.test.ts— Test, modified, 3 changed lines. Updates the unsupported-tool parser assertion.WorkflowRunnerDeps,WorkflowToolContext, andRunWorkflowOptionscarryexecutionGuardthrough approval,startUrlnavigation, page evaluation, and later navigation.WorkflowRunResult,NormalizedWorkflowRunResult, andscriptEnvelopeSchemapreserveeffectsUncertain; old failed results without metadata now interrupt instead of triggering recovery.requestApprovalstill uses existing permissions; aborted approvals cancel, uncertain envelopes stop before another action, and only confirmed evaluation failures permit navigation recovery.Files
apps/extension/src/shared/agent-workflow-runner.ts— Source, modified, 185 changed lines. ReusesEvalTabResult, guards each step, and preserves uncertainty through envelopes and dry-run results. Marks invoked non-function scripts uncertain and limits navigation recovery to confirmed failures.apps/extension/entrypoints/sidepanel/agent-workflow-runtime.ts— Source, modified, 78 changed lines. Guards navigation and evaluation; maps lost responses and navigation timeouts to uncertainty while retaining existing timeout budgets.apps/extension/entrypoints/sidepanel/agent-workflow-tool-runtime.ts— Source, modified, 64 changed lines. Uses shared dependency types without a cast, guards storage and approval boundaries, and preserves metadata during result conversion.apps/extension/src/shared/agent-workflow-runner.test.ts— Test, modified, 316 changed lines. Updates workflow guard, navigation, envelope, and uncertainty coverage.apps/extension/entrypoints/sidepanel/agent-workflow-runtime.test.ts— Test, modified, 48 changed lines. Updates browser navigation and evaluation assertions.apps/extension/entrypoints/sidepanel/agent-workflow-tool-runtime.test.ts— Test, modified, 233 changed lines. Updates workflow adapter, approval, guard, and result assertions.RunLlmTurnOptions,RunSafeLlmTurnOptions, andRunDangerousLlmTurnOptionsacceptexecutionGuard; both wrappers returnLlmTurnOutcomeand forward the guard to every tool family. Safe and Dangerous tool selection, workflow permissions, remote filtering, and streaming callbacks stay unchanged. Dangerousevaluses the guarded transport with the existing five-second timeout; workflow evaluation retains thirty seconds.Files
apps/extension/entrypoints/sidepanel/agent-llm-turn-runner.ts— Source, modified, 41 changed lines. Returns the outcome, accepts a remote executor guard argument, falls back to the workflow guard, and shares guarded evaluation.apps/extension/entrypoints/sidepanel/agent-safe-llm-turn-runner.ts— Source, modified, 36 changed lines. Returns the outcome and forwards the normalized guard through discovery, workflows, remote tools, and search; retains optional page-tool discovery.apps/extension/entrypoints/sidepanel/agent-llm-turn-runner.test.ts— Test, modified, 503 changed lines. Updates Dangerous-mode outcome, guard, and real-parser regression coverage.apps/extension/entrypoints/sidepanel/agent-safe-llm-turn-runner.test.ts— Test, modified, 338 changed lines. Updates Safe-mode outcome, guard, and real-parser regression coverage.createSafeToolExecutoraccepts an optionalExecutionGuardand returnsNormalizedEvalTabResultwithout changing snapshot caching or memory reads. Page snapshots and screenshots retain producer uncertainty, and uncertain snapshots never enter the cache. Local argument, cache, and parsed-snapshot errors remain confirmed failures, so the runner can recover from them.Files
apps/extension/entrypoints/sidepanel/agent-safe-tool-runtime.ts— Source, modified, 88 changed lines. Normalizes browser replies, rejects uncertain snapshots, and adds guarded execution while preserving memory and cache behavior.apps/extension/entrypoints/sidepanel/agent-safe-tool-runtime.test.ts— Test, modified, 32 changed lines. Updates safe-tool guard and result assertions.apps/extension/src/shared/agent-safe-tool-runtime.test.ts— Test, modified, 11 changed lines. Updates the shared safe-tool assertions.discoverWebMcpToolsandexecuteWebMcpToolCallacceptExecutionGuard; confirmed discovery failures still disable page tools, but uncertain discovery now interrupts. Execution preserveseffectsUncertainthrough result parsing and size limits, distinguishes pre-dispatch serialization failures, and propagates typed stops. Existing document and definition checks still gate page tools; the change adds no new permission.Files
apps/extension/entrypoints/sidepanel/agent-web-mcp-tool-runtime.ts— Source, modified, 114 changed lines. Preserves certainty through discovery, execution, JavaScript Object Notation (JSON) parsing, and output limits; guards both entrypoints.apps/extension/entrypoints/sidepanel/agent-web-mcp-tool-runtime.test.ts— Test, modified, 76 changed lines. Updates page-tool discovery and execution tests.Remote Model Context Protocol (MCP) calls now carry
ExecutionGuardthroughcallRemoteMcpToolandexecuteRemoteMcpToolCall, preserving interruption reasons and post-dispatch uncertainty.CallToolResult.isErrorandMcpErrorcodesInvalidParamsorMethodNotFoundremain recoverable; lost results, transport failures, and request timeouts stay uncertain. Remote calls still use plain fetch, existing route/server filtering, and capped results, so Kilo credentials stay separate from third-party authentication.Files
apps/extension/entrypoints/sidepanel/agent-remote-mcp-tool-runtime.ts— Source, modified, 59 changed lines. Forwards the guard to the real client and distinguishes confirmed tool errors from uncertain transport failures without changing filtering or result limits.apps/extension/entrypoints/sidepanel/remote-mcp-client.ts— Source, modified, 35 changed lines. Rechecks authority after connection setup, tracks dispatch, preserves owner cancellation reasons, and distinguishes protocol rejections from uncertain failures.apps/extension/entrypoints/sidepanel/agent-remote-mcp-tool-runtime.test.ts— Test, modified, 427 changed lines. Updates real-client interruption, lease-loss, and protocol-rejection coverage.apps/extension/entrypoints/sidepanel/remote-mcp-client-call.test.ts— Test, modified, 108 changed lines. Updates client cancellation, error, and dispatch assertions.WebSearchContext.executionGuardnow gates the existing search request;executeWebSearchToolCallandcreateWebSearchExecutorreturnNormalizedEvalTabResult. Lost replies, invalid successful replies, and cancellation while reading a successful reply preserve uncertainty; confirmed server errors remain recoverable. Query and budget denials remain confirmed; the five-search budget and result limits stay unchanged, and Stop cannot undo an issued request.Files
apps/extension/entrypoints/sidepanel/agent-web-search-tool-runtime.ts— Source, modified, 69 changed lines. Guards billable requests, preserves cancellation during fetch and body reads, and separates uncertainty from confirmed search errors.apps/extension/entrypoints/sidepanel/agent-web-search-tool-runtime.test.ts— Test, modified, 116 changed lines. Updates search guard, parsing, and cancellation tests.Tests: 15 unit test files modified, with 3,018 changed lines.
Generated: 0 files changed.
Verification
Manual verification: not run. Runtime verification remains pending on the stack tips.
Visual Changes
Visual Changes: N/A
Reviewer Notes
Human steps
Recorded check evidence
Stack and review scope
browser-task-0787-s4...browser-task-0787-s7, with headf1a542f21./Users/igor/Projects/.worktrees/browser-task-0787, branchbrowser-task-0787-s7; this level is the review scope./Users/igor/Projects/.worktrees/browser-task-0787-kilocode, branchbrowser-task-0787-s6; the published integration is dependency context only.Notes
Runtime verification remains pending on the stack tips. Profile locks, provider integration, consent controls, and native browser proof follow in later levels.
Stacked PRs — merge bottom to top. Each level shows only its own diff.
Runtime verification (E2E, user advocacy, simplify) runs on the tip PR over every level.
Every level keeps its own checks, its own bot review, and its own threads; each one is answered on its own PR.
Each level is its own deliverable: it builds and passes its own checks alone.
A finding on a level is repaired on that level, then carried upward with stack.sh forward.
browser-task-0787— feat(browser-task): define negotiated job contracts #5638browser-task-0787-s2— feat(session-ingest): persist owned browser jobs #5644browser-task-0787-s3— feat(session-ingest): route fenced browser providers #5648browser-task-0787-s4— feat(cloud-agent-sdk): support browser provider jobs #5653browser-task-0787-s7— fix(extension): expose honest browser runner outcomes #5681 ← this PRbrowser-task-0787-s8— feat(extension): coordinate browser execution across panels #5694browser-task-0787-s9— fix(extension): invalidate ended browser task approvals #5698browser-task-0787-s10— feat(extension): persist browser jobs and profile consent #5702 (tip)