From f29a2e99c63db3ea6dc11b0b12b6921cf278f712 Mon Sep 17 00:00:00 2001 From: Victor Garcia Date: Mon, 28 Sep 2026 00:22:14 -0600 Subject: [PATCH] feat(worker): keep publish history across publish-only retries (U8) withPublishSummary moves a summary it replaces into publish_history (at most the last five, oldest first) so a retry no longer erases the stop code, failures, and repair rounds of the run before it (R15). History is the first thing dropped to stay within MaxResultBytes, oldest entry first. A first publish, replacing the "not_attempted" string, adds none. Co-Authored-By: Claude Opus 5.5 --- internal/worker/publish.go | 37 ++++++++-- internal/worker/publish_ci_test.go | 109 +++++++++++++++++++++++++++++ 2 files changed, 142 insertions(+), 4 deletions(-) diff --git a/internal/worker/publish.go b/internal/worker/publish.go index 02cfdcf..7bf1498 100644 --- a/internal/worker/publish.go +++ b/internal/worker/publish.go @@ -983,9 +983,16 @@ func publishBody(target publishTarget, head string, changedPaths []string) strin return body.String() } +// maxPublishHistory bounds publish_history: the summaries publish-only +// retries replaced, oldest first. +const maxPublishHistory = 5 + // withPublishSummary replaces the engine's "publish": "not_attempted" marker -// with what publish actually did. The size discipline is the engine's: cut -// inputs, never serialized bytes, so the document always parses. +// with what publish actually did. A summary it replaces (a retry's +// predecessor) moves to publish_history first, so the stop codes and rounds +// an attempt went through outlive the retry that followed them (R15). The +// size discipline is the engine's: cut inputs, never serialized bytes, so the +// document always parses. func withPublishSummary(result string, summary PublishSummary) string { document := map[string]any{} if strings.TrimSpace(result) != "" { @@ -995,9 +1002,31 @@ func withPublishSummary(result string, summary PublishSummary) string { } } } + history, _ := document["publish_history"].([]any) + if previous, ok := document["publish"].(map[string]any); ok { + // Only a summary object is history: the engine's "not_attempted" + // string is the absence of one. + history = append(history, previous) + } + if len(history) > maxPublishHistory { + history = history[len(history)-maxPublishHistory:] + } document["publish"] = summary - if body, err := json.Marshal(document); err == nil && len(body) <= protocol.MaxResultBytes { - return string(body) + for { + if len(history) == 0 { + delete(document, "publish_history") + } else { + document["publish_history"] = history + } + if body, err := json.Marshal(document); err == nil && len(body) <= protocol.MaxResultBytes { + return string(body) + } + if len(history) == 0 { + break + } + // History gives first, oldest entry first: the phase detail and the + // current verdict stay as long as they can. + history = history[1:] } // The phase envelopes are the heavy part; the verdict and the proof are // what a retry and an operator need. diff --git a/internal/worker/publish_ci_test.go b/internal/worker/publish_ci_test.go index 0db8e7f..5606e36 100644 --- a/internal/worker/publish_ci_test.go +++ b/internal/worker/publish_ci_test.go @@ -7,6 +7,7 @@ package worker import ( "context" + "encoding/json" "errors" "fmt" "os" @@ -197,6 +198,114 @@ func TestRedCIEndsAcceptedUnpublishedAndTheRetryJudgesTheCurrentHead(t *testing. if created, _ := gateway.counts(); created != 1 { t.Fatalf("gateway created %d pull requests, want 1", created) } + if history := publishHistoryOf(t, attempt.Result); len(history) != 0 { + t.Fatalf("first publish history = %+v, want none", history) + } + history := publishHistoryOf(t, retried.Result) + if len(history) != 1 || history[0].Code != "ci_failed" || history[0].CIRef != redSHA || + len(history[0].CIFailures) != 1 || history[0].CIFailures[0].Name != "test" { + t.Fatalf("retry history = %+v, want the red run's ci_failed with its failing test check", history) + } +} + +// publishHistoryOf decodes the summaries publish-only retries replaced. +func publishHistoryOf(t *testing.T, result string) []PublishSummary { + t.Helper() + var document struct { + History []PublishSummary `json:"publish_history"` + } + if err := json.Unmarshal([]byte(result), &document); err != nil { + t.Fatalf("decode publish history from %q: %v", result, err) + } + return document.History +} + +// ---- publish history across retries (R15) ----------------------------------- + +// A repair-exhausted attempt that a person fixes and retries is accepted, +// and its history still carries the stop code and the rounds it ran. +func TestARetryAfterExhaustedRepairKeepsTheRoundsInHistory(t *testing.T) { + s := newRepairScenario(t, 1, 2, fixRound()) + attempt, summary := s.run(t) + if summary.Code != "ci_repair_exhausted" { + t.Fatalf("summary = %+v, want ci_repair_exhausted", summary) + } + pushToBranch(t, s.originDir, s.branch) + retried, err := s.w.RetryPublish(context.Background(), s.job.ID, s.gateway, fastCIOptions()) + if err != nil { + t.Fatalf("publish retry: %v", err) + } + if retried.ID != attempt.ID || retried.State != protocol.AttemptAccepted { + t.Fatalf("retried = %s %s, want attempt %s accepted", retried.ID, retried.State, attempt.ID) + } + history := publishHistoryOf(t, retried.Result) + if len(history) != 1 || history[0].Code != "ci_repair_exhausted" || + len(history[0].CIRepairs) != 1 || history[0].CIRepairs[0].Outcome != "pushed" { + t.Fatalf("history = %+v, want the exhausted summary with its one pushed round", history) + } +} + +// historyCodes lists the codes publish_history holds, oldest first. +func historyCodes(t *testing.T, result string) []string { + t.Helper() + var codes []string + for _, entry := range publishHistoryOf(t, result) { + codes = append(codes, entry.Code) + } + return codes +} + +// Each retry appends the summary it replaces; only the newest five stay. +func TestPublishHistoryKeepsTheLastFiveRetries(t *testing.T) { + result := withPublishSummary(`{"publish":"not_attempted"}`, + PublishSummary{State: PublishStateFailed, Code: "run-0"}) + if codes := historyCodes(t, result); len(codes) != 0 { + t.Fatalf("first publish history = %v, want none", codes) + } + for retry := 1; retry <= 6; retry++ { + result = withPublishSummary(result, + PublishSummary{State: PublishStateFailed, Code: fmt.Sprintf("run-%d", retry)}) + } + if codes := historyCodes(t, result); !slices.Equal(codes, + []string{"run-1", "run-2", "run-3", "run-4", "run-5"}) { + t.Fatalf("history = %v, want run-1..run-5 oldest first", codes) + } + if summary := publishSummaryOf(t, result); summary.Code != "run-6" { + t.Fatalf("publish = %+v, want the latest run", summary) + } +} + +// A history that would breach the result cap loses its oldest entries first, +// before the phase detail or the verdict give anything up. +func TestPublishHistoryOverTheCapDropsTheOldestFirst(t *testing.T) { + bulky := func(code string) PublishSummary { + return PublishSummary{State: PublishStateFailed, Code: code, + Detail: strings.Repeat("x", protocol.MaxResultBytes/4)} + } + result := withPublishSummary(`{"phases":{"build":"done"},"publish":"not_attempted"}`, bulky("run-0")) + for retry := 1; retry <= 5; retry++ { + result = withPublishSummary(result, bulky(fmt.Sprintf("run-%d", retry))) + if len(result) > protocol.MaxResultBytes { + t.Fatalf("retry %d result is %d bytes, over the cap", retry, len(result)) + } + } + codes := historyCodes(t, result) + if len(codes) == 0 || len(codes) >= 5 || codes[len(codes)-1] != "run-4" { + t.Fatalf("history = %v, want a newest-last suffix ending at run-4", codes) + } + if want := fmt.Sprintf("run-%d", 5-len(codes)); codes[0] != want { + t.Fatalf("history = %v, want it to start at %s", codes, want) + } + var document map[string]any + if err := json.Unmarshal([]byte(result), &document); err != nil { + t.Fatalf("decode: %v", err) + } + if document["phases"] == nil || document["truncated"] != nil { + t.Fatalf("result = %s, want the phase detail kept while history can give", result) + } + if publishSummaryOf(t, result).Code != "run-5" { + t.Fatal("the current verdict was not kept") + } } // ---- scenario: no CI configured ---------------------------------------------