johno/beads
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Improve bd-my64 fix based on oracle review

Browse Source 
Oracle identified a critical race condition in the initial fix:
- Pre-push hook checked for changes but didn't flush first
- Pending 5s-debounced flushes could land after the check
- Result: stale JSONL could still be pushed

Improvements:
1. Pre-push now flushes pending changes FIRST (bd sync --flush-only)
2. Uses git status --porcelain to catch ALL change types:
   - Staged, unstaged, untracked, deleted, renamed, conflicts
3. Handles both beads.jsonl and issues.jsonl (backward compat)
4. Works even without bd installed (git-only check)
5. Pre-commit stages both JSONL files (simpler loop)

This completely eliminates the race condition.
This commit is contained in:
Steve Yegge
2025-11-06 19:01:28 -08:00
parent ff1f25ea63
commit 0b0d9a43d1
4 changed files with 44 additions and 43 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
.beads/beads.jsonl
View File

@@ -243,7 +243,7 @@
{"id":"bd-lwnt","content_hash":"ddfa247870eb3734ffa7a4d0da6fcd4a359d2b48e02d70aad8560ec4bc13afdc","title":"Test P1 priority","description":"","status":"closed","priority":1,"issue_type":"task","created_at":"2025-11-05T12:58:38.074112-08:00","updated_at":"2025-11-05T12:58:44.711763-08:00","closed_at":"2025-11-05T12:58:44.711763-08:00","source_repo":"."}
{"id":"bd-mlcz","content_hash":"35434fed7e20e8b67f7b598c46ce5dc65c111de237ea38b36a88a671b023b58b","title":"Implement bd migrate command","description":"Add bd migrate command to move issues between repos with filtering. Should support: filtering by status/priority/labels, dry-run mode, preserving dependencies, handling source_repo field updates.","status":"closed","priority":1,"issue_type":"task","created_at":"2025-11-05T18:04:29.902151-08:00","updated_at":"2025-11-05T18:42:52.536951-08:00","closed_at":"2025-11-05T18:42:52.536951-08:00","source_repo":".","dependencies":[{"issue_id":"bd-mlcz","depends_on_id":"bd-8rd","type":"parent-child","created_at":"2025-11-05T18:04:39.072312-08:00","created_by":"daemon"}]}
{"id":"bd-mn9p","content_hash":"89752e08d5a278ed301c655753e07bc2b564e710ce6d9e7e535bbd9ec48af182","title":"bd-hv01: Brittle string comparison breaks with JSON field reordering","description":"## Problem\ndeletion_tracking.go:125 uses string comparison to detect unchanged issues:\n\n```go\nif leftLine, existsInLeft := leftIndex[id]; existsInLeft \u0026\u0026 leftLine == baseLine {\n deletions = append(deletions, id)\n}\n```\n\nThis breaks if:\n- JSON field order changes (legal in JSON)\n- Timestamps updated by import/export\n- Whitespace/formatting changes\n- Floating point precision varies\n\n## Example Failure\n```json\n// baseLine\n{\"id\":\"bd-1\",\"priority\":1,\"status\":\"open\"}\n// leftLine (same data, different order)\n{\"id\":\"bd-1\",\"status\":\"open\",\"priority\":1}\n```\nThese are semantically identical but string comparison fails.\n\n## Fix\nParse and compare JSON semantically:\n```go\nfunc jsonEquals(a, b string) bool {\n var objA, objB map[string]interface{}\n json.Unmarshal([]byte(a), \u0026objA)\n json.Unmarshal([]byte(b), \u0026objB)\n return reflect.DeepEqual(objA, objB)\n}\n```\n\n## Files Affected\n- cmd/bd/deletion_tracking.go:125\n- cmd/bd/deletion_tracking.go:134-170 (buildIDToLineMap)","status":"closed","priority":1,"issue_type":"bug","created_at":"2025-11-06T18:15:35.090716-08:00","updated_at":"2025-11-06T18:46:55.889888-08:00","closed_at":"2025-11-06T18:46:55.889888-08:00","source_repo":".","dependencies":[{"issue_id":"bd-mn9p","depends_on_id":"bd-rbxi","type":"parent-child","created_at":"2025-11-06T18:19:14.790898-08:00","created_by":"daemon"}]}
{"id":"bd-my64","content_hash":"5ce1e4594cd5eede3f8eb0dbc1c903d90fd46ae7805179688d9425472b711311","title":"Pre-push hook and daemon export produce different JSONL","description":"After committing and pushing, git status shows .beads/beads.jsonl as dirty. Investigation shows:\n\n1. Pre-push hook ran successfully and exported DB → JSONL\n2. Push completed\n3. Shortly after, daemon exported DB → JSONL again with different content\n4. Diff shows comments added to old issues (bd-23a8, bd-6049, bd-87a0)\n\nTimeline:\n- Commit c731c45 \"Update beads JSONL\"\n- Pre-push hook exported JSONL\n- Push succeeded\n- Daemon PID 33314 exported again with different content\n\nQuestions:\n1. Did someone run a command between commit and daemon export?\n2. Is there a timing issue where pre-push hook doesn't capture all DB changes?\n3. Should pre-commit hook flush daemon changes before committing?\n\nThe comments appear to be from Nov 5 (created_at: 2025-11-05T08:38:46Z) but are only appearing in JSONL now. This suggests the DB had these comments but they weren't exported during pre-push.\n\nPossible causes:\n- Pre-push hook uses BEADS_NO_DAEMON=1 which might skip pending writes\n- Daemon has unflushed changes in memory\n- Race condition between pre-push export and daemon's periodic export","status":"closed","priority":1,"issue_type":"bug","created_at":"2025-11-06T18:49:54.570993-08:00","updated_at":"2025-11-06T18:57:42.710282-08:00","closed_at":"2025-11-06T18:57:42.710282-08:00","source_repo":"."}
{"id":"bd-my64","content_hash":"8f4eb8056f81096e7090813f319b3aa996ada6dc5809d81305271d0584c2f364","title":"Pre-push hook and daemon export produce different JSONL","description":"After committing and pushing, git status shows .beads/beads.jsonl as dirty. Investigation shows:\n\n1. Pre-push hook ran successfully and exported DB → JSONL\n2. Push completed\n3. Shortly after, daemon exported DB → JSONL again with different content\n4. Diff shows comments added to old issues (bd-23a8, bd-6049, bd-87a0)\n\nTimeline:\n- Commit c731c45 \"Update beads JSONL\"\n- Pre-push hook exported JSONL\n- Push succeeded\n- Daemon PID 33314 exported again with different content\n\nQuestions:\n1. Did someone run a command between commit and daemon export?\n2. Is there a timing issue where pre-push hook doesn't capture all DB changes?\n3. Should pre-commit hook flush daemon changes before committing?\n\nThe comments appear to be from Nov 5 (created_at: 2025-11-05T08:38:46Z) but are only appearing in JSONL now. This suggests the DB had these comments but they weren't exported during pre-push.\n\nPossible causes:\n- Pre-push hook uses BEADS_NO_DAEMON=1 which might skip pending writes\n- Daemon has unflushed changes in memory\n- Race condition between pre-push export and daemon's periodic export","notes":"Improved fix based on oracle code review:\n1. Pre-push now flushes pending changes first (prevents debounce race)\n2. Uses git status --porcelain to catch all change types\n3. Handles both beads.jsonl and issues.jsonl\n4. Works even if bd not installed (git-only check)","status":"closed","priority":1,"issue_type":"bug","created_at":"2025-11-06T18:49:54.570993-08:00","updated_at":"2025-11-06T19:01:14.549032-08:00","closed_at":"2025-11-06T18:57:42.710282-08:00","source_repo":"."}
{"id":"bd-ng56","content_hash":"82c7d4d6f8a2fa73193de02e52de172f7325f38c02e4b9d397aec4833234ff5f","title":"bd-hv01: Three full JSONL reads on every sync (performance)","description":"Problem: computeAcceptedDeletions reads three JSONL files completely into memory (base, left, merged). For 1000 issues at 1KB each, this is 3MB read and 3000 JSON parse operations.\n\nImpact: Acceptable now (~20-35ms overhead) but will be slow for large repos (10k+ issues).\n\nPossible optimizations: single-pass streaming, memory-mapped files, binary format, incremental snapshots.\n\nFiles: cmd/bd/deletion_tracking.go:101-208","status":"open","priority":3,"issue_type":"task","created_at":"2025-11-06T18:16:25.653076-08:00","updated_at":"2025-11-06T18:16:25.653076-08:00","source_repo":".","dependencies":[{"issue_id":"bd-ng56","depends_on_id":"bd-rbxi","type":"parent-child","created_at":"2025-11-06T18:19:15.148149-08:00","created_by":"daemon"}]}
{"id":"bd-nqes","content_hash":"d7ca10b54b92c464a0c4c1c932f2918a6e0e952a5fd7337acb9160cd524dc59a","title":"bd-hv01: Non-atomic snapshot operations can cause data loss","description":"## Problem\nIn sync.go:146-155 and daemon_sync.go:502-505, snapshot capture failures are logged as warnings but sync continues:\n\n```go\nif err := exportToJSONL(ctx, jsonlPath); err != nil { ... }\nif err := captureLeftSnapshot(jsonlPath); err != nil {\n fmt.Fprintf(os.Stderr, \"Warning: failed to capture snapshot...\")\n}\n```\n\nIf export succeeds but snapshot capture fails, the merge uses stale snapshot data, potentially deleting wrong issues.\n\n## Impact\n- Critical data integrity issue\n- Could delete issues incorrectly during multi-workspace sync\n\n## Fix\nMake snapshot capture mandatory:\n```go\nif err := captureLeftSnapshot(jsonlPath); err != nil {\n return fmt.Errorf(\"failed to capture snapshot (required for deletion tracking): %w\", err)\n}\n```\n\n## Files Affected\n- cmd/bd/sync.go:146-155\n- cmd/bd/daemon_sync.go:502-505","status":"closed","priority":1,"issue_type":"bug","created_at":"2025-11-06T18:15:33.574158-08:00","updated_at":"2025-11-06T18:46:55.874814-08:00","closed_at":"2025-11-06T18:46:55.874814-08:00","source_repo":".","dependencies":[{"issue_id":"bd-nqes","depends_on_id":"bd-rbxi","type":"parent-child","created_at":"2025-11-06T18:19:14.749153-08:00","created_by":"daemon"}]}
{"id":"bd-o43","content_hash":"4caa0f14a58127378a533362ec0292833b6d59195e503fab7505180c9c5c0438","title":"Add richer query capabilities to bd list","description":"Current bd list filters are limited to basic field matching (status, priority, type, assignee, label). This forces users to resort to piping through jq for common queries.\n\nMissing query capabilities:\n- Pattern matching: --title-contains, --desc-contains\n- Date ranges: --created-after, --updated-before, --closed-after\n- Empty/null checks: --empty-description, --no-assignee, --no-labels\n- Numeric ranges: --priority-min, --priority-max\n- Complex boolean logic: --and, --or operators\n- Full-text search: --search across all text fields\n- Negation: --not-status, --exclude-label\n\nExample use cases:\n- Find issues with empty descriptions\n- Find stale issues not updated in 30 days\n- Find high-priority bugs with no assignee\n- Search for keyword across title/description/notes\n\nImplementation approach:\n- Add query builder pattern to storage layer\n- Support --query DSL for complex queries\n- Keep simple flags for common cases\n- Add --json output for programmatic use","notes":"## Progress Update\n\n**Completed:**\n- ✅ Extended IssueFilter struct with new fields (pattern matching, date ranges, empty/null checks, priority ranges)\n- ✅ Updated SQLite SearchIssues implementation \n- ✅ Added CLI flags to list.go\n- ✅ Added parseTimeFlag helper\n- ✅ Comprehensive tests added - all passing\n\n**Remaining:**\n- ⚠️ RPC layer needs updating (internal/rpc/protocol.go ListArgs)\n- ⚠️ Daemon handler needs to forward new filters\n- ⚠️ End-to-end testing with daemon mode\n- 📝 Documentation updates\n\n**Files Modified:**\n- internal/types/types.go\n- internal/storage/sqlite/sqlite.go \n- cmd/bd/list.go\n- cmd/bd/list_test.go\n\n**Next Steps:**\n1. Update RPC protocol\n2. Update daemon handler \n3. Test with daemon mode\n4. Update docs","status":"closed","priority":2,"issue_type":"feature","created_at":"2025-11-05T00:17:48.677493-08:00","updated_at":"2025-11-05T00:33:38.998433-08:00","closed_at":"2025-11-05T00:33:38.998433-08:00","source_repo":"."}

