Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 21 additions & 2 deletions scripts/loop/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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/<n>-*` **and** `loop-task: #<n>`) opened/synchronize | run(pr-review) |
| same-repo PR, other branches | run(triage) — PRs are a triage surface |
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down
23 changes: 22 additions & 1 deletion scripts/loop/entry.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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
Expand Down
26 changes: 26 additions & 0 deletions scripts/loop/shared/route.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading