Keep local sync running while a push runs by cpcloud · Pull Request #2303 · kenn-io/agentsview

cpcloud · 2026-10-16T18:19:05Z

TL;DR

A push --watch process that owned the archive stopped ingesting sessions while a remote push ran, so an hour-long full push kept new sessions out of the archive and the UI for an hour. Local sync and pushes now run side by side, in the CLI watch and in the daemon, through one shared push path.

The benefit is user-facing. New sessions reach the archive and the UI while a push runs.

How

Pushes run without the sync lock. A push calls Engine.SettleForPush to wait for any running sync, then copies the archive while watcher batches and interval syncs keep writing.
Each push reads its cutoff under the archive write lock. DB.MirrorPushCutoff holds that lock while it reads the clock, and waits while the daemon has handed its writer to a sync worker process. No write is in flight at that moment, so each write lands either before the cutoff and goes in this push, or after it and goes in the next one. ClickHouse, PostgreSQL, and DuckDB pushes all read it this way.
One push path. internal/archivepush runs friction review, pricing refresh, connect, schema setup, and the push for both the daemon push routes and the CLI. Both CLI watch modes share one watch runner, whether the CLI opened the archive or the daemon owns it.
The daemon runs one push at a time. A push takes its turn before its own sync, so one push's archive rebuild never replaces the archive another push is reading. A push that has to wait tells the CLI, and stops waiting if its request ends.
Pushes wait for one complete local sync. A full push deletes remote rows that are missing locally, so it must not read an archive that a failed startup sync left incomplete. A failed startup sync retries, including any archive rebuild it still needs. The first retry waits one change debounce, and each later wait doubles, up to the floor interval. The first push is full when the user asked for --full or when the archive was rebuilt since the watch started.

Reviewer notes

A /resync or compaction can still replace the archive during a push. The rebuilt archive starts without push bookmarks, so the next push runs full. For PostgreSQL, an archive with no bookmark whose push marker is already in the database pushes in full. A PostgreSQL push saves its bookmarks in one archive transaction that first checks the archive is the one it read, and fails if it is not. Those saves wait for the writer while the daemon has handed it to a sync worker.
A failed full push retries as full only if it never recorded a full push to resume, such as a connect failure, an unreachable daemon, a failed daemon sync before the copy, or an early PostgreSQL, ClickHouse, or DuckDB error. Daemon push errors say which, and the daemon reports when the copy starts, so a dropped stream also tells. A push that rebuilt the archive counts as started, because every mirror pushes in full after a rebuild; otherwise each retry would rebuild the archive again.
Friction review still takes the sync lock briefly before each push to a replica that stores friction findings.
Shutdown stops the file watcher, then syncs once before the final push. A watcher batch can hold the sync lock, and the final push waits for that lock. The sync picks up changes the watcher had not handed off. Stopping during the first push runs the sync but skips a second push.
Every push refreshes model pricing first. One-shot DuckDB pushes and daemon mirror pushes used to skip it.
An interval sync that wrote nothing no longer queues a push. The push loop still runs its own interval push.
Look closest at the cutoff callers in each backend and at archiveSyncLane in cmd/agentsview/archive_sync_lane.go, which decides when a push may run.

🤖 Generated with Claude Code

A push --watch process that owns the archive stopped ingesting session files for as long as a remote push ran. The startup push held watcher dispatch closed, and every later push ran its local sync and the remote copy as one step under the sync lock. A full push can take an hour, so new sessions stayed out of the local archive, and out of the UI, for that long. Local ingestion now runs beside the push. Watcher batches, the interval sync, and lifecycle wakes write the archive at once, and each push copies whatever the archive holds when it starts. This covers the ClickHouse, PostgreSQL, and DuckDB local owners. The daemon-delegated owners are unchanged; the daemon still holds its sync lock for the whole push. Running sync during a push opens a race in incremental pushes. A push takes its cutoff before it lists sessions. A sync transaction that stamps sync_marker before the cutoff but commits after the listing would sit below every later window and never reach the remote. Each push therefore records its start time while no sync transaction is open, and afterwards moves sessions marked since then up to now. This writes sync_marker on archive rows, as annotation writes already do. Moving the fix into the mirror push code would avoid that write, but another change owns that code. Pushes wait until one complete local sync has run, because a full push deletes remote rows that are missing locally. When startup sync fails, watcher dispatch stays closed and pushes are skipped until the interval sync succeeds. A full push requested by a startup resync goes to the first push that runs. If that push fails, the mirror resumes the unfinished full push on the next push, so the retry does not ask for a full push again, which would start it over. The sync step that used to run before each push also reviewed stale friction findings and woke the embedding scheduler. A replica that stores friction now reviews them under the sync lock before each push, and a watch with --embed starts the scheduler before each push. Generated with Claude Code (claude-opus-5-5) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Now that local sync keeps writing while a push runs, an incremental push could miss a session. A push records a cutoff, lists sessions changed since the last cutoff, and stores the new cutoff for the next push. A sync transaction could stamp a session just before the cutoff and commit just after the listing. That session was then older than every later cutoff, so no later incremental push sent it. The previous commit worked around this by restamping sessions after each push, which also rewrote sync_marker on archive rows and forced usage-cache recompute for them. The cutoff is now read while holding the archive write lock. No write transaction is open at that moment, so every earlier stamp has committed and every later stamp is at or after the cutoff. All ClickHouse, PostgreSQL, and DuckDB pushes, including a DuckDB rebuild, take the cutoff this way. A push waits for an open sync transaction before it starts, and writers wait only for the clock read, not for the listing. The interval sync also stops queueing a push when it wrote nothing. The push loop's own interval push still runs, so anything a sync does not count still reaches the remote on that schedule. The first complete sync after a failed startup still queues the push that startup skipped. Generated with Claude Code (claude-opus-5-5) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

When the startup sync failed, the lane waited a full floor interval (15 minutes by default) before trying again, and remote pushes stayed paused the whole time. A transient discovery error at startup cost a quarter hour of pushes. The lane now retries on the change debounce until one sync completes, then falls back to the interval. The logs now say that remote pushes are paused and why, so an operator reading them does not assume the push itself is broken. Two tests could not fail: one asserted that a window error skips the connect step, which no production window returns, and one compared the push cutoff against a stamp taken a minute in the past. Both are gone; the remaining assertion still fails when the cutoff ignores the write lock. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

A filtered PostgreSQL push lists sessions that moved out of the selected projects before it reads the push cutoff. A sync that moved a session out of scope could commit between that listing and the cutoff. Its stamp then sat below the cutoff, so this push did not delete the session from PostgreSQL and no later push looked at it again. The remote kept a session the filter had excluded. The push now reads the cutoff before any listing, so every write that the listings miss stamps at or after the cutoff and reaches the next push. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

The daemon's push routes and a CLI push that opened the archive itself each had their own copy of the push steps: friction review, pricing refresh, replica connect, schema setup, and the push. The copies had drifted. The daemon held the sync lock for the whole push, so local ingestion stopped while a remote push ran, and some paths skipped the pricing refresh. Both now call the same archivepush package. A push waits for any sync in progress to settle and then runs without the sync lock, so ingestion keeps writing during the push. Friction review still needs the sync lock, so the shared path takes it briefly before each push to a replica that stores friction. The two CLI watch modes (archive opened locally, or archive owned by the daemon) also share one watch runner now, so a fix to one cannot miss the other. The full-vector repair state of a local watch moved into the vector reconcile state that both modes already share. Shutting down a watch could hang. The final push after cancel waited on the file watcher, which was still delivering events and queueing more work. The watcher now stops as soon as the context is cancelled, before the final push runs. Visible changes: - One-shot DuckDB pushes and daemon mirror pushes refresh model pricing first, as the other pushes already did. - Daemon schema errors start with "schema:", and a failed startup push logs as "push (startup) failed", matching the local watch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

