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
14 changes: 14 additions & 0 deletions scripts/loop/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ GitHub event ─▶ loop.yml (concurrency group `loop` = platform-level single w
| same-repo PR, other branches | run(triage) — PRs are a triage surface |
| `pull_request_review` on a loop PR | run(pr-review) |
| `pull_request_target` closed (loop PR) | metrics: merged / closed-unmerged **and** the task moves to `accepted` / `rejected` |
| `pull_request_target` closed (other PR) | task → `accepted` / `rejected` only; **no metric** (not loop-produced) |
| `release` published | metrics: released |
| `schedule` daily / weekly | system run: sweep / retro-scheduled |
| `workflow_dispatch` | run per inputs (`task`/`stage`); `execute_writes=true` = L2 rehearsal |
Expand Down Expand Up @@ -510,6 +511,19 @@ was *correct*, and whether new events produce proposals that match reality.
the row through the existing `writer` field (`writer: "sweep-corrected"`),
so dedup still matches. Recorded now because the ambiguity is in the norms
layer, and whoever implements the backfill will read that sentence first.
- [x] D33 (A1) A non-loop PR closing left its task stranded. `pull_request_target`
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
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.

## Environment facts

Expand Down
26 changes: 25 additions & 1 deletion scripts/loop/entry.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,28 @@ async function handleMetrics({ decision }) {
return { state: 'metrics', wrote, row, transition };
}

/* --------------------------------------------------------------- terminal */

/**
* Terminalise a task with no metric attached — a PR that closed was not
* loop-produced, so it must not enter the acceptance rate, but its task still has
* to finish. Shares `markTaskTerminal` with the metrics path so there is one
* definition of "what terminal means".
*/
async function handleTaskTerminal({ decision }) {
const transition = await markTaskTerminal(S, decision.taskId, decision.to, {
event: decision.to === 'accepted' ? 'merged' : 'closed-unmerged',
detail: `${decision.pr ? `PR #${decision.pr} ` : ''}${decision.reason}`,
});
if (transition.changed) {
log(`task #${decision.taskId}: ${transition.from} → ${decision.to} (${decision.reason})`);
await renderStateSummary(S);
} else {
log(`task #${decision.taskId}: no transition needed (${transition.reason})`);
}
return { state: 'terminal', transition, note: decision.reason };
}

/* ------------------------------------------------------------------ run */

async function doRun({ decision, ctx, repo, writeLevel }) {
Expand Down Expand Up @@ -394,10 +416,12 @@ async function main() {
log(result.wrote
? `metrics written: ${JSON.stringify(result.row)}`
: 'metrics already present, skipped (idempotent)');
} else if (decision.action === 'terminal') {
result = await handleTaskTerminal({ decision });
}

await writeStepSummary(renderStepSummary({ decision, result, writeLevel }));
return 0; // noop / inbox-only / metrics / a completed run all exit successfully
return 0; // noop / inbox-only / metrics / terminal / a completed run all exit successfully
}

main()
Expand Down
42 changes: 31 additions & 11 deletions scripts/loop/shared/route.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ const noop = (reason) => ({ action: 'noop', reason });
const run = (o) => ({ action: 'run', ...o });
const metrics = (o) => ({ action: 'metrics', ...o });
const inbox = (o) => ({ action: 'inbox', ...o });
/**
* Move a task to a terminal state without recording metrics — for closures that
* must not count toward the acceptance rate because the PR was not loop-produced.
*/
const terminal = (o) => ({ action: 'terminal', ...o });

/**
* Loop PR test: head branch `loop/<n>-*` and body containing `loop-task: #<n>`
Expand Down Expand Up @@ -121,18 +126,33 @@ export function route({ eventName, action, event = {} }) {
case 'pull_request_target': {
const pr = event.pull_request ?? {};
if (action !== 'closed') return noop(`pull_request_target.${action} has no Loop action`);
const loopTask = loopTaskOf(pr);
if (!loopTask) return noop('closing a non-loop PR is not counted for auto-acceptance');
const merged = Boolean(pr.merged);
return metrics({
reason: `loop PR ${merged ? 'merged' : 'closed-unmerged'}`,
metrics: {
event: merged ? 'merged' : 'closed-unmerged',
taskId: loopTask,
pr: pr.number,
accepted: merged,
writer: 'workflow',
},
const loopTask = loopTaskOf(pr);
if (loopTask) {
return metrics({
reason: `loop PR ${merged ? 'merged' : 'closed-unmerged'}`,
metrics: {
event: merged ? 'merged' : 'closed-unmerged',
taskId: loopTask,
pr: pr.number,
accepted: merged,
writer: 'workflow',
},
});
}
// 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.
return terminal({
reason: `non-loop PR ${merged ? 'merged' : 'closed-unmerged'} `
+ '(not counted for auto-acceptance)',
taskId: pr.number,
to: merged ? 'accepted' : 'rejected',
pr: pr.number,
});
}

Expand Down
Loading