[selul]

Requested by Marius · project thread

All Submissions:

• Have you followed the Optimole Contributing guideline?
• Does your code follow the WordPress' coding standards?
• Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

Part of #1108, covering the 2026-09-22 report of a transfer that stayed "in progress" for 20+ hours.

When a background transfer step throws (an API error, or a lookup that keeps failing), start_processing_images() reschedules the same step 10 seconds later with no limit. If the cause doesn't go away, the chain retries that step forever and the dashboard keeps polling an active transfer.

Each step's failed attempts are now counted in the non-autoloaded optml_transfer_step_failures option (an option, so an object cache eviction can't reset the count). The record carries the transfer lock token, so failures from an earlier or stopped transfer never count toward a new one, and it is deleted when a transfer starts and when it finishes.

• After MAX_STEP_RETRIES (5) failures in a row, the chain logs it and moves on to the next step, so the transfer can reach its end. A successful step clears the count.
• The record also counts the skipped steps. A transfer that ends with skipped steps doesn't show the "Transfer Complete" notice; it logs how many steps were skipped and that the transfer should be started again. Images left behind stay in the rollback/offload queries and can be retried.
• The transfer lock is still kept across retries, as before.

Not covered: the other condition in the issue, an action that is scheduled but never executes (WP-Cron or Action Scheduler not running on the site). The plugin can only detect that, not run the work. The current transfer lock already lets maybe_reschedule() restart the chain once the lock expires.

How to test the changes in this Pull Request:

1. Start a rollback and make every step throw, for example by blocking generateurls-prod.i.optimole.com.
2. Watch the transfer log: each step is retried 5 times, then "Step N failed 5 times, moving on to the next one." appears and the transfer finishes instead of spinning.
3. When it ends, the "Transfer Complete" notice is not shown and the log says how many steps were skipped.

Other information:

• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes, as applicable?
• Have you successfully ran tests with your changes locally?

Check	Result
PHPUnit full suite (WP trunk, PHP 8.3)	PASS, 433 tests
New tests: failing step moves on after 5 tries, successful step clears the count, failures of another transfer ignored, no success notice after skips, success notice without skips	PASS
Mutation: remove the retry limit / remove the reset / drop the token check / always set the success notice	each makes its test fail
phpstan on inc/media_offload.php	No errors
PHP 7.4 lint, phpcs	PASS

Not proven: a live stuck transfer. Codex review was skipped because the Codex CLI isn't available in this environment.

---

Generated by Claude Code

[pirate-bot]

🟡 Changes requested

A failing step is now skipped, but the transfer then reports success, and the failure count leaks into later transfers.

Questions for the author

• Can schedule_action() fail to enqueue the next step after the fifth failure deleted the counter? If so, what restarts the chain?

Validation details

• Files reviewed: 2/2 changed files. The row "Caps and limits: Dropped items" was not settled.
• The new tests pass at HEAD cd /repo && set -o pipefail; timeout 600 vendor/bin/phpunit --do-not-cache-result --configuration=phpunit.xml --filter '… printed OK (2 tests, 4 assertions) (run R2).
• A base tree with only the new constants fails both tests [R4].
• An every-attempt-throws run ended with show_offload_finish_notice set to rollback [R6].

Untested areas

• Caps and limits: Dropped items. Open inc/media_offload.php:1585. Check which images a later step selects after a skip. The plugin sets no query order.
• Real WP-Cron delivery and dashboard rendering were not run.

🤖 Automated review · run code-review-agent_6ad7d074b3a561.25362889.

---

🤖 Review agent — review posted ✅ on a2936822 · changes requested · 5 findings · 32 min

Run code-review-agent_6ad7d074b3a561.25362889 · trail

[selul]

On the review question: if schedule_action() fails after a skip, the chain stops but keeps its transfer lock until the lock expires. Then maybe_reschedule() sees a transfer still marked in progress with no lock, and calls move_images() again, which starts a fresh chain with a new token. The failure record no longer gets deleted on a skip, and a new token ignores the old record anyway.

---

Generated by Claude Code

[pirate-bot]

🟡 Changes requested

The earlier correctness and type findings are fixed, but one comment in the finish branch is now false.

Questions for the author

• Five failed attempts per step can mean five tries for each remaining step. What data or support report shows that this limit fits real transfer sizes?

Validation details

• Files reviewed: 2/2 changed files. The rows "Caps and limits: Dropped items" and "Caps and limits: Stable order" were not settled.
• The Test_Media class passes at HEAD: OK (61 tests, 224 assertions) [R4].
• Three mutants each fail one of the new tests [R5].
• A run where every step throws ends with no success notice [R6].
• The prior run code-review-agent_6ad7d074b3a561.25362889 showed the success notice at a293682. It was not rerun.

Untested areas

• Caps and limits: Dropped items. Caps and limits: Stable order. Open inc/media_offload.php:1586. The query sets no order, so check which images a step selects after a skip.
• The object cache behavior of the new option was not checked against the installed WordPress code.
• Real WP-Cron delivery and dashboard rendering were not run.

🤖 Automated review · run code-review-agent_6ad7dd9b91b1b9.00424458.

---

🤖 Review agent — review posted ✅ on 2aa00e47 · changes requested · 1 finding · 22 min

Run code-review-agent_6ad7dd9b91b1b9.00424458 · trail

[selul]

On the retry limit: there's no support data behind 5; it's a judgment call. The batch is retried 10 seconds apart, so 5 attempts gives a transient API or memory error about 50 seconds to clear before the chain moves on, and the worst case for a transfer where every step fails is 5 × the number of batches attempts instead of an endless loop. The constant MAX_STEP_RETRIES is the single place to tune it if real reports show it's too low or too high.

---

Generated by Claude Code

[pirate-bot]

🟢 Approved

The PR passes. Both open threads are fixed, and the one new commit only corrects a comment.

Validation details

• Files reviewed: 2/2 changed files. The row "Caps and limits: Realistic volume" was not settled.
• Since the prior review, commit 8db9a18 changes only the comment at inc/media_offload.php:2298.
• The WordPress 7.1.3 option code reads a non-autoloaded option back from the database on a cache miss. A cache eviction cannot reset the failure count.
• A skipped step leaves no marker on images. Later steps and new transfers select those images again.
• Run code-review-agent_6ad7dd9b91b1b9.00424458 at 2aa00e4 passed Test_Media with 61 tests. Its mutants each failed a new test. Not rerun.

Untested areas

• Caps and limits: Realistic volume. No data shows how long real API outages last. Five attempts cover about 50 seconds per step.
• Real WP-Cron delivery and dashboard rendering were not run.

🤖 Automated review · run code-review-agent_6ad7e4bfc50668.80138776.

---

🤖 Review agent — review posted ✅ on 8db9a187 · approved · 0 findings · 23 min

Run code-review-agent_6ad7e4bfc50668.80138776 · trail

[pirate-bot]

Plugin build for 8db9a18 is ready 🛎️!

• Download build
• Download dev