From 9e47270dfbf9bf01debd8c3df6e06355592bd269 Mon Sep 17 00:00:00 2001 From: Alex Chao Date: Wed, 21 Oct 2026 23:30:31 +0800 Subject: [PATCH] chore(loop): let a maintainer's claim label exit the waiting-human inbox --- scripts/loop/README.md | 23 +++++++++++++++++++++-- scripts/loop/entry.mjs | 23 ++++++++++++++++++++++- scripts/loop/shared/route.mjs | 26 ++++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 3 deletions(-) diff --git a/scripts/loop/README.md b/scripts/loop/README.md index 64b5ccc..460ed31 100644 --- a/scripts/loop/README.md +++ b/scripts/loop/README.md @@ -330,7 +330,7 @@ commit/push instructions are suppressed for it (the reviewer's job is the verdic | Event | Action | | --- | --- | | `issues` opened/reopened | run(triage) | -| `issues` labeled `ready-for-agent` | run(bugfix/feature) — the claim signal; fires only for labels applied by **people**, since GitHub suppresses runs for `GITHUB_TOKEN`-raised events | +| `issues` labeled `ready-for-agent` | run(bugfix/feature) — the claim signal; fires only for labels applied by **people**, since GitHub suppresses runs for `GITHUB_TOKEN`-raised events. By hand it is also the inbox's exit: a `waiting-human` task is moved to `ready` first (D37) | | `issues` edited, `issue_comment` created | inbox first (never silently dropped), then run per task status | | loop PR (`loop/-*` **and** `loop-task: #`) opened/synchronize | run(pr-review) | | same-repo PR, other branches | run(triage) — PRs are a triage surface | @@ -803,7 +803,9 @@ the first `auto` run is a verification step rather than a milestone. the state actually requires (inbox → a human decides first; waiting-merge → awaiting merge). Verified across five scenarios: inbox and awaiting-merge are refused without touching the decision, `closed` + a genuinely reopened - GitHub item is still let through, and `closed` + a closed item is not. + GitHub item is still let through, and `closed` + a closed item is not. The + inbox's one legitimate exit — a maintainer applying `ready-for-agent` by + hand — was still missing and is D37. - [x] D30 (A1) Gate events recorded the acceptance row but never moved the task: `handleMetrics` appended to `metrics/acceptance.jsonl` and returned, so a loop PR that merged was counted for the auto-acceptance rate while its task @@ -907,6 +909,23 @@ the first `auto` run is a verification step rather than a milestone. absence a `request-changes` gate before line-level review. The four headings live in `PR_CONTRACT_SECTIONS` (`shared/state.mjs`) and the maker prompt imports them, so the check and the template cannot drift. +- [x] D37 (#77) The inbox had no exit door. `ops.md` promises that a maintainer + applying `ready-for-agent` by hand *is* the claim, and `route()` does return + `run(bugfix/feature)` for `issues.labeled` — but `claimability` refuses + `waiting-human` (D29), so the run logged + `skip: task #77 not claimable (status=waiting-human)` and the task stayed in + the inbox forever. Meanwhile `issue_comment.created` on a `waiting-human` + task only lands in `eventInbox` (`handleInbox` re-runs for `new`/`ready`/ + `waiting-info` only), so *both* things a human can do to an inbox task — + comment, then apply the claim label — were no-ops, and the only remaining + entry was `workflow_dispatch`, which hits the same claim check. + `humanClaimOf` (`shared/route.mjs`) now recognises exactly that event — + `issues.labeled`, the claim label, applied by an OWNER/MEMBER/COLLABORATOR, + out of `waiting-human` — and `doRun` moves the task to `ready` (timeline + `claimed`, labels refreshed from the event) before claiming it. Narrow on + purpose: D29's repair path, `start --task` and PR `synchronize` still cannot + convert an inbox item, so the cognitive guard is unchanged and only an + explicit human act opens it. ## Environment facts diff --git a/scripts/loop/entry.mjs b/scripts/loop/entry.mjs index e96ffaf..303a35d 100644 --- a/scripts/loop/entry.mjs +++ b/scripts/loop/entry.mjs @@ -35,7 +35,7 @@ import { nowIso, outcomeAllowed, allowedOutcomes, describeActions, LOOP_BRANCH_RE, missingPrContractSections, } from './shared/state.mjs'; -import { route, eventKey, stageForIssue } from './shared/route.mjs'; +import { route, eventKey, stageForIssue, humanClaimOf, CLAIM_LABEL } from './shared/route.mjs'; import { buildPrompt, runPi, parseEvents, summarize, classify } from './shared/agent.mjs'; import { runDoubleReview } from './shared/review.mjs'; @@ -285,6 +285,27 @@ async function doRun({ decision, ctx, repo, writeLevel }) { if (!isSystem) { const kind = decision.kind ?? 'issue'; task = await prepareTask({ taskId: decision.taskId, kind, repo }); + // A maintainer's claim label is the inbox's exit (D37): `waiting-human` is + // deliberately not claimable (D29), so without this the `issues.labeled` run + // that route() just produced would be skipped and the task would sit in the + // inbox forever. Only this explicit human act converts it; every routine path + // still cannot. + const claimedBy = humanClaimOf(ctx, task.status); + if (claimedBy) { + log(`human claim: task #${task.id} waiting-human → ready ` + + `(${CLAIM_LABEL} applied by ${claimedBy} — the label is the inbox decision)`); + task.status = 'ready'; + task.labels = (ctx.event.issue?.labels ?? []) + .map((l) => (typeof l === 'string' ? l : l?.name)); + task.timeline = [...(task.timeline ?? []), { + at: nowIso(), + event: 'claimed', + by: claimedBy, + detail: `applied ${CLAIM_LABEL} by hand: inbox (waiting-human) → claimable`, + }]; + task.updatedAt = nowIso(); + await saveTask(S, task); + } const claim = claimability(task); if (!claim.ok) { // A skip must be logged: during an event storm, a human must see at a glance in the diff --git a/scripts/loop/shared/route.mjs b/scripts/loop/shared/route.mjs index aa935ee..88ef64e 100644 --- a/scripts/loop/shared/route.mjs +++ b/scripts/loop/shared/route.mjs @@ -65,6 +65,32 @@ const labelNames = (labels = []) => labels.map((l) => (typeof l === 'string' ? l export const stageForIssue = (labels = []) => labelNames(labels).some((n) => BUG_LABELS.includes(n)) ? 'bugfix' : 'feature'; +/** + * The human inbox's exit door: a maintainer applying the claim label by hand *is* + * the decision the inbox exists to await (ops.md "The claim signal"). + * + * The event still routes to `run(stage)`, but `waiting-human` is deliberately not + * claimable (D29), so without this the run is skipped and the task stays in the + * inbox forever — the only door a human has was locked from the inside. Observed + * on #77: the owner applying `ready-for-agent` produced + * `skip: task #77 not claimable (status=waiting-human)`. + * + * Deliberately narrow — an `issues.labeled` event, the claim label, applied by a + * repository owner/member/collaborator, for that very issue, out of + * `waiting-human` only. The routine paths (D9's reopen repair, `start --task`, + * PR synchronize) keep D29's guarantee that an inbox item never becomes + * claimable without a human saying so. + * + * @returns {string|null} the maintainer's login when this event is the claim. + */ +export function humanClaimOf({ eventName, action, event = {} }, status) { + if (eventName !== 'issues' || action !== 'labeled') return null; + if (event.label?.name !== CLAIM_LABEL) return null; + if (status !== 'waiting-human') return null; + if (!OWNER_ROLES.includes(event.sender?.author_association ?? '')) return null; + return event.sender?.login ?? 'a maintainer'; +} + /** * Idempotency key: skip an event whose key has already been consumed. * Prefer immutable identifiers (PR head sha / comment id), falling back to the