CRITICAL BUG: The previous fix had a race condition where the
importInProgress flag could be released twice, allowing two goroutines
to think they both hold the lock.
Bug scenario:
1. Goroutine A: acquires lock (CAS true)
2. Goroutine A: manually releases at line 208 for git dirty skip
3. Goroutine B: CAS succeeds, acquires lock
4. Goroutine A: defer runs, releases flag AGAIN (clears B lock)
5. Goroutine C: CAS succeeds - now TWO goroutines have lock
Root cause: Using both manual Store(false) AND defer Store(false)
created a window where the flag could be cleared twice.
Fix: Use a shouldDeferRelease flag to disable the deferred release
when we manually release early. This ensures exactly one release
per acquisition.
Testing: All auto-import tests still passing
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
CRITICAL FIX: The daemon could enter a corrupt state when auto-import
was triggered while uncommitted changes existed in .beads/ files. This
caused mysterious EOF errors requiring daemon restarts.
Root Causes Fixed:
1. No git dirty check before auto-import
2. Missing timeout protection (could hang indefinitely)
3. Race condition in importInProgress flag management
4. Improper git status parsing (false positives)
5. Context leak in export goroutine (accessing closed DB)
6. Data loss in triggerExport (missing deps/labels/comments)
Changes:
- Add hasUncommittedBeadsFiles() to check git status before import
- Properly parses git porcelain format ("XY filename")
- Ignores untracked files, only checks tracked .jsonl changes
- 5-second timeout on git command to prevent hangs
- Add 30-second timeout to import operations
- Prevents daemon from hanging on stuck imports
- Returns clear error message on timeout
- Fix race condition in importInProgress flag
- Explicitly release before early returns
- Prevents other requests seeing incorrect "import in progress"
- Fix context leak in onChanged export goroutine
- Check daemon shutdown state before export
- Suppress expected "database closed" errors during shutdown
- Pass storage/path as parameters to avoid closure issues
- Fix data loss in triggerExport()
- Populate dependencies, labels, comments (mirrors handleExport)
- Use atomic file write (temp + rename)
- Sort issues for consistent output
Impact: Prevents daemon corruption in collaborative workflows where
users frequently pull updates while having local uncommitted changes.
Testing: All auto-import tests passing
- TestDaemonAutoImportAfterGitPull ✓
- TestDaemonAutoImportDataCorruption ✓
- internal/autoimport tests ✓
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Hash-based IDs make collision resolution unnecessary. The flag was
already non-functional (handleCollisions returns error on collision
regardless of flag value).
Removed:
- --resolve-collisions flag from bd import
- ResolveCollisions field from ImportOptions and importer.Options
- All references in daemon, auto-import, and tests
- Updated error messages to reflect hash IDs don't collide
All import tests pass.
Amp-Thread-ID: https://ampcode.com/threads/T-47dfa0cc-bb71-4467-ac86-f0966a7c5d58
Co-authored-by: Amp <amp@ampcode.com>
- Replaced all getStorageForRequest(req) calls with direct s.storage access
- Updated 5 handler files: server_issues_epics.go (~8 calls), server_labels_deps_comments.go (~4 calls), server_compact.go (~2 calls), server_export_import_auto.go (~2 calls), server_routing_validation_diagnostics.go (~1 call)
- Only remaining references are in server_cache_storage.go (to be deleted in bd-33) and server_eviction_test.go (to be deleted in bd-34)
- Part of bd-29 epic to remove daemon storage cache
Amp-Thread-ID: https://ampcode.com/threads/T-239a5531-68a5-4c98-b85d-0e3512b2553c
Co-authored-by: Amp <amp@ampcode.com>
