fix(run): bound catalog watches to declaration space - #316
Closed
schickling wants to merge 2 commits into
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The supervisor registers one recursive filesystem watch over the whole catalog.
notifyeagerly traverses Resource worktrees before callback filtering, so large payloads can delay the first reconcile, allocate unbounded inotify watches, and silently fall back to timer-only polling if registration fails.The fallback also has a cadence bug: when watcher installation fails, its sender is dropped and the receiver reports
Disconnected. The old wait loop treated that as an immediate wake, so it could continuously reconcile instead of polling on the configured interval.Goal
Bound watcher installation to the declaration namespace while retaining immediate declaration create/update/remove detection and a real timer fallback that cannot spin when watcher installation fails.
Decisions
agent_spec::is_catalog_pathsemantics stop traversal before Resource, inbox, archive, status, PTY, and control trees.None.Callback-only filtering was rejected because it runs after recursive registration has already traversed the payload. A single shallow root watch was rejected because later edits in newly nested declaration directories would not wake immediately. Restarting the reconciliation protocol on channel disconnect was rejected because the existing timer is already the intended fallback.
Verification
cargo run --release->recursive watches: 519,declaration-only shallow watches: 4.cargo test --quiet --workspace --lib --bins -- --test-threads=1: 373 passed, 0 failed.--checkandgit diff --check: passed.nix build .#default --no-link --max-jobs 1 --cores 2 --option min-free 50000000000: passed.Complexity
One small watcher guard owns the desired shallow subscription set and refresh operation. One wait helper owns event/timer/stop semantics. Both reuse existing discovery and reconciliation machinery; there is no new dependency, background thread, or protocol.
Concerns
The declaration namespace itself remains recursively discoverable by design. An organizational directory named
resourcesis not excluded unless it is actual Agent state, preserving current discovery semantics.Friction & bottlenecks
Follow-ups
None.
References