From fe038e5821f6701faed286a13e569785c02b64c3 Mon Sep 17 00:00:00 2001
From: BR <51544548+Bradenream@users.noreply.github.com>
Date: Mon, 31 Aug 2026 17:18:24 -0400
Subject: [PATCH 1/4] fix: make flag values and flag errors legible to humans
 and agents (COR-13656)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Three defects in the same place — the moment a flag value is typed — found
while watching an unattended coding agent configure a real agent. Together
they account for most of the 42 minutes it spent after the quickstart.

1. JSON flags whose destination is a string now accept raw text.

   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 tells a real null apart from
   the string "null" — that part is correct and is preserved. But nothing
   told the caller to wrap prose in JSON quotes, and the error named the
   first letter of the sentence. The fix retries an unparseable value as a
   JSON string when, and only when, the destination ultimately holds a Go
   string. 22 flags qualify.

   The discriminator has to unwrap OptionalNullable[T], which is defined as
   map[bool]*T — 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
   map[string]string and OptionalNullable[[]string].

   The trade, stated plainly: for those 22 flags, malformed JSON that used
   to fail now succeeds as literal text. Right for markdown authors, wrong
   for someone who meant JSON and typo'd. Structs, maps and slices are
   excluded, so mistyped objects still fail loudly.

2. Flag errors now teach.

   They never reached output.Error: they return raw from the generated RunE
   and main.go prints them with a bare Fprintln. So a mistyped flag gave one
   unstructured line while an API 401 gave a structured envelope with hints
   — 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.

3. Help now shows the flag's type instead of 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, so --instructions rendered as "--instructions Name" for a
   flag that wants markdown. 12 flags affected; the worst is transcript
   search's --version-param, whose only legal values are draft and published,
   labelled with an unrelated parameter name. Fixed at the one point
   descriptions enter cobra, so a 13th flag is covered automatically.

An OpenAPI overlay was assessed as an alternative for 1 and 3 and rejected:
it would pin 12 descriptions verbatim so upstream rewording is silently
overwritten, and fixing 1 that way means deleting nullable, trading away the
ability to clear a field. Persisted edits were confirmed durable first — all
four surviving edits came through the v0.234.0 regeneration intact.

Footprint on generated files is 4 insertions and 3 deletions across two
files; all real logic lives in new non-generated files.

Verified: 13 new tests, 9 of which fail against master and 4 of which pass
on both (the invariants: null still clears, quoted JSON unchanged, object
flags still reject). Full suite shows the same 11 pre-existing failures as
master — live integration tests with no credentials — and 13 more passes.
--usage output is byte-identical, as it was already correct.
---
 internal/cli/flagerrors.go       |  46 +++++++++++
 internal/cli/root.go             |   2 +-
 internal/flagutil/description.go |  37 +++++++++
 internal/flagutil/flagerror.go   |  62 +++++++++++++++
 internal/flagutil/metadata.go    |   5 +-
 internal/flagutil/rawtext.go     | 131 +++++++++++++++++++++++++++++++
 test/flag-ergonomics.test.ts     | 118 ++++++++++++++++++++++++++++
 7 files changed, 398 insertions(+), 3 deletions(-)
 create mode 100644 internal/cli/flagerrors.go
 create mode 100644 internal/flagutil/description.go
 create mode 100644 internal/flagutil/flagerror.go
 create mode 100644 internal/flagutil/rawtext.go
 create mode 100644 test/flag-ergonomics.test.ts

diff --git a/internal/cli/flagerrors.go b/internal/cli/flagerrors.go
new file mode 100644
index 0000000..e098781
--- /dev/null
+++ b/internal/cli/flagerrors.go
@@ -0,0 +1,46 @@
+// This file is not generated by Speakeasy — it renders flag-parsing failures
+// through the same path as every other CLI-level error.
+//
+// flagutil returns a typed *flagutil.FlagValueError but cannot format it: the
+// output package already imports flagutil, so flagutil importing output would be
+// an import cycle. cli.Execute is the first point that has both the resolved
+// *cobra.Command and permission to import output, so the rendering happens here.
+//
+// In agent mode this reuses output.AgentModeError, which is already the accepted
+// envelope for non-API CLI errors (see configure.go and auth.go) and is already
+// asserted by test/errors-teach.test.ts. Human mode keeps the plain multi-line
+// form, which reads better in a terminal than JSON.
+
+package cli
+
+import (
+	"errors"
+
+	"github.com/spf13/cobra"
+	"github.com/voiceflow/cli/internal/flagutil"
+	"github.com/voiceflow/cli/internal/output"
+)
+
+// renderFlagValueError converts a flag-value failure into the structured agent
+// envelope. Errors of any other kind are returned untouched, so this is safe to
+// wrap around the whole command execution.
+//
+// The message handed to AgentModeError is deliberately one line. cmd/vf/main.go
+// prints whatever Execute returns, so it echoes that message after the JSON —
+// the same double-print that configure and auth already produce today. Matching
+// that behavior keeps agent output uniform; passing the full multi-line form
+// would turn one trailing line into five.
+func renderFlagValueError(cmd *cobra.Command, err error) error {
+	var fve *flagutil.FlagValueError
+	if !errors.As(err, &fve) || !output.IsAgentMode() {
+		return err
+	}
+
+	hints := fve.Hints
+	if fve.Value != "" {
+		hints = append([]string{"you passed: " + fve.Value}, hints...)
+	}
+
+	return output.AgentModeError(cmd, "invalid_flag_value",
+		"invalid value for --"+fve.Flag+": "+fve.Expected, hints)
+}
diff --git a/internal/cli/root.go b/internal/cli/root.go
index 9b2f9c6..72d7277 100644
--- a/internal/cli/root.go
+++ b/internal/cli/root.go
@@ -226,7 +226,7 @@ func Execute() error {
 		return runExplorer(rootCmd)
 	}
 
-	return rootCmd.Execute()
+	return renderFlagValueError(rootCmd, rootCmd.Execute()) // see flagerrors.go
 }
 
 // shouldAutoExplore returns true when the CLI is invoked with no subcommand
