Skip to content

Report missing requested agent usage collectors - #14096

Open
sccashma-hermes wants to merge 1 commit into
omacom:quattrofrom
sccashma-hermes:fix/missing-agent-usage-collector
Open

sccashma-hermes wants to merge 1 commit into
omacom:quattrofrom
sccashma-hermes:fix/missing-agent-usage-collector

Conversation

@sccashma-hermes

Copy link
Copy Markdown

When a usage record has been seeded manually but its collector is not installed, omarchy agent usage-update hermes currently exits successfully without refreshing anything. The panel can continue displaying that old snapshot indefinitely, and the explicit refresh gives no indication that it did no work.

Track which collectors were actually dispatched and return a nonzero status with a diagnostic for each requested, non-excluded collector that was not found or executable. Available collectors still run when another requested collector is missing, and existing snapshots remain untouched. Automatic discovery with no agent arguments is unchanged.

This follow-up was prompted by #13505: that PR is closed in favor of the still-open #14039, while the installed package on the affected machine did not yet contain a Hermes collector. The actual local refresh was restored with a user timer running the collector from #14039. This PR fixes the misleading success result; it does not duplicate the pending Hermes collector or add a timer to Omarchy.

Validation:

  • Added regression reproduces the false success before the fix and passes afterward.
  • test/shell.d/agent-usage-update-test.sh: missing and non-executable collectors, mixed available/missing selection, preservation of the old record, the updater's reserved name, and --except precedence pass.
  • test/shell.d/agent-usage-accounts-test.sh and test/shell.d/agent-account-autoswitch-test.sh pass.
  • Bash syntax and git diff --check pass.
  • timeout 180 ./test/all did not complete within three minutes. Before timeout it reported failures outside this change: VS Code theme symlink, missing companion omarchy-pkgs checkout, Elsewhen old-cache cleanup, kernel-header selection, and browser-window focus. The focused updater and account tests above passed; the full suite is not claimed green.

@greptile-apps

greptile-apps Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium risk] Agent usage collector adds error reporting for missing collectors.

This PR appears safe to merge.

What we checked:

  • Automatic discovery stays unchanged: With no agent arguments, only is empty. The new check has nothing to inspect, so discovery still runs available collectors as before.
  • Available collectors still finish: The missing-collector check sets status but does not exit. The updater still waits for every launched collector.

Summary

omarchy-agent-usage-update now reports failure when a requested, non-excluded collector is missing or not executable.

  • Available collectors still run and finish.
  • Calls without agent arguments keep their existing behavior.
  • Added tests cover missing collectors, preserved records, mixed selections, non-executable collectors, the reserved name, and exclusions.
  • No actionable issues found. Review was based on code inspection; tests were not run.

Reviews (1) · Last reviewed commit: "Report missing requested agent usage col..."

@chessing234 chessing234 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

everything seems fine, ready to merge.

@omarchybot

Copy link
Copy Markdown
Collaborator

Reviewed at e48fe8e, and the fix holds. I reproduced it on a worker built from the Omarchy ISO. On the base, omarchy-agent-usage-update typoed exits 0 having run nothing. On this head it exits 1 and prints typoed collector not found or not executable. With the fix reverted, the new test in test/shell.d/agent-usage-update-test.sh fails. With the fix in place, all 11 of its checks pass, both on this head and with the branch merged onto current quattro. The change is the size the problem needs. Discovery with no agent names is unchanged. None of the callers sees a new failure in normal use: the panel refreshes without agent names and only logs stderr, and the account commands pass a provider that has a collector and discard the output.

Codex Medium reviewed the diff, every caller, and what has landed on quattro since this branch forked. It found no defects. Its one note, which I agree with, is minor: an empty agent name (usage-update "") now gets a second bad array subscript message. That path already failed the same way before this change. Codex can read this review's working notes, so its agreement is not guaranteed to be independent.

test/cli and test/shell are not fully green on the worker. The same unrelated failures (a VS Code theme test, the logo animation and column tests, kernel headers, a few fixture tests) appear without this change. Current quattro with the change merged has its own unrelated failures. Nothing is waiting on you: this is with the maintainer.

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

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants