Add Poseidon PR Review Command#129
Conversation
Allows users to run AI code reviews on pull requests using local `gh` and `claude` CLIs. Enhances accessibility across different repositories, including those outside the a8cteam51 organization. Synced with Poseidon yaml file.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds a new Symfony Console command 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
commands/Poseidon_PR_Review.php (2)
129-142: ⚡ Quick winReplace
exit()calls with exceptions for proper Symfony Console lifecycle.Using
exit()ininitialize()bypasses Symfony Console's error handling and makes the command untestable. ThrowRuntimeExceptioninstead—Symfony Console will catch it, display the error, and return the appropriate exit code.Proposed fix
if ( ! $this->command_exists( 'claude' ) ) { - $output->writeln( '<error>The `claude` CLI is not installed or not on PATH. Install Claude Code and try again.</error>' ); - exit( Command::FAILURE ); + throw new \RuntimeException( 'The `claude` CLI is not installed or not on PATH. Install Claude Code and try again.' ); } $this->gh_username = $this->resolve_gh_username( $output ); if ( null === $this->gh_username ) { - exit( Command::FAILURE ); + throw new \RuntimeException( 'Could not resolve GitHub username. See error above.' ); } $this->pr_title = $this->resolve_pr_title( $output ); if ( null === $this->pr_title ) { - exit( Command::FAILURE ); + throw new \RuntimeException( 'Could not access the pull request. See error above.' ); }🤖 Prompt for 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. In `@commands/Poseidon_PR_Review.php` around lines 129 - 142, The initialize() flow currently calls exit() in three places — when command_exists('claude') is false and after resolve_gh_username() or resolve_pr_title() return null — which bypasses Symfony Console; replace each exit(Command::FAILURE) with throwing a RuntimeException (e.g. throw new \RuntimeException('The `claude` CLI is not installed or not on PATH. Install Claude Code and try again.');) and for the other two cases throw meaningful RuntimeException messages where resolve_gh_username() or resolve_pr_title() return null; keep the same output text presently sent to $output->writeln and ensure you import or fully qualify RuntimeException so Symfony handles the error lifecycle and exit code.
384-393: 💤 Low valueQuote parsing doesn't handle escaped quotes in the YAML string.
strpos($yaml, '"', $start)finds the first unescaped double-quote, but if the upstream prompt contains\"(escaped quotes), this will truncate the extracted prompt prematurely. The sanity check at line 395 helps catch this, but a malformed prompt extraction would produce a confusing error.Consider using a proper YAML parser like
symfony/yamlfor robustness, or at minimum handle\"sequences.🤖 Prompt for 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. In `@commands/Poseidon_PR_Review.php` around lines 384 - 393, The current extraction of the system prompt using strpos($yaml, '"', $start) is vulnerable to escaped quotes and should be replaced: either parse the action.yml with a proper YAML parser (e.g., symfony/yaml's Yaml::parseFile and then pull the system prompt key) or, if you must keep string-scanning, replace the strpos call with a loop that advances from $start and finds the next unescaped double-quote (skip any \" by checking preceding backslashes) before computing $end; update the subsequent $prompt/substr and preg_replace logic to use that safe $end and still trim indentation from $prompt derived from $matches.
🤖 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 `@commands/Poseidon_PR_Review.php`:
- Around line 1-2: This file is missing the strict types declaration; add
declare(strict_types=1); immediately after the opening <?php tag (before any
code, class or function definitions) so the typed properties and return type
declarations in this file are enforced at runtime.
- Around line 217-241: The Process instantiation that builds $claude is passing
'--allowedTools' followed by multiple separate args
('Bash','Read','Write','Edit','Glob','Grep','WebFetch','WebSearch'); change this
so '--allowedTools' is followed by a single string argument with comma-separated
tool rules (e.g. "--allowedTools", "Read,Edit,Bash,WebFetch") or remove the
'--allowedTools' entry entirely if it's redundant with '--permission-mode'
bypassPermissions; update the args array in the Process constructor where
$claude is created to use the single-string form (reference the '--allowedTools'
token and the $claude Process creation to locate the code).
---
Nitpick comments:
In `@commands/Poseidon_PR_Review.php`:
- Around line 129-142: The initialize() flow currently calls exit() in three
places — when command_exists('claude') is false and after resolve_gh_username()
or resolve_pr_title() return null — which bypasses Symfony Console; replace each
exit(Command::FAILURE) with throwing a RuntimeException (e.g. throw new
\RuntimeException('The `claude` CLI is not installed or not on PATH. Install
Claude Code and try again.');) and for the other two cases throw meaningful
RuntimeException messages where resolve_gh_username() or resolve_pr_title()
return null; keep the same output text presently sent to $output->writeln and
ensure you import or fully qualify RuntimeException so Symfony handles the error
lifecycle and exit code.
- Around line 384-393: The current extraction of the system prompt using
strpos($yaml, '"', $start) is vulnerable to escaped quotes and should be
replaced: either parse the action.yml with a proper YAML parser (e.g.,
symfony/yaml's Yaml::parseFile and then pull the system prompt key) or, if you
must keep string-scanning, replace the strpos call with a loop that advances
from $start and finds the next unescaped double-quote (skip any \" by checking
preceding backslashes) before computing $end; update the subsequent
$prompt/substr and preg_replace logic to use that safe $end and still trim
indentation from $prompt derived from $matches.
🪄 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: b33da8ee-66db-4b87-995b-c78e03ddcc1b
📒 Files selected for processing (1)
commands/Poseidon_PR_Review.php
| <?php | ||
|
|
There was a problem hiding this comment.
Add declare(strict_types=1); declaration.
As per coding guidelines, PHP files should use declare(strict_types=1); where appropriate. This new file uses typed properties and return type declarations throughout, making strict types appropriate.
Proposed fix
<?php
+declare(strict_types=1);
namespace WPCOMSpecialProjects\CLI\Command;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <?php | |
| <?php | |
| declare(strict_types=1); | |
| namespace WPCOMSpecialProjects\CLI\Command; |
🤖 Prompt for 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.
In `@commands/Poseidon_PR_Review.php` around lines 1 - 2, This file is missing the
strict types declaration; add declare(strict_types=1); immediately after the
opening <?php tag (before any code, class or function definitions) so the typed
properties and return type declarations in this file are enforced at runtime.
There was a problem hiding this comment.
Other commands don't have it, for consistency, I prefer not to add it.
There was a problem hiding this comment.
@ecairol, understood — keeping consistency with the existing command files makes sense. I'll note this for future reviews.
✏️ Learnings added
Learnt from: ecairol
Repo: a8cteam51/team51-cli PR: 129
File: commands/Poseidon_PR_Review.php:1-2
Timestamp: 2026-05-31T02:11:12.697Z
Learning: In the `a8cteam51/team51-cli` repository, PHP command files under the `commands/` directory do not use `declare(strict_types=1);`. Do not suggest adding strict types declarations to files in this directory, as the project convention is to omit it for consistency across all commands.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
|
👋 I learned last week that there is an ongoing Systems project to move repos from the github.com/Automattic to the Automattic org in GitHub Enterprise; that is the reason why all our public email were added to GHE a couple of weeks ago; Does this still need to exist if we will be getting the Claude Opus 4.8 review on each PR once we get the repos in GHE? Maybe a cleaner solution would be to simply install this skill locally for work on WPCOM in the mean time? https://github.a8c.com/Automattic/claude-code-plugins/tree/trunk/plugins/systems-review |
|
@ahegyes which repos will be moved? All of them? Even our sites/plugins? I think we can add this because I just figured out that Poseidon Reviews don't work on our public repos, for example, Team51 CLI, and this is a good way to be able to review it. I think the systems skill is mostly for work on WPCOM, or is it a general review? @ecairol before merging this I would fix the commands so if you just use |
I'm not entirely sure but I think the current scope are all private repos on http://github.com/Automattic |
…n:pr-review command
3 participants
What
Adds a new CLI command,
poseidon:pr-review, that runs the Poseidon AI code review against a pull request from your local machine, using your ownghandclaudeauthentication (no API keys required).Why
To extend
poseidon-actions/pr-reviewability to GitHub organizations outsidea8cteam51.Specific use case: PR reviews on Automattic's GitHub org, eg:
Automattic/wp-calypso,Automattic/senseiHow it works
OWNER/REPO#Nand pre-flights access withgh pr view.a8cteam51/poseidon-actions, so it always uses the latest upstream version automaticallygh pr checkout --detach, so the agent can readAGENTS.md/.agents/and surroundingcode — mirroring the Action's
actions/checkout.claudeCLI headless against the PR.finally.ghusername). Otherwise, the review is printed on the TerminalUsage
Summary by CodeRabbit