diff --git a/skills/ce-babysit-pr/references/tick.md b/skills/ce-babysit-pr/references/tick.md index eb5911260..233958e64 100644 --- a/skills/ce-babysit-pr/references/tick.md +++ b/skills/ce-babysit-pr/references/tick.md @@ -37,9 +37,9 @@ Do **not** pass `--settle-seconds` or `--blocked-external-drain-seconds` on the **`` in `STATE_DIR` is load-bearing for GitHub Enterprise.** Derive it from the PR URL's host (or `gh repo view --json url`); use the same value in every `mark`. Keying only by `--` would let two PRs with the same `owner/repo#N` on *different* hosts (github.com + a GHE instance) share one `state.json`, so one host's dispositions/dispatched CI would silence or contaminate the other's actionable set. On plain github.com the host segment is just `github.com`. **Pass the same host in `--repo //`** (the documented `[HOST/]OWNER/REPO` selector) so `pr-snapshot`'s first `gh pr view` — which runs before it parses the URL host — queries the right host instead of the checkout's default `github.com`. -The snapshot emits the **attention set** — unresolved threads you have not yet acted on, **non-thread feedback candidates** (top-level PR comments + review-submission bodies) you have not yet classified, and failing checks on the current head you have not yet dispatched — plus the exact current `branch_currency` item and its `attention` route. It also emits `pr_state`, `mergeable`, `merge_state_status`, `base`, `base_ref_blocker`, `host_branch_update_capability`, `branch_currency_blocker`, `review_decision`, `head_sha`, `head_changed`, `quiet_seconds`, `invocation_elapsed_seconds`, `invocation_remaining_seconds`, `persisted_state_age_seconds`, `checks_awaiting_approval` / `blocked_external`, and the head-scoped `blocked_external_first_seen_at`, `blocked_external_review_last_activity_at`, `blocked_external_review_quiet_seconds`, and `blocked_external_review_moved_this_tick` review-drain facts (see Step 3), plus a `pr_chain` block and a `trajectory` block (cross-tick facts: `check_recur_max`, `recurring_checks`, `unresolved_trend`, `new_threads_this_tick`, `stream_alternations`, `heads_since_progress`). `base.historical_oid` is GitHub's historical `baseRefOid`; it is diagnostic and is not the current base tip. Current-base identity requires the independent exact Git ref (`base.oid`) to match the PR `baseRef.target.oid` (`base.graphql_oid`). For a mergeable result, the generated `potentialMergeCommit` must also name that current base and the observed PR head as its two parents. Only that proven binding emits `base.identity == "current"`; a base movement race emits `race`, temporary merge-commit generation emits `mergeability-pending`, and a failed or malformed probe emits `probe-error`. These transient blockers disable `mergeability_certain` and re-poll. A `DIRTY` / `CONFLICTING` result may omit `potentialMergeCommit`; matching current-base observations still make the conflict result usable. Invocation time and persisted-state age are separate; never report one as the other. `pr_chain` carries the two independent axes: `manager_status` (`confirmed|absent|probe-error`) and `relationship_status` (`dependent|independent|probe-error`), plus manager source, target/upstack freshness, ordered entries, and ordinary parent/dependent PRs when available. The JSON field remains `actionable.comments` for the claim→act→confirm protocol, but its members are candidates awaiting semantic classification, not detector-proven action items. For non-thread feedback, the deterministic fetch excludes only empty bodies — including the PR author's own comments, since an author asking for a change on their agent-opened PR is ordinary feedback; loop prevention is the `dispatched` mark, not identity. It does **not** decide from content, bot identity, or comment-vs-review surface whether an external message is valid feedback; `ce-resolve` applies that judgment. The snapshot **never** marks a surfaced item handled just from observing it; an item stays in the attention set until you confirm you acted or classified it (`mark`) or remote truth removes it (a resolved thread drops out of the fetch). Every `mark` write must present the same `RUN_INVOCATION_ID`, `RUN_STARTED_AT`, and `RUN_BUDGET_SECONDS`; a stale resolver tick must fail before it can silence work in a replacement invocation. So a crashed, failed, or superseded resolve pass leaves its items in the set next tick. Read `references/watch-loop.md` for the state schema and the claim→act→confirm protocol before acting. +The snapshot emits the **attention set** — unresolved threads you have not yet acted on, **non-thread feedback candidates** (top-level PR comments + review-submission bodies) you have not yet classified, and failing checks on the current head you have not yet dispatched — plus the exact current `branch_currency` item and its `attention` route. It also emits `pr_state`, `mergeable`, `merge_state_status`, `base`, `base_ref_blocker`, `host_branch_update_capability`, `branch_currency_blocker`, `review_decision`, `head_sha`, `head_changed`, `quiet_seconds`, `invocation_elapsed_seconds`, `invocation_remaining_seconds`, `persisted_state_age_seconds`, `checks_awaiting_approval` / `blocked_external`, and the head-scoped `blocked_external_first_seen_at`, `blocked_external_review_last_activity_at`, `blocked_external_review_quiet_seconds`, and `blocked_external_review_moved_this_tick` review-drain facts (see Step 3), plus a `pr_chain` block and a `trajectory` block (cross-tick facts: `check_recur_max`, `recurring_checks`, `unresolved_trend`, `new_threads_this_tick`, `stream_alternations`, `heads_since_progress`, `invariant_rounds`). `base.historical_oid` is GitHub's historical `baseRefOid`; it is diagnostic and is not the current base tip. Current-base identity requires the independent exact Git ref (`base.oid`) to match the PR `baseRef.target.oid` (`base.graphql_oid`). For a mergeable result, the generated `potentialMergeCommit` must also name that current base and the observed PR head as its two parents. Only that proven binding emits `base.identity == "current"`; a base movement race emits `race`, temporary merge-commit generation emits `mergeability-pending`, and a failed or malformed probe emits `probe-error`. These transient blockers disable `mergeability_certain` and re-poll. A `DIRTY` / `CONFLICTING` result may omit `potentialMergeCommit`; matching current-base observations still make the conflict result usable. Invocation time and persisted-state age are separate; never report one as the other. `pr_chain` carries the two independent axes: `manager_status` (`confirmed|absent|probe-error`) and `relationship_status` (`dependent|independent|probe-error`), plus manager source, target/upstack freshness, ordered entries, and ordinary parent/dependent PRs when available. The JSON field remains `actionable.comments` for the claim→act→confirm protocol, but its members are candidates awaiting semantic classification, not detector-proven action items. For non-thread feedback, the deterministic fetch excludes only empty bodies — including the PR author's own comments, since an author asking for a change on their agent-opened PR is ordinary feedback; loop prevention is the `dispatched` mark, not identity. It does **not** decide from content, bot identity, or comment-vs-review surface whether an external message is valid feedback; `ce-resolve` applies that judgment. The snapshot **never** marks a surfaced item handled just from observing it; an item stays in the attention set until you confirm you acted or classified it (`mark`) or remote truth removes it (a resolved thread drops out of the fetch). Every `mark` write must present the same `RUN_INVOCATION_ID`, `RUN_STARTED_AT`, and `RUN_BUDGET_SECONDS`; a stale resolver tick must fail before it can silence work in a replacement invocation. So a crashed, failed, or superseded resolve pass leaves its items in the set next tick. Read `references/watch-loop.md` for the state schema and the claim→act→confirm protocol before acting. -**The `trajectory` is facts, not a verdict — you hand it to the leaves, they judge convergence.** When it crosses a trigger (`check_recur_max >= 2`, `stream_alternations >= 3`, a rising `unresolved_trend` with `new_threads_this_tick > 0` across passes, or `heads_since_progress >= 2`), pass the trajectory to that tick's `ce-debug`/`ce-resolve-pr-feedback` invocation as **mandatory input** and let it decide whether this is ordinary progress or genuine non-convergence (a leaf may then return a `needs-human` residual that parks the *whole stream*, e.g. an emergent CI trade-off or a wrong-approach nitpick cluster). Never declare non-convergence yourself. Read `references/watch-loop.md` (**Non-convergence** section) for the trigger→route→park→re-open protocol before acting on it. +**The `trajectory` is facts, not a verdict — you hand it to the leaves, they judge convergence.** When it crosses a trigger (`check_recur_max >= 2`, `stream_alternations >= 3`, a rising `unresolved_trend` with `new_threads_this_tick > 0` across passes, `heads_since_progress >= 2`, or any `invariant_rounds[].rounds >= 2` — a key at 2 recorded rounds means the next fix would be its third), pass the trajectory to that tick's `ce-debug`/`ce-resolve-pr-feedback` invocation as **mandatory input** **before** that leaf mutates and let it decide whether this is ordinary progress or genuine non-convergence (a leaf may then return a `needs-human` residual that parks the *whole stream*, e.g. an emergent CI trade-off, a wrong-approach nitpick cluster, or a third invariant round). Never declare non-convergence yourself. Read `references/watch-loop.md` (**Non-convergence** section) for the trigger→route→park→re-open protocol before acting on it. ## Ordering invariant (full text) @@ -50,7 +50,7 @@ The snapshot emits the **attention set** — unresolved threads you have not yet **Managed-stack pre-push baseline.** Before invoking a delegate that may push the active target in a confirmed managed stack, record a recoverable baseline from a fresh `gh stack view --json`: the manager-ordered open branches at or above the target (target plus open dependents) and each branch's current remote-tracking OID on the tracking remote. Require a clean worktree and still-confirmed manager membership for the target/current branch. If either precondition fails, this is a true stop for the active invocation in every mode: do not invoke a delegate, run another tick, or arm/re-arm a watcher; state the residual and give the host-rendered resume invocation. Do not stop for missing atomic multi-ref push proof — current `gh stack push` may update branches non-atomically (`github/gh-stack#216`); prefer all-or-none when an installed manager later proves atomic push, but always re-probe after push rather than assuming it. -3. **Feedback before CI.** If the attention set has **either** unresolved threads **or** non-thread feedback candidates (`counts.threads > 0` or `counts.comments > 0`), invoke `ce-resolve-pr-feedback` **once**, passing the resolved PR ref — the base `[HOST/]OWNER/REPO#N` or the full PR URL from the snapshot's `url` (so a fork→upstream PR resolves against the **upstream base**, not the fork checkout's `origin`, which would query the wrong PR namespace) — in full mode **with `mode:pipeline`** (non-interactive: it parks any `needs-human` on the thread and returns it as a structured residual instead of pausing on a blocking user question, which would stall the autonomous watch — the same reason Step 2 step 5 invokes `ce-debug mode:pipeline`); it re-fetches and judges *all* feedback — inline threads, review bodies, and top-level comments — and is idempotent on empty. The `actionable.comments` field contains the top-level/review-body candidates the resolver would otherwise not know the loop cares about — a Changes-Requested review body or a bare top-level "please rename X" with **no inline thread** must still trigger a pass. **When the review trigger above is crossed (rising backlog, new-item arrivals, or a repeating cluster), pass the `trajectory`** so it can judge a treadmill / wrong-approach nitpick cluster and return one approach-level `needs-human` instead of fixing forever — **and, when the recurring items are *valid* and share one root and fix, request a bounded-class assessment** so it consolidates the equivalent sites this PR touched into a single fix rather than dripping one per head (`references/watch-loop.md`, Non-convergence). One resolve pass per tick — never fan out multiple. +3. **Feedback before CI.** If the attention set has **either** unresolved threads **or** non-thread feedback candidates (`counts.threads > 0` or `counts.comments > 0`), invoke `ce-resolve-pr-feedback` **once**, passing the resolved PR ref — the base `[HOST/]OWNER/REPO#N` or the full PR URL from the snapshot's `url` (so a fork→upstream PR resolves against the **upstream base**, not the fork checkout's `origin`, which would query the wrong PR namespace) — in full mode **with `mode:pipeline`** (non-interactive: it parks any `needs-human` on the thread and returns it as a structured residual instead of pausing on a blocking user question, which would stall the autonomous watch — the same reason Step 2 step 5 invokes `ce-debug mode:pipeline`); it re-fetches and judges *all* feedback — inline threads, review bodies, and top-level comments — and is idempotent on empty. The `actionable.comments` field contains the top-level/review-body candidates the resolver would otherwise not know the loop cares about — a Changes-Requested review body or a bare top-level "please rename X" with **no inline thread** must still trigger a pass. **When the review trigger above is crossed (rising backlog, new-item arrivals, a repeating cluster, or any `invariant_rounds[].rounds >= 2`), pass the `trajectory`** so it can judge a treadmill / wrong-approach nitpick cluster and return one approach-level `needs-human` instead of fixing forever — **and, when the recurring items are *valid* and share one root and fix, request a bounded-class assessment** so it consolidates the equivalent sites this PR touched into a single fix rather than dripping one per head (`references/watch-loop.md`, Non-convergence). An `invariant_rounds[].rounds >= 2` trigger routes that trajectory **before** this pass may fix/commit/push. One resolve pass per tick — never fan out multiple. For any delegate result, process its typed `needs-human` residuals through one boundary. Immediately render every complete payload under `## Needs your decision`, preserve it unchanged for caller return, and write it to an OS-temp JSON file. Persist each residual once: the snapshot validates the full schema, freezes every source's current observation, and publishes one decision under the locked state write. It emits the unchanged payload in canonical `needs_human_residuals` and its answer-routing ID in `human_decisions`; source dispositions remain ordinary open/dispatched facts. Any covered observation changing or disappearing invalidates the whole decision and reactivates its surviving sources, but remote activity is never an answer. Continue independent work without resolving covered threads. @@ -60,10 +60,10 @@ After the resolver, reconcile every **comment you passed that is not covered by SKILL_DIR=""; SCRATCH_ROOT="/tmp/compound-engineering-$(id -u)"; [ ! -L "$SCRATCH_ROOT" ] && (umask 077; mkdir -p "$SCRATCH_ROOT") 2>/dev/null && [ ! -L "$SCRATCH_ROOT" ] && [ -O "$SCRATCH_ROOT" ] && [ -w "$SCRATCH_ROOT" ] || SCRATCH_ROOT="${TMPDIR:-/tmp}/compound-engineering-$(id -u)"; [ ! -L "$SCRATCH_ROOT" ] && (umask 077; mkdir -p "$SCRATCH_ROOT") && [ ! -L "$SCRATCH_ROOT" ] && [ -O "$SCRATCH_ROOT" ] && chmod 700 "$SCRATCH_ROOT" || { echo "unsafe scratch root: $SCRATCH_ROOT" >&2; exit 1; }; STATE_DIR="$SCRATCH_ROOT/ce-babysit-pr/---"; (umask 077; mkdir -p "$STATE_DIR") || exit 1; chmod 700 "$STATE_DIR" || exit 1; PY="$(for c in python3 python py; do command -v "$c" >/dev/null 2>&1 && "$c" -c '' >/dev/null 2>&1 && { echo "$c"; break; }; done)"; [ -n "$PY" ] || { echo "no working Python 3 interpreter on PATH" >&2; exit 1; }; "$PY" "$SKILL_DIR/scripts/pr-snapshot" mark --pr --repo <[host/]owner/repo> --state-dir "$STATE_DIR" --invocation-id "$RUN_INVOCATION_ID" --session-started-at "$RUN_STARTED_AT" --invocation-budget-seconds "$RUN_BUDGET_SECONDS" --disposition needs-human --residual-file -"$PY" "$SKILL_DIR/scripts/pr-snapshot" mark --state-dir "$STATE_DIR" --invocation-id "$RUN_INVOCATION_ID" --session-started-at "$RUN_STARTED_AT" --invocation-budget-seconds "$RUN_BUDGET_SECONDS" --comment --disposition dispatched +"$PY" "$SKILL_DIR/scripts/pr-snapshot" mark --state-dir "$STATE_DIR" --invocation-id "$RUN_INVOCATION_ID" --session-started-at "$RUN_STARTED_AT" --invocation-budget-seconds "$RUN_BUDGET_SECONDS" --comment --disposition dispatched --invariant-key ``` -Passing `--pr`/`--repo` on the shared residual mark is load-bearing: `mark` re-reads every covered thread's current last comment (including your just-posted replies) before the atomic write, so later reviewer activity re-opens the whole group instead of being swallowed. Publish the decision only when every covered thread was re-read; a missing observation leaves the complete source set actionable. Covered comments and review bodies retain their snapshot edit identities for the same reason. A **dispatched comment** mark needs no baseline: it stays silenced until an explicit `mark --disposition open` and is never auto-reactivated by a body edit, because status bots rewrite their bodies on every push. A genuinely new request arrives as a review thread or a new comment, both still surfaced. +Passing `--pr`/`--repo` on the shared residual mark is load-bearing: `mark` re-reads every covered thread's current last comment (including your just-posted replies) before the atomic write, so later reviewer activity re-opens the whole group instead of being swallowed. Publish the decision only when every covered thread was re-read; a missing observation leaves the complete source set actionable. Covered comments and review bodies retain their snapshot edit identities for the same reason. A **dispatched comment** mark needs no baseline: it stays silenced until an explicit `mark --disposition open` and is never auto-reactivated by a body edit, because status bots rewrite their bodies on every push. A genuinely new request arrives as a review thread or a new comment, both still surfaced. On a **fix** outcome, persist the leaf-returned `invariant_key` on that same dispatched mark (`--thread` or `--comment` plus `--invariant-key`); omit the flag when the leaf returned none. The leaf does not run `pr-snapshot`. When the user answers, map their response to the displayed `decision_id`, preserve the exact response in a file, and record the shared answer transition before acting on it. The answer is consumed only when this mark succeeds. Read-only prohibits executing the transition, not rendering it. To complete a read-only envelope, return the literal command below as the sole pending transition; substitute exact known values and retain explicit placeholders for unavailable invocation metadata or the answer-file path. That rendered command must include the literal `--answer-decision` and `--answer-file` flags. A prose paraphrase or an in-memory state move is incomplete. diff --git a/skills/ce-babysit-pr/references/watch-loop.md b/skills/ce-babysit-pr/references/watch-loop.md index eccac139c..054c37496 100644 --- a/skills/ce-babysit-pr/references/watch-loop.md +++ b/skills/ce-babysit-pr/references/watch-loop.md @@ -54,8 +54,8 @@ The round/time budget above is a **blunt cost floor**, not a convergence detecto A loop can churn without finishing: CI **ping-pong** (fix A surfaces B, fix B brings A back — often an emergent trade-off), a review-bot **treadmill** (each commit spawns fresh nits), or **wrong-approach whack-a-mole** (each nit is valid but the approach, e.g. a regex, is the problem). A raw attempt counter can't tell these from *legitimate progress* (four independent failures each fixed once) — so the decision is **agent reasoning over the trajectory**, and the split is strict: -- **`pr-snapshot` (babysit) ships facts.** The `trajectory` block is deterministic and coarse: `check_recur_max`/`recurring_checks` (a check that failed → cleared → failed again on a *new* head; same-head flapping is excluded, so this is not flaky noise), `unresolved_trend` + `new_threads_this_tick` (backlog growing / fresh threads arriving), `stream_alternations` (ci↔review bouncing — cross-stream churn only babysit can see), `heads_since_progress` (heads moved without a new low in open problems). Babysit **never** labels this "non-convergence." -- **The leaf judges.** When a trigger fires (the thresholds are in SKILL.md Step 2 — the single source of truth; do not re-list them here), pass the trajectory into that tick's `ce-debug`/`ce-resolve-pr-feedback` as **mandatory input**. It must either demonstrate progress (name the invariant the next bounded fix resolves) or return a `needs-human` that **parks the whole stream** with a `decision_context` (the tension/root, options, tradeoffs, its lean). +- **`pr-snapshot` (babysit) ships facts.** The `trajectory` block is deterministic and coarse: `check_recur_max`/`recurring_checks` (a check that failed → cleared → failed again on a *new* head; same-head flapping is excluded, so this is not flaky noise), `unresolved_trend` + `new_threads_this_tick` (backlog growing / fresh threads arriving), `stream_alternations` (ci↔review bouncing — cross-stream churn only babysit can see), `heads_since_progress` (heads moved without a new low in open problems), `invariant_rounds` (resolver-supplied `invariant_key` values counted per unique head — the script never infers a key from paths, regexes, or comment text). Babysit **never** labels this "non-convergence." +- **The leaf judges.** When a trigger fires (the thresholds are in SKILL.md Step 2 / `references/tick.md` — the single source of truth; do not re-list them here), pass the trajectory into that tick's `ce-debug`/`ce-resolve-pr-feedback` as **mandatory input**. It must either demonstrate progress (name the invariant the next bounded fix resolves) or return a `needs-human` that **parks the whole stream** with a `decision_context` (the tension/root, options, tradeoffs, its lean). On a **fix** outcome, the leaf **returns** a stable `invariant_key` for each root it fixed, associated with the items that root covered; it does not own `pr-snapshot` and must not mark. Babysit persists each key at its existing atomic dispatched-mark boundary (`--thread` / `--comment` plus `--invariant-key`). A pass that would begin a third unique-head round for the same key (`invariant_rounds[].rounds >= 2`, since a round is recorded only after its fix completes) is itself a trigger: route the trajectory **before** another fix/commit/push so the leaf returns one approach-level `needs-human` instead of mutating again. Unrelated keys stay independently actionable. One- and two-round progress is not a blocker. **The anti-cry-wolf line (put it to the leaf):** *progressive failure migration* — A fixed → B appears once → B fixed → done — is ordinary repair; **do not park.** *Oscillation* — A returns after B's fix, the failing set cycles, defects migrate X→Y→Z with the same invariant unsatisfied, or fix size grows superlinearly — is non-convergence; park. "We've tried a lot" is never enough. @@ -141,7 +141,8 @@ State lives at `/ce-babysit-pr/---/state.js "unresolved_series": [2, 3, 4], "stream_series": ["ci", "review", "ci"], "min_open_problems": 1, - "heads_since_progress": 0 + "heads_since_progress": 0, + "invariant_heads": {} } } ``` @@ -154,7 +155,7 @@ A `check_key` is `"/"` (or `""` when there is no workflow) The rule that makes ticks idempotent *and* crash-safe: **the snapshot never marks an item handled just from observing it.** An item leaves the actionable set only when the agent confirms it acted (via `mark`) or when remote truth removes it. So if a resolve/debug pass crashes, errors, or returns without finishing, the item is still actionable on the next tick — the loop cannot silently drop work. -- **Review threads.** A thread is actionable while it is unresolved and you have not recorded acting on it. After a resolve pass, `mark --thread --disposition dispatched` handles an ordinary unresolved thread. The shared residual mark freezes its complete source observations without changing their dispositions. A later reviewer comment invalidates any covering decision and reopens every surviving sibling; it does not answer the decision. Every mark carries the active invocation tuple, so stale ticks cannot silence work in a newer invocation. +- **Review threads.** A thread is actionable while it is unresolved and you have not recorded acting on it. After a resolve pass, `mark --thread --disposition dispatched` handles an ordinary unresolved thread; when the leaf returned an `invariant_key`, that same mark carries `--invariant-key`. The shared residual mark freezes its complete source observations without changing their dispositions. A later reviewer comment invalidates any covering decision and reopens every surviving sibling; it does not answer the decision. Every mark carries the active invocation tuple, so stale ticks cannot silence work in a newer invocation. - **Non-thread feedback candidates** (top-level PR comments + review-submission bodies). These appear as `actionable.comments` when feedback has no inline thread. The detector excludes only empty bodies and never classifies content, authors, or posting surfaces; `ce-resolve` owns that judgment. Because there is no remote resolve, every passed candidate must either be marked `dispatched` or be covered by one validated current decision. A dispatched candidate stays silent across body edits because status bots routinely rewrite comments; an edit to a covered candidate invalidates the decision. A new comment has a new ID and is actionable. Both feedback surfaces remain one review stream for trajectory and backlog accounting. - **CI checks.** A failing check on the current head is actionable until you `mark --check `. A typed decision enters the same canonical set through the shared residual mark, never through check-specific decision state. A new head clears dispatch state, invalidates check-sourced residuals, and re-evaluates the decision against the new commit. diff --git a/skills/ce-babysit-pr/scripts/pr-snapshot b/skills/ce-babysit-pr/scripts/pr-snapshot index 1b525ecf4..806f0ab37 100755 --- a/skills/ce-babysit-pr/scripts/pr-snapshot +++ b/skills/ce-babysit-pr/scripts/pr-snapshot @@ -1334,6 +1334,7 @@ def _empty_trajectory(): "heads_since_progress": 0, # head changes since progress (a new low OR something cleared) "last_head": None, # head as of the last AGENT tick — hsp counts moves between ticks, # NOT poll-observed head moves (state["head_sha"] advances on polls) + "invariant_heads": {}, # resolver-supplied invariant_key -> unique heads it was marked on } @@ -1350,6 +1351,36 @@ def _load_trajectory(state): return tj +_INVARIANT_KEY_RE = re.compile(r"^[A-Za-z0-9._:-]{1,120}$") + + +def _record_invariant_round(state, key, head): + """Count a resolver-supplied invariant against unique heads. Never infer keys.""" + if not key: + return + if not _INVARIANT_KEY_RE.match(key): + raise SystemExit("--invariant-key must be 1-120 chars of A-Za-z0-9._:-") + if not head: + return + tj = _load_trajectory(state) + heads = tj.setdefault("invariant_heads", {}) + seen = heads.setdefault(key, []) + if not isinstance(seen, list): + seen = [] + heads[key] = seen + if head not in seen: + seen.append(head) + + +def _invariant_rounds_view(tj): + """Public trajectory view: resolver keys counted per unique head.""" + items = [] + for key, heads in sorted((tj.get("invariant_heads") or {}).items()): + if isinstance(heads, list): + items.append({"key": key, "rounds": len(heads)}) + return items + + def _push_bounded(lst, item, cap): """Append to a sliding window that keeps only the last `cap` items.""" lst.append(item) @@ -1484,6 +1515,7 @@ def _update_trajectory(state, head, new_checks, new_threads, new_feedback, actio "new_threads_this_tick": len(new_arrivals), "stream_alternations": _stream_alternations(tj["stream_series"]), "heads_since_progress": tj["heads_since_progress"], + "invariant_rounds": _invariant_rounds_view(tj), } @@ -3431,6 +3463,8 @@ def cmd_mark(args): entry["acted_identity"] = [args.acted_edit_id] state["last_action"] = f"{args.disposition} {label} {item_id}" marked = marked or item_id + if getattr(args, "invariant_key", None) and args.disposition == DISPOSITION_DISPATCHED: + _record_invariant_round(state, args.invariant_key, state.get("head_sha")) if isinstance(marked, dict): print(json.dumps({"marked": marked.get("id"), "decision": marked}, indent=2)) else: @@ -3523,6 +3557,8 @@ def main(): help="exact current decision ID whose human answer is being recorded") m.add_argument("--answer-file", default=None, help="file containing the human answer to preserve until covered work moves") + m.add_argument("--invariant-key", default=None, + help="resolver-supplied review invariant; counted per unique head, never inferred") m.set_defaults(func=cmd_mark) w = sub.add_parser("watch") diff --git a/skills/ce-resolve-pr-feedback/SKILL.md b/skills/ce-resolve-pr-feedback/SKILL.md index b1a7db0ac..5456eef94 100644 --- a/skills/ce-resolve-pr-feedback/SKILL.md +++ b/skills/ce-resolve-pr-feedback/SKILL.md @@ -11,7 +11,7 @@ Evaluate and fix PR review feedback, then reply and resolve threads. The orchest **Escalations never block.** `needs-human` is the escalation channel: leave the thread open with a natural reply and report the structured `decision_context`. Never pause mid-run to ask. That is what lets an autonomous caller — `ce-babysit-pr` running unattended, for example — loop this skill. Items that need a human decision come back as `needs-human` results for the caller to surface, rather than stalling the run; that includes a fix that would change behavior the author chose deliberately (see the rubric). -**`mode:pipeline`** (set by an orchestrator like `ce-babysit-pr` or `lfg`): the run is unattended, so **never call the blocking-question tool for any reason**, and read `references/pipeline-mode.md` before acting. It owns the two things ordinary mode leaves open. First, the open thread is the escalation ledger, so never write a PR-body residual section of your own. Second, the caller may pass a `trajectory` (`unresolved_trend`, `new_threads_this_tick`); when it shows that the feedback is not converging, answer with one approach-level `needs-human` rather than fixing nit after nit. +**`mode:pipeline`** (set by an orchestrator like `ce-babysit-pr` or `lfg`): the run is unattended, so **never call the blocking-question tool for any reason**, and read `references/pipeline-mode.md` before acting. It owns the two things ordinary mode leaves open. First, the open thread is the escalation ledger, so never write a PR-body residual section of your own. Second, the caller may pass a `trajectory` (`unresolved_trend`, `new_threads_this_tick`, `invariant_rounds`); when it shows that the feedback is not converging, or `invariant_rounds[].rounds >= 2` for a key this pass would continue (the next fix would be that key's third round) and that key's escalation is unanswered, answer with one approach-level `needs-human` rather than fixing nit after nit — an answered escalation authorizes the next action instead. On a fix outcome, return a stable `invariant_key` per fixed root; do not run `pr-snapshot`. **Authority in pipeline mode.** Being invoked by an orchestrator is **not** itself authorization. You act under the **inherited** scope it holds from the user: **actions** = fix / commit / push / reply / resolve on the PR head, plus ticking a `## Unapplied review findings` bullet a committed fix closed (below); **exclusions** = merge, rebase, force-push, approve CI. You may *narrow* this (decline a fix, defer a `needs-human`) but never *broaden* it — if resolving a thread would require an excluded action, defer it as `needs-human` rather than perform it. diff --git a/skills/ce-resolve-pr-feedback/references/pipeline-mode.md b/skills/ce-resolve-pr-feedback/references/pipeline-mode.md index ac30253c4..2186ae8c3 100644 --- a/skills/ce-resolve-pr-feedback/references/pipeline-mode.md +++ b/skills/ce-resolve-pr-feedback/references/pipeline-mode.md @@ -14,6 +14,10 @@ Return the exact typed residual defined by the rubric: `type: "needs-human"`, `s ## 3. Non-convergence (wrong-approach cluster / treadmill) -When the caller passes a `trajectory` (rising `unresolved_trend`, `new_threads_this_tick > 0` across passes), check whether the feedback is *not converging*: several nits that share a **root** — the approach itself is the problem (canonical: "your regex misses case X" repeated for X after X, an unbounded whack-a-mole) — or a bot re-posting fresh nits every commit without end. If so, raise **one** approach-level `needs-human` about the root decision (e.g. "regex is the wrong tool here — options: exhaustive table / a real parser / accept known limits; lean: …") and stop fixing the individual instances, rather than dutifully fixing nit after nit. +When the caller passes a `trajectory` (rising `unresolved_trend`, `new_threads_this_tick > 0` across passes, or any `invariant_rounds[].rounds >= 2`), decide each root's standing before fixing anything on it: -Hold the anti-cry-wolf line: this fires only on a *demonstrated* shared root or a *demonstrated* treadmill across passes — a normal batch of unrelated valid nits is just fixed, one pass, as usual. +- **Escalate** — raise **one** approach-level `needs-human` about the root decision (e.g. "regex is the wrong tool here — options: exhaustive table / a real parser / accept known limits; lean: …") **before** any fix/commit/push — when the root's feedback is *demonstrably* not converging (several nits sharing one root, "your regex misses case X" repeated for X after X; or a bot re-posting fresh nits every commit without end), or when a fix would begin the root's third recorded round (`invariant_rounds[].rounds >= 2` for a key this pass would continue; rounds are recorded after a fix completes). +- **Execute an answered escalation** — when the open thread already carries a human's decision on the root, that answer authorizes the next action; apply it. Re-raising the same `needs-human` is rejected by the persistence layer. +- **Otherwise fix as usual** — a normal batch of unrelated valid nits is just fixed, one pass. + +On a **fix** outcome, return a stable `invariant_key` (1–120 chars of `A-Za-z0-9._:-`) for **each** root a fix resolved, associated with the threads/comments that root covered — unrelated roots fixed in one pass carry distinct keys, so each accumulates its own rounds. Do not run `pr-snapshot`; the caller persists each key on that item's dispatched mark. diff --git a/tests/ce-babysit-pr-contract.test.ts b/tests/ce-babysit-pr-contract.test.ts index f87c0d141..009d1387b 100644 --- a/tests/ce-babysit-pr-contract.test.ts +++ b/tests/ce-babysit-pr-contract.test.ts @@ -70,6 +70,7 @@ const TRAJECTORY_FIELDS = [ "new_threads_this_tick", "stream_alternations", "heads_since_progress", + "invariant_rounds", ] const BABYSIT_TRAJECTORY_REFS = [ "check_recur_max", @@ -78,8 +79,9 @@ const BABYSIT_TRAJECTORY_REFS = [ "new_threads_this_tick", "stream_alternations", "heads_since_progress", + "invariant_rounds", ] -const CERESOLVE_TRAJECTORY_REFS = ["unresolved_trend", "new_threads_this_tick"] +const CERESOLVE_TRAJECTORY_REFS = ["unresolved_trend", "new_threads_this_tick", "invariant_rounds"] function emittedTrajectoryKeys(script: string): string[] { const fn = script.slice(script.indexOf("def _update_trajectory")) diff --git a/tests/ce-babysit-pr-snapshot.test.ts b/tests/ce-babysit-pr-snapshot.test.ts index 02c38622a..c2570756b 100644 --- a/tests/ce-babysit-pr-snapshot.test.ts +++ b/tests/ce-babysit-pr-snapshot.test.ts @@ -3137,6 +3137,27 @@ print(json.dumps({ expect(d.trajectory.heads_since_progress).toBe(1) // head moved s1->s2 between agent ticks; not starved by the poll }, 15000) + test("invariant_rounds count unique heads for a resolver-supplied key (#1575)", () => { + const dir = mkdtempSync(path.join(tmpdir(), "inv-rounds-")) + const state = path.join(dir, "state") + const t = (head: string) => ({ + ...FAILING, + head_sha: head, + checks: [], + threads: [{ thread_id: "T1", last_comment_id: head, last_comment_at: head }], + }) + snapshot(state, fetchFile(dir, "h1.json", t("h1"))) + mark(state, ["--thread", "T1", "--invariant-key", "golden-boundary"]) + snapshot(state, fetchFile(dir, "h2.json", t("h2"))) + mark(state, ["--thread", "T1", "--invariant-key", "golden-boundary"]) + // Two recorded rounds is the trigger state: the next fix would be the third. + const atTrigger = snapshot(state, fetchFile(dir, "h3.json", t("h3"))) + expect(atTrigger.trajectory.invariant_rounds).toEqual([{ key: "golden-boundary", rounds: 2 }]) + mark(state, ["--thread", "T1", "--invariant-key", "golden-boundary"]) + const d = snapshot(state, fetchFile(dir, "h3b.json", { ...FAILING, head_sha: "h3", checks: [], threads: [] })) + expect(d.trajectory.invariant_rounds).toEqual([{ key: "golden-boundary", rounds: 3 }]) + }) + test("check recurrence catches a CLEAR observed only on a watch poll (C1)", () => { const sd = path.join(dir, "recurwatch") const RED = { key: "CI/x", name: "x", status: "COMPLETED", conclusion: "FAILURE", details_url: "u" }