johno/beads
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Replace getStorageForRequest with s.storage (bd-32)

Browse Source 
- Replaced all getStorageForRequest(req) calls with direct s.storage access
- Updated 5 handler files: server_issues_epics.go (~8 calls), server_labels_deps_comments.go (~4 calls), server_compact.go (~2 calls), server_export_import_auto.go (~2 calls), server_routing_validation_diagnostics.go (~1 call)
- Only remaining references are in server_cache_storage.go (to be deleted in bd-33) and server_eviction_test.go (to be deleted in bd-34)
- Part of bd-29 epic to remove daemon storage cache

Amp-Thread-ID: https://ampcode.com/threads/T-239a5531-68a5-4c98-b85d-0e3512b2553c
Co-authored-by: Amp <amp@ampcode.com>
This commit is contained in:
Steve Yegge
2025-10-27 23:14:38 -07:00
parent 085a64f573
commit bbb1725c32
6 changed files with 19 additions and 125 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
.beads/beads.jsonl
View File

@@ -22,8 +22,8 @@
{"id":"bd-29","title":"Remove Daemon Storage Cache","description":"The daemon's multi-repo storage cache is the root cause of stale data bugs. Since global daemon is deprecated, we only ever serve one repository, making the cache unnecessary complexity. This epic removes the cache entirely for simpler, more reliable direct storage access.","design":"For local daemon (single repository), eliminate the cache entirely:\n- Use s.storage field directly (opened at daemon startup)\n- Remove getStorageForRequest() routing logic\n- Remove server_cache_storage.go entirely (~300 lines)\n- Remove cache-related tests\n- Simplify Server struct\n\nBenefits:\n✅ No staleness bugs: Always using live SQLite connection\n✅ Simpler code: Remove ~300 lines of cache management\n✅ Easier debugging: Direct storage access, no cache indirection\n✅ Same performance: Cache was always 1 entry for local daemon anyway","acceptance_criteria":"- Daemon has no storage cache code\n- All tests pass\n- MCP integration works\n- No stale data bugs\n- Documentation updated\n- Performance validated","status":"open","priority":1,"issue_type":"epic","created_at":"2025-10-27T22:55:10.388648-07:00","updated_at":"2025-10-27T22:55:10.388648-07:00"}
{"id":"bd-3","title":"Investigate and upgrade to modernc.org/sqlite 1.39.1+","description":"We had to pin modernc.org/sqlite to v1.38.2 due to a FOREIGN KEY constraint regression in v1.39.1 (SQLite 3.50.4).\n\n**Issue:** [deleted:bd-47], GH #144\n\n**Symptom:** CloseIssue fails with \"FOREIGN KEY constraint failed (787)\" when called via MCP/daemon, but works fine via CLI.\n\n**Root Cause:** Unknown - likely stricter FK enforcement in SQLite 3.50.4 or modernc.org wrapper changes.\n\n**Workaround:** Pinned to v1.38.2 (SQLite 3.49.x)\n\n**TODO:**\n1. Monitor modernc.org/sqlite releases for fixes\n2. Check SQLite 3.50.5+ changelogs for FK-related fixes\n3. Investigate why daemon mode fails but CLI succeeds (connection reuse? transaction isolation?)\n4. Consider filing upstream issue with reproducible test case\n5. Upgrade when safe","status":"open","priority":2,"issue_type":"task","created_at":"2025-10-24T11:49:12.836292-07:00","updated_at":"2025-10-27T22:22:23.813745-07:00"}
{"id":"bd-30","title":"Audit Current Cache Usage","description":"Understand exactly what code depends on the storage cache","acceptance_criteria":"- Document showing all cache dependencies\n- Confirmation that removing cache won't break MCP\n- List of tests that need updating\n\nFiles to examine:\n- internal/rpc/server_cache_storage.go (cache implementation)\n- internal/rpc/client.go (how req.Cwd is set)\n- internal/rpc/server_*.go (all getStorageForRequest calls)\n- integrations/beads-mcp/ (MCP multi-repo logic)\n\nTasks:\n- Document all callers of getStorageForRequest()\n- Verify req.Cwd is only set by RPC client for database discovery\n- Confirm MCP server doesn't rely on multi-repo cache behavior\n- Check if any tests assume multi-repo routing\n- Review environment variables: BEADS_DAEMON_MAX_CACHE_SIZE, BEADS_DAEMON_CACHE_TTL, BEADS_DAEMON_MEMORY_THRESHOLD_MB","notes":"AUDIT COMPLETE\n\ngetStorageForRequest() callers: 17 production + 11 test\n- server_issues_epics.go: 8 calls\n- server_labels_deps_comments.go: 4 calls \n- server_export_import_auto.go: 2 calls\n- server_compact.go: 2 calls\n- server_routing_validation_diagnostics.go: 1 call\n- server_eviction_test.go: 11 calls (DELETE entire file)\n\nPattern everywhere: store, err := s.getStorageForRequest(req) → store := s.storage\n\nreq.Cwd usage: Only for multi-repo routing. Local daemon always serves 1 repo, so routing is unused.\n\nMCP server: Uses separate daemons per repo (no req.Cwd usage found). NOT affected by cache removal.\n\nCache env vars to deprecate:\n- BEADS_DAEMON_MAX_CACHE_SIZE (used in server_core.go:63)\n- BEADS_DAEMON_CACHE_TTL (used in server_core.go:72)\n- BEADS_DAEMON_MEMORY_THRESHOLD_MB (used in server_cache_storage.go:47)\n\nServer struct fields to remove:\n- storageCache, cacheMu, maxCacheSize, cacheTTL, cleanupTicker, cacheHits, cacheMisses\n\nTests to delete:\n- server_eviction_test.go (entire file - 9 tests)\n- limits_test.go cache assertions\n\nSpecial consideration: ValidateDatabase endpoint uses findDatabaseForCwd() outside cache. Verify if used, then remove or inline.\n\nSafe to proceed with removal - cache always had 1 entry in local daemon model.","status":"closed","priority":1,"issue_type":"task","created_at":"2025-10-27T22:55:19.3723-07:00","updated_at":"2025-10-27T23:04:55.127012-07:00","closed_at":"2025-10-27T23:02:41.30653-07:00","dependencies":[{"issue_id":"bd-30","depends_on_id":"bd-29","type":"parent-child","created_at":"2025-10-27T22:55:19.373816-07:00","created_by":"stevey"}]}
{"id":"bd-31","title":"Remove Storage Cache from Server Struct","description":"Eliminate cache fields and use s.storage directly","acceptance_criteria":"- Server struct has no cache fields\n- NewServer() doesn't initialize cache\n- Start() doesn't run cache cleanup goroutines\n- Stop() only closes single s.storage\n\nChanges needed:\n- Remove cache-related fields from Server struct in server_core.go\n- Remove cache size/TTL parsing from env vars in NewServer()\n- Remove cleanup ticker goroutine from Start()\n- Remove cache cleanup logic from Stop()","status":"open","priority":1,"issue_type":"task","created_at":"2025-10-27T22:55:25.474412-07:00","updated_at":"2025-10-27T22:55:25.474412-07:00","dependencies":[{"issue_id":"bd-31","depends_on_id":"bd-29","type":"parent-child","created_at":"2025-10-27T22:55:25.475344-07:00","created_by":"stevey"}]}
{"id":"bd-32","title":"Replace getStorageForRequest with Direct Access","description":"Replace all getStorageForRequest(req) calls with s.storage","acceptance_criteria":"- No references to getStorageForRequest() in codebase (except in deleted file)\n- All handlers use s.storage directly\n- Code compiles without errors\n\nFiles to update:\n- internal/rpc/server_issues_epics.go (~8 calls)\n- internal/rpc/server_labels_deps_comments.go (~4 calls)\n- internal/rpc/server_compact.go (~2 calls)\n- internal/rpc/server_export_import_auto.go (~2 calls)\n- internal/rpc/server_routing_validation_diagnostics.go (~1 call)\n\nPattern: store, err := s.getStorageForRequest(req) → store := s.storage","status":"open","priority":1,"issue_type":"task","created_at":"2025-10-27T22:55:33.196818-07:00","updated_at":"2025-10-27T22:55:33.196818-07:00","dependencies":[{"issue_id":"bd-32","depends_on_id":"bd-29","type":"parent-child","created_at":"2025-10-27T22:55:33.19824-07:00","created_by":"stevey"},{"issue_id":"bd-32","depends_on_id":"bd-31","type":"blocks","created_at":"2025-10-27T22:55:33.198782-07:00","created_by":"stevey"}]}
{"id":"bd-31","title":"Remove Storage Cache from Server Struct","description":"Eliminate cache fields and use s.storage directly","acceptance_criteria":"- Server struct has no cache fields\n- NewServer() doesn't initialize cache\n- Start() doesn't run cache cleanup goroutines\n- Stop() only closes single s.storage\n\nChanges needed:\n- Remove cache-related fields from Server struct in server_core.go\n- Remove cache size/TTL parsing from env vars in NewServer()\n- Remove cleanup ticker goroutine from Start()\n- Remove cache cleanup logic from Stop()","status":"closed","priority":1,"issue_type":"task","created_at":"2025-10-27T22:55:25.474412-07:00","updated_at":"2025-10-27T23:06:57.205442-07:00","closed_at":"2025-10-27T23:06:57.205442-07:00","dependencies":[{"issue_id":"bd-31","depends_on_id":"bd-29","type":"parent-child","created_at":"2025-10-27T22:55:25.475344-07:00","created_by":"stevey"}]}
{"id":"bd-32","title":"Replace getStorageForRequest with Direct Access","description":"Replace all getStorageForRequest(req) calls with s.storage","acceptance_criteria":"- No references to getStorageForRequest() in codebase (except in deleted file)\n- All handlers use s.storage directly\n- Code compiles without errors\n\nFiles to update:\n- internal/rpc/server_issues_epics.go (~8 calls)\n- internal/rpc/server_labels_deps_comments.go (~4 calls)\n- internal/rpc/server_compact.go (~2 calls)\n- internal/rpc/server_export_import_auto.go (~2 calls)\n- internal/rpc/server_routing_validation_diagnostics.go (~1 call)\n\nPattern: store, err := s.getStorageForRequest(req) → store := s.storage","status":"closed","priority":1,"issue_type":"task","created_at":"2025-10-27T22:55:33.196818-07:00","updated_at":"2025-10-27T23:12:20.944306-07:00","closed_at":"2025-10-27T23:12:20.944306-07:00","dependencies":[{"issue_id":"bd-32","depends_on_id":"bd-29","type":"parent-child","created_at":"2025-10-27T22:55:33.19824-07:00","created_by":"stevey"},{"issue_id":"bd-32","depends_on_id":"bd-31","type":"blocks","created_at":"2025-10-27T22:55:33.198782-07:00","created_by":"stevey"}]}
{"id":"bd-33","title":"Delete server_cache_storage.go","description":"Remove the entire cache implementation file (~286 lines)","acceptance_criteria":"- File deleted from repository\n- No compilation errors\n- No references to deleted functions\n\nFunctions being removed:\n- StorageCacheEntry struct\n- evictStaleStorage() - LRU eviction\n- evictCacheBasedOnMemory() - memory pressure eviction\n- getStorageForRequest() - cache lookup and routing\n- findDatabaseForCwd() - database discovery\n- evictStorageForRequest() - manual eviction","status":"open","priority":1,"issue_type":"task","created_at":"2025-10-27T22:55:38.729299-07:00","updated_at":"2025-10-27T22:55:38.729299-07:00","dependencies":[{"issue_id":"bd-33","depends_on_id":"bd-29","type":"parent-child","created_at":"2025-10-27T22:55:38.730254-07:00","created_by":"stevey"},{"issue_id":"bd-33","depends_on_id":"bd-32","type":"blocks","created_at":"2025-10-27T22:55:38.730747-07:00","created_by":"stevey"}]}
{"id":"bd-34","title":"Remove Cache-Related Tests","description":"Delete or update tests that assume multi-repo caching","acceptance_criteria":"- server_eviction_test.go deleted\n- limits_test.go updated (no cache assertions)\n- All tests pass: go test ./internal/rpc/...\n\nTests to delete:\n- TestCacheEviction\n- TestMemoryPressureEviction\n- TestMtimeInvalidation\n- TestConcurrentCacheAccess\n- TestSubdirectoryCanonicalization\n- TestManualEviction\n- TestLRUEviction\n\nFiles to update:\n- internal/rpc/server_eviction_test.go (DELETE entire file)\n- internal/rpc/limits_test.go (remove cache assertions)","status":"open","priority":1,"issue_type":"task","created_at":"2025-10-27T22:55:44.511897-07:00","updated_at":"2025-10-27T22:55:44.511897-07:00","dependencies":[{"issue_id":"bd-34","depends_on_id":"bd-29","type":"parent-child","created_at":"2025-10-27T22:55:44.512885-07:00","created_by":"stevey"},{"issue_id":"bd-34","depends_on_id":"bd-32","type":"blocks","created_at":"2025-10-27T22:55:44.51336-07:00","created_by":"stevey"}]}
{"id":"bd-35","title":"Update Metrics and Health Endpoints","description":"Remove cache-related metrics from health/metrics endpoints","acceptance_criteria":"- bd daemon --health output has no cache fields\n- bd daemon --metrics output has no cache fields\n- No compilation errors\n\nChanges needed:\n- Remove cache_size from health endpoint in server_routing_validation_diagnostics.go\n- Remove cache_size, cache_hits, cache_misses from metrics endpoint\n- Remove CacheHits and CacheMisses fields from internal/rpc/metrics.go","status":"open","priority":1,"issue_type":"task","created_at":"2025-10-27T22:55:49.212047-07:00","updated_at":"2025-10-27T22:55:49.212047-07:00","dependencies":[{"issue_id":"bd-35","depends_on_id":"bd-29","type":"parent-child","created_at":"2025-10-27T22:55:49.213529-07:00","created_by":"stevey"},{"issue_id":"bd-35","depends_on_id":"bd-32","type":"blocks","created_at":"2025-10-27T22:55:49.214149-07:00","created_by":"stevey"}]}

16
internal/rpc/server_compact.go
View File

@@ -18,13 +18,7 @@ func (s *Server) handleCompact(req *Request) Response {
}
}
store, err := s.getStorageForRequest(req)
if err != nil {
return Response{
Success: false,
Error: fmt.Sprintf("failed to get storage: %v", err),
}
}
store := s.storage
sqliteStore, ok := store.(*sqlite.SQLiteStorage)
if !ok {
@@ -233,13 +227,7 @@ func (s *Server) handleCompactStats(req *Request) Response {
}
}
store, err := s.getStorageForRequest(req)
if err != nil {
return Response{
Success: false,
Error: fmt.Sprintf("failed to get storage: %v", err),
}
}
store := s.storage
sqliteStore, ok := store.(*sqlite.SQLiteStorage)
if !ok {

13
internal/rpc/server_export_import_auto.go
View File

@@ -25,13 +25,7 @@ func (s *Server) handleExport(req *Request) Response {
}
}
store, err := s.getStorageForRequest(req)
if err != nil {
return Response{
Success: false,
Error: fmt.Sprintf("failed to get storage: %v", err),
}
}
store := s.storage
ctx := s.reqCtx(req)
@@ -171,10 +165,7 @@ func (s *Server) handleImport(req *Request) Response {
// This fixes bd-132: daemon shows stale data after git pull
func (s *Server) checkAndAutoImportIfStale(req *Request) error {
// Get storage for this request
store, err := s.getStorageForRequest(req)
if err != nil {
return fmt.Errorf("failed to get storage: %w", err)
}
store := s.storage
ctx := s.reqCtx(req)

64
internal/rpc/server_issues_epics.go
View File

@@ -71,13 +71,7 @@ func (s *Server) handleCreate(req *Request) Response {
}
}
store, err := s.getStorageForRequest(req)
if err != nil {
return Response{
Success: false,
Error: fmt.Sprintf("storage error: %v", err),
}
}
store := s.storage
var design, acceptance, assignee *string
if createArgs.Design != "" {
@@ -181,13 +175,7 @@ func (s *Server) handleUpdate(req *Request) Response {
}
}
store, err := s.getStorageForRequest(req)
if err != nil {
return Response{
Success: false,
Error: fmt.Sprintf("storage error: %v", err),
}
}
store := s.storage
ctx := s.reqCtx(req)
updates := updatesFromArgs(updateArgs)
@@ -226,13 +214,7 @@ func (s *Server) handleClose(req *Request) Response {
}
}
store, err := s.getStorageForRequest(req)
if err != nil {
return Response{
Success: false,
Error: fmt.Sprintf("storage error: %v", err),
}
}
store := s.storage
ctx := s.reqCtx(req)
if err := store.CloseIssue(ctx, closeArgs.ID, closeArgs.Reason, s.reqActor(req)); err != nil {
@@ -259,13 +241,7 @@ func (s *Server) handleList(req *Request) Response {
}
}
store, err := s.getStorageForRequest(req)
if err != nil {
return Response{
Success: false,
Error: fmt.Sprintf("storage error: %v", err),
}
}
store := s.storage
filter := types.IssueFilter{
Limit: listArgs.Limit,
@@ -343,13 +319,7 @@ func (s *Server) handleShow(req *Request) Response {
}
}
store, err := s.getStorageForRequest(req)
if err != nil {
return Response{
Success: false,
Error: fmt.Sprintf("storage error: %v", err),
}
}
store := s.storage
ctx := s.reqCtx(req)
issue, err := store.GetIssue(ctx, showArgs.ID)
@@ -396,13 +366,7 @@ func (s *Server) handleReady(req *Request) Response {
}
}
store, err := s.getStorageForRequest(req)
if err != nil {
return Response{
Success: false,
Error: fmt.Sprintf("storage error: %v", err),
}
}
store := s.storage
wf := types.WorkFilter{
Status: types.StatusOpen,
@@ -431,13 +395,7 @@ func (s *Server) handleReady(req *Request) Response {
}
func (s *Server) handleStats(req *Request) Response {
store, err := s.getStorageForRequest(req)
if err != nil {
return Response{
Success: false,
Error: fmt.Sprintf("storage error: %v", err),
}
}
store := s.storage
ctx := s.reqCtx(req)
stats, err := store.GetStatistics(ctx)
@@ -464,13 +422,7 @@ func (s *Server) handleEpicStatus(req *Request) Response {
}
}
store, err := s.getStorageForRequest(req)
if err != nil {
return Response{
Success: false,
Error: fmt.Sprintf("storage error: %v", err),
}
}
store := s.storage
ctx := s.reqCtx(req)
epics, err := store.GetEpicsEligibleForClosure(ctx)

32
internal/rpc/server_labels_deps_comments.go
View File

@@ -18,13 +18,7 @@ func (s *Server) handleDepAdd(req *Request) Response {
}
}
store, err := s.getStorageForRequest(req)
if err != nil {
return Response{
Success: false,
Error: fmt.Sprintf("storage error: %v", err),
}
}
store := s.storage
dep := &types.Dependency{
IssueID: depArgs.FromID,
@@ -53,13 +47,7 @@ func (s *Server) handleSimpleStoreOp(req *Request, argsPtr interface{}, argDesc
}
}
store, err := s.getStorageForRequest(req)
if err != nil {
return Response{
Success: false,
Error: fmt.Sprintf("storage error: %v", err),
}
}
store := s.storage
ctx := s.reqCtx(req)
if err := opFunc(ctx, store, s.reqActor(req)); err != nil {
@@ -102,13 +90,7 @@ func (s *Server) handleCommentList(req *Request) Response {
}
}
store, err := s.getStorageForRequest(req)
if err != nil {
return Response{
Success: false,
Error: fmt.Sprintf("storage error: %v", err),
}
}
store := s.storage
ctx := s.reqCtx(req)
comments, err := store.GetIssueComments(ctx, commentArgs.ID)
@@ -135,13 +117,7 @@ func (s *Server) handleCommentAdd(req *Request) Response {
}
}
store, err := s.getStorageForRequest(req)
if err != nil {
return Response{
Success: false,
Error: fmt.Sprintf("storage error: %v", err),
}
}
store := s.storage
ctx := s.reqCtx(req)
comment, err := store.AddIssueComment(ctx, commentArgs.ID, commentArgs.Author, commentArgs.Text)

15
internal/rpc/server_routing_validation_diagnostics.go
View File

@@ -294,20 +294,7 @@ func (s *Server) handleHealth(req *Request) Response {
var m runtime.MemStats
runtime.ReadMemStats(&m)
store, err := s.getStorageForRequest(req)
if err != nil {
data, _ := json.Marshal(HealthResponse{
Status: "unhealthy",
Version: ServerVersion,
Uptime: time.Since(s.startTime).Seconds(),
Error: fmt.Sprintf("storage error: %v", err),
})
return Response{
Success: false,
Data: data,
Error: fmt.Sprintf("storage error: %v", err),
}
}
store := s.storage
healthCtx, cancel := context.WithTimeout(context.Background(), 1*time.Second)
defer cancel()