[Graphite MQ] Draft PR GROUP:spec_accf3e (PRs 39)

internal/client/diagnostics.go

@@ -14,6 +14,7 @@ import (
 	"github.com/spf13/cobra"
 
 	"github.com/voiceflow/cli/internal/flagutil"
+	"github.com/voiceflow/cli/internal/output"
 )
 
 // maxBodyPreview is the maximum number of bytes to show in body previews.
@@ -244,10 +245,17 @@ func (c *DryRunClient) Do(req *http.Request) (*http.Response, error) {
 	}
 	fmt.Fprintf(c.Stderr, "[DRY-RUN] Network call skipped.\n")
 
+	// The marker tells output.Error that an SDK error about this response
+	// (operations that succeed only with 201 reject a 200) is not an API
+	// error. See internal/output/dryrun.go.
+	header := http.Header{}
+	header.Set("Content-Type", "application/json")
+	header.Set(output.DryRunResponseHeader, "true")
+
 	return &http.Response{
 		StatusCode: http.StatusOK,
 		Status:     "200 OK",
-		Header:     http.Header{"Content-Type": []string{"application/json"}},
+		Header:     header,
 		Body:       io.NopCloser(bytes.NewReader([]byte("{}"))),
 		Request:    req,
 	}, nil

internal/client/diagnostics_test.go

@@ -0,0 +1,29 @@
+package client
+
+import (
+	"bytes"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"github.com/voiceflow/cli/internal/output"
+)
+
+// output.Error recognizes the stand-in response by this marker, so a dry run
+// of an operation that succeeds only with 201 does not report an API error.
+func TestDryRunClientMarksItsStandInResponse(t *testing.T) {
+	var stderr bytes.Buffer
+	req := httptest.NewRequest(http.MethodPost, "https://api.example.com/v1/stable/project", strings.NewReader(`{"name":"n"}`))
+
+	res, err := (&DryRunClient{Stderr: &stderr}).Do(req)
+	if err != nil {
+		t.Fatalf("Do: %v", err)
+	}
+	if res.Header.Get(output.DryRunResponseHeader) == "" {
+		t.Errorf("the stand-in response is not marked with %s", output.DryRunResponseHeader)
+	}
+	if !strings.Contains(stderr.String(), "[DRY-RUN] Network call skipped.") {
+		t.Errorf("no request preview printed:\n%s", stderr.String())
+	}
+}

internal/output/dryrun.go

@@ -0,0 +1,39 @@
+// This file is not generated by Speakeasy. It stops a dry run from reporting an
+// API error about a response the API never sent.
+//
+// --dry-run swaps the HTTP client for one that prints the request and, instead
+// of sending it, hands the SDK a stand-in response: 200 with an empty JSON
+// body. The SDK accepts only an operation's documented success code, and 28
+// operations succeed with 201 alone: every create, plus environment clone and
+// publish, evaluation run and test run create. Their dry runs printed the
+// preview, then "API Error (HTTP 200): unknown status code returned", and
+// exited 1. The stand-in carries DryRunResponseHeader, and Error does not
+// report an error about it: the preview has printed and nothing was sent.
+// Errors raised before the preview, such as a body that fails to serialize,
+// carry no response and are still reported.
+//
+// The marker counts only while --dry-run is on. A header is something any
+// server reached with --server-url could send, and it must not be able to turn
+// its own error into a silent success. During a dry run nothing is sent, so
+// the stand-in is the only response there can be.
+
+package output
+
+import (
+	"github.com/spf13/cobra"
+
+	"github.com/voiceflow/cli/internal/flagutil"
+)
+
+// DryRunResponseHeader marks the stand-in response that a dry run returns in
+// place of sending the request.
+const DryRunResponseHeader = "X-Vf-Dry-Run"
+
+// isAboutDryRunResponse reports whether err concerns a dry run's stand-in
+// response rather than anything the API returned.
+func isAboutDryRunResponse(cmd *cobra.Command, err error) bool {
+	if isDryRun, _ := flagutil.GetBoolFlag(cmd, "dry-run"); !isDryRun {
+		return false
+	}
+	return extractErrorResponseHeaders(err).Get(DryRunResponseHeader) != ""
+}

internal/output/dryrun_test.go

