bd sync: 2025-10-26 21:42:08

This commit is contained in:
Steve Yegge
2025-10-26 21:42:08 -07:00
parent cabe5e8d9b
commit 669c054332

View File

@@ -72,7 +72,7 @@
{"id":"bd-163","title":"Add .beads/config.json for database path configuration","description":"Create config file to eliminate ambiguity about which database is active.\n\nConfig file format (.beads/config.json):\n{\n \"database\": \"beads.db\",\n \"version\": \"0.17.5\",\n \"jsonl_export\": \"beads.jsonl\" // Allow user to rename\n}\n\nImplementation:\n- bd init creates config.json\n- Daemon and clients read config first (single source of truth)\n- Fall back to beads.db if config missing (backward compat)\n- bd init --jsonl-name allows customizing export filename\n- Gitignore: do NOT ignore config.json (part of repo state)\n\nBenefits:\n- Explicit configuration over convention\n- Allows JSONL renaming for git history hygiene\n- Single source of truth for file paths","status":"closed","priority":2,"issue_type":"task","created_at":"2025-10-26T18:06:07.571909-07:00","updated_at":"2025-10-26T18:44:16.133085-07:00","closed_at":"2025-10-26T18:44:16.133085-07:00","dependencies":[{"issue_id":"bd-163","depends_on_id":"bd-159","type":"parent-child","created_at":"2025-10-26T18:06:07.572636-07:00","created_by":"daemon"}]}
{"id":"bd-164","title":"Add migration tooling for database upgrades","description":"Create bd migrate command and auto-migration logic for version upgrades.\n\nImplementation:\n- bd migrate command (or bd init --migrate)\n- Auto-run on first command after daemon version upgrade\n- Detection logic:\n - Find all .db files in .beads/\n - Check schema version in each\n - Prompt to migrate/rename/delete\n- Migration operations:\n - Rename old database to beads.db\n - Update schema version metadata\n - Remove stale databases (with confirmation)\n- Could be part of daemon auto-start logic\n\nUser experience:\n$ bd ready\nDatabase schema mismatch detected.\n Found: vc.db (schema v0.16.0)\n Expected: beads.db (schema v0.17.5)\n \nRun 'bd migrate' to migrate automatically.\n\nBenefits:\n- Smooth upgrade path\n- Prevents confusion on version changes\n- Clean up stale databases\n\nDepends on:\n- Canonical naming (bd-160)\n- Schema versioning (bd-161)","status":"closed","priority":2,"issue_type":"task","created_at":"2025-10-26T18:06:07.571855-07:00","updated_at":"2025-10-26T19:04:02.023089-07:00","closed_at":"2025-10-26T19:04:02.023089-07:00","dependencies":[{"issue_id":"bd-164","depends_on_id":"bd-159","type":"parent-child","created_at":"2025-10-26T18:06:07.573546-07:00","created_by":"daemon"},{"issue_id":"bd-164","depends_on_id":"bd-162","type":"blocks","created_at":"2025-10-26T18:06:17.327717-07:00","created_by":"daemon"},{"issue_id":"bd-164","depends_on_id":"bd-160","type":"blocks","created_at":"2025-10-26T18:06:17.351768-07:00","created_by":"daemon"}]}
{"id":"bd-165","title":"Enforce canonical database name (beads.db)","description":"Always use beads.db as the canonical database name. Never auto-detect from multiple .db files.\n\nImplementation:\n- bd init always creates/uses beads.db\n- bd init detects and migrates old databases (vc.db → beads.db, bd.db → beads.db)\n- Daemon refuses to start if multiple .db files exist in .beads/ (exit with ambiguity error)\n- Update database discovery logic to prefer beads.db, error on ambiguity\n\nBenefits:\n- Prevents accidental use of stale databases\n- Clear single source of truth\n- Migration path for existing users","status":"closed","priority":1,"issue_type":"task","created_at":"2025-10-26T18:06:18.33827-07:00","updated_at":"2025-10-26T18:10:34.132537-07:00","closed_at":"2025-10-26T18:10:34.132537-07:00","dependencies":[{"issue_id":"bd-165","depends_on_id":"bd-159","type":"parent-child","created_at":"2025-10-26T18:06:18.339465-07:00","created_by":"daemon"}]}
{"id":"bd-166","title":"bd import/sync created 173 duplicate issues with wrong prefix","description":"## What Happened\nDuring corruption recovery investigation (beads-173), discovered the database contained 338 issues instead of expected 165:\n- 165 issues with correct `bd-` prefix \n- 173 duplicate issues with `beads-` prefix (bd-1 → beads-1, etc.)\n- Database config was set to `beads` prefix instead of `bd`\n\n## Root Cause\nSome bd operation (likely import or sync) created duplicate issues with the wrong prefix. The database should have rejected or warned about prefix mismatch, but instead:\n1. Silently created duplicates with wrong prefix\n2. Changed database prefix config from `bd` to `beads`\n\n## Impact\n- **Data integrity violation**: Duplicate issues with different IDs\n- **Silent corruption**: No error or warning during creation \n- **Wrong prefix**: Database config changed without user consent\n- **Confusion**: Users see double the issues, dependencies broken\n\n## Recovery \nHad to manually fix the `issue_prefix` config key (not `prefix` as initially thought):\n```bash\nsqlite3 .beads/beads.db \"UPDATE config SET value = 'bd' WHERE key = 'issue_prefix';\"\nsqlite3 .beads/beads.db \"DELETE FROM issues WHERE id LIKE 'beads-%';\"\n```\n\n## What Should Happen\n1. **Reject prefix mismatch**: If importing issues with different prefix than configured, error or require `--rename-on-import`\n2. **Never auto-change prefix**: Database prefix should only change via explicit `bd rename-prefix` command \n3. **Validate on import**: Check imported issue IDs match configured prefix before creating\n4. **Warn on duplicate IDs**: Even with different prefixes, detect potential duplicates\n\n## Related\n- Discovered during beads-173 (database corruption investigation)\n- Similar to existing prefix validation in `bd sync` (bd-21)","status":"open","priority":0,"issue_type":"bug","created_at":"2025-10-26T21:38:39.096165-07:00","updated_at":"2025-10-26T21:38:55.013079-07:00"}
{"id":"bd-166","title":"bd import/sync created 173 duplicate issues with wrong prefix","description":"## What Happened\nDuring corruption recovery investigation (beads-173), discovered the database contained 338 issues instead of expected 165:\n- 165 issues with correct `bd-` prefix \n- 173 duplicate issues with `beads-` prefix (bd-1 → beads-1, etc.)\n- Database config was set to `beads` prefix instead of `bd`\n\n## Root Cause\nSome bd operation (likely import or sync) created duplicate issues with the wrong prefix. The database should have rejected or warned about prefix mismatch, but instead:\n1. Silently created duplicates with wrong prefix\n2. Changed database prefix config from `bd` to `beads`\n\n## Impact\n- **Data integrity violation**: Duplicate issues with different IDs\n- **Silent corruption**: No error or warning during creation \n- **Wrong prefix**: Database config changed without user consent\n- **Confusion**: Users see double the issues, dependencies broken\n\n## Recovery \nHad to manually fix the `issue_prefix` config key (not `prefix` as initially thought):\n```bash\nsqlite3 .beads/beads.db \"UPDATE config SET value = 'bd' WHERE key = 'issue_prefix';\"\nsqlite3 .beads/beads.db \"DELETE FROM issues WHERE id LIKE 'beads-%';\"\n```\n\n## What Should Happen\n1. **Reject prefix mismatch**: If importing issues with different prefix than configured, error or require `--rename-on-import`\n2. **Never auto-change prefix**: Database prefix should only change via explicit `bd rename-prefix` command \n3. **Validate on import**: Check imported issue IDs match configured prefix before creating\n4. **Warn on duplicate IDs**: Even with different prefixes, detect potential duplicates\n\n## Related\n- Discovered during beads-173 (database corruption investigation)\n- Similar to existing prefix validation in `bd sync` (bd-21)","notes":"## Root Cause Analysis\n\nFound the smoking gun! The bug is a combination of three factors:\n\n**1. Database filename-based prefix fallback (sqlite.go:577-589)**\n```go\nfunc derivePrefixFromPath(dbPath string) string {\n dbFileName := filepath.Base(dbPath)\n dbFileName = strings.TrimSuffix(dbFileName, \".db\")\n prefix := strings.TrimSuffix(dbFileName, \"-\")\n if prefix == \"\" {\n prefix = \"bd\"\n }\n return prefix\n}\n```\nWhen `issue_prefix` config is missing, CreateIssue uses database filename (beads.db → \"beads\" prefix).\n\n**2. Auto-import skips prefix validation (autoimport.go:175)**\n```go\nopts := ImportOptions{\n ResolveCollisions: true,\n SkipPrefixValidation: true, // Auto-import is lenient about prefixes\n}\n```\nThis allows importing bd- issues without error even when config says \"beads\".\n\n**3. Missing issue_prefix config**\nSomehow the database lost its `issue_prefix = 'bd'` config (should be set by `bd init --prefix bd`), triggering the fallback.\n\n**Timeline:**\n1. Database exists with bd- issues\n2. `issue_prefix` config gets lost (corruption? manual deletion? reinit?)\n3. Auto-import brings in bd- issues from git (SkipPrefixValidation allows this)\n4. New issue creation falls back to filename → uses \"beads\" prefix\n5. Result: 165 bd- issues + 173 beads- duplicates\n\n**Fixes needed:**\n1. Never derive prefix from filename - always require explicit config\n2. Auto-import should SET issue_prefix if missing (from first imported issue)\n3. Add validation that rejects operations if issue_prefix is missing\n4. Warn loudly if prefix fallback is triggered","status":"open","priority":0,"issue_type":"bug","created_at":"2025-10-26T21:38:39.096165-07:00","updated_at":"2025-10-26T21:42:08.17707-07:00"}
{"id":"bd-17","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-25T23:15:33.478579-07:00","closed_at":"2025-10-22T19:41:19.895847-07:00","dependencies":[{"issue_id":"bd-17","depends_on_id":"bd-10","type":"discovered-from","created_at":"2025-10-24T13:17:40.32522-07:00","created_by":"renumber"}]}
{"id":"bd-18","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-25T23:15:33.479496-07:00","closed_at":"2025-10-22T22:02:18.479512-07:00","dependencies":[{"issue_id":"bd-18","depends_on_id":"bd-10","type":"related","created_at":"2025-10-24T13:17:40.325463-07:00","created_by":"renumber"}]}
{"id":"bd-19","title":"MCP close tool method signature error - takes 1 positional argument but 2 were given","description":"The close approval routing fix in beads-mcp v0.11.0 works correctly and successfully routes update(status=\"closed\") calls to close() tool. However, the close() tool has a Python method signature bug that prevents execution.\n\nImpact: All MCP-based close operations are broken. Workaround: Use bd CLI directly.\n\nError: BdDaemonClient.close() takes 1 positional argument but 2 were given\n\nRoot cause: BdDaemonClient.close() only accepts self, but MCP tool passes issue_id and reason.\n\nAdditional issue: CLI close has FOREIGN KEY constraint error when recording reason parameter.\n\nSee GitHub issue #107 for full details.","status":"closed","priority":0,"issue_type":"bug","created_at":"2025-10-22T17:25:34.67056-07:00","updated_at":"2025-10-25T23:15:33.480292-07:00","closed_at":"2025-10-22T17:36:55.463445-07:00"}