Fix: Ensure Media Attachments Are Properly Linked to Posts When Reusing WP_CLI::runcommand() - #6066

Codecov Report

Attention: Patch coverage is 0% with 19 lines in your changes missing coverage. Please review.

📢 Thoughts on this report? Let us know!

Thanks for your PR!

We shouldn't add command-specific code into the main framework in this repo. If there's a bug in runcommand() we'd need to fix it in a command-agnostic way. And if it's a bug within the command itself, it needs to be in the command's respective repo.

• When running WP_CLI::runcommand() with launch => false, the media import command creates an attachment but does not correctly associate it with the provided --post_id.

So why is this happening exactly?

So why is this happening exactly?

Thanks for the review! @swissspidy

The issue occurs because WP_CLI::runcommand() with launch => false does not create a new process, meaning the standard output (stdout) does not behave the same way as when launch => true is used. In the media import command, when --porcelain is used, the expected attachment ID is typically retrieved from stdout. However, with launch => false, the stdout value does not persist reliably, leading to a missing attachment ID.

Since the attachment ID isn't captured, the media file is imported but remains unattached to the specified post. The solution ensures that if --porcelain is used and WP_CLI::runcommand() is run with launch => false, we retrieve the last inserted attachment ID as a fallback.

Would you suggest an alternative way to address this within the command itself rather than the framework? I'm happy to adjust the PR accordingly.

In the media import command, when --porcelain is used, the expected attachment ID is typically retrieved from stdout.

Where is this happening? Is this something you're doing in your own code? Not sure I follow 🤔

AFAICT, the attachment's post_parent is set here by media_handle_sideload():

https://github.com/wp-cli/media-command/blob/09e7ccde11f587cc75f3141aa2ee5397181a12f2/src/Media_Command.php#L415C16-L415C37

And the _thumbnail_id meta value is set here:

https://github.com/wp-cli/media-command/blob/09e7ccde11f587cc75f3141aa2ee5397181a12f2/src/Media_Command.php#L434-L437

I don't see how this is relates to --porcelain usage.

Maybe it would help writing a test first to demonstrate the problem, e.g. in features/runcommand.feature

You're right—the issue isn't directly related to --porcelain. The core problem is that when using WP_CLI::runcommand() with launch => false, the media import command completes successfully, but the attachment isn’t linked to the post as expected.

Since media_handle_sideload() is responsible for setting post_parent, it seems like this behavior might be affected when the command runs in-process instead of spawning a new one. I'll investigate further and look into writing a test case to demonstrate the issue clearly.

Would you suggest a preferred approach to confirm the expected behavior in a test?

Would you suggest a preferred approach to confirm the expected behavior in a test?

A Behat test works great for this. Anything that demonstrates the bug that an attachment isn't linked properly :-)

A Behat test works great for this. Anything that demonstrates the bug that an attachment isn't linked properly :-)

@swissspidy Below is a screenshot and video demonstrating the Behat test I have written for this demo. I am encountering some undefined errors and missing step definitions. Could you please guide me on how to proceed and outline the next steps required to resolve these issues?

I recommend looking at other tests to see how they're written.

"And a post exists" is not a step that is supported.

What you are looking for is

When I run `wp post create --post_title='Test post' --porcelain`

That creates a post for you.

If you then want to use that post's ID in the next step, you can do:

When I run `wp post create --post_title="Foo Bar" --porcelain`
Then STDOUT should be a number
And save STDOUT as {POST_ID}

When I run `wp media import 'http://wp-cli.org/behat-data/codeispoetry.png' --post_id={POST_ID} --title='My\Title' --caption='Caption\Here' --alt='Alt\Here' --desc='Desc\Here' --porcelain`

For regex matching STDOUT you should write it on one line:

Then STDOUT should match /^\d$/

Only if it's not a regex you need to use the quotes and multiline:

Then STDOUT should contain:
  """
  mystring
  """

Checking in here, what's the current status?

Ideally there are some reproduction steps for this reported issue. I recommend sharing them on #6025 first so that they can be validated. Right now it sounds more like a configuration issue or anything, not a bug in WP-CLI.

Closing this PR in favor of discussion on the reported issue as it a) needs reproduction steps and b) this is the wrong approach, as mentioned further above.