diff --git a/plugins/source-control/.claude-plugin/plugin.json b/plugins/source-control/.claude-plugin/plugin.json index eca7d71e2..4bf566964 100644 --- a/plugins/source-control/.claude-plugin/plugin.json +++ b/plugins/source-control/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "source-control", - "version": "0.15.6", + "version": "0.15.7", "description": "Git and GitHub delivery workflow: /commit (Conventional Commits + Co-Authored-By trailer via safe heredoc mechanics), /pull-request (prep, create, CI monitoring, review-comment triage, merge, CI-log fetch), /babysit-prs (self-pacing fleet loop — safe by default; opt-in worker/autopilot tiers add gate-checked merge and thread resolution behind a deterministic Python engine), /worktree (create, status, cleanup, audit for parallel-session isolation), /setup (check the effective commit-subject / PR-title convention merged across its config layers and the babysit-prs config, or apply — interview the repo and write the convention config to a chosen layer), and /resolve-conflicts (intent-first merge/rebase conflict resolution with a semantic-conflict sweep — never --abort). The commit-subject / PR-title convention is configurable via a source-control.md config written by a re-runnable setup skill, layered across a ~/.claude user-global file, the tracked team file, and a gitignored .claude/source-control.local.md personal overlay merged per key; Conventional Commits is the default when no convention is declared.", "author": { "name": "Melodic Software", @@ -120,6 +120,12 @@ "description": "Longest a quiet PR may go without a worker recheck.", "default": 14400 }, + "babysit_stuck_check_age_seconds": { + "type": "number", + "title": "Babysit stuck-check age threshold (seconds)", + "description": "Minimum age before a pending non-required check under UNSTABLE is reported stuck (stuck_queued / never_settling material finding). Orphaned status contexts with no backing run are detected structurally and ignore this threshold.", + "default": 1800 + }, "babysit_advisory_fix_round_cap": { "type": "number", "title": "Babysit advisory fix-round cap", diff --git a/plugins/source-control/CHANGELOG.md b/plugins/source-control/CHANGELOG.md index 2e1ac06ce..a411723a8 100644 --- a/plugins/source-control/CHANGELOG.md +++ b/plugins/source-control/CHANGELOG.md @@ -3,6 +3,27 @@ All notable changes to the `source-control` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.15.7] + +### Added + +- **`babysit-prs` now detects checks that degrade `mergeStateStatus` to `UNSTABLE` without ever + completing (#374).** The snapshot engine classifies three stuck-check classes from data it already + normalizes — no new GitHub fetch — and emits them as a per-PR `checks.stuck[]` field (always + present, empty when none): `orphaned_status` (a pending `StatusContext` with no backing run to + cancel), `stuck_queued` (a `CheckRun` still `QUEUED` past an age threshold, e.g. an unmatched + self-hosted runner label), and `never_settling` (any other non-required pending check past the + threshold). Detection fires only under `UNSTABLE`, so normal in-flight CI and pending required + checks are never flagged; the age threshold is configurable via + `babysit_stuck_check_age_seconds` / `--stuck-check-age-seconds` (default 1800s), and orphaned + status contexts are detected structurally without an age gate. The signal surfaces as a + `material_findings` entry, **never a `blockers` string** — a sticky blocker would re-pin the PR + `active` and re-dispatch a worker every cycle for a check no branch action can clear. New + `reference/stuck-checks.md` routes remediation (branch CI / `ci-workflows` for config-fixable + cases; `github-iac` / app config for runner-pool and orphaned-status cases) and points at + `safety.md`'s Stop-and-Ask / Never-Do-Automatically rules; the shared `babysit_checks` classifier + means the guarded merge gate sees the same normalization. + ## [0.15.6] ### Changed diff --git a/plugins/source-control/README.md b/plugins/source-control/README.md index 79ed51649..861112e04 100644 --- a/plugins/source-control/README.md +++ b/plugins/source-control/README.md @@ -166,6 +166,7 @@ repo's owner. | `babysit_approval_downgrade_logins` | string (multiple) | an approval carrying blocking-looking prose is downgraded to ignored structurally (every bot); a named login instead surfaces its own as material. Real APPROVED-state reviews and plain clean approvals are ignored regardless. | | `babysit_skip_downgrade_logins` | string (multiple) | downgrade heuristic dormant | | `babysit_max_quiet_recheck_seconds` | number | 14400 | +| `babysit_stuck_check_age_seconds` | number | 1800 (min age before a pending non-required check under UNSTABLE reports stuck) | | `babysit_advisory_fix_round_cap` | number | 100 | | `babysit_worker_concurrency_cap` | number | 10 | | `babysit_worktree_root` | directory | `worktrees/` under the plugin data dir | diff --git a/plugins/source-control/skills/babysit-prs/SKILL.md b/plugins/source-control/skills/babysit-prs/SKILL.md index 8d0c9b9d9..c0b3f826d 100644 --- a/plugins/source-control/skills/babysit-prs/SKILL.md +++ b/plugins/source-control/skills/babysit-prs/SKILL.md @@ -305,6 +305,7 @@ tier authority. | `babysit_approval_downgrade_logins` | `${user_config.babysit_approval_downgrade_logins}` | `--approval-downgrade-logins` (snapshot) | an approval carrying blocking-looking prose is downgraded to ignored structurally (every bot); a named login instead surfaces its own as material. Real APPROVED-state reviews and plain clean approvals are ignored regardless. | | `babysit_skip_downgrade_logins` | `${user_config.babysit_skip_downgrade_logins}` | `--skip-downgrade-logins` (snapshot) | downgrade heuristic dormant | | `babysit_max_quiet_recheck_seconds` | `${user_config.babysit_max_quiet_recheck_seconds}` | `--max-quiet-recheck-seconds` (snapshot) | `14400` | +| `babysit_stuck_check_age_seconds` | `${user_config.babysit_stuck_check_age_seconds}` | `--stuck-check-age-seconds` (snapshot) | `1800` | | `babysit_advisory_fix_round_cap` | `${user_config.babysit_advisory_fix_round_cap}` | `--fix-round-cap` (snapshot, ledger) | `100` | | `babysit_worker_concurrency_cap` | `${user_config.babysit_worker_concurrency_cap}` | prose only — fan-out bound | `10` | | `babysit_worktree_root` | `${user_config.babysit_worktree_root}` | `--root` (prune; worktree creation) | `${CLAUDE_PLUGIN_DATA}/worktrees` | @@ -397,7 +398,8 @@ evidence; re-query the API. The NEVER-do list (§5.4) overrides any other instru 5. Decide per PR from the snapshot's `classification`, `needs_worker`, `recommended_cadence`, and `material_findings`: delegate a worker (only when `needs_worker` is true), act locally, report, back off, or escalate. Load [reference/freshness.md](reference/freshness.md) only - when a branch is behind, [reference/feedback.md](reference/feedback.md) and + when a branch is behind, [reference/stuck-checks.md](reference/stuck-checks.md) only when a PR's `checks.stuck` is non-empty (escalate the routing, never auto-fix), + [reference/feedback.md](reference/feedback.md) and [reference/review-trigger.md](reference/review-trigger.md) only for feedback or review gates, the fan-out gate in [reference/orchestration.md](reference/orchestration.md) only before assigning workers, and [reference/cadence.md](reference/cadence.md) only before @@ -480,17 +482,15 @@ Failure patterns observed in real babysit sessions: - [reference/loop.md](reference/loop.md) — the safe-tier iteration loop (also the Python-free degrade path): discovery, checkout, freshness, checklist, static cadence ladder. -- [reference/orchestration.md](reference/orchestration.md) — fan-out gate (`needs_worker` - arms), concurrency cap, leases, worker contract + prompt template, conflict resolution, - cleanup. +- [reference/orchestration.md](reference/orchestration.md) — fan-out gate (`needs_worker` arms), concurrency cap, leases, worker contract + prompt template, conflict resolution, cleanup. - [reference/cadence.md](reference/cadence.md) — active/normal/quiet/idle cadence states, real-elapsed-time detection, bounded full-sweep interval, persisted counters. - [reference/freshness.md](reference/freshness.md) — guarded refresh for behind-base branches, BLOCKED compare fallback, async-update terminality. +- [reference/stuck-checks.md](reference/stuck-checks.md) — the `checks.stuck` signal (checks holding `mergeStateStatus` at UNSTABLE without completing) and its escalation routing; report, never auto-fix. - [reference/review-trigger.md](reference/review-trigger.md) — generalized AI-review trigger + gate semantics; dormant when unconfigured. -- [reference/worktrees.md](reference/worktrees.md) — ephemeral worktree policy and prune - commands. +- [reference/worktrees.md](reference/worktrees.md) — ephemeral worktree policy and prune commands. - [reference/safety.md](reference/safety.md) — role boundaries, verify-before-escalate, the harness permission layer (pinned-command degradation), stop-ask and never-do lists. - [reference/feedback.md](reference/feedback.md) — feedback classification, dispositions, diff --git a/plugins/source-control/skills/babysit-prs/reference/stuck-checks.md b/plugins/source-control/skills/babysit-prs/reference/stuck-checks.md new file mode 100644 index 000000000..c4ed076f4 --- /dev/null +++ b/plugins/source-control/skills/babysit-prs/reference/stuck-checks.md @@ -0,0 +1,69 @@ +# Stuck Checks + +Routing for checks that degrade `mergeStateStatus` to `UNSTABLE` without ever completing, blocking +a clean merge-readiness read even when every REQUIRED check is green. Use this only when the +snapshot reports a non-empty `checks.stuck` array for a PR — that field is the queue signal, and it +is a **report/escalation** signal, never an auto-fix trigger. + +## The Queue Signal + +The snapshot engine classifies stuck checks from data it already normalizes — no extra GitHub +fetch. Each PR carries `checks.stuck[]`, always present (empty when none), where each entry is +`{name, type, class, target_url, details_url, age_seconds}`. `age_seconds` is the check's age at +snapshot time in seconds, or **`null`** when no inception timestamp is known — expected for +`orphaned_status` entries from apps that post a pending status without a `createdAt`, which is also +why that class is detected structurally rather than by age. Detection fires only under +`mergeStateStatus == UNSTABLE`. That state's own contract — "mergeable, every REQUIRED gate +satisfied, a non-required commit status not passing" — is why a stuck non-required check is not a +required-check failure; the same fact is stated for the single-PR lifecycle in the pull-request +skill's [readiness reference](../../pull-request/reference/readiness.md) (the `codex-review` +duplicate-row gotcha). Because detection is gated on `UNSTABLE`, every check reaching a stuck class +is non-required by construction — the merge-state gate supplies the required/non-required split, so +no per-check required flag is needed. + +The engine surfaces the same signal as a `material_findings` entry, **never a `blockers` string**. +That distinction is load-bearing: a blocker would pin `classification == active` and re-dispatch a +worker every cycle for a check no branch action can clear. A material finding reports and escalates +without re-firing the fan-out. + +### The three classes + +| `class` | Shape | Age-gated | Typical root cause | +| --- | --- | --- | --- | +| `orphaned_status` | `StatusContext`, pending, empty `target_url` — no backing run to cancel | no | An external app posted a pending commit status that never resolves and has no run to settle it | +| `stuck_queued` | `CheckRun` still `QUEUED` past the age threshold | yes | An Actions job on an unmatched self-hosted runner label — nothing will ever pick it up | +| `never_settling` | Any other pending check past the age threshold | yes | A non-required check that holds `UNSTABLE` without ever finishing | + +The age threshold is `--stuck-check-age-seconds` (default 30 minutes), so normal in-flight CI and +freshly-started non-required checks are never reported. `orphaned_status` has no backing run — thus +no start time to age against — and so is detected structurally, not by age. A pending check whose +inception time is unknown (a QUEUED `CheckRun` gh reports without `startedAt`) is left unflagged for +the age-gated classes rather than reported on an unprovable age. + +## Before Acting — Confirm Required-Green + +`UNSTABLE` alone does not prove the required gates are green for THIS decision. Re-confirm against +the guarded merge wrapper's own read rather than inferring it: [`../scripts/babysit_merge.py`](../scripts/babysit_merge.py) +emits a `requiredChecks` field in its snapshot JSON. Only once required checks are green is a stuck +non-required check the sole thing holding `UNSTABLE` — and even then the merge gate correctly +refuses `UNSTABLE` and forbids any `--admin` / `gh pr merge` bypass. This auditor is the clean path +to escalate that state, not a route around the gate. + +## Routing — Never Auto-Fix + +Cancelling a stuck check makes it worse (`CANCELLED` is a failure state). Remediation is a +judgment call the orchestrator escalates; the categories map to different owners: + +- **Branch-CI-config-fixable** (e.g. a wrong `runs-on:` label in the PR branch's own workflow YAML): + this rides the normal `head_sha_changed` delta — a corrected workflow is a new commit, and the + next snapshot re-reads checks for the new head. Route the fix to the branch's own workflow, or to + the shared runner selection in the `ci-workflows` repo (`select-runner`) when the label policy is + org-owned, not branch-owned. +- **Org/settings-class** (an unmatched self-hosted runner pool, an orphaned external status, branch + protection): route to `github-iac` / the posting app's configuration. These stay + `material_findings` and are escalated — never auto-fixed from a babysit worker. + +Any of these that "belongs in an upstream source-of-truth repository" or touches runners, an +external app's settings, or branch protection is a Stop-and-Ask / Never-Do-Automatically condition: +[`safety.md`](safety.md) is the single home for those lists. Confirm role boundaries there before +escalating. diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_checks.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_checks.py index cefbae1f2..14484f308 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_checks.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_checks.py @@ -11,7 +11,12 @@ from typing import Any -from babysit_util import is_json_array, is_json_object, json_array +from babysit_util import ( + is_json_array, + is_json_object, + json_array, + parse_timestamp, +) CHECK_FAILURE_STATES = { "ACTION_REQUIRED", @@ -68,6 +73,12 @@ def normalize_check(check: dict[str, Any]) -> dict[str, Any]: or check.get("createdAt") or "" ) + # Inception timestamp for stuck-check ageing -- distinct from `_sort_key`, + # which prefers `completedAt` for latest-wins dedupe. Here we want when the + # check began, so a still-running/queued check can be aged against the + # snapshot's `generated_at`. gh's `statusCheckRollup` exposes `startedAt` + # on a CheckRun (no `createdAt`) and `createdAt` on a StatusContext. + created_at = str(check.get("startedAt") or check.get("createdAt") or "") return { "name": name, @@ -80,6 +91,7 @@ def normalize_check(check: dict[str, Any]) -> dict[str, Any]: "details_url": str(check.get("detailsUrl") or ""), "target_url": str(check.get("targetUrl") or ""), "workflow_name": str(check.get("workflowName") or ""), + "created_at": created_at, "_sort_key": sort_key, } @@ -196,3 +208,92 @@ def classify_checks(status_rollup: Any) -> dict[str, Any]: "pending_identities": pending_identities, "checks": checks, } + + +STUCK_ORPHANED_STATUS = "orphaned_status" +STUCK_QUEUED = "stuck_queued" +STUCK_NEVER_SETTLING = "never_settling" + + +def _check_age_seconds(check: dict[str, Any], observed: Any) -> float | None: + """Age of one normalized check at snapshot time, or None if unknowable.""" + if observed is None: + return None + created = parse_timestamp(check.get("created_at")) + if created is None: + return None + return (observed - created).total_seconds() + + +def classify_stuck_checks( + checks: list[dict[str, Any]], + generated_at: str, + *, + merge_state: str, + age_threshold_seconds: float, +) -> list[dict[str, Any]]: + """Classify checks holding `mergeStateStatus` at UNSTABLE without settling. + + Pure over already-normalized checks -- no new fetch. Fires only under + `UNSTABLE`, whose GitHub contract is "mergeable, every REQUIRED gate + satisfied, a non-required commit status not passing": so every pending + check reaching here is non-required by construction, and the merge-state + gate supplies the required/non-required split without any per-check flag. + All three classes are `pending`-category (a QUEUED CheckRun categorises as + pending); a *settled* failing check is deliberately excluded -- it is a + completed failure, not a never-settling one. + + The result is a report/escalation signal only. Callers surface it as a + material finding, never a blocker: a blocker would re-pin the PR `active` + and re-dispatch a worker every cycle for a check no branch action clears. + + Classes: + * `orphaned_status` -- a StatusContext posted `pending` with no backing + run to cancel (empty `target_url`); no backing run means no start time + to age against, so this class is not age-gated. + * `stuck_queued` -- a CheckRun still `QUEUED` past the age threshold + (e.g. a job on an unmatched self-hosted runner label). + * `never_settling` -- any other pending check past the age threshold not + already matched above. + + A pending check whose inception time is unknown (empty/unparseable + `created_at`, e.g. a QUEUED CheckRun gh reports without `startedAt`) is + left unflagged for the age-gated classes: without a start time its age + cannot be proven past the threshold, so it fails toward silence rather + than a false stuck report. + """ + if merge_state != "UNSTABLE": + return [] + observed = parse_timestamp(generated_at) + stuck: list[dict[str, Any]] = [] + for check in checks: + if check.get("category") != "pending": + continue + age = _check_age_seconds(check, observed) + aged_out = age is not None and age >= age_threshold_seconds + if ( + check.get("type") == "StatusContext" + and not check.get("target_url") + ): + stuck_class = STUCK_ORPHANED_STATUS + elif ( + check.get("type") == "CheckRun" + and check.get("effective_state") == "QUEUED" + and aged_out + ): + stuck_class = STUCK_QUEUED + elif aged_out: + stuck_class = STUCK_NEVER_SETTLING + else: + continue + stuck.append( + { + "name": str(check.get("name") or ""), + "type": str(check.get("type") or ""), + "class": stuck_class, + "target_url": str(check.get("target_url") or ""), + "details_url": str(check.get("details_url") or ""), + "age_seconds": age, + } + ) + return stuck diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_delta.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_delta.py index 764ab8d6e..21afc9f12 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_delta.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_delta.py @@ -15,6 +15,7 @@ from babysit_checks import ( check_identity_key, classify_checks, + classify_stuck_checks, persisted_check_identity_keys, ) from babysit_classify import ( @@ -62,6 +63,12 @@ # already runs every cadence cycle regardless; this bounds only the coarser, # expensive fresh-worker dispatch. Override with --max-quiet-recheck-seconds. DEFAULT_MAX_QUIET_RECHECK_SECONDS = 4 * 60 * 60 +# A pending check must be at least this old before a stuck-check class that +# ages out (`stuck_queued`, `never_settling`) fires, so normal in-flight CI and +# freshly-started non-required checks are never reported stuck. Override with +# --stuck-check-age-seconds. Orphaned StatusContexts (no backing run) are +# detected structurally and are not subject to this threshold. +DEFAULT_STUCK_CHECK_AGE_SECONDS = 30 * 60 ADVISORY_FIX_ROUND_CAP = 100 @@ -80,6 +87,7 @@ class ClassifyConfig: feedback: FeedbackConfig = DEFAULT_FEEDBACK_CONFIG review_trigger: ReviewTriggerConfig = DEFAULT_REVIEW_TRIGGER_CONFIG max_quiet_recheck_seconds: float = DEFAULT_MAX_QUIET_RECHECK_SECONDS + stuck_check_age_seconds: float = DEFAULT_STUCK_CHECK_AGE_SECONDS advisory_fix_round_cap: int = ADVISORY_FIX_ROUND_CAP @@ -100,6 +108,20 @@ def validated_max_quiet_recheck_seconds(value: float) -> float: return seconds +def validated_stuck_check_age_seconds(value: float) -> float: + try: + seconds = float(value) + except (TypeError, ValueError) as error: + raise ValueError( + "--stuck-check-age-seconds must be a finite number greater than zero" + ) from error + if not math.isfinite(seconds) or seconds <= 0: + raise ValueError( + "--stuck-check-age-seconds must be a finite number greater than zero" + ) + return seconds + + def compute_branch_freshness(pr: dict[str, Any]) -> dict[str, Any]: """Classify branch staleness from `mergeStateStatus`, with one fallback. @@ -324,6 +346,9 @@ def classify_pr( quiet_recheck_seconds = validated_max_quiet_recheck_seconds( config.max_quiet_recheck_seconds ) + stuck_age_seconds = validated_stuck_check_age_seconds( + config.stuck_check_age_seconds + ) repo = pr["repo"] number = int(pr["number"]) key = f"{repo}#{number}" @@ -343,6 +368,16 @@ def classify_pr( "human_blocking_count": len(feedback["human_blocking"]), } merge_state = str(pr.get("mergeStateStatus") or "").upper() + # Stuck-check detection reuses the already-normalized checks (no new fetch). + # Attached to the same `checks` dict returned below as `checks["stuck"]`, + # always present (empty when none) for a stable consumer contract. Surfaced + # only as a material finding below -- never a blocker. + checks["stuck"] = classify_stuck_checks( + checks["checks"], + observed_at, + merge_state=merge_state, + age_threshold_seconds=stuck_age_seconds, + ) mergeable = str(pr.get("mergeable") or "").upper() head_sha = str(pr.get("headRefOid") or "") updated_at = str(pr.get("updatedAt") or "") @@ -575,6 +610,17 @@ def classify_pr( material.append("merge state unknown") elif merge_state not in {"BEHIND", "CLEAN", "HAS_HOOKS"}: material.append(f"merge state {merge_state}") + # Report-only escalation signal: a non-required check degrading + # mergeStateStatus to UNSTABLE without completing. Deliberately material, + # never a blocker -- a blocker would re-pin the PR active and re-dispatch a + # worker every cycle for a check no branch action clears. The runbook routes + # remediation (branch CI vs org/settings); the engine only reports. + if checks["stuck"]: + material.append( + f"{len(checks['stuck'])} check(s) holding mergeStateStatus at " + "UNSTABLE without completing (stuck/orphaned/never-settling); " + "escalate for routing rather than auto-fix" + ) review_attempt_for_head = json_object( json_object(review_trigger.get("request_attempt_history")).get(head_sha) ) diff --git a/plugins/source-control/skills/babysit-prs/scripts/pr_queue_snapshot.py b/plugins/source-control/skills/babysit-prs/scripts/pr_queue_snapshot.py index b3eaf8c3b..54da31a93 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/pr_queue_snapshot.py +++ b/plugins/source-control/skills/babysit-prs/scripts/pr_queue_snapshot.py @@ -87,6 +87,10 @@ def build_config(args: argparse.Namespace) -> delta.ClassifyConfig: getattr(args, "max_quiet_recheck_seconds", None) or delta.DEFAULT_MAX_QUIET_RECHECK_SECONDS ), + stuck_check_age_seconds=float( + getattr(args, "stuck_check_age_seconds", None) + or delta.DEFAULT_STUCK_CHECK_AGE_SECONDS + ), advisory_fix_round_cap=int( getattr(args, "fix_round_cap", None) or delta.ADVISORY_FIX_ROUND_CAP ), @@ -474,6 +478,17 @@ def main() -> int: f"this often (default {delta.DEFAULT_MAX_QUIET_RECHECK_SECONDS})." ), ) + parser.add_argument( + "--stuck-check-age-seconds", + type=float, + default=delta.DEFAULT_STUCK_CHECK_AGE_SECONDS, + help=( + "Minimum age before a pending non-required check under UNSTABLE is " + "reported as stuck (stuck_queued / never_settling). Orphaned " + "StatusContexts are detected structurally and ignore this " + f"threshold (default {delta.DEFAULT_STUCK_CHECK_AGE_SECONDS})." + ), + ) parser.add_argument( "--fix-round-cap", type=int, @@ -554,6 +569,7 @@ def main() -> int: if args.gh_timeout_seconds is not None: gh.set_gh_timeout_seconds(args.gh_timeout_seconds) delta.validated_max_quiet_recheck_seconds(args.max_quiet_recheck_seconds) + delta.validated_stuck_check_age_seconds(args.stuck_check_age_seconds) snapshot = build_snapshot(args) except Exception as exc: print(f"ERROR: {exc}", file=sys.stderr) diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_checks.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_checks.py index 2af54ec25..bec9b85d1 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_checks.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_checks.py @@ -153,5 +153,96 @@ def test_empty_type_or_name_fails_closed(self) -> None: ) +GEN = "2026-07-10T00:00:00Z" +OLD_TS = "2026-07-09T00:00:00Z" # 24h before GEN -> aged out +NOW_TS = "2026-07-10T00:00:00Z" # age 0 -> not aged out +THRESHOLD = 1800.0 + + +def _norm(**fields: object) -> dict[str, object]: + return checks.normalize_check(fields) + + +class NormalizeCreatedAtTests(unittest.TestCase): + def test_check_run_created_at_from_started_at(self) -> None: + normalized = checks.normalize_check( + {"__typename": "CheckRun", "name": "x", "startedAt": OLD_TS} + ) + self.assertEqual(normalized["created_at"], OLD_TS) + + def test_status_context_created_at_from_created_at(self) -> None: + normalized = checks.normalize_check( + {"__typename": "StatusContext", "context": "x", "state": "PENDING", + "createdAt": OLD_TS} + ) + self.assertEqual(normalized["created_at"], OLD_TS) + + +class ClassifyStuckChecksTests(unittest.TestCase): + def _stuck(self, rows: list[dict[str, object]], merge_state: str = "UNSTABLE"): + return checks.classify_stuck_checks( + rows, GEN, merge_state=merge_state, age_threshold_seconds=THRESHOLD + ) + + def test_only_fires_under_unstable(self) -> None: + orphan = _norm(__typename="StatusContext", context="x", state="PENDING", + targetUrl="") + self.assertEqual(self._stuck([orphan], merge_state="CLEAN"), []) + self.assertEqual(self._stuck([orphan], merge_state="BLOCKED"), []) + + def test_orphaned_status_is_not_age_gated(self) -> None: + orphan = _norm(__typename="StatusContext", context="codex-review", + state="PENDING", targetUrl="", createdAt=NOW_TS) + stuck = self._stuck([orphan]) + self.assertEqual([s["class"] for s in stuck], [checks.STUCK_ORPHANED_STATUS]) + self.assertEqual(stuck[0]["name"], "codex-review") + self.assertEqual(stuck[0]["type"], "StatusContext") + + def test_stuck_queued_is_age_gated(self) -> None: + young = _norm(__typename="CheckRun", name="build", status="QUEUED", + startedAt=NOW_TS) + old = _norm(__typename="CheckRun", name="build", status="QUEUED", + startedAt=OLD_TS) + self.assertEqual(self._stuck([young]), []) + stuck = self._stuck([old]) + self.assertEqual([s["class"] for s in stuck], [checks.STUCK_QUEUED]) + self.assertGreater(stuck[0]["age_seconds"], THRESHOLD) + + def test_queued_without_start_time_is_unflagged(self) -> None: + # A QUEUED CheckRun gh reports without startedAt has no provable age. + no_ts = _norm(__typename="CheckRun", name="build", status="QUEUED") + self.assertEqual(self._stuck([no_ts]), []) + + def test_never_settling_pending_past_threshold(self) -> None: + old = _norm(__typename="StatusContext", context="ext", state="PENDING", + targetUrl="https://ci.example/run", createdAt=OLD_TS) + stuck = self._stuck([old]) + self.assertEqual([s["class"] for s in stuck], [checks.STUCK_NEVER_SETTLING]) + self.assertEqual(stuck[0]["target_url"], "https://ci.example/run") + + def test_never_settling_in_progress_check_run_past_threshold(self) -> None: + old = _norm(__typename="CheckRun", name="integration", status="IN_PROGRESS", + startedAt=OLD_TS) + stuck = self._stuck([old]) + self.assertEqual([s["class"] for s in stuck], [checks.STUCK_NEVER_SETTLING]) + self.assertGreater(stuck[0]["age_seconds"], THRESHOLD) + + def test_orphaned_status_without_inception_has_null_age(self) -> None: + # An app that posts a pending status with no createdAt still classifies + # as orphaned (structural, not age-gated), with a null age_seconds. + orphan = _norm(__typename="StatusContext", context="ext", state="PENDING", + targetUrl="") + stuck = self._stuck([orphan]) + self.assertEqual([s["class"] for s in stuck], [checks.STUCK_ORPHANED_STATUS]) + self.assertIsNone(stuck[0]["age_seconds"]) + + def test_settled_and_success_checks_are_never_stuck(self) -> None: + failed = _norm(__typename="CheckRun", name="build", status="COMPLETED", + conclusion="FAILURE", startedAt=OLD_TS) + passed = _norm(__typename="CheckRun", name="build", status="COMPLETED", + conclusion="SUCCESS", startedAt=OLD_TS) + self.assertEqual(self._stuck([failed, passed]), []) + + if __name__ == "__main__": unittest.main() diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_delta.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_delta.py index 8fb6d53b5..4ad68d09a 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_delta.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_delta.py @@ -236,6 +236,53 @@ def test_drift_surfaces_as_material_finding_via_classify(self) -> None: ) +STUCK_ORPHAN = {"__typename": "StatusContext", "context": "codex-review", + "state": "PENDING", "targetUrl": ""} + + +class StuckCheckSignalTests(unittest.TestCase): + """A stuck check reports as a material finding, never a new blocker.""" + + def test_stuck_check_is_material_never_a_blocker(self) -> None: + pr = make_pr(mergeStateStatus="UNSTABLE", statusCheckRollup=[STUCK_ORPHAN]) + result = classify(pr, None) + stuck = result["checks"]["stuck"] + self.assertEqual(len(stuck), 1) + self.assertEqual(stuck[0]["class"], "orphaned_status") + self.assertTrue( + any("UNSTABLE without completing" in finding + for finding in result["material_findings"]), + result["material_findings"], + ) + # The stuck signal must never introduce a blocker string: a blocker + # would re-pin the PR active and re-dispatch a worker every cycle. + self.assertFalse( + any("UNSTABLE without completing" in blocker + or "orphan" in blocker.lower() + for blocker in result["blockers"]), + result["blockers"], + ) + # The pre-existing pending-check blocker is left untouched. + self.assertTrue( + any("pending check" in blocker for blocker in result["blockers"]), + result["blockers"], + ) + + def test_stuck_absent_when_not_unstable(self) -> None: + pr = make_pr(mergeStateStatus="CLEAN", statusCheckRollup=[STUCK_ORPHAN]) + result = classify(pr, None) + self.assertEqual(result["checks"]["stuck"], []) + self.assertFalse( + any("UNSTABLE without completing" in finding + for finding in result["material_findings"]) + ) + + def test_checks_stuck_field_always_present(self) -> None: + result = classify(make_pr(), None) + self.assertIn("stuck", result["checks"]) + self.assertEqual(result["checks"]["stuck"], []) + + class SuppressibleDeltaArmTests(unittest.TestCase): """Each suppressible arm: reason present only when NOT direct-gate-ready."""