fix(merge-retrospective): track per-gate resolution for multi-gate retrospective issues - #1298
fix(merge-retrospective): track per-gate resolution for multi-gate retrospective issues#1298tvna wants to merge 6 commits into
Conversation
…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.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 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. 🚀 New features to boost your workflow:
|
Step 8 two-layer independent review (drafting-a-pr-to-merge)Re-run against the current head ( Outer layer: Inner layer (always runs): adversarial 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), Generated by Claude Code Generated by Claude Code |
Summary
Fixes the per-issue-only granularity of
.gitapex/ssot.jsongates[].tracking_issuecorroboration: a multi-gate retrospective issue (one whose Repairs section proposed several distinct gates) no longer clears fromgitapex_scan_retrospective_gate_drift.py's no-citation report, orgitapex_check_retro_gate_resolved.py's resolved partition, the moment any one of its proposed gates is built.Facts
.gitapex/ssot.json'sgate.tracking_issuefield isinteger|null, and both consumer scripts previously treated a retrospective issue as resolved once anygates[]entry cited it (issue fix(gates): narrow scan_retrospective_gate_drift.py's citation match so a bare #N does not silently clear an issue #709/fix(merge-retrospective): reuse gitapex_scan_retrospective_gate_drift.py's two-signal check in Step 1 #1176's existing two-signal citation+registry check). Verified against live data: issues feat(ci): add documentation-integrity gates -- convention citations, Routine scope, provenance disclosure #520, feat(evals): move split.md machine-read facts to split.json and unify both markdown conventions #928, and chore(ci): adopt ruleset-as-code (or classic branch protection) for gitapex's own main branch #439 each already have 3 separategates[]entries sharing onetracking_issue(Migrate plugin-identity SSOT to agent-plugins.org-compliant root plugin.json #1028 and feat(ci): adopt deterministic detectors for the fail-closed defect classes ruff cannot see #682 have 2 each) -- the multi-gate shape this issue describes already exists in production, not only issue chore(retrospective): merge retrospective for PR #1120 #1129's still-unbuilt 6-gate case.docs/superpowers/specs/2026-08-23-retrospective-gate-per-gate-granularity-design.md, reached via aneliciting-a-designdialogue with the repo owner.Assumptions
Risk / blast radius
.gitapex/ssot.schema.json'sschema_versionbump (1.3.0 -> 1.4.0) is additive only (proposed_gatesis a new required-but-emptyable array); no existing required key was removed or retyped, and the real.gitapex/ssot.jsonwas updated in the same PR so it stays schema-valid.proposed_gatesis populated only going forward; every retrospective issue without a manifest entry (all ~223 currently-open ones, plus 46 of the 51 already-registeredtracking_issuevalues) 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 theproposed_gatesmanifest entry, which Step 5 previously never did (pureissue_write, no commit).Rollback
.gitapex/ssot.json'sproposed_gatesarray and.gitapex/ssot.schema.json'sschema_versionbump 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):
tests/test_gitapex_scan_retrospective_gate_drift.py,skills/merge-retrospective/scripts/test_gitapex_check_retro_gate_resolved.pyschema_versionbump on anyssot.schema.jsonchangessot-schema-driftgate continues to pass;schema_versionbumped in the same diffuv run --frozen python3 .github/scripts/gitapex_scan_ssot_schema.py->No ssot.json drift found.uv run --frozen python3 -m pytesttest_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).gitapex/ssot.jsongitapex_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 casesChecklist
skills/merge-retrospective/SKILL.md(Step 5/7),docs/superpowers/specs/2026-08-23-retrospective-gate-per-gate-granularity-design.mdevals/*/split.mdKept-edit-log entry in this PRSKILL.mdin this PR (see waiver reasons below)Skill audit evidence
skills/merge-retrospective/SKILL.md's own frontmatterdescription: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.\b(security|gate|trust)matching "gates" inmerge-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./code-review(high effort) reviewed the full diff range2b174537..HEAD, includingdocs/superpowers/specs/2026-08-23-retrospective-gate-per-gate-granularity-design.md; no findings against the design doc's own content./code-review(high effort) pass over2b174537..HEADfound 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 sharedload_gate_and_proposed_gate_corroborationsingle-read loader; (2) a stale docstring reference to the pre-renameload_gate_tracking_issuesfunction name -- fixed.skills/evaluating-deterministic-gate-quality/references/dimensions.mdincluding 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 newfind_duplicate_proposed_gate_tracking_issuescheck and for the widened resolution logic in both scripts. Found one real fail-open asymmetry (a malformedproposed_gates[]entry was silently skipped rather than raised, unlike the safe skip already used for a malformedgates[].tracking_issue) -- fixed in this PR (both loaders now raiseSsotLedgerErroron a malformedproposed_gates[]entry instead of dropping it).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