Opened 6 weeks ago
Closed 4 weeks ago
#65782 closed defect (bug) (fixed)
AI Client: the wp_ai_client_prevent_prompt filter gets a shallow clone and can change the prompt
| Reported by: | bejignesh | Owned by: | gziolo |
|---|---|---|---|
| Priority: | normal | Milestone: | 7.2 |
| Component: | AI | Version: | 7.0 |
| Severity: | normal | Keywords: | has-patch has-unit-tests |
| Cc: | Focuses: |
Description
WP_AI_Client_Prompt_Builder::__call() hands a clone of itself to the wp_ai_client_prevent_prompt filter, and the hook doc calls it read only:
/** * @param bool $prevent Whether to prevent the prompt. Default false. * @param WP_AI_Client_Prompt_Builder $builder A clone of the prompt builder instance (read-only). */ $prevent = (bool) apply_filters( 'wp_ai_client_prevent_prompt', false, clone $this );
The class does not define __clone(), so that is a shallow copy and the clone still points at the same wrapped PromptBuilder. The wrapped builder mutates itself rather than returning new instances:
public function withText(string $text): self { $part = new MessagePart($text); $this->appendPartToMessages($part); return $this; }
So whatever a filter callback does to the clone lands on the original, and the prompt that is then sent to the provider is not the prompt the caller built.
Steps to reproduce
add_filter( 'wp_ai_client_prevent_prompt', static function ( $prevent, $builder ) { $builder->with_text( 'Added by the filter' ); return $prevent; }, 10, 2 ); $builder = wp_ai_client_prompt( 'Original prompt' ); $builder->is_supported(); // The prompt is now 'Original promptAdded by the filter'.
This runs on every call that reaches the filter, so support checks and generating methods both.
Expected
A filter receiving the clone cannot change the prompt that gets generated, which is what the hook doc promises.
Patch
Add __clone() to WP_AI_Client_Prompt_Builder so the wrapped builder is cloned as well. The bundled client already implements PromptBuilder::__clone() to deep clone its messages, model config and request options. It was written for exactly this, it just never ran because only the WordPress wrapper was being cloned.
Any stored WP_Error is copied too. Nothing today clones the wrapper while it holds an error, since the filter is only reached when the error is still null, but __clone() is a magic method any caller can trigger and leaving half the state shared would be the same bug in a different place.
The existing test test_prevent_prompt_filter_receives_cloned_builder_instance() only checks that the outer object differs, which is why this went unnoticed. It still passes.
Change History (4)
This ticket was mentioned in PR #12801 on WordPress/wordpress-develop by @bejignesh.
6 weeks ago
#1
- Keywords has-patch has-unit-tests added
@bejignesh commented on PR #12801:
4 weeks ago
#3
Good catch, corrected to @since 7.2.0 in 5c4c3bcf. Confirmed trunk is on 7.2-alpha-63166-src.
ai-client group still passes at 259 tests and phpcs is clean. The sibling PR #12800 adds no @since, so nothing to change there.
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
WP_AI_Client_Prompt_Builder::__call()hands a clone of itself to thewp_ai_client_prevent_promptfilter, and the hook doc calls it read only:The class has no
__clone(), so that is a shallow copy and the clone still points at the same wrappedPromptBuilder. The wrapped builder mutates itself rather than returning new instances:So whatever a filter does to the clone lands on the original, and the prompt sent to the provider is not the one the caller built.
This runs on every call that reaches the filter, so support checks and generating methods both.
## Fix
Add
__clone()so the wrapped builder is cloned too. The bundled client already implementsPromptBuilder::__clone()to deep clone messages, model config and request options. It was written for this, it just never ran because only the wrapper was being cloned.A stored
WP_Erroris copied as well. Nothing today clones the wrapper while it holds an error, since the filter is only reached when the error is still null, but__clone()is a magic method any caller can trigger and leaving half the state shared would be the same bug in a different place.## Testing
Two tests, both failing on trunk:
Both pass with the patch. The existing
test_prevent_prompt_filter_receives_cloned_builder_instance()only asserted the outer objects differ, which is why this went unnoticed, and it still passes. The fullai-clientgroup is green, 259 tests.phpcspasses on both changed files.## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: Reading through the AI client for places where the code disagrees with its own documented behaviour, which is how this turned up, and drafting this description. I confirmed the mutation by hand, checked that the bundled client already has a deep
__clone(), wrote and ran the tests, checked they fail without the patch, ranphpcs, and I take responsibility for the change.