feat(agent): refresh the inventory when its source files change

[jagansanikommu]

The interval from #75 bounds how stale the inventory can get. This bounds how
long a change takes to appear: adding an MCP server no longer means waiting
out inventoryInterval before the fleet view reflects it.

Refs #62. I left that issue open after #75 and asked whether you wanted this as
a follow-up; this is that follow-up, so close #62 with it if the pair is what
you had in mind.

Approach

After every published snapshot the daemon watches the directories holding the
files that snapshot came from — the parent of each discovered MCP configuration
and each discovered skill file — and rescans when one reports activity. It
reuses the debounced-watch approach already in controller/src/daemon_config.rs,
including the same 250ms window.

Deriving the watch set from the inventory rather than from a hardcoded path list
means no provider code is touched. A new provider is followed automatically
by virtue of reporting source and path in its discovery. That also keeps
this clear of #94, which rewrites every provider's discovery.rs.

Parent directories rather than the files themselves, so an editor replacing a
file atomically and a sibling file appearing beside a discovered one both
register. Events are not inspected past debouncing: a scan is the only way to
know whether the inventory really changed, and the existing publish-on-change
comparison already discards a scan that matches the current snapshot, so a
spurious wake costs one discovery walk and nothing downstream.

Executable directories are deliberately not watched — an upgrade changes a
version rather than a configuration, and install directories are noisy enough to
make most of those scans wasted. The interval still catches upgrades.

What this does not catch

A tool that had no discovered files at the last scan contributes nothing to
the watch set, so a brand-new install is still picked up by the interval rather
than immediately. Watching for that would mean watching speculative paths for
every supported tool on every device, which seemed a bad trade against a 15m
default. Happy to revisit if you disagree.

Update after review

Two real findings, both mine, both fixed.

Unbounded change signals. Every debounced batch was queued, so a burst ran
one full discovery per token. The description above originally argued a spurious
wake "costs one discovery walk and nothing downstream" — true for correctness,
false for amplification. The signal channel now has capacity one and is sent
with try_send, so activity arriving while a signal is pending is dropped
rather than queued; a scan answers every change that preceded it.
InventoryWatch now owns that channel instead of accepting a sender, so the
guarantee cannot be widened by a caller.

Sibling skills were missed. Watching each SKILL.md's own directory missed
skills added elsewhere in the tree — and on a real machine Codex nests them two
levels below the root (.codex/skills/.system/review-agent/SKILL.md), so
neither the root nor the intermediate directory was watched. A discovered skill
now contributes the root its walk started from, watched recursively. MCP
configurations still contribute their own directory shallowly, since $HOME
recursively would be indefensible.

Recovering that root walks up to the nearest ancestor named skills, which is
the shape of every root the providers scan. Retaining the providers' own roots
would be the better fix, but it means editing every discovery.rs and #94 is
still an open draft rewriting exactly those files. Happy to switch once that
lands.

Consequences worth flagging

Claude Code keeps .claude.json directly in the home directory, so the home
directory is in the watch set, non-recursively. Direct children of $HOME
therefore wake a rescan — now coalesced, so a burst costs one scan rather than
one per event. If you would rather not watch $HOME at all, the cost is that
edits to .claude.json, the main Claude Code MCP surface, stop being noticed.

A tool with no discovered files contributes nothing to the watch set, so a
brand-new install is still picked up by the interval rather than immediately.
Watching speculative paths for every supported tool on every device seemed a bad
trade against a 15m default.

Validation

make check, make generate-schema (no diff), and the workspace suite —
145 tests, nine new. Coverage includes the skill root being watched rather
than the leaf, the fallback when a skill sits outside a skills root, the watch
set re-syncing as tools gain and lose configuration, a real write reported
through the debouncer, a burst of writes producing at most one pending signal,
and a change signal refreshing without waiting for the interval. The filesystem
tests use generous timeouts because macOS delivers through FSEvents, which
batches with its own latency on top of the debounce window.

Rebased onto current main.

Not run: the provider integration suite. It requires a Linux-built daemon
(ensure!(cfg!(target_os = "linux")) in tests/common/container.rs) and I am
on macOS. Nothing here touches provider discovery, but flagging it rather than
implying otherwise.

notify and notify-debouncer-full were already workspace dependencies; adding
them to agentdesktop-agent is the only Cargo.lock change.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Copilot review overview

🟡 Changes recommended

Sibling skill additions can be missed, and unbounded change signals can create an indefinite scan backlog.

Review effort: Balanced
Findings: 1 · 1

Open (2)

• Coalesce refresh signals to prevent unbounded scan backlog · New
• Watch discovery roots to detect newly added sibling skills · New

[peterj]

@copilot resolve the merge conflicts in this pull request

[jagansanikommu]

Taking this one — rebased onto current main and pushed, so @copilot can stand down.

The conflicts were only import collisions in lib.rs and daemon.rs, between llm_proxy from #134 and inventory_watch here. Nothing semantic: refresh_inventory_with is untouched by anything that landed, and the reconcile tick added in #146 is a separate interval that does not overlap with the inventory one.

Kept as a single commit rather than resolving in the web editor, which would have added a merge commit. 341 tests pass locally against the rebase.

Sorry for letting it drift to the point of needing a nudge.

[jagansanikommu]

Test and lint failed on the rebase, but I think the source is fine and the job compiled against stale artifacts. Evidence, in case it is useful beyond this PR.

Every unresolved symbol is something #146 added, and all of them live in crates this PR does not touch:

unresolved import `agentdesktop_core::config::ProxyUnavailable`
no field `reconcile_interval` on type `DaemonStartupConfig`
no field `when_proxy_unavailable` on type `&LlmGatewayConfig`
unresolved imports `agentdesktop_proto::fleet::ProgramState`, `ProgramStatus`
struct `ConfigStatus` has no field named `programs` / `programs_reported`

So agentdesktop-agent was built post-#146 against pre-#146 agentdesktop-core and agentdesktop-proto.

I checked out the exact merge commit the job tested — b76ddba, merging 50f399e into ff46fc4 — and cargo test --workspace --no-run completes cleanly on it locally. ff46fc4 is an ancestor of the branch, and all the symbols above are present in it.

As for why: .github/actions/rust-build-cache sets cache-workspace-crates: true and restores source mtimes so cached workspace artifacts stay valid across a fresh checkout, which is what makes reusing a stale core/proto rlib possible at all. My guess at the trigger is that this PR changes Cargo.lock (it adds notify to the agent crate), so its cache key misses and falls back through restore-keys to an older entry — and the entries under shared-key: ci around #146 were written by several main runs finishing within about two minutes of each other, where the first writer wins.

That is inference rather than something I can prove from outside, and the falsifiable form is: any PR that touches Cargo.lock soon after a core or proto change should hit the same thing.

I cannot re-run the job myself (needs admin). Could someone re-run it, ideally after clearing the ci cache entry? Happy to be shown I am wrong about the cause — but the merge commit does build, so I do not think there is anything to fix in the branch.

[jagansanikommu]

The re-run reproduced it identically, and its log confirms the cause rather than just repeating the symptom:

Cache hit for restore-key: v0-rust-ci-Linux-x64-7f7ca0a9-d7bf88ff
Restored from cache key "v0-rust-ci-Linux-x64-7f7ca0a9-d7bf88ff" full match: false.

full match: false is the fallback I guessed at. This PR changes Cargo.lock, so the exact key misses and restore-keys lands on an older entry — one whose agentdesktop-core and agentdesktop-proto artifacts predate #146. With cache-workspace-crates: true and restored source mtimes, cargo treats those as fresh and does not rebuild them, so agentdesktop-agent compiles against a core and proto that do not have #146 in them.

The Container job is the control. It builds in Docker with no cache, compiles agentdesktop-proto and agentdesktop-core from source, and passes. The only job that fails is the one that restored a partial cache.

So a plain re-run will not clear this — it restores the same entry. The ci cache would need evicting, or the key would need to cover workspace source state.

The part worth more than this PR: the failure direction here is the lucky one. The same mechanism can also make a PR pass against stale workspace artifacts — green on a core or proto contract it was never actually compiled against, then broken once it lands on main and the cache is cold. Caching workspace crates buys build minutes at the cost of CI sometimes answering a different question than the one asked. Setting cache-workspace-crates: false, or folding the workspace source hash into the key, would close it.

Happy to open a separate issue for that if you would like it tracked away from this PR.