fix(merge-retrospective): file missing-deterministic-gate findings as standalone issues - #1417
Conversation
…tive Refs #1405 Consolidates missing-deterministic-gate findings under one long-lived GitHub-native parent issue via sub-issues, so a retrospective issue's own lifecycle no longer depends on an unrelated historical backlog's completion state -- the structural cause of ~300 open retrospective issues found while filing #1405.
…ue hierarchy Refs #1405 Adversarial review of the sub-issue-hierarchy draft found it structurally unfit (GitHub's 100-sub-issue-per-parent cap, a CI permission gap, an unresolvable parent-creation race, and a wrong tool citation). Redesigned around flat, independently-labelled standalone issues instead -- no parent, no hierarchy, no shared-resource creation race -- with the new label's own name registered in .gitapex/ssot.json rather than duplicated as independent hardcoded literals.
Refs #1405 The previous commit only captured the file rename; this commit carries the actual content rewrite. Drops the sub-issue-hierarchy mechanism (rejected: GitHub's 100-sub-issue-per-parent cap, a CI permission gap, an unresolvable parent-creation race, a wrong tool citation -- each verified against primary sources) in favor of flat, independently labelled standalone issues with no shared parent, plus registering the new label's own name in .gitapex/ssot.json instead of duplicating it as independent hardcoded literals.
Refs #1405 Fixes six verified findings: filed issues now carry a real Acceptance Criteria Map instead of a tracking waiver (the disqualifying gap -- this repository own pr-issue-acm-disclosure hook denies Closes-citing a tracking-waived issue, which would have reproduced the exact re-file-under-a-new-number pattern this design exists to remove); search-before-create idempotency for individual filings; drops the gitapex ssot dot json label registration (its own schema explicitly reserves label fields for a not-yet-built label-policy mechanism per issue 123) in favor of the repository existing parallel-copy-plus- sync-test precedent; specifies the closed-issue integrity pass concrete API shape and state_reason exemption; adds a label-liveness guard; defines the deterministic title used for idempotent search-before-create.
Refs #1405 Fixes four verified findings: the idempotency title is now keyed on each repair own fixed index within the cycle, not its free-text label, after the review found two distinct repairs could share a generic label (Failed CI rerun style) and silently collide -- the second repair search-before-create would then find the first repair issue and skip filing, a silent finding loss worse than the duplicate risk it was meant to prevent; Step 5 filing moves from prose-only tool calls into a new bundled script (gitapex_file_gate_proposal.py) so Decision 6 sync-test precedent actually compares two real scripts, not a script against SKILL.md prose; the race-elimination claim is corrected to a race-reduction-plus-detection claim, since search-then- create is not atomic; the closed-but-unverified integrity pass now fails the run and reopens a still-unverified issue on window-exit, instead of reporting into a window that could silently expire.
Fourth adversarial review found the third revision own fixes had each grown a new secondary mechanism, and each new mechanism produced its own blocking or significant problem: a pre-Step-5 body write for retry stability collided with Step 0 own unchanged stub-marker branching and the stale-stub autocloser; a window-exit reopen needed CI write permission the workflow does not have and contradicted its own gating rule; the filing script drafted a direct issue_write call a plain bundled script cannot actually make. Per operator direction, this revision removes each secondary mechanism rather than patching it further: - Decision 1: drop the pre-Step-5 body write; the enumerated repair index is assigned once in-memory during classification and written only as part of Step 5 own single existing body write. Names the resulting session-interruption limitation as an accepted residual risk instead of solving it, and extends the same single-pass indexing to a future legacy-backlog audit. - Decision 2: adds an explicit GPRR-compatibility clause -- the inline Status: missing-deterministic-gate line stays unchanged alongside the new Filed as: line, so gitapex_compute_gprr.py keeps parsing correctly. - Decision 5: replaces the 7-day closed_at window and its reopen action with an unbounded, zero-tolerance integrity pass over every closed labelled issue -- no missed-cron escape hatch, no new CI write scope. - Decision 6: gitapex_file_gate_proposal.py becomes a pure, network-free helper (title/ACM-body/label computation only); every actual GitHub write stays a direct agent tool call, keeping the existing ACM-disclosure hook in the loop. Also names the label literal: gate-proposal. - Non-goals, Architecture, Components, Data flow, Error handling, Testing, and Open questions updated for consistency. Refs #1405
|
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 #1417 +/- ##
=======================================
Coverage 99.57% 99.57%
=======================================
Files 127 127
Lines 22539 22558 +19
Branches 2766 2767 +1
=======================================
+ Hits 22443 22462 +19
Misses 96 96 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Status note on the two currently-failing checks ( This PR is a work-in-progress draft opened by
Generated by Claude Code |
…1406 Rewrites skills/merge-retrospective/SKILL.md's Steps 1-5 per the flat-gate-proposal-issues design: Step 1 becomes a no-op (the automatic carry-forward sweep and its .gitapex/ssot.json/git log dependency are removed; the legacy backlog stays explicitly out of scope). Steps 2-4 assign each repair its own 1-based index in memory during the same classification pass, reusing the existing Repairs-list N. numbering rather than a new counter. Step 5's one existing body write now opens with the enumerated repair list, invokes the new gitapex_file_gate_proposal.py helper (built in a parallel task) once per missing-deterministic-gate repair, performs the search-then-create-then- verify sequence via direct mcp__github__* tool calls, and records a "Filed as: #<N>" line next to each repair's existing "Status: missing-deterministic-gate" line without removing it. Close now follows the existing attended/unattended confirm rule for every close this rewrite introduces, not only the zero-repair case. unclear-agent- instruction and external-human-decision repairs are unaffected. Adds error-handling text for a create failure, an unconfirmed write, and the search step finding more than one existing issue (fail closed, mirroring Step 0's own ambiguous-stub-match discipline). Step 0's own CI-stub-marker-based branching is byte-for-byte unchanged. Refs #1406
The daily retrospective-gate-drift meta-check swept every retrospective-labelled issue for a no-citation count, the mechanism the flat gate-proposal-issues design (Decision 5) replaces. Rescope gitapex_scan_retrospective_gate_drift.py to its own independent GATE_PROPOSAL_LABEL copy, a label-liveness guard (closing the prior evaluate() gap that read an empty search result as an unconditional pass), a threshold-gated open-issue-count report (unchanged at 20), and an unbounded zero-tolerance integrity pass over every closed labelled issue via the existing two-signal check, state_reason-exempt and with no reopen action. Workflow permissions stay contents/issues/pull-requests: read only. Refs #1406
…lose Task A's own Step 5 rewrite unified the attended/unattended confirm-then-close rule across every close this skill now performs, not only the zero-repair fast-close case, but Step 7's own close-verification text still scoped its re-fetch/retry logic to "the zero-repair fast-close path applied." Broadened the same wording to cover a close following the full Repairs path too -- caught during merge-back screening of Task A's diff, not by a separate task. Refs #1406
… builder New pure, network-free helper (Task B of the flat gate-proposal issues design) that computes the deterministic, collision-proof gate-proposal issue title and the fully-populated Acceptance Criteria Map body for a missing-deterministic-gate retrospective repair, plus the GATE_PROPOSAL_LABEL constant SKILL.md's Step 5 and the CI-side scan script both key off. Makes no GitHub API calls of its own; the skill's own Step 5 prose is what invokes issue_write/issue_read directly with the values this module returns. Refs #1406
…on-intent wording for issue #1406 Task A rewrite legitimately removed the separate two-field carried-forward record schema (Decision 2 unifies it into the same filing path as any other missing-deterministic-gate repair) and added the Filed as field (Decision 1), but tests/test_gitapex_merge_retrospective_record_format.py was outside all three Wave 1 tasks file-ownership map and still asserted against the removed schema, failing after merge. Updated the drift gate to match: repairs-section boundary now ends at Notes (no more Carried-forward gate heading), the now-dead carried-forward test and helper removed, and a new Filed as presence/absence check added per repair category, mirroring the existing Proposed-gate check. Also fixed a script-execution-intent-stated shape-check regression the new bundled script triggered: SKILL.md mentioned gitapex_file_gate_proposal.py by its own bare filename without either of the checker required qualifying phrases. Found and fixed during Wave 1 merge-back verification (full repo pytest run), not by a separate task. Refs #1406
0144ead to
6ae1703
Compare
Task D of the Branch Plan (docs/superpowers/plans/2026-08-29-claude-gitapex-pr-1395-f1t7w4.md): skills/merge-retrospective/scripts/gitapex_file_gate_proposal.py and .github/scripts/gitapex_scan_retrospective_gate_drift.py each carry their own independent copy of GATE_PROPOSAL_LABEL by design (the plugin-install boundary forbids a cross-tree import), so nothing else guards the two literals from silently drifting apart. Adds a sync test, following tests/test_gitapex_pr_title_convention_regex_sync.py's exact shape, loading both modules by file path and asserting the two constants stay equal. Refs #1406
…st for issue #1406 Adds five new evals/merge-retrospective/tasks/*.yaml fixtures covering the five scenarios named in the flat-gate-proposal-issues design doc Testing section: zero-repair fast-close unchanged; zero-repair fast-close despite a pre-existing legacy backlog explicitly out of scope; attended multi-repair filing-and-close; unattended filing-with-stay-open; a resumed run after a partial filing failure retrying only the unfiled repair. All five pass evals/scripts/gitapex_lint_fixture_assertions.py with 0 warnings against the current SKILL.md. Adds one test to tests/test_gitapex_compute_gprr.py confirming a repair inline Status: missing-deterministic-gate line still parses correctly via gprr.parse_status_tags when immediately followed by the new Repair record format own Filed as: #<N> line -- no change to gitapex_compute_gprr.py itself; its existing suite (42 tests) passes unmodified. Disclosed, out-of-scope follow-up: evals/merge-retrospective/tasks/ carried-forward-gate-implemented-test.yaml and carried-forward-gate-unimplemented-train.yaml test the OLD Step 1 carry-forward-check mechanism this redesign Step 1 rewrite removed entirely. They are now stale and were deliberately left untouched, along with split.json/split.md (retiring them requires updating the declared train/selection/test partition arithmetic and equivalence-class bookkeeping those files track, a separate, larger concern outside this task scope). Refs #1406
Task E added 5 new evals/merge-retrospective/tasks/*.yaml fixtures for issue #1406, but evals/merge-retrospective/eval-status.md own stated committed-task-files count was outside Task E own file-ownership scope and still said 20, tripping test_gitapex_skill_eval_status_sync.py and the derived docs/skill-eval-status.md regeneration check. Updated the count to 25 (20 in the formal held-out split plus 5 new, not-yet-split-assigned fixtures), and while touching this paragraph corrected its now-stale claim of Step 0 carry-forward-check eval coverage -- that mechanism no longer exists after Task A rewrite, so the two fixtures it names are flagged stale here too (same disclosed, out-of-scope follow-up Task E already flagged for the fixtures themselves; split.json/split.md remain untouched). Regenerated docs/skill-eval-status.md via its own generator script. Found and fixed during Wave 2 merge-back verification (full repo pytest run), not by a separate task. Refs #1406
|
Status on the
Re-ran the failed job once ( The two other still-red checks ( Generated by Claude Code |
…lding Behavior-preserving cleanup over the accumulated branch diff, where three Wave 1 agents and two Wave 2 agents each wrote their own copy of the same test scaffolding with no visibility into the others. tests/test_gitapex_scan_retrospective_gate_drift.py - add one `_url_capturing_opener` helper and use it at the four sites that each re-declared an identical URL-recording `opener` closure; - collapse the `state=open` / `state=closed` URL tests into one parametrized test (same two cases, same assertions); - extract `_install_recording_fakes`, the ~25-line block of four recording fakes both "validation never silently trims" tests duplicated verbatim. tests/test_gitapex_merge_retrospective_record_format.py - move `_FILED_AS_RE` up beside the other module-level record regexes instead of leaving it stranded between two helper functions; - fold the two consecutive branches on the same `slug == "missing-deterministic-gate"` condition into one, keeping every assertion, its message, and their existing evaluation order. skills/merge-retrospective/scripts/test_gitapex_file_gate_proposal.py - parametrize the two title-format tests (the weaker startswith/endswith one becomes a second exact-equality case) and the zero/negative index rejection pair. evals/merge-retrospective/eval-status.md - re-wrap the orphaned "Step 0's own dedup" line the fixture-count edit left mid-paragraph. No production code touched and no assertion dropped. The full suite still reports the same 6679 passing tests as before this commit, and both ruff check and ruff format --check pass on every file changed here. Refs #1406
…wo escaping defects
Step 8 adversarial review of the flat gate-proposal-issues branch.
SKILL.md
- Step 0's "match found, body no longer carries the marker" branch stopped
the run before Step 2, which made Step 5's own resumed-run rule ("a
later, resumed run retries only the repairs still missing a Filed as:
line") unreachable through the skill's own entry point: an interrupted
filing could never be finished, and the
gate-proposal-resumed-run-partial-filing-retry eval fixture described
behavior the procedure forbade. That branch now carves out exactly the
unfinished-filing case and hands it to Step 5; its dangling reference to
the deleted Step 1 carry-forward check is gone with it.
- The Repair record format claimed Step 5 performs a "single body write",
which cannot hold: each Filed as: line is added once its own filing is
confirmed, so the body is written again. Reworded to "first body write"
and stated explicitly.
- Step 5 required creating the retrospective label first when absent but
said nothing for gate-proposal, whose absence hard-fails the daily drift
check. The same instruction now covers both.
Scripts
- gitapex_scan_retrospective_gate_drift.label_exists percent-quotes the
label while list_labelled_issue_records interpolated it raw, so the
liveness guard and the two passes it guards could ask about different
labels: a label containing a space emitted a raw space into the request
line, and one containing an ampersand injected a second state=
parameter ahead of the caller's own -- a silently-wrong clean count of
exactly the class the guard exists to rule out. Quoted in both now.
- gitapex_file_gate_proposal._sanitize_cell escaped the pipe but not the
backslash, so free text already carrying an escaped pipe (a proposed
gate naming a regex or a grep alternation) turned into an escaped
backslash plus a live column delimiter, silently adding a seventh column
to the ACM table. Backslashes are escaped first now.
Registry
- .gitapex/ssot.json's retrospective-gate-drift-scan entry still described
the retired retrospective-label citation sweep in its rule text and named
the retrospective-labelled state=all issue query as a target. rule,
local_exclusion, and target rewritten against the script's real behavior
(label liveness, open-count threshold, closed-issue two-signal
integrity); 1406 appended to tracking_issue.
Tests
- The reopen-wording assertion was a self-satisfying disjunction that the
FAIL branch's own disclaimer already satisfied, so it would still pass
beside a line promising a reopen. Replaced with an occurrence count.
- Added defeat coverage for the label-URL quoting, both halves of the
two-signal check at main level, and five non-exempt state_reason values.
- Strengthened the ACM body test to assert column position rather than mere
substring presence, and added the already-escaped-pipe case.
- Every new assertion was confirmed to fail against the unfixed code.
Refs #1406
…usting it battle-testing-a-skill dispatched three independent trials against the current SKILL.md (dimensions catalog, cold-enumerated per trial). Two of three trials FAILed on dimension 13 (cross-session memory poisoning): a resumed run skipped a repair whose retrospective-issue body already carried a Filed as line, with no re-verification that the referenced issue still exists -- an externally editable body could carry a stale or tampered line and silently suppress a real filing, while the same run already demands re-fetch confirmation for anything it files itself. Fixed by requiring the same re-fetch confirmation before trusting a pre-existing line; an unverified line is now treated exactly like an unconfirmed write and the repair proceeds through the normal search-then-create flow. New eval fixture defeat-tests this exact case (tagged adversarial, per this repository's own convention for this class of fixture). Also closed dimension 16 (encoding/obfuscation coverage, flagged by two of three trials) with one clarifying sentence: the injection-containment rule never parses or executes quoted text, so it already treats an obfuscated payload identically to a literal one -- this was previously left implicit. evaluating-skill-quality's own single dispatch (WELL-FORMED-NOT-MATURE) separately found the metadata sidecar's spec.references corrections (issues #1176, #1196) cite a Step 1 dependency that this PR's own Task A rewrite already removed, so their stated rationale for executionRequirements.tools.read no longer holds against current content. Appended a new correction entry (append-only, matching the sidecar's own discipline) correcting read from [files] to [] and re-justifying shell from Step 5's own script call instead; left spec.portability exactly as the prior correction set it, since that axis is a human decision this entry does not make. Refs #1406
…5-f1t7w4 # Conflicts: # .gitapex/ssot.json
…n fix The reviewing-an-artifact-equivalent independent-review dispatch (Step 8 of drafting-a-pr-to-merge) found the design doc still documented the insecure resumed-run behavior that a fixture in the same PR proves was fixed: Decision 1 residual-risk paragraph, Data flow step 4, and Error handling all still said a resumed run trusts a pre-existing Filed as line at face value and stops at Step 0 instead of finishing an interrupted filing. Updated all three sections plus Testing and Open questions to describe the actual, corrected behavior (Step 0 routes an enriched-but-incomplete body to Step 5, and Step 5 re-fetches a pre-existing Filed as line before trusting it) and record this as a fifth, post-elicitation adversarial finding instead of leaving stale prose next to the code it used to describe. Refs #1406
Duplicate-PR-waiver: issue #1406 was reframed (2026-08-29T17:50:03Z) from
its original, narrower "close an existing stub after a zero-repair
cycle" scope into a much larger fix, after PR #1416 (a different,
concurrent session, opened 2026-08-29T17:51:11Z) had already branched
from the pre-reframe scope. PR #1416's own small fix is a valid subset,
independently subsumed by this PR's own Task A (see Facts below and the
comment posted on PR #1416). Per explicit operator direction this PR
proceeds regardless of PR #1416's outcome; if PR #1416 merges first and
closes #1406 prematurely, a new issue carrying this same Acceptance
Criteria Map will be filed and this PR's
Closescitation updated tomatch.
Summary
Implements issue #1406's redesign of
merge-retrospective's filing/closingmechanism so a
missing-deterministic-gatefinding is filed as its ownstandalone, ACM-carrying, closeable-by-citation issue at the moment it is
classified -- removing the structural cause of the 277-of-345 unresolved
retrospective-issue backlog. All 6 ACM rows implemented; Step 8's mandatory
refactor and adversarial-review passes complete, including a real
cross-session-trust defect the review found and this PR fixed; full repo
test suite green (6750 passed).
Facts
(
docs/superpowers/specs/2026-08-29-flat-gate-proposal-issues-design.md,4 rounds of independent adversarial review) and the task-decomposed
Branch Plan (
docs/superpowers/plans/2026-08-29-claude-gitapex-pr-1395-f1t7w4.md).re-verified
planning-a-branch-from-an-issueat 2026-08-29T17:58:38Z.executing-a-branch-plan(Workflow tool,branch-plan-taskagents,isolation: 'worktree'): Wave 1 (Tasks A/B/C,parallel) then Wave 2 (Tasks D/E, parallel, after Wave 1 merged back).
fixed 3 issues outside any task's own file-ownership map: a pre-existing
test (
tests/test_gitapex_merge_retrospective_record_format.py) assertingagainst the removed carried-forward record schema; a
script-execution-intent-statedshape-check regression from the newbundled script's own bare-filename mention; and
evals/merge-retrospective/eval-status.md's stale committed-fixturecount after Task E added 5 new fixtures. All three fixed by the main
thread, not a separate task -- see the Execution log below.
branch-plan-tasksubagent commits(Task A's original, and Task E's original) appended an undisclosed
Claude-Session:URL and aCo-Authored-By: Claude Sonnet 5trailer --neither authorized by this repository's own provenance-disclosure
convention (no such trailer is ratified in
CONTRIBUTING.md). Caughtduring merge-back review before merge, per this repository's own
outward-artifact-preflightchecklist. Task A's commit had already beenpushed to this branch by that point; the branch's commit chain from
the task-list-file commit onward was rebuilt via
git commit-tree(identical trees throughout, confirmed via
git diffproducing nooutput -- only commit messages changed) and force-pushed
(
--force-with-lease) to this exclusive, single-session branch. TaskE's commit was never pushed with the trailer -- cherry-picked with a
corrected message from the start. No file content was altered by this
remediation.
battle-testing-a-skilltrials against the pre-fix
SKILL.mdFAILed 2-of-3 on dimension 13(cross-session memory poisoning): a resumed run trusted a pre-existing
Filed as: #Nline in the (externally editable) retrospective issuebody with no re-fetch, unlike every filing the same run makes itself.
Fixed by requiring the identical re-fetch confirmation before trusting
a pre-existing line (commit
0599e20c); a 4th confirmation trial againstthe fixed text returned PASS on both this and the related dimension 16
(obfuscation coverage) gap. New adversarial-tagged eval fixture
(
gate-proposal-resumed-run-stale-filed-as-line-refiles.yaml)defeat-tests the fix. Full detail in Skill audit evidence below.
evaluating-skill-qualitydispatchseparately found the
metadata/gitapex.yamlsidecar'sspec.referencescorrections cited a Step 1 dependency (
gitapex_check_retro_gate_resolved.py/
.gitapex/ssot.json) that Task A's own rewrite had already removed --stale rationale for
executionRequirements.tools.read. Fixed via a new,append-only correction entry (commit
0599e20c);spec.portabilityleftunchanged, since that axis is a human decision this entry does not make.
dispatch (drafting-a-pr-to-merge Step 8) found the design doc still
documented the pre-fix, insecure resumed-run behavior (trusting a
Filed as:line with no re-fetch) that the Step 8 fix above (commit0599e20c) had already corrected inSKILL.mdand the eval suite --stale prose next to code it used to describe. Fixed by updating Decision
1, Data flow step 4, Error handling, Testing, and Open questions to
match (commit
89117fc9).gate-proposalGitHub label does not yetexist on this repository and no available tool in this session can
create one without an unauthorized raw API write -- a human with
repo-admin access must create it once, manually, before this
mechanism's first real filing.
(
evals/merge-retrospective/tasks/carried-forward-gate-implemented-test.yaml,carried-forward-gate-unimplemented-train.yaml) test the OLD Step 1carry-forward-check mechanism this redesign removed entirely. Left
untouched, along with
evals/merge-retrospective/split.json/split.md-- retiring them properly requires updating the declared
train/selection/test partition arithmetic and equivalence-class
bookkeeping those files track, a separate, larger concern than this PR's
own scope.
gitapex_scan_execution_requirements_drift.pyreports a pre-existing (confirmed identical against this PR's own merge-base,
not introduced by this PR)
tools-write-vs-script-contentfinding for thisskill -- a false positive from AST-detecting
write_textcalls insidetest_gitapex_check_retro_gate_resolved.py's own tmp-path test fixtures,not a real behavioral write. Left as-is, consistent with this PR's own
narrow change-surface discipline.
issue fix(merge-retrospective): file missing-deterministic-gate findings as standalone issues so carried-forward debt can close #1406's own pre-reframe, narrower scope and cites
Closes #1406;a comment flagging the conflict is posted there. Per explicit operator
direction this PR proceeds regardless.
Assumptions
None beyond the issue/design doc. Files this design depends on were
re-confirmed current/unchanged as of this session's re-verification pass.
Risk / blast radius
Touches
skills/merge-retrospective/SKILL.md(an actively-used skill),one new script plus its tests,
.github/scripts/gitapex_scan_retrospective_gate_drift.pyand its CI workflow (kept read-only, no permission widened), and new
tests and eval fixtures.
.gitapex/ssot.jsongains only atracking_issuelist entry (
1406) and updatedrule/local_exclusion/targetprose forthe already-rescoped
retrospective-gate-drift-scangate -- no schema orenforcement-behavior change. Full accounting in the design doc's own
Constraints/Non-goals.
Rollback
git revertthis PR's merge commit. No schema or persisted-state changeto unwind; the
gate-proposallabel itself is inert if left on GitHubafter a revert.
Verification
Acceptance Criteria Map (from issue #1406) -- restated row by row:
SKILL.md's Step 1 sweep and Step 5 filing/closing must stop forcing a retrospective issue carrying carried-forward debt to stay open forevergitapex_check_retro_gate_resolved.pymust not changegate-proposallabel's literal name must not silently drift between its two copiesFull repo suite:
6750 passed(uv run --frozen python3 -m pytest --no-cov -q).Local pre-push preflight: 41/41 wired gates PASS.
Execution log
Wave 1, Wave 2, and Step 8 (refactor + adversarial review + this PR's own
skill-quality-audit dispatches) all complete; full repo suite green;
branch merged with
origin/mainrepeatedly as it advanced and pushed(force-with-lease once, for the history-rewrite remediation above; plain
pushes thereafter).
drafting-a-pr-to-merge's own two-layer independentreview (Step 8) complete -- see Independent review verdict below.
Skill audit evidence
This diff modifies
skills/merge-retrospective/SKILL.md(including itsfrontmatter
description:line), addsdocs/superpowers/specs/2026-08-29-flat-gate-proposal-issues-design.md,adds/modifies
.github/scripts/gitapex_scan_retrospective_gate_drift.pyand the new
skills/merge-retrospective/scripts/gitapex_file_gate_proposal.py(+ its test file), and modifies
.gitapex/ssot.json(a tracked deterministicgate's own registry entry).
battle-testing-a-skill: PASS -- three independent isolated dispatches(this repository's own verified
claude -p-from-isolated-cwd mechanism,reconfirmed current for this session's exact platform signals in
evaluating-skill-quality/references/adversarial-self-audit.md's Knownentries) cold-enumerated the 22-dimension catalog against the pre-fix
SKILL.md. 2 of 3 FAILed on dimension 13 (cross-session memorypoisoning) and 2 of 3 flagged dimension 16 (obfuscation coverage) as a
gap. Per this skill's own aggregation rule, a cross-trial disagreement
stays visible rather than resolved by majority -- rather than disclose
it unresolved, both findings were fixed in this PR (commit
0599e20c).A 4th, independent confirmation dispatch against the fixed text returned
a clean PASS on all 18 applicable dimensions (19-22 correctly N/A for
this domain), including explicit confirmation that both prior gaps are
now closed with concrete, mechanical language rather than a restated
warning.
evaluating-skill-quality: WELL-FORMED-NOT-MATURE -- one fresh isolateddispatch (same verified mechanism) ran the full Procedure: shape check
40/40 established in the main thread first and carried forward; Mechanism
fit clears with no whole-artifact finding (single-outcome sequential
cohesion); nine dimensions walked in order. Four named gaps block Mature:
dimension 2 (a sediment paragraph in Step 1 restating retired-mechanism
history not needed to execute the current step), dimension 5 (no
references/split; two full worked examples load on every invocation),dimension 6 (the sidecar drift above -- fixed in this same PR, commit
0599e20c), dimension 7 (the bundled script's own source was not givento this dispatch, so full compliance could not be verified from its
input alone; independently confirmed harmless by the main thread --
gitapex_check_retro_gate_resolved.pyis unchanged and zero-diffedacross this whole PR). Dimensions 8-9 are named explicitly as unmeasured
(no per-model eval data exists in this repository yet), which does not
by itself block Mature; dimensions 1, 3, 4 clear cleanly. The remaining
dimension-2/5/7 gaps are left as disclosed, out-of-scope follow-up,
consistent with this PR's own narrow-change-surface discipline.
adversarial-coverage-mapping: RAN --merge-retrospectiveissecurity-relevant (automated GitHub issue-filing) per this repo's own
keyword heuristic. Coverage: the battle-testing-a-skill dimension-13 FAIL
above (a real trust-boundary defect, found and fixed), plus the Step 8
adversarial-review pass's own 23-case defeat-probe suite against the
changed scripts (see checker-script-adversarial-review below).
design-doc-adversarial-review: RAN --docs/superpowers/specs/2026-08-29-flat-gate-proposal-issues-design.mdwent through 4 rounds of independent Fable-subagent adversarial review
before being finalized in this session, including one round that
surfaced 9 new issues from the prior round's own fixes, resolved by a
deliberate scope reduction (stripping secondary mechanisms) rather than
continued patching. A 5th round, run post-implementation as part of
drafting-a-pr-to-mergeStep 8's own independent review, found the dochad gone stale against the Step 8 SKILL.md fix (commit
0599e20c) andwas updated to match (commit
89117fc9).checker-script-adversarial-review: RAN -- the mandatory Step 8fresh-subagent adversarial review pass (pinned to a stronger-reasoning
model tier) constructed and ran a throwaway 23-case defeat-probe suite
against all three touched checker/script files, finding and fixing two
real defects:
_sanitize_cellescaped|but not\, letting analready-escaped pipe in free text add a live seventh column to the ACM
table; and
label_existspercent-quoted the label whilelist_labelled_issue_recordsinterpolated it raw, letting a labelcontaining a space or
&produce a request the liveness guard wouldnever actually be checking. Commit
fd2c14d9.deterministic-gate-quality: RAN --.gitapex/ssot.jsonandgitapex_scan_retrospective_gate_drift.pyread againstskills/evaluating-deterministic-gate-quality/references/dimensions.mddimension 15 (fail-closed default on incomplete or malformed input):
label_existsraises rather than silently reporting "missing" on anynon-404 status (persistent 5xx or other 4xx) -- confirmed by dedicated
non-happy-path tests
(
test_label_exists_raises_rather_than_reporting_missing_on_persistent_5xx,test_label_exists_raises_on_persistent_4xx_other_than_404,test_main_exits_one_on_label_exists_github_api_error), not resting onthe bundled happy-path test alone, per dimension 15's own requirement.
defeat-test-disclosure: RAN -- Task B's title-collision defeat test,Task D's label-drift defeat test (mutate one copy, confirm failure,
revert, confirm pass), the Step 8 pass's 23-case probe suite (2 real
defeats found and fixed, plus new permanent regression coverage for 3
near-misses that did not defeat but had no prior test), and this PR's
own post-hoc fix: a new eval fixture
(
gate-proposal-resumed-run-stale-filed-as-line-refiles.yaml, taggedadversarial) specifically defeat-tests the dimension-13 fix by staginga
Filed as:line whose referenced issue does not re-verify.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.mdKept-edit-log entry addedskills/*/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)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
Two-layer review per
drafting-a-pr-to-mergeStep 8:request_copilot_review; no requested reviewer or review appeared on the PR afterward, so this layer's coverage is not confirmed to have run -- disclosed rather than assumed clean. No Anthropic "Claude Code Review" App installation was confirmed for this repository either.skills/*/SKILL.md, a deterministic CI gate script, and non-deferred content (a design doc, a new pure-computation script and its tests, eval fixtures) --reviewing-an-artifact's own Step 0 mixed-target rule applies: each specialist-owned portion deferred to its own skill, the rest reviewed directly.evaluating-skill-quality(SKILL.md): WELL-FORMED-NOT-MATURE, withbattle-testing-a-skillPASS after a fix -- full detail already recorded in Skill audit evidence above; not repeated here.evaluating-deterministic-gate-quality(the CI gate script,.github/scripts/gitapex_scan_retrospective_gate_drift.py, and its.gitapex/ssot.jsonentry): a fresh, isolated dispatch walked the 25-dimension catalog. Result: well-formed, with only named non-blocking gaps (notimeout=on thegit logsubprocess call;owner/repointerpolated unquoted into constructed URLs, inconsistent with the script's own meticulous label-quoting elsewhere -- low exploitability since both are workflow-set constants, not PR-controlled; a claimed CI-step-summary write that could not be verified without the workflow YAML; no live-run evidence beyond mocked tests; no disclosed tamper-detection layer; and a deny-path UX gap where the threshold-exceeded and generic-exception failure messages name no concrete next action). The dispatch independently re-confirmed (not merely accepted) the fail-closedlabel_existsbehavior already disclosed underdeterministic-gate-qualityabove, and confirmed the closed-issue integrity check's two-signalorgenuinely covers both directions with dedicated defeat tests for each. Zero confirmed blocking findings.reviewing-an-artifact-equivalent dispatch (five review lenses -- correctness, blast radius, reuse, convention, security -- each candidate finding independently re-verified before being reported) covered the remaining diff (the design doc,gitapex_file_gate_proposal.pyand its tests,test_gitapex_retro_gate_label_sync.py, the record-format and GPRR test changes, and all 6 eval fixtures). It found one confirmed finding: the design doc still documented the pre-fix, insecure resumed-run behavior (trust aFiled as:line with no re-fetch) that this PR's own Step 8 fix (commit0599e20c) had already corrected inSKILL.mdand the eval suite -- a stale, misleading source of truth for anyone implementing or re-implementing the mechanism from the doc alone. Fixed by updating the design doc's Decision 1, Data flow step 4, Error handling, Testing, and Open questions sections to match the actual, corrected behavior (commit89117fc9). Two unconfirmed concerns were independently checked and resolved: a bare import in the new test file resolves via pytest's own same-directoryprependimport mode when invoked directly (matching Task B's own already-disclosed 67-test run and the already-disclosed, out-of-scope testpaths gap above), and no residual## Carried-forward gateheading remains inSKILL.md's worked example to be silently swallowed by the record-format test's updated section boundary -- confirmed absent by direct inspection.6750 passed) and the local pre-push preflight re-confirmed 41/41 after each of the two fixes above;origin/mainmerged in repeatedly as it advanced throughout this review, with no further conflicts.mergeable_stateis currentlyblocked, but only because theeval-gaterequired status check is red (failed twice with an unrelated "model CLI exited 1" crash) -- the operator explicitly instructed this session to leaveeval-gateuninvestigated and resolve every other CI red instead, which this PR has done. Every other required check is green. This verdict reflects the diff's own review outcome, not a claim that the PR is currently mergeable.Related Issue
Closes #1406