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

Add UnderlyingDB() method for extension database access

Browse Source 
Implements database platform layer for extensions like VC to create
their own tables in the same SQLite database.

Changes:
- Add UnderlyingDB() *sql.DB to Storage interface
- Implement in SQLiteStorage to expose underlying connection
- Add comprehensive test suite (5 tests, -race clean)
- Tests cover: basic access, extension tables, concurrency,
  lifecycle safety, and transaction behavior

This allows VC to host its executor_instances and other tables
alongside beads core tables with proper FK enforcement.

Related issues: bd-57, bd-64, bd-65, bd-66

Amp-Thread-ID: https://ampcode.com/threads/T-a6715beb-fe92-4dee-b931-3c9327124875
Co-authored-by: Amp <amp@ampcode.com>
This commit is contained in:
Steve Yegge
2025-10-22 17:19:57 -07:00
parent a689c66f77
commit 6829372c39
6 changed files with 312 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
.beads/beads.jsonl
View File

@@ -58,6 +58,9 @@
{"id":"bd-61","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":"open","priority":2,"issue_type":"task","created_at":"2025-10-22T14:04:51.320435-07:00","updated_at":"2025-10-22T14:04:51.320435-07:00","dependencies":[{"issue_id":"bd-61","depends_on_id":"bd-58","type":"parent-child","created_at":"2025-10-22T14:04:51.321526-07:00","created_by":"daemon"},{"issue_id":"bd-61","depends_on_id":"bd-60","type":"blocks","created_at":"2025-10-22T14:04:51.321935-07:00","created_by":"daemon"}]}
{"id":"bd-62","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":"open","priority":2,"issue_type":"task","created_at":"2025-10-22T14:05:07.755107-07:00","updated_at":"2025-10-22T14:05:07.755107-07:00","dependencies":[{"issue_id":"bd-62","depends_on_id":"bd-58","type":"parent-child","created_at":"2025-10-22T14:05:07.756023-07:00","created_by":"daemon"},{"issue_id":"bd-62","depends_on_id":"bd-61","type":"blocks","created_at":"2025-10-22T14:05:07.75651-07:00","created_by":"daemon"}]}
{"id":"bd-63","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-22T14:34:44.084241-07:00","closed_at":"2025-10-22T14:34:44.084241-07:00","labels":["library-usage"],"dependencies":[{"issue_id":"bd-63","depends_on_id":"bd-1","type":"discovered-from","created_at":"2025-10-22T14:34:44.082772-07:00","created_by":"library-example"}],"comments":[{"id":7,"issue_id":"bd-63","author":"library-example","text":"This is a programmatic comment","created_at":"2025-10-22T21:34:44Z"}]}
{"id":"bd-64","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":"open","priority":1,"issue_type":"task","created_at":"2025-10-22T17:07:56.812983-07:00","updated_at":"2025-10-22T17:07:56.812983-07:00"}
{"id":"bd-65","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":"open","priority":1,"issue_type":"task","created_at":"2025-10-22T17:07:56.820056-07:00","updated_at":"2025-10-22T17:07:56.820056-07:00","dependencies":[{"issue_id":"bd-65","depends_on_id":"bd-57","type":"discovered-from","created_at":"2025-10-22T17:07:56.822413-07:00","created_by":"daemon"}]}
{"id":"bd-66","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":"open","priority":3,"issue_type":"feature","created_at":"2025-10-22T17:07:56.832638-07:00","updated_at":"2025-10-22T17:07:56.832638-07:00","dependencies":[{"issue_id":"bd-66","depends_on_id":"bd-57","type":"related","created_at":"2025-10-22T17:07:56.835844-07:00","created_by":"daemon"}]}
{"id":"bd-7","title":"Write tests for merge functionality","description":"Unit tests: validation, merge logic, data integrity. Integration tests: end-to-end workflow, export/import. Edge case tests: chains, circular refs, epics.","status":"closed","priority":1,"issue_type":"task","created_at":"2025-10-21T23:53:44.31362-07:00","updated_at":"2025-10-22T11:56:36.523309-07:00","closed_at":"2025-10-22T01:07:04.72062-07:00"}
{"id":"bd-8","title":"Improve error handling in dependency removal during remapping","description":"In updateDependencyReferences(), RemoveDependency errors are caught and ignored with continue (line 392). Comment says 'if dependency doesn't exist' but this catches ALL errors including real failures. Should check error type with errors.Is(err, ErrDependencyNotFound) and only ignore not-found errors, returning other errors properly.","status":"closed","priority":3,"issue_type":"bug","created_at":"2025-10-21T23:53:44.31362-07:00","updated_at":"2025-10-22T11:56:36.523529-07:00","closed_at":"2025-10-18T09:41:18.209717-07:00"}
{"id":"bd-9","title":"Test issue 2","description":"","status":"closed","priority":1,"issue_type":"task","created_at":"2025-10-21T23:53:44.31362-07:00","updated_at":"2025-10-22T11:56:36.523753-07:00","closed_at":"2025-10-21T22:06:41.257019-07:00","labels":["test-label"]}