From f42ee2a12d2f13bb67540b2e11bd0f45cccce388 Mon Sep 17 00:00:00 2001 From: Tynan Daly Date: Mon, 6 Apr 2026 20:17:35 -0700 Subject: [PATCH 1/2] feat: add --name and --description flags to commit create Add --name/-n and --description/-d flags to 'vers commit create' so commits can be given human-readable names instead of the auto-generated 'commit: of_vm= timestamp=' format. After creating the commit, if name or description is provided, a follow-up PATCH call is made to update the commit metadata. If the update fails, the error is returned but the commit ID is still available. Note: The API currently ignores name/description on the PATCH endpoint (see #177). The CLI is ready and will work end-to-end once the API supports it. Closes #177 --- cmd/commit.go | 27 ++- internal/handlers/commit_create.go | 42 ++++- internal/handlers/commit_create_test.go | 221 ++++++++++++++++++++++++ 3 files changed, 281 insertions(+), 9 deletions(-) create mode 100644 internal/handlers/commit_create_test.go diff --git a/cmd/commit.go b/cmd/commit.go index 2fbfb8d..5cea032 100644 --- a/cmd/commit.go +++ b/cmd/commit.go @@ -9,7 +9,11 @@ import ( "github.com/spf13/cobra" ) -var commitFormat string +var ( + commitFormat string + commitName string + commitDescription string +) // commitCmd is the parent command for commit operations. // Bare `vers commit` (no args, no subcommand) creates a commit of HEAD for backward compat. @@ -42,7 +46,14 @@ var commitCreateCmd = &cobra.Command{ Long: `Save the current state of a VM as a commit. If no VM ID or alias is provided, commits the current HEAD VM. -Use --format json for machine-readable output.`, +Use --name to give the commit a human-readable name. +Use --description to add additional context. +Use --format json for machine-readable output. + +Examples: + vers commit create --name "golden-image-v3" + vers commit create --name "pre-deploy" --description "Before deploying auth changes" + vers commit create vm-123 --name "checkpoint"`, Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { target := "" @@ -54,7 +65,9 @@ Use --format json for machine-readable output.`, defer cancel() res, err := handlers.HandleCommitCreate(apiCtx, application, handlers.CommitCreateReq{ - Target: target, + Target: target, + Name: commitName, + Description: commitDescription, }) if err != nil { return err @@ -70,6 +83,12 @@ Use --format json for machine-readable output.`, } fmt.Printf("✓ Committed VM '%s'\n", res.VmID) fmt.Printf("Commit ID: %s\n", res.CommitID) + if res.Name != "" { + fmt.Printf("Name: %s\n", res.Name) + } + if res.Description != "" { + fmt.Printf("Description: %s\n", res.Description) + } } return nil }, @@ -228,6 +247,8 @@ func init() { rootCmd.AddCommand(commitCmd) commitCreateCmd.Flags().StringVar(&commitFormat, "format", "", "Output format (json)") + commitCreateCmd.Flags().StringVarP(&commitName, "name", "n", "", "Human-readable name for the commit") + commitCreateCmd.Flags().StringVarP(&commitDescription, "description", "d", "", "Description for the commit") commitCmd.AddCommand(commitCreateCmd) commitListCmd.Flags().BoolVar(&commitListPublic, "public", false, "List public commits instead of your own") diff --git a/internal/handlers/commit_create.go b/internal/handlers/commit_create.go index b892baf..769a509 100644 --- a/internal/handlers/commit_create.go +++ b/internal/handlers/commit_create.go @@ -7,16 +7,21 @@ import ( "github.com/hdresearch/vers-cli/internal/app" "github.com/hdresearch/vers-cli/internal/utils" vers "github.com/hdresearch/vers-sdk-go" + "github.com/hdresearch/vers-sdk-go/option" ) type CommitCreateReq struct { - Target string + Target string + Name string + Description string } type CommitCreateView struct { - CommitID string `json:"commit_id"` - VmID string `json:"vm_id"` - UsedHEAD bool `json:"used_head,omitempty"` + CommitID string `json:"commit_id"` + VmID string `json:"vm_id"` + UsedHEAD bool `json:"used_head,omitempty"` + Name string `json:"name,omitempty"` + Description string `json:"description,omitempty"` } func HandleCommitCreate(ctx context.Context, a *app.App, r CommitCreateReq) (CommitCreateView, error) { @@ -30,9 +35,34 @@ func HandleCommitCreate(ctx context.Context, a *app.App, r CommitCreateReq) (Com return CommitCreateView{}, fmt.Errorf("failed to commit VM '%s': %w", resolved.ID, err) } - return CommitCreateView{ + view := CommitCreateView{ CommitID: resp.CommitID, VmID: resolved.ID, UsedHEAD: resolved.UsedHEAD, - }, nil + } + + // If name or description provided, update the commit metadata + if r.Name != "" || r.Description != "" { + var opts []option.RequestOption + if r.Name != "" { + opts = append(opts, option.WithJSONSet("name", r.Name)) + } + if r.Description != "" { + opts = append(opts, option.WithJSONSet("description", r.Description)) + } + + _, err := a.Client.Commits.Update(ctx, resp.CommitID, vers.CommitUpdateParams{ + UpdateCommitRequest: vers.UpdateCommitRequestParam{ + IsPublic: vers.F(false), + }, + }, opts...) + if err != nil { + return view, fmt.Errorf("commit created (%s) but failed to set name/description: %w", resp.CommitID, err) + } + + view.Name = r.Name + view.Description = r.Description + } + + return view, nil } diff --git a/internal/handlers/commit_create_test.go b/internal/handlers/commit_create_test.go new file mode 100644 index 0000000..07a92ab --- /dev/null +++ b/internal/handlers/commit_create_test.go @@ -0,0 +1,221 @@ +package handlers_test + +import ( + "context" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "testing" + + "github.com/hdresearch/vers-cli/internal/handlers" +) + +func TestHandleCommitCreate_WithName(t *testing.T) { + var patchBody map[string]interface{} + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + + switch { + case r.Method == http.MethodGet && r.URL.Path == "/api/v1/vm/vm-123/status": + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"vm_id":"vm-123","owner_id":"owner-1","created_at":"2026-01-01T00:00:00Z","state":"running"}`)) + + case r.Method == http.MethodPost && r.URL.Path == "/api/v1/vm/vm-123/commit": + w.WriteHeader(http.StatusCreated) + w.Write([]byte(`{"commit_id":"commit-abc"}`)) + + case r.Method == http.MethodPatch && r.URL.Path == "/api/v1/commits/commit-abc": + body, _ := io.ReadAll(r.Body) + json.Unmarshal(body, &patchBody) + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{ + "commit_id": "commit-abc", + "name": "my-commit", + "owner_id": "owner-1", + "created_at": "2026-01-01T00:00:00Z", + "is_public": false, + "description": "my description" + }`)) + + default: + t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path) + w.WriteHeader(http.StatusNotFound) + } + })) + defer server.Close() + + a := testApp(server.URL) + res, err := handlers.HandleCommitCreate(context.Background(), a, handlers.CommitCreateReq{ + Target: "vm-123", + Name: "my-commit", + Description: "my description", + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if res.CommitID != "commit-abc" { + t.Errorf("expected commit ID commit-abc, got %s", res.CommitID) + } + if res.VmID != "vm-123" { + t.Errorf("expected VM ID vm-123, got %s", res.VmID) + } + if res.Name != "my-commit" { + t.Errorf("expected name my-commit, got %s", res.Name) + } + if res.Description != "my description" { + t.Errorf("expected description 'my description', got %s", res.Description) + } + + // Verify the PATCH request included name and description + if patchBody == nil { + t.Fatal("expected PATCH request to be made") + } + if patchBody["name"] != "my-commit" { + t.Errorf("expected PATCH body name=my-commit, got %v", patchBody["name"]) + } + if patchBody["description"] != "my description" { + t.Errorf("expected PATCH body description='my description', got %v", patchBody["description"]) + } +} + +func TestHandleCommitCreate_WithoutName(t *testing.T) { + patchCalled := false + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + + switch { + case r.Method == http.MethodGet && r.URL.Path == "/api/v1/vm/vm-123/status": + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"vm_id":"vm-123","owner_id":"owner-1","created_at":"2026-01-01T00:00:00Z","state":"running"}`)) + + case r.Method == http.MethodPost && r.URL.Path == "/api/v1/vm/vm-123/commit": + w.WriteHeader(http.StatusCreated) + w.Write([]byte(`{"commit_id":"commit-abc"}`)) + + case r.Method == http.MethodPatch: + patchCalled = true + t.Error("PATCH should not be called when no name/description provided") + + default: + t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path) + w.WriteHeader(http.StatusNotFound) + } + })) + defer server.Close() + + a := testApp(server.URL) + res, err := handlers.HandleCommitCreate(context.Background(), a, handlers.CommitCreateReq{ + Target: "vm-123", + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if res.CommitID != "commit-abc" { + t.Errorf("expected commit ID commit-abc, got %s", res.CommitID) + } + if patchCalled { + t.Error("PATCH should not have been called") + } + if res.Name != "" { + t.Errorf("expected empty name, got %s", res.Name) + } +} + +func TestHandleCommitCreate_NameOnly(t *testing.T) { + var patchBody map[string]interface{} + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + + switch { + case r.Method == http.MethodGet && r.URL.Path == "/api/v1/vm/vm-123/status": + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"vm_id":"vm-123","owner_id":"owner-1","created_at":"2026-01-01T00:00:00Z","state":"running"}`)) + + case r.Method == http.MethodPost && r.URL.Path == "/api/v1/vm/vm-123/commit": + w.WriteHeader(http.StatusCreated) + w.Write([]byte(`{"commit_id":"commit-abc"}`)) + + case r.Method == http.MethodPatch && r.URL.Path == "/api/v1/commits/commit-abc": + body, _ := io.ReadAll(r.Body) + json.Unmarshal(body, &patchBody) + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{ + "commit_id": "commit-abc", + "name": "just-a-name", + "owner_id": "owner-1", + "created_at": "2026-01-01T00:00:00Z", + "is_public": false + }`)) + + default: + t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path) + w.WriteHeader(http.StatusNotFound) + } + })) + defer server.Close() + + a := testApp(server.URL) + res, err := handlers.HandleCommitCreate(context.Background(), a, handlers.CommitCreateReq{ + Target: "vm-123", + Name: "just-a-name", + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if res.Name != "just-a-name" { + t.Errorf("expected name just-a-name, got %s", res.Name) + } + if res.Description != "" { + t.Errorf("expected empty description, got %s", res.Description) + } + + // Verify name was sent but description was not + if patchBody["name"] != "just-a-name" { + t.Errorf("expected name in PATCH body, got %v", patchBody["name"]) + } + if _, hasDesc := patchBody["description"]; hasDesc { + t.Error("description should not be in PATCH body when not provided") + } +} + +func TestHandleCommitCreate_UpdateFails(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + + switch { + case r.Method == http.MethodGet && r.URL.Path == "/api/v1/vm/vm-123/status": + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"vm_id":"vm-123","owner_id":"owner-1","created_at":"2026-01-01T00:00:00Z","state":"running"}`)) + + case r.Method == http.MethodPost && r.URL.Path == "/api/v1/vm/vm-123/commit": + w.WriteHeader(http.StatusCreated) + w.Write([]byte(`{"commit_id":"commit-abc"}`)) + + case r.Method == http.MethodPatch: + w.WriteHeader(http.StatusInternalServerError) + w.Write([]byte(`{"error":"internal server error"}`)) + + default: + w.WriteHeader(http.StatusNotFound) + } + })) + defer server.Close() + + a := testApp(server.URL) + res, err := handlers.HandleCommitCreate(context.Background(), a, handlers.CommitCreateReq{ + Target: "vm-123", + Name: "my-commit", + }) + + // Should return an error but still have the commit ID + if err == nil { + t.Fatal("expected error when update fails") + } + if res.CommitID != "commit-abc" { + t.Errorf("expected commit ID to be returned even on update failure, got %s", res.CommitID) + } +} From f195dac48a964bedac67c53c41cb79ef047fa782 Mon Sep 17 00:00:00 2001 From: Tynan Daly Date: Mon, 6 Apr 2026 20:27:30 -0700 Subject: [PATCH 2/2] refactor: send name/description directly on commit request Instead of a follow-up PATCH call, send name and description in the POST /vm/{id}/commit request body using WithJSONSet. This is simpler, atomic, and avoids the race window where a commit exists briefly without its metadata. Requires the companion chelsea change that accepts name/description on the commit endpoint. --- internal/handlers/commit_create.go | 46 ++++------- internal/handlers/commit_create_test.go | 102 ++++-------------------- 2 files changed, 33 insertions(+), 115 deletions(-) diff --git a/internal/handlers/commit_create.go b/internal/handlers/commit_create.go index 769a509..6fa3a46 100644 --- a/internal/handlers/commit_create.go +++ b/internal/handlers/commit_create.go @@ -30,39 +30,25 @@ func HandleCommitCreate(ctx context.Context, a *app.App, r CommitCreateReq) (Com return CommitCreateView{}, err } - resp, err := a.Client.Vm.Commit(ctx, resolved.ID, vers.VmCommitParams{}) - if err != nil { - return CommitCreateView{}, fmt.Errorf("failed to commit VM '%s': %w", resolved.ID, err) + // Build request options to send name/description in the request body + var opts []option.RequestOption + if r.Name != "" { + opts = append(opts, option.WithJSONSet("name", r.Name)) } - - view := CommitCreateView{ - CommitID: resp.CommitID, - VmID: resolved.ID, - UsedHEAD: resolved.UsedHEAD, + if r.Description != "" { + opts = append(opts, option.WithJSONSet("description", r.Description)) } - // If name or description provided, update the commit metadata - if r.Name != "" || r.Description != "" { - var opts []option.RequestOption - if r.Name != "" { - opts = append(opts, option.WithJSONSet("name", r.Name)) - } - if r.Description != "" { - opts = append(opts, option.WithJSONSet("description", r.Description)) - } - - _, err := a.Client.Commits.Update(ctx, resp.CommitID, vers.CommitUpdateParams{ - UpdateCommitRequest: vers.UpdateCommitRequestParam{ - IsPublic: vers.F(false), - }, - }, opts...) - if err != nil { - return view, fmt.Errorf("commit created (%s) but failed to set name/description: %w", resp.CommitID, err) - } - - view.Name = r.Name - view.Description = r.Description + resp, err := a.Client.Vm.Commit(ctx, resolved.ID, vers.VmCommitParams{}, opts...) + if err != nil { + return CommitCreateView{}, fmt.Errorf("failed to commit VM '%s': %w", resolved.ID, err) } - return view, nil + return CommitCreateView{ + CommitID: resp.CommitID, + VmID: resolved.ID, + UsedHEAD: resolved.UsedHEAD, + Name: r.Name, + Description: r.Description, + }, nil } diff --git a/internal/handlers/commit_create_test.go b/internal/handlers/commit_create_test.go index 07a92ab..bafadfa 100644 --- a/internal/handlers/commit_create_test.go +++ b/internal/handlers/commit_create_test.go @@ -12,7 +12,7 @@ import ( ) func TestHandleCommitCreate_WithName(t *testing.T) { - var patchBody map[string]interface{} + var commitBody map[string]interface{} server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") @@ -23,22 +23,11 @@ func TestHandleCommitCreate_WithName(t *testing.T) { w.Write([]byte(`{"vm_id":"vm-123","owner_id":"owner-1","created_at":"2026-01-01T00:00:00Z","state":"running"}`)) case r.Method == http.MethodPost && r.URL.Path == "/api/v1/vm/vm-123/commit": + body, _ := io.ReadAll(r.Body) + json.Unmarshal(body, &commitBody) w.WriteHeader(http.StatusCreated) w.Write([]byte(`{"commit_id":"commit-abc"}`)) - case r.Method == http.MethodPatch && r.URL.Path == "/api/v1/commits/commit-abc": - body, _ := io.ReadAll(r.Body) - json.Unmarshal(body, &patchBody) - w.WriteHeader(http.StatusOK) - w.Write([]byte(`{ - "commit_id": "commit-abc", - "name": "my-commit", - "owner_id": "owner-1", - "created_at": "2026-01-01T00:00:00Z", - "is_public": false, - "description": "my description" - }`)) - default: t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path) w.WriteHeader(http.StatusNotFound) @@ -68,21 +57,19 @@ func TestHandleCommitCreate_WithName(t *testing.T) { t.Errorf("expected description 'my description', got %s", res.Description) } - // Verify the PATCH request included name and description - if patchBody == nil { - t.Fatal("expected PATCH request to be made") + // Verify name and description were sent in the commit request body + if commitBody == nil { + t.Fatal("expected commit request to have a body") } - if patchBody["name"] != "my-commit" { - t.Errorf("expected PATCH body name=my-commit, got %v", patchBody["name"]) + if commitBody["name"] != "my-commit" { + t.Errorf("expected body name=my-commit, got %v", commitBody["name"]) } - if patchBody["description"] != "my description" { - t.Errorf("expected PATCH body description='my description', got %v", patchBody["description"]) + if commitBody["description"] != "my description" { + t.Errorf("expected body description='my description', got %v", commitBody["description"]) } } func TestHandleCommitCreate_WithoutName(t *testing.T) { - patchCalled := false - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") @@ -95,10 +82,6 @@ func TestHandleCommitCreate_WithoutName(t *testing.T) { w.WriteHeader(http.StatusCreated) w.Write([]byte(`{"commit_id":"commit-abc"}`)) - case r.Method == http.MethodPatch: - patchCalled = true - t.Error("PATCH should not be called when no name/description provided") - default: t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path) w.WriteHeader(http.StatusNotFound) @@ -116,16 +99,13 @@ func TestHandleCommitCreate_WithoutName(t *testing.T) { if res.CommitID != "commit-abc" { t.Errorf("expected commit ID commit-abc, got %s", res.CommitID) } - if patchCalled { - t.Error("PATCH should not have been called") - } if res.Name != "" { t.Errorf("expected empty name, got %s", res.Name) } } func TestHandleCommitCreate_NameOnly(t *testing.T) { - var patchBody map[string]interface{} + var commitBody map[string]interface{} server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") @@ -136,21 +116,11 @@ func TestHandleCommitCreate_NameOnly(t *testing.T) { w.Write([]byte(`{"vm_id":"vm-123","owner_id":"owner-1","created_at":"2026-01-01T00:00:00Z","state":"running"}`)) case r.Method == http.MethodPost && r.URL.Path == "/api/v1/vm/vm-123/commit": + body, _ := io.ReadAll(r.Body) + json.Unmarshal(body, &commitBody) w.WriteHeader(http.StatusCreated) w.Write([]byte(`{"commit_id":"commit-abc"}`)) - case r.Method == http.MethodPatch && r.URL.Path == "/api/v1/commits/commit-abc": - body, _ := io.ReadAll(r.Body) - json.Unmarshal(body, &patchBody) - w.WriteHeader(http.StatusOK) - w.Write([]byte(`{ - "commit_id": "commit-abc", - "name": "just-a-name", - "owner_id": "owner-1", - "created_at": "2026-01-01T00:00:00Z", - "is_public": false - }`)) - default: t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path) w.WriteHeader(http.StatusNotFound) @@ -174,48 +144,10 @@ func TestHandleCommitCreate_NameOnly(t *testing.T) { } // Verify name was sent but description was not - if patchBody["name"] != "just-a-name" { - t.Errorf("expected name in PATCH body, got %v", patchBody["name"]) - } - if _, hasDesc := patchBody["description"]; hasDesc { - t.Error("description should not be in PATCH body when not provided") - } -} - -func TestHandleCommitCreate_UpdateFails(t *testing.T) { - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - - switch { - case r.Method == http.MethodGet && r.URL.Path == "/api/v1/vm/vm-123/status": - w.WriteHeader(http.StatusOK) - w.Write([]byte(`{"vm_id":"vm-123","owner_id":"owner-1","created_at":"2026-01-01T00:00:00Z","state":"running"}`)) - - case r.Method == http.MethodPost && r.URL.Path == "/api/v1/vm/vm-123/commit": - w.WriteHeader(http.StatusCreated) - w.Write([]byte(`{"commit_id":"commit-abc"}`)) - - case r.Method == http.MethodPatch: - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(`{"error":"internal server error"}`)) - - default: - w.WriteHeader(http.StatusNotFound) - } - })) - defer server.Close() - - a := testApp(server.URL) - res, err := handlers.HandleCommitCreate(context.Background(), a, handlers.CommitCreateReq{ - Target: "vm-123", - Name: "my-commit", - }) - - // Should return an error but still have the commit ID - if err == nil { - t.Fatal("expected error when update fails") + if commitBody["name"] != "just-a-name" { + t.Errorf("expected name in body, got %v", commitBody["name"]) } - if res.CommitID != "commit-abc" { - t.Errorf("expected commit ID to be returned even on update failure, got %s", res.CommitID) + if _, hasDesc := commitBody["description"]; hasDesc { + t.Error("description should not be in body when not provided") } }