Add PR ready workflow

[jbsmith7741]

Quality check

• Documentation included
• Test coverage

Summary

Adds a PR-ready workflow for checking branch changes against CONTRIBUTING.md, AGENTS.md, and project Cursor rules before opening a PR:

• commands/pr_ready.md + skills/pr-ready/ — invoke via /ai-toolkit/pr_ready or @pr-ready
• scripts/review_sum.sh — collects diff scope, policy docs, applicable rules, and a checklist seed (installed to ~/.cursor/ai-toolkit/ via install.sh)

Also clarifies Go guidance and aligns related skills:

• rules/go-standards.mdc — helper extraction guidance; restores Concurrency essentials section
• rules/go-testing.mdc — test file layout, naming, config type placement, no shared fixture packages
• skills/red-green-bug-fix/ — RED phase test placement and stop-before-GREEN workflow
• README.md — documents PR ready and review_sum.sh

No Go packages changed; review_sum.sh was smoke-tested with bash -n and manual runs. Automated tests not added for the shell helper.

[qodo-code-review]

PR Summary by Qodo

Add PR-ready workflow and local review context collector

✨ Enhancement 📝 Documentation 🕐 40+ Minutes

AI Description

• Add /ai-toolkit/pr_ready + @pr-ready workflow to assess PR readiness against repo policies.
• Introduce review_sum.sh to gather diffs, policy docs, applicable rules, and a checklist seed.
• Clarify Go helper/testing guidance and align @red-green-bug-fix test placement + RED gate.

Diagram

graph TD
  dev(("Developer")) --> cmd(["/ai-toolkit/pr_ready"]) --> skill["@pr-ready skill"] --> script["review_sum.sh"] --> out["Context + checklist seed"]
  script --> repo[("Git repo")]
  repo --> policy["CONTRIBUTING.md + AGENTS.md"]
  repo --> rules[".cursor/rules/*.mdc or rules-source"]

  subgraph Legend
    direction LR
    _actor(("Actor")) ~~~ _proc(["Process"]) ~~~ _db[("Repo/state")] ~~~ _doc["Docs/rules"]
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Use an existing YAML/glob parser dependency (yq/python)

• ➕ More robust parsing of multi-line YAML frontmatter and complex glob patterns
• ➕ Less bespoke bash logic to maintain/test
• ➖ Adds non-trivial external dependencies (hurts portability)
• ➖ Increases install/setup surface area for a lightweight helper

2. Integrate with GitHub CLI / PR APIs for base/diff context

• ➕ Can auto-detect PR base/target and fetch richer metadata (labels, checks)
• ➕ Avoids some local base-resolution heuristics
• ➖ Requires gh auth and network access; conflicts with stated 'no gh required' goal
• ➖ Less useful for purely local/uncommitted review workflows

Recommendation: The current approach (portable bash + no gh dependency) is appropriate for a pre-PR local standards pass, and matches the repo’s install model (scripts/ copied to ~/.cursor/ai-toolkit/). If rule frontmatter/glob requirements expand beyond single-line values, consider swapping the minimal awk parser for a small, optional dependency (or explicitly documenting the supported frontmatter subset).

Files changed (7) +796 / -7

Enhancement (2) +688 / -0

review_sum.shAdd context collector for PR-ready reviews +580/-0

Add context collector for PR-ready reviews

• Adds a standalone bash script to resolve an integration base, optionally fetch remotes, compute changed files (branch vs uncommitted scope), discover CONTRIBUTING/AGENTS documents along changed paths, and select applicable Cursor rules via 'alwaysApply' and 'globs' frontmatter. Outputs a structured, human-readable summary plus a PR Ready checklist seed.

scripts/review_sum.sh

SKILL.mdAdd '@pr-ready' skill workflow, rubric, and report template +108/-0

Add '@pr-ready' skill workflow, rubric, and report template

• Defines the PR-ready skill metadata and a step-by-step procedure: run 'review_sum.sh', read listed policy/rules in full, review the diff mapped to sources, and emit a standardized compliance/findings/test plan/verdict report without auto-fixing.

skills/pr-ready/SKILL.md

Documentation (5) +108 / -7

README.mdDocument PR-ready workflow and 'review_sum.sh' availability +2/-1

Document PR-ready workflow and 'review_sum.sh' availability

• Updates installation notes to mention 'review_sum.sh' (no 'gh' requirement) and adds a README entry describing the PR-ready command/skill and recommended rule installation flow.

README.md

pr_ready.mdAdd '/ai-toolkit/pr_ready' command documentation +47/-0

Add '/ai-toolkit/pr_ready' command documentation

• Introduces a command doc describing when and how to run the PR-ready workflow, how to invoke 'review_sum.sh' (installed vs local clone), and what report structure to produce.

commands/pr_ready.md

go-standards.mdcClarify helper extraction guidance and restore concurrency section header +14/-1

Clarify helper extraction guidance and restore concurrency section header

• Adds explicit guidance discouraging single-line helper wrappers, provides criteria for when helper extraction is warranted, and restores/anchors the concurrency essentials section.

rules/go-standards.mdc

go-testing.mdcAdd Go test file layout and test type/fixture guidance +31/-1

Add Go test file layout and test type/fixture guidance

• Adds rules for pairing '_test.go' files with source files, discourages feature-specific test file sprawl, clarifies naming/case organization, and defines a hierarchy for where to place test-only config types. Also disallows shared fixture packages for test-only structs.

rules/go-testing.mdc

SKILL.mdAlign RED/GREEN workflow with Go testing placement rules +14/-4

Align RED/GREEN workflow with Go testing placement rules

