Correcting a command before allowing it
An approval card was Allow, Always, or Don't. A model proposing the right command with one flag wrong therefore cost a whole round trip to explain in prose. There is an Edit button on it now. Where the edit lands is the whole of the feature, and it is one line. `arguments` is the list `_run_calls` hands to `run_tool` as `parsed=`, and `run_tool` never re-parses -- so writing into it inside `_authorise` is the only mutation the runner can see. Editing the Item would do nothing: it is frozen and display-only. Two things had to move with it. The raw `call["arguments"]` string is rewritten beside the parsed dict, and the assistant turn is now built *after* `_authorise` rather than before it -- the old order told the model it ran what it proposed while something else ran, and every later round would have reasoned from a transcript that was quietly false. And `_remember_always` reads the edit, or "always allow this" would store a standing permission for a command nobody approved; it still derives the pattern itself through `policy.subject`, which yields nothing for a composed command line. Nothing is re-checked against the mode or the lists, and that is not a shortcut. The deny list resolves to ASK rather than to a refusal -- it means "always ask about this" -- so a person who has typed the command and pressed Allow is exactly the asking it was demanding, and re-asking would put the same card up with no way past it. It is the line the terminal panel already draws. The box is only offered where the detail *is* an argument and can be put back: a tool with no entry in `tool_labels.DETAIL_KEYS` gets a `k=repr(v)` summary, and a box there would silently change nothing. Both halves are always in the DOM with one hidden, rather than the field being created on click -- a field that does not exist until a handler runs is a field that submits nothing if the handler fails, and this one decides what runs on somebody's machine. The transcript says "edited by you". Attributing somebody's own typing to a model is the same misattribution as the other way round. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -17,6 +17,7 @@ from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import contextlib
|
||||
import json
|
||||
import logging
|
||||
import time
|
||||
import uuid
|
||||
@@ -585,15 +586,6 @@ async def _run(generation: Generation) -> None:
|
||||
generation.touch()
|
||||
break
|
||||
|
||||
messages = [
|
||||
# The **raw** arguments string, not the parsed dict: the
|
||||
# endpoint has to see back exactly what it sent, or an
|
||||
# id-matching server pairs its own call with something it does
|
||||
# not recognise.
|
||||
*payload["messages"],
|
||||
tools_service.assistant_turn(calls, "".join(round_text)),
|
||||
]
|
||||
|
||||
# Parsed once, here, and shared by everything below: the approval
|
||||
# card, `policy.decide`, and the runner. See `_arguments_for`.
|
||||
arguments = _arguments_for(tool_context, calls)
|
||||
@@ -602,10 +594,27 @@ async def _run(generation: Generation) -> None:
|
||||
# together under a semaphore, and four people-shaped pauses inside
|
||||
# that gather would queue behind each other invisibly -- see
|
||||
# services/interaction.py.
|
||||
decided, allowed = await _authorise(generation, tool_context, calls, arguments)
|
||||
decided, allowed, edited = await _authorise(
|
||||
generation, tool_context, calls, arguments
|
||||
)
|
||||
if generation.stopped:
|
||||
break
|
||||
|
||||
messages = [
|
||||
# The **raw** arguments string, not the parsed dict: the
|
||||
# endpoint has to see back exactly what it sent, or an
|
||||
# id-matching server pairs its own call with something it does
|
||||
# not recognise.
|
||||
#
|
||||
# Built after `_authorise` rather than before it, because a
|
||||
# command corrected on the approval card is written back into
|
||||
# `calls` there. The other order sent the model the command it
|
||||
# proposed while a different one ran, and every later round
|
||||
# reasoned from a transcript that was quietly false.
|
||||
*payload["messages"],
|
||||
tools_service.assistant_turn(calls, "".join(round_text)),
|
||||
]
|
||||
|
||||
generation.status = _tool_status(calls)
|
||||
generation.touch()
|
||||
try:
|
||||
@@ -616,7 +625,14 @@ async def _run(generation: Generation) -> None:
|
||||
generation.status = ""
|
||||
generation.touch()
|
||||
|
||||
for call, outcome in zip(calls, outcomes, strict=True):
|
||||
for index, (call, outcome) in enumerate(zip(calls, outcomes, strict=True)):
|
||||
if index in edited:
|
||||
# A command somebody corrected on the card is theirs, not
|
||||
# the model's. Shown as such, for the same reason a plan
|
||||
# goes back quoted and attributed: text must not arrive
|
||||
# wearing an authorship it does not have, in either
|
||||
# direction.
|
||||
outcome.event["edited"] = True
|
||||
generation.tool_events.append(outcome.event)
|
||||
generation.output_bytes += len(outcome.content)
|
||||
messages.append(tools_service.tool_turn(call, outcome.content))
|
||||
@@ -1102,6 +1118,12 @@ def _approvals(context, calls: list[dict], arguments: list[dict]) -> list[intera
|
||||
detail=detail,
|
||||
reason=decision.reason,
|
||||
purpose=agent_tools.why_of(args),
|
||||
# A card showing one argument can offer to correct it. A model
|
||||
# proposing the right command with one flag wrong is the common
|
||||
# case, and Allow-or-Don't makes that a whole round trip to
|
||||
# explain. Anything whose detail is a summary rather than a
|
||||
# value cannot be put back and is not offered the box.
|
||||
editable=bool(tool_labels.DETAIL_KEYS.get(call["name"])),
|
||||
)
|
||||
)
|
||||
return items
|
||||
@@ -1171,12 +1193,13 @@ def _questions_in(args: dict) -> list[dict]:
|
||||
|
||||
async def _authorise(
|
||||
generation, context, calls: list[dict], arguments: list[dict]
|
||||
) -> tuple[dict[int, ToolOutcome], set[int]]:
|
||||
) -> tuple[dict[int, ToolOutcome], set[int], set[int]]:
|
||||
"""Which of this round's calls may run, and what the others answer instead.
|
||||
|
||||
Returns outcomes keyed by the call's index. Every index the caller does not
|
||||
find here is cleared to run; every index it does find is answered without
|
||||
the runner being reached at all. That is what keeps
|
||||
Returns outcomes keyed by the call's index, the indices a person allowed,
|
||||
and the indices whose command they corrected on the way. Every index the
|
||||
caller does not find in the first is cleared to run; every index it does
|
||||
find is answered without the runner being reached at all. That is what keeps
|
||||
`zip(calls, outcomes, strict=True)` aligned -- an endpoint matching on
|
||||
`tool_call_id` pairs the wrong content with the right id otherwise.
|
||||
|
||||
@@ -1184,12 +1207,17 @@ async def _authorise(
|
||||
told. They re-check the mode as a backstop and would otherwise refuse the
|
||||
very thing that was just approved -- the mode says "ask", and asking is what
|
||||
happened.
|
||||
|
||||
A command corrected on the card is written back into `arguments` **in
|
||||
place**, because that same list is what `_run_calls` hands to `run_tool` as
|
||||
`parsed=` and `run_tool` never re-parses. Editing the item would do nothing:
|
||||
`Item` is display-only and frozen. This is the one place the two meet.
|
||||
"""
|
||||
questions = _ask_items(context, calls, arguments)
|
||||
approvals = _approvals(context, calls, arguments)
|
||||
items = [*approvals, *questions]
|
||||
if not items:
|
||||
return {}, set()
|
||||
return {}, set(), set()
|
||||
|
||||
timeout = float(context.interaction_timeout or 900)
|
||||
pause = interaction.build(uuid.uuid4().hex, items, timeout=timeout)
|
||||
@@ -1199,19 +1227,25 @@ async def _authorise(
|
||||
|
||||
if reply.ended:
|
||||
generation.stopped = True
|
||||
return {}, set()
|
||||
return {}, set(), set()
|
||||
|
||||
decided: dict[int, ToolOutcome] = {}
|
||||
allowed: set[int] = set()
|
||||
edited: set[int] = set()
|
||||
|
||||
# An approval that came back as a refusal answers its call without the
|
||||
# runner being reached; one that came back allowed is simply left out, which
|
||||
# is how `_run_calls` is told to go ahead.
|
||||
for item in approvals:
|
||||
if reply.permitted:
|
||||
allowed.add(item.index)
|
||||
if not reply.permitted:
|
||||
decided[item.index] = _not_allowed(item, reply)
|
||||
continue
|
||||
decided[item.index] = _not_allowed(item, reply)
|
||||
if _apply_edit(calls, arguments, item, reply) != item.detail:
|
||||
# So the transcript can say the command was changed before it ran.
|
||||
# Without it a reader scrolling back sees a command attributed to
|
||||
# the model that the model never wrote.
|
||||
edited.add(item.index)
|
||||
allowed.add(item.index)
|
||||
|
||||
# Questions are grouped back by call, because one `ask_user` call may have
|
||||
# carried several and the endpoint expects exactly one tool turn per call.
|
||||
@@ -1221,7 +1255,52 @@ async def _authorise(
|
||||
for index, asked in grouped.items():
|
||||
decided[index] = _answered(asked, reply)
|
||||
|
||||
return decided, allowed
|
||||
return decided, allowed, edited
|
||||
|
||||
|
||||
def _apply_edit(
|
||||
calls: list[dict],
|
||||
arguments: list[dict],
|
||||
item: interaction.Item,
|
||||
reply: interaction.Reply,
|
||||
) -> str:
|
||||
"""Put a corrected command back where the runner will find it.
|
||||
|
||||
Returns what is going to run, edited or not, so the caller can record the
|
||||
right thing. Two writes, and both are needed:
|
||||
|
||||
`arguments[index]` is what `run_tool` is handed as `parsed=`, and it never
|
||||
re-parses -- so this is the only write that reaches the runner. Editing the
|
||||
item would do nothing at all: `Item` is frozen and display-only.
|
||||
|
||||
`call["arguments"]`, the raw string, is rewritten beside it, because that is
|
||||
what goes back to the endpoint as the assistant turn. Otherwise the model is
|
||||
told it ran what it proposed rather than what actually ran, and every later
|
||||
round reasons from a transcript that is quietly false.
|
||||
|
||||
Nothing is re-checked against the mode or the lists. That is the same line
|
||||
the terminal panel and the directory browser draw, and here it is not even
|
||||
close: the deny list resolves to ASK rather than to a refusal -- it means
|
||||
"always ask about this" -- and a person who has typed the command themselves
|
||||
and pressed Allow is exactly the asking it was demanding. Re-asking would
|
||||
put the same card up again with no way past it. The instance's list still
|
||||
governs the *model*: a pattern remembered by "always allow" is checked by
|
||||
`decide`, where a deny hit wins before the allow list is even read.
|
||||
"""
|
||||
if not item.editable:
|
||||
return item.detail
|
||||
|
||||
edited = reply.answer_to(item)
|
||||
key = tool_labels.DETAIL_KEYS.get(item.tool_name)
|
||||
if not edited or edited == item.detail or not key:
|
||||
return item.detail
|
||||
|
||||
arguments[item.index] = {**arguments[item.index], key: edited}
|
||||
calls[item.index] = {
|
||||
**calls[item.index],
|
||||
"arguments": json.dumps(arguments[item.index]),
|
||||
}
|
||||
return edited
|
||||
|
||||
|
||||
def _not_allowed(item: interaction.Item, reply: interaction.Reply) -> ToolOutcome:
|
||||
|
||||
@@ -85,6 +85,12 @@ class Item:
|
||||
purpose: str = ""
|
||||
options: tuple[str, ...] = ()
|
||||
allow_free_text: bool = True
|
||||
# Whether `detail` can be corrected before this is allowed. Only where the
|
||||
# detail *is* one argument and can be put back where it came from -- a tool
|
||||
# with no entry in `tool_labels.DETAIL_KEYS` gets a `k=repr(v)` summary that
|
||||
# cannot be parsed back, and offering a box that silently changed nothing
|
||||
# would be worse than offering none.
|
||||
editable: bool = False
|
||||
|
||||
|
||||
@dataclass
|
||||
|
||||
Reference in New Issue
Block a user