ums: do not recreate a merged PR head from empty ls-remote - #2341
Open
dem-extra1 wants to merge 3 commits into
Open
ums: do not recreate a merged PR head from empty ls-remote#2341dem-extra1 wants to merge 3 commits into
dem-extra1 wants to merge 3 commits into
Conversation
Empty ls-remote after squash-merge auto-delete looks like a first publish. A dry-run then prints [new branch]. Query that PR state first. Co-authored-by: dem-extra1 <dem-extra1@users.noreply.github.com>
Contributor
|
Driving this PR to clean --- please hold off until done. Posted by Cursor Grok 4.6 (AI agent) --- not written by a human. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
GitHub Claude on #2341: do not treat CLOSED as auto-delete, query gh pr list --state all --head rather than session recall, count this as the 2nd recreate failure not the 3rd, and restore the no-PR plain-push remedy in the compact copies. Co-authored-by: dem-extra1 <dem-extra1@users.noreply.github.com>
Contributor
|
Addressed GitHub Claude of
Do not merge. Blocked on review of Posted by Cursor Grok 4.6 (AI agent) --- not written by a human. |
Contributor
Built to branch |
Contributor
|
Claude finished review — View run All other checks are green; only this in-progress review job is pending (as expected, since I'm producing it now). I've now completed a thorough re-review. My conclusion: all 9 findings from the prior round were genuinely fixed in commit Code Review: PR #2341 — "ums: do not recreate a merged PR head from empty ls-remote"Context: This round follows a prior Claude review (posted 2026-08-26T20:34:00Z, reviewing commit Verification performed:
Additional independent checks (not just re-reading the diff):
Not a finding, but worth noting: a local working-tree modification to I found no new issues in this round. The prior round's findings were substantively fixed rather than merely reworded, and I could not find any regression introduced by the fixes. VerdictReady for merge. All 9 findings from the previous review round (2026-08-26T20:34:00Z, reviewing Reviewed commit: a59da18 |
Contributor
Collaborator
|
Status: the 20:52 hold above ("Do not merge. Blocked on review of 8af4edc") has resolved by its own condition --- the review of head a59da18 (which contains 8af4edc) arrived at 21:15 with verdict Ready for merge, independently verifying all 9 dispositions. CI is green and the branch is MERGEABLE (behind main, no conflict). That 20:52 comment was the prior driving session's status ledger, not a reviewer verdict; no findings remain open. Posted by Claude Code (AI agent) --- not written by a human. |
d-morrison
added a commit
that referenced
this pull request
Aug 27, 2026
…hipped Three corrections, one of them the reason this could not stand as written. The case record recommended inverting the driver-ledger gate to a POSITIVE signature -- the agent-disclosure marker -- on the premise that no reviewer report emits one. That premise is false: self-review-fallback.md requires a dispatched or cross-vendor review to be published verbatim with the marker appended, so a genuine not-clean review carries it too. The gate was built on fix/2409-driver-comments and dropped exactly such a review. Recommending it here would have walked the next reader into the hole this record exists to document, which is worse than having no record. Both attempts are now recorded together, with the reason they share: every discriminator available in a comment body is one some real reviewer also emits, so no body-shape test can safely decide to DROP an item. A positive signature is not safer than a negative one merely for being positive. The record now also says what shipped. Executing classify_verdict over the #2341 comment's parts showed neither the Disposition table nor the "Do not merge. Blocked on review of <sha>" hold produces a verdict at all -- the sole not-clean signal was the header's parenthetical citation of the round being disposed of. Both classifiers had been built to detect the parts that never mattered. The shipped fix neutralizes that one citation shape inside strip_cited_finding_vocab and drops nothing, so #2341's scan reports four examined items where the drop design reported two. Also: "neither finding was fixed" is no longer true -- #2430 covers both and both are fixed on fix/2409-driver-comments. CI fix: the quoted disclosure marker's own underscores tripped MD049, and the first attempt swapped them for asterisks, which silences the linter by corrupting the marker being quoted. It is a code span now, per examples-are-scanned.md's rule to render the example so it cannot match. A line opening with #2429 then tripped MD018 the same way and was rephrased. markdownlint: 0 errors over 702 files; check-links clean. The memories/ deletions the PR diff showed were a stale-base artifact, cleared by merging main.
d-morrison
added a commit
that referenced
this pull request
Aug 28, 2026
…r to guess (#2448) * fix: a driver session's ledger is not a reviewer verdict (closes #2409) * fix: harden ledger guards (label line, fingerprint, stripped markers, formal-review twin) with guard-exercising tests (round 2) * fix: emphasis-tolerant guards on the stripped body; per-guard and formal-twin tests (round 2) * fix: underscore emphasis in the guards; label-line and stub sole-protector fixtures (round 3) * fix: unified verdict-line guard (lookahead, not word-boundary) with emphasis fixtures (round 4) * fix: gate driver-ledger classification on the agent-disclosure marker (round 5) Round 4 classified a comment as a driving session's ledger from bare English markers (`hold off`, a `Disposition` table row) guarded only by NEGATIVE tests keyed on Claude's and Cursor's own report format. An adversarial round proved that fails open, and reproduced it against origin/main: - a Copilot report saying "hold off on merging until the null check is added", carrying a real finding, was swallowed and the PR reported FULLY CLEAN; - so was a Cursor not-clean whose verdict and fingerprint lines arrived blockquoted, which is the shape GitHub's own Reply control produces. Both are cases where the marker over-matches and every guard abstains, because a Copilot, Codex, or human reviewer emits none of the structure the guards look for. The fix inverts the gate: classification now requires a POSITIVE driver signature, the agent-disclosure marker, before any ledger marker is consulted. Every agent-posted forge comment carries it (CLAUDE.md mandates it on claims, releases, status, and disposition comments) and no reviewer report does; both measured #2341 driver comments end with it. A body without it is never a ledger, whatever else it says, so the gate errs toward admitting. Also: - `>` added to both leading character classes, so a blockquoted verdict or fingerprint still fires its guard -- which keeps the item ADMITTED, the conservative outcome; - the `_is_structured_review_body` guard removed as provably dead: its own fingerprint regex is a strict subset of the fingerprint guard above it, and neutering it failed no test; - the marker regex is locked to hooks/require-agent-disclosure.py's canonical MARKER_RE by a parity test. Every ledger fixture now carries the disclosure marker, because every real one does. Omitting it made them imitate an artifact that does not exist, and let the per-guard tests pass through the marker gate rather than the guard each was named for. Base parity, by execution across origin/main, b4b49e1, and this head: the two fail-open inputs veto here exactly as they do on base, and the measured #2341 ledger is the only behavioural difference from base. Per-guard neutering: all six guards (marker gate, verdict line, fingerprint, claude-finished, and the two blockquote tolerances) fail at least one test when neutered alone. Suite: 275 passed, 0 failed. * fix: neutralize an ARD ledger's citation of the round it disposes of (round 6) Replaces the whole drop-based `_is_driver_ledger` classifier, deleting ~97 lines. Nothing is removed from the verdict scan any more. Two adversarial rounds proved the drop design fails open, each on different grounds, and both were reproduced against origin/main: - round 5: a Copilot report saying "hold off on merging until the null check is added", carrying a real finding, was dropped and the PR reported FULLY CLEAN; so was a Cursor not-clean whose Verdict and Reviewed-Commit lines arrived blockquoted, the shape GitHub's Reply control produces; - round 6: gating on the agent-disclosure marker instead did not help, because the marker's premise was false. `self-review-fallback.md` requires a published cross-vendor review to carry that marker, so a genuine not-clean review carries it too. Both rounds landed on the same underlying point: every discriminator available in a comment body is one some real reviewer also emits, so no body-shape test can safely decide to DROP an item. Measurement redirects the fix. Executing `classify_verdict` over the #2341 ledger's parts shows the header citation alone is what made it not-clean: Addressed GitHub Claude of `9508454e` (Needs more work). -> not-clean | # | Tag | Disposition | ... (the table) -> no verdict Do not merge. Blocked on review of `8af4edc9`. -> no verdict So the `Disposition` table and the self-imposed hold, which both earlier designs were built to detect, never mattered. The defect is one citation shape `strip_cited_finding_vocab` does not know: a prior round's verdict in a bare parenthetical, with no quotes and no bold. The new rule is a fourth citation shape in that function, gated on BOTH signals the way the #1752 rule was after its own two over-broad versions were refuted: the sentence must OPEN with an ARD disposition verb, which a live finding never does, and the parenthetical must follow a backticked SHA immediately and hold NOTHING BUT the verdict phrase. The second is what keeps a reviewer rejecting a claimed fix -- "Addressed in `abc1234` (still Needs more work)" -- out of the match. Only the parenthetical is blanked. Base parity, by execution across origin/main, b4b49e1, 0c33927, and this head: all three prior-round exploits and a reviewer-rejects-the-fix control veto here exactly as they do on base, and the measured #2341 ledger is the only row where this head differs from base. Per-component neutering: dropping the strip, the disposition-verb anchor, the cited-SHA requirement, or the exact-paren-body requirement each fails the test named for it. Two first-draft mutants were discarded as invalid -- one emitted a malformed regex and one widened the paren in the direction no test guards -- which is the same wrong-branch trap Pattern 15's extension names. Live: #2341 FULLY CLEAN, and its scan still examines 4 items rather than the 2 the drop design left, so the ledger stays visible in the count. #2313 still correctly NOT clean. Suite: 267 passed, 0 failed. * fix: backtick a quoted verdict in ARD summaries, instead of guessing in the checker (closes #2409) Reverts every checker change on this branch, leaving scripts/ untouched, and fixes #2409 at the author's end instead. Three checker-side designs were built here and deleted, each refuted by an adversarial round that reproduced its failure against origin/main: 1. a driver-ledger classifier keyed on claim wording and a Disposition table row, guarded by negative tests keyed on Claude/Cursor report format. It dropped a Copilot review saying "hold off on merging until the null check is added" and reported the PR FULLY CLEAN. 2. the same classifier gated on the agent-disclosure marker. Refuted because self-review-fallback.md requires a published cross-vendor review to carry that marker, so a genuine not-clean review carries it too. 3. a citation strip keyed on the disposition verb plus a parenthetical holding nothing but the verdict phrase. It blanked the verdict in "Addressed the null-check nit in `9508454e` (Needs more work): the fix introduced a new NoneType dereference at foo.py:42", where the parenthetical IS the live verdict and the explanation sits outside the blanked span. All three failed for one reason. Every discriminator available in a comment body is one some real reviewer also emits, so no body-shape test can separate a driver citing a stale round from a reviewer rejecting a claimed fix -- the two write the same sentence. Measurement says the fix is one pair of backticks. Executing classify_verdict over the #2341 comment's parts shows neither the Disposition table nor the "Do not merge. Blocked on review of `8af4edc9`" hold produces a verdict; the sole not-clean signal was the header's bare parenthetical. `strip_cited_finding_vocab` already blanks inline code spans as citation (#1202), so backticking the quoted phrase neutralizes it through machinery that is already shipped and reviewed, and adds no new fail-open surface to the instrument at all. When a driver forgets, the PR reads not-clean -- the recoverable direction, and one the driver sees on their own PR. The guard that would catch a forgotten pair of backticks at authoring time is filed as #2443 rather than built here, since it is a separate artifact with its own README-catalog and activation-gate requirements. markdownlint 0 errors over 702 files; check-links clean; the checker's own suite is main's, unmodified, at 252 passed. * fix: a bare verdict in a table cell trips the same wire as the header line Review finding: the convention's only worked example was the opening line, so an author pattern-matching from it would backtick that and miss a disposition table cell or a Rebuttal paraphrase, which carry the identical bug shape. Verified by execution against the checker: | 1 | Reviewer said Needs more work on the null check | ... | -> not-clean | 1 | Reviewer said `Needs more work` on the null check | ... | -> no verdict The "wherever it appears" wording already covered this, and wording is not what an author copies from. A second example and a sweep-before-posting step are.
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
3 participants
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.
UMS from wrapping the session that drove #2272.
git ls-remotegoing empty after a squash-merge auto-delete looks like a first publish. A--dry-runofgit push -u origin <branch>then prints* [new branch](and oftenWould set upstream of ...). That is the recreate telluse-existing-pr-branchalready names for a live push. Query the PR'sstatebefore pushing. MERGED or CLOSED: do not recreate; open a follow-up offorigin/<default-branch>(post-mergestep 1.25).3rd occurrence of this class (2026-08-26 PDT, Cursor Cloud, #2272 Address dry-run). Prior:
memories/git.md#857 -> #872; the live-push tell inuse-existing-pr-branch.md.Does not edit
memories/git.md(at the 1200-line cap; split tracked as #2328).Do not merge until a clean review verdict lands on the latest commit.