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

Fix tests after --resolve-collisions removal

Browse Source 
Remove obsolete test file and update remaining tests to not use the
removed --resolve-collisions flag. Hash-based IDs make collision
resolution unnecessary.

Changes:
- Delete internal/rpc/server_autoimport_test.go (obsolete)
- Remove --resolve-collisions from beads_nway_test.go
- Remove --resolve-collisions from beads_twoclone_test.go
- Remove --resolve-collisions from cmd/bd/sync.go

All tests now pass.
This commit is contained in:
Steve Yegge
2025-10-31 01:25:05 -07:00
parent 0725c33fcc
commit 9225114c0b
5 changed files with 9 additions and 65 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
.beads/beads.jsonl
View File

@@ -33,6 +33,7 @@
{"id":"bd-3240cd79","content_hash":"3e8b654c9dd6a631db40adc23bf6b7e291c13f977dceaf08d447963f99b3124d","title":"Another hash test","description":"","status":"closed","priority":2,"issue_type":"task","created_at":"2025-10-30T14:27:28.658778-07:00","updated_at":"2025-10-30T17:05:26.02898-07:00","closed_at":"2025-10-30T14:27:33.073977-07:00"}
{"id":"bd-325da116","content_hash":"d7c5637527778c5c835f5e4b6e15fbd51a3476d6749ab3155b8aeac08a8ef339","title":"Fix N-way collision convergence","description":"Epic to fix the N-way collision convergence problem documented in n-way-collision-convergence.md.\n\n## Problem Summary\nThe current collision resolution implementation works correctly for 2-way collisions but does not converge for 3-way (and by extension N-way) collisions. TestThreeCloneCollision demonstrates this with reproducible failures.\n\n## Root Causes Identified\n1. Pairwise resolution doesn't scale - each clone makes local decisions without global context\n2. DetectCollisions modifies state during detection (line 83-86 in collision.go)\n3. No remapping history - can't track transitive remap chains (test-1 → test-2 → test-3)\n4. Import-time resolution is too late - happens after git merge\n\n## Solution Architecture\nReplace pairwise resolution with deterministic global N-way resolution using:\n- Content-addressable identity (content hashing)\n- Global collision resolution (sort all versions by hash)\n- Read-only detection phase (separate from modification)\n- Idempotent imports (content-first matching)\n\n## Success Criteria\n- TestThreeCloneCollision passes without skipping\n- All clones converge to identical content after final pull\n- No data loss (all issues present in all clones)\n- Works for N workers (test with 5+ clones)\n- Idempotent imports (importing same JSONL multiple times is safe)\n\n## Implementation Phases\nSee child issues for detailed breakdown of each phase.","status":"closed","priority":0,"issue_type":"epic","created_at":"2025-10-29T23:05:13.889079-07:00","updated_at":"2025-10-30T20:02:31.539548-07:00","closed_at":"2025-10-30T20:02:31.539548-07:00"}
{"id":"bd-3262e055","content_hash":"0c150383d4c2ce7aeecf70fe53f2599e9720eccffc7ab717a2abfef8e37f9dcc","title":"Deep nested","description":"","status":"closed","priority":2,"issue_type":"task","created_at":"2025-10-30T15:46:53.570315-07:00","updated_at":"2025-10-30T17:05:26.031513-07:00","closed_at":"2025-10-30T15:46:59.619236-07:00"}
{"id":"bd-34e79e","content_hash":"29d56b87e8a862dfff904eea51d5893fd757989229b9b40dd1acaa8c8cd93748","title":"Rebase and integrate PR #161: Mermaid.js dependency tree visualization","description":"PR #161 by @mrdavidlaing adds `bd dep tree --format mermaid` for visualizing dependency trees as Mermaid.js diagrams.\n\n**Status:** 186 commits behind main (from Oct 27), JSONL merge conflict, all CI checks failed\n\n**What needs doing:**\n1. Rebase onto current main (handle JSONL conflict - our database is wholly different now)\n2. Review code changes - may need reimplementation due to storage layer changes\n3. Update tests to work with current codebase\n4. Ensure mermaid output logic still valid\n5. Get CI passing\n6. Merge\n\n**Original features (worth preserving):**\n- Status indicators: ☐ open, ◧ in_progress, ⚠ blocked, ☑ closed\n- Theme-agnostic design\n- Works with --reverse flag\n- Well-tested with TDD\n- Clean implementation per steveyegge review","status":"open","priority":2,"issue_type":"task","created_at":"2025-10-31T01:16:58.185175-07:00","updated_at":"2025-10-31T01:16:58.185175-07:00","external_ref":"github:161"}
{"id":"bd-36320a04","content_hash":"883eb385fa9eded3826008fa6db3b842cabb2ce0e93a23293449f65024303fb7","title":"Add mutation channel to internal/rpc/server.go","description":"Add mutationChan chan MutationEvent to Server struct. Emit events on CreateIssue, UpdateIssue, DeleteIssue, AddComment. Non-blocking send with default case for full channel.","status":"open","priority":1,"issue_type":"task","created_at":"2025-10-29T19:42:29.860173-07:00","updated_at":"2025-10-30T17:05:26.040943-07:00"}
{"id":"bd-36870264","content_hash":"e88e5d98a2a5bebc38b3ac505b00687bfe78bd72654bd0c756bceee4a01e15f5","title":"Enforce daemon singleton per workspace with file locking","description":"Agent in ~/src/wyvern discovered 4 simultaneous daemon processes running, causing infinite directory recursion (.beads/.beads/.beads/...). Each daemon used relative paths and created nested .beads/ directories.\n\nRoot cause: No singleton enforcement. Multiple `bd daemon` processes can start in same workspace.\n\nExpected: One daemon per workspace (each workspace = separate .beads/ dir with bd.sock)\nActual: Multiple daemons can run simultaneously in same workspace\n\nNote: Separate git clones = separate workspaces = separate daemons (correct). Git worktrees share .beads/ and have known limitations (documented, use --no-daemon).","design":"Use flock (file locking) on daemon socket or database file to enforce singleton:\n\n1. On daemon start, attempt exclusive lock on .beads/bd.sock or .beads/daemon.lock\n2. If lock held by another process, refuse to start (exit with clear error)\n3. Hold lock for lifetime of daemon process\n4. Release lock on daemon shutdown\n\nAlternative: Use PID file with stale detection (check if PID is still running)\n\nImplementation location: Daemon startup code in cmd/bd/ or internal/daemon/","acceptance_criteria":"1. Starting second daemon process in same workspace fails with clear error\n2. Test: Start daemon, attempt second start, verify failure\n3. Killing daemon releases lock, allowing new daemon to start\n4. No infinite .beads/ directory recursion possible\n5. Works correctly with auto-start mechanism","status":"in_progress","priority":0,"issue_type":"bug","created_at":"2025-10-25T23:13:12.269549-07:00","updated_at":"2025-10-30T17:05:26.038883-07:00"}
{"id":"bd-37f7","content_hash":"72cb1f58d7ad6a91d3a02233bf4e78d0992834c15fbd8c6d9dc56df7095dc0a3","title":"Remove or archive collision resolution documentation","description":"Clean up documentation that's specific to sequential ID collision resolution:\n- docs/collision-resolution-failure-analysis.md (can archive or remove)\n- Remove collision resolution sections from AGENTS.md, FAQ.md, TROUBLESHOOTING.md\n- Update HASH_ID_DESIGN.md to reflect that collision resolution is fully removed\n- Update CHANGELOG.md to note the removal\n- Keep only minimal docs about hash ID accidental collisions","status":"open","priority":2,"issue_type":"chore","created_at":"2025-10-30T22:16:48.499529-07:00","updated_at":"2025-10-30T22:16:48.499529-07:00","labels":["cleanup","documentation","hash-ids"]}

