Merge pull request #13 from aoagents/feat/draft-pr-state
feat: handle draft PR lifecycle state
This commit is contained in:
commit
bbaf6650e3
|
|
@ -89,8 +89,9 @@ func ResolveProbeDecision(in ProbeInput) LifecycleDecision {
|
|||
}
|
||||
|
||||
// ResolveOpenPRDecision walks the PR pipeline ladder. CI failure dominates
|
||||
// everything, then requested changes, then the approval/merge states, then a
|
||||
// pending review, then a stalled (idle-beyond-threshold) PR, else plain open.
|
||||
// everything. Draft PRs then surface as draft and do not enter the review or
|
||||
// merge states. Open PRs continue through requested changes, approval/merge
|
||||
// states, pending review, stalled (idle-beyond-threshold), then plain open.
|
||||
func ResolveOpenPRDecision(in OpenPRInput) LifecycleDecision {
|
||||
// evidence is a stable, timestamp-free summary "<condition> #<num> <url>"
|
||||
// for logs/traceability; it folds in the PR identity inputs (Number/URL).
|
||||
|
|
@ -104,13 +105,17 @@ func ResolveOpenPRDecision(in OpenPRInput) LifecycleDecision {
|
|||
}
|
||||
return s
|
||||
}
|
||||
prState := domain.PROpen
|
||||
if in.Draft {
|
||||
prState = domain.PRDraft
|
||||
}
|
||||
base := func(status domain.SessionStatus, cond string, prReason domain.PRReason, ss domain.SessionState, sr domain.SessionReason) LifecycleDecision {
|
||||
return LifecycleDecision{
|
||||
Status: status,
|
||||
Evidence: evidence(cond),
|
||||
SessionState: ss,
|
||||
SessionReason: sr,
|
||||
PRState: domain.PROpen,
|
||||
PRState: prState,
|
||||
PRReason: prReason,
|
||||
}
|
||||
}
|
||||
|
|
@ -118,6 +123,8 @@ func ResolveOpenPRDecision(in OpenPRInput) LifecycleDecision {
|
|||
switch {
|
||||
case in.CIFailing:
|
||||
return base(domain.StatusCIFailed, "ci_failing", domain.PRReasonCIFailing, domain.SessionWorking, domain.ReasonFixingCI)
|
||||
case in.Draft:
|
||||
return base(domain.StatusDraft, "draft", domain.PRReasonInProgress, domain.SessionWorking, domain.ReasonPRCreated)
|
||||
case in.ChangesRequested:
|
||||
return base(domain.StatusChangesRequested, "changes_requested", domain.PRReasonChangesRequested, domain.SessionWorking, domain.ReasonResolvingReviewComments)
|
||||
case in.Mergeable:
|
||||
|
|
|
|||
|
|
@ -155,11 +155,12 @@ func TestResolveProbeDecision(t *testing.T) {
|
|||
|
||||
func TestResolveOpenPRDecision(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
in OpenPRInput
|
||||
wantStatus domain.SessionStatus
|
||||
wantPR domain.PRReason
|
||||
wantState domain.SessionState
|
||||
name string
|
||||
in OpenPRInput
|
||||
wantStatus domain.SessionStatus
|
||||
wantPR domain.PRReason
|
||||
wantPRState domain.PRState
|
||||
wantState domain.SessionState
|
||||
}{
|
||||
{
|
||||
name: "ci failing dominates everything",
|
||||
|
|
@ -168,6 +169,22 @@ func TestResolveOpenPRDecision(t *testing.T) {
|
|||
wantPR: domain.PRReasonCIFailing,
|
||||
wantState: domain.SessionWorking,
|
||||
},
|
||||
{
|
||||
name: "draft with failing CI maps to ci_failed",
|
||||
in: OpenPRInput{Draft: true, CIFailing: true, ChangesRequested: true, Approved: true, Mergeable: true},
|
||||
wantStatus: domain.StatusCIFailed,
|
||||
wantPR: domain.PRReasonCIFailing,
|
||||
wantPRState: domain.PRDraft,
|
||||
wantState: domain.SessionWorking,
|
||||
},
|
||||
{
|
||||
name: "draft ignores review and merge states",
|
||||
in: OpenPRInput{Draft: true, ChangesRequested: true, Approved: true, Mergeable: true, ReviewPending: true, IdleBeyond: true},
|
||||
wantStatus: domain.StatusDraft,
|
||||
wantPR: domain.PRReasonInProgress,
|
||||
wantPRState: domain.PRDraft,
|
||||
wantState: domain.SessionWorking,
|
||||
},
|
||||
{
|
||||
name: "changes requested before approval states",
|
||||
in: OpenPRInput{ChangesRequested: true, Approved: true, Mergeable: true},
|
||||
|
|
@ -235,8 +252,12 @@ func TestResolveOpenPRDecision(t *testing.T) {
|
|||
if got.PRReason != tt.wantPR {
|
||||
t.Errorf("PRReason = %q, want %q", got.PRReason, tt.wantPR)
|
||||
}
|
||||
if got.PRState != domain.PROpen {
|
||||
t.Errorf("PRState = %q, want %q", got.PRState, domain.PROpen)
|
||||
wantPRState := tt.wantPRState
|
||||
if wantPRState == "" {
|
||||
wantPRState = domain.PROpen
|
||||
}
|
||||
if got.PRState != wantPRState {
|
||||
t.Errorf("PRState = %q, want %q", got.PRState, wantPRState)
|
||||
}
|
||||
if got.SessionState != tt.wantState {
|
||||
t.Errorf("SessionState = %q, want %q", got.SessionState, tt.wantState)
|
||||
|
|
@ -287,6 +308,8 @@ func TestDecidersDeriveConsistently(t *testing.T) {
|
|||
var decisions []LifecycleDecision
|
||||
|
||||
for _, in := range []OpenPRInput{
|
||||
{Draft: true, CIFailing: true},
|
||||
{Draft: true, ChangesRequested: true, Approved: true, Mergeable: true, ReviewPending: true, IdleBeyond: true},
|
||||
{CIFailing: true},
|
||||
{ChangesRequested: true},
|
||||
{Approved: true, Mergeable: true},
|
||||
|
|
@ -363,6 +386,13 @@ func TestResolveTerminalPRStateDecision(t *testing.T) {
|
|||
wantState: domain.SessionWorking,
|
||||
wantReason: domain.ReasonTaskInProgress,
|
||||
},
|
||||
{
|
||||
name: "non-terminal draft is a working no-op",
|
||||
pr: domain.PRDraft,
|
||||
wantStatus: domain.StatusWorking,
|
||||
wantState: domain.SessionWorking,
|
||||
wantReason: domain.ReasonTaskInProgress,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
|
|
|
|||
|
|
@ -52,8 +52,9 @@ const (
|
|||
ProcessIndeterminate ProcessLiveness = "indeterminate"
|
||||
)
|
||||
|
||||
// OpenPRInput drives the PR pipeline ladder for an open PR.
|
||||
// OpenPRInput drives the PR pipeline ladder for an open or draft PR.
|
||||
type OpenPRInput struct {
|
||||
Draft bool
|
||||
CIFailing bool
|
||||
ChangesRequested bool
|
||||
Approved bool
|
||||
|
|
|
|||
|
|
@ -90,6 +90,7 @@ type PRState string
|
|||
|
||||
const (
|
||||
PRNone PRState = "none"
|
||||
PRDraft PRState = "draft"
|
||||
PROpen PRState = "open"
|
||||
PRMerged PRState = "merged"
|
||||
PRClosed PRState = "closed"
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ const (
|
|||
StatusWorking SessionStatus = "working"
|
||||
StatusDetecting SessionStatus = "detecting"
|
||||
StatusPROpen SessionStatus = "pr_open"
|
||||
StatusDraft SessionStatus = "draft"
|
||||
StatusCIFailed SessionStatus = "ci_failed"
|
||||
StatusReviewPending SessionStatus = "review_pending"
|
||||
StatusChangesRequested SessionStatus = "changes_requested"
|
||||
|
|
@ -32,8 +33,9 @@ const (
|
|||
// 1. Terminal / hard session states (done, terminated, needs_input, stuck,
|
||||
// detecting, not_started) map directly — these OUTRANK PR facts.
|
||||
// 2. Otherwise a merged PR wins.
|
||||
// 3. Otherwise an open PR maps by its reason.
|
||||
// 4. Otherwise fall through to the SOFT session state (idle/working).
|
||||
// 3. Otherwise a draft PR maps to draft, except CI failure still dominates.
|
||||
// 4. Otherwise an open PR maps by its reason.
|
||||
// 5. Otherwise fall through to the SOFT session state (idle/working).
|
||||
//
|
||||
// So "PR facts dominate session facts" applies only to the soft states: an idle
|
||||
// or working session with an open, CI-failing PR displays as ci_failed — but a
|
||||
|
|
@ -59,6 +61,10 @@ func DeriveLegacyStatus(l CanonicalSessionLifecycle) SessionStatus {
|
|||
return StatusMerged
|
||||
}
|
||||
|
||||
if l.PR.State == PRDraft {
|
||||
return draftPRStatus(l.PR.Reason)
|
||||
}
|
||||
|
||||
if l.PR.State == PROpen {
|
||||
return openPRStatus(l.PR.Reason)
|
||||
}
|
||||
|
|
@ -82,6 +88,13 @@ func terminatedStatus(r SessionReason) SessionStatus {
|
|||
}
|
||||
}
|
||||
|
||||
func draftPRStatus(r PRReason) SessionStatus {
|
||||
if r == PRReasonCIFailing {
|
||||
return StatusCIFailed
|
||||
}
|
||||
return StatusDraft
|
||||
}
|
||||
|
||||
func openPRStatus(r PRReason) SessionStatus {
|
||||
switch r {
|
||||
case PRReasonCIFailing:
|
||||
|
|
|
|||
|
|
@ -49,6 +49,22 @@ func TestDeriveLegacyStatus(t *testing.T) {
|
|||
},
|
||||
want: StatusCIFailed,
|
||||
},
|
||||
{
|
||||
name: "draft PR with failing CI maps to ci_failed",
|
||||
in: CanonicalSessionLifecycle{
|
||||
Session: SessionSubstate{State: SessionWorking},
|
||||
PR: PRSubstate{State: PRDraft, Reason: PRReasonCIFailing},
|
||||
},
|
||||
want: StatusCIFailed,
|
||||
},
|
||||
{
|
||||
name: "draft PR ignores review and merge reasons",
|
||||
in: CanonicalSessionLifecycle{
|
||||
Session: SessionSubstate{State: SessionWorking},
|
||||
PR: PRSubstate{State: PRDraft, Reason: PRReasonMergeReady},
|
||||
},
|
||||
want: StatusDraft,
|
||||
},
|
||||
{
|
||||
name: "open PR approved",
|
||||
in: CanonicalSessionLifecycle{
|
||||
|
|
|
|||
|
|
@ -196,9 +196,9 @@ func (m *Manager) ApplyRuntimeObservation(ctx context.Context, id domain.Session
|
|||
}
|
||||
|
||||
// ApplySCMObservation maps PR facts onto the PR axis. A failed fetch is dropped
|
||||
// (failed probe != "no PR"). An open PR writes only the PR sub-state — the
|
||||
// session axis stays owned by activity, and DeriveLegacyStatus surfaces the PR
|
||||
// reason for display. A terminal PR (merged/closed) also parks the session.
|
||||
// (failed probe != "no PR"). An open or draft PR writes only the PR sub-state —
|
||||
// the session axis stays owned by activity, and DeriveLegacyStatus surfaces the
|
||||
// PR reason for display. A terminal PR (merged/closed) also parks the session.
|
||||
func (m *Manager) ApplySCMObservation(ctx context.Context, id domain.SessionID, f ports.SCMFacts) error {
|
||||
tr, err := m.mutate(ctx, id, func(cur domain.CanonicalSessionLifecycle, exists bool) (ports.LifecyclePatch, bool, error) {
|
||||
if !exists || !f.Fetched {
|
||||
|
|
@ -206,6 +206,14 @@ func (m *Manager) ApplySCMObservation(ctx context.Context, id domain.SessionID,
|
|||
}
|
||||
|
||||
switch f.PRState {
|
||||
case domain.PRDraft:
|
||||
in := openPRInput(f)
|
||||
in.Draft = true
|
||||
d := decide.ResolveOpenPRDecision(in)
|
||||
var patch ports.LifecyclePatch
|
||||
changed := setPRIfChanged(&patch, cur, d, f)
|
||||
return patch, changed, nil
|
||||
|
||||
case domain.PROpen:
|
||||
d := decide.ResolveOpenPRDecision(openPRInput(f))
|
||||
var patch ports.LifecyclePatch
|
||||
|
|
|
|||
|
|
@ -263,6 +263,38 @@ func TestApplySCMObservation(t *testing.T) {
|
|||
}
|
||||
})
|
||||
|
||||
t.Run("draft PR writes draft or ci_failed without review states", func(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
facts ports.SCMFacts
|
||||
wantReason domain.PRReason
|
||||
wantStatus domain.SessionStatus
|
||||
}{
|
||||
{"draft with failing CI", ports.SCMFacts{Fetched: true, PRState: domain.PRDraft, CISummary: ports.CIFailing}, domain.PRReasonCIFailing, domain.StatusCIFailed},
|
||||
{"draft ignores review and merge facts", ports.SCMFacts{Fetched: true, PRState: domain.PRDraft, ReviewDecision: ports.ReviewApproved, Mergeability: ports.Mergeability{Mergeable: true}}, domain.PRReasonInProgress, domain.StatusDraft},
|
||||
}
|
||||
for _, c := range cases {
|
||||
t.Run(c.name, func(t *testing.T) {
|
||||
mgr, store := newManager()
|
||||
wantSession := domain.SessionSubstate{State: domain.SessionWorking, Reason: domain.ReasonTaskInProgress}
|
||||
store.seed(sid, lc(wantSession.State, wantSession.Reason, domain.RuntimeAlive))
|
||||
if err := mgr.ApplySCMObservation(context.Background(), sid, c.facts); err != nil {
|
||||
t.Fatalf("apply: %v", err)
|
||||
}
|
||||
l := mustLoad(t, store)
|
||||
if l.PR.State != domain.PRDraft || l.PR.Reason != c.wantReason {
|
||||
t.Errorf("pr = %v/%v, want draft/%v", l.PR.State, l.PR.Reason, c.wantReason)
|
||||
}
|
||||
if l.Session != wantSession {
|
||||
t.Errorf("session = %+v, want untouched %+v", l.Session, wantSession)
|
||||
}
|
||||
if got := domain.DeriveLegacyStatus(l); got != c.wantStatus {
|
||||
t.Errorf("display = %v, want %v", got, c.wantStatus)
|
||||
}
|
||||
})
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("merged PR parks the session and displays merged", func(t *testing.T) {
|
||||
mgr, store := newManager()
|
||||
seed := lc(domain.SessionWorking, domain.ReasonTaskInProgress, domain.RuntimeAlive)
|
||||
|
|
@ -295,7 +327,8 @@ func TestApplySCMObservation(t *testing.T) {
|
|||
for _, c := range cases {
|
||||
t.Run(c.name, func(t *testing.T) {
|
||||
mgr, store := newManager()
|
||||
store.seed(sid, lc(domain.SessionWorking, domain.ReasonTaskInProgress, domain.RuntimeAlive))
|
||||
wantSession := domain.SessionSubstate{State: domain.SessionWorking, Reason: domain.ReasonTaskInProgress}
|
||||
store.seed(sid, lc(wantSession.State, wantSession.Reason, domain.RuntimeAlive))
|
||||
if err := mgr.ApplySCMObservation(context.Background(), sid, c.facts); err != nil {
|
||||
t.Fatalf("apply: %v", err)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -237,18 +237,23 @@ func (m *Manager) react(ctx context.Context, id domain.SessionID, tr *transition
|
|||
}
|
||||
|
||||
// incidentOver reports that a PR-pipeline incident has truly ended (PR no longer
|
||||
// open, or the session terminal), so all trackers for the session may reset.
|
||||
// active, or the session terminal), so all trackers for the session may reset.
|
||||
func incidentOver(l domain.CanonicalSessionLifecycle) bool {
|
||||
return l.PR.State != domain.PROpen || isTerminal(l.Session.State)
|
||||
return !isActivePRState(l.PR.State) || isTerminal(l.Session.State)
|
||||
}
|
||||
|
||||
func isActivePRState(s domain.PRState) bool {
|
||||
return s == domain.PROpen || s == domain.PRDraft
|
||||
}
|
||||
|
||||
// recovered reports a genuinely-green open PR: an approved/mergeable state, which
|
||||
// unambiguously means CI is no longer failing (the open-PR ladder ranks ci_failing
|
||||
// above approved, so an approved display cannot coexist with failing CI). Unlike
|
||||
// the ambiguous review_pending state — which may just be CI re-running — reaching
|
||||
// this ends a ci-failed incident and re-arms its budget.
|
||||
// this ends a ci-failed incident and re-arms its budget. Draft PRs are active,
|
||||
// but not recoverable via review/merge state.
|
||||
func recovered(l domain.CanonicalSessionLifecycle) bool {
|
||||
if l.PR.State != domain.PROpen {
|
||||
if !isActivePRState(l.PR.State) || l.PR.State == domain.PRDraft {
|
||||
return false
|
||||
}
|
||||
switch l.PR.Reason {
|
||||
|
|
|
|||
|
|
@ -211,6 +211,32 @@ func TestReaction_CIFailedNumericEscalation(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestReaction_DraftPRDoesNotEndCIFailedIncident(t *testing.T) {
|
||||
m, store, _, _ := newReactive()
|
||||
seed := lc(domain.SessionWorking, domain.ReasonTaskInProgress, domain.RuntimeAlive)
|
||||
seed.PR = domain.PRSubstate{State: domain.PRDraft, Reason: domain.PRReasonInProgress, Number: 7}
|
||||
store.seed(sid, seed)
|
||||
|
||||
tail := "fail"
|
||||
if err := m.ApplySCMObservation(ctx(), sid, ports.SCMFacts{
|
||||
Fetched: true, PRState: domain.PRDraft, CISummary: ports.CIFailing, PRNumber: 7, CIFailureLogTail: &tail,
|
||||
}); err != nil {
|
||||
t.Fatalf("draft fail: %v", err)
|
||||
}
|
||||
if sessionTrackerCount(m, sid) == 0 {
|
||||
t.Fatalf("precondition: expected a ci-failed tracker")
|
||||
}
|
||||
|
||||
if err := m.ApplySCMObservation(ctx(), sid, ports.SCMFacts{
|
||||
Fetched: true, PRState: domain.PRDraft, CISummary: ports.CIPending, PRNumber: 7,
|
||||
}); err != nil {
|
||||
t.Fatalf("draft pending: %v", err)
|
||||
}
|
||||
if n := sessionTrackerCount(m, sid); n == 0 {
|
||||
t.Errorf("draft PR is still active; ci-failed tracker should survive, got %d", n)
|
||||
}
|
||||
}
|
||||
|
||||
func TestReaction_DurationEscalationFiresOnTick(t *testing.T) {
|
||||
m, store, notf, msgr := newReactive()
|
||||
store.seed(sid, lcOpenPR(domain.PRReasonReviewPending))
|
||||
|
|
|
|||
Loading…
Reference in New Issue