From 1c843f68e575a7d8b8b922fc12bbce64398f7ada Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Sat, 8 Aug 2026 19:52:00 +0100 Subject: [PATCH] fix: prepare SSH workspaces before agentctl (#2413) (#2439) --- .../runtime/lifecycle/default_scripts.go | 65 ++++ .../runtime/lifecycle/default_scripts_test.go | 23 ++ .../lifecycle/executor_sprites_lifecycle.go | 18 +- .../executor_sprites_lifecycle_test.go | 15 + .../agent/runtime/lifecycle/executor_ssh.go | 115 ++++--- .../runtime/lifecycle/executor_ssh_scripts.go | 267 ++++++++++++++++ .../lifecycle/executor_ssh_scripts_test.go | 288 ++++++++++++++++++ apps/web/e2e/fixtures/ssh-test-base.ts | 139 ++++++--- apps/web/e2e/tests/ssh/launch-task.spec.ts | 106 +++++++ docs/plans/ssh-workspace-preparation/plan.md | 144 +++++++++ .../task-01-ssh-workspace-lifecycle.md | 80 +++++ .../task-02-ssh-container-regression.md | 71 +++++ .../task-03-public-documentation-alignment.md | 64 ++++ docs/public/executors.md | 10 +- docs/public/feature-status.md | 2 +- docs/specs/ssh-executor/spec.md | 68 ++++- 16 files changed, 1366 insertions(+), 109 deletions(-) create mode 100644 apps/backend/internal/agent/runtime/lifecycle/executor_ssh_scripts.go create mode 100644 apps/backend/internal/agent/runtime/lifecycle/executor_ssh_scripts_test.go create mode 100644 docs/plans/ssh-workspace-preparation/plan.md create mode 100644 docs/plans/ssh-workspace-preparation/task-01-ssh-workspace-lifecycle.md create mode 100644 docs/plans/ssh-workspace-preparation/task-02-ssh-container-regression.md create mode 100644 docs/plans/ssh-workspace-preparation/task-03-public-documentation-alignment.md diff --git a/apps/backend/internal/agent/runtime/lifecycle/default_scripts.go b/apps/backend/internal/agent/runtime/lifecycle/default_scripts.go index ce01c66a32..559c9a2acd 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/default_scripts.go +++ b/apps/backend/internal/agent/runtime/lifecycle/default_scripts.go @@ -11,6 +11,8 @@ func DefaultPrepareScript(executorType string) string { return defaultDockerPrepareScript case "sprites": return defaultSpritesPrepareScript + case "ssh": + return defaultSSHPrepareScript default: return "" } @@ -182,3 +184,66 @@ echo "Starting agent controller..." {{kandev.agentctl.start}} echo "Prepare complete." ` + +//nolint:dupword // repeated shell `fi` tokens close distinct control-flow blocks. +const defaultSSHPrepareScript = `#!/bin/bash +# Prepare an SSH task workspace on the remote host. +# The script runs before the per-session agentctl instance starts. + +set -euo pipefail + +workspace={{workspace.path}} +repository_url={{repository.clone_url}} +repository_branch={{repository.branch}} + +# ---- Configure git/gh authentication ---- +{{github.auth_setup}} + +# ---- Git identity ---- +{{git.identity_setup}} + +mkdir -p "$workspace" + +# ---- Materialize or reuse the primary repository ---- +# The task directory already contains .kandev session files, so initialize the +# repository in place rather than using git clone into a non-empty directory. +if [ -n "$repository_url" ]; then + if git -C "$workspace" rev-parse --git-dir >/dev/null 2>&1; then + configured_url=$(git -C "$workspace" config --get remote.origin.url 2>/dev/null || true) + if [ -z "$configured_url" ]; then + git -C "$workspace" remote add origin "$repository_url" + elif [ "$configured_url" != "$repository_url" ]; then + echo 'kandev: target origin identity conflict' >&2 + exit 1 + fi + else + git init -q "$workspace" + git -C "$workspace" remote add origin "$repository_url" + fi + + git_dir=$(git -C "$workspace" rev-parse --absolute-git-dir) + exclude_file="$git_dir/info/exclude" + mkdir -p "$(dirname "$exclude_file")" + touch "$exclude_file" + grep -Fqx '/.kandev/' "$exclude_file" || printf '%s\n' '/.kandev/' >>"$exclude_file" + + if [ -z "$repository_branch" ]; 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 + echo 'kandev: target base branch is unavailable' >&2 + exit 1 + fi + # A reused checkout keeps its current branch, local commits, and untracked + # files. Only an empty repository needs its initial branch created. + if ! git -C "$workspace" rev-parse --verify HEAD >/dev/null 2>&1; then + git -C "$workspace" checkout -b "$repository_branch" "origin/$repository_branch" + fi +fi + +cd "$workspace" + +# ---- Repository setup (if configured) ---- +{{repository.setup_script}} +` diff --git a/apps/backend/internal/agent/runtime/lifecycle/default_scripts_test.go b/apps/backend/internal/agent/runtime/lifecycle/default_scripts_test.go index e46f3fd11b..4184a7e567 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/default_scripts_test.go +++ b/apps/backend/internal/agent/runtime/lifecycle/default_scripts_test.go @@ -212,3 +212,26 @@ func TestDefaultPrepareScripts_NoInlineFeatureBranchCheckout(t *testing.T) { }) } } + +func TestDefaultPrepareScript_SSHMaterializesPrimaryWorkspace(t *testing.T) { + script := DefaultPrepareScript("ssh") + if script == "" { + t.Fatal("DefaultPrepareScript(\"ssh\") returned empty") + } + for _, want := range []string{ + "{{workspace.path}}", + "{{repository.clone_url}}", + "{{repository.branch}}", + "{{repository.setup_script}}", + "git init", + "fetch --no-tags", + "remote.origin.url", + } { + if !strings.Contains(script, want) { + t.Errorf("SSH default prepare script missing %q", want) + } + } + if strings.Contains(script, "git checkout -B {{worktree.branch}} origin/{{worktree.branch}}") { + t.Fatal("SSH default prepare script must leave feature-branch selection to the postlude") + } +} diff --git a/apps/backend/internal/agent/runtime/lifecycle/executor_sprites_lifecycle.go b/apps/backend/internal/agent/runtime/lifecycle/executor_sprites_lifecycle.go index 975171692d..d35f8dd3ec 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/executor_sprites_lifecycle.go +++ b/apps/backend/internal/agent/runtime/lifecycle/executor_sprites_lifecycle.go @@ -135,17 +135,23 @@ func (r *SpritesExecutor) runTerminalCleanupScript(ctx context.Context, sprite * // shouldRunExecutorCleanup. Stale execution cleanup is intentionally excluded // from this shared set: Docker has a runtime-specific helper to remove local // stale containers, while Sprites preserves cloud sandboxes for faster and less -// destructive recovery. +// destructive recovery. Task-tree reasons are emitted by the archive/delete +// handoff path before its durable cascade cleanup job runs. const ( - StopReasonTaskArchived = "task archived" - StopReasonTaskDeleted = "task deleted" - StopReasonSessionArchived = "session archived" - StopReasonSessionDeleted = "session deleted" + StopReasonTaskArchived = "task archived" + StopReasonTaskDeleted = "task deleted" + StopReasonSessionArchived = "session archived" + StopReasonSessionDeleted = "session deleted" + StopReasonCascadeArchive = "cascade archive" + StopReasonCascadeDelete = "cascade delete" + StopReasonTaskTreeArchived = "task tree archived" + StopReasonTaskTreeDeleted = "task tree deleted" ) func shouldRunExecutorCleanup(reason string) bool { switch strings.ToLower(strings.TrimSpace(reason)) { - case StopReasonTaskArchived, StopReasonTaskDeleted, StopReasonSessionArchived, StopReasonSessionDeleted: + case StopReasonTaskArchived, StopReasonTaskDeleted, StopReasonSessionArchived, StopReasonSessionDeleted, + StopReasonCascadeArchive, StopReasonCascadeDelete, StopReasonTaskTreeArchived, StopReasonTaskTreeDeleted: return true default: return false diff --git a/apps/backend/internal/agent/runtime/lifecycle/executor_sprites_lifecycle_test.go b/apps/backend/internal/agent/runtime/lifecycle/executor_sprites_lifecycle_test.go index 2ce448ee8a..2a1be128f3 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/executor_sprites_lifecycle_test.go +++ b/apps/backend/internal/agent/runtime/lifecycle/executor_sprites_lifecycle_test.go @@ -119,6 +119,21 @@ func TestSpritesStopInstancePreservesSandboxOnSessionStop(t *testing.T) { } } +func TestShouldRunExecutorCleanupIncludesCascadeTerminalReasons(t *testing.T) { + for _, reason := range []string{ + StopReasonCascadeArchive, + StopReasonCascadeDelete, + StopReasonTaskTreeArchived, + StopReasonTaskTreeDeleted, + } { + t.Run(reason, func(t *testing.T) { + if !shouldRunExecutorCleanup(reason) { + t.Fatalf("shouldRunExecutorCleanup(%q) = false, want true", reason) + } + }) + } +} + // TestSpritesStopThenResumeReconnects proves the end-to-end integration of // the resume-fast path: a preserving StopInstance leaves enough state on the // instance metadata for the next launch to (a) decide it's a reconnect and diff --git a/apps/backend/internal/agent/runtime/lifecycle/executor_ssh.go b/apps/backend/internal/agent/runtime/lifecycle/executor_ssh.go index 167393f65d..1f6ad01891 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/executor_ssh.go +++ b/apps/backend/internal/agent/runtime/lifecycle/executor_ssh.go @@ -44,8 +44,12 @@ type sshSessionState struct { agentctlClient *agentctl.Client pid int remoteDir string + remoteTaskDir string authToken string reusingProcess bool + metadata map[string]interface{} + prepareEnv map[string]string + platform SSHRemotePlatform } // SSHExecutor implements ExecutorBackend for SSH-reachable Linux and macOS hosts. @@ -61,6 +65,8 @@ type SSHExecutor struct { logger *logger.Logger brokerPreflight func(context.Context, *ssh.Client, *ExecutorCreateRequest, SSHRemotePlatform) error stopRemote func(context.Context, *ssh.Client, string, int) error + closeClient func(*ssh.Client) error + cleanupScript func(context.Context, *ssh.Client, string, map[string]interface{}, map[string]string, SSHRemotePlatform, string, string) error mu sync.Mutex sessions map[string]*sshSessionState // keyed by ExecutorInstance.InstanceID @@ -83,6 +89,8 @@ func NewSSHExecutor( } executor.brokerPreflight = executor.preflightGitHubCredentialBroker executor.stopRemote = stopRemoteAgentctl + executor.closeClient = func(client *ssh.Client) error { return client.Close() } + executor.cleanupScript = executor.runCleanupScript return executor } @@ -111,12 +119,22 @@ func (r *SSHExecutor) Close() error { _ = s.forwarder.Close() } if s.client != nil { - _ = s.client.Close() + _ = r.closeSSHClient(s.client) } } return nil } +func (r *SSHExecutor) closeSSHClient(client *ssh.Client) error { + if client == nil { + return nil + } + if r.closeClient != nil { + return r.closeClient(client) + } + return client.Close() +} + // targetFromMetadata builds an SSHTarget from the executor metadata // propagated via buildLaunchMetadata (req.ExecutorConfig keys merged in). func (r *SSHExecutor) targetFromMetadata(md map[string]interface{}) (*SSHTarget, error) { @@ -205,39 +223,43 @@ func (r *SSHExecutor) CreateInstance(ctx context.Context, req *ExecutorCreateReq return nil, err } - if err := r.preflightAgentBinary(ctx, client, req, platform); err != nil { - return nil, err - } - workdir := r.workdirRoot(req.Metadata) - taskDir, sessionDir, err := r.prepareRemoteDirs(ctx, client, workdir, req) + taskDir, err := r.prepareRemoteTaskDir(ctx, client, workdir, req) if err != nil { return nil, err } r.maybeUploadCredentials(ctx, client, req, platform) + if err := r.runPrepareScript(ctx, client, taskDir, req, platform, agentctlBin); err != nil { + return nil, err + } + if err := r.verifyPrimaryCheckout(ctx, client, taskDir, req, platform); err != nil { + return nil, err + } + sessionDir, err := r.prepareRemoteSessionDir(ctx, client, taskDir, req) + if err != nil { + return nil, err + } + if err := r.preflightAgentBinary(ctx, client, req, platform); err != nil { + return nil, err + } port, pid, fwd, authToken, err := r.startAndForwardAgentctl(ctx, client, agentctlBin, taskDir, sessionDir, req, platform) if err != nil { return nil, err } - if binding, ok := req.RemoteContributions[""]; ok { - if err := r.materializeSSHRemoteContribution(ctx, client, agentctlBin, taskDir, req, platform, &binding); err != nil { - _ = fwd.Close() - cleanupCtx, cancel := sshRemoteCleanupContext(ctx) - _ = stopRemoteAgentctl(cleanupCtx, client, sessionDir, pid) - cancel() - return nil, err - } - } r.mu.Lock() r.sessions[req.InstanceID] = &sshSessionState{ - target: target, - client: client, - forwarder: fwd, - pid: pid, - remoteDir: sessionDir, - authToken: authToken, + target: target, + client: client, + forwarder: fwd, + pid: pid, + remoteDir: sessionDir, + remoteTaskDir: taskDir, + authToken: authToken, + metadata: cloneSSHMetadata(req.Metadata), + prepareEnv: sshRemoteContributionEnv(req, agentctlBin), + platform: platform, } r.mu.Unlock() released = true // ownership transferred to session state; released on StopInstance @@ -306,20 +328,25 @@ func (r *SSHExecutor) prepareRemoteHost( return agentctlBin, info.Platform, nil } -// prepareRemoteDirs makes /tasks/ and /.kandev/sessions/. -func (r *SSHExecutor) prepareRemoteDirs(ctx context.Context, client *ssh.Client, workdir string, req *ExecutorCreateRequest) (string, string, error) { +// prepareRemoteTaskDir makes /tasks/ before preparation runs. +func (r *SSHExecutor) prepareRemoteTaskDir(ctx context.Context, client *ssh.Client, workdir string, req *ExecutorCreateRequest) (string, error) { taskDir, err := ensureRemoteTaskDir(ctx, client, workdir, sshTaskDirName(req)) if err != nil { r.report(req.OnProgress, "Preparing task directory", PrepareStepFailed, err.Error()) - return "", "", err - } - sessionDir, err := ensureRemoteSessionDir(ctx, client, taskDir, req.SessionID) - if err != nil { - r.report(req.OnProgress, "Preparing task directory", PrepareStepFailed, err.Error()) - return "", "", err + return "", err } r.report(req.OnProgress, "Preparing task directory", PrepareStepCompleted, taskDir) - return taskDir, sessionDir, nil + return taskDir, nil +} + +func (r *SSHExecutor) prepareRemoteSessionDir(ctx context.Context, client *ssh.Client, taskDir string, req *ExecutorCreateRequest) (string, error) { + sessionDir, err := ensureRemoteSessionDir(ctx, client, taskDir, req.SessionID) + if err != nil { + r.report(req.OnProgress, "Preparing session directory", PrepareStepFailed, err.Error()) + return "", err + } + r.report(req.OnProgress, "Preparing session directory", PrepareStepCompleted, sessionDir) + return sessionDir, nil } // startAndForwardAgentctl spawns the per-session agentctl on the remote, @@ -501,12 +528,18 @@ func (r *SSHExecutor) StopInstance(ctx context.Context, instance *ExecutorInstan if state.forwarder != nil { _ = state.forwarder.Close() } + if shouldRunExecutorCleanup(instance.StopReason) && state.client != nil && r.cleanupScript != nil { + metadata := mergeSSHMetadata(state.metadata, instance.Metadata) + if err := r.cleanupScript(ctx, state.client, state.remoteTaskDir, metadata, state.prepareEnv, state.platform, instance.InstanceID, instance.StopReason); err != nil { + r.logger.Warn("failed to run SSH cleanup script", zap.String("instance_id", instance.InstanceID), zap.Error(err)) + } + } if !sshShouldStopRemoteAgentctl(instance, force) { r.logger.Info("preserving SSH agentctl after agent stop", zap.String("instance_id", instance.InstanceID), zap.String("stop_reason", instance.StopReason)) if state.client != nil { - _ = state.client.Close() + _ = r.closeSSHClient(state.client) } return nil } @@ -516,9 +549,13 @@ func (r *SSHExecutor) StopInstance(ctx context.Context, instance *ExecutorInstan // the connection on the floor. if state.client != nil { cleanupCtx, cancel := sshRemoteCleanupContext(ctx) - _ = stopRemoteAgentctl(cleanupCtx, state.client, state.remoteDir, state.pid) + stopRemote := r.stopRemote + if stopRemote == nil { + stopRemote = stopRemoteAgentctl + } + _ = stopRemote(cleanupCtx, state.client, state.remoteDir, state.pid) cancel() - _ = state.client.Close() + _ = r.closeSSHClient(state.client) } return nil } @@ -588,6 +625,11 @@ func (r *SSHExecutor) ResumeRemoteInstance(ctx context.Context, req *ExecutorCre if err != nil { return fmt.Errorf("ssh resume: connect: %w", err) } + agentctlBin, err := expandRemoteHome(ctx, client, sshRemoteAgentctlPath) + if err != nil { + _ = client.Close() + return fmt.Errorf("ssh resume: resolve agentctl path: %w", err) + } // SSH liveness is judged on the REMOTE host: MetadataKeySSHRemoteAgentctlPID // (mirrored into executors_running.pid) is a pid on the remote box, so it is @@ -624,8 +666,11 @@ func (r *SSHExecutor) ResumeRemoteInstance(ctx context.Context, req *ExecutorCre agentctlClient: instanceClient, pid: pid, remoteDir: sessionDir, + remoteTaskDir: taskDir, authToken: req.AuthToken, reusingProcess: reusingProcess, + metadata: cloneSSHMetadata(req.Metadata), + prepareEnv: sshRemoteContributionEnv(req, agentctlBin), } r.mu.Unlock() @@ -731,12 +776,12 @@ func (r *SSHExecutor) resetTrackedManagedBrokerResume( } if err := stopRemote(ctx, state.client, state.remoteDir, state.pid); err != nil { if state.client != nil { - _ = state.client.Close() + _ = r.closeSSHClient(state.client) } return fmt.Errorf("ssh resume: stop stale broker-backed agentctl: %w", err) } if state.client != nil { - _ = state.client.Close() + _ = r.closeSSHClient(state.client) } clearSSHResumeRuntimeMetadata(req.Metadata) return nil diff --git a/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_scripts.go b/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_scripts.go new file mode 100644 index 0000000000..a79fc7c882 --- /dev/null +++ b/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_scripts.go @@ -0,0 +1,267 @@ +package lifecycle + +import ( + "context" + "encoding/json" + "fmt" + "strings" + "time" + + "go.uber.org/zap" + "golang.org/x/crypto/ssh" + + "github.com/kandev/kandev/internal/scriptengine" +) + +const ( + sshPrepareTimeout = 10 * time.Minute + sshCleanupTimeout = 60 * time.Second + sshPrepareOutputMaxLines = 20 +) + +func cloneSSHMetadata(metadata map[string]interface{}) map[string]interface{} { + if len(metadata) == 0 { + return nil + } + clone := make(map[string]interface{}, len(metadata)) + for key, value := range metadata { + clone[key] = value + } + return clone +} + +func mergeSSHMetadata(base, overlay map[string]interface{}) map[string]interface{} { + merged := cloneSSHMetadata(base) + if merged == nil { + merged = make(map[string]interface{}) + } + for key, value := range overlay { + merged[key] = value + } + return merged +} + +func (r *SSHExecutor) resolvePrepareScript(req *ExecutorCreateRequest, workspacePath, agentctlBin string) (string, error) { + if req == nil { + return "", fmt.Errorf("ssh: prepare request is required") + } + script := strings.TrimSpace(getMetadataString(req.Metadata, MetadataKeySetupScript)) + if script == "" { + script = DefaultPrepareScript("ssh") + } + if script == "" { + return "", nil + } + script += KandevBranchCheckoutPostlude() + if binding, ok := req.RemoteContributions[""]; ok { + targetURL := getMetadataString(req.Metadata, "repository_clone_url") + if targetURL == "" { + return "", fmt.Errorf("ssh: remote contribution target has no clone URL") + } + script += "\n" + sshRemoteContributionScript(workspacePath, targetURL, &binding) + } + return r.resolveSSHScript(req, workspacePath, agentctlBin, script) +} + +func (r *SSHExecutor) resolveSSHScript(req *ExecutorCreateRequest, workspacePath, agentctlBin, script string) (string, error) { + if req == nil { + return "", fmt.Errorf("ssh: script request is required") + } + resolver := scriptengine.NewResolver(). + WithStatic(map[string]string{ + "repository.clone_url": "''", + "kandev.agentctl.port": "", + "kandev.agentctl.install": "", + "kandev.agentctl.start": "", + }). + WithProvider(scriptengine.WorkspaceProvider(workspacePath)). + WithProvider(scriptengine.GitIdentityProvider(req.Metadata)). + WithProvider(scriptengine.GitHubAuthProvider(req.Env)). + WithProvider(scriptengine.AgentInstallProvider(r.collectAgentInstallScripts(req))). + WithProvider(scriptengine.WorktreeProvider( + workspacePath, + workspacePath, + getMetadataString(req.Metadata, MetadataKeyWorktreeID), + getMetadataString(req.Metadata, MetadataKeyWorktreeBranch), + getMetadataString(req.Metadata, MetadataKeyBaseBranch), + )). + WithProvider(scriptengine.RepositoryProvider(req.Metadata, nil, getGitRemoteURL, nil)) + + resolved := resolver.Resolve(script) + if strings.Contains(resolved, "{{") { + return "", fmt.Errorf("ssh: unresolved prepare script placeholder") + } + _ = agentctlBin // agentctl placeholders are intentionally disabled for SSH scripts. + return strings.TrimSpace(resolved), nil +} + +func (r *SSHExecutor) collectAgentInstallScripts(req *ExecutorCreateRequest) []string { + if r.agentList == nil || req == nil { + return nil + } + ids := map[string]bool{} + if req.AgentConfig != nil { + ids[req.AgentConfig.ID()] = true + } + addMethodAgentIDs(ids, req.Metadata, "remote_credentials", func(raw string) []string { + var values []string + if json.Unmarshal([]byte(raw), &values) == nil { + return values + } + return nil + }) + addMethodAgentIDs(ids, req.Metadata, "remote_auth_secrets", func(raw string) []string { + var values map[string]string + if json.Unmarshal([]byte(raw), &values) == nil { + keys := make([]string, 0, len(values)) + for key := range values { + keys = append(keys, key) + } + return keys + } + return nil + }) + var scripts []string + for _, agent := range r.agentList.ListEnabled() { + if ids[agent.ID()] && strings.TrimSpace(agent.InstallScript()) != "" { + scripts = append(scripts, agent.InstallScript()) + } + } + return scripts +} + +func addMethodAgentIDs(ids map[string]bool, metadata map[string]interface{}, key string, decode func(string) []string) { + raw := getMetadataString(metadata, key) + for _, methodID := range decode(raw) { + if agentID := extractAgentID(methodID); agentID != "" { + ids[agentID] = true + } + } +} + +func (r *SSHExecutor) runPrepareScript(ctx context.Context, client *ssh.Client, taskDir string, req *ExecutorCreateRequest, platform SSHRemotePlatform, agentctlBin string) error { + script, err := r.resolvePrepareScript(req, taskDir, agentctlBin) + if err != nil { + return err + } + if script == "" { + return nil + } + env := sshRemoteContributionEnv(req, agentctlBin) + envScript, err := buildSSHEnvInitScript(env) + if err != nil { + return fmt.Errorf("ssh: prepare script environment: %w", err) + } + r.report(req.OnProgress, "Running prepare script", PrepareStepRunning, "") + stepCtx, cancel := context.WithTimeout(ctx, sshPrepareTimeout) + defer cancel() + shell := sshShellForRemote(req.Metadata, platform) + command := WrapLoginShell(shell, sshScriptWithEnvironment(script)) + stdout, stderr, runErr := runSSHCommandStdin(stepCtx, client, command, strings.NewReader(envScript)) + output := redactSSHScriptOutput(stdout+"\n"+stderr, env) + if runErr != nil { + r.report(req.OnProgress, "Running prepare script", PrepareStepFailed, output) + return fmt.Errorf("ssh: prepare script failed") + } + r.report(req.OnProgress, "Running prepare script", PrepareStepCompleted, output) + return nil +} + +func sshScriptWithEnvironment(script string) string { + return "set -ae\n. /dev/stdin\nset +a\nset -e\n" + script +} + +func redactSSHScriptOutput(output string, env map[string]string) string { + for _, value := range env { + if value != "" { + output = strings.ReplaceAll(output, value, "") + } + } + return lastLines(strings.TrimSpace(output), sshPrepareOutputMaxLines) +} + +func sshCommandFailureDetail(stdout, stderr string, err error) string { + detail := strings.TrimSpace(stdout + "\n" + stderr) + if err != nil { + if detail != "" { + detail += "\n" + } + detail += err.Error() + } + return detail +} + +func normalizeSSHRemotePath(path string) string { + path = strings.TrimSpace(path) + if path == "" { + return "" + } + trimmed := strings.TrimRight(path, "/") + if trimmed == "" { + return "/" + } + return trimmed +} + +func (r *SSHExecutor) verifyPrimaryCheckout(ctx context.Context, client *ssh.Client, taskDir string, req *ExecutorCreateRequest, platform SSHRemotePlatform) error { + targetURL := getMetadataString(req.Metadata, "repository_clone_url") + if targetURL == "" { + return nil + } + shell := sshShellForRemote(req.Metadata, platform) + canonicalTaskDir, taskDirStderr, taskDirErr := runSSHCommand(ctx, client, WrapLoginShell(shell, "cd "+shellQuote(taskDir)+" && pwd -P")) + if taskDirErr != nil { + r.report(req.OnProgress, "Verifying primary checkout", PrepareStepFailed, sshCommandFailureDetail(canonicalTaskDir, taskDirStderr, taskDirErr)) + return fmt.Errorf("ssh: primary repository checkout is missing") + } + root, rootStderr, rootErr := runSSHCommand(ctx, client, WrapLoginShell(shell, "git -C "+shellQuote(taskDir)+" rev-parse --show-toplevel")) + if rootErr != nil || normalizeSSHRemotePath(root) != normalizeSSHRemotePath(canonicalTaskDir) { + detail := sshCommandFailureDetail(root, rootStderr, rootErr) + if rootErr == nil { + detail = fmt.Sprintf("checkout root %q does not match task directory %q", strings.TrimSpace(root), strings.TrimSpace(canonicalTaskDir)) + } + r.report(req.OnProgress, "Verifying primary checkout", PrepareStepFailed, detail) + return fmt.Errorf("ssh: primary repository checkout is missing") + } + origin, originStderr, originErr := runSSHCommand(ctx, client, WrapLoginShell(shell, "git -C "+shellQuote(taskDir)+" config --get remote.origin.url")) + if originErr != nil || strings.TrimSpace(origin) != targetURL { + r.report(req.OnProgress, "Verifying primary checkout", PrepareStepFailed, sshCommandFailureDetail(origin, originStderr, originErr)) + return fmt.Errorf("ssh: primary repository origin does not match configured repository") + } + if head, headStderr, headErr := runSSHCommand(ctx, client, WrapLoginShell(shell, "git -C "+shellQuote(taskDir)+" rev-parse --verify HEAD")); headErr != nil { + r.report(req.OnProgress, "Verifying primary checkout", PrepareStepFailed, sshCommandFailureDetail(head, headStderr, headErr)) + return fmt.Errorf("ssh: primary repository has no checkout") + } + r.report(req.OnProgress, "Verifying primary checkout", PrepareStepCompleted, strings.TrimSpace(canonicalTaskDir)) + return nil +} + +func (r *SSHExecutor) runCleanupScript(ctx context.Context, client *ssh.Client, taskDir string, metadata map[string]interface{}, env map[string]string, platform SSHRemotePlatform, instanceID, reason string) error { + script := strings.TrimSpace(getMetadataString(metadata, MetadataKeyCleanupScript)) + if script == "" { + return nil + } + if taskDir == "" { + return fmt.Errorf("ssh: cleanup task directory is missing") + } + resolved, err := r.resolveSSHScript(&ExecutorCreateRequest{Metadata: metadata, Env: env}, taskDir, "", script) + if err != nil { + return err + } + envScript, err := buildSSHEnvInitScript(env) + if err != nil { + return err + } + cleanupCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), sshCleanupTimeout) + defer cancel() + shell := sshShellForRemote(metadata, platform) + stdout, stderr, runErr := runSSHCommandStdin(cleanupCtx, client, WrapLoginShell(shell, sshScriptWithEnvironment(resolved)), strings.NewReader(envScript)) + if runErr != nil { + r.logger.Warn("ssh cleanup script failed", + zap.String("instance_id", instanceID), zap.String("reason", reason), + zap.String("output", redactSSHScriptOutput(stdout+"\n"+stderr, env)), zap.Error(runErr)) + return fmt.Errorf("ssh: cleanup script failed") + } + r.logger.Debug("ssh cleanup script completed", zap.String("instance_id", instanceID), zap.String("reason", reason)) + return nil +} diff --git a/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_scripts_test.go b/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_scripts_test.go new file mode 100644 index 0000000000..3c703c6ac2 --- /dev/null +++ b/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_scripts_test.go @@ -0,0 +1,288 @@ +package lifecycle + +import ( + "context" + "errors" + "os/exec" + "path/filepath" + "strings" + "testing" + + "golang.org/x/crypto/ssh" + + "github.com/kandev/kandev/internal/common/logger" +) + +func TestSSHResolvePrepareScriptUsesWorkspaceProviders(t *testing.T) { + executor := &SSHExecutor{} + req := &ExecutorCreateRequest{ + TaskID: "task-1", + Metadata: map[string]interface{}{ + MetadataKeySetupScript: "printf setup > {{workspace.path}}/custom-marker", + "repository_clone_url": "https://github.com/acme/widgets.git", + MetadataKeyBaseBranch: "main", + MetadataKeyWorktreeBranch: "feature/task-1", + MetadataKeyRepoSetupScript: "printf repo > repo-marker", + }, + } + + script, err := executor.resolvePrepareScript(req, "/remote/task", "/remote/bin/agentctl") + if err != nil { + t.Fatalf("resolvePrepareScript() error = %v", err) + } + for _, want := range []string{ + "/remote/task", + "custom-marker", + "kandev-managed: ensure session feature branch is checked out", + } { + if !strings.Contains(script, want) { + t.Errorf("resolved SSH prepare script missing %q:\n%s", want, script) + } + } + if strings.Contains(script, "{{") { + t.Fatalf("resolved SSH prepare script still contains placeholders:\n%s", script) + } + if strings.Contains(script, "agentctl --") || strings.Contains(script, "chmod +x /usr/local/bin/agentctl") { + t.Fatalf("SSH prepare script must not start or install a duplicate agentctl:\n%s", script) + } +} + +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") + seed := filepath.Join(root, "seed") + 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") + runIn(t, seed, "git", "config", "user.name", "Test") + runIn(t, seed, "sh", "-c", "printf repository > README.md") + runIn(t, seed, "git", "add", "README.md") + runIn(t, seed, "git", "commit", "--quiet", "-m", "init") + 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")) + + executor := &SSHExecutor{} + req := &ExecutorCreateRequest{ + TaskID: "task-1", + Metadata: map[string]interface{}{ + "repository_clone_url": origin, + MetadataKeyBaseBranch: "main", + MetadataKeyWorktreeBranch: "feature/task-1", + MetadataKeyRepoSetupScript: "printf setup > setup-marker", + }, + } + script, err := executor.resolvePrepareScript(req, workspace, "/remote/bin/agentctl") + if err != nil { + t.Fatalf("resolvePrepareScript() error = %v", err) + } + cmd := exec.Command("bash", "-e", "-c", script) + cmd.Dir = root + output, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("SSH prepare script failed: %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", "remote", "get-url", "origin"))); got != origin { + t.Fatalf("prepared origin = %q, want %q", got, origin) + } + if got := strings.TrimSpace(string(runIn(t, workspace, "cat", "README.md"))); got != "repository" { + t.Fatalf("prepared repository content = %q, want repository", got) + } + if got := strings.TrimSpace(string(runIn(t, workspace, "cat", "setup-marker"))); got != "setup" { + t.Fatalf("setup marker = %q, want setup", got) + } + if got := strings.TrimSpace(string(runIn(t, workspace, "cat", filepath.Join(".git", "info", "exclude")))); !strings.Contains(got, "/.kandev/") { + t.Fatalf("git exclude = %q, want /.kandev/", got) + } + + runIn(t, workspace, "sh", "-c", "printf local > local.txt") + reuseCmd := exec.Command("bash", "-e", "-c", script) + reuseCmd.Dir = root + if output, err := reuseCmd.CombinedOutput(); err != nil { + t.Fatalf("reused SSH prepare script failed: %v\n%s", err, output) + } + if got := strings.TrimSpace(string(runIn(t, workspace, "cat", "local.txt"))); got != "local" { + t.Fatalf("reused workspace lost local file: %q", got) + } +} + +func TestSSHDefaultPrepareScriptRejectsConflictingOrigin(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() + workspace := filepath.Join(root, "workspace") + runIn(t, root, "git", "init", "--quiet", workspace) + runIn(t, workspace, "git", "remote", "add", "origin", "https://github.com/acme/other.git") + + executor := &SSHExecutor{} + req := &ExecutorCreateRequest{Metadata: map[string]interface{}{ + "repository_clone_url": originURLForTest, + MetadataKeyBaseBranch: "main", + }} + script, err := executor.resolvePrepareScript(req, workspace, "/remote/bin/agentctl") + if err != nil { + t.Fatalf("resolvePrepareScript() error = %v", err) + } + if output, err := exec.Command("bash", "-e", "-c", script).CombinedOutput(); err == nil { + t.Fatalf("conflicting origin unexpectedly succeeded; output: %s", output) + } +} + +func TestSSHCleanupScriptResolvesRemoteWorkspace(t *testing.T) { + executor := &SSHExecutor{} + metadata := map[string]interface{}{ + MetadataKeyCleanupScript: "printf cleanup > {{workspace.path}}/cleanup-marker", + MetadataKeySSHHost: "remote.example", + } + resolved, err := executor.resolveSSHScript(&ExecutorCreateRequest{Metadata: metadata}, "/remote/task", "", metadata[MetadataKeyCleanupScript].(string)) + if err != nil { + t.Fatalf("resolveSSHScript() error = %v", err) + } + if !strings.Contains(resolved, "printf cleanup > '/remote/task'/cleanup-marker") { + t.Fatalf("cleanup workspace placeholder not resolved: %s", resolved) + } + if strings.Contains(resolved, "{{") { + t.Fatalf("cleanup script still contains placeholders: %s", resolved) + } +} + +func TestSSHStopInstanceRunsTerminalCleanupBeforeControllerTeardown(t *testing.T) { + tests := []struct { + name string + reason string + wantCleanup bool + wantStop bool + cleanupErr error + wantCleanupError bool + }{ + { + name: "task deletion runs cleanup", + reason: StopReasonTaskDeleted, + wantCleanup: true, + wantStop: true, + }, + { + name: "task tree archive runs cleanup", + reason: StopReasonTaskTreeArchived, + wantCleanup: true, + wantStop: true, + }, + { + name: "cascade delete runs cleanup", + reason: StopReasonCascadeDelete, + wantCleanup: true, + wantStop: true, + }, + { + name: "cleanup failure still stops controller", + reason: StopReasonSessionArchived, + wantCleanup: true, + wantStop: true, + cleanupErr: errors.New("cleanup failed"), + wantCleanupError: true, + }, + { + name: "ordinary stop preserves workspace without cleanup", + reason: "stopped via API", + wantStop: true, + }, + { + name: "backend shutdown skips cleanup and remote stop", + reason: StopReasonBackendShutdown, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + executor := NewSSHExecutor(nil, nil, nil, logger.Default()) + var cleanupCalls, stopCalls, closeCalls int + var cleanupTaskDir, cleanupReason string + var order []string + executor.cleanupScript = func( + _ context.Context, + _ *ssh.Client, + taskDir string, + _ map[string]interface{}, + _ map[string]string, + _ SSHRemotePlatform, + _ string, + reason string, + ) error { + cleanupCalls++ + order = append(order, "cleanup") + cleanupTaskDir = taskDir + cleanupReason = reason + return tc.cleanupErr + } + executor.stopRemote = func(context.Context, *ssh.Client, string, int) error { + stopCalls++ + order = append(order, "stop") + return nil + } + executor.closeClient = func(*ssh.Client) error { + closeCalls++ + order = append(order, "close") + return nil + } + executor.sessions["instance-1"] = &sshSessionState{ + client: &ssh.Client{}, + remoteDir: "/remote/session", + remoteTaskDir: "/remote/task", + metadata: map[string]interface{}{MetadataKeySSHRemoteTaskDir: "/wrong/metadata/task"}, + prepareEnv: map[string]string{"GITHUB_TOKEN": "secret"}, + } + + err := executor.StopInstance(context.Background(), &ExecutorInstance{ + InstanceID: "instance-1", + StopReason: tc.reason, + }, false) + if err != nil { + t.Fatalf("StopInstance() error = %v", err) + } + if got := cleanupCalls > 0; got != tc.wantCleanup { + t.Fatalf("cleanup called = %v, want %v", got, tc.wantCleanup) + } + if tc.wantCleanup { + if cleanupTaskDir != "/remote/task" { + t.Fatalf("cleanup task directory = %q, want /remote/task", cleanupTaskDir) + } + if cleanupReason != tc.reason { + t.Fatalf("cleanup reason = %q, want %q", cleanupReason, tc.reason) + } + } + if got := stopCalls > 0; got != tc.wantStop { + t.Fatalf("remote stop called = %v, want %v", got, tc.wantStop) + } + if closeCalls != 1 { + t.Fatalf("SSH client close calls = %d, want 1", closeCalls) + } + if tc.wantCleanupError && cleanupCalls != 1 { + t.Fatalf("cleanup error path calls = %d, want 1", cleanupCalls) + } + if tc.wantCleanup { + if len(order) < 3 || order[0] != "cleanup" { + t.Fatalf("teardown order = %v, want cleanup before stop/close", order) + } + } + }) + } +} + +const originURLForTest = "https://github.com/acme/widgets.git" diff --git a/apps/web/e2e/fixtures/ssh-test-base.ts b/apps/web/e2e/fixtures/ssh-test-base.ts index 494ccc99fc..7111a58b5a 100644 --- a/apps/web/e2e/fixtures/ssh-test-base.ts +++ b/apps/web/e2e/fixtures/ssh-test-base.ts @@ -1,12 +1,17 @@ import { expect, type Page, test as base } from "@playwright/test"; -import { execSync } from "node:child_process"; -import fs from "node:fs"; +import { execFileSync } from "node:child_process"; +import { + execInContainer, + startSSHServer, + stopSSHServer, + type SSHServerHandle, +} from "../helpers/ssh"; import path from "node:path"; import { backendFixture, type BackendContext } from "./backend"; import { hasSSHContainerSupport, buildE2ESSHImage, SSH_E2E_IMAGE_TAG } from "./ssh-image"; -import { startSSHServer, stopSSHServer, type SSHServerHandle } from "../helpers/ssh"; import { ApiClient } from "../helpers/api-client"; import { makeGitEnv } from "../helpers/git-helper"; +import { startHTTPGitFixture } from "../helpers/http-git-server"; import type { WorkflowStep } from "../../lib/types/http"; export type SSHSeedData = { @@ -26,6 +31,8 @@ export type SSHSeedData = { sshExecutorProfileId: string; /** Live handle to the sshd container the executor connects to. */ sshTarget: SSHServerHandle; + /** Removes the worker-scoped HTTP Git fixture and its SSH URL rewrite. */ + sshGitFixtureCleanup: () => Promise; }; /** @@ -86,10 +93,12 @@ export const sshTest = backendFixture.extend< throw e; } + let seed: Omit | undefined; try { - const seed = await seedSSHWorkspace(apiClient, backend, sshTarget); + seed = await seedSSHWorkspace(apiClient, backend, sshTarget); await use({ ...seed, sshTarget }); } finally { + await seed?.sshGitFixtureCleanup().catch(() => undefined); stopSSHServer(sshTarget); } }, @@ -171,53 +180,83 @@ async function seedSSHWorkspace( const sorted = steps.sort((a, b) => a.position - b.position); const startStep = sorted.find((s) => s.is_start_step) ?? sorted[0]; - // SSH executor clones inside the remote container; needs a fetchable URL. - // Same offline-bare-repo trick the Docker fixture uses. - const remoteDir = path.join(backend.tmpDir, "repos", "e2e-ssh-remote.git"); - fs.mkdirSync(path.dirname(remoteDir), { recursive: true }); - const gitEnv = makeGitEnv(backend.tmpDir); - execSync(`git init --bare -b main "${remoteDir}"`, { env: gitEnv }); - - const repoDir = path.join(backend.tmpDir, "repos", "e2e-ssh-repo"); - fs.mkdirSync(repoDir, { recursive: true }); - execSync("git init -b main", { cwd: repoDir, env: gitEnv }); - execSync('git commit --allow-empty -m "init"', { cwd: repoDir, env: gitEnv }); - execSync(`git remote add origin "file://${remoteDir}"`, { cwd: repoDir, env: gitEnv }); - execSync("git push origin main", { cwd: repoDir, env: gitEnv }); - const repo = await apiClient.createRepository(workspace.id, repoDir); - - const { agents } = await apiClient.listAgents(); - const mock = agents.find((a) => a.name === "mock-agent"); - const agentProfileId = mock?.profiles[0]?.id; - if (!agentProfileId) throw new Error("SSH E2E seed: mock-agent profile missing"); - - const sshExecutor = await apiClient.createSSHExecutor("E2E SSH Target", { - ssh_host: sshTarget.host, - ssh_port: String(sshTarget.port), - ssh_user: sshTarget.user, - ssh_identity_source: "file", - ssh_identity_file: sshTarget.identityFile, - ssh_host_fingerprint: sshTarget.hostFingerprint, - }); - - const profile = await apiClient.createExecutorProfile(sshExecutor.id, { - name: "E2E SSH", - config: {}, - prepare_script: "", - cleanup_script: "", - env_vars: [], - }); - - return { - workspaceId: workspace.id, - workflowId: workflow.id, - startStepId: startStep.id, - steps: sorted, - repositoryId: repo.id, - agentProfileId, - sshExecutorId: sshExecutor.id, - sshExecutorProfileId: profile.id, + // SSH executor clones inside the remote container, so the fixture must be + // reachable from that container rather than using a host-only file:// URL. + // The HTTP server is disposable and the SSH target rewrites the canonical + // provider URL to its Docker-bridge endpoint without changing Git origin. + const gitFixture = await startHTTPGitFixture(backend.tmpDir, "e2e-ssh"); + const localRepoDir = path.join(backend.tmpDir, "repos", "e2e-ssh-repo"); + const localGitEnv = makeGitEnv(backend.tmpDir); + execFileSync( + "git", + ["clone", path.join(backend.tmpDir, "fixture", "e2e-ssh.git"), localRepoDir], + { env: localGitEnv }, + ); + const rewriteKey = gitFixture.gitConfigEnvVars.find( + ({ key }) => key === "GIT_CONFIG_KEY_0", + )?.value; + const rewriteValue = gitFixture.gitConfigEnvVars.find( + ({ key }) => key === "GIT_CONFIG_VALUE_0", + )?.value; + if (!rewriteKey || !rewriteValue) { + await gitFixture.close(); + throw new Error("SSH E2E seed: HTTP Git fixture did not provide its URL rewrite"); + } + const sshGitFixtureCleanup = async () => { + try { + execInContainer(sshTarget, ["git", "config", "--system", "--unset-all", rewriteKey]); + } catch { + // The target may already have exited after a failed fixture setup. + } + await gitFixture.close(); }; + try { + execInContainer(sshTarget, ["git", "config", "--system", rewriteKey, rewriteValue]); + const repo = await apiClient.createRepository(workspace.id, gitFixture.checkoutPath, "main", { + name: "fixture/e2e-ssh", + provider: "gitlab", + provider_host: "https://gitlab.com", + provider_owner: "fixture", + provider_name: "e2e-ssh", + }); + + const { agents } = await apiClient.listAgents(); + const mock = agents.find((a) => a.name === "mock-agent"); + const agentProfileId = mock?.profiles[0]?.id; + if (!agentProfileId) throw new Error("SSH E2E seed: mock-agent profile missing"); + + const sshExecutor = await apiClient.createSSHExecutor("E2E SSH Target", { + ssh_host: sshTarget.host, + ssh_port: String(sshTarget.port), + ssh_user: sshTarget.user, + ssh_identity_source: "file", + ssh_identity_file: sshTarget.identityFile, + ssh_host_fingerprint: sshTarget.hostFingerprint, + }); + + const profile = await apiClient.createExecutorProfile(sshExecutor.id, { + name: "E2E SSH", + config: {}, + prepare_script: "", + cleanup_script: "", + env_vars: [], + }); + + return { + workspaceId: workspace.id, + workflowId: workflow.id, + startStepId: startStep.id, + steps: sorted, + repositoryId: repo.id, + agentProfileId, + sshExecutorId: sshExecutor.id, + sshExecutorProfileId: profile.id, + sshGitFixtureCleanup, + }; + } catch (error) { + await sshGitFixtureCleanup().catch(() => undefined); + throw error; + } } export { expect } from "@playwright/test"; diff --git a/apps/web/e2e/tests/ssh/launch-task.spec.ts b/apps/web/e2e/tests/ssh/launch-task.spec.ts index cafee06daf..59dba48866 100644 --- a/apps/web/e2e/tests/ssh/launch-task.spec.ts +++ b/apps/web/e2e/tests/ssh/launch-task.spec.ts @@ -39,6 +39,112 @@ test.describe("ssh executor — task launch", () => { const env = await apiClient.getTaskEnvironment(task.id); expect(env).not.toBeNull(); expect(env!.executor_type).toBe("ssh"); + + const row = (await apiClient.listSSHSessions(seedData.sshExecutorId)).find( + (session) => session.task_id === task.id, + ); + expect(row?.remote_task_dir).toBeTruthy(); + const workspace = row!.remote_task_dir!; + expect( + execInContainer(seedData.sshTarget, [ + "git", + "-c", + "safe.directory=*", + "-C", + workspace, + "rev-parse", + "--show-toplevel", + ]).trim(), + ).toBe(workspace); + expect( + execInContainer(seedData.sshTarget, [ + "git", + "-c", + "safe.directory=*", + "-C", + workspace, + "branch", + "--show-current", + ]).trim(), + ).not.toBe("main"); + expect(readRemoteFile(seedData.sshTarget, `${workspace}/remote-source.txt`)).toBe( + "e2e-ssh fixture\n", + ); + }); + + test("runs custom prepare and terminal cleanup hooks on the remote workspace", async ({ + apiClient, + seedData, + }) => { + test.setTimeout(180_000); + const profile = await apiClient.createExecutorProfile(seedData.sshExecutorId, { + name: "E2E SSH hooks", + config: {}, + prepare_script: `#!/bin/bash +set -euo pipefail +workspace={{workspace.path}} +repository_url={{repository.clone_url}} +repository_branch={{repository.branch}} +git init -q "$workspace" +git -C "$workspace" remote add origin "$repository_url" +git -C "$workspace" fetch --no-tags origin "+refs/heads/$repository_branch:refs/remotes/origin/$repository_branch" +git -C "$workspace" checkout -b "$repository_branch" "origin/$repository_branch" +printf 'prepared\\n' > "$workspace/custom-prepare-marker" +`, + cleanup_script: "printf 'cleaned\\n' > {{workspace.path}}/custom-cleanup-marker", + env_vars: [], + }); + + try { + const task = await apiClient.createTaskWithAgent( + seedData.workspaceId, + "SSH custom hooks", + seedData.agentProfileId, + { + description: 'e2e:delay(30000)\ne2e:message("still running")', + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + executor_profile_id: profile.id, + }, + ); + let workspace = ""; + await expect + .poll( + async () => { + const row = (await apiClient.listSSHSessions(seedData.sshExecutorId)).find( + (session) => session.task_id === task.id, + ); + workspace = row?.remote_task_dir ?? ""; + return ( + workspace !== "" && + (row?.remote_agentctl_port ?? 0) > 0 && + (await apiClient.getTask(task.id)).state === "IN_PROGRESS" && + readRemoteFile(seedData.sshTarget, `${workspace}/custom-prepare-marker`) === + "prepared\n" + ); + }, + { + timeout: 60_000, + message: "Wait for custom SSH prepare hook", + }, + ) + .toBe(true); + expect(remotePathExists(seedData.sshTarget, `${workspace}/custom-cleanup-marker`)).toBe( + false, + ); + + await apiClient.archiveTask(task.id); + await expect + .poll(() => remotePathExists(seedData.sshTarget, `${workspace}/custom-cleanup-marker`), { + timeout: 30_000, + message: "Wait for terminal SSH cleanup hook", + }) + .toBe(true); + expect(remotePathExists(seedData.sshTarget, workspace)).toBe(true); + } finally { + await apiClient.deleteExecutorProfile(profile.id).catch(() => undefined); + } }); test("agentctl is uploaded on first launch and sha256 sidecar lands", async ({ diff --git a/docs/plans/ssh-workspace-preparation/plan.md b/docs/plans/ssh-workspace-preparation/plan.md new file mode 100644 index 0000000000..6ac92e692d --- /dev/null +++ b/docs/plans/ssh-workspace-preparation/plan.md @@ -0,0 +1,144 @@ +--- +spec: docs/specs/ssh-executor/spec.md +issue: https://github.com/kdlbs/kandev/issues/2413 +created: 2026-08-08 +status: implemented +--- + +# Implementation Plan: SSH Workspace Preparation + +## Overview + +Issue #2413 is caused by a split launch pipeline: `SSHPreparer.Prepare` reports a successful +validation-only step, while `SSHExecutor.CreateInstance` creates the remote task directory and starts +`agentctl` without resolving or executing the profile prepare script. The only current root checkout +is the special `RemoteContributions[""]` path, so an ordinary provider-backed repository reaches the +agent as an empty directory. The repair gives SSH the same prepare/cleanup contract as Sprites while +preserving SSH host-key, credential-forwarding, and resumable-workspace boundaries. + +--- + +## Backend + +### SSH default and script resolution + +- Add an idempotent `ssh` case to + `apps/backend/internal/agent/runtime/lifecycle/default_scripts.go`. It must initialize the primary + checkout at the remote task workspace root, verify a reused checkout's credential-free `origin`, + run `{{repository.setup_script}}`, and leave feature-branch selection to the unconditional + `KandevBranchCheckoutPostlude`. +- Add SSH prepare/cleanup resolution beside the executor (prefer a focused + `executor_ssh_scripts.go` rather than growing `executor_ssh.go`). Reuse `scriptengine` providers for + workspace, repository, worktree, Git identity, GitHub auth, and remote contribution bindings. Map + agentctl install/start placeholders to the explicit SSH lifecycle so a profile script cannot start + an unauthenticated duplicate controller. +- Execute the resolved prepare script through the selected remote login shell with the effective + profile/repository environment explicitly configured for that executor. Keep secret values on + stdin, bound execution with a timeout, bound retained output, and return a generic checkout error + where Git stderr could contain credentials. Preserve the narrower existing allowlist for the agent + subprocess; prepare-script access must not accidentally broaden long-lived agent environment. + +### Launch ordering and postcondition + +- Refactor `SSHExecutor.CreateInstance` in + `apps/backend/internal/agent/runtime/lifecycle/executor_ssh.go` into the observable order: connect + and verify host; upload/resolve `agentctl`; create the task workspace; upload credentials; execute + preparation; verify that an attached primary repository is a matching Git checkout; create the + session runtime directory; preflight the selected agent binary; then launch/forward `agentctl` from + the remote workspace. +- Remove the post-controller ordinary/special-case gap by routing contribution setup through the same + resolved prepare pipeline. Preserve exact-head validation and source-upstream behavior from + `executor_ssh_remote_contribution.go`; consolidate or delete that SSH-only script only after its + existing security and resume tests are represented in the shared path. +- A failed or cancelled preparation must close the SSH client and leave no remote controller, port + forward, or tracked session. A reused checkout must preserve local descendants and untracked files. + +### Terminal cleanup + +- Resolve and run `MetadataKeyCleanupScript` over the tracked SSH connection in `StopInstance` only + when `shouldRunExecutorCleanup(instance.StopReason)` is true. Run it against + `MetadataKeySSHRemoteTaskDir` with the same non-secret providers as preparation, then stop the remote + controller and close forwarding/SSH even if cleanup fails. +- Keep any cleanup environment needed after launch only in the in-memory `sshSessionState`; rebuild it + from the resume request after backend recovery and never persist resolved secret values. +- Ordinary user Stop and `StopReasonBackendShutdown` must not run cleanup. Preserve the remote task + directory under all current SSH stop paths; this repair does not add automatic remote deletion. + +--- + +## Tests + +- **What:** `DefaultPrepareScript("ssh")` is populated, contains the repository setup placeholder, + has no inline task-branch checkout, and safely reuses a matching workspace. + **File:** `apps/backend/internal/agent/runtime/lifecycle/default_scripts_test.go` and focused SSH + script tests. + **How:** execute the resolved script against temporary bare repositories; assert root checkout, + credential-free `origin`, task branch, setup marker, preserved local descendant, and conflicting + origin rejection. +- **What:** SSH preparation resolves supported placeholders, receives only the approved environment, + runs before controller launch, and fails closed on non-zero exit, timeout, absent checkout, or + cancellation. + **File:** focused tests beside `executor_ssh_scripts.go` plus narrow executor orchestration tests. + **How:** fake command/session seams record ordering and output without dialing a user host. +- **What:** terminal cleanup runs for archive/delete reasons, skips Stop/backend shutdown, resolves + the remote workspace, and never prevents controller teardown after failure. + **File:** focused SSH lifecycle tests beside `executor_ssh.go`. + **How:** fake SSH command and controller-stop seams assert invocation order and error handling. +- **What:** contribution checkouts keep target `origin`, exact source SHA, source upstream, and local + descendant reuse after consolidation. + **File:** existing/focused tests in + `apps/backend/internal/agent/runtime/lifecycle/executor_ssh_connection_test.go` or the new script + test file. + **How:** temporary target/source bare repositories with URL rewrites. + +## E2E Tests + +- **Scenario:** GIVEN a provider-backed repository and the default SSH profile, WHEN a task starts, + THEN the remote task directory is a Git checkout on the task branch, contains the repository file, + and is the workspace used by `agentctl` and the agent. + **File:** `apps/web/e2e/tests/ssh/launch-task.spec.ts` or a focused sibling SSH workspace spec. + **What to verify:** real SSH transport, remote `git rev-parse --show-toplevel`, repository content, + session metadata, and controller log location. +- **Scenario:** GIVEN custom prepare and cleanup scripts, WHEN the task starts and is later archived + or deleted, THEN the prepare marker exists before the agent completes and the cleanup marker appears + only during terminal teardown. + **File:** the same focused SSH workspace spec. + **What to verify:** profile persistence, placeholder resolution, preparation failure gating, cleanup + reason gating, and retained remote workspace evidence. +- Extend `apps/web/e2e/helpers/http-git-server.ts` only as needed to expose a disposable provider URL + reachable from the SSH container. Keep the target isolated, credential-free, and torn down by the + owning fixture. + +## Public Documentation + +- Update `docs/public/executors.md` to describe automatic primary checkout, prepare/cleanup script + execution, failure behavior, and retained workspace cleanup semantics. +- Update the SSH row in `docs/public/feature-status.md` so it no longer claims repository and script + preparation are unsupported. +- Treat `docs/public/executors.md` as a how-to/reference hybrid whose dominant type is how-to; + `docs/public/feature-status.md` remains reference. + +## Verification Results + +- Task 01 targeted SSH lifecycle tests: `rtk go test ./internal/agent/runtime/lifecycle -run 'Test.*SSH.*(Prepare|Workspace|Cleanup|Contribution|Origin|Checkout)' -count=1` — 23 passed. +- Task 01 full lifecycle package: `rtk go test ./internal/agent/runtime/lifecycle -count=1` — 1,141 passed. +- Task 01 race-focused run: `rtk go test -race ./internal/agent/runtime/lifecycle -run 'TestSSH|TestShouldRunExecutorCleanupIncludesCascadeTerminalReasons' -count=1` — 47 passed. +- Task 02 container-backed SSH regression: `rtk env KANDEV_E2E_CONTAINERS=1 rtk pnpm e2e:run --no-build --project containers tests/ssh/launch-task.spec.ts` — 9 passed. +- Task 03 public docs tests: `rtk node --test scripts/validate-public-docs.test.mjs` — 58 passed; `rtk node scripts/validate-public-docs.mjs` — 41 pages validated. + +## Implementation Waves And Parallel Candidates + +Wave 1: + +- [x] [Task 01: SSH workspace lifecycle](task-01-ssh-workspace-lifecycle.md) + +Wave 2: + +- [x] [Task 02: SSH container regression](task-02-ssh-container-regression.md) + +Wave 3: + +- [x] [Task 03: Public documentation alignment](task-03-public-documentation-alignment.md) + +Execution is sequential in the primary conversation. The tasks share behavior contracts and are not +marked parallel-safe. diff --git a/docs/plans/ssh-workspace-preparation/task-01-ssh-workspace-lifecycle.md b/docs/plans/ssh-workspace-preparation/task-01-ssh-workspace-lifecycle.md new file mode 100644 index 0000000000..5234652654 --- /dev/null +++ b/docs/plans/ssh-workspace-preparation/task-01-ssh-workspace-lifecycle.md @@ -0,0 +1,80 @@ +--- +id: "01-ssh-workspace-lifecycle" +title: "SSH workspace lifecycle" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +spec: "../../specs/ssh-executor/spec.md" +--- + +# Task 01: SSH Workspace Lifecycle + +## Acceptance + +- An ordinary provider-backed SSH task resolves and runs the default or stored prepare script on the + target before `agentctl`, then starts the controller with the verified primary checkout as its + workspace. +- Preparation failure, timeout, cancellation, missing checkout, or conflicting `origin` fails closed + without a controller/forwarder leak; a matching existing checkout preserves local work. +- Terminal archive/delete runs the stored cleanup script before controller teardown; ordinary Stop + and backend restart skip cleanup, and cleanup failure remains best-effort. +- Existing remote-contribution identity, exact-SHA, upstream, credential-redaction, and resume + guarantees remain intact. + +## Verification + +```bash +cd apps/backend +rtk go test ./internal/agent/runtime/lifecycle -run 'Test.*SSH.*(Prepare|Workspace|Cleanup|Contribution|Origin|Checkout)' +``` + +## Files likely touched + +- `apps/backend/internal/agent/runtime/lifecycle/default_scripts.go` +- `apps/backend/internal/agent/runtime/lifecycle/default_scripts_test.go` +- `apps/backend/internal/agent/runtime/lifecycle/executor_ssh.go` +- `apps/backend/internal/agent/runtime/lifecycle/executor_ssh_operations.go` +- `apps/backend/internal/agent/runtime/lifecycle/executor_ssh_remote_contribution.go` +- `apps/backend/internal/agent/runtime/lifecycle/executor_ssh_connection_test.go` +- `apps/backend/internal/agent/runtime/lifecycle/executor_ssh_scripts.go` (new) +- `apps/backend/internal/agent/runtime/lifecycle/executor_ssh_scripts_test.go` (new) + +## Dependencies + +None. + +## Parallelism + +Sequential. Preparation and cleanup share SSH lifecycle ordering, metadata, command execution, and +security tests. + +## Inputs + +- SSH spec sections **Workspace lifecycle on the remote**, **Telemetry & errors**, and the new + preparation/cleanup scenarios. +- Sprites patterns in `executor_sprites_operations.go`, `executor_sprites.go`, and + `executor_sprites_lifecycle.go`. +- Existing SSH contribution materialization and credential-stdin handling. + +## Risks + +- Never put environment secrets into remote argv, persisted metadata, progress output, or returned + errors. +- Keep prepare/cleanup environment access distinct from the existing narrow agent-subprocess + allowlist; cache cleanup values only in live SSH session state and reconstruct them on resume. +- Do not reset a reused task branch or delete local/untracked work. +- Do not allow a custom prepare script to start a duplicate controller through agentctl placeholders. +- Preserve SSH client/forwarder cleanup on every failure edge. + +## Output contract + +Report root-cause path replaced, files changed, RED/GREEN evidence, exact targeted test result, +credential and cleanup boundaries, blockers/risks, and synchronized task/plan status. + +## Results + +- Implemented the SSH default prepare script and pre-agentctl lifecycle ordering. +- Added workspace/repository/script-engine resolution, fail-closed checkout verification, bounded/redacted prepare output, and terminal cleanup execution. +- Added explicit archive/delete tree stop reasons so handoff teardown invokes SSH cleanup before the later durable cascade stop. +- Targeted SSH lifecycle tests: 23 passed; full lifecycle package: 1,141 passed; race-focused SSH run: 47 passed. diff --git a/docs/plans/ssh-workspace-preparation/task-02-ssh-container-regression.md b/docs/plans/ssh-workspace-preparation/task-02-ssh-container-regression.md new file mode 100644 index 0000000000..53f9a34bde --- /dev/null +++ b/docs/plans/ssh-workspace-preparation/task-02-ssh-container-regression.md @@ -0,0 +1,71 @@ +--- +id: "02-ssh-container-regression" +title: "SSH container regression" +status: done +wave: 2 +depends_on: ["01-ssh-workspace-lifecycle"] +plan: "plan.md" +spec: "../../specs/ssh-executor/spec.md" +--- + +# Task 02: SSH Container Regression + +## Acceptance + +- A real container-backed SSH task with a disposable provider repository proves that the remote task + directory is the primary Git checkout and contains the expected repository content. +- A custom profile prepare script is observable before the agent turn completes; a terminal cleanup + script is observable after archive/delete and is absent after a non-terminal stop. +- The test owns and tears down every repository server, Git rewrite, profile, and SSH container state + it creates, including failure paths. + +## Verification + +```bash +cd apps +rtk pnpm install --frozen-lockfile +cd web +rtk env KANDEV_E2E_CONTAINERS=1 pnpm e2e:run --project containers tests/ssh/launch-task.spec.ts +``` + +If the scenario lives in a focused sibling spec, substitute that exact file in the final command and +record Playwright's discovered/passed test count. + +## Files likely touched + +- `apps/web/e2e/tests/ssh/launch-task.spec.ts` or + `apps/web/e2e/tests/ssh/workspace-preparation.spec.ts` (new) +- `apps/web/e2e/helpers/http-git-server.ts` only if a provider-URL option is required +- `apps/web/e2e/helpers/api-client.ts` only for a reusable default-script helper + +## Dependencies + +Task 01. + +## Parallelism + +Sequential. It validates Task 01's runtime behavior against the real SSH transport. + +## Inputs + +- The SSH spec's default preparation, custom hook, failure, reuse, and cleanup scenarios. +- `apps/web/e2e/fixtures/ssh-test-base.ts`, `apps/web/e2e/helpers/ssh.ts`, and the existing SSH + workspace-source test's disposable HTTP Git pattern. + +## Risks + +- Do not expose host credentials or a developer checkout to the SSH container. +- Restore any system Git URL rewrite and shared profile/repository state in `finally`/fixture cleanup. +- Confirm the `containers` project discovers the intended test count; a skipped Docker gate is not + passing evidence. + +## Output contract + +Report files changed, RED/GREEN run commands and counts, artifact paths on failure, owned-resource +teardown evidence, blockers/risks, and synchronized task/plan status. + +## Results + +- Updated the SSH fixture to serve a disposable provider-backed HTTP Git repository reachable from the SSH container via a scoped Git URL rewrite. +- Added assertions for the remote primary checkout, task branch, repository content, custom prepare marker, archive cleanup marker, preserved task directory, and session-directory behavior. +- Rebuilt the Linux mock-agent fixture and ran the exact container project command: 9 passed. diff --git a/docs/plans/ssh-workspace-preparation/task-03-public-documentation-alignment.md b/docs/plans/ssh-workspace-preparation/task-03-public-documentation-alignment.md new file mode 100644 index 0000000000..51c0ae94ae --- /dev/null +++ b/docs/plans/ssh-workspace-preparation/task-03-public-documentation-alignment.md @@ -0,0 +1,64 @@ +--- +id: "03-public-documentation-alignment" +title: "Public documentation alignment" +status: done +wave: 3 +depends_on: ["01-ssh-workspace-lifecycle", "02-ssh-container-regression"] +plan: "plan.md" +spec: "../../specs/ssh-executor/spec.md" +--- + +# Task 03: Public Documentation Alignment + +## Acceptance + +- The executor guide describes SSH repository preparation, profile prepare/cleanup hooks, failure + behavior, resume reuse, and retained task directories without overstating unsupported isolation or + cleanup guarantees. +- The feature-status SSH row matches the shipped behavior and no longer contains the contradictory + “does not materialize / ignores scripts” boundary. +- Public-doc validation passes. + +## Verification + +```bash +rtk node --test scripts/validate-public-docs.test.mjs +rtk node scripts/validate-public-docs.mjs +``` + +## Files likely touched + +- `docs/public/executors.md` +- `docs/public/feature-status.md` + +## Dependencies + +Tasks 01 and 02. + +## Parallelism + +Sequential. Documentation records the behavior proven by the runtime and E2E tasks. + +## Inputs + +- Updated SSH executor spec. +- Exact lifecycle behavior and limitations from Tasks 01 and 02. +- `/docs-maintainer` guidance; `executors.md` is primarily a how-to page and + `feature-status.md` is reference. + +## Risks + +- Do not claim automatic remote directory deletion, remote Windows support, password auth, or general + host isolation. +- Keep prepare failure and cleanup best-effort semantics explicit. + +## Output contract + +Report changed public pages, their primary Diátaxis types, exact validation results, blockers/risks, +and synchronized task/plan status. + +## Results + +- Updated `docs/public/executors.md` and `docs/public/feature-status.md` for SSH checkout preparation, profile hooks, failure gating, terminal cleanup, and retained task directories. +- `rtk node --test scripts/validate-public-docs.test.mjs`: 58 passed. +- `rtk node scripts/validate-public-docs.mjs`: 41 published docs pages validated. diff --git a/docs/public/executors.md b/docs/public/executors.md index a3acbb24a2..d98d4dab5c 100644 --- a/docs/public/executors.md +++ b/docs/public/executors.md @@ -81,9 +81,9 @@ Do not treat the two script fields as universal hooks: | Local / Worktree | Runs on the host during preparation. A failure is shown but is non-fatal, so the agent can still start for diagnosis. | Not executed by the executor runtime. Repository-level worktree cleanup is a separate repository setting. | | Local Docker | Runs inside the container before `agentctl`. Failure is logged but `agentctl` still starts. | Not executed. | | Sprites | Runs inside a newly created sandbox. Failure aborts the launch and destroys that new sandbox. | Runs, with a 60-second limit, only when a live execution is stopped with a task/session archived or deleted reason; failure does not prevent the subsequent destroy attempt. Plain Stop, **Reset Environment**, and profile-page direct destroy do not run this script. | -| SSH | **Currently not executed.** | **Currently not executed.** | +| SSH | Runs on the target before `agentctl`. An empty profile script uses the SSH default, which materializes the primary repository at the task-workspace root, reuses a matching checkout, runs repository setup, and selects the Kandev branch. A non-zero exit, timeout, missing checkout, or conflicting origin aborts the launch. | Runs on the target, with a 60-second limit, only for task/session archive or delete stops. Failure is logged but does not prevent controller teardown. Plain Stop and backend restart preserve the task workspace and skip cleanup. | -Keep working prepare scripts noninteractive and idempotent. Kandev resolves supported placeholders and appends its managed branch checkout for Docker and Sprites after the user script. A profile cleanup script must never remove paths outside the environment it owns. +Keep working prepare scripts noninteractive and idempotent. Kandev resolves supported placeholders and appends its managed branch checkout for Docker, Sprites, and SSH after the user script. A profile cleanup script must never remove paths outside the environment it owns. Two current preparation exceptions are easy to miss: @@ -291,11 +291,11 @@ The remote-auth card is built from the currently enabled agents. Depending on an
SSH repository source details -SSH materializes attached repository sources in the remote task directory. It still ignores profile prepare and cleanup scripts. Ensure the remote host has the required Git credentials and can reach every selected remote; folders cannot be attached to SSH tasks. +SSH materializes the primary attached repository at the remote task-workspace root and additional repository sources in direct child directories. The default prepare script initializes or reuses the root checkout, verifies the configured origin, fetches the base branch, runs repository setup, and lets Kandev's managed postlude select the task branch. A stored profile script replaces that default and must leave the same verified primary checkout before `agentctl` starts. Existing matching checkouts retain local commits and untracked files; a different origin, missing checkout, failed script, timeout, or cancellation fails the launch before a controller is started. Ensure the remote host has the required Git credentials and can reach every selected remote; folders cannot be attached to SSH tasks. The runtime preflights the selected agent command and reports an installation hint when missing; it does not install the agent or its toolchain. Only these resolved credential environment names are forwarded to the remote agent: `CLAUDE_CODE_OAUTH_TOKEN`, `ANTHROPIC_API_KEY`, `OPENAI_API_KEY`, `GEMINI_API_KEY`, `GOOGLE_API_KEY`, `GITHUB_TOKEN`, and `GH_TOKEN`. Arbitrary profile variables and control-plane process variables are not forwarded to that agent process. Agent-specific auth setup scripts can still consume a selected stored secret and materialize their own remote login state. -Stop attempts to kill the session's remote `agentctl` and remove only the remote session-runtime directory, then closes forwarding and SSH. Remote cleanup is best-effort: when the connection has already failed, the process or session directory can remain. The task directory always remains and no background sweeper currently removes it. The cached helper and checksum at `~/.kandev/bin/agentctl` and `agentctl.sha256` also remain for later sessions. Periodically audit the remote process list, session directories, and `/tasks/` after confirming no session needs the data. Resume re-dials SSH and reuses a live recorded PID when possible; otherwise Kandev starts a fresh remote controller. +Stop attempts to kill the session's remote `agentctl` and remove only the remote session-runtime directory, then closes forwarding and SSH. Terminal archive/delete stops run the profile cleanup script first; cleanup is best-effort, so a failure does not block controller teardown. Plain Stop and backend restart skip cleanup and preserve the task workspace for resume. The task directory always remains and no background sweeper currently removes it. The cached helper and checksum at `~/.kandev/bin/agentctl` and `agentctl.sha256` also remain for later sessions. Periodically audit the remote process list, session directories, and `/tasks/` after confirming no session needs the data. Resume re-dials SSH and reuses a live recorded PID when possible; otherwise Kandev starts a fresh remote controller and re-runs preparation.
@@ -303,7 +303,7 @@ Stop attempts to kill the session's remote `agentctl` and remove only the remote The task environment reports `creating`, `ready`, `stopped`, or `failed`; individual execution records have finer states. Stop is deliberately not synonymous with destroy for resumable Docker and Sprites environments. -Use a task's **Reset Environment** action when you need a clean materialization. Kandev blocks reset while a task session is starting or running, can optionally push the current branch, and requests teardown of the recorded worktree/container/sandbox. It normally keeps the environment record when teardown returns an error. The current Sprites credential-context limitation described above can instead report success while leaving the provider sandbox, so verify it separately. SSH task-directory cleanup remains manual as described above. +Use a task's **Reset Environment** action when you need a clean materialization. Kandev blocks reset while a task session is starting or running, can optionally push the current branch, and requests teardown of the recorded worktree/container/sandbox. It normally keeps the environment record when teardown returns an error. The current Sprites credential-context limitation described above can instead report success while leaving the provider sandbox, so verify it separately. SSH task directories are intentionally retained; use the profile cleanup script for terminal hook work and remove the directory manually only after confirming no session needs it. Before deleting any environment, push or otherwise preserve uncommitted work. Profile deletion and provider/daemon-side deletion can bypass normal lifecycle safeguards. diff --git a/docs/public/feature-status.md b/docs/public/feature-status.md index 28b7121b60..49d276a9e8 100644 --- a/docs/public/feature-status.md +++ b/docs/public/feature-status.md @@ -91,7 +91,7 @@ Released builds can lag this page. Check **Settings > System > About** or `kande | Profile permissions, CLI flags, literal environment values, and encrypted secret references | Supported | Secret references resolve at launch; a missing/deleted reference omits that variable. Auto-approve and permission-skipping settings enlarge the process trust boundary. | | Local and Worktree executors | Supported | Both are seeded. They execute on the Kandev host; a worktree isolates Git checkouts, not processes, network, credentials, or the rest of the filesystem. | | Local Docker and Sprites executors | Dependency-bound | Require the relevant daemon or provider, compatible host/image, token, platform, and credential-copy configuration. | -| SSH executor | Limited | Remote sessions support trusted Linux or macOS hosts on amd64 or arm64, with public-key authentication, SFTP, and TCP forwarding. Kandev does not currently clone/materialize attached repositories there, ignores profile prepare/cleanup scripts, and retains the remote task directory for manual cleanup. | +| SSH executor | Limited | Remote sessions support trusted Linux or macOS hosts on amd64 or arm64, with public-key authentication, SFTP, and TCP forwarding. The runtime materializes the primary repository at the remote task-workspace root, reuses matching checkouts without resetting local work, runs the resolved prepare hook before `agentctl`, and runs cleanup hooks only for terminal archive/delete stops. Failed preparation blocks launch; task directories remain for resume/manual housekeeping, and password authentication, hostile-user isolation, and automatic directory deletion are not provided. | | Remote Docker executor | In progress | The type and legacy/direct settings fields are registered, but runtime create/stop return `not yet implemented`, and the current **Settings > Executors** hub does not offer it. | | GitHub integration | Dependency-bound | Workspace repository scope, issue/PR browsing, actions, watches, presets, reviews, checks, and Gists depend on GitHub auth, scopes, rate limits, and repository permissions. Supported Local Docker, Sprites, and SSH launch paths may receive the globally stored token as `GITHUB_TOKEN`/`GH_TOKEN`; workspace repository scope does not constrain its privileges. | | Azure DevOps integration | Limited | Azure DevOps Services supports workspace PAT configuration, boards, work-item/PR browsing, constrained assignment and board moves, feedback, task launch and associations, configurable actions/queries, and polling watches. Azure DevOps Server/TFS, Entra OAuth, arbitrary work-item editing, and webhooks are not supported; neither `gh` nor `az` is required. | diff --git a/docs/specs/ssh-executor/spec.md b/docs/specs/ssh-executor/spec.md index 862c3b19d9..f24243c883 100644 --- a/docs/specs/ssh-executor/spec.md +++ b/docs/specs/ssh-executor/spec.md @@ -1,5 +1,5 @@ --- -status: draft +status: implemented created: 2026-05-16 owner: tbd --- @@ -23,20 +23,25 @@ SSH is the lowest-common-denominator transport for reaching a remote Linux box. A new executor type `ssh` joins `local_pc` / `local_docker` / `sprites` in the existing executor framework. From the user's perspective: configure an SSH target once in settings, then pick it as the executor for any task; tasks run on the remote box exactly as they would on a Sprite. -### Remote layout: mirrors local `~/.kandev/tasks/` +### Remote layout: one task workspace per task -Kandev's local convention is one directory per task containing one or more repo worktrees: +Kandev creates one remote workspace per task. The primary repository owns the workspace root so a +single-repository agent starts inside a Git checkout; additional repository branches use direct +child directories: ``` ~/.kandev/tasks/ ├── / -│ ├── / # git worktree on the task's feature branch -│ └── / # second repo worktree, same task +│ ├── .git/ # primary repository on the task's feature branch +│ └── -/ # additional repository or branch └── / - └── / + └── .git/ # another task's primary repository ``` -The SSH executor mirrors this exact layout on the remote host. Workspace root defaults to `~/.kandev/` on the remote and is **configurable per profile** (so one host can serve `~/.kandev-team-a/tasks/` and `~/.kandev-team-b/tasks/` for different profiles on the same SSH user). Each task gets `/tasks///` cloned + worktree'd just like the local case. Concurrent sessions on the same task share this directory; concurrent tasks on the same host get different `` siblings. +The workspace root defaults to `~/.kandev/` on the remote and is **configurable per profile** (so +one host can serve `~/.kandev-team-a/tasks/` and `~/.kandev-team-b/tasks/` for different profiles on +the same SSH user). Concurrent sessions on the same task share the task workspace; concurrent tasks +on the same host get different `` siblings. ### Per-session, not per-task: agentctl process, remote port, local forward @@ -74,9 +79,28 @@ Multiple sessions in the *same* task share the same worktree on disk (same files ### Workspace lifecycle on the remote -- On `CreateInstance` for a task that has no existing remote task dir: SSH `mkdir -p /tasks/`, then the prepare-script + scriptengine pipeline (`git clone`, worktree, checkout) runs the same way it does for Sprites. -- On subsequent sessions for the same task on the same host: detect the existing task dir, skip clone, attach a new agentctl to it. -- On `StopInstance` of the *last* session in a task: leave the task dir intact (so a later resume keeps history); only remove it on explicit task-level cleanup (out of scope for v1 — see below). +- On `CreateInstance`, SSH creates `/tasks/` and resolves the stored + profile prepare script through the same scriptengine providers used by Sprites. An empty stored + script falls back to the SSH default, which idempotently materializes the primary repository at + the task workspace root, runs the repository setup script, and selects the Kandev task branch. +- The resolved prepare script runs on the SSH target with the selected profile shell and the + explicitly configured profile/repository environment. Those values are delivered without placing + secrets in remote process arguments or persisted metadata. It completes before the per-session + `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. +- 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 + workspace is shared. +- `agentctl` starts with the task workspace as its working directory only after preparation succeeds. + Additional repository sources are materialized before the agent process starts. +- Profile cleanup scripts run on the SSH target for terminal task/session archive or deletion, with + the same workspace placeholders as preparation. Ordinary Stop and backend restart preserve both + the workspace and cleanup hook for resume. Cleanup failure is reported in logs but does not prevent + controller teardown. +- The task directory remains after controller teardown so a later resume keeps history; automatic + task-directory deletion remains out of scope. ### agentctl binary upload with content-hash cache @@ -121,7 +145,26 @@ Multiple sessions in the *same* task share the same worktree on disk (same files - **GIVEN** a user has filled the SSH executor form and clicked Test Connection, **WHEN** the test succeeds, **THEN** the host fingerprint is shown in the UI, the user must tick "Trust this host" before Save is enabled, and on Save the fingerprint is pinned to the executor — silent first-connect TOFU never happens. -- **GIVEN** a user has a configured + trusted SSH executor pointed at `dev.example.com`, **WHEN** they start a task and select that executor, **THEN** kandev SSHes in (reusing any open connection for this host), uploads agentctl to `~/.kandev/bin/agentctl` if the sha256 differs, clones the repo into `/tasks///`, starts a per-session agentctl on a fresh remote port with a fresh local SSH forward, and the user sees the same chat/shell/git UI as for any other executor. +- **GIVEN** a user has a configured + trusted SSH executor and a provider-backed repository, **WHEN** + they start a task with the default SSH prepare script, **THEN** Kandev materializes the repository + at `/tasks/`, checks out the task branch, runs repository setup, and + starts the agent there so `git rev-parse --show-toplevel` returns that remote task directory. + +- **GIVEN** an SSH profile has a custom prepare script, **WHEN** a task starts, **THEN** the script + resolves SSH-supported scriptengine placeholders and completes on the target before the + per-session `agentctl` starts in the prepared workspace. + +- **GIVEN** SSH preparation exits non-zero, times out, leaves an attached primary repository absent, + or finds a different `origin`, **WHEN** a task starts, **THEN** launch fails with a preparation step + error and neither `agentctl` nor the agent is started for that session. + +- **GIVEN** an SSH task already has local commits or untracked work in its remote checkout, **WHEN** a + later session starts for the same task, **THEN** default preparation reuses the checkout without + resetting or deleting that work before starting the new session controller. + +- **GIVEN** an SSH profile has a cleanup script and its task or session is archived or deleted, + **WHEN** Kandev tears down the terminal execution, **THEN** the cleanup script runs on the SSH target + with the task workspace as `{{workspace.path}}`; an ordinary Stop or backend restart does not run it. - **GIVEN** a user keeps their SSH key in 1Password (no key file on disk) with their agent running, **WHEN** they select "ssh-agent" as identity source, **THEN** the connection succeeds without kandev ever touching key material; passphrase-protected keys without an agent are explicitly rejected with a "load this key into ssh-agent first" message. @@ -163,7 +206,8 @@ Multiple sessions in the *same* task share the same worktree on disk (same files - **Pooled host capacity**: configure 3 SSH hosts as a pool, kandev round-robins / load-balances tasks across them. - **SSH-tunneled MCP servers**: expose a user's local MCP servers to a remote-running agent via reverse forward. - **Orphan task-dir housekeeper** as a backend background job. -- **Stream-richer prepare script execution** in SSHExecutor (currently delegated to the standard prepare-script pipeline; matching Sprites' inline progress output would improve the launch UX). +- **Byte-for-byte live prepare output streaming.** SSH reports bounded preparation progress and the + final diagnostic tail, but matching Sprites' continuous stdout/stderr streaming can be added later. ## Open questions