diff --git a/README.md b/README.md index 60af309..2e3a048 100644 --- a/README.md +++ b/README.md @@ -177,6 +177,12 @@ a CI job could read: this machine's CLI session, a platform token or an API key. A platform token needs the `automations:read` scope to list and inspect automations and `automations:run` to start one. +The text views show workspace and automation names, looked up once per +command; detail views print `name (id)` so the id stays copyable. A token +without `workspaces:read` (or `automations:read`, for automation names in +`nan approvals ls` and `nan runs show`) gets the ids instead, never an error. +`--json` prints the API's objects unchanged, with ids only. + ### Exit codes `nan run`, `nan runs logs -f` and `nan automations run` exit with the run's outcome, so a script can diff --git a/cmd/approvals.go b/cmd/approvals.go index 91e7108..de96d36 100644 --- a/cmd/approvals.go +++ b/cmd/approvals.go @@ -35,7 +35,8 @@ CLI session or tokens (an agent in a workspace, a CI job) can approve the actions an agent proposed. Authentication is the same as nan run (see nan run --help); a platform token -needs the runs scope. +needs the runs scope. Automation names are looked up with automations:read; +without it the automation's id is shown instead. Docs: https://nan.builders/docs/runs`, Annotations: sysexits, @@ -126,6 +127,9 @@ func doApprovalsLs(ctx context.Context, env *runsEnv, state string) error { } return nil } + for i := range list.Data { + env.nameAutomation(ctx, list.Data[i].Automation) + } tw := tabwriter.NewWriter(env.stdout, 0, 0, 2, ' ', 0) fmt.Fprintln(tw, "RUN\tSTATE\tAUTOMATION\tWAITING FOR APPROVAL\tEXPIRES") for _, a := range list.Data { diff --git a/cmd/automations.go b/cmd/automations.go index cd26567..e57a62c 100644 --- a/cmd/automations.go +++ b/cmd/automations.go @@ -66,7 +66,8 @@ an API key. Authentication is the same as nan run (see nan run --help). A platform token needs the automations:read scope to list and inspect, and automations:run to -start a run. +start a run. Workspace names are looked up with workspaces:read; without it +the workspace's id is shown instead. Docs: https://nan.builders/docs/runs`, Annotations: sysexits, @@ -246,13 +247,16 @@ func doAutomationsLs(ctx context.Context, env *runsEnv) error { fmt.Fprintf(env.stderr, "No automations yet. Create one in the portal: %s\n", runs.NewAutomationURL) return nil } + for i := range list.Data { + env.nameWorkspace(ctx, list.Data[i].Workspace) + } tw := tabwriter.NewWriter(env.stdout, 0, 0, 2, ' ', 0) fmt.Fprintln(tw, "ID\tNAME\tWORKSPACE\tAGENT\tSTATUS\tAPPROVAL") for _, a := range list.Data { fmt.Fprintf(tw, "%s\t%s\t%s\t%s\t%s\t%s\n", runs.SanitizeLine(a.ID), runs.Truncate(runs.SanitizeLine(a.Name), 40), - runs.SanitizeLine(a.Workspace.Label()), + runs.Truncate(runs.SanitizeLine(a.Workspace.Label()), 40), runs.SanitizeLine(a.Agent), automationStatus(a), runs.Truncate(approvalSummary(a.ApprovalKinds), 40)) @@ -295,6 +299,7 @@ func doAutomationsShow(ctx context.Context, env *runsEnv, ref string) error { if autoShowJSON { return printJSON(env.stdout, a, true) } + env.nameWorkspace(ctx, a.Workspace) triggers, trigErr := env.client.ListTriggers(ctx, id) tw := tabwriter.NewWriter(env.stdout, 0, 0, 2, ' ', 0) @@ -310,7 +315,9 @@ func doAutomationsShow(ctx context.Context, env *runsEnv, ref string) error { if a.Template != "" && a.Template != "custom" { row("template", runs.SanitizeLine(a.Template)) } - row("workspace", runs.SanitizeLine(a.Workspace.Label())) + if a.Workspace != nil { + row("workspace", runs.NameAndID(a.Workspace.Name, a.Workspace.ID)) + } agent := runs.SanitizeLine(a.Agent) if a.Model != nil && *a.Model != "" { agent += " · model " + runs.SanitizeLine(*a.Model) @@ -486,7 +493,12 @@ func doAutomationsRun(ctx context.Context, env *runsEnv, ref string, input json. return runExit(run) } if !replayed { - name := runs.SanitizeLine(run.Automation.Label()) + // The platform names the automation by id; what the member typed + // (a name, usually) reads better than a UUID they did not. + name := "" + if run.Automation != nil { + name = runs.SanitizeLine(run.Automation.Name) + } if name == "" { name = runs.Truncate(runs.SanitizeLine(ref), 64) } diff --git a/cmd/automations_test.go b/cmd/automations_test.go index bf87ef5..0a688fc 100644 --- a/cmd/automations_test.go +++ b/cmd/automations_test.go @@ -86,7 +86,8 @@ func (f *fakePhase2) ServeHTTP(w http.ResponseWriter, r *http.Request) { f.runIdem = r.Header.Get("Idempotency-Key") _ = json.NewDecoder(r.Body).Decode(&f.runBody) w.WriteHeader(http.StatusCreated) - _, _ = io.WriteString(w, f.runs.run(api.StateQueued)) + // The platform names the automation by id only. + _, _ = io.WriteString(w, strings.Replace(f.runs.run(api.StateQueued), `"object":"run",`, `"object":"run","automation_id":"`+testAutoID+`",`, 1)) case r.Method == http.MethodGet && p == "/v1/approvals": f.approvalsQ = r.URL.RawQuery _, _ = io.WriteString(w, `{"object":"list","data":[{"run_id":"`+testRunID+`","state":"pending","approval_from":1, @@ -262,7 +263,7 @@ func TestAutomationsRunExitsTenOnAwaitingApproval(t *testing.T) { t.Errorf("request: idem %q body %v", fake.runIdem, fake.runBody) } out := h.stdout.String() + h.stderr.String() - for _, want := range []string{"approval required for: git_push", "awaiting approval", "nan runs proposal " + testRunID, "nan runs reject " + testRunID, "browser sign-in"} { + for _, want := range []string{"of PR review queued on develop", "approval required for: git_push", "awaiting approval", "nan runs proposal " + testRunID, "nan runs reject " + testRunID, "browser sign-in"} { if !strings.Contains(out, want) { t.Errorf("output lacks %q:\n%s", want, out) } diff --git a/cmd/golden_test.go b/cmd/golden_test.go index a1bb779..96ca8c3 100644 --- a/cmd/golden_test.go +++ b/cmd/golden_test.go @@ -8,6 +8,7 @@ import ( "os" "path/filepath" "strings" + "sync" "testing" "github.com/nxssie/nan-cli/internal/api" @@ -16,6 +17,15 @@ import ( // goldenEnv serves cloud-api's own golden responses (copied into // internal/api/testdata/phase2) on the paths that return them. func goldenEnv(t *testing.T) (*runsEnv, *bytes.Buffer, *bytes.Buffer) { + t.Helper() + env, out, errOut, _ := goldenEnvWith(t, "") + return env, out, errOut +} + +// goldenEnvWith is goldenEnv that also answers GET /api/workspaces with +// workspaces (a JSON body; "" leaves it a 404, as for a platform token +// without workspaces:read), and records every request it serves. +func goldenEnvWith(t *testing.T, workspaces string) (*runsEnv, *bytes.Buffer, *bytes.Buffer, func() []string) { t.Helper() const run = "6f1c2a9b-0000-4000-8000-000000000001" const auto = "a0000000-0000-4000-8000-000000000001" @@ -27,7 +37,17 @@ func goldenEnv(t *testing.T) (*runsEnv, *bytes.Buffer, *bytes.Buffer) { "/v1/automations/" + auto + "/triggers": "automation_triggers.json", "/v1/automations/" + auto + "/deliveries": "automation_deliveries.json", } + var mu sync.Mutex + var seen []string srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + mu.Lock() + seen = append(seen, r.Method+" "+r.URL.Path) + mu.Unlock() + if r.URL.Path == "/api/workspaces" && r.Method == http.MethodGet && workspaces != "" { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(workspaces)) + return + } name, ok := routes[r.URL.Path] if !ok || r.Method != http.MethodGet { http.NotFound(w, r) @@ -43,7 +63,11 @@ func goldenEnv(t *testing.T) (*runsEnv, *bytes.Buffer, *bytes.Buffer) { t.Cleanup(srv.Close) var out, errOut bytes.Buffer env := &runsEnv{client: api.NewRunsClient("s", "").WithBaseURL(srv.URL), stdout: &out, stderr: &errOut} - return env, &out, &errOut + return env, &out, &errOut, func() []string { + mu.Lock() + defer mu.Unlock() + return append([]string(nil), seen...) + } } func wantAll(t *testing.T, what, out string, wants ...string) { @@ -66,7 +90,8 @@ func TestGoldenRender(t *testing.T) { if err := doRunsShow(ctx, env, run); err != nil { t.Fatal(err) } - wantAll(t, "runs show", out.String(), "awaiting_approval", "propose", "a0000000-0000-4000-8000-000000000001", + wantAll(t, "runs show", out.String(), "awaiting_approval", "propose", "PR review api (a0000000-0000-4000-8000-000000000001)", + "develop ("+ws+")", "acme/api", "GITHUB_TOKEN (the agent)", "pending", "3 actions", "d0000000-0000-4000-8000-000000000004", "github_pr_review ok", "nan runs reject "+run, "/workspaces/"+ws+"/runs/"+run) @@ -88,7 +113,7 @@ func TestGoldenRender(t *testing.T) { if err := doApprovalsLs(ctx, env, api.ApprovalPending); err != nil { t.Fatal(err) } - wantAll(t, "approvals ls", out.String(), run, "pending", "a0000000-0000-4000-8000-000000000001", "github_pr_review") + wantAll(t, "approvals ls", out.String(), run, "pending", "PR review api", "github_pr_review") if strings.Contains(out.String(), "github_pr_comment") { t.Errorf("approvals ls lists an action that applied automatically:\n%s", out.String()) } @@ -99,6 +124,30 @@ func TestGoldenRender(t *testing.T) { } wantAll(t, "automations ls", out.String(), "PR review api", ws, "enabled", "github_pr_review") + // With workspaces:read the workspace is named, from one lookup. + const workspaces = `[{"workspace_uuid":"` + ws + `","name":"develop","status":"running"}]` + env, out, _, seen := goldenEnvWith(t, workspaces) + if err := doAutomationsLs(ctx, env); err != nil { + t.Fatal(err) + } + if lines := strings.Split(out.String(), "\n"); len(lines) < 2 || !strings.Contains(lines[1], "develop") || strings.Contains(lines[1], ws) { + t.Errorf("automations ls does not name the workspace:\n%s", out.String()) + } + env, out, _, seen = goldenEnvWith(t, workspaces) + if err := doAutomationsShow(ctx, env, "PR review api"); err != nil { + t.Fatal(err) + } + wantAll(t, "automations show", out.String(), "develop ("+ws+")") + n := 0 + for _, r := range seen() { + if r == "GET /api/workspaces" { + n++ + } + } + if n != 1 { + t.Errorf("automations show: GET /api/workspaces %d times, want 1: %v", n, seen()) + } + env, out, _ = goldenEnv(t) if err := doAutomationsShow(ctx, env, "PR review api"); err != nil { t.Fatal(err) diff --git a/cmd/names.go b/cmd/names.go new file mode 100644 index 0000000..27146d6 --- /dev/null +++ b/cmd/names.go @@ -0,0 +1,108 @@ +package cmd + +import ( + "context" + "strings" + "time" + + "github.com/nxssie/nan-cli/internal/api" +) + +// The platform names a run's or an approval's workspace and automation by id +// only. People look for names, so the text views look the names up here, +// once per command, and show the id when that is not possible: a platform +// token without workspaces:read, a network error, an id that is not in the +// list. A name is a nicety; it never fails a command, and --json output +// never asks for one (it stays the API's own shape). + +// nameLookupTimeout bounds each lookup, so a slow answer to a question the +// command does not need answered cannot hold up the one it does. +const nameLookupTimeout = 3 * time.Second + +// nameCache holds the names a command looked up. The zero value is ready: +// each list is fetched the first time a name from it is asked for, and +// never again, whatever the answer was. +type nameCache struct { + workspaces, automations map[string]string + workspacesTried, automationsTried bool +} + +func lookupKey(id string) string { return strings.ToLower(strings.TrimSpace(id)) } + +// workspaceName is the name of the member's workspace with this id, or "" +// when it cannot be known. +func (env *runsEnv) workspaceName(ctx context.Context, id string) string { + if lookupKey(id) == "" { + return "" + } + if !env.names.workspacesTried { + env.names.workspacesTried = true + lctx, cancel := context.WithTimeout(ctx, nameLookupTimeout) + defer cancel() + if list, err := env.client.ListWorkspaces(lctx); err == nil { + env.names.workspaces = make(map[string]string, len(list)) + for _, w := range list { + env.names.workspaces[lookupKey(w.ID)] = w.Name + } + } + } + return env.names.workspaces[lookupKey(id)] +} + +// automationName is the name of the member's automation with this id, or "" +// when it cannot be known. +func (env *runsEnv) automationName(ctx context.Context, id string) string { + if lookupKey(id) == "" { + return "" + } + if !env.names.automationsTried { + env.names.automationsTried = true + lctx, cancel := context.WithTimeout(ctx, nameLookupTimeout) + defer cancel() + if list, err := env.client.ListAutomations(lctx); err == nil { + env.names.automations = make(map[string]string, len(list.Data)) + for _, a := range list.Data { + env.names.automations[lookupKey(a.ID)] = a.Name + } + } + } + return env.names.automations[lookupKey(id)] +} + +// nameWorkspace fills in a workspace reference's name when the platform sent +// only its id. +func (env *runsEnv) nameWorkspace(ctx context.Context, r *api.Ref) { + if r != nil && r.Name == "" { + r.Name = env.workspaceName(ctx, r.ID) + } +} + +// nameAutomation fills in an automation reference's name when the platform +// sent only its id. +func (env *runsEnv) nameAutomation(ctx context.Context, r *api.Ref) { + if r != nil && r.Name == "" { + r.Name = env.automationName(ctx, r.ID) + } +} + +// nameRunWorkspace fills in a run's workspace name when it has none (the +// platform sends it, except for a workspace deleted since). +func (env *runsEnv) nameRunWorkspace(ctx context.Context, run *api.Run) { + if run.Workspace.Name == "" { + run.Workspace.Name = env.workspaceName(ctx, run.Workspace.ID) + } +} + +// nameRun fills in the names a run's text views show. +func (env *runsEnv) nameRun(ctx context.Context, run *api.Run) { + env.nameRunWorkspace(ctx, run) + env.nameAutomation(ctx, run.Automation) +} + +// refLabel is a name for a table: the name when it is known, else the id. +func refLabel(name, id string) string { + if name != "" { + return name + } + return id +} diff --git a/cmd/names_test.go b/cmd/names_test.go new file mode 100644 index 0000000..1e4aec9 --- /dev/null +++ b/cmd/names_test.go @@ -0,0 +1,480 @@ +package cmd + +import ( + "bytes" + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "sync" + "testing" + + "github.com/nxssie/nan-cli/internal/api" +) + +// The platform's runs, approvals and automations name their workspace and +// automation by id. The text views look the names up (GET /api/workspaces, +// GET /v1/automations) at most once per command, show "name (id)" in detail +// views and the name in tables, fall back to the id when the lookup fails, +// and never look anything up for --json, whose output stays the API's shape. + +const ( + namesRun = "6f1c2a9b-0000-4000-8000-000000000001" + namesAuto = "a0000000-0000-4000-8000-000000000001" + namesWS = "11111111-2222-4333-8444-555555555555" + + namesWorkspaces = `[{"workspace_uuid":"` + namesWS + `","name":"develop","status":"running"}, +{"workspace_uuid":"22222222-2222-4333-8444-555555555555","name":"other","status":"stopped"}]` + + scopeDenied = `{"error":{"message":"this token lacks the workspaces:read scope","type":"permission_error","code":"token_scope_denied"}}` +) + +// answer is a canned response; drop closes the connection without one. +type answer struct { + status int + body string + drop bool +} + +// namesServer serves the phase 2 golden responses, with overrides per path +// ("" body for /api/workspaces means a 404), and records every request. +type namesServer struct { + t *testing.T + overrides map[string]answer + + mu sync.Mutex + seen []string +} + +func goldenBody(t *testing.T, name string) string { + t.Helper() + b, err := os.ReadFile(filepath.Join("..", "internal", "api", "testdata", "phase2", name)) + if err != nil { + t.Fatal(err) + } + return string(b) +} + +func (s *namesServer) ServeHTTP(w http.ResponseWriter, r *http.Request) { + s.mu.Lock() + s.seen = append(s.seen, r.Method+" "+r.URL.Path) + s.mu.Unlock() + if a, ok := s.overrides[r.URL.Path]; ok { + if a.drop { + conn, _, err := w.(http.Hijacker).Hijack() + if err == nil { + _ = conn.Close() + } + return + } + w.Header().Set("Content-Type", "application/json") + if a.status != 0 { + w.WriteHeader(a.status) + } + _, _ = w.Write([]byte(a.body)) + return + } + routes := map[string]string{ + "/v1/runs/" + namesRun: "run_get_propose.json", + "/v1/runs": "run_list_propose.json", + "/v1/approvals": "approvals_list.json", + "/v1/automations": "automations_list.json", + "/v1/automations/" + namesAuto: "automation_get.json", + "/v1/automations/" + namesAuto + "/triggers": "automation_triggers.json", + } + name, ok := routes[r.URL.Path] + if !ok || r.Method != http.MethodGet { + http.NotFound(w, r) + return + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(goldenBody(s.t, name))) +} + +func (s *namesServer) count(req string) int { + s.mu.Lock() + defer s.mu.Unlock() + n := 0 + for _, r := range s.seen { + if r == req { + n++ + } + } + return n +} + +func (s *namesServer) requests() []string { + s.mu.Lock() + defer s.mu.Unlock() + return append([]string(nil), s.seen...) +} + +func newNamesEnv(t *testing.T, overrides map[string]answer) (*runsEnv, *namesServer, *bytes.Buffer, *bytes.Buffer) { + t.Helper() + resetPhase2Flags() + t.Cleanup(resetPhase2Flags) + s := &namesServer{t: t, overrides: overrides} + srv := httptest.NewServer(s) + t.Cleanup(srv.Close) + var out, errOut bytes.Buffer + env := &runsEnv{client: api.NewRunsClient("", "nan_pat_test").WithBaseURL(srv.URL), stdout: &out, stderr: &errOut} + return env, s, &out, &errOut +} + +func withWorkspaces() map[string]answer { + return map[string]answer{"/api/workspaces": {body: namesWorkspaces}} +} + +// lineOf is the line of out that starts with key, for the detail views. +func lineOf(t *testing.T, out, key string) string { + t.Helper() + for _, l := range strings.Split(out, "\n") { + if strings.HasPrefix(l, key+" ") { + return strings.Join(strings.Fields(l), " ") + } + } + t.Fatalf("no %q line in:\n%s", key, out) + return "" +} + +// tableRow is the first data row of a table, its columns split on runs of +// spaces (no golden value has two spaces inside it). +func tableRow(t *testing.T, out string) string { + t.Helper() + lines := strings.Split(strings.TrimSpace(out), "\n") + if len(lines) < 2 { + t.Fatalf("no table row in:\n%s", out) + } + return lines[1] +} + +func TestNamesAutomationsLs(t *testing.T) { + env, s, out, _ := newNamesEnv(t, withWorkspaces()) + if err := doAutomationsLs(context.Background(), env); err != nil { + t.Fatal(err) + } + row := tableRow(t, out.String()) + if !strings.Contains(row, "develop") || strings.Contains(row, namesWS) { + t.Errorf("automations ls row shows no workspace name:\n%s", out.String()) + } + if n := s.count("GET /api/workspaces"); n != 1 { + t.Errorf("GET /api/workspaces %d times, want 1: %v", n, s.requests()) + } + if n := s.count("GET /v1/automations"); n != 1 { + t.Errorf("GET /v1/automations %d times, want 1 (the list itself): %v", n, s.requests()) + } +} + +func TestNamesAutomationsShow(t *testing.T) { + env, s, out, _ := newNamesEnv(t, withWorkspaces()) + if err := doAutomationsShow(context.Background(), env, namesAuto); err != nil { + t.Fatal(err) + } + if got, want := lineOf(t, out.String(), "workspace"), "workspace develop ("+namesWS+")"; got != want { + t.Errorf("workspace line = %q, want %q", got, want) + } + if n := s.count("GET /api/workspaces"); n != 1 { + t.Errorf("GET /api/workspaces %d times, want 1: %v", n, s.requests()) + } + if n := s.count("GET /v1/automations"); n != 0 { + t.Errorf("show by id lists the automations: %v", s.requests()) + } +} + +func TestNamesApprovalsLs(t *testing.T) { + env, s, out, _ := newNamesEnv(t, withWorkspaces()) + if err := doApprovalsLs(context.Background(), env, api.ApprovalPending); err != nil { + t.Fatal(err) + } + row := tableRow(t, out.String()) + if !strings.Contains(row, "PR review api") || strings.Contains(row, namesAuto) { + t.Errorf("approvals ls row shows no automation name:\n%s", out.String()) + } + if n := s.count("GET /v1/automations"); n != 1 { + t.Errorf("GET /v1/automations %d times, want 1: %v", n, s.requests()) + } + if n := s.count("GET /api/workspaces"); n != 0 { + t.Errorf("approvals ls looks up workspaces it does not show: %v", s.requests()) + } +} + +func TestNamesRunsShowAndProposal(t *testing.T) { + for _, tc := range []struct { + name string + do func(*runsEnv) error + }{ + {"show", func(env *runsEnv) error { return doRunsShow(context.Background(), env, namesRun) }}, + {"proposal", func(env *runsEnv) error { return doRunsProposal(context.Background(), env, namesRun, false, false) }}, + } { + t.Run(tc.name, func(t *testing.T) { + env, s, out, _ := newNamesEnv(t, withWorkspaces()) + if err := tc.do(env); err != nil { + t.Fatal(err) + } + if got, want := lineOf(t, out.String(), "automation"), "automation PR review api ("+namesAuto+")"; got != want { + t.Errorf("automation line = %q, want %q", got, want) + } + if n := s.count("GET /v1/automations"); n != 1 { + t.Errorf("GET /v1/automations %d times, want 1: %v", n, s.requests()) + } + // The run names its workspace itself: nothing to look up. + if n := s.count("GET /api/workspaces"); n != 0 { + t.Errorf("looked up a workspace name the run carries: %v", s.requests()) + } + }) + } +} + +func TestNamesRunsShowWorkspace(t *testing.T) { + env, _, out, _ := newNamesEnv(t, withWorkspaces()) + if err := doRunsShow(context.Background(), env, namesRun); err != nil { + t.Fatal(err) + } + if got, want := lineOf(t, out.String(), "workspace"), "workspace develop ("+namesWS+")"; got != want { + t.Errorf("workspace line = %q, want %q", got, want) + } +} + +// runListWithoutWorkspaceName is the golden run list with the workspace's name +// gone, as for a run whose workspace was deleted or renamed away. +func runListWithoutWorkspaceName(t *testing.T) string { + t.Helper() + var list map[string]any + if err := json.Unmarshal([]byte(goldenBody(t, "run_list_propose.json")), &list); err != nil { + t.Fatal(err) + } + run := list["data"].([]any)[0].(map[string]any) + run["workspace"] = map[string]any{"id": namesWS} + b, err := json.Marshal(list) + if err != nil { + t.Fatal(err) + } + return string(b) +} + +func TestNamesRunsLsLooksUpOnlyWhatIsMissing(t *testing.T) { + env, s, out, _ := newNamesEnv(t, withWorkspaces()) + if err := doRunsLs(context.Background(), env); err != nil { + t.Fatal(err) + } + if !strings.Contains(tableRow(t, out.String()), "develop") || s.count("GET /api/workspaces") != 0 { + t.Errorf("runs ls with named workspaces: %v\n%s", s.requests(), out.String()) + } + + overrides := withWorkspaces() + overrides["/v1/runs"] = answer{body: runListWithoutWorkspaceName(t)} + env, s, out, _ = newNamesEnv(t, overrides) + if err := doRunsLs(context.Background(), env); err != nil { + t.Fatal(err) + } + if !strings.Contains(tableRow(t, out.String()), "develop") || s.count("GET /api/workspaces") != 1 { + t.Errorf("runs ls did not name the workspace: %v\n%s", s.requests(), out.String()) + } +} + +// Every way the lookups can fail leaves the command working, with the ids. +func TestNamesFallBackToTheID(t *testing.T) { + failures := map[string]answer{ + "403 token_scope_denied": {status: http.StatusForbidden, body: scopeDenied}, + "500": {status: http.StatusInternalServerError, body: `{"error":"boom"}`}, + "401": {status: http.StatusUnauthorized, body: `{"error":"invalid session"}`}, + "not JSON": {body: ``}, + "connection dropped": {drop: true}, + } + for what, fail := range failures { + t.Run(what, func(t *testing.T) { + // automations ls: the list itself works, the workspaces do not. + env, _, out, errOut := newNamesEnv(t, map[string]answer{"/api/workspaces": fail}) + if err := doAutomationsLs(context.Background(), env); err != nil { + t.Fatalf("automations ls failed: %v", err) + } + if !strings.Contains(tableRow(t, out.String()), namesWS) || errOut.Len() != 0 { + t.Errorf("automations ls fallback:\n%s\nstderr: %s", out.String(), errOut.String()) + } + + env, _, out, errOut = newNamesEnv(t, map[string]answer{"/api/workspaces": fail}) + if err := doAutomationsShow(context.Background(), env, namesAuto); err != nil { + t.Fatalf("automations show failed: %v", err) + } + if got := lineOf(t, out.String(), "workspace"); got != "workspace "+namesWS || errOut.Len() != 0 { + t.Errorf("automations show workspace line = %q, stderr %q", got, errOut.String()) + } + + // approvals ls and the run views: the automations list fails. + env, _, out, _ = newNamesEnv(t, map[string]answer{"/v1/automations": fail}) + if err := doApprovalsLs(context.Background(), env, api.ApprovalPending); err != nil { + t.Fatalf("approvals ls failed: %v", err) + } + if !strings.Contains(tableRow(t, out.String()), namesAuto) { + t.Errorf("approvals ls fallback:\n%s", out.String()) + } + + env, _, out, errOut = newNamesEnv(t, map[string]answer{"/v1/automations": fail}) + if err := doRunsShow(context.Background(), env, namesRun); err != nil { + t.Fatalf("runs show failed: %v", err) + } + if got := lineOf(t, out.String(), "automation"); got != "automation "+namesAuto || errOut.Len() != 0 { + t.Errorf("runs show automation line = %q, stderr %q", got, errOut.String()) + } + + env, _, out, _ = newNamesEnv(t, map[string]answer{"/v1/automations": fail}) + if err := doRunsProposal(context.Background(), env, namesRun, false, false); err != nil { + t.Fatalf("runs proposal failed: %v", err) + } + if got := lineOf(t, out.String(), "automation"); got != "automation "+namesAuto { + t.Errorf("runs proposal automation line = %q", got) + } + }) + } +} + +// An id the lookup does not know (an automation deleted since) is shown as +// the id, after the one lookup. +func TestNamesUnknownIDFallsBack(t *testing.T) { + env, s, out, _ := newNamesEnv(t, map[string]answer{"/api/workspaces": {body: `[{"workspace_uuid":"99999999-2222-4333-8444-555555555555","name":"x"}]`}}) + if err := doAutomationsShow(context.Background(), env, namesAuto); err != nil { + t.Fatal(err) + } + if got := lineOf(t, out.String(), "workspace"); got != "workspace "+namesWS { + t.Errorf("workspace line = %q", got) + } + if n := s.count("GET /api/workspaces"); n != 1 { + t.Errorf("GET /api/workspaces %d times: %v", n, s.requests()) + } +} + +// A name is agent-adjacent text from the platform: it is sanitised before +// it reaches the terminal. +func TestNamesAreSanitised(t *testing.T) { + env, _, out, _ := newNamesEnv(t, map[string]answer{"/api/workspaces": {body: `[{"workspace_uuid":"` + namesWS + `","name":"dev\u001b[2Jelop\u0007"}]`}}) + if err := doAutomationsShow(context.Background(), env, namesAuto); err != nil { + t.Fatal(err) + } + assertNoControls(t, "automations show", out.String()) +} + +// --json prints the API's shape, with no lookup: the same bytes whether or +// not the names could be had, and only the command's own requests. +func TestNamesNeverWithJSON(t *testing.T) { + cases := []struct { + name string + set func() + do func(*runsEnv) error + want []string + }{ + {"automations ls", func() { autoLsJSON = true }, + func(env *runsEnv) error { return doAutomationsLs(context.Background(), env) }, + []string{"GET /v1/automations"}}, + {"automations show", func() { autoShowJSON = true }, + func(env *runsEnv) error { return doAutomationsShow(context.Background(), env, namesAuto) }, + []string{"GET /v1/automations/" + namesAuto}}, + {"approvals ls", func() { approvalsJSON = true }, + func(env *runsEnv) error { return doApprovalsLs(context.Background(), env, api.ApprovalPending) }, + []string{"GET /v1/approvals"}}, + {"runs show", func() { showJSON = true }, + func(env *runsEnv) error { return doRunsShow(context.Background(), env, namesRun) }, + []string{"GET /v1/runs/" + namesRun}}, + {"runs proposal", func() {}, + func(env *runsEnv) error { return doRunsProposal(context.Background(), env, namesRun, false, true) }, + []string{"GET /v1/runs/" + namesRun}}, + {"runs ls", func() { lsJSON = true }, + func(env *runsEnv) error { return doRunsLs(context.Background(), env) }, + []string{"GET /v1/runs"}}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + overrides := withWorkspaces() + overrides["/v1/runs"] = answer{body: runListWithoutWorkspaceName(t)} + env, s, out, _ := newNamesEnv(t, overrides) + tc.set() + if err := tc.do(env); err != nil { + t.Fatal(err) + } + if got := s.requests(); strings.Join(got, ",") != strings.Join(tc.want, ",") { + t.Errorf("requests = %v, want %v", got, tc.want) + } + // The same command where no name can be had prints the same. + noNames := map[string]answer{ + "/api/workspaces": {status: http.StatusForbidden, body: scopeDenied}, + "/v1/runs": overrides["/v1/runs"], + } + env2, _, out2, _ := newNamesEnv(t, noNames) + tc.set() + if err := tc.do(env2); err != nil { + t.Fatal(err) + } + if out.String() != out2.String() { + t.Errorf("--json differs with names available:\n%s\nvs\n%s", out.String(), out2.String()) + } + }) + } +} + +// Many rows, several ids: still one lookup per list. +func TestNamesOneLookupPerCommand(t *testing.T) { + const otherWS = "22222222-2222-4333-8444-555555555555" + var autos map[string]any + if err := json.Unmarshal([]byte(goldenBody(t, "automations_list.json")), &autos); err != nil { + t.Fatal(err) + } + first := autos["data"].([]any)[0].(map[string]any) + var data []any + for i, ws := range []string{namesWS, otherWS, namesWS, "33333333-2222-4333-8444-555555555555"} { + a := map[string]any{} + for k, v := range first { + a[k] = v + } + a["id"] = strings.Replace(namesAuto, "1", string(rune('1'+i)), 1) + a["workspace"] = ws + data = append(data, a) + } + autos["data"] = data + autosBody, _ := json.Marshal(autos) + + overrides := withWorkspaces() + overrides["/v1/automations"] = answer{body: string(autosBody)} + env, s, out, _ := newNamesEnv(t, overrides) + if err := doAutomationsLs(context.Background(), env); err != nil { + t.Fatal(err) + } + if n := s.count("GET /api/workspaces"); n != 1 { + t.Errorf("GET /api/workspaces %d times for 4 rows, want 1: %v", n, s.requests()) + } + wantAll(t, "automations ls", out.String(), "develop", "other", "33333333-2222-4333-8444-555555555555") + + var approvals map[string]any + if err := json.Unmarshal([]byte(goldenBody(t, "approvals_list.json")), &approvals); err != nil { + t.Fatal(err) + } + item := approvals["data"].([]any)[0] + approvals["data"] = []any{item, item, item} + approvalsBody, _ := json.Marshal(approvals) + env, s, out, _ = newNamesEnv(t, map[string]answer{ + "/v1/approvals": {body: string(approvalsBody)}, + "/v1/automations": {status: http.StatusForbidden, body: scopeDenied}, + }) + if err := doApprovalsLs(context.Background(), env, api.ApprovalPending); err != nil { + t.Fatal(err) + } + if n := s.count("GET /v1/automations"); n != 1 { + t.Errorf("GET /v1/automations %d times for 3 rows after a refusal, want 1: %v", n, s.requests()) + } + if strings.Count(out.String(), namesAuto) != 3 { + t.Errorf("approvals ls fallback:\n%s", out.String()) + } +} + +// Ids compare without regard to case: the platform's and the list's may +// differ in it. +func TestNamesIDsIgnoreCase(t *testing.T) { + env, _, out, _ := newNamesEnv(t, map[string]answer{"/api/workspaces": {body: `[{"workspace_uuid":"` + strings.ToUpper(namesWS) + `","name":"develop"}]`}}) + if err := doAutomationsShow(context.Background(), env, namesAuto); err != nil { + t.Fatal(err) + } + if got, want := lineOf(t, out.String(), "workspace"), "workspace develop ("+namesWS+")"; got != want { + t.Errorf("workspace line = %q, want %q", got, want) + } +} diff --git a/cmd/run.go b/cmd/run.go index 732223f..9d9f38c 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -163,6 +163,10 @@ type runsEnv struct { stdout io.Writer stderr io.Writer interrupts <-chan os.Signal + + // names caches the workspace and automation names the text views + // look up (names.go). + names nameCache } func newRunsEnv() (*runsEnv, error) { diff --git a/cmd/runs.go b/cmd/runs.go index b521a73..84bd303 100644 --- a/cmd/runs.go +++ b/cmd/runs.go @@ -44,7 +44,8 @@ var runsCmd = &cobra.Command{ the portal, or by your automations. These commands authenticate the same way as nan run: NAN_TOKEN, --token-file PATH, a token saved with nan auth login --api-token, or your nan auth login session (the full order is in -nan run --help). +nan run --help). Automation names are looked up with automations:read; +without it the automation's id is shown instead. A run of an automation can stop in awaiting_approval: the agent is done and the actions it proposed (a push, a pull request, a comment) wait for you. @@ -280,13 +281,16 @@ func doRunsLs(ctx context.Context, env *runsEnv) error { } return nil } + for i := range list.Data { + env.nameRunWorkspace(ctx, &list.Data[i]) + } tw := tabwriter.NewWriter(env.stdout, 0, 0, 2, ' ', 0) fmt.Fprintln(tw, "ID\tSTATE\tWORKSPACE\tAGENT\tCREATED\tPROMPT") for _, r := range list.Data { fmt.Fprintf(tw, "%s\t%s\t%s\t%s\t%s\t%s\n", runs.SanitizeLine(r.ID), runs.SanitizeLine(r.State), - runs.SanitizeLine(r.Workspace.Name), + runs.Truncate(runs.SanitizeLine(refLabel(r.Workspace.Name, r.Workspace.ID)), 40), runs.SanitizeLine(r.Agent), localTime(r.CreatedAt), runs.Truncate(runs.SanitizeLine(r.PromptPreview), 60)) @@ -323,6 +327,7 @@ func doRunsShow(ctx context.Context, env *runsEnv, id string) error { if showJSON { return printJSON(env.stdout, run, true) } + env.nameRun(ctx, run) tw := tabwriter.NewWriter(env.stdout, 0, 0, 2, ' ', 0) row := func(k, v string) { @@ -345,7 +350,7 @@ func doRunsShow(ctx context.Context, env *runsEnv, id string) error { if run.ExitCode != nil { row("exit code", fmt.Sprint(*run.ExitCode)) } - row("workspace", runs.SanitizeLine(run.Workspace.Name)) + row("workspace", runs.NameAndID(run.Workspace.Name, run.Workspace.ID)) agent := runs.SanitizeLine(run.Agent) if run.Model != nil && *run.Model != "" { agent += " · model " + runs.SanitizeLine(*run.Model) @@ -356,7 +361,7 @@ func doRunsShow(ctx context.Context, env *runsEnv, id string) error { row("phase", runs.SanitizeLine(run.Phase)+phaseNote(run.Phase)) } if run.Automation != nil { - row("automation", runs.SanitizeLine(run.Automation.Label())) + row("automation", runs.NameAndID(run.Automation.Name, run.Automation.ID)) } if run.GitHubRepo != nil { row("repository", runs.SanitizeLine(*run.GitHubRepo)) @@ -569,6 +574,7 @@ func doRunsProposal(ctx context.Context, env *runsEnv, id string, withDiff, asJS if asJSON { return printJSON(env.stdout, run.Approval, true) } + env.nameAutomation(ctx, run.Automation) runs.Proposal(env.stdout, run) if !withDiff { return nil diff --git a/internal/runs/proposal_test.go b/internal/runs/proposal_test.go index a6c8b2b..63f4c23 100644 --- a/internal/runs/proposal_test.go +++ b/internal/runs/proposal_test.go @@ -129,3 +129,18 @@ func TestDiffIsSanitised(t *testing.T) { t.Errorf("%q", b.String()) } } + +func TestNameAndID(t *testing.T) { + const id = "a0000000-0000-4000-8000-000000000001" + for _, tc := range []struct{ name, id, want string }{ + {"PR review", id, "PR review (" + id + ")"}, + {"", id, id}, + {"PR review", "", "PR review"}, + {id, id, id}, + {"bad\x1b[2Jname", id, "badname (" + id + ")"}, + } { + if got := NameAndID(tc.name, tc.id); got != tc.want { + t.Errorf("NameAndID(%q, %q) = %q, want %q", tc.name, tc.id, got, tc.want) + } + } +} diff --git a/internal/runs/render.go b/internal/runs/render.go index 001abd3..ce4d568 100644 --- a/internal/runs/render.go +++ b/internal/runs/render.go @@ -119,6 +119,20 @@ func Truncate(s string, n int) string { return string(runes[:n-1]) + "…" } +// NameAndID shows a named object in a detail view: "name (id)" when the +// name is known, so the id stays there to copy, and the id alone when it is +// not. Both are sanitised. +func NameAndID(name, id string) string { + name, id = SanitizeLine(name), SanitizeLine(id) + switch { + case name == "": + return id + case id == "" || name == id: + return name + } + return name + " (" + id + ")" +} + // Renderer prints a run's events as text. It remembers whether the last // thing printed left the cursor mid-line, because assistant text arrives in // chunks and the next event must not be glued onto the end of one. @@ -448,7 +462,7 @@ func Proposal(w io.Writer, run *api.Run) { row("run", id+" ("+strings.ReplaceAll(SanitizeLine(run.State), "_", " ")+")") if run.Automation != nil { - row("automation", SanitizeLine(run.Automation.Label())) + row("automation", NameAndID(run.Automation.Name, run.Automation.ID)) } state := SanitizeLine(a.State) switch a.State { diff --git a/internal/tui/tui.go b/internal/tui/tui.go index 892f7a0..7b3bb55 100644 --- a/internal/tui/tui.go +++ b/internal/tui/tui.go @@ -3559,7 +3559,7 @@ func (m model) renderSetup(l layout) string { // ── about renderer ─────────────────────────────────────────────────────────── -const Version = "0.1.26" +const Version = "0.1.27" func renderAbout(l layout) string { var b strings.Builder