跳到正文
DEV Community· Emery Lin·· 3 小時前AI 評分57

已取消的審查執行不代表通過審查

A Cancelled Review Run Is Not a Passing Review

AI 導讀

CI 流程應把已取消的審查執行視為缺少證據,而非通過;文章提供以判定產物和獨立 gate 落實這項規則的實作方案。判定產物綁定 head_sha、input_digest 和 run_attempt,工作取消或被終止時不會寫入,gate 會拒絕缺失、過期或輸入不匹配的判定。

正文

The rule, in one line. A cancelled run is missing evidence, and a missing verdict must never be read as a pass.

That sentence is the article. The rest is the machinery that enforces it in a pipeline where one non-deterministic lane — an AI review step, a fuzz run, an end-to-end suite — sits next to the checks that actually block a merge.

Why the cancelled lane is the expensive one

Most teams bolt on an AI review step because it is cheap and advisory. Then they do two reasonable things that combine badly.

  1. They set concurrency: cancel-in-progress: true, so a fresh push cancels the review of the previous commit. That is the correct cost decision.
  2. They either require the review check in branch protection, or they treat the bot's comment as the verdict. Both make a cancelled run load-bearing.

Put those together and you get the worst kind of bug: a lane that has never once emitted "no evidence", merged into a system where nobody has tested what "no evidence" means. A check whose failure path has never run is not a check. It is a habit.

The same shape appears in browser suites cancelled on the third retry, scanners that time out, and any job with a 10-minute timeout sitting next to a 6-minute model call.

Bind the verdict to fields, not to a comment

A comment is not evidence. Evidence is a small artifact with fields the gate can verify without trusting the producer. Four fields carry the weight:

  • head_sha — the commit the verdict is about. Without it, a review of commit A authorizes a merge of commit B.
  • input_digest — a hash of exactly what the lane read: tracked file modes and contents plus lockfiles. Without it, a review of last week's lockfile authorizes today's dependency bump.
  • run_attempt — which attempt produced this. A verdict from attempt 1 must not authorize attempt 3.
  • status: complete — set only on normal completion of the lane. A cancelled or killed job writes nothing.

That last point is the design: a cancelled lane produces no artifact. Absence is the signal. You never have to teach the gate to interpret cancelled, because there is nothing to interpret.

Step 1 — the advisory workflow

Reference implementation, not something I have run against your repository. Treat it as a starting point, and verify the cancellation behaviour on your own runner first, because step-level conditions during cancellation have changed between runner versions.

name: advisory-review
on:
  pull_request:
    types: [opened, synchronize, reopened]

concurrency:
  group: advisory-review-${{ github.event.pull_request.number }}
  cancel-in-progress: true

permissions:
  contents: read
  pull-requests: write

jobs:
  review:
    if: github.event.pull_request.draft == false
    runs-on: ubuntu-latest
    timeout-minutes: 10
    steps:
      - uses: actions/checkout@v4
        with:
          ref: ${{ github.event.pull_request.head.sha }}

      - name: Compute input digest
        id: digest
        run: |
          DIGEST="$( { git ls-files -s | sort; sha256sum package-lock.json 2>/dev/null || true; } | sha256sum | cut -d' ' -f1 )"
          echo "digest=$DIGEST" >> "$GITHUB_OUTPUT"

      - name: Run review lane
        id: lane
        env:
          MONKEYCODE_API_KEY: ${{ secrets.MONKEYCODE_API_KEY }}
        run: ./ci/run-review.sh > review.md

      - name: Write verdict
        if: success() || failure()
        run: |
          STATUS="fail"
          [ "${{ steps.lane.outcome }}" = "success" ] && STATUS="pass"
          jq -n \
            --arg sha "${{ github.event.pull_request.head.sha }}" \
            --arg digest "${{ steps.digest.outputs.digest }}" \
            --arg attempt "${{ github.run_attempt }}" \
            --arg status "$STATUS" \
            '{status: "complete", conclusion: $status, head_sha: $sha,
              input_digest: $digest, run_attempt: $attempt}' > verdict.json

      - name: Upload verdict
        if: success() || failure()
        uses: actions/upload-artifact@v4
        with:
          name: verdict-${{ github.event.pull_request.number }}-${{ github.run_attempt }}
          retention-days: 7
          path: verdict.json

Note the asymmetry: the artifact is uploaded on both success and failure of the lane, but not on cancellation. A red verdict is a verdict; a missing verdict is not green.

Step 2 — the required gate

Keep the gate in a separate workflow triggered by workflow_run, or as a job that downloads the artifact. It reads the repository and nothing else. No secrets, no tokens, no write access.

#!/usr/bin/env bash
# ci/gate.sh — required check. Fails closed on any missing or stale evidence.
set -euo pipefail

