Skip to content

fix(sync): bound OpenCode background work across watcher and polling modes - #1262

Closed
rodboev wants to merge 20 commits into
kenn-io:mainfrom
rodboev:pr/1208-opencode-bounded-background-work
Closed

fix(sync): bound OpenCode background work across watcher and polling modes#1262
rodboev wants to merge 20 commits into
kenn-io:mainfrom
rodboev:pr/1208-opencode-bounded-background-work

Conversation

@rodboev

@rodboev rodboev commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Degraded polling now retains provider-owned watch scopes end to end. Persistent polling reasons carry their originating watch scopes, so a shared configured dir keeps a single complete owner when one exists and falls back to generic only for mixed or incomplete ownership. Provider-scoped polling reconciles only the exact selected configured roots, which keeps an unchanged nested OpenCode scope suppressed while a changed parent scope still syncs through the existing provider path.

One predicate now answers whether a pass reaches a configured root, and both discovery admission and tombstone accounting call it. A provider-scoped pass therefore reports only the roots it actually selected as covered, so a session relocated into a configured nested root the pass did not select is not read as deleted; that tombstone waits for a pass covering every configured root. Descendant scopes a pass excludes are excluded by the ownership query rather than after its rows are paged, so a small selected scope no longer does work proportional to a large unselected nested archive.

Each provider-owned directory carries its own polling obligation once a reason covers more than one, matching what the runtime watcher fallback already did. One obligation carries one probe, so a probe no longer answers for a directory it never observed. Obligation keys are namespaced by the reason that created them, which keeps two reasons on the same root distinct without either having to be split per directory to tell them apart.

The group-scoped acknowledgement reset from the prior rebuild stays intact, and every eligible polling group still runs in deterministic order. The bounded journal change feed remains deferred to a follow-up for issue #1208.

Part of #1208

@roborev-ci

roborev-ci Bot commented Jul 24, 2026

Copy link
Copy Markdown

roborev: Combined Review (badd3c8)

Review verdict: Three medium-severity correctness issues could cause missed sessions, skipped polls, or unnecessary archive-wide work.

Medium

  • internal/parser/opencode_provider.go:116 — A probe created while a root is SQLite-only permanently tracks only the database. If storage/session later appears and the root becomes hybrid, an unchanged database token suppresses degraded polling, causing new storage-backed sessions to be missed.

    • Fix: Re-resolve the OpenCode storage mode on every probe and fall back to authoritative polling whenever the root is no longer SQLite-only. Add a SQLite-to-hybrid transition test.
  • cmd/agentsview/unwatched_poll.go:175pollObligations, pollRevisions, and pollStates are updated in separate critical sections. A worker can snapshot a replacement obligation with stale state or without a revision, causing a required fresh poll to be skipped or stale state to be committed after replacement or removal.

    • Fix: Publish the obligation snapshot, revision increment, and state invalidation atomically under pollMu. Commit state only when the captured revision exists and still matches.
  • cmd/agentsview/unwatched_poll.go:378 — Omitting an unchanged degraded obligation from the candidate set does not fully suppress its reconciliation. An unprobed obligation with an overlapping ancestor or descendant root can trigger ReconcileWatchRoots, whose cross-provider expansion reintroduces the suppressed OpenCode root and restores archive-cardinality work every tick.

    • Fix: Preserve provider ownership through polling and reconcile provider-scoped roots, or partition overlapping obligations so unrelated candidates cannot expand into an unchanged degraded scope.

Reviewers: 2 done | Synthesis: codex, 12s | Total: 7m5s

@roborev-ci

roborev-ci Bot commented Jul 24, 2026

Copy link
Copy Markdown

roborev: Combined Review (badd3c8)

Code review found one medium-severity issue requiring attention.

Medium

  • internal/parser/opencode_provider.go:116 — The probe permanently tracks only the SQLite file after initial creation. If an OpenCode root later transitions from SQLite-only to hybrid/file storage while watcher coverage is degraded, storage changes will not alter the probe token, potentially suppressing reconciliation indefinitely.
    • Suggested fix: Retain the provider root and re-resolve its storage mode on every check. Return an error to fall back to authoritative polling whenever the mode is no longer SQLite-only, and add a transition test.

Reviewers: 2 done | Synthesis: codex, 6s | Total: 3m41s

@berenddeboer

Copy link
Copy Markdown
Contributor

Thanks @rodboev for having a go at this. As you can see this is a massive PR. I don't know the code, but it doesn't feel right. What about having agentsview startup with sync, then having a queue and job workers to sync things in the background? It just feels to me this needs a different model. But then I don't know the code :-)

@roborev-ci

roborev-ci Bot commented Jul 24, 2026

Copy link
Copy Markdown

roborev: Combined Review (4826ee1)

Medium issue found: the OpenCode freshness probe can miss SQLite-to-hybrid storage transitions.

Medium

  • internal/parser/opencode_provider.go:110 — The SQLite-only probe permanently captures only dbPath. If storage/session later appears while the SQLite database remains unchanged, the root becomes hybrid, but the cached token remains equal, causing degraded polling to skip reconciliation. New file-backed sessions may remain undiscovered until another reconciliation occurs.

    Suggested fix: Retain the configured root/provider in the probe and re-resolve the source mode on every poll. Fail open or change the token when the root is no longer purely SQLite. Add a regression test covering a SQLite-to-hybrid transition after the initial token is committed.


Reviewers: 2 done | Synthesis: codex, 7s | Total: 5m19s

@roborev-ci

roborev-ci Bot commented Jul 24, 2026

Copy link
Copy Markdown

roborev: Combined Review (b727827)

Changes need revision: three medium-severity polling and provider-ownership issues remain.

Medium

  • Provider ownership is lost in obligation handlingcmd/agentsview/unwatched_poll.go:134, cmd/agentsview/main.go:2009
    updateRoots drops Agent when copying an obligation, while startup and fallback obligations never populate it. Polling consequently uses generic ReconcileWatchRoots, which expands across overlapping providers and may bypass unchanged degraded-probe suppression. Preserve Agent, set it when the final scope belongs to one provider, and add a coordinator-level test verifying ReconcileProviderRoots is called.

  • Agent aggregation is order-dependent and incompleteinternal/sync/watch_backend.go:57
    Processing stops at the first scope without a degraded probe, ignoring later mixed-provider scopes. Multiple directories grouped under one key can also overwrite degradedAgent, incorrectly assigning the obligation to a single provider and leaving others unreconciled during fallback. Scan every scope, determine probe eligibility independently, aggregate ownership across all directories, and set Agent only when exactly one provider owns the entire obligation.

  • Probe errors leave stale cached statecmd/agentsview/unwatched_poll.go:421
    When a degraded probe fails, reconciliation runs but the previous cached state remains. After a temporary hybrid/storage-mode transition, returning to the same unchanged SQLite database can match the stale token and skip the authoritative reconciliation needed to remove storage-backed sources or restore shadowed SQLite sources. Invalidate the revision’s cached state on probe failure and add a stable → error/unsupported → stable regression test.


Reviewers: 2 done | Synthesis: codex, 15s | Total: 7m10s

@rodboev

rodboev commented Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

Fair question. By raw count a good chunk of this is tests, but more to the point it's not a new model: it's deliberately the smaller half of the design already written up in #1208. Keep provider-owned watch units, add a cheap freshness probe so degraded polling and no-op watcher events skip authoritative work when nothing changed. Real changes still flow through the existing sync path, so there's no second runtime to reason about. The bounded journal feed from that design, which bounds active-use work as well, is the natural next slice, so I'll switch this from Closes to Part of #1208 to make that explicit. On startup sync plus a queue and job workers: the design writeup in the issue argues against durable queues since they couple source progress to archive writes, and probe-then-escalate gets the same idle-cost result without a new subsystem. I'm also simplifying how the probe is carried through watcher fallback, which should shrink the non-test diff before this is ready.

@roborev-ci

roborev-ci Bot commented Jul 24, 2026

Copy link
Copy Markdown

roborev: Combined Review (b727827)

Medium-severity issues remain in provider-scoped polling and ownership propagation.

Medium

  • cmd/agentsview/unwatched_poll.go:133, cmd/agentsview/main.go:2009 — Provider ownership is lost because updateRoots omits Agent when copying an obligation, while startup obligations never populate it. Provider-owned polls therefore fall back to generic cross-provider reconciliation, scanning unrelated overlapping roots and defeating bounded polling.

    Suggested fix: Preserve Agent in updateRoots, and set it in watchRoot.pollingObligations when all grouped scopes belong to one provider. Add a coordinator-level test covering AddObligation.

  • internal/sync/watch_backend.go:57 — Agent ownership is calculated per directory but stored under a shared obligation key. Directories from different providers can overwrite that agent nondeterministically, and a nil probe prematurely stops ownership inspection. Once ownership is propagated, reconciliation may select an arbitrary provider and leave others stale.

    Suggested fix: Calculate probe eligibility and agent ownership independently across every scope and directory assigned to the obligation key, clearing ownership on any mismatch. Test reversed scope ordering and multiple directories owned by different agents.


Reviewers: 2 done | Synthesis: codex, 11s | Total: 3m58s

@roborev-ci

roborev-ci Bot commented Jul 24, 2026

Copy link
Copy Markdown

roborev: Combined Review (ac59c6b)

No issues found.


Reviewers: 2 done | Synthesis: codex | Total: 4m46s

@roborev-ci

roborev-ci Bot commented Jul 24, 2026

Copy link
Copy Markdown

roborev: Combined Review (ac59c6b)

Code changes are sound, but test compilation and expectations need updates before merging.

Medium

  • cmd/agentsview/main_test.go:1653, internal/sync/watch_backend_fsnotify_test.go:293 — Tests still expect unsuffixed polling keys and empty agents, while single-provider obligations now append |<syncDir> and set Agent. Update affected expectations and map lookups to use the scoped key and expected agent.

  • internal/sync/watcher_darwin_test.go:1616 — The test references parser.AgentOpenCode and parser.AgentKilo without importing go.kenn.io/agentsview/internal/parser, preventing the macOS test package from compiling. Add the missing import.


Reviewers: 2 done | Synthesis: codex, 9s | Total: 6m58s

@rodboev
rodboev force-pushed the pr/1208-opencode-bounded-background-work branch from ac59c6b to 3ce6653 Compare July 24, 2026 23:04
@roborev-ci

roborev-ci Bot commented Jul 24, 2026

Copy link
Copy Markdown

roborev: Combined Review (3ce6653)

The change is directionally sound, but one medium-severity polling-scope bug remains.

Medium

  • internal/sync/watch_backend.go:59 — A polling obligation retains its agent only when the watch root contains multiple sync directories. In the common single-provider, single-directory handoff, Agent is empty, bypassing the degraded-state probe and triggering generic cross-provider reconciliation on every poll. Set obligationAgent whenever all directory scopes share a non-empty agent; only condition the key suffix on splitByDir. Add a single-scope regression test.

Reviewers: 2 done | Synthesis: codex, 7s | Total: 2m45s

@roborev-ci

roborev-ci Bot commented Jul 24, 2026

Copy link
Copy Markdown

roborev: Combined Review (3ce6653)

Two medium-severity issues remain: common fallback paths discard provider ownership, causing unnecessary archive-scale reconciliation.

Medium

  • cmd/agentsview/main.go:1909 — Pending roots are registered with providerScoped=false, so an OpenCode root missing at startup receives an empty Agent. If it later becomes a SQLite root, the degraded-state probe never activates, and portable watcher fallback continues generic archive-scale reconciliation every polling interval.

    • Fix: Preserve provider ownership for homogeneous pending scopes and add a test for an absent OpenCode root that later becomes SQLite.
  • internal/sync/watch_backend.go:59pollingObligationsForScopes sets Agent only when multiple distinct directories exist. A single-directory runtime handoff loses provider ownership, preventing unchanged-state suppression and forcing generic cross-provider reconciliation after fsnotify or Darwin coverage loss.

    • Fix: Set Agent whenever a directory’s scopes share one provider; use splitByDir only to determine whether the obligation key needs a directory suffix.

Reviewers: 2 done | Synthesis: codex, 9s | Total: 5m9s

@rodboev
rodboev force-pushed the pr/1208-opencode-bounded-background-work branch from 3ce6653 to 80b2389 Compare July 24, 2026 23:14
@roborev-ci

roborev-ci Bot commented Jul 24, 2026

Copy link
Copy Markdown

roborev: Combined Review (80b2389)

Medium-severity issues remain in SQLite freshness detection and symlinked provider polling scope.

Medium

  • SQLite WAL reuse can hide real commitsinternal/parser/opencode_provider.go:1023, internal/sync/opencode_container_gate.go:174
    After a WAL restart, later commits may overwrite existing frame slots without changing WAL size, header salts/checkpoint sequence, database change counter, or second-resolution mtime. These commits can be misclassified as unchanged, causing degraded polling and watcher classification to skip updates indefinitely. Include a reliable logical commit marker, such as the committed WAL frame position/checksum or a SQLite-backed change counter, and add a regression for WAL reuse with multiple same-second commits.

  • Symlinked provider roots lose agent ownershipcmd/agentsview/main.go:1942, cmd/agentsview/main.go:2086
    A symlinked pure-SQLite OpenCode root becomes a persistent polling root, but its obligations have an empty Agent. This bypasses provider-specific degraded-state resolution and triggers generic archive-scale reconciliation on every tick. Preserve provider scope in symlink and persistent polling metadata, using generic reconciliation only for genuinely mixed ownership, and add an unchanged-tick regression for a symlinked OpenCode SQLite root.


Reviewers: 2 done | Synthesis: codex, 10s | Total: 7m37s

@roborev-ci

roborev-ci Bot commented Jul 24, 2026

Copy link
Copy Markdown

roborev: Combined Review (80b2389)

Potential stale-session regressions remain in the new degraded polling logic.

Medium

  • cmd/agentsview/unwatched_poll.go:101 — Probe state is cached before reconciliation and independently of obligation replacement. An in-flight worker can restore state for an old obligation after AddObligation replaces it and clears the cache. If the replacement has different roots but the same probe token, its first reconciliation may be skipped indefinitely. Version obligations and commit probe state only after successful reconciliation when the key/version is still current, or include the complete obligation identity in the cache and reject stale in-flight results.

  • cmd/agentsview/main.go:2053 — Watcher-construction fallback clears coverageRoots even when keyed polling registration fails. The affected scope then has neither keyed polling ownership nor fallback coverage and may remain stale indefinitely. Suppress fallback coverage only after all keyed obligations are installed successfully; otherwise retain or retry failed scoped obligations and propagate the registration error.


Reviewers: 2 done | Synthesis: codex, 9s | Total: 7m36s

@roborev-ci

roborev-ci Bot commented Jul 24, 2026

Copy link
Copy Markdown

roborev: Combined Review (7a5614f)

Degraded polling has one medium-severity correctness issue involving overlapping provider roots.

Medium

  • cmd/agentsview/unwatched_poll.go:467-524 — Missing probes populate a single global path-based blocker set. A missing scope for one provider can therefore suppress polling of an overlapping scope owned by another provider, even though ReconcileProviderRoots cannot expand into or tombstone the missing provider’s data. Changes for the available provider may remain stale indefinitely.

    Suggested fix: Track blockers per agent, plus a global set for unscoped obligations. Agent-scoped candidates should honor only global and same-agent blockers, while generic candidates should honor all blockers. Add a regression test covering overlapping roots owned by different agents.


