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`); }