auto-recover: orphaned worker branch for #2321 - #2324

tests/SdAiAgent/Services/CustomerAgentRuntimeServiceTest.php (4)

425-431: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Two of the five forbidden keys aren't exercised.

apply_consumer_policy_narrowing() denies system_instruction, system_prompt, client_abilities, attachments, and tool_calls, but this table omits system_instruction and tool_calls — one array entry each in the existing loop.

Asserting the exact error code per case rather than just the forbidden_ prefix would also stop a mis-mapped rejection from passing silently.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/SdAiAgent/Services/CustomerAgentRuntimeServiceTest.php` around lines
425 - 431, Update the forbidden-context cases in CustomerAgentRuntimeServiceTest
to include entries for system_instruction and tool_calls, matching the keys
denied by apply_consumer_policy_narrowing(). Strengthen the loop assertions to
verify each case’s exact expected forbidden error code rather than only checking
the forbidden_ prefix.

---

59-63: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Clean up tombstones for every tracked profile key, not just self::INTEGRATION.

Line 59 hardcodes the service's private tombstone naming (REMOVED_PROFILE_OPTION_PREFIX plus hash('sha256', 'integration|' . $key)) for one constant. It covers today's tests, but any future test that calls remove_profile() on a different key leaks a tombstone, and because resolve_integration() checks is_removed_profile() first, the leak surfaces as a confusing profile_removed (410) in an unrelated test.

Since $managed_profile_keys is already tracked, folding the tombstone delete into that loop makes cleanup self-maintaining.

♻️ Derive cleanup from the tracked keys

-		delete_option( 'sd_ai_agent_customer_agent_removed_profile_' . hash( 'sha256', 'integration|' . self::INTEGRATION ) );
-		foreach ( $this->managed_profile_keys as $profile_key ) {
+		foreach ( array_unique( array_merge( $this->managed_profile_keys, array( self::INTEGRATION ) ) ) as $profile_key ) {
+			delete_option( 'sd_ai_agent_customer_agent_removed_profile_' . hash( 'sha256', 'integration|' . $profile_key ) );
 			Agent::delete_managed_customer_profile( $profile_key );
 		}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/SdAiAgent/Services/CustomerAgentRuntimeServiceTest.php` around lines 59
- 63, Update the teardown cleanup around managed_profile_keys to delete each
tracked profile’s tombstone within the existing loop, deriving the option name
with the service’s established tombstone prefix and hash format; remove the
hardcoded self::INTEGRATION deletion, then retain managed profile deletion and
reset the tracking array.

---

470-479: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Only the happy path of handoff validation is covered.

This pins the valid-intent round trip, but normalise_handoff()'s rejection rules are untested: an intent outside the four-value allowlist, a missing/empty reason, and a non-array handoff must all yield no handoff key in the DTO. A plain non-JSON reply (the documented backward-compatible path) is also unasserted.

Those are the branches that keep unvalidated model output out of the consumer contract, so they're worth pinning.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/SdAiAgent/Services/CustomerAgentRuntimeServiceTest.php` around lines
470 - 479, Extend the handoff validation tests around inspect_job and
normalise_handoff to cover an intent outside the four-value allowlist, missing
or empty reason, and non-array handoff; assert each DTO omits the handoff key.
Also add a plain non-JSON reply case and assert it remains supported without a
handoff entry, while preserving the existing valid-intent assertions.

---

206-220: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use keys that normalize to the same slug prefix.

managed-slug-a_b and managed-slug-a-b already produce distinct sanitize_title() prefixes because underscores are preserved in WordPress title sanitization, so the hash suffix is not the factor making this test pass. Use two keys that collapse to the same sanitize_title() output so the test actually guards collision resistance.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/SdAiAgent/Services/CustomerAgentRuntimeServiceTest.php` around lines
206 - 220, Update the test around ensure_profile and
Agent::get_by_managed_profile_key to use two managed profile keys whose
sanitize_title() results are identical, such as variants differing only by
characters WordPress removes or normalizes. Keep the existing assertions
verifying both profiles are created, retrieved, and receive different agent
slugs.

includes/Services/CustomerAgentRuntimeService.php (3)

1371-1378: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

drift has two incompatible shapes across the managed and legacy paths.

The managed branch returns drift with safety_envelope_version and operator_resources_missing (Lines 371-374); this legacy branch returns only legacy_registration. Since profile_status() is an integration-facing contract typed loosely as array<string,mixed>, a consumer written against the managed shape hits undefined indexes on a legacy profile.

Emitting the full key set from both branches (with legacy_registration as an additional flag) keeps the response shape stable.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@includes/Services/CustomerAgentRuntimeService.php` around lines 1371 - 1378,
The legacy response construction in CustomerAgentRuntimeService must emit the
same drift keys as the managed branch. Update the drift array in
profile_status() to include safety_envelope_version and
operator_resources_missing with the corresponding legacy-path values, while
retaining legacy_registration as an additional flag.

---

346-357: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

N+1 collection lookups run on every customer request, including job polling.

profile_status() is reached from resolve_integration() (Line 1049), so enqueue_turn, create_or_recover_conversation, inspect_job, cancel_job, close_conversation, and process_job each issue one KnowledgeDatabase::get_collection_by_slug() query per approved collection — and get_collection_by_slug() is an uncached direct $wpdb->get_row (includes/Knowledge/KnowledgeDatabase.php lines 175-193). Consumers poll inspect_job in a loop, so this multiplies.

Consider a single batched lookup for the availability probe, and/or restricting the availability probe to the write paths (ensure_profile, profile_status) rather than every read.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@includes/Services/CustomerAgentRuntimeService.php` around lines 346 - 357,
Replace the per-collection KnowledgeDatabase::get_collection_by_slug() calls in
profile_status() with a single batched availability lookup, or limit this
availability validation to the write paths ensure_profile() and profile_status()
so read and job-polling flows avoid repeated queries. Preserve the existing
customer_collections_not_approved and collections_unavailable reason behavior.

---

1255-1281: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

abilities is validated then discarded, and collections is uncapped.

Two gaps in the spec contract:

• Lines 1265-1281 validate $spec['abilities'] against SAFE_ABILITIES, but the normalized spec never carries it and provisioning always writes self::SAFE_ABILITIES (Line 207). A consumer passing 'abilities' => array() is told nothing and silently gets knowledge-search. Either honour the narrowing or reject the key outright, so the validation isn't misleading.
• Line 1256 accepts an unbounded collections list. Since profile_status() probes each approved slug individually, a large list turns every customer request into a proportionally large query burst. A modest cap here would bound that.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@includes/Services/CustomerAgentRuntimeService.php` around lines 1255 - 1281,
Update the customer profile normalization flow around $profile_version,
$collections, and $spec['abilities'] so validated abilities are either preserved
and used by provisioning instead of always writing self::SAFE_ABILITIES, or the
abilities key is rejected rather than ignored. Add a finite maximum for the
sanitized collections list and return the existing invalid-spec error when it
exceeds that limit, keeping valid lists unchanged.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes