diff --git a/backend/internal/domain/decide/decide.go b/backend/internal/domain/decide/decide.go index 586276329..e43ad7e40 100644 --- a/backend/internal/domain/decide/decide.go +++ b/backend/internal/domain/decide/decide.go @@ -166,7 +166,11 @@ func ResolveOpenPRDecision(in OpenPRInput) LifecycleDecision { return base(domain.StatusCIFailed, domain.PRReasonCIFailing, domain.SessionWorking, domain.ReasonFixingCI) case in.ChangesRequested: return base(domain.StatusChangesRequested, domain.PRReasonChangesRequested, domain.SessionWorking, domain.ReasonResolvingReviewComments) - case in.Approved && in.Mergeable: + case in.Mergeable: + // Mergeability is the authoritative merge gate, so it already folds in + // "approved if review is required". Checking it before Approved means a + // PR on a no-required-review repo (mergeable, not formally approved) is + // still surfaced as ready-to-merge instead of falling through to PR_OPEN. return base(domain.StatusMergeable, domain.PRReasonMergeReady, domain.SessionIdle, domain.ReasonAwaitingExternalReview) case in.Approved: return base(domain.StatusApproved, domain.PRReasonApproved, domain.SessionIdle, domain.ReasonAwaitingExternalReview) @@ -270,6 +274,11 @@ func HashEvidence(evidence string) string { // timestampPatterns strip the time-varying parts of an evidence string before // hashing. Order matters: the full datetime form is removed before the bare // time-of-day and epoch forms so they don't partially match. +// +// The epoch pattern strips any bare 10-13 digit run (unix seconds/millis). That +// is broad enough to also clobber a same-width numeric ID if one ever appears in +// an evidence string; evidence is decider-authored, so today nothing else lands +// in that range, but keep IDs out of evidence strings to preserve hash fidelity. var timestampPatterns = []*regexp.Regexp{ regexp.MustCompile(`\d{4}-\d{2}-\d{2}[T ]\d{2}:\d{2}:\d{2}(?:\.\d+)?(?:Z|[+-]\d{2}:?\d{2})?`), regexp.MustCompile(`\d{2}:\d{2}:\d{2}(?:\.\d+)?`), diff --git a/backend/internal/domain/decide/decide_test.go b/backend/internal/domain/decide/decide_test.go index 9d615cdba..d24497b22 100644 --- a/backend/internal/domain/decide/decide_test.go +++ b/backend/internal/domain/decide/decide_test.go @@ -182,6 +182,13 @@ func TestResolveOpenPRDecision(t *testing.T) { wantPR: domain.PRReasonMergeReady, wantState: domain.SessionIdle, }, + { + name: "mergeable without formal approval (no required review) -> mergeable", + in: OpenPRInput{Mergeable: true}, + wantStatus: domain.StatusMergeable, + wantPR: domain.PRReasonMergeReady, + wantState: domain.SessionIdle, + }, { name: "approved but not mergeable -> approved", in: OpenPRInput{Approved: true}, @@ -238,26 +245,50 @@ func TestResolveOpenPRDecision(t *testing.T) { } } -func TestResolveOpenPRDecisionDerivesConsistently(t *testing.T) { - // The display Status produced by the ladder must equal what DeriveLegacyStatus - // would produce from the same canonical (session, pr) it emits. - inputs := []OpenPRInput{ +func TestDecidersDeriveConsistently(t *testing.T) { + // Every decision a decider produces must be self-consistent: the display + // Status it reports must equal what DeriveLegacyStatus produces from the + // canonical (session, pr) sub-states it emits. This locks the deciders and + // the display-derivation against drifting apart. + // + // The ResolveTerminalPRStateDecision none/open default is intentionally + // excluded — it is a documented no-op for misuse, not a real verdict. + var decisions []LifecycleDecision + + for _, in := range []OpenPRInput{ {CIFailing: true}, {ChangesRequested: true}, {Approved: true, Mergeable: true}, + {Mergeable: true}, {Approved: true}, {ReviewPending: true}, {IdleBeyond: true}, {}, + } { + decisions = append(decisions, ResolveOpenPRDecision(in)) } - for _, in := range inputs { - d := ResolveOpenPRDecision(in) + + decisions = append(decisions, + ResolveTerminalPRStateDecision(domain.PRMerged), + ResolveTerminalPRStateDecision(domain.PRClosed), + ) + + for _, in := range []ProbeInput{ + {KillRequested: true, Now: t0}, + {Runtime: domain.RuntimeAlive, Process: ProcessAlive, Now: t0}, + {Runtime: domain.RuntimeMissing, Process: ProcessIndeterminate, Now: t0}, + {Runtime: domain.RuntimeExited, Process: ProcessDead, Now: t0}, + } { + decisions = append(decisions, ResolveProbeDecision(in)) + } + + for _, d := range decisions { l := domain.CanonicalSessionLifecycle{ Session: domain.SessionSubstate{State: d.SessionState, Reason: d.SessionReason}, PR: domain.PRSubstate{State: d.PRState, Reason: d.PRReason}, } if got := domain.DeriveLegacyStatus(l); got != d.Status { - t.Errorf("input %+v: decision Status=%q but DeriveLegacyStatus=%q", in, d.Status, got) + t.Errorf("decision %+v: Status=%q but DeriveLegacyStatus=%q", d, d.Status, got) } } }