Commit Graph

14 Commits

Author SHA1 Message Date
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
c9a2e7a8b2 Fix bd-v0y: Remove mtime fast-path in hasJSONLChanged
Git doesn't preserve mtime on checkout, causing false negatives where
hasJSONLChanged() incorrectly returns false after git pull updates JSONL.
This caused bd sync to overwrite pulled JSONL instead of importing it,
resurrecting deleted issues.

Solution: Always compute content hash for comparison (Option 1).
Performance impact is minimal (~10-50ms for sync operations).

Changes:
- cmd/bd/integrity.go: Remove mtime fast-path, always compute hash
- cmd/bd/sync.go: Remove mtime storage after import
- cmd/bd/import.go: Remove mtime storage after import
- cmd/bd/daemon_sync.go: Remove mtime storage and update comments
- cmd/bd/daemon_sync_test.go: Remove mtime assertions from tests

All tests pass. Existing test 'mtime changed but content same - git
operation scenario' verifies the fix works correctly.

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

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-23 22:53:54 -08:00
Steve Yegge
2e4171a5f8 Properly enforce ZFC: always import JSONL before export in sync
- Removed arbitrary 50% divergence check
- bd sync now unconditionally imports JSONL first (source of truth)
- Simpler, more correct: JSONL -> DB -> JSONL flow
2025-11-23 22:38:13 -08:00
Steve Yegge
1ba068fabd Fix bd-l0r: Enforce ZFC (JSONL First Consistency) - refuse export when DB >> JSONL 2025-11-23 22:34:19 -08:00
Steve Yegge
e816e91ecb Complete bd-ar2 P2 tasks: metadata, resurrection, and testing improvements
This commit addresses the remaining P2 tasks from bd-ar2 code review follow-up:

## Completed Tasks

### bd-ar2.4: Improve parent chain resurrection
- Modified `tryResurrectParentWithConn()` to recursively resurrect ancestor chain
- When resurrecting bd-root.1.2, now also resurrects bd-root.1 if missing
- Handles deeply nested hierarchies where intermediate parents are deleted
- All resurrection tests pass including new edge cases

### bd-ar2.5: Add error handling guidance
- Documented metadata update failure strategy in `updateExportMetadata()`
- Explained trade-off: warnings vs errors (safe, prevents data loss)
- Added user-facing message: "Next export may require running 'bd import' first"
- Clarifies that worst case is requiring import before next export

### bd-ar2.6: Document transaction boundaries
- Added comprehensive documentation for atomicity trade-offs
- Explained crash scenarios and recovery (bd import)
- Documented decision to defer defensive checks (Option 4) until needed
- No code changes - current approach is acceptable for now

### bd-ar2.12: Add metadata key validation
- Added keySuffix validation in `updateExportMetadata()` and `hasJSONLChanged()`
- Prevents ':' separator in keySuffix to avoid malformed metadata keys
- Documented metadata key format in function comments
- Single-repo: "last_import_hash", multi-repo: "last_import_hash:<repo_key>"

### bd-ar2.7: Add edge case tests for GetNextChildID resurrection
- TestGetNextChildID_ResurrectParent_NotInJSONL: parent not in history
- TestGetNextChildID_ResurrectParent_NoJSONL: missing JSONL file
- TestGetNextChildID_ResurrectParent_MalformedJSONL: invalid JSON lines
- TestGetNextChildID_ResurrectParentChain: deeply nested missing parents
- All tests pass, resurrection is robust against edge cases

## Files Changed
- cmd/bd/daemon_sync.go: Metadata validation, error handling docs
- cmd/bd/integrity.go: Added strings import, keySuffix validation
- internal/storage/sqlite/hash_ids.go: Improved resurrection comments
- internal/storage/sqlite/resurrection.go: Recursive ancestor resurrection
- internal/storage/sqlite/child_id_test.go: Added 4 new edge case tests

## Testing
All export, sync, metadata, and resurrection tests pass.
Edge cases properly handled: missing JSONL, malformed JSON, deep nesting.

## Remaining Tasks
- bd-ar2.8 (P3): Additional export metadata edge case tests (deferred)

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

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-21 11:40:37 -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
167ab6788b Fix #328: Remove duplicate computeJSONLHash declaration
The function was declared twice:
- Line 80: New version (simpler, no error wrapping)
- Line 390: Old version (with error wrapping)

This caused compilation failure in CI. Removed the old declaration at line 390.

Also fixed integrity_content_test.go to pass context.Context to sqlite.New()
as required by the updated API.

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

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-20 22:06:38 -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
d0e70473cc Fix bd-lm2q: Use content-based comparison to prevent false-positive timestamp skew
This change fixes the issue where 'bd sync' would fail with a false-positive
"JSONL is newer than database" error after the daemon auto-exports.

Root Cause:
- Daemon exports local changes to JSONL, updating its timestamp
- bd sync sees JSONL.mtime > DB.mtime and incorrectly assumes external changes
- This blocks export even though content is identical

Solution:
- Modified isJSONLNewer() to use SHA256 content hash comparison
- Only triggers auto-import when JSONL is newer AND content differs
- Prevents false positives from daemon auto-export timestamp updates
- Maintains conservative fallback if hashes can't be computed

Changes:
- Added computeJSONLHash() and computeDBHash() helper functions
- Created isJSONLNewerWithStore() to support testing with explicit store
- Added comprehensive tests for content-based comparison logic
- All existing tests pass, including export_mtime tests

Fixes: bd-lm2q
2025-11-20 21:23:26 -05:00
Steve Yegge
a1e507520c Fix bd-ca0b: bd sync now auto-resolves conflicts instead of failing
Previously, bd sync would fail with "Pre-export validation failed:
refusing to export: JSONL is newer than database" when JSONL was
modified (e.g., after git pull).

Now bd sync intelligently handles this by:
- Detecting when JSONL is newer than the database
- Automatically importing before exporting
- Continuing with the normal sync flow

This makes the workflow much smoother - users can just run 'bd sync'
and it figures out what needs to be done.

Changes:
- Added isJSONLNewer() helper to check file timestamps
- Modified sync command to auto-import when JSONL is newer
- Extracted timestamp check logic for reusability

Resolves: bd-ca0b
🤖 Generated with Claude Code

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-20 20:33:52 -05:00
Steve Yegge
5438485fb8 Fix bd-9bsx: Add sync validation to prevent infinite dirty loop
- Added dbNeedsExport() to check if DB and JSONL are in sync
- Only re-export after import if DB has changes that differ from JSONL
- Prevents unconditional re-export that caused infinite dirty state
- Added comprehensive tests for sync validation

Fixes recurring dirty state after merge conflicts that plagued users for weeks.

Amp-Thread-ID: https://ampcode.com/threads/T-f4f8c8c6-07bc-4334-9109-4626b4fd7a24
Co-authored-by: Amp <amp@ampcode.com>
2025-11-05 18:10:06 -08: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
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