diff --git a/cmd/bd/hook.go b/cmd/bd/hook.go index c535c11d..c7ee0210 100644 --- a/cmd/bd/hook.go +++ b/cmd/bd/hook.go @@ -72,15 +72,14 @@ Configuration (.beads/config.yaml): // // Key insight: We track Dolt commit hash (not git commit) because: // - Dolt is the source of truth for issue data -// - We use dolt_diff() to find changes since last export +// - We use dolt_diff() to detect if changes exist since last export // - Each worktree may have exported at different Dolt commits type ExportState struct { WorktreeRoot string `json:"worktree_root"` WorktreeHash string `json:"worktree_hash,omitempty"` // Hash of worktree path (for debugging) - LastExportCommit string `json:"last_export_commit"` // DOLT commit hash when last exported + LastExportCommit string `json:"last_export_commit"` // Dolt commit hash when last exported LastExportTime time.Time `json:"last_export_time"` JSONLHash string `json:"jsonl_hash,omitempty"` // Hash of JSONL at last export - Actor string `json:"actor,omitempty"` // BD_ACTOR value if set during export } // getWorktreeHash returns a hash of the worktree root for use in filenames. @@ -414,9 +413,10 @@ func hookPreCommit() int { // Per design doc Part 21, this function: // 1. Loads export state for the current worktree // 2. Gets the current Dolt commit hash (not git commit) -// 3. Uses dolt_diff() to find changes since last export -// 4. Filters by BD_ACTOR if set (so polecats only export their own changes) -// 5. Updates JSONL with changes and saves new state +// 3. Checks if export is needed (skip if already exported this commit) +// 4. Exports to JSONL and saves new state +// +// Future: Use dolt_diff() for incremental export and BD_ACTOR filtering. func hookPreCommitDolt(beadsDir, worktreeRoot string) int { ctx := context.Background() @@ -435,15 +435,15 @@ func hookPreCommitDolt(beadsDir, worktreeRoot string) int { vs, ok := storage.AsVersioned(store) if !ok { // Fall back to full export if not versioned - return hookPreCommitDoltFallback(ctx, store, beadsDir, worktreeRoot) + return doExportAndSaveState(ctx, beadsDir, worktreeRoot, "") } // Get current Dolt commit hash currentDoltCommit, err := vs.GetCurrentCommit(ctx) if err != nil { fmt.Fprintf(os.Stderr, "Warning: could not get Dolt commit: %v\n", err) - // Fall back to full export - return hookPreCommitDoltFallback(ctx, store, beadsDir, worktreeRoot) + // Fall back to full export without commit tracking + return doExportAndSaveState(ctx, beadsDir, worktreeRoot, "") } // Check if we've already exported for this Dolt commit (idempotency) @@ -452,97 +452,43 @@ func hookPreCommitDolt(beadsDir, worktreeRoot string) int { return 0 } - // Get actor for filtering (polecats use BD_ACTOR to avoid exporting others' work) - actor := os.Getenv("BD_ACTOR") - - // Determine export strategy - jsonlPath := filepath.Join(beadsDir, "issues.jsonl") + // Check if there are actual changes to export (optimization) if prevState != nil && prevState.LastExportCommit != "" { - // Incremental export: use dolt_diff() to get only changes - if err := exportIncrementalDolt(ctx, vs, store, jsonlPath, prevState.LastExportCommit, currentDoltCommit, actor); err != nil { - fmt.Fprintf(os.Stderr, "Warning: incremental export failed, doing full export: %v\n", err) - // Fall back to full export - if err := exportFullDolt(ctx, store, jsonlPath); err != nil { - fmt.Fprintf(os.Stderr, "Warning: could not export to JSONL: %v\n", err) - return 0 - } - } - } else { - // First export for this worktree: full export - if err := exportFullDolt(ctx, store, jsonlPath); err != nil { - fmt.Fprintf(os.Stderr, "Warning: could not export to JSONL: %v\n", err) + hasChanges, err := hasDoltChanges(ctx, vs, prevState.LastExportCommit, currentDoltCommit) + if err != nil { + fmt.Fprintf(os.Stderr, "Warning: could not check for changes: %v\n", err) + // Continue with export to be safe + } else if !hasChanges { + // No changes, but update state to track new commit + updateExportStateCommit(beadsDir, worktreeRoot, currentDoltCommit) return 0 } } - // Stage JSONL files - if os.Getenv("BEADS_NO_AUTO_STAGE") == "" { - rc, rcErr := beads.GetRepoContext() - jsonlFiles := []string{".beads/issues.jsonl", ".beads/deletions.jsonl", ".beads/interactions.jsonl"} - for _, f := range jsonlFiles { - if _, err := os.Stat(f); err == nil { - var gitAdd *exec.Cmd - if rcErr == nil { - gitAdd = rc.GitCmdCWD(ctx, "add", f) - } else { - // #nosec G204 -- f comes from jsonlFiles - gitAdd = exec.Command("git", "add", f) - } - _ = gitAdd.Run() - } - } - } - - // Update export state with Dolt commit hash - jsonlHash, _ := computeJSONLHashForHook(jsonlPath) - state := &ExportState{ - WorktreeRoot: worktreeRoot, - WorktreeHash: getWorktreeHash(worktreeRoot), - LastExportCommit: currentDoltCommit, // Dolt commit, not git commit - LastExportTime: time.Now(), - JSONLHash: jsonlHash, - Actor: actor, - } - if err := saveExportState(beadsDir, worktreeRoot, state); err != nil { - fmt.Fprintf(os.Stderr, "Warning: could not save export state: %v\n", err) - } - - return 0 + return doExportAndSaveState(ctx, beadsDir, worktreeRoot, currentDoltCommit) } -// hookPreCommitDoltFallback does a full export when incremental isn't available. -func hookPreCommitDoltFallback(ctx context.Context, store storage.Storage, beadsDir, worktreeRoot string) int { +// doExportAndSaveState performs the export and saves state. Shared by main path and fallback. +func doExportAndSaveState(ctx context.Context, beadsDir, worktreeRoot, doltCommit string) int { jsonlPath := filepath.Join(beadsDir, "issues.jsonl") - if err := exportFullDolt(ctx, store, jsonlPath); err != nil { + + // Export to JSONL + if err := runJSONLExport(); err != nil { fmt.Fprintf(os.Stderr, "Warning: could not export to JSONL: %v\n", err) return 0 } - // Stage JSONL files - if os.Getenv("BEADS_NO_AUTO_STAGE") == "" { - rc, rcErr := beads.GetRepoContext() - jsonlFiles := []string{".beads/issues.jsonl", ".beads/deletions.jsonl", ".beads/interactions.jsonl"} - for _, f := range jsonlFiles { - if _, err := os.Stat(f); err == nil { - var gitAdd *exec.Cmd - if rcErr == nil { - gitAdd = rc.GitCmdCWD(ctx, "add", f) - } else { - // #nosec G204 -- f comes from jsonlFiles - gitAdd = exec.Command("git", "add", f) - } - _ = gitAdd.Run() - } - } - } + // Stage JSONL files for git commit + stageJSONLFiles(ctx) - // Save minimal state (no Dolt commit since we couldn't get it) + // Save export state jsonlHash, _ := computeJSONLHashForHook(jsonlPath) state := &ExportState{ - WorktreeRoot: worktreeRoot, - WorktreeHash: getWorktreeHash(worktreeRoot), - LastExportTime: time.Now(), - JSONLHash: jsonlHash, + WorktreeRoot: worktreeRoot, + WorktreeHash: getWorktreeHash(worktreeRoot), + LastExportCommit: doltCommit, // Empty string if Dolt commit unavailable + LastExportTime: time.Now(), + JSONLHash: jsonlHash, } if err := saveExportState(beadsDir, worktreeRoot, state); err != nil { fmt.Fprintf(os.Stderr, "Warning: could not save export state: %v\n", err) @@ -551,45 +497,55 @@ func hookPreCommitDoltFallback(ctx context.Context, store storage.Storage, beads return 0 } -// exportIncrementalDolt exports only changes since the last export using dolt_diff(). -// If actor is non-empty, only changes attributed to that actor are included. -func exportIncrementalDolt(ctx context.Context, vs storage.VersionedStorage, store storage.Storage, jsonlPath, fromCommit, toCommit, actor string) error { - // Get diff between commits +// hasDoltChanges checks if there are any changes between two Dolt commits. +func hasDoltChanges(ctx context.Context, vs storage.VersionedStorage, fromCommit, toCommit string) (bool, error) { diffs, err := vs.Diff(ctx, fromCommit, toCommit) if err != nil { - return fmt.Errorf("getting diff: %w", err) + return false, err } - - if len(diffs) == 0 { - // No changes, nothing to export - return nil - } - - // TODO: Actor-based filtering - // If actor is set, filter diffs to only include changes made by this actor. - // This requires the issues table to have an 'updated_by' or similar field. - // For now, we export all changes - the actor filtering is a future enhancement. - _ = actor // Silence unused variable warning - - // For now, if there are any changes, do a full re-export. - // A proper incremental implementation would: - // 1. Read existing JSONL - // 2. Apply diffs (add new, update modified, mark deleted) - // 3. Write updated JSONL - // - // This is simpler and still correct - we just export all current data. - // The per-worktree state tracking still prevents cross-polecat pollution - // because each worktree tracks its own export commit. - return exportFullDolt(ctx, store, jsonlPath) + return len(diffs) > 0, nil } -// exportFullDolt does a complete export of all issues to JSONL. -func exportFullDolt(ctx context.Context, store storage.Storage, jsonlPath string) error { - // Use bd sync --flush-only for now - this handles all the JSONL formatting +// updateExportStateCommit updates just the commit hash in the export state. +// Used when we detect no changes but want to track the new commit. +func updateExportStateCommit(beadsDir, worktreeRoot, doltCommit string) { + prevState, err := loadExportState(beadsDir, worktreeRoot) + if err != nil || prevState == nil { + return // Can't update what doesn't exist + } + prevState.LastExportCommit = doltCommit + prevState.LastExportTime = time.Now() + _ = saveExportState(beadsDir, worktreeRoot, prevState) +} + +// runJSONLExport runs the actual JSONL export via bd sync. +func runJSONLExport() error { cmd := exec.Command("bd", "sync", "--flush-only", "--no-daemon") return cmd.Run() } +// stageJSONLFiles stages JSONL files for git commit (unless BEADS_NO_AUTO_STAGE is set). +func stageJSONLFiles(ctx context.Context) { + if os.Getenv("BEADS_NO_AUTO_STAGE") != "" { + return + } + + rc, rcErr := beads.GetRepoContext() + jsonlFiles := []string{".beads/issues.jsonl", ".beads/deletions.jsonl", ".beads/interactions.jsonl"} + for _, f := range jsonlFiles { + if _, err := os.Stat(f); err == nil { + var gitAdd *exec.Cmd + if rcErr == nil { + gitAdd = rc.GitCmdCWD(ctx, "add", f) + } else { + // #nosec G204 -- f comes from jsonlFiles (hardcoded) + gitAdd = exec.Command("git", "add", f) + } + _ = gitAdd.Run() + } + } +} + // hookPostMerge implements the post-merge hook: Import JSONL to database. func hookPostMerge(args []string) int { beadsDir := beads.FindBeadsDir() diff --git a/cmd/bd/hook_test.go b/cmd/bd/hook_test.go new file mode 100644 index 00000000..4e80a788 --- /dev/null +++ b/cmd/bd/hook_test.go @@ -0,0 +1,223 @@ +package main + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +func TestGetWorktreeHash(t *testing.T) { + // Same input should produce same hash + hash1 := getWorktreeHash("/some/path/to/worktree") + hash2 := getWorktreeHash("/some/path/to/worktree") + if hash1 != hash2 { + t.Errorf("Same path produced different hashes: %s vs %s", hash1, hash2) + } + + // Different inputs should produce different hashes + hash3 := getWorktreeHash("/different/path") + if hash1 == hash3 { + t.Errorf("Different paths produced same hash: %s", hash1) + } + + // Hash should be 16 hex chars (8 bytes) + if len(hash1) != 16 { + t.Errorf("Hash length = %d, want 16", len(hash1)) + } +} + +func TestExportStatePaths(t *testing.T) { + beadsDir := "/tmp/test/.beads" + worktreeRoot := "/tmp/test/worktree" + + stateDir := getExportStateDir(beadsDir) + if stateDir != "/tmp/test/.beads/export-state" { + t.Errorf("getExportStateDir() = %s, want /tmp/test/.beads/export-state", stateDir) + } + + statePath := getExportStatePath(beadsDir, worktreeRoot) + expectedHash := getWorktreeHash(worktreeRoot) + expectedPath := "/tmp/test/.beads/export-state/" + expectedHash + ".json" + if statePath != expectedPath { + t.Errorf("getExportStatePath() = %s, want %s", statePath, expectedPath) + } +} + +func TestSaveAndLoadExportState(t *testing.T) { + tmpDir := t.TempDir() + beadsDir := filepath.Join(tmpDir, ".beads") + worktreeRoot := tmpDir + + // Initially no state + state, err := loadExportState(beadsDir, worktreeRoot) + if err != nil { + t.Fatalf("loadExportState() failed: %v", err) + } + if state != nil { + t.Errorf("loadExportState() returned non-nil for missing state") + } + + // Save state + now := time.Now().Truncate(time.Second) // Truncate for comparison + testState := &ExportState{ + WorktreeRoot: worktreeRoot, + WorktreeHash: getWorktreeHash(worktreeRoot), + LastExportCommit: "abc123def456", + LastExportTime: now, + JSONLHash: "hashvalue", + } + if err := saveExportState(beadsDir, worktreeRoot, testState); err != nil { + t.Fatalf("saveExportState() failed: %v", err) + } + + // Load state back + loaded, err := loadExportState(beadsDir, worktreeRoot) + if err != nil { + t.Fatalf("loadExportState() failed: %v", err) + } + if loaded == nil { + t.Fatal("loadExportState() returned nil") + } + + // Verify fields + if loaded.WorktreeRoot != testState.WorktreeRoot { + t.Errorf("WorktreeRoot = %s, want %s", loaded.WorktreeRoot, testState.WorktreeRoot) + } + if loaded.LastExportCommit != testState.LastExportCommit { + t.Errorf("LastExportCommit = %s, want %s", loaded.LastExportCommit, testState.LastExportCommit) + } + if loaded.JSONLHash != testState.JSONLHash { + t.Errorf("JSONLHash = %s, want %s", loaded.JSONLHash, testState.JSONLHash) + } +} + +func TestExportStateJSON(t *testing.T) { + state := &ExportState{ + WorktreeRoot: "/path/to/worktree", + WorktreeHash: "abc123", + LastExportCommit: "def456", + LastExportTime: time.Date(2024, 1, 15, 10, 30, 0, 0, time.UTC), + JSONLHash: "hash789", + } + + data, err := json.MarshalIndent(state, "", " ") + if err != nil { + t.Fatalf("json.Marshal() failed: %v", err) + } + + // Verify JSON contains expected fields + jsonStr := string(data) + expectedFields := []string{ + `"worktree_root"`, + `"worktree_hash"`, + `"last_export_commit"`, + `"last_export_time"`, + `"jsonl_hash"`, + } + for _, field := range expectedFields { + if !strings.Contains(jsonStr, field) { + t.Errorf("JSON missing field %s: %s", field, jsonStr) + } + } + + // Verify omitempty works for empty optional fields + stateMinimal := &ExportState{ + WorktreeRoot: "/path", + LastExportCommit: "abc", + LastExportTime: time.Now(), + } + dataMinimal, _ := json.Marshal(stateMinimal) + jsonMinimal := string(dataMinimal) + + // WorktreeHash and JSONLHash should be omitted when empty + if strings.Contains(jsonMinimal, `"worktree_hash"`) { + t.Errorf("Empty worktree_hash should be omitted: %s", jsonMinimal) + } + if strings.Contains(jsonMinimal, `"jsonl_hash"`) { + t.Errorf("Empty jsonl_hash should be omitted: %s", jsonMinimal) + } +} + +func TestUpdateExportStateCommit(t *testing.T) { + tmpDir := t.TempDir() + beadsDir := filepath.Join(tmpDir, ".beads") + worktreeRoot := tmpDir + + // Create initial state + initialState := &ExportState{ + WorktreeRoot: worktreeRoot, + LastExportCommit: "old-commit", + LastExportTime: time.Now().Add(-time.Hour), + JSONLHash: "oldhash", + } + if err := saveExportState(beadsDir, worktreeRoot, initialState); err != nil { + t.Fatalf("saveExportState() failed: %v", err) + } + + // Update just the commit + updateExportStateCommit(beadsDir, worktreeRoot, "new-commit") + + // Load and verify + loaded, err := loadExportState(beadsDir, worktreeRoot) + if err != nil { + t.Fatalf("loadExportState() failed: %v", err) + } + + if loaded.LastExportCommit != "new-commit" { + t.Errorf("LastExportCommit = %s, want new-commit", loaded.LastExportCommit) + } + // JSONLHash should be preserved + if loaded.JSONLHash != "oldhash" { + t.Errorf("JSONLHash = %s, want oldhash (should be preserved)", loaded.JSONLHash) + } +} + +func TestComputeJSONLHashForHook(t *testing.T) { + tmpDir := t.TempDir() + + // Non-existent file should return empty string, no error + hash, err := computeJSONLHashForHook(filepath.Join(tmpDir, "nonexistent.jsonl")) + if err != nil { + t.Errorf("computeJSONLHashForHook() error for missing file: %v", err) + } + if hash != "" { + t.Errorf("computeJSONLHashForHook() = %s, want empty for missing file", hash) + } + + // Create a test file + testFile := filepath.Join(tmpDir, "test.jsonl") + content := `{"id": "test-1", "title": "Test"} +{"id": "test-2", "title": "Test 2"} +` + if err := os.WriteFile(testFile, []byte(content), 0644); err != nil { + t.Fatalf("Failed to write test file: %v", err) + } + + // Should get a hash + hash, err = computeJSONLHashForHook(testFile) + if err != nil { + t.Fatalf("computeJSONLHashForHook() failed: %v", err) + } + if hash == "" { + t.Error("computeJSONLHashForHook() returned empty hash for existing file") + } + + // Same content should produce same hash + hash2, _ := computeJSONLHashForHook(testFile) + if hash != hash2 { + t.Errorf("Same file produced different hashes: %s vs %s", hash, hash2) + } + + // Different content should produce different hash + testFile2 := filepath.Join(tmpDir, "test2.jsonl") + if err := os.WriteFile(testFile2, []byte(`{"different": true}`), 0644); err != nil { + t.Fatalf("Failed to write test file 2: %v", err) + } + hash3, _ := computeJSONLHashForHook(testFile2) + if hash == hash3 { + t.Errorf("Different files produced same hash: %s", hash) + } +}