fix: harden mcp and cloud recovery paths

Author: Alan-TheGentleman

cmd/engram/main.go

@@ -482,10 +482,53 @@ func preflightCloudSync(s *store.Store, cfg store.Config, project string, mutate
 			}
 			return nil, fmt.Errorf("cloud sync blocked_unenrolled: %s", message)
 		}
+		if err := preflightCloudSyncLegacyMutations(s, project, targetKey, mutateState); err != nil {
+			return nil, err
+		}
 	}
 	return cc, nil
 }
 
+func preflightCloudSyncLegacyMutations(s *store.Store, project, targetKey string, mutateState bool) error {
+	report, err := s.DiagnoseCloudUpgradeLegacyMutations(project)
+	if err != nil {
+		return fmt.Errorf("cloud sync legacy mutation preflight: %w", err)
+	}
+	if report.BlockedCount == 0 && report.RepairableCount == 0 {
+		return nil
+	}
+
+	reasonCode := store.UpgradeReasonRepairableLegacyMutationPayload
+	message := fmt.Sprintf(
+		"legacy mutation payloads require repair before cloud sync for project %q: run `engram cloud upgrade doctor --project %s` then `engram cloud upgrade repair --project %s --apply`",
+		project, project, project,
+	)
+	if report.BlockedCount > 0 {
+		reasonCode = store.UpgradeReasonBlockedLegacyMutationManual
+		first := firstBlockedLegacyMutationFinding(report)
+		message = fmt.Sprintf(
+			"legacy mutation payloads require manual action before cloud sync for project %q (seq=%d entity=%s op=%s): %s; inspect with `engram cloud upgrade doctor --project %s` and run `engram cloud upgrade repair --project %s --apply` for deterministic repairs",
+			project, first.Seq, first.Entity, first.Op, first.Message, project, project,
+		)
+	}
+	if mutateState {
+		_ = s.MarkSyncBlocked(targetKey, reasonCode, message)
+	}
+	return fmt.Errorf("cloud sync %s: %s", reasonCode, message)
+}
+
+func firstBlockedLegacyMutationFinding(report store.CloudUpgradeLegacyMutationReport) store.CloudUpgradeLegacyMutationFinding {
+	for _, finding := range report.Findings {
+		if !finding.Repairable {
+			return finding
+		}
+	}
+	if len(report.Findings) > 0 {
+		return report.Findings[0]
+	}
+	return store.CloudUpgradeLegacyMutationFinding{}
+}
+
 func cloudTargetKeyForProject(project string) string {
 	project = strings.TrimSpace(project)
 	if project == "" {

cmd/engram/main_extra_test.go

@@ -861,6 +861,84 @@ func TestCmdCloudUpgradeDoctorRequiresProjectAndIsDeterministic(t *testing.T) {
 	})
 }
 
+func TestCmdSyncCloudPreflightsLegacyMutationPayloads(t *testing.T) {
+	stubExitWithPanic(t)
+	stubRuntimeHooks(t)
+
+	cfg := testConfig(t)
+	if err := saveCloudConfig(cfg, &cloudConfig{ServerURL: "https://cloud.example.test"}); err != nil {
+		t.Fatalf("save cloud config: %v", err)
+	}
+
+	s, err := store.New(cfg)
+	if err != nil {
+		t.Fatalf("open store: %v", err)
+	}
+	if err := s.CreateSession("sync-legacy-s1", "sync-legacy", "/tmp/sync-legacy"); err != nil {
+		_ = s.Close()
+		t.Fatalf("create session: %v", err)
+	}
+	if _, err := s.AddObservation(store.AddObservationParams{SessionID: "sync-legacy-s1", Type: "decision", Title: "Canonical title", Content: "Canonical content", Project: "sync-legacy", Scope: "project"}); err != nil {
+		_ = s.Close()
+		t.Fatalf("add observation: %v", err)
+	}
+	if err := s.EnrollProject("sync-legacy"); err != nil {
+		_ = s.Close()
+		t.Fatalf("enroll project: %v", err)
+	}
+	_ = s.Close()
+
+	db, err := sql.Open("sqlite", filepath.Join(cfg.DataDir, "engram.db"))
+	if err != nil {
+		t.Fatalf("open raw db: %v", err)
+	}
+	defer db.Close()
+	var syncID string
+	if err := db.QueryRow(`SELECT sync_id FROM observations WHERE session_id = ? ORDER BY id DESC LIMIT 1`, "sync-legacy-s1").Scan(&syncID); err != nil {
+		t.Fatalf("lookup sync id: %v", err)
+	}
+	legacyPayload := `{"sync_id":"` + syncID + `","session_id":"sync-legacy-s1","type":"decision","content":"legacy payload missing title","scope":"project"}`
+	if _, err := db.Exec(
+		`INSERT INTO sync_mutations (target_key, entity, entity_key, op, payload, source, project) VALUES (?, ?, ?, ?, ?, ?, ?)`,
+		store.DefaultSyncTargetKey,
+		store.SyncEntityObservation,
+		syncID,
+		store.SyncOpUpsert,
+		legacyPayload,
+		store.SyncSourceLocal,
+		"sync-legacy",
+	); err != nil {
+		t.Fatalf("insert legacy mutation: %v", err)
+	}
+
+	exportCalled := false
+	oldSyncExport := syncExport
+	syncExport = func(_ *engramsync.Syncer, _, _ string) (*engramsync.SyncResult, error) {
+		exportCalled = true
+		return &engramsync.SyncResult{}, nil
+	}
+	t.Cleanup(func() { syncExport = oldSyncExport })
+
+	withArgs(t, "engram", "sync", "--cloud", "--project", "sync-legacy")
+	_, stderr, recovered := captureOutputAndRecover(t, func() { cmdSync(cfg) })
+	if _, ok := recovered.(exitCode); !ok {
+		t.Fatalf("expected cloud sync preflight to fail loudly, got %v", recovered)
+	}
+	if exportCalled {
+		t.Fatal("cloud sync must block before export/canonicalization")
+	}
+	if !strings.Contains(stderr, "legacy mutation payloads require repair before cloud sync") ||
+		!strings.Contains(stderr, "engram cloud upgrade doctor --project sync-legacy") ||
+		!strings.Contains(stderr, "engram cloud upgrade repair --project sync-legacy --apply") {
+		t.Fatalf("expected actionable legacy mutation guidance, got %q", stderr)
+	}
+
+	var persistedPayload string
+	if err := db.QueryRow(`SELECT payload FROM sync_mutations WHERE project = ? AND payload = ?`, "sync-legacy", legacyPayload).Scan(&persistedPayload); err != nil {
+		t.Fatalf("expected sync preflight not to auto-repair payload: %v", err)
+	}
+}
+
 func TestCmdCloudUpgradeBootstrapStatusAndRollbackSemantics(t *testing.T) {
 	stubExitWithPanic(t)
 	stubRuntimeHooks(t)

internal/mcp/mcp.go

@@ -1417,9 +1417,10 @@ func handleSessionStart(s *store.Store, cfg MCPConfig, activity *SessionActivity
 	return func(ctx context.Context, req mcp.CallToolRequest) (*mcp.CallToolResult, error) {
 		id, _ := req.GetArguments()["id"].(string)
 		directory, _ := req.GetArguments()["directory"].(string)
+		explicitDirectory := strings.TrimSpace(directory)
 		// project field intentionally not read — auto-detect only (REQ-308)
 
-		detRes, err := resolveWriteProject()
+		detRes, err := resolveSessionStartProject(explicitDirectory)
 		if err != nil {
 			// JW1: use AvailableProjects from detection result (repos in cwd).
 			return errorWithMeta("ambiguous_project",
@@ -1430,7 +1431,7 @@ func handleSessionStart(s *store.Store, cfg MCPConfig, activity *SessionActivity
 		project, _ := store.NormalizeProject(detRes.Project)
 
 		activity.RecordToolCall(defaultSessionID(project))
-		if strings.TrimSpace(directory) == "" {
+		if explicitDirectory == "" {
 			directory = strings.TrimSpace(detRes.Path)
 			if directory == "" {
 				directory = currentWorkingDirectory()
@@ -1446,6 +1447,20 @@ func handleSessionStart(s *store.Store, cfg MCPConfig, activity *SessionActivity
 	}
 }
 
+func resolveSessionStartProject(explicitDirectory string) (projectpkg.DetectionResult, error) {
+	if explicitDirectory == "" {
+		return resolveWriteProject()
+	}
+	res := projectpkg.DetectProjectFull(explicitDirectory)
+	if res.Error != nil {
+		return res, res.Error
+	}
+	if res.Source == projectpkg.SourceDirBasename {
+		return resolveWriteProject()
+	}
+	return res, nil
+}
+
 func handleSessionEnd(s *store.Store, cfg MCPConfig, activity *SessionActivity) server.ToolHandlerFunc {
 	return func(ctx context.Context, req mcp.CallToolRequest) (*mcp.CallToolResult, error) {
 		id, _ := req.GetArguments()["id"].(string)

internal/mcp/mcp_test.go

@@ -2668,6 +2668,56 @@ func TestSessionStartWithExplicitDirectoryPreservesDirectory(t *testing.T) {
 	assertSessionSyncMutationDirectory(t, s, "session-start-explicit", explicitDir)
 }
 
+func TestSessionStartWithExplicitDirectoryResolvesProjectFromDirectory(t *testing.T) {
+	s := newMCPTestStore(t)
+
+	workspace := t.TempDir()
+	rightRepo := filepath.Join(workspace, "right-repo")
+	wrongRepo := filepath.Join(workspace, "wrong-repo")
+	if err := os.MkdirAll(filepath.Join(rightRepo, "nested"), 0755); err != nil {
+		t.Fatalf("create right repo nested dir: %v", err)
+	}
+	if err := os.MkdirAll(wrongRepo, 0755); err != nil {
+		t.Fatalf("create wrong repo dir: %v", err)
+	}
+	initTestGitRepo(t, rightRepo)
+	initTestGitRepo(t, wrongRepo)
+	cmd := exec.Command("git", "-C", rightRepo, "remote", "add", "origin",
+		"git@github.com:user/explicit-session-project.git")
+	if out, err := cmd.CombinedOutput(); err != nil {
+		t.Fatalf("git remote add right repo: %v\n%s", err, out)
+	}
+	cmd = exec.Command("git", "-C", wrongRepo, "remote", "add", "origin",
+		"git@github.com:user/wrong-session-project.git")
+	if out, err := cmd.CombinedOutput(); err != nil {
+		t.Fatalf("git remote add wrong repo: %v\n%s", err, out)
+	}
+	t.Chdir(workspace)
+
+	explicitDir := filepath.Join(rightRepo, "nested")
+	start := handleSessionStart(s, MCPConfig{}, NewSessionActivity(10*time.Minute))
+	res, err := start(context.Background(), mcppkg.CallToolRequest{
+		Params: mcppkg.CallToolParams{Arguments: map[string]any{
+			"id":        "session-start-explicit-project",
+			"directory": explicitDir,
+		}},
+	})
+	if err != nil || res.IsError {
+		t.Fatalf("session start: err=%v isError=%v text=%s", err, res.IsError, callResultText(t, res))
+	}
+
+	sess, err := s.GetSession("session-start-explicit-project")
+	if err != nil {
+		t.Fatalf("get session: %v", err)
+	}
+	if sess.Project != "explicit-session-project" {
+		t.Fatalf("expected explicit directory project, got %q", sess.Project)
+	}
+	if sess.Directory != explicitDir {
+		t.Fatalf("expected persisted directory=%q, got %q", explicitDir, sess.Directory)
+	}
+}
+
 // ─── Batch 4: Write handler schema + auto-detect ─────────────────────────────
 
 // TestWriteSchema_NoProjectField asserts that the 6 write tools do NOT include

internal/store/store.go

@@ -30,6 +30,17 @@ var openDB = sql.Open
 // See https://www.sqlite.org/rescode.html#constraint_foreignkey
 const sqliteConstraintForeignKey = 787
 
+const (
+	sqlitePrimaryBusy   = 5
+	sqlitePrimaryLocked = 6
+)
+
+var sqliteWriteRetryBackoffs = []time.Duration{
+	10 * time.Millisecond,
+	25 * time.Millisecond,
+	50 * time.Millisecond,
+}
+
 // Sentinel errors returned by delete operations so callers can use errors.Is.
 var (
 	ErrSessionNotFound        = errors.New("session not found")
@@ -4164,15 +4175,46 @@ func (s *Store) PruneProject(project string) (*PruneResult, error) {
 // ─── Helpers ─────────────────────────────────────────────────────────────────
 
 func (s *Store) withTx(fn func(tx *sql.Tx) error) error {
-	tx, err := s.beginTxHook()
-	if err != nil {
-		return err
+	return withSQLiteWriteRetry(func() error {
+		tx, err := s.beginTxHook()
+		if err != nil {
+			return err
+		}
+		defer tx.Rollback()
+		if err := fn(tx); err != nil {
+			return err
+		}
+		return s.commitHook(tx)
+	})
+}
+
+func withSQLiteWriteRetry(fn func() error) error {
+	var lastErr error
+	for attempt := 0; attempt <= len(sqliteWriteRetryBackoffs); attempt++ {
+		if err := fn(); err != nil {
+			lastErr = err
+			if !isRetryableSQLiteLockError(err) || attempt == len(sqliteWriteRetryBackoffs) {
+				return err
+			}
+			time.Sleep(sqliteWriteRetryBackoffs[attempt])
+			continue
+		}
+		return nil
 	}
-	defer tx.Rollback()
-	if err := fn(tx); err != nil {
-		return err
+	return lastErr
+}
+
+func isRetryableSQLiteLockError(err error) bool {
+	if err == nil {
+		return false
 	}
-	return s.commitHook(tx)
+	var sqliteErr *sqlite.Error
+	if errors.As(err, &sqliteErr) {
+		primaryCode := sqliteErr.Code() & 0xff
+		return primaryCode == sqlitePrimaryBusy || primaryCode == sqlitePrimaryLocked
+	}
+	msg := strings.ToLower(err.Error())
+	return strings.Contains(msg, "database is locked") || strings.Contains(msg, "database is busy") || strings.Contains(msg, "sqlite_busy") || strings.Contains(msg, "sqlite_locked")
 }
 
 func (s *Store) createSessionTx(tx *sql.Tx, id, project, directory string) error {

internal/store/store_test.go

@@ -3616,6 +3616,66 @@ func TestHookFallbacksAndAdditionalBranches(t *testing.T) {
 	})
 }
 
+func TestSQLiteWriteRetryRetriesTransientLockErrors(t *testing.T) {
+	oldBackoffs := sqliteWriteRetryBackoffs
+	sqliteWriteRetryBackoffs = []time.Duration{0, 0, 0}
+	t.Cleanup(func() { sqliteWriteRetryBackoffs = oldBackoffs })
+
+	t.Run("begin lock is retried and succeeds", func(t *testing.T) {
+		s := newTestStore(t)
+		origBegin := s.hooks.beginTx
+		attempts := 0
+		s.hooks.beginTx = func(db *sql.DB) (*sql.Tx, error) {
+			attempts++
+			if attempts < 3 {
+				return nil, errors.New("database is locked")
+			}
+			return origBegin(db)
+		}
+
+		if err := s.CreateSession("retry-session", "retry-project", "/tmp/retry-project"); err != nil {
+			t.Fatalf("expected retry to succeed, got %v", err)
+		}
+		if attempts != 3 {
+			t.Fatalf("expected 3 begin attempts, got %d", attempts)
+		}
+	})
+
+	t.Run("non lock error is not retried", func(t *testing.T) {
+		s := newTestStore(t)
+		attempts := 0
+		s.hooks.beginTx = func(_ *sql.DB) (*sql.Tx, error) {
+			attempts++
+			return nil, errors.New("permanent begin failure")
+		}
+
+		err := s.CreateSession("no-retry-session", "retry-project", "/tmp/retry-project")
+		if err == nil || !strings.Contains(err.Error(), "permanent begin failure") {
+			t.Fatalf("expected permanent error, got %v", err)
+		}
+		if attempts != 1 {
+			t.Fatalf("expected one attempt for permanent error, got %d", attempts)
+		}
+	})
+
+	t.Run("lock errors remain bounded", func(t *testing.T) {
+		s := newTestStore(t)
+		attempts := 0
+		s.hooks.beginTx = func(_ *sql.DB) (*sql.Tx, error) {
+			attempts++
+			return nil, errors.New("SQLITE_BUSY: database is locked")
+		}
+
+		err := s.CreateSession("bounded-session", "retry-project", "/tmp/retry-project")
+		if err == nil || !isRetryableSQLiteLockError(err) {
+			t.Fatalf("expected retryable lock error after exhaustion, got %v", err)
+		}
+		if attempts != len(sqliteWriteRetryBackoffs)+1 {
+			t.Fatalf("expected bounded attempts=%d, got %d", len(sqliteWriteRetryBackoffs)+1, attempts)
+		}
+	})
+}
+
 func TestStoreUncoveredBranchesPushToHundred(t *testing.T) {
 	t.Run("new open database hook error", func(t *testing.T) {
 		orig := openDB