GOOWOO-786: MAPI product sync

[alaca]

Changes proposed in this Pull Request:

Existing products should sync to the Merchant API on their own, with no manual step. The scheduled sync already runs, but sending a full catalog one product at a time is slow and prone to errors. This PR makes the automated sync fast and resilient enough to push a full catalog unattended: it retries transient failures, sends requests concurrently, groups them into single multipart calls, and skips products that have not changed since the last sync.

Closes https://linear.app/a8c/issue/GOOWOO-786/mapi-product-sync

Replace this with a good description of your changes & reasoning.

Screenshots:

Detailed test instructions:

Trigger a sync (complete MC setup, re-save products, or run the UpdateAllProducts scheduled action via Action Scheduler). Products should appear in Merchant Center.

Additional details:

Changelog entry

[codecov]

Codecov Report

❌ Patch coverage is 93.82022% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.4%. Comparing base (8d2ccd3) to head (6f1f1dc).
⚠️ Report is 264 commits behind head on feature/notifications-system.

Files with missing lines	Patch %	Lines
src/API/Google/Mapi/MerchantApiClient.php	81.0%	11 Missing ⚠️

Additional details and impacted files

@@                       Coverage Diff                        @@
##             feature/notifications-system   #3587     +/-   ##
================================================================
+ Coverage                            68.1%   69.4%   +1.3%     
- Complexity                           5571    6071    +500     
================================================================
  Files                                 544     564     +20     
  Lines                               22216   23782   +1566     
================================================================
+ Hits                                15124   16494   +1370     
- Misses                               7092    7288    +196     

