From b088144fd9abc14bb2e0d3b175664a780889eb52 Mon Sep 17 00:00:00 2001 From: viettranx Date: Sat, 28 Mar 2026 08:31:18 +0700 Subject: [PATCH] fix(sqlite): bootstrap not running on first chat due to per-connection PRAGMA gap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause: pool.go applied busy_timeout PRAGMA via db.Exec() which only affects one connection in the pool. Other connections had no busy_timeout, causing immediate SQLITE_BUSY errors during concurrent startup operations (agent creation, WebSocket connect, health checks). This silently aborted context file seeding — BOOTSTRAP.md, USER.md, AGENTS.md all missing from system prompt on first interaction. Fix (3 layers): 1. pragmaConnector: wraps sql.Driver to apply PRAGMAs (busy_timeout, WAL, etc.) on every new connection, not just one. All SQLite queries benefit. 2. CacheInvalidateFunc: clears ContextFileInterceptor cache after seeding so LoadContextFiles sees newly seeded files on the first turn. 3. fallbackBootstrap: if DB seed still fails, injects embedded templates in-memory so the first turn still gets onboarding. Clears after use. --- cmd/gateway_callbacks.go | 12 ++++ cmd/gateway_managed.go | 1 + internal/agent/loop_history.go | 26 +++++++++ internal/agent/loop_types.go | 13 ++++- internal/agent/loop_utils.go | 10 ++++ internal/agent/resolver.go | 2 + internal/bootstrap/seed_store.go | 26 +++++++++ internal/store/sqlitestore/pool.go | 91 +++++++++++++++++++++++------- 8 files changed, 160 insertions(+), 21 deletions(-) diff --git a/cmd/gateway_callbacks.go b/cmd/gateway_callbacks.go index 66f6fddb..d0b34eb2 100644 --- a/cmd/gateway_callbacks.go +++ b/cmd/gateway_callbacks.go @@ -73,6 +73,18 @@ func buildBootstrapCleanup(as store.AgentStore) agent.BootstrapCleanupFunc { } } +// buildCacheInvalidate creates a callback that invalidates the context file cache +// for a user after SeedUserFiles writes via raw agentStore. Without this, +// LoadContextFiles may return stale (empty) cached results on the first turn. +func buildCacheInvalidate(intc *tools.ContextFileInterceptor) agent.CacheInvalidateFunc { + if intc == nil { + return nil + } + return func(agentID uuid.UUID, userID string) { + intc.InvalidateUser(agentID, userID) + } +} + // buildContextFileLoader creates the per-request context file loader callback. // Delegates to the ContextFileInterceptor for type-aware routing. func buildContextFileLoader(intc *tools.ContextFileInterceptor) agent.ContextFileLoaderFunc { diff --git a/cmd/gateway_managed.go b/cmd/gateway_managed.go index 49ead5f8..d53c0e7e 100644 --- a/cmd/gateway_managed.go +++ b/cmd/gateway_managed.go @@ -144,6 +144,7 @@ func wireExtras( SeedUserFiles: seedUserFiles, ContextFileLoader: contextFileLoader, BootstrapCleanup: buildBootstrapCleanup(stores.Agents), + CacheInvalidate: buildCacheInvalidate(contextFileInterceptor), InjectionAction: injectionAction, MaxMessageChars: appCfg.Gateway.MaxMessageChars, CompactionCfg: appCfg.Agents.Defaults.Compaction, diff --git a/internal/agent/loop_history.go b/internal/agent/loop_history.go index 57718066..aa112831 100644 --- a/internal/agent/loop_history.go +++ b/internal/agent/loop_history.go @@ -137,6 +137,17 @@ func (l *Loop) buildMessages(ctx context.Context, history []providers.Message, s var contextFiles []bootstrap.ContextFile if !lightContext { contextFiles = l.resolveContextFiles(ctx, userID) + + // Fallback: if DB seeding failed (e.g. SQLITE_BUSY) but we have + // in-memory embedded templates, merge them so the first turn still + // gets bootstrap onboarding. Only applies when DB returned no user files. + if val, ok := l.userSetups.Load(userID); ok { + if fb := val.(*userSetup).fallbackBootstrap; len(fb) > 0 { + contextFiles = l.mergeContextFallback(contextFiles, fb) + // Clear after first use — next turn should read from DB. + val.(*userSetup).fallbackBootstrap = nil + } + } } hadBootstrap := false for _, cf := range contextFiles { @@ -318,6 +329,21 @@ func (l *Loop) resolveContextFiles(ctx context.Context, userID string) []bootstr return merged } +// mergeContextFallback adds fallback (in-memory) files into contextFiles, +// skipping any that already exist. Used when DB seeding failed. +func (l *Loop) mergeContextFallback(contextFiles, fallback []bootstrap.ContextFile) []bootstrap.ContextFile { + existing := make(map[string]struct{}, len(contextFiles)) + for _, f := range contextFiles { + existing[f.Path] = struct{}{} + } + for _, fb := range fallback { + if _, ok := existing[fb.Path]; !ok { + contextFiles = append(contextFiles, fb) + } + } + return contextFiles +} + // bootstrapToolAllowlist is the set of tools available during bootstrap onboarding. // Only write_file (and its alias Write) are needed to save USER.md and clear BOOTSTRAP.md. var bootstrapToolAllowlist = map[string]bool{ diff --git a/internal/agent/loop_types.go b/internal/agent/loop_types.go index b3e0a6fa..830a6d81 100644 --- a/internal/agent/loop_types.go +++ b/internal/agent/loop_types.go @@ -28,8 +28,9 @@ const bootstrapAutoCleanupTurns = 3 // Consolidates workspace resolution and context file seeding into one struct // to prevent desync between the two concerns. type userSetup struct { - workspace string // effective workspace from user_agent_profiles (expanded, absolute) - seeded bool // whether SeedUserFiles has been called this instance + workspace string // effective workspace from user_agent_profiles (expanded, absolute) + seeded bool // whether SeedUserFiles has been called this instance + fallbackBootstrap []bootstrap.ContextFile // in-memory fallback when DB seed fails (e.g. SQLITE_BUSY) } // EnsureUserProfileFunc creates/resolves a user's profile and workspace. @@ -55,6 +56,11 @@ type ContextFileLoaderFunc func(ctx context.Context, agentID uuid.UUID, userID, // Called automatically so the system doesn't rely on the LLM to delete it. type BootstrapCleanupFunc func(ctx context.Context, agentID uuid.UUID, userID string) error +// CacheInvalidateFunc invalidates the context file cache for a user after seeding. +// SeedUserFiles writes via raw agentStore (bypassing ContextFileInterceptor cache), +// so this callback ensures LoadContextFiles sees the newly seeded files. +type CacheInvalidateFunc func(agentID uuid.UUID, userID string) + // Loop is the agent execution loop for one agent instance. // Think → Act → Observe cycle with tool execution. type Loop struct { @@ -101,6 +107,7 @@ type Loop struct { ensureUserFiles EnsureUserFilesFunc // legacy combined callback (fallback) contextFileLoader ContextFileLoaderFunc bootstrapCleanup BootstrapCleanupFunc + cacheInvalidate CacheInvalidateFunc // invalidate context file cache after seeding userSetups sync.Map // userID → *userSetup (workspace + seeding state, per Loop instance) // Compaction config (memory flush settings) @@ -250,6 +257,7 @@ type LoopConfig struct { EnsureUserFiles EnsureUserFilesFunc // legacy: combined (used when above are nil) ContextFileLoader ContextFileLoaderFunc BootstrapCleanup BootstrapCleanupFunc + CacheInvalidate CacheInvalidateFunc // invalidate context file cache after seeding // Tracing collector (nil = no tracing) TraceCollector *tracing.Collector @@ -365,6 +373,7 @@ func NewLoop(cfg LoopConfig) *Loop { ensureUserFiles: cfg.EnsureUserFiles, contextFileLoader: cfg.ContextFileLoader, bootstrapCleanup: cfg.BootstrapCleanup, + cacheInvalidate: cfg.CacheInvalidate, compactionCfg: cfg.CompactionCfg, contextPruningCfg: cfg.ContextPruningCfg, sandboxEnabled: cfg.SandboxEnabled, diff --git a/internal/agent/loop_utils.go b/internal/agent/loop_utils.go index 40f7223a..e9947003 100644 --- a/internal/agent/loop_utils.go +++ b/internal/agent/loop_utils.go @@ -8,6 +8,7 @@ import ( "slices" "strings" + "github.com/nextlevelbuilder/goclaw/internal/bootstrap" "github.com/nextlevelbuilder/goclaw/internal/config" "github.com/nextlevelbuilder/goclaw/internal/providers" "github.com/nextlevelbuilder/goclaw/internal/tools" @@ -91,6 +92,15 @@ func (l *Loop) getOrCreateUserSetup(ctx context.Context, userID, channel string, // Passes isNew so SeedUserFiles knows whether to skip existing files. if err := l.seedUserFiles(ctx, l.agentUUID, userID, l.agentType, isNew); err != nil { slog.Warn("failed to seed user context files", "error", err) + // Seeding failed (e.g. SQLITE_BUSY after retries). Inject + // embedded bootstrap templates in-memory so the first turn + // still gets onboarding. DB seed will retry next session. + setup.fallbackBootstrap = bootstrap.EmbeddedUserFiles(l.agentType) + } else if l.cacheInvalidate != nil { + // SeedUserFiles writes via raw agentStore, bypassing the + // ContextFileInterceptor cache. Invalidate so LoadContextFiles + // sees newly seeded BOOTSTRAP.md/USER.md on the first turn. + l.cacheInvalidate(l.agentUUID, userID) } setup.seeded = true } else if l.ensureUserFiles != nil { diff --git a/internal/agent/resolver.go b/internal/agent/resolver.go index 6583caec..6c4cb914 100644 --- a/internal/agent/resolver.go +++ b/internal/agent/resolver.go @@ -40,6 +40,7 @@ type ResolverDeps struct { SeedUserFiles SeedUserFilesFunc ContextFileLoader ContextFileLoaderFunc BootstrapCleanup BootstrapCleanupFunc + CacheInvalidate CacheInvalidateFunc // Security InjectionAction string // "log", "warn", "block", "off" @@ -370,6 +371,7 @@ func NewManagedResolver(deps ResolverDeps) ResolverFunc { SeedUserFiles: deps.SeedUserFiles, ContextFileLoader: deps.ContextFileLoader, BootstrapCleanup: deps.BootstrapCleanup, + CacheInvalidate: deps.CacheInvalidate, OnEvent: deps.OnEvent, TraceCollector: deps.TraceCollector, InjectionAction: deps.InjectionAction, diff --git a/internal/bootstrap/seed_store.go b/internal/bootstrap/seed_store.go index 62b00c9b..b5001588 100644 --- a/internal/bootstrap/seed_store.go +++ b/internal/bootstrap/seed_store.go @@ -44,6 +44,7 @@ func SeedToStore(ctx context.Context, agentStore store.AgentStore, agentID uuid. existing, err := agentStore.GetAgentContextFiles(ctx, agentID) if err != nil { + slog.Warn("bootstrap: failed to check existing agent files", "agent", agentID, "error", err) return nil, err } @@ -145,12 +146,14 @@ func SeedUserFiles(ctx context.Context, agentStore store.AgentStore, agentID uui // Check existing per-user files to avoid overwriting personalized content existing, err := agentStore.GetUserContextFiles(ctx, agentID, userID) if err != nil { + slog.Warn("bootstrap: failed to check existing user files", "agent", agentID, "user", userID, "error", err) return nil, err } // Early exit: user already has files → not a brand-new user. // Avoids re-seeding BOOTSTRAP.md after auto-cleanup on server restart. if skipIfAnyExist && len(existing) > 0 { + slog.Debug("bootstrap: skip user seed (existing files)", "agent", agentID, "user", userID, "existing", len(existing)) return nil, nil } @@ -221,3 +224,26 @@ func SeedUserFiles(ctx context.Context, agentStore store.AgentStore, agentID uui return seeded, nil } + +// EmbeddedUserFiles returns in-memory context files from embedded templates. +// Used as a fallback when DB seeding fails (e.g. SQLITE_BUSY) so the first +// turn still gets bootstrap onboarding without waiting for DB recovery. +func EmbeddedUserFiles(agentType string) []ContextFile { + files := userSeedFilesOpen + if agentType == store.AgentTypePredefined { + files = userSeedFilesPredefined + } + var result []ContextFile + for _, name := range files { + templateName := name + if agentType == store.AgentTypePredefined && name == BootstrapFile { + templateName = "BOOTSTRAP_PREDEFINED.md" + } + content, err := templateFS.ReadFile(filepath.Join("templates", templateName)) + if err != nil { + continue + } + result = append(result, ContextFile{Path: name, Content: string(content)}) + } + return result +} diff --git a/internal/store/sqlitestore/pool.go b/internal/store/sqlitestore/pool.go index 3b7316f5..429e8811 100644 --- a/internal/store/sqlitestore/pool.go +++ b/internal/store/sqlitestore/pool.go @@ -3,47 +3,89 @@ package sqlitestore import ( + "context" "database/sql" + "database/sql/driver" "fmt" "log/slog" _ "modernc.org/sqlite" // pure-Go SQLite driver ) +// connectionPragmas are applied to EVERY new SQLite connection. +// Critical: PRAGMA settings are per-connection in SQLite. Using db.Exec() +// only applies to ONE connection in the pool — other connections won't have +// busy_timeout, causing immediate SQLITE_BUSY errors under concurrency. +var connectionPragmas = []string{ + "PRAGMA journal_mode = WAL", + "PRAGMA busy_timeout = 15000", + "PRAGMA synchronous = NORMAL", + "PRAGMA cache_size = -8000", // 8MB cache + "PRAGMA foreign_keys = ON", +} + +// pragmaConnector wraps a sql.Driver to apply PRAGMAs on every new connection. +// This ensures ALL connections in the pool have busy_timeout, WAL mode, etc. +type pragmaConnector struct { + driver driver.Driver + dsn string + pragmas []string +} + +func (c *pragmaConnector) Connect(ctx context.Context) (driver.Conn, error) { + conn, err := c.driver.Open(c.dsn) + if err != nil { + return nil, err + } + + // Apply PRAGMAs to this specific connection. + for _, p := range c.pragmas { + if execer, ok := conn.(driver.ExecerContext); ok { + if _, execErr := execer.ExecContext(ctx, p, nil); execErr != nil { + slog.Warn("sqlite pragma failed on new conn", "pragma", p, "error", execErr) + } + } else if execer, ok := conn.(driver.Execer); ok { //nolint:staticcheck + if _, execErr := execer.Exec(p, nil); execErr != nil { + slog.Warn("sqlite pragma failed on new conn", "pragma", p, "error", execErr) + } + } + } + return conn, nil +} + +func (c *pragmaConnector) Driver() driver.Driver { return c.driver } + // OpenDB opens a SQLite database at the given path with WAL mode and recommended pragmas. // Uses modernc.org/sqlite (pure Go, zero CGo). // // Desktop app concurrency model: // - WAL mode: allows concurrent readers alongside a single writer -// - MaxOpenConns(2): limits contention; one reader + one writer can proceed in parallel -// - busy_timeout=15000: writer retries for 15s before SQLITE_BUSY error +// - busy_timeout=15000: ALL connections wait up to 15s before SQLITE_BUSY // - _txlock=immediate: write transactions acquire lock immediately (fail-fast on contention) +// +// PRAGMAs are applied per-connection via pragmaConnector, ensuring every +// connection in the pool has consistent settings (busy_timeout, WAL, etc.). func OpenDB(path string) (*sql.DB, error) { - dsn := fmt.Sprintf("file:%s?_txlock=immediate&_foreign_keys=ON", path) - db, err := sql.Open("sqlite", dsn) + dsn := fmt.Sprintf("file:%s?_txlock=immediate", path) + + // Get the registered driver to wrap with pragmaConnector. + drv, err := getSQLiteDriver() if err != nil { - return nil, fmt.Errorf("open sqlite: %w", err) + return nil, err } + db := sql.OpenDB(&pragmaConnector{ + driver: drv, + dsn: dsn, + pragmas: connectionPragmas, + }) + // SQLite is single-writer; WAL allows concurrent readers. // 4 connections: up to 3 readers + 1 writer can proceed in parallel, // reducing connection pool starvation during concurrent operations. db.SetMaxOpenConns(4) - // Set PRAGMAs explicitly — DSN params may not be applied by modernc.org/sqlite. - pragmas := []string{ - "PRAGMA journal_mode = WAL", - "PRAGMA busy_timeout = 15000", - "PRAGMA synchronous = NORMAL", - "PRAGMA cache_size = -8000", // 8MB cache - } - for _, p := range pragmas { - if _, err := db.Exec(p); err != nil { - slog.Warn("sqlite pragma failed", "pragma", p, "error", err) - } - } - - // Verify connection works. + // Verify connection works (also triggers first pragma application). if err := db.Ping(); err != nil { db.Close() return nil, fmt.Errorf("ping sqlite: %w", err) @@ -51,3 +93,14 @@ func OpenDB(path string) (*sql.DB, error) { return db, nil } + +// getSQLiteDriver retrieves the registered "sqlite" driver instance. +func getSQLiteDriver() (driver.Driver, error) { + db, err := sql.Open("sqlite", ":memory:") + if err != nil { + return nil, fmt.Errorf("get sqlite driver: %w", err) + } + drv := db.Driver() + db.Close() + return drv, nil +}