Files
kandev/apps/backend/internal/agentctl/server/process/runner_shells_test.go
T
João Salavisa 16495a6dcc fix(terminal): expose executor-profile env vars to terminal shells (#1992)
* fix(terminal): expose executor-profile env vars to terminal shells

PR #1971 got the profile env into the agent subprocess and the repository
setup script, but the terminal panel was still a gap: the local user shell
was started with `InteractiveStartRequest.Env` unset, so it inherited only
the backend process environment, and agentctl's per-terminal shells were
started from `shell.DefaultConfig` with no env overrides at all.

Local/worktree path: resolve the task environment's executor profile at
terminal start (revealing secret-backed entries) and pass it through
UserShellOptions.Env into the deferred PTY start. Resolution happens per
terminal rather than being cached at launch, so a terminal opened before
the agent ever runs still gets the vars.

Remote path (Docker/Sprites/SSH): StartTerminalShell now folds the agent
instance's env into the shell config, so a terminal inside the workspace
sees what the agent sees. SSH is unchanged in scope — its instance env is
still only the credential allowlist, not arbitrary profile variables.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(terminal): resolve terminal env from the session's executor profile

Address PR review. The resolver read executor_profile_id off the
task_environments row, but persistTaskEnvironment's reuse branch never
refreshes that column — it keeps whatever the first session stamped on it.
buildLaunchAgentRequest resolves the agent's env from
session.ExecutorProfileID instead, so a task environment reused by a later
session that picked a different profile of the same executor type would give
the terminal stale secrets and omit the ones the agent just received.

Prefer the session's profile and fall back to the environment row only when
the session recorded none. Also document why mergeAgentEnvIntoShellConfig
needs no defensive copy and why StartEnvForTesting is exported.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test(terminal): give env-row fallback subtests distinct session IDs

Address PR review nit: both subtests shared the hardcoded "session-3" at
the call site, so the error-path case read as a repeat of the first.
Move the session ID into the table as session-no-profile / session-error.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Kandev Agent <agent@kandev.dev>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-28 00:08:57 +01:00

516 lines
17 KiB
Go

package process
import (
"context"
"strings"
"testing"
"time"
)
func TestInteractiveRunner_IsUserShellIsolation(t *testing.T) {
log := newTestLogger(t)
runner := NewInteractiveRunner(nil, log, 2*1024*1024)
// Start a regular (non-user-shell) process
cmd, env := fixtureExec("cat")
agentReq := InteractiveStartRequest{
SessionID: "isolation-test",
Command: cmd,
Env: env,
ImmediateStart: true,
DefaultCols: 80,
DefaultRows: 24,
}
agentInfo, err := runner.Start(context.Background(), agentReq)
if err != nil {
t.Fatalf("Start() agent error = %v", err)
}
// Start a user shell process for the same session
shellReq := InteractiveStartRequest{
SessionID: "isolation-test",
Command: cmd,
Env: env,
IsUserShell: true,
ImmediateStart: true,
DefaultCols: 80,
DefaultRows: 24,
}
shellInfo, err := runner.Start(context.Background(), shellReq)
if err != nil {
t.Fatalf("Start() shell error = %v", err)
}
time.Sleep(100 * time.Millisecond)
// ResizeBySession should target the agent process, not the user shell
if err := runner.ResizeBySession("isolation-test", 120, 40); err != nil {
t.Errorf("ResizeBySession() error = %v", err)
}
// GetPtyWriterBySession should return the agent process, not the user shell
_, procID, err := runner.GetPtyWriterBySession("isolation-test")
if err != nil {
t.Fatalf("GetPtyWriterBySession() error = %v", err)
}
if procID != agentInfo.ID {
t.Errorf("GetPtyWriterBySession() returned process %q, want agent process %q", procID, agentInfo.ID)
}
// SetDirectOutput on user shell should NOT track at session level
writer := &mockDirectWriter{}
if err := runner.SetDirectOutput(shellInfo.ID, writer); err != nil {
t.Fatalf("SetDirectOutput() shell error = %v", err)
}
// Session-level WebSocket should NOT be set for user shells
if runner.HasActiveWebSocketBySession("isolation-test") {
t.Error("user shell should not set session-level WebSocket tracking")
}
// SetDirectOutput on agent process SHOULD track at session level
agentWriter := &mockDirectWriter{}
if err := runner.SetDirectOutput(agentInfo.ID, agentWriter); err != nil {
t.Fatalf("SetDirectOutput() agent error = %v", err)
}
if !runner.HasActiveWebSocketBySession("isolation-test") {
t.Error("agent process should set session-level WebSocket tracking")
}
// ClearDirectOutputBySession should clear agent but not user shell
runner.ClearDirectOutputBySession("isolation-test")
// Agent should have its direct output cleared
if runner.HasActiveWebSocket(agentInfo.ID) {
t.Error("agent WebSocket should be cleared after ClearDirectOutputBySession")
}
// User shell should still have its direct output
if !runner.HasActiveWebSocket(shellInfo.ID) {
t.Error("user shell WebSocket should NOT be cleared by ClearDirectOutputBySession")
}
ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
defer cancel()
_ = runner.Stop(ctx, agentInfo.ID)
_ = runner.Stop(ctx, shellInfo.ID)
}
func TestInteractiveRunner_TrackSessionWebSocketBeforePtyAccess(t *testing.T) {
log := newTestLogger(t)
runner := NewInteractiveRunner(nil, log, 2*1024*1024)
const sessionID = "pre-pty-session"
writer := &mockDirectWriter{}
runner.TrackSessionWebSocket(sessionID, writer)
if !runner.HasActiveWebSocketBySession(sessionID) {
t.Fatal("TrackSessionWebSocket should set session-level WebSocket tracking")
}
cmd, env := fixtureExec("cat")
req := InteractiveStartRequest{
SessionID: sessionID,
Command: cmd,
Env: env,
ImmediateStart: true,
DefaultCols: 80,
DefaultRows: 24,
}
info, err := runner.Start(context.Background(), req)
if err != nil {
t.Fatalf("Start() error = %v", err)
}
if !runner.ConnectSessionWebSocket(info.ID) {
t.Fatal("ConnectSessionWebSocket should attach the tracked writer to the process")
}
if !runner.HasActiveWebSocket(info.ID) {
t.Fatal("process should have an active WebSocket after session-level connect")
}
if err := runner.WriteToDirectOutputBySession(sessionID, []byte("fallback banner")); err != nil {
t.Fatalf("WriteToDirectOutputBySession() error = %v", err)
}
writer.mu.Lock()
got := string(writer.data)
writer.mu.Unlock()
if got != "fallback banner" {
t.Fatalf("writer data = %q, want fallback banner", got)
}
runner.ClearDirectOutputBySession(sessionID)
if runner.HasActiveWebSocketBySession(sessionID) {
t.Fatal("ClearDirectOutputBySession should clear session-level tracking")
}
if runner.HasActiveWebSocket(info.ID) {
t.Fatal("ClearDirectOutputBySession should clear process direct output")
}
ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
defer cancel()
_ = runner.Stop(ctx, info.ID)
}
func TestInteractiveRunner_CreateUserShell_First(t *testing.T) {
log := newTestLogger(t)
runner := NewInteractiveRunner(nil, log, 2*1024*1024)
result := runner.CreateUserShell("session-1")
if result.TerminalID == "" {
t.Error("CreateUserShell() returned empty TerminalID")
}
if !strings.HasPrefix(result.TerminalID, "shell-") {
t.Errorf("CreateUserShell() TerminalID = %q, want prefix 'shell-'", result.TerminalID)
}
if result.Label != "Terminal" {
t.Errorf("CreateUserShell() Label = %q, want 'Terminal'", result.Label)
}
if result.Closable {
t.Error("CreateUserShell() first terminal should not be closable")
}
}
func TestInteractiveRunner_CreateUserShell_Subsequent(t *testing.T) {
log := newTestLogger(t)
runner := NewInteractiveRunner(nil, log, 2*1024*1024)
first := runner.CreateUserShell("session-1")
second := runner.CreateUserShell("session-1")
if first.TerminalID == second.TerminalID {
t.Error("CreateUserShell() should return different terminal IDs")
}
if second.Label != "Terminal 2" {
t.Errorf("CreateUserShell() second Label = %q, want 'Terminal 2'", second.Label)
}
if !second.Closable {
t.Error("CreateUserShell() second terminal should be closable")
}
third := runner.CreateUserShell("session-1")
if third.Label != "Terminal 3" {
t.Errorf("CreateUserShell() third Label = %q, want 'Terminal 3'", third.Label)
}
if !third.Closable {
t.Error("CreateUserShell() third terminal should be closable")
}
}
func TestInteractiveRunner_CreateUserShell_DifferentSessions(t *testing.T) {
log := newTestLogger(t)
runner := NewInteractiveRunner(nil, log, 2*1024*1024)
r1 := runner.CreateUserShell("session-1")
r2 := runner.CreateUserShell("session-2")
// Both should be "Terminal" (first in each scope)
if r1.Label != "Terminal" {
t.Errorf("session-1 Label = %q, want 'Terminal'", r1.Label)
}
if r2.Label != "Terminal" {
t.Errorf("session-2 Label = %q, want 'Terminal'", r2.Label)
}
if r1.Closable || r2.Closable {
t.Error("first terminal in each scope should not be closable")
}
}
// Two sessions in the same task share a TaskEnvironmentID, and callers pass
// that env as the scopeID. The runner must return one shared shell list for
// them — that's the whole reason terminals are env-keyed.
func TestInteractiveRunner_SharedScope_AcrossSessions(t *testing.T) {
log := newTestLogger(t)
runner := NewInteractiveRunner(nil, log, 2*1024*1024)
envID := "env-shared"
first := runner.CreateUserShell(envID)
second := runner.CreateUserShell(envID)
// Subsequent shell in the same scope must increment.
if first.Label != "Terminal" {
t.Errorf("first Label = %q, want 'Terminal'", first.Label)
}
if second.Label != "Terminal 2" {
t.Errorf("second Label = %q, want 'Terminal 2' (same scope, incremented)", second.Label)
}
// Both terminals must show up when any session in that env lists shells.
shells := runner.ListUserShells(envID)
if len(shells) != 2 {
t.Fatalf("ListUserShells(envID) returned %d, want 2", len(shells))
}
ids := map[string]bool{}
for _, s := range shells {
ids[s.TerminalID] = true
}
if !ids[first.TerminalID] || !ids[second.TerminalID] {
t.Error("ListUserShells did not include both shells created under the shared scope")
}
// A different env must remain isolated.
otherShells := runner.ListUserShells("env-other")
for _, s := range otherShells {
if s.TerminalID == first.TerminalID || s.TerminalID == second.TerminalID {
t.Error("shells leaked across envs — scope isolation broken")
}
}
}
func TestInteractiveRunner_RegisterScriptShell(t *testing.T) {
log := newTestLogger(t)
runner := NewInteractiveRunner(nil, log, 2*1024*1024)
runner.RegisterScriptShell("session-1", "script-abc", "npm start", "npm run start")
// Auto-create-first-shell was removed; the script registration is the
// only entry the runner now knows about for this scope.
shells := runner.ListUserShells("session-1")
if len(shells) != 1 {
t.Fatalf("ListUserShells() returned %d shells, want 1", len(shells))
}
scriptShell := &shells[0]
if scriptShell.TerminalID != "script-abc" {
t.Fatalf("script shell terminalID = %q, want script-abc", scriptShell.TerminalID)
}
{
if scriptShell.Label != "npm start" {
t.Errorf("script shell Label = %q, want 'npm start'", scriptShell.Label)
}
if scriptShell.InitialCommand != "npm run start" {
t.Errorf("script shell InitialCommand = %q, want 'npm run start'", scriptShell.InitialCommand)
}
if !scriptShell.Closable {
t.Error("script shell should be closable")
}
}
if scriptShell.ProcessID != "" {
t.Errorf("script shell should have empty ProcessID before WebSocket connect, got %q", scriptShell.ProcessID)
}
}
func TestInteractiveRunner_RegisterScriptShell_DoesNotAffectShellCount(t *testing.T) {
log := newTestLogger(t)
runner := NewInteractiveRunner(nil, log, 2*1024*1024)
// Register a script terminal
runner.RegisterScriptShell("session-1", "script-abc", "Build", "npm run build")
// Create a plain shell - should be "Terminal" (first plain shell), not "Terminal 2"
result := runner.CreateUserShell("session-1")
if result.Label != "Terminal" {
t.Errorf("CreateUserShell() Label = %q, want 'Terminal' (scripts should not count)", result.Label)
}
if result.Closable {
t.Error("first plain shell should not be closable regardless of script terminals")
}
}
func TestInteractiveRunner_ListUserShells_EmptyByDefault(t *testing.T) {
log := newTestLogger(t)
runner := NewInteractiveRunner(nil, log, 2*1024*1024)
// Auto-create-first-shell was removed; an untouched scope returns nothing.
// Backend's terminal service owns the auto-create policy now.
shells := runner.ListUserShells("session-1")
if len(shells) != 0 {
t.Fatalf("ListUserShells() returned %d shells, want 0", len(shells))
}
}
func TestInteractiveRunner_ListUserShells_Sorted(t *testing.T) {
log := newTestLogger(t)
runner := NewInteractiveRunner(nil, log, 2*1024*1024)
// Create shells with a small time gap
runner.CreateUserShell("session-1")
time.Sleep(10 * time.Millisecond)
runner.CreateUserShell("session-1")
time.Sleep(10 * time.Millisecond)
runner.RegisterScriptShell("session-1", "script-1", "Build", "make build")
shells := runner.ListUserShells("session-1")
if len(shells) != 3 {
t.Fatalf("ListUserShells() returned %d shells, want 3", len(shells))
}
// Should be sorted by creation time
for i := 1; i < len(shells); i++ {
if shells[i].CreatedAt.Before(shells[i-1].CreatedAt) {
t.Errorf("shells not sorted by creation time: shell[%d] (%v) before shell[%d] (%v)",
i, shells[i].CreatedAt, i-1, shells[i-1].CreatedAt)
}
}
}
func TestInteractiveRunner_ListUserShells_IsolatedBySessions(t *testing.T) {
log := newTestLogger(t)
runner := NewInteractiveRunner(nil, log, 2*1024*1024)
runner.CreateUserShell("session-1")
runner.CreateUserShell("session-1")
runner.CreateUserShell("session-2")
shells1 := runner.ListUserShells("session-1")
shells2 := runner.ListUserShells("session-2")
if len(shells1) != 2 {
t.Errorf("session-1 should have 2 shells, got %d", len(shells1))
}
if len(shells2) != 1 {
t.Errorf("session-2 should have 1 shell, got %d", len(shells2))
}
}
func TestInteractiveRunner_StopUserShell(t *testing.T) {
log := newTestLogger(t)
runner := NewInteractiveRunner(nil, log, 2*1024*1024)
// Create a shell
result := runner.CreateUserShell("session-1")
// Verify it's in the list
shells := runner.ListUserShells("session-1")
found := false
for _, s := range shells {
if s.TerminalID == result.TerminalID {
found = true
break
}
}
if !found {
t.Fatal("created shell not found in list")
}
// Stop the shell (no process running, so this should remove the entry)
ctx := context.Background()
err := runner.StopUserShell(ctx, "session-1", result.TerminalID)
// Error is expected since there's no process to stop
_ = err
// Shell should be removed from tracking
shells = runner.ListUserShells("session-1")
for _, s := range shells {
if s.TerminalID == result.TerminalID {
t.Error("stopped shell should be removed from list")
}
}
}
func TestInteractiveRunner_StopUserShell_NonExistent(t *testing.T) {
log := newTestLogger(t)
runner := NewInteractiveRunner(nil, log, 2*1024*1024)
// Stopping a non-existent shell should not error
ctx := context.Background()
err := runner.StopUserShell(ctx, "session-1", "nonexistent")
if err != nil {
t.Errorf("StopUserShell() for non-existent shell should return nil, got %v", err)
}
}
func TestInteractiveRunner_StopUserShell_ScriptTerminal(t *testing.T) {
log := newTestLogger(t)
runner := NewInteractiveRunner(nil, log, 2*1024*1024)
// Register and then stop a script terminal
runner.RegisterScriptShell("session-1", "script-abc", "Build", "npm run build")
ctx := context.Background()
err := runner.StopUserShell(ctx, "session-1", "script-abc")
_ = err // Error expected since no process
// Script should be removed from list (auto-created "Terminal" may still appear)
shells := runner.ListUserShells("session-1")
for _, s := range shells {
if s.TerminalID == "script-abc" {
t.Error("stopped script terminal should be removed from list")
}
}
}
func TestInteractiveRunner_CreateUserShell_AtomicRegistration(t *testing.T) {
log := newTestLogger(t)
runner := NewInteractiveRunner(nil, log, 2*1024*1024)
// Create a shell - it should be immediately visible in ListUserShells
result := runner.CreateUserShell("session-1")
shells := runner.ListUserShells("session-1")
found := false
for _, s := range shells {
if s.TerminalID == result.TerminalID {
found = true
if s.Label != result.Label {
t.Errorf("shell Label mismatch: list=%q, create=%q", s.Label, result.Label)
}
if s.Closable != result.Closable {
t.Errorf("shell Closable mismatch: list=%v, create=%v", s.Closable, result.Closable)
}
break
}
}
if !found {
t.Error("CreateUserShell() result should be immediately visible in ListUserShells()")
}
}
// User shell terminals must export the executor-profile env vars the caller
// resolved, so the terminal sees the same variables as the agent subprocess and
// the repository setup script.
func TestInteractiveRunner_StartUserShellForwardsOptionEnv(t *testing.T) {
log := newTestLogger(t)
runner := NewInteractiveRunner(nil, log, 2*1024*1024)
info, err := runner.StartUserShell(
context.Background(),
"env-scope", "env-session", "term-1", t.TempDir(), "",
&UserShellOptions{Label: "Terminal", Env: map[string]string{"FONTAWESOME_NPM_AUTH_TOKEN": "fa-secret-value"}},
)
if err != nil {
t.Fatalf("StartUserShell() error = %v", err)
}
runner.mu.RLock()
proc, ok := runner.processes[info.ID]
runner.mu.RUnlock()
if !ok {
t.Fatalf("process %q not registered", info.ID)
}
if got := proc.startEnv["FONTAWESOME_NPM_AUTH_TOKEN"]; got != "fa-secret-value" {
t.Fatalf("startEnv[FONTAWESOME_NPM_AUTH_TOKEN] = %q, want profile secret; env=%#v", got, proc.startEnv)
}
}
func TestMergeAgentEnvIntoShellConfig(t *testing.T) {
if got := mergeAgentEnvIntoShellConfig(nil, nil); got != nil {
t.Fatalf("mergeAgentEnvIntoShellConfig(nil, nil) = %#v, want nil (inherit process env)", got)
}
// Instance agent env is inherited; explicit overrides still win, and
// malformed entries are dropped rather than producing empty keys.
merged := mergeAgentEnvIntoShellConfig(
[]string{"FONTAWESOME_NPM_AUTH_TOKEN=fa-secret-value", "TERM=dumb", "MALFORMED", "=novalue"},
map[string]string{"TERM": "xterm-256color"},
)
if merged["FONTAWESOME_NPM_AUTH_TOKEN"] != "fa-secret-value" {
t.Fatalf("merged token = %q, want inherited agent env value", merged["FONTAWESOME_NPM_AUTH_TOKEN"])
}
if merged["TERM"] != "xterm-256color" {
t.Fatalf("merged TERM = %q, want caller override to win", merged["TERM"])
}
if _, exists := merged[""]; exists {
t.Fatalf("merged contains empty key: %#v", merged)
}
if _, exists := merged["MALFORMED"]; exists {
t.Fatalf("merged contains valueless entry: %#v", merged)
}
// Overrides-only with no agent env passes the map through untouched.
overrides := map[string]string{"TOKEN": "t"}
if got := mergeAgentEnvIntoShellConfig(nil, overrides); got["TOKEN"] != "t" {
t.Fatalf("mergeAgentEnvIntoShellConfig(nil, overrides)[TOKEN] = %q, want t", got["TOKEN"])
}
}