fix: allow execute actions to accept array bodies#340
Conversation
Co-authored-by: Rohan Gupta <thisrohangupta@users.noreply.github.com>
Co-authored-by: Rohan Gupta <thisrohangupta@users.noreply.github.com>
Co-authored-by: Rohan Gupta <thisrohangupta@users.noreply.github.com>
Co-authored-by: Rohan Gupta <thisrohangupta@users.noreply.github.com>
|
|
Co-authored-by: Rohan Gupta <thisrohangupta@users.noreply.github.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Found 1 blocking architecture issue.
harness_executeshared-body contract drift. This PR fixes a real bug, but it does it by teaching the generic execute tool to accept raw top-level arrays. In this repo, other array-wire endpoints intentionally keep the public tool contract object-shaped and let the resourcebodyBuilderunwrap the array (freeze.toggle_status,fme_identity.create,fme_segment_keys.update). That keeps the shared tools consistent and preserves a fail-loud place for required-content validation. Wideningharness_execute.bodycreates a new generic transport exception instead of fixingeval_metric_set.replace_metricsin its resource contract.
Assumption: the existing object-wrapper pattern for array-backed endpoints is still the standard we want to preserve here.
Change summary: the new regression is correctly exercising the public harness_execute path, and the AI Evals request-path assertion looks good. The remaining issue is where the public-contract change lives.
Sent by Cursor Automation: Sunil On Demand Architecture Review
| inputs: z.union([z.string(), z.record(z.string(), z.unknown())]).describe("Pipeline runtime inputs: key-value pairs like {branch: 'main'} (auto-resolved), or full YAML string. Check runtime_input_template first via harness_get.").optional(), | ||
| input_set_ids: z.array(z.string()).describe("Input set IDs for complex pipelines. List available: harness_list(resource_type='input_set', filters={pipeline_id: '...'}).").optional(), | ||
| body: z.record(z.string(), z.unknown()).describe("Additional body payload for the action").optional(), | ||
| body: z.union([z.record(z.string(), z.unknown()), z.array(z.unknown())]).describe("Additional body payload for the action").optional(), |
There was a problem hiding this comment.
Allowing top-level arrays on the shared harness_execute.body schema fixes eval_metric_set.replace_metrics, but it also breaks the repo's established public contract for array-wire endpoints. Other resources that send arrays on the wire keep the shared tool input object-shaped and let the resource bodyBuilder extract the array (freeze.toggle_status, fme_identity.create, fme_segment_keys.update). That keeps generic-tool semantics consistent and gives us a place to fail loud on missing content. Please keep body object-shaped here and move the array unwrap into the eval_metric_set.replace_metrics action contract instead (for example body.metrics / body.items -> raw array).
2 participants
Description
Fixes a public MCP contract bug where
eval_metric_set.replace_metricsdocumented and required a raw JSON array body, butharness_executeonly accepted object bodies in its registered input schema. Strict clients rejected valid calls before dispatch, making metric-set replacement unreachable.The fix allows
harness_execute.bodyto be either an object payload or a raw array payload, and adds tool-layer regressions proving the schema accepts array bodies andeval_metric_set.replace_metricssends the array unchanged to the API.Type of Change
Checklist
Verification:
pnpm exec vitest run tests/tools/tool-handlers.test.ts,pnpm typecheck,pnpm build,pnpm docs:generate,pnpm docs:check,pnpm test,git diff --check HEAD~3..HEAD.