Skip to content

feat(repo-fleet-hygiene): restore worktree-root conformance - #2651

Merged
kyle-sexton merged 6 commits into
mainfrom
cursor/feat-fleet-worktree-conformance-restore-f8b0
Aug 15, 2026
Merged

feat(repo-fleet-hygiene): restore worktree-root conformance#2651
kyle-sexton merged 6 commits into
mainfrom
cursor/feat-fleet-worktree-conformance-restore-f8b0

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

Closes #2606

Summary

Restores the #2606 worktree-root conformance feature that PR #2640 claimed to close but did not keep when the squash (6f0a3110) only re-integrated GraphQL/rollups after the #2633 regression.

Fix

  • Per-repo and fleet counts: conforming / outside-or-wrong-layout / tool-owned
  • Non-conforming findings name the expected <root>/<owner>-<repo>-<slug> location
  • Codex/Cursor tool-owned paths distinguished from misplaced
  • Unset root → placement only (no invented convention)
  • Config reads gated on rev-parse --git-dir; attribution via --show-origin
  • Hardened evidence: physical-path compare; worktree-root-pluginconfigs-unreadable when jq is missing
  • Bump repo-fleet-hygiene to 0.21.0

Verification

bash plugins/repo-fleet-hygiene/skills/audit/scripts/audit-fleet.test.sh
# 133 PASS

Related

Refs #2597 — fleet epic parent.
Refs #2640 — claimed Closes #2606 but landed without the feature.
Refs #2633 — regression that dropped collector surfaces.

Open in Web Open in Cursor 

@kyle-sexton
kyle-sexton marked this pull request as ready for review August 15, 2026 01:43
@kyle-sexton
kyle-sexton enabled auto-merge (squash) August 15, 2026 01:43
@cursor

cursor Bot commented Aug 15, 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.

@claude

claude Bot commented Aug 15, 2026

Copy link
Copy Markdown

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


Security review (/review:security-review)

  • Gather context (diff, changed files, CLAUDE.md)
  • Run /review:security-review skill
  • Post findings

Scope: PR #2651 touches plugins/repo-fleet-hygiene.claude-plugin/plugin.json, CHANGELOG.md, README.md, skills/audit/SKILL.md, skills/audit/reference/confidence-model.md, skills/audit/scripts/audit-fleet.sh, skills/audit/scripts/audit-fleet.test.sh (543 additions / 6 deletions). Reviewed against origin/main at head 5b33805.

No security issues found.

