fix: parse YAML strings for normalized create bodies#319
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>
There was a problem hiding this comment.
I found one important contract issue and one smaller coverage gap against the architecture-review standards.
- Important:
src/utils/body-normalizer.tsnow parses raw YAML for everybuildBodyNormalized()consumer, but null-like YAML still bypasses local validation and gets dropped as an absent body instead of failing loudly. - Minor:
tests/integration/mock-harness-api.test.tscovers the wrappedconnector.createpath, but this PR changes a shared helper used by wrapped, unwrapped, and injected-field resources. A focused regression for the null-like YAML case, and ideally one unwrapped-resource path, would better match the contract-test standard.
Assumption:
- I could not rerun
pnpm build/pnpm typecheck/pnpm testin this workspace becausenode_modulesis not installed here, so this review is based on static inspection of the diff and surrounding call sites.
The overall direction looks right: the fix stays out of the separate pipeline raw-YAML path, and the connector JSON-body regression is valuable.
Sent by Cursor Automation: Sunil On Demand Architecture Review
| body = parseYamlBody(body); | ||
| } | ||
|
|
||
| if (body !== undefined && body !== null && (typeof body !== "object" || Array.isArray(body))) { |
There was a problem hiding this comment.
Null-like YAML still slips through here. yaml.parse(""), yaml.parse("null"), and yaml.parse("~") return null, which bypasses this guard because it only rejects non-object values when body !== null. The shared builder then returns null, and HarnessClient treats that as an absent body, so connector/service/environment/service_override writes skip the local contract check instead of failing loudly. This should reject null the same way it rejects arrays/scalars, and I’d add a regression for the empty-string / null case.
| }); | ||
| }); | ||
|
|
||
| it("connector create rejects raw YAML string bodies that do not parse to objects", async () => { |
There was a problem hiding this comment.
Worth extending the coverage a bit further here. This PR changes the shared buildBodyNormalized() helper, but the new tests only exercise the wrapped connector.create path plus array-YAML rejection. A small regression for the null-like YAML case ("", "null", "~") and ideally one unwrapped consumer would better match the repo’s “focused coverage for changed body builders” standard.


Description
Parse raw YAML string bodies in the shared normalized JSON body builder before dispatching connector/service/environment-style create/update requests. This fixes connector create HTTP 415s when agents pass raw YAML to
harness_createinstead of a JSON object, while leaving pipeline raw YAML behavior on its separate body path unchanged.Added integration coverage for connector raw YAML conversion to
application/jsonand local rejection of non-object YAML bodies.Type of Change
Checklist