From 9d06a572b58631cc9d53446390aeba344fb7a9d7 Mon Sep 17 00:00:00 2001 From: Qwynn Marcelle Date: Wed, 12 Aug 2026 11:39:13 -0400 Subject: [PATCH 1/2] docs(review): record Greptile Review as a required context MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Branch protection was changed on 2026-08-12 after ratification, so the merge policy as committed no longer described the live setting. merge-policy.md §2 declares that when the API and this document disagree the API is right and the document is the defect, to be corrected by the PR that notices — this is that correction. Records the before/after protection state, that strict, both build-and-smoke contexts, and required_conversation_resolution were preserved, that the context is pinned to app id 867647, and that Sourcery was deliberately not promoted. Also records the first post-policy proof on PR #12 at 76d495d: all three required contexts green, SonarCloud failing but unrequired and therefore not blocking, and mergeStateStatus BLOCKED on one unresolved Greptile P1 — the status half proving the head was reviewed, the resolution half holding the merge. States the accepted cost plainly: PR #6 predates the policy and has no Greptile run, so it must update before it can merge. The gate was not weakened for it. No product behavior change. No changeset. Refs META-322 --- docs/review/calibration-2026-08.md | 56 ++++++++++++++++++-------- docs/review/merge-policy.md | 63 +++++++++++++++++++++++++----- 2 files changed, 92 insertions(+), 27 deletions(-) diff --git a/docs/review/calibration-2026-08.md b/docs/review/calibration-2026-08.md index 29075b8..743e4ad 100644 --- a/docs/review/calibration-2026-08.md +++ b/docs/review/calibration-2026-08.md @@ -178,28 +178,50 @@ 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`) remain a separate question, recorded in +[`merge-policy.md`](merge-policy.md) §2 and deliberately untouched. The two unrequired CI contexts (`parity-receipt-reproduction`, `standard-candidate-consumption`) are a separate question, recorded in diff --git a/docs/review/merge-policy.md b/docs/review/merge-policy.md index 238b9ee..d971aac 100644 --- a/docs/review/merge-policy.md +++ b/docs/review/merge-policy.md @@ -49,7 +49,7 @@ Values below were read from the GitHub branch-protection API for `main`. | 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 +58,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 @@ -113,13 +130,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 +165,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 From e3f8d0e7afb767ec337720102dc9b44c6e226ea9 Mon Sep 17 00:00:00 2001 From: Qwynn Marcelle Date: Wed, 12 Aug 2026 11:43:17 -0400 Subject: [PATCH 2/2] docs(review): reconcile review findings on the required-context record MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - correct the measurement stamp in merge-policy.md §2: the table describes main at f61e0cb, not the pre-policy a31242b reading, which stays in the calibration record as the before half - remove a duplicated paragraph in the calibration decision section - state the division of responsibility between the two documents: merge-policy is the current contract, calibration is the evidence and change record Refs META-322 --- docs/review/calibration-2026-08.md | 4 ---- docs/review/merge-policy.md | 13 ++++++++++++- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/docs/review/calibration-2026-08.md b/docs/review/calibration-2026-08.md index 743e4ad..a3088d1 100644 --- a/docs/review/calibration-2026-08.md +++ b/docs/review/calibration-2026-08.md @@ -222,7 +222,3 @@ collateral damage to be designed around. The gate was not weakened to preserve i The two unrequired CI contexts (`parity-receipt-reproduction`, `standard-candidate-consumption`) remain a separate question, recorded in [`merge-policy.md`](merge-policy.md) §2 and deliberately untouched. - -The two unrequired CI contexts (`parity-receipt-reproduction`, -`standard-candidate-consumption`) are 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 d971aac..f5f5049 100644 --- a/docs/review/merge-policy.md +++ b/docs/review/merge-policy.md @@ -47,6 +47,13 @@ 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)`, **`Greptile Review`** | @@ -111,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