tooling(review): repo-owned Greptile review policy for the CLI producer (META-321) - #22
Conversation
… failure classes (META-321) The standard's rollout pattern is proven, but its rules are about a schema surface this repository does not own. Copying them would have installed a gate that fires on things the CLI cannot do and stays silent on the things it can. So the rules here are derived from defects this producer has actually shipped or nearly shipped. The load-bearing one is the P1 Greptile found on PR #20: an explicit `mineHistory: true` request could fall back to stale preserved history and return a successful-looking result, with the refusal reason computed and then discarded. From the artifact alone that is indistinguishable from a refresh that completed. Every CLI-specific rule is a specialization of that one failure class — a producer emitting a plausible artifact for evidence it did not gather: refusal reported as success, an empty coChange block for a repository that could not be analyzed, a basis pin advanced without recounting, a preserved block rebuilt rather than passed through, UTF-16 ordering substituted for the UTF-8 byte order two producers are compared on. Five ecosystem rules are carried because they are genuinely repository-independent. Seven standard rules are deliberately not carried, and `.greptile/rules.md` records which and why, so the omissions read as decisions rather than oversights — four of them survive in producer form, stated from the emitting side rather than the schema-authoring side. `REVIEW.md` states what the checks mean. Check completion is not semantic approval: the status check answers "did review complete on this head", and conversation resolution answers "was every actionable finding dealt with". Greptile Review is deliberately NOT made a required status here. A rule that has not been shown to fail for the defect it names is not eligible to be a hard gate, and installing one before calibration would be the same defect these rules exist to catch in the producer. That decision follows the canary, on evidence. Sourcery stays defense-in-depth. A check run existing on recent heads is not calibration. No release impact: every changed path is outside `packages/`, each publishable manifest's `files` field lists only directory-local entries, and no workflow copies root files into a package before pack. No published bytes, exports, schema, or package metadata change, so no changeset accompanies this.
Reviewer's GuideIntroduces a repo-owned Greptile review policy for workspacejson/cli by adding semantic review rules, defining the review/merge contract, and wiring these into contributor docs and Greptile configuration, without touching producer or release behavior. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider aligning
.greptile/rules.mdmore tightly with the machine-enforced config by referencing concrete rule IDs or config keys in the prose, so changes toconfig.jsoncannot silently drift from the documented semantics. - The scope lists in
.greptile/rules.mdand the paths called out inREVIEW.mdandCONTRIBUTING.mdshould be periodically checked against the actual directory layout (e.g., if files move or new producer surfaces are added) to avoid rules accidentally no longer covering the intended code.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider aligning `.greptile/rules.md` more tightly with the machine-enforced config by referencing concrete rule IDs or config keys in the prose, so changes to `config.json` cannot silently drift from the documented semantics.
- The scope lists in `.greptile/rules.md` and the paths called out in `REVIEW.md` and `CONTRIBUTING.md` should be periodically checked against the actual directory layout (e.g., if files move or new producer surfaces are added) to avoid rules accidentally no longer covering the intended code.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
| Filename | Overview |
|---|---|
| .greptile/config.json | Adds structured Greptile configuration and repository-specific review rules. |
| .greptile/rules.md | Documents producer-focused semantic, evidence-integrity, and architecture rules. |
| REVIEW.md | Defines current-head review evidence, finding reconciliation, and merge-check semantics. |
| scripts/check-architecture.mjs | Adds exact exemptions for the two self-referential policy files while retaining scanning of other policy content. |
| scripts/check-architecture.test.mjs | Adds regression coverage proving the .greptile exemption is file-specific rather than directory-wide. |
Reviews (2): Last reviewed commit: "fix(review): let the clean-room policy n..." | Re-trigger Greptile
…TA-321) The policy files tripped the guard they describe. `check:architecture` reported four clean-room violations on `.greptile/config.json` and `.greptile/rules.md`, both for `@marcelle-labs/` and `workspace.vreko.json` — the vocabulary the rule has to name to be enforceable at all. Greptile flagged this itself on PR #22 as a P1, citing the very rule the files had just introduced, which is the first evidence that branch-local configuration is read. There were two ways out and only one of them is honest. Paraphrasing the scopes out of the policy would turn the build green while leaving the reviewer without the strings it matches on — adjusting the measurement to fit the behavior, which is the move `.greptile/rules.md` explicitly prohibits. So the two files join `SELF_REFERENTIAL`, which already holds OWNERSHIP.md, CONTRIBUTING.md, README.md and the guard's own source for exactly this reason, and whose comment already anticipated "the ownership documentation that explains the rules". Membership is by exact path, not directory prefix. That distinction is invisible from a passing run — "two files are exempt" and "the whole directory is exempt" produce identical green — so it is asserted rather than commented: a new red case writes `@marcelle-labs/` into `.greptile/notes.md` and requires the guard to still reject it. Injecting the directory-wide mistake takes the suite to 20 passed / 1 failed; restoring it returns 21 / 0. REVIEW.md is deliberately NOT exempted. It does not name the forbidden vocabulary, and exempting a file against a hypothetical future edit would give up real coverage for nothing. Waived coverage, stated rather than glossed: `SELF_REFERENTIAL` gates four checks. For `rules.md` the other three cannot apply — `copied-schema` is JSON-only, `shadowed-standard-types` is `.d.ts`-only, `neutral-producer-purity` is scoped to `packages/cli/`. For `config.json` the one additional waiver is `copied-schema`, which needs `$schema` + `$id` + `type: "object"` + `properties`; the Greptile config has none of them.
…eck status does not (META-321) (#24) Canary PR #23 answered the question META-321 actually asked, and the answer is not the one the staged rollout was set up to expect. The rules caught 3/3 positive controls, each citing the rule by name and each sourced from `.greptile`, which also proves branch-local configuration is read. Typecheck was clean on all three defects, so none of them is visible to the compiler — the semantic layer earned its place. The check STATUS is a different question, and it fails in both directions: - cannot-ever-fail: on PR #22 the reviewer posted a P1 at 13:30:16 and the Greptile Review check concluded success at 13:30:18; - cannot-ever-pass: on the canary's revert head, whose diff nets to empty against main, no Greptile Review check run was posted at all. A required context that goes green with an open P1 and can also never appear would neither block a bad head nor clear a good one. That is the vacuous-check shape .greptile/rules.md prohibits, so it is not installed. Branch protection is unchanged: test (20), test (22), Compatibility parity vs frozen source, strict, required conversation resolution. What does enforce review was measured rather than assumed. With the rollup at SUCCESS and the branch MERGEABLE, PR #22 sat at BLOCKED on one unresolved thread and moved to CLEAN when that thread resolved, head and rollup unchanged. Conversation resolution is the load-bearing mechanism; Greptile produces the findings it counts. Sourcery keeps its non-required status on new evidence rather than inheritance: it concluded success on the head carrying all three deliberate defects. No release impact: REVIEW.md is outside packages/, no published bytes change, no changeset.
Closes the policy half of META-321. Calibration and the required-check decision follow on the canary, in a second PR.
What this is
A repo-owned Greptile policy (
.greptile/config.json,.greptile/rules.md,.greptile/files.json) plusREVIEW.md, the review/merge contract.The rules are written for this repository, not copied from
workspacejson/standard. The standard's rules govern a schema surface the CLI does not own; carried across wholesale they would fire on things this repository cannot do and stay silent on the things it can.Where the rules come from
Every CLI-specific rule is a specialization of one defect: a producer emitting a plausible-looking artifact for evidence it did not gather.
The concrete precedent is the P1 Greptile found on PR #20 — an explicit
mineHistory: truerequest could fall back to stale preserved history and return a successful-looking result, with the refusal reason computed and then thrown away. From the artifact alone that is indistinguishable from a refresh that completed. That is why the CLI needs its own semantic vocabulary rather than a generic config.15 CLI-specific rules cover: refresh-refusal observability, ordinary generation never mining,
basisRevisionnever advancing without recomputation, incomplete history emitting nothing rather than an empty block, canonical UTF-8 endpoint ordering, byte-for-byte carry-forward, validator non-bypass, the exact registry dependency boundary, candidate substitution detection, the frozenagents-auditbridge, no derived/prescriptive emission, reader-tolerance not relaxing producer obligations, deterministic artifact bytes, daemon-free output, and no local schema redefinition.5 ecosystem rules are carried because they are genuinely repository-independent: load-bearing evidence, absence-is-not-success, metrics-perturb, bidirectional checks, clean-room boundary.
7 standard rules are deliberately not carried.
.greptile/rules.mdends with a table naming each and why — four survive in producer form (stated from the emitting side rather than the schema-authoring side), three have no equivalent surface here. The omissions are recorded so they read as decisions.What
REVIEW.mdsettlesGreptile Reviewcheck means review ran on that head. It is not approval. The status check and conversation resolution answer different questions and are not interchangeable.Sourcery reviewcheck run existing on recent heads is not calibration evidence.What this deliberately does not do
Greptile Reviewis not made a required branch-protection status in this PR. A rule that has not been shown to fail for the defect it names is not eligible to be a hard gate — installing one before calibration would report conformance it never measured, which is the same defect these rules exist to catch in the producer.Branch protection on
mainis untouched here. Current required contexts remaintest (20),test (22),Compatibility parity vs frozen source.Release impact — measured, not assumed
None. No changeset.
packages/.filesfield lists only directory-local entries (dist,README.md,LICENSE,CHANGELOG.md) — nothing escapes the package directory..github/workflows/*.yml).No published bytes, public exports, schema behavior, or package metadata change.
Scope
Producer behavior, history/co-change semantics (META-297/META-310), release workflows, and
NPM_TOKENauthority are all untouched. No publish of any kind.Summary by Sourcery
Introduce a repo-owned Greptile review policy and contract for workspacejson/cli without changing any producer or release behavior.
Enhancements:
Documentation: