From 3ed2aa07cb591c7da13fa02a69bd010a4b87a531 Mon Sep 17 00:00:00 2001 From: Steve Yegge Date: Thu, 30 Oct 2025 14:42:08 -0700 Subject: [PATCH] Implement hierarchical child ID generation (bd-171) - Add GetNextChildID to storage interface for generating child IDs - Implement in SQLiteStorage with atomic counter using child_counters table - Implement in MemoryStorage with in-memory counter - Add --parent flag to bd create command - Support hierarchical IDs (bd-a3f8e9.1, bd-a3f8e9.1.5) in CreateIssue - Validate parent exists when creating hierarchical issues - Enforce max depth of 3 levels - Update ID validation to accept hierarchical IDs with dots - Add comprehensive tests for child ID generation - Manual testing confirms: sequential children, nested hierarchies, depth enforcement --- .beads/beads.jsonl | 2 +- cmd/bd/create.go | 50 ++++-- internal/storage/memory/memory.go | 26 +++ internal/storage/sqlite/child_id_test.go | 203 +++++++++++++++++++++++ internal/storage/sqlite/sqlite.go | 48 ++++++ internal/storage/storage.go | 3 + 6 files changed, 319 insertions(+), 13 deletions(-) create mode 100644 internal/storage/sqlite/child_id_test.go diff --git a/.beads/beads.jsonl b/.beads/beads.jsonl index 2d22eb7e..c1ede089 100644 --- a/.beads/beads.jsonl +++ b/.beads/beads.jsonl @@ -65,7 +65,7 @@ {"id":"bd-169","content_hash":"d565476761fdf87fd8ec031cdae0e6698734ace357c4f00e1b9947e4d7101eb7","title":"Update JSONL format to use hash IDs","description":"Update JSONL format to store hierarchical hash-based IDs.\n\n## Changes\n- ID field: bd-af78e9a2 (top-level) or bd-af78e9a2.1.2 (hierarchical)\n- No alias field needed (removed from original plan)\n- All other fields remain the same\n\n## Example\n```json\n{\"id\":\"bd-af78e9a2\",\"title\":\"Auth System\",\"type\":\"epic\",...}\n{\"id\":\"bd-af78e9a2.1\",\"title\":\"Login Flow\",\"type\":\"epic\",...}\n{\"id\":\"bd-af78e9a2.1.1\",\"title\":\"Design UI\",\"type\":\"task\",...}\n```\n\n## Benefits\n- Hierarchical structure visible in JSONL\n- Git merge conflicts reduced (different IDs = different lines)\n- Easy to grep for epic children: grep \"bd-af78e9a2\\.\" issues.jsonl\n\n## Backward Compatibility\nMigration tool will convert bd-1 → bd-{hash} with mapping preserved.","status":"closed","priority":1,"issue_type":"task","created_at":"2025-10-29T21:24:47.408106-07:00","updated_at":"2025-10-30T14:27:39.953114-07:00","closed_at":"2025-10-30T14:27:39.953114-07:00","dependencies":[{"issue_id":"bd-169","depends_on_id":"bd-165","type":"parent-child","created_at":"2025-10-29T21:24:47.409489-07:00","created_by":"stevey"},{"issue_id":"bd-169","depends_on_id":"bd-168","type":"blocks","created_at":"2025-10-29T21:24:47.409977-07:00","created_by":"stevey"},{"issue_id":"bd-169","depends_on_id":"bd-167","type":"blocks","created_at":"2025-10-29T21:29:45.975499-07:00","created_by":"stevey"},{"issue_id":"bd-169","depends_on_id":"bd-192","type":"blocks","created_at":"2025-10-30T14:22:59.347557-07:00","created_by":"import-remap"}]} {"id":"bd-17","content_hash":"404b82a19dde2fdece7eb6bb3b816db7906e81a03a5a05341ed631af7a2a8e87","title":"Remove unreachable RPC methods","description":"Several RPC server and client methods are unreachable and should be removed:\n\nServer methods (internal/rpc/server.go):\n- `Server.GetLastImportTime` (line 2116)\n- `Server.SetLastImportTime` (line 2123)\n- `Server.findJSONLPath` (line 2255)\n\nClient methods (internal/rpc/client.go):\n- `Client.Import` (line 311) - RPC import not used (daemon uses autoimport)\n\nEvidence:\n```bash\ngo run golang.org/x/tools/cmd/deadcode@latest -test ./...\n```\n\nImpact: Removes ~80 LOC of unused RPC code","acceptance_criteria":"- Remove the 4 unreachable methods (~80 LOC total)\n- Verify no callers: `grep -r \"GetLastImportTime\\|SetLastImportTime\\|findJSONLPath\" .`\n- All tests pass: `go test ./internal/rpc/...`\n- Daemon functionality works: test daemon start/stop/operations","status":"closed","priority":2,"issue_type":"task","created_at":"2025-10-27T20:30:19.962209-07:00","updated_at":"2025-10-28T16:07:26.103703-07:00","closed_at":"2025-10-28T16:07:26.103703-07:00","labels":["cleanup","dead-code","phase-1","rpc"],"dependencies":[{"issue_id":"bd-17","depends_on_id":"bd-26","type":"parent-child","created_at":"2025-10-27T20:30:19.965239-07:00","created_by":"daemon"}]} {"id":"bd-170","content_hash":"9a2120c5d56a818ae4d0b2acc3518b6705d62b6cb866703dd524e3bc1a462397","title":"CLI accepts hash ID prefixes (Git-style)","description":"Implement prefix-optional parsing for hash IDs throughout CLI.\n\n## Parsing Logic\n```go\n// internal/utils/id_parser.go\nfunc ParseIssueID(input string) (issueID string, err error) {\n // Already has prefix: bd-a3f8e9 or bd-a3f8e9.1.2\n if strings.HasPrefix(input, \"bd-\") {\n return input, nil\n }\n \n // Add prefix: a3f8e9 → bd-a3f8e9\n // Works with hierarchical too: a3f8e9.1.2 → bd-a3f8e9.1.2\n return \"bd-\" + input, nil\n}\n\n// Prefix matching for partial IDs\nfunc ResolvePartialID(input string) (fullID string, err error) {\n parsedID := ParseIssueID(input) // Ensure prefix\n \n // Query with LIKE for prefix match\n matches := storage.FindByPrefix(parsedID)\n \n if len(matches) == 0 {\n return \"\", fmt.Errorf(\"no issue found matching %q\", input)\n }\n if len(matches) \u003e 1 {\n return \"\", fmt.Errorf(\"ambiguous ID %q matches: %v\", input, matches)\n }\n return matches[0].ID, nil\n}\n```\n\n## Behavior Examples\n```bash\n# All these inputs work:\nbd show a3f8e9 # Add prefix → bd-a3f8e9\nbd show bd-a3f8e9 # Already has prefix\nbd show a3f8 # Shorter prefix match\nbd show a3f8e9.1 # Hierarchical without prefix\nbd show bd-a3f8e9.1.2 # Full hierarchical with prefix\n\n# Output always shows full ID with prefix:\nbd-a3f8e9 [epic] Auth System\nbd-a3f8e9.1 [epic] Login Flow\nbd-a3f8e9.1.2 [task] Backend validation\n```\n\n## Error Handling\n```bash\n# Not found\n$ bd show xyz789\nError: no issue found matching \"bd-xyz789\"\n\n# Ambiguous (multiple matches)\n$ bd show a3\nError: ambiguous ID \"bd-a3\" matches: bd-a3f8e9, bd-a34721, bd-a38f92\nUse more characters to disambiguate.\n```\n\n## Commands to Update\nAll commands that accept issue IDs:\n\n### Read operations\n- bd show \u003cid\u003e\n- bd list --id \u003cids\u003e\n- bd dep tree \u003cid\u003e\n\n### Write operations\n- bd update \u003cid\u003e\n- bd close \u003cid\u003e\n- bd reopen \u003cid\u003e\n- bd label add \u003cid\u003e\n- bd dep add \u003cid1\u003e \u003cid2\u003e\n- bd comment add \u003cid\u003e\n\n### Multiple IDs\n```bash\n# All work with optional prefix:\nbd show a3f8e9 b7c2d1 # → bd-a3f8e9 bd-b7c2d1\nbd dep add a3f8e9.1 b7c2d1 # → bd-a3f8e9.1 blocks bd-b7c2d1\n```\n\n## Display Format\n**Always show full ID with prefix** in output for:\n- Copy-paste clarity\n- External reference (git commits, docs)\n- Unambiguous identification\n\n```bash\n$ bd list\nbd-a3f8e9 [P1] Auth System [epic] open\nbd-b7c2d1 [P2] Fix daemon crash [bug] open\nbd-1a2b3c4d [P3] Add logging [task] open\n```\n\n## Files to Create/Modify\n- internal/utils/id_parser.go (new - parsing logic)\n- cmd/bd/show.go\n- cmd/bd/update.go\n- cmd/bd/close.go\n- cmd/bd/reopen.go\n- cmd/bd/dep.go\n- cmd/bd/label.go\n- cmd/bd/comment.go\n- cmd/bd/list.go\n\n## Testing\n- Test prefix omitted: a3f8e9 → bd-a3f8e9\n- Test prefix included: bd-a3f8e9 → bd-a3f8e9\n- Test hierarchical: a3f8e9.1.2 → bd-a3f8e9.1.2\n- Test partial match: a3f8 → bd-a3f8e9\n- Test ambiguous ID error\n- Test not found error\n- Test mixed input: bd show a3f8e9 bd-b7c2d1","status":"open","priority":1,"issue_type":"task","created_at":"2025-10-29T21:25:06.256317-07:00","updated_at":"2025-10-30T00:32:47.510446-07:00","dependencies":[{"issue_id":"bd-170","depends_on_id":"bd-165","type":"parent-child","created_at":"2025-10-29T21:25:06.257796-07:00","created_by":"stevey"},{"issue_id":"bd-170","depends_on_id":"bd-167","type":"blocks","created_at":"2025-10-29T21:25:06.258307-07:00","created_by":"stevey"},{"issue_id":"bd-170","depends_on_id":"bd-169","type":"blocks","created_at":"2025-10-29T21:29:45.993274-07:00","created_by":"stevey"}]} -{"id":"bd-171","content_hash":"2a102864134b5192b5ee4e2a773cb4860b4330c9f3242b094ce8e92b01d20d80","title":"Implement hierarchical child ID generation","description":"Implement sequential child ID generation within parent contexts.\n\n## Function Signature\n```go\nfunc (s *SQLiteStorage) getNextChildID(ctx context.Context, parentID string) (string, error)\n```\n\n## Logic\n1. Insert or update child_counters for parent_id\n2. Return incremented counter\n3. Format as parentID.{counter}\n4. Works at any depth (bd-a3f8e9.1 → bd-a3f8e9.1.5)\n\n## Collision Handling\n- In single-player mode: No collisions (sequential)\n- In multi-player mode (future): Rare collisions, manual resolution needed\n- Epic ownership makes collisions naturally rare\n\n## Integration\n- Called from CreateIssue when --parent flag is used\n- Validates parent exists and depth \u003c= 3","status":"open","priority":1,"issue_type":"task","created_at":"2025-10-29T21:25:27.389191-07:00","updated_at":"2025-10-30T00:24:05.531466-07:00","dependencies":[{"issue_id":"bd-171","depends_on_id":"bd-165","type":"parent-child","created_at":"2025-10-29T21:25:27.390611-07:00","created_by":"stevey"},{"issue_id":"bd-171","depends_on_id":"bd-167","type":"blocks","created_at":"2025-10-29T21:25:27.391127-07:00","created_by":"stevey"},{"issue_id":"bd-171","depends_on_id":"bd-169","type":"blocks","created_at":"2025-10-29T21:25:27.39154-07:00","created_by":"stevey"}]} +{"id":"bd-171","content_hash":"e81f6af609fb707773b386ead4defedd146fc0f937eb60e6bfcf3bb63224237d","title":"Implement hierarchical child ID generation","description":"Implement sequential child ID generation within parent contexts.\n\n## Function Signature\n```go\nfunc (s *SQLiteStorage) getNextChildID(ctx context.Context, parentID string) (string, error)\n```\n\n## Logic\n1. Insert or update child_counters for parent_id\n2. Return incremented counter\n3. Format as parentID.{counter}\n4. Works at any depth (bd-a3f8e9.1 → bd-a3f8e9.1.5)\n\n## Collision Handling\n- In single-player mode: No collisions (sequential)\n- In multi-player mode (future): Rare collisions, manual resolution needed\n- Epic ownership makes collisions naturally rare\n\n## Integration\n- Called from CreateIssue when --parent flag is used\n- Validates parent exists and depth \u003c= 3","status":"closed","priority":1,"issue_type":"task","created_at":"2025-10-29T21:25:27.389191-07:00","updated_at":"2025-10-30T14:39:56.341051-07:00","closed_at":"2025-10-30T14:39:56.341051-07:00","dependencies":[{"issue_id":"bd-171","depends_on_id":"bd-165","type":"parent-child","created_at":"2025-10-29T21:25:27.390611-07:00","created_by":"stevey"},{"issue_id":"bd-171","depends_on_id":"bd-167","type":"blocks","created_at":"2025-10-29T21:25:27.391127-07:00","created_by":"stevey"},{"issue_id":"bd-171","depends_on_id":"bd-169","type":"blocks","created_at":"2025-10-29T21:25:27.39154-07:00","created_by":"stevey"}]} {"id":"bd-172","content_hash":"cb24777a804129f91ae8d96937c762ba4877e2a0273d389d099f678ed2080a54","title":"Delete collision resolution code","description":"Remove ~2,100 LOC of ID collision detection and resolution code (no longer needed with hash IDs).\n\n## Files to Delete Entirely\n```\ninternal/storage/sqlite/collision.go (~800 LOC)\ninternal/storage/sqlite/collision_test.go (~300 LOC)\ncmd/bd/autoimport_collision_test.go (~400 LOC)\n```\n\n## Code to Remove from Existing Files\n\n### internal/importer/importer.go\nRemove:\n- `DetectCollisions()` calls\n- `ScoreCollisions()` logic\n- `RemapCollisions()` calls\n- `handleRename()` function\n- All collision-related error handling\n\nKeep:\n- Basic import logic\n- Exact match detection (idempotent import)\n\n### beads_twoclone_test.go\nRemove:\n- `TestTwoCloneCollision` (bd-86)\n- `TestThreeCloneCollision` (bd-185)\n- `TestFiveCloneCollision` (bd-151)\n- All N-way collision tests\n\n### cmd/bd/import.go\nRemove:\n- `--resolve-collisions` flag\n- `--dry-run` collision preview\n- Collision reporting\n\n## Issues Closed by This Change\n- bd-86: Add test for symmetric collision\n--89: Content-hash collision resolution\n- bd-185: N-way collision resolution epic\n- bd-95: Add ScoreCollisions (already done but now unnecessary)\n- bd-96: Make DetectCollisions read-only\n- bd-97: ResolveNWayCollisions function\n- bd-98: Multi-round import convergence\n- bd-108: Multi-round convergence for N-way collisions\n- bd-109: Transaction + retry logic for collisions\n- bd-160: Test case for symmetric collision\n\n## Verification Steps\n1. `grep -r \"collision\" --include=\"*.go\"` → should only find alias conflicts\n2. `go test ./...` → all tests pass\n3. `go build ./cmd/bd` → clean build\n4. Check LOC reduction: `git diff --stat`\n\n## Expected Metrics\n- **Files deleted**: 3\n- **LOC removed**: ~2,100\n- **Test coverage**: Should increase (less untested code)\n- **Binary size**: Slightly smaller\n\n## Caution\nDo NOT delete:\n- Alias conflict resolution (new code in bd-171)\n- Duplicate detection (bd-59, bd-149) - different from ID collisions\n- Merge conflict resolution (bd-65, bd-103) - git conflicts, not ID collisions\n\n## Files to Modify\n- internal/importer/importer.go (remove collision handling)\n- cmd/bd/import.go (remove --resolve-collisions flag)\n- beads_twoclone_test.go (remove collision tests)\n- Delete: internal/storage/sqlite/collision.go\n- Delete: internal/storage/sqlite/collision_test.go \n- Delete: cmd/bd/autoimport_collision_test.go\n\n## Testing\n- Ensure all remaining tests pass\n- Manual test: create issue on two clones, sync → no collisions\n- Verify error if somehow hash collision occurs (extremely unlikely)","status":"open","priority":1,"issue_type":"task","created_at":"2025-10-29T21:25:50.976383-07:00","updated_at":"2025-10-29T23:14:44.171339-07:00","dependencies":[{"issue_id":"bd-172","depends_on_id":"bd-165","type":"parent-child","created_at":"2025-10-29T21:25:50.977857-07:00","created_by":"stevey"},{"issue_id":"bd-172","depends_on_id":"bd-168","type":"blocks","created_at":"2025-10-29T21:25:50.978395-07:00","created_by":"stevey"},{"issue_id":"bd-172","depends_on_id":"bd-169","type":"blocks","created_at":"2025-10-29T21:25:50.978842-07:00","created_by":"stevey"},{"issue_id":"bd-172","depends_on_id":"bd-192","type":"blocks","created_at":"2025-10-30T14:22:59.348038-07:00","created_by":"import-remap"}]} {"id":"bd-173","content_hash":"9d78f9471bf147696d5295cc89324d3486feb4bbe16c6e89524320fab229bcd1","title":"Migration tool: sequential → hash IDs","description":"Create migration tool to convert sequential IDs to hierarchical hash-based IDs.\n\n## Command\n```bash\nbd migrate --to-hash-ids [--dry-run]\n```\n\n## Process\n1. For each top-level issue (no parent):\n - Generate hash ID from content\n - Create mapping: bd-1 → bd-a3f8e9a2\n \n2. For each child issue (has parent):\n - Find parent's new hash ID\n - Assign sequential child number based on creation order\n - bd-5 (parent: bd-1) → bd-a3f8e9a2.1\n \n3. Update all references:\n - Dependencies (blocks, parent-child)\n - Comments (issue_id foreign keys)\n - External refs (if containing old IDs)\n\n4. Preserve:\n - Creation timestamps\n - All content\n - All relationships\n - History in comments\n\n## Output\n- Mapping file: old_id → new_id (for reference)\n- Updated JSONL with new IDs\n- Migration log\n\n## Validation\n- Verify all relationships intact\n- Check no orphaned issues\n- Confirm total count unchanged\n- Test rollback procedure\n\n## Safety\n- Backup database before migration\n- Dry-run mode shows what would change\n- Rollback script provided","status":"open","priority":1,"issue_type":"task","created_at":"2025-10-29T21:26:24.563993-07:00","updated_at":"2025-10-30T00:26:03.862157-07:00","dependencies":[{"issue_id":"bd-173","depends_on_id":"bd-165","type":"parent-child","created_at":"2025-10-29T21:26:24.565325-07:00","created_by":"stevey"},{"issue_id":"bd-173","depends_on_id":"bd-168","type":"blocks","created_at":"2025-10-29T21:26:24.565945-07:00","created_by":"stevey"},{"issue_id":"bd-173","depends_on_id":"bd-192","type":"blocks","created_at":"2025-10-30T14:22:59.348502-07:00","created_by":"import-remap"}]} {"id":"bd-174","content_hash":"07d57a6c273c712250bbb96ca4db01c0845b4aa054c879f023c25e4e1fd48789","title":"Add hierarchy visualization commands","description":"Add commands to visualize and navigate hierarchical issue structures.\n\n## Commands\n\n### bd tree \u003cid\u003e\nShow hierarchical tree view:\n```\nbd tree a3f8e9\n\nbd-a3f8e9 [epic] Auth System\n├─ bd-a3f8e9.1 [epic] Login Flow\n│ ├─ bd-a3f8e9.1.1 [task] Design login UI ✓\n│ ├─ bd-a3f8e9.1.2 [task] Backend validation (in progress)\n│ └─ bd-a3f8e9.1.3 [task] Integration tests\n├─ bd-a3f8e9.2 [epic] Password Reset ✓\n└─ bd-a3f8e9.3 [task] Update documentation\n```\n\n### bd stats \u003cid\u003e --recursive\nShow progress statistics:\n```\nAuth System (bd-a3f8e9): 7/27 complete (25%)\n Login Flow: 2/3 complete (67%)\n Password Reset: 3/3 complete (100%) ✓\n Documentation: 2/21 complete (10%)\n```\n\n### bd list \u003cid\u003e --leaves\nShow only leaf nodes (actual work):\n```\nbd list a3f8e9 --leaves\n\nbd-a3f8e9.1.1 [task] Design login UI\nbd-a3f8e9.1.2 [task] Backend validation\n...\n```\n\n## Sort Order\n- Implement numeric comparison of ID components\n- Ensure bd-a3f8e9.10 comes after bd-a3f8e9.9 (not lexicographic)","status":"open","priority":2,"issue_type":"task","created_at":"2025-10-29T21:26:53.751795-07:00","updated_at":"2025-10-30T00:25:24.186868-07:00","dependencies":[{"issue_id":"bd-174","depends_on_id":"bd-165","type":"parent-child","created_at":"2025-10-29T21:26:53.753259-07:00","created_by":"stevey"},{"issue_id":"bd-174","depends_on_id":"bd-170","type":"blocks","created_at":"2025-10-29T21:26:53.753733-07:00","created_by":"stevey"},{"issue_id":"bd-174","depends_on_id":"bd-171","type":"blocks","created_at":"2025-10-29T21:26:53.754112-07:00","created_by":"stevey"}]} diff --git a/cmd/bd/create.go b/cmd/bd/create.go index 8f25ae45..72e746e3 100644 --- a/cmd/bd/create.go +++ b/cmd/bd/create.go @@ -60,28 +60,54 @@ var createCmd = &cobra.Command{ assignee, _ := cmd.Flags().GetString("assignee") labels, _ := cmd.Flags().GetStringSlice("labels") explicitID, _ := cmd.Flags().GetString("id") + parentID, _ := cmd.Flags().GetString("parent") externalRef, _ := cmd.Flags().GetString("external-ref") deps, _ := cmd.Flags().GetStringSlice("deps") forceCreate, _ := cmd.Flags().GetBool("force") jsonOutput, _ := cmd.Flags().GetBool("json") - // Validate explicit ID format if provided (prefix-number) - if explicitID != "" { - // Check format: must contain hyphen and have numeric suffix - parts := strings.Split(explicitID, "-") - if len(parts) != 2 { - fmt.Fprintf(os.Stderr, "Error: invalid ID format '%s' (expected format: prefix-number, e.g., 'bd-42')\n", explicitID) + // Check for conflicting flags + if explicitID != "" && parentID != "" { + fmt.Fprintf(os.Stderr, "Error: cannot specify both --id and --parent flags\n") + os.Exit(1) + } + + // If parent is specified, generate child ID + if parentID != "" { + ctx := context.Background() + var childID string + var err error + + if daemonClient != nil { + // TODO: Add RPC support for GetNextChildID (bd-171) + fmt.Fprintf(os.Stderr, "Error: --parent flag not yet supported in daemon mode\n") os.Exit(1) + } else { + // Direct mode - use storage + childID, err = store.GetNextChildID(ctx, parentID) + if err != nil { + fmt.Fprintf(os.Stderr, "Error: %v\n", err) + os.Exit(1) + } } - // Validate numeric suffix - if _, err := fmt.Sscanf(parts[1], "%d", new(int)); err != nil { - fmt.Fprintf(os.Stderr, "Error: invalid ID format '%s' (numeric suffix required, e.g., 'bd-42')\n", explicitID) + explicitID = childID // Set as explicit ID for the rest of the flow + } + + // Validate explicit ID format if provided + // Supports: prefix-number (bd-42), prefix-hash (bd-a3f8e9), or hierarchical (bd-a3f8e9.1) + if explicitID != "" { + // Must contain hyphen + if !strings.Contains(explicitID, "-") { + fmt.Fprintf(os.Stderr, "Error: invalid ID format '%s' (expected format: prefix-hash or prefix-hash.number, e.g., 'bd-a3f8e9' or 'bd-a3f8e9.1')\n", explicitID) os.Exit(1) } + // Extract prefix (before the first hyphen) + hyphenIdx := strings.Index(explicitID, "-") + requestedPrefix := explicitID[:hyphenIdx] + // Validate prefix matches database prefix (unless --force is used) if !forceCreate { - requestedPrefix := parts[0] ctx := context.Background() // Get database prefix from config @@ -96,8 +122,7 @@ var createCmd = &cobra.Command{ if dbPrefix != "" && dbPrefix != requestedPrefix { fmt.Fprintf(os.Stderr, "Error: prefix mismatch detected\n") - fmt.Fprintf(os.Stderr, " This database uses prefix '%s-', but you specified '%s-'\n", dbPrefix, requestedPrefix) - fmt.Fprintf(os.Stderr, " Did you mean to create '%s-%s'?\n", dbPrefix, parts[1]) + fmt.Fprintf(os.Stderr, " This database uses prefix '%s', but you specified '%s'\n", dbPrefix, requestedPrefix) fmt.Fprintf(os.Stderr, " Use --force to create with mismatched prefix anyway\n") os.Exit(1) } @@ -243,6 +268,7 @@ func init() { createCmd.Flags().StringP("assignee", "a", "", "Assignee") createCmd.Flags().StringSliceP("labels", "l", []string{}, "Labels (comma-separated)") createCmd.Flags().String("id", "", "Explicit issue ID (e.g., 'bd-42' for partitioning)") + createCmd.Flags().String("parent", "", "Parent issue ID for hierarchical child (e.g., 'bd-a3f8e9')") createCmd.Flags().String("external-ref", "", "External reference (e.g., 'gh-9', 'jira-ABC')") createCmd.Flags().StringSlice("deps", []string{}, "Dependencies in format 'type:id' or 'id' (e.g., 'discovered-from:bd-20,blocks:bd-15' or 'bd-20')") createCmd.Flags().Bool("force", false, "Force creation even if prefix doesn't match database prefix") diff --git a/internal/storage/memory/memory.go b/internal/storage/memory/memory.go index b5766019..5267444c 100644 --- a/internal/storage/memory/memory.go +++ b/internal/storage/memory/memory.go @@ -827,6 +827,32 @@ func (m *MemoryStorage) ClearDirtyIssuesByID(ctx context.Context, issueIDs []str return nil } +// ID Generation +func (m *MemoryStorage) GetNextChildID(ctx context.Context, parentID string) (string, error) { + m.mu.Lock() + defer m.mu.Unlock() + + // Validate parent exists + if _, exists := m.issues[parentID]; !exists { + return "", fmt.Errorf("parent issue %s does not exist", parentID) + } + + // Calculate depth (count dots) + depth := strings.Count(parentID, ".") + if depth >= 3 { + return "", fmt.Errorf("maximum hierarchy depth (3) exceeded for parent %s", parentID) + } + + // Get or initialize counter for this parent + counter := m.counters[parentID] + counter++ + m.counters[parentID] = counter + + // Format as parentID.counter + childID := fmt.Sprintf("%s.%d", parentID, counter) + return childID, nil +} + // Config func (m *MemoryStorage) SetConfig(ctx context.Context, key, value string) error { m.mu.Lock() diff --git a/internal/storage/sqlite/child_id_test.go b/internal/storage/sqlite/child_id_test.go new file mode 100644 index 00000000..92f6973a --- /dev/null +++ b/internal/storage/sqlite/child_id_test.go @@ -0,0 +1,203 @@ +package sqlite + +import ( + "context" + "os" + "testing" + + "github.com/steveyegge/beads/internal/types" +) + +func TestGetNextChildID(t *testing.T) { + tmpFile := t.TempDir() + "/test.db" + defer os.Remove(tmpFile) + store := newTestStore(t, tmpFile) + defer store.Close() + ctx := context.Background() + + // Create a parent issue with hash ID + parent := &types.Issue{ + ID: "bd-a3f8e9", + Title: "Parent Epic", + Description: "Parent issue", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeEpic, + } + if err := store.CreateIssue(ctx, parent, "test"); err != nil { + t.Fatalf("failed to create parent: %v", err) + } + + // Test: Generate first child ID + childID1, err := store.GetNextChildID(ctx, parent.ID) + if err != nil { + t.Fatalf("GetNextChildID failed: %v", err) + } + expectedID1 := "bd-a3f8e9.1" + if childID1 != expectedID1 { + t.Errorf("expected %s, got %s", expectedID1, childID1) + } + + // Test: Generate second child ID (sequential) + childID2, err := store.GetNextChildID(ctx, parent.ID) + if err != nil { + t.Fatalf("GetNextChildID failed: %v", err) + } + expectedID2 := "bd-a3f8e9.2" + if childID2 != expectedID2 { + t.Errorf("expected %s, got %s", expectedID2, childID2) + } + + // Create the first child and test nested hierarchy + child1 := &types.Issue{ + ID: childID1, + Title: "Child Task 1", + Description: "First child", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeTask, + } + if err := store.CreateIssue(ctx, child1, "test"); err != nil { + t.Fatalf("failed to create child: %v", err) + } + + // Test: Generate nested child (depth 2) + nestedID1, err := store.GetNextChildID(ctx, childID1) + if err != nil { + t.Fatalf("GetNextChildID failed for nested: %v", err) + } + expectedNested1 := "bd-a3f8e9.1.1" + if nestedID1 != expectedNested1 { + t.Errorf("expected %s, got %s", expectedNested1, nestedID1) + } + + // Create the nested child + nested1 := &types.Issue{ + ID: nestedID1, + Title: "Nested Task", + Description: "Nested child", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeTask, + } + if err := store.CreateIssue(ctx, nested1, "test"); err != nil { + t.Fatalf("failed to create nested child: %v", err) + } + + // Test: Generate third level (depth 3, maximum) + deepID1, err := store.GetNextChildID(ctx, nestedID1) + if err != nil { + t.Fatalf("GetNextChildID failed for depth 3: %v", err) + } + expectedDeep1 := "bd-a3f8e9.1.1.1" + if deepID1 != expectedDeep1 { + t.Errorf("expected %s, got %s", expectedDeep1, deepID1) + } + + // Create the deep child + deep1 := &types.Issue{ + ID: deepID1, + Title: "Deep Task", + Description: "Third level", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeTask, + } + if err := store.CreateIssue(ctx, deep1, "test"); err != nil { + t.Fatalf("failed to create deep child: %v", err) + } + + // Test: Attempt to create fourth level (should fail) + _, err = store.GetNextChildID(ctx, deepID1) + if err == nil { + t.Errorf("expected error for depth 4, got nil") + } + if err != nil && err.Error() != "maximum hierarchy depth (3) exceeded for parent bd-a3f8e9.1.1.1" { + t.Errorf("unexpected error message: %v", err) + } +} + +func TestGetNextChildID_ParentNotExists(t *testing.T) { + tmpFile := t.TempDir() + "/test.db" + defer os.Remove(tmpFile) + store := newTestStore(t, tmpFile) + defer store.Close() + ctx := context.Background() + + // Test: Attempt to get child ID for non-existent parent + _, err := store.GetNextChildID(ctx, "bd-nonexistent") + if err == nil { + t.Errorf("expected error for non-existent parent, got nil") + } + if err != nil && err.Error() != "parent issue bd-nonexistent does not exist" { + t.Errorf("unexpected error message: %v", err) + } +} + +func TestCreateIssue_HierarchicalID(t *testing.T) { + tmpFile := t.TempDir() + "/test.db" + defer os.Remove(tmpFile) + store := newTestStore(t, tmpFile) + defer store.Close() + ctx := context.Background() + + // Create parent + parent := &types.Issue{ + ID: "bd-parent1", + Title: "Parent", + Description: "Parent issue", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeEpic, + } + if err := store.CreateIssue(ctx, parent, "test"); err != nil { + t.Fatalf("failed to create parent: %v", err) + } + + // Test: Create child with explicit hierarchical ID + child := &types.Issue{ + ID: "bd-parent1.1", + Title: "Child", + Description: "Child issue", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeTask, + } + if err := store.CreateIssue(ctx, child, "test"); err != nil { + t.Fatalf("failed to create child: %v", err) + } + + // Verify child was created + retrieved, err := store.GetIssue(ctx, child.ID) + if err != nil { + t.Fatalf("failed to retrieve child: %v", err) + } + if retrieved.ID != child.ID { + t.Errorf("expected ID %s, got %s", child.ID, retrieved.ID) + } +} + +func TestCreateIssue_HierarchicalID_ParentNotExists(t *testing.T) { + tmpFile := t.TempDir() + "/test.db" + defer os.Remove(tmpFile) + store := newTestStore(t, tmpFile) + defer store.Close() + ctx := context.Background() + + // Test: Attempt to create child without parent + child := &types.Issue{ + ID: "bd-nonexistent.1", + Title: "Child", + Description: "Child issue", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeTask, + } + err := store.CreateIssue(ctx, child, "test") + if err == nil { + t.Errorf("expected error for child without parent, got nil") + } + if err != nil && err.Error() != "parent issue bd-nonexistent does not exist" { + t.Errorf("unexpected error message: %v", err) + } +} diff --git a/internal/storage/sqlite/sqlite.go b/internal/storage/sqlite/sqlite.go index 0387cee8..0f48298b 100644 --- a/internal/storage/sqlite/sqlite.go +++ b/internal/storage/sqlite/sqlite.go @@ -665,6 +665,37 @@ func (s *SQLiteStorage) getNextChildNumber(ctx context.Context, parentID string) return nextChild, nil } +// GetNextChildID generates the next hierarchical child ID for a given parent +// Returns formatted ID as parentID.{counter} (e.g., bd-a3f8e9.1 or bd-a3f8e9.1.5) +// Works at any depth (max 3 levels) +func (s *SQLiteStorage) GetNextChildID(ctx context.Context, parentID string) (string, error) { + // Validate parent exists + var count int + err := s.db.QueryRowContext(ctx, `SELECT COUNT(*) FROM issues WHERE id = ?`, parentID).Scan(&count) + if err != nil { + return "", fmt.Errorf("failed to check parent existence: %w", err) + } + if count == 0 { + return "", fmt.Errorf("parent issue %s does not exist", parentID) + } + + // Calculate current depth by counting dots + depth := strings.Count(parentID, ".") + if depth >= 3 { + return "", fmt.Errorf("maximum hierarchy depth (3) exceeded for parent %s", parentID) + } + + // Get next child number atomically + nextNum, err := s.getNextChildNumber(ctx, parentID) + if err != nil { + return "", err + } + + // Format as parentID.counter + childID := fmt.Sprintf("%s.%d", parentID, nextNum) + return childID, nil +} + // SyncAllCounters synchronizes all ID counters based on existing issues in the database // This scans all issues and updates counters to prevent ID collisions with auto-generated IDs // Note: This unconditionally overwrites counter values, allowing them to decrease after deletions @@ -807,10 +838,27 @@ func (s *SQLiteStorage) CreateIssue(ctx context.Context, issue *types.Issue, act } else { // Validate that explicitly provided ID matches the configured prefix (bd-177) // This prevents wrong-prefix bugs when IDs are manually specified + // Support both top-level (bd-a3f8e9) and hierarchical (bd-a3f8e9.1) IDs expectedPrefix := prefix + "-" if !strings.HasPrefix(issue.ID, expectedPrefix) { return fmt.Errorf("issue ID '%s' does not match configured prefix '%s'", issue.ID, prefix) } + + // For hierarchical IDs (bd-a3f8e9.1), validate parent exists + if strings.Contains(issue.ID, ".") { + // Extract parent ID (everything before the last dot) + lastDot := strings.LastIndex(issue.ID, ".") + parentID := issue.ID[:lastDot] + + var parentCount int + err = conn.QueryRowContext(ctx, `SELECT COUNT(*) FROM issues WHERE id = ?`, parentID).Scan(&parentCount) + if err != nil { + return fmt.Errorf("failed to check parent existence: %w", err) + } + if parentCount == 0 { + return fmt.Errorf("parent issue %s does not exist", parentID) + } + } } // Insert issue diff --git a/internal/storage/storage.go b/internal/storage/storage.go index 2b24626e..980573ef 100644 --- a/internal/storage/storage.go +++ b/internal/storage/storage.go @@ -65,6 +65,9 @@ type Storage interface { GetJSONLFileHash(ctx context.Context) (string, error) SetJSONLFileHash(ctx context.Context, fileHash string) error + // ID Generation + GetNextChildID(ctx context.Context, parentID string) (string, error) + // Config SetConfig(ctx context.Context, key, value string) error GetConfig(ctx context.Context, key string) (string, error)