Repository navigation
feat(policy): the review record reader and its mapping onto merge evidence (#102, part 1) - #107
Merged
Merged
Conversation
The merge driver refuses every merge until a verdict exists for the head, but nothing yet recorded what a reviewer actually concluded. A model's claim that it reviewed something is not a record of a review. .skein/review.json is that record, read through a closed schema so an unknown key is refused rather than dropped. Each refusal names its own reason token, keeping "nobody has reviewed yet" distinguishable from "a review that does not count" instead of collapsing both into no verdict. The head SHA carries the weight. A verdict is refused unless it names the SHA it covers, so a stale LGTM cannot cover a new head, and a verdict with no SHA attached cannot read as an approval. The mapper stamps the review SHA from headSHA, leaves a record with no verdict as no verdict rather than a pass, and leaves gates and CI to the driver, which measures them against a SHA. Not yet wired into the loop or merge driver; this only establishes the record and the mapping. Each of the five guards was mutated in turn and observed to fail exactly the test that owns it.
Four changes from the independent read. Drop reviewedSHA. The spec says an approval applies to "the SHA it recorded", and that SHA is headSHA — a review is made for one head. A second field naming the same commit is an invariant that can only ever agree with the first, and the driver already refuses any record whose headSHA is not the head it is merging. It protected against a state that cannot arise, at the cost of a rule. Carry independence onto the evidence. It was being read and discarded, so a same-model review mapped to exactly the same passing verdict as an independent one and "the reviewer differs from the author" was unenforceable. It is now an optional field on MergeEvidence.reviewVerdict, mapped but not yet enforced: the policy knob that decides whether same-model suffices comes later, and it cannot be decided from data that was not recorded. Require reviewer.sessionID. The record lives in the author's own working tree and a model can write a file, so today the author could write its own LGTM. The session id is on the record and charset-checked, but it is still only a claim: binding it to an authenticated identity is the writer's job and belongs to the next change, which will also fence .skein/review.json from model edits. Validate base as a full object id, since the record anchors a base..head range. Findings: line is Schema.Int rather than Schema.Number, which observed as accepting 0, -3, 1.5, NaN and Infinity. A present line must be at least 1, and a blocking finding must carry one, because a blocking finding nobody can point at cannot be fixed; an advisory finding may omit it, since a design or test-gap finding has no location. Eight guards mutated in turn; every one observed red and each failed only the test that owns it. The head and base checks share FULL_SHA, so that one mutation trips both by design. No writer in this change: it needs the authenticity design first.
|
This PR doesn't fully meet our contributing guidelines and PR template. What needs to be fixed:
Please edit this PR description to address the above within 2 hours, or it will be automatically closed. If you believe this was flagged incorrectly, please let a maintainer know. |
This was referenced Oct 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
A closed-schema reader for
.skein/review.json(what a reviewer said about one exact commit) and a mapper onto the merge driver'sMergeEvidence. Nothing is wired into the loop or the merge driver, so behaviour is unchanged today. Part 1 of #102.NEEDS_WORKmaps through as itself.independence(independent or same-model) is carried onto the evidence (one optional field indrivers.ts). It is carried, not enforced: a test pins that today's driver still accepts a same-model LGTM, so enforcing it later is a deliberate change.reviewer.sessionIDis required and charset-checked.Known limit (read this)
The record is not trustworthy yet. It lives in the author's working tree, so an author's model can write its own LGTM. The reader carries the reviewer's identity only as a claim. Binding it is the next phase and is written into
review-on-donetasks (Phase 1b): written only by a tool with the authenticated reviewer identity, path fenced against model writes, driver refuses reviewer == author.Evidence
Typecheck clean. Policy, spec-queue and peer suites: 384 pass, 8 skip, 0 fail.
fork:verify: 250/250 owned, 117/117 patched, 0 unregistered. Eight guards mutation-checked; each fails exactly the test that owns it (head and base share one constant, so that mutation trips both tests: one guard, two consumers). Built by one agent, reviewed by me (changes requested and made: dropped a redundant field, tightened line numbers, added independence and reviewer identity), awaiting an independent read.🤖 Generated with Claude Code