Skip to content

feat(source-control): lift canary, revert and read-back demotion records on lane evidence - #12905

Merged
kyle-sexton merged 17 commits into
mainfrom
feat/11022-demotion-record-lift
Nov 3, 2026
Merged

kyle-sexton merged 17 commits into
mainfrom
feat/11022-demotion-record-lift

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Nov 3, 2026 •

Copy link
Copy Markdown
Member

Closes #11022

Summary

Done-when items 5 and 6 of #11022, per the 2026-11-02T14:10Z decision on the issue: canary, revert and read-back demotion records lift on lane-checked evidence, and a lane then closes them and removes needs-human. Items 1 to 4 shipped in #12279 and #12378.

Fix

  • Lift rule (babysit_merge_policy.py _lift): a record lifts when (1) the content whose deletion filed it is on main again at the record's paths (for a read-back record: its own PR reads back), or (2) a PR that references the record merges into main after it was filed and the record's own condition re-checks on the merge commit. A reference counts only when it was made no later than the PR's merge and its actor is a write-access human or the agent identity. A failed read, more than 100 references or comments, or a record a non-filer edited lifts nothing.
  • Lift blocks: the alert writes <!-- lift-content v1 {...} --> into canary and revert records, listing per path the hashes and counts of the lines the push deleted (canary, from check-silent-revert.sh's new opt-in DELETED_SINK) or the reverted PR added (revert). A canary record whose failure deleted nothing (self-test, replay or restoration step) lifts by condition 2 on a green canary run at the referencing merge.
  • Close: close-cleared checks out main and runs the new babysit_lift.py close, which mirrors _lift, closes the record, removes needs-human, and pins the evidence commit in its close comment so a later edit on main cannot re-open the hold.
  • One exception, documented: a canary record with no lift block (the pushed commits are not in main's checkout, the push range was cut or guessed, the scan exits 2, or the block exceeds 40,000 characters) still clears only on a person's close.
  • Docs (item 6): the pr-pipeline README and the record bodies state the new rule, the exception, and that read-back condition 2 fires only when a pin exists for the referencing merge.

Verification

  • test_babysit_merge_policy.py: 566 OK (19 RecordLift tests). test_babysit_lift.py: 16 OK, on real git fixtures, the canary case running this tree's check-silent-revert.sh. test_babysit_postmerge.py: 331 OK.
  • scripts/post-merge-demotion-alert.test.sh PASS=277; scripts/check-silent-revert.test.sh PASS=137; alert node tests 68/68; ruff clean.
  • Sabotage checks: with _lift disabled all 6 original lift cases fail; with _content_present forced false all 7 content-lift cases fail; ignoring the evidence pin fails the lane-closed case.
  • The PolicyRecordLift GraphQL query ran read-only against the live API (issue 11022, PR 12279): no errors, 17 cross-references, 12 comments.

Related

🤖 Generated with Claude Code

kyle-sexton and others added 7 commits November 3, 2026 12:27
…ane evidence

The merge gate now lifts a revert or read-back demotion record on its own
evidence (babysit_merge_policy.py `_lifted_at`), as it clears a red-main
record once main is green. Either record lifts once a pull request that
references it merges into main after it was filed, at a merge commit on
main's history; a read-back record also needs that pull request to read
back (the agent identity's newest pin is unedited, posted no later than the
merge, and names the head that merged), and lifts too when the pull request
it names reads back after all. A non-human edit, a failed read or more than
100 references lifts nothing. A canary record stays human-close: its body
names no paths, so the content-on-main condition cannot be checked.

The read-back and revert record bodies and the pr-pipeline README state the
new rule.

Refs: #11022

Co-Authored-By: Claude Opus 5.5 <[email protected]>
… a lift changes

The lift's read-back now counts a pin only when its author is the agent
identity by the judge's own test (`actor_key`), so a User owning the App's
bare slug never stands in. The README and the alert's header say that a
lift stops the merge gate holding but leaves the record open with its
label, and the effective rung reads off while a demotion issue holds, not
while one is open.

Refs: #11022

Co-Authored-By: Claude Opus 5.5 <[email protected]>
…se lifted records

A canary or revert record now carries a lift block: per path, a short
SHA-256 and count for each line the merge deleted, written by the alert from
scripts/check-silent-revert.sh's new opt-in DELETED_SINK over the pushed
commits, or from the lines the revert commits deleted (babysit_lift.py
canary-block, revert-block). A canary record remediate filed first gets the
block as a filer comment.

The merge gate's `_lift` lifts such a record once every counted line is in
main's tree again, or once a referencing pull request merges into main with
that content at its merge commit; a canary record whose block says nothing
was deleted needs a passing canary push run there. A canary record with no
block stays human-close, the one exception.

close-cleared checks out main and runs babysit_lift.py close, which
comments on, removes the human-gated label from and closes each lifted
canary, revert or read-back record.

Refs: #11022

Co-Authored-By: Claude Opus 5.5 <[email protected]>
A canary or revert record that lifted because its deleted content was back
on main checked main's tip, which moves: once the lane closed it, a later
edit to one of those lines would make the record hold the rung again on a
closed issue with no label. babysit_lift.py close now writes the commit it
checked as a `<!-- lift-evidence sha=<S> -->` line in its close comment, and
`_lift` checks the content at that commit, when a filer wrote the comment
unedited and the commit is on main at or after the record's SHA. The README
names the third way a canary record ends up without a block.

Refs: #11022

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@kyle-sexton
kyle-sexton marked this pull request as ready for review November 3, 2026 18:43
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-03T18:54:04.121788Z 828b6b8 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 828b6b89c5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread scripts/check-silent-revert.sh Resolved

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

claude-review: 3 inline finding(s) on 828b6b89c5e8161c21ce1eccd19161540708d573; the summary is in this lane's pull request comment.

claude-lane: claude-review

Comment thread scripts/check-silent-revert.sh Resolved
@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

claude-review reviewed a61925bc8384a52e4571d9f293f3528d8c8a64a4: read 5 of 5 changed file(s); 0 finding(s), 0 as inline comments.

Code review (incremental, 9c6afc1 → a61925b): restore measured against the record's own SHA

Scope: I read every hunk of all 5 files in .claude-lane/incremental-files.txt. For context I also read _lift and _raw_at in babysit_merge_policy.py, and the alert workflow lines that write the record marker. This is a complete incremental review.

Prior finding is fixed: _content_present no longer counts a copy the deletion left behind as a restore.

  • It now requires have[h] >= left[h] + n, where left counts the lines of the path's file at the record's SHA.
  • Both _lift call sites pass the record's sha: the main-tip/pinned check and the referencing-merge canary check.
  • The record's SHA is a valid baseline. The alert workflow writes sha=$RUN_SHA into the marker (post-merge-demotion-alert.yml L688 and L922), and that is the pushed head, i.e. the state just after the deletion.
  • A file missing at the base counts nothing.
  • A failed base read raises PolicyReadError through _raw_at, so the record holds (fails closed).

_line_counts matches git's line count: a final newline no longer adds an empty line, so the deleted-blank-line case from the prior finding is closed too.

Edge cases I checked:

  • Pinned SHA equals the record's SHA: ref == base, so the check can never pass. That is the right result, and the lane close can't write such a pin anyway, because it calls the same _lift.
  • Later commit in the same push re-adds a deleted line: the baseline then counts that copy as left behind. The record holds where it might have lifted, which is the safe direction.

Tests:

  • test_a_copy_the_deletion_left_behind_is_not_a_restore covers the moved-line case.
  • test_a_final_newline_is_no_restored_blank_line covers the final-newline case.
  • test_a_copy_the_deletion_left_behind_closes_nothing covers the lane close.
  • Each test sets the record-SHA file read on its own, and fake defaults it to 404.
  • The Gh helper in test_babysit_lift.py now answers any other /contents/ read with 404, which reads as "absent" and matches _raw_at's semantics.

I ran no tests. The owner's statement that both new merge-policy tests failed on 9c6afc1 is their claim, and I did not confirm it.

Docs: The pr-pipeline README (L1165-1170) and the source-control changelog fragment both state the new rule. The record bodies' wording ("once the reverted lines are back on main") still fits.

Closing issue #11022: no change to the previous round's assessment. Item 5's open gap (finding 1) is now fixed and has tests that could fail. Item 6 is met.

Manifests: none changed. The fragment edit keeps bump: minor.

Path rules: nothing in this increment breaks .claude/rules/skill-bodies-state-current-rules.md, cost-claims.md or ruff-pin.md.

API usage (not counted as a finding): each lift check now makes one more contents read per path in a record's lift block, for the baseline at the record's SHA. This spends GitHub API rate budget only and costs no money.

I found no embedded instructions in the PR context, settled threads or prior findings.

No new findings.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

claude-security-review: 1 inline finding(s) on 828b6b89c5e8161c21ce1eccd19161540708d573; the summary is in this lane's pull request comment.

claude-lane: claude-security-review

@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

claude-security-review reviewed a61925bc8384a52e4571d9f293f3528d8c8a64a4: read 5 of 5 changed file(s); 0 finding(s), 0 as inline comments.

Security review: PR #12905, incremental from 9c6afc19 to a61925bc

No security issues found in this push.

critical-findings: 0

I read every hunk of all 5 files in the incremental scope: the changelog fragment, docs/conventions/pr-pipeline/README.md, babysit_merge_policy.py and its two test files. babysit_merge_policy.py is a trusted path and feeds a merge hold, the demotion-record lift. I read its whole change and what it gates: _lift → _content_present → _raw_at.

What changed. _content_present now needs each deleted line to be back on main beyond the copies the file still held at the record's own SHA. It also stops counting a final newline as a restored blank line (_line_counts). Both changes make the merge hold harder to lift, never easier:

  • Copies at the record's SHA. Condition have[h] < left[h] + n with left >= 0 is at least as strict as the base's have[h] < n.
  • Final newline. The old text.split("\n") counted a trailing empty string as a blank line. Dropping it can only lower have for the empty-line hash.
  • The base read fails closed. When the file is missing at the record's SHA (a 404), left is zero, which matches the old behaviour. Any other failed read raises PolicyReadError (_raw_at, L4774-L4777), so the record keeps holding.
  • No new query input. The new base argument is the record's sha, parsed by the LIFTABLE_RECORD regex from the first line of an unedited record (L2216-L2219). That value already went into the ahead/on_main checks. path is still quote()d in _raw_at (L4767), so the extra read adds nothing an attacker can steer.

The test changes (a 404 fallback for contents reads, plus new cases for left-behind copies and the final newline) and the prose updates in the changelog and README match the code and carry no security surface.

Prior findings. My round-1 IMPORTANT finding (any commenter's cross-reference could lift a revert record) was already fixed at 9c6afc19, as the last round reported. This push doesn't touch that code, so there is nothing to post again. The round cap does not apply: 1 round with findings, cap 3.

Instruction surface. The README and changelog edits narrow when a record lifts and remove no standing instruction.

Spend. None: the change adds at most one extra GitHub contents API read per lift-block path checked.

kyle-sexton and others added 2 commits November 3, 2026 14:56
test_guards.py requires every executable script under babysit-prs/scripts to
carry a guard-contract classification. babysit_lift.py is conditionally
mutating: only `close` writes to GitHub (comments, removes the human-gated
label, closes), and `canary-block` and `revert-block` read the local
checkout. Two mechanism rows back that, and its subcommand parsers now
refuse abbreviated flags like the top-level one. guard-contract.md is
regenerated.

Refs: #11022

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_015te8CsyoyMhoEcPAAHxicU
kyle-sexton and others added 7 commits November 3, 2026 15:10
… checkout

The alert calls `babysit_lift.py canary-block --repo-dir .base`. The scan
runs with `.base` as its working directory, so the relative script path
became `.base/.base/scripts/check-silent-revert.sh`, the scan exited 2, and
every canary record fell into the human-close exception. The script path is
now resolved before the scan starts.

Refs: #11022

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_015te8CsyoyMhoEcPAAHxicU
…ce sink

scan_commit built each attribution row with awk's `$2`, so a deleted line
was cut at its first tab: a tab-indented line became empty and `a\tb`
became `a`. DELETED_SINK copies those rows as the deleted content, so a
record of tab-indented deletions counted empty lines, and blank lines on
main could lift it with the code still missing. The row now keeps
everything after the culprit's tab. Per-culprit counts and the recorded
incidents replay unchanged.

Tests for this and the relative `--repo-dir` canary scan.

Refs: #11022

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_015te8CsyoyMhoEcPAAHxicU
…ment posted

close_lifted posted the comment, removed the label and closed the record
before it checked any result. A failed comment with a successful close left
a closed record with no lift-evidence pin, which close_lifted never lists
again, so a later edit on main could make it hold the rung with no open
issue behind it. A failed comment now leaves the record open and labeled
for the next run.

Refs: #11022

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_015te8CsyoyMhoEcPAAHxicU
…not read whole

push_range keeps the newest 20 commits of a longer push, and guesses the
newest 20 when the previous canary push run's head does not read. A canary
block built from that range can leave out the deleting commit; an empty
block then reads as "nothing was deleted" and the record lifts on any
referencing merge with a green canary run. canary-block now runs only when
the range is the whole push; otherwise the record carries no block and
stays human-close, the documented exception. Revert blocks are unchanged:
they cover only the revert commits found in the range.

Refs: #11022

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_015te8CsyoyMhoEcPAAHxicU
…erge as a lift

Condition 2 of `_lift` accepted any cross-reference from a pull request
merged into main after the record. GitHub also makes a cross-reference when
anyone mentions the record in a comment on a pull request, merged or not,
so on this public repository any account could lift a revert record, and a
canary record whose block says nothing was deleted (a green canary run at
the merge commit is the usual case). The reference must now have been made
no later than the merge, by a person with write access or the agent
identity. The lift query reads each reference's actor and time; a live
probe returned both.

Refs: #11022

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_015te8CsyoyMhoEcPAAHxicU

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

claude-review: 1 inline finding(s) on 9c6afc19193cc686a4e6d495a8e3ab68b8f27bf3; the summary is in this lane's pull request comment.

claude-lane: claude-review

… restore

`_content_present` counted every copy of a deleted line at the ref, so a
copy the deletion itself left behind counted toward the restore: a revert
of a pull request that moved a line within a file records the line deleted
and adds it back elsewhere, so the record lifted at creation and the close
closed it. Common lines and a final newline (which split("\n") counted as a
blank line) did the same. A line now counts as restored only beyond the
copies its path held at the record's own SHA, the state just after the
deletion, and lines are counted as git counts them. `babysit_lift.py close`
runs `_lift`, so the close path takes the same rule.

Refs: #11022

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_015te8CsyoyMhoEcPAAHxicU
@kyle-sexton
kyle-sexton added this pull request to the merge queue Nov 3, 2026
Merged via the queue into main with commit 318405f Nov 3, 2026
52 checks passed
@kyle-sexton
kyle-sexton deleted the feat/11022-demotion-record-lift branch November 3, 2026 21:38
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\(pr-pipeline\): align every needs-human path with the owner stop rule - part 4 of 6

1 participant