Branch names like "polecat/furiosa-mkb0vq9f" don't contain the actual issue ID, causing gt done to incorrectly parse "furiosa-mkb0vq9f" as the issue. This broke integration branch auto-detection since the wrong issue was used for parent epic lookup. Changes: - After parsing branch name, check the agent's hook_bead field which contains the actual issue ID (e.g., "gt-845.1") - Fix parseBranchName to not extract fake issue IDs from modern polecat branches - Fix detectIntegrationBranch to traverse full parent chain (molecule → bug → epic) - Include issue ID in polecat branch names when HookBead is set Added tests covering: - Agent hook returns correct issue ID - Modern polecat branch format parsing - Integration branch detection through parent chain Fixes #411 Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -215,6 +215,16 @@ func runDone(cmd *cobra.Command, args []string) error {
|
|||||||
agentBeadID = getAgentBeadID(ctx)
|
agentBeadID = getAgentBeadID(ctx)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// If issue ID not set by flag or branch name, try agent's hook_bead.
|
||||||
|
// This handles cases where branch name doesn't contain issue ID
|
||||||
|
// (e.g., "polecat/furiosa-mkb0vq9f" doesn't have the actual issue).
|
||||||
|
if issueID == "" && agentBeadID != "" {
|
||||||
|
bd := beads.New(beads.ResolveBeadsDir(cwd))
|
||||||
|
if hookIssue := getIssueFromAgentHook(bd, agentBeadID); hookIssue != "" {
|
||||||
|
issueID = hookIssue
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// Get configured default branch for this rig
|
// Get configured default branch for this rig
|
||||||
defaultBranch := "main" // fallback
|
defaultBranch := "main" // fallback
|
||||||
if rigCfg, err := rig.LoadRigConfig(filepath.Join(townRoot, rigName)); err == nil && rigCfg.DefaultBranch != "" {
|
if rigCfg, err := rig.LoadRigConfig(filepath.Join(townRoot, rigName)); err == nil && rigCfg.DefaultBranch != "" {
|
||||||
@@ -617,6 +627,21 @@ func updateAgentStateOnDone(cwd, townRoot, exitType, _ string) { // issueID unus
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// getIssueFromAgentHook retrieves the issue ID from an agent's hook_bead field.
|
||||||
|
// This is the authoritative source for what work a polecat is doing, since branch
|
||||||
|
// names may not contain the issue ID (e.g., "polecat/furiosa-mkb0vq9f").
|
||||||
|
// Returns empty string if agent doesn't exist or has no hook.
|
||||||
|
func getIssueFromAgentHook(bd *beads.Beads, agentBeadID string) string {
|
||||||
|
if agentBeadID == "" {
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
agentBead, err := bd.Show(agentBeadID)
|
||||||
|
if err != nil {
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
return agentBead.HookBead
|
||||||
|
}
|
||||||
|
|
||||||
// getDispatcherFromBead retrieves the dispatcher agent ID from the bead's attachment fields.
|
// getDispatcherFromBead retrieves the dispatcher agent ID from the bead's attachment fields.
|
||||||
// Returns empty string if no dispatcher is recorded.
|
// Returns empty string if no dispatcher is recorded.
|
||||||
func getDispatcherFromBead(cwd, issueID string) string {
|
func getDispatcherFromBead(cwd, issueID string) string {
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ package cmd
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"os"
|
"os"
|
||||||
|
"os/exec"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
@@ -246,3 +247,92 @@ func TestDoneCircularRedirectProtection(t *testing.T) {
|
|||||||
t.Errorf("circular redirect should return original: got %s, want %s", resolved, beadsDir)
|
t.Errorf("circular redirect should return original: got %s, want %s", resolved, beadsDir)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestGetIssueFromAgentHook verifies that getIssueFromAgentHook correctly
|
||||||
|
// retrieves the issue ID from an agent's hook_bead field.
|
||||||
|
// This is critical because branch names like "polecat/furiosa-mkb0vq9f" don't
|
||||||
|
// contain the actual issue ID (test-845.1), but the agent's hook does.
|
||||||
|
func TestGetIssueFromAgentHook(t *testing.T) {
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
agentBeadID string
|
||||||
|
setupBeads func(t *testing.T, bd *beads.Beads) // setup agent bead with hook
|
||||||
|
wantIssueID string
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "agent with hook_bead returns issue ID",
|
||||||
|
agentBeadID: "test-testrig-polecat-furiosa",
|
||||||
|
setupBeads: func(t *testing.T, bd *beads.Beads) {
|
||||||
|
// Create a task that will be hooked
|
||||||
|
_, err := bd.CreateWithID("test-456", beads.CreateOptions{
|
||||||
|
Title: "Task to be hooked",
|
||||||
|
Type: "task",
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("create task bead: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Create agent bead using CreateAgentBead
|
||||||
|
// Agent ID format: <prefix>-<rig>-<role>-<name>
|
||||||
|
_, err = bd.CreateAgentBead("test-testrig-polecat-furiosa", "Test polecat agent", nil)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("create agent bead: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Set hook_bead on agent
|
||||||
|
if err := bd.SetHookBead("test-testrig-polecat-furiosa", "test-456"); err != nil {
|
||||||
|
t.Fatalf("set hook bead: %v", err)
|
||||||
|
}
|
||||||
|
},
|
||||||
|
wantIssueID: "test-456",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "agent without hook_bead returns empty",
|
||||||
|
agentBeadID: "test-testrig-polecat-idle",
|
||||||
|
setupBeads: func(t *testing.T, bd *beads.Beads) {
|
||||||
|
// Create agent bead without hook
|
||||||
|
_, err := bd.CreateAgentBead("test-testrig-polecat-idle", "Test agent without hook", nil)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("create agent bead: %v", err)
|
||||||
|
}
|
||||||
|
},
|
||||||
|
wantIssueID: "",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "nonexistent agent returns empty",
|
||||||
|
agentBeadID: "test-nonexistent",
|
||||||
|
setupBeads: func(t *testing.T, bd *beads.Beads) {},
|
||||||
|
wantIssueID: "",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "empty agent ID returns empty",
|
||||||
|
agentBeadID: "",
|
||||||
|
setupBeads: func(t *testing.T, bd *beads.Beads) {},
|
||||||
|
wantIssueID: "",
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tt := range tests {
|
||||||
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
|
tmpDir := t.TempDir()
|
||||||
|
|
||||||
|
// Initialize the beads database
|
||||||
|
cmd := exec.Command("bd", "--no-daemon", "init", "--prefix", "test", "--quiet")
|
||||||
|
cmd.Dir = tmpDir
|
||||||
|
if output, err := cmd.CombinedOutput(); err != nil {
|
||||||
|
t.Fatalf("bd init: %v\n%s", err, output)
|
||||||
|
}
|
||||||
|
|
||||||
|
// beads.New expects the .beads directory path
|
||||||
|
beadsDir := filepath.Join(tmpDir, ".beads")
|
||||||
|
bd := beads.New(beadsDir)
|
||||||
|
|
||||||
|
tt.setupBeads(t, bd)
|
||||||
|
|
||||||
|
got := getIssueFromAgentHook(bd, tt.agentBeadID)
|
||||||
|
if got != tt.wantIssueID {
|
||||||
|
t.Errorf("getIssueFromAgentHook(%q) = %q, want %q", tt.agentBeadID, got, tt.wantIssueID)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -28,16 +28,36 @@ type branchInfo struct {
|
|||||||
// parseBranchName extracts issue ID and worker from a branch name.
|
// parseBranchName extracts issue ID and worker from a branch name.
|
||||||
// Supports formats:
|
// Supports formats:
|
||||||
// - polecat/<worker>/<issue> → issue=<issue>, worker=<worker>
|
// - polecat/<worker>/<issue> → issue=<issue>, worker=<worker>
|
||||||
|
// - polecat/<worker>-<timestamp> → issue="", worker=<worker> (modern polecat branches)
|
||||||
// - <issue> → issue=<issue>, worker=""
|
// - <issue> → issue=<issue>, worker=""
|
||||||
func parseBranchName(branch string) branchInfo {
|
func parseBranchName(branch string) branchInfo {
|
||||||
info := branchInfo{Branch: branch}
|
info := branchInfo{Branch: branch}
|
||||||
|
|
||||||
// Try polecat/<worker>/<issue> format
|
// Try polecat/<worker>/<issue> or polecat/<worker>/<issue>@<timestamp> format
|
||||||
if strings.HasPrefix(branch, constants.BranchPolecatPrefix) {
|
if strings.HasPrefix(branch, constants.BranchPolecatPrefix) {
|
||||||
parts := strings.SplitN(branch, "/", 3)
|
parts := strings.SplitN(branch, "/", 3)
|
||||||
if len(parts) == 3 {
|
if len(parts) == 3 {
|
||||||
info.Worker = parts[1]
|
info.Worker = parts[1]
|
||||||
info.Issue = parts[2]
|
// Strip @timestamp suffix if present (e.g., "gt-abc@mk123" -> "gt-abc")
|
||||||
|
issue := parts[2]
|
||||||
|
if atIdx := strings.Index(issue, "@"); atIdx > 0 {
|
||||||
|
issue = issue[:atIdx]
|
||||||
|
}
|
||||||
|
info.Issue = issue
|
||||||
|
return info
|
||||||
|
}
|
||||||
|
// Modern polecat branch format: polecat/<worker>-<timestamp>
|
||||||
|
// The second part is "worker-timestamp", not an issue ID.
|
||||||
|
// Don't try to extract an issue ID - gt done will use hook_bead fallback.
|
||||||
|
if len(parts) == 2 {
|
||||||
|
// Extract worker name from "worker-timestamp" format
|
||||||
|
workerPart := parts[1]
|
||||||
|
if dashIdx := strings.LastIndex(workerPart, "-"); dashIdx > 0 {
|
||||||
|
info.Worker = workerPart[:dashIdx]
|
||||||
|
} else {
|
||||||
|
info.Worker = workerPart
|
||||||
|
}
|
||||||
|
// Explicitly don't set info.Issue - let hook_bead fallback handle it
|
||||||
return info
|
return info
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -186,54 +206,55 @@ func runMqSubmit(cmd *cobra.Command, args []string) error {
|
|||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// detectIntegrationBranch checks if an issue is a child of an epic that has an integration branch.
|
// detectIntegrationBranch checks if an issue is a descendant of an epic that has an integration branch.
|
||||||
|
// Traverses up the parent chain until it finds an epic or runs out of parents.
|
||||||
// Returns the integration branch target (e.g., "integration/gt-epic") if found, or "" if not.
|
// Returns the integration branch target (e.g., "integration/gt-epic") if found, or "" if not.
|
||||||
func detectIntegrationBranch(bd *beads.Beads, g *git.Git, issueID string) (string, error) {
|
func detectIntegrationBranch(bd *beads.Beads, g *git.Git, issueID string) (string, error) {
|
||||||
// Get the source issue
|
// Traverse up the parent chain looking for an epic with an integration branch
|
||||||
issue, err := bd.Show(issueID)
|
// Limit depth to prevent infinite loops in case of circular references
|
||||||
if err != nil {
|
const maxDepth = 10
|
||||||
return "", fmt.Errorf("looking up issue %s: %w", issueID, err)
|
currentID := issueID
|
||||||
|
|
||||||
|
for depth := 0; depth < maxDepth; depth++ {
|
||||||
|
// Get the current issue
|
||||||
|
issue, err := bd.Show(currentID)
|
||||||
|
if err != nil {
|
||||||
|
return "", fmt.Errorf("looking up issue %s: %w", currentID, err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Check if this issue is an epic
|
||||||
|
if issue.Type == "epic" {
|
||||||
|
// Found an epic - check if it has an integration branch
|
||||||
|
integrationBranch := "integration/" + issue.ID
|
||||||
|
|
||||||
|
// Check local first (faster)
|
||||||
|
exists, err := g.BranchExists(integrationBranch)
|
||||||
|
if err != nil {
|
||||||
|
return "", fmt.Errorf("checking local branch: %w", err)
|
||||||
|
}
|
||||||
|
if exists {
|
||||||
|
return integrationBranch, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// Check remote
|
||||||
|
exists, err = g.RemoteBranchExists("origin", integrationBranch)
|
||||||
|
if err != nil {
|
||||||
|
// Remote check failure is non-fatal, continue to parent
|
||||||
|
} else if exists {
|
||||||
|
return integrationBranch, nil
|
||||||
|
}
|
||||||
|
// Epic found but no integration branch - continue checking parents
|
||||||
|
// in case there's a higher-level epic with an integration branch
|
||||||
|
}
|
||||||
|
|
||||||
|
// Move to parent
|
||||||
|
if issue.Parent == "" {
|
||||||
|
return "", nil // No more parents, no integration branch found
|
||||||
|
}
|
||||||
|
currentID = issue.Parent
|
||||||
}
|
}
|
||||||
|
|
||||||
// Check if issue has a parent
|
return "", nil // Max depth reached, no integration branch found
|
||||||
if issue.Parent == "" {
|
|
||||||
return "", nil // No parent, no integration branch
|
|
||||||
}
|
|
||||||
|
|
||||||
// Get the parent issue
|
|
||||||
parent, err := bd.Show(issue.Parent)
|
|
||||||
if err != nil {
|
|
||||||
return "", fmt.Errorf("looking up parent %s: %w", issue.Parent, err)
|
|
||||||
}
|
|
||||||
|
|
||||||
// Check if parent is an epic
|
|
||||||
if parent.Type != "epic" {
|
|
||||||
return "", nil // Parent is not an epic
|
|
||||||
}
|
|
||||||
|
|
||||||
// Check if integration branch exists
|
|
||||||
integrationBranch := "integration/" + parent.ID
|
|
||||||
|
|
||||||
// Check local first (faster)
|
|
||||||
exists, err := g.BranchExists(integrationBranch)
|
|
||||||
if err != nil {
|
|
||||||
return "", fmt.Errorf("checking local branch: %w", err)
|
|
||||||
}
|
|
||||||
if exists {
|
|
||||||
return integrationBranch, nil
|
|
||||||
}
|
|
||||||
|
|
||||||
// Check remote
|
|
||||||
exists, err = g.RemoteBranchExists("origin", integrationBranch)
|
|
||||||
if err != nil {
|
|
||||||
// Remote check failure is non-fatal
|
|
||||||
return "", nil
|
|
||||||
}
|
|
||||||
if exists {
|
|
||||||
return integrationBranch, nil
|
|
||||||
}
|
|
||||||
|
|
||||||
return "", nil // No integration branch found
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// polecatCleanup sends a lifecycle shutdown request to the witness and waits for termination.
|
// polecatCleanup sends a lifecycle shutdown request to the witness and waits for termination.
|
||||||
|
|||||||
@@ -68,6 +68,24 @@ func TestParseBranchName(t *testing.T) {
|
|||||||
wantIssue: "gt-abc.1",
|
wantIssue: "gt-abc.1",
|
||||||
wantWorker: "Worker",
|
wantWorker: "Worker",
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
name: "polecat branch with issue and timestamp",
|
||||||
|
branch: "polecat/furiosa/gt-jns7.1@mk123456",
|
||||||
|
wantIssue: "gt-jns7.1",
|
||||||
|
wantWorker: "furiosa",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "modern polecat branch (timestamp format)",
|
||||||
|
branch: "polecat/furiosa-mkc36bb9",
|
||||||
|
wantIssue: "", // Should NOT extract fake issue from worker-timestamp
|
||||||
|
wantWorker: "furiosa",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "modern polecat branch with longer name",
|
||||||
|
branch: "polecat/citadel-mk0vro62",
|
||||||
|
wantIssue: "",
|
||||||
|
wantWorker: "citadel",
|
||||||
|
},
|
||||||
{
|
{
|
||||||
name: "simple issue branch",
|
name: "simple issue branch",
|
||||||
branch: "gt-xyz",
|
branch: "gt-xyz",
|
||||||
|
|||||||
@@ -249,9 +249,18 @@ func (m *Manager) AddWithOptions(name string, opts AddOptions) (*Polecat, error)
|
|||||||
polecatDir := m.polecatDir(name)
|
polecatDir := m.polecatDir(name)
|
||||||
clonePath := filepath.Join(polecatDir, m.rig.Name)
|
clonePath := filepath.Join(polecatDir, m.rig.Name)
|
||||||
|
|
||||||
// Unique branch per run - prevents drift from stale branches
|
// Branch naming: include issue ID when available for better traceability.
|
||||||
// Use base36 encoding for shorter branch names (8 chars vs 13 digits)
|
// Format: polecat/<worker>/<issue>@<timestamp> when HookBead is set
|
||||||
branchName := fmt.Sprintf("polecat/%s-%s", name, strconv.FormatInt(time.Now().UnixMilli(), 36))
|
// The @timestamp suffix ensures uniqueness if the same issue is re-slung.
|
||||||
|
// parseBranchName strips the @suffix to extract the issue ID.
|
||||||
|
timestamp := strconv.FormatInt(time.Now().UnixMilli(), 36)
|
||||||
|
var branchName string
|
||||||
|
if opts.HookBead != "" {
|
||||||
|
branchName = fmt.Sprintf("polecat/%s/%s@%s", name, opts.HookBead, timestamp)
|
||||||
|
} else {
|
||||||
|
// Fallback to timestamp format when no issue is known at spawn time
|
||||||
|
branchName = fmt.Sprintf("polecat/%s-%s", name, timestamp)
|
||||||
|
}
|
||||||
|
|
||||||
// Create polecat directory (polecats/<name>/)
|
// Create polecat directory (polecats/<name>/)
|
||||||
if err := os.MkdirAll(polecatDir, 0755); err != nil {
|
if err := os.MkdirAll(polecatDir, 0755); err != nil {
|
||||||
@@ -574,8 +583,14 @@ func (m *Manager) RepairWorktreeWithOptions(name string, force bool, opts AddOpt
|
|||||||
// Create fresh worktree with unique branch name, starting from origin's default branch
|
// Create fresh worktree with unique branch name, starting from origin's default branch
|
||||||
// Old branches are left behind - they're ephemeral (never pushed to origin)
|
// Old branches are left behind - they're ephemeral (never pushed to origin)
|
||||||
// and will be cleaned up by garbage collection
|
// and will be cleaned up by garbage collection
|
||||||
// Use base36 encoding for shorter branch names (8 chars vs 13 digits)
|
// Branch naming: include issue ID when available for better traceability.
|
||||||
branchName := fmt.Sprintf("polecat/%s-%s", name, strconv.FormatInt(time.Now().UnixMilli(), 36))
|
timestamp := strconv.FormatInt(time.Now().UnixMilli(), 36)
|
||||||
|
var branchName string
|
||||||
|
if opts.HookBead != "" {
|
||||||
|
branchName = fmt.Sprintf("polecat/%s/%s@%s", name, opts.HookBead, timestamp)
|
||||||
|
} else {
|
||||||
|
branchName = fmt.Sprintf("polecat/%s-%s", name, timestamp)
|
||||||
|
}
|
||||||
if err := repoGit.WorktreeAddFromRef(newClonePath, branchName, startPoint); err != nil {
|
if err := repoGit.WorktreeAddFromRef(newClonePath, branchName, startPoint); err != nil {
|
||||||
return nil, fmt.Errorf("creating fresh worktree from %s: %w", startPoint, err)
|
return nil, fmt.Errorf("creating fresh worktree from %s: %w", startPoint, err)
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user