fix(repo-fleet-hygiene): keep analysis after moved-identity - #2629
Conversation
github-remote-moved evidence now states that branch/worktree classification continues against the resolved full_name, and the collector fixture requires exact-OID merged findings from that identity so a silent skip cannot regress. Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
|
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. |
|
Claude finished @kyle-sexton's task in 1m 23s —— View job Security review complete ✅
No security issues found. This PR removes a premature Checked specifically:
This is a code-quality/correctness fix (restoring dropped findings), not a security-relevant change — findings scope check confirmed no logic flaws, injection surfaces, or trust-boundary issues introduced. |
|
Last security-reviewed head: |
|
Claude finished @kyle-sexton's task in 26s —— View job Code review skippedThis PR is already merged (state: |
|
Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count. |
…#2689) Closes #2687 ## Summary A branch carrying exact-OID merged-PR evidence that is **also protected** fell through both arms of the `pr_match` block and emitted nothing. The `merged-worktree` arm requires a non-main worktree; the `merged-local-branch` arm requires `protected=false`. A branch attached to the main worktree, or the canonical checkout's current branch, satisfies neither — so the strongest merge evidence the collector holds was computed and then discarded. The tell that this was an oversight rather than a protection rule: the weaker `merged-pr-tip-drift` immediately below carries no protection guard at all and does emit. Silence on the strong path read as "nothing merged" rather than "merged, but protected". ## Fix Adds an `else` arm emitting `LOW merged-protected-branch`, naming the PR number, the matching `headRefOid`, and which protection applies. The protection rule is unchanged. The new kind is deliberately absent from `branch_action_kind()` — which matches only `merged-local-branch` — so it is reported and can never become a cleanup candidate, inflate the rollup candidate count, or enter the action plan. Documented in `reference/confidence-model.md`; the repo already gates on every emitted kind appearing in that tier table, and that gate caught the omission during development. ## Verification Replaying the exact conditions the collector evaluates, across all five reachable input shapes: ``` BEFORE: unattached, unprotected HIGH merged-local-branch attached to NON-main worktree HIGH merged-worktree attached to MAIN worktree <NOTHING EMITTED> current branch, unattached <NOTHING EMITTED> no merged PR <NOTHING EMITTED> AFTER: unattached, unprotected HIGH merged-local-branch attached to NON-main worktree HIGH merged-worktree attached to MAIN worktree LOW merged-protected-branch current branch, unattached LOW merged-protected-branch no merged PR <NOTHING EMITTED> ``` Two cases go from silent to reported; the other three are byte-identical, so no existing classification changes. Also run: `bash -n` clean, `shellcheck -x` clean, `markdownlint-cli2` 0 errors on the reference doc. ## What I could not verify, stated plainly **The full `audit-fleet.test.sh` suite does not complete on Windows under Git Bash.** It exceeded 500s here with no buffered output; two independent earlier attempts hit the same wall at a 9-minute budget. So the decision-table evidence above comes from isolating the arm, not from a suite run. I drafted a fixture assertion and then **reverted it**, because I could not run the suite to confirm it, and an unvalidated fixture edit to a shared harness risks breaking CI for everyone else. The shape a follow-up needs: a branch attached to the main worktree (index 0 of the worktree porcelain list) that is **not** the default branch, plus a matching row in the PR fixture table. Note the default branch cannot be used — it is excluded from merge-evidence collection upstream, which I discovered by trying it first. That suite's Windows runtime is worth its own issue; it is now the second finding this session that could not be closed by running the tests that exist. No version bump or CHANGELOG entry — a consolidating bump is landing separately, and parallel PRs staging the same version collide on `check-changelog-parity.sh --check-order`. ## Related Supersedes the mechanism described in #2600, which was closed by #2629 against a moved-identity stop that does not exist. That fix is real but addresses a different path; this defect survived it, which is why the symptom recurred after the issue was closed. N/A
Closes #2600
Summary
On a moved-identity checkout, the audit emitted
github-remote-movedand silently skipped branch/worktree analysis, dropping genuine HIGH findings.Fix
Continue branch and worktree analysis after
github-remote-moved; do not silent-skip local facts that do not depend on the remote identity being current.Verification
See
audit-fleet.test.shon PR checks.Related
Refs #2597 — fleet hygiene epic.