Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions .claude/commands/respond-to-critique.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,23 @@ Read `.claude/commands/_shared-rails.md` first. Highlights:
## Args

- `<pr-number>` — the PR you're discussing with B
- `<head-sha>` — the commit SHA this response runs against (the workflow
captures it at checkout and passes it here). Use it everywhere this prompt
writes `<sha>`. Don't compute the SHA yourself — re-deriving it is the
#2848 race; `gh pr view` / `gh pr diff` remain how you read the exchange.

## Steps

1. **Idempotency check.** Read PR comments + reviews via
`gh pr view <pr-number> --json reviews,comments,headRefOid -q '.'`.
1. **Idempotency check.** Throughout this command, `<sha>` means the
`<head-sha>` argument you were passed; don't compute it yourself
(issue #2848). Read PR comments + reviews via
`gh pr view <pr-number> --json reviews,comments -q '.'`.
- Find the most recent `wheels-bot[bot]` PR comment whose body
contains `wheels-bot:review-b:<pr>:<sha>:<N>` for the current
head SHA. That's B's latest round number.
contains `wheels-bot:review-b:<pr>:<head-sha>:<N>`. That's B's
latest round number.
- Find your most recent review on this SHA (initial or prior
response). If its body contains
`wheels-bot:review-a-response:<pr>:<sha>:<N>` and N matches B's
`wheels-bot:review-a-response:<pr>:<head-sha>:<N>` and N matches B's
latest round, exit silently — you've already responded to that
critique.
- Your response round number = B's latest round number.
Expand Down
19 changes: 14 additions & 5 deletions .claude/commands/review-pr.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,21 @@ below. Highlights for this command:
## Args

- `<pr-number>` — the PR to review
- `<head-sha>` — the commit SHA this review runs against; the workflow
captures it once at checkout and passes it here. Use it verbatim as the
marker SHA, and don't compute the SHA any other way — re-deriving it
mid-session is the #2848 race. This governs only where the *marker SHA*
comes from: you still use `gh pr view` / `gh pr diff` normally to read the
PR's title, diff, files, and existing reviews.

## Steps

1. **Idempotency check.** Read existing reviews on the PR with
`gh pr view <pr-number> --json reviews,headRefOid --jq '.'`. If any review
body contains the marker `<!-- wheels-bot:review-a:<pr>:<sha> -->` for the
current head SHA, exit silently — there is nothing to do.
`gh pr view <pr-number> --json reviews --jq '.'`. If any review body
contains the marker `<!-- wheels-bot:review-a:<pr>:<head-sha> -->` for the
`<head-sha>` you were passed, exit silently — there is nothing to do.
Always take the marker SHA from the `<head-sha>` argument; don't compute
it yourself (issue #2848).

2. **Gather context.** Read in this order, then build a mental model:
- `gh pr view <pr-number>` — title, body, author, base, head, labels
Expand Down Expand Up @@ -115,8 +123,9 @@ below. Highlights for this command:
`### Security` — omit empty sections
- For each finding, cite the file + line, quote the offending snippet,
and propose a concrete fix
- End with the marker `<!-- wheels-bot:review-a:<pr>:<sha> -->` where
`<sha>` is the head SHA you saw at step 2
- End with the marker `<!-- wheels-bot:review-a:<pr>:<head-sha> -->` where
`<head-sha>` is the SHA passed to this command — never a value re-derived
from `gh pr view` during the session (issue #2848)

Submit verdict:
- `--request-changes` if any **Correctness**, **Cross-engine**, or
Expand Down
19 changes: 13 additions & 6 deletions .claude/commands/review-the-review.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,24 @@ After your critique, you choose one of three outcomes:

- `<pr-number>` — the PR being reviewed
- `<review-id>` — the Reviewer A review to critique
- `<head-sha>` — the commit Reviewer A's review was attached to (the
workflow passes `github.event.review.commit_id`). Use it everywhere this
prompt writes `<sha>` — the round marker AND the convergence markers.
Don't compute the SHA yourself — re-deriving it is the #2848 race;
`gh pr view` / `gh pr diff` remain how you read the PR and A's review.

## Steps

1. **Idempotency + round counting.** Read the PR comments via
`gh pr view <pr-number> --json comments,headRefOid`. Count comments
whose body matches `wheels-bot:review-b:<pr-number>:<sha>:` (any round).
1. **Idempotency + round counting.** Throughout this command, `<sha>` means
the `<head-sha>` argument you were passed; don't compute it yourself
(issue #2848). Read the PR comments via
`gh pr view <pr-number> --json comments`. Count comments whose body
matches `wheels-bot:review-b:<pr-number>:<head-sha>:` (any round).

- If the most recent matching comment has the **current head SHA**
**AND** the comment count on the current SHA already equals the
- If the most recent matching comment has the passed `<head-sha>`
**AND** the comment count on that SHA already equals the
review-id you're processing (a precise dedup), exit silently.
- Round number for the current SHA =
- Round number for `<head-sha>` =
(count of B comments on this exact SHA) + 1.
- **If round > 10**: post the terminal comment and exit. The cap
exists so the loop terminates when A and B can't align — humans
Expand Down
33 changes: 30 additions & 3 deletions .github/workflows/bot-review-a.yml
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ jobs:
GH_TOKEN: ${{ steps.app-token.outputs.token }}
PR_FROM_PR_EVENT: ${{ github.event.pull_request.number }}
PR_FROM_COMMENT_EVENT: ${{ github.event.issue.number }}
# Body of the comment that triggered response mode (Reviewer B's
# critique). Passed through env — never interpolate event text
# straight into the script (injection). Empty on pull_request events.
COMMENT_BODY: ${{ github.event.comment.body }}
run: |
set -euo pipefail
if [ "${GITHUB_EVENT_NAME}" = "pull_request" ]; then
Expand All @@ -73,7 +77,21 @@ jobs:
echo "::error::Invalid PR number from issue context: $pr_num"
exit 1
fi
sha=$(gh pr view "$pr_num" --repo wheels-dev/wheels --json headRefOid -q '.headRefOid')
# Anchor the response to the exact commit Reviewer B critiqued — not
# the PR's (possibly newer) head. The triggering comment is B's
# critique, whose marker carries that SHA:
# `wheels-bot:review-b:<pr>:<sha>:<round>`. Re-deriving the head via
# `gh pr view` would float to the current head and diverge from B's
# commit_id-anchored marker once a push lands mid-loop, leaving the
# response unable to match B's round (#2848). COMMENT_BODY is read
# from env, never spliced into the script as a workflow expression.
sha=$(printf '%s\n' "$COMMENT_BODY" \
| grep -oiP "wheels-bot:review-b:${pr_num}:\K[0-9a-f]{7,40}" \
| head -1 || true)
if [ -z "$sha" ]; then
echo "::error::Could not extract the reviewed SHA from the triggering review-b comment"
exit 1
fi
mode="response"
fi
echo "pr_num=$pr_num" >> "$GITHUB_OUTPUT"
Expand Down Expand Up @@ -103,10 +121,19 @@ jobs:
- name: Determine prompt
id: cmd
run: |
# Thread the SHA resolved in "Resolve PR info" — the exact commit the
# Checkout step pinned — into the prompt as a second argument. The
# model emits the idempotency marker from this value instead of
# re-deriving it with `gh pr view`, which races with pushes that land
# mid-session and left the marker pointing at the wrong commit
# (issue #2848). The Run Reviewer A step's Bash allowlist is gh +
# read-only git (no echo/printenv), so the model can't read a step
# env var — the SHA must travel in the prompt text, the same channel
# the PR number already uses.
if [ "${{ steps.pr.outputs.mode }}" = "initial" ]; then
echo "cmd=/review-pr ${{ steps.pr.outputs.pr_num }}" >> "$GITHUB_OUTPUT"
echo "cmd=/review-pr ${{ steps.pr.outputs.pr_num }} ${{ steps.pr.outputs.sha }}" >> "$GITHUB_OUTPUT"
else
echo "cmd=/respond-to-critique ${{ steps.pr.outputs.pr_num }}" >> "$GITHUB_OUTPUT"
echo "cmd=/respond-to-critique ${{ steps.pr.outputs.pr_num }} ${{ steps.pr.outputs.sha }}" >> "$GITHUB_OUTPUT"
fi

- name: Run Reviewer A
Expand Down
16 changes: 12 additions & 4 deletions .github/workflows/bot-review-b.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,16 @@ jobs:
|| github.event.pull_request.draft == false
)
steps:
- name: Checkout PR head
- name: Checkout the reviewed commit
uses: actions/checkout@v6
with:
ref: ${{ github.event.pull_request.head.sha }}
# Check out the commit Reviewer A's review was attached to — not the
# PR's (possibly newer) head. B critiques A's review of THIS commit,
# so it must read exactly what A read, and the marker it emits must
# key off the same SHA the skip-check below gates on. commit_id is a
# GitHub-generated SHA, immune to head drift from concurrent pushes
# (issue #2848).
ref: ${{ github.event.review.commit_id }}
fetch-depth: 0

- name: Generate App token
Expand All @@ -47,7 +53,9 @@ jobs:
with:
target-type: pr
target-number: ${{ github.event.pull_request.number }}
marker-pattern: 'wheels-bot:review-b:${{ github.event.pull_request.number }}:${{ github.event.pull_request.head.sha }}:'
# Key off the reviewed commit (same SHA checked out above and threaded
# into the prompt) so the gate and the emitted marker agree (#2848).
marker-pattern: 'wheels-bot:review-b:${{ github.event.pull_request.number }}:${{ github.event.review.commit_id }}:'
github-token: ${{ steps.app-token.outputs.token }}

- name: Run Reviewer B
Expand All @@ -62,7 +70,7 @@ jobs:
anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }}
github_token: ${{ steps.app-token.outputs.token }}
prompt: |
/review-the-review ${{ github.event.pull_request.number }} ${{ github.event.review.id }}
/review-the-review ${{ github.event.pull_request.number }} ${{ github.event.review.id }} ${{ github.event.review.commit_id }}
claude_args: |
--model claude-sonnet-4-6
--max-turns 300
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ All historical references to "CFWheels" in this changelog have been preserved fo

