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

Fix SQL timestamp scanning error on macOS (bd-161, GH-88)

Browse Source 
Fixes timestamp scanning error reported in GH-88 where DATETIME columns
were being returned as strings instead of time.Time on macOS 13.5.

Root cause: modernc.org/sqlite driver doesn't recognize mattn-style DSN
parameters (_journal_mode, _foreign_keys). When these incompatible
parameters are present, the driver ignores _time_format=sqlite on some
platforms, causing DATETIME values to remain as strings.

Solution: Use modernc's native _pragma syntax for all database options:
- Changed _journal_mode=WAL to _pragma=journal_mode(WAL)
- Changed _foreign_keys=ON to _pragma=foreign_keys(ON)
- Kept _pragma=busy_timeout(30000) and _time_format=sqlite

This ensures all parameters are properly recognized and DATETIME columns
are automatically parsed to time.Time across all platforms.

Fixes #88

Amp-Thread-ID: https://ampcode.com/threads/T-44d1817a-3709-4f1d-a27a-78bb2fa4d3dc
Co-authored-by: Amp <amp@ampcode.com>
This commit is contained in:
Steve Yegge
2025-10-19 14:36:05 -07:00
parent c71da4c267
commit 7d695f0b87
2 changed files with 6 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
.beads/issues.jsonl
View File

