Polish review app command UX before 5.0.0 stable#285
Conversation
Rename the generated PR commands to a single namespaced family:
/deploy-review-app -> +review-app-deploy
/delete-review-app -> +review-app-delete
/help -> +review-app-help
Updates the trigger workflows, the PR-open quick reference, the long-form
help (.github/cpflow-help.md), the AI rollout prompt, and the CI automation
docs to use the new names consistently. Existing repos that have already
generated this flow will need to regenerate the workflows or update the
old command names by hand; this is acceptable now because the flow has
not yet shipped in a stable release (currently 5.0.0.rc.0).
Help message changes (.github/cpflow-help.md):
- Self-identify the message ("You asked for review app help. These commands
are generated by cpflow.") so PR readers know what they are looking at.
- Rename "Repository secrets / variables" to "GitHub Actions secrets /
variables" to match where they are configured.
- Clarify Control Plane wording: tokens are scoped to a Control Plane org
on controlplane.com; orgs are named accordingly.
- Wrap the secrets/variables tables in a collapsed <details> block so
casual readers see the commands without scrolling past a wall of config.
- Add an "Advanced: testing changes to generated workflows" section that
documents the gh workflow run pattern for testing PR-branch workflow
edits before merge.
PR-open message (cpflow-review-app-help.yml):
- Drop the long quick reference; list the three commands and tell readers
to comment +review-app-help for setup details.
Contributing docs:
- Add a "Developing the GitHub flow generator" section explaining where
the templates live, which surface area to keep consistent on a command
rename, how to run the generator spec, where actionlint applies, and
how to dispatch comment-trigger workflows on a PR branch with gh
workflow run before merge.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
WalkthroughThis PR migrates review app GitHub Actions comment commands from slash-style syntax ( ChangesReview App Command Syntax Migration
🎯 2 (Simple) | ⏱️ ~8 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Code ReviewOverviewThis PR renames the generated review-app PR comment commands from What's working well
IssuesMissing CHANGELOG entryThe Spec gap: trigger command strings are not assertedThe specs verify Minor notes
|
Greptile SummaryThis PR renames the three generated PR comment commands from the old
Confidence Score: 5/5Safe to merge — every command-name reference across workflows, documentation, and the AI prompt has been updated consistently with no old names remaining. The change is a pure string rename across nine files. All six trigger-condition strings in the workflow YAML, every prose reference in docs, the help markdown, and the Ruby AI-prompt heredoc were updated in one coherent pass. A grep for the old No files require special attention. Important Files Changed
Reviews (1): Last reviewed commit: "Polish review app command UX before 5.0...." | Re-trigger Greptile |
- CHANGELOG: add Unreleased Changed entry for the +review-app-* rename - Spec: pin the public +review-app-deploy/-delete/-help trigger strings so a future template edit cannot silently rename them - cpflow-help.md: spell out the exact-match rule on the +review-app-delete bullet so it is self-contained for first-time readers - cpflow-help-command.yml: comment why workflow_dispatch intentionally skips the author_association gate (GitHub already restricts manual dispatches to actions:write users) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Review feedback addressed (8f6bbeb)From @claude[bot]'s code review
Other comments — triaged, no action needed
Verification: |
|
You have reached your Codex usage limits. You can see your limits in the Codex usage dashboard. |
|
The test count going from 39 → 40 with all checks clean is a good sign. Happy to do another pass if you'd like a fresh review of the new commit. ᶘ ᵒᴥᵒᶅ ✅ Actions performedReview triggered.
|
|
All four points addressed cleanly — thanks for the thorough follow-up summary. Quick verification of each item:
One minor observation (no action needed before merge): the CI is still pending. Once that's green and you've done the live Tip: You can customize Greptile's behavior for this repo with |
Review: Polish review app command UX before 5.0.0 stableOverviewClean, targeted rename that namespaces the three PR comment triggers under a single Positives
Issues1. Spec gap — The new test pins trigger strings in the three event-driven workflows (deploy, delete, help-command) but not the PR-open quick reference in def pr_open_help_workflow_path
playground.join(".github/workflows/cpflow-review-app-help.yml")
end
# in the pinning test:
expect(pr_open_help_workflow_path.read).to include("+review-app-deploy")
expect(pr_open_help_workflow_path.read).to include("+review-app-delete")
expect(pr_open_help_workflow_path.read).to include("+review-app-help")2. The 3. CONTRIBUTING.md The bullet gives SecurityNo new concerns. The |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
- Extend the trigger-string spec so it also pins the +review-app-* command names in the PR-open quick-reference workflow (cpflow-review-app-help.yml). A future edit to that bullet list cannot now silently drift away from the real triggers. - Add the same exact-equality caveat to the +review-app-help bullet in cpflow-help.md that the +review-app-deploy and +review-app-delete bullets already carry. - Spell out cpflow-delete-review-app.yml and cpflow-help-command.yml explicitly in the CONTRIBUTING.md gh workflow run example so a developer reading that section does not have to cross-reference cpflow-help.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Second review-app rename pass addressed (08ac574)Closing out the three remaining nits from @claude[bot]'s most recent reviews. Each was addressed via the inline threads (now resolved):
Other comments — triaged, no action
Verification: |
|
You have reached your Codex usage limits. You can see your limits in the Codex usage dashboard. |
|
All three items check out cleanly in the diff:
The surrounding changes (the |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/github_flow_templates/.github/cpflow-help.md`:
- Line 63: Update the trigger behavior sentence to say “default branch” instead
of “base branch”: in the sentence that describes comment-triggered runs
(`+review-app-deploy`, `+review-app-delete`, `+review-app-help`) replace the
phrase “the base branch — not your PR branch” with wording that states GitHub
resolves workflow definitions from the repository’s default branch (e.g., “the
default branch — not your PR branch”) so the doc accurately reflects trigger
resolution.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 47bf45da-1598-41df-8f65-27b9b7174eb0
📒 Files selected for processing (3)
CONTRIBUTING.mdlib/github_flow_templates/.github/cpflow-help.mdspec/command/generate_github_actions_spec.rb
🚧 Files skipped from review as they are similar to previous changes (1)
- spec/command/generate_github_actions_spec.rb
|
Tip For best results, initiate chat on the files or code changes.
40 examples, 0 failures and a clean RuboCop run are good closing signals. Happy to do a fresh incremental review of the 08ac574 changes if you'd like a final pass before merge — just let me know! ᶘ ᵒᴥᵒᶅ |
Code ReviewOverallClean, consistent rename. All command strings are updated across templates, docs, the AI prompt, and the spec in one coherent sweep — no stale Security
IssuesUX: the shortened PR-open message assumes all authors can run The new quick-reference comment tells every PR author to "comment
Spec: The three trigger-string checks correctly assert the full expression Minor
|
| "", | ||
| "`/help`", | ||
| "Show the required GitHub variables, secrets, and workflow behavior." | ||
| "For setup details, comment `+review-app-help`." |
There was a problem hiding this comment.
This tells every PR author — including external contributors — to comment +review-app-help for setup details, but that command is gated on author_association (OWNER / MEMBER / COLLABORATOR). A contributor without those roles will get a silent no-op with no explanation.
The old message avoided this problem by surfacing all the config details upfront. Consider either adding a caveat ("repo members only") here, or keeping at least a summary of the required secrets/variables visible to everyone.
|
Saw your post about Polish review app command UX before 5.0.0 stable. I deploy custom AI assistants — local models, persistent memory, no subscriptions. Your data stays yours. Price: $600, 48h. Free 15-min scope call first if you want. — Lyn Reply or email: lynchatta@gmail.com |
Polishing nits from the latest reviewer round: - cpflow-help.md and CONTRIBUTING.md now say comment-triggered runs use the workflow code from the repository's default branch, not the base branch. GitHub resolves issue_comment workflows from the default branch, which is technically distinct from the PR's base branch and matters for repos that target non-default branches. - The pr_open_help block in the generator spec now pins each command string to its actual bullet form (`"- \`+review-app-deploy\``), so a silent rename in the trigger-relevant text fails the spec even if the bare command name still appears elsewhere in the file. - CHANGELOG entry now explains why `+` was picked over `/` (avoid collision with GitHub's reserved slash-command surface and other bots that listen on `/help`, and to make the three commands obviously part of one namespaced family).
Third review-app rename pass addressed (9a5ecbf)Closing out the three OPTIONAL nits from the most recent round; one item declined for this PR. Addressed inline (threads resolved)
Addressed in docs
Declined for this PR — leaving thread open
Triage of non-actionable comments
Verification: |
|
Tip For best results, initiate chat on the files or code changes.
The deferral on the external-contributor silent no-op is a reasonable call for pre-stable — it's genuinely a UX design decision (PR-open summary vs. caveat text) that deserves its own focused discussion rather than a hasty last-minute patch. 40 examples, 0 failures and a clean RuboCop run are solid closing signals. Looks merge-ready from my side! ᶘ ᵒᴥᵒᶅ |
|
Tip For best results, initiate chat on the files or code changes.
The deferral on the external-contributor silent no-op is the right call for a pre-stable release — it's a UX policy decision (caveat vs. summary vs. redirect) that deserves its own issue rather than a last-minute patch. Leaving the thread open is a good breadcrumb. 40 examples, 0 failures, RuboCop clean — looks ready to merge. 🎉 |
|
Overall: clean and well-executed. More details in inline comments. |
|
Overall: clean and well-executed. The rename rationale is sound, changes are consistent across all relevant files, and test coverage additions are appropriate. |
|
Strengths:
|
|
Issue 1 -- Spec gap: The new test pins trigger strings in workflow Suggest adding to the help_md = playground.join(".github/cpflow-help.md").read
expect(help_md).to include("`+review-app-deploy`")
expect(help_md).to include("`+review-app-delete`")
expect(help_md).to include("`+review-app-help`") |
|
Issue 2 -- Spec test name covers two distinct concerns
Issue 3 -- Brittle JS-literal matching in spec (low priority) Lines 237-239 match raw JS string literal syntax including the leading expect(pr_open_help).to include('"- `+review-app-deploy`')This is intentional but would silently break if the template switches from array-join to a heredoc or template literal. A brief comment explaining the leading Minor nit: The CHANGELOG migration note says "update saved instructions or runbooks" -- consider also naming the generated |
| pr_open_help = pr_open_help_workflow_path.read | ||
| expect(pr_open_help).to include('"- `+review-app-deploy`') | ||
| expect(pr_open_help).to include('"- `+review-app-delete`') | ||
| expect(pr_open_help).to include('"- `+review-app-help`') |
There was a problem hiding this comment.
The leading " is load-bearing: it matches the opening quote of the JS array element in the template ("- \+review-app-deploy` - create or redeploy..."). A future maintainer reading this without context might trim it thinking it's a stray character. Worth a short comment here, and consider also extending the itblock with assertions oncpflow-help.md` itself:
# The leading " is the opening quote of the JS array string literal in cpflow-review-app-help.yml
expect(pr_open_help).to include('"- `+review-app-deploy`')
# Also pin command names in the long-form help markdown
help_md = playground.join(".github/cpflow-help.md").read
expect(help_md).to include("`+review-app-deploy`")
expect(help_md).to include("`+review-app-delete`")
expect(help_md).to include("`+review-app-help`")Without the cpflow-help.md assertions a rename could update workflow triggers and miss the help text entirely.
| expect(contents).to include('fs.readFileSync(".github/cpflow-help.md"') | ||
| end | ||
|
|
||
| it "pins the public +review-app-* PR comment trigger strings" do |
There was a problem hiding this comment.
The test name "pins the public +review-app-* PR comment trigger strings" covers two distinct concerns: workflow if: trigger conditions (lines 232-234) and the PR-open comment body (lines 236-239). If the PR-open assertions fail the error message names a test called "trigger strings" which is mildly misleading. Consider splitting into two it blocks or broadening the name to "pins the +review-app-* command strings in workflows and PR-open message".
Code ReviewOverviewThis PR renames the generated review-app PR comment triggers from The rename is clean, well-reasoned (avoids GitHub's reserved CorrectnessRename coverage is complete. Every touch-point is updated:
A scan for old slash-command strings confirms none remain outside the CHANGELOG's migration note. ✅ SecurityThe security posture is unchanged and well-maintained:
Test CoverageThe three new spec examples pin the new command names across all three surfaces (workflow One minor observation on the PR-open spec (see inline comment). Potential Issues1. The PR-open workflow only fires on the 2. Exact-equality match UX friction The 3. Placeholder inconsistency between the two
SummaryWell-executed, low-risk polish PR with clear rationale, thorough coverage, and solid test protection. The migration note is honest about the breaking change. Issues raised are minor nits — none are blockers. Verdict: Approve with minor nits. ✅ |
|
|
||
| expect(pr_open_help).to include('"Repo owners, members, and collaborators can use these commands:"') | ||
| # The leading " is the opening quote of the JS array string literal in cpflow-review-app-help.yml. | ||
| expect(pr_open_help).to include('"- `+review-app-deploy`') |
There was a problem hiding this comment.
The leading " prefix in these assertions is coupling the test to the YAML JS array string-literal syntax. If the template is ever reformatted (e.g., single quotes or a template-literal instead of a JS array), these assertions will fail for a reason unrelated to the actual command name.
Consider asserting on the rendered content instead — e.g. check that the comment body that GitHub would receive contains the strings, not that the YAML source does:
| expect(pr_open_help).to include('"- `+review-app-deploy`') | |
| expect(pr_open_help).to include('`+review-app-deploy`') |
The body value itself (+review-app-deploy) is what actually matters for the trigger; the surrounding YAML encoding is an implementation detail.
| - **Test PR-branch workflow edits in a real repo.** Comment-triggered runs (`+review-app-deploy`, `+review-app-delete`, `+review-app-help`) execute the workflow code from the repository's default branch, so they will not exercise your PR-branch changes. Generate the workflows into a downstream test repo, push to a feature branch, then dispatch each affected workflow with `gh`: | ||
|
|
||
| ```sh | ||
| gh workflow run cpflow-deploy-review-app.yml --ref <your-branch> -f pr_number=<pr> |
There was a problem hiding this comment.
Placeholder naming differs from the matching block in lib/github_flow_templates/.github/cpflow-help.md, which uses <your-pr-branch> and <pr-number>. Aligning them reduces friction for readers comparing both docs:
| gh workflow run cpflow-deploy-review-app.yml --ref <your-branch> -f pr_number=<pr> | |
| gh workflow run cpflow-deploy-review-app.yml --ref <your-pr-branch> -f pr_number=<pr-number> |
Code ReviewOverviewClean UX polish before 5.0.0 stable. The rename from What's well done
IssuesMinor — inconsistent punctuation in The Observation — The pattern handles Observation — spec assertions are string-inclusion only The new specs verify that command strings appear somewhere in the generated files, not that they're structurally in the correct position (e.g., inside the VerdictNo blocking issues. The rename is a net improvement to the UX surface, and the implementation is clean and consistent across all touch points. The migration note is appropriately scoped. Approving with the minor punctuation nit noted inline. |
| - Comment body must be exactly `+review-app-delete`, with no surrounding text or trailing spaces. A single trailing newline from GitHub's comment editor is accepted. Same command-match rule as `+review-app-deploy`. | ||
|
|
||
| `+review-app-help` | ||
| - Posts this message on the PR. |
There was a problem hiding this comment.
Minor punctuation inconsistency: this bullet ends with a period, but the equivalent first bullets under +review-app-deploy ("Creates the review app if it does not exist") and +review-app-delete ("Deletes the review app when the PR is done") do not.
| - Posts this message on the PR. | |
| - Posts this message on the PR |
| (github.event_name == 'issue_comment' && | ||
| github.event.issue.pull_request && | ||
| github.event.comment.body == '/deploy-review-app' && | ||
| contains(fromJson('["+review-app-deploy","+review-app-deploy\n","+review-app-deploy\r\n"]'), github.event.comment.body) && |
There was a problem hiding this comment.
The contains(fromJson([...])) pattern is correct and handles GitHub's actual trailing-newline behavior well. One note for future maintainers: the three-element array covers the exact string, \n-terminated, and \r\n-terminated variants. If GitHub ever adds a leading/trailing space (e.g., from a mobile editor auto-correction), those would silently no-op. The help text already documents this boundary clearly, so this is just a heads-up rather than a request to change anything.
2 participants
Summary
Polishes the generated review app PR-command UX based on observations from the react-webpack-rails-tutorial #734 deploys. Lands before 5.0.0 stable so the public surface ships clean.
Command rename to a single namespaced family:
/deploy-review-app→+review-app-deploy/delete-review-app→+review-app-delete/help→+review-app-helpPR-open message is now short — just lists the three commands and tells the reader to comment
+review-app-helpfor setup details, instead of dumping the full quick reference.+review-app-helpreply (.github/cpflow-help.md) now:"You asked for review app help. These commands are generated by cpflow.") so PR readers know what they're looking at.Repository secrets→GitHub Actions secrets,Repository variables→GitHub Actions variables(which is where they're configured).<details>block so casual readers see commands without scrolling past a wall of config.Advanced: testing changes to generated workflowssection documenting thegh workflow run --ref <pr-branch>pattern for testing PR-branch workflow edits before merge.Contributing docs:
Developing the GitHub flow generatorsection inCONTRIBUTING.mdexplaining template locations, surface area to keep consistent on a command rename, the generator spec (bundle exec rspec spec/command/generate_github_actions_spec.rb),actionlintusage, and how to dispatch comment-trigger workflows on a PR branch withgh workflow runbefore merge.Migration note: Repos that have already generated this flow will need to regenerate the workflows or hand-edit the old command names. Acceptable because the flow only shipped in 5.0.0.rc.0 — this is the right moment to fix the public surface before stable.
Test plan
bundle exec rspec spec/command/generate_github_actions_spec.rb— 39 examples, 0 failuresbundle exec rubocop— 187 files, no offensesactionlint lib/github_flow_templates/.github/workflows/*.yml— only a pre-existing SC2129 style note incpflow-deploy-review-app.yml:94unrelated to this change+review-app-helpposts the rewritten message; PR-open posts the short quick reference)🤖 Generated with Claude Code
Note
Medium Risk
Medium risk because it changes the public comment-command surface and the exact-match trigger logic in generated GitHub Actions workflows, which can break existing repos until they regenerate templates or update runbooks.
Overview
Renames the generated review-app PR comment commands from slash commands to a namespaced
+review-app-*family (+review-app-deploy,+review-app-delete,+review-app-help) and updates all generated workflow triggers to accept exact matches including GitHub’s trailing newline variants.Polishes the generated help UX: the PR-open message is shortened to a command list, and
.github/cpflow-help.mdis rewritten to be clearer (updated wording for secrets/variables, collapsible advanced config/testing sections) and aligned across docs and the AI rollout prompt.Extends the generator spec to assert the new command strings across workflows/help text, and adds contributor guidance for editing/linting/testing the GitHub flow templates.
Reviewed by Cursor Bugbot for commit 1328734. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
Documentation
+review-app-deploy,+review-app-delete, and+review-app-helpacross docs, help text, generated workflows, and changelog.Tests
+review-app-*comment triggers.