Reviewers: 2 done | Synthesis: codex, 8s | Total: 5m36s

@roborev-ci

roborev-ci Bot commented Jul 24, 2026

Copy link
Copy Markdown

roborev: Combined Review (7a5614f)

No issues found.


Reviewers: 2 done | Synthesis: codex | Total: 5m40s

@rodboev
rodboev force-pushed the pr/1208-opencode-bounded-background-work branch from 7a5614f to 9869a76 Compare July 25, 2026 00:00
@roborev-ci

roborev-ci Bot commented Jul 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (9869a76)

Changes look sound overall, but macOS test compilation is currently broken.

Medium

  • internal/sync/watcher_darwin_test.go:1616 — The Darwin-only test references parser.AgentOpenCode and parser.AgentKilo without importing internal/parser, causing macOS compilation to fail with undefined: parser. Add go.kenn.io/agentsview/internal/parser to the imports.

Reviewers: 2 done | Synthesis: codex, 6s | Total: 4m27s

@roborev-ci

roborev-ci Bot commented Jul 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (7778eeb)

Medium-severity issue: degraded polling can leave later provider roots permanently stale after an earlier reconciliation failure.

Medium

  • cmd/agentsview/unwatched_poll.go:417 — Polling stops after the first reconciliation error. A persistently failing unscoped group or alphabetically earlier provider prevents subsequent provider-owned roots from being reconciled, leaving additions and deletions stale indefinitely. Continue reconciling independent groups, aggregate errors with errors.Join, and add a test proving a later provider is polled when an earlier one fails.

Reviewers: 2 done | Synthesis: codex, 6s | Total: 4m39s

@roborev-ci

roborev-ci Bot commented Jul 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (7778eeb)

Code changes are generally sound, but one medium-severity polling provenance issue remains.

Medium

  • cmd/agentsview/main.go:1921 — Persistent polling directories lose their originating agent and inherit the agent of the first watch root containing that directory. For shared directories—such as Claude watching a directory containing a symlinked OpenCode storage root—this creates a Claude-scoped obligation and can trigger archive-sized Claude reconciliation on every poll.

    Suggested fix: Preserve persistent polling reasons as agent-plus-directory scopes, generate obligations from that provenance, and add a shared-directory regression test covering a symlink-gated provider alongside another provider.


Reviewers: 2 done | Synthesis: codex, 7s | Total: 8m13s

@rodboev
rodboev force-pushed the pr/1208-opencode-bounded-background-work branch from 7778eeb to 2b69ddc Compare July 25, 2026 00:33
@roborev-ci

roborev-ci Bot commented Jul 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (2b69ddc)

No issues found.


Reviewers: 2 done | Synthesis: codex | Total: 4m45s

@roborev-ci

roborev-ci Bot commented Jul 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (2b69ddc)

Changes need adjustment: one medium-severity bounded-work regression was found.

Medium

  • cmd/agentsview/unwatched_poll.go:321 — When any reconciliation group fails, resetTracked(tracked) clears degraded-probe state for every tracked obligation, including provider groups that reconciled successfully. A persistent failure in an unrelated provider can therefore force unchanged OpenCode SQLite roots through archive-scale discovery on every polling tick, defeating the bounded-work optimization.

    Suggested fix: Track reconciliation results per generic/provider group and clear probe state only for obligations belonging to failed or unexecuted groups. Preserve state for groups that completed successfully.


Reviewers: 2 done | Synthesis: codex, 7s | Total: 6m43s

@rodboev
rodboev marked this pull request as draft July 25, 2026 02:13
@roborev-ci

roborev-ci Bot commented Jul 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (92fe8d2)

