docs(repo-fleet-hygiene): restore the documented argument grammar - #2670
Conversation
|
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 3m 6s —— View job Code review complete
Skip-gate check: PR is open, not a draft, and the change is substantive (grammar-correctness restoration across a skill's documented contract) — full review performed. Verification performed: I independently checked every behavioral claim in the PR description against
I could not execute Finding (posted inline on
|
|
Claude finished @kyle-sexton's task in 48s —— View job Security review
Skip gate triggered — no findings to report. All 22 additions / 14 deletions in this PR are prose-only:
No script, workflow, or executable logic changed — This is a documentation-accuracy fix (skill prose vs. already-shipped script behavior) with no security-relevant surface. Skip gate condition 2 applies — stopping here per the review criteria. |
|
Last security-reviewed head: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a24ddf5138
ℹ️ 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".
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
Claude has reviewed this PR 2 times. The lane skips further automatic reviews after 5; deleting this comment resets the count. |
The skill body told an agent to reject `audit D:` while the bundled script accepted it. #2638 shipped bare positional paths and drive roots and replaced the project-directory fallback with a hard scope failure; #2646 landed eight minutes later from a branch cut before it, and its rebase carried the pre-#2638 prose forward as a revert. Restores, against the script as it actually behaves on main: - the bare positional `<dir>` form, in the argument list and the hint - `--project-dir` as a config rung only, not a scope fallback - the no-scope paragraph, which now describes the hard failure and its remedies instead of the removed project-directory target - the reject-outside-this-grammar clause, which now says plainly that a bare path IS in the grammar and only unknown flags are refused Also documents `bare-repo-with-working-tree` (#2633/#2602), whose handoff row was dropped by the same revert, and corrects the machine-wide-discovery claim in the README and plugin description: the ladder is not built, a no-scope run fails, and #2599 tracks it. Catalog regenerated from the manifest. Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
Bump to 0.22.1 with a CHANGELOG entry so plugin-file edits clear changelog-parity. Align the audit skill description/Purpose with the cross-repository wording already used in README and plugin.json, replace the stale no-argument Quick start with an explicit --repo example, and stop naming closed #2599 as the tracker for the still-unshipped machine-wide discovery ladder. Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
2608220 to
fd4168c
Compare
|
Claude finished @cursor[bot]'s task in 2m 46s —— View job Code review complete
Skip-gate check: PR is open, not a draft, and the change is substantive (grammar-correctness + capability-claim restoration across a skill's documented contract). Full review performed. What changed since the last review round: the newest commit (
Verification performed: re-checked every restored behavioral claim directly against
I could not run Finding
The README's opening sentence still claims No other correctness or maintainability issues found — the restored argument grammar, the no-scope failure copy, the |
) No linked issue ## Summary The skill body instructed an agent to **reject `/repo-fleet-hygiene:audit D:`** — the exact invocation #2599 exists to enable — while the bundled script accepted it. This restores the documented grammar to match the script as it actually behaves on `main`. ## Fix PR #2638 shipped bare positional paths and drive roots, and replaced the project-directory scope fallback with a hard failure that names its remedies. PR #2646 merged eight minutes later from a branch cut before it; the rebase was textually clean but carried pre-#2638 prose forward, reverting the skill layer while leaving the script untouched. Restored, each verified against `origin/main`'s `audit-fleet.sh`: | Restored | Script behaviour it now matches | |---|---| | bare positional `<dir>` in the argument list and `argument-hint` | `:484` `[[ -n "$1" ]] \|\| fail "bare path requires a directory"`, with `normalize_discovery_root` mapping `D:` → `D:/` | | `--project-dir` documented as a config rung only | project dir is no longer a scope target anywhere in the script | | the no-scope paragraph, rewritten to describe the hard failure | `:1069` `fail "no scope resolved: no bare path, --root, or --repo, and no config-supplied fleet.root/fleet.repo"` | | the reject-outside-the-grammar clause | now says plainly that a bare path **is** in the grammar; only an unrecognized `-`-prefixed flag is refused, matching the script's `-*)` arm | Also restores the `bare-repo-with-working-tree` handoff row (#2633 / #2602), dropped by the same revert — `git grep -c` on `main` returned 0 — with its documented remedy (`git config --local core.bare false`) and the fact that linked worktrees keep working, since the state looks alarming and is not. ## Also corrected: an unimplemented capability claimed as shipped `README.md` and the plugin description both claimed **machine-wide** repository discovery. It does not exist: `grep -n "ghq\|claude\.json"` over the landed script returns nothing, only the configured-roots rung is implemented, and a no-argument run hard-fails. The README row is split into what shipped (bounded discovery via bare path, drive root, `--root`, `--repo`, config rungs) and what did not (the ghq / configured-roots / agent-state / bounded-sweep ladder, still unshipped remaining contract work). The plugin description now says "Cross-repository" rather than "Machine-wide". `docs/CATALOG.md` regenerated from the manifest rather than hand-edited. ## Verification ``` node scripts/generate-catalog.mjs --check → Catalog is in sync with the manifests. node scripts/generate-cheatsheet.mjs --check → Cheat sheet is in sync with skill frontmatter. markdownlint-cli2 (SKILL.md, README.md) → 0 error(s) check-skill.sh audit → all 8 base-ref trigger phrases preserved description 642/1536 chars SKILL.md 302/500 lines markdownlint clean ``` The trigger-preservation check matters here specifically: the plugin description was reworded, and that gate fails on a silently dropped trigger phrase. Version bumped to 0.22.1 with a CHANGELOG entry so changelog-parity clears for the plugin-file edits. ## Why nothing caught this `scripts/docs-only-paths.txt` contains exactly one prefix (`docs/topics/`), so `plugin-gate` **did** run the full suite on #2646 — all 38 checks green. There is simply no assertion tying `SKILL.md` prose to the script's actual argument parser. That gap is worth its own issue: this is the second silent revert on `main` in a day (the first was #2633's stale-base squash, repaired by #2640), and both passed CI. ## Related Refs #2599 (already closed by #2638; this PR only restores the documented grammar that #2638 shipped). --------- Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
1 participant
No linked issue
Summary
The skill body instructed an agent to reject
/repo-fleet-hygiene:audit D:— the exact invocation #2599 exists to enable — while the bundled script accepted it. This restores the documented grammar to match the script as it actually behaves onmain.Fix
PR #2638 shipped bare positional paths and drive roots, and replaced the project-directory scope fallback with a hard failure that names its remedies. PR #2646 merged eight minutes later from a branch cut before it; the rebase was textually clean but carried pre-#2638 prose forward, reverting the skill layer while leaving the script untouched.
Restored, each verified against
origin/main'saudit-fleet.sh:<dir>in the argument list andargument-hint:484[[ -n "$1" ]] || fail "bare path requires a directory", withnormalize_discovery_rootmappingD:→D:/--project-dirdocumented as a config rung only:1069fail "no scope resolved: no bare path, --root, or --repo, and no config-supplied fleet.root/fleet.repo"--prefixed flag is refused, matching the script's-*)armAlso restores the
bare-repo-with-working-treehandoff row (#2633 / #2602), dropped by the same revert —git grep -conmainreturned 0 — with its documented remedy (git config --local core.bare false) and the fact that linked worktrees keep working, since the state looks alarming and is not.Also corrected: an unimplemented capability claimed as shipped
README.mdand the plugin description both claimed machine-wide repository discovery. It does not exist:grep -n "ghq\|claude\.json"over the landed script returns nothing, only the configured-roots rung is implemented, and a no-argument run hard-fails. The README row is split into what shipped (bounded discovery via bare path, drive root,--root,--repo, config rungs) and what did not (the ghq / configured-roots / agent-state / bounded-sweep ladder, still unshipped remaining contract work). The plugin description now says "Cross-repository" rather than "Machine-wide".docs/CATALOG.mdregenerated from the manifest rather than hand-edited.Verification
The trigger-preservation check matters here specifically: the plugin description was reworded, and that gate fails on a silently dropped trigger phrase.
Version bumped to 0.22.1 with a CHANGELOG entry so changelog-parity clears for the plugin-file edits.
Why nothing caught this
scripts/docs-only-paths.txtcontains exactly one prefix (docs/topics/), soplugin-gatedid run the full suite on #2646 — all 38 checks green. There is simply no assertion tyingSKILL.mdprose to the script's actual argument parser. That gap is worth its own issue: this is the second silent revert onmainin a day (the first was #2633's stale-base squash, repaired by #2640), and both passed CI.Related
Refs #2599 (already closed by #2638; this PR only restores the documented grammar that #2638 shipped).