From 352b9f7e6b1f2110d1f078803598052e5260db12 Mon Sep 17 00:00:00 2001 From: Steve Yegge Date: Wed, 26 Nov 2025 11:14:59 -0800 Subject: [PATCH] bd sync: 2025-11-26 11:14:59 --- .beads/beads.jsonl | 2 +- internal/storage/memory/memory.go | 104 ++++++++++++------ internal/storage/memory/memory_test.go | 142 +++++++++++++++++++++++++ 3 files changed, 216 insertions(+), 32 deletions(-) diff --git a/.beads/beads.jsonl b/.beads/beads.jsonl index d8c27fcc..91f95740 100644 --- a/.beads/beads.jsonl +++ b/.beads/beads.jsonl @@ -9,7 +9,7 @@ {"id":"bd-4l5","title":"bd prime: Detect ephemeral branches and adjust workflow output","description":"When 'bd prime' runs on a branch with no upstream (ephemeral branch), it should output a different SESSION CLOSE PROTOCOL.\n\n**Current output (wrong for ephemeral branches):**\n```\n[ ] 1. git status\n[ ] 2. git add \u003cfiles\u003e\n[ ] 3. bd sync\n[ ] 4. git commit -m \"...\"\n[ ] 5. bd sync\n[ ] 6. git push\n```\n\n**Needed output for ephemeral branches:**\n```\n[ ] 1. git status\n[ ] 2. git add \u003cfiles\u003e\n[ ] 3. bd sync --from-main (pull updates from main)\n[ ] 4. git commit -m \"...\"\n[ ] 5. (no push - branch is ephemeral)\n```\n\n**Detection:** `git rev-parse --abbrev-ref --symbolic-full-name @{u}` returns error code 128 if no upstream.\n\nAlso update Sync \u0026 Collaboration section to mention `bd sync --from-main` for ephemeral branches.\n\n**Use case:** Gastown polecats work on ephemeral local branches that are never pushed. Their code gets merged to main via local merge, and beads changes stay local (communicated via gm mail to Overseer).","status":"closed","priority":1,"issue_type":"feature","created_at":"2025-11-25T16:55:24.984104-08:00","updated_at":"2025-11-25T17:12:46.604978-08:00","closed_at":"2025-11-25T17:12:46.604978-08:00"} {"id":"bd-736d","title":"Refactor path canonicalization into helper function","description":"The path canonicalization logic (filepath.Abs + EvalSymlinks) is duplicated in 3 places:\n- beads.go:131-137 (BEADS_DIR handling)\n- cmd/bd/main.go:446-451 (--no-db cleanup)\n- cmd/bd/nodb.go:26-31 (--no-db initialization)\n\nRefactoring suggestion:\nExtract to a helper function like:\n func canonicalizePath(path string) string\n\nThis would:\n- Reduce code duplication\n- Make the logic easier to maintain\n- Ensure consistent behavior across all path handling\n\nRelated to bd-e16b implementation.","status":"closed","priority":3,"issue_type":"chore","created_at":"2025-11-02T18:33:47.727443-08:00","updated_at":"2025-11-25T22:27:33.738672-08:00","closed_at":"2025-11-25T22:27:33.738672-08:00"} {"id":"bd-81a","title":"Add programmatic tip injection API","description":"Allow tips to be programmatically injected at runtime based on detected conditions. This enables dynamic tips (not just pre-defined ones) to be shown with custom priority and frequency.","status":"closed","priority":2,"issue_type":"feature","created_at":"2025-11-11T23:29:46.645583-08:00","updated_at":"2025-11-25T17:52:35.096882-08:00","closed_at":"2025-11-25T17:52:35.096882-08:00","dependencies":[{"issue_id":"bd-81a","depends_on_id":"bd-d4i","type":"blocks","created_at":"2025-11-11T23:29:46.646327-08:00","created_by":"daemon"}]} -{"id":"bd-9e23","title":"Optimize Memory backend GetIssueByExternalRef with index","description":"Currently GetIssueByExternalRef in Memory storage uses O(n) linear search through all issues.\n\nCurrent code (memory.go:282-308):\nfor _, issue := range m.issues {\n if issue.ExternalRef != nil \u0026\u0026 *issue.ExternalRef == externalRef {\n return \u0026issueCopy, nil\n }\n}\n\nProposed optimization:\n- Add externalRefToID map[string]string to MemoryStorage\n- Maintain it in CreateIssue, UpdateIssue, DeleteIssue\n- Achieve O(1) lookup like SQLite's index\n\nImpact: Low (--no-db mode typically has smaller datasets)\nRelated: bd-1022","status":"open","priority":4,"issue_type":"chore","created_at":"2025-11-02T15:32:30.242357-08:00","updated_at":"2025-11-02T15:32:30.242357-08:00"} +{"id":"bd-9e23","title":"Optimize Memory backend GetIssueByExternalRef with index","description":"Currently GetIssueByExternalRef in Memory storage uses O(n) linear search through all issues.\n\nCurrent code (memory.go:282-308):\nfor _, issue := range m.issues {\n if issue.ExternalRef != nil \u0026\u0026 *issue.ExternalRef == externalRef {\n return \u0026issueCopy, nil\n }\n}\n\nProposed optimization:\n- Add externalRefToID map[string]string to MemoryStorage\n- Maintain it in CreateIssue, UpdateIssue, DeleteIssue\n- Achieve O(1) lookup like SQLite's index\n\nImpact: Low (--no-db mode typically has smaller datasets)\nRelated: bd-1022","status":"closed","priority":4,"issue_type":"chore","created_at":"2025-11-02T15:32:30.242357-08:00","updated_at":"2025-11-26T11:14:49.172418-08:00","closed_at":"2025-11-26T11:14:49.172418-08:00"} {"id":"bd-9li4","title":"Create Docker image for Agent Mail","description":"Containerize Agent Mail server for easy deployment.\n\nAcceptance Criteria:\n- Dockerfile with Python 3.14\n- Health check endpoint\n- Volume mount for storage\n- Environment variable configuration\n- Multi-arch builds (amd64, arm64)\n\nFile: deployment/agent-mail/Dockerfile","status":"closed","priority":3,"issue_type":"task","created_at":"2025-11-07T22:43:43.231964-08:00","updated_at":"2025-11-25T17:47:30.777486-08:00","closed_at":"2025-11-25T17:47:30.777486-08:00"} {"id":"bd-b8h","title":"Refactor check-health DB access to avoid repeated path resolution","description":"The runCheckHealth lightweight checks (hintsDisabled, checkVersionMismatch, checkSyncBranchQuick) each have duplicated database path resolution logic. Extract a helper function to DRY this up.","status":"closed","priority":2,"issue_type":"task","created_at":"2025-11-25T19:27:35.075929-08:00","updated_at":"2025-11-25T19:50:21.272961-08:00","closed_at":"2025-11-25T19:50:21.272961-08:00"} {"id":"bd-bgs","title":"Git history fallback doesn't escape regex special chars in IDs","description":"## Problem\n\nIn `batchCheckGitHistory`, IDs are directly interpolated into a regex pattern:\n\n```go\npatterns = append(patterns, fmt.Sprintf(\\`\"id\":\"%s\"\\`, id))\nsearchPattern := strings.Join(patterns, \"|\")\ncmd := exec.Command(\"git\", \"log\", \"--all\", \"-G\", searchPattern, ...)\n```\n\nIf an ID contains regex special characters (e.g., `bd-foo.bar` or `bd-test+1`), the pattern will be malformed or match unintended strings.\n\n## Location\n`internal/importer/importer.go:923-926`\n\n## Impact\n- False positives: IDs with `.` could match any character\n- Regex errors: IDs with `[` or `(` could cause git to fail\n- Security: potential for regex injection (low risk since IDs are validated)\n\n## Fix\nEscape regex special characters:\n\n```go\nimport \"regexp\"\n\nescapedID := regexp.QuoteMeta(id)\npatterns = append(patterns, fmt.Sprintf(\\`\"id\":\"%s\"\\`, escapedID))\n```","status":"closed","priority":2,"issue_type":"bug","created_at":"2025-11-25T12:50:30.132232-08:00","updated_at":"2025-11-25T15:04:06.217695-08:00","closed_at":"2025-11-25T15:04:06.217695-08:00"} diff --git a/internal/storage/memory/memory.go b/internal/storage/memory/memory.go index 4cb6936a..179e1a12 100644 --- a/internal/storage/memory/memory.go +++ b/internal/storage/memory/memory.go @@ -30,6 +30,9 @@ type MemoryStorage struct { metadata map[string]string // Metadata key-value pairs counters map[string]int // Prefix -> Last ID + // Indexes for O(1) lookups + externalRefToID map[string]string // ExternalRef -> IssueID + // For tracking dirty map[string]bool // IssueIDs that have been modified @@ -40,16 +43,17 @@ type MemoryStorage struct { // New creates a new in-memory storage backend func New(jsonlPath string) *MemoryStorage { return &MemoryStorage{ - issues: make(map[string]*types.Issue), - dependencies: make(map[string][]*types.Dependency), - labels: make(map[string][]string), - events: make(map[string][]*types.Event), - comments: make(map[string][]*types.Comment), - config: make(map[string]string), - metadata: make(map[string]string), - counters: make(map[string]int), - dirty: make(map[string]bool), - jsonlPath: jsonlPath, + issues: make(map[string]*types.Issue), + dependencies: make(map[string][]*types.Dependency), + labels: make(map[string][]string), + events: make(map[string][]*types.Event), + comments: make(map[string][]*types.Comment), + config: make(map[string]string), + metadata: make(map[string]string), + counters: make(map[string]int), + externalRefToID: make(map[string]string), + dirty: make(map[string]bool), + jsonlPath: jsonlPath, } } @@ -67,6 +71,11 @@ func (m *MemoryStorage) LoadFromIssues(issues []*types.Issue) error { // Store the issue m.issues[issue.ID] = issue + // Index external ref for O(1) lookup + if issue.ExternalRef != nil && *issue.ExternalRef != "" { + m.externalRefToID[*issue.ExternalRef] = issue.ID + } + // Store dependencies if len(issue.Dependencies) > 0 { m.dependencies[issue.ID] = issue.Dependencies @@ -184,6 +193,11 @@ func (m *MemoryStorage) CreateIssue(ctx context.Context, issue *types.Issue, act m.issues[issue.ID] = issue m.dirty[issue.ID] = true + // Index external ref for O(1) lookup + if issue.ExternalRef != nil && *issue.ExternalRef != "" { + m.externalRefToID[*issue.ExternalRef] = issue.ID + } + // Record event event := &types.Event{ IssueID: issue.ID, @@ -244,6 +258,11 @@ func (m *MemoryStorage) CreateIssues(ctx context.Context, issues []*types.Issue, m.issues[issue.ID] = issue m.dirty[issue.ID] = true + // Index external ref for O(1) lookup + if issue.ExternalRef != nil && *issue.ExternalRef != "" { + m.externalRefToID[*issue.ExternalRef] = issue.ID + } + // Record event event := &types.Event{ IssueID: issue.ID, @@ -288,28 +307,31 @@ func (m *MemoryStorage) GetIssueByExternalRef(ctx context.Context, externalRef s m.mu.RLock() defer m.mu.RUnlock() - // Linear search through all issues to find match by external_ref - for _, issue := range m.issues { - if issue.ExternalRef != nil && *issue.ExternalRef == externalRef { - // Return a copy to avoid mutations - issueCopy := *issue - - // Attach dependencies - if deps, ok := m.dependencies[issue.ID]; ok { - issueCopy.Dependencies = deps - } - - // Attach labels - if labels, ok := m.labels[issue.ID]; ok { - issueCopy.Labels = labels - } - - return &issueCopy, nil - } + // O(1) lookup using index + issueID, exists := m.externalRefToID[externalRef] + if !exists { + return nil, nil } - // Not found - return nil, nil + issue, exists := m.issues[issueID] + if !exists { + return nil, nil + } + + // Return a copy to avoid mutations + issueCopy := *issue + + // Attach dependencies + if deps, ok := m.dependencies[issue.ID]; ok { + issueCopy.Dependencies = deps + } + + // Attach labels + if labels, ok := m.labels[issue.ID]; ok { + issueCopy.Labels = labels + } + + return &issueCopy, nil } // UpdateIssue updates fields on an issue @@ -375,9 +397,23 @@ func (m *MemoryStorage) UpdateIssue(ctx context.Context, id string, updates map[ issue.Assignee = "" } case "external_ref": + // Update external ref index + oldRef := issue.ExternalRef if v, ok := value.(string); ok { + // Remove old index entry if exists + if oldRef != nil && *oldRef != "" { + delete(m.externalRefToID, *oldRef) + } + // Add new index entry + if v != "" { + m.externalRefToID[v] = id + } issue.ExternalRef = &v } else if value == nil { + // Remove old index entry if exists + if oldRef != nil && *oldRef != "" { + delete(m.externalRefToID, *oldRef) + } issue.ExternalRef = nil } } @@ -417,10 +453,16 @@ func (m *MemoryStorage) DeleteIssue(ctx context.Context, id string) error { defer m.mu.Unlock() // Check if issue exists - if _, ok := m.issues[id]; !ok { + issue, ok := m.issues[id] + if !ok { return fmt.Errorf("issue not found: %s", id) } + // Remove external ref index entry + if issue.ExternalRef != nil && *issue.ExternalRef != "" { + delete(m.externalRefToID, *issue.ExternalRef) + } + // Delete the issue delete(m.issues, id) diff --git a/internal/storage/memory/memory_test.go b/internal/storage/memory/memory_test.go index 94d564cd..41d6e811 100644 --- a/internal/storage/memory/memory_test.go +++ b/internal/storage/memory/memory_test.go @@ -879,3 +879,145 @@ func TestClose(t *testing.T) { t.Error("Store should be closed") } } + +func TestGetIssueByExternalRef(t *testing.T) { + store := setupTestMemory(t) + defer store.Close() + + ctx := context.Background() + + // Create an issue with external ref + extRef := "github#123" + issue := &types.Issue{ + Title: "Test issue with external ref", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeTask, + ExternalRef: &extRef, + } + + if err := store.CreateIssue(ctx, issue, "test-user"); err != nil { + t.Fatalf("CreateIssue failed: %v", err) + } + + // Lookup by external ref should find it + found, err := store.GetIssueByExternalRef(ctx, "github#123") + if err != nil { + t.Fatalf("GetIssueByExternalRef failed: %v", err) + } + if found == nil { + t.Fatal("Expected to find issue by external ref") + } + if found.ID != issue.ID { + t.Errorf("Expected issue ID %s, got %s", issue.ID, found.ID) + } + + // Lookup by non-existent ref should return nil + notFound, err := store.GetIssueByExternalRef(ctx, "nonexistent") + if err != nil { + t.Fatalf("GetIssueByExternalRef failed: %v", err) + } + if notFound != nil { + t.Error("Expected nil for non-existent external ref") + } + + // Update external ref and verify index is updated + newRef := "github#456" + if err := store.UpdateIssue(ctx, issue.ID, map[string]interface{}{ + "external_ref": newRef, + }, "test-user"); err != nil { + t.Fatalf("UpdateIssue failed: %v", err) + } + + // Old ref should not find anything + oldRefResult, err := store.GetIssueByExternalRef(ctx, "github#123") + if err != nil { + t.Fatalf("GetIssueByExternalRef failed: %v", err) + } + if oldRefResult != nil { + t.Error("Old external ref should not find issue after update") + } + + // New ref should find the issue + newRefResult, err := store.GetIssueByExternalRef(ctx, "github#456") + if err != nil { + t.Fatalf("GetIssueByExternalRef failed: %v", err) + } + if newRefResult == nil { + t.Fatal("New external ref should find issue") + } + if newRefResult.ID != issue.ID { + t.Errorf("Expected issue ID %s, got %s", issue.ID, newRefResult.ID) + } + + // Delete issue and verify index is cleaned up + if err := store.DeleteIssue(ctx, issue.ID); err != nil { + t.Fatalf("DeleteIssue failed: %v", err) + } + + // External ref should not find anything after delete + deletedResult, err := store.GetIssueByExternalRef(ctx, "github#456") + if err != nil { + t.Fatalf("GetIssueByExternalRef failed: %v", err) + } + if deletedResult != nil { + t.Error("External ref should not find issue after delete") + } +} + +func TestGetIssueByExternalRefLoadFromIssues(t *testing.T) { + store := New("") + defer store.Close() + + ctx := context.Background() + + // Load issues with external refs + extRef1 := "jira#100" + extRef2 := "jira#200" + issues := []*types.Issue{ + { + ID: "bd-1", + Title: "Issue 1", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeTask, + ExternalRef: &extRef1, + }, + { + ID: "bd-2", + Title: "Issue 2", + Status: types.StatusOpen, + Priority: 2, + IssueType: types.TypeBug, + ExternalRef: &extRef2, + }, + { + ID: "bd-3", + Title: "Issue 3 (no external ref)", + Status: types.StatusOpen, + Priority: 3, + IssueType: types.TypeFeature, + }, + } + + if err := store.LoadFromIssues(issues); err != nil { + t.Fatalf("LoadFromIssues failed: %v", err) + } + + // Both external refs should be indexed + found1, err := store.GetIssueByExternalRef(ctx, "jira#100") + if err != nil { + t.Fatalf("GetIssueByExternalRef failed: %v", err) + } + if found1 == nil || found1.ID != "bd-1" { + t.Errorf("Expected to find bd-1 by external ref jira#100") + } + + found2, err := store.GetIssueByExternalRef(ctx, "jira#200") + if err != nil { + t.Fatalf("GetIssueByExternalRef failed: %v", err) + } + if found2 == nil || found2.ID != "bd-2" { + t.Errorf("Expected to find bd-2 by external ref jira#200") + } +}