Fix bd-u4f5: Add warning when import syncs with working tree but not git HEAD
- Detect uncommitted changes in .beads/issues.jsonl - Warn users when database matches working tree but differs from git HEAD - Clarify import status messages (working tree vs git sync) - Add comprehensive tests for dirty working tree scenarios - Prevents false confidence about sync status Amp-Thread-ID: https://ampcode.com/threads/T-5a0f1045-a690-42ef-8bfc-f8cf30ee4084 Co-authored-by: Amp <amp@ampcode.com>
This commit is contained in:
Steve Yegge
110
cmd/bd/import.go
110
cmd/bd/import.go
@@ -6,6 +6,8 @@ import (
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"os"
|
||||
"os/exec"
|
||||
"path/filepath"
|
||||
"sort"
|
||||
"strings"
|
||||
|
||||
@@ -128,6 +130,12 @@ NOTE: Import requires direct database access and does not work with daemon mode.
|
||||
|
||||
result, err := importIssuesCore(ctx, dbPath, store, allIssues, opts)
|
||||
|
||||
// Check for uncommitted changes in JSONL after import
|
||||
// Only check if we have an input file path (not stdin) and it's the default beads file
|
||||
if input != "" && (input == ".beads/issues.jsonl" || input == ".beads/beads.jsonl") {
|
||||
checkUncommittedChanges(input, result)
|
||||
}
|
||||
|
||||
// Handle errors and special cases
|
||||
if err != nil {
|
||||
// Check if it's a prefix mismatch error
|
||||
@@ -282,6 +290,108 @@ NOTE: Import requires direct database access and does not work with daemon mode.
|
||||
},
|
||||
}
|
||||
|
||||
// checkUncommittedChanges detects if the JSONL file has uncommitted changes
|
||||
// and warns the user if the working tree differs from git HEAD
|
||||
func checkUncommittedChanges(filePath string, result *ImportResult) {
|
||||
// Only warn if no actual changes were made (database already synced)
|
||||
if result.Created > 0 || result.Updated > 0 {
|
||||
return
|
||||
}
|
||||
|
||||
// Get the directory containing the file to use as git working directory
|
||||
workDir := filepath.Dir(filePath)
|
||||
|
||||
// Use git diff to check if working tree differs from HEAD
|
||||
cmd := fmt.Sprintf("git diff --quiet HEAD %s", filePath)
|
||||
exitCode, _ := runGitCommand(cmd, workDir)
|
||||
|
||||
// Exit code 0 = no changes, 1 = changes exist, >1 = error
|
||||
if exitCode == 1 {
|
||||
// Get line counts for context
|
||||
workingTreeLines := countLines(filePath)
|
||||
headLines := countLinesInGitHEAD(filePath, workDir)
|
||||
|
||||
fmt.Fprintf(os.Stderr, "\n⚠️ Warning: .beads/issues.jsonl has uncommitted changes\n")
|
||||
fmt.Fprintf(os.Stderr, " Working tree: %d lines\n", workingTreeLines)
|
||||
if headLines > 0 {
|
||||
fmt.Fprintf(os.Stderr, " Git HEAD: %d lines\n", headLines)
|
||||
}
|
||||
fmt.Fprintf(os.Stderr, "\n Import complete: database already synced with working tree\n")
|
||||
fmt.Fprintf(os.Stderr, " Run: git diff %s\n", filePath)
|
||||
fmt.Fprintf(os.Stderr, " To review uncommitted changes\n")
|
||||
}
|
||||
}
|
||||
|
||||
// runGitCommand executes a git command and returns exit code and output
|
||||
// workDir is the directory to run the command in (empty = current dir)
|
||||
func runGitCommand(cmd string, workDir string) (int, string) {
|
||||
// #nosec G204 - command is constructed internally
|
||||
gitCmd := exec.Command("sh", "-c", cmd)
|
||||
if workDir != "" {
|
||||
gitCmd.Dir = workDir
|
||||
}
|
||||
output, err := gitCmd.CombinedOutput()
|
||||
if err != nil {
|
||||
if exitErr, ok := err.(*exec.ExitError); ok {
|
||||
return exitErr.ExitCode(), string(output)
|
||||
}
|
||||
return -1, string(output)
|
||||
}
|
||||
return 0, string(output)
|
||||
}
|
||||
|
||||
// countLines counts the number of lines in a file
|
||||
func countLines(filePath string) int {
|
||||
// #nosec G304 - file path is controlled by caller
|
||||
f, err := os.Open(filePath)
|
||||
if err != nil {
|
||||
return 0
|
||||
}
|
||||
defer func() { _ = f.Close() }()
|
||||
|
||||
scanner := bufio.NewScanner(f)
|
||||
lines := 0
|
||||
for scanner.Scan() {
|
||||
lines++
|
||||
}
|
||||
return lines
|
||||
}
|
||||
|
||||
// countLinesInGitHEAD counts lines in the file as it exists in git HEAD
|
||||
func countLinesInGitHEAD(filePath string, workDir string) int {
|
||||
// First, find the git root
|
||||
findRootCmd := "git rev-parse --show-toplevel 2>/dev/null"
|
||||
exitCode, gitRootOutput := runGitCommand(findRootCmd, workDir)
|
||||
if exitCode != 0 {
|
||||
return 0
|
||||
}
|
||||
gitRoot := strings.TrimSpace(gitRootOutput)
|
||||
|
||||
// Make filePath relative to git root
|
||||
absPath, err := filepath.Abs(filePath)
|
||||
if err != nil {
|
||||
return 0
|
||||
}
|
||||
|
||||
relPath, err := filepath.Rel(gitRoot, absPath)
|
||||
if err != nil {
|
||||
return 0
|
||||
}
|
||||
|
||||
cmd := fmt.Sprintf("git show HEAD:%s 2>/dev/null | wc -l", relPath)
|
||||
exitCode, output := runGitCommand(cmd, workDir)
|
||||
if exitCode != 0 {
|
||||
return 0
|
||||
}
|
||||
|
||||
var lines int
|
||||
_, err = fmt.Sscanf(strings.TrimSpace(output), "%d", &lines)
|
||||
if err != nil {
|
||||
return 0
|
||||
}
|
||||
return lines
|
||||
}
|
||||
|
||||
func init() {
|
||||
importCmd.Flags().StringP("input", "i", "", "Input file (default: stdin)")
|
||||
importCmd.Flags().BoolP("skip-existing", "s", false, "Skip existing issues instead of updating them")
|
||||
|
||||
351
cmd/bd/import_uncommitted_test.go
Normal file
351
cmd/bd/import_uncommitted_test.go
Normal file
@@ -0,0 +1,351 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"encoding/json"
|
||||
"os"
|
||||
"os/exec"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/steveyegge/beads/internal/types"
|
||||
)
|
||||
|
||||
// TestImportWarnsUncommittedChanges tests bd-u4f5
|
||||
// Import should warn when database matches working tree but not git HEAD
|
||||
func TestImportWarnsUncommittedChanges(t *testing.T) {
|
||||
if testing.Short() {
|
||||
t.Skip("Skipping git-dependent test in short mode")
|
||||
}
|
||||
|
||||
// Create temporary directory with git repo
|
||||
tmpDir, err := os.MkdirTemp("", "beads-test-")
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create temp dir: %v", err)
|
||||
}
|
||||
defer os.RemoveAll(tmpDir)
|
||||
|
||||
// Initialize git repo
|
||||
gitInit := exec.Command("git", "init")
|
||||
gitInit.Dir = tmpDir
|
||||
if err := gitInit.Run(); err != nil {
|
||||
t.Fatalf("Failed to init git: %v", err)
|
||||
}
|
||||
|
||||
// Configure git user
|
||||
gitConfig1 := exec.Command("git", "config", "user.email", "test@example.com")
|
||||
gitConfig1.Dir = tmpDir
|
||||
if err := gitConfig1.Run(); err != nil {
|
||||
t.Fatalf("Failed to configure git: %v", err)
|
||||
}
|
||||
gitConfig2 := exec.Command("git", "config", "user.name", "Test User")
|
||||
gitConfig2.Dir = tmpDir
|
||||
if err := gitConfig2.Run(); err != nil {
|
||||
t.Fatalf("Failed to configure git: %v", err)
|
||||
}
|
||||
|
||||
// Create .beads directory
|
||||
beadsDir := filepath.Join(tmpDir, ".beads")
|
||||
if err := os.MkdirAll(beadsDir, 0755); err != nil {
|
||||
t.Fatalf("Failed to create .beads dir: %v", err)
|
||||
}
|
||||
|
||||
dbPath := filepath.Join(beadsDir, "beads.db")
|
||||
jsonlPath := filepath.Join(beadsDir, "issues.jsonl")
|
||||
|
||||
// Initialize database
|
||||
store := newTestStore(t, dbPath)
|
||||
ctx := context.Background()
|
||||
|
||||
// Step 1: Create initial issue and export
|
||||
issue1 := &types.Issue{
|
||||
ID: "test-1",
|
||||
Title: "Original Issue",
|
||||
Description: "Original description",
|
||||
Status: types.StatusOpen,
|
||||
Priority: 2,
|
||||
IssueType: types.TypeTask,
|
||||
}
|
||||
|
||||
if err := store.CreateIssue(ctx, issue1, "test"); err != nil {
|
||||
t.Fatalf("Failed to create issue: %v", err)
|
||||
}
|
||||
|
||||
// Export to JSONL
|
||||
issues, err := store.SearchIssues(ctx, "", types.IssueFilter{})
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to search issues: %v", err)
|
||||
}
|
||||
|
||||
f, err := os.Create(jsonlPath)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create JSONL: %v", err)
|
||||
}
|
||||
encoder := json.NewEncoder(f)
|
||||
for _, issue := range issues {
|
||||
if err := encoder.Encode(issue); err != nil {
|
||||
f.Close()
|
||||
t.Fatalf("Failed to encode issue: %v", err)
|
||||
}
|
||||
}
|
||||
f.Close()
|
||||
|
||||
// Commit the initial JSONL to git
|
||||
gitAdd := exec.Command("git", "add", ".beads/issues.jsonl")
|
||||
gitAdd.Dir = tmpDir
|
||||
if err := gitAdd.Run(); err != nil {
|
||||
t.Fatalf("Failed to git add: %v", err)
|
||||
}
|
||||
|
||||
gitCommit := exec.Command("git", "commit", "-m", "Initial commit")
|
||||
gitCommit.Dir = tmpDir
|
||||
if err := gitCommit.Run(); err != nil {
|
||||
t.Fatalf("Failed to git commit: %v", err)
|
||||
}
|
||||
|
||||
// Step 2: Add a new issue to database and export (creating uncommitted change)
|
||||
issue2 := &types.Issue{
|
||||
ID: "test-2",
|
||||
Title: "New Issue",
|
||||
Description: "New description",
|
||||
Status: types.StatusOpen,
|
||||
Priority: 1,
|
||||
IssueType: types.TypeBug,
|
||||
}
|
||||
|
||||
if err := store.CreateIssue(ctx, issue2, "test"); err != nil {
|
||||
t.Fatalf("Failed to create second issue: %v", err)
|
||||
}
|
||||
|
||||
// Export again (now JSONL has 2 issues, but git HEAD has 1)
|
||||
issues, err = store.SearchIssues(ctx, "", types.IssueFilter{})
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to search issues: %v", err)
|
||||
}
|
||||
|
||||
f, err = os.Create(jsonlPath)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to recreate JSONL: %v", err)
|
||||
}
|
||||
encoder = json.NewEncoder(f)
|
||||
for _, issue := range issues {
|
||||
if err := encoder.Encode(issue); err != nil {
|
||||
f.Close()
|
||||
t.Fatalf("Failed to encode issue: %v", err)
|
||||
}
|
||||
}
|
||||
f.Close()
|
||||
|
||||
// Step 3: Run import and capture output
|
||||
// Database already matches working tree, so import will report 0 created, 0 updated
|
||||
// But working tree differs from git HEAD, so we should see a warning
|
||||
|
||||
opts := ImportOptions{
|
||||
DryRun: false,
|
||||
SkipUpdate: false,
|
||||
Strict: false,
|
||||
}
|
||||
|
||||
// Read JSONL for import
|
||||
importData, err := os.ReadFile(jsonlPath)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to read JSONL: %v", err)
|
||||
}
|
||||
|
||||
var importIssues []*types.Issue
|
||||
lines := bytes.Split(importData, []byte("\n"))
|
||||
for _, line := range lines {
|
||||
if len(line) == 0 {
|
||||
continue
|
||||
}
|
||||
var issue types.Issue
|
||||
if err := json.Unmarshal(line, &issue); err != nil {
|
||||
t.Fatalf("Failed to unmarshal: %v", err)
|
||||
}
|
||||
importIssues = append(importIssues, &issue)
|
||||
}
|
||||
|
||||
result, err := importIssuesCore(ctx, dbPath, store, importIssues, opts)
|
||||
if err != nil {
|
||||
t.Fatalf("Import failed: %v", err)
|
||||
}
|
||||
|
||||
// Verify no changes (database already synced)
|
||||
if result.Created != 0 || result.Updated != 0 {
|
||||
t.Errorf("Expected 0 created, 0 updated, got created=%d updated=%d", result.Created, result.Updated)
|
||||
}
|
||||
|
||||
// Now test the warning detection function directly
|
||||
// Capture stderr to check for warning
|
||||
oldStderr := os.Stderr
|
||||
r, w, _ := os.Pipe()
|
||||
os.Stderr = w
|
||||
|
||||
checkUncommittedChanges(jsonlPath, result)
|
||||
|
||||
w.Close()
|
||||
os.Stderr = oldStderr
|
||||
|
||||
var buf bytes.Buffer
|
||||
buf.ReadFrom(r)
|
||||
output := buf.String()
|
||||
|
||||
// Verify warning is present
|
||||
if !strings.Contains(output, "Warning") {
|
||||
t.Errorf("Expected warning about uncommitted changes, got: %s", output)
|
||||
}
|
||||
if !strings.Contains(output, "uncommitted changes") {
|
||||
t.Errorf("Expected warning to mention 'uncommitted changes', got: %s", output)
|
||||
}
|
||||
if !strings.Contains(output, "Working tree:") {
|
||||
t.Errorf("Expected warning to show working tree line count, got: %s", output)
|
||||
}
|
||||
if !strings.Contains(output, "database already synced with working tree") {
|
||||
t.Errorf("Expected warning to clarify sync status, got: %s", output)
|
||||
}
|
||||
// Git HEAD line count is optional - may not show if git command fails
|
||||
// The important part is that we detect uncommitted changes at all
|
||||
}
|
||||
|
||||
// TestImportNoWarningWhenClean tests that import doesn't warn when working tree matches git HEAD
|
||||
func TestImportNoWarningWhenClean(t *testing.T) {
|
||||
if testing.Short() {
|
||||
t.Skip("Skipping git-dependent test in short mode")
|
||||
}
|
||||
|
||||
// Create temporary directory with git repo
|
||||
tmpDir, err := os.MkdirTemp("", "beads-test-")
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create temp dir: %v", err)
|
||||
}
|
||||
defer os.RemoveAll(tmpDir)
|
||||
|
||||
// Initialize git repo
|
||||
gitInit := exec.Command("git", "init")
|
||||
gitInit.Dir = tmpDir
|
||||
if err := gitInit.Run(); err != nil {
|
||||
t.Fatalf("Failed to init git: %v", err)
|
||||
}
|
||||
|
||||
// Configure git user
|
||||
gitConfig1 := exec.Command("git", "config", "user.email", "test@example.com")
|
||||
gitConfig1.Dir = tmpDir
|
||||
if err := gitConfig1.Run(); err != nil {
|
||||
t.Fatalf("Failed to configure git: %v", err)
|
||||
}
|
||||
gitConfig2 := exec.Command("git", "config", "user.name", "Test User")
|
||||
gitConfig2.Dir = tmpDir
|
||||
if err := gitConfig2.Run(); err != nil {
|
||||
t.Fatalf("Failed to configure git: %v", err)
|
||||
}
|
||||
|
||||
// Create .beads directory
|
||||
beadsDir := filepath.Join(tmpDir, ".beads")
|
||||
if err := os.MkdirAll(beadsDir, 0755); err != nil {
|
||||
t.Fatalf("Failed to create .beads dir: %v", err)
|
||||
}
|
||||
|
||||
dbPath := filepath.Join(beadsDir, "beads.db")
|
||||
jsonlPath := filepath.Join(beadsDir, "issues.jsonl")
|
||||
|
||||
// Initialize database
|
||||
store := newTestStore(t, dbPath)
|
||||
ctx := context.Background()
|
||||
|
||||
// Create and export issue
|
||||
issue := &types.Issue{
|
||||
ID: "test-1",
|
||||
Title: "Test Issue",
|
||||
Description: "Test description",
|
||||
Status: types.StatusOpen,
|
||||
Priority: 2,
|
||||
IssueType: types.TypeTask,
|
||||
}
|
||||
|
||||
if err := store.CreateIssue(ctx, issue, "test"); err != nil {
|
||||
t.Fatalf("Failed to create issue: %v", err)
|
||||
}
|
||||
|
||||
// Export to JSONL
|
||||
issues, err := store.SearchIssues(ctx, "", types.IssueFilter{})
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to search issues: %v", err)
|
||||
}
|
||||
|
||||
f, err := os.Create(jsonlPath)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create JSONL: %v", err)
|
||||
}
|
||||
encoder := json.NewEncoder(f)
|
||||
for _, issue := range issues {
|
||||
if err := encoder.Encode(issue); err != nil {
|
||||
f.Close()
|
||||
t.Fatalf("Failed to encode issue: %v", err)
|
||||
}
|
||||
}
|
||||
f.Close()
|
||||
|
||||
// Commit to git (now working tree matches HEAD)
|
||||
gitAdd := exec.Command("git", "add", ".beads/issues.jsonl")
|
||||
gitAdd.Dir = tmpDir
|
||||
if err := gitAdd.Run(); err != nil {
|
||||
t.Fatalf("Failed to git add: %v", err)
|
||||
}
|
||||
|
||||
gitCommit := exec.Command("git", "commit", "-m", "Commit JSONL")
|
||||
gitCommit.Dir = tmpDir
|
||||
if err := gitCommit.Run(); err != nil {
|
||||
t.Fatalf("Failed to git commit: %v", err)
|
||||
}
|
||||
|
||||
// Run import
|
||||
opts := ImportOptions{
|
||||
DryRun: false,
|
||||
SkipUpdate: false,
|
||||
Strict: false,
|
||||
}
|
||||
|
||||
importData, err := os.ReadFile(jsonlPath)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to read JSONL: %v", err)
|
||||
}
|
||||
|
||||
var importIssues []*types.Issue
|
||||
lines := bytes.Split(importData, []byte("\n"))
|
||||
for _, line := range lines {
|
||||
if len(line) == 0 {
|
||||
continue
|
||||
}
|
||||
var iss types.Issue
|
||||
if err := json.Unmarshal(line, &iss); err != nil {
|
||||
t.Fatalf("Failed to unmarshal: %v", err)
|
||||
}
|
||||
importIssues = append(importIssues, &iss)
|
||||
}
|
||||
|
||||
result, err := importIssuesCore(ctx, dbPath, store, importIssues, opts)
|
||||
if err != nil {
|
||||
t.Fatalf("Import failed: %v", err)
|
||||
}
|
||||
|
||||
// Capture stderr
|
||||
oldStderr := os.Stderr
|
||||
r, w, _ := os.Pipe()
|
||||
os.Stderr = w
|
||||
|
||||
checkUncommittedChanges(jsonlPath, result)
|
||||
|
||||
w.Close()
|
||||
os.Stderr = oldStderr
|
||||
|
||||
var buf bytes.Buffer
|
||||
buf.ReadFrom(r)
|
||||
output := buf.String()
|
||||
|
||||
// Verify NO warning when clean
|
||||
if strings.Contains(output, "Warning") || strings.Contains(output, "uncommitted") {
|
||||
t.Errorf("Expected no warning when working tree matches git HEAD, got: %s", output)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user