From 707d4bd83eb5fc2330e1e96856b7541d48c24a1e Mon Sep 17 00:00:00 2001 From: pratikbin <68642400+pratikbin@users.noreply.github.com> Date: Mon, 17 Aug 2026 18:38:25 +0530 Subject: [PATCH] feat(sandbox): match sandbox refs by ID or name prefix Passing a partial sandbox ID did nothing useful: resolveSandboxRef returned any `sb-` prefixed ref verbatim, so `sandbox rm sb-01243e` reached the API as a literal and came back "not found". Resolve refs the way Docker resolves container refs. Precedence runs most-specific first: exact ID, unique ID prefix, exact name (newest wins on duplicates, as before), then unique name prefix. A prefix matching several sandboxes is refused with the candidates listed rather than guessed at, which matters most for `rm`. All 17 sandbox subcommands already funnel through resolveSandboxRef, so no call sites change. The matching rules move into a pure matchSandboxRef so they can be unit-tested: SandboxClient wraps a live resty client and offers no mock seam. Two details worth keeping: Errors are *api.APIError, not fmt.Errorf. api.UserMessage rewrites every other error type into a generic "something went wrong", and rm.go prints resolve failures through it, so a plain error would have been swallowed on exactly the command that needs it most. The pre-existing not-found error had the same defect and is fixed too. An ID that matches nothing is still returned verbatim, and an ID-shaped ref survives a failed list call. The visible list is capped at 200 rows, so the API must stay the authority on whether an ID exists instead of the CLI inventing a not-found. --- README.md | 12 ++ cmd/sandbox/resolve.go | 162 +++++++++++++++++++++----- cmd/sandbox/resolve_test.go | 221 ++++++++++++++++++++++++++++++++++++ 3 files changed, 366 insertions(+), 29 deletions(-) create mode 100644 cmd/sandbox/resolve_test.go diff --git a/README.md b/README.md index 25445e5..bd03147 100644 --- a/README.md +++ b/README.md @@ -292,6 +292,18 @@ Sensitive and noisy files are always excluded: `.env`, `.env.*`, secrets/keys (` Sandboxes are fast-booting VMs — isolated environments you can exec into, sync files to, tunnel ports through, and snapshot at will. +Anywhere a command takes a sandbox, you can pass its full ID, its name, or +just the first few characters of either — the same shortcut Docker allows: + +```bash +createos sandbox rm sb-01243e # enough of the ID to be unique +createos sandbox get my-box # the name you gave it +createos sandbox exec my-b -- ls # enough of the name to be unique +``` + +If what you type matches more than one sandbox, the CLI lists the matches and +asks for a few more characters rather than guessing. + | Command | Description | | ---------------------------------- | ------------------------------------------------------------- | | `createos sandbox create` | Create a new sandbox | diff --git a/cmd/sandbox/resolve.go b/cmd/sandbox/resolve.go index 355424c..36fe10a 100644 --- a/cmd/sandbox/resolve.go +++ b/cmd/sandbox/resolve.go @@ -3,6 +3,7 @@ package sandbox import ( "context" "fmt" + "net/http" "sort" "strings" @@ -52,48 +53,151 @@ func splitForceFlag(args []string) (refs []string, force bool) { } // resolveSandboxRef resolves a sandbox identifier supplied on the CLI. -// The user can pass either a raw id (`sb-`) or a friendly name -// they set at create time. Names are unique within a session but the -// API doesn't enforce uniqueness — when multiple sandboxes share the -// same name, the most-recently-created one wins (matches the -// "what they probably meant" intuition). -// -// Behavior: -// - Input that already starts with `sb-` is returned verbatim — no -// extra round-trip. We let the actual operation (GET / DELETE) -// surface the "not found" if the id is bogus. -// - Otherwise we list the caller's sandboxes (any status, up to 200) -// and pick the most recent with a matching name. -// - Whitespace is trimmed; comparison is case-sensitive (matches -// how the server stores the name). -// -// Returns a friendly error pointing at `sandbox list` when no match -// is found. +// The user can pass a full id (`sb-`), an unambiguous leading +// chunk of one (`sb-01243e`), a friendly name, or a leading chunk of a +// name. It lists the caller's sandboxes (any status, up to 200) and +// hands the rows to matchSandboxRef, which holds all the matching +// rules — see there for precedence and ambiguity behavior. func resolveSandboxRef(ctx context.Context, client *api.SandboxClient, ref string) (string, error) { ref = strings.TrimSpace(ref) if ref == "" { return "", fmt.Errorf("please provide a sandbox ID or name") } - if strings.HasPrefix(ref, sandboxIDPrefix) { - return ref, nil - } rows, _, err := client.ListSandboxes(ctx, api.ListSandboxesOpts{Limit: 200}) if err != nil { + // Prefix matching needs the list, but an id-shaped ref used to + // resolve without one. Keep that working when listing fails so a + // flaky list endpoint can't break `rm sb-`; the real + // operation still reports an authoritative error if the id is + // wrong. A name-shaped ref has no such fallback. + if strings.HasPrefix(ref, sandboxIDPrefix) { + return ref, nil + } return "", err } - matches := make([]api.SandboxView, 0) + return matchSandboxRef(rows, ref) +} + +// matchSandboxRef picks the sandbox a user meant from the list they can +// see. It is pure so the matching rules can be unit-tested without an +// API — SandboxClient wraps a live resty client with no mock seam. +// +// Precedence is most-specific-first, the same shape Docker uses for +// container refs: +// +// ref starts with `sb-` → treated as an id +// exact id match → that sandbox (ids are unique) +// one id prefix match → that sandbox +// many prefix matches → ambiguous; ask for more characters +// no match → the ref verbatim, so the server decides +// otherwise → treated as a name +// exact name match → most-recently-created, since the API does +// not enforce unique names +// one name prefix hit → that sandbox +// many prefix hits → ambiguous; ask for more characters +// no match → friendly error pointing at `sandbox list` +// +// Falling back to the verbatim ref on a zero-match id is deliberate: +// the visible list is capped, so a valid id outside that window (or a +// destroyed sandbox) must still reach the API for an authoritative +// answer rather than getting a wrong "not found" from the CLI. +// +// Comparison is case-sensitive throughout, matching how the server +// stores names. +func matchSandboxRef(rows []api.SandboxView, ref string) (string, error) { + if strings.HasPrefix(ref, sandboxIDPrefix) { + var prefixed []api.SandboxView + for _, r := range rows { + if r.ID == ref { + return r.ID, nil + } + if strings.HasPrefix(r.ID, ref) { + prefixed = append(prefixed, r) + } + } + switch len(prefixed) { + case 0: + return ref, nil + case 1: + return prefixed[0].ID, nil + default: + return "", ambiguousRefError(ref, prefixed) + } + } + + var exact, prefixed []api.SandboxView for _, r := range rows { - if r.Name != nil && *r.Name == ref { - matches = append(matches, r) + if r.Name == nil { + continue } + switch { + case *r.Name == ref: + exact = append(exact, r) + case strings.HasPrefix(*r.Name, ref): + prefixed = append(prefixed, r) + } + } + if len(exact) > 0 { + return mostRecent(exact).ID, nil } - if len(matches) == 0 { - return "", fmt.Errorf("no sandbox named %q\n\n To see your sandboxes, run:\n createos sandbox list", ref) + switch len(prefixed) { + case 0: + // APIError rather than a bare fmt.Errorf so the text survives + // api.UserMessage, which rewrites any other error type into a + // generic "something went wrong" (see internal/api/types.go). + return "", &api.APIError{ + StatusCode: http.StatusNotFound, + Message: fmt.Sprintf("no sandbox matching %q\n\n To see your sandboxes, run:\n createos sandbox list", ref), + } + case 1: + return prefixed[0].ID, nil + default: + return "", ambiguousRefError(ref, prefixed) } - // Most-recent wins. Stable sort so deterministic when timestamps tie. - sort.SliceStable(matches, func(i, j int) bool { - return matches[i].CreatedAt.After(matches[j].CreatedAt) +} + +// mostRecent returns the newest sandbox of the bunch. Stable sort keeps +// the pick deterministic when timestamps tie. +func mostRecent(rows []api.SandboxView) api.SandboxView { + sorted := make([]api.SandboxView, len(rows)) + copy(sorted, rows) + sort.SliceStable(sorted, func(i, j int) bool { + return sorted[i].CreatedAt.After(sorted[j].CreatedAt) }) - return matches[0].ID, nil + return sorted[0] +} + +// ambiguousRefErrorLimit caps how many candidates an ambiguity error +// lists, so a very short prefix doesn't flood the terminal. +const ambiguousRefErrorLimit = 10 + +// ambiguousRefError explains which sandboxes a prefix hit and asks for +// more characters. Candidates are listed newest first so the one the +// user most likely meant is at the top. +func ambiguousRefError(ref string, matches []api.SandboxView) error { + sorted := make([]api.SandboxView, len(matches)) + copy(sorted, matches) + sort.SliceStable(sorted, func(i, j int) bool { + return sorted[i].CreatedAt.After(sorted[j].CreatedAt) + }) + + var b strings.Builder + fmt.Fprintf(&b, "%q matches %d sandboxes:\n", ref, len(sorted)) + for i, r := range sorted { + if i == ambiguousRefErrorLimit { + fmt.Fprintf(&b, " … and %d more\n", len(sorted)-i) + break + } + if r.Name != nil && *r.Name != "" { + fmt.Fprintf(&b, " %s (%s)\n", r.ID, *r.Name) + } else { + fmt.Fprintf(&b, " %s\n", r.ID) + } + } + b.WriteString("\n Type more characters to pick just one, or run:\n createos sandbox list") + // APIError so the text survives api.UserMessage — see the not-found + // branch in matchSandboxRef for why. 400: the ref is under-specified, + // not missing. + return &api.APIError{StatusCode: http.StatusBadRequest, Message: b.String()} } diff --git a/cmd/sandbox/resolve_test.go b/cmd/sandbox/resolve_test.go new file mode 100644 index 0000000..25a994b --- /dev/null +++ b/cmd/sandbox/resolve_test.go @@ -0,0 +1,221 @@ +package sandbox + +import ( + "errors" + "net/http" + "strings" + "testing" + "time" + + "github.com/NodeOps-app/createos-cli/internal/api" +) + +// sbView builds a SandboxView with just the fields matchSandboxRef reads. +// age shifts CreatedAt backwards so "most recent wins" is testable. +func sbView(id, name string, age time.Duration) api.SandboxView { + v := api.SandboxView{ + ID: id, + CreatedAt: time.Date(2026, 8, 17, 12, 0, 0, 0, time.UTC).Add(-age), + } + if name != "" { + v.Name = &name + } + return v +} + +func TestMatchSandboxRefByID(t *testing.T) { + rows := []api.SandboxView{ + sbView("sb-01243eaysdgfh", "alpha", 3*time.Hour), + sbView("sb-01243fnq8k2m1", "beta", 2*time.Hour), + sbView("sb-09zzz000000ab", "", time.Hour), + } + + cases := map[string]string{ + // Full id resolves to itself. + "sb-01243eaysdgfh": "sb-01243eaysdgfh", + // The reported bug: a unique leading chunk must resolve. + "sb-01243e": "sb-01243eaysdgfh", + "sb-01243f": "sb-01243fnq8k2m1", + "sb-09": "sb-09zzz000000ab", + // One character short of ambiguous. + "sb-01243ea": "sb-01243eaysdgfh", + } + for ref, want := range cases { + got, err := matchSandboxRef(rows, ref) + if err != nil { + t.Errorf("matchSandboxRef(%q) unexpected err: %v", ref, err) + continue + } + if got != want { + t.Errorf("matchSandboxRef(%q) = %q, want %q", ref, got, want) + } + } +} + +func TestMatchSandboxRefExactIDBeatsPrefix(t *testing.T) { + // "sb-01" is both a real id and a prefix of the other two. The exact + // match must win rather than reporting ambiguity. + rows := []api.SandboxView{ + sbView("sb-01aaa", "", time.Hour), + sbView("sb-01bbb", "", time.Hour), + sbView("sb-01", "", time.Hour), + } + got, err := matchSandboxRef(rows, "sb-01") + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if got != "sb-01" { + t.Errorf("exact id match = %q, want %q", got, "sb-01") + } +} + +func TestMatchSandboxRefAmbiguousID(t *testing.T) { + rows := []api.SandboxView{ + sbView("sb-01243eaysdgfh", "alpha", 3*time.Hour), + sbView("sb-01243fnq8k2m1", "beta", time.Hour), + } + _, err := matchSandboxRef(rows, "sb-01243") + if err == nil { + t.Fatal(`matchSandboxRef("sb-01243") expected ambiguity error, got nil`) + } + + // Must be an APIError or api.UserMessage rewrites it to a generic + // "something went wrong" at the rm.go call site. + var apiErr *api.APIError + if !errors.As(err, &apiErr) { + t.Fatalf("expected *api.APIError, got %T", err) + } + if apiErr.StatusCode != http.StatusBadRequest { + t.Errorf("StatusCode = %d, want %d", apiErr.StatusCode, http.StatusBadRequest) + } + + msg := apiErr.Message + for _, want := range []string{"sb-01243eaysdgfh", "sb-01243fnq8k2m1", "matches 2 sandboxes"} { + if !strings.Contains(msg, want) { + t.Errorf("ambiguity message missing %q:\n%s", want, msg) + } + } + // Newest first, so the likeliest intent is at the top. + if strings.Index(msg, "sb-01243fnq8k2m1") > strings.Index(msg, "sb-01243eaysdgfh") { + t.Errorf("expected newest candidate listed first:\n%s", msg) + } +} + +func TestMatchSandboxRefUnknownIDPassesThrough(t *testing.T) { + // The visible list is capped at 200, so an id that matches nothing + // must still reach the API for an authoritative answer instead of + // the CLI inventing a "not found". + rows := []api.SandboxView{sbView("sb-01aaa", "alpha", time.Hour)} + got, err := matchSandboxRef(rows, "sb-99notinlist") + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if got != "sb-99notinlist" { + t.Errorf("unknown id = %q, want it returned verbatim", got) + } +} + +func TestMatchSandboxRefByName(t *testing.T) { + rows := []api.SandboxView{ + sbView("sb-01aaa", "web-server", time.Hour), + sbView("sb-01bbb", "worker", time.Hour), + sbView("sb-01ccc", "", time.Hour), // unnamed rows must not panic + } + + cases := map[string]string{ + "web-server": "sb-01aaa", // exact + "web": "sb-01aaa", // unique prefix + "wor": "sb-01bbb", + } + for ref, want := range cases { + got, err := matchSandboxRef(rows, ref) + if err != nil { + t.Errorf("matchSandboxRef(%q) unexpected err: %v", ref, err) + continue + } + if got != want { + t.Errorf("matchSandboxRef(%q) = %q, want %q", ref, got, want) + } + } +} + +func TestMatchSandboxRefExactNameBeatsPrefix(t *testing.T) { + // "web" exactly names one sandbox and prefixes another. Exact wins. + rows := []api.SandboxView{ + sbView("sb-01aaa", "web-server", time.Hour), + sbView("sb-01bbb", "web", time.Hour), + } + got, err := matchSandboxRef(rows, "web") + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if got != "sb-01bbb" { + t.Errorf("exact name match = %q, want %q", got, "sb-01bbb") + } +} + +func TestMatchSandboxRefDuplicateNamesPickNewest(t *testing.T) { + // The API does not enforce unique names; newest wins. + rows := []api.SandboxView{ + sbView("sb-old", "api", 5*time.Hour), + sbView("sb-new", "api", time.Hour), + sbView("sb-mid", "api", 3*time.Hour), + } + got, err := matchSandboxRef(rows, "api") + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if got != "sb-new" { + t.Errorf("duplicate names = %q, want newest %q", got, "sb-new") + } +} + +func TestMatchSandboxRefAmbiguousName(t *testing.T) { + rows := []api.SandboxView{ + sbView("sb-01aaa", "web-server", time.Hour), + sbView("sb-01bbb", "web-worker", time.Hour), + } + _, err := matchSandboxRef(rows, "web-") + if err == nil { + t.Fatal(`matchSandboxRef("web-") expected ambiguity error, got nil`) + } + msg := api.UserMessage(err) + for _, want := range []string{"web-server", "web-worker", "sb-01aaa", "sb-01bbb"} { + if !strings.Contains(msg, want) { + t.Errorf("ambiguity message missing %q:\n%s", want, msg) + } + } +} + +func TestMatchSandboxRefUnknownNameErrors(t *testing.T) { + rows := []api.SandboxView{sbView("sb-01aaa", "web-server", time.Hour)} + _, err := matchSandboxRef(rows, "database") + if err == nil { + t.Fatal(`matchSandboxRef("database") expected not-found error, got nil`) + } + var apiErr *api.APIError + if !errors.As(err, &apiErr) { + t.Fatalf("expected *api.APIError, got %T", err) + } + if apiErr.StatusCode != http.StatusNotFound { + t.Errorf("StatusCode = %d, want %d", apiErr.StatusCode, http.StatusNotFound) + } + if !strings.Contains(apiErr.Message, "database") { + t.Errorf("not-found message should quote the ref:\n%s", apiErr.Message) + } +} + +func TestAmbiguousRefErrorTruncates(t *testing.T) { + rows := make([]api.SandboxView, 0, 15) + for i := range 15 { + rows = append(rows, sbView("sb-01"+string(rune('a'+i)), "", time.Duration(i)*time.Hour)) + } + err := ambiguousRefError("sb-01", rows) + msg := api.UserMessage(err) + if !strings.Contains(msg, "matches 15 sandboxes") { + t.Errorf("expected full count in header:\n%s", msg) + } + if !strings.Contains(msg, "and 5 more") { + t.Errorf("expected truncation notice after %d entries:\n%s", ambiguousRefErrorLimit, msg) + } +}