arch-updater: fix the news-timer watchdog kill; terminal/background update modes, ignore management, one polkit password, update history with rollback - #384
Conversation
|
waiting on author review |
The startup branch of the Arch news check compares the counter against AUTO_CHECK_DELAY but resets it to 0, so after the first run the counter climbs back to AUTO_CHECK_DELAY and checkNews() fires every 10 ticks forever. Each firing is an HTTP fetch plus RSS parsing in the service callback; the repeated work exceeds the 25 ms CPU budget and the health watchdog auto-disables the whole plugin about 50 seconds after the shell starts whenever check_arch_news is enabled (the default). Reset the counter back to AUTO_CHECK_DELAY instead, so the startup branch cannot re-fire and the periodic branch keeps its 6-hour cadence. Fixes noctalia-dev#382 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… password Rework the update flow so one click upgrades the system in the background instead of opening a terminal: - The run is spawned detached (double-fork + setsid) with pkexec for pacman and '--sudo pkexec' plus non-interactive flags for the AUR helper, writing to <data>/update.log between ::START/::EXIT markers. The engine tails the log every 2 s; the panel replaces the package list with a live log tail and a progress bar, the bar widget shows percent. A restarted shell re-attaches to an unfinished run; a log silent for 30 minutes fails the run. On success: notification plus automatic re-check. - The old terminal flow stays as an explicit 'Retry in terminal' fallback after a failed run (and an update_terminal IPC event), tee'd into the same log. - Ignore management in the panel: packages held back by pacman.conf's IgnorePkg arrive as '[ignored]' lines and no longer count as pending; they show in a new expandable Ignored section instead. Each package row gets an ignore button (persisted to <data>/ignore.json since the panel cannot write settings), panel-managed entries get a restore button, and settings/pacman.conf entries are tagged with the place they are managed. New ignore:NAME / unignore:NAME IPC events. - pkexec re-authenticates every pacman transaction, so a run could raise several password dialogs. The panel offers to install (one confirmed pkexec call) a narrow polkit rule - auth_admin_keep for pkexec running /usr/bin/pacman, active local wheel session - so one password covers a run; the rule also ships in polkit/ for manual install. rules.d is not readable by regular users on Arch, so a marker in the data dir remembers a successful install. - Settings: assume_yes, show_activity_graph and activity_history_length are gone (the background run is always non-interactive and the activity graph gave way to the log view); log_lines and hide_polkit_hint added; terminal is now fallback-only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
4437898 to
51217e4
Compare
|
Thanks for the effort here, the background-run mechanics (detached run, Blocking1. Please keep the activity graph (opt-in instead of opt-out is fine, removal is not) 2. A terminal must remain a first-class way to run the update, not just a post-failure fallback. Please expose "run in terminal" as a normal, equally-weighted action alongside the background run e.g. a setting like 3. Correctness bug: the 30-minute stale-log timeout must not apply to terminal-driven runs. Given point 2 above, this needs a real fix, not just a wider timeout: please skip (or separately track) staleness detection for terminal-launched runs there's no way to distinguish "stuck" from "user is reading a diff" from log silence alone. 4. Missing dependency: Non-blocking, please take a look5. Progress bar/percentage undercounts when Flatpak has pending updates. 6. 7. Please explicitly confirm the polkit rule's scope is intentional. 8. Please attach some screenshots, as you said in the PR "Panel screenshots (idle with the Ignored section, the polkit hint, and a run in progress) will be attached in a comment below." Overall: solid engineering on the mechanics, but I'd like points 1–2 addressed as design requirements (not just my preference, losing "always-visible terminal" and "activity history" are real regressions for existing users), and point 3 is a genuine correctness bug once point 2 is addressed. Happy to re-review once these are in. |
… rollback, opt-in activity graph (2.4.0)
Review follow-up for the background-update PR:
- The terminal is a first-class way to run the update again: a new
update_mode setting ("terminal" | "background"), defaulting to the
terminal like the plugin always did; the background run is opt-in.
Terminal runs tee into the same log, so the live tail, progress and
history work in both modes.
- The activity graph is restored with its settings, data file and
rendering, gated on show_activity_graph (opt-in default), sitting above
the new history strip.
- The 30-minute stale-log guard no longer applies to terminal runs.
- resumeRunIfActive no longer races the startup auto-check.
- All shelled-out commands are listed in dependencies (install included).
- Flatpak's per-ref output now counts toward the progress bar.
- The polkit rule's system-wide scope is stated plainly in the tooltip and
README.
New since the previous revision of this PR:
- Update history strip with per-package and whole-run rollback from the
pacman/AUR caches (pactree-resolved same-run dependencies, cache and
reverse-dependency probing, never --nodeps).
- History entries are verified with pacman -Q before being recorded, so
packages declined in an interactive terminal run are not offered for
rollback.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@Reiling-Jeff thank you for the review. Every point was justified, and two of them (3 and 6) were real bugs. All of them are addressed in the new commit (version 2.4.0); the PR body has been updated accordingly. Point by point: 1. Activity graph — restored. 2. The terminal is a first-class update path again — and it is the default. A new 3. Stale-log timeout — fixed as described. The run's mode is tracked (and persisted in 4. Missing dependency — declared. 5. Flatpak progress — fixed. The progress count now also matches Flatpak's per-ref 6. Startup race — fixed. The startup auto-check now waits until 7. Polkit scope — explicitly confirmed as intentional. Yes: the rule grants 8. Screenshots — below: the idle panel with the Ignored section, the polkit hint, the activity graph and the history strip; a background run in progress; the panel right after an update.
Beyond the review, the commit also introduces the update history and rollback feature, described in detail in the updated PR body. Field-testing it on real system packages exposed one additional bug, fixed in the same commit: I will adjust any of the above on request, including reverting the graph's dot axis to the original invisible hover row. I do not speak English; this text was translated with AI assistance. |
|
This pull request was automatically closed because its description no longer contains Missing:
Please add the items listed above back to the description, keeping their exact wording, then |
|
Continued in #392. The template bot closed this pull request when I marked it ready for review — I had left the thumbnail checklist item unchecked. I fixed the description right away (the #392 carries the same commits from a fresh branch, plus one further commit fixing a locale bug, and it answers @Reiling-Jeff's review point by point with the screenshots attached. Apologies for the extra pull request. I do not speak English; this text was translated with AI assistance. |



Plugin
yuuto/arch-updaterplugin.toml)What it does
Three commits. The first is a standalone bugfix and can be cherry-picked on its own if the rest is not wanted; the second and third change the plugin's update flow, so they need @yuuto's sign-off per the repo rules. The third commit addresses @Reiling-Jeff's review of the first revision.
Commit 1 — fix the news timer (fixes #382), version 1.1.1.
The startup branch of the Arch news check (
sinceNewsCheck == AUTO_CHECK_DELAY) resets the counter to0, so it re-fires every 10 seconds forever. Each firing is an HTTP fetch plus RSS parsing in the service callback; the repeated work exceeds the 25 ms CPU budget and Noctalia's health watchdog auto-disables the whole plugin ~50 seconds after the shell starts whenevercheck_arch_newsis on (the default). The fix resets the counter back toAUTO_CHECK_DELAY, so news is checked once at startup and then every 6 hours, as intended.Commit 2 — background updates, ignore management, one polkit password, version 2.0.0.
Background update run. The run is spawned fully detached (double-fork + setsid):
pkexec pacman -Syufor repo packages, the AUR helper with--sudo pkexecplus non-interactive flags (--noconfirm --noprogressbar --color never, paru:--skipreview, yay:--answerdiff/--answerclean/--answeredit None), then optionallyflatpak update -y --noninteractive. All output goes to<data>/update.logbetween::START <epoch>/::EXIT <code>markers. The engine tails the log every 2 s: the panel swaps the package list for a live log tail (configurablelog_lines) with a progress bar, and the bar widget shows a percentage. Because the runner is detached it survives a shell restart — a fresh engine re-attaches to an unfinished log (younger than 6 h). On success: notification + automatic re-check; a failed run keeps its log on screen and offers Retry in terminal,tee'd into the same log.Ignore management. Packages held back by
pacman.conf'sIgnorePkgcome out ofcheckupdates/-Quawith an[ignored]suffix; previously they were counted and listed as pending even though the run skips them. They are now excluded from the count and shown in a new expandable Ignored section, together with the plugin-side ignore list. Each package row gets an ignore button; those entries persist in<data>/ignore.json(the panel cannot write plugin settings) and have a restore button. Entries from theignore_packagessetting and frompacman.confare tagged with where they are managed (the settings tag opens the plugin settings;pacman.confis never edited by the plugin). Newignore:NAME/unignore:NAMEIPC events.One polkit password per run.
pkexecauthenticates each pacman transaction separately, so a db sync plus a couple of AUR install batches meant three password dialogs. The panel shows a hint with an Ask once button until a keep-authorization rule is installed: one user-confirmedpkexec installcall placespolkit/49-arch-updater-pacman.rules(shipped in the plugin directory, also embedded inservice.luaubecause Lua cannot see the plugin dir) into/etc/polkit-1/rules.d/.rules.dis750 root:polkitdon Arch — not probeable by a regular user — so a marker file in the data dir remembers a successful install; the hint can also be hidden with thehide_polkit_hintsetting.Commit 3 — review follow-up + update history with rollback, version 2.4.0.
The terminal is a first-class update path again (review point 2). A new
update_modesetting: "terminal" (the default — the same interactive flow the plugin always had: prompts, conflicts and the PKGBUILD review work exactly as on the command line) or "background" (the opt-in non-interactive run from commit 2). Update follows the setting;update_background/update_terminalIPC events force one mode. Terminal runsteeinto the same log, so the live tail, progress bar, bar-widget percentage and the history all work in both modes. Net effect vs. upstream 1.1.0: the default behavior is unchanged, the background run is something you turn on.The activity graph is restored (review point 1).
show_activity_graphandactivity_history_lengthare back, along with thehistory_state.jsonread/write path (old-format data migrates) and the panel section — gated on the setting, opt-in default as suggested. One deviation from a 1:1 restore, submitted for the reviewer's consideration: the hover hit-zones under the graph used to be invisible ghost buttons; their geometry and behavior are unchanged, but they are now drawn as a row of small axis dots (the hovered dot is highlighted in the accent color). The invisible zones were difficult to discover, and the ghost buttons' hover flash was more prominent than the graph itself. This detail will be reverted on request.Stale-run guard fixed (review point 3). The 30-minute no-log-growth timeout now applies only to background runs. The run's mode is tracked and persisted in
run_meta.json, so the exemption survives a shell restart mid-run. A user reading a PKGBUILD diff for an hour no longer gets their run declared failed (and can no longer be offered a retry that would truncate the log under the live process).Missing dependency (review point 4).
installis declared, and while at it every other shelled-out helper too:date,grep,head,less,rm,tee,wc.Flatpak progress (review point 5). The progress count now also matches Flatpak's per-ref
Updating app/...lines, so a run with pending Flatpak updates can reach 100%.Startup race (review point 6). The startup auto-check now waits until
resumeRunIfActive's log probe has answered, so it can no longer steal the phase from a still-running detached update.Polkit scope disclosure (review point 7). The install button's tooltip and the README now state plainly that the rule covers any
pkexec-launched/usr/bin/pacmancall from an active localwheelsession for ~5 minutes after an authentication — not only this plugin's calls — and how to remove it.New: update history with rollback. A strip at the bottom of the panel shows one segment per recorded run (last 15, stored in
<data>/runs.json; rollbacks get their own segments). Click a segment to open the run's package list (name from → toper row): each row has a rollback button (second click confirms), the header rolls back the whole run in one transaction. Rollback installs the old package files straight from the caches (/var/cache/pacman/pkg, paru's/yay's build dirs) withpkexec pacman -U; dependencies updated in the same run ride along in the same transaction (resolved withpactreeagainst the run's package list, version constraints stripped).--nodepsis never used, so a downgrade that would break other packages makes pacman refuse the whole transaction before anything changes. Opening a run probes the caches and reverse dependencies: packages whose old file is gone are greyed out, and the tooltip warns how many installed packages require the one being rolled back. Flatpak entries are listed but not rollbackable. Before a run is recorded,pacman -Qverifies which packages actually changed, so anything declined during an interactive terminal run is not offered for rollback; failed runs are not recorded. An optionalrollback_auto_ignoresetting (off by default) adds rolled-back packages to the plugin ignore list.Also new:
run_meta.jsonpersists the current run's kind and package list, so a run resumed after a shell restart keeps its progress percentage and still lands in the history; Open full log opens the log in a terminal pager (less +G) instead ofxdg-open, which dies silently whentext/plainmaps to a terminal editor; the polkit hint only shows in background mode (terminal runs go throughsudoand never hit the multi-prompt problem); the retry-in-terminal button is not offered after a failed rollback (retrying the update command is not a retry of the rollback).External dependencies
All declared in
dependenciesinplugin.toml:pacman-contrib(checkupdates, pactree),pacman,pkexec(polkit escalation for the background run and rollback),sh,awk,sed,grep,tail,head,tee,wc,date,rm,install,less,test,uname, plus optionalparu/yay,flatpak,xdg-open, andsudo(terminal mode and fallback).Full accounting: network access is the same set of mirror/AUR/Flatpak contacts a manual upgrade makes, plus the Arch news feed fetch (startup + every 6 h). Files written are confined to the plugin data directory (
update.log,run_meta.json,runs.json,history_state.json,ignore.json,news_state.json, a staged copy of the polkit rule, an install marker) — with one exception,/etc/polkit-1/rules.d/49-arch-updater-pacman.rules, written only bypkexec installafter the user explicitly clicks Ask once and authenticates. Processes spawned are listed in the README's Notes section.Testing
Exercised on a real Arch system over several sessions: checks across pacman/AUR (paru)/Flatpak; full background runs (paru building and installing AUR packages, log tail + progress in the panel,
::EXIT 0, notification, auto re-check) with the polkit rule asking exactly one password; the[ignored]filtering against a realIgnorePkgentry; ignore/unignore via panel buttons and IPC (persistence verified); the polkit rule install and removal via the panel flow (rule active, marker recorded, hint gone; hint returns after removal); live per-package rollback of real system packages with a same-run dependency riding along (fzf+linux-api-headersvia glibc'spactreeclosure — including the version-constraint parsing fix this exposed), a whole-run rollback, and a roll-forward through the rollback's own history segment; history verification againstpacman -Qafter a run; the activity graph recording checks, surviving reloads and drawing the post-update drop; hot-reload and full plugin reload with no[ERR]lines; the news fix running for days withcheck_arch_newson and no watchdog trips.Screenshots / Videos
Attached in a comment below: idle panel with the Ignored section, the polkit hint, the activity graph and the history strip; a background run in progress with the live log; the panel after an update (graph showing the drop, history grown).
A note on language: I do not speak English; this text and the review conversation are translated with AI assistance. The changes themselves are tested on a real system as described above.
🤖 Generated with Claude Code
Checklist
idafter the/inplugin.tomlexactly.plugin.toml,README.md,thumbnail.webp, andtranslations/en.json.README.mdfollows theREADME template, documents
every entry id and dependency, and includes exact panel IPC commands and launcher prefixes where applicable.
thumbnail.webpwith the thumbnail generator.versionfollows semver and is bumped in this PR;plugin_apiis the oldest API level this plugin requires.understand that language well enough to review and maintain it (no unreviewed machine/LLM translations).
catalog.toml; CI generates it.Note on the thumbnail item:
thumbnail.webpis the plugin's existing generator-made thumbnail, shipped unchanged by this PR (the box is checked because the repository's template check requires every item checked on a non-draft PR). This PR adds no non-English translations; the existingde/fr/trfiles are untouched (their new keys fall back to English until the translation service catches up).Code review attestation
Plugins run as trusted, unsandboxed Luau in the user's session. Confirm:
licensedeclared inplugin.toml.