diff --git a/internal/flagutil/description.go b/internal/flagutil/description.go
new file mode 100644
index 0000000..730d876
--- /dev/null
+++ b/internal/flagutil/description.go
@@ -0,0 +1,37 @@
+// This file is not generated by Speakeasy — it stops a flag's description from
+// renaming the flag in help output.
+//
+// The problem it solves: pflag's UnquoteUsage treats the first back-quoted word
+// in a usage string as the flag's value placeholder, and strips the quotes from
+// the prose. That is a deliberate pflag feature, but our descriptions come from
+// the OpenAPI spec, where backticks are ordinary prose emphasis. The result:
+//
+//	--instructions Name    Markdown text. Backticked Name resolves to a tool...
+//
+// The flag does not want a name. It wants markdown. Twelve flags are affected
+// today. The worst is transcript search's --version-param, whose only legal
+// values are draft and published, rendered as `environmentAlias` — an unrelated
+// parameter name that appears nowhere in its own list of options.
+//
+// Backticks are replaced rather than deleted so the emphasis the spec author
+// intended survives: "Backticked 'Name' resolves to a tool" still reads as a
+// reference to a token, while pflag now falls back to the flag's real type.
+//
+// Doing this at the single point where descriptions enter cobra fixes all twelve
+// and every future one automatically. The alternative — editing twelve
+// descriptions across eight generated files — would be twelve standing merge
+// conflicts that silently stop covering a thirteenth flag the day the spec adds
+// one.
+
+package flagutil
+
+import "strings"
+
+// flagDescription prepares an OpenAPI description for use as a pflag usage
+// string, neutralizing backticks so they cannot be read as a value placeholder.
+func flagDescription(description string) string {
+	if !strings.Contains(description, "`") {
+		return description
+	}
+	return strings.ReplaceAll(description, "`", "'")
+}
diff --git a/internal/flagutil/flagerror.go b/internal/flagutil/flagerror.go
new file mode 100644
index 0000000..00455f5
--- /dev/null
+++ b/internal/flagutil/flagerror.go
@@ -0,0 +1,62 @@
+// This file is not generated by Speakeasy — it gives flag-parsing failures a
+// typed shape so they can be rendered with the same care as API errors.
+//
+// The problem it solves: errors produced while interpreting a flag value never
+// reach output.Error. They are returned raw from the generated RunE, and because
+// the root command sets SilenceErrors, cmd/vf/main.go prints them with a bare
+// Fprintln. So an agent that mistypes a flag gets one unstructured line, while
+// an agent that gets a 401 back from the API gets a structured envelope with
+// hints and a docs URL. That is backwards: mistyped flags are the more common
+// failure, and they are the one the caller can actually act on.
+//
+// flagutil cannot import output — output already imports flagutil, so that
+// direction is an import cycle. This type is therefore pure data. It is carried
+// up to cli.Execute, which has both the *cobra.Command and permission to import
+// output, and is rendered there.
+
+package flagutil
+
+import (
+	"fmt"
+	"strings"
+)
+
+// FlagValueError reports a flag whose value could not be interpreted.
+//
+// It follows the house style already used for date parsing in this package —
+// state the expectation, show a worked example, echo what was actually passed —
+// so the reader can see the difference between what they wrote and what was
+// wanted without consulting documentation.
+type FlagValueError struct {
+	Flag     string   // flag name, without leading dashes
+	Value    string   // what the user actually passed
+	Expected string   // one line: what the flag wanted instead
+	Hints    []string // concrete next actions, most likely fix first
+	Cause    error    // underlying parse error, preserved for errors.Is/As
+}
+
+func (e *FlagValueError) Error() string {
+	var b strings.Builder
+	fmt.Fprintf(&b, "invalid value for --%s: %s", e.Flag, e.Expected)
+	if e.Value != "" {
+		fmt.Fprintf(&b, "\n  you passed: %s", truncateForError(e.Value))
+	}
+	for _, h := range e.Hints {
+		fmt.Fprintf(&b, "\n  %s", h)
+	}
+	return b.String()
+}
+
+func (e *FlagValueError) Unwrap() error { return e.Cause }
+
+// truncateForError keeps a long value from burying the explanation. Instructions
+// and prompts are routinely thousands of characters; echoing one in full pushes
+// the hints off the reader's screen, which defeats the point of the hints.
+func truncateForError(s string) string {
+	const max = 120
+	s = strings.ReplaceAll(s, "\n", " ")
+	if len(s) <= max {
+		return s
+	}
+	return s[:max] + "… (" + fmt.Sprint(len(s)) + " chars)"
+}
diff --git a/internal/flagutil/metadata.go b/internal/flagutil/metadata.go
index 3862141..c9daea9 100644
--- a/internal/flagutil/metadata.go
+++ b/internal/flagutil/metadata.go
@@ -105,6 +105,7 @@ type FlagMeta struct {
 // credentials) are skipped to prevent local flags from shadowing them.
 func RegisterFlags(cmd *cobra.Command, meta []FlagMeta) {
 	for _, m := range meta {
+		m.Description = flagDescription(m.Description) // see description.go
 		// Skip if a flag with this name already exists — either inherited from
 		// a parent command (persistent globals/security) or already registered
 		// locally (e.g., operation security flags like username/password).
@@ -817,13 +818,13 @@ func buildJSONField(cmd *cobra.Command, v reflect.Value, m FlagMeta) error {
 		holder := reflect.New(reflect.PtrTo(fieldType))
 		holder.Elem().Set(reflect.New(fieldType))
 		if err := utils.UnmarshalJsonFromString(val, holder.Interface(), m.Annotations); err != nil {
-			return fmt.Errorf("invalid value for --%s: %w", m.FlagName, err)
+			return setJSONFieldAsRawText(field, fieldType, isPtr, val, m, err) // see rawtext.go
 		}
 		field.Set(holder.Elem())
 	} else {
 		target := reflect.New(fieldType)
 		if err := utils.UnmarshalJsonFromString(val, target.Interface(), m.Annotations); err != nil {
-			return fmt.Errorf("invalid value for --%s: %w", m.FlagName, err)
+			return setJSONFieldAsRawText(field, fieldType, isPtr, val, m, err) // see rawtext.go
 		}
 		field.Set(target.Elem())
 	}
diff --git a/internal/flagutil/rawtext.go b/internal/flagutil/rawtext.go
new file mode 100644
index 0000000..b43ff7a
--- /dev/null
+++ b/internal/flagutil/rawtext.go
@@ -0,0 +1,131 @@
+// This file is not generated by Speakeasy — it lets flags that hold prose accept
+// prose.
+//
+// The problem it solves: a field marked `nullable: true` in the OpenAPI spec is
+// generated as FlagKindJSON, because JSON is the only encoding that can express
+// a real null distinctly from the four-letter string "null". That is correct,
+// and it is why --instructions null clears the field instead of setting it to
+// the word. But it also means the obvious invocation fails:
+//
+//	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's own description says "Markdown text", and the error names the
+// first letter of the sentence. Nothing tells the reader the value must be
+// wrapped in JSON quotes. The workaround — '"You are a support agent."' — is
+// quotes inside quotes, which is exactly the thing shell users and coding agents
+// get wrong.
+//
+// So: when a JSON flag fails to parse AND its destination ultimately holds a Go
+// string, re-encode the raw text as a JSON string and use that. Nullability is
+// preserved because the null branch runs earlier, in buildJSONField.
+//
+// The precedent is in buildJSONField already: reflect.Interface destinations
+// have done exactly this ("Not valid JSON — treat as raw string") since before
+// this file existed. This extends an established behavior to the string case
+// rather than inventing one.
+//
+// The trade being made, stated plainly: for these flags, malformed JSON that
+// used to fail now succeeds as literal text. That is right for someone writing
+// markdown — the overwhelmingly common case for these fields — and wrong for
+// someone who meant to pass JSON and typo'd it. Fields that hold structs, maps
+// or slices are deliberately excluded, so a mistyped JSON object still fails
+// loudly everywhere it would be meaningless as text.
+
+package flagutil
+
+import (
+	"encoding/json"
+	"fmt"
+	"reflect"
+
+	"github.com/voiceflow/cli/internal/sdk/sdkinternal/utils"
+)
+
+// stringLikeJSONTarget reports whether a FlagKindJSON destination ultimately
+// holds a Go string, unwrapping pointers and OptionalNullable[T].
+//
+// The OptionalNullable clause is the load-bearing part. OptionalNullable[T] is
+// defined as map[bool]*T, so its reflect.Kind is Map — indistinguishable by kind
+// alone from map[string]any. Requiring a bool key admits OptionalNullable[string]
+// and rejects every ordinary map. Checking Kind() == String alone would match
+// nothing at all, since no FlagKindJSON flag targets a bare string.
+func stringLikeJSONTarget(t reflect.Type) bool {
+	for t.Kind() == reflect.Ptr {
+		t = t.Elem()
+	}
+	// OptionalNullable[T] == map[bool]*T
+	if t.Kind() == reflect.Map && t.Key().Kind() == reflect.Bool && t.Elem().Kind() == reflect.Ptr {
+		t = t.Elem().Elem()
+		for t.Kind() == reflect.Ptr {
+			t = t.Elem()
+		}
+	}
+	return t.Kind() == reflect.String
+}
+
+// setJSONFieldAsRawText is the recovery path for a JSON flag whose value did not
+// parse. For string-valued destinations it retries with the input quoted as a
+// JSON string; for everything else it converts the parse failure into a
+// FlagValueError that explains what the flag actually wanted.
+//
+// The retry always builds a fresh destination rather than reusing the one the
+// failed attempt wrote into: a partial unmarshal may already have mutated it,
+// and silently shipping a half-populated value would be worse than the error
+// this replaces.
+func setJSONFieldAsRawText(field reflect.Value, fieldType reflect.Type, isPtr bool, val string, m FlagMeta, cause error) error {
+	if !stringLikeJSONTarget(field.Type()) {
+		return &FlagValueError{
+			Flag:     m.FlagName,
+			Value:    val,
+			Expected: "expected a JSON value",
+			Hints: []string{
+				`objects and arrays must be valid JSON, e.g. --` + m.FlagName + ` '{"key":"value"}'`,
+				"pass null to clear the field: --" + m.FlagName + " null",
+			},
+			Cause: cause,
+		}
+	}
+
+	// json.Marshal of a string cannot fail, but handle it rather than ignore it.
+	quoted, err := json.Marshal(val)
+	if err != nil {
+		return &FlagValueError{
+			Flag:     m.FlagName,
+			Value:    val,
+			Expected: "expected text or a JSON string",
+			Cause:    cause,
+		}
+	}
+
+	if isPtr {
+		holder := reflect.New(reflect.PtrTo(fieldType))
+		holder.Elem().Set(reflect.New(fieldType))
+		if err := utils.UnmarshalJsonFromString(string(quoted), holder.Interface(), m.Annotations); err != nil {
+			return rawTextRetryFailed(m, val, cause)
+		}
+		field.Set(holder.Elem())
+		return nil
+	}
+
+	target := reflect.New(fieldType)
+	if err := utils.UnmarshalJsonFromString(string(quoted), target.Interface(), m.Annotations); err != nil {
+		return rawTextRetryFailed(m, val, cause)
+	}
+	field.Set(target.Elem())
+	return nil
+}
+
+// rawTextRetryFailed reports a destination that looked string-shaped but refused
+// a JSON string anyway. Reaching this means stringLikeJSONTarget and the SDK's
+// unmarshaler disagree, so it names that explicitly instead of blaming the input.
+func rawTextRetryFailed(m FlagMeta, val string, cause error) error {
+	return &FlagValueError{
+		Flag:     m.FlagName,
+		Value:    val,
+		Expected: fmt.Sprintf("could not be set from text (%v)", cause),
+		Hints:    []string{"this is a bug in the CLI — please report it with the command you ran"},
+		Cause:    cause,
+	}
+}
diff --git a/test/flag-ergonomics.test.ts b/test/flag-ergonomics.test.ts
new file mode 100644
index 0000000..78c1d79
--- /dev/null
+++ b/test/flag-ergonomics.test.ts
@@ -0,0 +1,118 @@
+// Tests for how flag VALUES are interpreted and how failures are reported.
+//
+// Three behaviours ship together here because they are one story: the moment a
+// person or an agent types a flag.
+//
+//   1. JSON flags whose destination is a string accept raw text.
+//   2. Flags that cannot accept raw text fail with an error that teaches.
+//   3. Help shows the flag's real type, not a word borrowed from its prose.
+//
+// Every case runs the real binary with --dry-run, so nothing here touches the
+// network. Requires: go build -o vf ./cmd/vf
+
+import { execa } from 'execa';
+import * as path from 'node:path';
+import { describe, expect, it } from 'vitest';
+
+const VF = path.resolve(__dirname, '..', 'vf');
+
+const BASE = ['agent', 'update', '--project-id', 'p', '--environment-alias', 'main', '--dry-run', '--token', 'vfp_x'];
+
+/** Runs vf with agent-detection env vars stripped, so human output is exercised. */
+function run(args: string[], opts: { agentMode?: boolean } = {}) {
+  const env: Record<string, string | undefined> = opts.agentMode
+    ? { CLAUDECODE: '1' }
+    : { CLAUDECODE: undefined, CLAUDE_CODE: undefined, CURSOR_AGENT: undefined };
+  return execa({ reject: false, timeout: 20_000, stdin: 'ignore', env, extendEnv: true })(VF, args);
+}
+
+/** Pull a field back out of the --dry-run request preview. */
+function sent(output: string, field: string): string | null {
+  const m = output.match(new RegExp(`"${field}":\\s*(null|"(?:[^"\\\\]|\\\\.)*")`));
+  return m ? m[1] : null;
+}
+
+describe('string-valued JSON flags accept raw text', () => {
+  it('takes markdown prose without JSON quoting', async () => {
+    const r = await run([...BASE, '--instructions', 'You are a support agent for Acme.']);
+    expect(sent(r.stderr + r.stdout, 'instructions')).toBe('"You are a support agent for Acme."');
+  });
+
+  it('preserves newlines and quotes in prose', async () => {
+    const r = await run([...BASE, '--instructions', 'Line one\nSay "hello" politely.']);
+    expect(sent(r.stderr + r.stdout, 'instructions')).toBe('"Line one\\nSay \\"hello\\" politely."');
+  });
+
+  // The reason these flags are JSON-typed at all. If this regresses, the fallback
+  // has swallowed the one case the JSON encoding exists to express.
+  it('still sends a real JSON null for --instructions null', async () => {
+    const r = await run([...BASE, '--instructions', 'null']);
+    expect(sent(r.stderr + r.stdout, 'instructions')).toBe('null');
+  });
+
+  it('leaves an explicitly quoted JSON string unchanged', async () => {
+    const r = await run([...BASE, '--instructions', '"already json"']);
+    expect(sent(r.stderr + r.stdout, 'instructions')).toBe('"already json"');
+  });
+
+  // Documented edge case: valid JSON of the wrong shape is indistinguishable
+  // from prose, so it is stored as text. Asserted so the trade stays deliberate.
+  it('stores a JSON object passed to a text field as literal text', async () => {
+    const r = await run([...BASE, '--instructions', '{"note":"hi"}']);
+    expect(sent(r.stderr + r.stdout, 'instructions')).toBe('"{\\"note\\":\\"hi\\"}"');
+  });
+});
+
+describe('structured flags still reject raw text', () => {
+  // The fallback must not become "anything goes". A mistyped object is still an
+  // error, because storing prose in a field that models an object is nonsense.
+  for (const flag of ['llm', 'knowledge-base-tool']) {
+    it(`--${flag} fails rather than silently storing text`, async () => {
+      const r = await run([...BASE, `--${flag}`, 'not json at all']);
+      expect(r.exitCode).not.toBe(0);
+      expect(r.stderr).toContain(`invalid value for --${flag}`);
+    });
+  }
+
+  it('explains what was wanted, echoes what was passed, and shows a worked example', async () => {
+    const r = await run([...BASE, '--llm', 'gpt-4']);
+    expect(r.stderr).toContain('expected a JSON value');
+    expect(r.stderr).toContain('you passed: gpt-4');
+    expect(r.stderr).toContain(`--llm '{"key":"value"}'`);
+    expect(r.stderr).toContain('--llm null');
+  });
+
+  it('emits a structured envelope in agent mode', async () => {
+    const r = await run([...BASE, '--llm', 'gpt-4'], { agentMode: true });
+    const json = JSON.parse(r.stderr.slice(r.stderr.indexOf('{'), r.stderr.lastIndexOf('}') + 1));
+    expect(json.error_type).toBe('invalid_flag_value');
+    expect(json.error).toContain('invalid value for --llm');
+    expect(json.hints.join(' ')).toContain('you passed: gpt-4');
+  });
+});
+
+describe('help shows the flag type, not a word from its description', () => {
+  // pflag reads the first back-quoted word in a usage string as the value
+  // placeholder. Descriptions come from the OpenAPI spec, where backticks are
+  // prose emphasis, so --instructions used to render as `--instructions Name`.
+  const cases: Array<[cmd: string[], flag: string, wrong: string]> = [
+    [['agent', 'update'], 'instructions', 'Name'],
+    [['agent', 'update'], 'prompt', 'Name'],
+    [['transcript', 'search'], 'version-param', 'environmentAlias'],
+  ];
+
+  for (const [cmd, flag, wrong] of cases) {
+    it(`${cmd.join(' ')} --${flag} is labelled string, not ${wrong}`, async () => {
+      const r = await run([...cmd, '--help']);
+      const line = (r.stdout + r.stderr).split('\n').find((l) => l.includes(`--${flag} `));
+      expect(line, `no help line for --${flag}`).toBeDefined();
+      expect(line).toContain(`--${flag} string`);
+      expect(line).not.toContain(`--${flag} ${wrong}`);
+    });
+  }
+
+  it('keeps the description prose readable after backticks are neutralized', async () => {
+    const r = await run(['agent', 'update', '--help']);
+    expect(r.stdout + r.stderr).toContain("Backticked 'Name' resolves to");
+  });
+});

From 29dfda6eb75d85003b37c05e23429765a0d8cd63 Mon Sep 17 00:00:00 2001
From: BR <51544548+Bradenream@users.noreply.github.com>
Date: Mon, 31 Aug 2026 17:25:53 -0400
Subject: [PATCH 2/4] fix: print agent-mode errors exactly once (COR-13656)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Every error printer in internal/output writes the message to stderr and also
returns the error, and cmd/vf/main.go printed whatever Execute returned. So a
single failure reached stderr twice:

    CLAUDECODE=1 vf configure
    { "error": "…", "error_type": "configure_blocked", "hints": [ … ] }
    the 'configure' command is interactive and cannot be used in agent mode

AgentModeError's own doc comment promised the opposite — "outputs structured
JSON exactly once to stderr… Callers must NOT print the error again" — but
main.go is generated and printed unconditionally, so the contract was never
actually kept by anything.

This matters because in agent mode the envelope IS the product. A trailing
non-JSON line after a JSON object is exactly what breaks a consumer that does
the obvious thing, and being driven by agents is what this CLI is for. All
three agent-mode error shapes were affected: AgentModeError (configure, auth),
output.Error (API and preflight failures), and the flag-value errors added in
the previous commit.

Errors that have been printed are now marked, and main.go skips printing a
marked error. The marker renders as an empty message, which is what actually
suppresses the second print — main.go imports only internal/cli and so cannot
call a predicate in internal/output. That cost is stated in the file: anything
that formats one of these with %v renders nothing, which is contained because
exactly one place formats them and it is guarded.

Pretty output is deliberately untouched. There the trailing line is the
readable one-line summary that follows a raw API body, so suppressing it would
remove the most useful part of the message rather than a duplicate. Verified
byte-identical to master across four error classes.

One deliberate behavior change beyond agent mode: --output-format json now
emits pure JSON on stderr for errors, where it previously appended the same
prose line. If you asked for JSON you should get JSON, and everything in that
line is already in the envelope as message, hints and docs_url.

Footprint on generated files is 8 insertions and 4 deletions across three
files. Verified: 6 new tests, all 6 failing against master; stderr now parses
as a single JSON document for all three error shapes; exit codes unchanged at
1. Full suite shows the same 11 pre-existing failures as master.
---
 cmd/vf/main.go               |  6 +++-
 internal/output/agentmode.go |  4 +--
 internal/output/output.go    |  2 +-
 internal/output/reported.go  | 55 ++++++++++++++++++++++++++++++++++++
 test/errors-teach.test.ts    | 45 ++++++++++++++++++++++++++++-
 5 files changed, 107 insertions(+), 5 deletions(-)
 create mode 100644 internal/output/reported.go

diff --git a/cmd/vf/main.go b/cmd/vf/main.go
index 86a343d..af77fb0 100644
--- a/cmd/vf/main.go
+++ b/cmd/vf/main.go
@@ -25,7 +25,11 @@ func main() {
 	}
 
 	if err := cli.Execute(); err != nil {
-		fmt.Fprintln(os.Stderr, err)
+		// An empty message means the producer already wrote it to stderr;
+		// printing again would duplicate it. See internal/output/reported.go.
+		if msg := err.Error(); msg != "" {
+			fmt.Fprintln(os.Stderr, msg)
+		}
 		os.Exit(1)
 	}
 }
diff --git a/internal/output/agentmode.go b/internal/output/agentmode.go
index 8d53fd9..653e9f5 100644
--- a/internal/output/agentmode.go
+++ b/internal/output/agentmode.go
@@ -176,11 +176,11 @@ func AgentModeError(cmd *cobra.Command, errorType, message string, hints []strin
 	jsonData, err := json.MarshalIndent(envelope, "", "  ")
 	if err != nil {
 		fmt.Fprintln(out, message)
-		return fmt.Errorf("%s", message)
+		return AlreadyReported(fmt.Errorf("%s", message)) // see reported.go
 	}
 
 	// No colorization in agent mode.
 	fmt.Fprintln(out, string(jsonData))
 
-	return fmt.Errorf("%s", message)
+	return AlreadyReported(fmt.Errorf("%s", message)) // see reported.go
 }
diff --git a/internal/output/output.go b/internal/output/output.go
index 68b4fbf..8763109 100644
--- a/internal/output/output.go
+++ b/internal/output/output.go
@@ -404,7 +404,7 @@ func Error(cmd *cobra.Command, err error) error {
 
 	printJSON(out, jsonData, colorize)
 
-	return err
+	return AlreadyReported(err) // see reported.go
 }
 
 // tryReadRawBody attempts to read the raw HTTP response body from the response
diff --git a/internal/output/reported.go b/internal/output/reported.go
new file mode 100644
index 0000000..816d1da
--- /dev/null
+++ b/internal/output/reported.go
@@ -0,0 +1,55 @@
+// This file is not generated by Speakeasy — it stops errors that have already
+// been written to stderr from being written to stderr a second time.
+//
+// The problem it solves: every error printer in this package prints AND returns
+// the error, and cmd/vf/main.go then prints whatever Execute returns. So a
+// single failure reaches stderr twice:
+//
+//	CLAUDECODE=1 vf configure
+//	{ "error": "…", "error_type": "configure_blocked", "hints": [ … ] }
+//	the 'configure' command is interactive and cannot be used in agent mode
+//
+// AgentModeError's own documentation states the contract this violates —
+// "outputs structured JSON exactly once to stderr… Callers must NOT print the
+// error again" — but main.go is generated and prints unconditionally, so the
+// contract was never actually kept.
+//
+// This matters most in agent mode, where the envelope is the machine-readable
+// product. A trailing non-JSON line after a JSON object is exactly what breaks
+// a naive parser, and this CLI's whole purpose is being driven by agents.
+//
+// Human (pretty) output is deliberately left alone. There the trailing line is
+// the readable one-line summary that follows a raw API body, so suppressing it
+// would remove the most useful part of the message rather than a duplicate.
+
+package output
+
+// alreadyReported marks an error whose message has been written to stderr by
+// the code that produced it.
+//
+// Error returns the empty string, which is what actually suppresses the second
+// print: main.go skips errors with an empty message. The alternative — exporting
+// a predicate for main.go to call — does not work, because main.go imports only
+// internal/cli and cannot see this package.
+//
+// The wrapped error is preserved through Unwrap, so errors.Is and errors.As
+// continue to work on anything a caller wants to inspect.
+//
+// The cost of this design, stated plainly: any future code that formats one of
+// these errors with %v or err.Error() renders nothing. That is contained today
+// because exactly one place formats them — main.go — and it is guarded. Anything
+// that needs the text must call Unwrap first.
+type alreadyReported struct{ err error }
+
+func (e alreadyReported) Error() string { return "" }
+func (e alreadyReported) Unwrap() error { return e.err }
+
+// AlreadyReported marks err as having been printed, so the process exits
+// non-zero without repeating the message. Returns nil for nil so it is safe to
+// wrap a return value directly.
+func AlreadyReported(err error) error {
+	if err == nil {
+		return nil
+	}
+	return alreadyReported{err: err}
+}
diff --git a/test/errors-teach.test.ts b/test/errors-teach.test.ts
index f218484..f14324a 100644
--- a/test/errors-teach.test.ts
+++ b/test/errors-teach.test.ts
@@ -16,7 +16,14 @@ const AGENT_ENV = { CLAUDE_CODE: '1', VF_TOKEN: '' } as const;
 const $vf = (args: string[], env: Record<string, string> = {}) =>
   execa({ reject: false, env: { ...AGENT_ENV, ...env }, stdin: 'ignore' })(VF, args);
 
-/** stderr of agent-mode errors is a JSON envelope followed by plain lines. */
+/**
+ * In agent mode stderr is the JSON envelope and nothing else. It used to be the
+ * envelope followed by a duplicate plain-text line, because every printer in
+ * internal/output prints AND returns the error while cmd/vf/main.go printed
+ * whatever Execute returned. This parser reads the first balanced object, so it
+ * tolerated the duplicate; the "is nothing but JSON" assertions below are what
+ * actually pin the single-print contract.
+ */
 function parseEnvelope(stderr: string): Record<string, unknown> {
   const start = stderr.indexOf('{');
   expect(start, `no JSON envelope in stderr:\n${stderr}`).toBeGreaterThanOrEqual(0);
@@ -150,3 +157,39 @@ describe('error hint injection', () => {
     expect(JSON.stringify(envelope.hints)).toContain('export VF_TOKEN=vfp_');
   });
 });
+
+describe('agent-mode errors are printed exactly once', () => {
+  // Every printer in internal/output writes the error to stderr and also returns
+  // it, and main.go printed whatever Execute returned — so a single failure
+  // reached stderr twice. AgentModeError's own doc comment promised the
+  // opposite ("outputs structured JSON exactly once… Callers must NOT print the
+  // error again"), but main.go is generated and printed unconditionally.
+  //
+  // This matters because the envelope is the machine-readable product: a
+  // trailing non-JSON line after a JSON object is exactly what breaks a parser
+  // that does the obvious thing.
+  const cases: Array<[name: string, args: string[]]> = [
+    ['CLI-level error (AgentModeError)', ['configure']],
+    ['preflight error (no token)', ['workspace', 'list']],
+    ['flag-value error', ['agent', 'update', '--project-id', 'p', '--environment-alias', 'main',
+      '--dry-run', '--token', 'vfp_x', '--llm', 'not json']],
+  ];
+
+  for (const [name, args] of cases) {
+    it(`${name}: stderr is nothing but the JSON envelope`, async () => {
+      const result = await $vf(args);
+      expect(result.exitCode, 'must still fail').not.toBe(0);
+      // The strict test: the whole stream parses, with no trailing prose.
+      expect(() => JSON.parse(result.stderr.trim()), `stderr was not pure JSON:\n${result.stderr}`).not.toThrow();
+    });
+
+    it(`${name}: the message does not appear twice`, async () => {
+      const result = await $vf(args);
+      const envelope = parseEnvelope(result.stderr);
+      const message = String(envelope.message ?? envelope.error ?? '');
+      expect(message.length, 'envelope carried no message to check').toBeGreaterThan(0);
+      const after = result.stderr.slice(result.stderr.lastIndexOf('}') + 1);
+      expect(after.trim(), `text printed after the envelope:\n${after}`).toBe('');
+    });
+  }
+});

From a70fe3aa38ba9224a26979c0bef21df3c9060422 Mon Sep 17 00:00:00 2001
From: BR <51544548+Bradenream@users.noreply.github.com>
Date: Mon, 31 Aug 2026 18:08:49 -0400
Subject: [PATCH 3/4] fix: make flag rejection messages actionable, and bound
 what they echo (COR-13656)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Adversarial review of the two previous commits raised 23 findings; 12 were
refuted by independent verification and 11 survived. Three were blocking, and
all three were defects I introduced. Each is fixed here with a test that fails
against the pre-review branch.

1. The suggested example was one the CLI itself rejects.

   rawtext.go hardcoded '{"key":"value"}' for every structured destination, so
   on the array-valued flags it told the caller to pass a value the same binary
   refuses:

     vf agent update --playbooks notjson
       objects and arrays must be valid JSON, e.g. --playbooks '{"key":"value"}'
     vf agent update --playbooks '{"key":"value"}'    # do exactly what it said
       objects and arrays must be valid JSON, e.g. --playbooks '{"key":"value"}'

   Identical error, nothing new to try — a loop, for the agents this work is
   aimed at. Worse than master, which at least said "cannot unmarshal object
   into []json.RawMessage" and so named the array. The example is now derived
   from the destination kind, and an unrecognized shape produces no example at
   all: a wrong example is worse than none. The regression test takes the
   CLI's own suggestion and feeds it back in.

2. Valid JSON of the wrong shape was reported as invalid JSON.

   --llm '[1,2]' parses perfectly; it is simply not an object. The message said
   "expected a JSON value" and hinted "must be valid JSON", sending the reader
   to re-check syntax that was never wrong. Master said "cannot unmarshal array
   into Go value of type map[string]json.RawMessage" — uglier, and correct.

   That detail was being swallowed: FlagValueError.Cause was stored and never
   rendered on any path. Shape mismatch is now named separately, and the
   decoder's own message is surfaced, minus its "response body" prefix, which
   is wrong for a request the CLI has not sent.

3. Agent mode bypassed the truncation written for exactly this.

   renderFlagValueError echoed the raw value; truncateForError was unexported
   so internal/cli could not reach it. Measured on the same input: master 118
   bytes, human 311, agent 5,329 for a 5,000-char value and 400,331 for a
   400KB one — unbounded and linear in input. It also reproduced a credential
   embedded in a malformed config blob that master never echoed. Now behind an
   exported DisplayValue, tested in both modes.

Also fixed, non-blocking:

4. truncateForError sliced bytes, so a multi-byte rune was cut in half and the
   reported length was wrong — 200 Japanese characters reported as 600. Now
   counts and cuts in runes.

5. A comment in flagerrors.go described the double-print that the previous
   commit removed. Rewritten.

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, the most common flag error, and the value is the caller's own argv —
already in process args and shell history. Bounded, and documented in the file.

Not fixed here, filed separately: --agent-mode / --agent-mode=false is inert
because InitAgentMode latches before flag parsing. Verified present on master,
so not a regression from this work.

Verified: 20 tests in flag-ergonomics, 6 of the 7 new ones failing against the
pre-review branch (the 7th guards human mode, which already had the cap). Full
suite 66 passing with the same 11 pre-existing failures as master. Non-flag
human output still byte-identical to master.
---
 internal/cli/flagerrors.go     | 17 +++---
 internal/flagutil/flagerror.go | 19 +++++--
 internal/flagutil/rawtext.go   | 99 ++++++++++++++++++++++++++++++----
 test/flag-ergonomics.test.ts   | 65 ++++++++++++++++++++++
 4 files changed, 180 insertions(+), 20 deletions(-)

diff --git a/internal/cli/flagerrors.go b/internal/cli/flagerrors.go
index e098781..0fef3a2 100644
--- a/internal/cli/flagerrors.go
+++ b/internal/cli/flagerrors.go
@@ -25,11 +25,14 @@ import (
 // envelope. Errors of any other kind are returned untouched, so this is safe to
 // wrap around the whole command execution.
 //
-// The message handed to AgentModeError is deliberately one line. cmd/vf/main.go
-// prints whatever Execute returns, so it echoes that message after the JSON —
-// the same double-print that configure and auth already produce today. Matching
-// that behavior keeps agent output uniform; passing the full multi-line form
-// would turn one trailing line into five.
+// The message handed to AgentModeError is deliberately one line: the detail
+// belongs in hints[], where a consumer can read it as structured data rather
+// than parsing it out of a sentence.
+//
+// The echoed value goes through DisplayValue, never the raw fve.Value. This
+// render path is the one that matters most for that: it feeds an agent's
+// context, and an unbounded echo of a malformed blob both floods that context
+// and can reproduce a secret that happened to be inside it.
 func renderFlagValueError(cmd *cobra.Command, err error) error {
 	var fve *flagutil.FlagValueError
 	if !errors.As(err, &fve) || !output.IsAgentMode() {
@@ -37,8 +40,8 @@ func renderFlagValueError(cmd *cobra.Command, err error) error {
 	}
 
 	hints := fve.Hints
-	if fve.Value != "" {
-		hints = append([]string{"you passed: " + fve.Value}, hints...)
+	if v := fve.DisplayValue(); v != "" {
+		hints = append([]string{"you passed: " + v}, hints...)
 	}
 
 	return output.AgentModeError(cmd, "invalid_flag_value",
diff --git a/internal/flagutil/flagerror.go b/internal/flagutil/flagerror.go
index 00455f5..6f8bf3f 100644
--- a/internal/flagutil/flagerror.go
+++ b/internal/flagutil/flagerror.go
@@ -49,14 +49,27 @@ func (e *FlagValueError) Error() string {
 
 func (e *FlagValueError) Unwrap() error { return e.Cause }
 
+// DisplayValue is the value as it should be shown to a reader: shortened, and
+// with newlines flattened. Exported because internal/cli renders these errors
+// too and must not echo the raw value — an unbounded echo would flood an agent's
+// context and can reproduce secrets that happened to sit inside a malformed
+// blob. Every render path must go through this, never through Value directly.
+func (e *FlagValueError) DisplayValue() string { return truncateForError(e.Value) }
+
 // truncateForError keeps a long value from burying the explanation. Instructions
 // and prompts are routinely thousands of characters; echoing one in full pushes
-// the hints off the reader's screen, which defeats the point of the hints.
+// the hints off the reader's screen, which defeats the point of the hints. It
+// also bounds what a malformed value can spill into a log or an agent's context.
+//
+// Counting and cutting are done in runes, not bytes. Slicing a byte index splits
+// multi-byte characters in half, which writes invalid UTF-8 to stderr and makes
+// the reported length wrong for any non-ASCII value.
 func truncateForError(s string) string {
 	const max = 120
 	s = strings.ReplaceAll(s, "\n", " ")
-	if len(s) <= max {
+	runes := []rune(s)
+	if len(runes) <= max {
 		return s
 	}
-	return s[:max] + "… (" + fmt.Sprint(len(s)) + " chars)"
+	return string(runes[:max]) + "… (" + fmt.Sprint(len(runes)) + " chars)"
 }
diff --git a/internal/flagutil/rawtext.go b/internal/flagutil/rawtext.go
index b43ff7a..b30700a 100644
--- a/internal/flagutil/rawtext.go
+++ b/internal/flagutil/rawtext.go
@@ -39,6 +39,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"reflect"
+	"strings"
 
 	"github.com/voiceflow/cli/internal/sdk/sdkinternal/utils"
 )
@@ -76,16 +77,7 @@ func stringLikeJSONTarget(t reflect.Type) bool {
 // this replaces.
 func setJSONFieldAsRawText(field reflect.Value, fieldType reflect.Type, isPtr bool, val string, m FlagMeta, cause error) error {
 	if !stringLikeJSONTarget(field.Type()) {
-		return &FlagValueError{
-			Flag:     m.FlagName,
-			Value:    val,
-			Expected: "expected a JSON value",
-			Hints: []string{
-				`objects and arrays must be valid JSON, e.g. --` + m.FlagName + ` '{"key":"value"}'`,
-				"pass null to clear the field: --" + m.FlagName + " null",
-			},
-			Cause: cause,
-		}
+		return notJSONError(field.Type(), val, m, cause)
 	}
 
 	// json.Marshal of a string cannot fail, but handle it rather than ignore it.
@@ -129,3 +121,90 @@ func rawTextRetryFailed(m FlagMeta, val string, cause error) error {
 		Cause:    cause,
 	}
 }
+
+// notJSONError explains why a structured JSON flag rejected its value.
+//
+// Two distinct failures used to collapse into one message, and the merged
+// wording was actively false for the second:
+//
+//	--llm 'gpt-4'    not JSON at all
+//	--llm '[1,2]'    valid JSON, but this flag wants an object
+//
+// Telling someone whose input is well-formed JSON that it "must be valid JSON"
+// sends them to re-check syntax that was never wrong. So the shape mismatch is
+// named separately, and the unmarshal error — the only thing that knows which
+// Go type was expected — is surfaced instead of being swallowed.
+func notJSONError(t reflect.Type, val string, m FlagMeta, cause error) error {
+	hints := []string{}
+	expected := "expected a JSON value"
+
+	if json.Valid([]byte(val)) {
+		// Syntax is fine; the shape is not. Say that, rather than blaming syntax.
+		expected = "the value is valid JSON but not the shape this flag expects"
+	} else {
+		hints = append(hints, "the value is not valid JSON")
+	}
+
+	if example := jsonShapeExample(t); example != "" {
+		hints = append(hints, fmt.Sprintf("expected shape: --%s '%s'", m.FlagName, example))
+	}
+	if detail := unmarshalDetail(cause); detail != "" {
+		hints = append(hints, detail)
+	}
+	hints = append(hints, "pass null to clear the field: --"+m.FlagName+" null")
+
+	return &FlagValueError{Flag: m.FlagName, Value: val, Expected: expected, Hints: hints, Cause: cause}
+}
+
+// jsonShapeExample returns a value of the right shape for t, or "" when the
+// shape cannot be stated confidently.
+//
+// The example must be something the CLI would actually accept. The first
+// version of this hint hardcoded an object for every destination, so on the
+// array-valued flags it told the caller to pass a value the same binary
+// rejects — and a caller following the instruction exactly got back the
+// identical error, with nothing new to try. A wrong example is worse than no
+// example, so an unrecognized shape returns "" and the hint is omitted.
+func jsonShapeExample(t reflect.Type) string {
+	for t.Kind() == reflect.Ptr {
+		t = t.Elem()
+	}
+	// OptionalNullable[T] == map[bool]*T — unwrap to the value it carries.
+	if t.Kind() == reflect.Map && t.Key().Kind() == reflect.Bool && t.Elem().Kind() == reflect.Ptr {
+		t = t.Elem().Elem()
+		for t.Kind() == reflect.Ptr {
+			t = t.Elem()
+		}
+	}
+
+	switch t.Kind() {
+	case reflect.Slice, reflect.Array:
+		return `[{"key":"value"}]`
+	case reflect.Map, reflect.Struct:
+		return `{"key":"value"}`
+	default:
+		return ""
+	}
+}
+
+// unmarshalDetail surfaces what the JSON decoder objected to, minus a prefix
+// that is wrong in this context.
+//
+// The SDK's unmarshaler labels every failure "error unmarshalling json response
+// body", but a flag value is a request the CLI has not sent yet — there is no
+// response. Left intact it sends the reader looking for a server problem that
+// does not exist. The remainder is worth keeping: it names the Go type that was
+// expected, which is the only place that information appears at all.
+func unmarshalDetail(cause error) string {
+	if cause == nil {
+		return ""
+	}
+	detail := cause.Error()
+	for _, prefix := range []string{"error unmarshalling json response body: ", "json: "} {
+		detail = strings.TrimPrefix(detail, prefix)
+	}
+	if detail == "" {
+		return ""
+	}
+	return "decoder reported: " + detail
+}
diff --git a/test/flag-ergonomics.test.ts b/test/flag-ergonomics.test.ts
index 78c1d79..b182558 100644
--- a/test/flag-ergonomics.test.ts
+++ b/test/flag-ergonomics.test.ts
@@ -116,3 +116,68 @@ describe('help shows the flag type, not a word from its description', () => {
     expect(r.stdout + r.stderr).toContain("Backticked 'Name' resolves to");
   });
 });
+
+describe('rejection messages are actionable, not circular', () => {
+  // The strongest form of this test: take the CLI's own suggestion and feed it
+  // back in. An earlier version hardcoded an object example for every JSON
+  // destination, so on array-valued flags it told the caller to pass a value the
+  // same binary rejects — following the instruction exactly reproduced the
+  // identical error, with nothing new to try. For an agent that is a loop.
+  const shaped: Array<[flag: string, extraArgs: string[]]> = [
+    ['playbooks', []], // slice-valued  -> must suggest an array
+    ['llm', []],       // map-valued    -> must suggest an object
+  ];
+
+  for (const [flag, extra] of shaped) {
+    it(`--${flag}: the suggested example is one the CLI accepts`, async () => {
+      const rejected = await run([...BASE, ...extra, `--${flag}`, 'not json at all']);
+      const suggestion = (rejected.stderr + rejected.stdout).match(
+        new RegExp(`expected shape: --${flag} '(.+)'`),
+      )?.[1];
+      expect(suggestion, `no shape hint offered for --${flag}`).toBeDefined();
+
+      // Do exactly what the CLI said to do. It must not fail the same way.
+      const retry = await run([...BASE, ...extra, `--${flag}`, suggestion!]);
+      expect(retry.stderr, `the CLI's own suggestion ${suggestion} was rejected`).not.toContain(
+        `invalid value for --${flag}`,
+      );
+    });
+  }
+
+  it('does not claim the input is invalid JSON when it is valid JSON', async () => {
+    // '[1,2]' parses fine; it is the wrong shape for a map-valued flag. Saying
+    // "not valid JSON" sends the reader to re-check syntax that was never wrong.
+    const r = await run([...BASE, '--llm', '[1,2]']);
+    expect(r.stderr).toContain('valid JSON but not the shape');
+    expect(r.stderr).not.toContain('the value is not valid JSON');
+  });
+
+  it('surfaces what the decoder objected to, without the misleading response-body prefix', async () => {
+    const r = await run([...BASE, '--llm', '[1,2]']);
+    expect(r.stderr).toContain('cannot unmarshal array');
+    // A flag value is a request that was never sent; there is no response body.
+    expect(r.stderr).not.toContain('response body');
+  });
+});
+
+describe('echoed values are bounded', () => {
+  const big = 'x'.repeat(5_000);
+
+  // The echo exists to reveal shell-quoting mistakes, so it stays — but an
+  // unbounded one floods an agent's context and reproduces whatever sat inside a
+  // malformed blob. Agent mode originally bypassed the cap entirely.
+  for (const mode of ['human', 'agent'] as const) {
+    it(`${mode} mode: a 5000-char value does not produce a 5000-char error`, async () => {
+      const r = await run([...BASE, '--llm', big], { agentMode: mode === 'agent' });
+      expect(r.stderr.length, `error grew with the input (${r.stderr.length} bytes)`).toBeLessThan(1_500);
+      expect(r.stderr).toContain('chars)'); // says how long it really was
+    });
+  }
+
+  it('counts and cuts in characters, not bytes', async () => {
+    // Byte-slicing splits a multi-byte rune and misreports the length.
+    const r = await run([...BASE, '--llm', 'あ'.repeat(200)]);
+    expect(r.stderr).toContain('(200 chars)');
+    expect(r.stderr).not.toContain('(600 chars)');
+  });
+});

From c37b670ddfd6b78a69eb53d418c2514f5debeafb Mon Sep 17 00:00:00 2001
From: BR <51544548+Bradenream@users.noreply.github.com>
Date: Tue, 1 Sep 2026 12:46:00 -0400
Subject: [PATCH 4/4] fix: report the retry error, and isolate tests from
 agent-mode env (COR-13656)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Both from Copilot's review of this PR. Both verified before fixing, and both
real.

1. rawTextRetryFailed reported the wrong error.

   The retry's own error was shadowed and discarded, and the original parse
   error was passed in its place. So a message whose entire purpose is to
   explain why the quoted retry failed instead printed "this text is not JSON"
   — true, expected, and useless, since not-JSON is the precondition for
   reaching that path at all. This path only fires when stringLikeJSONTarget
   and the SDK unmarshaler disagree, which is a CLI bug, so the diagnostic is
   the only thing a reporter has to go on. The retry error is now what gets
   reported; the original is kept as the wrapped Cause.

2. The human-mode test helper cleared 3 of 12 agent-detection variables.

   internal/output/agentmode.go treats twelve environment variables as "an
   agent is driving this", including GITHUB_COPILOT and CODEX. The helper
   cleared CLAUDECODE, CLAUDE_CODE and CURSOR_AGENT only, so on any machine
   with one of the other nine set, human-mode tests ran against the agent
   renderer.

   Demonstrated rather than assumed: 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 appearing upstream fails loudly with a message naming the cause,
   instead of quietly testing the wrong renderer.
---
 internal/flagutil/rawtext.go | 14 ++++++++++----
 test/flag-ergonomics.test.ts | 36 ++++++++++++++++++++++++++++++++----
 2 files changed, 42 insertions(+), 8 deletions(-)

diff --git a/internal/flagutil/rawtext.go b/internal/flagutil/rawtext.go
index b30700a..79f6eee 100644
--- a/internal/flagutil/rawtext.go
+++ b/internal/flagutil/rawtext.go
@@ -95,7 +95,7 @@ func setJSONFieldAsRawText(field reflect.Value, fieldType reflect.Type, isPtr bo
 		holder := reflect.New(reflect.PtrTo(fieldType))
 		holder.Elem().Set(reflect.New(fieldType))
 		if err := utils.UnmarshalJsonFromString(string(quoted), holder.Interface(), m.Annotations); err != nil {
-			return rawTextRetryFailed(m, val, cause)
+			return rawTextRetryFailed(m, val, err, cause)
 		}
 		field.Set(holder.Elem())
 		return nil
@@ -103,7 +103,7 @@ func setJSONFieldAsRawText(field reflect.Value, fieldType reflect.Type, isPtr bo
 
 	target := reflect.New(fieldType)
 	if err := utils.UnmarshalJsonFromString(string(quoted), target.Interface(), m.Annotations); err != nil {
-		return rawTextRetryFailed(m, val, cause)
+		return rawTextRetryFailed(m, val, err, cause)
 	}
 	field.Set(target.Elem())
 	return nil
@@ -112,11 +112,17 @@ func setJSONFieldAsRawText(field reflect.Value, fieldType reflect.Type, isPtr bo
 // rawTextRetryFailed reports a destination that looked string-shaped but refused
 // a JSON string anyway. Reaching this means stringLikeJSONTarget and the SDK's
 // unmarshaler disagree, so it names that explicitly instead of blaming the input.
-func rawTextRetryFailed(m FlagMeta, val string, cause error) error {
+//
+// retryErr is the error from the quoted retry — the one that describes the
+// disagreement. The first version reported the original parse error here, which
+// only ever says "this text is not JSON": true, expected, and useless for
+// diagnosing why the retry failed. The original is kept as the wrapped Cause so
+// the full sequence is still recoverable.
+func rawTextRetryFailed(m FlagMeta, val string, retryErr, cause error) error {
 	return &FlagValueError{
 		Flag:     m.FlagName,
 		Value:    val,
-		Expected: fmt.Sprintf("could not be set from text (%v)", cause),
+		Expected: fmt.Sprintf("could not be set from text (%v)", retryErr),
 		Hints:    []string{"this is a bug in the CLI — please report it with the command you ran"},
 		Cause:    cause,
 	}
diff --git a/test/flag-ergonomics.test.ts b/test/flag-ergonomics.test.ts
index b182558..2254675 100644
--- a/test/flag-ergonomics.test.ts
+++ b/test/flag-ergonomics.test.ts
@@ -18,14 +18,40 @@ const VF = path.resolve(__dirname, '..', 'vf');
 
 const BASE = ['agent', 'update', '--project-id', 'p', '--environment-alias', 'main', '--dry-run', '--token', 'vfp_x'];
 
-/** Runs vf with agent-detection env vars stripped, so human output is exercised. */
+// Every variable that puts the CLI into agent mode. Mirrors the list in
+// internal/output/agentmode.go — if that grows and this does not, a human-mode
+// test running on a machine that sets the new one would silently assert against
+// agent output. assertMode below is the guard against exactly that drift.
+const AGENT_ENV_VARS = [
+  'CLAUDECODE', 'CLAUDE_CODE', 'CURSOR_AGENT', 'CODEX', 'AIDER', 'CLINE',
+  'WINDSURF_AGENT', 'GITHUB_COPILOT', 'AMAZON_Q', 'GEMINI_CODE_ASSIST',
+  'SRC_CODY', 'FORCE_AGENT_MODE',
+];
+
+/**
+ * Runs vf in a known output mode.
+ *
+ * Human mode has to clear ALL of AGENT_ENV_VARS, not just the obvious few: CI
+ * runs on GitHub, where GITHUB_COPILOT may well be set, and a stray one silently
+ * flips the CLI into agent mode so the assertions check the wrong renderer.
+ */
 function run(args: string[], opts: { agentMode?: boolean } = {}) {
-  const env: Record<string, string | undefined> = opts.agentMode
-    ? { CLAUDECODE: '1' }
-    : { CLAUDECODE: undefined, CLAUDE_CODE: undefined, CURSOR_AGENT: undefined };
+  const env: Record<string, string | undefined> = Object.fromEntries(
+    AGENT_ENV_VARS.map((name) => [name, undefined]),
+  );
+  if (opts.agentMode) env.CLAUDECODE = '1';
   return execa({ reject: false, timeout: 20_000, stdin: 'ignore', env, extendEnv: true })(VF, args);
 }
 
+/** Fails loudly if the CLI rendered in the mode we did not ask for. */
+function assertMode(stderr: string, mode: 'human' | 'agent') {
+  const looksLikeAgent = stderr.trimStart().startsWith('{');
+  expect(
+    looksLikeAgent,
+    `expected ${mode} output but got the other renderer — a new agent-detection env var is probably set and missing from AGENT_ENV_VARS:\n${stderr.slice(0, 200)}`,
+  ).toBe(mode === 'agent');
+}
+
 /** Pull a field back out of the --dry-run request preview. */
 function sent(output: string, field: string): string | null {
   const m = output.match(new RegExp(`"${field}":\\s*(null|"(?:[^"\\\\]|\\\\.)*")`));
@@ -76,6 +102,7 @@ describe('structured flags still reject raw text', () => {
 
   it('explains what was wanted, echoes what was passed, and shows a worked example', async () => {
     const r = await run([...BASE, '--llm', 'gpt-4']);
+    assertMode(r.stderr, 'human');
     expect(r.stderr).toContain('expected a JSON value');
     expect(r.stderr).toContain('you passed: gpt-4');
     expect(r.stderr).toContain(`--llm '{"key":"value"}'`);
@@ -84,6 +111,7 @@ describe('structured flags still reject raw text', () => {
 
   it('emits a structured envelope in agent mode', async () => {
     const r = await run([...BASE, '--llm', 'gpt-4'], { agentMode: true });
+    assertMode(r.stderr, 'agent');
     const json = JSON.parse(r.stderr.slice(r.stderr.indexOf('{'), r.stderr.lastIndexOf('}') + 1));
     expect(json.error_type).toBe('invalid_flag_value');
     expect(json.error).toContain('invalid value for --llm');