fix(aggregate): escalate unmatched edits to REVIEW instead of APPROVE

docs/decisions/decisions.md

@@ -188,3 +188,4 @@ project/process decisions.
 | D-182 | 2026-09-25 | **`mr.labels` is WIRED through the live `assent run` path rather than retired by a lint hard error. The frozen predicate-scope contract keeps its label field, and the fail-open is closed by making the forge fetch honest.** The six-leg review found `mr.labels` schema-carried (`evaluation-input.schema.json`), engine-bound (`aggregate/evaluate.go` binds `labels`) and harness-tested (`internal/adoptertest/mr_test.go`) but never fetched on the live path: `cmd/assent/run.go:581` `mrFrom` built `aggregate.MR` with no `Labels`, and the GitLab adapter's two MR decoders (`GetMR` in `internal/forge/gitlab/gitlab.go` and `mrWithAuthor` in `snapshot.go`) ignored the API's `labels` array. **Consequence, and why it is a safety defect not a missing feature:** every label predicate in production evaluated against an empty list, so a NEGATIVE guard (`!('security-hold' in mr.labels)`) was vacuously true and a `security-hold` MR could APPROVE and merge. **Decision — the alternative is recorded and rejected:** a lint hard error rejecting any `mr.labels` reference plus a `predicate-scope.md` amendment would have made the contract truthful by deleting a capability adopters were already sold; wiring keeps the contract and costs two JSON decodes. **Implementation:** `forge.MRInfo` gains `Labels`; `GetMR` and `mrWithAuthor` decode the string array (absent field → nil, never an error); `Snapshot` surfaces them on `forge.MRHeads.Labels`; `mrFrom` copies `info.Labels` into `aggregate.MR.Labels`. **Proven, not asserted:** a run-path test through the real GitLab adapter over `httptest` (`fakeGitLab`) shows a labelled MR reaching the engine (a label-proven obligation APPROVEs with the label, REVIEWs without it) and the negative-guard polarity failing CLOSED (the guarded label yields BLOCK with zero approve/merge writes). Spec: `openspec/specs/p5-e4-gitlab-forge/spec.md` REQ-E4-S02-05 + REQ-E4-S06-08. Revert: drop `Labels` from `MRInfo`/`MRHeads`, the two decodes and the `mrFrom` copy — reopens the empty-label fail-open for every negative guard. |
 | D-183 | 2026-09-25 | **The six-review round's first fix slice lands: `assent run` reads every judged byte at the pinned commit SHA, not the mutable branch name (U-04 item 1).** Origin: the 2026-09-25 six-leg review (`data/assent-unify/report.md` §4.1 U-04, §6 lane B1; `data/assent-diff-qfn/report.md` QFN-01) at HEAD `b8123cc`. **The defect, stated precisely:** `orchestrate` pins `info.SourceSHA`/`info.TargetSHA` from `GetMR` (`run.go:177`), and the approval and `merge?sha=` CAS are pinned to those commits — but the six content reads on the live path resolved a mutable branch name (`run.go:203,211,230,249,270` = `info.TargetBranch`; `:274` = `info.SourceBranch`). The invariant the merge leans on, *final head == pin ⇒ final head == what was judged*, is therefore false: a contributor with push rights on the MR branch pushes benign X2 (judged), then force-pushes back to pinned X1 before `Reconcile`'s re-read, the SHA-guard passes (head == pin), and the forge merges X1 — bytes never evaluated. The `DecisionRecord` pins `policySha/sourceSha/targetSha` but no content digest, so the record cannot distinguish the states either; ADR-0015 §2's "Merge is SHA-guarded (no TOCTOU)" heading describes the *branch-moved* guard, not a bytes-equal-to-pin guarantee. **The change:** pass `info.TargetSHA` at `:203`, `:211`, `:230`, `:249`, `:270` and `info.SourceSHA` at `:274`. The GitLab adapter already accepts a commit SHA as `?ref=` (`gitlab.go:474`, the same URL-escaped parameter), so this is one argument at six sites — no adapter API change, no new discriminator, no reordering. Target-side reads are pinned too (judgment call (a)): not strictly required for the source-side attack, but it removes the maintainer-push-window ambiguity on the policy/config/pack loads and leaves one uniform rule rather than an asymmetry a later reader must re-justify. **The tests are FLIPPED, not merely updated (judgment call (b)):** the run-path fake's file router now serves the pinned SHAs and errors on any other ref, and `TestFileAtRef` asserts the SHA it sends — so a regression to `info.TargetBranch`/`info.SourceBranch` reddens the existing suite rather than passing against a fake that still accepts branch names. `run_self_vouch_test.go:70`'s policy-load ref assertion moves from `f.target` to `f.targetTip` for the same reason. **The move-and-restore case is in two places, deliberately (judgment call (c)):** the `internal/forge/conformance` case `sha-guard-source-moved-and-restored` (with a new `Fixture.MoveSourceHead` seam and a `catalog.yaml` row) drives `forge.Reconcile` only, so it proves the CAS **merges once the head is back at the pin** — i.e. the CAS alone cannot catch a move-and-restore, which is exactly why the read must be pinned; the run-path case `TestRunJudgesPinnedSHAWhenBranchMoves` proves the read itself follows the pin (branch points at benign bytes, pin at violating bytes, decision follows the pin). **Strictly stronger pinning, no behaviour change beyond it:** the same bytes are read when the branch has not moved. **Explicitly NOT in this lane:** U-04 items 2–4 (SHA-bound `/repository/compare` enumeration re-fold — shares its mechanism with the D-139 trio, lane B5; the `CI_MERGE_REQUEST_SOURCE_BRANCH_SHA` belt; the additive `headContentSha` pin, lane B9), the `--checkout` tree↔SHA binding (U-08/QFN-04, lane B5), the `forgePort` shape (E10), and the `docs/usage/cli.md` "runs without `--checkout` are not exposed" claim (its enumeration half is item 2). Spec: `openspec/specs/p5-rev1-pinned-sha/spec.md`. **Revert:** restore `info.TargetBranch`/`info.SourceBranch` at the six sites and un-flip the fake routers — which re-opens the move-and-restore merge of un-evaluated bytes on the checkout-less path the docs call safe. |
 | D-184 | 2026-09-25 | **RVW-S01 — an empty/absent binding `require:` must never arm APPROVE: the run path refuses it, `assent lint` hard-errors `binding-require-empty`, and this row supersedes D-021's "vacuously covered" clause. The frozen schema's `minItems: 1` is deferred.** **Finding (reproduced on the run path):** a schema-valid `RulesetBinding` binding with `require: []` (or no `require:` key) decides **APPROVE with zero findings** for any governed change its block rules do not fire on. The obligation layer iterates `bind.Require`, so an empty list makes it vacuous; with the forge-probed arming preconditions met the run then approves and merges. The schema accepts it (`require` is not in the binding's `required` list and carries no `minItems`; `schemas/policy/v1alpha1/ruleset-binding.schema.json:52-57`) and lint passed it (`checkObligationCoverage` iterates the same empty list), while two in-repo authorities say the opposite: `GUIDELINES.md` §Safety-1 ("Every change must be positively vouched") and `internal/core/decision/record.go`'s own seam note (S03 review F3: the CLI wiring "must guarantee require is non-empty before an APPROVE is armed"). D-021 added `require` as optional and recorded "absent/empty ⇒ no required obligations (vacuously covered)" — a true statement about the schema's shape that was read as a safety property, and was never one. **Decision:** the invariant wins. (a) **Run path** — `decide`'s decidable, non-reserved arm returns a hard error when the covering binding's `require` is empty; the run exits `1` with a contributor-readable message naming the binding `(class, environment)`, and **zero forge writes** happen (the guard sits before `buildEvaluationInput`, so no record is emitted either). It is deliberately NOT placed in `selectBinding`: that function is shared with `assent compare`, whose whole purpose is to compare permissive candidates, and refusing to compare an unsafe candidate would defeat it. (b) **Lint** — new hard error `binding-require-empty`, located to the binding, with a `good`/`bad` fixture pair registered in the E3-S08 `hardErrorCorpus`. (c) **Reserved-class exemption, stated because it is a real carve-out:** the reserved `assent-policy` meta-class is block-by-default (ADR-0015 §1) and its safety comes from GUARD 1 (any `.assent/**` edit BLOCKs), not from obligation coverage — and it *cannot* carry a non-empty `require` at all, because any `prove` rule in a reserved-bound pack routes the policy class to a vouch/APPROVE-arming disposition the reserved-class check forbids. The lint check therefore skips `class == assent-policy`; the run-path guard still refuses an empty require whenever the SUBJECT is not the reserved class. (d) **Reconciliation** — the schema description's "Absent or empty ⇒ no required obligations (vacuously covered)" and D-021's identical clause are superseded as *safety* statements by this row; D-021 is left byte-unchanged (supersede, don't edit); D-134's ARM-08 run-path consequence ("with no `require:` on the binding it turns a BLOCK into APPROVE+merge") is superseded the same way — the engine still decides APPROVE for an empty `require`, but `decide` (a) refuses before it, so that approve+merge path is gone. The schema's `minItems: 1` is **deferred to the next schema change window**: `schemas/**/*.json` is under the ref-relative freeze guard (D-132) against the released `v0.1.0` tag, which permits exactly one diff (the D-120 `toolDigest` description) and no other, so editing the frozen file in this lane would red `hack/audit/exitgate_test.sh`. Until that window, (a)+(b) are the enforcement. **Not in scope, deliberately:** the unmatched-edit fail-open (a rule that names an obligation but whose `match` selects nothing still marks it covered) shares the `cover()` invariant but is its own register row and is gated on a held captain decision; `assent test` does not arm APPROVE and is unchanged; `assent compare` is unchanged per (a). **Scope note:** `examples/lint-fixtures/{fail-open,reserved-class}/` were touched — `fail-open`'s bindings gained `require: [reviewed]` (the rule already proved it), and `reserved-class/good` is covered by the exemption; both were previously "clean" only because the check did not exist. **Revert:** delete `checkBindingRequireEmpty` and its corpus row, drop the `decide` guard, restore the two `fail-open` bindings, and the vacuous APPROVE returns. |
