[jkubo]

Adds switches for Codex, Claude Code, Fireworks and Grok to the agents panel. The gear or s opens settings; j/k selects a provider, Enter or Space toggles it, and Escape returns or closes the panel. The bar entry stays reachable when every provider is off.

This PR is a draft while three confirmed review findings are corrected:

• A switch currently updates collection-visible settings before the asynchronous FileView write finishes. A failed write is not reliably reflected in the displayed or collection-visible value.
• A snapshot of a symlink preserves the link, but can leave its external target changed after restoration. Top-level linked settings, usage and cache paths, and nested links outside captured trees, need refusal before mutation. Ordinary parent-directory links and internal links to captured targets need to remain supported.
• Installed acceptance presses Space and takes a screenshot without asserting that the selected provider's stored enabled value changed.

The correction will confirm writes before publishing settings to collectors, serialize shared configuration mutations, and preserve unrelated queued settings on failure. Acceptance must assert the actual saved toggle and fail truthfully when the writer cannot save. These corrections and their new runtime checks are pending.

Existing validation at a527a0b covers the earlier ordinary-path implementation: 219 focused checks with zero skips and 532 real processes; additional writer and kernel-lock probes; and an installed Btrfs acceptance checkpoint with matching captured and restored state under the actual exclusive lease. All 2,116 tracked installed files matched that candidate tree on the official 4.0.4 test VM, using Qt 6.11.2 and Quickshell 0.3.1. Screenshots confirmed the account picker, four switches, Codex interaction and Escape. That evidence does not establish asynchronous write-failure handling, external symlink-target restoration, or an asserted saved-toggle transition.

A correction must also retain the newer upstream changes when rebased onto current quattro. Qt 6.12 provider behavior, other themes and other display scales remain unverified. The usage/cache gate coordinates cooperating writers sharing the same state context; unrelated writers and QML persistence remain outside that protocol. Its restoration checkpoint does not prevent legitimate later refreshes.

[llstrk]

Automated AI review

Community review: Independent automated community review, unaffiliated with the Omarchy team, intended to help prepare PRs for their review.

Verified: in a headless check of the head against the merge-base, the provider switches behave as the PR body describes. Four smaller points follow: a stale view after switching a provider back on, an inaccurate startup-gate comment, an overlap with open PR #11967, and a pre-existing writer limitation that the switches now expose.

Scenario (synthetic records and settings)	Observed at head
Flip claude off with providers: {claude: {enabled: true, note: "hand"}, zeta: {enabled: false}}	Writes claude: {enabled: false, note: "hand"}, keeps zeta; update command gets --except claude --except zeta
Same flip repeated	No writer call, no notification
Writer returns false, or no writer	Previous settings restored; one 'omarchy-notification-send' 'Couldn'\''t save provider settings' 'The switch was put back.'
Every provider with data switched off	Bar icon stays visible (base: hidden); panel opens on the switches at row 0
j/k, Enter, s, Esc (simulated through the key catcher's signals)	j/k clamp to the list, Enter flips the highlighted row, s opens the view at row 0; Esc returns to the dashboard, or closes the panel when everything is off
Only an all-zero leftover record	Icon stays hidden, with or without a providers map

From source review, the dashboard keys, provider switching and IPC paths are unchanged outside the switch view; of those, only Enter-to-refresh on the dashboard was exercised.

Switching a provider back on shows its old numbers until the next refresh

setProviderEnabled (and the Toggle and Enter paths that call it) never triggers a collection. While a provider is off, the updater skips it, but its old <id>.json record stays in the usage directory. Switching it back on makes enabledProviders show that record immediately, so the hero, limits, "Tokens by day" and model rows can show figures from whenever it was switched off.

Impact: stale usage is displayed as current until the next refresh: a manual one (r, or Enter on the dashboard), or the next scheduled one, which is up to refreshIntervalSec away (900 s by default). Limits alone also refresh on the next panel open.

Suggested change: after a successful enable, run a targeted collection such as usage.runUpdate("normal", [id]). updateCommand already passes positional ids, and the updater treats them as an allow-list.

Startup-gate comment does not match Bar.qml

The comment above settingsReady in Main.qml says injectProps delivers bar, moduleName and settings "a tick after it builds the widget". In shell/plugins/bar/Bar.qml the Loader's onLoaded calls slot.injectProps() synchronously, then again through Qt.callLater. Timer.triggeredOnStart fires from a posted event (QQmlTimerPrivate::maybeTick()), so on the built-in bar the pre-PR first refresh should already see the injected settings. A harness that mirrors Bar.qml's injection order confirmed this; the real bar was not loaded.

First refresh, synthetic host with `codex` switched off (headless, single run)
host injection            base                     head
sync in onLoaded (bar)    --except codex           --except codex
one tick later            no --except (scrapes)    --except codex
no bar at all             --except codex           --except codex after ~6 s

Impact: the gate is correct and does help hosts that inject a tick later. On the default bar it should change nothing (no path where it does was found), and a host without a bar now waits about 6 s for its first collection. The comment will mislead anyone maintaining the gate.

Suggested change: reword the comment to describe the case the gate actually covers. If a specific host showed the start-up scrape, naming it in the comment would help.

Overlap with open PR #11967

Open PR #11967 ("Agents: user collectors + Go connection settings card") also adds a gear through trailingControl on the same PanelHero in shell/plugins/agents/Panel.qml. A textual three-way merge of the two branches produces no conflict markers, but it leaves two trailingControl: Component { ... } bindings on one object. Loading the merged Panel.qml headlessly fails with Panel.qml:791:13: Property value set multiple times.

Impact: if both changes are combined as they stand, the agents panel fails to load, and agents-panel-test.sh still passes on that merged source, so it would not catch this.

Suggested change: coordinate with #11967 on a single hero control, for example one container holding both gears.

Optional (pre-existing): updateEntryInline in shell/shell.qml matches only object layout entries. A bare string entry such as "omarchy.agents", which is what the rename migration migrations/1786099804.sh produces from a string entry, makes every flip return false. The PR then reverts correctly and shows "Couldn't save provider settings" with no reason. Promoting a matching string entry to {id} in the shared writer would fix this for every inline writer.

Optional test improvement: agents-panel-test.sh and qml-text-format-test.sh, which the PR body cites, pass identically on the merge-base and head. The six assertions in agents-panel-test.sh only match the bar button's click wiring, so an empty setProviderEnabled would still pass them. A small test for the merge and no-op rules, the allProvidersOff visibility rule and the resulting --except arguments would catch that.

---

Review information

Test scope: Source review of head ef7ff3e against merge-base 47de632 and the current quattro tip (whose only agents-plugin change, IpcHandler to ShellIpc, does not interact). Headless Quickshell harness loading the real Panel.qml and Main.qml with a synthetic bar, synthetic usage records, a stub updater and a stub settings writer. Keyboard input was simulated through the panel key catcher's signals. The keyboard panel window was replaced by a same-API stub, so window placement, focus, rendering, hover styling and scrolling of a long switch list were not exercised. No desktop session and no screenshots. updateEntryInline was run extracted from shell/shell.qml against three shell.json shapes. The #11967 overlap was checked by a textual three-way merge of Panel.qml and a headless load of the merged file.

AI process: Opus 5.5 Medium coordination and synthesis, Opus 5.5 Xhigh technical review and final fact check, GPT 6 Sol Xhigh search for related issues, Opus 5.5 Medium editorial check.

Opt out: To stop receiving these reviews, reply to this comment saying so.

[jkubo]

Rechecked the provider implementation at ef7ff3e8897bfed86e1d5f140101bc7f2a588256 against quattro d9b970dd626da8fdf8174dada67c5e564664e2f8. A local diagnostic executes the actual QML JavaScript functions with stubbed persistence and collector processes, without changing desktop configuration:

• A successful provider re-enable starts zero fresh collections.
• A limits-only request followed by two targeted normal refreshes replays only omarchy-agent-usage-update --limits-only, dropping the requested provider IDs.
• The inline writer refuses a matching string layout entry. It does update both matching object entries, so the defect is string-entry handling rather than an only-last-widget write.

The repair needs a fresh normal collection after a saved re-enable, preserved/merged queued IDs with normal work taking priority over limits-only work, and string-entry promotion in the writer. The startup comment also needs to match the bar's synchronous injection followed by its deferred pass.

Upstream integration requires composing the existing account controls and provider settings into one hero control and retaining the upstream keyboard/refresh behavior. A mechanical rebase alone does not fix these defects.

This diagnostic verifies JavaScript behavior only. It does not establish QML load correctness, timer timing, or visual acceptance; the updated code still needs those checks and the requested screenshot before the PR is ready.

[greptile-apps]

Confidence Score: 3/5

[High risk] Adds provider on-off switching to the agents panel.

Fix failed-save feedback and restoration of linked directories before merging.

Findings

1.  Unsaved switches look successful ▶
2.  Linked directories stay changed ▶
3.  Broken switches pass acceptance ▶

Summary

Adds provider switches to the agents panel, targeted refreshes when providers are enabled, and a queue that keeps refresh scope and force behavior.

• Adds a shared gate for usage publishers and direct cache collectors.
• Adds acceptance capture, restoration, and checkpoint manifests.
• Settings write failures are not reflected by the switches.
• Linked usage/cache directories are not fully restored.
• Acceptance does not assert that toggling saved a change.

jkubo explicitly limits restoration to the checkpoint under the gate. Later normal refreshes, unrelated writers, and QML settings persistence are outside that guarantee.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A["User toggles provider"] --> B["Update displayed settings"]
  B --> C["updateEntryInline"]
  C --> D["Write shell.json"]
  C --> E["Return whether config changed"]
  E --> F{"Accepted?"}
  F -->|No| G["Restore displayed settings"]
  F -->|Yes, enabling| H["Targeted normal refresh"]
  H --> I["Queue while updater is busy"]
  I --> J["Updater and collectors hold shared gate"]
  K["Acceptance capture and restore"] --> L["Hold exclusive gate"]
  L --> M["Copy and verify saved paths"]
  M --> N["Write checkpoint manifests"]
  N --> O["Release gate"]

Loading

_{Reviews (1) · Last reviewed commit: "Add provider switches to the agents pane..." · Reviewed by Greptile}

[greptile-apps]

Unsaved switches look successful

If shell.json cannot be written, setProviderEnabled() still treats updateEntryInline() returning true as a successful save. That return value only means the config changed; persistShellConfig() updates memory and calls setText() without checking whether the write succeeded. The switch stays flipped, no failure notice appears, and enabling still starts collection. The setting disappears after restarting the shell. Keep the previous settings until the write succeeds, and restore them when it fails.

[greptile-apps]

Linked directories stay changed

If the usage or cache directory is a symlink, agents_provider_capture_path() saves only the link with cp -a, not the contents it points to. Collectors write through that link during acceptance. Cleanup restores the same link and checks only its target string, so it reports success and deletes the backup while the target still contains the test's changes. Capture and restore the target separately, or refuse these top-level symlinks before changing the session.

[greptile-apps]

Broken switches pass acceptance

The acceptance test presses Space and takes a screenshot, but never checks that a provider changed or that shell.json saved the change. The PROVIDERS header check proves only that the page opened. A missing or refusing writer can therefore pass the test without a working switch. Assert that the selected provider's saved enabled value changed before running cleanup.