pipelines

pr-review.yml

# yaml-language-server: $schema=../steps.schema.json
#
# Deep review of one author's open PRs. A planner picks the review dimensions,
# one reviewer per dimension runs concurrently, an obligations pass checks the
# changed lines against their real definitions, a falsifier challenges every
# finding, a gatekeeper decides what blocks, a synthesizer writes the review,
# and it posts as a comment on the PR.
#
#   steps validate pr-review.yml --var repo=owner/name --var author=login --var slack_user=UXXXX
#   steps run      pr-review.yml --job review  (same vars)
#   steps web      pr-review.yml               (same vars; every new push re-reviews)
#
# Or put the three in a vars file and pass `--vars-file`. Vars are hashed into
# state.db — config, not secrets — so a token never goes in one.
#
# Needs on the host: docker, `claude` on PATH (a CLI agent is always a host
# subprocess), SLACK_BOT_TOKEN, and GH_TOKEN with repo scope
# (`export GH_TOKEN=$(gh auth token)` in .envrc). No `gh` anywhere: GitHub is
# a JSON API, so the resource is expression-backed like Linear and Slack are.
# On macOS also `export TMPDIR="$HOME/.steps-tmp"`: the daemon's VM does not
# share /var/folders, and docker mounts an unshared path as an EMPTY directory.
#
# Containers are the reproducibility and the fence. Every shell command —
# the checkout, the anatomy grep, an agent's run_shell, the final gate — runs
# in a pinned alpine; the model's own conversation never enters one. An
# agent's read_file/list_dir/search_files are host-side and workspace-confined
# already, so an agent with no shell tool names no image: nothing to contain.
# The one thing not containerized is the expression resource, which runs no
# command at all (image: on it is a load error).
#
# Model split follows recoverability, not cost. Sonnet where a stronger model
# re-derives the work afterwards (reviewer, obligations, synthesizer); opus
# where nothing downstream can recover a mistake (planner, falsifier,
# gatekeeper). No `budget:` on any agent: the CLI authenticates from a
# subscription, so the dollar figure is a price nobody is billed — `timeout:`
# is the ceiling that means something.
#
# No agent declares `settings:` — no configuration scope loads, and deliberately
# not the reviewed branch's own `.claude/`. A PR that installs hooks into its
# own review reviews itself.
#
# From Agent-Field/pr-af, adopted: deterministic anatomy before planning, the
# obligations (literal-correctness) pass, AI-written-PR skepticism, "drop what
# the PR did not introduce", directive-first comments. Rejected: the coverage
# loop (a second planning round on one PR doubles the bill for a gap the
# falsifier already reads across), scoring formulas (severity is a vocabulary
# here, not a number), and review events (the author cannot submit a review
# with a verdict on their own PR).

workspace:
  strategy: copy    # the default; a collecting matrix requires it

# No defaults.model: a CLI spelling there also lands on the @builtin/* agent
# profiles, one of which grants a sub-agent tool a CLI source cannot have.

