Fix goconst linter issues (bd-116)

- Use windowsOS constant in reinit_test.go
- Use testIssueBD1 constant in compact_test.go and counter_sync_test.go
- Merged duplicate bd-126 into bd-116

.beads/beads.jsonl

@@ -28,7 +28,7 @@
 {"id":"bd-123","title":"Daemon storage cache doesn't detect external database modifications","description":"When bd commands bypass the daemon and directly modify the database (e.g., `bd import` with direct file access, or deleting/recreating bd.db), the daemon's cached storage connection becomes stale and serves outdated data.\n\n**Reproduction**:\n1. Start daemon: `bd daemon`\n2. Run bd stats → shows N issues\n3. Delete database: `rm .beads/bd.db`  \n4. Reinit and import: `bd init \u0026\u0026 bd import -i .beads/issues.jsonl`\n5. Run bd stats → shows 0 issues (wrong!)\n6. Direct query: `sqlite3 .beads/bd.db 'SELECT COUNT(*) FROM issues'` → shows correct count\n7. Restart daemon: `bd daemon --stop` then retry stats → now shows correct count\n\n**Root cause**: \n- server.go:1410-1414 retrieves cached storage without checking if DB file changed\n- Cache only evicts based on TTL (30min) or LRU, never on external modifications\n- Direct file operations bypass daemon, leaving cache stale\n\n**Impact**:\n- Users see incorrect/stale data after external DB operations\n- Confusing behavior with no clear indication cache is stale\n- Requires daemon restart to fix\n\n**Proposed fixes**:\n1. Check mtime on cache hit, invalidate if file changed\n2. Add cache eviction API (bd cache --clear)\n3. Use file locking to prevent external modifications while daemon running\n4. SQLite WAL mode change notifications","design":"**Better approach: Check DB file mtime on cache lookup**\n\nToo many commands bypass the daemon (import, init, renumber, compact, delete, dep tree, export, stale). Notifying from each would be error-prone and easy to forget when adding new commands.\n\n**Implementation:**\n\n1. Add `dbMtime time.Time` field to `StorageCacheEntry`\n2. In `getStorageForRequest()` on cache hit:\n   - Stat the DB file to get current mtime\n   - If mtime changed since cached, evict entry and reopen\n   - Otherwise return cached connection\n3. Store mtime when initially caching\n\n**Code location:**\n- `internal/rpc/server.go:1410-1414` (cache hit path)\n- `internal/rpc/server.go:49-52` (StorageCacheEntry struct)\n\n**Benefits:**\n- Simple, centralized check\n- Works for all commands that bypass daemon\n- Works for external tools modifying DB\n- No need to update every command\n- Minimal performance overhead (one stat() call on cache hit)\n\n**Trade-offs:**\n- Small overhead on every cache hit (negligible - stat is fast)\n- mtime granularity may miss rapid changes (unlikely in practice)","status":"closed","priority":1,"issue_type":"bug","created_at":"2025-10-24T13:35:23.108829-07:00","updated_at":"2025-10-24T13:51:54.433938-07:00","closed_at":"2025-10-21T21:51:22.331957-07:00"}
 {"id":"bd-124","title":"Implement bd quickstart command","description":"Add bd quickstart command to show context-aware repo information: recent issues, database location, configured prefix, example queries. Helps AI agents understand current project state. Companion to bd onboard.","notes":"After review, we already have good context tools: bd stats, bd list, bd ready, and the AGENTS.md onboarding section. Adding bd quickstart would be redundant and doesn't add enough value to justify maintenance cost. Closing as won't implement.","status":"closed","priority":2,"issue_type":"feature","created_at":"2025-10-24T13:35:23.109187-07:00","updated_at":"2025-10-24T13:51:54.434128-07:00","closed_at":"2025-10-22T21:30:29.491988-07:00"}
 {"id":"bd-125","title":"Global daemon should warn/reject --auto-commit and --auto-push","description":"When user runs 'bd daemon --global --auto-commit', it's unclear which repo the daemon will commit to (especially after fixing bd-38 where global daemon won't open a DB).\n\nOptions:\n1. Warn and ignore the flags in global mode\n2. Error out with clear message\n\nLine 87-91 already checks autoPush, but should skip check entirely for global mode. Add user-friendly messaging about flag incompatibility.","status":"closed","priority":3,"issue_type":"feature","created_at":"2025-10-24T13:35:23.10938-07:00","updated_at":"2025-10-24T13:51:54.443558-07:00","closed_at":"2025-10-17T23:04:30.223432-07:00"}
