diff --git a/.beads/beads.jsonl b/.beads/beads.jsonl index 2a8203f3..a87cd468 100644 --- a/.beads/beads.jsonl +++ b/.beads/beads.jsonl @@ -117,7 +117,7 @@ {"id":"bd-9ae788be","content_hash":"19599f6bcc268e97438593e08eb6343b551ce765f0d91956591aa811cbb90690","title":"Implement clone-scoped ID allocation to prevent N-way collisions","description":"## Problem\nCurrent ID allocation uses per-clone atomic counters (issue_counters table) that sync based on local database state. In N-way collision scenarios:\n- Clone B sees {test-1} locally, allocates test-2\n- Clone D sees {test-1, test-2, test-3} locally, allocates test-4\n- When same content gets assigned test-2 and test-4, convergence fails\n\nRoot cause: Each clone independently allocates IDs without global coordination, leading to overlapping assignments for the same content.\n\n## Solution\nAdd clone UUID to ID allocation to make every ID globally unique:\n\n**Current format:** `test-1`, `test-2`, `test-3`\n**New format:** `test-1-a7b3`, `test-2-a7b3`, `test-3-c4d9`\n\nWhere suffix is first 4 chars of clone UUID.\n\n## Implementation\n\n### 1. Add clone_identity table\n```sql\nCREATE TABLE clone_identity (\n clone_uuid TEXT PRIMARY KEY,\n created_at DATETIME DEFAULT CURRENT_TIMESTAMP\n);\n```\n\n### 2. Modify getNextIDForPrefix()\n```go\nfunc (s *SQLiteStorage) getNextIDForPrefix(ctx context.Context, prefix string) (string, error) {\n cloneUUID := s.getOrCreateCloneUUID(ctx)\n shortUUID := cloneUUID[:4]\n \n nextNum := s.getNextCounterForPrefix(ctx, prefix)\n return fmt.Sprintf(\"%s-%d-%s\", prefix, nextNum, shortUUID), nil\n}\n```\n\n### 3. Update ID parsing logic\nAll places that parse IDs (utils.ExtractIssueNumber, etc.) need to handle new format.\n\n### 4. Migration strategy\n- Existing IDs remain unchanged (no suffix)\n- New IDs get clone suffix automatically\n- Display layer can hide suffix in UI: `bd-cb64c226.3-a7b3` → `#42`\n\n## Benefits\n- **Zero collision risk**: Same content in different clones gets different IDs\n- **Maintains readability**: Still sequential numbering within clone\n- **No coordination needed**: Works offline, no central authority\n- **Scales to 100+ clones**: 4-char hex = 65,536 unique clones\n\n## Concerns\n- ID format change may break existing integrations\n- Need migration path for existing databases\n- Display logic needs update to hide/show suffixes appropriately\n\n## Success Criteria\n- 10+ clone collision test passes without failures\n- Existing issues continue to work (backward compatibility)\n- Documentation updated with new ID format\n- Migration guide for v1.x → v2.x\n\n## Timeline\nMedium-term (v1.1-v1.2), 2-3 weeks implementation\n\n## References\n- Related to bd-e6d71828 (immediate fix)\n- See beads_nway_test.go for failing N-way tests","status":"open","priority":2,"issue_type":"feature","created_at":"2025-10-29T10:22:52.260524-07:00","updated_at":"2025-10-30T17:12:58.18193-07:00"} {"id":"bd-9e8d","content_hash":"8a2616a707f5ae932357049b0bc922716f4d729724fb8c38b256c91e292f851b","title":"Test Issue","description":"","status":"closed","priority":1,"issue_type":"bug","created_at":"2025-10-31T21:41:11.107393-07:00","updated_at":"2025-11-01T20:02:28.292279-07:00","closed_at":"2025-11-01T20:02:28.292279-07:00"} {"id":"bd-9f1fce5d","content_hash":"14b0d330680e979e504043d2c560bd2eda204698f5622c3bdc6f91816f861d22","title":"Add internal/ai package for LLM integration","description":"Shared AI client for repair commands.\n\nProviders:\n- Anthropic (Claude)\n- OpenAI (GPT)\n- Ollama (local)\n\nEnv vars:\n- BEADS_AI_PROVIDER\n- BEADS_AI_API_KEY\n- BEADS_AI_MODEL\n\nFiles: internal/ai/client.go (new)","status":"open","priority":1,"issue_type":"task","created_at":"2025-10-28T14:48:29.072473-07:00","updated_at":"2025-10-30T17:12:58.219706-07:00"} -{"id":"bd-9f20","content_hash":"523ec11b4c3e82c8d5b36deeb284ebae19eee2090a7051b599da79366c642238","title":"DetectCycles SQL query has bug preventing cycle detection","description":"The DetectCycles function's SQL query has a bug in the LIKE filter that prevents it from detecting cycles.\n\nCurrent code (line 571):\n```sql\nAND p.path NOT LIKE '%' || d.depends_on_id || '→%'\n```\n\nThis prevents ANY revisit to nodes, including returning to the start node to complete a cycle.\n\nFix:\n```sql\nAND (d.depends_on_id = p.start_id OR p.path NOT LIKE '%' || d.depends_on_id || '→%')\n```\n\nThis allows revisiting the start node (to detect the cycle) while still preventing intermediate node revisits.\n\nImpact: Currently DetectCycles cannot detect any cycles, but this hasn't been noticed because AddDependency prevents cycles from being created. The function would only matter if cycles were manually inserted into the database.","status":"open","priority":3,"issue_type":"bug","created_at":"2025-11-01T22:50:32.552763-07:00","updated_at":"2025-11-01T22:50:32.552763-07:00"} +{"id":"bd-9f20","content_hash":"523ec11b4c3e82c8d5b36deeb284ebae19eee2090a7051b599da79366c642238","title":"DetectCycles SQL query has bug preventing cycle detection","description":"The DetectCycles function's SQL query has a bug in the LIKE filter that prevents it from detecting cycles.\n\nCurrent code (line 571):\n```sql\nAND p.path NOT LIKE '%' || d.depends_on_id || '→%'\n```\n\nThis prevents ANY revisit to nodes, including returning to the start node to complete a cycle.\n\nFix:\n```sql\nAND (d.depends_on_id = p.start_id OR p.path NOT LIKE '%' || d.depends_on_id || '→%')\n```\n\nThis allows revisiting the start node (to detect the cycle) while still preventing intermediate node revisits.\n\nImpact: Currently DetectCycles cannot detect any cycles, but this hasn't been noticed because AddDependency prevents cycles from being created. The function would only matter if cycles were manually inserted into the database.","status":"closed","priority":3,"issue_type":"bug","created_at":"2025-11-01T22:50:32.552763-07:00","updated_at":"2025-11-01T22:52:02.247443-07:00","closed_at":"2025-11-01T22:52:02.247443-07:00"} {"id":"bd-a03d5e36","content_hash":"b4ee73e439a133a77e5df27e3e457674dbc9968fdbee0dc630175726960bb8cf","title":"Improve integration test coverage for stateful features","description":"","design":"## Context\n\nbd-70419816 revealed a critical gap: the export deduplication feature had unit tests but no integration tests simulating real-world git operations. This led to silent data loss in production.\n\n## Root Cause\n- Unit tests only tested functions in isolation\n- No integration tests for git operations (pull, reset, checkout) modifying JSONL\n- No tests validating export_hashes and JSONL stay in sync\n- Missing tests for stateful distributed system interactions (DB + JSONL + git)\n\n## Completed (bd-70419816)\n✓ TestJSONLIntegrityValidation - unit tests for validation logic\n✓ TestImportClearsExportHashes - tests import clears hashes\n✓ TestExportIntegrityAfterJSONLTruncation - simulates git reset (would have caught bd-70419816)\n✓ TestExportIntegrityAfterJSONLDeletion - tests recovery from file deletion\n✓ TestMultipleExportsStayConsistent - tests repeated exports\n\n## Still Needed (High Priority)\n1. Multi-repo sync test - two clones staying in sync after push/pull\n2. Auto-flush integration test - JSONL integrity preserved during auto-flush\n3. Daemon auto-sync integration test - complex state management\n4. Import after corruption test - recovery from partial data loss\n\n## Medium Priority\n- Partial export failure handling (disk full, network interruption)\n- Concurrent export/import race conditions\n- Large dataset performance tests (1000+ issues)\n- Export hash migration tests (version upgrades)\n\n## Testing Principles\n1. Test real-world scenarios: git ops, user errors, system failures, concurrent ops\n2. Integration tests for stateful systems (DB + files + git)\n3. Regression test for every bug fix\n4. Test invariants: JSONL count == DB count, hash consistency, etc.\n\n## Key Lesson\nStateful distributed systems need integration tests, not just unit tests.","acceptance_criteria":"- [ ] Multi-repo sync test implemented\n- [ ] Auto-flush integration test implemented \n- [ ] Daemon auto-sync integration test implemented\n- [ ] Testing guidelines added to CONTRIBUTING.md\n- [ ] CI runs integration tests\n- [ ] All critical workflows have integration test coverage","status":"open","priority":2,"issue_type":"epic","created_at":"2025-10-29T21:53:15.397137-07:00","updated_at":"2025-10-30T17:12:58.206063-07:00"} {"id":"bd-a1691807","content_hash":"23f0119ee9df98f1bf6d648dba06065c156963064ef1c7308dfb02c8bdd5bc58","title":"Integration test: mutation to export latency","description":"Measure time from bd create to JSONL update. Verify \u003c500ms latency. Test with multiple rapid mutations to verify batching.","status":"closed","priority":1,"issue_type":"task","created_at":"2025-10-29T20:49:49.105247-07:00","updated_at":"2025-10-31T12:00:43.198883-07:00","closed_at":"2025-10-31T12:00:43.198883-07:00"} {"id":"bd-a40f374f","content_hash":"599448515198700decd2494cf0cea3335b013c573bdcbda90a151564585cf845","title":"bd validate - Comprehensive health check","description":"Run all validation checks in one command.\n\nChecks:\n- Duplicates\n- Orphaned dependencies\n- Test pollution\n- Git conflicts\n\nSupports --fix-all for auto-repair.\n\nDepends on bd-cbed9619.1, bd-0dcea000, bd-31aab707, bd-9826b69a.\n\nFiles: cmd/bd/validate.go (new)","status":"closed","priority":1,"issue_type":"task","created_at":"2025-10-29T20:02:47.956664-07:00","updated_at":"2025-10-30T17:12:58.195108-07:00","closed_at":"2025-10-29T20:02:15.318966-07:00"} @@ -150,13 +150,13 @@ {"id":"bd-cbed9619.3","content_hash":"ecebc4a18d5355bafc88e778ee87365717f894d3590d325a97ecf8b3f763d54d","title":"Implement global N-way collision resolution algorithm","description":"## Overview\nPhase 3: Replace pairwise collision resolution with global N-way resolution that produces deterministic results regardless of sync order.\n\n## Current Problem\nScoreCollisions (collision.go:228) compares issues pairwise:\n```go\ncollision.RemapIncoming = existingHash \u003c incomingHash\n```\n\nThis works for 2-way but fails for 3+ way because:\n- Each clone makes local decisions without global context\n- No guarantee intermediate states are consistent\n- Remapping decisions depend on sync order\n- Can't detect transitive remap chains (test-1 → test-2 → test-3)\n\n## Solution\nImplement global resolution that:\n1. Collects ALL versions of same logical issue\n2. Sorts by content hash (deterministic)\n3. Assigns sequential IDs based on sorted order\n4. All clones converge to same assignments\n\n## Implementation Tasks\n\n### 1. Create ResolveNWayCollisions function\nFile: internal/storage/sqlite/collision.go\n\nReplace ScoreCollisions with:\n```go\n// ResolveNWayCollisions handles N-way collisions deterministically.\n// Groups all versions with same base ID, sorts by content hash,\n// assigns sequential IDs. Returns mapping of old ID → new ID.\nfunc ResolveNWayCollisions(ctx context.Context, s *SQLiteStorage,\n collisions []*CollisionDetail, incoming []*types.Issue) (map[string]string, error) {\n \n if len(collisions) == 0 {\n return make(map[string]string), nil\n }\n \n // Group by base ID pattern (e.g., test-1, test-2 → base \"test-1\")\n groups := groupCollisionsByBaseID(collisions)\n \n idMapping := make(map[string]string)\n \n for baseID, versions := range groups {\n // 1. Collect all unique versions by content hash\n uniqueVersions := deduplicateVersionsByContentHash(versions)\n \n // 2. Sort by content hash (deterministic!)\n sort.Slice(uniqueVersions, func(i, j int) bool {\n return uniqueVersions[i].ContentHash \u003c uniqueVersions[j].ContentHash\n })\n \n // 3. Assign sequential IDs based on sorted order\n prefix := extractPrefix(baseID)\n baseNum := extractNumber(baseID)\n \n for i, version := range uniqueVersions {\n targetID := fmt.Sprintf(\"%s-%d\", prefix, baseNum+i)\n \n // Map this version to its deterministic ID\n if version.ID != targetID {\n idMapping[version.ID] = targetID\n }\n }\n }\n \n return idMapping, nil\n}\n```\n\n### 2. Implement helper functions\n\n```go\n// groupCollisionsByBaseID groups collisions by their logical base ID\nfunc groupCollisionsByBaseID(collisions []*CollisionDetail) map[string][]*types.Issue {\n groups := make(map[string][]*types.Issue)\n for _, c := range collisions {\n baseID := c.ID // All share same ID (that's why they collide)\n groups[baseID] = append(groups[baseID], c.ExistingIssue, c.IncomingIssue)\n }\n return groups\n}\n\n// deduplicateVersionsByContentHash keeps one issue per unique content hash\nfunc deduplicateVersionsByContentHash(issues []*types.Issue) []*types.Issue {\n seen := make(map[string]*types.Issue)\n for _, issue := range issues {\n if _, found := seen[issue.ContentHash]; !found {\n seen[issue.ContentHash] = issue\n }\n }\n result := make([]*types.Issue, 0, len(seen))\n for _, issue := range seen {\n result = append(result, issue)\n }\n return result\n}\n```\n\n### 3. Update handleCollisions in importer\nFile: internal/importer/importer.go\n\nReplace ScoreCollisions call with:\n```go\n// OLD:\nif err := sqlite.ScoreCollisions(ctx, sqliteStore, collisionResult.Collisions, allExistingIssues); err != nil {\n return nil, fmt.Errorf(\"failed to score collisions: %w\", err)\n}\n\n// NEW:\nidMapping, err := sqlite.ResolveNWayCollisions(ctx, sqliteStore, \n collisionResult.Collisions, issues)\nif err != nil {\n return nil, fmt.Errorf(\"failed to resolve collisions: %w\", err)\n}\n```\n\n### 4. Update RemapCollisions\nRemapCollisions currently uses collision.RemapIncoming field. Update to use idMapping directly:\n- Remove RemapIncoming logic\n- Use idMapping to determine what to remap\n- Simplify to just apply the computed mapping\n\n### 5. Add comprehensive tests\n\nTest cases:\n1. 3-way collision with different content → 3 sequential IDs\n2. 3-way collision with 2 identical content → 2 IDs (dedupe works)\n3. Sync order independence (A→B→C vs C→A→B produce same result)\n4. Content hash ordering is respected\n5. Works with 5+ clones\n\n## Acceptance Criteria\n- ResolveNWayCollisions implemented and replaces ScoreCollisions\n- Groups all versions of same ID together\n- Deduplicates by content hash\n- Sorts by content hash deterministically\n- Assigns sequential IDs starting from base ID\n- Returns complete mapping (old ID → new ID)\n- All clones converge to same ID assignments\n- Works for arbitrary N-way collisions\n- TestThreeCloneCollision passes (or gets much closer)\n\n## Files to Modify\n- internal/storage/sqlite/collision.go (new function, helpers)\n- internal/importer/importer.go (call new function)\n- internal/storage/sqlite/collision_test.go (comprehensive tests)\n\n## Testing Strategy\n\n### Unit Tests\n- groupCollisionsByBaseID correctly groups\n- deduplicateVersionsByContentHash removes duplicates\n- Sorting by hash is stable and deterministic\n- Sequential ID assignment is correct\n\n### Integration Tests\n- 3-way collision resolves to 3 issues\n- Sync order doesn't affect final IDs\n- Content hash ordering determines winner\n\n### Property Tests\n- For any N clones with same content, all converge to same IDs\n- Idempotent: running resolution twice produces same result\n\n## Dependencies\n- Requires bd-cbed9619.5 (ContentHash field) to be completed first\n- Requires bd-cbed9619.4 (read-only detection) for clean integration\n\n## Notes\nThis is the core algorithm that enables convergence. The key insight:\n**Sort by content hash globally, not pairwise comparison.**","status":"closed","priority":1,"issue_type":"task","created_at":"2025-10-28T18:37:42.85616-07:00","updated_at":"2025-10-30T17:12:58.228707-07:00","closed_at":"2025-10-28T20:03:26.675257-07:00","dependencies":[{"issue_id":"bd-cbed9619.3","depends_on_id":"bd-325da116","type":"parent-child","created_at":"2025-10-28T18:39:20.593102-07:00","created_by":"daemon"},{"issue_id":"bd-cbed9619.3","depends_on_id":"bd-cbed9619.5","type":"blocks","created_at":"2025-10-28T18:39:28.30886-07:00","created_by":"daemon"},{"issue_id":"bd-cbed9619.3","depends_on_id":"bd-cbed9619.4","type":"blocks","created_at":"2025-10-28T18:39:28.336312-07:00","created_by":"daemon"}]} {"id":"bd-cbed9619.4","content_hash":"49aad5fa2497f7f88fb74d54553825b93c1021ed7db04cfb2e58682699d8dca9","title":"Make DetectCollisions read-only (separate detection from modification)","description":"## Overview\nPhase 2: Separate collision detection from state modification to enable safe, composable collision resolution.\n\n## Current Problem\nDetectCollisions (collision.go:38-111) modifies database state during detection:\n- Line 83-86: Deletes issues when content matches but ID differs\n- This violates separation of concerns\n- Causes race conditions when processing multiple issues\n- Makes contentToDBIssue map stale after first deletion\n- Partial failures leave DB in inconsistent state\n\n## Solution\nMake DetectCollisions purely read-only. Move all modifications to a separate ApplyCollisionResolution function.\n\n## Implementation Tasks\n\n### 1. Add RenameDetail to CollisionResult\nFile: internal/storage/sqlite/collision.go\n```go\ntype CollisionResult struct {\n ExactMatches []string\n Collisions []*CollisionDetail\n NewIssues []string\n Renames []*RenameDetail // NEW\n}\n\ntype RenameDetail struct {\n OldID string // ID in database\n NewID string // ID in incoming\n Issue *types.Issue // The issue with new ID\n}\n```\n\n### 2. Remove deletion from DetectCollisions\nReplace lines 83-86:\n```go\n// OLD (DELETE THIS):\nif err := s.DeleteIssue(ctx, dbMatch.ID); err != nil {\n return nil, fmt.Errorf(\"failed to delete renamed issue...\")\n}\n\n// NEW (ADD THIS):\nresult.Renames = append(result.Renames, \u0026RenameDetail{\n OldID: dbMatch.ID,\n NewID: incoming.ID,\n Issue: incoming,\n})\ncontinue // Don't mark as NewIssue yet\n```\n\n### 3. Create ApplyCollisionResolution function\nNew function to apply all modifications atomically:\n```go\nfunc ApplyCollisionResolution(ctx context.Context, s *SQLiteStorage,\n result *CollisionResult, mapping map[string]string) error {\n \n // Phase 1: Handle renames (delete old IDs)\n for _, rename := range result.Renames {\n if err := s.DeleteIssue(ctx, rename.OldID); err != nil {\n return fmt.Errorf(\"failed to delete renamed issue %s: %w\", \n rename.OldID, err)\n }\n }\n \n // Phase 2: Create new IDs (from mapping)\n // Phase 3: Update references\n return nil\n}\n```\n\n### 4. Update callers to use two-phase approach\nFile: internal/importer/importer.go (handleCollisions)\n```go\n// Phase 1: Detect (read-only)\ncollisionResult, err := sqlite.DetectCollisions(ctx, sqliteStore, issues)\n\n// Phase 2: Resolve (compute mapping)\nmapping, err := sqlite.ResolveNWayCollisions(ctx, sqliteStore, collisionResult)\n\n// Phase 3: Apply (modify DB)\nerr = sqlite.ApplyCollisionResolution(ctx, sqliteStore, collisionResult, mapping)\n```\n\n### 5. Update tests\n- Verify DetectCollisions doesn't modify DB\n- Test ApplyCollisionResolution separately\n- Add test for rename detection without modification\n\n## Acceptance Criteria\n- DetectCollisions performs zero writes to database\n- DetectCollisions returns RenameDetail entries for content matches\n- ApplyCollisionResolution handles all modifications\n- All existing tests still pass\n- New test verifies read-only detection\n- contentToDBIssue map stays consistent throughout detection\n\n## Files to Modify\n- internal/storage/sqlite/collision.go (DetectCollisions, new function)\n- internal/importer/importer.go (handleCollisions caller)\n- internal/storage/sqlite/collision_test.go (add tests)\n\n## Testing\n- Unit test: DetectCollisions with content match doesn't delete DB issue\n- Unit test: RenameDetail correctly populated\n- Unit test: ApplyCollisionResolution applies renames\n- Integration test: Full flow still works end-to-end\n\n## Risk Mitigation\nThis is a significant refactor of core collision logic. Recommend:\n1. Add comprehensive tests before modifying\n2. Use feature flag to enable/disable new behavior\n3. Test thoroughly with TestTwoCloneCollision first","status":"closed","priority":1,"issue_type":"task","created_at":"2025-10-28T18:37:09.652326-07:00","updated_at":"2025-10-30T17:12:58.228266-07:00","closed_at":"2025-10-28T19:08:17.715416-07:00","dependencies":[{"issue_id":"bd-cbed9619.4","depends_on_id":"bd-325da116","type":"parent-child","created_at":"2025-10-28T18:39:20.570276-07:00","created_by":"daemon"},{"issue_id":"bd-cbed9619.4","depends_on_id":"bd-cbed9619.5","type":"blocks","created_at":"2025-10-28T18:39:28.285653-07:00","created_by":"daemon"}]} {"id":"bd-cbed9619.5","content_hash":"12cd30dee3c08ba58d03e4468e6fe261a47d58c3b75397d9f14f38ee644fab6e","title":"Add content-addressable identity to Issue type","description":"## Overview\nPhase 1: Add content hashing to enable global identification of issues regardless of their assigned IDs.\n\n## Current Problem\nThe system identifies issues only by ID (e.g., test-1, test-2). When multiple clones create the same ID with different content, there's no way to identify that these are semantically different issues without comparing all fields.\n\n## Solution\nAdd a ContentHash field to the Issue type that represents the canonical content fingerprint.\n\n## Implementation Tasks\n\n### 1. Add ContentHash field to Issue type\nFile: internal/types/types.go\n```go\ntype Issue struct {\n ID string\n ContentHash string // SHA256 of canonical content\n // ... existing fields\n}\n```\n\n### 2. Add content hash computation method\nUse existing hashIssueContent from collision.go:186 as foundation:\n```go\nfunc (i *Issue) ComputeContentHash() string {\n return hashIssueContent(i)\n}\n```\n\n### 3. Compute hash at creation time\n- Modify CreateIssue to compute and store ContentHash\n- Modify CreateIssues (batch) to compute hashes\n\n### 4. Compute hash at import time \n- Modify ImportIssues to compute ContentHash for all incoming issues\n- Store hash in database\n\n### 5. Add database column\n- Add migration to add content_hash column to issues table\n- Update SELECT/INSERT statements to include content_hash\n- Index on content_hash for fast lookups\n\n### 6. Populate existing issues\n- Add migration step to compute ContentHash for all existing issues\n- Use hashIssueContent function\n\n## Acceptance Criteria\n- Issue type has ContentHash field\n- Hash is computed automatically at creation time\n- Hash is computed for imported issues\n- Database stores content_hash column\n- All existing issues have non-empty ContentHash\n- Hash is deterministic (same content → same hash)\n- Hash excludes ID, timestamps (only semantic content)\n\n## Files to Modify\n- internal/types/types.go\n- internal/storage/sqlite/sqlite.go (schema, CreateIssue, CreateIssues)\n- internal/storage/sqlite/migrations.go (new migration)\n- internal/importer/importer.go (compute hash during import)\n- cmd/bd/create.go (compute hash at creation)\n\n## Testing\n- Unit test: same content produces same hash\n- Unit test: different content produces different hash \n- Unit test: hash excludes ID and timestamps\n- Integration test: hash persists in database\n- Migration test: existing issues get hashes populated","status":"closed","priority":1,"issue_type":"task","created_at":"2025-10-28T18:36:44.914967-07:00","updated_at":"2025-10-30T17:12:58.2279-07:00","closed_at":"2025-10-28T18:57:10.985198-07:00","dependencies":[{"issue_id":"bd-cbed9619.5","depends_on_id":"bd-325da116","type":"parent-child","created_at":"2025-10-28T18:39:20.547325-07:00","created_by":"daemon"}]} -{"id":"bd-cdf7","content_hash":"ebe962c7eb6dba6d112f7ccf59a8920e0354ea9cd8b039974a8fc4a58373809b","title":"Add tests for DetectCycles to improve coverage from 29.6%","description":"DetectCycles currently has 29.6% coverage. Need comprehensive tests for:\n- Simple cycles (A-\u003eB-\u003eA)\n- Complex multi-node cycles\n- Acyclic graphs (should not detect cycles)\n- Self-loops\n- Multiple independent cycles\n- Edge cases (empty graph, single node)","status":"in_progress","priority":2,"issue_type":"task","created_at":"2025-11-01T22:40:58.977156-07:00","updated_at":"2025-11-01T22:47:23.173432-07:00"} +{"id":"bd-cdf7","content_hash":"ebe962c7eb6dba6d112f7ccf59a8920e0354ea9cd8b039974a8fc4a58373809b","title":"Add tests for DetectCycles to improve coverage from 29.6%","description":"DetectCycles currently has 29.6% coverage. Need comprehensive tests for:\n- Simple cycles (A-\u003eB-\u003eA)\n- Complex multi-node cycles\n- Acyclic graphs (should not detect cycles)\n- Self-loops\n- Multiple independent cycles\n- Edge cases (empty graph, single node)","status":"closed","priority":2,"issue_type":"task","created_at":"2025-11-01T22:40:58.977156-07:00","updated_at":"2025-11-01T22:52:02.243223-07:00","closed_at":"2025-11-01T22:52:02.243223-07:00"} {"id":"bd-ce37850f","content_hash":"3ef2872c3fcb1e5acc90d33fd5a76291742cbcecfbf697b611aa5b4d8ce80078","title":"Add embedding generation for duplicate detection","description":"Use embeddings for scalable duplicate detection.\n\nModel: text-embedding-3-small (OpenAI) or all-MiniLM-L6-v2 (local)\nStorage: SQLite vector extension or in-memory\nCost: ~/bin/bash.0002 per 100 issues\n\nMuch cheaper than LLM comparisons for large databases.\n\nFiles: internal/embeddings/ (new package)","status":"open","priority":2,"issue_type":"task","created_at":"2025-10-28T14:48:29.072913-07:00","updated_at":"2025-10-30T17:12:58.219921-07:00"} {"id":"bd-cf349eb3","content_hash":"1b42289a0cb1da0626a69c6f004bf62fc9ba6e3a0f8eb70159c5f1446497020b","title":"Update LINTING.md with current baseline","description":"After cleanup, document the remaining acceptable baseline in LINTING.md so we can track regression.","status":"closed","priority":2,"issue_type":"task","created_at":"2025-10-27T23:20:10.39272-07:00","updated_at":"2025-10-30T17:12:58.215471-07:00","closed_at":"2025-10-27T23:05:31.945614-07:00"} {"id":"bd-d33c","content_hash":"0c3eb277be0ec16edae305156aa8824b6bc9c37fbd6151477f235e859e9b6181","title":"Separate process/lock/PID concerns into process.go","description":"Create internal/daemonrunner/process.go with: acquireDaemonLock, PID file read/write, stopDaemon, isDaemonRunning, getPIDFilePath, socket path helpers, version check.","status":"open","priority":1,"issue_type":"task","created_at":"2025-11-01T11:41:14.871122-07:00","updated_at":"2025-11-01T11:41:14.871122-07:00"} {"id":"bd-d355a07d","content_hash":"b4f98403e209eadf33dd4913660c1538fd922c89339a9ed034ef504aac358662","title":"Import validation falsely reports data loss on collision resolution","description":"## Problem\n\nPost-import validation reports 'data loss detected!' when import count reduces due to legitimate collision resolution.\n\n## Example\n\n```\nImport complete: 1 created, 8 updated, 142 unchanged, 19 skipped, 1 issues remapped\nPost-import validation failed: import reduced issue count: 165 → 164 (data loss detected!)\n```\n\nThis was actually successful collision resolution (bd-70419816 duplicated → remapped to-70419816), not data loss.\n\n## Impact\n\n- False alarms waste investigation time\n- Undermines confidence in import validation\n- Confuses users/agents about sync health\n\n## Solution\n\nImprove validation to distinguish:\n- Collision-resolution merges (expected count reduction)\n- Actual data loss (unexpected disappearance)\n\nTrack remapped issue count and adjust expected post-import count accordingly.","status":"open","priority":2,"issue_type":"bug","created_at":"2025-10-29T23:15:00.815227-07:00","updated_at":"2025-10-31T19:38:09.19996-07:00"} {"id":"bd-d4ec5a82","content_hash":"872448809bfa26d39d68ba6cac5071379756c30bcd3b08dc75de6da56c133956","title":"Add MCP functions for repair commands","description":"Add repair commands to beads-mcp for agent access:\n- beads_resolve_conflicts()\n- beads_find_duplicates()\n- beads_detect_pollution()\n- beads_validate()\n\nFiles: integrations/beads-mcp/src/beads_mcp/server.py","status":"open","priority":2,"issue_type":"task","created_at":"2025-10-28T14:48:29.071495-07:00","updated_at":"2025-10-30T17:12:58.219499-07:00"} -{"id":"bd-d68f","content_hash":"dc162e1d7b3422fd276a927c1234b55cd375d0fa445a0dcd690d929cacca9e70","title":"Add tests for Comments API (AddIssueComment, GetIssueComments)","description":"Comments API currently has 0% coverage. Need tests for:\n- AddIssueComment - adding comments to issues\n- GetIssueComments - retrieving comments\n- Comment ordering and pagination\n- Edge cases (non-existent issues, empty comments)","status":"open","priority":3,"issue_type":"task","created_at":"2025-11-01T22:40:58.980688-07:00","updated_at":"2025-11-01T22:40:58.980688-07:00"} +{"id":"bd-d68f","content_hash":"4b5b5340749fba1c419c22f9937717b363ee8a49e4c5e0a5e0066a24b652a936","title":"Add tests for Comments API (AddIssueComment, GetIssueComments)","description":"Comments API currently has 0% coverage. Need tests for:\n- AddIssueComment - adding comments to issues\n- GetIssueComments - retrieving comments\n- Comment ordering and pagination\n- Edge cases (non-existent issues, empty comments)","status":"in_progress","priority":3,"issue_type":"task","created_at":"2025-11-01T22:40:58.980688-07:00","updated_at":"2025-11-01T22:52:06.908316-07:00"} {"id":"bd-d7e88238","content_hash":"b69ec861618b03129fad7807b085ee6365860cfd2e9901b49eb846e192b95a0d","title":"Rapid 3","description":"","status":"open","priority":3,"issue_type":"task","created_at":"2025-10-29T19:11:57.459655-07:00","updated_at":"2025-10-30T17:12:58.189494-07:00"} {"id":"bd-d9e0","content_hash":"67a706abbb956ca078d37d718b8f4d2d6c0c1b71f7a2267c64a67dab4938f433","title":"Extract validation functions to validators.go","description":"Move validatePriority, validateStatus, validateIssueType, validateTitle, validateEstimatedMinutes, validateFieldUpdate to validators.go","status":"open","priority":1,"issue_type":"task","created_at":"2025-11-01T19:28:54.915909-07:00","updated_at":"2025-11-01T19:28:54.915909-07:00"} {"id":"bd-da4d8951","content_hash":"a4e81b23d88d41c8fd3fe31fb7ef387f99cb54ea42a6baa210ede436ecce3288","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-28T16:20:02.430127-07:00","updated_at":"2025-10-30T17:12:58.22099-07:00","closed_at":"2025-10-28T19:20:58.312809-07:00"} diff --git a/internal/storage/sqlite/comments_test.go b/internal/storage/sqlite/comments_test.go new file mode 100644 index 00000000..73f3d305 --- /dev/null +++ b/internal/storage/sqlite/comments_test.go @@ -0,0 +1,368 @@ +package sqlite + +import ( + "context" + "testing" + + "github.com/steveyegge/beads/internal/types" +) + +// TestAddIssueComment tests basic comment addition +func TestAddIssueComment(t *testing.T) { + store, cleanup := setupTestDB(t) + defer cleanup() + + ctx := context.Background() + + // Create an issue + issue := &types.Issue{ + Title: "Test issue", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeTask, + } + if err := store.CreateIssue(ctx, issue, "test-user"); err != nil { + t.Fatalf("CreateIssue failed: %v", err) + } + + // Add a comment + comment, err := store.AddIssueComment(ctx, issue.ID, "alice", "This is a test comment") + if err != nil { + t.Fatalf("AddIssueComment failed: %v", err) + } + + // Verify comment fields + if comment.IssueID != issue.ID { + t.Errorf("Expected IssueID %s, got %s", issue.ID, comment.IssueID) + } + if comment.Author != "alice" { + t.Errorf("Expected Author 'alice', got '%s'", comment.Author) + } + if comment.Text != "This is a test comment" { + t.Errorf("Expected Text 'This is a test comment', got '%s'", comment.Text) + } + if comment.ID == 0 { + t.Error("Expected non-zero comment ID") + } + if comment.CreatedAt.IsZero() { + t.Error("Expected non-zero CreatedAt timestamp") + } +} + +// TestAddIssueCommentNonexistentIssue tests adding comment to non-existent issue +func TestAddIssueCommentNonexistentIssue(t *testing.T) { + store, cleanup := setupTestDB(t) + defer cleanup() + + ctx := context.Background() + + // Try to add comment to non-existent issue + _, err := store.AddIssueComment(ctx, "nonexistent-id", "alice", "comment") + if err == nil { + t.Fatal("Expected error when adding comment to non-existent issue, got nil") + } +} + +// TestGetIssueComments tests retrieving comments +func TestGetIssueComments(t *testing.T) { + store, cleanup := setupTestDB(t) + defer cleanup() + + ctx := context.Background() + + // Create an issue + issue := &types.Issue{ + Title: "Test issue", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeTask, + } + if err := store.CreateIssue(ctx, issue, "test-user"); err != nil { + t.Fatalf("CreateIssue failed: %v", err) + } + + // Add multiple comments + testComments := []struct { + author string + text string + }{ + {"alice", "First comment"}, + {"bob", "Second comment"}, + {"charlie", "Third comment"}, + } + + for _, tc := range testComments { + _, err := store.AddIssueComment(ctx, issue.ID, tc.author, tc.text) + if err != nil { + t.Fatalf("AddIssueComment failed: %v", err) + } + } + + // Retrieve comments + comments, err := store.GetIssueComments(ctx, issue.ID) + if err != nil { + t.Fatalf("GetIssueComments failed: %v", err) + } + + // Verify number of comments + if len(comments) != len(testComments) { + t.Fatalf("Expected %d comments, got %d", len(testComments), len(comments)) + } + + // Verify comment content and ordering (should be chronological) + for i, comment := range comments { + if comment.Author != testComments[i].author { + t.Errorf("Comment %d: expected author %s, got %s", i, testComments[i].author, comment.Author) + } + if comment.Text != testComments[i].text { + t.Errorf("Comment %d: expected text %s, got %s", i, testComments[i].text, comment.Text) + } + if comment.IssueID != issue.ID { + t.Errorf("Comment %d: expected IssueID %s, got %s", i, issue.ID, comment.IssueID) + } + } +} + +// TestGetIssueCommentsOrdering tests that comments are returned in chronological order +func TestGetIssueCommentsOrdering(t *testing.T) { + store, cleanup := setupTestDB(t) + defer cleanup() + + ctx := context.Background() + + // Create an issue + issue := &types.Issue{ + Title: "Test issue", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeTask, + } + if err := store.CreateIssue(ctx, issue, "test-user"); err != nil { + t.Fatalf("CreateIssue failed: %v", err) + } + + // Add comments with identifiable ordering + for i := 1; i <= 5; i++ { + text := "Comment " + string(rune('0'+i)) + _, err := store.AddIssueComment(ctx, issue.ID, "alice", text) + if err != nil { + t.Fatalf("AddIssueComment failed: %v", err) + } + } + + // Retrieve comments + comments, err := store.GetIssueComments(ctx, issue.ID) + if err != nil { + t.Fatalf("GetIssueComments failed: %v", err) + } + + // Verify chronological ordering + if len(comments) != 5 { + t.Fatalf("Expected 5 comments, got %d", len(comments)) + } + + for i := 0; i < len(comments); i++ { + expectedText := "Comment " + string(rune('0'+i+1)) + if comments[i].Text != expectedText { + t.Errorf("Comment %d: expected text %s, got %s", i, expectedText, comments[i].Text) + } + + // Verify timestamps are in ascending order + if i > 0 && comments[i].CreatedAt.Before(comments[i-1].CreatedAt) { + t.Errorf("Comments not in chronological order: comment %d (%v) is before comment %d (%v)", + i, comments[i].CreatedAt, i-1, comments[i-1].CreatedAt) + } + } +} + +// TestGetIssueCommentsEmpty tests retrieving comments for issue with no comments +func TestGetIssueCommentsEmpty(t *testing.T) { + store, cleanup := setupTestDB(t) + defer cleanup() + + ctx := context.Background() + + // Create an issue + issue := &types.Issue{ + Title: "Test issue", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeTask, + } + if err := store.CreateIssue(ctx, issue, "test-user"); err != nil { + t.Fatalf("CreateIssue failed: %v", err) + } + + // Retrieve comments (should be empty) + comments, err := store.GetIssueComments(ctx, issue.ID) + if err != nil { + t.Fatalf("GetIssueComments failed: %v", err) + } + + if len(comments) != 0 { + t.Errorf("Expected 0 comments, got %d", len(comments)) + } +} + +// TestGetIssueCommentsNonexistentIssue tests retrieving comments for non-existent issue +func TestGetIssueCommentsNonexistentIssue(t *testing.T) { + store, cleanup := setupTestDB(t) + defer cleanup() + + ctx := context.Background() + + // Retrieve comments for non-existent issue + comments, err := store.GetIssueComments(ctx, "nonexistent-id") + if err != nil { + t.Fatalf("GetIssueComments failed: %v", err) + } + + // Should return empty slice, not error + if len(comments) != 0 { + t.Errorf("Expected 0 comments for non-existent issue, got %d", len(comments)) + } +} + +// TestAddIssueCommentEmptyText tests adding comment with empty text +func TestAddIssueCommentEmptyText(t *testing.T) { + store, cleanup := setupTestDB(t) + defer cleanup() + + ctx := context.Background() + + // Create an issue + issue := &types.Issue{ + Title: "Test issue", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeTask, + } + if err := store.CreateIssue(ctx, issue, "test-user"); err != nil { + t.Fatalf("CreateIssue failed: %v", err) + } + + // Add comment with empty text (should succeed - validation is caller's responsibility) + comment, err := store.AddIssueComment(ctx, issue.ID, "alice", "") + if err != nil { + t.Fatalf("AddIssueComment with empty text failed: %v", err) + } + + if comment.Text != "" { + t.Errorf("Expected empty text, got '%s'", comment.Text) + } +} + +// TestAddIssueCommentMarksDirty tests that adding a comment marks the issue dirty +func TestAddIssueCommentMarksDirty(t *testing.T) { + store, cleanup := setupTestDB(t) + defer cleanup() + + ctx := context.Background() + + // Create an issue + issue := &types.Issue{ + Title: "Test issue", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeTask, + } + if err := store.CreateIssue(ctx, issue, "test-user"); err != nil { + t.Fatalf("CreateIssue failed: %v", err) + } + + // Clear dirty flag (simulating after export) + if err := store.ClearDirtyIssuesByID(ctx, []string{issue.ID}); err != nil { + t.Fatalf("ClearDirtyIssuesByID failed: %v", err) + } + + // Add a comment + _, err := store.AddIssueComment(ctx, issue.ID, "alice", "test comment") + if err != nil { + t.Fatalf("AddIssueComment failed: %v", err) + } + + // Verify issue is marked dirty + var exists bool + err = store.db.QueryRowContext(ctx, `SELECT EXISTS(SELECT 1 FROM dirty_issues WHERE issue_id = ?)`, issue.ID).Scan(&exists) + if err != nil { + t.Fatalf("Failed to check dirty flag: %v", err) + } + + if !exists { + t.Error("Expected issue to be marked dirty after adding comment") + } +} + +// TestGetIssueCommentsMultipleIssues tests that comments are properly isolated per issue +func TestGetIssueCommentsMultipleIssues(t *testing.T) { + store, cleanup := setupTestDB(t) + defer cleanup() + + ctx := context.Background() + + // Create two issues + issue1 := &types.Issue{ + Title: "Issue 1", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeTask, + } + issue2 := &types.Issue{ + Title: "Issue 2", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeTask, + } + if err := store.CreateIssue(ctx, issue1, "test-user"); err != nil { + t.Fatalf("CreateIssue failed: %v", err) + } + if err := store.CreateIssue(ctx, issue2, "test-user"); err != nil { + t.Fatalf("CreateIssue failed: %v", err) + } + + // Add comments to each issue + _, err := store.AddIssueComment(ctx, issue1.ID, "alice", "Comment for issue 1") + if err != nil { + t.Fatalf("AddIssueComment failed: %v", err) + } + _, err = store.AddIssueComment(ctx, issue1.ID, "bob", "Another comment for issue 1") + if err != nil { + t.Fatalf("AddIssueComment failed: %v", err) + } + _, err = store.AddIssueComment(ctx, issue2.ID, "charlie", "Comment for issue 2") + if err != nil { + t.Fatalf("AddIssueComment failed: %v", err) + } + + // Retrieve comments for issue 1 + comments1, err := store.GetIssueComments(ctx, issue1.ID) + if err != nil { + t.Fatalf("GetIssueComments failed: %v", err) + } + + // Retrieve comments for issue 2 + comments2, err := store.GetIssueComments(ctx, issue2.ID) + if err != nil { + t.Fatalf("GetIssueComments failed: %v", err) + } + + // Verify each issue has the correct number of comments + if len(comments1) != 2 { + t.Errorf("Expected 2 comments for issue 1, got %d", len(comments1)) + } + if len(comments2) != 1 { + t.Errorf("Expected 1 comment for issue 2, got %d", len(comments2)) + } + + // Verify comments belong to correct issues + for _, c := range comments1 { + if c.IssueID != issue1.ID { + t.Errorf("Comment has wrong IssueID: expected %s, got %s", issue1.ID, c.IssueID) + } + } + for _, c := range comments2 { + if c.IssueID != issue2.ID { + t.Errorf("Comment has wrong IssueID: expected %s, got %s", issue2.ID, c.IssueID) + } + } +}