Repository navigation
feat(merge-retrospective): per-family gate proposals, ABSORBED-BY, escalation (#2097) - #2099
Merged
Merged
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Repository: tvna/gitapex/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…creates (#2097) The gate-proposal builder gains a multi-row family ACM body, an append-only recurrence comment keyed on retro and repair index, an occurrence counter with the escalation threshold and label, and the per-mechanism absorption title. Only NEW may create an issue now, in the builder and in the Dedup-sweep hook alike. Its tests move to tests/, which CI collects; the old location under skills/merge-retrospective/scripts was never in pytest testpaths.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2099 +/- ##
=======================================
Coverage 99.60% 99.60%
=======================================
Files 180 180
Lines 31467 31521 +54
Branches 3877 3891 +14
=======================================
+ Hits 31343 31397 +54
Misses 124 124 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The consolidation scan now also checks the reverse direction: a closed gate-proposal duplicate of an open umbrella must appear on its Consolidates line (the 1800/2089 shape). It also flags an open family whose occurrence count reached the escalation threshold without the gate-proposal-escalated label. A sync test keeps the scan's copies of the label, threshold and record regexes equal to the skill builder's.
Adds an optional generic_mechanism boolean to ssot gates (schema 1.7.0) and sets it on defeat-test-mutation-coverage and detection-logic-property-coverage. The registry scan rejects the declaration on a gate that is not active, so ABSORBED-BY can only route repairs onto a mechanism that is live.
Step 4b gains the ABSORBED-BY verdict, verification rules for DUPLICATE-OF, ABSORBED-BY and CLUSTER outside the dispatch, and an action for every verdict: only NEW creates a family issue, while duplicates and absorbed repairs post an append-only recurrence comment and escalate the family at three occurrences. The retrospective body still keeps one entry per repair. ADR 0006 records the decision, and the ranking rubric scores an escalated family as a defect.
Seven merge-retrospective fixtures cover the new filing actions: a verified cluster files one family issue; a duplicate posts a recurrence comment; concurrent runs keep both records; an absorbed repair lands on its mechanism issue; an undeclared ABSORBED-BY id falls back to NEW; and escalation fires at the third occurrence but not the second.
Owner
Author
|
No fix exists that I can port: the failure is in CI credentials, not in code. I am not re-running the job, because the #2093 runs show it fails the same way on every head. I will keep watching this PR. Generated by Claude Code |
…2097) - review-persona call site 6 now lists ABSORBED-BY, so the Step 4b reviewer can actually return it. - Gate ids reject a trailing newline; recurrence comments reject retro and repair values their own parser cannot read back. - Record parsing in the builder and the consolidation scan normalizes CRLF, and the scan fetches comments when the count field is missing. - The Dedup-sweep hook recognizes any verdict text, so a second non-NEW sweep line is always ambiguous. - ALREADY-SHIPPED records Covered by and is exempt from the close condition; a resumed run looks for an existing record before re-running Step 4b; stale per-repair title wording is updated. - Two escalation fixtures now assert what they claim.
Battle-testing found that any comment shaped like a recurrence key was counted toward escalation and could suppress a real post on resume. Escalation now counts a prior key only when its retrospective, re-fetched, records that repair as Filed as this family (count_verified_family_occurrences). A resume skips the post only for a key authored by the account the run posts as. Three adversarial fixtures cover a closed DUPLICATE-OF target, a wrong CLUSTER, and a forged key; CRLF normalization gains direct tests.
… by us (#2097) The battle-test re-run confirmed an attacker could open their own issue whose body imitates a Filed as line, comment the matching recurrence key, and have it counted. A key now counts only when the issue it names is labelled retrospective and was authored by a trusted account: the one this run posts as, or the repository stub opener, since Step 0 fills a stub in place. A bare string of authors is rejected so membership never degrades into substring matching. The forged-key fixture now uses an attacker body that does record the filing.
) The third battle-test run confirmed that even a trusted retrospective can carry a forged Filed as line inside multi-line quoted prose, so verifying keys against retrospective bodies cannot be made sound. count_verified_family_occurrences now counts a key only when the comment carrying it was authored by an account that runs this procedure (plus this run's own key). A comment's author is recorded by GitHub and cannot be forged from text; an account missing from the trusted set undercounts, which fails safe. Adds a fixture for an ABSORBED-BY id missing from the registry.
#2097) The fourth battle-test round found two more ways to inflate the escalation count: a Consolidates line in a family body its non-member opener can edit, and the drift scan counting comments from anyone. Every text-based check has now been forged in some round, so both the skill counter and the scan copy trust the GitHub author_association field instead: a recurrence key counts only from a comment by an OWNER, MEMBER or COLLABORATOR (outside fenced blocks), and a Consolidates line only when such an account opened the family issue. Such an account could add the label directly, so this grants nothing new. The author-blind counter is gone, comment listing is paged to exhaustion, and a deprecated-gate ABSORBED-BY fixture is added.
) Five battle-test rounds forged every text-based recurrence record (comments, the retrospective they name, quoted prose, an editable Consolidates line). Per the owner's choice (case E), a recurrence is now a standalone gate-proposal issue whose generator-made sweep line reads `verdict DUPLICATE-OF #N`, closed with state_reason duplicate and duplicate_of N. The escalation count is 1 plus the distinct titles of closed gate-proposal duplicates of the family: label, closure and sweep line are all push- or triage-gated, so no free text is read. - builder: accept NEW or DUPLICATE-OF #N; add duplicate_target; count by distinct titles; drop the recurrence comment, association and fence helpers - hook: accept DUPLICATE-OF #N again, keep denying ABSORBED-BY - scan: count via GraphQL duplicateOf; drop the reverse-direction check and comment fetching, since the duplicate relation is now the record - references: new recording procedure; extend-issue matches need the label and an ACM (N3); resume trusts only records this account opened (N5) - ADR 0006, branch plan, six eval fixtures (two renamed) and eval status Live proof: #2102 closed through MCP issue_write with duplicate_of 2097 reads state_reason duplicate, and #2097 gained a marked_as_duplicate event.
#2097) evaluate now accepts NEW and DUPLICATE-OF #N and denies ABSORBED-BY; the detection-logic-property-coverage gate asks for a hypothesis property over that regex.
…iled (#2097) Battle-test round 6: an issue author can rewrite their own body after a maintainer labels it and closes it as a duplicate, re-pointing its sweep line at any family. The skill now counts a closed duplicate only when it was opened by the account the procedure posts as. The drift scan reads GraphQL duplicateOf, so it was never exposed. The forged record fixture gains that edited-body case.
…#2097) Battle-test round 7 (overall PASS) found the count filter lived only in prose and that SKILL.md still described the dedup-sweep hook as NEW-only. family_duplicate_titles now keeps only closed-as-duplicate, labelled records opened by this account whose sweep line names the family, with a unit test, and the hook description names both verdicts.
…gn (#2097) - an extend-issue title search counts open issues only, so a repair is never filed on a closed mechanism issue (B1) - Filed as for an absorbed repair names the record unless it created the extend issue, matching the eval and the resume rule (B2) - the hook accepts DUPLICATE-OF only in the shape duplicate_target reads, with an agreement test; duplicate_target also reads lone-CR bodies - the scan fails loudly on an unresolvable or untitled closed duplicate instead of dropping it from the count - plan ACM rows restated for the duplicate design; two eval fixtures pin escalation more tightly
…rops (#2097) Independent review round 1 (drafting-a-pr-to-merge Step 8): - F1: the hook allowed indented, spaced, lowercase or fence-adjacent DUPLICATE-OF sweep lines that duplicate_target never reads back, so the skill would undercount escalation. A DUPLICATE-OF creation now needs exactly one line in the generator exact shape, with a test over each variant. - F3: SKILL.md now says ABSORBED-BY creates the mechanism extend issue when none is open, matching its references.
Independent review round 2, finding B: a fenced exact-shape line naming one family next to a visible, non-exact line naming another passed the hook, and the skill counted the record toward the fenced target. A DUPLICATE-OF creation now needs its single exact line outside any code block and naming the same target as the verified sweep line.
…#2097) Independent review round 3, finding F2: the fixed record structure in SKILL.md gained Absorbed by but not the Covered by line its references and close condition require for an ALREADY-SHIPPED repair.
) Independent review round 4, finding F1: a fenced exact NEW line beside a DUPLICATE-OF line passed the hook, while duplicate_target saw two lines and named no target. Rounds 1, 2 and 4 each found a shape where the hook and the reader disagreed, so the hook now matches exact sweep lines of any verdict with the reader own pattern, pinned equal by a test. Finding F4: 4b.3 now names every repair that never reaches a family issue (ALREADY-SHIPPED, review-worked-as-designed, reclassified).
tvna
marked this pull request as draft
September 25, 2026 00:26
tvna
marked this pull request as ready for review
September 27, 2026 02:52
This branch was successfully deployed
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
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.
Summary
Executed from the approved Branch Plan in
docs/gitapex/plans/2026-09-24-claude-gitapex-pr-2097-6f7ocd.md.merge-retrospectivenow files gate proposals per family instead of per repair, absorbs repairs a declared generic mechanism can express, and escalates a family that recurs 3 times. ADR 0006 records the decision.gate-proposalrecord whose generator-made sweep line readsverdict DUPLICATE-OF #N, then closes it withstate_reason: duplicate,duplicate_of: N.<gate-id>applies only to anactivessot gate declaring the newgeneric_mechanism: true; the repair lands on the single opengate-proposal: extend <gate-id>issue (created through the NEW flow the first time, otherwise recorded as its duplicate). Anything else falls back to NEW.gate-proposalrecords marked as the family's duplicates. At 3 or more the family getsgate-proposal-escalated, whichranking-the-open-queuescores as Defect.Facts
gate-proposalissues against a drift threshold of 20; umbrella gate-proposal-umbrella: skill-audit-disclosure post-push re-check gap #1800 omitted gate-proposal: retro #2083 repair 4: skill-audit-disclosure failed in CI after later pushes added SKILL.md and checker-script changes #2089, which had been closed as its duplicate.extendfamily issue, the escalation label counting the original filing (threshold 3), ADR 0006, and -- after battle-testing -- the closed-duplicate record design ("case E") in place of the originally planned recurrence comment.Consolidates:line). Case E reads only push- or triage-gated state: thegate-proposallabel, the duplicate closure, and the sweep line of a record this procedure's own account opened.issue_writetool withstate_reason: duplicateandduplicate_of: 2097, readsstate_reason: duplicateover REST, and feat(merge-retrospective): file gate proposals per family, add ABSORBED-BY verdict, escalate on recurrence #2097's timeline gained amarked_as_duplicateevent.reason:completedandreason:"not planned", so the skill lists closedgate-proposalissues and filters them withfamily_duplicate_titlesrather than searching by reason.Assumptions
marked_as_duplicateevent is what sets GraphQLIssue.duplicateOf. This session cannot query GraphQL, so the field the CI scan reads was not read directly on test: live proof that issue_write duplicate_of sets duplicateOf (#2097) #2102; the first scheduled drift run is the direct check.gate-proposallabel (GitHub drops label changes from others), per the REST issues documentation relied on in the design.Risk / blast radius
retrospective-gate-driftrun after merge. The scan now counts every closedgate-proposalduplicate via GraphQLduplicateOf, including legacyConsolidates:sources (for example gate-proposal-umbrella: environment/repo-state preconditions #1566-gate-proposal-umbrella: subagent/worktree execution contract #1575's 32 sources and gate-proposal-umbrella: skill-audit-disclosure post-push re-check gap #1800), so open umbrellas with 2 or more legacy duplicates reach 3 without the escalation label. The skill never counts legacy duplicates (no sweep line), so it will not apply that label; backfilling it is backlog triage, a non-goal of feat(merge-retrospective): file gate proposals per family, add ABSORBED-BY verdict, escalate on recurrence #2097. The owner accepted this red in session.Consolidates:) is not shipped: under case E every recurrence is such an issue, so the check would flag every legitimate record.gate-proposalissue to re-check its provenance (battle-test round 7, dimension 11, low).gate-proposalissues keep growing with repairs; only the open count grows with families.Rollback
git revertthe merge commit.Verification
state_reason: duplicate,duplicate_of: N; the entry recordsFiled as:that recordduplicate_target), dedup-sweep hookgeneric_mechanism: true; otherwise NEW. The repair lands on the opengate-proposal: extend <gate-id>issue; a closed one is finished workduplicateOf); at 3 or more addgate-proposal-escalatedfamily_duplicate_titles,count_family_occurrences, scan escalation check, sync test, ranking rubric notedocs/adr/0006-file-gate-proposals-per-family.mdExecution log
PlanApproved{run_id: 1a8e210f}TaskCompleted{run_id: 1a8e210f, task_id: T1, commit_sha: 55537ac3}TaskCompleted{run_id: 1a8e210f, task_id: T2, commit_sha: 55537ac3}TaskCompleted{run_id: 1a8e210f, task_id: T3, commit_sha: b1bf79a0}TaskCompleted{run_id: 1a8e210f, task_id: T4, commit_sha: d1f6d886}TaskCompleted{run_id: 1a8e210f, task_id: T5, commit_sha: e778d458}TaskCompleted{run_id: 1a8e210f, task_id: T6, commit_sha: 979f62cf}StageDeviated{run_id: 1a8e210f, task_id: null, reason: battle-testing rounds 1-5 forged the recurrence-comment escalation count, action: escalate}-- the owner chose the closed-duplicate design (case E), implemented in 419f46b and hardened through 478bd9a.Checklist
skills/*/SKILL.md, adocs/superpowers/specs/*.mddesign doc, a security-relevant skill, or a deterministic checker script (skills/*/scripts/*.py,evals/scripts/*.py,.github/scripts/*.py), a## Skill audit evidencesection discloses the required verdicts/waivers (see.github/scripts/gitapex_gate_skill_audit_disclosure.py)evals/*/split.md, that entry discloses a Transfer check line (see.github/scripts/gitapex_gate_transfer_check_disclosure.py)skills/*/SKILL.md's Stop-boundary bullets or named dispatch branches,evals/<skill>/tasks/*.yamlgained at least as many new fixtures (see.github/scripts/gitapex_gate_skill_branch_fixture_coverage.py)Skill audit evidence
tests/.skills/evaluating-deterministic-gate-quality/references/dimensions.md; its dimension-15 finding, an unresolvable duplicate silently dropped, now fails loudly)test_defeat_*cases for forged sweep lines, free-text injection and text-based inflation, plus hook/reader agreement and an unresolvable-duplicate test)Merge gate: independent review
This PR is also subject to the
independent-review-pendingrequiredstatus check (see
.github/workflows/independent-review-pending.yml/.github/scripts/gitapex_gate_independent_review_pending.py). It stayspending/failing until a
## Independent review verdictsection namingthis PR's current head commit is recorded in this body --
drafting-a-pr-to-merge's own Step 8 records it once its independentreview completes. There is nothing for you to do here now: do not
pre-fill this section yourself, and do not remove this note.
Independent review verdict
request_copilot_reviewon 2026-09-24 (around 23:29Z); noreview_requestedevent or requested reviewer appeared on the PR, so this layer did not run. Copilot is Comment-only with no pass/fail signal even when it runs; the Claude Code Review App is not confirmed installed here.reviewing-an-artifactdispatch (low effort) against head 73ff510, with partial Step 0 deferral toevaluating-skill-quality(isolated dispatch),evaluating-deterministic-gate-qualityandevaluating-context-channel-maturity(both condensed, in-context). The Execution log holds noTaskCompletedentry for this head, so no record was cited.retrospective-gate-driftrun (for example gate-proposal-umbrella: skill-audit-disclosure post-push re-check gap #1800, about 7 occurrences, unlabelled); disclosed under Risk and accepted by the owner. A code comment ingitapex_file_gate_proposal.pycalls the duplicate closure the relation the count is derived from, while the target is read from the sweep line (wording only).duplicate_target(needs a hand-written body from the procedure's own account, later closed as a duplicate); two concurrent runs could both create onegate-proposal: extend <gate-id>issue in a narrow window (the Dedup-sweep count check denies the later create otherwise, and the next run fails closed on two matches); a same-cycle CLUSTER counts as one occurrence (design question under the owner's filing-count decision).Covered by:in the record block (86b381b), and the 4b.3 exception list (73ff510).Related Issue
Closes #2097