Uh oh!
There was an error while loading. Please reload this page.
fix(cu): let a model read back the Computer Use call it actually made - #1891
Conversation
7e276a7 to
f09d53dComparef09d53d to
8f597feComparehqhq1025
commented
Aug 3, 2026
Pushed fixes for all six findings. The claim in the PR body that nothing behaves differently was wrong, and finding 1 was the reason. Finding 1, resolution: I scoped the constant to what main's schema accepts rather than stacking the PR on the executor branch. Verified first — main's wire enum at computer-use-tools.ts is list_apps, observe, click_element, set_value, select_text, secondary_action, press_key plus CU_ACTION_TYPES, and none of launch_app, scroll_element, element_sequence or window_action is in it. The four names went into APPROVAL_ACTIONS through COMPUTER_USE_SEMANTIC_ACTIONS, so computerUseApprovalSummary flipped knownAction to true for calls the SDK rejects before the tool runs, and rememberForTurnAllowed could evaluate true for the two that also joined SEMANTIC_ACTIONS. I chose scoping over stacking for two reasons. The constant's stated purpose is to be held against the tool schema, so declaring names the schema does not have is the same drift it exists to prevent, only in the direction nothing was checking. And stacking would make the PR unmergeable until the executor lands, which delays the parts that are correct today. The executor's vocabulary belongs on the branch that also ships the executor. Finding 2: the promised file did not exist, but a guard did — computer-use-schema-parity.test.ts, added in #1870 after this branch was cut. It only walks from the schemas to the catalog, which catches a missing name and not an extra one, so the widening in finding 1 would have passed it. I added the reverse comparison to that file rather than creating the second one the comment named, and corrected the comment to point at the real file. Negative control: reintroducing the four names turns the new check red and it names all four. There is also an in-file control that runs the same comparator over a fixed catalog, so it keeps proving the comparator can return something on the day the real catalog is correct. Finding 3: confirmed. Only COMPUTER_USE_SEMANTIC_ACTIONS was exported, so computerUseModelCallArgs was unreachable from @maka/runtime and the whole headline fix was dead code while the harmful widening was the only part that shipped. The five missing symbols are exported now, and the projection is wired into projectPiToolArgs in pi-agent-backend.ts, which writes the persisted tool_call message and the tool_start event. Negative control: reverting that one line turns the pi-agent-backend projection test red with the approval dialect back in the diff. Finding 4: confirmed and dropped rather than wired. ComputerUseSettings appears in none of AppSettings, UpdateAppSettingsInput, the defaults, the merge or the normalize pass, unlike every peer settings module, and select-backend.ts assembles the CU tools with no enable check anywhere. A type that says enabled: false while the shipped app always has it on is worse than no type. Wiring a real switch needs a UI and a migration and does not belong under a types commit. Finding 5: confirmed, and it is exactly as described. text is six arguments under one name — the key for press_key, key and hold_key, the element action name for secondary_action, a screen substring for select_text, typed content for type. MODEL_CALL_PLAIN_VALUES is now a map from action to the argument names that are plain for that action. An action the tool does not accept has no entry, so it falls through to shapes rather than to whichever set was used last. Negative control: reverting to the name-keyed set turns two tests red, showing press_key coming back as text: . Finding 6, partially rejected with evidence. dispatch_refused is gone. It is not a declaration — COMPUTER_USE_ERROR_CODES is a runtime value feeding isComputerUseErrorCode, which decides at cua-driver-result.ts:88 whether a raw string from the driver process is trusted or replaced with capture_failed. Widening that with no producer is a live change. The other five named items are different in kind. messageIsAppTextFree, truncated, actions, subrole and placeholder are optional fields on TypeScript interfaces with no runtime effect, and they are not distinguishable from about ten other unproduced declarations in the same diff that the review accepted — focused, appAlias, obscuringRects, menu, query, CuLaunchedApp, scroll_element, window_action, launchApp. Dropping the named five and keeping those would be arbitrary; dropping all of them empties the branch of its purpose. Declaring the shape an executor returns is what this branch is for, so they stay, and the absent-means-withheld polarity on messageIsAppTextFree is kept as suggested. Also rebased onto current main, which the branch was one commit behind — the missing commit was #1870, the parity test finding 2 turns out to depend on. Test delta, measured against a baseline taken on clean origin/main on the same machine. This machine's Node has no node:sqlite, so @maka/runtime carries 64 pre-existing failing test names there. After the change: 2722 tests, zero new failures and zero fixed against that set. @maka/core goes 703/703 to 710/710, the seven new ones being the model-call projection tests. biome check clean on the six touched files, check-console passes. |
hqhq1025
commented
Aug 3, 2026
Second-round review addressed. Two findings fixed, one dropped after checking, one scope decision. Coordinates are back. The doc said typed text, written values and coordinates are screen-derived and stay out, and that is false for the third one: a coordinate is the model's own output, four digits it chose and sent. Reduced to The out-of-enum action is reported as sent. On the AI SDK path the question is moot — a schema-rejected call never produces a The declared-and-unconsumed set is dropped. What is left is what has a consumer: Negative controls, run rather than asserted. Reverting the geometry branch turned the coordinate tests red on One thing I did not change, because it is outside this branch: Test delta against a clean rebuild of main on this machine: none in |
0fcfb46 to
03fe3d9Comparehqhq1025
commented
Aug 3, 2026
Blocking finding addressed: the fix is moved to the path the shipping app runs. It was wired into PiAgentBackend, which the desktop app never instantiates. The new assertion is on the tool_start event, which is what model-history.ts On the coupling with #1888 and the privacy boundary, since the description computer-use-privacy-boundary.test.ts is not new to #1888; it has asserted this Two things now cross. element_id, which is an index the model chose into an The argument for carrying coordinates is that they are the model's own output, element_identity is added to the host-only set. The Computer Use tool's own For #1888 specifically: when the two are merged, computer-action-label.ts Non-blocking finding: the title and body are rewritten. 0fcfb46 removed the Rebased onto current main, cleanly. @maka/core and @maka/runtime suites pass, |
…ence it returns Types only. Nothing selects them, nothing behaves differently, and every existing test passes unchanged — this is the vocabulary a second executor backend needs before it can be written against the same interface. What it adds: - `launchApp` on `CuDispatchBackend`. Starting an application is something an executor can do and the host could not ask for, so a model that needed a closed app had no move but to tell the user to open it. - `truncated` on `CuObservation` and `actions` on `CuObservedElement`. A tree that was cut short and an element that offers more than a press are both facts the executor knows and the host had no field to carry, so they were read, validated and then dropped. - `COMPUTER_USE_SEMANTIC_ACTIONS`, so the set of element actions has one written-down list rather than being spelled again at each place that switches on it. Optional throughout: a backend that provides none of it satisfies the interface exactly as before.
… accepts The previous version of this branch said nothing behaved differently. Four things did. `COMPUTER_USE_SEMANTIC_ACTIONS` declared `launch_app`, `scroll_element`, `element_sequence` and `window_action`. None of those is in the tool's wire enum on main; they exist only on the branch that adds the executor. The constant feeds `APPROVAL_ACTIONS`, so `computerUseApprovalSummary` flipped `knownAction` to true for four names the SDK rejects before the tool ever runs, and `rememberForTurnAllowed` could evaluate true for two of them. A person would have read an approval for an action that did not happen. The constant is now scoped to what this schema accepts. Declaring an executor's vocabulary is worth doing on the branch that also ships the executor. `dispatch_refused` had no producer, and adding it to `COMPUTER_USE_ERROR_CODES` widened `isComputerUseErrorCode` — the filter that decides whether a raw string from the driver process is passed through or replaced with `capture_failed`. That is a live trust boundary, not a declaration. Removed until something produces it. The unproduced fields on the observation and outcome types are left as they are: they have no runtime effect, and declaring the shape the executor returns is what this branch is for. The drift guard the constant's rationale rests on did not exist. The comment promised a file that was never written, and the guard on main only walks from the schemas to the catalog, which is the direction that catches a missing name — not the direction this branch broke. `computer-use-schema-parity.test.ts` now compares the two in both directions, with a negative control that runs the real comparator over a fixed catalog so it keeps proving something on the day the real one is correct. `ComputerUseSettings` declared `enabled: false` and was registered nowhere: not in `AppSettings`, not in `UpdateAppSettingsInput`, not in the defaults, merge or normalize passes that every peer settings module goes through. In the shipped app Computer Use tools are assembled with no enable check at all, so the type told a reader the capability is off by default while it is always on. Dropped. Wiring a real switch is a change with a UI and a migration, and it does not belong under a types commit. The model-call projection could not be used. Only `COMPUTER_USE_SEMANTIC_ACTIONS` was exported from the package entry, so `computerUseModelCallArgs` — the headline fix — was unreachable from `@maka/runtime`, and the defective projection it exists to replace stayed in place. It is exported now and wired into `projectPiToolArgs`, which is what writes the persisted `tool_call` message and the `tool_start` event: the record the model reads back as its own call. It had been reading the host's approval summary, which renames `window_id` to `windowId`, adds two fields the model never sent, and drops every argument it does not need. And the projection withheld the wrong half of `text`. It keyed on argument name, but `text` is six arguments under one name: the key for `press_key`, `key` and `hold_key`, the element action name for `secondary_action`, a substring of the window for `select_text`, and typed content for `type`. Excluding all six is right for two of them. For the rest the model was reading back `press_key ... text: <text>` and could not see which key it had pressed — the exact failure the comment cited as motivation. It now keys on the action. An action the tool does not accept has no plain set, so it falls through to shapes rather than to whichever set was used last.
…ations nothing produces The projection withheld coordinates on the grounds that they are screen-derived. They are not: a coordinate is the model's own output, four digits it chose and sent. Reduced to `<point>`, a model that clicked [412, 88] and missed could not tell whether it had already tried that point — the repeated-call shape this projection exists to make visible, reintroduced by the projection itself. `coordinate`, `start_coordinate` and `region` now come back whole when they are integers, and still degrade to a shape when they are not. An action the schema does not carry was recorded as `unknown`, so a model whose call was rejected for the name it sent read its own history as a call it never made and could not connect the rejection to the cause. Collapsing an unrecognised name is `computerUseApprovalSummary`'s job, where `knownAction` decides what a person is asked to allow; this record is the model's own arguments, and the action is one of them. It is bounded and redacted like every other model-supplied string. The rest of the branch declared fourteen fields, one interface, one backend method and two union members that nothing in the repository sets and nothing reads. The parity guard added here covers the action-name catalog and nothing else, so those declarations had nothing holding them honest — which is the same condition the guard's own header describes: `window_action` sat in the strict union and not on the wire and was unreachable from the day it shipped, and nothing failed. This branch was putting `window_action` back in that state, along with `scroll_element`. They come with the executor that produces them, where a guard can check that they arrive. What is left is what has a consumer: `COMPUTER_USE_SEMANTIC_ACTIONS` with the two-way parity check that keeps it and the wire enum in step, and `computerUseModelCallArgs` wired into the Pi backend's tool_call record.
The projection was wired into PiAgentBackend, which the desktop app never
instantiates: boot.ts registers only ai-sdk and fake, and the pi backend
is reachable only from harbor-cell.ts behind MAKA_PI_*, driving an
external CLI, and it does not build a ToolRuntime at all. tool-runtime.ts
had a zero-byte diff, so on the shipping path nothing changed.
Measured on the real ToolRuntime before this commit, for a model that
sent {action:'press_key', app:'TextEdit', window_id:41,
observation_id:'obs-7', element_id:'e12', text:'cmd+s'}, the persisted
tool_start args were {action, approvalClass, rememberForTurnAllowed, app,
windowId:41, observationId} — a key the tool rejects, two host fields the
model never sent, no element, and a press_key with no key. model-history
replays event.content.args, so that is what the model read back of its
own call, and it went on calling it that way.
The line moves; the projection does not. The permission prompt still
reads permissionArgs and the approval scope key is still computed from
the raw call, so only what is written down changes.
Two things now cross the privacy boundary that did not, and
computer-use-privacy-boundary.test.ts says so where it asserts them.
element_id: an index the model chose into an observation it had already
read. Admitted only when it is a stable identifier, so an accessibility
label arriving under that key is dropped rather than persisted. Without
it every element action in a turn reads the same.
coordinate, start_coordinate, region: geometry the model wrote into the
call itself, integers only, so a mistyped value still degrades to a
shape. A coordinate is not read off the screen; withholding it left a
model that clicked a point and missed unable to see that it had already
tried that point.
What still does not cross: the value of text for type and select_text,
the value of set_value, and every other argument whose value is screen
content or something a person asked to have typed — those keep their key
and lose their value to <text>. text for press_key, key, hold_key and
secondary_action is a name from a closed set the executor publishes, so
it is carried; that is one argument name meaning six things, and only two
of the six come from outside the model.
element_identity joins approvalClass and rememberForTurnAllowed as
host-only. The Computer Use tool's own permissionArgs resolves the
model's element_id against the live observation and attaches it, and
permissionArgs is what this projection now runs on, so without that the
model would read back a key it cannot send whose value came off the
accessibility tree.
The new assertion is on the tool_start event, not on the projection
function. A test that calls computerUseModelCallArgs directly stays green
while the runtime writes the approval summary, which is how this survived
being fixed once already.03fe3d9 to
71df4c1CompareUh oh!
There was an error while loading. Please reload this page.
…e now persisted in This branch was written when `ToolRuntime` persisted a Computer Use call as `computerUseApprovalSummary(...)`, which spells the target `windowId` and `elementId`. apache#1891 changed that line to `computerUseModelCallArgs(...)`, which spells the same two fields `window_id` and `element_id`, because the model reads its own history back through those args and was copying a dialect the tool rejects. Reading the old names off the new projection is not a crash and not a type error. Every element action falls back to the generic "点击该元素" and every window-scoped observe loses its window — which is the exact defect this branch exists to remove, reintroduced by the rebase and invisible from either renderer's own suite, because both build their fixtures by calling a projection and neither could tell which one the runtime uses. - `computer-action-label.ts` and `pi-transcript-tools.ts` read `window_id` and `element_id`. The label module's key type now filters the projection's index signature back out, so a stale name is a build error rather than an `undefined`; before, `keyof` widened to `string` and accepted anything. - Both renderers' fixtures go through `computerUseModelCallArgs`, and the seam test says in one place which object is the dialect and which two files read it. - The row refuses to print an element id that is not identifier-shaped. The persisted record keeps whatever the model sent, deliberately — it has to read back the call it made — but free text under that key is either a label copied off the screen or a mistake, and neither belongs in a sentence a person reads. The redaction this branch carries is unchanged in effect and now lives where the value actually lands: `computerUseModelCallArgs` already runs `element_id` through `redactSecrets`, and the seam test pins it — an `element_id` of `sk-ant-api03-…` is persisted as `[redacted]`, matching `app`. The addition of `elementId` to `ComputerUseApprovalSummary` is dropped: after apache#1891 that projection is neither persisted nor rendered, and its only remaining caller reads `approvalClass`, so the field would have had no producer and no reader.
…e now persisted in This branch was written when `ToolRuntime` persisted a Computer Use call as `computerUseApprovalSummary(...)`, which spells the target `windowId` and `elementId`. apache#1891 changed that line to `computerUseModelCallArgs(...)`, which spells the same two fields `window_id` and `element_id`, because the model reads its own history back through those args and was copying a dialect the tool rejects. Reading the old names off the new projection is not a crash and not a type error. Every element action falls back to the generic "点击该元素" and every window-scoped observe loses its window — which is the exact defect this branch exists to remove, reintroduced by the rebase and invisible from either renderer's own suite, because both build their fixtures by calling a projection and neither could tell which one the runtime uses. - `computer-action-label.ts` and `pi-transcript-tools.ts` read `window_id` and `element_id`. The label module's key type now filters the projection's index signature back out, so a stale name is a build error rather than an `undefined`; before, `keyof` widened to `string` and accepted anything. - Both renderers' fixtures go through `computerUseModelCallArgs`, and the seam test says in one place which object is the dialect and which two files read it. - The row refuses to print an element id that is not identifier-shaped. The persisted record keeps whatever the model sent, deliberately — it has to read back the call it made — but free text under that key is either a label copied off the screen or a mistake, and neither belongs in a sentence a person reads. The redaction this branch carries is unchanged in effect and now lives where the value actually lands: `computerUseModelCallArgs` already runs `element_id` through `redactSecrets`, and the seam test pins it — an `element_id` of `sk-ant-api03-…` is persisted as `[redacted]`, matching `app`. The addition of `elementId` to `ComputerUseApprovalSummary` is dropped: after apache#1891 that projection is neither persisted nor rendered, and its only remaining caller reads `approvalClass`, so the field would have had no producer and no reader.
…1888) * feat(ui): give driving the computer its own row, and say what it did Computer Use's display name is a noun — "Maka Computer" — so a turn that observed a window, clicked a button and observed again rendered three identical rows. `computerActionLabel` derives a row label from the call's own arguments instead: 观察「计算器」窗口, 点击元素 e7. The label is derived, never declared. The model is not given an `intent` field to write; every word it produces is owned by the runtime, and a free text field would be one more place it can be wrong. What the renderer can read is `computerUseApprovalSummary(...)`, which `ToolRuntime.executeTool` substitutes for the wire call before anything is persisted. `element_id` had to be carried on that projection for a row to name its target at all, and this adds it — but a shape filter is not a privacy boundary. `stableIdentifier` admits `[A-Za-z0-9._:-]{1,256}`, which is also the shape of an API key, and arguments are not validated before the projection runs, so a model that put a token under `element_id` wrote it verbatim into the persisted `tool_call`, onto `tool_start` and into the row — while the same string under `observation_id` came out `[redacted]`. It now takes the same `redactSecrets` pass `observationId` and `app` do. Also puts `'computer'` on the wire as a tool activity kind and has the Session decoder read the one list rather than a second hand-written copy of it, which had already fallen behind and rejected the new kind outright. Note for clients behind the runtime-host Session protocol: the `tool_start` frame carries no `args`, so such a client gets the kind but cannot derive the label. The desktop receives full SessionEvents over IPC and is unaffected. * fix(ui): read the Computer Use row's arguments in the dialect they are now persisted in This branch was written when `ToolRuntime` persisted a Computer Use call as `computerUseApprovalSummary(...)`, which spells the target `windowId` and `elementId`. #1891 changed that line to `computerUseModelCallArgs(...)`, which spells the same two fields `window_id` and `element_id`, because the model reads its own history back through those args and was copying a dialect the tool rejects. Reading the old names off the new projection is not a crash and not a type error. Every element action falls back to the generic "点击该元素" and every window-scoped observe loses its window — which is the exact defect this branch exists to remove, reintroduced by the rebase and invisible from either renderer's own suite, because both build their fixtures by calling a projection and neither could tell which one the runtime uses. - `computer-action-label.ts` and `pi-transcript-tools.ts` read `window_id` and `element_id`. The label module's key type now filters the projection's index signature back out, so a stale name is a build error rather than an `undefined`; before, `keyof` widened to `string` and accepted anything. - Both renderers' fixtures go through `computerUseModelCallArgs`, and the seam test says in one place which object is the dialect and which two files read it. - The row refuses to print an element id that is not identifier-shaped. The persisted record keeps whatever the model sent, deliberately — it has to read back the call it made — but free text under that key is either a label copied off the screen or a mistake, and neither belongs in a sentence a person reads. The redaction this branch carries is unchanged in effect and now lives where the value actually lands: `computerUseModelCallArgs` already runs `element_id` through `redactSecrets`, and the seam test pins it — an `element_id` of `sk-ant-api03-…` is persisted as `[redacted]`, matching `app`. The addition of `elementId` to `ComputerUseApprovalSummary` is dropped: after #1891 that projection is neither persisted nor rendered, and its only remaining caller reads `approvalClass`, so the field would have had no producer and no reader.
The record a model reads back of its own Computer Use call was the host's
approval projection, not the call.
model-history.tsreplaysevent.content.args, and those args came fromcomputerUseApprovalSummary,which exists to decide and display a permission: it renames
window_idtowindowId, addsapprovalClassandrememberForTurnAllowed, and drops everyargument it does not need.
Measured on the real
ToolRuntime, a model that sentread back
— a key the tool rejects, two fields it never sent, no element, and a
press_keywith no key. A model imitates the shape it is shown, so it went oncalling it that way; a real desktop run failed six of eleven calls on shapes
copied from its own history.
This adds
computerUseModelCallArgs: the same privacy rule, in the names thetool accepts, and wires it at
tool-runtime.ts, which is the line everyshipping backend goes through. The permission prompt still reads
permissionArgsand the approval scope key is still computed from the rawcall, so only what is written down changes.
What now crosses the privacy boundary
computer-use-privacy-boundary.test.tsasserted this line before this PR, andit is updated in place rather than around, so the change is visible in the
diff. Two things cross that did not:
element_id— an index the model chose into an observation it had alreadyread (
e12). Admitted only when it is a stable identifier, so anaccessibility label arriving under that key is dropped rather than persisted.
Without it, ten element actions in a turn are indistinguishable.
coordinate,start_coordinate,region— geometry the model wrote intothe call itself. Integers only, so a mistyped value still degrades to a
shape. A coordinate is not read off the screen; withholding it leaves a model
that clicked a point and missed unable to see that it had already tried that
point.
What still does not cross: the value of
textfortypeandselect_text,the value of
set_value, and every other argument whose value is screencontent or something a person asked to have typed. Those keep their key and
lose their value to
<text>— the key has to stay, or the model reads its ownhistory as a call it never made.
textforpress_key,key,hold_keyandsecondary_actionis a name from a closed set the executor publishes, so it iscarried: one argument name meaning six things, and only two of the six come
from outside the model.
element_identityjoinsapprovalClassandrememberForTurnAllowedashost-only. The Computer Use tool's own
permissionArgsresolves the model'selement_idagainst the live observation and attaches it, andpermissionArgsis what this projection runs on.
Also here
COMPUTER_USE_SEMANTIC_ACTIONSreplaces a hand-written action list that hadalready drifted:
window_actionwas added to the strict union and not to thelist, so every window move, resize and minimise would have summarised as
unknown— in the approval a person reads, and in the record the model readsback.
computer-use-schema-parity.test.tsnow compares the two in bothdirections.
Relationship to #1888
#1888 reached the same conclusion from the renderer's side and carried
elementIdin the approval summary so its labels could tell two element rowsapart. That intent is subsumed here:
element_idis carried, in the tool's owncasing. When the two are merged, #1888's
computer-action-label.tsshould readwindow_idandelement_idrather thanwindowIdandelementId, and theelementIdcase it added tocomputer-use-privacy-boundary.test.tsis coveredby the projection assertion in this branch.
Not here any more
The executor capability declarations this PR was originally titled for were
removed in 0fcfb46: nothing produced them, so they were a type surface with
no writer. The title and this description are rewritten to match what the
branch actually does.
Verification
The new assertion is on the
tool_startevent, not on the projection function.Reverted to
computerUseApprovalSummaryattool-runtime.tsand rebuilt, itfails with exactly the diff quoted above; restored, it passes. A test that
calls
computerUseModelCallArgsdirectly stays green either way, which is howthis survived being fixed once already in
PiAgentBackend— a backend thedesktop app never instantiates.
@maka/coreand@maka/runtimesuites pass;npx biome checkclean on thetouched files;
node scripts/check-console.mjspasses. Rebased onto currentmain, cleanly.