feat(cu): rewrite what the model reads — observation, action surface, refusals - #1952
Conversation
8b5170b to
bfaad8f
Compare
|
Both halves of the red test job are fixed, and the rest of the review is addressed. Taking them in order. The test job. @maka/runtime: this PR rewrote the exclusive-step refusal and updated one of its two consumers. tool-runtime-sqlite-boundary.test.ts:71 still pinned the old wording: actual: '{"error":"Tool agent_output did not run: agent_swarm cannot share an assistant step with other tool calls. Send agent_output again in a later step."}' @maka/computer-use: computer-use-cumulative-e2e.test.ts did JSON.parse on observation text at four sites, which this PR deliberately changed to an indented rendering: SyntaxError: Unexpected token 'o', "observatio"... is not valid JSON The in-package readers had moved to observation-text-reader.ts and this cross-package one had not. Rather than a fourth copy of the parser — which its own comment warns about — @maka/runtime gained a test-only entry point and that suite imports it. Both packages are green now: runtime's only remaining failure is the workspace-executor PTY test that fails on this machine under load and failed before any of my changes, and computer-use is 224 pass 0 fail. The model taught to send values the tool rejects. element_identity is added by permissionArgs, is not in computerWireParams, and computerWireParams is .strict() — so a model imitating its own last recorded call is refused by the SDK before impl and the refusal never reaches the debug journal. It is host-only now, alongside approvalClass. The free-text case was worse because it was legal. value and text are z.string().max(8000) with no lower bound and no pattern, so "" passed both schemas and a model replaying its own set_value typed those six characters into the user's field. The placeholder is now text:18 — a description of the value rather than a blank to fill, which is also what that projection's own test title already claimed — and the tool refuses one that comes back rather than typing it. Vocabulary with no producer. query was the one that cost something: the renderer does the filtering itself from observation.query and the executor was expected to echo it, which the one shipped executor did not, so the filter never ran and the header said nothing. That is host work, so the request is stamped onto the observation and the filter runs whatever the executor echoes. menu cannot be synthesized — opening one is a walk of the menu bar — so an observation with no menu bar in it answers menu_bar=unavailable instead of saying nothing. The element-level vocabulary is described as what the executor reports, because the absence of [focused] across a whole observation means the executor does not report focus, not that the window has nothing focused. duplicate_action. A retired action is its own reason now, still under the duplicate_action code, with a sentence that agrees with the refusal one call earlier instead of contradicting it. The two messages were telling the model to observe for a change that provably had not happened. target_mismatch is in COMPUTER_USE_ERROR_CODES and both returns carry it, so a refusal is no longer recorded as a successful invocation. A frame that moves during dispatch no longer erases the executor's account. The executor's leads, because it is the one that says what to do differently; the frame's follows, because the retry does have to be re-observed. readSchemaFields. Computer Use answers per action from the strict union, or undefined for an action it does not know. Reverted, the answer to a click_element was action, app, window_id, include_screenshot, menu, wait_for_text, wait_for_text_gone, query and the rest of all 22. Both vacuous tests. computer-use-wire-schema reads computerActionNames() now, and a second test requires every action in the union to have a sample call, since the per-call assertions only cover what CALLS names. The [focused] marker is pinned exactly, and a companion test pins disabled and selected the same way. Negative controls, each one run. Renaming the marker to NEGCTL_focused: ✖ an element says what it accepts beyond a click, and where the keys go Injecting an action into the strict union only: ✖ every action in the strict union is offered by the wire enum Reverting the element_identity strip and the text:18 placeholder: ✖ never shows the element identity the host resolved for itself Dropping the target_mismatch error field: ✖ target_mismatch carries an error code rather than passing as a success Dropping the withheld-value guard: ✖ a replayed placeholder from the call record is refused, not typed Neutralizing the query stamp and the menu note: ✖ a query filters and announces itself even when the executor ignores it Pointing retired_action back at the old duplicate_action sentence: ✖ a repeat of a refusal that never ran is not told to observe for a change Neutralizing the executor-refusal merge: ✖ a frame that moves during dispatch does not erase the executor s own refusal Reverting the per-action field lookup: ✖ Computer Use answers per action, not with every field of every action Reverting the retired_action reason: ✖ the same action is not offered twice after being retired Each was restored and re-run green. Two things I did not change and should say. The isConsumed pre-check in computer-use-tools cannot match a semantic action bound with an element id — it recomputes the binding without one — so it was already inert for element actions before this PR, and my retired/dispatched split there rides along with it. The live path is claimAction, which is fixed and covered. And this branch still adds dispatch_refused to COMPUTER_USE_ERROR_CODES; #1953 adds it too at a different position, and since #1953 should merge first, this one should drop its own on the rebase. |
f2404c9 to
3235e37
Compare
|
Third-round findings addressed in 4658441. Details, per finding, with the evidence I ran rather than the reasoning I did. On the shapeOf collision, I kept The privacy split survives and is now asserted rather than assumed. Geometry the model authored passes through — including On the erased discriminating arguments, all five were real, and there was a sixth: The four that still hold a placeholder are the privacy boundary, and they now stay wire-valid so the refusal is visible instead of dying above The replay guard reads every string argument and every string member of an array argument, rather than the three names it was written with. Measured against the real tool, all seven now refuse by name: On the contradictory refusals, reproduced and fixed. No "still current" sentence, and the next call the model makes is the one the reply told it to make. For a code outside One correction on the retire path: The rest:
The B6 headline test was green and hollow.
The description claimed every successful action yields a fresh full observation. Rebase leftovers: Every new assertion was negative-controlled by reverting the fix, rebuilding and observing red, then restoring and observing green. The reversions and what they produced: CI's eight failures are resolved. One thing outside these findings: main has moved nine commits since a3d4490 and the branch is CONFLICTING again, in |
4658441 to
21b2d86
Compare
…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.
… refusals The Computer Use model-facing surface in packages/runtime: what the tool description says the model can do, what an observation reads like, and what a refusal tells it to do next. Observation text stops being JSON. One element per line with indentation for containment, state written only when it is the exception, and the same content costs 3.5x less than JSON and 2.1x less than YAML. Structural wrappers that carry no name, value, action, focus or selection are lifted into their parent — a line goes, never an element, and ids are unchanged. The action surface gains the verbs a real task needed and did not have: the menu bar with a scope so it is affordable to carry, element-addressed scroll, window move/resize/minimize, a query filter on observe, wait_for_text, and an app argument that takes the name a person would use rather than only a bundle id. Refusals say what to do next. Arguments that failed validation come back with the field names the tool does accept, taken from the tool's own schema, instead of only a validator's complaint. An exclusive-step refusal says first that nothing ran. The sandbox hint names request_sandbox_boundary. The call the model reads back as its own is now the call it sent, not the host's approval projection — that projection taught it to send approvalClass and windowId, two fields the tool does not take and one key in a dialect it rejects.
… that contradict each other Review of the model-facing surface, from the two red test jobs down to what a refusal tells the model to do next. Two suites were left behind by this branch's own changes. The exclusive-step refusal was rewritten and only one of its two consumers followed; the other still pinned the old wording. The observation stopped being JSON and the three in-package readers moved to a test-only parser, but the cross-package one in @maka/computer-use kept its JSON.parse and threw on every observation. That parser now has an entry point rather than a fourth copy. The call the model reads back as its own carried element_identity, which the host injects and the wire schema — .strict() — rejects. A model imitating its last call is refused by the SDK before impl, so the refusal never reaches the debug journal this file's header exists to warn about. Withheld values were written as `<text>`, a legal string at both schemas: a model replaying its own set_value typed those six characters into the user's field. They are now `<text:18>`, a description rather than a blank to fill, and the tool refuses one that comes back instead of typing it. target_mismatch was written into refusal text without being in COMPUTER_USE_ERROR_CODES, and both returns carried no error at all — a refusal recorded as a successful invocation, under a word in no table the model has. duplicate_action told the model to observe and see whether the action took effect. For an action retired without being dispatched nothing had happened, and the refusal one call earlier had said so: the two consecutive messages gave opposite instructions. A retired repeat is now its own reason with its own sentence, still under the duplicate_action code. A frame that moved during dispatch replaced the executor's own dispatch_refused with a generic stale_frame, so the model re-observed, re-picked the same element and learned nothing. Both are said now, executor first. The camelCase-argument refusal read the flat wire object and answered with all 22 keys of every action, so a click_element was told maka_computer takes menu, duration and region. It answers per action, from the strict union, or says nothing. query and menu were stated as facts of the observation format and neither was produced by the only executor there is. The query filter is the renderer's own work, so the request is stamped on the observation and the filter runs whatever the executor echoes; a menu cannot be synthesized, so an observation with no menu bar says menu_bar=unavailable rather than nothing. The element-level vocabulary is described as what the executor reports, because absence of [focused] across a whole observation means the executor does not report focus, not that the window has nothing focused. Two tests proved nothing. computer-use-wire-schema built its expectation from its own CALLS array, so injecting an action into the strict union alone — the window_action bug class it was written for — left it at 24 pass, 0 fail; it reads computerActionNames() now, and every action in the union has a sample call. The [focused] marker matched /focused/, so renaming it to [NEGCTL_focused] left 19 pass, 0 fail.
The record a model reads as its own previous call is the shape it imitates, and five of the actions this branch adds came back with the one argument that discriminates them replaced by a description of it. window_action and scroll_element read back their verb as "<text:4>" against a z.enum, and position and size as "<point>" against a tuple, so a model replaying its own window move was rejected by the .strict() wire schema above impl — invisible to the debug journal, which is the failure computer-use-wire-schema.test.ts exists to prevent. element_sequence read back steps as "<2 items>", a string where an array belongs, and died the same way. The accepted ones are worse. observe's query and menu and wait's wait_for_text are plain strings, so the replay went through: a model that filtered a 1,200-element window with query:"下载" and wanted that view again replayed query:"<text:2>", matched nothing, and read "showing 0 of 1200" as proof the control does not exist. None of these come off the screen. query, menu and wait_for_text are predicates the model composed; the rest are picks from sets the schema publishes. They come back whole. What a person asked to have typed and a verbatim quote of what a window is showing still do not, and the replay guard now reads every argument rather than the three that could carry a shape when it was written — including bare <text>, which the previous release wrote and old history still holds. shapeOf keeps <text:N>. A bare <text> is a fill-in-the-blank, and value and text are z.string().max(8000) with no lower bound or pattern, so it was a legal call at both schemas; a length is a description of the value rather than a substitute for one. Tests that landed against <text> are updated to match. Two consecutive refusals contradicted each other. A refusal that dispatched nothing was told "observation X is still current, use it rather than observing again" while the same reply attached a fresh observation that superseded X — and the call the model was told to make came back stale_frame, telling it to observe. The sentence is now written against what was actually handed back. Also: CuObservation.appAlias had no producer, so the escape that keeps "Dictionary" from contradicting an observation of 词典 could never fire; observe now sets it and a re-observation of the same app carries it. The codec's post-dispatch headline said computer.<action>, a tool the model cannot call, and the test claiming otherwise never sampled it. user_intervened told the model to observe, which is refused identically in intervention_debounce; blocked_url told it to work on another app, which is refused too. The description claimed every successful action yields a fresh observation, which minimize does not. CuOverlayHookContext carried targetStacking and targetWindowId that no hook reads, and ComputerUseSettings was exported by nothing and imported by nobody; both are gone rather than documented as a gate.
The trajectory analyser's vocabulary guard proves two things: every action the tool accepts classifies, and an action nobody has heard of is refused rather than counted. The second half used `launch_app` as its unknown name, which was true on main and stops being true here — this branch puts `launch_app`, `window_action` and `element_sequence` on the wire, so the guard failed on a correct change. Any name that reads like a plausible next action expires the day that action ships. The unknown is now one of the regex ghosts the same comment already names as having zero occurrences on the wire, with an assertion that it is still absent from `CU_TOOL_ACTION_TYPES` — so if it is ever adopted, this fails by saying the case no longer tests anything rather than by looking fine.
Two tests that main added while this branch was open read an observe result by parsing its `modelText` as JSON. That is what the tool used to answer with, and the point of this branch is that it no longer does — it answers in the rendered observation format, so both threw a SyntaxError on the first line of their setup and never reached the thing they were written to check. Neither test is wrong about what it wants. The cursor tests want the point an element action is aimed at, and the runtime test wants the window a presentation is bound to; both only needed the observation id. They now read it with the parser the runtime's own tests already use, rather than a second copy of the grammar living in a test file.
21b2d86 to
287dc1a
Compare
|
Rebased onto main at ceb23ec. This was a standalone pass rather than something folded into the last round of fixes, because the conflicting files are the three this branch edits most and resolving them quietly is how two merge-invisible regressions got in earlier. Main moved three times while this ran, so the branch was rebased onto 1705111, then 61a0ca1, then ceb23ec. Only the last one matters; the earlier two are recorded because the third brought #1883 and #1888 into the same files and changed two of the answers. What was resolved, and why. packages/core/src/computer-use.ts. Main's #1884 replaced the hand-written action vocabulary with a derived one: CU_SEMANTIC_ACTION_TYPES feeds COMPUTER_USE_SEMANTIC_ACTIONS feeds CU_TOOL_ACTION_TYPES, and APPROVAL_ACTIONS is derived from the last. This branch had added launch_app, scroll_element, window_action and element_sequence to the old hand-written list. The derivation is kept and the four names were fed into it rather than restated beside it: scroll_element, window_action and element_sequence go into CU_SEMANTIC_ACTION_TYPES because all three dispatch through runSemantic and name an element, and launch_app becomes a third opener alongside list_apps and observe because it names an application and has no observation to be bound to. The resulting wire order is identical to the one this branch had written by hand. packages/core/src/index.ts. Union of both export lists. Main's #1986 and #2008 exports and #1884's six new symbols are all present alongside COMPUTER_USE_WITHHELD_VALUE. packages/runtime/src/computer-use-tools.ts. The wire enum takes main's z.enum(CU_TOOL_ACTION_TYPES) rather than this branch's literal list, which now yields exactly the same set. The lease classification needed more than a straight take. Main replaced two hand-written condition chains with isCuObservingAction and isCuMutatingAction, and that partition is two-way while this branch's behaviour is three-way: launch_app takes an observation lease, not an action lease, because beforeAction only grants while the session is active and a session is active only after a fresh observation, which cannot exist for an app that is not running yet; element_sequence takes neither at the top level because it takes an action lease per step and a fresh observation lease between steps. Main's helpers are kept as the base with those two named as explicit exceptions and the reason written down, rather than reverting to the chains or letting the partition silently change what a launch does. Two upstream changes landed in the same code while this was open, and both changed an answer. #1883 added targetWindowId to CuOverlayHookContext with a real consumer. This branch deleted targetWindowId as unused, which was true when it was written and is not true now, so main's field and its producer are kept and only targetStacking is removed. #1883 also solved the same element-frame-to-cursor-point problem this branch had solved, more recently and with a dedicated helper, so main's version is kept there too. Worth recording: resolving that file with checkout --ours took main's whole file and silently dropped retireAction, which had merged cleanly and is what stops a refused action from invalidating the frame. It was caught, the conflict was recreated, and the three hunks were resolved individually. That is the exact failure this pass exists to avoid, and it took thirty seconds to make. #1888 and #1883 added tests that read an observe result by parsing modelText as JSON. This branch is the change that stops observe answering in JSON, so those tests threw a SyntaxError in their setup and never reached what they were checking. They now read the observation id with the parser the runtime's own tests use. Separately, main's trajectory analyser test used launch_app as its example of an action nobody has heard of, which this branch makes real; the example is now one of the regex ghosts the same comment already names, with an assertion that it is still off the wire so it fails loudly rather than quietly if it is ever adopted. Verification. Every action was replayed through the real projection and parsed back through both schemas: computerUseModelCallArgs to build the persisted args, then computerWireParams (what the SDK validates) and computerParams (the strict union). 37 cases covering all 28 actions in the union, no action unsampled. All 37 accepted by both schemas. Exactly four hold a privacy placeholder and all four are still wire-valid, so the replay guard's refusal reaches the journal instead of dying above impl: set_value.value, select_text.text, type.text, and element_sequence's steps[].label and steps[].value. Tests, against a baseline measured on clean origin/main at ceb23ec in the same environment. core 756 to 768, computer-use 133 to 133, runtime 2882 to 3104. Zero failures on both sides of all three. npx biome check clean on the 37 touched files and across the three packages, node scripts/check-console.mjs passes, npx knip --workspace apps/desktop exits 0, npm run test:scripts 65 of 65, and a full npm run build:test including apps/desktop compiles clean. Nothing upstream was dropped. No file is deleted relative to main. Diffing every line main added since the original merge base against every line this branch removes leaves only syntax noise and the handful of lines described above, each of which is a deliberate edit rather than a lost one. Spot checks: computerUseModelCallArgs still lives in tool-runtime.ts, DEFAULT_CU_BACKEND_ID is still 'maka-cu', there are no cua-driver imports anywhere, and computer-use-schema-parity.test.ts is intact. One thing worth knowing beyond this PR. The packages/runtime @ai-sdk/provider-utils failures that have been treated as environment noise are not noise, and #1995 does fix them, but not in a way a source checkout alone will show. #1995 landed patches/@ai-sdk+provider-utils+5.0.11.patch, applied by the root postinstall through patch-package. A node_modules tree installed before that commit stays unpatched, and the applier's own header says so: an unpatched tree turns model-factory-tool-call-index.test.ts red. On an unpatched tree here that was 36 failures on clean origin/main, all of them StreamingToolCallTracker. Applying the patch alone was not enough, because the consuming @ai-sdk packages resolve provider-utils through their own real path; making them real directories so resolution finds the patched copy took runtime to zero failures on clean main. So "clean" for the remaining PRs in this batch should mean zero runtime failures, and any tree still showing them needs a reinstall rather than an exemption. Two things left alone deliberately. scripts/cu-process-restart-e2e.mjs still parses an observe result as JSON and will need the same treatment as the two tests above; it is a real-machine soak script outside CI and it was already in that state before this rebase, so it is noted rather than changed here. And one flake was observed twice under load average 23 and passed on a single rerun each time: workspace-executor's "reports abort with captured output" and maka-cu-service's tmpdir teardown. Both are timing-sensitive and unrelated to this change. |
…holder The row test pinned the literal '<text>'. This branch gives the placeholder a length, so it became '<text:26>' and the assertion reddened for a change that was correct — the length is what stops a model reading the placeholder as a fill-in-the-blank template and typing those characters into the user's field. Matched against COMPUTER_USE_WITHHELD_VALUE, the pattern the tool itself refuses on, so the claim is that the value is withheld rather than how it is spelled. The secret is still asserted absent.
c2f5cc8 to
471f19b
Compare
1 participant
What this adds
maka-cuis a Maka-owned native macOS Computer Use executor. It speaks themaka.cu/2host protocol over stdio JSON-RPC, and this PR adds the host side ofthat conversation: a
CuDispatchBackendimplementation, the protocol readers,the child-process supervisor, and the script that builds the executor from source
and pins its digest.
Why it exists
The executor Maka ships against today re-resolves what the model pointed at. The
host hands down an element index, the executor walks the tree it can see now, and
if the window changed in between, the action lands somewhere else — or lands
nowhere and reports that it landed.
maka.cu/2moves frame binding into the executor. A dispatch quotes threethings: the snapshot id, the element token, and the digest the host was given for
that token. The executor compares them against the tree it minted and answers in
a closed vocabulary —
snapshot_spent,element_changed,element_released,process_replaced— instead of guessing. That is why this backend has nore-match pass, no occlusion geometry, and no path inference: it carries identity
down and maps declared answers back.
Two consequences follow from owning the executor rather than consuming one.
Waiting for a launched app's first window happens inside the executor, which is
where the information is, so
launch_appstops returning an empty window arraythat costs the model a whole extra observe cycle. And a refusal arrives as an
enum with a path and an effect, so the host can tell a model that its action was
attempted and declined apart from one that could never have reached the target.
What this PR does not do
It changes no default.
selectComputerUseBackendreturns cua-driver for everycaller that does not pass
backendId: 'maka-cu', and no caller in thisrepository passes it. It deletes nothing belonging to cua-driver: the backend,
its service, its page-target resolver, its snapshot readers, its provenance
script and its bundled-tools entry are all untouched.
The follow-up PRs in the chain are, in order: switch the desktop host to select
maka-cu, then remove cua-driver.
Contract surface
The executor answers more about a tree than Maka's shared Computer Use contract
has fields for — per-element placeholder text, subrole and advertised actions,
whether the walk was truncated, the menu scope, what is stacked over the target
window — and it can carry out window and scroll actions that the tool schema
cannot yet express. Rather than widen
CuObservation,CuObservedElementandCuSemanticActionwith members no tool can produce and no renderer reads, thosewidenings are local to
@maka/computer-use. Structurally aMakaCuObservationis a
CuObservation, so nothing downstream changes; the PR that gives these amodel-facing home is the one that moves the declarations up.
One shared change was unavoidable:
dispatch_refusedjoinsCOMPUTER_USE_ERROR_CODES. It cannot be local because the union is closed, andit cannot be folded into a neighbour without losing the distinction the model
acts on.
capture_failednames the wrong subsystem, andunsupported_actioniswhere "the element does not offer this" already lands. "It offered it, we tried,
the OS said no" is a different next move.
apps/desktop/src/main/capability-snapshot.tschanged because the backend id itreports is no longer a single-member union. The capability now reads "available"
for any selected executor instead of for one named executor, which is what it
meant all along.
Testing
Covered by tests that run in CI:
packages/computer-use/src/__tests__/maka-cu-protocol.test.ts— every readerrefuses a malformed or incomplete envelope rather than defaulting a missing
field. A declared field that is absent is version skew and has to be caught at
the boundary.
packages/computer-use/src/__tests__/maka-cu-backend.test.ts— 60-odd casesagainst a scripted executor: frame binding and its four release reasons,
session lifecycle and restart, the refusal vocabulary and the sentence each
refusal gives the model, key chord parsing including the
Object.prototypespellings that used to parse into a chord with a non-string key, image
handling, abort propagation, and the truncation and occlusion fields.
scripts/computer-use-provenance.test.mjs— a rot guard ondocs/computer-use-provenance.md: every repository path the record names mustexist, its three sections must stay separate, and the manifest must agree with
what the record claims about each executor.
maka-cuselects it, that omitting the digestyields
'none'rather than a backend willing to spawn whatever is at the path,and that a caller who names nothing still gets cua-driver.
Not covered here, needs a real machine:
above talk to a scripted process over the same pipe, which proves the host
reads the protocol correctly and proves nothing about AX behaviour.
process.platformcheckand do not execute on the Linux CI runner.
macOS identifies it by its code directory hash and Accessibility has to be
granted again after every rebuild.
MAKA_CU_SIGN_IDENTITYmakes the grantsurvive; there is no notarized artifact yet.
distributionReadyis false andscripts/verify-macos-arm64-dmg.mjsnow forbids the binary's path in apackaged build, so this executor runs in a development build only.