VERDICT="${1:-verdict.json}"
[[ -f "$VERDICT" ]] || { echo "No verdict artifact: fail closed." >&2; exit 1; }

HEAD_SHA="$(git rev-parse HEAD)"
DIGEST="$( { git ls-files -s | sort; sha256sum package-lock.json 2>/dev/null || true; } | sha256sum | cut -d' ' -f1 )"

jq -e --arg sha "$HEAD_SHA" --arg digest "$DIGEST" '
  .status            == "complete"
  and .conclusion    == "pass"
  and .head_sha      == $sha
  and .input_digest  == $digest
' "$VERDICT" >/dev/null \
  || { echo "Verdict rejected: stale, partial, or mismatched input." >&2; exit 1; }

echo "Evidence accepted for $HEAD_SHA"

The digest formula has to be identical in both places. Copy it from one file into the other, or export it from a shared script — the moment the two commands drift by one sort flag, every verdict is silently rejected, and someone "fixes" it by loosening the gate.

Step 3 — a pre-push hook that keeps the deterministic subset honest

Hooks do not replace CI. They keep the cheap, deterministic checks off the critical path, so the required lanes stay short enough that cancelling them is never tempting.

#!/usr/bin/env bash
# .githooks/pre-push
set -euo pipefail
BASE="${BASE_REF:-origin/main}"

CHANGED="$(git diff --name-only "$BASE"...HEAD || true)"

if echo "$CHANGED" | grep -qE '^(ci/|\.github/workflows/)'; then
  echo "Pipeline config changed — running the gate self-test locally." >&2
  ./ci/gate.sh --self-test
fi

./ci/test.sh --subset=hermetic --no-retry

Enable it with git config core.hooksPath .githooks, and add one CI job that asserts the hook path is still set — otherwise the hook is decoration on everyone's machine but yours.

Step 4 — quarantine flakes with an expiry date, not a retry count

A retry nobody records is a false green with extra steps. Keep a small ledger, and make the gate read it.

# ci/flakes.yaml
- id: "tests/test_upload.py::test_resume_after_timeout"
  first_seen: 2026-09-02
  expires: 2026-10-02
  owner: "@emery"
  reason: "S3 mock intermittently returns 500"

Two rules make the ledger work:

  1. The gate fails if an entry is expired. Not warns. Fails. An expiry you can ignore is a permanent exemption with a nice date on it.
  2. Any retry increments retry_count in the verdict, and a non-zero retry_count on a required lane opens a tracking issue automatically. The retry stays visible instead of dissolving into a green check.

Choosing what is allowed to block

Lane Required? Can write to the repo? Verdict artifact?
Advisory AI review No No (comments only) Yes, complete-only
Hermetic unit tests Yes No No
Integration suite Yes, after quarantine review No No
Gate / evidence check Yes No Reads only
Deploy preview No Preview environment only No

The rule behind the table: anything that can produce a verdict must not be able to produce the input it is judging, and anything that can write to the repository must not be required.

Limitations, and who should skip this

This costs roughly a day of setup and one more artifact to keep alive. If you merge a handful of PRs a month, or your tests are not hermetic yet, the quarantine ledger becomes a place where failures go to be forgotten. Fix flakiness first.

Three more cases where this is the wrong trade:

  • Repositories where the model cannot legally see the code. If your data policy forbids sending source to a hosted model, the advisory lane does not exist and neither does most of this article.
  • Teams without artifact retention. If artifacts expire in a day and PRs live for three, the gate fails on old PRs for the wrong reason.
  • Pipelines where the expensive lane is genuinely required. A required non-deterministic lane is a queue you cannot merge through. Make it deterministic or make it advisory; do not make it required and cancellable.

Where a free lane actually fits

The advisory review lane is the right place to experiment, because it is the one lane in this design that cannot authorize a merge, cannot write to the repository, and can be cancelled without losing evidence.

The operator states that the MonkeyCode open-source project offers free model access and a free server option. Those are the two claims I am passing along. I am deliberately not quoting a token figure, a model name, or a duration here, because free-tier quotas and terms are exactly the kind of data that goes stale between writing and reading. Check the project's current terms before you size a lane around them.

If you run the lane on a free server, treat the box as ephemeral: no durable caches, no long-lived secrets, and rotate anything you put there. The gate above assumes the advisory lane is untrusted, which is the correct assumption for a free host.

Disclosure: This article was prepared as part of MonkeyCode's product outreach.

One way to use it: point the advisory lane at the free server on a scratch fork, then push a second commit while the first review is still running and watch what your gate does. If it accepts the cancelled verdict, you have found a real bug — and the bug was never in the model.

來源:DEV Community · dev.to