fix(opencode): split the watch plan into database and storage coverage units - #1318
fix(opencode): split the watch plan into database and storage coverage units#1318rodboev wants to merge 6 commits into
Conversation
roborev: Combined Review (
|
|
The storage tree narrowing is discussed in the PR body: the watch plan is cached at startup, so the shallow unit observes The reason an always-emitted storage unit doesn't work is documented on agentsview/internal/parser/opencode_provider.go Lines 560 to 573 in bebdb83 A unit emitted before the directory exists is a permanently missing probe on pure-SQLite roots. The poller defers every candidate overlapping a blocked probe, so it would silence the fallback polling that matters when watcher coverage degrades. A pending watch that owns the ancestor lifecycle would avoid the probe. The Darwin backend has that shape, but the portable backend and the engine's cached changed-path plan don't, so dynamic activation belongs to the bounded change-feed follow-up on #1208. The |
roborev: Combined Review (
|
|
Same finding as the last run; the commit in between only clarified the |
roborev: Combined Review (
|
roborev: Combined Review (
|
…#1307) When one provider loses native watch coverage, the unwatched-root poller flattened every obligation into a single root list and called the unscoped reconcile entry point, which expands each root across every configured agent. One provider's coverage gap therefore dragged every overlapping provider through a full authoritative discovery every two minutes, and an authoritative pass for the wrong provider can tombstone sessions under a lifecycle-owned missing root. Two smaller bugs compounded it: obligations for different reasons over one physical root collapsed into one map entry (releasing one silently released the other), and the poll interval was measured between ticker deadlines, so a pass that ran longer than the interval was followed immediately by the next one. What this changes: - `PollingObligation` now carries `(agent, root)` scopes instead of bare roots. Provider identity travels through registration, the watcher backends, and the polling handoff instead of being reconstructed downstream (`internal/sync/watcher.go`, `cmd/agentsview/main.go`). - The coordinator groups available scopes by agent and reconciles each provider only over its own roots. Obligation keys are namespaced by reason and provider (`degraded:<agent>:<path>`, `nowatcher:<agent>:<path>`, `persistent:<dir>`), so overlapping obligations stay independently releasable (`cmd/agentsview/unwatched_poll.go`). - Persistent-polling obligations carry scopes only for the providers that requested persistent polling for a directory, not every agent sharing it. `collectWatchRoots` records that provenance and `watchPollingObligations` consumes it (`cmd/agentsview/main.go`). - A poll pass issues one grouped engine call, `Engine.ReconcileProviderRootsGrouped`, which runs each provider's bounded pass but shares a single epilogue — global subagent linking and skip-cache persistence — across the batch. Without this, that archive-sized work ran once per provider per tick. The sync lock is held across the groups and the epilogue so a concurrent pass cannot persist newer skip state that the shared epilogue would then overwrite; a canceled batch stops between groups and skips the epilogue rather than blocking shutdown on it; epilogue eligibility is recorded at each pass's own gate sites (after page writes commit, before tombstoning), so a tombstoning or cleanup failure cannot leave synced sessions unlinked; and session events are emitted once per batch outside the lock (`internal/sync/engine.go`). - The pre-discovery SQLite container capture is scoped to the pass: a group outside the OpenCode container family probes no containers, and an in-family group probes only the containers overlapping its roots, so probe work per poll is bounded by the batch instead of group count or total configuration (`internal/sync/opencode_container_gate.go`). - The worker waits a full interval measured from the previous pass's completion, not from the previous ticker deadline. Deferral is conservative where scopes mix: an obligation without a provider keeps the unscoped path, so a blocked provider-less root defers every provider on it and vice versa. Probe gates, remote roots, watcher recovery, and the daily audit are unchanged; the audit remains the backstop for anything a scoped poll declines to prove. Each provider's pass still performs a full discovery over its scope; making the pass itself incremental needs provider-declared coverage units and is deferred to #1318, which builds on the polling-scope API introduced here. The 15-minute scheduled sync (`runScheduledSyncPass`) still reconciles providers individually and could adopt the grouped call in a follow-up. Refs #1208. Co-authored-by: Rod Boev <rodboev@users.noreply.github.com>
ea14de5 to
902bc7d
Compare
roborev: Combined Review (
|
|
This rebased cleanly on top of #1307 after that one merged. |
…1319) Part of the prep work for #1208, along with #1307 and #1318. The branch was squashed and includes #1291's changes; if #1291 merges on its own, this needs a rebase. ## The bug Asking reconciliation to check one directory could delete sessions in other directories. The requested path got widened to the whole configured root, and that widened claim was treated as proof when deciding what to tombstone. ## The fix Each provider now says exactly what a request covers: - Discovery may walk a wider directory, but the pass can only touch what was actually asked for. - Deleting anything requires proof the pass really looked there. - A pass that covered one whole container (a SQLite file, a Hermes archive) can clean up its members — after checking the session didn't just move somewhere else. - Blank or unrelated paths now do nothing instead of matching everything. Hermes, OpenCode, Zed, Shelley, Trae, Aider, VS Copilot, and Omnigent each get topology rules for their own layouts (shared databases, archives, profile folders). ## Also in here (from #1291) Watcher performance for shared SQLite databases: one session's write used to re-parse the whole database. Now each session has its own change signal, and a file event does work proportional to what changed, not to the archive size. ## Where to look - `internal/parser/provider.go` — the scope plan - `internal/sync/engine.go` — how the engine uses it - `internal/parser/hermes_provider.go`, `opencode_provider.go`, `multi_session_container.go` — per-provider rules Co-authored-by: Rod Boev <rodboev@users.noreply.github.com>
902bc7d to
be31f47
Compare
roborev: Combined Review (
|
|
Rebased onto current |
1 participant
The OpenCode provider declares one recursive watch root per configured directory, so registration walks the whole tree and draws on the process-wide recursive budget. On a large installation the budget runs out, the root gets no native coverage, and every poll becomes an authoritative reconciliation over the archive. The database and its write-ahead log are direct children of the configured root and never needed recursion.
This splits the plan into two coverage units per root: a non-recursive unit on the configured root covering only the database and its WAL, exempt from the recursive budget, and a recursive unit on the storage subtree, emitted only when the root resolves to file-backed storage. The units fail independently, so losing storage coverage no longer takes SQLite observation with it. A changed path is claimed by exactly one unit while the storage directory's existence is stable, and Kilo, MiMoCode, and ICodeMate inherit the same shape through the shared provider spec. The watch plan is cached at startup, so a storage tree appearing at runtime gets recursive coverage at the next startup, with the daily audit as backstop. A unit that loses coverage is still polled by its configured root at the same archive scale as before; carrying unit identity through the polling obligation belongs to the follow-up.
Measured on windows/amd64 with fsnotify v1.10.1: the non-recursive watch observes the database, its WAL, and storage-directory creation, and does not observe grandchildren. The watcher-exhaustion trace and the production measurements came from the issue report. It is one of three preparatory PRs for #1208, with #1307 and the reconciliation-scoping change (#1319), staged separately so roborev reviews each in isolation; the bounded change feed and coalesced audit that close the issue follow once they land. This branch overlaps #1307 on the polling obligation surface and rebases onto it after #1307 merges.
Refs #1208