diff --git a/docs/review/calibration-2026-08.md b/docs/review/calibration-2026-08.md index 29075b8..a3088d1 100644 --- a/docs/review/calibration-2026-08.md +++ b/docs/review/calibration-2026-08.md @@ -178,29 +178,47 @@ The test still asserts the pre-META-291 absolute-suffix fallback, so it is red o both heads for opposite reasons, and no gate runs it. Tracked as **META-329**, not repaired here. -## Decision +## Decision — ratified and applied 2026-08-12 -`Greptile Review` **is eligible** to become a required current-head merge gate on -this repository. It fires, binds to the exact head, caught its positive control, -stayed quiet on the clean head, and retriggers on push. +`Greptile Review` **is required** on `main`, in the narrow role it demonstrably +performs: it fires, binds to the exact head, caught its positive control, stayed +quiet on the clean head, and retriggers on push. -Recommended change — **not applied by META-322**, because it has a side effect on -another stream's open PR: +The ratified contract is two-part: -``` -required_status_checks.contexts += "Greptile Review" -``` +1. `Greptile Review` required status — **the current head was actually reviewed**; +2. the already-enabled `required_conversation_resolution` — **actionable findings + are not left open**. -Two things must be understood by whoever applies it: +Greptile status is **not** semantic approval. Its concluding `success` on the canary +head carrying a P1 is not a defect in the gate, because the status was assigned only +the completion role. The semantic half is conversation resolution plus the written +per-finding protocol in [`merge-policy.md`](merge-policy.md) §3. -* It enforces **review completion on the current head**, not absence of findings — - see *The check state does not encode findings*. Pairing it with the already-enabled - `required_conversation_resolution` is what makes the combination meaningful. -* PR #6 is currently `BEHIND` and predates the policy. Adding the context requires - it to update and take a Greptile run before it can merge. That is a cost to a PR - this issue does not own, which is why the mutation is left as an explicit, - separately-owned step rather than made silently here. +### Branch protection, before and after + +| Setting | Before (`a31242b`) | After (`f61e0cb`) | +| --- | --- | --- | +| Required contexts | `build-and-smoke (20)`, `build-and-smoke (22)` | `build-and-smoke (20)`, `build-and-smoke (22)`, **`Greptile Review`** (app id `867647`) | +| `strict` | `true` | `true` — preserved | +| `required_conversation_resolution` | `true` | `true` — preserved | +| `required_approving_review_count` | `0` | `0` — preserved | +| `dismiss_stale_reviews` | `true` | `true` — preserved | +| `enforce_admins` | `false` | `false` — preserved | +| `allow_force_pushes` / `allow_deletions` | `false` / `false` | unchanged | +| `Sourcery review` required | no | **no — deliberately not promoted** | +| Rulesets | `[]` | `[]` | + +Verified by an independent read-back of the protection API, not from the write +response. + +### Deliberately accepted cost + +PR #6 predates the policy and its head carries no Greptile run at all, so it cannot +satisfy the context until it updates. Requiring it to update and receive a +current-policy review is the intended effect of a stricter merge contract, not +collateral damage to be designed around. The gate was not weakened to preserve it. The two unrequired CI contexts (`parity-receipt-reproduction`, -`standard-candidate-consumption`) are a separate question, recorded in +`standard-candidate-consumption`) remain a separate question, recorded in [`merge-policy.md`](merge-policy.md) §2 and deliberately untouched. diff --git a/docs/review/merge-policy.md b/docs/review/merge-policy.md index 238b9ee..f5f5049 100644 --- a/docs/review/merge-policy.md +++ b/docs/review/merge-policy.md @@ -47,9 +47,16 @@ Nothing in this document elevates Sourcery on the basis of its check state. Values below were read from the GitHub branch-protection API for `main`. +> **Division of responsibility.** This document is the *current contract*: what is +> required now and how to read it. [`calibration-2026-08.md`](calibration-2026-08.md) +> is the *evidence and change record*: the before/after protection state, the +> calibration that justified the change, and the SHAs it was measured on. When a +> setting changes, the new state is recorded here and the transition is recorded +> there — so the two are not two copies of the same claim. + | Setting | Measured value | | --- | --- | -| Required status checks | `build-and-smoke (20)`, `build-and-smoke (22)` | +| Required status checks | `build-and-smoke (20)`, `build-and-smoke (22)`, **`Greptile Review`** | | `strict` (branch must be up to date) | `true` | | `required_conversation_resolution` | `true` | | `required_approving_review_count` | `0` | @@ -58,9 +65,26 @@ Values below were read from the GitHub branch-protection API for `main`. | `allow_force_pushes` / `allow_deletions` | `false` / `false` | | Repository rulesets | none (`[]`) | -So today, a merge into `main` requires: the two `build-and-smoke` contexts green on -an up-to-date head, and **every conversation resolved**. No review approval is -required, and no reviewer app is required. +So a merge into `main` requires: the two `build-and-smoke` contexts and +`Greptile Review` green on an up-to-date head, and **every conversation resolved**. +No review approval is required. + +`Greptile Review` was added to the required contexts on 2026-08-12 after the +calibration in [`calibration-2026-08.md`](calibration-2026-08.md), and is pinned to +app id `867647` — only that app can satisfy the context, so a same-named check from +elsewhere cannot. `Sourcery review` is deliberately **not** required. + +**Read the requirement precisely.** The Greptile context asserts *the current head +was reviewed*, not *the review found nothing* — the check concludes `success` even +on a head carrying a P1 finding. The semantic half of the gate is +`required_conversation_resolution` plus the written per-finding protocol in §3. The +two are only meaningful together, and neither is a substitute for reading the +findings. + +This was observed working end to end on PR #12 at head `76d495d`: all three required +contexts `success`, `SonarCloud Code Analysis` **failing but not required and +therefore not blocking**, and `mergeStateStatus=BLOCKED` on a single unresolved +Greptile P1 thread. ### Observed gap — recorded, not acted on here @@ -94,7 +118,11 @@ is stale — treat that as a documentation defect, and correct it in the PR that noticed. A merge-eligibility claim that has drifted from the setting it describes is worse than no claim, because it will be believed. -Measured on 2026-08-12 against `main` at `a31242b`. +Measured on 2026-08-12 against `main` at `f61e0cb`, immediately after +`Greptile Review` was added to the required contexts. The earlier reading in +[`calibration-2026-08.md`](calibration-2026-08.md) — `main` at `a31242b`, two +required contexts — is the pre-policy baseline and is kept there as the *before* +half of the record, not as a description of current state. ## 3. Conversation resolution @@ -113,13 +141,13 @@ The protocol: ## 4. `Greptile Review` as a required check -**Current state: not required. Measured as eligible — see -[`calibration-2026-08.md`](calibration-2026-08.md) — with the required-contexts -change left as an explicit, separately-owned step.** +**Current state: required, as of 2026-08-12.** It was promoted only after all seven +criteria below were observed on this repository, and only into the narrow role it +demonstrably performs — see [`calibration-2026-08.md`](calibration-2026-08.md). -The bar for adding `Greptile Review` to the required-contexts list is behavioral, -not configurational. Each of the following must be observed on this repository — -not inherited from `workspacejson/standard`, where the mechanism was proven: +The bar was behavioral, not configurational. Each of the following was observed on +this repository — not inherited from `workspacejson/standard`, where the mechanism +was proven: 1. Branch-local `.greptile/` configuration is demonstrably read on a PR head. 2. The review is associated with the **exact current head SHA**, not with the PR in @@ -148,6 +176,32 @@ and the two are only meaningful together. And that gate is itself softer than it appears: GitHub auto-resolves a thread whose lines a later commit removed, so resolution can be satisfied without anyone answering the finding. Hence §3 — the receipt has to be written into the thread. +GitHub cannot encode that distinction mechanically, which is why the written +per-finding disposition, not the `isResolved` bit, is the evidence. + +### Observed working + +First post-policy proof, PR #12 (META-285) at head `76d495d`, base `f61e0cb`: + +| Signal | Observed | +| --- | --- | +| `build-and-smoke (20)` / `(22)` | success / success | +| `Greptile Review` | success — `5 files reviewed, 0 comments added` on this head | +| `SonarCloud Code Analysis` | **failure — not a required context, did not block** | +| `Sourcery review` | success (not required) | +| Unresolved threads | 1 — a Greptile P1 on `scripts/migration/verify-receipt.mjs` | +| `mergeStateStatus` | **`BLOCKED`** | + +Every required status context was satisfied and the PR was still blocked, on the +conversation-resolution half alone. That is the two-part contract working: status +proved the current head was reviewed, resolution kept an open actionable finding +from being merged past. + +The same update also demonstrated the intended cost of a stricter contract: PR #12 +and PR #6 both went `BEHIND` when `main` moved, and each must take a current-policy +review before merging. PR #6's pre-policy head has **no** Greptile run at all, so it +cannot satisfy the context until it updates — intended behavior, not collateral +damage. ## 5. What a check state does and does not mean