Merge pull request #364 from gastownhall/fix/routed-to-metadata-audit-complete

Use gc.routed_to for dispatched work routing

Author: julianknutsen

cmd/gc/build_desired_state.go

@@ -423,7 +423,7 @@ func collectAssignedWorkBeads(
 
 func appendAssignedUnique(dst *[]beads.Bead, beadList []beads.Bead, seen map[string]struct{}) {
 	for _, b := range beadList {
-		if strings.TrimSpace(b.Assignee) == "" && strings.TrimSpace(b.Metadata["gc.routed_to"]) == "" {
+		if strings.TrimSpace(b.Assignee) == "" && strings.TrimSpace(b.Metadata["gc.routed_to"]) == "" && !hasPoolLabel(b) {
 			continue
 		}
 		if _, ok := seen[b.ID]; ok {
@@ -434,6 +434,15 @@ func appendAssignedUnique(dst *[]beads.Bead, beadList []beads.Bead, seen map[str
 	}
 }
 
+func hasPoolLabel(b beads.Bead) bool {
+	for _, l := range b.Labels {
+		if strings.HasPrefix(l, "pool:") {
+			return true
+		}
+	}
+	return false
+}
+
 func controlDispatcherOnlyConfig(cfg *config.City) *config.City {
 	if cfg == nil {
 		return nil

cmd/gc/cmd_convoy_dispatch.go

@@ -290,7 +290,7 @@ func graphFallbackBindingForBead(source beads.Bead, store beads.Store, cityName
 
 	binding := graphRouteBinding{qualifiedName: agentCfg.QualifiedName()}
 	if isMultiSessionCfgAgent(&agentCfg) {
-		binding.label = "pool:" + agentCfg.QualifiedName()
+		binding.metadataOnly = true
 		return binding, nil
 	}
 	if source.Assignee != "" {

cmd/gc/cmd_convoy_dispatch_test.go

@@ -274,15 +274,11 @@ func TestDecorateDynamicFragmentRecipePreservesPoolFallbackAndScopeMetadata(t *t
 	if review.Metadata["gc.routed_to"] != "frontend/reviewer" {
 		t.Fatalf("review gc.routed_to = %q, want frontend/reviewer", review.Metadata["gc.routed_to"])
 	}
-	foundPoolLabel := false
 	for _, label := range review.Labels {
 		if label == "pool:frontend/reviewer" {
-			foundPoolLabel = true
+			t.Fatalf("review labels = %#v, should not contain legacy pool label", review.Labels)
 		}
 	}
-	if !foundPoolLabel {
-		t.Fatalf("review labels = %#v, want pool label", review.Labels)
-	}
 	if review.Metadata["gc.scope_ref"] != "body" {
 		t.Fatalf("review gc.scope_ref = %q, want body", review.Metadata["gc.scope_ref"])
 	}

cmd/gc/cmd_order.go

@@ -51,7 +51,7 @@ func newOrderListCmd(stdout, stderr io.Writer) *cobra.Command {
 	return &cobra.Command{
 		Use:   "list",
 		Short: "List available orders",
-		Long: `List all available orders with their gate type, schedule, and target pool.
+		Long: `List all available orders with their gate type, schedule, and target.
 
 Scans formula layers for formulas that have order metadata
 (gate, interval, schedule, check, pool).`,
@@ -73,7 +73,7 @@ func newOrderShowCmd(stdout, stderr io.Writer) *cobra.Command {
 		Long: `Display detailed information about a named order.
 
 Shows the order name, description, formula reference, gate type,
-scheduling parameters, check command, target pool, and source file.
+scheduling parameters, check command, target, and source file.
 Use --rig to disambiguate same-name orders in different rigs.`,
 		Args: cobra.ExactArgs(1),
 		RunE: func(_ *cobra.Command, args []string) error {
@@ -95,7 +95,7 @@ func newOrderRunCmd(stdout, stderr io.Writer) *cobra.Command {
 		Long: `Execute an order manually, bypassing its gate conditions.
 
 Instantiates a wisp from the order's formula and routes it to the
-target pool (if configured). Useful for testing orders or triggering
+configured target (if any). Useful for testing orders or triggering
 them outside their normal schedule.
 Use --rig to disambiguate same-name orders in different rigs.`,
 		Args: cobra.ExactArgs(1),
@@ -303,9 +303,9 @@ func doOrderList(aa []orders.Order, stdout io.Writer) int {
 
 	hasRig := anyOrderHasRig(aa)
 	if hasRig {
-		fmt.Fprintf(stdout, "%-20s %-8s %-12s %-15s %-15s %s\n", "NAME", "TYPE", "GATE", "INTERVAL/SCHED", "RIG", "POOL") //nolint:errcheck
+		fmt.Fprintf(stdout, "%-20s %-8s %-12s %-15s %-15s %s\n", "NAME", "TYPE", "GATE", "INTERVAL/SCHED", "RIG", "TARGET") //nolint:errcheck
 	} else {
-		fmt.Fprintf(stdout, "%-20s %-8s %-12s %-15s %s\n", "NAME", "TYPE", "GATE", "INTERVAL/SCHED", "POOL") //nolint:errcheck
+		fmt.Fprintf(stdout, "%-20s %-8s %-12s %-15s %s\n", "NAME", "TYPE", "GATE", "INTERVAL/SCHED", "TARGET") //nolint:errcheck
 	}
 	for _, a := range aa {
 		typ := "formula"
@@ -394,7 +394,7 @@ func doOrderShow(aa []orders.Order, name, rig string, stdout, stderr io.Writer)
 		w(fmt.Sprintf("On:          %s", a.On))
 	}
 	if a.Pool != "" {
-		w(fmt.Sprintf("Pool:        %s", a.Pool))
+		w(fmt.Sprintf("Target:      %s", a.Pool))
 	}
 	w(fmt.Sprintf("Source:      %s", a.Source))
 	return 0
@@ -424,7 +424,7 @@ func cmdOrderRun(name, rig string, stdout, stderr io.Writer) int {
 
 // doOrderRun executes an order manually: instantiates a wisp from the
 // order's formula (or runs exec script directly) and routes it to the
-// target pool.
+// configured target.
 func doOrderRun(aa []orders.Order, name, rig, cityPath string, runner SlingRunner, store beads.Store, ep events.Provider, stdout, stderr io.Writer) int {
 	a, ok := findOrder(aa, name, rig)
 	if !ok {
@@ -482,15 +482,16 @@ func doOrderRun(aa []orders.Order, name, rig, cityPath string, runner SlingRunne
 	}
 	rootID := cookResult.RootID
 
-	// Label with order-run:<scopedName> for tracking, plus pool routing if specified.
-	// For event gates, also add order:<scopedName> and seq:<headSeq> for cursor tracking.
+	// Label with order-run:<scopedName> for tracking, plus root routing metadata
+	// when a target is configured. For event gates, also add order:<scopedName>
+	// and seq:<headSeq> for cursor tracking.
 	routeCmd := fmt.Sprintf("bd update %s --add-label=order-run:%s", rootID, scoped)
 	if a.Gate == "event" && ep != nil {
 		routeCmd += fmt.Sprintf(" --add-label=order:%s --add-label=seq:%d", scoped, headSeq)
 	}
 	if a.Pool != "" {
 		pool := qualifyPool(a.Pool, a.Rig)
-		routeCmd += fmt.Sprintf(" --add-label=pool:%s", pool)
+		routeCmd += fmt.Sprintf(" --set-metadata gc.routed_to=%s", pool)
 	}
 	if _, err := runner("", routeCmd, nil); err != nil {
 		fmt.Fprintf(stderr, "gc order run: labeling wisp: %v\n", err) //nolint:errcheck // best-effort stderr
@@ -499,7 +500,7 @@ func doOrderRun(aa []orders.Order, name, rig, cityPath string, runner SlingRunne
 
 	fmt.Fprintf(stdout, "Order %q executed: wisp %s", name, rootID) //nolint:errcheck
 	if a.Pool != "" {
-		fmt.Fprintf(stdout, " → pool:%s", a.Pool) //nolint:errcheck
+		fmt.Fprintf(stdout, " → gc.routed_to=%s", qualifyPool(a.Pool, a.Rig)) //nolint:errcheck
 	}
 	fmt.Fprintln(stdout) //nolint:errcheck
 	return 0

cmd/gc/cmd_order_test.go

@@ -340,12 +340,12 @@ func TestOrderRun(t *testing.T) {
 	if len(calls) != 1 {
 		t.Fatalf("got %d runner calls, want 1: %v", len(calls), calls)
 	}
-	// Should include both order-run label and pool label in a single bd update.
+	// Should include both order-run tracking and routed_to metadata in a single bd update.
 	if !strings.Contains(calls[0], "--add-label=order-run:digest") {
 		t.Errorf("call[0] = %q, want --add-label=order-run:digest", calls[0])
 	}
-	if !strings.Contains(calls[0], "--add-label=pool:dog") {
-		t.Errorf("call[0] = %q, want --add-label=pool:dog", calls[0])
+	if !strings.Contains(calls[0], "--set-metadata gc.routed_to=dog") {
+		t.Errorf("call[0] = %q, want --set-metadata gc.routed_to=dog", calls[0])
 	}
 }
 
@@ -375,9 +375,9 @@ func TestOrderRunNoPool(t *testing.T) {
 	if !strings.Contains(calls[0], "--add-label=order-run:cleanup") {
 		t.Errorf("call[0] = %q, want --add-label=order-run:cleanup", calls[0])
 	}
-	// Should NOT contain pool label.
-	if strings.Contains(calls[0], "--add-label=pool:") {
-		t.Errorf("call[0] = %q, should not contain pool label", calls[0])
+	// Should NOT contain routed_to metadata.
+	if strings.Contains(calls[0], "--set-metadata gc.routed_to=") {
+		t.Errorf("call[0] = %q, should not contain routed_to metadata", calls[0])
 	}
 	// Verify wisp ID appears in stdout (MemStore generates gc-N IDs).
 	if !strings.Contains(stdout.String(), "gc-1") {
@@ -722,8 +722,8 @@ func TestOrderRunRigQualifiesPool(t *testing.T) {
 	if !strings.Contains(calls[0], "--add-label=order-run:db-health:rig:demo-repo") {
 		t.Errorf("call[0] = %q, want --add-label=order-run:db-health:rig:demo-repo", calls[0])
 	}
-	// Auto-qualified pool.
-	if !strings.Contains(calls[0], "--add-label=pool:demo-repo/polecat") {
-		t.Errorf("call[0] = %q, want --add-label=pool:demo-repo/polecat", calls[0])
+	// Auto-qualified routed_to target.
+	if !strings.Contains(calls[0], "--set-metadata gc.routed_to=demo-repo/polecat") {
+		t.Errorf("call[0] = %q, want --set-metadata gc.routed_to=demo-repo/polecat", calls[0])
 	}
 }

cmd/gc/cmd_sling.go

@@ -949,9 +949,10 @@ func slingFormulaUsesTargetBranch(formula string) bool {
 }
 
 // resolveSlingEnv returns extra env vars for the sling command.
-// For fixed (non-pool) agents, resolves the target's session name from
-// the bead store and returns it as GC_SLING_TARGET. Pool agents don't
-// need this — they use label-based dispatch.
+// For fixed single-session agents, resolves the target's session name from
+// the bead store and returns it as GC_SLING_TARGET. Default routing uses
+// gc.routed_to metadata for all agents, but custom sling_query templates may
+// still rely on the resolved concrete session target.
 func resolveSlingEnv(a config.Agent, deps slingDeps) map[string]string {
 	if isMultiSessionCfgAgent(&a) {
 		return nil
@@ -1121,7 +1122,7 @@ func decorateGraphWorkflowRecipe(recipe *formula.Recipe, routeVars map[string]st
 	if sessionName != "" {
 		defaultRoute.sessionName = sessionName
 	} else {
-		defaultRoute.label = "pool:" + routedTo
+		defaultRoute.metadataOnly = true
 	}
 	routingRigContext := graphRouteRigContext(defaultRoute.qualifiedName)
 	controlRoute, err := controlDispatcherBinding(store, cityName, cfg, routingRigContext)
@@ -1213,7 +1214,7 @@ func workflowStoreRefForDir(storeDir, cityPath, cityName string, cfg *config.Cit
 type graphRouteBinding struct {
 	qualifiedName string
 	sessionName   string
-	label         string
+	metadataOnly  bool
 }
 
 func resolveGraphStepBinding(stepID string, stepByID map[string]*formula.RecipeStep, stepAlias map[string]string, depsByStep map[string][]string, cache map[string]graphRouteBinding, resolving map[string]bool, fallback graphRouteBinding, rigContext string, store beads.Store, cityName string, cfg *config.City) (graphRouteBinding, error) {
@@ -1327,7 +1328,7 @@ func resolveGraphStepBindingWithVars(stepID string, stepByID map[string]*formula
 	}
 	binding := graphRouteBinding{qualifiedName: agentCfg.QualifiedName()}
 	if isMultiSessionCfgAgent(&agentCfg) {
-		binding.label = "pool:" + agentCfg.QualifiedName()
+		binding.metadataOnly = true
 		cache[stepID] = binding
 		return binding, nil
 	}
@@ -1366,15 +1367,6 @@ func graphRouteRigContext(route string) string {
 	return route[:idx]
 }
 
-func appendUniqueString(in []string, value string) []string {
-	for _, existing := range in {
-		if existing == value {
-			return in
-		}
-	}
-	return append(in, value)
-}
-
 func shouldPromoteWorkflowLaunchStatus(status string) bool {
 	switch strings.ToLower(strings.TrimSpace(status)) {
 	case "", "open", "ready", "todo", "triage", "backlog":
@@ -1461,6 +1453,9 @@ func checkBeadState(q BeadQuerier, beadID string, a config.Agent) beadCheckResul
 		if b.Assignee != "" {
 			warnings = append(warnings, fmt.Sprintf("warning: bead %s already assigned to %q", beadID, b.Assignee))
 		}
+		if routedTo := strings.TrimSpace(b.Metadata["gc.routed_to"]); routedTo != "" {
+			warnings = append(warnings, fmt.Sprintf("warning: bead %s already routed to %q", beadID, routedTo))
+		}
 		for _, l := range b.Labels {
 			if strings.HasPrefix(l, "pool:") {
 				warnings = append(warnings, fmt.Sprintf("warning: bead %s already has pool label %q", beadID, l))
@@ -1470,8 +1465,20 @@ func checkBeadState(q BeadQuerier, beadID string, a config.Agent) beadCheckResul
 	}
 
 	target := a.QualifiedName()
+	if strings.TrimSpace(b.Metadata["gc.routed_to"]) == target {
+		// Only idempotent if the bead is unassigned or already assigned
+		// consistently with the target. Otherwise the bead would be
+		// invisible to the target's work_query (which requires --unassigned
+		// for pool work in tier 3).
+		if b.Assignee == "" || b.Assignee == target {
+			return beadCheckResult{Idempotent: true}
+		}
+		return beadCheckResult{
+			Warnings: []string{fmt.Sprintf("warning: bead %s routed to %q but assigned to %q", beadID, target, b.Assignee)},
+		}
+	}
 
-	// Fixed agent: check assignee match.
+	// Fixed agent: check legacy assignee routing as a compatibility fallback.
 	if !isMultiSessionCfgAgent(&a) {
 		if b.Assignee == target {
 			return beadCheckResult{Idempotent: true}
@@ -1480,6 +1487,9 @@ func checkBeadState(q BeadQuerier, beadID string, a config.Agent) beadCheckResul
 		if b.Assignee != "" {
 			warnings = append(warnings, fmt.Sprintf("warning: bead %s already assigned to %q", beadID, b.Assignee))
 		}
+		if routedTo := strings.TrimSpace(b.Metadata["gc.routed_to"]); routedTo != "" {
+			warnings = append(warnings, fmt.Sprintf("warning: bead %s already routed to %q", beadID, routedTo))
+		}
 		for _, l := range b.Labels {
 			if strings.HasPrefix(l, "pool:") {
 				warnings = append(warnings, fmt.Sprintf("warning: bead %s already has pool label %q", beadID, l))
@@ -1488,17 +1498,24 @@ func checkBeadState(q BeadQuerier, beadID string, a config.Agent) beadCheckResul
 		return beadCheckResult{Warnings: warnings}
 	}
 
-	// Pool: check for matching pool label.
-	poolLabel := "pool:" + target
-	for _, l := range b.Labels {
-		if l == poolLabel {
-			return beadCheckResult{Idempotent: true}
+	// Multi-session targets: pool labels are a legacy fallback only when
+	// gc.routed_to is absent. If gc.routed_to is set (even to a different
+	// target), it is authoritative — a stale pool label must not short-circuit.
+	if strings.TrimSpace(b.Metadata["gc.routed_to"]) == "" {
+		poolLabel := "pool:" + target
+		for _, l := range b.Labels {
+			if l == poolLabel {
+				return beadCheckResult{Idempotent: true}
+			}
 		}
 	}
 	var warnings []string
 	if b.Assignee != "" {
 		warnings = append(warnings, fmt.Sprintf("warning: bead %s already assigned to %q", beadID, b.Assignee))
 	}
+	if routedTo := strings.TrimSpace(b.Metadata["gc.routed_to"]); routedTo != "" {
+		warnings = append(warnings, fmt.Sprintf("warning: bead %s already routed to %q", beadID, routedTo))
+	}
 	for _, l := range b.Labels {
 		if strings.HasPrefix(l, "pool:") {
 			warnings = append(warnings, fmt.Sprintf("warning: bead %s already has pool label %q", beadID, l))

cmd/gc/cmd_sling_test.go

@@ -2830,25 +2830,25 @@ func TestDryRunNilQuerier(t *testing.T) {
 // --- Idempotency detection (checkBeadState + integration) tests ---
 
 func TestCheckBeadStateIdempotentFixedAgent(t *testing.T) {
-	q := &fakeQuerier{bead: beads.Bead{ID: "BL-42", Assignee: "mayor"}}
+	q := &fakeQuerier{bead: beads.Bead{ID: "BL-42", Metadata: map[string]string{"gc.routed_to": "mayor"}}}
 	a := config.Agent{Name: "mayor", MaxActiveSessions: intPtr(1)}
 
 	result := checkBeadState(q, "BL-42", a)
 	if !result.Idempotent {
-		t.Error("expected Idempotent=true for matching assignee")
+		t.Error("expected Idempotent=true for matching gc.routed_to")
 	}
 	if len(result.Warnings) != 0 {
 		t.Errorf("expected no warnings, got %v", result.Warnings)
 	}
 }
 
 func TestCheckBeadStateIdempotentPool(t *testing.T) {
-	q := &fakeQuerier{bead: beads.Bead{ID: "BL-42", Labels: []string{"pool:hw/polecat"}}}
+	q := &fakeQuerier{bead: beads.Bead{ID: "BL-42", Metadata: map[string]string{"gc.routed_to": "hw/polecat"}}}
 	a := config.Agent{Name: "polecat", Dir: "hw", MinActiveSessions: intPtr(1), MaxActiveSessions: intPtr(3)}
 
 	result := checkBeadState(q, "BL-42", a)
 	if !result.Idempotent {
-		t.Error("expected Idempotent=true for matching pool label")
+		t.Error("expected Idempotent=true for matching gc.routed_to")
 	}
 	if len(result.Warnings) != 0 {
 		t.Errorf("expected no warnings, got %v", result.Warnings)
@@ -2857,14 +2857,15 @@ func TestCheckBeadStateIdempotentPool(t *testing.T) {
 
 func TestCheckBeadStateIdempotentPoolMultiLabels(t *testing.T) {
 	q := &fakeQuerier{bead: beads.Bead{
-		ID:     "BL-42",
-		Labels: []string{"priority:high", "pool:hw/polecat", "sprint:3"},
+		ID:       "BL-42",
+		Labels:   []string{"priority:high", "sprint:3"},
+		Metadata: map[string]string{"gc.routed_to": "hw/polecat"},
 	}}
 	a := config.Agent{Name: "polecat", Dir: "hw", MinActiveSessions: intPtr(1), MaxActiveSessions: intPtr(3)}
 
 	result := checkBeadState(q, "BL-42", a)
 	if !result.Idempotent {
-		t.Error("expected Idempotent=true for matching pool label among others")
+		t.Error("expected Idempotent=true for matching gc.routed_to among other labels")
 	}
 }
 
@@ -2921,7 +2922,7 @@ func TestDoSlingIdempotentSkipsRouting(t *testing.T) {
 	sp := runtime.NewFake()
 	cfg := &config.City{Workspace: config.Workspace{Name: "test-city"}}
 	a := config.Agent{Name: "mayor", MaxActiveSessions: intPtr(1)}
-	q := &fakeQuerier{bead: beads.Bead{ID: "BL-42", Assignee: "mayor"}}
+	q := &fakeQuerier{bead: beads.Bead{ID: "BL-42", Metadata: map[string]string{"gc.routed_to": "mayor"}}}
 
 	deps, stdout, _ := testDeps(cfg, sp, runner.run)
 	opts := testOpts(a, "BL-42")

cmd/gc/dispatch_runtime.go

@@ -64,8 +64,7 @@ func controlDispatcherBinding(store beads.Store, cityName string, cfg *config.Ci
 
 func applyGraphRouteBinding(step *formula.RecipeStep, binding graphRouteBinding) {
 	step.Metadata["gc.routed_to"] = binding.qualifiedName
-	if binding.label != "" {
-		step.Labels = appendUniqueString(step.Labels, binding.label)
+	if binding.metadataOnly {
 		step.Assignee = ""
 		return
 	}

cmd/gc/order_dispatch.go

@@ -256,8 +256,7 @@ func (m *memoryOrderDispatcher) dispatchWisp(ctx context.Context, a orders.Order
 	}
 
 	// Decorate graph workflow recipes with routing metadata so child step
-	// beads get gc.routed_to set. Without this, only the root bead gets the
-	// pool label and agents cannot discover their step work.
+	// beads get gc.routed_to set before instantiation.
 	if a.Pool != "" {
 		pool := qualifyPool(a.Pool, a.Rig)
 		if err := applyGraphRouting(recipe, nil, pool, nil, "", "", "", "", m.store, m.cityName, m.cfg); err != nil {
@@ -287,7 +286,7 @@ func (m *memoryOrderDispatcher) dispatchWisp(ctx context.Context, a orders.Order
 	}
 	if a.Pool != "" {
 		pool := qualifyPool(a.Pool, a.Rig)
-		args = append(args, fmt.Sprintf("--add-label=pool:%s", pool))
+		args = append(args, "--set-metadata", "gc.routed_to="+pool)
 	}
 	if _, err := m.runner(cityPath, "bd", args...); err != nil {
 		// Label failure is critical for duplicate-dispatch prevention.