Skip to content

feat(repo-fleet-hygiene): report merged remote branches still on origin - #2645

Merged
kyle-sexton merged 1 commit into
mainfrom
cursor/feat-fleet-merged-remote-branches-f8b0
Aug 15, 2026
Merged

feat(repo-fleet-hygiene): report merged remote branches still on origin#2645
kyle-sexton merged 1 commit into
mainfrom
cursor/feat-fleet-merged-remote-branches-f8b0

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

Closes #2607

Summary

Nothing in the plugin fleet cleaned up or even reported merged remote branches that remain on origin when delete_branch_on_merge is not enabled.

Fix

Add merged-remote-branch findings (gh-based evidence) and handoff guidance. Enabling GitHub delete_branch_on_merge remains complementary and is not changed by this PR.

Verification

See collector tests on PR checks.

Related

Refs #2597 — fleet epic.
Refs #2604 — GraphQL merge evidence (related).

Open in Web Open in Cursor 

@cursor

cursor Bot commented Aug 14, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@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: e184d14620

ℹ️ 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 plugins/repo-fleet-hygiene/skills/audit/scripts/audit-fleet.sh Outdated
@kyle-sexton
kyle-sexton force-pushed the cursor/feat-fleet-merged-remote-branches-f8b0 branch from e184d14 to da65b10 Compare August 14, 2026 23:23
@claude

claude Bot commented Aug 14, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 2m 8s —— View job


Claude finished the security review

  • Gather context (PR diff, files changed)
  • Run /review:security-review per instructions
  • Post findings

Security review — no issues found

Reviewed the full diff (git diff origin/main...HEAD, 11 files) for this PR, which adds a merged-remote-branch finding (gh-based evidence for remote-tracking heads still on origin after a merged PR) to the read-only repo-fleet-hygiene audit script, plus supporting docs/changelog/evals.

