diff --git a/apps/backend/cmd/kandev/adapters.go b/apps/backend/cmd/kandev/adapters.go index a121fd0c48..5e19957a24 100644 --- a/apps/backend/cmd/kandev/adapters.go +++ b/apps/backend/cmd/kandev/adapters.go @@ -8,6 +8,7 @@ import ( "go.uber.org/zap" + "github.com/kandev/kandev/internal/agent/agents" "github.com/kandev/kandev/internal/agent/registry" "github.com/kandev/kandev/internal/agent/runtime/agentctl" "github.com/kandev/kandev/internal/agent/runtime/lifecycle" @@ -360,6 +361,11 @@ func (a *lifecycleAdapter) WritePassthroughStdin(ctx context.Context, sessionID return a.mgr.WritePassthroughStdin(ctx, sessionID, data) } +// ResolvePassthroughConfig returns the resolved PassthroughConfig for a session's agent. +func (a *lifecycleAdapter) ResolvePassthroughConfig(ctx context.Context, sessionID string) (agents.PassthroughConfig, error) { + return a.mgr.ResolvePassthroughConfig(ctx, sessionID) +} + func (a *lifecycleAdapter) MarkPassthroughRunning(sessionID string) error { return a.mgr.MarkPassthroughRunning(sessionID) } diff --git a/apps/backend/internal/agent/agents/agent.go b/apps/backend/internal/agent/agents/agent.go index c80fc5db9c..0e7c3abb17 100644 --- a/apps/backend/internal/agent/agents/agent.go +++ b/apps/backend/internal/agent/agents/agent.go @@ -252,6 +252,25 @@ type PassthroughConfig struct { ResumeFlag Param // generic "continue last session" (e.g. NewParam("-c"), NewParam("--resume", "latest")) SessionResumeFlag Param // resume a specific session by ID (e.g. NewParam("--resume")) WaitForTerminal bool + // AutoInjectPrompt enables writing the task description to the PTY stdin + // after the first idle window. Default false preserves today's behavior. + AutoInjectPrompt bool + // SubmitSequence is appended after the prompt text when auto-injecting + // and when routing chat-compose messages to the PTY. "\r" for most TUIs. + // Empty inherits DefaultPassthroughSubmitSequence at PTY write sites. + SubmitSequence string + // DisableBracketedPaste sends prompt bytes verbatim (plus SubmitSequence). + // Claude Code enables bracketed-paste *mode* (?2004h) in its Ink TUI; injecting + // ESC[200~…ESC[201~ delimiters breaks input (nothing appears in the prompt). + DisableBracketedPaste bool + // SubmitDelay is the wait inserted before each non-first chunk when writing the + // prompt+submit sequence to PTY stdin. Ink-based TUIs (Claude Code) detect a + // "paste burst" when many stdin bytes arrive in one read and absorb the + // trailing \r into the pasted content instead of dispatching it as Enter. + // Splitting the prompt body from the submit byte with a small delay forces the + // submit to arrive as a discrete keystroke. 0 disables (other TUIs handle one + // atomic write fine). + SubmitDelay time.Duration } // DefaultBufferMaxBytes is the default maximum buffer size for passthrough mode (2 MB). diff --git a/apps/backend/internal/agent/agents/claude_acp.go b/apps/backend/internal/agent/agents/claude_acp.go index 2a88e27bbe..d720f1b52a 100644 --- a/apps/backend/internal/agent/agents/claude_acp.go +++ b/apps/backend/internal/agent/agents/claude_acp.go @@ -35,15 +35,24 @@ func NewClaudeACP() *ClaudeACP { StandardPassthrough: StandardPassthrough{ PermSettings: emptyPermSettings, Cfg: PassthroughConfig{ - Supported: true, - Label: "CLI Passthrough", - Description: "Show terminal directly instead of chat interface", - PassthroughCmd: NewCommand("npx", "-y", "@anthropic-ai/claude-code", "--verbose"), - ModelFlag: NewParam("--model", "{model}"), - IdleTimeout: 3 * time.Second, - BufferMaxBytes: DefaultBufferMaxBytes, - ResumeFlag: NewParam("-c"), - SessionResumeFlag: NewParam("--resume"), + Supported: true, + Label: "CLI Passthrough", + Description: "Show terminal directly instead of chat interface", + PassthroughCmd: NewCommand("npx", "-y", "@anthropic-ai/claude-code", "--verbose"), + ModelFlag: NewParam("--model", "{model}"), + IdleTimeout: 3 * time.Second, + BufferMaxBytes: DefaultBufferMaxBytes, + ResumeFlag: NewParam("-c"), + SessionResumeFlag: NewParam("--resume"), + AutoInjectPrompt: true, + SubmitSequence: "\r", + DisableBracketedPaste: true, + // Claude Code's Ink TUI coalesces multi-byte stdin reads into a + // paste burst, absorbing trailing "\r" into the input rather than + // dispatching Enter. A short delay before the submit byte forces + // it to arrive as a discrete keystroke. 150ms is just over Ink's + // paste-detection window and still feels instant to the user. + SubmitDelay: 150 * time.Millisecond, }, }, } diff --git a/apps/backend/internal/agent/agents/codex_acp.go b/apps/backend/internal/agent/agents/codex_acp.go index fb537f6143..bfe06de7be 100644 --- a/apps/backend/internal/agent/agents/codex_acp.go +++ b/apps/backend/internal/agent/agents/codex_acp.go @@ -30,18 +30,34 @@ type CodexACP struct { StandardPassthrough } +// codexPassthroughPermSettings maps profile toggles to @openai/codex CLI flags. +// The legacy --full-auto flag was removed; auto_approve uses --ask-for-approval never. +var codexPassthroughPermSettings = map[string]PermissionSetting{ + "auto_approve": { + Supported: true, + Default: true, + Label: "Auto approve", + Description: "Skip command approval prompts (--ask-for-approval never)", + ApplyMethod: PermissionApplyMethodCLIFlag, + CLIFlag: "--ask-for-approval", + CLIFlagValue: "never", + }, +} + func NewCodexACP() *CodexACP { return &CodexACP{ StandardPassthrough: StandardPassthrough{ - PermSettings: emptyPermSettings, + PermSettings: codexPassthroughPermSettings, Cfg: PassthroughConfig{ - Supported: true, - Label: "CLI Passthrough", - Description: "Show terminal directly instead of chat interface", - PassthroughCmd: NewCommand("npx", "-y", "@openai/codex", "--full-auto"), - ModelFlag: NewParam("--model", "{model}"), - IdleTimeout: 3 * time.Second, - BufferMaxBytes: DefaultBufferMaxBytes, + Supported: true, + Label: "CLI Passthrough", + Description: "Show terminal directly instead of chat interface", + PassthroughCmd: NewCommand("npx", "-y", "@openai/codex"), + ModelFlag: NewParam("--model", "{model}"), + IdleTimeout: 3 * time.Second, + BufferMaxBytes: DefaultBufferMaxBytes, + AutoInjectPrompt: true, + SubmitSequence: "\r", }, }, } @@ -149,7 +165,7 @@ func (a *CodexACP) InstallScript() string { func (a *CodexACP) BillingType() usage.BillingType { return codexBillingType() } func (a *CodexACP) PermissionSettings() map[string]PermissionSetting { - return emptyPermSettings + return codexPassthroughPermSettings } // InferenceConfig returns configuration for one-shot inference using ACP. diff --git a/apps/backend/internal/agent/agents/codex_acp_passthrough_test.go b/apps/backend/internal/agent/agents/codex_acp_passthrough_test.go new file mode 100644 index 0000000000..52140335b3 --- /dev/null +++ b/apps/backend/internal/agent/agents/codex_acp_passthrough_test.go @@ -0,0 +1,34 @@ +package agents + +import ( + "slices" + "testing" +) + +func TestCodexACP_PassthroughCmd_NoRemovedFullAutoFlag(t *testing.T) { + pt := NewCodexACP().PassthroughConfig().PassthroughCmd.Args() + want := []string{"npx", "-y", "@openai/codex"} + if !slices.Equal(pt, want) { + t.Fatalf("PassthroughCmd = %#v, want %#v", pt, want) + } +} + +func TestCodexACP_BuildPassthroughCommand_AutoApprove(t *testing.T) { + cmd := NewCodexACP().BuildPassthroughCommand(PassthroughOptions{ + PermissionValues: map[string]bool{"auto_approve": true}, + }) + want := []string{"npx", "-y", "@openai/codex", "--ask-for-approval", "never"} + if !slices.Equal(cmd.Args(), want) { + t.Fatalf("argv = %#v, want %#v", cmd.Args(), want) + } +} + +func TestCodexACP_BuildPassthroughCommand_AutoApproveDisabled(t *testing.T) { + cmd := NewCodexACP().BuildPassthroughCommand(PassthroughOptions{ + PermissionValues: map[string]bool{"auto_approve": false}, + }) + want := []string{"npx", "-y", "@openai/codex"} + if !slices.Equal(cmd.Args(), want) { + t.Fatalf("argv = %#v, want %#v", cmd.Args(), want) + } +} diff --git a/apps/backend/internal/agent/agents/passthrough_payload.go b/apps/backend/internal/agent/agents/passthrough_payload.go new file mode 100644 index 0000000000..5dd836f8be --- /dev/null +++ b/apps/backend/internal/agent/agents/passthrough_payload.go @@ -0,0 +1,69 @@ +package agents + +import ( + "strings" + "time" +) + +const ( + bracketedPasteStart = "\x1b[200~" + bracketedPasteEnd = "\x1b[201~" +) + +// PassthroughStdinChunk is one PTY stdin write planned for a passthrough prompt. +// DelayBefore is sleep applied before writing this chunk; it is zero on the first +// chunk and inherits PassthroughConfig.SubmitDelay on subsequent chunks. Splitting +// the prompt body from the submit byte with a delay defeats Ink-style paste-burst +// detection in TUIs like Claude Code (see PassthroughConfig.SubmitDelay). +type PassthroughStdinChunk struct { + Data string + DelayBefore time.Duration +} + +// PassthroughSubmitSequence returns the byte sequence to append after passthrough +// stdin text. Empty SubmitSequence inherits DefaultPassthroughSubmitSequence ("\r"). +func PassthroughSubmitSequence(cfg PassthroughConfig) string { + return EffectiveSubmitSequence(cfg.SubmitSequence) +} + +// BuildPassthroughPayload assembles the bytes for one atomic PTY stdin write. +// Multi-line prompts use bracketed paste so embedded newlines are not treated as +// premature Enter presses, unless DisableBracketedPaste is set (Claude Code). +func BuildPassthroughPayload(prompt string, cfg PassthroughConfig) string { + submit := PassthroughSubmitSequence(cfg) + if cfg.DisableBracketedPaste || !strings.Contains(prompt, "\n") { + return prompt + submit + } + return bracketedPasteStart + prompt + bracketedPasteEnd + submit +} + +// PlanPassthroughStdinChunks plans PTY stdin writes for a passthrough prompt. +// When SubmitDelay > 0 (Claude path) the prompt body and submit byte are emitted +// as two chunks with DelayBefore set on the submit chunk so it arrives as a +// discrete keystroke. Otherwise a single atomic write is returned, preserving +// today's behavior for TUIs that handle prompt+submit in one read (Cursor, Codex, +// OpenCode). +func PlanPassthroughStdinChunks(prompt string, cfg PassthroughConfig) []PassthroughStdinChunk { + if cfg.SubmitDelay > 0 { + body := prompt + if !cfg.DisableBracketedPaste && strings.Contains(prompt, "\n") { + body = bracketedPasteStart + prompt + bracketedPasteEnd + } + return []PassthroughStdinChunk{ + {Data: body}, + {Data: PassthroughSubmitSequence(cfg), DelayBefore: cfg.SubmitDelay}, + } + } + return []PassthroughStdinChunk{{Data: BuildPassthroughPayload(prompt, cfg)}} +} + +// PlanPassthroughStdinWrites is the legacy string-only view of PlanPassthroughStdinChunks. +// Retained for tests and call sites that don't need to honor per-chunk delays. +func PlanPassthroughStdinWrites(prompt string, cfg PassthroughConfig) []string { + chunks := PlanPassthroughStdinChunks(prompt, cfg) + out := make([]string, len(chunks)) + for i, c := range chunks { + out[i] = c.Data + } + return out +} diff --git a/apps/backend/internal/agent/agents/passthrough_payload_test.go b/apps/backend/internal/agent/agents/passthrough_payload_test.go new file mode 100644 index 0000000000..e1d98383fa --- /dev/null +++ b/apps/backend/internal/agent/agents/passthrough_payload_test.go @@ -0,0 +1,82 @@ +package agents + +import ( + "testing" + "time" +) + +func TestPlanPassthroughStdinWrites_SingleAtomicWrite(t *testing.T) { + cfg := PassthroughConfig{SubmitSequence: "\r"} + got := PlanPassthroughStdinWrites("line1\nline2", cfg) + if len(got) != 1 { + t.Fatalf("got %d chunks, want 1 atomic write: %#v", len(got), got) + } + want := "\x1b[200~line1\nline2\x1b[201~\r" + if got[0] != want { + t.Errorf("payload = %q, want %q", got[0], want) + } +} + +func TestPlanPassthroughStdinWrites_SingleLine(t *testing.T) { + cfg := PassthroughConfig{SubmitSequence: "\r"} + got := PlanPassthroughStdinWrites("hello", cfg) + if len(got) != 1 || got[0] != "hello\r" { + t.Fatalf("got %#v, want [\"hello\\r\"]", got) + } +} + +// Claude's PassthroughConfig sets SubmitDelay so the submit byte arrives as a +// discrete keystroke (defeats Ink's paste-burst detection). Single-line and +// multi-line prompts both emit body+submit as two chunks; the submit chunk +// carries DelayBefore = SubmitDelay. +func TestPlanPassthroughStdinChunks_ClaudeSplitsSubmit(t *testing.T) { + cfg := NewClaudeACP().PassthroughConfig() + if cfg.SubmitDelay <= 0 { + t.Fatalf("Claude config must set SubmitDelay > 0 for paste-burst workaround, got %v", cfg.SubmitDelay) + } + + for _, prompt := range []string{"hello", "### Review Comments\n\n> fix"} { + chunks := PlanPassthroughStdinChunks(prompt, cfg) + if len(chunks) != 2 { + t.Fatalf("prompt %q: got %d chunks, want 2 (body, submit): %#v", prompt, len(chunks), chunks) + } + if chunks[0].Data != prompt { + t.Errorf("prompt %q: body chunk = %q, want verbatim prompt", prompt, chunks[0].Data) + } + if chunks[0].DelayBefore != 0 { + t.Errorf("prompt %q: body chunk DelayBefore = %v, want 0", prompt, chunks[0].DelayBefore) + } + if chunks[1].Data != "\r" { + t.Errorf("prompt %q: submit chunk = %q, want \\r", prompt, chunks[1].Data) + } + if chunks[1].DelayBefore != cfg.SubmitDelay { + t.Errorf("prompt %q: submit DelayBefore = %v, want %v", prompt, chunks[1].DelayBefore, cfg.SubmitDelay) + } + } +} + +// Non-Claude TUIs (SubmitDelay == 0) keep the single-atomic-write semantics so we +// don't accidentally regress Cursor/Codex/OpenCode by splitting their submit. +func TestPlanPassthroughStdinChunks_AtomicWhenNoDelay(t *testing.T) { + cfg := PassthroughConfig{SubmitSequence: "\r"} + chunks := PlanPassthroughStdinChunks("hello", cfg) + if len(chunks) != 1 { + t.Fatalf("got %d chunks, want 1 atomic write: %#v", len(chunks), chunks) + } + if chunks[0].DelayBefore != 0 { + t.Errorf("atomic chunk DelayBefore = %v, want 0", chunks[0].DelayBefore) + } + if chunks[0].Data != "hello\r" { + t.Errorf("atomic chunk Data = %q, want \"hello\\r\"", chunks[0].Data) + } +} + +// Sanity: a custom config with SubmitDelay set but no Claude-specific flags +// also splits — the field is the lever, not the Claude struct identity. +func TestPlanPassthroughStdinChunks_SubmitDelayDrivesSplit(t *testing.T) { + cfg := PassthroughConfig{SubmitSequence: "\r", DisableBracketedPaste: true, SubmitDelay: 50 * time.Millisecond} + chunks := PlanPassthroughStdinChunks("hi", cfg) + if len(chunks) != 2 || chunks[1].DelayBefore != 50*time.Millisecond || chunks[1].Data != "\r" { + t.Fatalf("expected split with 50ms delay before \\r, got %#v", chunks) + } +} diff --git a/apps/backend/internal/agent/agents/passthrough_submit.go b/apps/backend/internal/agent/agents/passthrough_submit.go new file mode 100644 index 0000000000..a47726834c --- /dev/null +++ b/apps/backend/internal/agent/agents/passthrough_submit.go @@ -0,0 +1,14 @@ +package agents + +// DefaultPassthroughSubmitSequence is appended after PTY prompt text when an agent +// omits PassthroughConfig.SubmitSequence. Most TUI CLIs expect carriage-return. +const DefaultPassthroughSubmitSequence = "\r" + +// EffectiveSubmitSequence returns the bytes to append after passthrough stdin text. +// Agents that leave SubmitSequence empty inherit DefaultPassthroughSubmitSequence. +func EffectiveSubmitSequence(seq string) string { + if seq == "" { + return DefaultPassthroughSubmitSequence + } + return seq +} diff --git a/apps/backend/internal/agent/agents/passthrough_submit_test.go b/apps/backend/internal/agent/agents/passthrough_submit_test.go new file mode 100644 index 0000000000..6252fef109 --- /dev/null +++ b/apps/backend/internal/agent/agents/passthrough_submit_test.go @@ -0,0 +1,12 @@ +package agents + +import "testing" + +func TestEffectiveSubmitSequence(t *testing.T) { + if got := EffectiveSubmitSequence(""); got != "\r" { + t.Errorf("empty = %q, want \\r", got) + } + if got := EffectiveSubmitSequence("\n"); got != "\n" { + t.Errorf("explicit \\n = %q", got) + } +} diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_interaction.go b/apps/backend/internal/agent/runtime/lifecycle/manager_interaction.go index 9e383e62fe..20d895f879 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/manager_interaction.go +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_interaction.go @@ -372,12 +372,26 @@ var ErrCancelEscalated = errors.New("cancel escalated: agent did not acknowledge // CancelAgentBySessionID cancels the current agent turn for a specific session. // Returns ErrNoExecutionForSession (wrapped) when no execution is tracked for the session. +// +// Passthrough sessions don't speak ACP — Ctrl-C (0x03) written to PTY stdin is +// the only stop signal a TUI CLI understands. A write failure is non-fatal: +// the caller still reconciles DB state so the UI unsticks even if the PTY is +// already gone. func (m *Manager) CancelAgentBySessionID(ctx context.Context, sessionID string) error { execution, exists := m.executionStore.GetBySessionID(sessionID) if !exists { return fmt.Errorf("session %q: %w", sessionID, ErrNoExecutionForSession) } + if execution.PassthroughProcessID != "" { + if err := m.WritePassthroughStdin(ctx, sessionID, "\x03"); err != nil { + m.logger.Warn("failed to write Ctrl-C to passthrough stdin", + zap.String("session_id", sessionID), + zap.Error(err)) + } + return nil + } + return m.CancelAgent(ctx, execution.ID) } diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_passthrough.go b/apps/backend/internal/agent/runtime/lifecycle/manager_passthrough.go index 9f84bd69e9..6d5f579bfb 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/manager_passthrough.go +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_passthrough.go @@ -201,6 +201,19 @@ func (m *Manager) resolvePassthroughAgent(ctx context.Context, execution *AgentE }, nil } +// promptForPassthroughCommand returns the prompt that should be passed to +// BuildPassthroughCommand. When the agent uses idle-based auto-inject and has +// no PromptFlag, the prompt would otherwise be appended as a positional arg +// (putting TUIs like Claude into non-interactive `-p` mode and exiting before +// auto-inject fires). In that case we return "" so the prompt is delivered via +// PTY stdin in autoInjectInitialPrompt. +func promptForPassthroughCommand(pt agents.PassthroughConfig, taskDescription string) string { + if pt.AutoInjectPrompt && pt.PromptFlag.IsEmpty() { + return "" + } + return taskDescription +} + // passthroughAgentCommand validates passthrough support and builds the command for a passthrough session. // Returns the PassthroughAgent, PassthroughConfig, RuntimeConfig pointer, command, and any error. func (m *Manager) passthroughAgentCommand(execution *AgentExecution, profileInfo *AgentProfileInfo) (agents.PassthroughAgent, agents.PassthroughConfig, *agents.RuntimeConfig, agents.Command, error) { @@ -217,11 +230,12 @@ func (m *Manager) passthroughAgentCommand(execution *AgentExecution, profileInfo pt := ptAgent.PassthroughConfig() rt := agentConfig.Runtime() taskDescription := getTaskDescriptionFromMetadata(execution) + promptForCmd := promptForPassthroughCommand(pt, taskDescription) cmd := ptAgent.BuildPassthroughCommand(agents.PassthroughOptions{ Model: profileModel(profileInfo), SessionID: execution.ACPSessionID, - Prompt: taskDescription, + Prompt: promptForCmd, PermissionValues: profilePermissionValues(profileInfo), CLIFlagTokens: m.profileCLIFlagTokens(profileInfo), }) @@ -330,6 +344,8 @@ func (m *Manager) startPassthroughSession(ctx context.Context, execution *AgentE go m.streamManager.connectWorkspaceStream(execution, nil) } + go m.autoInjectInitialPrompt(execution, pt) + return nil } @@ -786,6 +802,9 @@ func (m *Manager) attemptResumeFallback(execution *AgentExecution, runner *proce if m.streamManager != nil && execution.agentctl != nil && execution.GetWorkspaceStream() == nil { go m.streamManager.connectWorkspaceStream(execution, nil) } + + // Fallback path is a fresh session (no --resume) — re-inject the prompt. + go m.autoInjectInitialPrompt(execution, pt) } // attemptPassthroughRestart announces the restart on the terminal, waits the @@ -912,6 +931,104 @@ func isFastFailExit(startedAt, exitedAt time.Time, exitCode int, window time.Dur return exitedAt.Sub(startedAt) < window } +// passthroughRunner is the minimal seam autoInjectInitialPrompt needs from +// *process.InteractiveRunner. Defined as an interface so tests can supply a +// fake runner without spinning up a real PTY subprocess. +type passthroughRunner interface { + WaitForFirstIdle(ctx context.Context, processID string) error + WriteStdin(processID string, data string) error +} + +// autoInjectInitialPrompt writes the task description to the PTY stdin once +// the agent is idle (ready for input). Opt-in per agent via PassthroughConfig. +// Called from startPassthroughSession and attemptResumeFallback only — never +// from ResumePassthroughSession (would duplicate the prompt in agent history). +func (m *Manager) autoInjectInitialPrompt(execution *AgentExecution, pt agents.PassthroughConfig) { + runner := m.GetInteractiveRunner() + if runner == nil { + return + } + m.autoInjectInitialPromptWith(runner, execution, pt) +} + +// autoInjectInitialPromptWith is the testable inner of autoInjectInitialPrompt, +// taking a runner seam so unit tests can avoid spawning a real PTY. +func (m *Manager) autoInjectInitialPromptWith(runner passthroughRunner, execution *AgentExecution, pt agents.PassthroughConfig) { + if !pt.AutoInjectPrompt { + return + } + if !pt.PromptFlag.IsEmpty() { + // The agent already received the prompt as a CLI flag. + return + } + description := getTaskDescriptionFromMetadata(execution) + if description == "" { + return + } + processID := execution.PassthroughProcessID + if processID == "" { + m.logger.Warn("autoInjectInitialPrompt called without passthrough process", + zap.String("execution_id", execution.ID)) + return + } + ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) + defer cancel() + if err := runner.WaitForFirstIdle(ctx, processID); err != nil { + m.logger.Warn("autoInjectInitialPrompt timed out waiting for idle", + zap.String("execution_id", execution.ID), + zap.String("process_id", processID), + zap.Error(err)) + return + } + // WaitForFirstIdle also unblocks when the process exits — skip the write + // during shutdown so we don't race the lifecycle manager's teardown. If the + // process is already gone, WriteStdin returns "process not found" and the + // existing error branch logs it. + if m.IsShuttingDown() { + return + } + // Mark RUNNING before the chunk loop so a composer/message.add fired during + // the inter-chunk SubmitDelay window (150ms for Claude) is blocked by + // checkSessionPromptable instead of racing into the same PTY mid-submit. + if err := m.MarkPassthroughRunning(execution.SessionID); err != nil { + m.logger.Warn("failed to mark passthrough as running before auto-inject", + zap.String("execution_id", execution.ID), + zap.String("session_id", execution.SessionID), + zap.Error(err)) + } + for _, chunk := range agents.PlanPassthroughStdinChunks(description, pt) { + if chunk.DelayBefore > 0 { + time.Sleep(chunk.DelayBefore) + } + if err := runner.WriteStdin(processID, chunk.Data); err != nil { + m.logger.Warn("autoInjectInitialPrompt write failed", + zap.String("execution_id", execution.ID), + zap.String("process_id", processID), + zap.Error(err)) + return + } + } + m.logger.Info("autoInjectInitialPrompt wrote task description to PTY", + zap.String("execution_id", execution.ID), + zap.String("process_id", processID), + zap.Int("description_len", len(description))) +} + +// ResolvePassthroughConfig returns the PassthroughConfig for a session's agent. +// Used by callers outside this package (e.g. orchestrator) that need the submit +// sequence to write to PTY stdin. +func (m *Manager) ResolvePassthroughConfig(ctx context.Context, sessionID string) (agents.PassthroughConfig, error) { + execution, exists := m.executionStore.GetBySessionID(sessionID) + if !exists { + return agents.PassthroughConfig{}, fmt.Errorf("no execution for session %q", sessionID) + } + resolved, err := m.resolvePassthroughAgent(ctx, execution) + if err != nil { + return agents.PassthroughConfig{}, err + } + return resolved.pt, nil +} + // 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/runtime/lifecycle/manager_passthrough_autoinject_test.go b/apps/backend/internal/agent/runtime/lifecycle/manager_passthrough_autoinject_test.go new file mode 100644 index 0000000000..27677fc144 --- /dev/null +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_passthrough_autoinject_test.go @@ -0,0 +1,213 @@ +package lifecycle + +import ( + "context" + "sync" + "testing" + "time" + + "github.com/kandev/kandev/internal/agent/agents" +) + +// fakePassthroughRunner is a minimal stub of the passthroughRunner seam used +// to drive autoInjectInitialPromptWith in tests without spawning a real PTY. +type fakePassthroughRunner struct { + mu sync.Mutex + + // waitErr is returned from WaitForFirstIdle. If waitBlock is true, the call + // blocks until ctx is canceled regardless of waitErr. + waitErr error + waitBlock bool + + // writeErr is returned from WriteStdin. + writeErr error + + // captured data passed to WriteStdin (last call). + writtenProcessID string + writtenData string + writeCalled bool + + // every chunk passed to WriteStdin, in order. Used by SubmitDelay tests that + // need to assert the body/submit split (the single-string captures above only + // hold the most recent write). + writes []string + // writeTimes records the wall-clock time of each WriteStdin call so tests can + // assert that SubmitDelay was honored between chunks without coupling to a + // specific clock. + writeTimes []time.Time +} + +func (f *fakePassthroughRunner) WaitForFirstIdle(ctx context.Context, processID string) error { + if f.waitBlock { + <-ctx.Done() + return ctx.Err() + } + return f.waitErr +} + +func (f *fakePassthroughRunner) WriteStdin(processID string, data string) error { + f.mu.Lock() + defer f.mu.Unlock() + f.writeCalled = true + f.writtenProcessID = processID + f.writtenData = data + f.writes = append(f.writes, data) + f.writeTimes = append(f.writeTimes, time.Now()) + return f.writeErr +} + +func newAutoInjectExecution(description string) *AgentExecution { + exec := &AgentExecution{ + ID: "exec-1", + SessionID: "sess-1", + PassthroughProcessID: "proc-abc", + } + if description != "" { + exec.Metadata = map[string]interface{}{ + "task_description": description, + } + } + return exec +} + +func TestAutoInject_disabled_does_nothing(t *testing.T) { + mgr := newTestManager() + runner := &fakePassthroughRunner{} + + mgr.autoInjectInitialPromptWith(runner, newAutoInjectExecution("do a thing"), agents.PassthroughConfig{ + AutoInjectPrompt: false, + SubmitSequence: "\r", + }) + + if runner.writeCalled { + t.Fatalf("expected no stdin write when AutoInjectPrompt is false, got data=%q", runner.writtenData) + } +} + +func TestAutoInject_skipped_when_PromptFlag_set(t *testing.T) { + mgr := newTestManager() + runner := &fakePassthroughRunner{} + + mgr.autoInjectInitialPromptWith(runner, newAutoInjectExecution("do a thing"), agents.PassthroughConfig{ + AutoInjectPrompt: true, + SubmitSequence: "\r", + PromptFlag: agents.NewParam("--prompt", "{prompt}"), + }) + + if runner.writeCalled { + t.Fatalf("expected no stdin write when PromptFlag is set, got data=%q", runner.writtenData) + } +} + +func TestAutoInject_skipped_when_description_empty(t *testing.T) { + mgr := newTestManager() + runner := &fakePassthroughRunner{} + + mgr.autoInjectInitialPromptWith(runner, newAutoInjectExecution(""), agents.PassthroughConfig{ + AutoInjectPrompt: true, + SubmitSequence: "\r", + }) + + if runner.writeCalled { + t.Fatalf("expected no stdin write when task description is empty, got data=%q", runner.writtenData) + } +} + +func TestAutoInject_writes_description_plus_submit(t *testing.T) { + mgr := newTestManager() + runner := &fakePassthroughRunner{} + + mgr.autoInjectInitialPromptWith(runner, newAutoInjectExecution("hello world"), agents.PassthroughConfig{ + AutoInjectPrompt: true, + SubmitSequence: "\r", + }) + + if !runner.writeCalled { + t.Fatalf("expected WriteStdin to be called") + } + if runner.writtenProcessID != "proc-abc" { + t.Errorf("WriteStdin processID = %q, want %q", runner.writtenProcessID, "proc-abc") + } + if runner.writtenData != "hello world\r" { + t.Errorf("WriteStdin data = %q, want %q", runner.writtenData, "hello world\r") + } +} + +// TestAutoInject_returns_when_wait_errors exercises the WaitForFirstIdle +// error-return path: the helper must skip the stdin write and exit cleanly +// rather than panic or loop. +func TestAutoInject_returns_when_wait_errors(t *testing.T) { + mgr := newTestManager() + runner := &fakePassthroughRunner{waitErr: context.DeadlineExceeded} + + done := make(chan struct{}) + go func() { + defer close(done) + mgr.autoInjectInitialPromptWith(runner, newAutoInjectExecution("hello"), agents.PassthroughConfig{ + AutoInjectPrompt: true, + SubmitSequence: "\r", + }) + }() + + select { + case <-done: + case <-time.After(2 * time.Second): + t.Fatalf("autoInjectInitialPromptWith did not return within 2s") + } + + if runner.writeCalled { + t.Fatalf("expected no stdin write on wait timeout, got data=%q", runner.writtenData) + } +} + +// TestAutoInject_SubmitDelay_splits_writes_with_pause is the Claude regression: +// when SubmitDelay > 0 the helper must write the body chunk first, then sleep +// SubmitDelay, then write the submit byte alone, so Ink's paste-burst detector +// sees the trailing \r as a discrete keystroke instead of absorbing it into the +// pasted text. +func TestAutoInject_SubmitDelay_splits_writes_with_pause(t *testing.T) { + mgr := newTestManager() + runner := &fakePassthroughRunner{} + + const delay = 40 * time.Millisecond + mgr.autoInjectInitialPromptWith(runner, newAutoInjectExecution("hello world"), agents.PassthroughConfig{ + AutoInjectPrompt: true, + SubmitSequence: "\r", + DisableBracketedPaste: true, + SubmitDelay: delay, + }) + + if got := len(runner.writes); got != 2 { + t.Fatalf("expected 2 writes (body, submit), got %d: %#v", got, runner.writes) + } + if runner.writes[0] != "hello world" { + t.Errorf("body write = %q, want %q", runner.writes[0], "hello world") + } + if runner.writes[1] != "\r" { + t.Errorf("submit write = %q, want %q", runner.writes[1], "\r") + } + // Allow generous slack — CI clocks are noisy but a 40ms gap is still easy + // to detect. The submit write must arrive at least ~SubmitDelay after the + // body write; if the two were back-to-back this delta would be sub-ms. + gap := runner.writeTimes[1].Sub(runner.writeTimes[0]) + if gap < delay/2 { + t.Errorf("submit write came too soon after body: gap=%v, want >= %v", gap, delay/2) + } +} + +func TestAutoInject_skipped_when_process_id_missing(t *testing.T) { + mgr := newTestManager() + runner := &fakePassthroughRunner{} + + exec := newAutoInjectExecution("hello") + exec.PassthroughProcessID = "" + + mgr.autoInjectInitialPromptWith(runner, exec, agents.PassthroughConfig{ + AutoInjectPrompt: true, + SubmitSequence: "\r", + }) + + if runner.writeCalled { + t.Fatalf("expected no stdin write when PassthroughProcessID empty, got data=%q", runner.writtenData) + } +} diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_passthrough_command_test.go b/apps/backend/internal/agent/runtime/lifecycle/manager_passthrough_command_test.go new file mode 100644 index 0000000000..c3690131e0 --- /dev/null +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_passthrough_command_test.go @@ -0,0 +1,57 @@ +package lifecycle + +import ( + "testing" + + "github.com/kandev/kandev/internal/agent/agents" +) + +// TestPromptForPassthroughCommand asserts the gating used by passthroughAgentCommand: +// AutoInjectPrompt + empty PromptFlag suppresses the positional prompt arg, so the +// description is delivered exclusively via PTY stdin in autoInjectInitialPrompt. +// Without this guard, BuildPassthroughCommand would append the description as a +// positional arg and put Claude (and similar TUIs) into non-interactive `-p` mode. +func TestPromptForPassthroughCommand(t *testing.T) { + tests := []struct { + name string + pt agents.PassthroughConfig + desc string + want string + }{ + { + name: "auto-inject suppresses prompt arg", + pt: agents.PassthroughConfig{AutoInjectPrompt: true}, + desc: "refactor cron handler", + want: "", + }, + { + name: "auto-inject off keeps prompt", + pt: agents.PassthroughConfig{AutoInjectPrompt: false}, + desc: "refactor cron handler", + want: "refactor cron handler", + }, + { + name: "auto-inject with explicit PromptFlag keeps prompt — flag delivery wins", + pt: agents.PassthroughConfig{ + AutoInjectPrompt: true, + PromptFlag: agents.NewParam("--prompt", "{prompt}"), + }, + desc: "refactor cron handler", + want: "refactor cron handler", + }, + { + name: "empty description always empty", + pt: agents.PassthroughConfig{AutoInjectPrompt: true}, + desc: "", + want: "", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := promptForPassthroughCommand(tc.pt, tc.desc) + if got != tc.want { + t.Errorf("promptForPassthroughCommand = %q, want %q", got, tc.want) + } + }) + } +} diff --git a/apps/backend/internal/agent/settings/controller/agent_discovery.go b/apps/backend/internal/agent/settings/controller/agent_discovery.go index 1ea3f3c345..37fec162c7 100644 --- a/apps/backend/internal/agent/settings/controller/agent_discovery.go +++ b/apps/backend/internal/agent/settings/controller/agent_discovery.go @@ -119,9 +119,11 @@ func (c *Controller) buildAvailableAgentDTO(ctx context.Context, ag agents.Agent if ptAgent, ok := ag.(agents.PassthroughAgent); ok { pt := ptAgent.PassthroughConfig() passthroughConfig = &dto.PassthroughConfigDTO{ - Supported: pt.Supported, - Label: pt.Label, - Description: pt.Description, + Supported: pt.Supported, + Label: pt.Label, + Description: pt.Description, + AutoInjectPrompt: pt.AutoInjectPrompt, + SubmitSequence: pt.SubmitSequence, } } diff --git a/apps/backend/internal/agent/settings/dto/dto.go b/apps/backend/internal/agent/settings/dto/dto.go index 64940502b7..337565b1d1 100644 --- a/apps/backend/internal/agent/settings/dto/dto.go +++ b/apps/backend/internal/agent/settings/dto/dto.go @@ -146,9 +146,11 @@ type PermissionSettingDTO struct { } type PassthroughConfigDTO struct { - Supported bool `json:"supported"` - Label string `json:"label"` - Description string `json:"description"` + Supported bool `json:"supported"` + Label string `json:"label"` + Description string `json:"description"` + AutoInjectPrompt bool `json:"auto_inject_prompt"` + SubmitSequence string `json:"submit_sequence"` } type ToolStatusDTO struct { diff --git a/apps/backend/internal/agentctl/server/process/interactive_io.go b/apps/backend/internal/agentctl/server/process/interactive_io.go index b96f02fd92..e6a154883c 100644 --- a/apps/backend/internal/agentctl/server/process/interactive_io.go +++ b/apps/backend/internal/agentctl/server/process/interactive_io.go @@ -190,6 +190,11 @@ func (r *InteractiveRunner) resetIdleTimer(proc *interactiveProcess) { } func (r *InteractiveRunner) emitTurnComplete(proc *interactiveProcess) { + // Close firstIdleCh exactly once — used by callers that want to react to + // the very first idle window (e.g. auto-injecting the task prompt). + proc.firstIdleOnce.Do(func() { + close(proc.firstIdleCh) + }) if r.turnCompleteCallback != nil { r.turnCompleteCallback(proc.info.SessionID) } diff --git a/apps/backend/internal/agentctl/server/process/interactive_lifecycle.go b/apps/backend/internal/agentctl/server/process/interactive_lifecycle.go index dfe1a0aa37..746b01f960 100644 --- a/apps/backend/internal/agentctl/server/process/interactive_lifecycle.go +++ b/apps/backend/internal/agentctl/server/process/interactive_lifecycle.go @@ -62,6 +62,13 @@ type interactiveProcess struct { // race with stdin echo (which would otherwise duplicate the command in output). firstOutputOnce sync.Once firstOutputCh chan struct{} + + // firstIdleCh is closed the first time the idle detector fires for this + // process — i.e. the CLI has finished its startup output and is ready for + // input. Used by the lifecycle manager to auto-inject the task description + // at the right moment when AutoInjectPrompt is enabled. + firstIdleOnce sync.Once + firstIdleCh chan struct{} } // Start creates an interactive process entry and defers PTY creation until first resize. @@ -124,6 +131,7 @@ func (r *InteractiveRunner) Start(ctx context.Context, req InteractiveStartReque stopSignal: make(chan struct{}), waitDone: make(chan struct{}), firstOutputCh: make(chan struct{}), + firstIdleCh: make(chan struct{}), // Store start parameters for deferred initialization started: false, startCmd: req.Command, @@ -349,6 +357,11 @@ func (r *InteractiveRunner) Stop(ctx context.Context, processID string) error { return fmt.Errorf("process not found: %s", processID) } + // Unblock anyone waiting on first-idle — the process is going away. + proc.firstIdleOnce.Do(func() { + close(proc.firstIdleCh) + }) + // Signal output reader to exit proc.stopOnce.Do(func() { close(proc.stopSignal) @@ -486,6 +499,11 @@ func (r *InteractiveRunner) get(id string) (*interactiveProcess, bool) { // 3. Adding a timeout here would leave the process unreachable and create leaks func (r *InteractiveRunner) wait(proc *interactiveProcess) { defer close(proc.waitDone) + // Unblock first-idle waiters on exit so an early crash doesn't strand + // callers (e.g. autoInjectInitialPrompt) until their context times out. + // Callers can't distinguish "idle and ready" from "process exited" — the + // subsequent WriteStdin returns "process not found" which is logged. + defer proc.firstIdleOnce.Do(func() { close(proc.firstIdleCh) }) proc.mu.Lock() ptyHandle := proc.ptmx diff --git a/apps/backend/internal/agentctl/server/process/interactive_runner.go b/apps/backend/internal/agentctl/server/process/interactive_runner.go index 50efc2742b..768e519a16 100644 --- a/apps/backend/internal/agentctl/server/process/interactive_runner.go +++ b/apps/backend/internal/agentctl/server/process/interactive_runner.go @@ -7,6 +7,8 @@ package process import ( + "context" + "fmt" "io" "sync" "time" @@ -149,3 +151,21 @@ func (r *InteractiveRunner) SetStateCallback(cb AgentStateCallback) { func createStatusDetector() StatusDetector { return NewIdleDetector() } + +// WaitForFirstIdle blocks until the idle detector for processID fires for +// the first time, or ctx is canceled. Returns nil on first-idle, error otherwise. +// Returns an error immediately if processID is unknown. +func (r *InteractiveRunner) WaitForFirstIdle(ctx context.Context, processID string) error { + r.mu.RLock() + proc, ok := r.processes[processID] + r.mu.RUnlock() + if !ok { + return fmt.Errorf("process %q not found", processID) + } + select { + case <-proc.firstIdleCh: + return nil + case <-ctx.Done(): + return ctx.Err() + } +} diff --git a/apps/backend/internal/agentctl/server/process/interactive_runner_firstidle_test.go b/apps/backend/internal/agentctl/server/process/interactive_runner_firstidle_test.go new file mode 100644 index 0000000000..042422eb3b --- /dev/null +++ b/apps/backend/internal/agentctl/server/process/interactive_runner_firstidle_test.go @@ -0,0 +1,94 @@ +package process + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/kandev/kandev/internal/agentctl/types" +) + +// newRunnerWithProcess installs a bare interactiveProcess into the runner's +// map so tests can drive firstIdle semantics without spawning a real PTY. +func newRunnerWithProcess(t *testing.T, processID string) (*InteractiveRunner, *interactiveProcess) { + t.Helper() + log := newTestLogger(t) + runner := NewInteractiveRunner(nil, log, 2*1024*1024) + + proc := &interactiveProcess{ + info: InteractiveProcessInfo{ + ID: processID, + SessionID: "sess-" + processID, + Status: types.ProcessStatusRunning, + StartedAt: time.Now().UTC(), + UpdatedAt: time.Now().UTC(), + }, + stopSignal: make(chan struct{}), + waitDone: make(chan struct{}), + firstOutputCh: make(chan struct{}), + firstIdleCh: make(chan struct{}), + } + + runner.mu.Lock() + runner.processes[processID] = proc + runner.mu.Unlock() + return runner, proc +} + +func TestWaitForFirstIdle_fires_on_idle_event(t *testing.T) { + runner, proc := newRunnerWithProcess(t, "proc-fires") + + done := make(chan error, 1) + go func() { + done <- runner.WaitForFirstIdle(context.Background(), "proc-fires") + }() + + // emitTurnComplete is what fires the idle event in production. + go runner.emitTurnComplete(proc) + + select { + case err := <-done: + if err != nil { + t.Fatalf("WaitForFirstIdle returned error: %v", err) + } + case <-time.After(2 * time.Second): + t.Fatalf("WaitForFirstIdle did not return after idle fired") + } +} + +func TestWaitForFirstIdle_returns_ctx_error(t *testing.T) { + runner, _ := newRunnerWithProcess(t, "proc-ctx") + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + err := runner.WaitForFirstIdle(ctx, "proc-ctx") + if !errors.Is(err, context.Canceled) { + t.Fatalf("WaitForFirstIdle err = %v, want context.Canceled", err) + } +} + +func TestWaitForFirstIdle_unknown_process_errors(t *testing.T) { + log := newTestLogger(t) + runner := NewInteractiveRunner(nil, log, 2*1024*1024) + + err := runner.WaitForFirstIdle(context.Background(), "does-not-exist") + if err == nil { + t.Fatalf("WaitForFirstIdle returned nil error for unknown process") + } +} + +func TestEmitTurnComplete_closes_firstIdle_only_once(t *testing.T) { + runner, proc := newRunnerWithProcess(t, "proc-once") + + runner.emitTurnComplete(proc) + runner.emitTurnComplete(proc) // second call must not panic on double close + + select { + case <-proc.firstIdleCh: + // ok + default: + t.Fatalf("firstIdleCh was not closed after emitTurnComplete") + } +} diff --git a/apps/backend/internal/integration/simulated_agent_manager_test.go b/apps/backend/internal/integration/simulated_agent_manager_test.go index 2da67a8bf7..61907f8c89 100644 --- a/apps/backend/internal/integration/simulated_agent_manager_test.go +++ b/apps/backend/internal/integration/simulated_agent_manager_test.go @@ -11,6 +11,7 @@ import ( "github.com/google/uuid" "go.uber.org/zap" + "github.com/kandev/kandev/internal/agent/agents" "github.com/kandev/kandev/internal/agent/runtime/agentctl" "github.com/kandev/kandev/internal/agent/runtime/lifecycle" "github.com/kandev/kandev/internal/agentctl/types/streams" @@ -491,6 +492,9 @@ func (s *SimulatedAgentManagerClient) IsPassthroughSession(ctx context.Context, func (s *SimulatedAgentManagerClient) WritePassthroughStdin(_ context.Context, _ string, _ string) error { return nil } +func (s *SimulatedAgentManagerClient) ResolvePassthroughConfig(_ context.Context, _ string) (agents.PassthroughConfig, error) { + return agents.PassthroughConfig{}, nil +} func (s *SimulatedAgentManagerClient) MarkPassthroughRunning(_ string) error { return nil } diff --git a/apps/backend/internal/orchestrator/event_handlers_test.go b/apps/backend/internal/orchestrator/event_handlers_test.go index 90a7d2d758..8543a4d744 100644 --- a/apps/backend/internal/orchestrator/event_handlers_test.go +++ b/apps/backend/internal/orchestrator/event_handlers_test.go @@ -13,6 +13,7 @@ import ( "github.com/jmoiron/sqlx" _ "github.com/mattn/go-sqlite3" + "github.com/kandev/kandev/internal/agent/agents" "github.com/kandev/kandev/internal/agent/runtime/agentctl" "github.com/kandev/kandev/internal/agent/runtime/lifecycle" "github.com/kandev/kandev/internal/agentctl/types/streams" @@ -148,6 +149,12 @@ type mockAgentManager struct { markPassthroughCalls []string // session IDs markPassthroughErr error + // Passthrough config resolution. When zero-valued and isPassthrough is true, + // the mock returns a default config with SubmitSequence == "\r". + passthroughConfig agents.PassthroughConfig + passthroughConfigSet bool + passthroughConfigErr error + // Optional override for GetExecutionIDForSession. When unset, the default // implementation reads from repoForExecutionLookup if provided so tests that // seed an executors_running row don't also have to mock this function. @@ -291,6 +298,20 @@ func (m *mockAgentManager) WritePassthroughStdin(_ context.Context, sessionID st m.passthroughStdinCalls = append(m.passthroughStdinCalls, passthroughStdinCall{SessionID: sessionID, Data: data}) return m.passthroughStdinErr } +func (m *mockAgentManager) ResolvePassthroughConfig(_ context.Context, _ string) (agents.PassthroughConfig, error) { + m.mu.Lock() + defer m.mu.Unlock() + if m.passthroughConfigErr != nil { + return agents.PassthroughConfig{}, m.passthroughConfigErr + } + if m.passthroughConfigSet { + return m.passthroughConfig, nil + } + if !m.isPassthrough { + return agents.PassthroughConfig{Supported: false}, nil + } + return agents.PassthroughConfig{Supported: true, SubmitSequence: "\r"}, nil +} func (m *mockAgentManager) MarkPassthroughRunning(sessionID string) error { m.mu.Lock() defer m.mu.Unlock() @@ -1042,9 +1063,12 @@ func TestDeliverPassthroughPrompt(t *testing.T) { agentMgr.mu.Lock() defer agentMgr.mu.Unlock() - // Should not call markPassthroughRunning when stdin write fails - if len(agentMgr.markPassthroughCalls) != 0 { - t.Errorf("markPassthroughRunning should not be called when stdin fails, got %d calls", len(agentMgr.markPassthroughCalls)) + // markPassthroughRunning now fires BEFORE any writes so concurrent + // PromptTask calls are blocked during the inter-chunk SubmitDelay + // window (Greptile P1). Expect exactly one mark call even when the + // subsequent write fails. + if len(agentMgr.markPassthroughCalls) != 1 { + t.Errorf("markPassthroughRunning should fire once before the write; got %d calls", len(agentMgr.markPassthroughCalls)) } }) } diff --git a/apps/backend/internal/orchestrator/event_handlers_workflow.go b/apps/backend/internal/orchestrator/event_handlers_workflow.go index 1eceb2d070..8490bd0a44 100644 --- a/apps/backend/internal/orchestrator/event_handlers_workflow.go +++ b/apps/backend/internal/orchestrator/event_handlers_workflow.go @@ -8,6 +8,7 @@ import ( "go.uber.org/zap" + "github.com/kandev/kandev/internal/agent/agents" "github.com/kandev/kandev/internal/events" "github.com/kandev/kandev/internal/events/bus" "github.com/kandev/kandev/internal/orchestrator/executor" @@ -1057,17 +1058,39 @@ func (s *Service) drainQueuedMessageAfterTransition(ctx context.Context, session } // deliverPassthroughPrompt writes a prompt to PTY stdin and marks the session as running. -// Appends \r (carriage return) to simulate pressing Enter — TUI agents in raw terminal mode -// expect CR, not LF, as the submit key. Returns an error only if writing fails. +// Uses the per-agent PlanPassthroughStdinChunks so Claude's inter-chunk SubmitDelay is +// honored here too (queued / workflow-auto-start path); other agents stay on the single +// atomic write. Falls back to the simple "\r" append if config resolution fails so a +// transient lookup error never silently swallows the prompt. func (s *Service) deliverPassthroughPrompt(ctx context.Context, sessionID, content string) error { - if err := s.agentManager.WritePassthroughStdin(ctx, sessionID, content+"\r"); err != nil { - return fmt.Errorf("write to passthrough stdin: %w", err) + pt, cfgErr := s.agentManager.ResolvePassthroughConfig(ctx, sessionID) + if cfgErr != nil { + s.logger.Warn("failed to resolve passthrough config, falling back to \\r submit", + zap.String("session_id", sessionID), + zap.Error(cfgErr)) } + // Mark RUNNING before any writes so concurrent PromptTask / queued-message + // delivery is blocked by checkSessionPromptable during the inter-chunk + // SubmitDelay window (150ms for Claude). Mark error is non-fatal. if err := s.agentManager.MarkPassthroughRunning(sessionID); err != nil { - s.logger.Warn("failed to mark passthrough as running", + s.logger.Warn("failed to mark passthrough as running before prompt", zap.String("session_id", sessionID), zap.Error(err)) } + if cfgErr != nil { + if err := s.agentManager.WritePassthroughStdin(ctx, sessionID, content+"\r"); err != nil { + return fmt.Errorf("write to passthrough stdin: %w", err) + } + return nil + } + for _, chunk := range agents.PlanPassthroughStdinChunks(content, pt) { + if chunk.DelayBefore > 0 { + time.Sleep(chunk.DelayBefore) + } + if err := s.agentManager.WritePassthroughStdin(ctx, sessionID, chunk.Data); err != nil { + return fmt.Errorf("write to passthrough stdin: %w", err) + } + } return nil } @@ -1167,8 +1190,10 @@ func (s *Service) autoStartStepPrompt( // (HasKandevContext guard) so the pre-wrap doesn't double. // The HasKandevContext check on `prompt` also guards against any future // caller that ever pre-wraps before reaching here (none today). + // Passthrough sessions skip the wrap: the prompt is typed straight into + // the agent CLI's TTY and the user sees it verbatim. recordedPrompt := prompt - if session.State == models.TaskSessionStateCreated && (prompt != "" || len(attachments) > 0) && !sysprompt.HasKandevContext(prompt) { + if session.State == models.TaskSessionStateCreated && !session.IsPassthrough && (prompt != "" || len(attachments) > 0) && !sysprompt.HasKandevContext(prompt) { recordedPrompt = sysprompt.InjectKandevContext(taskID, sessionID, prompt) } s.recordAutoStartMessage(ctx, taskID, sessionID, recordedPrompt, planMode, origin) diff --git a/apps/backend/internal/orchestrator/executor/executor.go b/apps/backend/internal/orchestrator/executor/executor.go index 46bc82cdf5..7c83cbb3a6 100644 --- a/apps/backend/internal/orchestrator/executor/executor.go +++ b/apps/backend/internal/orchestrator/executor/executor.go @@ -7,6 +7,7 @@ import ( "sync" "time" + "github.com/kandev/kandev/internal/agent/agents" "github.com/kandev/kandev/internal/agent/runtime/agentctl" "github.com/kandev/kandev/internal/agent/runtime/lifecycle" "github.com/kandev/kandev/internal/agentctl/types/streams" @@ -151,6 +152,11 @@ type AgentManagerClient interface { // WritePassthroughStdin writes data to the agent's PTY stdin for passthrough sessions. WritePassthroughStdin(ctx context.Context, sessionID string, data string) error + // ResolvePassthroughConfig returns the resolved PassthroughConfig for a session's agent. + // Used by the orchestrator to route chat-compose prompts and Stop button presses into + // the agent's PTY stdin (with the correct submit sequence) instead of through ACP. + ResolvePassthroughConfig(ctx context.Context, sessionID string) (agents.PassthroughConfig, error) + // MarkPassthroughRunning marks a passthrough execution as running. MarkPassthroughRunning(sessionID string) error diff --git a/apps/backend/internal/orchestrator/executor/executor_interaction.go b/apps/backend/internal/orchestrator/executor/executor_interaction.go index 0aee6bd9ce..e8ea843905 100644 --- a/apps/backend/internal/orchestrator/executor/executor_interaction.go +++ b/apps/backend/internal/orchestrator/executor/executor_interaction.go @@ -6,6 +6,7 @@ import ( "fmt" "time" + "github.com/kandev/kandev/internal/agent/agents" "github.com/kandev/kandev/internal/agent/runtime/lifecycle" "github.com/kandev/kandev/internal/task/models" v1 "github.com/kandev/kandev/pkg/api/v1" @@ -117,15 +118,11 @@ func (e *Executor) StopByTaskID(ctx context.Context, taskID string, reason strin return nil } -// passthroughSubmitSequence is appended to a passthrough prompt before writing -// to PTY stdin. "\r" submits the current line in every TUI agent CLI currently -// supported in passthrough mode (Claude, Codex, OpenCode, Cursor, Auggie, …). -// If a future agent needs a different sequence, lift this into a per-agent -// PassthroughConfig field — see the plan for issue #989. -const ( - passthroughSubmitSequence = "\r" - stopReasonPassthrough = "passthrough_dispatched" -) +// stopReasonPassthrough is the StopReason returned by Executor.Prompt when a +// passthrough session's prompt is dispatched to PTY stdin (no ACP turn to +// observe). The submit sequence is resolved per-agent via ResolvePassthroughConfig +// — see promptPassthrough below. +const stopReasonPassthrough = "passthrough_dispatched" // Prompt sends a follow-up prompt to a running agent for a task // Returns PromptResult indicating if the agent needs input @@ -182,6 +179,10 @@ func (e *Executor) Prompt(ctx context.Context, taskID, sessionID string, prompt // supplies any, we log a warning so an operator can see they were dropped // (image-only messages are rejected outright since there is nothing to write). // +// The submit sequence is resolved per-agent via ResolvePassthroughConfig — most +// TUI CLIs use "\r" but the config field lets a future agent override it +// without touching this code path. +// // A WritePassthroughStdin failure (no live PTY, runner unavailable) is returned // as an error so Service.handlePromptError can revert session state and surface // the failure to the user. A MarkPassthroughRunning failure is non-fatal — the @@ -196,15 +197,31 @@ func (e *Executor) promptPassthrough(ctx context.Context, taskID, sessionID, pro zap.String("session_id", sessionID), zap.Int("attachments_count", len(attachments))) } - if err := e.agentManager.WritePassthroughStdin(ctx, sessionID, prompt+passthroughSubmitSequence); err != nil { - return nil, fmt.Errorf("failed to write to passthrough stdin: %w", err) + pt, err := e.agentManager.ResolvePassthroughConfig(ctx, sessionID) + if err != nil { + return nil, fmt.Errorf("resolve passthrough config: %w", err) } + // Mark RUNNING before the chunk loop so concurrent PromptTask calls are + // blocked by checkSessionPromptable during the inter-chunk SubmitDelay + // window (150ms for Claude). Otherwise a rapid double-send or workflow + // event firing in parallel passes the WAITING_FOR_INPUT guard and writes + // a second prompt onto the same PTY stdin mid-submit. Mark-error stays + // non-fatal — at worst we miss the AgentRunning event but the prompt + // still gets through. if err := e.agentManager.MarkPassthroughRunning(sessionID); err != nil { - e.logger.Warn("failed to mark passthrough as running after prompt; data already in PTY", + e.logger.Warn("failed to mark passthrough as running before prompt; concurrent send window is open", zap.String("task_id", taskID), zap.String("session_id", sessionID), zap.Error(err)) } + for _, chunk := range agents.PlanPassthroughStdinChunks(prompt, pt) { + if chunk.DelayBefore > 0 { + time.Sleep(chunk.DelayBefore) + } + if err := e.agentManager.WritePassthroughStdin(ctx, sessionID, chunk.Data); err != nil { + return nil, fmt.Errorf("failed to write to passthrough stdin: %w", err) + } + } return &PromptResult{StopReason: stopReasonPassthrough}, nil } diff --git a/apps/backend/internal/orchestrator/executor/executor_mocks_test.go b/apps/backend/internal/orchestrator/executor/executor_mocks_test.go index b22546a794..2210b55ef4 100644 --- a/apps/backend/internal/orchestrator/executor/executor_mocks_test.go +++ b/apps/backend/internal/orchestrator/executor/executor_mocks_test.go @@ -6,6 +6,7 @@ import ( "sync" "testing" + "github.com/kandev/kandev/internal/agent/agents" agentdto "github.com/kandev/kandev/internal/agent/dto" "github.com/kandev/kandev/internal/agent/runtime/agentctl" "github.com/kandev/kandev/internal/agentctl/types/streams" @@ -28,6 +29,7 @@ type mockAgentManager struct { isPassthroughSessionFunc func(ctx context.Context, sessionID string) bool writePassthroughStdinFunc func(ctx context.Context, sessionID, data string) error markPassthroughRunningFunc func(sessionID string) error + resolvePassthroughConfigFunc func(ctx context.Context, sessionID string) (agents.PassthroughConfig, error) launchAgentCallCount int cleanupStaleExecutionCallCount int isAgentRunningForSessionCallArgs []string @@ -142,6 +144,17 @@ func (m *mockAgentManager) WritePassthroughStdin(ctx context.Context, sessionID } return nil } +func (m *mockAgentManager) ResolvePassthroughConfig(ctx context.Context, sessionID string) (agents.PassthroughConfig, error) { + if m.resolvePassthroughConfigFunc != nil { + return m.resolvePassthroughConfigFunc(ctx, sessionID) + } + // Default to a sensible passthrough config (SubmitSequence "\r") when the + // session is in passthrough mode, so most tests don't need to override. + if m.isPassthroughSessionFunc != nil && m.isPassthroughSessionFunc(ctx, sessionID) { + return agents.PassthroughConfig{Supported: true, SubmitSequence: "\r"}, nil + } + return agents.PassthroughConfig{}, nil +} func (m *mockAgentManager) MarkPassthroughRunning(sessionID string) error { m.markPassthroughRunningCalls = append(m.markPassthroughRunningCalls, sessionID) if m.markPassthroughRunningFunc != nil { diff --git a/apps/backend/internal/orchestrator/executor/executor_passthrough_prompt_test.go b/apps/backend/internal/orchestrator/executor/executor_passthrough_prompt_test.go index 5ae33988c1..4a0b2ffadf 100644 --- a/apps/backend/internal/orchestrator/executor/executor_passthrough_prompt_test.go +++ b/apps/backend/internal/orchestrator/executor/executor_passthrough_prompt_test.go @@ -5,7 +5,9 @@ import ( "errors" "strings" "testing" + "time" + "github.com/kandev/kandev/internal/agent/agents" "github.com/kandev/kandev/internal/task/models" v1 "github.com/kandev/kandev/pkg/api/v1" ) @@ -51,7 +53,9 @@ func TestExecutor_Prompt_PassthroughWritesStdin(t *testing.T) { if call.SessionID != "sess-1" { t.Errorf("WritePassthroughStdin session = %q, want sess-1", call.SessionID) } - wantData := "hello agent" + passthroughSubmitSequence + // Mock's ResolvePassthroughConfig returns SubmitSequence "\r" by default + // when isPassthroughSessionFunc reports the session is passthrough. + wantData := "hello agent\r" if call.Data != wantData { t.Errorf("WritePassthroughStdin data = %q, want %q", call.Data, wantData) } @@ -91,10 +95,14 @@ func TestExecutor_Prompt_PassthroughWriteErrorReturnsError(t *testing.T) { if result != nil { t.Errorf("expected nil result on error, got %+v", result) } - // MarkPassthroughRunning must not be called when the write fails — otherwise - // the UI would flash "running" for a prompt the agent never received. - if got := len(agentManager.markPassthroughRunningCalls); got != 0 { - t.Errorf("MarkPassthroughRunning must not be called when stdin write fails; got %d call(s)", got) + // MarkPassthroughRunning is now called BEFORE the chunk loop so concurrent + // PromptTask calls are blocked during the inter-chunk SubmitDelay window + // (Greptile P1). The UI may briefly show "running" for a prompt that then + // fails to deliver — preferable to a second prompt racing into the PTY + // mid-submit. Expect exactly one MarkPassthroughRunning call even when the + // subsequent write fails. + if got := len(agentManager.markPassthroughRunningCalls); got != 1 { + t.Errorf("MarkPassthroughRunning should be called once before the write; got %d call(s)", got) } } @@ -180,12 +188,84 @@ func TestExecutor_Prompt_PassthroughPreservesUnicodeAndMultiline(t *testing.T) { if _, err := exec.Prompt(context.Background(), "task-1", "sess-1", prompt, nil, false); err != nil { t.Fatalf("Prompt returned error: %v", err) } + cfg := agents.PassthroughConfig{SubmitSequence: "\r"} + want := agents.BuildPassthroughPayload(prompt, cfg) + if len(agentManager.writePassthroughStdinCalls) != 1 { + t.Fatalf("expected 1 atomic WritePassthroughStdin call for multiline, got %d", len(agentManager.writePassthroughStdinCalls)) + } + if agentManager.writePassthroughStdinCalls[0].Data != want { + t.Errorf("PTY payload = %q, want %q", agentManager.writePassthroughStdinCalls[0].Data, want) + } +} + +// TestExecutor_Prompt_PassthroughHonoursDynamicSubmitSequence verifies the +// PTY submit suffix is taken from PassthroughConfig (per-agent) rather than +// hardcoded. A future TUI that submits on "\n" or "\x0d\x0a" plugs in via +// PassthroughConfig.SubmitSequence with no code change here. +func TestExecutor_Prompt_PassthroughHonoursDynamicSubmitSequence(t *testing.T) { + repo := newMockRepository() + agentManager := &mockAgentManager{ + isPassthroughSessionFunc: func(_ context.Context, _ string) bool { return true }, + resolvePassthroughConfigFunc: func(_ context.Context, _ string) (agents.PassthroughConfig, error) { + return agents.PassthroughConfig{Supported: true, SubmitSequence: "\n"}, nil + }, + } + seedPassthroughSession(t, repo, agentManager, "task-1", "sess-1", "exec-1") + exec := newTestExecutor(t, agentManager, repo) + + if _, err := exec.Prompt(context.Background(), "task-1", "sess-1", "hi", nil, false); err != nil { + t.Fatalf("Prompt returned error: %v", err) + } if got := len(agentManager.writePassthroughStdinCalls); got != 1 { t.Fatalf("expected 1 WritePassthroughStdin call, got %d", got) } - want := prompt + "\r" + want := "hi\n" if agentManager.writePassthroughStdinCalls[0].Data != want { - t.Errorf("PTY payload = %q, want %q", agentManager.writePassthroughStdinCalls[0].Data, want) + t.Errorf("PTY payload = %q, want %q (dynamic SubmitSequence not applied)", agentManager.writePassthroughStdinCalls[0].Data, want) + } +} + +// TestExecutor_Prompt_PassthroughSubmitDelaySplitsWrites verifies the compose-box +// follow-up path (Executor.Prompt → promptPassthrough) honors SubmitDelay: the +// body and submit byte must arrive as two separate WritePassthroughStdin calls +// with the body first and the submit ("\r") second, so Claude's Ink TUI sees the +// trailing Enter as a discrete keystroke instead of absorbing it into a paste +// burst. The auto-inject path is covered by manager_passthrough_autoinject_test.go; +// this guards the executor path against regressing back to a single atomic write. +func TestExecutor_Prompt_PassthroughSubmitDelaySplitsWrites(t *testing.T) { + repo := newMockRepository() + agentManager := &mockAgentManager{ + isPassthroughSessionFunc: func(_ context.Context, _ string) bool { return true }, + resolvePassthroughConfigFunc: func(_ context.Context, _ string) (agents.PassthroughConfig, error) { + return agents.PassthroughConfig{ + Supported: true, + SubmitSequence: "\r", + DisableBracketedPaste: true, + SubmitDelay: 20 * time.Millisecond, + }, nil + }, + } + seedPassthroughSession(t, repo, agentManager, "task-1", "sess-1", "exec-1") + exec := newTestExecutor(t, agentManager, repo) + + if _, err := exec.Prompt(context.Background(), "task-1", "sess-1", "hello agent", nil, false); err != nil { + t.Fatalf("Prompt returned error: %v", err) + } + + if got := len(agentManager.writePassthroughStdinCalls); got != 2 { + t.Fatalf("expected 2 WritePassthroughStdin calls (body, submit), got %d: %#v", got, agentManager.writePassthroughStdinCalls) + } + if agentManager.writePassthroughStdinCalls[0].Data != "hello agent" { + t.Errorf("body write Data = %q, want %q", agentManager.writePassthroughStdinCalls[0].Data, "hello agent") + } + if agentManager.writePassthroughStdinCalls[1].Data != "\r" { + t.Errorf("submit write Data = %q, want %q", agentManager.writePassthroughStdinCalls[1].Data, "\r") + } + // MarkPassthroughRunning must still fire exactly once after both writes — the + // session should flip to RUNNING for the UI even though the two writes were + // split. Regressing this would leave the spinner stuck on the composer. + if got := len(agentManager.markPassthroughRunningCalls); got != 1 { + t.Errorf("expected 1 MarkPassthroughRunning call, got %d", got) } } diff --git a/apps/backend/internal/orchestrator/scheduler/scheduler_test.go b/apps/backend/internal/orchestrator/scheduler/scheduler_test.go index 433363310e..341ef7f962 100644 --- a/apps/backend/internal/orchestrator/scheduler/scheduler_test.go +++ b/apps/backend/internal/orchestrator/scheduler/scheduler_test.go @@ -10,6 +10,7 @@ import ( "time" "github.com/jmoiron/sqlx" + "github.com/kandev/kandev/internal/agent/agents" "github.com/kandev/kandev/internal/agent/runtime/agentctl" "github.com/kandev/kandev/internal/agentctl/types/streams" "github.com/kandev/kandev/internal/common/logger" @@ -104,6 +105,9 @@ func (m *mockAgentManager) IsPassthroughSession(ctx context.Context, sessionID s func (m *mockAgentManager) WritePassthroughStdin(_ context.Context, _ string, _ string) error { return nil } +func (m *mockAgentManager) ResolvePassthroughConfig(_ context.Context, _ string) (agents.PassthroughConfig, error) { + return agents.PassthroughConfig{}, nil +} func (m *mockAgentManager) MarkPassthroughRunning(_ string) error { return nil } diff --git a/apps/backend/internal/orchestrator/task_operations.go b/apps/backend/internal/orchestrator/task_operations.go index 7c605b416f..8a2ba99936 100644 --- a/apps/backend/internal/orchestrator/task_operations.go +++ b/apps/backend/internal/orchestrator/task_operations.go @@ -356,7 +356,10 @@ func (s *Service) StartCreatedSession(ctx context.Context, taskID, sessionID, ag // Message.ToAPI strips for display). Idempotent — upstream call sites that // record the user message themselves (wsAddMessage on CREATED sessions) // wrap first, and the HasKandevContext guard prevents a second wrap here. - if (effectivePrompt != "" || len(attachments) > 0) && !sysprompt.HasKandevContext(effectivePrompt) { + // Passthrough profiles skip the wrap: the prompt is typed straight into the + // agent CLI's TTY and the user sees it verbatim — they don't want a wall of + // MCP-tool boilerplate prepended to "hello". + if (effectivePrompt != "" || len(attachments) > 0) && !sysprompt.HasKandevContext(effectivePrompt) && !session.IsPassthrough { effectivePrompt = sysprompt.InjectKandevContext(taskID, sessionID, effectivePrompt) } @@ -568,7 +571,15 @@ func (s *Service) startTask(ctx context.Context, taskID string, agentProfileID s // recordAutoStartMessage) wrap before recording the user message so the DB // row carries the block; the HasKandevContext guard makes this orchestrator // pass a no-op in those cases instead of double-wrapping. - if (effectivePrompt != "" || len(attachments) > 0) && !sysprompt.HasKandevContext(effectivePrompt) { + // Passthrough sessions skip the wrap: the prompt is typed straight into the + // agent CLI's TTY and the user sees it verbatim — they don't want a wall of + // MCP-tool boilerplate prepended to "hello". Use the session snapshot, not a + // live profile lookup, so a mid-run profile edit cannot change wrap behavior. + skipKandevMCPWrap := false + if launchSession, sessErr := s.repo.GetTaskSession(ctx, sessionID); sessErr == nil { + skipKandevMCPWrap = launchSession.IsPassthrough + } + if (effectivePrompt != "" || len(attachments) > 0) && !sysprompt.HasKandevContext(effectivePrompt) && !skipKandevMCPWrap { effectivePrompt = sysprompt.InjectKandevContext(task.ID, sessionID, effectivePrompt) } @@ -1861,6 +1872,17 @@ func (s *Service) PromptTask(ctx context.Context, taskID, sessionID string, prom return nil, err } + // Inject config context for config-mode sessions (dedicated settings chat, not plan mode) + effectivePrompt := prompt + if cm, ok := session.Metadata["config_mode"].(bool); ok && cm { + effectivePrompt = sysprompt.InjectConfigContext(sessionID, prompt) + } + + // Inject plan mode prefix for follow-up messages in plan mode sessions. + if planMode { + effectivePrompt = sysprompt.InjectPlanMode(effectivePrompt) + } + // Ensure the agent process is actually running. After a lazy backend restart, // the session may be in WAITING_FOR_INPUT but no agent process exists yet. if err := s.ensureSessionRunning(ctx, sessionID, session); err != nil { @@ -1876,17 +1898,14 @@ func (s *Service) PromptTask(ctx context.Context, taskID, sessionID string, prom // freshly-persisted AgentExecutionID. if reloaded, err := s.repo.GetTaskSession(ctx, sessionID); err == nil && reloaded != nil { session = reloaded - } - - // Inject config context for config-mode sessions (dedicated settings chat, not plan mode) - effectivePrompt := prompt - if cm, ok := session.Metadata["config_mode"].(bool); ok && cm { - effectivePrompt = sysprompt.InjectConfigContext(sessionID, prompt) - } - - // Inject plan mode prefix for follow-up messages in plan mode sessions. - if planMode { - effectivePrompt = sysprompt.InjectPlanMode(effectivePrompt) + // Re-apply transforms in case metadata changed during ensureSessionRunning. + effectivePrompt = prompt + if cm, ok := session.Metadata["config_mode"].(bool); ok && cm { + effectivePrompt = sysprompt.InjectConfigContext(sessionID, prompt) + } + if planMode { + effectivePrompt = sysprompt.InjectPlanMode(effectivePrompt) + } } // Check if model switching is requested @@ -2107,6 +2126,9 @@ func (s *Service) CancelAgent(ctx context.Context, sessionID string) error { // just to host the cancel message. cancelTurnID := s.getActiveTurnID(sessionID) + // The agent manager routes the cancel to the right signal: ACP cancel for + // regular sessions, Ctrl-C via PTY stdin for passthrough sessions. Service + // stays protocol-agnostic; the seam is in lifecycle.Manager.CancelAgentBySessionID. if err := s.agentManager.CancelAgent(ctx, sessionID); err != nil { switch { case errors.Is(err, lifecycle.ErrNoExecutionForSession): diff --git a/apps/backend/internal/orchestrator/task_operations_passthrough_test.go b/apps/backend/internal/orchestrator/task_operations_passthrough_test.go index c7036281a5..b0574994e6 100644 --- a/apps/backend/internal/orchestrator/task_operations_passthrough_test.go +++ b/apps/backend/internal/orchestrator/task_operations_passthrough_test.go @@ -128,12 +128,15 @@ func TestPromptTask_PassthroughWriteFailureRevertsSession(t *testing.T) { t.Errorf("expected session reverted to WAITING_FOR_INPUT, got %q", updated.State) } - // MarkPassthroughRunning must NOT be called when the write failed — otherwise - // the UI would flash "running" for a prompt the agent never received. + // MarkPassthroughRunning now fires BEFORE the chunk loop so concurrent + // PromptTask calls are blocked during the inter-chunk SubmitDelay window + // (Greptile P1). When the subsequent write fails the session is still + // reverted to WAITING_FOR_INPUT by handlePromptError above, so the brief + // RUNNING flash is bounded and the UI re-enables the composer. agentMgr.mu.Lock() markCount := len(agentMgr.markPassthroughCalls) agentMgr.mu.Unlock() - if markCount != 0 { - t.Errorf("MarkPassthroughRunning must not fire on PTY write failure; got %d call(s)", markCount) + if markCount != 1 { + t.Errorf("MarkPassthroughRunning should fire once before the write; got %d call(s)", markCount) } } diff --git a/apps/backend/internal/task/handlers/message_handlers.go b/apps/backend/internal/task/handlers/message_handlers.go index 88a055314a..ac5425c5d0 100644 --- a/apps/backend/internal/task/handlers/message_handlers.go +++ b/apps/backend/internal/task/handlers/message_handlers.go @@ -229,8 +229,11 @@ func (h *MessageHandlers) wsAddMessage(ctx context.Context, msg *ws.Message) (*w // block and bypass server-side injection of the canonical task/session/ // tool context. Wrap unconditionally; the orchestrator's own guard sees // our wrap downstream and skips its second pass. + // Passthrough sessions skip the wrap: the prompt is typed straight into + // the agent CLI's TTY and the user sees it verbatim — they don't want a + // wall of MCP-tool boilerplate prepended to "hello". storedContent := req.Content - if isCreatedSession && (req.Content != "" || len(req.Attachments) > 0) { + if isCreatedSession && !sessionResp.Session.IsPassthrough && (req.Content != "" || len(req.Attachments) > 0) { storedContent = sysprompt.InjectKandevContext(req.TaskID, req.TaskSessionID, req.Content) req.Content = storedContent } diff --git a/apps/web/app/settings/agents/[agentId]/agent-save-helpers.test.ts b/apps/web/app/settings/agents/[agentId]/agent-save-helpers.test.ts index 0c74712df7..fae6ec2f57 100644 --- a/apps/web/app/settings/agents/[agentId]/agent-save-helpers.test.ts +++ b/apps/web/app/settings/agents/[agentId]/agent-save-helpers.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect } from "vitest"; import { agentProfileId as toAgentProfileId, type AgentProfile } from "@/lib/types/http"; -import { isProfileDirty, type DraftProfile } from "./agent-save-helpers"; +import { isProfileDirty, toAgentProfilePatch, type DraftProfile } from "./agent-save-helpers"; +import type { ProfileFormData } from "@/components/settings/profile-form-fields"; const baseProfile: AgentProfile = { id: toAgentProfileId("p1"), @@ -23,6 +24,32 @@ const draftFrom = (saved: AgentProfile, overrides: Partial = {}): const ALLOW_ALL_TOOLS_FLAG = "--allow-all-tools"; +describe("toAgentProfilePatch", () => { + it("maps snake_case form keys to camelCase AgentProfile fields", () => { + const patch: Partial = { + name: "CLI", + model: "claude-sonnet", + mode: "default", + allow_indexing: true, + cli_passthrough: true, + cli_flags: [{ flag: ALLOW_ALL_TOOLS_FLAG, enabled: true, description: "" }], + }; + expect(toAgentProfilePatch(patch)).toEqual({ + name: "CLI", + model: "claude-sonnet", + mode: "default", + allowIndexing: true, + cliPassthrough: true, + cliFlags: [{ flag: ALLOW_ALL_TOOLS_FLAG, enabled: true, description: "" }], + }); + }); + + it("omits undefined keys so partial patches do not clobber unrelated fields", () => { + expect(toAgentProfilePatch({ cli_passthrough: false })).toEqual({ cliPassthrough: false }); + expect(toAgentProfilePatch({})).toEqual({}); + }); +}); + describe("isProfileDirty", () => { it("returns false when draft equals saved", () => { expect(isProfileDirty(draftFrom(baseProfile), baseProfile)).toBe(false); diff --git a/apps/web/app/settings/agents/[agentId]/agent-save-helpers.ts b/apps/web/app/settings/agents/[agentId]/agent-save-helpers.ts index 05397979f1..873d78c2af 100644 --- a/apps/web/app/settings/agents/[agentId]/agent-save-helpers.ts +++ b/apps/web/app/settings/agents/[agentId]/agent-save-helpers.ts @@ -16,6 +16,25 @@ import type { } from "@/lib/types/http"; import { permissionsToProfilePatch, arePermissionsDirty } from "@/lib/agent-permissions"; import { areCLIFlagsEqual } from "@/lib/cli-flags"; +import type { ProfileFormData } from "@/components/settings/profile-form-fields"; + +/** + * Translates a ProfileFormData patch (snake_case form keys) into a + * Partial (camelCase). Profiles in client state use the + * canonical camelCase AgentProfile shape, so without this translation + * patches like { cli_passthrough: true } would land as a new snake_case + * key and the camelCase reader would never see them. + */ +export function toAgentProfilePatch(patch: Partial): Partial { + const next: Partial = {}; + if (patch.name !== undefined) next.name = patch.name; + if (patch.model !== undefined) next.model = patch.model; + if (patch.mode !== undefined) next.mode = patch.mode; + if (patch.allow_indexing !== undefined) next.allowIndexing = patch.allow_indexing; + if (patch.cli_passthrough !== undefined) next.cliPassthrough = patch.cli_passthrough; + if (patch.cli_flags !== undefined) next.cliFlags = patch.cli_flags; + return next; +} function areEnvVarsEqual(a?: ProfileEnvVar[], b?: ProfileEnvVar[]): boolean { const left = a ?? []; diff --git a/apps/web/app/settings/agents/[agentId]/agent-setup-parts.tsx b/apps/web/app/settings/agents/[agentId]/agent-setup-parts.tsx index 05eeccc04f..9d58bba8b9 100644 --- a/apps/web/app/settings/agents/[agentId]/agent-setup-parts.tsx +++ b/apps/web/app/settings/agents/[agentId]/agent-setup-parts.tsx @@ -20,7 +20,7 @@ import { ProfileEnvVarsSection } from "@/components/settings/agent-profile-page" import { CustomCLIFlagsCard } from "@/components/settings/cli-flags-field"; import type { Agent, ModelConfig, PermissionSetting, PassthroughConfig } from "@/lib/types/http"; import { ProfileMcpConfigCard } from "./profile-mcp-config-card"; -import type { DraftProfile, DraftAgent } from "./agent-save-helpers"; +import { toAgentProfilePatch, type DraftProfile, type DraftAgent } from "./agent-save-helpers"; export type AgentHeaderProps = { displayName: string; @@ -124,7 +124,7 @@ export function ProfileCardItem({ cli_passthrough: profile.cliPassthrough ?? false, cli_flags: profile.cliFlags ?? [], }} - onChange={(patch) => onProfileChange(profile.id, patch)} + onChange={(patch) => onProfileChange(profile.id, toAgentProfilePatch(patch))} modelConfig={currentAgentModelConfig} permissionSettings={permissionSettings} passthroughConfig={passthroughConfig} diff --git a/apps/web/components/settings/agent-profile-page.tsx b/apps/web/components/settings/agent-profile-page.tsx index ba8e37bdea..a70d2750d0 100644 --- a/apps/web/components/settings/agent-profile-page.tsx +++ b/apps/web/components/settings/agent-profile-page.tsx @@ -12,6 +12,7 @@ import { Separator } from "@kandev/ui/separator"; import { useToast } from "@/components/toast-provider"; import { UnsavedChangesBadge, UnsavedSaveButton } from "@/components/settings/unsaved-indicator"; import { ProfileFormFields, type ProfileFormData } from "@/components/settings/profile-form-fields"; +import { toAgentProfilePatch } from "@/app/settings/agents/[agentId]/agent-save-helpers"; import { deleteAgentProfileAction, updateAgentProfileAction } from "@/app/actions/agents"; import type { ActiveSessionInfo } from "@/lib/types/agent-profile-errors"; import { @@ -171,17 +172,6 @@ function ProfileSettingsCard({ ); } -function toAgentProfilePatch(patch: Partial): Partial { - const next: Partial = {}; - if (patch.name !== undefined) next.name = patch.name; - if (patch.model !== undefined) next.model = patch.model; - if (patch.mode !== undefined) next.mode = patch.mode; - if (patch.allow_indexing !== undefined) next.allowIndexing = patch.allow_indexing; - if (patch.cli_passthrough !== undefined) next.cliPassthrough = patch.cli_passthrough; - if (patch.cli_flags !== undefined) next.cliFlags = patch.cli_flags; - return next; -} - function useSyncAgentsToStore() { const setSettingsAgents = useAppStore((state) => state.setSettingsAgents); const setAgentProfiles = useAppStore((state) => state.setAgentProfiles); diff --git a/apps/web/components/task-create-dialog-footer.test.ts b/apps/web/components/task-create-dialog-footer.test.ts index 21e8936837..a511bdc51e 100644 --- a/apps/web/components/task-create-dialog-footer.test.ts +++ b/apps/web/components/task-create-dialog-footer.test.ts @@ -23,7 +23,6 @@ function makeProps( isCreateMode: true, isEditMode: false, isTaskStarted: false, - isPassthroughProfile: false, isCreatingSession: false, isCreatingTask: false, hasTitle: true, @@ -155,13 +154,15 @@ describe("computeDisabledReason (default)", () => { ).toBe(REASON_DESCRIPTION); }); - it("does not flag missing description for passthrough profiles", () => { + it("requires description in session mode even for CLI/passthrough profiles", () => { + // CLI-mode parity: the backend now auto-injects the prompt into the CLI's + // stdin, so the prompt is required just like in ACP mode. expect( computeDisabledReason( - makeProps({ isSessionMode: true, hasDescription: false, isPassthroughProfile: true }), + makeProps({ isSessionMode: true, hasDescription: false }), KIND_DEFAULT, ), - ).toBeNull(); + ).toBe(REASON_DESCRIPTION); }); it("ignores base reasons in session mode to match DefaultSubmitButton disabled logic", () => { diff --git a/apps/web/components/task-create-dialog-footer.tsx b/apps/web/components/task-create-dialog-footer.tsx index 2dd444fafb..17d1f0b11b 100644 --- a/apps/web/components/task-create-dialog-footer.tsx +++ b/apps/web/components/task-create-dialog-footer.tsx @@ -151,7 +151,6 @@ type DefaultSubmitButtonProps = { isCreateMode: boolean; isEditMode: boolean; hasDescription: boolean; - isPassthroughProfile: boolean; disabled: boolean; }; @@ -162,7 +161,6 @@ function DefaultSubmitButton({ isCreateMode, isEditMode, hasDescription, - isPassthroughProfile, disabled, }: DefaultSubmitButtonProps) { const planModeStyle = @@ -176,10 +174,7 @@ function DefaultSubmitButton({ variant="default" className={`w-full h-10 cursor-pointer sm:w-auto sm:h-7 gap-1.5 ${planModeStyle}`} disabled={ - disabled || - isCreatingSession || - isCreatingTask || - (isSessionMode ? !hasDescription && !isPassthroughProfile : false) + disabled || isCreatingSession || isCreatingTask || (isSessionMode ? !hasDescription : false) } > {(() => { @@ -211,7 +206,6 @@ export type TaskCreateDialogFooterProps = { isCreateMode: boolean; isEditMode: boolean; isTaskStarted: boolean; - isPassthroughProfile: boolean; isCreatingSession: boolean; isCreatingTask: boolean; hasTitle: boolean; @@ -291,14 +285,10 @@ function baseReason(props: TaskCreateDialogFooterProps): string | null { return null; } -function missingSessionDescription(props: TaskCreateDialogFooterProps): boolean { - return !props.hasDescription && !props.isPassthroughProfile; -} - function sessionDefaultReason(props: TaskCreateDialogFooterProps): string | null { if (props.noCompatibleAgent) return noCompatibleAgentReason(props.executorProfileName); if (!props.agentProfileId) return REASON_AGENT; - if (missingSessionDescription(props)) return REASON_DESCRIPTION; + if (!props.hasDescription) return REASON_DESCRIPTION; return null; } @@ -324,7 +314,7 @@ function resolveButtonKind(props: TaskCreateDialogFooterProps, showStartTask: bo function computeFooterState(props: TaskCreateDialogFooterProps) { const showStartTask = - (props.isCreateMode && (props.hasDescription || props.isPassthroughProfile)) || + (props.isCreateMode && props.hasDescription) || Boolean(props.isEditMode && props.agentProfileId); const blocked = Boolean(props.submitBlockedReason); const altDisabled = computeBaseDisabled(props) || blocked; @@ -350,7 +340,6 @@ export const TaskCreateDialogFooter = memo(function TaskCreateDialogFooter( isCreateMode, isEditMode, isTaskStarted, - isPassthroughProfile, isCreatingSession, isCreatingTask, hasTitle, @@ -417,7 +406,6 @@ export const TaskCreateDialogFooter = memo(function TaskCreateDialogFooter( isCreateMode={isCreateMode} isEditMode={isEditMode} hasDescription={hasDescription} - isPassthroughProfile={isPassthroughProfile} disabled={defaultDisabled} /> ); diff --git a/apps/web/components/task-create-dialog-form-body.test.tsx b/apps/web/components/task-create-dialog-form-body.test.tsx index 3a3de28e03..f13c486f5a 100644 --- a/apps/web/components/task-create-dialog-form-body.test.tsx +++ b/apps/web/components/task-create-dialog-form-body.test.tsx @@ -1,6 +1,12 @@ +import { createRef } from "react"; import { render, screen } from "@testing-library/react"; import { describe, expect, it, vi } from "vitest"; -import { CreateEditSelectors, WorkflowSection } from "./task-create-dialog-form-body"; +import { + CreateEditSelectors, + DialogPromptSection, + WorkflowSection, +} from "./task-create-dialog-form-body"; +import type { DialogFormState, TaskFormInputsHandle } from "./task-create-dialog-types"; vi.mock("@/components/workflow-selector-row", () => ({ WorkflowSelectorRow: ({ selectedWorkflowId }: { selectedWorkflowId: string | null }) => ( @@ -8,6 +14,22 @@ vi.mock("@/components/workflow-selector-row", () => ({ ), })); +// Capture every render of TaskFormInputs so passthrough-related assertions can +// inspect the props that DialogPromptSection forwards to the textarea. +const taskFormInputsCalls: Array> = []; +vi.mock("@/components/task-create-dialog-selectors", () => ({ + TaskFormInputs: (props: Record) => { + taskFormInputsCalls.push(props); + return ( +