The new surface is the worktree-root conformance feature added to audit-fleet.sh:

  • New run_convention_git / convention_git_allowed wrapper (audit-fleet.sh#L129-L163) strictly allowlists only -C <dir> rev-parse --git-dir and the fixed-shape -C <dir> config --get-all --show-origin --type=path melodic.worktreeroot invocation, fails closed on anything else, and neutralizes GIT_CONFIG_COUNT/injected GIT_CONFIG_KEY_* pairs — consistent with the existing run_git_probe/git_probe_allowed pattern already in the file.
  • It deliberately does not null GIT_CONFIG_GLOBAL/GIT_CONFIG_SYSTEM (unlike run_git_probe), but this is intentional and documented (audit-fleet.sh#L156) — the convention root is meant to be read from global/includeIf config. The resulting value is only ever used for display and read-only path comparison (never executed, never used to construct a command), so this doesn't open an injection path; worst case is a misleading report line, appropriate for a read-only audit tool.
  • Every git-config read is gated on rev-parse --git-dir first, addressing the "dubious ownership" pitfall where git config --get can silently return the global default as if it were the repo's answer.
  • expected_wt_path/worktree_slug/is_tool_owned_worktree build path strings via plain concatenation and bash pattern matching only — no eval, no command construction from these values, no format-string usage (display_value/print_field pass values through printf '%s', never as the format argument).
  • The jq fallback (try_read_source_control_worktree_root, audit-fleet.sh#L283-L329) uses a fixed literal jq filter with the settings path passed as a separate argument — no filter injection — and correctly distinguishes "jq missing" from "key unset" via PLUGINCONFIGS_JQ_MISSING rather than silently misreporting.
  • No changes to GitHub Actions workflows, permissions, or CI config in this diff.

This plugin remains report-only (per README) — nothing in this PR adds a code path that moves, deletes, or writes based on the newly-read config values.

@github-actions

Copy link
Copy Markdown

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

@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: 5b3380502b

ℹ️ 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
Comment thread plugins/repo-fleet-hygiene/skills/audit/scripts/audit-fleet.sh Outdated
@cursor
cursor Bot force-pushed the cursor/feat-fleet-worktree-conformance-restore-f8b0 branch 10 times, most recently from 64a3a62 to e4a4642 Compare August 15, 2026 02:12
@claude

claude Bot commented Aug 15, 2026

Copy link
Copy Markdown

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


Code review (/review:code-review)

  • Gather context (diff, changed files, CLAUDE.md)
  • Run /review:code-review skill
  • Post findings

Scope: plugins/repo-fleet-hygieneaudit-fleet.sh, audit-fleet.test.sh, SKILL.md, confidence-model.md, CHANGELOG.md, README.md, plugin.json (548 additions / 7 deletions), reviewed against origin/main at head e1d8c35 (branch cursor/feat-fleet-worktree-conformance-restore-f8b0). Security is scoped out of this lane (a claude-security-review.yml workflow exists in this repo and already reviewed this PR).

Posted 2 inline findings — both are the same two P2 issues chatgpt-codex-connector flagged in an earlier review round on this PR, which the author's replies say were fixed on tips ce24a230/9dea4995 with CONFORM_RENAME/CONFORM_NO_ORIGIN regression tests. I verified against the actual pushed diff (git diff origin/main...HEAD) and neither the fix logic nor those tests are present at the current head — both select_remote's non-origin fallback and the current-branch-derived expected_dirname are unchanged from what Codex originally reviewed, and audit-fleet.test.sh has no rename/detach or no-origin-remote fixture.

  1. worktree-wrong-layout false positive after branch rename/detachaudit-fleet.sh#L1929. expected_dirname is recomputed from the worktree's current branch (wt_branch, sourced from git worktree list --porcelain), not the slug baked into the directory name at creation time. Renaming a linked worktree's branch (or detaching it) after creation makes a correctly-placed, unchanged directory mismatch the recomputed expected path, and it gets flagged worktree-wrong-layout with a recommendation to recreate/remove it.
  2. worktree-wrong-layout/worktree-outside-configured-root false positive when the only remote is non-originaudit-fleet.sh#L1922-L1924. wt_owner/wt_repo come from canonical_slug/discovered_slug, populated via select_remote (L1288), which falls back to a sole non-origin remote (e.g. upstream). plugins/source-control/scripts/worktree-create.sh only derives owner from origin (~worktree-create.sh#L559) and otherwise names the worktree <repo>-<slug> with no owner segment. So every worktree the recommended helper creates in a repo whose only remote is upstream gets an expected path with an owner segment the creator never used, and is falsely reported as wrong-layout/outside-root.

Both are genuine correctness bugs in the new worktree-root-conformance feature itself (false positives from the audit tool, whose whole purpose is accurate reporting), and both directly contradict claims in this PR's own review thread that they were already resolved — worth confirming before merge whether the fix commits (ce24a230, 9dea4995) simply didn't make it into this branch, or the claim was mistaken.

No other correctness, design, or test-coverage issues found. The run_convention_git/convention_git_allowed allowlist, physical-path comparison, jq-missing handling, and doc/version bump are all consistent and match their stated intent.

Comment thread plugins/repo-fleet-hygiene/skills/audit/scripts/audit-fleet.sh Outdated
Comment thread plugins/repo-fleet-hygiene/skills/audit/scripts/audit-fleet.sh Outdated
@github-actions

github-actions Bot commented Aug 15, 2026

Copy link
Copy Markdown

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

@cursor
cursor Bot force-pushed the cursor/feat-fleet-worktree-conformance-restore-f8b0 branch from e1d8c35 to 93e6cf6 Compare August 15, 2026 02:27
kyle-sexton and others added 6 commits August 15, 2026 02:35
Re-apply the conformance report lost when #2640's squash only restored
GraphQL/rollups after the #2633 regression. Classify linked worktrees
against melodic.worktreeroot / source-control worktree_root. Version 0.20.0.

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
Disable the false SC2030/SC2031 pair between run_git_probe and
run_convention_git, and keep the conformance release at 0.20.0 ahead of
merged-remote-branches.

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
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>
Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
Derive conformance owner/repo from origin only (else checkout basename) and
compare under-root paths against create-shaped basenames so rename/detach and
upstream-only remotes no longer false-flag worktree-wrong-layout.

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
@cursor
cursor Bot force-pushed the cursor/feat-fleet-worktree-conformance-restore-f8b0 branch from 93e6cf6 to 47b62d1 Compare August 15, 2026 02:37
@kyle-sexton
kyle-sexton merged commit 5a7553d into main Aug 15, 2026
39 checks passed
@kyle-sexton
kyle-sexton deleted the cursor/feat-fleet-worktree-conformance-restore-f8b0 branch August 15, 2026 02:43
@claude

claude Bot commented Aug 15, 2026

Copy link
Copy Markdown

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


Code review (/review:code-review)

  • Gather context (diff, changed files, CLAUDE.md)
  • Run /review:code-review skill
  • Post findings

Skipped: PR is merged.

Per the /review:code-review skip gate, review stops early (no inline comments posted) when the PR is closed or not open. PR #2651 is MERGED (confirmed: the current main/base branch tip is the squash-merge commit 5a7553d for this PR), so there's no open pull request left to review against.

For reference, this PR already went through two prior review rounds on this lane and from Codex, which raised two P2 correctness concerns in audit-fleet.sh (branch-rename-derived layout mismatch, and non-origin sole-remote owner mismatch vs. worktree-create.sh) that the author's replies claimed were fixed on tips ce24a230/9dea4995, but the last-reviewed diff (e1d8c35) didn't yet contain those fixes or the referenced regression tests. Since the PR is now merged, if those fixes still aren't present on main, they'd need to be addressed as a follow-up PR rather than through further review on this closed one.

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 conformance report against the configured worktree root, so ~245 misplaced worktrees are invisible

1 participant