From 4ccdd7a2df36199756f8ce40fd8e01399b8729a6 Mon Sep 17 00:00:00 2001
From: Eugene Sukhodolin <eugene@sukhodolin.com>
Date: Tue, 20 Jan 2026 14:05:40 -0800
Subject: [PATCH] fix(routing): close original store before replacing with
 target store (#1215)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* fix(routing): close original store before replacing with target store

Fix "database is closed" error during auto-flush when contributor routing
redirects issue creation to a different repository (e.g., ~/.beads-planning).

When a user is detected as a contributor (based on git remote URL pattern),
the default configuration routes new issues to ~/.beads-planning instead of
the current repository. This routing caused auto-flush to fail with:

  Warning: auto-flush failed: failed to get stored JSONL hash:
  failed to get jsonl_file_hash: sql: database is closed

In create.go, when switching to the planning repo:
1. targetStore was opened for the planning database
2. A defer was set to close targetStore at function end
3. store = targetStore replaced the global store pointer
4. Issue was created successfully
5. Command Run ended → defer closed targetStore
6. PersistentPostRun tried to flush using the global store → error!

The bug: the defer closed targetStore, but the global `store` variable
pointed to the same object. When PersistentPostRun called flushManager.Shutdown(),
it tried to use the already-closed store for the flush operation.

- Remove the defer that prematurely closed targetStore
- Explicitly close the ORIGINAL store before replacing it (it won't be used)
- Let PersistentPostRun close whatever store is current at exit time

This ensures proper store lifecycle:
- Original store: closed when replaced (no longer needed)
- Target store: closed by PersistentPostRun after flush completes

1. Have a git repo with a remote URL that doesn't match maintainer patterns
   (e.g., github-a:org/repo.git instead of git@github.com:org/repo.git)
2. Have ~/.beads-planning directory with beads initialized
3. Run: cd /path/to/repo && bd create "Test issue"
4. Observe: Issue created successfully, but followed by:
   Warning: auto-flush failed: ... sql: database is closed

Added debug prints to SQLiteStorage.Close() and GetJSONLFileHash() which
revealed two different databases were involved:
- /path/to/repo/.beads/beads.db (closed first, unexpectedly)
- ~/.beads-planning/.beads/beads.db (used for flush after being closed)

* fix(test): resolve race condition in TestAutoFlushOnExit

Not directly related to the previous routing fix, but exposed by CI timing.

Root cause: The test had a latent race condition between two channel operations:
1. markDirtyAndScheduleFlush() sends event to markDirtyCh
2. Shutdown() sends request to shutdownCh

Without any delay, Go's scheduler might process the shutdown event first.
Since isDirty was still false (markDirty event not yet processed), the
FlushManager would skip the flush entirely, causing the test to fail with
"Expected JSONL file to be created on exit".

The race was always present but only manifested in CI environments with
different timing characteristics (CPU contention, virtualization, scheduler
behavior).

Fix: Add a 10ms sleep after markDirtyAndScheduleFlush() to allow the
FlushManager's background goroutine to process the event before shutdown.
This mimics real-world behavior where there's always some time between
CRUD operations and process exit.
---
 cmd/bd/create.go    | 13 ++++++++-----
 cmd/bd/main_test.go |  6 ++++++
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/cmd/bd/create.go b/cmd/bd/create.go
index 545c0b7a..00ba9916 100644
--- a/cmd/bd/create.go
+++ b/cmd/bd/create.go
@@ -361,11 +361,14 @@ var createCmd = &cobra.Command{
 			if err != nil {
 				FatalError("failed to open target store: %v", err)
 			}
-			defer func() {
-				if err := targetStore.Close(); err != nil {
-					fmt.Fprintf(os.Stderr, "warning: failed to close target store: %v\n", err)
-				}
-			}()
+
+			// Close the original store before replacing it (it won't be used anymore)
+			// Note: We don't defer-close targetStore here because PersistentPostRun
+			// will close whatever store is assigned to the global `store` variable.
+			// This fixes the "database is closed" error during auto-flush (GH#routing-close-bug).
+			if store != nil {
+				_ = store.Close()
+			}
 
 			// Replace store for remainder of create operation
 			// This also bypasses daemon mode since daemon owns the current repo's store
diff --git a/cmd/bd/main_test.go b/cmd/bd/main_test.go
index b94291b0..e0ce4744 100644
--- a/cmd/bd/main_test.go
+++ b/cmd/bd/main_test.go
@@ -76,6 +76,12 @@ func TestAutoFlushOnExit(t *testing.T) {
 	// Mark dirty (simulating CRUD operation)
 	markDirtyAndScheduleFlush()
 
+	// Allow time for the FlushManager's background goroutine to process the markDirty event.
+	// Without this, there's a race condition: if Shutdown() is processed before the markDirty
+	// event, isDirty remains false and no flush occurs. This mimics real-world behavior where
+	// there's always some time between CRUD operations and process exit.
+	time.Sleep(10 * time.Millisecond)
+
 	// Simulate PersistentPostRun exit behavior - shutdown FlushManager
 	// This performs the final flush before exit
 	if err := flushManager.Shutdown(); err != nil {