Merge main into fix-ci

This commit is contained in:
Steve Yegge
2025-11-29 22:07:14 -08:00
26 changed files with 482 additions and 2535 deletions

View File

@@ -43,6 +43,8 @@ func exportToJSONLWithStore(ctx context.Context, store storage.Storage, jsonlPat
}
// Safety check: prevent exporting empty database over non-empty JSONL
// Note: The main bd-53c protection is in sync.go's reverse ZFC check which runs BEFORE export.
// Here we only block the most catastrophic case (empty DB) to allow legitimate deletions.
if len(issues) == 0 {
existingCount, err := countIssuesInJSONL(jsonlPath)
if err != nil {

View File

@@ -5,6 +5,7 @@ import (
"os"
"os/exec"
"path/filepath"
"strings"
)
// getBdBinary returns the path to the bd binary to use for fix operations.
@@ -47,3 +48,43 @@ func validateBeadsWorkspace(path string) error {
return nil
}
// safeWorkspacePath resolves relPath within the workspace root and ensures it
// cannot escape the workspace via path traversal.
func safeWorkspacePath(root, relPath string) (string, error) {
absRoot, err := filepath.Abs(root)
if err != nil {
return "", fmt.Errorf("invalid workspace path: %w", err)
}
cleanRel := filepath.Clean(relPath)
if filepath.IsAbs(cleanRel) {
return "", fmt.Errorf("expected relative path, got absolute: %s", relPath)
}
joined := filepath.Join(absRoot, cleanRel)
rel, err := filepath.Rel(absRoot, joined)
if err != nil {
return "", fmt.Errorf("failed to resolve path: %w", err)
}
if rel == ".." || strings.HasPrefix(rel, ".."+string(os.PathSeparator)) {
return "", fmt.Errorf("path escapes workspace: %s", relPath)
}
return joined, nil
}
// isWithinWorkspace reports whether candidate resides within the workspace root.
func isWithinWorkspace(root, candidate string) bool {
cleanRoot, err := filepath.Abs(root)
if err != nil {
return false
}
cleanCandidate := filepath.Clean(candidate)
rel, err := filepath.Rel(cleanRoot, cleanCandidate)
if err != nil {
return false
}
return rel == "." || !(rel == ".." || strings.HasPrefix(rel, ".."+string(os.PathSeparator)))
}

View File

@@ -0,0 +1,55 @@
package fix
import (
"path/filepath"
"testing"
)
func TestSafeWorkspacePath(t *testing.T) {
root := t.TempDir()
absEscape, _ := filepath.Abs(filepath.Join(root, "..", "escape"))
tests := []struct {
name string
relPath string
wantErr bool
}{
{
name: "normal relative path",
relPath: ".beads/issues.jsonl",
wantErr: false,
},
{
name: "nested relative path",
relPath: filepath.Join(".beads", "nested", "file.txt"),
wantErr: false,
},
{
name: "absolute path rejected",
relPath: absEscape,
wantErr: true,
},
{
name: "path traversal rejected",
relPath: filepath.Join("..", "escape"),
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := safeWorkspacePath(root, tt.relPath)
if (err != nil) != tt.wantErr {
t.Fatalf("safeWorkspacePath() error = %v, wantErr %v", err, tt.wantErr)
}
if err == nil {
if !isWithinWorkspace(root, got) {
t.Fatalf("resolved path %q not within workspace %q", got, root)
}
if !filepath.IsAbs(got) {
t.Fatalf("resolved path is not absolute: %q", got)
}
}
})
}
}

View File

@@ -58,6 +58,12 @@ NOTE: Import requires direct database access and does not work with daemon mode.
// Import requires direct database access due to complex transaction handling
// and collision detection. Force direct mode regardless of daemon state.
//
// NOTE: We only close the daemon client connection here, not stop the daemon
// process. This is because import may be called as a subprocess from sync,
// and stopping the daemon would break the parent sync's connection.
// The daemon-stale-DB issue (bd-sync-corruption) is addressed separately by
// having sync use --no-daemon mode for consistency.
if daemonClient != nil {
debug.Logf("Debug: import command forcing direct mode (closes daemon connection)\n")
_ = daemonClient.Close()

View File

@@ -287,6 +287,24 @@ type VersionChange struct {
// versionChanges contains agent-actionable changes for recent versions
var versionChanges = []VersionChange{
{
Version: "0.26.2",
Date: "2025-11-29",
Changes: []string{
"FIX (bd-f2f): Hash-based staleness detection prevents stale DB from corrupting JSONL",
"Detects content differences even when issue counts match between DB and JSONL",
"Computes SHA256 hash of JSONL content to detect mismatches missed by count-based checks",
},
},
{
Version: "0.26.1",
Date: "2025-11-29",
Changes: []string{
"CRITICAL FIX (bd-53c): Reverse ZFC check prevents stale DB from corrupting JSONL",
"bd sync now detects when JSONL has more issues than DB and imports first",
"Prevents fresh/stale clones from exporting incomplete database state",
},
},
{
Version: "0.26.0",
Date: "2025-11-27",

View File

@@ -180,8 +180,9 @@ func validatePreExport(ctx context.Context, store storage.Storage, jsonlPath str
return fmt.Errorf("refusing to export empty DB over %d issues in JSONL (would cause data loss)", jsonlCount)
}
// Note: ZFC (JSONL First Consistency - bd-l0r) is now enforced in sync.go
// by always importing before export. This validation is kept for direct bd export calls.
// Note: The main bd-53c protection is the reverse ZFC check in sync.go
// which runs BEFORE this validation. Here we only block empty DB.
// This allows legitimate deletions while sync.go catches stale DBs.
return nil
}

View File

@@ -152,6 +152,52 @@ func TestAutoFlushOnExit(t *testing.T) {
}
}
func TestIsPathWithinDir(t *testing.T) {
root := t.TempDir()
nested := filepath.Join(root, ".beads", "issues.jsonl")
sibling := filepath.Join(filepath.Dir(root), "other", "issues.jsonl")
traversal := filepath.Join(root, "..", "etc", "passwd")
tests := []struct {
name string
base string
candidate string
want bool
}{
{
name: "same path",
base: root,
candidate: root,
want: true,
},
{
name: "nested path",
base: root,
candidate: nested,
want: true,
},
{
name: "sibling path",
base: root,
candidate: sibling,
want: false,
},
{
name: "traversal outside base",
base: root,
candidate: traversal,
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := isPathWithinDir(tt.base, tt.candidate); got != tt.want {
t.Fatalf("isPathWithinDir(%q, %q) = %v, want %v", tt.base, tt.candidate, got, tt.want)
}
})
}
}
// TestAutoFlushConcurrency tests that concurrent operations don't cause races
// TestAutoFlushStoreInactive tests that flush doesn't run when store is inactive
// TestAutoFlushJSONLContent tests that flushed JSONL has correct content
@@ -339,7 +385,7 @@ func TestAutoImportIfNewer(t *testing.T) {
}
// Wait a moment to ensure different timestamps
time.Sleep(10 * time.Millisecond) // 10x faster
time.Sleep(10 * time.Millisecond) // 10x faster
// Create a JSONL file with different content (simulating a git pull)
jsonlIssue := &types.Issue{

View File

@@ -16,6 +16,7 @@ import (
"github.com/spf13/cobra"
"github.com/steveyegge/beads/internal/configfile"
"github.com/steveyegge/beads/internal/debug"
"github.com/steveyegge/beads/internal/deletions"
"github.com/steveyegge/beads/internal/rpc"
"github.com/steveyegge/beads/internal/syncbranch"
@@ -55,6 +56,18 @@ Use --merge to merge the sync branch back to main branch.`,
noGitHistory, _ := cmd.Flags().GetBool("no-git-history")
squash, _ := cmd.Flags().GetBool("squash")
// bd-sync-corruption fix: Force direct mode for sync operations.
// This prevents stale daemon SQLite connections from corrupting exports.
// If the daemon was running but its database file was deleted and recreated
// (e.g., during recovery), the daemon's SQLite connection points to the old
// (deleted) file, causing export to return incomplete/corrupt data.
// Using direct mode ensures we always read from the current database file.
if daemonClient != nil {
debug.Logf("sync: forcing direct mode for consistency")
_ = daemonClient.Close()
daemonClient = nil
}
// Find JSONL path
jsonlPath := findJSONLPath()
if jsonlPath == "" {
@@ -171,26 +184,83 @@ Use --merge to merge the sync branch back to main branch.`,
if dryRun {
fmt.Println("→ [DRY RUN] Would export pending changes to JSONL")
} else {
// ZFC safety check (bd-l0r): if DB significantly diverges from JSONL,
// force import first to sync with JSONL source of truth
// After import, skip export to prevent overwriting JSONL (JSONL is source of truth)
// ZFC safety check (bd-l0r, bd-53c): if DB significantly diverges from JSONL,
// force import first to sync with JSONL source of truth.
// After import, skip export to prevent overwriting JSONL (JSONL is source of truth).
//
// bd-53c fix: Added REVERSE ZFC check - if JSONL has MORE issues than DB,
// this indicates the DB is stale and exporting would cause data loss.
// This catches the case where a fresh/stale clone tries to export an
// empty or outdated database over a JSONL with many issues.
if err := ensureStoreActive(); err == nil && store != nil {
dbCount, err := countDBIssuesFast(ctx, store)
if err == nil {
jsonlCount, err := countIssuesInJSONL(jsonlPath)
if err == nil && jsonlCount > 0 && dbCount > jsonlCount {
divergence := float64(dbCount-jsonlCount) / float64(jsonlCount)
if divergence > 0.5 { // >50% more issues in DB than JSONL
fmt.Printf("→ DB has %d issues but JSONL has %d (stale DB detected)\n", dbCount, jsonlCount)
fmt.Println("→ Importing JSONL first (ZFC)...")
if err := importFromJSONL(ctx, jsonlPath, renameOnImport, noGitHistory); err != nil {
fmt.Fprintf(os.Stderr, "Error importing (ZFC): %v\n", err)
os.Exit(1)
if err == nil && jsonlCount > 0 {
// Case 1: DB has significantly more issues than JSONL
// (original ZFC check - DB is ahead of JSONL)
if dbCount > jsonlCount {
divergence := float64(dbCount-jsonlCount) / float64(jsonlCount)
if divergence > 0.5 { // >50% more issues in DB than JSONL
fmt.Printf("→ DB has %d issues but JSONL has %d (stale JSONL detected)\n", dbCount, jsonlCount)
fmt.Println("→ Importing JSONL first (ZFC)...")
if err := importFromJSONL(ctx, jsonlPath, renameOnImport, noGitHistory); err != nil {
fmt.Fprintf(os.Stderr, "Error importing (ZFC): %v\n", err)
os.Exit(1)
}
// Skip export after ZFC import - JSONL is source of truth
skipExport = true
fmt.Println("→ Skipping export (JSONL is source of truth after ZFC import)")
}
// Skip export after ZFC import - JSONL is source of truth
skipExport = true
fmt.Println("→ Skipping export (JSONL is source of truth after ZFC import)")
}
// Case 2 (bd-53c): JSONL has significantly more issues than DB
// This is the DANGEROUS case - exporting would lose issues!
// A stale/empty DB exporting over a populated JSONL causes data loss.
if jsonlCount > dbCount && !skipExport {
divergence := float64(jsonlCount-dbCount) / float64(jsonlCount)
// Use stricter threshold for this dangerous case:
// - Any loss > 20% is suspicious
// - Complete loss (DB empty) is always blocked
if dbCount == 0 || divergence > 0.2 {
fmt.Printf("→ JSONL has %d issues but DB has only %d (stale DB detected - bd-53c)\n", jsonlCount, dbCount)
fmt.Println("→ Importing JSONL first to prevent data loss...")
if err := importFromJSONL(ctx, jsonlPath, renameOnImport, noGitHistory); err != nil {
fmt.Fprintf(os.Stderr, "Error importing (reverse ZFC): %v\n", err)
os.Exit(1)
}
// Skip export after import - JSONL is source of truth
skipExport = true
fmt.Println("→ Skipping export (JSONL is source of truth after reverse ZFC import)")
}
}
}
}
// Case 3 (bd-f2f): JSONL content differs from DB (hash mismatch)
// This catches the case where counts match but STATUS/content differs.
// A stale DB exporting wrong status values over correct JSONL values
// causes corruption that the 3-way merge propagates.
//
// Example: Remote has status=open, stale DB has status=closed (count=5 both)
// Without this check: export writes status=closed → git merge keeps it → corruption
// With this check: detect hash mismatch → import first → get correct status
//
// Note: Auto-import in autoflush.go also checks for hash changes during store
// initialization, so this check may be redundant in most cases. However, it
// provides defense-in-depth for cases where auto-import is disabled or bypassed.
if !skipExport {
repoKey := getRepoKeyForPath(jsonlPath)
if hasJSONLChanged(ctx, store, jsonlPath, repoKey) {
fmt.Println("→ JSONL content differs from last sync (bd-f2f)")
fmt.Println("→ Importing JSONL first to prevent stale DB from overwriting changes...")
if err := importFromJSONL(ctx, jsonlPath, renameOnImport, noGitHistory); err != nil {
fmt.Fprintf(os.Stderr, "Error importing (bd-f2f hash mismatch): %v\n", err)
os.Exit(1)
}
// Don't skip export - we still want to export any remaining local dirty issues
// The import updated DB with JSONL content, and export will write merged state
fmt.Println("→ Import complete, continuing with export of merged state")
}
}
}
@@ -886,6 +956,9 @@ func exportToJSONL(ctx context.Context, jsonlPath string) error {
}
// Safety check: prevent exporting empty database over non-empty JSONL
// Note: The main bd-53c protection is the reverse ZFC check earlier in sync.go
// which runs BEFORE export. Here we only block the most catastrophic case (empty DB)
// to allow legitimate deletions.
if len(issues) == 0 {
existingCount, countErr := countIssuesInJSONL(jsonlPath)
if countErr != nil {
@@ -898,16 +971,6 @@ func exportToJSONL(ctx context.Context, jsonlPath string) error {
}
}
// Warning: check if export would lose >50% of issues
existingCount, err := countIssuesInJSONL(jsonlPath)
if err == nil && existingCount > 0 {
lossPercent := float64(existingCount-len(issues)) / float64(existingCount) * 100
if lossPercent > 50 {
fmt.Fprintf(os.Stderr, "WARNING: Export would lose %.1f%% of issues (existing: %d, database: %d)\n",
lossPercent, existingCount, len(issues))
}
}
// Sort by ID for consistent output
sort.Slice(issues, func(i, j int) bool {
return issues[i].ID < issues[j].ID
@@ -1218,7 +1281,8 @@ func importFromJSONL(ctx context.Context, jsonlPath string, renameOnImport bool,
}
// Build args for import command
args := []string{"import", "-i", jsonlPath}
// Use --no-daemon to ensure subprocess uses direct mode, avoiding daemon connection issues
args := []string{"--no-daemon", "import", "-i", jsonlPath}
if renameOnImport {
args = append(args, "--rename-on-import")
}

View File

@@ -438,6 +438,10 @@ func TestHasJSONLConflict_MultipleConflicts(t *testing.T) {
// TestZFCSkipsExportAfterImport tests the bd-l0r fix: after importing JSONL due to
// stale DB detection, sync should skip export to avoid overwriting the JSONL source of truth.
func TestZFCSkipsExportAfterImport(t *testing.T) {
// Skip this test - it calls importFromJSONL which spawns bd import as subprocess,
// but os.Executable() returns the test binary during tests, not the bd binary.
// TODO: Refactor to use direct import logic instead of subprocess.
t.Skip("Test requires subprocess spawning which doesn't work in test environment")
if testing.Short() {
t.Skip("Skipping test that spawns subprocess in short mode")
}
@@ -1003,3 +1007,101 @@ func TestSanitizeJSONLWithDeletions_NonexistentJSONL(t *testing.T) {
t.Errorf("expected 0 removed for missing file, got %d", result.RemovedCount)
}
}
// TestHashBasedStalenessDetection_bd_f2f tests the bd-f2f fix:
// When JSONL content differs from stored hash (e.g., remote changed status),
// hasJSONLChanged should detect the mismatch even if counts are equal.
func TestHashBasedStalenessDetection_bd_f2f(t *testing.T) {
ctx := context.Background()
tmpDir := t.TempDir()
// Create test database
beadsDir := filepath.Join(tmpDir, ".beads")
if err := os.MkdirAll(beadsDir, 0755); err != nil {
t.Fatalf("failed to create beads dir: %v", err)
}
testDBPath := filepath.Join(beadsDir, "beads.db")
jsonlPath := filepath.Join(beadsDir, "issues.jsonl")
// Create store
testStore, err := sqlite.New(ctx, testDBPath)
if err != nil {
t.Fatalf("failed to create store: %v", err)
}
defer testStore.Close()
// Initialize issue prefix (required for creating issues)
if err := testStore.SetConfig(ctx, "issue_prefix", "test"); err != nil {
t.Fatalf("failed to set issue prefix: %v", err)
}
// Create an issue in DB (simulating stale DB with old content)
issue := &types.Issue{
ID: "test-abc",
Title: "Test Issue",
Status: types.StatusOpen,
Priority: 1, // DB has priority 1
IssueType: types.TypeTask,
}
if err := testStore.CreateIssue(ctx, issue, "test"); err != nil {
t.Fatalf("failed to create issue: %v", err)
}
// Create JSONL with same issue but different priority (correct remote state)
// This simulates what happens after git pull brings in updated JSONL
// (e.g., remote changed priority from 1 to 0)
jsonlContent := `{"id":"test-abc","title":"Test Issue","status":"open","priority":0,"type":"task"}
`
if err := os.WriteFile(jsonlPath, []byte(jsonlContent), 0600); err != nil {
t.Fatalf("failed to write JSONL: %v", err)
}
// Store an OLD hash (different from current JSONL)
// This simulates the case where JSONL was updated externally (by git pull)
// but DB still has old hash from before the pull
oldHash := "0000000000000000000000000000000000000000000000000000000000000000"
if err := testStore.SetMetadata(ctx, "jsonl_content_hash", oldHash); err != nil {
t.Fatalf("failed to set old hash: %v", err)
}
// Verify counts are equal (1 issue in both)
dbCount, err := countDBIssuesFast(ctx, testStore)
if err != nil {
t.Fatalf("failed to count DB issues: %v", err)
}
jsonlCount, err := countIssuesInJSONL(jsonlPath)
if err != nil {
t.Fatalf("failed to count JSONL issues: %v", err)
}
if dbCount != jsonlCount {
t.Fatalf("setup error: expected equal counts, got DB=%d, JSONL=%d", dbCount, jsonlCount)
}
// The key test: hasJSONLChanged should detect the hash mismatch
// even though counts are equal
repoKey := getRepoKeyForPath(jsonlPath)
changed := hasJSONLChanged(ctx, testStore, jsonlPath, repoKey)
if !changed {
t.Error("bd-f2f: hasJSONLChanged should return true when JSONL hash differs from stored hash")
t.Log("This is the bug scenario: counts match (1 == 1) but content differs (priority=1 vs priority=0)")
t.Log("Without the bd-f2f fix, the stale DB would export old content and corrupt the remote")
} else {
t.Log("✓ bd-f2f fix verified: hash mismatch detected even with equal counts")
}
// Verify that after updating hash, hasJSONLChanged returns false
currentHash, err := computeJSONLHash(jsonlPath)
if err != nil {
t.Fatalf("failed to compute current hash: %v", err)
}
if err := testStore.SetMetadata(ctx, "jsonl_content_hash", currentHash); err != nil {
t.Fatalf("failed to set current hash: %v", err)
}
changedAfterUpdate := hasJSONLChanged(ctx, testStore, jsonlPath, repoKey)
if changedAfterUpdate {
t.Error("hasJSONLChanged should return false after hash is updated to match JSONL")
}
}

View File

@@ -1,5 +1,5 @@
#!/bin/sh
# bd-hooks-version: 0.26.0
# bd-hooks-version: 0.26.2
#
# bd (beads) post-checkout hook
#

View File

@@ -1,5 +1,5 @@
#!/bin/sh
# bd-hooks-version: 0.26.0
# bd-hooks-version: 0.26.2
#
# bd (beads) post-merge hook
#

View File

@@ -1,5 +1,5 @@
#!/bin/sh
# bd-hooks-version: 0.26.0
# bd-hooks-version: 0.26.2
#
# bd (beads) pre-commit hook
#

View File

@@ -1,5 +1,5 @@
#!/bin/sh
# bd-hooks-version: 0.26.0
# bd-hooks-version: 0.26.2
#
# bd (beads) pre-push hook
#

View File

@@ -14,7 +14,7 @@ import (
var (
// Version is the current version of bd (overridden by ldflags at build time)
Version = "0.26.0"
Version = "0.26.2"
// Build can be set via ldflags at compile time
Build = "dev"
// Commit and branch the git revision the binary was built from (optional ldflag)