githubnext/chopinPublic

Let's compose rich plans together

Check what the model actually sent before believing the schema #23

MergedMaggieAppletonmerged 2 commits intomainfromrefs/pull/23/head
  1. 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)

  2. MaggieAppletonmerged 92b497c392ed3 months ago on GitHub

  3. MaggieAppletonclosed3 months ago on GitHub

  4. MaggieAppletonhead-ref-deleted3 months ago on GitHub