After the shared push path landed, the watch code still repeated three rules, so a fix to one copy could miss the other: - A daemon watch reuses its backend for the startup push and resolves the archive owner again for each later push. The DuckDB and replica watches each had their own copy. - A replica watch picks the vector scope for each push and records the outcome for the next one. The local and daemon watches each did this, and differ only in how the push is sent. - The DuckDB and replica watches had separate test hooks for the startup sync and each picked the default themselves. The watch runner now picks it. TestLocalPushWatchStopsWatcherBeforeFinalPush failed now and then. It cancelled the watch while the startup push was still running, and a watch cancelled then skips the final push. The test now waits until the push loop is idle before it cancels. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Review of the push decoupling found that the daemon no longer ordered pushes. Two watch processes could run archive pushes against the daemon at the same time. The daemon now gives pushes one turn each, and a push that waits for its turn stops when its request ends. A local watch with --full lost the full push when the startup sync failed, because only a successful rebuild requested one. The first push now resends every session when the user asked for --full, whatever the startup sync did. A daemon watch retried a failed startup push against the address the watch started with. A daemon that restarted on a new address left the watch retrying a dead address until the next interval push. Only the first push now reuses the launch connection. Watch logs again say whether a replica push failed while connecting or while copying, and that a partial push will retry. The paused-push lines now say that remote pushes wait for a local sync that finishes without errors, and they stop saying pushes are paused once pushes are open. Tests no longer read goroutine stacks to learn when the push loop is idle; the loop reports that through a hook. The stack check that remains waits for a goroutine parked on a mutex, which Go has no API for, and now lives in one shared test helper. The context-cancel test for SettleForPush now cancels only after the call waits on the lock, so it cannot pass by accident. Reviewed and left as is: local sync retries after a failed sync keep their fixed debounce with no backoff. A retry after a failed full watch push already sends full=false, and shutdown already stops the watcher before the final push. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Review of the push decoupling found ways a push could miss a session or keep rows the local archive no longer has. A daemon sync can run in a child process that writes the archive while the daemon's own writer is closed. A push that took its cutoff then could miss a write the child committed after the push listed sessions, and every later push would skip it. The cutoff now waits until the daemon has its writer back. Waiting on the engine sync lock instead would deadlock the one-shot push and the DuckDB rebuild, which already hold that lock while they push. A second daemon push could rebuild or resync the archive while the first push still read it, because pushes took their turn only after their own sync. Pushes now take their turn first. A /resync or compaction can still replace the archive during a push, so a DuckDB push now records the archive it started from, as ClickHouse and PostgreSQL already did, and the next push rebuilds the mirror. A local push watch whose startup sync failed retried with a plain sync, so a rebuild the archive still needed never ran. A rebuild that swapped in a new archive but also hit parse failures reported no rebuild, so the first push was incremental and kept remote rows the rebuild dropped. The watch now retries the full startup sync, and makes the first push full whenever the archive's database ID changed since the watch started. A --full request was used up by a push that never reached its destination, such as a connect failure or an unreachable daemon. The watch now keeps it for those failures. A push that started and failed still clears it, because the mirror resumes that push. A daemon push that waits for another push now tells the client, and the CLI prints one line saying so. Reviewed and left as is: watch push failure logs keep main's prefixes, as decided in the previous review round. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Stopping a local push watch skipped files that changed after the last local sync. The watcher stops first, and the final push only waited for a running sync, so this watch exited without archiving or pushing those files. On main the final push synced first. The shutdown push now runs one local sync again, within the existing shutdown timeout. A daemon watch started with --full still lost the request when the daemon accepted the push and then failed its own sync or rebuild. The CLI could not tell that failure from a copy that failed partway, so it cleared the request and the retry was incremental. Push error bodies now say whether the copy started, and the CLI keeps --full when it did not. A daemon that cannot stream sends the same field, so a push that failed partway there resumes instead of restarting. Older daemons omit the field; the CLI then keeps its previous guess for each case. A startup sync that keeps failing now retries less often. Each retry can rebuild the whole archive, and a missing session folder could make the watch rebuild every 30 seconds forever. The wait now doubles after each failure, up to the floor interval. The progress-hook test pushed twice to check that the hook is restored, but a missing restore still produces the same stages, so it now pushes once and checks only that the caller's hook fires. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Review found more ways a daemon push could keep stale rows, fail, or loop now that pushes run outside the sync lock. PostgreSQL never forced a full push when the local archive was rebuilt. A /resync or compaction during a push mixed rows from two archives, and the push then saved a bookmark that hid rows the rebuild had written. PostgreSQL now records the archive's database ID it pushed, pushes in full when that ID changes, and fails a push whose archive changed under it before the push saves its bookmark. The previous round assumed PostgreSQL already did this; it did not. A foreground sync or the daily audit closes the daemon's writer while a worker process runs. A PostgreSQL push that saved its progress then failed with "writer closed". Those saves now wait for the writer and try again. PostgreSQL and ClickHouse now report a --full push that failed before it recorded a run it could resume, such as a failed data-version check, so the watch keeps --full for the retry. A daemon push that rebuilt the archive counts as started even if it then failed to connect: every mirror pushes in full after a rebuild, and keeping --full made the daemon rebuild the archive on every retry while the destination was down. Stopping a local watch during its first push now runs the final local sync, as a normal shutdown does, but skips a second push, so the watch still exits within the shutdown timeout. The final sync says it is running, and a timeout says the next run will pick the changes up. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…tarted The previous commit made PostgreSQL record the archive's database ID and push in full when it changed. A rebuild does not carry that ID, or the push bookmark, into the new archive, so the check never fired. A rebuilt archive does keep this archive's push marker in PostgreSQL. Now an archive with no push bookmark but a marker in PostgreSQL pushes in full, which removes rows the rebuild dropped. Two tests that cleared the bookmark to reach the fingerprint shortcut now also remove the marker, since a cleared bookmark alone means a full push. PostgreSQL saved the bookmark, boundary state, and target fingerprint one write at a time after checking the database ID. A rebuild could land between the check and the writes. The save now runs in one archive transaction that checks the ID first. The save after a failed full push also has a 30-second limit, because it waits for an archive writer that a stopping daemon may never reopen. A DuckDB --full push that failed before its rebuild recorded a run, for example because another push held the rebuild lock, did not say so. The watch then dropped --full. DuckDB now reports these failures as not started, as PostgreSQL and ClickHouse do, and so does a DuckDB watch push the daemon deferred. When the stream from the daemon dropped, the CLI could not tell a push still waiting for its turn from one that had started copying, so it cleared --full. The daemon now sends a copying report just before the copy. Phase reports skip the progress throttle, since a waiting report within 200 ms would otherwise hide the copying one. A stream that ends before that report keeps --full. Daemons from before this change never send it, so a dropped stream from one also keeps --full, which costs at most one extra full push. A push that waits for the local sync to hand the archive writer back now logs that it is waiting after five seconds and says so if it is stopped. The rebuild and final-sync messages now say what happens next. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

roborev-ci · 2026-10-17T22:53:48Z

roborev: Combined Review (`40b438f`)

Verdict: Changes require fixes for 2 findings.

Medium

internal/server/huma_routes_push.go:576: The copying event is emitted before destination preparation or durable full-run creation, but the client treats it as proof the push started. If a --full resync falls back to incremental sync without replacing the archive, a disconnect during preparation clears fullPending, allowing retries to skip the requested full repair.

Fix: Keep copying progress informational and preserve --full until archive replacement or durable resumable-run creation is acknowledged. Add regression coverage for disconnection during preparation.
internal/postgres/push.go:1256: On the archive's first PostgreSQL push, a worker handoff can close the writer after the push releases the sync lock. Global marker initialization calls GetOrCreateSyncState directly on s.local, bypassing handoff handling, so ErrWriterClosed aborts an otherwise healthy push.

Fix: Initialize the global push marker through an unscoped handoffSyncState or wrap its creation with retryAcrossHandoff.

Reviewers: 3x codex, codex (security) | Synthesis: codex, 7s | Total: 12m47s

rodboev · 2026-10-18T01:45:40Z

Flaky CI job: CI / run / Go Test (race detector, Other packages)
Repair PR: #2315

Navigation Menu

Repository navigation

Keep local sync running while a push runs - #2303

Keep local sync running while a push runs#2303
cpcloud wants to merge 11 commits into
mainfrom
push-decouple-local-sync

cpcloud commented Oct 16, 2026 •

edited

Loading

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

roborev-ci Bot commented Oct 17, 2026

Uh oh!

rodboev commented Oct 18, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

Conversation

cpcloud commented Oct 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TL;DR

How

Reviewer notes

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

roborev-ci Bot commented Oct 17, 2026

roborev: Combined Review (40b438f)

Medium

Uh oh!

rodboev commented Oct 18, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

cpcloud commented Oct 16, 2026 •

edited

Loading

roborev: Combined Review (`40b438f`)