14
examples/git-hooks/README.md
View File

@@ -69,18 +69,20 @@ The hook is silent on success, fast (no git operations), and safe (fails commit
### pre-push
Before each push, the hook checks:
Before each push, the hook:
```bash
git diff --quiet .beads/beads.jsonl
bd sync --flush-only # Flush pending changes (if bd available)
git status --porcelain .beads/*.jsonl # Check for uncommitted changes
```
This prevents pushing stale JSONL by:
1. Checking if JSONL has uncommitted changes (working tree or staging)
2. Failing the push with clear error message if changes exist
3. Instructing user to commit JSONL before pushing again
1. Flushing pending in-memory changes from daemon's 5s debounce
2. Checking for uncommitted changes (staged, unstaged, untracked, deleted)
3. Failing the push with clear error message if changes exist
4. Instructing user to commit JSONL before pushing again
This solves bd-my64: changes made between commit and push are caught before reaching remote.
This solves bd-my64: changes made between commit and push (or pending debounced flushes) are caught before reaching remote.
### post-merge

17
examples/git-hooks/pre-commit
View File

@@ -35,17 +35,10 @@ if ! bd sync --flush-only >/dev/null 2>&1; then
exit 1
fi
# Find the JSONL file (could be issues.jsonl or beads.jsonl)
JSONL_FILE=""
if [ -f .beads/beads.jsonl ]; then
JSONL_FILE=".beads/beads.jsonl"
elif [ -f .beads/issues.jsonl ]; then
JSONL_FILE=".beads/issues.jsonl"
fi
# If the JSONL file was modified, stage it
if [ -n "$JSONL_FILE" ] && [ -f "$JSONL_FILE" ]; then
git add "$JSONL_FILE" 2>/dev/null || true
fi
# Stage both possible JSONL files (backward compatibility)
# git add is harmless if file doesn't exist
for f in .beads/beads.jsonl .beads/issues.jsonl; do
[ -f "$f" ] && git add "$f" 2>/dev/null || true
done
exit 0

54
examples/git-hooks/pre-push
View File

@@ -3,11 +3,14 @@
#
# bd (beads) pre-push hook
#
# This hook checks if the database has uncommitted changes and prevents
# pushing stale JSONL by failing with a clear error message.
# This hook prevents pushing stale JSONL by:
# 1. Flushing any pending in-memory changes to JSONL (if bd available)
# 2. Checking for uncommitted changes (staged, unstaged, untracked, deleted)
# 3. Failing the push with clear instructions if changes found
#
# The pre-commit hook should have already exported changes, but if new
# changes were made between commit and push, this hook catches that.
# The pre-commit hook already exports changes, but this catches:
# - Changes made between commit and push
# - Pending debounced flushes (5s daemon delay)
#
# Installation:
# cp examples/git-hooks/pre-push .git/hooks/pre-push
@@ -16,37 +19,40 @@
# Or use the install script:
# examples/git-hooks/install.sh
# Check if bd is available
if ! command -v bd >/dev/null 2>&1; then
# If bd is not available, we can't check - allow push
exit 0
fi
# Check if we're in a bd workspace
if [ ! -d .beads ]; then
# Not a bd workspace, nothing to do
exit 0
fi
# Find the JSONL file (could be issues.jsonl or beads.jsonl)
JSONL_FILE=""
if [ -f .beads/beads.jsonl ]; then
JSONL_FILE=".beads/beads.jsonl"
elif [ -f .beads/issues.jsonl ]; then
JSONL_FILE=".beads/issues.jsonl"
# Optionally flush pending bd changes so they surface in JSONL
# This prevents the race where a debounced flush lands after the check
if command -v bd >/dev/null 2>&1; then
bd sync --flush-only >/dev/null 2>&1 || true
fi
# Check if JSONL file has uncommitted changes
if [ -n "$JSONL_FILE" ] && [ -f "$JSONL_FILE" ]; then
# Check git status for the JSONL file
if ! git diff --quiet "$JSONL_FILE" 2>/dev/null || ! git diff --cached --quiet "$JSONL_FILE" 2>/dev/null; then
echo "❌ Error: $JSONL_FILE has uncommitted changes" >&2
# Collect all tracked or existing JSONL files (supports both old and new names)
FILES=""
for f in .beads/beads.jsonl .beads/issues.jsonl; do
# Include file if it exists in working tree OR is tracked by git (even if deleted)
if git ls-files --error-unmatch "$f" >/dev/null 2>&1 || [ -f "$f" ]; then
FILES="$FILES $f"
fi
done
# Check for any uncommitted changes using porcelain status
# This catches: staged, unstaged, untracked, deleted, renamed, and conflicts
if [ -n "$FILES" ]; then
# shellcheck disable=SC2086
if [ -n "$(git status --porcelain -- $FILES 2>/dev/null)" ]; then
echo "❌ Error: Beads JSONL has uncommitted changes" >&2
echo "" >&2
echo "You made changes to bd issues between your last commit and this push." >&2
echo "Please commit the updated JSONL file before pushing:" >&2
echo "Please commit the updated JSONL before pushing:" >&2
echo "" >&2
echo " git add $JSONL_FILE" >&2
echo " git commit -m \"Update beads JSONL\"" >&2
# shellcheck disable=SC2086
echo " git add $FILES" >&2
echo ' git commit -m "Update bd JSONL"' >&2
echo " git push" >&2
echo "" >&2
exit 1