From 9132c5d97c2fb893fabe21c57e4ee26407798f28 Mon Sep 17 00:00:00 2001 From: Alex Chao Date: Fri, 11 Sep 2026 19:53:48 +0800 Subject: [PATCH] chore(loop): correct the D33 note (dependabot PRs never reach triage) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The D33 entry claimed "every owner or dependabot PR gets a task from `pull_request` triage". Dependabot is not one of them: the credential guard (D28) skips the whole job before triage, because a Dependabot-triggered `pull_request` run gets a read-only token and no secrets. So the new `terminal` branch is a no-op for them. The code was always right — `markTaskTerminal` treats a missing task as "no transition needed" — only the reasoning was wrong. Still worth fixing: the note is what the next reader trusts, and a wrong premise there invites a wrong change later. Also fold in what the 2026-09-11 daily sweep established, since it changes how D33 reads: the #39 drift was self-healed by r-20260911-080231-zja0, but only because the #40 triage report had mentioned it in passing. sweep cannot see closures, which is the accidental path D30/D33 exist to remove. --- scripts/loop/README.md | 26 ++++++++++++++++++-------- scripts/loop/shared/route.mjs | 9 +++++---- 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/scripts/loop/README.md b/scripts/loop/README.md index 502e16c..3a58fbd 100644 --- a/scripts/loop/README.md +++ b/scripts/loop/README.md @@ -515,15 +515,25 @@ was *correct*, and whether new events produce proposals that match reality. returned `noop` for any PR without `loop-task:`, which is right about the **metric** (not loop-produced ⇒ never in the acceptance rate) but wrong about the **task**: ops.md maps "(none, merged) → accepted" and "(none, - closed unmerged) → rejected" regardless of provenance. Every owner or - dependabot PR gets a task from `pull_request` triage, so each merged PR left - a zombie in `waiting-human` — the task for the very PR that fixed D30 (#39) - demonstrated it live — visible forever under "Inbox" in SUMMARY.md and + closed unmerged) → rejected" regardless of provenance. Every owner PR gets a + task from `pull_request` triage, so each merge left a zombie in + `waiting-human` — the task for the very PR that fixed D30 (#39) showed it + live — visible forever under "Inbox" in SUMMARY.md and, in general, uncorrectable by sweep, which lists *open* GitHub items and so cannot see a - closure. The router now emits a `terminal` decision for that case; the - entry handles it through the same `markTaskTerminal` as the metrics path, so - "what terminal means" has one definition, and the acceptance row is still - never written for a non-loop PR. + closure. (Dependabot PRs are skipped before triage by the credential guard + (D28), so for them the new branch is a no-op; it still matters if D28 is + ever lifted, and it correctly terminalises the older dependabot task #35.) + The router now emits a `terminal` decision for that case; the entry handles + it through the same `markTaskTerminal` as the metrics path, so "what + terminal means" has one definition, and the acceptance row is still never + written for a non-loop PR. + Verified live on 2026-09-11 (`34595150462`, PR #40 merged): `task #40: + waiting-human → accepted`, and R2 then reported `closed=1 accepted=9` with + acceptance rows still at 3 — no metric for a non-loop PR. + The #39 instance was **not** this fix's doing: the drift was self-healed by + the 09-11 daily sweep (`r-20260911-080231-zja0`), which only found it + because the #40 triage report had mentioned it in passing. That is the + accidental path D30/D33 exist to remove. ## Environment facts diff --git a/scripts/loop/shared/route.mjs b/scripts/loop/shared/route.mjs index de023c0..c692880 100644 --- a/scripts/loop/shared/route.mjs +++ b/scripts/loop/shared/route.mjs @@ -143,10 +143,11 @@ export function route({ eventName, action, event = {} }) { // Not loop-produced, so it must not enter the acceptance metric — but its own // task still has to reach a terminal state, or it sits in the inbox forever // (ops.md maps "(none, merged) -> accepted", "(none, closed unmerged) -> - // rejected"). Every owner/dependabot PR gets a task from `pull_request` - // triage, so without this each one strands a zombie in the state layer, and - // sweep cannot repair it: sweep lists *open* GitHub items, which cannot see - // a closure. + // rejected"). Every owner PR gets a task from `pull_request` triage, so + // without this each merge strands a zombie in the state layer, and sweep + // cannot repair it: sweep lists *open* GitHub items, which cannot see a + // closure. (Dependabot PRs are skipped before triage by the credential + // guard, D28, so for them this branch is a no-op.) return terminal({ reason: `non-loop PR ${merged ? 'merged' : 'closed-unmerged'} ` + '(not counted for auto-acceptance)',