Skip to content

fix(merge-retrospective): track per-gate resolution for multi-gate retrospective issues - #1298

Draft
tvna wants to merge 6 commits into
mainfrom
claude/pr-1177-prep-ifq6u7
Draft

fix(merge-retrospective): track per-gate resolution for multi-gate retrospective issues#1298
tvna wants to merge 6 commits into
mainfrom
claude/pr-1177-prep-ifq6u7

Conversation

@tvna

@tvna tvna commented Aug 24, 2026

Copy link
Copy Markdown
Owner

Summary

Fixes the per-issue-only granularity of .gitapex/ssot.json gates[].tracking_issue corroboration: a multi-gate retrospective issue (one whose Repairs section proposed several distinct gates) no longer clears from gitapex_scan_retrospective_gate_drift.py's no-citation report, or gitapex_check_retro_gate_resolved.py's resolved partition, the moment any one of its proposed gates is built.

Facts

Assumptions

Risk / blast radius

  • .gitapex/ssot.schema.json's schema_version bump (1.3.0 -> 1.4.0) is additive only (proposed_gates is a new required-but-emptyable array); no existing required key was removed or retyped, and the real .gitapex/ssot.json was updated in the same PR so it stays schema-valid.
  • proposed_gates is populated only going forward; every retrospective issue without a manifest entry (all ~223 currently-open ones, plus 46 of the 51 already-registered tracking_issue values) keeps today's exact single-citation behavior -- no backfill required for correctness.
  • skills/merge-retrospective/SKILL.md's Step 5 gains a real new cost: a cycle proposing 2+ new gates must now also open/update a small PR registering the proposed_gates manifest entry, which Step 5 previously never did (pure issue_write, no commit).

Rollback

  • Revert this PR's commits. .gitapex/ssot.json's proposed_gates array and .gitapex/ssot.schema.json's schema_version bump both revert cleanly since nothing outside this PR depends on the new field yet.

Verification

