diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index b2e3c2a..d5de6fb 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -1,3 +1,11 @@ +## Base commit + + + +- Base sha: `` +- Checks at that sha: + ## What & why @@ -12,7 +20,10 @@ - [ ] `npm run build` passes - [ ] `npm run lint` passes - [ ] `npm run typecheck` passes +- [ ] `npm test -- --run` passes - [ ] Drove the affected route(s) in the browser (dev server on :3100) +- [ ] Any repair of shared breakage is claimed in `REPAIRS.md` and kept in its + own commit ## Notes diff --git a/.github/scripts/base-branch-gate.sh b/.github/scripts/base-branch-gate.sh new file mode 100755 index 0000000..9a3fbcd --- /dev/null +++ b/.github/scripts/base-branch-gate.sh @@ -0,0 +1,159 @@ +#!/usr/bin/env bash +# +# Publishes the `base-branch-green` commit status on open pull requests. +# +# It answers one question: is the pipeline of the branch this PR would merge +# INTO passing right now? It runs no checks and adds no verification. It reads +# the result CI already produced for the base branch's tip commit and republishes +# it as a status on the PR, where branch protection can require it. +# +# Usage: +# base-branch-gate.sh [pr-number ...] +# +# With no PR numbers, every open PR targeting is updated. That is +# how a PR un-blocks itself: when main's CI finishes, the gate re-runs and +# rewrites the status on all of them, so nobody has to push an empty commit to +# ask again. +# +# Environment: +# GH_TOKEN required, needs `statuses: write` on the repo +# REPO owner/name, defaults to the current repo +# DRY_RUN if set to 1, print what would be posted and post nothing +# +# Run it locally to see the current verdict without changing anything: +# DRY_RUN=1 .github/scripts/base-branch-gate.sh main + +set -euo pipefail + +BASE_BRANCH="${1:-main}" +shift || true + +REPO="${REPO:-$(gh repo view --json nameWithOwner --jq .nameWithOwner)}" +DRY_RUN="${DRY_RUN:-0}" + +# The workflow whose result counts as "the branch's own pipeline". +CI_WORKFLOW="${CI_WORKFLOW:-ci.yml}" + +# A PR carrying this label is exempt. A repair is how a red branch becomes +# green again, so gating repairs on the branch being green would lock the +# branch shut. The exemption is not a judgement call and not silent: the label +# is on the PR and the claim is a row in REPAIRS.md. +REPAIR_LABEL="${REPAIR_LABEL:-repair}" + +CONTEXT="base-branch-green" + +log() { printf '%s\n' "$*" >&2; } + +# --------------------------------------------------------------------------- +# The verdict on the base branch, computed once for all PRs that share it. +# --------------------------------------------------------------------------- + +base_sha="$(gh api "repos/${REPO}/commits/${BASE_BRANCH}" --jq .sha)" +short_sha="${base_sha:0:7}" + +# The newest CI run for that exact commit, whatever event produced it. +run_json="$( + gh api "repos/${REPO}/actions/workflows/${CI_WORKFLOW}/runs?head_sha=${base_sha}&per_page=100" \ + --jq '[.workflow_runs[]] | sort_by(.run_started_at // .created_at) | last // empty' +)" + +if [ -z "${run_json}" ]; then + # Nothing has judged this commit. That is not a failure, and refusing merges + # over it would gate on silence. + state="success" + description="No CI run found for ${BASE_BRANCH} @ ${short_sha}" + target_url="https://github.com/${REPO}/commits/${BASE_BRANCH}" +else + run_status="$(printf '%s' "${run_json}" | jq -r '.status')" + run_conclusion="$(printf '%s' "${run_json}" | jq -r '.conclusion // ""')" + target_url="$(printf '%s' "${run_json}" | jq -r '.html_url')" + + if [ "${run_status}" != "completed" ]; then + state="pending" + description="${BASE_BRANCH} @ ${short_sha} is still building" + else + case "${run_conclusion}" in + success) + state="success" + description="${BASE_BRANCH} @ ${short_sha} is green" + ;; + failure | timed_out | startup_failure) + state="failure" + description="${BASE_BRANCH} @ ${short_sha} is failing (${run_conclusion}) - do not merge onto it" + ;; + cancelled | skipped | neutral | stale) + # A cancelled run judged nothing, so it reports nothing. Treating it as + # a failure is the false record this gate exists to stop repeating. + state="success" + description="${BASE_BRANCH} @ ${short_sha}: last run was ${run_conclusion}, not a failure" + ;; + *) + state="success" + description="${BASE_BRANCH} @ ${short_sha}: unrecognised conclusion ${run_conclusion}, not treated as failure" + ;; + esac + fi +fi + +log "base=${BASE_BRANCH} sha=${base_sha} state=${state}" +log "reason=${description}" + +# --------------------------------------------------------------------------- +# The pull requests to publish it on. +# --------------------------------------------------------------------------- + +if [ "$#" -gt 0 ]; then + pr_numbers="$*" +else + pr_numbers="$( + gh pr list --repo "${REPO}" --base "${BASE_BRANCH}" --state open \ + --limit 100 --json number --jq '.[].number' + )" +fi + +if [ -z "${pr_numbers}" ]; then + log "No open pull requests target ${BASE_BRANCH}; nothing to publish." + exit 0 +fi + +exit_code=0 + +for pr in ${pr_numbers}; do + pr_json="$(gh pr view "${pr}" --repo "${REPO}" --json headRefOid,labels,title,isDraft)" + head_sha="$(printf '%s' "${pr_json}" | jq -r '.headRefOid')" + has_repair_label="$( + printf '%s' "${pr_json}" | jq -r --arg l "${REPAIR_LABEL}" \ + '[.labels[].name] | index($l) != null' + )" + + pr_state="${state}" + pr_description="${description}" + + if [ "${has_repair_label}" = "true" ] && [ "${state}" = "failure" ]; then + pr_state="success" + pr_description="Repair PR: gate waived so a red ${BASE_BRANCH} can be fixed. Claim it in REPAIRS.md." + fi + + # The statuses API truncates past 140 characters; do it here so the text + # stays a sentence rather than a fragment. + pr_description="$(printf '%.140s' "${pr_description}")" + + if [ "${DRY_RUN}" = "1" ]; then + printf 'would post: pr=#%s sha=%s state=%s "%s"\n' \ + "${pr}" "${head_sha}" "${pr_state}" "${pr_description}" + continue + fi + + if gh api --silent --method POST "repos/${REPO}/statuses/${head_sha}" \ + -f "state=${pr_state}" \ + -f "context=${CONTEXT}" \ + -f "description=${pr_description}" \ + -f "target_url=${target_url}"; then + log "posted: pr=#${pr} sha=${head_sha} state=${pr_state}" + else + log "FAILED to post status on pr=#${pr} sha=${head_sha}" + exit_code=1 + fi +done + +exit "${exit_code}" diff --git a/.github/workflows/base-branch-gate.yml b/.github/workflows/base-branch-gate.yml new file mode 100644 index 0000000..67db8a5 --- /dev/null +++ b/.github/workflows/base-branch-gate.yml @@ -0,0 +1,83 @@ +name: Base branch gate + +# Publishes the `base-branch-green` status on open pull requests, which branch +# protection requires before a merge. It runs no checks of its own -- it reads +# the result CI already produced for the tip of the branch a PR would merge +# into, and republishes it where a merge button can see it. +# +# Why: main broke four times through pairs of changes that never touched the +# same lines. Every one of those sessions merged onto a base that was already +# failing, or onto one whose failure was about to be discovered, and CI on the +# PR could not say so because the PR was green in isolation. This does not add +# a check. It gives an existing check's result authority over a merge. + +on: + pull_request: + # `labeled`/`unlabeled` are here because the `repair` label waives the gate, + # so adding it has to re-publish the status. + types: [opened, synchronize, reopened, ready_for_review, labeled, unlabeled] + workflow_run: + workflows: [CI] + types: [completed] + workflow_dispatch: + inputs: + base_branch: + description: Branch to re-publish the gate for + required: false + default: main + +permissions: + contents: read + actions: read + pull-requests: read + statuses: write + +# Never cancel: a cancelled run here would leave a stale verdict standing. +concurrency: + group: base-gate-${{ github.event.pull_request.number || github.event.workflow_run.head_branch || github.ref }} + cancel-in-progress: false + +jobs: + publish: + # Deliberately NOT named `base-branch-green`. This job succeeds whenever it + # manages to publish, including when what it publishes is a failure. The + # gate is the commit status it posts, and branch protection must require + # that; a same-named check run here would shadow it and always be green. + name: Publish base branch verdict + runs-on: ubuntu-latest + # On workflow_run, only a post-merge run on the branch itself changes the + # verdict. A PR's own CI run says nothing about the branch it targets. + if: >- + github.event_name != 'workflow_run' || github.event.workflow_run.event == 'push' + steps: + - name: Checkout + uses: actions/checkout@v4 + + # On `pull_request` this publishes for that PR alone. On `workflow_run` it + # publishes for every open PR targeting the branch whose CI just finished, + # so a PR blocked by a red main clears itself the moment main goes green. + # + # Note: a PR from a fork gets a read-only token, so the status cannot be + # posted and the required check stays absent -- the merge is refused + # rather than waved through. Chunk sessions push branches to this repo. + - name: Publish base-branch-green + env: + GH_TOKEN: ${{ github.token }} + REPO: ${{ github.repository }} + run: | + set -euo pipefail + case "${{ github.event_name }}" in + pull_request) + .github/scripts/base-branch-gate.sh \ + "${{ github.event.pull_request.base.ref }}" \ + "${{ github.event.pull_request.number }}" + ;; + workflow_run) + .github/scripts/base-branch-gate.sh \ + "${{ github.event.workflow_run.head_branch }}" + ;; + *) + .github/scripts/base-branch-gate.sh \ + "${{ inputs.base_branch || 'main' }}" + ;; + esac diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 34c8f59..25b1e67 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -9,9 +9,19 @@ on: permissions: contents: read +# Pull-request runs supersede each other: only the newest push to a PR branch +# needs an answer, and the run it cancels belongs to a sha nobody will merge. +# +# Post-merge runs on main are not interchangeable that way. Each one is the +# record of what a single merge commit shipped, and two merges landing a minute +# apart share a ref, so the old grouping had the second merge cancel the first. +# A cancelled run is recorded as a red check against the commit it superseded -- +# see 1622ed7 (F5), cancelled 17s in by the R2 merge and still carrying an X for +# code that was never judged. So push runs get a group per sha and are never +# cancelled: the record of a merge is always that merge's own result. concurrency: - group: ci-${{ github.workflow }}-${{ github.ref }} - cancel-in-progress: true + group: ci-${{ github.workflow }}-${{ github.event_name == 'pull_request' && github.ref || github.sha }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} jobs: checks: diff --git a/AGENTS.md b/AGENTS.md index 0c4f300..c365233 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,5 +1,71 @@ # Repository instructions +## Before you build: check your base commit + +`main` has broken four times in three days, and never once through a git +conflict. Each time, two changes that did not overlap textually were merged, +and the breakage existed only in the combination that neither branch had been +tested against. Git cannot raise a conflict for that. These rules are what +stands in its place. + +- **Run the checks at your base commit and report the sha, before you build + anything.** + + ``` + git rev-parse HEAD + npm run lint && npm run typecheck && npm test -- --run && npm run build + ``` + + Put that sha in your first report and in the PR description. "The checks + pass" is not a fact about this repo; it is a fact about one commit. + +- **Do not inherit a failing-check list from an earlier report.** Another + session's account of what was broken was true of that session's base commit, + not yours. Re-run the checks and report what you actually saw. + +- **If the base is red, say so and stop.** Report the sha and the failing step. + Do not start the chunk on top of it. + +- **Do not repair a red base inside a feature chunk.** That buries a shared + defect in an unrelated diff, where it cannot be reviewed on its own, cannot be + reverted on its own, and cannot be found by the session that hits it next. + +## Repairing breakage you did not come here to write + +If you must repair shared breakage, claim it **before you write the fix**, in +`REPAIRS.md` at the repo root. That file is the only place a repair is claimed, +and reading it is part of starting work: + +``` +git fetch origin +git show origin/main:REPAIRS.md # claims that have landed +gh pr list --label repair # claims still in flight +``` + +- **If the repair is already claimed, do not write your own.** The row names the + branch; rebase onto it (`git rebase origin/`). Two independent + fixes for one defect is how `main` ended up with duplicated code that git had + no conflict to raise. +- **If it is unclaimed and you are taking it**, add the row to `REPAIRS.md`, + open the PR straight away, and label it `repair`. +- **Put the repair in its own commit**, separate from any feature work, so it + can be reviewed or reverted alone. + +## Merging + +- **A merge into `main` is refused while `main`'s own pipeline is failing.** The + `base-branch-green` check on your PR republishes the result of `main`'s last + CI run; branch protection requires it. This is not a warning and not a + reviewer's judgement — the merge is blocked. It gates the merge, not your + build: keep working, and merge when the base is green. +- **You do not have to push anything to ask again.** When `main`'s CI finishes, + the check re-posts itself on every open PR. +- **A `repair`-labelled PR is exempt**, because a repair is how a red branch + becomes green. That label is a claim in `REPAIRS.md`, not a way past the gate. +- **Never weaken a guard to make a pipeline pass** — no deleted assertion, no + skipped test, no relaxed lint rule. If a guard is wrong, that is a repair: + claim it and fix it in its own commit. + ## Node and npm reproducibility - Webflow Cloud installs this project with npm `10.9.7`. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2f37a73..dc73d30 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -4,10 +4,12 @@ This repo uses a **branch + pull request** workflow. Don't commit directly to `m ## Workflow -1. Branch off an up-to-date `main`: +1. Branch off an up-to-date `main`, and check that it is actually green before + you build on it (see AGENTS.md, "Before you build: check your base commit"): ``` git checkout main git pull + git rev-parse HEAD # report this sha git checkout -b / ``` Use a `type/` prefix: `feat/`, `fix/`, `chore/`, `docs/`, `refactor/`. @@ -29,6 +31,12 @@ This repo uses a **branch + pull request** workflow. Don't commit directly to `m 4. Merge after review and green checks. Squash-merge keeps `main` history tidy. + A PR also carries a `base-branch-green` check, which reports whether `main` + itself is passing. While `main` is red the merge is refused; the check + re-posts itself when `main`'s CI finishes, so there is nothing to push. If + you are fixing `main`, claim it in `REPAIRS.md` and label the PR `repair`, + which waives that check. + ## Local setup - `npm install` (this environment wraps npm with Socket Firewall; if an install is diff --git a/REPAIRS.md b/REPAIRS.md new file mode 100644 index 0000000..2b52062 --- /dev/null +++ b/REPAIRS.md @@ -0,0 +1,97 @@ +# Repair ledger + +A **repair** is a change to code you did not come here to write, made because +`main` is already broken. Two sessions repairing the same breakage independently +is how `main` acquired duplicated fixes and, once, a pair of conflict markers +nobody resolved. This file is the one place a repair is claimed, and it is +claimed **before it is written**. + +Read it before you start work. Add to it before you fix anything you do not own. + +## Before you start + +``` +git fetch origin +git show origin/main:REPAIRS.md # every claim that has landed +gh pr list --label repair # every claim still in flight +``` + +The second command matters: a claim is minutes old before it reaches `main`, and +minutes is exactly the window in which two sessions collide. A `repair`-labelled +PR is a claim whether or not you can see its row on `main` yet. + +## If the breakage you found is already claimed + +**Do not write your own fix.** The row names the branch. Rebase onto it: + +``` +git fetch origin +git rebase origin/ +``` + +If it has not landed yet and you cannot proceed without it, say so in your +report and stop. Waiting one round costs less than two fixes for one defect, +which is what `main` got the last time, and one of them survived as duplicate +code because git had no conflict to raise. + +## If it is not claimed and you are taking it + +1. **Add the row first.** Append an entry under _Open_ below, with the base sha + you saw it at, the symptom, and the branch you will fix it on. +2. **Open the PR immediately** — draft is fine — and label it `repair`. The + label is what makes the claim visible to a session that fetched a minute ago, + and it waives the `base-branch-green` merge gate, because a repair is how a + red branch becomes green again. +3. **Keep the repair in its own commit**, separate from any feature work in the + same PR, so it can be reviewed or reverted on its own. +4. When it lands, move the row to _Landed_. + +A repair is not a place to also fix the thing next to it. One defect, one row, +one commit. + +--- + +## Open + +### Conflict markers committed into `DECISIONS.md` + +- **Claimed by:** _unclaimed_ +- **Seen at:** `6f0076d` (`main`) +- **Introduced by:** `aeeca8b`, "Merge main (S8, PERSON fix) into S4", landed via + PR #88 +- **Symptom:** `DECISIONS.md` lines 3–13 are a literal unresolved merge + conflict — `<<<<<<< HEAD`, `=======`, `>>>>>>> origin/main` — sitting in the + intro paragraph. No check reads markdown, so CI was green on the commit that + shipped it and is green on `main` now. +- **What the two sides are:** the `HEAD` side (from S4, `b9870b8`) dropped the + count and says "product questions"; the `origin/main` side (from S8, + `296106d`) still says "five previously-undefined product questions" and + "a one-line edit". The file now has six numbered decisions, and decision 6 is + not a one-line edit, so the un-counted wording is the one that is true. +- **Whoever takes it:** this is one paragraph and its owner is whoever owns + `DECISIONS.md` prose. Claim it here first, then fix it in its own commit. + +## Landed + +### F4's field rename left S7's callers and fixtures behind + +- **Claimed by:** R2 (retroactive entry; the ledger did not exist yet) +- **Seen at:** `728eb6c` (`main`, red) +- **Branch / PR:** `chunk-r2` / #86 — `0d5b849` +- **Symptom:** F4 renamed a field and swept its own callers. S7's tests passed + their fixtures the old shape. Neither branch touched the other's lines, both + were green alone, and the merge result was red. +- **Repaired twice.** `chunk-f5` also fixed it, in `04c4c2e` ("finish F4's + caller sweep over S7's files"), across three of the same four files. Git + found no conflict and took both copies, which is why `bd73eb0` ("one PERSON + caller per test file, not two", PR #87) had to follow and delete the + duplicates. This entry is the reason this file exists. + +### Duplicate `PERSON` const in three test files + +- **Claimed by:** the F5/R2 double-repair above +- **Seen at:** `96964f9` (`main`, red) +- **Branch / PR:** `fix/duplicate-person-const` / #87 — `bd73eb0` +- **Symptom:** `digest-arrival.test.ts`, `digest.test.ts` and `webhook.test.ts` + each declared the same const twice, one copy from each of the two independent + repairs above.