Check what the model actually sent before believing the schema #23
- MA
Fixes a crash hit while running the app: `TypeError: operations.some is not a function` from `[agent/edit_plan]`.
Off `main`, unrelated to the design stack (#20, #21).
## Root cause
`apps/server/src/agent/tools.ts:152` cast the model's tool arguments instead of validating them:
```ts let args = raw as { revision: number; operations: edit.Operation[] }; ```
`raw` is `unknown` and `as` is erased at runtime. The JSON schema declaring `operations` as `type: "array"` is only *advertised* to the model, never *enforced* on the way back in. The model sent a JSON-encoded string, which reached `edit.apply` and crashed.
The two guards it passed are the tell — `operations.length === 0` and `> 1` were counting **characters**, so even a single-operation batch got through a check meant to catch multi-operation ones. Confirmed by replaying both guards:
``` JSON string: THREW — ops.some is not a function one-op JSON string: THREW — ops.some is not a function array-like object: THREW — ops.some is not a function real array: passed both guards ```
**The second-order damage is worse than the crash.** `answer()` caught the `TypeError` and returned `Error: operations.some is not a function` to the model — naming nothing it could change, so it would retry the same malformed call.
## The fix
New `apps/server/src/agent/arguments.ts`: `editPlan(raw)` and `anchorPlan(raw)` take `unknown` and return validated values or throw a message the model can act on. Every rule in the JSON schema is mirrored in code beside a comment naming the schema line it enforces — the two disagreeing silently is the failure mode being fixed. The per-op conditionals the schema *can't* express (`index` required unless the op is `insert_root`/`replace_root`, and so on) are now in code rather than only in a description string.
Behaviour now:
``` REJECT operations as a JSON string -> `operations` must be an array of operation objects. REJECT single-op JSON string -> `operations` must be an array of operation objects. REJECT array-like object -> `operations` must be an array of operation objects. REJECT unknown op -> `operations[0].op` must be one of insert, insert_root, … REJECT move without a destination -> `operations[0].to` is required for "move". REJECT empty batch -> `operations` requires at least 1 operation. REJECT revision missing -> `edit_plan arguments` is missing field: revision. OK well formed -> isArray=true n=2 ```
**Nothing is coerced.** A JSON string could be `JSON.parse`d and the turn would succeed, but the model would keep sending strings and the next tool to trust its input would find out the hard way. This is the contract `ask` has always had via `@chopin/question`'s `normalize` — `edit_plan` was the tool that skipped it.
`anchor_plan` had the identical cast and is fixed alongside; it runs after every successful edit, so it's no less reachable.
`edit.apply` keeps its `Operation[]` signature — validation belongs where input arrives, and the type it always claimed is now true.
## Verification
| | | |---|---| | `bun run ci` | clean | | `bun run types` | all packages + e2e, exit 0 | | `bun test` | 551 pass, 0 fail (24 new) |
New `arguments.test.ts` covers the reported bug, the single-op character-length trap, array-like objects, missing/empty/over-limit arrays, unknown ops, missing per-op conditional fields, non-integer revisions, and bad digests — plus the happy paths asserting `Array.isArray` on the way out.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
MaggieAppletonmerged 92b497c392ed3 months ago on GitHub
MaggieAppletonhead-ref-deleted3 months ago on GitHub