Skip to content

Promote Develop: Fail Closed on an Unread Bare Push, Read Copilot's Excluded-Files Refusal, and Fix backlog-burndown and Prose Gate Gaps - #3778

Merged
ptr727 merged 7 commits into
mainfrom
develop
Nov 1, 2026
Merged

ptr727 merged 7 commits into
mainfrom
develop

Conversation

@ptr727

@ptr727 ptr727 commented Nov 1, 2026 •

Copy link
Copy Markdown
Owner

Promotes the changes merged into develop since promotion #3762 to main, the first round of a backlog burndown.

  • #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 Read a Bare Push's Branch After an Earlier Branch Switch in the Same Command in the Write Guard #3729 and Decide Whether Every Bare Push With an Undetermined Repository Fails Closed in the Write Guard #3730.
  • #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 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 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 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 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 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

ptr727 and others added 4 commits November 1, 2026 11:14
…te Guard (#3764)

## 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 unrecognized `env`
option was involved. This change reads every such push as writing each
protected-by-default branch (`main`, `master`, `develop`), as `--all`
is. Live rules then decide, and an unreadable rule fails closed as
before.

- **#3730, fail closed for all.** A bare push whose repository the
reading does not determine is now read that way. That covers a run-time
`cd`, `git -C`, or `env -C`, an exported or earlier-set `GIT_DIR`, and a
directory with no branch to read. Before, only the
unrecognized-`env`-option case was.
- **#3729, a branch switch in the same command.** A bare push in a
command that can switch the branch is read as writing the branch read
plus every protected default. The movers are `git switch`, a `checkout`
other than of paths after `--`, and a `rebase` naming the branch to
rebase. They also include `stash branch`, a `branch` rename, a
`symbolic-ref` write, and `gh pr checkout`. A one-upstream rebase, a
`symbolic-ref` read, 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 origin` from a `feature/a` checkout was
allowed while git pushed `develop`. It is now refused.
- **#3723.** `_bare_push_branch` drops its unused `leading_cd` and
`moved` parameters. `TestBarePushRootedDirectoryOnWindows` now passes
the directory `_cd` produces as `cwd`, which is `None` where a rooted
path with no drive stays unresolved on Windows.
- The refusal message now names both causes, and requirement 4 in
`host-setup/agent-safety/README.md` is 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 --selftest`
passes. New rows cover each mover, the loop-or-function order, a wrapper
body, a named refspec after a switch (allowed), a non-moving `git branch
-d` (allowed), and the run-time `cd`, `-C`, `env -C`, absent directory,
wrapper `GIT_DIR`, and exported `GIT_DIR` shapes. The old guard allows
the switch rows it now denies.
- `python3 -m unittest test_install` in
`host-setup/agent-safety/claude/` passes. A new
`TestBarePushAfterABranchSwitch` pins the read branch's own reason, and
fails on the previous commit.
- `tests/test_guard_size.py` passes. The guard is 2150 lines against its
2500 ceiling.
- ruff format, ruff check, and mypy are clean.

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](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…de (#3765)

## Summary

The prose gate's diff mode could report clean on findings a change
introduced without editing the lines they sit on. This happened in two
cases.

- **A move that changes a path-dependent rule (#3711).** A moved file
whose lexer is unchanged is now checked twice: once as if it still sat
at its old path, and once at its new path. Outside the lines the change
edits, a run reports only the findings the new path adds. That covers a
move off a Compose name, which exposes container-side home paths. It
also covers a move into another directory, which can turn a relative
mention dead. An older finding that both paths report stays unreported.
This replaces the hand-kept `path_readings` table, so the
`file_exemption` and `issue-ref` cases it held now go through the same
comparison. A rule that later gains a path dependency needs no new table
entry.
- **An edited shebang that changes the lexer (#3714).** `changed_lines`
now rebuilds the file's old first line from the diff's hunks. It reads
the file whole when the lexer at its old path and old first line differs
from the lexer at its new path and new first line. A move whose lexer
changes is read whole too, as before. The check now also catches a move
that edits the shebang along the way.

## Tests

The new cases fail on `develop` and pass here, all in
`TestChangedLines`:

-
`test_a_move_off_a_compose_name_reports_the_container_paths_it_exposes`
- `test_a_move_to_another_directory_reports_the_mentions_it_turns_dead`
- `test_an_edited_shebang_changing_the_lexer_reads_the_script_whole`
covers a changed, inserted, or deleted shebang, a shebang edit that
keeps the lexer, a body edit, and a move with a shebang edit run end to
end.
-
`test_a_moved_file_is_judged_where_it_sat_by_the_first_line_it_had_there`
covers a move onto a suffix that keeps the lexer while the shebang
changes. The run at the old path reads the old first line, so no older
finding comes back. A BOM subtest in the shebang case checks that a byte
order mark is dropped before the lexer comparison, as `check_file`
already does. The local strict-review pass raised both of these, and
both are fixed in the second commit.
- `test_a_move_into_instruction_text_reports_its_issue_references`
checks that the comparison still covers the case `path_readings` used
to.

The existing move cases now expect a pure move's destination in scope
with no lines and its source recorded in `moves`.

`tests/test_prose_lint.py`, `test_repo_gate.py`, `test_cli_usage.py`,
`test_pre_commit_hook.py`, and `test_tooling_encoding.py` all pass
locally. So do the hook's ruff format and check, mypy, and the prose
gate.

Closes on promotion: #3711, #3714

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
## Summary

Fixes gaps in `backlog-burndown`'s ranking, claim tracking, liveness
read, and cleanup.

- **Ranking (#3761).** An attended run now holds back an issue whose fix
edits `host-setup/agent-safety/` until an approval is recorded on it,
and asks the maintainer for one otherwise. This matches the hold "Needs
the Maintainer" already gives an away run.
- **In-flight enumeration (#1275).**
- A claim comment writes its branch name in backticks, and a `gh issue
list` command walks from an enumerated branch back to its claim.
- A branch no claim covers is read against its own base, the pull
request's `baseRefName` or `develop`, rather than a hardcoded
`origin/develop`.
- The text now says why `Closes on promotion: #N` closes nothing: GitHub
reads a closing keyword only where the issue reference follows it
directly.
- **Claim release and "finished" (#1274).** The orchestrator records a
release by editing the claim comment, once removal succeeds. "Finished"
now means merged everywhere it appears.
- **Liveness read (#1306).** A silent worker counts as finished only
with a merged pull request. A dead worker's clean tree over commits
develop lacks is retired alone, and a fresh worker is dispatched on the
same branch rather than stopping for the maintainer.
- **Standalone clone (#1301, decided in #3742).** The worker reports its
checkout path, and after a merge the orchestrator removes a standalone
clone at that path per `repo-worktree` "Listing and Cleanup". It first
confirms the clone is clean and at the pull request's `headRefOid`.
- **Abandoned worker (#3733, decided).** A worker told to abandon
discards its own uncommitted edits in its own linked worktree before
reporting. The skill text grants that discard on the orchestrator's
abandon instruction, so the orchestrator's unforced `git worktree
remove` and `git branch -d` then succeed. No agent-safety edit.

## Local Review

The local `frontier` pass raised 11 findings. The fix commit addresses
the introduced ones: the claim lookup now trusts only visible
write-access comments, a dead-worker redispatch is bounded to once, the
clone check has a failure branch, and the closing-keyword and
discard-grant wording is corrected. The second pass raised one wording
finding, fixed in the following commit. The third pass, over the first
pushed head, raised none. After the Copilot round, later passes found
that the release form had to be stated once for every release site and
match the promotion-hold test. Those were fixed in e194c98, 71fbfd8,
and 3fe9d12, and the last pass raised none.

### Pre-existing findings, filed

- `.agents/skills/backlog-burndown/SKILL.md` "Bounding the Wait on a
Worker" and "Cleanup Is the Orchestrator's": a dead or abandoned
worker's standalone clone has no path (`pre-existing`). Filed as #3771.
- `.agents/skills/backlog-burndown/SKILL.md` "Grouping and File Claims":
the agent-safety hold is not reapplied when a group's file set grows
(`pre-existing`). Filed as #3772.

### Deferred

- `.agents/skills/git-commit-conventions/SKILL.md` and `GOVERNANCE.md`
"Git and Commit Rules" do not name the abandon-discard exception this
change adds. Fixing that is outside this change's files, so it is filed
as #3773.

### Dropped findings

- `.agents/skills/backlog-burndown/SKILL.md` "Ranking", the
stopped-group release sentence names no actor (wording, untouched text).

`.github/skills/` and `.claude-plugin/` are regenerated with
`scripts/build_dist.py`, and `--check` is clean.

Closes on promotion: #3761, #1275, #1274, #1306, #1301, #3733

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
… Gate Item 2 (#3775)

## Summary

- **Excluded-files refusal (#2180, decided via #3740).** Copilot's
"wasn't able to review any files in this pull request" refusal now reads
as its own `excluded` refusal kind in `scripts/reviewers/copilot.json`.
It no longer blocks `local_cover`, so on a pull request into a
non-default branch the attested local pass covers the head. `wait` exits
`49` until that pass is attested and `0` after. Where no local pass can
cover the head, as on a promotion, `wait` exits `41` with its own
`status=REVIEW_EXCLUDED_FILES_ONLY` line. That line says neither
re-requesting nor splitting clears it and hands it to the maintainer.
- The body-less liveness read takes a head refusal for a finished round.
So when it reports the head done, `wait` reads the full payload once and
reuses it for the final read. The new `polled_past` helper then decides
whether an excluded-files refusal holds the head for its local pass, or
waits on a pending request. A held head gets the same check poll as any
other.
- **Earlier-head refusal (#3571).** The pr-review-conduct Merge Gate
item 2 text now says what the code already does. On a pull request into
a non-default branch, an attested local pass on a later head clears an
earlier-head quota or error refusal, `coverage=local`. The `wait` help
text says the same. A test pins the error-refusal case beside the
existing quota one.
- **Copilot label spellings (#3732, #3713).** `review findings remain:`
and `open review findings:` are added to the Copilot reader's `labels`
and `finding_lists`, with test cases.
- **Lineless-file allowance (#3726).** Item 2, the canonical
`.github/copilot-instructions.md` coverage paragraph, and
`GOVERNANCE.md` "PR Review Etiquette" now say a file table may leave out
a file with no changed line, such as a binary. The same sentence was
missing from that `GOVERNANCE.md` section, so it is fixed there too. The
copilot-instructions paragraph also names the excluded-files refusal as
the one refusal on the head that does not block the local-pass reading.
- The vocabulary test now checks that each reviewer file's refusal kinds
are a subset of `REFUSALS`, with Copilot's file carrying all of them.
That lets the CodeRabbit and Qodo files leave out a kind their reviewer
never posts.

## Local pass findings

- `introduced`, scripts/pr_review_rounds.py `polled_past`: the excluded
branch did not require that no Copilot round covers the head. Fixed in
6a0b134, with a test that fails without it.

## Deferred, filed

- `introduced`, scripts/pr_review_digest.py `refusal_field`: an
excluded-files refusal prints `refusal=YES`, like a file-count one.
Deferred to #3777, since that file sits outside this change's file
claim.
- `introduced`, scripts/README.md lines 232 and 254: the refusal kinds
list and the `wait` exit `41` paragraph leave out the excluded kind and
the later-head local cover. Deferred to #3777 for the same reason.

## Not fixed here

#3758 stays open. `run_wait` already requests nothing on a pull request
into a non-default branch, attested or not
(`test_a_feature_pull_request_with_no_round_is_never_requested`). On the
recent feature pull requests sampled, the Copilot request lands about
two seconds after the pull request is created, before any `wait` runs,
and the develop ruleset carries no Copilot rule. So the request comes
from the platform when the pull request is created rather than from
`pr_review.py`. The evidence is on the issue.

Closes on promotion: #3571, #2180, #3732, #3713, #3726

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings November 1, 2026 19:56
@coderabbitai

coderabbitai Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

Important

  • Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

Run configuration
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

Share

@codecov

codecov Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.34%. Comparing base (71194c9) to head (27d1ca4).
⚠️ Report is 457 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3778      +/-   ##
==========================================
+ Coverage   82.31%   82.34%   +0.02%     
==========================================
  Files          28       28              
  Lines        9438     9454      +16     
==========================================
+ Hits         7769     7785      +16     
  Misses       1669     1669              
Flag Coverage Δ
python-3.13 82.34% <100.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI 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.

🟡 Changes recommended

Critical and moderate findings remain in the backlog-burndown procedure, including unsafe claim detection and cleanup paths that can cause overlapping work or delete the wrong clone.

3 open issues
What changed in this PR

This pull request promotes the accumulated develop changes to main, hardening write-guard behavior, prose-gate diff handling, backlog orchestration, and Copilot review-state processing.

Changes:

  • Fails closed for unread bare pushes and commands that may switch branches.
  • Expands prose-gate handling for moved files and shebang-driven lexer changes.
  • Improves backlog claim tracking, worker liveness, cleanup, and Copilot excluded-file refusal handling.
  • Updates related tests, documentation, generated skill distributions, and source digests.
File Summary Final review comments
tests/​test_prose_lint.py Adds coverage for moved paths, path-dependent findings, and shebang lexer changes. —
tests/​test_pr_review.py Tests excluded-file refusals, local-pass coverage, and stale refusal handling. —
tests/​test_pr_review_copilot.py Extends refusal and findings-label vocabulary tests. —
scripts/​reviewers/​copilot.json Adds excluded-file refusal and additional finding-label spellings. —
scripts/​pr_review.py Handles excluded-file refusals during review waits and status decisions. —
scripts/​pr_review_rounds.py Adds excluded-refusal and local-coverage logic. —
scripts/​pr_review_copilot.py Adds the excluded refusal kind to the shared vocabulary. —
host-setup/​agent-safety/​README.md Documents fail-closed behavior for unread bare pushes and branch switches. —
host-setup/​agent-safety/​claude/​test_install.py Updates bare-push helper tests and adds branch-switch coverage. —
host-setup/​agent-safety/​claude/​gh-write-guard.py Detects branch-changing commands and fails unread bare pushes closed. —
GOVERNANCE.md Documents lineless-file allowances in review table coverage. —
.github/​skills/​pr-review-conduct/​SKILL.md Aligns review-gate prose with excluded refusals and local coverage. —
.github/​skills/​backlog-burndown/​SKILL.md Updates ranking, claims, worker liveness, and cleanup procedures. —
.github/​copilot-instructions.md Updates Copilot review coverage guidance. —
.github/​actions/​prose-gate/​prose_lint.py Compares moved-file findings across old and new paths and detects lexer changes. Moderate, 1 vote: The old-path comparison reuses allowed; it should run without diff scoping so pre-existing findings are excluded correctly.
.claude-plugin/​fleet-skills/​skills/​pr-review-conduct/​SKILL.md Regenerates the Claude review-conduct skill distribution. —
.claude-plugin/​fleet-skills/​skills/​backlog-burndown/​SKILL.md Regenerates the Claude backlog-burndown skill distribution. —
.claude-plugin/​fleet-skills/​.source-digests/​pr-review-conduct Updates the generated review-conduct source digest. —
.claude-plugin/​fleet-skills/​.source-digests/​backlog-burndown Updates the generated backlog-burndown source digest. —
.agents/​skills/​pr-review-conduct/​SKILL.md Updates the canonical review-conduct skill source. —
.agents/​skills/​backlog-burndown/​SKILL.md Updates ranking, claims, worker liveness, and cleanup procedures. Critical, 2 votes: Match an actual backticked branch name rather than the literal <branch> placeholder. Moderate, 2 votes: Keep claims active while abandoned remote branches remain, or provide verified remote cleanup. Moderate, 1 vote: Exclude released claim comments from active-claim enumeration. Critical, 1 vote: Validate the standalone clone path, origin, and branch before removal.

Review effort: Lite

@ptr727

ptr727 commented Nov 1, 2026

Copy link
Copy Markdown
Owner Author

Answers to the 2 findings round 1 names in its file table with no thread (2).

  1. .github/actions/prose-gate/prose_lint.py, the moved-file comparison in main (Counter(check_file(f, rules, scan_root, allowed, at=(scan_root / source, opener)))): "The old-path comparison reuses allowed; it should run without diff scoping so pre-existing findings are excluded correctly." Disproven. allowed reaches only over_cap, the sentence-length rule: grep -nE '\.allowed\b' on the file at develop's head prints 16 hits, each the ctx.allowed argument of an over_cap call, and no other rule reads it. Sentence-length does not depend on the path, so both runs report the same sentence-length findings when both get the same allowed. Run unscoped, the old-path read would instead report a long sentence on its first line (rows[0]) where the scoped new-path read reports its first changed line. The Counter subtraction would then miss that pair and report an old sentence as one the move added. Passing the same allowed to both runs is what keeps the comparison like for like.
  2. .agents/skills/backlog-burndown/SKILL.md, the claim lookup under "Record every claim": "Exclude released claim comments from active-claim enumeration." No change needed. The command walks from a branch back to the claim that explains it, and a released claim still explains that branch. Whether the claim holds files is decided by reading it, and the same bullet states that "A claim comment opening with one holds no files". So a released claim the lookup returns adds nothing to the in-flight set.

…3779)

## Summary

Fixes the two findings promotion PR #3778's review raised against
`backlog-burndown`.

- **Checked remote delete for an abandoned group's pushed branch.**
"Cleanup Is the Orchestrator's" used to report such a branch to the
maintainer rather than delete it, so the next round kept deferring the
re-formed group. Per the maintainer's answer on #1274, the orchestrator
now records the local tip before removal. It deletes the remote branch
only when that tip is an ancestor of `origin/develop`, the live remote
tip equals it, and no open pull request uses the branch as its head or
base. The delete is named as an outward-facing write under
`GOVERNANCE.md` "Repository Boundaries and Write Safety". The claim is
released only once reading the ref back confirms the delete. Where a
check fails, the claim is kept and the group stops for the maintainer.
"Bounding the Wait on a Worker" and "The Round" step 6 are aligned with
it.
- **The claim-lookup command's `<branch>` placeholder.** One sentence
now states that `<branch>` is replaced with the branch name being looked
up before the command runs.

Generated trees regenerated with `scripts/build_dist.py`, and `--check`
passes.

## Local review

Three `local-strict-review` passes, the last over the pushed head with
no findings. The first two raised findings that were fixed in this
branch, apart from the two below.

- Deferred to #3733, class `introduced` (design, reuse):
`scripts/pr_review.py cleanup` already holds a verify-then-delete in
code but refuses an unmerged branch. A branch-named mode there, which
#3733 already proposes for the never-pushed case, should take the pushed
case too and replace this skill's hand-typed remote delete. That needs
files outside this change.

## Pre-existing findings, not filed

- `.agents/skills/backlog-burndown/SKILL.md` "Cleanup Is the
Orchestrator's", the line "`git branch -d` refuses rather than discards
where that check was wrong", class `pre-existing`: for a branch with an
upstream, `git branch -d` checks against the upstream rather than
develop, so it is no backstop there. The remote delete added here no
longer relies on it, since it carries its own `git merge-base
--is-ancestor` check.

Closes on promotion: #1274

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

Copilot AI 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.

🟡 Changes recommended

Three active findings remain, including critical issues in checkout branch-switch detection and pending-review polling, so the change is not yet ready.

2 open issues
1 previously missed issue

In code that hasn't changed since last review

Medium severity Exclude released claims from the in-flight scan

.agents/​skills/​backlog-burndown/​SKILL.md:214

[P1] Exclude released claim comments from the in-flight scan

The new rule says a release edits the claim comment so it opens with a release and therefore holds no files, but this predicate still selects any visible comment containing the backticked branch. After a group is released, its same comment still contains that branch, so every later enumeration treats the released issue as an active claim and can keep the issue blocked or report a false collision. Make the query recognize the release marker and exclude those comments, or define a stable release prefix that it can match.

3 resolved since last review

Review effort: Lite

Comment thread scripts/pr_review_rounds.py Resolved
@ptr727

ptr727 commented Nov 1, 2026

Copy link
Copy Markdown
Owner Author

Answer to the 1 finding round 2 lists under "Previously missed" (1).

  1. .agents/skills/backlog-burndown/SKILL.md:214, the claim lookup under "Record every claim": "Exclude released claim comments from the in-flight scan." This is the third time this point has been raised: once on Fix backlog-burndown Ranking, Claim, and Cleanup Gaps #3776, once in round 1's file table here, and now this. In round 1 I answered it No change needed, because the bullet says "A claim comment opening with one holds no files". Since it keeps coming back, that answer is withdrawn. The lookup's predicate does not apply that rule, and nothing names a release marker it could match. A fix that gives the release a stable prefix and has the lookup exclude it is being dispatched as its own feature -> develop pull request. The outcome will be posted here when that fix lands.

ptr727 and others added 2 commits November 1, 2026 13:08
)

## Summary

The `backlog-burndown` claim lookup read a released claim comment as a
live claim, because nothing gave a release a shape the lookup could
match. This was raised on #3776 and in two rounds of #3778.

- A release edit now opens with the fixed prefix `**Released
(<date>):**` and why the claim holds no files, with the claim text kept
below it. That is the shape the orchestrator already writes.
- The lookup's jq adds `(.body | startswith("**Released") | not)`, so a
comment opening with a release no longer counts as a claim.
- The generated trees are regenerated with `scripts/build_dist.py`, and
`--check` passes.

## Verification

The new lookup ran live and read-only through `gh issue list --jq`
against the open issues. For a branch whose round-1 claims were
released, the old predicate matched every issue holding a released
claim. The new one matched only the issues where a write-access comment
that does not open with `**Released` still names the branch.

## Pre-existing findings, filed

- `.agents/skills/backlog-burndown/SKILL.md:215`, correctness: the
lookup selects an issue when any write-access comment names the branch,
not only a claim comment, so a state comment naming a released claim's
branch still lists the issue. Filed as #3780.

Closes on promotion: #1275

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…#3782)

`wait --request` could stop before the review it requested had landed.
It ended once its body-less liveness read took a head excluded-files
refusal for a finished round, even though a Copilot request was still
pending.

Cause: `run_wait` read the full payload only where `done and
(a.ignore_quota_signal or not a.request)`. So under `--request` without
`--ignore-quota-signal`, `polled_past` never ran. `polled_past` also
required `not request` for an excluded-files refusal, which contradicted
its own docstring ("read past while a request is pending").

Fix:

- `run_wait` reads the full payload whenever the liveness read says
done. The read is not an extra call, since the verdict at the end reuses
it in place of its own `full_read`.
- `polled_past` reads past an excluded-files refusal whenever the
reviewer is pending, and keeps the `holds(pr)` path gated by `not
request`. So `--request` on a head with no pending request still ends on
`REVIEW_EXCLUDED_FILES_ONLY`, as
`test_an_excluded_files_refusal_with_no_local_cover_names_its_own_remedy`
pins.

Test:
`test_a_requested_round_past_an_excluded_files_refusal_is_polled_for`
runs `wait --request` on a pending request behind an excluded-files
refusal, with a round landing two reads later. Before the fix it returns
`30` without polling. After the fix it polls and returns `0` on the
landed round. The poll's first read still shows only the refusal, so the
test also fails if the round-count bump past the refusal is dropped.

The local strict review made two passes. The first raised one
`introduced` finding: the test did not pin that round-count bump. It was
fixed in c9a8d11. The second pass found nothing.

Closes on promotion: #2180

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
@ptr727

ptr727 commented Nov 1, 2026

Copy link
Copy Markdown
Owner Author

Outcome for the "Previously missed" finding answered in #3778 (comment) (1), .agents/skills/backlog-burndown/SKILL.md:214, "Exclude released claim comments from the in-flight scan":

  1. Fixed in Skip Released Claim Comments in the backlog-burndown Claim Lookup #3781 (merge commit 429294b), included in this pull request's head 27d1ca4. A release edit now opens the claim comment with **Released (<date>):** and the reason, with the claim text kept below it. The lookup's jq adds (.body | startswith("**Released") | not), so a released claim no longer lists its issue as in flight.

Copilot AI 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.

🔵 Needs a closer look

Critical unresolved issues leave the cleanup workflow vulnerable to destructive branch deletion, shell and jq injection, and a regression test that fails unconditionally.

No open issues 🎉

4 previously missed issues

In code that hasn't changed since last review

High severity Pass the branch to jq as data, not filter text

.agents/​skills/​backlog-burndown/​SKILL.md:215

[P1] Escape the branch before building the jq expression. <branch> is replaced verbatim inside a single-quoted --jq program, so a valid ref such as fix'one breaks the command and leaves the claim scan empty. That can make the orchestrator dispatch over an existing claim. Pass the live branch as data (for example with a separate jq --arg) instead of interpolating it into the filter.

High severity Pass branch names as quoted shell arguments

.agents/​skills/​backlog-burndown/​SKILL.md:409

[P1] Keep the branch name out of shell source

<exact-branch> is replaced with live branch text in these shell commands, but valid Git ref names can contain shell syntax such as $() or backticks. A branch like that would execute command substitutions before git or gh sees it, and this line is a destructive remote delete. Preserve the value in a shell variable and pass it as a quoted argument (or use a helper that invokes commands with an argument array) for every branch/ref query and the delete.

High severity Reject permanent branches before manual deletion

.agents/​skills/​backlog-burndown/​SKILL.md:409

[P1] Refuse permanent branches before the hand-typed delete

These checks do not exclude main, master, develop, or the repository's default branch. If a malformed claim or branch choice names one of them and no open pull request happens to use it, the ancestor/tip checks can all pass and this command deletes the integration branch. scripts/pr_cleanup.py already rejects permanent branches in permanent_branch; apply the same guard before this manual cleanup path.

High severity Avoid creating the existing docs directory in the test

tests/​test_prose_lint.py:5857

[P1] Do not create an existing directory in this test. commit() has already written docs/b.md, so the docs directory exists even after git rm docs/b.md; this unconditional mkdir() raises FileExistsError before the move and makes the regression test fail every time. Remove this call or make it idempotent.

2 resolved since last review

Review effort: Lite

@ptr727

ptr727 commented Nov 1, 2026

Copy link
Copy Markdown
Owner Author

Answers to the 4 findings round 3 lists under "Previously missed" on head 27d1ca4 (4). The fifth finding the digest counts, the released-claim lookup from round 2, is answered Fixed in #3781 in #3778 (comment).

  1. tests/test_prose_lint.py:5857, "Avoid creating the existing docs directory in the test": "this unconditional mkdir() raises FileExistsError before the move and makes the regression test fail every time." Disproven. git rm deletes a directory that it leaves empty, so docs/ is gone after git rm docs/b.md and the mkdir() succeeds. Constructed case, in a scratch repository:

    $ mkdir docs && echo B > docs/b.md && git add . && git commit -qm init
    $ git rm --quiet docs/b.md && ls -d docs
    ls: cannot access 'docs': No such file or directory
    $ mkdir docs && echo "mkdir ok"
    mkdir ok
    

    The test also passes in this head's Unit test job (Python 3.13) and Windows unit test job, both green.

  2. .agents/skills/backlog-burndown/SKILL.md:215, "Pass the branch to jq as data, not filter text": a ref such as fix'one breaks the single-quoted --jq program. No change needed, per GOVERNANCE.md "Trust Boundaries and Hardening Effort". The <branch> substituted here is a branch on this repository's own origin, and only accounts with write access and the repository's own bots push one there. The orchestrator and its workers name theirs by repo-worktree's layout convention. That is trusted input, and a quote character in a branch name is not a realistic mistake in that deployment, so this is hardening against an adversarial shape the section rules out.

  3. .agents/skills/backlog-burndown/SKILL.md:409, "Pass branch names as quoted shell arguments": a ref holding $() or backticks would run a command substitution. No change needed, on the same grounds as 2. The hand-typed delete runs only for an abandoned group's own branch, which the orchestrator's worker created, and its name is read back with git rev-parse refs/heads/<exact-branch> from that group's own worktree. A branch name built to inject shell syntax has no path into that input.

  4. .agents/skills/backlog-burndown/SKILL.md:409, "Reject permanent branches before manual deletion": the checks do not exclude main, master, develop, or the default branch. No change needed. The fleet's own ruleset payloads carry a deletion rule on both permanent branches: repo-config/develop.json, repo-config/main.json, and repo-config/operational/develop.json. This repository's live rules read the same: gh api repos/ptr727/ProjectTemplate/rules/branches/develop --jq '[.[].type] | unique' lists deletion, and so does the same call for main. So GitHub refuses git push origin --delete develop. The skill reads the ref back with git ls-remote --exit-code, and only exit 2 confirms the delete. Any other result keeps the claim and stops the group for the maintainer. A mistaken branch name therefore fails loudly and deletes nothing, which leaves no destructive or silent failure for a further check to guard against.

@ptr727
ptr727 merged commit ed5e014 into main Nov 1, 2026
14 checks passed
ptr727 added a commit that referenced this pull request Nov 2, 2026
…ings With an Acknowledgment Record, and Mark Direct-Push Closures (#3797)

Promotes the changes merged into `develop` since promotion #3778 to
`main`, the second round of a backlog burndown.

- [#3784](#3784) has the
write guard read a push option attached to `-o` as that option's value,
as git does, rather than as more short flags. It also walks a
short-option cluster carrying a digit, such as `-4f`.
- [#3796](#3796) fixes
five pull-request-gate and release-task defects:
- Each aggregator's `name:` gives the ruleset-bound required check name
only on a `pull_request` run, so a dispatch run cannot mask it.
`spec/audit.py` `check_interface` reads that expression.
- The stubs drop the `pull_request` base-branch filter, and "Adopting
the Gates" states the task's preconditions.
- A re-run release takes the Latest flag only where its version sorts at
or above the current Latest release.
- The `lint` job runs its Python steps under the first `python-versions`
entry.
- `dotnet format` and `dotnet test` run per tracked workspace where the
root holds no solution or project.
- [#3793](#3793) has
`pr_review.py merge` read an acknowledgment record, posted with `comment
--ack KEY=OUTCOME`, for findings no thread carries and for a verdict
naming no counted finding. Every `merge` run ends on one `next=` line
the skills follow, and the digest reads the excluded-files refusal as
`refusal=EXCLUDED`.
- [#3792](#3792) has a
direct push to `develop` on an operational-model repository name the
issues it fixes in its commit message's on-promotion line.
`backlog-burndown` assembles the promotion body from the pull request
that merged each commit, falling back to a direct push's own message.
Its claim lookup matches only claim comments, and
`git-commit-conventions` names the abandon-discard exception.
- [#3798](#3798) has
`has_prose` read an inline token's text children outside code, so an
inline-code span alone no longer counts as prose. It answers this
promotion's first review round.
- [#3799](#3799) has
`check_interface` read each job by its key on a workflow a non-PR event
triggers. A job whose off-PR name is the required check gets its own
DRIFT. It answers this promotion's first review round.

## Downstream effect

From #3796: once this reaches `main`, every downstream
`test-pull-request.yml` whose aggregator carries a literal `name:` shows
`check_interface` DRIFT. It shows that until its next resync. A resync
clears it by replacing the aggregator's `name:` line with the
expression. No ruleset changes, since the `context:` a pull request run
posts is unchanged.

Fixes #3720
Fixes #3334
Fixes #1516
Fixes #3487
Fixes #1485
Fixes #1938
Fixes #3718
Fixes #3724
Fixes #3777
Fixes #3773
Fixes #1276
Fixes #1307
Fixes #3780

🤖 Generated with [Claude Code](https://claude.com/claude-code)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment