From a1008f6f58249b47ecb3756d14c54909d7f0f260 Mon Sep 17 00:00:00 2001 From: Julian Knutsen Date: Mon, 12 Jan 2026 09:45:25 +0000 Subject: [PATCH] fix(sling): pass both feature and issue vars in formula-on-bead mode (#382) When using `gt sling --on `, the code was only passing the `feature` variable (set to bead title). This broke formulas that expect `issue` (set to bead ID), like mol-polecat-work. Now passes both common variables: - feature: bead title (for shiny-style formulas) - issue: bead ID (for mol-polecat-work-style formulas) This allows either formula type to work with --on without requiring the user to manually specify variables. Fixes #355 Co-authored-by: Claude Opus 4.5 --- internal/cmd/sling.go | 7 +- internal/cmd/sling_test.go | 141 +++++++++++++++++++++++++++++++++++++ 2 files changed, 145 insertions(+), 3 deletions(-) diff --git a/internal/cmd/sling.go b/internal/cmd/sling.go index 254c9768..e76b2327 100644 --- a/internal/cmd/sling.go +++ b/internal/cmd/sling.go @@ -362,7 +362,7 @@ func runSling(cmd *cobra.Command, args []string) error { if formulaName != "" { fmt.Printf("Would instantiate formula %s:\n", formulaName) fmt.Printf(" 1. bd cook %s\n", formulaName) - fmt.Printf(" 2. bd mol wisp %s --var feature=\"%s\"\n", formulaName, info.Title) + fmt.Printf(" 2. bd mol wisp %s --var feature=\"%s\" --var issue=\"%s\"\n", formulaName, info.Title, beadID) fmt.Printf(" 3. bd mol bond %s\n", beadID) fmt.Printf(" 4. bd update --status=hooked --assignee=%s\n", targetAgent) } else { @@ -398,9 +398,10 @@ func runSling(cmd *cobra.Command, args []string) error { return fmt.Errorf("cooking formula %s: %w", formulaName, err) } - // Step 2: Create wisp with feature variable from bead title + // Step 2: Create wisp with feature and issue variables from bead featureVar := fmt.Sprintf("feature=%s", info.Title) - wispArgs := []string{"--no-daemon", "mol", "wisp", formulaName, "--var", featureVar, "--json"} + issueVar := fmt.Sprintf("issue=%s", beadID) + wispArgs := []string{"--no-daemon", "mol", "wisp", formulaName, "--var", featureVar, "--var", issueVar, "--json"} wispCmd := exec.Command("bd", wispArgs...) wispCmd.Dir = formulaWorkDir wispCmd.Stderr = os.Stderr diff --git a/internal/cmd/sling_test.go b/internal/cmd/sling_test.go index c2225315..5847b29b 100644 --- a/internal/cmd/sling_test.go +++ b/internal/cmd/sling_test.go @@ -345,6 +345,147 @@ exit 0 } } +// TestSlingFormulaOnBeadPassesFeatureAndIssueVars verifies that when using +// gt sling --on , both --var feature= and --var issue=<beadID> +// are passed to the bd mol wisp command. +func TestSlingFormulaOnBeadPassesFeatureAndIssueVars(t *testing.T) { + townRoot := t.TempDir() + + // Minimal workspace marker so workspace.FindFromCwd() succeeds. + if err := os.MkdirAll(filepath.Join(townRoot, "mayor", "rig"), 0755); err != nil { + t.Fatalf("mkdir mayor/rig: %v", err) + } + + // Create a rig path that owns gt-* beads, and a routes.jsonl pointing to it. + rigDir := filepath.Join(townRoot, "gastown", "mayor", "rig") + if err := os.MkdirAll(filepath.Join(townRoot, ".beads"), 0755); err != nil { + t.Fatalf("mkdir .beads: %v", err) + } + if err := os.MkdirAll(rigDir, 0755); err != nil { + t.Fatalf("mkdir rigDir: %v", err) + } + routes := strings.Join([]string{ + `{"prefix":"gt-","path":"gastown/mayor/rig"}`, + `{"prefix":"hq-","path":"."}`, + "", + }, "\n") + if err := os.WriteFile(filepath.Join(townRoot, ".beads", "routes.jsonl"), []byte(routes), 0644); err != nil { + t.Fatalf("write routes.jsonl: %v", err) + } + + // Stub bd so we can observe the arguments passed to mol wisp. + binDir := filepath.Join(townRoot, "bin") + if err := os.MkdirAll(binDir, 0755); err != nil { + t.Fatalf("mkdir binDir: %v", err) + } + logPath := filepath.Join(townRoot, "bd.log") + bdPath := filepath.Join(binDir, "bd") + // The stub returns a specific title so we can verify it appears in --var feature= + bdScript := `#!/bin/sh +set -e +echo "ARGS:$*" >> "${BD_LOG}" +if [ "$1" = "--no-daemon" ]; then + shift +fi +cmd="$1" +shift || true +case "$cmd" in + show) + echo '[{"title":"My Test Feature","status":"open","assignee":"","description":""}]' + ;; + formula) + # formula show <name> - must output something for verifyFormulaExists + echo '{"name":"mol-review"}' + exit 0 + ;; + cook) + exit 0 + ;; + mol) + sub="$1" + shift || true + case "$sub" in + wisp) + echo '{"new_epic_id":"gt-wisp-xyz"}' + ;; + bond) + echo '{"root_id":"gt-wisp-xyz"}' + ;; + esac + ;; +esac +exit 0 +` + if err := os.WriteFile(bdPath, []byte(bdScript), 0755); err != nil { + t.Fatalf("write bd stub: %v", err) + } + + t.Setenv("BD_LOG", logPath) + t.Setenv("PATH", binDir+string(os.PathListSeparator)+os.Getenv("PATH")) + t.Setenv(EnvGTRole, "mayor") + t.Setenv("GT_POLECAT", "") + t.Setenv("GT_CREW", "") + + cwd, err := os.Getwd() + if err != nil { + t.Fatalf("getwd: %v", err) + } + t.Cleanup(func() { _ = os.Chdir(cwd) }) + if err := os.Chdir(filepath.Join(townRoot, "mayor", "rig")); err != nil { + t.Fatalf("chdir: %v", err) + } + + // Ensure we don't leak global flag state across tests. + prevOn := slingOnTarget + prevVars := slingVars + prevDryRun := slingDryRun + prevNoConvoy := slingNoConvoy + t.Cleanup(func() { + slingOnTarget = prevOn + slingVars = prevVars + slingDryRun = prevDryRun + slingNoConvoy = prevNoConvoy + }) + + slingDryRun = false + slingNoConvoy = true + slingVars = nil + slingOnTarget = "gt-abc123" + + if err := runSling(nil, []string{"mol-review"}); err != nil { + t.Fatalf("runSling: %v", err) + } + + logBytes, err := os.ReadFile(logPath) + if err != nil { + t.Fatalf("read bd log: %v", err) + } + + // Find the mol wisp command and verify both --var arguments + logLines := strings.Split(string(logBytes), "\n") + var wispLine string + for _, line := range logLines { + if strings.Contains(line, "mol wisp") { + wispLine = line + break + } + } + + if wispLine == "" { + t.Fatalf("mol wisp command not found in log: %s", string(logBytes)) + } + + // Verify --var feature=<title> is present + if !strings.Contains(wispLine, "--var feature=My Test Feature") { + t.Errorf("mol wisp missing --var feature=<title>\ngot: %s", wispLine) + } + + // Verify --var issue=<beadID> is present + if !strings.Contains(wispLine, "--var issue=gt-abc123") { + t.Errorf("mol wisp missing --var issue=<beadID>\ngot: %s", wispLine) + } +} + // TestVerifyBeadExistsAllowStale reproduces the bug in gtl-ncq where beads // visible via regular bd show fail with --no-daemon due to database sync issues. // The fix uses --allow-stale to skip the sync check for existence verification.