Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
115 changes: 115 additions & 0 deletions cmd/kitchen-sink/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down
13 changes: 13 additions & 0 deletions internal/cmd/migration/watch/view.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"strings"
"time"

"github.com/github/gh-elm/internal/render"
"github.com/github/gh-elm/internal/theme"
)

Expand Down Expand Up @@ -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)
}
Expand Down
57 changes: 57 additions & 0 deletions internal/cmd/migration/watch/watch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
})
}
32 changes: 32 additions & 0 deletions internal/elmapi/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down
73 changes: 73 additions & 0 deletions internal/elmapi/migrations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
})
}
25 changes: 22 additions & 3 deletions internal/render/migration.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
)
}
Expand All @@ -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 {
Expand Down Expand Up @@ -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 ""
}
Expand All @@ -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 {
Expand Down
Loading
Loading