Acceptance Criteria Map (from issue #1177, restated row by row):

Criterion Proof method Result
Per-gate resolution, not clear-on-first-citation Unit fixtures shaped like issue #1129 (6 proposals, 1 built) stay unresolved; a fully-met manifest clears; an off-by-one manifest stays unresolved Passing: tests/test_gitapex_scan_retrospective_gate_drift.py, skills/merge-retrospective/scripts/test_gitapex_check_retro_gate_resolved.py
schema_version bump on any ssot.schema.json change ssot-schema-drift gate continues to pass; schema_version bumped in the same diff uv run --frozen python3 .github/scripts/gitapex_scan_ssot_schema.py -> No ssot.json drift found.
Full pytest suite green uv run --frozen python3 -m pytest 5452 passed (1 pre-existing, unrelated failure deselected: test_gitapex_scan_harden_checkout_pin_drift.py::test_repository_workflows_are_drift_free, a shallow-clone artifact of this sandbox, reproduced identically on the unmodified base branch)
Live dry run against the real repository Both scripts run against the actual .gitapex/ssot.json gitapex_scan_ssot_schema.py: no drift. gitapex_scan_retrospective_gate_drift.py --owner tvna --repo gitapex: ran end-to-end against live GitHub data. gitapex_check_retro_gate_resolved.py 520 928 439 1028 682 1129: {"unresolved": [1129], "resolved": [520, 928, 439, 1028, 682]} -- correctly unresolved for the still-unbuilt 6-gate case, correctly resolved for the fully-built cases

Checklist

  • Tests pass locally
  • Docs updated: skills/merge-retrospective/SKILL.md (Step 5/7), docs/superpowers/specs/2026-08-23-retrospective-gate-per-gate-granularity-design.md
  • Issue number cited in every commit
  • Skill audit evidence disclosed below
  • N/A: no evals/*/split.md Kept-edit-log entry in this PR
  • N/A: no new Stop-boundary bullet or named dispatch branch added to any SKILL.md in this PR (see waiver reasons below)

Skill audit evidence

  • battle-testing-a-skill: WAIVED: skills/merge-retrospective/SKILL.md's own frontmatter description: line is unchanged by this PR; the diff adds one bullet each to Step 5 and Step 7 (a new "open/update a small PR" procedural requirement) and updates one existing sentence in Step 1's own prose to describe the widened two-signal check -- no new Stop-boundary bullet, no new dispatch branch, no new external-input handling or hostile-input surface for an adversarial-input audit to probe.
  • evaluating-skill-quality: WAIVED: same reason as above -- a minor, additive procedural bullet to an existing numbered step, not a structural or nine-dimension-relevant change.
  • adversarial-coverage-mapping: WAIVED: flagged security-relevant by this repo's own keyword heuristic (\b(security|gate|trust) matching "gates" in merge-retrospective's pre-existing frontmatter description, unchanged by this PR) rather than by anything this diff adds. The added bullets are internal process/bookkeeping requirements with no new external-input handling.
  • design-doc-adversarial-review: RAN -- /code-review (high effort) reviewed the full diff range 2b174537..HEAD, including docs/superpowers/specs/2026-08-23-retrospective-gate-per-gate-granularity-design.md; no findings against the design doc's own content.
  • checker-script-adversarial-review: RAN -- the same /code-review (high effort) pass over 2b174537..HEAD found two real issues in the changed checker scripts, both fixed in this PR: (1) both consumer scripts' main() independently re-read and re-parsed .gitapex/ssot.json, contradicting the design doc's own "no new I/O" claim -- fixed by a shared load_gate_and_proposed_gate_corroboration single-read loader; (2) a stale docstring reference to the pre-rename load_gate_tracking_issues function name -- fixed.
  • deterministic-gate-quality: RAN -- graded per skills/evaluating-deterministic-gate-quality/references/dimensions.md including dimension 15 (fail-closed-on-malformed-input). Run as an isolated subagent dispatch (per that skill's own dispatch requirement, since this session authored the artifact), with live execution against synthetic fixtures (duplicate-tracking-issue injection, malformed-entry injection, non-UTF-8/truncated-JSON injection, empty-manifest, missing-proposed_gates) plus the real repository state -- not read-only inference. Verdict: well-formed and well-placed for the new find_duplicate_proposed_gate_tracking_issues check and for the widened resolution logic in both scripts. Found one real fail-open asymmetry (a malformed proposed_gates[] entry was silently skipped rather than raised, unlike the safe skip already used for a malformed gates[].tracking_issue) -- fixed in this PR (both loaders now raise SsotLedgerError on a malformed proposed_gates[] entry instead of dropping it).
  • defeat-test-disclosure: RAN -- tests specifically constructed to defeat (not merely exercise the happy path of) the new/changed detection logic: test_duplicate_proposed_gate_tracking_issue_is_flagged, test_a_single_proposal_manifest_entry_is_schema_invalid (schema layer); test_find_no_citation_issues_stays_uncleared_when_gate_count_falls_one_short, test_partition_resolved_stays_unresolved_when_gate_count_falls_one_short (off-by-one); test_load_proposed_gate_requirements_raises_on_duplicate_tracking_issue, test_load_proposed_gate_requirements_raises_on_non_dict_entry, ..._raises_on_non_integer_tracking_issue, ..._raises_on_non_list_proposals (malformed-input fail-closed behavior).

Related Issue

Closes #1177


Generated by Claude Code

claude added 6 commits August 23, 2026 23:40
Refs #1177 (refs #709, #1176). Design-then-implement doc per this
repo's own plan-first discipline. Records the converged decision (a
proposed_gates manifest in .gitapex/ssot.json, schema_version bump)
plus the two rejected alternatives (per-gate child issues, issue
open/closed state) and why.
…irements

Refs #1177. .gitapex/ssot.schema.json gains a required top-level
proposed_gates array ({tracking_issue, proposals[minItems:2]}),
schema_version bumped 1.3.0 -> 1.4.0. An issue absent from this
manifest keeps today's single-gate behavior unchanged; the five
already-known multi-gate cases (#520, #928, #439, #1028, #682) get
manifest entries as a live-data proof.

gitapex_scan_ssot_schema.py gains a proposedGateManifestEntry model,
a proposed_gates field on SsotRegistry, and a new
find_duplicate_proposed_gate_tracking_issues drift check, so a
duplicate tracking_issue across proposed_gates entries is caught by
the same registry-integrity gate that already checks cluster/policy-ref
references and duplicate ids.
Refs #1177. find_no_citation_issues previously cleared a retrospective
issue the moment ANY one gates[] entry cited it, even when that
issue's own Repairs section proposed several distinct gates (verified
against #520/#928/#439, each already carrying 3 separate gates[]
entries under one tracking_issue). load_gate_tracking_issues (set[int])
becomes load_gate_tracking_issue_counts (dict[int, int]); a new
load_proposed_gate_requirements reads the proposed_gates manifest.
An issue now clears only once a citing commit exists AND the
registered-and-cited gate count meets its own manifest requirement
(default 1, unchanged for every issue without a manifest entry).

Tests cover both directions with a #1129-shaped fixture (6 proposals,
1 built -> stays unresolved) and a fully-met case (clears), plus the
existing single-gate/#314/#665 regressions unchanged.
…script

Refs #1177. gitapex_check_retro_gate_resolved.py's partition_resolved
gets the identical treatment as its CI sibling: load_gate_tracking_issues
becomes load_gate_tracking_issue_counts, a new
load_proposed_gate_requirements reads the same proposed_gates manifest,
and resolution requires the registered-and-cited gate count to meet
the manifest's own requirement (default 1) rather than clearing on
any single citation.

SKILL.md's Step 5 gains a Multi-gate manifest registration bullet: a
cycle proposing 2+ new gates must also open/update a small PR
registering the proposed_gates entry before Step 6 cross-links the
PR; Step 7 gains a matching verification bullet. Step 1's own
description of the two-signal check is updated to describe the
count-based comparison instead of bare set membership.
…tion readers

Refs #1177. An adversarial code-review pass over this PR's diff found
that main() in both gitapex_scan_retrospective_gate_drift.py and
gitapex_check_retro_gate_resolved.py called load_gate_tracking_issue_counts
and load_proposed_gate_requirements separately, each independently
reading and JSON-decoding .gitapex/ssot.json -- contradicting this
PR's own design doc, which stated no new I/O was added. Both scripts
gain a load_gate_and_proposed_gate_corroboration(path) that reads the
registry once and returns both dicts; main() now calls that instead.
The two individual loaders are unchanged for standalone callers/tests.

Also fixes a stale docstring reference in
gitapex_scan_retrospective_gate_drift.py's module docstring, which
still named the pre-rename load_gate_tracking_issues function.
…tes entry

Refs #1177. An isolated evaluating-deterministic-gate-quality
subagent review of this PR's diff found a real, disclosed-but-not-yet-
fixed asymmetry: _proposed_gate_requirements_from_registry silently
skipped a malformed individual proposed_gates[] entry (bad
tracking_issue type, missing/bad proposals), the same tolerant
pattern already used for a malformed gates[].tracking_issue value.

The two skips have opposite risk directions. Skipping a malformed
gates[] entry only under-counts a citation, which can never falsely
resolve an issue (fail-closed, safe). Skipping a malformed
proposed_gates[] entry falls that issue's requirement back to the
weaker default of 1, which can falsely resolve a multi-gate issue on
its first citation -- exactly the false-clear this PR exists to
prevent. Both scripts now raise SsotLedgerError on a malformed
proposed_gates[] entry instead of dropping it silently.
@tvna
tvna deployed to ruleset-verify August 24, 2026 00:38 — with GitHub Actions Active
@coderabbitai

coderabbitai Bot commented Aug 24, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 05540714-a5ba-4891-a094-580ad092f90f


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Aug 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.51%. Comparing base (2b17453) to head (df06249).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1298   +/-   ##
=======================================
  Coverage   99.51%   99.51%           
=======================================
  Files         115      115           
  Lines       20682    20736   +54     
  Branches     2433     2446   +13     
=======================================
+ Hits        20581    20635   +54     
  Misses        101      101           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

tvna commented Aug 24, 2026

Copy link
Copy Markdown
Owner Author

Step 8 two-layer independent review (drafting-a-pr-to-merge)

Re-run against the current head (df06249), which includes the two fix commits (double-read loader consolidation, fail-closed proposed_gates parsing) that landed after the earlier reviews cited in this PR's own "Skill audit evidence" section -- so this is a fresh, non-stale verdict, not a carry-forward of the pre-fix reviews.

Outer layer: github:request_copilot_review was called against this PR at 00:41 UTC. No review was posted within ~10 minutes across two checks. No Anthropic "Claude Code Review" App confirmation exists for this repository either. Recorded per this skill's own rule: outer layer did not run / did not respond -- disclosed, not silently treated as passing.

Inner layer (always runs): adversarial /code-review (high effort) against 2b174537..HEAD (the full PR diff, current head). Verdict: clean, zero findings survived scrutiny across all 8 review angles. No inverted conditions, no regression in the widened resolution logic, no reuse/simplification issue, no convention violation.

Combined with the two prior rounds already disclosed in this PR's body (which found and fixed the double-I/O and fail-open issues before this final pass), mergeable_state is clean, no unresolved review threads exist, and this PR is proceeding to draft per step 9.


Generated by Claude Code


Generated by Claude Code

@tvna
tvna marked this pull request as draft August 24, 2026 00:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(merge-retrospective): track per-gate resolution for multi-gate retrospective issues, not just per-issue

2 participants