Flag	Coverage Δ
php-unit-tests	69.4% <93.8%> (+1.3%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
.../Google/Mapi/Services/MapiProductInputsService.php	100.0% <100.0%> (ø)
...nal/DependencyManagement/GoogleServiceProvider.php	91.9% <100.0%> (+2.1%)	⬆️
src/Product/BatchProductHelper.php	92.6% <100.0%> (+1.8%)	⬆️
src/Product/ProductHelper.php	90.4% <100.0%> (-0.4%)	⬇️
src/Product/ProductMetaHandler.php	84.9% <ø> (ø)
src/Product/ProductRepository.php	86.3% <100.0%> (ø)
src/Product/ProductSyncer.php	91.9% <100.0%> (-0.9%)	⬇️
src/API/Google/Mapi/MerchantApiClient.php	83.5% <81.0%> (ø)

... and 30 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[joemcgill]

@alaca This looks good to me. Claude came up with a few suggestions that I've added here, but nothing blocking. It would be good to review and handle the retry suggestions either before merging or as a quick follow-up. I'm going to pre-approve pending your review.

🤖 AI Code Review

Issue: MAPI product sync
Branch: feature/GOOWOO-786-auto-product-sync → feature/notifications-system

Summary

The implementation fully covers the issue requirement (automatic product sync without user interaction) and satisfactorily delivers on all four behaviors stated in the PR description: transient-failure retries, concurrency, multipart batching, and skip-if-unchanged delta logic. Test coverage for new behavior is solid and the overall structure is clean. There is one noteworthy dead-code / middleware-ordering bug in the retry layer that the tests do not catch, one RFC compliance gap in the multipart format, and a handful of smaller items worth tidying before merge.

🟡 Suggestions

• Retry middleware ordering makes if (429 === $code) unreachable, and Retry-After is never honoured (src/Internal/DependencyManagement/GoogleServiceProvider.php): In Guzzle's HandlerStack, the last push() call creates the outermost wrapper. Because error_handler is pushed first and retry_on_transient_error is pushed last, retry wraps error_handler. When the HTTP layer returns a 4xx or 5xx, error_handler converts it to a rejected promise (a RequestException) before retry's decider ever sees the raw ResponseInterface. The decider therefore always enters the ! $response instanceof ResponseInterface branch; the $code = $response->getStatusCode() block — including if ( 429 === $code ) { return true; } — is dead code, and the Retry-After path in retry_delay is never exercised. The comment on line 330 ("Innermost: retry transient failures before the error handler turns them into exceptions") has the semantics backwards. For the PR's primary scope (product/batch POST paths) the practical impact is zero — those paths get retried on both 429 and 5xx via the RequestException branch anyway — but non-product POST paths that receive a 429 are not retried despite the comment claiming "any method is safe," and the Retry-After cap logic is dead. Fix: either push retry_on_transient_error before error_handler so it's truly innermost, or extract the status from $reason->getResponse() in the rejected branch and check it there, then delete the unreachable $response instanceof ResponseInterface block.

• Inner request-line missing HTTP version string (MerchantApiClient.php, build_batch_request): emits POST /path\r\n with no protocol token. RFC 7230 / Google's batch spec expect POST /path HTTP/1.1. A strict parser could reject the inner request, surfacing as an opaque batch-level error rather than a per-product one. Fix: "%s /%s HTTP/1.1\r\n".

• is_retryable_request batch-path check is over-broad (GoogleServiceProvider.php): strpos( $path, 'batch' ) matches any path containing "batch", not just the dedicated endpoint. Prefer an exact/suffix match on /batch.

• retry_delay jitter applied after the cap, so the return value can exceed the stated 30s cap (GoogleServiceProvider.php): min( $backoff, $max_delay ) + wp_rand( 0, 1000 ) can hit 31,000ms even though the docblock and constant claim a 30s ceiling (the test even asserts <= 31000). Apply jitter before capping, or update the docblock/constant to reflect the true max.

🔵 Nitpicks

• generate_mapi_update_entries's @return/@param docblocks (BatchProductHelper.php, ProductSyncer.php) don't mention the new hash entry key.
• delete_many's $build_request callable is declared function ( int $index ) but always called with two args; declare the unused ProductInput $input param to match the call site and insert_many's signature.
• BATCH_BOUNDARY is only ever used as a prefix (real boundary appends a random suffix) — BATCH_BOUNDARY_PREFIX would be clearer.
• parse_batch_response silently returns [] on an unparseable boundary, which surfaces as ordinary per-product 500s in run_in_batches with no do_action( 'woocommerce_gla_error', ... ) — a malformed batch response would be very hard to distinguish from normal per-product failures in logs.

---

This review was generated by Claude. A human review is still required before merging.

[alaca]

Thanks @joemcgill. Applied 2, 3, 4 and the nitpicks.

1 is a false positive: Guzzle's HandlerStack::resolve() reverses the stack, so retry (pushed last) is innermost and sees the raw response before error_handler turns it into an exception, so the 429 branch and Retry-After both work. Added a test that proves it (and shows the suggested flip breaks retrying), so I left the order as-is.

[joemcgill]

LGTM — nothing blocking left. Already approved so I'm going to go ahead and merge.

🤖 AI Review Notes

Re-reviewed the 2026-07-15 fixup commits against the 2026-07-14 review notes: all 3 code-level "Suggestions" (HTTP/1.1 request line, tightened batch-path matching, jitter-before-cap) are fixed, plus 3 of 4 nitpicks (hash docblocks, boundary naming, error logging on unparseable batch). Only remaining nitpick: delete_many's $build_request closure signature (function ( int $index )) doesn't match its 2-arg call site — cosmetic, non-blocking.

Correction to my earlier AI review: the "retry middleware ordering" item was a false positive — I had Guzzle's HandlerStack push-order semantics backwards. The last-pushed middleware (retry_on_transient_error) is actually the innermost handler, so it already saw the raw response before error_handler (pushed first, outermost) could convert it to an exception. The new test_retry_runs_before_the_error_handler_on_the_full_stack test confirms this, and CI is green across all PHP/WP/WC matrix combinations. No code change was needed there.

---

This review was generated by Claude.