Sandbox tool views, llamacpp /props context, structured worker errors#190
Merged
Conversation
Console:
- New chat/sandbox module: parsers for the sandbox::* wire shapes plus
per-tool renderers (exec, run, stop, list, create, fs.{read,write,
grep,ls,mkdir,mv,rm,sed,stat,chmod}), an ANSI terminal output, code
highlight pane, and a unified error view.
- FunctionCallMessage shows a sandbox preview while pending/running and
switches to terminal/raw-json tabs once the call completes.
- Examples + Playground get sandbox fixtures and scenario updates so the
new views can be exercised in isolation.
Harness:
- llamacpp discovery now reads n_ctx from the server-wide /props
endpoint to populate context_window, falling back to the embedded
default on failure / non-2xx / malformed JSON.
- turn-orchestrator agent-trigger extracts and forwards structured
worker error envelopes (S-code, docs_url, fix, retryable) intact via
errorResult instead of burying them under gate_unavailable.reason.
Misc: small refactors in providers, auto-accept, conversations context,
trace transform, and assorted UI cleanups.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
More reviews will be available in 47 minutes and 45 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR introduces a complete sandbox tool rendering system with schema-driven validation, comprehensive error handling, and terminal UI components for all sandbox operations (exec, run, create, stop, list, filesystem utilities). It also enhances the auto-accept policy with allow-listing before deny checking, improves llama.cpp discovery with ChangesSandbox Tool UI System
Auto-Accept Allow-Listing Enhancement
Syntax Highlighting Enhancement
Example and Fixture System
Harness Provider and Error Handling
🐇 Hoppy hop through the sandbox, parsing schemas with care, 🎯 3 (Moderate) | ⏱️ ~20 minutes
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Contributor
skill-check — worker1 verified, 11 skipped (no docs/). 1 error across the verified workers.
|
The new /props test introduced a double-quoted string and a multi-line new Response(...) call that biome formats differently. biome ci fails on the formatting diff. No behavior change.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
console/web/src/components/chat/FunctionCallMessage.tsx (1)
96-103: ⚡ Quick winAvoid eager sandbox rendering when the row is collapsed.
Line 96 and Line 97 compute sandbox preview/terminal unconditionally. This does avoidable parsing/render work for closed rows in long chat lists. Gate these calls by
open(or compute inside the open block) so hidden messages stay cheap.♻️ Suggested change
- const sandboxPreview = SandboxToolView.tryRenderPreview(message) - const sandboxTerminal = !pending ? SandboxToolView.tryRender(message) : null + const sandboxPreview = + open && pending ? SandboxToolView.tryRenderPreview(message) : null + const sandboxTerminal = + open && !pending ? SandboxToolView.tryRender(message) : null🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@console/web/src/components/chat/FunctionCallMessage.tsx` around lines 96 - 103, The component eagerly calls SandboxToolView.tryRenderPreview and SandboxToolView.tryRender for every message (creating sandboxPreview, sandboxTerminal, hasSandboxTerminal), which does unnecessary work when the row is collapsed; change this so those calls are only executed when the row is open (use the open prop) or move their computation inside the block that renders the expanded view, then recompute hasSandboxTerminal and showRequestPaneAbove based on those lazy values so collapsed rows avoid parsing/render work.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@console/web/src/components/chat/sandbox/ErrorView.tsx`:
- Around line 71-80: The external link rendering in ErrorView (component
ErrorView.tsx) uses error.docs_url directly; validate that the URL's protocol is
either "http:" or "https:" before rendering the <a> tag to avoid unsafe
protocols. Update the rendering logic that checks error.docs_url to parse or
test the value (e.g., new URL(...) or a startsWith/protocol check) and only
return the anchor when the protocol is http/https; otherwise render null or a
safe fallback, preserving the existing target/rel attributes.
In `@console/web/src/components/chat/sandbox/format.ts`:
- Around line 193-201: isStreamChannelRef currently only validates channel_id
but its type guard promises channel_id, access_key, and direction; update
isStreamChannelRef to also check that value is a non-null object and that
'access_key' and 'direction' are in value and are strings (e.g., add
"'access_key' in value && typeof (value as { access_key: unknown }).access_key
=== 'string' && 'direction' in value && typeof (value as { direction: unknown
}).direction === 'string'") so the narrowed type truly matches the runtime
validation.
In `@console/web/src/components/chat/sandbox/FsLsView.tsx`:
- Line 54: The mode prefix currently uses `${e.is_dir ? 'd' : '-'}` which treats
symlinks as regular files; change the prefix logic in FsLsView (where the
template uses e.is_dir and formatMode) to check e.is_symlink first and emit 'l'
for symlinks, then 'd' for directories, otherwise '-' (e.g. if e.is_symlink ?
'l' : e.is_dir ? 'd' : '-'), leaving formatMode(e.mode) unchanged.
In `@console/web/src/components/chat/sandbox/FsSedView.tsx`:
- Around line 69-75: The TooltipTrigger currently wraps a non-focusable <span>,
preventing keyboard users from opening the tooltip; update the trigger so it is
keyboard-accessible by replacing or modifying the wrapped element referenced by
TooltipTrigger (the inline span around TriangleAlert and "err") to be focusable
and semantics-correct — e.g., use a real button element or add tabIndex=0 plus
role="button" and an accessible name/aria-describedby that references the
tooltip content (r.error) so keyboard users can focus and invoke the tooltip via
TooltipTrigger.
In `@console/web/src/components/chat/sandbox/parsers.ts`:
- Around line 566-576: parseSandboxErrorDisplay currently only inspects
value.error for a function_error envelope so wrapped payloads (e.g., {content,
details}) are skipped; update the parsing to also unwrap common wrappers before
calling functionErrorEnvelopeSchema.safeParse. Specifically, when encountering
an object in the branch using functionErrorEnvelopeSchema.safeParse, check for
nested candidates like value.content, value.details, and value.content?.error or
value.details?.error (falling back to the original value.error), then run
functionErrorEnvelopeSchema.safeParse on that unwrapped object and continue to
use invocationFromFunctionError and the existing { variant: 'invocation', error:
invocation } return path if parsed.success and kind === 'function_error'.
In `@console/web/src/components/chat/sandbox/RunView.tsx`:
- Around line 82-84: The source path label currently renders the function call
literally; inside RunView.tsx (the JSX span in RunView) replace the literal
"/tmp/run.{extFor(req.lang)}" with a proper JSX expression that concatenates or
templates the extension, e.g. use {`/tmp/run.${extFor(req.lang)}`} or
{'/tmp/run.' + extFor(req.lang)} so the extFor(req.lang) result is evaluated and
displayed.
In `@console/web/src/pages/Examples/sections/message-variants.tsx`:
- Around line 319-323: The VariantCard labels for sandboxFixtures use only
fixture.functionId causing duplicate titles; update the label in the
sandboxFixtures map (where VariantCard is rendered) to include a scenario suffix
such as fixture.id or fixture.state so each label is unique (e.g.,
label={`sandbox · ${fixture.functionId} · ${fixture.id}`} or include
fixture.state) to ensure distinct card titles when rendering sandboxFixtures.
In `@harness/src/provider-llamacpp/discover.ts`:
- Around line 147-149: The current isPositiveInteger type-guard allows positive
non-integers (e.g., 4096.5) which is invalid for n_ctx/context_window; change
its check to require an integer (use Number.isInteger(value) or value % 1 === 0)
and update any validation/usage of n_ctx/context_window in discover.ts to use
this corrected isPositiveInteger function so only positive integers pass
validation. Ensure the function signature remains value is number and that
callers that set n_ctx/context_window reject non-integer values.
In `@harness/src/turn-orchestrator/agent-trigger.ts`:
- Around line 185-188: The structured payload returned by
extractStructuredHandlerError can contain its own error key that overwrites the
intended discriminator; change the merge so the handler discriminator is applied
last (e.g., spread structured first then set error: 'handler_error') when
calling errorResult in the agent-trigger flow (see extractStructuredHandlerError
and errorResult usage) to ensure the 'handler_error' value is preserved.
---
Nitpick comments:
In `@console/web/src/components/chat/FunctionCallMessage.tsx`:
- Around line 96-103: The component eagerly calls
SandboxToolView.tryRenderPreview and SandboxToolView.tryRender for every message
(creating sandboxPreview, sandboxTerminal, hasSandboxTerminal), which does
unnecessary work when the row is collapsed; change this so those calls are only
executed when the row is open (use the open prop) or move their computation
inside the block that renders the expanded view, then recompute
hasSandboxTerminal and showRequestPaneAbove based on those lazy values so
collapsed rows avoid parsing/render work.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fa4489e9-3f3f-4fe7-82ba-0656212c3249
⛔ Files ignored due to path filters (4)
console/Cargo.lockis excluded by!**/*.lockdatabase/Cargo.lockis excluded by!**/*.lockshell/Cargo.lockis excluded by!**/*.lockstorage/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (73)
console/web/src/App.tsxconsole/web/src/components/chat/AutoAcceptToggle.tsxconsole/web/src/components/chat/ChatView.tsxconsole/web/src/components/chat/Composer.tsxconsole/web/src/components/chat/FunctionCallGroup.tsxconsole/web/src/components/chat/FunctionCallMessage.tsxconsole/web/src/components/chat/LexicalShell.tsxconsole/web/src/components/chat/MessageList.tsxconsole/web/src/components/chat/ModelPicker.tsxconsole/web/src/components/chat/sandbox/CodeHighlight.tsxconsole/web/src/components/chat/sandbox/CreateView.tsxconsole/web/src/components/chat/sandbox/ErrorView.tsxconsole/web/src/components/chat/sandbox/ExecView.tsxconsole/web/src/components/chat/sandbox/FsChmodView.tsxconsole/web/src/components/chat/sandbox/FsGrepView.tsxconsole/web/src/components/chat/sandbox/FsLsView.tsxconsole/web/src/components/chat/sandbox/FsMkdirView.tsxconsole/web/src/components/chat/sandbox/FsMvView.tsxconsole/web/src/components/chat/sandbox/FsReadView.tsxconsole/web/src/components/chat/sandbox/FsRmView.tsxconsole/web/src/components/chat/sandbox/FsSedView.tsxconsole/web/src/components/chat/sandbox/FsStatView.tsxconsole/web/src/components/chat/sandbox/FsWriteView.tsxconsole/web/src/components/chat/sandbox/ListView.tsxconsole/web/src/components/chat/sandbox/RunView.tsxconsole/web/src/components/chat/sandbox/StopView.tsxconsole/web/src/components/chat/sandbox/__tests__/parsers.test.tsconsole/web/src/components/chat/sandbox/format.tsconsole/web/src/components/chat/sandbox/index.tsxconsole/web/src/components/chat/sandbox/parsers.tsconsole/web/src/components/chat/sandbox/shared.tsxconsole/web/src/components/chat/sandbox/terminal/AnsiOutput.tsxconsole/web/src/components/chat/sandbox/terminal/Terminal.tsxconsole/web/src/components/providers/ProviderRow.tsxconsole/web/src/components/providers/ProviderSettingsDialog.tsxconsole/web/src/components/ui/LiveRegion.tsxconsole/web/src/components/ui/ModeToggle.tsxconsole/web/src/components/ui/Select.tsxconsole/web/src/hooks/use-auto-accept-approvals.tsconsole/web/src/hooks/use-chat-dock.tsconsole/web/src/hooks/use-functions-catalog.tsconsole/web/src/hooks/use-hash-route.tsconsole/web/src/hooks/use-live-announcer.tsconsole/web/src/lib/backend/auto-accept-policy.test.tsconsole/web/src/lib/backend/auto-accept-policy.tsconsole/web/src/lib/backend/auto-accept.test.tsconsole/web/src/lib/backend/auto-accept.tsconsole/web/src/lib/backend/history.tsconsole/web/src/lib/backend/pending-approvals-store.test.tsconsole/web/src/lib/conversations-context.tsxconsole/web/src/lib/format-stop-reason.test.tsconsole/web/src/lib/functions-catalog.tsconsole/web/src/lib/functions.tsconsole/web/src/lib/providers.test.tsconsole/web/src/lib/providers.tsconsole/web/src/lib/syntax.tsxconsole/web/src/pages/Configuration/index.tsxconsole/web/src/pages/Examples/sections/message-variants.tsxconsole/web/src/pages/Examples/sections/sandbox-fixtures.tsconsole/web/src/pages/Playground/scenarios/happy-ask.tsconsole/web/src/pages/Playground/scenarios/index.tsconsole/web/src/pages/Playground/scenarios/multi-function-agent.tsconsole/web/src/pages/Traces/components/SessionDetailPanel.tsxconsole/web/src/pages/Traces/components/WaterfallChart.tsxconsole/web/src/pages/Traces/hooks/useResizablePanels.tsconsole/web/src/pages/Traces/lib/traceTransform.test.tsconsole/web/src/pages/Traces/lib/traceTransform.tsconsole/web/src/types/chat.tsharness/src/provider-llamacpp/discover.tsharness/src/runtime/iii.tsharness/src/turn-orchestrator/agent-trigger.tsharness/tests/provider-llamacpp/discover.test.tsharness/tests/turn-orchestrator/agent-trigger.test.ts
Comment on lines
+71
to
+80
| {error.docs_url ? ( | ||
| <a | ||
| href={error.docs_url} | ||
| target="_blank" | ||
| rel="noreferrer noopener" | ||
| className="font-mono text-[11px] uppercase tracking-[0.06em] text-accent hover:underline w-fit" | ||
| > | ||
| docs ↗ | ||
| </a> | ||
| ) : null} |
There was a problem hiding this comment.
Validate docs_url protocol before rendering the external link.
docs_url comes from payload data and is inserted directly into href. Restrict it to http:/https: before rendering.
💡 Suggested fix
+function safeHttpUrl(url: string): string | null {
+ try {
+ const parsed = new URL(url)
+ return parsed.protocol === 'http:' || parsed.protocol === 'https:' ? url : null
+ } catch {
+ return null
+ }
+}
+
export function ErrorView({ error }: ErrorViewProps) {
const retryable = error.retryable === true
const streams = execStreamsFromFix(error)
+ const docsUrl = error.docs_url ? safeHttpUrl(error.docs_url) : null
return (
@@
- {error.docs_url ? (
+ {docsUrl ? (
<a
- href={error.docs_url}
+ href={docsUrl}
target="_blank"
rel="noreferrer noopener"
className="font-mono text-[11px] uppercase tracking-[0.06em] text-accent hover:underline w-fit"
>
docs ↗
</a>
) : null}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@console/web/src/components/chat/sandbox/ErrorView.tsx` around lines 71 - 80,
The external link rendering in ErrorView (component ErrorView.tsx) uses
error.docs_url directly; validate that the URL's protocol is either "http:" or
"https:" before rendering the <a> tag to avoid unsafe protocols. Update the
rendering logic that checks error.docs_url to parse or test the value (e.g., new
URL(...) or a startsWith/protocol check) and only return the anchor when the
protocol is http/https; otherwise render null or a safe fallback, preserving the
existing target/rel attributes.
Comment on lines
+193
to
+201
| export function isStreamChannelRef( | ||
| value: unknown, | ||
| ): value is { channel_id: string; access_key: string; direction: string } { | ||
| return ( | ||
| !!value && | ||
| typeof value === 'object' && | ||
| 'channel_id' in value && | ||
| typeof (value as { channel_id: unknown }).channel_id === 'string' | ||
| ) |
There was a problem hiding this comment.
Tighten isStreamChannelRef to match its declared narrowed type.
The predicate only checks channel_id, but it narrows to an object that also guarantees access_key and direction. That can misclassify payloads and cause downstream undefined access.
💡 Suggested fix
export function isStreamChannelRef(
value: unknown,
): value is { channel_id: string; access_key: string; direction: string } {
return (
!!value &&
typeof value === 'object' &&
'channel_id' in value &&
- typeof (value as { channel_id: unknown }).channel_id === 'string'
+ typeof (value as { channel_id: unknown }).channel_id === 'string' &&
+ 'access_key' in value &&
+ typeof (value as { access_key: unknown }).access_key === 'string' &&
+ 'direction' in value &&
+ ((value as { direction: unknown }).direction === 'read' ||
+ (value as { direction: unknown }).direction === 'write')
)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export function isStreamChannelRef( | |
| value: unknown, | |
| ): value is { channel_id: string; access_key: string; direction: string } { | |
| return ( | |
| !!value && | |
| typeof value === 'object' && | |
| 'channel_id' in value && | |
| typeof (value as { channel_id: unknown }).channel_id === 'string' | |
| ) | |
| export function isStreamChannelRef( | |
| value: unknown, | |
| ): value is { channel_id: string; access_key: string; direction: string } { | |
| return ( | |
| !!value && | |
| typeof value === 'object' && | |
| 'channel_id' in value && | |
| typeof (value as { channel_id: unknown }).channel_id === 'string' && | |
| 'access_key' in value && | |
| typeof (value as { access_key: unknown }).access_key === 'string' && | |
| 'direction' in value && | |
| ((value as { direction: unknown }).direction === 'read' || | |
| (value as { direction: unknown }).direction === 'write') | |
| ) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@console/web/src/components/chat/sandbox/format.ts` around lines 193 - 201,
isStreamChannelRef currently only validates channel_id but its type guard
promises channel_id, access_key, and direction; update isStreamChannelRef to
also check that value is a non-null object and that 'access_key' and 'direction'
are in value and are strings (e.g., add "'access_key' in value && typeof (value
as { access_key: unknown }).access_key === 'string' && 'direction' in value &&
typeof (value as { direction: unknown }).direction === 'string'") so the
narrowed type truly matches the runtime validation.
| {e.is_dir ? '—' : formatBytes(e.size)} | ||
| </td> | ||
| <td className="px-2 py-1.5 text-ink-faint tabular-nums"> | ||
| {`${e.is_dir ? 'd' : '-'}${formatMode(e.mode)}`} |
There was a problem hiding this comment.
Render symlink mode prefix correctly.
Line 54 treats symlinks as regular files in the mode prefix. For ls-style output, symlinks should use l.
💡 Suggested fix
- {`${e.is_dir ? 'd' : '-'}${formatMode(e.mode)}`}
+ {`${e.is_symlink ? 'l' : e.is_dir ? 'd' : '-'}${formatMode(e.mode)}`}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| {`${e.is_dir ? 'd' : '-'}${formatMode(e.mode)}`} | |
| {`${e.is_symlink ? 'l' : e.is_dir ? 'd' : '-'}${formatMode(e.mode)}`} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@console/web/src/components/chat/sandbox/FsLsView.tsx` at line 54, The mode
prefix currently uses `${e.is_dir ? 'd' : '-'}` which treats symlinks as regular
files; change the prefix logic in FsLsView (where the template uses e.is_dir and
formatMode) to check e.is_symlink first and emit 'l' for symlinks, then 'd' for
directories, otherwise '-' (e.g. if e.is_symlink ? 'l' : e.is_dir ? 'd' : '-'),
leaving formatMode(e.mode) unchanged.
Comment on lines
+69
to
+75
| <Tooltip> | ||
| <TooltipTrigger asChild> | ||
| <span className="inline-flex items-center gap-1 text-warn cursor-help"> | ||
| <TriangleAlert aria-hidden className="w-3.5 h-3.5" /> | ||
| err | ||
| </span> | ||
| </TooltipTrigger> |
There was a problem hiding this comment.
Make the error tooltip trigger keyboard-accessible.
On Line 70, TooltipTrigger wraps a non-focusable <span>, so keyboard users can’t open the tooltip and read r.error.
♿ Suggested fix
) : r.error ? (
<Tooltip>
<TooltipTrigger asChild>
- <span className="inline-flex items-center gap-1 text-warn cursor-help">
+ <button
+ type="button"
+ className="inline-flex items-center gap-1 text-warn cursor-help focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-accent"
+ aria-label={`Show error details for ${r.path}`}
+ >
<TriangleAlert aria-hidden className="w-3.5 h-3.5" />
err
- </span>
+ </button>
</TooltipTrigger>
<TooltipContent>{r.error}</TooltipContent>
</Tooltip>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <Tooltip> | |
| <TooltipTrigger asChild> | |
| <span className="inline-flex items-center gap-1 text-warn cursor-help"> | |
| <TriangleAlert aria-hidden className="w-3.5 h-3.5" /> | |
| err | |
| </span> | |
| </TooltipTrigger> | |
| <Tooltip> | |
| <TooltipTrigger asChild> | |
| <button | |
| type="button" | |
| className="inline-flex items-center gap-1 text-warn cursor-help focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-accent" | |
| aria-label={`Show error details for ${r.path}`} | |
| > | |
| <TriangleAlert aria-hidden className="w-3.5 h-3.5" /> | |
| err | |
| </button> | |
| </TooltipTrigger> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@console/web/src/components/chat/sandbox/FsSedView.tsx` around lines 69 - 75,
The TooltipTrigger currently wraps a non-focusable <span>, preventing keyboard
users from opening the tooltip; update the trigger so it is keyboard-accessible
by replacing or modifying the wrapped element referenced by TooltipTrigger (the
inline span around TriangleAlert and "err") to be focusable and
semantics-correct — e.g., use a real button element or add tabIndex=0 plus
role="button" and an accessible name/aria-describedby that references the
tooltip content (r.error) so keyboard users can focus and invoke the tooltip via
TooltipTrigger.
Comment on lines
+566
to
+576
| if (value && typeof value === 'object' && !Array.isArray(value)) { | ||
| const parsed = functionErrorEnvelopeSchema.safeParse( | ||
| (value as Record<string, unknown>).error, | ||
| ) | ||
| if (parsed.success && parsed.data.kind === 'function_error') { | ||
| const invocation = invocationFromFunctionError(parsed.data) | ||
| if (invocation) { | ||
| return { variant: 'invocation', error: invocation } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Handle wrapped function_error envelopes in the same candidate pass.
parseSandboxErrorDisplay only checks value.error for function_error. When the payload is wrapped (for example via { content, details }), this branch is skipped even though candidates already contain unwrapped objects.
💡 Suggested fix
export function parseSandboxErrorDisplay(
value: unknown,
): SandboxErrorDisplay | null {
const candidates = collectErrorCandidates(value)
@@
for (const candidate of candidates) {
+ if (candidate && typeof candidate === 'object' && !Array.isArray(candidate)) {
+ const obj = candidate as Record<string, unknown>
+ const parsed = functionErrorEnvelopeSchema.safeParse(obj.error)
+ if (parsed.success && parsed.data.kind === 'function_error') {
+ const invocation = invocationFromFunctionError(parsed.data)
+ if (invocation) return { variant: 'invocation', error: invocation }
+ }
+ }
+
const denial = tryParseDenial(candidate)
if (denial) {
return {
variant: 'invocation',
error: denialToInvocation(denial),
}
}
}
-
- if (value && typeof value === 'object' && !Array.isArray(value)) {
- const parsed = functionErrorEnvelopeSchema.safeParse(
- (value as Record<string, unknown>).error,
- )
- if (parsed.success && parsed.data.kind === 'function_error') {
- const invocation = invocationFromFunctionError(parsed.data)
- if (invocation) {
- return { variant: 'invocation', error: invocation }
- }
- }
- }
return null
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (value && typeof value === 'object' && !Array.isArray(value)) { | |
| const parsed = functionErrorEnvelopeSchema.safeParse( | |
| (value as Record<string, unknown>).error, | |
| ) | |
| if (parsed.success && parsed.data.kind === 'function_error') { | |
| const invocation = invocationFromFunctionError(parsed.data) | |
| if (invocation) { | |
| return { variant: 'invocation', error: invocation } | |
| } | |
| } | |
| } | |
| export function parseSandboxErrorDisplay( | |
| value: unknown, | |
| ): SandboxErrorDisplay | null { | |
| const candidates = collectErrorCandidates(value) | |
| for (const candidate of candidates) { | |
| if (candidate && typeof candidate === 'object' && !Array.isArray(candidate)) { | |
| const obj = candidate as Record<string, unknown> | |
| const parsed = functionErrorEnvelopeSchema.safeParse(obj.error) | |
| if (parsed.success && parsed.data.kind === 'function_error') { | |
| const invocation = invocationFromFunctionError(parsed.data) | |
| if (invocation) return { variant: 'invocation', error: invocation } | |
| } | |
| } | |
| const denial = tryParseDenial(candidate) | |
| if (denial) { | |
| return { | |
| variant: 'invocation', | |
| error: denialToInvocation(denial), | |
| } | |
| } | |
| } | |
| return null | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@console/web/src/components/chat/sandbox/parsers.ts` around lines 566 - 576,
parseSandboxErrorDisplay currently only inspects value.error for a
function_error envelope so wrapped payloads (e.g., {content, details}) are
skipped; update the parsing to also unwrap common wrappers before calling
functionErrorEnvelopeSchema.safeParse. Specifically, when encountering an object
in the branch using functionErrorEnvelopeSchema.safeParse, check for nested
candidates like value.content, value.details, and value.content?.error or
value.details?.error (falling back to the original value.error), then run
functionErrorEnvelopeSchema.safeParse on that unwrapped object and continue to
use invocationFromFunctionError and the existing { variant: 'invocation', error:
invocation } return path if parsed.success and kind === 'function_error'.
Comment on lines
+82
to
+84
| <span className="text-ink-ghost normal-case tracking-normal"> | ||
| · /tmp/run.{extFor(req.lang)} · {lineCount}{' '} | ||
| {lineCount === 1 ? 'line' : 'lines'} |
There was a problem hiding this comment.
Fix literal interpolation in source path label.
Line 83 renders extFor(req.lang) as plain text, so users see /tmp/run.{extFor(req.lang)} instead of the actual extension.
💡 Suggested fix
- <span className="text-ink-ghost normal-case tracking-normal">
- · /tmp/run.{extFor(req.lang)} · {lineCount}{' '}
+ <span className="text-ink-ghost normal-case tracking-normal">
+ {` · /tmp/run.${extFor(req.lang)} · ${lineCount} `}
{lineCount === 1 ? 'line' : 'lines'}
</span>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@console/web/src/components/chat/sandbox/RunView.tsx` around lines 82 - 84,
The source path label currently renders the function call literally; inside
RunView.tsx (the JSX span in RunView) replace the literal
"/tmp/run.{extFor(req.lang)}" with a proper JSX expression that concatenates or
templates the extension, e.g. use {`/tmp/run.${extFor(req.lang)}`} or
{'/tmp/run.' + extFor(req.lang)} so the extFor(req.lang) result is evaluated and
displayed.
Comment on lines
+319
to
+323
| {sandboxFixtures.map((fixture) => ( | ||
| <VariantCard | ||
| key={fixture.id} | ||
| label={`sandbox · ${fixture.functionId}`} | ||
| > |
There was a problem hiding this comment.
Make sandbox variant labels unique per fixture.
Using only functionId creates duplicate card titles (for example multiple sandbox::exec states), which makes the examples harder to scan. Include a scenario suffix (e.g., fixture.id or state) in the label.
Suggested tweak
- label={`sandbox · ${fixture.functionId}`}
+ label={`sandbox · ${fixture.functionId} · ${fixture.id}`}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| {sandboxFixtures.map((fixture) => ( | |
| <VariantCard | |
| key={fixture.id} | |
| label={`sandbox · ${fixture.functionId}`} | |
| > | |
| {sandboxFixtures.map((fixture) => ( | |
| <VariantCard | |
| key={fixture.id} | |
| label={`sandbox · ${fixture.functionId} · ${fixture.id}`} | |
| > |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@console/web/src/pages/Examples/sections/message-variants.tsx` around lines
319 - 323, The VariantCard labels for sandboxFixtures use only
fixture.functionId causing duplicate titles; update the label in the
sandboxFixtures map (where VariantCard is rendered) to include a scenario suffix
such as fixture.id or fixture.state so each label is unique (e.g.,
label={`sandbox · ${fixture.functionId} · ${fixture.id}`} or include
fixture.state) to ensure distinct card titles when rendering sandboxFixtures.
Comment on lines
+147
to
+149
| function isPositiveInteger(value: unknown): value is number { | ||
| return typeof value === 'number' && Number.isFinite(value) && value > 0; | ||
| } |
There was a problem hiding this comment.
Enforce integer validation for n_ctx.
Line 148 accepts positive non-integers (for example 4096.5), but n_ctx/context_window should be an integer.
Suggested fix
function isPositiveInteger(value: unknown): value is number {
- return typeof value === 'number' && Number.isFinite(value) && value > 0;
+ return typeof value === 'number' && Number.isSafeInteger(value) && value > 0;
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@harness/src/provider-llamacpp/discover.ts` around lines 147 - 149, The
current isPositiveInteger type-guard allows positive non-integers (e.g., 4096.5)
which is invalid for n_ctx/context_window; change its check to require an
integer (use Number.isInteger(value) or value % 1 === 0) and update any
validation/usage of n_ctx/context_window in discover.ts to use this corrected
isPositiveInteger function so only positive integers pass validation. Ensure the
function signature remains value is number and that callers that set
n_ctx/context_window reject non-integer values.
Comment on lines
+185
to
+188
| const structured = extractStructuredHandlerError(err); | ||
| if (structured) { | ||
| return errorResult({ error: 'handler_error', ...structured }); | ||
| } |
There was a problem hiding this comment.
Preserve the handler_error discriminator when merging structured payloads.
At Line 187, ...structured can overwrite error: 'handler_error' if the envelope already has an error key.
Suggested fix
- return errorResult({ error: 'handler_error', ...structured });
+ return errorResult({ ...structured, error: 'handler_error' });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const structured = extractStructuredHandlerError(err); | |
| if (structured) { | |
| return errorResult({ error: 'handler_error', ...structured }); | |
| } | |
| const structured = extractStructuredHandlerError(err); | |
| if (structured) { | |
| return errorResult({ ...structured, error: 'handler_error' }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@harness/src/turn-orchestrator/agent-trigger.ts` around lines 185 - 188, The
structured payload returned by extractStructuredHandlerError can contain its own
error key that overwrites the intended discriminator; change the merge so the
handler discriminator is applied last (e.g., spread structured first then set
error: 'handler_error') when calling errorResult in the agent-trigger flow (see
extractStructuredHandlerError and errorResult usage) to ensure the
'handler_error' value is preserved.
sergiofilhowz
approved these changes
May 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
chat/sandbox/module renderssandbox::*tool calls as rich, tool-specific views (exec, run, stop, list, create, and 10fs.*operations) with ANSI terminal output, code highlighting, and a unified error view.FunctionCallMessageshows a sandbox preview while a call is pending/running, and switches to terminal/raw-json tabs once it completes.n_ctxfrom the server-wideGET /propsendpoint to populatecontext_window, with graceful fallback to the embedded default on offline / non-2xx / malformed responses.docs_url,fix,retryable) are extracted fromIIIInvocationErrorand forwarded viaerrorResultinstead of being buried ingate_unavailable.reason— agents now see the full error contract fromsandbox::*handlers.Test plan
cd console/web && pnpm test— unit tests including newsandbox/__tests__/parsers.test.tspasscd harness && pnpm test— covers updatedprovider-llamacpp/discover.test.tsandturn-orchestrator/agent-trigger.test.tsn_ctxinstead of the fallback defaultsandbox::*handler error in chat: agent receives structured envelope withcode+docs_url, not agate_unavailablewrapperSummary by CodeRabbit
Release Notes
New Features
Improvements