Skip to content

Commit ef93382

Browse files
committed
fix(serve): release startup maintenance from foreground resync fallbacks
The foreground resync runner's in-process branches called SyncAll or ResyncAll directly. Both record startup reconciliation, which closes the gate the deferred startup fallback checks, but neither releases the startup-maintenance gate, so on a skip-initial-sync daemon the signals and identity backfills stayed blocked until shutdown. The direct ResyncAll call also dropped SyncThenRun's abort-to-incremental fallback for a safely aborted resync. Route the spawn-failure/test fallback through SyncThenRun(full=true), matching the handler's no-runner arm, and give the worker-aborted arm an explicit release via a helper that keeps the gate closed when the pass was cancelled so the deferred fallback still owns recovery.
1 parent 1faa4c0 commit ef93382

2 files changed

Lines changed: 152 additions & 2 deletions

File tree

cmd/agentsview/main.go

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -942,7 +942,9 @@ func newForegroundResyncRunner(
942942
// "aborted" verdict takes this path: operational build
943943
// failures report "failed" and must surface their error
944944
// rather than masquerade as a successful incremental sync.
945-
return engine.SyncAll(ctx, progress), nil
945+
return syncAllReleasingStartupMaintenance(
946+
ctx, engine, progress,
947+
), nil
946948
}
947949
return statsFromWorkerResult(result), err
948950
}
@@ -951,10 +953,34 @@ func newForegroundResyncRunner(
951953
"(falling back in-process)", err,
952954
)
953955
}
954-
return engine.ResyncAll(ctx, progress), nil
956+
// SyncThenRun, not ResyncAll: it keeps the abort-to-incremental
957+
// fallback for a safely aborted in-process resync and releases
958+
// startup maintenance on success, matching the handler's no-runner
959+
// arm (runResyncWithFallback) and the sync runner's fallback above.
960+
return engine.SyncThenRun(
961+
ctx, true, progress, func(bool) error { return nil },
962+
)
955963
}
956964
}
957965

966+
// syncAllReleasingStartupMaintenance runs an incremental pass and, mirroring
967+
// SyncThenRun, releases startup maintenance when the pass was not cancelled.
968+
// SyncAll records startup reconciliation on its own but never releases the
969+
// maintenance gate; without the explicit release, a skip-initial-sync daemon
970+
// whose foreground resync took this arm would keep archive-wide backfills
971+
// gated until shutdown, because the deferred startup fallback observes the
972+
// closed reconciliation gate and returns without releasing. A cancelled pass
973+
// leaves the gate closed so the deferred fallback still owns recovery.
974+
func syncAllReleasingStartupMaintenance(
975+
ctx context.Context, engine *sync.Engine, progress func(sync.Progress),
976+
) sync.SyncStats {
977+
stats := engine.SyncAll(ctx, progress)
978+
if ctx.Err() == nil {
979+
engine.ReleaseStartupMaintenance()
980+
}
981+
return stats
982+
}
983+
958984
// runWorkerResyncBuild builds a resync replacement in a worker process behind the
959985
// write barrier, then swaps it in and resets caches. It closes the writer for the
960986
// whole build-and-swap window (readers keep serving, direct writes fail with

cmd/agentsview/resync_worker_test.go

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,15 @@ package main
22

33
import (
44
"context"
5+
"path/filepath"
56
"testing"
7+
"time"
68

79
"github.com/stretchr/testify/assert"
810
"github.com/stretchr/testify/require"
11+
"go.kenn.io/agentsview/internal/config"
912
"go.kenn.io/agentsview/internal/db"
13+
"go.kenn.io/agentsview/internal/dbtest"
1014
"go.kenn.io/agentsview/internal/sync"
1115
)
1216

@@ -32,3 +36,123 @@ func TestForegroundResyncRunnerFallsBackInProcess(t *testing.T) {
3236
assert.Equal(t, 3, stats.Synced, "in-process resync fallback rebuilds the archive")
3337
assert.False(t, database.NeedsResync())
3438
}
39+
40+
// requireStartupMaintenanceReleased asserts that RunStartupMaintenance is no
41+
// longer gated: its work function must run promptly instead of blocking on the
42+
// startup-maintenance gate until shutdown.
43+
func requireStartupMaintenanceReleased(t *testing.T, engine *sync.Engine) {
44+
t.Helper()
45+
maintenanceRan := make(chan struct{})
46+
maintenanceDone := make(chan error, 1)
47+
go func() {
48+
maintenanceDone <- engine.RunStartupMaintenance(
49+
t.Context(), func() error {
50+
close(maintenanceRan)
51+
return nil
52+
},
53+
)
54+
}()
55+
select {
56+
case <-maintenanceRan:
57+
case <-time.After(2 * time.Second):
58+
require.FailNow(t, "startup maintenance still gated after the pass")
59+
}
60+
require.NoError(t, <-maintenanceDone)
61+
}
62+
63+
// TestForegroundResyncRunnerReleasesStartupMaintenance covers the
64+
// skip-initial-sync daemon shape: when the in-process fallback drives startup
65+
// reconciliation, it must also release the startup-maintenance gate.
66+
// Otherwise the deferred startup fallback sees the closed reconciliation gate,
67+
// returns early, and archive-wide backfills stay blocked until shutdown.
68+
func TestForegroundResyncRunnerReleasesStartupMaintenance(t *testing.T) {
69+
cfg := testConfigWithClaudeFixture(t)
70+
database, err := db.Open(cfg.DBPath)
71+
require.NoError(t, err)
72+
t.Cleanup(func() { require.NoError(t, database.Close()) })
73+
engineCfg := workerEngineConfig(cfg)
74+
engineCfg.DeferStartupMaintenance = true
75+
engine := sync.NewEngine(database, engineCfg)
76+
t.Cleanup(engine.Close)
77+
78+
runner := newForegroundResyncRunner(context.Background(), cfg, engine, database)
79+
_, err = runner(context.Background(), nil)
80+
81+
require.NoError(t, err)
82+
require.True(t, engine.StartupReconciled(),
83+
"a successful in-process resync closes the reconciliation gate")
84+
requireStartupMaintenanceReleased(t, engine)
85+
}
86+
87+
// TestForegroundResyncRunnerAbortedResyncFallsBackIncremental verifies the
88+
// in-process fallback keeps SyncThenRun's abort semantics: a safely aborted
89+
// resync catches up with an incremental pass instead of surfacing the bare
90+
// abort, and startup still reconciles and releases maintenance.
91+
func TestForegroundResyncRunnerAbortedResyncFallsBackIncremental(t *testing.T) {
92+
database := dbtest.OpenTestDB(t)
93+
missingPath := filepath.Join(t.TempDir(), "missing.jsonl")
94+
dbtest.SeedSession(t, database, "existing", "proj", func(s *db.Session) {
95+
s.FilePath = &missingPath
96+
})
97+
engine := sync.NewEngine(database, sync.EngineConfig{
98+
Machine: "local",
99+
DeferStartupMaintenance: true,
100+
})
101+
t.Cleanup(engine.Close)
102+
103+
runner := newForegroundResyncRunner(
104+
context.Background(), config.Config{}, engine, database,
105+
)
106+
stats, err := runner(context.Background(), nil)
107+
108+
require.NoError(t, err)
109+
assert.False(t, stats.Aborted,
110+
"a safely aborted resync must fall back to an incremental sync")
111+
assert.True(t, engine.StartupReconciled(),
112+
"the incremental fallback pass reconciles startup")
113+
requireStartupMaintenanceReleased(t, engine)
114+
}
115+
116+
// TestSyncAllReleasingStartupMaintenance covers the worker-aborted resync arm
117+
// (unreachable under a test binary): a completed incremental catch-up must
118+
// release the startup-maintenance gate, while a cancelled pass leaves the gate
119+
// closed so the deferred startup fallback still owns recovery.
120+
func TestSyncAllReleasingStartupMaintenance(t *testing.T) {
121+
t.Run("releases after completed pass", func(t *testing.T) {
122+
database := dbtest.OpenTestDB(t)
123+
engine := sync.NewEngine(database, sync.EngineConfig{
124+
Machine: "local",
125+
DeferStartupMaintenance: true,
126+
})
127+
t.Cleanup(engine.Close)
128+
129+
syncAllReleasingStartupMaintenance(context.Background(), engine, nil)
130+
131+
requireStartupMaintenanceReleased(t, engine)
132+
})
133+
t.Run("keeps gate closed when cancelled", func(t *testing.T) {
134+
database := dbtest.OpenTestDB(t)
135+
engine := sync.NewEngine(database, sync.EngineConfig{
136+
Machine: "local",
137+
DeferStartupMaintenance: true,
138+
})
139+
t.Cleanup(engine.Close)
140+
ctx, cancel := context.WithCancel(context.Background())
141+
cancel()
142+
143+
syncAllReleasingStartupMaintenance(ctx, engine, nil)
144+
145+
maintenanceErr := make(chan error, 1)
146+
blockedCtx, blockedCancel := context.WithTimeout(
147+
context.Background(), 100*time.Millisecond,
148+
)
149+
defer blockedCancel()
150+
go func() {
151+
maintenanceErr <- engine.RunStartupMaintenance(
152+
blockedCtx, func() error { return nil },
153+
)
154+
}()
155+
require.ErrorIs(t, <-maintenanceErr, context.DeadlineExceeded,
156+
"a cancelled pass must leave maintenance to the deferred fallback")
157+
})
158+
}

0 commit comments

Comments
 (0)