What I checked, focused on the new shell logic in audit-fleet.sh:

  • The new loop (audit-fleet.sh#L1428-L1476) reuses the existing run_bounded_gh pr list --repo ... --head <branch> exact-match query shape, which is already covered by the fixed gh_probe_allowed allowlist (L225-229) — no new gh command shape or endpoint is introduced.
  • remote_branch_short values come only from local git for-each-ref refs/remotes/<remote>/ output (no network), preserving the plugin's existing privacy guarantee that GitHub only ever sees identifiers the local remote-tracking inventory already carries.
  • The --head allowlist check rejects values starting with - or containing control characters, preventing gh flag-injection via a crafted branch name — same protection the pre-existing local-branch path relies on.
  • The git push --delete --dry-run ... handoff text is only ever passed to emit_findingprint_field, which prints it with a fixed %s format (L868-885) — it is never executed. git push is not present in git_probe_allowed, and the added test asserts it's rejected if attempted (audit-fleet.test.sh#L1039-1040).
  • No changes to allowed-tools frontmatter (still scoped to the one bundled script), no GitHub Actions workflow changes, no permission widening, no new API/network surface, no secrets/credential handling introduced.
  • The self-review note added to reference/security-review.md accurately reflects the above — reuses existing evidence gathering, no mutation allowlist changes, no org-admin/settings writes.

Out of scope for this lane: the existing Codex review comment on audit-fleet.sh:1465 flags that a merged-PR match against a possibly-stale local remote-tracking ref can misattribute a HIGH finding when the branch was actually already deleted upstream. That's a report-accuracy/correctness concern (the emitted handoff is a --dry-run preview only, never executed), not an exploitable security issue, so it belongs to /review:code-review rather than this security lane.

No CRITICAL, IMPORTANT, or SUGGESTION security findings to report.

@kyle-sexton
kyle-sexton enabled auto-merge (squash) August 14, 2026 23:26
@github-actions

Copy link
Copy Markdown

Last security-reviewed head: da65b103f6c05e5586f4e927793f1af0ff733f11. On the next push, the relevance gate compares only the commits since this SHA; delete this comment to force a full re-review.

@cursor
cursor Bot force-pushed the cursor/feat-fleet-merged-remote-branches-f8b0 branch 17 times, most recently from 2b413ca to 498bd25 Compare August 15, 2026 00:48
@claude

claude Bot commented Aug 15, 2026

Copy link
Copy Markdown

Claude finished @cursor[bot]'s task in 7m 8s —— View job


Code review — /review:code-review

  • Gather context (PR diff, files changed)
  • Run /review:code-review criteria against changed files
  • Post findings

Scope: this PR's own diff vs. its merge-base (git diff bfb66be..HEAD, since this branch's single commit is a direct child of bfb66be — the just-merged #2644). Reviewed per this org's code-review lane (plugins/review/skills/code-review/SKILL.md + REVIEW.md): correctness, maintainability, docs/tests — security is out of scope here (this repo has claude-security-review, and it already ran on this PR).

🔴 Important — this PR silently reverts two just-merged features, and shipped docs contradict the shipped code

This commit is a direct child of bfb66be (#2644, "add finding rollups and scalable handoff plans"). Its diff doesn't just add merged-remote-branch reporting — it also deletes the entire rollup/action-plan/--detail/--plan-file/--apply-plan feature (F_CONF/F_KIND/begin_repo_record/repo_verdict/print_collapsed_target_detail/the JSON action-plan writer, ~700 lines) that #2644 shipped, and reverts the aliased-GraphQL merge-evidence approach from #2604/#2642 back to the pre-#2604 gh pr list --state merged REST window + privacy-gated --head fallback:

None of this is disclosed: the PR description only claims to add merged-remote-branch reporting; CHANGELOG.md's new [0.20.0] entry only has Added/Fixed sections for that feature, with no removal notice for the #2608/#2609 rollup or the #2604 GraphQL work CHANGELOG.md itself documents at [0.18.0]/[0.17.0]. Worse, SKILL.md's "Merged branch" section (lines 103-117) is untouched by this PR and still describes the aliased-GraphQL approach as current, stating it "retires the REST merged-pr-window-truncated disclosure" — but the code this PR ships no longer uses GraphQL at all and reintroduces exactly that REST-window disclosure. Documentation and shipped behavior now directly contradict each other.

Either this branch needs to be rebased onto current main so the diff only contains the intended merged-remote-branch addition, or — if reverting the rollup/GraphQL work is actually intentional — that needs to be stated explicitly in the PR description and reflected in CHANGELOG.md/SKILL.md. As-is this looks like an accidental revert from a stale branch/bad rebase, and would delete recently-shipped, documented functionality if merged.

🔴 Important — two broken emit_finding calls silently drop findings (correctness)

Two if blocks that are supposed to call emit_finding are missing the command name/confidence/kind/target — only the trailing evidence-string arguments remain, so bash tries to execute the first string literal as a command (fails with "command not found", non-fatal under set -uo pipefail) and the finding is never emitted:

  • audit-fleet.sh:1288-1292 — the reintroduced merged-pr-window-truncated disclosure (inline comment posted).
  • audit-fleet.sh:1505-1509 — the merge-evidence-privacy-gated aggregate warning. (GitHub's review API wouldn't resolve a line position for this hunk for me, so it's noted here instead of inline; same fix shape as the first.)

Both are masked by the test suite: audit-fleet.test.sh still asserts assert_not_contains "Finding: merged-pr-window-truncated" / "Finding: merge-evidence-privacy-gated" with comments claiming these kinds are "retired" under GraphQL — true for the pre-revert code, but now those assertions pass for the wrong reason (the calls are simply broken) rather than validating real behavior. The wt-canon fixture (1000 rows == MERGED_PR_WINDOW) and the local-only feature/mismatch branch fixture both exercise these broken paths already.


No other correctness/maintainability issues found in the rest of the diff (the ls-remote-gated merged-remote-branch logic itself, docs, evals, and allowlist changes look sound). The prior Codex P2 comment about proving current remote existence before HIGH is already addressed by this PR's git ls-remote --heads gating.
· branch

@cursor
cursor Bot force-pushed the cursor/feat-fleet-merged-remote-branches-f8b0 branch 2 times, most recently from 8f45a0c to e861daf Compare August 15, 2026 01:19
Comment thread plugins/repo-fleet-hygiene/skills/audit/scripts/audit-fleet.sh
@github-actions

github-actions Bot commented Aug 15, 2026

Copy link
Copy Markdown

Claude has reviewed this PR 4 times. The lane skips further automatic reviews after 5; deleting this comment resets the count.

@cursor
cursor Bot force-pushed the cursor/feat-fleet-merged-remote-branches-f8b0 branch from e861daf to 58434dc Compare August 15, 2026 01:25
@claude

claude Bot commented Aug 15, 2026

Copy link
Copy Markdown

Claude finished @cursor[bot]'s task in 2m 3s —— View job


Code review — /review:code-review

  • Gather context (PR diff, files changed, CLAUDE.md)
  • Run /review:code-review against current PR tip (HEAD 58434dc)
  • Post findings

Scope: git diff origin/main...HEAD (10 files, 211+/-11, matching this PR's stated diff). Reviewed per this org's code-review lane (plugins/review/skills/code-review/SKILL.md + REVIEW.md) — correctness, maintainability, docs/tests; security is out of scope here since claude-security-review already ran on this PR.

No high-signal findings

This tip has diverged from what the two prior reviews on this PR examined (e184d146/498bd25d), and the concerns they raised no longer apply to the current diff:

Beyond that, everything checked out:

  • New emit_finding calls match the function's confidence kind target evidence disposition handoff signature correctly.
  • GQL_BRANCHES dedup between local BRANCH_NAMES and remote-only REMOTE_BRANCH_NAMES avoids querying the same branch twice (L1789-L1805).
  • SKILL.md, confidence-model.md, official-sources.md, README.md, and CHANGELOG.md all describe the same ls-remote-gated HIGH/MEDIUM/no-finding behavior the code implements — no doc/code contradiction this time.
  • Version bump (0.19.00.20.0) is consistent between plugin.json and CHANGELOG.md.
  • Test coverage (audit-fleet.test.sh) exercises all three branches (ls-remote confirms/fails/empty), a remote-only head with no local branch, the moved-identity case, and asserts git push stays rejected by the mutation allowlist even though the handoff text mentions push --delete --dry-run.

I read (but could not execute, given this lane's tool grant) audit-fleet.test.sh; its logic and fixtures line up correctly with the implementation, so I'm not flagging anything there.

No correctness or maintainability issues found. Nothing to report as a formal blocking finding.
· branch

@kyle-sexton
kyle-sexton disabled auto-merge August 15, 2026 01:36
@cursor
cursor Bot force-pushed the cursor/feat-fleet-merged-remote-branches-f8b0 branch from 58434dc to 87d3460 Compare August 15, 2026 01:36
@kyle-sexton
kyle-sexton enabled auto-merge (squash) August 15, 2026 01:43
@kyle-sexton
kyle-sexton disabled auto-merge August 15, 2026 01:43
@claude

claude Bot commented Aug 15, 2026

Copy link
Copy Markdown

Claude finished @cursor[bot]'s task in 2m 49s —— View job


Code review — /review:code-review

  • Gather context (PR diff, files changed, prior review history)
  • Run /review:code-review against current PR tip (HEAD 87d3460)
  • Post findings

Scope: git diff origin/main...HEAD (10 files, +211/-11, matches this PR's stated diff). Reviewed per this org's code-review lane (plugins/review/skills/code-review/SKILL.md) — correctness and maintainability; security is out of scope here since claude-security-review already ran clean on this PR.

No high-signal findings

This tip's diff is unchanged in substance from the one the prior review already cleared at 58434dc (same additive merged-remote-branch feature layered on the GraphQL-restore base, nothing removed). I independently re-read the changed logic rather than just trusting that prior pass:

  • git_probe_allowed's new ls-remote case (audit-fleet.sh#L174-L181) — positional arg count/shape ($1=-C $2=path $3=ls-remote $4=--heads $5=remote $6=refs/heads/*) matches exactly how it's invoked at L1957-1958. No off-by-one in the $# guard.
  • The new remote-tip matching loop (L1942-1994) runs independently of the earlier local-branch loop, matches merged-PR rows against REMOTE_BRANCH_TIPS (not local tip), and correctly gates HIGH on a live ls-remote match, MEDIUM on probe failure, and no finding on empty/mismatched ls-remote — verified this is intentional per the feature/shared fixture (an attached branch can legitimately emit both merged-local-branch and merged-remote-branch).
  • GQL_BRANCHES construction (L1789-1805) correctly dedups remote-only heads against local branches before querying, so remote-only merged heads still get exact headRefName GraphQL rows without double-querying shared branches.
  • Doc/version consistency checked: plugin.json (0.19.10.20.0) matches CHANGELOG.md's new [0.20.0] entry; SKILL.md's renumbered evidence-source list (4→6) and tier-precedence list (3→5) are internally consistent; confidence-model.md's new merged-remote-branch row and official-sources.md's addition both match the shipped ls-remote-gated HIGH/MEDIUM/no-finding behavior — no doc/code contradiction.
  • Test coverage (audit-fleet.test.sh) exercises all three ls-remote outcomes (confirmed/failed/empty), a remote-only head with no local branch, an attached branch matching on both local and remote tips, and asserts git push stays rejected by the mutation allowlist even though the handoff text mentions push --delete --dry-run.

Non-blocking nit only (not filing inline, doesn't meet the high-signal bar): pr_merged is parsed out of the matched PR row in the new loop but never used (L1949-1956) — consistent with the same unused-field pattern already present in the pre-existing local-branch loop above it, so not a new issue introduced by this PR.

Nothing to report as a formal blocking finding.
· branch

@cursor
cursor Bot force-pushed the cursor/feat-fleet-merged-remote-branches-f8b0 branch 2 times, most recently from 805a80a to 347460c Compare August 15, 2026 01:44
@kyle-sexton
kyle-sexton enabled auto-merge (squash) August 15, 2026 01:46
@cursor
cursor Bot force-pushed the cursor/feat-fleet-merged-remote-branches-f8b0 branch from 4853a7b to aa22ccf Compare August 15, 2026 01:53
Add merged-remote-branch findings on the GraphQL + bare-path + epic-docs tip:
match merged-PR evidence to remote-tracking tips, require ls-remote for HIGH,
and keep REST pr list out of the merge path. Version 0.20.0 after #2646.

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
@cursor
cursor Bot force-pushed the cursor/feat-fleet-merged-remote-branches-f8b0 branch from aa22ccf to b438670 Compare August 15, 2026 01:53
@kyle-sexton
kyle-sexton merged commit c88370f into main Aug 15, 2026
39 checks passed
@kyle-sexton
kyle-sexton deleted the cursor/feat-fleet-merged-remote-branches-f8b0 branch August 15, 2026 02:00
@claude

claude Bot commented Aug 15, 2026

Copy link
Copy Markdown

Claude finished @cursor[bot]'s task in 51s —— View job


Code review — /review:code-review

  • Gather context (PR diff, files changed, prior review history)
  • Run /review:code-review against current PR tip (HEAD b438670)
  • Post findings

Skipped — PR is closed (merged)

This PR's state is MERGED. The /review:code-review skip gate stops early whenever "PR is closed or not open" and directs posting nothing else in that case, so no new findings are being posted here.

For reference, this PR's content is already on main (the merge commit c88370f shares the identical tree with the PR tip b438670), and it received three full code-review passes plus a security review while open — the last pass (at tip 87d3460, tree-identical to the final merged state) found no high-signal issues after the two earlier passes' concerns (a suspected feature revert, and broken emit_finding calls from a stale rebase) were confirmed stale/superseded by the GraphQL-restore rebase (#2640). No further action taken.

cursor Bot pushed a commit that referenced this pull request Aug 15, 2026
Keep merged-remote-branch under 0.20.0; ship worktree-root conformance
as 0.21.0 after #2645 landed first.

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
cursor Bot pushed a commit that referenced this pull request Aug 15, 2026
Keep merged-remote-branch under 0.20.0; ship worktree-root conformance
as 0.21.0 after #2645 landed first.

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
cursor Bot pushed a commit that referenced this pull request Aug 15, 2026
Keep merged-remote-branch under 0.20.0; ship worktree-root conformance
as 0.21.0 after #2645 landed first.

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
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.

repo-fleet-hygiene: no merged-remote-branch capability, and no plugin in the fleet owns one

1 participant