ensure_worker_watches_zips() exists because a worker unit written before zip downloads had their own queue watches default only, and a zip enqueued to zips then waits forever with nothing to say why. It was called from exactly one place: inside the branch that reloads PHP-FPM, nested inside the branch that found a worker unit -- so it ran only when a PHP-FPM unit had been detected. A worker is a different unit from PHP-FPM, and not finding one says nothing about the other: a host running mod_php, or one whose FPM unit is named in a way this script does not recognise, still has a systemd worker that may predate the zips queue, and it got no check and no mention.
The check now also runs in the else branch, where it costs nothing -- its own first line returns immediately unless systemd and a worker unit are both present -- and the worker is restarted after it, paired exactly as the FPM branch pairs them. That pairing is the point: the new --queue argument reaches the worker only when systemd next starts it from ExecStart, and the queue:restart projectsend:update signals cannot deliver it, because that makes a worker pick up new code and it has already run by the time this block is reached. Editing the unit without a restart would leave the operator told that zip downloads were fixed while they still could not finish, which is worse than the silence it replaces: silence sends somebody looking and a success message does not.
Under --no-restart it is said rather than done. Editing a unit file is exactly what that flag asks us not to do, but a worker that cannot finish a zip is broken whether or not we may touch it, and this is the only place that knows to mention it.
Verified before merging: bash -n parses, and the restart block was driven through four host shapes with say, warn, systemctl and the check itself stubbed, on both this branch and main, rather than relying on the transcript in the PR. Before: fpm+worker reached; worker without fpm silent; --no-restart silent; no systemd silent. After: the first two both reached and restarted, --no-restart not reached but said so, no systemd reached and a no-op. That no-op rests on update.sh:330, which returns unless systemd and a worker unit are both present, so it was read rather than assumed.
Reported and fixed by @denkfabrik-li.
ensure_worker_watches_zips reads the unit file with `systemctl show -p
FragmentPath`, and an empty answer meant an immediate, silent return. The
common cause is a mistyped --worker: systemd does not know the unit, the
check never runs, and the operator finishes the update believing their
worker was inspected.
Which produces precisely the outcome the function exists to prevent. Its
own comment says a worker that does not watch the zips queue finishes no
zip downloads while cheerfully sending every email, and that nothing says
why. Skipping the check in silence is a quieter way to arrive there.
It now warns, names the consequence, and says what to check. The
read-only case is separated out too: a unit file somebody else owns
cannot be repaired, but it can still be read, so a worker that is missing
the queue is diagnosed rather than passed over.
Worth recording why this was looked at. The portal session found a deploy
script that had printed `next run` followed by nothing for its whole
life, because `systemctl show` answers an unknown property with an empty
value and a zero exit -- a line always blank is worse than no line, since
somebody believes a check is being performed. FragmentPath here is
correct, verified against a real unit; the failure was the same shape one
step further on, in what an empty answer was taken to mean.
ensure_worker_watches_zips() exists because a worker unit written before
zip downloads had their own queue watches 'default' only, and a zip
enqueued to 'zips' then waits forever with nothing saying why. It is
called from exactly one place: inside the branch that reloads PHP-FPM,
nested inside the branch that found a worker unit.
So it runs only when a PHP-FPM unit was detected. Driving the restart
block through four host shapes, with everything it touches stubbed:
systemd + fpm + worker reached, restarted
systemd + worker, no fpm silent
--no-restart silent
no systemd at all silent
The second line is the one that matters. A worker unit is a different
service from PHP-FPM, and not finding one says nothing about the other: a
host running mod_php, or one whose FPM unit is named in a way this script
does not recognise, can still have a systemd worker that predates the
zips queue. That host gets no check and no mention.
The check now runs in the else branch too, where it costs nothing -- its
own first line returns immediately unless systemd and a worker unit are
both present -- and the worker is restarted after it, paired exactly as
the FPM branch pairs them. That pairing is the point rather than a
flourish: the new --queue argument reaches the worker only when systemd
next starts it from ExecStart. The queue:restart that projectsend:update
signals cannot deliver it, because that makes a worker pick up new *code*
and it has already run by the time this block is reached, so the worker
came back on the old command line. Editing the unit without the restart
would leave the operator told that zip downloads were fixed while they
still could not finish -- worse than the silence it replaces, since
silence sends somebody looking.
Under --no-restart it is said rather than done: editing a unit file is
exactly what that flag asks us not to do, but a worker that cannot finish
a zip is broken whether or not we are allowed to touch it, and this is the
only place that knows to mention it.
Same four shapes afterwards:
systemd + fpm + worker reached, restarted
systemd + worker, no fpm reached, restarted
--no-restart not reached, but said so
no systemd at all reached, no restart (returns immediately)
No test: the suite cannot drive a shell script that restarts services.
bash -n parses, and the harness above is the evidence.
INSTALL.md tells an operator to cache routes, views and events once, and
promises: "You only run these once: projectsend:update notices they are in
place and rebuilds them for you after every update."
It cannot, for anybody who updates with update.sh. The script wipes
bootstrap/cache/*.php while replacing the application files, and
UpdateInstallation::warmCaches() decides what to rebuild by asking
file_exists() on those very paths -- forty lines later. Every installation
looks like one that never cached anything.
Measured in a copy of the tree, the two orderings:
wiping everything, as the script does it
→ "Cleared the compiled configuration, events, routes and views."
→ bootstrap/cache is empty afterwards
keeping the route and event caches
→ "Rebuilt the route, event and view caches — they were in place before."
→ routes-v7.php and events.php are back
So the site quietly loses route, event and view caching on every update,
and the operator is never told.
The wipe now names what it removes rather than taking the directory:
- packages.php and services.php, because they must not survive the swap:
they name the old release's package providers, and the first artisan
run after the copy would try to load classes this version no longer
ships.
- config.php, for a sharper reason. It is read at every boot, so leaving
it means projectsend:update reads the *previous* release's version out
of it. Measured with a doctored version inside a cached config: the run
said "Re-applied 2.2.0" while the release on disk was 9.9.9, recorded
that old version as the one applied, and ran the migrations under the
old configuration. With it removed: "Updated from 2.2.0 to 9.9.9".
The route and event caches stay, since neither is read at boot -- both are
arrays of class names, consulted when a route is matched or an event
dispatched -- and projectsend:update clears them itself moments later.
Removing config.php here would have taken the command's "a cached
configuration was found" warning with it, since it warns about what it
finds. The script now says it, in the same words, at the moment it removes
the file.
No test: the suite covers the command's decision (UpdateCommandTest pins
the whole rewarm matrix) and cannot run a shell script that replaces an
installation. The measurements above are the evidence; bash -n parses.
Splitting zip builds onto their own queue (92a132d) left manual installs
carrying the one job the release note has to do, and the failure it
produces is the worst shape available: a worker still watching only
`default` sends every email cheerfully and finishes no zip downloads,
with nothing in any log to say why. An upgrade note is a poor place to
put that, because it is read on a laptop and needed on a server.
update.sh already finds projectsend-worker.service, so it now reads the
unit's ExecStart and offers to add --queue=default,zips, keeping a copy
of the original beside it. Before the restart, so the worker comes back
on the command it is going to keep.
Only the unambiguous case is rewritten: a queue:work line with no
--queue at all, which consumes `default` and nothing else. A unit that
already names its queues is somebody's deliberate arrangement, possibly
with a second worker for zips, so that one is described rather than
edited — and one that already includes zips is silently left alone.
Exercised against five unit shapes rather than reasoned about: the plain
command is rewritten and backed up, a declined prompt leaves it untouched
with a warning, a unit already naming zips is a no-op, a custom queue list
without zips warns instead of editing, and a unit that is not queue:work
at all is ignored. The sed itself would double-append if it ran twice;
it cannot, because the --queue= guard above it returns first, and both
read the same first ExecStart line.
The last thing an operator sees after a successful update is now where the
people are. Printed rather than asked: the answer to "would you like to
join?" is a browser, and this runs over SSH on a server that has none, so
a y/n could only print the URL its "no" branch prints anyway — and --yes
would answer it on behalf of a cron job that cannot join anything.
Only on success. --check is a report somebody may be scripting, and a
failed run's last line should stay the recovery instructions.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`chown -R` on the install directory also tries to change the ownership of
public/storage, which is a symlink — and some filesystems refuse that even
for root. It turned up on a bind-mounted install (a `fakeowner` mount
refuses lchown outright) and would equally turn up on an NFS export with
root_squash. The update stopped there: files replaced, database not yet
migrated, which is the worst place to stop.
A symlink's own ownership decides nothing, since access is governed by
whatever it points at, so those are now skipped rather than tolerated by
ignoring errors that might have mattered elsewhere.
Found by running the thing on a manual install rather than by reading it.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Updating a server install cost nine artisan invocations plus a PHP-FPM
reload, written out in three places that had already drifted apart. One
of those steps is silently fatal to skip: with opcache.validate_timestamps
off — what production guides recommend and what our own image ships — the
database moves to the new version while every visitor keeps being served
the old code, and artisan reports the new version throughout.
`sudo ./update.sh` is now the whole procedure. It asks whether to check
GitHub, asks whether to download the release and verifies the checksum
published beside it, and asks whether there is a backup — offering to dump
the database when the answer is no. Then it takes the site down, replaces
the files, runs the update, reloads PHP-FPM, restarts the worker and
brings the site back. The application still has no self-updater: nothing
is fetched or applied unless somebody runs this and answers yes.
Underneath it is `php artisan projectsend:update`, which is everything an
update does that needs no root — and now the only definition of it. Both
container entrypoints call it instead of carrying their own copy of the
sequence, so the two paths cannot drift again.
Three findings worth keeping in the record, all from rehearsing rather
than reasoning:
- queue:restart has to come last. It writes its signal into the cache,
so clearing the cache afterwards deletes it and the worker runs old
code forever.
- optimize:clear is not safe to recommend. It runs cache:clear, which
on Redis is FLUSHDB — harmless on the default two-database layout,
but on a single-database Redis it takes the sessions and the queue
with it. The compiled caches are cleared individually instead.
- update.sh overwrites itself mid-run, because the zip contains it and
bash reads its own script lazily by byte offset. It re-execs from a
temporary copy before touching anything.
And when the reload is skipped anyway, the application now says so:
projectsend:update records the version it applied, and any staff page
compares that with what the running process actually compiled. The same
check catches the mirror image — new files in place, update never run.
Rehearsed end to end against real installs: a container upgrade (69 to 73
migrations, key and data intact, healthy), a scripted update on a real
nginx + php-fpm install with OPcache pinned (web process moved 2.1.0 to
2.1.1), the skipped-reload case (banner appears naming both versions, and
clears on reload), the refusals (downgrade, non-release zip, truncated
zip, URL passed to --zip, non-root), a database taken down mid-update
(site comes back out of maintenance mode by itself), and a real download
of the published 2.0.0 zip with its checksum verified.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Ignacio Nelson