diff --git a/.beads/beads.jsonl b/.beads/beads.jsonl index 6c076276..dd2d1be1 100644 --- a/.beads/beads.jsonl +++ b/.beads/beads.jsonl @@ -50,6 +50,9 @@ {"id":"bd-143","title":"bd daemon auto-sync can wipe out issues.jsonl when database is empty","description":"During dogfooding session, bd daemon auto-sync exported empty database to JSONL, losing all 177 issues. Had to git restore to recover.\n\nRoot cause: bd export doesn't check if database is empty before exporting. When daemon has empty/wrong database, it wipes out valid JSONL file.\n\nImpact: DATA LOSS","design":"Add safeguard in bd export:\n1. Count total issues in database before export\n2. If count is 0, refuse to export and show error\n3. Provide --force flag to override if truly want empty export\n\nAlternative: Check if target JSONL exists and has issues, warn if about to replace with empty export","acceptance_criteria":"- bd export refuses to export when database has 0 issues\n- Clear error message: \"Refusing to export empty database (0 issues). Use --force to override.\"\n- --force flag allows override for intentional empty exports\n- Test: export with empty db fails, export with --force succeeds","status":"closed","priority":0,"issue_type":"bug","created_at":"2025-10-25T16:29:16.045548-07:00","updated_at":"2025-10-25T16:35:38.233384-07:00","closed_at":"2025-10-25T16:35:38.233384-07:00"} {"id":"bd-144","title":"bd import doesn't update database modification time (WAL mode)","description":"When running bd import in WAL mode, the -wal file is updated but main .db file timestamp stays old. This breaks staleness detection which only checks main .db file.\n\nDiscovered during dogfooding when import didn't trigger staleness refresh.\n\nImpact: Staleness checks fail to detect that database is newer than expected","design":"Two options:\n1. Checkpoint WAL after import to flush changes to main .db file\n2. Update staleness detection to check both .db and -wal file timestamps\n\nOption 1 is simpler and safer - just add PRAGMA wal_checkpoint(FULL) after import completes","acceptance_criteria":"- After bd import, main .db file modification time is updated\n- Staleness detection correctly sees database as fresh\n- Test: import, check .db mtime, verify it's recent","status":"closed","priority":1,"issue_type":"bug","created_at":"2025-10-25T16:29:16.048176-07:00","updated_at":"2025-10-25T16:37:49.940187-07:00","closed_at":"2025-10-25T16:37:49.940187-07:00"} {"id":"bd-145","title":"bd should show which database file it's using","description":"During dogfooding, bd showed \"0 issues\" when correct database had 177 issues. Confusion arose from which database path was being used (daemon default vs explicit --db flag).\n\nUsers need clear feedback about which database file bd is actually using, especially when daemon is involved.\n\nImpact: User confusion, working with wrong database unknowingly","design":"Add database path to verbose output or as a bd info command:\n1. bd info shows current database path, daemon status\n2. OR: bd ready/list/etc --verbose shows \"Using database: /path/to/.beads/beads.db\"\n3. Consider adding to bd status output\n\nWhen database path differs from expected, show warning","acceptance_criteria":"- User can easily determine which database file bd is using\n- bd info or similar command shows full database path\n- When using unexpected database (e.g., daemon vs explicit --db), show clear indication\n- Documentation updated with how to check database path","status":"closed","priority":1,"issue_type":"feature","assignee":"amp","created_at":"2025-10-25T16:29:16.059118-07:00","updated_at":"2025-10-25T16:42:45.768187-07:00","closed_at":"2025-10-25T16:42:45.768187-07:00"} +{"id":"bd-146","title":"Add configurable SortPolicy to GetReadyWork","description":"Add SortPolicy field to WorkFilter to support different ordering strategies: hybrid (default), priority-first, oldest-first. See SORT_POLICY_DESIGN.md for full specification.","design":"See SORT_POLICY_DESIGN.md for complete design.\n\nImplementation summary:\n1. Add SortPolicy type and constants (hybrid, priority, oldest)\n2. Add SortPolicy field to WorkFilter \n3. Implement buildOrderByClause() to generate SQL based on policy\n4. Default to hybrid for backwards compatibility\n5. Add --sort flag to bd ready command\n\nThis enables autonomous execution systems (like VC) to use strict priority ordering while preserving the current hybrid behavior for interactive use.","acceptance_criteria":"Unit tests verify each policy generates correct ORDER BY. Integration tests verify priority, hybrid, and oldest policies select issues in expected order. CLI bd ready --sort priority works. Empty SortPolicy defaults to hybrid (backwards compatible).","status":"closed","priority":1,"issue_type":"feature","created_at":"2025-10-25T18:44:27.907777-07:00","updated_at":"2025-10-25T18:50:52.816914-07:00","closed_at":"2025-10-25T18:50:52.816914-07:00"} +{"id":"bd-147","title":"Implement configurable sort policy for GetReadyWork","description":"Add SortPolicy field to WorkFilter to support different sorting strategies:\n- hybrid (default): Recent issues by priority, old by age\n- priority: Strict priority ordering for autonomous execution\n- oldest: Backlog clearing mode\n\nSolves issue where VC executor selects low-priority work instead of critical P0 work.","design":"See SORT_POLICY_DESIGN.md for complete design spec including:\n- Type definitions and constants\n- SQL ORDER BY clause generation\n- Testing strategy with test cases\n- CLI integration with --sort flag\n- Migration plan and backwards compatibility","acceptance_criteria":"- SortPolicy type added to types.go\n- buildOrderByClause() implemented in ready.go\n- Unit tests for all 3 policies pass\n- Integration tests verify priority selection order\n- bd ready --sort priority|oldest|hybrid works\n- Empty SortPolicy defaults to hybrid (backwards compatible)\n- Documentation updated in README.md and WORKFLOW.md","status":"in_progress","priority":1,"issue_type":"feature","created_at":"2025-10-25T18:46:44.35198-07:00","updated_at":"2025-10-25T18:47:36.416113-07:00"} +{"id":"bd-148","title":"Add configurable SortPolicy to GetReadyWork","description":"Add SortPolicy field to WorkFilter to support different ordering strategies: hybrid (default), priority-first, oldest-first. See SORT_POLICY_DESIGN.md for full specification.","design":"See SORT_POLICY_DESIGN.md for complete design.\n\nImplementation summary:\n1. Add SortPolicy type and constants (hybrid, priority, oldest)\n2. Add SortPolicy field to WorkFilter \n3. Implement buildOrderByClause() to generate SQL based on policy\n4. Default to hybrid for backwards compatibility\n5. Add --sort flag to bd ready command\n\nThis enables autonomous execution systems (like VC) to use strict priority ordering while preserving the current hybrid behavior for interactive use.","acceptance_criteria":"Unit tests verify each policy generates correct ORDER BY. Integration tests verify priority, hybrid, and oldest policies select issues in expected order. CLI bd ready --sort priority works. Empty SortPolicy defaults to hybrid (backwards compatible).","status":"open","priority":1,"issue_type":"feature","created_at":"2025-10-25T18:51:11.560979-07:00","updated_at":"2025-10-25T18:51:11.560979-07:00"} {"id":"bd-15","title":"Make merge command idempotent for safe retry after partial failures","description":"The merge command currently performs 3 operations without an outer transaction:\n1. Migrate dependencies from source → target\n2. Update text references across all issues\n3. Close source issues\n\nIf merge fails mid-operation (network issue, daemon crash, etc.), a retry will fail or produce incorrect results because some operations already succeeded.\n\n**Goal:** Make merge idempotent so retrying after partial failure is safe and completes the remaining work.\n\n**Idempotency checks needed:**\n- Skip dependency migration if target already has the dependency\n- Skip text reference updates if already updated\n- Skip closing source issues if already closed\n- Report which operations were skipped vs performed\n\n**Example output:**\n```\n✓ Merged 2 issue(s) into bd-63\n - Dependencies: 3 migrated, 2 already existed\n - Text references: 5 updated, 0 already correct\n - Source issues: 1 closed, 1 already closed\n```\n\n**Related:** bd-115 originally requested transaction support, but idempotency is a better solution for this use case since individual operations are already atomic.","design":"Current merge code already has some idempotency:\n- Dependency migration checks `alreadyExists` before adding (line ~145-151 in merge.go)\n- Text reference updates are naturally idempotent (replacing bd-X with bd-Y twice has same result)\n\nMissing idempotency:\n- CloseIssue fails if source already closed\n- Error messages don't distinguish \"already done\" from \"real failure\"\n\nImplementation:\n1. Check source issue status before closing - skip if already closed\n2. Track which operations succeeded/skipped\n3. Return detailed results for user visibility\n4. Consider adding --dry-run output showing what would be done vs skipped","status":"closed","priority":2,"issue_type":"feature","created_at":"2025-10-22T00:47:43.165434-07:00","updated_at":"2025-10-24T13:51:54.437619-07:00","closed_at":"2025-10-22T11:56:36.526276-07:00"} {"id":"bd-16","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-62 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-22T00:47:43.165645-07:00","updated_at":"2025-10-24T13:51:54.437812-07:00","closed_at":"2025-10-17T23:04:30.223432-07:00"} {"id":"bd-17","title":"Add cross-repo issue references (future enhancement)","description":"Support referencing issues across different beads repositories. Useful for tracking dependencies between separate projects.\n\nProposed syntax:\n- Local reference: bd-63 (current behavior)\n- Cross-repo by path: ~/src/other-project#bd-456\n- Cross-repo by workspace name: @project2:bd-789\n\nUse cases:\n1. Frontend project depends on backend API issue\n2. Shared library changes blocking multiple projects\n3. System administrator tracking work across machines\n4. Monorepo with separate beads databases per component\n\nImplementation challenges:\n- Storage layer needs to query external databases\n- Dependency resolution across repos\n- What if external repo not available?\n- How to handle in JSONL export/import?\n- Security: should repos be able to read others?\n\nDesign questions to resolve first:\n1. Read-only references vs full cross-repo dependencies?\n2. How to handle repo renames/moves?\n3. Absolute paths vs workspace names vs git remotes?\n4. Should bd-38 auto-discover related repos?\n\nRecommendation: \n- Gather user feedback first\n- Start with read-only references\n- Implement as plugin/extension?\n\nContext: This is mentioned in bd-38 as approach #2. Much more complex than daemon multi-repo approach. Only implement if there's strong user demand.\n\nPriority: Backlog (4) - wait for user feedback before designing","status":"closed","priority":4,"issue_type":"feature","created_at":"2025-10-22T00:47:43.165857-07:00","updated_at":"2025-10-24T13:51:54.438011-07:00","closed_at":"2025-10-20T22:00:31.966891-07:00"} @@ -97,7 +100,7 @@ {"id":"bd-55","title":"Refactor high complexity functions (gocyclo)","description":"11 functions exceed cyclomatic complexity threshold (\u003e30): runDaemonLoop (42), importIssuesCore (71), TestLabelCommands (67), issueDataChanged (39), etc.","design":"Break down complex functions into smaller, testable units. Extract validation, error handling, and business logic into separate functions.","notes":"Refactored issueDataChanged from complexity 39 → 11 by extracting into fieldComparator struct with methods for each comparison type.\n\nRefactored runDaemonLoop from complexity 42 → 7 by extracting:\n- setupDaemonLogger: Logger initialization logic\n- setupDaemonLock: Lock and PID file management\n- startRPCServer: RPC server startup with error handling\n- runGlobalDaemon: Global daemon mode handling\n- createSyncFunc: Sync cycle logic (export, commit, pull, import, push)\n- runEventLoop: Signal handling and main event loop\n\nCode review fixes:\n- Fixed sync overlap: Changed initial sync from `go doSync()` to synchronous `doSync()` to prevent race with ticker\n- Fixed resource cleanup: Replaced `os.Exit(1)` with `return` after acquiring locks to ensure defers run and clean up PID files/locks\n- Added signal.Stop(sigChan) in runEventLoop and runGlobalDaemon to prevent lingering notifications\n- Added server.Stop() in serverErrChan case for consistent cleanup\n\nRefactored TestLabelCommands from complexity 67 → \u003c10 by extracting labelTestHelper with methods:\n- createIssue: Issue creation helper\n- addLabel/addLabels: Label addition helpers\n- removeLabel: Label removal helper\n- getLabels: Label retrieval helper\n- assertLabelCount/assertHasLabel/assertHasLabels/assertNotHasLabel: Assertion helpers\n- assertLabelEvent: Event verification helper\n\nRefactored TestReopenCommand from complexity 37 → \u003c10 by extracting reopenTestHelper with methods:\n- createIssue: Issue creation helper\n- closeIssue/reopenIssue: State transition helpers\n- getIssue: Issue retrieval helper\n- addComment: Comment addition helper\n- assertStatus/assertClosedAtSet/assertClosedAtNil: Status assertion helpers\n- assertCommentEvent: Event verification helper\n\nRefactored tryAutoStartDaemon from complexity 34 → \u003c10 by extracting:\n- debugLog: Centralized debug logging helper\n- isDaemonHealthy: Fast-path health check\n- acquireStartLock: Lock acquisition with wait/retry logic\n- handleStaleLock: Stale lock detection and retry\n- handleExistingSocket: Socket cleanup and validation\n- determineSocketMode: Global vs local daemon logic\n- startDaemonProcess: Process spawning and readiness wait\n- setupDaemonIO: I/O redirection setup\n\nRefactored DeleteIssues from complexity 37 → \u003c10 by extracting:\n- buildIDSet: ID deduplication\n- resolveDeleteSet: Cascade/force/validation mode routing\n- expandWithDependents: Recursive dependent collection\n- validateNoDependents: Dependency validation\n- checkSingleIssueValidation: Per-issue dependent check\n- trackOrphanedIssues: Force-mode orphan tracking\n- collectOrphansForID: Per-issue orphan collection\n- buildSQLInClause: SQL placeholder generation\n- populateDeleteStats: Dry-run statistics collection\n- executeDelete: Actual deletion execution\n\nCode review fix (via oracle):\n- Added rows.Err() check in checkSingleIssueValidation to catch iterator errors\n\nRefactored TestLibraryIntegration from complexity 32 → \u003c10 by extracting integrationTestHelper with methods:\n- createIssue/createFullIssue: Issue creation helpers\n- updateIssue/closeIssue: Issue modification helpers\n- addDependency/addLabel/addComment: Relationship helpers\n- getIssue/getDependencies/getLabels/getComments: Retrieval helpers\n- assertID/assertEqual/assertNotNil/assertCount: Assertion helpers\n\nRefactored TestExportImport from complexity 31 → \u003c10 by extracting exportImportHelper with methods:\n- createIssue/createFullIssue: Issue creation helpers\n- searchIssues/getIssue/updateIssue: Storage operations\n- encodeJSONL/validateJSONLines: JSONL encoding and validation\n- assertCount/assertEqual/assertSorted: Assertion helpers\n\nRefactored TestListCommand from complexity 31 → \u003c10 by extracting listTestHelper with methods:\n- createTestIssues: Batch test data creation\n- addLabel: Label addition\n- search: Issue search with filters\n- assertCount/assertEqual/assertAtMost: Assertion helpers\n\nRefactored TestGetEpicsEligibleForClosure from complexity 32 → \u003c10 by extracting epicTestHelper with methods:\n- createEpic/createTask: Issue creation\n- addParentChildDependency/closeIssue: Issue relationships\n- getEligibleEpics/findEpic: Epic status queries\n- assertEpicStats/assertEpicFound/assertEpicNotFound: Epic-specific assertions\n\nRefactored TestCreateIssues from complexity 35 → \u003c10 by extracting createIssuesTestHelper with methods:\n- newIssue: Issue construction helper\n- createIssues: Batch issue creation\n- assertNoError/assertError: Error assertions\n- assertCount/assertIDSet/assertTimestampSet: Field assertions\n- assertUniqueIDs/assertEqual/assertNotNil: Validation helpers\n- assertNoAutoGenID: Error-case validation\n\nAll tests pass after refactoring. ✅\n\n**importIssuesCore was already refactored** (complexity 71 → ~10) using phase-based extraction:\n- getOrCreateStore, handlePrefixMismatch, handleCollisions\n- upsertIssues, importDependencies, importLabels, importComments\n\n**Status:** All 11 high-complexity functions have been refactored to \u003c10 complexity.","status":"closed","priority":1,"issue_type":"task","created_at":"2025-10-24T01:01:36.989066-07:00","updated_at":"2025-10-25T13:16:42.865768-07:00","closed_at":"2025-10-25T13:16:42.865768-07:00","dependencies":[{"issue_id":"bd-55","depends_on_id":"bd-52","type":"parent-child","created_at":"2025-10-24T13:17:40.323992-07:00","created_by":"renumber"}]} {"id":"bd-56","title":"Fix revive style issues (78 issues)","description":"Style violations: unused parameters (many cmd/args in cobra commands), missing exported comments, stuttering names (SQLiteStorage), indent-error-flow issues.","design":"Rename unused params to _, add godoc comments to exported types, fix stuttering names, simplify control flow.","notes":"Fixed 19 revive issues:\n- 14 unused-parameter (renamed to _)\n- 2 redefines-builtin-id (max→maxCount, min→minInt)\n- 3 indent-error-flow (gofmt fixed 2, skipped 1 complex nested one)\n\nRemaining issues are acceptable: 11 unused-params in deeper code, 2 empty-blocks with comments, 1 complex indent case, 1 superfluous-else in test.","status":"closed","priority":3,"issue_type":"task","created_at":"2025-10-24T01:01:36.99984-07:00","updated_at":"2025-10-25T18:13:45.059903-07:00","closed_at":"2025-10-25T18:13:45.059903-07:00","dependencies":[{"issue_id":"bd-56","depends_on_id":"bd-52","type":"parent-child","created_at":"2025-10-24T13:17:40.322412-07:00","created_by":"renumber"}]} {"id":"bd-57","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).","notes":"Fixed security issues:\n- Changed file permissions from 0644 → 0600 for JSONL exports and config files \n- Changed directory permissions from 0755 → 0750 in all test code\n- Updated .golangci.yml with proper exclusions for false positives\n\nRemaining gosec warnings (down from 102 to 22, all are false positives or acceptable):\n- G304: File inclusion via variable (test files only - reading test fixtures)\n- G204: Subprocess launches (git commands from trusted sources)\n- G115: Integer overflow conversions (safe controlled conversions)\n- G201: SQL string formatting (constructed from constants)\n\nAll real security issues have been addressed.","status":"closed","priority":2,"issue_type":"task","created_at":"2025-10-24T01:01:37.0139-07:00","updated_at":"2025-10-25T13:49:14.833807-07:00","closed_at":"2025-10-25T13:49:08.124412-07:00","dependencies":[{"issue_id":"bd-57","depends_on_id":"bd-52","type":"parent-child","created_at":"2025-10-24T13:17:40.324202-07:00","created_by":"renumber"}]} -{"id":"bd-58","title":"Handle unchecked errors (errcheck - 683 issues)","description":"683 unchecked error returns, mostly in tests (Close, Rollback, RemoveAll). Many already excluded in config but still showing up.","design":"Review .golangci.yml exclude-rules. Most defer Close/Rollback errors in tests can be ignored. Add systematic exclusions or explicit _ = assignments where appropriate.","status":"in_progress","priority":3,"issue_type":"task","created_at":"2025-10-24T01:01:37.018404-07:00","updated_at":"2025-10-25T18:29:25.455845-07:00","dependencies":[{"issue_id":"bd-58","depends_on_id":"bd-52","type":"parent-child","created_at":"2025-10-24T13:17:40.324423-07:00","created_by":"renumber"}]} +{"id":"bd-58","title":"Handle unchecked errors (errcheck - 683 issues)","description":"683 unchecked error returns, mostly in tests (Close, Rollback, RemoveAll). Many already excluded in config but still showing up.","design":"Review .golangci.yml exclude-rules. Most defer Close/Rollback errors in tests can be ignored. Add systematic exclusions or explicit _ = assignments where appropriate.","notes":"Fixed all errcheck warnings in production code:\n- Enabled errcheck linter (was disabled)\n- Set tests: false in .golangci.yml to focus on production code\n- Fixed 27 total errors in production code using Oracle guidance:\n * Database patterns: defer func() { _ = rows.Close() }() and defer func() { _ = tx.Rollback() }()\n * Best-effort closers: _ = store.Close(), _ = client.Close()\n * Proper error handling for file writes, fmt.Scanln(), os.Remove()\n- All tests pass\n- Only 2 \"unused\" linter warnings remain (not errcheck)","status":"closed","priority":3,"issue_type":"task","created_at":"2025-10-24T01:01:37.018404-07:00","updated_at":"2025-10-25T18:44:01.656877-07:00","closed_at":"2025-10-25T18:44:01.656877-07:00","dependencies":[{"issue_id":"bd-58","depends_on_id":"bd-52","type":"parent-child","created_at":"2025-10-24T13:17:40.324423-07:00","created_by":"renumber"}]} {"id":"bd-59","title":"Update LINTING.md with current baseline","description":"After cleanup, document the remaining acceptable baseline in LINTING.md so we can track regression.","status":"open","priority":2,"issue_type":"task","created_at":"2025-10-24T01:01:37.02745-07:00","updated_at":"2025-10-24T13:51:54.419194-07:00","dependencies":[{"issue_id":"bd-59","depends_on_id":"bd-52","type":"parent-child","created_at":"2025-10-24T13:17:40.327184-07:00","created_by":"renumber"},{"issue_id":"bd-59","depends_on_id":"bd-53","type":"blocks","created_at":"2025-10-24T13:17:40.327422-07:00","created_by":"renumber"},{"issue_id":"bd-59","depends_on_id":"bd-54","type":"blocks","created_at":"2025-10-24T13:17:40.327627-07:00","created_by":"renumber"},{"issue_id":"bd-59","depends_on_id":"bd-55","type":"blocks","created_at":"2025-10-24T13:17:40.327827-07:00","created_by":"renumber"},{"issue_id":"bd-59","depends_on_id":"bd-56","type":"blocks","created_at":"2025-10-24T13:17:40.32803-07:00","created_by":"renumber"},{"issue_id":"bd-59","depends_on_id":"bd-57","type":"blocks","created_at":"2025-10-24T13:17:40.328233-07:00","created_by":"renumber"},{"issue_id":"bd-59","depends_on_id":"bd-58","type":"blocks","created_at":"2025-10-24T13:51:54.447799-07:00","created_by":"renumber"}]} {"id":"bd-6","title":"Add transaction support to storage layer for atomic multi-operation workflows","description":"Currently each storage method (CreateIssue, UpdateIssue, etc.) starts its own transaction. This makes it impossible to perform atomic multi-step operations like collision resolution. Add support for passing *sql.Tx through the storage interface, or create transaction-aware versions of methods. This would make remapCollisions and other batch operations truly atomic.","status":"closed","priority":4,"issue_type":"feature","created_at":"2025-10-21T23:53:44.31362-07:00","updated_at":"2025-10-24T13:51:54.3808-07:00","closed_at":"2025-10-14T02:51:52.199176-07:00"} {"id":"bd-60","title":"Fix Windows CI test failures (5 failing tests)","description":"Windows CI has 5 flaky/failing tests: TestTryDaemonLockDetectsRunning, TestIsDaemonRunning_CurrentProcess (PID detection issues), TestScripts/import, TestMetricsSnapshot/uptime, TestSocketCleanup (socket in use).","design":"Investigate Windows-specific PID/process detection and socket cleanup. These may be race conditions or platform differences in how Windows handles process IDs and file locks.","status":"closed","priority":1,"issue_type":"bug","created_at":"2025-10-24T09:28:17.976175-07:00","updated_at":"2025-10-24T13:51:54.419583-07:00","closed_at":"2025-10-24T09:36:59.351114-07:00"} diff --git a/README.md b/README.md index d6fb9275..5453fee7 100644 --- a/README.md +++ b/README.md @@ -237,6 +237,11 @@ bd ready --limit 20 bd ready --priority 1 bd ready --assignee alice +# Sort policies (hybrid is default) +bd ready --sort priority # Strict priority order (P0, P1, P2, P3) +bd ready --sort oldest # Oldest issues first (backlog clearing) +bd ready --sort hybrid # Recent by priority, old by age (default) + # Show blocked issues bd blocked diff --git a/cmd/bd/ready.go b/cmd/bd/ready.go index e9fa5645..ac979a18 100644 --- a/cmd/bd/ready.go +++ b/cmd/bd/ready.go @@ -19,10 +19,12 @@ var readyCmd = &cobra.Command{ Run: func(cmd *cobra.Command, args []string) { limit, _ := cmd.Flags().GetInt("limit") assignee, _ := cmd.Flags().GetString("assignee") + sortPolicy, _ := cmd.Flags().GetString("sort") filter := types.WorkFilter{ // Leave Status empty to get both 'open' and 'in_progress' (bd-165) - Limit: limit, + Limit: limit, + SortPolicy: types.SortPolicy(sortPolicy), } // Use Changed() to properly handle P0 (priority=0) if cmd.Flags().Changed("priority") { @@ -33,11 +35,18 @@ var readyCmd = &cobra.Command{ filter.Assignee = &assignee } + // Validate sort policy + if !filter.SortPolicy.IsValid() { + fmt.Fprintf(os.Stderr, "Error: invalid sort policy '%s'. Valid values: hybrid, priority, oldest\n", sortPolicy) + os.Exit(1) + } + // If daemon is running, use RPC if daemonClient != nil { readyArgs := &rpc.ReadyArgs{ - Assignee: assignee, - Limit: limit, + Assignee: assignee, + Limit: limit, + SortPolicy: sortPolicy, } if cmd.Flags().Changed("priority") { priority, _ := cmd.Flags().GetInt("priority") @@ -283,6 +292,7 @@ func init() { readyCmd.Flags().IntP("limit", "n", 10, "Maximum issues to show") readyCmd.Flags().IntP("priority", "p", 0, "Filter by priority") readyCmd.Flags().StringP("assignee", "a", "", "Filter by assignee") + readyCmd.Flags().StringP("sort", "s", "hybrid", "Sort policy: hybrid (default), priority, oldest") rootCmd.AddCommand(readyCmd) rootCmd.AddCommand(blockedCmd) diff --git a/internal/rpc/protocol.go b/internal/rpc/protocol.go index 0bd93b35..02ab8c46 100644 --- a/internal/rpc/protocol.go +++ b/internal/rpc/protocol.go @@ -104,9 +104,10 @@ type ShowArgs struct { // ReadyArgs represents arguments for the ready operation type ReadyArgs struct { - Assignee string `json:"assignee,omitempty"` - Priority *int `json:"priority,omitempty"` - Limit int `json:"limit,omitempty"` + Assignee string `json:"assignee,omitempty"` + Priority *int `json:"priority,omitempty"` + Limit int `json:"limit,omitempty"` + SortPolicy string `json:"sort_policy,omitempty"` } // DepAddArgs represents arguments for adding a dependency diff --git a/internal/rpc/server.go b/internal/rpc/server.go index c41de3ff..57dc8581 100644 --- a/internal/rpc/server.go +++ b/internal/rpc/server.go @@ -1177,9 +1177,10 @@ func (s *Server) handleReady(req *Request) Response { } wf := types.WorkFilter{ - Status: types.StatusOpen, - Priority: readyArgs.Priority, - Limit: readyArgs.Limit, + Status: types.StatusOpen, + Priority: readyArgs.Priority, + Limit: readyArgs.Limit, + SortPolicy: types.SortPolicy(readyArgs.SortPolicy), } if readyArgs.Assignee != "" { wf.Assignee = &readyArgs.Assignee diff --git a/internal/storage/sqlite/ready.go b/internal/storage/sqlite/ready.go index 35919fcc..53a89c26 100644 --- a/internal/storage/sqlite/ready.go +++ b/internal/storage/sqlite/ready.go @@ -44,6 +44,13 @@ func (s *SQLiteStorage) GetReadyWork(ctx context.Context, filter types.WorkFilte args = append(args, filter.Limit) } + // Default to hybrid sort for backwards compatibility + sortPolicy := filter.SortPolicy + if sortPolicy == "" { + sortPolicy = types.SortPolicyHybrid + } + orderBySQL := buildOrderByClause(sortPolicy) + // Query with recursive CTE to propagate blocking through parent-child hierarchy // Algorithm: // 1. Find issues directly blocked by 'blocks' dependencies @@ -85,23 +92,9 @@ func (s *SQLiteStorage) GetReadyWork(ctx context.Context, filter types.WorkFilte AND NOT EXISTS ( SELECT 1 FROM blocked_transitively WHERE issue_id = i.id ) - ORDER BY - -- Hybrid sort: recent issues (48 hours) by priority, then oldest-first - CASE - WHEN datetime(i.created_at) >= datetime('now', '-48 hours') THEN 0 - ELSE 1 - END ASC, - CASE - WHEN datetime(i.created_at) >= datetime('now', '-48 hours') THEN i.priority - ELSE NULL - END ASC, - CASE - WHEN datetime(i.created_at) < datetime('now', '-48 hours') THEN i.created_at - ELSE NULL - END ASC, - i.created_at ASC - %s - `, whereSQL, limitSQL) + %s + %s + `, whereSQL, orderBySQL, limitSQL) rows, err := s.db.QueryContext(ctx, query, args...) if err != nil { @@ -180,3 +173,32 @@ func (s *SQLiteStorage) GetBlockedIssues(ctx context.Context) ([]*types.BlockedI return blocked, nil } + +// buildOrderByClause generates the ORDER BY clause based on sort policy +func buildOrderByClause(policy types.SortPolicy) string { + switch policy { + case types.SortPolicyPriority: + return `ORDER BY i.priority ASC, i.created_at ASC` + + case types.SortPolicyOldest: + return `ORDER BY i.created_at ASC` + + case types.SortPolicyHybrid: + fallthrough + default: + return `ORDER BY + CASE + WHEN datetime(i.created_at) >= datetime('now', '-48 hours') THEN 0 + ELSE 1 + END ASC, + CASE + WHEN datetime(i.created_at) >= datetime('now', '-48 hours') THEN i.priority + ELSE NULL + END ASC, + CASE + WHEN datetime(i.created_at) < datetime('now', '-48 hours') THEN i.created_at + ELSE NULL + END ASC, + i.created_at ASC` + } +} diff --git a/internal/storage/sqlite/ready_test.go b/internal/storage/sqlite/ready_test.go index cb20760a..063a2ed6 100644 --- a/internal/storage/sqlite/ready_test.go +++ b/internal/storage/sqlite/ready_test.go @@ -916,3 +916,181 @@ func TestExplainQueryPlanReadyWork(t *testing.T) { t.Error("Query plan contains table scans - indexes may not be used efficiently") } } + +// TestSortPolicyPriority tests strict priority-first sorting +func TestSortPolicyPriority(t *testing.T) { + store, cleanup := setupTestDB(t) + defer cleanup() + + ctx := context.Background() + + // Create issues with mixed ages and priorities + // Old issues (72 hours ago) + issueP0Old := &types.Issue{Title: "old-P0", Status: types.StatusOpen, Priority: 0, IssueType: types.TypeTask} + issueP2Old := &types.Issue{Title: "old-P2", Status: types.StatusOpen, Priority: 2, IssueType: types.TypeTask} + issueP1Old := &types.Issue{Title: "old-P1", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + + // Recent issues (12 hours ago) + issueP3New := &types.Issue{Title: "new-P3", Status: types.StatusOpen, Priority: 3, IssueType: types.TypeTask} + issueP1New := &types.Issue{Title: "new-P1", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + + // Create old issues first (to have older created_at) + store.CreateIssue(ctx, issueP0Old, "test-user") + store.CreateIssue(ctx, issueP2Old, "test-user") + store.CreateIssue(ctx, issueP1Old, "test-user") + + // Create new issues + store.CreateIssue(ctx, issueP3New, "test-user") + store.CreateIssue(ctx, issueP1New, "test-user") + + // Use priority sort policy + ready, err := store.GetReadyWork(ctx, types.WorkFilter{ + Status: types.StatusOpen, + SortPolicy: types.SortPolicyPriority, + }) + if err != nil { + t.Fatalf("GetReadyWork failed: %v", err) + } + + if len(ready) != 5 { + t.Fatalf("Expected 5 ready issues, got %d", len(ready)) + } + + // Verify strict priority ordering: P0, P1, P1, P2, P3 + // Within same priority, older created_at comes first + expectedOrder := []struct { + title string + priority int + }{ + {"old-P0", 0}, + {"old-P1", 1}, + {"new-P1", 1}, + {"old-P2", 2}, + {"new-P3", 3}, + } + + for i, expected := range expectedOrder { + if ready[i].Title != expected.title { + t.Errorf("Position %d: expected %s, got %s", i, expected.title, ready[i].Title) + } + if ready[i].Priority != expected.priority { + t.Errorf("Position %d: expected P%d, got P%d", i, expected.priority, ready[i].Priority) + } + } +} + +// TestSortPolicyOldest tests oldest-first sorting (ignoring priority) +func TestSortPolicyOldest(t *testing.T) { + store, cleanup := setupTestDB(t) + defer cleanup() + + ctx := context.Background() + + // Create issues in order: P2, P0, P1 (mixed priority, chronological creation) + issueP2 := &types.Issue{Title: "first-P2", Status: types.StatusOpen, Priority: 2, IssueType: types.TypeTask} + issueP0 := &types.Issue{Title: "second-P0", Status: types.StatusOpen, Priority: 0, IssueType: types.TypeTask} + issueP1 := &types.Issue{Title: "third-P1", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + + store.CreateIssue(ctx, issueP2, "test-user") + store.CreateIssue(ctx, issueP0, "test-user") + store.CreateIssue(ctx, issueP1, "test-user") + + // Use oldest sort policy + ready, err := store.GetReadyWork(ctx, types.WorkFilter{ + Status: types.StatusOpen, + SortPolicy: types.SortPolicyOldest, + }) + if err != nil { + t.Fatalf("GetReadyWork failed: %v", err) + } + + if len(ready) != 3 { + t.Fatalf("Expected 3 ready issues, got %d", len(ready)) + } + + // Should be sorted by creation time only (oldest first) + expectedTitles := []string{"first-P2", "second-P0", "third-P1"} + for i, expected := range expectedTitles { + if ready[i].Title != expected { + t.Errorf("Position %d: expected %s, got %s", i, expected, ready[i].Title) + } + } +} + +// TestSortPolicyHybrid tests hybrid sort (default behavior) +func TestSortPolicyHybrid(t *testing.T) { + store, cleanup := setupTestDB(t) + defer cleanup() + + ctx := context.Background() + + // Create issues with different priorities + // All created recently (within 48 hours in test), so should sort by priority + issueP0 := &types.Issue{Title: "issue-P0", Status: types.StatusOpen, Priority: 0, IssueType: types.TypeTask} + issueP2 := &types.Issue{Title: "issue-P2", Status: types.StatusOpen, Priority: 2, IssueType: types.TypeTask} + issueP1 := &types.Issue{Title: "issue-P1", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + issueP3 := &types.Issue{Title: "issue-P3", Status: types.StatusOpen, Priority: 3, IssueType: types.TypeTask} + + store.CreateIssue(ctx, issueP2, "test-user") + store.CreateIssue(ctx, issueP0, "test-user") + store.CreateIssue(ctx, issueP3, "test-user") + store.CreateIssue(ctx, issueP1, "test-user") + + // Use hybrid sort policy (explicit) + ready, err := store.GetReadyWork(ctx, types.WorkFilter{ + Status: types.StatusOpen, + SortPolicy: types.SortPolicyHybrid, + }) + if err != nil { + t.Fatalf("GetReadyWork failed: %v", err) + } + + if len(ready) != 4 { + t.Fatalf("Expected 4 ready issues, got %d", len(ready)) + } + + // Since all issues are created recently (< 48 hours in test context), + // hybrid sort should order by priority: P0, P1, P2, P3 + expectedPriorities := []int{0, 1, 2, 3} + for i, expected := range expectedPriorities { + if ready[i].Priority != expected { + t.Errorf("Position %d: expected P%d, got P%d", i, expected, ready[i].Priority) + } + } +} + +// TestSortPolicyDefault tests that empty sort policy defaults to hybrid +func TestSortPolicyDefault(t *testing.T) { + store, cleanup := setupTestDB(t) + defer cleanup() + + ctx := context.Background() + + // Create test issues with different priorities + issueP1 := &types.Issue{Title: "issue-P1", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + issueP2 := &types.Issue{Title: "issue-P2", Status: types.StatusOpen, Priority: 2, IssueType: types.TypeTask} + + store.CreateIssue(ctx, issueP2, "test-user") + store.CreateIssue(ctx, issueP1, "test-user") + + // Use default (empty) sort policy + ready, err := store.GetReadyWork(ctx, types.WorkFilter{ + Status: types.StatusOpen, + // SortPolicy not specified - should default to hybrid + }) + if err != nil { + t.Fatalf("GetReadyWork failed: %v", err) + } + + if len(ready) != 2 { + t.Fatalf("Expected 2 ready issues, got %d", len(ready)) + } + + // Should behave like hybrid: since both are recent, sort by priority (P1 first) + if ready[0].Priority != 1 { + t.Errorf("Expected P1 first (hybrid default, recent by priority), got P%d", ready[0].Priority) + } + if ready[1].Priority != 2 { + t.Errorf("Expected P2 second, got P%d", ready[1].Priority) + } +} diff --git a/internal/types/types.go b/internal/types/types.go index ad789faf..55778782 100644 --- a/internal/types/types.go +++ b/internal/types/types.go @@ -216,12 +216,41 @@ type IssueFilter struct { Limit int } +// SortPolicy determines how ready work is ordered +type SortPolicy string + +// Sort policy constants +const ( + // SortPolicyHybrid prioritizes recent issues by priority, older by age + // Recent = created within 48 hours + // This is the default for backwards compatibility + SortPolicyHybrid SortPolicy = "hybrid" + + // SortPolicyPriority always sorts by priority first, then creation date + // Use for autonomous execution, CI/CD, priority-driven workflows + SortPolicyPriority SortPolicy = "priority" + + // SortPolicyOldest always sorts by creation date (oldest first) + // Use for backlog clearing, preventing issue starvation + SortPolicyOldest SortPolicy = "oldest" +) + +// IsValid checks if the sort policy value is valid +func (s SortPolicy) IsValid() bool { + switch s { + case SortPolicyHybrid, SortPolicyPriority, SortPolicyOldest, "": + return true + } + return false +} + // WorkFilter is used to filter ready work queries type WorkFilter struct { - Status Status - Priority *int - Assignee *string - Limit int + Status Status + Priority *int + Assignee *string + Limit int + SortPolicy SortPolicy } // EpicStatus represents an epic with its completion status