* feat(abilities): scaffold Abilities_Registrar coordinator + Domain base
Phase I — adds the new ppcp-abilities module with the Abilities_Registrar
coordinator, the AbstractPpcpAbility Domain base, the
woocommerce_paypal_payments_abilities_enabled feature-flag gate
(default false), the WC 10.9 AbilitiesLoader presence check
(silent no-op on older WC versions), the can_manage_woocommerce()
capability helper that mirrors the wc/v3/wc_paypal/* REST controllers'
shared check_permission() resolution, and seven unit tests covering
the testable branches in the Brain Monkey environment. Concrete
abilities land in subsequent commits.
CATEGORY_SLUG is hardcoded to `woocommerce` — Woo Core (10.9+)
registers that category, so this registrar does not. Plugin ownership
lives in the ability namespace (woocommerce-paypal-payments/<name>).
Wires AbilitiesModule into modules.php so the Syde Modularity
container picks it up; per-ability registration stays gated behind
the feature flag.
Refs RSM-108
* feat(abilities): add woocommerce-paypal-payments/get-connection-status ability
Phase II (reference ability). Adds the smallest-safest read that backs
onto the existing wc/v3/wc_paypal/common/merchant route. Establishes
the Domain-class shape the remaining six reads will copy:
- One PHP file per ability under modules/ppcp-abilities/src/Domain/.
- Extends AbstractPpcpAbility, implements AbilityDefinition, lists
itself in Abilities_Registrar::ABILITY_CLASSES.
- Shape 2 (delegate to REST via the abstract base helper) — the
backing controller emits no telemetry and fires no hooks, so the
Shape 3 service-extraction overhead is not justified for this read.
Strips clientId + clientSecret from the merchant payload before
returning to the agent — the underlying $merchant_info_map exposes
both fields for the admin UI's manual-credentials flow, and an agent
echoing them would leak the OAuth credential pair.
Tests cover: namespace correctness (uses the plugin slug, not
`woocommerce/`), wiring (callbacks point at the Domain class + shared
helper), all three annotations, both projection opt-ins (show_in_rest
+ mcp.public), and the projection method's secret-redaction +
features-passthrough + WP_Error-on-envelope-failure contracts. The
delegate→REST→envelope happy path is exercised by the Phase V
integration harness against a real WC 10.9 install.
Adds a stub of \Automattic\WooCommerce\Abilities\AbilityDefinition to
tests/stubs/ so Domain classes autoload in the Brain Monkey unit-test
environment.
Refs RSM-108
* feat(abilities): register remaining 6 Phase III reads
Phase III — adds the rest of the MVP read surface as one coherent
batch. Every ability ships behind the same
woocommerce_paypal_payments_abilities_enabled feature flag (default
false), so a per-ability micro-commit split would add review noise
without isolating revert-able units.
Shape 2 (REST delegate via the abstract base helper) — the backing
controllers are zero-arg config reads that emit no telemetry and
fire no hooks:
- get-payment-methods → PaymentRestEndpoint::get_details
- get-settings → SettingsRestEndpoint::get_details
- get-webhook-status → WebhookSettingsEndpoint::get_webhooks
(issues a synchronous PayPal API call)
Shape 3 (direct service call) — these abilities expose operational
state that does not have an existing REST surface, so the ability
calls the plugin's container service directly:
- get-last-webhook-event → WebhookEventStorage::get_data()
- get-order-tracking → OrderTrackingEndpoint::list_tracking_information(int $wc_order_id)
(issues two synchronous PayPal API calls)
- get-paypal-order → OrderEndpointCached::order($paypal_id_or_wc_order)
(uses the cached endpoint to amortize cost
when invoked multiple times in a session)
Notable per-ability discipline:
- get-order-tracking declares wc_order_id as a JSON-schema-required
integer (minimum 1) AND re-validates in the execute callback.
- get-paypal-order accepts EITHER paypal_order_id OR wc_order_id;
"exactly one of" is enforced in the execute callback rather than
in the schema (no JSON Schema oneOf at this layer).
- get-paypal-order's description carries an explicit "may return
payer PII" notice so callers can apply downstream handling
appropriate to their context.
- get-last-webhook-event projects WebhookEventStorage's storage
payload into { received: bool, id, received_time, received_iso }
— the ISO timestamp is appended for agent ergonomics.
Service-resolved abilities (Shape 3) catch LogicException from the
PPCP container (raised when init runs before the plugin bootstraps)
and return a structured WP_Error rather than letting it bubble.
All 7 Domain classes are now listed in
Abilities_Registrar::ABILITY_CLASSES. Phase I + Phase II + this
commit bring the MVP surface to 7 reads.
Refs RSM-108
* chore(agents): note ability-registration audit when changing controller code
Phase VI.1 — drops the drift guard from the abilities-api-implement
playbook into AGENTS.md. Single-sentence reminder so an agent (human
or otherwise) modifying a backing controller / service knows to
audit the corresponding ability registration for shape, annotation,
or schema updates before merging.
Refs RSM-108
* fix(abilities): address round-1 review — PII redaction, format guards, refactor
Round-1 review feedback addressed (PR #4374 review-id 4293551129).
- GetPaypalOrder now redacts payer PII + per-purchase-unit shipping
addresses by default; callers opt in via include_payer_pii: true
when the calling context legitimately needs payer identity. Mirrors
the data-minimization pattern GetConnectionStatus already uses.
- GetPaypalOrder validates paypal_order_id against ^[A-Z0-9]{1,64}$
before passing it to OrderEndpointCached::order() (which
interpolates without rawurlencode); blocks path-traversal-style
payloads from altering the PayPal API URL path.
- GetPaypalOrder + GetOrderTracking no longer forward raw
PayPalApiException::getMessage() (which can include
information_link URLs) to the agent — generic message returned,
full exception logged via error_log() server-side.
- Three Shape-3 abilities now share resolve_service() on
AbstractPpcpAbility instead of each carrying an identical ~22-line
resolve_*() method. The shared helper also adds error_log() on the
bare-Throwable + unexpected-type paths so on-call has a server-side
trace when the container misbehaves.
- GetLastWebhookEvent description now accurately states the no-event
response shape ({ received: false }) rather than the previous
incorrect "null" claim.
- Renamed Abilities_Registrar -> AbilitiesRegistrar to match every
other module's PascalCase registrar naming
(WebhookRegistrar, InboxNoteRegistrar, TaskRegistrar).
- AbilitiesModule::run() now carries an explicit comment about why
the injected ContainerInterface is unused (static coordinator;
Shape-3 services resolve lazily at execute()-time via
PPCP::container()).
- AbilitiesRegistrar::$initialized now documents the hook-timing
requirement (must be invoked at-or-after plugins_loaded so WC
autoloading is warm; earlier hooks would let the class_exists
gate trip false and the guard would never re-arm).
- AbilitiesRegistrar feature-flag filter now carries a naming
rationale (runtime per-ability switch vs the dot-form module-level
gates in modules.php; intentionally different convention).
New tests:
- GetPaypalOrder paypal_order_id format guard (path traversal +
lowercase rejection)
- GetPaypalOrder wc_order_id with wc_get_order returning false
- GetPaypalOrder::project_order() PII redaction (default-strip,
shipping-address strip, opt-in passthrough)
Total: 42 unit tests / 141 assertions (was 36 / 125), full suite
1036 tests still green. PHPCS + PHPStan clean.
Pushback (will reply on the thread): kept
AbilitiesRegistrar::reset_initialized_for_testing() as public; the
@internal docblock + clearly-named method is the local convention,
and adding a PHPUNIT_RUNNING gate adds complexity for a theoretical
concern (the registrar is itself @internal and ships with
default-false abilities anyway).
Refs RSM-108
* fix(abilities): unblock npm run lint after AbilitiesRegistrar refactor
Round-2 review found two HIGH lint failures that the round-1 commit
(736e66f51) inadvertently introduced or surfaced:
- PHPCS error on AbstractPpcpAbility::resolve_service() — the
docblock used class-string<T> while the PHP signature was string,
which the project's Slevomat sniff rejects. Switched to the
@phpstan-* tag family so PHPStan still gets generic narrowing
while PHPCS sees a plain string/string match.
- PHPStan reported 7 interface.notFound errors for
Automattic\WooCommerce\Abilities\AbilityDefinition — the
woocommerce-stubs package the project pulls in tracks WC 9.x and
has no stubs for the WC 10.9 Abilities API surface. Added
stubs/abilities.php declaring AbilityDefinition + AbilitiesLoader
in their respective namespaces and wired it into phpstan.neon's
scanFiles. Production behaviour is unaffected (file is only
loaded by static analysis).
npm run lint now passes clean. 42 unit tests still green.
Refs RSM-108
* fix(abilities): address round-2 review — close exception leak, harden PII strip, tighten tests
Round-2 feedback addressed (PR #4374 review-id 4293713638).
- unwrap_envelope() now redacts the upstream message by default
(writes the original to error_log, returns a generic
"see server log for details" string in the WP_Error). Closes
the symmetric exception leak the round-1 fix only patched on
the Shape-3 path. New $redact_message=true default applies to
every Shape-2 ability automatically.
- GetConnectionStatus::project_merchant_payload() applies the
same redaction inline (it has its own success-false branch
that doesn't go through unwrap_envelope).
- GetPaypalOrder docblock + description corrected: payment_source
is no longer falsely listed as a returned/redacted field.
Description now lists what to_array() actually emits.
- REDACTED_TOP_LEVEL_KEYS const promotes payment_source from
"not relevant today" to "stripped defensively" so any future
Woo Core change that exposes payment_source through
Order::to_array() can't silently leak payer email/name/address.
Pinned by test_project_order_does_not_leak_synthetic_payment_source.
- error_log() lines in the three Throwable-catch sites now
include get_class($e) for triage convenience
(PayPalApiException vs TypeError vs RuntimeException at a
glance, no message-text scanning needed).
- test_init_bails_when_feature_flag_is_disabled and
test_init_re_evaluates_gates now assert AbilitiesRegistrar's
static $initialized via reflection instead of leaning on
addToAssertionCount(1) — failures of Brain Monkey's deferred
verification can no longer pass silently with a phantom
assertion count.
- test_append_classes_round_trip switched from order-sensitive
assertSame to assertEqualsCanonicalizing + assertCount —
Phase II/III ability additions only need to update one place
instead of dancing the declaration order.
- test_project_merchant_payload_returns_wp_error_on_envelope_failure
now uses an information_link-bearing payload and asserts the
redacted message instead of the raw text — locks in the
redaction.
42 → 43 unit tests / 141 → 145 assertions; full suite 1037 tests
green; npm run lint passes clean.
Refs RSM-108
* fix(abilities): address round-3 review — PSR-3 logger seam, dedup envelope handling, strengthen tests
Round 3 of the in-PR review iteration on RSM-108 surfaced six findings;
five are addressed here. The sixth (hoisting the abilities feature flag
to modules.php for symmetry with other optional modules) is declined —
the inner gate in AbilitiesRegistrar::init() exists precisely because
the WC 10.9 AbilitiesLoader class_exists() check must run after
plugins_loaded, and splitting the two gates would lose that
co-location. The author's docblock at AbilitiesRegistrar.php:77-86
explains the timing constraint.
Changes:
- AbstractPpcpAbility now holds a static PSR-3 LoggerInterface seam
(set_logger / logger / reset_logger_for_testing). AbilitiesModule::run()
resolves `woocommerce.logger.woocommerce` from the container and wires
it in, with a NullLogger fallback so abilities can never fail to load
because of a logger-resolution problem. The four runtime call sites
that previously used PHP's error_log() — envelope_error_or_null (two
in the redact branch), GetOrderTracking::execute, and
GetPaypalOrder::execute — now flow through $logger->warning() /
$logger->error(). resolve_service()'s Throwable branch deliberately
keeps error_log() because that path can fire before the container is
healthy enough to resolve the logger (its docblock now records the
intentional divergence).
- The `success=false` envelope handling that was duplicated between
GetConnectionStatus::project_merchant_payload and unwrap_envelope is
consolidated. unwrap_envelope now delegates to a new
envelope_error_or_null() helper, and GetConnectionStatus calls that
helper directly (it can't use unwrap_envelope wholesale because
CommonRestEndpoint puts merchant/features at the envelope top level
alongside `data`, and unwrap_envelope would extract `data` and drop
them). project_merchant_payload is reduced to the success-branch
projection only; its return type is now `array`.
- envelope_error_or_null drops the optional `details` key from the
redacted WP_Error data when `$redact_message=true` (logged
server-side instead). Closes a forward-looking redaction gap raised
by the security review — current backing endpoints don't populate
`details`, but future ones could surface structured PayPal API error
bodies through it.
- AbilitiesRegistrarTest gains a single-line assertion that
AbilitiesRegistrar::CATEGORY_SLUG and
AbstractPpcpAbility::CATEGORY_SLUG stay in sync, so the deliberate
mirror fails loudly at CI if either constant drifts rather than
silently in production.
- GetOrderTrackingTest::test_serialize_shipment_delegates_to_entity_to_array
was tautological (asserted a mock returned what the mock was
programmed to return). Replaced with two tests that pin
ShipmentInterface accessor -> wire-key edges and assert the
serializer passes through only the keys to_array() emits, so a
future serialization change actually fails the test.
- New AbstractPpcpAbilityTest covers envelope_error_or_null directly
(via a tiny test-only subclass seam), including the new
message-and-details redaction guarantee and the opt-out passthrough.
The corresponding scenario was removed from GetConnectionStatusTest
because project_merchant_payload no longer handles the failure path.
Verification:
- `vendor/bin/phpunit --filter Abilities` -> OK (49 tests, 161 assertions).
- `vendor/bin/phpunit` -> OK (1043 tests, 4853 assertions, 3 skipped — pre-existing).
- `npm run lint` -> PHPCS clean (warnings on resolve_service's two
intentional error_log calls + pre-existing wc-gateway alignment
warnings unrelated to this PR); PHPStan [OK] No errors.
Refs RSM-108
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(abilities): address round-4 review — sub-namespace test seam, codebase NullLogger, doc/test polish
Round 2 of the in-PR review iteration surfaced five small refinements
(2 medium, 3 low). All addressed here.
- GetConnectionStatus::project_merchant_payload docblock updated to
reference envelope_error_or_null() (the helper execute() now calls
directly) instead of unwrap_envelope(), and to spell out why
unwrap_envelope() can't be used wholesale here (CommonRestEndpoint
puts merchant/features alongside `data` at the envelope top level).
- AbilitiesModule and AbstractPpcpAbility now import
WooCommerce\WooCommerce\Logging\Logger\NullLogger (the plugin's
bundled implementation used by ppcp-settings, ppcp-store-sync,
ppcp-wc-gateway, and woocommerce-logging itself) instead of the
upstream Psr\Log\NullLogger. Behaviour is identical — codebase
consistency only.
- Dropped the tautological `instanceof LoggerInterface` guard in
AbilitiesModule::run(): both branches of the surrounding try/catch
already produce a LoggerInterface (the service factory's return type
guarantees it on success; NullLogger implements LoggerInterface in
the catch path), so the guard could never be false. Removing it also
drops the now-unused LoggerInterface import.
- Moved the test seam class from `_AbilityTestSeam` (declared in the
production namespace `WooCommerce\PayPalCommerce\Abilities\Domain`)
to `AbilityTestSeam` under a new test-only sub-namespace
`WooCommerce\PayPalCommerce\Abilities\_Seams`, in its own file at
tests/PHPUnit/Abilities/_Seams/AbilityTestSeam.php. Defense-in-depth:
if a future autoload-map change ever wildcards the test directory
into the production autoload, the seam can no longer land as a live
subclass of AbstractPpcpAbility in the production Domain namespace.
- AbstractPpcpAbilityTest gains a companion assertion that
envelope_error_or_null also returns a non-empty WP_Error message on
the default redact-on branch when the envelope omits `message`. The
pre-existing fallback-message test only exercised the redact-off
branch, so a regression that left the redact path with an empty
message would have slipped through.
Verification:
- `vendor/bin/phpunit --filter Abilities` -> OK (50 tests, 164 assertions).
- `vendor/bin/phpunit` -> OK (1044 tests, 4856 assertions, 3 skipped — pre-existing).
- `npm run lint` -> PHPCS clean (only the two intentional error_log
warnings on resolve_service plus unrelated pre-existing alignment
warnings in ppcp-wc-gateway); PHPStan [OK] No errors.
Refs RSM-108
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(abilities): drop get-settings and get-webhook-status abilities
Removes two of the seven read-only abilities registered in this PR:
- `woocommerce-paypal-payments/get-settings`
- `woocommerce-paypal-payments/get-webhook-status`
The remaining five reads still ship: `get-connection-status`,
`get-payment-methods`, `get-last-webhook-event`, `get-order-tracking`,
`get-paypal-order`.
Both removed abilities were Shape-2 (REST delegates). Their backing
controllers (`SettingsRestEndpoint`, `WebhookSettingsEndpoint`) are
unchanged and still accessible to merchants through the existing
admin REST routes — only the Abilities API surface is dropped.
Updates AbilitiesRegistrar::ABILITY_CLASSES, the
test_append_classes_round_trip_returns_full_ability_class_list
expected list, and removes the two Domain files plus their tests.
Verification:
- `vendor/bin/phpunit --filter Abilities` -> OK (46 tests, 142 assertions).
- `npm run lint` -> PHPCS clean, PHPStan [OK] No errors.
Refs RSM-108
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(abilities): drop get-last-webhook-event ability
Removes `woocommerce-paypal-payments/get-last-webhook-event` from the
initial Phase 1 read surface. Remaining four reads still ship:
`get-connection-status`, `get-payment-methods`, `get-order-tracking`,
`get-paypal-order`.
`WebhookEventStorage::get_data()` (the Shape-3 backing the ability
called directly) is unchanged — only the Abilities API surface is
dropped.
Updates AbilitiesRegistrar::ABILITY_CLASSES and the
test_append_classes_round_trip_returns_full_ability_class_list
expected list, and removes the Domain file plus its test.
Verification:
- `vendor/bin/phpunit --filter Abilities` -> OK (42 tests, 124 assertions).
- `vendor/bin/phpunit` -> OK (1036 tests, 4816 assertions, 3 skipped — pre-existing).
- `npm run lint` -> PHPCS clean, PHPStan [OK] No errors.
Refs RSM-108
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(abilities): return not_found for unknown order in get-order-tracking
<commit_analysis>
- Previous behavior: GetOrderTracking::execute() delegated straight to
OrderTrackingEndpoint::list_tracking_information(), which returns an empty
array when wc_get_order() is falsy.
- Problem: a non-existent WooCommerce order surfaced to the agent as
`shipments: []` — identical to an order that exists but has no trackers.
An agent could not distinguish a typo'd order ID from a genuinely
untracked one.
- Solution: pre-validate the order with wc_get_order() before delegating and
return a structured woocommerce_paypal_payments_not_found, mirroring the
pattern already used in GetPaypalOrder::extract_identifier().
- Consequences: the check short-circuits before the container is touched, so
it is unit-testable via Brain Monkey; the backing service's own
empty-array-for-missing-order branch becomes unreachable from this ability.
</commit_analysis>
The backing OrderTrackingEndpoint::list_tracking_information() returns array()
for an unknown order, indistinguishable from "exists but has no trackers".
Pre-validating the order in the ability lets the agent tell a missing order
from an untracked one. Mirrors GetPaypalOrder::extract_identifier(); covered
by a new test that stubs wc_get_order() to false.
Refs RSM-108
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(abilities): treat null tracker response as empty shipments
<commit_analysis>
- Previous behavior: GetOrderTracking::execute() mapped a null return from
OrderTrackingEndpoint::list_tracking_information() to a
woocommerce_paypal_payments_tracking_lookup_failed WP_Error.
- Problem: the backing service returns null on ANY non-200 from PayPal's
/v1/shipping/trackers endpoint — most commonly the 404 "no trackers
registered yet" response for an order that simply hasn't shipped. The very
common no-trackers path surfaced to the agent as a generic lookup failure.
- Solution: coerce null to an empty shipment list, matching how the existing
order-tracking meta box renders the same null (MetaBoxRenderer does
`... ?? array()`). The ability now follows the UI's interpretation.
- Consequences: genuine transport failures still throw above (is_wp_error in
the endpoint raises RuntimeException) and surface as tracking_lookup_failed,
so real errors are not masked. Fix stays at the ability layer; the shared
backing service (also used by the meta box) is untouched.
</commit_analysis>
list_tracking_information() returns null on any non-200 from PayPal's
trackers endpoint, predominantly the 404 no-trackers case. Treating that as
an error meant the common untracked-order path looked like a failure to
agents. Coerce it to an empty shipment list, matching MetaBoxRenderer's
existing handling; genuine transport failures still throw and surface as
tracking_lookup_failed.
Refs RSM-108
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(abilities): scope not-initialized catch to container resolution
<commit_analysis>
- Previous behavior: resolve_service() wrapped both PPCP::container() and
->get($service_id) in a single try whose LogicException catch returned
woocommerce_paypal_payments_not_initialized.
- Problem: PPCP::container() is the only intended source of the
"not initialized" LogicException, but ->get() invokes the service factory,
which can throw its own LogicException (a validation or contract bug). That
factory error was rewritten as "PayPal Payments is not initialized",
masking the real bug at triage.
- Solution: split the try so the LogicException catch wraps only
PPCP::container(); the ->get() factory call sits in its own try under a
Throwable catch that logs and returns service_unavailable.
- Consequences: a factory LogicException now surfaces honestly as
service_unavailable (logged via error_log), while the genuine
not-initialized case is unchanged. Docblock updated to describe the split.
</commit_analysis>
The single try meant a service-factory LogicException from ->get() was
mislabeled as "PayPal Payments is not initialized", hiding real factory bugs.
Scope the LogicException catch to PPCP::container() (its only intended source)
and run ->get() under a separate Throwable catch so factory failures are
logged and surfaced as service_unavailable.
Refs RSM-108
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(abilities): trim verbose AI-generated PHPDoc and comments
The ppcp-abilities module shipped with heavy AI-generated docblocks and
inline comments — 54-70% of the heaviest files were comment lines, with
the logger-divergence rationale repeated across three sites and class
docblocks re-enumerating fields the code already lists. @wjrosa's round-5
review (PR #4374, approved) flagged this as a readability nit.
Strip the rationale prose to terse one-liners while keeping the
load-bearing "why": the PII/credential redaction notes, the
path-traversal guard on PAYPAL_ORDER_ID_PATTERN, the resolve_service()
logger-divergence (stated once, not three times), and the init()
hook-timing constraint. All runtime strings (labels, descriptions, error
messages), translators comments, @phan-file-suppress lines, and the
type-bearing PHPDoc tags PHPStan relies on (@phpstan-template, @param,
@return, @var) are preserved verbatim.
Comment-only change: 416 deletions / 145 insertions, no logic touched.
PHPCS + PHPStan clean; the 43-test abilities suite passes unchanged.
Refs RSM-108
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2008af9dfe
