Skip to content

fix(flatpak): correct metered check in appstream refresh service - #1035

Open
HuntedRaven7 wants to merge 1 commit into
projectbluefin:mainfrom
HuntedRaven7:issue-1027
Open

fix(flatpak): correct metered check in appstream refresh service#1035
HuntedRaven7 wants to merge 1 commit into
projectbluefin:mainfrom
HuntedRaven7:issue-1027

Conversation

@HuntedRaven7

Copy link
Copy Markdown
Contributor

ExecCondition skipped NM_METERED_YES(1) and NM_METERED_NO(2), so an explicitly-unmetered connection 2 was never refreshed while a guessed-metered connection 3 was. NetworkManager's NMMetered docs say to treat NM_METERED_GUESS_YES(3) as metered and all else as unmetered.

Skip only 1 and 3; keep 0/unknown fail-open. Moved to a case statement for clearer intent and to avoid future numeric miscomparisons.

Closes #1027

Assisted-by: big-pickle (opencode) via OpenCode

ExecCondition skipped NM_METERED_YES(1) and NM_METERED_NO(2), so an
explicitly-unmetered connection 2 was never refreshed while a
guessed-metered connection 3 was. NetworkManager's NMMetered docs say to
treat NM_METERED_GUESS_YES(3) as metered and all else as unmetered.

Skip only 1 and 3; keep 0/unknown fail-open. Moved to a case statement
for clearer intent and to avoid future numeric miscomparisons.

Closes projectbluefin#1027

Assisted-by: big-pickle (opencode) via GitHub Copilot
Co-authored-by: big-pickle <big-pickle@opencode.ai>

@hanthor hanthor left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct fix, and the bug was real: the old condition [ "$v" != "1" ] && [ "$v" != "2" ] skipped the refresh for both NM_METERED_YES(1) and NM_METERED_NO(2) — meaning an explicitly-unmetered connection was being denied a refresh it should always get, while NM_METERED_GUESS_YES(3) (which NetworkManager's own docs say to treat as metered) fell through and refreshed anyway. The new case "$v" in 1|3) exit 1;; esac skips only on 1 and 3 and fail-opens (runs the refresh) for everything else, including unknown/empty $v on a busctl failure — matches NM's documented semantics exactly. CI green (test, Compose PR test image, validate, both arch builds).


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

flatpak-appstream-refresh.service checks the wrong NetworkManager metered values

2 participants