diff --git a/TEMPLATES.md b/TEMPLATES.md index 195f0b10e..661c954dd 100644 --- a/TEMPLATES.md +++ b/TEMPLATES.md @@ -6792,8 +6792,8 @@ No recent schema changes 1 active schema change - APPLY ID EXTERNAL OP ID DATABASE ENV DEPLOYMENT STATE STARTED SOURCE - apply-multi-a1b2c3d4 remote-op-us-east-001 orders-db production us-east Waiting for cutover 8 minutes ago https://github.com/acme/shop/pull/412 + APPLY ID EXTERNAL APPLY ID EXTERNAL OP ID DATABASE ENV STATE STARTED SOURCE + apply-multi-a1b2c3d4 remote-apply-us-east-001 remote-op-us-east-001 orders-db production Waiting for cutover 8 minutes ago https://github.com/acme/shop/pull/412 Use 'schemabot status ' to view details @@ -6801,8 +6801,17 @@ Multiple matching operations: 1 active schema change - APPLY ID EXTERNAL OP ID DATABASE ENV DEPLOYMENT STATE STARTED SOURCE - apply-sharded-d5e6f7g8 - inventory-db production us-east Running 4 minutes ago https://github.com/acme/shop/pull/412 + APPLY ID EXTERNAL APPLY ID DATABASE ENV STATE STARTED SOURCE + apply-sharded-d5e6f7g8 remote-apply-us-east-002 inventory-db production Running 4 minutes ago https://github.com/acme/shop/pull/412 + +Use 'schemabot status ' to view details + +No data-plane apply id recorded: + +1 active schema change + + APPLY ID DATABASE ENV STATE STARTED SOURCE + apply-pending-l3m4n5o6 payments-db production Pending 1 minute ago https://github.com/acme/shop/pull/412 Use 'schemabot status ' to view details diff --git a/pkg/api/handlers_test.go b/pkg/api/handlers_test.go index f3f353202..87e795e18 100644 --- a/pkg/api/handlers_test.go +++ b/pkg/api/handlers_test.go @@ -4316,73 +4316,139 @@ func TestHandleStatusDeploymentFilterProjectsMatchingOperation(t *testing.T) { assert.Equal(t, state.Apply.Completed, resp.Applies[0].State) } -func TestHandleStatusDeploymentFilterSummarizesMatchingOperations(t *testing.T) { +// A deployment applied per shard has exactly one data-plane apply, so the +// deployment-filtered status list surfaces exactly one remote apply handle per +// row: the shared id the operations recorded, or the parent apply row's when +// the drive recorded it there instead (a drive that is not operation-scoped +// writes the remote id to the parent). Per-operation remote ids surface only +// when the filter matches a single operation; a fold keeps them in the detail +// views. +func TestHandleStatusDeploymentFilterRemoteHandles(t *testing.T) { now := time.Now().UTC() startedAt := now.Add(-2 * time.Minute) completedAt := now.Add(-time.Minute) - applies := &recentApplyStore{ - applies: []*storage.Apply{ - { - ID: 101, - ApplyIdentifier: "apply-deployment", - ExternalID: "parent-external", - Database: "orders", - Environment: "staging", - Deployment: "deploy-a", - Engine: storage.EngineSpirit, - State: state.Apply.Completed, - Caller: "cli", - CreatedAt: now, - UpdatedAt: now, - }, - }, + completedOp := func(externalID, externalOperationID string) *storage.ApplyOperation { + return &storage.ApplyOperation{ + ID: 202, + ApplyID: 101, + Deployment: "deploy-a", + ExternalID: externalID, + ExternalOperationID: externalOperationID, + State: state.Apply.Completed, + StartedAt: &startedAt, + CompletedAt: &completedAt, + CreatedAt: now.Add(-2 * time.Minute), + UpdatedAt: completedAt, + } } - operations := &staticApplyOperationStore{ - operations: []*storage.ApplyOperation{ - { - ID: 202, - ApplyID: 101, - Deployment: "deploy-a", - ExternalOperationID: "remote-operation-202", - State: state.Apply.Completed, - StartedAt: &startedAt, - CompletedAt: &completedAt, - CreatedAt: now.Add(-2 * time.Minute), - UpdatedAt: completedAt, - }, - { - ID: 203, - ApplyID: 101, - Deployment: "deploy-a", - ExternalOperationID: "remote-operation-203", - State: state.Apply.Running, - StartedAt: &startedAt, - CreatedAt: now.Add(-90 * time.Second), - UpdatedAt: now, - }, + runningOp := func(externalID, externalOperationID string) *storage.ApplyOperation { + return &storage.ApplyOperation{ + ID: 203, + ApplyID: 101, + Deployment: "deploy-a", + ExternalID: externalID, + ExternalOperationID: externalOperationID, + State: state.Apply.Running, + StartedAt: &startedAt, + CreatedAt: now.Add(-90 * time.Second), + UpdatedAt: now, + } + } + + cases := []struct { + name string + parentExternalID string + applyState string + operations []*storage.ApplyOperation + wantActiveCount int + wantState string + wantExternalID string + wantExternalOpID string + }{ + { + name: "a fold surfaces the shared data-plane apply id and no per-operation id", + applyState: state.Apply.Running, + operations: []*storage.ApplyOperation{completedOp("remote-apply-shared", "remote-operation-202"), runningOp("remote-apply-shared", "remote-operation-203")}, + wantActiveCount: 1, + wantState: state.Apply.Running, + wantExternalID: "remote-apply-shared", + wantExternalOpID: "", + }, + { + name: "a fold whose operations recorded no remote id keeps the parent apply row's", + parentExternalID: "parent-external", + applyState: state.Apply.Completed, + operations: []*storage.ApplyOperation{completedOp("", "remote-operation-202"), runningOp("", "remote-operation-203")}, + wantActiveCount: 1, + wantState: state.Apply.Running, + wantExternalID: "parent-external", + wantExternalOpID: "", + }, + { + name: "a single matching operation without a remote id keeps the parent apply row's", + parentExternalID: "parent-external", + applyState: state.Apply.Running, + operations: []*storage.ApplyOperation{runningOp("", "")}, + wantActiveCount: 1, + wantState: state.Apply.Running, + wantExternalID: "parent-external", + wantExternalOpID: "", + }, + { + name: "a single matching operation's own remote ids win over the parent's", + parentExternalID: "parent-external", + applyState: state.Apply.Completed, + operations: []*storage.ApplyOperation{completedOp("remote-apply-own", "remote-operation-202")}, + wantActiveCount: 0, + wantState: state.Apply.Completed, + wantExternalID: "remote-apply-own", + wantExternalOpID: "remote-operation-202", }, } - stor := &mockStorageWithApplyStores{applies: applies, operations: operations} - logger := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelError})) - svc := New(stor, testServerConfig(), nil, logger) - mux := http.NewServeMux() - svc.ConfigureRoutes(mux) - req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/api/status?environment=staging&deployment=deploy-a", nil) - w := httptest.NewRecorder() - mux.ServeHTTP(w, req) + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + applies := &recentApplyStore{ + applies: []*storage.Apply{ + { + ID: 101, + ApplyIdentifier: "apply-deployment", + ExternalID: tc.parentExternalID, + Database: "orders", + Environment: "staging", + Deployment: "deploy-a", + Engine: storage.EngineSpirit, + State: tc.applyState, + Caller: "cli", + CreatedAt: now, + UpdatedAt: now, + }, + }, + } + operations := &staticApplyOperationStore{operations: tc.operations} + stor := &mockStorageWithApplyStores{applies: applies, operations: operations} + logger := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelError})) + svc := New(stor, testServerConfig(), nil, logger) + mux := http.NewServeMux() + svc.ConfigureRoutes(mux) - require.Equal(t, http.StatusOK, w.Code) - var resp apitypes.StatusResponse - require.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp)) + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/api/status?environment=staging&deployment=deploy-a", nil) + w := httptest.NewRecorder() + mux.ServeHTTP(w, req) - assert.Equal(t, 1, resp.ActiveCount) - require.Len(t, resp.Applies, 1) - assert.Equal(t, "apply-deployment", resp.Applies[0].ApplyID) - assert.Empty(t, resp.Applies[0].ExternalID) - assert.Empty(t, resp.Applies[0].ExternalOperationID) - assert.Equal(t, "deploy-a", resp.Applies[0].Deployment) - assert.Equal(t, state.Apply.Running, resp.Applies[0].State) + require.Equal(t, http.StatusOK, w.Code) + var resp apitypes.StatusResponse + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp)) + + assert.Equal(t, tc.wantActiveCount, resp.ActiveCount) + require.Len(t, resp.Applies, 1) + assert.Equal(t, "apply-deployment", resp.Applies[0].ApplyID) + assert.Equal(t, tc.wantExternalID, resp.Applies[0].ExternalID) + assert.Equal(t, tc.wantExternalOpID, resp.Applies[0].ExternalOperationID) + assert.Equal(t, "deploy-a", resp.Applies[0].Deployment) + assert.Equal(t, tc.wantState, resp.Applies[0].State) + }) + } } func TestHandleStatusFailedFilter(t *testing.T) { diff --git a/pkg/api/progress_handlers.go b/pkg/api/progress_handlers.go index e8d9f7f00..a22009fdd 100644 --- a/pkg/api/progress_handlers.go +++ b/pkg/api/progress_handlers.go @@ -992,7 +992,13 @@ func activeApplyResponseFromStorage(apply *storage.Apply, op *storage.ApplyOpera } if op != nil { active.Deployment = op.Deployment - active.ExternalID = op.ExternalID + // A drive that is not operation-scoped records the remote apply id on + // the parent apply row, not the operation, so an empty operation-level + // id keeps the parent's rather than hiding the deployment's one remote + // handle. + if op.ExternalID != "" { + active.ExternalID = op.ExternalID + } active.ExternalOperationID = op.ExternalOperationID active.State = op.State active.ErrorMessage = op.ErrorMessage diff --git a/pkg/cmd/internal/templates/preview_status.go b/pkg/cmd/internal/templates/preview_status.go index 4def5ab2d..5b54c01fe 100644 --- a/pkg/cmd/internal/templates/preview_status.go +++ b/pkg/cmd/internal/templates/preview_status.go @@ -89,6 +89,7 @@ func previewStatusDeploymentOutput() { Applies: []ActiveApplyData{ { ApplyID: "apply-multi-a1b2c3d4", + ExternalID: "remote-apply-us-east-001", ExternalOperationID: "remote-op-us-east-001", Database: "orders-db", Environment: "production", @@ -112,6 +113,7 @@ func previewStatusDeploymentOutput() { Applies: []ActiveApplyData{ { ApplyID: "apply-sharded-d5e6f7g8", + ExternalID: "remote-apply-us-east-002", Database: "inventory-db", Environment: "production", Deployment: "us-east", @@ -123,6 +125,28 @@ func previewStatusDeploymentOutput() { }, }, }) + + fmt.Println() + fmt.Println("No data-plane apply id recorded:") + fmt.Println() + WriteStatusList(StatusListData{ + ActiveCount: 1, + ShowExternalID: true, + Deployment: "us-east", + Applies: []ActiveApplyData{ + { + ApplyID: "apply-pending-l3m4n5o6", + Database: "payments-db", + Environment: "production", + Deployment: "us-east", + State: state.Apply.Pending, + Engine: "Spirit", + Caller: "github:octocat@acme/shop#412", + StartedAt: previewTime.Add(-1 * time.Minute).Format(time.RFC3339), + UpdatedAt: previewTime.Add(-10 * time.Second).Format(time.RFC3339), + }, + }, + }) } func previewStatusHistoryOutput() { diff --git a/pkg/cmd/internal/templates/progress.go b/pkg/cmd/internal/templates/progress.go index 84b5a3f06..d640c4f16 100644 --- a/pkg/cmd/internal/templates/progress.go +++ b/pkg/cmd/internal/templates/progress.go @@ -1045,125 +1045,29 @@ func WriteStatusList(data StatusListData) { writeStatusStateSummary(data.StateCounts) fmt.Println() - // Calculate column widths from data - showDeployment := statusListShowsDeployment(data) - maxID := 8 // "APPLY ID" - maxExternal := len(statusExternalIDHeader(data)) - maxDB := 8 // "DATABASE" - maxEnv := 3 // "ENV" - maxDeployment := 10 // "DEPLOYMENT" - maxState := 5 // "STATE" - maxStarted := 7 // "STARTED" - for _, a := range data.Applies { - maxID = maxLen(maxID, len(a.ApplyID)) - if data.ShowExternalID { - maxExternal = maxLen(maxExternal, len(statusExternalID(data, a))) - } - maxDB = maxLen(maxDB, len(a.Database)) - maxEnv = maxLen(maxEnv, len(a.Environment)) - if showDeployment { - maxDeployment = maxLen(maxDeployment, len(a.Deployment)) - } - maxState = maxLen(maxState, len(state.Label(a.State))) - maxStarted = maxLen(maxStarted, len(formatStartedAt(a.StartedAt))) - } + columns := statusListColumns(data) + widths := statusListColumnWidths(columns, data.Applies) // Table header - switch { - case data.ShowExternalID && showDeployment: - fmt.Printf(" %s%-*s %-*s %-*s %-*s %-*s %-*s %-*s %s%s\n", - ANSIDim, - maxID, "APPLY ID", - maxExternal, statusExternalIDHeader(data), - maxDB, "DATABASE", - maxEnv, "ENV", - maxDeployment, "DEPLOYMENT", - maxState, "STATE", - maxStarted, "STARTED", - "SOURCE", - ANSIReset) - case data.ShowExternalID: - fmt.Printf(" %s%-*s %-*s %-*s %-*s %-*s %-*s %s%s\n", - ANSIDim, - maxID, "APPLY ID", - maxExternal, statusExternalIDHeader(data), - maxDB, "DATABASE", - maxEnv, "ENV", - maxState, "STATE", - maxStarted, "STARTED", - "SOURCE", - ANSIReset) - case showDeployment: - fmt.Printf(" %s%-*s %-*s %-*s %-*s %-*s %-*s %s%s\n", - ANSIDim, - maxID, "APPLY ID", - maxDB, "DATABASE", - maxEnv, "ENV", - maxDeployment, "DEPLOYMENT", - maxState, "STATE", - maxStarted, "STARTED", - "SOURCE", - ANSIReset) - default: - fmt.Printf(" %s%-*s %-*s %-*s %-*s %-*s %s%s\n", - ANSIDim, - maxID, "APPLY ID", - maxDB, "DATABASE", - maxEnv, "ENV", - maxState, "STATE", - maxStarted, "STARTED", - "SOURCE", - ANSIReset) + fmt.Print(" " + ANSIDim) + for i, column := range columns { + fmt.Print(statusCell(column.header, widths[i], column.last)) } + fmt.Println(ANSIReset) // Table rows for _, a := range data.Applies { - label := state.Label(a.State) - colorFn := stateColorFunc(a.State) - padded := fmt.Sprintf("%-*s", maxState, label) - coloredState := padded - if colorFn != nil { - coloredState = colorFn(padded) - } - - switch { - case data.ShowExternalID && showDeployment: - fmt.Printf(" %-*s %-*s %-*s %-*s %-*s %s %-*s %s\n", - maxID, a.ApplyID, - maxExternal, statusExternalID(data, a), - maxDB, a.Database, - maxEnv, a.Environment, - maxDeployment, a.Deployment, - coloredState, - maxStarted, formatStartedAt(a.StartedAt), - applySource(a.Caller)) - case data.ShowExternalID: - fmt.Printf(" %-*s %-*s %-*s %-*s %s %-*s %s\n", - maxID, a.ApplyID, - maxExternal, statusExternalID(data, a), - maxDB, a.Database, - maxEnv, a.Environment, - coloredState, - maxStarted, formatStartedAt(a.StartedAt), - applySource(a.Caller)) - case showDeployment: - fmt.Printf(" %-*s %-*s %-*s %-*s %s %-*s %s\n", - maxID, a.ApplyID, - maxDB, a.Database, - maxEnv, a.Environment, - maxDeployment, a.Deployment, - coloredState, - maxStarted, formatStartedAt(a.StartedAt), - applySource(a.Caller)) - default: - fmt.Printf(" %-*s %-*s %-*s %s %-*s %s\n", - maxID, a.ApplyID, - maxDB, a.Database, - maxEnv, a.Environment, - coloredState, - maxStarted, formatStartedAt(a.StartedAt), - applySource(a.Caller)) + fmt.Print(" ") + for i, column := range columns { + cell := statusCell(statusColumnValue(column, a), widths[i], column.last) + if column.colored { + if colorFn := stateColorFunc(a.State); colorFn != nil { + cell = colorFn(cell) + } + } + fmt.Print(cell) } + fmt.Println() } writeStatusListFooter(data) @@ -1217,36 +1121,118 @@ func writeFailedStatusList(data StatusListData) { } } -func statusExternalID(data StatusListData, a ActiveApplyData) string { - if a.ExternalOperationID != "" { - return a.ExternalOperationID +// statusColumn is one column of the status list. An optional column is dropped +// when no row on the page has a value for it, so an operator only ever sees the +// columns their own fleet populates: a deployment that drives its applies +// locally has no remote handles to show, and an unfiltered list of a +// single-deployment fleet has no deployment to distinguish. +type statusColumn struct { + header string + value func(a ActiveApplyData) string + optional bool + colored bool + last bool +} + +// statusListColumns returns the columns the list renders, in order. The +// deployment-filtered list names both remote handles the way the detail views +// already do — the deployment's shared data-plane apply id and the +// per-operation remote row id — and omits DEPLOYMENT, which every row repeats +// back to the operator who named it. +func statusListColumns(data StatusListData) []statusColumn { + columns := []statusColumn{ + {header: "APPLY ID", value: func(a ActiveApplyData) string { return a.ApplyID }}, + } + if data.ShowExternalID { + if data.Deployment != "" { + columns = append(columns, + statusColumn{header: "EXTERNAL APPLY ID", optional: true, value: func(a ActiveApplyData) string { return a.ExternalID }}, + statusColumn{header: "EXTERNAL OP ID", optional: true, value: func(a ActiveApplyData) string { return a.ExternalOperationID }}, + ) + } else { + // Unconditional: the operator asked for this column by flag, so an + // all-dash column positively answers "nothing recorded" — dropping + // it would be indistinguishable from the flag doing nothing. + columns = append(columns, + statusColumn{header: "EXTERNAL ID", value: unfilteredStatusExternalID}, + ) + } + } + columns = append(columns, + statusColumn{header: "DATABASE", value: func(a ActiveApplyData) string { return a.Database }}, + statusColumn{header: "ENV", value: func(a ActiveApplyData) string { return a.Environment }}, + ) + if data.Deployment == "" { + columns = append(columns, + statusColumn{header: "DEPLOYMENT", optional: true, value: func(a ActiveApplyData) string { return a.Deployment }}, + ) + } + columns = append(columns, + statusColumn{header: "STATE", colored: true, value: func(a ActiveApplyData) string { return state.Label(a.State) }}, + statusColumn{header: "STARTED", value: func(a ActiveApplyData) string { return formatStartedAt(a.StartedAt) }}, + statusColumn{header: "SOURCE", last: true, value: func(a ActiveApplyData) string { return applySource(a.Caller) }}, + ) + return retainPopulatedStatusColumns(columns, data.Applies) +} + +// retainPopulatedStatusColumns drops every optional column no row fills in. +func retainPopulatedStatusColumns(columns []statusColumn, applies []ActiveApplyData) []statusColumn { + retained := make([]statusColumn, 0, len(columns)) + for _, column := range columns { + if column.optional && !anyStatusRowFillsColumn(column, applies) { + continue + } + retained = append(retained, column) } - if data.Deployment != "" { - return "-" + return retained +} + +func anyStatusRowFillsColumn(column statusColumn, applies []ActiveApplyData) bool { + for _, a := range applies { + if column.value(a) != "" { + return true + } } - if a.ExternalID == "" { - return "-" + return false +} + +// statusColumnValue renders a row's cell, standing a dash in for a value this +// row is missing from a column other rows on the page do fill. +func statusColumnValue(column statusColumn, a ActiveApplyData) string { + if value := column.value(a); value != "" { + return value } - return a.ExternalID + return "-" } -func statusExternalIDHeader(data StatusListData) string { - if data.Deployment != "" { - return "EXTERNAL OP ID" +func statusListColumnWidths(columns []statusColumn, applies []ActiveApplyData) []int { + widths := make([]int, len(columns)) + for i, column := range columns { + widths[i] = len(column.header) + for _, a := range applies { + widths[i] = maxLen(widths[i], len(statusColumnValue(column, a))) + } } - return "EXTERNAL ID" + return widths } -func statusListShowsDeployment(data StatusListData) bool { - if data.Deployment != "" { - return true +// statusCell pads a cell to its column width, leaving the last column ragged so +// the row carries no trailing whitespace. +func statusCell(value string, width int, last bool) string { + if last { + return value } - for _, apply := range data.Applies { - if apply.Deployment != "" { - return true - } + return fmt.Sprintf("%-*s ", width, value) +} + +// unfilteredStatusExternalID collapses both remote handles into the single +// EXTERNAL ID column an unfiltered list shows, preferring the per-operation row +// id when the apply has one. +func unfilteredStatusExternalID(a ActiveApplyData) string { + if a.ExternalOperationID != "" { + return a.ExternalOperationID } - return false + return a.ExternalID } func statusFailureActor(a ActiveApplyData, showExternalID bool) string { @@ -1254,7 +1240,11 @@ func statusFailureActor(a ActiveApplyData, showExternalID bool) string { if !showExternalID { return actor } - return actor + "; external_id=" + statusExternalID(StatusListData{}, a) + externalID := unfilteredStatusExternalID(a) + if externalID == "" { + externalID = "-" + } + return actor + "; external_id=" + externalID } func formatFailureTimestamp(a ActiveApplyData) string { diff --git a/pkg/cmd/internal/templates/progress_states_test.go b/pkg/cmd/internal/templates/progress_states_test.go index f321fad65..343347c4f 100644 --- a/pkg/cmd/internal/templates/progress_states_test.go +++ b/pkg/cmd/internal/templates/progress_states_test.go @@ -198,7 +198,80 @@ func TestWriteStatusListExternalID(t *testing.T) { assert.Contains(t, output, "apply-complete") } -func TestWriteStatusListDeploymentExternalOperationID(t *testing.T) { +// An operator who asked for the external-id column gets it even when no apply +// on the page recorded a remote id: an all-dash column positively answers +// "nothing recorded", where a missing column would be indistinguishable from +// the flag doing nothing. +func TestWriteStatusListExternalIDColumnRendersWithoutValues(t *testing.T) { + output := captureStdout(t, func() { + WriteStatusList(StatusListData{ + ActiveCount: 0, + Limit: 20, + MaxLimit: 1000, + ShowExternalID: true, + Applies: []ActiveApplyData{ + { + ApplyID: "apply-local", + Database: "orders", + Environment: "staging", + State: state.Apply.Completed, + StartedAt: "2026-05-28T12:00:00Z", + CompletedAt: "2026-05-28T12:00:02Z", + Caller: "cli", + }, + }, + }) + }) + + assert.Contains(t, output, "EXTERNAL ID", "the requested column renders even with nothing recorded") + assert.Contains(t, output, "apply-local -", "a row with no remote id shows a dash in the column") +} + +// On a mixed unfiltered page the DEPLOYMENT column is retained for the rows +// that carry one, and a row without a deployment shows a dash rather than +// blank padding, so the gap reads as "none recorded" instead of an alignment +// artifact. +func TestWriteStatusListMixedDeploymentRowsShowDash(t *testing.T) { + output := captureStdout(t, func() { + WriteStatusList(StatusListData{ + ActiveCount: 0, + Limit: 20, + MaxLimit: 1000, + Applies: []ActiveApplyData{ + { + ApplyID: "apply-deployed", + Database: "orders", + Environment: "staging", + Deployment: "deploy-a", + State: state.Apply.Completed, + StartedAt: "2026-05-28T12:00:00Z", + CompletedAt: "2026-05-28T12:00:02Z", + Caller: "cli", + }, + { + ApplyID: "apply-local", + Database: "orders", + Environment: "staging", + State: state.Apply.Completed, + StartedAt: "2026-05-28T12:01:00Z", + CompletedAt: "2026-05-28T12:01:02Z", + Caller: "cli", + }, + }, + }) + }) + + assert.Contains(t, output, "DEPLOYMENT", "one populated row retains the column for the page") + assert.Contains(t, output, "deploy-a") + assert.Contains(t, output, "staging -", "a deployment-less row shows a dash in the retained column") +} + +// A deployment-filtered list names each remote handle in its own column, the +// way the detail views do: the deployment's shared data-plane apply id and the +// per-operation remote row id. APPLY ID stays the control-plane id the status +// drill-down resolves, and DEPLOYMENT is dropped because every row repeats it +// back to the operator who named it. +func TestWriteStatusListDeploymentNamesBothRemoteHandles(t *testing.T) { output := captureStdout(t, func() { WriteStatusList(StatusListData{ ActiveCount: 1, @@ -209,7 +282,7 @@ func TestWriteStatusListDeploymentExternalOperationID(t *testing.T) { Applies: []ActiveApplyData{ { ApplyID: "apply-running", - ExternalID: "parent-external", + ExternalID: "apply-remote-a", ExternalOperationID: "remote-operation-a", Database: "orders", Environment: "staging", @@ -222,11 +295,75 @@ func TestWriteStatusListDeploymentExternalOperationID(t *testing.T) { }) }) + assert.Contains(t, output, "EXTERNAL APPLY ID") assert.Contains(t, output, "EXTERNAL OP ID") - assert.Contains(t, output, "DEPLOYMENT") + assert.Contains(t, output, "apply-remote-a") assert.Contains(t, output, "remote-operation-a") - assert.NotContains(t, output, "parent-external") - assert.Contains(t, output, "deploy-a") + assert.Contains(t, output, "apply-running", + "APPLY ID stays the control-plane id the status drill-down resolves") + assert.NotContains(t, output, "DEPLOYMENT", + "a list filtered to one deployment repeats it on every row, so the column carries nothing") + assert.Contains(t, output, "Use 'schemabot status ' to view details", + "every id in the APPLY ID column feeds the drill-down, so the footer needs no qualifier") +} + +// A deployment that drives its applies locally records no remote handles, so +// the remote-id columns are left out rather than rendered as a column of +// dashes. The same holds for an apply not yet dispatched to a data plane. +func TestWriteStatusListDeploymentOmitsUnpopulatedRemoteColumns(t *testing.T) { + output := captureStdout(t, func() { + WriteStatusList(StatusListData{ + ActiveCount: 1, + Limit: 20, + MaxLimit: 1000, + ShowExternalID: true, + Deployment: "deploy-a", + Applies: []ActiveApplyData{ + { + ApplyID: "apply-pending", + Database: "orders", + Environment: "staging", + Deployment: "deploy-a", + State: state.Apply.Pending, + Caller: "cli", + }, + }, + }) + }) + + assert.Contains(t, output, "apply-pending") + assert.NotContains(t, output, "EXTERNAL APPLY ID") + assert.NotContains(t, output, "EXTERNAL OP ID") +} + +// A deployment whose operations fold into one shared data-plane apply has no +// per-operation remote row id, so only the shared handle gets a column. +func TestWriteStatusListDeploymentOmitsOperationColumnWhenOnlyTheSharedApplyIsRecorded(t *testing.T) { + output := captureStdout(t, func() { + WriteStatusList(StatusListData{ + ActiveCount: 1, + Limit: 20, + MaxLimit: 1000, + ShowExternalID: true, + Deployment: "deploy-a", + Applies: []ActiveApplyData{ + { + ApplyID: "apply-sharded", + ExternalID: "apply-remote-shared", + Database: "inventory", + Environment: "staging", + Deployment: "deploy-a", + State: state.Apply.Running, + StartedAt: "2026-05-28T12:00:00Z", + Caller: "cli", + }, + }, + }) + }) + + assert.Contains(t, output, "EXTERNAL APPLY ID") + assert.Contains(t, output, "apply-remote-shared") + assert.NotContains(t, output, "EXTERNAL OP ID") } func TestWriteStatusListFailedOnly(t *testing.T) { diff --git a/pkg/tern/grpc_client_test.go b/pkg/tern/grpc_client_test.go index 042bd40a0..565d292a9 100644 --- a/pkg/tern/grpc_client_test.go +++ b/pkg/tern/grpc_client_test.go @@ -918,7 +918,15 @@ func (m *mockTaskStore) GetByApplyOperationID(_ context.Context, applyOperationI if m.getByOperationIDErr != nil { return nil, m.getByOperationIDErr } - return m.tasks, nil + // The real store returns a non-nil empty slice when an operation owns no + // tasks, and callers are entitled to rely on that. + scoped := make([]*storage.Task, 0, len(m.tasks)) + for _, task := range m.tasks { + if task.ApplyOperationID != nil && *task.ApplyOperationID == applyOperationID { + scoped = append(scoped, task) + } + } + return scoped, nil } func (m *mockTaskStore) Update(context.Context, *storage.Task) error { return m.updateErr } @@ -1405,8 +1413,15 @@ func TestGRPCClient_ResumeApplyOperationDispatchesScopedTasks(t *testing.T) { ID: apply.PlanID, PlanIdentifier: "plan-op-scoped", }}, + // The real task query joins a work operation to its tasks on + // operation_key = namespace/shard/table_name, so the operation carries the + // key its task derives — a keyless operation owns no task rows. operations: &mockApplyOperationStore{ops: map[int64]*storage.ApplyOperation{ - operationID: {ID: operationID, ApplyID: apply.ID, Deployment: "testdb-deployment", State: state.ApplyOperation.Pending}, + operationID: { + ID: operationID, ApplyID: apply.ID, Deployment: "testdb-deployment", + OperationKind: storage.ApplyOperationKindWork, OperationKey: "default/-80/users", + State: state.ApplyOperation.Pending, + }, }}, } @@ -1426,6 +1441,98 @@ func TestGRPCClient_ResumeApplyOperationDispatchesScopedTasks(t *testing.T) { assert.Equal(t, []string{"-80"}, req.TargetShards) } +// A deployment applied per shard dispatches each shard's operation to the +// remote data plane under one deployment-keyed idempotency key, so the data +// plane lands every sibling in the deployment's single data-plane apply: the +// first dispatch creates it, each later sibling attaches its own operation. +// Both operation rows record that one remote apply id, and the parent apply's +// external_id stays untouched — one deployment, one data-plane apply. +func TestGRPCClient_SiblingShardOperationsRecordOneRemoteApply(t *testing.T) { + server := &capturingTernServer{ + remoteApplyID: "remote-shared-1", + progressTables: []*ternv1.TableProgress{{ + Namespace: "commerce", + TableName: "users", + Status: state.Task.Completed, + PercentComplete: 100, + }}, + } + client, cleanup := testCapturingGRPCClient(t, server) + defer cleanup() + + apply := &storage.Apply{ + ID: 7, + ApplyIdentifier: "apply-sharded", + PlanID: 99, + Database: "commerce", + DatabaseType: storage.DatabaseTypeStrata, + Environment: "staging", + State: state.Apply.Pending, + } + apply.SetOptions(storage.ApplyOptions{Target: "commerce-target"}) + opA, opB := int64(41), int64(42) + taskA := &storage.Task{ + ID: 11, TaskIdentifier: "task-users-a", ApplyID: apply.ID, ApplyOperationID: &opA, + TableName: "users", Shard: "-80", Namespace: "commerce", + DDL: "ALTER TABLE users ADD COLUMN email varchar(255)", DDLAction: "alter", State: state.Task.Pending, + } + taskB := &storage.Task{ + ID: 12, TaskIdentifier: "task-users-b", ApplyID: apply.ID, ApplyOperationID: &opB, + TableName: "users", Shard: "80-", Namespace: "commerce", + DDL: "ALTER TABLE users ADD COLUMN email varchar(255)", DDLAction: "alter", State: state.Task.Pending, + } + operationStore := &mockApplyOperationStore{ops: map[int64]*storage.ApplyOperation{ + opA: {ID: opA, ApplyID: apply.ID, Deployment: "commerce-deployment", OperationKey: "commerce/-80/users", OperationKind: storage.ApplyOperationKindWork, State: state.ApplyOperation.Pending}, + opB: {ID: opB, ApplyID: apply.ID, Deployment: "commerce-deployment", OperationKey: "commerce/80-/users", OperationKind: storage.ApplyOperationKindWork, State: state.ApplyOperation.Pending}, + }} + client.storage = &mockStorage{ + applies: &mockApplyStore{apply: apply}, + tasks: &mockTaskStore{tasks: []*storage.Task{taskA, taskB}}, + plans: &mockPlanStore{plan: &storage.Plan{ID: apply.PlanID, PlanIdentifier: "plan-sharded"}}, + operations: operationStore, + } + + // Each dispatch is poll-driven and costs at least one progress tick, so + // give every drive its own deadline rather than sharing one across both. + driveCtx := func() (context.Context, context.CancelFunc) { + return context.WithTimeout(t.Context(), 2*time.Second) + } + + firstCtx, cancelFirst := driveCtx() + defer cancelFirst() + require.NoError(t, client.ResumeApplyOperation(firstCtx, apply, opA)) + firstReq := server.getApplyRequest() + require.NotNil(t, firstReq, "the first shard operation must dispatch to the data plane") + assert.Equal(t, []string{"-80"}, firstReq.TargetShards) + assert.Equal(t, "remote-shared-1", operationStore.ops[opA].ExternalID) + + secondCtx, cancelSecond := driveCtx() + defer cancelSecond() + require.NoError(t, client.ResumeApplyOperation(secondCtx, apply, opB)) + secondReq := server.getApplyRequest() + require.NotNil(t, secondReq, "the sibling shard operation must dispatch to the data plane") + assert.Equal(t, []string{"80-"}, secondReq.TargetShards) + + require.NotEmpty(t, firstReq.IdempotencyKey) + assert.Equal(t, firstReq.IdempotencyKey, secondReq.IdempotencyKey, + "sibling dispatches must carry the deployment-keyed idempotency key so the data plane attaches them into one apply") + + // The idempotency key routes a sibling into the shared apply; the generation + // manifest is what makes that apply wait for the siblings still to come, so + // both dispatches must declare the deployment's whole operation set. + expectedManifest := []string{"commerce/-80/users", "commerce/80-/users"} + assert.Equal(t, expectedManifest, firstReq.GenerationOperationKeys, + "the dispatch that creates the shared apply must declare every sibling it will wait for") + assert.Equal(t, expectedManifest, secondReq.GenerationOperationKeys, + "an attaching sibling must declare the same generation manifest so the data plane can verify agreement") + + assert.Equal(t, "remote-shared-1", operationStore.ops[opA].ExternalID) + assert.Equal(t, "remote-shared-1", operationStore.ops[opB].ExternalID, + "every operation of the deployment must record the deployment's one remote apply id") + assert.Empty(t, apply.ExternalID, + "a multi-operation dispatch must not write the parent apply external_id") +} + func TestGRPCClient_ResumeApplyOperationDispatchesGroupFinalizerAsVSchemaOnly(t *testing.T) { // A task-less group_finalizer operation is driven over the remote path by // dispatching the namespace VSchema as a VSchema-only apply (no DDL, no target