[Graphite MQ] Draft PR GROUP:spec_90e92e (PRs 24) - #29
Closed
graphite-app[bot] wants to merge 1 commit into
Closed
Conversation
…OR-13656) (#24) Stacked on #23. This is the contested half of #21, split out so it can be judged on its own. ## What it does Fields marked `nullable: true` generate as `FlagKindJSON`, because JSON is the only encoding that expresses a real `null` distinctly from the string `"null"`. That is correct, and it is why `--instructions null` clears the field. The cost is that prose has to arrive JSON-quoted, and the workaround is quotes inside quotes — exactly what shell users and coding agents get wrong: ```bash vf agent update --instructions '"You are a support agent for Acme."' ``` When a JSON flag fails to parse **and its destination ultimately holds a Go string**, the value is re-encoded as a JSON string and retried. 22 of 82 JSON flags qualify. Nullability is untouched — the `null` branch runs earlier. ## On the review objection @effervescentia — you asked for changes on the grounds that this accepts a `string` for `Markup` fields, and so belongs at the API level rather than downstream in the generated CLI. **The principle is right. I could not reproduce the premise**, and rather than argue it I have pinned both halves with tests: **No field's type changes.** The retry only fires where the spec already declares `type: string`. The request body is byte-identical to what the JSON-quoted form already produced, and to master: ``` branch, raw text "instructions": "You are a support agent." branch, JSON-quoted "instructions": "You are a support agent." master, JSON-quoted "instructions": "You are a support agent." ``` This is how the CLI reads a flag, not what the API accepts. Verified across plain text, quoted JSON, `null`, objects, arrays, empty and numeric input. **Markup is unreachable.** `Markup` is a union struct, and every field holding one holds it as a struct or a slice of them. `stringLikeJSONTarget` admits neither — unit-tested against `Markup`, `[]Markup`, `*Markup`, `OptionalNullable[Markup]` and `OptionalNullable[[]Markup]`. `--url` on `mcp-server create` is `[]components.Markup` and still rejects raw text; there is a regression test for that specifically. Separately, no property in `.speakeasy/out.openapi.yaml` references `Markup` at all. **So there is nothing to fix upstream** — the spec is already right. What is awkward is the mapping from "nullable string" to a JSON-encoded flag, which is codegen rather than API design. If you would still rather not carry this in the CLI, I am happy to drop it; #23 has three-quarters of the value and no type-shaped surface at all. ## The discriminator `OptionalNullable[T]` is `map[bool]*T`, so its `reflect.Kind` is `Map` — indistinguishable from `map[string]any` by kind alone. Requiring a **bool key** admits `OptionalNullable[string]` and rejects every ordinary map. Unit-tested against all 13 shapes that occur, including the near-misses `map[string]string` and `OptionalNullable[[]string]`. ## The trade, stated plainly For those 22 flags, malformed JSON that used to error is now stored as literal text. Right for markdown authors — the overwhelmingly common case for these fields — and wrong for someone who meant JSON and typo'd. Structs, maps and slices are excluded, so a mistyped object still fails loudly everywhere it would be meaningless as text. ## Verification - 7 new tests, including the two that this change lives or dies on: wire payload unchanged, and Markup left strict. - One test from #23 is **updated rather than kept** — it asserted that `--instructions` rejects prose and explains how to quote it. That rejection is what this removes, so the test now pins the behaviour that replaced it. - Full suite shows the same pre-existing failures as master.
graphite-app
Bot
deleted the
gtmq_spec_90e92e_1788538510543-257cbfb4-d52d-4430-9840-5ad5692ad6bb
branch
September 4, 2026 16:15
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: