fix: make flag values, errors, and help legible to humans and agents (COR-13656)

[Bradenream]

Why

An unattended coding agent, given only an access token and the README, built a working support agent and held a real conversation — but it took ~45 minutes. About 3 of those were the quickstart. The other ~42 were spent fighting the CLI itself.

This fixes four defects from that session. All of them live at the same moment: when a flag value is typed.

What changes

1. Flags that hold prose now accept prose

vf agent update --instructions 'You are a support agent for Acme.'
  invalid value for --instructions: error unmarshalling json response body:
  invalid character 'Y' looking for beginning of value

The field holds markdown. It is FlagKindJSON because the spec marks it nullable, and JSON is the only encoding that distinguishes a real null from the string "null" — that is correct and is preserved. But nothing told the caller to wrap prose in JSON quotes, and the error named the first letter of their sentence.

An unparseable value is now retried JSON-quoted, but only when the destination unwraps to a Go string — 22 of 82 JSON flags. Structs, maps and slices still fail loudly, because prose is meaningless there.

The discriminator has to unwrap OptionalNullable[T], which is map[bool]*T and so 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, wrong for someone who meant JSON and typo'd.

2. Rejection messages are actionable

Flag errors never reached output.Error — they returned raw from the generated RunE and printed via a bare Fprintln. A mistyped flag got one unstructured line while an API 401 got a structured envelope. Backwards, since mistyped flags are the more common failure.

flagutil cannot import output (output already imports flagutil), so failures carry a typed FlagValueError up to Execute, which renders it.

invalid value for --llm: the value is valid JSON but not the shape this flag expects
  you passed: [1,2]
  expected shape: --llm '{"key":"value"}'
  decoder reported: cannot unmarshal array into Go value of type map[string]json.RawMessage
  pass null to clear the field: --llm null

3. Help shows the flag's type, not a word from its prose

pflag reads the first back-quoted word in a usage string as the value placeholder. Our descriptions come from the OpenAPI spec, where backticks are emphasis:

	before	after
agent update	--instructions Name	--instructions string
transcript search	--version-param environmentAlias	--version-param string

12 flags affected. The worst is --version-param, whose only legal values are draft and published, labelled with an unrelated parameter name. Fixed at the single point descriptions enter cobra, so a 13th flag is covered automatically. --usage output is byte-identical — it was already correct.

4. Agent-mode errors print exactly once

Every printer in internal/output writes to stderr and returns the error, and main.go printed whatever Execute returned. So one failure reached stderr twice. AgentModeError's own doc comment promised the opposite — but main.go is generated and printed unconditionally, so the contract was never kept.

stderr is now a single parseable JSON document for all three agent-mode error shapes. Pretty output is deliberately untouched: there the trailing line is the readable summary after a raw API body, so suppressing it would remove the most useful part rather than a duplicate. Verified byte-identical to master.

Design notes

Footprint on generated files is 12 insertions and 7 deletions across five files. All real logic lives in six new non-generated files. That was the binding constraint — regenerations land regularly (v0.234.0 was one), so every touched line of generated code is carried forever.

An OpenAPI overlay was assessed for items 1 and 3, and rejected. It would pin 12 descriptions verbatim so upstream rewording is silently overwritten and a 13th flag is missed; and fixing item 1 that way means deleting nullable, trading away the ability to clear a field. Persisted-edit durability was confirmed first — all four pre-existing edits survived the v0.234.0 regeneration intact.

Adversarial review

A 53-agent hostile review ran against the built binaries, with every finding independently verified by two skeptics instructed to refute it. 23 findings, 12 refuted, 11 survived, 3 blocking. All three blocking findings were real defects in my own code, and all three are fixed in a70fe3a:

1. The suggested example was one the CLI rejects. An object example was hardcoded for every structured flag, so on array-valued flags the hint told the caller to pass a value the same binary refuses — following it exactly reproduced the identical error. A loop, for the audience this PR exists for. The example is now derived from the destination kind, and an unrecognized shape yields no example at all.

2. Valid JSON of the wrong shape was reported as invalid JSON. --llm '[1,2]' parses fine. The decoder's real explanation was stored in Cause and never rendered on any path.

3. Agent mode bypassed the truncation written for exactly this. Measured: master 118 B, human 311 B, agent 5,329 B for a 5,000-char value and 400,331 B for a 400 KB one. It also echoed a credential from a malformed blob that master never echoed.

Plus two non-blocking: byte-slicing split multi-byte runes (200 Japanese characters reported as 600), and a stale comment.

Residual, deliberately kept: up to 120 characters of the value are still echoed where master echoed none. The echo is what reveals shell-quoting mistakes, and the value is the caller's own argv — already in process args and shell history. Bounded and documented.

Verification

• 39 new tests. The flag tests fail against master; the review-fix tests fail against the pre-review branch. Four pass on both — the invariants that must not move: --instructions null still sends a real null, quoted JSON is unchanged, object flags still reject.
• One test takes the CLI's own suggested example and feeds it back in — the only assertion that actually proves the retry loop is closed.
• Full suite: 66 passing, with the same 11 pre-existing failures as master (live integration tests with no credentials).
• go build, go vet, gofmt clean. Non-flag human output byte-identical to master.

Found but not fixed

--agent-mode / --agent-mode=false is inert: InitAgentMode latches at root.go:222 before flags parse, so the post-parse call returns early. Only env vars work. Verified present on master — not a regression from this work, and worth its own ticket.

[linear-code]

COR-13656

[Copilot]

🟡 Changes recommended

