Repository navigation
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
There was a problem hiding this comment.
🟡 Changes recommended
The CLI regression test is not run in CI, and its guard may miss the --user conflict warning.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds WP-CLI regression coverage for global --user handling and updates CLI documentation and examples.
Changes:
- Adds shell and PHP fixtures for registration and user-context checks.
- Adds the
npm run test:clicommand. - Corrects generated examples and documents
--useras a global option.
File summaries
| File | Description | Review notes |
|---|---|---|
tests/cli/test-serve-user-context.sh |
Runs registration and user-context checks. | Moderate: assert that the global/local --user conflict warning is absent. |
tests/cli/assert-serve-user-context.php |
Validates the active WordPress user. | No final comments. |
package.json |
Defines the CLI test command. | Moderate: add npm run test:cli to CI after the WP environment starts. |
includes/Cli/McpCommand.php |
Corrects generated command examples. | No final comments. |
docs/guides/cli-usage.md |
Documents global --user usage. |
No final comments. |
Review details
Suppressed comments (1)
tests/cli/test-serve-user-context.sh:12
- This guard does not actually cover the regression described in #328: it only rejects output containing
already registered, while WP-CLI's global/local--usercollision can be reported as a conflict warning. Because the later assertion only requiresThe STDIO transport is disabled., a collision warning could be emitted and this test would still pass. Assert the collision-warning text (or otherwise verify it is absent) in the help output so reintroducing the conflicting option fails the test.
case "$help_output" in
*"already registered"*)
printf '%s\n' "$help_output" >&2
exit 1
;;
esac
- Files reviewed: 5/5 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| "lint:php:fix": "wp-env run cli --env-cwd=wp-content/plugins/$(basename \"$(pwd)\")/ composer run-script format", | ||
| "lint:php:stan": "wp-env run cli --env-cwd=wp-content/plugins/$(basename \"$(pwd)\")/ composer run-script phpstan", | ||
| "plugin-zip": "wp-scripts plugin-zip", | ||
| "test:cli": "npm run wp-env:test -- run cli --env-cwd=wp-content/plugins/$(basename \"$(pwd)\")/ sh tests/cli/test-serve-user-context.sh", |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## trunk #329 +/- ##
=========================================
Coverage 88.78% 88.78%
Complexity 1264 1264
=========================================
Files 54 54
Lines 4164 4164
=========================================
Hits 3697 3697
Misses 467 467
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Summary
--userremains a WP-CLI global option, not a serve optionmcp-adapterFixes #328.
Testing
composer run-script lintcomposer run-script phpstanwp --skip-wordpress --require=/tmp/register-command.php help mcp-adapter servenpm run test:cliis ready for CI; local execution is blocked because Docker is unavailable.AI Assistance Disclosure
OpenCode using
openai/gpt-5.6-terraauthored this change under Chris Huber's authorization. The regression fixture uses only synthetic user-context assertions.