Paginate attachment query, harden SEO meta cache flush, fix grant test precision, expand grant test coverage, tighten test permissions

[Copilot]

Seven targeted fixes across production code and tests: memory safety for large media libraries, a cache-flush correctness bug in the SEO meta tool, and several gaps/inaccuracies in grant test coverage.

Production

• class-tool-cleanup-assets.php — get_all_attachments() was fetching all IDs in a single unbounded query (posts_per_page => -1). Replaced with a paginated loop (500/page, no_found_rows => true):

do {
    $query = new WP_Query([
        'posts_per_page' => 500,
        'paged'          => $page,
        'no_found_rows'  => true,
        ...
    ]);
    $batch_ids = array_map('intval', $query->posts);
    $all_ids   = array_merge($all_ids, $batch_ids);
    $page++;
} while (count($batch_ids) === 500);

• class-tool-set-seo-meta.php — clean_post_cache() was unconditionally called after update_post_meta() regardless of whether the write succeeded. Now only fields where update_post_meta() returns non-false are tracked as updated, and the cache flush is skipped when nothing was actually written.

Tests

• GrantTest.php — test_tampered_payload_rejected was constructing a fresh payload (new nonce, new timestamps) rather than decoding and mutating the original, meaning the test could pass due to nonce/params mismatch instead of signature failure. Fixed to decode the original claims and mutate only tool. Added b64urldec() helper. Added test_empty_params_grant_rejected_with_non_empty_params for the empty→{} normalization boundary case.

• RestWriteGrantTest.php — Added test_update_translations_binds_empty_params (parallel to the existing core update test) and test_expired_grant_is_denied.

• SetSeoMetaTest.php + tests/bootstrap.php — Added test_no_cache_flush_when_write_fails. Extended the update_post_meta mock to honour $GLOBALS['_sa_state']['update_post_meta_return'][$post_id][$key] overrides; added _sa_state reset to sa_reset_state().

• SnapshotsTest.php — mkdir permissions narrowed from 0777 to 0755.

Original prompt

Please apply the following diffs and create a pull request.
Once the PR is ready, give it a title based on the messages of the fixes being applied.

