Combine upstream code-review plugin with Quarto-tailored prompt

[d-morrison]

Stacked on top of #70 — review that one first; this PR's diff is against add-claude-pr-creation, not main.

Summary

Keeps the upstream code-review@claude-code-plugins plugin invocation in .github/workflows/claude-code-review.yml (so we pick up future improvements to the canonical review skill) and layers a Quarto/R-specific addendum on top so the reviewer also catches issues particular to this template repo.

The combined prompt:

• Leads with /code-review:code-review <PR-ref> — the standard structured review.
• Appends a Quarto-tailored addendum prioritizing:
  • Quarto / prose correctness — .qmd chunk option syntax, cross-ref resolution, the bullet-list rule, code-fold discipline.
  • R code — .lintr.R compliance, no new dependencies without justification (template repo → every dep propagates to downstream books).
  • Build hygiene — no accidental _site/ / _freeze/ / .quarto/ commits, README.Rmd vs README.md, BibTeX hygiene.
  • CI / template impact — workflow contracts (copilot-setup-steps.yml), spell/link-check failures fixed at source.

Also:

• Each run posts a fresh review comment; prior reviews stay visible on the PR (no use_sticky_comment, no delete-prior-sticky pre-step) — the team prefers a visible review history over a single rolling sticky.
• track_progress: 'true' for the checklist UI in the action's run.
• --allowedTools scoped to the inline-comment MCP + read-only gh commands + gh pr comment create (narrowed from gh pr comment:* so the reviewer can post a top-level summary but can't edit or delete prior comments).
• pull-requests: write (was read) so the action can leave inline + top-level review comments. issues: stays read.

Test plan

• Open a small no-op PR after merging, confirm the review fires, the upstream plugin's structured review appears, and the Quarto addendum is reflected in the output.
• Verify inline comments land on the right lines (the MCP tool is in --allowedTools).
• Verify subsequent pushes append a new review comment rather than editing the prior one.
• Confirm Dependabot/draft skip still works (carried over from Enable @claude workflow to open PRs #70).

🤖 Generated with Claude Code

[d-morrison]

@claude review

[d-morrison]

@claude review

[claude]

Claude finished @d-morrison's task in 1m 36s —— View job

---

PR Review

• Gathering context
• Reading changed file and diff
• Analyzing workflow logic and security
• Posting review feedback

---

Overall this is a clean, well-scoped change. The Quarto-addendum prompt is specific, actionable, and well-structured. A few things worth addressing before merge:

---

Stale PR description

The Summary section describes features that don't exist in the final file:

• "Delete-prior-sticky pre-step" — not present (intentionally removed in f4f2856)
• "use_sticky_comment: 'true'" — not present
• "issues: write (the delete-comment API is under the issues scope)" — issues stays read

The code is correct; the description just wasn't updated after f4f2856. Worth a quick edit to the PR body to avoid confusing reviewers.

---

Bash(gh pr comment:*) is broader than needed

.github/workflows/claude-code-review.yml line 87

--allowedTools "mcp__github_inline_comment__create_inline_comment,Bash(gh pr view:*),Bash(gh pr diff:*),Bash(gh pr comment:*)"

gh pr comment:* matches any gh pr comment invocation, including --edit and --delete-last. Since inline comments are handled by the MCP tool, gh pr comment is only needed if Claude wants to post a top-level review comment. If that's intentional, tightening to Bash(gh pr comment create:*) (or Bash(gh pr comment --body:*)) would reduce the blast radius. If it's not needed at all, removing it narrows the tool surface further.

Fix this →

---

Plugin pinned to a branch, not a tag

Line 50

plugins: 'code-review@claude-code-plugins'

claude-code-plugins appears to be a branch name, not a version tag. If the upstream plugin changes in a breaking or behavior-altering way, reviews will silently change on the next run. This is acknowledged in the PR description ("so we pick up future improvements") and may be intentional, but it's worth a comment in the YAML noting it's unpinned by design, so a future reader doesn't "fix" it.

---

Minor: track_progress without use_sticky_comment accumulates comments

track_progress: 'true' creates a progress-checklist comment on each run. Without use_sticky_comment, every push to a PR creates a new review comment (old ones stay). The header comment at line 8–9 says this is intentional ("keeps a visible history"). That's fine, but on a PR with many pushes this could produce significant comment volume. No change needed — just confirming this is intentional.

---

Minor: step id removed

The old step had id: claude-review. The new step drops it. Nothing else in this file references steps.claude-review.outputs.*, so this is fine as-is.

---

Summary: The prompt content is solid and the permission upgrade (pull-requests: write) is correctly scoped. Main action item is updating the PR description to match what's actually in the file, and optionally tightening the gh pr comment glob.

[d-morrison]

@claude please review the new commit 4f76551 (tightens Bash(gh pr comment:*) to Bash(gh pr comment create:*) per your previous feedback; PR body also rewritten to match what's actually in the file). Report any remaining actionable issues.

[claude]

Claude finished @d-morrison's task in 1m 29s —— View job

---

PR Review — commit 4f76551

• Gathering context
• Reading diff vs base branch
• Verifying previous feedback items are addressed
• Checking for remaining actionable issues
• Posting findings

---

Both previous actionable items are resolved:

• gh pr comment glob — correctly tightened to Bash(gh pr comment create:*) on line 90. The inline comment added above claude_args (lines 86–88) cleanly explains the intent. ✅
• PR description — rewritten per commit message; no longer describes features that were removed. ✅

---

One remaining item from the prior review

Line 50 — unpinned plugin, no comment

plugins: 'code-review@claude-code-plugins'

The previous review flagged that claude-code-plugins is a branch name, not a version tag, and suggested adding a brief inline comment so a future reader doesn't treat it as something to "fix." That comment is still absent. The one-liner is trivial:

plugins: 'code-review@claude-code-plugins'  # unpinned by design — tracks upstream improvements

Fix this →

---

New observation — id-token: write may be unnecessary

Line 36

id-token: write

The action authenticates via claude_code_oauth_token (a repository secret). The id-token: write permission grants the job the ability to request OIDC tokens from GitHub's token endpoint, which is a broader permission than needed if OAuth is the only auth path. Worth checking whether claude-code-action@v1 actually requires this permission in OAuth-token mode — if not, dropping it narrows the blast radius of a compromised workflow run.

Fix this →

---

Summary: The two previously flagged items are cleanly addressed. One minor suggestion carries over (unpinned-plugin comment); one new low-priority observation about id-token: write. Neither blocks merge.

[d-morrison]

@claude please review the new commit 9d81620 (adds inline comments explaining the unpinned plugin reference and why id-token: write is required for the App-token exchange even with OAuth auth — both per your previous feedback). Anything still actionable?

[claude]

Claude finished @d-morrison's task in 54s —— View job

---

PR Review — commit 9d81620

• Gathering context
• Reading changed file
• Verifying previous feedback items are addressed
• Checking for remaining actionable issues
• Posting findings

---

Both outstanding items from the previous round are resolved. Nothing actionable remains.

Unpinned plugin comment — lines 53–56 ✅

# `claude-code-plugins` is a branch name, not a version tag.
# Intentionally unpinned so we pick up future improvements to the
# upstream code-review skill; pin to a tag if reviews ever start
# changing in surprising ways.
plugins: 'code-review@claude-code-plugins'

Clear, includes the escape hatch ("pin to a tag if…"), and placed directly above the line it explains.

id-token: write rationale — lines 36–39 ✅

id-token: write       # required by claude-code-action for the App-token
                      # exchange it performs even with OAuth auth
                      # (without this, the action fails with
                      # "App token exchange failed: 401 Unauthorized")

The concrete error string is the right kind of documentation here — it explains what goes wrong if a future reader removes it, not just why it's there in the abstract.

---

The workflow file is clean. This PR is ready to merge once #70 lands.

[d-morrison]

@claude address comments and failing workflows

[claude]

Claude finished @d-morrison's task in 2m 1s —— View job

---

Addressing comments and failing workflows

• Read workflow file and diff to understand what was removed at line 40
• Check failing workflows
• Address review comment ("why removed?")
• Report findings

---

Review comment — "why removed?" (line 40 of base branch)

Line 40 in the base branch was id: claude-review — the step identifier on the review step. It was dropped in commit 6ab2c41 because nothing in the workflow references steps.claude-review.outputs.*. The id: field on a step only matters if something downstream consumes its outputs or outcome; with no such references, it was dead boilerplate from the GitHub-generated workflow template. The removal is correct — no action needed.

---

Failing workflow: Claude Code Review

Root cause: Expected behavior, not a PR defect.

The action performs an OIDC token exchange that validates the workflow file is identical to the default branch version before granting elevated permissions. Since this PR modifies the review workflow itself, validation fails with:

App token exchange failed: 401 Unauthorized - Workflow validation failed.
The workflow file must exist and have identical content to the version
on the repository's default branch.

The action's own error message acknowledges this: "If you're seeing this on a PR when you first add a code review workflow file to your repository, this is normal and you should ignore this error."

This failure is structural — it happens on any PR that touches .github/workflows/claude-code-review.yml. It will self-resolve once this PR merges to add-claude-pr-creation and the workflow file on the default branch matches what's running.

No change needed.

---

Failing workflow: Check Links

Root cause: Transient network connectivity failure from the Azure runner, not a broken link.

[ERROR] https://quarto.org/ | Network error: Connection failed.
  Check network connectivity and firewall settings

The Azure westcentralus runner could not reach https://quarto.org/ at the time of the run. The URL is valid; this is a runner-side connectivity blip. Both errors are the same URL (quarto.org) duplicated across chapters/chapter1.qmd and CLAUDE.md.

Recommended fix: Re-run the Check Links workflow — this class of error does not warrant adding quarto.org to the lychee exclusion list since the link is real and should remain checked. If this runner-connectivity issue recurs consistently, adding https://quarto.org/.* to lychee.toml's exclude list would be the pragmatic fix, but a single re-run is the right first step.

---

Bottom line: Neither failure is caused by the changes in this PR. The Claude review failure is a known, expected limitation when reviewing workflow-file changes; the link failure is transient. The PR is ready to merge once #70 lands — re-run Check Links first to get a clean signal.

[d-morrison]

@claude the link issue reoccured: https://github.com/d-morrison/qwt/actions/runs/26112538437/job/76794986372?pr=71 why?

[claude]

Claude finished @d-morrison's task in 1m 5s —— View job

---

• Fetching link-check job log
• Analyzing failure cause
• Posting findings

---

Why the link check keeps failing

Short answer: quarto.org is blocking requests from GitHub Actions runner IPs. This is not a transient network blip — it's a structural incompatibility between lychee's HTTP client and quarto.org's network policy.

Evidence from the log:

[ERROR] https://quarto.org/ | Network error: Connection failed …
[ERROR] https://quarto.org/docs/get-started/ | Network error: Connection failed …

The failures appear at timestamp 17:12:06.951 — roughly 80 milliseconds after lychee started at 17:12:06.836. A real network timeout would take seconds (lychee defaults to 20s). An 80ms "connection failed" means the TCP connection was refused or dropped by a firewall before it was ever established. quarto.org (served via Cloudflare) is actively refusing connections from GitHub Actions runner IP ranges, which is a well-known pattern for sites that block scraper/bot traffic from cloud provider CIDRs.

Both failures are also consistent across Azure regions (westcentralus the first time, westus this time), confirming the block is at the destination side, not a runner-pool anomaly.

What to do:

The right fix here is to add quarto.org to the lychee exclusion list in lychee.toml. The URLs are real and valid — they just can't be verified by lychee from GitHub Actions runners. A comment in the config explains why:

exclude = [
  # quarto.org blocks GitHub Actions runner IPs (Cloudflare firewall);
  # the URLs are valid but lychee cannot reach them from CI
  "https://quarto.org/.*",
]

Fix this →

The "links checked by hand" label is a one-PR workaround, not a root-cause fix — you'd need to apply it on every PR that touches files referencing quarto.org, which is most of them.

[d-morrison]

@claude please review the new commit 19de75f (mirrors PR #70 round-4's same-repo guard fix to this branch's claude-code-review.yml so the merge from main won't conflict when #70 lands). Anything still actionable?

[claude]

Claude finished @d-morrison's task in 54s —— View job

---

PR Review — commit 19de75f

• Reading diff vs base branch
• Verifying same-repo guard is correctly ported
• Checking for remaining actionable issues
• Posting findings

---

The same-repo guard is correctly ported. The three-condition if: block (lines 31–34) is syntactically valid YAML, uses the right expression path (head.repo.full_name == github.repository), and the inline comment cleanly explains all three skip conditions and why fork PRs are excluded.

One minor nit — the file-level header comment at line 10:

# Skips PRs from Dependabot and from drafts. Project guidance is in CLAUDE.md.

This predates 19de75f and wasn't updated — fork PRs are now a third skip condition but aren't mentioned here. The inline if: comment at lines 28–30 does cover it, so a reader won't be confused, but strictly the top-level summary is now incomplete.

# Skips drafts, Dependabot bumps, and fork PRs. Project guidance is in CLAUDE.md.

This is too minor to hold the PR — call it optional cleanup.

---

All prior feedback items remain resolved. Nothing actionable blocks merge. The PR is ready once #70 lands; this commit correctly pre-empts the conflict that would arise when #70's version of claude-code-review.yml hits the base branch.

[d-morrison]

@claude please review the new commit 8660f27 (header comment update for fork-PR skip, per your previous nit). Anything still actionable?

[claude]

Claude finished @d-morrison's task in 37s —— View job

---

PR Review — commit 8660f27

• Gathering context
• Reading diff vs base branch
• Verifying header comment update
• Checking for remaining actionable issues
• Posting findings

---

The nit is addressed. Lines 11–13 now read:

# Skips drafts, Dependabot bumps, and fork PRs (fork PRs can't read repo
# secrets). Project guidance is in CLAUDE.md. Requires the
# CLAUDE_CODE_OAUTH_TOKEN repository secret.

That matches all three conditions in the if: guard and the inline comment block at lines 29–31. The parenthetical reason ("fork PRs can't read repo secrets") is a nice touch — same rationale as the inline comment, just shorter.

---

Nothing actionable remains. The workflow file is clean end-to-end. This PR is ready to merge once #70 lands.

[d-morrison]

Closing+reopening to force GitHub to recompute a stuck CONFLICTING mergeability flag — local git confirms origin/main is a clean ancestor of this branch and the merge is conflict-free (net diff is just claude-code-review.yml). No content change.

[d-morrison]

Claude Code CLI (local session) is working on this — paws off until I'm done.

[d-morrison]

Done with this local session — unclaimed. Addressed the open inline comment (explained the removed step id); no code change. Paws back on.

[github-actions]

PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-05-25 19:00 PDT