test(cu): hold the two argument schemas against each other - #1870
Conversation
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
P2. The action-name direction is not held. Test 2 is named "the union covers every action name", but its body only asserts arms.length > 0 and action !== 'undefined'. Add a new arm to computerParams (every field already exists on the wire) or add a name to CU_ACTION_TYPES without a matching arm: all three tests stay green. This is exactly the window_action failure class the file's own doc comment cites. Compare wire.shape.action.options against the arm literals in both subset directions.
P2. A third, handwritten action catalog is outside the parity test. computerUseApprovalSummary (packages/core/src/computer-use.ts:254-280, consumed at packages/runtime/src/tool-runtime.ts:775-777 and packages/runtime/src/pi-agent-backend.ts:456-458) downgrades any unknown action to unknown in persisted audit records and disables turn-level remember. An action added to both schemas passes this test while silently degrading those consumers. Either walk computerParams and assert computerUseApprovalSummary({ action }).action === action, or derive wire, union, and approval sets from one shared constant.
P3. Comparison is name-only; a field type drift (e.g. duration becomes z.string() in the union while the wire keeps z.number()) is invisible.
P3. The negative control rebuilds Set/pretend by hand and never exercises the zod introspection (unionArms, .shape, literal .value) that the real test depends on. A zod upgrade that makes extraction return empty would pass both. Have the comparator take real zod schemas and feed it a deliberately broken schema.
2b258da to
49751cb
Compare
`maka_computer` takes arguments through two hand-written schemas. `computerWireParams` is the flat object the SDK validates a model's call against — one shape covering every action, most fields optional. `computerParams` is the strict discriminated union the tool narrows to before it does anything. A field has to exist in both: the first to be accepted off the wire, the second to survive narrowing. Nothing checks that they agree, and the failure mode is silent in an unusual way. A field present in the union and absent from the wire makes its action unreachable, and no test notices: the SDK rejects those calls above the layer the debug journal records, the unit tests exercise the union directly, and a real-machine probe goes around the tool entirely. This walks the union rather than sampling it, because the next gap will be an action nobody thought to sample. It passes on main today — this is a guard, not a bug report. The third case proves the comparison can fail, so that a green result means something. `computerWireParams` becomes exported for this. It is the only thing that can tell the two schemas apart.
…alog too Review found the first version did not hold the thing its own name claimed. Test 2 was called "the union covers every action name" and asserted only that there were arms and that each discriminated on something. Adding an arm, or a wire action with no arm, left all three tests green — which is exactly the `window_action` failure class the file's doc comment cites. It now compares the wire enum against the arm literals in both directions, because each direction is a different failure. An arm the wire cannot name is unreachable. A wire name with no arm is accepted off the wire and then falls through narrowing, which reaches the model as a validation error naming nothing it did wrong. Removing one action from the wire enum now fails. A third handwritten catalog was outside the check entirely. `computerUseApprovalSummary` downgrades an action it does not know to `unknown`, and that value is what lands in the persisted audit record and what turn-level remember is keyed on. An action added to both schemas passed every check here while silently degrading those consumers. The negative control also built its own arrays and never touched the zod introspection the real checks depend on, so a zod upgrade that made extraction return nothing would have passed the control while the real checks compared two empty sets. The control now runs the same comparator, and a separate test asserts the readers return something from the actual schemas.
49751cb to
2836c17
Compare
2 participants
Two schemas, written by hand, with nothing comparing them
computerWireParamscomputerParamsA field has to exist in both. A field present in the union and absent from the wire makes its action unreachable.
Why that failure is silent
Three layers of coverage, none of which can see the gap.
What this adds
A test that walks
computerParams.optionsand asserts every arm's field names are a subset ofcomputerWireParams.shape— walking rather than sampling, because the next gap will be an action nobody thought to sample.It passes on main today. This is a guard, not a bug report.
The third case is a negative control: it runs the same comparison against a deliberately broken pair and asserts it reports the gap, so a green result means the check ran rather than that it had nothing to say.
computerWireParamsbecomes exported for this — it is the only thing that can tell the two schemas apart.Verification
packages/runtimeclean rebuild against current main; all 89computer-use-*tests pass.