@@ -57,7 +57,7 @@
{"id":"bd-15","title":"Add performance benchmarks document","description":"Document actual performance metrics with hyperfine tests","status":"closed","priority":3,"issue_type":"task","created_at":"2025-10-16T20:46:08.971822-07:00","updated_at":"2025-10-18T18:35:11.75028-07:00","closed_at":"2025-10-18T10:09:23.532938-07:00"}
{"id":"bd-150","title":"Improve daemon fallback visibility and user feedback","description":"When daemon is unavailable, bd silently falls back to direct mode. Users don't know:\n- That daemon exists\n- Why auto-start failed\n- That they're in degraded mode\n- How to fix it\n\nThis creates confusion for multi-repo users who get slower performance without explanation.\n\nLocation: cmd/bd/main.go:98-130","design":"Add visibility at multiple levels:\n\n1. Debug logging (existing BD_DEBUG):\n - Already shows daemon connection attempts\n - Add auto-start success/failure\n\n2. Verbose mode (BD_VERBOSE):\n - Show warning when falling back\n - Suggest 'bd daemon --status' to check\n\n3. Status indicator:\n - Add daemon status to all commands when --json\n - Example: {\"daemon_status\": \"healthy\", \"daemon_type\": \"local\", ...}\n\n4. Explicit status command:\n - bd daemon --status shows detailed info\n - Shows whether daemon is running/healthy/unavailable\n\n5. Helpful error messages:\n - When auto-start fails repeatedly\n - When falling back after health check failure\n - With actionable next steps","acceptance_criteria":"- Users can see daemon status easily\n- Fallback warnings are helpful not noisy\n- JSON output includes daemon status\n- Error messages are actionable\n- Documentation explains status indicators\n- bd daemon --status command works","status":"closed","priority":1,"issue_type":"feature","created_at":"2025-10-18T13:06:46.212558-07:00","updated_at":"2025-10-18T18:36:51.769633-07:00","closed_at":"2025-10-18T18:36:51.769633-07:00"}
{"id":"bd-151","title":"Add version compatibility checks for daemon RPC protocol","description":"Client (bd CLI) and daemon may be different versions after upgrade. This causes:\n- Missing features (newer CLI, older daemon)\n- Protocol mismatches (older CLI, newer daemon)\n- Silent failures or confusing errors\n- No guidance to restart daemon\n\nLocation: internal/rpc/protocol.go, internal/rpc/client.go","design":"Add version field to RPC protocol:\n\n1. Add ClientVersion to Request struct\n2. Populate from Version constant in client\n3. Server checks compatibility in handleRequest()\n\nCompatibility rules:\n- Major version must match\n- Minor version backward compatible\n- Patch version always compatible\n\nOn mismatch:\n- Return clear error message\n- Suggest 'bd daemon --stop \u0026\u0026 bd daemon'\n- Log version info for debugging\n\nAdd to ping/health response:\n- Server version\n- Protocol version\n- Compatibility info\n\nAdd bd version --daemon command to check running daemon version.","acceptance_criteria":"- Version field in RPC protocol\n- Server validates client version\n- Clear error messages on mismatch\n- Health check returns version info\n- bd version --daemon command works\n- Documentation on version policy\n- Tests for version compatibility","status":"closed","priority":1,"issue_type":"feature","created_at":"2025-10-18T13:06:57.417411-07:00","updated_at":"2025-10-18T18:46:03.047035-07:00","closed_at":"2025-10-18T18:46:03.047035-07:00"}
{"id":"bd-152","title":"Add resource limits to daemon (connections, cache, memory)","description":"Daemon has no resource limits. Under heavy load or attack, it could:\n- Accept unlimited connections\n- Cache unlimited databases\n- Use unbounded memory\n- Exhaust file descriptors\n\nNeed limits for:\n- Max concurrent RPC connections (default: 100)\n- Max storage cache size (default: 50)\n- Request timeout enforcement (default: 30s)\n- Memory pressure detection\n\nLocation: internal/rpc/server.go","design":"Add resource tracking to Server:\n\ntype Server struct {\n // ... existing\n maxConns int32\n activeConns int32 // atomic\n connSemaphore chan struct{}\n}\n\nUse semaphore pattern for connection limiting:\n- Acquire token before handling connection\n- Release on completion\n- Reject connections when full\n\nAdd configurable limits via env vars:\n- BEADS_DAEMON_MAX_CONNS (default: 100)\n- BEADS_DAEMON_MAX_CACHE_SIZE (default: 50)\n- BEADS_DAEMON_REQUEST_TIMEOUT (default: 30s)\n\nAdd memory pressure detection:\n- Monitor runtime.MemStats\n- Trigger cache eviction at threshold\n- Log warnings at high memory use","acceptance_criteria":"- Connection limit enforced\n- Excess connections rejected gracefully\n- Request timeouts work\n- Memory limits configurable\n- Metrics expose current usage\n- Tests for limit enforcement\n- Documentation on tuning limits","status":"in_progress","priority":2,"issue_type":"feature","created_at":"2025-10-18T13:07:09.810963-07:00","updated_at":"2025-10-19T13:17:16.992675-07:00"}
{"id":"bd-152","title":"Add resource limits to daemon (connections, cache, memory)","description":"Daemon has no resource limits. Under heavy load or attack, it could:\n- Accept unlimited connections\n- Cache unlimited databases\n- Use unbounded memory\n- Exhaust file descriptors\n\nNeed limits for:\n- Max concurrent RPC connections (default: 100)\n- Max storage cache size (default: 50)\n- Request timeout enforcement (default: 30s)\n- Memory pressure detection\n\nLocation: internal/rpc/server.go","design":"Add resource tracking to Server:\n\ntype Server struct {\n // ... existing\n maxConns int32\n activeConns int32 // atomic\n connSemaphore chan struct{}\n}\n\nUse semaphore pattern for connection limiting:\n- Acquire token before handling connection\n- Release on completion\n- Reject connections when full\n\nAdd configurable limits via env vars:\n- BEADS_DAEMON_MAX_CONNS (default: 100)\n- BEADS_DAEMON_MAX_CACHE_SIZE (default: 50)\n- BEADS_DAEMON_REQUEST_TIMEOUT (default: 30s)\n\nAdd memory pressure detection:\n- Monitor runtime.MemStats\n- Trigger cache eviction at threshold\n- Log warnings at high memory use","acceptance_criteria":"- Connection limit enforced\n- Excess connections rejected gracefully\n- Request timeouts work\n- Memory limits configurable\n- Metrics expose current usage\n- Tests for limit enforcement\n- Documentation on tuning limits","status":"closed","priority":2,"issue_type":"feature","created_at":"2025-10-18T13:07:09.810963-07:00","updated_at":"2025-10-19T13:21:47.891925-07:00","closed_at":"2025-10-19T13:21:47.891925-07:00"}
{"id":"bd-153","title":"Add telemetry and observability to daemon","description":"Daemon has no metrics or observability. Cannot monitor:\n- Request latency (p50, p95, p99)\n- Cache hit/miss rates\n- Active connections\n- Error rates\n- Resource usage over time\n\nNeeded for:\n- Performance debugging\n- Capacity planning\n- Production monitoring\n- SLA tracking\n\nLocation: internal/rpc/server.go","design":"Add metrics collection to daemon:\n\n1. Request metrics:\n - Total requests by operation\n - Latency histogram\n - Error count by type\n\n2. Cache metrics:\n - Hit/miss ratio\n - Eviction count\n - Current size\n\n3. Connection metrics:\n - Active connections\n - Total connections\n - Rejected connections\n\n4. Resource metrics:\n - Memory usage\n - Goroutine count\n - File descriptor count\n\nAdd metrics endpoint:\n- bd daemon --metrics (JSON output)\n- OpMetrics RPC operation\n- Prometheus-compatible format option\n\nAdd to health check response for free monitoring.","acceptance_criteria":"- Metrics collected for key operations\n- bd daemon --metrics command works\n- Metrics include timestamps\n- Latency percentiles calculated\n- Zero performance overhead\n- Documentation on metrics","status":"open","priority":2,"issue_type":"feature","created_at":"2025-10-18T13:07:19.835495-07:00","updated_at":"2025-10-18T18:35:11.751751-07:00"}
{"id":"bd-154","title":"Add log rotation for daemon.log","description":"daemon.log grows forever without rotation. With sync every 5 minutes:\n- ~105k log entries per year\n- No size limit\n- No cleanup\n- Eventually fills disk\n\nNeed automatic log rotation with:\n- Size-based rotation (default: 10MB)\n- Age-based cleanup (default: 7 days)\n- Compression of old logs\n- Configurable retention\n\nLocation: cmd/bd/daemon.go:455","design":"Use lumberjack library for rotation:\n\nimport \"gopkg.in/natefinch/lumberjack.v2\"\n\nlogF := \u0026lumberjack.Logger{\n Filename: logPath,\n MaxSize: 10, // MB\n MaxBackups: 3,\n MaxAge: 7, // days\n Compress: true,\n}\n\nMake configurable via env vars:\n- BEADS_DAEMON_LOG_MAX_SIZE (default: 10MB)\n- BEADS_DAEMON_LOG_MAX_BACKUPS (default: 3)\n- BEADS_DAEMON_LOG_MAX_AGE (default: 7 days)\n\nAdd to daemon status output:\n- Current log size\n- Number of archived logs\n- Oldest log timestamp","acceptance_criteria":"- Log rotation works automatically\n- Old logs are compressed\n- Retention policy enforced\n- Configuration via env vars works\n- Log size stays bounded\n- No log data loss during rotation\n- Documentation updated","status":"closed","priority":1,"issue_type":"feature","created_at":"2025-10-18T13:07:30.94896-07:00","updated_at":"2025-10-18T18:35:11.752336-07:00","closed_at":"2025-10-18T16:27:51.349037-07:00"}
{"id":"bd-155","title":"Daemon production readiness","description":"Make beads daemon production-ready for long-running use, multi-repo deployments, and resilient operation.\n\nCurrent state: Good foundation, works well for development\nTarget state: Production-ready for individual developers and small teams\n\nGap areas:\n1. Resource management (cache eviction, limits)\n2. Health monitoring and crash recovery\n3. Process lifecycle management\n4. User experience (visibility, feedback)\n5. Operational concerns (logging, metrics)\n\nSuccess criteria:\n- Can run for weeks without restart\n- Handles 50+ repositories efficiently\n- Recovers from crashes automatically\n- Users understand daemon status\n- Observable and debuggable","acceptance_criteria":"All child issues completed:\n- P0 issues: Storage cache, health checks, crash recovery, MCP cleanup\n- P1 issues: Global auto-start, visibility, version checks\n- P2 issues: Resource limits, telemetry, log rotation\n\nValidation:\n- Run daemon for 7+ days without issues\n- Test with 50+ repositories\n- Verify crash recovery\n- Confirm resource usage is bounded\n- Check metrics and logs are useful","status":"in_progress","priority":0,"issue_type":"epic","created_at":"2025-10-18T13:07:43.543715-07:00","updated_at":"2025-10-18T18:35:11.752924-07:00"}

8
internal/storage/sqlite/sqlite.go
View File

@@ -30,10 +30,12 @@ func New(path string) (*SQLiteStorage, error) {
}
// Open database with WAL mode for better concurrency and busy timeout for parallel writes
// Use modernc.org/sqlite's _pragma syntax for all options to ensure consistent behavior
// _pragma=journal_mode(WAL) enables Write-Ahead Logging for better concurrency
// _pragma=foreign_keys(ON) enforces foreign key constraints
// _pragma=busy_timeout(30000) means wait up to 30 seconds for locks instead of failing immediately
// Higher timeout helps with parallel issue creation from multiple processes
// _time_format=sqlite uses SQLite's native time format for DATETIME columns
db, err := sql.Open("sqlite", path+"?_journal_mode=WAL&_foreign_keys=ON&_pragma=busy_timeout(30000)&_time_format=sqlite")
// _time_format=sqlite enables automatic parsing of DATETIME columns to time.Time
db, err := sql.Open("sqlite", path+"?_pragma=journal_mode(WAL)&_pragma=foreign_keys(ON)&_pragma=busy_timeout(30000)&_time_format=sqlite")
if err != nil {
return nil, fmt.Errorf("failed to open database: %w", err)
}