resource_types:
# check + in for the get, out for the put; one type, two resources below.
# A status is data to http(), so every stage guards on it: a 401 would
# otherwise read as "no open PRs" and quietly stop the pipeline.
- name: github-pr
  env: [GH_TOKEN]                # the only host var the expression may read
  config:
    expr:
      # number@headSHA so a push is a new build, not a cache hit on the number.
      # sortBy because the version contract is oldest-first and GitHub answers newest-first.
      check: |
        let auth = {Authorization: "Bearer " + env("GH_TOKEN"), Accept: "application/vnd.github+json"};
        let r = http({url: "https://api.github.com/repos/" + source.repo + "/pulls",
                      query: {state: "open", per_page: "100"}},
                     {headers: auth, retry: {on: [429, 503], max: 3}});
        r.status != 200
          ? fail("github pulls: " + string(r.status) + " " + (r.json.message ?? ""))
          : r.json
            | filter(#.user.login == source.author)
            | sortBy(#.number)
            | map(({ref: string(#.number) + "@" + #.head.sha}))

      # Three requests, one batched call. The diff is the same URL with a
      # different Accept; a request's own header wins over the shared one.
      in: |
        let auth = {Authorization: "Bearer " + env("GH_TOKEN"), Accept: "application/vnd.github+json"};
        let parts = split(version.ref, "@");
        let base = "https://api.github.com/repos/" + source.repo + "/pulls/" + parts[0];
        let r = http([{url: base},
                      {url: base, headers: {Accept: "application/vnd.github.diff"}},
                      {url: base + "/files", query: {per_page: "100"}}],
                     {headers: auth, retry: {on: [429, 503], max: 3}});
        !all(r, #.status == 200)
          ? fail("github pr " + parts[0] + ": " + string(map(r, #.status)))
          : {
              "pr.json": toJSON({
                number: r[0].json.number, title: r[0].json.title, body: r[0].json.body ?? "",
                baseRefName: r[0].json.base.ref, headSha: r[0].json.head.sha,
                files: r[2].json | map(({path: #.filename, status: #.status,
                                         additions: #.additions, deletions: #.deletions})),
              }),
              "pr.diff": r[1].body,
              "pr.number": parts[0],
              "pr.sha": parts[1],
            }

      # An issue comment, not a review: GitHub refuses a review with a verdict
      # from the PR's own author, and a comment is all that is left.
      out: |
        let auth = {Authorization: "Bearer " + env("GH_TOKEN"), Accept: "application/vnd.github+json"};
        let number = trim(file("pr/pr.number"));
        file("review/findings.json") == "" ? fail("no findings file") : (
          let posted = http({url: "https://api.github.com/repos/" + source.repo + "/issues/" + number + "/comments",
                             json: {body: file("review/summary.md")}},
                            {headers: auth, retry: {on: [429, 503], max: 3}});
          posted.status == 201
            ? {ref: string(posted.json.id)}
            : fail("github comment: " + string(posted.status) + " " + (posted.json.message ?? ""))
        )

# Failure DM. Not the built-in slack-reply: that reads channel and text from a
# put's inputs, and a job-level hook runs in an empty workspace where inputs:
# is a load error.
- name: slack-dm
  env: [SLACK_BOT_TOKEN]
  config:
    expr:
      # A user id as the channel is a DM from the bot; needs im:write.
      out: |
        let posted = http({
          url: (source.base_url ?? "https://slack.com") + "/api/chat.postMessage",
          headers: {Authorization: "Bearer " + env("SLACK_BOT_TOKEN")},
          json: {channel: source.user, text: params.text, unfurl_links: false},
        }, {retry: {on: [429, 503], max: 3}}).json;

        // Slack refuses with 200 + ok:false; without this a rejected DM reads as sent.
        posted.ok ? {channel: posted.channel, ts: posted.ts}
                  : fail("slack chat.postMessage: " + (posted.error ?? "failed"))

resources:
- name: pr
  type: github-pr
  source:
    repo: ((repo))
    author: ((author))
- name: pr-review
  type: github-pr
  source:
    repo: ((repo))
    author: ((author))
- name: dm
  type: slack-dm
  source:
    user: ((slack_user))     # Slack member id; the DM goes to them, from the bot

# All read-only: withholding the write tools is what makes "this agent reviews,
# it does not edit" a guarantee. Where an agent gets run_shell, it runs in a
# pinned alpine with no network — it can read the workspace and reach nothing.
agents:
- name: planner
  source:
    model: "@claude/opus"      # nothing downstream recovers a missed dimension
  system: |
    You decide what kinds of review a change needs — you do not review it.

    A dimension is a SPECIFIC INVESTIGATION QUESTION, not a category.
      good: "Does the migration from sync to async preserve error propagation
             to callers?"
      bad:  "Check for concurrency issues"

    The diff and an anatomy file are in your context. Plan from them; the
    reviewers are the ones that read the codebase, each opening with the brief
    you write it. anatomy/blast.txt lists, per changed file, the files that
    mention it — a grep, not an analysis — so a brief can point a reviewer at
    the callers the diff does not show.

    Ignore documentation, lockfiles, vendored and generated code unless the
    change is ABOUT them. If the change carries no risk on some axis, propose
    nothing for it. Do not pad.

    If the change reads as model-written — uniform structure, comments on the
    obvious, tests mirroring the implementation — add one dimension that
    verifies every referenced API, module and method actually exists with the
    signature used, and that the tests assert behaviour rather than restate
    the code. Model-written code fails in those two ways more than any other.
  max_turns: 8
  timeout: 15m           # reads a diff, writes briefs; longer than this is stuck
  max_context_bytes: 400000     # a large PR is never truncated on the step whose job is to see it whole
  tools: [read_file, write_file]

- name: reviewer
  source:
    model: "@claude/sonnet"    # the volume step; the falsifier checks its work
  image: alpine:3.20
  network: none
  system: |
    You are a senior engineer reviewing one change through ONE assigned
    dimension. If the change carries no risk on it, write an empty findings
    array — do not pad.

    Read the target files properly: control flow, data flow, error paths, and
    what happens at boundaries — entry and exit, early returns, exception
    handlers. Then trace implications. If a signature changed, who calls it?
    If a default changed, where is it consumed?

    Think about what is NOT in the diff. The most dangerous bugs live in code
    that was not changed but should have been: a signature that moved and left
    its callers behind, an enum variant with no matching case.

    ## Before you report anything, pass three gates

    1. REACHABILITY. Trace the exact path from a real entry point to the code.
       If you cannot construct a concrete scenario where this triggers, it is
       speculation, not a finding.

    2. EVIDENCE CHAIN. Write the steps out:
         Step 1: <entry point> calls <function> with <args>
         Step 2: <function> passes <value> to <downstream>
         Step 3: <downstream> expects <x> but receives <y>
         Step 4: this fails as <specific failure>
       If you cannot write that chain, you cannot report it.

    3. CONFIDENCE. Rate yourself honestly and report nothing below 0.6.
       0.9+ means you traced the path and verified the failure.

    Three well-proven findings are worth more than ten speculative ones. When
    in doubt, drop it.

    ## Severity is a fixed vocabulary

    Exactly one of: critical, important, suggestion, nitpick. Never "high",
    "medium", "low", or "warning".

      critical    it WILL fail in production — crash, corruption, security
                  hole, or silently wrong results, and you can state the exact
                  failure scenario
      important   it CAN fail under known conditions — missing error handling,
                  contract violation, a race under realistic load
      suggestion  it works, but could be more robust
      nitpick     cosmetic

    Write your findings as a JSON array of flat objects with severity, file,
    line, claim, and the evidence chain you actually traced, to the path your
    prompt names.
  max_turns: 50
  timeout: 20m
  tools: [read_file, list_dir, search_files, run_shell, write_file]

# pr-af's "deepen" pass. The dimensions above ask WHERE the risk is; this one
# asks, line by line, whether the changed code is literally correct against
# the real definitions it depends on. Different question, different misses.
- name: obligations
  source:
    model: "@claude/sonnet"    # exhaustive, not judicious; the falsifier judges
  image: alpine:3.20
  network: none
  system: |
    You are the literal-correctness verifier. Other reviewers have covered
    design, architecture and systemic risk; you go line by line through the
    CHANGED code and check it against ground truth — the real definitions of
    every symbol it touches. Open them; do not reason from memory.

    For each changed call, argument, assignment, condition and type assumption,
    ask: what must be true ELSEWHERE for this line to be correct? Then go read
    that place — the definition it calls, where the value it passes is
    produced, the sibling branch, the code that consumes what it produces —
    and compare the two ends. When they disagree, that is a finding; quote the
    code at BOTH ends. Two sibling bugs on adjacent lines are two findings.

    Be exhaustive over the diff, not selective. Do not repeat a finding another
    reviewer already made — their reports are in findings/. Report nothing
    below 0.6 confidence, and if the changed code is literally correct, an
    empty array is the right answer. Severity vocabulary: critical, important,
    suggestion, nitpick.
  max_turns: 50
  timeout: 20m
  tools: [read_file, list_dir, search_files, run_shell, write_file]

- name: falsifier
  source:
    model: "@claude/opus"      # the gate — a cheap skeptic rubber-stamps
  image: alpine:3.20
  network: none
  system: |
    You are the adversarial reviewer. You CHALLENGE every finding handed to
    you to decide whether it is real or a false positive. You are skeptical
    by default.

    A finding is a CLAIM ABOUT CODE, and the code is available to you. The
    protocol is a comparison, not an opinion:

      1. read what the reviewer claims the code does
      2. open the file and read what it ACTUALLY does
      3. claim contradicts the code       -> false positive, drop it
      4. claim matches the code           -> then check the callers, and
                                             confirm the failure is reachable

    A reviewer saying "X uses string comparison" when the code plainly calls
    errors.Is() is the case this step exists to catch.

    Also drop a finding that: describes a problem the PR did not introduce
    (pre-existing code the diff leaves no more reachable or frequent than
    before — a diff that makes an old failure path fire more often owns it);
    flags the repository's own established convention as a defect; or is
    already mitigated somewhere the reviewer did not look. Ask of each
    survivor whether this is the intended behaviour. A finding you cannot
    knock down survives — say what made it survive.

    Write the survivors to confirmed.json, as a JSON array of flat objects
    with id, severity, file, line, claim. Write what you dropped to
    dismissed.json — id, file, line, claim, and one sentence on why — so
    the human reading the review can catch a dismissal you got wrong.
  max_turns: 50
  timeout: 30m
  tools: [read_file, list_dir, search_files, run_shell, write_file]

- name: gatekeeper
  source:
    model: "@claude/opus"      # small input, irreversible call
  system: |
    You are the release manager. For each finding you answer ONE question,
    and it is not the question severity answers.

      severity   how bad is this?
      you        must it be fixed BEFORE this ships?

    Those come apart in both directions. A pedantic `critical` in unreachable
    code can ship. A subtle `important` on a hot path cannot.

    Apply a TIGHT bar. Blocking only if it breaks the build or tests, is a
    security hole reachable from a real user-facing path, causes data loss or
    corruption, breaks a public contract real callers depend on with no
    migration, or regresses behaviour that worked before this change.

    NOT blocking: style, naming, refactors, missing tests, defensive
    programming with no demonstrated reachable exploit, performance that does
    not change correctness, documentation, architectural critiques with no
    concrete production impact, or "should also handle X" when X is not
    currently reachable. If the evidence does not concretely demonstrate one
    of the blocking criteria, it is not blocking — whatever the label says.

    When you cannot decide, answer NOT blocking. That asymmetry is deliberate:
    a real blocker that slips through is caught by the human reading the
    review, while an advisory nit that blocks a merge teaches the team to
    ignore the whole tool.

    Write blocking.json: a JSON array of the finding ids that block — and
    only those. An empty array is a legitimate answer and the common one.
  max_turns: 50
  timeout: 10m
  tools: [read_file, list_dir, search_files, write_file]

- name: synthesizer
  source:
    model: "@claude/sonnet"    # writes prose from an already-judged list
  system: |
    You write the review a human will read. Cluster related findings into
    compound risks where two of them are worse together than apart. A
    finding whose id is not in blocking.json is advisory, not a blocker —
    say so, do not drop it.

    Each finding opens with a one-sentence directive: what to change, where.
    Then one short paragraph on the concrete failure — why it is a problem,
    quoting the exact code or call path. Then the fix and its tradeoff if
    there is more than one way. The author should be able to act on it in
    thirty seconds.

    You are writing on the author's own pull request, so skip the ceremony:
    no greeting, no summary of the PR back to them, no sign-off. Lead with
    one line: how many findings block, how many are advisory. End with a
    collapsed <details> section, "Considered and dismissed", listing each
    dismissed claim as one line with file:line and the reason — that is
    how the author sees what was checked, and catches a wrong dismissal.
  max_turns: 50
  timeout: 15m
  tools: [read_file, list_dir, write_file]

jobs:
- name: review
  # Covers the worst sequential path through the plan, not the typical one:
  # 15m + 2 waves x 20m + 20m + 30m + 10m + 15m = 2h10m.
  timeout: 3h
  plan:
  - get: pr
    trigger: true
    version: every     # one build per PR and per push; a bad answer on one cannot contaminate another

  # The tree at the reviewed commit, so reviewers read source rather than trust
  # the patch. `-c http.extraheader` lends git the token for one command; a
  # token in the remote URL would sit in .git/config where every reviewer can
  # read it. apk per build is ~15MB pinned by the 3.20 branch; the alternative
  # was a 400MB floating-tag CI image, and every image that ships git alone
  # has an ENTRYPOINT that swallows the container's keepalive.
  - task: checkout
    image: alpine:3.20
    env: [GH_TOKEN]
    inputs: [pr]
    outputs: [repo]
    run: |
      set -e
      apk add --no-cache git >/dev/null
      sha=$(cat pr/pr.sha)
      cd repo
      git init -q .
      git -c http.extraheader="Authorization: Basic $(printf 'x-access-token:%s' "$GH_TOKEN" | base64 | tr -d '\n')" \
        fetch -q --no-tags --depth 1 "https://github.com/((repo)).git" "$sha"
      git checkout -q FETCH_HEAD
      echo "checked out $(git rev-parse --short HEAD)"
    assert:
      stdout: checked out

  # Computation, not reasoning: the changed paths and a grep for what mentions
  # each one. Deterministic, free, and the planner's only view past the diff.
  - task: anatomy
    image: alpine:3.20
    inputs: [pr, repo]
    outputs: [anatomy]
    run: |
      set -e
      grep '^+++ b/' pr/pr.diff | sed 's|^+++ b/||' | sort -u > anatomy/changed.txt
      : > anatomy/blast.txt
      head -40 anatomy/changed.txt | while read -r f; do
        # Ruby autoloads by constant, so grep the constant (app/integrations/facebook_ads/client.rb -> FacebookAds::Client); a bare "client" matched twenty unrelated files.
        case "$f" in
          *.rb) key=$(echo "${f%.rb}" | awk -F/ '{s=($1=="app")?3:(NF>1?2:1); o=""; for(i=s;i<=NF;i++){n=split($i,w,"_"); c=""; for(j=1;j<=n;j++) c=c toupper(substr(w[j],1,1)) substr(w[j],2); o=o (i>s?"::":"") c}; print o}') ;;
          *) key=$(basename "$f"); key="${key%%.*}" ;;
        esac
        echo "## $f ($key)" >> anatomy/blast.txt
        find repo -path repo/.git -prune -o -type f -print0 \
          | xargs -0 grep -lF -- "$key" 2>/dev/null \
          | grep -v -F -- "repo/$f" | sed 's|^repo/||' | head -20 >> anatomy/blast.txt
      done
      echo "changed $(wc -l < anatomy/changed.txt) files"
    assert:
      files: [anatomy/blast.txt]

  # Writes the work list (a JSON array of dimension ids) and one brief each.
  - agent: planner
    inputs: [pr, anatomy]
    outputs: [dims]
    context_paths: [pr/pr.diff, pr/pr.json, anatomy/blast.txt]
    messages:
    - |
      Decide the review dimensions for this change.

      Write dims/index.json: a JSON array of dimension ids. Each id must be
      short kebab-case (letters, digits, - _ .), unique, and there must be at
      most SIX — fewer is better than overlapping. A trivial change justifies
      one or two.

      For each id, also write dims/<id>.md: what a reviewer taking this
      dimension looks for (one paragraph), and the repo-relative paths under
      repo/ to start from.
    assert:
      files: [dims/index.json]     # no work list, no matrix width

  # One reviewer per dimension. `from_file:` sets the width from what the
  # planner wrote; `outputs:` on the block collects every cell into
  # findings/<dim>/report.json; try: means one failed cell contributes no
  # directory instead of costing the review.
  - across:
    - var: dim
      from_file: dims/index.json
    # Throttle, not budget: three concurrent CLIs is what one subscription
    # takes without rate-limit retries eating the timeout.
    max_in_flight: 3
    try:
      agent: reviewer
      inputs: [pr, repo, dims]
      outputs: [findings]
      context_paths: ["dims/{{ .vars.dim }}.md"]
      messages:
      - |
        Review this change through one dimension only. Your brief is already
        in your context — it says what to look for and where to start under
        repo/. The diff is pr/pr.diff. Read the code before you claim
        anything about it.

        Write your findings to findings/report.json.

  # Its report lands beside the matrix cells' so the falsifier reads one layout.
  - agent: obligations
    inputs: [pr, repo, findings]
    outputs: [findings]
    context_paths: [pr/pr.diff]
    messages:
    - |
      Verify the changed code in pr/pr.diff line by line against the real
      definitions under repo/. findings/*/report.json are what other reviewers
      already found; do not repeat them.

      Write findings/obligations/report.json.
    assert:
      files: [findings/obligations/report.json]

  # A separate step: an agent grading its own work is not a gate. This is
  # also what pays for the cheaper reviewers above.
  - agent: falsifier
    inputs: [pr, repo, findings]
    outputs: [confirmed]
    messages:
    - |
      List findings/ — each <dimension>/report.json in it is a set of CLAIMS,
      not facts. Try to invalidate each one against the code under repo/.

      Write the survivors to confirmed/confirmed.json and the rest, with
      why, to confirmed/dismissed.json. Both files always, even if empty.
    assert:
      files: [confirmed/confirmed.json, confirmed/dismissed.json]

  - agent: gatekeeper
    inputs: [repo, confirmed]
    outputs: [blocking]
    context_paths: [confirmed/confirmed.json]
    messages:
    - |
      For each confirmed finding above, decide whether it must be fixed
      before this PR merges. The code is under repo/.

      Write blocking/blocking.json.
    assert:
      files: [blocking/blocking.json]

  - agent: synthesizer
    inputs: [repo, confirmed, blocking]
    outputs: [review]
    context_paths: [confirmed/confirmed.json, confirmed/dismissed.json, blocking/blocking.json]
    messages:
    - |
      Write the review from the confirmed findings above. Cluster any that
      are worse together than apart into a single compound risk. The code is
      under repo/ if you need to quote it. The dismissed list goes in the
      collapsed section at the end, one line each.

      Write review/summary.md (what a human reads) and review/findings.json
      (the structured list). Nothing else — the PR number is the pipeline's
      to know, not yours.
    assert:
      files: [review/summary.md, review/findings.json]   # success without output is otherwise indistinguishable

  # The deterministic gate before anything is posted.
  - task: check-draft
    image: alpine:3.20
    inputs: [pr, review]
    run: |
      set -e
      test -s review/summary.md    || { echo "no review summary was written" >&2; exit 1; }
      test -s review/findings.json || { echo "no findings file was written" >&2; exit 1; }
      test -s pr/pr.number         || { echo "the PR number was never fetched" >&2; exit 1; }
      echo "draft ready for review"
    assert:
      stdout: draft ready for review

  # No approval: a comment on the author's own PR. check-draft is the gate.
  - put: pr-review
    inputs: [pr, review]

  # on_failure: a step said no (assert, nonzero task, step timeout).
  # on_error: the infrastructure said no (model, GitHub, docker, workspace).
  # One fires, never both. A job hook runs in an empty workspace with no
  # ((var)) substitution, so the text names the command that knows which build.
  on_failure:
    put: dm
    params:
      text: "*steps: pr-review failed* — a review of one of your open PRs stopped on a verdict. `steps runs pr-review.yml` for which build, then `steps run pr-review.yml --resume <id>` to pick it back up."
  on_error:
    put: dm
    params:
      text: "*steps: pr-review errored* — not a verdict: the model, GitHub, docker, or the workspace fell over. `steps runs pr-review.yml` for the build, `docker info` and `gh auth status` for the rest."

  # No assert.execution: the matrix width is decided at run time.
  assert:
    outcome: succeeded