-{"id":"bd-126","title":"Convert repeated strings to constants (goconst)","description":"12 instances of repeated strings that should be constants: \"alice\", \"windows\", \"bd-4\", \"daemon\", \"import\", \"healthy\", \"unhealthy\", \"1.0.0\", \"custom-1\", \"custom-2\"","design":"Create package-level or test-level constants for frequently used test strings and command names.","status":"open","priority":2,"issue_type":"task","created_at":"2025-10-24T13:35:23.109555-07:00","updated_at":"2025-10-24T13:51:54.443714-07:00"}
+{"id":"bd-126","title":"Convert repeated strings to constants (goconst)","description":"12 instances of repeated strings that should be constants: \"alice\", \"windows\", \"bd-4\", \"daemon\", \"import\", \"healthy\", \"unhealthy\", \"1.0.0\", \"custom-1\", \"custom-2\"","design":"Create package-level or test-level constants for frequently used test strings and command names.","status":"closed","priority":2,"issue_type":"task","created_at":"2025-10-24T13:35:23.109555-07:00","updated_at":"2025-10-25T18:02:26.966923-07:00","closed_at":"2025-10-25T18:02:26.966923-07:00"}
 {"id":"bd-127","title":"Auto-flush writes test pollution and session work to git-tracked issues.jsonl","description":"Auto-flush exports ALL issues from DB to issues.jsonl every 5 seconds, including:\n- Test issues (bd-4053 through bd-4059 were version test junk)\n- Issues created during debugging sessions\n- Test pollution from stress tests\n- Temporary diagnostic issues\n\nThis pollutes the git-tracked issues.jsonl with garbage that shouldn't be committed.\n\nExample from today:\n- Git had 49 clean issues\n- Our DB grew to 100+ with test junk and session work\n- Auto-flush wrote all 100+ to issues.jsonl\n- Git status showed modified issues.jsonl with 50+ unwanted issues\n\nImpact:\n- Pollutes git history with test/debug garbage\n- Makes code review difficult (noise in diffs)\n- Can't distinguish real work from session artifacts\n- Other team members pull polluted issues\n\nSolutions to consider:\n1. Disable auto-flush by default (require explicit --enable-auto-flush)\n2. Add .beadsignore to exclude issue ID patterns\n3. Make auto-flush only export 'real' issues (exclude test-*)\n4. Require manual 'bd sync' for git commit\n5. Auto-flush to separate file (.beads/session.jsonl vs issues.jsonl)\n\nRelated: bd-11 (test pollution), isolation_test.go (test DB separation)","status":"closed","priority":1,"issue_type":"bug","created_at":"2025-10-24T13:35:23.109731-07:00","updated_at":"2025-10-24T13:51:54.443881-07:00","closed_at":"2025-10-22T01:05:59.459797-07:00"}
 {"id":"bd-128","title":"Issue counter gets out of sync with actual issues","description":"The issue counter in issue_counters table frequently desyncs from actual max issue ID, causing:\n- Import from JSONL leaves counter at old high value\n- Test pollution increments counter but cleanup doesn't decrement it\n- Delete issues doesn't update counter\n- Only fix is 'rm -rf .beads' which is destructive\n\nExamples from today's session:\n- Had 48 issues but counter at 7714 after test pollution\n- Import from git didn't reset counter\n- Next new issue would be bd-7715 instead of bd-11\n\nProposed fixes:\n1. Auto-recalculate counter from max(issue_id) on import\n2. Add 'bd fix-counter' command\n3. Make counter lazy (always compute from DB, don't store)\n4. Import should reset counter to match imported data\n\nRelated:-254 (test isolation), bd-12 (init timestamp bug)","status":"closed","priority":1,"issue_type":"bug","created_at":"2025-10-24T13:35:23.109915-07:00","updated_at":"2025-10-24T13:51:54.444058-07:00","closed_at":"2025-10-21T23:13:04.249149-07:00"}
 {"id":"bd-129","title":"Counter not synced after import on existing DB with populated issue_counters table","description":"The counter sync fix in counter_sync_test.go only syncs during initial migration when issue_counters table is empty (migrateIssueCountersTable checks count==0). For existing databases with stale counters:\n\n- Import doesn't resync the counter\n- Delete doesn't update counter  \n- Renumber doesn't fix counter\n- Counter remains stuck at old high value\n\nExample from today:\n- Had 49 issues after clean import\n- Counter stuck at 4106 from previous test pollution\n- Next issue would be bd-4107 instead of bd-12\n- Even after renumber, counter stayed at 4106\n\nRoot cause: Migration only syncs if table is empty (line 182 in sqlite.go). Once populated, never resyncs.\n\nFix needed: \n1. Sync counter after import operations (not just empty table)\n2. Add counter resync after renumber\n3. Daemon caches counter value - needs to reload after external changes\n\nRelated: bd-11 (original counter sync fix), bd-7 (daemon cache staleness)","notes":"## Investigation Results\n\nAfter thorough code review, all the fixes mentioned in the issue description have ALREADY been implemented:\n\n### ✅ Fixes Already in Place:\n\n1. **Import DOES resync counters**\n   - `cmd/bd/import_shared.go:253` calls `SyncAllCounters()` after batch import\n   - Verified with new test `TestCounterSyncAfterImport`\n\n2. **Delete DOES update counters**\n   - `internal/storage/sqlite/sqlite.go:1424` calls `SyncAllCounters()` after deletion\n   - Both single delete and batch delete sync properly\n   - Verified with existing tests: `TestCounterSyncAfterDelete`, `TestCounterSyncAfterBatchDelete`\n\n3. **Renumber DOES fix counters**\n   - `cmd/bd/renumber.go:298-304` calls `ResetCounter()` then `SyncAllCounters()`\n   - Forces counter to actual max ID (not just MAX with stale value)\n\n4. **Daemon cache DOES detect external changes**\n   - `internal/rpc/server.go:1466-1487` checks file mtime and evicts stale cache\n   - When DB file changes externally, cached storage is evicted and reopened\n\n### Tests Added:\n\n- `TestCounterSyncAfterImport`: Confirms import syncs counters from stale value (4106) to actual max (49)\n- `TestCounterNotSyncedWithoutExplicitSync`: Documents what would happen without the fix (bd-4107 instead of bd-12)\n\n### Conclusion:\n\nThe issue described in bd-12 has been **fully resolved**. All operations (import, delete, renumber) now properly sync counters. The daemon correctly detects external DB changes via file modification time.\n\nThe root cause (migration only syncing empty tables) was fixed by adding explicit `SyncAllCounters()` calls after import, delete, and renumber operations.","status":"closed","priority":1,"issue_type":"bug","created_at":"2025-10-24T13:35:23.110118-07:00","updated_at":"2025-10-24T13:51:54.444298-07:00","closed_at":"2025-10-22T00:03:46.697918-07:00"}