@@ -0,0 +1,84 @@
+package output
+
+import (
+	"bytes"
+	"errors"
+	"net/http"
+	"testing"
+
+	"github.com/spf13/cobra"
+	"github.com/voiceflow/cli/internal/sdk/models/sdkerrors"
+)
+
+// statusError is what the SDK returns for a response whose status code the
+// operation does not expect, as a 201-only operation does for a dry run's 200.
+func statusError(header http.Header) error {
+	res := &http.Response{StatusCode: http.StatusOK, Status: "200 OK", Header: header}
+	return sdkerrors.NewSDKDefaultError("unknown status code returned", http.StatusOK, "{}", res)
+}
+
+func standInHeader() http.Header {
+	header := http.Header{}
+	header.Set("Content-Type", "application/json")
+	header.Set(DryRunResponseHeader, "true")
+	return header
+}
+
+// reportError runs Error, with or without --dry-run, and returns what it
+// returned and printed.
+func reportError(t *testing.T, err error, isDryRun bool) (error, string) {
+	t.Helper()
+	var stderr bytes.Buffer
+	cmd := &cobra.Command{Use: "vf"}
+	cmd.Flags().Bool("dry-run", false, "")
+	if isDryRun {
+		if setErr := cmd.Flags().Set("dry-run", "true"); setErr != nil {
+			t.Fatal(setErr)
+		}
+	}
+	cmd.SetErr(&stderr)
+	return Error(cmd, err), stderr.String()
+}
+
+func TestErrorIgnoresTheDryRunStandInResponse(t *testing.T) {
+	for name, isAgentMode := range map[string]bool{"human mode": false, "agent mode": true} {
+		t.Run(name, func(t *testing.T) {
+			ResetAgentMode()
+			t.Cleanup(ResetAgentMode)
+			if isAgentMode {
+				t.Setenv("FORCE_AGENT_MODE", "1")
+				InitAgentMode(&cobra.Command{Use: "vf"})
+			}
+
+			got, printed := reportError(t, statusError(standInHeader()), true)
+			if got != nil || printed != "" {
+				t.Fatalf("Error = %v, printed %q; want nil and nothing printed", got, printed)
+			}
+		})
+	}
+}
+
+func TestErrorStillReportsEveryOtherError(t *testing.T) {
+	ResetAgentMode()
+	t.Cleanup(ResetAgentMode)
+
+	cases := []struct {
+		name     string
+		err      error
+		isDryRun bool
+	}{
+		{"a response the API sent", statusError(http.Header{"Content-Type": {"application/json"}}), true},
+		{"an error before the preview", errors.New("error serializing request body: boom"), true},
+		// The marker is a header any server could send; outside a dry run it
+		// must not turn that server's error into a success.
+		{"a marked response without --dry-run", statusError(standInHeader()), false},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			got, printed := reportError(t, tc.err, tc.isDryRun)
+			if got == nil || printed == "" {
+				t.Fatalf("Error = %v, printed %q; want the error returned and reported", got, printed)
+			}
+		})
+	}
+}

internal/output/output.go

