Skip to content

Give plugins the current bar configuration after shell.json changes - #11661

Open
rgouveiamendes wants to merge 1 commit into
omacom:quattrofrom
rgouveiamendes:fix-plugin-bar-config-snapshot
Open

rgouveiamendes wants to merge 1 commit into
omacom:quattrofrom
rgouveiamendes:fix-plugin-bar-config-snapshot

Conversation

@rgouveiamendes

@rgouveiamendes rgouveiamendes commented Sep 13, 2026 •

Copy link
Copy Markdown

Problem

Third-party plugins read the bar section of shell.json through their shell facade's barConfig.
After shell.json changes, that snapshot holds the configuration from before the change.

syncPluginApis() refreshes each facade's barConfig from publicBarConfig(), and it runs from onShellConfigChanged (via pluginsChanged).
publicBarConfig() copies the barConfig binding, which has not yet re-evaluated when that handler runs.
So each change delivers the previous configuration, and a plugin that writes a setting through updateEntryInline and reads it back sees the old value until the next change.
A plugin that merges its current settings before writing also reverts the setting it changed last.

Minimal reproduction of the ordering in Quickshell:

property var shellConfig: ({ bar: { n: 0 } })
onShellConfigChanged: seen.push({ binding: barConfig.n, direct: shellConfig.bar.n })
readonly property var barConfig: shellConfig.bar
// shellConfig = { bar: { n: 1 } }; shellConfig = { bar: { n: 2 } }
// seen: [{binding:0,direct:1}, {binding:1,direct:2}]

Change

publicBarConfig() copies shellConfig.bar directly (falling back to builtinShellConfig.bar, as the binding does).
publicIdleConfigFor() already reads shellConfig this way.
A static assertion in plugin-auth-boundary-test.sh guards against reading the binding again.

Testing

  • test/shell.d/plugin-auth-boundary-test.sh passes; the new assertion fails without the change.
  • ./test/all: the remaining failures (bar-icon-geometry, config, snapper, unowned-system-paths) also fail on quattro at 93d9d0c on this machine; three need an omarchy-pkgs checkout.
  • On Omarchy 4.0.3 with this patch applied to a copy of the installed shell, a third-party service reading barConfig saw each updateEntryInline write on its next read, including writes 1 s apart. Without the patch it saw the previous write. Plugin used: boringday.wallpapers.

🤖 Generated with Claude Code

https://claude.ai/code/session_01XZQ9FGgM8QFtDgpoQfcdtv

Fixes #11505

Fixes #11852

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XZQ9FGgM8QFtDgpoQfcdtv
@csfh

csfh commented Sep 13, 2026

Copy link
Copy Markdown
Member

This seems reasonable. The only thing I'm wondering about is if it breaks any existing plug-ins and how do we find out or deal with that?

@rgouveiamendes

Copy link
Copy Markdown
Author

It changes what plugins receive only while a shell.json change is being handled.

publicBarConfig() now copies the same expression the barConfig binding evaluates (shellConfig.bar, else builtinShellConfig.bar). Wherever the binding is already up to date, the snapshot is identical to before: facade creation at plugin load, barConfigFor() from onBarConfigChanged / configureBar, and syncs triggered by the widget registry. The difference is in syncPluginApis() when it runs from onShellConfigChanged (via pluginsChanged), and in facades first created by _syncServices() in that same pass: those now get the configuration after the change instead of the one before it. Before the change, a plugin kept that previous snapshot until the next sync; boringday.wallpapers still read the previous value 3 s after writing a setting.

So a plugin behaves differently only if it relied on receiving the previous configuration after a change.

To check existing plugins, I searched GitHub code (default branches that GitHub indexes, so not every plugin) for third-party use of barConfig and updateEntryInline:

Each reads the current value; none compares old and new snapshots. I have only run boringday.wallpapers against the patch, not the other two.

@csfh csfh added bug Something isn't working verified Omarchy Triage has verified that this issue is ready for final review labels Sep 14, 2026
@omarchybot omarchybot added the ready Good to merge label Oct 1, 2026
@omarchybot

Copy link
Copy Markdown
Collaborator

Reviewed at 5600622. The fix is correct, I reproduced the bug it fixes, and I found nothing to change.

Reproduction, on a disposable Omarchy VM running the real shell. I installed a small third-party service plugin that records shell.barConfig each time it changes. Then I wrote a bar setting three times from outside the shell (1, 2, 3) and changed the clock format three times through omarchy bar set (A, B, C), which writes shell.json from inside the shell:

  • On current quattro (8b4eae6), after each outside write the service stays one change behind: it sees nothing, then 1, then 2. After each write from inside the shell, its first update still carries the previous format, and it only catches up on a later sync.
  • At this head, the first update after every write carries the new value, on both paths.
  • At this head with only shell/shell.qml reverted, it lags again exactly as on quattro.

The new expression in publicBarConfig() matches the barConfig binding in every case: a missing config, a bar that is not an object, and the fallback to builtinShellConfig.bar. It still returns a detached copy. Both facade maps in syncPluginApis(), newly created facades and third-party barConfigFor() all go through it.

On whether it breaks existing plugins: plugins receive the same shape as before, through the same triggers. Only the value delivered while a shell.json change is being handled is different, and it is now the current one. I found no reader in the shell that depends on getting the previous configuration; I did not run the third-party plugins named above.

Tests, on the same VM. ./test/cli passes. ./test/shell passes except "Hi is 18 columns wide" in ascii-test.sh and "a logo of plain block art animates" in branding-about-animation-test.sh, which fail the same way on quattro. The new assertion in plugin-auth-boundary-test.sh fails with the fix reverted and passes with it. It checks the function's source text, not its behaviour, so it would not catch, say, a version that returns the host's own object. As a regression guard, that is proportionate to a two-line fix.

Second opinion: Codex at medium effort reviewed the diff and found no production defect. It agreed the change is the smallest correct one and that no other reader delivers a stale value; independence from my own reasoning is not guaranteed. The point that the test checks text rather than behaviour is its finding.

Related: this fixes #11505 and #11852, now linked so they close when it merges. #11578 makes the same change to publicBarConfig() with a much larger test. #11888 fixes the same cause but also adds a second syncPluginApis() call and changes how updateEntryInline() handles an entry it cannot find. Asked separately which of the three to prefer, Codex chose this one.

Nothing is left for you to do. Whether it lands is the maintainer's decision.

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

Labels

bug Something isn't working ready Good to merge verified Omarchy Triage has verified that this issue is ready for final review

Projects

None yet

3 participants