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

Fix test failures: add missing issue_prefix config and use valid bd- prefixes

Browse Source 
- Add SetConfig('issue_prefix', 'bd') to collision tests (bd-166)
- Update test cases to use 'bd-' prefix instead of invalid custom IDs (bd-177)
- Fixes 5 test failures in TestDetectCollisions, TestScoreCollisions, TestRemapCollisions, TestCreateIssues

All tests now pass.

Amp-Thread-ID: https://ampcode.com/threads/T-b2413b0e-2720-45b1-9b3d-acaa7d4cf9b4
Co-authored-by: Amp <amp@ampcode.com>
This commit is contained in:
Steve Yegge
2025-10-27 13:11:45 -07:00
parent 228ef67ebf
commit 49dac2b767
3 changed files with 43 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
.beads/bd.jsonl
View File

@@ -88,6 +88,8 @@
{"id":"bd-178","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-27T12:51:52.033528-07:00","updated_at":"2025-10-27T12:51:52.033528-07:00"}
{"id":"bd-179","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-27T12:51:52.034442-07:00","updated_at":"2025-10-27T12:51:52.034442-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-180","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-27T13:08:48.798508-07:00","updated_at":"2025-10-27T13:08:48.798508-07:00"}
{"id":"bd-181","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-27T13:08:48.799037-07:00","updated_at":"2025-10-27T13:08:48.799037-07:00"}
{"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"}
{"id":"bd-20","title":"Fix pre-existing MCP test failures - show/update return arrays not dicts","description":"9 tests fail in beads-mcp because bd CLI commands return arrays but MCP client expects dicts:\n\nFailing tests:\n- test_create_and_show_issue: show returns array, expects dict\n- test_update_issue: update returns array, expects dict \n- test_add_dependency: show returns array, expects dict\n- test_invalid_issue_id: show returns empty dict instead of error\n- test_dependency_types: show returns array, expects dict\n- test_show_issue_tool: show returns array, expects dict\n- test_update_issue_tool: update returns array, expects dict\n- test_update_partial_fields: update returns array, expects dict\n- test_client_lazy_initialization: BdClient import issue\n\nRoot cause: bd CLI commands like 'bd show' and 'bd update' output JSON arrays, but BdCliClient.show() and BdCliClient.update() expect single dict objects.\n\nExample:\n```bash\nbd show test-1 --json\n[{\"id\":\"test-1\",...}] # Array, not dict\n```\n\nFix needed: Update bd_client.py to handle array responses and extract first element, or change CLI to return single object for single-ID operations.","status":"closed","priority":1,"issue_type":"bug","created_at":"2025-10-22T17:43:23.29302-07:00","updated_at":"2025-10-25T23:15:33.481154-07:00","closed_at":"2025-10-22T20:05:49.3826-07:00"}

15
internal/storage/sqlite/collision_test.go
View File