• Strengthens guidance to extend existing tests rather than forking new test functions/files, adds an explicit stop-after-RED approval gate before production changes, and adds a dedicated section summarizing test placement expectations aligned with 'go-testing.mdc'.

skills/red-green-bug-fix/SKILL.md

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Untracked files excluded ✓ Resolved 🐞 Bug ≡ Correctness

Description

collect_changed_files only uses git diff --name-only (and --cached) which omits untracked
files, so the workflow can incorrectly report “Changed file count: 0” and stop even when new files
exist but aren’t staged.

Code

scripts/review_sum.sh[R150-153]

+  if [[ "${REVIEW_SCOPE}" == "uncommitted" ]]; then
+    while IFS= read -r f; do add_file "${f}"; done < <(git diff --name-only 2>/dev/null || true)
+    while IFS= read -r f; do add_file "${f}"; done < <(git diff --cached --name-only 2>/dev/null || true)
+  else

Evidence

collect_changed_files gathers paths exclusively from git diff --name-only sources, and the
command/skill workflow instructs stopping the review when that computed count is zero.

scripts/review_sum.sh[150-157]
scripts/review_sum.sh[481-484]
commands/pr_ready.md[26-30]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`scripts/review_sum.sh` does not include untracked files in `CHANGED_FILES`. This can cause false “nothing to review” outcomes, especially in `--scope uncommitted` mode when a user created new files but hasn’t staged them.

## Issue Context
The PR-ready procedure explicitly says to stop on zero changed files, so missing untracked files can prematurely terminate the review workflow.

## Fix Focus Areas
- scripts/review_sum.sh[137-158]
- scripts/review_sum.sh[481-484]
- commands/pr_ready.md[26-30]

## Proposed fix
Augment `collect_changed_files` to add untracked files (excluding ignored) for both scopes:
- Append: `git ls-files --others --exclude-standard` and feed results into `add_file`.
- Consider whether to include untracked directories or only files.

Then ensure the “Files changed” section includes those paths and the changed file count reflects them.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Glob '**/' misses root ✓ Resolved 🐞 Bug ≡ Correctness

Description

glob_to_regex converts ** to .* and treats / as required, so a glob like **/*.go requires
at least one / and will not match root-level files like main.go, causing applicable rules to be
skipped.

Code

scripts/review_sum.sh[R80-83]

+      '*')
+        if [[ "${glob:0:1}" == '*' ]]; then
+          regex+=".*"
+          glob="${glob:1}"

Evidence

The conversion logic turns ** into .* and then appends / literally, so **/ becomes ^.*/
which cannot match a path without a slash. The repo’s Go rules use globs: "**/*.go" and `globs:
"**/*_test.go"`, so root-level files won’t match these rules.

scripts/review_sum.sh[71-99]
rules/go-standards.mdc[1-5]
rules/go-testing.mdc[1-5]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The glob-to-regex conversion treats `**/` as “some chars then a required slash”, which breaks the common glob meaning “zero or more directories”. This makes globs like `**/*.go` fail to match root-level files (e.g. `main.go`).

## Issue Context
This script uses frontmatter `globs:` from `.mdc` rule files to decide which rules to include. Several core rules use leading `**/` patterns.

## Fix Focus Areas
- scripts/review_sum.sh[71-99]
- scripts/review_sum.sh[101-107]
- rules/go-standards.mdc[1-5]
- rules/go-testing.mdc[1-5]

## Proposed fix
Update `glob_to_regex` to treat the sequence `**/` as an optional directory prefix. For example:
- When parsing `*`, if the next char is `*` and the following char is `/`, consume all three characters and append a regex like `(.*/)?`.
- Keep existing handling for bare `**` (not followed by `/`) as `.*`.

Add a quick self-check (even as comments) that:
- glob `**/*.go` matches both `main.go` and `pkg/main.go`
- glob `**/*_test.go` matches both `foo_test.go` and `x/foo_test.go`

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Go test changes missed ✓ Resolved 🐞 Bug ≡ Correctness

Description

In discover_applicable_rules, the case checks *.go before *_test.go, so _test.go files
match the first arm and never set has_go_test_changes, preventing go-testing.mdc from being
suggested via the Go fallback when only tests change.

Code

scripts/review_sum.sh[R213-216]

+    case "${path}" in
+      *.go) has_go_changes="true" ;;
+      *_test.go) has_go_test_changes="true" ;;
+      *go-standards.mdc|*go-testing.mdc|*go-project-structure.md)

Evidence

The case statement’s first-match semantics mean *_test.go paths are consumed by the earlier
*.go arm, and the go-testing fallback later depends on has_go_test_changes being true.

scripts/review_sum.sh[212-216]
scripts/review_sum.sh[268-291]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`scripts/review_sum.sh` misclassifies Go test file changes because the `case` statement matches `*_test.go` with the earlier `*.go` pattern. As a result, `has_go_test_changes` remains `false` and the explicit Go testing rule fallback won’t load when only `_test.go` files changed.

## Issue Context
This impacts the PR-ready workflow’s ability to surface `go-testing.mdc` guidance.

## Fix Focus Areas
- scripts/review_sum.sh[212-216]
- scripts/review_sum.sh[268-291]

## Proposed fix
Reorder or refactor the detection logic so `*_test.go` is handled first and sets both flags:
- Match `*_test.go)` before `*.go)`
- In the `*_test.go)` arm, set `has_go_test_changes=true` and also `has_go_changes=true` (tests are Go files too)

After change, validate that changing only a `foo_test.go` file causes `go-testing.mdc` to be listed under “read for review”.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Tip of the day

💡 Did you know, you can type 'qodo, fix this' on a finding and the fix lands right on your PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