From 8dae3beaea8951061d98db3b7ec1c391550c8205 Mon Sep 17 00:00:00 2001 From: BR <51544548+Bradenream@users.noreply.github.com> Date: Tue, 25 Aug 2026 20:30:11 -0400 Subject: [PATCH 1/4] =?UTF-8?q?feat:=20errors=20that=20teach=20=E2=80=94?= =?UTF-8?q?=20every=20failure=20names=20its=20own=20fix?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For both humans and AI coding agents, the error text is the documentation they actually read. Three request-lifecycle hooks (all in the non-generated hooks extension point): - No token configured: fail before the network call with the exact fix (Settings → Access tokens, export VF_TOKEN=vfp_...) and a docs_url. --dry-run stays usable without credentials. - Malformed token: warn once on stderr for legacy VF.DM. Dialog Manager keys and whitespace-wrapped tokens, without blocking the request. - 4XX responses: inject actionable hints (401 token-expiry renewal path, 403 acts-as-you semantics, 404 verified 'vf list' commands, 429 Retry-After, 400/422 --dry-run guidance) and a docs_url derived from the operation's verified docs command-group page. Keys already in the body — including future server-provided hints — are never overwritten. Covered by hermetic tests against a local mock API. --- .../sdk/sdkinternal/hooks/registration.go | 4 + internal/sdk/sdkinternal/hooks/teach.go | 282 ++++++++++++++++++ test/errors-teach.test.ts | 112 +++++++ 3 files changed, 398 insertions(+) create mode 100644 internal/sdk/sdkinternal/hooks/teach.go create mode 100644 test/errors-teach.test.ts diff --git a/internal/sdk/sdkinternal/hooks/registration.go b/internal/sdk/sdkinternal/hooks/registration.go index 4eb3bcc..c4c0335 100644 --- a/internal/sdk/sdkinternal/hooks/registration.go +++ b/internal/sdk/sdkinternal/hooks/registration.go @@ -4,4 +4,8 @@ package hooks // Speakeasy — add any additional hooks here. func initHooks(h *Hooks) { h.registerBeforeRequestHook(&clientTypeHook{}) + + teach := &teachHook{} + h.registerBeforeRequestHook(teach) + h.registerAfterErrorHook(teach) } diff --git a/internal/sdk/sdkinternal/hooks/teach.go b/internal/sdk/sdkinternal/hooks/teach.go new file mode 100644 index 0000000..655a15f --- /dev/null +++ b/internal/sdk/sdkinternal/hooks/teach.go @@ -0,0 +1,282 @@ +// This file is not generated by Speakeasy — it implements "errors that teach": +// every failure the CLI surfaces should name its own fix, because for both +// humans and AI coding agents the error text is the documentation they read. +// +// Three behaviors, all request-lifecycle hooks (see registration.go): +// +// - No credential configured -> fail before the network call with setup +// guidance (the one moment that genuinely needs a human: creating a token). +// - Malformed credential -> warn once on stderr (legacy Dialog Manager +// keys, stray whitespace) but still send the request. +// - 4XX/5XX responses -> inject actionable "hints" and a "docs_url" +// into the JSON error body. internal/output preserves body-provided keys +// in both agent-mode envelopes and pretty output, and server-provided +// hints always win — injection only fills keys the body does not have. + +package hooks + +import ( + "bytes" + "encoding/json" + "fmt" + "io" + "net/http" + "os" + "strconv" + "strings" + "sync" +) + +const ( + authDocsURL = "https://www.voiceflow.com/docs/api-reference/authentication" + docsCommandBase = "https://www.voiceflow.com/docs/cli/commands" + tokenEnvVar = "VF_TOKEN" +) + +type teachHook struct { + warnOnce sync.Once +} + +var _ beforeRequestHook = (*teachHook)(nil) +var _ afterErrorHook = (*teachHook)(nil) + +// authSetupError carries a structured JSON body so internal/output renders it +// like an API error envelope (Body is extracted via reflection there). +type authSetupError struct { + Body string + message string +} + +func (e *authSetupError) Error() string { return e.message } + +// BeforeRequest fails fast when no access token is configured, and warns once +// when the configured token is recognizably malformed. Runs after the SDK +// populates security, so the Authorization header reflects every source +// (--token flag, VF_TOKEN env, keyring, config file). +func (h *teachHook) BeforeRequest(_ BeforeRequestContext, req *http.Request) (*http.Request, error) { + authorization := req.Header.Get("Authorization") + // With no token configured the SDK still writes "Authorization: Bearer " + // (empty credential) — strip the scheme to see whether a credential exists. + credential := strings.TrimSpace(strings.TrimPrefix(strings.TrimSpace(authorization), "Bearer")) + + if credential == "" { + // --dry-run must keep working without credentials: it is a + // request-inspection tool and never contacts the API. + if dryRunRequested() { + return req, nil + } + body, _ := json.Marshal(map[string]interface{}{ + "error": "no access token configured", + "error_type": "authentication_error", + "message": "No access token configured — the request was not sent.", + "hints": []string{ + "Create a personal access token in Voiceflow under Settings → Access tokens (tokens start with vfp_)", + fmt.Sprintf("Then run: export %s=vfp_... (every command also accepts --token)", tokenEnvVar), + "Verify what is configured, without a network call: vf whoami", + }, + "docs_url": authDocsURL, + }) + return req, &authSetupError{ + Body: string(body), + message: fmt.Sprintf("no access token configured — create one under Settings → Access tokens, then export %s=vfp_... (%s)", tokenEnvVar, authDocsURL), + } + } + + token := strings.TrimPrefix(authorization, "Bearer ") + switch { + case strings.HasPrefix(credential, "VF.DM."): + h.warnOnce.Do(func() { + fmt.Fprintf(os.Stderr, "Warning: the configured token looks like a legacy Dialog Manager API key (VF.DM....). The CLI needs a personal access token (vfp_...) — create one under Settings → Access tokens: %s\n", authDocsURL) + }) + case strings.TrimSpace(token) != token: + h.warnOnce.Do(func() { + fmt.Fprintln(os.Stderr, "Warning: the configured token has leading or trailing whitespace — the API will likely reject it.") + }) + } + + return req, nil +} + +// AfterError injects hints and a docs_url into JSON error bodies. Keys already +// present in the body (for example server-provided hints) are never replaced. +func (h *teachHook) AfterError(hookCtx AfterErrorContext, res *http.Response, err error) (*http.Response, error) { + if res == nil || res.Body == nil || res.StatusCode < 400 { + return res, err + } + + raw, readErr := io.ReadAll(res.Body) + res.Body.Close() + restore := func(body []byte) { + res.Body = io.NopCloser(bytes.NewReader(body)) + res.ContentLength = int64(len(body)) + res.Header.Set("Content-Length", strconv.Itoa(len(body))) + } + if readErr != nil { + restore(nil) + return res, err + } + + var parsed map[string]interface{} + if json.Unmarshal(raw, &parsed) != nil || parsed == nil { + restore(raw) + return res, err + } + + changed := false + if _, has := parsed["hints"]; !has { + if hints := hintsForStatus(res.StatusCode, res.Header, hookCtx.OperationID); len(hints) > 0 { + parsed["hints"] = hints + changed = true + } + } + if _, has := parsed["docs_url"]; !has { + if docsURL := docsURLForOperation(hookCtx.OperationID); docsURL != "" { + parsed["docs_url"] = docsURL + changed = true + } + } + + if !changed { + restore(raw) + return res, err + } + if enriched, marshalErr := json.Marshal(parsed); marshalErr == nil { + restore(enriched) + } else { + restore(raw) + } + return res, err +} + +// dryRunRequested reports whether --dry-run is on the command line. The hook +// has no cobra command reference, and --dry-run is flag-only (no env or config +// source), so the process arguments are its single source of truth. +func dryRunRequested() bool { + for _, arg := range os.Args[1:] { + if arg == "--dry-run" || arg == "--dry-run=true" || arg == "--dry-run=1" { + return true + } + } + return false +} + +// hintsForStatus returns fix-naming hints for a failed response. Empty for +// statuses where the generic enrichment in internal/output already says +// everything useful (5XX). +func hintsForStatus(statusCode int, headers http.Header, operationID string) []string { + switch statusCode { + case 401: + return []string{ + "Access tokens expire (default 30 days) — this one may have lapsed or been revoked", + fmt.Sprintf("Create a new token in Voiceflow under Settings → Access tokens, then run: export %s=vfp_...", tokenEnvVar), + "See which credential source is being used: vf whoami", + } + case 403: + hints := []string{ + "Your token carries your own account's access — this resource belongs to a workspace or organization your account cannot act on", + } + if hasListCommand(commandGroupForOperation(operationID)) { + hints = append(hints, "List what your account can reach: vf workspace list") + } + return hints + case 404: + hints := []string{"Verify the resource identifier — it may be from another workspace or environment"} + if group := commandGroupForOperation(operationID); hasListCommand(group) { + hints = append(hints, fmt.Sprintf("List available resources: vf %s list", group)) + } + return hints + case 429: + if retryAfter := headers.Get("Retry-After"); retryAfter != "" { + return []string{fmt.Sprintf("Rate limited — retry after %s seconds", retryAfter)} + } + return []string{"Rate limited — retry after a short delay"} + case 400, 422: + return []string{ + "Preview the exact request without sending it: add --dry-run", + "Field paths in details[] refer to the API request body — the matching CLI flag usually shares the last path segment (run the command with --help)", + } + default: + return nil + } +} + +// docsGroupPages is the set of command-group pages that exist under +// https://www.voiceflow.com/docs/cli/commands/ (verified against the docs +// site's index). Operations whose group is not listed fall back to overview. +var docsGroupPages = map[string]bool{ + "agent": true, "analytics": true, "api-tool": true, "conversation": true, + "document": true, "environment": true, "evaluation": true, "function": true, + "knowledge-base": true, "mcp-server": true, "mcp-tool": true, "playbook": true, + "project": true, "tool": true, "transcript": true, "variable": true, "workspace": true, +} + +// listCommandGroups is the set of top-level command groups that actually have +// a `list` subcommand — hints must only name commands that exist. +var listCommandGroups = map[string]bool{ + "api-tool": true, "document": true, "environment": true, "evaluation": true, + "function": true, "mcp-server": true, "mcp-tool": true, "playbook": true, + "project": true, "test": true, "tool": true, "variable": true, "workspace": true, +} + +func hasListCommand(group string) bool { return listCommandGroups[group] } + +// commandGroupForOperation derives the CLI command group from an operation ID +// like "StableAPIToolController_list" -> "api-tool". Sub-resource controllers +// (e.g. StableFunctionPathController) resolve to their parent group by +// trimming trailing segments until a known group matches. +func commandGroupForOperation(operationID string) string { + name, _, found := strings.Cut(operationID, "Controller") + if !found { + return "" + } + name = strings.TrimPrefix(name, "Stable") + kebab := camelToKebab(name) + + for kebab != "" { + if docsGroupPages[kebab] || listCommandGroups[kebab] { + return kebab + } + lastDash := strings.LastIndex(kebab, "-") + if lastDash < 0 { + return "" + } + kebab = kebab[:lastDash] + } + return "" +} + +// docsURLForOperation maps an operation to its command group's documentation +// page, falling back to the commands overview. +func docsURLForOperation(operationID string) string { + if group := commandGroupForOperation(operationID); group != "" && docsGroupPages[group] { + return docsCommandBase + "/" + group + } + return docsCommandBase + "/overview" +} + +// camelToKebab converts CamelCase with initialisms to kebab-case: +// "APITool" -> "api-tool", "MCPServer" -> "mcp-server", "Workspace" -> "workspace". +func camelToKebab(name string) string { + var out strings.Builder + runes := []rune(name) + for i, r := range runes { + if i > 0 && isUpper(r) { + prevLower := !isUpper(runes[i-1]) + nextLower := i+1 < len(runes) && !isUpper(runes[i+1]) + if prevLower || nextLower { + out.WriteByte('-') + } + } + out.WriteRune(toLower(r)) + } + return out.String() +} + +func isUpper(r rune) bool { return r >= 'A' && r <= 'Z' } + +func toLower(r rune) rune { + if isUpper(r) { + return r + ('a' - 'A') + } + return r +} diff --git a/test/errors-teach.test.ts b/test/errors-teach.test.ts new file mode 100644 index 0000000..1eaf5b1 --- /dev/null +++ b/test/errors-teach.test.ts @@ -0,0 +1,112 @@ +// Hermetic tests for the "errors that teach" hooks (internal/sdk/sdkinternal/ +// hooks/teach.go): no-token preflight, malformed-token warnings, and 4XX hint +// + docs_url injection. Uses a local mock API — no live credentials. +// +// Requires the CLI binary at the repo root: go build -o vf ./cmd/vf + +import { execa } from 'execa'; +import { createServer, type Server } from 'node:http'; +import { AddressInfo } from 'node:net'; +import * as path from 'node:path'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; + +const VF = path.resolve(__dirname, '..', 'vf'); +const AGENT_ENV = { CLAUDE_CODE: '1', VF_TOKEN: '' } as const; + +const $vf = (args: string[], env: Record = {}) => + execa({ reject: false, env: { ...AGENT_ENV, ...env }, stdin: 'ignore' })(VF, args); + +/** stderr of agent-mode errors is a JSON envelope followed by plain lines. */ +function parseEnvelope(stderr: string): Record { + const start = stderr.indexOf('{'); + expect(start, `no JSON envelope in stderr:\n${stderr}`).toBeGreaterThanOrEqual(0); + let depth = 0; + for (let i = start; i < stderr.length; i += 1) { + if (stderr[i] === '{') depth += 1; + if (stderr[i] === '}') depth -= 1; + if (depth === 0) return JSON.parse(stderr.slice(start, i + 1)); + } + throw new Error(`unterminated JSON envelope in stderr:\n${stderr}`); +} + +let mock: Server; +let mockURL: string; + +beforeAll(async () => { + mock = createServer((req, res) => { + const reply = (code: number, body: object, headers: Record = {}) => { + const data = JSON.stringify(body); + res.writeHead(code, { 'Content-Type': 'application/json', ...headers }); + res.end(data); + }; + if (req.url?.includes('/project/')) return reply(404, { statusCode: 404, message: 'Project not found' }); + if (req.url?.includes('/workspace')) return reply(429, { statusCode: 429, message: 'Too many requests' }, { 'Retry-After': '17' }); + return reply(401, { statusCode: 401, message: 'Unauthorized' }); + }); + await new Promise((resolve) => mock.listen(0, '127.0.0.1', resolve)); + mockURL = `http://127.0.0.1:${(mock.address() as AddressInfo).port}`; +}); + +afterAll(() => { + mock.close(); +}); + +describe('no-token preflight', () => { + it('fails before any network call with setup guidance', async () => { + const result = await $vf(['workspace', 'list', '--server-url', 'http://127.0.0.1:1']); // unroutable — must not be contacted + expect(result.exitCode).toBe(1); + + const envelope = parseEnvelope(result.stderr); + expect(envelope.error_type).toBe('authentication_error'); + expect(envelope.message).toContain('request was not sent'); + expect(envelope.docs_url).toBe('https://www.voiceflow.com/docs/api-reference/authentication'); + expect(JSON.stringify(envelope.hints)).toContain('export VF_TOKEN=vfp_'); + }); + + it('does not block --dry-run', async () => { + const result = await $vf(['workspace', 'list', '--dry-run']); + expect(result.exitCode).toBe(0); + expect(result.stderr).toContain('[DRY-RUN]'); + expect(result.stderr).toContain('Network call skipped'); + }); +}); + +describe('malformed-token warnings', () => { + it('warns on a legacy Dialog Manager key but still sends the request', async () => { + const result = await $vf(['workspace', 'list', '--token', 'VF.DM.legacy', '--server-url', mockURL]); + expect(result.exitCode).toBe(1); // mock replies 429 — request WAS sent + expect(result.stderr).toContain('legacy Dialog Manager API key'); + expect(result.stderr).toContain('vfp_'); + }); + + it('warns on surrounding whitespace', async () => { + const result = await $vf(['workspace', 'list', '--token', 'vfp_abc ', '--server-url', mockURL]); + expect(result.stderr).toContain('whitespace'); + }); +}); + +describe('error hint injection', () => { + it('injects a list-command hint and docs_url on 404', async () => { + const result = await $vf(['project', 'get', '--project-id', 'missing', '--token', 'vfp_x', '--server-url', mockURL]); + expect(result.exitCode).toBe(1); + + const envelope = parseEnvelope(result.stderr); + expect(envelope.error_type).toBe('not_found'); + expect(envelope.docs_url).toBe('https://www.voiceflow.com/docs/cli/commands/project'); + expect(JSON.stringify(envelope.hints)).toContain('vf project list'); + }); + + it('surfaces Retry-After on 429', async () => { + const result = await $vf(['workspace', 'list', '--token', 'vfp_x', '--server-url', mockURL]); + const envelope = parseEnvelope(result.stderr); + expect(JSON.stringify(envelope.hints)).toContain('17 seconds'); + }); + + it('teaches token expiry and renewal on 401', async () => { + const result = await $vf(['playbook', 'list', '--project-id', 'p', '--environment-alias', 'main', '--token', 'vfp_expired', '--server-url', mockURL]); + const envelope = parseEnvelope(result.stderr); + expect(envelope.error_type).toBe('authentication_error'); + expect(JSON.stringify(envelope.hints)).toContain('expire'); + expect(JSON.stringify(envelope.hints)).toContain('export VF_TOKEN=vfp_'); + }); +}); From b7602c94fe106cec88d7d1d552eebceaa0a35677 Mon Sep 17 00:00:00 2001 From: BR <51544548+Bradenream@users.noreply.github.com> Date: Tue, 25 Aug 2026 21:29:35 -0400 Subject: [PATCH 2/4] fix: enforce required params locally on params-only commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BuildRequest already validates required flags, but its nullable-body relaxation also fired for params-only commands (bodyFieldPath and bodyFlagName both empty — get/delete with only path or query params), stripping Required from every field. Requests left with empty required params and failed server-side with field paths ('resources[0].id') no CLI user or coding agent can map back to a flag. Restrict the relaxation to commands that actually register a body flag. 'vf project get' now fails locally, before any network call, with 'missing required flag: --project-id' — the same message shape cobra produces for enforced query params. Whole-body input (--body/stdin) still satisfies required body fields exactly as before. --- internal/flagutil/metadata.go | 7 ++++++- test/errors-teach.test.ts | 21 +++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/internal/flagutil/metadata.go b/internal/flagutil/metadata.go index a84b18b..8e36516 100644 --- a/internal/flagutil/metadata.go +++ b/internal/flagutil/metadata.go @@ -282,7 +282,12 @@ func BuildRequest[T any](cmd *cobra.Command, meta []FlagMeta, bodyFieldPath stri // provided via --body/stdin, check if any individual flags were changed. // If not, the user didn't attempt to provide a body at all — relax required // checks so nullable/optional bodies work without erroring on inner required fields. - if !bodyPrePopulated && bodyFieldPath == "" { + // Only when the command actually has a body flag (bodyFlagName != ""): a + // params-only command (get/delete — no --body registered) has no body to + // relax for, and relaxing would let requests leave with required path or + // query params empty, failing server-side with field paths no CLI user + // can map back to a flag. + if !bodyPrePopulated && bodyFieldPath == "" && bodyFlagName != "" { anyChanged := false for _, m := range meta { if FlagChanged(cmd, m.FlagName) { diff --git a/test/errors-teach.test.ts b/test/errors-teach.test.ts index 1eaf5b1..d3e530c 100644 --- a/test/errors-teach.test.ts +++ b/test/errors-teach.test.ts @@ -85,6 +85,27 @@ describe('malformed-token warnings', () => { }); }); +describe('required-flag preflight', () => { + it('fails locally with the exact flag name when a required path param is missing', async () => { + // Unroutable server: proves the request never leaves the machine. + const result = await $vf(['project', 'get', '--token', 'vfp_x', '--server-url', 'http://127.0.0.1:1']); + expect(result.exitCode).toBe(1); + expect(result.stderr).toContain('missing required flag: --project-id'); + expect(result.stderr).not.toContain('statusCode'); // no server round-trip happened + }); + + it('still accepts whole-body input in place of individual required flags', async () => { + const result = await $vf([ + 'project', 'create', + '--body', '{"name":"x","workspaceID":"w","type":"webchat"}', + '--token', 'vfp_x', '--server-url', mockURL, + ]); + // The mock replies 401 — reaching it proves the body path was not blocked locally. + const envelope = parseEnvelope(result.stderr); + expect(envelope.statusCode).toBe(401); + }); +}); + describe('error hint injection', () => { it('injects a list-command hint and docs_url on 404', async () => { const result = await $vf(['project', 'get', '--project-id', 'missing', '--token', 'vfp_x', '--server-url', mockURL]); From 4eb136bd995c0245453b3674aa454a00b8b9955f Mon Sep 17 00:00:00 2001 From: BR <51544548+Bradenream@users.noreply.github.com> Date: Tue, 25 Aug 2026 22:34:48 -0400 Subject: [PATCH 3/4] fix: address adversarial-review findings on the error hooks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Agent-mode detection: Claude Code actually sets CLAUDECODE (no underscore) — verified in a live session. Without it in the list the flagship coding agent never triggered agent mode at all. - dryRunRequested now accepts every bool spelling cobra does (--dry-run=True/T/1/...), instead of string-matching two of them. - Piped stdin no longer bypasses required-param enforcement on params-only commands: 'echo {} | vf project get' used to re-relax required path params and send 'GET /project/' with an empty segment. - 429 hint echoes Retry-After verbatim (it may be an HTTP-date, not delta-seconds). --- internal/flagutil/metadata.go | 7 ++++-- internal/output/agentmode.go | 4 ++++ internal/sdk/sdkinternal/hooks/teach.go | 11 +++++++-- test/errors-teach.test.ts | 31 ++++++++++++++++++++----- 4 files changed, 43 insertions(+), 10 deletions(-) diff --git a/internal/flagutil/metadata.go b/internal/flagutil/metadata.go index 8e36516..348d8ff 100644 --- a/internal/flagutil/metadata.go +++ b/internal/flagutil/metadata.go @@ -247,8 +247,11 @@ func BuildRequest[T any](cmd *cobra.Command, meta []FlagMeta, bodyFieldPath stri } } - // Priority 2: stdin - if !bodyPrePopulated && HasStdinInput(cmd) { + // Priority 2: stdin. Params-only commands (no body field and no --body + // flag) have no body for stdin to fill — consuming piped JSON there would + // both surprise pipelines and re-relax required path/query params via the + // bodyPrePopulated relaxation below. + if !bodyPrePopulated && (bodyFieldPath != "" || bodyFlagName != "") && HasStdinInput(cmd) { stdinData, err := io.ReadAll(cmd.InOrStdin()) if err != nil { return nil, fmt.Errorf("failed to read stdin: %w", err) diff --git a/internal/output/agentmode.go b/internal/output/agentmode.go index 66f28b5..8d53fd9 100644 --- a/internal/output/agentmode.go +++ b/internal/output/agentmode.go @@ -22,6 +22,10 @@ var ( // agentEnvVars lists environment variables set by known AI coding agents. var agentEnvVars = []string{ + // Claude Code actually sets CLAUDECODE=1 (no underscore) — verified in a + // live session. CLAUDE_CODE is kept for compatibility with the generated + // list, but without CLAUDECODE the flagship agent never triggers agent mode. + "CLAUDECODE", "CLAUDE_CODE", "CURSOR_AGENT", "CODEX", diff --git a/internal/sdk/sdkinternal/hooks/teach.go b/internal/sdk/sdkinternal/hooks/teach.go index 655a15f..5706884 100644 --- a/internal/sdk/sdkinternal/hooks/teach.go +++ b/internal/sdk/sdkinternal/hooks/teach.go @@ -153,9 +153,15 @@ func (h *teachHook) AfterError(hookCtx AfterErrorContext, res *http.Response, er // source), so the process arguments are its single source of truth. func dryRunRequested() bool { for _, arg := range os.Args[1:] { - if arg == "--dry-run" || arg == "--dry-run=true" || arg == "--dry-run=1" { + if arg == "--dry-run" { return true } + if value, found := strings.CutPrefix(arg, "--dry-run="); found { + // Accept every spelling cobra's bool parsing accepts (True, T, 1, ...). + if parsed, err := strconv.ParseBool(value); err == nil { + return parsed + } + } } return false } @@ -187,7 +193,8 @@ func hintsForStatus(statusCode int, headers http.Header, operationID string) []s return hints case 429: if retryAfter := headers.Get("Retry-After"); retryAfter != "" { - return []string{fmt.Sprintf("Rate limited — retry after %s seconds", retryAfter)} + // Retry-After may be delta-seconds or an HTTP-date; echo it verbatim. + return []string{fmt.Sprintf("Rate limited — Retry-After: %s", retryAfter)} } return []string{"Rate limited — retry after a short delay"} case 400, 422: diff --git a/test/errors-teach.test.ts b/test/errors-teach.test.ts index d3e530c..f218484 100644 --- a/test/errors-teach.test.ts +++ b/test/errors-teach.test.ts @@ -63,11 +63,21 @@ describe('no-token preflight', () => { expect(JSON.stringify(envelope.hints)).toContain('export VF_TOKEN=vfp_'); }); - it('does not block --dry-run', async () => { - const result = await $vf(['workspace', 'list', '--dry-run']); - expect(result.exitCode).toBe(0); - expect(result.stderr).toContain('[DRY-RUN]'); - expect(result.stderr).toContain('Network call skipped'); + it('does not block --dry-run, in every bool spelling cobra accepts', async () => { + for (const spelling of ['--dry-run', '--dry-run=true', '--dry-run=True', '--dry-run=T', '--dry-run=1']) { + const result = await $vf(['workspace', 'list', spelling]); + expect(result.exitCode, `${spelling}: ${result.stderr}`).toBe(0); + expect(result.stderr).toContain('[DRY-RUN]'); + } + }); + + it('fires under the env var Claude Code actually sets (CLAUDECODE, no underscore)', async () => { + const result = await execa({ reject: false, env: { CLAUDECODE: '1', CLAUDE_CODE: '', VF_TOKEN: '' }, stdin: 'ignore' })( + VF, ['workspace', 'list', '--server-url', 'http://127.0.0.1:1'], + ); + expect(result.exitCode).toBe(1); + const envelope = parseEnvelope(result.stderr); // agent-mode structured envelope, not pretty output + expect(envelope.error_type).toBe('authentication_error'); }); }); @@ -94,6 +104,15 @@ describe('required-flag preflight', () => { expect(result.stderr).not.toContain('statusCode'); // no server round-trip happened }); + it('is not bypassed by piped stdin on params-only commands', async () => { + const result = await execa({ reject: false, env: { ...AGENT_ENV }, input: '{}' })( + VF, ['project', 'get', '--token', 'vfp_x', '--server-url', 'http://127.0.0.1:1'], + ); + expect(result.exitCode).toBe(1); + expect(result.stderr).toContain('missing required flag: --project-id'); + expect(result.stderr).not.toContain('statusCode'); + }); + it('still accepts whole-body input in place of individual required flags', async () => { const result = await $vf([ 'project', 'create', @@ -120,7 +139,7 @@ describe('error hint injection', () => { it('surfaces Retry-After on 429', async () => { const result = await $vf(['workspace', 'list', '--token', 'vfp_x', '--server-url', mockURL]); const envelope = parseEnvelope(result.stderr); - expect(JSON.stringify(envelope.hints)).toContain('17 seconds'); + expect(JSON.stringify(envelope.hints)).toContain('Retry-After: 17'); }); it('teaches token expiry and renewal on 401', async () => { From 4cf42a00ec244b02b33e56804594462aef8dff41 Mon Sep 17 00:00:00 2001 From: BR <51544548+Bradenream@users.noreply.github.com> Date: Wed, 26 Aug 2026 00:28:38 -0400 Subject: [PATCH 4/4] fix: address Copilot review on the error hooks - io.ReadAll can return bytes alongside an error; restore whatever arrived instead of blanking the body, so a truncated error message still reaches the user. - The 403 hint gated on the operation's own command group but always emitted 'vf workspace list'. Always emit it: workspace list is the command that reveals reachable scope, and it exists for every account. --- internal/sdk/sdkinternal/hooks/teach.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/internal/sdk/sdkinternal/hooks/teach.go b/internal/sdk/sdkinternal/hooks/teach.go index 5706884..26170cf 100644 --- a/internal/sdk/sdkinternal/hooks/teach.go +++ b/internal/sdk/sdkinternal/hooks/teach.go @@ -112,7 +112,10 @@ func (h *teachHook) AfterError(hookCtx AfterErrorContext, res *http.Response, er res.Header.Set("Content-Length", strconv.Itoa(len(body))) } if readErr != nil { - restore(nil) + // io.ReadAll can return bytes alongside an error. Restore whatever + // arrived rather than blanking the body — a truncated error message + // still tells the user more than an empty one. + restore(raw) return res, err } @@ -181,9 +184,9 @@ func hintsForStatus(statusCode int, headers http.Header, operationID string) []s hints := []string{ "Your token carries your own account's access — this resource belongs to a workspace or organization your account cannot act on", } - if hasListCommand(commandGroupForOperation(operationID)) { - hints = append(hints, "List what your account can reach: vf workspace list") - } + // Always name a command that exists: workspace list is the widest thing + // any account can run, and it is what reveals the reachable scope. + hints = append(hints, "List what your account can reach: vf workspace list") return hints case 404: hints := []string{"Verify the resource identifier — it may be from another workspace or environment"}