GH#988: docs: improve AGENTS.md guidance for 4 recurring contributor error patterns#989
Conversation
…erns (GH#988) Address 4 recurring error patterns observed in contributor sessions: - bash:other (77x): document phpcbf exit code 1 behavior on successful auto-fixes - edit:not_read_first (46x): add prominent session-restart read-state warning - webfetch:other (39x): make the list of failing URL domains explicit and specific - read:file_not_found (38x): expand the non-existent paths table with 7 more entries Ref #988
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
📝 WalkthroughWalkthroughThis PR updates AGENTS.md documentation to address recurring agent failure patterns, including expanded path existence checks, clarified webfetch restrictions, documented session state behavior, and detailed phpcbf exit codes with post-fix file reading requirements. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
|
Performance Test Results Performance test results for f76e458 are in 🛎️! Note: the numbers in parentheses show the difference to the previous (baseline) test run. Differences below 2% or 0.5 in absolute values are not shown. URL:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@AGENTS.md`:
- Around line 519-522: Replace the suggested "vendor/bin/phpcbf ... || true"
pattern because it masks real failures; instead update the docs to run
"vendor/bin/phpcbf inc/path/to/file.php", capture its exit status, and
explicitly fail only when the status is greater than 1 (printing an error and
exiting with that status) while allowing 0 (clean) and 1 (fixes applied) to
proceed; specifically remove the "|| true" recommendation and show using the
phpcbf command, storing "$?" and checking if status > 1 to echo a failure
message and exit with that code.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| ```bash | ||
| vendor/bin/phpcbf inc/path/to/file.php # exits 1 even when it fixed everything (false failure) | ||
| vendor/bin/phpcbf inc/path/to/file.php || true # correct — ignore expected exit code 1 | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What are the official PHPCBF exit codes (0, 1, 2+) and their meanings in PHP_CodeSniffer documentation?
💡 Result:
The official PHPCBF exit codes, as documented in the PHP_CodeSniffer (PHPCSStandards) wiki "Advanced Usage" page (current as of PHP_CodeSniffer 4.0.0+), are bit-wise cumulative flags with the following base meanings (applicable to phpcbf): - 0: clean code base / auto-fixed with no issues remaining / successful non-scan request - 1: issues found/remaining, auto-fixable - 2: issues found/remaining, non-auto-fixable - 4: failure to fix some files/fixer conflict (phpcbf only) - 16: processing error - blocking the actual run of PHP_CodeSniffer (e.g., parse error in XML ruleset) - 64: requirements for running not met (e.g., PHP version or missing extensions) Common combinations include: - 3 (1+2): mix of auto-fixable and non-auto-fixable issues (phpcs only, but relevant context) - 5 (1+4): auto-fixable issues, but some failed to fix (phpcbf only) - 7 (1+2+4): mix, with some failed to fix (phpcbf only) Exit codes can be modified via config flags like ignore_errors_on_exit, ignore_warnings_on_exit, ignore_non_auto_fixable_on_exit. For legacy PHP_CodeSniffer 3.x: - 0: no fixable errors found, nothing fixed - 1: all fixable errors fixed correctly - 2: failed to fix some fixable errors - 3: processing error Since today is 2026-04-29, version 4+ semantics apply (revision implemented by May 2025 per issues).
Citations:
- 1: https://github.com/PHPCSStandards/PHP_CodeSniffer/wiki/Advanced-Usage
- 2: https://github.com/PHPCSStandards/PHP_CodeSniffer/wiki/Reporting
- 3: 4.0 | Revise exit codes for PHPCS and PHPCBF commands PHPCSStandards/PHP_CodeSniffer#184
- 4: Revise exit codes for PHPCS and PHPCBF commands squizlabs/PHP_CodeSniffer#2898
- 5: Getting incorrect Exit Code with PHPCBF PHPCSStandards/PHP_CodeSniffer#1372
Do not recommend phpcbf ... || true — it masks real failures.
|| true suppresses all non-zero exit codes, including genuine failures (codes 2+, 16, 64), which can hide broken runs in CI or scripts. The doc should show a pattern that accepts only expected codes (0 for clean/fixed, 1 for remaining auto-fixable issues) and fails on any actual error:
vendor/bin/phpcbf inc/path/to/file.php
status=$?
if [ "$status" -gt 1 ]; then
echo "phpcbf failed with exit code $status"
exit "$status"
fi
# status 0 = nothing to fix, status 1 = fixes applied🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@AGENTS.md` around lines 519 - 522, Replace the suggested "vendor/bin/phpcbf
... || true" pattern because it masks real failures; instead update the docs to
run "vendor/bin/phpcbf inc/path/to/file.php", capture its exit status, and
explicitly fail only when the status is greater than 1 (printing an error and
exiting with that status) while allowing 0 (clean) and 1 (fixes applied) to
proceed; specifically remove the "|| true" recommendation and show using the
phpcbf command, storing "$?" and checking if status > 1 to echo a failure
message and exit with that code.
SummaryAddresses 4 recurring tool failure patterns observed in contributor
Files Modified
VerificationChanges are documentation-only ( aidevops.sh v3.13.10 plugin for OpenCode v1.3.17 with claude-sonnet-4-6 spent 1m and 3,194 tokens on this as a headless worker. Merged via PR #989 to main. |
Summary
Addresses 4 recurring tool failure patterns observed in contributor
superdav42's sessions, by improving the agent guidance section ofAGENTS.md.bash:otherphpcbfnon-standard exit code 1 (successful auto-fix = exit 1, not error)edit:not_read_firstwebfetch:otherread:file_not_foundFiles Modified
EDIT: AGENTS.md— 30 lines added to Quick Session Checklist, Read Before Edit, No External URL Fetching, and Common bash:other Failures sectionsVerification
Changes are documentation-only (
AGENTS.md). No code modified, no tests required. The content was derived directly from the 4 error patterns reported in the issue body.Resolves #988
aidevops.sh v3.13.10 plugin for OpenCode v1.3.17 with claude-sonnet-4-6 spent 1m and 3,194 tokens on this as a headless worker.
Summary by CodeRabbit