diff --git a/internal/commands/jobs.go b/internal/commands/jobs.go index 11c07aa..98573c0 100644 --- a/internal/commands/jobs.go +++ b/internal/commands/jobs.go @@ -984,47 +984,56 @@ func (r *jobsRunner) followLogs(cmd *cobra.Command, runID string, intervalSec fl } // completeRuntimes and completeRegions provide shell completion for the -// --runtime and --run-region flags, sourced from the organization's available -// runtimes/regions via GET /notebooks/config-hint. They fail open (no -// suggestions) so completion never blocks or errors out. +// --runtime and --run-region flags. Both fail open (no suggestions) so +// completion never blocks or errors out. func (r *jobsRunner) completeRuntimes(cmd *cobra.Command, _ []string, _ string) ([]string, cobra.ShellCompDirective) { - return r.completeConfigHint(cmd, "runtimes.hint", "id"), cobra.ShellCompDirectiveNoFileComp -} - -func (r *jobsRunner) completeRegions(cmd *cobra.Command, _ []string, _ string) ([]string, cobra.ShellCompDirective) { - return r.completeConfigHint(cmd, "regions.hint", "regionName"), cobra.ShellCompDirectiveNoFileComp -} - -// completeConfigHint fetches the config hint and returns the value of valueKey -// for each enabled item under arrayPath (e.g. "runtimes.hint" / "id"). -func (r *jobsRunner) completeConfigHint(cmd *cobra.Command, arrayPath, valueKey string) []string { if r.getConfigHint == nil { - return nil + return nil, cobra.ShellCompDirectiveNoFileComp } - ctx := cmd.Context() - if ctx == nil { - ctx = context.Background() - } - ctx, cancel := context.WithTimeout(ctx, 5*time.Second) - defer cancel() - - body, err := r.execOnce(ctx, r.getConfigHint, nil, nil, "") + body, err := r.fetchForCompletion(cmd, r.getConfigHint) if err != nil { - return nil + return nil, cobra.ShellCompDirectiveNoFileComp } - var values []string - gjson.GetBytes(body, arrayPath).ForEach(func(_, item gjson.Result) bool { - // Skip items the org cannot use; be lenient when "enabled" is absent. + gjson.GetBytes(body, "runtimes.hint").ForEach(func(_, item gjson.Result) bool { + // Skip runtimes the org cannot use; be lenient when "enabled" is absent. if item.Get("enabled").Exists() && !item.Get("enabled").Bool() { return true } - if v := strings.TrimSpace(item.Get(valueKey).String()); v != "" { + if v := strings.TrimSpace(item.Get("id").String()); v != "" { + values = append(values, v) + } + return true + }) + return values, cobra.ShellCompDirectiveNoFileComp +} + +func (r *jobsRunner) completeRegions(cmd *cobra.Command, _ []string, _ string) ([]string, cobra.ShellCompDirective) { + if r.getOrganization == nil { + return nil, cobra.ShellCompDirectiveNoFileComp + } + body, err := r.fetchForCompletion(cmd, r.getOrganization) + if err != nil { + return nil, cobra.ShellCompDirectiveNoFileComp + } + var values []string + gjson.GetBytes(body, "allowedRegions").ForEach(func(_, item gjson.Result) bool { + if v := strings.TrimSpace(item.String()); v != "" { values = append(values, v) } return true }) - return values + return values, cobra.ShellCompDirectiveNoFileComp +} + +func (r *jobsRunner) fetchForCompletion(cmd *cobra.Command, op *spec.Operation) ([]byte, error) { + ctx := cmd.Context() + if ctx == nil { + ctx = context.Background() + } + ctx, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() + return r.execOnce(ctx, op, nil, nil, "") } func (r *jobsRunner) execOnce(ctx context.Context, op *spec.Operation, pathArgs []string, query []executor.QueryPair, body string) ([]byte, error) { diff --git a/internal/commands/jobs_test.go b/internal/commands/jobs_test.go index 2de1c6d..e3e8126 100644 --- a/internal/commands/jobs_test.go +++ b/internal/commands/jobs_test.go @@ -851,6 +851,84 @@ func TestBuildRunPayloadRejectsBadDependency(t *testing.T) { } } +func TestCompleteRegionsReturnsAllowedRegions(t *testing.T) { + t.Parallel() + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodGet && r.URL.Path == "/organization" { + w.Header().Set("Content-Type", "application/json") + _, _ = io.WriteString(w, `{"allowedRegions":["aws-us-west-2","aws-eu-west-1","byoc-acme-us-east-1"]}`) + return + } + http.NotFound(w, r) + })) + defer server.Close() + + runner := newJobsRunnerForTest(server.URL) + cmd := &cobra.Command{} + got, _ := runner.completeRegions(cmd, nil, "") + want := []string{"aws-us-west-2", "aws-eu-west-1", "byoc-acme-us-east-1"} + if !equalStrings(got, want) { + t.Fatalf("completeRegions = %v, want %v", got, want) + } +} + +func TestCompleteRegionsFailsOpenOnError(t *testing.T) { + t.Parallel() + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + })) + defer server.Close() + + runner := newJobsRunnerForTest(server.URL) + cmd := &cobra.Command{} + got, _ := runner.completeRegions(cmd, nil, "") + if got != nil { + t.Fatalf("expected nil on error (fail open), got %v", got) + } +} + +func TestCompleteRuntimesSkipsDisabled(t *testing.T) { + t.Parallel() + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodGet && r.URL.Path == "/notebooks/config-hint" { + w.Header().Set("Content-Type", "application/json") + _, _ = io.WriteString(w, `{"runtimes":{"hint":[{"id":"tiny","enabled":true},{"id":"small"},{"id":"x-large","enabled":false}]}}`) + return + } + http.NotFound(w, r) + })) + defer server.Close() + + runner := newJobsRunnerForTest(server.URL) + cmd := &cobra.Command{} + got, _ := runner.completeRuntimes(cmd, nil, "") + want := []string{"tiny", "small"} + if !equalStrings(got, want) { + t.Fatalf("completeRuntimes = %v, want %v", got, want) + } +} + +func newJobsRunnerForTest(baseURL string) *jobsRunner { + cfg := config.Config{AppName: "wherobots", APIKey: "test-key", HTTPTimeout: time.Second} + runner, _ := newJobsRunner(cfg, jobsTestRuntimeSpec(baseURL), http.DefaultClient) + return runner +} + +func equalStrings(a, b []string) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} + func buildJobsTestRoot(baseURL string) *cobra.Command { return buildJobsTestRootWithConfig(baseURL, nil) } @@ -864,7 +942,11 @@ func buildJobsTestRootWithConfig(baseURL string, mutate func(*config.Config)) *c if mutate != nil { mutate(&cfg) } - runtime := &spec.RuntimeSpec{ + return BuildRootCommand(cfg, jobsTestRuntimeSpec(baseURL)) +} + +func jobsTestRuntimeSpec(baseURL string) *spec.RuntimeSpec { + return &spec.RuntimeSpec{ BaseURL: baseURL, Operations: []*spec.Operation{ { @@ -893,6 +975,10 @@ func buildJobsTestRootWithConfig(baseURL string, mutate func(*config.Config)) *c {Name: "key", Location: "query", Required: true, Type: "string"}, }, }, + { + Method: "GET", + Path: "/notebooks/config-hint", + }, { Method: "POST", Path: "/runs", @@ -926,7 +1012,6 @@ func buildJobsTestRootWithConfig(baseURL string, mutate func(*config.Config)) *c }, }, } - return BuildRootCommand(cfg, runtime) } func serverURLFromRequest(r *http.Request) string {