@@ -29,6 +29,11 @@ func TestDetectCollisions(t *testing.T) {
ctx := context.Background()
// Set issue prefix to prevent "database not initialized" errors
if err := store.SetConfig(ctx, "issue_prefix", "bd"); err != nil {
t.Fatalf("failed to set issue_prefix: %v", err)
}
// Setup: Create some existing issues in the database
existingIssue1 := &types.Issue{
ID: testIssueBD1,
@@ -531,6 +536,11 @@ func TestScoreCollisions(t *testing.T) {
ctx := context.Background()
// Set issue prefix to prevent "database not initialized" errors
if err := store.SetConfig(ctx, "issue_prefix", "bd"); err != nil {
t.Fatalf("failed to set issue_prefix: %v", err)
}
// Setup: Create issues with various reference patterns
issue1 := &types.Issue{
ID: "bd-1",
@@ -801,6 +811,11 @@ func TestRemapCollisions(t *testing.T) {
ctx := context.Background()
// Set issue prefix to prevent "database not initialized" errors
if err := store.SetConfig(ctx, "issue_prefix", "bd"); err != nil {
t.Fatalf("failed to set issue_prefix: %v", err)
}
// Setup: Create an existing issue in the database with a high ID number
// This ensures that when we remap bd-2 and bd-3, they get new IDs that don't conflict
existingIssue := &types.Issue{

52
internal/storage/sqlite/sqlite_test.go
View File

@@ -286,8 +286,8 @@ func (h *createIssuesTestHelper) assertNoAutoGenID(issues []*types.Issue, wantEr
if issue == nil {
continue
}
hasCustomID := issue.ID != "" && (issue.ID == "custom-1" || issue.ID == "custom-2" ||
issue.ID == "duplicate-id" || issue.ID == "existing-id")
hasCustomID := issue.ID != "" && (issue.ID == "bd-100" || issue.ID == "bd-200" ||
issue.ID == "bd-999" || issue.ID == "bd-existing")
if !hasCustomID && issue.ID != "" {
h.t.Errorf("issue %d: ID should not be auto-generated on error, got %s", i, issue.ID)
}
@@ -346,21 +346,21 @@ func TestCreateIssues(t *testing.T) {
},
},
{
name: "mixed ID assignment - explicit and auto-generated",
issues: []*types.Issue{
h.newIssue("custom-1", "Custom ID 1", types.StatusOpen, 1, types.TypeTask, nil),
h.newIssue("", "Auto ID", types.StatusOpen, 1, types.TypeTask, nil),
h.newIssue("custom-2", "Custom ID 2", types.StatusOpen, 1, types.TypeTask, nil),
},
wantErr: false,
checkFunc: func(t *testing.T, h *createIssuesTestHelper, issues []*types.Issue) {
h.assertCount(issues, 3)
h.assertEqual("custom-1", issues[0].ID, "ID")
if issues[1].ID == "" || issues[1].ID == "custom-1" || issues[1].ID == "custom-2" {
t.Errorf("expected auto-generated ID, got %s", issues[1].ID)
}
h.assertEqual("custom-2", issues[2].ID, "ID")
},
name: "mixed ID assignment - explicit and auto-generated",
issues: []*types.Issue{
h.newIssue("bd-100", "Custom ID 1", types.StatusOpen, 1, types.TypeTask, nil),
h.newIssue("", "Auto ID", types.StatusOpen, 1, types.TypeTask, nil),
h.newIssue("bd-200", "Custom ID 2", types.StatusOpen, 1, types.TypeTask, nil),
},
wantErr: false,
checkFunc: func(t *testing.T, h *createIssuesTestHelper, issues []*types.Issue) {
h.assertCount(issues, 3)
h.assertEqual("bd-100", issues[0].ID, "ID")
if issues[1].ID == "" || issues[1].ID == "bd-100" || issues[1].ID == "bd-200" {
t.Errorf("expected auto-generated ID, got %s", issues[1].ID)
}
h.assertEqual("bd-200", issues[2].ID, "ID")
},
},
{
name: "validation error - missing title",
@@ -384,13 +384,13 @@ func TestCreateIssues(t *testing.T) {
checkFunc: func(t *testing.T, h *createIssuesTestHelper, issues []*types.Issue) {},
},
{
name: "duplicate ID error",
issues: []*types.Issue{
h.newIssue("duplicate-id", "First issue", types.StatusOpen, 1, types.TypeTask, nil),
h.newIssue("duplicate-id", "Second issue", types.StatusOpen, 1, types.TypeTask, nil),
},
wantErr: true,
checkFunc: func(t *testing.T, h *createIssuesTestHelper, issues []*types.Issue) {},
name: "duplicate ID error",
issues: []*types.Issue{
h.newIssue("bd-999", "First issue", types.StatusOpen, 1, types.TypeTask, nil),
h.newIssue("bd-999", "Second issue", types.StatusOpen, 1, types.TypeTask, nil),
},
wantErr: true,
checkFunc: func(t *testing.T, h *createIssuesTestHelper, issues []*types.Issue) {},
},
{
name: "closed_at invariant - open status with closed_at",
@@ -506,7 +506,7 @@ func TestCreateIssuesRollback(t *testing.T) {
t.Run("rollback on conflict with existing ID", func(t *testing.T) {
// Create an issue with explicit ID
existingIssue := &types.Issue{
ID: "existing-id",
ID: "bd-existing",
Title: "Existing issue",
Status: types.StatusOpen,
Priority: 1,
@@ -526,7 +526,7 @@ func TestCreateIssuesRollback(t *testing.T) {
IssueType: types.TypeTask,
},
{
ID: "existing-id",
ID: "bd-existing",
Title: "Conflict",
Status: types.StatusOpen,
Priority: 1,