diff --git a/apps/backend/internal/agent/agents/agent.go b/apps/backend/internal/agent/agents/agent.go index 2ff8a771ee..9aa5a4c37a 100644 --- a/apps/backend/internal/agent/agents/agent.go +++ b/apps/backend/internal/agent/agents/agent.go @@ -125,6 +125,10 @@ type PassthroughOptions struct { Resume bool // generic "continue last session" (e.g. -c, --resume latest) PermissionValues map[string]bool // e.g. {"auto_approve": true} WorkDir string + // CLIFlagTokens are user-configured CLI flag argv tokens derived from + // AgentProfile.CLIFlags (only Enabled entries, shell-tokenised). Appended + // verbatim to the built passthrough command, mirroring CommandOptions. + CLIFlagTokens []string } // RuntimeConfig holds Docker / standalone runtime settings. diff --git a/apps/backend/internal/agent/agents/passthrough.go b/apps/backend/internal/agent/agents/passthrough.go index ea51b25eb7..c478e4bbed 100644 --- a/apps/backend/internal/agent/agents/passthrough.go +++ b/apps/backend/internal/agent/agents/passthrough.go @@ -16,7 +16,8 @@ func (p *StandardPassthrough) PassthroughConfig() PassthroughConfig { func (p *StandardPassthrough) BuildPassthroughCommand(opts PassthroughOptions) Command { b := p.Cfg.PassthroughCmd.With(). Model(p.Cfg.ModelFlag, opts.Model). - Settings(p.PermSettings, opts.PermissionValues) + Settings(p.PermSettings, opts.PermissionValues). + Flag(opts.CLIFlagTokens...) switch { case opts.SessionID != "" && !p.Cfg.SessionResumeFlag.IsEmpty(): diff --git a/apps/backend/internal/agent/lifecycle/manager_passthrough.go b/apps/backend/internal/agent/lifecycle/manager_passthrough.go index 80dcbf733c..23cc9cab6d 100644 --- a/apps/backend/internal/agent/lifecycle/manager_passthrough.go +++ b/apps/backend/internal/agent/lifecycle/manager_passthrough.go @@ -10,6 +10,7 @@ import ( "github.com/kandev/kandev/internal/agent/agents" "github.com/kandev/kandev/internal/agent/executor" + "github.com/kandev/kandev/internal/agent/settings/cliflags" agentctl "github.com/kandev/kandev/internal/agentctl/client" "github.com/kandev/kandev/internal/agentctl/server/process" agentctltypes "github.com/kandev/kandev/internal/agentctl/types" @@ -221,6 +222,7 @@ func (m *Manager) passthroughAgentCommand(execution *AgentExecution, profileInfo SessionID: execution.ACPSessionID, Prompt: taskDescription, PermissionValues: profilePermissionValues(profileInfo), + CLIFlagTokens: m.profileCLIFlagTokens(profileInfo), }) if cmd.IsEmpty() { return nil, agents.PassthroughConfig{}, nil, agents.Command{}, fmt.Errorf("passthrough command is empty for agent %s", agentConfig.ID()) @@ -228,6 +230,24 @@ func (m *Manager) passthroughAgentCommand(execution *AgentExecution, profileInfo return ptAgent, pt, rt, cmd, nil } +// profileCLIFlagTokens resolves the user-configured CLI flag argv tokens from +// a profile, mirroring the ACP launch path (manager_launch.go). Returns nil +// on resolve error and logs a warning so a malformed flag does not block the +// session — matches the warn-and-continue behaviour of the ACP path. +func (m *Manager) profileCLIFlagTokens(p *AgentProfileInfo) []string { + if p == nil { + return nil + } + tokens, err := cliflags.Resolve(p.CLIFlags) + if err != nil { + m.logger.Warn("failed to resolve cli_flags for passthrough profile, launching without user-configured flags", + zap.String("profile_id", p.ProfileID), + zap.Error(err)) + return nil + } + return tokens +} + // buildInteractiveStartRequest builds the InteractiveStartRequest for a passthrough session. // immediateStart overrides pt.WaitForTerminal when true (used for restart/resume where the // terminal WebSocket is already connected). @@ -292,6 +312,7 @@ func (m *Manager) startPassthroughSession(ctx context.Context, execution *AgentE } execution.PassthroughProcessID = processInfo.ID + execution.PassthroughStartedAt = time.Now() m.logger.Info("passthrough session started", zap.String("execution_id", execution.ID), @@ -341,6 +362,7 @@ func (m *Manager) freshPassthroughCommand(ctx context.Context, execution *AgentE cmd := resolved.agent.BuildPassthroughCommand(agents.PassthroughOptions{ Model: profileModel(resolved.profile), PermissionValues: profilePermissionValues(resolved.profile), + CLIFlagTokens: m.profileCLIFlagTokens(resolved.profile), }) if cmd.IsEmpty() { return agents.PassthroughConfig{}, nil, agents.Command{}, fmt.Errorf("passthrough command is empty for agent %s", resolved.agentID) @@ -368,6 +390,7 @@ func (m *Manager) restartPassthroughProcess(ctx context.Context, execution *Agen oldProcessID := execution.PassthroughProcessID execution.PassthroughProcessID = "" + execution.PassthroughStartedAt = time.Time{} if err := interactiveRunner.Stop(ctx, oldProcessID); err != nil { m.logger.Warn("failed to stop passthrough process during context reset", @@ -394,6 +417,7 @@ func (m *Manager) restartPassthroughProcess(ctx context.Context, execution *Agen // 4. Update execution with new process ID execution.PassthroughProcessID = processInfo.ID + execution.PassthroughStartedAt = time.Now() m.logger.Info("passthrough process restarted with fresh context", zap.String("execution_id", execution.ID), @@ -433,6 +457,7 @@ func (m *Manager) ResumePassthroughSession(ctx context.Context, sessionID string Model: profileModel(resolved.profile), Resume: true, PermissionValues: profilePermissionValues(resolved.profile), + CLIFlagTokens: m.profileCLIFlagTokens(resolved.profile), }) if cmd.IsEmpty() { return fmt.Errorf("passthrough resume command is empty for agent %s", resolved.agentID) @@ -463,6 +488,7 @@ func (m *Manager) ResumePassthroughSession(ctx context.Context, sessionID string } execution.PassthroughProcessID = processInfo.ID + execution.PassthroughStartedAt = time.Now() m.logger.Info("passthrough session resumed", zap.String("session_id", sessionID), @@ -568,7 +594,10 @@ func (m *Manager) handlePassthroughStatus(status *agentctltypes.ProcessStatusUpd if status.Status == agentctltypes.ProcessStatusExited || status.Status == agentctltypes.ProcessStatusFailed { // Only trigger auto-restart for the passthrough process, not for user shell terminals if execution.PassthroughProcessID != "" && status.ProcessID == execution.PassthroughProcessID { - go m.handlePassthroughExit(execution, status) + // Snapshot the start time synchronously so the goroutine doesn't + // race the next launch's write to execution.PassthroughStartedAt. + startedAt := execution.PassthroughStartedAt + go m.handlePassthroughExit(execution, status, startedAt) } else { m.logger.Debug("process exited but not the passthrough process, skipping auto-restart", zap.String("session_id", status.SessionID), @@ -580,9 +609,16 @@ func (m *Manager) handlePassthroughStatus(status *agentctltypes.ProcessStatusUpd // handlePassthroughExit handles auto-restart logic when a passthrough process exits. // This function is called asynchronously to allow the old process to be cleaned up first. -func (m *Manager) handlePassthroughExit(execution *AgentExecution, status *agentctltypes.ProcessStatusUpdate) { +// startedAt is the snapshot of execution.PassthroughStartedAt taken synchronously +// at the call site — passed in rather than re-read here to avoid racing with +// the next launch's write to that field. +func (m *Manager) handlePassthroughExit(execution *AgentExecution, status *agentctltypes.ProcessStatusUpdate, startedAt time.Time) { const restartDelay = 500 * time.Millisecond const cleanupDelay = 100 * time.Millisecond // Wait for old process cleanup + // fastFailWindow is short enough to catch launch-time failures (bad CLI + // flag, missing binary, immediate auth rejection) but long enough that a + // healthy agent that does any startup work won't be mistaken for one. + const fastFailWindow = 2 * time.Second sessionID := execution.SessionID @@ -623,19 +659,41 @@ func (m *Manager) handlePassthroughExit(execution *AgentExecution, status *agent exitCode = *status.ExitCode } + // Use the exit timestamp from the status event (set when the child + // actually exited), not time.Now() — the cleanupDelay sleep and goroutine + // hops above would otherwise inflate the measured uptime by ~100 ms. + exitedAt := status.Timestamp + if exitedAt.IsZero() { + exitedAt = time.Now() + } + + // Fast-fail short-circuit: a non-zero exit shortly after start almost + // always means the launch itself was wrong (bad CLI flag, missing binary, + // auth failure). Restarting just thrashes — the next run hits the same + // failure at the same speed. Surface the failure to the user instead. + if isFastFailExit(startedAt, exitedAt, exitCode, fastFailWindow) { + m.notifyFastFailExit(interactiveRunner, sessionID, exitedAt.Sub(startedAt), exitCode, fastFailWindow) + return + } + + m.attemptPassthroughRestart(execution, interactiveRunner, sessionID, exitCode, restartDelay) +} + +// attemptPassthroughRestart announces the restart on the terminal, waits the +// restart delay, re-checks shutdown/WebSocket, and resumes the session. +// Reconnects the existing WebSocket to the new process on success. +func (m *Manager) attemptPassthroughRestart(execution *AgentExecution, runner *process.InteractiveRunner, sessionID string, exitCode int, restartDelay time.Duration) { m.logger.Info("passthrough process exited with active WebSocket, attempting auto-restart", zap.String("session_id", sessionID), zap.Int("exit_code", exitCode)) - // Send restart notification to terminal (use session-level to survive process deletion) restartMsg := "\r\n\x1b[33m[Agent exited. Restarting...]\x1b[0m\r\n" - if err := interactiveRunner.WriteToDirectOutputBySession(sessionID, []byte(restartMsg)); err != nil { + if err := runner.WriteToDirectOutputBySession(sessionID, []byte(restartMsg)); err != nil { m.logger.Debug("failed to write restart message to terminal", zap.String("session_id", sessionID), zap.Error(err)) } - // Delay before restart time.Sleep(restartDelay) // Shutdown may have started during the sleep; re-check before touching @@ -646,23 +704,19 @@ func (m *Manager) handlePassthroughExit(execution *AgentExecution, status *agent return } - // Check WebSocket is still connected after delay (use session-level tracking) - if !interactiveRunner.HasActiveWebSocketBySession(sessionID) { + if !runner.HasActiveWebSocketBySession(sessionID) { m.logger.Debug("WebSocket disconnected during restart delay, aborting", zap.String("session_id", sessionID)) return } - // Attempt restart - ctx := context.Background() - if err := m.ResumePassthroughSession(ctx, sessionID); err != nil { + if err := m.ResumePassthroughSession(context.Background(), sessionID); err != nil { m.logger.Error("failed to auto-restart passthrough session", zap.String("session_id", sessionID), zap.Error(err)) - // Send error message to terminal errorMsg := fmt.Sprintf("\r\n\x1b[31m[Restart failed: %s]\x1b[0m\r\n", err.Error()) - if writeErr := interactiveRunner.WriteToDirectOutputBySession(sessionID, []byte(errorMsg)); writeErr != nil { + if writeErr := runner.WriteToDirectOutputBySession(sessionID, []byte(errorMsg)); writeErr != nil { m.logger.Debug("failed to write restart error message to terminal", zap.String("session_id", sessionID), zap.Error(writeErr)) @@ -670,8 +724,7 @@ func (m *Manager) handlePassthroughExit(execution *AgentExecution, status *agent return } - // Connect the session's existing WebSocket to the new process - if interactiveRunner.ConnectSessionWebSocket(execution.PassthroughProcessID) { + if runner.ConnectSessionWebSocket(execution.PassthroughProcessID) { m.logger.Info("passthrough session auto-restarted and reconnected WebSocket", zap.String("session_id", sessionID), zap.String("new_process_id", execution.PassthroughProcessID)) @@ -682,6 +735,38 @@ func (m *Manager) handlePassthroughExit(execution *AgentExecution, status *agent } } +// notifyFastFailExit logs the fast-fail decision and writes a one-shot +// banner to the terminal explaining why the auto-restart was skipped. +// uptime is the measured process lifetime (status timestamp minus start +// time), pre-computed by the caller so the log reflects true child uptime. +func (m *Manager) notifyFastFailExit(runner *process.InteractiveRunner, sessionID string, uptime time.Duration, exitCode int, window time.Duration) { + m.logger.Warn("passthrough process exited fast with non-zero code, skipping auto-restart", + zap.String("session_id", sessionID), + zap.Int("exit_code", exitCode), + zap.Duration("uptime", uptime)) + failMsg := fmt.Sprintf("\r\n\x1b[31m[Agent exited (code %d) within %s. Likely cause: bad CLI flag, missing binary, or auth failure. Edit your profile and reconnect to retry.]\x1b[0m\r\n", + exitCode, window) + if err := runner.WriteToDirectOutputBySession(sessionID, []byte(failMsg)); err != nil { + m.logger.Debug("failed to write fast-fail message to terminal", + zap.String("session_id", sessionID), + zap.Error(err)) + } +} + +// isFastFailExit reports whether a passthrough process exit looks like a +// launch failure rather than a runtime exit worth restarting. A zero start +// time disables the check (e.g. recovered executions where the start time +// is unknown), so the legacy restart path remains the default. exitedAt is +// the wall-clock time the process actually exited (status.Timestamp), kept +// distinct from time.Now() so the cleanupDelay sleep above the call site +// doesn't shrink the effective window. +func isFastFailExit(startedAt, exitedAt time.Time, exitCode int, window time.Duration) bool { + if exitCode == 0 || startedAt.IsZero() { + return false + } + return exitedAt.Sub(startedAt) < window +} + // GetInteractiveRunner returns the interactive runner for passthrough mode. // Returns nil if the runtime is not available or doesn't support passthrough. func (m *Manager) GetInteractiveRunner() *process.InteractiveRunner { diff --git a/apps/backend/internal/agent/lifecycle/manager_passthrough_test.go b/apps/backend/internal/agent/lifecycle/manager_passthrough_test.go index 3d620c8791..ec393db7df 100644 --- a/apps/backend/internal/agent/lifecycle/manager_passthrough_test.go +++ b/apps/backend/internal/agent/lifecycle/manager_passthrough_test.go @@ -7,6 +7,7 @@ import ( "time" "github.com/kandev/kandev/internal/agent/agents" + settingsmodels "github.com/kandev/kandev/internal/agent/settings/models" agentctltypes "github.com/kandev/kandev/internal/agentctl/types" ) @@ -201,6 +202,30 @@ func TestBuildPassthroughCommand(t *testing.T) { opts: agents.PassthroughOptions{Model: "mock-fast", SessionID: "sess-123"}, wantCmd: []string{"mock-agent", "--tui", "--model", "mock-fast", "--resume", "sess-123"}, }, + { + name: "user cli flag tokens appended after model + settings", + agent: &testAgent{ + id: "test-agent", + StandardPassthrough: agents.StandardPassthrough{ + Cfg: agents.PassthroughConfig{ + Supported: true, + PassthroughCmd: agents.NewCommand("test-cli"), + ModelFlag: agents.NewParam("--model", "{model}"), + ResumeFlag: agents.NewParam("-c"), + }, + PermSettings: map[string]agents.PermissionSetting{ + "auto_approve": {Supported: true, ApplyMethod: "cli_flag", CLIFlag: "--yes"}, + }, + }, + }, + opts: agents.PassthroughOptions{ + Model: "gpt-4", + Resume: true, + PermissionValues: map[string]bool{"auto_approve": true}, + CLIFlagTokens: []string{"--debug", "--log-level", "trace"}, + }, + wantCmd: []string{"test-cli", "--model", "gpt-4", "--yes", "--debug", "--log-level", "trace", "-c"}, + }, } for _, tt := range tests { @@ -251,13 +276,122 @@ func TestManager_HandlePassthroughExit_SkipsDuringShutdown(t *testing.T) { status := &agentctltypes.ProcessStatusUpdate{SessionID: "sess-1"} start := time.Now() - mgr.handlePassthroughExit(execution, status) + mgr.handlePassthroughExit(execution, status, start) if elapsed := time.Since(start); elapsed != 0 { t.Errorf("handlePassthroughExit advanced fake time by %v — did not short-circuit during shutdown", elapsed) } }) } +// TestIsFastFailExit covers the predicate that decides whether a passthrough +// exit looks like a launch failure (bad CLI flag, missing binary, auth +// rejection) and should bypass the auto-restart loop. +func TestIsFastFailExit(t *testing.T) { + const window = 2 * time.Second + now := time.Now() + + tests := []struct { + name string + startedAt time.Time + exitedAt time.Time + exitCode int + want bool + }{ + { + name: "fast exit with non-zero code → fast-fail", + startedAt: now, + exitedAt: now.Add(100 * time.Millisecond), + exitCode: 1, + want: true, + }, + { + name: "slow exit with non-zero code → restart", + startedAt: now, + exitedAt: now.Add(5 * time.Second), + exitCode: 1, + want: false, + }, + { + name: "fast exit with zero code → not fast-fail (clean exit)", + startedAt: now, + exitedAt: now.Add(100 * time.Millisecond), + exitCode: 0, + want: false, + }, + { + name: "zero start time → check disabled (recovered execution)", + startedAt: time.Time{}, + exitedAt: now, + exitCode: 1, + want: false, + }, + { + name: "exit-time-based measurement is independent of caller delays", + // Process actually ran 50ms; caller's wall-clock-since-start would + // be much larger (e.g. after the cleanupDelay sleep), but the + // status.Timestamp pins true uptime to 50ms → fast-fail. + startedAt: now, + exitedAt: now.Add(50 * time.Millisecond), + exitCode: 127, + want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := isFastFailExit(tt.startedAt, tt.exitedAt, tt.exitCode, window); got != tt.want { + t.Errorf("isFastFailExit() = %v, want %v", got, tt.want) + } + }) + } +} + +// TestManager_ProfileCLIFlagTokens confirms profile-configured cli_flags +// reach the passthrough launch path (regression for issue #718, where the +// passthrough builder silently dropped them). +func TestManager_ProfileCLIFlagTokens(t *testing.T) { + mgr := newTestManager() + + t.Run("nil profile returns nil", func(t *testing.T) { + if got := mgr.profileCLIFlagTokens(nil); got != nil { + t.Errorf("profileCLIFlagTokens(nil) = %v, want nil", got) + } + }) + + t.Run("enabled flags tokenised, disabled skipped", func(t *testing.T) { + profile := &AgentProfileInfo{ + ProfileID: "p1", + CLIFlags: []settingsmodels.CLIFlag{ + {Flag: "--allow-all-tools", Enabled: true}, + {Flag: "--skip-me", Enabled: false}, + {Flag: "--add-dir /shared", Enabled: true}, + }, + } + got := mgr.profileCLIFlagTokens(profile) + want := []string{"--allow-all-tools", "--add-dir", "/shared"} + if len(got) != len(want) { + t.Fatalf("profileCLIFlagTokens() = %v, want %v", got, want) + } + for i, tok := range want { + if got[i] != tok { + t.Errorf("profileCLIFlagTokens()[%d] = %q, want %q", i, got[i], tok) + } + } + }) + + t.Run("malformed flag does not abort — returns nil and warns", func(t *testing.T) { + profile := &AgentProfileInfo{ + ProfileID: "p2", + CLIFlags: []settingsmodels.CLIFlag{ + {Flag: `--broken "unterminated`, Enabled: true}, + }, + } + if got := mgr.profileCLIFlagTokens(profile); got != nil { + t.Errorf("profileCLIFlagTokens(malformed) = %v, want nil", got) + } + }) +} + func TestManager_VerifyPassthroughEnabled(t *testing.T) { tests := []struct { name string diff --git a/apps/backend/internal/agent/lifecycle/types.go b/apps/backend/internal/agent/lifecycle/types.go index 0b71f94b90..774500d2dc 100644 --- a/apps/backend/internal/agent/lifecycle/types.go +++ b/apps/backend/internal/agent/lifecycle/types.go @@ -58,7 +58,8 @@ type AgentExecution struct { standalonePort int // Port of the standalone execution // Passthrough mode info (CLI passthrough without ACP) - PassthroughProcessID string // Process ID in the interactive runner (empty if not in passthrough mode) + PassthroughProcessID string // Process ID in the interactive runner (empty if not in passthrough mode) + PassthroughStartedAt time.Time // When the current passthrough process was launched; used to detect fast-fail exits and skip auto-restart loops // isResumedSession is true when this execution was created as part of a session resume // (e.g., after backend restart). Used by StartAgentProcess to route passthrough sessions