diff --git a/internal/cli/exports.go b/internal/cli/exports.go index b09a092..fbdf934 100644 --- a/internal/cli/exports.go +++ b/internal/cli/exports.go @@ -69,6 +69,12 @@ func NewCloudClient() (*ags.Client, error) { // disabled by global flags. func NonInteractive() bool { return nonInteractive } +// IsJSONOutput reports whether the resolved output format is JSON or NDJSON. +func IsJSONOutput() bool { return isJSON() || isNDJSON() } + +// NoColor reports whether ANSI color/escape output is disabled. +func NoColor() bool { return noColor } + // CfgFile returns the config file path supplied on the command line. func CfgFile() string { return cfgFile } diff --git a/internal/commands/instance/create/command.go b/internal/commands/instance/create/command.go index 1ba282a..dcf76a1 100644 --- a/internal/commands/instance/create/command.go +++ b/internal/commands/instance/create/command.go @@ -7,9 +7,11 @@ import ( "strings" "github.com/TencentCloudAgentRuntime/ags-cli/internal/apicli" + "github.com/TencentCloudAgentRuntime/ags-cli/internal/cli" "github.com/TencentCloudAgentRuntime/ags-cli/internal/command" instanceview "github.com/TencentCloudAgentRuntime/ags-cli/internal/commands/instance/internal/instanceview" "github.com/TencentCloudAgentRuntime/ags-cli/internal/output" + "github.com/TencentCloudAgentRuntime/ags-cli/internal/progress" ags "github.com/tencentcloud/tencentcloud-sdk-go/tencentcloud/ags/v20250920" ) @@ -36,6 +38,7 @@ func Module() command.Module { Source: "mixed-api", }, Build: func(deps command.Deps) (command.Runtime, error) { + deps = deps.WithDefaults() builder := apicli.NewRequestBuilder(api) executor := apicli.NewExecutor(api, deps.ControlPlane) return command.Runtime{Handler: command.HandlerFunc(func(ctx context.Context, req command.Request) (*command.Result, error) { @@ -48,15 +51,24 @@ func Module() command.Module { if err != nil { return nil, err } + + // Show spinner for interactive text mode only. + sp := progress.NewForCLI(deps.IO.ErrOut, cli.IsJSONOutput(), cli.NonInteractive(), deps.IO.IsStderrTTY(), cli.NoColor()) + sp.Start("Creating instance...") + result, err := executor.Execute(ctx, apiReq) if err != nil { + sp.Stop("✗", "Failed to create instance") return nil, err } response, ok := result.Data.(*ags.StartSandboxInstanceResponseParams) if !ok { + // Response type mismatch — we cannot confirm creation succeeded. + sp.Cleanup() return result, nil } if response.Instance == nil { + sp.Stop("✗", "Failed to create instance") return nil, output.NewCLIError(&output.Failure{ Code: "INTERNAL_ERROR", Kind: output.KindGenericError, @@ -64,6 +76,7 @@ func Module() command.Module { Hint: "Rerun with --debug. If the issue persists, inspect the control-plane response.", }) } + sp.Stop("✓", "Instance created") return instanceCreateResult(response, result), nil })}, nil }, diff --git a/internal/commands/instance/create/command_test.go b/internal/commands/instance/create/command_test.go index bf69e1e..e577f2f 100644 --- a/internal/commands/instance/create/command_test.go +++ b/internal/commands/instance/create/command_test.go @@ -3,14 +3,18 @@ package create import ( "bytes" "context" + "errors" "strings" "testing" "github.com/TencentCloudAgentRuntime/ags-cli/internal/command" + "github.com/TencentCloudAgentRuntime/ags-cli/internal/iostreams" "github.com/TencentCloudAgentRuntime/ags-cli/internal/output" ags "github.com/tencentcloud/tencentcloud-sdk-go/tencentcloud/ags/v20250920" ) +// fakeMixedControlPlane implements apicli.ControlPlane for testing the full +// create flow (parameter validation → request build → executor → response render). type fakeMixedControlPlane struct { action string request map[string]any @@ -23,12 +27,91 @@ func (f *fakeMixedControlPlane) Call(_ context.Context, action string, request m if f.resp != nil { return f.resp, nil } + return map[string]any{"ok": true}, nil +} + +// allInstanceCreateFlags returns a complete flag set mimicking what Cobra registers +// for `agr instance create`. Unset optional flags have Changed=false. +func allInstanceCreateFlags() map[string]command.FlagValue { + return map[string]command.FlagValue{ + "tool-id": {Name: "tool-id", Type: command.FlagString}, + "tool-name": {Name: "tool-name", Type: command.FlagString}, + "timeout": {Name: "timeout", Type: command.FlagString}, + "client-token": {Name: "client-token", Type: command.FlagString}, + "mount-options": {Name: "mount-options", Type: command.FlagString}, + "custom-configuration": {Name: "custom-configuration", Type: command.FlagString}, + "auth-mode": {Name: "auth-mode", Type: command.FlagString}, + "metadata": {Name: "metadata", Type: command.FlagString}, + "request": {Name: "request", Type: command.FlagString}, + } +} + +// withChanged returns a copy of flags with the named flag set to the given value. +func withChanged(flags map[string]command.FlagValue, name, value string) map[string]command.FlagValue { + out := map[string]command.FlagValue{} + for k, v := range flags { + out[k] = v + } + f := out[name] + f.String = value + f.Changed = true + out[name] = f + return out +} + +func TestModuleBuildsWithoutError(t *testing.T) { + module := Module() + if module.Descriptor.Spec.ID == "" { + t.Fatalf("expected non-empty spec ID") + } + if module.Build == nil { + t.Fatalf("expected non-nil Build func") + } +} + +func TestModuleRequiresToolSelection(t *testing.T) { + module := Module() + runtime, err := module.Build(command.Deps{ControlPlane: &fakeMixedControlPlane{}}) + if err != nil { + t.Fatalf("Build: %v", err) + } + // No --tool-name or --tool-id set → should fail. + _, err = runtime.Handler.Run(context.Background(), command.Request{ + Flags: allInstanceCreateFlags(), + }) + if err == nil { + t.Fatalf("expected missing tool selection error") + } + if !strings.Contains(err.Error(), "tool-name") { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestModuleRejectsConflictingToolFlags(t *testing.T) { + module := Module() + runtime, err := module.Build(command.Deps{ControlPlane: &fakeMixedControlPlane{}}) + if err != nil { + t.Fatalf("Build: %v", err) + } + // Both --tool-name and --tool-id set → should fail. + flags := withChanged(allInstanceCreateFlags(), "tool-name", "my-tool") + flags = withChanged(flags, "tool-id", "sdt-123") + _, err = runtime.Handler.Run(context.Background(), command.Request{Flags: flags}) + if err == nil { + t.Fatalf("expected conflicting flags error") + } + if !strings.Contains(err.Error(), "both") { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestModuleCreatesInstanceAndRendersText(t *testing.T) { id := "ins-created" - name, _ := request["ToolName"].(string) + name := "tool-name" status := "RUNNING" created := "2026-05-21T10:00:00Z" mountName := "workspace" - return &ags.StartSandboxInstanceResponseParams{ + cp := &fakeMixedControlPlane{resp: &ags.StartSandboxInstanceResponseParams{ Instance: &ags.SandboxInstance{ InstanceId: &id, ToolName: &name, @@ -36,106 +119,278 @@ func (f *fakeMixedControlPlane) Call(_ context.Context, action string, request m CreateTime: &created, MountOptions: []*ags.MountOption{{Name: &mountName}}, }, - }, nil + }} + runtime, err := Module().Build(command.Deps{ControlPlane: cp}) + if err != nil { + t.Fatalf("Build returned error: %v", err) + } + // Simulate: agr instance create --tool-name tool-name --timeout 6m --mount-options '[...]' + flags := withChanged(allInstanceCreateFlags(), "tool-name", "tool-name") + flags = withChanged(flags, "timeout", "6m") + flags = withChanged(flags, "mount-options", `[{"Name":"workspace","MountPath":"/workspace"}]`) + result, err := runtime.Handler.Run(context.Background(), command.Request{Flags: flags}) + if err != nil { + t.Fatalf("Run returned error: %v", err) + } + if cp.action != "StartSandboxInstance" { + t.Fatalf("action = %q, want StartSandboxInstance", cp.action) + } + // Verify request fields were sent. + if cp.request["ToolName"] != "tool-name" { + t.Fatalf("request ToolName = %v", cp.request["ToolName"]) + } + if cp.request["Timeout"] != "6m" { + t.Fatalf("request Timeout = %v", cp.request["Timeout"]) + } + // Verify text output contains instance ID and mount info. + var buf bytes.Buffer + if result.Text == nil { + t.Fatalf("expected text renderer, got nil") + } + result.Text(&buf) + text := buf.String() + if !strings.Contains(text, id) { + t.Fatalf("text output missing instance ID %q: %s", id, text) + } + if !strings.Contains(text, "workspace") { + t.Fatalf("text output missing mount name: %s", text) + } + if !strings.Contains(text, "RUNNING") { + t.Fatalf("text output missing status: %s", text) + } } -func TestModuleCreatesInstanceAndRendersText(t *testing.T) { - cp := &fakeMixedControlPlane{} +func TestModuleCreatesInstanceWithToolID(t *testing.T) { + id := "ins-byid" + toolID := "sdt-abc123" + status := "RUNNING" + created := "2026-05-21T10:00:00Z" + cp := &fakeMixedControlPlane{resp: &ags.StartSandboxInstanceResponseParams{ + Instance: &ags.SandboxInstance{ + InstanceId: &id, + ToolId: &toolID, + Status: &status, + CreateTime: &created, + }, + }} runtime, err := Module().Build(command.Deps{ControlPlane: cp}) if err != nil { t.Fatalf("Build returned error: %v", err) } - result, err := runtime.Handler.Run(context.Background(), command.Request{Flags: map[string]command.FlagValue{ - "tool-name": {Name: "tool-name", Type: command.FlagString, String: "demo", Changed: true}, - "timeout": {Name: "timeout", Type: command.FlagString, String: "600s", Changed: true}, - "auth-mode": {Name: "auth-mode", Type: command.FlagString, String: "NONE", Changed: true}, - "mount-options": {Name: "mount-options", Type: command.FlagString, String: `[{"Name":"data","MountPath":"/workspace"}]`, Changed: true}, - "metadata": {Name: "metadata", Type: command.FlagString, String: `[{"Name":"owner","Value":"ci"}]`, Changed: true}, - }}) + // Simulate: agr instance create --tool-id sdt-abc123 --auth-mode TOKEN + flags := withChanged(allInstanceCreateFlags(), "tool-id", toolID) + flags = withChanged(flags, "auth-mode", "TOKEN") + result, err := runtime.Handler.Run(context.Background(), command.Request{Flags: flags}) if err != nil { t.Fatalf("Run returned error: %v", err) } - if cp.action != "StartSandboxInstance" || cp.request["ToolName"] != "demo" || cp.request["Timeout"] != "600s" || cp.request["AuthMode"] != "NONE" { - t.Fatalf("action=%q request=%#v", cp.action, cp.request) + if cp.action != "StartSandboxInstance" { + t.Fatalf("action = %q, want StartSandboxInstance", cp.action) } - mounts, ok := cp.request["MountOptions"].([]any) - if !ok || len(mounts) != 1 { - t.Fatalf("MountOptions = %#v", cp.request["MountOptions"]) + if got := cp.request["ToolId"]; got != toolID { + t.Fatalf("request ToolId = %v, want %q", got, toolID) } - mount, ok := mounts[0].(map[string]any) - if !ok || mount["Name"] != "data" || mount["MountPath"] != "/workspace" { - t.Fatalf("MountOptions = %#v", cp.request["MountOptions"]) + if got := cp.request["AuthMode"]; got != "TOKEN" { + t.Fatalf("request AuthMode = %v, want TOKEN", got) } - data := result.Data.(map[string]any) - if data["InstanceId"] != "ins-created" || len(result.Effects) != 1 { - t.Fatalf("result = %#v", result) - } - var text bytes.Buffer - result.Text(&text) - if !strings.Contains(text.String(), "Instance created: ins-created") || !strings.Contains(text.String(), "MountOptions:") { - t.Fatalf("text = %q", text.String()) + var buf bytes.Buffer + result.Text(&buf) + if !strings.Contains(buf.String(), id) { + t.Fatalf("text output missing instance ID: %s", buf.String()) } } -func TestModuleValidatesToolSelection(t *testing.T) { - runtime, err := Module().Build(command.Deps{ControlPlane: &fakeMixedControlPlane{}}) +func TestModuleCreatesInstanceWithMetadata(t *testing.T) { + id := "ins-meta" + status := "RUNNING" + created := "2026-05-21T10:00:00Z" + cp := &fakeMixedControlPlane{resp: &ags.StartSandboxInstanceResponseParams{ + Instance: &ags.SandboxInstance{ + InstanceId: &id, + Status: &status, + CreateTime: &created, + }, + }} + runtime, err := Module().Build(command.Deps{ControlPlane: cp}) if err != nil { t.Fatalf("Build returned error: %v", err) } - for _, tc := range []struct { - name string - flags map[string]command.FlagValue - want string - }{ - {name: "missing", flags: map[string]command.FlagValue{}, want: "must specify either"}, - {name: "conflict", flags: map[string]command.FlagValue{ - "tool-name": {Name: "tool-name", Type: command.FlagString, String: "demo", Changed: true}, - "tool-id": {Name: "tool-id", Type: command.FlagString, String: "sdt-a", Changed: true}, - }, want: "cannot specify both"}, - } { - t.Run(tc.name, func(t *testing.T) { - _, err := runtime.Handler.Run(context.Background(), command.Request{Flags: tc.flags}) - if err == nil || !strings.Contains(err.Error(), tc.want) { - t.Fatalf("error = %v, want %q", err, tc.want) - } - }) - } -} - -func TestModuleAcceptsRequestBody(t *testing.T) { - cp := &fakeMixedControlPlane{} + // Simulate: agr instance create --tool-name my-tool --metadata '[{"Key":"env","Value":"test"}]' + flags := withChanged(allInstanceCreateFlags(), "tool-name", "my-tool") + flags = withChanged(flags, "metadata", `[{"Key":"env","Value":"test"}]`) + result, err := runtime.Handler.Run(context.Background(), command.Request{Flags: flags}) + if err != nil { + t.Fatalf("Run returned error: %v", err) + } + // Metadata should be in request. + if cp.request["Metadata"] == nil { + t.Fatalf("request Metadata is nil") + } + if result.Text == nil { + t.Fatalf("expected text renderer") + } +} + +func TestModuleBypassesValidationWithRequestFlag(t *testing.T) { + // --request mode sends raw JSON and skips tool-name/tool-id validation. + id := "ins-raw" + status := "RUNNING" + created := "2026-05-21T10:00:00Z" + cp := &fakeMixedControlPlane{resp: &ags.StartSandboxInstanceResponseParams{ + Instance: &ags.SandboxInstance{ + InstanceId: &id, + Status: &status, + CreateTime: &created, + }, + }} runtime, err := Module().Build(command.Deps{ControlPlane: cp}) if err != nil { t.Fatalf("Build returned error: %v", err) } - _, err = runtime.Handler.Run(context.Background(), command.Request{Flags: map[string]command.FlagValue{ - "request": {Name: "request", Type: command.FlagString, String: `{"ToolName":"demo"}`, Changed: true}, - }}) + // Simulate: agr instance create --request '{"ToolId":"sdt-xyz","Timeout":"10m"}' + flags := allInstanceCreateFlags() + f := flags["request"] + f.String = `{"ToolId":"sdt-xyz","Timeout":"10m"}` + f.Changed = true + flags["request"] = f + result, err := runtime.Handler.Run(context.Background(), command.Request{Flags: flags}) if err != nil { t.Fatalf("Run returned error: %v", err) } - if cp.request["ToolName"] != "demo" { - t.Fatalf("request = %#v", cp.request) + if cp.request["ToolId"] != "sdt-xyz" { + t.Fatalf("request ToolId = %v", cp.request["ToolId"]) + } + var buf bytes.Buffer + result.Text(&buf) + if !strings.Contains(buf.String(), id) { + t.Fatalf("text output missing instance ID: %s", buf.String()) + } +} + +func TestModuleReturnsResultWhenResponseNotTyped(t *testing.T) { + // When the fake CP returns a non-typed response, the handler should not panic. + cp := &fakeMixedControlPlane{} // no resp → returns map[string]any{"ok": true} + runtime, err := Module().Build(command.Deps{ControlPlane: cp}) + if err != nil { + t.Fatalf("Build returned error: %v", err) + } + flags := withChanged(allInstanceCreateFlags(), "tool-name", "t") + result, err := runtime.Handler.Run(context.Background(), command.Request{Flags: flags}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if result == nil { + t.Fatalf("expected non-nil result") } } -func TestModuleReturnsStructuredErrorWhenAPIResponseIsMissingInstance(t *testing.T) { - cp := &fakeMixedControlPlane{resp: &ags.StartSandboxInstanceResponseParams{}} +func TestModuleReturnsErrorWhenInstanceIsNil(t *testing.T) { + // Regression: when API returns a typed response but Instance is nil, + // the spinner must NOT print "✓ Instance created". + cp := &fakeMixedControlPlane{resp: &ags.StartSandboxInstanceResponseParams{ + Instance: nil, // no instance in response + }} runtime, err := Module().Build(command.Deps{ControlPlane: cp}) if err != nil { t.Fatalf("Build returned error: %v", err) } - _, err = runtime.Handler.Run(context.Background(), command.Request{Flags: map[string]command.FlagValue{ - "tool-name": {Name: "tool-name", Type: command.FlagString, String: "demo", Changed: true}, - }}) + flags := withChanged(allInstanceCreateFlags(), "tool-name", "t") + _, err = runtime.Handler.Run(context.Background(), command.Request{Flags: flags}) if err == nil { - t.Fatal("expected structured error") + t.Fatal("expected error for nil Instance") + } + if !strings.Contains(err.Error(), "no instance returned from API") { + t.Fatalf("unexpected error: %v", err) + } + // Verify structured error code is preserved. + var cliErr *output.CLIError + if !errors.As(err, &cliErr) { + t.Fatal("expected *output.CLIError") + } + if cliErr.Failure.Code != "INTERNAL_ERROR" { + t.Fatalf("Failure.Code = %q, want INTERNAL_ERROR", cliErr.Failure.Code) + } +} + +// stderrIO returns an IOStreams backed by in-memory buffers with stderr forced +// to look like a TTY (or not), for spinner wiring tests. +func stderrIO(stderrTTY bool) (*iostreams.IOStreams, *bytes.Buffer) { + _, _, _, stderr := iostreams.Test() + ios := &iostreams.IOStreams{ErrOut: stderr} + ios.SetStderrTTY(stderrTTY) + return ios, stderr +} + +func TestSpinnerWritesSuccessLineWhenStderrIsTTY(t *testing.T) { + // In a real interactive terminal (stderr is a TTY, text mode, interactive), + // the spinner must run and emit the final "✓ Instance created" status line + // to stderr — while stdout stays pure (no spinner noise on the data stream). + id := "ins-sp" + status := "RUNNING" + created := "2026-05-21T10:00:00Z" + cp := &fakeMixedControlPlane{resp: &ags.StartSandboxInstanceResponseParams{ + Instance: &ags.SandboxInstance{ + InstanceId: &id, Status: &status, CreateTime: &created, + }, + }} + ios, stderrBuf := stderrIO(true) + runtime, err := Module().Build(command.Deps{ControlPlane: cp, IO: ios}) + if err != nil { + t.Fatalf("Build: %v", err) + } + flags := withChanged(allInstanceCreateFlags(), "tool-name", "t") + result, err := runtime.Handler.Run(context.Background(), command.Request{Flags: flags}) + if err != nil { + t.Fatalf("Run: %v", err) + } + + // Final status line reached stderr. + if got := stderrBuf.String(); !strings.Contains(got, "✓ Instance created") { + t.Fatalf("stderr missing success status line: %q", got) + } + // stdout (the text renderer) must not be polluted by spinner frames. + var stdout bytes.Buffer + if result.Text == nil { + t.Fatalf("expected text renderer") + } + result.Text(&stdout) + for _, frame := range []string{"⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", "⠧", "⠇", "⠏"} { + if strings.Contains(stdout.String(), frame) { + t.Fatalf("stdout polluted with spinner frame %q: %q", frame, stdout.String()) + } + } +} + +func TestSpinnerIsNopWhenStderrIsNotTTY(t *testing.T) { + // Piped/redirected stderr (non-TTY): the spinner is a Nop. No frames, no + // status line, and the command still succeeds normally. + id := "ins-nop" + status := "RUNNING" + created := "2026-05-21T10:00:00Z" + cp := &fakeMixedControlPlane{resp: &ags.StartSandboxInstanceResponseParams{ + Instance: &ags.SandboxInstance{ + InstanceId: &id, Status: &status, CreateTime: &created, + }, + }} + ios, stderrBuf := stderrIO(false) + runtime, err := Module().Build(command.Deps{ControlPlane: cp, IO: ios}) + if err != nil { + t.Fatalf("Build: %v", err) + } + flags := withChanged(allInstanceCreateFlags(), "tool-name", "t") + result, err := runtime.Handler.Run(context.Background(), command.Request{Flags: flags}) + if err != nil { + t.Fatalf("Run: %v", err) + } + if got := stderrBuf.String(); got != "" { + t.Fatalf("expected empty stderr in non-TTY mode, got: %q", got) } - cliErr := output.ClassifyError(err) - if cliErr == nil || cliErr.Failure == nil { - t.Fatalf("error = %#v", err) + var stdout bytes.Buffer + if result.Text == nil { + t.Fatalf("expected text renderer") } - if cliErr.Failure.Code != "INTERNAL_ERROR" || cliErr.Failure.Message != "no instance returned from API" { - t.Fatalf("failure = %#v", cliErr.Failure) + result.Text(&stdout) + if !strings.Contains(stdout.String(), id) { + t.Fatalf("text output missing instance ID: %s", stdout.String()) } } diff --git a/internal/commands/tool/create/command.go b/internal/commands/tool/create/command.go index 0955177..771c2f3 100644 --- a/internal/commands/tool/create/command.go +++ b/internal/commands/tool/create/command.go @@ -7,8 +7,10 @@ import ( "strings" "github.com/TencentCloudAgentRuntime/ags-cli/internal/apicli" + "github.com/TencentCloudAgentRuntime/ags-cli/internal/cli" "github.com/TencentCloudAgentRuntime/ags-cli/internal/command" "github.com/TencentCloudAgentRuntime/ags-cli/internal/output" + "github.com/TencentCloudAgentRuntime/ags-cli/internal/progress" ags "github.com/tencentcloud/tencentcloud-sdk-go/tencentcloud/ags/v20250920" ) @@ -30,6 +32,7 @@ func Module() command.Module { Source: "mixed-api", }, Build: func(deps command.Deps) (command.Runtime, error) { + deps = deps.WithDefaults() builder := apicli.NewRequestBuilder(api) executor := apicli.NewExecutor(api, deps.ControlPlane) return command.Runtime{ @@ -43,11 +46,23 @@ func Module() command.Module { return nil, err } } + + // Show spinner for interactive text mode only. + sp := progress.NewForCLI(deps.IO.ErrOut, cli.IsJSONOutput(), cli.NonInteractive(), deps.IO.IsStderrTTY(), cli.NoColor()) + sp.Start("Creating tool...") + result, err := executor.Execute(ctx, apiReq) if err != nil { + sp.Stop("✗", "Failed to create tool") return nil, err } - applyCreateResultText(result, req) + + if applyCreateResultText(result, req) { + sp.Stop("✓", "Tool created") + } else { + // Response type mismatch or missing ToolId — cannot confirm success. + sp.Cleanup() + } return result, nil }), }, nil @@ -55,15 +70,21 @@ func Module() command.Module { } } -func applyCreateResultText(result *command.Result, req command.Request) { +// applyCreateResultText enriches the command result with text rendering and +// effects when the response is a valid CreateSandboxToolResponseParams with a +// non-empty ToolId. Returns true if the response was confirmed valid. +func applyCreateResultText(result *command.Result, req command.Request) bool { if result == nil { - return + return false } response, ok := result.Data.(*ags.CreateSandboxToolResponseParams) if !ok { - return + return false } toolID := derefString(response.ToolId) + if toolID == "" { + return false + } result.Effects = append(result.Effects, output.Effect{Kind: "create", Resource: "tool", Id: toolID}) result.Text = func(w io.Writer) { fmt.Fprintf(w, "Tool created: %s\n", toolID) @@ -77,6 +98,7 @@ func applyCreateResultText(result *command.Result, req command.Request) { {key: "Description", value: stringFlag(req, "description")}, }) } + return true } type kv struct { diff --git a/internal/commands/tool/create/command_test.go b/internal/commands/tool/create/command_test.go index c1ab2df..6e824c7 100644 --- a/internal/commands/tool/create/command_test.go +++ b/internal/commands/tool/create/command_test.go @@ -1,159 +1,443 @@ package create import ( + "bytes" "context" + "errors" + "strings" "testing" - "github.com/TencentCloudAgentRuntime/ags-cli/internal/apicli" "github.com/TencentCloudAgentRuntime/ags-cli/internal/command" + "github.com/TencentCloudAgentRuntime/ags-cli/internal/iostreams" "github.com/TencentCloudAgentRuntime/ags-cli/internal/output" + ags "github.com/tencentcloud/tencentcloud-sdk-go/tencentcloud/ags/v20250920" ) -type fakeControlPlane struct { +// fakeMixedControlPlane implements apicli.ControlPlane for testing the full +// tool create flow (validation → request build → executor → response render). +type fakeMixedControlPlane struct { action string request map[string]any + resp *ags.CreateSandboxToolResponseParams } -func (f *fakeControlPlane) Call(_ context.Context, action string, request map[string]any) (any, error) { +func (f *fakeMixedControlPlane) Call(_ context.Context, action string, request map[string]any) (any, error) { f.action = action f.request = request + if f.resp != nil { + return f.resp, nil + } return map[string]any{"ok": true}, nil } -func TestModuleBuildsCreateRequest(t *testing.T) { - cp := &fakeControlPlane{} +// allToolCreateFlags returns a complete flag set mimicking what Cobra registers +// for `agr tool create`. Unset optional flags have Changed=false. +func allToolCreateFlags() map[string]command.FlagValue { + return map[string]command.FlagValue{ + "tool-name": {Name: "tool-name", Type: command.FlagString}, + "tool-type": {Name: "tool-type", Type: command.FlagString}, + "network-configuration": {Name: "network-configuration", Type: command.FlagString}, + "description": {Name: "description", Type: command.FlagString}, + "default-timeout": {Name: "default-timeout", Type: command.FlagString}, + "tags": {Name: "tags", Type: command.FlagString}, + "client-token": {Name: "client-token", Type: command.FlagString}, + "role-arn": {Name: "role-arn", Type: command.FlagString}, + "storage-mounts": {Name: "storage-mounts", Type: command.FlagString}, + "custom-configuration": {Name: "custom-configuration", Type: command.FlagString}, + "log-configuration": {Name: "log-configuration", Type: command.FlagString}, + "persistent": {Name: "persistent", Type: command.FlagBool}, + "request": {Name: "request", Type: command.FlagString}, + } +} + +// withChanged returns a copy of flags with the named flag set to the given string value. +func withChanged(flags map[string]command.FlagValue, name, value string) map[string]command.FlagValue { + out := map[string]command.FlagValue{} + for k, v := range flags { + out[k] = v + } + f := out[name] + f.String = value + f.Changed = true + out[name] = f + return out +} + +// withBoolChanged returns a copy of flags with the named bool flag set. +func withBoolChanged(flags map[string]command.FlagValue, name string, value bool) map[string]command.FlagValue { + out := map[string]command.FlagValue{} + for k, v := range flags { + out[k] = v + } + f := out[name] + f.Bool = value + f.Changed = true + out[name] = f + return out +} + +// minRequiredFlags returns flags with the three required fields set. +func minRequiredFlags() map[string]command.FlagValue { + flags := allToolCreateFlags() + flags = withChanged(flags, "tool-name", "my-tool") + flags = withChanged(flags, "tool-type", "code-interpreter") + flags = withChanged(flags, "network-configuration", `{"NetworkMode":"SANDBOX"}`) + return flags +} + +func TestModuleBuildsWithoutError(t *testing.T) { + module := Module() + if module.Descriptor.Spec.ID == "" { + t.Fatalf("expected non-empty spec ID") + } + if module.Build == nil { + t.Fatalf("expected non-nil Build func") + } +} + +func TestModuleRequiresToolName(t *testing.T) { + runtime, err := Module().Build(command.Deps{ControlPlane: &fakeMixedControlPlane{}}) + if err != nil { + t.Fatalf("Build: %v", err) + } + // Missing --tool-name; other required fields present. + flags := allToolCreateFlags() + flags = withChanged(flags, "tool-type", "code-interpreter") + flags = withChanged(flags, "network-configuration", `{"NetworkMode":"SANDBOX"}`) + _, err = runtime.Handler.Run(context.Background(), command.Request{Flags: flags}) + if err == nil { + t.Fatalf("expected missing tool name error") + } + if !strings.Contains(err.Error(), "tool-name") { + t.Fatalf("unexpected error: %v", err) + } + // Verify structured error code (stable contract, not just message text). + var cliErr *output.CLIError + if !errors.As(err, &cliErr) { + t.Fatalf("expected *output.CLIError, got %T", err) + } + if cliErr.Failure.Code != "MISSING_REQUIRED_FLAG" { + t.Fatalf("Failure.Code = %q, want MISSING_REQUIRED_FLAG", cliErr.Failure.Code) + } +} + +func TestModuleRequiresToolType(t *testing.T) { + runtime, err := Module().Build(command.Deps{ControlPlane: &fakeMixedControlPlane{}}) + if err != nil { + t.Fatalf("Build: %v", err) + } + // Missing --tool-type; other required fields present. + flags := allToolCreateFlags() + flags = withChanged(flags, "tool-name", "my-tool") + flags = withChanged(flags, "network-configuration", `{"NetworkMode":"SANDBOX"}`) + _, err = runtime.Handler.Run(context.Background(), command.Request{Flags: flags}) + if err == nil { + t.Fatalf("expected missing tool type error") + } + if !strings.Contains(err.Error(), "tool-type") { + t.Fatalf("unexpected error: %v", err) + } + // Verify structured error code (stable contract, not just message text). + var cliErr *output.CLIError + if !errors.As(err, &cliErr) { + t.Fatalf("expected *output.CLIError, got %T", err) + } + if cliErr.Failure.Code != "MISSING_REQUIRED_FLAG" { + t.Fatalf("Failure.Code = %q, want MISSING_REQUIRED_FLAG", cliErr.Failure.Code) + } +} + +func TestModuleRequiresNetworkConfiguration(t *testing.T) { + runtime, err := Module().Build(command.Deps{ControlPlane: &fakeMixedControlPlane{}}) + if err != nil { + t.Fatalf("Build: %v", err) + } + // Missing --network-configuration; other required fields present. + flags := allToolCreateFlags() + flags = withChanged(flags, "tool-name", "my-tool") + flags = withChanged(flags, "tool-type", "code-interpreter") + _, err = runtime.Handler.Run(context.Background(), command.Request{Flags: flags}) + if err == nil { + t.Fatalf("expected missing network-configuration error") + } + if !strings.Contains(err.Error(), "network-configuration") { + t.Fatalf("unexpected error: %v", err) + } + // Verify structured error code (stable contract, not just message text). + var cliErr *output.CLIError + if !errors.As(err, &cliErr) { + t.Fatalf("expected *output.CLIError, got %T", err) + } + if cliErr.Failure.Code != "MISSING_REQUIRED_FLAG" { + t.Fatalf("Failure.Code = %q, want MISSING_REQUIRED_FLAG", cliErr.Failure.Code) + } +} + +func TestModuleCreatesToolAndRendersText(t *testing.T) { + toolID := "sdt-new123" + cp := &fakeMixedControlPlane{resp: &ags.CreateSandboxToolResponseParams{ + ToolId: &toolID, + }} runtime, err := Module().Build(command.Deps{ControlPlane: cp}) if err != nil { t.Fatalf("Build returned error: %v", err) } - _, err = runtime.Handler.Run(context.Background(), command.Request{ - Flags: map[string]command.FlagValue{ - "tool-name": {Name: "tool-name", Type: command.FlagString, String: "demo", Changed: true}, - "tool-type": {Name: "tool-type", Type: command.FlagString, String: "code-interpreter", Changed: true}, - "network-configuration": {Name: "network-configuration", Type: command.FlagString, String: `{"NetworkMode":"SANDBOX"}`, Changed: true}, - "tags": {Name: "tags", Type: command.FlagString, String: `[{"Key":"env","Value":"unit"}]`, Changed: true}, - "request": {Name: "request", Type: command.FlagString}, - }, - }) + // Simulate: agr tool create -n my-tool -t code-interpreter --network-configuration '...' -d "A test tool" + flags := minRequiredFlags() + flags = withChanged(flags, "description", "A test tool") + result, err := runtime.Handler.Run(context.Background(), command.Request{Flags: flags}) if err != nil { t.Fatalf("Run returned error: %v", err) } if cp.action != "CreateSandboxTool" { - t.Fatalf("action = %q", cp.action) + t.Fatalf("action = %q, want CreateSandboxTool", cp.action) + } + // Verify request fields. + if cp.request["ToolName"] != "my-tool" { + t.Fatalf("request ToolName = %v", cp.request["ToolName"]) + } + if cp.request["ToolType"] != "code-interpreter" { + t.Fatalf("request ToolType = %v", cp.request["ToolType"]) + } + if cp.request["Description"] != "A test tool" { + t.Fatalf("request Description = %v", cp.request["Description"]) } - if cp.request["ToolName"] != "demo" || cp.request["ToolType"] != "code-interpreter" { - t.Fatalf("request = %#v", cp.request) + // Verify text output. + var buf bytes.Buffer + if result.Text == nil { + t.Fatalf("expected text renderer, got nil") } - network := cp.request["NetworkConfiguration"].(map[string]any) - if network["NetworkMode"] != "SANDBOX" { - t.Fatalf("NetworkConfiguration = %#v", network) + result.Text(&buf) + text := buf.String() + if !strings.Contains(text, toolID) { + t.Fatalf("text output missing tool ID %q: %s", toolID, text) + } + if !strings.Contains(text, "my-tool") { + t.Fatalf("text output missing tool name: %s", text) + } + if !strings.Contains(text, "code-interpreter") { + t.Fatalf("text output missing tool type: %s", text) + } + // Verify effects. + if len(result.Effects) == 0 { + t.Fatalf("expected at least one effect") + } + if result.Effects[0].Kind != "create" || result.Effects[0].Resource != "tool" { + t.Fatalf("effect = %+v", result.Effects[0]) } } -func TestModuleRejectsMissingName(t *testing.T) { - cp := &fakeControlPlane{} +func TestModuleCreatesToolWithAllOptionalFlags(t *testing.T) { + toolID := "sdt-full" + cp := &fakeMixedControlPlane{resp: &ags.CreateSandboxToolResponseParams{ + ToolId: &toolID, + }} runtime, err := Module().Build(command.Deps{ControlPlane: cp}) if err != nil { t.Fatalf("Build returned error: %v", err) } - _, err = runtime.Handler.Run(context.Background(), command.Request{ - Flags: map[string]command.FlagValue{ - "tool-type": {Name: "tool-type", Type: command.FlagString, String: "code-interpreter", Changed: true}, - "request": {Name: "request", Type: command.FlagString}, - }, - }) - if cliErr, ok := err.(*output.CLIError); !ok || cliErr.Failure.Code != "MISSING_REQUIRED_FLAG" { - t.Fatalf("error = %#v, want MISSING_REQUIRED_FLAG", err) + // Simulate a fully-loaded create command with all optional flags. + flags := minRequiredFlags() + flags = withChanged(flags, "description", "full tool") + flags = withChanged(flags, "default-timeout", "30m") + flags = withChanged(flags, "tags", `[{"Key":"team","Value":"platform"}]`) + flags = withChanged(flags, "client-token", "tok-abc") + flags = withBoolChanged(flags, "persistent", true) + result, err := runtime.Handler.Run(context.Background(), command.Request{Flags: flags}) + if err != nil { + t.Fatalf("Run returned error: %v", err) + } + // Verify optional fields in request. + if cp.request["DefaultTimeout"] != "30m" { + t.Fatalf("request DefaultTimeout = %v", cp.request["DefaultTimeout"]) + } + if cp.request["ClientToken"] != "tok-abc" { + t.Fatalf("request ClientToken = %v", cp.request["ClientToken"]) + } + if cp.request["Tags"] == nil { + t.Fatalf("request Tags is nil") + } + var buf bytes.Buffer + result.Text(&buf) + if !strings.Contains(buf.String(), toolID) { + t.Fatalf("text output missing tool ID: %s", buf.String()) + } +} + +func TestModuleCreatesToolWithStorageMountsRequiresRoleArn(t *testing.T) { + runtime, err := Module().Build(command.Deps{ControlPlane: &fakeMixedControlPlane{}}) + if err != nil { + t.Fatalf("Build returned error: %v", err) + } + // --storage-mounts without --role-arn should fail convenience validation. + flags := minRequiredFlags() + flags = withChanged(flags, "storage-mounts", `[{"Name":"data","MountPath":"/data","StorageSource":{"Cos":{"Endpoint":"cos.ap-guangzhou.myqcloud.com","BucketName":"b","BucketPath":"/"}}}]`) + _, err = runtime.Handler.Run(context.Background(), command.Request{Flags: flags}) + if err == nil { + t.Fatalf("expected role-arn required error") + } + if !strings.Contains(err.Error(), "role-arn") { + t.Fatalf("unexpected error: %v", err) + } + // Verify structured error code (stable contract, not just message text). + var cliErr *output.CLIError + if !errors.As(err, &cliErr) { + t.Fatalf("expected *output.CLIError, got %T", err) + } + if cliErr.Failure.Code != "MISSING_REQUIRED_FLAG" { + t.Fatalf("Failure.Code = %q, want MISSING_REQUIRED_FLAG", cliErr.Failure.Code) } } -func TestModuleRejectsMissingType(t *testing.T) { - cp := &fakeControlPlane{} +func TestModuleCreatesToolWithStorageMountsAndRoleArn(t *testing.T) { + toolID := "sdt-cos" + cp := &fakeMixedControlPlane{resp: &ags.CreateSandboxToolResponseParams{ + ToolId: &toolID, + }} runtime, err := Module().Build(command.Deps{ControlPlane: cp}) if err != nil { t.Fatalf("Build returned error: %v", err) } - _, err = runtime.Handler.Run(context.Background(), command.Request{ - Flags: map[string]command.FlagValue{ - "tool-name": {Name: "tool-name", Type: command.FlagString, String: "demo", Changed: true}, - "request": {Name: "request", Type: command.FlagString}, - }, - }) - if cliErr, ok := err.(*output.CLIError); !ok || cliErr.Failure.Code != "MISSING_REQUIRED_FLAG" { - t.Fatalf("error = %#v, want MISSING_REQUIRED_FLAG", err) + // Valid: --storage-mounts with --role-arn. + flags := minRequiredFlags() + flags = withChanged(flags, "role-arn", "qcs::cam::uin/100000:roleName/my-role") + flags = withChanged(flags, "storage-mounts", `[{"Name":"data","MountPath":"/data","StorageSource":{"Cos":{"Endpoint":"cos.ap-guangzhou.myqcloud.com","BucketName":"b","BucketPath":"/"}}}]`) + _, err = runtime.Handler.Run(context.Background(), command.Request{Flags: flags}) + if err != nil { + t.Fatalf("Run returned error: %v", err) + } + if cp.request["RoleArn"] != "qcs::cam::uin/100000:roleName/my-role" { + t.Fatalf("request RoleArn = %v", cp.request["RoleArn"]) + } + if cp.request["StorageMounts"] == nil { + t.Fatalf("request StorageMounts is nil") } } -func TestModuleRejectsMissingNetworkConfiguration(t *testing.T) { - cp := &fakeControlPlane{} +func TestModuleBypassesConvenienceValidationWithRequestFlag(t *testing.T) { + // --request mode sends raw JSON; convenience validation is skipped. + toolID := "sdt-raw456" + cp := &fakeMixedControlPlane{resp: &ags.CreateSandboxToolResponseParams{ + ToolId: &toolID, + }} runtime, err := Module().Build(command.Deps{ControlPlane: cp}) if err != nil { t.Fatalf("Build returned error: %v", err) } - _, err = runtime.Handler.Run(context.Background(), command.Request{ - Flags: map[string]command.FlagValue{ - "tool-name": {Name: "tool-name", Type: command.FlagString, String: "demo", Changed: true}, - "tool-type": {Name: "tool-type", Type: command.FlagString, String: "code-interpreter", Changed: true}, - "request": {Name: "request", Type: command.FlagString}, - }, - }) - if cliErr, ok := err.(*output.CLIError); !ok || cliErr.Failure.Code != "MISSING_REQUIRED_FLAG" { - t.Fatalf("error = %#v, want MISSING_REQUIRED_FLAG", err) - } -} - -func TestValidateConvenienceRequestRequiresRoleArnForMounts(t *testing.T) { - for _, tc := range []struct { - name string - mounts any - }{ - {name: "any slice", mounts: []any{map[string]any{"Bucket": "b"}}}, - {name: "map slice", mounts: []map[string]any{{"Bucket": "b"}}}, - {name: "string fallback", mounts: "cos://bucket/path"}, - } { - t.Run(tc.name, func(t *testing.T) { - err := validateConvenienceRequest(map[string]any{ - "ToolName": "demo", - "ToolType": "code-interpreter", - "StorageMounts": tc.mounts, - }) - if cliErr, ok := err.(*output.CLIError); !ok || cliErr.Failure.Code != "MISSING_REQUIRED_FLAG" { - t.Fatalf("error = %#v, want MISSING_REQUIRED_FLAG", err) - } - }) - } -} - -func TestValidateConvenienceRequestAllowsEmptyMounts(t *testing.T) { - for _, mounts := range []any{[]any{}, []map[string]any{}, nil} { - err := validateConvenienceRequest(map[string]any{ - "ToolName": "demo", - "ToolType": "code-interpreter", - "StorageMounts": mounts, - }) - if err != nil { - t.Fatalf("validateConvenienceRequest(%#v) = %v", mounts, err) + // Simulate: agr tool create --request '{"ToolName":"raw","ToolType":"code-interpreter","NetworkConfiguration":{...}}' + flags := allToolCreateFlags() + f := flags["request"] + f.String = `{"ToolName":"raw","ToolType":"code-interpreter","NetworkConfiguration":{"NetworkMode":"SANDBOX"}}` + f.Changed = true + flags["request"] = f + result, err := runtime.Handler.Run(context.Background(), command.Request{Flags: flags}) + if err != nil { + t.Fatalf("Run returned error: %v", err) + } + if cp.action != "CreateSandboxTool" { + t.Fatalf("action = %q, want CreateSandboxTool", cp.action) + } + if cp.request["ToolName"] != "raw" { + t.Fatalf("request ToolName = %v", cp.request["ToolName"]) + } + var buf bytes.Buffer + if result.Text != nil { + result.Text(&buf) + if !strings.Contains(buf.String(), toolID) { + t.Fatalf("text output missing tool ID: %s", buf.String()) } } } -func TestModuleAcceptsRequestBody(t *testing.T) { - cp := &fakeControlPlane{} +func TestModuleReturnsResultWhenResponseNotTyped(t *testing.T) { + // Non-typed response (e.g. API schema change) should not panic. + cp := &fakeMixedControlPlane{} // returns map[string]any{"ok": true} runtime, err := Module().Build(command.Deps{ControlPlane: cp}) if err != nil { t.Fatalf("Build returned error: %v", err) } - _, err = runtime.Handler.Run(context.Background(), command.Request{ - Flags: map[string]command.FlagValue{ - "request": {Name: "request", Type: command.FlagString, String: `{"ToolName":"json","ToolType":"code-interpreter","NetworkConfiguration":{"NetworkMode":"PUBLIC"}}`, Changed: true}, - }, - }) + result, err := runtime.Handler.Run(context.Background(), command.Request{Flags: minRequiredFlags()}) if err != nil { - t.Fatalf("Run returned error: %v", err) + t.Fatalf("unexpected error: %v", err) } - if cp.request["ToolName"] != "json" { - t.Fatalf("request = %#v", cp.request) + if result == nil { + t.Fatalf("expected non-nil result") } } -var _ apicli.ControlPlane = (*fakeControlPlane)(nil) +// stderrIO returns an IOStreams backed by in-memory buffers with stderr forced +// to look like a TTY, for spinner wiring tests. +func stderrIO(stderrTTY bool) (*iostreams.IOStreams, *bytes.Buffer) { + _, _, _, stderr := iostreams.Test() + ios := &iostreams.IOStreams{ErrOut: stderr} + ios.SetStderrTTY(stderrTTY) + return ios, stderr +} + +func TestSpinnerWritesSuccessLineWhenStderrIsTTY(t *testing.T) { + // Interactive terminal (stderr TTY, text mode, interactive): the spinner + // runs and emits "✓ Tool created" to stderr; stdout stays pure. + toolID := "sdt-sp" + cp := &fakeMixedControlPlane{resp: &ags.CreateSandboxToolResponseParams{ToolId: &toolID}} + ios, stderrBuf := stderrIO(true) + runtime, err := Module().Build(command.Deps{ControlPlane: cp, IO: ios}) + if err != nil { + t.Fatalf("Build: %v", err) + } + result, err := runtime.Handler.Run(context.Background(), command.Request{Flags: minRequiredFlags()}) + if err != nil { + t.Fatalf("Run: %v", err) + } + if got := stderrBuf.String(); !strings.Contains(got, "✓ Tool created") { + t.Fatalf("stderr missing success status line: %q", got) + } + // stdout must not be polluted by spinner frames. + var stdout bytes.Buffer + if result.Text == nil { + t.Fatalf("expected text renderer") + } + result.Text(&stdout) + for _, frame := range []string{"⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", "⠧", "⠇", "⠏"} { + if strings.Contains(stdout.String(), frame) { + t.Fatalf("stdout polluted with spinner frame %q: %q", frame, stdout.String()) + } + } +} + +func TestSpinnerIsNopWhenStderrIsNotTTY(t *testing.T) { + // Piped/redirected stderr (non-TTY): spinner is a Nop — no frames, no + // status line — and the command still succeeds normally. + toolID := "sdt-nop" + cp := &fakeMixedControlPlane{resp: &ags.CreateSandboxToolResponseParams{ToolId: &toolID}} + ios, stderrBuf := stderrIO(false) + runtime, err := Module().Build(command.Deps{ControlPlane: cp, IO: ios}) + if err != nil { + t.Fatalf("Build: %v", err) + } + result, err := runtime.Handler.Run(context.Background(), command.Request{Flags: minRequiredFlags()}) + if err != nil { + t.Fatalf("Run: %v", err) + } + if got := stderrBuf.String(); got != "" { + t.Fatalf("expected empty stderr in non-TTY mode, got: %q", got) + } + var stdout bytes.Buffer + if result.Text == nil { + t.Fatalf("expected text renderer") + } + result.Text(&stdout) + if !strings.Contains(stdout.String(), toolID) { + t.Fatalf("text output missing tool ID: %s", stdout.String()) + } +} + +func TestModuleDescriptor(t *testing.T) { + module := Module() + spec := module.Descriptor.Spec + if spec.ID != "tool.create" { + t.Fatalf("spec ID = %q, want tool.create", spec.ID) + } + if module.Descriptor.Generated == nil { + t.Fatalf("expected generated descriptor") + } +} diff --git a/internal/iostreams/iostreams.go b/internal/iostreams/iostreams.go index 9b0b3b0..f7424d0 100644 --- a/internal/iostreams/iostreams.go +++ b/internal/iostreams/iostreams.go @@ -20,6 +20,7 @@ type IOStreams struct { isStdoutTTY bool isStdinTTY bool + isStderrTTY bool } // System returns an IOStreams connected to the real terminal. @@ -30,6 +31,7 @@ func System() *IOStreams { ErrOut: os.Stderr, isStdoutTTY: term.IsTerminal(int(os.Stdout.Fd())), isStdinTTY: term.IsTerminal(int(os.Stdin.Fd())), + isStderrTTY: term.IsTerminal(int(os.Stderr.Fd())), } } @@ -54,8 +56,14 @@ func (s *IOStreams) IsStdoutTTY() bool { return s.isStdoutTTY } // IsStdinTTY reports whether stdin was detected or forced to be a terminal. func (s *IOStreams) IsStdinTTY() bool { return s.isStdinTTY } +// IsStderrTTY reports whether stderr was detected or forced to be a terminal. +func (s *IOStreams) IsStderrTTY() bool { return s.isStderrTTY } + // SetStdoutTTY overrides TTY detection (for tests). func (s *IOStreams) SetStdoutTTY(v bool) { s.isStdoutTTY = v } // SetStdinTTY overrides stdin TTY detection (for tests). func (s *IOStreams) SetStdinTTY(v bool) { s.isStdinTTY = v } + +// SetStderrTTY overrides stderr TTY detection (for tests). +func (s *IOStreams) SetStderrTTY(v bool) { s.isStderrTTY = v } diff --git a/internal/progress/spinner.go b/internal/progress/spinner.go new file mode 100644 index 0000000..a3a198f --- /dev/null +++ b/internal/progress/spinner.go @@ -0,0 +1,167 @@ +// Package progress provides a lightweight terminal spinner for long-running +// operations. The spinner writes to stderr and is automatically disabled when +// stderr is not a TTY, --non-interactive is set, or -o json is active. +// +// Design inspired by AgentCore CLI's deploy/progress.ts: +// - onProgress(step, status) callback pattern with start/success/error states +// - 80ms Braille dot animation on \r overwrite +// - --json mode: Nop() spinner (no output, stdout stays pure) +// - cleanup() for safe teardown on interrupt/error +// - Multi-step support via repeated Start()/Stop() calls +package progress + +import ( + "fmt" + "io" + "sync" + "time" +) + +// frames is the Braille dots spinner animation (10 frames, matching AgentCore). +var frames = []string{"⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", "⠧", "⠇", "⠏"} + +// Spinner displays a terminal spinner with a message on stderr. +// It is safe for concurrent use. Use New() for interactive mode or +// Nop() for JSON/non-interactive mode. +type Spinner struct { + w io.Writer + mu sync.Mutex + msg string + stop chan struct{} + stopped chan struct{} + active bool + started bool // tracks whether Start was ever called + noColor bool // when true, skip ANSI escape sequences +} + +// New creates a new Spinner that writes to the given writer (typically stderr). +// If w is nil, the spinner is a no-op. +func New(w io.Writer) *Spinner { + return &Spinner{w: w} +} + +// Nop returns a no-op Spinner that does nothing. +// Use this when output is JSON or non-interactive (mirrors AgentCore's +// "onProgress = undefined" pattern). +func Nop() *Spinner { + return &Spinner{w: nil} +} + +// NewForCLI creates a Spinner appropriate for a CLI command context. +// It returns a real spinner only when all conditions are met: +// - jsonOutput is false (text mode) +// - nonInteractive is false +// - w is not nil (stderr writer) +// - isTTY is true (stderr is a terminal) +// +// When noColor is true, the spinner omits ANSI escape sequences (uses simple +// overwrite with spaces instead of \033[K). Otherwise returns Nop(). +// This eliminates duplicate guard logic across commands. +func NewForCLI(w io.Writer, jsonOutput, nonInteractive, isTTY, noColor bool) *Spinner { + if !jsonOutput && !nonInteractive && w != nil && isTTY { + sp := New(w) + sp.noColor = noColor + return sp + } + return Nop() +} + +// Start begins the spinner animation with the given message. +// Can be called multiple times for multi-step flows (each call resets the +// spinner to the new step, matching AgentCore's startStep() pattern). +func (s *Spinner) Start(msg string) { + if s.w == nil { + return + } + s.mu.Lock() + if s.active { + // Already running — stop the current animation before starting new one. + s.mu.Unlock() + s.stopInternal() + s.mu.Lock() + } + s.msg = msg + s.active = true + s.started = true + s.stop = make(chan struct{}) + s.stopped = make(chan struct{}) + s.mu.Unlock() + + go s.run() +} + +// Stop stops the spinner and prints a final status line. +// status is typically "✓" (success) or "✗" (error). +// If Start was never called, Stop is a no-op (prevents orphan status lines). +// This matches AgentCore's endStep(status) pattern. +func (s *Spinner) Stop(status string, msg string) { + if s.w == nil { + return + } + s.mu.Lock() + wasStarted := s.started + s.mu.Unlock() + if !wasStarted { + return + } + s.stopInternal() + // Print final status line. + fmt.Fprintf(s.w, "%s %s\n", status, msg) +} + +// Cleanup stops the spinner without printing a final status. +// Use in defer or signal handlers to prevent orphan spinner output. +// Matches AgentCore's cleanup() pattern. +func (s *Spinner) Cleanup() { + if s.w == nil { + return + } + s.stopInternal() +} + +func (s *Spinner) stopInternal() { + s.mu.Lock() + if !s.active { + s.mu.Unlock() + return + } + s.active = false + s.mu.Unlock() + + close(s.stop) + <-s.stopped +} + +func (s *Spinner) run() { + defer close(s.stopped) + ticker := time.NewTicker(80 * time.Millisecond) + defer ticker.Stop() + + i := 0 + for { + select { + case <-s.stop: + // Clear the spinner line before returning. + s.clearLine() + return + case <-ticker.C: + s.mu.Lock() + msg := s.msg + s.mu.Unlock() + s.clearLine() + fmt.Fprintf(s.w, "\r%s %s", frames[i%len(frames)], msg) + i++ + } + } +} + +// clearLine moves cursor to start and clears the line. When noColor is set, +// uses spaces padding instead of ANSI escape \033[K. +func (s *Spinner) clearLine() { + if s.noColor { + // Overwrite with spaces (80 chars should cover most lines). + fmt.Fprintf(s.w, "\r%-80s\r", "") + } else { + fmt.Fprintf(s.w, "\r\033[K") + } +} diff --git a/internal/progress/spinner_test.go b/internal/progress/spinner_test.go new file mode 100644 index 0000000..d27a1cf --- /dev/null +++ b/internal/progress/spinner_test.go @@ -0,0 +1,163 @@ +package progress + +import ( + "bytes" + "strings" + "testing" + "time" +) + +func TestNopSpinnerDoesNothing(t *testing.T) { + sp := Nop() + // Should not panic. + sp.Start("hello") + sp.Stop("✓", "done") + sp.Cleanup() +} + +func TestNopSpinnerWithNilWriter(t *testing.T) { + sp := New(nil) + // Should not panic. + sp.Start("hello") + sp.Stop("✓", "done") + sp.Cleanup() +} + +func TestSpinnerWritesFramesToWriter(t *testing.T) { + var buf bytes.Buffer + sp := New(&buf) + sp.Start("Loading...") + // Wait for at least 2 frames to render (80ms per frame; 300ms is ~3-4 frames). + time.Sleep(300 * time.Millisecond) + sp.Stop("✓", "Done") + + output := buf.String() + // Should contain at least one spinner frame. + hasFrame := false + for _, f := range frames { + if strings.Contains(output, f) { + hasFrame = true + break + } + } + if !hasFrame { + t.Fatalf("expected spinner frame in output, got: %q", output) + } + // Should contain the final status line. + if !strings.Contains(output, "✓ Done") { + t.Fatalf("expected final status '✓ Done' in output, got: %q", output) + } +} + +func TestSpinnerStopWithError(t *testing.T) { + var buf bytes.Buffer + sp := New(&buf) + sp.Start("Creating...") + time.Sleep(100 * time.Millisecond) + sp.Stop("✗", "Failed") + + output := buf.String() + if !strings.Contains(output, "✗ Failed") { + t.Fatalf("expected '✗ Failed' in output, got: %q", output) + } +} + +func TestSpinnerCleanupDoesNotPrintStatus(t *testing.T) { + var buf bytes.Buffer + sp := New(&buf) + sp.Start("Working...") + time.Sleep(100 * time.Millisecond) + sp.Cleanup() + + output := buf.String() + // Cleanup should NOT print a status line (no ✓ or ✗). + if strings.Contains(output, "✓") || strings.Contains(output, "✗") { + t.Fatalf("cleanup should not print status, got: %q", output) + } +} + +func TestSpinnerMultiStep(t *testing.T) { + var buf bytes.Buffer + sp := New(&buf) + + // Step 1 + sp.Start("Step 1...") + time.Sleep(100 * time.Millisecond) + sp.Stop("✓", "Step 1 complete") + + // Step 2 + sp.Start("Step 2...") + time.Sleep(100 * time.Millisecond) + sp.Stop("✓", "Step 2 complete") + + output := buf.String() + if !strings.Contains(output, "✓ Step 1 complete") { + t.Fatalf("missing step 1 in output: %q", output) + } + if !strings.Contains(output, "✓ Step 2 complete") { + t.Fatalf("missing step 2 in output: %q", output) + } +} + +func TestSpinnerDoubleStopDoesNotPanic(t *testing.T) { + var buf bytes.Buffer + sp := New(&buf) + sp.Start("test") + time.Sleep(100 * time.Millisecond) + sp.Stop("✓", "first") + // Second stop should not panic. + sp.Stop("✓", "second") +} + +func TestSpinnerStartOverwritesPreviousStep(t *testing.T) { + var buf bytes.Buffer + sp := New(&buf) + sp.Start("First step") + time.Sleep(100 * time.Millisecond) + // Starting a new step without stopping should cleanly transition. + sp.Start("Second step") + time.Sleep(100 * time.Millisecond) + sp.Stop("✓", "Done") + + output := buf.String() + if !strings.Contains(output, "✓ Done") { + t.Fatalf("expected final status in output: %q", output) + } +} + +func TestSpinnerMessage(t *testing.T) { + var buf bytes.Buffer + sp := New(&buf) + sp.Start("Creating instance...") + time.Sleep(150 * time.Millisecond) + sp.Stop("✓", "Instance created") + + output := buf.String() + if !strings.Contains(output, "Creating instance...") { + t.Fatalf("expected message in spinner output: %q", output) + } +} + +func TestSpinnerStopWithoutStartIsNoop(t *testing.T) { + var buf bytes.Buffer + sp := New(&buf) + // Stop without Start should not print anything or panic. + sp.Stop("✓", "should not appear") + + output := buf.String() + if output != "" { + t.Fatalf("expected no output when Stop called without Start, got: %q", output) + } +} + +func TestSpinnerCleanupWithoutStartIsNoop(t *testing.T) { + var buf bytes.Buffer + sp := New(&buf) + // Cleanup without Start should not panic. + sp.Cleanup() + + output := buf.String() + if output != "" { + t.Fatalf("expected no output, got: %q", output) + } +}