From 73d4d5ecb2d51c1bb447d5c39d3d79c7f1cf6fb1 Mon Sep 17 00:00:00 2001 From: Steven Syrek Date: Mon, 19 Jan 2026 19:08:56 +0100 Subject: [PATCH] fix(create): validate explicit IDs against allowed_prefixes using starts-with matching (#1137) When creating issues with explicit IDs like `bd create --id hq-cv-test`, the prefix validation was failing even when `hq-cv` was in `allowed_prefixes`. Root cause: `ExtractIssuePrefix("hq-cv-test")` returns `"hq"` (not `"hq-cv"`) because "test" looks like an English word, causing the algorithm to fall back to the first hyphen. The validation then checked if `"hq"` was in the allowed list containing `"hq-cv"` - which failed. The fix adds `ValidateIDPrefixAllowed()` which validates the full ID using "starts with" matching (the same approach the importer uses successfully). This correctly handles multi-hyphen prefixes like `hq-cv-` regardless of what the suffix looks like. Fixes #1135 Co-authored-by: Steven Syrek Co-authored-by: Claude Opus 4.5 --- cmd/bd/create.go | 16 +++++----- internal/validation/bead.go | 43 +++++++++++++++++++++++++++ internal/validation/bead_test.go | 50 ++++++++++++++++++++++++++++++++ 3 files changed, 100 insertions(+), 9 deletions(-) diff --git a/cmd/bd/create.go b/cmd/bd/create.go index 40924aac..295df5a7 100644 --- a/cmd/bd/create.go +++ b/cmd/bd/create.go @@ -400,20 +400,15 @@ var createCmd = &cobra.Command{ // Validate explicit ID format if provided if explicitID != "" { - var requestedPrefix string - var err error - - // For agent types, use agent-aware prefix extraction. + // For agent types, use agent-aware validation. // This fixes the bug where 3-char polecat names like "nux" in - // "nx-nexus-polecat-nux" were incorrectly treated as hash suffixes, - // causing prefix to be extracted as "nx-nexus-polecat" instead of "nx". + // "nx-nexus-polecat-nux" were incorrectly treated as hash suffixes. if issueType == "agent" { if err := validation.ValidateAgentID(explicitID); err != nil { FatalError("invalid agent ID: %v", err) } - requestedPrefix = validation.ExtractAgentPrefix(explicitID) } else { - requestedPrefix, err = validation.ValidateIDFormat(explicitID) + _, err := validation.ValidateIDFormat(explicitID) if err != nil { FatalError("%v", err) } @@ -442,7 +437,10 @@ var createCmd = &cobra.Command{ allowedPrefixes, _ = store.GetConfig(ctx, "allowed_prefixes") } - if err := validation.ValidatePrefixWithAllowed(requestedPrefix, dbPrefix, allowedPrefixes, forceCreate); err != nil { + // Use ValidateIDPrefixAllowed which handles multi-hyphen prefixes correctly (GH#1135) + // This checks if the ID starts with an allowed prefix, rather than extracting + // the prefix first (which can fail for IDs like "hq-cv-test" where "test" looks like a word) + if err := validation.ValidateIDPrefixAllowed(explicitID, dbPrefix, allowedPrefixes, forceCreate); err != nil { FatalError("%v", err) } } diff --git a/internal/validation/bead.go b/internal/validation/bead.go index 6f64d074..344b8cc1 100644 --- a/internal/validation/bead.go +++ b/internal/validation/bead.go @@ -123,6 +123,49 @@ func ValidatePrefixWithAllowed(requestedPrefix, dbPrefix, allowedPrefixes string return fmt.Errorf("prefix mismatch: database uses '%s' but you specified '%s' (use --force to override)", dbPrefix, requestedPrefix) } +// ValidateIDPrefixAllowed checks that an issue ID's prefix is allowed. +// Unlike ValidatePrefixWithAllowed which takes an extracted prefix, this function +// takes the full ID and checks if it starts with any allowed prefix. +// This correctly handles multi-hyphen prefixes like "hq-cv-" where the suffix +// might look like an English word (e.g., "hq-cv-test"). +// (GH#1135) +// +// It matches if: +// - force is true +// - dbPrefix is empty +// - id starts with dbPrefix + "-" +// - id starts with any prefix in allowedPrefixes + "-" +// Returns an error if none of these conditions are met. +func ValidateIDPrefixAllowed(id, dbPrefix, allowedPrefixes string, force bool) error { + if force || dbPrefix == "" { + return nil + } + + // Check if ID starts with the database prefix + if strings.HasPrefix(id, dbPrefix+"-") { + return nil + } + + // Check if ID starts with any allowed prefix + if allowedPrefixes != "" { + for _, allowed := range strings.Split(allowedPrefixes, ",") { + allowed = strings.TrimSpace(allowed) + // Normalize: remove trailing - if present (we add it for matching) + allowed = strings.TrimSuffix(allowed, "-") + if allowed != "" && strings.HasPrefix(id, allowed+"-") { + return nil + } + } + } + + // Build helpful error message + if allowedPrefixes != "" { + return fmt.Errorf("prefix mismatch: database uses '%s-' (allowed: %s) but ID '%s' doesn't match any allowed prefix (use --force to override)", + dbPrefix, allowedPrefixes, id) + } + return fmt.Errorf("prefix mismatch: database uses '%s-' but ID '%s' doesn't match (use --force to override)", dbPrefix, id) +} + // ValidAgentRoles are the known agent role types for ID pattern validation var ValidAgentRoles = []string{ "mayor", // Town-level: gt-mayor diff --git a/internal/validation/bead_test.go b/internal/validation/bead_test.go index cd480c69..6f56d685 100644 --- a/internal/validation/bead_test.go +++ b/internal/validation/bead_test.go @@ -334,6 +334,56 @@ func TestValidatePrefixWithAllowed(t *testing.T) { } } +// TestValidateIDPrefixAllowed tests the new function that validates IDs using +// "starts with" matching to handle multi-hyphen prefixes correctly (GH#1135). +func TestValidateIDPrefixAllowed(t *testing.T) { + tests := []struct { + name string + id string + dbPrefix string + allowedPrefixes string + force bool + wantError bool + }{ + // Basic cases + {"matching prefix", "bd-abc123", "bd", "", false, false}, + {"empty db prefix", "bd-abc123", "", "", false, false}, + {"mismatched with force", "foo-abc123", "bd", "", true, false}, + {"mismatched without force", "foo-abc123", "bd", "", false, true}, + + // Multi-hyphen prefix cases (GH#1135 - the main bug) + {"hq-cv prefix with word suffix", "hq-cv-test", "djdefi-ops", "hq,hq-cv", false, false}, + {"hq-cv prefix with hash suffix", "hq-cv-abc123", "djdefi-ops", "hq,hq-cv", false, false}, + {"djdefi-ops with word suffix", "djdefi-ops-test", "djdefi-ops", "", false, false}, + + // Allowed prefixes list + {"allowed prefix gt", "gt-abc123", "hq", "gt,hmc", false, false}, + {"allowed prefix hmc", "hmc-abc123", "hq", "gt,hmc", false, false}, + {"primary prefix still works", "hq-abc123", "hq", "gt,hmc", false, false}, + {"prefix not in allowed list", "foo-abc123", "hq", "gt,hmc", false, true}, + + // Edge cases + {"allowed with spaces", "gt-abc123", "hq", "gt, hmc, foo", false, false}, + {"allowed with trailing dash", "gt-abc123", "hq", "gt-, hmc-", false, false}, + {"empty allowed list", "gt-abc123", "hq", "", false, true}, + {"single allowed prefix", "gt-abc123", "hq", "gt", false, false}, + + // Multi-hyphen allowed prefixes + {"multi-hyphen in allowed list", "my-cool-prefix-abc123", "hq", "my-cool-prefix,other", false, false}, + {"partial match should fail", "hq-cv-extra-test", "hq", "hq-cv-extra", false, false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateIDPrefixAllowed(tt.id, tt.dbPrefix, tt.allowedPrefixes, tt.force) + if (err != nil) != tt.wantError { + t.Errorf("ValidateIDPrefixAllowed(%q, %q, %q) error = %v, wantError %v", + tt.id, tt.dbPrefix, tt.allowedPrefixes, err, tt.wantError) + } + }) + } +} + func TestValidateAgentID(t *testing.T) { tests := []struct { name string