Skip to content

Remove agent usage records left behind when their collector goes away - #10911

Open
Daniluvatar wants to merge 1 commit into
omacom:quattrofrom
Daniluvatar:remove-orphan-agent-usage-records
Open

Daniluvatar wants to merge 1 commit into
omacom:quattrofrom
Daniluvatar:remove-orphan-agent-usage-records

Conversation

@Daniluvatar

Copy link
Copy Markdown

Problema

O painel de agentes lista todos os arquivos .json do diretório de uso, sem checar se ainda existe coletor para aquele agente. Quando um coletor vai embora (plugin desinstalado, agente renomeado, etc.), o registro antigo continua aparecendo no painel — como um fantasma — para sempre.

É o que aconteceu, por exemplo, com antigos plugins que escreviam copilot.json: mesmo depois da remediação do plugin, a linha "copilot" continuava visível no bar.

Correção (em duas camadas)

  1. Migração (migrations/1788919234.sh) — limpa os órfãos já existentes em instalações atuais. Como migrações rodam na omarchy update/login, todos os usuários passam por ela uma vez.

  2. GC no orquestrador (bin/omarchy-agent-usage-update) — na primeira coleta de cada dia, apaga registros órfãos, previne o acúmulo futuro e também cobre quem está atrasado nas migrações.

Regra de segurança

Um registro só é apagado quando ambas condições valem:

  • nenhum coletor instalado produz esse agente (procura em bin/ padrão, plugins/*/scripts/ e plugins/*/bin/, e um wrapper do próprio orquestrador não conta como coletor), e
  • o arquivo não foi gravado nos últimos 7 dias.

Agentes que ainda estão ativos — inclusive plugins que escrevem o JSON sem coletor próprio — não são tocados nunca: sua gravação recente os protege.

Testes

  • 6 novos testes de GC em test/shell.d/agent-usage-update-test.sh (mantém agente vivo, preserva registro recente de coletor ausente, apaga órfão antigo, não conta wrapper como coletor, preserva coletor de plugin);
  • novo test/shell.d/agent-usage-orphan-migration-test.sh com 6 testes (remota órfão, não remota agente vivo, não conta wrapper, preserva registro recente, reporta o que remove, idempotência);
  • ./test/all roda limpo para todos os testes relacionados (as 5 falhas restantes em shell.d — config, locate, runtime-smoke, snapper, unowned-system-paths — falham idênticas na árvore limpa, por falta de checkout de omarchy-pkgs/ambiente, não por esta mudança).

The agents panel lists every file in the usage directory, so a record
written by a collector that is no longer installed keeps showing up
forever even after the plugin is removed.

The orchestrator now prunes such records on every run, and a migration
clears the ones already left behind. A record only disappears when no
installed collector produces it and it has not been written to for seven
days, so plugins that write fresh data without a collector of their own
are not disturbed.
@omarchybot

Copy link
Copy Markdown
Collaborator

Reviewed at 897b36e by Claude Opus 5.5, with a second opinion from Codex Medium.

The bug is real and the change fixes it. On an Omarchy ISO worker, a 30-day-old copilot.json with no collector survives omarchy-agent-usage-update on quattro (821ae58), is removed on this head and on a local merge of this branch with quattro, and survives again with the change reverted. The migration removes it too. agent-usage-update-test.sh (13) and agent-usage-orphan-migration-test.sh (6) pass on the worker, both on the head and on the merge, as do the agent account tests on the merge. ./test/cli passes on the head; on the merge its one failure (vscode generated theme references current theme file) fails identically on quattro alone.

What needs a decision before this can go further:

  • It deletes records that are not stale. shell/plugins/agents/README.md on quattro says the panel "picks up any record that lands in the directory regardless of who wrote it". remove_orphans (bin/omarchy-agent-usage-update:83) deletes any such record not written in 7 days, so a writer that is still installed but only writes when its tool is used loses its record after a quiet week. On the worker, an 8-day-old thirdparty.json was deleted along with copilot.json. Disabled providers are passed as --except, and the cleanup ignores that too. Whether records outside bin/ may be garbage-collected at all is a decision about that contract, so this waits on the maintainer.
  • The plugin paths are not a collector mechanism. Nothing on quattro runs ~/.config/omarchy/plugins/*/scripts/ or */bin/omarchy-agent-usage-*, and the check uses -f, not -x. So a leftover file there protects an abandoned record forever, and a working writer with any other name gets no protection.
  • The migration is redundant. The panel runs omarchy-agent-usage-update every 900 seconds, and migration 1786099804 runs it once already, so migrations/1788919234.sh repeats work the next refresh does anyway. A proportionate version is the cleanup in the updater alone, or none at all if the contract stays as written.
  • grep -qx "$agent" (bin/omarchy-agent-usage-update:95) matches the record name as a regex, so an orphan named clau.e.json survives because claude is installed. grep -Fxq -- is the literal match. Codex found this, and I confirmed it.
  • The branch conflicts with quattro in bin/omarchy-agent-usage-update, where quattro added an account refresh before the collectors and an autoswitch loop after them. Both sides keep their code, with remove_orphans after the autoswitch loop. Because the branch doesn't merge cleanly, verified could not be applied.

Codex agreed on the contract and proportionality points. Its independence is not guaranteed, since it can read this session. Its regex finding was new. It also raised a race in which a writer replaces an old record between the stat and the rm, so the fresh record gets deleted. That is real but narrow.

Nothing was pushed to the branch. Next step: the maintainer decides whether records outside a collector may be removed. After that, the author needs to resolve the conflict and update the change to match.

@jandrusk

Copy link
Copy Markdown

Sensible orphan cleanup — the 7-day mtime guard protects writers that don't use the collector naming scheme. CONFLICTING; rebase and skim against the other open agent-usage work (#12353 collectors dir, #14096 missing collectors, #13198 freshness).

@omarchybot

Copy link
Copy Markdown
Collaborator

Re-checked at 897b36e against current quattro by Claude Opus 5.5, with a second opinion from Codex Medium. Nothing was run this time: the head has not moved, and the worker results in the earlier comment still apply.

Nothing on quattro since the last review fixes orphaned records or changes the contract. bin/omarchy-agent-usage-update is unchanged, and shell/plugins/agents/README.md still says the panel picks up any record in the directory regardless of who wrote it. The earlier findings stand, including the conflict in bin/omarchy-agent-usage-update.

On the 7-day guard: it protects a writer that writes often, not one that is installed. A hook that only writes custom.json when its tool is used, and then sits unused for eight days, has its record deleted by remove_orphans (bin/omarchy-agent-usage-update:89-98) and by the migration, and the record stays gone until the tool runs again. This is the thirdparty.json case the earlier worker run showed.

One interaction is new. #12353 discovers collectors in ~/.config/omarchy/agents/collectors. If it lands, the collector list here (bin/omarchy-agent-usage-update:58-74, migrations/1788919234.sh:17-31) has to include that directory too. Otherwise the record of a collector installed there is deleted whenever that collector has not refreshed it for seven days. Codex raised this and I confirmed that neither file looks there. #14096 and #13198 touch the same area but do not change this.

This still waits on the maintainer to decide whether records not written by a collector in bin/ may be removed at all. After that, the author needs to resolve the conflict and adjust the change to match.

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

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants