Skip to content

fix(sync): bound OpenCode background work to the changed session batch - #1331

Closed
rodboev wants to merge 16 commits into
kenn-io:mainfrom
rodboev:pr/1208-opencode-journal-feed
Closed

fix(sync): bound OpenCode background work to the changed session batch#1331
rodboev wants to merge 16 commits into
kenn-io:mainfrom
rodboev:pr/1208-opencode-journal-feed

Conversation

@rodboev

@rodboev rodboev commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Recurring OpenCode work is scheduled per root, so its cost follows the container instead of what actually moved. Any write to the shared database breaks the freshness trust that lets a pass skip unchanged sessions; the next pass rebuilds the per-session digest over every message and part row, and each watcher batch that advances a session's update time re-parses that session's entire history. OpenCode publishes a durable event journal recording which sessions changed and which finished changing. This makes that journal the unit of scheduling: with 5,000 stored sessions and one changed, the recurring pass previously evaluated all 5,000 sessions to skip 4,999, while the journal drain reads one row and returns one session identity.

What this changes:

  • A bounded change-feed capability, declared by the provider and unsupported by default, reads a fixed window of journal rows and returns session identities, continuation state, and an audit reason. Row metadata is read for every candidate and payload only for the small settlement-bearing types, with the size predicate in SQL so an oversized row is never materialized. A session becomes ready when an assistant message update carries a completion timestamp, and only a part update returns it to pending, because ordinary message and session updates routinely trail a completed turn. (internal/parser/opencode_change_feed.go, internal/parser/capabilities.go)
  • Feed eligibility is schema provenance rather than table presence, since the journal tables exist in forks that never write them; anything else falls through to existing behavior. (internal/parser/opencode_provider.go, internal/parser/sqlite_container_state.go)
  • Deletion never appears in the feed's output: deleting a session cascades its event history away without a tombstone, so removal keeps converging through existing verification and the audit. Cursor anchors span multiple sessions so an ordinary deletion cannot erase every witness, and a changed file identity, schema generation, or rewound row id forces one audit. (internal/parser/opencode_change_feed.go, internal/parser/sqlite_file_identity_unix.go, internal/parser/sqlite_file_identity_windows.go)
  • Scheduling moves behind one keyed worker built from the existing per-key scheduler. It holds one active pass per coverage unit, unions escalating reasons so a later ordinary event cannot downgrade them, measures degraded intervals from completion with backoff, and commits a checkpoint only after the archive write for that pass succeeded. (internal/sync/opencode_coverage_worker.go, internal/sync/keyed_schedule.go)
  • The engine resolves journal identities through its existing source-lookup contract, with archive ownership batch-loaded per pass; the feed itself returns no source paths and performs no filesystem walk. (internal/sync/opencode_coverage_coordinator.go, internal/sync/engine.go, internal/db/sessions.go)
  • Archive-scale reconciliation becomes reachable only through a sealed runner held by startup and the audit worker; watcher callbacks, degraded polling, and retries hold a bounded executor that can request recovery through a deduplicated audit lane but cannot run it inline. The grouped polling path now requires a named provider instead of treating an empty one as an unscoped pass. (internal/sync/coverage_executor.go, internal/sync/engine.go, cmd/agentsview/main.go, cmd/agentsview/unwatched_poll.go)
  • The portable watch backend owns missing recursive roots' lifecycle the way the macOS backend already does: the watch activates when the directory appears, files created in the handoff window are covered by the existing bounded created-subtree scan, and the storage coverage unit is emitted from the plan rather than from directory existence, so a pending root never blocks fallback polling behind a missing probe. This closes the runtime-creation gap disclosed in fix(opencode): split the watch plan into database and storage coverage units #1318, where a storage tree created after startup got no live coverage until restart or the daily audit. (internal/sync/watch_backend_fsnotify.go, internal/sync/watch_backend.go)

Session lineage and termination classification are unchanged, and providers without the capability construct no adapter. Journal checkpoints are in-memory, so a restart rebaselines under startup reconciliation. Pure-SQLite OpenCode roots remain in the startup and daily audit scope when storage is absent, while poll-path deletion detection for admitted containers still defers to the daily audit.

Based on #1318; the diff collapses to this change alone when it lands.

Closes #1208

@roborev-ci

roborev-ci Bot commented Aug 1, 2026

Copy link
Copy Markdown

roborev: Combined Review (93e1ca4)

Four medium-severity correctness issues could cause missed journal events or stale provider archives.

Medium

  • internal/sync/opencode_coverage_coordinator.go:130 — Runtime admission registers an uninitialized checkpoint, but the first drain treats it as baseline initialization and skips all existing journal rows, including the triggering event. A newly created compatible database can lose its first completed session until a later audit.

    • Fix: Introduce an explicit origin-drain checkpoint or mode for runtime admission, with an integration test using the real drain and applier.
  • internal/parser/opencode_change_feed.go:394 — Anchor validation accepts a checkpoint if any anchor survives, while the cursor remains based on the maximum anchor rowid even if that anchor was deleted. SQLite may reuse deleted tail rowids, causing subsequent events below the stale cursor to be skipped permanently.

    • Fix: Require the maximum cursor anchor to match, or rewind to the highest verified anchor and replay; trigger an audit when continuity cannot be established.
  • internal/sync/opencode_coverage_worker.go:428 — Audit recovery captures its new baseline after reconciliation. Events committed between the reconciliation source scan and baseline capture are covered by neither the audit nor the next journal drain.

    • Fix: Capture a fresh baseline before reconciliation, commit it only after the audit succeeds, and then drain forward from it.
  • internal/sync/opencode_coverage_coordinator.go:237 — When overlapping provider configurations share a root, one physical database can map to multiple coverage units, but keyForPath wakes only the first map entry. Selection is nondeterministic, leaving other providers’ archives stale.

    • Fix: Return and wake every matching coverage key, and admit all matching providers during runtime admission.

Reviewers: 2 done | Synthesis: codex, 12s | Total: 15m36s

@roborev-ci

roborev-ci Bot commented Aug 1, 2026

Copy link
Copy Markdown

roborev: Combined Review (e5945a6)

Code review found four medium-severity correctness issues; no critical or high-severity findings.

Medium

  • Lost-event recovery may miss rewritten filesinternal/sync/engine.go:3359
    Reconciliation is always called with force=false. The lost-events marker only changes OpenCode wake urgency, so non-journal providers and OpenCode storage roots can miss same-size, same-mtime rewrites after watcher overflow. Propagate the marker into the reconciliation force argument or add an equivalent provider-scoped forced recovery path.

  • Deleted cursor anchor can cause silent event lossinternal/parser/opencode_change_feed.go:393
    Checkpoint validation succeeds when any anchor survives, even if the maximum anchor row—the actual cursor—was deleted. SQLite may reuse that rowid, and a new event at that position is excluded by rowid > positionRowID. Require the cursor anchor itself to match and rebaseline on mismatch, or use a monotonic, non-reusable journal position.

  • Late database admission can skip existing sessionsinternal/sync/opencode_coverage_coordinator.go:130
    Late admission registers an uninitialized zero checkpoint, whose first drain baselines at the current maximum row without returning IDs. Because the triggering event has already been diverted from normal classification, a new or moved-in database may have all completed sessions skipped until a later audit. Initialize admission with an explicit row-zero cursor or reconcile the triggering contents before committing the baseline.

  • Events written during an audit can be skippedinternal/sync/opencode_coverage_worker.go:427
    The worker captures its new baseline after archive reconciliation. Events committed after their session is reconciled but before baseline capture become part of that baseline and are never applied. Capture the recovery boundary before reconciliation and resume from it afterward, preserving events written during the audit window.


Reviewers: 2 done | Synthesis: codex, 12s | Total: 12m47s

@roborev-ci

roborev-ci Bot commented Aug 1, 2026

Copy link
Copy Markdown

roborev: Combined Review (6b0979b)

Verdict: Changes require fixes for one high-severity synchronization regression and two medium-severity recovery gaps.

High

  • internal/sync/engine.go:893, internal/sync/opencode_coverage_coordinator.go:129SyncPathsContext returns success immediately after asynchronously waking coverage, breaking its synchronous contract for callers such as direct session sync. Engines outside watcher startup never call InitializeBoundedCoverage; their first compatible OpenCode path registers a zero checkpoint, which DrainOpenCodeJournal treats as a baseline and skips the triggering event entirely.
    • Fix: Keep generic path synchronization synchronous and move journal wakes to a watcher-specific entry point, or wait for worker completion and propagate errors. Initialize runtime admission with an explicit row-zero checkpoint rather than the zero value.

Medium

  • cmd/agentsview/main.go:2529 — Multi-provider recovery stops at the first failing provider. If that error supplies scoped retry roots, later provider groups are omitted from the retry and can remain permanently stale after an overflow.

    • Fix: Attempt every provider group, aggregate errors and failed roots, then construct the retry scope. Add a regression test where the first of multiple groups fails.
  • internal/sync/opencode_coverage_worker.go:428 — Audit recovery reconciles before capturing a fresh baseline. An OpenCode event committed after the reconciliation snapshot but before baseline capture can be skipped, while a wake received during the audit is cleared by run, potentially leaving the update unarchived. The stored auditAnchors are unused.

    • Fix: Capture the baseline before reconciliation, or resume from the saved audit anchor afterward, so events crossing the audit boundary are drained normally.

Reviewers: 2 done | Synthesis: codex, 12s | Total: 15m45s

@roborev-ci

roborev-ci Bot commented Aug 1, 2026

Copy link
Copy Markdown

roborev: Combined Review (b139f51)

The PR has three medium-severity correctness issues in OpenCode journal synchronization and lifecycle handling.

Medium

  • internal/sync/opencode_coverage_coordinator.go:132 — Runtime admission registers a zero checkpoint, but the first drain treats it as a baseline capture and skips all existing events, including the admission-triggering event. Because the path is removed from normal SyncPathsContext processing, a populated database added to the watched root may remain unsynced until an audit.

    • Fix: Initialize runtime-admitted units with an explicit row-zero checkpoint, or perform scoped reconciliation before baseline capture. Add a test proving that a settled pre-admission event reaches the real applier.
  • internal/sync/opencode_coverage_coordinator.go:38 — An admitted database remains routed to the journal worker after deletion. A missing file is treated as a successful no-op while authoritative reconciliation is suppressed, leaving archived sessions active until a later audit.

    • Fix: Do not claim or filter an admitted unit when its database is missing. Allow missing-path reconciliation, or explicitly schedule an audit and retire or rebaseline the unit.
  • internal/parser/opencode_change_feed.go:393 — Cursor validation accepts any surviving anchor while using the maximum committed anchor as the cursor. If the aggregate owning the highest anchor is deleted, SQLite can reuse its implicit rowid; an older surviving anchor then allows the newly reused cursor row to be skipped permanently.

    • Fix: Validate the maximum cursor anchor specifically. If it disappeared or changed, audit or rewind to the highest verified anchor and discard invalid anchors. Add coverage for highest-row deletion followed by rowid reuse.

Reviewers: 2 done | Synthesis: codex, 12s | Total: 13m35s

@roborev-ci

roborev-ci Bot commented Aug 2, 2026

Copy link
Copy Markdown

roborev: Combined Review (06a5cd8)

Medium-severity regressions remain in OpenCode journal reconciliation and watcher coverage.

Medium

  • internal/sync/opencode_coverage_coordinator.go:47 — Once a container is admitted, matching paths bypass normal synchronous processing. If the main database is deleted, the journal drain succeeds without changing the checkpoint, so missing-source tombstoning does not run and archived sessions remain active until a later audit. Check whether the admitted database still exists before consuming the path; if missing, retire the coverage unit and route the deletion through normal path processing or enqueue authoritative provider reconciliation.

  • internal/sync/opencode_coverage_worker.go:388 — The worker applies only ReadyIDs, while PendingIDs are excluded from the next checkpoint. A journal batch containing only session.created/session.updated, or no completed assistant event, can therefore advance the cursor without syncing affected sessions. Retain and reschedule pending IDs until bounded quiescence, or treat durable session metadata events as ready. Add a regression test for an update-only terminal batch.

  • internal/sync/opencode_coverage_coordinator.go:263 — SHM and header-only WAL events wake the coverage worker without the provider’s existing relevance checks. Because read-only SQLite access can create these sidecars, this may cause repeated self-triggered drains and regress SHM-only suppression. Apply ChangedPathRelevance before waking coverage, ignore SHM files and WAL files without frames, and add a sidecar-only event test.


Reviewers: 2 done | Synthesis: codex, 11s | Total: 16m44s

@roborev-ci

roborev-ci Bot commented Aug 2, 2026

Copy link
Copy Markdown

roborev: Combined Review (756bbc2)

No critical or high-severity issues found; one medium-severity reliability issue requires attention.

Medium

  • internal/parser/opencode_change_feed.go:295 — Stat, database-open, and transaction-start failures return a successful empty result. The worker then clears the wake without scheduling a retry, while startup baseline capture may accept an uninitialized checkpoint. This can cause a final update to be missed until another event or the daily audit.
    • Fix: Propagate transient I/O and database errors, reject uninitialized baseline results, and handle file disappearance explicitly through unit retirement or normal reconciliation.

Reviewers: 2 done | Synthesis: codex, 7s | Total: 16m53s

@roborev-ci

roborev-ci Bot commented Aug 2, 2026

Copy link
Copy Markdown

roborev: Combined Review (6598842)

High-severity issues remain in watcher coverage and retry reconciliation, with two medium-severity OpenCode lifecycle gaps.

High

  • cmd/agentsview/main.go:2005isCoveredMissingSubroot suppresses probe gates even when no watcher was constructed. For providers such as Gemini, an existing shallow root can make a missing tmp subtree appear covered, allowing fallback polling to reconcile an incomplete root and falsely tombstone archived sessions beneath it.

    • Fix: Trust only MissingRootLifecycleOwned from an actual backend result. Model OpenCode’s optional storage subtree explicitly instead of applying plan-based suppression to every provider.
  • cmd/agentsview/main.go:2497 — When no retry root exactly matches the current availability map and only one agent remains represented, all retry roots are assigned to that agent without checking availability. A root that disappeared between attempts can then be reconciled, falsely tombstoning its sessions.

    • Fix: Remove this fallback, retain provider ownership in retry batches, and reconcile only roots that still match the probed available scope.

Medium

  • internal/parser/opencode_provider.go:687, cmd/agentsview/main.go:844 — Every OpenCode root plans a recursive storage unit even when it is normally absent, while probeWatchRecoveryScope treats every missing planned unit as requiring deferral. Pure-SQLite OpenCode roots are consequently omitted from watcher recovery and the daily archive audit, so deletions without journal events never converge.

    • Fix: Mark the storage lifecycle unit as optional and exclude it from authoritative-scope deferral while retaining its creation watch.
  • internal/sync/opencode_coverage_coordinator.go:388 — After a journal-backed database covers a root, reconciliation retains the storage subtree only while it exists. A directory-removal event therefore drops the missing storage scope entirely, leaving storage-backed sessions active instead of tombstoning or recanonicalizing them.

    • Fix: Preserve the storage reconciliation scope for authoritative removal and rename recovery even when the directory is absent.

Reviewers: 2 done | Synthesis: codex, 14s | Total: 17m25s

@roborev-ci

roborev-ci Bot commented Aug 2, 2026

Copy link
Copy Markdown

roborev: Combined Review (bdce696)

Changes need revision: three medium-severity reliability issues could leave synchronization or coverage stalled.

Medium

  • internal/sync/opencode_coverage_worker.go:486 — If a coverage unit is retired after startAudit marks it running but before runEncodedAudit begins, the retired branch returns without calling finish. Re-admitting a recreated database clears retired but leaves running=true, so subsequent wakes are ignored until restart. Call finish(key) on this branch or centralize runner cleanup in a defer, and add a retire/recreate race test.

  • internal/sync/watch_backend.go:72 — A missing recursive root is marked lifecycle-owned based only on another planned root’s Exists flag. If the covering watch failed or exhausted its budget, polling is suppressed even though no native watch can observe creation, potentially leaving the root uncovered until an audit or restart. Claim ownership only after confirming an actual registered watch covers the creation path; otherwise retain polling.

  • cmd/agentsview/sync_worker.go:176 — A transient bounded-coverage baseline failure aborts the one-shot startup/audit worker before its authoritative sync. Because the worker does not watch paths and loses its in-memory checkpoint when it exits, the optional probe adds no lasting coverage while making all-provider synchronization vulnerable to transient failure. Omit initialization here or log the failure and continue, leaving baseline ownership to the long-lived daemon/watch engine.


Reviewers: 2 done | Synthesis: codex, 14s | Total: 20m10s

@roborev-ci

roborev-ci Bot commented Aug 2, 2026

Copy link
Copy Markdown

roborev: Combined Review (c39906f)

The change has four medium-severity correctness and recovery issues; no concrete security regression was identified.

Medium

  • cmd/agentsview/main.go:2432,2522 — Provider-owned retries can be silently discarded. Path failures omit ReconcileGroups, while retry filtering requires roots to exactly match physical watch roots. Configured ancestor roots, such as Copilot’s state directory, can therefore yield an empty recovery group, causing the watcher to acknowledge the retry without reconciliation.

    • Fix: Preserve ReconcileGroups on path failures and validate grouped roots using the same containment and deferred-scope logic as coversProviderRoot, rather than exact path equality.
  • internal/sync/opencode_coverage_coordinator.go:436-481 — Invalid or unresolved ready IDs are skipped, but the worker still treats the apply as successful and advances the journal checkpoint. A transient FindSource miss or deleted source can therefore permanently consume an event without archiving or auditing it.

    • Fix: Treat every unresolved ready ID as incomplete coverage and trigger retry or audit. Advance the checkpoint only after every ready identity is conclusively handled.
  • internal/parser/opencode_change_feed.go:335-345 — Most journal query failures, including feed deadlines and caller cancellation, are classified as structural anomalies that schedule an archive-scale audit. Only metaRows.Err() distinguishes context errors, so ordinary lock contention may unexpectedly invoke the expensive audit path.

    • Fix: Consistently propagate context.Canceled and context.DeadlineExceeded from every query and scan step, reserving AuditRequired for verified schema or continuity anomalies.
  • internal/sync/watch_backend_fsnotify.go:176-183,482-518 — The fsnotify backend ignores the polling-release callback. If a lifecycle-owned root degrades, disappears, and is later recreated with complete native coverage, its fallback polling obligation remains installed and performs authoritative reconciliation every two minutes indefinitely.

    • Fix: Retain OnPollingReleased and release the fsnotify-runtime:<root> obligation once the recreated subtree is fully watched.

Reviewers: 2 done | Synthesis: codex, 17s | Total: 20m43s

@rodboev
rodboev force-pushed the pr/1208-opencode-journal-feed branch from c39906f to dc84c77 Compare August 3, 2026 10:15
@roborev-ci

roborev-ci Bot commented Aug 3, 2026

Copy link
Copy Markdown

roborev: Combined Review (dc84c77)

High-severity issues can miss OpenCode updates or leave deleted sessions active; medium-severity issues weaken bounded coverage and recovery.

High

  • cmd/agentsview/unwatched_poll.go:335 — Refreshing an existing poll-owned lease does not set pendingWake. After the initial drain, the generic polling scope is excluded, so subsequent ticks perform no bounded or fallback work and degraded OpenCode polling stops detecting changes.
    Fix: Mark existing non-native, poll-owned states pending on every polling refresh.

  • cmd/agentsview/unwatched_poll.go:267 — If admission overlaps a running coverage pass, the code waits for completion and then skips the binding. Because the caller removes the corresponding path from ordinary processing, a final journal event can remain unsynchronized indefinitely.
    Fix: Retry admission for the same binding after passDone, or retain its original path unless admission succeeds.

  • internal/sync/opencode_bounded_coverage.go:491 — The authoritative repair synchronizes only sources currently present in the container and does not tombstone archived sessions that were deleted, despite advancing past the structural event.
    Fix: Perform lease-scoped authoritative reconciliation against complete container membership and tombstone missing sessions.

Medium

  • cmd/agentsview/unwatched_poll.go:746checkpointAfterBoundedAudit retains invalid cursor anchors. If all anchors disappear, every subsequent drain detects the same anomaly and may repeatedly launch up to 32 repair workers without rebaselining.
    Fix: Replace invalid anchors with a freshly verified post-audit boundary.

  • cmd/agentsview/main.go:350 — Successfully admitted database/WAL paths are still appended to ordinary watcher processing. Existing admissions are also reported as admitted, so each event retains the O(session-count) watermark scan that bounded journal coverage was meant to eliminate.
    Fix: Route successfully admitted paths exclusively through the bounded feed; reserve ordinary processing for unadmitted or failed bindings.

  • cmd/agentsview/main.go:2131 — Polling scopes are rewritten from the provider root to the database file. If that database is deleted while native watching is unavailable, the existence check fails and authoritative reconciliation never runs, leaving sessions from the removed container active.
    Fix: Track the provider reconciliation root separately from the bounded-feed path and reconcile that root when the database disappears.


Reviewers: 2 done | Synthesis: codex, 15s | Total: 10m5s

@roborev-ci

roborev-ci Bot commented Aug 3, 2026

Copy link
Copy Markdown

roborev: Combined Review (800cc91)

High-severity correctness issues could lose events and leave deleted sessions stale; one medium issue can cause repeated audits.

High

  • cmd/agentsview/unwatched_poll.go:267 — When a coverage pass is active, admission waits for it and then continues to the next binding, silently skipping the current binding. Because the watcher subsequently removes that database path from normal processing, events beyond the running pass’s high-water can be lost indefinitely.

    • Fix: After passDone closes, retry admission for the same binding; add a concurrency test covering an event arriving during an active drain.
  • internal/sync/opencode_bounded_coverage.go:491 — Structural repair syncs only existing sources without authoritative membership reconciliation or tombstoning. A deleted session is absent from sources, so its archived row remains stale even as the checkpoint advances past the deletion.

    • Fix: Make lease-bound repair authoritatively reconcile the exact physical container, including tombstones for missing members, while preserving the lease identity fence.

Medium

  • cmd/agentsview/unwatched_poll.go:741 — After an audit caused by all cursor anchors disappearing, the checkpoint retains invalid anchors. With no replacement high-water anchor, every subsequent drain repeats the missing-witness audit and prevents normal journal draining.
    • Fix: Rebaseline the checkpoint against the journal’s post-audit high-water and replace invalid anchors; test complete-anchor-deletion recovery through the coordinator.

Reviewers: 2 done | Synthesis: codex, 11s | Total: 7m4s

@roborev-ci

roborev-ci Bot commented Aug 3, 2026

Copy link
Copy Markdown

roborev: Combined Review (1b29b2e)

Code review found three medium-severity correctness and performance regressions; no security issues were identified.

Medium

  • internal/sync/engine.go:4620 — Incorrect reconciliation identity

    Reconciliation ignores SourceRef.ReconciliationIdentity and passes source paths to ReconciliationMemberIdentity, whose contract expects a full session ID. Because OpenCode does not implement that resolver, duplicate logical sessions across roots are keyed by path rather than session ID, bypassing canonical-source ranking and potentially allowing a stale duplicate to overwrite the preferred session.

    Fix: Use source.ReconciliationIdentity for discovered candidates, preserve OpenCode’s raw-session-ID extraction, and call ReconciliationMemberIdentity only with stored full session IDs.

  • cmd/agentsview/main.go:350 — Bounded paths still trigger archive-sized synchronization

    Paths successfully admitted to bounded coverage are added back to remaining, causing every OpenCode database/WAL event to run SyncPathsContext. This streams the entire SQLite session watermark set before the bounded journal drain, defeating the intended bounded recurring workload.

    Fix: Keep successfully admitted paths out of the ordinary watcher batch. The existing retry error already restores the original paths when admission fails.

  • cmd/agentsview/main.go:2859 — Optional root creation may remain uncovered

    Missing optional roots are removed from missingRoots before the watcher confirms lifecycle coverage. If the OpenCode container’s shallow watch fails while storage/ is absent, neither lifecycle watching nor polling covers its later creation, since the degraded obligation is narrowed to the SQLite database. Storage-backed sessions can therefore be missed.

    Fix: Retain optional missing roots as pending metadata and suppress polling only after MissingRootLifecycleOwned confirms native coverage.


Reviewers: 2 done | Synthesis: codex, 13s | Total: 17m40s

@mariusvniekerk

Copy link
Copy Markdown
Collaborator

This looks rather chonky? Do we need all of this?

@rodboev

rodboev commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

Yeah, it's a complex feature and evolved to be more than a parser change. To get it to be lossless, deletion-correct and safe across retries and replacement races it crosses across the parser, provider/watchers, engine, and coordinator. It can be split roughly across those lines:

  • Parser journal feed: 1,200 lines of code, 1,200 lines tests.
  • Provider watch: 200 lines code, 500 lines tests.
  • Engine lease and reconciliation: 1,200 lines code, 500 lines tests.
  • Coordinator integration: 1,000 lines code, 500 lines tests.

That may be easier to review but they all work together, and the coordinator depends on the other three.

@rodboev

rodboev commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

Closing this since a closer inspection shows that it doesn't hold up on the premise that OpenCode exposes a producer-owned cross-drain journal cursor that could be turned into a bounded feed. Since it does not, the feed slice collapses and the rest of the split doesn't make sense since it's scaffolding for that feed. The reconciliation slice has no production caller, the coordinator was always deferred, and only the watch-topology piece fixes a real production failure. That one is going out standalone and will supersede #1308.

@rodboev rodboev closed this Aug 6, 2026
wesm pushed a commit that referenced this pull request Aug 9, 2026
…get (#1355)

OpenCode's SQLite container, its WAL, and its storage archive share one recursive watch unit over the configured root, so the live container draws on the same shared recursive budget as the archive. A root reached after that budget is spent registers no native watch at all, and exhaustion inside the root's own walk marks the container's coverage degraded along with the archive's.

This separates each OpenCode-family root into a shallow container unit covering the database and its WAL and a recursive unit at `<root>/storage`. A shallow watch never draws on the recursive budget, so the container's coverage no longer depends on the archive's size. The split applies only where it is representable: a root with no storage directory keeps the single recursive unit, because naming an absent watch root would plan a polling obligation probed on a path that may never appear, and a probe that cannot be satisfied defers every other obligation on the same configured dir. A symlinked root also keeps it, since the daemon refuses to watch a recursive root through a symlink and gates the configured dir's reconciliation on the link target instead.

A native watch shared by overlapping roots is now charged to the shared budget once. `RecursiveWatchResult.Watched` still counts the directories a root covers; `Allocated` counts the watches a registration installed, and registration subtracts that. Reuse is settled before the budget check, so a root whose directories are already watched registers cleanly against an exhausted budget and records its ownership, and reclaiming a shared watch returns one budget slot rather than one per root that walked it.

Two units share one configured root and the engine dispatches every changed path once per emitted watch root, so `unitScopeAllows` scopes classification to the units of the root that owns the path; without it a WAL write would run the shared container's whole session listing twice. Configured roots can nest, so the deepest one containing the path owns it, and a virtual member path resolves to its physical container before the check. The container unit still claims storage paths: whether the storage unit exists is a filesystem fact, while the engine resolves each provider's watch roots once and reuses that set, so deferring would leave a storage tree created afterwards claimed by nothing. That costs one repeated source lookup per storage event, which the engine's per-pass source set discards; the database fan-out cannot double, because a storage watch root never contains the database or its WAL.

This PR stands alone and supersedes PR #1318, whose conditional storage-unit split covered the same provider topology. Portable lifecycle ownership of a missing watch root is not attempted here. The bounded parser feed that motivated PR #1331 is abandoned rather than shipped: OpenCode exposes no producer-owned cross-drain journal cursor, so that work was closed and this slice is the salvage.

Refs #1208


Co-authored-by: Rod Boev <rodboev@users.noreply.github.com>
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.

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

2 participants