From a0c0e8367e7ff7ee168dee77cb5f12fd38c66a29 Mon Sep 17 00:00:00 2001 From: "C. Ross" Date: Mon, 31 Aug 2026 09:15:50 -0400 Subject: [PATCH 1/3] Add target lifecycle attribution Stamp customer initiator metadata and a fresh operation UUID on target create, pause, resume, and abort requests from CLI and TUI workflows. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- go.mod | 1 + go.sum | 2 + internal/cmd/target/migration.go | 12 +++- internal/cmd/target/migration_test.go | 46 ++++++++++++++ internal/elmapi/target_migrations.go | 37 ++++++++++-- internal/elmapi/target_migrations_test.go | 51 ++++++++++++++-- internal/workflow/service.go | 12 +++- internal/workflow/service_test.go | 73 +++++++++++++++++++++++ 8 files changed, 219 insertions(+), 15 deletions(-) diff --git a/go.mod b/go.mod index bfb572f..d7f45c4 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,7 @@ require ( github.com/charmbracelet/huh v1.0.0 github.com/charmbracelet/lipgloss v1.1.0 github.com/charmbracelet/x/ansi v0.9.3 + github.com/google/uuid v1.6.0 github.com/muesli/termenv v0.16.0 github.com/spf13/cobra v1.10.2 github.com/stretchr/testify v1.11.1 diff --git a/go.sum b/go.sum index 2109ae3..0b426fe 100644 --- a/go.sum +++ b/go.sum @@ -49,6 +49,8 @@ github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f h1:Y/CXytFA4m6 github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f/go.mod h1:vw97MGsxSvLiUE2X8qFplwetxpGLQrlU1Q9AUEIzCaM= github.com/godbus/dbus/v5 v5.2.2 h1:TUR3TgtSVDmjiXOgAAyaZbYmIeP3DPkld3jgKGV8mXQ= github.com/godbus/dbus/v5 v5.2.2/go.mod h1:3AAv2+hPq5rdnr5txxxRwiGjPXamgoIHgz9FPBfOp3c= +github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= +github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= github.com/lucasb-eyer/go-colorful v1.2.0 h1:1nnpGOrhyZZuNyfu1QjKiUICQ74+3FNCN69Aj6K7nkY= diff --git a/internal/cmd/target/migration.go b/internal/cmd/target/migration.go index ae8a9aa..6dcad59 100644 --- a/internal/cmd/target/migration.go +++ b/internal/cmd/target/migration.go @@ -146,6 +146,9 @@ func newMigrationCreateCmd() *cobra.Command { Description: description, ExporterMigrationGUID: exporterGUID, } + transition := elmapi.NewCustomerTargetMigrationTransitionRequest() + req.Initiator = transition.Initiator + req.OperationID = transition.OperationID raw, err := client.CreateTargetMigration(cmd.Context(), req) if err != nil { return annotateAuthError(err, targetURLResolved) @@ -240,7 +243,8 @@ func newMigrationPauseCmd() *cobra.Command { if err != nil { return err } - if err := client.PauseTargetMigration(cmd.Context(), migrationID); err != nil { + req := elmapi.NewCustomerTargetMigrationTransitionRequest() + if err := client.PauseTargetMigration(cmd.Context(), migrationID, req); err != nil { return annotateMigrationActionError(err, targetURLResolved, migrationID) } fmt.Fprintf(cmd.OutOrStdout(), "Migration %d paused.\n", migrationID) @@ -280,7 +284,8 @@ func newMigrationResumeCmd() *cobra.Command { if err != nil { return err } - if err := client.ResumeTargetMigration(cmd.Context(), migrationID); err != nil { + req := elmapi.NewCustomerTargetMigrationTransitionRequest() + if err := client.ResumeTargetMigration(cmd.Context(), migrationID, req); err != nil { return annotateMigrationActionError(err, targetURLResolved, migrationID) } fmt.Fprintf(cmd.OutOrStdout(), "Migration %d resumed.\n", migrationID) @@ -323,7 +328,8 @@ func newMigrationAbortCmd() *cobra.Command { if err != nil { return err } - if err := client.AbortTargetMigration(cmd.Context(), migrationID); err != nil { + req := elmapi.NewCustomerTargetMigrationTransitionRequest() + if err := client.AbortTargetMigration(cmd.Context(), migrationID, req); err != nil { return annotateMigrationActionError(err, targetURLResolved, migrationID) } fmt.Fprintf(cmd.OutOrStdout(), "Migration %d aborted.\n", migrationID) diff --git a/internal/cmd/target/migration_test.go b/internal/cmd/target/migration_test.go index ccecc9f..4efec51 100644 --- a/internal/cmd/target/migration_test.go +++ b/internal/cmd/target/migration_test.go @@ -8,8 +8,11 @@ import ( "strings" "testing" + "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/github/gh-elm/internal/elmapi" ) func TestMigrationList(t *testing.T) { @@ -131,6 +134,7 @@ func TestMigrationCreate(t *testing.T) { assert.Equal(t, "https://source.example/octo/repo", gotBody["source_url"]) assert.Equal(t, []any{"octo/repo"}, gotBody["repositories"]) assert.Equal(t, "test migration", gotBody["description"]) + assertCustomerTransition(t, gotBody) assert.Contains(t, out, "Migration 42 created.") assert.Contains(t, out, "Expires at:") }) @@ -250,8 +254,10 @@ func TestMigrationStatus(t *testing.T) { func TestMigrationPauseResumeAbort(t *testing.T) { t.Run("pause posts to the pause endpoint and prints confirmation", func(t *testing.T) { var gotPath string + var gotBody map[string]any srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { gotPath = r.URL.Path + require.NoError(t, json.NewDecoder(r.Body).Decode(&gotBody)) w.WriteHeader(http.StatusNoContent) })) defer srv.Close() @@ -260,13 +266,17 @@ func TestMigrationPauseResumeAbort(t *testing.T) { "--target-url", srv.URL, "--target-token", "tok") assert.Equal(t, "/enterprise/migration/42/pause", gotPath) + assert.Len(t, gotBody, 2) + assertCustomerTransition(t, gotBody) assert.Contains(t, out, "Migration 42 paused.") }) t.Run("resume posts to the resume endpoint and prints confirmation", func(t *testing.T) { var gotPath string + var gotBody map[string]any srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { gotPath = r.URL.Path + require.NoError(t, json.NewDecoder(r.Body).Decode(&gotBody)) w.WriteHeader(http.StatusNoContent) })) defer srv.Close() @@ -275,13 +285,17 @@ func TestMigrationPauseResumeAbort(t *testing.T) { "--target-url", srv.URL, "--target-token", "tok") assert.Equal(t, "/enterprise/migration/42/resume", gotPath) + assert.Len(t, gotBody, 2) + assertCustomerTransition(t, gotBody) assert.Contains(t, out, "Migration 42 resumed.") }) t.Run("abort posts to the abort endpoint and prints confirmation", func(t *testing.T) { var gotPath string + var gotBody map[string]any srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { gotPath = r.URL.Path + require.NoError(t, json.NewDecoder(r.Body).Decode(&gotBody)) w.WriteHeader(http.StatusNoContent) })) defer srv.Close() @@ -290,9 +304,30 @@ func TestMigrationPauseResumeAbort(t *testing.T) { "--target-url", srv.URL, "--target-token", "tok") assert.Equal(t, "/enterprise/migration/42/abort", gotPath) + assert.Len(t, gotBody, 2) + assertCustomerTransition(t, gotBody) assert.Contains(t, out, "Migration 42 aborted.") }) + t.Run("each command invocation generates a fresh operation ID", func(t *testing.T) { + var operationIDs []string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var body map[string]any + require.NoError(t, json.NewDecoder(r.Body).Decode(&body)) + operationIDs = append(operationIDs, assertCustomerTransition(t, body)) + w.WriteHeader(http.StatusNoContent) + })) + defer srv.Close() + + runMigration(t, "migration", "pause", "--migration-id", "42", + "--target-url", srv.URL, "--target-token", "tok") + runMigration(t, "migration", "pause", "--migration-id", "42", + "--target-url", srv.URL, "--target-token", "tok") + + require.Len(t, operationIDs, 2) + assert.NotEqual(t, operationIDs[0], operationIDs[1]) + }) + t.Run("requires --migration-id", func(t *testing.T) { for _, sub := range []string{"pause", "resume", "abort"} { err := runMigrationErr(t, "migration", sub, "--target-url", "https://x", "--target-token", "tok") @@ -377,3 +412,14 @@ func execMigration(t *testing.T, args ...string) (string, error) { err := cmd.Execute() return buf.String(), err } + +func assertCustomerTransition(t *testing.T, body map[string]any) string { + t.Helper() + + assert.Equal(t, elmapi.TargetMigrationInitiatorCustomer, body["initiator"]) + assert.NotContains(t, body, "actor") + operationID, ok := body["operation_id"].(string) + require.True(t, ok, "operation_id must be a string") + require.NoError(t, uuid.Validate(operationID)) + return operationID +} diff --git a/internal/elmapi/target_migrations.go b/internal/elmapi/target_migrations.go index 7f07175..ab3138d 100644 --- a/internal/elmapi/target_migrations.go +++ b/internal/elmapi/target_migrations.go @@ -9,6 +9,8 @@ import ( "net/url" "strconv" "time" + + "github.com/google/uuid" ) // targetMigrationBasePath is the base path for the target (GHEC/Proxima) @@ -35,6 +37,27 @@ const ( // IterTargetMigrations. const targetMigrationsPageSize = 100 +// TargetMigrationInitiatorCustomer identifies lifecycle mutations initiated by +// a customer-operated gh-elm invocation. +const TargetMigrationInitiatorCustomer = "customer" + +// TargetMigrationTransitionRequest attributes a target migration lifecycle +// mutation to the customer invocation that caused it. +type TargetMigrationTransitionRequest struct { + Initiator string `json:"initiator"` + OperationID string `json:"operation_id"` +} + +// NewCustomerTargetMigrationTransitionRequest returns metadata for one logical +// customer-initiated lifecycle mutation. Callers should reuse the returned +// request if they retry that mutation. +func NewCustomerTargetMigrationTransitionRequest() TargetMigrationTransitionRequest { + return TargetMigrationTransitionRequest{ + Initiator: TargetMigrationInitiatorCustomer, + OperationID: uuid.NewString(), + } +} + // CreateTargetMigrationRequest is the body of a create-migration call against // the target (GHEC/Proxima) migration-management API. Repositories currently // accepts exactly one entry; the API does not yet support multi-repository @@ -44,6 +67,8 @@ type CreateTargetMigrationRequest struct { Repositories []string `json:"repositories"` Description string `json:"description,omitempty"` ExporterMigrationGUID string `json:"exporter_migration_guid,omitempty"` + Initiator string `json:"initiator"` + OperationID string `json:"operation_id"` } // CreateTargetMigration creates a migration on the target (GHEC/Proxima) side @@ -204,9 +229,9 @@ func (c *Client) GetTargetMigrationStatus(ctx context.Context, migrationID int64 // PauseTargetMigration pauses a migration on the target (GHEC/Proxima) side. // POST /enterprise/migration/{id}/pause. Returns 204. -func (c *Client) PauseTargetMigration(ctx context.Context, migrationID int64) error { +func (c *Client) PauseTargetMigration(ctx context.Context, migrationID int64, req TargetMigrationTransitionRequest) error { path := fmt.Sprintf("%s/%d/pause", targetMigrationBasePath, migrationID) - if err := c.post(ctx, path, nil, nil, http.StatusNoContent); err != nil { + if err := c.post(ctx, path, req, nil, http.StatusNoContent); err != nil { return fmt.Errorf("pausing target migration: %w", err) } return nil @@ -214,9 +239,9 @@ func (c *Client) PauseTargetMigration(ctx context.Context, migrationID int64) er // ResumeTargetMigration resumes a paused migration on the target side. // POST /enterprise/migration/{id}/resume. Returns 204. -func (c *Client) ResumeTargetMigration(ctx context.Context, migrationID int64) error { +func (c *Client) ResumeTargetMigration(ctx context.Context, migrationID int64, req TargetMigrationTransitionRequest) error { path := fmt.Sprintf("%s/%d/resume", targetMigrationBasePath, migrationID) - if err := c.post(ctx, path, nil, nil, http.StatusNoContent); err != nil { + if err := c.post(ctx, path, req, nil, http.StatusNoContent); err != nil { return fmt.Errorf("resuming target migration: %w", err) } return nil @@ -224,9 +249,9 @@ func (c *Client) ResumeTargetMigration(ctx context.Context, migrationID int64) e // AbortTargetMigration aborts a migration on the target side. This is a // terminal action. POST /enterprise/migration/{id}/abort. Returns 204. -func (c *Client) AbortTargetMigration(ctx context.Context, migrationID int64) error { +func (c *Client) AbortTargetMigration(ctx context.Context, migrationID int64, req TargetMigrationTransitionRequest) error { path := fmt.Sprintf("%s/%d/abort", targetMigrationBasePath, migrationID) - if err := c.post(ctx, path, nil, nil, http.StatusNoContent); err != nil { + if err := c.post(ctx, path, req, nil, http.StatusNoContent); err != nil { return fmt.Errorf("aborting target migration: %w", err) } return nil diff --git a/internal/elmapi/target_migrations_test.go b/internal/elmapi/target_migrations_test.go index c07f136..58071fd 100644 --- a/internal/elmapi/target_migrations_test.go +++ b/internal/elmapi/target_migrations_test.go @@ -8,10 +8,23 @@ import ( "net/url" "testing" + "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +const testTargetOperationID = "0f8fad5b-d9cb-469f-a165-70867728950e" + +func TestNewCustomerTargetMigrationTransitionRequest(t *testing.T) { + first := NewCustomerTargetMigrationTransitionRequest() + second := NewCustomerTargetMigrationTransitionRequest() + + assert.Equal(t, TargetMigrationInitiatorCustomer, first.Initiator) + require.NoError(t, uuid.Validate(first.OperationID)) + require.NoError(t, uuid.Validate(second.OperationID)) + assert.NotEqual(t, first.OperationID, second.OperationID) +} + func TestCreateTargetMigration(t *testing.T) { t.Run("sends the request body and decodes the raw response", func(t *testing.T) { var gotPath, gotMethod string @@ -31,6 +44,8 @@ func TestCreateTargetMigration(t *testing.T) { Repositories: []string{"octo/repo"}, Description: "test migration", ExporterMigrationGUID: "11111111-1111-1111-1111-111111111111", + Initiator: TargetMigrationInitiatorCustomer, + OperationID: testTargetOperationID, }) require.NoError(t, err, "CreateTargetMigration") @@ -39,6 +54,9 @@ func TestCreateTargetMigration(t *testing.T) { assert.Equal(t, "https://source.example/octo/repo", gotBody["source_url"]) assert.Equal(t, []any{"octo/repo"}, gotBody["repositories"]) assert.Equal(t, "test migration", gotBody["description"]) + assert.Equal(t, TargetMigrationInitiatorCustomer, gotBody["initiator"]) + assert.Equal(t, testTargetOperationID, gotBody["operation_id"]) + assert.NotContains(t, gotBody, "actor") assert.JSONEq(t, `{"migrationId":"42","expiresAt":"2024-01-01T00:00:00Z"}`, string(raw)) }) @@ -303,46 +321,64 @@ func TestGetTargetMigrationStatus(t *testing.T) { func TestPauseResumeAbortTargetMigration(t *testing.T) { t.Run("pause posts to the pause path and expects 204", func(t *testing.T) { var gotPath, gotMethod string + var gotBody map[string]any srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { gotPath = r.URL.Path gotMethod = r.Method + require.NoError(t, json.NewDecoder(r.Body).Decode(&gotBody)) w.WriteHeader(http.StatusNoContent) })) defer srv.Close() c := NewClient(srv.URL, "tok") - err := c.PauseTargetMigration(t.Context(), 42) + err := c.PauseTargetMigration(t.Context(), 42, targetTransitionRequestForTest()) require.NoError(t, err, "PauseTargetMigration") assert.Equal(t, http.MethodPost, gotMethod) assert.Equal(t, "/enterprise/migration/42/pause", gotPath) + assert.Equal(t, map[string]any{ + "initiator": TargetMigrationInitiatorCustomer, + "operation_id": testTargetOperationID, + }, gotBody) }) t.Run("resume posts to the resume path and expects 204", func(t *testing.T) { var gotPath string + var gotBody map[string]any srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { gotPath = r.URL.Path + require.NoError(t, json.NewDecoder(r.Body).Decode(&gotBody)) w.WriteHeader(http.StatusNoContent) })) defer srv.Close() c := NewClient(srv.URL, "tok") - err := c.ResumeTargetMigration(t.Context(), 42) + err := c.ResumeTargetMigration(t.Context(), 42, targetTransitionRequestForTest()) require.NoError(t, err, "ResumeTargetMigration") assert.Equal(t, "/enterprise/migration/42/resume", gotPath) + assert.Equal(t, map[string]any{ + "initiator": TargetMigrationInitiatorCustomer, + "operation_id": testTargetOperationID, + }, gotBody) }) t.Run("abort posts to the abort path and expects 204", func(t *testing.T) { var gotPath string + var gotBody map[string]any srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { gotPath = r.URL.Path + require.NoError(t, json.NewDecoder(r.Body).Decode(&gotBody)) w.WriteHeader(http.StatusNoContent) })) defer srv.Close() c := NewClient(srv.URL, "tok") - err := c.AbortTargetMigration(t.Context(), 42) + err := c.AbortTargetMigration(t.Context(), 42, targetTransitionRequestForTest()) require.NoError(t, err, "AbortTargetMigration") assert.Equal(t, "/enterprise/migration/42/abort", gotPath) + assert.Equal(t, map[string]any{ + "initiator": TargetMigrationInitiatorCustomer, + "operation_id": testTargetOperationID, + }, gotBody) }) t.Run("returns HTTPError with 412 on precondition failed", func(t *testing.T) { @@ -352,10 +388,17 @@ func TestPauseResumeAbortTargetMigration(t *testing.T) { defer srv.Close() c := NewClient(srv.URL, "tok") - err := c.PauseTargetMigration(t.Context(), 1) + err := c.PauseTargetMigration(t.Context(), 1, targetTransitionRequestForTest()) require.Error(t, err) var httpErr *HTTPError require.ErrorAs(t, err, &httpErr) assert.Equal(t, http.StatusPreconditionFailed, httpErr.StatusCode) }) } + +func targetTransitionRequestForTest() TargetMigrationTransitionRequest { + return TargetMigrationTransitionRequest{ + Initiator: TargetMigrationInitiatorCustomer, + OperationID: testTargetOperationID, + } +} diff --git a/internal/workflow/service.go b/internal/workflow/service.go index 52233bd..0165720 100644 --- a/internal/workflow/service.go +++ b/internal/workflow/service.go @@ -327,11 +327,14 @@ func (s *Service) CreateTargetMigration(ctx context.Context, in TargetCreateInpu if err != nil { return nil, err } + transition := elmapi.NewCustomerTargetMigrationTransitionRequest() return client.CreateTargetMigration(ctx, elmapi.CreateTargetMigrationRequest{ SourceURL: strings.TrimSpace(in.SourceRepositoryURL), Repositories: []string{strings.TrimSpace(in.Repository)}, Description: strings.TrimSpace(in.Description), ExporterMigrationGUID: strings.TrimSpace(in.ExporterGUID), + Initiator: transition.Initiator, + OperationID: transition.OperationID, }) } @@ -669,7 +672,11 @@ func (s *Service) sourceAction(ctx context.Context, id SourceMigrationID, action return action(client, ctx, string(id)) } -func (s *Service) targetAction(ctx context.Context, id TargetMigrationID, action func(*elmapi.Client, context.Context, int64) error) error { +func (s *Service) targetAction( + ctx context.Context, + id TargetMigrationID, + action func(*elmapi.Client, context.Context, int64, elmapi.TargetMigrationTransitionRequest) error, +) error { if err := requireTargetID(id); err != nil { return err } @@ -677,7 +684,8 @@ func (s *Service) targetAction(ctx context.Context, id TargetMigrationID, action if err != nil { return err } - return action(client, ctx, int64(id)) + req := elmapi.NewCustomerTargetMigrationTransitionRequest() + return action(client, ctx, int64(id), req) } func (s *Service) reportClient(in ReportInput) (*elmapi.Client, string, error) { diff --git a/internal/workflow/service_test.go b/internal/workflow/service_test.go index 91a6e10..2bd0c6e 100644 --- a/internal/workflow/service_test.go +++ b/internal/workflow/service_test.go @@ -6,6 +6,7 @@ import ( "net/http/httptest" "testing" + "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -137,6 +138,67 @@ func TestCheckAuthentication(t *testing.T) { assert.NoError(t, service.CheckTargetAuthentication(t.Context())) } +func TestTargetLifecycleRequests(t *testing.T) { + t.Setenv("GH_ELM_CONFIG_DIR", t.TempDir()) + t.Setenv("GH_ELM_CREDENTIAL_STORE", "file") + + bodies := make(map[string]map[string]any) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var body map[string]any + require.NoError(t, json.NewDecoder(r.Body).Decode(&body)) + bodies[r.URL.Path] = body + if r.URL.Path == "/enterprise/migration/create" { + w.WriteHeader(http.StatusCreated) + _, _ = w.Write([]byte(`{"migrationId":"42"}`)) + return + } + w.WriteHeader(http.StatusNoContent) + })) + t.Cleanup(server.Close) + + service := New() + require.NoError(t, service.SaveConfiguration(t.Context(), ConfigurationInput{ + SourceURL: "https://source.example", + TargetURL: server.URL, + TargetToken: "target-token", + })) + + _, err := service.CreateTargetMigration(t.Context(), TargetCreateInput{ + SourceRepositoryURL: "https://source.example/octo/repo", + Repository: "octo/repo", + Description: "test migration", + ExporterGUID: "11111111-1111-1111-1111-111111111111", + }) + require.NoError(t, err) + require.NoError(t, service.PauseTargetMigration(t.Context(), 42)) + require.NoError(t, service.ResumeTargetMigration(t.Context(), 42)) + require.NoError(t, service.AbortTargetMigration(t.Context(), 42)) + + createBody := bodies["/enterprise/migration/create"] + assert.Equal(t, "https://source.example/octo/repo", createBody["source_url"]) + assert.Equal(t, []any{"octo/repo"}, createBody["repositories"]) + assert.Equal(t, "test migration", createBody["description"]) + assert.Equal(t, "11111111-1111-1111-1111-111111111111", createBody["exporter_migration_guid"]) + + operationIDs := make([]string, 0, 4) + for _, path := range []string{ + "/enterprise/migration/create", + "/enterprise/migration/42/pause", + "/enterprise/migration/42/resume", + "/enterprise/migration/42/abort", + } { + body, ok := bodies[path] + require.True(t, ok, "missing request to %s", path) + operationIDs = append(operationIDs, assertWorkflowCustomerTransition(t, body)) + } + assert.Len(t, map[string]struct{}{ + operationIDs[0]: {}, + operationIDs[1]: {}, + operationIDs[2]: {}, + operationIDs[3]: {}, + }, 4, "each TUI action must use a fresh operation ID") +} + func TestRepositoryCatalog(t *testing.T) { t.Setenv("GH_ELM_CONFIG_DIR", t.TempDir()) t.Setenv("GH_ELM_CREDENTIAL_STORE", "file") @@ -209,3 +271,14 @@ func TestListSourceMigrations(t *testing.T) { assert.Equal(t, "created-1", migrations[0].MigrationID) assert.Equal(t, []string{"", "created"}, statuses) } + +func assertWorkflowCustomerTransition(t *testing.T, body map[string]any) string { + t.Helper() + + assert.Equal(t, elmapi.TargetMigrationInitiatorCustomer, body["initiator"]) + assert.NotContains(t, body, "actor") + operationID, ok := body["operation_id"].(string) + require.True(t, ok, "operation_id must be a string") + require.NoError(t, uuid.Validate(operationID)) + return operationID +} From 83fbc361b3f296dbc14163f42e035eb08805c92c Mon Sep 17 00:00:00 2001 From: "C. Ross" Date: Tue, 1 Sep 2026 08:32:36 -0400 Subject: [PATCH 2/3] Simplify target attribution metadata Use the customer initiator constant directly and generate only the operation ID at each lifecycle invocation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f16596bc-066c-4b27-80d6-6860b9063bab --- internal/cmd/target/migration.go | 20 ++++++++++++++------ internal/elmapi/target_migrations.go | 13 +++++-------- internal/elmapi/target_migrations_test.go | 15 +++++++-------- internal/workflow/service.go | 10 ++++++---- 4 files changed, 32 insertions(+), 26 deletions(-) diff --git a/internal/cmd/target/migration.go b/internal/cmd/target/migration.go index 6dcad59..db68b12 100644 --- a/internal/cmd/target/migration.go +++ b/internal/cmd/target/migration.go @@ -146,9 +146,8 @@ func newMigrationCreateCmd() *cobra.Command { Description: description, ExporterMigrationGUID: exporterGUID, } - transition := elmapi.NewCustomerTargetMigrationTransitionRequest() - req.Initiator = transition.Initiator - req.OperationID = transition.OperationID + req.Initiator = elmapi.TargetMigrationInitiatorCustomer + req.OperationID = elmapi.NewTargetMigrationOperationID() raw, err := client.CreateTargetMigration(cmd.Context(), req) if err != nil { return annotateAuthError(err, targetURLResolved) @@ -243,7 +242,10 @@ func newMigrationPauseCmd() *cobra.Command { if err != nil { return err } - req := elmapi.NewCustomerTargetMigrationTransitionRequest() + req := elmapi.TargetMigrationTransitionRequest{ + Initiator: elmapi.TargetMigrationInitiatorCustomer, + OperationID: elmapi.NewTargetMigrationOperationID(), + } if err := client.PauseTargetMigration(cmd.Context(), migrationID, req); err != nil { return annotateMigrationActionError(err, targetURLResolved, migrationID) } @@ -284,7 +286,10 @@ func newMigrationResumeCmd() *cobra.Command { if err != nil { return err } - req := elmapi.NewCustomerTargetMigrationTransitionRequest() + req := elmapi.TargetMigrationTransitionRequest{ + Initiator: elmapi.TargetMigrationInitiatorCustomer, + OperationID: elmapi.NewTargetMigrationOperationID(), + } if err := client.ResumeTargetMigration(cmd.Context(), migrationID, req); err != nil { return annotateMigrationActionError(err, targetURLResolved, migrationID) } @@ -328,7 +333,10 @@ func newMigrationAbortCmd() *cobra.Command { if err != nil { return err } - req := elmapi.NewCustomerTargetMigrationTransitionRequest() + req := elmapi.TargetMigrationTransitionRequest{ + Initiator: elmapi.TargetMigrationInitiatorCustomer, + OperationID: elmapi.NewTargetMigrationOperationID(), + } if err := client.AbortTargetMigration(cmd.Context(), migrationID, req); err != nil { return annotateMigrationActionError(err, targetURLResolved, migrationID) } diff --git a/internal/elmapi/target_migrations.go b/internal/elmapi/target_migrations.go index ab3138d..ac1f14c 100644 --- a/internal/elmapi/target_migrations.go +++ b/internal/elmapi/target_migrations.go @@ -48,14 +48,11 @@ type TargetMigrationTransitionRequest struct { OperationID string `json:"operation_id"` } -// NewCustomerTargetMigrationTransitionRequest returns metadata for one logical -// customer-initiated lifecycle mutation. Callers should reuse the returned -// request if they retry that mutation. -func NewCustomerTargetMigrationTransitionRequest() TargetMigrationTransitionRequest { - return TargetMigrationTransitionRequest{ - Initiator: TargetMigrationInitiatorCustomer, - OperationID: uuid.NewString(), - } +// NewTargetMigrationOperationID returns the operation ID for one logical +// target migration lifecycle mutation. Callers should reuse it if they retry +// that mutation. +func NewTargetMigrationOperationID() string { + return uuid.NewString() } // CreateTargetMigrationRequest is the body of a create-migration call against diff --git a/internal/elmapi/target_migrations_test.go b/internal/elmapi/target_migrations_test.go index 58071fd..4cda28d 100644 --- a/internal/elmapi/target_migrations_test.go +++ b/internal/elmapi/target_migrations_test.go @@ -15,14 +15,13 @@ import ( const testTargetOperationID = "0f8fad5b-d9cb-469f-a165-70867728950e" -func TestNewCustomerTargetMigrationTransitionRequest(t *testing.T) { - first := NewCustomerTargetMigrationTransitionRequest() - second := NewCustomerTargetMigrationTransitionRequest() - - assert.Equal(t, TargetMigrationInitiatorCustomer, first.Initiator) - require.NoError(t, uuid.Validate(first.OperationID)) - require.NoError(t, uuid.Validate(second.OperationID)) - assert.NotEqual(t, first.OperationID, second.OperationID) +func TestNewTargetMigrationOperationID(t *testing.T) { + first := NewTargetMigrationOperationID() + second := NewTargetMigrationOperationID() + + require.NoError(t, uuid.Validate(first)) + require.NoError(t, uuid.Validate(second)) + assert.NotEqual(t, first, second) } func TestCreateTargetMigration(t *testing.T) { diff --git a/internal/workflow/service.go b/internal/workflow/service.go index 0165720..1997f99 100644 --- a/internal/workflow/service.go +++ b/internal/workflow/service.go @@ -327,14 +327,13 @@ func (s *Service) CreateTargetMigration(ctx context.Context, in TargetCreateInpu if err != nil { return nil, err } - transition := elmapi.NewCustomerTargetMigrationTransitionRequest() return client.CreateTargetMigration(ctx, elmapi.CreateTargetMigrationRequest{ SourceURL: strings.TrimSpace(in.SourceRepositoryURL), Repositories: []string{strings.TrimSpace(in.Repository)}, Description: strings.TrimSpace(in.Description), ExporterMigrationGUID: strings.TrimSpace(in.ExporterGUID), - Initiator: transition.Initiator, - OperationID: transition.OperationID, + Initiator: elmapi.TargetMigrationInitiatorCustomer, + OperationID: elmapi.NewTargetMigrationOperationID(), }) } @@ -684,7 +683,10 @@ func (s *Service) targetAction( if err != nil { return err } - req := elmapi.NewCustomerTargetMigrationTransitionRequest() + req := elmapi.TargetMigrationTransitionRequest{ + Initiator: elmapi.TargetMigrationInitiatorCustomer, + OperationID: elmapi.NewTargetMigrationOperationID(), + } return action(client, ctx, int64(id), req) } From 3f048d56a34171a4453c615758e503d87da9554c Mon Sep 17 00:00:00 2001 From: "C. Ross" Date: Tue, 1 Sep 2026 09:09:21 -0400 Subject: [PATCH 3/3] Encapsulate target transition request metadata Construct typed lifecycle request metadata in elmapi while retaining public initiator and operation ID helpers for create requests. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f16596bc-066c-4b27-80d6-6860b9063bab --- internal/cmd/target/migration.go | 15 +++------------ internal/elmapi/target_migrations.go | 9 +++++++++ internal/elmapi/target_migrations_test.go | 7 +++++++ internal/workflow/service.go | 5 +---- 4 files changed, 20 insertions(+), 16 deletions(-) diff --git a/internal/cmd/target/migration.go b/internal/cmd/target/migration.go index db68b12..3a0a17e 100644 --- a/internal/cmd/target/migration.go +++ b/internal/cmd/target/migration.go @@ -242,10 +242,7 @@ func newMigrationPauseCmd() *cobra.Command { if err != nil { return err } - req := elmapi.TargetMigrationTransitionRequest{ - Initiator: elmapi.TargetMigrationInitiatorCustomer, - OperationID: elmapi.NewTargetMigrationOperationID(), - } + req := elmapi.NewTargetMigrationTransitionRequest() if err := client.PauseTargetMigration(cmd.Context(), migrationID, req); err != nil { return annotateMigrationActionError(err, targetURLResolved, migrationID) } @@ -286,10 +283,7 @@ func newMigrationResumeCmd() *cobra.Command { if err != nil { return err } - req := elmapi.TargetMigrationTransitionRequest{ - Initiator: elmapi.TargetMigrationInitiatorCustomer, - OperationID: elmapi.NewTargetMigrationOperationID(), - } + req := elmapi.NewTargetMigrationTransitionRequest() if err := client.ResumeTargetMigration(cmd.Context(), migrationID, req); err != nil { return annotateMigrationActionError(err, targetURLResolved, migrationID) } @@ -333,10 +327,7 @@ func newMigrationAbortCmd() *cobra.Command { if err != nil { return err } - req := elmapi.TargetMigrationTransitionRequest{ - Initiator: elmapi.TargetMigrationInitiatorCustomer, - OperationID: elmapi.NewTargetMigrationOperationID(), - } + req := elmapi.NewTargetMigrationTransitionRequest() if err := client.AbortTargetMigration(cmd.Context(), migrationID, req); err != nil { return annotateMigrationActionError(err, targetURLResolved, migrationID) } diff --git a/internal/elmapi/target_migrations.go b/internal/elmapi/target_migrations.go index ac1f14c..818a959 100644 --- a/internal/elmapi/target_migrations.go +++ b/internal/elmapi/target_migrations.go @@ -55,6 +55,15 @@ func NewTargetMigrationOperationID() string { return uuid.NewString() } +// NewTargetMigrationTransitionRequest returns attribution metadata for one +// customer-initiated target migration lifecycle mutation. +func NewTargetMigrationTransitionRequest() TargetMigrationTransitionRequest { + return TargetMigrationTransitionRequest{ + Initiator: TargetMigrationInitiatorCustomer, + OperationID: NewTargetMigrationOperationID(), + } +} + // CreateTargetMigrationRequest is the body of a create-migration call against // the target (GHEC/Proxima) migration-management API. Repositories currently // accepts exactly one entry; the API does not yet support multi-repository diff --git a/internal/elmapi/target_migrations_test.go b/internal/elmapi/target_migrations_test.go index 4cda28d..21def45 100644 --- a/internal/elmapi/target_migrations_test.go +++ b/internal/elmapi/target_migrations_test.go @@ -24,6 +24,13 @@ func TestNewTargetMigrationOperationID(t *testing.T) { assert.NotEqual(t, first, second) } +func TestNewTargetMigrationTransitionRequest(t *testing.T) { + req := NewTargetMigrationTransitionRequest() + + assert.Equal(t, TargetMigrationInitiatorCustomer, req.Initiator) + require.NoError(t, uuid.Validate(req.OperationID)) +} + func TestCreateTargetMigration(t *testing.T) { t.Run("sends the request body and decodes the raw response", func(t *testing.T) { var gotPath, gotMethod string diff --git a/internal/workflow/service.go b/internal/workflow/service.go index 1997f99..51ad331 100644 --- a/internal/workflow/service.go +++ b/internal/workflow/service.go @@ -683,10 +683,7 @@ func (s *Service) targetAction( if err != nil { return err } - req := elmapi.TargetMigrationTransitionRequest{ - Initiator: elmapi.TargetMigrationInitiatorCustomer, - OperationID: elmapi.NewTargetMigrationOperationID(), - } + req := elmapi.NewTargetMigrationTransitionRequest() return action(client, ctx, int64(id), req) }