From 07634c8a1de8d35612336380e362421e204e3027 Mon Sep 17 00:00:00 2001 From: nexxo Date: Tue, 28 Jul 2026 22:25:07 +0200 Subject: [PATCH] Answer a check at once, start an update at once, and count down to neither MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Checking for updates was a request written into the same mailbox as a real update, collected by the same agent on the same systemd timer — so asking what was new was answered on the next tick, and the console, having nothing else to show, counted down to it. An operator who asked a question was told an update would start in 3:44. A systemd path unit now wakes the agent the moment the request file is written, for a check and for a run alike. PathChanged, not PathExists: PathExists re-arms as soon as the service goes inactive, so a request the agent could not consume — it holds a lock for the length of an update — would restart it in a tight loop. The timer stays as the fallback, and a drop-in disables systemd's start rate limit, because two triggers on one oneshot service can otherwise wedge the unit and leave neither of them able to run. The countdown is gone from both the settings card and the maintenance overlay. It could not be made honest: its target was recomputed on every poll, and where the interval the agent reports is shorter than the timer actually installed on that host it had already gone by, so the fallback moved it forward again each time. It ran backwards four seconds and snapped back to a full minute, forever — reported exactly that way. A queued run says it is starting; a queued check says nothing beyond its badge, because a check starts nothing. Co-Authored-By: Claude Opus 5 --- app/Services/Deployment/UpdateChannel.php | 80 ++++------ deploy/install-agent.sh | 44 +++++ deploy/update-agent.sh | 14 ++ lang/de/admin_settings.php | 14 +- lang/en/admin_settings.php | 14 +- resources/js/app.js | 58 +------ resources/views/layouts/admin.blade.php | 18 ++- .../views/livewire/admin/settings.blade.php | 32 ++-- routes/admin.php | 4 - tests/Feature/Admin/UpdateButtonTest.php | 150 ++++++++++++++---- 10 files changed, 256 insertions(+), 172 deletions(-) diff --git a/app/Services/Deployment/UpdateChannel.php b/app/Services/Deployment/UpdateChannel.php index 3a3395f..00690dd 100644 --- a/app/Services/Deployment/UpdateChannel.php +++ b/app/Services/Deployment/UpdateChannel.php @@ -152,11 +152,18 @@ final class UpdateChannel 'requested_at' => $request !== null ? $this->timestamp($request['requested_at'] ?? null) : null, 'requested_by' => $request['requested_by'] ?? null, - // When the agent will next look. A queued update sits until the - // timer fires, and "in a few minutes" is exactly the answer that - // leaves an operator refreshing the page wondering whether the - // button did anything at all. - 'next_check_at' => $agentAlive ? $this->nextCheckAt($checkedAt, $status) : null, + // Is the agent woken the moment a request lands, or does the + // request wait for the next tick? + // + // Reported by the agent rather than decided here: the trigger is a + // systemd path unit on the host (deploy/install-agent.sh), which + // the panel cannot see from inside its container, and a server + // that has not re-run install-agent.sh since this landed still + // waits for the timer. It decides whether the console may say + // "now" or has to name a time — and promising "now" on an + // installation that will take five minutes is the failure this + // whole field exists to avoid. + 'instant' => $agentAlive && ($status['request_watch'] ?? null) === true, // The step the deployment script is on — a key, translated here. // Read live from the phase file; older agents write none, so its @@ -242,47 +249,20 @@ final class UpdateChannel return $key; } - /** - * The latest moment a queued update can still be picked up. + /* + * There was a nextCheckAt() here, and a checkInterval() feeding it, so the + * console could count down to the agent's next tick. Both are gone. * - * Normally one interval after the agent's last check. But a systemd timer - * that ran late leaves a last check already older than its interval, and - * `checked_at + interval` is then a time in the PAST — the page would - * promise a start that has demonstrably not happened. In that case the only - * honest upper bound is one interval from now. + * It could not be made honest. `checked_at + interval` is in the past + * whenever the interval the agent reports is shorter than the timer + * actually installed on that host — which is every server that has not + * re-run install-agent.sh — and the fallback then returned "now + interval" + * afresh on every poll. The operator watched it run down four seconds and + * snap back to a full minute, forever. Reported exactly that way. * - * @param array $status + * It is also no longer needed: the request wakes the agent (the path unit + * in deploy/install-agent.sh), so there is no wait to describe. */ - private function nextCheckAt(?Carbon $checkedAt, array $status): ?Carbon - { - if ($checkedAt === null) { - return null; - } - - $interval = $this->checkInterval($status); - $due = $checkedAt->copy()->addMinutes($interval); - - return $due->isFuture() ? $due : Carbon::now()->addMinutes($interval); - } - - /** - * How often the agent looks, in minutes. - * - * Reported by the agent rather than assumed here: the interval lives in the - * systemd timer, and a server whose timer was tuned would otherwise be - * given a next-check time that quietly never arrives. - * - * @param array $status - */ - private function checkInterval(array $status): int - { - $minutes = (int) ($status['check_interval_minutes'] ?? 0); - - // Clamped, not trusted: this value ends up in a promise to the operator, - // and a corrupt file must not produce "next check in 1970" or a time so - // far out that the page looks broken. - return $minutes >= 1 && $minutes <= 60 ? $minutes : 5; - } /** * Has the agent checked in recently enough to be considered alive? @@ -339,11 +319,15 @@ final class UpdateChannel * Ask the agent only to look — never to apply anything. * * The agent already fetches on every tick regardless of this; the request - * exists so an operator who only wants an answer gets one on the next - * tick rather than waiting on whichever tick happens to come next on its - * own, and so the answer never runs through the "run" branch — no - * maintenance mode, no restart, for a question that was only "is there - * something new". + * exists so an operator who asks gets an answer at the moment they ask, + * and so the answer never runs through the "run" branch — no maintenance + * mode, no restart, for a question that was only "is there something new". + * + * A question must not sit in a queue. Where the host runs the path unit + * from deploy/install-agent.sh this file wakes the agent within a second; + * where it does not, the timer is the fallback and the console says so + * rather than counting down to a start that is not happening — see + * 'instant' in state(). */ public function requestCheck(string $by): bool { diff --git a/deploy/install-agent.sh b/deploy/install-agent.sh index 1fdd712..4e3df87 100755 --- a/deploy/install-agent.sh +++ b/deploy/install-agent.sh @@ -34,6 +34,17 @@ WorkingDirectory=$ROOT ExecStart=$ROOT/deploy/update-agent.sh EOF +# Two things start this service now — the timer below and the path unit after +# it. A burst of starts would otherwise trip systemd's default rate limit and +# leave the unit in a failed state, at which point NEITHER trigger runs and the +# console can no longer update the installation at all. A busy agent is a far +# smaller problem than a wedged one. +mkdir -p /etc/systemd/system/clupilot-update-agent.service.d +cat > /etc/systemd/system/clupilot-update-agent.service.d/rate-limit.conf <<'EOF' +[Unit] +StartLimitIntervalSec=0 +EOF + cat > /etc/systemd/system/clupilot-update-agent.timer <<'EOF' [Unit] Description=Check for CluPilot updates and run requested ones @@ -54,6 +65,38 @@ AccuracySec=5s WantedBy=timers.target EOF +# ── Answering a question at the moment it is asked ─────────────────────────── +# The timer above decides how often the agent looks on its own. It must not also +# decide how long someone waits after pressing a button. +# +# "Nach Aktualisierungen suchen" is a QUESTION. Routed through the timer it was +# answered on the next tick — up to five minutes later on a server whose timer +# still had the old interval — and the console, having nothing else to show, +# counted down to it. An operator who asked what was new was told an update +# would start in 3:44. Nothing was starting; nothing had been asked to start. +# +# So the request file gets its own trigger. The panel writes it inside the +# container, the checkout is bind-mounted, and the host sees the write on the +# same inode — the agent is woken within a second, for a check and for a real +# run alike. +# +# PathChanged, deliberately, not PathExists: PathExists re-arms as soon as the +# service goes inactive, so a request the agent could not consume — it holds a +# lock while an update runs — would restart it in a tight loop for the length of +# that update. PathChanged fires once, when the file is written and closed. A +# trigger missed because the agent was busy is picked up by the timer. +cat > /etc/systemd/system/clupilot-update-agent.path </dev/null +systemctl enable --now clupilot-update-agent.path >/dev/null # Run it once now, so the panel has a status to show instead of "never reported # in" for the first minute. diff --git a/deploy/update-agent.sh b/deploy/update-agent.sh index dec1617..cc086c2 100755 --- a/deploy/update-agent.sh +++ b/deploy/update-agent.sh @@ -53,6 +53,19 @@ REQUEST_EXPIRES_MINUTES=30 # whether anything is happening at all. CHECK_INTERVAL_MINUTES=1 +# Can this agent be woken the moment a request lands, or only on the next tick? +# +# Installed by install-agent.sh as clupilot-update-agent.path. It is reported +# rather than assumed because it lives on the host: a server that has not run +# install-agent.sh since this landed still waits for the timer, and the console +# has to say so instead of promising an answer it will not get. Reported by the +# agent for the same reason CHECK_INTERVAL_MINUTES is — the panel cannot see +# systemd from inside its container. +REQUEST_WATCH=false +if systemctl is-active --quiet clupilot-update-agent.path 2>/dev/null; then + REQUEST_WATCH=true +fi + mkdir -p "$STATE_DIR" # One agent at a time. Two overlapping runs of update.sh fight over the @@ -143,6 +156,7 @@ write_status() { "mode": "$(json_escape "$MODE")", "checked_at": "$(date -u +%Y-%m-%dT%H:%M:%SZ)", "check_interval_minutes": ${CHECK_INTERVAL_MINUTES}, + "request_watch": ${REQUEST_WATCH}, "started_at": "$(json_escape "${STARTED_AT:-}")", "finished_at": "$(json_escape "${FINISHED_AT:-}")", "local_commit": "$(json_escape "${LOCAL_COMMIT:-}")", diff --git a/lang/de/admin_settings.php b/lang/de/admin_settings.php index c024590..9981f8e 100644 --- a/lang/de/admin_settings.php +++ b/lang/de/admin_settings.php @@ -127,10 +127,16 @@ return [ // auf der Einstellungsseite. 'update_overlay_title' => 'Aktualisierung läuft.', - // Nur das Label — die Ziffern daneben zählen im Browser live herunter - // (resources/js/app.js), damit "Startet in 0:47" auch wirklich 0:47 sagt - // und nicht eine irgendwann errechnete Uhrzeit. - 'update_starts_in' => 'Startet in', + // Kein Countdown mehr, weder beim Prüfen noch beim Aktualisieren. Der alte + // lief rückwärts vier Sekunden und sprang zurück auf eine Minute, weil das + // Ziel bei jedem Poll neu gerechnet wurde und auf jedem Server, dessen + // Timer länger ist als das gemeldete Intervall, in der Vergangenheit lag. + // Und er wird nicht gebraucht: der Dienst wird beim Anfordern geweckt. + 'update_starting' => 'Wird jetzt gestartet.', + // Nur für Server, auf denen deploy/install-agent.sh noch nicht neu gelaufen + // ist — dort holt der Timer die Anforderung ab, und das wird gesagt statt + // eine Sofortigkeit zu versprechen, die dieser Server nicht liefert. + 'update_starting_queued' => 'Wird gestartet, sobald der Dienst die Anforderung übernimmt.', 'update_step' => 'Schritt: :step', 'update_since' => 'Läuft seit :when', 'update_offline_hint' => 'Während der Aktualisierung ist die Seite kurz im Wartungsmodus — diese Anzeige bleibt so lange stehen und meldet sich danach von selbst zurück.', diff --git a/lang/en/admin_settings.php b/lang/en/admin_settings.php index 4846f5e..e8df274 100644 --- a/lang/en/admin_settings.php +++ b/lang/en/admin_settings.php @@ -126,10 +126,16 @@ return [ // on every console page while an update is running, not just Settings. 'update_overlay_title' => 'Update in progress.', - // Label only — the digits beside it count down live in the browser - // (resources/js/app.js), so "Starts in 0:47" actually says 0:47 rather - // than a clock time computed once and left to go stale. - 'update_starts_in' => 'Starts in', + // No countdown any more, for a check or for a run. The old one ran back + // four seconds and snapped to a full minute, because its target was + // recomputed on every poll and landed in the past on every host whose timer + // is longer than the interval the agent reports. Nor is it needed: the + // agent is woken when the request lands. + 'update_starting' => 'Starting now.', + // Only for servers where deploy/install-agent.sh has not been re-run — the + // timer collects the request there, and saying so beats promising an + // immediacy that host does not deliver. + 'update_starting_queued' => 'Starting as soon as the agent picks the request up.', 'update_step' => 'Step: :step', 'update_since' => 'Running since :when', 'update_offline_hint' => 'During the update the site is briefly in maintenance mode — this view stays put and comes back on its own afterwards.', diff --git a/resources/js/app.js b/resources/js/app.js index e09e2b0..eea91e9 100644 --- a/resources/js/app.js +++ b/resources/js/app.js @@ -175,21 +175,6 @@ window.fetch = async (...args) => { return response; }; -// Minutes:seconds until `targetIso`, floored at zero. Shared by the -// maintenance overlay and the settings card's own countdowns — the one bit of -// arithmetic behind both, kept in one place so a rounding fix does not need -// to be made twice. Never assembles a sentence, only the digits: the words -// around it are always server-translated. -function minutesSeconds(targetIso, nowMs) { - if (!targetIso) return '0:00'; - - const remaining = Math.max(0, Math.round((new Date(targetIso).getTime() - nowMs) / 1000)); - const m = Math.floor(remaining / 60); - const s = remaining % 60; - - return `${m}:${String(s).padStart(2, '0')}`; -} - document.addEventListener('alpine:init', () => { // ── Watching a deployment that restarts the watcher ────────────────── // @@ -203,7 +188,7 @@ document.addEventListener('alpine:init', () => { // state, no assets — and, crucially, treats a failed request as the // restart rather than as an error worth giving up over. window.Alpine.data('updateWatcher', ({ - url, commit, running, step = null, nextCheckAt = null, runningSince = null, log = null, + url, commit, running, step = null, runningSince = null, log = null, }) => ({ // The build this page was rendered from. When the server stops // agreeing, the page is stale by definition and must be replaced — @@ -217,19 +202,10 @@ document.addEventListener('alpine:init', () => { step, runningSince, log, - // Not text: the instant the agent will next look, for the live - // countdown below to count down to. - nextCheckAt, - now: Date.now(), timer: null, - clock: null, init() { this.schedule(); - // A second hand for the countdown, independent of how often - // `check()` actually polls — otherwise "Startet in" would jump in - // three-second steps instead of reading as live. - this.clock = setInterval(() => { this.now = Date.now(); }, 1000); // Nothing to watch while the tab is hidden; pick straight back up // rather than waiting out a whole interval on return. document.addEventListener('visibilitychange', () => { @@ -237,12 +213,6 @@ document.addEventListener('alpine:init', () => { }); }, - // Queued: the agent has not picked the request up yet, so there is no - // step to show. Once it has, `step` arrives and this stops mattering. - get countdownLabel() { - return minutesSeconds(this.nextCheckAt, this.now); - }, - schedule() { clearTimeout(this.timer); this.timer = setTimeout(() => this.check(), this.wasRunning ? 3000 : 20000); @@ -279,7 +249,6 @@ document.addEventListener('alpine:init', () => { this.step = state.step ?? null; this.runningSince = state.running_since ?? null; this.log = state.log ?? null; - this.nextCheckAt = state.next_check_at ?? null; } catch { // Expected while the containers are down. Keep asking: this is // the middle of the very event being watched, not a fault. @@ -291,31 +260,6 @@ document.addEventListener('alpine:init', () => { destroy() { clearTimeout(this.timer); - clearInterval(this.clock); - }, - })); - - // ── A live "starts in" countdown ────────────────────────────────────── - // Used on the settings card wherever it is waiting on the agent's next - // tick — a queued check or a queued run. Self-contained rather than - // reading off updateWatcher above: this lives inside a Livewire-rendered - // card, and a poll-driven re-render is not guaranteed to preserve a - // nested component's state the way a plain re-render of this one is. - window.Alpine.data('countdown', (target) => ({ - target, - now: Date.now(), - timer: null, - - init() { - this.timer = setInterval(() => { this.now = Date.now(); }, 1000); - }, - - get label() { - return minutesSeconds(this.target, this.now); - }, - - destroy() { - clearInterval(this.timer); }, })); diff --git a/resources/views/layouts/admin.blade.php b/resources/views/layouts/admin.blade.php index 2693203..f6dc560 100644 --- a/resources/views/layouts/admin.blade.php +++ b/resources/views/layouts/admin.blade.php @@ -111,7 +111,6 @@ commit: @js($updateState['commit']), running: @js($updateState['running']), step: @js($updateStep), - nextCheckAt: @js($updateState['next_check_at']?->toIso8601String()), runningSince: @js($updateRunningSince), log: @js($updateLog), })" @@ -132,13 +131,20 @@

