fix(pm): hold a bulk send's attachments for the length of the cron run

[e107help]

Why

A private message send over pm_max_send goes out in chunks: the residue is sent now, and the rest of the recipients are serialised into the generic table for the cron task to deliver later. Those rows are the only thing saying an attachment is still owed to the members whose messages do not exist yet.

The cron task deleted that row before it inserted anything (e_cron.php L141-L147). For the length of the run the recipients it had yet to reach held their attachment by neither a private_msg row nor a queue row, so a delete landing in that window read the file as unused and unlinked it, and the download every one of those members was about to be handed was broken before they got it.

@Deltik settled the shape of the fix on 2026-09-12: mark the queued row in progress for the run and remove it after, rather than deleting it first, and rather than simply moving the delete after the send, because that turns a send lost to a mid-run fatal into a send delivered twice.

Refs #6159

What Changed

• The cron task claims a queued send instead of removing it: the row's gen_type moves from pm_bulk to pm_bulk_running for the length of the run, and the row is deleted once add() returns.
• The claim is conditional on the row still being queued, so two runs that both read the same row can only have one of them send it. The update's affected-row count is the answer, and a claim that cannot be made at all, an unreadable table or a lost connection, declines the run rather than sending the chunk twice.
• The survivor check reads both values, so a claimed row still speaks for the attachments it owes (pm_class.php L284-L294 is the query that widens).
• That check also reads the queue before it reads private_msg now, which closes the two-statement version of the same window: a run drops its claimed row only once the messages are in the table, so a delete that reads the queue first can miss neither (pm_class.php L243-L268 is the check as it stood).
• Two class constants carry the two values rather than five string literals across two files. No new public method, and no signature or return shape changes.
• The messenger the run sends through comes from a protected factory and the queue read is protected. Those are the two seams the new tests drive: one puts a delete inside a run, the other finishes a run inside a delete. The attachment fixtures both pm unit tests need move into a base they share.
• pm_cron declares the property it has always written. Creating it on the fly is deprecated from PHP 8.2, and the new tests are the first thing in the unit suite to reach that line.
• plugin.xml takes the date of this commit. The version stays at 3.1, which was already bumped after the last tag.

What the survivor check costs on a large site, a full scan of private_msg per attachment and a read of every queued chunk, is #6420. It predates this PR and its fix is a schema change, so it is filed rather than folded in.

How It Was Tested

New unit coverage in e107_tests/tests/unit/plugins/pm/pm_e_cronTest.php, on the double that puts both attachment trees somewhere disposable: a delete landing inside a bulk run keeps the attachment the queue still owes and the queued row goes once the messages are in; a run that dies part way through keeps its claim, and the next run does not send that chunk again; two runs cannot claim the same send; a generic row of another type is not a send to claim and is left as it stands. That last one is what reds if the claim stops reading the row's type.

The read order has its own case in private_messageAttachmentDeletionTest.php: a bulk run finishes between the survivor check's two reads, and the attachment survives. Reverting the whole file cannot express that mutation, because the seam the test needs is itself part of the change, so it was proven by swapping the two reads by hand: that case fails on its own and the other five stay green.

Run on PHP 8.5 with MariaDB 10.11: the cron cases are green with the fix, red without it, green again with it restored. The whole unit suite, PHP lint and the downgrade check pass. The acceptance suite did not run locally; CI is its first run and the first run of the full matrix.

Backwards Compatibility

Rendered HTML is untouched, no signature changes, and nothing outside the plugin reads or writes gen_type='pm_bulk', so a row moving to pm_bulk_running for the length of a run hides it from nothing that was looking. A queue row written by the current code and processed by the new code is read the same way, and the reverse holds too, so an upgrade mid-queue delivers what is waiting.

One consequence is worth stating plainly, because it is the cost of the decision rather than an oversight. A run that dies part way through a chunk leaves its row claimed. The recipients in that chunk stay undelivered, as they did before, and now the attachments that row names stay held as well, so nothing will unlink them until the row is cleared. Ageing a claim out would deliver part of that chunk a second time, which is the same delivery question again, so this PR does not answer it. On a site where that happens, the row is visible in generic as gen_type='pm_bulk_running'.

AI Model

Claude Opus 5 (claude-opus-5), as e107help.

Checklist

• Traced on master at aa36836 before changing anything
• Reproduction test fails without the fix
• British spelling, no new comments outside docblocks
• Citations pinned to a commit, not a branch
• Adversarial review run twice on the diff; the read-order and coverage findings fixed, the dead-run hold accepted and stated above
• Full CI run