Fix Windows and Nix test failures
Windows test fixes (bd-web8): - Add sanitizeMetadataKey() to replace colons with underscores - Windows absolute paths (e.g., C:\...) contain colons that conflict with metadata key separator ':' - Update tests to use sanitized keys when checking metadata - Tests now pass on Windows by auto-sanitizing path-based keys Nix flake fixes (bd-8y1a): - Update vendorHash in default.nix to match current Go module dependencies - Hash mismatch was causing Nix build failures in CI Test improvements: - Rename TestUpdateExportMetadataInvalidKeySuffix to reflect new behavior - Test now verifies sanitization instead of rejection - All tests pass locally 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
File diff suppressed because one or more lines are too long
@@ -238,6 +238,14 @@ func getRepoKeyForPath(jsonlPath string) string {
|
|||||||
return ""
|
return ""
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// sanitizeMetadataKey removes or replaces characters that conflict with metadata key format.
|
||||||
|
// On Windows, absolute paths contain colons (e.g., C:\...) which conflict with the ':' separator
|
||||||
|
// used in multi-repo metadata keys. This function replaces colons with underscores to make
|
||||||
|
// paths safe for use as metadata key suffixes (bd-web8).
|
||||||
|
func sanitizeMetadataKey(key string) string {
|
||||||
|
return strings.ReplaceAll(key, ":", "_")
|
||||||
|
}
|
||||||
|
|
||||||
// updateExportMetadata updates last_import_hash and related metadata after a successful export.
|
// updateExportMetadata updates last_import_hash and related metadata after a successful export.
|
||||||
// This prevents "JSONL content has changed since last import" errors on subsequent exports (bd-ymj fix).
|
// This prevents "JSONL content has changed since last import" errors on subsequent exports (bd-ymj fix).
|
||||||
// In multi-repo mode, keySuffix should be the stable repo identifier (e.g., ".", "../frontend").
|
// In multi-repo mode, keySuffix should be the stable repo identifier (e.g., ".", "../frontend").
|
||||||
@@ -246,6 +254,7 @@ func getRepoKeyForPath(jsonlPath string) string {
|
|||||||
// - Single-repo mode: "last_import_hash", "last_import_time", "last_import_mtime"
|
// - Single-repo mode: "last_import_hash", "last_import_time", "last_import_mtime"
|
||||||
// - Multi-repo mode: "last_import_hash:<repo_key>", "last_import_time:<repo_key>", etc.
|
// - Multi-repo mode: "last_import_hash:<repo_key>", "last_import_time:<repo_key>", etc.
|
||||||
// where <repo_key> is a stable repo identifier like "." or "../frontend"
|
// where <repo_key> is a stable repo identifier like "." or "../frontend"
|
||||||
|
// - Windows paths: Colons in absolute paths (e.g., C:\...) are replaced with underscores (bd-web8)
|
||||||
//
|
//
|
||||||
// Transaction boundaries (bd-ar2.6):
|
// Transaction boundaries (bd-ar2.6):
|
||||||
// This function does NOT provide atomicity between JSONL write, metadata updates, and DB mtime.
|
// This function does NOT provide atomicity between JSONL write, metadata updates, and DB mtime.
|
||||||
@@ -256,10 +265,9 @@ func getRepoKeyForPath(jsonlPath string) string {
|
|||||||
// 3. Current approach is simple and doesn't require complex WAL or format changes
|
// 3. Current approach is simple and doesn't require complex WAL or format changes
|
||||||
// Future: Consider Option 4 (defensive checks on startup) if this becomes a common issue.
|
// Future: Consider Option 4 (defensive checks on startup) if this becomes a common issue.
|
||||||
func updateExportMetadata(ctx context.Context, store storage.Storage, jsonlPath string, log daemonLogger, keySuffix string) {
|
func updateExportMetadata(ctx context.Context, store storage.Storage, jsonlPath string, log daemonLogger, keySuffix string) {
|
||||||
// Validate keySuffix doesn't contain the separator character (bd-ar2.12)
|
// Sanitize keySuffix to handle Windows paths with colons (bd-web8)
|
||||||
if keySuffix != "" && strings.Contains(keySuffix, ":") {
|
if keySuffix != "" {
|
||||||
log.log("Error: invalid keySuffix contains ':' separator: %s", keySuffix)
|
keySuffix = sanitizeMetadataKey(keySuffix)
|
||||||
return
|
|
||||||
}
|
}
|
||||||
|
|
||||||
currentHash, err := computeJSONLHash(jsonlPath)
|
currentHash, err := computeJSONLHash(jsonlPath)
|
||||||
|
|||||||
@@ -3,10 +3,8 @@ package main
|
|||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
"fmt"
|
|
||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"strings"
|
|
||||||
"testing"
|
"testing"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
@@ -450,8 +448,8 @@ func TestUpdateExportMetadataMultiRepo(t *testing.T) {
|
|||||||
updateExportMetadata(ctx, store, jsonlPath1, mockLogger, jsonlPath1)
|
updateExportMetadata(ctx, store, jsonlPath1, mockLogger, jsonlPath1)
|
||||||
updateExportMetadata(ctx, store, jsonlPath2, mockLogger, jsonlPath2)
|
updateExportMetadata(ctx, store, jsonlPath2, mockLogger, jsonlPath2)
|
||||||
|
|
||||||
// Verify per-repo metadata was set with correct keys
|
// Verify per-repo metadata was set with correct keys (bd-web8: keys are sanitized)
|
||||||
hash1Key := "last_import_hash:" + jsonlPath1
|
hash1Key := "last_import_hash:" + sanitizeMetadataKey(jsonlPath1)
|
||||||
hash1, err := store.GetMetadata(ctx, hash1Key)
|
hash1, err := store.GetMetadata(ctx, hash1Key)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("failed to get %s: %v", hash1Key, err)
|
t.Fatalf("failed to get %s: %v", hash1Key, err)
|
||||||
@@ -460,7 +458,7 @@ func TestUpdateExportMetadataMultiRepo(t *testing.T) {
|
|||||||
t.Errorf("expected %s to be set", hash1Key)
|
t.Errorf("expected %s to be set", hash1Key)
|
||||||
}
|
}
|
||||||
|
|
||||||
hash2Key := "last_import_hash:" + jsonlPath2
|
hash2Key := "last_import_hash:" + sanitizeMetadataKey(jsonlPath2)
|
||||||
hash2, err := store.GetMetadata(ctx, hash2Key)
|
hash2, err := store.GetMetadata(ctx, hash2Key)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("failed to get %s: %v", hash2Key, err)
|
t.Fatalf("failed to get %s: %v", hash2Key, err)
|
||||||
@@ -478,8 +476,8 @@ func TestUpdateExportMetadataMultiRepo(t *testing.T) {
|
|||||||
t.Error("expected global last_import_hash to not be set when using per-repo keys")
|
t.Error("expected global last_import_hash to not be set when using per-repo keys")
|
||||||
}
|
}
|
||||||
|
|
||||||
// Verify mtime metadata was also set per-repo
|
// Verify mtime metadata was also set per-repo (bd-web8: keys are sanitized)
|
||||||
mtime1Key := "last_import_mtime:" + jsonlPath1
|
mtime1Key := "last_import_mtime:" + sanitizeMetadataKey(jsonlPath1)
|
||||||
mtime1, err := store.GetMetadata(ctx, mtime1Key)
|
mtime1, err := store.GetMetadata(ctx, mtime1Key)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("failed to get %s: %v", mtime1Key, err)
|
t.Fatalf("failed to get %s: %v", mtime1Key, err)
|
||||||
@@ -488,7 +486,7 @@ func TestUpdateExportMetadataMultiRepo(t *testing.T) {
|
|||||||
t.Errorf("expected %s to be set", mtime1Key)
|
t.Errorf("expected %s to be set", mtime1Key)
|
||||||
}
|
}
|
||||||
|
|
||||||
mtime2Key := "last_import_mtime:" + jsonlPath2
|
mtime2Key := "last_import_mtime:" + sanitizeMetadataKey(jsonlPath2)
|
||||||
mtime2, err := store.GetMetadata(ctx, mtime2Key)
|
mtime2, err := store.GetMetadata(ctx, mtime2Key)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("failed to get %s: %v", mtime2Key, err)
|
t.Fatalf("failed to get %s: %v", mtime2Key, err)
|
||||||
@@ -587,8 +585,8 @@ func TestExportWithMultiRepoConfigUpdatesAllMetadata(t *testing.T) {
|
|||||||
updateExportMetadata(ctx, store, path, mockLogger, repoKey)
|
updateExportMetadata(ctx, store, path, mockLogger, repoKey)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Verify metadata for primary repo
|
// Verify metadata for primary repo (bd-web8: keys are sanitized)
|
||||||
primaryHashKey := "last_import_hash:" + primaryDir
|
primaryHashKey := "last_import_hash:" + sanitizeMetadataKey(primaryDir)
|
||||||
primaryHash, err := store.GetMetadata(ctx, primaryHashKey)
|
primaryHash, err := store.GetMetadata(ctx, primaryHashKey)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("failed to get %s: %v", primaryHashKey, err)
|
t.Fatalf("failed to get %s: %v", primaryHashKey, err)
|
||||||
@@ -597,7 +595,7 @@ func TestExportWithMultiRepoConfigUpdatesAllMetadata(t *testing.T) {
|
|||||||
t.Errorf("expected %s to be set after export", primaryHashKey)
|
t.Errorf("expected %s to be set after export", primaryHashKey)
|
||||||
}
|
}
|
||||||
|
|
||||||
primaryTimeKey := "last_import_time:" + primaryDir
|
primaryTimeKey := "last_import_time:" + sanitizeMetadataKey(primaryDir)
|
||||||
primaryTime, err := store.GetMetadata(ctx, primaryTimeKey)
|
primaryTime, err := store.GetMetadata(ctx, primaryTimeKey)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("failed to get %s: %v", primaryTimeKey, err)
|
t.Fatalf("failed to get %s: %v", primaryTimeKey, err)
|
||||||
@@ -606,7 +604,7 @@ func TestExportWithMultiRepoConfigUpdatesAllMetadata(t *testing.T) {
|
|||||||
t.Errorf("expected %s to be set after export", primaryTimeKey)
|
t.Errorf("expected %s to be set after export", primaryTimeKey)
|
||||||
}
|
}
|
||||||
|
|
||||||
primaryMtimeKey := "last_import_mtime:" + primaryDir
|
primaryMtimeKey := "last_import_mtime:" + sanitizeMetadataKey(primaryDir)
|
||||||
primaryMtime, err := store.GetMetadata(ctx, primaryMtimeKey)
|
primaryMtime, err := store.GetMetadata(ctx, primaryMtimeKey)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("failed to get %s: %v", primaryMtimeKey, err)
|
t.Fatalf("failed to get %s: %v", primaryMtimeKey, err)
|
||||||
@@ -615,8 +613,8 @@ func TestExportWithMultiRepoConfigUpdatesAllMetadata(t *testing.T) {
|
|||||||
t.Errorf("expected %s to be set after export", primaryMtimeKey)
|
t.Errorf("expected %s to be set after export", primaryMtimeKey)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Verify metadata for additional repo
|
// Verify metadata for additional repo (bd-web8: keys are sanitized)
|
||||||
additionalHashKey := "last_import_hash:" + additionalDir
|
additionalHashKey := "last_import_hash:" + sanitizeMetadataKey(additionalDir)
|
||||||
additionalHash, err := store.GetMetadata(ctx, additionalHashKey)
|
additionalHash, err := store.GetMetadata(ctx, additionalHashKey)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("failed to get %s: %v", additionalHashKey, err)
|
t.Fatalf("failed to get %s: %v", additionalHashKey, err)
|
||||||
@@ -625,7 +623,7 @@ func TestExportWithMultiRepoConfigUpdatesAllMetadata(t *testing.T) {
|
|||||||
t.Errorf("expected %s to be set after export", additionalHashKey)
|
t.Errorf("expected %s to be set after export", additionalHashKey)
|
||||||
}
|
}
|
||||||
|
|
||||||
additionalTimeKey := "last_import_time:" + additionalDir
|
additionalTimeKey := "last_import_time:" + sanitizeMetadataKey(additionalDir)
|
||||||
additionalTime, err := store.GetMetadata(ctx, additionalTimeKey)
|
additionalTime, err := store.GetMetadata(ctx, additionalTimeKey)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("failed to get %s: %v", additionalTimeKey, err)
|
t.Fatalf("failed to get %s: %v", additionalTimeKey, err)
|
||||||
@@ -634,7 +632,7 @@ func TestExportWithMultiRepoConfigUpdatesAllMetadata(t *testing.T) {
|
|||||||
t.Errorf("expected %s to be set after export", additionalTimeKey)
|
t.Errorf("expected %s to be set after export", additionalTimeKey)
|
||||||
}
|
}
|
||||||
|
|
||||||
additionalMtimeKey := "last_import_mtime:" + additionalDir
|
additionalMtimeKey := "last_import_mtime:" + sanitizeMetadataKey(additionalDir)
|
||||||
additionalMtime, err := store.GetMetadata(ctx, additionalMtimeKey)
|
additionalMtime, err := store.GetMetadata(ctx, additionalMtimeKey)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("failed to get %s: %v", additionalMtimeKey, err)
|
t.Fatalf("failed to get %s: %v", additionalMtimeKey, err)
|
||||||
@@ -707,39 +705,36 @@ func TestUpdateExportMetadataInvalidKeySuffix(t *testing.T) {
|
|||||||
t.Fatalf("export failed: %v", err)
|
t.Fatalf("export failed: %v", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Create mock logger that captures error messages
|
// Create mock logger
|
||||||
var logMessages []string
|
|
||||||
mockLogger := daemonLogger{
|
mockLogger := daemonLogger{
|
||||||
logFunc: func(format string, args ...interface{}) {
|
logFunc: func(format string, args ...interface{}) {
|
||||||
msg := fmt.Sprintf(format, args...)
|
t.Logf(format, args...)
|
||||||
logMessages = append(logMessages, msg)
|
|
||||||
t.Logf("%s", msg)
|
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
|
|
||||||
// Try to update metadata with invalid keySuffix containing ':'
|
// Update metadata with keySuffix containing ':' (bd-web8: should be auto-sanitized)
|
||||||
invalidKeySuffix := "repo:path"
|
// This simulates Windows absolute paths like "C:\Users\..."
|
||||||
updateExportMetadata(ctx, store, jsonlPath, mockLogger, invalidKeySuffix)
|
keySuffixWithColon := "C:/Users/repo/path"
|
||||||
|
updateExportMetadata(ctx, store, jsonlPath, mockLogger, keySuffixWithColon)
|
||||||
|
|
||||||
// Verify that error was logged
|
// Verify metadata WAS set with sanitized key (colons replaced with underscores)
|
||||||
var foundError bool
|
sanitized := sanitizeMetadataKey(keySuffixWithColon)
|
||||||
for _, msg := range logMessages {
|
sanitizedKey := "last_import_hash:" + sanitized
|
||||||
if strings.Contains(msg, "Error: invalid keySuffix") && strings.Contains(msg, invalidKeySuffix) {
|
hash, err := store.GetMetadata(ctx, sanitizedKey)
|
||||||
foundError = true
|
|
||||||
break
|
|
||||||
}
|
|
||||||
}
|
|
||||||
if !foundError {
|
|
||||||
t.Error("expected error log for invalid keySuffix containing ':'")
|
|
||||||
}
|
|
||||||
|
|
||||||
// Verify metadata was NOT set (update should have been rejected)
|
|
||||||
invalidKey := "last_import_hash:" + invalidKeySuffix
|
|
||||||
hash, err := store.GetMetadata(ctx, invalidKey)
|
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("failed to get metadata: %v", err)
|
t.Fatalf("failed to get metadata: %v", err)
|
||||||
}
|
}
|
||||||
if hash != "" {
|
if hash == "" {
|
||||||
t.Errorf("expected no metadata to be set with invalid key, but got: %s", hash)
|
t.Errorf("expected metadata to be set with sanitized key %s", sanitizedKey)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Verify that the original unsanitized key was NOT used
|
||||||
|
unsanitizedKey := "last_import_hash:" + keySuffixWithColon
|
||||||
|
unsanitizedHash, err := store.GetMetadata(ctx, unsanitizedKey)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("failed to check unsanitized key: %v", err)
|
||||||
|
}
|
||||||
|
if unsanitizedHash != "" {
|
||||||
|
t.Errorf("expected unsanitized key %s to NOT be set", unsanitizedKey)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -9,7 +9,7 @@ pkgs.buildGoModule {
|
|||||||
subPackages = [ "cmd/bd" ];
|
subPackages = [ "cmd/bd" ];
|
||||||
doCheck = false;
|
doCheck = false;
|
||||||
# Go module dependencies hash (computed via nix build)
|
# Go module dependencies hash (computed via nix build)
|
||||||
vendorHash = "sha256-jpaeKw5dbZuhV9Z18aQ9tDMS/Eo7HaXiZefm26UlPyI=";
|
vendorHash = "sha256-oXPlcLVLoB3odBZzvS5FN8uL2Z9h8UMIbBKs/vZq03I=";
|
||||||
|
|
||||||
# Git is required for tests
|
# Git is required for tests
|
||||||
nativeBuildInputs = [ pkgs.git ];
|
nativeBuildInputs = [ pkgs.git ];
|
||||||
|
|||||||
Reference in New Issue
Block a user