fix(ssh): brace refspec variables for zsh login shells (#3383)

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Carlos Florencio <carlosmflorencio@gmail.com>

apps/backend/internal/agent/runtime/lifecycle/default_scripts.go

@@ -298,7 +298,7 @@ if [ -n "$repository_url" ]; then
     echo 'kandev: target repository has no base branch' >&2
     exit 1
   fi
-  if ! git -C "$workspace" fetch --no-tags origin "+refs/heads/$repository_branch:refs/remotes/origin/$repository_branch" >/dev/null 2>&1; then
+  if ! git -C "$workspace" fetch --no-tags origin "+refs/heads/${repository_branch}:refs/remotes/origin/${repository_branch}" >/dev/null 2>&1; then
     echo 'kandev: target base branch is unavailable' >&2
     exit 1
   fi

apps/backend/internal/agent/runtime/lifecycle/default_scripts_test.go

@@ -3,8 +3,12 @@ package lifecycle
 import (
 	"os/exec"
 	"path/filepath"
+	"regexp"
 	"strings"
 	"testing"
+
+	"github.com/kandev/kandev/internal/scriptengine"
+	"github.com/kandev/kandev/internal/task/models"
 )
 
 // TestKandevBranchCheckoutPostlude_HasInvariantSteps asserts the kandev-
@@ -235,3 +239,49 @@ func TestDefaultPrepareScript_SSHMaterializesPrimaryWorkspace(t *testing.T) {
 		t.Fatal("SSH default prepare script must leave feature-branch selection to the postlude")
 	}
 }
+
+// TestManagedScripts_BraceParameterExpansionsBeforeColon guards every managed
+// script that can run under a user-selected login shell. zsh parses an
+// unbraced `$name:r` as a modifier, so `refs/heads/$branch:refs/...` silently
+// became `refs/heads/mainefs/...` on macOS SSH targets. Bracing is the only
+// form that sh, bash, and zsh all read the same way.
+func TestManagedScripts_BraceParameterExpansionsBeforeColon(t *testing.T) {
+	unbraced := regexp.MustCompile(`\$[A-Za-z_][A-Za-z0-9_]*:`)
+	destination := models.ContributionDestination{
+		Version:  models.ContributionDestinationVersion,
+		Provider: models.ContributionDestinationProviderGitHub,
+		SourceRepository: models.ContributionDestinationRepository{
+			Host: "github.com", Path: "kdlbs/kandev", ProviderID: "100", RemoteURL: "https://github.com/kdlbs/kandev.git",
+		},
+		TargetRepository: models.ContributionDestinationRepository{
+			Host: "github.com", Path: "contributor/kandev", ProviderID: "200", RemoteURL: "https://github.com/contributor/kandev.git",
+		},
+	}
+	destinationScript, err := scriptengine.ContributionDestinationSetupScriptAt(&destination, "/workspace")
+	if err != nil {
+		t.Fatalf("build contribution destination script: %v", err)
+	}
+	scripts := map[string]string{
+		"branch checkout postlude": KandevBranchCheckoutPostlude(),
+		"ssh remote contribution": strings.Join(
+			append(sshRemoteContributionSetupLines(), sshRemoteContributionCheckoutLines()...), "\n"),
+		"ssh contribution destination": destinationScript,
+	}
+	for _, executorType := range []string{"local", "worktree", "local_docker", "k8s", "sprites", executorTypeSSH} {
+		scripts["default prepare script for "+executorType] = DefaultPrepareScript(executorType)
+	}
+	for name, script := range scripts {
+		if script == "" {
+			t.Fatalf("%s: empty script", name)
+		}
+		for index, line := range strings.Split(script, "\n") {
+			if strings.HasPrefix(strings.TrimSpace(line), "#") {
+				continue
+			}
+			if match := unbraced.FindString(line); match != "" {
+				t.Errorf("%s line %d: %q must be written as ${name}: so zsh does not treat the colon as a modifier:\n%s",
+					name, index+1, match, strings.TrimSpace(line))
+			}
+		}
+	}
+}

apps/backend/internal/agent/runtime/lifecycle/executor_ssh_remote_contribution.go

@@ -163,7 +163,7 @@ func sshRemoteContributionSetupLines() []string {
 		"mkdir -p \"$(dirname \"$exclude_file\")\"",
 		"touch \"$exclude_file\"",
 		"grep -Fqx '/.kandev/' \"$exclude_file\" || printf '%%s\\n' '/.kandev/' >>\"$exclude_file\"",
-		"if ! git -C \"$workspace\" fetch --no-tags origin \"+refs/heads/$base_branch:refs/remotes/origin/$base_branch\" >/dev/null 2>&1; then",
+		"if ! git -C \"$workspace\" fetch --no-tags origin \"+refs/heads/${base_branch}:refs/remotes/origin/${base_branch}\" >/dev/null 2>&1; then",
 		"  echo 'kandev: target base branch is unavailable' >&2",
 		"  exit 1",
 		"fi",

apps/backend/internal/agent/runtime/lifecycle/executor_ssh_scripts.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"regexp"
 	"strings"
 	"time"
 
@@ -41,6 +42,24 @@ func mergeSSHMetadata(base, overlay map[string]interface{}) map[string]interface
 	return merged
 }
 
+// legacyFetchRefspec matches the fetch refspec Kandev generated before its
+// managed scripts braced their expansions.
+var legacyFetchRefspec = regexp.MustCompile(
+	`refs/heads/\$([A-Za-z_][A-Za-z0-9_]*):refs/remotes/origin/\$([A-Za-z_][A-Za-z0-9_]*)`)
+
+// repairLegacyFetchRefspec braces the expansions in a Kandev-authored fetch
+// refspec that a profile persisted before the managed scripts were fixed. The
+// profile editor stores the generated default in
+// executor_profiles.prepare_script and resolvePrepareScript prefers that stored
+// value, so correcting the default alone would leave those profiles fetching
+// `refs/heads/mainefs/remotes/origin/main` under zsh.
+//
+// Only that exact fragment is rewritten. A hand-written script that uses a zsh
+// modifier of its own keeps whatever the author wrote.
+func repairLegacyFetchRefspec(script string) string {
+	return legacyFetchRefspec.ReplaceAllString(script, `refs/heads/$${$1}:refs/remotes/origin/$${$2}`)
+}
+
 func (r *SSHExecutor) resolvePrepareScript(req *ExecutorCreateRequest, workspacePath, agentctlBin string) (string, error) {
 	if req == nil {
 		return "", fmt.Errorf("ssh: prepare request is required")
@@ -48,6 +67,8 @@ func (r *SSHExecutor) resolvePrepareScript(req *ExecutorCreateRequest, workspace
 	script := strings.TrimSpace(getMetadataString(req.Metadata, MetadataKeySetupScript))
 	if script == "" {
 		script = DefaultPrepareScript(executorTypeSSH)
+	} else {
+		script = repairLegacyFetchRefspec(script)
 	}
 	if script == "" {
 		return "", nil

apps/backend/internal/agent/runtime/lifecycle/executor_ssh_scripts_test.go

@@ -75,18 +75,15 @@ func TestSSHResolvePrepareScriptUsesRemoteWorkspaceForContributionDestination(t
 	}
 }
 
-func TestSSHDefaultPrepareScriptExecutesCheckoutAndSetup(t *testing.T) {
-	if _, err := exec.LookPath("git"); err != nil {
-		t.Skip("git not available")
-	}
-	if _, err := exec.LookPath("bash"); err != nil {
-		t.Skip("bash not available")
-	}
-
-	root := t.TempDir()
-	origin := filepath.Join(root, "origin.git")
+// newSSHPrepareFixture seeds a bare origin with one commit on main and returns
+// the temp root, that origin path, and a task workspace that already carries
+// the session runtime directory the SSH executor creates before preparation.
+func newSSHPrepareFixture(t *testing.T) (root, origin, workspace string) {
+	t.Helper()
+	root = t.TempDir()
+	origin = filepath.Join(root, "origin.git")
 	seed := filepath.Join(root, "seed")
-	workspace := filepath.Join(root, "workspace")
+	workspace = filepath.Join(root, "workspace")
 	runIn(t, root, "git", "init", "--quiet", "--bare", "--initial-branch=main", origin)
 	runIn(t, root, "git", "init", "--quiet", "--initial-branch=main", seed)
 	runIn(t, seed, "git", "config", "user.email", "test@example.com")
@@ -97,6 +94,147 @@ func TestSSHDefaultPrepareScriptExecutesCheckoutAndSetup(t *testing.T) {
 	runIn(t, seed, "git", "remote", "add", "origin", origin)
 	runIn(t, seed, "git", "push", "--quiet", "origin", "main")
 	runIn(t, root, "mkdir", "-p", filepath.Join(workspace, ".kandev", "sessions", "session-1"))
+	return root, origin, workspace
+}
+
+// TestSSHDefaultPrepareScriptExecutesUnderZsh runs the resolved default
+// prepare script through zsh, the login shell macOS SSH targets use unless a
+// profile overrides it. An unbraced `$repository_branch:` in the fetch refspec
+// made zsh strip the `:r` and fetch `refs/heads/mainefs/remotes/origin/main`.
+func TestSSHDefaultPrepareScriptExecutesUnderZsh(t *testing.T) {
+	if _, err := exec.LookPath("git"); err != nil {
+		t.Skip("git not available")
+	}
+	if _, err := exec.LookPath("zsh"); err != nil {
+		t.Skip("zsh not available")
+	}
+
+	root, origin, workspace := newSSHPrepareFixture(t)
+	script, err := (&SSHExecutor{}).resolvePrepareScript(&ExecutorCreateRequest{
+		TaskID: "task-1",
+		Metadata: map[string]interface{}{
+			"repository_clone_url":    origin,
+			MetadataKeyBaseBranch:     "main",
+			MetadataKeyWorktreeBranch: "feature/task-1",
+		},
+	}, workspace, "/remote/bin/agentctl")
+	if err != nil {
+		t.Fatalf("resolvePrepareScript() error = %v", err)
+	}
+	cmd := exec.Command("zsh", "-e", "-c", script)
+	cmd.Dir = root
+	if output, err := cmd.CombinedOutput(); err != nil {
+		t.Fatalf("SSH prepare script failed under zsh: %v\n%s\nscript:\n%s", err, output, script)
+	}
+	if got := strings.TrimSpace(string(runIn(t, workspace, "git", "branch", "--show-current"))); got != "feature/task-1" {
+		t.Fatalf("prepared branch = %q, want feature/task-1", got)
+	}
+	if got := strings.TrimSpace(string(runIn(t, workspace, "git", "rev-parse", "--verify", "origin/main"))); got == "" {
+		t.Fatal("origin/main was not fetched")
+	}
+}
+
+// TestSSHPrepareScriptRepairsPersistedLegacyRefspec covers the upgrade path.
+// The profile editor persists the generated default in
+// executor_profiles.prepare_script, and resolvePrepareScript prefers that
+// stored value over DefaultPrepareScript, so bracing the managed script alone
+// leaves every profile created before that change still fetching
+// `refs/heads/mainefs/remotes/origin/main` under zsh.
+func TestSSHPrepareScriptRepairsPersistedLegacyRefspec(t *testing.T) {
+	if _, err := exec.LookPath("git"); err != nil {
+		t.Skip("git not available")
+	}
+	if _, err := exec.LookPath("zsh"); err != nil {
+		t.Skip("zsh not available")
+	}
+
+	// Exactly what a pre-fix profile stored: the current default with the
+	// refspec expansions unbraced again.
+	legacy := strings.ReplaceAll(
+		DefaultPrepareScript(executorTypeSSH),
+		"+refs/heads/${repository_branch}:refs/remotes/origin/${repository_branch}",
+		"+refs/heads/$repository_branch:refs/remotes/origin/$repository_branch",
+	)
+	if !strings.Contains(legacy, "+refs/heads/$repository_branch:") {
+		t.Fatal("legacy fixture did not reproduce the unbraced refspec")
+	}
+
+	root, origin, workspace := newSSHPrepareFixture(t)
+	script, err := (&SSHExecutor{}).resolvePrepareScript(&ExecutorCreateRequest{
+		TaskID: "task-1",
+		Metadata: map[string]interface{}{
+			MetadataKeySetupScript:    legacy,
+			"repository_clone_url":    origin,
+			MetadataKeyBaseBranch:     "main",
+			MetadataKeyWorktreeBranch: "feature/task-1",
+		},
+	}, workspace, "/remote/bin/agentctl")
+	if err != nil {
+		t.Fatalf("resolvePrepareScript() error = %v", err)
+	}
+	if strings.Contains(script, "+refs/heads/$repository_branch:") {
+		t.Fatalf("stored legacy refspec was not repaired:\n%s", script)
+	}
+
+	cmd := exec.Command("zsh", "-e", "-c", script)
+	cmd.Dir = root
+	if output, err := cmd.CombinedOutput(); err != nil {
+		t.Fatalf("repaired legacy prepare script failed under zsh: %v\n%s\nscript:\n%s", err, output, script)
+	}
+	if got := strings.TrimSpace(string(runIn(t, workspace, "git", "rev-parse", "--verify", "origin/main"))); got == "" {
+		t.Fatal("origin/main was not fetched")
+	}
+}
+
+// TestRepairLegacyFetchRefspec pins the repair to the fragment Kandev itself
+// generated, so a hand-written script that deliberately uses a zsh modifier is
+// left untouched.
+func TestRepairLegacyFetchRefspec(t *testing.T) {
+	cases := []struct{ name, in, want string }{
+		{
+			name: "kandev fetch refspec",
+			in:   `git fetch origin "+refs/heads/$repository_branch:refs/remotes/origin/$repository_branch"`,
+			want: `git fetch origin "+refs/heads/${repository_branch}:refs/remotes/origin/${repository_branch}"`,
+		},
+		{
+			name: "contribution base branch refspec",
+			in:   `git fetch origin "+refs/heads/$base_branch:refs/remotes/origin/$base_branch"`,
+			want: `git fetch origin "+refs/heads/${base_branch}:refs/remotes/origin/${base_branch}"`,
+		},
+		{
+			name: "already braced",
+			in:   `git fetch origin "+refs/heads/${repository_branch}:refs/remotes/origin/${repository_branch}"`,
+			want: `git fetch origin "+refs/heads/${repository_branch}:refs/remotes/origin/${repository_branch}"`,
+		},
+		{
+			name: "unrelated zsh modifier is preserved",
+			in:   `printf '%s\n' "$archive:t"`,
+			want: `printf '%s\n' "$archive:t"`,
+		},
+		{
+			name: "unrelated colon expansion is preserved",
+			in:   `PATH="$extra_bin:$PATH"`,
+			want: `PATH="$extra_bin:$PATH"`,
+		},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			if got := repairLegacyFetchRefspec(tc.in); got != tc.want {
+				t.Errorf("repairLegacyFetchRefspec()\n got: %s\nwant: %s", got, tc.want)
+			}
+		})
+	}
+}
+
+func TestSSHDefaultPrepareScriptExecutesCheckoutAndSetup(t *testing.T) {
+	if _, err := exec.LookPath("git"); err != nil {
+		t.Skip("git not available")
+	}
+	if _, err := exec.LookPath("bash"); err != nil {
+		t.Skip("bash not available")
+	}
+
+	root, origin, workspace := newSSHPrepareFixture(t)
 
 	executor := &SSHExecutor{}
 	req := &ExecutorCreateRequest{

docs/specs/executors/system-design/ssh-executor.md

@@ -105,6 +105,13 @@ Multiple sessions in the *same* task share the same worktree on disk (same files
   `agentctl` starts. A non-zero exit, timeout, missing primary checkout, or conflicting `origin`
   fails the launch; Kandev does not
   start `agentctl` or the agent in an empty or ambiguous workspace.
+- Managed preparation text is shell-portable. The profile shell may be `sh`, `bash`, or `zsh`, so
+  every parameter expansion that a `:` follows is braced (`${name}:`); zsh otherwise parses `$name:r`
+  as a modifier and silently corrupts fetch refspecs.
+- A profile stores the generated prepare script, and a stored script wins over the managed default,
+  so preparation repairs the one fragment Kandev itself generated before that rule existed: an
+  unbraced `refs/heads/$name:refs/remotes/origin/$name` is braced when the stored script is
+  resolved. Any other expansion in a stored script is left exactly as its author wrote it.
 - On subsequent sessions for the same task on the same host, the default preparation path reuses the
   matching checkout without deleting local commits or untracked work. The profile prepare script may
   run again because it is a per-session pre-launch hook and must therefore be idempotent when the