+| D-185 | 2026-09-26 | **RVW-S02 — an unmatched EDIT never arms APPROVE: an additive fail-safe escalation in `cover()` mirrors the D-063/D-064 unmatched-whole-file-DELETE guard, and the `promotion-gates` comparison baseline is corrected because one of its cases had accidentally depended on the fail-open.** **Finding (report C-1, reproduced):** `cover()` marks a required obligation *covered* as soon as any enforce-phase rule names it in `prove.obligation` — BEFORE it computes `matchChanges` (`internal/core/aggregate/coverage.go:150-152` vs `:154-160`). So a rule that names the obligation but whose `match` selects none of the governed changes still marks it covered, contributes no finding, and the decision falls through to APPROVE with an EMPTY finding set: a policy whose only rule matches `valueChanges.pointers: ["/partitions"]` and proves `non-destructive`, under `require: [non-destructive]`, APPROVED a change at `/replicas` with zero findings. GUIDELINES §Safety-1 and D-142's REQ-DEM-S10-02 both intend REVIEW. The intended runtime backstop, `classify.ValidateRouting`, has no production caller and `assent run` never runs lint, so nothing else catches it. **Decision (captain: option (a), "add an unmatched-edit escalation to REVIEW, mirroring the D-064 unmatched-delete guard"):** after the rule loop, for each VALUE-LEVEL change (`ch.Path != ""`), if no enforce-effective prove rule selects it, `worse(decision, REVIEW)` and emit a synthetic `aggregate.unmatchedEdit` finding (`effect: require-review`, `code: change.unmatchedEdit`, subject the change's subject). **Scope — value-level edits only:** whole-file (`path==""`) lifecycle events are the fileEvents domain — a delete is the D-064 guard's subject, an add is non-destructive (D-063) — so neither escalates here; this also leaves the D-016 golden's whole-file rename event untouched. **Gate:** the escalation runs only when `requiredObligationCovered` (some obligation the binding requires has an enforce proving rule, i.e. `covered[obl]`). An empty/absent `require` is D-184's run-path seam and the engine's vacuous-APPROVE reading is deliberately unchanged; a `require` with no proving rule already trips the uncovered guard. Without the gate a policy with no obligation layer would escalate every value change — outside C-1's scope. **"Governed" (`editGoverned`) mirrors `fileDeleteGoverned` exactly:** some enforce-effective prove rule selects the change, obligation-agnostic (a non-required signal rule that matches is a real evaluation, and the points model depends on it being able to leave the decision at APPROVE). Observe-phase (and pack-ceiling-observe) rules do NOT govern — their findings are decision-excluded, so treating them as governing would suppress escalation into APPROVE, the same D-063 fail-open the delete guard names. One finding per unvouched SUBJECT (deduped; a subject can carry many value changes and duplicate identical findings would be noise). **Additive and deterministic:** it only ever raises toward REVIEW via `worse()`, never relaxes a BLOCK, and the findings are canonically sorted; a fully governed changeset is byte-identical. **Corpus correction, stated because it is a real behaviour change to a shipped example:** `examples/comparison/promotion-gates/cases/challenge-intervention-added` changed `/retentionMs` under a baseline (`prod-strict@6`) with NO rule matching that pointer, so it APPROVED only because of this fail-open; with the guard the baseline REVIEWs and the case's `stricter-intervention-added` delta disappears. The baseline gains a permissive `retention-permissive` rule (proves a required obligation cleanly true), so the baseline genuinely governs and permits the edit and the candidate's `retention-ack` challenge is a real delta again — the record is byte-identical. **Reproduced then fixed:** the pre-fix `Cover` returned `APPROVE findings=[]`; the regression test `TestUnmatchedEditFailsSafeReview` pins the REVIEW + `aggregate.unmatchedEdit` finding, both polarities, the gate, the scope and the observe cases. **Supersession:** this row closes the clause D-184's *Not in scope* left open ("the unmatched-edit fail-open … is its own register row and is gated on a held captain decision"); D-184 is left byte-unchanged (supersede, don't edit). Spec: `openspec/specs/p5-rvw-review-remediation/spec.md` RVW-S02. **Revert:** delete the escalation block + `editGoverned`/`requiredObligationCovered` + `ruleUnmatchedEdit`, restore `baseline.yaml`, and the unmatched edit silently APPROVEs again. |

examples/comparison/promotion-gates/baseline.yaml

@@ -45,3 +45,23 @@ spec:
       onFailure:
         effect: comment
         code: value-bumped
+    # RVW-S02: the baseline must GOVERN a /retentionMs edit for it to be a valid
+    # APPROVE side. Before the unmatched-edit escalation it approved /retentionMs
+    # only because no rule matched it — the fail-open this corpus case accidentally
+    # depended on. This permissive rule proves a required obligation cleanly true,
+    # so the baseline genuinely permits retention changes; the candidate's
+    # retention-ack challenge is then a real stricter-intervention-added delta.
+    - name: retention-permissive
+      phase: enforce
+      match:
+        valueChanges:
+          pointers: ["/retentionMs"]
+          kinds: [modify]
+      prove:
+        obligation: non-destructive
+        when:
+          cel: "true"
+          message: "baseline permits retention changes"
+      onFailure:
+        effect: block
+        code: retention.shrunk

hack/release/changelog_gate_test.sh

@@ -573,7 +573,7 @@ sgit() {
     -u GIT_ALTERNATE_OBJECT_DIRECTORIES -u GIT_COMMON_DIR \
     git -C "$SANDBOX" \
     -c user.name='changelog gate' -c user.email='gate@example.invalid' \
-    -c commit.gpgsign=false -c core.hooksPath=/dev/null \
+    -c commit.gpgsign=false -c tag.gpgsign=false -c core.hooksPath=/dev/null \
     -c init.defaultBranch=main -c advice.detachedHead=false "$@"
 }
 env -u GIT_DIR -u GIT_WORK_TREE -u GIT_INDEX_FILE -u GIT_OBJECT_DIRECTORY \
@@ -1001,7 +1001,7 @@ ssgit() {
     -u GIT_ALTERNATE_OBJECT_DIRECTORIES -u GIT_COMMON_DIR \
     git -C "$SUBJ_SANDBOX" \
     -c user.name='commit subject gate' -c user.email='gate@example.invalid' \
-    -c commit.gpgsign=false -c core.hooksPath=/dev/null \
+    -c commit.gpgsign=false -c tag.gpgsign=false -c core.hooksPath=/dev/null \
     -c init.defaultBranch=main -c advice.detachedHead=false "$@"
 }
 env -u GIT_DIR -u GIT_WORK_TREE -u GIT_INDEX_FILE -u GIT_OBJECT_DIRECTORY \
@@ -1151,7 +1151,7 @@ dgit() {
     -u GIT_ALTERNATE_OBJECT_DIRECTORIES -u GIT_COMMON_DIR \
     git -C "$DSB" \
     -c user.name='changelog gate' -c user.email='gate@example.invalid' \
-    -c commit.gpgsign=false -c core.hooksPath=/dev/null \
+    -c commit.gpgsign=false -c tag.gpgsign=false -c core.hooksPath=/dev/null \
     -c advice.detachedHead=false "$@"
 }
 head_sha="$(git -C "$ROOT" rev-parse HEAD)"
@@ -1400,7 +1400,7 @@ if [[ -z "${ASSENT_CHANGELOG_GATE_NESTED:-}" ]]; then
       -u GIT_ALTERNATE_OBJECT_DIRECTORIES -u GIT_COMMON_DIR \
       git -C "$TSB" \
       -c user.name='changelog gate' -c user.email='gate@example.invalid' \
-      -c commit.gpgsign=false -c core.hooksPath=/dev/null \
+      -c commit.gpgsign=false -c tag.gpgsign=false -c core.hooksPath=/dev/null \
       -c advice.detachedHead=false "$@"
   }
   env -u GIT_DIR -u GIT_WORK_TREE -u GIT_INDEX_FILE -u GIT_OBJECT_DIRECTORY \