### Fixed

- wheels-bot no longer re-fires Reviewer A/B on commits it has already reviewed. The review idempotency markers (`<!-- wheels-bot:review-a:<pr>:<sha> -->` / `review-b`) embedded a SHA the skill prompts re-derived at review time via `gh pr view --json headRefOid`, which races with pushes that land mid-session: between the workflow's checkout and the model's `gh pr view` call a new push could move the PR head, so the emitted marker SHA lagged the commit the review actually ran against. The skip-check gate then failed to recognise an already-reviewed head and Reviewer A re-fired on superseded commits while Reviewer B emitted contradictory verdicts on different SHAs (observed across the #2847 review cycle, where Reviewer B self-diagnosed the drift twice). The workflows now capture the head SHA exactly once and thread it into the prompts as an explicit `<head-sha>` argument: `bot-review-a.yml` passes the already-checked-out `steps.pr.outputs.sha` into `/review-pr` and `/respond-to-critique`, and `bot-review-b.yml` keys its checkout, skip-check marker-pattern, and `/review-the-review` invocation off `github.event.review.commit_id` (the commit Reviewer A's review was attached to, immune to head drift from concurrent pushes). The prompts emit the marker from that argument instead of re-deriving it — the Reviewer A/B Bash allowlist is `gh` + read-only `git` (no `echo`/`printenv`), so a step-level env var would be unreadable by the model and the SHA must travel in the prompt text, the same channel the PR number already uses. A structural spec, `vendor/wheels/tests/specs/cli/BotReviewMarkerShaThreadingSpec.cfc`, guards the wiring across both workflow YAMLs and all three prompts (#2848)
- `wheels new <app> --no-sqlite`, `wheels generate admin <Model> --no-routes`, `wheels test --no-test-db` and every other `--no-*` flag the CLI documents now reach their command-level parsers again. LuCLI normalizes `--no-key` on the command line to `key=false` in the arg collection it hands modules, and `Module.cfc::argsFromCollection()` was silently dropping `false` entries — so the literal-token matchers in `new()`, `g admin`, and `test()` never saw the user's negation and the defaults stuck (SQLite still scaffolded, routes still generated, test DB still applied). The rebuild now re-emits `--no-<key>` for `false` values, so all four `--no-*` flags surface to the command handlers unchanged. `--nosqlite` (no hyphen) was never affected because LuCLI does not strip a leading `no` that lacks the hyphen. Spotted in #2855 after the prior `--no-sqlite` plumbing fix in #2624.
- `wheels new <name>` no longer crashes on a fresh Windows (Scoop) install with `lucee.runtime.exp.NativeException: there is no Resource provider available with the name [c]` before any module output appears. LuCLI hands `Module.init()` a `cwd` of the JVM's `user.dir` (e.g. `C:\Users\cy`, backslashes), and the early scaffold path concatenated `cwd & "/" & appName` into a mixed-slash string like `C:\Users\cy/blog`. Lucee 7's `ResourceUtil` runs a URI scheme-detection regex (`^[a-zA-Z][a-zA-Z0-9+.-]*:`) ahead of its Windows drive-letter special case on this code path, matches `c:`, extracts `c` as a resource-provider scheme, finds none (only `ftp` / `zip` / `tar` / `tgz` / `http` / `https` / `ram` / `s3`), and throws — pure-backslash and pure-forward-slash paths both work, only the mixed form fails. A new `$normalizePath()` replaces backslashes with forward slashes on `variables.cwd` in `init()` and on every `java.io.File.getCanonicalPath()` result in `resolveProjectRoot()` / `resolveFrameworkSource()`, so `C:/Users/cy/blog` matches Lucee's Windows-path detection before the URI regex ever runs; a `$safeDirExists()` wrapper adds a `java.io.File.isDirectory()` fallback for any path that still reaches a `directoryExists()` check with a drive-letter prefix (a user-supplied `WHEELS_FRAMEWORK_PATH`, a CFML mapping). Both no-op on macOS/Linux, where paths carry no `<letter>:` prefix. Latent since the Scoop install first shipped, but masked until the `-Dlucli.binary.name=wheels` routing fix (`wheels-dev/scoop-wheels@30ea6e5`) let `wheels new` actually reach this code (#2835)
- Running the `wheels` CLI with no arguments no longer errors out with `Component [modules.wheels.Module] has no function with name [main]`. LuCLI dispatches a bare `wheels` invocation to a `main()` subcommand on the module; previously `cli/lucli/Module.cfc` only defined `showHelp()`, so picocli's routing surfaced the missing-method exception. `Module.cfc` now defines `main()` as a thin delegate to `showHelp()` (and the function is added to `mcpHiddenTools()` so it doesn't appear as an MCP tool), restoring the expected behavior of printing the help banner when no subcommand is supplied (#2840)
Expand Down
Loading
Loading