@@ -279,6 +279,10 @@ func Error(cmd *cobra.Command, err error) error {
 	if err == nil {
 		return nil
 	}
+	// A dry run's stand-in response is not an API error. See dryrun.go.
+	if isAboutDryRunResponse(cmd, err) {
+		return nil
+	}
 
 	format := resolveOutputFormat(cmd)
 	jqExpr, _ := flagutil.GetStringFlag(cmd, "jq")

test/dry-run.test.ts

@@ -0,0 +1,116 @@
+// Tests for --dry-run (internal/client/diagnostics.go, internal/output/dryrun.go):
+// it prints the request it would send and exits 0, whatever status code the
+// real operation succeeds with. Nothing reaches the network.
+//
+// A dry run hands the SDK a stand-in 200 instead of sending the request, and
+// the SDK accepts only an operation's documented success code. The 28
+// operations that succeed only with 201 (every create, plus environment clone
+// and publish, evaluation run and test run create) used to print the preview,
+// then "API Error (HTTP 200): unknown status code returned", and exit 1.
+//
+// Requires: go build -o vf ./cmd/vf
+
+import { execa } from 'execa';
+import * as fs from 'node:fs';
+import * as http from 'node:http';
+import type { AddressInfo } from 'node:net';
+import * as os from 'node:os';
+import * as path from 'node:path';
+import { afterAll, beforeAll, describe, expect, it } from 'vitest';
+
+const VF = path.resolve(__dirname, '..', 'vf');
+
+// Every variable that puts the CLI into agent mode. Mirrors the list in
+// internal/output/agentmode.go; a stray one on the host would flip the renderer.
+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',
+];
+
+let home: string;
+let server: http.Server;
+let serverURL: string;
+
+beforeAll(async () => {
+  // vf keeps credentials under HOME; an empty one keeps the developer's out.
+  home = fs.mkdtempSync(path.join(os.tmpdir(), 'vf-dry-run-home-'));
+
+  // A real server that answers with the dry-run marker, as any server reached
+  // with --server-url could.
+  server = http.createServer((_req, res) => {
+    res.writeHead(200, { 'content-type': 'application/json', 'x-vf-dry-run': 'true' });
+    res.end('{}');
+  });
+  await new Promise<void>((resolve) => server.listen(0, '127.0.0.1', resolve));
+  serverURL = `http://127.0.0.1:${(server.address() as AddressInfo).port}`;
+});
+
+afterAll(async () => {
+  await new Promise<void>((resolve) => server.close(() => resolve()));
+  fs.rmSync(home, { recursive: true, force: true });
+});
+
+function vf(args: string[], opts: { agentMode?: boolean } = {}) {
+  const env: Record<string, string | undefined> = Object.fromEntries(AGENT_ENV_VARS.map((name) => [name, undefined]));
+  env.HOME = home;
+  if (opts.agentMode) env.CLAUDECODE = '1';
+  return execa({ reject: false, timeout: 20_000, stdin: 'ignore', env, extendEnv: true })(VF, [...args, '--token', 'vfp_x']);
+}
+
+const dryRun = (args: string[], opts: { agentMode?: boolean } = {}) => vf([...args, '--dry-run'], opts);
+
+const PREVIEWED = '[DRY-RUN] Network call skipped.';
+
+// Operations that succeed only with 201, and one that succeeds with 200, which
+// always worked.
+const COMMANDS: Array<[name: string, args: string[]]> = [
+  ['project create', ['project', 'create', '--name', 'n', '--type', 'webchat', '--workspace-id', 'w']],
+  ['environment publish', ['environment', 'publish', '--project-id', 'p', '--environment-alias', 'main', '--name', 'v1']],
+  ['agent update', ['agent', 'update', '--project-id', 'p', '--environment-alias', 'main', '--prompt', 'x']],
+];
+
+describe('--dry-run', () => {
+  for (const [name, args] of COMMANDS) {
+    it(`previews vf ${name} and exits 0`, async () => {
+      const result = await dryRun(args);
+
+      expect(result.exitCode, result.stderr).toBe(0);
+      expect(result.stderr).toContain(PREVIEWED);
+      expect(result.stderr).not.toContain('API Error');
+      expect(result.stderr).not.toContain('unknown status code');
+    });
+  }
+
+  it('reports no error in agent mode or with --output-format json either', async () => {
+    const [, createProject] = COMMANDS[0]!;
+    for (const result of [
+      await dryRun(createProject, { agentMode: true }),
+      await dryRun([...createProject, '--output-format', 'json']),
+    ]) {
+      expect(result.exitCode, result.stderr).toBe(0);
+      expect(result.stderr).toContain(PREVIEWED);
+      expect(result.stderr).not.toContain('"error');
+    }
+  });
+
+  // The marker is only honoured during a dry run. Otherwise a server could send
+  // it on an error response and turn the error into a silent success.
+  it('still reports an unexpected response that carries the marker without --dry-run', async () => {
+    const [, createProject] = COMMANDS[0]!;
+    const result = await vf([...createProject, '--server-url', serverURL]);
+
+    expect(result.exitCode, result.stderr).toBe(1);
+    expect(result.stderr).toContain('unknown status code');
+  });
+
+  // The rule must not hide real failures: a request that cannot be built fails
+  // before anything is previewed, and is still an error.
+  it('still fails when the request cannot be built', async () => {
+    const result = await dryRun(['tool', 'create', '--project-id', 'p', '--environment-alias', 'main']);
+
+    expect(result.exitCode).toBe(1);
+    expect(result.stderr).not.toContain(PREVIEWED);
+    expect(result.stderr).toContain('error serializing request body');
+  });
+});