From f2a58e5ea0edf7132f4016553603ccbd3d755a9d Mon Sep 17 00:00:00 2001 From: Alex Chao Date: Wed, 16 Sep 2026 02:07:03 +0800 Subject: [PATCH] chore(loop): let a triaged task actually reach the implementing stage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Triage could decide an issue was `ready-for-agent` and nothing would ever happen to it. The route table had no `feature`/`bugfix` entry at all: `run()` was reachable only for `triage`, `pr-review`, `deps` and the two schedules, so `STAGE_OUTCOMES.bugfix` was unreachable on Actions and the A1 acceptance item for "bugfix-feature" was never unsampled — it was structurally unreachable. The loop could classify work and then never do any. Two mechanisms, because the obvious one does not work: - **A person applies `ready-for-agent`** → `issues.labeled` runs the stage directly, and `stageForIssue` picks `bugfix` when the issue also carries `bug`, else `feature`. This doubles as the manual claim path: labelling any issue hands it to the loop. - **The loop applies it** (the ordinary triage verdict) → no run exists to route. GitHub suppresses workflow runs for events raised by `GITHUB_TOKEN`, which is the documented recursion guard, and it is why the loop has never had an `issues` event. `workflow_dispatch` is the stated exception, so `entry.mjs` asks for the next stage explicitly (`dispatchStage`). That is the whole reason for the new `actions: write` scope — the narrowest one that makes the loop able to start its own work. Only `ready-for-agent` is a claim. The other four role labels are states the loop itself writes, and reacting to them would make it chase its own output. Also in scope, because it defines this path's vocabulary and had drifted from it: `docs/agents/ops.md` now lists the claim signal in the trigger map and states both mechanisms, and `scripts/loop/README.md` carries the same for the workflow table. Verification: the router is a pure function, so the route is tested offline rather than by firing GitHub events — 17 assertions covering the claim (both stages), the four non-claim labels, a stale `ready-for-agent` that must not hijack another label event, the untouched pre-existing routes, and `stageForIssue` against both label shapes. `gh-check` now reports the dispatch scope as a needed capability rather than a forbidden one. --- .github/workflows/loop.yml | 11 +++++++- docs/agents/ops.md | 20 ++++++++++++++ scripts/loop/README.md | 2 ++ scripts/loop/entry.mjs | 50 +++++++++++++++++++++++++++++++++-- scripts/loop/gh-check.mjs | 20 ++++++++------ scripts/loop/shared/route.mjs | 39 +++++++++++++++++++++++++++ 6 files changed, 131 insertions(+), 11 deletions(-) diff --git a/.github/workflows/loop.yml b/.github/workflows/loop.yml index ec2bc02..1c8a31e 100644 --- a/.github/workflows/loop.yml +++ b/.github/workflows/loop.yml @@ -5,7 +5,9 @@ name: Loop # lock (R2 has no concurrent writes). on: issues: - types: [opened, reopened, edited] + # `labeled` carries the claim signal (`ready-for-agent`): triage's verdict when the + # loop applied it, or a human claiming an issue for the loop by hand. + types: [opened, reopened, edited, labeled] issue_comment: types: [created] pull_request: @@ -55,10 +57,17 @@ concurrency: # `pull_request` mode — so an unapproved direct push is refused for every credential # including the loop's. `permissions:` carries no `administration`, so the loop # cannot weaken that ruleset either. +# +# `actions: write` is the newest scope and the narrowest justified one: triage hands a +# claimed task to the implementing stage with `gh workflow run`, because a label the +# loop applies with `GITHUB_TOKEN` raises an event GitHub deliberately does not turn +# into a run. It lets the loop start workflows — not change `main`, not edit workflow +# files (that needs `workflows`, which `permissions:` cannot grant). permissions: contents: write issues: write pull-requests: write + actions: write jobs: loop: diff --git a/docs/agents/ops.md b/docs/agents/ops.md index 91b67db..adbef91 100644 --- a/docs/agents/ops.md +++ b/docs/agents/ops.md @@ -19,6 +19,7 @@ the relevant skill in `skills/`. | Event | Stage | | --- | --- | | issue opened/reopened/edited | `triage` → `bugfix`/`feature`/close/`needs-info` | +| issue labeled `ready-for-agent` | `bugfix` (`bug` label) / `feature` — the claim signal | | loop PR opened/synchronize/review | `pr-review` | | external PR opened | read-only analysis + `needs-triage` | | PR merged (loop PR) | acceptance record (dispatcher) | @@ -27,6 +28,25 @@ the relevant skill in `skills/`. | rejection / review failure | `retro-immediate` | | dependency/security | `deps`/`security` → bugfix template or `needs-triage` | +### The claim signal + +Triage's `ready-for-agent` verdict is the hand-off: it means "classified, high or +medium confidence, the loop may implement this". The task then runs `bugfix` when the +issue also carries `bug`, and `feature` otherwise. + +Two ways it fires, and they are deliberately different mechanisms: + +- **A human applies `ready-for-agent`** → the `issues.labeled` event runs the stage + directly. This is also how a maintainer claims any issue for the loop by hand. +- **The loop applies it** (the ordinary triage verdict) → GitHub suppresses runs for + events raised by `GITHUB_TOKEN`, so no `labeled` run exists. `entry.mjs` therefore + dispatches the next stage with `gh workflow run`, which is the documented exception + to that suppression. This is what the loop's `actions: write` scope is for. + +Consequence to keep in mind when auditing scopes: the loop can start its own +workflows. It still cannot change `main` (ruleset) and cannot edit workflow files +(no `workflows` permission exists to grant). + ## Run ritual Opening (always): diff --git a/scripts/loop/README.md b/scripts/loop/README.md index b94d262..209c175 100644 --- a/scripts/loop/README.md +++ b/scripts/loop/README.md @@ -215,6 +215,7 @@ reason a future reader may see the parameter and wonder where it came from. | `agentEnv(writeLevel)` | `shared/agent.mjs` | Builds the child environment from scratch: strips the R2 keys (the state bucket is the host's business, not the agent's) and injects a git identity under `auto`. It does **not** hand the agent a weaker token — a job has only one. | | `persist-credentials: false` | the `actions/checkout` step | Keeps the job token out of the repo's git config, where every later `git` invocation would pick it up. Defence in depth, not the mechanism. | | `vars.LOOP_EXECUTE_WRITES` | job env | The switch. `workflow_dispatch` can override it per run: leave the input empty to follow the variable, or pass `true`/`false` to force one run. (The input has no `default:` on purpose — a non-empty default like `'false'` short-circuits the `github.event.inputs.x \|\| vars.x` fallback and makes the variable unreachable from the one entry point a human uses to test it, which is exactly what happened before this was fixed.) | +| stage hand-off | `entry.mjs` (`dispatchStage`) | After a `triaged` **issue**, triage dispatches the implementing stage (`bugfix` if the issue carries `bug`, else `feature`). It cannot rely on its own label: GitHub suppresses runs for `GITHUB_TOKEN`-raised events, and `workflow_dispatch` is the documented exception. This is the only consumer of `actions: write`. A failed dispatch warns and leaves the task claimable rather than failing the run. | | `push` action | `planActions` | `pr-opened` plans `push` → `pr-create` → label/comment. A failure in either of the first two **aborts the chain**, so a PR that never appeared is never labelled. | | branch check | `applyActions` | `loop/-` only — no other ref is pushed, whatever the branch field says. | | dirty-tree check | `applyActions` | A dirty tree is refused: the commit would silently stay behind and the PR would ship an empty diff. | @@ -263,6 +264,7 @@ GitHub event ─▶ loop.yml (concurrency group `loop` = platform-level single w | 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` 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 | diff --git a/scripts/loop/entry.mjs b/scripts/loop/entry.mjs index 3a7707c..1a324d5 100644 --- a/scripts/loop/entry.mjs +++ b/scripts/loop/entry.mjs @@ -33,7 +33,7 @@ import { beginRun, finishRun, lockExpired, nowIso, outcomeAllowed, allowedOutcomes, describeActions, LOOP_BRANCH_RE, } from './shared/state.mjs'; -import { route, eventKey } from './shared/route.mjs'; +import { route, eventKey, stageForIssue } from './shared/route.mjs'; import { buildPrompt, runPi, parseEvents, summarize, classify } from './shared/agent.mjs'; const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..', '..'); @@ -235,6 +235,34 @@ function validatePush({ writeLevel, stage, taskId, push }) { return { ok: true, push: { ...push, branch } }; } +/** + * Hand a triaged task to the stage that implements it. + * + * The obvious mechanism — let triage's `ready-for-agent` label raise an + * `issues.labeled` run — cannot work when the loop applies that label itself: + * GitHub suppresses workflow runs for events raised by `GITHUB_TOKEN`, precisely so + * a workflow cannot call itself in a loop. `workflow_dispatch` is the documented + * exception (it *always* creates a run), so the host asks for the next stage + * explicitly. One run stays one stage; the cost is `actions: write`. + * + * Not dispatching is not a failure of this run: the task is `ready` and the label is + * on the issue, so a human can start it by hand or a later sweep can pick it up. It + * is worth a warning because it stalls the claim silently otherwise. + */ +function dispatchStage({ repo, taskId, stage, log, warn }) { + const r = spawnSync('gh', ['workflow', 'run', 'loop.yml', '-R', repo, + '-f', `task=${taskId}`, '-f', `stage=${stage}`], { encoding: 'utf8' }); + if (r.status !== 0) { + warn(`stage=${stage} not dispatched for task #${taskId}: ` + + `${(r.stderr || '').trim()} — the task is claimable, start it by hand or let a` + + ' sweep pick it up'); + return false; + } + log(`dispatched stage=${stage} for task #${taskId} (the loop cannot trigger itself` + + ' with a label, so it asks explicitly)'); + return true; +} + async function doRun({ decision, ctx, repo, writeLevel }) { const stage = decision.stage; const isSystem = !decision.taskId; @@ -336,10 +364,24 @@ async function doRun({ decision, ctx, repo, writeLevel }) { } if (done) log(`task #${done.id} → status=${done.status}${done.labels?.length ? `, label=${done.labels.join(',')}` : ''}`); + // Triage decided this is loop work → start the stage that implements it. Only + // issues: a triaged *PR* would have the loop push a branch and label the PR it is + // supposed to be reviewing. + let handedOff = null; + if (finalOutcome === 'triaged' && done?.kind === 'issue') { + try { + const cur = ghJson(['issue', 'view', String(done.id), '-R', repo, '--json', 'labels']); + const next = stageForIssue(cur.labels); + if (dispatchStage({ repo, taskId: done.id, stage: next, log, warn })) handedOff = next; + } catch (e) { + warn(`could not read task #${done.id} labels to start the next stage: ${e.message}`); + } + } + return { state: 'run', taskId: done?.id ?? decision.taskId ?? null, runId: rid, stage, outcome: finalOutcome, exit: cls.exit, actions, usage, toolCalls, turns, sessionDir, - system: isSystem, reportFile: path.join(S.reportsDir, `${rid}.md`), + system: isSystem, handedOff, reportFile: path.join(S.reportsDir, `${rid}.md`), }; } @@ -381,6 +423,10 @@ function renderStepSummary({ decision, result, writeLevel }) { L.push('', head, ''); for (const d of describeActions(result.actions)) L.push(`- [${writeLevel === 'auto' ? 'x' : ' '}] ${d}`); } + if (result?.handedOff) { + L.push('', `- handed off to \`${result.handedOff}\` (dispatched; the loop cannot trigger` + + ' itself with a label)'); + } return L.join('\n'); } diff --git a/scripts/loop/gh-check.mjs b/scripts/loop/gh-check.mjs index d710bc4..759bdff 100644 --- a/scripts/loop/gh-check.mjs +++ b/scripts/loop/gh-check.mjs @@ -235,15 +235,16 @@ const canPushBranches = probeBranch }, token) : null; +// The hand-off needs `actions: write`. Probe an *impossible* workflow id: a scope the +// credential lacks entirely answers `403`, while `404` means it has the scope and the +// id simply does not exist. Nothing is dispatched either way. +const dispatchProbe = gh(['api', '--method', 'POST', + `repos/${repo}/actions/workflows/${IMPOSSIBLE}/dispatches`, '-f', 'ref=main'], { token }); +const canDispatch = hasAnyScope(dispatchProbe.status); +console.log(` ${canDispatch ? 'present' : 'ABSENT '} Actions: write (dispatch the next stage) ` + + `(HTTP ${dispatchProbe.status ?? 'n/a'})`); + console.log('\n[loop:gh] scopes the loop should NOT have:'); -const dangerous = [ - ['Actions: write (trigger/disable workflows)', ['api', '--method', 'POST', - `repos/${repo}/actions/workflows/${IMPOSSIBLE}/dispatches`, '-f', 'ref=main']], -]; -for (const [label, args] of dangerous) { - const r = gh(args, { token }); - console.log(` ${hasAnyScope(r.status) ? 'present' : 'ABSENT '} ${label} (HTTP ${r.status ?? 'n/a'})`); -} // Read-only probe: listing secret *names* requires the Secrets permission. The // listing itself is never printed — only whether the permission is present. const secrets = gh(['api', `repos/${repo}/actions/secrets`], { token }); @@ -265,6 +266,9 @@ console.log(`[loop:gh] label/comment/close: ${canLabel ? 'available' : 'NOT available — the loop cannot do its job'}`); console.log(`[loop:gh] branch push: ${canPushBranches ? 'available' : 'NOT available'}` + ' (product stages need it; `main` is excluded by the ruleset, not by missing scope)'); +console.log(`[loop:gh] stage hand-off: ${canDispatch ? 'available — triage can start the' + + ' implementing stage' : 'NOT available — triage will leave the task claimable instead' + + ' of starting it (needs `actions: write`)'}`); console.log(`[loop:gh] reads: ${brokenReads.length ? `${brokenReads.length} FAILED — the host's sweep will break: ${brokenReads.map((r) => r.label.trim().split(/\s+/)[0]).join(', ')}` : 'all OK'}`); diff --git a/scripts/loop/shared/route.mjs b/scripts/loop/shared/route.mjs index c692880..4cab377 100644 --- a/scripts/loop/shared/route.mjs +++ b/scripts/loop/shared/route.mjs @@ -45,6 +45,26 @@ export const isDependabotPr = (pr) => export const isExternalPr = (pr) => !OWNER_ROLES.includes(pr?.author_association ?? ''); +/** + * The claim signal: the label that says "the loop may implement this". + * + * It is `ready-for-agent` because triage already applies exactly that on a + * `bug`/`feature` verdict of high or medium confidence (`skills/triage/SKILL.md` + * step 5), so the hand-off needs no new vocabulary. Applied by a *human* it also + * works as a manual trigger for any issue, which is why the route exists. + */ +export const CLAIM_LABEL = 'ready-for-agent'; + +/** Labels that pick `bugfix` over `feature`. `bug` is the only signal the repo carries. */ +const BUG_LABELS = ['bug']; + +/** Label names, from either `string[]` or the API's `{ name }[]` shape. */ +const labelNames = (labels = []) => labels.map((l) => (typeof l === 'string' ? l : l?.name)); + +/** Which implementation stage a claimed task starts. */ +export const stageForIssue = (labels = []) => + labelNames(labels).some((n) => BUG_LABELS.includes(n)) ? 'bugfix' : 'feature'; + /** * Idempotency key: skip an event whose key has already been consumed. * Prefer immutable identifiers (PR head sha / comment id), falling back to the @@ -77,6 +97,25 @@ export function route({ eventName, action, event = {} }) { if (action === 'edited') { return inbox({ taskId: n, kind: 'issue', stage: 'triage', reason: 'issues.edited' }); } + // Claim signal: someone decided this is loop work and said so on the issue. + // Only `ready-for-agent` qualifies — the other four role labels are states, not + // requests, and reacting to them would make the loop chase its own writing. + // + // Note this branch fires only for labels a *human* applies: GitHub suppresses + // runs for events raised by `GITHUB_TOKEN`, so the loop's own label is invisible + // here and the automatic hand-off is a dispatch from entry.mjs instead. + if (action === 'labeled') { + const label = event.label?.name; + if (label !== CLAIM_LABEL) { + return noop(`issues.labeled (${label ?? 'unknown'}) is not a claim signal`); + } + return run({ + taskId: n, + kind: 'issue', + stage: stageForIssue(event.issue?.labels), + reason: `issues.labeled ${CLAIM_LABEL}`, + }); + } return noop(`issues.${action} has no Loop action`); }