The raw-text retry failure path reports the wrong underlying error (hiding the actual retry failure), and a new test helper does not fully isolate agent-detection env vars, risking flaky mode-dependent assertions.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Improves CLI ergonomics at the “flag value parsing” boundary by (1) accepting raw prose for certain JSON-typed string destinations, (2) surfacing actionable, structured flag parsing errors (especially in agent mode), (3) preventing backticks in OpenAPI-derived descriptions from breaking help placeholders, and (4) ensuring agent-mode errors are emitted exactly once.

Changes:

• Add raw-text fallback for JSON flags whose destination unwraps to a Go string, while keeping structured JSON flags strict and improving mismatch diagnostics.
• Introduce a typed FlagValueError and render it via the existing internal/output machinery for consistent agent-mode JSON envelopes.
• Neutralize backticks in flag descriptions so help placeholders show the flag’s real type, and suppress duplicate stderr printing via AlreadyReported.

File summaries

File	Description
test/flag-ergonomics.test.ts	New integration-style tests covering raw-text fallback, actionable errors, help placeholder behavior, and bounded echoing.
test/errors-teach.test.ts	Tightens agent-mode assertions to require pure JSON stderr and single-print behavior.
internal/output/reported.go	Adds AlreadyReported wrapper to prevent stderr duplication while preserving Unwrap.
internal/output/output.go	Returns AlreadyReported(err) after printing error JSON to avoid double-printing from main.go.
internal/output/agentmode.go	Wraps returned errors with AlreadyReported so agent-mode envelopes aren’t duplicated.
internal/flagutil/rawtext.go	Implements string-destination raw-text retry + improved structured-flag error shaping/hints.
internal/flagutil/metadata.go	Routes JSON unmarshal failures through raw-text retry / typed errors; neutralizes description backticks at registration.
internal/flagutil/flagerror.go	Introduces FlagValueError with bounded, rune-aware value truncation for safe echoing.
internal/flagutil/description.go	Replaces backticks in descriptions to prevent pflag placeholder hijacking.
internal/cli/root.go	Wraps execution with flag-value error rendering for agent mode.
internal/cli/flagerrors.go	Converts FlagValueError into output.AgentModeError envelope in agent mode.
cmd/vf/main.go	Skips printing errors with an empty message to prevent double stderr output.

Review details

Files not reviewed (5)

• cmd/vf/main.go: Generated file
• internal/cli/root.go: Generated file
• internal/flagutil/metadata.go: Generated file
• internal/output/agentmode.go: Generated file
• internal/output/output.go: Generated file

Suppressed comments (1)

internal/flagutil/rawtext.go:107

• Same issue as above: the non-pointer retry path reports the original parse error (cause) instead of the retry error, so the user/dev never sees why the quoted retry failed.

	target := reflect.New(fieldType)
	if err := utils.UnmarshalJsonFromString(string(quoted), target.Interface(), m.Annotations); err != nil {
		return rawTextRetryFailed(m, val, cause)
	}

• Files reviewed: 7/12 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Bradenream]

Confirmed and fixed in c37b670.

The retry error was shadowed and cause passed in its place, so a message whose only job is explaining why the quoted retry failed instead printed the original parse error — "this text is not JSON". That is the precondition for reaching this path at all, so it was true, expected, and useless for the one case the diagnostic exists for.

rawTextRetryFailed now takes the retry error and reports that; the original is kept as the wrapped Cause so the full sequence is still recoverable.

[Bradenream]

Confirmed and fixed in c37b670 — and it was not theoretical.

internal/output/agentmode.go treats twelve variables as "an agent is driving this". The helper cleared three. I reproduced it: with GITHUB_COPILOT=1 in the environment, the old helper failed 3 of 20 tests. The new one passes 20/20 with GITHUB_COPILOT, CODEX and AMAZON_Q all set.

The list is duplicated from Go into TypeScript and can drift, so tests that depend on which renderer ran now assert the mode explicitly. A thirteenth variable upstream fails loudly naming the cause, rather than quietly testing the wrong renderer.

[effervescentia]

changes to the type of a field (here accepting a string for Markup fields) should happen at the API level, not by modifying the generated flags downstream in the CLI

[Bradenream]

Splitting this rather than arguing it — @effervescentia's objection is isolated to one of the four changes, so the other three should not wait on it.

• fix: make flag failures legible, and help honest about flag types (COR-13656) #23 — error legibility, help placeholders, single-print. No parsing behaviour changes at all: every value accepted before is accepted now, every value rejected before is still rejected, and the request body is byte-identical to master across seven input shapes. Only the messages differ.
• feat: accept raw text for JSON flags whose destination is a string (COR-13656) #24 — the raw-text fallback, stacked on fix: make flag failures legible, and help honest about flag types (COR-13656) #23, with the review objection addressed by test rather than by argument.

On the objection itself, for the record here as well as on #24: I could not reproduce the premise.

• Markup is unreachable by this change. 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], OptionalNullable[[]Markup]. --url on mcp-server create is []components.Markup and still rejects raw text, with a regression test pinning it. No property in .speakeasy/out.openapi.yaml references Markup at all.
• No field's type changes. The retry only fires where the spec already says type: string — instructions is nullable: true, type: string. The bytes on the wire are identical to what the JSON-quoted form already produced, and identical to master.

So there is nothing to change at the API level: the spec is already correct. What is awkward is the codegen mapping from "nullable string" to a JSON-encoded flag. If you would still rather not carry that in the CLI, I am happy to drop #24 — #23 stands on its own and carries most of the value.

Closing this in favour of the two.