diff --git a/.github/workflows/claude-assist.yml b/.github/workflows/claude-assist.yml index 8365b1f..24feb12 100644 --- a/.github/workflows/claude-assist.yml +++ b/.github/workflows/claude-assist.yml @@ -45,11 +45,16 @@ concurrency: cancel-in-progress: false jobs: + # The picker runs INSIDE this job, after the last step that can still say + # no. It is itself a hosted job, so it must never run for an event that will + # not proceed, and folding it in here means a proceeding run costs one + # hosted start instead of two. gate: runs-on: ubuntu-latest - timeout-minutes: 5 + timeout-minutes: 10 # the picker calls the org API and may retry outputs: proceed: ${{ steps.gate.outputs.proceed }} + runner: ${{ steps.pick.outputs.runner }} steps: - id: gate uses: dodi-smart/.github/actions/agent-gate@v1 @@ -71,19 +76,21 @@ jobs: bots: 'reject' skip-draft: 'false' - pick-runner: - needs: [gate] - if: needs.gate.outputs.proceed == 'true' - uses: dodi-smart/.github/.github/workflows/pick-runner.yml@v1 - with: - weight: ${{ inputs.runner-weight }} - labels: ${{ inputs.runner-labels }} - secrets: inherit + # Only when the run above is still proceeding: a kill-switched or + # reserved-verb mention must never pay for a hosted picker. + - id: pick + if: steps.gate.outputs.proceed == 'true' + uses: dodi-smart/.github/actions/pick-runner@v1 + with: + weight: ${{ inputs.runner-weight }} + labels: ${{ inputs.runner-labels }} + gh-app-client-id: ${{ secrets.GH_APP_CLIENT_ID }} + gh-app-private-key: ${{ secrets.GH_APP_PRIVATE_KEY }} assist: - needs: [gate, pick-runner] + needs: [gate] if: needs.gate.outputs.proceed == 'true' - runs-on: ${{ fromJson(needs.pick-runner.outputs.runner) }} + runs-on: ${{ fromJson(needs.gate.outputs.runner) }} timeout-minutes: ${{ inputs.timeout-minutes }} permissions: contents: read diff --git a/.github/workflows/deps-verify.yml b/.github/workflows/deps-verify.yml index d95f602..8d6617b 100644 --- a/.github/workflows/deps-verify.yml +++ b/.github/workflows/deps-verify.yml @@ -54,12 +54,17 @@ concurrency: cancel-in-progress: true jobs: + # The picker runs INSIDE this job, after the last step that can still say + # no. It is itself a hosted job, so it must never run for an event that will + # not proceed, and folding it in here means a proceeding run costs one + # hosted start instead of two. gate: runs-on: ubuntu-latest - timeout-minutes: 5 + timeout-minutes: 10 # the picker calls the org API and may retry outputs: proceed: ${{ steps.gate.outputs.proceed }} stack: ${{ steps.stack.outputs.stack }} + runner: ${{ steps.pick.outputs.runner }} steps: # `bots: only` is the inverse of every other agent workflow: this one runs # for Renovate and Dependabot and nothing else. It also absorbs the author @@ -94,21 +99,23 @@ jobs: echo "stack=$s" >> "$GITHUB_OUTPUT" echo "stack: $s" - pick-runner: - needs: [gate] - if: needs.gate.outputs.proceed == 'true' - uses: dodi-smart/.github/.github/workflows/pick-runner.yml@v1 - with: - weight: ${{ inputs.runner-weight }} - labels: ${{ inputs.runner-labels }} - secrets: inherit + # Only when the run above is still proceeding: a human-authored PR (this + # workflow runs for bots only) must never pay for a hosted picker. + - id: pick + if: steps.gate.outputs.proceed == 'true' + uses: dodi-smart/.github/actions/pick-runner@v1 + with: + weight: ${{ inputs.runner-weight }} + labels: ${{ inputs.runner-labels }} + gh-app-client-id: ${{ secrets.GH_APP_CLIENT_ID }} + gh-app-private-key: ${{ secrets.GH_APP_PRIVATE_KEY }} # Deterministic gates FIRST. If the build is red the compiler has already # explained why, and an agent adds nothing but cost. build: - needs: [gate, pick-runner] + needs: [gate] if: needs.gate.outputs.proceed == 'true' - runs-on: ${{ fromJson(needs.pick-runner.outputs.runner) }} + runs-on: ${{ fromJson(needs.gate.outputs.runner) }} timeout-minutes: 25 outputs: verdict: ${{ steps.result.outputs.verdict }} @@ -170,9 +177,9 @@ jobs: fi review: - needs: [gate, pick-runner, build] + needs: [gate, build] if: needs.gate.outputs.proceed == 'true' - runs-on: ${{ fromJson(needs.pick-runner.outputs.runner) }} + runs-on: ${{ fromJson(needs.gate.outputs.runner) }} timeout-minutes: 20 permissions: contents: read diff --git a/.github/workflows/issue-implement.yml b/.github/workflows/issue-implement.yml index c7ead9b..e049565 100644 --- a/.github/workflows/issue-implement.yml +++ b/.github/workflows/issue-implement.yml @@ -50,14 +50,19 @@ concurrency: cancel-in-progress: false # never kill a run that may have pushed a branch jobs: + # The picker runs INSIDE this job, after the last step that can still say + # no. It is itself a hosted job, so it must never run for an event that will + # not proceed, and folding it in here means a proceeding run costs one + # hosted start instead of two. gate: runs-on: ubuntu-latest - timeout-minutes: 5 + timeout-minutes: 10 # the picker calls the org API and may retry permissions: contents: read issues: write # to explain a refusal and clear the request label outputs: proceed: ${{ steps.check.outputs.proceed }} + runner: ${{ steps.pick.outputs.runner }} steps: # agent:no-touch first, unconditionally. - id: gate @@ -143,19 +148,21 @@ jobs: echo "proceed=true" >> "$GITHUB_OUTPUT" - pick-runner: - needs: [gate] - if: needs.gate.outputs.proceed == 'true' - uses: dodi-smart/.github/.github/workflows/pick-runner.yml@v1 - with: - weight: ${{ inputs.runner-weight }} - labels: ${{ inputs.runner-labels }} - secrets: inherit + # Only when the run above is still proceeding: an unplanned issue or a + # refused actor must never pay for a hosted picker. + - id: pick + if: steps.check.outputs.proceed == 'true' + uses: dodi-smart/.github/actions/pick-runner@v1 + with: + weight: ${{ inputs.runner-weight }} + labels: ${{ inputs.runner-labels }} + gh-app-client-id: ${{ secrets.GH_APP_CLIENT_ID }} + gh-app-private-key: ${{ secrets.GH_APP_PRIVATE_KEY }} implement: - needs: [gate, pick-runner] + needs: [gate] if: needs.gate.outputs.proceed == 'true' - runs-on: ${{ fromJson(needs.pick-runner.outputs.runner) }} + runs-on: ${{ fromJson(needs.gate.outputs.runner) }} timeout-minutes: ${{ inputs.timeout-minutes }} permissions: contents: write # to push the branch diff --git a/.github/workflows/issue-triage.yml b/.github/workflows/issue-triage.yml index 26c0aa8..a23cd4f 100644 --- a/.github/workflows/issue-triage.yml +++ b/.github/workflows/issue-triage.yml @@ -58,14 +58,17 @@ concurrency: cancel-in-progress: true jobs: - # Gate BEFORE picking a runner. The picker is itself a hosted job, so it must - # never run for an event that will not proceed. + # The picker runs INSIDE this job, after the last step that can still say no. + # It is itself a hosted job, so it must never run for an event that will not + # proceed, and folding it in here means a proceeding run costs one hosted + # start instead of two. gate: runs-on: ubuntu-latest - timeout-minutes: 5 + timeout-minutes: 10 # the picker calls the org API and may retry outputs: proceed: ${{ steps.bots.outputs.proceed }} mode: ${{ steps.bots.outputs.mode }} + runner: ${{ steps.pick.outputs.runner }} steps: # agent:no-touch is evaluated inside this action, first and # unconditionally. Nothing below may run ahead of it. The gate reads the @@ -120,19 +123,21 @@ jobs: echo "proceed=$proceed" >> "$GITHUB_OUTPUT" echo "mode=${MODE:-triage}" >> "$GITHUB_OUTPUT" - pick-runner: - needs: [gate] - if: needs.gate.outputs.proceed == 'true' - uses: dodi-smart/.github/.github/workflows/pick-runner.yml@v1 - with: - weight: ${{ inputs.runner-weight }} - labels: ${{ inputs.runner-labels }} - secrets: inherit + # Only when the run above is still proceeding: a no-op dispatch or a + # kill-switched issue must never pay for a hosted picker. + - id: pick + if: steps.bots.outputs.proceed == 'true' + uses: dodi-smart/.github/actions/pick-runner@v1 + with: + weight: ${{ inputs.runner-weight }} + labels: ${{ inputs.runner-labels }} + gh-app-client-id: ${{ secrets.GH_APP_CLIENT_ID }} + gh-app-private-key: ${{ secrets.GH_APP_PRIVATE_KEY }} triage: - needs: [gate, pick-runner] + needs: [gate] if: needs.gate.outputs.proceed == 'true' - runs-on: ${{ fromJson(needs.pick-runner.outputs.runner) }} + runs-on: ${{ fromJson(needs.gate.outputs.runner) }} timeout-minutes: 20 permissions: contents: read diff --git a/.github/workflows/pr-review.yml b/.github/workflows/pr-review.yml index 0beb356..cec4fdf 100644 --- a/.github/workflows/pr-review.yml +++ b/.github/workflows/pr-review.yml @@ -52,9 +52,13 @@ concurrency: cancel-in-progress: true jobs: + # The picker runs INSIDE this job, after the last step that can still say + # no. It is itself a hosted job, so it must never run for an event that will + # not proceed, and folding it in here means a proceeding run costs one + # hosted start instead of two. gate: runs-on: ubuntu-latest - timeout-minutes: 8 + timeout-minutes: 10 # the picker calls the org API and may retry permissions: contents: read checks: read @@ -65,6 +69,7 @@ jobs: level: ${{ steps.level.outputs.level }} reason: ${{ steps.level.outputs.reason }} ci: ${{ steps.ci.outputs.summary }} + runner: ${{ steps.pick.outputs.runner }} steps: # Payload-only rules first, before the checkout. agent:no-touch, drafts # and bot PRs are decided without fetching anything. Bot dependency PRs @@ -174,19 +179,23 @@ jobs: echo "summary=$out" >> "$GITHUB_OUTPUT" echo "collected $n failing check(s)" - pick-runner: - needs: [gate] - if: needs.gate.outputs.proceed == 'true' - uses: dodi-smart/.github/.github/workflows/pick-runner.yml@v1 - with: - weight: ${{ inputs.runner-weight }} - labels: ${{ inputs.runner-labels }} - secrets: inherit + # Only when the run above is still proceeding: a draft, a bot PR or a + # docs-only diff must never pay for a hosted picker. `steps.level`, not + # `steps.gate`, is the last step that can still say no -- it narrows the + # gate's own verdict further (no changed files, docs only). + - id: pick + if: steps.level.outputs.proceed == 'true' + uses: dodi-smart/.github/actions/pick-runner@v1 + with: + weight: ${{ inputs.runner-weight }} + labels: ${{ inputs.runner-labels }} + gh-app-client-id: ${{ secrets.GH_APP_CLIENT_ID }} + gh-app-private-key: ${{ secrets.GH_APP_PRIVATE_KEY }} review: - needs: [gate, pick-runner] + needs: [gate] if: needs.gate.outputs.proceed == 'true' - runs-on: ${{ fromJson(needs.pick-runner.outputs.runner) }} + runs-on: ${{ fromJson(needs.gate.outputs.runner) }} timeout-minutes: 25 permissions: contents: read diff --git a/.github/workflows/self-test.yml b/.github/workflows/self-test.yml index bc701cd..0c14e69 100644 --- a/.github/workflows/self-test.yml +++ b/.github/workflows/self-test.yml @@ -201,6 +201,68 @@ jobs: print('pr-checks.yml has one picker job (%s) and an always-on pr-checks summary job' % pickers[0]) " + # Each agent workflow used to run the picker as its own hosted job, needed + # by the agent job. That billed two hosted starts before a proceeding run + # ever reached the agent. Now the picker is a STEP inside `gate`, after + # `agent-gate`, guarded on the gate's own `proceed`. Assert the new shape + # rather than trust it stays this way: a `pick-runner` job creeping back + # in "just for this one workflow" is exactly how it drifted before. + - name: Agent workflows pick their runner inside the gate job + run: | + set -euo pipefail + python3 -c " + import sys, yaml + + agent_workflows = [ + ('issue-triage.yml', 'triage'), + ('issue-implement.yml','implement'), + ('claude-assist.yml', 'assist'), + ('pr-review.yml', 'review'), + ('deps-verify.yml', 'build'), + ] + + bad = [] + for f, agent_job in agent_workflows: + path = '.github/workflows/' + f + jobs = yaml.safe_load(open(path))['jobs'] + + if 'pick-runner' in jobs: + bad.append(f + ': still has a standalone pick-runner job') + continue + + gate = jobs.get('gate') + if gate is None: + bad.append(f + ': has no gate job') + continue + + steps = gate.get('steps') or [] + pick = [s for s in steps if 'pick-runner@' in str(s.get('uses', ''))] + if len(pick) != 1: + bad.append(f + ': gate job must have exactly one pick-runner step, found ' + str(len(pick))) + continue + + cond = str(pick[0].get('if', '')) + if 'proceed' not in cond: + bad.append(f + ': the pick step is not guarded on a proceed output (if: %r)' % cond) + + if 'runner' not in (gate.get('outputs') or {}): + bad.append(f + ': gate job has no runner output for the agent job to read') + + job = jobs.get(agent_job) + if job is None: + bad.append(f + ': has no ' + agent_job + ' job') + continue + if 'pick-runner' in (job.get('needs') or []): + bad.append(f + ': ' + agent_job + ' job still needs pick-runner') + if 'needs.gate.outputs.runner' not in str(job.get('runs-on', '')): + bad.append(f + ': ' + agent_job + ' job does not read runs-on from needs.gate.outputs.runner') + + if bad: + sys.exit('::error::' + '; '.join(bad) + '. Folding the picker into the gate job is what makes a ' + 'proceeding agent run cost one hosted start instead of two.') + print('every agent workflow picks its runner inside the gate job, guarded on proceed') + " + # pick-runner.yml is a thin wrapper around actions/pick-runner now. Every # existing caller, internal or external, references these names, so the # wrapper losing one is a breaking change hiding behind a passing lint. diff --git a/AGENTS.md b/AGENTS.md index 31bd312..7d10d79 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -35,7 +35,7 @@ why, not history. | File | Rule | Why | |---|---|---| | `actions/agent-gate/` | `agent:no-touch` is evaluated FIRST, with no exemption | Position matters as much as existence. A check after an early return stops covering that path. `test.sh` asserts it across every workflow shape. Do not weaken those cases. | -| all agent workflows | Deterministic gates run before any agent step, and the runner picker carries the same `if:` as the job it feeds | An agent explaining a compile error is waste, and the picker is itself a hosted job. Neither may run for an event that will not proceed. | +| all agent workflows | Deterministic gates run before any agent step. The runner picker runs INSIDE the gate job, as a step after `actions/agent-gate`, conditioned on the gate's own `proceed` | An agent explaining a compile error is waste, and the picker is itself a hosted job. Running it as a step of the gate job, gated on `proceed`, means it can never run for an event that will not proceed, and a run that does proceed pays for one hosted start instead of two. It still never precedes the kill switch: `agent-gate` runs first in the same job, unconditionally. | | `.github/workflows/pr-review.yml` | No `synchronize` trigger | Reviewing every push is what gets a review bot muted, and a muted bot reviews nothing. | | `.github/workflows/deps-verify.yml` | Verification never merges, approves, or changes mergeability | Evidence is only useful if it is allowed to be wrong. Merging on a clean verdict forces conservative tuning, which produces noise, which gets the report ignored. | | `.github/workflows/issue-implement.yml` | A plan is required, `Agent mode` gates who may ask, and the PR is always a draft | The check is a field comparison in a gate job, never a question put to the agent, and it has no override. An agent asked whether a plan is adequate will sometimes accept a two-line issue body. | diff --git a/README.md b/README.md index df97e57..4b18350 100644 --- a/README.md +++ b/README.md @@ -271,10 +271,10 @@ including when it refuses. If it is still there, the work is genuinely running. ``` `agent:implement` does nothing unless the issue is planned. That is a field -comparison in a gate job, run before a runner is picked, with no override. An -agent asked to judge whether a plan is good enough will sometimes accept a -two-line issue body, and the cost is twenty minutes of confident work on the -wrong thing. +comparison in the gate job, which also picks the runner once that comparison +passes, with no override. An agent asked to judge whether a plan is good +enough will sometimes accept a two-line issue body, and the cost is twenty +minutes of confident work on the wrong thing. ## Stacks @@ -430,8 +430,9 @@ jobs: `pick-runner.yml` takes a semantic `weight` and resolves it. It is a thin wrapper around `actions/pick-runner`, the composite action that does the actual selecting; call the action directly from inside a job that is already hosted -(as `pr-checks.yml`'s `pick` job does, twice) rather than paying for a second -hosted job just to reuse the workflow. +(as `pr-checks.yml`'s `pick` job does, twice, and as every agent workflow's +`gate` job does, once, after `agent-gate` decides the run should proceed) +rather than paying for a second hosted job just to reuse the workflow. | `weight` | Selector | Use | |---|---|---|