.

Author: BnjmnZmmrmn-parafin

.forge/FORGE_PR_DESCRIPTION.md

@@ -0,0 +1,32 @@
+## Summary
+Fix a panic on `/new` caused by SQLite lock contention when `ForgeRepo` created a second connection pool pointing at the same `.forge.db` file while the previous pool was still alive.
+
+## Context
+Every time a user ran `/new`, `ForgeRepo::new` created a brand-new `DatabasePool` via `DatabasePool::try_from(...).unwrap()`. The old pool — still held alive by `tokio::spawn`'d hydration tasks — would hold open connections to the same SQLite file. r2d2's pool builder validates connections at construction time with a 5-second timeout; after 5 retries the error bubbled back to the `.unwrap()`, crashing the process with:
+
+```
+ERROR: called `Result::unwrap()` on an `Err` value: Failed to create connection pool: timed out waiting for connection
+```
+
+## Changes
+- **`forge_repo`**: `ForgeRepo::new` now accepts an `Arc<DatabasePool>` instead of constructing its own. The `DatabasePool` and `PoolConfig` types are re-exported from the crate root so callers don't need to reach into internal modules.
+- **`forge_api`**: `ForgeAPI::init` creates the pool once and passes it to `ForgeRepo::new`. The return type changes from `Self` to `Result<Self>` so a pool failure produces a clean error message instead of a panic.
+- **`forge_main`**: `UI`'s factory closure type changes from `Fn(ForgeConfig) -> A` to `Fn(ForgeConfig) -> Result<A>` to accommodate the fallible `ForgeAPI::init`. `on_new` propagates the error via `?` instead of unwrapping.
+
+### Key Implementation Details
+The pool is now created once per `forge` process — inside `ForgeAPI::init` using `ConfigReader::base_path().join(".forge.db")`, which is the canonical path already used by `Environment::database_path()`. On each `/new`, a new `ForgeRepo` is built pointing at the same `Arc<DatabasePool>`, so the underlying SQLite file is only ever opened once and there is no contention window.
+
+## Testing
+```bash
+# Build
+cargo build -p forge_repo -p forge_api -p forge_main
+
+# Manually verify: start forge, run /new multiple times, confirm no crash
+forge
+/new
+/new
+/new
+```
+
+## Links
+- Root cause: `crates/forge_repo/src/forge_repo.rs` — the `.unwrap()` on pool construction that caused the panic

Cargo.lock

@@ -13,7 +13,7 @@ use forge_app::{
 use forge_config::ForgeConfig;
 use forge_domain::{Agent, ConsoleWriter, *};
 use forge_infra::ForgeInfra;
-use forge_repo::ForgeRepo;
+use forge_repo::{DatabasePool, ForgeRepo};
 use forge_services::ForgeServices;
 use forge_stream::MpscStream;
 use futures::stream::BoxStream;
@@ -42,17 +42,26 @@ impl<A, F> ForgeAPI<A, F> {
 }
 
 impl ForgeAPI<ForgeServices<ForgeRepo<ForgeInfra>>, ForgeRepo<ForgeInfra>> {
-    /// Creates a fully-initialized [`ForgeAPI`] from a pre-read configuration.
+    /// Creates a fully-initialized [`ForgeAPI`] from a pre-read configuration
+    /// and a shared database pool.
+    ///
+    /// The pool is created once in `main` and passed in here so that a single
+    /// SQLite connection pool is reused across `/new` conversation resets,
+    /// eliminating the lock contention that previously caused a panic.
     ///
     /// # Arguments
     /// * `cwd` - The working directory path for environment and file resolution
     /// * `config` - Pre-read application configuration (from startup)
-    /// * `services_url` - Pre-validated URL for the gRPC workspace server
-    pub fn init(cwd: PathBuf, config: ForgeConfig) -> Self {
+    /// * `db_pool` - Shared database pool created once at process startup
+    ///
+    /// # Errors
+    ///
+    /// Returns an error if the API cannot be initialised.
+    pub fn init(cwd: PathBuf, config: ForgeConfig, db_pool: Arc<DatabasePool>) -> Result<Self> {
         let infra = Arc::new(ForgeInfra::new(cwd, config));
-        let repo = Arc::new(ForgeRepo::new(infra.clone()));
+        let repo = Arc::new(ForgeRepo::new(infra.clone(), db_pool));
         let app = Arc::new(ForgeServices::new(repo.clone()));
-        ForgeAPI::new(app, repo)
+        Ok(ForgeAPI::new(app, repo))
     }
 
     pub async fn get_skills_internal(&self) -> Result<Vec<Skill>> {

crates/forge_api/src/forge_api.rs

@@ -23,6 +23,7 @@ forge_display.workspace = true
 forge_tracker.workspace = true
 
 forge_spinner.workspace = true
+forge_repo.workspace = true
 forge_select.workspace = true
 
 merge.workspace = true

crates/forge_main/Cargo.toml

@@ -1,6 +1,7 @@
 use std::io::Read;
 use std::panic;
 use std::path::PathBuf;
+use std::sync::Arc;
 
 use anyhow::{Context, Result};
 use clap::Parser;
@@ -120,8 +121,21 @@ async fn run() -> Result<()> {
         (_, _) => std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")),
     };
 
+    // Create the database pool once for the entire process lifetime.  The same
+    // Arc<DatabasePool> is cloned into every ForgeAPI created on /new, so the
+    // SQLite file is opened exactly once and there is no contention window.
+    //
+    // Note: ConfigReader::base_path().join(".forge.db") is identical to what
+    // Environment::database_path() returns — both derive from the same base path.
+    let db_pool = Arc::new(
+        forge_repo::DatabasePool::try_from(forge_repo::PoolConfig::new(
+            forge_config::ConfigReader::base_path().join(".forge.db"),
+        ))
+        .context("Failed to open Forge database")?,
+    );
+
     let mut ui = UI::init(cli, config, move |config| {
-        ForgeAPI::init(cwd.clone(), config)
+        ForgeAPI::init(cwd.clone(), config, db_pool.clone())
     })?;
     ui.run().await;
 

crates/forge_main/src/main.rs

@@ -101,10 +101,10 @@ fn format_mcp_headers(server: &forge_domain::McpServerConfig) -> Option<String>
     }
 }
 
-pub struct UI<A: ConsoleWriter, F: Fn(ForgeConfig) -> A> {
+pub struct UI<A: ConsoleWriter, F: Fn(ForgeConfig) -> anyhow::Result<A>> {
     markdown: MarkdownFormat,
     state: UIState,
-    api: Arc<F::Output>,
+    api: Arc<A>,
     new_api: Arc<F>,
     console: Console,
     command: Arc<ForgeCommandManager>,
@@ -115,7 +115,7 @@ pub struct UI<A: ConsoleWriter, F: Fn(ForgeConfig) -> A> {
     _guard: forge_tracker::Guard,
 }
 
-impl<A: API + ConsoleWriter + 'static, F: Fn(ForgeConfig) -> A + Send + Sync> UI<A, F> {
+impl<A: API + ConsoleWriter + 'static, F: Fn(ForgeConfig) -> anyhow::Result<A> + Send + Sync> UI<A, F> {
     /// Writes a line to the console output
     /// Takes anything that implements ToString trait
     fn writeln<T: ToString>(&mut self, content: T) -> anyhow::Result<()> {
@@ -161,7 +161,7 @@ impl<A: API + ConsoleWriter + 'static, F: Fn(ForgeConfig) -> A + Send + Sync> UI
     async fn on_new(&mut self) -> Result<()> {
         let config = forge_config::ForgeConfig::read().unwrap_or_default();
         self.config = config.clone();
-        self.api = Arc::new((self.new_api)(config));
+        self.api = Arc::new((self.new_api)(config)?);
         self.init_state(false).await?;
 
         // Set agent if provided via CLI
@@ -216,7 +216,7 @@ impl<A: API + ConsoleWriter + 'static, F: Fn(ForgeConfig) -> A + Send + Sync> UI
     ///   from `forge config set` are reflected in new conversations
     pub fn init(cli: Cli, config: ForgeConfig, f: F) -> Result<Self> {
         // Parse CLI arguments first to get flags
-        let api = Arc::new(f(config.clone()));
+        let api = Arc::new(f(config.clone())?);
         let env = api.environment();
         let command = Arc::new(ForgeCommandManager::default());
         let spinner = SharedSpinner::new(SpinnerManager::new(api.clone()));

crates/forge_main/src/ui.rs

@@ -1,4 +1,3 @@
-#![allow(dead_code)]
 use std::path::PathBuf;
 use std::time::Duration;
 

crates/forge_repo/src/database/pool.rs

@@ -26,7 +26,7 @@ use url::Url;
 use crate::agent::ForgeAgentRepository;
 use crate::context_engine::ForgeContextEngineRepository;
 use crate::conversation::ConversationRepositoryImpl;
-use crate::database::{DatabasePool, PoolConfig};
+use crate::database::DatabasePool;
 use crate::fs_snap::ForgeFileSnapshotService;
 use crate::fuzzy_search::ForgeFuzzySearchRepository;
 use crate::provider::{ForgeChatRepository, ForgeProviderRepository};
@@ -60,13 +60,17 @@ impl<
         + HttpInfra,
 > ForgeRepo<F>
 {
-    pub fn new(infra: Arc<F>) -> Self {
+    /// Creates a new [`ForgeRepo`] with the provided infrastructure and a
+    /// shared database pool.
+    ///
+    /// The pool is created once at application startup and reused across
+    /// `/new` conversation resets so that a fresh `ForgeRepo` never races
+    /// with the previous instance for the same SQLite file.
+    pub fn new(infra: Arc<F>, db_pool: Arc<DatabasePool>) -> Self {
         let env = infra.get_environment();
         let file_snapshot_service = Arc::new(ForgeFileSnapshotService::new(env.clone()));
-        let db_pool =
-            Arc::new(DatabasePool::try_from(PoolConfig::new(env.database_path())).unwrap());
         let conversation_repository = Arc::new(ConversationRepositoryImpl::new(
-            db_pool.clone(),
+            db_pool,
             env.workspace_hash(),
         ));
 

crates/forge_repo/src/forge_repo.rs

@@ -14,5 +14,8 @@ mod proto_generated {
     tonic::include_proto!("forge.v1");
 }
 
+// Expose the database pool types so callers (e.g. forge_api) can construct
+// and share a pool without depending on internal module paths.
+pub use database::{DatabasePool, PoolConfig};
 // Only expose forge_repo container
 pub use forge_repo::*;