Skip to content

Commit e6fa88c

Browse files
fix(duckdb-watch): tombstone deletions on unwatched/deferred roots (#31)
* fix(duckdb-watch): add unwatched-root poller for local watch mode Local duckdb watch omitted the probe-gated authoritative poller that pg watch uses for deferred/unwatched roots. Deletions under those scopes were never tombstoned in SQLite or the DuckDB mirror because interval pushes only run SyncAll, which does not prove missed deletions. Mirror pg watch: share one sync engine with the watcher, register polling obligations for unwatched roots, and reconcile them via ReconcileWatchRoots. * refactor(duckdb-watch): address PR review nits — comment + shared test helper
1 parent 45dac3d commit e6fa88c

3 files changed

Lines changed: 288 additions & 59 deletions

File tree

cmd/periscope/archive_write_backend.go

Lines changed: 121 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,10 @@ type archivePushWatchHooks struct {
7575
context.Context, *syncpkg.Engine, bool,
7676
) (bool, error)
7777
newPGPusher func(*syncpkg.Engine) *pgPusher
78+
newDuckDBPusher func(*syncpkg.Engine) *duckDBPusher
79+
duckDBStartupSync func(
80+
context.Context, *syncpkg.Engine, bool,
81+
) (bool, error)
7882
newUnwatchedPoller func(context.Context, unwatchedPollSyncer) unwatchedRootPoller
7983
}
8084

@@ -87,7 +91,7 @@ type unwatchedRootPoller interface {
8791
Stop()
8892
}
8993

90-
// newArchivePushUnwatchedPoller builds the pg watch polling owner for
94+
// newArchivePushUnwatchedPoller builds the pg/duckdb watch polling owner for
9195
// deferred scopes. The watcher's full recovery and rename promotion defer
9296
// unavailable scopes to their polling probes, and the interval push runs a
9397
// plain SyncAll that never tombstones missed deletions, so without this owner
@@ -723,53 +727,143 @@ func (b *localArchiveWriteBackend) DuckDBPushWatch(
723727
if debounce <= 0 {
724728
debounce = defaultWatchDebounce
725729
}
726-
push := func(pctx context.Context, reason pushReason, full bool) error {
727-
pushCfg := cfg
728-
pushCfg.Full = full
729-
// Watch pushes are automatic: a mirror held by a live serve
730-
// process defers instead of rebuilding the whole archive on
731-
// every changed batch, and archive-scale diagnostics are
732-
// skipped. Push ignores the defer behavior when full is set.
733-
pushCfg.Automatic = true
734-
var res duckdbsync.PushResult
735-
var err error
736-
if b.watchHooks != nil && b.watchHooks.duckDBPush != nil {
737-
res, err = b.watchHooks.duckDBPush(pctx, reason, full)
738-
} else {
739-
res, err = b.DuckDBPush(
740-
pctx, duckCfg, pushCfg, projects, exclude,
741-
)
742-
}
743-
if err != nil {
744-
return err
730+
for _, def := range parser.Registry {
731+
if !b.appCfg.IsUserConfigured(def.Type) {
732+
continue
745733
}
746-
return completeDuckDBWatchPush(res, reason)
734+
warnMissingDirs(b.appCfg.ResolveDirs(def.Type), string(def.Type))
747735
}
736+
cleanResyncTemp(b.appCfg.DBPath)
737+
738+
engine := syncpkg.NewEngine(b.database, syncpkg.EngineConfig{
739+
AgentDirs: b.appCfg.AgentDirs,
740+
IncludeCwdPrefixes: b.appCfg.SyncIncludeCwdPrefixes,
741+
Machine: b.appCfg.LocalMachineName,
742+
BlockedResultCategories: b.appCfg.ResultContentBlockedCategories,
743+
})
744+
defer engine.Close()
745+
746+
var pusher *duckDBPusher
747+
if b.watchHooks != nil && b.watchHooks.newDuckDBPusher != nil {
748+
pusher = b.watchHooks.newDuckDBPusher(engine)
749+
} else {
750+
pusher = b.newDuckDBPusher(
751+
func(c context.Context) error {
752+
stats := engine.SyncAll(c, nil)
753+
if err := c.Err(); err != nil {
754+
return err
755+
}
756+
if !stats.AuthoritativeDiscoveryComplete() {
757+
return errors.New("local sync discovery incomplete")
758+
}
759+
engine.FlushSignals()
760+
return nil
761+
},
762+
duckCfg, projects, exclude,
763+
)
764+
}
765+
766+
fmt.Printf(
767+
"periscope duckdb watch: pushing to DuckDB "+
768+
"(debounce %s, floor %s)\n",
769+
debounce, interval,
770+
)
771+
748772
loop, stopLoop := newArchivePushLoop(
749773
b.watchHooks,
750774
"duckdb watch", debounce, interval,
751775
func(c context.Context, r pushReason) error {
752-
return push(c, r, false)
776+
pushCfg := cfg
777+
pushCfg.Automatic = true
778+
if b.watchHooks != nil && b.watchHooks.duckDBPush != nil {
779+
res, err := b.watchHooks.duckDBPush(c, r, false)
780+
if err != nil {
781+
return err
782+
}
783+
return completeDuckDBWatchPush(res, r)
784+
}
785+
return pusher.push(c, r, false, pushCfg)
753786
},
754787
)
755788
defer stopLoop()
756789

790+
poller := newArchivePushUnwatchedPoller(ctx, b.watchHooks, engine)
791+
defer poller.Stop()
792+
757793
stopWatcher, openDispatch, unwatchedDirs := startArchivePushWatcher(
758-
b.watchHooks, b.appCfg, nil,
794+
b.watchHooks, b.appCfg, engine,
759795
func(callbackCtx context.Context, batch syncpkg.WatchBatch) error {
796+
scope := func() watchRecoveryScope {
797+
return probeWatchRecoveryScope(b.appCfg)
798+
}
799+
if err := syncWatchBatch(callbackCtx, engine, batch, scope); err != nil {
800+
return err
801+
}
760802
return notifyPushForWatchBatch(callbackCtx, loop, batch)
761803
},
762-
syncpkg.WatcherOptions{OnCoverageDegraded: loop.NotifyCoverageDegraded},
804+
syncpkg.WatcherOptions{
805+
OnCoverageDegraded: func(roots []string) error {
806+
if err := poller.AddObligation(pollingObligation{
807+
Key: "watcher-fallback", Roots: roots,
808+
}); err != nil {
809+
return err
810+
}
811+
return loop.NotifyCoverageDegraded(roots)
812+
},
813+
OnPollingRequired: func(obligation syncpkg.PollingObligation) error {
814+
return poller.AddObligation(pollingObligation{
815+
Key: obligation.Key,
816+
Roots: obligation.Roots,
817+
Probe: obligation.Probe,
818+
})
819+
},
820+
OnPollingReleased: poller.RemoveObligation,
821+
},
763822
)
764823
defer stopWatcher()
765824
if len(unwatchedDirs) > 0 {
766825
log.Printf(
767-
"duckdb watch: %d root(s) not watched; relying on the %s floor for coverage",
768-
len(unwatchedDirs), interval,
826+
"duckdb watch: %d root(s) not watched; polling every %s",
827+
len(unwatchedDirs), unwatchedPollInterval,
769828
)
770829
}
771-
initialErr := push(ctx, reasonStartup, cfg.Full)
830+
831+
startupSync := runPGWatchStartupSync
832+
// DuckDB watch shares the same SyncAll-based startup path as pg watch;
833+
// runPGWatchStartupSync is the shared default when no duckDBStartupSync
834+
// hook overrides it (pgStartupSync is only a test harness fallback).
835+
if b.watchHooks != nil && b.watchHooks.duckDBStartupSync != nil {
836+
startupSync = b.watchHooks.duckDBStartupSync
837+
} else if b.watchHooks != nil && b.watchHooks.pgStartupSync != nil {
838+
startupSync = b.watchHooks.pgStartupSync
839+
}
840+
didResync, startupErr := startupSync(ctx, engine, cfg.Full)
841+
if startupErr != nil && errors.Is(startupErr, context.Canceled) {
842+
return nil
843+
}
844+
initialErr := startupErr
845+
if initialErr == nil {
846+
pushCfg := cfg
847+
pushCfg.Automatic = true
848+
if b.watchHooks != nil && b.watchHooks.duckDBPush != nil {
849+
res, err := b.watchHooks.duckDBPush(
850+
ctx, reasonStartup, cfg.Full || didResync,
851+
)
852+
if err != nil {
853+
initialErr = err
854+
} else {
855+
initialErr = completeDuckDBWatchPush(res, reasonStartup)
856+
}
857+
} else {
858+
initialErr = pusher.push(
859+
ctx, reasonStartup, cfg.Full || didResync, pushCfg,
860+
)
861+
}
862+
}
772863
if initialErr != nil {
864+
if errors.Is(initialErr, context.Canceled) && ctx.Err() != nil {
865+
return nil
866+
}
773867
log.Printf("duckdb watch: initial push failed: %v", initialErr)
774868
}
775869
completePushWatchStartup(ctx, initialErr, loop, openDispatch)

cmd/periscope/archive_write_backend_test.go

Lines changed: 95 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -810,15 +810,21 @@ func (noopPGTarget) PushWithOptions(
810810
func (noopPGTarget) Close() error { return nil }
811811

812812
// The watcher's full recovery and rename promotion defer unavailable scopes
813-
// to their polling probes, and pg watch's interval push is a plain SyncAll
814-
// that never tombstones missed deletions. Local pg watch must therefore own
815-
// those deferred scopes with a probe-gated authoritative poller: a root the
816-
// watcher cannot cover (here a symlinked recursive root, the same obligation
817-
// machinery that owns roots missing at startup on portable backends)
818-
// registers a polling obligation, and the poller reconciles its scope
819-
// authoritatively on ticks — with no watcher event and no floor push
820-
// involved.
821-
func TestLocalPGPushWatchGivesDeferredScopesAPollingOwner(t *testing.T) {
813+
// to their polling probes, and local push-watch interval pushes are a plain
814+
// SyncAll that never tombstones missed deletions. Local pg/duckdb watch must
815+
// therefore own those deferred scopes with a probe-gated authoritative poller.
816+
type localPushWatchPollingOwnerSpec struct {
817+
obligationMsg string
818+
exitMsg string
819+
sessionUUID string
820+
wireHooks func(*archivePushWatchHooks)
821+
runWatch func(context.Context, *localArchiveWriteBackend) error
822+
}
823+
824+
func testLocalPushWatchGivesDeferredScopesAPollingOwner(
825+
t *testing.T, spec localPushWatchPollingOwnerSpec,
826+
) {
827+
t.Helper()
822828
dataDir := t.TempDir()
823829
dbPath := filepath.Join(dataDir, "sessions.db")
824830
database := dbtest.OpenTestDBAt(t, dbPath)
@@ -856,15 +862,6 @@ func TestLocalPGPushWatchGivesDeferredScopesAPollingOwner(t *testing.T) {
856862
label: label,
857863
}, func() {}
858864
},
859-
pgStartupSync: func(context.Context, *syncpkg.Engine, bool) (bool, error) {
860-
return false, nil
861-
},
862-
newPGPusher: func(*syncpkg.Engine) *pgPusher {
863-
return &pgPusher{
864-
localSync: func(context.Context) error { return nil },
865-
connect: func() (pgTarget, error) { return noopPGTarget{}, nil },
866-
}
867-
},
868865
newUnwatchedPoller: func(
869866
ctx context.Context, engine unwatchedPollSyncer,
870867
) unwatchedRootPoller {
@@ -876,41 +873,37 @@ func TestLocalPGPushWatchGivesDeferredScopesAPollingOwner(t *testing.T) {
876873
)
877874
},
878875
}
876+
spec.wireHooks(backend.watchHooks)
879877

880878
ctx, cancel := context.WithCancel(context.Background())
881879
t.Cleanup(cancel)
882880
done := make(chan error, 1)
883-
go func() {
884-
done <- backend.PGPushWatch(
885-
ctx, pgTargetSelection{}, PGPushConfig{}, nil, nil,
886-
time.Hour, time.Hour,
887-
)
888-
}()
881+
go func() { done <- spec.runWatch(ctx, backend) }()
889882

890883
select {
891884
case roots := <-owned:
892-
assert.Contains(t, roots, codexRoot,
893-
"the unwatchable root's polling obligation must reach the pg watch poller")
885+
assert.Contains(t, roots, codexRoot, spec.obligationMsg)
894886
case err := <-done:
895-
t.Fatalf("pg watch exited before registering obligations: %v", err)
887+
t.Fatalf("watch exited before registering obligations: %v", err)
896888
case <-time.After(10 * time.Second):
897889
t.Fatal("no polling obligation was registered for the unwatchable root")
898890
}
899891

900892
// A session lands under the unwatched scope; only the poller's
901893
// authoritative tick can bring it into the archive.
902-
uuid := "d4e5f6a7-4444-4555-8666-777788889999"
903894
day := filepath.Join(codexRoot, "2026", "05", "04")
904895
require.NoError(t, os.MkdirAll(day, 0o755))
905896
content := testjsonl.NewSessionBuilder().
906897
AddCodexMeta(
907-
"2026-05-04T14:00:00Z", uuid, "/home/user/code/api",
898+
"2026-05-04T14:00:00Z", spec.sessionUUID, "/home/user/code/api",
908899
"codex_cli_rs",
909900
).
910901
AddCodexMessage("2026-05-04T14:00:01Z", "user", "hello").
911902
String()
912903
require.NoError(t, os.WriteFile(
913-
filepath.Join(day, "rollout-2026-05-04T14-31-58-"+uuid+".jsonl"),
904+
filepath.Join(
905+
day, "rollout-2026-05-04T14-31-58-"+spec.sessionUUID+".jsonl",
906+
),
914907
[]byte(content), 0o644,
915908
))
916909

@@ -919,7 +912,9 @@ func TestLocalPGPushWatchGivesDeferredScopesAPollingOwner(t *testing.T) {
919912
case ticks <- time.Now():
920913
default:
921914
}
922-
session, err := database.GetSession(context.Background(), "codex:"+uuid)
915+
session, err := database.GetSession(
916+
context.Background(), "codex:"+spec.sessionUUID,
917+
)
923918
return err == nil && session != nil
924919
}, 10*time.Second, 20*time.Millisecond,
925920
"the returned root must be reconciled by the poller without watcher events or floor pushes")
@@ -929,10 +924,78 @@ func TestLocalPGPushWatchGivesDeferredScopesAPollingOwner(t *testing.T) {
929924
case err := <-done:
930925
require.NoError(t, err)
931926
case <-time.After(30 * time.Second):
932-
t.Fatal("pg watch did not shut down")
927+
t.Fatal(spec.exitMsg)
933928
}
934929
}
935930

931+
func TestLocalPGPushWatchGivesDeferredScopesAPollingOwner(t *testing.T) {
932+
testLocalPushWatchGivesDeferredScopesAPollingOwner(
933+
t, localPushWatchPollingOwnerSpec{
934+
obligationMsg: "the unwatchable root's polling obligation must reach the pg watch poller",
935+
exitMsg: "pg watch did not shut down",
936+
sessionUUID: "d4e5f6a7-4444-4555-8666-777788889999",
937+
wireHooks: func(hooks *archivePushWatchHooks) {
938+
hooks.pgStartupSync = func(
939+
context.Context, *syncpkg.Engine, bool,
940+
) (bool, error) {
941+
return false, nil
942+
}
943+
hooks.newPGPusher = func(*syncpkg.Engine) *pgPusher {
944+
return &pgPusher{
945+
localSync: func(context.Context) error { return nil },
946+
connect: func() (pgTarget, error) {
947+
return noopPGTarget{}, nil
948+
},
949+
}
950+
}
951+
},
952+
runWatch: func(
953+
ctx context.Context, backend *localArchiveWriteBackend,
954+
) error {
955+
return backend.PGPushWatch(
956+
ctx, pgTargetSelection{}, PGPushConfig{}, nil, nil,
957+
time.Hour, time.Hour,
958+
)
959+
},
960+
},
961+
)
962+
}
963+
964+
func TestLocalDuckDBPushWatchGivesDeferredScopesAPollingOwner(t *testing.T) {
965+
testLocalPushWatchGivesDeferredScopesAPollingOwner(
966+
t, localPushWatchPollingOwnerSpec{
967+
obligationMsg: "the unwatchable root's polling obligation must reach the duckdb watch poller",
968+
exitMsg: "duckdb watch did not shut down",
969+
sessionUUID: "e5f6a7b8-5555-4666-8777-888899990000",
970+
wireHooks: func(hooks *archivePushWatchHooks) {
971+
hooks.duckDBStartupSync = func(
972+
context.Context, *syncpkg.Engine, bool,
973+
) (bool, error) {
974+
return false, nil
975+
}
976+
hooks.newDuckDBPusher = func(*syncpkg.Engine) *duckDBPusher {
977+
return &duckDBPusher{
978+
localSync: func(context.Context) error { return nil },
979+
pushMirror: func(
980+
context.Context, DuckDBPushConfig, bool,
981+
) (duckdbsync.PushResult, error) {
982+
return duckdbsync.PushResult{}, nil
983+
},
984+
}
985+
}
986+
},
987+
runWatch: func(
988+
ctx context.Context, backend *localArchiveWriteBackend,
989+
) error {
990+
return backend.DuckDBPushWatch(
991+
ctx, config.DuckDBConfig{}, DuckDBPushConfig{}, nil, nil,
992+
time.Hour, time.Hour,
993+
)
994+
},
995+
},
996+
)
997+
}
998+
936999
func testLocalArchiveWriteBackend(t *testing.T) *localArchiveWriteBackend {
9371000
t.Helper()
9381001
dataDir := t.TempDir()

0 commit comments

Comments
 (0)