diff --git a/internal/beads/beads_queue.go b/internal/beads/beads_queue.go index 3c5daf6e..9cc54ec6 100644 --- a/internal/beads/beads_queue.go +++ b/internal/beads/beads_queue.go @@ -14,6 +14,7 @@ import ( // These are stored as "key: value" lines in the description. type QueueFields struct { Name string // Queue name (human-readable identifier) + ClaimPattern string // Pattern for who can claim from queue (e.g., "gastown/polecats/*") Status string // active, paused, closed MaxConcurrency int // Maximum number of concurrent workers (0 = unlimited) ProcessingOrder string // fifo, priority (default: fifo) @@ -52,6 +53,12 @@ func FormatQueueDescription(title string, fields *QueueFields) string { lines = append(lines, "name: null") } + if fields.ClaimPattern != "" { + lines = append(lines, fmt.Sprintf("claim_pattern: %s", fields.ClaimPattern)) + } else { + lines = append(lines, "claim_pattern: *") // Default: anyone can claim + } + if fields.Status != "" { lines = append(lines, fmt.Sprintf("status: %s", fields.Status)) } else { @@ -79,6 +86,7 @@ func ParseQueueFields(description string) *QueueFields { fields := &QueueFields{ Status: QueueStatusActive, ProcessingOrder: QueueOrderFIFO, + ClaimPattern: "*", // Default: anyone can claim } for _, line := range strings.Split(description, "\n") { @@ -101,6 +109,10 @@ func ParseQueueFields(description string) *QueueFields { switch strings.ToLower(key) { case "name": fields.Name = value + case "claim_pattern": + if value != "" { + fields.ClaimPattern = value + } case "status": fields.Status = value case "max_concurrency": @@ -266,3 +278,72 @@ func (b *Beads) DeleteQueueBead(id string) error { _, err := b.run("delete", id, "--hard", "--force") return err } + +// MatchClaimPattern checks if an identity matches a claim pattern. +// Patterns support: +// - "*" matches anyone +// - "gastown/polecats/*" matches any polecat in gastown rig +// - "*/witness" matches any witness role across rigs +// - Exact match for specific identities +func MatchClaimPattern(pattern, identity string) bool { + // Wildcard matches anyone + if pattern == "*" { + return true + } + + // Exact match + if pattern == identity { + return true + } + + // Wildcard pattern matching + if strings.Contains(pattern, "*") { + // Convert to simple glob matching + // "gastown/polecats/*" should match "gastown/polecats/capable" + // "*/witness" should match "gastown/witness" + parts := strings.Split(pattern, "*") + if len(parts) == 2 { + prefix := parts[0] + suffix := parts[1] + if strings.HasPrefix(identity, prefix) && strings.HasSuffix(identity, suffix) { + // Check that the middle part doesn't contain path separators + // unless the pattern allows it (e.g., "*/" at start) + middle := identity[len(prefix) : len(identity)-len(suffix)] + // Only allow single segment match (no extra slashes) + if !strings.Contains(middle, "/") { + return true + } + } + } + } + + return false +} + +// FindEligibleQueues returns all queue beads that the given identity can claim from. +func (b *Beads) FindEligibleQueues(identity string) ([]*Issue, []*QueueFields, error) { + queues, err := b.ListQueueBeads() + if err != nil { + return nil, nil, err + } + + var eligibleIssues []*Issue + var eligibleFields []*QueueFields + + for _, issue := range queues { + fields := ParseQueueFields(issue.Description) + + // Skip inactive queues + if fields.Status != QueueStatusActive { + continue + } + + // Check if identity matches claim pattern + if MatchClaimPattern(fields.ClaimPattern, identity) { + eligibleIssues = append(eligibleIssues, issue) + eligibleFields = append(eligibleFields, fields) + } + } + + return eligibleIssues, eligibleFields, nil +} diff --git a/internal/beads/beads_queue_test.go b/internal/beads/beads_queue_test.go new file mode 100644 index 00000000..251fda9f --- /dev/null +++ b/internal/beads/beads_queue_test.go @@ -0,0 +1,301 @@ +package beads + +import ( + "strings" + "testing" +) + +func TestMatchClaimPattern(t *testing.T) { + tests := []struct { + name string + pattern string + identity string + want bool + }{ + // Wildcard matches anyone + { + name: "wildcard matches anyone", + pattern: "*", + identity: "gastown/crew/max", + want: true, + }, + { + name: "wildcard matches town-level agent", + pattern: "*", + identity: "mayor/", + want: true, + }, + + // Exact match + { + name: "exact match", + pattern: "gastown/crew/max", + identity: "gastown/crew/max", + want: true, + }, + { + name: "exact match fails on different identity", + pattern: "gastown/crew/max", + identity: "gastown/crew/nux", + want: false, + }, + + // Suffix wildcard + { + name: "suffix wildcard matches", + pattern: "gastown/polecats/*", + identity: "gastown/polecats/capable", + want: true, + }, + { + name: "suffix wildcard matches different name", + pattern: "gastown/polecats/*", + identity: "gastown/polecats/nux", + want: true, + }, + { + name: "suffix wildcard doesn't match nested path", + pattern: "gastown/polecats/*", + identity: "gastown/polecats/sub/capable", + want: false, + }, + { + name: "suffix wildcard doesn't match different rig", + pattern: "gastown/polecats/*", + identity: "bartertown/polecats/capable", + want: false, + }, + + // Prefix wildcard + { + name: "prefix wildcard matches", + pattern: "*/witness", + identity: "gastown/witness", + want: true, + }, + { + name: "prefix wildcard matches different rig", + pattern: "*/witness", + identity: "bartertown/witness", + want: true, + }, + { + name: "prefix wildcard doesn't match different role", + pattern: "*/witness", + identity: "gastown/refinery", + want: false, + }, + + // Crew patterns + { + name: "crew wildcard", + pattern: "gastown/crew/*", + identity: "gastown/crew/max", + want: true, + }, + { + name: "crew wildcard matches any crew member", + pattern: "gastown/crew/*", + identity: "gastown/crew/jack", + want: true, + }, + + // Edge cases + { + name: "empty identity doesn't match", + pattern: "*", + identity: "", + want: true, // * matches anything + }, + { + name: "empty pattern doesn't match", + pattern: "", + identity: "gastown/crew/max", + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := MatchClaimPattern(tt.pattern, tt.identity) + if got != tt.want { + t.Errorf("MatchClaimPattern(%q, %q) = %v, want %v", + tt.pattern, tt.identity, got, tt.want) + } + }) + } +} + +func TestFormatQueueDescription(t *testing.T) { + tests := []struct { + name string + title string + fields *QueueFields + want []string // Lines that should be present + }{ + { + name: "basic queue", + title: "Queue: work-requests", + fields: &QueueFields{ + Name: "work-requests", + ClaimPattern: "gastown/crew/*", + Status: QueueStatusActive, + }, + want: []string{ + "Queue: work-requests", + "name: work-requests", + "claim_pattern: gastown/crew/*", + "status: active", + }, + }, + { + name: "queue with default claim pattern", + title: "Queue: public", + fields: &QueueFields{ + Name: "public", + Status: QueueStatusActive, + }, + want: []string{ + "name: public", + "claim_pattern: *", // Default + "status: active", + }, + }, + { + name: "queue with counts", + title: "Queue: processing", + fields: &QueueFields{ + Name: "processing", + ClaimPattern: "*/refinery", + Status: QueueStatusActive, + AvailableCount: 5, + ProcessingCount: 2, + CompletedCount: 10, + FailedCount: 1, + }, + want: []string{ + "name: processing", + "claim_pattern: */refinery", + "available_count: 5", + "processing_count: 2", + "completed_count: 10", + "failed_count: 1", + }, + }, + { + name: "nil fields", + title: "Just Title", + fields: nil, + want: []string{"Just Title"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := FormatQueueDescription(tt.title, tt.fields) + for _, line := range tt.want { + if !strings.Contains(got, line) { + t.Errorf("FormatQueueDescription() missing line %q in:\n%s", line, got) + } + } + }) + } +} + +func TestParseQueueFields(t *testing.T) { + tests := []struct { + name string + description string + wantName string + wantPattern string + wantStatus string + }{ + { + name: "basic queue", + description: `Queue: work-requests + +name: work-requests +claim_pattern: gastown/crew/* +status: active`, + wantName: "work-requests", + wantPattern: "gastown/crew/*", + wantStatus: QueueStatusActive, + }, + { + name: "queue with defaults", + description: `Queue: minimal + +name: minimal`, + wantName: "minimal", + wantPattern: "*", // Default + wantStatus: QueueStatusActive, + }, + { + name: "empty description", + description: "", + wantName: "", + wantPattern: "*", // Default + wantStatus: QueueStatusActive, + }, + { + name: "queue with counts", + description: `Queue: processing + +name: processing +claim_pattern: */refinery +status: paused +available_count: 5 +processing_count: 2`, + wantName: "processing", + wantPattern: "*/refinery", + wantStatus: QueueStatusPaused, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := ParseQueueFields(tt.description) + if got.Name != tt.wantName { + t.Errorf("Name = %q, want %q", got.Name, tt.wantName) + } + if got.ClaimPattern != tt.wantPattern { + t.Errorf("ClaimPattern = %q, want %q", got.ClaimPattern, tt.wantPattern) + } + if got.Status != tt.wantStatus { + t.Errorf("Status = %q, want %q", got.Status, tt.wantStatus) + } + }) + } +} + +func TestQueueBeadID(t *testing.T) { + tests := []struct { + name string + queueName string + isTownLevel bool + want string + }{ + { + name: "town-level queue", + queueName: "dispatch", + isTownLevel: true, + want: "hq-q-dispatch", + }, + { + name: "rig-level queue", + queueName: "merge", + isTownLevel: false, + want: "gt-q-merge", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := QueueBeadID(tt.queueName, tt.isTownLevel) + if got != tt.want { + t.Errorf("QueueBeadID(%q, %v) = %q, want %q", + tt.queueName, tt.isTownLevel, got, tt.want) + } + }) + } +} diff --git a/internal/cmd/mail.go b/internal/cmd/mail.go index 5538f940..91b7673c 100644 --- a/internal/cmd/mail.go +++ b/internal/cmd/mail.go @@ -277,27 +277,27 @@ Examples: } var mailClaimCmd = &cobra.Command{ - Use: "claim ", + Use: "claim [queue-name]", Short: "Claim a message from a queue", Long: `Claim the oldest unclaimed message from a work queue. SYNTAX: - gt mail claim + gt mail claim [queue-name] BEHAVIOR: -1. List unclaimed messages in the queue -2. Pick the oldest unclaimed message -3. Set assignee to caller identity -4. Set status to in_progress -5. Print claimed message details +1. If queue specified, claim from that queue +2. If no queue specified, claim from any eligible queue +3. Add claimed-by and claimed-at labels to the message +4. Print claimed message details ELIGIBILITY: -The caller must match a pattern in the queue's workers list -(defined in ~/gt/config/messaging.json). +The caller must match the queue's claim_pattern (stored in the queue bead). +Pattern examples: "*" (anyone), "gastown/polecats/*" (specific rig crew). Examples: - gt mail claim work/gastown # Claim from gastown work queue`, - Args: cobra.ExactArgs(1), + gt mail claim work-requests # Claim from specific queue + gt mail claim # Claim from any eligible queue`, + Args: cobra.MaximumNArgs(1), RunE: runMailClaim, } @@ -311,14 +311,14 @@ SYNTAX: BEHAVIOR: 1. Find the message by ID -2. Verify caller is the one who claimed it (assignee matches) -3. Set assignee back to queue: (from message labels) -4. Set status back to open -5. Message returns to queue for others to claim +2. Verify caller is the one who claimed it (claimed-by label matches) +3. Remove claimed-by and claimed-at labels +4. Message returns to queue for others to claim ERROR CASES: - Message not found -- Message not claimed (still assigned to queue) +- Message is not a queue message +- Message not claimed - Caller did not claim this message Examples: diff --git a/internal/cmd/mail_queue.go b/internal/cmd/mail_queue.go index 887e689e..9d71b19a 100644 --- a/internal/cmd/mail_queue.go +++ b/internal/cmd/mail_queue.go @@ -6,52 +6,86 @@ import ( "fmt" "os" "os/exec" - "path/filepath" "sort" "strings" "time" "github.com/spf13/cobra" - "github.com/steveyegge/gastown/internal/config" + "github.com/steveyegge/gastown/internal/beads" "github.com/steveyegge/gastown/internal/style" "github.com/steveyegge/gastown/internal/workspace" ) // runMailClaim claims the oldest unclaimed message from a work queue. +// If a queue name is provided, claims from that specific queue. +// If no queue name is provided, claims from any queue the caller is eligible for. func runMailClaim(cmd *cobra.Command, args []string) error { - queueName := args[0] - // Find workspace townRoot, err := workspace.FindFromCwdOrError() if err != nil { return fmt.Errorf("not in a Gas Town workspace: %w", err) } - // Load queue config from messaging.json - configPath := config.MessagingConfigPath(townRoot) - cfg, err := config.LoadMessagingConfig(configPath) - if err != nil { - return fmt.Errorf("loading messaging config: %w", err) - } - - queueCfg, ok := cfg.Queues[queueName] - if !ok { - return fmt.Errorf("unknown queue: %s", queueName) - } - // Get caller identity caller := detectSender() + beadsDir := beads.ResolveBeadsDir(townRoot) + bd := beads.NewWithBeadsDir(townRoot, beadsDir) - // Check if caller is eligible (matches any pattern in workers list) - if !isEligibleWorker(caller, queueCfg.Workers) { - return fmt.Errorf("not eligible to claim from queue %s (caller: %s, workers: %v)", - queueName, caller, queueCfg.Workers) + var queueName string + var queueFields *beads.QueueFields + + if len(args) > 0 { + // Specific queue requested + queueName = args[0] + + // Look up the queue bead + queueID := beads.QueueBeadID(queueName, true) // Try town-level first + issue, fields, err := bd.GetQueueBead(queueID) + if err != nil { + return fmt.Errorf("looking up queue: %w", err) + } + if issue == nil { + // Try rig-level + queueID = beads.QueueBeadID(queueName, false) + issue, fields, err = bd.GetQueueBead(queueID) + if err != nil { + return fmt.Errorf("looking up queue: %w", err) + } + if issue == nil { + return fmt.Errorf("unknown queue: %s", queueName) + } + } + queueFields = fields + + // Check if caller is eligible + if !beads.MatchClaimPattern(queueFields.ClaimPattern, caller) { + return fmt.Errorf("not eligible to claim from queue %s (caller: %s, pattern: %s)", + queueName, caller, queueFields.ClaimPattern) + } + } else { + // No queue specified - find any queue the caller can claim from + eligibleIssues, eligibleFields, err := bd.FindEligibleQueues(caller) + if err != nil { + return fmt.Errorf("finding eligible queues: %w", err) + } + if len(eligibleIssues) == 0 { + fmt.Printf("%s No queues available for claiming (caller: %s)\n", + style.Dim.Render("○"), caller) + return nil + } + + // Use the first eligible queue + queueFields = eligibleFields[0] + queueName = queueFields.Name + if queueName == "" { + // Fallback to ID-based name + queueName = eligibleIssues[0].ID + } } // List unclaimed messages in the queue - // Queue messages have assignee=queue: and status=open - queueAssignee := "queue:" + queueName - messages, err := listQueueMessages(townRoot, queueAssignee) + // Queue messages have queue: label and no claimed-by label + messages, err := listUnclaimedQueueMessages(beadsDir, queueName) if err != nil { return fmt.Errorf("listing queue messages: %w", err) } @@ -64,8 +98,8 @@ func runMailClaim(cmd *cobra.Command, args []string) error { // Pick the oldest unclaimed message (first in list, sorted by created) oldest := messages[0] - // Claim the message: set assignee to caller and status to in_progress - if err := claimMessage(townRoot, oldest.ID, caller); err != nil { + // Claim the message: add claimed-by and claimed-at labels + if err := claimQueueMessage(beadsDir, oldest.ID, caller); err != nil { return fmt.Errorf("claiming message: %w", err) } @@ -96,60 +130,18 @@ type queueMessage struct { From string Created time.Time Priority int + ClaimedBy string + ClaimedAt *time.Time } -// isEligibleWorker checks if the caller matches any pattern in the workers list. -// Patterns support wildcards: "gastown/polecats/*" matches "gastown/polecats/capable". -func isEligibleWorker(caller string, patterns []string) bool { - for _, pattern := range patterns { - if matchWorkerPattern(pattern, caller) { - return true - } - } - return false -} - -// matchWorkerPattern checks if caller matches the pattern. -// Supports simple wildcards: * matches a single path segment (no slashes). -func matchWorkerPattern(pattern, caller string) bool { - // Handle exact match - if pattern == caller { - return true - } - - // Handle wildcard patterns - if strings.Contains(pattern, "*") { - // Convert to simple glob matching - // "gastown/polecats/*" should match "gastown/polecats/capable" - // but NOT "gastown/polecats/sub/capable" - parts := strings.Split(pattern, "*") - if len(parts) == 2 { - prefix := parts[0] - suffix := parts[1] - if strings.HasPrefix(caller, prefix) && strings.HasSuffix(caller, suffix) { - // Check that the middle part doesn't contain path separators - middle := caller[len(prefix) : len(caller)-len(suffix)] - if !strings.Contains(middle, "/") { - return true - } - } - } - } - - return false -} - -// listQueueMessages lists unclaimed messages in a queue. -func listQueueMessages(townRoot, queueAssignee string) ([]queueMessage, error) { - // Use bd list to find messages with assignee=queue: and status=open - beadsDir := filepath.Join(townRoot, ".beads") - +// listUnclaimedQueueMessages lists unclaimed messages in a queue. +// Unclaimed messages have queue: label but no claimed-by label. +func listUnclaimedQueueMessages(beadsDir, queueName string) ([]queueMessage, error) { + // Use bd list to find messages with queue: label and status=open args := []string{"list", - "--assignee", queueAssignee, + "--label", "queue:" + queueName, "--status", "open", "--type", "message", - "--sort", "created", - "--limit", "0", // No limit "--json", } @@ -186,7 +178,7 @@ func listQueueMessages(townRoot, queueAssignee string) ([]queueMessage, error) { return nil, fmt.Errorf("parsing bd output: %w", err) } - // Convert to queueMessage, extracting 'from' from labels + // Convert to queueMessage, filtering out already claimed messages var messages []queueMessage for _, issue := range issues { msg := queueMessage{ @@ -197,18 +189,27 @@ func listQueueMessages(townRoot, queueAssignee string) ([]queueMessage, error) { Priority: issue.Priority, } - // Extract 'from' from labels (format: "from:address") + // Extract labels for _, label := range issue.Labels { if strings.HasPrefix(label, "from:") { msg.From = strings.TrimPrefix(label, "from:") - break + } else if strings.HasPrefix(label, "claimed-by:") { + msg.ClaimedBy = strings.TrimPrefix(label, "claimed-by:") + } else if strings.HasPrefix(label, "claimed-at:") { + ts := strings.TrimPrefix(label, "claimed-at:") + if t, err := time.Parse(time.RFC3339, ts); err == nil { + msg.ClaimedAt = &t + } } } - messages = append(messages, msg) + // Only include unclaimed messages + if msg.ClaimedBy == "" { + messages = append(messages, msg) + } } - // Sort by created time (oldest first) + // Sort by created time (oldest first) for FIFO ordering sort.Slice(messages, func(i, j int) bool { return messages[i].Created.Before(messages[j].Created) }) @@ -216,13 +217,13 @@ func listQueueMessages(townRoot, queueAssignee string) ([]queueMessage, error) { return messages, nil } -// claimMessage claims a message by setting assignee and status. -func claimMessage(townRoot, messageID, claimant string) error { - beadsDir := filepath.Join(townRoot, ".beads") +// claimQueueMessage claims a message by adding claimed-by and claimed-at labels. +func claimQueueMessage(beadsDir, messageID, claimant string) error { + now := time.Now().UTC().Format(time.RFC3339) - args := []string{"update", messageID, - "--assignee", claimant, - "--status", "in_progress", + args := []string{"label", "add", messageID, + "claimed-by:" + claimant, + "claimed-at:" + now, } cmd := exec.Command("bd", args...) @@ -255,11 +256,13 @@ func runMailRelease(cmd *cobra.Command, args []string) error { return fmt.Errorf("not in a Gas Town workspace: %w", err) } + beadsDir := beads.ResolveBeadsDir(townRoot) + // Get caller identity caller := detectSender() // Get message details to verify ownership and find queue - msgInfo, err := getMessageInfo(townRoot, messageID) + msgInfo, err := getQueueMessageInfo(beadsDir, messageID) if err != nil { return fmt.Errorf("getting message: %w", err) } @@ -270,16 +273,15 @@ func runMailRelease(cmd *cobra.Command, args []string) error { } // Verify caller is the one who claimed it - if msgInfo.Assignee != caller { - if strings.HasPrefix(msgInfo.Assignee, "queue:") { - return fmt.Errorf("message %s is not claimed (still in queue)", messageID) - } - return fmt.Errorf("message %s was claimed by %s, not %s", messageID, msgInfo.Assignee, caller) + if msgInfo.ClaimedBy == "" { + return fmt.Errorf("message %s is not claimed", messageID) + } + if msgInfo.ClaimedBy != caller { + return fmt.Errorf("message %s was claimed by %s, not %s", messageID, msgInfo.ClaimedBy, caller) } - // Release the message: set assignee back to queue and status to open - queueAssignee := "queue:" + msgInfo.QueueName - if err := releaseMessage(townRoot, messageID, queueAssignee, caller); err != nil { + // Release the message: remove claimed-by and claimed-at labels + if err := releaseQueueMessage(beadsDir, messageID, caller); err != nil { return fmt.Errorf("releasing message: %w", err) } @@ -290,19 +292,18 @@ func runMailRelease(cmd *cobra.Command, args []string) error { return nil } -// messageInfo holds details about a queue message. -type messageInfo struct { +// queueMessageInfo holds details about a queue message. +type queueMessageInfo struct { ID string Title string - Assignee string QueueName string + ClaimedBy string + ClaimedAt *time.Time Status string } -// getMessageInfo retrieves information about a message. -func getMessageInfo(townRoot, messageID string) (*messageInfo, error) { - beadsDir := filepath.Join(townRoot, ".beads") - +// getQueueMessageInfo retrieves information about a queue message. +func getQueueMessageInfo(beadsDir, messageID string) (*queueMessageInfo, error) { args := []string{"show", messageID, "--json"} cmd := exec.Command("bd", args...) @@ -327,7 +328,6 @@ func getMessageInfo(townRoot, messageID string) (*messageInfo, error) { var issues []struct { ID string `json:"id"` Title string `json:"title"` - Assignee string `json:"assignee"` Labels []string `json:"labels"` Status string `json:"status"` } @@ -341,48 +341,76 @@ func getMessageInfo(townRoot, messageID string) (*messageInfo, error) { } issue := issues[0] - info := &messageInfo{ - ID: issue.ID, - Title: issue.Title, - Assignee: issue.Assignee, - Status: issue.Status, + info := &queueMessageInfo{ + ID: issue.ID, + Title: issue.Title, + Status: issue.Status, } - // Extract queue name from labels (format: "queue:") + // Extract fields from labels for _, label := range issue.Labels { if strings.HasPrefix(label, "queue:") { info.QueueName = strings.TrimPrefix(label, "queue:") - break + } else if strings.HasPrefix(label, "claimed-by:") { + info.ClaimedBy = strings.TrimPrefix(label, "claimed-by:") + } else if strings.HasPrefix(label, "claimed-at:") { + ts := strings.TrimPrefix(label, "claimed-at:") + if t, err := time.Parse(time.RFC3339, ts); err == nil { + info.ClaimedAt = &t + } } } return info, nil } -// releaseMessage releases a claimed message back to its queue. -func releaseMessage(townRoot, messageID, queueAssignee, actor string) error { - beadsDir := filepath.Join(townRoot, ".beads") - - args := []string{"update", messageID, - "--assignee", queueAssignee, - "--status", "open", +// releaseQueueMessage releases a claimed message by removing claim labels. +func releaseQueueMessage(beadsDir, messageID, actor string) error { + // Get current message info to find the exact claim labels + info, err := getQueueMessageInfo(beadsDir, messageID) + if err != nil { + return err } - cmd := exec.Command("bd", args...) - cmd.Env = append(os.Environ(), - "BEADS_DIR="+beadsDir, - "BD_ACTOR="+actor, - ) + // Remove claimed-by label + if info.ClaimedBy != "" { + args := []string{"label", "remove", messageID, "claimed-by:" + info.ClaimedBy} + cmd := exec.Command("bd", args...) + cmd.Env = append(os.Environ(), + "BEADS_DIR="+beadsDir, + "BD_ACTOR="+actor, + ) - var stderr bytes.Buffer - cmd.Stderr = &stderr + var stderr bytes.Buffer + cmd.Stderr = &stderr - if err := cmd.Run(); err != nil { - errMsg := strings.TrimSpace(stderr.String()) - if errMsg != "" { - return fmt.Errorf("%s", errMsg) + if err := cmd.Run(); err != nil { + errMsg := strings.TrimSpace(stderr.String()) + if errMsg != "" && !strings.Contains(errMsg, "does not have label") { + return fmt.Errorf("%s", errMsg) + } + } + } + + // Remove claimed-at label if present + if info.ClaimedAt != nil { + claimedAtStr := info.ClaimedAt.Format(time.RFC3339) + args := []string{"label", "remove", messageID, "claimed-at:" + claimedAtStr} + cmd := exec.Command("bd", args...) + cmd.Env = append(os.Environ(), + "BEADS_DIR="+beadsDir, + "BD_ACTOR="+actor, + ) + + var stderr bytes.Buffer + cmd.Stderr = &stderr + + if err := cmd.Run(); err != nil { + errMsg := strings.TrimSpace(stderr.String()) + if errMsg != "" && !strings.Contains(errMsg, "does not have label") { + return fmt.Errorf("%s", errMsg) + } } - return err } return nil diff --git a/internal/cmd/mail_test.go b/internal/cmd/mail_test.go index 7f302532..f2625db6 100644 --- a/internal/cmd/mail_test.go +++ b/internal/cmd/mail_test.go @@ -5,10 +5,13 @@ import ( "strings" "testing" + "github.com/steveyegge/gastown/internal/beads" "github.com/steveyegge/gastown/internal/config" ) -func TestMatchWorkerPattern(t *testing.T) { +// TestClaimPatternMatching tests claim pattern matching via the beads package. +// This verifies that the pattern matching used for queue eligibility works correctly. +func TestClaimPatternMatching(t *testing.T) { tests := []struct { name string pattern string @@ -55,43 +58,9 @@ func TestMatchWorkerPattern(t *testing.T) { want: false, }, - // Crew patterns + // Universal wildcard { - name: "crew wildcard matches", - pattern: "gastown/crew/*", - caller: "gastown/crew/max", - want: true, - }, - { - name: "crew wildcard doesn't match polecats", - pattern: "gastown/crew/*", - caller: "gastown/polecats/capable", - want: false, - }, - - // Different rigs - { - name: "different rig wildcard", - pattern: "beads/polecats/*", - caller: "beads/polecats/capable", - want: true, - }, - - // Edge cases - { - name: "empty pattern", - pattern: "", - caller: "gastown/polecats/capable", - want: false, - }, - { - name: "empty caller", - pattern: "gastown/polecats/*", - caller: "", - want: false, - }, - { - name: "pattern is just wildcard", + name: "universal wildcard matches anything", pattern: "*", caller: "anything", want: true, @@ -100,103 +69,47 @@ func TestMatchWorkerPattern(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := matchWorkerPattern(tt.pattern, tt.caller) + got := beads.MatchClaimPattern(tt.pattern, tt.caller) if got != tt.want { - t.Errorf("matchWorkerPattern(%q, %q) = %v, want %v", + t.Errorf("MatchClaimPattern(%q, %q) = %v, want %v", tt.pattern, tt.caller, got, tt.want) } }) } } -func TestIsEligibleWorker(t *testing.T) { - tests := []struct { - name string - caller string - patterns []string - want bool - }{ - { - name: "matches first pattern", - caller: "gastown/polecats/capable", - patterns: []string{"gastown/polecats/*", "gastown/crew/*"}, - want: true, - }, - { - name: "matches second pattern", - caller: "gastown/crew/max", - patterns: []string{"gastown/polecats/*", "gastown/crew/*"}, - want: true, - }, - { - name: "matches none", - caller: "beads/polecats/capable", - patterns: []string{"gastown/polecats/*", "gastown/crew/*"}, - want: false, - }, - { - name: "empty patterns list", - caller: "gastown/polecats/capable", - patterns: []string{}, - want: false, - }, - { - name: "nil patterns", - caller: "gastown/polecats/capable", - patterns: nil, - want: false, - }, - { - name: "exact match in list", - caller: "mayor/", - patterns: []string{"mayor/", "gastown/witness"}, - want: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got := isEligibleWorker(tt.caller, tt.patterns) - if got != tt.want { - t.Errorf("isEligibleWorker(%q, %v) = %v, want %v", - tt.caller, tt.patterns, got, tt.want) - } - }) - } -} - -// TestMailReleaseValidation tests the validation logic for the release command. +// TestQueueMessageReleaseValidation tests the validation logic for the release command. // This tests that release correctly identifies: -// - Messages not claimed (still in queue) +// - Messages not claimed (no claimed-by label) // - Messages claimed by a different worker // - Messages without queue labels (non-queue messages) -func TestMailReleaseValidation(t *testing.T) { +func TestQueueMessageReleaseValidation(t *testing.T) { tests := []struct { name string - msgInfo *messageInfo + msgInfo *queueMessageInfo caller string wantErr bool errContains string }{ { - name: "caller matches assignee - valid release", - msgInfo: &messageInfo{ + name: "caller matches claimed-by - valid release", + msgInfo: &queueMessageInfo{ ID: "hq-test1", Title: "Test Message", - Assignee: "gastown/polecats/nux", - QueueName: "work/gastown", - Status: "in_progress", + ClaimedBy: "gastown/polecats/nux", + QueueName: "work-requests", + Status: "open", }, caller: "gastown/polecats/nux", wantErr: false, }, { - name: "message still in queue - not claimed", - msgInfo: &messageInfo{ + name: "message not claimed", + msgInfo: &queueMessageInfo{ ID: "hq-test2", Title: "Test Message", - Assignee: "queue:work/gastown", - QueueName: "work/gastown", + ClaimedBy: "", // Not claimed + QueueName: "work-requests", Status: "open", }, caller: "gastown/polecats/nux", @@ -205,12 +118,12 @@ func TestMailReleaseValidation(t *testing.T) { }, { name: "claimed by different worker", - msgInfo: &messageInfo{ + msgInfo: &queueMessageInfo{ ID: "hq-test3", Title: "Test Message", - Assignee: "gastown/polecats/other", - QueueName: "work/gastown", - Status: "in_progress", + ClaimedBy: "gastown/polecats/other", + QueueName: "work-requests", + Status: "open", }, caller: "gastown/polecats/nux", wantErr: true, @@ -218,10 +131,10 @@ func TestMailReleaseValidation(t *testing.T) { }, { name: "not a queue message", - msgInfo: &messageInfo{ + msgInfo: &queueMessageInfo{ ID: "hq-test4", Title: "Test Message", - Assignee: "gastown/polecats/nux", + ClaimedBy: "gastown/polecats/nux", QueueName: "", // No queue label Status: "open", }, @@ -233,7 +146,7 @@ func TestMailReleaseValidation(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := validateRelease(tt.msgInfo, tt.caller) + err := validateQueueRelease(tt.msgInfo, tt.caller) if tt.wantErr { if err == nil { t.Error("expected error, got nil") @@ -251,20 +164,22 @@ func TestMailReleaseValidation(t *testing.T) { } } -// validateRelease checks if a message can be released by the caller. -// This is extracted for testing; the actual release command uses this logic inline. -func validateRelease(msgInfo *messageInfo, caller string) error { +// validateQueueRelease checks if a queue message can be released by the caller. +// This mirrors the validation logic in runMailRelease. +func validateQueueRelease(msgInfo *queueMessageInfo, caller string) error { // Verify message is a queue message if msgInfo.QueueName == "" { return fmt.Errorf("message %s is not a queue message (no queue label)", msgInfo.ID) } + // Verify message is claimed + if msgInfo.ClaimedBy == "" { + return fmt.Errorf("message %s is not claimed", msgInfo.ID) + } + // Verify caller is the one who claimed it - if msgInfo.Assignee != caller { - if strings.HasPrefix(msgInfo.Assignee, "queue:") { - return fmt.Errorf("message %s is not claimed (still in queue)", msgInfo.ID) - } - return fmt.Errorf("message %s was claimed by %s, not %s", msgInfo.ID, msgInfo.Assignee, caller) + if msgInfo.ClaimedBy != caller { + return fmt.Errorf("message %s was claimed by %s, not %s", msgInfo.ID, msgInfo.ClaimedBy, caller) } return nil