Skip to content

GH#988: docs: improve AGENTS.md guidance for 4 recurring contributor error patterns #989

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
superdav42 merged 1 commit into from
Apr 29, 2026
+30 −3
Merged

GH#988: docs: improve AGENTS.md guidance for 4 recurring contributor error patterns #989

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 30 additions & 3 deletions AGENTS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,12 @@ read them will produce `read:file_not_found`:
| `inc/class-membership-manager.php`, `inc/class-customer-manager.php` (etc.) | Do not exist at root of `inc/`; manager classes are in `inc/managers/` (e.g. `inc/managers/class-membership-manager.php`) |
| `inc/class-gateway.php`, `inc/class-payment-gateway.php` | Do not exist; gateway classes are in `inc/gateways/` |
| `inc/functions/class-*.php` | Functions in `inc/functions/` are plain procedural PHP (not OOP class files); they are named `inc/functions/customer.php`, `inc/functions/checkout.php`, etc. |
| `class-wp-ultimo.php` (at repo root) | Does not exist at root; the main plugin class is at `inc/class-wp-ultimo.php` |
| `inc/managers/class-manager.php` | Does not exist; manager classes are concrete (e.g. `inc/managers/class-membership-manager.php`, not a base `class-manager.php`) |
| `inc/helpers/class-arr.php`, `inc/helpers/class-hash.php` | May exist — verify with `git ls-files 'inc/helpers/class-*.php'`; file names in helpers vary |
| `tests/unit/*.php` | The `tests/unit/` directory exists but contains limited files — verify with `git ls-files 'tests/unit/'` before reading |
| `inc/checkout/class-signup.php` | Does not exist; signup flow uses `inc/checkout/class-checkout.php` |
| `assets/js/*.js` (unminified) | Verify with `git ls-files 'assets/js/'`; some files have both `.js` and `.min.js`, others only `.min.js` |

Always verify a file is tracked before reading it with `git ls-files '<path>'`. An empty result means the file does not exist in the repo.

Expand All @@ -286,9 +292,14 @@ messages or tool output. Constructed URLs (e.g. building a GitHub raw URL from a
guessing a docs URL from a package name) account for the majority of `webfetch:other` failures
in this codebase.

Do **not** use webfetch for WordPress documentation, PHP manual pages, Stripe/PayPal API docs,
BerlinDB documentation, WooCommerce docs, npm package docs, WP Plugin Handbook, or any other
developer documentation URLs — these requests consistently fail.
Do **not** use webfetch for any of these — they consistently fail:

- WordPress documentation (`developer.wordpress.org`, `make.wordpress.org`, `codex.wordpress.org`)
- PHP manual pages (`php.net/manual/`)
- Stripe/PayPal API docs (`stripe.com/docs`, `developer.paypal.com`)
- BerlinDB documentation
- WooCommerce docs, npm package docs, WP Plugin Handbook
- Any other developer documentation URLs

Do **not** use webfetch for GitHub URLs (issues, PRs, raw content, commits). Use the `gh` CLI
instead:
Expand All @@ -313,6 +324,8 @@ Use the codebase itself for API/hook research:

The Edit tool **requires** a prior Read call on the same file in the **current conversation session**. A prior read from a previous conversation does not count — if the session restarts or the context is cleared, re-read every file before editing it. If you attempt to edit without reading first, the tool will fail. Always read the complete target file before editing — even for small changes.

> **Session restart resets ALL read state.** If your conversation context was cleared, compacted, or this is a new session, you have NOT read any file — even if you remember doing so in a prior session. Re-read every file you intend to edit before making any edits.

**Required per-file workflow — follow this every time:**

1. Call `Read` on the target file.
Expand Down Expand Up @@ -497,6 +510,20 @@ vendor/bin/phpunit --filter 'WP_Ultimo\\Checkout\\Cart_Test' # fully-qual

Only run the full suite (`vendor/bin/phpunit`) for final verification, not during iterative development.

**`vendor/bin/phpcbf` exits 1 on successful auto-fixes** — phpcbf uses a non-standard exit
code convention: exits `0` only when no violations were found (nothing to fix), exits `1`
when it successfully auto-fixed violations, and exits `2+` on hard errors. This means a
clean auto-fix run returns exit code `1`, which `&&` chains and `set -e` scripts treat as
failure:

```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
```
Comment on lines +519 to +522
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 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:

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.


After phpcbf runs and modifies a file, you MUST re-read the file before calling Edit on it
(see [Read Before Edit](#read-before-edit-mandatory)).

**`npm run env:*` and `npm run cy:*` require Docker and `@wordpress/env`** — the `env:start`,
`env:stop`, and Cypress E2E scripts (`cy:open:*`, `cy:run:*`) use the `@wordpress/env`
Docker-based environment. These are **not** part of the standard development workflow for unit
Expand Down
Loading