cmd/bd/reinit_test.go

@@ -438,7 +438,7 @@ func writeJSONL(path string, issues []*types.Issue) error {
 // normalizeGitPath converts a path to use forward slashes for git compatibility
 // Git always uses forward slashes internally, even on Windows
 func normalizeGitPath(path string) string {
-	if runtime.GOOS == "windows" {
+	if runtime.GOOS == windowsOS {
 		return filepath.ToSlash(path)
 	}
 	return path

internal/storage/sqlite/compact_test.go

@@ -17,7 +17,7 @@ func TestGetTier1Candidates(t *testing.T) {
 	// Create test issues
 	// Old closed issue (eligible)
 	issue1 := &types.Issue{
-		ID:          "bd-1",
+		ID:          testIssueBD1,
 		Title:       "Old closed issue",
 		Description: "This is a test description",
 		Status:      "closed",
@@ -91,7 +91,7 @@ func TestGetTier1Candidates(t *testing.T) {
 		t.Errorf("Expected 1 candidate, got %d", len(candidates))
 	}
 
-	if len(candidates) > 0 && candidates[0].IssueID != "bd-1" {
+	if len(candidates) > 0 && candidates[0].IssueID != testIssueBD1 {
 		t.Errorf("Expected candidate bd-1, got %s", candidates[0].IssueID)
 	}
 }
@@ -145,7 +145,7 @@ func TestGetTier2Candidates(t *testing.T) {
 		t.Errorf("Expected 1 candidate, got %d", len(candidates))
 	}
 
-	if len(candidates) > 0 && candidates[0].IssueID != "bd-1" {
+	if len(candidates) > 0 && candidates[0].IssueID != testIssueBD1 {
 		t.Errorf("Expected candidate bd-1, got %s", candidates[0].IssueID)
 	}
 }

internal/storage/sqlite/counter_sync_test.go

@@ -221,7 +221,7 @@ func TestCounterSyncAfterDeleteAll(t *testing.T) {
 	if err := store.CreateIssue(ctx, newIssue, "test"); err != nil {
 		t.Fatalf("Failed to create new issue: %v", err)
 	}
-	if newIssue.ID != "bd-1" {
+	if newIssue.ID != testIssueBD1 {
 		t.Errorf("Expected new issue to be bd-1, got %s", newIssue.ID)
 	}
 }