From 340016c9c668415f6acc59d970cc7d942dec1639 Mon Sep 17 00:00:00 2001 From: Steve Yegge Date: Sat, 29 Nov 2025 17:01:36 -0800 Subject: [PATCH] fix(bd-53c): Add reverse ZFC check to prevent stale DB from corrupting JSONL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause: bd sync exports DB to JSONL BEFORE pulling from remote. If the local DB is stale (fewer issues than JSONL), the stale data gets exported and committed, potentially corrupting the remote when pushed. The existing ZFC (Zero-Fill Check) only detected when DB had MORE issues than JSONL, missing the dangerous reverse case. Fix: Added "reverse ZFC" check in sync.go that detects when JSONL has significantly more issues than DB (>20% divergence or empty DB). When detected, it imports JSONL first to sync the database before any export occurs. This prevents stale/fresh clones from exporting their incomplete database state over a well-populated JSONL file. Version bump: 0.26.0 -> 0.26.1 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- cmd/bd/daemon_sync.go | 2 ++ cmd/bd/integrity.go | 5 +-- cmd/bd/sync.go | 71 ++++++++++++++++++++++++++++--------------- cmd/bd/version.go | 2 +- 4 files changed, 53 insertions(+), 27 deletions(-) diff --git a/cmd/bd/daemon_sync.go b/cmd/bd/daemon_sync.go index c0cf2150..ebcfcfaa 100644 --- a/cmd/bd/daemon_sync.go +++ b/cmd/bd/daemon_sync.go @@ -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 { diff --git a/cmd/bd/integrity.go b/cmd/bd/integrity.go index 2f6f677e..5cef8890 100644 --- a/cmd/bd/integrity.go +++ b/cmd/bd/integrity.go @@ -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 } diff --git a/cmd/bd/sync.go b/cmd/bd/sync.go index 6d433942..4eeb9d8c 100644 --- a/cmd/bd/sync.go +++ b/cmd/bd/sync.go @@ -171,25 +171,55 @@ 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)") + } + } + + // 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)") } - // Skip export after ZFC import - JSONL is source of truth - skipExport = true - fmt.Println("→ Skipping export (JSONL is source of truth after ZFC import)") } } } @@ -886,6 +916,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 +931,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 diff --git a/cmd/bd/version.go b/cmd/bd/version.go index c097392d..57267a22 100644 --- a/cmd/bd/version.go +++ b/cmd/bd/version.go @@ -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.1" // Build can be set via ldflags at compile time Build = "dev" // Commit and branch the git revision the binary was built from (optional ldflag)