Add follow-up issues for PR #149: test coverage, prefix validation, docs

This commit is contained in:
Steve Yegge
2025-10-27 10:47:16 -07:00
parent edb85700e6
commit 2d8d2cb8f6

View File

@@ -78,6 +78,13 @@
{"id":"bd-169","title":"Add test for CreateIssue with missing issue_prefix","description":"Add explicit test case that verifies CreateIssue fails correctly when issue_prefix config is missing.\n\n**Test:**\n```go\nfunc TestCreateIssue_MissingPrefix(t *testing.T) {\n store, cleanup := setupTestDB(t)\n defer cleanup()\n \n ctx := context.Background()\n \n // Clear the issue_prefix config\n err := store.SetConfig(ctx, \"issue_prefix\", \"\")\n require.NoError(t, err)\n \n // Attempt to create issue should fail\n issue := \u0026types.Issue{\n Title: \"Test issue\",\n Status: types.StatusOpen,\n Priority: 1,\n IssueType: types.TypeTask,\n }\n \n err = store.CreateIssue(ctx, issue, \"test\")\n require.Error(t, err)\n assert.Contains(t, err.Error(), \"database not initialized\")\n assert.Contains(t, err.Error(), \"issue_prefix config is missing\")\n}\n```\n\nThis ensures the fix for bd-166 doesn't regress.","status":"open","priority":3,"issue_type":"task","created_at":"2025-10-26T21:54:36.63521-07:00","updated_at":"2025-10-26T21:54:36.63521-07:00","dependencies":[{"issue_id":"bd-169","depends_on_id":"bd-166","type":"discovered-from","created_at":"2025-10-26T21:54:41.995525-07:00","created_by":"daemon"}]}
{"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-170","title":"Clean up beads-* duplicate issues and review corrupt backup for missing issues","description":"## Current State\n- Database has 3 duplicate beads-* issues (beads-2, beads-3, beads-4) that should be deleted\n- Have corrupt backup: `.beads/beads.db.corrupt-backup` (4.4MB) from bd-166 corruption incident\n- Current clean DB has 172 issues (155 closed, 14 open, 3 in_progress)\n\n## Tasks\n1. **Delete beads-* duplicates** - these are corrupted duplicates from bd-166\n ```bash\n sqlite3 .beads/beads.db \"DELETE FROM issues WHERE id LIKE 'beads-%';\"\n ```\n\n2. **Review corrupt backup for missing issues**\n - Open corrupt backup: `sqlite3 .beads/beads.db.corrupt-backup`\n - Compare issue counts: backup had ~338 issues (165 bd- + 173 beads- duplicates)\n - Check if any ~8 issues exist in backup that are NOT in current DB\n - Cherry-pick any legitimate issues that were lost during cleanup\n\n3. **Verification**\n - Compare issue IDs between corrupt backup and current DB\n - Identify any missing issues worth recovering\n - Document findings\n\n## Why P0\nThis blocks clean database state and may contain lost work from the corruption incident.","status":"closed","priority":0,"issue_type":"task","created_at":"2025-10-26T22:30:00.126524-07:00","updated_at":"2025-10-26T22:30:35.01995-07:00","closed_at":"2025-10-26T22:30:35.01995-07:00"}
{"id":"bd-171","title":"Address gosec security warnings (102 issues)","description":"Security linter warnings: file permissions (0755 should be 0750), G304 file inclusion via variable, G204 subprocess launches. Many are false positives but should be reviewed.","design":"Review each gosec warning. Add exclusions for legitimate cases to .golangci.yml. Fix real security issues (overly permissive file modes).","status":"open","priority":2,"issue_type":"task","created_at":"2025-10-26T23:54:06.320626-07:00","updated_at":"2025-10-26T23:54:06.320626-07:00"}
{"id":"bd-172","title":"Add optional post-merge git hook example for bd sync","description":"Create example git hook that auto-runs bd sync after git pull/merge.\n\nAdd to examples/git-hooks/:\n- post-merge hook that checks if .beads/issues.jsonl changed\n- If changed: run `bd sync` automatically\n- Make it optional/documented (not auto-installed)\n\nBenefits:\n- Zero-friction sync after git pull\n- Complements auto-detection as belt-and-suspenders\n\nNote: post-merge hook already exists for pre-commit/post-merge. Extend it to support sync.","status":"open","priority":2,"issue_type":"task","created_at":"2025-10-26T23:54:06.321545-07:00","updated_at":"2025-10-26T23:54:06.321545-07:00"}
{"id":"bd-173","title":"Address gosec security warnings (102 issues)","description":"Security linter warnings: file permissions (0755 should be 0750), G304 file inclusion via variable, G204 subprocess launches. Many are false positives but should be reviewed.","design":"Review each gosec warning. Add exclusions for legitimate cases to .golangci.yml. Fix real security issues (overly permissive file modes).","status":"open","priority":2,"issue_type":"task","created_at":"2025-10-26T23:55:58.5422-07:00","updated_at":"2025-10-26T23:55:58.5422-07:00"}
{"id":"bd-174","title":"Add optional post-merge git hook example for bd sync","description":"Create example git hook that auto-runs bd sync after git pull/merge.\n\nAdd to examples/git-hooks/:\n- post-merge hook that checks if .beads/issues.jsonl changed\n- If changed: run `bd sync` automatically\n- Make it optional/documented (not auto-installed)\n\nBenefits:\n- Zero-friction sync after git pull\n- Complements auto-detection as belt-and-suspenders\n\nNote: post-merge hook already exists for pre-commit/post-merge. Extend it to support sync.","status":"open","priority":2,"issue_type":"task","created_at":"2025-10-26T23:55:58.542616-07:00","updated_at":"2025-10-26T23:55:58.542616-07:00"}
{"id":"bd-175","title":"Add test coverage for internal/storage/memory backend","description":"","design":"Create internal/storage/memory/memory_test.go with test coverage similar to internal/storage/sqlite tests.\n\nTest areas:\n1. Basic CRUD: CreateIssue, GetIssue, UpdateIssue, DeleteIssue\n2. Bulk operations: CreateIssues, ListIssues with filters\n3. Dependencies: AddDependency, GetDependencies, RemoveDependency\n4. Labels: AddLabel, RemoveLabel, ListLabels\n5. Comments: AddComment, GetComments\n6. ID generation: Prefix handling, counter management\n7. LoadFromIssues: Proper initialization from JSONL data\n8. Thread safety: Concurrent operations with go test -race","status":"open","priority":1,"issue_type":"task","created_at":"2025-10-27T10:45:33.145874-07:00","updated_at":"2025-10-27T10:45:46.868985-07:00"}
{"id":"bd-176","title":"Document distinction between corruption prevention and collision resolution","description":"Clarify that the hash/fingerprint/collision architecture solves logical consistency (wrong prefixes, ID collisions) but NOT physical SQLite corruption. --no-db mode is still needed for multi-process/container scenarios.","status":"open","priority":2,"issue_type":"task","created_at":"2025-10-27T10:45:46.872233-07:00","updated_at":"2025-10-27T10:45:46.872233-07:00"}
{"id":"bd-177","title":"Add prefix validation in SQLite mode to fail fast on mismatches","description":"The new hash/collision architecture prevents logical consistency issues, but doesn't prevent wrong-prefix bugs. Add validation to reject writes with mismatched prefixes.","status":"open","priority":2,"issue_type":"feature","created_at":"2025-10-27T10:45:46.87772-07:00","updated_at":"2025-10-27T10:45:46.87772-07:00"}
{"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"}
{"id":"bd-2","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-25T23:15:33.462194-07:00","closed_at":"2025-10-18T09:41:18.209717-07:00"}