{{ __('admin_settings.update_overlay_title') }}

{{ __('admin_settings.update_offline_hint') }}

- {{-- Queued: the agent has not picked the request up yet, so - there is no step. A live countdown instead of silence — - "Startet in 0:47" reads as something happening; several - minutes of nothing reads as broken. --}} + {{-- Queued: the agent has not picked the request up yet, so there + is no step yet either. + + This used to count down to the agent's next tick. It was wrong + twice over. Wrong in fact: the countdown target was recomputed + on every poll, and where the reported interval was shorter than + the timer actually installed it landed in the past every time + and reset to a full minute — so it ran backwards four seconds + and jumped, forever. And wrong in kind: with the agent woken + the moment the request lands there is nothing left to wait for. + A sentence, not a clock. --}}

- {{ __('admin_settings.update_starts_in') }} + {{ $updateState['instant'] ? __('admin_settings.update_starting') : __('admin_settings.update_starting_queued') }}

{{-- Running: the step, how long it has been going, and the tail diff --git a/resources/views/livewire/admin/settings.blade.php b/resources/views/livewire/admin/settings.blade.php index 6c91978..97b913e 100644 --- a/resources/views/livewire/admin/settings.blade.php +++ b/resources/views/livewire/admin/settings.blade.php @@ -122,13 +122,21 @@ @endif {{-- Where it is. "Läuft gerade" alone is what sends an operator to - the shell: the service checks on a timer, so a queued update - does nothing at all until the agent's next tick, and while it - runs the site is in maintenance mode and this page is - unreachable. Both of those look identical to "wedged" without - a time — so a live countdown while queued, replacing what used + the shell: a queued update does nothing visible until the agent + picks it up, and while it runs the site is in maintenance mode + and this page is unreachable. Both look identical to "wedged" + without a time — hence the live countdown, replacing what used to be a static "spätestens um 17:19" that read as a promise - nothing then visibly kept. --}} + nothing then visibly kept. + + There is no countdown any more, for either. It was wrong in + fact — the target was recomputed on every poll, and where the + interval the agent reported was shorter than the timer actually + installed on that host it landed in the past every time and + reset to a full minute, so it ran backwards a few seconds and + jumped, forever. And it was wrong in kind: a check starts + nothing, and an update now starts at once. A queued run says so + in a sentence; a queued check says nothing beyond its badge. --}} @if ($update['running'])
@if ($update['phase']) @@ -136,19 +144,13 @@ @if ($update['started_at'])

{{ __('admin_settings.update_since', ['when' => $update['started_at']->diffForHumans()]) }}

@endif - @elseif ($update['next_check_at']) -

- {{ __('admin_settings.update_starts_in') }} + @else +

+ {{ $update['instant'] ? __('admin_settings.update_starting') : __('admin_settings.update_starting_queued') }}

@endif

{{ __('admin_settings.update_offline_hint') }}

- @elseif ($update['checking'] && $update['next_check_at']) -
-

- {{ __('admin_settings.update_starts_in') }} -

-
@endif @if ($update['last_state'] !== null && ! $update['running']) diff --git a/routes/admin.php b/routes/admin.php index b0ab9d6..1f28a2f 100644 --- a/routes/admin.php +++ b/routes/admin.php @@ -98,10 +98,6 @@ Route::get('/update/state', function (UpdateChannel $channel) { 'step' => $state['phase'] !== null ? __('admin_settings.update_step', ['step' => $state['phase']]) : null, - // The raw instant, not a sentence: the overlay ticks a live "Startet - // in 0:47" from this on the client, which needs an unambiguous point - // in time to count down to, not translated text. - 'next_check_at' => $state['next_check_at']?->toIso8601String(), // Ready to print — the same sentence update_since renders on the // settings card — so the overlay never assembles it either. 'running_since' => $started diff --git a/tests/Feature/Admin/UpdateButtonTest.php b/tests/Feature/Admin/UpdateButtonTest.php index ddd4ea9..7194a3c 100644 --- a/tests/Feature/Admin/UpdateButtonTest.php +++ b/tests/Feature/Admin/UpdateButtonTest.php @@ -120,21 +120,21 @@ it('survives a status file that is corrupt', function () { ->assertOk(); }); -it('carries a live countdown target for a queued update, not a server-formatted clock time', function () { - // The agent checks on a timer, so pressing the button visibly does - // nothing until the next tick. Silence there reads as a broken button — - // reported exactly that way — so the card now counts down live in the - // browser (resources/js/app.js) instead of naming a clock time. +it('says a queued update is starting, and never counts down to it', function () { + // There was a live countdown here to the agent's next tick. It ran + // backwards a few seconds and snapped back to a full minute, over and + // over, and it did so on every server whose systemd timer is longer than + // the interval the agent reports — the target was recomputed on each poll + // and had already gone by, so the "honest upper bound" fallback moved it + // forward again every time. Reported exactly that way. // - // Frozen: the assertion is about the arithmetic, and a clock that moves - // between writing the file and reading it turns 1 into 0.998. - Carbon::setTestNow(Carbon::parse('2026-07-27 12:34:00')); - + // Nothing counts down now. The agent is woken when the request lands. writeStatus([ 'state' => 'idle', 'checked_at' => now()->toIso8601String(), 'check_interval_minutes' => 1, 'behind' => 1, + 'request_watch' => true, ]); app(UpdateChannel::class)->request('owner@example.com'); @@ -142,17 +142,11 @@ it('carries a live countdown target for a queued update, not a server-formatted $state = app(UpdateChannel::class)->state(); expect($state['running'])->toBeTrue() - ->and($state['next_check_at']?->toIso8601String())->toBe(now()->addMinutes(1)->toIso8601String()); + ->and($state)->not->toHaveKey('next_check_at'); - // The raw instant, not a clock time formatted server-side: there is no - // local()-vs-utc conversion left here for R19 to catch a regression of — - // the browser owns the countdown and localises it itself by construction - // (new Date() on an ISO string with an explicit offset). What matters is - // that the RIGHT instant reaches the page. Livewire::actingAs(operator('Owner'), 'operator') ->test(AdminSettings::class) - ->assertSee(__('admin_settings.update_starts_in')) - ->assertSee($state['next_check_at']->toIso8601String(), escape: false); + ->assertSee(__('admin_settings.update_starting')); }); it('names the step the deployment is on, in the operator’s language', function () { @@ -221,7 +215,7 @@ it('does not present the last failure’s step as this update’s progress', fun Livewire::actingAs(operator('Owner'), 'operator') ->test(AdminSettings::class) - ->assertSee(__('admin_settings.update_starts_in')) + ->assertSee(__('admin_settings.update_starting_queued')) ->assertDontSee(__('admin_settings.update_phase.migrate')); }); @@ -246,28 +240,32 @@ it('ignores a step written before the run that is going on now', function () { expect(app(UpdateChannel::class)->state()['phase'])->toBeNull(); }); -it('never promises a start time that has already gone by', function () { - // A systemd timer that ran late leaves the last check older than its own - // interval. checked_at + interval is then in the past, and the page would - // promise a start that demonstrably did not happen. +it('promises no start time at all, on the exact configuration that used to loop', function () { + // The reported case, reproduced: the agent reports a one-minute interval + // (the value in the repo) while the host's systemd timer is still five, so + // checked_at + interval is in the past for four minutes out of every five. + // The old code answered "now + interval" afresh on every poll, which is a + // countdown that resets forever instead of reaching zero. + // + // There is nothing left to get wrong: no time is promised anywhere. Carbon::setTestNow(Carbon::parse('2026-07-27 12:34:00')); writeStatus([ 'state' => 'idle', - 'checked_at' => now()->subMinutes(12)->toIso8601String(), - 'check_interval_minutes' => 5, + 'checked_at' => now()->subMinutes(4)->toIso8601String(), + 'check_interval_minutes' => 1, 'behind' => 1, ]); app(UpdateChannel::class)->request('owner@example.com'); - $next = app(UpdateChannel::class)->state()['next_check_at']; + expect(app(UpdateChannel::class)->state())->not->toHaveKey('next_check_at'); - expect($next)->not->toBeNull() - ->and($next->isFuture())->toBeTrue() - // One interval from now is the only honest upper bound once the timer - // is already overdue. - ->and($next->toIso8601String())->toBe(now()->addMinutes(5)->toIso8601String()); + Livewire::actingAs(operator('Owner'), 'operator') + ->test(AdminSettings::class) + // No clock, and — the part that actually broke — no element for a + // browser-side timer to keep resetting. + ->assertDontSee('x-data="countdown', escape: false); }); it('names the step a failed update stopped at', function () { @@ -446,6 +444,89 @@ it('confirms a check was requested, worded for a check rather than an update', f ->assertDispatched('notify', message: __('admin_settings.update_check_requested')); }); +/** + * A check is a question, and a question does not get a countdown. + * + * Reported verbatim: "ich suche nur nach neuen Updates, was ist das da für ein + * Timer, den ich nicht gestartet habe?" The check had been routed through the + * same timer as a real update, so the card showed the only thing it had — + * "Startet in 3:44" — for an operator who had asked for nothing to start. + * + * The agent is now woken the moment the request lands (the path unit in + * deploy/install-agent.sh) and reports whether that trigger exists, because a + * server that has not re-run the installer still waits for the timer and must + * not be told otherwise. + */ +it('shows a pending check its badge and nothing else — no time, no clock', function () { + // Even on a host with no path unit, where the answer really does wait for + // the timer. A check starts nothing, so there is nothing to announce; the + // badge saying it is looking is the whole of what can honestly be said. + writeStatus([ + 'state' => 'idle', + 'checked_at' => now()->toIso8601String(), + 'check_interval_minutes' => 5, + 'behind' => 0, + 'request_watch' => false, + ]); + + app(UpdateChannel::class)->requestCheck('owner@example.com'); + + Livewire::actingAs(operator('Owner'), 'operator') + ->test(AdminSettings::class) + ->assertSee(__('admin_settings.update_checking')) + ->assertDontSee(__('admin_settings.update_starting')) + ->assertDontSee(__('admin_settings.update_starting_queued')); +}); + +it('tells a queued update apart from an immediate one, without either naming a time', function () { + // The one thing 'instant' still decides: whether the console may say the + // update starts NOW, or has to admit it starts when the agent gets to it. + // Neither answer contains a number. + writeStatus([ + 'state' => 'idle', + 'checked_at' => now()->toIso8601String(), + 'check_interval_minutes' => 5, + 'behind' => 1, + 'request_watch' => false, + ]); + + app(UpdateChannel::class)->request('owner@example.com'); + + Livewire::actingAs(operator('Owner'), 'operator') + ->test(AdminSettings::class) + ->assertSee(__('admin_settings.update_starting_queued')) + ->assertDontSee(__('admin_settings.update_starting')); +}); + +it('does not believe a stale agent about being woken on demand', function () { + // The field says what was installed when the agent last ran. From an agent + // that has since stopped it is not a promise anybody can keep. + writeStatus([ + 'state' => 'idle', + 'checked_at' => now()->subHour()->toIso8601String(), + 'behind' => 0, + 'request_watch' => true, + ]); + + expect(app(UpdateChannel::class)->state()['instant'])->toBeFalse(); +}); + +it('keeps the panel, the agent and the installer in step about the on-demand trigger', function () { + // Three files, one contract: the installer creates the path unit, the agent + // reports whether it is active, the panel decides from that whether it may + // say "now". If they drift, the console promises an instant answer on a + // server that still waits five minutes for it — which is the exact failure + // this replaced. + $installer = File::get(base_path('deploy/install-agent.sh')); + $agent = File::get(base_path('deploy/update-agent.sh')); + + expect($installer)->toContain('/etc/systemd/system/clupilot-update-agent.path') + ->and($installer)->toContain('PathChanged=') + ->and($installer)->toContain('systemctl enable --now clupilot-update-agent.path') + ->and($agent)->toContain('is-active --quiet clupilot-update-agent.path') + ->and($agent)->toContain('"request_watch"'); +}); + it('offers "Jetzt aktualisieren" even when the last check found nothing', function () { // Reported as "I cannot run an update, it says everything is current". // `behind` is a READING taken up to a minute ago, not the state of the @@ -533,7 +614,7 @@ it('answers the watcher without Livewire in the way', function () { $this->actingAs(operator('Owner'), 'operator') ->get(route('admin.update.state')) ->assertOk() - ->assertJsonStructure(['running', 'commit', 'behind', 'last_finished_at', 'step', 'next_check_at', 'running_since', 'log']) + ->assertJsonStructure(['running', 'commit', 'behind', 'last_finished_at', 'step', 'running_since', 'log']) ->assertHeader('Cache-Control', 'no-store, private'); }); @@ -602,8 +683,8 @@ it('carries the tail of the run log in the watcher JSON once a run has actually }); it('carries no running-since or log while only queued, not yet actually running', function () { - // Otherwise this would be the PREVIOUS run's leftovers, arriving under a - // countdown that says the run has not started yet. + // Otherwise this would be the PREVIOUS run's leftovers, presented as the + // progress of a run that has not begun. writeStatus(['state' => 'idle', 'checked_at' => now()->toIso8601String(), 'behind' => 1]); File::ensureDirectoryExists(storage_path('app/deploy')); File::put(storage_path('app/deploy/update-last-run.log'), "an earlier run\n"); @@ -614,7 +695,8 @@ it('carries no running-since or log while only queued, not yet actually running' ->get(route('admin.update.state')) ->assertOk() ->assertJson(['running_since' => null, 'log' => null]) - ->assertJsonStructure(['next_check_at']); + // Nothing for the overlay to count down to, either. + ->assertJsonMissing(['next_check_at' => null]); }); it('does not answer the watcher for somebody who is not an operator', function () {