From 759b21dbe1c64bbd4fda72c0d11dbfe583ffb423 Mon Sep 17 00:00:00 2001 From: noah Date: Mon, 20 Apr 2026 15:34:17 -0400 Subject: [PATCH 1/2] feat(run-commit): add --ref flag for repo:tag references Before this change, 'vers run-commit ' always sent the argument as {"commit_id": ...} in the API request. Passing a repo:tag reference like 'my-app:latest' failed with 422 Unprocessable Entity because the server tried to parse it as a UUID. --ref switches the underlying payload to {"ref": ...}, which the API resolves against tags in the caller's own org. The positive case was verified end-to-end against pi-agent:latest. Also added a friendly error when the argument looks like a repo:tag (contains ':') and --ref was omitted, pointing the user at the right flag instead of hitting a generic 422 from the API. --- cmd/run_commit.go | 60 +++++++++++++++++++++++++++++++-- internal/handlers/run_commit.go | 20 +++++++++-- 2 files changed, 74 insertions(+), 6 deletions(-) diff --git a/cmd/run_commit.go b/cmd/run_commit.go index 1aef3f7..b42cae5 100644 --- a/cmd/run_commit.go +++ b/cmd/run_commit.go @@ -2,7 +2,9 @@ package cmd import ( "context" + "fmt" "os" + "strings" "github.com/hdresearch/vers-cli/internal/handlers" "github.com/hdresearch/vers-cli/internal/jobs" @@ -16,19 +18,40 @@ var ( runCommitJSON bool runCommitFormat string runCommitWait bool + runCommitIsRef bool ) // runCommitCmd represents the run-commit command var runCommitCmd = &cobra.Command{ - Use: "run-commit [commit-key]", + Use: "run-commit [commit-key | repo:tag]", Short: "Start a development environment from a commit", - Long: `Start a Vers development environment from an existing commit using its commit key. + Long: `Start a Vers development environment from an existing commit. + +The argument is treated as a commit ID (UUID) by default. Pass --ref to interpret +it as a repository reference in "repo_name:tag_name" format instead — the API +will resolve the tag to the commit it currently points at within your own org. + +Examples: + vers run-commit c123456789abcdef + vers run-commit my-app:latest --ref + vers run-commit my-app:latest --ref --vm-alias dev --wait Use --json for machine-readable output. Use --wait to block until the VM is running.`, Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { commitKey := args[0] + + // Friendly nudge for the common gotcha: user passed something that + // looks like a repo:tag ref without --ref. + if !runCommitIsRef && looksLikeRepoRef(commitKey) { + return fmt.Errorf( + "'%s' looks like a repo:tag reference; did you mean '--ref'?\n"+ + " vers run-commit %s --ref", + commitKey, commitKey, + ) + } + cfg, err := runconfig.Load() if err != nil { return err @@ -36,7 +59,12 @@ Use --wait to block until the VM is running.`, applyFlagOverrides(cmd, cfg) apiCtx, cancel := context.WithTimeout(context.Background(), application.Timeouts.APILong) defer cancel() - req := handlers.RunCommitReq{CommitKey: commitKey, VMAlias: commitVmAlias, Wait: runCommitWait} + req := handlers.RunCommitReq{ + CommitKey: commitKey, + VMAlias: commitVmAlias, + Wait: runCommitWait, + IsRef: runCommitIsRef, + } var jobID string if runCommitWait { jobID, _ = jobs.Submit(jobs.Submission{ @@ -70,6 +98,31 @@ Use --wait to block until the VM is running.`, }, } +// looksLikeRepoRef returns true for strings in "word:word" shape where the +// parts contain only characters that are legal in repo/tag names. Used only +// to guess user intent and suggest --ref; never to actually dispatch. +func looksLikeRepoRef(s string) bool { + i := strings.IndexByte(s, ':') + if i <= 0 || i == len(s)-1 { + return false + } + legal := func(c byte) bool { + return (c >= 'a' && c <= 'z') || + (c >= 'A' && c <= 'Z') || + (c >= '0' && c <= '9') || + c == '-' || c == '_' || c == '.' + } + for j := 0; j < len(s); j++ { + if j == i { + continue + } + if !legal(s[j]) { + return false + } + } + return true +} + func init() { rootCmd.AddCommand(runCommitCmd) @@ -78,4 +131,5 @@ func init() { runCommitCmd.Flags().StringVar(&runCommitFormat, "format", "", "Output format (json) [deprecated: use --json]") _ = runCommitCmd.Flags().MarkDeprecated("format", "use --json instead") runCommitCmd.Flags().BoolVar(&runCommitWait, "wait", false, "Wait until VM is running") + runCommitCmd.Flags().BoolVar(&runCommitIsRef, "ref", false, "Interpret the argument as a repo:tag reference instead of a commit ID") } diff --git a/internal/handlers/run_commit.go b/internal/handlers/run_commit.go index 43291e6..82d6f8c 100644 --- a/internal/handlers/run_commit.go +++ b/internal/handlers/run_commit.go @@ -11,16 +11,30 @@ import ( ) type RunCommitReq struct { + // CommitKey is either a commit ID (UUID) or, when IsRef is true, a + // repository reference in "repo_name:tag_name" format. CommitKey string VMAlias string Wait bool + // IsRef switches the underlying API payload from {"commit_id": ...} to + // {"ref": ...}, which enables resolving own-org repository tags like + // "my-app:latest" instead of raw commit UUIDs. + IsRef bool } func HandleRunCommit(ctx context.Context, a *app.App, r RunCommitReq) (presenters.RunCommitView, error) { - body := vers.VmRestoreFromCommitParams{ - VmFromCommitRequest: vers.VmFromCommitRequestParam{ + var reqUnion vers.VmFromCommitRequestUnionParam + if r.IsRef { + reqUnion = vers.VmFromCommitRequestRefParam{ + Ref: vers.F(r.CommitKey), + } + } else { + reqUnion = vers.VmFromCommitRequestCommitIDParam{ CommitID: vers.F(r.CommitKey), - }, + } + } + body := vers.VmRestoreFromCommitParams{ + VmFromCommitRequest: reqUnion, } resp, err := a.Client.Vm.RestoreFromCommit(ctx, body) From dc8dcc4c9faec9736da15cfe721cdacc30ff67ee Mon Sep 17 00:00:00 2001 From: noah Date: Mon, 20 Apr 2026 18:35:57 -0400 Subject: [PATCH 2/2] test: add unit tests for run-commit --ref feature MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two new files, matching the existing repo test style (tags_test.go pattern for handler tests via httptest.NewServer, routing_test.go table-driven pattern for cmd-level pure-function tests): internal/handlers/run_commit_test.go: - TestHandleRunCommit_CommitIDPath — verifies default (IsRef=false) sends commit_id, no ref key in body - TestHandleRunCommit_RefPath — verifies IsRef=true sends ref, no commit_id key - TestHandleRunCommit_ServerError — verifies non-2xx surfaces as a caller-visible error (no silent swallowing) cmd/run_commit_test.go: - TestLooksLikeRepoRef (table-driven, 16 cases) — positive: canonical repo:tag shapes with various legal chars. Negative: empty/no-colon, malformed colon placement, chars outside the legal name set (space, slash, @, #, non-ASCII). - TestLooksLikeRepoRef_MultipleColons — documents the current (intended) behavior for a:b:c as NOT matching, since the second colon isn't a legal name char. --- cmd/run_commit_test.go | 60 +++++++++++++ internal/handlers/run_commit_test.go | 124 +++++++++++++++++++++++++++ 2 files changed, 184 insertions(+) create mode 100644 cmd/run_commit_test.go create mode 100644 internal/handlers/run_commit_test.go diff --git a/cmd/run_commit_test.go b/cmd/run_commit_test.go new file mode 100644 index 0000000..5974ee7 --- /dev/null +++ b/cmd/run_commit_test.go @@ -0,0 +1,60 @@ +package cmd + +import "testing" + +// TestLooksLikeRepoRef verifies the heuristic used to suggest --ref when +// the user passes a repo:tag-shaped argument. False positives are cheap +// (we just show an extra suggestion); false negatives mean the user hits +// a cryptic 422 from the API, so the positive cases get careful coverage. +func TestLooksLikeRepoRef(t *testing.T) { + tests := []struct { + name string + input string + want bool + }{ + // Positive: canonical repo:tag shapes + {"simple", "my-app:latest", true}, + {"minimal", "a:b", true}, + {"semver-tag", "my_app:v1.0.0", true}, + {"dotted-parts", "x.y:1.2", true}, + {"numeric-parts", "123:456", true}, + {"mixed-legal-chars", "pi-agent:2026-04-19-v4", true}, + + // Negative: no colon + {"empty", "", false}, + {"no-colon", "abc-123", false}, + {"uuid", "aa84beb8-a080-4be5-bb7f-86374a68b380", false}, + + // Negative: malformed colon placement + {"colon-at-start", ":latest", false}, + {"colon-at-end", "my-app:", false}, + {"lone-colon", ":", false}, + + // Negative: contains characters not valid in repo/tag names + {"space", "my app:latest", false}, + {"slash", "owner/my-app:latest", false}, + {"at-sign", "my-app@v1:latest", false}, + {"hash", "my-app#1:latest", false}, + {"non-ascii", "my-app:päivä", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := looksLikeRepoRef(tt.input) + if got != tt.want { + t.Errorf("looksLikeRepoRef(%q) = %v, want %v", tt.input, got, tt.want) + } + }) + } +} + +// TestLooksLikeRepoRef_MultipleColons documents the current behavior for +// strings with more than one colon. The first colon splits repo from tag, +// and subsequent characters (including another colon) must be legal name +// chars. A bare `a:b:c` has a colon in the tag part, which fails the +// legal-char check and returns false. +func TestLooksLikeRepoRef_MultipleColons(t *testing.T) { + if looksLikeRepoRef("a:b:c") { + t.Error("expected a:b:c to NOT look like a repo:tag (colon in tag part is not a legal name char)") + } +} diff --git a/internal/handlers/run_commit_test.go b/internal/handlers/run_commit_test.go new file mode 100644 index 0000000..f8db915 --- /dev/null +++ b/internal/handlers/run_commit_test.go @@ -0,0 +1,124 @@ +package handlers_test + +import ( + "context" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "testing" + + "github.com/hdresearch/vers-cli/internal/handlers" +) + +// TestHandleRunCommit_CommitIDPath verifies that when IsRef is false, the +// request body places the argument under "commit_id" (no "ref" key). +// This is the default behavior — any existing callers must not regress. +func TestHandleRunCommit_CommitIDPath(t *testing.T) { + var receivedBody map[string]interface{} + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/api/v1/vm/from_commit" { + t.Errorf("unexpected path: %s", r.URL.Path) + } + if r.Method != http.MethodPost { + t.Errorf("expected POST, got %s", r.Method) + } + body, _ := io.ReadAll(r.Body) + json.Unmarshal(body, &receivedBody) + + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"vm_id": "vm-uuid-1"}`)) + })) + defer server.Close() + + a := testApp(server.URL) + res, err := handlers.HandleRunCommit(context.Background(), a, handlers.RunCommitReq{ + CommitKey: "abc-123", + IsRef: false, + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if res.RootVmID != "vm-uuid-1" { + t.Errorf("expected VM ID vm-uuid-1, got %s", res.RootVmID) + } + if res.CommitKey != "abc-123" { + t.Errorf("expected CommitKey abc-123, got %s", res.CommitKey) + } + + // The SDK flattens the vm_from_commit_request union into the top-level + // request body (MarshalJSON on VmRestoreFromCommitParams inlines it). + if receivedBody["commit_id"] != "abc-123" { + t.Errorf("expected commit_id=abc-123, got %v", receivedBody["commit_id"]) + } + if _, refPresent := receivedBody["ref"]; refPresent { + t.Errorf("expected no 'ref' key when IsRef=false, got: %v", receivedBody) + } +} + +// TestHandleRunCommit_RefPath verifies that when IsRef is true, the request +// body uses "ref" instead of "commit_id". This is the new behavior added +// by the --ref flag. +func TestHandleRunCommit_RefPath(t *testing.T) { + var receivedBody map[string]interface{} + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/api/v1/vm/from_commit" { + t.Errorf("unexpected path: %s", r.URL.Path) + } + if r.Method != http.MethodPost { + t.Errorf("expected POST, got %s", r.Method) + } + body, _ := io.ReadAll(r.Body) + json.Unmarshal(body, &receivedBody) + + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"vm_id": "vm-uuid-2"}`)) + })) + defer server.Close() + + a := testApp(server.URL) + res, err := handlers.HandleRunCommit(context.Background(), a, handlers.RunCommitReq{ + CommitKey: "my-app:latest", + IsRef: true, + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if res.RootVmID != "vm-uuid-2" { + t.Errorf("expected VM ID vm-uuid-2, got %s", res.RootVmID) + } + if res.CommitKey != "my-app:latest" { + t.Errorf("expected CommitKey my-app:latest, got %s", res.CommitKey) + } + + // The SDK flattens the vm_from_commit_request union into the top-level + // request body (MarshalJSON on VmRestoreFromCommitParams inlines it). + if receivedBody["ref"] != "my-app:latest" { + t.Errorf("expected ref=my-app:latest, got %v", receivedBody["ref"]) + } + if _, commitIDPresent := receivedBody["commit_id"]; commitIDPresent { + t.Errorf("expected no 'commit_id' key when IsRef=true, got: %v", receivedBody) + } +} + +// TestHandleRunCommit_ServerError verifies that non-2xx responses surface as +// errors to the caller (no silent swallowing). +func TestHandleRunCommit_ServerError(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusUnprocessableEntity) + w.Write([]byte(`{"error": "invalid commit id"}`)) + })) + defer server.Close() + + a := testApp(server.URL) + _, err := handlers.HandleRunCommit(context.Background(), a, handlers.RunCommitReq{ + CommitKey: "not-a-uuid", + IsRef: false, + }) + if err == nil { + t.Fatal("expected error for 422 response, got nil") + } +}