Commit Graph

13 Commits

Author SHA1 Message Date
Joshua Shanks
cf4f11cff7 Fix error handling consistency in auto-import and fallback paths (#47)
* Fix error handling consistency in auto-import and fallback paths

- Add error checking/warnings for auto-import CRUD operations (UpdateIssue, CreateIssue, AddDependency)
- Add error checking/warnings for auto-import label operations (AddLabel, RemoveLabel)
- Add warning when import hash storage fails (prevents unnecessary re-imports)
- Add proper error handling for UserHomeDir with fallback to current directory

These changes make auto-import error handling consistent with manual operations
and prevent silent failures that could confuse users or cause data inconsistencies.

* Remove invalid version property from golangci-lint config

* Fix linter errors: errcheck, unused, goconst, and misspell

- Fix unchecked error returns in ROLLBACK statements
- Fix unchecked type assertion for status field
- Extract LIMIT SQL constant to reduce duplication
- Fix spelling: cancelled -> canceled
- Remove unused ensureCounterInitialized function
- Remove unused parameter in parallel test goroutine
2025-10-15 23:34:33 -07:00
Steve Yegge
5ab7ff0f84 Add comprehensive unit tests for CreateIssues (bd-241)
- Added 12 table-driven test cases covering all validation scenarios
- Added 2 rollback tests (validation error + DB conflict)
- Added nil item detection to prevent panic
- Fixed nil pointer panic in CreateIssues
- Tests verify ID uniqueness, timestamps, closed_at invariant
- All tests pass

Amp-Thread-ID: https://ampcode.com/threads/T-61c584cd-d873-4a1a-bfa6-d739b630b3e5
Co-authored-by: Amp <amp@ampcode.com>
2025-10-15 19:33:53 -07:00
Steve Yegge
d2b50e6cdc Add closed_at timestamp tracking to issues
- Add closed_at field to Issue type with JSON marshaling
- Implement closed_at timestamp in SQLite storage layer
- Update import/export to handle closed_at field
- Add comprehensive tests for closed_at functionality
- Maintain backward compatibility with existing databases

Amp-Thread-ID: https://ampcode.com/threads/T-f3a7799b-f91e-4432-a690-aae0aed364b3
Co-authored-by: Amp <amp@ampcode.com>
2025-10-15 14:52:29 -07:00
Steve Yegge
587e1d9d0f test: Add comprehensive test coverage for storage layer
Added test files for core SQLite storage functionality:
- beads_test.go: Database path detection tests
- dirty_test.go: Dirty tracking for auto-flush
- events_test.go: Event logging tests
- labels_test.go: Label management tests
- sqlite_test.go: Added metadata tests (SetMetadata, GetMetadata)

Merged with upstream TestParallelIssueCreation (bd-89 regression test).

All tests passing. Ready to push.

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

Co-Authored-By: Claude <noreply@anthropic.com>
2025-10-15 01:30:28 -07:00
Steve Yegge
42d2f71925 fix: Fix race condition in parallel issue creation (bd-89)
Use IMMEDIATE transactions with dedicated connections to fix race condition
where multiple processes creating issues concurrently caused "UNIQUE constraint
failed: issues.id" errors.

Key changes:
- Use BEGIN IMMEDIATE to acquire RESERVED lock early
- Use dedicated connection (sql.Conn) for transaction to ensure all operations
  happen on same connection
- Increase busy_timeout from 10s to 30s for better parallel write handling
- Use context.Background() for ROLLBACK to ensure cleanup even if ctx cancelled

Added regression test TestParallelIssueCreation that creates 20 issues in
parallel and verifies no ID collisions occur.

Fixes #6

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

Co-Authored-By: Claude <noreply@anthropic.com>
2025-10-15 01:26:27 -07:00
Joshua Shanks
3f42cab0d1 fix: Handle NULL values in GetStatistics for empty databases (#37)
When GetStatistics is called on an empty database, SQL SUM() returns NULL
which causes a scan error when converting to int. Wrap SUM expressions with
COALESCE to return 0 instead of NULL.

Add TestGetStatistics to verify both empty and populated database cases.

Fixes issue where `bd stats` and MCP stats tool crash on fresh databases.

Signed-off-by: Joshua Shanks <jjshanks@gmail.com>
2025-10-15 00:30:36 -07:00
Steve Yegge
b74f57c087 Remove concurrency torture tests and document limitation
- Removed TestConcurrentIDGeneration and TestMultiProcessIDGeneration
- These stress tests (100+ simultaneous operations) fail with pure Go SQLite
- Added documentation in DESIGN.md about the concurrency limitation
- Added troubleshooting note in README.md
- All other tests pass; normal usage unaffected
- Pure Go driver benefits (no CGO, cross-compilation) outweigh limitation
2025-10-14 11:21:25 -07:00
guillaume
2550e7fb6a feat: Remove CGO dependency by migrating to pure Go SQLite driver
Migrates from github.com/mattn/go-sqlite3 (requires CGO) to modernc.org/sqlite (pure Go).

Benefits:
- Cross-compilation without C toolchain
- Faster builds (no CGO overhead)
- Static binary distribution
- Deployment in CGO-restricted environments

Changes:
- Updated go.mod to use modernc.org/sqlite v1.38.2
- Changed driver name from sqlite3 to sqlite in all sql.Open() calls
- Updated documentation (DESIGN.md, EXTENDING.md, examples)
- Removed concurrency torture tests that exposed pure Go driver limitations
- Documented known limitation under extreme parallel load (100+ ops)

All real-world tests pass. Normal usage with WAL mode unaffected.

Co-authored-by: yome <yome@users.noreply.github.com>
2025-10-14 11:20:27 -07:00
Steve Yegge
5c2cff4837 fix: Post-PR #8 critical improvements (bd-64, bd-65, bd-66, bd-67)
This commit addresses all critical follow-up issues identified in the
code review of PR #8 (atomic counter implementation).

## bd-64: Fix SyncAllCounters performance bottleneck (P0)
- Replace SyncAllCounters() on every CreateIssue with lazy initialization
- Add ensureCounterInitialized() that only scans prefix-specific issues on first use
- Performance improvement: O(n) full table scan → O(1) for subsequent creates
- Add comprehensive tests in lazy_init_test.go

## bd-65: Add migration for issue_counters table (P1)
- Add migrateIssueCountersTable() similar to migrateDirtyIssuesTable()
- Checks if table is empty and syncs from existing issues on first open
- Handles both fresh databases and migrations from old databases
- Add comprehensive tests in migration_test.go (3 scenarios)

## bd-66: Make import counter sync failure fatal (P1)
- Change SyncAllCounters() failure from warning to fatal error in import
- Prevents ID collisions when counter sync fails
- Data integrity > convenience

## bd-67: Update test comments (P2)
- Update TestMultiProcessIDGeneration comments to reflect fix is in place
- Change "With the bug, we expect errors" → "After the fix, all should succeed"

All tests pass. Atomic counter implementation is now production-ready.

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

Co-Authored-By: Claude <noreply@anthropic.com>
2025-10-14 01:57:43 -07:00
v4rgas
838d884988 fix: sync counters on every CreateIssue to prevent race conditions
Move counter sync from import to CreateIssue to handle parallel issue creation.
This ensures the counter is always up-to-date before generating new IDs,
preventing collisions when multiple processes create issues concurrently.

Remove unused SyncCounterForPrefix method and its test.
2025-10-14 01:19:11 -07:00
v4rgas
73f5acadfa fix: sync ID counters after import to prevent collisions
When importing issues with explicit high IDs (e.g., bd-100), the
issue_counters table wasn't being updated. This caused the next
auto-generated issue to collide with existing IDs (bd-4 instead of bd-101).

Changes:
- Add SyncAllCounters() to scan all issues and update counters atomically
- Add SyncCounterForPrefix() for granular counter synchronization
- Call SyncAllCounters() in import command after creating issues
- Add comprehensive tests for counter sync functionality
- Update TestImportCounterSyncAfterHighID to verify fix

The fix uses a single efficient SQL query to prevent ID collisions
with subsequently auto-generated issues.
2025-10-14 01:19:03 -07:00
v4rgas
d85ff17f40 test: add failing test for multi-process ID generation race
Add TestMultiProcessIDGeneration to reproduce the bug where multiple
bd create processes fail with UNIQUE constraint errors when run
simultaneously. Each goroutine opens a separate database connection
to simulate independent processes.

Test currently fails with 17/20 processes getting UNIQUE constraint
errors, confirming the race condition in the in-memory ID counter.
2025-10-14 01:17:02 -07:00
Steve Yegge
15afb5ad17 Implement JSONL export/import and shift to text-first architecture
This is a fundamental architectural shift from binary SQLite to JSONL as
the source of truth for git workflows.

## New Features

- `bd export --format=jsonl` - Export issues to JSON Lines format
- `bd import` - Import issues from JSONL (create new, update existing)
- `--skip-existing` flag for import to only create new issues

## Architecture Change

**Before:** Binary SQLite database committed to git
**After:** JSONL text files as source of truth, SQLite as ephemeral cache

Benefits:
- Git-friendly text format with clean diffs
- AI-resolvable merge conflicts (append-only is 95% conflict-free)
- Human-readable issue tracking in git
- No binary merge conflicts

## Documentation

- Updated README with JSONL-first workflow and git hooks
- Added TEXT_FORMATS.md analyzing JSONL vs CSV vs binary
- Updated GIT_WORKFLOW.md with historical context
- .gitignore now excludes *.db, includes .beads/*.jsonl

## Implementation Details

- Export sorts issues by ID for consistent diffs
- Import handles both creates and updates atomically
- Proper handling of pointer fields (EstimatedMinutes)
- All tests passing

## Breaking Changes

- Database files (*.db) should now be gitignored
- Use export/import workflow for git collaboration
- Git hooks recommended for automation

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

Co-Authored-By: Claude <noreply@anthropic.com>
2025-10-12 01:17:50 -07:00