fix(daemon): scope degraded-coverage polling to one provider per pass - #1307
Conversation
roborev: Combined Review (
|
|
Looks good in principle, giving it some review and will merge |
roborev: Combined Review (
|
|
Sounds good. Fixed the lint issue, and opened the other two groundwork PRs for #1208: #1318 overlaps this one on the polling handoff and will rebase onto it once this lands, just to keep that as a smaller reviewable slice (versus adding ~2.5k lines); #1319 is based on #1291 so its diff shrinks when that merges. The bounded change feed that actually closes the issue comes as a fourth PR on top of all three (this one, #1318, and #1319) — so the integration builds on code that's fully reviewed. |
roborev: Combined Review (
|
Persistent obligation scopes were derived from every agent configured on a syncDir, so one provider's persistent-polling fallback reconciled unrelated providers sharing the dir and could tombstone their sessions under a lifecycle-owned missing root. collectWatchRoots now records which provider requested persistent polling for each dir and watchPollingObligations emits persistent scopes only for those requesters, replacing the syncDirToAgents lookup (and the symlinkGatedDirs parameter it existed for).
roborev: Combined Review (
|
Each per-provider ReconcileProviderRoots call ran the full pass epilogue: global subagent linking plus a clone-and-rewrite of the archive-sized skip cache. The provider-scoped poller therefore multiplied that database work by the number of provider groups on every tick. Engine.ReconcileProviderRootsGrouped runs each group's bounded pass with the epilogue deferred and performs linking and skip-cache persistence once after the last group. Linking still runs when any group committed its page writes and persistence when any group completed cleanly, matching the per-pass rules. The poll coordinator now issues one grouped call per pass.
roborev: Combined Review (
|
…pture The grouped reconciliation epilogue ran after each group's pass released syncMu, so a concurrent pass could update and persist newer skip state between the epilogue's in-memory snapshot and its table rewrite, restoring removed skip entries after a restart. ReconcileProviderRootsGrouped now holds syncMu across every group and the shared epilogue, with the session event emitted once per batch outside the lock to keep emitter re-entry off the critical section. The pre-discovery SQLite container capture also probed every configured OpenCode-family container for every group. It is now scoped to the pass's provider: out-of-family groups probe nothing, in-family groups probe only their own containers, and the unscoped pass still captures everything.
roborev: Combined Review (
|
A grouped batch canceled after an eligible group still ran the shared epilogue: linking and skip-cache persistence are not context-aware, so poller shutdown could block on archive-sized database work, and a canceled batch with no per-group error reported success. The grouped pass now stops between groups on cancellation, skips the epilogue, and returns the cancellation error. In-memory skip promotions survive and persist on the next clean pass. The in-family container capture also probed every configured dir for the agent regardless of the batch. It now probes only dirs overlapping the pass's reconciliation roots, using the same bidirectional overlap as logicalRootsForAgentWatchRoots so capture matches what discovery can stream.
roborev: Combined Review (
|
The grouped reconciliation inferred shared-epilogue eligibility from each group's final error, but that error also reflects tombstoning and spool cleanup failures that occur after page writes commit. In the per-pass ordering, linking and skip-cache persistence run before tombstoning, so a persistent tombstone failure never suppressed them; the grouped inference did, which could leave successfully synced subagents unlinked indefinitely. The streamed pass now records link and persist eligibility at its own gate sites and returns it through the scoped pass, and the grouped call consumes those flags instead of classifying the returned error. A side effect is that groups that did no work (all roots remote-excluded) no longer trigger the epilogue at all.
roborev: Combined Review (
|
…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>
2 participants
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:
PollingObligationnow carries(agent, root)scopes instead of bareroots. 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).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 independentlyreleasable (
cmd/agentsview/unwatched_poll.go).requested persistent polling for a directory, not every agent sharing it.
collectWatchRootsrecords that provenance andwatchPollingObligationsconsumes it (
cmd/agentsview/main.go).Engine.ReconcileProviderRootsGrouped, which runs each provider's boundedpass 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).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).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 providersindividually and could adopt the grouped call in a follow-up.
Refs #1208.