From a50066253174b06b40b20ffbee8d8192cffcbc28 Mon Sep 17 00:00:00 2001 From: Tmwakalasya Date: Thu, 29 Oct 2026 16:43:15 -0400 Subject: [PATCH] fix(client): preserve terminal watch outcomes --- submitqueue/client/history_read_test.go | 29 +++++++++++++++++++++ submitqueue/client/view.go | 34 ++++++++----------------- submitqueue/client/view_test.go | 28 ++++++++++++++++++++ 3 files changed, 67 insertions(+), 24 deletions(-) diff --git a/submitqueue/client/history_read_test.go b/submitqueue/client/history_read_test.go index 5c5a16589..b9a8f7b59 100644 --- a/submitqueue/client/history_read_test.go +++ b/submitqueue/client/history_read_test.go @@ -44,6 +44,35 @@ func (f historySourceFunc) GetRequestHistoryByID( return f(ctx, request) } +func TestHistoryLateEventsPreserveTerminalOutcome(t *testing.T) { + for _, terminal := range []string{"landed", "cancelled", "error"} { + for _, late := range []*pb.HistoryEvent{{Status: "cancelling"}, {Event: "built"}} { + t.Run(terminal+"/"+late.Status+late.Event, func(t *testing.T) { + note := "" + if terminal == "error" { + note = "merge conflict" + } + row := &Row{SQID: "42"} + tracker := NewTracker([]*Row{row}) + reader := historyReaderFunc(func(context.Context, string, string) ([]*pb.HistoryEvent, error) { + return []*pb.HistoryEvent{{Status: terminal, LastError: note}, late}, nil + }) + captureStdout(t, func() { + tracker.Seal() + require.NoError(t, tracker.refreshHistory(context.Background(), reader, "queue", true)) + }) + assert.Equal(t, terminal, row.Status) + assert.Equal(t, note, row.Note) + assert.True(t, row.Done) + assert.True(t, isClosed(tracker.Settled())) + if terminal == "landed" { + assert.NoError(t, summarize([]*Row{row})) + } + }) + } + } +} + func TestPollHistoryPermanentFailures(t *testing.T) { for _, code := range []codes.Code{codes.PermissionDenied, codes.Unauthenticated, codes.InvalidArgument} { t.Run(code.String(), func(t *testing.T) { diff --git a/submitqueue/client/view.go b/submitqueue/client/view.go index 949be748f..8ba7c632f 100644 --- a/submitqueue/client/view.go +++ b/submitqueue/client/view.go @@ -148,8 +148,8 @@ func Draw(rows []*Row, status string) { } // digest reduces a request's recorded history to the trail worth showing, the -// status it currently holds, and the error the latest event carried. A status -// recorded more than once in a row is one step in the trail, not several. +// status it currently holds, and its diagnostic. Terminal outcomes take precedence +// over later nonterminal entries. Consecutive repeated statuses form one trail step. // // The history holds two kinds of entry. A status is a position the request // reached, and those are the trail's spine. An event is something that happened @@ -193,13 +193,17 @@ func digest(events []*pb.HistoryEvent) (trail []string, status, note string) { } for _, e := range events { - if e == nil { + if e == nil || (e.Status == "" && e.Event == "") { continue } - if e.Status == "" { - if e.Event == "" { - continue + // Late cancellation intents and background work cannot reopen a settled request. + if !terminalStatuses[status] || terminalStatuses[e.Status] { + if e.Status != "" { + status = e.Status } + note = e.LastError + } + if e.Status == "" { if counts[e.Event] == 0 { pending = append(pending, e.Event) } @@ -217,13 +221,6 @@ func digest(events []*pb.HistoryEvent) (trail []string, status, note string) { } flush() - if last := events[len(events)-1]; last != nil { - status, note = last.Status, last.LastError - } - // The last entry may be an event, which leaves the request where it was. - if status == "" { - status = currentStatus(events) - } return trail, status, note } @@ -241,17 +238,6 @@ func annotate(order []string, counts map[string]int) []string { return out } -// currentStatus is the last position the request reached, ignoring anything -// recorded while it sat there. -func currentStatus(events []*pb.HistoryEvent) string { - for i := len(events) - 1; i >= 0; i-- { - if e := events[i]; e != nil && e.Status != "" { - return e.Status - } - } - return "" -} - // outcome is the one-line verdict shown under the finished table. func outcome(rows []*Row) string { landed := 0 diff --git a/submitqueue/client/view_test.go b/submitqueue/client/view_test.go index aa1491959..994c4b7d8 100644 --- a/submitqueue/client/view_test.go +++ b/submitqueue/client/view_test.go @@ -169,6 +169,34 @@ func TestDigest(t *testing.T) { wantStatus: "speculating", wantNote: "runner unreachable", }, + { + name: "a late cancellation stays in the trail without reopening a landed request", + events: []*pb.HistoryEvent{ + {Status: "landed"}, {Status: "cancelling"}, + }, + wantTrail: []string{"landed", "cancelling"}, + wantStatus: "landed", + }, + { + name: "background work cannot replace a terminal failure reason", + events: []*pb.HistoryEvent{ + {Status: "error", LastError: "merge conflict"}, + {Event: "built", LastError: "unused build failed"}, + }, + wantTrail: []string{"error [after: built]"}, + wantStatus: "error", + wantNote: "merge conflict", + }, + { + name: "a repeated terminal status updates its diagnostic", + events: []*pb.HistoryEvent{ + {Status: "error", LastError: "failed"}, {Status: "cancelling"}, + {Status: "error", LastError: "merge conflict"}, + }, + wantTrail: []string{"error", "cancelling", "error"}, + wantStatus: "error", + wantNote: "merge conflict", + }, } for _, tt := range tests {