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

fix: Add collision detection to auto-import (bd-228)

Browse Source 
CRITICAL FIX: Auto-import was silently overwriting local changes without any
collision detection or warning. This caused data loss in multi-developer workflows.

Changes:
- Auto-import now uses sqlite.DetectCollisions() before importing
- Colliding issues are skipped (preserves local changes)
- Warning printed with list of skipped issues and resolution instructions
- Added autoImportWithoutCollisionDetection() fallback for non-SQLite backends
- All tests pass

Impact:
- Local changes are now preserved during git pull
- Users are informed when collisions occur
- Can manually resolve with 'bd import --resolve-collisions'
- No more silent data corruption

Also:
- Removed critical warning banner from README
- Created bd-229 for data recovery investigation
- Closed bd-228 as fixed

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Steve Yegge
2025-10-15 02:11:42 -07:00
parent 38ae26d9e9
commit 6b88d60d6e
3 changed files with 110 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
.beads/issues.jsonl
View File

@@ -117,7 +117,8 @@
{"id":"bd-225","title":"Ultrathink: Choose solution for status/closed_at inconsistency (bd-224)","description":"Deep analysis of solution options for bd-224 data model inconsistency issue.\n\nContext:\n- Target users: individual devs and small teams\n- Future: swarms of agent workers\n- Brand-new codebase with few users (can break things)\n- Issue: status='open' with closed_at!=NULL creates liminal state\n\nOptions to evaluate:\nA) Database CHECK constraint\nB) Application-level enforcement \nC) Add explicit reopened event\nD) Remove closed_at field entirely\n\nAnalysis framework:\n1. Simplicity (mental model for devs)\n2. Robustness (hard to break, especially for agent swarms)\n3. Migration cost (schema changes, data cleanup)\n4. Future extensibility\n5. Performance\n6. Consistency guarantees\n\nNeed to determine best approach given tradeoffs.","notes":"Analysis complete. Recommendation: Hybrid approach with DB CHECK constraint + smart UpdateIssue + import enforcement + reopen command.\n\nKey insights:\n- DB constraint provides defense-in-depth perfect for agent swarms\n- Statistics calculation currently uses 'closed_at IS NOT NULL' which is BROKEN by inconsistent data\n- UpdateIssue and Import don't manage the invariant\n- EventReopened exists but is unused\n\nSee ULTRATHINK_BD224.md for full analysis.","status":"closed","priority":1,"issue_type":"task","created_at":"2025-10-15T01:47:25.564925-07:00","updated_at":"2025-10-15T01:58:30.041321-07:00","closed_at":"2025-10-15T01:49:43.078431-07:00","dependencies":[{"issue_id":"bd-225","depends_on_id":"bd-224","type":"discovered-from","created_at":"2025-10-15T01:47:25.567014-07:00","created_by":"stevey"}]}
{"id":"bd-226","title":"Epic: Fix status/closed_at inconsistency (bd-224 solution)","description":"Implement hybrid solution to enforce status/closed_at invariant:\n- Database CHECK constraint\n- Smart UpdateIssue logic\n- Import enforcement\n- Reopen command\n\nThis is a data integrity issue that affects statistics and will cause problems for agent swarms. The hybrid approach provides defense-in-depth.\n\nSee ULTRATHINK_BD224.md for full analysis and rationale.\n\nParent of all implementation tasks for this fix.","status":"open","priority":1,"issue_type":"epic","created_at":"2025-10-15T01:58:41.041574-07:00","updated_at":"2025-10-15T01:58:41.041574-07:00"}
{"id":"bd-227","title":"Audit and document all inconsistent issues in database","description":"Before we add the constraint, we need to know what data is inconsistent.\n\nSteps:\n1. Query database for status/closed_at mismatches\n2. Document each case (how many, which issues, patterns)\n3. Decide on cleanup strategy (trust status vs trust closed_at)\n4. Create SQL script for cleanup\n\nOutput: Document with counts and cleanup SQL ready to review.\n\nThis unblocks the migration work.","status":"open","priority":1,"issue_type":"task","created_at":"2025-10-15T01:58:50.908363-07:00","updated_at":"2025-10-15T01:58:50.908363-07:00","dependencies":[{"issue_id":"bd-227","depends_on_id":"bd-226","type":"parent-child","created_at":"2025-10-15T01:58:50.909081-07:00","created_by":"stevey"}]}
{"id":"bd-228","title":"Critical: Auto-import silently overwrites local changes without collision detection","description":"## Problem\n\nAuto-import is silently overwriting local changes when pulling from Git, without using collision detection or warning the user.\n\n## Evidence\n\nIssue bd-89 timeline:\n1. 08:07:31 - Manual update: set status=in_progress\n2. 08:13:08 - Manual close: set status=closed, closed_at set\n3. 08:33:25 - **Auto-import silently overwrote to status=open** (discarded local change)\n4. 08:57:59 - Auto-import overwrote again\n5. 08:58:30 - Auto-import overwrote again\n\nQuery shows 10+ issues with multiple auto-import overwrites, including bd-198, bd-199, bd-200-206.\n\n## Root Cause Hypothesis\n\nWhen two clones both modify issue N:\n- Clone A: modifies bd-89, exports to JSONL, commits\n- Clone B: also modifies bd-89 differently, pulls from A\n- Auto-import uses UpdateIssue which **unconditionally overwrites**\n- Clone B's local changes are silently lost\n- No collision detection warning\n- References and work context corrupted\n\n## Impact\n\n- **Data loss**: Local changes silently discarded\n- **Corruption**: Issue state doesn't reflect actual work done\n- **Lost references**: Comments/deps referencing lost state are now wrong\n- **Trust**: Can't trust the database reflects reality\n- **Agent swarms**: Catastrophic for parallel workers\n\n## Expected Behavior\n\nAuto-import should:\n1. Detect collisions (same ID, different content)\n2. Warn user or fail import\n3. Offer resolution options (--resolve-collisions)\n4. Never silently overwrite without user confirmation\n\n## Current Behavior\n\nAuto-import silently overwrites using UpdateIssue, no collision detection.\n\n## Reproduction\n\n1. Clone A: bd update bd-1 --status in_progress\n2. Clone A: git add .beads/issues.jsonl \u0026\u0026 git commit \u0026\u0026 git push\n3. Clone B: bd update bd-1 --status closed \n4. Clone B: git pull (triggers auto-import)\n5. Result: bd-1 status silently reverted to in_progress, closed state lost\n\n## Files Involved\n\n- cmd/bd/main.go: auto-import trigger logic\n- cmd/bd/import.go: import without collision detection\n- internal/storage/sqlite/sqlite.go: UpdateIssue unconditionally overwrites","status":"open","priority":0,"issue_type":"bug","created_at":"2025-10-15T02:06:30.671918-07:00","updated_at":"2025-10-15T02:06:30.671918-07:00"}
{"id":"bd-228","title":"Critical: Auto-import silently overwrites local changes without collision detection","description":"## Problem\n\nAuto-import is silently overwriting local changes when pulling from Git, without using collision detection or warning the user.\n\n## Evidence\n\nIssue bd-89 timeline:\n1. 08:07:31 - Manual update: set status=in_progress\n2. 08:13:08 - Manual close: set status=closed, closed_at set\n3. 08:33:25 - **Auto-import silently overwrote to status=open** (discarded local change)\n4. 08:57:59 - Auto-import overwrote again\n5. 08:58:30 - Auto-import overwrote again\n\nQuery shows 10+ issues with multiple auto-import overwrites, including bd-198, bd-199, bd-200-206.\n\n## Root Cause Hypothesis\n\nWhen two clones both modify issue N:\n- Clone A: modifies bd-89, exports to JSONL, commits\n- Clone B: also modifies bd-89 differently, pulls from A\n- Auto-import uses UpdateIssue which **unconditionally overwrites**\n- Clone B's local changes are silently lost\n- No collision detection warning\n- References and work context corrupted\n\n## Impact\n\n- **Data loss**: Local changes silently discarded\n- **Corruption**: Issue state doesn't reflect actual work done\n- **Lost references**: Comments/deps referencing lost state are now wrong\n- **Trust**: Can't trust the database reflects reality\n- **Agent swarms**: Catastrophic for parallel workers\n\n## Expected Behavior\n\nAuto-import should:\n1. Detect collisions (same ID, different content)\n2. Warn user or fail import\n3. Offer resolution options (--resolve-collisions)\n4. Never silently overwrite without user confirmation\n\n## Current Behavior\n\nAuto-import silently overwrites using UpdateIssue, no collision detection.\n\n## Reproduction\n\n1. Clone A: bd update bd-1 --status in_progress\n2. Clone A: git add .beads/issues.jsonl \u0026\u0026 git commit \u0026\u0026 git push\n3. Clone B: bd update bd-1 --status closed \n4. Clone B: git pull (triggers auto-import)\n5. Result: bd-1 status silently reverted to in_progress, closed state lost\n\n## Files Involved\n\n- cmd/bd/main.go: auto-import trigger logic\n- cmd/bd/import.go: import without collision detection\n- internal/storage/sqlite/sqlite.go: UpdateIssue unconditionally overwrites","status":"closed","priority":0,"issue_type":"bug","created_at":"2025-10-15T02:06:30.671918-07:00","updated_at":"2025-10-15T02:11:30.794518-07:00","closed_at":"2025-10-15T02:11:30.794518-07:00"}
{"id":"bd-229","title":"Investigate data recovery for issues overwritten by auto-import bug","description":"Auto-import bug (bd-228) has been silently overwriting local changes since the feature was introduced. We need to investigate if any important data was lost and if it can be recovered.\n\n## Data Sources for Recovery\n\n1. **Git history of .beads/issues.jsonl** - May contain older versions with lost changes\n2. **Events table** - Has full audit trail with old_value/new_value\n3. **Commit messages** - May reference work that was done but later overwritten\n\n## Investigation Steps\n\n1. Check events table for patterns:\n - Look for auto-import events that changed status from closed → open\n - Look for manual updates followed by auto-import reverts\n - Identify issues with suspicious event sequences\n\n2. Git archaeology:\n - Check git log for .beads/issues.jsonl\n - Look for commits where status=closed was changed to status=open\n - Compare git history vs current database state\n\n3. Document lost work:\n - Create list of issues that were closed but reverted to open\n - Identify any critical work that was marked done but lost\n - Check if any issue references are now broken\n\n## Recovery Actions\n\nIf significant data loss is found:\n- Create script to reconstruct lost state from events table\n- Manually review and restore critical closed issues\n- Document lessons learned for preventing future data loss\n\n## Example Query\n\n```sql\n-- Find issues where status was manually changed then reverted by auto-import\nSELECT \n issue_id,\n GROUP_CONCAT(event_type || ':' || actor, ' -\u003e ') as event_chain\nFROM events \nWHERE issue_id IN (\n SELECT DISTINCT issue_id \n FROM events \n WHERE actor = 'auto-import' AND event_type = 'status_changed'\n)\nGROUP BY issue_id\nHAVING event_chain LIKE '%stevey%auto-import%';\n```","status":"open","priority":1,"issue_type":"task","created_at":"2025-10-15T02:10:40.724826-07:00","updated_at":"2025-10-15T02:10:40.724826-07:00","dependencies":[{"issue_id":"bd-229","depends_on_id":"bd-228","type":"discovered-from","created_at":"2025-10-15T02:10:40.740673-07:00","created_by":"stevey"}]}
{"id":"bd-23","title":"Optimize export dependency queries (N+1 problem)","description":"Export triggers separate GetDependencyRecords() per issue. For large DBs (1000+ issues), this is N+1 queries. Add GetAllDependencyRecords() to fetch all dependencies in one query. Location: cmd/bd/export.go:52-59, import.go:138-142","status":"closed","priority":3,"issue_type":"task","created_at":"2025-10-14T14:43:06.910124-07:00","updated_at":"2025-10-15T01:58:30.041954-07:00"}
{"id":"bd-24","title":"Support ID space partitioning for parallel worker agents","description":"Enable external orchestrators (like AI worker swarms) to control issue ID assignment. Add --id flag to 'bd create' for explicit ID specification. Optionally support 'bd config set next_id N' to set the starting point for auto-increment. Storage layer already supports pre-assigned IDs (sqlite.go:52-71), just need CLI wiring. This keeps beads simple while letting orchestrators implement their own ID partitioning strategies to minimize merge conflicts. Complementary to bd-9's collision resolution.","status":"closed","priority":1,"issue_type":"feature","created_at":"2025-10-14T14:43:06.910467-07:00","updated_at":"2025-10-15T01:58:30.042402-07:00"}
{"id":"bd-25","title":"Add transaction support to storage layer for atomic multi-operation workflows","description":"Currently each storage method (CreateIssue, UpdateIssue, etc.) starts its own transaction. This makes it impossible to perform atomic multi-step operations like collision resolution. Add support for passing *sql.Tx through the storage interface, or create transaction-aware versions of methods. This would make remapCollisions and other batch operations truly atomic.","status":"closed","priority":4,"issue_type":"feature","created_at":"2025-10-14T14:43:06.910892-07:00","updated_at":"2025-10-15T01:58:30.043082-07:00"}