From c58ff83a6ad7b03286c666189d9d196d76fdbe75 Mon Sep 17 00:00:00 2001 From: Alex Chao Date: Tue, 15 Sep 2026 23:34:56 +0800 Subject: [PATCH 1/5] =?UTF-8?q?chore(loop):=20implement=20L2c=20=E2=80=94?= =?UTF-8?q?=20the=20host=20pushes=20the=20branch,=20the=20agent=20never=20?= =?UTF-8?q?can?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The A1 report boundary proved the loop is safe but left the write half of the host unexecuted: with no credential, every proposal died in the report file. This lands the L2c split the observation week was gating, so the loop can produce a PR without ever being able to merge one. Three credentials, three holders: github.token agent, report read-only (permissions:) reading LOOP_GH_TOKEN agent, auto Issues+PR write, no Contents labels, PR LOOP_PUSH_TOKEN host process Contents write branch push `agentEnv` builds the child environment instead of inheriting it: it deletes the push credential, the PAT and the R2 keys, and sets GH_TOKEN to the least token the level allows. `persist-credentials: false` is what makes that hold — checkout's default writes the job token into the repo's git config, where the agent's own `git push` would find it and no environment can un-write a file. `pr-opened` now plans push → pr-create → label/comment, executed by the host from the agent's proposal in result.json. A failed push or PR creation aborts the rest, and the outcome is downgraded before it is recorded, so a task cannot wait in `waiting-merge` for a PR that does not exist. The host pushes nothing but `loop/-`, refuses a dirty tree (the commit would stay behind and the PR would ship an incomplete diff), and repairs the `Closes #n` / `loop-task: #n` markers the router pairs with the branch — a missing marker would make the loop's own PR look external, costing it the acceptance row. `validatePush` rejects an unbacked `pr-opened` in `entry.mjs` for the same reason. The PR number that `gh pr create` returns is recorded in `task.prs` with a `pr-created` timeline row. Verification (local, four throwaway harnesses, all green): - `applyActions` against a local bare repo (git's url.insteadOf standing in for GitHub): the push lands, and the refusals fire for a dirty tree, a branch outside `loop/`, and a missing push credential — with the label correctly skipped after a failed PR creation. - `agentEnv`: the push credential is absent at both write levels, the report level hands over the read-only token, and `auto` injects a git identity for the local commit. - `finishRun` success path through the new `exec` seam: push → PR #4242 → label → comment, `waiting-merge` with `ready-for-human`, PR number and timeline recorded, lock released, markers present in the PR body. - `entry.mjs` end to end with a stub engine: the branch really lands, and each fail-closed path (bad branch name, report boundary, no push token, live GitHub refusing the PR) downgrades to `failed` with the task in the inbox. The GitHub `auto` path itself has never run on Actions: that is the first thing to sample once `LOOP_EXECUTE_WRITES=true` and the two secrets exist. --- .github/workflows/loop.yml | 26 ++++- scripts/loop/README.md | 126 +++++++++++++++------- scripts/loop/entry.mjs | 65 ++++++++++-- scripts/loop/gh-check.mjs | 98 +++++++++++++---- scripts/loop/shared/agent.mjs | 95 ++++++++++++++--- scripts/loop/shared/state.mjs | 192 ++++++++++++++++++++++++++++++++-- 6 files changed, 512 insertions(+), 90 deletions(-) diff --git a/.github/workflows/loop.yml b/.github/workflows/loop.yml index b9d5007..faec662 100644 --- a/.github/workflows/loop.yml +++ b/.github/workflows/loop.yml @@ -50,9 +50,11 @@ concurrency: group: loop cancel-in-progress: false -# Least privilege: GitHub reads use GITHUB_TOKEN; writes (only the -# execute_writes=true rehearsal) go through LOOP_GH_TOKEN and exist only in -# default-branch contexts — fork events carry no secrets (platform guarantee). +# Least privilege: GitHub reads use GITHUB_TOKEN; the agent's own writes go through +# LOOP_GH_TOKEN, and branch pushes through LOOP_PUSH_TOKEN — neither of which the +# agent ever receives (see runPi/agentEnv). `github.token` stays read-only on +# purpose: it is the credential the agent holds under the report boundary, so +# granting the job `contents: write` would hand the agent the merge permission. permissions: contents: read issues: read @@ -91,8 +93,11 @@ jobs: LOOP_EVENT_ACTION: ${{ github.event.action }} LOOP_EVENT_JSON: ${{ toJSON(github.event) }} LOOP_REPO: ${{ github.repository }} - # For non-dispatch events github.event.inputs is null → default to report (report-only) - LOOP_EXECUTE_WRITES: ${{ github.event.inputs.execute_writes || 'false' }} + # Write level for non-dispatch events. A1 shipped `false` (report-only); L2 flips + # it by setting the repository variable LOOP_EXECUTE_WRITES=true — no code change, + # and one place for a human to turn it back off. `workflow_dispatch` can still + # override per run (the rehearsal path). + LOOP_EXECUTE_WRITES: ${{ github.event.inputs.execute_writes || vars.LOOP_EXECUTE_WRITES || 'false' }} LOOP_SMOKE: ${{ github.event.inputs.smoke || 'false' }} LOOP_MODEL: ${{ github.event.inputs.model || 'deepseek/deepseek-v4-flash' }} LOOP_RUN_TIMEOUT_MS: '3300000' # 55min, leaving headroom for push (job timeout 60min) @@ -103,6 +108,10 @@ jobs: # if LOOP_GH_TOKEN is a write-capable PAT. Host-side writes still use # GH_TOKEN, in the host's own process. GH_READ_TOKEN: ${{ github.token }} + # L2c's third role: the only credential with `Contents: write`, held by the host + # process and *removed* from the agent's environment by `agentEnv`. It exists to + # push `loop/-*` and nothing else. + LOOP_PUSH_TOKEN: ${{ secrets.LOOP_PUSH_TOKEN }} # Credentials: secrets only DEEPSEEK_API_KEY: ${{ secrets.DEEPSEEK_API_KEY }} R2_ACCESS_KEY_ID: ${{ secrets.R2_ACCESS_KEY_ID }} @@ -136,6 +145,13 @@ jobs: # pull_request_target. - uses: actions/checkout@v4 with: + # L2c: its default is `true`, which writes the job token into the repository's + # git config so later steps can run authenticated git commands — including the + # agent's. That is the one credential the agent must never hold (it is the + # push/merge permission), and `agentEnv` cannot un-write a file. With this off, + # a `git push` from the agent has no credential at all; the host pushes through + # `http.extraheader` using LOOP_PUSH_TOKEN. + persist-credentials: false ref: >- ${{ github.event_name == 'workflow_dispatch' && github.ref diff --git a/scripts/loop/README.md b/scripts/loop/README.md index 84c837b..366fb57 100644 --- a/scripts/loop/README.md +++ b/scripts/loop/README.md @@ -46,33 +46,43 @@ pnpm loop summary | view # summary / task list | Level | Effect | | --- | --- | -| `report` | State layer + report only. GitHub write actions are **listed, never executed** (A1 semantics). | -| `auto` | Executes labels/comments/close (the A0 closed-loop boundary; L2 default, and the rehearsal mode). Note this is *permission* to write, not *ability*: with `LOOP_GH_TOKEN` unset, `auto` still cannot write — see the credential section below. | +| `report` | State layer + report only. GitHub write actions are **listed, never executed** (A1 semantics); the agent holds the read-only job token. | +| `auto` | Executes labels/comments/close, and for `pr-opened` the host pushes the branch and opens the PR. The agent holds `LOOP_GH_TOKEN` (Issues/Pull requests write, no `Contents`) — see the credential section below. | In `report` mode the pending actions are appended to `state/reports/.md` as a checklist and surfaced in the Actions Step Summary, so a human can execute them by hand. -### GitHub write credential (`LOOP_GH_TOKEN`, unset today) +### GitHub write credentials (three roles) -`GH_TOKEN: ${{ secrets.LOOP_GH_TOKEN || github.token }}`. The secret is -**deliberately not configured**: `github.token` is capped by the workflow's -`permissions: {contents: read, issues: read, pull-requests: read}`, so today a -run that ignores its prompt and calls `gh pr merge` gets a 403. Two independent -locks, and the platform one does not depend on the model behaving. +`GH_TOKEN: ${{ secrets.LOOP_GH_TOKEN || github.token }}`. Until L2 the secret was +deliberately unset, so `github.token` — capped read-only by the workflow's +`permissions:` block — was the only credential a run could reach, and a run that +ignored its prompt got a 403 from `gh pr merge`. That is the *report boundary* +credential; configuring the secret replaces it for every `gh` call in the job, +which is why `gh-check.mjs` measures rather than assumes. -Configuring the secret lifts only the platform lock — `writeLevel` still gates -`applyActions` in `state.mjs`, and its default is `report`, so ordinary runs -keep reporting. +There are now three credentials, and the loop's central guarantee — it opens PRs +and can never merge — rests on which process holds which: -It must not also lift the **agent's** lock. `runPi` spawns the agent with -`env: process.env`, which would hand it whatever `GH_TOKEN` holds — and since -that is now a PAT, the agent would inherit the PAT's full reach. So under the -report boundary the child gets `GH_READ_TOKEN` instead: `github.token`, which -`permissions:` caps at read. The agent therefore reaches GitHub read-only even -if it ignores its prompt, while host-side writes keep using the real `GH_TOKEN` -in the host's own process. Under `auto` the agent gets the write token, which is -the point of that mode. +| Credential | Held by | Scopes | Used for | +| --- | --- | --- | --- | +| `github.token` (`GH_READ_TOKEN`) | the agent, under `report` | read-only (`permissions:`) | reading GitHub: triage, sweep, review | +| `LOOP_GH_TOKEN` (PAT) | the agent under `auto`; the host otherwise | `Issues: RW`, `Pull requests: RW` — **no Contents** | labels, comments, closing, opening the PR | +| `LOOP_PUSH_TOKEN` (PAT) | the host process only | `Contents: RW` | pushing `loop/-*` — nothing else | + +Configuring `LOOP_GH_TOKEN` lifts only the platform lock — `writeLevel` still +gates `applyActions` in `state.mjs`, and its default is `report`, so ordinary +runs keep reporting. + +It must not also lift the **agent's** lock. `agentEnv` in `shared/agent.mjs` builds +the child environment from scratch: it deletes `LOOP_PUSH_TOKEN`, `LOOP_GH_TOKEN`, +`GH_READ_TOKEN` and the R2 keys, and sets `GH_TOKEN` to the least credential the +write level allows — `GH_READ_TOKEN` (i.e. `github.token`, capped at read by +`permissions:`) under `report`, and `LOOP_GH_TOKEN` under `auto`. The agent +therefore reaches GitHub read-only under the report boundary even if it ignores +its prompt, while host-side writes keep using the real `GH_TOKEN` in the host's +own process. Both are measurable without spending a token: `node scripts/loop/gh-check.mjs` probes each credential's scopes side-effect @@ -161,10 +171,34 @@ Server-side protection does **not** substitute for any of this: this is a solo repository (one collaborator, `admin: true`), so a ruleset requiring approvals would block the owner's own PRs forever (GitHub forbids self-approval), and adding an admin `bypass_actor` would let the loop's PAT bypass it too. The -existing `Main branch` ruleset is `enforcement: disabled` and contains only -`deletion` and `non_fast_forward` — it never restricted merging. The guarantee +`Main branch` ruleset is `enforcement: active`, but it contains only `deletion` +and `non_fast_forward`, so it never restricted merging either way. The guarantee has to come from the credential split. +#### L2c as implemented + +The three roles above are now wired end to end. What a human has to configure is +two secrets; everything else is code that fails closed. + +| Piece | Where | Behaviour | +| --- | --- | --- | +| `LOOP_PUSH_TOKEN` env | `.github/workflows/loop.yml` | The only `Contents: write` credential. Deleted from the agent's environment by `agentEnv`. | +| `persist-credentials: false` | the `actions/checkout` step | Without it the job token is written into the repo's git config, where the agent's `git push` would find it; `agentEnv` cannot un-write a file. | +| `vars.LOOP_EXECUTE_WRITES` | job env | `workflow_dispatch` still overrides per run, so the A1 rehearsal entry point is unchanged. | +| `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. | +| `validatePush` | `entry.mjs` | `pr-opened` without a valid proposal (wrong branch, empty title, or `writeLevel=report`) is downgraded to `failed`, so a task can never sit in `waiting-merge` waiting for a PR nobody opened. | +| `ensureLoopPrBody` | `planActions` | Repairs the PR body's `Closes #` / `loop-task: #` markers. The router requires both, and a missing marker would make the loop's own PR look external — no acceptance row, and its merge would close the task by the wrong path. | +| PR number recorded | `finishRun` | The number comes back from `gh pr create` and lands in `task.prs` + a `pr-created` timeline row. | +| git identity | `agentEnv` | `GIT_AUTHOR_*`/`GIT_COMMITTER_*` are injected under `auto`; a fresh runner has no global git config, and the commit itself is local (no credential involved). | + +The failure that remains possible is deliberate: with `auto` on and +`LOOP_PUSH_TOKEN` unset, every product stage fails at the push and the task goes +to the inbox. The alternative — falling back to the agent's credential — would +also hand the agent the merge permission, which is the one thing L2c exists to +prevent. + ## A1 architecture (Actions + R2) ``` @@ -181,9 +215,11 @@ GitHub event ─▶ loop.yml (concurrency group `loop` = platform-level single w built in (`DEEPSEEK_API_KEY`), ~21 MB with no install scripts, Node >= 22.19. `--approve` is **mandatory**: without it non-interactive runs silently ignore project-local resources. -- **pi has no built-in permission gating** (verified). The job's `permissions:` - block and the absence of secrets in fork contexts are the whole boundary — - do not add write credentials to a job that touches untrusted input. +- **pi has no built-in permission gating** (verified), so the boundary is built + from the environment instead: the job's `permissions:` block, the absence of + secrets in fork contexts, and `agentEnv` (which strips `LOOP_PUSH_TOKEN`, the + PAT and the R2 keys from the child). Do not add write credentials to a job + that touches untrusted input, and do not rely on the prompt for any of it. - **One writer for closing:** the agent writes `state/reports/.result.json`; `entry.mjs` performs the transition via `finishRun`. Agents never run `pnpm loop end` themselves. @@ -393,6 +429,22 @@ design, so a task whose file says `needs-triage` and whose GitHub item has no label is **expected**, not drift. What must be judged is whether each proposal was *correct*, and whether new events produce proposals that match reality. +**Observation week closed 2026-09-14** (report boundary, no GitHub writes). The +result that matters is on the safety side: 25 recorded runs, zero `retry` / +`failed` / `aborted`, no drift between GitHub and the state layer — four +consecutive sweeps found the open items and the non-terminal tasks in exact 1:1 +agreement — and **no incorrect proposal** against an item the loop did not +control. Cost was flat (triage 58–82k tokens, sweep 60–82k) with no +context-growth drift, and the credential probes measured what the host and the +agent actually hold rather than assuming it. + +What the week did *not* prove is that the loop is **complete**, and that is the +honest reason L2c is being landed now: with no write credential every proposal +was unexecutable, so the write half of the host (`applyActions`, +`planActions`'s `pr-create`, the acceptance gate) never met production. The +unchecked boxes below are exactly that half, and they are the first items to +sample once `auto` is switched on. + - [x] `workflow_dispatch` smoke (`-f smoke=true`): pipeline green end to end (checkout → pi install → R2 pull → push) with no agent run — run `34509932312`, 51s; re-run `34511261607` also confirmed the seeded state @@ -463,12 +515,15 @@ was *correct*, and whether new events produce proposals that match reality. `Install engine (pi)`, before `entry.mjs`, so they write no run row at all — see the failure-classification item above. (Distinction worth keeping: 21 workflow executions, 18 run records.) -- [ ] No drift **and** no incorrect proposal after a week → human decides on L2. - Owner's target is **L2c**: the loop opens PRs but can never merge. That - needs the credential split (host pushes branches with `GITHUB_TOKEN`; the - agent's PAT carries no `Contents`) plus `persist-credentials: false` — see - "GitHub write credential" above for the endpoint-level evidence, and note - that a single credential cannot express it. +- [x] No drift **and** no incorrect proposal for a week → **owner decided L2c** + (2026-09-14): the loop opens PRs but can never merge. The split is + implemented — the host pushes branches with `LOOP_PUSH_TOKEN` and opens the + PR, the agent's PAT carries no `Contents`, and `persist-credentials: false` + keeps the job token out of the agent's reach; see "L2c as implemented" and + the endpoint-level evidence under "GitHub write credentials". What remains + is configuration, not code: the two secrets above must exist before + `LOOP_EXECUTE_WRITES=true`, and the unchecked boxes in this list become + sampleable only then. ### Not reachable under A1 (structural — decide before the L2 switchover) @@ -478,16 +533,17 @@ validated at the report boundary however long it runs: | Capability | Why it cannot run | Disposition | | --- | --- | --- | -| `metrics` write path | A1 produces no loop PR, so `handleMetrics` never fires | accepted unverified; first item at L2 | -| `pr-review` route | also needs a loop PR; A1 never produces one | untested here — exercised in A0 | +| `metrics` write path | A1 produces no loop PR, so `handleMetrics` never fires | L2c makes it reachable; **still unverified — first item once `auto` is on** | +| `pr-review` route | also needs a loop PR; A1 never produces one | L2c makes it reachable; exercised in A0, never on Actions | | External-PR read-only analysis | D28 — a fork/Dependabot `pull_request` run carries no secrets, so not even a read-only analysis can reach the LLM | accepted for A1; needs its own credentials to enable | -| Label/comment effects on GitHub | the report boundary never writes | proposals must be judged on *correctness*, not on effect | -| `execute_writes` rehearsal | `LOOP_GH_TOKEN` is unset, so `auto` cannot write even when requested | correct as defence in depth — but it means **no write path has ever executed**. Configuring the PAT is a prerequisite for testing any of it; see "GitHub write credential" | +| Label/comment effects on GitHub | the report boundary never writes | L2c executes them; the first `auto` run is what turns "correct on paper" into "observed" | +| `execute_writes` rehearsal | `LOOP_GH_TOKEN` is unset, so `auto` cannot write even when requested | the code path is now exercised locally (push → PR → label, plus the dirty-tree, branch-shape and missing-credential refusals); it has never run on Actions | | R2 bucket versioning | R2 offers no object versioning | accepted deviation from 05 §8 (see Environment facts); `push`/`seed` never use `--delete` | Consequence for the L2 decision: a clean observation week proves the loop is **safe**, not that it is **complete**. The write half of the host is the part -that has never met production. +that has never met production, which is why L2c lands as its own change and why +the first `auto` run is a verification step rather than a milestone. ## Defect log diff --git a/scripts/loop/entry.mjs b/scripts/loop/entry.mjs index cdb559f..3a7707c 100644 --- a/scripts/loop/entry.mjs +++ b/scripts/loop/entry.mjs @@ -31,7 +31,7 @@ import { makeState, ensureDirs, readJson, readJsonl, writeJson, saveTask, listTasks, appendAcceptance, markTaskTerminal, GATE_TRANSITIONS, renderStateSummary, beginRun, finishRun, lockExpired, - nowIso, outcomeAllowed, allowedOutcomes, describeActions, + nowIso, outcomeAllowed, allowedOutcomes, describeActions, LOOP_BRANCH_RE, } from './shared/state.mjs'; import { route, eventKey } from './shared/route.mjs'; import { buildPrompt, runPi, parseEvents, summarize, classify } from './shared/agent.mjs'; @@ -201,6 +201,40 @@ async function handleTaskTerminal({ decision }) { /* ------------------------------------------------------------------ run */ +/** + * A `pr-opened` outcome is a claim the host has to back with a push, so it is checked + * before it can move a task to `waiting-merge`. An unbacked claim is the D33 failure + * mode in a new place: the task would sit in the inbox forever waiting for a PR that + * never existed, and nothing in the loop could see that. + * + * The branch shape is enforced here as well as in `applyActions` (which re-checks it + * against the credential it is about to spend) because a mismatch is an agent error + * worth reporting as `failed`, not a silent no-op. + */ +function validatePush({ writeLevel, stage, taskId, push }) { + if (writeLevel !== 'auto') { + return { ok: false, reason: + `stage=${stage} cannot open a PR at writeLevel=${writeLevel}: the report boundary ` + + 'forbids pushing a branch. The stage must report what it produced instead.' }; + } + if (!push || typeof push !== 'object') { + return { ok: false, reason: 'no "push" proposal in the result file (branch/title missing)' }; + } + const branch = String(push.branch ?? ''); + if (!LOOP_BRANCH_RE.test(branch)) { + return { ok: false, reason: + `branch "${branch}" does not match loop/- (ops.md; ` + + 'the host pushes nothing else)' }; + } + if (!branch.startsWith(`loop/${taskId}-`)) { + return { ok: false, reason: + `branch "${branch}" does not carry this task's number — the router pairs the ref ` + + `with the body marker, so it must start with loop/${taskId}-` }; + } + if (!String(push.title ?? '').trim()) return { ok: false, reason: 'the PR title is empty' }; + return { ok: true, push: { ...push, branch } }; +} + async function doRun({ decision, ctx, repo, writeLevel }) { const stage = decision.stage; const isSystem = !decision.taskId; @@ -253,7 +287,7 @@ async function doRun({ decision, ctx, repo, writeLevel }) { const result = await readJson(path.join(S.reportsDir, `${rid}.result.json`)); const cls = classify({ code: res.code, signal: res.signal, stderr: res.stderr, timedOut: res.timedOut, stopReason }); - let outcome; let note; + let outcome; let note; let push = null; if (result?.outcome && outcomeAllowed(stage, result.outcome)) { outcome = result.outcome; note = result.note ?? `agent decision (exit=${res.code})`; @@ -268,16 +302,31 @@ async function doRun({ decision, ctx, repo, writeLevel }) { warn(`no ${rid}.result.json found — ${note}`); } - const { task: done, actions } = await finishRun(S, { + // `pr-opened` must be backed by a branch the host can actually push. + if (outcome === 'pr-opened') { + const v = validatePush({ writeLevel, stage, taskId: task.id, push: result.push }); + if (v.ok) push = v.push; + else { + outcome = 'failed'; + note = `pr-opened refused by the host: ${v.reason}`; + warn(note); + } + } + + const { task: done, actions, outcome: finalOutcome } = await finishRun(S, { runId: rid, outcome, note, comment: result?.comment ?? null, decision: result?.decision ?? null, pr: result?.pr ?? null, + // The agent's PR proposal; only `pr-opened` uses it, and the host (not the agent) + // pushes the branch and opens the PR — see planActions/applyActions. + push, tokens: usage?.totalTokens ?? null, - writeLevel, log, warn, + writeLevel, cwd: ROOT, log, warn, }); - log(`run ${rid} finished: outcome=${outcome} (exit ${cls.exit})` + log(`run ${rid} finished: outcome=${finalOutcome} (exit ${cls.exit})` + + `${finalOutcome !== outcome ? ` [agent said pr-opened, the host downgraded it]` : ''}` + `${usage?.totalTokens ? `, tokens=${usage.totalTokens}` : ''}`); if (actions.length) { log(writeLevel === 'auto' @@ -288,9 +337,9 @@ async function doRun({ decision, ctx, repo, writeLevel }) { if (done) log(`task #${done.id} → status=${done.status}${done.labels?.length ? `, label=${done.labels.join(',')}` : ''}`); return { - state: 'run', taskId: done?.id ?? decision.taskId ?? null, runId: rid, stage, outcome, - exit: cls.exit, actions, usage, toolCalls, turns, sessionDir, system: isSystem, - reportFile: path.join(S.reportsDir, `${rid}.md`), + 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`), }; } diff --git a/scripts/loop/gh-check.mjs b/scripts/loop/gh-check.mjs index 13141f4..3ff6b9a 100644 --- a/scripts/loop/gh-check.mjs +++ b/scripts/loop/gh-check.mjs @@ -158,6 +158,26 @@ if (hostPerm.ok) { console.log(` (info) repo .permissions = ${hostPerm.out.replace(/\s+/g, ' ')}` + ' — the *user\'s* role, not this token\'s scopes; do not read it as a scope report'); } +// The credential `GH_TOKEN` resolves to is also the agent's under `auto` (agentEnv +// hands the child `LOOP_GH_TOKEN`), so a PAT with Contents here is the one +// configuration that would let the agent merge: measure it, do not assume it. + +// L2c's third role. This is the *only* credential that may hold Contents, it belongs +// to the host process (applyActions' push), and `agentEnv` deletes it from the child +// environment — a push is the one write that cannot be delegated to the agent, because +// the same scope that creates a branch also merges one. +console.log('\n[loop:gh] push credential (LOOP_PUSH_TOKEN — host only, absent from the agent env):'); +const pushToken = process.env.LOOP_PUSH_TOKEN || null; +let canPushBranch = null; +if (!pushToken) { + console.log(' (not set — the host cannot push, so no product stage can reach `pr-opened`;' + + ' set it from a PAT with Contents: Read and write, or set LOOP_EXECUTE_WRITES=true only' + + ' after it exists)'); +} else { + canPushBranch = idempotentWrite( + 'Contents: write (branch push — the host\'s job)', refProbe, pushToken, + ); +} console.log('\n[loop:gh] scopes reached by a credential that lacks them entirely:'); const absent = [ @@ -174,34 +194,74 @@ const secrets = gh(['api', `repos/${repo}/actions/secrets`], { token: hostToken console.log(` ${secrets.ok ? 'present' : 'ABSENT '} Secrets access (list/overwrite CI secrets)`); const brokenReads = readResults.filter((r) => !r.ok); +const writeLevel = process.env.LOOP_WRITE_LEVEL + ?? (process.env.LOOP_EXECUTE_WRITES === 'true' ? 'auto' : 'report'); console.log(''); if (canPush) { - console.log('[loop:gh] WARNING: the host holds Contents:write. That single scope grants merging' - + ' (PUT /pulls/{n}/merge), branch pushes and POST /releases — none of which the loop needs' - + ' until L2. See "GitHub write credential" in scripts/loop/README.md. Recreate the PAT with:' - + ' Issues Read and write, Pull requests Read and write, Contents Read-only, and Nothing for' - + ' Actions/Secrets/Administration/Workflows.'); + console.log('[loop:gh] WARNING: the host credential holds Contents:write, and under `auto`' + + ' the agent is handed that same credential — so the agent could merge (PUT' + + ' /pulls/{n}/merge), push branches and cut releases. L2c keeps that scope out of the' + + ' agent\'s reach: the host pushes with LOOP_PUSH_TOKEN, and LOOP_GH_TOKEN is scoped to' + + ' Issues + Pull requests only. See "GitHub write credential" in scripts/loop/README.md;' + + ' recreate the PAT with Issues Read and write, Pull requests Read and write, Contents' + + ' Read-only, and Nothing for Actions/Secrets/Administration/Workflows.'); } else if (canPush === false) { - console.log('[loop:gh] host holds no Contents:write — merging, branch pushes and releases are' - + ' impossible for it.'); + console.log('[loop:gh] host credential holds no Contents:write — merging, branch pushes and' + + ' releases are impossible for it, which is the L2c invariant.'); } console.log(`[loop:gh] label/comment/close: ${canLabel ? 'available' : 'NOT available — the loop cannot do its job'}`); 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'}`); +console.log(`[loop:gh] write level (this configuration): ${writeLevel}`); +if (writeLevel === 'auto' && !canLabel) { + console.log(' WARNING: `auto` is switched on but the credential cannot label/comment — the loop' + + ' will run, propose, and silently fail to write. Configure LOOP_GH_TOKEN first.'); +} +if (writeLevel === 'auto' && !pushToken) { + console.log(' WARNING: `auto` is switched on but LOOP_PUSH_TOKEN is unset — no product stage can' + + ' produce a PR. Either set it, or keep LOOP_EXECUTE_WRITES=false until it exists.'); +} else if (writeLevel === 'auto' && canPushBranch === false) { + console.log(' WARNING: LOOP_PUSH_TOKEN lacks Contents:write — every product stage will fail at' + + ' the host\'s push. That credential needs Contents: Read and write on this repository only.'); +} -// The credential the *agent* will hold. `runPi` swaps GH_TOKEN for GH_READ_TOKEN -// under the report boundary, so this decides whether the agent could write if it -// ignored its prompt. +// What the agent actually holds is decided by `agentEnv(writeLevel)`, and it is worth +// measuring rather than reading off the workflow file, because `GH_TOKEN` is +// `secrets.LOOP_GH_TOKEN || github.token` — which one resolves is invisible here. // -// Identity cannot be read from `GET /user` here: `github.token` is a GitHub App -// installation token and is refused there entirely, which is why the check below -// combines two signals that *are* observable — whether the credential can read -// the repository at all, and whether a write is refused. -console.log('\n[loop:gh] credential handed to the agent under the report boundary:'); -if (!agentToken) { - console.log(' (GH_READ_TOKEN is not set — runPi would pass GH_TOKEN through unchanged,' - + ' i.e. the agent inherits the host credential above.)'); +// Identity cannot be read from `GET /user` for `github.token`: it is a GitHub App +// installation token and is refused there entirely, which is why these checks combine +// two signals that *are* observable — whether the credential can read the repository, +// and whether a write is refused. +console.log('\n[loop:gh] credential handed to the agent:'); +if (writeLevel === 'auto') { + if (!hostToken) { + console.log(' (no LOOP_GH_TOKEN and no GH_TOKEN — gh in the agent would use its own login.)'); + } else { + const canRead = gh(['api', `repos/${repo}`, '--jq', '.full_name'], { token: hostToken }); + const asUser = gh(['api', 'user', '--jq', '.login'], { token: hostToken }); + console.log(` can read the repo: ${canRead.ok ? 'yes' : `NO — the agent cannot read GitHub (${canRead.status})`}`); + console.log(` acts as a user: ${asUser.ok ? `yes (${asUser.out}) — a PAT` : 'no — an App token (github.token)'}`); + const agentWrite = idempotentWrite('Issues: write (labels/comments)', issueProbe, hostToken); + idempotentWrite('Pull requests: write', prProbe, hostToken); + const agentPush = idempotentWrite('Contents: write (must be ABSENT)', refProbe, hostToken); + if (!canRead.ok) { + console.log(' WARNING: the agent cannot read GitHub, so triage/sweep will find nothing.'); + } else if (agentPush) { + console.log(' WARNING: the agent holds Contents:write — it can push AND merge, so "no' + + ' self-merge" is a prompt instruction only. L2c requires a PAT without Contents.'); + } else if (!agentWrite) { + console.log(' WARNING: the agent holds no Issues:write — it cannot label or comment, so every' + + ' `auto` write will fail. `auto` needs the PAT (LOOP_GH_TOKEN); github.token is read-only.'); + } else { + console.log(' OK: the agent can label/comment/open PRs but holds no Contents — it cannot push' + + ' or merge even if it ignores its prompt.'); + } + } +} else if (!agentToken) { + console.log(' (GH_READ_TOKEN is not set, so agentEnv leaves the child without GH_TOKEN — `gh`' + + ' there would fall back to any local login. On Actions this variable is always set.)'); } else { const canRead = gh(['api', `repos/${repo}`, '--jq', '.full_name'], { token: agentToken }); const asUser = gh(['api', 'user', '--jq', '.login'], { token: agentToken }); @@ -214,7 +274,7 @@ if (!agentToken) { console.log(' WARNING: the agent cannot read GitHub, so triage/sweep will find nothing.'); } else if (asUser.ok) { console.log(` WARNING: the agent holds a *user* token (${asUser.out}) — it inherits that` - + " user's reach, so the report boundary is a prompt instruction only."); + + ' user\'s reach, so the report boundary is a prompt instruction only.'); } else if (agentWrite || agentPush) { console.log(' WARNING: the agent can write (github.token is not capped to read — check the' + " workflow's `permissions:` block)."); diff --git a/scripts/loop/shared/agent.mjs b/scripts/loop/shared/agent.mjs index 7212926..f4dae67 100644 --- a/scripts/loop/shared/agent.mjs +++ b/scripts/loop/shared/agent.mjs @@ -61,7 +61,23 @@ export function buildPrompt({ task, stage, runId, writeLevel, mode, repo, lead = L.push('(labels, comments, closing, opening a PR) must be written into the report'); L.push('as a proposal for a human to execute.'); } else { - L.push('Write actions are permitted per ops.md (labels/comments/PR as the stage requires).'); + L.push('You may write to GitHub through `gh` — **but only** labels, comments and'); + L.push('closing (`gh issue edit|comment|close`, `gh pr edit|comment`).'); + L.push('Two steps belong to the host, not to you:'); + L.push(''); + L.push('- **Pushing the branch.** Your credential carries no `Contents: write` (that is'); + L.push(' also the merge permission, so by design you do not hold it; `git push` and'); + L.push(' `gh pr merge` answer 403 — do not attempt them).'); + L.push('- **Opening the PR.** You propose it; the host pushes and creates it.'); + L.push(''); + L.push('For a product stage (`bugfix`/`feature`/`deps`/`security`) that produces a change:'); + L.push('1. Do the work in the working tree, then **commit it locally** on a branch named'); + L.push(' `loop/-` (e.g. `git switch -c loop/33-ci-workflow`). Commits'); + L.push(' are local and need no network credential; set `user.name`/`user.email` if git asks.'); + L.push('2. Leave the tree clean — `git status --porcelain` must be empty. The host refuses'); + L.push(' to push a dirty tree, since the branch would silently lose the uncommitted work.'); + L.push('3. Propose the PR in your result file (closing ritual below). The host pushes the'); + L.push(' branch, opens the PR and records the PR number it gets back.'); } if (mode === 'readonly') { L.push(''); @@ -81,6 +97,17 @@ export function buildPrompt({ task, stage, runId, writeLevel, mode, repo, lead = L.push(' ```'); L.push(' The orchestrator reads this file and performs the state transition — one writer'); L.push(' keeps the state layer consistent. Do NOT run `pnpm loop end` yourself.'); + if (writeLevel === 'auto') { + L.push(' For a product stage, add the PR proposal the host will execute:'); + L.push(' ```json'); + L.push(' "push": { "branch": "loop/-", "base": "main",'); + L.push(' "title": "",'); + L.push(' "body": "Closes #\\n\\nloop-task: #\\n\\n" }'); + L.push(' ```'); + L.push(' The body must pass the loop-PR test in `docs/agents/ops.md` (`Closes #` and'); + L.push(' `loop-task: #`). Omit `push` when the stage produced no branch — triage,'); + L.push(' sweep, or a `needs-info`/`needs-triage` verdict never pushes anything.'); + } if (task) { L.push(` Allowed outcomes for stage \`${stage}\` are declared in \`scripts/loop/shared/state.mjs\``); L.push(' (STAGE_OUTCOMES). Never invent one.'); @@ -125,6 +152,47 @@ export function summarize(events) { */ export const SESSION_HINT = 'session files land under --session-dir (uploaded as an artifact)'; +/** + * The environment handed to the agent subprocess. + * + * `{ ...process.env }` used to give the child everything the host holds, including + * the push credential and the R2 keys — neither of which any stage needs, and the + * push credential is exactly the one that carries `Contents: write` (i.e. merge). + * The agent reaches GitHub through `GH_TOKEN` only, set to the least credential the + * write level allows: + * + * `report` → `GH_READ_TOKEN` (github.token, capped at read by the workflow's + * `permissions:` block — a platform guarantee, not a prompt one) + * `auto` → `LOOP_GH_TOKEN` (the PAT: Issues + Pull requests write, **no + * Contents**), so the L2c loop can label/comment but never push or + * merge, even if it ignores its prompt + * + * Everything else that authenticates to something is deleted. Locally both tokens + * are unset, so no `GH_TOKEN` key survives and `gh` falls back to the developer's + * own login — which is the correct behaviour for a local run. + */ +export function agentEnv(writeLevel, base = process.env) { + const env = { ...base }; + for (const k of [ + 'LOOP_PUSH_TOKEN', 'LOOP_GH_TOKEN', 'GH_READ_TOKEN', 'GITHUB_TOKEN', + 'R2_ACCESS_KEY_ID', 'R2_SECRET_ACCESS_KEY', 'R2_ACCOUNT_ID', 'R2_BUCKET', + ]) delete env[k]; + const token = writeLevel === 'auto' ? (base.LOOP_GH_TOKEN ?? base.GH_TOKEN) : base.GH_READ_TOKEN; + if (token) env.GH_TOKEN = token; + else delete env.GH_TOKEN; + // Product stages commit the branch the host pushes, and the identity must not come + // from the sandbox's global git config (there is none on a fresh runner). Set it + // here rather than making the agent discover it: a commit is a local operation, and + // these four variables are the only thing git needs for one. + if (writeLevel === 'auto') { + env.GIT_AUTHOR_NAME = env.GIT_AUTHOR_NAME ?? 'tiny-oss loop'; + env.GIT_AUTHOR_EMAIL = env.GIT_AUTHOR_EMAIL ?? 'loop@users.noreply.github.com'; + env.GIT_COMMITTER_NAME = env.GIT_COMMITTER_NAME ?? env.GIT_AUTHOR_NAME; + env.GIT_COMMITTER_EMAIL = env.GIT_COMMITTER_EMAIL ?? env.GIT_AUTHOR_EMAIL; + } + return env; +} + export function runPi({ prompt, cwd, sessionDir, model, timeoutMs = 3600000, writeLevel = 'report', log = () => {}, }) { @@ -143,19 +211,18 @@ export function runPi({ // another CLI or wrapper script). const engine = (process.env.LOOP_ENGINE_CMD || 'pi').split(/\s+/).filter(Boolean); - // `env: process.env` hands the agent everything the host sees, including - // `GH_TOKEN` — which is `secrets.LOOP_GH_TOKEN || github.token`, so as soon - // as a PAT is configured the agent inherits whatever that PAT can do. Under - // the report boundary nothing the agent does is supposed to reach GitHub, - // and that must not rest on the prompt alone: hand it the job token instead, - // which the workflow caps at `read-only` via `permissions:`. Writes by the - // *host* (applyActions) still use the real GH_TOKEN in its own process. - const childEnv = { ...process.env }; - if (writeLevel !== 'auto' && process.env.GH_READ_TOKEN) { - childEnv.GH_TOKEN = process.env.GH_READ_TOKEN; - log('[loop] agent is given a read-only GitHub token (report boundary)'); - } else if (writeLevel === 'auto') { - log('[loop] agent is given the write-capable GitHub token (auto mode)'); + // The agent never inherits a credential by accident: see `agentEnv` above for + // what it holds at each write level and why the push credential is absent from + // both. Host-side writes (applyActions) still use the real GH_TOKEN in the + // host's own process. + const childEnv = agentEnv(writeLevel); + if (childEnv.GH_TOKEN) { + log(writeLevel === 'auto' + ? '[loop] agent GitHub credential: the PAT (Issues/Pull requests write, no Contents)' + : '[loop] agent GitHub credential: the read-only job token (report boundary)'); + } else { + log('[loop] agent GitHub credential: none at this write level — `gh` there would fall' + + ' back to any local login, so the run may not be able to read GitHub at all'); } const child = spawn(engine[0], [...engine.slice(1), ...args], { diff --git a/scripts/loop/shared/state.mjs b/scripts/loop/shared/state.mjs index 7cfa9eb..f02a56f 100644 --- a/scripts/loop/shared/state.mjs +++ b/scripts/loop/shared/state.mjs @@ -356,14 +356,62 @@ export function ghRepoOf(url) { return m ? `${m[1]}/${m[2]}` : null; } +/** + * The only branch shape the host will push (ops.md): `loop/-`. The + * host holds a `Contents: write` credential that the agent never sees, so this + * check is what keeps that credential from becoming a general push capability. + */ +export const LOOP_BRANCH_RE = /^loop\/\d+-[a-z0-9][a-z0-9._-]*$/; + +/** + * The loop-PR test lives in the router (`loopTaskOf`): head ref `loop/-*` **and** + * a body carrying `loop-task: #`, agreeing on ``. A PR missing the marker is + * treated as a non-loop PR — it never enters the acceptance rate and its merge + * closes the task by the wrong path. That is a structural contract, so the host + * enforces it rather than trusting the model to remember it. + */ +export function ensureLoopPrBody(body, id) { + const b = String(body ?? '').trim(); + const has = (re) => new RegExp(`(^|\\n)\\s*${re}\\b`, 'i').test(b); + const missing = []; + if (!has(`Closes\\s+#${id}`)) missing.push(`Closes #${id}`); + if (!has(`loop-task:\\s*#${id}`)) missing.push(`loop-task: #${id}`); + return [b, ...missing].filter(Boolean).join('\n\n'); +} + +/** + * The two actions that create the PR: the branch push and the PR itself. Split out + * because `finishRun` must execute exactly these *before* it decides the task's fate — + * a `pr-opened` whose push or PR creation fails is a task that would otherwise wait in + * `waiting-merge` for a PR that does not exist, which is the D33 failure mode in a new + * place. + */ +export function planPush(t, push, repo) { + return [ + { kind: 'push', repo, branch: push.branch, base: push.base ?? null }, + { + kind: 'pr-create', gh: 'pr', repo, number: t.id, + branch: push.branch, base: push.base ?? null, title: push.title, + body: ensureLoopPrBody(push.body, t.id), + }, + ]; +} + /** * Derive the GitHub write-action list from the outcome — **pure function, never * touches the network**. * report mode only calls this function and writes `describeActions` output into the * report; auto mode (L2+ / dry run) hands it to `applyActions` — the one "report * only" switch: no apply, no GitHub writes. + * + * `push` is the agent's PR proposal (`{ branch, base, title, body }`), read from + * `.result.json`. It only matters for `pr-opened`: under L2c the agent + * commits locally and the *host* pushes and opens the PR, so the label and the + * comment are queued behind `pr-create` — `applyActions` aborts the chain when + * the push or the PR creation fails, rather than labelling a PR that never + * appeared. */ -export function planActions(t, outcome, { label, comment, note } = {}) { +export function planActions(t, outcome, { label, comment, note, push = null } = {}) { if (!t.url) return []; // a local synthetic task has no GitHub target if (outcome === 'retry') return []; // nothing actually ran (failure/draft), so no write actions const repo = ghRepoOf(t.url); @@ -372,6 +420,19 @@ export function planActions(t, outcome, { label, comment, note } = {}) { const gh = t.kind === 'pr' ? 'pr' : 'issue'; const acts = []; const l = label ?? OUTCOME_MAP[outcome]?.label ?? null; + + if (outcome === 'pr-opened' && push) { + acts.push(...planPush(t, push, repo)); + } + // `pr-opened` without a `push` proposal is the local/A0 shape: there the agent runs + // with the developer's own credentials, so it opened the PR itself and reported the + // number. Labelling still applies — the PR exists either way. (On Actions `entry.mjs` + // refuses that shape outright: the agent holds no `Contents`, so it cannot have + // pushed anything, and a task must not sit in `waiting-merge` waiting for a PR that + // was never created.) + // + // The label goes to the task's own item (`gh`'s issue and PR surfaces differ, and + // `gh issue edit ` fails). if (l) acts.push({ kind: 'label', gh, repo, number: t.id, label: l, stripRoles: true }); if (outcome === 'closed') { acts.push({ kind: 'close', gh, repo, number: t.id, body: comment ?? note ?? 'Closed by the loop.' }); @@ -387,6 +448,10 @@ export function describeActions(actions) { if (a.kind === 'label') { return `label #${a.number}: +${a.label}${a.stripRoles ? ' (remove other role labels)' : ''}`; } + if (a.kind === 'push') return `push branch ${a.branch}${a.base ? ` (base ${a.base})` : ''}`; + if (a.kind === 'pr-create') { + return `open PR: ${a.branch} → ${a.base ?? 'default'} — ${firstLine(a.title)}`; + } if (a.kind === 'close') return `close #${a.number} (comment: ${firstLine(a.body)})`; return `comment #${a.number}: ${firstLine(a.body)}`; }); @@ -394,16 +459,91 @@ export function describeActions(actions) { const firstLine = (s) => String(s ?? '').split('\n')[0].slice(0, 120); +/** Never let a credential reach a log: git/gh stderr can echo what we passed them. */ +const redact = (s) => String(s ?? '') + .replace(/(x-access-token:)[A-Za-z0-9_-]+/g, '$1***') + .replace(/\b(gh[pousr]_)[A-Za-z0-9]{8,}/g, '$1***'); + /** * Execute write actions (only called on the auto / dry-run path). Failures only * warn; local state is never rolled back. + * + * The L2c credential split shows up here. `push` is the single action that needs + * `Contents: write`, and it spends `LOOP_PUSH_TOKEN` — a credential the agent + * never receives (`runPi` strips it from the child environment), which is what + * makes "the loop opens PRs but can never merge" a platform guarantee rather + * than a prompt instruction. Every `gh` call uses the host's `GH_TOKEN`, whose + * scopes carry no `Contents`. A failed `push`/`pr-create` aborts the rest of the + * chain instead of labelling a PR that does not exist. */ -export function applyActions(actions, { log = console.log, warn = console.warn } = {}) { +export function applyActions(actions, { + log = console.log, warn = console.warn, cwd = process.cwd(), exec = spawnSync, +} = {}) { const applied = []; + let aborted = null; for (const a of actions) { - const gh = (args) => spawnSync('gh', args, { encoding: 'utf8' }); + if (aborted) { + warn(`[loop] warn: skipping ${a.kind} — ${aborted} failed`); + continue; + } + const gh = (args) => exec('gh', args, { encoding: 'utf8' }); const sub = a.gh ?? 'issue'; // PRs go to gh pr, issues to gh issue - if (a.kind === 'label') { + if (a.kind === 'push') { + const token = process.env.LOOP_PUSH_TOKEN; + if (!token) { + warn('[loop] warn: LOOP_PUSH_TOKEN is not set — the host cannot push the branch.' + + " The agent's own credential deliberately has no Contents: write, so there is" + + ' no fallback that would not also hand the agent the merge permission.'); + aborted = 'push'; + continue; + } + if (!LOOP_BRANCH_RE.test(a.branch)) { + warn(`[loop] warn: refusing to push "${a.branch}" — the host only ever pushes` + + ' branches matching loop/- (ops.md)'); + aborted = 'push'; + continue; + } + const dirty = exec('git', ['status', '--porcelain'], { cwd, encoding: 'utf8' }); + if (dirty.status !== 0) { + warn(`[loop] warn: git status failed in ${cwd}: ${redact(dirty.stderr)}`); + aborted = 'push'; + continue; + } + if (dirty.stdout.trim()) { + warn('[loop] warn: the working tree is dirty — the agent left work uncommitted, so the' + + ' branch would not carry it and the PR would ship an incomplete diff; refusing to' + + ' push (the uncommitted work is lost with the checkout, which is why this fails' + + ' loudly instead of opening a hollow PR)'); + aborted = 'push'; + continue; + } + // `http..extraheader` keeps the token out of argv and out of any URL git + // might echo back in an error message. + const header = 'Authorization: Basic ' + + Buffer.from(`x-access-token:${token}`).toString('base64'); + const r = exec('git', [ + '-c', `http.https://github.com/.extraheader=${header}`, + 'push', `https://github.com/${a.repo}.git`, `HEAD:refs/heads/${a.branch}`, + ], { cwd, encoding: 'utf8' }); + if (r.status !== 0) { + warn(`[loop] warn: push ${a.branch} failed: ${redact(r.stderr)}`); + aborted = 'push'; + } else { log(`[loop] git: pushed ${a.branch}`); applied.push(a); } + } else if (a.kind === 'pr-create') { + const args = ['pr', 'create', '-R', a.repo, '--head', a.branch, + '--title', a.title, '--body', a.body]; + if (a.base) args.push('--base', a.base); + const r = gh(args); + if (r.status !== 0) { + warn(`[loop] warn: opening a PR for ${a.branch} failed: ${redact(r.stderr)}`); + aborted = 'pr-create'; + } else { + // `gh pr create` prints the new PR's URL (and does not support --json). + const n = Number((/\/pull\/(\d+)/.exec(r.stdout ?? '') ?? [])[1]); + log(`[loop] gh: opened PR${Number.isInteger(n) ? ` #${n}` : ''} from ${a.branch}`); + applied.push({ ...a, prNumber: Number.isInteger(n) ? n : null }); + } + } else if (a.kind === 'label') { let stale = []; if (a.stripRoles) { const cur = gh([sub, 'view', String(a.number), '-R', a.repo, '--json', 'labels', '-q', '.labels[].name']); @@ -466,7 +606,8 @@ export async function appendActionBlock(s, runId, actions) { */ export async function finishRun(s, { runId, outcome, note = null, comment = null, label = null, decision = null, pr = null, - tokens = null, writeLevel = 'report', noGithub = false, log = () => {}, warn = () => {}, + push = null, tokens = null, writeLevel = 'report', noGithub = false, + cwd = process.cwd(), exec = spawnSync, log = () => {}, warn = () => {}, }) { if (!outcome) throw new Error('outcome is required'); const rows = await readRunLines(s, runId); @@ -496,6 +637,24 @@ export async function finishRun(s, { } const at = nowIso(); + // `pr-opened` is only true if the PR actually exists, so the two actions that create it + // run *before* the outcome is recorded and the task moves. A failed push or PR creation + // downgrades the run to `failed`: the task goes to the inbox, where a human sees it, + // instead of `waiting-merge`, where nothing would ever resolve it. + let created = null; + if (t && !noGithub && writeLevel === 'auto' && outcome === 'pr-opened' && push) { + const done = applyActions(planPush(t, push, ghRepoOf(t.url)), { log, warn, cwd, exec }); + created = done.find((a) => a.kind === 'pr-create' && a.prNumber) ?? null; + if (created) pr = created.prNumber; + else { + outcome = 'failed'; + note = `${note ? `${note} — ` : ''}the host could not open the PR ` + + '(see the push/pr-create warnings above); the task returns to the inbox ' + + 'rather than waiting for a PR that does not exist'; + warn(`[loop] ${note}`); + } + } + await appendRunRow(s, runId, { runId, event: 'end', at, outcome, durationMs: Date.now() - Date.parse(start.at), @@ -522,15 +681,30 @@ export async function finishRun(s, { await saveTask(s, t); await releaseLock(s, t, t.stage); - const actions = noGithub ? [] : planActions(t, outcome, { label: label ?? m.label, comment, note }); - let applied = []; + const actions = noGithub ? [] : planActions(t, outcome, { + // The push/PR actions already ran above; passing `null` here would drop the label + // too, so pass them through only when they have not been executed. + label: label ?? m.label, comment, note, push: created ? null : push, + }); + let applied = created ? [created] : []; if (actions.length) { - if (writeLevel === 'auto') applied = applyActions(actions, { log, warn }); + if (writeLevel === 'auto') applied = [...applied, ...applyActions(actions, { log, warn, cwd, exec })]; else await appendActionBlock(s, runId, actions); } + // A PR the host just opened exists on GitHub but not yet in the state layer: record + // its number right after the action ran, so the task file carries it without a second + // writer (`planActions` cannot know a number it does not create). + if (created) { + if (!t.prs.includes(created.prNumber)) t.prs.push(created.prNumber); + t.timeline.push({ + at: nowIso(), event: 'pr-created', by: runId, detail: `pr #${created.prNumber}`, + }); + await saveTask(s, t); + } + await renderStateSummary(s); - return { task: t, actions, applied }; + return { task: t, actions, applied, outcome }; } /* ------------------------------------------------------------------ CLI */ From 93bca02e03e88b9db2d2a6b2f2a5acc282e01edb Mon Sep 17 00:00:00 2001 From: Alex Chao Date: Tue, 15 Sep 2026 23:58:17 +0800 Subject: [PATCH 2/5] chore(loop): give the agent no write credential at all, not a narrower one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewing the split landed in the previous commit showed the middle credential earned nothing. Every GitHub write — labels, comments, closing, opening the PR, pushing the branch — is executed by `applyActions` in the *host's* process from the agent's `result.json`. The agent's `Issues/Pull requests write` PAT was never used by any code path: the agent exports a file and reads GitHub. So the split is by who runs the code, not by scope list: agent → github.token, read-only, in BOTH modes (via agentEnv) host → LOOP_GH_TOKEN, one PAT: Contents + Issues + Pull requests write This removes a secret and makes the guarantee structural twice over. Before, "the loop cannot merge" rested on scoping one PAT to omit Contents; now the agent holds no write credential whatsoever, and the `Contents: write` the host needs — which is also the merge permission — exists only where the untrusted party cannot reach it. The agent cannot relabel, comment, push or merge however it is prompted, and the job's `permissions:` block stays read-only, so the platform lock is unchanged. Behaviour changes beyond the credential layout: - The `auto` prompt no longer offers `gh` writes to the agent; it states plainly that every write answers 403 and must not be retried, and it now also tells the `report` agent not to commit (no branch will be pushed, so a commit is discarded with the checkout — the work belongs in the report as a patch). - `applyActions`' push spends `process.env.GH_TOKEN` (the host PAT) instead of `LOOP_PUSH_TOKEN`; the warning names both the Actions secret and the local `GH_TOKEN=$(gh auth token)` form. - `gh-check.mjs` drops the third-credential section and checks the agent's read-only property unconditionally, independent of the write level. Verification (local, four harnesses, all green): `agentEnv` hands over the read-only token and strips the PAT at both levels; `applyActions` pushes with the host credential and refuses `main`, `loop/fix-thing`, `loop/42-`, a dirty tree and a missing credential, aborting the label/comment chain after a failed PR; `finishRun` walks the success path (push → PR #4242 → label → comment → `waiting-merge`); `entry.mjs` end to end pushes the branch for real and fails closed on a bad branch name, a branch carrying another task's number, the report boundary and a missing host credential. --- .github/workflows/loop.yml | 28 +++++----- scripts/loop/README.md | 93 ++++++++++++++++---------------- scripts/loop/gh-check.mjs | 99 ++++++++++------------------------- scripts/loop/shared/agent.mjs | 81 ++++++++++++++-------------- scripts/loop/shared/state.mjs | 25 +++++---- 5 files changed, 144 insertions(+), 182 deletions(-) diff --git a/.github/workflows/loop.yml b/.github/workflows/loop.yml index faec662..565a392 100644 --- a/.github/workflows/loop.yml +++ b/.github/workflows/loop.yml @@ -50,11 +50,11 @@ concurrency: group: loop cancel-in-progress: false -# Least privilege: GitHub reads use GITHUB_TOKEN; the agent's own writes go through -# LOOP_GH_TOKEN, and branch pushes through LOOP_PUSH_TOKEN — neither of which the -# agent ever receives (see runPi/agentEnv). `github.token` stays read-only on -# purpose: it is the credential the agent holds under the report boundary, so -# granting the job `contents: write` would hand the agent the merge permission. +# Least privilege: GitHub reads use GITHUB_TOKEN, the loop's writes use +# LOOP_GH_TOKEN. The agent never receives the PAT — `agentEnv` strips it and hands +# the child `github.token` instead — so `github.token` staying read-only is what +# makes "the agent cannot write to GitHub at all" a platform guarantee. Granting the +# job `contents: write` would hand the agent the merge permission. permissions: contents: read issues: read @@ -101,17 +101,15 @@ jobs: LOOP_SMOKE: ${{ github.event.inputs.smoke || 'false' }} LOOP_MODEL: ${{ github.event.inputs.model || 'deepseek/deepseek-v4-flash' }} LOOP_RUN_TIMEOUT_MS: '3300000' # 55min, leaving headroom for push (job timeout 60min) + # The loop's write credential — a PAT with Contents write plus Issues/Pull + # requests write. Held by the *host* process (applyActions); `agentEnv` deletes + # it from the child environment, so the agent reads with `github.token` below + # and cannot write to GitHub however it is prompted. GH_TOKEN: ${{ secrets.LOOP_GH_TOKEN || github.token }} - # Handed to the agent subprocess in place of GH_TOKEN under the report - # boundary (see runPi in shared/agent.mjs). `github.token` here is capped - # at read by the `permissions:` block above, so the agent cannot write even - # if LOOP_GH_TOKEN is a write-capable PAT. Host-side writes still use - # GH_TOKEN, in the host's own process. + # Handed to the agent subprocess in place of GH_TOKEN. `github.token` here is + # capped at read by the `permissions:` block above, and the agent gets it in + # every mode — the loop's writes are the host's job, not the agent's. GH_READ_TOKEN: ${{ github.token }} - # L2c's third role: the only credential with `Contents: write`, held by the host - # process and *removed* from the agent's environment by `agentEnv`. It exists to - # push `loop/-*` and nothing else. - LOOP_PUSH_TOKEN: ${{ secrets.LOOP_PUSH_TOKEN }} # Credentials: secrets only DEEPSEEK_API_KEY: ${{ secrets.DEEPSEEK_API_KEY }} R2_ACCESS_KEY_ID: ${{ secrets.R2_ACCESS_KEY_ID }} @@ -150,7 +148,7 @@ jobs: # agent's. That is the one credential the agent must never hold (it is the # push/merge permission), and `agentEnv` cannot un-write a file. With this off, # a `git push` from the agent has no credential at all; the host pushes through - # `http.extraheader` using LOOP_PUSH_TOKEN. + # `http.extraheader` using its own GH_TOKEN. persist-credentials: false ref: >- ${{ diff --git a/scripts/loop/README.md b/scripts/loop/README.md index 366fb57..fae0192 100644 --- a/scripts/loop/README.md +++ b/scripts/loop/README.md @@ -53,45 +53,48 @@ In `report` mode the pending actions are appended to `state/reports/.md` as a checklist and surfaced in the Actions Step Summary, so a human can execute them by hand. -### GitHub write credentials (three roles) +### GitHub write credentials (two roles) `GH_TOKEN: ${{ secrets.LOOP_GH_TOKEN || github.token }}`. Until L2 the secret was deliberately unset, so `github.token` — capped read-only by the workflow's `permissions:` block — was the only credential a run could reach, and a run that -ignored its prompt got a 403 from `gh pr merge`. That is the *report boundary* -credential; configuring the secret replaces it for every `gh` call in the job, -which is why `gh-check.mjs` measures rather than assumes. +ignored its prompt got a 403 from `gh pr merge`. Configuring the secret replaces it +for every `gh` call in the job, which is why `gh-check.mjs` measures rather than +assumes. -There are now three credentials, and the loop's central guarantee — it opens PRs -and can never merge — rests on which process holds which: +There are two credentials, split by **who runs the code**, not by scope list: | Credential | Held by | Scopes | Used for | | --- | --- | --- | --- | -| `github.token` (`GH_READ_TOKEN`) | the agent, under `report` | read-only (`permissions:`) | reading GitHub: triage, sweep, review | -| `LOOP_GH_TOKEN` (PAT) | the agent under `auto`; the host otherwise | `Issues: RW`, `Pull requests: RW` — **no Contents** | labels, comments, closing, opening the PR | -| `LOOP_PUSH_TOKEN` (PAT) | the host process only | `Contents: RW` | pushing `loop/-*` — nothing else | +| `github.token` (`GH_READ_TOKEN`) | the agent, in every mode | read-only (`permissions:`) | reading GitHub: triage, sweep, review | +| `LOOP_GH_TOKEN` (PAT) | the host process only | `Contents: RW` + `Issues: RW` + `Pull requests: RW` | every loop write: labels, comments, closing, branch push, opening the PR | + +The agent gets **no write credential at all**, in either mode. It does not need one: +`applyActions` derives the write list from the agent's `result.json` and executes it +in the host's process. That is why the guarantee does not depend on scoping one +token finely — a read-only agent cannot merge, push or relabel regardless of what +its prompt says, and the scope the host needs (`Contents: write`, which is also the +merge permission) exists only where the untrusted party cannot reach it. Configuring `LOOP_GH_TOKEN` lifts only the platform lock — `writeLevel` still -gates `applyActions` in `state.mjs`, and its default is `report`, so ordinary -runs keep reporting. - -It must not also lift the **agent's** lock. `agentEnv` in `shared/agent.mjs` builds -the child environment from scratch: it deletes `LOOP_PUSH_TOKEN`, `LOOP_GH_TOKEN`, -`GH_READ_TOKEN` and the R2 keys, and sets `GH_TOKEN` to the least credential the -write level allows — `GH_READ_TOKEN` (i.e. `github.token`, capped at read by -`permissions:`) under `report`, and `LOOP_GH_TOKEN` under `auto`. The agent -therefore reaches GitHub read-only under the report boundary even if it ignores -its prompt, while host-side writes keep using the real `GH_TOKEN` in the host's -own process. +gates `applyActions` in `state.mjs`, and its default is `report`, so ordinary runs +keep reporting. + +The agent's side is enforced in code, not in prose: `agentEnv` in +`shared/agent.mjs` builds the child environment from scratch — it deletes +`LOOP_GH_TOKEN`, `GH_READ_TOKEN`, `GITHUB_TOKEN` and the R2 keys, then sets +`GH_TOKEN` to `GH_READ_TOKEN` (`github.token`, capped at read by `permissions:`). +`persist-credentials: false` on the checkout step closes the other half: without it +git's config would carry the job token, which no environment variable can revoke. Both are measurable without spending a token: `node scripts/loop/gh-check.mjs` probes each credential's scopes side-effect free (write endpoints against an impossible id — `403` means absent, `404`/`422` -means present) and reports what the host holds and what the agent will hold. +means present, plus an idempotent write that restores the value it read) and +reports what the host holds and what the agent will hold. -**Do not grant the agent's credential `Contents: write`.** Merging a PR is not -under "Pull requests" — it is under "Contents" (GitHub's endpoint→permission -table): +**The agent must never hold `Contents: write`.** Merging a PR is not under +"Pull requests" — it is under "Contents" (GitHub's endpoint→permission table): | Action | gh command | Permission required | | --- | --- | --- | @@ -112,7 +115,7 @@ Two consequences that are easy to get wrong: 2. **"Can open a PR" and "can merge a PR" are the same permission** — within a single credential. Opening a PR needs a head branch; creating one needs `Contents: write`; and that is the merge permission. Separating them means - splitting the *roles*, which is what L2c below does. + separating the *processes*, which is what L2c below does. So the L2 step has three sizes rather than two. Two are pure scope choices; the third is the one that satisfies "loop may open PRs but must never merge them", @@ -177,12 +180,13 @@ has to come from the credential split. #### L2c as implemented -The three roles above are now wired end to end. What a human has to configure is -two secrets; everything else is code that fails closed. +The split above is wired end to end. What a human has to configure is **one +secret**; everything else is code that fails closed. | Piece | Where | Behaviour | | --- | --- | --- | -| `LOOP_PUSH_TOKEN` env | `.github/workflows/loop.yml` | The only `Contents: write` credential. Deleted from the agent's environment by `agentEnv`. | +| `LOOP_GH_TOKEN` secret | `.github/workflows/loop.yml` | The host's PAT: `Contents: RW` + `Issues: RW` + `Pull requests: RW`, scoped to this repository. The only write credential in the system. | +| `agentEnv(writeLevel)` | `shared/agent.mjs` | Builds the child environment from scratch: strips the PAT, `GITHUB_TOKEN` and the R2 keys; the agent's `GH_TOKEN` is `github.token` in **both** modes. | | `persist-credentials: false` | the `actions/checkout` step | Without it the job token is written into the repo's git config, where the agent's `git push` would find it; `agentEnv` cannot un-write a file. | | `vars.LOOP_EXECUTE_WRITES` | job env | `workflow_dispatch` still overrides per run, so the A1 rehearsal entry point is unchanged. | | `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. | @@ -193,11 +197,11 @@ two secrets; everything else is code that fails closed. | PR number recorded | `finishRun` | The number comes back from `gh pr create` and lands in `task.prs` + a `pr-created` timeline row. | | git identity | `agentEnv` | `GIT_AUTHOR_*`/`GIT_COMMITTER_*` are injected under `auto`; a fresh runner has no global git config, and the commit itself is local (no credential involved). | -The failure that remains possible is deliberate: with `auto` on and -`LOOP_PUSH_TOKEN` unset, every product stage fails at the push and the task goes -to the inbox. The alternative — falling back to the agent's credential — would -also hand the agent the merge permission, which is the one thing L2c exists to -prevent. +The failure that remains possible is deliberate: with `auto` on and no PAT +configured, `GH_TOKEN` falls back to `github.token`, every product stage fails at +the push, and the task goes to the inbox. Withdrawing the host's write access is +exactly how you turn the loop back into a reporting one — the agent's side never +changes, because it never had write access to begin with. ## A1 architecture (Actions + R2) @@ -217,9 +221,10 @@ GitHub event ─▶ loop.yml (concurrency group `loop` = platform-level single w project-local resources. - **pi has no built-in permission gating** (verified), so the boundary is built from the environment instead: the job's `permissions:` block, the absence of - secrets in fork contexts, and `agentEnv` (which strips `LOOP_PUSH_TOKEN`, the - PAT and the R2 keys from the child). Do not add write credentials to a job - that touches untrusted input, and do not rely on the prompt for any of it. + secrets in fork contexts, and `agentEnv` (which strips the PAT, `GH_READ_TOKEN` + and the R2 keys from the child, then hands it the read-only job token). Do not + add write credentials to a job that touches untrusted input, and do not rely on + the prompt for any of it. - **One writer for closing:** the agent writes `state/reports/.result.json`; `entry.mjs` performs the transition via `finishRun`. Agents never run `pnpm loop end` themselves. @@ -516,14 +521,14 @@ sample once `auto` is switched on. — see the failure-classification item above. (Distinction worth keeping: 21 workflow executions, 18 run records.) - [x] No drift **and** no incorrect proposal for a week → **owner decided L2c** - (2026-09-14): the loop opens PRs but can never merge. The split is - implemented — the host pushes branches with `LOOP_PUSH_TOKEN` and opens the - PR, the agent's PAT carries no `Contents`, and `persist-credentials: false` - keeps the job token out of the agent's reach; see "L2c as implemented" and - the endpoint-level evidence under "GitHub write credentials". What remains - is configuration, not code: the two secrets above must exist before - `LOOP_EXECUTE_WRITES=true`, and the unchecked boxes in this list become - sampleable only then. + (2026-09-14): the loop opens PRs but can never merge. Implemented as a + two-credential split — the host alone holds a PAT with `Contents: write` and + does every write, while the agent gets `github.token`, read-only, in both + modes; `persist-credentials: false` closes the git-config path. See "L2c as + implemented" and the endpoint-level evidence under "GitHub write + credentials". What remains is configuration, not code: one secret must exist + before `LOOP_EXECUTE_WRITES=true`, and the unchecked boxes in this list + become sampleable only then. ### Not reachable under A1 (structural — decide before the L2 switchover) diff --git a/scripts/loop/gh-check.mjs b/scripts/loop/gh-check.mjs index 3ff6b9a..cb7ca7f 100644 --- a/scripts/loop/gh-check.mjs +++ b/scripts/loop/gh-check.mjs @@ -158,26 +158,9 @@ if (hostPerm.ok) { console.log(` (info) repo .permissions = ${hostPerm.out.replace(/\s+/g, ' ')}` + ' — the *user\'s* role, not this token\'s scopes; do not read it as a scope report'); } -// The credential `GH_TOKEN` resolves to is also the agent's under `auto` (agentEnv -// hands the child `LOOP_GH_TOKEN`), so a PAT with Contents here is the one -// configuration that would let the agent merge: measure it, do not assume it. - -// L2c's third role. This is the *only* credential that may hold Contents, it belongs -// to the host process (applyActions' push), and `agentEnv` deletes it from the child -// environment — a push is the one write that cannot be delegated to the agent, because -// the same scope that creates a branch also merges one. -console.log('\n[loop:gh] push credential (LOOP_PUSH_TOKEN — host only, absent from the agent env):'); -const pushToken = process.env.LOOP_PUSH_TOKEN || null; -let canPushBranch = null; -if (!pushToken) { - console.log(' (not set — the host cannot push, so no product stage can reach `pr-opened`;' - + ' set it from a PAT with Contents: Read and write, or set LOOP_EXECUTE_WRITES=true only' - + ' after it exists)'); -} else { - canPushBranch = idempotentWrite( - 'Contents: write (branch push — the host\'s job)', refProbe, pushToken, - ); -} +// `canPush` above is the host's ability to push `loop/-*` **and** to merge: one +// scope covers both, which is exactly why it must not exist on any credential the +// agent can reach. The agent check at the end of this script confirms it does not. console.log('\n[loop:gh] scopes reached by a credential that lacks them entirely:'); const absent = [ @@ -198,16 +181,15 @@ const writeLevel = process.env.LOOP_WRITE_LEVEL ?? (process.env.LOOP_EXECUTE_WRITES === 'true' ? 'auto' : 'report'); console.log(''); if (canPush) { - console.log('[loop:gh] WARNING: the host credential holds Contents:write, and under `auto`' - + ' the agent is handed that same credential — so the agent could merge (PUT' - + ' /pulls/{n}/merge), push branches and cut releases. L2c keeps that scope out of the' - + ' agent\'s reach: the host pushes with LOOP_PUSH_TOKEN, and LOOP_GH_TOKEN is scoped to' - + ' Issues + Pull requests only. See "GitHub write credential" in scripts/loop/README.md;' - + ' recreate the PAT with Issues Read and write, Pull requests Read and write, Contents' - + ' Read-only, and Nothing for Actions/Secrets/Administration/Workflows.'); + console.log('[loop:gh] host credential holds Contents:write — branch pushes and PR creation are' + + ' possible. This is required for L2c, and it is also the merge permission, which is why' + + ' the agent is never given this credential (see the check at the end). One fine-grained' + + ' PAT with Contents + Issues + Pull requests write, scoped to this repository, is the' + + ' whole configuration.'); } else if (canPush === false) { - console.log('[loop:gh] host credential holds no Contents:write — merging, branch pushes and' - + ' releases are impossible for it, which is the L2c invariant.'); + console.log('[loop:gh] host credential holds no Contents:write — the loop cannot push a branch,' + + ' so no product stage can ever reach `pr-opened`. Keep LOOP_EXECUTE_WRITES=false until' + + ' LOOP_GH_TOKEN carries Contents: Read and write on this repository.'); } console.log(`[loop:gh] label/comment/close: ${canLabel ? 'available' : 'NOT available — the loop cannot do its job'}`); console.log(`[loop:gh] reads: ${brokenReads.length @@ -218,48 +200,23 @@ if (writeLevel === 'auto' && !canLabel) { console.log(' WARNING: `auto` is switched on but the credential cannot label/comment — the loop' + ' will run, propose, and silently fail to write. Configure LOOP_GH_TOKEN first.'); } -if (writeLevel === 'auto' && !pushToken) { - console.log(' WARNING: `auto` is switched on but LOOP_PUSH_TOKEN is unset — no product stage can' - + ' produce a PR. Either set it, or keep LOOP_EXECUTE_WRITES=false until it exists.'); -} else if (writeLevel === 'auto' && canPushBranch === false) { - console.log(' WARNING: LOOP_PUSH_TOKEN lacks Contents:write — every product stage will fail at' - + ' the host\'s push. That credential needs Contents: Read and write on this repository only.'); +if (writeLevel === 'auto' && canPush === false) { + console.log(' WARNING: `auto` is switched on but the host cannot push — every product stage will' + + ' fail at the push and land in the human inbox. Either grant LOOP_GH_TOKEN Contents:' + + ' Read and write, or keep LOOP_EXECUTE_WRITES=false.'); } -// What the agent actually holds is decided by `agentEnv(writeLevel)`, and it is worth -// measuring rather than reading off the workflow file, because `GH_TOKEN` is -// `secrets.LOOP_GH_TOKEN || github.token` — which one resolves is invisible here. +// What the agent actually holds, after `agentEnv` has built the child environment. The +// agent is read-only in **every** mode, so this check is not conditional on the write +// level: the loop's writes are the host's job, and a credential that could write here +// would be a configuration error, not a mode. // // Identity cannot be read from `GET /user` for `github.token`: it is a GitHub App // installation token and is refused there entirely, which is why these checks combine // two signals that *are* observable — whether the credential can read the repository, // and whether a write is refused. -console.log('\n[loop:gh] credential handed to the agent:'); -if (writeLevel === 'auto') { - if (!hostToken) { - console.log(' (no LOOP_GH_TOKEN and no GH_TOKEN — gh in the agent would use its own login.)'); - } else { - const canRead = gh(['api', `repos/${repo}`, '--jq', '.full_name'], { token: hostToken }); - const asUser = gh(['api', 'user', '--jq', '.login'], { token: hostToken }); - console.log(` can read the repo: ${canRead.ok ? 'yes' : `NO — the agent cannot read GitHub (${canRead.status})`}`); - console.log(` acts as a user: ${asUser.ok ? `yes (${asUser.out}) — a PAT` : 'no — an App token (github.token)'}`); - const agentWrite = idempotentWrite('Issues: write (labels/comments)', issueProbe, hostToken); - idempotentWrite('Pull requests: write', prProbe, hostToken); - const agentPush = idempotentWrite('Contents: write (must be ABSENT)', refProbe, hostToken); - if (!canRead.ok) { - console.log(' WARNING: the agent cannot read GitHub, so triage/sweep will find nothing.'); - } else if (agentPush) { - console.log(' WARNING: the agent holds Contents:write — it can push AND merge, so "no' - + ' self-merge" is a prompt instruction only. L2c requires a PAT without Contents.'); - } else if (!agentWrite) { - console.log(' WARNING: the agent holds no Issues:write — it cannot label or comment, so every' - + ' `auto` write will fail. `auto` needs the PAT (LOOP_GH_TOKEN); github.token is read-only.'); - } else { - console.log(' OK: the agent can label/comment/open PRs but holds no Contents — it cannot push' - + ' or merge even if it ignores its prompt.'); - } - } -} else if (!agentToken) { +console.log('\n[loop:gh] credential handed to the agent (agentEnv: read-only in every mode):'); +if (!agentToken) { console.log(' (GH_READ_TOKEN is not set, so agentEnv leaves the child without GH_TOKEN — `gh`' + ' there would fall back to any local login. On Actions this variable is always set.)'); } else { @@ -272,15 +229,13 @@ if (writeLevel === 'auto') { if (!canRead.ok) { console.log(' WARNING: the agent cannot read GitHub, so triage/sweep will find nothing.'); - } else if (asUser.ok) { - console.log(` WARNING: the agent holds a *user* token (${asUser.out}) — it inherits that` - + ' user\'s reach, so the report boundary is a prompt instruction only.'); } else if (agentWrite || agentPush) { - console.log(' WARNING: the agent can write (github.token is not capped to read — check the' - + " workflow's `permissions:` block)."); + console.log(' WARNING: the agent CAN WRITE. It should hold `github.token` capped at read by' + + " the workflow's `permissions:` block, so check that block — and check that GH_READ_TOKEN" + + ' is `github.token`, not a PAT.'); } else { - console.log(' OK: github.token, read-only by the workflow\'s `permissions:` block — the agent' - + ' reaches GitHub read-only even if it ignores its prompt. The report boundary is a' - + ' platform guarantee.'); + console.log(' OK: the agent can read GitHub and cannot write anything (no Issues, no Contents)' + + ' — so it cannot label, comment, push or merge, whatever its prompt says. Every write is' + + ' the host\'s, executed from the agent\'s result file.'); } } diff --git a/scripts/loop/shared/agent.mjs b/scripts/loop/shared/agent.mjs index f4dae67..ee84150 100644 --- a/scripts/loop/shared/agent.mjs +++ b/scripts/loop/shared/agent.mjs @@ -52,24 +52,26 @@ export function buildPrompt({ task, stage, runId, writeLevel, mode, repo, lead = } L.push(''); L.push(`## Write boundary: ${writeLevel}`); + L.push('### GitHub: you never write'); + L.push('Your GitHub credential is read-only in **every** mode — `gh issue edit` /'); + L.push('`gh issue comment` / `gh issue close` / `gh pr create` / `gh pr merge` /'); + L.push('`git push` all answer 403. That is deliberate, not a misconfiguration: do not'); + L.push('retry them, and do not look for another credential. Label/comment/close/PR'); + L.push('actions belong to the host, which executes them from your result file.'); + L.push(''); + L.push('Allowed: reading GitHub, editing the working tree, running the repo gates,'); + L.push('reading and writing `state/`.'); if (writeLevel === 'report') { - L.push('**Read-only with respect to GitHub.** Do NOT run any of:'); - L.push('`gh issue edit` / `gh issue comment` / `gh issue close` / `gh pr create` /'); - L.push('`gh pr merge` / `git push` / any other command that mutates GitHub.'); - L.push('Allowed: reading GitHub, editing the working tree, running the repo gates,'); - L.push('reading and writing `state/`. Any GitHub action you conclude is needed'); - L.push('(labels, comments, closing, opening a PR) must be written into the report'); - L.push('as a proposal for a human to execute.'); - } else { - L.push('You may write to GitHub through `gh` — **but only** labels, comments and'); - L.push('closing (`gh issue edit|comment|close`, `gh pr edit|comment`).'); - L.push('Two steps belong to the host, not to you:'); L.push(''); - L.push('- **Pushing the branch.** Your credential carries no `Contents: write` (that is'); - L.push(' also the merge permission, so by design you do not hold it; `git push` and'); - L.push(' `gh pr merge` answer 403 — do not attempt them).'); - L.push('- **Opening the PR.** You propose it; the host pushes and creates it.'); + L.push('### `report` mode: the host writes nothing either'); + L.push('**Do not commit.** No branch will be pushed — a commit would be discarded with'); + L.push('the checkout. Deliver the change as a patch inside your report (the diff, or the'); + L.push('exact edits), and say what a human must do. Any GitHub action you conclude is'); + L.push('needed (labels, comments, closing, opening a PR) goes into the report as a'); + L.push('proposal.'); + } else { L.push(''); + L.push('### `auto` mode: the host pushes the branch and opens the PR'); L.push('For a product stage (`bugfix`/`feature`/`deps`/`security`) that produces a change:'); L.push('1. Do the work in the working tree, then **commit it locally** on a branch named'); L.push(' `loop/-` (e.g. `git switch -c loop/33-ci-workflow`). Commits'); @@ -155,30 +157,31 @@ export const SESSION_HINT = 'session files land under --session-dir (uploaded as /** * The environment handed to the agent subprocess. * - * `{ ...process.env }` used to give the child everything the host holds, including - * the push credential and the R2 keys — neither of which any stage needs, and the - * push credential is exactly the one that carries `Contents: write` (i.e. merge). - * The agent reaches GitHub through `GH_TOKEN` only, set to the least credential the - * write level allows: + * `{ ...process.env }` used to give the child everything the host holds, including a + * push-capable token and the R2 keys. Now the agent holds **no write credential at + * all**, at either write level: + * + * agent → `GH_READ_TOKEN` (github.token, capped at read by the workflow's + * `permissions:` block — a platform guarantee, not a prompt one) + * host → the PAT, used by `applyActions` in its own process * - * `report` → `GH_READ_TOKEN` (github.token, capped at read by the workflow's - * `permissions:` block — a platform guarantee, not a prompt one) - * `auto` → `LOOP_GH_TOKEN` (the PAT: Issues + Pull requests write, **no - * Contents**), so the L2c loop can label/comment but never push or - * merge, even if it ignores its prompt + * The agent does not need write access: every GitHub write is the host's job + * (`applyActions` executes the label/comment/close/push/PR actions that + * `planActions` derives from the agent's result file). Restricting it to reads means + * "the loop never self-merges" no longer depends on scoping one credential finely — + * the agent simply cannot write, whichever mode is running. * - * Everything else that authenticates to something is deleted. Locally both tokens - * are unset, so no `GH_TOKEN` key survives and `gh` falls back to the developer's - * own login — which is the correct behaviour for a local run. + * Everything that authenticates to something else is deleted too. Locally both + * variables are unset, so no `GH_TOKEN` key survives and `gh` falls back to the + * developer's own login — the correct behaviour for a local run. */ export function agentEnv(writeLevel, base = process.env) { const env = { ...base }; for (const k of [ - 'LOOP_PUSH_TOKEN', 'LOOP_GH_TOKEN', 'GH_READ_TOKEN', 'GITHUB_TOKEN', + 'LOOP_GH_TOKEN', 'GH_READ_TOKEN', 'GITHUB_TOKEN', 'R2_ACCESS_KEY_ID', 'R2_SECRET_ACCESS_KEY', 'R2_ACCOUNT_ID', 'R2_BUCKET', ]) delete env[k]; - const token = writeLevel === 'auto' ? (base.LOOP_GH_TOKEN ?? base.GH_TOKEN) : base.GH_READ_TOKEN; - if (token) env.GH_TOKEN = token; + if (base.GH_READ_TOKEN) env.GH_TOKEN = base.GH_READ_TOKEN; else delete env.GH_TOKEN; // Product stages commit the branch the host pushes, and the identity must not come // from the sandbox's global git config (there is none on a fresh runner). Set it @@ -211,18 +214,16 @@ export function runPi({ // another CLI or wrapper script). const engine = (process.env.LOOP_ENGINE_CMD || 'pi').split(/\s+/).filter(Boolean); - // The agent never inherits a credential by accident: see `agentEnv` above for - // what it holds at each write level and why the push credential is absent from - // both. Host-side writes (applyActions) still use the real GH_TOKEN in the - // host's own process. + // The agent never inherits a credential by accident: see `agentEnv` above. It is + // read-only in every mode; the host's own writes (applyActions) use the real + // GH_TOKEN in the host's process. const childEnv = agentEnv(writeLevel); if (childEnv.GH_TOKEN) { - log(writeLevel === 'auto' - ? '[loop] agent GitHub credential: the PAT (Issues/Pull requests write, no Contents)' - : '[loop] agent GitHub credential: the read-only job token (report boundary)'); + log('[loop] agent GitHub credential: read-only (github.token) — it cannot write to' + + ' GitHub in any mode'); } else { - log('[loop] agent GitHub credential: none at this write level — `gh` there would fall' - + ' back to any local login, so the run may not be able to read GitHub at all'); + log('[loop] agent GitHub credential: none — `gh` there would fall back to any local' + + ' login, so the run may not be able to read GitHub at all'); } const child = spawn(engine[0], [...engine.slice(1), ...args], { diff --git a/scripts/loop/shared/state.mjs b/scripts/loop/shared/state.mjs index f02a56f..74de36f 100644 --- a/scripts/loop/shared/state.mjs +++ b/scripts/loop/shared/state.mjs @@ -468,13 +468,12 @@ const redact = (s) => String(s ?? '') * Execute write actions (only called on the auto / dry-run path). Failures only * warn; local state is never rolled back. * - * The L2c credential split shows up here. `push` is the single action that needs - * `Contents: write`, and it spends `LOOP_PUSH_TOKEN` — a credential the agent - * never receives (`runPi` strips it from the child environment), which is what - * makes "the loop opens PRs but can never merge" a platform guarantee rather - * than a prompt instruction. Every `gh` call uses the host's `GH_TOKEN`, whose - * scopes carry no `Contents`. A failed `push`/`pr-create` aborts the rest of the - * chain instead of labelling a PR that does not exist. + * This is the only place the loop writes to GitHub, and it runs in the host's own + * process — the agent holds a read-only credential and cannot reach any of it. + * `push` is the single action needing `Contents: write`, so it spends the host's + * `GH_TOKEN` (the PAT); every `gh` call uses the same credential. A failed + * `push`/`pr-create` aborts the rest of the chain instead of labelling a PR that + * does not exist. */ export function applyActions(actions, { log = console.log, warn = console.warn, cwd = process.cwd(), exec = spawnSync, @@ -489,11 +488,15 @@ export function applyActions(actions, { const gh = (args) => exec('gh', args, { encoding: 'utf8' }); const sub = a.gh ?? 'issue'; // PRs go to gh pr, issues to gh issue if (a.kind === 'push') { - const token = process.env.LOOP_PUSH_TOKEN; + // The host's own credential — one PAT holding `Contents: write` plus the + // Issues/Pull-requests scopes `gh` needs. The agent never receives it + // (`agentEnv` strips it), so a push is only ever performed here, by trusted + // code, on a ref the check below has already constrained. + const token = process.env.GH_TOKEN; if (!token) { - warn('[loop] warn: LOOP_PUSH_TOKEN is not set — the host cannot push the branch.' - + " The agent's own credential deliberately has no Contents: write, so there is" - + ' no fallback that would not also hand the agent the merge permission.'); + warn('[loop] warn: the host has no GitHub credential (GH_TOKEN unset) — cannot push.' + + ' On Actions, configure LOOP_GH_TOKEN with Contents write plus Issues/Pull requests' + + ' write; locally, export GH_TOKEN (e.g. `GH_TOKEN=$(gh auth token)`).'); aborted = 'push'; continue; } From 63f794014296673ac6f2665fe140bd82da987b1d Mon Sep 17 00:00:00 2001 From: Alex Chao Date: Wed, 16 Sep 2026 00:41:02 +0800 Subject: [PATCH 3/5] chore(loop): make the branch ruleset the gate, not the GitHub credential MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The loop's safety property is "nothing reaches `main` without a human". The previous commits on this branch tried to express it with credentials — a PAT for the host, a read-only job token for the agent — and that does not hold. A PAT acts as its owner, who is this repository's only admin and therefore the ruleset's bypass actor, so a loop holding one can merge an unapproved PR. Worse, every PR it opens is authored by the owner, whom GitHub forbids approving their own work: the loop's output would be neither approvable nor safely mergeable. Measured, not assumed: the agent can read the host's environment through /proc//environ, so "the child does not inherit the token" was never a containment argument to begin with. Repository configuration settles it instead, so no credential has to: Main branch ruleset (21855851, ~DEFAULT_BRANCH, enforcement: active) rules: deletion · non_fast_forward · pull_request(1 approval) bypass_actors: User(owner), bypass_mode: pull_request | actor | push to main | merge without an approval | | the owner | refused | allowed (bypass) — merging is the consent | | the loop (github.token) | refused | refused — and it cannot approve its own PR | `bypass_mode: pull_request` is the load-bearing part: it lets the owner merge their own PR (solo repo, self-approval is forbidden) while refusing every direct push, including the owner's. Verified — an idempotent `PATCH /git/refs/heads/main` answers `422 Changes must be made through a pull request`. `permissions:` cannot grant `administration`, so the loop cannot weaken the rule that bounds it. Consequences, all of them deliberate: - `GH_TOKEN` is now just `github.token`; no PAT exists anywhere in the loop, and `agentEnv` strips only the R2 keys. The host still owns every write, but as a division of labour (one place to enforce branch/PR-marker rules), not as a permission boundary — if the agent wrote to GitHub itself, `main` would still be refused. - Job permissions rise to contents/issues/pull-requests write, because the loop now needs them for real; the ruleset, not the scope list, is what bounds them. - The loop can no longer change `.github/workflows/**`: the job token has no `workflows` permission and `permissions:` has no key to grant one. Such tasks route to a human, and `applyActions` recognises the rejection and says so instead of retrying. A PAT would push them and bring back the bypass — a real trade, resolved in favour of the gate. - `gh-check.mjs` is no longer a scope report: it asserts the two properties that matter (main is un-writable, the loop cannot edit the ruleset), then the scopes it needs to work, and exits non-zero when either property fails so it can gate the switchover. - The agent prompt now says plainly that the job token can write and that the ruleset is the boundary, rather than claiming a 403 that would no longer happen. Docs: `scripts/loop/README.md` gains "Branch protection is the gate" and "The ruleset, exactly" (the API payload and its verification); the L2a/L2b/L2c tier table is recorded as a dead end with the reason, so the next reader does not re-derive it. `docs/agents/ops.md` states the gate next to the no-self-merge rule. Verification: four local harnesses green (agentEnv, applyActions push/refusals incl. the workflow-file message, finishRun's push→PR→label path, entry.mjs end to end); `gh-check.mjs` run live against the repository reports main protection holding and exits 0. --- .github/workflows/loop.yml | 56 +++--- docs/agents/ops.md | 11 ++ scripts/loop/README.md | 323 ++++++++++++++++++---------------- scripts/loop/gh-check.mjs | 292 ++++++++++++++++-------------- scripts/loop/shared/agent.mjs | 73 ++++---- scripts/loop/shared/state.mjs | 42 +++-- 6 files changed, 432 insertions(+), 365 deletions(-) diff --git a/.github/workflows/loop.yml b/.github/workflows/loop.yml index 565a392..729fa43 100644 --- a/.github/workflows/loop.yml +++ b/.github/workflows/loop.yml @@ -50,15 +50,16 @@ concurrency: group: loop cancel-in-progress: false -# Least privilege: GitHub reads use GITHUB_TOKEN, the loop's writes use -# LOOP_GH_TOKEN. The agent never receives the PAT — `agentEnv` strips it and hands -# the child `github.token` instead — so `github.token` staying read-only is what -# makes "the agent cannot write to GitHub at all" a platform guarantee. Granting the -# job `contents: write` would hand the agent the merge permission. +# The loop's writes use the job token, and the *ruleset* is what keeps it out of +# `main`: `Main branch` requires every change to arrive through a pull request with +# one approving review, and its only bypass actor is the repository owner in +# `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. permissions: - contents: read - issues: read - pull-requests: read + contents: write + issues: write + pull-requests: write jobs: loop: @@ -101,15 +102,10 @@ jobs: LOOP_SMOKE: ${{ github.event.inputs.smoke || 'false' }} LOOP_MODEL: ${{ github.event.inputs.model || 'deepseek/deepseek-v4-flash' }} LOOP_RUN_TIMEOUT_MS: '3300000' # 55min, leaving headroom for push (job timeout 60min) - # The loop's write credential — a PAT with Contents write plus Issues/Pull - # requests write. Held by the *host* process (applyActions); `agentEnv` deletes - # it from the child environment, so the agent reads with `github.token` below - # and cannot write to GitHub however it is prompted. - GH_TOKEN: ${{ secrets.LOOP_GH_TOKEN || github.token }} - # Handed to the agent subprocess in place of GH_TOKEN. `github.token` here is - # capped at read by the `permissions:` block above, and the agent gets it in - # every mode — the loop's writes are the host's job, not the agent's. - GH_READ_TOKEN: ${{ github.token }} + # The loop's only GitHub credential: the job token. What it may do to `main` + # is decided by the `Main branch` ruleset, not by this file — see the + # `permissions:` block above. No PAT is involved anywhere in the loop. + GH_TOKEN: ${{ github.token }} # Credentials: secrets only DEEPSEEK_API_KEY: ${{ secrets.DEEPSEEK_API_KEY }} R2_ACCESS_KEY_ID: ${{ secrets.R2_ACCESS_KEY_ID }} @@ -143,12 +139,11 @@ jobs: # pull_request_target. - uses: actions/checkout@v4 with: - # L2c: its default is `true`, which writes the job token into the repository's - # git config so later steps can run authenticated git commands — including the - # agent's. That is the one credential the agent must never hold (it is the - # push/merge permission), and `agentEnv` cannot un-write a file. With this off, - # a `git push` from the agent has no credential at all; the host pushes through - # `http.extraheader` using its own GH_TOKEN. + # Its default is `true`, and it writes the job token into the repository's git + # config so that later `git` invocations pick it up implicitly — including the + # agent's. The loop does not need that: the host pushes through + # `http.extraheader` with an explicit token. Defence in depth, not the + # mechanism — what bounds the loop is the `Main branch` ruleset. persist-credentials: false ref: >- ${{ @@ -188,14 +183,13 @@ jobs: fi echo "model $want is available" - # Measure what the GitHub credential can actually do, before the agent can - # reach it. `GH_TOKEN` is `secrets.LOOP_GH_TOKEN || github.token`, so - # configuring a PAT silently changes every `gh` call in this job — it can - # narrow the read-only ones sweep depends on, or widen what the agent can - # write. Neither is visible from this file. Probes are side-effect free: - # they call write endpoints against an impossible id, so `403` means the - # scope is absent and `404`/`422` means it is present. - - name: Check GitHub credential + # Assert the loop's safety property before anything can act on it. The property + # is "nothing reaches `main` without a human", and it lives in repository + # configuration no file here can express — so it is measured, not restated: + # an idempotent write to `refs/heads/main` must be refused for a pull request, + # and the credential must not be able to edit the ruleset that refuses it. + # Side-effect free: the ref probe writes back the SHA it just read. + - name: Check the main-branch boundary run: node scripts/loop/gh-check.mjs - name: Pull state from R2 diff --git a/docs/agents/ops.md b/docs/agents/ops.md index 4c640ac..91b67db 100644 --- a/docs/agents/ops.md +++ b/docs/agents/ops.md @@ -73,3 +73,14 @@ Never do work outside the declared stage; never modify norms-layer files `loop-task: #`. - Don't claim a task whose lock is live; don't fight a live lock. - No self-merge; no norms-layer edits without a PR; no npm publish by the loop. +- **`main` is protected by the `Main branch` ruleset**, and that — not a credential + — is what makes the line above true: every change to `main` must arrive through a + pull request carrying one approving review, the only bypass actor is the repository + owner, and the job token has no `administration` scope, so the loop cannot weaken + the rule. A direct push to `main` is refused for every credential the loop can + hold. Do not plan around it, and do not try to reproduce it with token scoping: a + PAT acts as its owner, who *is* the bypass actor. Details and the exact payload: + `scripts/loop/README.md` § "Branch protection is the gate". +- The job token cannot push changes to `.github/workflows/**` (no `workflows` + permission exists for `permissions:` to grant). A task that needs one is not + loop work — route it to `needs-triage`/`ready-for-human`. diff --git a/scripts/loop/README.md b/scripts/loop/README.md index fae0192..7279361 100644 --- a/scripts/loop/README.md +++ b/scripts/loop/README.md @@ -46,55 +46,79 @@ pnpm loop summary | view # summary / task list | Level | Effect | | --- | --- | -| `report` | State layer + report only. GitHub write actions are **listed, never executed** (A1 semantics); the agent holds the read-only job token. | -| `auto` | Executes labels/comments/close, and for `pr-opened` the host pushes the branch and opens the PR. The agent holds `LOOP_GH_TOKEN` (Issues/Pull requests write, no `Contents`) — see the credential section below. | +| `report` | State layer + report only. GitHub write actions are **listed, never executed** (A1 semantics). | +| `auto` | Executes labels/comments/close, and for `pr-opened` the host pushes the branch and opens the PR. | In `report` mode the pending actions are appended to `state/reports/.md` as a checklist and surfaced in the Actions Step Summary, so a human can execute them by hand. -### GitHub write credentials (two roles) - -`GH_TOKEN: ${{ secrets.LOOP_GH_TOKEN || github.token }}`. Until L2 the secret was -deliberately unset, so `github.token` — capped read-only by the workflow's -`permissions:` block — was the only credential a run could reach, and a run that -ignored its prompt got a 403 from `gh pr merge`. Configuring the secret replaces it -for every `gh` call in the job, which is why `gh-check.mjs` measures rather than -assumes. - -There are two credentials, split by **who runs the code**, not by scope list: - -| Credential | Held by | Scopes | Used for | -| --- | --- | --- | --- | -| `github.token` (`GH_READ_TOKEN`) | the agent, in every mode | read-only (`permissions:`) | reading GitHub: triage, sweep, review | -| `LOOP_GH_TOKEN` (PAT) | the host process only | `Contents: RW` + `Issues: RW` + `Pull requests: RW` | every loop write: labels, comments, closing, branch push, opening the PR | - -The agent gets **no write credential at all**, in either mode. It does not need one: -`applyActions` derives the write list from the agent's `result.json` and executes it -in the host's process. That is why the guarantee does not depend on scoping one -token finely — a read-only agent cannot merge, push or relabel regardless of what -its prompt says, and the scope the host needs (`Contents: write`, which is also the -merge permission) exists only where the untrusted party cannot reach it. - -Configuring `LOOP_GH_TOKEN` lifts only the platform lock — `writeLevel` still -gates `applyActions` in `state.mjs`, and its default is `report`, so ordinary runs -keep reporting. - -The agent's side is enforced in code, not in prose: `agentEnv` in -`shared/agent.mjs` builds the child environment from scratch — it deletes -`LOOP_GH_TOKEN`, `GH_READ_TOKEN`, `GITHUB_TOKEN` and the R2 keys, then sets -`GH_TOKEN` to `GH_READ_TOKEN` (`github.token`, capped at read by `permissions:`). -`persist-credentials: false` on the checkout step closes the other half: without it -git's config would carry the job token, which no environment variable can revoke. - -Both are measurable without spending a token: -`node scripts/loop/gh-check.mjs` probes each credential's scopes side-effect -free (write endpoints against an impossible id — `403` means absent, `404`/`422` -means present, plus an idempotent write that restores the value it read) and -reports what the host holds and what the agent will hold. - -**The agent must never hold `Contents: write`.** Merging a PR is not under -"Pull requests" — it is under "Contents" (GitHub's endpoint→permission table): +### Branch protection is the gate + +`GH_TOKEN: ${{ github.token }}` — the job token, and the loop's **only** GitHub +credential. There is no PAT anywhere in the system. + +That is not a weakness, because the property being enforced is not "the agent holds +a narrow token" — it is **"nothing reaches `main` without a human"**, and that is +enforced by repository configuration, not by scope lists: + +``` +Main branch (ruleset, active, target ~DEFAULT_BRANCH) + rules: deletion · non_fast_forward · pull_request + └─ required_approving_review_count: 1 + bypass_actors: User(owner) with bypass_mode: pull_request +``` + +| Actor | direct push to `main` | merge a PR without an approval | +| --- | --- | --- | +| the owner (you) | ❌ refused | ✅ (bypass, `pull_request` mode) — **merging *is* the consent** | +| the loop (`github.token`) | ❌ refused — not a bypass actor | ❌ refused — and it cannot approve its own PR | + +Two consequences worth stating plainly: + +1. **`bypass_mode: pull_request` means the owner cannot push to `main` either.** + The ruleset is not "the loop is restricted"; it is "every change arrives through a + pull request", for everybody, with the owner able to merge their own without a + second reviewer. Verified: an idempotent write to `refs/heads/main` answers + `422 Changes must be made through a pull request`. +2. **A PAT would break this.** A user token *acts as its owner*, who is the bypass + actor — so a loop holding one could merge an unapproved PR, and its PRs would also + be authored by the owner, whom GitHub forbids approving their own work: the loop's + PRs would then be unmergeable *and* unapproved-mergeable. Both halves are wrong, + which is why the loop uses `github.token` and the PR author is + `github-actions[bot]` — an identity you *can* approve. + +`permissions:` cannot grant `administration`, so the loop cannot delete or weaken +that ruleset. `gh-check.mjs` asserts exactly that pair of properties (see below). + +What bounds the loop beyond `main` is therefore *not* a credential: the host owns +every write (`applyActions` derives them from the agent's `result.json`) as a division +of labour, and the agent is asked not to write — but if it did, the ruleset would +still refuse the only writes that matter. + +**Cost of this design: the loop cannot change `.github/workflows/**`.** The job token +carries no `workflows` permission and `permissions:` has no key that grants one, so a +push containing a workflow-file change is rejected outright +(`refusing to allow a GitHub App to create or update workflow … without workflows +permission`). A PAT would push it, and would also carry the bypass above — so this is +a real trade, resolved in favour of the gate. Tasks that need a workflow change go to +a human; `applyActions` recognises the error and says so in the run log. + +`node scripts/loop/gh-check.mjs` measures the whole boundary without spending a +token, in order of how much it matters: + +1. **Is `main` un-writable?** `PATCH /git/refs/heads/main` writing back the SHA it + just read — a `422 … pull request` is the pass, a `2xx` is a hard failure (and + changes nothing either way). +2. **Can the loop edit its own gate?** A deliberately invalid `POST /rulesets`: + `403` means no `administration`, `422` means the credential could remove the rule. +3. **Can the loop still work?** Reads, plus the Issues/Pull-requests/Contents writes + it needs for labels, comments and branch pushes. + +It exits non-zero when (1) or (2) fails, so the check can gate a switchover. + +**The scope facts behind that**, since they are easy to get wrong — merging a PR is +under "Contents", not "Pull requests": | Action | gh command | Permission required | | --- | --- | --- | @@ -112,96 +136,92 @@ Two consequences that are easy to get wrong: `PATCH /pulls/{n}`, reviews and review comments — the merge endpoint sits in the `Contents` section, because merging means writing commits to the target branch. -2. **"Can open a PR" and "can merge a PR" are the same permission** — within a - single credential. Opening a PR needs a head branch; creating one needs - `Contents: write`; and that is the merge permission. Separating them means - separating the *processes*, which is what L2c below does. - -So the L2 step has three sizes rather than two. Two are pure scope choices; the -third is the one that satisfies "loop may open PRs but must never merge them", -and it needs a small host change. - -| | Scopes / changes | Loop can | Loop cannot | -| --- | --- | --- | --- | -| **L2a** | `Issues: RW` + `Pull requests: RW` + `Contents: read` | label, comment, close, review | open PRs, merge, touch code or branches | -| **L2c** | L2a, **plus** the host pushes `loop/*` branches with its own token | label, comment, close, review, **open PRs** | **merge anything** | -| **L2b** | L2a + `Contents: RW` on the loop's own credential | everything, PR creation included | — (nothing; this is the unchecked size) | - -Prefer a **fine-grained** PAT scoped to `Alex1990/tiny-oss`; a classic `repo` -token grants all of the above at once, defeating the point. Set an expiry and -record who renews it, or L2 fails silently later. - -#### L2c — the loop opens PRs and still cannot merge - -Worth stating up front: **a single credential cannot express this.** Opening a -PR needs a head branch, creating one needs `Contents: write`, and that is the -same permission `PUT /pulls/{n}/merge` requires. The split has to happen across -*roles*, not inside one scope list: - -| Credential | Held by | Scopes | Used for | -| --- | --- | --- | --- | -| `GITHUB_TOKEN` (job) | the workflow's own steps | `contents: write` | creating and pushing the `loop/-*` branch | -| `LOOP_GH_TOKEN` (PAT) | the agent subprocess | `Issues: RW`, `Pull requests: RW` — **no `Contents`** | labels, comments, opening the PR | - -Two details are required and both are easy to miss: - -- `actions/checkout` must set **`persist-credentials: false`**. Its default is - `true` and writes the job token into the repository's git config so scripts - can run authenticated git commands — which would hand the agent exactly the - `Contents: write` this design exists to remove. -- Pushing is the *only* thing that has to move to a host step. Committing is a - local operation with no network credential, so the agent can still stage and - commit its work in the workspace; the host pushes the branch and opens the PR - from the agent's proposed metadata. That also fits the existing division of - labour — the agent proposes, the host executes. - -Result: the agent has no route to `PUT /pulls/{n}/merge` (403 — no `Contents`), -and no route to push anything anywhere. The loop still produces PRs. `no -self-merge` becomes a platform guarantee instead of a prompt instruction, which -is the whole point. - -One side effect to plan for: a PR created with `GITHUB_TOKEN` produces a -`pull_request` event whose workflow runs start in an **approval-required** state -(except `closed`/`labeled`/`edited`, which do not create runs at all). So the -loop will not triage or review its own PR, and CI will not run on it, until a -human clicks "Approve workflows to run". For this loop that is a feature — it -removes the recursive self-review the `pr-review` route would otherwise perform -on the loop's own output, and it matches D26's direction. A human merging the PR -still fires `pull_request_target.closed` normally, which is what drives the -`metrics` gate. - -Server-side protection does **not** substitute for any of this: this is a solo -repository (one collaborator, `admin: true`), so a ruleset requiring approvals -would block the owner's own PRs forever (GitHub forbids self-approval), and -adding an admin `bypass_actor` would let the loop's PAT bypass it too. The -`Main branch` ruleset is `enforcement: active`, but it contains only `deletion` -and `non_fast_forward`, so it never restricted merging either way. The guarantee -has to come from the credential split. - -#### L2c as implemented - -The split above is wired end to end. What a human has to configure is **one -secret**; everything else is code that fails closed. +2. **"Can open a PR" and "can merge a PR" are the same permission** — within a single + credential. Opening a PR needs a head branch; creating one needs `Contents: write`; + and that is also the merge permission. **No scope list separates them**, which is + why the separation here is not attempted with scopes: the loop may hold + `Contents: write` and still never reach `main`, because reaching `main` requires an + approval it cannot give itself. That is the design — see above. + +(An earlier revision of this file tried to express it with three credential tiers — +`L2a` read-only, `L2b` full `Contents`, `L2c` split across two tokens. It does not +work. A PAT belongs to its owner, who is this repository's only admin and therefore +the ruleset's bypass actor, so a "narrow" PAT still inherits the ability to merge +unapproved PRs; and because that same owner authors the PRs, GitHub forbids approving +them — so the loop's output would be simultaneously un-approvable and +unapproved-mergeable. The tiers are recorded only so the next reader does not +re-derive them.) + +### The ruleset, exactly + +This is the part a human owns, and the only part no file in this repo can express. The +`Main branch` ruleset (id `21855851`, `target: branch`, `conditions.ref_name.include: +["~DEFAULT_BRANCH"]`): + +```json +{ + "name": "Main branch", + "target": "branch", + "enforcement": "active", + "bypass_actors": [ + { "actor_id": , "actor_type": "User", "bypass_mode": "pull_request" } + ], + "conditions": { "ref_name": { "include": ["~DEFAULT_BRANCH"], "exclude": [] } }, + "rules": [ + { "type": "deletion" }, + { "type": "non_fast_forward" }, + { "type": "pull_request", "parameters": { + "required_approving_review_count": 1, + "dismiss_stale_reviews_on_push": true, + "require_code_owner_review": false, + "require_last_push_approval": false, + "required_review_thread_resolution": false, + "allowed_merge_methods": ["merge", "squash", "rebase"] + } } + ] +} +``` + +Applied with `gh api --method PUT repos/{owner}/{repo}/rulesets/21855851 --input +`, and verified by reading back `current_user_can_bypass` — it must come back +`pull_requests_only` for the owner. If it comes back `never`, the `actor_id` did not +match anyone and **you are locked out too**: you could not merge your own PR, because +GitHub forbids approving your own work. + +`bypass_mode: "pull_request"` is the load-bearing choice. `always` would let the owner +push straight to `main` (and any PAT ever handed to the loop would inherit that); +`pull_request` allows exactly one thing — merging a PR — which is the consent act. +The alternative, leaving `bypass_actors` empty, is stricter still and also coherent +(the owner would approve-and-merge loop PRs by hand), but it leaves the owner unable +to merge a PR they authored themselves. + +`dismiss_stale_reviews_on_push: true` means a push to a PR branch invalidates its +approval: approving a diff approves that diff, not whatever arrives next. + +#### What the loop relies on, piece by piece | Piece | Where | Behaviour | | --- | --- | --- | -| `LOOP_GH_TOKEN` secret | `.github/workflows/loop.yml` | The host's PAT: `Contents: RW` + `Issues: RW` + `Pull requests: RW`, scoped to this repository. The only write credential in the system. | -| `agentEnv(writeLevel)` | `shared/agent.mjs` | Builds the child environment from scratch: strips the PAT, `GITHUB_TOKEN` and the R2 keys; the agent's `GH_TOKEN` is `github.token` in **both** modes. | -| `persist-credentials: false` | the `actions/checkout` step | Without it the job token is written into the repo's git config, where the agent's `git push` would find it; `agentEnv` cannot un-write a file. | +| `Main branch` ruleset | repository settings (`gh-check.mjs` asserts it) | The gate. Every change to `main` arrives through a PR with one approval; the owner is the only bypass, and only for merging. | +| `permissions:` | `.github/workflows/loop.yml` | `contents: write`, `issues: write`, `pull-requests: write`. No `administration` — so the loop cannot edit the ruleset, verified by probe. | +| `GH_TOKEN` | job env | `github.token`. No PAT exists in the system, so no credential carries the owner's bypass. | +| `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 | `workflow_dispatch` still overrides per run, so the A1 rehearsal entry point is unchanged. | | `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. | -| `validatePush` | `entry.mjs` | `pr-opened` without a valid proposal (wrong branch, empty title, or `writeLevel=report`) is downgraded to `failed`, so a task can never sit in `waiting-merge` waiting for a PR nobody opened. | +| workflow-file recognition | `applyActions` | A push rejected for touching `.github/workflows/**` is explained (see the cost above) instead of retried. | +| `validatePush` | `entry.mjs` | `pr-opened` without a valid proposal (wrong branch, empty title, another task's number, or `writeLevel=report`) is downgraded to `failed`, so a task can never sit in `waiting-merge` waiting for a PR nobody opened. | | `ensureLoopPrBody` | `planActions` | Repairs the PR body's `Closes #` / `loop-task: #` markers. The router requires both, and a missing marker would make the loop's own PR look external — no acceptance row, and its merge would close the task by the wrong path. | | PR number recorded | `finishRun` | The number comes back from `gh pr create` and lands in `task.prs` + a `pr-created` timeline row. | | git identity | `agentEnv` | `GIT_AUTHOR_*`/`GIT_COMMITTER_*` are injected under `auto`; a fresh runner has no global git config, and the commit itself is local (no credential involved). | -The failure that remains possible is deliberate: with `auto` on and no PAT -configured, `GH_TOKEN` falls back to `github.token`, every product stage fails at -the push, and the task goes to the inbox. Withdrawing the host's write access is -exactly how you turn the loop back into a reporting one — the agent's side never -changes, because it never had write access to begin with. +Note what is **not** load-bearing: the host-versus-agent split. It exists because the +writes must be derived from the agent's result file in one place that can enforce the +branch and PR-marker rules — not because the host holds a credential the agent must +not see. If the agent wrote to GitHub itself, the only consequence that matters +(`main`) would still be refused by the ruleset. ## A1 architecture (Actions + R2) @@ -219,12 +239,11 @@ GitHub event ─▶ loop.yml (concurrency group `loop` = platform-level single w built in (`DEEPSEEK_API_KEY`), ~21 MB with no install scripts, Node >= 22.19. `--approve` is **mandatory**: without it non-interactive runs silently ignore project-local resources. -- **pi has no built-in permission gating** (verified), so the boundary is built - from the environment instead: the job's `permissions:` block, the absence of - secrets in fork contexts, and `agentEnv` (which strips the PAT, `GH_READ_TOKEN` - and the R2 keys from the child, then hands it the read-only job token). Do not - add write credentials to a job that touches untrusted input, and do not rely on - the prompt for any of it. +- **pi has no built-in permission gating** (verified), so the boundary is built from + the platform instead: the `Main branch` ruleset (asserted by `gh-check.mjs`), the + job's `permissions:` block, the absence of secrets in fork contexts, and `agentEnv` + (which strips the R2 keys from the child). Do not add write credentials to a job + that touches untrusted input, and do not rely on the prompt for any of it. - **One writer for closing:** the agent writes `state/reports/.result.json`; `entry.mjs` performs the transition via `finishRun`. Agents never run `pnpm loop end` themselves. @@ -374,10 +393,14 @@ so the account ID is part of an address, and bucket names are not sensitive): | --- | --- | | `R2_ACCOUNT_ID` | Cloudflare account ID | -Optional, with sane defaults: `LOOP_GH_TOKEN` (fine-grained PAT; unused while -report-only, since `github.token` covers read access) and `R2_BUCKET` (defaults -to the derived `loop-state--`). Both the `R2_ACCOUNT_ID` and -`R2_BUCKET` lookups fall back to the same-named secret, so either store works. +Optional, with a sane default: `R2_BUCKET` (defaults to the derived +`loop-state--`). The `R2_ACCOUNT_ID` and `R2_BUCKET` lookups fall +back to the same-named secret, so either store works. + +**No GitHub credential is configured.** `GH_TOKEN` is `github.token`, generated per +job. A `LOOP_GH_TOKEN` PAT used to live here and must not come back: it would act as +its owner, who is the `Main branch` ruleset's bypass actor — see "Branch protection +is the gate". ```bash gh secret set R2_ACCESS_KEY_ID -R Alex1990/tiny-oss @@ -444,7 +467,7 @@ context-growth drift, and the credential probes measured what the host and the agent actually hold rather than assuming it. What the week did *not* prove is that the loop is **complete**, and that is the -honest reason L2c is being landed now: with no write credential every proposal +honest reason the write path is being landed now: at the report boundary every proposal was unexecutable, so the write half of the host (`applyActions`, `planActions`'s `pr-create`, the acceptance gate) never met production. The unchecked boxes below are exactly that half, and they are the first items to @@ -502,9 +525,9 @@ sample once `auto` is switched on. ×2, `workflow-backfill`). D30/D33 cover the *shared* machinery (`markTaskTerminal`, the `taskId|event|pr` dedup key) and D33 verified the *no-write* half live; the write half (`planActions`) has never executed in - any host. Validating it needs `LOOP_GH_TOKEN` plus `execute_writes=true`, + any host. Validating it needs `execute_writes=true` plus a merged loop PR, i.e. leaving the report boundary — deliberately not done during the - observation week. **First item after the L2 switchover**: confirm the + observation week. **First item after the switchover**: confirm the first loop-PR merge writes exactly one row. - [x] Cost readable: `tokens` and `durationMs` land in the end row — real runs recorded 102,860 and 77,850 tokens; the install step prints @@ -520,15 +543,14 @@ sample once `auto` is switched on. `Install engine (pi)`, before `entry.mjs`, so they write no run row at all — see the failure-classification item above. (Distinction worth keeping: 21 workflow executions, 18 run records.) -- [x] No drift **and** no incorrect proposal for a week → **owner decided L2c** - (2026-09-14): the loop opens PRs but can never merge. Implemented as a - two-credential split — the host alone holds a PAT with `Contents: write` and - does every write, while the agent gets `github.token`, read-only, in both - modes; `persist-credentials: false` closes the git-config path. See "L2c as - implemented" and the endpoint-level evidence under "GitHub write - credentials". What remains is configuration, not code: one secret must exist - before `LOOP_EXECUTE_WRITES=true`, and the unchecked boxes in this list - become sampleable only then. +- [x] No drift **and** no incorrect proposal for a week → **gate moved to the + ruleset** (2026-09-15). The loop's safety property is now "nothing reaches + `main` without a human", enforced by the `Main branch` ruleset rather than by + scoping a credential: every change arrives through a PR with one approving + review, the owner is the only bypass actor (and only for merging), and the job + token carries no `administration` so the loop cannot weaken the rule. See + "Branch protection is the gate" and "The ruleset, exactly". This replaced an + earlier two-credential split that a PAT could not have kept honest. ### Not reachable under A1 (structural — decide before the L2 switchover) @@ -538,16 +560,17 @@ validated at the report boundary however long it runs: | Capability | Why it cannot run | Disposition | | --- | --- | --- | -| `metrics` write path | A1 produces no loop PR, so `handleMetrics` never fires | L2c makes it reachable; **still unverified — first item once `auto` is on** | -| `pr-review` route | also needs a loop PR; A1 never produces one | L2c makes it reachable; exercised in A0, never on Actions | +| `metrics` write path | A1 produces no loop PR, so `handleMetrics` never fires | the switchover makes it reachable; **still unverified — first item once `auto` is on** | +| `pr-review` route | also needs a loop PR; A1 never produces one | reachable after the switchover; exercised in A0, never on Actions | | External-PR read-only analysis | D28 — a fork/Dependabot `pull_request` run carries no secrets, so not even a read-only analysis can reach the LLM | accepted for A1; needs its own credentials to enable | -| Label/comment effects on GitHub | the report boundary never writes | L2c executes them; the first `auto` run is what turns "correct on paper" into "observed" | -| `execute_writes` rehearsal | `LOOP_GH_TOKEN` is unset, so `auto` cannot write even when requested | the code path is now exercised locally (push → PR → label, plus the dirty-tree, branch-shape and missing-credential refusals); it has never run on Actions | +| Label/comment effects on GitHub | the report boundary never writes | `auto` executes them; the first such run turns "correct on paper" into "observed" | +| `execute_writes` rehearsal | now runnable — `permissions:` carries the write scopes and the ruleset bounds what they can reach | the code path is exercised locally (push → PR → label, plus the dirty-tree, branch-shape and missing-credential refusals); it has never run on Actions | +| Changing `.github/workflows/**` | the job token has no `workflows` permission and `permissions:` cannot grant one | structural: such tasks route to a human. A PAT would push them **and** inherit the ruleset bypass — see the cost note above | | R2 bucket versioning | R2 offers no object versioning | accepted deviation from 05 §8 (see Environment facts); `push`/`seed` never use `--delete` | Consequence for the L2 decision: a clean observation week proves the loop is **safe**, not that it is **complete**. The write half of the host is the part -that has never met production, which is why L2c lands as its own change and why +that has never met production, which is why the write path lands separately and why the first `auto` run is a verification step rather than a milestone. ## Defect log diff --git a/scripts/loop/gh-check.mjs b/scripts/loop/gh-check.mjs index cb7ca7f..c10c8c6 100644 --- a/scripts/loop/gh-check.mjs +++ b/scripts/loop/gh-check.mjs @@ -1,31 +1,37 @@ #!/usr/bin/env node /** - * GitHub credential self-check — invoked as a standalone workflow step. + * Loop boundary self-check — invoked as a standalone workflow step. * * Usage: node scripts/loop/gh-check.mjs [owner/repo] * - * Why this exists: `GH_TOKEN` is `secrets.LOOP_GH_TOKEN || github.token`. The - * moment a real PAT is configured it replaces the job token for **every** `gh` - * call, including the read-only ones sweep depends on. A PAT that is narrower - * than the job token therefore breaks reads silently, and one that is broader - * silently widens what the loop can do. Neither is visible from the workflow - * file, so it has to be measured. + * The loop's safety property is not "the agent holds a weak credential" — host and + * agent share the job token, so there is only one. It is that **`main` cannot be + * changed without a human**: the `Main branch` ruleset requires every change to + * arrive through a pull request with one approving review, and the loop is not a + * bypass actor. This script measures that claim instead of restating it, because it + * depends on repository configuration that no file in this repo can express. * - * How permissions are measured — two methods, because the obvious one is wrong: + * Three questions, in order of how much they matter: * - * 1. GitHub's own `permissions` object for the credential. No inference. - * 2. **Idempotent writes** for scopes the object does not describe (Issues and - * Pull requests). The probe reads the current value and writes it straight - * back, so nothing changes — but a `2xx` means the write really executed, - * which only happens with the write scope. + * 1. Is `main` actually un-writable? An idempotent `PATCH /git/refs/heads/main` + * that writes back the SHA it just read: a ruleset-protected branch refuses it + * with `422 Changes must be made through a pull request`, and **nothing + * changes**. A `2xx` means the protection is gone — a hard failure. + * 2. Can the loop weaken that protection? `administration` is not a scope + * `permissions:` can grant, so the probe against the rulesets collection must + * be refused; if it is not, the loop could delete its own gate. + * 3. Can the loop still do its job? Reads (sweep depends on them) and the + * Issues/Pull-requests/Contents writes it needs for labels, comments and + * branch pushes. * - * The tempting method — call a write endpoint against an impossible id and read - * `403` as "no permission", `404` as "permission present" — does **not** work. - * Measured against a `github.token` capped to read-only by `permissions:`, the - * write probes returned `404`, not `403`. So `404` means only "the credential - * has *some* scope for this resource", read or write, and the method reports - * false positives. `403` remains meaningful for a scope the token lacks - * entirely, which is why the Actions and Secrets checks still use it. + * How scopes are measured — the tempting method is wrong and was tried first: + * calling a write endpoint against an impossible id and reading `403` as "no + * permission" / `404` as "permission present". Measured against a `github.token` + * capped to read-only by `permissions:`, those probes returned `404`, not `403`, so + * `404` means only "the credential has *some* scope for this resource" — the method + * reports false positives. What works is an **idempotent write**: read the current + * value, write it straight back, and require `2xx`. `403` remains meaningful for a + * scope the credential lacks entirely. * * The token itself is never printed. Only the login it authenticates as. */ @@ -47,13 +53,13 @@ function gh(args, { token = null } = {}) { status: m ? Number(m[1]) : (r.status === 0 ? 200 : null), out: (r.stdout || '').trim(), err: (r.stderr || '').trim(), + all: all.trim(), }; } /** A scope the credential lacks entirely answers `403`; anything else is "has some access". */ const hasAnyScope = (status) => status !== null && status !== 403 && status !== 401; -/** Identity is the decisive fact for a credential we did not choose. */ function identity(token) { const r = gh(['api', 'user', '--jq', '.login'], { token }); return r.ok ? r.out : null; @@ -74,35 +80,104 @@ function idempotentWrite(label, { read, write }, token) { return w.ok; } +/** + * The inverse: a write that **must** be refused for the loop's safety property to + * hold. Restores nothing, because a successful probe changes nothing (it writes back + * the value it read). + * Returns `true` when refused as expected, `false` when it went through (bad), or + * `null` when the probe could not run. + */ +function expectRefused(label, { read, write }, token, expected) { + const cur = gh(read, { token }); + if (!cur.ok || !cur.out) { + console.log(` UNKNOWN ${label} (nothing to probe against)`); + return null; + } + const w = gh(write(cur.out), { token }); + const refused = !w.ok; + console.log(` ${refused ? 'refused' : 'ALLOWED'} ${label} (HTTP ${w.status ?? 'n/a'})`); + if (refused && expected && !expected.test(w.all)) { + console.log(` ↳ refused, but not for the expected reason; got: ` + + `${w.all.replace(/\s+/g, ' ').slice(0, 160)}`); + } + if (!refused) { + console.log(` ↳ the write SUCCEEDED — the protection is not in place. ` + + `It was reverted to the value it already had, so nothing changed.`); + } + return refused; +} + const repo = process.argv[2] || process.env.GITHUB_REPOSITORY || ''; if (!repo) { console.error('[loop:gh] error: pass owner/repo or set GITHUB_REPOSITORY'); process.exit(1); } -const hostToken = process.env.GH_TOKEN || null; -const agentToken = process.env.GH_READ_TOKEN || null; +const token = process.env.GH_TOKEN || null; +const who = identity(token); +const isJob = who === 'github-actions[bot]'; +const source = !token + ? '(GH_TOKEN unset — gh used its own local login; the ruleset still applies)' + : isJob ? 'github.token (the job token — the loop\'s only credential)' + : `a user token for ${who} (PAT/OAuth — the loop should not use one: it carries the` + + ' owner\'s ruleset bypass)'; console.log(`[loop:gh] repo: ${repo}`); +console.log(`[loop:gh] credential: ${source}`); +if (who && !isJob && token) { + console.log(' WARNING: a user token is present. On Actions the loop must authenticate as' + + ' `github-actions[bot]`; a PAT acts as its owner, who *is* the ruleset bypass actor —' + + ' so the loop could merge an unapproved PR and the gate would be gone.'); +} -const hostWho = identity(hostToken); -if (!hostWho) { - // A token that cannot even identify itself is the one hard failure: every - // later `gh` call in the job would fail too, so fail loudly and early. - console.error('[loop:gh] error: GH_TOKEN cannot authenticate'); - process.exit(1); +/* ------------------------------------------------- 1. is main really protected? */ + +console.log('\n[loop:gh] MAIN PROTECTION — the property that matters:'); +const rules = gh(['api', `repos/${repo}/rules/branches/main`], { token }); +if (rules.ok) { + let types = []; + try { types = JSON.parse(rules.out).map((r) => r.type); } catch { /* keep [] */ } + console.log(` active rules on main: ${types.length ? types.join(', ') : '(none)'}`); + if (!types.includes('pull_request')) { + console.log(' ERROR: no `pull_request` rule on main — a direct push is allowed.'); + } } -const hostIsJob = hostWho === 'github-actions[bot]'; -// Locally there is no GH_TOKEN — `gh` falls back to the developer's own login, -// so saying "github.token" there would be plainly wrong. -const hostSource = !hostToken - ? '(GH_TOKEN unset — gh used its own local login)' - : hostIsJob ? 'github.token (job token)' : 'LOOP_GH_TOKEN (a PAT, replacing the job token)'; -console.log(`[loop:gh] host credential: ${hostSource} — authenticates as ${hostWho}`); - -console.log('\n[loop:gh] reads — the host depends on these; a miss breaks sweep:'); -// sweep lists open issues/PRs and calls `gh release list`; repo metadata is the -// cheapest liveness check. +const refProbe = { + read: ['api', `repos/${repo}/git/refs/heads/main`, '--jq', '.object.sha'], + write: (sha) => ['api', '--method', 'PATCH', + `repos/${repo}/git/refs/heads/main`, '-f', `sha=${sha}`], +}; +const mainRefused = expectRefused( + 'direct push to main (idempotent; writes back the SHA it read)', + refProbe, token, /pull request/i, +); + +/* ------------------------------------- 2. can the loop weaken that protection? */ + +console.log('\n[loop:gh] CAN THE LOOP EDIT ITS OWN GATE (administration)?'); +// An invalid create is the cheapest probe: `403` means no administration scope, +// `422` means the credential may write rulesets — the loop could remove the rule. +const adminProbe = gh(['api', '--method', 'POST', `repos/${repo}/rulesets`, + '-f', 'name=probe (invalid on purpose: no enforcement)'], { token }); +const canEditRulesets = adminProbe.status !== null + && adminProbe.status !== 403 && adminProbe.status !== 404; +// Locally there is no GH_TOKEN and `gh` uses the developer's own login, which is *meant* +// to be able to edit rulesets — the question is only what the loop's credential can do. +const usingLoopCredential = Boolean(token); +console.log(` ${canEditRulesets ? 'ALLOWED' : 'refused'} create a ruleset (HTTP ${adminProbe.status ?? 'n/a'})`); +if (canEditRulesets && usingLoopCredential) { + console.log(' ERROR: the loop\'s credential may write rulesets, so it could delete the' + + ' protection above. The job token must not carry `administration` — and since' + + ' `permissions:` cannot grant it, a PAT in the loop is the usual cause.'); +} else if (canEditRulesets) { + console.log(' (informational) your own login can edit rulesets — correct for a human' + + ' operator, and exactly what the loop must never be able to do. On Actions this' + + ' probe runs against `github.token` and must be refused.'); +} + +/* ------------------------------------------------- 3. can the loop still work? */ + +console.log('\n[loop:gh] reads — sweep depends on these; a miss breaks it:'); const reads = [ ['repo metadata (GET /repos)', ['api', `repos/${repo}`, '--jq', '.full_name']], ['issues list (sweep)', ['issue', 'list', '-R', repo, '--state', 'open', '--limit', '1']], @@ -110,13 +185,13 @@ const reads = [ ['releases list (sweep)', ['release', 'list', '-R', repo, '--limit', '1']], ]; const readResults = reads.map(([label, args]) => { - const r = gh(args, { token: hostToken }); + const r = gh(args, { token }); console.log(` ${r.ok ? 'OK ' : 'FAILED '} ${label}` + `${r.ok ? '' : ` — ${(r.err || r.status).toString().slice(0, 90)}`}`); return { label, ok: r.ok }; }); -console.log('\n[loop:gh] host write scopes (idempotent probes — each restores the value it read):'); +console.log('\n[loop:gh] write scopes (idempotent probes — each restores the value it read):'); const issueProbe = { read: ['api', `repos/${repo}/issues?state=open&per_page=1`, '--jq', `.[0] | "\\(.number)${SEP}\\(.title)"`], write: (cur) => { @@ -131,111 +206,68 @@ const prProbe = { return ['api', '--method', 'PATCH', `repos/${repo}/pulls/${n}`, '-f', `title=${title}`]; }, }; -const canLabel = idempotentWrite('Issues: write (label/comment/close)', issueProbe, hostToken); -idempotentWrite('Pull requests: write (open/edit PR, reviews)', prProbe, hostToken); - -// `PATCH /git/refs/{ref}` is in the Contents write set, and setting a ref to the -// SHA it already has is a no-op — so a 2xx means real write ability and nothing -// changes. -// -// This is the only sound way to measure Contents. The repository's -// `.permissions` object is NOT a token scope report: it describes the -// *authenticated user's* role, so it answers `push: true` for any token -// belonging to an admin, no matter how narrowly the token was scoped. Reading it -// here reported a Contents Read-only PAT as write-capable. -const defaultBranch = gh(['api', `repos/${repo}`, '--jq', '.default_branch'], - { token: hostToken }).out || 'main'; -const refProbe = { - read: ['api', `repos/${repo}/git/refs/heads/${defaultBranch}`, '--jq', '.object.sha'], - write: (sha) => ['api', '--method', 'PATCH', - `repos/${repo}/git/refs/heads/${defaultBranch}`, '-f', `sha=${sha}`], -}; -const canPush = idempotentWrite( - `Contents: write (MERGE + push + release)`, refProbe, hostToken, -); -const hostPerm = gh(['api', `repos/${repo}`, '--jq', '.permissions'], { token: hostToken }); -if (hostPerm.ok) { - console.log(` (info) repo .permissions = ${hostPerm.out.replace(/\s+/g, ' ')}` - + ' — the *user\'s* role, not this token\'s scopes; do not read it as a scope report'); -} -// `canPush` above is the host's ability to push `loop/-*` **and** to merge: one -// scope covers both, which is exactly why it must not exist on any credential the -// agent can reach. The agent check at the end of this script confirms it does not. +const canLabel = idempotentWrite('Issues: write (label/comment/close)', issueProbe, token); +idempotentWrite('Pull requests: write (open/edit PR)', prProbe, token); +// A ref that is *not* `main`: this one must succeed, which is how the loop pushes +// `loop/-*`. Probe against an existing branch so the write is a no-op. +const branchList = gh(['api', `repos/${repo}/branches?per_page=1`, '--jq', '.[0].name'], { token }); +const probeBranch = branchList.ok && branchList.out ? branchList.out : null; +const canPushBranches = probeBranch + ? idempotentWrite(`Contents: write (push loop/-* branches)`, { + read: ['api', `repos/${repo}/git/refs/heads/${probeBranch}`, '--jq', '.object.sha'], + write: (sha) => ['api', '--method', 'PATCH', + `repos/${repo}/git/refs/heads/${probeBranch}`, '-f', `sha=${sha}`], + }, token) + : null; -console.log('\n[loop:gh] scopes reached by a credential that lacks them entirely:'); -const absent = [ +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 absent) { - const r = gh(args, { token: hostToken }); +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: hostToken }); +const secrets = gh(['api', `repos/${repo}/actions/secrets`], { token }); console.log(` ${secrets.ok ? 'present' : 'ABSENT '} Secrets access (list/overwrite CI secrets)`); +/* -------------------------------------------------------------------- verdict */ + const brokenReads = readResults.filter((r) => !r.ok); const writeLevel = process.env.LOOP_WRITE_LEVEL ?? (process.env.LOOP_EXECUTE_WRITES === 'true' ? 'auto' : 'report'); + console.log(''); -if (canPush) { - console.log('[loop:gh] host credential holds Contents:write — branch pushes and PR creation are' - + ' possible. This is required for L2c, and it is also the merge permission, which is why' - + ' the agent is never given this credential (see the check at the end). One fine-grained' - + ' PAT with Contents + Issues + Pull requests write, scoped to this repository, is the' - + ' whole configuration.'); -} else if (canPush === false) { - console.log('[loop:gh] host credential holds no Contents:write — the loop cannot push a branch,' - + ' so no product stage can ever reach `pr-opened`. Keep LOOP_EXECUTE_WRITES=false until' - + ' LOOP_GH_TOKEN carries Contents: Read and write on this repository.'); -} -console.log(`[loop:gh] label/comment/close: ${canLabel ? 'available' : 'NOT available — the loop cannot do its job'}`); +console.log(`[loop:gh] main protection: ${mainRefused === null ? 'UNKNOWN' + : mainRefused ? 'holding — a direct push is refused' : 'MISSING — a direct push succeeded'}`); +console.log(`[loop:gh] gate integrity: ${canEditRulesets + ? (usingLoopCredential ? 'loop can edit its own ruleset (bad)' : 'your login can (loop credential not in play)') + : 'loop cannot edit the ruleset'}`); +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] reads: ${brokenReads.length ? `${brokenReads.length} FAILED — the host's sweep will break: ${brokenReads.map((r) => r.label.trim().split(/\s+/)[0]).join(', ')}` : 'all OK'}`); -console.log(`[loop:gh] write level (this configuration): ${writeLevel}`); -if (writeLevel === 'auto' && !canLabel) { - console.log(' WARNING: `auto` is switched on but the credential cannot label/comment — the loop' - + ' will run, propose, and silently fail to write. Configure LOOP_GH_TOKEN first.'); -} -if (writeLevel === 'auto' && canPush === false) { - console.log(' WARNING: `auto` is switched on but the host cannot push — every product stage will' - + ' fail at the push and land in the human inbox. Either grant LOOP_GH_TOKEN Contents:' - + ' Read and write, or keep LOOP_EXECUTE_WRITES=false.'); +console.log(`[loop:gh] write level: ${writeLevel}`); +if (writeLevel === 'auto' && (!canLabel || canPushBranches === false)) { + console.log(' WARNING: `auto` is on but a write scope is missing — runs will fail after' + + ' spending tokens. Fix the `permissions:` block, or keep LOOP_EXECUTE_WRITES=false.'); } -// What the agent actually holds, after `agentEnv` has built the child environment. The -// agent is read-only in **every** mode, so this check is not conditional on the write -// level: the loop's writes are the host's job, and a credential that could write here -// would be a configuration error, not a mode. -// -// Identity cannot be read from `GET /user` for `github.token`: it is a GitHub App -// installation token and is refused there entirely, which is why these checks combine -// two signals that *are* observable — whether the credential can read the repository, -// and whether a write is refused. -console.log('\n[loop:gh] credential handed to the agent (agentEnv: read-only in every mode):'); -if (!agentToken) { - console.log(' (GH_READ_TOKEN is not set, so agentEnv leaves the child without GH_TOKEN — `gh`' - + ' there would fall back to any local login. On Actions this variable is always set.)'); -} else { - const canRead = gh(['api', `repos/${repo}`, '--jq', '.full_name'], { token: agentToken }); - const asUser = gh(['api', 'user', '--jq', '.login'], { token: agentToken }); - console.log(` can read the repo: ${canRead.ok ? 'yes' : `NO — the agent cannot read GitHub (${canRead.status})`}`); - console.log(` acts as a user: ${asUser.ok ? `yes (${asUser.out}) — a PAT or user token` : 'no — a GitHub App token, i.e. github.token'}`); - const agentWrite = idempotentWrite('Issues: write', issueProbe, agentToken); - const agentPush = idempotentWrite('Contents: write', refProbe, agentToken); - - if (!canRead.ok) { - console.log(' WARNING: the agent cannot read GitHub, so triage/sweep will find nothing.'); - } else if (agentWrite || agentPush) { - console.log(' WARNING: the agent CAN WRITE. It should hold `github.token` capped at read by' - + " the workflow's `permissions:` block, so check that block — and check that GH_READ_TOKEN" - + ' is `github.token`, not a PAT.'); - } else { - console.log(' OK: the agent can read GitHub and cannot write anything (no Issues, no Contents)' - + ' — so it cannot label, comment, push or merge, whatever its prompt says. Every write is' - + ' the host\'s, executed from the agent\'s result file.'); - } +// The two properties that must hold before the loop is allowed to write anything: +// it cannot reach `main` directly, and it cannot take the protection away. +if (mainRefused === false || (canEditRulesets && usingLoopCredential)) { + console.log('\n[loop:gh] FAIL: the loop\'s safety property is not satisfied — see the' + + ' ERROR lines above. Do not enable `auto`.'); + process.exit(1); +} +if (mainRefused === null) { + console.log('\n[loop:gh] WARNING: main protection could not be measured (no ref to probe).'); } +console.log('[loop:gh] OK: `main` requires a reviewed pull request, the loop cannot edit that' + + ' rule, and the loop\'s own writes work.'); diff --git a/scripts/loop/shared/agent.mjs b/scripts/loop/shared/agent.mjs index ee84150..84845fa 100644 --- a/scripts/loop/shared/agent.mjs +++ b/scripts/loop/shared/agent.mjs @@ -52,12 +52,20 @@ export function buildPrompt({ task, stage, runId, writeLevel, mode, repo, lead = } L.push(''); L.push(`## Write boundary: ${writeLevel}`); - L.push('### GitHub: you never write'); - L.push('Your GitHub credential is read-only in **every** mode — `gh issue edit` /'); - L.push('`gh issue comment` / `gh issue close` / `gh pr create` / `gh pr merge` /'); - L.push('`git push` all answer 403. That is deliberate, not a misconfiguration: do not'); - L.push('retry them, and do not look for another credential. Label/comment/close/PR'); - L.push('actions belong to the host, which executes them from your result file.'); + L.push('### GitHub: the host writes, you do not'); + L.push('Your `GH_TOKEN` is the job token and it *can* write — labels, comments,'); + L.push('branches, pull requests. Do not use it for that. Every GitHub write in this'); + L.push('system is executed by the host process from your result file: that is the'); + L.push('division of labour, not a limitation you should work around. Do not run'); + L.push('`gh issue edit|comment|close`, `gh pr create`, `gh pr merge`, `gh pr review` or'); + L.push('`git push` yourself.'); + L.push(''); + L.push('It is also not the only line of defence, and deliberately so: `main` is'); + L.push('protected by the `Main branch` ruleset — every change must arrive through a pull'); + L.push('request carrying one approving review, the only bypass actor is the repository'); + L.push('owner, and this job has no `administration` scope. A direct push to `main`, an'); + L.push('unapproved merge, or an attempt to weaken the ruleset fails for *any* credential'); + L.push('you could hold. Do not spend turns probing it.'); L.push(''); L.push('Allowed: reading GitHub, editing the working tree, running the repo gates,'); L.push('reading and writing `state/`.'); @@ -80,6 +88,12 @@ export function buildPrompt({ task, stage, runId, writeLevel, mode, repo, lead = L.push(' to push a dirty tree, since the branch would silently lose the uncommitted work.'); L.push('3. Propose the PR in your result file (closing ritual below). The host pushes the'); L.push(' branch, opens the PR and records the PR number it gets back.'); + L.push(''); + L.push('Note the consequence for your own work: `git push` here fails for any commit'); + L.push('touching `.github/workflows/**` — the job token carries no `workflows`'); + L.push('permission and no `permissions:` value can grant one. A task that requires a'); + L.push('workflow-file change cannot be completed by this loop: report it and let a human'); + L.push('do it, rather than producing a PR that cannot be pushed.'); } if (mode === 'readonly') { L.push(''); @@ -157,36 +171,26 @@ export const SESSION_HINT = 'session files land under --session-dir (uploaded as /** * The environment handed to the agent subprocess. * - * `{ ...process.env }` used to give the child everything the host holds, including a - * push-capable token and the R2 keys. Now the agent holds **no write credential at - * all**, at either write level: - * - * agent → `GH_READ_TOKEN` (github.token, capped at read by the workflow's - * `permissions:` block — a platform guarantee, not a prompt one) - * host → the PAT, used by `applyActions` in its own process - * - * The agent does not need write access: every GitHub write is the host's job - * (`applyActions` executes the label/comment/close/push/PR actions that - * `planActions` derives from the agent's result file). Restricting it to reads means - * "the loop never self-merges" no longer depends on scoping one credential finely — - * the agent simply cannot write, whichever mode is running. + * The agent holds the job token, because a job has exactly one and both the host and + * the agent run inside it. What keeps the loop out of `main` is therefore **not** the + * agent's credential — it is the `Main branch` ruleset: every change must arrive + * through a pull request with one approving review, the sole bypass is the repository + * owner, and this job's `permissions:` carry no `administration`, so the loop cannot + * weaken that ruleset either. See "Branch protection is the gate" in + * scripts/loop/README.md. * - * Everything that authenticates to something else is deleted too. Locally both - * variables are unset, so no `GH_TOKEN` key survives and `gh` falls back to the - * developer's own login — the correct behaviour for a local run. + * What is still worth removing here is everything the agent could reach that the + * ruleset does **not** cover: the R2 credentials address the state-layer bucket + * directly, and only the host's `r2-sync` steps need them. */ export function agentEnv(writeLevel, base = process.env) { const env = { ...base }; for (const k of [ - 'LOOP_GH_TOKEN', 'GH_READ_TOKEN', 'GITHUB_TOKEN', 'R2_ACCESS_KEY_ID', 'R2_SECRET_ACCESS_KEY', 'R2_ACCOUNT_ID', 'R2_BUCKET', ]) delete env[k]; - if (base.GH_READ_TOKEN) env.GH_TOKEN = base.GH_READ_TOKEN; - else delete env.GH_TOKEN; // Product stages commit the branch the host pushes, and the identity must not come // from the sandbox's global git config (there is none on a fresh runner). Set it - // here rather than making the agent discover it: a commit is a local operation, and - // these four variables are the only thing git needs for one. + // here rather than making the agent discover it: a commit is a local operation. if (writeLevel === 'auto') { env.GIT_AUTHOR_NAME = env.GIT_AUTHOR_NAME ?? 'tiny-oss loop'; env.GIT_AUTHOR_EMAIL = env.GIT_AUTHOR_EMAIL ?? 'loop@users.noreply.github.com'; @@ -214,17 +218,12 @@ export function runPi({ // another CLI or wrapper script). const engine = (process.env.LOOP_ENGINE_CMD || 'pi').split(/\s+/).filter(Boolean); - // The agent never inherits a credential by accident: see `agentEnv` above. It is - // read-only in every mode; the host's own writes (applyActions) use the real - // GH_TOKEN in the host's process. + // The agent holds the same job token the host does — a job has one. What keeps the + // loop out of `main` is the branch ruleset, not a credential difference; see + // `agentEnv` above and "Branch protection is the gate" in scripts/loop/README.md. const childEnv = agentEnv(writeLevel); - if (childEnv.GH_TOKEN) { - log('[loop] agent GitHub credential: read-only (github.token) — it cannot write to' - + ' GitHub in any mode'); - } else { - log('[loop] agent GitHub credential: none — `gh` there would fall back to any local' - + ' login, so the run may not be able to read GitHub at all'); - } + log('[loop] agent GitHub credential: the job token (its write reach is bounded by the' + + ' `Main branch` ruleset, not by this variable)'); const child = spawn(engine[0], [...engine.slice(1), ...args], { cwd, env: childEnv, stdio: ['pipe', 'pipe', 'pipe'], diff --git a/scripts/loop/shared/state.mjs b/scripts/loop/shared/state.mjs index 74de36f..d425c96 100644 --- a/scripts/loop/shared/state.mjs +++ b/scripts/loop/shared/state.mjs @@ -405,11 +405,10 @@ export function planPush(t, push, repo) { * only" switch: no apply, no GitHub writes. * * `push` is the agent's PR proposal (`{ branch, base, title, body }`), read from - * `.result.json`. It only matters for `pr-opened`: under L2c the agent - * commits locally and the *host* pushes and opens the PR, so the label and the - * comment are queued behind `pr-create` — `applyActions` aborts the chain when - * the push or the PR creation fails, rather than labelling a PR that never - * appeared. + * `.result.json`. It only matters for `pr-opened`: the agent commits locally + * and the *host* pushes and opens the PR, so the label and the comment are queued + * behind `pr-create` — `applyActions` aborts the chain when the push or the PR + * creation fails, rather than labelling a PR that never appeared. */ export function planActions(t, outcome, { label, comment, note, push = null } = {}) { if (!t.url) return []; // a local synthetic task has no GitHub target @@ -469,11 +468,12 @@ const redact = (s) => String(s ?? '') * warn; local state is never rolled back. * * This is the only place the loop writes to GitHub, and it runs in the host's own - * process — the agent holds a read-only credential and cannot reach any of it. - * `push` is the single action needing `Contents: write`, so it spends the host's - * `GH_TOKEN` (the PAT); every `gh` call uses the same credential. A failed - * `push`/`pr-create` aborts the rest of the chain instead of labelling a PR that - * does not exist. + * process — a division of labour, not a permission boundary: host and agent share the + * job token, and what actually bounds the loop is the `Main branch` ruleset (every + * change through a reviewed PR). The host still owns the writes so that they come + * from the agent's *result file*, in one place that can enforce the branch-name, + * clean-tree and PR-marker rules. A failed `push`/`pr-create` aborts the rest of the + * chain instead of labelling a PR that does not exist. */ export function applyActions(actions, { log = console.log, warn = console.warn, cwd = process.cwd(), exec = spawnSync, @@ -488,15 +488,15 @@ export function applyActions(actions, { const gh = (args) => exec('gh', args, { encoding: 'utf8' }); const sub = a.gh ?? 'issue'; // PRs go to gh pr, issues to gh issue if (a.kind === 'push') { - // The host's own credential — one PAT holding `Contents: write` plus the - // Issues/Pull-requests scopes `gh` needs. The agent never receives it - // (`agentEnv` strips it), so a push is only ever performed here, by trusted - // code, on a ref the check below has already constrained. + // The job token. It is the only credential in the system, and it is deliberately + // *not* what bounds the loop: the `Main branch` ruleset refuses any push to a + // protected ref that is not part of a reviewed pull request, whichever token + // performs it. This check stays because the host pushes exactly one ref shape. const token = process.env.GH_TOKEN; if (!token) { warn('[loop] warn: the host has no GitHub credential (GH_TOKEN unset) — cannot push.' - + ' On Actions, configure LOOP_GH_TOKEN with Contents write plus Issues/Pull requests' - + ' write; locally, export GH_TOKEN (e.g. `GH_TOKEN=$(gh auth token)`).'); + + ' On Actions this is `github.token`; locally, export GH_TOKEN (e.g.' + + ' `GH_TOKEN=$(gh auth token)`).'); aborted = 'push'; continue; } @@ -529,7 +529,15 @@ export function applyActions(actions, { 'push', `https://github.com/${a.repo}.git`, `HEAD:refs/heads/${a.branch}`, ], { cwd, encoding: 'utf8' }); if (r.status !== 0) { - warn(`[loop] warn: push ${a.branch} failed: ${redact(r.stderr)}`); + const err = redact(r.stderr); + warn(`[loop] warn: push ${a.branch} failed: ${err}`); + if (/workflow.*permission|refusing to allow/i.test(err)) { + warn('[loop] warn: the commit touches `.github/workflows/**`, which the job token' + + ' can never push (a `permissions:` block cannot grant the `workflows`' + + ' permission, and a PAT would carry the owner\'s ruleset bypass — see' + + ' "Branch protection is the gate" in scripts/loop/README.md). This task cannot' + + ' be completed by the loop: report it for a human instead of retrying.'); + } aborted = 'push'; } else { log(`[loop] git: pushed ${a.branch}`); applied.push(a); } } else if (a.kind === 'pr-create') { From 27bcaf2997b7ffd8b09a562b30824326350a18dd Mon Sep 17 00:00:00 2001 From: Alex Chao Date: Wed, 16 Sep 2026 00:46:35 +0800 Subject: [PATCH 4/5] chore(loop): tell a job token apart from a PAT in the boundary check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Actions run showed the check reporting its own job token as "a user token for null": `GET /user` is refused for an installation token, and the code read that refusal as a missing login rather than as the identifying signal it is. Since the whole point of the check is to say which credential the loop holds, getting that wrong is worse than not asking. `identity()` now returns the status, and the three outcomes are distinguished: a login (user token — the configuration that would defeat the ruleset), a 403/404 (`github.token`, refused there by design), and a 401 (an unusable credential, said plainly instead of being mislabelled as the job token). Verified against all three: Actions (job token, reported correctly), the local developer login, and a deliberately invalid token (reported as unusable). --- scripts/loop/gh-check.mjs | 35 +++++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/scripts/loop/gh-check.mjs b/scripts/loop/gh-check.mjs index c10c8c6..d710bc4 100644 --- a/scripts/loop/gh-check.mjs +++ b/scripts/loop/gh-check.mjs @@ -60,9 +60,15 @@ function gh(args, { token = null } = {}) { /** A scope the credential lacks entirely answers `403`; anything else is "has some access". */ const hasAnyScope = (status) => status !== null && status !== 403 && status !== 401; +/** + * `GET /user` is the decisive probe for *what kind of* credential this is: + * a user token (PAT/OAuth) answers with a login; a GitHub App installation token — + * i.e. `github.token` — is refused there entirely, which is exactly the signal we + * want. `ok: false` therefore means "App token", not "broken credential". + */ function identity(token) { const r = gh(['api', 'user', '--jq', '.login'], { token }); - return r.ok ? r.out : null; + return { login: r.ok ? r.out : null, ok: r.ok, status: r.status }; } /** @@ -115,19 +121,28 @@ if (!repo) { const token = process.env.GH_TOKEN || null; const who = identity(token); -const isJob = who === 'github-actions[bot]'; +// A token that cannot answer `GET /user` is an installation token — the job token. A +// 401 means the credential itself is unusable, which is worth saying rather than +// mislabelling; a 403/404 is `github.token` being refused there, as designed. +const isUserToken = Boolean(token) && who.ok; +const isAppToken = Boolean(token) && !who.ok && who.status !== 401; const source = !token - ? '(GH_TOKEN unset — gh used its own local login; the ruleset still applies)' - : isJob ? 'github.token (the job token — the loop\'s only credential)' - : `a user token for ${who} (PAT/OAuth — the loop should not use one: it carries the` - + ' owner\'s ruleset bypass)'; + ? '(GH_TOKEN unset — gh used its own local login; the ruleset applies to it all the same)' + : isUserToken + ? `a user token for ${who.login} (PAT/OAuth)` + : isAppToken + ? 'github.token (the job token — the loop\'s only credential)' + : `an unusable credential (GET /user answered ${who.status ?? 'nothing'})`; console.log(`[loop:gh] repo: ${repo}`); console.log(`[loop:gh] credential: ${source}`); -if (who && !isJob && token) { - console.log(' WARNING: a user token is present. On Actions the loop must authenticate as' - + ' `github-actions[bot]`; a PAT acts as its owner, who *is* the ruleset bypass actor —' - + ' so the loop could merge an unapproved PR and the gate would be gone.'); +if (isUserToken) { + console.log(' WARNING: the loop is authenticating with a *user* token. On Actions it must' + + ' be `github.token`: a PAT acts as its owner, who is the ruleset bypass actor, so the' + + ' loop could merge an unapproved PR and the gate would be gone.'); +} else if (token && !isUserToken && !isAppToken) { + console.log(' WARNING: the credential is not usable — every probe below will report' + + ' UNKNOWN or absent scopes, and the loop cannot read GitHub either.'); } /* ------------------------------------------------- 1. is main really protected? */ From e9fd860e2b14c46b13344df4a13e8d70ee5ade33 Mon Sep 17 00:00:00 2001 From: Alex Chao Date: Wed, 16 Sep 2026 00:50:14 +0800 Subject: [PATCH 5/5] docs(loop): note the unattributed-approval ruleset default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GitHub adds `require_extra_approval_for_unattributed_changes: true` to any ruleset carrying a `pull_request` rule — a public-preview default that appears in the read-back payload without having been set. It targets Copilot-authored PRs, so it does not touch the loop's path (its PRs come from `github-actions[bot]`, and the owner's bypass merges regardless of the count), but a future reader comparing the payload to this file would otherwise have to rediscover where the parameter came from. --- scripts/loop/README.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/scripts/loop/README.md b/scripts/loop/README.md index 7279361..0925a89 100644 --- a/scripts/loop/README.md +++ b/scripts/loop/README.md @@ -198,6 +198,13 @@ to merge a PR they authored themselves. `dismiss_stale_reviews_on_push: true` means a push to a PR branch invalidates its approval: approving a diff approves that diff, not whatever arrives next. +GitHub also sets `require_extra_approval_for_unattributed_changes: true` on rulesets +carrying a `pull_request` rule (a public-preview default, not something set here). +It applies to Copilot-authored PRs and asks for one approval beyond the configured +count. It does not change the loop's path — its PRs come from `github-actions[bot]`, +and the owner's bypass allows the merge regardless of the count — but it is the +reason a future reader may see the parameter and wonder where it came from. + #### What the loop relies on, piece by piece | Piece | Where | Behaviour |