7
beads_nway_test.go
View File

@@ -191,8 +191,8 @@ func syncCloneWithConflictResolution(t *testing.T, cloneDir, name string, isFirs
runCmd(t, cloneDir, "git", "commit", "-m", "Resolve merge conflict")
}
// Import with collision resolution
runCmdWithEnv(t, cloneDir, map[string]string{"BEADS_NO_DAEMON": "1"}, "./bd", "import", "-i", ".beads/issues.jsonl", "--resolve-collisions")
// Import (no collision resolution needed with hash IDs)
runCmdWithEnv(t, cloneDir, map[string]string{"BEADS_NO_DAEMON": "1"}, "./bd", "import", "-i", ".beads/issues.jsonl")
runCmd(t, cloneDir, "git", "push", "origin", "master")
}
}
@@ -216,8 +216,7 @@ func finalPullForClone(t *testing.T, cloneDir, name string) {
}
// Import JSONL to update database
// Use --resolve-collisions to handle any remaining ID conflicts
runCmdOutputWithEnvAllowError(t, cloneDir, map[string]string{"BEADS_NO_DAEMON": "1"}, true, "./bd", "import", "-i", ".beads/issues.jsonl", "--resolve-collisions")
runCmdOutputWithEnvAllowError(t, cloneDir, map[string]string{"BEADS_NO_DAEMON": "1"}, true, "./bd", "import", "-i", ".beads/issues.jsonl")
}
// getTitlesFromClone extracts all issue titles from a clone's database

6
beads_twoclone_test.go
View File

@@ -551,7 +551,7 @@ func testThreeCloneCollisionWithSyncOrder(t *testing.T, first, second, third str
}
// Import with collision resolution
runCmd(t, secondDir, "./bd", "import", "-i", ".beads/issues.jsonl", "--resolve-collisions")
runCmd(t, secondDir, "./bd", "import", "-i", ".beads/issues.jsonl")
runCmd(t, secondDir, "git", "push", "origin", "master")
_ = pullOut
}
@@ -590,7 +590,7 @@ func testThreeCloneCollisionWithSyncOrder(t *testing.T, first, second, third str
}
// Import with collision resolution
runCmd(t, thirdDir, "./bd", "import", "-i", ".beads/issues.jsonl", "--resolve-collisions")
runCmd(t, thirdDir, "./bd", "import", "-i", ".beads/issues.jsonl")
runCmd(t, thirdDir, "git", "push", "origin", "master")
_ = pullOut
}
@@ -623,7 +623,7 @@ func testThreeCloneCollisionWithSyncOrder(t *testing.T, first, second, third str
}
}
// Import JSONL to update database (but don't use --resolve-collisions to avoid creating duplicates)
// Import JSONL to update database
runCmdOutputAllowError(t, clone, "./bd", "import", "-i", ".beads/issues.jsonl")
}

4
cmd/bd/sync.go
View File

@@ -483,12 +483,12 @@ func importFromJSONL(ctx context.Context, jsonlPath string, renameOnImport bool)
}
// Build args for import command
args := []string{"import", "-i", jsonlPath, "--resolve-collisions"}
args := []string{"import", "-i", jsonlPath}
if renameOnImport {
args = append(args, "--rename-on-import")
}
// Run import command with --resolve-collisions to automatically handle conflicts
// Run import command
cmd := exec.CommandContext(ctx, exe, args...) // #nosec G204 - bd import command from trusted binary
output, err := cmd.CombinedOutput()
if err != nil {

56
internal/rpc/server_autoimport_test.go
View File

@@ -1,56 +0,0 @@
package rpc
import (
"testing"
"github.com/steveyegge/beads/internal/importer"
)
// TestAutoImportDoesNotUseResolveCollisions ensures auto-import NEVER uses
// ResolveCollisions flag. That flag is ONLY for explicit user-driven imports
// (bd import --resolve-collisions) when merging branches.
//
// Auto-import should update existing issues by ID, not create duplicates.
// Using ResolveCollisions in auto-import causes catastrophic duplicate creation
// where every git pull creates new duplicate issues, leading to endless ping-pong
// between agents (bd-247).
//
// This test enforces that checkAndAutoImportIfStale uses ResolveCollisions: false.
func TestAutoImportDoesNotUseResolveCollisions(t *testing.T) {
// This is a compile-time and code inspection test.
// We verify the Options struct used in checkAndAutoImportIfStale.
// The correct options for auto-import
correctOpts := importer.Options{
ResolveCollisions: false, // MUST be false for auto-import
RenameOnImport: true, // Safe: handles prefix mismatches
// SkipPrefixValidation is false by default
}
// Verify ResolveCollisions is false
if correctOpts.ResolveCollisions {
t.Fatal("Auto-import MUST NOT use ResolveCollisions=true. This causes duplicate creation (bd-247).")
}
// This test will fail if someone changes the auto-import code to use ResolveCollisions.
// To fix this test, you need to:
// 1. Change server_export_import_auto.go line ~221 to ResolveCollisions: false
// 2. Add a comment explaining why it must be false
// 3. Update AGENTS.md to document the auto-import behavior
}
// TestResolveCollisionsOnlyForExplicitImport documents when ResolveCollisions
// should be used: ONLY for explicit user-driven imports from CLI.
func TestResolveCollisionsOnlyForExplicitImport(t *testing.T) {
t.Log("ResolveCollisions should ONLY be used for:")
t.Log(" - bd import --resolve-collisions (user explicitly requested)")
t.Log(" - Branch merge scenarios (different issues with same ID)")
t.Log("")
t.Log("ResolveCollisions should NEVER be used for:")
t.Log(" - Auto-import after git pull (daemon/auto-sync)")
t.Log(" - Background JSONL updates")
t.Log(" - Normal agent synchronization")
t.Log("")
t.Log("Violation causes: Endless duplicate creation, database pollution, ping-pong commits")
t.Log("See: bd-247 for catastrophic failure caused by this bug")
}