[{"message":"Fetching all attachment IDs with `posts_per_page => -1` on a large media library will load every attachment ID into memory in a single query. Consider paginating in batches (e.g. 500 at a time) to avoid memory exhaustion on sites with thousands of media items.","fixFiles":[{"filePath":"digitizer-site-worker/includes/tools/class-tool-cleanup-assets.php","diff":"diff --git a/digitizer-site-worker/includes/tools/class-tool-cleanup-assets.php b/digitizer-site-worker/includes/tools/class-tool-cleanup-assets.php\n--- a/digitizer-site-worker/includes/tools/class-tool-cleanup-assets.php\n+++ b/digitizer-site-worker/includes/tools/class-tool-cleanup-assets.php\n@@ -157,13 +157,30 @@\n \t * @return int[]\n \t */\n \tprivate function get_all_attachments() {\n-\t\t$query = new WP_Query( array(\n-\t\t\t'post_type'      => 'attachment',\n-\t\t\t'post_status'    => 'inherit',\n-\t\t\t'posts_per_page' => -1,\n-\t\t\t'fields'         => 'ids',\n-\t\t) );\n-\t\treturn array_map( 'intval', $query->posts );\n+\t\t$all_ids    = array();\n+\t\t$batch_size = 500;\n+\t\t$page       = 1;\n+\n+\t\tdo {\n+\t\t\t$query = new WP_Query( array(\n+\t\t\t\t'post_type'      => 'attachment',\n+\t\t\t\t'post_status'    => 'inherit',\n+\t\t\t\t'posts_per_page' => $batch_size,\n+\t\t\t\t'paged'          => $page,\n+\t\t\t\t'fields'         => 'ids',\n+\t\t\t\t'no_found_rows'  => true,\n+\t\t\t) );\n+\n+\t\t\t$batch_ids = array_map( 'intval', $query->posts );\n+\t\t\tif ( empty( $batch_ids ) ) {\n+\t\t\t\tbreak;\n+\t\t\t}\n+\n+\t\t\t$all_ids = array_merge( $all_ids, $batch_ids );\n+\t\t\t$page++;\n+\t\t} while ( count( $batch_ids ) === $batch_size );\n+\n+\t\treturn $all_ids;\n \t}\n \n \t/**\n"}]},{"message":"The `test_valid_grant_with_empty_params` test only verifies that an empty-params grant passes. There is no corresponding test that verifies an empty-params grant is rejected when called with non-empty params, which would be an important boundary case for the canonical JSON empty-container normalization logic (`empty array → {}`).","fixFiles":[{"filePath":"tests/unit/GrantTest.php","diff":"diff --git a/tests/unit/GrantTest.php b/tests/unit/GrantTest.php\n--- a/tests/unit/GrantTest.php\n+++ b/tests/unit/GrantTest.php\n@@ -87,6 +87,11 @@\n \t\t$this->assertTrue( Aura_Worker_Grant::verify( $grant, 'execute_php', array() ) );\n \t}\n \n+\tpublic function test_empty_params_grant_rejected_with_non_empty_params(): void {\n+\t\t$grant = $this->mint( array( 'params' => array() ) );\n+\t\t$this->assertIsString( Aura_Worker_Grant::verify( $grant, 'execute_php', array( 'code' => '1+1' ) ) );\n+\t}\n+\n \t// --- rejections -----------------------------------------------------------\n \n \tpublic function test_missing_grant_rejected(): void {\n"}]},{"message":"`list( , $s )` only captures the second element of the explode result, discarding the first. However, the variable `$s` is then used as the original signature from the un-tampered grant, while a completely new payload is constructed and base64-encoded separately. The test re-uses the original signature with a freshly constructed (different nonce and timestamps) payload, meaning the tampered grant's nonce would differ from the one in the original signed payload. This could make the test pass for the wrong reason (nonce mismatch or params mismatch rather than signature failure). The payload reconstruction should reuse the original nonce, `iat`, and `exp` values from the decoded original payload to ensure the test is specifically validating the signature check.","fixFiles":[{"filePath":"tests/unit/GrantTest.php","diff":"diff --git a/tests/unit/GrantTest.php b/tests/unit/GrantTest.php\n--- a/tests/unit/GrantTest.php\n+++ b/tests/unit/GrantTest.php\n@@ -124,23 +124,14 @@\n \t}\n \n \tpublic function test_tampered_payload_rejected(): void {\n-\t\t// Change a real field (tool) in the signed payload → the original\n-\t\t// signature no longer matches, so verification must fail.\n-\t\t$grant         = $this->mint();\n-\t\tlist( , $s )   = explode( '.', $grant );\n-\t\t$payload       = wp_json_encode(\n-\t\t\tarray(\n-\t\t\t\t'v'             => 1,\n-\t\t\t\t'tool'          => 'db_query', // was execute_php\n-\t\t\t\t'params_sha256' => hash( 'sha256', Aura_Worker_Grant::canonical_json( array( 'code' => '1+1' ) ) ),\n-\t\t\t\t'site'          => $this->site_hash,\n-\t\t\t\t'nonce'         => bin2hex( random_bytes( 16 ) ),\n-\t\t\t\t'iat'           => time(),\n-\t\t\t\t'exp'           => time() + 300,\n-\t\t\t),\n-\t\t\tJSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE\n-\t\t);\n-\t\t$tampered = $this->b64url( $payload ) . '.' . $s;\n+\t\t// Change only one signed field (tool) while preserving all original claims\n+\t\t// so this test specifically validates signature mismatch due to tampering.\n+\t\t$grant       = $this->mint();\n+\t\tlist...

[BenKalsky]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3a5e03ee90

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Treat idempotent SEO updates as successful

When the requested SEO value is already present, WordPress update_post_meta() returns false for “unchanged”, not only for write failures. With this guard, an idempotent call such as setting the same title reports “Nothing to update” and skips clean_post_cache()/Yoast indexable invalidation, so a site whose postmeta is correct but SEO cache is stale cannot be repaired by rerunning the tool. Consider distinguishing unchanged existing meta from an actual write failure before deciding nothing was updated.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Build the expired-grant fixture with real claim names

This new test signs claims named act, args, jti, and sh, but Aura_Worker_Grant::verify() only recognizes tool, params_sha256, nonce, and site. As a result the denial happens at tool/site/params validation before the expiry branch is reached, so a regression that accepted expired otherwise-valid grants would not fail this test. Reuse the existing mint() format with only exp in the past to cover the intended path.

Useful? React with 👍 / 👎.

[BenKalsky]

@codex review

[BenKalsky]

@codex review

[BenKalsky]

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Already looking forward to the next diff.

Reviewed commit: 22419a97e2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".