[Graphite MQ] Draft PR GROUP:spec_90ff7c (PRs 23)

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)
 	}
 }

internal/cli/flagerrors.go

@@ -0,0 +1,49 @@
+// 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: 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() {
+		return err
+	}
+
+	hints := fve.Hints
+	if v := fve.DisplayValue(); v != "" {
+		hints = append([]string{"you passed: " + v}, hints...)
+	}
+
+	return output.AgentModeError(cmd, "invalid_flag_value",
+		"invalid value for --"+fve.Flag+": "+fve.Expected, hints)
+}

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

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, "`", "'")
+}

internal/flagutil/flagerror.go

@@ -0,0 +1,75 @@
+// 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 }
+
+// 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. 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", " ")
+	runes := []rune(s)
+	if len(runes) <= max {
+		return s
+	}
+	return string(runes[:max]) + "… (" + fmt.Sprint(len(runes)) + " chars)"
+}

internal/flagutil/jsonerror.go

@@ -0,0 +1,115 @@
+// This file is not generated by Speakeasy — it explains why a structured JSON
+// flag rejected the value it was given.
+//
+// The failure it replaces named the wrong thing twice over:
+//
+//	vf agent update --llm 'gpt-4'
+//	  invalid value for --llm: error unmarshalling json response body:
+//	  invalid character 'g' looking for beginning of value
+//
+// There is no response body — the request was never sent. And when the input IS
+// valid JSON of the wrong shape, the same message insisted it was not valid
+// JSON, sending the reader to re-check syntax that was never wrong. Meanwhile
+// the decoder's own explanation, the only thing that knows which Go type was
+// expected, was discarded.
+
+package flagutil
+
+import (
+	"encoding/json"
+	"fmt"
+	"reflect"
+	"strings"
+)
+
+// 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 jsonValueError(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"}`
+	case reflect.String:
+		// A nullable string is JSON-typed so that null can be expressed at all,
+		// which is why prose has to arrive JSON-quoted. Nothing said so before;
+		// the error named the first character of the sentence and stopped.
+		return `"your text here"`
+	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
+}

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 jsonValueError(field.Type(), val, m, err) // see jsonerror.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 jsonValueError(field.Type(), val, m, err) // see jsonerror.go
 		}
 		field.Set(target.Elem())
 	}

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
 }

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