Repository navigation
Read an Unread Bare Push Branch as Every Protected Default in the Write Guard - #3764
Merged
Merged
Conversation
…te Guard A push naming no refspec, or HEAD, whose branch the guard does not read is now read as writing main, master, and develop, as --all is. That covers a repository the reading does not determine (a run-time cd or -C, an exported GIT_DIR, a directory with no branch to read) and a command that also runs a branch switch, since the guard reads the branch before the command runs. Drop the unused leading_cd and moved parameters from _bare_push_branch and repoint its Windows tests at the directory _cd produces. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A switch in the same command now adds every protected default to the branch read rather than replacing it, since a push before the switch writes the branch read. A one-upstream rebase, a symbolic-ref read, and a checkout of paths after -- no longer count as switching. Drop a dead argv guard and two dead mocks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A push before a same-command switch writes the branch read, so its denial now gives that branch's own reason rather than the unread-branch message. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
 |
Owner
Author
|
A recorded local strict-review pass covers head |
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Because the change alters a security-sensitive destructive-write classifier and fail-closed shell/Git parsing across multiple platform paths, maintainer validation of production command shapes remains necessary despite passing tests.
No open issues 🎉
What changed in this PR
This pull request hardens the Git write guard so unreadable bare pushes and pushes that may follow a branch switch fail closed against protected default branches.
Changes:
- Treat unresolved bare-push branches as writes to
main,master, anddevelop. - Detect branch-changing Git and GitHub CLI commands, including switches, rebases, symbolic-ref operations, stash branch, and
gh pr checkout. - Simplify bare-push resolution and update related tests and documentation.
- One unresolved moderate finding remains: clustered
symbolic-refdelete flags such as-qdare not detected.
| File | Description |
|---|---|
host-setup/agent-safety/README.md |
Documents fail-closed handling for unreadable branches and possible branch switches. |
host-setup/agent-safety/claude/test_install.py |
Updates Windows path tests and adds coverage for bare pushes after branch switches. |
host-setup/agent-safety/claude/gh-write-guard.py |
Implements branch-switch detection, protected-default fallback handling, and simplified bare-push resolution. |
Review effort: Lite
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #3764 +/- ##
========================================
Coverage 82.31% 82.31%
========================================
Files 28 28
Lines 9438 9438
========================================
Hits 7769 7769
Misses 1669 1669
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Owner
Author
|
Answering the review verdict on 81219c3, which names no finding and asks for maintainer validation of production command shapes. That validation is already this pull request's merge condition: it edits the agent-safety tree, so it is not merged by the driving agent and waits for the maintainer's go-ahead after he reads the diff. The command shapes it changes are each pinned in the guard's self-test in both directions (the switch movers, the narrowed non-movers, the loop-or-function order, and the run-time cd, -C, env -C, absent-directory, and GIT_DIR shapes), the self-test and test_install pass, and three recorded local strict-review passes over the whole diff ended with no open finding. Nothing the base guard denied is newly allowed. |
This was referenced Nov 1, 2026
Decide Whether Every Bare Push With an Undetermined Repository Fails Closed in the Write Guard
#3730
Closed
Closed
ptr727
added a commit
that referenced
this pull request
Nov 1, 2026
…xcluded-Files Refusal, and Fix backlog-burndown and Prose Gate Gaps (#3778) Promotes the changes merged into `develop` since promotion #3762 to `main`, the first round of a backlog burndown. - [#3764](#3764) reads a bare push whose branch the write guard cannot read as writing every protected default (`main`, `master`, `develop`), as `--all` is. That covers a run-time `cd`, `git -C` or `env -C`, an exported `GIT_DIR`, and a command that can switch the branch before the push. It also drops the unused `_bare_push_branch` parameters. Naming the branch in the refspec is the remedy, per the maintainer's decisions on #3729 and #3730. - [#3765](#3765) has the prose gate's diff mode report what a move adds by checking a moved file at its old path and its new path, replacing the hand-kept `path_readings` table. It also reads a script whole when an edited shebang changes its lexer. - [#3776](#3776) fixes `backlog-burndown`'s ranking, claim, liveness and cleanup gaps: - An attended run holds an unapproved agent-safety issue. - Claim comments name their branch in backticks and are released by an edit. - A silent worker counts as finished only with a merged pull request. - The orchestrator removes a standalone clone after its merge. - An abandoned worker discards its own uncommitted edits. - [#3775](#3775) reads Copilot's excluded-files refusal as its own `excluded` kind, which an attested local pass covers on a pull request into a non-default branch. It aligns Merge Gate item 2 with the code on an earlier-head refusal, adds two Copilot label spellings, and states the lineless-file allowance where the prose describes table coverage. - [#3779](#3779) deletes an abandoned group's pushed branch with a checked remote delete before its claim is released, and says the claim lookup's `<branch>` is replaced before it runs. It answers this promotion's first review round. - [#3781](#3781) has a release open the claim comment with `**Released (<date>):**`, and has the claim lookup skip a comment that opens with it. It answers this promotion's second review round. - [#3782](#3782) has `pr_review.py wait` poll a pending review request past an excluded-files refusal under `--request` as well. It answers this promotion's second review round. Fixes #3729 Fixes #3730 Fixes #3723 Fixes #3711 Fixes #3714 Fixes #3761 Fixes #1275 Fixes #1274 Fixes #1306 Fixes #1301 Fixes #3733 Fixes #3571 Fixes #2180 Fixes #3732 Fixes #3713 Fixes #3726 🤖 Generated with [Claude Code](https://claude.com/claude-code)
ptr727
added a commit
that referenced
this pull request
Nov 2, 2026
… merge Reads (#3793) ## Summary `pr_review.py merge` now reads an acknowledgment record for findings no thread carries. It also ends every run on one `next=` line, and the skills follow that field instead of listing every status line. The digest gives the excluded-files refusal its own `refusal=EXCLUDED` value. ### Acknowledgment record (#3718, and the verdict case from #3724) - New library module `scripts/pr_review_answers.py`. It makes no call and imports neither the gate nor a transport. It gives each answerable item a key. That is a SHA-256 prefix over the item's kind, the round or comment raising it, and the item's text. The items are: - each `suppressed`, `previously_missed`, and `cr_outside_diff` section on every round - each open Qodo entry - the head round's `FINDINGS WITH NO THREAD` block - each `REVIEWER OUTPUT NOTED` part - a verdict naming no counted finding One key covers every finding a collapsed section or block holds. A key holding a finding still awaiting the maintainer takes no `--ack`. A Qodo entry's key covers its title and text, since Qodo edits one comment in place. A reviewer login drift gets no key and takes no answer. `merge` refuses it as `REVIEWER_LOGIN_DRIFT` for the maintainer, since that login's rounds stay unread while the drift stands. - `pr_review.py comment --ack KEY=OUTCOME`, repeatable, posts the written outcomes with one `<!-- fleet-finding-ack: key=<key> outcome=<outcome> -->` line per item. Some bodies post nothing and exit `69`: one with no prose outside code (for `--fingerprint` too), one naming a key no item carries, and one with an outcome the item does not take. Both acknowledgment kinds count only where markdown-it finds prose outside code. - The outcomes are `fixed`, `declined`, `filed`, `listed`, and `dropped`, after `pr-review-conduct`'s verdicts. `Awaiting maintainer` has no outcome, so a value call stays the maintainer's. - A record counts only when it is all of these: - from an owner, member, or collaborator - in a comment that is not hidden - written beside outcomes in prose, not just the marker - outside code - `merge` clears a count only once every item under it carries an outcome. A partial answer, a new round raising the same text (new key), a `?` count, or a cut window still refuses. - A verdict naming no counted finding (`VERDICT_NAMES_UNCOUNTED_FINDINGS`, exit `50`) clears once its key is acknowledged. That is the case that blocked #3764 and #3765. It stays fail-closed: - An unanswered verdict still exits `50`. - `--ack` refuses `fixed` for a verdict, since a fix moves the head. - A headline naming a concrete failing input is a real finding under outcome 1, so the driver fixes it and pushes. - After a fix push, a head hold that was acknowledged turns into noted output. That output now reads as answered by its own round's shape acknowledgment. `shape_hold` takes the commit to read for this. A refusal's parts never read as answered this way. The shape acknowledgment and the finding acknowledgment now read a written outcome the same way, through `answering_comments`. - `report_verdict` moved from `pr_review.py` into the new module. That keeps `pr_review.py` under its ceiling (2196 lines before, 2166 now). The gate-item reading moved out of `pr_merge.py` as well (227 lines now). No ceiling was raised. The new module has its own ceiling, 450, and sits in the readers' import contract test (384 lines). ### `next=` route (#3724) - `merge` passes its output through and ends on `next=done|cleanup|readback|redrive|retry|maintainer`. One table in `pr_review_answers.py` decides it from each `status=` line, `wait`'s gate lines included. A line the table does not name goes to the maintainer. - A run naming several routes takes the first of `readback`, `cleanup`, `redrive`, `retry`, and `maintainer`, so what the seat can clear goes first. The skill keeps the one-rerun bound. - A generic `REVIEW_IS_A_REFUSAL` routes to the maintainer, since only a file-count refusal has a remedy this seat can apply, and the line does not say which kind it is. - A test pins every status line `pr_merge.py`, `pr_review.py`, and `pr_review_answers.py` print to its route. - `drive-pr` step 4 now follows `next=` and drops its per-status list. `scripts/README.md` "`pr_merge.py`" carries the route table. - The Qodo open item: an open Qodo entry is now answered by `--ack` and needs no `/review` re-review for `merge` to read. The untracked re-review wait therefore no longer gates a merge. ### Excluded-files refusal kind (#3777) - The digest prints `refusal=EXCLUDED` for the excluded-files refusal, apart from `refusal=YES`. - `scripts/README.md` now lists `excluded` among the adapter's refusal kinds. The `wait` exit-`41` paragraph now covers the `49`/`0`/`41` split for that kind and the earlier-head quota or error refusal under an attested later head. The `coverage=` paragraph already described the lineless-file allowance (from #3702), so it is unchanged. ### Skills - `pr-review-conduct` Merge Gate item 3 (verdict) and item 4 (noted parts), and "Answering a suppressed finding" (the `--ack` outcome mapping), agree with the code. The generated trees are regenerated. ## Verification - `tests/test_pr_review.py`, `tests/test_pr_merge.py`, `tests/test_pr_review_copilot.py`, and `tests/test_pr_review_size.py` pass, as do ruff, mypy, the prose gate (with the `comments` label), and `build_dist.py --check`. - New cases cover: - the clearing of each count - an untrusted, hidden, marker-only, unknown-outcome, or code-quoted record not clearing - a partial answer still refusing - per-round keys - shape-acknowledged noted parts - a verdict clearing as `declined` but not as `fixed` or from an outsider - `comment --ack` posting the marker, and refusing with `69` - `refusal=EXCLUDED` - each route, through `merge` and as a table pinned to the status lines in the sources Closes on promotion: #3718, #3724, #3777 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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
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.
Summary
The write guard reads the branch a push naming no refspec, or
HEAD, writes from the live checkout before the command runs. Until now, a bare push whose branch it could not read passed, unless an unrecognizedenvoption was involved. This change reads every such push as writing each protected-by-default branch (main,master,develop), as--allis. Live rules then decide, and an unreadable rule fails closed as before.cd,git -C, orenv -C, an exported or earlier-setGIT_DIR, and a directory with no branch to read. Before, only the unrecognized-env-option case was.git switch, acheckoutother than of paths after--, and arebasenaming the branch to rebase. They also includestash branch, abranchrename, asymbolic-refwrite, andgh pr checkout. A one-upstream rebase, asymbolic-refread, and a path checkout do not count. The mover counts anywhere in the text rather than only before the push, since a loop or a shell function can run a later switch before the push. The branch read keeps its own rule's reason in the refusal. Constructed case:git switch develop && git push originfrom afeature/acheckout was allowed while git pusheddevelop. It is now refused._bare_push_branchdrops its unusedleading_cdandmovedparameters.TestBarePushRootedDirectoryOnWindowsnow passes the directory_cdproduces ascwd, which isNonewhere a rooted path with no drive stays unresolved on Windows.host-setup/agent-safety/README.mdis amended to match.Accepted cost, per the maintainer's decisions on #3729 and #3730: such a push is refused even onto a feature branch, and naming the branch in the refspec is the remedy.
Verification
python3 host-setup/agent-safety/claude/gh-write-guard.py --selftestpasses. New rows cover each mover, the loop-or-function order, a wrapper body, a named refspec after a switch (allowed), a non-movinggit branch -d(allowed), and the run-timecd,-C,env -C, absent directory, wrapperGIT_DIR, and exportedGIT_DIRshapes. The old guard allows the switch rows it now denies.python3 -m unittest test_installinhost-setup/agent-safety/claude/passes. A newTestBarePushAfterABranchSwitchpins the read branch's own reason, and fails on the previous commit.tests/test_guard_size.pypasses. The guard is 2150 lines against its 2500 ceiling.Three recorded local strict-review passes ran over the whole diff. The first raised five findings, the second two, and the third none. One first-pass finding was declined as a wording preference: it asked that the shared refusal message name which cause applied. "Name the branch in the refspec" is a valid remedy for every cause. Each other finding is fixed on this branch.
This edits the agent-safety tree, so it waits for the maintainer's merge after reading the diff.
Closes on promotion: #3729, #3730, #3723
🤖 Generated with Claude Code