@@ -1438,7 +1438,7 @@ if [[ -z "${ASSENT_CHANGELOG_GATE_NESTED:-}" ]]; then
     env -u GIT_DIR -u GIT_WORK_TREE -u GIT_INDEX_FILE -u GIT_OBJECT_DIRECTORY \
       -u GIT_ALTERNATE_OBJECT_DIRECTORIES -u GIT_COMMON_DIR \
       git -c user.name='changelog gate' -c user.email='gate@example.invalid' \
-      -c commit.gpgsign=false -c core.hooksPath=/dev/null "$@"
+      -c commit.gpgsign=false -c tag.gpgsign=false -c core.hooksPath=/dev/null "$@"
   }
   mkdir -p "$OSRC/sub"
   echo kept >"$OSRC/kept" && echo unstaged >"$OSRC/unstaged-rm" && echo staged >"$OSRC/sub/staged-rm"

internal/core/aggregate/aggregate.go

@@ -220,6 +220,11 @@ const (
 	// event earns (EFE-S02, Judgment call (a) / D-063): a delete no evaluated
 	// fileEvents rule covers must never silently APPROVE.
 	ruleUnmatchedDelete = "aggregate.unmatchedDelete"
+	// ruleUnmatchedEdit labels the fail-safe REVIEW a value-level change earns when
+	// NO enforce-effective prove rule selects it, under a binding that requires an
+	// obligation some rule proves (RVW-S02 / C-1): an edit outside every rule's
+	// match scope is not positively vouched, so it must never silently APPROVE.
+	ruleUnmatchedEdit = "aggregate.unmatchedEdit"
 )
 
 // ReservedPolicyClass is the built-in meta-class (ADR-0008/ADR-0015 §1) that an