Code review found two medium-severity issues in provider-scoped polling and reconciliation.

Medium

  • cmd/agentsview/main.go:1919 — Persistent polling tracks only directory paths and associates each with the first matching watch root. When providers share a configured directory, reconciliation can be restricted to the wrong agent, causing a poll-only provider to lose coverage while suppressing the generic fallback.

    Fix: Preserve provider ownership when collecting persistent polling reasons, merge all owners per directory, and leave Agent empty for mixed or incomplete ownership. Add a shared-directory regression test covering a poll-only provider and a separately watched provider.

  • cmd/agentsview/unwatched_poll.go:518 — Dropping an unchanged provider obligation does not prevent an overlapping selected obligation for the same agent from reintroducing it. Because ReconcileProviderRoots expands selected roots to every overlapping configured directory, a changed or unsupported parent OpenCode root can repeatedly rescan a large unchanged nested SQLite root, defeating bounded degraded polling.

    Fix: Reconcile exact provider-obligation scopes without expanding into probe-suppressed roots for the same agent, while preserving safe deletion boundaries. Add coverage for nested OpenCode roots where one remains pollable and the other has an unchanged degraded state.


Reviewers: 2 done | Synthesis: codex, 9s | Total: 9m44s

@rodboev

rodboev commented Jul 25, 2026

Copy link
Copy Markdown
Contributor Author

@wesm I just pushed the class fix on this branch. The invariant is now: degraded work stays probe-gated, real changes still flow through the existing sync path, and the journal change feed stays deferred to a follow-up under #1208.

I also tightened provider polling to the exact selected configured roots, so an unchanged nested OpenCode scope does not get pulled back in by an overlapping parent. If this is the right shape, great. If the size is the real problem, the watcher no-op suppression half is separable and I can split that out, but I held that in reserve instead of guessing. Would you want this kept together or split?

@roborev-ci

roborev-ci Bot commented Jul 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (31f83b3)

Code review found two medium-severity correctness issues in nested-root reconciliation; no security issues were identified.

Medium

  • internal/sync/engine.go:3876 — Provider-scoped reconciliation treats requested roots as exact, but reconciliationCoversConfiguredRoots considers a selected ancestor sufficient to cover nested configured roots. A partial spool may therefore be treated as authoritative, soft-deleting aggregate members, replacements, or persistent-archive sessions that still exist under an unselected nested root.

    • Fix: Require every configured root to be explicitly selected before declaring full provider coverage. Add a regression test where the same session/member exists across overlapping roots.
  • internal/sync/engine.go:3917unselectedDescendantConfiguredRoots can over-exclude explicitly selected nested scopes. With configured roots parent, parent/child, and parent/child/grandchild, selecting parent and grandchild excludes child; the path check then also skips sources in the selected grandchild, leaving deletions active until a later full audit.

    • Fix: Determine ownership using the most-specific configured root, or subtract explicitly selected descendant scopes from excluded regions. Add a regression test covering interleaved selected and unselected nested roots.

Reviewers: 2 done | Synthesis: codex, 11s | Total: 10m55s

@rodboev
rodboev force-pushed the pr/1208-opencode-bounded-background-work branch from 31f83b3 to e4a1430 Compare July 25, 2026 03:14
@roborev-ci

roborev-ci Bot commented Jul 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (e4a1430)

Medium severity findings remain in provider-scoped reconciliation and polling.

Medium

  • internal/sync/engine.go:3881reconciliationCoversConfiguredRoots treats selecting an ancestor as covering nested configured roots, although agent-scoped discovery excludes those nested roots. This can incorrectly trust a partial spool as complete and tombstone a session moved from the selected parent into an excluded nested root. Require every configured root to be explicitly selected for agent-scoped reconciliation before setting allProviderRootsCovered, and add a regression test covering a move into an excluded nested root.

  • internal/sync/engine.go:3911 — Descendant scopes are excluded only after ownership rows are paged from the database. Polling a small parent scope can therefore scan every ownership row in a large nested configured scope, leaving background work unbounded by the changed scope. Apply descendant exclusions in the ownership query and add a small-versus-large nested-archive cardinality regression.


