From 3cbbfbb0de16fed11869e5a531fa7d9055d41f5d Mon Sep 17 00:00:00 2001 From: Jim Boyle <95828167+boylejj@users.noreply.github.com> Date: Thu, 17 Sep 2026 13:00:11 -0400 Subject: [PATCH 1/3] Display observed source repository archive state Render the nullable source archive observation in migration status, TUI details, and watch independently of target progress. Preserve raw JSON and existing lifecycle and refresh behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- README.md | 9 ++ integration/cli_test.go | 61 +++++++++++ internal/cmd/migration/migration_test.go | 44 +++++++- internal/cmd/migration/watch/view.go | 8 +- internal/cmd/migration/watch/watch_test.go | 117 ++++++++++++++++++++- internal/elmapi/migrations.go | 16 +-- internal/elmapi/migrations_test.go | 60 +++++++++++ internal/render/migration.go | 31 +++--- internal/render/migration_test.go | 72 ++++++++++++- internal/tui/model_test.go | 60 +++++++++++ 10 files changed, 446 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index de9476d..6704b2a 100644 --- a/README.md +++ b/README.md @@ -117,6 +117,15 @@ gh elm migration cutover revert gh elm migration cutover revert --json | jq .success ``` +Migration status, live watch (`gh elm migration watch `), and TUI details +show the source repository's observed archive state separately from migration +progress and completion. The `source_repository_archived` response field is +`true`, `false`, or `null`. A missing or null field, including on servers that do +not support the observation, displays "Source repository archive state unavailable". +The observation is a sample, not proof of a successful archive operation or a +guarantee that an unarchived repository is writable. `--json` preserves the complete +response, including this field and legacy progress fields. + Look up a migration's destination (GitHub with Data Residency) migration ID — `gh elm migration target-id` (human-readable by default; add `--json` for a machine-readable object). The numeric target migration ID it returns is the positional diff --git a/integration/cli_test.go b/integration/cli_test.go index 1a74379..ea5f769 100644 --- a/integration/cli_test.go +++ b/integration/cli_test.go @@ -61,6 +61,67 @@ func TestInvalidCommand(t *testing.T) { } func TestMigrationStatus(t *testing.T) { + t.Run("source archive observation and raw JSON", func(t *testing.T) { + cases := []struct { + name string + body string + want string + }{ + {"true", `{"source_repository_archived":true}`, "Source repository archived"}, + {"false with completed legacy true", `{"source_repository_archived":false,"combined_state":{"status":"completed"},"target_state":{"repository_progress":[{"repository_locked":true}]},"future_field":{"value":1}}`, "Source repository not archived"}, + {"null", `{"source_repository_archived":null,"migration":{}}`, "Source repository archive state unavailable"}, + {"absent", `{"migration":{}}`, "Source repository archive state unavailable"}, + {"empty", `{}`, "No migration status data returned."}, + {"null only", `{"source_repository_archived":null}`, "No migration status data returned."}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + var requests atomic.Int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests.Add(1) + assert.Equal(t, http.MethodGet, r.Method) + assert.Equal(t, "/api/v3/enterprise/live-migrations/mig-1", r.URL.Path) + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(tc.body)) + })) + t.Cleanup(srv.Close) + args := []string{"migration", "status", "mig-1", "--source-url", srv.URL, "--source-token", "fixture-token"} + result := runCLI(t, nil, args...) + require.Zero(t, result.ExitCode, result.Stderr) + assert.Empty(t, result.Stderr) + assert.Contains(t, result.Stdout, tc.want) + assert.Equal(t, 1, strings.Count(result.Stdout, tc.want)) + assert.NotContains(t, result.Stdout, "Source repository locked") + assert.NotContains(t, result.Stdout, "Source repository unlocked") + t.Logf("Controlled-response CLI output:\n%s", result.Stdout) + + result = runCLI(t, nil, append(args, "--json")...) + require.Zero(t, result.ExitCode, result.Stderr) + assert.Empty(t, result.Stderr) + assert.JSONEq(t, tc.body, result.Stdout) + assert.Equal(t, int32(2), requests.Load()) + }) + } + }) + + t.Run("invalid observation is an error only for typed human output", func(t *testing.T) { + const response = `{"source_repository_archived":"unexpected","future_field":[false,null,42]}` + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(response)) + })) + t.Cleanup(srv.Close) + args := []string{"migration", "status", "mig-1", "--source-url", srv.URL, "--source-token", "fixture-token"} + result := runCLI(t, nil, args...) + require.NotZero(t, result.ExitCode) + assert.Empty(t, result.Stdout) + assert.Contains(t, result.Stderr, "source_repository_archived") + + result = runCLI(t, nil, append(args, "--json")...) + require.Zero(t, result.ExitCode, result.Stderr) + assert.Empty(t, result.Stderr) + assert.JSONEq(t, response, result.Stdout) + }) + t.Run("succeeds", func(t *testing.T) { const response = `{"migration":{"migration_id":"mig-1","status":"in_progress"}}` diff --git a/internal/cmd/migration/migration_test.go b/internal/cmd/migration/migration_test.go index 690b26a..4265e56 100644 --- a/internal/cmd/migration/migration_test.go +++ b/internal/cmd/migration/migration_test.go @@ -263,6 +263,48 @@ func TestStart(t *testing.T) { } func TestStatus(t *testing.T) { + t.Run("renders source-only observations and empty documents", func(t *testing.T) { + cases := []struct { + body string + want string + }{ + {`{"source_repository_archived":true}`, "Source repository archived"}, + {`{"source_repository_archived":false}`, "Source repository not archived"}, + {`{"migration":{},"source_repository_archived":null}`, "Source repository archive state unavailable"}, + {`{"migration":{}}`, "Source repository archive state unavailable"}, + {`{}`, "No migration status data returned."}, + {`{"source_repository_archived":null}`, "No migration status data returned."}, + {`null`, "No migration status data returned."}, + } + for _, tc := range cases { + t.Run(tc.body, func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(tc.body)) + })) + t.Cleanup(srv.Close) + + out := run(t, "status", "mig-1", "--source-url", srv.URL, "--source-token", "tok") + assert.Contains(t, out, tc.want) + assert.NotContains(t, out, "Source repository locked") + assert.NotContains(t, out, "Source repository unlocked") + }) + } + }) + + t.Run("invalid observation fails human output but survives raw JSON", func(t *testing.T) { + const body = `{"source_repository_archived":"unexpected","future_field":{"value":1}}` + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(body)) + })) + t.Cleanup(srv.Close) + + out, err := exec(t, "status", "mig-1", "--source-url", srv.URL, "--source-token", "tok") + require.ErrorContains(t, err, "source_repository_archived") + assert.NotContains(t, out, "Source repository archived") + out = run(t, "status", "mig-1", "--json", "--source-url", srv.URL, "--source-token", "tok") + assert.JSONEq(t, body, out) + }) + t.Run("prints human-readable status", func(t *testing.T) { const respBody = `{"migration":{"migration_id":"mig-1","status":"in_progress"},"target_state":null,"combined_state":null,"messages":[]}` srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -280,7 +322,7 @@ func TestStatus(t *testing.T) { }) t.Run("--json preserves the raw status response", func(t *testing.T) { - const respBody = `{"migration":{"migration_id":"mig-1"},"future_field":{"value":1}}` + const respBody = `{"migration":{"migration_id":"mig-1"},"source_repository_archived":false,"target_state":{"repository_progress":[{"repository_locked":true}]},"future_field":{"value":1}}` srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { _, _ = w.Write([]byte(respBody)) })) diff --git a/internal/cmd/migration/watch/view.go b/internal/cmd/migration/watch/view.go index 8aec740..7ec6abd 100644 --- a/internal/cmd/migration/watch/view.go +++ b/internal/cmd/migration/watch/view.go @@ -5,6 +5,7 @@ import ( "strings" "time" + "github.com/github/gh-elm/internal/render" "github.com/github/gh-elm/internal/theme" ) @@ -37,6 +38,8 @@ func (m Model) View() string { var b strings.Builder w(&b, m.renderHeader()) + w(&b, render.SourceRepositoryArchiveState(m.detail.SourceRepositoryArchived)) + w(&b, "\n") w(&b, "\n") w(&b, m.renderTimeline()) w(&b, m.renderPreflight()) @@ -308,13 +311,12 @@ func (m Model) cutoverDetail() string { return "Waiting for cutover status..." } - locked := boolCheck(rp.RepositoryLocked, m.styles) gitPush := boolCheck(rp.InitialGitPushComplete, m.styles) resourcesSent := boolCheck(rp.AllResourcesSent, m.styles) var b strings.Builder - w(&b, fmt.Sprintf("Repo locked: %s Git push: %s All resources sent: %s", - locked, gitPush, resourcesSent)) + w(&b, fmt.Sprintf("Git push: %s All resources sent: %s", + gitPush, resourcesSent)) if cs := m.detail.CombinedState; cs != nil && cs.DisplayMessage != "" { w(&b, "\n"+m.styles.Warning.Render(cs.DisplayMessage)) diff --git a/internal/cmd/migration/watch/watch_test.go b/internal/cmd/migration/watch/watch_test.go index 709e148..4c96778 100644 --- a/internal/cmd/migration/watch/watch_test.go +++ b/internal/cmd/migration/watch/watch_test.go @@ -1,10 +1,14 @@ package watch import ( + "net/http" + "net/http/httptest" + "strings" "testing" "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/github/gh-elm/internal/elmapi" ) @@ -90,6 +94,55 @@ func TestDerivePhase(t *testing.T) { } func TestView(t *testing.T) { + t.Run("source observation stays above timeline through cutover and completion", func(t *testing.T) { + cases := []struct { + name string + archived *bool + want string + }{ + {"true", new(true), "Source repository archived"}, + {"false", new(false), "Source repository not archived"}, + {"unavailable", nil, "Source repository archive state unavailable"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + m := New("mig-1", time.Second, nil) + m.width = 60 + m.detail = &elmapi.MigrationDetail{ + SourceRepositoryArchived: tc.archived, + TargetState: &elmapi.TargetState{RepositoryProgress: []elmapi.RepositoryProgress{{ + RepositoryLocked: true, + InitialGitPushComplete: true, + AllResourcesSent: true, + }}}, + } + m.basePhase = PhaseCuttingOver + out := m.View() + assert.Contains(t, out, tc.want) + assert.Equal(t, 1, strings.Count(out, "Source repository")) + assert.Less(t, strings.Index(out, tc.want), strings.Index(out, "Created")) + assert.NotContains(t, out, "Repo locked") + assert.Contains(t, out, "Git push: ✓ All resources sent: ✓") + + m.basePhase = PhaseCompleted + m.detail.TargetState = nil + out = m.View() + assert.Contains(t, out, tc.want) + assert.Equal(t, 1, strings.Count(out, "Source repository")) + assert.Contains(t, out, "Migration completed successfully.") + }) + } + }) + + t.Run("source-only response and empty response preserve existing timeline", func(t *testing.T) { + m := New("id", time.Second, nil) + m.detail = &elmapi.MigrationDetail{SourceRepositoryArchived: new(false)} + assert.Contains(t, m.View(), "Source repository not archived") + m.detail = &elmapi.MigrationDetail{} + assert.Contains(t, m.View(), "Source repository archive state unavailable") + assert.Contains(t, m.View(), "Created") + }) + t.Run("renders timeline and progress", func(t *testing.T) { m := New("11112222-3333-4444-5555-666677778888", 2*time.Second, nil) m.detail = &elmapi.MigrationDetail{ @@ -116,6 +169,7 @@ func TestView(t *testing.T) { {MessageType: "info", Message: "hello world"}, }, } + m.basePhase, m.overlay = DerivePhase(m.detail) out := m.View() @@ -136,6 +190,67 @@ func TestView(t *testing.T) { t.Run("loading", func(t *testing.T) { m := New("id", time.Second, nil) - assert.Contains(t, m.View(), "Loading migration status") + assert.Equal(t, "Loading migration status...\n", m.View()) + }) +} + +func TestUpdate(t *testing.T) { + t.Run("successful refresh replaces archive observation and request failure retains it", func(t *testing.T) { + type response struct { + body string + status int + } + responses := make(chan response, 1) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + next := <-responses + w.WriteHeader(next.status) + _, _ = w.Write([]byte(next.body)) + })) + t.Cleanup(srv.Close) + m := New("mig-1", time.Second, elmapi.NewClient(srv.URL, "tok")) + cases := []struct { + body string + want *bool + text string + }{ + {`{"source_repository_archived":true}`, new(true), "Source repository archived"}, + {`{"source_repository_archived":false}`, new(false), "Source repository not archived"}, + {`{"source_repository_archived":true}`, new(true), "Source repository archived"}, + {`{"source_repository_archived":null}`, nil, "Source repository archive state unavailable"}, + {`{"source_repository_archived":true}`, new(true), "Source repository archived"}, + {`{}`, nil, "Source repository archive state unavailable"}, + } + for _, tc := range cases { + responses <- response{body: tc.body, status: http.StatusOK} + msg := fetchStatus(m.client, m.migrationID, m.interval) + updated, cmd := m.Update(msg) + m = updated.(Model) + require.NoError(t, m.fetchErr) + require.NotNil(t, cmd) + assert.Equal(t, tc.want, m.detail.SourceRepositoryArchived) + assert.Contains(t, m.View(), tc.text) + } + + responses <- response{body: `{"source_repository_archived":true}`, status: http.StatusOK} + updated, _ := m.Update(fetchStatus(m.client, m.migrationID, m.interval)) + m = updated.(Model) + lastDetail, lastUpdated := m.detail, m.lastUpdated + responses <- response{status: http.StatusServiceUnavailable} + updated, cmd := m.Update(fetchStatus(m.client, m.migrationID, m.interval)) + m = updated.(Model) + require.Error(t, m.fetchErr) + assert.NotNil(t, cmd) + assert.Same(t, lastDetail, m.detail) + assert.Equal(t, lastUpdated, m.lastUpdated) + assert.Contains(t, m.View(), "Source repository archived") + assert.Contains(t, m.View(), "Failed to refresh (retrying...)") + assert.Contains(t, m.View(), "Last updated: "+formatTimestamp(lastUpdated)) + + responses <- response{body: `{}`, status: http.StatusOK} + updated, _ = m.Update(fetchStatus(m.client, m.migrationID, m.interval)) + m = updated.(Model) + assert.NoError(t, m.fetchErr) + assert.Nil(t, m.detail.SourceRepositoryArchived) + assert.NotContains(t, m.View(), "Failed to refresh") }) } diff --git a/internal/elmapi/migrations.go b/internal/elmapi/migrations.go index 51db8f3..b0c6149 100644 --- a/internal/elmapi/migrations.go +++ b/internal/elmapi/migrations.go @@ -197,16 +197,16 @@ func (c *Client) migrationPath(migrationID string, action ...string) string { return p.String() } -// --- Typed views over the GET status document, used by `watch`. --- +// --- Typed views over the GET status document. --- -// MigrationDetail is a partial typed decode of the GET status document. Only the -// fields the watch renderer needs are modeled; the raw document (from -// GetMigration) is the source of truth for `status`. +// MigrationDetail is a partial typed decode for human-readable status displays. +// GetMigration preserves the complete raw document for JSON output. type MigrationDetail struct { - Migration *MigrationSummary `json:"migration"` - TargetState *TargetState `json:"target_state"` - CombinedState *CombinedState `json:"combined_state"` - Messages []MigrationMessage `json:"messages"` + Migration *MigrationSummary `json:"migration"` + SourceRepositoryArchived *bool `json:"source_repository_archived"` + TargetState *TargetState `json:"target_state"` + CombinedState *CombinedState `json:"combined_state"` + Messages []MigrationMessage `json:"messages"` } // MigrationSummary is the core migration record. diff --git a/internal/elmapi/migrations_test.go b/internal/elmapi/migrations_test.go index d395625..594afea 100644 --- a/internal/elmapi/migrations_test.go +++ b/internal/elmapi/migrations_test.go @@ -1,6 +1,7 @@ package elmapi import ( + "encoding/json" "net/http" "net/http/httptest" "testing" @@ -9,6 +10,65 @@ import ( "github.com/stretchr/testify/require" ) +func TestGetMigrationDetail(t *testing.T) { + t.Run("decodes nullable source archive observation independently of progress", func(t *testing.T) { + cases := []struct { + name string + body string + want *bool + }{ + {"true", `{"source_repository_archived":true}`, new(true)}, + {"false", `{"source_repository_archived":false}`, new(false)}, + {"null", `{"source_repository_archived":null}`, nil}, + {"absent", `{}`, nil}, + {"null document", `null`, nil}, + {"completed with disagreeing legacy progress", `{"source_repository_archived":false,"migration":{"status":"completed"},"target_state":{"repository_progress":[{"repository_locked":true}]}}`, new(false)}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, http.MethodGet, r.Method) + assert.Equal(t, "/enterprise/live-migrations/mig-1", r.URL.Path) + _, _ = w.Write([]byte(tc.body)) + })) + t.Cleanup(srv.Close) + + detail, err := NewClient(srv.URL, "tok").GetMigrationDetail(t.Context(), "mig-1") + require.NoError(t, err) + assert.Equal(t, tc.want, detail.SourceRepositoryArchived) + }) + } + }) + + t.Run("rejects invalid observation types", func(t *testing.T) { + for _, value := range []string{`"true"`, `1`, `{}`, `[]`} { + t.Run(value, func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(`{"source_repository_archived":` + value + `}`)) + })) + t.Cleanup(srv.Close) + + detail, err := NewClient(srv.URL, "tok").GetMigrationDetail(t.Context(), "mig-1") + var typeError *json.UnmarshalTypeError + require.ErrorAs(t, err, &typeError) + assert.Equal(t, "source_repository_archived", typeError.Field) + assert.Nil(t, detail) + }) + } + }) + + t.Run("preserves whole request failure", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusServiceUnavailable) + })) + t.Cleanup(srv.Close) + + detail, err := NewClient(srv.URL, "tok").GetMigrationDetail(t.Context(), "mig-1") + require.ErrorContains(t, err, "503") + assert.Nil(t, detail) + }) +} + func TestMigrationResponses(t *testing.T) { t.Run("create retains raw JSON while decoding typed fields", func(t *testing.T) { const body = `{"migration_id":"mig-1","expires_at":null,"future_field":"preserved"}` diff --git a/internal/render/migration.go b/internal/render/migration.go index f974113..a519b1d 100644 --- a/internal/render/migration.go +++ b/internal/render/migration.go @@ -29,18 +29,31 @@ func MigrationCancel(migrationID string) string { // MigrationStatus renders a migration status response. func MigrationStatus(v elmapi.MigrationDetail) string { - if v.Migration == nil && v.TargetState == nil && v.CombinedState == nil && len(v.Messages) == 0 { + if v.Migration == nil && v.SourceRepositoryArchived == nil && v.TargetState == nil && v.CombinedState == nil && len(v.Messages) == 0 { return "No migration status data returned.\n" } return joinSections( renderMigrationSummary(v.Migration), + renderSection("Source", " "+SourceRepositoryArchiveState(v.SourceRepositoryArchived)), renderTargetState(v.TargetState), renderCombinedState(v.CombinedState), renderMessages(v.Messages), ) } +// SourceRepositoryArchiveState renders a nullable source observation, not migration progress. +func SourceRepositoryArchiveState(archived *bool) string { + styles := theme.New() + if archived == nil { + return styles.Muted.Render("Source repository archive state unavailable") + } + if *archived { + return styles.Primary.Render("Source repository archived") + } + return styles.Primary.Render("Source repository not archived") +} + // CutoverStatus renders the cutover portion of a migration status response. func CutoverStatus(v elmapi.MigrationDetail) string { if v.CombinedState == nil { @@ -106,11 +119,9 @@ func renderRepositoryProgress(progress elmapi.RepositoryProgress) string { resources := positiveState(progress.AllResourcesSent, "All resources sent", "Resources still being sent") gitPush := positiveState(progress.InitialGitPushComplete, "Initial Git push complete", "Initial Git push pending") - lock := neutralState(progress.RepositoryLocked, "Source repository locked", "Source repository unlocked") lines = append(lines, bullet(resources.glyph, resources.text), bullet(gitPush.glyph, gitPush.text), - bullet(lock.glyph, lock.text), ) return renderSection("Progress · "+valueOrEmpty(progress.RepositoryNWO), lines...) @@ -367,20 +378,6 @@ func failureState(value bool, trueText, falseText string) state { } } -func neutralState(value bool, trueText, falseText string) state { - styles := theme.New() - if value { - return state{ - glyph: styles.Warning.Render("●"), - text: styles.Warning.Render(trueText), - } - } - return state{ - glyph: styles.Muted.Render("○"), - text: styles.Muted.Render(falseText), - } -} - func stateLine(value bool, trueText, falseText string) string { result := positiveState(value, trueText, falseText) return bullet(result.glyph, result.text) diff --git a/internal/render/migration_test.go b/internal/render/migration_test.go index edccf59..7160b05 100644 --- a/internal/render/migration_test.go +++ b/internal/render/migration_test.go @@ -3,6 +3,7 @@ package render import ( "bytes" "errors" + "strings" "testing" "github.com/charmbracelet/lipgloss" @@ -74,6 +75,48 @@ func TestMigrationCancel(t *testing.T) { } func TestMigrationStatus(t *testing.T) { + t.Run("source observation is independent of target progress and completion", func(t *testing.T) { + cases := []struct { + name string + archived *bool + locked bool + want string + }{ + {"archived with legacy false", new(true), false, "Source repository archived"}, + {"not archived with legacy true", new(false), true, "Source repository not archived"}, + {"unavailable with legacy true", nil, true, "Source repository archive state unavailable"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + output := MigrationStatus(elmapi.MigrationDetail{ + SourceRepositoryArchived: tc.archived, + CombinedState: &elmapi.CombinedState{Status: new("completed")}, + TargetState: &elmapi.TargetState{RepositoryProgress: []elmapi.RepositoryProgress{ + {RepositoryNWO: "target/one", RepositoryLocked: tc.locked}, + {RepositoryNWO: "target/two", RepositoryLocked: tc.locked}, + }}, + }) + assert.Contains(t, output, tc.want) + assert.Equal(t, 1, strings.Count(output, "Source repository")) + assert.Less(t, strings.Index(output, tc.want), strings.Index(output, "Target\n")) + assert.Contains(t, output, "Completed") + assert.Contains(t, output, "Progress · target/two") + assert.NotContains(t, output, "Source repository locked") + assert.NotContains(t, output, "Source repository unlocked") + }) + } + }) + + t.Run("renders source-only true", func(t *testing.T) { + assert.Equal(t, "Source\n Source repository archived\n", + MigrationStatus(elmapi.MigrationDetail{SourceRepositoryArchived: new(true)})) + }) + + t.Run("renders source-only false", func(t *testing.T) { + assert.Equal(t, "Source\n Source repository not archived\n", + MigrationStatus(elmapi.MigrationDetail{SourceRepositoryArchived: new(false)})) + }) + t.Run("renders nested status sections", func(t *testing.T) { status := "in_progress" phase := "backfill" @@ -136,7 +179,10 @@ func TestMigrationStatus(t *testing.T) { }, }) - assert.Equal(t, `Cutover + assert.Equal(t, `Source + Source repository archive state unavailable + +Cutover ✓ Ready for cutover Repository states @@ -156,7 +202,10 @@ Repository states }, }) - assert.Equal(t, `Cutover + assert.Equal(t, `Source + Source repository archive state unavailable + +Cutover ✓ Completed Migration completed successfully `, output) @@ -183,6 +232,25 @@ Repository states }) } +func TestSourceRepositoryArchiveState(t *testing.T) { + previousProfile := lipgloss.ColorProfile() + lipgloss.SetColorProfile(termenv.ANSI256) + t.Cleanup(func() { + lipgloss.SetColorProfile(previousProfile) + }) + styles := theme.New() + + t.Run("true is a neutral fact", func(t *testing.T) { + assert.Equal(t, styles.Primary.Render("Source repository archived"), SourceRepositoryArchiveState(new(true))) + }) + t.Run("false is a neutral fact", func(t *testing.T) { + assert.Equal(t, styles.Primary.Render("Source repository not archived"), SourceRepositoryArchiveState(new(false))) + }) + t.Run("nil is explicitly unavailable", func(t *testing.T) { + assert.Equal(t, styles.Muted.Render("Source repository archive state unavailable"), SourceRepositoryArchiveState(nil)) + }) +} + func TestProgressBar(t *testing.T) { t.Run("renders proportional progress", func(t *testing.T) { assert.Equal(t, "████████░░", ProgressBar(8, 10, 10)) diff --git a/internal/tui/model_test.go b/internal/tui/model_test.go index cdf27dd..4eeea7a 100644 --- a/internal/tui/model_test.go +++ b/internal/tui/model_test.go @@ -333,6 +333,66 @@ func TestModel(t *testing.T) { assert.Zero(t, model.targetID) }) + t.Run("source archive refresh replaces observations and retains detail on request failure", func(t *testing.T) { + model := New(t.Context(), &fakeService{}) + model.screen = screenSourceDetail + model.width, model.height = 100, 60 + model.sourceWatching = true + cases := []struct { + body string + want *bool + text string + }{ + {`{"source_repository_archived":true}`, new(true), "Source repository archived"}, + {`{"source_repository_archived":false}`, new(false), "Source repository not archived"}, + {`{"source_repository_archived":true}`, new(true), "Source repository archived"}, + {`{"migration":{},"source_repository_archived":null}`, nil, "Source repository archive state unavailable"}, + {`{"source_repository_archived":true}`, new(true), "Source repository archived"}, + {`{"migration":{}}`, nil, "Source repository archive state unavailable"}, + } + for _, tc := range cases { + var detail elmapi.MigrationDetail + require.NoError(t, json.Unmarshal([]byte(tc.body), &detail)) + updated, cmd := model.Update(sourceDetailMsg{detail: &detail}) + model = updated.(*Model) + require.NoError(t, model.err) + assert.NotNil(t, cmd) + assert.Same(t, &detail, model.sourceDetail) + assert.Equal(t, tc.want, model.sourceDetail.SourceRepositoryArchived) + assert.Contains(t, model.View(), tc.text) + assert.Equal(t, 1, strings.Count(model.View(), "Source repository")) + } + + previous := &elmapi.MigrationDetail{SourceRepositoryArchived: new(true)} + _, _ = model.Update(sourceDetailMsg{detail: previous}) + _, cmd := model.Update(sourceDetailMsg{err: assert.AnError}) + assert.ErrorIs(t, model.err, assert.AnError) + assert.Same(t, previous, model.sourceDetail) + assert.NotNil(t, cmd) + assert.Contains(t, model.View(), "Source repository archived") + assert.Contains(t, model.View(), assert.AnError.Error()) + }) + + t.Run("completed source detail with disagreeing target progress displays archive state once", func(t *testing.T) { + model := New(t.Context(), &fakeService{}) + model.screen = screenSourceDetail + model.width, model.height = 100, 60 + model.sourceDetail = &elmapi.MigrationDetail{ + SourceRepositoryArchived: new(false), + CombinedState: &elmapi.CombinedState{Status: new("completed")}, + TargetState: &elmapi.TargetState{RepositoryProgress: []elmapi.RepositoryProgress{ + {RepositoryNWO: "target/one", RepositoryLocked: true}, + {RepositoryNWO: "target/two", RepositoryLocked: true}, + }}, + } + out := model.View() + assert.Contains(t, out, "Source repository not archived") + assert.Equal(t, 1, strings.Count(out, "Source repository")) + assert.Contains(t, out, "Completed") + assert.NotContains(t, out, "Source repository locked") + assert.NotContains(t, out, "Source repository unlocked") + }) + t.Run("source actions remain visible in a standard terminal", func(t *testing.T) { model := New(t.Context(), &fakeService{}) model.screen = screenSourceDetail From 312b00177fb825334f909c7c1eddec9af090dec5 Mon Sep 17 00:00:00 2001 From: Jim Boyle <95828167+boylejj@users.noreply.github.com> Date: Thu, 17 Sep 2026 13:52:29 -0400 Subject: [PATCH 2/3] Read source archive state from migration metadata Decode migration.source_repository_archived and render unavailable when migration metadata or the observation is missing. Update status, watch, TUI, and raw-response fixtures without changing migration progress or refresh behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- README.md | 2 +- integration/cli_test.go | 13 +++++---- internal/cmd/migration/migration_test.go | 14 +++++---- internal/cmd/migration/watch/view.go | 2 +- internal/cmd/migration/watch/watch_test.go | 27 +++++++++-------- internal/elmapi/migrations.go | 34 +++++++++++----------- internal/elmapi/migrations_test.go | 34 ++++++++++++++++------ internal/render/migration.go | 10 +++---- internal/render/migration_test.go | 26 +++++++++++------ internal/tui/model_test.go | 24 ++++++++------- 10 files changed, 110 insertions(+), 76 deletions(-) diff --git a/README.md b/README.md index 6704b2a..e9e8b1a 100644 --- a/README.md +++ b/README.md @@ -119,7 +119,7 @@ gh elm migration cutover revert --json | jq .success Migration status, live watch (`gh elm migration watch `), and TUI details show the source repository's observed archive state separately from migration -progress and completion. The `source_repository_archived` response field is +progress and completion. The `migration.source_repository_archived` response field is `true`, `false`, or `null`. A missing or null field, including on servers that do not support the observation, displays "Source repository archive state unavailable". The observation is a sample, not proof of a successful archive operation or a diff --git a/integration/cli_test.go b/integration/cli_test.go index ea5f769..c5cf02c 100644 --- a/integration/cli_test.go +++ b/integration/cli_test.go @@ -67,12 +67,15 @@ func TestMigrationStatus(t *testing.T) { body string want string }{ - {"true", `{"source_repository_archived":true}`, "Source repository archived"}, - {"false with completed legacy true", `{"source_repository_archived":false,"combined_state":{"status":"completed"},"target_state":{"repository_progress":[{"repository_locked":true}]},"future_field":{"value":1}}`, "Source repository not archived"}, - {"null", `{"source_repository_archived":null,"migration":{}}`, "Source repository archive state unavailable"}, + {"true", `{"migration":{"source_repository_archived":true}}`, "Source repository archived"}, + {"false with completed legacy true", `{"migration":{"source_repository_archived":false,"future_field":{"value":1}},"combined_state":{"status":"completed"},"target_state":{"repository_progress":[{"repository_locked":true}]},"future_field":{"value":1}}`, "Source repository not archived"}, + {"null", `{"migration":{"source_repository_archived":null}}`, "Source repository archive state unavailable"}, {"absent", `{"migration":{}}`, "Source repository archive state unavailable"}, + {"missing migration", `{"combined_state":{"status":"completed"}}`, "Source repository archive state unavailable"}, + {"null migration", `{"migration":null,"combined_state":{"status":"completed"}}`, "Source repository archive state unavailable"}, + {"ignores top-level observation", `{"migration":{},"source_repository_archived":true}`, "Source repository archive state unavailable"}, {"empty", `{}`, "No migration status data returned."}, - {"null only", `{"source_repository_archived":null}`, "No migration status data returned."}, + {"null only", `{"migration":null}`, "No migration status data returned."}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { @@ -105,7 +108,7 @@ func TestMigrationStatus(t *testing.T) { }) t.Run("invalid observation is an error only for typed human output", func(t *testing.T) { - const response = `{"source_repository_archived":"unexpected","future_field":[false,null,42]}` + const response = `{"migration":{"source_repository_archived":"unexpected"},"future_field":[false,null,42]}` srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { _, _ = w.Write([]byte(response)) })) diff --git a/internal/cmd/migration/migration_test.go b/internal/cmd/migration/migration_test.go index 4265e56..ef1aa5e 100644 --- a/internal/cmd/migration/migration_test.go +++ b/internal/cmd/migration/migration_test.go @@ -268,12 +268,14 @@ func TestStatus(t *testing.T) { body string want string }{ - {`{"source_repository_archived":true}`, "Source repository archived"}, - {`{"source_repository_archived":false}`, "Source repository not archived"}, - {`{"migration":{},"source_repository_archived":null}`, "Source repository archive state unavailable"}, + {`{"migration":{"source_repository_archived":true}}`, "Source repository archived"}, + {`{"migration":{"source_repository_archived":false}}`, "Source repository not archived"}, + {`{"migration":{"source_repository_archived":null}}`, "Source repository archive state unavailable"}, {`{"migration":{}}`, "Source repository archive state unavailable"}, + {`{"target_state":{}}`, "Source repository archive state unavailable"}, + {`{"migration":null,"target_state":{}}`, "Source repository archive state unavailable"}, {`{}`, "No migration status data returned."}, - {`{"source_repository_archived":null}`, "No migration status data returned."}, + {`{"migration":null}`, "No migration status data returned."}, {`null`, "No migration status data returned."}, } for _, tc := range cases { @@ -292,7 +294,7 @@ func TestStatus(t *testing.T) { }) t.Run("invalid observation fails human output but survives raw JSON", func(t *testing.T) { - const body = `{"source_repository_archived":"unexpected","future_field":{"value":1}}` + const body = `{"migration":{"source_repository_archived":"unexpected"},"future_field":{"value":1}}` srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { _, _ = w.Write([]byte(body)) })) @@ -322,7 +324,7 @@ func TestStatus(t *testing.T) { }) t.Run("--json preserves the raw status response", func(t *testing.T) { - const respBody = `{"migration":{"migration_id":"mig-1"},"source_repository_archived":false,"target_state":{"repository_progress":[{"repository_locked":true}]},"future_field":{"value":1}}` + const respBody = `{"migration":{"migration_id":"mig-1","source_repository_archived":false,"future_field":{"value":1}},"target_state":{"repository_progress":[{"repository_locked":true}]},"future_field":{"value":1}}` srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { _, _ = w.Write([]byte(respBody)) })) diff --git a/internal/cmd/migration/watch/view.go b/internal/cmd/migration/watch/view.go index 7ec6abd..a39986c 100644 --- a/internal/cmd/migration/watch/view.go +++ b/internal/cmd/migration/watch/view.go @@ -38,7 +38,7 @@ func (m Model) View() string { var b strings.Builder w(&b, m.renderHeader()) - w(&b, render.SourceRepositoryArchiveState(m.detail.SourceRepositoryArchived)) + w(&b, render.SourceRepositoryArchiveState(m.detail.Migration)) w(&b, "\n") w(&b, "\n") w(&b, m.renderTimeline()) diff --git a/internal/cmd/migration/watch/watch_test.go b/internal/cmd/migration/watch/watch_test.go index 4c96778..9fed49c 100644 --- a/internal/cmd/migration/watch/watch_test.go +++ b/internal/cmd/migration/watch/watch_test.go @@ -109,7 +109,7 @@ func TestView(t *testing.T) { m := New("mig-1", time.Second, nil) m.width = 60 m.detail = &elmapi.MigrationDetail{ - SourceRepositoryArchived: tc.archived, + Migration: &elmapi.MigrationSummary{SourceRepositoryArchived: tc.archived}, TargetState: &elmapi.TargetState{RepositoryProgress: []elmapi.RepositoryProgress{{ RepositoryLocked: true, InitialGitPushComplete: true, @@ -136,7 +136,7 @@ func TestView(t *testing.T) { t.Run("source-only response and empty response preserve existing timeline", func(t *testing.T) { m := New("id", time.Second, nil) - m.detail = &elmapi.MigrationDetail{SourceRepositoryArchived: new(false)} + m.detail = &elmapi.MigrationDetail{Migration: &elmapi.MigrationSummary{SourceRepositoryArchived: new(false)}} assert.Contains(t, m.View(), "Source repository not archived") m.detail = &elmapi.MigrationDetail{} assert.Contains(t, m.View(), "Source repository archive state unavailable") @@ -210,15 +210,18 @@ func TestUpdate(t *testing.T) { m := New("mig-1", time.Second, elmapi.NewClient(srv.URL, "tok")) cases := []struct { body string - want *bool text string }{ - {`{"source_repository_archived":true}`, new(true), "Source repository archived"}, - {`{"source_repository_archived":false}`, new(false), "Source repository not archived"}, - {`{"source_repository_archived":true}`, new(true), "Source repository archived"}, - {`{"source_repository_archived":null}`, nil, "Source repository archive state unavailable"}, - {`{"source_repository_archived":true}`, new(true), "Source repository archived"}, - {`{}`, nil, "Source repository archive state unavailable"}, + {`{"migration":{"source_repository_archived":true}}`, "Source repository archived"}, + {`{"migration":{"source_repository_archived":false}}`, "Source repository not archived"}, + {`{"migration":{"source_repository_archived":true}}`, "Source repository archived"}, + {`{"migration":{"source_repository_archived":null}}`, "Source repository archive state unavailable"}, + {`{"migration":{"source_repository_archived":true}}`, "Source repository archived"}, + {`{"migration":{}}`, "Source repository archive state unavailable"}, + {`{"migration":{"source_repository_archived":true}}`, "Source repository archived"}, + {`{"migration":null}`, "Source repository archive state unavailable"}, + {`{"migration":{"source_repository_archived":true}}`, "Source repository archived"}, + {`{}`, "Source repository archive state unavailable"}, } for _, tc := range cases { responses <- response{body: tc.body, status: http.StatusOK} @@ -227,11 +230,11 @@ func TestUpdate(t *testing.T) { m = updated.(Model) require.NoError(t, m.fetchErr) require.NotNil(t, cmd) - assert.Equal(t, tc.want, m.detail.SourceRepositoryArchived) assert.Contains(t, m.View(), tc.text) + assert.Equal(t, 1, strings.Count(m.View(), "Source repository")) } - responses <- response{body: `{"source_repository_archived":true}`, status: http.StatusOK} + responses <- response{body: `{"migration":{"source_repository_archived":true}}`, status: http.StatusOK} updated, _ := m.Update(fetchStatus(m.client, m.migrationID, m.interval)) m = updated.(Model) lastDetail, lastUpdated := m.detail, m.lastUpdated @@ -250,7 +253,7 @@ func TestUpdate(t *testing.T) { updated, _ = m.Update(fetchStatus(m.client, m.migrationID, m.interval)) m = updated.(Model) assert.NoError(t, m.fetchErr) - assert.Nil(t, m.detail.SourceRepositoryArchived) + assert.Nil(t, m.detail.Migration) assert.NotContains(t, m.View(), "Failed to refresh") }) } diff --git a/internal/elmapi/migrations.go b/internal/elmapi/migrations.go index b0c6149..924e854 100644 --- a/internal/elmapi/migrations.go +++ b/internal/elmapi/migrations.go @@ -202,27 +202,27 @@ func (c *Client) migrationPath(migrationID string, action ...string) string { // MigrationDetail is a partial typed decode for human-readable status displays. // GetMigration preserves the complete raw document for JSON output. type MigrationDetail struct { - Migration *MigrationSummary `json:"migration"` - SourceRepositoryArchived *bool `json:"source_repository_archived"` - TargetState *TargetState `json:"target_state"` - CombinedState *CombinedState `json:"combined_state"` - Messages []MigrationMessage `json:"messages"` + Migration *MigrationSummary `json:"migration"` + TargetState *TargetState `json:"target_state"` + CombinedState *CombinedState `json:"combined_state"` + Messages []MigrationMessage `json:"messages"` } // MigrationSummary is the core migration record. type MigrationSummary struct { - MigrationID string `json:"migration_id"` - Status *string `json:"status"` - SourceOrganizationLogin string `json:"source_organization_login"` - TargetOrganizationLogin string `json:"target_organization_login"` - SourceRepositoryName string `json:"source_repository_name"` - TargetRepositoryName string `json:"target_repository_name"` - TargetVisibility *string `json:"target_visibility"` - TargetMigrationID int64 `json:"target_migration_id"` - CreatedAt *string `json:"created_at"` - StartedAt *string `json:"started_at"` - CompletedAt *string `json:"completed_at"` - ExpiresAt *string `json:"expires_at"` + MigrationID string `json:"migration_id"` + Status *string `json:"status"` + SourceOrganizationLogin string `json:"source_organization_login"` + TargetOrganizationLogin string `json:"target_organization_login"` + SourceRepositoryName string `json:"source_repository_name"` + SourceRepositoryArchived *bool `json:"source_repository_archived"` + TargetRepositoryName string `json:"target_repository_name"` + TargetVisibility *string `json:"target_visibility"` + TargetMigrationID int64 `json:"target_migration_id"` + CreatedAt *string `json:"created_at"` + StartedAt *string `json:"started_at"` + CompletedAt *string `json:"completed_at"` + ExpiresAt *string `json:"expires_at"` } // TargetState carries destination-side aggregate progress. diff --git a/internal/elmapi/migrations_test.go b/internal/elmapi/migrations_test.go index 594afea..dddfadf 100644 --- a/internal/elmapi/migrations_test.go +++ b/internal/elmapi/migrations_test.go @@ -17,12 +17,12 @@ func TestGetMigrationDetail(t *testing.T) { body string want *bool }{ - {"true", `{"source_repository_archived":true}`, new(true)}, - {"false", `{"source_repository_archived":false}`, new(false)}, - {"null", `{"source_repository_archived":null}`, nil}, - {"absent", `{}`, nil}, - {"null document", `null`, nil}, - {"completed with disagreeing legacy progress", `{"source_repository_archived":false,"migration":{"status":"completed"},"target_state":{"repository_progress":[{"repository_locked":true}]}}`, new(false)}, + {"true", `{"migration":{"source_repository_archived":true}}`, new(true)}, + {"false", `{"migration":{"source_repository_archived":false}}`, new(false)}, + {"null", `{"migration":{"source_repository_archived":null}}`, nil}, + {"absent", `{"migration":{}}`, nil}, + {"completed with disagreeing legacy progress", `{"migration":{"status":"completed","source_repository_archived":false},"target_state":{"repository_progress":[{"repository_locked":true}]}}`, new(false)}, + {"ignores top-level observation", `{"migration":{},"source_repository_archived":true}`, nil}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { @@ -35,7 +35,23 @@ func TestGetMigrationDetail(t *testing.T) { detail, err := NewClient(srv.URL, "tok").GetMigrationDetail(t.Context(), "mig-1") require.NoError(t, err) - assert.Equal(t, tc.want, detail.SourceRepositoryArchived) + require.NotNil(t, detail.Migration) + assert.Equal(t, tc.want, detail.Migration.SourceRepositoryArchived) + }) + } + }) + + t.Run("does not synthesize missing migration metadata", func(t *testing.T) { + for _, body := range []string{`{}`, `{"migration":null}`, `null`, `{"source_repository_archived":true}`} { + t.Run(body, func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(body)) + })) + t.Cleanup(srv.Close) + + detail, err := NewClient(srv.URL, "tok").GetMigrationDetail(t.Context(), "mig-1") + require.NoError(t, err) + assert.Nil(t, detail.Migration) }) } }) @@ -44,14 +60,14 @@ func TestGetMigrationDetail(t *testing.T) { for _, value := range []string{`"true"`, `1`, `{}`, `[]`} { t.Run(value, func(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - _, _ = w.Write([]byte(`{"source_repository_archived":` + value + `}`)) + _, _ = w.Write([]byte(`{"migration":{"source_repository_archived":` + value + `}}`)) })) t.Cleanup(srv.Close) detail, err := NewClient(srv.URL, "tok").GetMigrationDetail(t.Context(), "mig-1") var typeError *json.UnmarshalTypeError require.ErrorAs(t, err, &typeError) - assert.Equal(t, "source_repository_archived", typeError.Field) + assert.Equal(t, "migration.source_repository_archived", typeError.Field) assert.Nil(t, detail) }) } diff --git a/internal/render/migration.go b/internal/render/migration.go index a519b1d..d2244ed 100644 --- a/internal/render/migration.go +++ b/internal/render/migration.go @@ -29,13 +29,13 @@ func MigrationCancel(migrationID string) string { // MigrationStatus renders a migration status response. func MigrationStatus(v elmapi.MigrationDetail) string { - if v.Migration == nil && v.SourceRepositoryArchived == nil && v.TargetState == nil && v.CombinedState == nil && len(v.Messages) == 0 { + if v.Migration == nil && v.TargetState == nil && v.CombinedState == nil && len(v.Messages) == 0 { return "No migration status data returned.\n" } return joinSections( renderMigrationSummary(v.Migration), - renderSection("Source", " "+SourceRepositoryArchiveState(v.SourceRepositoryArchived)), + renderSection("Source", " "+SourceRepositoryArchiveState(v.Migration)), renderTargetState(v.TargetState), renderCombinedState(v.CombinedState), renderMessages(v.Messages), @@ -43,12 +43,12 @@ func MigrationStatus(v elmapi.MigrationDetail) string { } // SourceRepositoryArchiveState renders a nullable source observation, not migration progress. -func SourceRepositoryArchiveState(archived *bool) string { +func SourceRepositoryArchiveState(migration *elmapi.MigrationSummary) string { styles := theme.New() - if archived == nil { + if migration == nil || migration.SourceRepositoryArchived == nil { return styles.Muted.Render("Source repository archive state unavailable") } - if *archived { + if *migration.SourceRepositoryArchived { return styles.Primary.Render("Source repository archived") } return styles.Primary.Render("Source repository not archived") diff --git a/internal/render/migration_test.go b/internal/render/migration_test.go index 7160b05..27641bb 100644 --- a/internal/render/migration_test.go +++ b/internal/render/migration_test.go @@ -89,8 +89,8 @@ func TestMigrationStatus(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { output := MigrationStatus(elmapi.MigrationDetail{ - SourceRepositoryArchived: tc.archived, - CombinedState: &elmapi.CombinedState{Status: new("completed")}, + Migration: &elmapi.MigrationSummary{SourceRepositoryArchived: tc.archived}, + CombinedState: &elmapi.CombinedState{Status: new("completed")}, TargetState: &elmapi.TargetState{RepositoryProgress: []elmapi.RepositoryProgress{ {RepositoryNWO: "target/one", RepositoryLocked: tc.locked}, {RepositoryNWO: "target/two", RepositoryLocked: tc.locked}, @@ -108,13 +108,15 @@ func TestMigrationStatus(t *testing.T) { }) t.Run("renders source-only true", func(t *testing.T) { - assert.Equal(t, "Source\n Source repository archived\n", - MigrationStatus(elmapi.MigrationDetail{SourceRepositoryArchived: new(true)})) + assert.Contains(t, MigrationStatus(elmapi.MigrationDetail{ + Migration: &elmapi.MigrationSummary{SourceRepositoryArchived: new(true)}, + }), "Source\n Source repository archived\n") }) t.Run("renders source-only false", func(t *testing.T) { - assert.Equal(t, "Source\n Source repository not archived\n", - MigrationStatus(elmapi.MigrationDetail{SourceRepositoryArchived: new(false)})) + assert.Contains(t, MigrationStatus(elmapi.MigrationDetail{ + Migration: &elmapi.MigrationSummary{SourceRepositoryArchived: new(false)}, + }), "Source\n Source repository not archived\n") }) t.Run("renders nested status sections", func(t *testing.T) { @@ -241,12 +243,18 @@ func TestSourceRepositoryArchiveState(t *testing.T) { styles := theme.New() t.Run("true is a neutral fact", func(t *testing.T) { - assert.Equal(t, styles.Primary.Render("Source repository archived"), SourceRepositoryArchiveState(new(true))) + assert.Equal(t, styles.Primary.Render("Source repository archived"), + SourceRepositoryArchiveState(&elmapi.MigrationSummary{SourceRepositoryArchived: new(true)})) }) t.Run("false is a neutral fact", func(t *testing.T) { - assert.Equal(t, styles.Primary.Render("Source repository not archived"), SourceRepositoryArchiveState(new(false))) + assert.Equal(t, styles.Primary.Render("Source repository not archived"), + SourceRepositoryArchiveState(&elmapi.MigrationSummary{SourceRepositoryArchived: new(false)})) }) - t.Run("nil is explicitly unavailable", func(t *testing.T) { + t.Run("nil observation is explicitly unavailable", func(t *testing.T) { + assert.Equal(t, styles.Muted.Render("Source repository archive state unavailable"), + SourceRepositoryArchiveState(&elmapi.MigrationSummary{})) + }) + t.Run("nil migration is explicitly unavailable", func(t *testing.T) { assert.Equal(t, styles.Muted.Render("Source repository archive state unavailable"), SourceRepositoryArchiveState(nil)) }) } diff --git a/internal/tui/model_test.go b/internal/tui/model_test.go index 4eeea7a..d03c782 100644 --- a/internal/tui/model_test.go +++ b/internal/tui/model_test.go @@ -340,15 +340,18 @@ func TestModel(t *testing.T) { model.sourceWatching = true cases := []struct { body string - want *bool text string }{ - {`{"source_repository_archived":true}`, new(true), "Source repository archived"}, - {`{"source_repository_archived":false}`, new(false), "Source repository not archived"}, - {`{"source_repository_archived":true}`, new(true), "Source repository archived"}, - {`{"migration":{},"source_repository_archived":null}`, nil, "Source repository archive state unavailable"}, - {`{"source_repository_archived":true}`, new(true), "Source repository archived"}, - {`{"migration":{}}`, nil, "Source repository archive state unavailable"}, + {`{"migration":{"source_repository_archived":true}}`, "Source repository archived"}, + {`{"migration":{"source_repository_archived":false}}`, "Source repository not archived"}, + {`{"migration":{"source_repository_archived":true}}`, "Source repository archived"}, + {`{"migration":{"source_repository_archived":null}}`, "Source repository archive state unavailable"}, + {`{"migration":{"source_repository_archived":true}}`, "Source repository archived"}, + {`{"migration":{}}`, "Source repository archive state unavailable"}, + {`{"migration":{"source_repository_archived":true}}`, "Source repository archived"}, + {`{"migration":null,"combined_state":{"status":"completed"}}`, "Source repository archive state unavailable"}, + {`{"migration":{"source_repository_archived":true}}`, "Source repository archived"}, + {`{"combined_state":{"status":"completed"}}`, "Source repository archive state unavailable"}, } for _, tc := range cases { var detail elmapi.MigrationDetail @@ -358,12 +361,11 @@ func TestModel(t *testing.T) { require.NoError(t, model.err) assert.NotNil(t, cmd) assert.Same(t, &detail, model.sourceDetail) - assert.Equal(t, tc.want, model.sourceDetail.SourceRepositoryArchived) assert.Contains(t, model.View(), tc.text) assert.Equal(t, 1, strings.Count(model.View(), "Source repository")) } - previous := &elmapi.MigrationDetail{SourceRepositoryArchived: new(true)} + previous := &elmapi.MigrationDetail{Migration: &elmapi.MigrationSummary{SourceRepositoryArchived: new(true)}} _, _ = model.Update(sourceDetailMsg{detail: previous}) _, cmd := model.Update(sourceDetailMsg{err: assert.AnError}) assert.ErrorIs(t, model.err, assert.AnError) @@ -378,8 +380,8 @@ func TestModel(t *testing.T) { model.screen = screenSourceDetail model.width, model.height = 100, 60 model.sourceDetail = &elmapi.MigrationDetail{ - SourceRepositoryArchived: new(false), - CombinedState: &elmapi.CombinedState{Status: new("completed")}, + Migration: &elmapi.MigrationSummary{SourceRepositoryArchived: new(false)}, + CombinedState: &elmapi.CombinedState{Status: new("completed")}, TargetState: &elmapi.TargetState{RepositoryProgress: []elmapi.RepositoryProgress{ {RepositoryNWO: "target/one", RepositoryLocked: true}, {RepositoryNWO: "target/two", RepositoryLocked: true}, From 273f0349d34b7b79505822fa31b2a3826ae823f9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 18 Sep 2026 13:48:30 +0000 Subject: [PATCH 3/3] Fix lint workflow and address golangci-lint findings Co-authored-by: jeffsaracco <248182+jeffsaracco@users.noreply.github.com> --- .github/workflows/lint.yaml | 8 +---- internal/cmd/migration/watch/watch_test.go | 2 +- internal/render/migration_test.go | 36 +++++++++++----------- internal/tui/model_test.go | 2 +- 4 files changed, 21 insertions(+), 27 deletions(-) diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml index 287e05f..5d00f5f 100644 --- a/.github/workflows/lint.yaml +++ b/.github/workflows/lint.yaml @@ -10,7 +10,6 @@ on: merge_group: permissions: - id-token: write contents: read jobs: @@ -26,12 +25,7 @@ jobs: - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6.5.0 with: go-version-file: go.mod - # setup-go caches by default; disable it here because this workflow - # also runs on tags and has id-token: write. - cache: false - - - name: OIDC Setup for goproxy - uses: github/setup-goproxy@5e60e1074d42316dfe2949ebf9a92bf77b24645b # v1.1.0 + cache: true - name: Run Go linter run: make lint diff --git a/internal/cmd/migration/watch/watch_test.go b/internal/cmd/migration/watch/watch_test.go index 9fed49c..9de1c64 100644 --- a/internal/cmd/migration/watch/watch_test.go +++ b/internal/cmd/migration/watch/watch_test.go @@ -252,7 +252,7 @@ func TestUpdate(t *testing.T) { responses <- response{body: `{}`, status: http.StatusOK} updated, _ = m.Update(fetchStatus(m.client, m.migrationID, m.interval)) m = updated.(Model) - assert.NoError(t, m.fetchErr) + require.NoError(t, m.fetchErr) assert.Nil(t, m.detail.Migration) assert.NotContains(t, m.View(), "Failed to refresh") }) diff --git a/internal/render/migration_test.go b/internal/render/migration_test.go index 27641bb..ea9fcbd 100644 --- a/internal/render/migration_test.go +++ b/internal/render/migration_test.go @@ -110,13 +110,13 @@ func TestMigrationStatus(t *testing.T) { t.Run("renders source-only true", func(t *testing.T) { assert.Contains(t, MigrationStatus(elmapi.MigrationDetail{ Migration: &elmapi.MigrationSummary{SourceRepositoryArchived: new(true)}, - }), "Source\n Source repository archived\n") + }), "Source\n Source repository archived\n") //nolint:dupword // header label followed by output line, not a real repeated word }) t.Run("renders source-only false", func(t *testing.T) { assert.Contains(t, MigrationStatus(elmapi.MigrationDetail{ Migration: &elmapi.MigrationSummary{SourceRepositoryArchived: new(false)}, - }), "Source\n Source repository not archived\n") + }), "Source\n Source repository not archived\n") //nolint:dupword // header label followed by output line, not a real repeated word }) t.Run("renders nested status sections", func(t *testing.T) { @@ -181,15 +181,15 @@ func TestMigrationStatus(t *testing.T) { }, }) - assert.Equal(t, `Source - Source repository archive state unavailable - -Cutover - ✓ Ready for cutover - -Repository states - • elm-test/the-hook2 · Ready for cutover -`, output) + want := "Source\n" + + " Source repository archive state unavailable\n" + + "\n" + + "Cutover\n" + + " ✓ Ready for cutover\n" + + "\n" + + "Repository states\n" + + " • elm-test/the-hook2 · Ready for cutover\n" + assert.Equal(t, want, output) }) t.Run("suppresses completed-state readiness and stale blockers", func(t *testing.T) { @@ -204,13 +204,13 @@ Repository states }, }) - assert.Equal(t, `Source - Source repository archive state unavailable - -Cutover - ✓ Completed - Migration completed successfully -`, output) + want := "Source\n" + + " Source repository archive state unavailable\n" + + "\n" + + "Cutover\n" + + " ✓ Completed\n" + + " Migration completed successfully\n" + assert.Equal(t, want, output) }) t.Run("preserves distinct repository phase and status", func(t *testing.T) { diff --git a/internal/tui/model_test.go b/internal/tui/model_test.go index d03c782..ea46617 100644 --- a/internal/tui/model_test.go +++ b/internal/tui/model_test.go @@ -368,7 +368,7 @@ func TestModel(t *testing.T) { previous := &elmapi.MigrationDetail{Migration: &elmapi.MigrationSummary{SourceRepositoryArchived: new(true)}} _, _ = model.Update(sourceDetailMsg{detail: previous}) _, cmd := model.Update(sourceDetailMsg{err: assert.AnError}) - assert.ErrorIs(t, model.err, assert.AnError) + require.ErrorIs(t, model.err, assert.AnError) assert.Same(t, previous, model.sourceDetail) assert.NotNil(t, cmd) assert.Contains(t, model.View(), "Source repository archived")