From aa456b44536da1885d2af81a2bad7835336db76e Mon Sep 17 00:00:00 2001 From: Qwynn Marcelle Date: Wed, 12 Aug 2026 09:31:37 -0400 Subject: [PATCH 1/2] chore(review): add repo-owned Greptile policy and merge-policy documentation Integrations has had no repository-owned review policy, so reviewer behavior here has never been governed by this repository's own failure classes. Rules are derived from verified Integrations surfaces and defects, not copied from workspacejson/standard: - proven-root-path-identity, ported-copies-must-not-diverge: META-291 / PR #8 removed a suffix fallback that matched a stored key against a file in an unrelated repository; src/path-match.ts records that a second, drifted matcher is how a deny silently became a warn. extension/src/pathMatch.ts is still a hand-synced port. - unavailable-is-not-approval: hooks/pre-edit-check.mjs routes failures through emitUnavailable and never emits an approval; src/reviewer.ts keeps UNAVAILABLE distinct from PASS. Exercised by the smoke suite. - reader-tolerance-is-not-repair: normalizeWorkspace degrades to empty and never fabricates; no read path holds a write to the artifact. - packed-artifact-must-be-measured: dist/ is gitignored but shipped, and the hook resolves ../dist/* at runtime, so source-only tests do not prove what a consumer receives. - probes-must-not-be-destructive: META-285 records --help running a destructive install into the source repo; the surrounding installer surface is genuinely destructive, so probe versus action has to be explicit. - host-contract-claims-need-a-watched-version: the Codex output contract is recorded as watched on 0.144.1 (2026-07-13) with a single adapter point. - clean-room-public-boundary, consume-standard-do-not-vendor, and the four ecosystem evidence rules. Standard's four-read-paths, negative-fixture, and derived-probability rules are deliberately not carried over; CLI producer/history rules have no equivalent verified failure mode here. .greptile/rules.md records each omission. .sourcery.yaml is path scoping only. Sourcery keeps no semantic authority: a success check from it is not review approval. config/repository-structure.json gains .greptile and .sourcery.yaml via its documented allowlist path; the checker script is untouched. The check was watched failing with 2 violations before the amendment. No product behavior change. No package bytes, exports, or metadata change. Refs META-322 --- .greptile/config.json | 126 +++++++++++++ .greptile/files.json | 44 +++++ .greptile/rules.md | 273 +++++++++++++++++++++++++++++ .sourcery.yaml | 18 ++ config/repository-structure.json | 4 +- docs/review/calibration-2026-08.md | 102 +++++++++++ docs/review/merge-policy.md | 131 ++++++++++++++ 7 files changed, 697 insertions(+), 1 deletion(-) create mode 100644 .greptile/config.json create mode 100644 .greptile/files.json create mode 100644 .greptile/rules.md create mode 100644 .sourcery.yaml create mode 100644 docs/review/calibration-2026-08.md create mode 100644 docs/review/merge-policy.md diff --git a/.greptile/config.json b/.greptile/config.json new file mode 100644 index 0000000..0f87d06 --- /dev/null +++ b/.greptile/config.json @@ -0,0 +1,126 @@ +{ + "strictness": 1, + "triggerOnUpdates": true, + "triggerOnDrafts": true, + "statusCheck": true, + "ignorePatterns": "node_modules/**\nextension/node_modules/**\ndist/**\nvsix/**\ncoverage/**\n.parity-ci/**\n.git/**\npackage-lock.json\nextension/package-lock.json", + "summarySection": { + "included": true, + "collapsible": true, + "defaultOpen": false + }, + "issuesTableSection": { + "included": true, + "collapsible": false + }, + "confidenceScoreSection": { + "included": true, + "collapsible": true, + "defaultOpen": false + }, + "instructions": "This repository is workspacejson/integrations. It owns host adaptation, delivery, packaging, hooks, installers, and first-party adapter parity for workspace.json. It CONSUMES the public workspace.json contract; it does not own, copy, repair, or re-specify it — workspacejson/standard owns schema, validation semantics, rules, and ADRs. Published surface: the @workspacejson/codex-mcp npm package (MCP server, Codex hook, installer) and the VS Code extension. Two things make this repository's failure modes distinct from standard's: (1) it reads an artifact it did not produce, on a host machine, so path identity, tolerance, and unavailability semantics are the risk surface, not schema correctness; (2) it ships a packed artifact whose runtime layout differs from the source tree — dist/ is gitignored, hooks/pre-edit-check.mjs resolves ../dist/* at runtime, and package.json 'files' governs what a consumer actually receives, so source-only tests do not prove shipped behavior. The repository is read-only and descriptive toward the artifact: it surfaces and adapts evidence, and never rewrites, repairs, or adds policy to workspace.json. It is a public, clean-room repository: no private organizational tracker id, private sidecar artifact, private source reference, or running-daemon assumption may appear in any file, including comments. See .greptile/rules.md for each rule's verified failure class and the rules deliberately NOT carried over from workspacejson/standard.", + "rules": [ + { + "id": "evidence-load-bearing", + "rule": "Evidence claims must be load-bearing. A verification check that cannot fail proves nothing. If a test, guard, smoke assertion, or receipt comparison is added or changed, it must be capable of failing for the defect it names. A check whose subject is stubbed, mocked away, or unreachable is documentation, not verification.", + "severity": "high" + }, + { + "id": "absence-not-success", + "rule": "Absence, skipped, unsupported, or unavailable is never success, false, safe, empty, allow, or green. A gate that goes green because it could not find the thing it measures reports conformance it never measured. Missing evidence must be reported as missing. This applies to CI jobs, smoke checks, parity receipts, and host adapters alike.", + "severity": "high" + }, + { + "id": "metrics-perturb", + "rule": "Metrics and receipts must perturb when their referent changes. A parity receipt, hash, count, or score that does not move when the thing it measures moves is decorative, not a measurement. If a receipt or metric is added or updated, it must be shown to respond to a change in its input.", + "severity": "high" + }, + { + "id": "bidirectional-checks", + "rule": "Every verification check must be considered in both directions: cannot-ever-pass and cannot-ever-fail. A guard that denies everything and a guard that allows everything both look identical to a working guard from a green build. When a check is added or modified, both failure directions must be reasoned about.", + "severity": "high" + }, + { + "id": "clean-room-public-boundary", + "rule": "This is a public, clean-room repository. No private organizational tracker identifier (for example VR-nnn, HAC-nnn), no private product or platform name, no private sidecar artifact, no cross-organization implementation dependency, and no assumption that a daemon or live service is running may appear in any file, including source comments, docs, config, and test fixtures. Public issue references (META-nnn, ADR-nnn) and public package names are fine. The artifact and this integration must remain meaningful with nothing running.", + "severity": "high" + }, + { + "id": "consume-standard-do-not-vendor", + "rule": "This repository consumes the public workspace.json contract; it must not vendor or reimplement it. Do not add a copied JSON Schema, a parallel validator, a re-derived rules engine, or a second definition of conformance here. Reading and normalizing the artifact into a local consumer model is correct; asserting normative validity, or repairing the artifact toward a schema, belongs to workspacejson/standard. If a contract gap blocks work, the fix is an upstream issue, not a local reimplementation.", + "scope": ["src/**", "extension/src/**", "hooks/**", "scripts/**"], + "severity": "high" + }, + { + "id": "reader-tolerance-is-not-repair", + "rule": "Reader tolerance must never become repair or mutation of the committed artifact. Normalization may degrade to empty and may accept legacy shapes, but it must not fabricate values, invent defaults that are then presented as producer output, or write back to workspace.json. No code path that reads the artifact may open it for writing. Tolerating an unknown shape is not the same as declaring it valid.", + "scope": ["src/services/**", "src/tools/**", "src/evidence.ts", "hooks/**", "extension/src/**"], + "severity": "high" + }, + { + "id": "proven-root-path-identity", + "rule": "Path identity follows ratified ADR-006 §4/§8. An absolute host query is comparable only after containment in a repository root the caller has PROVEN; if containment cannot be proven the answer is no-match, which is a refusal, never a nearest guess. Do not introduce suffix matching, basename matching, endsWith comparison, symmetric fuzzy matching, or an optional/defaulted root parameter that lets an absolute query silently degrade. Stored keys are repo-root-relative POSIX. The removed suffix fallback matched a stored src/a.ts against /elsewhere/unrelated-repo/src/a.ts — an assertion landing on a file in a different repository.", + "scope": [ + "src/path-match.ts", + "src/services/**", + "src/tools/**", + "src/evidence.ts", + "extension/src/pathMatch.ts", + "extension/src/**", + "hooks/**" + ], + "severity": "high" + }, + { + "id": "ported-copies-must-not-diverge", + "rule": "src/path-match.ts and extension/src/pathMatch.ts are the same contract expressed twice: the extension is a standalone package with no build-time dependency on the server, so the matcher is ported by hand rather than imported. A change to the semantics of one without a corresponding change or an explicit written reconciliation for the other is a divergence defect, not a local edit. A second, drifted matcher is how a deny silently became a warn once already. The same applies to any other capability duplicated between the server and a host adapter.", + "scope": ["src/path-match.ts", "extension/src/pathMatch.ts", "src/**", "extension/src/**"], + "severity": "high" + }, + { + "id": "unavailable-is-not-approval", + "rule": "Malformed, absent, unreadable, or unsupported artifact state must remain observable and distinct, and must never be rendered as approval or as a clean result. A hook that cannot load the artifact must say no determination was made; it must not emit an approve, and silence must not read as endorsement. A reviewer or verdict surface must keep UNAVAILABLE distinct from PASS, and PASS must mean no blocking issue was found in the examined scope, never a safety certification. Do not collapse an error branch into the same output as a clean branch.", + "scope": [ + "hooks/**", + "src/reviewer.ts", + "src/tools/**", + "src/services/**", + "src/evidence.ts", + "extension/src/reviewerVerdict.ts", + "extension/src/**" + ], + "severity": "high" + }, + { + "id": "host-adaptation-stays-descriptive", + "rule": "This repository adapts and surfaces what the artifact describes; it does not turn workspace.json into prescriptive policy. Do not write policy, approval, gate, or merge-blocking fields into the artifact, and do not document the standard as mandating an action. Host-side enforcement driven by observed evidence (for example a pre-edit hook decision) is in scope for this repository, but it must be presented as this integration's behavior over described evidence, never as a requirement the standard imposes.", + "scope": ["src/**", "extension/src/**", "hooks/**", "docs/**", "README.md"], + "severity": "high" + }, + { + "id": "packed-artifact-must-be-measured", + "rule": "Shipped behavior must be measured on the packed or installed artifact, not inferred from source-only tests. dist/ is gitignored and hooks/pre-edit-check.mjs resolves ../dist/* at runtime, so package.json 'files', 'bin', 'exports', the build step, and the hook's runtime resolution can break the shipped package while every source test still passes. Any change to those surfaces needs evidence from a pack, install, or equivalent packaged run. A source-tree test passing is not evidence that a consumer receives working files.", + "scope": [ + "package.json", + "extension/package.json", + "scripts/install.mjs", + "hooks/**", + ".github/workflows/**", + "tests/**" + ], + "severity": "high" + }, + { + "id": "probes-must-not-be-destructive", + "rule": "A command presented as harmless — help, version, status, check, probe, dry run, smoke — must not perform destructive or installing work. It must not install or uninstall an extension, delete or overwrite files outside a disposable temporary directory it created, mutate the user's or the repository's checkout, or write into the source repository under test. Destructive setup and teardown must be confined to a temp root the script created and must be named as such. If a check needs a real install to be meaningful, it must be explicitly opt-in and must say so.", + "scope": ["scripts/**", "hooks/**", ".github/workflows/**"], + "severity": "high" + }, + { + "id": "host-contract-claims-need-a-watched-version", + "rule": "Host output contracts are observed facts about a specific host version, not stable APIs. A claim that a host contract works — a Codex hook permissionDecision or exit-code channel, a VS Code API behavior, an editor CLI flag — must name the host version and the date it was watched, and the adapter point must stay single and identifiable. Do not widen, silently re-target, or add a second emission channel for a host contract without recording the version it was verified against. An unversioned claim that a host accepts an output shape is unverified.", + "scope": ["hooks/**", "scripts/install.mjs", "extension/src/**", "src/index.ts", "docs/**"], + "severity": "high" + } + ] +} diff --git a/.greptile/files.json b/.greptile/files.json new file mode 100644 index 0000000..a4e787f --- /dev/null +++ b/.greptile/files.json @@ -0,0 +1,44 @@ +{ + "files": [ + { + "path": "docs/failure-modes.md", + "description": "The degradation contract. Enumerates every condition where evidence is incomplete, wrong, or absent and states the required behavior for each — missing artifact, unparseable JSON, structurally invalid shape, pathless event, reviewer unavailable, installer collisions. Every row is exercised by `npm run smoke` or `tests/unit/installer.test.ts`, or marked as guaranteed by the absence of a connecting code path. Use it to judge whether a change silently converts an unavailable state into an approval or a clean result." + }, + { + "path": "docs/operational-guarantees.md", + "description": "Seven guarantees, each cited to specific source and tests: missing evidence never becomes approval; malformed evidence never crashes the edit loop; reviewer output never controls deterministic enforcement; installation never overwrites unmanaged configuration; uninstall removes only owned artifacts; every VERIFIED claim maps to a reproducible command; the editor extension installs only with explicit consent. A change that weakens one of these is a boundary change, not an implementation detail." + }, + { + "path": "docs/workspace-contract.md", + "description": "The consumer boundary. Names exactly which workspace.json fields this repository reads (manual.fragileFiles, manual.coChangePatterns, generated.fileIndex, generated.frameworkManifest) and states that src/services/workspace.ts is the single place that touches raw file shape — everything downstream depends on the normalized model in src/types.ts, never the raw file. Establishes that this repository consumes the standard rather than defining it." + }, + { + "path": "src/path-match.ts", + "description": "THE path matcher for the server: the single definition of whether two paths denote the same file. Resolution is exact per ADR-006 §4/§8 with no suffix matching; `root` is a required parameter on purpose so an absolute query cannot silently degrade at a call site that forgot to pass one. Its header records why: a second, drifted matcher is how a deny silently became a warn, and the removed suffix fallback matched a stored key against a file in an unrelated repository (META-291)." + }, + { + "path": "extension/src/pathMatch.ts", + "description": "The VS Code extension's hand-ported copy of the same path-identity contract, expressed in two steps (relativeWorkspacePath proves containment, then pathsMatch is exact equality) rather than one. Ported and not imported because the extension is a standalone package with no build-time dependency on the server. This file and src/path-match.ts must be reconciled together; divergence between them is a defect, not a local edit." + }, + { + "path": "src/services/workspace.ts", + "description": "Artifact discovery and tolerant normalization. Contains the root-marker upward walk that never crosses a Git boundary, the central derivation of the proven repository root from the matched artifact candidate (not from a generic dirname, which would be one level short of `.agents/workspace.json`), and the single normalizer that touches raw file shape. Its contract is degrade-to-empty, never fabricate; producer-emitted tier and confidence values are dropped rather than trusted. No read path here may acquire a write." + }, + { + "path": "hooks/pre-edit-check.mjs", + "description": "The Codex PreToolUse hook and the repository's host-contract adapter point. Decision semantics: deny on an evidenced-fragile file with omitted co-change partners, warn on evidenced fragility or missing partners, never approve. Every failure routes through emitUnavailable, which states that no determination was made. The Codex output contract (permissionDecision / additionalContext JSON, exit code 2 = block) is recorded as watched live on Codex 0.144.1 (2026-07-13), and emitDecision() is deliberately the only place that speaks it. It resolves ../dist/* at runtime, so it depends on build output rather than on the source tree." + }, + { + "path": "package.json", + "description": "Defines what a consumer actually receives: the `files` allowlist (dist, hooks, scripts/install.mjs, .codex-plugin, .mcp.json, vsix, README.md, LICENSE), the two `bin` commands mapping to scripts/install.mjs, the `exports` map, and the check/verify/pack pipeline. dist/ is gitignored but shipped, so this file is the seam where a green source test can coexist with a broken published package." + }, + { + "path": "docs/verification.md", + "description": "Honest verification tiers for this repository's own claims — what the smoke suite actually drives versus what was watched live on a specific host version. Records the Codex 0.144.1 (2026-07-13) observation in detail and notes that the output contract and manifest remain isolated to single adapter points. The model for how a host-contract claim must be stated: named version, named date, named observation." + }, + { + "path": "AGENTS.md", + "description": "Working discipline for agents in this repository and in repositories that install it: check before you edit, absence is not approval, trust evidence rather than labels, and the reviewer is advisory and never overrides the deterministic hook decision. Useful as the plain-language statement of the postures the structured rules enforce." + } + ] +} diff --git a/.greptile/rules.md b/.greptile/rules.md new file mode 100644 index 0000000..83aab3e --- /dev/null +++ b/.greptile/rules.md @@ -0,0 +1,273 @@ +# Review rules for workspacejson/integrations + +This repository owns host adaptation, delivery, packaging, hooks, installers, and +first-party adapter parity for `workspace.json`. It **consumes** the public +contract; `workspacejson/standard` owns the schema, validation semantics, rules, +and ADRs. + +The rules below are enforced as structured rules in `config.json` and elaborated +here in prose so the reasoning is reviewable alongside the code. + +Every Integrations-specific rule names the **verified failure class** it was +derived from — an observable surface or a recorded defect in this repository, not +a rule inherited because another repository has one. Rules that could not be +grounded in this repository's own evidence were not written; see +[Deliberately not carried over](#deliberately-not-carried-over). + +## Why this repository's rules differ from `standard`'s + +`workspacejson/standard` is the normative surface: its risk is that the schema +says the wrong thing. This repository never says what is normative. Its risk is +different in two specific ways. + +1. **It reads an artifact it did not produce, on a host machine.** Path identity, + tolerance, and unavailability semantics are the failure surface — not schema + correctness. +2. **What ships is not the source tree.** `dist/` is gitignored, + `hooks/pre-edit-check.mjs` resolves `../dist/*` at runtime, and `package.json` + `files` decides what a consumer actually receives. Source-only tests can be + green while the packed package is broken. + +## Ecosystem-wide rules + +These carry across the `workspacejson` ecosystem. They are kept here because each +also has a live surface in this repository, not merely because `standard` has them. + +### Evidence must be load-bearing + +A verification check that cannot fail proves nothing. If you add a test, guard, +smoke assertion, or receipt comparison, it must be capable of failing for the +defect it names. + +### Absence is not success + +Absence, skipped, unsupported, or unavailable is never success, false, safe, +empty, allow, or green. A gate that goes green because it could not find the +thing it measures reports conformance it never measured. + +### Measurements must perturb + +A parity receipt, hash, count, or score that does not move when its referent +moves is decorative. `npm run verify:receipt` regenerates a receipt and compares +it against the committed reference; a comparison that cannot disagree is not a +comparison. + +### Checks cannot be vacuous + +Every check must be considered in both directions: cannot-ever-pass and +cannot-ever-fail. A hook that denies everything and a hook that allows everything +both look identical to a working hook from a green build. + +### Clean-room public boundary + +**Verified failure class.** This repository is public and clean-room by +construction — `src/services/workspace.ts` carries its own root-marker resolver +precisely because the private platform's resolver is not importable across that +boundary. The boundary is nonetheless porous in the direction of *references*: +private tracker identifiers and private platform names have reached source +comments in the published package's source tree. + +No private organizational tracker identifier (`VR-nnn`, `HAC-nnn`), private +product or platform name, private sidecar artifact, cross-organization +implementation dependency, or running-daemon assumption may appear in any file, +including source comments, docs, config, and fixtures. Public references +(`META-nnn`, `ADR-nnn`, published package names) are fine. The artifact and this +integration must remain meaningful with nothing running. + +> Pre-existing instances of this class exist on `main` and are recorded on +> META-322 for separate triage. They are not repaired by the policy PR — the rule +> governs changed lines going forward. + +## Integrations-specific rules + +### Consume the standard, do not vendor it + +**Verified failure class.** The dependency direction is currently intact and is +worth keeping that way: runtime dependencies are `@modelcontextprotocol/sdk` and +`zod` only, no schema is vendored, and `hooks/pre-edit-check.mjs` points users at +`npx @workspacejson/spec validate` rather than validating locally. `src/types.ts` +is deliberately a *consumer* model, not a schema. + +Do not add a copied JSON Schema, a parallel validator, a re-derived rules engine, +or a second definition of conformance here. Reading and normalizing the artifact +into a local consumer model is correct; asserting normative validity, or +repairing the artifact toward a schema, is `standard`'s job. If a contract gap +blocks work, the fix is an upstream issue, not a local reimplementation. + +Scope: `src/**`, `extension/src/**`, `hooks/**`, `scripts/**` + +### Reader tolerance is not repair + +**Verified failure class.** `normalizeWorkspace` in `src/services/workspace.ts` +is the single place that touches the raw artifact, and its contract is explicit: +*degrades to empty, never fabricates*. Producer-emitted tier/confidence fields are +dropped rather than trusted. The property that makes this safe is that no read +path can write: today the only writes anywhere in `src/` are reviewer receipts +under `.local/workspacejson/reviewer/`, and nothing opens `workspace.json` for +writing. + +Normalization may degrade to empty and may accept legacy shapes. It must not +fabricate values, invent defaults that are then presented as producer output, or +write back to the artifact. Tolerating an unknown shape is not declaring it valid. + +Scope: `src/services/**`, `src/tools/**`, `src/evidence.ts`, `hooks/**`, +`extension/src/**` + +### Path identity resolves against a proven root + +**Verified failure class.** META-291 / PR #8. The matcher previously carried a +suffix fallback that matched a stored `src/a.ts` against +`/elsewhere/unrelated-repo/src/a.ts` — a fragility assertion landing on a file in +a different repository. The fallback was removed and the behavior is now pinned by +`tests/unit/path-match.test.ts`, `tests/unit/workspace.test.ts`, +`tests/unit/evidence.test.ts`, and `scripts/smoke.mjs`. + +Path identity follows ratified ADR-006 §4/§8. An absolute host query is comparable +only after containment in a repository root the caller has **proven**; if +containment cannot be proven the answer is no-match, which is a refusal, never a +nearest guess. Do not introduce suffix matching, basename matching, `endsWith` +comparison, symmetric fuzzy matching, or an optional/defaulted `root` parameter +that lets an absolute query silently degrade at a call site that forgot to pass +one. Stored keys are repo-root-relative POSIX. + +The proven root is derived centrally from the matched artifact candidate, not from +a generic `dirname()` — the canonical location is `.agents/workspace.json`, so a +`dirname()` root is one level short and makes every legitimate absolute query fail +containment. + +Scope: `src/path-match.ts`, `src/services/**`, `src/tools/**`, `src/evidence.ts`, +`extension/src/pathMatch.ts`, `extension/src/**`, `hooks/**` + +### Ported copies must not diverge silently + +**Verified failure class.** `src/path-match.ts` records it directly: a second +matcher is how a deny silently became a warn — the enforcement layer had drifted +to a symmetric fuzzy suffix match while the read layer was tightened. The +duplication is still live and deliberate: `extension/src/pathMatch.ts` is ported +rather than imported because the extension is a standalone package with no +build-time dependency on the server. + +A change to the semantics of one copy without a corresponding change, or an +explicit written reconciliation, for the other is a divergence defect rather than +a local edit. The same applies to any other capability duplicated between the +server and a host adapter. + +Scope: `src/path-match.ts`, `extension/src/pathMatch.ts`, `src/**`, +`extension/src/**` + +### Unavailable is not approval + +**Verified failure class.** This repository already treats the distinction as +load-bearing, and the smoke suite exercises it against malformed JSON, a +wrong-typed root, and a removed artifact. `hooks/pre-edit-check.mjs` routes every +failure through `emitUnavailable`, which states that no fragility or co-change +determination was made, and its no-history branch exits silently with the comment +*never an approval message*. `src/reviewer.ts` keeps `UNAVAILABLE` a distinct +status from a `PASS` verdict, and instructs that PASS means no blocking issue was +found in scope, never a safety certification, and that missing evidence is a gap, +never approval. + +Malformed, absent, unreadable, or unsupported state must stay observable and +distinct, and must never be rendered as approval or as a clean result. Do not +collapse an error branch into the same output as a clean branch. + +Scope: `hooks/**`, `src/reviewer.ts`, `src/tools/**`, `src/services/**`, +`src/evidence.ts`, `extension/src/reviewerVerdict.ts`, `extension/src/**` + +### Host adaptation stays descriptive + +**Verified failure class.** The reviewer's own instructions pin the posture: *you +are read-only and have no enforcement authority*. The pre-edit hook does emit a +`deny`, and that is legitimate — it is this integration's host-side behavior over +described evidence, with tiers derived mechanically from evidence the artifact +already carries. The line to hold is that none of it flows back into the artifact +or gets attributed to the standard. + +Do not write policy, approval, gate, or merge-blocking fields into +`workspace.json`, and do not document the standard as mandating an action. Present +host enforcement as this integration's behavior, never as a requirement the +standard imposes. + +Scope: `src/**`, `extension/src/**`, `hooks/**`, `docs/**`, `README.md` + +### Packed artifact behavior must be measured + +**Verified failure class.** The gap between source tree and shipped package is +structural here. `dist/` is gitignored but shipped; `hooks/pre-edit-check.mjs` +resolves `../dist/services/workspace.js`, `../dist/evidence.js`, and +`../dist/config.js` at runtime, so the hook depends on a build output that no +source test exercises through the same resolution path. `package.json` `files` +enumerates `dist`, `hooks`, `scripts/install.mjs`, `.codex-plugin`, `.mcp.json`, +`vsix`, `README.md`, `LICENSE`, and `bin` maps two commands into +`scripts/install.mjs`. Any of those can break the consumer while the source tests +stay green. + +Changes to those surfaces need evidence from a pack, install, or equivalent +packaged run — `npm run pack:check` and the installed-hook path, not only +`vitest`. A source-tree test passing is not evidence that a consumer receives +working files. + +Scope: `package.json`, `extension/package.json`, `scripts/install.mjs`, +`hooks/**`, `.github/workflows/**`, `tests/**` + +### Probes must not be destructive + +**Verified failure class.** META-285, recorded against this repository: the +candidate-consumption harness has checks that cannot pass, and `--help` runs a +destructive install into the source repository. The surrounding surface is +genuinely destructive by nature — `scripts/install.mjs` invokes +`--install-extension` and `--uninstall-extension` and removes a managed root; +`scripts/smoke.mjs` writes and `rmSync`s fixture roots — so the distinction +between *a probe* and *an action* has to be explicit rather than assumed. + +A command presented as harmless — help, version, status, check, probe, dry run, +smoke — must not install or uninstall anything, must not delete or overwrite files +outside a disposable temporary directory it created, and must not mutate the +user's checkout or the source repository under test. If a check needs a real +install to be meaningful, it must be explicitly opt-in and say so. + +> META-285 owns the repair of the existing harness. This rule governs changed +> lines; it is not a request to fix that defect in an unrelated PR. + +Scope: `scripts/**`, `hooks/**`, `.github/workflows/**` + +### Host contract claims need a watched version + +**Verified failure class.** The one host output contract this repository depends +on is recorded with an explicit evidence tier: +`hooks/pre-edit-check.mjs` states that the Codex output contract +(`permissionDecision` / `additionalContext` JSON, exit code 2 = block) was +*watched live on Codex 0.144.1 (2026-07-13)*, with deny-all and fixture-specific +denies observed blocking `apply_patch`, and names `emitDecision()` as the single +adapter point. That is the standard to hold, because a host contract is an +observed fact about a version, not a stable API. + +A claim that a host contract works — a Codex hook channel, a VS Code API behavior, +an editor CLI flag — must name the host version and the date it was watched, and +the adapter point must stay single and identifiable. Do not widen, silently +re-target, or add a second emission channel for a host contract without recording +the version it was verified against. + +Scope: `hooks/**`, `scripts/install.mjs`, `extension/src/**`, `src/index.ts`, +`docs/**` + +## Deliberately not carried over + +Recorded so that a later reader can tell a considered omission from an oversight. + +| Rule in another repository | Status here | Reason | +| --- | --- | --- | +| `four-read-paths-breaking` (standard) | **Not carried** | The four stable read paths are `standard`'s compatibility surface to define. This repository consumes them; `normalizeWorkspace` already reads only those four and says so. A rule here would assert ownership this repository does not have. | +| `negative-fixtures-single-defect` (standard) | **Not carried** | No normative negative-fixture corpus exists in this repository. Encoding it would be a rule with no surface. | +| `no-derived-probability` (standard) | **Not carried** | Emission-shape governance belongs to the producer/standard side. This repository reads `strength`/`confidence` tolerantly and emits no artifact. | +| `descriptive-not-prescriptive` (standard, scoped to `packages/spec/**`) | **Adapted, not copied** | Kept as *host adaptation stays descriptive*, rewritten for a repository that legitimately performs host-side enforcement while still never making the artifact prescriptive. | +| `daemon-free` (standard) | **Folded in** | Merged into *clean-room public boundary* rather than standing alone: here the daemon risk arrives as a private-platform reference, not as an artifact field. | +| `reader-producer-distinct` (standard) | **Adapted, not copied** | This repository has no producer. The live half of the concern — tolerance quietly becoming repair — is kept as *reader tolerance is not repair*. | +| CLI producer / commit-history rules | **Not carried** | This repository emits no `workspace.json` and reads no commit graph. No equivalent Integrations failure mode was independently verified, so nothing was encoded. | + +## Reviewer roles + +`Greptile Review` and `Sourcery` are separate layers with different authority. +See [`docs/review/merge-policy.md`](../docs/review/merge-policy.md) for the +current, measured merge-eligibility contract — including which checks are +required today and what evidence would be needed to change that. diff --git a/.sourcery.yaml b/.sourcery.yaml new file mode 100644 index 0000000..09ac13b --- /dev/null +++ b/.sourcery.yaml @@ -0,0 +1,18 @@ +# Sourcery configuration for workspacejson/integrations. +# +# Scope only. This file deliberately grants Sourcery no semantic rule authority: +# on this repository Sourcery is defense in depth, a second review-completion +# layer, and a `success` check from it is never evidence of semantic approval. +# See docs/review/merge-policy.md §1. Semantic review rules are repo-owned and +# live in .greptile/. +# +# The ignore list is build output and vendored trees only — nothing that would +# hide a reviewable source surface. +version: '1' + +ignore: + - node_modules + - extension/node_modules + - dist + - vsix + - .parity-ci diff --git a/config/repository-structure.json b/config/repository-structure.json index 69c5020..8e5fa58 100644 --- a/config/repository-structure.json +++ b/config/repository-structure.json @@ -13,10 +13,12 @@ "biome.json", ".gitignore", ".mcp.json", - ".nvmrc" + ".nvmrc", + ".sourcery.yaml" ], "allowedRootDirectories": [ ".agents", + ".greptile", ".codex", ".codex-plugin", ".github", diff --git a/docs/review/calibration-2026-08.md b/docs/review/calibration-2026-08.md new file mode 100644 index 0000000..a28e1c9 --- /dev/null +++ b/docs/review/calibration-2026-08.md @@ -0,0 +1,102 @@ +# Greptile calibration — `workspacejson/integrations`, August 2026 + +Owner: META-322. Policy under test: [`.greptile/`](../../.greptile/). Merge +contract: [`merge-policy.md`](merge-policy.md). + +This is the evidence record for whether `Greptile Review` may become a required +merge gate on this repository. It exists because installing a reviewer and seeing a +green check is not calibration — a reviewer that does not catch the defect it was +configured to catch would, as a required gate, block merges without adding +detection. + +Results are filled in from observation. Anything not observed is recorded as +**not measured**, never as a pass. + +## Baseline (measured before any policy landed) + +Repository at `a31242b`, read from the GitHub API on 2026-08-12. + +| Fact | Observed | +| --- | --- | +| `.greptile/` on `main` | absent | +| `Greptile` check ever observed on this repository | never — PR #8 (merged 2026-08-10) and PR #6 (open) both show no Greptile check run | +| Reviewer apps actually observed | `sourcery-ai[bot]`, `copilot-pull-request-reviewer[bot]` | +| `main` head check runs | 4, all `github-actions`: `build-and-smoke (20)`, `build-and-smoke (22)`, `parity-receipt-reproduction`, `standard-candidate-consumption` | +| Required contexts | `build-and-smoke (20)`, `build-and-smoke (22)` | +| `required_conversation_resolution` | `true` | + +Greptile's absence on PR #8 and PR #6 is **not** evidence that the app is +uninstalled: both heads predate the 2026-08-11 rollout in `workspacejson/standard`. +Installation state was unmeasured going in, and the first controlled PR head is the +measurement. + +Local gates on the policy branch before any canary, for comparison against the +canary head: + +| Gate | Result | +| --- | --- | +| `npm run check:structure` | pass (after the allowlist amendment; watched failing with 2 violations before it) | +| `npm run typecheck` / `lint` / `build` | pass | +| `npm run test` | 147/147 passed, 14/14 files | +| `npm run smoke` | 43 PASS, 0 FAIL | + +## Why this canary defect + +The positive control reintroduces a **symmetric suffix fallback** into +`extension/src/pathMatch.ts`. + +It was chosen because it is the one defect class this repository has already +suffered and fixed, so it is a real failure mode rather than a synthetic one: + +* META-291 / PR #8 removed exactly this fallback from the server matcher, where a + stored `src/a.ts` matched `/elsewhere/unrelated-repo/src/a.ts` — a fragility + assertion landing on a file in a different repository. +* `src/path-match.ts` records that a *second* matcher is how a deny silently became + a warn: the enforcement layer drifted to a symmetric fuzzy suffix match while the + read layer was tightened. + +It violates two branch-local custom rules at once — `proven-root-path-identity` +and `ported-copies-must-not-diverge` — so a finding that names either one is +evidence that branch-local configuration was read, not that a generic reviewer +noticed loose code. + +### It is deliberately invisible to every deterministic gate + +This is the property that makes it a clean control. On the canary head, CI should +stay green while the defect is present: + +* `tsconfig.json` has `"include": ["src/**/*"]`, so `npm run build` and + `npm run typecheck` never see `extension/src/`. +* `npm run test` is `vitest`, which collects `tests/**` only; `.check.ts` files do + not match its default patterns. +* CI never runs `npm --prefix extension run test`. The only job that touches the + extension is `standard-candidate-consumption` via `npm run build:extension`, + which compiles and packages it — a suffix fallback is type-valid and compiles. +* No test covering `extension/src/pathMatch.ts` exists in `extension/test/`. + +So if the deterministic gates stay green and the reviewer catches it, semantic +review is demonstrably adding detection that CI cannot provide. If the +deterministic gates stay green and the reviewer misses it, that is the reportable +outcome, and it argues against making the check required. + +The change is confined to a disposable canary branch, is reverted in place, and is +never merged. + +## Protocol + +1. Open the policy PR. Measure whether a Greptile check appears at all, and against + which head SHA. +2. Branch the canary from the policy head so the branch carries `.greptile/`. +3. Push the defect. Record caught / not-caught, and which rule id was cited. +4. Revert the defect in a new commit. Record whether the finding repeats on the + clean head. +5. Record whether the new push retriggered review against the new SHA. +6. Reconcile each actionable finding individually, in its own thread. +7. Decide branch protection only from what steps 1–6 actually showed. +8. Close the canary without merging. + +## Results + +**Status: not yet measured.** This section is completed from observation on the +canary PR; until then nothing here should be read as a pass, and `Greptile Review` +stays out of the required-contexts list. diff --git a/docs/review/merge-policy.md b/docs/review/merge-policy.md new file mode 100644 index 0000000..68cb143 --- /dev/null +++ b/docs/review/merge-policy.md @@ -0,0 +1,131 @@ +# Review and merge policy + +This document is the repository-owned statement of what has to be true before a +change merges into `main`, and — just as importantly — what is currently +**measured** versus **assumed**. + +Every claim here is either a value read back from the GitHub API or an observation +from a named pull request head. Where something has not been measured, this +document says so rather than inferring it. + +Owner: META-322. Companion policy: [`.greptile/rules.md`](../../.greptile/rules.md). + +## 1. Reviewer roles + +The two automated reviewers on this repository are not interchangeable and do not +carry the same authority. + +| Layer | Role | Authority | +| --- | --- | --- | +| **CI** (`.github/workflows/ci.yml`) | Deterministic build, test, smoke, parity, and candidate-consumption gates | Blocking, for the contexts listed in §2 | +| **Greptile Review** | Semantic review against the repo-owned rules in `.greptile/` | See §4 — governed by measured calibration, not by installation | +| **Sourcery** | Defense in depth. A second, independent review-completion layer | **Not semantic authority.** A `success` Sourcery check is not evidence that a change is semantically correct, and must never be cited as review approval | +| **Human review** | Reconciliation of actionable findings; final judgement | Authoritative | + +### Why Sourcery is not authority here + +`Sourcery review` has been observed reporting `pass` on this repository (PR #8) in +the same run where a human reviewer subsequently commented on the change. A check +that completes tells you a reviewer *ran*; it does not tell you a defect class was +*examined*. That distinction is the whole point of separating the layers, and it is +the same conclusion `workspacejson/standard` reached. This repository keeps it +unless an Integrations-specific calibration independently falsifies it. + +Nothing in this document elevates Sourcery on the basis of its check state. + +### Repo-owned reviewer configuration + +* `.greptile/config.json` — rules, scopes, severities, trigger behavior +* `.greptile/rules.md` — the prose reasoning and the verified failure class behind + each rule, plus the rules deliberately **not** carried over from other repositories +* `.greptile/files.json` — orientation files the reviewer should read first +* `.sourcery.yaml` — path scoping only. It deliberately grants Sourcery no rule + authority; it exists so the second layer does not spend attention on build + output and vendored trees + +## 2. Measured merge eligibility + +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)` | +| `strict` (branch must be up to date) | `true` | +| `required_conversation_resolution` | `true` | +| `required_approving_review_count` | `0` | +| `dismiss_stale_reviews` | `true` | +| `enforce_admins` | `false` | +| `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. + +### Observed gap — recorded, not acted on here + +CI produces four check runs, but only two are required: + +| Check run | Required? | +| --- | --- | +| `build-and-smoke (20)` | yes | +| `build-and-smoke (22)` | yes | +| `parity-receipt-reproduction` | **no** | +| `standard-candidate-consumption` | **no** | + +A failure in either unrequired job does not block a merge today. That is a separate +governance decision from the reviewer question this document's owner issue covers, +and it is recorded here so it is not mistaken for a setting someone already chose +deliberately. Changing it is out of scope for META-322. + +## 3. Conversation resolution + +`required_conversation_resolution` is already enabled, which makes unresolved +actionable threads part of merge eligibility on this repository today. + +The protocol: + +* **Reconcile findings individually.** Each actionable thread gets its own reply + stating what was changed, or why the finding does not hold. Bulk-resolving a set + of threads destroys the receipt and is not evidence of reconciliation. +* **Resolution is a claim.** Resolving a thread asserts that the finding was + addressed on the current head — not that it was read. +* **Disagreement is a legitimate resolution**, provided the reasoning is written in + the thread. "Not applicable because X" is a receipt; silent resolution is not. + +## 4. `Greptile Review` as a required check + +**Current state: not required. Not eligible to be required on installation +evidence alone.** + +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: + +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 + the abstract. +3. A deliberate, Integrations-specific positive-control defect is **caught** by a + custom rule from `.greptile/config.json`. +4. The reverted, non-violating head does **not** repeat that finding. +5. A further push **retriggers** review against the new head. +6. Actionable findings are reconcilable individually, with thread receipts intact. +7. Merge eligibility then behaves as claimed. + +If the positive control is **missed**, that is a reportable result and a reason not +to make the check required — a reviewer that does not catch the defect it was +configured to catch would, as a required gate, block merges without adding +detection. Installing an app and seeing a green check is not calibration. + +Calibration results for this repository are recorded in +[`calibration-2026-08.md`](calibration-2026-08.md). + +## 5. What a check state does and does not mean + +* A check that has **not run** is not a pass. Absence of a review is absence of + evidence, in exactly the sense `.greptile/rules.md` requires of the product code. +* A **completed** review is not an approval. Completion means a reviewer finished; + it says nothing about whether it examined the defect class you care about. +* A check green on an **older head** says nothing about the current head. Review + state is a property of a SHA. +* A reviewer being **installed** is not a reviewer being **calibrated**. From bd52289c864a755090ecf82dad780090d12f777e Mon Sep 17 00:00:00 2001 From: Qwynn Marcelle Date: Wed, 12 Aug 2026 09:42:35 -0400 Subject: [PATCH 2/2] docs(review): record calibration results and reconcile review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Calibration ran on PR #11 (canary, closed unmerged) against this policy. All seven criteria observed. Greptile caught the positive control (P1 on extension/src/pathMatch.ts at e22d6f5) citing the branch-local rule by name, stayed silent on the reverted clean head (447a318, 0 comments), and retriggered on push. Merge eligibility behaved as claimed: PR #10 sat at BLOCKED on one unresolved thread. Two results qualify how a required check should be read, and are recorded rather than smoothed over: - the Greptile check concluded success on the head carrying the P1, so the check encodes review completion, not absence of findings; - GitHub auto-resolved the canary thread when the revert removed its lines, so required_conversation_resolution can be satisfied without anyone answering a finding. Decision: Greptile Review is eligible as a required current-head gate. The required-contexts mutation is deliberately not applied here — PR #6 is BEHIND and predates the policy, so the change costs a PR this issue does not own. Reconciles review findings on #10 individually: - Greptile P2: corrected a false coverage claim. extension/test/intelligence.check.ts does exercise pathsMatch; the real limitation is that no gate runs that suite. Chasing it down found the suite is already red on main against pre-META-291 assertions — filed as META-329, not repaired here. - Sourcery 1: audited rules.md/config.json scope parity, all nine scoped rules match; config.json declared authoritative for scope so drift has a resolution direction. - Sourcery 2: added a refresh procedure and measurement stamp to the branch protection section, which transcribes settings that can change externally. Refs META-322, META-329 --- .greptile/rules.md | 7 ++ docs/review/calibration-2026-08.md | 112 +++++++++++++++++++++++++++-- docs/review/merge-policy.md | 35 ++++++++- 3 files changed, 147 insertions(+), 7 deletions(-) diff --git a/.greptile/rules.md b/.greptile/rules.md index 83aab3e..2eef701 100644 --- a/.greptile/rules.md +++ b/.greptile/rules.md @@ -8,6 +8,13 @@ and ADRs. The rules below are enforced as structured rules in `config.json` and elaborated here in prose so the reasoning is reviewable alongside the code. +> **`config.json` is authoritative for scope.** The `Scope:` line under each rule +> here restates that rule's `scope` array so the reasoning is readable in one +> place, which means the two can drift. If they ever disagree, `config.json` is +> what the reviewer actually applies and this file is the thing to correct. As of +> the commit that introduced them, all nine scoped rules match exactly, and the +> five ecosystem-wide rules carry no `scope` key and no `Scope:` line. + Every Integrations-specific rule names the **verified failure class** it was derived from — an observable surface or a recorded defect in this repository, not a rule inherited because another repository has one. Rules that could not be diff --git a/docs/review/calibration-2026-08.md b/docs/review/calibration-2026-08.md index a28e1c9..29075b8 100644 --- a/docs/review/calibration-2026-08.md +++ b/docs/review/calibration-2026-08.md @@ -72,7 +72,11 @@ stay green while the defect is present: * CI never runs `npm --prefix extension run test`. The only job that touches the extension is `standard-candidate-consumption` via `npm run build:extension`, which compiles and packages it — a suffix fallback is type-valid and compiles. -* No test covering `extension/src/pathMatch.ts` exists in `extension/test/`. +* Coverage for `extension/src/pathMatch.ts` **does** exist — + `extension/test/intelligence.check.ts` imports `pathsMatch` directly — but that + suite is excluded from every deterministic gate. Absent coverage and existing + coverage that no gate runs are different failures, and this record distinguishes + them. See [Incidental finding](#incidental-finding-the-extension-suite-is-red-on-main-and-no-gate-reports-it). So if the deterministic gates stay green and the reviewer catches it, semantic review is demonstrably adding detection that CI cannot provide. If the @@ -97,6 +101,106 @@ never merged. ## Results -**Status: not yet measured.** This section is completed from observation on the -canary PR; until then nothing here should be read as a pass, and `Greptile Review` -stays out of the required-contexts list. +Measured 2026-08-12 on PR #10 (policy) and PR #11 (canary, closed unmerged). + +| # | Criterion | Result | Evidence | +| --- | --- | --- | --- | +| 1 | Branch-local `.greptile/` is read | **pass** | The canary finding cites `Rule Used: Path identity follows ratified ADR-006 §4/§8…` with `source: .greptile`. That rule exists only on these branches, never on `main` | +| 2 | Review is associated with the exact current head | **pass** | Separate `Greptile Review` check runs per SHA: `aa456b4`, `e22d6f5`, `447a318`; the review comment carries `commit_id=e22d6f5` | +| 3 | Positive control caught | **pass — 1 of 1** | P1 *"Suffix matching breaks path identity"* on `extension/src/pathMatch.ts`, head `e22d6f5`. Summary: `8 files reviewed, 1 comments added` | +| 4 | Reverted clean head does not repeat | **pass — 0 of 0** | Head `447a318`, summary: `7 files reviewed, 0 comments added` | +| 5 | A further push retriggers review | **pass** | A new `Greptile Review` check run appeared on `447a318` after the revert push and ran to completion | +| 6 | Findings reconcilable individually | **pass, with a caveat** | Each finding answered in its own thread. Caveat in *Conversation resolution is weaker than it looks* below | +| 7 | Merge eligibility behaves as claimed | **pass** | PR #10 `mergeStateStatus=BLOCKED` on one unresolved thread; PR #11 `CLEAN` once its thread resolved. `required_conversation_resolution` is load-bearing | + +Reviewer output on the two canary heads: + +| Head | Defect present | Greptile | Sourcery check | CI (4 Actions contexts) | +| --- | --- | --- | --- | --- | +| `e22d6f5` | yes | **1 comment, P1** | `success` | 4/4 success | +| `447a318` | no (reverted) | 0 comments | `success` | 4/4 success | + +### The check state does not encode findings + +`Greptile Review` concluded **`success` on `e22d6f5`** — the head carrying the P1 +finding. The check reports that review completed, not that review found nothing. + +This decides what requiring the context would actually buy: it enforces *a review +ran against this head*, and nothing more. The semantic gate is +`required_conversation_resolution`, which was already enabled, and which was +observed holding PR #10 at `BLOCKED` on a single unresolved thread. + +### Conversation resolution is weaker than it looks + +When the revert removed the offending lines, GitHub marked the canary thread +`isOutdated=true, isResolved=true` **automatically**, with no one replying to it. +So a required-conversation-resolution gate can be satisfied by deleting the code a +finding pointed at. + +It remains load-bearing — an unresolved thread genuinely blocks — but *resolved* is +not by itself evidence that a human engaged with a finding. Bulk resolution and +auto-resolution look identical in the API. Receipts have to be written into the +thread, which is why each finding in this calibration was answered individually +before resolution. + +### Sourcery + +Sourcery's **review body independently named the same defect** on `e22d6f5`, +including the cross-repo false-match consequence and the divergence from +`src/path-match.ts` — while its **check concluded `success`**. + +That is the cleanest possible illustration of why the two are separated: the text +carried real semantic signal, the check state carried none. Sourcery therefore +stays defense in depth. The prior conclusion is **not falsified**; if anything its +value as a second layer is better supported than before, and its check state is +confirmed as non-authoritative. + +### Incidental finding — the extension suite is red on `main` and no gate reports it + +Greptile's one comment on the **policy** PR corrected a factual error in an earlier +draft of this document, which claimed no test covered `extension/src/pathMatch.ts`. +Coverage exists — `extension/test/intelligence.check.ts` imports `pathsMatch` +directly — and chasing that down surfaced a real defect: + +``` +$ npm --prefix extension ci && npm --prefix extension test # clean head aa456b4 +not ok 21 - pathsMatch: exact match and absolute-suffix fallback only +# tests 82 # pass 81 # fail 1 +``` + +| Assertion | `aa456b4` (clean) | `e22d6f5` (defect) | +| --- | --- | --- | +| `pathsMatch("src/a.ts","src/a.ts") === true` | ok | ok | +| `pathsMatch("/repo/src/a.ts","src/a.ts") === true` | **FAIL** (`false`) | ok | +| `pathsMatch("/other/a.ts","a.ts") === false` | ok | **FAIL** (`true`) | + +The test still asserts the pre-META-291 absolute-suffix fallback, so it is red on +both heads for opposite reasons, and no gate runs it. Tracked as **META-329**, not +repaired here. + +## Decision + +`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. + +Recommended change — **not applied by META-322**, because it has a side effect on +another stream's open PR: + +``` +required_status_checks.contexts += "Greptile Review" +``` + +Two things must be understood by whoever applies it: + +* 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. + +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 68cb143..238b9ee 100644 --- a/docs/review/merge-policy.md +++ b/docs/review/merge-policy.md @@ -78,6 +78,24 @@ governance decision from the reviewer question this document's owner issue cover and it is recorded here so it is not mistaken for a setting someone already chose deliberately. Changing it is out of scope for META-322. +### Refreshing this section + +Everything in the two tables above is a live GitHub setting that can be changed +outside this repository, which makes the section the most perishable part of this +document. Re-read it rather than trusting it whenever it matters: + +```bash +gh api repos/workspacejson/integrations/branches/main/protection +gh api repos/workspacejson/integrations/rulesets +``` + +If the output disagrees with what is written here, the API is right and this file +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`. + ## 3. Conversation resolution `required_conversation_resolution` is already enabled, which makes unresolved @@ -95,8 +113,9 @@ The protocol: ## 4. `Greptile Review` as a required check -**Current state: not required. Not eligible to be required on installation -evidence alone.** +**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.** 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 — @@ -117,9 +136,19 @@ to make the check required — a reviewer that does not catch the defect it was configured to catch would, as a required gate, block merges without adding detection. Installing an app and seeing a green check is not calibration. -Calibration results for this repository are recorded in +All seven were observed on 2026-08-12; results, exact SHAs, and check states are in [`calibration-2026-08.md`](calibration-2026-08.md). +**One result changes how the requirement should be read.** The `Greptile Review` +check concluded `success` on the head that carried a P1 finding. The check encodes +*review completed*, not *review found nothing*. Requiring the context therefore buys +current-head review completion; the semantic gate is `required_conversation_resolution`, +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. + ## 5. What a check state does and does not mean * A check that has **not run** is not a pass. Absence of a review is absence of