From d47378cfbc21d90b84a955ce5144b7c3a9b5050e Mon Sep 17 00:00:00 2001 From: Steve Yegge Date: Mon, 27 Oct 2025 20:52:51 -0700 Subject: [PATCH] Remove skipped tests and unreachable RPC methods (bd-212, bd-213) bd-212: Delete 3 permanently skipped test functions (~150 LOC) - TestImportDependencyUpdates (import_collision_test.go) - TestImportChainDependencies (import_collision_test.go) - TestUpdateDependencyReferences (collision_test.go) bd-213: Remove 4 unreachable RPC methods (~80 LOC) - Server.GetLastImportTime - Server.SetLastImportTime - Server.findJSONLPath - Client.Import All tests pass. Phase 1 dead code cleanup continues. --- .beads/beads.jsonl | 4 +- cmd/bd/import_collision_test.go | 240 ---------------------- internal/rpc/client.go | 5 - internal/rpc/server.go | 35 ---- internal/storage/sqlite/collision_test.go | 139 ------------- 5 files changed, 2 insertions(+), 421 deletions(-) diff --git a/.beads/beads.jsonl b/.beads/beads.jsonl index c5a2ce2b..d492d538 100644 --- a/.beads/beads.jsonl +++ b/.beads/beads.jsonl @@ -124,8 +124,8 @@ {"id":"bd-21","title":"Fix bd sync prefix mismatch error message suggesting non-existent flag","description":"GH #103: bd sync suggests using --rename-on-import flag that doesn't exist. Need to either implement the flag or fix the error message to suggest the correct workflow.","status":"closed","priority":1,"issue_type":"bug","created_at":"2025-10-22T17:54:24.473508-07:00","updated_at":"2025-10-25T23:15:33.481941-07:00","closed_at":"2025-10-22T17:57:46.973029-07:00"} {"id":"bd-210","title":"Delete cmd/bd/import_phases.go - entire file is dead code","description":"The file `cmd/bd/import_phases.go` (377 LOC) contains 7 functions that are all unreachable according to the deadcode analyzer. This appears to be an abandoned import refactoring that was never completed or has been replaced by the current implementation in `import.go`.\n\nUnreachable functions:\n- `getOrCreateStore` (line 15)\n- `handlePrefixMismatch` (line 43)\n- `handleCollisions` (line 87)\n- `upsertIssues` (line 155)\n- `importDependencies` (line 240)\n- `importLabels` (line 281)\n- `importComments` (line 316)\n\nEvidence:\n```bash\ngo run golang.org/x/tools/cmd/deadcode@latest -test ./...\n# Shows all 7 functions as unreachable\n```\n\nNo external callers found via:\n```bash\ngrep -r \"getOrCreateStore\\|handlePrefixMismatch\\|handleCollisions\\|upsertIssues\" cmd/bd/*.go\n# Only matches within import_phases.go itself\n```\n\nImpact: Removes 377 LOC, simplifies import logic","acceptance_criteria":"- Delete `cmd/bd/import_phases.go`\n- Verify all tests still pass: `go test ./cmd/bd/...`\n- Verify import functionality works: test `bd import` command\n- Run deadcode analyzer to confirm no new unreachable code","status":"closed","priority":1,"issue_type":"task","created_at":"2025-10-27T20:30:19.931508-07:00","updated_at":"2025-10-27T20:34:54.268126-07:00","closed_at":"2025-10-27T20:34:54.268126-07:00","labels":["cleanup","dead-code","phase-1"],"dependencies":[{"issue_id":"bd-210","depends_on_id":"bd-209","type":"parent-child","created_at":"2025-10-27T20:30:19.933155-07:00","created_by":"daemon"}]} {"id":"bd-211","title":"Remove deprecated rename functions from import_shared.go","description":"The file `cmd/bd/import_shared.go` contains deprecated and unreachable rename functions (~100 LOC) that are no longer used. The active implementation has moved to `internal/importer/importer.go`.\n\nFunctions to remove:\n- `renameImportedIssuePrefixes` (line 262) - wrapper function, unused\n- `renameImportedIssuePrefixesOld` (line 267) - marked Deprecated, 70 LOC\n- `replaceIDReferences` (line 332) - only called by deprecated function\n\nEvidence:\n```bash\ngo run golang.org/x/tools/cmd/deadcode@latest -test ./...\n# Shows these as unreachable\n```\n\nThe actual implementation is in:\n- `internal/importer/importer.go` - `RenameImportedIssuePrefixes`\n\nImpact: Removes ~100 LOC of deprecated code","acceptance_criteria":"- Remove lines 262-340 from `cmd/bd/import_shared.go`\n- Verify no callers exist: `grep -r \"renameImportedIssuePrefixes\\|replaceIDReferences\" cmd/bd/`\n- All tests pass: `go test ./cmd/bd/...`\n- Import with rename works: `bd import --rename-on-import`","status":"closed","priority":1,"issue_type":"task","created_at":"2025-10-27T20:30:19.948413-07:00","updated_at":"2025-10-27T20:38:13.769416-07:00","closed_at":"2025-10-27T20:38:13.769416-07:00","labels":["cleanup","dead-code","phase-1"],"dependencies":[{"issue_id":"bd-211","depends_on_id":"bd-209","type":"parent-child","created_at":"2025-10-27T20:30:19.950023-07:00","created_by":"daemon"}]} -{"id":"bd-212","title":"Delete skipped tests for \"old buggy behavior\"","description":"Three test functions are permanently skipped with comments indicating they test behavior that was fixed in GH#120. These tests will never run again and should be deleted.\n\nTest functions to remove:\n\n1. `cmd/bd/import_collision_test.go:228`\n ```go\n t.Skip(\"Test expects old buggy behavior - needs rewrite for GH#120 fix\")\n ```\n\n2. `cmd/bd/import_collision_test.go:505`\n ```go\n t.Skip(\"Test expects old buggy behavior - needs rewrite for GH#120 fix\")\n ```\n\n3. `internal/storage/sqlite/collision_test.go:919`\n ```go\n t.Skip(\"Test expects old buggy behavior - needs rewrite for GH#120 fix\")\n ```\n\nImpact: Removes ~150 LOC of permanently skipped tests","acceptance_criteria":"- Delete the 3 test functions entirely (~150 LOC total)\n- Update test file comments to reference GH#120 fix if needed\n- All remaining tests pass: `go test ./...`\n- No reduction in meaningful test coverage (these test fixed bugs)","status":"open","priority":1,"issue_type":"task","created_at":"2025-10-27T20:30:19.961185-07:00","updated_at":"2025-10-27T20:30:19.961185-07:00","labels":["cleanup","dead-code","phase-1","test-cleanup"],"dependencies":[{"issue_id":"bd-212","depends_on_id":"bd-209","type":"parent-child","created_at":"2025-10-27T20:30:19.962815-07:00","created_by":"daemon"}]} -{"id":"bd-213","title":"Remove unreachable RPC methods","description":"Several RPC server and client methods are unreachable and should be removed:\n\nServer methods (internal/rpc/server.go):\n- `Server.GetLastImportTime` (line 2116)\n- `Server.SetLastImportTime` (line 2123)\n- `Server.findJSONLPath` (line 2255)\n\nClient methods (internal/rpc/client.go):\n- `Client.Import` (line 311) - RPC import not used (daemon uses autoimport)\n\nEvidence:\n```bash\ngo run golang.org/x/tools/cmd/deadcode@latest -test ./...\n```\n\nImpact: Removes ~80 LOC of unused RPC code","acceptance_criteria":"- Remove the 4 unreachable methods (~80 LOC total)\n- Verify no callers: `grep -r \"GetLastImportTime\\|SetLastImportTime\\|findJSONLPath\" .`\n- All tests pass: `go test ./internal/rpc/...`\n- Daemon functionality works: test daemon start/stop/operations","status":"open","priority":2,"issue_type":"task","created_at":"2025-10-27T20:30:19.962209-07:00","updated_at":"2025-10-27T20:30:19.962209-07:00","labels":["cleanup","dead-code","phase-1","rpc"],"dependencies":[{"issue_id":"bd-213","depends_on_id":"bd-209","type":"parent-child","created_at":"2025-10-27T20:30:19.965239-07:00","created_by":"daemon"}]} +{"id":"bd-212","title":"Delete skipped tests for \"old buggy behavior\"","description":"Three test functions are permanently skipped with comments indicating they test behavior that was fixed in GH#120. These tests will never run again and should be deleted.\n\nTest functions to remove:\n\n1. `cmd/bd/import_collision_test.go:228`\n ```go\n t.Skip(\"Test expects old buggy behavior - needs rewrite for GH#120 fix\")\n ```\n\n2. `cmd/bd/import_collision_test.go:505`\n ```go\n t.Skip(\"Test expects old buggy behavior - needs rewrite for GH#120 fix\")\n ```\n\n3. `internal/storage/sqlite/collision_test.go:919`\n ```go\n t.Skip(\"Test expects old buggy behavior - needs rewrite for GH#120 fix\")\n ```\n\nImpact: Removes ~150 LOC of permanently skipped tests","acceptance_criteria":"- Delete the 3 test functions entirely (~150 LOC total)\n- Update test file comments to reference GH#120 fix if needed\n- All remaining tests pass: `go test ./...`\n- No reduction in meaningful test coverage (these test fixed bugs)","status":"closed","priority":1,"issue_type":"task","created_at":"2025-10-27T20:30:19.961185-07:00","updated_at":"2025-10-27T20:52:40.36471-07:00","closed_at":"2025-10-27T20:52:40.36471-07:00","labels":["cleanup","dead-code","phase-1","test-cleanup"],"dependencies":[{"issue_id":"bd-212","depends_on_id":"bd-209","type":"parent-child","created_at":"2025-10-27T20:30:19.962815-07:00","created_by":"daemon"}]} +{"id":"bd-213","title":"Remove unreachable RPC methods","description":"Several RPC server and client methods are unreachable and should be removed:\n\nServer methods (internal/rpc/server.go):\n- `Server.GetLastImportTime` (line 2116)\n- `Server.SetLastImportTime` (line 2123)\n- `Server.findJSONLPath` (line 2255)\n\nClient methods (internal/rpc/client.go):\n- `Client.Import` (line 311) - RPC import not used (daemon uses autoimport)\n\nEvidence:\n```bash\ngo run golang.org/x/tools/cmd/deadcode@latest -test ./...\n```\n\nImpact: Removes ~80 LOC of unused RPC code","acceptance_criteria":"- Remove the 4 unreachable methods (~80 LOC total)\n- Verify no callers: `grep -r \"GetLastImportTime\\|SetLastImportTime\\|findJSONLPath\" .`\n- All tests pass: `go test ./internal/rpc/...`\n- Daemon functionality works: test daemon start/stop/operations","status":"closed","priority":2,"issue_type":"task","created_at":"2025-10-27T20:30:19.962209-07:00","updated_at":"2025-10-27T20:52:40.366402-07:00","closed_at":"2025-10-27T20:52:40.366402-07:00","labels":["cleanup","dead-code","phase-1","rpc"],"dependencies":[{"issue_id":"bd-213","depends_on_id":"bd-209","type":"parent-child","created_at":"2025-10-27T20:30:19.965239-07:00","created_by":"daemon"}]} {"id":"bd-214","title":"Remove unreachable utility functions","description":"Several small utility functions are unreachable:\n\nFiles to clean:\n1. `internal/storage/sqlite/hash.go` - `computeIssueContentHash` (line 17)\n - Check if entire file can be deleted if only contains this function\n\n2. `internal/config/config.go` - `FileUsed` (line 151)\n - Delete unused config helper\n\n3. `cmd/bd/git_sync_test.go` - `verifyIssueOpen` (line 300)\n - Delete dead test helper\n\n4. `internal/compact/haiku.go` - `HaikuClient.SummarizeTier2` (line 81)\n - Tier 2 summarization not implemented\n - Options: implement feature OR delete method\n\nImpact: Removes 50-100 LOC depending on decisions","acceptance_criteria":"- Remove unreachable functions\n- If entire files can be deleted (like hash.go), delete them\n- For SummarizeTier2: decide to implement or delete, document decision\n- All tests pass: `go test ./...`\n- Verify no callers exist for each function","status":"closed","priority":2,"issue_type":"task","created_at":"2025-10-27T20:30:19.963392-07:00","updated_at":"2025-10-27T20:44:57.549063-07:00","closed_at":"2025-10-27T20:44:57.549063-07:00","labels":["cleanup","dead-code","phase-1"],"dependencies":[{"issue_id":"bd-214","depends_on_id":"bd-209","type":"parent-child","created_at":"2025-10-27T20:30:19.968126-07:00","created_by":"daemon"}]} {"id":"bd-215","title":"Split internal/rpc/server.go into focused modules","description":"The file `internal/rpc/server.go` is 2,273 lines with 50+ methods, making it difficult to navigate and prone to merge conflicts. Split into 8 focused files with clear responsibilities.\n\nCurrent structure: Single 2,273-line file with:\n- Connection handling\n- Request routing\n- All 40+ RPC method implementations\n- Storage caching\n- Health checks \u0026 metrics\n- Cleanup loops\n\nTarget structure:\n```\ninternal/rpc/\n├── server.go # Core server, connection handling (~300 lines)\n├── methods_issue.go # Issue operations (~400 lines)\n├── methods_deps.go # Dependency operations (~200 lines)\n├── methods_labels.go # Label operations (~150 lines)\n├── methods_ready.go # Ready work queries (~150 lines)\n├── methods_compact.go # Compaction operations (~200 lines)\n├── methods_comments.go # Comment operations (~150 lines)\n├── storage_cache.go # Storage caching logic (~300 lines)\n└── health.go # Health \u0026 metrics (~200 lines)\n```\n\nMigration strategy:\n1. Create new files with appropriate methods\n2. Keep `server.go` as main file with core server logic\n3. Test incrementally after each file split\n4. Final verification with full test suite","acceptance_criteria":"- All 50 methods split into appropriate files\n- Each file \u003c500 LOC\n- All methods remain on `*Server` receiver (no behavior change)\n- All tests pass: `go test ./internal/rpc/...`\n- Verify daemon works: start daemon, run operations, check health\n- Update internal documentation if needed\n- No change to public API","status":"open","priority":1,"issue_type":"task","created_at":"2025-10-27T20:30:47.870646-07:00","updated_at":"2025-10-27T20:30:47.870646-07:00","labels":["architecture","phase-2","refactor"],"dependencies":[{"issue_id":"bd-215","depends_on_id":"bd-209","type":"parent-child","created_at":"2025-10-27T20:30:47.872396-07:00","created_by":"daemon"}]} {"id":"bd-216","title":"Extract SQLite migrations into separate files","description":"The file `internal/storage/sqlite/sqlite.go` is 2,136 lines and contains 11 sequential migrations alongside core storage logic. Extract migrations into a versioned system.\n\nCurrent issues:\n- 11 migration functions mixed with core logic\n- Hard to see migration history\n- Sequential migrations slow database open\n- No clear migration versioning\n\nMigration functions to extract:\n- `migrateDirtyIssuesTable()`\n- `migrateIssueCountersTable()`\n- `migrateExternalRefColumn()`\n- `migrateCompositeIndexes()`\n- `migrateClosedAtConstraint()`\n- `migrateCompactionColumns()`\n- `migrateSnapshotsTable()`\n- `migrateCompactionConfig()`\n- `migrateCompactedAtCommitColumn()`\n- `migrateExportHashesTable()`\n- Plus 1 more (11 total)\n\nTarget structure:\n```\ninternal/storage/sqlite/\n├── sqlite.go # Core storage (~800 lines)\n├── schema.go # Table definitions (~200 lines)\n├── migrations.go # Migration orchestration (~200 lines)\n└── migrations/ # Individual migrations\n ├── 001_initial_schema.go\n ├── 002_dirty_issues.go\n ├── 003_issue_counters.go\n [... through 011_export_hashes.go]\n```\n\nBenefits:\n- Clear migration history\n- Each migration self-contained\n- Easier to review migration changes in PRs\n- Future migrations easier to add","acceptance_criteria":"- All 11 migrations extracted to separate files\n- Migration version tracking in database\n- Migrations run in order on fresh database\n- Existing databases upgrade correctly\n- All tests pass: `go test ./internal/storage/sqlite/...`\n- Database initialization time unchanged or improved\n- Add migration rollback capability (optional, nice-to-have)","status":"open","priority":2,"issue_type":"task","created_at":"2025-10-27T20:30:47.870671-07:00","updated_at":"2025-10-27T20:30:47.870671-07:00","labels":["database","phase-2","refactor"],"dependencies":[{"issue_id":"bd-216","depends_on_id":"bd-209","type":"parent-child","created_at":"2025-10-27T20:30:47.875564-07:00","created_by":"daemon"}]} diff --git a/cmd/bd/import_collision_test.go b/cmd/bd/import_collision_test.go index f146a6bb..5ebf01c2 100644 --- a/cmd/bd/import_collision_test.go +++ b/cmd/bd/import_collision_test.go @@ -219,149 +219,6 @@ func TestImportMultipleCollisions(t *testing.T) { } } -// TestImportDependencyUpdates tests that dependencies are updated during remapping -// SKIPPED: This test expects the OLD buggy behavior where existing issue dependencies pointing -// to a collided ID get retargeted to the remapped ID. Per GH issue #120, this is incorrect. -// Existing dependencies should remain unchanged. Only dependencies from imported issues should -// be remapped. This test needs to be rewritten to test the correct import semantics. -func TestImportDependencyUpdates(t *testing.T) { - t.Skip("Test expects old buggy behavior - needs rewrite for GH#120 fix") - tmpDir, err := os.MkdirTemp("", "bd-collision-test-*") - if err != nil { - t.Fatalf("Failed to create temp dir: %v", err) - } - defer func() { - if err := os.RemoveAll(tmpDir); err != nil { - t.Logf("Warning: cleanup failed: %v", err) - } - }() - - dbPath := filepath.Join(tmpDir, "test.db") - testStore := newTestStoreWithPrefix(t, dbPath, "bd") - - ctx := context.Background() - - // Create existing issues with dependencies - issue1 := &types.Issue{ - ID: "bd-10", - Title: "Issue 1", - Status: types.StatusOpen, - Priority: 1, - IssueType: types.TypeTask, - CreatedAt: time.Now(), - UpdatedAt: time.Now(), - } - issue2 := &types.Issue{ - ID: "bd-11", - Title: "Issue 2", - Status: types.StatusOpen, - Priority: 1, - IssueType: types.TypeTask, - CreatedAt: time.Now(), - UpdatedAt: time.Now(), - } - issue3 := &types.Issue{ - ID: "bd-12", - Title: "Issue 3", - Status: types.StatusOpen, - Priority: 1, - IssueType: types.TypeTask, - CreatedAt: time.Now(), - UpdatedAt: time.Now(), - } - - if err := testStore.CreateIssue(ctx, issue1, "test"); err != nil { - t.Fatalf("Failed to create issue 1: %v", err) - } - if err := testStore.CreateIssue(ctx, issue2, "test"); err != nil { - t.Fatalf("Failed to create issue 2: %v", err) - } - if err := testStore.CreateIssue(ctx, issue3, "test"); err != nil { - t.Fatalf("Failed to create issue 3: %v", err) - } - - // Add dependencies: bd-1 → bd-2, bd-3 → bd-2 - dep1 := &types.Dependency{ - IssueID: "bd-10", - DependsOnID: "bd-11", - Type: types.DepBlocks, - } - dep2 := &types.Dependency{ - IssueID: "bd-12", - DependsOnID: "bd-11", - Type: types.DepBlocks, - } - - if err := testStore.AddDependency(ctx, dep1, "test"); err != nil { - t.Fatalf("Failed to add dep1: %v", err) - } - if err := testStore.AddDependency(ctx, dep2, "test"); err != nil { - t.Fatalf("Failed to add dep2: %v", err) - } - - // Import colliding bd-2 - incomingIssues := []*types.Issue{ - { - ID: "bd-11", - Title: "Modified Issue 2", - Description: "Changed", - Status: types.StatusOpen, - Priority: 2, - IssueType: types.TypeBug, - }, - } - - result, err := sqlite.DetectCollisions(ctx, testStore, incomingIssues) - if err != nil { - t.Fatalf("DetectCollisions failed: %v", err) - } - - if len(result.Collisions) != 1 { - t.Fatalf("Expected 1 collision, got %d", len(result.Collisions)) - } - - // Resolve collision - allExisting, _ := testStore.SearchIssues(ctx, "", types.IssueFilter{}) - if err := sqlite.ScoreCollisions(ctx, testStore, result.Collisions, allExisting); err != nil { - t.Fatalf("ScoreCollisions failed: %v", err) - } - - idMapping, err := sqlite.RemapCollisions(ctx, testStore, result.Collisions, allExisting) - if err != nil { - t.Fatalf("RemapCollisions failed: %v", err) - } - - newID := idMapping["bd-11"] - if newID == "" { - t.Fatal("bd-2 not remapped") - } - - // Verify dependencies were updated - // bd-1 should now depend on newID - deps1, err := testStore.GetDependencyRecords(ctx, "bd-10") - if err != nil { - t.Fatalf("Failed to get deps for bd-1: %v", err) - } - if len(deps1) != 1 { - t.Fatalf("Expected 1 dependency for bd-1, got %d", len(deps1)) - } - if deps1[0].DependsOnID != newID { - t.Errorf("bd-1 dependency not updated: %s, want %s", deps1[0].DependsOnID, newID) - } - - // bd-3 should now depend on newID - deps3, err := testStore.GetDependencyRecords(ctx, "bd-12") - if err != nil { - t.Fatalf("Failed to get deps for bd-3: %v", err) - } - if len(deps3) != 1 { - t.Fatalf("Expected 1 dependency for bd-3, got %d", len(deps3)) - } - if deps3[0].DependsOnID != newID { - t.Errorf("bd-3 dependency not updated: %s, want %s", deps3[0].DependsOnID, newID) - } -} - // TestImportTextReferenceUpdates tests that text references are updated during remapping func TestImportTextReferenceUpdates(t *testing.T) { tmpDir, err := os.MkdirTemp("", "bd-collision-test-*") @@ -498,103 +355,6 @@ func TestImportTextReferenceUpdates(t *testing.T) { } } -// TestImportChainDependencies tests remapping with chained dependencies -// SKIPPED: This test expects the OLD buggy behavior where existing issue dependencies pointing -// to a collided ID get retargeted to the remapped ID. Per GH issue #120, this is incorrect. -func TestImportChainDependencies(t *testing.T) { - t.Skip("Test expects old buggy behavior - needs rewrite for GH#120 fix") - tmpDir, err := os.MkdirTemp("", "bd-collision-test-*") - if err != nil { - t.Fatalf("Failed to create temp dir: %v", err) - } - defer func() { - if err := os.RemoveAll(tmpDir); err != nil { - t.Logf("Warning: cleanup failed: %v", err) - } - }() - - dbPath := filepath.Join(tmpDir, "test.db") - testStore := newTestStoreWithPrefix(t, dbPath, "bd") - - ctx := context.Background() - - // Create chain: bd-100 → bd-101 → bd-102 → bd-103 - for i := 100; i <= 103; i++ { - issue := &types.Issue{ - ID: fmt.Sprintf("bd-%d", i), - Title: fmt.Sprintf("Issue %d", i), - Status: types.StatusOpen, - Priority: 1, - IssueType: types.TypeTask, - CreatedAt: time.Now(), - UpdatedAt: time.Now(), - } - if err := testStore.CreateIssue(ctx, issue, "test"); err != nil { - t.Fatalf("Failed to create issue %d: %v", i, err) - } - } - - // Add chain dependencies - for i := 100; i <= 102; i++ { - dep := &types.Dependency{ - IssueID: fmt.Sprintf("bd-%d", i), - DependsOnID: fmt.Sprintf("bd-%d", i+1), - Type: types.DepBlocks, - } - if err := testStore.AddDependency(ctx, dep, "test"); err != nil { - t.Fatalf("Failed to add dependency %d: %v", i, err) - } - } - - // Import colliding bd-101 - incomingIssues := []*types.Issue{ - { - ID: "bd-101", - Title: "Modified Issue 101", - Status: types.StatusInProgress, - Priority: 2, - IssueType: types.TypeBug, - }, - } - - result, err := sqlite.DetectCollisions(ctx, testStore, incomingIssues) - if err != nil { - t.Fatalf("DetectCollisions failed: %v", err) - } - - // Resolve collision - allExisting, _ := testStore.SearchIssues(ctx, "", types.IssueFilter{}) - if err := sqlite.ScoreCollisions(ctx, testStore, result.Collisions, allExisting); err != nil { - t.Fatalf("ScoreCollisions failed: %v", err) - } - - idMapping, err := sqlite.RemapCollisions(ctx, testStore, result.Collisions, allExisting) - if err != nil { - t.Fatalf("RemapCollisions failed: %v", err) - } - - newID := idMapping["bd-101"] - - // Verify chain is maintained - // bd-100 → newID (was bd-101) - deps1, _ := testStore.GetDependencyRecords(ctx, "bd-100") - if len(deps1) != 1 || deps1[0].DependsOnID != newID { - t.Errorf("bd-100 dependency broken: %v", deps1) - } - - // newID → bd-102 - depsNew, _ := testStore.GetDependencyRecords(ctx, newID) - if len(depsNew) != 1 || depsNew[0].DependsOnID != "bd-102" { - t.Errorf("newID dependency broken: %v", depsNew) - } - - // bd-102 → bd-103 (unchanged) - deps3, _ := testStore.GetDependencyRecords(ctx, "bd-102") - if len(deps3) != 1 || deps3[0].DependsOnID != "bd-103" { - t.Errorf("bd-102 dependency broken: %v", deps3) - } -} - // TestImportPartialIDMatch tests word boundary matching (bd-10 vs bd-100) func TestImportPartialIDMatch(t *testing.T) { tmpDir, err := os.MkdirTemp("", "bd-collision-test-*") diff --git a/internal/rpc/client.go b/internal/rpc/client.go index 7a7c9071..e65a40e1 100644 --- a/internal/rpc/client.go +++ b/internal/rpc/client.go @@ -307,11 +307,6 @@ func (c *Client) Export(args *ExportArgs) (*Response, error) { return c.Execute(OpExport, args) } -// Import imports issues from JSONL format -func (c *Client) Import(args *ImportArgs) (*Response, error) { - return c.Execute(OpImport, args) -} - // EpicStatus gets epic completion status via the daemon func (c *Client) EpicStatus(args *EpicStatusArgs) (*Response, error) { return c.Execute(OpEpicStatus, args) diff --git a/internal/rpc/server.go b/internal/rpc/server.go index 31f42789..5444f452 100644 --- a/internal/rpc/server.go +++ b/internal/rpc/server.go @@ -2112,20 +2112,6 @@ func (s *Server) handleShutdown(_ *Request) Response { } } -// GetLastImportTime returns the last JSONL import timestamp -func (s *Server) GetLastImportTime() time.Time { - s.importMu.RLock() - defer s.importMu.RUnlock() - return s.lastImportTime -} - -// SetLastImportTime updates the last JSONL import timestamp -func (s *Server) SetLastImportTime(t time.Time) { - s.importMu.Lock() - defer s.importMu.Unlock() - s.lastImportTime = t -} - // checkAndAutoImportIfStale checks if JSONL is newer than last import and triggers auto-import // This fixes bd-132: daemon shows stale data after git pull func (s *Server) checkAndAutoImportIfStale(req *Request) error { @@ -2250,24 +2236,3 @@ func (s *Server) triggerExport(ctx context.Context, store storage.Storage, dbPat return nil } - -// findJSONLPath finds the JSONL file path for the request's repository -func (s *Server) findJSONLPath(req *Request) string { - // Extract repo root from request's working directory - // For now, use a simple heuristic: look for .beads/ in request's cwd - beadsDir := filepath.Join(req.Cwd, ".beads") - - // Try canonical filenames in order - candidates := []string{ - filepath.Join(beadsDir, "beads.jsonl"), - filepath.Join(beadsDir, "issues.jsonl"), - } - - for _, path := range candidates { - if _, err := os.Stat(path); err == nil { - return path - } - } - - return "" -} diff --git a/internal/storage/sqlite/collision_test.go b/internal/storage/sqlite/collision_test.go index bf7cb4d9..4dc3fba1 100644 --- a/internal/storage/sqlite/collision_test.go +++ b/internal/storage/sqlite/collision_test.go @@ -911,145 +911,6 @@ func TestRemapCollisions(t *testing.T) { } } -// SKIPPED: This test expects the OLD buggy behavior where existing issue dependencies -// get updated during collision resolution. Per GH issue #120, existing dependencies -// should be preserved, not updated. This test needs to be rewritten to test the correct -// semantics (only update dependencies belonging to remapped issues, not existing ones). -func TestUpdateDependencyReferences(t *testing.T) { - t.Skip("Test expects old buggy behavior - needs rewrite for GH#120 fix") - // Create temporary database - tmpDir, err := os.MkdirTemp("", "dep-remap-test-*") - if err != nil { - t.Fatalf("failed to create temp dir: %v", err) - } - defer os.RemoveAll(tmpDir) - - dbPath := filepath.Join(tmpDir, "test.db") - store, err := New(dbPath) - if err != nil { - t.Fatalf("failed to create storage: %v", err) - } - defer store.Close() - - ctx := context.Background() - - // Create issues - issue1 := &types.Issue{ - ID: "bd-1", - Title: "Issue 1", - Description: "First issue", - Status: types.StatusOpen, - Priority: 1, - IssueType: types.TypeTask, - } - - issue2 := &types.Issue{ - ID: "bd-2", - Title: "Issue 2", - Description: "Second issue", - Status: types.StatusOpen, - Priority: 1, - IssueType: types.TypeTask, - } - - issue3 := &types.Issue{ - ID: "bd-3", - Title: "Issue 3", - Description: "Third issue", - Status: types.StatusOpen, - Priority: 1, - IssueType: types.TypeTask, - } - - // Create the new (remapped) issue - issue100 := &types.Issue{ - ID: "bd-100", - Title: "Remapped Issue (was bd-2)", - Description: "This is the remapped version", - Status: types.StatusOpen, - Priority: 1, - IssueType: types.TypeTask, - } - - if err := store.CreateIssue(ctx, issue1, "test"); err != nil { - t.Fatalf("failed to create issue1: %v", err) - } - if err := store.CreateIssue(ctx, issue2, "test"); err != nil { - t.Fatalf("failed to create issue2: %v", err) - } - if err := store.CreateIssue(ctx, issue3, "test"); err != nil { - t.Fatalf("failed to create issue3: %v", err) - } - if err := store.CreateIssue(ctx, issue100, "test"); err != nil { - t.Fatalf("failed to create issue100: %v", err) - } - - // Add dependencies - // bd-1 depends on bd-2 - dep1 := &types.Dependency{ - IssueID: "bd-1", - DependsOnID: "bd-2", - Type: types.DepBlocks, - } - // bd-3 depends on bd-2 - dep2 := &types.Dependency{ - IssueID: "bd-3", - DependsOnID: "bd-2", - Type: types.DepBlocks, - } - - if err := store.AddDependency(ctx, dep1, "test"); err != nil { - t.Fatalf("failed to add dep1: %v", err) - } - if err := store.AddDependency(ctx, dep2, "test"); err != nil { - t.Fatalf("failed to add dep2: %v", err) - } - - // Create ID mapping (bd-2 was remapped to bd-100) - idMapping := map[string]string{ - "bd-2": "bd-100", - } - - // Update dependency references - if err := updateDependencyReferences(ctx, store, idMapping); err != nil { - t.Fatalf("updateDependencyReferences failed: %v", err) - } - - // Verify dependencies were updated - // bd-1 should now depend on bd-100 - deps1, err := store.GetDependencyRecords(ctx, "bd-1") - if err != nil { - t.Fatalf("failed to get deps for bd-1: %v", err) - } - if len(deps1) != 1 { - t.Fatalf("expected 1 dependency for bd-1, got %d", len(deps1)) - } - if deps1[0].DependsOnID != "bd-100" { - t.Errorf("expected bd-1 to depend on bd-100, got %s", deps1[0].DependsOnID) - } - - // bd-3 should now depend on bd-100 - deps3, err := store.GetDependencyRecords(ctx, "bd-3") - if err != nil { - t.Fatalf("failed to get deps for bd-3: %v", err) - } - if len(deps3) != 1 { - t.Fatalf("expected 1 dependency for bd-3, got %d", len(deps3)) - } - if deps3[0].DependsOnID != "bd-100" { - t.Errorf("expected bd-3 to depend on bd-100, got %s", deps3[0].DependsOnID) - } - - // Old dependency bd-2 should not have any dependencies anymore - deps2, err := store.GetDependencyRecords(ctx, "bd-2") - if err != nil { - t.Fatalf("failed to get deps for bd-2: %v", err) - } - if len(deps2) != 0 { - t.Errorf("expected 0 dependencies for bd-2, got %d", len(deps2)) - } -} - // BenchmarkReplaceIDReferences benchmarks the old approach (compiling regex every time) func BenchmarkReplaceIDReferences(b *testing.B) { // Simulate a realistic scenario: 10 ID mappings