Skip to content

Read a sandboxed tool's refusal under the id its renderer is given - #402

Open
kevin9327 wants to merge 1 commit into
CopilotKit:mainfrom
kevin9327:fix/sandboxed-refusal-key
Open

Read a sandboxed tool's refusal under the id its renderer is given#402
kevin9327 wants to merge 1 commit into
CopilotKit:mainfrom
kevin9327:fix/sandboxed-refusal-key

Conversation

@kevin9327

Copy link
Copy Markdown
Contributor

What this changes

A sandboxed component that the server refused is drawn anyway. The refusal card cannot appear,
because the key it is looked up under is always undefined.

The handler records the refusal under the id it is given:

handler: async (_args, context: { toolCall?: { id?: string } } = {}) => {
  const id = context?.toolCall?.id;
  if (id) setRefusals((current) => new Map(current).set(id, reason));

and the renderer reads it back under a different one:

const refusal = props.toolCall?.id ? refusals.get(props.toolCall.id) : undefined;

A renderer's props are not a handler's context. The id arrives flat, as toolCallId. Both siblings
in this folder already read it that way — gallery-tools.tsx:93 types its props
{ toolCallId?: string; … }, and computer-tools.tsx:296 destructures
({ result, status, toolCallId }) — and so does the library's own renderer:

function DefaultToolCallRenderer({ name, toolCallId, parameters, status, result })

computer-tools.tsx:274 even converts between the two shapes deliberately, sending
{ toolCallId: toolCall.id } from a handler. This is the one place the two were confused.

props.toolCall is not a field that exists, so the expression is undefined, refusals.get is
never called, and render falls through to the isHeld branch and draws the component.

What it costs

The decision request is what makes "revoke it and watch it go" true rather than nearly true — the
file says so itself:

The list of tools a run is offered is a snapshot taken when that run started, so a grant revoked
one second later is still in the model's hands. Asking at call time is what makes "revoke it and
watch it go" true rather than nearly true

That question is still asked and the refusal is still audited on the server. Only the answer on
screen was lost. Two reachable cases:

  • an administrator revokes a component from a Bot, or unpublishes it, while a conversation is open.
    The tool list is refreshed by a five-second poll, so until it catches up the component keeps being
    drawn instead of showing why it should not be.
  • the decision request itself fails. The handler returns the reason to the model, and the person
    sees the component render as though nothing happened.

Fix

Read props.toolCallId, and type the props as the sibling does. The type is the point as much as
the access: with the props typed { toolCallId?: string; … }, the old line is a compile error
rather than a silent undefined.

app typecheck: src/lib/copilot/sandboxed-tools.tsx(90,29): error TS2551:
  Property 'toolCall' does not exist on type
  '{ args?: ...; status?: ...; toolCallId?: string | undefined; }'.
  Did you mean 'toolCallId'?

That is this change with only the property access put back, and it is the regression guard: the
mistake cannot be made again without bun run --filter app typecheck saying so. I did not add a
runtime test — reaching this renderer means standing up the CopilotKit provider and a live tool
registration around a hook, and the assertion it would make is the one the compiler now makes for
free.

Where it runs

  • New state that outlives a request? None. refusals is per-component-instance React state
    and stays that way; this fixes which key it is read under.
  • What happens on the second replica? Unchanged — this is browser code. The decision it
    renders comes from POST /components/:name/decision, which is answered from Postgres, so
    every replica gives the same verdict and this now shows it.
  • Anything serialised? No.
  • Anything fanned out to a browser? No new fan-out. The existing five-second grant poll is
    untouched; this makes the call-time refusal visible in the window before it fires.
  • New listener, port, or schedule? None.

Boundary and audit

  • Every acting call still goes through the gateway: decideComponent is unchanged and still
    runs before anything is drawn.
  • New refusals and new failures each write a row: the server already writes component.refused
    for this path. No row was missing — the row was written and the person was shown the
    component anyway, which is the gap this closes.
  • Nothing new is trusted from the client: nothing new is read at all; one existing field is read
    under its real name.

Changelog

  • A line in CHANGELOG.md under Unreleased.

How I tested

Windows 11, Bun 1.3.14. bun run --filter app typecheck clean, and failing with TS2551 when the old
access is put back. bun test app/tests is 319 passed, 1 failed — the failure is
skill-creator-slug.test.ts, which is a pre-existing Windows path issue on main and the subject of
#394. bunx biome check clean.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant