diff --git a/labs/12-product-engineering-loop/boatstack-distribution/release-notes/2026-07-28-pr-phase-observation.md b/labs/12-product-engineering-loop/boatstack-distribution/release-notes/2026-07-28-pr-phase-observation.md new file mode 100644 index 000000000..3e6b079dc --- /dev/null +++ b/labs/12-product-engineering-loop/boatstack-distribution/release-notes/2026-07-28-pr-phase-observation.md @@ -0,0 +1,5 @@ +### Status now shows where your published PR actually stands + +After you publish, `next-status` and `recovery-status` observe the live pull request and report its position: checks still running, checks failing (with the failing check names), changes requested, a required review still owed, or clean and eligible to merge. Before this, a published feature reported only open/merged/closed, and you had to open GitHub to learn why an open PR was not moving. + +The new detail comes from the same single read-only GitHub lookup Boatstack already performed, and it is never stored: anything the observation cannot classify with certainty is reported as unknown and left for you, and your delivery records are written only when the PR reaches a terminal merged or closed state, exactly as before. If your `gh` version does not support the richer lookup, status falls back to the previous behavior unchanged. diff --git a/labs/12-product-engineering-loop/product-engineering-loop/next.go b/labs/12-product-engineering-loop/product-engineering-loop/next.go index 8e7ae9119..7c5430002 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/next.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/next.go @@ -27,6 +27,10 @@ type NextStatus struct { Reason string `json:"reason"` BlockingAmbiguity []string `json:"blocking_ambiguity,omitempty"` Lifecycle string `json:"lifecycle,omitempty"` + PRPhase string `json:"pr_phase,omitempty"` + PRReviewDecision string `json:"pr_review_decision,omitempty"` + PRMergeState string `json:"pr_merge_state,omitempty"` + PRFailingChecks []string `json:"pr_failing_checks,omitempty"` PRURL string `json:"pr_url,omitempty"` HeadBranch string `json:"head_branch,omitempty"` ParentDelivery string `json:"parent_delivery,omitempty"` @@ -147,13 +151,31 @@ func nextForPublished(repo string, state DeliveryState) NextStatus { TotalSlices: len(state.Slices), ObservedStage: "PUBLISHED", NextOperation: "none", Lifecycle: pr.Lifecycle, PRURL: pr.URL, HeadBranch: pr.Branch, ParentDelivery: state.ParentDelivery, + PRPhase: string(pr.Phase), PRReviewDecision: pr.ReviewDecision, + PRMergeState: pr.MergeState, PRFailingChecks: pr.FailingChecks, } switch pr.Lifecycle { case "PUBLISHED_MERGED": status.ObservedStage = "FEATURE_COMPLETE" status.Reason = fmt.Sprintf("The published PR for feature %q is merged.", state.Feature) case "PUBLISHED_OPEN": - status.Reason = fmt.Sprintf("Feature %q is published in an open PR; review and required checks may still produce a corrective delivery.", state.Feature) + // The observed PR phase sharpens the reason when it is known; the + // pre-phase sentence remains the fallback so a degraded observation + // reads exactly as it always did. + switch pr.Phase { + case PRPhaseChecksPending: + status.Reason = fmt.Sprintf("Feature %q is published; checks on its PR are still running.", state.Feature) + case PRPhaseChecksFailing: + status.Reason = fmt.Sprintf("Feature %q is published; %d PR check(s) are failing (%s).", state.Feature, pr.ChecksFailed, strings.Join(pr.FailingChecks, ", ")) + case PRPhaseChangesRequested: + status.Reason = fmt.Sprintf("Feature %q is published; its PR review requested changes.", state.Feature) + case PRPhaseReviewRequired: + status.Reason = fmt.Sprintf("Feature %q is published; its PR checks pass and a required review approval is still owed.", state.Feature) + case PRPhaseMergeEligible: + status.Reason = fmt.Sprintf("Feature %q is published; its PR has passing checks, satisfied reviews, and a clean merge state.", state.Feature) + default: + status.Reason = fmt.Sprintf("Feature %q is published in an open PR; review and required checks may still produce a corrective delivery.", state.Feature) + } case "PUBLISHED_CLOSED": status.Reason = fmt.Sprintf("The PR for feature %q is closed without a verified merge; a future correction requires a fresh PR.", state.Feature) default: @@ -411,6 +433,15 @@ func FormatNextStatus(status NextStatus) string { if status.Lifecycle != "" { parts = append(parts, "Lifecycle: "+status.Lifecycle) } + // An Unknown phase adds nothing the lifecycle line does not already say, + // so only a positively derived phase earns a line. + if status.PRPhase != "" && status.PRPhase != string(PRPhaseUnknown) { + phase := "PR phase: " + status.PRPhase + if len(status.PRFailingChecks) > 0 { + phase += " (" + strings.Join(status.PRFailingChecks, ", ") + ")" + } + parts = append(parts, phase) + } if status.PRURL != "" { parts = append(parts, "PR: "+status.PRURL) } diff --git a/labs/12-product-engineering-loop/product-engineering-loop/pr_phase.go b/labs/12-product-engineering-loop/product-engineering-loop/pr_phase.go new file mode 100644 index 000000000..2bacbd772 --- /dev/null +++ b/labs/12-product-engineering-loop/product-engineering-loop/pr_phase.go @@ -0,0 +1,171 @@ +package boatstack + +import "strings" + +// PRPhase is the observed position of a published pull request between +// publication and merge. It is derived ONLY from a live GitHub observation +// (checks, review decision, merge state) at the moment of a read-only +// resolution; it is never persisted, recorded by an agent, or accepted from +// text. Anything the derivation cannot classify with certainty degrades to +// PRPhaseUnknown, which downstream classification treats as the operator's. +// control-law: pr-phase-derives-only-from-live-observation +type PRPhase string + +const ( + // PRPhaseUnknown: the observation is missing, partial, or names a + // combination this derivation does not understand. Fail-closed default. + PRPhaseUnknown PRPhase = "PR_UNKNOWN" + // PRPhaseChecksPending: the PR is open and at least one check has not finished. + PRPhaseChecksPending PRPhase = "PR_CHECKS_PENDING" + // PRPhaseChecksFailing: the PR is open and at least one check concluded badly. + PRPhaseChecksFailing PRPhase = "PR_CHECKS_FAILING" + // PRPhaseChangesRequested: a reviewer requested changes. This outranks check + // status: a human review verdict is a stronger signal than CI and hands the + // step to the operator regardless of what the checks are doing. + PRPhaseChangesRequested PRPhase = "PR_CHANGES_REQUESTED" + // PRPhaseReviewRequired: checks are green but a required review approval is + // still owed. Granting approval is never Boatstack's or the agent's to do. + PRPhaseReviewRequired PRPhase = "PR_REVIEW_REQUIRED" + // PRPhaseMergeEligible: checks green, review satisfied, and GitHub reports + // the branch cleanly mergeable. + PRPhaseMergeEligible PRPhase = "PR_MERGE_ELIGIBLE" + // PRPhaseMerged / PRPhaseClosed: terminal, mirrors the PR lifecycle. + PRPhaseMerged PRPhase = "PR_MERGED" + PRPhaseClosed PRPhase = "PR_CLOSED" +) + +// prStatusCheck is one element of gh's statusCheckRollup array. GitHub emits +// two shapes — CheckRun (Actions/checks API: status+conclusion+name) and +// StatusContext (legacy commit status: state+context) — and this struct holds +// the union so one decode covers both. +type prStatusCheck struct { + TypeName string `json:"__typename"` + Name string `json:"name"` + Status string `json:"status"` + Conclusion string `json:"conclusion"` + Context string `json:"context"` + State string `json:"state"` +} + +// prCheckSummary aggregates a statusCheckRollup. Unrecognized is sticky: one +// entry the tables below cannot classify poisons the whole summary, because a +// phase derived from a partially understood rollup would be a guess. +type prCheckSummary struct { + Total int + Passed int + Failed int + Pending int + Failing []string + Unrecognized bool +} + +// prFailingChecksCap bounds the failing-check name list carried into status +// output so one enormous check matrix cannot flood a rendered response. +const prFailingChecksCap = 8 + +func summarizeCheckRollup(entries []prStatusCheck) prCheckSummary { + summary := prCheckSummary{Total: len(entries)} + for _, entry := range entries { + name := strings.TrimSpace(entry.Name) + if name == "" { + name = strings.TrimSpace(entry.Context) + } + switch classifyStatusCheck(entry) { + case "passed": + summary.Passed++ + case "pending": + summary.Pending++ + case "failed": + summary.Failed++ + if name != "" && len(summary.Failing) < prFailingChecksCap { + summary.Failing = append(summary.Failing, name) + } + default: + summary.Unrecognized = true + } + } + return summary +} + +// classifyStatusCheck maps one rollup entry to passed/pending/failed, or "" +// when the entry's vocabulary is not in the tables. The typename is trusted +// first; when absent, the populated field set identifies the shape. +func classifyStatusCheck(entry prStatusCheck) string { + shape := strings.TrimSpace(entry.TypeName) + if shape == "" { + switch { + case entry.State != "" || entry.Context != "": + shape = "StatusContext" + case entry.Status != "" || entry.Conclusion != "": + shape = "CheckRun" + } + } + switch shape { + case "CheckRun": + if !strings.EqualFold(strings.TrimSpace(entry.Status), "COMPLETED") { + return "pending" + } + switch strings.ToUpper(strings.TrimSpace(entry.Conclusion)) { + case "SUCCESS", "NEUTRAL", "SKIPPED": + return "passed" + case "FAILURE", "TIMED_OUT", "CANCELLED", "ACTION_REQUIRED", "STARTUP_FAILURE", "STALE": + return "failed" + } + case "StatusContext": + switch strings.ToUpper(strings.TrimSpace(entry.State)) { + case "SUCCESS": + return "passed" + case "PENDING", "EXPECTED": + return "pending" + case "FAILURE", "ERROR": + return "failed" + } + } + return "" +} + +// derivePRPhase turns one live observation into a PRPhase. The derivation is +// pure and total: every input lands somewhere, and everything outside the +// explicitly understood combinations lands on PRPhaseUnknown. Notably absent +// on purpose: DIRTY/BEHIND/BLOCKED/DRAFT merge states (conflicts, stale base, +// branch protection this derivation cannot see, drafts) all stay Unknown so +// they reach the operator instead of being guessed at. +func derivePRPhase(prState string, checks prCheckSummary, reviewDecision, mergeState string) PRPhase { + switch strings.ToUpper(strings.TrimSpace(prState)) { + case "MERGED": + return PRPhaseMerged + case "CLOSED": + return PRPhaseClosed + case "OPEN": + default: + return PRPhaseUnknown + } + if checks.Unrecognized { + return PRPhaseUnknown + } + decision := strings.ToUpper(strings.TrimSpace(reviewDecision)) + if decision == "CHANGES_REQUESTED" { + return PRPhaseChangesRequested + } + if checks.Failed > 0 { + return PRPhaseChecksFailing + } + if checks.Pending > 0 { + return PRPhaseChecksPending + } + switch decision { + case "REVIEW_REQUIRED": + return PRPhaseReviewRequired + case "", "APPROVED": + default: + return PRPhaseUnknown + } + // An empty rollup means no checks are configured; green-by-absence is + // acceptable only because merge eligibility still requires GitHub itself + // to report the branch cleanly mergeable below. + switch strings.ToUpper(strings.TrimSpace(mergeState)) { + case "CLEAN", "HAS_HOOKS": + return PRPhaseMergeEligible + } + return PRPhaseUnknown +} diff --git a/labs/12-product-engineering-loop/product-engineering-loop/pr_phase_conformance_test.go b/labs/12-product-engineering-loop/product-engineering-loop/pr_phase_conformance_test.go new file mode 100644 index 000000000..6df714030 --- /dev/null +++ b/labs/12-product-engineering-loop/product-engineering-loop/pr_phase_conformance_test.go @@ -0,0 +1,232 @@ +package boatstack + +// control-law: pr-phase-derives-only-from-live-observation +// +// The post-publish PR phase (checks pending/failing, changes requested, +// review required, merge eligible, merged, closed) is derived exclusively +// from one live GitHub observation at read-only resolution time. It is never +// persisted, never accepted from an agent's text, and every observation the +// derivation does not understand with certainty degrades to PR_UNKNOWN. +// Companion law re-pinned here: gate resolution stays network-free — a +// non-terminal observation must leave the delivery ledger byte-identical +// (persistObservedTerminalPRState caches terminal lifecycles only). +// +// Test classes: positive (each understood observation → its phase, through +// the real ResolveNext path, over both statusCheckRollup shapes), negative +// (degraded/malformed/unrecognized observations → PR_UNKNOWN), bypass (a +// non-terminal observation writes nothing), failure-state (an older gh that +// rejects the enriched field list still yields the legacy lifecycle). + +import ( + "errors" + "fmt" + "os" + "strings" + "testing" +) + +const ( + rollupCheckRunPass = `{"__typename":"CheckRun","name":"unit","status":"COMPLETED","conclusion":"SUCCESS"}` + rollupCheckRunFail = `{"__typename":"CheckRun","name":"unit","status":"COMPLETED","conclusion":"FAILURE"}` + rollupCheckRunPending = `{"__typename":"CheckRun","name":"unit","status":"IN_PROGRESS","conclusion":""}` + rollupContextPass = `{"__typename":"StatusContext","context":"ci/lint","state":"SUCCESS"}` + rollupContextFail = `{"__typename":"StatusContext","context":"ci/lint","state":"FAILURE"}` + rollupContextPending = `{"__typename":"StatusContext","context":"ci/lint","state":"PENDING"}` + rollupUnrecognized = `{"__typename":"CheckRun","name":"novel","status":"COMPLETED","conclusion":"SOMETHING_NEW"}` +) + +func phaseObservationPayload(prState, reviewDecision, mergeState, rollup string) func(string, ...string) (string, error) { + return func(_ string, _ ...string) (string, error) { + return fmt.Sprintf( + `{"state":%q,"headRefName":"feat/phase","headRefOid":"head1","url":"https://example.invalid/pr/9","baseRefName":"main","mergeable":"MERGEABLE","mergeStateStatus":%q,"reviewDecision":%q,"statusCheckRollup":[%s]}`, + prState, mergeState, reviewDecision, rollup), nil + } +} + +func publishedPhaseRepo(t *testing.T) string { + t.Helper() + repo := nextTestRepo(t) + writeNextDelivery(t, repo, "phased", "PUBLISHED", 1) + updateRecoveryDelivery(t, repo, "phased", "feat/phase", "https://example.invalid/pr/9", "") + return repo +} + +// Positive: every understood live observation maps to exactly one phase, +// through the real ResolveNext path, over both rollup shapes. +func TestResolveNextDerivesPRPhaseFromLiveObservation(t *testing.T) { + for _, test := range []struct { + name string + prState string + reviewDecision string + mergeState string + rollup string + wantPhase PRPhase + wantStage string + reasonContains string + }{ + {"green_approved_clean_is_merge_eligible", "OPEN", "APPROVED", "CLEAN", rollupCheckRunPass + "," + rollupContextPass, PRPhaseMergeEligible, "PUBLISHED", "clean merge state"}, + {"no_required_review_green_clean_is_merge_eligible", "OPEN", "", "CLEAN", rollupCheckRunPass, PRPhaseMergeEligible, "PUBLISHED", "clean merge state"}, + {"no_checks_configured_green_by_absence", "OPEN", "APPROVED", "CLEAN", "", PRPhaseMergeEligible, "PUBLISHED", "clean merge state"}, + {"has_hooks_is_merge_eligible", "OPEN", "APPROVED", "HAS_HOOKS", rollupCheckRunPass, PRPhaseMergeEligible, "PUBLISHED", "clean merge state"}, + {"failing_check_run", "OPEN", "APPROVED", "CLEAN", rollupCheckRunFail + "," + rollupContextPass, PRPhaseChecksFailing, "PUBLISHED", "failing (unit)"}, + {"failing_status_context", "OPEN", "", "CLEAN", rollupCheckRunPass + "," + rollupContextFail, PRPhaseChecksFailing, "PUBLISHED", "failing (ci/lint)"}, + {"pending_check_run", "OPEN", "", "CLEAN", rollupCheckRunPending, PRPhaseChecksPending, "PUBLISHED", "still running"}, + {"pending_status_context", "OPEN", "", "CLEAN", rollupContextPending, PRPhaseChecksPending, "PUBLISHED", "still running"}, + {"review_required_after_green", "OPEN", "REVIEW_REQUIRED", "BLOCKED", rollupCheckRunPass, PRPhaseReviewRequired, "PUBLISHED", "required review approval"}, + {"changes_requested_outranks_failing_checks", "OPEN", "CHANGES_REQUESTED", "CLEAN", rollupCheckRunFail, PRPhaseChangesRequested, "PUBLISHED", "requested changes"}, + {"merged_pr_is_terminal", "MERGED", "", "", "", PRPhaseMerged, "FEATURE_COMPLETE", "is merged"}, + {"closed_pr_is_terminal", "CLOSED", "", "", "", PRPhaseClosed, "PUBLISHED", "closed without a verified merge"}, + } { + t.Run(test.name, func(t *testing.T) { + repo := publishedPhaseRepo(t) + withRecoveryGh(t, phaseObservationPayload(test.prState, test.reviewDecision, test.mergeState, test.rollup)) + status, err := ResolveNext(repo, "") + if err != nil { + t.Fatal(err) + } + if status.PRPhase != string(test.wantPhase) { + t.Fatalf("phase = %q, want %q (%#v)", status.PRPhase, test.wantPhase, status) + } + if status.ObservedStage != test.wantStage { + t.Fatalf("stage = %q, want %q", status.ObservedStage, test.wantStage) + } + if !strings.Contains(status.Reason, test.reasonContains) { + t.Fatalf("reason %q does not mention %q", status.Reason, test.reasonContains) + } + if status.NextOperation != "none" { + t.Fatalf("phase observation must not change the prescribed operation yet: %q", status.NextOperation) + } + }) + } +} + +// Positive: the failing-check names ride along for status output, bounded by +// the cap so a huge check matrix cannot flood a rendered response. +func TestFailingCheckNamesSurfaceBounded(t *testing.T) { + repo := publishedPhaseRepo(t) + entries := make([]string, 0, prFailingChecksCap+4) + for i := 0; i < prFailingChecksCap+4; i++ { + entries = append(entries, fmt.Sprintf(`{"__typename":"CheckRun","name":"job-%02d","status":"COMPLETED","conclusion":"FAILURE"}`, i)) + } + withRecoveryGh(t, phaseObservationPayload("OPEN", "", "CLEAN", strings.Join(entries, ","))) + status, err := ResolveNext(repo, "") + if err != nil { + t.Fatal(err) + } + if status.PRPhase != string(PRPhaseChecksFailing) { + t.Fatalf("phase = %q", status.PRPhase) + } + if len(status.PRFailingChecks) != prFailingChecksCap { + t.Fatalf("failing names = %d, want cap %d", len(status.PRFailingChecks), prFailingChecksCap) + } + if status.PRFailingChecks[0] != "job-00" { + t.Fatalf("unexpected first failing check: %v", status.PRFailingChecks) + } +} + +// Negative: degraded, malformed, or partially understood observations all +// land on PR_UNKNOWN and keep the pre-phase behavior intact. +func TestPRPhaseFailsClosedToUnknown(t *testing.T) { + for _, test := range []struct { + name string + gh func(string, ...string) (string, error) + wantLifecycle string + }{ + {"gh_unavailable", func(string, ...string) (string, error) { return "", errors.New("not authenticated") }, "PUBLISHED_UNKNOWN"}, + {"malformed_payload", func(string, ...string) (string, error) { return "not json", nil }, "PUBLISHED_UNKNOWN"}, + {"unrecognized_rollup_entry", phaseObservationPayload("OPEN", "APPROVED", "CLEAN", rollupUnrecognized), "PUBLISHED_OPEN"}, + {"unrecognized_review_decision", phaseObservationPayload("OPEN", "SOMETHING_NEW", "CLEAN", rollupCheckRunPass), "PUBLISHED_OPEN"}, + {"dirty_merge_state_is_not_guessed", phaseObservationPayload("OPEN", "APPROVED", "DIRTY", rollupCheckRunPass), "PUBLISHED_OPEN"}, + {"behind_merge_state_is_not_guessed", phaseObservationPayload("OPEN", "APPROVED", "BEHIND", rollupCheckRunPass), "PUBLISHED_OPEN"}, + {"draft_merge_state_is_not_guessed", phaseObservationPayload("OPEN", "", "DRAFT", ""), "PUBLISHED_OPEN"}, + } { + t.Run(test.name, func(t *testing.T) { + repo := publishedPhaseRepo(t) + withRecoveryGh(t, test.gh) + status, err := ResolveNext(repo, "") + if err != nil { + t.Fatal(err) + } + if status.PRPhase != string(PRPhaseUnknown) { + t.Fatalf("phase = %q, want PR_UNKNOWN", status.PRPhase) + } + if status.Lifecycle != test.wantLifecycle { + t.Fatalf("lifecycle = %q, want %q", status.Lifecycle, test.wantLifecycle) + } + if status.NextOperation != "none" { + t.Fatalf("unexpected operation %q", status.NextOperation) + } + if rendered := FormatNextStatus(status); strings.Contains(rendered, "PR phase:") { + t.Fatalf("an Unknown phase must not earn a rendered line:\n%s", rendered) + } + }) + } +} + +// Bypass: a non-terminal observation — however rich — must leave the delivery +// ledger byte-identical. Only a terminal lifecycle is cached, exactly as +// before the enrichment. +func TestNonTerminalPhaseObservationWritesNothing(t *testing.T) { + repo := publishedPhaseRepo(t) + statePath, err := deliveryStatePath(repo, "phased") + if err != nil { + t.Fatal(err) + } + before, err := os.ReadFile(statePath) + if err != nil { + t.Fatal(err) + } + withRecoveryGh(t, phaseObservationPayload("OPEN", "APPROVED", "CLEAN", rollupCheckRunFail)) + if _, err := ResolveNext(repo, ""); err != nil { + t.Fatal(err) + } + after, err := os.ReadFile(statePath) + if err != nil { + t.Fatal(err) + } + if string(before) != string(after) { + t.Fatal("a non-terminal observation modified the delivery ledger") + } + + // Relation: the terminal cache write still happens after the enrichment. + withRecoveryGh(t, phaseObservationPayload("MERGED", "", "", "")) + if _, err := ResolveNext(repo, ""); err != nil { + t.Fatal(err) + } + state, err := LoadDeliveryState(repo, "phased") + if err != nil { + t.Fatal(err) + } + if state.Slices[len(state.Slices)-1].PRState != "PUBLISHED_MERGED" { + t.Fatalf("terminal lifecycle was not cached: %#v", state.Slices) + } +} + +// Failure-state: an older gh that rejects the enriched field list must not +// cost the basic lifecycle observation — the observer falls back to the +// legacy field list and the phase stays Unknown. +func TestObservationFallsBackToLegacyFieldsOnOlderGh(t *testing.T) { + repo := publishedPhaseRepo(t) + var requested []string + withRecoveryGh(t, func(_ string, args ...string) (string, error) { + fields := args[len(args)-1] + requested = append(requested, fields) + if strings.Contains(fields, "statusCheckRollup") { + return "", errors.New("unknown JSON field: statusCheckRollup") + } + return `{"state":"OPEN","headRefName":"feat/phase","headRefOid":"head1","url":"https://example.invalid/pr/9"}`, nil + }) + status, err := ResolveNext(repo, "") + if err != nil { + t.Fatal(err) + } + if status.Lifecycle != "PUBLISHED_OPEN" { + t.Fatalf("legacy lifecycle lost: %q", status.Lifecycle) + } + if status.PRPhase != string(PRPhaseUnknown) { + t.Fatalf("phase = %q, want PR_UNKNOWN on a legacy observation", status.PRPhase) + } + if len(requested) != 2 || requested[0] != publishedPRFields || requested[1] != publishedPRLegacyFields { + t.Fatalf("unexpected field negotiation: %v", requested) + } +} diff --git a/labs/12-product-engineering-loop/product-engineering-loop/recovery.go b/labs/12-product-engineering-loop/product-engineering-loop/recovery.go index 108f79b9a..56519790c 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/recovery.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/recovery.go @@ -24,6 +24,8 @@ type RecoveryStatus struct { Slice string `json:"slice,omitempty"` ParentDelivery string `json:"parent_delivery,omitempty"` Lifecycle string `json:"lifecycle,omitempty"` + PRPhase string `json:"pr_phase,omitempty"` + PRFailingChecks []string `json:"pr_failing_checks,omitempty"` PRURL string `json:"pr_url,omitempty"` HeadBranch string `json:"head_branch,omitempty"` ObservedPRHeadSHA string `json:"observed_pr_head_sha,omitempty"` @@ -49,8 +51,30 @@ type publishedPRObservation struct { URL string Branch string HeadSHA string + // Post-publish position, observed live and never persisted. Phase is the + // fail-closed classification; the remaining fields carry the raw facts it + // was derived from so status output can explain the classification. + // control-law: pr-phase-derives-only-from-live-observation + Phase PRPhase + BaseBranch string + ReviewDecision string + MergeState string + FailingChecks []string + ChecksTotal int + ChecksPassed int + ChecksFailed int + ChecksPending int } +// publishedPRFields is the field list for the single live PR observation. +// publishedPRLegacyFields is the pre-phase list kept as a fallback so an older +// gh binary that rejects the newer fields still yields the basic lifecycle +// observation it always did. +const ( + publishedPRFields = "state,headRefName,headRefOid,url,baseRefName,statusCheckRollup,mergeable,mergeStateStatus,reviewDecision" + publishedPRLegacyFields = "state,headRefName,headRefOid,url" +) + var recoveryGh = func(repo string, arguments ...string) (string, error) { return commandOutput(repo, "gh", arguments...) } @@ -173,7 +197,7 @@ func selectRecoveryDelivery(states []DeliveryState, explicitFeature, currentBran func observePublishedPR(repo string, state DeliveryState) publishedPRObservation { branch, _, prURL := deliveryBranchAndSlice(state) - observation := publishedPRObservation{Lifecycle: "PUBLISHED_UNKNOWN", URL: prURL, Branch: branch} + observation := publishedPRObservation{Lifecycle: "PUBLISHED_UNKNOWN", URL: prURL, Branch: branch, Phase: PRPhaseUnknown} target := prURL if target == "" { target = branch @@ -181,15 +205,26 @@ func observePublishedPR(repo string, state DeliveryState) publishedPRObservation if target == "" { return observation } - value, err := recoveryGh(repo, "pr", "view", target, "--json", "state,headRefName,headRefOid,url") + value, err := recoveryGh(repo, "pr", "view", target, "--json", publishedPRFields) if err != nil { - return observation + // An older gh may reject the phase fields; fall back to the legacy + // list so the lifecycle observation this function always produced is + // never lost to the enrichment. The phase stays Unknown. + value, err = recoveryGh(repo, "pr", "view", target, "--json", publishedPRLegacyFields) + if err != nil { + return observation + } } var payload struct { - State string `json:"state"` - HeadRefName string `json:"headRefName"` - HeadRefOID string `json:"headRefOid"` - URL string `json:"url"` + State string `json:"state"` + HeadRefName string `json:"headRefName"` + HeadRefOID string `json:"headRefOid"` + URL string `json:"url"` + BaseRefName string `json:"baseRefName"` + Mergeable string `json:"mergeable"` + MergeStateStatus string `json:"mergeStateStatus"` + ReviewDecision string `json:"reviewDecision"` + StatusCheckRollup []prStatusCheck `json:"statusCheckRollup"` } if DecodeJSON("inspect published PR", target, []byte(value), &payload) != nil { return observation @@ -209,6 +244,16 @@ func observePublishedPR(repo string, state DeliveryState) publishedPRObservation case "CLOSED": observation.Lifecycle = "PUBLISHED_CLOSED" } + checks := summarizeCheckRollup(payload.StatusCheckRollup) + observation.BaseBranch = payload.BaseRefName + observation.ReviewDecision = payload.ReviewDecision + observation.MergeState = payload.MergeStateStatus + observation.FailingChecks = checks.Failing + observation.ChecksTotal = checks.Total + observation.ChecksPassed = checks.Passed + observation.ChecksFailed = checks.Failed + observation.ChecksPending = checks.Pending + observation.Phase = derivePRPhase(payload.State, checks, payload.ReviewDecision, payload.MergeStateStatus) return observation } @@ -417,6 +462,8 @@ func ResolveRecovery(options RecoveryStatusOptions) (RecoveryStatus, error) { pr := observePublishedPR(repo, selected) persistObservedTerminalPRState(repo, selected, pr) status.Lifecycle = pr.Lifecycle + status.PRPhase = string(pr.Phase) + status.PRFailingChecks = pr.FailingChecks status.PRURL = pr.URL status.ObservedPRHeadSHA = pr.HeadSHA if pr.Branch != "" {