chore(loop): move tasks to terminal states on GitHub gate events

scripts/loop/README.md

@@ -88,7 +88,7 @@ GitHub event ─▶ loop.yml (concurrency group `loop` = platform-level single w
 | 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 |
 | `pull_request_review` on a loop PR | run(pr-review) |
-| `pull_request_target` closed (loop PR) | metrics: merged / closed-unmerged |
+| `pull_request_target` closed (loop PR) | metrics: merged / closed-unmerged **and** the task moves to `accepted` / `rejected` |
 | `release` published | metrics: released |
 | `schedule` daily / weekly | system run: sweep / retro-scheduled |
 | `workflow_dispatch` | run per inputs (`task`/`stage`); `execute_writes=true` = L2 rehearsal |
@@ -426,13 +426,24 @@ was *correct*, and whether new events produce proposals that match reality.
       Summary, so the Actions log showed a route line and then nothing — a skip
       was indistinguishable from a silent failure. Every path (skip, inbox
       no-op, inbox-only, duplicate event, no-op route) now logs its reason.
-- [ ] D26 (A1, open) A task in `waiting-merge` cannot be re-claimed, so a loop
-      PR that receives a new commit (`synchronize`) is skipped instead of
-      re-reviewed, and `review_requested`-style re-entry has no path back.
-      Re-review after new commits is normal in a PR loop; the fix probably
-      belongs in the `pr-review` route (reopen `waiting-merge` → `ready` on
-      `synchronize` for loop PRs) rather than in claimability. Left open
-      deliberately: A1's week should show how often it actually bites.
+- [ ] D26 (A1, open) A `pull_request.synchronize` on a PR task is skipped whenever
+      its state is not claimable, so a PR that gains a commit is never re-analysed
+      or re-reviewed. Two faces, both observed on real PRs:
+      - `waiting-merge`: a loop PR that receives a new commit is skipped instead of
+        re-reviewed, so `review_requested`-style re-entry has no path back.
+      - `waiting-human`: **observed live on #39** — the first push opened the PR and
+        triaged it to `waiting-human`; the second push logged
+        `skip: task #39 not claimable (status=waiting-human)`. Every later push is
+        skipped too, so the task's analysis is frozen at whatever the first
+        revision looked like.
+
+      This is a real consequence of running at the report boundary: `flows` §2a
+      describes request-changes → `processing` → re-review on `synchronize`, but a
+      report-boundary `pr-review` cannot comment or relabel, so it can only land in
+      `waiting-human` — where nothing re-enters. Left open deliberately: A1's week
+      should quantify how often this bites (it will have hit every loop PR by then),
+      and the fix is a design choice — re-open the task on `synchronize` for PR
+      kinds, or add a bounded re-triage — not a one-line guard.
 - [x] D27 (A1) `run.mjs`'s subcommands did not catch errors thrown by the shared
       library, so `pnpm loop start --task <missing>` printed a full Node stack
       trace instead of a one-line reason — the library throws (correct for the
@@ -462,6 +473,43 @@ was *correct*, and whether new events produce proposals that match reality.
       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.
+- [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
+      stayed in `waiting-merge`. That contradicts the state machine ops.md
+      defines (`waiting-merge → accepted | rejected`), and sweep cannot repair it
+      — sweep reconciles by listing *open* GitHub items, which by construction
+      cannot see a closure. Any loop PR merged unattended would have stranded its
+      task under "Awaiting human merge" permanently. `handleMetrics` now also
+      applies the transition via `markTaskTerminal` (merged → `accepted`,
+      closed-unmerged → `rejected`) and refreshes `SUMMARY.md`. The two effects
+      are idempotent independently — the row on `taskId+event+pr`, the task on
+      its own status — so a crash between them is repaired by a repeated event
+      rather than leaving a permanent inconsistency. Verified: both directions,
+      duplicate events, a non-`waiting-merge` originating state (recorded as
+      `(was …)` so an unexpected route is visible), `released` leaving the status
+      alone, and a missing task not throwing.
+- [x] D31 (A1) `renderSummary` named two different things: the shared library's
+      exporter writes `state/SUMMARY.md`, while `entry.mjs`'s local function
+      renders the Actions Step Summary. Importing the former alongside the latter
+      was a `SyntaxError` at module load — the orchestrator would not start. The
+      real problem is the name, not the collision, so they are now
+      `renderStateSummary` (state layer) and `renderStepSummary` (Actions UI,
+      paired with the existing `writeStepSummary`). Terminal-state lists, which
+      had also been written out inline in more than one place, are now the single
+      exported `TERMINAL_STATUSES` — duplicated lists are this codebase's
+      recurring failure mode.
+- [ ] D32 (A1, open — not yet reachable) The acceptance dedup key is
+      `taskId|event|pr`, but sweep's mandate (docs + `skills/sweep/SKILL.md`) is
+      to rewrite a *missed* event "labelled `sweep-corrected`". Read literally
+      that means writing `event: "sweep-corrected"`, which changes the key and
+      therefore appends a **second** row for the same closure — double-counting
+      the PR in the auto-acceptance rate, which is the one number retro exists
+      to produce. Not yet triggerable: no sweep implementation writes it today.
+      The fix when sweep lands is to keep the original `event` value and mark
+      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.
 
 ## Environment facts
 

scripts/loop/entry.mjs

@@ -29,7 +29,8 @@ import { spawnSync } from 'node:child_process';
 
 import {
   makeState, ensureDirs, readJson, readJsonl, writeJson, saveTask, listTasks,
-  appendAcceptance, beginRun, finishRun, lockExpired,
+  appendAcceptance, markTaskTerminal, GATE_TRANSITIONS, renderStateSummary,
+  beginRun, finishRun, lockExpired,
   nowIso, outcomeAllowed, allowedOutcomes, describeActions,
 } from './shared/state.mjs';
 import { route, eventKey } from './shared/route.mjs';
@@ -137,6 +138,15 @@ async function handleInbox({ ctx, repo }) {
 
 /* ---------------------------------------------------------------- metrics */
 
+/**
+ * GitHub gate events (a loop PR merged / closed unmerged, a release) are the only
+ * place a task reaches a terminal state without a run. Two effects, each
+ * idempotent on its own footing:
+ *   1. append the acceptance row (deduplicated by taskId+event+pr);
+ *   2. move the task to its terminal state.
+ * They are deliberately not gated on each other — if the process dies between
+ * them, a repeated event still repairs the task instead of leaving it stranded.
+ */
 async function handleMetrics({ decision }) {
   const row = { at: nowIso(), ...decision.metrics };
   if (row.event === 'released' && !row.taskId) {
@@ -149,7 +159,22 @@ async function handleMetrics({ decision }) {
     }
   }
   const wrote = await appendAcceptance(S, row);
-  return { state: 'metrics', wrote, row };
+
+  let transition = null;
+  const to = GATE_TRANSITIONS[row.event];
+  if (to && row.taskId) {
+    transition = await markTaskTerminal(S, row.taskId, to, {
+      event: row.event,
+      detail: `${row.pr ? `PR #${row.pr} ` : ''}${row.event}`,
+    });
+    if (transition.changed) {
+      log(`task #${row.taskId}: ${transition.from} → ${to} (${row.event})`);
+      await renderStateSummary(S);
+    } else {
+      log(`task #${row.taskId}: no transition needed (${transition.reason})`);
+    }
+  }
+  return { state: 'metrics', wrote, row, transition };
 }
 
 /* ------------------------------------------------------------------ run */
@@ -258,7 +283,7 @@ async function readTextSafe(file) {
   try { return await fs.readFile(file, 'utf8'); } catch { return ''; }
 }
 
-function renderSummary({ decision, result, writeLevel }) {
+function renderStepSummary({ decision, result, writeLevel }) {
   const L = [`## Loop — ${decision.action}`, ''];
   L.push(`- event: \`${decision.reason}\``);
   if (result?.runId) {
@@ -273,6 +298,12 @@ function renderSummary({ decision, result, writeLevel }) {
   } else if (result?.note) {
     L.push(`- ${result.state}: ${result.note}`);
   }
+  if (result?.transition) {
+    const tr = result.transition;
+    L.push(tr.changed
+      ? `- task #${tr.task.id}: **${tr.from} → ${tr.task.status}**`
+      : `- task: no transition needed (${tr.reason})`);
+  }
   if (result?.actions?.length) {
     const head = writeLevel === 'auto' ? '### GitHub actions executed' : '### Proposed GitHub actions (report boundary — NOT executed)';
     L.push('', head, '');
@@ -365,7 +396,7 @@ async function main() {
       : 'metrics already present, skipped (idempotent)');
   }
 
-  await writeStepSummary(renderSummary({ decision, result, writeLevel }));
+  await writeStepSummary(renderStepSummary({ decision, result, writeLevel }));
   return 0; // noop / inbox-only / metrics / a completed run all exit successfully
 }
 

scripts/loop/run.mjs

@@ -44,7 +44,7 @@ import { spawnSync } from 'node:child_process';
 import {
   makeState, ensureDirs, loadTask, saveTask, nextTaskId, listTasks,
   readRunLines, appendRunRow, readJson, writeJson,
-  nowIso, parseArgs, renderSummary,
+  nowIso, parseArgs, renderStateSummary,
   OUTCOME_MAP, lockExpired, describeActions,
   beginRun, finishRun,
 } from './shared/state.mjs';
@@ -307,7 +307,7 @@ write level: LOOP_WRITE_LEVEL=report|auto (default report)`);
     if (cmd === 'start') return await cmdStart(args);
     if (cmd === 'end') return await cmdEnd(args);
     if (cmd === 'checkpoint') return await cmdCheckpoint(args);
-    if (cmd === 'summary') return await renderSummary(S);
+    if (cmd === 'summary') return await renderStateSummary(S);
     if (cmd === 'view') return await cmdView(args);
   } catch (e) {
     fail(e.message);

scripts/loop/shared/state.mjs

@@ -17,6 +17,13 @@ import { spawnSync } from 'node:child_process';
 /** Five canonical role labels (triage-labels.md): mutually exclusive, never stacked. */
 export const ROLE_LABELS = ['needs-triage', 'needs-info', 'ready-for-agent', 'ready-for-human', 'wontfix'];
 
+/**
+ * Terminal task states (ops.md state machine). Single definition on purpose:
+ * this list used to be written out inline in several places, and duplicated
+ * lists are how this codebase drifts.
+ */
+export const TERMINAL_STATUSES = ['accepted', 'rejected', 'closed'];
+
 /** outcome → task terminal status + label to apply (ops.md "Labels → task state"). */
 export const OUTCOME_MAP = {
   'triaged':      { status: 'ready',         label: 'ready-for-agent' },
@@ -212,9 +219,54 @@ export async function appendAcceptance(s, row) {
   return true;
 }
 
+/**
+ * Move a task to a terminal state in response to a GitHub gate event.
+ *
+ * ops.md defines `waiting-merge → accepted (merged) / rejected (closed-unmerged)`,
+ * and GitHub is the source of truth for "a human accepted or rejected this".
+ * Nothing else in the host performed that transition: the router recorded the
+ * acceptance row and left the task sitting in `waiting-merge` forever, where it
+ * kept showing up under "Awaiting human merge" and no sweep could correct it
+ * (sweep reconciles by listing *open* GitHub items, which cannot see this
+ * direction). A loop PR merged with nobody watching would strand its task.
+ *
+ * Idempotent on the task's own state rather than on the acceptance row: if the
+ * process dies between writing the row and the transition, a repeated event
+ * still repairs the task.
+ *
+ * @returns {{task: object|null, changed: boolean, from?: string, reason?: string}}
+ */
+export async function markTaskTerminal(s, taskId, to, { event, detail, by = 'workflow' } = {}) {
+  if (!TERMINAL_STATUSES.includes(to)) {
+    throw new Error(`markTaskTerminal: ${to} is not terminal (expected one of ${TERMINAL_STATUSES.join(', ')})`);
+  }
+  const t = await readJson(s.taskFile(taskId));
+  if (!t) return { task: null, changed: false, reason: `task #${taskId} does not exist` };
+  if (TERMINAL_STATUSES.includes(t.status)) {
+    return { task: t, changed: false, reason: `already ${t.status}` };
+  }
+  const from = t.status;
+  t.status = to;
+  t.labels = []; // terminal states carry no role label
+  t.timeline.push({
+    at: nowIso(),
+    event,
+    by,
+    detail: `${detail}${from === 'waiting-merge' ? '' : ` (was ${from})`}`,
+  });
+  await saveTask(s, t);
+  return { task: t, changed: true, from };
+}
+
+/** The router's gate events → the terminal state they imply. */
+export const GATE_TRANSITIONS = {
+  merged: 'accepted',
+  'closed-unmerged': 'rejected',
+};
+
 /* -------------------------------------------------------------- SUMMARY */
 
-export async function renderSummary(s) {
+export async function renderStateSummary(s) {
   const tasks = await listTasks(s);
   const L = [];
   L.push(`# Loop state summary (updated ${nowIso()})`);
@@ -257,7 +309,7 @@ export async function renderSummary(s) {
     return `#${t.id} (${kind}) — ${t.title} (${when(t)})`;
   }));
 
-  const terminal = tasks.filter((t) => ['closed', 'accepted', 'rejected'].includes(t.status));
+  const terminal = tasks.filter((t) => TERMINAL_STATUSES.includes(t.status));
   if (terminal.length) {
     const c = (st) => terminal.filter((t) => t.status === st).length;
     L.push('## Terminal counts', '');
@@ -451,7 +503,7 @@ export async function finishRun(s, {
   });
 
   if (!t) {
-    await renderSummary(s);
+    await renderStateSummary(s);
     return { task: null, actions: [], applied: [] };
   }
 
@@ -477,7 +529,7 @@ export async function finishRun(s, {
     else await appendActionBlock(s, runId, actions);
   }
 
-  await renderSummary(s);
+  await renderStateSummary(s);
   return { task: t, actions, applied };
 }