diff --git a/cmd/kitchen-sink/main.go b/cmd/kitchen-sink/main.go index 25d9b22..13b8205 100644 --- a/cmd/kitchen-sink/main.go +++ b/cmd/kitchen-sink/main.go @@ -98,6 +98,121 @@ var fixtures = []struct { "created_at": "2026-08-06T09:42:00Z" } ] +}`), + render: renderStatus, + }, + { + // A failed migration, carrying the terminal failure the server records. + // display_message deliberately repeats the authored summary, which is + // what the server sends on a failure, so this fixture also previews the + // de-duplication between the failure section and the cutover section. + name: "gh elm migration status (failed)", + raw: json.RawMessage(`{ + "migration": { + "migration_id": "b41d0c6e-9d2a-4f70-8c1b-5a6f2f0f4e18", + "status": "failed", + "source_organization_login": "source-org", + "source_repository_name": "billing", + "target_organization_login": "target-org", + "target_repository_name": "billing", + "target_visibility": "internal", + "target_migration_id": 4307, + "created_at": "2026-09-04T12:55:00Z", + "started_at": "2026-09-04T12:56:10Z", + "completed_at": null, + "expires_at": "2026-09-11T12:55:00Z" + }, + "target_state": { + "status": "failed", + "target_unavailable": false, + "repository_progress": [ + { + "repository_nwo": "target-org/billing", + "backfill_resources_added": 24, + "backfill_resources_processed": 22, + "backfill_resources_failed": 2, + "live_update_resources_added": 0, + "live_update_resources_processed": 0, + "live_update_resources_failed": 0, + "all_resources_sent": false, + "initial_git_push_complete": false, + "repository_locked": false + } + ], + "terminal_failure": { + "code": "repository_policy", + "summary": "Creating the target repository was blocked by a policy on the target organization or enterprise.", + "occurred_at": "2026-09-04T12:58:37Z" + } + }, + "combined_state": { + "status": "failed", + "display_message": "Creating the target repository was blocked by a policy on the target organization or enterprise.", + "ready_for_cutover": false, + "cutover_blockers": [], + "repositories": [ + { + "repository_nwo": "target-org/billing", + "phase": "backfill", + "display_status": "Failed: 2 resources failed" + } + ], + "terminal_failure": { + "code": "repository_policy", + "summary": "Creating the target repository was blocked by a policy on the target organization or enterprise.", + "occurred_at": "2026-09-04T12:58:37Z" + } + }, + "messages": [ + { + "message_type": "error", + "message": "Two backfill resources failed and should be reviewed.", + "created_at": "2026-09-04T12:58:40Z" + } + ] +}`), + render: renderStatus, + }, + { + // A terminated migration whose target independently reported a fault. + // The operator aborted this deliberately, so the combined state carries + // no cause and the target's must NOT be attributed: this fixture should + // render no Failure section at all. + name: "gh elm migration status (terminated, target reported a fault)", + raw: json.RawMessage(`{ + "migration": { + "migration_id": "c52e1d7f-ae3b-4081-9d2c-6b7a3a1b5f29", + "status": "terminated", + "source_organization_login": "source-org", + "source_repository_name": "payments", + "target_organization_login": "target-org", + "target_repository_name": "payments", + "target_visibility": "internal", + "target_migration_id": 4311, + "created_at": "2026-09-04T13:10:00Z", + "started_at": "2026-09-04T13:11:05Z", + "completed_at": null, + "expires_at": "2026-09-11T13:10:00Z" + }, + "target_state": { + "status": "failed", + "target_unavailable": false, + "repository_progress": [], + "terminal_failure": { + "code": "critical_resource", + "summary": "A critical resource could not be migrated.", + "occurred_at": "2026-09-04T13:12:44Z" + } + }, + "combined_state": { + "status": "terminated", + "display_message": "Migration terminated by request.", + "ready_for_cutover": false, + "cutover_blockers": [], + "repositories": [], + "terminal_failure": null + }, + "messages": [] }`), render: renderStatus, }, diff --git a/internal/cmd/migration/watch/view.go b/internal/cmd/migration/watch/view.go index 8aec740..1c8ef8e 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" ) @@ -341,7 +342,19 @@ func (m Model) completedDetail() string { return msg } +// failedDetail describes why the migration failed, preferring the cause the +// server recorded over the display message derived from it. +// +// The summary and code come from the shared render helpers so this line cannot +// drift from what `gh elm migration status` prints for the same migration. func (m Model) failedDetail() string { + if failure := render.TerminalFailureFor(*m.detail); failure != nil { + text := m.styles.Failure.Render(render.TerminalFailureSummary(failure)) + if code := strings.TrimSpace(failure.Code); code != "" { + text += m.styles.Muted.Render(" (" + code + ")") + } + return text + } if cs := m.detail.CombinedState; cs != nil && cs.DisplayMessage != "" { return m.styles.Failure.Render(cs.DisplayMessage) } diff --git a/internal/cmd/migration/watch/watch_test.go b/internal/cmd/migration/watch/watch_test.go index 709e148..4ed25fa 100644 --- a/internal/cmd/migration/watch/watch_test.go +++ b/internal/cmd/migration/watch/watch_test.go @@ -139,3 +139,60 @@ func TestView(t *testing.T) { assert.Contains(t, m.View(), "Loading migration status") }) } + +func TestFailedDetail(t *testing.T) { + failed := combinedFailed + + newModel := func(detail *elmapi.MigrationDetail) Model { + m := New("id", time.Second, nil) + m.detail = detail + return m + } + + t.Run("prefers the recorded cause over the display message", func(t *testing.T) { + m := newModel(&elmapi.MigrationDetail{ + CombinedState: &elmapi.CombinedState{ + Status: &failed, + DisplayMessage: "Failed: 2 resources failed", + TerminalFailure: &elmapi.TerminalFailure{ + Code: "repository_policy", + Summary: "Policy blocked it.", + }, + }, + }) + + detail := m.failedDetail() + + assert.Contains(t, detail, "Policy blocked it.") + assert.Contains(t, detail, "repository_policy") + assert.NotContains(t, detail, "Failed: 2 resources failed") + }) + + t.Run("falls back to the target state cause", func(t *testing.T) { + m := newModel(&elmapi.MigrationDetail{ + CombinedState: &elmapi.CombinedState{Status: &failed}, + TargetState: &elmapi.TargetState{ + TerminalFailure: &elmapi.TerminalFailure{ + Code: "critical_resource", + Summary: "A required resource could not be imported.", + }, + }, + }) + + assert.Contains(t, m.failedDetail(), "A required resource could not be imported.") + }) + + t.Run("falls back to the display message when no cause was recorded", func(t *testing.T) { + m := newModel(&elmapi.MigrationDetail{ + CombinedState: &elmapi.CombinedState{Status: &failed, DisplayMessage: "Failed: 2 resources failed"}, + }) + + assert.Contains(t, m.failedDetail(), "Failed: 2 resources failed") + }) + + t.Run("falls back to a generic sentence with nothing to show", func(t *testing.T) { + m := newModel(&elmapi.MigrationDetail{CombinedState: &elmapi.CombinedState{Status: &failed}}) + + assert.Contains(t, m.failedDetail(), "Migration failed") + }) +} diff --git a/internal/elmapi/migrations.go b/internal/elmapi/migrations.go index 51db8f3..447384c 100644 --- a/internal/elmapi/migrations.go +++ b/internal/elmapi/migrations.go @@ -230,6 +230,33 @@ type TargetState struct { Status *string `json:"status"` TargetUnavailable bool `json:"target_unavailable"` RepositoryProgress []RepositoryProgress `json:"repository_progress"` + // TerminalFailure is the destination's own report of why it failed, passed + // through faithfully. It is nil when the target is unavailable or has not + // failed, and may be set for states that CombinedState does not describe as + // a failure. + TerminalFailure *TerminalFailure `json:"terminal_failure"` +} + +// TerminalFailure is the recorded cause of a migration reaching a terminal +// state. The server records exactly one per migration and never overwrites it, +// so this is the root cause rather than a later consequence. +// +// The whole object is nil on a migration that has not failed, and on any GHES +// released before the field existed. +type TerminalFailure struct { + // Code categorizes the failure and is a stable contract: branch on it + // rather than on Summary. Callers must tolerate codes they do not + // recognize, because the server adds new ones independently of this client. + Code string `json:"code"` + // Summary is the customer-facing sentence describing what happened. The + // server authors it from a fixed table keyed by Code and never derives it + // from upstream error text, so it carries no customer or target data and is + // safe to render verbatim. + Summary string `json:"summary"` + // OccurredAt is an RFC 3339 timestamp, nil when the server recorded no + // time. The server deliberately sends null rather than the Unix epoch so + // that "no time recorded" stays distinguishable from "failed in 1970". + OccurredAt *string `json:"occurred_at"` } // RepositoryProgress is per-repository backfill/live-update counts. @@ -253,6 +280,11 @@ type CombinedState struct { ReadyForCutover bool `json:"ready_for_cutover"` CutoverBlockers []string `json:"cutover_blockers"` Repositories []CombinedRepositoryState `json:"repositories"` + // TerminalFailure is the user-facing cause of the failure. The server sets + // it only when Status is failed, so a terminated migration — a user- + // initiated abort rather than a fault — carries no cause here even though + // TargetState may still report one. + TerminalFailure *TerminalFailure `json:"terminal_failure"` } // CombinedRepositoryState is per-repository derived phase/status. diff --git a/internal/elmapi/migrations_test.go b/internal/elmapi/migrations_test.go index d395625..030094c 100644 --- a/internal/elmapi/migrations_test.go +++ b/internal/elmapi/migrations_test.go @@ -57,3 +57,76 @@ func TestMigrationResponses(t *testing.T) { assert.Equal(t, body, string(resp.Raw)) }) } + +func TestMigrationDetailTerminalFailure(t *testing.T) { + const body = `{ + "migration": {"migration_id": "mig-1", "status": "failed"}, + "target_state": { + "status": "failed", + "terminal_failure": { + "code": "repository_policy", + "summary": "Policy blocked it.", + "occurred_at": "2026-09-04T12:58:37Z" + } + }, + "combined_state": { + "status": "failed", + "terminal_failure": {"code": "critical_resource", "summary": "Resource failed.", "occurred_at": null} + } +}` + + newServer := func(t *testing.T, payload string) *httptest.Server { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(payload)) + })) + t.Cleanup(srv.Close) + return srv + } + + t.Run("decodes the cause on both states", func(t *testing.T) { + srv := newServer(t, body) + + detail, err := NewClient(srv.URL, "tok").GetMigrationDetail(t.Context(), "mig-1") + require.NoError(t, err) + + require.NotNil(t, detail.TargetState.TerminalFailure) + assert.Equal(t, "repository_policy", detail.TargetState.TerminalFailure.Code) + assert.Equal(t, "Policy blocked it.", detail.TargetState.TerminalFailure.Summary) + require.NotNil(t, detail.TargetState.TerminalFailure.OccurredAt) + assert.Equal(t, "2026-09-04T12:58:37Z", *detail.TargetState.TerminalFailure.OccurredAt) + + require.NotNil(t, detail.CombinedState.TerminalFailure) + assert.Equal(t, "critical_resource", detail.CombinedState.TerminalFailure.Code) + }) + + t.Run("leaves a null occurred_at nil so it stays distinct from the epoch", func(t *testing.T) { + srv := newServer(t, body) + + detail, err := NewClient(srv.URL, "tok").GetMigrationDetail(t.Context(), "mig-1") + require.NoError(t, err) + + require.NotNil(t, detail.CombinedState.TerminalFailure) + assert.Nil(t, detail.CombinedState.TerminalFailure.OccurredAt) + }) + + t.Run("leaves the cause nil when the server omits it", func(t *testing.T) { + srv := newServer(t, `{"migration":{"migration_id":"mig-1"},"target_state":{},"combined_state":{}}`) + + detail, err := NewClient(srv.URL, "tok").GetMigrationDetail(t.Context(), "mig-1") + require.NoError(t, err) + + assert.Nil(t, detail.TargetState.TerminalFailure) + assert.Nil(t, detail.CombinedState.TerminalFailure) + }) + + t.Run("preserves the cause verbatim in the raw document", func(t *testing.T) { + srv := newServer(t, body) + + raw, err := NewClient(srv.URL, "tok").GetMigration(t.Context(), "mig-1") + require.NoError(t, err) + + // `--json` writes this document straight through, so the contract + // reaches scripts without the typed structs having to model it. + assert.JSONEq(t, body, string(raw)) + }) +} diff --git a/internal/render/migration.go b/internal/render/migration.go index f974113..63b439d 100644 --- a/internal/render/migration.go +++ b/internal/render/migration.go @@ -33,10 +33,15 @@ func MigrationStatus(v elmapi.MigrationDetail) string { return "No migration status data returned.\n" } + failure := TerminalFailureFor(v) return joinSections( renderMigrationSummary(v.Migration), + // The cause sits directly below the summary: on a failed migration it + // is the one thing the operator needs, and burying it under progress + // bars is how it gets missed. + renderTerminalFailure(failure), renderTargetState(v.TargetState), - renderCombinedState(v.CombinedState), + renderCombinedState(v.CombinedState, failure), renderMessages(v.Messages), ) } @@ -46,7 +51,15 @@ func CutoverStatus(v elmapi.MigrationDetail) string { if v.CombinedState == nil { return "No combined state reported for this migration yet.\n" } - return renderCombinedState(v.CombinedState) + // The failure section must render here too, not just in MigrationStatus: + // renderCombinedState suppresses a display_message that repeats the + // summary, so without this the cause would be silently dropped from the + // cutover view rather than de-duplicated. + failure := TerminalFailureFor(v) + return joinSections( + renderTerminalFailure(failure), + renderCombinedState(v.CombinedState, failure), + ) } func renderMigrationSummary(migration *elmapi.MigrationSummary) string { @@ -116,7 +129,7 @@ func renderRepositoryProgress(progress elmapi.RepositoryProgress) string { return renderSection("Progress · "+valueOrEmpty(progress.RepositoryNWO), lines...) } -func renderCombinedState(combined *elmapi.CombinedState) string { +func renderCombinedState(combined *elmapi.CombinedState, failure *elmapi.TerminalFailure) string { if combined == nil { return "" } @@ -128,6 +141,12 @@ func renderCombinedState(combined *elmapi.CombinedState) string { bullet(statusGlyph(status), statusText(status)), } renderedValues := []string{status} + // The server prefers the authored failure summary for display_message on a + // failed migration, so without seeding it here the same sentence would + // render twice: once in the failure section, once again below. + if summary := TerminalFailureSummary(failure); summary != "" { + renderedValues = append(renderedValues, summary) + } completed := completedStatus(status) readinessText := "Not ready for cutover" if combined.ReadyForCutover { diff --git a/internal/render/terminal_failure.go b/internal/render/terminal_failure.go new file mode 100644 index 0000000..3942229 --- /dev/null +++ b/internal/render/terminal_failure.go @@ -0,0 +1,150 @@ +package render + +import ( + "fmt" + "strings" + "time" + + "github.com/github/gh-elm/internal/elmapi" + "github.com/github/gh-elm/internal/theme" +) + +// nowFunc is the clock used to render relative failure times. It is a variable +// so tests can pin it; widening the exported renderers to accept a clock would +// churn every call site for one line of output. +var nowFunc = time.Now + +// genericFailureSummary is shown when the server recorded a failure but no +// usable text, so a failed migration never renders as though nothing happened. +const genericFailureSummary = "Migration failed" + +// TerminalFailureFor returns the cause to display for a migration, or nil when +// none was recorded or none should be attributed. +// +// CombinedState wins because the server populates it only for a genuine +// failure, making it the user-facing view. +// +// TargetState is a narrower fallback. It is the destination's faithful report +// and can be set while the combined status describes something that is not a +// failure at all — a user-initiated abort, or a migration still running — so it +// is used only when the combined state is absent, undecided, or itself failed. +// Falling back unconditionally would render a cause for a migration the server +// does not consider failed, misattributing a deliberate abort as a fault. +func TerminalFailureFor(detail elmapi.MigrationDetail) *elmapi.TerminalFailure { + if combined := detail.CombinedState; combined != nil && combined.TerminalFailure != nil { + return combined.TerminalFailure + } + if target := detail.TargetState; target != nil && target.TerminalFailure != nil { + if detail.CombinedState == nil || allowsTargetFailure(pointerString(detail.CombinedState.Status)) { + return target.TerminalFailure + } + } + return nil +} + +// allowsTargetFailure reports whether a combined status permits attributing a +// target-reported cause to the migration. +// +// This deliberately does not reuse statusGlyph/statusText, which lump failed in +// with terminated and cancelled for presentation purposes. That conflation is +// exactly the distinction this gate turns on: a terminated migration is a +// deliberate abort, not a fault. +func allowsTargetFailure(status string) bool { + switch normalizedValue(status) { + // Empty and unknown mean the combined view has not reached a verdict, so + // the target's report is the best information available; failed means it + // agrees, and simply did not author its own cause. + case "", "unknown", "unspecified", "failed", "failure": + return true + default: + // Terminated, cancelled, completed, and anything still in flight: the + // server does not describe this migration as failed, so neither do we. + return false + } +} + +// TerminalFailureSummary returns the sentence describing a failure, falling +// back to the code and then to a generic sentence. +// +// The summary is never derived from the code beyond that fallback: the server +// authors the text, so deriving it here would silently go stale as new codes +// are added. +func TerminalFailureSummary(failure *elmapi.TerminalFailure) string { + if failure == nil { + return "" + } + if summary := strings.TrimSpace(failure.Summary); summary != "" { + return summary + } + if code := strings.TrimSpace(failure.Code); code != "" { + return friendlyValue(code) + } + return genericFailureSummary +} + +// renderTerminalFailure renders the failure section for a migration status +// document, or "" when no cause was recorded. +func renderTerminalFailure(failure *elmapi.TerminalFailure) string { + if failure == nil { + return "" + } + + styles := theme.New() + lines := []string{ + bullet(styles.Failure.Render("✗"), styles.Failure.Bold(true).Render(TerminalFailureSummary(failure))), + } + // The code is rendered verbatim rather than prettified: it is a stable + // contract value, so keeping it greppable and quotable in a support + // escalation is worth more than title casing. + if code := strings.TrimSpace(failure.Code); code != "" { + lines = append(lines, field("Code", styles.Bold.Render(code))) + } + if occurred := formatOccurredAt(failure.OccurredAt); occurred != "" { + lines = append(lines, field("Occurred", occurred)) + } + return renderSection("Failure", lines...) +} + +// formatOccurredAt renders a failure timestamp as " ()", +// or "" when no time was recorded. +// +// An absent timestamp yields no line at all rather than an em dash, which would +// imply the failure happened at an unknown time instead of simply not having +// been stamped. +func formatOccurredAt(occurredAt *string) string { + if occurredAt == nil || strings.TrimSpace(*occurredAt) == "" { + return "" + } + + absolute := strings.TrimSpace(*occurredAt) + parsed, err := time.Parse(time.RFC3339, absolute) + if err != nil { + // An unparseable timestamp is still information; show it as sent + // rather than dropping the line. + return absolute + } + + styles := theme.New() + return relativeTime(parsed, nowFunc()) + styles.Muted.Render(" ("+absolute+")") +} + +// relativeTime renders how long before now a moment was, in the same units as +// the watch timeline. +func relativeTime(moment, now time.Time) string { + elapsed := now.Sub(moment) + if elapsed < 0 { + // Clock skew between the appliance and this machine; "just now" beats + // rendering a negative age. + return "just now" + } + switch { + case elapsed < time.Minute: + return fmt.Sprintf("%ds ago", int(elapsed.Seconds())) + case elapsed < time.Hour: + return fmt.Sprintf("%dm ago", int(elapsed.Minutes())) + case elapsed < 24*time.Hour: + return fmt.Sprintf("%dh %dm ago", int(elapsed.Hours()), int(elapsed.Minutes())%60) + default: + return fmt.Sprintf("%dd ago", int(elapsed.Hours())/24) + } +} diff --git a/internal/render/terminal_failure_test.go b/internal/render/terminal_failure_test.go new file mode 100644 index 0000000..5977237 --- /dev/null +++ b/internal/render/terminal_failure_test.go @@ -0,0 +1,319 @@ +package render + +import ( + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/github/gh-elm/internal/elmapi" +) + +// pinNow freezes the clock used for relative failure times so assertions do not +// drift with wall time. +func pinNow(t *testing.T, now time.Time) { + previous := nowFunc + nowFunc = func() time.Time { return now } + t.Cleanup(func() { nowFunc = previous }) +} + +func failure(code, summary string, occurredAt *string) *elmapi.TerminalFailure { + return &elmapi.TerminalFailure{Code: code, Summary: summary, OccurredAt: occurredAt} +} + +func TestTerminalFailureFor(t *testing.T) { + t.Run("prefers the combined state cause", func(t *testing.T) { + detail := elmapi.MigrationDetail{ + TargetState: &elmapi.TargetState{TerminalFailure: failure("critical_resource", "Target view", nil)}, + CombinedState: &elmapi.CombinedState{TerminalFailure: failure("repository_policy", "Combined view", nil)}, + } + + resolved := TerminalFailureFor(detail) + + require.NotNil(t, resolved) + assert.Equal(t, "repository_policy", resolved.Code) + }) + + t.Run("falls back to the target state cause", func(t *testing.T) { + detail := elmapi.MigrationDetail{ + TargetState: &elmapi.TargetState{TerminalFailure: failure("critical_resource", "Target view", nil)}, + CombinedState: &elmapi.CombinedState{}, + } + + resolved := TerminalFailureFor(detail) + + require.NotNil(t, resolved) + assert.Equal(t, "critical_resource", resolved.Code) + }) + + t.Run("uses the target cause when combined state is absent", func(t *testing.T) { + resolved := TerminalFailureFor(elmapi.MigrationDetail{ + TargetState: &elmapi.TargetState{TerminalFailure: failure("critical_resource", "Target view", nil)}, + }) + + require.NotNil(t, resolved) + assert.Equal(t, "critical_resource", resolved.Code) + }) + + t.Run("uses the target cause when combined state failed without one", func(t *testing.T) { + failedStatus := "failed" + resolved := TerminalFailureFor(elmapi.MigrationDetail{ + TargetState: &elmapi.TargetState{TerminalFailure: failure("critical_resource", "Target view", nil)}, + CombinedState: &elmapi.CombinedState{Status: &failedStatus}, + }) + + require.NotNil(t, resolved) + assert.Equal(t, "critical_resource", resolved.Code) + }) + + // A terminated migration is a deliberate abort. The target may still report + // a fault, but attributing it would tell the operator their own cancellation + // was a failure. + t.Run("ignores the target cause when the migration was terminated", func(t *testing.T) { + terminated := "terminated" + assert.Nil(t, TerminalFailureFor(elmapi.MigrationDetail{ + TargetState: &elmapi.TargetState{TerminalFailure: failure("critical_resource", "Target view", nil)}, + CombinedState: &elmapi.CombinedState{Status: &terminated}, + })) + }) + + t.Run("ignores the target cause while the migration is still in flight", func(t *testing.T) { + for _, status := range []string{"in_progress", "paused"} { + t.Run(status, func(t *testing.T) { + assert.Nil(t, TerminalFailureFor(elmapi.MigrationDetail{ + TargetState: &elmapi.TargetState{TerminalFailure: failure("critical_resource", "Target view", nil)}, + CombinedState: &elmapi.CombinedState{Status: &status}, + })) + }) + } + }) + + t.Run("ignores the target cause when the migration completed", func(t *testing.T) { + completed := "completed" + assert.Nil(t, TerminalFailureFor(elmapi.MigrationDetail{ + TargetState: &elmapi.TargetState{TerminalFailure: failure("critical_resource", "Target view", nil)}, + CombinedState: &elmapi.CombinedState{Status: &completed}, + })) + }) + + // The combined cause is authored only for a genuine failure, so when it is + // present it is trusted regardless of how the status reads. + t.Run("keeps the combined cause even for a terminated status", func(t *testing.T) { + terminated := "terminated" + resolved := TerminalFailureFor(elmapi.MigrationDetail{ + CombinedState: &elmapi.CombinedState{Status: &terminated, TerminalFailure: failure("repository_policy", "Combined view", nil)}, + }) + + require.NotNil(t, resolved) + assert.Equal(t, "repository_policy", resolved.Code) + }) + + t.Run("returns nil when nothing failed", func(t *testing.T) { + assert.Nil(t, TerminalFailureFor(elmapi.MigrationDetail{ + TargetState: &elmapi.TargetState{}, + CombinedState: &elmapi.CombinedState{}, + })) + }) + + t.Run("returns nil for an empty document", func(t *testing.T) { + assert.Nil(t, TerminalFailureFor(elmapi.MigrationDetail{})) + }) +} + +func TestTerminalFailureSummary(t *testing.T) { + t.Run("uses the server summary", func(t *testing.T) { + assert.Equal(t, "Policy blocked it.", TerminalFailureSummary(failure("repository_policy", "Policy blocked it.", nil))) + }) + + t.Run("falls back to the code when no summary is sent", func(t *testing.T) { + assert.Equal(t, "Repository policy", TerminalFailureSummary(failure("repository_policy", "", nil))) + }) + + t.Run("falls back to a generic sentence when the failure is empty", func(t *testing.T) { + assert.Equal(t, "Migration failed", TerminalFailureSummary(failure("", " ", nil))) + }) + + t.Run("returns an empty string for no failure", func(t *testing.T) { + assert.Empty(t, TerminalFailureSummary(nil)) + }) +} + +func TestRenderTerminalFailure(t *testing.T) { + t.Run("renders the summary, code, and time", func(t *testing.T) { + pinNow(t, time.Date(2026, 9, 4, 13, 0, 37, 0, time.UTC)) + occurredAt := "2026-09-04T12:58:37Z" + + output := renderTerminalFailure(failure("repository_policy", "Policy blocked it.", &occurredAt)) + + assert.Contains(t, output, "Failure") + assert.Contains(t, output, "Policy blocked it.") + assert.Contains(t, output, "repository_policy") + assert.Contains(t, output, "2m ago (2026-09-04T12:58:37Z)") + }) + + t.Run("renders a code it does not recognize", func(t *testing.T) { + output := renderTerminalFailure(failure("some_future_code", "Something new went wrong.", nil)) + + assert.Contains(t, output, "Something new went wrong.") + assert.Contains(t, output, "some_future_code") + }) + + t.Run("omits the occurred line when no time was recorded", func(t *testing.T) { + output := renderTerminalFailure(failure("repository_policy", "Policy blocked it.", nil)) + + assert.Contains(t, output, "Policy blocked it.") + assert.NotContains(t, output, "Occurred") + }) + + t.Run("renders nothing when no failure was recorded", func(t *testing.T) { + assert.Empty(t, renderTerminalFailure(nil)) + }) +} + +func TestFormatOccurredAt(t *testing.T) { + t.Run("returns an unparseable timestamp unchanged", func(t *testing.T) { + malformed := "not-a-timestamp" + + assert.Equal(t, "not-a-timestamp", formatOccurredAt(&malformed)) + }) + + t.Run("returns an empty string for a nil timestamp", func(t *testing.T) { + assert.Empty(t, formatOccurredAt(nil)) + }) + + t.Run("returns an empty string for a blank timestamp", func(t *testing.T) { + blank := " " + + assert.Empty(t, formatOccurredAt(&blank)) + }) +} + +func TestRelativeTime(t *testing.T) { + now := time.Date(2026, 9, 4, 12, 0, 0, 0, time.UTC) + + t.Run("renders seconds", func(t *testing.T) { + assert.Equal(t, "30s ago", relativeTime(now.Add(-30*time.Second), now)) + }) + + t.Run("renders minutes", func(t *testing.T) { + assert.Equal(t, "5m ago", relativeTime(now.Add(-5*time.Minute), now)) + }) + + t.Run("renders hours and minutes", func(t *testing.T) { + assert.Equal(t, "3h 5m ago", relativeTime(now.Add(-(3*time.Hour+5*time.Minute)), now)) + }) + + t.Run("renders days", func(t *testing.T) { + assert.Equal(t, "2d ago", relativeTime(now.Add(-50*time.Hour), now)) + }) + + t.Run("renders a future timestamp as just now", func(t *testing.T) { + assert.Equal(t, "just now", relativeTime(now.Add(time.Minute), now)) + }) +} + +func TestMigrationStatusTerminalFailure(t *testing.T) { + failedStatus := "failed" + + t.Run("renders the failure section above target state", func(t *testing.T) { + pinNow(t, time.Date(2026, 9, 4, 13, 0, 37, 0, time.UTC)) + occurredAt := "2026-09-04T12:58:37Z" + output := MigrationStatus(elmapi.MigrationDetail{ + Migration: &elmapi.MigrationSummary{MigrationID: "mig-1", Status: &failedStatus}, + TargetState: &elmapi.TargetState{ + Status: &failedStatus, + TerminalFailure: failure("repository_policy", "Policy blocked it.", &occurredAt), + }, + }) + + require.Contains(t, output, "Failure") + assert.Less(t, strings.Index(output, "Failure"), strings.Index(output, "Target")) + assert.Contains(t, output, "Policy blocked it.") + }) + + t.Run("does not repeat the summary as the combined display message", func(t *testing.T) { + summary := "Policy blocked it." + output := MigrationStatus(elmapi.MigrationDetail{ + Migration: &elmapi.MigrationSummary{MigrationID: "mig-1", Status: &failedStatus}, + CombinedState: &elmapi.CombinedState{ + Status: &failedStatus, + DisplayMessage: summary, + TerminalFailure: failure("repository_policy", summary, nil), + }, + }) + + assert.Equal(t, 1, strings.Count(output, summary), + "the authored summary should render once, in the failure section") + }) + + t.Run("still renders a display message that differs from the summary", func(t *testing.T) { + output := MigrationStatus(elmapi.MigrationDetail{ + Migration: &elmapi.MigrationSummary{MigrationID: "mig-1", Status: &failedStatus}, + CombinedState: &elmapi.CombinedState{ + Status: &failedStatus, + DisplayMessage: "Failed: 2 resources failed", + TerminalFailure: failure("repository_policy", "Policy blocked it.", nil), + }, + }) + + assert.Contains(t, output, "Policy blocked it.") + assert.Contains(t, output, "Failed: 2 resources failed") + }) + + t.Run("renders no failure section for a healthy migration", func(t *testing.T) { + inProgress := "in_progress" + output := MigrationStatus(elmapi.MigrationDetail{ + Migration: &elmapi.MigrationSummary{MigrationID: "mig-1", Status: &inProgress}, + TargetState: &elmapi.TargetState{Status: &inProgress}, + }) + + assert.NotContains(t, output, "Failure") + }) +} + +func TestCutoverStatusTerminalFailure(t *testing.T) { + // Guards both regressions at once: the summary must survive (it was being + // suppressed as a duplicate of a section that never rendered) and must not + // appear twice once the section does render. + t.Run("renders the cause exactly once", func(t *testing.T) { + failedStatus := "failed" + summary := "Policy blocked it." + output := CutoverStatus(elmapi.MigrationDetail{ + CombinedState: &elmapi.CombinedState{ + Status: &failedStatus, + DisplayMessage: summary, + TerminalFailure: failure("repository_policy", summary, nil), + }, + }) + + assert.Equal(t, 1, strings.Count(output, summary), "cause should render once, in the failure section") + assert.Contains(t, output, "Failure") + assert.Contains(t, output, "repository_policy") + }) + + t.Run("keeps a display message that is not the failure summary", func(t *testing.T) { + failedStatus := "failed" + output := CutoverStatus(elmapi.MigrationDetail{ + CombinedState: &elmapi.CombinedState{ + Status: &failedStatus, + DisplayMessage: "Cutover is blocked.", + TerminalFailure: failure("repository_policy", "Policy blocked it.", nil), + }, + }) + + assert.Contains(t, output, "Cutover is blocked.") + assert.Contains(t, output, "Policy blocked it.") + }) + + t.Run("renders no failure section when nothing failed", func(t *testing.T) { + inProgress := "in_progress" + output := CutoverStatus(elmapi.MigrationDetail{ + CombinedState: &elmapi.CombinedState{Status: &inProgress, DisplayMessage: "Still running."}, + }) + + assert.NotContains(t, output, "Failure") + assert.Contains(t, output, "Still running.") + }) +}