fix(aggregate): escalate unmatched edits to REVIEW instead of APPROVE - #151
Merged
Merged
Conversation
The gate is meant to be hermetic — its sandbox git helpers already pass -c commit.gpgsign=false so a global signing config cannot red it — but they missed tag.gpgsign. With tag.gpgsign=true in a developer global config, git tag <name> becomes a signed annotated tag that needs a message, so dgit tag v99.0.0 died with "no tag message?" (and, with an interactive editor configured, an editor prompt). Add -c tag.gpgsign=false beside commit.gpgsign=false in every sandbox helper (sgit/ssgit/dgit/ogit/tgit).
The adversarial review (finding C-1, reproduced) showed cover() marks a required obligation covered as soon as an enforce rule NAMES it in prove.obligation — before matchChanges runs — so a rule 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 REQ-DEM-S10-02 both intend REVIEW, and the intended runtime backstop (classify.ValidateRouting) has no production caller. Add an additive, fail-safe escalation in cover(), mirroring the D-063/D-064 unmatched-whole-file-DELETE guard: for each value-level change (path != "") that no enforce-effective prove rule selects, under a binding that requires an obligation some rule proves, worse(decision, REVIEW) and emit a synthetic aggregate.unmatchedEdit finding (code change.unmatchedEdit). Scope is value-level edits only (whole-file deletes are D-064, adds are non-destructive); observe-phase rules do not govern; one finding per unvouched subject. The comparison corpus baseline (promotion-gates) gains a permissive retention-permissive rule: the challenge-intervention-added case had depended on the fail-open (an ungoverned /retentionMs edit auto-approving), so the baseline now genuinely governs and permits the edit and the candidate retention challenge is a real delta again. Spec: openspec/specs/p5-rvw-review-remediation/spec.md RVW-S02.
|
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.
Intent
An unmatched edit currently fails open to APPROVE. A change that no enforce-phase rule match selects, under a binding that requires an obligation some rule proves, decides APPROVE with zero findings. The cause: cover() marks a required obligation covered as soon as any enforce-phase rule names it in prove.obligation, before it computes matchChanges, so a rule whose match selects none of the governed changes still marks the obligation covered, contributes no finding, and the decision falls through to APPROVE. Reproduced: a policy whose only rule matches valueChanges.pointers ["/partitions"] and proves "non-destructive", under require ["non-destructive"], APPROVEs a governed change at /replicas with an empty finding set. GUIDELINES section 1 (every change must be positively vouched; an empty, broken, or non-matching policy set never auto-merges) and D-142 intend REVIEW. Add an unmatched-edit escalation to REVIEW, mirroring the D-064 unmatched-delete guard: additive, fail-safe, only ever raising toward REVIEW, and a no-op on every change a rule actually governs. Evidence and the exact reproduction: data/assent-adv-dsk/report.md, finding C-1.
What Changed
aggregate.unmatchedEditescalation incover(): a value-level change that no enforce-effectiveproverule selects, under a binding that requires an obligation some rule covers, now raises the decision toward REVIEW and emits achange.unmatchedEditfinding, mirroring the D-063/D-064 unmatched-delete guard.editGovernedandrequiredObligationCoveredhelpers so only enforce-phase rules govern a change (observe-phase findings stay decision-excluded) and whole-file lifecycle events remain in the fileEvents domain; the guard only ever raises the decision viaworse()and is a no-op on fully governed changesets.unmatched_edit_test.goregression coverage and update the existing value/paths tests for the new escalation, correct thepromotion-gatesbaseline with aretention-permissiverule that genuinely governs/retentionMs, disable tag signing in the changelog gate sandboxes, and record decision D-185.Risk Assessment
✅ Low: The change is a small, additive, fail-safe REVIEW escalation that mirrors an existing guard, is a no-op on governed changes, and is covered by a reproduction test; no reachable wrong-result path was found.
Testing
Drove the change end-to-end against the real
assentbinary built from the worktree. Using the shippedassent testadopter harness on isolated policy repos, the exact C-1 reproduction (a rule matching only/*/partitionsthat provesnon-destructive, underrequire: [non-destructive], with a governed/replicasedit) now reports REVIEW with anaggregate.unmatchedEditfinding, while a control repo holding the pre-fix APPROVE golden fails and shows the actual REVIEW plus the escalation finding. Governed edits stay APPROVE (clean-true) or take the rule's own BLOCK (clean-false); an unmatched edit beside a governed BLOCK does not relax it; the gate/scope/observe boundaries behave as specified (no require and uncovered require do not emit the new finding, whole-file delete keeps the D-064 guard, whole-file add stays APPROVE).assent compare --suite examples/comparison/promotion-gatesruns green and a stripped-baseline control fails closed, confirming the corpus correction is required. All shipped example packs still pass. One scenario — the separately-bundledhack/release/changelog_gate_test.shtag-signing sandbox fix — could not be driven: this gate worktree is a tagless checkout, so the script fails its §1 release-tag precondition before reaching the sandbox, and it would need a full clone carrying the v0.1.0 release tags plus network to install git-cliff. No product defects found.Evidence: assent test CLI scenarios (unmatched-edit REVIEW, governed APPROVE/BLOCK, control APPROVE->REVIEW, observe/no-require/uncovered gates, whole-file scope)
Evidence: assent compare --suite promotion-gates (green) and the stripped-baseline fail-closed control
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
✅ **Test** - passed
✅ No issues found.
Built the product:go build -o /tmp/assent-test-bin ./cmd/assent(reportsassent 0.0.0-dev)assent test /tmp/assent-repro— reproduction: unmatched/replicasedit underrequire: [non-destructive]with a rule matching only/*/partitionsreportsPASS topics/unmatched-edit (REVIEW); governed clean-true -> APPROVE; governed clean-false -> BLOCK; unmatched edit beside a BLOCK stays BLOCK; whole-file delete -> REVIEW via aggregate.unmatchedDelete; whole-file add -> APPROVEassent test /tmp/assent-repro-control— a pre-fix APPROVE golden for the unmatched edit now FAILS withdecision: expected APPROVE, got REVIEWand actual findingrule="aggregate.unmatchedEdit" effect="require-review"assent test /tmp/assent-repro-observe— an observe-phase rule matching the change does NOT suppress the escalation (PASS topics/unmatched-edit (REVIEW))assent test /tmp/assent-repro-norequire— norequirekeeps the documented vacuous reading (PASS topics/unmatched-edit (APPROVE), no escalation)assent test /tmp/assent-repro-uncovered— arequirewith no proving rule REVIEWs via aggregate.uncovered withaggregate.unmatchedEditasserted absentassent compare --suite examples/comparison/promotion-gates— exit 0, all five gates PASS,challenge-intervention-addeddelta=stricter-intervention-added; a /tmp copy with the baseline correction stripped fails closed (exit 6), proving the corpus correction is load-bearingassent test examples/packs/{topic-registry,service-catalog,infra-vars}— all shipped example packs green (no-op on governed changes)go test ./internal/core/aggregate -run TestUnmatchedEditFailsSafeReview -count=1andgo test ./examples/comparison/... -run TestCompareCorpusRunsGreen -count=1— targeted regression tests pass✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.