[Graphite MQ] Draft PR GROUP:spec_8aea0e (PRs 40) - #48
Closed
graphite-app[bot] wants to merge 1 commit into
Closed
graphite-app[bot] wants to merge 1 commit into
graphite-app[bot] wants to merge 1 commit into
Conversation
## Summary Some commands expand a union variant into one flag per field (`--body-param.<variant>.<field>`). The generated metadata declares some of those flags as plain strings although their fields are objects or nullable strings. `setFieldByPath` cannot store text in those fields, so each failed with `cannot convert string to …` whatever value it was given; only `--body`, or the variant's whole-JSON flag, could set them. A survey of all 38 expanded variant fields found 11: - `test turn create --body-param.agent.payload` (an object) - `integration connect --body-param.<provider>.credentials`, for twilio, ujet, genesys, kustomer, dixa, sunshine and custom-handoff (objects) - `evaluation create --body-param.<type>.description`, for boolean, number and string (nullable strings, so plain text failed too) `buildStringField` now hands such a flag to `buildJSONField`, the builder that JSON flags use (`internal/flagutil/stringflag.go`). An object takes JSON, and a nullable string takes plain text or `null`. Flags whose field holds text are unchanged. ## Before and after | | master | this PR | |---|---|---| | The 11 flags, run on the built binary | 11 fail with `cannot convert string to …` | 11 send their value | | `--body-param.agent.payload sequential` (text for an object) | `cannot convert …` | `invalid value … expected a JSON value`, with an example | ## Test plan - [x] `gofmt`, `go vet ./...` and `go test ./...` pass; `go.mod` is unchanged. - [x] `internal/flagutil/stringflag_test.go` covers the type rule and the builder: - an object; - a nullable string, as text and as null; - text that stays text; - text rejected for an object. Without the change, 3 of them fail. - [x] `test/union-variant-flags.test.ts` covers the turn payload, Twilio credentials, an evaluation description (text and null) and a bad value. All 4 fail on master. - Most cases read the `--dry-run` body. The credentials case sends its request to a local server instead, because #42 hides credentials in the preview. - The exit code is not asserted, since #39 fixes dry runs of operations that return 201. - [x] Full suite with #37 applied: 82/82. With #37, #38 and the other three fixes from this batch: 101/101. - [ ] CI: `CLI behaviour` shows master's 4 existing failures until #37 merges.
graphite-app
Bot
deleted the
gtmq_spec_8aea0e_1790966356923-2fbb1e9f-ef61-4587-b45c-61a2208367a3
branch
October 2, 2026 18:40
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This draft PR was created by the Graphite merge queue.
Trunk will be fast forwarded to the HEAD of this PR when CI passes, and the original PRs will be closed.
The following PRs are included in this draft PR: