From 06a054487be432187ca1e9e0c5bce1302e3842ae Mon Sep 17 00:00:00 2001 From: salmonumbrella <182032677+salmonumbrella@users.noreply.github.com> Date: Wed, 2 Sep 2026 07:56:07 -0500 Subject: [PATCH 1/6] feat(api): add asynchronous historical import jobs - Acquire source ownership before loading resume checkpoints. - Return durable pending jobs before provider setup starts. - Fail pending jobs orphaned by a daemon restart. Generated with Codex Co-authored-by: Marius van Niekerk Co-authored-by: Wes McKinney Co-authored-by: Codex --- api/openapi.yaml | 222 ++++++++- .../cache_staleness_meeting_refresh_test.go | 5 +- cmd/msgvault/cmd/remove_account_test.go | 6 +- cmd/msgvault/cmd/serve.go | 44 +- cmd/msgvault/cmd/serve_test.go | 78 +++- cmd/msgvault/cmd/syncfull.go | 26 +- internal/api/cli_handlers.go | 1 + internal/api/import_jobs.go | 349 ++++++++++++++ internal/api/import_jobs_test.go | 352 ++++++++++++++ internal/api/openapi.go | 13 +- internal/api/openapi_test.go | 58 ++- internal/api/operation_gate.go | 4 +- internal/api/relationship_calendar_test.go | 2 +- internal/api/routes.go | 3 + internal/api/server.go | 30 +- internal/beeper/importer_test.go | 3 +- internal/calsync/calsync.go | 42 +- internal/calsync/review_fixes_test.go | 63 ++- internal/fbmessenger/importer.go | 20 +- internal/fbmessenger/importer_test.go | 8 +- internal/importer/emlx_import.go | 25 +- internal/importer/emlx_import_test.go | 3 + internal/importer/mbox_import.go | 28 +- internal/importer/mbox_import_test.go | 59 +++ internal/importer/pst_import.go | 25 +- internal/meetingimport/importer_test.go | 22 +- internal/slack/importer_test.go | 3 +- internal/store/dialect_pg.go | 2 + internal/store/dialect_sqlite.go | 2 + internal/store/export_test.go | 6 + internal/store/messages_test.go | 2 + internal/store/person_sweep_work_pg_test.go | 76 +-- internal/store/schema.sql | 15 +- internal/store/schema_pg.sql | 13 +- internal/store/store.go | 49 +- internal/store/sync.go | 437 +++++++++++++++--- internal/store/sync_context_test.go | 3 + internal/store/sync_execution_lock.go | 372 +++++++++++++++ internal/store/sync_test.go | 253 +++++++++- internal/sync/incremental.go | 13 +- internal/sync/sync.go | 289 ++++++++---- internal/sync/sync_test.go | 291 +++++++++--- internal/teams/importer_test.go | 3 +- pkg/client/generated/client.go | 135 ++++++ pkg/client/generated/client_options.go | 88 ++++ pkg/client/generated/client_with_response.go | 271 +++++++++++ pkg/client/generated/enums.go | 19 + pkg/client/generated/paths.go | 9 + pkg/client/generated/payloads.go | 2 + pkg/client/generated/responses.go | 51 ++ pkg/client/generated/types.go | 74 +++ pkg/client/openapi.yaml | 223 ++++++++- web/src/lib/api/generated/schema.d.ts | 243 ++++++++++ 53 files changed, 3996 insertions(+), 439 deletions(-) create mode 100644 internal/api/import_jobs.go create mode 100644 internal/api/import_jobs_test.go create mode 100644 internal/store/sync_execution_lock.go diff --git a/api/openapi.yaml b/api/openapi.yaml index 7d3e6524d..c06672184 100644 --- a/api/openapi.yaml +++ b/api/openapi.yaml @@ -4829,6 +4829,105 @@ components: required: - identifier type: object + ImportJobRequest: + additionalProperties: false + properties: + account: + minLength: 1 + type: string + after: + pattern: ^[0-9]{4}-[0-9]{2}-[0-9]{2}$ + type: string + before: + pattern: ^[0-9]{4}-[0-9]{2}-[0-9]{2}$ + type: string + limit: + format: int64 + minimum: 0 + type: integer + noresume: + type: boolean + query: + type: string + required: + - account + type: object + ImportJobResponse: + additionalProperties: true + properties: + account: + type: string + added: + format: int64 + type: integer + created_at: + format: date-time + type: string + error: + type: string + finished_at: + format: date-time + type: + - string + - "null" + job_id: + type: string + processed: + format: int64 + type: integer + skipped: + format: int64 + type: integer + started_at: + format: date-time + type: + - string + - "null" + status: + enum: + - pending + - running + - done + - failed + type: string + summary: + $ref: "#/components/schemas/ImportJobSummary" + required: + - job_id + - account + - status + - processed + - added + - skipped + - created_at + - started_at + - finished_at + type: object + ImportJobSummary: + additionalProperties: true + properties: + added: + format: int64 + type: integer + errors: + format: int64 + type: integer + processed: + format: int64 + type: integer + skipped: + format: int64 + type: integer + updated: + format: int64 + type: integer + required: + - processed + - added + - updated + - skipped + - errors + type: object ImportRequest: additionalProperties: false properties: @@ -11251,7 +11350,7 @@ components: type: apiKey info: title: msgvault API - version: 2.13.0 + version: 2.14.0 openapi: 3.1.0 paths: /api/ping: @@ -16318,6 +16417,127 @@ paths: summary: Import one meeting tags: - API + /api/v1/imports: + post: + operationId: createImportJob + requestBody: + content: + application/json: + schema: + $ref: "#/components/schemas/ImportJobRequest" + required: true + responses: + "202": + content: + application/json: + schema: + $ref: "#/components/schemas/ImportJobResponse" + description: Accepted + "400": + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorResponse" + description: Error + "401": + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorResponse" + description: Error + "404": + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorResponse" + description: Error + "409": + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorResponse" + description: Error + "413": + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorResponse" + description: Error + "415": + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorResponse" + description: Error + "422": + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorResponse" + description: Error + "500": + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorResponse" + description: Error + "503": + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorResponse" + description: Error + default: + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorResponse" + description: Error + security: + - apiKey: [] + summary: Start a bounded historical import + tags: + - API + /api/v1/imports/{job_id}: + get: + operationId: getImportJob + parameters: + - description: Historical import job ID + in: path + name: job_id + required: true + schema: + type: string + responses: + "200": + content: + application/json: + schema: + $ref: "#/components/schemas/ImportJobResponse" + description: OK + "401": + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorResponse" + description: Error + "404": + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorResponse" + description: Error + default: + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorResponse" + description: Error + security: + - apiKey: [] + summary: Get historical import status + tags: + - API /api/v1/integrations/tasks/search: get: operationId: searchIntegrationTasks diff --git a/cmd/msgvault/cmd/cache_staleness_meeting_refresh_test.go b/cmd/msgvault/cmd/cache_staleness_meeting_refresh_test.go index 895b8cf26..7c665c2e3 100644 --- a/cmd/msgvault/cmd/cache_staleness_meeting_refresh_test.go +++ b/cmd/msgvault/cmd/cache_staleness_meeting_refresh_test.go @@ -133,7 +133,7 @@ func TestCacheNeedsBuild_CirclebackRefresh(t *testing.T) { assert.Equal("Refreshed Meeting", cachedSubject) } -func TestCacheNeedsBuild_SupersededCirclebackRunWithoutCheckpoint(t *testing.T) { +func TestCacheNeedsBuild_FailedCirclebackRunWithoutCheckpoint(t *testing.T) { require := require.New(t) assert := assert.New(t) tmp := t.TempDir() @@ -169,8 +169,7 @@ func TestCacheNeedsBuild_SupersededCirclebackRunWithoutCheckpoint(t *testing.T) WHERE source_message_id = 'meeting:failed-refresh-1' `) require.NoError(err) - _, err = st.StartSync(first.SourceID, circleback.SourceType) - require.NoError(err, "starting the replacement run supersedes the abandoned run") + require.NoError(st.FailSync(abandonedRunID, "worker stopped")) var status string var additions, updates int64 diff --git a/cmd/msgvault/cmd/remove_account_test.go b/cmd/msgvault/cmd/remove_account_test.go index 1cc998df1..df8911757 100644 --- a/cmd/msgvault/cmd/remove_account_test.go +++ b/cmd/msgvault/cmd/remove_account_test.go @@ -570,7 +570,7 @@ func TestRemoveAccountCmd_SkipsDeletionWhenRemovedAccountHasActiveSync(t *testin // RemoveSource cascades away. _, err = s.StartSync(aliceSrc.ID, "full") require.NoError(err, "StartSync") - _ = s.Close() + t.Cleanup(func() { _ = s.Close() }) filePath := seedAttachmentFile(t, attachmentsDir, "dd/hashA", "content-a") @@ -606,7 +606,7 @@ func TestRemoveAccountConfirmedDoesNotBypassActiveSyncGuard(t *testing.T) { require.NoError(err, "GetSourceByIdentifier") _, err = s.StartSync(aliceSrc.ID, "full") require.NoError(err, "StartSync") - _ = s.Close() + t.Cleanup(func() { _ = s.Close() }) savedCfg := cfg defer func() { cfg = savedCfg }() @@ -1205,7 +1205,7 @@ func TestRemoveAccountCmd_DiscordPreservesTokenDuringActiveSync(t *testing.T) { require.NoError(err) _, err = st.StartSync(source.ID, "discord") require.NoError(err) - require.NoError(st.Close()) + t.Cleanup(func() { _ = st.Close() }) manager := discord.NewTokenManager(filepath.Join(tmpDir, "tokens")) require.NoError(manager.Save(discord.NewTokenRecord( diff --git a/cmd/msgvault/cmd/serve.go b/cmd/msgvault/cmd/serve.go index 3e7bf129d..d3c9cddef 100644 --- a/cmd/msgvault/cmd/serve.go +++ b/cmd/msgvault/cmd/serve.go @@ -242,6 +242,14 @@ func runServe(cmd *cobra.Command, args []string) error { if err := s.InitSchemaContext(cmd.Context()); err != nil { return fmt.Errorf("init schema: %w", err) } + failedPendingImports, err := s.FailPendingSyncOperationsContext(cmd.Context()) + if err != nil { + return fmt.Errorf("recover pending historical imports: %w", err) + } + if failedPendingImports > 0 { + logger.Warn("marked historical imports abandoned by the previous daemon as failed", + "count", failedPendingImports) + } logger.Info("daemon startup step complete", "step", "init_archive_schema") // Legacy [identity] migration is deferred to the first scheduled sync's // runPostSourceCreateMigrations call, which fires AFTER that sync's @@ -1457,7 +1465,30 @@ func (a *storeAPIAdapter) RunCLISync( req api.CLISyncRequest, emit func(api.CLISyncEvent) error, ) error { - return a.runCLISyncWithRunner(ctx, req, emit, runDaemonCLISubprocessStream) + return a.runCLISyncOperationWithRunner(ctx, req, emit, runDaemonCLISubprocessStream) +} + +func (a *storeAPIAdapter) runCLISyncOperationWithRunner( + ctx context.Context, + req api.CLISyncRequest, + emit func(api.CLISyncEvent) error, + run cliSyncSubprocessRunner, +) error { + err := a.runCLISyncWithRunner(ctx, req, emit, run) + if err == nil && ctx.Err() != nil { + err = ctx.Err() + } + if req.OperationID == "" { + return err + } + status := "done" + if err != nil { + status = "failed" + } + if finishErr := a.store.FinishSyncOperation(req.OperationID, status); finishErr != nil { + return errors.Join(err, fmt.Errorf("finish sync operation: %w", finishErr)) + } + return err } type cliSyncSubprocessRunner func( @@ -1505,6 +1536,9 @@ func cliSyncSubprocessArgs(req api.CLISyncRequest) []string { if req.SourceIDSet { args = append(args, "--source-id", strconv.FormatInt(req.SourceID, 10)) } + if req.OperationID != "" { + args = append(args, "--sync-operation-id", req.OperationID) + } if req.Query != "" { args = append(args, "--query", req.Query) } @@ -2732,6 +2766,14 @@ func (a *storeAPIAdapter) GetLatestSync(sourceID int64) (*store.SyncRun, error) return a.store.GetLatestSync(sourceID) } +func (a *storeAPIAdapter) GetSyncOperation(operationID string) (*store.SyncOperation, error) { + return a.store.GetSyncOperation(operationID) +} + +func (a *storeAPIAdapter) CreateSyncOperation(sourceID int64, operationID string) (*store.SyncOperation, error) { + return a.store.CreateSyncOperation(sourceID, operationID) +} + func (a *storeAPIAdapter) GetLastSuccessfulSync(sourceID int64) (*store.SyncRun, error) { return a.store.GetLastSuccessfulSync(sourceID) } diff --git a/cmd/msgvault/cmd/serve_test.go b/cmd/msgvault/cmd/serve_test.go index 519b464f6..68e6d6a4c 100644 --- a/cmd/msgvault/cmd/serve_test.go +++ b/cmd/msgvault/cmd/serve_test.go @@ -391,6 +391,52 @@ func TestRunServeStartsReadOnlyWithoutOAuthConfig(t *testing.T) { } } +func TestRunServeFailsPendingImportFromPreviousDaemon(t *testing.T) { + require := require.New(t) + assert := assert.New(t) + oldCfg := cfg + dataDir := t.TempDir() + c := lifecycleTestConfig(dataDir) + c.Server.APIPort = freeTCPPort(t) + c.Analytics.Engine = config.AnalyticsEngineSQL + c.Vector.Enabled = false + cfg = c + t.Cleanup(func() { cfg = oldCfg }) + + st, err := store.Open(c.DatabaseDSN()) + require.NoError(err) + require.NoError(st.InitSchema()) + source, err := st.GetOrCreateSource("gmail", "orphaned-import@example.com") + require.NoError(err) + _, err = st.CreateSyncOperation(source.ID, "orphaned-operation") + require.NoError(err) + require.NoError(st.Close()) + + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + cmd := &cobra.Command{Use: serveCmd.Use} + cmd.SetContext(ctx) + errCh := make(chan error, 1) + go func() { errCh <- runServe(cmd, nil) }() + waitForServeHealth(t, c.Server.APIPort, errCh) + + observer, err := store.Open(c.DatabaseDSN()) + require.NoError(err) + t.Cleanup(func() { require.NoError(observer.Close()) }) + op, err := observer.GetSyncOperation("orphaned-operation") + require.NoError(err) + assert.Equal("failed", op.Status) + assert.True(op.FinishedAt.Valid) + + cancel() + select { + case err := <-errCh: + require.NoError(err, "runServe") + case <-time.After(5 * time.Second): + require.FailNow("runServe did not stop after context cancellation") + } +} + func TestRunServeImmediateCancellationWaitsForAPIStart(t *testing.T) { require := require.New(t) oldCfg := cfg @@ -1468,8 +1514,10 @@ func TestCLISyncSubprocessArgsIncludesExactSourceID(t *testing.T) { cliSyncSubprocessArgs(api.CLISyncRequest{SourceID: 42, SourceIDSet: true}), ) assert.Equal(t, - []string{"sync-full", "--source-id", "42"}, - cliSyncSubprocessArgs(api.CLISyncRequest{Full: true, SourceID: 42, SourceIDSet: true}), + []string{"sync-full", "--source-id", "42", "--sync-operation-id", "operation-1"}, + cliSyncSubprocessArgs(api.CLISyncRequest{ + Full: true, SourceID: 42, SourceIDSet: true, OperationID: "operation-1", + }), ) } @@ -1574,6 +1622,32 @@ remote_enabled = true assert.NoFileExists(filepath.Join(manager.InProgressDir(), manifest.ID+".json")) } +func TestStoreAPIAdapterCanceledSyncOperationIsFailed(t *testing.T) { + require := require.New(t) + assert := assert.New(t) + f := storetest.New(t) + _, err := f.Store.CreateSyncOperation(f.Source.ID, "operation-1") + require.NoError(err) + ctx, cancel := context.WithCancel(t.Context()) + cancel() + adapter := &storeAPIAdapter{store: f.Store} + + err = adapter.runCLISyncOperationWithRunner( + ctx, + api.CLISyncRequest{Full: true, OperationID: "operation-1"}, + nil, + func(context.Context, []string, func(string, string) error) error { return nil }, + ) + + require.ErrorIs(err, context.Canceled) + op, err := f.Store.GetSyncOperation("operation-1") + require.NoError(err) + assert.Equal("failed", op.Status) + assert.False(op.StartedAt.Valid) + assert.True(op.FinishedAt.Valid) + assert.Empty(op.Runs) +} + func TestStoreAPIAdapterRunCLICommandPacksOnlyAllowlistedSuccess(t *testing.T) { tests := []struct { name string diff --git a/cmd/msgvault/cmd/syncfull.go b/cmd/msgvault/cmd/syncfull.go index d0ac2d898..70d55b140 100644 --- a/cmd/msgvault/cmd/syncfull.go +++ b/cmd/msgvault/cmd/syncfull.go @@ -28,6 +28,7 @@ var ( syncBefore string syncAfter string syncLimit int + syncOperationID string syncFolders []string // folder names to include (from --folder flag) syncSkipFolders []string // folder names to exclude (from --skip-folder flag) ) @@ -564,6 +565,7 @@ func runFullSync(ctx context.Context, s *store.Store, getOAuthMgr func(string) ( opts.Query = query opts.NoResume = syncNoResume opts.Limit = syncLimit + opts.OperationID = syncOperationID opts.AttachmentsDir = cfg.AttachmentsDir() // IMAP page tokens are numeric offsets into a message list @@ -592,7 +594,21 @@ func runFullSync(ctx context.Context, s *store.Store, getOAuthMgr func(string) ( } fmt.Println() - summary, err := syncer.Full(ctx, src.Identifier) + summary, err := syncer.FullWithFinalizer( + ctx, + src.Identifier, + func(summary *gmail.SyncSummary) error { + if src.SourceType != sourceTypeIMAP { + return nil + } + if err := saveIMAPFolderStates( + ctx, s, src, apiClient, summary, opts.Limit, + ); err != nil { + return fmt.Errorf("save IMAP incremental state: %w", err) + } + return nil + }, + ) if err != nil { if ctx.Err() != nil { if opts.NoResume { @@ -605,12 +621,6 @@ func runFullSync(ctx context.Context, s *store.Store, getOAuthMgr func(string) ( return fmt.Errorf("sync failed: %w", err) } - if src.SourceType == sourceTypeIMAP { - if err := saveIMAPFolderStates(ctx, s, src, apiClient, summary, opts.Limit); err != nil { - return fmt.Errorf("save IMAP incremental state: %w", err) - } - } - // Print summary; skip the spacer when no progress lines were // printed so a no-op sync doesn't emit stacked blank lines. if progress.printedAnything() { @@ -921,6 +931,8 @@ func imapSkipReason(src *store.Source) (string, error) { func init() { syncFullCmd.Flags().Int64("source-id", 0, "Exact source ID to sync") + syncFullCmd.Flags().StringVar(&syncOperationID, "sync-operation-id", "", "Attribute runs to a daemon sync operation") + _ = syncFullCmd.Flags().MarkHidden("sync-operation-id") syncFullCmd.Flags().StringVar(&syncQuery, "query", "", "Gmail search query") syncFullCmd.Flags().BoolVar(&syncNoResume, "noresume", false, "Force fresh sync (don't resume; re-enumerates all IMAP folders)") syncFullCmd.Flags().StringVar(&syncBefore, "before", "", "Only messages before this date (YYYY-MM-DD)") diff --git a/internal/api/cli_handlers.go b/internal/api/cli_handlers.go index dc64cce3c..81702bb76 100644 --- a/internal/api/cli_handlers.go +++ b/internal/api/cli_handlers.go @@ -465,6 +465,7 @@ type CLISyncRequest struct { Before string After string Limit int + OperationID string Folders []string SkipFolders []string } diff --git a/internal/api/import_jobs.go b/internal/api/import_jobs.go new file mode 100644 index 000000000..0e5980a6e --- /dev/null +++ b/internal/api/import_jobs.go @@ -0,0 +1,349 @@ +package api + +import ( + "crypto/rand" + "encoding/hex" + "encoding/json" + "errors" + "fmt" + "mime" + "net/http" + "strings" + "time" + + "github.com/danielgtaylor/huma/v2" + "go.kenn.io/msgvault/internal/store" +) + +const ( + importJobsEndpointPath = "/api/v1/imports" + maxImportRequestBytes = 16 << 10 +) + +var ( + errImportAmbiguousSource = errors.New("import account is ambiguous") + errImportUnsyncable = errors.New("import account is not syncable") +) + +type ImportJobRequest struct { + Account string `json:"account" minLength:"1"` + After string `json:"after,omitempty" pattern:"^[0-9]{4}-[0-9]{2}-[0-9]{2}$"` + Before string `json:"before,omitempty" pattern:"^[0-9]{4}-[0-9]{2}-[0-9]{2}$"` + Limit int `json:"limit,omitempty" minimum:"0"` + Query string `json:"query,omitempty"` + NoResume bool `json:"noresume,omitempty"` +} + +type ImportJobSummary struct { + Processed int64 `json:"processed"` + Added int64 `json:"added"` + Updated int64 `json:"updated"` + Skipped int64 `json:"skipped"` + Errors int64 `json:"errors"` +} + +type ImportJobResponse struct { + JobID string `json:"job_id"` + Account string `json:"account"` + Status string `json:"status" enum:"pending,running,done,failed"` + Processed int64 `json:"processed"` + Added int64 `json:"added"` + Skipped int64 `json:"skipped"` + Error string `json:"error,omitempty"` + CreatedAt time.Time `json:"created_at"` + StartedAt *time.Time `json:"started_at"` + FinishedAt *time.Time `json:"finished_at"` + Summary *ImportJobSummary `json:"summary,omitempty"` +} + +type importJobStore interface { + GetSourcesByIdentifierOrDisplayName(query string) ([]*store.Source, error) + GetSourceByID(id int64) (*store.Source, error) + GetActiveSync(sourceID int64) (*store.SyncRun, error) + CreateSyncOperation(sourceID int64, operationID string) (*store.SyncOperation, error) + GetSyncOperation(operationID string) (*store.SyncOperation, error) +} + +func (s *Server) registerImportJobRoutes(api huma.API) { + createOp := rawAPIV1Operation("createImportJob", http.MethodPost, "/imports", "Start a bounded historical import") + createOp.RequestBody = jsonRequestBodyFor[ImportJobRequest](api) + createOp.Responses = jsonResponsesFor[ImportJobResponse](api, http.StatusAccepted) + addErrorResponses(api, createOp.Responses, + http.StatusBadRequest, http.StatusConflict, http.StatusNotFound, + http.StatusRequestEntityTooLarge, http.StatusUnauthorized, + http.StatusUnsupportedMediaType, http.StatusUnprocessableEntity, + http.StatusInternalServerError, http.StatusServiceUnavailable, + ) + registerRawHumaRoute(api, createOp, s.handleCreateImportJob) + + getOp := rawAPIV1Operation("getImportJob", http.MethodGet, "/imports/{job_id}", "Get historical import status") + getOp.Responses = jsonResponsesFor[ImportJobResponse](api) + addErrorResponses(api, getOp.Responses, http.StatusNotFound, http.StatusUnauthorized) + registerRawHumaRoute(api, getOp, s.handleGetImportJob) +} + +func (s *Server) handleCreateImportJob(w http.ResponseWriter, r *http.Request) { + req, ok := decodeImportJobRequest(w, r) + if !ok { + return + } + runner, runnerOK := s.store.(CLISyncRunner) + jobStore, storeOK := s.store.(importJobStore) + if !runnerOK || !storeOK { + writeError(w, http.StatusServiceUnavailable, "service_unavailable", "Historical imports are unavailable") + return + } + sources, err := jobStore.GetSourcesByIdentifierOrDisplayName(req.Account) + if err != nil { + s.logger.Error("failed to resolve historical import account", "error", err) + writeError(w, http.StatusInternalServerError, "internal_error", "Failed to resolve import account") + return + } + source, err := resolveExactImportSource(sources, req.Account) + switch { + case errors.Is(err, errImportAmbiguousSource): + writeError(w, http.StatusConflict, "ambiguous_account", "Import account matches multiple syncable sources") + return + case errors.Is(err, errImportUnsyncable): + writeError(w, http.StatusUnprocessableEntity, "account_not_syncable", "Import account must be a Gmail or IMAP source") + return + case err != nil || source == nil: + writeError(w, http.StatusNotFound, "not_found", "Import account not found") + return + } + + release, acquired := func() (func(), bool) { + if s.operationGate == nil { + return func() {}, true + } + return beginGateWorkBounded(r.Context(), s.operationGate, "msgvault historical import") + }() + if !acquired { + writeOperationGateBusy(w, s.operationGate) + return + } + active, err := jobStore.GetActiveSync(source.ID) + if err == nil && active != nil { + release() + writeError(w, http.StatusConflict, "sync_already_active", "Import account already has an active sync") + return + } else if err != nil && !errors.Is(err, store.ErrSyncRunNotFound) { + release() + s.logger.Error("failed to inspect historical import checkpoint", "error", err) + writeError(w, http.StatusInternalServerError, "internal_error", "Failed to inspect import state") + return + } + + jobID, err := newImportJobID() + if err != nil { + release() + writeError(w, http.StatusInternalServerError, "internal_error", "Failed to create import job") + return + } + runReq := CLISyncRequest{ + Full: true, SourceID: source.ID, SourceIDSet: true, + Query: req.Query, NoResume: req.NoResume, Before: req.Before, + After: req.After, Limit: req.Limit, OperationID: jobID, + } + s.importMu.Lock() + if s.importsClosed { + s.importMu.Unlock() + release() + writeError(w, http.StatusServiceUnavailable, "service_unavailable", "Historical imports are unavailable") + return + } + s.importWG.Add(1) + s.importMu.Unlock() + op, err := jobStore.CreateSyncOperation(source.ID, jobID) + if err != nil { + s.importWG.Done() + release() + s.logger.Error("failed to persist historical import", "error", err) + writeError(w, http.StatusInternalServerError, "internal_error", "Failed to create import job") + return + } + go func() { + defer s.importWG.Done() + defer release() + err := runner.RunCLISync(s.importContext, runReq, func(CLISyncEvent) error { return nil }) + if err != nil { + s.logger.Error("historical import failed", "job_id", jobID, "error", err) + } + }() + + response, err := importJobResponse(jobStore, op) + if err != nil { + writeError(w, http.StatusInternalServerError, "internal_error", "Failed to read import job") + return + } + writeJSON(w, http.StatusAccepted, response) +} + +func decodeImportJobRequest(w http.ResponseWriter, r *http.Request) (ImportJobRequest, bool) { + mediaType, _, err := mime.ParseMediaType(r.Header.Get("Content-Type")) + if err != nil || mediaType != applicationJSONMediaType { + writeError(w, http.StatusUnsupportedMediaType, "unsupported_media_type", "Content-Type must be application/json") + return ImportJobRequest{}, false + } + var req ImportJobRequest + decoder := json.NewDecoder(http.MaxBytesReader(w, r.Body, maxImportRequestBytes)) + decoder.DisallowUnknownFields() + if err := decoder.Decode(&req); err != nil { + if _, ok := errors.AsType[*http.MaxBytesError](err); ok { + writeError(w, http.StatusRequestEntityTooLarge, "request_too_large", "Import request exceeds 16 KiB") + return ImportJobRequest{}, false + } + writeError(w, http.StatusBadRequest, "bad_request", "Invalid import request JSON") + return ImportJobRequest{}, false + } + if !requireSingleJSONValue(w, decoder, "bad_request") { + return ImportJobRequest{}, false + } + req.Account = strings.TrimSpace(req.Account) + if validationErr := validateImportJobRequest(req); validationErr != "" { + writeError(w, http.StatusUnprocessableEntity, "validation_failed", validationErr) + return ImportJobRequest{}, false + } + return req, true +} + +func (s *Server) handleGetImportJob(w http.ResponseWriter, r *http.Request) { + jobStore, ok := s.store.(importJobStore) + if !ok { + writeError(w, http.StatusServiceUnavailable, "service_unavailable", "Historical imports are unavailable") + return + } + op, err := jobStore.GetSyncOperation(r.PathValue("job_id")) + if errors.Is(err, store.ErrSyncRunNotFound) { + writeError(w, http.StatusNotFound, "not_found", "Import job not found") + return + } + if err != nil { + writeError(w, http.StatusInternalServerError, "internal_error", "Failed to read import job") + return + } + response, err := importJobResponse(jobStore, op) + if err != nil { + writeError(w, http.StatusInternalServerError, "internal_error", "Failed to read import job") + return + } + writeJSON(w, http.StatusOK, response) +} + +func importJobResponse(jobStore importJobStore, op *store.SyncOperation) (ImportJobResponse, error) { + if op == nil { + return ImportJobResponse{}, store.ErrSyncRunNotFound + } + source, err := jobStore.GetSourceByID(op.SourceID) + if err != nil { + return ImportJobResponse{}, err + } + response := ImportJobResponse{ + JobID: op.ID, Account: source.Identifier, Status: op.Status, + CreatedAt: op.CreatedAt.UTC(), + } + if op.StartedAt.Valid { + startedAt := op.StartedAt.Time.UTC() + response.StartedAt = &startedAt + } + var summary ImportJobSummary + for _, run := range op.Runs { + processed, added, skipped := importRunCounts(run) + summary.Processed += processed + summary.Added += added + summary.Updated += run.MessagesUpdated + summary.Skipped += skipped + summary.Errors += run.ErrorsCount + } + response.Processed = summary.Processed + response.Added = summary.Added + response.Skipped = summary.Skipped + if op.FinishedAt.Valid { + finishedAt := op.FinishedAt.Time.UTC() + response.FinishedAt = &finishedAt + } + switch op.Status { + case "done": + response.Summary = &summary + case "failed": + response.Error = "import failed" + } + return response, nil +} + +func importRunCounts(run *store.SyncRun) (processed, added, skipped int64) { + processed = run.MessagesProcessed + added = run.MessagesAdded + skipped = max(0, processed-added-run.MessagesUpdated) + return processed, added, skipped +} + +func newImportJobID() (string, error) { + var raw [16]byte + if _, err := rand.Read(raw[:]); err != nil { + return "", fmt.Errorf("generate import job id: %w", err) + } + return hex.EncodeToString(raw[:]), nil +} + +func validateImportJobRequest(req ImportJobRequest) string { + if req.Account == "" { + return "account is required" + } + if req.Limit < 0 { + return "limit must be a non-negative integer" + } + after, afterSet, err := parseImportDate(req.After) + if err != nil { + return "after must use YYYY-MM-DD" + } + before, beforeSet, err := parseImportDate(req.Before) + if err != nil { + return "before must use YYYY-MM-DD" + } + if afterSet && beforeSet && !after.Before(before) { + return "after must be earlier than before" + } + return "" +} + +func parseImportDate(raw string) (time.Time, bool, error) { + if raw == "" { + return time.Time{}, false, nil + } + parsed, err := time.Parse("2006-01-02", raw) + if err != nil { + return time.Time{}, true, fmt.Errorf("parse import date: %w", err) + } + return parsed, true, nil +} + +func resolveExactImportSource(sources []*store.Source, account string) (*store.Source, error) { + var match *store.Source + selectorFound := false + for _, source := range sources { + if source == nil { + continue + } + matchesIdentifier := strings.EqualFold(source.Identifier, account) + matchesDisplayName := source.DisplayName.Valid && strings.EqualFold(source.DisplayName.String, account) + if !matchesIdentifier && !matchesDisplayName { + continue + } + selectorFound = true + if source.SourceType != "gmail" && source.SourceType != "imap" { + continue + } + if match != nil { + return nil, errImportAmbiguousSource + } + match = source + } + if match != nil { + return match, nil + } + if selectorFound { + return nil, errImportUnsyncable + } + return nil, store.ErrSourceNotFound +} diff --git a/internal/api/import_jobs_test.go b/internal/api/import_jobs_test.go new file mode 100644 index 000000000..a564fbbc9 --- /dev/null +++ b/internal/api/import_jobs_test.go @@ -0,0 +1,352 @@ +package api + +import ( + "bytes" + "context" + "database/sql" + "encoding/json" + "errors" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "sync" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.kenn.io/msgvault/internal/config" + "go.kenn.io/msgvault/internal/store" +) + +type importJobTestStore struct { + *mockStore + + mu sync.Mutex + sources map[int64]*store.Source + active *store.SyncRun + operations map[string]*store.SyncOperation + entered chan CLISyncRequest + started chan CLISyncRequest + allowStart <-chan struct{} + release chan struct{} + releaseMu sync.Once + runErr error +} + +func newImportJobTestStore() *importJobTestStore { + source := &store.Source{ID: 42, SourceType: "gmail", Identifier: "archive@example.com"} + return &importJobTestStore{ + mockStore: &mockStore{sourcesByLookup: map[string][]*store.Source{ + "archive@example.com": {source}, + }}, + sources: map[int64]*store.Source{42: source}, operations: make(map[string]*store.SyncOperation), + entered: make(chan CLISyncRequest, 1), started: make(chan CLISyncRequest, 1), release: make(chan struct{}), + } +} + +func (s *importJobTestStore) RunCLISync(ctx context.Context, req CLISyncRequest, _ func(CLISyncEvent) error) error { + s.entered <- req + if s.allowStart != nil { + select { + case <-s.allowStart: + case <-ctx.Done(): + return ctx.Err() + } + } + startedAt := time.Now().UTC() + s.mu.Lock() + op := s.operations[req.OperationID] + op.Status = "running" + op.StartedAt = sql.NullTime{Time: startedAt, Valid: true} + op.Runs = []*store.SyncRun{{ID: 100, SourceID: req.SourceID, StartedAt: startedAt, Status: store.SyncStatusRunning}} + s.mu.Unlock() + s.started <- req + select { + case <-s.release: + case <-ctx.Done(): + s.runErr = ctx.Err() + } + s.mu.Lock() + op = s.operations[req.OperationID] + finishedAt := time.Now().UTC() + op.FinishedAt = sql.NullTime{Time: finishedAt, Valid: true} + if s.runErr != nil { + op.Status = "failed" + } else { + op.Status = "done" + for _, run := range op.Runs { + run.Status = store.SyncStatusCompleted + run.CompletedAt = sql.NullTime{Time: finishedAt, Valid: true} + } + } + s.mu.Unlock() + return s.runErr +} + +func (s *importJobTestStore) CreateSyncOperation(sourceID int64, id string) (*store.SyncOperation, error) { + s.mu.Lock() + defer s.mu.Unlock() + createdAt := time.Now().UTC() + op := &store.SyncOperation{ + ID: id, SourceID: sourceID, Status: "pending", CreatedAt: createdAt, + } + s.operations[id] = op + clone := *op + return &clone, nil +} + +func TestImportJobCreationDoesNotWaitForWorkerStartup(t *testing.T) { + assert := assert.New(t) + require := require.New(t) + st := newImportJobTestStore() + allowStart := make(chan struct{}) + st.allowStart = allowStart + t.Cleanup(func() { + close(allowStart) + st.finish() + }) + srv := NewServer(&config.Config{}, st, nil, testLogger()) + req := httptest.NewRequest(http.MethodPost, "/api/v1/imports", strings.NewReader(`{"account":"archive@example.com"}`)) + req.Header.Set("Content-Type", applicationJSONMediaType) + resp := httptest.NewRecorder() + responseDone := make(chan struct{}) + go func() { + srv.Router().ServeHTTP(resp, req) + close(responseDone) + }() + + <-st.entered + select { + case <-responseDone: + assert.Equal(http.StatusAccepted, resp.Code, resp.Body.String()) + var result importJobTestResponse + require.NoError(json.NewDecoder(resp.Body).Decode(&result)) + assert.Equal("pending", result.Status) + assert.Nil(result.StartedAt) + case <-time.After(time.Second): + require.Fail("POST /imports waited for worker startup") + } +} + +func (s *importJobTestStore) GetSourceByID(id int64) (*store.Source, error) { + s.mu.Lock() + defer s.mu.Unlock() + source := s.sources[id] + if source == nil { + return nil, store.ErrSourceNotFound + } + clone := *source + return &clone, nil +} + +func (s *importJobTestStore) GetActiveSync(sourceID int64) (*store.SyncRun, error) { + s.mu.Lock() + defer s.mu.Unlock() + if s.active == nil || s.active.SourceID != sourceID { + return nil, store.ErrSyncRunNotFound + } + clone := *s.active + return &clone, nil +} + +func (s *importJobTestStore) GetSyncOperation(id string) (*store.SyncOperation, error) { + s.mu.Lock() + defer s.mu.Unlock() + op := s.operations[id] + if op == nil { + return nil, store.ErrSyncRunNotFound + } + clone := *op + clone.Runs = make([]*store.SyncRun, len(op.Runs)) + for i, run := range op.Runs { + runClone := *run + clone.Runs[i] = &runClone + } + return &clone, nil +} + +func (s *importJobTestStore) finish() { s.releaseMu.Do(func() { close(s.release) }) } + +type importJobTestResponse struct { + JobID string `json:"job_id"` + Account string `json:"account"` + Status string `json:"status"` + Processed int64 `json:"processed"` + Added int64 `json:"added"` + Skipped int64 `json:"skipped"` + Error string `json:"error"` + CreatedAt time.Time `json:"created_at"` + StartedAt *time.Time `json:"started_at"` + FinishedAt *time.Time `json:"finished_at"` + Summary *ImportJobSummary `json:"summary"` +} + +func submitImportJob(t *testing.T, srv *Server, body, apiKey string) importJobTestResponse { + t.Helper() + req := httptest.NewRequest(http.MethodPost, "/api/v1/imports", bytes.NewBufferString(body)) + req.Header.Set("Content-Type", applicationJSONMediaType) + if apiKey != "" { + req.Header.Set("X-Api-Key", apiKey) + } + resp := httptest.NewRecorder() + srv.Router().ServeHTTP(resp, req) + require.Equal(t, http.StatusAccepted, resp.Code, resp.Body.String()) + var result importJobTestResponse + require.NoError(t, json.NewDecoder(resp.Body).Decode(&result)) + require.NotEmpty(t, result.JobID) + return result +} + +func getImportJob(t *testing.T, srv *Server, jobID string) (int, importJobTestResponse, string) { + t.Helper() + req := httptest.NewRequest(http.MethodGet, "/api/v1/imports/"+jobID, nil) + resp := httptest.NewRecorder() + srv.Router().ServeHTTP(resp, req) + var result importJobTestResponse + if resp.Code == http.StatusOK { + require.NoError(t, json.NewDecoder(resp.Body).Decode(&result)) + } + return resp.Code, result, resp.Body.String() +} + +func TestImportJobUsesDurableSyncOperationForProgressAndSummary(t *testing.T) { + assert := assert.New(t) + require := require.New(t) + st := newImportJobTestStore() + t.Cleanup(st.finish) + srv := NewServerWithOptions(ServerOptions{Config: &config.Config{}, Store: st, OperationGate: NewSerialOperationGate(), Logger: testLogger()}) + + created := submitImportJob(t, srv, `{"account":"archive@example.com","after":"2024-01-01","limit":25}`, "") + assert.Equal("pending", created.Status) + assert.False(created.CreatedAt.IsZero()) + assert.Nil(created.StartedAt) + req := <-st.started + assert.Equal(created.JobID, req.OperationID) + assert.Equal("2024-01-01", req.After) + assert.Equal(25, req.Limit) + + st.mu.Lock() + op := st.operations[created.JobID] + op.Runs[0].MessagesProcessed = 12 + op.Runs[0].MessagesAdded = 7 + op.Runs[0].MessagesUpdated = 2 + op.Runs = append(op.Runs, &store.SyncRun{ + ID: 101, SourceID: 42, StartedAt: time.Now().UTC(), Status: store.SyncStatusRunning, + MessagesProcessed: 5, MessagesAdded: 2, MessagesUpdated: 1, ErrorsCount: 1, + }) + st.mu.Unlock() + + code, running, body := getImportJob(t, srv, created.JobID) + require.Equal(http.StatusOK, code, body) + assert.Equal("running", running.Status) + assert.Equal(int64(17), running.Processed) + assert.Equal(int64(9), running.Added) + assert.Equal(int64(5), running.Skipped) + + st.finish() + require.Eventually(func() bool { + _, result, _ := getImportJob(t, srv, created.JobID) + return result.Status == "done" && result.Summary != nil + }, time.Second, 10*time.Millisecond) +} + +func TestImportJobRejectsAccountWithActiveSync(t *testing.T) { + st := newImportJobTestStore() + st.active = &store.SyncRun{ID: 7, SourceID: 42, Status: store.SyncStatusRunning} + t.Cleanup(st.finish) + srv := NewServer(&config.Config{}, st, nil, testLogger()) + req := httptest.NewRequest(http.MethodPost, "/api/v1/imports", strings.NewReader(`{"account":"archive@example.com"}`)) + req.Header.Set("Content-Type", applicationJSONMediaType) + resp := httptest.NewRecorder() + srv.Router().ServeHTTP(resp, req) + assert.Equal(t, http.StatusConflict, resp.Code, resp.Body.String()) + assert.Empty(t, st.started) +} + +func TestImportJobCreationHasNoOrdinaryRequestDeadline(t *testing.T) { + srv := NewServer(&config.Config{}, nil, nil, testLogger()) + t.Cleanup(func() { require.NoError(t, srv.Shutdown(context.Background())) }) + + _, bounded := srv.requestTimeoutForPath(importJobsEndpointPath) + assert.False(t, bounded) +} + +func TestImportJobFailureIsSanitized(t *testing.T) { + st := newImportJobTestStore() + st.runErr = errors.New("oauth secret-value rejected") + t.Cleanup(st.finish) + srv := NewServer(&config.Config{}, st, nil, testLogger()) + created := submitImportJob(t, srv, `{"account":"archive@example.com"}`, "") + st.finish() + require.Eventually(t, func() bool { + _, result, _ := getImportJob(t, srv, created.JobID) + return result.Status == "failed" + }, time.Second, 10*time.Millisecond) + _, failed, _ := getImportJob(t, srv, created.JobID) + assert.Equal(t, "import failed", failed.Error) + assert.NotContains(t, failed.Error, "secret-value") +} + +func TestImportJobReturnsBusyInsteadOfBuildingASecondQueue(t *testing.T) { + st := newImportJobTestStore() + gate := NewSerialOperationGate() + release, ok := gate.BeginLabeledWorkContext(t.Context(), "another archive operation") + require.True(t, ok) + defer release() + srv := NewServerWithOptions(ServerOptions{Config: &config.Config{}, Store: st, OperationGate: gate, Logger: testLogger()}) + oldLimit := operationGateWaitLimit + operationGateWaitLimit = 10 * time.Millisecond + t.Cleanup(func() { operationGateWaitLimit = oldLimit }) + + req := httptest.NewRequest(http.MethodPost, "/api/v1/imports", strings.NewReader(`{"account":"archive@example.com"}`)) + req.Header.Set("Content-Type", applicationJSONMediaType) + resp := httptest.NewRecorder() + srv.Router().ServeHTTP(resp, req) + assert.Equal(t, http.StatusServiceUnavailable, resp.Code, resp.Body.String()) +} + +func TestImportJobRequestValidation(t *testing.T) { + st := newImportJobTestStore() + srv := NewServer(&config.Config{}, st, nil, testLogger()) + tests := []struct { + body, contentType string + want int + }{ + {`{}`, "", http.StatusUnsupportedMediaType}, + {`{`, applicationJSONMediaType, http.StatusBadRequest}, + {`{}`, applicationJSONMediaType, http.StatusUnprocessableEntity}, + {`{"account":"archive@example.com","after":"yesterday"}`, applicationJSONMediaType, http.StatusUnprocessableEntity}, + {`{"account":"archive@example.com","after":"2024-02-01","before":"2024-01-01"}`, applicationJSONMediaType, http.StatusUnprocessableEntity}, + {`{"account":"missing@example.com"}`, applicationJSONMediaType, http.StatusNotFound}, + } + for i, tt := range tests { + t.Run(fmt.Sprintf("case-%d", i), func(t *testing.T) { + req := httptest.NewRequest(http.MethodPost, "/api/v1/imports", strings.NewReader(tt.body)) + if tt.contentType != "" { + req.Header.Set("Content-Type", tt.contentType) + } + resp := httptest.NewRecorder() + srv.Router().ServeHTTP(resp, req) + assert.Equal(t, tt.want, resp.Code, resp.Body.String()) + }) + } +} + +func TestImportJobsRequireAuthentication(t *testing.T) { + st := newImportJobTestStore() + t.Cleanup(st.finish) + srv := NewServer(&config.Config{Server: config.ServerConfig{APIKey: "key"}}, st, nil, testLogger()) + req := httptest.NewRequest(http.MethodPost, "/api/v1/imports", strings.NewReader(`{"account":"archive@example.com"}`)) + req.Header.Set("Content-Type", applicationJSONMediaType) + resp := httptest.NewRecorder() + srv.Router().ServeHTTP(resp, req) + assert.Equal(t, http.StatusUnauthorized, resp.Code) +} + +func TestGetImportJobReturnsNotFoundForUnknownID(t *testing.T) { + srv := NewServer(&config.Config{}, newImportJobTestStore(), nil, testLogger()) + code, _, body := getImportJob(t, srv, "missing") + assert.Equal(t, http.StatusNotFound, code, body) +} diff --git a/internal/api/openapi.go b/internal/api/openapi.go index c6cd33c95..29d434675 100644 --- a/internal/api/openapi.go +++ b/internal/api/openapi.go @@ -242,7 +242,10 @@ import ( // clients continue to receive the same result population. // 2.13.0 makes deduplicate planning use an explicit, version-gated backfill // confirmation protocol. -const APISchemaVersion = "2.13.0" +// 2.14.0 adds authenticated asynchronous historical import jobs at +// POST /api/v1/imports and GET /api/v1/imports/{job_id}. Existing synchronous +// CLI sync routes and source-status responses are unchanged. +const APISchemaVersion = "2.14.0" // OpenAPIDocument builds the API schema from the same Huma route registration // used by the daemon. It binds no socket and needs no database. @@ -707,6 +710,14 @@ func applyClientCodegenExtensions(doc *huma.OpenAPI) { "MeetingImportResponseStatusUpdated", }) } + if response := schemas["ImportJobResponse"]; response != nil { + setEnumNames(response.Properties["status"], []any{ + "ImportJobResponseStatusPending", + "ImportJobResponseStatusRunning", + "ImportJobResponseStatusDone", + "ImportJobResponseStatusFailed", + }) + } for schemaName, properties := range map[string]map[string][]any{ "AppendPersonNoteRequest": { exploreFilterSource: { diff --git a/internal/api/openapi_test.go b/internal/api/openapi_test.go index 3182ab25c..041d1847a 100644 --- a/internal/api/openapi_test.go +++ b/internal/api/openapi_test.go @@ -34,8 +34,41 @@ func TestOpenAPIDocumentUsesAPISchemaVersion(t *testing.T) { assert.NotEmpty(t, doc.Paths, "paths") } -func TestOpenAPISchemaVersionDeduplicatePlanProtocolIs2130(t *testing.T) { - assert.Equal(t, "2.13.0", APISchemaVersion) +func TestOpenAPISchemaVersionDeduplicateAndAsyncImportsIs2140(t *testing.T) { + assert.Equal(t, "2.14.0", APISchemaVersion) +} + +func TestOpenAPIImportJobContract(t *testing.T) { + assert := assert.New(t) + require := require.New(t) + doc := OpenAPIDocument() + + createPath := doc.Paths["/api/v1/imports"] + require.NotNil(createPath, "import collection path") + require.NotNil(createPath.Post, "create import operation") + assert.Equal("createImportJob", createPath.Post.OperationID) + require.Len(createPath.Post.Security, 1) + _, secured := createPath.Post.Security[0][apiKeySecurityScheme] + assert.True(secured, "create import requires API-key security") + require.NotNil(createPath.Post.RequestBody) + requestMedia := createPath.Post.RequestBody.Content[applicationJSONMediaType] + require.NotNil(requestMedia) + assert.Equal("#/components/schemas/ImportJobRequest", requestMedia.Schema.Ref) + accepted := createPath.Post.Responses["202"] + require.NotNil(accepted, "create import documents 202") + acceptedMedia := accepted.Content[applicationJSONMediaType] + require.NotNil(acceptedMedia) + assert.Equal("#/components/schemas/ImportJobResponse", acceptedMedia.Schema.Ref) + assert.Contains(createPath.Post.Responses, "503", "operation-gate contention is documented") + + statusPath := doc.Paths["/api/v1/imports/{job_id}"] + require.NotNil(statusPath, "import status path") + require.NotNil(statusPath.Get, "get import operation") + assert.Equal("getImportJob", statusPath.Get.OperationID) + require.Len(statusPath.Get.Parameters, 1) + assert.Equal("job_id", statusPath.Get.Parameters[0].Name) + assert.Equal("path", statusPath.Get.Parameters[0].In) + assert.True(statusPath.Get.Parameters[0].Required) } func TestCLISearchOpenAPIDocumentsDeletionScope(t *testing.T) { @@ -133,7 +166,7 @@ func TestOpenAPISeparatesParticipantAnalyticsFromDurablePeople(t *testing.T) { assert := assert.New(t) doc := OpenAPIDocument() - assert.Equal("2.13.0", APISchemaVersion) + assert.Equal("2.14.0", APISchemaVersion) for _, path := range []string{ "/api/v1/participants/search", "/api/v1/participants/{id}", @@ -155,11 +188,11 @@ func TestOpenAPISeparatesParticipantAnalyticsFromDurablePeople(t *testing.T) { } func TestAnalyticsCacheReadinessUsesAdditiveSchemaVersion(t *testing.T) { - assert.Equal(t, "2.13.0", APISchemaVersion) + assert.Equal(t, "2.14.0", APISchemaVersion) } func TestPersonFilesUseAdditiveSchemaVersion(t *testing.T) { - assert.Equal(t, "2.13.0", APISchemaVersion) + assert.Equal(t, "2.14.0", APISchemaVersion) } func TestPersonFileRoutesPublishTypedPathIDs(t *testing.T) { @@ -183,7 +216,7 @@ func TestPersonFileRoutesPublishTypedPathIDs(t *testing.T) { func TestOrganizationCreateOpenAPIDocumentsLocationHeader(t *testing.T) { require := require.New(t) - assert.Equal(t, "2.13.0", APISchemaVersion, + assert.Equal(t, "2.14.0", APISchemaVersion, "document and person-file search preserve the organization and employment contract") for _, document := range []*huma.OpenAPI{ OpenAPIDocument(), @@ -507,7 +540,7 @@ func TestOpenAPISearchDocumentsConversationID(t *testing.T) { func TestOpenAPIPersonAttributeContract(t *testing.T) { require := require.New(t) assert := assert.New(t) - assert.Equal("2.13.0", APISchemaVersion, + assert.Equal("2.14.0", APISchemaVersion, "activity, identity match review, document search, and person files preserve the structured profile contract") doc := OpenAPIDocument() @@ -621,7 +654,7 @@ func TestOpenAPIPersonProfilePatchUsesWritableEnvelopeShape(t *testing.T) { func TestOpenAPIOrganizationProfilePutDocumentsLimits(t *testing.T) { assertions := assert.New(t) requirements := require.New(t) - assertions.Equal("2.13.0", APISchemaVersion, + assertions.Equal("2.14.0", APISchemaVersion, "organization profile write limits advance the published contract") doc := OpenAPIDocument() path := doc.Paths["/api/v1/organizations/{id}/profile"] @@ -641,7 +674,7 @@ func TestOpenAPIPersonProfileMediaContentContract(t *testing.T) { require := require.New(t) assert := assert.New(t) - assert.Equal("2.13.0", APISchemaVersion, + assert.Equal("2.14.0", APISchemaVersion, "activity, identity match review, document search, and person files preserve the raw profile media contract") doc := OpenAPIDocument() path := doc.Paths["/api/v1/people/{id}/profile/media/{media_id}/content"] @@ -669,7 +702,7 @@ func TestOpenAPIIdentityMatchReviewContract(t *testing.T) { requirements := require.New(t) assertions := assert.New(t) - assertions.Equal("2.13.0", APISchemaVersion, + assertions.Equal("2.14.0", APISchemaVersion, "document and person-file search preserve the identity match review contract") doc := OpenAPIDocument() @@ -716,8 +749,9 @@ func TestOpenAPIMeetingImportContract(t *testing.T) { // 2.5.0. Person search in 2.6.0, structured filters in 2.7.0, CardDAV routes // in 2.8.0, person merge/split operations in 2.9.0, and relationship // calendars in 2.10.0, person fact diagnostics in 2.11.0, and lexical - // deletion scope in 2.12.0 did not touch it. - assert.Equal("2.13.0", APISchemaVersion, "meeting import remains in the additive schema") + // deletion scope in 2.12.0, deduplicate planning in 2.13.0, and historical + // import jobs in 2.14.0 did not touch it. + assert.Equal("2.14.0", APISchemaVersion, "meeting import is an additive schema release") doc := OpenAPIDocument() path := doc.Paths["/api/v1/import/meeting"] diff --git a/internal/api/operation_gate.go b/internal/api/operation_gate.go index 960056364..4944903ec 100644 --- a/internal/api/operation_gate.go +++ b/internal/api/operation_gate.go @@ -344,7 +344,8 @@ func writeOperationGateBusy(w http.ResponseWriter, gate OperationGate) { // NOT exempt: its subprocess opens the store read-write and runs schema // init/migrations. // -// Backup freeze begin and meeting import coordinate the gate in their handlers. +// Backup freeze begin, meeting import, and historical import jobs coordinate +// the gate in their handlers. // Meeting import first reads and validates its bounded request body so a slow // authenticated upload cannot hold the gate. Backup freeze end bypasses the // gate so it can release the freeze held by begin. Routing these through the @@ -353,6 +354,7 @@ var operationGateExemptPaths = map[string]bool{ queryEndpointPath: true, sessionPath: true, sessionLoginPath: true, + importJobsEndpointPath: true, meetingImportEndpointPath: true, "/api/v1/cli/add-calendar/plan": true, "/api/v1/cli/delete-staged/plan": true, diff --git a/internal/api/relationship_calendar_test.go b/internal/api/relationship_calendar_test.go index d892f6dca..63235115a 100644 --- a/internal/api/relationship_calendar_test.go +++ b/internal/api/relationship_calendar_test.go @@ -177,7 +177,7 @@ func TestRelationshipCalendarRequestRejectsTrailingJSON(t *testing.T) { func TestRelationshipCalendarOpenAPIContract(t *testing.T) { assert := assert.New(t) require := require.New(t) - assert.Equal("2.13.0", APISchemaVersion) + assert.Equal("2.14.0", APISchemaVersion) document := OpenAPIDocument() path := document.Paths["/api/v1/relationships/{id}/calendar"] require.NotNil(path) diff --git a/internal/api/routes.go b/internal/api/routes.go index 7b1004cd8..aa6c2f181 100644 --- a/internal/api/routes.go +++ b/internal/api/routes.go @@ -224,6 +224,7 @@ func (s *Server) registerHumaRoutes(api huma.API, apiV1 huma.API) { }, s.handleDaemonShutdown) registerAPIV1RawHumaJSONRoute[StatsResponse](apiV1, "getStats", http.MethodGet, "/stats", "Get archive statistics", s.handleStats) + s.registerImportJobRoutes(apiV1) s.registerSettingsRoutes(apiV1) s.registerCardDAVRoutes(apiV1) s.registerSavedViewRoutes(apiV1) @@ -612,6 +613,8 @@ func rawRouteParameters(operationID string) []*huma.Param { switch operationID { case "getCLIStats": return scopeParams() + case "getImportJob": + return []*huma.Param{pathStringParam("job_id", "Historical import job ID")} case "searchCLI": return append([]*huma.Param{ queryStringParam("q", "Search query", true), diff --git a/internal/api/server.go b/internal/api/server.go index 6ddd089a7..8356bff87 100644 --- a/internal/api/server.go +++ b/internal/api/server.go @@ -270,6 +270,11 @@ type Server struct { visualCoverageRateLimiter *RateLimiter idleTracker *IdleTracker operationGate OperationGate + importContext context.Context + cancelImports context.CancelFunc + importMu sync.Mutex + importsClosed bool + importWG sync.WaitGroup // ftsIndexComplete memoizes that the FTS index is fully populated so // handleCLISearch stops probing on every request. NeedsFTSBackfill runs an // anti-join that scans every message when the index is complete (the @@ -518,6 +523,7 @@ func NewServerWithOptions(opts ServerOptions) *Server { if fastmailInventoryFactory == nil { fastmailInventoryFactory = provideridentity.NewFastmailInventory } + importContext, cancelImports := context.WithCancel(context.Background()) s := &Server{ cfg: opts.Config, store: opts.Store, @@ -540,6 +546,8 @@ func NewServerWithOptions(opts ServerOptions) *Server { daemonVersion: opts.DaemonVersion, idleTracker: opts.IdleTracker, operationGate: opts.OperationGate, + importContext: importContext, + cancelImports: cancelImports, blobStore: opts.BlobStore, remoteImages: newRemoteImageFetcher(), inlineCache: newInlineParseCache(inlineCacheMaxEntries, inlineCacheMaxBytes), @@ -777,6 +785,23 @@ func (s *Server) StartOnListener(ln net.Listener) error { // Shutdown gracefully shuts down the server. func (s *Server) Shutdown(ctx context.Context) error { + s.importMu.Lock() + s.importsClosed = true + if s.cancelImports != nil { + s.cancelImports() + } + s.importMu.Unlock() + importsDone := make(chan struct{}) + go func() { + s.importWG.Wait() + close(importsDone) + }() + var importJobsErr error + select { + case <-importsDone: + case <-ctx.Done(): + importJobsErr = ctx.Err() + } if s.rateLimiter != nil { s.rateLimiter.Close() } @@ -796,10 +821,10 @@ func (s *Server) Shutdown(ctx context.Context) error { server := s.server s.serverMu.RUnlock() if server == nil { - return nil + return importJobsErr } s.logger.Info("shutting down API server") - return server.Shutdown(ctx) + return errors.Join(importJobsErr, server.Shutdown(ctx)) } // Router returns the HTTP router for testing. @@ -1088,6 +1113,7 @@ func (s *Server) requestTimeoutForPath(path string) (time.Duration, bool) { func isLongDaemonRequest(path string) bool { switch path { case "/api/v1/cli/build-cache", + importJobsEndpointPath, "/api/v1/carddav/sync", "/api/v1/cli/deduplicate/plan", meetingImportEndpointPath, diff --git a/internal/beeper/importer_test.go b/internal/beeper/importer_test.go index d72925578..353408778 100644 --- a/internal/beeper/importer_test.go +++ b/internal/beeper/importer_test.go @@ -25,8 +25,7 @@ func TestImporterScopesEveryStoreOwningHelper(t *testing.T) { imp := NewImporter(st, nil) imp.res = newParticipantResolver(st, "account-a") scoped := imp.scopedToSync(source.ID, runID) - _, err = st.StartSync(source.ID, sourceTypeBeeper) - requirements.NoError(err) + requirements.NoError(st.FailSync(runID, "worker stopped")) _, err = scoped.res.resolveID("user-a", "User A") requirements.ErrorIs(err, store.ErrSyncRunSuperseded) diff --git a/internal/calsync/calsync.go b/internal/calsync/calsync.go index 2b1da713b..c4d4d252e 100644 --- a/internal/calsync/calsync.go +++ b/internal/calsync/calsync.go @@ -198,11 +198,21 @@ func (s *Syncer) listCalendars(ctx context.Context) ([]gcal.Calendar, error) { // set. Bounded/limited runs deliberately do not checkpoint page tokens because // replaying that token under a later unbounded request can skip or corrupt the // traversal. -func (s *Syncer) syncCalendarFull(ctx context.Context, cal gcal.Calendar, result *Result) error { +func (s *Syncer) syncCalendarFull( + ctx context.Context, cal gcal.Calendar, result *Result, +) (retErr error) { src, err := s.getOrCreateCalendarSource(ctx, cal) if err != nil { return fmt.Errorf("get/create source: %w", err) } + ownershipCtx := context.WithoutCancel(ctx) + execution, err := s.store.AcquireSyncExecutionContext(ownershipCtx, src.ID) + if err != nil { + return fmt.Errorf("acquire sync execution: %w", err) + } + defer func() { + retErr = errors.Join(retErr, execution.Release()) + }() if err := s.store.UpdateSourceSyncConfig(src.ID, s.sourceConfigJSON(cal)); err != nil { return fmt.Errorf("write sync config: %w", err) } @@ -210,30 +220,26 @@ func (s *Syncer) syncCalendarFull(ctx context.Context, cal gcal.Calendar, result return err } - // Resume an interrupted run from its checkpoint, then ALWAYS StartSync. We do - // NOT reuse the prior run's id: StartSync is the only path that takes the - // source row's writer lock and supersedes other 'running' runs, so going - // through it serializes concurrent/overlapping full syncs (a manual run racing - // the daemon) instead of two callers sharing — and clobbering — one sync_run - // row. The prior run's counters are carried forward so a resumed run's stats - // stay accurate (UpdateSyncCheckpoint overwrites counters absolutely). + // Resume a stopped run from its checkpoint, then start a new run. StartSync + // rejects a running sync, so a live worker cannot be replaced. The prior + // run's counters are carried forward so resumed stats stay accurate. var resumePageToken string var priorProcessed, priorAdded, priorUpdated int64 resumeEligible := s.fullSyncResumeEligible() if resumeEligible { - if active, _ := s.store.GetActiveSync(src.ID); active != nil && active.Status == store.SyncStatusRunning { - if pageToken, ok := decodeCalendarFullCheckpoint(active.CursorBefore); ok { + if prior, _ := s.store.GetLatestCheckpointedSyncByType(src.ID, "full"); prior != nil { + if pageToken, ok := decodeCalendarFullCheckpoint(prior.CursorBefore); ok { resumePageToken = pageToken - priorProcessed = active.MessagesProcessed - priorAdded = active.MessagesAdded - priorUpdated = active.MessagesUpdated + priorProcessed = prior.MessagesProcessed + priorAdded = prior.MessagesAdded + priorUpdated = prior.MessagesUpdated s.logger.Info("resuming interrupted calendar sync", "calendar", cal.ID, "page_token", resumePageToken) } else { s.logger.Info("ignoring legacy calendar sync checkpoint; restarting full sync", "calendar", cal.ID) } } } - syncID, err := s.store.StartSync(src.ID, "full") + syncID, err := execution.StartSyncContext(ownershipCtx, "full", "") if err != nil { return fmt.Errorf("start sync: %w", err) } @@ -252,14 +258,8 @@ func (s *Syncer) syncCalendarFull(ctx context.Context, cal gcal.Calendar, result ingested := 0 limitHit := false - // A cancelled sync (Ctrl-C, daemon shutdown, a scheduled sync yielding to - // a waiting operation) keeps status='running' with its saved checkpoint so - // the next full sync resumes; marking it failed would discard the - // checkpoint and restart from scratch. fail := func(e error) error { - if !errors.Is(e, context.Canceled) && !errors.Is(e, context.DeadlineExceeded) { - _ = s.store.FailSync(syncID, e.Error()) - } + _ = s.store.FailSync(syncID, e.Error()) return e } diff --git a/internal/calsync/review_fixes_test.go b/internal/calsync/review_fixes_test.go index bdd884c64..93e6e1a70 100644 --- a/internal/calsync/review_fixes_test.go +++ b/internal/calsync/review_fixes_test.go @@ -3,6 +3,7 @@ package calsync import ( "context" "errors" + "path/filepath" "testing" "time" @@ -418,6 +419,7 @@ func TestFull_LegacyResumeCheckpointIgnored(t *testing.T) { require.NoError(st.UpdateSyncCheckpoint(oldSyncID, &store.Checkpoint{ PageToken: "1", MessagesProcessed: 2, MessagesAdded: 2, })) + require.NoError(st.FailSync(oldSyncID, "worker stopped")) recorder := &listEventsRecorder{MockAPI: m} s := New(recorder, st, Options{AccountEmail: testAccount}).WithLogger(quietLogger()) @@ -514,11 +516,10 @@ func TestIncremental_PersistErrorDoesNotAdvanceCursor(t *testing.T) { assert.Equal("T1", src2.SyncCursor.String, "cursor must NOT advance past an event that failed to persist") } -// TestFull_ResumeSupersedesAndSeedsCounters is the regression for the resume -// path: it must go through StartSync (so a stale/concurrent running run is -// superseded under the writer lock, not shared) and carry the prior run's -// counters forward so a resumed run's stats are not reset to zero. -func TestFull_ResumeSupersedesAndSeedsCounters(t *testing.T) { +// TestFull_ResumeFailedRunAndSeedsCounters is the regression for the resume +// path: it starts a new run from a stopped worker's checkpoint and carries the +// prior counters forward so a resumed run's stats are not reset to zero. +func TestFull_ResumeFailedRunAndSeedsCounters(t *testing.T) { assert := assert.New(t) require := require.New(t) @@ -529,7 +530,7 @@ func TestFull_ResumeSupersedesAndSeedsCounters(t *testing.T) { s, st := newSyncer(t, m, Options{}) - // Simulate an interrupted prior run: a 'running' sync_run with 2 already + // Simulate an interrupted prior run with 2 already // processed, checkpointed to resume from the first page (""). src, err := st.GetOrCreateSource(gcal.SourceType, testAccount+"/primary") require.NoError(err) @@ -539,15 +540,16 @@ func TestFull_ResumeSupersedesAndSeedsCounters(t *testing.T) { require.NoError(st.UpdateSyncCheckpoint(oldSyncID, &store.Checkpoint{ PageToken: encodeCalendarFullCheckpoint(""), MessagesProcessed: 2, MessagesAdded: 2, })) + require.NoError(st.FailSync(oldSyncID, "worker stopped")) _, err = s.Full(context.Background()) require.NoError(err) - // The old run was superseded (no longer 'running'). + // The old run remains failed. var oldStatus string require.NoError(st.DB().QueryRow( st.Rebind("SELECT status FROM sync_runs WHERE id = ?"), oldSyncID).Scan(&oldStatus)) - assert.NotEqual(store.SyncStatusRunning, oldStatus, "resume must supersede the prior running run via StartSync") + assert.Equal(store.SyncStatusFailed, oldStatus) // The completed run's counter includes the 2 seeded + 1 newly ingested. var processed int64 @@ -557,6 +559,47 @@ func TestFull_ResumeSupersedesAndSeedsCounters(t *testing.T) { assert.Equal(int64(3), processed, "resumed run seeds prior counters (2) + new (1)") } +func TestFull_ResumesCheckpointAfterWorkerExit(t *testing.T) { + assert := assert.New(t) + require := require.New(t) + tmp := t.TempDir() + dbPath := filepath.Join(tmp, "msgvault.db") + + first, err := store.Open(dbPath) + require.NoError(err) + require.NoError(first.InitSchema()) + src, err := first.GetOrCreateSource(gcal.SourceType, testAccount+"/primary") + require.NoError(err) + require.NoError(first.UpdateSourceSyncConfig( + src.ID, `{"account_email":"`+testAccount+`","calendar_id":"primary"}`, + )) + runID, err := first.StartSync(src.ID, "full") + require.NoError(err) + require.NoError(first.UpdateSyncCheckpoint(runID, &store.Checkpoint{ + PageToken: encodeCalendarFullCheckpoint("1"), MessagesProcessed: 1, MessagesAdded: 1, + })) + require.NoError(first.Close()) + + second, err := store.Open(dbPath) + require.NoError(err) + t.Cleanup(func() { _ = second.Close() }) + mock := gcal.NewMockAPI() + mock.Calendars = []gcal.Calendar{{ID: "primary", AccessRole: "owner"}} + mock.FullEvents["primary"] = [][]gcal.Event{ + {timedEvent("e1", "One")}, + {timedEvent("e2", "Two")}, + } + mock.FullSyncToken["primary"] = "T1" + recorder := &listEventsRecorder{MockAPI: mock} + syncer := New(recorder, second, Options{AccountEmail: testAccount}).WithLogger(quietLogger()) + + _, err = syncer.Full(t.Context()) + + require.NoError(err) + require.NotEmpty(recorder.params) + assert.Equal("1", recorder.params[0].PageToken) +} + func TestIncremental_ExpiredCursorCannotEraseNewerGeneration(t *testing.T) { requirements := require.New(t) checks := assert.New(t) @@ -572,10 +615,10 @@ func TestIncremental_ExpiredCursorCannotEraseNewerGeneration(t *testing.T) { syncer := New(api, st, Options{AccountEmail: testAccount}).WithLogger(quietLogger()) _, err = syncer.Incremental(t.Context()) - requirements.ErrorIs(err, store.ErrSyncRunSuperseded) + requirements.ErrorIs(err, store.ErrSyncAlreadyActive) src, err = st.GetSourceByID(src.ID) requirements.NoError(err) - checks.Equal("newer-token", src.SyncCursor.String) + checks.Equal("expired-token", src.SyncCursor.String) checks.False(api.fullSyncAttempted) } diff --git a/internal/fbmessenger/importer.go b/internal/fbmessenger/importer.go index 57d30fe99..a5878542c 100644 --- a/internal/fbmessenger/importer.go +++ b/internal/fbmessenger/importer.go @@ -140,33 +140,23 @@ func ImportDYI(ctx context.Context, st *store.Store, opts ImportOptions) (*Impor return nil, fmt.Errorf("fbmessenger: source: %w", err) } - // Read any existing active-run checkpoint before calling - // StartSync (which marks active runs failed). This mirrors the - // pattern used by emlx_import / mbox_import. + // Resume only a checkpoint whose worker has stopped. StartSync rejects a + // running sync, so a live worker cannot be replaced by this import. var ( startThreadIdx int cp store.Checkpoint ) if !opts.NoResume { - // Look for a resumable sync run. Try active (running) first, - // then fall back to the latest checkpointed run (which includes - // failed/interrupted runs whose checkpoint is still valid). - prev, err := st.GetActiveSync(source.ID) + prev, err := st.GetLatestCheckpointedSync(source.ID) if err != nil && !errors.Is(err, store.ErrSyncRunNotFound) { - return nil, fmt.Errorf("fbmessenger: check active sync: %w", err) - } - if prev == nil || !prev.CursorBefore.Valid || prev.CursorBefore.String == "" { - prev, err = st.GetLatestCheckpointedSync(source.ID) - if err != nil && !errors.Is(err, store.ErrSyncRunNotFound) { - return nil, fmt.Errorf("fbmessenger: check checkpointed sync: %w", err) - } + return nil, fmt.Errorf("fbmessenger: check checkpointed sync: %w", err) } if prev != nil && prev.CursorBefore.Valid && prev.CursorBefore.String != "" { var prior fbmessengerCheckpoint if err := json.Unmarshal([]byte(prev.CursorBefore.String), &prior); err == nil { if prior.RootDir != "" && prior.RootDir != absRoot { return nil, fmt.Errorf( - "fbmessenger: active import is for a different root (%q), not %q; rerun with --no-resume to start fresh", + "fbmessenger: checkpointed import is for a different root (%q), not %q; rerun with --no-resume to start fresh", prior.RootDir, absRoot, ) } diff --git a/internal/fbmessenger/importer_test.go b/internal/fbmessenger/importer_test.go index ef93f864a..68819629a 100644 --- a/internal/fbmessenger/importer_test.go +++ b/internal/fbmessenger/importer_test.go @@ -1100,7 +1100,7 @@ func writeMultiThreadFixture(t *testing.T, n int) string { return tmp } -// TestImportDYI_ResumeFromCheckpoint seeds an active sync with a prior +// TestImportDYI_ResumeFromCheckpoint seeds a failed sync with a prior // fbmessengerCheckpoint pointing past the first thread, then runs // ImportDYI and verifies that (a) WasResumed is true and (b) the // already-processed thread is skipped on the second run (while still @@ -1120,9 +1120,8 @@ func TestImportDYI_ResumeFromCheckpoint(t *testing.T) { require.Equal(int64(3), first.MessagesAdded, "first run MessagesAdded") require.Equal(3, first.ThreadsProcessed, "first run ThreadsProcessed") - // Simulate an in-progress run: create a new running sync_run for - // the facebook_messenger source and write a fbmessengerCheckpoint - // whose ThreadIndex == 2 (two threads already done). + // Simulate an interrupted run with a checkpoint whose ThreadIndex == 2 + // (two threads already done). src, err := st.GetOrCreateSource("facebook_messenger", "test.user@facebook.messenger") require.NoError(err) syncID, err := st.StartSync(src.ID, "import-messenger") @@ -1140,6 +1139,7 @@ func TestImportDYI_ResumeFromCheckpoint(t *testing.T) { MessagesProcessed: 2, MessagesAdded: 2, })) + require.NoError(st.FailSync(syncID, "worker stopped")) // Second run: should detect the active checkpoint and resume, // processing only the 3rd thread. diff --git a/internal/importer/emlx_import.go b/internal/importer/emlx_import.go index d0e585ea0..a6cb0a9a3 100644 --- a/internal/importer/emlx_import.go +++ b/internal/importer/emlx_import.go @@ -93,7 +93,7 @@ const defaultMaxEmlxBytes int64 = 128 << 20 // 128 MiB func ImportEmlxDir( ctx context.Context, st *store.Store, rootDir string, opts EmlxImportOptions, -) (*EmlxImportSummary, error) { +) (retSummary *EmlxImportSummary, retErr error) { if opts.SourceType == "" { opts.SourceType = "apple-mail" } @@ -139,6 +139,14 @@ func ImportEmlxDir( return nil, fmt.Errorf("get/create source: %w", err) } summary.SourceID = src.ID + ownershipCtx := context.WithoutCancel(ctx) + execution, err := st.AcquireSyncExecutionContext(ownershipCtx, src.ID) + if err != nil { + return nil, fmt.Errorf("acquire sync execution: %w", err) + } + defer func() { + retErr = errors.Join(retErr, execution.Release()) + }() // Resume support. var ( @@ -149,9 +157,9 @@ func ImportEmlxDir( ) if !opts.NoResume { - active, err := st.GetActiveSync(src.ID) + active, err := st.GetLatestCheckpointedSyncByType(src.ID, "import-emlx") if err != nil && !errors.Is(err, store.ErrSyncRunNotFound) { - return nil, fmt.Errorf("check active sync: %w", err) + return nil, fmt.Errorf("check resumable sync: %w", err) } if active != nil { if active.CursorBefore.Valid && @@ -182,7 +190,6 @@ func ImportEmlxDir( mailboxes[ecp.MailboxIndex].Path, ) } - syncID = active.ID cp.MessagesProcessed = active.MessagesProcessed cp.MessagesAdded = active.MessagesAdded cp.MessagesUpdated = active.MessagesUpdated @@ -217,11 +224,9 @@ func ImportEmlxDir( } } - if syncID == 0 { - syncID, err = st.StartSync(src.ID, "import-emlx") - if err != nil { - return nil, fmt.Errorf("start sync: %w", err) - } + syncID, err = execution.StartSyncContext(ownershipCtx, "import-emlx", "") + if err != nil { + return nil, fmt.Errorf("start sync: %w", err) } st = st.ScopedToSync(src.ID, syncID) @@ -550,7 +555,7 @@ func ImportEmlxDir( // If cancelled, leave the sync run as "running" so resume works. if ctx.Err() != nil { - return summary, nil //nolint:nilerr // cancellation is signalled via summary, not error + return summary, nil // Cancellation is signalled via summary, not error. } if hardErrors { diff --git a/internal/importer/emlx_import_test.go b/internal/importer/emlx_import_test.go index ff22717ab..c957431eb 100644 --- a/internal/importer/emlx_import_test.go +++ b/internal/importer/emlx_import_test.go @@ -507,6 +507,7 @@ func TestImportEmlxDir_MailboxPathMismatchRejectsResume(t *testing.T) { "/old/path/to/OtherMailbox.mbox", "", &store.Checkpoint{}, ), "save checkpoint") + require.NoError(st.FailSync(syncID, "worker stopped"), "fail prior sync") _, err = ImportEmlxDir( context.Background(), st, root, EmlxImportOptions{ @@ -551,6 +552,7 @@ func TestImportEmlxDir_NegativeIndexRejectsResume(t *testing.T) { require.NoError(st.UpdateSyncCheckpoint(syncID, &store.Checkpoint{ PageToken: string(cpJSON), }), "save checkpoint") + require.NoError(st.FailSync(syncID, "worker stopped"), "fail prior sync") _, err = ImportEmlxDir( context.Background(), st, root, EmlxImportOptions{ @@ -583,6 +585,7 @@ func TestImportEmlxDir_RootMismatchRejectsResume(t *testing.T) { require.NoError(saveEmlxCheckpoint( st, syncID, absRootA, 0, "", "", &store.Checkpoint{}, ), "save checkpoint") + require.NoError(st.FailSync(syncID, "worker stopped"), "fail prior sync") // Create a mailbox at root B. rootB := filepath.Join(tmp, "MailB") diff --git a/internal/importer/mbox_import.go b/internal/importer/mbox_import.go index 97c6db37a..13136d732 100644 --- a/internal/importer/mbox_import.go +++ b/internal/importer/mbox_import.go @@ -85,7 +85,9 @@ const sourceTypeMbox = "mbox" // This is intended for services like HEY.com that provide an export in MBOX // format but do not expose IMAP/POP. The importer stores the raw MIME, // parsed bodies, participants, recipients, and (optionally) attachments. -func ImportMbox(ctx context.Context, st *store.Store, mboxPath string, opts MboxImportOptions) (*MboxImportSummary, error) { +func ImportMbox( + ctx context.Context, st *store.Store, mboxPath string, opts MboxImportOptions, +) (retSummary *MboxImportSummary, retErr error) { if opts.SourceType == "" { opts.SourceType = sourceTypeMbox } @@ -124,8 +126,17 @@ func ImportMbox(ctx context.Context, st *store.Store, mboxPath string, opts Mbox return nil, fmt.Errorf("get/create source: %w", err) } summary.SourceID = src.ID + ownershipCtx := context.WithoutCancel(ctx) + execution, err := st.AcquireSyncExecutionContext(ownershipCtx, src.ID) + if err != nil { + return nil, fmt.Errorf("acquire sync execution: %w", err) + } + defer func() { + retErr = errors.Join(retErr, execution.Release()) + }() - // Create or resume the sync run for this source. + // Resume from a recovered checkpoint, then create a new run under the + // source ownership held for this import. var ( syncID int64 cp store.Checkpoint @@ -134,12 +145,11 @@ func ImportMbox(ctx context.Context, st *store.Store, mboxPath string, opts Mbox ) if !opts.NoResume { - active, err := st.GetActiveSync(src.ID) + active, err := st.GetLatestCheckpointedSyncByType(src.ID, "import-mbox") if err != nil && !errors.Is(err, store.ErrSyncRunNotFound) { - return nil, fmt.Errorf("check active sync: %w", err) + return nil, fmt.Errorf("check resumable sync: %w", err) } if active != nil { - syncID = active.ID cp.MessagesProcessed = active.MessagesProcessed cp.MessagesAdded = active.MessagesAdded cp.MessagesUpdated = active.MessagesUpdated @@ -172,11 +182,9 @@ func ImportMbox(ctx context.Context, st *store.Store, mboxPath string, opts Mbox } } - if syncID == 0 { - syncID, err = st.StartSync(src.ID, "import-mbox") - if err != nil { - return nil, fmt.Errorf("start sync: %w", err) - } + syncID, err = execution.StartSyncContext(ownershipCtx, "import-mbox", "") + if err != nil { + return nil, fmt.Errorf("start sync: %w", err) } st = st.ScopedToSync(src.ID, syncID) diff --git a/internal/importer/mbox_import_test.go b/internal/importer/mbox_import_test.go index 33ab591a3..6e2c62492 100644 --- a/internal/importer/mbox_import_test.go +++ b/internal/importer/mbox_import_test.go @@ -728,6 +728,7 @@ func TestImportMbox_InvalidResumeOffsetBeyondEOF_FailsSync(t *testing.T) { cp := store.Checkpoint{} require.NoError(saveMboxCheckpoint(st, syncID, absPath, fi.Size()+1, 0, &cp), "save checkpoint") + require.NoError(st.FailSync(syncID, "worker stopped"), "fail prior sync") _, err = ImportMbox(context.Background(), st, absPath, MboxImportOptions{ SourceType: "mbox", @@ -746,6 +747,64 @@ func TestImportMbox_InvalidResumeOffsetBeyondEOF_FailsSync(t *testing.T) { require.Equal(0, messageCount, "messageCount") } +func TestImportMbox_RejectsLiveSyncOwner(t *testing.T) { + require := require.New(t) + tmp := t.TempDir() + dbPath := filepath.Join(tmp, "msgvault.db") + st, err := store.Open(dbPath) + require.NoError(err) + t.Cleanup(func() { _ = st.Close() }) + require.NoError(st.InitSchema()) + + mboxPath := filepath.Join(tmp, "export.mbox") + require.NoError(os.WriteFile(mboxPath, []byte( + "From sender@example.com Mon Jan 1 00:00:00 2024\nSubject: One\n\nBody\n", + ), 0o600)) + src, err := st.GetOrCreateSource("mbox", "user@example.com") + require.NoError(err) + _, err = st.StartSync(src.ID, "import-mbox") + require.NoError(err) + + _, err = ImportMbox(t.Context(), st, mboxPath, MboxImportOptions{ + Identifier: "user@example.com", + }) + + require.ErrorIs(err, store.ErrSyncAlreadyActive) +} + +func TestImportMbox_ResumesCheckpointAfterWorkerExit(t *testing.T) { + assert := assert.New(t) + require := require.New(t) + tmp := t.TempDir() + dbPath := filepath.Join(tmp, "msgvault.db") + mboxPath := filepath.Join(tmp, "export.mbox") + mboxData := []byte("From sender@example.com Mon Jan 1 00:00:00 2024\nSubject: One\n\nBody\n") + require.NoError(os.WriteFile(mboxPath, mboxData, 0o600)) + + first, err := store.Open(dbPath) + require.NoError(err) + require.NoError(first.InitSchema()) + src, err := first.GetOrCreateSource("mbox", "user@example.com") + require.NoError(err) + runID, err := first.StartSync(src.ID, "import-mbox") + require.NoError(err) + require.NoError(saveMboxCheckpoint( + first, runID, mboxPath, int64(len(mboxData)), 1, + &store.Checkpoint{MessagesProcessed: 1, MessagesAdded: 1}, + )) + require.NoError(first.Close()) + + second, err := store.Open(dbPath) + require.NoError(err) + t.Cleanup(func() { _ = second.Close() }) + summary, err := ImportMbox(t.Context(), second, mboxPath, MboxImportOptions{ + Identifier: "user@example.com", + }) + require.NoError(err) + assert.True(summary.WasResumed) + assert.Equal(int64(len(mboxData)), summary.ResumedOffset) +} + func TestImportMbox_HardErrorsStopsMultiFileLoop(t *testing.T) { require := require.New(t) // Verify that when ImportMbox reports HardErrors, the caller's diff --git a/internal/importer/pst_import.go b/internal/importer/pst_import.go index 456db696c..08931a389 100644 --- a/internal/importer/pst_import.go +++ b/internal/importer/pst_import.go @@ -97,7 +97,9 @@ const defaultMaxPstMessageBytes int64 = 128 << 20 // 128 MiB // Folder structure is preserved as labels. Non-email items (calendar, contacts, // tasks) are skipped automatically. The import is resumable: if interrupted, // rerunning with the same arguments continues from where it left off. -func ImportPst(ctx context.Context, st *store.Store, pstPath string, opts PstImportOptions) (*PstImportSummary, error) { +func ImportPst( + ctx context.Context, st *store.Store, pstPath string, opts PstImportOptions, +) (retSummary *PstImportSummary, retErr error) { if opts.SourceType == "" { opts.SourceType = "pst" } @@ -155,6 +157,14 @@ func ImportPst(ctx context.Context, st *store.Store, pstPath string, opts PstImp return nil, fmt.Errorf("get/create source: %w", err) } summary.SourceID = src.ID + ownershipCtx := context.WithoutCancel(ctx) + execution, err := st.AcquireSyncExecutionContext(ownershipCtx, src.ID) + if err != nil { + return nil, fmt.Errorf("acquire sync execution: %w", err) + } + defer func() { + retErr = errors.Join(retErr, execution.Release()) + }() // Set display name to the PST filename so it appears in list-accounts / get_stats. pstBase := filepath.Base(absPath) @@ -175,12 +185,11 @@ func ImportPst(ctx context.Context, st *store.Store, pstPath string, opts PstImp ) if !opts.NoResume { - active, err := st.GetActiveSync(src.ID) + active, err := st.GetLatestCheckpointedSyncByType(src.ID, "import-pst") if err != nil && !errors.Is(err, store.ErrSyncRunNotFound) { - return nil, fmt.Errorf("check active sync: %w", err) + return nil, fmt.Errorf("check resumable sync: %w", err) } if active != nil { - syncID = active.ID cp.MessagesProcessed = active.MessagesProcessed cp.MessagesAdded = active.MessagesAdded cp.MessagesUpdated = active.MessagesUpdated @@ -231,11 +240,9 @@ func ImportPst(ctx context.Context, st *store.Store, pstPath string, opts PstImp } } - if syncID == 0 { - syncID, err = st.StartSync(src.ID, "import-pst") - if err != nil { - return nil, fmt.Errorf("start sync: %w", err) - } + syncID, err = execution.StartSyncContext(ownershipCtx, "import-pst", "") + if err != nil { + return nil, fmt.Errorf("start sync: %w", err) } st = st.ScopedToSync(src.ID, syncID) diff --git a/internal/meetingimport/importer_test.go b/internal/meetingimport/importer_test.go index 7ec08c9a4..486bceb58 100644 --- a/internal/meetingimport/importer_test.go +++ b/internal/meetingimport/importer_test.go @@ -138,7 +138,7 @@ func TestImporterCreatesCanonicalMeetingAndSyncRun(t *testing.T) { assert.Equal(int64(0), latest.MessagesUpdated) } -func TestImporterPostSupersessionWriteFailsGenerationFence(t *testing.T) { +func TestImporterRejectsConcurrentStartWhileWriterIsBlocked(t *testing.T) { checks := assert.New(t) requirements := require.New(t) st := testutil.NewTestStore(t) @@ -190,27 +190,23 @@ func TestImporterPostSupersessionWriteFailsGenerationFence(t *testing.T) { waitForBlockedMeetingImportPID(t, st, holderPID, "SELECT source_message_id", "meeting importer did not pause in its post-start message lookup") - var oldRunID int64 - requirements.NoError(st.DB().QueryRowContext(t.Context(), st.Rebind(` - SELECT id FROM sync_runs - WHERE source_id = ? AND status = 'running' - `), baseline.SourceID).Scan(&oldRunID)) - newRunID, err := st.StartSyncContext(t.Context(), baseline.SourceID, "superseding-test-run") - requirements.NoError(err) - requirements.NotEqual(oldRunID, newRunID) + _, err = st.StartSyncContext(t.Context(), baseline.SourceID, "concurrent-test-run") + requirements.ErrorIs(err, store.ErrSyncAlreadyActive) requirements.NoError(tx.Commit()) locked = false got := <-importDone - requirements.ErrorIs(got.err, store.ErrSyncRunSuperseded) - checks.Zero(got.result.MessageID) + requirements.NoError(got.err) + checks.NotZero(got.result.MessageID) var messageCount int requirements.NoError(st.DB().QueryRowContext(t.Context(), st.Rebind(` SELECT COUNT(*) FROM messages WHERE source_id = ? AND source_message_id = ? `), baseline.SourceID, "meeting:42").Scan(&messageCount)) - checks.Zero(messageCount, - "a meeting writer must not commit after its sync generation is superseded") + checks.Equal(1, messageCount) + + newRunID, err := st.StartSyncContext(t.Context(), baseline.SourceID, "post-import-test-run") + requirements.NoError(err) requirements.NoError(st.FailSync(newRunID, "test cleanup")) } diff --git a/internal/slack/importer_test.go b/internal/slack/importer_test.go index 6ce203185..530cb3e46 100644 --- a/internal/slack/importer_test.go +++ b/internal/slack/importer_test.go @@ -27,8 +27,7 @@ func TestImporterScopesParticipantResolver(t *testing.T) { runID, err := st.StartSync(source.ID, sourceTypeSlack) requirements.NoError(err) scoped := NewImporter(st, nil, "team-a").scopedToSync(source.ID, runID) - _, err = st.StartSync(source.ID, sourceTypeSlack) - requirements.NoError(err) + requirements.NoError(st.FailSync(runID, "worker stopped")) _, err = scoped.res.resolveID("user-a") requirements.ErrorIs(err, store.ErrSyncRunSuperseded) diff --git a/internal/store/dialect_pg.go b/internal/store/dialect_pg.go index ed7c76e66..c42c9b1d4 100644 --- a/internal/store/dialect_pg.go +++ b/internal/store/dialect_pg.go @@ -627,6 +627,8 @@ func (d *PostgreSQLDialect) LegacyColumnMigrations() []ColumnMigration { {`ALTER TABLE carddav_conflicts ADD COLUMN IF NOT EXISTS pending_started_at TIMESTAMPTZ`, "carddav_conflicts.pending_started_at"}, {`ALTER TABLE sources ADD COLUMN IF NOT EXISTS sync_config JSONB`, "sync_config"}, {`ALTER TABLE sync_runs ADD COLUMN IF NOT EXISTS sync_type TEXT NOT NULL DEFAULT ''`, "sync_runs.sync_type"}, + {`ALTER TABLE sync_runs ADD COLUMN IF NOT EXISTS request_fingerprint TEXT`, "sync_runs.request_fingerprint"}, + {`ALTER TABLE sync_runs ADD COLUMN IF NOT EXISTS operation_id TEXT`, "sync_runs.operation_id"}, {`ALTER TABLE imap_folder_state ADD COLUMN IF NOT EXISTS highest_modseq NUMERIC(20, 0) NOT NULL DEFAULT 0`, "imap_folder_state.highest_modseq"}, {`ALTER TABLE messages ADD COLUMN IF NOT EXISTS rfc822_message_id TEXT`, "rfc822_message_id"}, {`ALTER TABLE sources ADD COLUMN IF NOT EXISTS oauth_app TEXT`, "oauth_app"}, diff --git a/internal/store/dialect_sqlite.go b/internal/store/dialect_sqlite.go index 335d09ea1..c69d2db71 100644 --- a/internal/store/dialect_sqlite.go +++ b/internal/store/dialect_sqlite.go @@ -1928,6 +1928,8 @@ func (d *SQLiteDialect) LegacyColumnMigrations() []ColumnMigration { {`ALTER TABLE carddav_conflicts ADD COLUMN pending_started_at DATETIME`, "carddav_conflicts.pending_started_at"}, {`ALTER TABLE sources ADD COLUMN sync_config JSON`, "sync_config"}, {`ALTER TABLE sync_runs ADD COLUMN sync_type TEXT NOT NULL DEFAULT ''`, "sync_runs.sync_type"}, + {`ALTER TABLE sync_runs ADD COLUMN request_fingerprint TEXT`, "sync_runs.request_fingerprint"}, + {`ALTER TABLE sync_runs ADD COLUMN operation_id TEXT`, "sync_runs.operation_id"}, {`ALTER TABLE imap_folder_state ADD COLUMN highest_modseq TEXT NOT NULL DEFAULT '0'`, "imap_folder_state.highest_modseq"}, {`ALTER TABLE messages ADD COLUMN rfc822_message_id TEXT`, "rfc822_message_id"}, {`ALTER TABLE sources ADD COLUMN oauth_app TEXT`, "oauth_app"}, diff --git a/internal/store/export_test.go b/internal/store/export_test.go index cf5226d0e..3237364d9 100644 --- a/internal/store/export_test.go +++ b/internal/store/export_test.go @@ -12,6 +12,12 @@ import ( // ParseDBTime is exported for testing unexported timestamp parsing behavior. var ParseDBTime = parseDBTime +// DBPathForTest returns the backend address used by a Store so an integration +// test can open a second independent handle to the same isolated database. +func DBPathForTest(s *Store) string { + return s.dbPath +} + // MessagesTableColumns returns the live column names of the messages table on // whichever backend the store uses. Test-only: it exists so // TestMessagesColumnClassificationIsExhaustive can compare the real table diff --git a/internal/store/messages_test.go b/internal/store/messages_test.go index 70860a9e9..e77225d1e 100644 --- a/internal/store/messages_test.go +++ b/internal/store/messages_test.go @@ -512,10 +512,12 @@ func TestReconcileSourceMessageSnapshotIsSourceScopedAndGenerationFenced(t *test assertMessageDeletedFromSource(t, st, source.ID, "missing", true) assertMessageDeletedFromSource(t, st, source.ID, "present", false) assertMessageDeletedFromSource(t, st, otherSource.ID, "missing", false) + require.NoError(st.CompleteSync(syncID, "initial")) staleSyncID, err := st.StartSync(source.ID, "full") require.NoError(err) stale := st.ScopedToSync(source.ID, staleSyncID) + require.NoError(st.FailSync(staleSyncID, "worker stopped")) _, err = st.StartSync(source.ID, "full") require.NoError(err) _, err = stale.ReconcileSourceMessageSnapshot(t.Context(), source.ID, map[string]struct{}{}) diff --git a/internal/store/person_sweep_work_pg_test.go b/internal/store/person_sweep_work_pg_test.go index 736305c55..3ecd3978c 100644 --- a/internal/store/person_sweep_work_pg_test.go +++ b/internal/store/person_sweep_work_pg_test.go @@ -139,73 +139,45 @@ func TestPersonSweepPostgreSQLUntrackingWinsPausedPublication(t *testing.T) { checks.Nil(lease) } -func TestPersonSweepPostgreSQLSupersededWriterFailsGenerationFence(t *testing.T) { +func TestPersonSweepPostgreSQLConcurrentStartDoesNotSupersedeWriter(t *testing.T) { checks := assert.New(t) requirements := require.New(t) f := newPersonSweepJournalFixture(t, true, false) if !f.store.IsPostgreSQL() { - t.Skip("PostgreSQL-only sync-generation race regression") + t.Skip("PostgreSQL-only concurrent sync regression") } - oldRunID, err := f.store.StartSync(f.sourceID, "incremental") + runID, err := f.store.StartSync(f.sourceID, "incremental") requirements.NoError(err) - oldWriter := f.store.ScopedToSync(f.sourceID, oldRunID) - partialID, err := oldWriter.UpsertMessage(&store.Message{ - SourceID: f.sourceID, SourceMessageID: "old-run-committed-partial", + writer := f.store.ScopedToSync(f.sourceID, runID) + partialID, err := writer.UpsertMessage(&store.Message{ + SourceID: f.sourceID, SourceMessageID: "active-run-committed-partial", ConversationID: f.conversationID, MessageType: "email", SenderID: sql.NullInt64{Int64: f.aliceID, Valid: true}, }) requirements.NoError(err) - partialSequence := latestPersonSweepSequence(t, f.store) deletePersonSweepWork(t, f.store, f.alicePersonID) - staleReady := make(chan struct{}) - releaseStale := make(chan struct{}) - staleDone := make(chan error, 1) - go func() { - close(staleReady) - <-releaseStale - _, staleErr := oldWriter.UpsertMessage(&store.Message{ - SourceID: f.sourceID, SourceMessageID: "old-run-post-supersession", - ConversationID: f.conversationID, MessageType: "email", - SenderID: sql.NullInt64{Int64: f.aliceID, Valid: true}, - }) - staleDone <- staleErr - }() - <-staleReady - - newRunID, err := f.store.StartSync(f.sourceID, "incremental") - requirements.NoError(err) - newWriter := f.store.ScopedToSync(f.sourceID, newRunID) - err = oldWriter.UpsertMessageBody(partialID, - sql.NullString{String: "stale old-run body", Valid: true}, sql.NullString{}) - requirements.ErrorIs(err, store.ErrSyncRunSuperseded) - newID, err := newWriter.UpsertMessage(&store.Message{ - SourceID: f.sourceID, SourceMessageID: "new-run-legitimate", + _, err = f.store.StartSync(f.sourceID, "incremental") + requirements.ErrorIs(err, store.ErrSyncAlreadyActive) + requirements.NoError(writer.UpsertMessageBody(partialID, + sql.NullString{String: "active run body", Valid: true}, sql.NullString{})) + secondID, err := writer.UpsertMessage(&store.Message{ + SourceID: f.sourceID, SourceMessageID: "active-run-continued", ConversationID: f.conversationID, MessageType: "email", SenderID: sql.NullInt64{Int64: f.aliceID, Valid: true}, }) requirements.NoError(err) - newSequence := latestPersonSweepSequence(t, f.store) - close(releaseStale) - requirements.ErrorIs(<-staleDone, store.ErrSyncRunSuperseded) - requirements.NoError(newWriter.CompleteSync(newRunID, "new-generation")) + secondSequence := latestPersonSweepSequence(t, f.store) + requirements.NoError(writer.CompleteSync(runID, "completed-generation")) checks.True(messageExistsByID(t, f.store, partialID)) - checks.True(messageExistsByID(t, f.store, newID)) - checks.False(messageExistsBySourceID(t, f.store, - f.sourceID, "old-run-post-supersession")) - changes := personSweepChangesAfter(t, f.store, f.alicePersonID, 0) - requirements.Len(changes, 2) - checks.Equal(partialSequence, changes[0].Sequence, - "the already-committed superseded-run mutation remains journal debt") - checks.Equal(newSequence, changes[1].Sequence) + checks.True(messageExistsByID(t, f.store, secondID)) rows, dirtyThrough := personSweepWorkState(t, f.store, f.alicePersonID) checks.Equal(1, rows) - checks.Equal(newSequence, dirtyThrough) - lower, upper := personSweepSyncPublicationBounds(t, f.store, newRunID) + checks.Equal(secondSequence, dirtyThrough) + _, upper := personSweepSyncPublicationBounds(t, f.store, runID) requirements.True(upper.Valid) - checks.Equal(partialSequence, lower) - checks.Equal(newSequence, upper.Int64) + checks.Equal(secondSequence, upper.Int64) } func TestPersonSweepPostgreSQLClaimDoesNotReversePublicationOptOutLocks(t *testing.T) { @@ -294,15 +266,3 @@ func messageExistsByID(t *testing.T, st *store.Store, messageID int64) bool { SELECT EXISTS (SELECT 1 FROM messages WHERE id = ?)`), messageID).Scan(&exists)) return exists } - -func messageExistsBySourceID( - t *testing.T, st *store.Store, sourceID int64, sourceMessageID string, -) bool { - t.Helper() - var exists bool - require.NoError(t, st.DB().QueryRowContext(t.Context(), st.Rebind(` - SELECT EXISTS ( - SELECT 1 FROM messages WHERE source_id = ? AND source_message_id = ? - )`), sourceID, sourceMessageID).Scan(&exists)) - return exists -} diff --git a/internal/store/schema.sql b/internal/store/schema.sql index 7ac3aad05..82808da24 100644 --- a/internal/store/schema.sql +++ b/internal/store/schema.sql @@ -1523,6 +1523,17 @@ CREATE TABLE IF NOT EXISTS visual_work_claims ( -- SYNC STATE -- ============================================================================ +-- Durable lifecycle for higher-level sync invocations. One operation can own +-- multiple sync runs, such as full enumeration followed by history catch-up. +CREATE TABLE IF NOT EXISTS sync_operations ( + id TEXT PRIMARY KEY, + source_id INTEGER NOT NULL REFERENCES sources(id) ON DELETE CASCADE, + status TEXT NOT NULL CHECK (status IN ('pending', 'running', 'done', 'failed')), + created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, + started_at DATETIME, + finished_at DATETIME +); + -- Sync runs (for debugging and resumability) CREATE TABLE IF NOT EXISTS sync_runs ( id INTEGER PRIMARY KEY, @@ -1540,7 +1551,9 @@ CREATE TABLE IF NOT EXISTS sync_runs ( error_message TEXT, cursor_before TEXT, - cursor_after TEXT + cursor_after TEXT, + request_fingerprint TEXT, + operation_id TEXT ); -- Exact journal cut owned by one source sync publication. The lower bound is diff --git a/internal/store/schema_pg.sql b/internal/store/schema_pg.sql index ff4fa515f..b8b2478a3 100644 --- a/internal/store/schema_pg.sql +++ b/internal/store/schema_pg.sql @@ -1372,6 +1372,15 @@ CREATE TABLE IF NOT EXISTS visual_work_claims ( -- SYNC STATE -- ============================================================================ +CREATE TABLE IF NOT EXISTS sync_operations ( + id TEXT PRIMARY KEY, + source_id BIGINT NOT NULL REFERENCES sources(id) ON DELETE CASCADE, + status TEXT NOT NULL CHECK (status IN ('pending', 'running', 'done', 'failed')), + created_at TIMESTAMPTZ NOT NULL DEFAULT CURRENT_TIMESTAMP, + started_at TIMESTAMPTZ, + finished_at TIMESTAMPTZ +); + CREATE TABLE IF NOT EXISTS sync_runs ( id BIGINT GENERATED ALWAYS AS IDENTITY PRIMARY KEY, source_id BIGINT NOT NULL REFERENCES sources(id) ON DELETE CASCADE, @@ -1388,7 +1397,9 @@ CREATE TABLE IF NOT EXISTS sync_runs ( error_message TEXT, cursor_before TEXT, - cursor_after TEXT + cursor_after TEXT, + request_fingerprint TEXT, + operation_id TEXT ); CREATE TABLE IF NOT EXISTS person_sweep_sync_publications ( diff --git a/internal/store/store.go b/internal/store/store.go index 77e73cca6..30c7efc6b 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -58,6 +58,9 @@ type Store struct { // share mutable run state. syncGeneration *syncGeneration syncBase *Store + // syncExecutionLocks is shared with sync-scoped views. Each held lock is + // owned by the worker process, not by a durable sync_runs row. + syncExecutionLocks *syncExecutionLockState sqliteOptimizeMu sync.Mutex documentVectorOperationMu sync.Mutex @@ -203,9 +206,10 @@ func openSQLite(dbPath, params string) (*Store, error) { } s := &Store{ - db: newLoggedDB(db, dialect.Rebind), - dbPath: dbPath, - dialect: dialect, + db: newLoggedDB(db, dialect.Rebind), + dbPath: dbPath, + dialect: dialect, + syncExecutionLocks: newSyncExecutionLockState(), } // Probe like the read-only opens do: a Store must know whether full-text @@ -262,10 +266,11 @@ func openPostgres(dbURL string) (*Store, error) { } s := &Store{ - db: newLoggedDB(db, dialect.Rebind), - dbPath: dbURL, - dialect: dialect, - closeCleanup: cleanup, + db: newLoggedDB(db, dialect.Rebind), + dbPath: dbURL, + dialect: dialect, + closeCleanup: cleanup, + syncExecutionLocks: newSyncExecutionLockState(), } // See openSQLite: availability is a property of the database, not of @@ -325,10 +330,11 @@ func OpenReadOnly(dbPath string) (*Store, error) { } s := &Store{ - db: newLoggedDB(db, dialect.Rebind), - dbPath: dbPath, - dialect: dialect, - readOnly: true, + db: newLoggedDB(db, dialect.Rebind), + dbPath: dbPath, + dialect: dialect, + readOnly: true, + syncExecutionLocks: newSyncExecutionLockState(), } // OpenReadOnly takes no context, so the probe cannot be cancelled and its @@ -376,11 +382,12 @@ func openPostgresReadOnly(dbURL string) (*Store, error) { } s := &Store{ - db: newLoggedDB(db, dialect.Rebind), - dbPath: dbURL, - dialect: dialect, - readOnly: true, - closeCleanup: cleanup, + db: newLoggedDB(db, dialect.Rebind), + dbPath: dbURL, + dialect: dialect, + readOnly: true, + closeCleanup: cleanup, + syncExecutionLocks: newSyncExecutionLockState(), } // As in OpenReadOnly: no context to honour here, but the error is checked @@ -472,12 +479,13 @@ func (s *Store) Close() error { // reduces the risk of corruption from stale WAL entries. _ = s.CheckpointWAL() } + lockErr := s.releaseAllSyncExecutionLocks() err := s.db.Close() if s.closeCleanup != nil { s.closeCleanup() s.closeCleanup = nil } - return err + return errors.Join(lockErr, err) } // CheckpointWAL forces a WAL checkpoint, folding the WAL back into the main @@ -1293,6 +1301,13 @@ func (s *Store) InitSchemaContext(ctx context.Context) error { lastModifiedColumnAdded = true } } + if _, err := s.db.ExecContext(ctx, ` + CREATE INDEX IF NOT EXISTS idx_sync_runs_operation + ON sync_runs(operation_id, id) + WHERE operation_id IS NOT NULL + `); err != nil { + return fmt.Errorf("create sync operation index: %w", err) + } if err := s.ensureCardDAVConflictPendingInvariant(ctx); err != nil { return fmt.Errorf("migrate CardDAV conflict pending state: %w", err) } diff --git a/internal/store/sync.go b/internal/store/sync.go index d6a7169ab..04aee2814 100644 --- a/internal/store/sync.go +++ b/internal/store/sync.go @@ -26,6 +26,10 @@ const ( // absence apart from real DB errors. var ErrSyncRunNotFound = errors.New("sync run not found") +// ErrSyncAlreadyActive is returned when a source already has a running sync. +// Callers must wait for that worker to exit before starting another one. +var ErrSyncAlreadyActive = errors.New("sync already active") + // ErrSyncRunSuperseded is returned when a terminal write belongs to a sync // generation that is no longer running or current for its source. var ErrSyncRunSuperseded = errors.New("sync run superseded") @@ -49,8 +53,9 @@ func (s *Store) ScopedToSync(sourceID, syncRunID int64) *Store { readOnly: base.readOnly, fts5Available: base.fts5Available, - syncGeneration: &syncGeneration{sourceID: sourceID, runID: syncRunID}, - syncBase: base, + syncGeneration: &syncGeneration{sourceID: sourceID, runID: syncRunID}, + syncBase: base, + syncExecutionLocks: base.syncExecutionLocks, initSchemaWindowHook: base.initSchemaWindowHook, attributeSeedReadHook: base.attributeSeedReadHook, @@ -279,7 +284,7 @@ func scanSyncRun(sc scanner) (*SyncRun, error) { err := sc.Scan( &run.ID, &run.SourceID, &startedAt, &run.CompletedAt, &run.Status, &run.MessagesProcessed, &run.MessagesAdded, &run.MessagesUpdated, &run.ErrorsCount, - &run.ErrorMessage, &run.CursorBefore, &run.CursorAfter, + &run.ErrorMessage, &run.CursorBefore, &run.CursorAfter, &run.RequestFingerprint, ) if err != nil { return nil, err @@ -293,18 +298,19 @@ func scanSyncRun(sc scanner) (*SyncRun, error) { // SyncRun represents a sync operation in progress or completed. type SyncRun struct { - ID int64 - SourceID int64 - StartedAt time.Time - CompletedAt sql.NullTime - Status string // SyncStatusRunning, SyncStatusCompleted, SyncStatusFailed - MessagesProcessed int64 - MessagesAdded int64 - MessagesUpdated int64 - ErrorsCount int64 - ErrorMessage sql.NullString - CursorBefore sql.NullString // Page token for resumption - CursorAfter sql.NullString // Final history ID + ID int64 + SourceID int64 + StartedAt time.Time + CompletedAt sql.NullTime + Status string // SyncStatusRunning, SyncStatusCompleted, SyncStatusFailed + MessagesProcessed int64 + MessagesAdded int64 + MessagesUpdated int64 + ErrorsCount int64 + ErrorMessage sql.NullString + CursorBefore sql.NullString // Page token for resumption + CursorAfter sql.NullString // Final history ID + RequestFingerprint sql.NullString // Full-sync request identity for safe resumption } // Checkpoint represents sync progress for resumption. @@ -345,22 +351,46 @@ type SourceImportItem struct { ErrorMessage sql.NullString } -// StartSync creates a new sync run record and returns its ID. The -// supersede UPDATE and the INSERT run inside a writer-locked -// transaction so concurrent StartSync calls cannot both find no -// running rows, both INSERT, and leave two 'running' rows alive. -// SQLite uses BEGIN IMMEDIATE; PostgreSQL takes a row lock on the -// source via SELECT ... FOR UPDATE before doing the read-modify-write -// on sync_runs. +// StartSync creates a new sync run record and returns its ID. It first takes a +// non-blocking execution lock for the source and holds that lock until the run +// completes or fails. SQLite uses an OS file lock; PostgreSQL uses a session +// advisory lock on a dedicated connection. Both release automatically when +// the worker process exits. Once the execution lock is held, any running row +// for that source has no live owner and is failed before the new row is added. +// The stale-row transition and INSERT share a writer-locked transaction. func (s *Store) StartSync(sourceID int64, syncType string) (int64, error) { return s.StartSyncContext(context.Background(), sourceID, syncType) } // StartSyncContext is the request-aware form of StartSync. func (s *Store) StartSyncContext(ctx context.Context, sourceID int64, syncType string) (int64, error) { + return s.startSyncContext(ctx, sourceID, syncType, "") +} + +// StartSyncOperation creates a sync run attributed to an operation previously +// persisted with CreateSyncOperation. Every phase in a multi-run sync uses the +// same operation ID. +func (s *Store) StartSyncOperation(sourceID int64, operationID string) (int64, error) { + return s.startSyncContext(context.Background(), sourceID, "", operationID) +} + +func (s *Store) startSyncContext( + ctx context.Context, sourceID int64, syncType, operationID string, +) (int64, error) { + return s.startSyncContextWithLock(ctx, sourceID, syncType, operationID, "", nil) +} + +func (s *Store) startSyncContextWithLock( + ctx context.Context, + sourceID int64, + syncType, operationID, requestFingerprint string, + heldLock syncExecutionLock, +) (int64, error) { const maxAttempts = 5 for range maxAttempts { - id, err := s.startSyncOnce(ctx, sourceID, syncType) + id, err := s.startSyncOnce( + ctx, sourceID, syncType, operationID, requestFingerprint, heldLock, + ) if err == nil { return id, nil } @@ -371,7 +401,26 @@ func (s *Store) StartSyncContext(ctx context.Context, sourceID int64, syncType s return 0, fmt.Errorf("start sync: gave up after %d retries on busy", maxAttempts) } -func (s *Store) startSyncOnce(ctx context.Context, sourceID int64, syncType string) (retID int64, retErr error) { +func (s *Store) startSyncOnce( + ctx context.Context, + sourceID int64, + syncType, operationID, requestFingerprint string, + heldLock syncExecutionLock, +) (retID int64, retErr error) { + executionLock := heldLock + releaseWhenDone := executionLock == nil + if releaseWhenDone { + var err error + executionLock, err = s.acquireSyncExecutionLock(ctx, sourceID) + if err != nil { + return 0, fmt.Errorf("acquire source %d sync execution lock: %w", sourceID, err) + } + defer func() { + if retErr != nil { + retErr = errors.Join(retErr, s.abandonSyncExecutionLock(sourceID, executionLock)) + } + }() + } conn, err := s.db.Conn(ctx) if err != nil { return 0, fmt.Errorf("acquire connection: %w", err) @@ -399,27 +448,37 @@ func (s *Store) startSyncOnce(ctx context.Context, sourceID int64, syncType stri // Serialize against concurrent StartSync for the same source. // SQLite already serializes writers under BEGIN IMMEDIATE; PG // needs an explicit row lock on the source so the read snapshot - // for the UPDATE below cannot miss a concurrently committed + // for the check below cannot miss a concurrently committed // running run. - var lockedID int64 - if err := conn.QueryRowContext(ctx, - rebind(`SELECT id FROM sources WHERE id = ?`+s.dialect.SelectForUpdate()), - sourceID, - ).Scan(&lockedID); err != nil { - return 0, fmt.Errorf("lock source row: %w", err) + if releaseWhenDone { + if err := s.recoverAbandonedSyncSourceQueries(ctx, conn, sourceID, now); err != nil { + return 0, err + } + } else { + var lockedID int64 + if err := conn.QueryRowContext(ctx, + rebind(`SELECT id FROM sources WHERE id = ?`+s.dialect.SelectForUpdate()), + sourceID, + ).Scan(&lockedID); err != nil { + return 0, fmt.Errorf("lock source row: %w", err) + } } - - if _, err := conn.ExecContext(ctx, - rebind(fmt.Sprintf(` - UPDATE sync_runs - SET status = 'failed', - error_message = 'superseded by new sync', - completed_at = %s - WHERE source_id = ? AND status = 'running' - `, now)), - sourceID, - ); err != nil { - return 0, fmt.Errorf("mark old syncs failed: %w", err) + if operationID != "" { + result, err := conn.ExecContext(ctx, rebind(fmt.Sprintf(` + UPDATE sync_operations + SET status = 'running', started_at = COALESCE(started_at, %s) + WHERE id = ? AND source_id = ? AND status IN ('pending', 'running') + `, now)), operationID, sourceID) + if err != nil { + return 0, fmt.Errorf("start sync operation %q: %w", operationID, err) + } + updated, err := result.RowsAffected() + if err != nil { + return 0, fmt.Errorf("start sync operation %q: rows affected: %w", operationID, err) + } + if updated != 1 { + return 0, fmt.Errorf("start sync operation %q: %w", operationID, ErrSyncRunNotFound) + } } var syncRunID int64 @@ -431,11 +490,15 @@ func (s *Store) startSyncOnce(ctx context.Context, sourceID int64, syncType stri } if err := conn.QueryRowContext(ctx, rebind(fmt.Sprintf(` - INSERT INTO sync_runs (source_id, sync_type, started_at, status, messages_processed, messages_added, messages_updated, errors_count) - VALUES (?, ?, %s, 'running', 0, 0, 0, 0) - RETURNING id - `, now)), - sourceID, syncType, + INSERT INTO sync_runs ( + source_id, sync_type, started_at, status, messages_processed, + messages_added, messages_updated, errors_count, + request_fingerprint, operation_id + ) + VALUES (?, ?, %s, 'running', 0, 0, 0, 0, NULLIF(?, ''), NULLIF(?, '')) + RETURNING id + `, now)), + sourceID, syncType, requestFingerprint, operationID, ).Scan(&syncRunID); err != nil { return 0, fmt.Errorf("insert sync_run: %w", err) } @@ -450,9 +513,202 @@ func (s *Store) startSyncOnce(ctx context.Context, sourceID int64, syncType stri return 0, fmt.Errorf("commit: %w", err) } committed = true + s.registerSyncExecutionLock(sourceID, syncRunID, executionLock, releaseWhenDone) return syncRunID, nil } +func (s *Store) recoverAbandonedSyncSource(ctx context.Context, sourceID int64) error { + return s.withoutSyncScope().withTxContext(ctx, func(tx *loggedTx) error { + return s.recoverAbandonedSyncSourceQueries(ctx, tx, sourceID, s.dialect.Now()) + }) +} + +func (s *Store) recoverAbandonedSyncSourceQueries( + ctx context.Context, q contextStatementQuerier, sourceID int64, now string, +) error { + var lockedID int64 + if err := q.QueryRowContext(ctx, + s.Rebind(`SELECT id FROM sources WHERE id = ?`+s.dialect.SelectForUpdate()), + sourceID, + ).Scan(&lockedID); err != nil { + return fmt.Errorf("lock source row: %w", err) + } + if _, err := q.ExecContext(ctx, s.Rebind(fmt.Sprintf(` + UPDATE sync_operations + SET status = 'failed', finished_at = %s + WHERE source_id = ? AND status = 'running'`, now)), sourceID); err != nil { + return fmt.Errorf("fail abandoned sync operation: %w", err) + } + if _, err := q.ExecContext(ctx, s.Rebind(fmt.Sprintf(` + UPDATE sync_runs + SET status = 'failed', completed_at = %s, + error_message = 'sync worker exited before recording completion' + WHERE source_id = ? AND status = 'running'`, now)), sourceID); err != nil { + return fmt.Errorf("fail abandoned sync: %w", err) + } + return nil +} + +// SyncOperation is the durable status of one higher-level sync invocation. +// A Gmail history recovery can contribute more than one SyncRun. +type SyncOperation struct { + ID string + SourceID int64 + Status string + CreatedAt time.Time + StartedAt sql.NullTime + FinishedAt sql.NullTime + Runs []*SyncRun +} + +// CreateSyncOperation persists a pending higher-level sync invocation before +// its worker starts. +func (s *Store) CreateSyncOperation(sourceID int64, operationID string) (*SyncOperation, error) { + if operationID == "" { + return nil, errors.New("create sync operation: empty operation ID") + } + var createdAt sql.NullTime + err := s.db.QueryRow(fmt.Sprintf(` + INSERT INTO sync_operations (id, source_id, status, created_at) + VALUES (?, ?, 'pending', %s) + RETURNING created_at + `, s.dialect.Now()), operationID, sourceID).Scan(&createdAt) + if err != nil { + return nil, fmt.Errorf("create sync operation %q: %w", operationID, err) + } + created, err := requireNullTime(createdAt, "created_at") + if err != nil { + return nil, fmt.Errorf("create sync operation %q: %w", operationID, err) + } + return &SyncOperation{ + ID: operationID, SourceID: sourceID, Status: "pending", CreatedAt: created, + }, nil +} + +// FailPendingSyncOperationsContext marks operations whose daemon exited before +// their worker created a sync run. Callers must hold exclusive daemon ownership +// so a live worker cannot still be preparing the operation. +func (s *Store) FailPendingSyncOperationsContext(ctx context.Context) (int64, error) { + result, err := s.db.ExecContext(ctx, fmt.Sprintf(` + UPDATE sync_operations + SET status = 'failed', finished_at = %s + WHERE status = 'pending' + `, s.dialect.Now())) + if err != nil { + return 0, fmt.Errorf("fail pending sync operations: %w", err) + } + failed, err := result.RowsAffected() + if err != nil { + return 0, fmt.Errorf("fail pending sync operations: rows affected: %w", err) + } + return failed, nil +} + +// GetSyncOperation returns every sync run attributed to operationID. +func (s *Store) GetSyncOperation(operationID string) (*SyncOperation, error) { + op, err := s.getSyncOperation(operationID) + if err != nil || op.Status != "running" || len(op.Runs) == 0 { + return op, err + } + recovered, err := s.recoverSyncSourceIfUnowned(context.Background(), op.Runs[0].SourceID) + if err != nil { + return nil, fmt.Errorf("recover sync operation %q: %w", operationID, err) + } + if !recovered { + return op, nil + } + return s.getSyncOperation(operationID) +} + +func (s *Store) getSyncOperation(operationID string) (*SyncOperation, error) { + op := &SyncOperation{ID: operationID} + var createdAt sql.NullTime + err := s.db.QueryRow(` + SELECT source_id, status, created_at, started_at, finished_at + FROM sync_operations + WHERE id = ? + `, operationID).Scan( + &op.SourceID, &op.Status, &createdAt, &op.StartedAt, &op.FinishedAt, + ) + if errors.Is(err, sql.ErrNoRows) { + return nil, fmt.Errorf("sync operation %q: %w", operationID, ErrSyncRunNotFound) + } + if err != nil { + return nil, err + } + op.CreatedAt, err = requireNullTime(createdAt, "created_at") + if err != nil { + return nil, fmt.Errorf("sync operation %q: %w", operationID, err) + } + rows, err := s.db.Query(` + SELECT id, source_id, started_at, completed_at, status, + messages_processed, messages_added, messages_updated, errors_count, + error_message, cursor_before, cursor_after + FROM sync_runs + WHERE operation_id = ? + ORDER BY id + `, operationID) + if err != nil { + return nil, err + } + defer func() { _ = rows.Close() }() + + for rows.Next() { + var run SyncRun + var startedAt sql.NullTime + if err := rows.Scan( + &run.ID, &run.SourceID, &startedAt, &run.CompletedAt, &run.Status, + &run.MessagesProcessed, &run.MessagesAdded, &run.MessagesUpdated, &run.ErrorsCount, + &run.ErrorMessage, &run.CursorBefore, &run.CursorAfter, + ); err != nil { + return nil, err + } + run.StartedAt, err = requireNullTime(startedAt, "started_at") + if err != nil { + return nil, fmt.Errorf("sync_run %d: %w", run.ID, err) + } + op.Runs = append(op.Runs, &run) + } + if err := rows.Err(); err != nil { + return nil, err + } + return op, nil +} + +func (s *Store) recoverSyncSourceIfUnowned(ctx context.Context, sourceID int64) (bool, error) { + execution, err := s.AcquireSyncExecutionContext(ctx, sourceID) + if errors.Is(err, ErrSyncAlreadyActive) { + return false, nil + } + if err != nil { + return false, err + } + return true, execution.Release() +} + +// FinishSyncOperation marks every phase of an operation with its final status. +func (s *Store) FinishSyncOperation(operationID, status string) error { + if status != "done" && status != "failed" { + return fmt.Errorf("invalid sync operation status %q", status) + } + result, err := s.db.Exec(fmt.Sprintf(` + UPDATE sync_operations + SET status = ?, finished_at = %s + WHERE id = ? + `, s.dialect.Now()), status, operationID) + if err != nil { + return err + } + updated, err := result.RowsAffected() + if err != nil { + return fmt.Errorf("finish sync operation %q: rows affected: %w", operationID, err) + } + if updated != 1 { + return fmt.Errorf("finish sync operation %q: %w", operationID, ErrSyncRunNotFound) + } + return nil +} + // UpdateSyncCheckpoint saves progress for resumption. func (s *Store) UpdateSyncCheckpoint(syncID int64, cp *Checkpoint) error { return s.UpdateSyncCheckpointContext(context.Background(), syncID, cp) @@ -623,7 +879,7 @@ func (s *Store) CompleteSyncContext(ctx context.Context, syncID int64, finalHist return err } completionStore.optimizeSQLiteBestEffort(ctx, "successful sync") - return nil + return completionStore.releaseSyncExecutionLock(syncID) } // CompleteSyncAndUpdateSourceCursor atomically publishes a source cursor and @@ -642,6 +898,29 @@ func (s *Store) CompleteSyncAndUpdateSourceCursor( // run completion. func (s *Store) CompleteSyncAndUpdateSourceCursorContext( ctx context.Context, syncID int64, sourceID int64, finalHistoryID string, +) error { + return s.completeSyncAndUpdateSourceContext( + ctx, syncID, sourceID, finalHistoryID, true, + ) +} + +// CompleteSyncAndPreserveSourceCursorContext completes a bounded full sync +// without treating its partial mailbox view as an incremental-sync baseline. +// The source still records when the successful run finished. +func (s *Store) CompleteSyncAndPreserveSourceCursorContext( + ctx context.Context, syncID int64, sourceID int64, finalHistoryID string, +) error { + return s.completeSyncAndUpdateSourceContext( + ctx, syncID, sourceID, finalHistoryID, false, + ) +} + +func (s *Store) completeSyncAndUpdateSourceContext( + ctx context.Context, + syncID int64, + sourceID int64, + finalHistoryID string, + updateCursor bool, ) error { completionStore := s.withoutSyncScope() if err := completionStore.withTxContext(ctx, func(tx *loggedTx) error { @@ -652,11 +931,21 @@ func (s *Store) CompleteSyncAndUpdateSourceCursorContext( } now := s.dialect.Now() - result, err := tx.ExecContext(ctx, fmt.Sprintf(` + updateSourceSQL := fmt.Sprintf(` UPDATE sources - SET sync_cursor = ?, last_sync_at = %s, updated_at = %s + SET last_sync_at = %s, updated_at = %s WHERE id = ? - `, now, now), finalHistoryID, sourceID) + `, now, now) + args := []any{sourceID} + if updateCursor { + updateSourceSQL = fmt.Sprintf(` + UPDATE sources + SET sync_cursor = ?, last_sync_at = %s, updated_at = %s + WHERE id = ? + `, now, now) + args = []any{finalHistoryID, sourceID} + } + result, err := tx.ExecContext(ctx, updateSourceSQL, args...) if err != nil { return fmt.Errorf("complete sync %d: update source cursor: %w", syncID, err) } @@ -693,7 +982,7 @@ func (s *Store) CompleteSyncAndUpdateSourceCursorContext( return err } completionStore.optimizeSQLiteBestEffort(ctx, "successful sync") - return nil + return completionStore.releaseSyncExecutionLock(syncID) } func validateCurrentSyncGeneration( @@ -756,7 +1045,10 @@ func (s *Store) FailSync(syncID int64, errMsg string) error { error_message = ? WHERE id = ? `, s.dialect.Now()), errMsg, syncID) - return err + if err != nil { + return err + } + return s.releaseSyncExecutionLock(syncID) } // FailSyncAndClearSourceCursorContext atomically rejects one expired cursor @@ -765,7 +1057,7 @@ func (s *Store) FailSyncAndClearSourceCursorContext( ctx context.Context, syncID, sourceID int64, errMsg string, ) error { transitionStore := s.withoutSyncScope() - return transitionStore.withTxContext(ctx, func(tx *loggedTx) error { + if err := transitionStore.withTxContext(ctx, func(tx *loggedTx) error { if err := validateCurrentSyncGeneration( ctx, tx, sourceID, syncID, SyncStatusRunning, ); err != nil { @@ -805,7 +1097,10 @@ func (s *Store) FailSyncAndClearSourceCursorContext( return fmt.Errorf("fail sync %d and clear source cursor: %w", syncID, ErrSyncRunSuperseded) } return nil - }) + }); err != nil { + return err + } + return transitionStore.releaseSyncExecutionLock(syncID) } // FailSyncWithCheckpoint marks a sync failed while preserving its last @@ -830,15 +1125,33 @@ func (s *Store) FailSyncWithCheckpoint(syncID int64, errMsg string, cp *Checkpoi WHERE id = ? `, s.dialect.Now()), errMsg, cp.PageToken, cp.MessagesProcessed, cp.MessagesAdded, cp.MessagesUpdated, cp.ErrorsCount, syncID) - return err + if err != nil { + return err + } + return s.releaseSyncExecutionLock(syncID) } // GetActiveSync returns the most recent running sync for a source, if any. func (s *Store) GetActiveSync(sourceID int64) (*SyncRun, error) { + run, err := s.getActiveSync(sourceID) + if err != nil { + return nil, err + } + recovered, err := s.recoverSyncSourceIfUnowned(context.Background(), sourceID) + if err != nil { + return nil, fmt.Errorf("recover active sync for source %d: %w", sourceID, err) + } + if !recovered { + return run, nil + } + return s.getActiveSync(sourceID) +} + +func (s *Store) getActiveSync(sourceID int64) (*SyncRun, error) { row := s.db.QueryRow(` SELECT id, source_id, started_at, completed_at, status, messages_processed, messages_added, messages_updated, errors_count, - error_message, cursor_before, cursor_after + error_message, cursor_before, cursor_after, request_fingerprint FROM sync_runs WHERE source_id = ? AND status = 'running' ORDER BY started_at DESC, id DESC @@ -857,7 +1170,7 @@ func (s *Store) GetLatestSync(sourceID int64) (*SyncRun, error) { row := s.db.QueryRow(` SELECT id, source_id, started_at, completed_at, status, messages_processed, messages_added, messages_updated, errors_count, - error_message, cursor_before, cursor_after + error_message, cursor_before, cursor_after, request_fingerprint FROM sync_runs WHERE source_id = ? ORDER BY started_at DESC, id DESC @@ -879,7 +1192,7 @@ func (s *Store) GetLatestCheckpointedSync(sourceID int64) (*SyncRun, error) { row := s.db.QueryRow(` SELECT id, source_id, started_at, completed_at, status, messages_processed, messages_added, messages_updated, errors_count, - error_message, cursor_before, cursor_after + error_message, cursor_before, cursor_after, request_fingerprint FROM sync_runs sr WHERE sr.source_id = ? AND status IN ('running', 'failed') @@ -907,7 +1220,7 @@ func (s *Store) GetLatestCheckpointedSyncByType(sourceID int64, syncType string) row := s.db.QueryRow(` SELECT id, source_id, started_at, completed_at, status, messages_processed, messages_added, messages_updated, errors_count, - error_message, cursor_before, cursor_after + error_message, cursor_before, cursor_after, request_fingerprint FROM sync_runs sr WHERE sr.source_id = ? AND sr.sync_type = ? @@ -1030,7 +1343,7 @@ func (s *Store) GetLastSuccessfulSync(sourceID int64) (*SyncRun, error) { row := s.db.QueryRow(` SELECT id, source_id, started_at, completed_at, status, messages_processed, messages_added, messages_updated, errors_count, - error_message, cursor_before, cursor_after + error_message, cursor_before, cursor_after, request_fingerprint FROM sync_runs WHERE source_id = ? AND status = 'completed' ORDER BY completed_at DESC, id DESC diff --git a/internal/store/sync_context_test.go b/internal/store/sync_context_test.go index 13bff3d21..ec2ef04cb 100644 --- a/internal/store/sync_context_test.go +++ b/internal/store/sync_context_test.go @@ -68,6 +68,9 @@ func (c *canceledStartSyncConn) QueryContext( c.cancel() return nil, context.Canceled } + if strings.Contains(query, "FROM sync_runs") { + return &singleInt64Rows{read: true}, nil + } return &singleInt64Rows{value: 1}, nil } diff --git a/internal/store/sync_execution_lock.go b/internal/store/sync_execution_lock.go new file mode 100644 index 000000000..86cd80ed3 --- /dev/null +++ b/internal/store/sync_execution_lock.go @@ -0,0 +1,372 @@ +package store + +import ( + "context" + "database/sql" + "errors" + "fmt" + "path/filepath" + "strconv" + "strings" + "sync" + "time" + + "github.com/gofrs/flock" +) + +const syncExecutionLockCleanupTimeout = 5 * time.Second + +type syncExecutionLock interface { + release() error +} + +type syncRunExecutionLock struct { + lock syncExecutionLock + releaseWhenDone bool +} + +type syncExecutionLockState struct { + mu sync.Mutex + byRun map[int64]syncRunExecutionLock + bySource map[int64]syncExecutionLock +} + +func newSyncExecutionLockState() *syncExecutionLockState { + return &syncExecutionLockState{ + byRun: make(map[int64]syncRunExecutionLock), + bySource: make(map[int64]syncExecutionLock), + } +} + +// SyncExecution owns the process-level execution lock for one source. A +// caller can create more than one durable sync run while retaining the same +// ownership, as Gmail history recovery does for its full and catch-up phases. +type SyncExecution struct { + store *Store + sourceID int64 + lock syncExecutionLock + + mu sync.Mutex + released bool +} + +// AcquireSyncExecutionContext takes exclusive ownership of one source and +// recovers rows left running by a worker that no longer owns the lock. +func (s *Store) AcquireSyncExecutionContext( + ctx context.Context, sourceID int64, +) (*SyncExecution, error) { + lock, err := s.acquireSyncExecutionLock(ctx, sourceID) + if err != nil { + return nil, err + } + if err := s.recoverAbandonedSyncSource(ctx, sourceID); err != nil { + return nil, errors.Join(err, s.abandonSyncExecutionLock(sourceID, lock)) + } + return &SyncExecution{store: s.withoutSyncScope(), sourceID: sourceID, lock: lock}, nil +} + +// StartSyncContext creates a durable run under this execution's existing +// source ownership. Completing the run does not release the source lock. +func (e *SyncExecution) StartSyncContext( + ctx context.Context, syncType, operationID string, +) (int64, error) { + return e.StartSyncWithRequestContext(ctx, syncType, operationID, "") +} + +// StartSyncWithRequestContext creates a durable run and records the request +// identity that controls whether its checkpoint can be resumed. +func (e *SyncExecution) StartSyncWithRequestContext( + ctx context.Context, syncType, operationID, requestFingerprint string, +) (int64, error) { + e.mu.Lock() + defer e.mu.Unlock() + if e.released { + return 0, errors.New("sync execution already released") + } + return e.store.startSyncContextWithLock( + ctx, e.sourceID, syncType, operationID, requestFingerprint, e.lock, + ) +} + +// Release gives up source ownership after every phase has stopped. +func (e *SyncExecution) Release() error { + e.mu.Lock() + defer e.mu.Unlock() + if e.released { + return nil + } + e.released = true + return e.store.releaseOwnedSyncExecutionLock(e.sourceID, e.lock) +} + +func (s *Store) acquireSyncExecutionLock( + ctx context.Context, sourceID int64, +) (syncExecutionLock, error) { + base := s.withoutSyncScope() + state := base.syncExecutionLocks + if state == nil { + return noOpSyncExecutionLock{}, nil + } + + state.mu.Lock() + if _, exists := state.bySource[sourceID]; exists { + state.mu.Unlock() + return nil, ErrSyncAlreadyActive + } + state.bySource[sourceID] = nil + state.mu.Unlock() + + lock, err := base.acquireBackendSyncExecutionLock(ctx, sourceID) + if err != nil { + state.mu.Lock() + delete(state.bySource, sourceID) + state.mu.Unlock() + return nil, err + } + + state.mu.Lock() + state.bySource[sourceID] = lock + state.mu.Unlock() + return lock, nil +} + +func (s *Store) acquireBackendSyncExecutionLock( + ctx context.Context, sourceID int64, +) (syncExecutionLock, error) { + if s.IsPostgreSQL() { + conn, err := s.db.Conn(ctx) + if err != nil { + return nil, fmt.Errorf("acquire PostgreSQL sync lock connection: %w", err) + } + lock := &postgresSyncExecutionLock{conn: conn, sourceID: sourceID, rebind: s.Rebind} + var acquired bool + err = conn.QueryRowContext(ctx, s.Rebind(` + SELECT pg_try_advisory_lock( + hashtextextended( + current_schema() || ':msgvault-sync:' || CAST(CAST(? AS BIGINT) AS TEXT), 0 + ) + )`), sourceID).Scan(&acquired) + if err != nil { + _ = conn.Close() + return nil, fmt.Errorf("acquire PostgreSQL sync lock: %w", err) + } + if !acquired { + _ = conn.Close() + return nil, ErrSyncAlreadyActive + } + return lock, nil + } + + if s.dbPath == ":memory:" || strings.Contains(s.dbPath, ":memory:") { + return noOpSyncExecutionLock{}, nil + } + + dbPath, err := filepath.Abs(s.dbPath) + if err != nil { + return nil, fmt.Errorf("resolve sync lock database path: %w", err) + } + if resolved, resolveErr := filepath.EvalSymlinks(dbPath); resolveErr == nil { + dbPath = resolved + } + lockPath := dbPath + ".sync-" + strconv.FormatInt(sourceID, 10) + ".lock" + + sqliteSyncLockRegistry.mu.Lock() + if _, exists := sqliteSyncLockRegistry.paths[lockPath]; exists { + sqliteSyncLockRegistry.mu.Unlock() + return nil, ErrSyncAlreadyActive + } + sqliteSyncLockRegistry.paths[lockPath] = struct{}{} + sqliteSyncLockRegistry.mu.Unlock() + + fileLock := flock.New(lockPath) + acquired, err := fileLock.TryLock() + if err != nil || !acquired { + sqliteSyncLockRegistry.mu.Lock() + delete(sqliteSyncLockRegistry.paths, lockPath) + sqliteSyncLockRegistry.mu.Unlock() + if err != nil { + return nil, fmt.Errorf("acquire SQLite sync lock: %w", err) + } + return nil, ErrSyncAlreadyActive + } + return &sqliteSyncExecutionLock{lock: fileLock, path: lockPath}, nil +} + +func (s *Store) registerSyncExecutionLock( + sourceID, runID int64, lock syncExecutionLock, releaseWhenDone bool, +) { + if lock == nil { + return + } + state := s.withoutSyncScope().syncExecutionLocks + if state == nil { + return + } + state.mu.Lock() + state.byRun[runID] = syncRunExecutionLock{ + lock: lock, + releaseWhenDone: releaseWhenDone, + } + if releaseWhenDone { + state.bySource[sourceID] = lock + } + state.mu.Unlock() +} + +func (s *Store) abandonSyncExecutionLock(sourceID int64, lock syncExecutionLock) error { + if lock == nil { + return nil + } + state := s.withoutSyncScope().syncExecutionLocks + if state == nil { + return lock.release() + } + if err := lock.release(); err != nil { + return err + } + state.mu.Lock() + if state.bySource[sourceID] == lock { + delete(state.bySource, sourceID) + } + state.mu.Unlock() + return nil +} + +func (s *Store) releaseSyncExecutionLock(runID int64) error { + state := s.withoutSyncScope().syncExecutionLocks + if state == nil { + return nil + } + state.mu.Lock() + runLock, exists := state.byRun[runID] + state.mu.Unlock() + if !exists { + return nil + } + if !runLock.releaseWhenDone { + state.mu.Lock() + delete(state.byRun, runID) + state.mu.Unlock() + return nil + } + if err := runLock.lock.release(); err != nil { + return fmt.Errorf("release sync %d execution lock: %w", runID, err) + } + state.mu.Lock() + delete(state.byRun, runID) + for sourceID, sourceLock := range state.bySource { + if sourceLock == runLock.lock { + delete(state.bySource, sourceID) + break + } + } + state.mu.Unlock() + return nil +} + +func (s *Store) releaseOwnedSyncExecutionLock(sourceID int64, lock syncExecutionLock) error { + state := s.withoutSyncScope().syncExecutionLocks + if state == nil { + return lock.release() + } + state.mu.Lock() + owned := state.bySource[sourceID] == lock + state.mu.Unlock() + if !owned { + return nil + } + if err := lock.release(); err != nil { + return fmt.Errorf("release source %d sync execution lock: %w", sourceID, err) + } + state.mu.Lock() + if state.bySource[sourceID] == lock { + delete(state.bySource, sourceID) + } + for runID, runLock := range state.byRun { + if runLock.lock == lock { + delete(state.byRun, runID) + } + } + state.mu.Unlock() + return nil +} + +func (s *Store) releaseAllSyncExecutionLocks() error { + state := s.withoutSyncScope().syncExecutionLocks + if state == nil { + return nil + } + state.mu.Lock() + locks := make([]syncExecutionLock, 0, len(state.bySource)) + for _, lock := range state.bySource { + if lock != nil { + locks = append(locks, lock) + } + } + state.mu.Unlock() + + var releaseErr error + for _, lock := range locks { + if err := lock.release(); err != nil { + releaseErr = errors.Join(releaseErr, err) + } + } + if releaseErr != nil { + return fmt.Errorf("release sync execution locks: %w", releaseErr) + } + state.mu.Lock() + clear(state.byRun) + clear(state.bySource) + state.mu.Unlock() + return nil +} + +type noOpSyncExecutionLock struct{} + +func (noOpSyncExecutionLock) release() error { return nil } + +var sqliteSyncLockRegistry = struct { + mu sync.Mutex + paths map[string]struct{} +}{paths: make(map[string]struct{})} + +type sqliteSyncExecutionLock struct { + lock *flock.Flock + path string +} + +func (l *sqliteSyncExecutionLock) release() error { + if err := l.lock.Unlock(); err != nil { + return fmt.Errorf("unlock SQLite sync lock: %w", err) + } + sqliteSyncLockRegistry.mu.Lock() + delete(sqliteSyncLockRegistry.paths, l.path) + sqliteSyncLockRegistry.mu.Unlock() + return nil +} + +type postgresSyncExecutionLock struct { + conn *sql.Conn + sourceID int64 + rebind func(string) string +} + +func (l *postgresSyncExecutionLock) release() error { + ctx, cancel := context.WithTimeout(context.Background(), syncExecutionLockCleanupTimeout) + defer cancel() + var released bool + err := l.conn.QueryRowContext(ctx, l.rebind(` + SELECT pg_advisory_unlock( + hashtextextended( + current_schema() || ':msgvault-sync:' || CAST(CAST(? AS BIGINT) AS TEXT), 0 + ) + )`), l.sourceID).Scan(&released) + closeErr := l.conn.Close() + if err != nil { + return errors.Join(fmt.Errorf("unlock PostgreSQL sync lock: %w", err), closeErr) + } + if !released { + return errors.Join(errors.New("PostgreSQL sync lock was not held"), closeErr) + } + return closeErr +} diff --git a/internal/store/sync_test.go b/internal/store/sync_test.go index 98f3b9cb0..28ec7ac71 100644 --- a/internal/store/sync_test.go +++ b/internal/store/sync_test.go @@ -3,6 +3,8 @@ package store_test import ( "context" "database/sql" + "os" + "path/filepath" "testing" "time" @@ -131,28 +133,167 @@ func TestStore_GetLatestSync(t *testing.T) { assert.Equal(store.SyncStatusRunning, run.Status, "Status") } -func TestStore_CompleteSyncRejectsSupersededRun(t *testing.T) { +func TestStore_StartSyncRejectsConcurrentRun(t *testing.T) { require := require.New(t) assert := assert.New(t) f := storetest.New(t) - oldID := f.StartSync() - newID := f.StartSync() - - err := f.Store.CompleteSync(oldID, "stale-cursor") - require.ErrorIs(err, store.ErrSyncRunSuperseded) - - var status string - var cursorAfter sql.NullString - require.NoError(f.Store.DB().QueryRow(f.Store.Rebind(` - SELECT status, cursor_after FROM sync_runs WHERE id = ? - `), oldID).Scan(&status, &cursorAfter)) - assert.Equal(store.SyncStatusFailed, status) - assert.False(cursorAfter.Valid) + activeID := f.StartSync() + _, err := f.Store.StartSync(f.Source.ID, "full") + require.ErrorIs(err, store.ErrSyncAlreadyActive) run, err := f.Store.GetActiveSync(f.Source.ID) require.NoError(err) - assert.Equal(newID, run.ID) + assert.Equal(activeID, run.ID) + + require.NoError(f.Store.CompleteSync(activeID, "cursor")) + _, err = f.Store.StartSync(f.Source.ID, "full") + require.NoError(err) +} + +func TestStore_StartSyncRejectsConcurrentRunAcrossSQLiteStores(t *testing.T) { + requirements := require.New(t) + testutil.SkipIfPostgres(t, "exercises the cross-process SQLite file lock") + dbPath := filepath.Join(t.TempDir(), "archive.db") + first, err := store.OpenForTest(dbPath) + requirements.NoError(err) + t.Cleanup(func() { _ = first.Close() }) + requirements.NoError(first.InitSchema()) + source, err := first.GetOrCreateSource("gmail", "lock-owner@example.com") + requirements.NoError(err) + + second, err := store.OpenForTest(dbPath) + requirements.NoError(err) + t.Cleanup(func() { _ = second.Close() }) + + firstRun, err := first.StartSync(source.ID, "full") + requirements.NoError(err) + _, err = second.StartSync(source.ID, "full") + requirements.ErrorIs(err, store.ErrSyncAlreadyActive) + + requirements.NoError(first.CompleteSync(firstRun, "cursor")) + secondRun, err := second.StartSync(source.ID, "full") + requirements.NoError(err) + requirements.NoError(second.FailSync(secondRun, "test complete")) +} + +func TestStore_StartSyncRecoversRunWhoseOwnerClosed(t *testing.T) { + requirements := require.New(t) + checks := assert.New(t) + first := testutil.NewTestStore(t) + source, err := first.GetOrCreateSource("gmail", "recovery@example.com") + requirements.NoError(err) + _, err = first.CreateSyncOperation(source.ID, "abandoned-operation") + requirements.NoError(err) + abandonedRun, err := first.StartSyncOperation(source.ID, "abandoned-operation") + requirements.NoError(err) + requirements.NoError(first.UpdateSyncCheckpoint(abandonedRun, &store.Checkpoint{ + PageToken: "resume-token", + MessagesProcessed: 17, + MessagesAdded: 11, + })) + requirements.NoError(first.Close()) + + second, err := store.OpenForTest(store.DBPathForTest(first)) + requirements.NoError(err) + t.Cleanup(func() { _ = second.Close() }) + active, err := second.GetActiveSync(source.ID) + requirements.ErrorIs(err, store.ErrSyncRunNotFound) + requirements.Nil(active) + recoveryRun, err := second.StartSync(source.ID, "full") + requirements.NoError(err) + + op, err := second.GetSyncOperation("abandoned-operation") + requirements.NoError(err) + checks.Equal("failed", op.Status) + checks.True(op.FinishedAt.Valid) + requirements.Len(op.Runs, 1) + checks.Equal(store.SyncStatusFailed, op.Runs[0].Status) + checks.Equal("resume-token", op.Runs[0].CursorBefore.String) + checks.Equal(int64(17), op.Runs[0].MessagesProcessed) + checks.Equal(int64(11), op.Runs[0].MessagesAdded) + checks.Equal("sync worker exited before recording completion", op.Runs[0].ErrorMessage.String) + + active, err = second.GetActiveSync(source.ID) + requirements.NoError(err) + checks.Equal(recoveryRun, active.ID) + requirements.NoError(second.FailSync(recoveryRun, "test complete")) +} + +func TestStore_SyncExecutionRetainsOwnershipAcrossRuns(t *testing.T) { + requirements := require.New(t) + first := testutil.NewTestStore(t) + source, err := first.GetOrCreateSource("gmail", "multi-phase-owner@example.com") + requirements.NoError(err) + second, err := store.OpenForTest(store.DBPathForTest(first)) + requirements.NoError(err) + t.Cleanup(func() { _ = second.Close() }) + + execution, err := first.AcquireSyncExecutionContext(t.Context(), source.ID) + requirements.NoError(err) + t.Cleanup(func() { _ = execution.Release() }) + _, err = first.CreateSyncOperation(source.ID, "multi-phase-operation") + requirements.NoError(err) + fullRun, err := execution.StartSyncContext(t.Context(), "full", "multi-phase-operation") + requirements.NoError(err) + requirements.NoError(first.CompleteSync(fullRun, "full-cursor")) + + _, err = second.StartSync(source.ID, "incremental") + requirements.ErrorIs(err, store.ErrSyncAlreadyActive) + + catchupRun, err := execution.StartSyncContext(t.Context(), "incremental", "multi-phase-operation") + requirements.NoError(err) + requirements.NoError(first.CompleteSync(catchupRun, "catchup-cursor")) + requirements.NoError(execution.Release()) + + nextRun, err := second.StartSync(source.ID, "incremental") + requirements.NoError(err) + requirements.NoError(second.FailSync(nextRun, "test complete")) +} + +func TestStore_GetSyncOperationRecoversTerminalRunWhoseOwnerExited(t *testing.T) { + requirements := require.New(t) + st := testutil.NewTestStore(t) + source, err := st.GetOrCreateSource("gmail", "operation-recovery@example.com") + requirements.NoError(err) + execution, err := st.AcquireSyncExecutionContext(t.Context(), source.ID) + requirements.NoError(err) + _, err = st.CreateSyncOperation(source.ID, "abandoned-terminal-operation") + requirements.NoError(err) + runID, err := execution.StartSyncContext(t.Context(), "full", "abandoned-terminal-operation") + requirements.NoError(err) + requirements.NoError(st.CompleteSync(runID, "final-cursor")) + requirements.NoError(execution.Release()) + + op, err := st.GetSyncOperation("abandoned-terminal-operation") + requirements.NoError(err) + requirements.Equal("failed", op.Status) + requirements.True(op.FinishedAt.Valid) + requirements.Len(op.Runs, 1) + requirements.Equal(store.SyncStatusCompleted, op.Runs[0].Status) +} + +func TestStore_StartSyncRejectsConcurrentRunAcrossPostgresStores(t *testing.T) { + requirements := require.New(t) + if !store.IsPostgresURL(os.Getenv("MSGVAULT_TEST_DB")) { + t.Skip("PostgreSQL integration test") + } + first := testutil.NewTestStore(t) + source, err := first.GetOrCreateSource("gmail", "lock-owner@example.com") + requirements.NoError(err) + second, err := store.OpenForTest(store.DBPathForTest(first)) + requirements.NoError(err) + t.Cleanup(func() { _ = second.Close() }) + + firstRun, err := first.StartSync(source.ID, "full") + requirements.NoError(err) + _, err = second.StartSync(source.ID, "full") + requirements.ErrorIs(err, store.ErrSyncAlreadyActive) + + requirements.NoError(first.CompleteSync(firstRun, "cursor")) + secondRun, err := second.StartSync(source.ID, "full") + requirements.NoError(err) + requirements.NoError(second.FailSync(secondRun, "test complete")) } func TestStore_CompleteSyncAndUpdateSourceCursorRejectsSupersededRunAtomically(t *testing.T) { @@ -162,6 +303,7 @@ func TestStore_CompleteSyncAndUpdateSourceCursorRejectsSupersededRunAtomically(t require.NoError(f.Store.UpdateSourceSyncCursor(f.Source.ID, "baseline-cursor")) oldID := f.StartSync() + require.NoError(f.Store.FailSync(oldID, "worker stopped")) newID := f.StartSync() err := f.Store.CompleteSyncAndUpdateSourceCursorContext( @@ -192,6 +334,7 @@ func TestStore_FailSyncAndClearSourceCursorRejectsSupersededRunAtomically(t *tes requirements.NoError(f.Store.UpdateSourceSyncCursor(f.Source.ID, "baseline-cursor")) oldID := f.StartSync() + requirements.NoError(f.Store.FailSync(oldID, "worker stopped")) newID := f.StartSync() err := f.Store.FailSyncAndClearSourceCursorContext( t.Context(), oldID, f.Source.ID, "expired cursor", @@ -214,15 +357,18 @@ func TestStore_FailSyncAndClearSourceCursorRejectsSupersededRunAtomically(t *tes } func TestScopedStoreRejectsEveryImporterMutationAfterSupersession(t *testing.T) { + requirements := require.New(t) + checks := assert.New(t) f := storetest.New(t) messageID := f.CreateMessage("generation-fence-message") var conversationID int64 - require.NoError(t, f.Store.DB().QueryRow(f.Store.Rebind( + requirements.NoError(f.Store.DB().QueryRow(f.Store.Rebind( `SELECT conversation_id FROM messages WHERE id = ?`), messageID, ).Scan(&conversationID)) participantID := f.EnsureParticipant("reactor@example.test", "Reactor", "example.test") oldID := f.StartSync() stale := f.Store.ScopedToSync(f.Source.ID, oldID) + requirements.NoError(f.Store.FailSync(oldID, "worker stopped")) _ = f.StartSync() tests := []struct { @@ -306,14 +452,14 @@ func TestScopedStoreRejectsEveryImporterMutationAfterSupersession(t *testing.T) } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - require.ErrorIs(t, test.write(), store.ErrSyncRunSuperseded) + require.New(t).ErrorIs(test.write(), store.ErrSyncRunSuperseded) }) } var staleEmailThreads int - require.NoError(t, f.Store.DB().QueryRow(f.Store.Rebind(`SELECT COUNT(*) FROM conversations + requirements.NoError(f.Store.DB().QueryRow(f.Store.Rebind(`SELECT COUNT(*) FROM conversations WHERE source_id = ? AND source_conversation_id = 'stale-email-thread'`), f.Source.ID).Scan(&staleEmailThreads)) - assert.Zero(t, staleEmailThreads) + checks.Zero(staleEmailThreads) } func TestScopedSourceWriteMatchesStartSyncLockOrder(t *testing.T) { @@ -410,6 +556,7 @@ func TestSupersededSyncDoesNotCoalescePersonSweep(t *testing.T) { requirements.NoError(err) f.insertMessage(t, "superseded-sync-change", "email", f.aliceID, time.Date(2026, 8, 23, 12, 0, 0, 0, time.UTC)) + requirements.NoError(f.store.FailSync(oldSyncID, "worker stopped")) newSyncID, err := f.store.StartSync(f.sourceID, "incremental") requirements.NoError(err) @@ -486,6 +633,65 @@ func TestStore_GetLatestCheckpointedSyncNeverFallsBackPastCompletion(t *testing. require.ErrorIs(err, store.ErrSyncRunNotFound) } +func TestStore_SyncOperationGroupsRunsAndPublishesFinalState(t *testing.T) { + require := require.New(t) + assert := assert.New(t) + f := storetest.New(t) + + pending, err := f.Store.CreateSyncOperation(f.Source.ID, "operation-1") + require.NoError(err) + assert.Equal("pending", pending.Status) + assert.Equal(f.Source.ID, pending.SourceID) + assert.False(pending.CreatedAt.IsZero()) + assert.False(pending.StartedAt.Valid) + assert.Empty(pending.Runs) + pending, err = f.Store.GetSyncOperation("operation-1") + require.NoError(err) + assert.Equal("pending", pending.Status) + assert.Equal(f.Source.ID, pending.SourceID) + assert.Empty(pending.Runs) + + execution, err := f.Store.AcquireSyncExecutionContext(t.Context(), f.Source.ID) + require.NoError(err) + t.Cleanup(func() { _ = execution.Release() }) + firstID, err := execution.StartSyncContext(t.Context(), "full", "operation-1") + require.NoError(err) + require.NoError(f.Store.CompleteSync(firstID, "first")) + secondID, err := execution.StartSyncContext(t.Context(), "incremental", "operation-1") + require.NoError(err) + require.NoError(f.Store.CompleteSync(secondID, "second")) + require.NoError(execution.Release()) + require.NoError(f.Store.FinishSyncOperation("operation-1", "done")) + + op, err := f.Store.GetSyncOperation("operation-1") + require.NoError(err) + assert.Equal("done", op.Status) + assert.True(op.StartedAt.Valid) + assert.True(op.FinishedAt.Valid) + require.Len(op.Runs, 2) + assert.Equal(firstID, op.Runs[0].ID) + assert.Equal(secondID, op.Runs[1].ID) +} + +func TestStore_FailPendingSyncOperations(t *testing.T) { + require := require.New(t) + assert := assert.New(t) + f := storetest.New(t) + + _, err := f.Store.CreateSyncOperation(f.Source.ID, "orphaned-operation") + require.NoError(err) + failed, err := f.Store.FailPendingSyncOperationsContext(t.Context()) + require.NoError(err) + assert.Equal(int64(1), failed) + + op, err := f.Store.GetSyncOperation("orphaned-operation") + require.NoError(err) + assert.Equal("failed", op.Status) + assert.False(op.StartedAt.Valid) + assert.True(op.FinishedAt.Valid) + assert.Empty(op.Runs) +} + func TestStore_SyncRunItems(t *testing.T) { require := require.New(t) assert := assert.New(t) @@ -631,12 +837,11 @@ func TestStore_HasAnyActiveSync(t *testing.T) { require.NoError(err, "HasAnyActiveSync (after StartSync)") assert.True(running, "expected active sync after StartSync") - // A second StartSync on the same source marks the prior one failed, but - // itself is running. - _ = f.StartSync() + _, err = f.Store.StartSync(f.Source.ID, "full") + require.ErrorIs(err, store.ErrSyncAlreadyActive) running, err = f.Store.HasAnyActiveSync() - require.NoError(err, "HasAnyActiveSync (after second StartSync)") - assert.True(running, "expected an active sync after second StartSync") + require.NoError(err, "HasAnyActiveSync (after rejected StartSync)") + assert.True(running, "the original sync remains active") // Mark the latest sync as completed. _, err = f.Store.DB().Exec( diff --git a/internal/sync/incremental.go b/internal/sync/incremental.go index 9391ecee5..50b2fa12e 100644 --- a/internal/sync/incremental.go +++ b/internal/sync/incremental.go @@ -23,7 +23,14 @@ func (s *Syncer) Incremental(ctx context.Context, source *store.Source) (summary if source == nil { return nil, errors.New("no source provided - run full sync first") } + return s.runWithSyncExecution(ctx, source.ID, func(execution *store.SyncExecution) (*gmail.SyncSummary, error) { + return s.incremental(ctx, source, execution) + }) +} +func (s *Syncer) incremental( + ctx context.Context, source *store.Source, execution *store.SyncExecution, +) (summary *gmail.SyncSummary, err error) { startTime := time.Now() summary = &gmail.SyncSummary{StartTime: startTime} @@ -38,7 +45,7 @@ func (s *Syncer) Incremental(ctx context.Context, source *store.Source) (summary } // Start sync - syncID, err := s.store.StartSync(source.ID, "incremental") + syncID, err := s.startSync(ctx, execution, "incremental", "") if err != nil { return nil, fmt.Errorf("start sync: %w", err) } @@ -83,7 +90,7 @@ func (s *Syncer) Incremental(ctx context.Context, source *store.Source) (summary if startHistoryID >= profile.HistoryID { s.logger.Info("already up to date") if err := s.completeSyncWithoutHook( - ctx, syncID, source.ID, strconv.FormatUint(profile.HistoryID, 10), + ctx, syncID, source.ID, strconv.FormatUint(profile.HistoryID, 10), true, ); err != nil { return nil, err } @@ -238,7 +245,7 @@ func (s *Syncer) Incremental(ctx context.Context, source *store.Source) (summary discoveryHealth.observe(s.runPageIdentityDiscovery(ctx, source.ID, identityDiscoveryIDs)) if ctxErr := ctx.Err(); ctxErr != nil { err := fmt.Errorf("sync canceled during identity discovery: %w", ctxErr) - s.failSyncUnlessCanceled(syncID, err) + s.failStoppedSync(syncID, err) return nil, err } diff --git a/internal/sync/sync.go b/internal/sync/sync.go index abd33acc3..6c40cb23b 100644 --- a/internal/sync/sync.go +++ b/internal/sync/sync.go @@ -4,6 +4,7 @@ package sync import ( "bytes" "context" + "crypto/sha256" "database/sql" "errors" "fmt" @@ -56,6 +57,17 @@ type Options struct { // The API listing call (which returns lightweight IDs, not bodies) may // return more IDs than the limit; only the truncated set is fetched. Limit int + + // OperationID attributes every sync phase to one daemon invocation. + OperationID string +} + +func (s *Syncer) startSync( + ctx context.Context, execution *store.SyncExecution, syncType, requestFingerprint string, +) (int64, error) { + return execution.StartSyncWithRequestContext( + ctx, syncType, s.opts.OperationID, requestFingerprint, + ) } // DefaultOptions returns sensible defaults. @@ -215,13 +227,25 @@ func (s *Syncer) runSuccessfulSyncHook(ctx context.Context, source *store.Source // completeSyncWithoutHook atomically publishes the source cursor and marks the // still-current run complete. func (s *Syncer) completeSyncWithoutHook( - ctx context.Context, syncID int64, sourceID int64, historyID string, + ctx context.Context, + syncID int64, + sourceID int64, + historyID string, + publishSourceCursor bool, ) error { - if err := s.store.CompleteSyncAndUpdateSourceCursorContext( - ctx, syncID, sourceID, historyID, - ); err != nil { + var err error + if publishSourceCursor { + err = s.store.CompleteSyncAndUpdateSourceCursorContext( + ctx, syncID, sourceID, historyID, + ) + } else { + err = s.store.CompleteSyncAndPreserveSourceCursorContext( + ctx, syncID, sourceID, historyID, + ) + } + if err != nil { if !errors.Is(err, store.ErrSyncRunSuperseded) { - s.failSyncUnlessCanceled(syncID, err) + s.failStoppedSync(syncID, err) } return fmt.Errorf("publish completed sync: %w", err) } @@ -234,7 +258,11 @@ func (s *Syncer) completeSyncAndRunHook( historyID string, source *store.Source, ) error { - if err := s.completeSyncWithoutHook(ctx, syncID, source.ID, historyID); err != nil { + publishSourceCursor := source.SourceType != "gmail" || + (s.opts.Query == "" && s.opts.Limit == 0) + if err := s.completeSyncWithoutHook( + ctx, syncID, source.ID, historyID, publishSourceCursor, + ); err != nil { return err } s.runSuccessfulSyncHook(ctx, source, true) @@ -357,53 +385,69 @@ type syncState struct { wasResumed bool } +const historyRecoveryRequestFingerprint = "gmail-history-recovery:v1" + +func checkpointMatchesRequest(run *store.SyncRun, requestFingerprint string) bool { + return run != nil && + run.RequestFingerprint.Valid && + run.RequestFingerprint.String == requestFingerprint +} + func isPinnedHistoryRecovery(run *store.SyncRun) bool { - return run != nil && run.CursorAfter.Valid && run.CursorAfter.String != "" + return checkpointMatchesRequest(run, historyRecoveryRequestFingerprint) && + run.CursorAfter.Valid && run.CursorAfter.String != "" } -// failSyncUnlessCanceled marks the run failed for real errors. A cancelled -// sync (Ctrl-C, daemon shutdown, a scheduled sync yielding to a waiting -// operation) keeps status='running' with its saved checkpoint, matching the -// killed-process semantics GetActiveSync resumes from; marking it failed -// would discard the checkpoint and restart the sync from scratch. -func (s *Syncer) failSyncUnlessCanceled(syncID int64, err error) { - if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { - return - } +func (s *Syncer) fullSyncRequestFingerprint() string { + request := fmt.Sprintf( + "full:v1\x00%s\x00%s\x00%d\x00%t", + s.opts.SourceType, s.opts.Query, s.opts.Limit, s.opts.NoResume, + ) + return fmt.Sprintf("full:v1:%x", sha256.Sum256([]byte(request))) +} + +// failStoppedSync marks the stopped worker's run failed. Checkpoints on +// failed runs remain resumable, while running status stays reserved for a live +// worker. +func (s *Syncer) failStoppedSync(syncID int64, err error) { _ = s.store.FailSync(syncID, err.Error()) } // initSyncState initializes sync state, resuming from checkpoint if possible. -func (s *Syncer) initSyncState(sourceID int64) (*syncState, error) { +func (s *Syncer) initSyncState( + ctx context.Context, sourceID int64, execution *store.SyncExecution, +) (*syncState, error) { + requestFingerprint := s.fullSyncRequestFingerprint() state := &syncState{ checkpoint: &store.Checkpoint{}, } if !s.opts.NoResume { - activeSync, err := s.store.GetActiveSync(sourceID) + priorSync, err := s.store.GetLatestCheckpointedSync(sourceID) if err != nil && !errors.Is(err, store.ErrSyncRunNotFound) { - return nil, fmt.Errorf("check active sync: %w", err) + return nil, fmt.Errorf("check checkpointed sync: %w", err) } - if activeSync != nil { - state.syncID = activeSync.ID - if activeSync.CursorBefore.Valid { - state.pageToken = activeSync.CursorBefore.String + if checkpointMatchesRequest(priorSync, requestFingerprint) { + if priorSync.Status == store.SyncStatusRunning { + return nil, fmt.Errorf("source %d sync %d: %w", sourceID, priorSync.ID, store.ErrSyncAlreadyActive) + } + if priorSync.CursorBefore.Valid { + state.pageToken = priorSync.CursorBefore.String } state.checkpoint = &store.Checkpoint{ PageToken: state.pageToken, - MessagesProcessed: activeSync.MessagesProcessed, - MessagesAdded: activeSync.MessagesAdded, - MessagesUpdated: activeSync.MessagesUpdated, - ErrorsCount: activeSync.ErrorsCount, + MessagesProcessed: priorSync.MessagesProcessed, + MessagesAdded: priorSync.MessagesAdded, + MessagesUpdated: priorSync.MessagesUpdated, + ErrorsCount: priorSync.ErrorsCount, } state.wasResumed = true s.logger.Info("resuming sync", "messages_processed", state.checkpoint.MessagesProcessed) - return state, nil } } // Start new sync - syncID, err := s.store.StartSync(sourceID, "full") + syncID, err := s.startSync(ctx, execution, "full", requestFingerprint) if err != nil { return nil, fmt.Errorf("start sync: %w", err) } @@ -414,28 +458,38 @@ func (s *Syncer) initSyncState(sourceID int64) (*syncState, error) { // initHistoryRecoveryState only resumes a full run that already pinned its // history handoff cursor. An ordinary full-sync checkpoint cannot be reused: // its processed prefix may have changed before recovery captured a cursor. -func (s *Syncer) initHistoryRecoveryState(sourceID int64) (*syncState, error) { +func (s *Syncer) initHistoryRecoveryState( + ctx context.Context, sourceID int64, execution *store.SyncExecution, +) (*syncState, error) { if !s.opts.NoResume { - activeSync, err := s.store.GetActiveSync(sourceID) + priorSync, err := s.store.GetLatestCheckpointedSync(sourceID) if err != nil && !errors.Is(err, store.ErrSyncRunNotFound) { - return nil, fmt.Errorf("check active history recovery: %w", err) + return nil, fmt.Errorf("check checkpointed history recovery: %w", err) + } + if priorSync != nil && priorSync.Status == store.SyncStatusRunning { + return nil, fmt.Errorf("source %d sync %d: %w", sourceID, priorSync.ID, store.ErrSyncAlreadyActive) } - if isPinnedHistoryRecovery(activeSync) { + if isPinnedHistoryRecovery(priorSync) { state := &syncState{ - syncID: activeSync.ID, checkpoint: &store.Checkpoint{}, - handoffCursor: activeSync.CursorAfter.String, + handoffCursor: priorSync.CursorAfter.String, wasResumed: true, } - if activeSync.CursorBefore.Valid { - state.pageToken = activeSync.CursorBefore.String + if priorSync.CursorBefore.Valid { + state.pageToken = priorSync.CursorBefore.String } state.checkpoint = &store.Checkpoint{ PageToken: state.pageToken, - MessagesProcessed: activeSync.MessagesProcessed, - MessagesAdded: activeSync.MessagesAdded, - MessagesUpdated: activeSync.MessagesUpdated, - ErrorsCount: activeSync.ErrorsCount, + MessagesProcessed: priorSync.MessagesProcessed, + MessagesAdded: priorSync.MessagesAdded, + MessagesUpdated: priorSync.MessagesUpdated, + ErrorsCount: priorSync.ErrorsCount, + } + state.syncID, err = s.startSync( + ctx, execution, "full", historyRecoveryRequestFingerprint, + ) + if err != nil { + return nil, fmt.Errorf("start history recovery: %w", err) } s.logger.Info("resuming Gmail history recovery", "messages_processed", state.checkpoint.MessagesProcessed, @@ -444,7 +498,7 @@ func (s *Syncer) initHistoryRecoveryState(sourceID int64) (*syncState, error) { } } - syncID, err := s.store.StartSync(sourceID, "full") + syncID, err := s.startSync(ctx, execution, "full", historyRecoveryRequestFingerprint) if err != nil { return nil, fmt.Errorf("start history recovery: %w", err) } @@ -883,24 +937,79 @@ func (s *Syncer) processBatch(ctx context.Context, syncID, sourceID int64, listR // Full performs a full synchronization. func (s *Syncer) Full(ctx context.Context, email string) (summary *gmail.SyncSummary, err error) { + return s.FullWithFinalizer(ctx, email, nil) +} + +// FullWithFinalizer performs a full synchronization and runs finalizer before +// releasing source ownership. Callers use it for provider state that must not +// race with the next sync. +func (s *Syncer) FullWithFinalizer( + ctx context.Context, + email string, + finalizer func(*gmail.SyncSummary) error, +) (summary *gmail.SyncSummary, err error) { sourceType := s.opts.SourceType if sourceType == "" { sourceType = "gmail" } - if sourceType == "gmail" && !s.opts.NoResume { - source, sourceErr := s.store.GetOrCreateSource(sourceType, email) - if sourceErr != nil { - return nil, fmt.Errorf("get/create source: %w", sourceErr) + source, err := s.store.GetOrCreateSource(sourceType, email) + if err != nil { + return nil, fmt.Errorf("get/create source: %w", err) + } + return s.runWithSyncExecution(ctx, source.ID, func(execution *store.SyncExecution) (*gmail.SyncSummary, error) { + if sourceType == "gmail" && !s.opts.NoResume && s.opts.Query == "" && s.opts.Limit == 0 { + prior, priorErr := s.store.GetLatestCheckpointedSync(source.ID) + if priorErr != nil && !errors.Is(priorErr, store.ErrSyncRunNotFound) { + return nil, fmt.Errorf("check checkpointed history recovery: %w", priorErr) + } + if isPinnedHistoryRecovery(prior) { + return s.recoverExpiredHistory(ctx, source, execution) + } } - active, activeErr := s.store.GetActiveSync(source.ID) - if activeErr != nil && !errors.Is(activeErr, store.ErrSyncRunNotFound) { - return nil, fmt.Errorf("check active history recovery: %w", activeErr) + summary, err := s.full(ctx, source, false, execution) + if err != nil { + return nil, err } - if isPinnedHistoryRecovery(active) { - return s.RecoverExpiredHistory(ctx, source) + if finalizer != nil { + if err := finalizer(summary); err != nil { + return nil, fmt.Errorf("finalize full sync: %w", err) + } } + return summary, nil + }) +} + +func (s *Syncer) runWithSyncExecution( + ctx context.Context, + sourceID int64, + run func(*store.SyncExecution) (*gmail.SyncSummary, error), +) (summary *gmail.SyncSummary, err error) { + execution, err := s.store.AcquireSyncExecutionContext(ctx, sourceID) + if err != nil { + return nil, fmt.Errorf("acquire source %d sync ownership: %w", sourceID, err) } - return s.full(ctx, email, false) + defer func() { + if recovered := recover(); recovered != nil { + if s.opts.OperationID != "" { + _ = s.store.FinishSyncOperation(s.opts.OperationID, "failed") + } + _ = execution.Release() + panic(recovered) + } + if s.opts.OperationID != "" { + status := "done" + if err != nil { + status = "failed" + } + if finishErr := s.store.FinishSyncOperation(s.opts.OperationID, status); finishErr != nil { + err = errors.Join(err, fmt.Errorf("finish sync operation: %w", finishErr)) + } + } + if releaseErr := execution.Release(); releaseErr != nil { + err = errors.Join(err, releaseErr) + } + }() + return run(execution) } // RecoverExpiredHistory rebuilds the archive from a complete Gmail listing, @@ -918,8 +1027,15 @@ func (s *Syncer) RecoverExpiredHistory( if s.opts.Query != "" || s.opts.Limit > 0 { return nil, errors.New("recover expired history requires an unfiltered, unlimited full sync") } + return s.runWithSyncExecution(ctx, source.ID, func(execution *store.SyncExecution) (*gmail.SyncSummary, error) { + return s.recoverExpiredHistory(ctx, source, execution) + }) +} - fullSummary, err := s.full(ctx, source.Identifier, true) +func (s *Syncer) recoverExpiredHistory( + ctx context.Context, source *store.Source, execution *store.SyncExecution, +) (*gmail.SyncSummary, error) { + fullSummary, err := s.full(ctx, source, true, execution) if err != nil { return nil, fmt.Errorf("recover expired history: full sync: %w", err) } @@ -927,7 +1043,7 @@ func (s *Syncer) RecoverExpiredHistory( if err != nil { return nil, fmt.Errorf("recover expired history: reload source: %w", err) } - catchup, err := s.Incremental(ctx, refreshed) + catchup, err := s.incremental(ctx, refreshed, execution) if err != nil { return nil, fmt.Errorf("recover expired history: catch up from full-sync cursor: %w", err) } @@ -953,48 +1069,57 @@ func (s *Syncer) IncrementalWithHistoryRecovery( if source == nil { return nil, errors.New("no source provided - run full sync first") } - active, err := s.store.GetActiveSync(source.ID) + return s.runWithSyncExecution(ctx, source.ID, func(execution *store.SyncExecution) (*gmail.SyncSummary, error) { + return s.incrementalWithHistoryRecovery(ctx, source, onRecovery, execution) + }) +} + +func (s *Syncer) incrementalWithHistoryRecovery( + ctx context.Context, + source *store.Source, + onRecovery func(resumed bool), + execution *store.SyncExecution, +) (*gmail.SyncSummary, error) { + prior, err := s.store.GetLatestCheckpointedSync(source.ID) if err != nil && !errors.Is(err, store.ErrSyncRunNotFound) { - return nil, fmt.Errorf("check active history recovery: %w", err) + return nil, fmt.Errorf("check checkpointed history recovery: %w", err) + } + if prior != nil && prior.Status == store.SyncStatusRunning { + return nil, fmt.Errorf("source %d sync %d: %w", source.ID, prior.ID, store.ErrSyncAlreadyActive) } - if !s.opts.NoResume && isPinnedHistoryRecovery(active) { + if !s.opts.NoResume && isPinnedHistoryRecovery(prior) { if onRecovery != nil { onRecovery(true) } - return s.RecoverExpiredHistory(ctx, source) + return s.recoverExpiredHistory(ctx, source, execution) } - summary, err := s.Incremental(ctx, source) + summary, err := s.incremental(ctx, source, execution) if !errors.Is(err, ErrHistoryExpired) { return summary, err } if onRecovery != nil { onRecovery(false) } - return s.RecoverExpiredHistory(ctx, source) + return s.recoverExpiredHistory(ctx, source, execution) } -func (s *Syncer) full(ctx context.Context, email string, reconcilePresence bool) (summary *gmail.SyncSummary, err error) { +func (s *Syncer) full( + ctx context.Context, + source *store.Source, + reconcilePresence bool, + execution *store.SyncExecution, +) (summary *gmail.SyncSummary, err error) { startTime := time.Now() summary = &gmail.SyncSummary{StartTime: startTime} - // Get or create source - sourceType := s.opts.SourceType - if sourceType == "" { - sourceType = "gmail" - } - source, err := s.store.GetOrCreateSource(sourceType, email) - if err != nil { - return nil, fmt.Errorf("get/create source: %w", err) - } - // Recovery may only resume a run that pinned its history handoff cursor. // Ordinary full-sync checkpoints predate that cursor and are unsafe to reuse. var state *syncState if reconcilePresence { - state, err = s.initHistoryRecoveryState(source.ID) + state, err = s.initHistoryRecoveryState(ctx, source.ID, execution) } else { - state, err = s.initSyncState(source.ID) + state, err = s.initSyncState(ctx, source.ID, execution) } if err != nil { return nil, err @@ -1022,7 +1147,7 @@ func (s *Syncer) full(ctx context.Context, email string, reconcilePresence bool) // Get profile to verify connection and get historyId profile, err := s.client.GetProfile(ctx) if err != nil { - s.failSyncUnlessCanceled(state.syncID, err) + s.failStoppedSync(state.syncID, err) return nil, fmt.Errorf("get profile: %w", err) } handoffHistoryID := profile.HistoryID @@ -1030,13 +1155,13 @@ func (s *Syncer) full(ctx context.Context, email string, reconcilePresence bool) if state.handoffCursor == "" { state.handoffCursor = strconv.FormatUint(profile.HistoryID, 10) if err := s.store.PinSyncHandoffCursorContext(ctx, state.syncID, state.handoffCursor); err != nil { - s.failSyncUnlessCanceled(state.syncID, err) + s.failStoppedSync(state.syncID, err) return nil, fmt.Errorf("pin Gmail history recovery cursor: %w", err) } } else { handoffHistoryID, err = strconv.ParseUint(state.handoffCursor, 10, 64) if err != nil { - s.failSyncUnlessCanceled(state.syncID, err) + s.failStoppedSync(state.syncID, err) return nil, fmt.Errorf("parse Gmail history recovery cursor %q: %w", state.handoffCursor, err) } } @@ -1047,7 +1172,7 @@ func (s *Syncer) full(ctx context.Context, email string, reconcilePresence bool) // Sync labels labelMap, err := s.syncLabels(ctx, source.ID) if err != nil { - s.failSyncUnlessCanceled(state.syncID, err) + s.failStoppedSync(state.syncID, err) return nil, fmt.Errorf("sync labels: %w", err) } @@ -1064,7 +1189,7 @@ func (s *Syncer) full(ctx context.Context, email string, reconcilePresence bool) // List messages listResp, err := s.client.ListMessages(ctx, s.opts.Query, pageToken) if err != nil { - s.failSyncUnlessCanceled(state.syncID, err) + s.failStoppedSync(state.syncID, err) return nil, fmt.Errorf("list messages: %w", err) } @@ -1095,14 +1220,14 @@ func (s *Syncer) full(ctx context.Context, email string, reconcilePresence bool) if checkpointErr := s.store.UpdateSyncCheckpoint(state.syncID, state.checkpoint); checkpointErr != nil { s.logger.Warn("failed to save checkpoint before failing sync", "error", checkpointErr) } - s.failSyncUnlessCanceled(state.syncID, err) + s.failStoppedSync(state.syncID, err) return nil, err } discoveryHealth.observe(s.runPageIdentityDiscovery(ctx, source.ID, result.sourceMessageIDs)) if ctxErr := ctx.Err(); ctxErr != nil { err := fmt.Errorf("sync canceled during identity discovery: %w", ctxErr) - s.failSyncUnlessCanceled(state.syncID, err) + s.failStoppedSync(state.syncID, err) return nil, err } if ack, ok := s.client.(messageAcknowledger); ok && len(result.acknowledged) > 0 { @@ -1144,12 +1269,12 @@ func (s *Syncer) full(ctx context.Context, email string, reconcilePresence bool) if reconcilePresence { present, err := s.listCompleteMessageSnapshot(ctx) if err != nil { - s.failSyncUnlessCanceled(state.syncID, err) + s.failStoppedSync(state.syncID, err) return nil, err } reconciled, err := s.store.ReconcileSourceMessageSnapshot(ctx, source.ID, present) if err != nil { - s.failSyncUnlessCanceled(state.syncID, err) + s.failStoppedSync(state.syncID, err) return nil, fmt.Errorf("reconcile Gmail message snapshot: %w", err) } if reconciled > 0 { diff --git a/internal/sync/sync_test.go b/internal/sync/sync_test.go index 1a60309f1..2fa6228c4 100644 --- a/internal/sync/sync_test.go +++ b/internal/sync/sync_test.go @@ -316,32 +316,28 @@ func TestFullSyncProviderHookDoesNotRunAfterFailedSync(t *testing.T) { assert.Zero(t, hookCalls) } -func TestFullSyncSupersededGenerationDoesNotPublishCursorOrReturnSuccess(t *testing.T) { +func TestFullSyncRejectsConcurrentStart(t *testing.T) { require := require.New(t) assert := assert.New(t) env := newTestEnv(t) source := env.CreateSource(t) require.NoError(env.Store.UpdateSourceSyncCursor(source.ID, "baseline-cursor")) - var newerSyncID int64 + var concurrentErr error env.Syncer = New(&supersedingProfileAPI{ MockAPI: env.Mock, supersede: func() { - var err error - newerSyncID, err = env.Store.StartSync(source.ID, "full") - require.NoError(err) + _, concurrentErr = env.Store.StartSync(source.ID, "full") }, }, env.Store, nil) summary, err := env.Syncer.Full(env.Context, testEmail) - require.ErrorIs(err, store.ErrSyncRunSuperseded) - assert.Nil(summary) - source, err = env.Store.GetSourceByID(source.ID) require.NoError(err) - assert.Equal("baseline-cursor", source.SyncCursor.String) - active, err := env.Store.GetActiveSync(source.ID) + require.ErrorIs(concurrentErr, store.ErrSyncAlreadyActive) + require.NotNil(summary) + source, err = env.Store.GetSourceByID(source.ID) require.NoError(err) - assert.Equal(newerSyncID, active.ID) + assert.NotEqual("baseline-cursor", source.SyncCursor.String) } func TestFullSyncCompletionFailureMarksRunFailed(t *testing.T) { @@ -374,32 +370,24 @@ func TestFullSyncCompletionFailureMarksRunFailed(t *testing.T) { assert.Contains(run.ErrorMessage.String, "forced sync completion failure") } -func TestIncrementalSyncSupersededGenerationDoesNotPublishCursorOrReturnSuccess(t *testing.T) { +func TestIncrementalSyncRejectsConcurrentStart(t *testing.T) { require := require.New(t) - assert := assert.New(t) env := newTestEnv(t) source := env.CreateSourceWithHistory(t, "1000") env.Mock.Profile.HistoryID = 1000 - var newerSyncID int64 + var concurrentErr error env.Syncer = New(&supersedingProfileAPI{ MockAPI: env.Mock, supersede: func() { - var err error - newerSyncID, err = env.Store.StartSync(source.ID, "incremental") - require.NoError(err) + _, concurrentErr = env.Store.StartSync(source.ID, "incremental") }, }, env.Store, nil) summary, err := env.Syncer.Incremental(env.Context, source) - require.ErrorIs(err, store.ErrSyncRunSuperseded) - assert.Nil(summary) - source, err = env.Store.GetSourceByID(source.ID) - require.NoError(err) - assert.Equal("1000", source.SyncCursor.String) - active, err := env.Store.GetActiveSync(source.ID) require.NoError(err) - assert.Equal(newerSyncID, active.ID) + require.ErrorIs(concurrentErr, store.ErrSyncAlreadyActive) + require.NotNil(summary) } // TestIncrementalSyncProviderHookRunsAfterSuccessfulCompletion also pins the @@ -635,7 +623,7 @@ func TestSyncPageRetryAfterCheckpointFailureIsCaseFoldedAndIdempotent(t *testing assertMessageCount(t, env.Store, 1) run, err := env.Store.GetLatestSync(source.ID) require.NoError(err, "GetLatestSync") - assert.Equal(store.SyncStatusRunning, run.Status, "cancelled run remains resumable") + assert.Equal(store.SyncStatusFailed, run.Status, "cancelled worker leaves a resumable failed run") assert.Equal(int64(0), run.MessagesProcessed, "failed checkpoint does not advance the page") identities, err := env.Store.ListAccountIdentities(source.ID) @@ -652,7 +640,7 @@ func TestSyncPageRetryAfterCheckpointFailureIsCaseFoldedAndIdempotent(t *testing env.Syncer = New(env.Mock, env.Store, nil) summary, err := env.Syncer.Full(env.Context, testEmail) require.NoError(err, "resume sync") - assert.True(summary.WasResumed, "retry resumes the uncheckpointed run") + assert.False(summary.WasResumed, "a failed checkpoint write restarts the traversal") identities, err = env.Store.ListAccountIdentities(source.ID) require.NoError(err, "ListAccountIdentities after retry") @@ -706,7 +694,7 @@ func (c *cancelOnSecondListAPI) ListMessages(ctx context.Context, query, pageTok return c.MockAPI.ListMessages(ctx, query, pageToken) } -func TestFullSyncCanceledKeepsRunResumable(t *testing.T) { +func TestFullSyncCanceledFailsRunAndKeepsCheckpointResumable(t *testing.T) { require := require.New(t) assert := assert.New(t) env := newTestEnv(t) @@ -721,7 +709,7 @@ func TestFullSyncCanceledKeepsRunResumable(t *testing.T) { require.NoError(err, "GetSourceByIdentifier") run, err := env.Store.GetLatestSync(source.ID) require.NoError(err, "GetLatestSync") - assert.Equal(store.SyncStatusRunning, run.Status, "cancelled run keeps status running") + assert.Equal(store.SyncStatusFailed, run.Status, "cancelled worker marks its run failed") assert.Equal(int64(2), run.MessagesProcessed, "checkpoint keeps first page progress") env.Syncer = New(env.Mock, env.Store, nil) @@ -733,6 +721,86 @@ func TestFullSyncCanceledKeepsRunResumable(t *testing.T) { assertMessageCount(t, env.Store, 4) } +func TestFullSyncRestartsWhenCheckpointRequestDiffers(t *testing.T) { + requirements := require.New(t) + checks := assert.New(t) + env := newTestEnv(t) + env.Mock.Profile.HistoryID = 12345 + seedPagedMessages(env, 4) + + firstOptions := DefaultOptions() + firstOptions.Query = "after:2024/01/01" + env.Syncer = New( + &cancelOnSecondListAPI{MockAPI: env.Mock}, env.Store, firstOptions, + ) + _, err := env.Syncer.Full(env.Context, testEmail) + requirements.ErrorIs(err, context.Canceled) + + secondOptions := DefaultOptions() + secondOptions.Query = "before:2024/01/01" + summary, err := New(env.Mock, env.Store, secondOptions).Full(env.Context, testEmail) + requirements.NoError(err) + checks.False(summary.WasResumed) + checks.Empty(summary.ResumedFromToken) +} + +func TestBoundedGmailFullSyncPreservesIncrementalCursor(t *testing.T) { + requirements := require.New(t) + checks := assert.New(t) + env := newTestEnv(t) + source := env.CreateSourceWithHistory(t, "1000") + env.Mock.Profile.HistoryID = 2000 + seedMessages(env, 1, 2000, "bounded-message") + + options := DefaultOptions() + options.Query = "after:2024/01/01" + _, err := New(env.Mock, env.Store, options).Full(env.Context, testEmail) + requirements.NoError(err) + + refreshed, err := env.Store.GetSourceByID(source.ID) + requirements.NoError(err) + requirements.True(refreshed.SyncCursor.Valid) + checks.Equal("1000", refreshed.SyncCursor.String) + checks.True(refreshed.LastSyncAt.Valid) + + run, err := env.Store.GetLatestSync(source.ID) + requirements.NoError(err) + checks.Equal("2000", run.CursorAfter.String) +} + +func TestFullSyncFinalizerRetainsSourceOwnership(t *testing.T) { + requirements := require.New(t) + checks := assert.New(t) + env := newTestEnv(t) + source := env.CreateSource(t) + env.Mock.Profile.HistoryID = 12345 + seedMessages(env, 1, 12345, "message") + + options := DefaultOptions() + options.OperationID = "finalizer-operation" + _, err := env.Store.CreateSyncOperation(source.ID, options.OperationID) + requirements.NoError(err) + syncer := New(env.Mock, env.Store, options) + var competingErr error + _, err = syncer.FullWithFinalizer( + env.Context, + testEmail, + func(*gmail.SyncSummary) error { + _, competingErr = env.Store.StartSync(source.ID, "competing") + op, opErr := env.Store.GetSyncOperation(options.OperationID) + requirements.NoError(opErr) + checks.Equal("running", op.Status) + return nil + }, + ) + requirements.NoError(err) + requirements.ErrorIs(competingErr, store.ErrSyncAlreadyActive) + + op, err := env.Store.GetSyncOperation(options.OperationID) + requirements.NoError(err) + checks.Equal("done", op.Status) +} + func TestFullSyncAcknowledgesOnlySafelyHandledMessages(t *testing.T) { require := require.New(t) assert := assert.New(t) @@ -940,7 +1008,7 @@ func TestSyncCancellationDuringDiscoveryStaysResumable(t *testing.T) { run, err := env.Store.GetLatestSync(source.ID) require.NoError(err, "GetLatestSync") - assert.Equal(store.SyncStatusRunning, run.Status, "a cancelled run stays resumable") + assert.Equal(store.SyncStatusFailed, run.Status, "a cancelled worker leaves a resumable failed run") found, _, err := env.Store.IdentityDiscoveryBacklogContext(context.Background(), source.ID) require.NoError(err, "IdentityDiscoveryBacklogContext") @@ -1583,11 +1651,15 @@ func TestRecoverExpiredHistoryMarksOnlyMissingSourceMetadata(t *testing.T) { type recoveryProfileSequenceAPI struct { *gmail.MockAPI - historyIDs []uint64 - calls int + historyIDs []uint64 + calls int + beforeFetch func(call int) } func (a *recoveryProfileSequenceAPI) GetProfile(context.Context) (*gmail.Profile, error) { + if a.beforeFetch != nil { + a.beforeFetch(a.calls) + } profile := *a.Profile if a.calls < len(a.historyIDs) { profile.HistoryID = a.historyIDs[a.calls] @@ -1596,6 +1668,51 @@ func (a *recoveryProfileSequenceAPI) GetProfile(context.Context) (*gmail.Profile return &profile, nil } +func TestFullRecoversCheckpointAfterSyncOwnerExit(t *testing.T) { + requirements := require.New(t) + checks := assert.New(t) + dbPath := filepath.Join(t.TempDir(), "owner-exit.db") + first, err := store.OpenForTest(dbPath) + requirements.NoError(err) + requirements.NoError(first.InitSchema()) + source, err := first.GetOrCreateSource("gmail", "owner-exit@example.com") + requirements.NoError(err) + _, err = first.CreateSyncOperation(source.ID, "owner-exit-operation") + requirements.NoError(err) + abandonedRun, err := first.StartSyncOperation(source.ID, "owner-exit-operation") + requirements.NoError(err) + fingerprint := New(gmail.NewMockAPI(), first, nil).fullSyncRequestFingerprint() + _, err = first.DB().Exec( + `UPDATE sync_runs SET request_fingerprint = ? WHERE id = ?`, + fingerprint, abandonedRun, + ) + requirements.NoError(err) + requirements.NoError(first.UpdateSyncCheckpoint(abandonedRun, &store.Checkpoint{ + PageToken: "page_1", + MessagesProcessed: 7, + MessagesAdded: 5, + })) + requirements.NoError(first.Close()) + + second, err := store.OpenForTest(dbPath) + requirements.NoError(err) + t.Cleanup(func() { _ = second.Close() }) + mock := gmail.NewMockAPI() + mock.Profile = &gmail.Profile{ + EmailAddress: source.Identifier, + MessagesTotal: 0, + HistoryID: 2000, + } + + summary, err := New(mock, second, nil).Full(t.Context(), source.Identifier) + requirements.NoError(err) + checks.True(summary.WasResumed) + checks.Equal(int64(7), summary.MessagesFound) + op, err := second.GetSyncOperation("owner-exit-operation") + requirements.NoError(err) + checks.Equal("failed", op.Status) +} + func TestRecoverExpiredHistoryConsumesChangesAfterSnapshotCursor(t *testing.T) { assert := assert.New(t) require := require.New(t) @@ -1626,6 +1743,50 @@ func TestRecoverExpiredHistoryConsumesChangesAfterSnapshotCursor(t *testing.T) { assert.Equal("2000", refreshed.SyncCursor.String, "persisted history cursor") } +func TestRecoverExpiredHistoryRetainsSourceOwnershipThroughCatchup(t *testing.T) { + requirements := require.New(t) + checks := assert.New(t) + env := newTestEnv(t) + seedMessages(env, 1, 1000, "present") + runFullSync(t, env) + source, err := env.Store.GetSourceByIdentifier(testEmail) + requirements.NoError(err) + competitor, err := store.OpenForTest(filepath.Join(env.TmpDir, "test.db")) + requirements.NoError(err) + t.Cleanup(func() { _ = competitor.Close() }) + + env.Mock.MessagePages = [][]string{{"present"}} + env.Mock.Profile.MessagesTotal = 1 + env.Mock.HistoryRecords = nil + env.Mock.HistoryID = 2000 + var probeErr error + var probeRunID int64 + api := &recoveryProfileSequenceAPI{ + MockAPI: env.Mock, + historyIDs: []uint64{1500, 2000}, + beforeFetch: func(call int) { + if call == 1 { + probeRunID, probeErr = competitor.StartSync(source.ID, "competing") + } + }, + } + options := DefaultOptions() + options.OperationID = "retained-ownership-operation" + _, err = env.Store.CreateSyncOperation(source.ID, options.OperationID) + requirements.NoError(err) + syncer := New(api, env.Store, options) + + _, err = syncer.RecoverExpiredHistory(t.Context(), source) + requirements.NoError(err) + requirements.ErrorIs(probeErr, store.ErrSyncAlreadyActive) + checks.Zero(probeRunID) + op, err := env.Store.GetSyncOperation(options.OperationID) + requirements.NoError(err) + checks.Equal("done", op.Status) + checks.True(op.FinishedAt.Valid) + requirements.Len(op.Runs, 2) +} + func TestRecoverExpiredHistoryRejectsPartialEnumerationOptions(t *testing.T) { tests := []struct { name string @@ -1705,9 +1866,8 @@ func TestRecoverExpiredHistoryDoesNotReconcileIncompleteSnapshot(t *testing.T) { assertDeletedFromSource(t, env.Store, "not-yet-enumerated", false) } -func TestRecoverExpiredHistoryDoesNotReuseUnmarkedFullCheckpoint(t *testing.T) { +func TestRecoverExpiredHistoryRejectsUnmarkedActiveSync(t *testing.T) { require := require.New(t) - assert := assert.New(t) env := newTestEnv(t) env.Mock.Profile.HistoryID = 12345 seedPagedMessages(env, 4) @@ -1726,19 +1886,16 @@ func TestRecoverExpiredHistoryDoesNotReuseUnmarkedFullCheckpoint(t *testing.T) { env.Mock.SnapshotListCalls = 0 summary, err := env.Syncer.RecoverExpiredHistory(env.Context, source) - require.NoError(err, "RecoverExpiredHistory") - assert.False(summary.WasResumed, "an ordinary full checkpoint has no pinned recovery cursor") - assert.Empty(summary.ResumedFromToken, "recovery restarts ordinary full enumeration") - assert.Equal(2, env.Mock.ListMessagesCalls, "recovery content enumeration starts at page zero") - assert.Equal(2, env.Mock.SnapshotListCalls, "presence snapshot starts at page zero") - assertDeletedFromSource(t, env.Store, "msg1", false) - assertDeletedFromSource(t, env.Store, "msg4", false) + require.ErrorIs(err, store.ErrSyncAlreadyActive) + require.Nil(summary) + require.Zero(env.Mock.ListMessagesCalls) + require.Zero(env.Mock.SnapshotListCalls) } func TestIncrementalWithHistoryRecoveryResumesPinnedCursorBeforeIncremental(t *testing.T) { require := require.New(t) assert := assert.New(t) - env, source, active := setupInterruptedHistoryRecoveryWithPrefixChange(t) + env, source, prior := setupInterruptedHistoryRecoveryWithPrefixChange(t) env.Syncer = New(env.Mock, env.Store, nil) var recoveryNotices []bool @@ -1748,7 +1905,7 @@ func TestIncrementalWithHistoryRecoveryResumesPinnedCursorBeforeIncremental(t *t require.NoError(err, "IncrementalWithHistoryRecovery") assert.Equal([]bool{true}, recoveryNotices, "retry announces the resumed recovery") assert.True(summary.WasResumed, "recovery uses its saved page checkpoint") - assert.Equal(active.ID, summary.SyncRunID, "retry does not supersede the recovery run") + assert.NotEqual(prior.ID, summary.SyncRunID, "retry creates a new recovery run") assertRawDataExists(t, env.Store, "arrived-before-resume") refreshed, err := env.Store.GetSourceByID(source.ID) require.NoError(err, "GetSourceByID") @@ -1758,13 +1915,13 @@ func TestIncrementalWithHistoryRecoveryResumesPinnedCursorBeforeIncremental(t *t func TestFullRoutesPinnedHistoryRecoveryThroughCatchup(t *testing.T) { require := require.New(t) assert := assert.New(t) - env, source, active := setupInterruptedHistoryRecoveryWithPrefixChange(t) + env, source, prior := setupInterruptedHistoryRecoveryWithPrefixChange(t) env.Syncer = New(env.Mock, env.Store, nil) summary, err := env.Syncer.Full(env.Context, testEmail) require.NoError(err, "Full") assert.True(summary.WasResumed, "full routes through the marked recovery") - assert.Equal(active.ID, summary.SyncRunID, "full does not reinterpret or supersede the recovery run") + assert.NotEqual(prior.ID, summary.SyncRunID, "resumed recovery uses a new run") assertRawDataExists(t, env.Store, "arrived-before-resume") refreshed, err := env.Store.GetSourceByID(source.ID) require.NoError(err, "GetSourceByID") @@ -1789,9 +1946,10 @@ func setupInterruptedHistoryRecoveryWithPrefixChange( env.Syncer = New(&cancelOnSecondListAPI{MockAPI: env.Mock}, env.Store, nil) _, err = env.Syncer.RecoverExpiredHistory(env.Context, source) require.ErrorIs(err, context.Canceled, "interrupt recovery after its first page") - active, err := env.Store.GetActiveSync(source.ID) - require.NoError(err, "GetActiveSync") - assert.Equal("15000", active.CursorAfter.String, "recovery pins its handoff cursor before enumeration") + prior, err := env.Store.GetLatestCheckpointedSync(source.ID) + require.NoError(err, "GetLatestCheckpointedSync") + assert.Equal(store.SyncStatusFailed, prior.Status, "the stopped recovery worker is not active") + assert.Equal("15000", prior.CursorAfter.String, "recovery pins its handoff cursor before enumeration") env.Mock.AddMessage("arrived-before-resume", testMIME(), []string{"INBOX"}) env.Mock.MessagePages = [][]string{ @@ -1801,7 +1959,7 @@ func setupInterruptedHistoryRecoveryWithPrefixChange( env.Mock.Profile.HistoryID = 20000 env.Mock.HistoryID = 20000 env.Mock.HistoryRecords = []gmail.HistoryRecord{historyAdded("arrived-before-resume")} - return env, source, active + return env, source, prior } func TestIncrementalSyncProfileError(t *testing.T) { @@ -2341,6 +2499,11 @@ func TestFullSyncResumeWithCursor(t *testing.T) { syncID, err := env.Store.StartSync(source.ID, "full") require.NoError(err, "StartSync") + _, err = env.Store.DB().Exec( + `UPDATE sync_runs SET request_fingerprint = ? WHERE id = ?`, + env.Syncer.fullSyncRequestFingerprint(), syncID, + ) + require.NoError(err, "record request fingerprint") checkpoint := &store.Checkpoint{ PageToken: "page_1", @@ -2348,6 +2511,7 @@ func TestFullSyncResumeWithCursor(t *testing.T) { MessagesAdded: 2, } require.NoError(env.Store.UpdateSyncCheckpoint(syncID, checkpoint), "UpdateSyncCheckpoint") + require.NoError(env.Store.FailSync(syncID, "worker stopped")) summary := runFullSync(t, env) @@ -2498,9 +2662,13 @@ func TestInitSyncState_NewSync(t *testing.T) { assert := assert.New(t) env := newTestEnv(t) source := env.CreateSource(t) + execution, err := env.Store.AcquireSyncExecutionContext(env.Context, source.ID) + require.NoError(t, err, "AcquireSyncExecutionContext") + t.Cleanup(func() { _ = execution.Release() }) - state, err := env.Syncer.initSyncState(source.ID) + state, err := env.Syncer.initSyncState(env.Context, source.ID, execution) require.NoError(t, err, "initSyncState") + t.Cleanup(func() { _ = env.Store.FailSync(state.syncID, "test complete") }) assert.False(state.wasResumed, "expected wasResumed = false for new sync") assert.Empty(state.pageToken, "pageToken") @@ -2514,9 +2682,14 @@ func TestInitSyncState_Resume(t *testing.T) { env := newTestEnv(t) source := env.CreateSource(t) - // Create an active sync with checkpoint + // Create a failed sync with a resumable checkpoint. syncID, err := env.Store.StartSync(source.ID, "full") require.NoError(err, "StartSync") + _, err = env.Store.DB().Exec( + `UPDATE sync_runs SET request_fingerprint = ? WHERE id = ?`, + env.Syncer.fullSyncRequestFingerprint(), syncID, + ) + require.NoError(err, "record request fingerprint") checkpoint := &store.Checkpoint{ PageToken: "resume_token_123", MessagesProcessed: 50, @@ -2525,13 +2698,18 @@ func TestInitSyncState_Resume(t *testing.T) { ErrorsCount: 2, } require.NoError(env.Store.UpdateSyncCheckpoint(syncID, checkpoint), "UpdateSyncCheckpoint") + require.NoError(env.Store.FailSync(syncID, "worker stopped")) + execution, err := env.Store.AcquireSyncExecutionContext(env.Context, source.ID) + require.NoError(err, "AcquireSyncExecutionContext") + t.Cleanup(func() { _ = execution.Release() }) - state, err := env.Syncer.initSyncState(source.ID) + state, err := env.Syncer.initSyncState(env.Context, source.ID, execution) require.NoError(err, "initSyncState") + t.Cleanup(func() { _ = env.Store.FailSync(state.syncID, "test complete") }) assert.True(state.wasResumed, "expected wasResumed = true") assert.Equal("resume_token_123", state.pageToken, "pageToken") - assert.Equal(syncID, state.syncID, "syncID") + assert.NotEqual(syncID, state.syncID, "resume starts a new run") assert.Equal(int64(50), state.checkpoint.MessagesProcessed, "MessagesProcessed") assert.Equal(int64(45), state.checkpoint.MessagesAdded, "MessagesAdded") } @@ -2554,12 +2732,9 @@ func TestInitSyncState_NoResumeOption(t *testing.T) { } require.NoError(env.Store.UpdateSyncCheckpoint(syncID, checkpoint), "UpdateSyncCheckpoint") - state, err := env.Syncer.initSyncState(source.ID) - require.NoError(err, "initSyncState") - - assert.False(state.wasResumed, "expected wasResumed = false with NoResume option") - assert.Empty(state.pageToken, "pageToken with NoResume") - assert.NotEqual(syncID, state.syncID, "expected new syncID, not the existing one") + state, err := env.Syncer.Full(env.Context, source.Identifier) + require.ErrorIs(err, store.ErrSyncAlreadyActive) + assert.Nil(state) } // Tests for processBatch diff --git a/internal/teams/importer_test.go b/internal/teams/importer_test.go index 41d27b3f6..17307ef88 100644 --- a/internal/teams/importer_test.go +++ b/internal/teams/importer_test.go @@ -32,8 +32,7 @@ func TestImporterScopesParticipantResolver(t *testing.T) { runID, err := st.StartSync(source.ID, sourceTypeTeams) requirements.NoError(err) scoped := NewImporter(st, nil).scopedToSync(source.ID, runID) - _, err = st.StartSync(source.ID, sourceTypeTeams) - requirements.NoError(err) + requirements.NoError(st.FailSync(runID, "worker stopped")) _, err = scoped.res.resolve(t.Context(), &Identity{ID: "user-a"}) requirements.ErrorIs(err, store.ErrSyncRunSuperseded) diff --git a/pkg/client/generated/client.go b/pkg/client/generated/client.go index a7c29c8bc..8605fcd9e 100644 --- a/pkg/client/generated/client.go +++ b/pkg/client/generated/client.go @@ -451,6 +451,14 @@ type ClientInterface interface { ImportMeeting(ctx context.Context, options *ImportMeetingRequestOptions, reqEditors ...runtime.RequestEditorFn) (*ImportMeetingResponseJSON, error) ImportMeetingWithResponse(ctx context.Context, options *ImportMeetingRequestOptions, reqEditors ...runtime.RequestEditorFn) (*ImportMeetingResp, error) + // CreateImportJob Start a bounded historical import + CreateImportJob(ctx context.Context, options *CreateImportJobRequestOptions, reqEditors ...runtime.RequestEditorFn) (*CreateImportJobResponse, error) + CreateImportJobWithResponse(ctx context.Context, options *CreateImportJobRequestOptions, reqEditors ...runtime.RequestEditorFn) (*CreateImportJobResp, error) + + // GetImportJob Get historical import status + GetImportJob(ctx context.Context, options *GetImportJobRequestOptions, reqEditors ...runtime.RequestEditorFn) (*GetImportJobResponse, error) + GetImportJobWithResponse(ctx context.Context, options *GetImportJobRequestOptions, reqEditors ...runtime.RequestEditorFn) (*GetImportJobResp, error) + // SearchIntegrationTasks Search tasks in the configured project SearchIntegrationTasks(ctx context.Context, options *SearchIntegrationTasksRequestOptions, reqEditors ...runtime.RequestEditorFn) (*SearchIntegrationTasksResponse, error) SearchIntegrationTasksWithResponse(ctx context.Context, options *SearchIntegrationTasksRequestOptions, reqEditors ...runtime.RequestEditorFn) (*SearchIntegrationTasksResp, error) @@ -7335,6 +7343,133 @@ func (c *Client) ImportMeeting(ctx context.Context, options *ImportMeetingReques return responseParser(ctx, resp) } +// CreateImportJob Start a bounded historical import +func (c *Client) CreateImportJob(ctx context.Context, options *CreateImportJobRequestOptions, reqEditors ...runtime.RequestEditorFn) (*CreateImportJobResponse, error) { + var err error + reqParams := runtime.RequestOptionsParameters{ + RequestURL: c.apiClient.GetBaseURL() + "/api/v1/imports", + Method: "POST", + Options: options, + ContentType: "application/json", + } + + req, err := c.apiClient.CreateRequest(ctx, reqParams, reqEditors...) + if err != nil { + return nil, fmt.Errorf("error creating request: %w", err) + } + + responseParser := func(ctx context.Context, resp *runtime.Response) (*CreateImportJobResponse, error) { + bodyBytes := resp.Content + if resp.StatusCode != 202 { + target := new(CreateImportJobErrorResponse) + // Handle empty error response body gracefully - skip unmarshal if no content + if len(bodyBytes) > 0 { + if err = json.Unmarshal(bodyBytes, target); err != nil { + return nil, &runtime.ResponseDecodeError{ + StatusCode: resp.StatusCode, + ContentType: resp.Headers.Get("Content-Type"), + ContentLength: len(bodyBytes), + TargetType: "CreateImportJobErrorResponse", + Body: bodyBytes, + Err: err, + } + } + } + // Return error with (possibly empty) target + if errTarget, ok := any(*target).(error); ok { + return nil, runtime.NewClientAPIError(errTarget, runtime.WithStatusCode(resp.StatusCode)) + } + return nil, runtime.NewClientAPIError(fmt.Errorf("API error (status %d): %v", resp.StatusCode, *target), + runtime.WithStatusCode(resp.StatusCode)) + } + target := new(CreateImportJobResponse) + // Handle empty response body gracefully + if len(bodyBytes) == 0 { + return target, nil + } + if err = json.Unmarshal(bodyBytes, target); err != nil { + return nil, &runtime.ResponseDecodeError{ + StatusCode: resp.StatusCode, + ContentType: resp.Headers.Get("Content-Type"), + ContentLength: len(bodyBytes), + TargetType: "CreateImportJobResponse", + Body: bodyBytes, + Err: err, + } + } + return target, nil + } + + resp, err := c.apiClient.ExecuteRequest(ctx, req, "/api/v1/imports") + if err != nil { + return nil, fmt.Errorf("error executing request: %w", err) + } + return responseParser(ctx, resp) +} + +// GetImportJob Get historical import status +func (c *Client) GetImportJob(ctx context.Context, options *GetImportJobRequestOptions, reqEditors ...runtime.RequestEditorFn) (*GetImportJobResponse, error) { + var err error + reqParams := runtime.RequestOptionsParameters{ + RequestURL: c.apiClient.GetBaseURL() + "/api/v1/imports/{job_id}", + Method: "GET", + Options: options, + } + + req, err := c.apiClient.CreateRequest(ctx, reqParams, reqEditors...) + if err != nil { + return nil, fmt.Errorf("error creating request: %w", err) + } + + responseParser := func(ctx context.Context, resp *runtime.Response) (*GetImportJobResponse, error) { + bodyBytes := resp.Content + if resp.StatusCode != 200 { + target := new(GetImportJobErrorResponse) + // Handle empty error response body gracefully - skip unmarshal if no content + if len(bodyBytes) > 0 { + if err = json.Unmarshal(bodyBytes, target); err != nil { + return nil, &runtime.ResponseDecodeError{ + StatusCode: resp.StatusCode, + ContentType: resp.Headers.Get("Content-Type"), + ContentLength: len(bodyBytes), + TargetType: "GetImportJobErrorResponse", + Body: bodyBytes, + Err: err, + } + } + } + // Return error with (possibly empty) target + if errTarget, ok := any(*target).(error); ok { + return nil, runtime.NewClientAPIError(errTarget, runtime.WithStatusCode(resp.StatusCode)) + } + return nil, runtime.NewClientAPIError(fmt.Errorf("API error (status %d): %v", resp.StatusCode, *target), + runtime.WithStatusCode(resp.StatusCode)) + } + target := new(GetImportJobResponse) + // Handle empty response body gracefully + if len(bodyBytes) == 0 { + return target, nil + } + if err = json.Unmarshal(bodyBytes, target); err != nil { + return nil, &runtime.ResponseDecodeError{ + StatusCode: resp.StatusCode, + ContentType: resp.Headers.Get("Content-Type"), + ContentLength: len(bodyBytes), + TargetType: "GetImportJobResponse", + Body: bodyBytes, + Err: err, + } + } + return target, nil + } + + resp, err := c.apiClient.ExecuteRequest(ctx, req, "/api/v1/imports/{job_id}") + if err != nil { + return nil, fmt.Errorf("error executing request: %w", err) + } + return responseParser(ctx, resp) +} + // SearchIntegrationTasks Search tasks in the configured project func (c *Client) SearchIntegrationTasks(ctx context.Context, options *SearchIntegrationTasksRequestOptions, reqEditors ...runtime.RequestEditorFn) (*SearchIntegrationTasksResponse, error) { var err error diff --git a/pkg/client/generated/client_options.go b/pkg/client/generated/client_options.go index a6227d288..1b4468543 100644 --- a/pkg/client/generated/client_options.go +++ b/pkg/client/generated/client_options.go @@ -4217,6 +4217,94 @@ func (o *ImportMeetingRequestOptions) GetHeader() (map[string]string, error) { return nil, nil } +// CreateImportJobRequestOptions is the options needed to make a request to CreateImportJob. +type CreateImportJobRequestOptions struct { + Body *CreateImportJobBody +} + +// Validate validates all the fields in the options. +// Use it if fields validation was not run. +func (o *CreateImportJobRequestOptions) Validate() error { + var errors runtime.ValidationErrors + + if o.Body != nil { + if v, ok := any(o.Body).(runtime.Validator); ok { + if err := v.Validate(); err != nil { + errors = errors.Append("Body", err) + } + } + } + if len(errors) == 0 { + return nil + } + + return errors +} + +// GetPathParams returns the path params as a map. +func (o *CreateImportJobRequestOptions) GetPathParams() (map[string]any, error) { + return nil, nil +} + +// GetQuery returns the query params as a map. +func (o *CreateImportJobRequestOptions) GetQuery() (map[string]any, error) { + return nil, nil +} + +// GetBody returns the payload in any type that can be marshalled to JSON by the client. +func (o *CreateImportJobRequestOptions) GetBody() any { + return o.Body +} + +// GetHeader returns the headers as a map. +func (o *CreateImportJobRequestOptions) GetHeader() (map[string]string, error) { + return nil, nil +} + +// GetImportJobRequestOptions is the options needed to make a request to GetImportJob. +type GetImportJobRequestOptions struct { + PathParams *GetImportJobPath +} + +// Validate validates all the fields in the options. +// Use it if fields validation was not run. +func (o *GetImportJobRequestOptions) Validate() error { + var errors runtime.ValidationErrors + + if o.PathParams != nil { + if v, ok := any(o.PathParams).(runtime.Validator); ok { + if err := v.Validate(); err != nil { + errors = errors.Append("PathParams", err) + } + } + } + if len(errors) == 0 { + return nil + } + + return errors +} + +// GetPathParams returns the path params as a map. +func (o *GetImportJobRequestOptions) GetPathParams() (map[string]any, error) { + return runtime.AsMap[any](o.PathParams) +} + +// GetQuery returns the query params as a map. +func (o *GetImportJobRequestOptions) GetQuery() (map[string]any, error) { + return nil, nil +} + +// GetBody returns the payload in any type that can be marshalled to JSON by the client. +func (o *GetImportJobRequestOptions) GetBody() any { + return nil +} + +// GetHeader returns the headers as a map. +func (o *GetImportJobRequestOptions) GetHeader() (map[string]string, error) { + return nil, nil +} + // SearchIntegrationTasksRequestOptions is the options needed to make a request to SearchIntegrationTasks. type SearchIntegrationTasksRequestOptions struct { Query *SearchIntegrationTasksQuery diff --git a/pkg/client/generated/client_with_response.go b/pkg/client/generated/client_with_response.go index f28580c7b..fe37aa014 100644 --- a/pkg/client/generated/client_with_response.go +++ b/pkg/client/generated/client_with_response.go @@ -9025,6 +9025,277 @@ func (c *Client) ImportMeetingWithResponse(ctx context.Context, options *ImportM } } +// CreateImportJob Start a bounded historical import +func (c *Client) CreateImportJobWithResponse(ctx context.Context, options *CreateImportJobRequestOptions, reqEditors ...runtime.RequestEditorFn) (*CreateImportJobResp, error) { + var err error + reqParams := runtime.RequestOptionsParameters{ + RequestURL: c.apiClient.GetBaseURL() + "/api/v1/imports", + Method: "POST", + Options: options, + ContentType: "application/json", + } + + req, err := c.apiClient.CreateRequest(ctx, reqParams, reqEditors...) + if err != nil { + return nil, fmt.Errorf("error creating request: %w", err) + } + + resp, err := c.apiClient.ExecuteRequest(ctx, req, "/api/v1/imports") + if err != nil { + return nil, fmt.Errorf("error executing request: %w", err) + } + + out := &CreateImportJobResp{ + HTTPResponse: resp.Raw, + Body: resp.Content, + StatusCode: resp.StatusCode, + } + + switch resp.StatusCode { + case 202: + out.JSON202 = new(CreateImportJobResponse) + bodyBytes := resp.Content + if len(bodyBytes) > 0 { + if err := json.Unmarshal(bodyBytes, out.JSON202); err != nil { + return out, &runtime.ResponseDecodeError{ + StatusCode: resp.StatusCode, + ContentType: resp.Headers.Get("Content-Type"), + ContentLength: len(bodyBytes), + TargetType: "CreateImportJobResponse", + Body: bodyBytes, + Err: err, + } + } + } + return out, nil + case 400: + out.JSON400 = new(CreateImportJobErrorResponse) + bodyBytes := resp.Content + if len(bodyBytes) > 0 { + if err := json.Unmarshal(bodyBytes, out.JSON400); err != nil { + return out, &runtime.ResponseDecodeError{ + StatusCode: resp.StatusCode, + ContentType: resp.Headers.Get("Content-Type"), + ContentLength: len(bodyBytes), + TargetType: "CreateImportJobErrorResponse", + Body: bodyBytes, + Err: err, + } + } + } + return out, runtime.NewClientAPIError(fmt.Errorf("API error (status %d)", resp.StatusCode), runtime.WithStatusCode(resp.StatusCode)) + case 401: + out.JSON401 = new(CreateImportJobErrorResponseJSON) + bodyBytes := resp.Content + if len(bodyBytes) > 0 { + if err := json.Unmarshal(bodyBytes, out.JSON401); err != nil { + return out, &runtime.ResponseDecodeError{ + StatusCode: resp.StatusCode, + ContentType: resp.Headers.Get("Content-Type"), + ContentLength: len(bodyBytes), + TargetType: "CreateImportJobErrorResponseJSON", + Body: bodyBytes, + Err: err, + } + } + } + return out, runtime.NewClientAPIError(fmt.Errorf("API error (status %d)", resp.StatusCode), runtime.WithStatusCode(resp.StatusCode)) + case 404: + out.JSON404 = new(CreateImportJobErrorResponseJSON404) + bodyBytes := resp.Content + if len(bodyBytes) > 0 { + if err := json.Unmarshal(bodyBytes, out.JSON404); err != nil { + return out, &runtime.ResponseDecodeError{ + StatusCode: resp.StatusCode, + ContentType: resp.Headers.Get("Content-Type"), + ContentLength: len(bodyBytes), + TargetType: "CreateImportJobErrorResponseJSON404", + Body: bodyBytes, + Err: err, + } + } + } + return out, runtime.NewClientAPIError(fmt.Errorf("API error (status %d)", resp.StatusCode), runtime.WithStatusCode(resp.StatusCode)) + case 409: + out.JSON409 = new(CreateImportJobErrorResponseJSON409) + bodyBytes := resp.Content + if len(bodyBytes) > 0 { + if err := json.Unmarshal(bodyBytes, out.JSON409); err != nil { + return out, &runtime.ResponseDecodeError{ + StatusCode: resp.StatusCode, + ContentType: resp.Headers.Get("Content-Type"), + ContentLength: len(bodyBytes), + TargetType: "CreateImportJobErrorResponseJSON409", + Body: bodyBytes, + Err: err, + } + } + } + return out, runtime.NewClientAPIError(fmt.Errorf("API error (status %d)", resp.StatusCode), runtime.WithStatusCode(resp.StatusCode)) + case 413: + out.JSON413 = new(CreateImportJobErrorResponseJSON413) + bodyBytes := resp.Content + if len(bodyBytes) > 0 { + if err := json.Unmarshal(bodyBytes, out.JSON413); err != nil { + return out, &runtime.ResponseDecodeError{ + StatusCode: resp.StatusCode, + ContentType: resp.Headers.Get("Content-Type"), + ContentLength: len(bodyBytes), + TargetType: "CreateImportJobErrorResponseJSON413", + Body: bodyBytes, + Err: err, + } + } + } + return out, runtime.NewClientAPIError(fmt.Errorf("API error (status %d)", resp.StatusCode), runtime.WithStatusCode(resp.StatusCode)) + case 415: + out.JSON415 = new(CreateImportJobErrorResponseJSON415) + bodyBytes := resp.Content + if len(bodyBytes) > 0 { + if err := json.Unmarshal(bodyBytes, out.JSON415); err != nil { + return out, &runtime.ResponseDecodeError{ + StatusCode: resp.StatusCode, + ContentType: resp.Headers.Get("Content-Type"), + ContentLength: len(bodyBytes), + TargetType: "CreateImportJobErrorResponseJSON415", + Body: bodyBytes, + Err: err, + } + } + } + return out, runtime.NewClientAPIError(fmt.Errorf("API error (status %d)", resp.StatusCode), runtime.WithStatusCode(resp.StatusCode)) + case 422: + out.JSON422 = new(CreateImportJobErrorResponseJSON422) + bodyBytes := resp.Content + if len(bodyBytes) > 0 { + if err := json.Unmarshal(bodyBytes, out.JSON422); err != nil { + return out, &runtime.ResponseDecodeError{ + StatusCode: resp.StatusCode, + ContentType: resp.Headers.Get("Content-Type"), + ContentLength: len(bodyBytes), + TargetType: "CreateImportJobErrorResponseJSON422", + Body: bodyBytes, + Err: err, + } + } + } + return out, runtime.NewClientAPIError(fmt.Errorf("API error (status %d)", resp.StatusCode), runtime.WithStatusCode(resp.StatusCode)) + case 500: + out.JSON500 = new(CreateImportJobErrorResponseJSON500) + bodyBytes := resp.Content + if len(bodyBytes) > 0 { + if err := json.Unmarshal(bodyBytes, out.JSON500); err != nil { + return out, &runtime.ResponseDecodeError{ + StatusCode: resp.StatusCode, + ContentType: resp.Headers.Get("Content-Type"), + ContentLength: len(bodyBytes), + TargetType: "CreateImportJobErrorResponseJSON500", + Body: bodyBytes, + Err: err, + } + } + } + return out, runtime.NewClientAPIError(fmt.Errorf("API error (status %d)", resp.StatusCode), runtime.WithStatusCode(resp.StatusCode)) + case 503: + out.JSON503 = new(CreateImportJobErrorResponseJSON503) + bodyBytes := resp.Content + if len(bodyBytes) > 0 { + if err := json.Unmarshal(bodyBytes, out.JSON503); err != nil { + return out, &runtime.ResponseDecodeError{ + StatusCode: resp.StatusCode, + ContentType: resp.Headers.Get("Content-Type"), + ContentLength: len(bodyBytes), + TargetType: "CreateImportJobErrorResponseJSON503", + Body: bodyBytes, + Err: err, + } + } + } + return out, runtime.NewClientAPIError(fmt.Errorf("API error (status %d)", resp.StatusCode), runtime.WithStatusCode(resp.StatusCode)) + default: + return out, runtime.NewClientAPIError(fmt.Errorf("unexpected status code: %d", resp.StatusCode), runtime.WithStatusCode(resp.StatusCode)) + } +} + +// GetImportJob Get historical import status +func (c *Client) GetImportJobWithResponse(ctx context.Context, options *GetImportJobRequestOptions, reqEditors ...runtime.RequestEditorFn) (*GetImportJobResp, error) { + var err error + reqParams := runtime.RequestOptionsParameters{ + RequestURL: c.apiClient.GetBaseURL() + "/api/v1/imports/{job_id}", + Method: "GET", + Options: options, + } + + req, err := c.apiClient.CreateRequest(ctx, reqParams, reqEditors...) + if err != nil { + return nil, fmt.Errorf("error creating request: %w", err) + } + + resp, err := c.apiClient.ExecuteRequest(ctx, req, "/api/v1/imports/{job_id}") + if err != nil { + return nil, fmt.Errorf("error executing request: %w", err) + } + + out := &GetImportJobResp{ + HTTPResponse: resp.Raw, + Body: resp.Content, + StatusCode: resp.StatusCode, + } + + switch resp.StatusCode { + case 200: + out.JSON200 = new(GetImportJobResponse) + bodyBytes := resp.Content + if len(bodyBytes) > 0 { + if err := json.Unmarshal(bodyBytes, out.JSON200); err != nil { + return out, &runtime.ResponseDecodeError{ + StatusCode: resp.StatusCode, + ContentType: resp.Headers.Get("Content-Type"), + ContentLength: len(bodyBytes), + TargetType: "GetImportJobResponse", + Body: bodyBytes, + Err: err, + } + } + } + return out, nil + case 401: + out.JSON401 = new(GetImportJobErrorResponse) + bodyBytes := resp.Content + if len(bodyBytes) > 0 { + if err := json.Unmarshal(bodyBytes, out.JSON401); err != nil { + return out, &runtime.ResponseDecodeError{ + StatusCode: resp.StatusCode, + ContentType: resp.Headers.Get("Content-Type"), + ContentLength: len(bodyBytes), + TargetType: "GetImportJobErrorResponse", + Body: bodyBytes, + Err: err, + } + } + } + return out, runtime.NewClientAPIError(fmt.Errorf("API error (status %d)", resp.StatusCode), runtime.WithStatusCode(resp.StatusCode)) + case 404: + out.JSON404 = new(GetImportJobErrorResponseJSON) + bodyBytes := resp.Content + if len(bodyBytes) > 0 { + if err := json.Unmarshal(bodyBytes, out.JSON404); err != nil { + return out, &runtime.ResponseDecodeError{ + StatusCode: resp.StatusCode, + ContentType: resp.Headers.Get("Content-Type"), + ContentLength: len(bodyBytes), + TargetType: "GetImportJobErrorResponseJSON", + Body: bodyBytes, + Err: err, + } + } + } + return out, runtime.NewClientAPIError(fmt.Errorf("API error (status %d)", resp.StatusCode), runtime.WithStatusCode(resp.StatusCode)) + default: + return out, runtime.NewClientAPIError(fmt.Errorf("unexpected status code: %d", resp.StatusCode), runtime.WithStatusCode(resp.StatusCode)) + } +} + // SearchIntegrationTasks Search tasks in the configured project func (c *Client) SearchIntegrationTasksWithResponse(ctx context.Context, options *SearchIntegrationTasksRequestOptions, reqEditors ...runtime.RequestEditorFn) (*SearchIntegrationTasksResp, error) { var err error diff --git a/pkg/client/generated/enums.go b/pkg/client/generated/enums.go index 7447e6c70..547565d58 100644 --- a/pkg/client/generated/enums.go +++ b/pkg/client/generated/enums.go @@ -572,6 +572,25 @@ func (i IdentitySearchSortField) Validate() error { } } +type ImportJobResponseStatus string + +const ( + ImportJobResponseStatusDone ImportJobResponseStatus = "done" + ImportJobResponseStatusFailed ImportJobResponseStatus = "failed" + ImportJobResponseStatusPending ImportJobResponseStatus = "pending" + ImportJobResponseStatusRunning ImportJobResponseStatus = "running" +) + +// Validate checks if the ImportJobResponseStatus value is valid +func (i ImportJobResponseStatus) Validate() error { + switch i { + case ImportJobResponseStatusDone, ImportJobResponseStatusFailed, ImportJobResponseStatusPending, ImportJobResponseStatusRunning: + return nil + default: + return runtime.NewValidationErrorsFromString("Enum", fmt.Sprintf("must be a valid ImportJobResponseStatus value, got: %v", i)) + } +} + type MeetingImportResponseStatus string const ( diff --git a/pkg/client/generated/paths.go b/pkg/client/generated/paths.go index 80460765f..4bee90302 100644 --- a/pkg/client/generated/paths.go +++ b/pkg/client/generated/paths.go @@ -259,6 +259,15 @@ type RejectIdentityMatchCandidatePath struct { ID int64 `json:"id"` } +type GetImportJobPath struct { + // JobID Historical import job ID + JobID string `json:"job_id" validate:"required"` +} + +func (g GetImportJobPath) Validate() error { + return runtime.ConvertValidatorError(typesValidator.Struct(g)) +} + type GetMessagePath struct { // ID Message ID ID int64 `json:"id"` diff --git a/pkg/client/generated/payloads.go b/pkg/client/generated/payloads.go index a1d642f54..66e2366f2 100644 --- a/pkg/client/generated/payloads.go +++ b/pkg/client/generated/payloads.go @@ -100,6 +100,8 @@ type UnlinkIdentityParticipantsBody = IdentityLinkRequest type ImportMeetingBody = MeetingImportRequest +type CreateImportJobBody = ImportJobRequest + type CreateOrLinkMessageTaskBody = TaskLinkMutationRequest type StartVisualAttachmentBuildBody = VisualBuildRequest diff --git a/pkg/client/generated/responses.go b/pkg/client/generated/responses.go index 5745bb263..015bf5b5d 100644 --- a/pkg/client/generated/responses.go +++ b/pkg/client/generated/responses.go @@ -1323,6 +1323,32 @@ type ImportMeetingResponseJSON = MeetingImportResponse type ImportMeetingErrorResponse = ErrorResponse +type CreateImportJobResponse = ImportJobResponse + +type CreateImportJobErrorResponse = ErrorResponse + +type CreateImportJobErrorResponseJSON = ErrorResponse + +type CreateImportJobErrorResponseJSON404 = ErrorResponse + +type CreateImportJobErrorResponseJSON409 = ErrorResponse + +type CreateImportJobErrorResponseJSON413 = ErrorResponse + +type CreateImportJobErrorResponseJSON415 = ErrorResponse + +type CreateImportJobErrorResponseJSON422 = ErrorResponse + +type CreateImportJobErrorResponseJSON500 = ErrorResponse + +type CreateImportJobErrorResponseJSON503 = ErrorResponse + +type GetImportJobResponse = ImportJobResponse + +type GetImportJobErrorResponse = ErrorResponse + +type GetImportJobErrorResponseJSON = ErrorResponse + type SearchIntegrationTasksResponse = TaskSearchResponse type SearchIntegrationTasksErrorResponse = ErrorResponse @@ -3686,6 +3712,31 @@ type ImportMeetingResp struct { JSON201 *ImportMeetingResponseJSON } +type CreateImportJobResp struct { + HTTPResponse *http.Response + Body []byte + StatusCode int + JSON202 *CreateImportJobResponse + JSON400 *CreateImportJobErrorResponse + JSON401 *CreateImportJobErrorResponseJSON + JSON404 *CreateImportJobErrorResponseJSON404 + JSON409 *CreateImportJobErrorResponseJSON409 + JSON413 *CreateImportJobErrorResponseJSON413 + JSON415 *CreateImportJobErrorResponseJSON415 + JSON422 *CreateImportJobErrorResponseJSON422 + JSON500 *CreateImportJobErrorResponseJSON500 + JSON503 *CreateImportJobErrorResponseJSON503 +} + +type GetImportJobResp struct { + HTTPResponse *http.Response + Body []byte + StatusCode int + JSON200 *GetImportJobResponse + JSON401 *GetImportJobErrorResponse + JSON404 *GetImportJobErrorResponseJSON +} + type SearchIntegrationTasksResp struct { HTTPResponse *http.Response Body []byte diff --git a/pkg/client/generated/types.go b/pkg/client/generated/types.go index 8b74f509b..937040e6c 100644 --- a/pkg/client/generated/types.go +++ b/pkg/client/generated/types.go @@ -4143,6 +4143,80 @@ func (i ImportEntry) Validate() error { return runtime.ConvertValidatorError(typesValidator.Struct(i)) } +type ImportJobRequest struct { + Account string `json:"account" validate:"required,min=1"` + After *string `json:"after,omitempty"` + Before *string `json:"before,omitempty"` + Limit *int64 `json:"limit,omitempty" validate:"omitempty,gte=0"` + Noresume *bool `json:"noresume,omitempty"` + Query *string `json:"query,omitempty"` +} + +func (i ImportJobRequest) Validate() error { + return runtime.ConvertValidatorError(typesValidator.Struct(i)) +} + +type ImportJobResponse struct { + Account string `json:"account" validate:"required"` + Added int64 `json:"added"` + CreatedAt time.Time `json:"created_at" validate:"required"` + ErrorData *string `json:"error,omitempty"` + FinishedAt *time.Time `json:"finished_at,omitempty" validate:"required"` + JobID string `json:"job_id" validate:"required"` + Processed int64 `json:"processed"` + Skipped int64 `json:"skipped"` + StartedAt *time.Time `json:"started_at,omitempty" validate:"required"` + Status ImportJobResponseStatus `json:"status" validate:"required"` + Summary *ImportJobSummary `json:"summary,omitempty"` +} + +func (i ImportJobResponse) Validate() error { + var errors runtime.ValidationErrors + if err := typesValidator.Var(i.Account, "required"); err != nil { + errors = errors.Append("Account", err) + } + if err := typesValidator.Var(i.CreatedAt, "required"); err != nil { + errors = errors.Append("CreatedAt", err) + } + if i.FinishedAt != nil { + if err := typesValidator.Var(i.FinishedAt, "required"); err != nil { + errors = errors.Append("FinishedAt", err) + } + } + if err := typesValidator.Var(i.JobID, "required"); err != nil { + errors = errors.Append("JobID", err) + } + if i.StartedAt != nil { + if err := typesValidator.Var(i.StartedAt, "required"); err != nil { + errors = errors.Append("StartedAt", err) + } + } + if v, ok := any(i.Status).(runtime.Validator); ok { + if err := v.Validate(); err != nil { + errors = errors.Append("Status", err) + } + } + if i.Summary != nil { + if v, ok := any(i.Summary).(runtime.Validator); ok { + if err := v.Validate(); err != nil { + errors = errors.Append("Summary", err) + } + } + } + if len(errors) == 0 { + return nil + } + return errors +} + +type ImportJobSummary struct { + Added int64 `json:"added"` + Errors int64 `json:"errors"` + Processed int64 `json:"processed"` + Skipped int64 `json:"skipped"` + Updated int64 `json:"updated"` +} + type ImportRequest struct { Account *string `json:"account,omitempty"` Apply *bool `json:"apply,omitempty"` diff --git a/pkg/client/openapi.yaml b/pkg/client/openapi.yaml index c7f4c206a..715fa6f46 100644 --- a/pkg/client/openapi.yaml +++ b/pkg/client/openapi.yaml @@ -4670,6 +4670,106 @@ components: required: - identifier type: object + ImportJobRequest: + additionalProperties: false + properties: + account: + minLength: 1 + type: string + after: + pattern: ^[0-9]{4}-[0-9]{2}-[0-9]{2}$ + type: string + before: + pattern: ^[0-9]{4}-[0-9]{2}-[0-9]{2}$ + type: string + limit: + format: int64 + minimum: 0 + type: integer + noresume: + type: boolean + query: + type: string + required: + - account + type: object + ImportJobResponse: + properties: + account: + type: string + added: + format: int64 + type: integer + created_at: + format: date-time + type: string + error: + type: string + finished_at: + format: date-time + nullable: true + type: string + job_id: + type: string + processed: + format: int64 + type: integer + skipped: + format: int64 + type: integer + started_at: + format: date-time + nullable: true + type: string + status: + enum: + - pending + - running + - done + - failed + type: string + x-enum-names: + - ImportJobResponseStatusPending + - ImportJobResponseStatusRunning + - ImportJobResponseStatusDone + - ImportJobResponseStatusFailed + summary: + $ref: "#/components/schemas/ImportJobSummary" + required: + - job_id + - account + - status + - processed + - added + - skipped + - created_at + - started_at + - finished_at + type: object + ImportJobSummary: + properties: + added: + format: int64 + type: integer + errors: + format: int64 + type: integer + processed: + format: int64 + type: integer + skipped: + format: int64 + type: integer + updated: + format: int64 + type: integer + required: + - processed + - added + - updated + - skipped + - errors + type: object ImportRequest: additionalProperties: false properties: @@ -10658,7 +10758,7 @@ components: type: apiKey info: title: msgvault API - version: 2.13.0 + version: 2.14.0 openapi: 3.0.3 paths: /api/ping: @@ -15725,6 +15825,127 @@ paths: summary: Import one meeting tags: - API + /api/v1/imports: + post: + operationId: createImportJob + requestBody: + content: + application/json: + schema: + $ref: "#/components/schemas/ImportJobRequest" + required: true + responses: + "202": + content: + application/json: + schema: + $ref: "#/components/schemas/ImportJobResponse" + description: Accepted + "400": + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorResponse" + description: Error + "401": + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorResponse" + description: Error + "404": + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorResponse" + description: Error + "409": + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorResponse" + description: Error + "413": + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorResponse" + description: Error + "415": + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorResponse" + description: Error + "422": + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorResponse" + description: Error + "500": + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorResponse" + description: Error + "503": + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorResponse" + description: Error + default: + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorResponse" + description: Error + security: + - apiKey: [] + summary: Start a bounded historical import + tags: + - API + /api/v1/imports/{job_id}: + get: + operationId: getImportJob + parameters: + - description: Historical import job ID + in: path + name: job_id + required: true + schema: + type: string + responses: + "200": + content: + application/json: + schema: + $ref: "#/components/schemas/ImportJobResponse" + description: OK + "401": + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorResponse" + description: Error + "404": + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorResponse" + description: Error + default: + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorResponse" + description: Error + security: + - apiKey: [] + summary: Get historical import status + tags: + - API /api/v1/integrations/tasks/search: get: operationId: searchIntegrationTasks diff --git a/web/src/lib/api/generated/schema.d.ts b/web/src/lib/api/generated/schema.d.ts index 45f27cb4e..ac2bafe4b 100644 --- a/web/src/lib/api/generated/schema.d.ts +++ b/web/src/lib/api/generated/schema.d.ts @@ -1532,6 +1532,40 @@ export interface paths { patch?: never; trace?: never; }; + "/api/v1/imports": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + /** Start a bounded historical import */ + post: operations["createImportJob"]; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/api/v1/imports/{job_id}": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + /** Get historical import status */ + get: operations["getImportJob"]; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/api/v1/integrations/tasks/search": { parameters: { query?: never; @@ -5308,6 +5342,51 @@ export interface components { identifier: string; state?: string; }; + ImportJobRequest: { + account: string; + after?: string; + before?: string; + /** Format: int64 */ + limit?: number; + noresume?: boolean; + query?: string; + }; + ImportJobResponse: { + account: string; + /** Format: int64 */ + added: number; + /** Format: date-time */ + created_at: string; + error?: string; + /** Format: date-time */ + finished_at: string | null; + job_id: string; + /** Format: int64 */ + processed: number; + /** Format: int64 */ + skipped: number; + /** Format: date-time */ + started_at: string | null; + /** @enum {string} */ + status: "pending" | "running" | "done" | "failed"; + summary?: components["schemas"]["ImportJobSummary"]; + } & { + [key: string]: unknown; + }; + ImportJobSummary: { + /** Format: int64 */ + added: number; + /** Format: int64 */ + errors: number; + /** Format: int64 */ + processed: number; + /** Format: int64 */ + skipped: number; + /** Format: int64 */ + updated: number; + } & { + [key: string]: unknown; + }; ImportRequest: { account?: string; apply?: boolean; @@ -13833,6 +13912,170 @@ export interface operations { }; }; }; + createImportJob: { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + requestBody: { + content: { + "application/json": components["schemas"]["ImportJobRequest"]; + }; + }; + responses: { + /** @description Accepted */ + 202: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["ImportJobResponse"]; + }; + }; + /** @description Error */ + 400: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["ErrorResponse"]; + }; + }; + /** @description Error */ + 401: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["ErrorResponse"]; + }; + }; + /** @description Error */ + 404: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["ErrorResponse"]; + }; + }; + /** @description Error */ + 409: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["ErrorResponse"]; + }; + }; + /** @description Error */ + 413: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["ErrorResponse"]; + }; + }; + /** @description Error */ + 415: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["ErrorResponse"]; + }; + }; + /** @description Error */ + 422: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["ErrorResponse"]; + }; + }; + /** @description Error */ + 500: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["ErrorResponse"]; + }; + }; + /** @description Error */ + 503: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["ErrorResponse"]; + }; + }; + /** @description Error */ + default: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["ErrorResponse"]; + }; + }; + }; + }; + getImportJob: { + parameters: { + query?: never; + header?: never; + path: { + /** @description Historical import job ID */ + job_id: string; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["ImportJobResponse"]; + }; + }; + /** @description Error */ + 401: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["ErrorResponse"]; + }; + }; + /** @description Error */ + 404: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["ErrorResponse"]; + }; + }; + /** @description Error */ + default: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["ErrorResponse"]; + }; + }; + }; + }; searchIntegrationTasks: { parameters: { query: { From 6ddffae05a0080d63236f3dbabf9ac5f0a7ff978 Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Wed, 2 Sep 2026 08:33:19 -0500 Subject: [PATCH 2/6] fix(api): finalize historical imports after cache refresh Historical imports could report completion before sync-full finished its analytics cache refresh. A later cache failure could then replace that result, and status polling could mistake the refresh interval for an abandoned worker. Let the daemon wrapper publish the first terminal result after the complete command returns. Recover unfinished work only when a new daemon starts. Reject IMAP queries before job creation because the IMAP command cannot apply them. Generated with Codex Co-authored-by: Codex --- api/openapi.yaml | 1 + cmd/msgvault/cmd/serve.go | 8 ++--- cmd/msgvault/cmd/serve_test.go | 29 ++++++++++++++++ internal/api/import_jobs.go | 6 +++- internal/api/import_jobs_test.go | 19 ++++++++++ internal/importer/eml_import_test.go | 1 + internal/store/sync.go | 52 +++++++++++++++------------- internal/store/sync_test.go | 40 +++++++++++++++++++-- internal/sync/sync.go | 12 ------- internal/sync/sync_test.go | 8 ++--- pkg/client/generated/types.go | 4 ++- pkg/client/openapi.yaml | 1 + 12 files changed, 132 insertions(+), 49 deletions(-) diff --git a/api/openapi.yaml b/api/openapi.yaml index c06672184..ac4d1e726 100644 --- a/api/openapi.yaml +++ b/api/openapi.yaml @@ -4848,6 +4848,7 @@ components: noresume: type: boolean query: + description: Gmail search query; not supported for IMAP sources type: string required: - account diff --git a/cmd/msgvault/cmd/serve.go b/cmd/msgvault/cmd/serve.go index d3c9cddef..63d1faf10 100644 --- a/cmd/msgvault/cmd/serve.go +++ b/cmd/msgvault/cmd/serve.go @@ -242,13 +242,13 @@ func runServe(cmd *cobra.Command, args []string) error { if err := s.InitSchemaContext(cmd.Context()); err != nil { return fmt.Errorf("init schema: %w", err) } - failedPendingImports, err := s.FailPendingSyncOperationsContext(cmd.Context()) + failedUnfinishedImports, err := s.FailUnfinishedSyncOperationsContext(cmd.Context()) if err != nil { - return fmt.Errorf("recover pending historical imports: %w", err) + return fmt.Errorf("recover unfinished historical imports: %w", err) } - if failedPendingImports > 0 { + if failedUnfinishedImports > 0 { logger.Warn("marked historical imports abandoned by the previous daemon as failed", - "count", failedPendingImports) + "count", failedUnfinishedImports) } logger.Info("daemon startup step complete", "step", "init_archive_schema") // Legacy [identity] migration is deferred to the first scheduled sync's diff --git a/cmd/msgvault/cmd/serve_test.go b/cmd/msgvault/cmd/serve_test.go index 68e6d6a4c..55fc235e7 100644 --- a/cmd/msgvault/cmd/serve_test.go +++ b/cmd/msgvault/cmd/serve_test.go @@ -1648,6 +1648,35 @@ func TestStoreAPIAdapterCanceledSyncOperationIsFailed(t *testing.T) { assert.Empty(op.Runs) } +func TestStoreAPIAdapterFinalizesSyncOperationAfterRunnerReturns(t *testing.T) { + require := require.New(t) + assert := assert.New(t) + f := storetest.New(t) + _, err := f.Store.CreateSyncOperation(f.Source.ID, "operation-1") + require.NoError(err) + adapter := &storeAPIAdapter{store: f.Store} + + err = adapter.runCLISyncOperationWithRunner( + t.Context(), + api.CLISyncRequest{Full: true, OperationID: "operation-1"}, + nil, + func(context.Context, []string, func(string, string) error) error { + runID, err := f.Store.StartSyncOperation(f.Source.ID, "operation-1") + require.NoError(err) + require.NoError(f.Store.CompleteSync(runID, "cursor")) + op, err := f.Store.GetSyncOperation("operation-1") + require.NoError(err) + assert.Equal("running", op.Status) + return nil + }, + ) + + require.NoError(err) + op, err := f.Store.GetSyncOperation("operation-1") + require.NoError(err) + assert.Equal("done", op.Status) +} + func TestStoreAPIAdapterRunCLICommandPacksOnlyAllowlistedSuccess(t *testing.T) { tests := []struct { name string diff --git a/internal/api/import_jobs.go b/internal/api/import_jobs.go index 0e5980a6e..824ea1b26 100644 --- a/internal/api/import_jobs.go +++ b/internal/api/import_jobs.go @@ -30,7 +30,7 @@ type ImportJobRequest struct { After string `json:"after,omitempty" pattern:"^[0-9]{4}-[0-9]{2}-[0-9]{2}$"` Before string `json:"before,omitempty" pattern:"^[0-9]{4}-[0-9]{2}-[0-9]{2}$"` Limit int `json:"limit,omitempty" minimum:"0"` - Query string `json:"query,omitempty"` + Query string `json:"query,omitempty" doc:"Gmail search query; not supported for IMAP sources"` NoResume bool `json:"noresume,omitempty"` } @@ -111,6 +111,10 @@ func (s *Server) handleCreateImportJob(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusNotFound, "not_found", "Import account not found") return } + if source.SourceType == "imap" && req.Query != "" { + writeError(w, http.StatusUnprocessableEntity, "validation_failed", "query is not supported for IMAP imports") + return + } release, acquired := func() (func(), bool) { if s.operationGate == nil { diff --git a/internal/api/import_jobs_test.go b/internal/api/import_jobs_test.go index a564fbbc9..9fa238d22 100644 --- a/internal/api/import_jobs_test.go +++ b/internal/api/import_jobs_test.go @@ -265,6 +265,25 @@ func TestImportJobRejectsAccountWithActiveSync(t *testing.T) { assert.Empty(t, st.started) } +func TestImportJobRejectsQueryForIMAPSource(t *testing.T) { + st := newImportJobTestStore() + st.sources[42].SourceType = "imap" + srv := NewServer(&config.Config{}, st, nil, testLogger()) + req := httptest.NewRequest( + http.MethodPost, + "/api/v1/imports", + strings.NewReader(`{"account":"archive@example.com","query":"from:alice@example.com"}`), + ) + req.Header.Set("Content-Type", applicationJSONMediaType) + resp := httptest.NewRecorder() + + srv.Router().ServeHTTP(resp, req) + + assert.Equal(t, http.StatusUnprocessableEntity, resp.Code, resp.Body.String()) + assert.Empty(t, st.operations) + assert.Empty(t, st.entered) +} + func TestImportJobCreationHasNoOrdinaryRequestDeadline(t *testing.T) { srv := NewServer(&config.Config{}, nil, nil, testLogger()) t.Cleanup(func() { require.NoError(t, srv.Shutdown(context.Background())) }) diff --git a/internal/importer/eml_import_test.go b/internal/importer/eml_import_test.go index 2741017db..41947e3ae 100644 --- a/internal/importer/eml_import_test.go +++ b/internal/importer/eml_import_test.go @@ -217,6 +217,7 @@ func TestImportEMLDirIgnoresIncompatibleCheckpoint(t *testing.T) { require.NoError(st.UpdateSyncCheckpoint(foreignSyncID, &store.Checkpoint{ PageToken: `{"file":"/tmp/archive.mbox","offset":1024}`, })) + require.NoError(st.FailSync(foreignSyncID, "interrupted foreign import")) summary, err := ImportEMLDir(t.Context(), st, root, EMLImportOptions{ Identifier: "mixed-imports@example.test", diff --git a/internal/store/sync.go b/internal/store/sync.go index 04aee2814..36560b6cf 100644 --- a/internal/store/sync.go +++ b/internal/store/sync.go @@ -536,7 +536,11 @@ func (s *Store) recoverAbandonedSyncSourceQueries( if _, err := q.ExecContext(ctx, s.Rebind(fmt.Sprintf(` UPDATE sync_operations SET status = 'failed', finished_at = %s - WHERE source_id = ? AND status = 'running'`, now)), sourceID); err != nil { + WHERE source_id = ? AND status = 'running' + AND EXISTS ( + SELECT 1 FROM sync_runs + WHERE operation_id = sync_operations.id AND status = 'running' + )`, now)), sourceID); err != nil { return fmt.Errorf("fail abandoned sync operation: %w", err) } if _, err := q.ExecContext(ctx, s.Rebind(fmt.Sprintf(` @@ -585,38 +589,27 @@ func (s *Store) CreateSyncOperation(sourceID int64, operationID string) (*SyncOp }, nil } -// FailPendingSyncOperationsContext marks operations whose daemon exited before -// their worker created a sync run. Callers must hold exclusive daemon ownership -// so a live worker cannot still be preparing the operation. -func (s *Store) FailPendingSyncOperationsContext(ctx context.Context) (int64, error) { +// FailUnfinishedSyncOperationsContext marks operations owned by an earlier +// daemon process. Callers must hold exclusive daemon ownership so no live +// worker can still be running or preparing an operation. +func (s *Store) FailUnfinishedSyncOperationsContext(ctx context.Context) (int64, error) { result, err := s.db.ExecContext(ctx, fmt.Sprintf(` UPDATE sync_operations SET status = 'failed', finished_at = %s - WHERE status = 'pending' + WHERE status IN ('pending', 'running') `, s.dialect.Now())) if err != nil { - return 0, fmt.Errorf("fail pending sync operations: %w", err) + return 0, fmt.Errorf("fail unfinished sync operations: %w", err) } failed, err := result.RowsAffected() if err != nil { - return 0, fmt.Errorf("fail pending sync operations: rows affected: %w", err) + return 0, fmt.Errorf("fail unfinished sync operations: rows affected: %w", err) } return failed, nil } // GetSyncOperation returns every sync run attributed to operationID. func (s *Store) GetSyncOperation(operationID string) (*SyncOperation, error) { - op, err := s.getSyncOperation(operationID) - if err != nil || op.Status != "running" || len(op.Runs) == 0 { - return op, err - } - recovered, err := s.recoverSyncSourceIfUnowned(context.Background(), op.Runs[0].SourceID) - if err != nil { - return nil, fmt.Errorf("recover sync operation %q: %w", operationID, err) - } - if !recovered { - return op, nil - } return s.getSyncOperation(operationID) } @@ -686,7 +679,8 @@ func (s *Store) recoverSyncSourceIfUnowned(ctx context.Context, sourceID int64) return true, execution.Release() } -// FinishSyncOperation marks every phase of an operation with its final status. +// FinishSyncOperation records the first terminal result for an operation. +// Repeated finalization is idempotent and cannot replace that result. func (s *Store) FinishSyncOperation(operationID, status string) error { if status != "done" && status != "failed" { return fmt.Errorf("invalid sync operation status %q", status) @@ -694,7 +688,7 @@ func (s *Store) FinishSyncOperation(operationID, status string) error { result, err := s.db.Exec(fmt.Sprintf(` UPDATE sync_operations SET status = ?, finished_at = %s - WHERE id = ? + WHERE id = ? AND status IN ('pending', 'running') `, s.dialect.Now()), status, operationID) if err != nil { return err @@ -703,10 +697,20 @@ func (s *Store) FinishSyncOperation(operationID, status string) error { if err != nil { return fmt.Errorf("finish sync operation %q: rows affected: %w", operationID, err) } - if updated != 1 { - return fmt.Errorf("finish sync operation %q: %w", operationID, ErrSyncRunNotFound) + if updated == 1 { + return nil } - return nil + var currentStatus string + if err := s.db.QueryRow(`SELECT status FROM sync_operations WHERE id = ?`, operationID).Scan(¤tStatus); err != nil { + if errors.Is(err, sql.ErrNoRows) { + return fmt.Errorf("finish sync operation %q: %w", operationID, ErrSyncRunNotFound) + } + return fmt.Errorf("read finished sync operation %q: %w", operationID, err) + } + if currentStatus == "done" || currentStatus == SyncStatusFailed { + return nil + } + return fmt.Errorf("finish sync operation %q: unexpected status %q", operationID, currentStatus) } // UpdateSyncCheckpoint saves progress for resumption. diff --git a/internal/store/sync_test.go b/internal/store/sync_test.go index 28ec7ac71..c177f85bf 100644 --- a/internal/store/sync_test.go +++ b/internal/store/sync_test.go @@ -251,7 +251,7 @@ func TestStore_SyncExecutionRetainsOwnershipAcrossRuns(t *testing.T) { requirements.NoError(second.FailSync(nextRun, "test complete")) } -func TestStore_GetSyncOperationRecoversTerminalRunWhoseOwnerExited(t *testing.T) { +func TestStore_UnfinishedSyncOperationIsRecoveredAtDaemonStartup(t *testing.T) { requirements := require.New(t) st := testutil.NewTestStore(t) source, err := st.GetOrCreateSource("gmail", "operation-recovery@example.com") @@ -267,6 +267,14 @@ func TestStore_GetSyncOperationRecoversTerminalRunWhoseOwnerExited(t *testing.T) op, err := st.GetSyncOperation("abandoned-terminal-operation") requirements.NoError(err) + requirements.Equal("running", op.Status) + requirements.False(op.FinishedAt.Valid) + + failed, err := st.FailUnfinishedSyncOperationsContext(t.Context()) + requirements.NoError(err) + requirements.Equal(int64(1), failed) + op, err = st.GetSyncOperation("abandoned-terminal-operation") + requirements.NoError(err) requirements.Equal("failed", op.Status) requirements.True(op.FinishedAt.Valid) requirements.Len(op.Runs, 1) @@ -673,14 +681,40 @@ func TestStore_SyncOperationGroupsRunsAndPublishesFinalState(t *testing.T) { assert.Equal(secondID, op.Runs[1].ID) } -func TestStore_FailPendingSyncOperations(t *testing.T) { +func TestStore_FinishSyncOperationPreservesFirstTerminalStatus(t *testing.T) { + f := storetest.New(t) + + for _, first := range []string{"done", store.SyncStatusFailed} { + t.Run(first, func(t *testing.T) { + requirements := require.New(t) + checks := assert.New(t) + operationID := "terminal-" + first + _, err := f.Store.CreateSyncOperation(f.Source.ID, operationID) + requirements.NoError(err) + requirements.NoError(f.Store.FinishSyncOperation(operationID, first)) + + second := "done" + if first == "done" { + second = store.SyncStatusFailed + } + requirements.NoError(f.Store.FinishSyncOperation(operationID, second)) + + op, err := f.Store.GetSyncOperation(operationID) + requirements.NoError(err) + checks.Equal(first, op.Status) + checks.True(op.FinishedAt.Valid) + }) + } +} + +func TestStore_FailUnfinishedSyncOperations(t *testing.T) { require := require.New(t) assert := assert.New(t) f := storetest.New(t) _, err := f.Store.CreateSyncOperation(f.Source.ID, "orphaned-operation") require.NoError(err) - failed, err := f.Store.FailPendingSyncOperationsContext(t.Context()) + failed, err := f.Store.FailUnfinishedSyncOperationsContext(t.Context()) require.NoError(err) assert.Equal(int64(1), failed) diff --git a/internal/sync/sync.go b/internal/sync/sync.go index 6c40cb23b..bc37d09f1 100644 --- a/internal/sync/sync.go +++ b/internal/sync/sync.go @@ -990,21 +990,9 @@ func (s *Syncer) runWithSyncExecution( } defer func() { if recovered := recover(); recovered != nil { - if s.opts.OperationID != "" { - _ = s.store.FinishSyncOperation(s.opts.OperationID, "failed") - } _ = execution.Release() panic(recovered) } - if s.opts.OperationID != "" { - status := "done" - if err != nil { - status = "failed" - } - if finishErr := s.store.FinishSyncOperation(s.opts.OperationID, status); finishErr != nil { - err = errors.Join(err, fmt.Errorf("finish sync operation: %w", finishErr)) - } - } if releaseErr := execution.Release(); releaseErr != nil { err = errors.Join(err, releaseErr) } diff --git a/internal/sync/sync_test.go b/internal/sync/sync_test.go index 2fa6228c4..9d154ebed 100644 --- a/internal/sync/sync_test.go +++ b/internal/sync/sync_test.go @@ -768,7 +768,7 @@ func TestBoundedGmailFullSyncPreservesIncrementalCursor(t *testing.T) { checks.Equal("2000", run.CursorAfter.String) } -func TestFullSyncFinalizerRetainsSourceOwnership(t *testing.T) { +func TestFullSyncLeavesOperationFinalizationToCaller(t *testing.T) { requirements := require.New(t) checks := assert.New(t) env := newTestEnv(t) @@ -798,7 +798,7 @@ func TestFullSyncFinalizerRetainsSourceOwnership(t *testing.T) { op, err := env.Store.GetSyncOperation(options.OperationID) requirements.NoError(err) - checks.Equal("done", op.Status) + checks.Equal("running", op.Status) } func TestFullSyncAcknowledgesOnlySafelyHandledMessages(t *testing.T) { @@ -1782,8 +1782,8 @@ func TestRecoverExpiredHistoryRetainsSourceOwnershipThroughCatchup(t *testing.T) checks.Zero(probeRunID) op, err := env.Store.GetSyncOperation(options.OperationID) requirements.NoError(err) - checks.Equal("done", op.Status) - checks.True(op.FinishedAt.Valid) + checks.Equal("running", op.Status) + checks.False(op.FinishedAt.Valid) requirements.Len(op.Runs, 2) } diff --git a/pkg/client/generated/types.go b/pkg/client/generated/types.go index 937040e6c..3dc51b46b 100644 --- a/pkg/client/generated/types.go +++ b/pkg/client/generated/types.go @@ -4149,7 +4149,9 @@ type ImportJobRequest struct { Before *string `json:"before,omitempty"` Limit *int64 `json:"limit,omitempty" validate:"omitempty,gte=0"` Noresume *bool `json:"noresume,omitempty"` - Query *string `json:"query,omitempty"` + + // Query Gmail search query; not supported for IMAP sources + Query *string `json:"query,omitempty"` } func (i ImportJobRequest) Validate() error { diff --git a/pkg/client/openapi.yaml b/pkg/client/openapi.yaml index 715fa6f46..f9c3676f9 100644 --- a/pkg/client/openapi.yaml +++ b/pkg/client/openapi.yaml @@ -4689,6 +4689,7 @@ components: noresume: type: boolean query: + description: Gmail search query; not supported for IMAP sources type: string required: - account From 44e4ee03535f8a3d4e02c3a186512196afdfb48f Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Wed, 2 Sep 2026 11:31:41 -0500 Subject: [PATCH 3/6] fix(sync): enforce import ownership across backends EML resume could attach to a live sync generation. Two workers could then change the same counters, checkpoint, and terminal status. SQLite file URI opens also derived execution locks from the URI instead of the backing file. Equivalent archive paths therefore did not share one ownership boundary. Resolve locks from the filesystem path saved at open, and recover EML checkpoints only after source ownership is acquired. Include operation rows in PostgreSQL source maintenance locks and refresh the generated web contract. Generated with Codex Co-authored-by: Codex --- internal/importer/eml_import.go | 21 +++++++----- internal/importer/eml_import_test.go | 47 +++++++++++++++++++++++++++ internal/store/dialect_pg.go | 1 + internal/store/store.go | 33 ++++++++++--------- internal/store/sync.go | 11 ++++--- internal/store/sync_execution_lock.go | 5 +-- internal/store/sync_test.go | 30 +++++++++++++++++ web/src/lib/api/generated/schema.d.ts | 1 + 8 files changed, 118 insertions(+), 31 deletions(-) diff --git a/internal/importer/eml_import.go b/internal/importer/eml_import.go index dcb68678a..10041bb7b 100644 --- a/internal/importer/eml_import.go +++ b/internal/importer/eml_import.go @@ -68,7 +68,7 @@ func ImportEMLDir( st *store.Store, root string, opts EMLImportOptions, -) (*EMLImportSummary, error) { +) (retSummary *EMLImportSummary, retErr error) { if opts.Identifier == "" { return nil, errors.New("identifier is required") } @@ -106,6 +106,14 @@ func ImportEMLDir( return nil, fmt.Errorf("get or create EML source: %w", err) } summary.SourceID = source.ID + ownershipCtx := context.WithoutCancel(ctx) + execution, err := st.AcquireSyncExecutionContext(ownershipCtx, source.ID) + if err != nil { + return nil, fmt.Errorf("acquire sync execution: %w", err) + } + defer func() { + retErr = errors.Join(retErr, execution.Release()) + }() var ( syncID int64 @@ -134,9 +142,6 @@ func ImportEMLDir( if saved.MailboxPath != "" && mailboxes[saved.MailboxIndex].Path != saved.MailboxPath { return nil, fmt.Errorf("EML mailbox tree changed at checkpoint index %d", saved.MailboxIndex) } - if resumable.Status == store.SyncStatusRunning { - syncID = resumable.ID - } checkpoint.MessagesProcessed = resumable.MessagesProcessed checkpoint.MessagesAdded = resumable.MessagesAdded checkpoint.MessagesUpdated = resumable.MessagesUpdated @@ -148,11 +153,9 @@ func ImportEMLDir( summary.WasResumed = true } } - if syncID == 0 { - syncID, err = st.StartSync(source.ID, "import-eml") - if err != nil { - return nil, fmt.Errorf("start EML import: %w", err) - } + syncID, err = execution.StartSyncContext(ownershipCtx, "import-eml", "") + if err != nil { + return nil, fmt.Errorf("start EML import: %w", err) } st = st.ScopedToSync(source.ID, syncID) diff --git a/internal/importer/eml_import_test.go b/internal/importer/eml_import_test.go index 41947e3ae..1514c72b1 100644 --- a/internal/importer/eml_import_test.go +++ b/internal/importer/eml_import_test.go @@ -147,6 +147,9 @@ func TestImportEMLDirResumesAfterInterruptedFileBoundary(t *testing.T) { require.ErrorIs(err, context.Canceled) require.NotNil(first) assert.Equal(int64(1), first.MessagesAdded) + interrupted, err := st.GetLatestSync(first.SourceID) + require.NoError(err) + assert.Equal(store.SyncStatusRunning, interrupted.Status) resumed, err := ImportEMLDir(t.Context(), st, root, EMLImportOptions{ Identifier: "resume@example.test", @@ -157,6 +160,15 @@ func TestImportEMLDirResumesAfterInterruptedFileBoundary(t *testing.T) { assert.Equal(int64(1), resumed.MessagesAdded) assert.Equal(int64(1), resumed.MessagesSkipped) assert.Equal(int64(2), resumed.MessagesProcessed) + completed, err := st.GetLatestSync(resumed.SourceID) + require.NoError(err) + assert.NotEqual(interrupted.ID, completed.ID, + "a resumed import must use a new sync generation") + var interruptedStatus string + require.NoError(st.DB().QueryRow( + `SELECT status FROM sync_runs WHERE id = ?`, interrupted.ID, + ).Scan(&interruptedStatus)) + assert.Equal(store.SyncStatusFailed, interruptedStatus) var messageCount int require.NoError(st.DB().QueryRow(`SELECT COUNT(*) FROM messages`).Scan(&messageCount)) @@ -201,6 +213,38 @@ func TestImportEMLDirResumesFailedCheckpoint(t *testing.T) { assert.Equal(store.SyncStatusCompleted, latest.Status) } +func TestImportEMLDirRejectsConcurrentSourceExecution(t *testing.T) { + requirements := require.New(t) + checks := assert.New(t) + st, tmp := openTestStore(t) + root := filepath.Join(tmp, "MailMate") + mailbox := filepath.Join(root, "Inbox.mailbox") + requirements.NoError(os.MkdirAll(mailbox, 0o700)) + writeTestEML(t, filepath.Join(mailbox, "1.eml"), "One") + absRoot, err := filepath.Abs(root) + requirements.NoError(err) + + source, err := st.GetOrCreateSource("eml", "concurrent@example.test") + requirements.NoError(err) + execution, err := st.AcquireSyncExecutionContext(t.Context(), source.ID) + requirements.NoError(err) + t.Cleanup(func() { _ = execution.Release() }) + runID, err := execution.StartSyncContext(t.Context(), "import-eml", "") + requirements.NoError(err) + requirements.NoError(saveEMLCheckpoint( + st, runID, absRoot, 0, mailbox, "", &store.Checkpoint{}, + )) + + _, err = ImportEMLDir(t.Context(), st, root, EMLImportOptions{ + Identifier: "concurrent@example.test", + }) + requirements.ErrorIs(err, store.ErrSyncAlreadyActive) + active, err := st.GetActiveSync(source.ID) + requirements.NoError(err) + checks.Equal(runID, active.ID) + requirements.NoError(st.FailSync(runID, "test complete")) +} + func TestImportEMLDirIgnoresIncompatibleCheckpoint(t *testing.T) { assert := assert.New(t) require := require.New(t) @@ -294,6 +338,7 @@ func TestImportEMLDirRejectsChangedMailboxAtCheckpoint(t *testing.T) { require.NoError(saveEMLCheckpoint( st, syncID, absRoot, 0, "/old/Inbox.mailbox", "", &store.Checkpoint{}, )) + require.NoError(st.FailSync(syncID, "interrupted test import")) _, err = ImportEMLDir(t.Context(), st, root, EMLImportOptions{ Identifier: "changed@example.test", @@ -321,6 +366,7 @@ func TestImportEMLDirRejectsNegativeCheckpointIndex(t *testing.T) { require.NoError(st.UpdateSyncCheckpoint(syncID, &store.Checkpoint{ PageToken: string(cursor), })) + require.NoError(st.FailSync(syncID, "interrupted test import")) _, err = ImportEMLDir(t.Context(), st, root, EMLImportOptions{ Identifier: "negative@example.test", @@ -346,6 +392,7 @@ func TestImportEMLDirRejectsCheckpointForDifferentRoot(t *testing.T) { require.NoError(saveEMLCheckpoint( st, syncID, otherRoot, 0, "", "", &store.Checkpoint{}, )) + require.NoError(st.FailSync(syncID, "interrupted test import")) _, err = ImportEMLDir(t.Context(), st, root, EMLImportOptions{ Identifier: "root@example.test", diff --git a/internal/store/dialect_pg.go b/internal/store/dialect_pg.go index c42c9b1d4..010e18c87 100644 --- a/internal/store/dialect_pg.go +++ b/internal/store/dialect_pg.go @@ -2307,6 +2307,7 @@ var exclusiveLockTables = []string{ "activity_events", "activity_event_persons", "person_contact_state", "activity_projection_queue", "collections", "collection_sources", "account_identities", "applied_migrations", + "sync_operations", "source_import_items", "sync_run_items", "sync_checkpoints", "imap_folder_state", "imap_message_memberships", } diff --git a/internal/store/store.go b/internal/store/store.go index 30c7efc6b..9091b95f6 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -44,12 +44,13 @@ const HNSWEfSearch = 1000 // methods, existing store code that does s.db.Query(...) compiles // unchanged and automatically routes through the logger. type Store struct { - db *loggedDB - dbPath string - dialect Dialect - readOnly bool // Opened via OpenReadOnly; skips WAL checkpoint on close - fts5Available bool // Whether FTS5 is available for full-text search - closeCleanup func() + db *loggedDB + dbPath string + sqliteFilesystemPath string + dialect Dialect + readOnly bool // Opened via OpenReadOnly; skips WAL checkpoint on close + fts5Available bool // Whether FTS5 is available for full-text search + closeCleanup func() // syncGeneration is immutable metadata on a per-run Store view. // Mutating transactions on that view fence the exact running source @@ -206,10 +207,11 @@ func openSQLite(dbPath, params string) (*Store, error) { } s := &Store{ - db: newLoggedDB(db, dialect.Rebind), - dbPath: dbPath, - dialect: dialect, - syncExecutionLocks: newSyncExecutionLockState(), + db: newLoggedDB(db, dialect.Rebind), + dbPath: dbPath, + sqliteFilesystemPath: filesystemPath, + dialect: dialect, + syncExecutionLocks: newSyncExecutionLockState(), } // Probe like the read-only opens do: a Store must know whether full-text @@ -330,11 +332,12 @@ func OpenReadOnly(dbPath string) (*Store, error) { } s := &Store{ - db: newLoggedDB(db, dialect.Rebind), - dbPath: dbPath, - dialect: dialect, - readOnly: true, - syncExecutionLocks: newSyncExecutionLockState(), + db: newLoggedDB(db, dialect.Rebind), + dbPath: dbPath, + sqliteFilesystemPath: filesystemPath, + dialect: dialect, + readOnly: true, + syncExecutionLocks: newSyncExecutionLockState(), } // OpenReadOnly takes no context, so the probe cannot be cancelled and its diff --git a/internal/store/sync.go b/internal/store/sync.go index 36560b6cf..1412a8d98 100644 --- a/internal/store/sync.go +++ b/internal/store/sync.go @@ -47,11 +47,12 @@ type syncGeneration struct { func (s *Store) ScopedToSync(sourceID, syncRunID int64) *Store { base := s.withoutSyncScope() return &Store{ - db: base.db, - dbPath: base.dbPath, - dialect: base.dialect, - readOnly: base.readOnly, - fts5Available: base.fts5Available, + db: base.db, + dbPath: base.dbPath, + sqliteFilesystemPath: base.sqliteFilesystemPath, + dialect: base.dialect, + readOnly: base.readOnly, + fts5Available: base.fts5Available, syncGeneration: &syncGeneration{sourceID: sourceID, runID: syncRunID}, syncBase: base, diff --git a/internal/store/sync_execution_lock.go b/internal/store/sync_execution_lock.go index 86cd80ed3..3b440ba6a 100644 --- a/internal/store/sync_execution_lock.go +++ b/internal/store/sync_execution_lock.go @@ -157,11 +157,12 @@ func (s *Store) acquireBackendSyncExecutionLock( return lock, nil } - if s.dbPath == ":memory:" || strings.Contains(s.dbPath, ":memory:") { + dbPath := s.sqliteFilesystemPath + if dbPath == ":memory:" || strings.Contains(dbPath, ":memory:") { return noOpSyncExecutionLock{}, nil } - dbPath, err := filepath.Abs(s.dbPath) + dbPath, err := filepath.Abs(dbPath) if err != nil { return nil, fmt.Errorf("resolve sync lock database path: %w", err) } diff --git a/internal/store/sync_test.go b/internal/store/sync_test.go index c177f85bf..99ecd0335 100644 --- a/internal/store/sync_test.go +++ b/internal/store/sync_test.go @@ -3,8 +3,10 @@ package store_test import ( "context" "database/sql" + "net/url" "os" "path/filepath" + "strings" "testing" "time" @@ -177,6 +179,34 @@ func TestStore_StartSyncRejectsConcurrentRunAcrossSQLiteStores(t *testing.T) { requirements.NoError(second.FailSync(secondRun, "test complete")) } +func TestStore_StartSyncUsesFilesystemPathForSQLiteFileURI(t *testing.T) { + requirements := require.New(t) + testutil.SkipIfPostgres(t, "exercises SQLite file URI lock resolution") + dbPath := filepath.Join(t.TempDir(), "archive.db") + uriPath := filepath.ToSlash(dbPath) + if filepath.VolumeName(dbPath) != "" && !strings.HasPrefix(uriPath, "/") { + uriPath = "/" + uriPath + } + dsn := (&url.URL{Scheme: "file", Path: uriPath}).String() + + first, err := store.OpenForTest(dsn) + requirements.NoError(err) + t.Cleanup(func() { _ = first.Close() }) + requirements.NoError(first.InitSchema()) + source, err := first.GetOrCreateSource("gmail", "uri-lock-owner@example.com") + requirements.NoError(err) + + second, err := store.OpenForTest(dbPath) + requirements.NoError(err) + t.Cleanup(func() { _ = second.Close() }) + + firstRun, err := first.StartSync(source.ID, "full") + requirements.NoError(err) + _, err = second.StartSync(source.ID, "full") + requirements.ErrorIs(err, store.ErrSyncAlreadyActive) + requirements.NoError(first.FailSync(firstRun, "test complete")) +} + func TestStore_StartSyncRecoversRunWhoseOwnerClosed(t *testing.T) { requirements := require.New(t) checks := assert.New(t) diff --git a/web/src/lib/api/generated/schema.d.ts b/web/src/lib/api/generated/schema.d.ts index ac2bafe4b..406302b9f 100644 --- a/web/src/lib/api/generated/schema.d.ts +++ b/web/src/lib/api/generated/schema.d.ts @@ -5349,6 +5349,7 @@ export interface components { /** Format: int64 */ limit?: number; noresume?: boolean; + /** @description Gmail search query; not supported for IMAP sources */ query?: string; }; ImportJobResponse: { From 57e499a79dd518667af71ccb1cb27a3c4208a6c4 Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Wed, 2 Sep 2026 14:24:23 -0500 Subject: [PATCH 4/6] fix(sync): keep recovery and IMAP state consistent Daemon startup could fail an operation while leaving its active run marked running. Status readers then saw contradictory terminal state and stale active work. Fail associated running generations in the same transaction as their operations. Keep scheduled IMAP source ownership until authoritative folder state is stored so the next generation cannot supersede that publication. Generated with Codex Co-authored-by: Codex --- cmd/msgvault/cmd/serve.go | 14 +++++++++---- internal/store/sync.go | 41 ++++++++++++++++++++++++++++--------- internal/store/sync_test.go | 35 +++++++++++++++++++++++++++++++ 3 files changed, 76 insertions(+), 14 deletions(-) diff --git a/cmd/msgvault/cmd/serve.go b/cmd/msgvault/cmd/serve.go index 63d1faf10..940c42119 100644 --- a/cmd/msgvault/cmd/serve.go +++ b/cmd/msgvault/cmd/serve.go @@ -3306,13 +3306,19 @@ func runScheduledIMAPSync(ctx context.Context, src *store.Source, s *store.Store return nil, fmt.Errorf("post-source-create migrations: %w", err) } - summary, err := syncer.Full(ctx, src.Identifier) + summary, err := syncer.FullWithFinalizer( + ctx, + src.Identifier, + func(summary *gmail.SyncSummary) error { + if err := saveIMAPFolderStates(ctx, s, src, apiClient, summary, 0); err != nil { + return fmt.Errorf("save IMAP incremental state: %w", err) + } + return nil + }, + ) if err != nil { return nil, fmt.Errorf("IMAP sync failed: %w", err) } - if err := saveIMAPFolderStates(ctx, s, src, apiClient, summary, 0); err != nil { - return nil, fmt.Errorf("save IMAP incremental state: %w", err) - } return summary, nil } diff --git a/internal/store/sync.go b/internal/store/sync.go index 1412a8d98..e57473ddd 100644 --- a/internal/store/sync.go +++ b/internal/store/sync.go @@ -594,17 +594,38 @@ func (s *Store) CreateSyncOperation(sourceID int64, operationID string) (*SyncOp // daemon process. Callers must hold exclusive daemon ownership so no live // worker can still be running or preparing an operation. func (s *Store) FailUnfinishedSyncOperationsContext(ctx context.Context) (int64, error) { - result, err := s.db.ExecContext(ctx, fmt.Sprintf(` - UPDATE sync_operations - SET status = 'failed', finished_at = %s - WHERE status IN ('pending', 'running') - `, s.dialect.Now())) - if err != nil { - return 0, fmt.Errorf("fail unfinished sync operations: %w", err) - } - failed, err := result.RowsAffected() + base := s.withoutSyncScope() + var failed int64 + err := base.withTxContext(ctx, func(tx *loggedTx) error { + now := base.dialect.Now() + if _, err := tx.ExecContext(ctx, fmt.Sprintf(` + UPDATE sync_runs + SET status = 'failed', completed_at = %s, + error_message = 'sync worker exited before recording completion' + WHERE status = 'running' + AND operation_id IN ( + SELECT id FROM sync_operations + WHERE status IN ('pending', 'running') + ) + `, now)); err != nil { + return fmt.Errorf("fail unfinished sync operation runs: %w", err) + } + result, err := tx.ExecContext(ctx, fmt.Sprintf(` + UPDATE sync_operations + SET status = 'failed', finished_at = %s + WHERE status IN ('pending', 'running') + `, now)) + if err != nil { + return fmt.Errorf("fail unfinished sync operations: %w", err) + } + failed, err = result.RowsAffected() + if err != nil { + return fmt.Errorf("fail unfinished sync operations: rows affected: %w", err) + } + return nil + }) if err != nil { - return 0, fmt.Errorf("fail unfinished sync operations: rows affected: %w", err) + return 0, err } return failed, nil } diff --git a/internal/store/sync_test.go b/internal/store/sync_test.go index 99ecd0335..8ce026a95 100644 --- a/internal/store/sync_test.go +++ b/internal/store/sync_test.go @@ -311,6 +311,41 @@ func TestStore_UnfinishedSyncOperationIsRecoveredAtDaemonStartup(t *testing.T) { requirements.Equal(store.SyncStatusCompleted, op.Runs[0].Status) } +func TestStore_UnfinishedSyncOperationRecoveryFailsRunningRun(t *testing.T) { + requirements := require.New(t) + checks := assert.New(t) + st := testutil.NewTestStore(t) + source, err := st.GetOrCreateSource("gmail", "running-operation-recovery@example.com") + requirements.NoError(err) + _, err = st.CreateSyncOperation(source.ID, "abandoned-running-operation") + requirements.NoError(err) + execution, err := st.AcquireSyncExecutionContext(t.Context(), source.ID) + requirements.NoError(err) + runID, err := execution.StartSyncContext(t.Context(), "full", "abandoned-running-operation") + requirements.NoError(err) + requirements.NoError(st.UpdateSyncCheckpoint(runID, &store.Checkpoint{ + PageToken: "resume-token", + MessagesProcessed: 9, + })) + requirements.NoError(execution.Release()) + + failed, err := st.FailUnfinishedSyncOperationsContext(t.Context()) + requirements.NoError(err) + checks.Equal(int64(1), failed) + op, err := st.GetSyncOperation("abandoned-running-operation") + requirements.NoError(err) + checks.Equal(store.SyncStatusFailed, op.Status) + requirements.Len(op.Runs, 1) + checks.Equal(runID, op.Runs[0].ID) + checks.Equal(store.SyncStatusFailed, op.Runs[0].Status) + checks.True(op.Runs[0].CompletedAt.Valid) + checks.Equal("sync worker exited before recording completion", op.Runs[0].ErrorMessage.String) + checks.Equal("resume-token", op.Runs[0].CursorBefore.String) + checks.Equal(int64(9), op.Runs[0].MessagesProcessed) + _, err = st.GetActiveSync(source.ID) + requirements.ErrorIs(err, store.ErrSyncRunNotFound) +} + func TestStore_StartSyncRejectsConcurrentRunAcrossPostgresStores(t *testing.T) { requirements := require.New(t) if !store.IsPostgresURL(os.Getenv("MSGVAULT_TEST_DB")) { From c050617e46409858e2acc4eb85ccdde506f64aaa Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Wed, 2 Sep 2026 15:25:03 -0500 Subject: [PATCH 5/6] fix(sync): validate history fallback options Filtered or limited incremental requests could enter full history reconciliation after an expired cursor. Windows socket resets were also treated as per-message omissions because network detection depended on Unix error text. Validate recovery at the shared entry point and recognize wrapped network errors so partial recovery never mutates archive state and failed reconnects end the batch. Generated with Codex Co-authored-by: Codex --- internal/imap/client.go | 4 ++++ internal/sync/sync.go | 14 ++++++++++++-- internal/sync/sync_test.go | 39 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 55 insertions(+), 2 deletions(-) diff --git a/internal/imap/client.go b/internal/imap/client.go index 585608242..f2691b9d9 100644 --- a/internal/imap/client.go +++ b/internal/imap/client.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "log/slog" + "net" "slices" "strconv" "strings" @@ -1153,6 +1154,9 @@ func isNetworkError(err error) bool { if err == nil { return false } + if _, ok := errors.AsType[net.Error](err); ok { + return true + } msg := err.Error() return strings.Contains(msg, "use of closed network connection") || strings.Contains(msg, "connection reset by peer") || diff --git a/internal/sync/sync.go b/internal/sync/sync.go index bc37d09f1..e137d97a7 100644 --- a/internal/sync/sync.go +++ b/internal/sync/sync.go @@ -1012,17 +1012,27 @@ func (s *Syncer) RecoverExpiredHistory( if source.SourceType != "gmail" { return nil, fmt.Errorf("recover expired history: source %d is %s, not gmail", source.ID, source.SourceType) } - if s.opts.Query != "" || s.opts.Limit > 0 { - return nil, errors.New("recover expired history requires an unfiltered, unlimited full sync") + if err := s.validateHistoryRecoveryOptions(); err != nil { + return nil, err } return s.runWithSyncExecution(ctx, source.ID, func(execution *store.SyncExecution) (*gmail.SyncSummary, error) { return s.recoverExpiredHistory(ctx, source, execution) }) } +func (s *Syncer) validateHistoryRecoveryOptions() error { + if s.opts.Query != "" || s.opts.Limit > 0 { + return errors.New("recover expired history requires an unfiltered, unlimited full sync") + } + return nil +} + func (s *Syncer) recoverExpiredHistory( ctx context.Context, source *store.Source, execution *store.SyncExecution, ) (*gmail.SyncSummary, error) { + if err := s.validateHistoryRecoveryOptions(); err != nil { + return nil, err + } fullSummary, err := s.full(ctx, source, true, execution) if err != nil { return nil, fmt.Errorf("recover expired history: full sync: %w", err) diff --git a/internal/sync/sync_test.go b/internal/sync/sync_test.go index 9d154ebed..e881ca331 100644 --- a/internal/sync/sync_test.go +++ b/internal/sync/sync_test.go @@ -1912,6 +1912,45 @@ func TestIncrementalWithHistoryRecoveryResumesPinnedCursorBeforeIncremental(t *t assert.Equal("20000", refreshed.SyncCursor.String, "catch-up advances from the pinned cursor") } +func TestIncrementalWithHistoryRecoveryRejectsPartialEnumerationOptions(t *testing.T) { + tests := []struct { + name string + modify func(*Options) + }{ + { + name: "query", + modify: func(options *Options) { + options.Query = "from:alice@example.com" + }, + }, + { + name: "limit", + modify: func(options *Options) { + options.Limit = 1 + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + requirements := require.New(t) + checks := assert.New(t) + env := newTestEnv(t) + source := env.CreateSourceWithHistory(t, "1000") + env.Mock.Profile.HistoryID = 2000 + env.Mock.HistoryError = &gmail.NotFoundError{Path: "/history"} + options := DefaultOptions() + test.modify(options) + syncer := New(env.Mock, env.Store, options) + + _, err := syncer.IncrementalWithHistoryRecovery(env.Context, source, nil) + requirements.ErrorContains(err, "requires an unfiltered, unlimited full sync") + checks.Zero(env.Mock.ListMessagesCalls) + checks.Zero(env.Mock.SnapshotListCalls) + }) + } +} + func TestFullRoutesPinnedHistoryRecoveryThroughCatchup(t *testing.T) { require := require.New(t) assert := assert.New(t) From 0e89cd2615d167700cf9c8c232325de9151b09fa Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Wed, 2 Sep 2026 17:43:39 -0500 Subject: [PATCH 6/6] fix(sync): fail runs with failed operations A canceled import subprocess could leave its operation failed while its child run stayed running. Active-sync gates then treated abandoned work as live indefinitely. Make failed-operation finalization and child cleanup one transaction. Startup recovery also repairs inconsistent rows left by an earlier daemon. Generated with Codex Co-authored-by: Codex --- internal/store/sync.go | 72 ++++++++++++++++++++++--------------- internal/store/sync_test.go | 64 +++++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+), 29 deletions(-) diff --git a/internal/store/sync.go b/internal/store/sync.go index e57473ddd..09fcb81be 100644 --- a/internal/store/sync.go +++ b/internal/store/sync.go @@ -18,6 +18,7 @@ const ( SyncRunItemStatusSkipped = "skipped" manualTransactionCleanupTimeout = 5 * time.Second + syncWorkerExitedMessage = "sync worker exited before recording completion" ) // ErrSyncRunNotFound is returned by the sync-run getters (GetActiveSync, @@ -547,8 +548,8 @@ func (s *Store) recoverAbandonedSyncSourceQueries( if _, err := q.ExecContext(ctx, s.Rebind(fmt.Sprintf(` UPDATE sync_runs SET status = 'failed', completed_at = %s, - error_message = 'sync worker exited before recording completion' - WHERE source_id = ? AND status = 'running'`, now)), sourceID); err != nil { + error_message = ? + WHERE source_id = ? AND status = 'running'`, now)), syncWorkerExitedMessage, sourceID); err != nil { return fmt.Errorf("fail abandoned sync: %w", err) } return nil @@ -601,13 +602,13 @@ func (s *Store) FailUnfinishedSyncOperationsContext(ctx context.Context) (int64, if _, err := tx.ExecContext(ctx, fmt.Sprintf(` UPDATE sync_runs SET status = 'failed', completed_at = %s, - error_message = 'sync worker exited before recording completion' + error_message = ? WHERE status = 'running' AND operation_id IN ( SELECT id FROM sync_operations - WHERE status IN ('pending', 'running') + WHERE status IN ('pending', 'running', 'failed') ) - `, now)); err != nil { + `, now), syncWorkerExitedMessage); err != nil { return fmt.Errorf("fail unfinished sync operation runs: %w", err) } result, err := tx.ExecContext(ctx, fmt.Sprintf(` @@ -707,32 +708,45 @@ func (s *Store) FinishSyncOperation(operationID, status string) error { if status != "done" && status != "failed" { return fmt.Errorf("invalid sync operation status %q", status) } - result, err := s.db.Exec(fmt.Sprintf(` - UPDATE sync_operations - SET status = ?, finished_at = %s - WHERE id = ? AND status IN ('pending', 'running') - `, s.dialect.Now()), status, operationID) - if err != nil { - return err - } - updated, err := result.RowsAffected() - if err != nil { - return fmt.Errorf("finish sync operation %q: rows affected: %w", operationID, err) - } - if updated == 1 { - return nil - } - var currentStatus string - if err := s.db.QueryRow(`SELECT status FROM sync_operations WHERE id = ?`, operationID).Scan(¤tStatus); err != nil { - if errors.Is(err, sql.ErrNoRows) { - return fmt.Errorf("finish sync operation %q: %w", operationID, ErrSyncRunNotFound) + base := s.withoutSyncScope() + return base.withTx(func(tx *loggedTx) error { + now := base.dialect.Now() + result, err := tx.Exec(fmt.Sprintf(` + UPDATE sync_operations + SET status = ?, finished_at = %s + WHERE id = ? AND status IN ('pending', 'running') + `, now), status, operationID) + if err != nil { + return err + } + updated, err := result.RowsAffected() + if err != nil { + return fmt.Errorf("finish sync operation %q: rows affected: %w", operationID, err) + } + currentStatus := status + if updated != 1 { + if err := tx.QueryRow(`SELECT status FROM sync_operations WHERE id = ?`, operationID).Scan(¤tStatus); err != nil { + if errors.Is(err, sql.ErrNoRows) { + return fmt.Errorf("finish sync operation %q: %w", operationID, ErrSyncRunNotFound) + } + return fmt.Errorf("read finished sync operation %q: %w", operationID, err) + } + } + if currentStatus != "done" && currentStatus != SyncStatusFailed { + return fmt.Errorf("finish sync operation %q: unexpected status %q", operationID, currentStatus) + } + if currentStatus != SyncStatusFailed { + return nil + } + if _, err := tx.Exec(fmt.Sprintf(` + UPDATE sync_runs + SET status = 'failed', completed_at = %s, error_message = ? + WHERE operation_id = ? AND status = 'running' + `, now), syncWorkerExitedMessage, operationID); err != nil { + return fmt.Errorf("fail running sync runs for operation %q: %w", operationID, err) } - return fmt.Errorf("read finished sync operation %q: %w", operationID, err) - } - if currentStatus == "done" || currentStatus == SyncStatusFailed { return nil - } - return fmt.Errorf("finish sync operation %q: unexpected status %q", operationID, currentStatus) + }) } // UpdateSyncCheckpoint saves progress for resumption. diff --git a/internal/store/sync_test.go b/internal/store/sync_test.go index 8ce026a95..328823a03 100644 --- a/internal/store/sync_test.go +++ b/internal/store/sync_test.go @@ -772,6 +772,31 @@ func TestStore_FinishSyncOperationPreservesFirstTerminalStatus(t *testing.T) { } } +func TestStore_FinishFailedSyncOperationFailsRunningRuns(t *testing.T) { + requirements := require.New(t) + checks := assert.New(t) + f := storetest.New(t) + const operationID = "failed-with-running-run" + + _, err := f.Store.CreateSyncOperation(f.Source.ID, operationID) + requirements.NoError(err) + runID, err := f.Store.StartSyncOperation(f.Source.ID, operationID) + requirements.NoError(err) + requirements.NoError(f.Store.FinishSyncOperation(operationID, store.SyncStatusFailed)) + + op, err := f.Store.GetSyncOperation(operationID) + requirements.NoError(err) + checks.Equal(store.SyncStatusFailed, op.Status) + requirements.Len(op.Runs, 1) + checks.Equal(runID, op.Runs[0].ID) + checks.Equal(store.SyncStatusFailed, op.Runs[0].Status) + checks.True(op.Runs[0].CompletedAt.Valid) + checks.Equal("sync worker exited before recording completion", op.Runs[0].ErrorMessage.String) + active, err := f.Store.HasAnyActiveSync() + requirements.NoError(err) + checks.False(active) +} + func TestStore_FailUnfinishedSyncOperations(t *testing.T) { require := require.New(t) assert := assert.New(t) @@ -791,6 +816,45 @@ func TestStore_FailUnfinishedSyncOperations(t *testing.T) { assert.Empty(op.Runs) } +func TestStore_FailUnfinishedSyncOperationsRepairsFailedOperationRuns(t *testing.T) { + requirements := require.New(t) + checks := assert.New(t) + first := testutil.NewTestStore(t) + source, err := first.GetOrCreateSource("gmail", "failed-operation-recovery@example.com") + requirements.NoError(err) + const operationID = "failed-operation-with-running-run" + _, err = first.CreateSyncOperation(source.ID, operationID) + requirements.NoError(err) + runID, err := first.StartSyncOperation(source.ID, operationID) + requirements.NoError(err) + _, err = first.DB().Exec(first.Rebind(` + UPDATE sync_operations + SET status = 'failed', finished_at = CURRENT_TIMESTAMP + WHERE id = ? + `), operationID) + requirements.NoError(err) + dbPath := store.DBPathForTest(first) + requirements.NoError(first.Close()) + + second, err := store.OpenForTest(dbPath) + requirements.NoError(err) + t.Cleanup(func() { _ = second.Close() }) + failed, err := second.FailUnfinishedSyncOperationsContext(t.Context()) + requirements.NoError(err) + checks.Zero(failed) + op, err := second.GetSyncOperation(operationID) + requirements.NoError(err) + checks.Equal(store.SyncStatusFailed, op.Status) + requirements.Len(op.Runs, 1) + checks.Equal(runID, op.Runs[0].ID) + checks.Equal(store.SyncStatusFailed, op.Runs[0].Status) + checks.True(op.Runs[0].CompletedAt.Valid) + checks.Equal("sync worker exited before recording completion", op.Runs[0].ErrorMessage.String) + active, err := second.HasAnyActiveSync() + requirements.NoError(err) + checks.False(active) +} + func TestStore_SyncRunItems(t *testing.T) { require := require.New(t) assert := assert.New(t)