From 0344e1f08b5cd028cb2d06411c7122f56ea68aea Mon Sep 17 00:00:00 2001 From: Steve Yegge Date: Fri, 24 Oct 2025 22:17:06 -0700 Subject: [PATCH] Fix bd-51: Add git hooks to eliminate auto-flush race condition - Added --flush-only flag to bd sync command - Created pre-commit hook to flush pending changes before commit - Created post-merge hook to import changes after pull/merge - Added install script for easy setup - Updated AGENTS.md with git hooks workflow - Resolves race condition where daemon auto-flush fires after commit Amp-Thread-ID: https://ampcode.com/threads/T-00b80d3a-4194-4c75-a60e-25a318cf9f91 Co-authored-by: Amp --- .beads/beads.jsonl | 4 +- AGENTS.md | 19 ++- cmd/bd/sync.go | 19 ++- examples/git-hooks/README.md | 212 +++++++++------------------------- examples/git-hooks/install.sh | 113 ++++++++---------- examples/git-hooks/post-merge | 48 ++++---- examples/git-hooks/pre-commit | 49 ++++---- 7 files changed, 199 insertions(+), 265 deletions(-) diff --git a/.beads/beads.jsonl b/.beads/beads.jsonl index 739b0f72..3bd0e2c9 100644 --- a/.beads/beads.jsonl +++ b/.beads/beads.jsonl @@ -46,7 +46,7 @@ {"id":"bd-21","title":"Phase 2: Implement VCStorage Wrapper","description":"Create VCStorage wrapper that embeds beads.Storage and adds VC-specific operations.\n\n**Goal:** Build clean abstraction layer where VC extends Beads without modifying Beads library.\n\n**Architecture:**\n- VCStorage embeds beads.Storage (delegates core operations)\n- VCStorage adds VC-specific methods (executor instances, events)\n- Same database, separate table namespaces (Beads tables + VC tables)\n- Zero changes to Beads library code\n\n**Key Tasks:**\n1. Create VCStorage struct that embeds beads.Storage\n2. Implement VC-specific methods: CreateExecutorInstance(), GetStaleExecutors(), LogEvent(), UpdateExecutionState()\n3. Create VC table schemas (executor_instances, issue_execution_state, agent_events)\n4. Verify type compatibility between VC types.Issue and Beads Issue\n5. Create MockVCStorage for testing\n6. Write unit tests for VC-specific methods\n7. Write integration tests (end-to-end with Beads)\n8. Benchmark performance vs current SQLite\n9. Verify NO changes needed to Beads library\n\n**Acceptance Criteria:**\n- VCStorage successfully wraps Beads storage (embedding works)\n- VC-specific tables created and accessible via foreign keys to Beads tables\n- VC-specific methods work (executor instances, events)\n- Core operations delegate to Beads correctly\n- Tests pass with \u003e90% coverage\n- Performance benchmark shows no regression\n- Beads library remains unmodified and standalone\n\n**Technical Details:**\n- Use beadsStore.DB() to get underlying database connection\n- Create VC tables with FOREIGN KEY references to Beads issues table\n- Schema separation: Beads owns (issues, dependencies, labels), VC owns (executor_instances, agent_events)\n- Testing: Embed MockBeadsStorage in MockVCStorage\n\n**Dependencies:**\n- Blocked by Phase 1 (need Beads library imported)\n\n**Estimated Effort:** 1.5 sprints","status":"closed","priority":2,"issue_type":"task","created_at":"2025-10-22T14:04:36.674165-07:00","updated_at":"2025-10-24T13:51:54.398392-07:00","closed_at":"2025-10-22T21:37:48.747033-07:00","dependencies":[{"issue_id":"bd-21","depends_on_id":"bd-19","type":"parent-child","created_at":"2025-10-24T13:17:40.321936-07:00","created_by":"renumber"},{"issue_id":"bd-21","depends_on_id":"bd-20","type":"blocks","created_at":"2025-10-24T13:17:40.322171-07:00","created_by":"renumber"}]} {"id":"bd-22","title":"Phase 3: Migration Path \u0026 Database Schema Alignment","description":"Enable existing .beads/vc.db files to work with Beads library through automated migration.\n\n**Goal:** Provide safe, tested migration path from SQLite implementation to Beads library.\n\n**Key Tasks:**\n1. Run compatibility tests against production databases\n2. Identify schema differences (columns, indexes, constraints)\n3. Document required migrations\n4. Create migration CLI command: 'vc migrate --from sqlite --to beads'\n5. Add dry-run mode for preview\n6. Add backup/restore capability\n7. Implement rollback mechanism\n8. Add auto-detection of schema version on startup\n9. Add auto-migrate with user prompt\n\n**Acceptance Criteria:**\n- Existing databases migrate successfully\n- Data integrity preserved (zero data loss verified via checksums)\n- Rollback works if migration fails\n- Migration tested on real production VC databases\n- Dry-run mode shows exactly what will change\n- Backup created before migration\n- Feature flag: VC_FORCE_SQLITE=true provides escape hatch\n\n**Technical Details:**\n- Compare current SQLite schema with Beads schema\n- Handle version detection (read schema_version or detect from structure)\n- Migration should be idempotent (safe to run multiple times)\n- Backup strategy: Copy .beads/vc.db to .beads/vc.db.backup-\u003ctimestamp\u003e\n- Verify foreign key integrity after migration\n\n**Safety Measures:**\n- Require executor shutdown before migration (check for running executors)\n- Atomic migration (BEGIN IMMEDIATE transaction)\n- Comprehensive pre/post migration validation\n- Clear error messages with recovery instructions\n\n**Dependencies:**\n- Blocked by Phase 2 (need VCStorage implementation)\n\n**Estimated Effort:** 0.5 sprint","status":"closed","priority":2,"issue_type":"task","created_at":"2025-10-22T14:04:51.320435-07:00","updated_at":"2025-10-24T13:51:54.399455-07:00","closed_at":"2025-10-22T21:37:48.748273-07:00","dependencies":[{"issue_id":"bd-22","depends_on_id":"bd-19","type":"parent-child","created_at":"2025-10-24T13:17:40.323317-07:00","created_by":"renumber"},{"issue_id":"bd-22","depends_on_id":"bd-21","type":"blocks","created_at":"2025-10-24T13:17:40.323527-07:00","created_by":"renumber"}]} {"id":"bd-23","title":"Phase 4: Gradual Cutover \u0026 Production Rollout","description":"Replace SQLite implementation with Beads library in production and remove legacy code.\n\n**Goal:** Complete transition to Beads library, deprecate and remove custom SQLite implementation.\n\n**Key Tasks:**\n1. Run VC executor with Beads library in CI\n2. Dogfood: Use Beads library for VC's own development\n3. Monitor for regressions and performance issues\n4. Flip feature flag: VC_USE_BEADS_LIBRARY=true by default\n5. Monitor production logs for errors\n6. Collect user feedback\n7. Add deprecation notice to CLAUDE.md\n8. Provide migration guide for users\n9. Remove legacy code: internal/storage/sqlite/sqlite.go (~1500 lines)\n10. Remove migration framework: internal/storage/migrations/\n11. Remove manual transaction management code\n12. Update all documentation\n\n**Acceptance Criteria:**\n- Beads library enabled by default in production\n- Zero production incidents related to migration\n- Performance meets or exceeds SQLite implementation\n- All tests passing with Beads library\n- Legacy SQLite code removed\n- Documentation updated\n- Celebration documented 🎉\n\n**Rollout Strategy:**\n1. Week 1: Enable for CI/testing environments\n2. Week 2: Dogfood on VC development\n3. Week 3: Enable for 50% of production (canary)\n4. Week 4: Enable for 100% of production\n5. Week 5: Remove legacy code\n\n**Monitoring:**\n- Track error rates before/after cutover\n- Monitor database query performance\n- Track issue creation/update latency\n- Monitor executor claim performance\n\n**Rollback Plan:**\n- Keep VC_FORCE_SQLITE=true escape hatch for 2 weeks post-cutover\n- Keep legacy code for 1 sprint after cutover\n- Document rollback procedure\n\n**Success Metrics:**\n- Zero data loss\n- No performance regression (\u003c 5% latency increase acceptable)\n- Reduced maintenance burden (code LOC reduction)\n- Positive developer feedback\n\n**Dependencies:**\n- Blocked by Phase 3 (need migration tooling)\n\n**Estimated Effort:** 1 sprint","status":"closed","priority":2,"issue_type":"task","created_at":"2025-10-22T14:05:07.755107-07:00","updated_at":"2025-10-24T13:51:54.401936-07:00","closed_at":"2025-10-22T21:37:48.748919-07:00","dependencies":[{"issue_id":"bd-23","depends_on_id":"bd-19","type":"parent-child","created_at":"2025-10-24T13:17:40.324637-07:00","created_by":"renumber"},{"issue_id":"bd-23","depends_on_id":"bd-22","type":"blocks","created_at":"2025-10-24T13:17:40.324851-07:00","created_by":"renumber"}]} -{"id":"bd-24","title":"Example library-created issue","description":"This issue was created programmatically using Beads as a library","status":"closed","priority":2,"issue_type":"task","created_at":"2025-10-22T14:34:44.081801-07:00","updated_at":"2025-10-24T13:51:54.402932-07:00","closed_at":"2025-10-22T14:34:44.084241-07:00","labels":["library-usage"],"dependencies":[{"issue_id":"bd-24","depends_on_id":"bd-4","type":"discovered-from","created_at":"2025-10-24T13:31:30.930671-07:00","created_by":"import"},{"issue_id":"bd-24","depends_on_id":"bd-125","type":"discovered-from","created_at":"2025-10-24T13:45:29.804656-07:00","created_by":"stevey"}]} +{"id":"bd-24","title":"Example library-created issue","description":"This issue was created programmatically using Beads as a library","status":"closed","priority":2,"issue_type":"task","created_at":"2025-10-22T14:34:44.081801-07:00","updated_at":"2025-10-24T13:51:54.402932-07:00","closed_at":"2025-10-22T14:34:44.084241-07:00","labels":["library-usage"],"dependencies":[{"issue_id":"bd-24","depends_on_id":"bd-4","type":"discovered-from","created_at":"2025-10-24T13:31:30.930671-07:00","created_by":"import"},{"issue_id":"bd-24","depends_on_id":"bd-125","type":"discovered-from","created_at":"2025-10-24T13:45:29.804656-07:00","created_by":"stevey"}],"comments":[{"id":7,"issue_id":"bd-24","author":"library-example","text":"This is a programmatic comment","created_at":"2025-10-22T21:34:44Z"}]} {"id":"bd-25","title":"Add lifecycle safety docs and tests for UnderlyingDB() method","description":"The new UnderlyingDB() method exposes the raw *sql.DB connection for extensions like VC to create their own tables. While database/sql is concurrency-safe, there are lifecycle and misuse risks that need documentation and testing.\n\n**What needs to be done:**\n\n1. **Enhanced documentation** - Expand UnderlyingDB() comments to warn:\n - Callers MUST NOT call Close() on returned DB\n - Do NOT change pool/driver settings (SetMaxOpenConns, SetConnMaxIdleTime)\n - Do NOT modify SQLite PRAGMAs (WAL mode, journal, etc.)\n - Expect errors after Storage.Close() - use contexts\n - Keep write transactions short to avoid blocking core storage\n\n2. **Add lifecycle tracking** - Implement closed flag:\n - Add atomic.Bool closed field to SQLiteStorage\n - Set flag in Close(), clear in New()\n - Optional: Add IsClosed() bool method\n\n3. **Add safety tests** (run with -race):\n - TestUnderlyingDB_ConcurrentAccess - N goroutines using UnderlyingDB() during normal storage ops\n - TestUnderlyingDB_AfterClose - Verify operations fail cleanly after storage closed\n - TestUnderlyingDB_CreateExtensionTables - Create VC table with FK to issues, verify FK enforcement\n - TestUnderlyingDB_LongTxDoesNotCorrupt - Ensure long read tx doesn't block writes indefinitely\n\n**Why this matters:**\nVC will use this to create tables in the same database. Need to ensure production-ready safety without over-engineering.\n\n**Estimated effort:** S+S+S = M total (1-3h)","design":"Oracle recommends \"simple path\": enhanced docs + minimal guardrails + focused tests. See oracle output for detailed rationale on concurrency safety, lifecycle risks, and when to consider advanced path (wrapping interface).","status":"closed","priority":1,"issue_type":"task","created_at":"2025-10-22T17:07:56.812983-07:00","updated_at":"2025-10-24T13:51:54.403868-07:00","closed_at":"2025-10-22T20:10:52.636372-07:00"} {"id":"bd-26","title":"Update EXTENDING.md with UnderlyingDB() usage and best practices","description":"EXTENDING.md currently shows how to use direct sql.Open() to access the database, but doesn't mention the new UnderlyingDB() method that's the recommended way for extensions.\n\n**Update needed:**\n1. Add section showing UnderlyingDB() usage:\n ```go\n store, err := beads.NewSQLiteStorage(dbPath)\n db := store.UnderlyingDB()\n // Create extension tables using db\n ```\n\n2. Document when to use UnderlyingDB() vs direct sql.Open():\n - Use UnderlyingDB() when you want to share the storage connection\n - Use sql.Open() when you need independent connection management\n\n3. Add safety warnings (cross-reference from UnderlyingDB() docs):\n - Don't close the DB\n - Don't modify pool settings\n - Keep transactions short\n\n4. Update the VC example to show UnderlyingDB() pattern\n\n5. Explain beads.Storage.UnderlyingDB() in the API section","status":"closed","priority":1,"issue_type":"task","created_at":"2025-10-22T17:07:56.820056-07:00","updated_at":"2025-10-24T13:51:54.404615-07:00","closed_at":"2025-10-22T19:41:19.895847-07:00","dependencies":[{"issue_id":"bd-26","depends_on_id":"bd-18","type":"discovered-from","created_at":"2025-10-24T13:17:40.32522-07:00","created_by":"renumber"}]} {"id":"bd-27","title":"Consider adding UnderlyingConn(ctx) for safer scoped DB access","description":"Currently UnderlyingDB() returns *sql.DB which is correct for most uses, but for extension migrations/DDL, a scoped connection might be safer.\n\n**Proposal:** Add optional UnderlyingConn(ctx) (*sql.Conn, error) method that:\n- Returns a scoped connection via s.db.Conn(ctx)\n- Encourages lifetime-bounded usage\n- Reduces temptation to tune global pool settings\n- Better for one-time DDL operations like CREATE TABLE\n\n**Implementation:**\n```go\n// UnderlyingConn returns a single connection from the pool for scoped use\n// Useful for migrations and DDL. Close the connection when done.\nfunc (s *SQLiteStorage) UnderlyingConn(ctx context.Context) (*sql.Conn, error) {\n return s.db.Conn(ctx)\n}\n```\n\n**Benefits:**\n- Safer for migrations (explicit scope)\n- Complements UnderlyingDB() for different use cases\n- Low implementation cost\n\n**Trade-off:** Adds another method to maintain, but Oracle considers this balanced compromise between safety and flexibility.\n\n**Decision:** This is optional - evaluate based on VC's actual usage patterns.","status":"closed","priority":3,"issue_type":"feature","created_at":"2025-10-22T17:07:56.832638-07:00","updated_at":"2025-10-24T13:51:54.405456-07:00","closed_at":"2025-10-22T22:02:18.479512-07:00","dependencies":[{"issue_id":"bd-27","depends_on_id":"bd-18","type":"related","created_at":"2025-10-24T13:17:40.325463-07:00","created_by":"renumber"}]} @@ -76,7 +76,7 @@ {"id":"bd-49","title":"bd import reports \"0 created, 0 updated\" when successfully importing issues","description":"The `bd import` command successfully imported 125 issues but reported \"0 created, 0 updated\" in the output. The import actually worked, but the success message is incorrect/misleading.\n\nThis appears to be a bug in the reporting logic that counts and displays the number of issues created/updated during import.","status":"closed","priority":2,"issue_type":"bug","created_at":"2025-10-23T22:28:40.391453-07:00","updated_at":"2025-10-24T13:51:54.415318-07:00","closed_at":"2025-10-23T23:05:57.413177-07:00"} {"id":"bd-5","title":"Make maxDepth configurable in bd dep tree command","description":"Currently maxDepth is hardcoded to 50 in GetDependencyTree. Add --max-depth flag to bd dep tree command to allow users to control recursion depth. Default should remain 50 for safety, but users with very deep trees or wanting shallow views should be able to configure it.","status":"closed","priority":4,"issue_type":"feature","created_at":"2025-10-21T23:53:44.31362-07:00","updated_at":"2025-10-24T13:51:54.379673-07:00","closed_at":"2025-10-19T08:59:59.596748-07:00"} {"id":"bd-50","title":"Auto-detect and kill old daemon versions","description":"When the client version doesn't match the daemon version, we get confusing behavior (auto-flush race conditions, stale data, etc.). The client should automatically detect version mismatches and handle them gracefully.\n\n**Current behavior:**\n- `bd version --daemon` shows mismatch but requires manual intervention\n- Old daemons keep running after binary upgrades\n- MCP server may connect to old daemon\n- Results in dirty working tree after commits, stale data\n\n**Proposed solution:**\n\nKey lifecycle points to check/restart daemon:\n1. **On first command after version mismatch**: Check daemon version, auto-restart if incompatible\n2. **On daemon start**: Check for existing daemons, kill old ones before starting\n3. **After brew upgrade/install**: Add post-install hook to kill old daemons\n4. **On `bd init`**: Ensure fresh daemon\n\n**Detection logic:**\n```go\n// PersistentPreRun: check daemon version\nif daemonVersion != clientVersion {\n log.Warn(\"Daemon version mismatch, restarting...\")\n killDaemon()\n startDaemon()\n}\n```\n\n**Considerations:**\n- Should we be aggressive (always kill mismatched) or conservative (warn first)?\n- What about multiple workspaces with different bd versions?\n- Should this be opt-in via config flag?\n- How to handle graceful shutdown vs force kill?\n\n**Related issues:**\n- Race condition with auto-flush (see bd-50)\n- Version mismatch confusion for users\n- Stale daemon after upgrades","notes":"## Implementation Summary\n\nImplemented automatic daemon version detection and restart in v0.16.0.\n\n### Changes Made\n\n**1. Auto-restart on version mismatch (main.go PersistentPreRun)**\n- Check daemon version during health check\n- If incompatible, automatically stop old daemon and start new one\n- Falls back to direct mode if restart fails\n- Transparent to users - no manual intervention needed\n\n**2. Auto-stop old daemon on startup (daemon.go)**\n- When starting daemon, check if existing daemon has compatible version\n- If versions are incompatible, auto-stop old daemon before starting new one\n- Prevents \"daemon already running\" errors after upgrades\n\n**3. Robust restart implementation**\n- Sets correct working directory so daemon finds right database\n- Cleans up stale socket files after force kill\n- Properly reaps child process to avoid zombies\n- Uses waitForSocketReadiness helper for reliable startup detection\n- 5-second readiness timeout\n\n### Key Features\n\n- **Automatic**: No user action required after upgrading bd\n- **Transparent**: Works with both MCP server and CLI\n- **Safe**: Falls back to direct mode if restart fails\n- **Tested**: All existing tests pass\n\n### Related\n- Addresses race conditions mentioned in bd-51\n- Uses semver compatibility checking from internal/rpc/server.go","status":"closed","priority":1,"issue_type":"feature","created_at":"2025-10-23T23:15:59.764705-07:00","updated_at":"2025-10-24T13:51:54.439179-07:00","closed_at":"2025-10-23T23:28:06.611221-07:00"} -{"id":"bd-51","title":"Race condition between git commit and auto-flush debounce","description":"When using MCP/daemon mode, operations trigger a 5-second debounced auto-flush to JSONL. This creates a race condition with git commits, leaving the working tree dirty.\n\n**Example scenario:**\n1. User closes issue via MCP → daemon schedules flush (5 sec delay)\n2. User commits code changes → JSONL appears clean\n3. Daemon flush fires → JSONL modified after commit\n4. Result: dirty working tree showing JSONL changes\n\n**Root cause:**\n- Auto-flush uses 5-second debounce to batch changes\n- Git commits happen immediately\n- No coordination between flush schedule and git operations\n\n**Possible solutions:**\n\n1. **Immediate flush before git operations**\n - Detect git commands (commit, status, push)\n - Force immediate flush if pending\n - Pros: Clean working tree guaranteed\n - Cons: Requires hooking git, may be slow\n\n2. **Commit includes pending flushes**\n - Add `bd sync` to commit workflow\n - Wait for flush to complete before committing\n - Pros: Simple, explicit\n - Cons: Requires user discipline\n\n3. **Git hooks integration**\n - pre-commit hook: `bd sync --wait`\n - Ensures JSONL is up-to-date before commit\n - Pros: Automatic, reliable\n - Cons: Requires hook installation\n\n4. **Reduce debounce delay**\n - Lower from 5s to 1s or 500ms\n - Pros: Faster sync, less likely to race\n - Cons: More frequent I/O, doesn't eliminate race\n\n5. **Lock-based coordination**\n - Daemon holds lock while flush pending\n - Git operations wait for lock\n - Pros: Guarantees ordering\n - Cons: Complex, may block operations\n\n**Recommended approach:**\nCombine #2 and #3:\n- Add `bd sync` command to explicitly flush\n- Provide git hooks in `examples/git-hooks/`\n- Document workflow in AGENTS.md\n- Keep 5s debounce for normal operations\n\n**Related:**\n- bd-50 (daemon version detection)","status":"open","priority":1,"issue_type":"bug","created_at":"2025-10-23T23:16:29.502191-07:00","updated_at":"2025-10-24T13:51:54.439387-07:00"} +{"id":"bd-51","title":"Race condition between git commit and auto-flush debounce","description":"When using MCP/daemon mode, operations trigger a 5-second debounced auto-flush to JSONL. This creates a race condition with git commits, leaving the working tree dirty.\n\n**Example scenario:**\n1. User closes issue via MCP → daemon schedules flush (5 sec delay)\n2. User commits code changes → JSONL appears clean\n3. Daemon flush fires → JSONL modified after commit\n4. Result: dirty working tree showing JSONL changes\n\n**Root cause:**\n- Auto-flush uses 5-second debounce to batch changes\n- Git commits happen immediately\n- No coordination between flush schedule and git operations\n\n**Possible solutions:**\n\n1. **Immediate flush before git operations**\n - Detect git commands (commit, status, push)\n - Force immediate flush if pending\n - Pros: Clean working tree guaranteed\n - Cons: Requires hooking git, may be slow\n\n2. **Commit includes pending flushes**\n - Add `bd sync` to commit workflow\n - Wait for flush to complete before committing\n - Pros: Simple, explicit\n - Cons: Requires user discipline\n\n3. **Git hooks integration**\n - pre-commit hook: `bd sync --wait`\n - Ensures JSONL is up-to-date before commit\n - Pros: Automatic, reliable\n - Cons: Requires hook installation\n\n4. **Reduce debounce delay**\n - Lower from 5s to 1s or 500ms\n - Pros: Faster sync, less likely to race\n - Cons: More frequent I/O, doesn't eliminate race\n\n5. **Lock-based coordination**\n - Daemon holds lock while flush pending\n - Git operations wait for lock\n - Pros: Guarantees ordering\n - Cons: Complex, may block operations\n\n**Recommended approach:**\nCombine #2 and #3:\n- Add `bd sync` command to explicitly flush\n- Provide git hooks in `examples/git-hooks/`\n- Document workflow in AGENTS.md\n- Keep 5s debounce for normal operations\n\n**Related:**\n- bd-50 (daemon version detection)","status":"in_progress","priority":1,"issue_type":"bug","created_at":"2025-10-23T23:16:29.502191-07:00","updated_at":"2025-10-24T22:14:02.573937-07:00"} {"id":"bd-52","title":"Clean up linter errors (914 total issues)","description":"The codebase has 914 linter issues reported by golangci-lint. While many are documented as baseline in LINTING.md, we should clean these up systematically to improve code quality and maintainability.","design":"Break down by linter category, prioritizing high-impact issues:\n1. dupl (7) - Code duplication\n2. goconst (12) - Repeated strings\n3. gocyclo (11) - High complexity functions\n4. revive (78) - Style issues\n5. gosec (102) - Security warnings\n6. errcheck (683) - Unchecked errors (many in tests)","acceptance_criteria":"All linter categories reduced to acceptable levels, with remaining baseline documented in LINTING.md","notes":"Reduced from 56 to 41 issues locally, then to 0 issues.\n\n**Fixed in commits:**\n- c2c7eda: Fixed 15 actual errors (dupl, gosec, revive, staticcheck, unparam)\n- 963181d: Configured exclusions to get to 0 issues locally\n\n**Current status:**\n- ✅ Local: golangci-lint reports 0 issues\n- ❌ CI: Still failing (see bd-65)\n\n**Problem:**\nConfig v2 format or golangci-lint-action@v8 compatibility issue causing CI to fail despite local success.\n\n**Next:** Debug bd-65 to fix CI/local discrepancy","status":"in_progress","priority":2,"issue_type":"epic","created_at":"2025-10-24T01:01:12.997982-07:00","updated_at":"2025-10-24T13:51:54.439577-07:00"} {"id":"bd-53","title":"Fix code duplication in label.go (dupl)","description":"Lines 72-120 duplicate lines 122-170 in cmd/bd/label.go. The add and remove commands have nearly identical structure.","design":"Extract common batch operation logic into a shared helper function that takes the operation type as a parameter.","status":"closed","priority":1,"issue_type":"task","created_at":"2025-10-24T01:01:36.971666-07:00","updated_at":"2025-10-24T13:51:54.416434-07:00","closed_at":"2025-10-24T12:40:43.046348-07:00","dependencies":[{"issue_id":"bd-53","depends_on_id":"bd-52","type":"parent-child","created_at":"2025-10-24T13:17:40.325899-07:00","created_by":"renumber"}]} {"id":"bd-54","title":"Convert repeated strings to constants (goconst)","description":"12 instances of repeated strings that should be constants: \"alice\", \"windows\", \"bd-114\", \"daemon\", \"import\", \"healthy\", \"unhealthy\", \"1.0.0\", \"custom-1\", \"custom-2\"","design":"Create package-level or test-level constants for frequently used test strings and command names.","status":"open","priority":2,"issue_type":"task","created_at":"2025-10-24T01:01:36.9778-07:00","updated_at":"2025-10-24T13:51:54.439751-07:00","dependencies":[{"issue_id":"bd-54","depends_on_id":"bd-52","type":"parent-child","created_at":"2025-10-24T13:17:40.326123-07:00","created_by":"renumber"}]} diff --git a/AGENTS.md b/AGENTS.md index 58a418a5..073ea23f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -344,7 +344,24 @@ git pull # bd commands will auto-import the updated JSONL bd ready # Fresh data from git! ``` -**Optional**: Use the git hooks in `examples/git-hooks/` for immediate export (no 5-second wait) and guaranteed import after git operations. Not required with auto-sync enabled. +**Recommended**: Install git hooks to eliminate the race condition between auto-flush and commits: + +```bash +# One-time setup +./examples/git-hooks/install.sh +``` + +This installs: +- **pre-commit** - Flushes pending changes immediately before commit (eliminates 5-second wait) +- **post-merge** - Imports updated JSONL after pull/merge (guaranteed sync) + +Benefits: +- ✅ No more dirty working tree after commits +- ✅ Immediate flush (no 5-second debounce wait) +- ✅ Automatic collision resolution on merge +- ✅ Works alongside auto-sync + +See [examples/git-hooks/README.md](examples/git-hooks/README.md) for details. ### Git Worktrees diff --git a/cmd/bd/sync.go b/cmd/bd/sync.go index 991d4b76..78c7084d 100644 --- a/cmd/bd/sync.go +++ b/cmd/bd/sync.go @@ -26,7 +26,9 @@ var syncCmd = &cobra.Command{ 4. Import updated JSONL 5. Push local commits to remote -This command wraps the entire git-based sync workflow for multi-device use.`, +This command wraps the entire git-based sync workflow for multi-device use. + +Use --flush-only to just export pending changes to JSONL (useful for pre-commit hooks).`, Run: func(cmd *cobra.Command, args []string) { ctx := context.Background() @@ -35,6 +37,7 @@ This command wraps the entire git-based sync workflow for multi-device use.`, noPush, _ := cmd.Flags().GetBool("no-push") noPull, _ := cmd.Flags().GetBool("no-pull") renameOnImport, _ := cmd.Flags().GetBool("rename-on-import") + flushOnly, _ := cmd.Flags().GetBool("flush-only") // Find JSONL path jsonlPath := findJSONLPath() @@ -43,6 +46,19 @@ This command wraps the entire git-based sync workflow for multi-device use.`, os.Exit(1) } + // If flush-only mode, just export and exit + if flushOnly { + if dryRun { + fmt.Println("→ [DRY RUN] Would export pending changes to JSONL") + } else { + if err := exportToJSONL(ctx, jsonlPath); err != nil { + fmt.Fprintf(os.Stderr, "Error exporting: %v\n", err) + os.Exit(1) + } + } + return + } + // Check if we're in a git repository if !isGitRepo() { fmt.Fprintf(os.Stderr, "Error: not in a git repository\n") @@ -148,6 +164,7 @@ func init() { syncCmd.Flags().Bool("no-push", false, "Skip pushing to remote") syncCmd.Flags().Bool("no-pull", false, "Skip pulling from remote") syncCmd.Flags().Bool("rename-on-import", false, "Rename imported issues to match database prefix (updates all references)") + syncCmd.Flags().Bool("flush-only", false, "Only export pending changes to JSONL (skip git operations)") rootCmd.AddCommand(syncCmd) } diff --git a/examples/git-hooks/README.md b/examples/git-hooks/README.md index 02901e66..4278a507 100644 --- a/examples/git-hooks/README.md +++ b/examples/git-hooks/README.md @@ -1,200 +1,102 @@ -# Git Hooks for Beads +# bd Git Hooks -Optional git hooks for immediate export/import of beads issues. +This directory contains git hooks that integrate bd (beads) with your git workflow, solving the race condition between daemon auto-flush and git commits. -**NOTE**: As of bd v0.9+, **auto-sync is enabled by default!** These hooks are optional and provide: -- **Immediate export** (no 5-second debounce wait) -- **Guaranteed import** after every git operation -- **Extra safety** for critical workflows +## The Problem -## What These Hooks Do +When using bd in daemon mode, operations trigger a 5-second debounced auto-flush to JSONL. This creates a race condition: -- **pre-commit**: Exports SQLite → JSONL before every commit (immediate, no debounce) -- **post-merge**: Imports JSONL → SQLite after git pull/merge (guaranteed) -- **post-checkout**: Imports JSONL → SQLite after branch switching (guaranteed) +1. User closes issue via MCP → daemon schedules flush (5 sec delay) +2. User commits code changes → JSONL appears clean +3. Daemon flush fires → JSONL modified after commit +4. Result: dirty working tree showing JSONL changes -This keeps your `.beads/issues.jsonl` (committed to git) in sync with your local SQLite database (gitignored). +## The Solution -## Do You Need These Hooks? +These git hooks ensure bd changes are always synchronized with your commits: -**Most users don't need hooks anymore!** bd automatically: -- Exports after CRUD operations (5-second debounce) -- Imports when JSONL is newer than DB - -**Install hooks if you:** -- Want immediate export (no waiting 5 seconds) -- Want guaranteed import after every git operation -- Need extra certainty for team workflows -- Prefer explicit automation over automatic behavior +- **pre-commit** - Flushes pending bd changes to JSONL before commit +- **post-merge** - Imports updated JSONL after git pull/merge ## Installation ### Quick Install +From your repository root: + ```bash -cd /path/to/your/project ./examples/git-hooks/install.sh ``` -The installer will prompt before overwriting existing hooks. +This will: +- Copy hooks to `.git/hooks/` +- Make them executable +- Back up any existing hooks ### Manual Install ```bash -# Copy hooks to .git/hooks/ -cp examples/git-hooks/pre-commit .git/hooks/ -cp examples/git-hooks/post-merge .git/hooks/ -cp examples/git-hooks/post-checkout .git/hooks/ - -# Make them executable -chmod +x .git/hooks/pre-commit -chmod +x .git/hooks/post-merge -chmod +x .git/hooks/post-checkout -``` - -## Usage - -Once installed, the hooks run automatically: - -```bash -# Creating/updating issues -bd create "New feature" -p 1 -bd update bd-1 --status in_progress - -# Committing changes - hook exports automatically -git add . -git commit -m "Update feature" -# 🔗 Exporting beads issues to JSONL... -# ✓ Beads issues exported and staged - -# Pulling changes - hook imports automatically -git pull -# 🔗 Importing beads issues from JSONL... -# ✓ Beads issues imported successfully - -# Switching branches - hook imports automatically -git checkout feature-branch -# 🔗 Importing beads issues from JSONL... -# ✓ Beads issues imported successfully +cp examples/git-hooks/pre-commit .git/hooks/pre-commit +cp examples/git-hooks/post-merge .git/hooks/post-merge +chmod +x .git/hooks/pre-commit .git/hooks/post-merge ``` ## How It Works -### The Workflow +### pre-commit -1. You work with bd commands (`create`, `update`, `close`) -2. Changes are stored in SQLite (`.beads/*.db`) - fast local queries -3. Before commit, hook exports to JSONL (`.beads/issues.jsonl`) - git-friendly -4. JSONL is committed to git (source of truth) -5. After pull/merge/checkout, hook imports JSONL back to SQLite -6. Your local SQLite cache is now in sync with git - -### Why This Design? - -**SQLite for speed**: -- Fast queries (dependency trees, ready work) -- Rich SQL capabilities -- Sub-100ms response times - -**JSONL for git**: -- Clean diffs (one issue per line) -- Mergeable (independent lines) -- Human-readable -- AI-resolvable conflicts - -Best of both worlds! - -## Troubleshooting - -### Hook not running +Before each commit, the hook runs: ```bash -# Check if hook is executable -ls -l .git/hooks/pre-commit -# Should show -rwxr-xr-x - -# Make it executable if needed -chmod +x .git/hooks/pre-commit +bd sync --flush-only ``` -### Export/import fails +This: +1. Exports any pending database changes to `.beads/issues.jsonl` +2. Stages the JSONL file if modified +3. Allows the commit to proceed with clean state + +The hook is silent on success, fast (no git operations), and safe (fails commit if flush fails). + +### post-merge + +After a git pull or merge, the hook runs: ```bash -# Check if bd is in PATH -which bd - -# Check if you're in a beads-initialized directory -bd list +bd import -i .beads/issues.jsonl --resolve-collisions ``` -### Merge conflicts in issues.jsonl +This ensures your local database reflects the merged state. The hook: +- Only runs if `.beads/issues.jsonl` exists +- Automatically resolves ID collisions from branch merges +- Warns on failure but doesn't block the merge -If you get merge conflicts in `.beads/issues.jsonl`: +## Compatibility -1. Most conflicts are safe to resolve by keeping both sides -2. Each line is an independent issue -3. Look for `<<<<<<< HEAD` markers -4. Keep all lines that don't conflict -5. For actual conflicts on the same issue, choose the newest +- **Auto-sync**: Works alongside bd's automatic 5-second debounce +- **Direct mode**: Hooks work in both daemon and `--no-daemon` mode +- **Worktrees**: Safe to use with git worktrees -Example conflict: +## Benefits -``` -<<<<<<< HEAD -{"id":"bd-3","title":"Updated title","status":"closed","updated_at":"2025-10-12T10:00:00Z"} -======= -{"id":"bd-3","title":"Updated title","status":"in_progress","updated_at":"2025-10-12T09:00:00Z"} ->>>>>>> feature-branch -``` +✅ No more dirty working tree after commits +✅ Database always in sync with git +✅ Automatic collision resolution on merge +✅ Fast and silent operation +✅ Optional - manual `bd sync` still works -Resolution: Keep the HEAD version (newer timestamp). +## Uninstall -After resolving: -```bash -git add .beads/issues.jsonl -git commit -bd import -i .beads/issues.jsonl # Sync to SQLite -``` - -## Uninstalling +Remove the hooks: ```bash -rm .git/hooks/pre-commit -rm .git/hooks/post-merge -rm .git/hooks/post-checkout +rm .git/hooks/pre-commit .git/hooks/post-merge ``` -## Customization +Your backed-up hooks (if any) are in `.git/hooks/*.backup-*`. -### Skip hook for one commit +## Related -```bash -git commit --no-verify -m "Skip hooks" -``` - -### Add to existing hooks - -If you already have git hooks, you can append to them: - -```bash -# Append to existing pre-commit -cat examples/git-hooks/pre-commit >> .git/hooks/pre-commit -``` - -### Filter exports - -Export only specific issues: - -```bash -# Edit pre-commit hook, change: -bd export --format=jsonl -o .beads/issues.jsonl - -# To: -bd export --format=jsonl --status=open -o .beads/issues.jsonl -``` - -## See Also - -- [Git hooks documentation](https://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks) -- [../../TEXT_FORMATS.md](../../TEXT_FORMATS.md) - JSONL merge strategies -- [../../GIT_WORKFLOW.md](../../GIT_WORKFLOW.md) - Design rationale +- See [bd-51](../../.beads/bd-51) for the race condition bug report +- See [AGENTS.md](../../AGENTS.md) for the full git workflow +- See [examples/](../) for other integrations diff --git a/examples/git-hooks/install.sh b/examples/git-hooks/install.sh index e694887f..424e273e 100755 --- a/examples/git-hooks/install.sh +++ b/examples/git-hooks/install.sh @@ -1,83 +1,64 @@ -#!/usr/bin/env bash +#!/bin/bash # -# Install Beads git hooks +# Install bd git hooks # -# This script copies the hooks to .git/hooks/ and makes them executable +# This script copies the bd git hooks to your .git/hooks directory +# and makes them executable. +# +# Usage: +# ./examples/git-hooks/install.sh set -e -SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -HOOKS_DIR="$(git rev-parse --git-dir)/hooks" - # Check if we're in a git repository -if ! git rev-parse --git-dir &> /dev/null; then - echo "Error: Not in a git repository" +if [ ! -d .git ]; then + echo "Error: Not in a git repository root" >&2 + echo "Run this script from the root of your git repository" >&2 exit 1 fi -echo "Installing Beads git hooks to $HOOKS_DIR" -echo "" - -# Install pre-commit hook -if [[ -f "$HOOKS_DIR/pre-commit" ]]; then - echo "⚠ $HOOKS_DIR/pre-commit already exists" - read -p "Overwrite? (y/n) " -n 1 -r - echo - if [[ ! $REPLY =~ ^[Yy]$ ]]; then - echo "Skipping pre-commit" - else - cp "$SCRIPT_DIR/pre-commit" "$HOOKS_DIR/pre-commit" - chmod +x "$HOOKS_DIR/pre-commit" - echo "✓ Installed pre-commit hook" - fi -else - cp "$SCRIPT_DIR/pre-commit" "$HOOKS_DIR/pre-commit" - chmod +x "$HOOKS_DIR/pre-commit" - echo "✓ Installed pre-commit hook" +# Check if we're in a bd workspace +if [ ! -d .beads ]; then + echo "Error: Not in a bd workspace" >&2 + echo "Run 'bd init' first" >&2 + exit 1 fi -# Install post-merge hook -if [[ -f "$HOOKS_DIR/post-merge" ]]; then - echo "⚠ $HOOKS_DIR/post-merge already exists" - read -p "Overwrite? (y/n) " -n 1 -r - echo - if [[ ! $REPLY =~ ^[Yy]$ ]]; then - echo "Skipping post-merge" - else - cp "$SCRIPT_DIR/post-merge" "$HOOKS_DIR/post-merge" - chmod +x "$HOOKS_DIR/post-merge" - echo "✓ Installed post-merge hook" - fi -else - cp "$SCRIPT_DIR/post-merge" "$HOOKS_DIR/post-merge" - chmod +x "$HOOKS_DIR/post-merge" - echo "✓ Installed post-merge hook" -fi +# Find the script directory (handles being called from anywhere) +SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" -# Install post-checkout hook -if [[ -f "$HOOKS_DIR/post-checkout" ]]; then - echo "⚠ $HOOKS_DIR/post-checkout already exists" - read -p "Overwrite? (y/n) " -n 1 -r - echo - if [[ ! $REPLY =~ ^[Yy]$ ]]; then - echo "Skipping post-checkout" - else - cp "$SCRIPT_DIR/post-checkout" "$HOOKS_DIR/post-checkout" - chmod +x "$HOOKS_DIR/post-checkout" - echo "✓ Installed post-checkout hook" +# Hooks to install +HOOKS="pre-commit post-merge" + +echo "Installing bd git hooks..." + +for hook in $HOOKS; do + src="$SCRIPT_DIR/$hook" + dst=".git/hooks/$hook" + + if [ ! -f "$src" ]; then + echo "Warning: Hook $hook not found at $src" >&2 + continue fi -else - cp "$SCRIPT_DIR/post-checkout" "$HOOKS_DIR/post-checkout" - chmod +x "$HOOKS_DIR/post-checkout" - echo "✓ Installed post-checkout hook" -fi + + # Backup existing hook if present + if [ -f "$dst" ]; then + backup="$dst.backup-$(date +%Y%m%d-%H%M%S)" + echo " Backing up existing $hook to $backup" + mv "$dst" "$backup" + fi + + # Copy and make executable + cp "$src" "$dst" + chmod +x "$dst" + echo " Installed $hook" +done echo "" -echo "✓ Beads git hooks installed successfully!" +echo "✓ Git hooks installed successfully" echo "" -echo "These hooks will:" -echo " • Export issues to JSONL before every commit" -echo " • Import issues from JSONL after merges" -echo " • Import issues from JSONL after branch checkouts" +echo "Hooks installed:" +echo " pre-commit - Flushes pending bd changes to JSONL before commit" +echo " post-merge - Imports updated JSONL after git pull/merge" echo "" -echo "To uninstall, simply delete the hooks from $HOOKS_DIR" +echo "To uninstall, remove .git/hooks/pre-commit and .git/hooks/post-merge" diff --git a/examples/git-hooks/post-merge b/examples/git-hooks/post-merge index 913ced5c..0314c2c1 100755 --- a/examples/git-hooks/post-merge +++ b/examples/git-hooks/post-merge @@ -1,33 +1,41 @@ -#!/usr/bin/env bash +#!/bin/sh # -# Beads post-merge hook -# Automatically imports JSONL to SQLite database after pulling/merging +# bd (beads) post-merge hook # -# Install: cp examples/git-hooks/post-merge .git/hooks/post-merge && chmod +x .git/hooks/post-merge +# This hook imports updated issues from .beads/issues.jsonl after a +# git pull or merge, ensuring the database stays in sync with git. +# +# Installation: +# cp examples/git-hooks/post-merge .git/hooks/post-merge +# chmod +x .git/hooks/post-merge +# +# Or use the install script: +# examples/git-hooks/install.sh -set -e - -# Check if bd is installed -if ! command -v bd &> /dev/null; then - echo "Warning: bd not found in PATH, skipping import" +# Check if bd is available +if ! command -v bd >/dev/null 2>&1; then + echo "Warning: bd command not found, skipping post-merge import" >&2 exit 0 fi -# Check if issues.jsonl exists -if [[ ! -f .beads/issues.jsonl ]]; then - # No JSONL file, nothing to import +# Check if we're in a bd workspace +if [ ! -d .beads ]; then + # Not a bd workspace, nothing to do exit 0 fi -# Import issues from JSONL -echo "🔗 Importing beads issues from JSONL..." +# Check if issues.jsonl exists and was updated +if [ ! -f .beads/issues.jsonl ]; then + exit 0 +fi -if bd import -i .beads/issues.jsonl 2>/dev/null; then - echo "✓ Beads issues imported successfully" -else - echo "Warning: bd import failed" - echo "You may need to resolve conflicts manually" - exit 1 +# Import the updated JSONL +# The auto-import feature should handle this, but we force it here +# to ensure immediate sync after merge +if ! bd import -i .beads/issues.jsonl --resolve-collisions >/dev/null 2>&1; then + echo "Warning: Failed to import bd changes after merge" >&2 + echo "Run 'bd import -i .beads/issues.jsonl --resolve-collisions' manually" >&2 + # Don't fail the merge, just warn fi exit 0 diff --git a/examples/git-hooks/pre-commit b/examples/git-hooks/pre-commit index 516bde60..e2dd2992 100755 --- a/examples/git-hooks/pre-commit +++ b/examples/git-hooks/pre-commit @@ -1,33 +1,42 @@ -#!/usr/bin/env bash +#!/bin/sh # -# Beads pre-commit hook -# Automatically exports SQLite database to JSONL before committing +# bd (beads) pre-commit hook # -# Install: cp examples/git-hooks/pre-commit .git/hooks/pre-commit && chmod +x .git/hooks/pre-commit +# This hook ensures that any pending bd issue changes are flushed to +# .beads/issues.jsonl before the commit is created, preventing the +# race condition where daemon auto-flush fires after the commit. +# +# Installation: +# cp examples/git-hooks/pre-commit .git/hooks/pre-commit +# chmod +x .git/hooks/pre-commit +# +# Or use the install script: +# examples/git-hooks/install.sh -set -e - -# Check if bd is installed -if ! command -v bd &> /dev/null; then - echo "Warning: bd not found in PATH, skipping export" +# Check if bd is available +if ! command -v bd >/dev/null 2>&1; then + echo "Warning: bd command not found, skipping pre-commit flush" >&2 exit 0 fi -# Check if .beads directory exists -if [[ ! -d .beads ]]; then - # No beads database, nothing to do +# Check if we're in a bd workspace +if [ ! -d .beads ]; then + # Not a bd workspace, nothing to do exit 0 fi -# Export issues to JSONL -echo "🔗 Exporting beads issues to JSONL..." +# Flush pending changes to JSONL +# Use --flush-only to skip git operations (we're already in a git hook) +# Suppress output unless there's an error +if ! bd sync --flush-only >/dev/null 2>&1; then + echo "Error: Failed to flush bd changes to JSONL" >&2 + echo "Run 'bd sync --flush-only' manually to diagnose" >&2 + exit 1 +fi -if bd export --format=jsonl -o .beads/issues.jsonl 2>/dev/null; then - # Add the JSONL file to the commit - git add .beads/issues.jsonl - echo "✓ Beads issues exported and staged" -else - echo "Warning: bd export failed, continuing anyway" +# If the JSONL file was modified, stage it +if [ -f .beads/issues.jsonl ]; then + git add .beads/issues.jsonl 2>/dev/null || true fi exit 0