Skip to content

Commit f589983

Browse files
committed
fix: keep v1 attachments on molecule flow
1 parent 459b501 commit f589983

4 files changed

Lines changed: 43 additions & 21 deletions

File tree

cmd/gc/cmd_sling_test.go

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2823,7 +2823,7 @@ func TestBatchAutoBurnStaleMolecules(t *testing.T) {
28232823
assertStoreRoutedTo(t, deps.Store, "BL-2", "mayor")
28242824
}
28252825

2826-
func TestOnFormulaPoolAttachmentRoutesLegacyStepsToTarget(t *testing.T) {
2826+
func TestOnFormulaPoolAttachmentKeepsLegacyStepsPrivate(t *testing.T) {
28272827
dir := testFormulaDir(t)
28282828
content := `
28292829
formula = "multi-step"
@@ -2896,17 +2896,14 @@ needs = ["prep"]
28962896
if bead.ParentID == "BL-42" {
28972897
t.Fatalf("internal bead %s ParentID = %q, want not outer bead", bead.ID, bead.ParentID)
28982898
}
2899-
// Regression for #796: legacy [[steps]] formulas must stamp
2900-
// gc.routed_to on every internal step bead so EffectiveWorkQuery
2901-
// tier-3 and pool scale_check see the work. The sling target is
2902-
// "repo/polecat".
29032899
if bead.Ref == "" {
29042900
continue
29052901
}
2906-
if got := bead.Metadata["gc.routed_to"]; got != a.QualifiedName() {
2907-
t.Fatalf("internal bead %s gc.routed_to = %q, want %q", bead.ID, got, a.QualifiedName())
2902+
if got := bead.Metadata["gc.routed_to"]; got != "" {
2903+
t.Fatalf("internal legacy bead %s gc.routed_to = %q, want empty; attached v1 formulas should route only the source bead", bead.ID, got)
29082904
}
29092905
}
2906+
assertStoreRoutedTo(t, deps.Store, "BL-42", a.QualifiedName())
29102907
}
29112908

29122909
func TestBatchSkipsClosedMolecules(t *testing.T) {

internal/graphroute/graphroute.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -457,9 +457,12 @@ func DecorateGraphWorkflowRecipe(recipe *formula.Recipe, routeVars map[string]st
457457

458458
// ApplyGraphRouting decorates a compiled recipe with routing metadata.
459459
// For graph.v2 workflows it delegates to DecorateGraphWorkflowRecipe. For
460-
// legacy [[steps]] recipes it stamps gc.routed_to on every non-root step
461-
// so EffectiveWorkQuery tier-3 and pool scale_check can see the work
462-
// (fixes #796). Returns early with no effect when cfg is nil.
460+
// standalone legacy [[steps]] recipes it stamps gc.routed_to on every
461+
// non-root step so EffectiveWorkQuery tier-3 and pool scale_check can see
462+
// the work (fixes #796). Attached legacy formulas intentionally stay on the
463+
// molecule_id flow: only the source bead is routed, and the internal molecule
464+
// steps remain private instructions for the assignee. Returns early with no
465+
// effect when cfg is nil.
463466
func ApplyGraphRouting(recipe *formula.Recipe, a *config.Agent, routedTo string, vars map[string]string, sourceBeadID, scopeKind, scopeRef, storeRef string, store beads.Store, cityName string, cfg *config.City, deps Deps) error {
464467
if recipe == nil || cfg == nil {
465468
return nil
@@ -470,6 +473,9 @@ func ApplyGraphRouting(recipe *formula.Recipe, a *config.Agent, routedTo string,
470473
// and Agent deep-copy on every controller tick that dispatches a legacy
471474
// order.
472475
if !IsCompiledGraphWorkflow(recipe) {
476+
if strings.TrimSpace(sourceBeadID) != "" {
477+
return nil
478+
}
473479
stampLegacyRecipeRouting(recipe, routedTo)
474480
return nil
475481
}

internal/graphroute/graphroute_test.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,24 @@ func TestApplyGraphRouting_LegacyNilAgent(t *testing.T) {
133133
}
134134
}
135135

136+
func TestApplyGraphRouting_LegacyAttachmentDoesNotStampSteps(t *testing.T) {
137+
r := &formula.Recipe{
138+
Name: "mol-legacy",
139+
Steps: []formula.RecipeStep{
140+
{ID: "mol-legacy", IsRoot: true, Metadata: map[string]string{}},
141+
{ID: "mol-legacy.step1", Metadata: map[string]string{}},
142+
},
143+
}
144+
a := config.Agent{Name: "worker", MaxActiveSessions: intPtr(1)}
145+
err := ApplyGraphRouting(r, &a, "worker", nil, "source-1", "", "", "", nil, "city", &config.City{}, Deps{})
146+
if err != nil {
147+
t.Fatalf("unexpected error: %v", err)
148+
}
149+
if _, ok := r.Steps[1].Metadata["gc.routed_to"]; ok {
150+
t.Errorf("attached legacy step gc.routed_to was stamped; v1 attachments should use source molecule_id flow")
151+
}
152+
}
153+
136154
func TestApplyGraphRouting_LegacyNilCfg(t *testing.T) {
137155
// ApplyGraphRouting is a no-op whenever cfg is nil.
138156
r := &formula.Recipe{

test/acceptance/tutorial_goldens/tutorial04_communication_test.go

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -106,22 +106,27 @@ func TestTutorial04Communication(t *testing.T) {
106106
}
107107
})
108108

109-
t.Run(`gc session nudge mayor "Check mail and hook status, then act accordingly"`, func(t *testing.T) {
110-
out, err := ws.runShell(`gc session nudge mayor "Check mail and hook status, then act accordingly"`, "")
109+
communicationNudge := `Review needed: check mail about auth module changes in my-project and coordinate with reviewer`
110+
nudgeMayor := func(context string) {
111+
out, err := ws.runShell(`gc session nudge mayor "`+communicationNudge+`"`, "")
111112
if err != nil {
112-
t.Fatalf("gc session nudge mayor: %v\n%s", err, out)
113+
t.Fatalf("%s: %v\n%s", context, err, out)
113114
}
114115
if !strings.Contains(out, "Nudged mayor") && !strings.Contains(out, "Queued nudge for mayor") {
115-
t.Fatalf("nudge output mismatch:\n%s", out)
116+
t.Fatalf("%s output mismatch:\n%s", context, out)
116117
}
118+
}
119+
120+
t.Run(`gc session nudge mayor "Review needed: check mail about auth module changes in my-project and coordinate with reviewer"`, func(t *testing.T) {
121+
nudgeMayor("gc session nudge mayor")
117122
})
118123

119124
t.Run("gc session peek mayor --lines 6", func(t *testing.T) {
120125
var out string
121126
mayorCommunicationVisible := func() bool {
122127
var err error
123128
out, err = ws.runShell("gc session peek mayor --lines 6", "")
124-
if err != nil || strings.TrimSpace(out) == "" {
129+
if err != nil {
125130
return false
126131
}
127132
return strings.Contains(out, "Review needed") ||
@@ -134,15 +139,11 @@ func TestTutorial04Communication(t *testing.T) {
134139
if out, err := ws.runShell("gc session wake mayor", ""); err != nil {
135140
t.Fatalf("wake mayor before communication retry: %v\n%s", err, out)
136141
}
137-
if out, err := ws.runShell(`gc session nudge mayor "Check mail and hook status, then act accordingly"`, ""); err != nil {
138-
t.Fatalf("re-nudge mayor before communication retry: %v\n%s", err, out)
139-
}
142+
nudgeMayor("re-nudge mayor before communication retry")
140143
}
141144
if !waitForCondition(t, 45*time.Second, 2*time.Second, mayorCommunicationVisible) {
142145
restartCity("mayor still did not surface the communication flow after wake")
143-
if out, err := ws.runShell(`gc session nudge mayor "Check mail and hook status, then act accordingly"`, ""); err != nil {
144-
t.Fatalf("re-nudge mayor after hidden restart: %v\n%s", err, out)
145-
}
146+
nudgeMayor("re-nudge mayor after hidden restart")
146147
}
147148
if !waitForCondition(t, 45*time.Second, 2*time.Second, mayorCommunicationVisible) {
148149
t.Fatalf("peek mayor did not surface communication flow in time:\n%s", out)

0 commit comments

Comments
 (0)