Reviewers: 2 done | Synthesis: codex, 8s | Total: 9m33s

@rodboev rodboev changed the title fix(sync): bound OpenCode background work across watcher and polling modes - Jul 29, 2026
@rodboev
rodboev marked this pull request as ready for review July 29, 2026 13:15
@roborev-ci

roborev-ci Bot commented Jul 29, 2026

Copy link
Copy Markdown

roborev: Combined Review (0826e14)

Medium-severity issues remain in degraded polling and exclusion-path handling.

Medium

  • cmd/agentsview/main.go:1915, cmd/agentsview/main.go:2182 — Pending and symlink polling can collapse multiple same-provider directories into one degraded obligation. OpenCode’s probe reports only the first matching root, so changes in another grouped root may be skipped indefinitely; a temporarily absent grouped root may also remain skipped after returning. Split provider-owned obligations per directory, as runtime watcher fallback already does, or aggregate state across every root and invalidate it whenever any root is filtered out.

  • internal/db/sessions.go:2784 — Exclusion containment uses lexical comparison, but exclusion roots are absolute while provider ownership scopes may retain relative configured paths. Equivalent relative and absolute paths therefore may not be excluded in SQL, causing each polling pass to page through the excluded archive before the later safety filter rejects it. Canonicalize scope and exclusion paths for comparison while preserving stored-path representation for SQL, and add mixed relative/absolute test coverage.


Reviewers: 2 done | Synthesis: codex, 10s | Total: 14m59s

@rodboev

rodboev commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Going to get this one finished, then take a look at the whole of it and decide whether a refactor is appropriate...

@rodboev rodboev changed the title - fix(sync): bound OpenCode background work across watcher and polling modes Jul 29, 2026
@roborev-ci

roborev-ci Bot commented Jul 29, 2026

Copy link
Copy Markdown

roborev: Combined Review (20dd6c7)

The changes need revision to address two medium-severity reconciliation risks.

Medium

  • cmd/agentsview/unwatched_poll.go:518 — Unchanged provider obligations are discarded before roots are grouped. With multiple configured OpenCode roots, changing one root reconciles only that root; an unchanged sibling containing a previously shadowed duplicate is not discovered. Deleting the canonical copy can therefore incorrectly tombstone the session until the daily full audit.

    • Fix: When any provider obligation changes, include all same-provider roots needed to establish the canonical source and prove deletion, or add a bounded cross-root replacement lookup.
  • internal/db/sessions.go:2925 — Each excluded descendant adds two predicates and bind parameters for every ownership scope. Hundreds of nested roots can exceed SQLite expression-depth or variable limits, causing degraded reconciliation to fail repeatedly.

    • Fix: Model exclusions with a bounded relation or temporary table, compact redundant descendants, and add a high-cardinality configured-root regression test.

Reviewers: 2 done | Synthesis: codex, 11s | Total: 8m59s

@rodboev

rodboev commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Splitting this up as @berenddeboer mentioned. The scheduler here is hand-rolled and would be properly done as a work queue.

The argument against a durable queue still stands for now since it couples source progress to archive writes; an in-process queue with job workers avoids most of those problems.

Two PRs:

  • Reconciliation scoping. A reconcile pass walks only the roots it was asked about, coverage accounting agrees with what discovery actually admitted, and stored-source lookups are bounded by that same scope instead of scanning every hint. About 440 lines across internal/sync and internal/db. This is correct no matter how the work gets scheduled; a queue would still need it, and it's the half that bounds the reads.

  • Polling coordination. Per-root obligations, degraded probes, unwatched-root polling. This is the half a queue would replace, so it's worth judging on that basis rather than riding along with the part that stands on its own.

Both will ref instead of close.

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants