Commit Graph

10 Commits

Author SHA1 Message Date
Steve Yegge
9f76cfda01 refactor: remove all deletions.jsonl code (bd-fom)
Complete removal of the legacy deletions.jsonl manifest system.
Tombstones are now the sole deletion mechanism.

Removed:
- internal/deletions/ - entire package
- cmd/bd/deleted.go - deleted command
- cmd/bd/doctor/fix/deletions.go - HydrateDeletionsManifest
- Tests for all removed functionality

Cleaned:
- cmd/bd/sync.go - removed sanitize, auto-compact
- cmd/bd/delete.go - removed dual-writes
- cmd/bd/doctor.go - removed checkDeletionsManifest
- internal/importer/importer.go - removed deletions checks
- internal/syncbranch/worktree.go - removed deletions merge
- cmd/bd/integrity.go - updated validation (warn-only on decrease)

Files removed: 12
Lines removed: ~7500

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

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2025-12-16 14:20:49 -08:00
Steve Yegge
f59f8c20d0 refactor: rename last_import_hash to jsonl_content_hash (bd-39o)
The metadata key 'last_import_hash' was misleading because it's updated on
both import AND export. Renamed to 'jsonl_content_hash' which more accurately
describes its purpose - tracking the content hash of the JSONL file.

Added migration support: read operations try new key first, then fall back
to old key for backwards compatibility with existing databases.

Files modified:
- cmd/bd/integrity.go: Update key name with migration support
- cmd/bd/import.go: Update key name
- cmd/bd/sync.go: Update key name
- cmd/bd/autoflush.go: Update key name with migration support
- cmd/bd/daemon_sync.go: Update key name
- cmd/bd/daemon_event_loop.go: Update key name with migration support
- internal/autoimport/autoimport.go: Update key name with migration support
- Updated all related tests

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

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-28 23:14:12 -08:00
Steve Yegge
95371ea116 Export and display close reasons for issues (beads-410)
- Add CloseReason field to Issue struct in types.go
- Add GetCloseReason() and batch GetCloseReasonsForIssues()
- Update issue loading to populate close reasons
- Update scanIssues() to include close_reason in JSONL export
- Update bd show to display close reason after status

Close reasons now survive sync between repos.

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

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-28 18:39:07 -08:00
Steve Yegge
fe4f851d33 Fix validatePostImport false positive for legitimate deletions (bd-pg1)
The sync validation was incorrectly triggering 'data loss detected' when
issue count decreased after import, even for legitimate deletions recorded
in deletions.jsonl.

Changes:
- Modified validatePostImport to accept jsonlPath and check deletions manifest
- When issue count decreases, check if decrease is within recorded deletions
- Updated all call sites in sync.go and daemon_sync.go
- Added comprehensive tests for deletion-aware validation

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

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-28 17:43:25 -08:00
Steve Yegge
351d2239d6 test: Refactor integrity_test.go to use shared DB pattern (bd-1rh P2)
CHANGES:
- Reduced from 15 DB setups to 4 shared DBs
- TestValidatePreExportSuite: 1 shared DB, 5 subtests
- TestValidatePostImport: No DB needed, kept as-is
- TestCountDBIssuesSuite: 1 shared DB, 1 subtest
- TestHasJSONLChangedSuite: 1 shared DB, 7 subtests (with unique keySuffixes)
- TestComputeJSONLHash: No DB needed, kept as-is
- TestCheckOrphanedDepsSuite: 1 shared DB, 2 subtests

PERFORMANCE:
- Before: 0.455s avg (15 DB setups)
- After: 0.373s avg (4 DB setups)
- Speedup: 1.22x (18% faster)

KEY LEARNINGS:
- Used unique keySuffix values for hasJSONLChanged subtests to avoid metadata pollution
- Metadata keys like last_import_hash are shared across subtests unless keySuffix is used
- TestValidatePostImport and TestComputeJSONLHash do not need DB at all

PATTERN:
Following create_test.go and dep_test.go pattern with shared DB setup

Part of Phase 2 test suite optimization (bd-1rh follow-up)

🤖 Generated with Claude Code

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-21 16:11:56 -05:00
Steve Yegge
cd8cb8b86a Fix bd-ar2 code review issues: metadata tracking and multi-repo support
This commit addresses critical code review findings from bd-dvd and bd-ymj fixes:

## Completed Tasks

### bd-ar2.1: Extract duplicated metadata update code
- Created `updateExportMetadata()` helper function
- Eliminated 22-line duplication between createExportFunc and createSyncFunc
- Single source of truth for metadata updates

### bd-ar2.2: Add multi-repo support to export metadata updates
- Added per-repo metadata key tracking with keySuffix parameter
- Both export and sync functions now update metadata for all repos

### bd-ar2.3: Fix tests to use actual daemon functions
- TestExportUpdatesMetadata now calls updateExportMetadata() directly
- Added TestUpdateExportMetadataMultiRepo() for multi-repo testing
- Fixed export_mtime_test.go tests to call updateExportMetadata()

### bd-ar2.9: Fix variable shadowing in GetNextChildID
- Changed `err` to `resurrectErr` to avoid shadowing
- Improves code clarity and passes linter checks

### bd-ar2.10: Fix hasJSONLChanged to support per-repo keys
- Updated hasJSONLChanged() to accept keySuffix parameter
- Reads metadata with correct per-repo keys
- All callers updated (validatePreExport, daemon import, sync command)

### bd-ar2.11: Use stable repo identifiers instead of paths
- Added getRepoKeyForPath() helper function
- Uses stable identifiers like ".", "../frontend" instead of absolute paths
- Metadata keys now portable across machines and clones
- Prevents orphaned metadata when repos are moved

## Files Changed
- cmd/bd/daemon_sync.go: Helper functions, metadata updates
- cmd/bd/integrity.go: hasJSONLChanged() with keySuffix support
- cmd/bd/sync.go: Updated to use getRepoKeyForPath()
- cmd/bd/*_test.go: Tests updated for new signatures
- internal/storage/sqlite/hash_ids.go: Fixed variable shadowing

## Testing
All export, sync, and integrity tests pass.

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

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-21 11:28:25 -05:00
Steve Yegge
0020eb490c Fix bd-khnb: Prevent auto-import from resurrecting deleted issues
Replace mtime-based staleness detection with content-based (SHA256 hash) to prevent
git operations from resurrecting deleted issues.

**Problem:**
Auto-import used file modification time to detect if JSONL was "newer" than database.
Git operations (checkout, merge, pull) restore old files with recent mtimes, causing
auto-import to load stale data over current database state, resurrecting deleted issues.

**Solution:**
- Added computeJSONLHash() to compute SHA256 of JSONL content
- Added hasJSONLChanged() with two-tier check:
  1. Fast-path: Check mtime first (99% of checks are instant)
  2. Slow-path: Compute hash only if mtime changed (catches git operations)
- Store metadata: last_import_hash, last_import_mtime, last_import_time
- Updated auto-import in daemon_sync.go to use content-based check
- Updated validatePreExport to use content-based check (bd-xwo)
- Graceful degradation: metadata failures are non-fatal warnings

**Changes:**
- cmd/bd/integrity.go: Add computeJSONLHash(), hasJSONLChanged()
- cmd/bd/integrity_test.go: Add comprehensive tests for new functions
- cmd/bd/import.go: Update metadata after import
- cmd/bd/sync.go: Use hasJSONLChanged() instead of isJSONLNewer()
- cmd/bd/daemon_sync.go: Use hasJSONLChanged() in auto-import

**Testing:**
- Unit tests pass (TestHasJSONLChanged with 7 scenarios)
- Integration test passes (test_bd_khnb_fix.sh)
- Verified git resurrection scenario prevented

Fixes: bd-khnb
Related: bd-3bg, bd-xwo, bd-39o, bd-56p, bd-m8t, bd-rfj, bd-t5o

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

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-20 22:00:20 -05:00
Steve Yegge
7421f525fb Fix daemon race condition: prevent stale exports
- Add JSONL timestamp check in validatePreExport
- Refuse export if JSONL is newer than database
- Force daemon to import before exporting when JSONL updated
- Add test case for JSONL-newer-than-DB scenario
- Fixes bd-89e2
2025-11-01 22:01:41 -07:00
Steve Yegge
1e2e066dc4 Fix remaining test database initialization errors (bd-207)
Fixed 38 tests failing with 'database not initialized: issue_prefix config is missing' by replacing manual sqlite.New() calls with test helper functions.

Modified files:
- dep_test.go (4 tests)
- merge_test.go (4 tests)
- export_import_test.go (4 instances)
- import_collision_test.go (10 instances)
- import_bug_test.go (1 instance)
- import_collision_regression_test.go (2 instances)
- import_idempotent_test.go (2 instances)
- init_test.go (4 instances)
- integrity_test.go (3 tests)
- main_test.go (multiple tests)

All database initialization errors are now resolved.
Remaining test failures (2) are unrelated to database initialization.

Amp-Thread-ID: https://ampcode.com/threads/T-a6b09458-b899-49eb-9a62-346fa67f62c7
Co-authored-by: Amp <amp@ampcode.com>
2025-10-27 20:00:49 -07:00
Steve Yegge
6271b521b4 bd-162: Add database integrity checks with oracle review fixes
- Added validatePreExport to prevent data loss
- Added checkDuplicateIDs to detect corruption
- Added checkOrphanedDeps to find orphaned dependencies (both sides)
- Added validatePostImport to ensure imports don't lose data
- CRITICAL FIX: Removed post-pull export that clobbered fresh JSONL
- Conservative checks when JSONL is unreadable
- Efficient COUNT(*) SQL path instead of loading all issues
- Comprehensive test coverage including edge cases
2025-10-26 20:17:48 -07:00