From 40beea5f2ed8ed108124f04f4d10a45bdab776a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Tue, 1 Sep 2026 12:28:47 +0000 Subject: [PATCH] fix: guard passthrough initial prompt turn (#3247) (#3249) --- .../runtime/lifecycle/manager_passthrough.go | 70 ++++++++-- .../manager_passthrough_autoinject_test.go | 43 ++++-- .../manager_passthrough_launch_test.go | 84 +++++++++++ .../manager_passthrough_runtime_test.go | 130 +++++++++++++++++- .../internal/agent/runtime/lifecycle/types.go | 3 + .../agentctl/server/process/interactive_io.go | 10 +- .../interactive_runner_firstidle_test.go | 26 ++++ ...assthrough-initial-prompt-turn-boundary.md | 60 ++++++++ docs/decisions/INDEX.md | 1 + .../plan.md | 87 ++++++++++++ .../task-01-guard-passthrough-initial-turn.md | 111 +++++++++++++++ docs/specs/tasks/README.md | 2 + ...assthrough-initial-prompt-turn-boundary.md | 62 +++++++++ ...assthrough-initial-prompt-turn-boundary.md | 106 ++++++++++++++ 14 files changed, 769 insertions(+), 26 deletions(-) create mode 100644 docs/decisions/2026-09-01-passthrough-initial-prompt-turn-boundary.md create mode 100644 docs/plans/passthrough-initial-prompt-turn-boundary/plan.md create mode 100644 docs/plans/passthrough-initial-prompt-turn-boundary/task-01-guard-passthrough-initial-turn.md create mode 100644 docs/specs/tasks/requirements/passthrough-initial-prompt-turn-boundary.md create mode 100644 docs/specs/tasks/system-design/passthrough-initial-prompt-turn-boundary.md diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_passthrough.go b/apps/backend/internal/agent/runtime/lifecycle/manager_passthrough.go index c760781894..f9887bd766 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/manager_passthrough.go +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_passthrough.go @@ -810,6 +810,11 @@ func (m *Manager) startPassthroughSession(ctx context.Context, execution *AgentE execution.PassthroughProcessID = processInfo.ID execution.PassthroughStartedAt = time.Now() execution.passthroughLaunchUsedResume = false + if requiresPassthroughInitialPrompt(execution, pt) { + execution.passthroughInitialPromptProcessID = processInfo.ID + } else { + execution.passthroughInitialPromptProcessID = "" + } execution.passthroughLifecycleMu.Unlock() m.logger.Info("passthrough session started", @@ -832,7 +837,7 @@ func (m *Manager) startPassthroughSession(ctx context.Context, execution *AgentE } } - go m.autoInjectInitialPrompt(execution, pt) + go m.autoInjectInitialPrompt(execution, pt, processInfo.ID) return nil } @@ -947,6 +952,7 @@ func (m *Manager) restartPassthroughProcess(ctx context.Context, execution *Agen // the deliberately-killed process. oldProcessID := execution.PassthroughProcessID execution.PassthroughProcessID = "" + execution.passthroughInitialPromptProcessID = "" execution.PassthroughStartedAt = time.Time{} if err := interactiveRunner.Stop(ctx, oldProcessID); err != nil { @@ -969,6 +975,7 @@ func (m *Manager) restartPassthroughProcess(ctx context.Context, execution *Agen execution.PassthroughProcessID = processInfo.ID execution.PassthroughStartedAt = time.Now() execution.passthroughLaunchUsedResume = false + execution.passthroughInitialPromptProcessID = "" m.logger.Info("passthrough process restarted with fresh context", zap.String("execution_id", execution.ID), @@ -1070,6 +1077,7 @@ func (m *Manager) resumePassthroughSession(ctx context.Context, sessionID, expec // (the ID is what routes a status update to this execution), so the flag has // to be set before the process can exit and publish a status. execution.passthroughLaunchUsedResume = useResume + execution.passthroughInitialPromptProcessID = "" execution.PassthroughProcessID = processInfo.ID m.logger.Info("passthrough session resumed", @@ -1106,11 +1114,21 @@ func (m *Manager) handlePassthroughTurnComplete(sessionID, processID string) { zap.String("session_id", sessionID)) return } - if execution.PassthroughProcessID != processID { + execution.passthroughLifecycleMu.Lock() + activeProcessID := execution.PassthroughProcessID + initialPromptPending := execution.passthroughInitialPromptProcessID == processID + execution.passthroughLifecycleMu.Unlock() + if activeProcessID != processID { m.logger.Debug("ignoring stale passthrough turn complete", zap.String("session_id", sessionID), zap.String("process_id", processID), - zap.String("active_process_id", execution.PassthroughProcessID)) + zap.String("active_process_id", activeProcessID)) + return + } + if initialPromptPending { + m.logger.Debug("ignoring passthrough turn complete while initial prompt is pending", + zap.String("session_id", sessionID), + zap.String("process_id", processID)) return } @@ -1387,6 +1405,11 @@ func (m *Manager) attemptResumeFallbackForProcess(execution *AgentExecution, run execution.PassthroughStartedAt = time.Now() execution.passthroughLaunchUsedResume = false execution.PassthroughProcessID = processInfo.ID + if requiresPassthroughInitialPrompt(execution, pt) { + execution.passthroughInitialPromptProcessID = processInfo.ID + } else { + execution.passthroughInitialPromptProcessID = "" + } if runner.ConnectSessionWebSocket(processInfo.ID) { m.logger.Info("passthrough resume fallback succeeded", @@ -1411,7 +1434,7 @@ func (m *Manager) attemptResumeFallbackForProcess(execution *AgentExecution, run } // Fallback path is a fresh session (no --resume) — re-inject the prompt. - go m.autoInjectInitialPrompt(execution, pt) + go m.autoInjectInitialPrompt(execution, pt, processInfo.ID) } // attemptPassthroughRestart announces the restart on the terminal, waits the @@ -1556,21 +1579,46 @@ type passthroughRunner interface { WriteStdin(processID string, data string) error } +func requiresPassthroughInitialPrompt(execution *AgentExecution, pt agents.PassthroughConfig) bool { + return pt.PromptFlag.IsEmpty() && getTaskDescriptionFromMetadata(execution) != "" +} + +// claimPassthroughInitialPrompt verifies that this goroutine is still +// associated with the active PTY process before proceeding with injection. +// startPassthroughSession pre-sets the marker; this check deliberately does +// not claim a marker for a replacement process. +func claimPassthroughInitialPrompt(execution *AgentExecution, processID string) bool { + execution.passthroughLifecycleMu.Lock() + defer execution.passthroughLifecycleMu.Unlock() + if execution.PassthroughProcessID != processID || execution.passthroughInitialPromptProcessID != processID { + return false + } + return true +} + +func clearPassthroughInitialPrompt(execution *AgentExecution, processID string) { + execution.passthroughLifecycleMu.Lock() + defer execution.passthroughLifecycleMu.Unlock() + if execution.passthroughInitialPromptProcessID == processID { + execution.passthroughInitialPromptProcessID = "" + } +} + // autoInjectInitialPrompt writes the task description to PTY stdin once a // passthrough agent without a PromptFlag is idle (ready for input). 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) { +func (m *Manager) autoInjectInitialPrompt(execution *AgentExecution, pt agents.PassthroughConfig, processID string) { runner := m.GetInteractiveRunner() if runner == nil { return } - m.autoInjectInitialPromptWith(runner, execution, pt) + m.autoInjectInitialPromptWith(runner, execution, pt, processID) } // 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) { +func (m *Manager) autoInjectInitialPromptWith(runner passthroughRunner, execution *AgentExecution, pt agents.PassthroughConfig, processID string) { if !pt.PromptFlag.IsEmpty() { // The agent already received the prompt as a CLI flag — no stdin // delivery. This mirrors promptForPassthroughCommand, which only @@ -1586,12 +1634,15 @@ func (m *Manager) autoInjectInitialPromptWith(runner passthroughRunner, executio // take this exit and never receive spurious stdin. return } - processID := execution.PassthroughProcessID if processID == "" { m.logger.Warn("autoInjectInitialPrompt called without passthrough process", zap.String("execution_id", execution.ID)) return } + if !claimPassthroughInitialPrompt(execution, processID) { + return + } + defer clearPassthroughInitialPrompt(execution, processID) ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) defer cancel() if err := runner.WaitForFirstIdle(ctx, processID); err != nil { @@ -1601,6 +1652,9 @@ func (m *Manager) autoInjectInitialPromptWith(runner passthroughRunner, executio zap.Error(err)) return } + if !m.passthroughProcessMatches(execution, processID) { + 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 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 index b8718ac77e..5e7b9bf50f 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/manager_passthrough_autoinject_test.go +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_passthrough_autoinject_test.go @@ -7,6 +7,7 @@ import ( "time" "github.com/kandev/kandev/internal/agent/agents" + v1 "github.com/kandev/kandev/pkg/api/v1" ) // fakePassthroughRunner is a minimal stub of the passthroughRunner seam used @@ -62,6 +63,7 @@ func newAutoInjectExecution(description string) *AgentExecution { SessionID: "sess-1", PassthroughProcessID: "proc-abc", } + exec.passthroughInitialPromptProcessID = exec.PassthroughProcessID if description != "" { exec.setMetadataValue("task_description", description) } @@ -81,7 +83,7 @@ func TestAutoInject_disabled_does_nothing(t *testing.T) { mgr.autoInjectInitialPromptWith(runner, newAutoInjectExecution(""), agents.PassthroughConfig{ AutoInjectPrompt: false, SubmitSequence: "\r", - }) + }, "proc-abc") if runner.writeCalled { t.Fatalf("expected no stdin write when task description is empty, got data=%q", runner.writtenData) @@ -100,7 +102,7 @@ func TestAutoInject_writes_without_auto_inject_flag(t *testing.T) { mgr.autoInjectInitialPromptWith(runner, newAutoInjectExecution("do a thing"), agents.PassthroughConfig{ AutoInjectPrompt: false, SubmitSequence: "\r", - }) + }, "proc-abc") if !runner.writeCalled { t.Fatalf("expected WriteStdin to be called for a no-flag custom TUI agent") @@ -113,16 +115,27 @@ func TestAutoInject_writes_without_auto_inject_flag(t *testing.T) { func TestAutoInject_skipped_when_PromptFlag_set(t *testing.T) { mgr := newTestManager(t) runner := &fakePassthroughRunner{} + execution := newAutoInjectExecution("do a thing") + execution.Status = v1.AgentStatusRunning + execution.passthroughInitialPromptProcessID = "" + if err := mgr.executionStore.Add(execution); err != nil { + t.Fatalf("add execution: %v", err) + } - mgr.autoInjectInitialPromptWith(runner, newAutoInjectExecution("do a thing"), agents.PassthroughConfig{ + mgr.autoInjectInitialPromptWith(runner, execution, agents.PassthroughConfig{ AutoInjectPrompt: true, SubmitSequence: "\r", PromptFlag: agents.NewParam("--prompt", "{prompt}"), - }) + }, execution.PassthroughProcessID) if runner.writeCalled { t.Fatalf("expected no stdin write when PromptFlag is set, got data=%q", runner.writtenData) } + + mgr.handlePassthroughTurnComplete(execution.SessionID, execution.PassthroughProcessID) + if execution.Status != v1.AgentStatusReady { + t.Fatalf("prompt-flag completion left status %q, want READY", execution.Status) + } } func TestAutoInject_skipped_when_description_empty(t *testing.T) { @@ -132,7 +145,7 @@ func TestAutoInject_skipped_when_description_empty(t *testing.T) { mgr.autoInjectInitialPromptWith(runner, newAutoInjectExecution(""), agents.PassthroughConfig{ AutoInjectPrompt: true, SubmitSequence: "\r", - }) + }, "proc-abc") if runner.writeCalled { t.Fatalf("expected no stdin write when task description is empty, got data=%q", runner.writtenData) @@ -146,7 +159,7 @@ func TestAutoInject_writes_description_plus_submit(t *testing.T) { mgr.autoInjectInitialPromptWith(runner, newAutoInjectExecution("hello world"), agents.PassthroughConfig{ AutoInjectPrompt: true, SubmitSequence: "\r", - }) + }, "proc-abc") if !runner.writeCalled { t.Fatalf("expected WriteStdin to be called") @@ -165,14 +178,19 @@ func TestAutoInject_writes_description_plus_submit(t *testing.T) { func TestAutoInject_returns_when_wait_errors(t *testing.T) { mgr := newTestManager(t) runner := &fakePassthroughRunner{waitErr: context.DeadlineExceeded} + execution := newAutoInjectExecution("hello") + execution.Status = v1.AgentStatusRunning + if err := mgr.executionStore.Add(execution); err != nil { + t.Fatalf("add execution: %v", err) + } done := make(chan struct{}) go func() { defer close(done) - mgr.autoInjectInitialPromptWith(runner, newAutoInjectExecution("hello"), agents.PassthroughConfig{ + mgr.autoInjectInitialPromptWith(runner, execution, agents.PassthroughConfig{ AutoInjectPrompt: true, SubmitSequence: "\r", - }) + }, execution.PassthroughProcessID) }() select { @@ -184,6 +202,11 @@ func TestAutoInject_returns_when_wait_errors(t *testing.T) { if runner.writeCalled { t.Fatalf("expected no stdin write on wait timeout, got data=%q", runner.writtenData) } + + mgr.handlePassthroughTurnComplete(execution.SessionID, execution.PassthroughProcessID) + if execution.Status != v1.AgentStatusReady { + t.Fatalf("completion after aborted injection left status %q, want READY", execution.Status) + } } // TestAutoInject_SubmitDelay_splits_writes_with_pause is the Claude regression: @@ -201,7 +224,7 @@ func TestAutoInject_SubmitDelay_splits_writes_with_pause(t *testing.T) { SubmitSequence: "\r", DisableBracketedPaste: true, SubmitDelay: delay, - }) + }, "proc-abc") if got := len(runner.writes); got != 2 { t.Fatalf("expected 2 writes (body, submit), got %d: %#v", got, runner.writes) @@ -231,7 +254,7 @@ func TestAutoInject_skipped_when_process_id_missing(t *testing.T) { mgr.autoInjectInitialPromptWith(runner, exec, agents.PassthroughConfig{ AutoInjectPrompt: true, SubmitSequence: "\r", - }) + }, exec.PassthroughProcessID) 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_launch_test.go b/apps/backend/internal/agent/runtime/lifecycle/manager_passthrough_launch_test.go index 39205cb54b..00f677d578 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/manager_passthrough_launch_test.go +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_passthrough_launch_test.go @@ -11,10 +11,13 @@ import ( "github.com/stretchr/testify/require" + "github.com/kandev/kandev/internal/agent/agents" "github.com/kandev/kandev/internal/agent/executor" settingsmodels "github.com/kandev/kandev/internal/agent/settings/models" "github.com/kandev/kandev/internal/agentctl/server/process" agentctltypes "github.com/kandev/kandev/internal/agentctl/types" + "github.com/kandev/kandev/internal/events" + eventbus "github.com/kandev/kandev/internal/events/bus" v1 "github.com/kandev/kandev/pkg/api/v1" ) @@ -138,6 +141,48 @@ func TestStartPassthroughSessionSurfacesLaunchFailure(t *testing.T) { require.NotEmpty(t, execution.ErrorMessage) } +// @covers AC-TASKS-PASSTHROUGH-INITIAL-TURN-001.1 +func TestStartPassthroughSessionClaimsInitialPromptBeforeReadyPublication(t *testing.T) { + mgr, runner, execution := newPassthroughRunnerManager(t) + const agentID = "pending-prompt-test-agent" + require.NoError(t, mgr.registry.Register(&testAgent{ + id: agentID, + enabled: true, + StandardPassthrough: agents.StandardPassthrough{Cfg: agents.PassthroughConfig{ + Supported: true, + PassthroughCmd: agents.NewCommand("sh", "-c", "sleep 30"), + IdleTimeout: time.Second, + SubmitSequence: "\r", + AutoInjectPrompt: true, + }}, + })) + mgr.profileResolver = &mockPassthroughProfileResolver{ + agentName: agentID, + cliPassthrough: true, + } + execution.AgentID = agentID + execution.Status = v1.AgentStatusRunning + execution.setMetadataValue("task_description", "Ship the release") + + var statusAtReadyPublication v1.AgentStatus + bus := mgr.eventBus.(*MockEventBus) + bus.OnPublish = func(subject string, _ *eventbus.Event) { + if subject != events.AgentctlReady { + return + } + mgr.handlePassthroughTurnComplete(execution.SessionID, execution.PassthroughProcessID) + statusAtReadyPublication = execution.Status + } + + require.NoError(t, mgr.startPassthroughSession(context.Background(), execution, nil)) + t.Cleanup(func() { + _ = runner.Stop(context.Background(), execution.PassthroughProcessID) + }) + + require.Equal(t, v1.AgentStatusRunning, statusAtReadyPublication, + "the launch must claim initial prompt delivery before later startup work can publish ready") +} + func TestResumePassthroughSessionSurfacesLaunchFailure(t *testing.T) { mgr, _, execution := newPassthroughRunnerManager(t) execution.WorkspacePath = missingWorkspace(t) @@ -261,6 +306,45 @@ func TestAttemptResumeFallbackAnnouncesAndRelaunchesFresh(t *testing.T) { require.False(t, execution.passthroughLaunchUsedResume) } +// @covers AC-TASKS-PASSTHROUGH-INITIAL-TURN-001.4 +func TestAttemptResumeFallbackClaimsFreshInitialPrompt(t *testing.T) { + mgr, runner, execution := newPassthroughRunnerManager(t) + const agentID = "fallback-pending-prompt-test-agent" + require.NoError(t, mgr.registry.Register(&testAgent{ + id: agentID, + enabled: true, + StandardPassthrough: agents.StandardPassthrough{Cfg: agents.PassthroughConfig{ + Supported: true, + PassthroughCmd: agents.NewCommand("sh", "-c", "sleep 30"), + IdleTimeout: time.Second, + SubmitSequence: "\r", + AutoInjectPrompt: true, + }}, + })) + mgr.profileResolver = &mockPassthroughProfileResolver{ + agentName: agentID, + cliPassthrough: true, + } + execution.AgentID = agentID + execution.Status = v1.AgentStatusRunning + execution.setMetadataValue("task_description", "Ship the release") + recorder := &terminalRecorder{} + runner.TrackSessionWebSocket(execution.SessionID, recorder) + + mgr.attemptResumeFallback(execution, runner, execution.SessionID, 1, 400*time.Millisecond) + + execution.passthroughLifecycleMu.Lock() + activeProcessID := execution.PassthroughProcessID + pendingProcessID := execution.passthroughInitialPromptProcessID + execution.passthroughLifecycleMu.Unlock() + t.Cleanup(func() { + _ = runner.Stop(context.Background(), activeProcessID) + }) + require.NotEmpty(t, activeProcessID) + require.Equal(t, activeProcessID, pendingProcessID, + "a fresh fallback must own its initial prompt before the injection goroutine runs") +} + func TestAttemptResumeFallbackSkipsDuringShutdown(t *testing.T) { mgr, runner, execution := newPassthroughRunnerManager(t) mgr.shuttingDown.Store(true) diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_passthrough_runtime_test.go b/apps/backend/internal/agent/runtime/lifecycle/manager_passthrough_runtime_test.go index 6e42f06944..abb1d0223c 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/manager_passthrough_runtime_test.go +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_passthrough_runtime_test.go @@ -503,6 +503,121 @@ func TestHandlePassthroughTurnCompleteUnknownSession(t *testing.T) { require.Empty(t, bus.PublishedEvents) } +type pendingInitialPromptRunner struct { + waiting chan struct{} + release chan struct{} + writeCalled bool +} + +func (r *pendingInitialPromptRunner) WaitForFirstIdle(context.Context, string) error { + close(r.waiting) + <-r.release + return nil +} + +func (r *pendingInitialPromptRunner) WriteStdin(string, string) error { + r.writeCalled = true + return nil +} + +// @covers AC-TASKS-PASSTHROUGH-INITIAL-TURN-001.1 +func TestHandlePassthroughTurnCompleteSuppressesPendingInitialPrompt(t *testing.T) { + mgr := newTestManager(t) + execution := newAutoInjectExecution("Ship the release") + execution.Status = v1.AgentStatusRunning + require.NoError(t, mgr.executionStore.Add(execution)) + + runner := &pendingInitialPromptRunner{ + waiting: make(chan struct{}), + release: make(chan struct{}), + } + done := make(chan struct{}) + go func() { + defer close(done) + mgr.autoInjectInitialPromptWith(runner, execution, agents.PassthroughConfig{SubmitSequence: "\r"}, execution.PassthroughProcessID) + }() + released := false + defer func() { + if !released { + close(runner.release) + <-done + } + }() + + <-runner.waiting + mgr.handlePassthroughTurnComplete(execution.SessionID, execution.PassthroughProcessID) + + require.Equal(t, v1.AgentStatusRunning, execution.Status, + "startup readiness must not complete a turn before initial prompt injection") + bus := mgr.eventBus.(*MockEventBus) + require.Empty(t, bus.PublishedEvents, "startup readiness must not publish a lifecycle event") + + close(runner.release) + released = true + <-done + mgr.handlePassthroughTurnComplete(execution.SessionID, execution.PassthroughProcessID) + + require.Equal(t, v1.AgentStatusReady, execution.Status) + require.Len(t, bus.PublishedEvents, 1) + require.Equal(t, events.AgentReady, bus.PublishedEvents[0].Type) +} + +// @covers AC-TASKS-PASSTHROUGH-INITIAL-TURN-001.5 +func TestAutoInjectCleanupDoesNotClearReplacementInitialPrompt(t *testing.T) { + mgr := newTestManager(t) + execution := newAutoInjectExecution("Ship the release") + execution.Status = v1.AgentStatusRunning + require.NoError(t, mgr.executionStore.Add(execution)) + + runner := &pendingInitialPromptRunner{ + waiting: make(chan struct{}), + release: make(chan struct{}), + } + done := make(chan struct{}) + go func() { + defer close(done) + mgr.autoInjectInitialPromptWith(runner, execution, agents.PassthroughConfig{SubmitSequence: "\r"}, execution.PassthroughProcessID) + }() + defer func() { + select { + case <-done: + default: + close(runner.release) + <-done + } + }() + + <-runner.waiting + execution.passthroughLifecycleMu.Lock() + execution.PassthroughProcessID = "proc-replacement" + execution.passthroughInitialPromptProcessID = "proc-replacement" + execution.passthroughLifecycleMu.Unlock() + close(runner.release) + <-done + + require.False(t, runner.writeCalled, + "an injection owned by the old process must not write after replacement") + mgr.handlePassthroughTurnComplete(execution.SessionID, "proc-replacement") + require.Equal(t, v1.AgentStatusRunning, execution.Status, + "cleanup from the old injection must not expose the replacement turn boundary") +} + +// @covers AC-TASKS-PASSTHROUGH-INITIAL-TURN-001.5 +func TestAutoInjectSkipsReplacementBeforeClaim(t *testing.T) { + mgr := newTestManager(t) + execution := newAutoInjectExecution("Ship the release") + execution.PassthroughProcessID = "proc-replacement" + execution.passthroughInitialPromptProcessID = "proc-replacement" + runner := &fakePassthroughRunner{} + + mgr.autoInjectInitialPromptWith(runner, execution, agents.PassthroughConfig{SubmitSequence: "\r"}, "proc-original") + + require.False(t, runner.writeCalled, + "an injector for the replaced process must not write to the replacement") + require.Equal(t, "proc-replacement", execution.passthroughInitialPromptProcessID, + "the replacement marker must remain available to its injector") +} + // TestAutoInjectMarksRunningBeforeWriting pins the ordering that blocks a // racing composer submission: the execution must be RUNNING before the first // chunk lands on the PTY, so checkSessionPromptable rejects a concurrent @@ -516,7 +631,7 @@ func TestAutoInjectMarksRunningBeforeWriting(t *testing.T) { var statusAtFirstWrite v1.AgentStatus runner := &statusObservingRunner{onWrite: func() { statusAtFirstWrite = execution.Status }} - mgr.autoInjectInitialPromptWith(runner, execution, agents.PassthroughConfig{SubmitSequence: "\r"}) + mgr.autoInjectInitialPromptWith(runner, execution, agents.PassthroughConfig{SubmitSequence: "\r"}, execution.PassthroughProcessID) require.Equal(t, []string{"Ship the release\r"}, runner.writes) require.Equal(t, v1.AgentStatusRunning, statusAtFirstWrite, @@ -534,10 +649,14 @@ func TestAutoInjectStopsAfterWriteFailure(t *testing.T) { SubmitSequence: "\r", DisableBracketedPaste: true, SubmitDelay: time.Millisecond, - }) + }, execution.PassthroughProcessID) require.Len(t, runner.writes, 1, "a failed write must abort the chunk loop instead of continuing to the submit byte") + + mgr.handlePassthroughTurnComplete(execution.SessionID, execution.PassthroughProcessID) + require.Equal(t, v1.AgentStatusReady, execution.Status, + "a failed injection must release the completion boundary for later manual work") } func TestAutoInjectSkipsDuringShutdown(t *testing.T) { @@ -545,9 +664,10 @@ func TestAutoInjectSkipsDuringShutdown(t *testing.T) { mgr.shuttingDown.Store(true) runner := &fakePassthroughRunner{} - mgr.autoInjectInitialPromptWith(runner, newAutoInjectExecution("Ship it"), agents.PassthroughConfig{ + execution := newAutoInjectExecution("Ship it") + mgr.autoInjectInitialPromptWith(runner, execution, agents.PassthroughConfig{ SubmitSequence: "\r", - }) + }, execution.PassthroughProcessID) require.False(t, runner.writeCalled, "teardown must not race a stdin write into a dying PTY") } @@ -561,7 +681,7 @@ func TestAutoInjectInitialPromptWithoutRunnerIsNoOp(t *testing.T) { execution.Status = v1.AgentStatusReady require.NoError(t, mgr.executionStore.Add(execution)) - mgr.autoInjectInitialPrompt(execution, agents.PassthroughConfig{SubmitSequence: "\r"}) + mgr.autoInjectInitialPrompt(execution, agents.PassthroughConfig{SubmitSequence: "\r"}, execution.PassthroughProcessID) require.Equal(t, v1.AgentStatusReady, execution.Status, "without a runner the execution must not be flipped to RUNNING") diff --git a/apps/backend/internal/agent/runtime/lifecycle/types.go b/apps/backend/internal/agent/runtime/lifecycle/types.go index dca08d1a79..45fc32b486 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/types.go +++ b/apps/backend/internal/agent/runtime/lifecycle/types.go @@ -128,6 +128,9 @@ type AgentExecution struct { passthroughLifecycleMu sync.Mutex 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 + // passthroughInitialPromptProcessID identifies the fresh PTY process that + // still needs its initial task prompt through stdin. + passthroughInitialPromptProcessID string // passthroughLaunchUsedResume is true if the current passthrough process was // launched via ResumePassthroughSession with the resume flag attached. The // fast-fail handler reads this to decide whether to retry once with a fresh diff --git a/apps/backend/internal/agentctl/server/process/interactive_io.go b/apps/backend/internal/agentctl/server/process/interactive_io.go index 6b3f2afc61..501bd99e87 100644 --- a/apps/backend/internal/agentctl/server/process/interactive_io.go +++ b/apps/backend/internal/agentctl/server/process/interactive_io.go @@ -191,14 +191,18 @@ func (r *InteractiveRunner) resetIdleTimer(proc *interactiveProcess) { } func (r *InteractiveRunner) emitTurnComplete(proc *interactiveProcess) { + // Invoke the callback before releasing firstIdleCh. Callers that inject the + // initial prompt use the callback to consume the startup completion boundary; + // waking the injector first could let it clear that marker before the + // callback observes it. + if r.turnCompleteCallback != nil { + r.turnCompleteCallback(proc.info.SessionID, proc.info.ID) + } // 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, proc.info.ID) - } r.logger.Debug("turn complete detected", zap.String("process_id", proc.info.ID), zap.String("session_id", proc.info.SessionID)) 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 index 042422eb3b..ad741f5d9a 100644 --- a/apps/backend/internal/agentctl/server/process/interactive_runner_firstidle_test.go +++ b/apps/backend/internal/agentctl/server/process/interactive_runner_firstidle_test.go @@ -92,3 +92,29 @@ func TestEmitTurnComplete_closes_firstIdle_only_once(t *testing.T) { t.Fatalf("firstIdleCh was not closed after emitTurnComplete") } } + +func TestEmitTurnComplete_callsCallbackBeforeReleasingFirstIdle(t *testing.T) { + runner, proc := newRunnerWithProcess(t, "proc-order") + callbackCalled := make(chan struct{}) + runner.SetTurnCompleteCallback(func(_, _ string) { + select { + case <-proc.firstIdleCh: + t.Fatal("firstIdleCh released before turn-complete callback") + default: + } + close(callbackCalled) + }) + + runner.emitTurnComplete(proc) + + select { + case <-callbackCalled: + default: + t.Fatal("turn-complete callback was not called") + } + select { + case <-proc.firstIdleCh: + default: + t.Fatal("firstIdleCh was not released after turn-complete callback") + } +} diff --git a/docs/decisions/2026-09-01-passthrough-initial-prompt-turn-boundary.md b/docs/decisions/2026-09-01-passthrough-initial-prompt-turn-boundary.md new file mode 100644 index 0000000000..41ea0c9b5d --- /dev/null +++ b/docs/decisions/2026-09-01-passthrough-initial-prompt-turn-boundary.md @@ -0,0 +1,60 @@ +# ADR-2026-09-01-passthrough-initial-prompt-turn-boundary: Keep Passthrough Initial Prompt State in Lifecycle + +**Status:** accepted +**Date:** 2026-09-01 +**Area:** backend, workflow + +## Context + +CLI passthrough uses the first PTY idle window as the readiness signal for +injecting an initial task prompt. The interactive runner also reports every +idle window as a completed turn. On a fresh process, the first callback can +therefore publish `agent.ready` before prompt injection. This event can place +the task in review and evaluate completion consumers before the agent receives +work. + +The raw detector cannot determine whether Kandev still owes the process an +initial prompt. That delivery context belongs to lifecycle and differs across +fresh stdin injection, prompt-flag launch, promptless launch, successful resume, +and fresh-start fallback. + +## Decision + +The lifecycle manager owns an in-memory initial-prompt marker bound to the +active passthrough process identity. Fresh launches and fresh-start fallbacks +install the marker only when a non-empty task prompt must be delivered through +PTY stdin. + +Passthrough completion handling ignores every raw completion callback from the +marked process until prompt injection finishes or aborts. Prompt injection +clears only the marker whose process identity it captured. Prompt-flag, +promptless, and resumed processes do not install a marker and retain ordinary +completion behavior. + +Agentctl remains a raw signal source. It does not receive task-description or +prompt-delivery policy, and it does not reinterpret the first idle globally. + +## Consequences + +Startup idle, prompt-pattern, and status signals cannot move a task or trigger +workflow completion before the initial prompt is submitted. Process identity +prevents delayed cleanup or callbacks from affecting a replacement process. + +The marker is runtime-only and must be cleared on successful submission, +timeout, shutdown, write failure, and process replacement. Mid-turn idle +accuracy is unchanged and remains a separate concern. + +## Alternatives Considered + +- Suppress the first idle inside `InteractiveRunner`. The runner lacks prompt + delivery context, and first idle is a real completion for prompt-flag agents. + Suppressing one callback also does not cover a concurrent prompt-pattern or + status callback before injection. +- Publish `agent.boot_ready` for the first idle. Boot-ready handling can make + the session promptable and drain queued work while the initial prompt is + still pending. +- Force an `agent.running` event during injection. That repairs state only + after a false completion and retains a visible state flicker and completion + side effects that cannot be undone. +- Increase the idle timeout. Cold-start duration is unbounded, and timeout + tuning cannot distinguish startup readiness from turn completion. diff --git a/docs/decisions/INDEX.md b/docs/decisions/INDEX.md index 19cd609592..45c902d9e5 100644 --- a/docs/decisions/INDEX.md +++ b/docs/decisions/INDEX.md @@ -228,3 +228,4 @@ Read individual ADRs for full context. Create new ones via `/record decision` or | 2026-08-31-revoke-merge-approval-after-untrusted-push | [Revoke merge approval after an untrusted pull-request push](2026-08-31-revoke-merge-approval-after-untrusted-push.md) | accepted | workflow, security | 2026-08-31 | | 2026-08-31-passthrough-running-publication | [Defer Passthrough Running Publication Until Guard Release](2026-08-31-passthrough-running-publication.md) | accepted | backend | 2026-08-31 | | 2026-08-31-agent-aware-mcp-tool-names | [Preserve Canonical MCP Tool Names Through Agent Namespacing](2026-08-31-agent-aware-mcp-tool-names.md) | accepted | backend, agentctl, protocol | 2026-08-31 | +| 2026-09-01-passthrough-initial-prompt-turn-boundary | [Keep Passthrough Initial Prompt State in Lifecycle](2026-09-01-passthrough-initial-prompt-turn-boundary.md) | accepted | backend, workflow | 2026-09-01 | diff --git a/docs/plans/passthrough-initial-prompt-turn-boundary/plan.md b/docs/plans/passthrough-initial-prompt-turn-boundary/plan.md new file mode 100644 index 0000000000..c0f82369f7 --- /dev/null +++ b/docs/plans/passthrough-initial-prompt-turn-boundary/plan.md @@ -0,0 +1,87 @@ +--- +created: 2026-09-01 +status: done +requirements: + - REQ-TASKS-PASSTHROUGH-INITIAL-TURN-001 +system_design: + - ../../specs/tasks/system-design/passthrough-initial-prompt-turn-boundary.md +legacy_specs: + - ../../specs/cli/requirements/cli-mode-parity.md +--- + +# Implementation Plan: Passthrough Initial Prompt Turn Boundary + +## Overview + +Add a process-scoped pending-initial-prompt marker to passthrough lifecycle, +then use it to reject completion callbacks until fresh prompt injection ends. +One work order keeps marker ownership, launch and fallback integration, cleanup, +and regression coverage atomic. + +## Scope + +### In scope + +- Mark fresh passthrough processes that still require initial stdin prompt + injection. +- Suppress all completion callback sources while the matching process owns the + marker. +- Clear the marker safely after successful or aborted injection. +- Preserve first-completion behavior for prompt-flag, promptless, and resumed + processes. +- Cover process replacement and fresh-start fallback behavior. + +### Out of scope + +- Replacing the idle detector or fixing mid-turn idle false positives. +- Changing prompt text, submit chunk planning, or idle timeout values. +- Changing `agent.ready`, `agent.boot_ready`, or workflow event semantics. +- Adding frontend behavior or browser automation for a backend lifecycle gate. + +## Technical approach + +Extend `AgentExecution` in +`apps/backend/internal/agent/runtime/lifecycle/types.go` with a runtime-only +pending initial-prompt process identity protected by +`passthroughLifecycleMu`. Add narrow helpers that install, query, and clear the +marker by process ID. + +In `manager_passthrough.go`, centralize the predicate for stdin initial-prompt +delivery. Install the marker alongside `PassthroughProcessID` for the original +fresh start and the fresh-start resume fallback. Do not install it for +`PromptFlag`, empty-description, or ordinary resume paths. + +Have `handlePassthroughTurnComplete` snapshot the active process and marker +under the lifecycle lock. Keep the existing stale-process rejection, then +return with a debug diagnostic when the active process still owns the pending +marker. In `autoInjectInitialPromptWith`, defer identity-checked cleanup after +capturing the process ID so success, timeout, shutdown, and write failure all +release only their own marker. + +## Tests + +| Acceptance criterion | Evidence | +| --- | --- | +| `AC-TASKS-PASSTHROUGH-INITIAL-TURN-001.1` | A lifecycle regression marks a running PTY as pending and fires turn completion. It asserts that no `agent.ready` event or Ready status occurs. | +| `AC-TASKS-PASSTHROUGH-INITIAL-TURN-001.2` | An auto-injection test submits all chunks, verifies marker cleanup, then fires completion and asserts exactly one ordinary `agent.ready` event. | +| `AC-TASKS-PASSTHROUGH-INITIAL-TURN-001.3` | Existing and extended tests prove unmarked active processes, prompt-flag launches, promptless launches, and resume paths retain ordinary behavior. | +| `AC-TASKS-PASSTHROUGH-INITIAL-TURN-001.4` | The fresh-fallback launch test asserts that the new process receives the marker before its injection goroutine runs. | +| `AC-TASKS-PASSTHROUGH-INITIAL-TURN-001.5` | Timeout/write-failure and replacement tests prove identity-checked cleanup and completion eligibility for the active replacement. | + +## Work orders + +- [x] [Task 01: Guard the passthrough initial turn](task-01-guard-passthrough-initial-turn.md) + +## Verification results + +- The focused lifecycle command passed 16 tests. +- The race-detector command passed the same 16 tests. +- The full lifecycle package command passed 2,100 tests. + +## Risks + +- A marker without process identity could suppress a replacement process. +- Cleanup that runs only on successful writes could leave manual follow-up work + permanently non-completing after an injection failure. +- Installing the marker for prompt-flag or resume paths would hide a genuine + first turn completion. diff --git a/docs/plans/passthrough-initial-prompt-turn-boundary/task-01-guard-passthrough-initial-turn.md b/docs/plans/passthrough-initial-prompt-turn-boundary/task-01-guard-passthrough-initial-turn.md new file mode 100644 index 0000000000..4cbfa0e42b --- /dev/null +++ b/docs/plans/passthrough-initial-prompt-turn-boundary/task-01-guard-passthrough-initial-turn.md @@ -0,0 +1,111 @@ +--- +id: "01-guard-passthrough-initial-turn" +title: "Guard the passthrough initial turn" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +requirements: + - REQ-TASKS-PASSTHROUGH-INITIAL-TURN-001 +acceptance_criteria: + - AC-TASKS-PASSTHROUGH-INITIAL-TURN-001.1 + - AC-TASKS-PASSTHROUGH-INITIAL-TURN-001.2 + - AC-TASKS-PASSTHROUGH-INITIAL-TURN-001.3 + - AC-TASKS-PASSTHROUGH-INITIAL-TURN-001.4 + - AC-TASKS-PASSTHROUGH-INITIAL-TURN-001.5 +system_design: + - ../../specs/tasks/system-design/passthrough-initial-prompt-turn-boundary.md +--- + +# Task 01: Guard the Passthrough Initial Turn + +## Summary + +Keep fresh passthrough executions running while Kandev waits to inject their +initial prompt. Bind the pending boundary to one PTY process so successful and +failed delivery, resume fallback, and process replacement remain deterministic. + +## In scope + +- Add process-scoped initial-prompt state to `AgentExecution`. +- Install it for original fresh launches and fresh-start resume fallbacks that + require stdin prompt delivery. +- Suppress matching completion callbacks until injection finishes or aborts. +- Clear only the captured process's marker on every injection exit path. +- Add focused lifecycle launch, completion, injection, failure, and replacement + regressions. + +## Out of scope + +- Agentctl detector changes and mid-turn idle detection. +- Orchestrator, workflow-engine, database, API, or frontend changes. +- Prompt formatting, submit sequences, and idle timeout tuning. + +## Acceptance + +- A startup completion signal for a process with pending initial injection + emits no `agent.ready` event and leaves the execution running. +- After injection clears the matching marker, one eligible signal uses the + existing Ready path. +- Abort and replacement cleanup cannot affect a different process. +- Prompt-flag, promptless, and resume paths never acquire the marker, while a + fresh-start fallback does. + +## Verification + +```bash +go test -tags fts5 ./internal/agent/runtime/lifecycle -run 'Test(HandlePassthroughTurnComplete|AutoInject|Passthrough.*Fallback)' -count=1 +go test -race -tags fts5 ./internal/agent/runtime/lifecycle -run 'Test(HandlePassthroughTurnComplete|AutoInject|Passthrough.*Fallback)' -count=1 +go test -tags fts5 ./internal/agent/runtime/lifecycle -count=1 +``` + +Run all commands from `apps/backend`. + +## Files likely touched + +- `apps/backend/internal/agent/runtime/lifecycle/types.go` +- `apps/backend/internal/agent/runtime/lifecycle/manager_passthrough.go` +- `apps/backend/internal/agent/runtime/lifecycle/manager_passthrough_test.go` +- `apps/backend/internal/agent/runtime/lifecycle/manager_passthrough_autoinject_test.go` +- `apps/backend/internal/agent/runtime/lifecycle/manager_passthrough_launch_test.go` +- `apps/backend/internal/agent/runtime/lifecycle/manager_passthrough_runtime_test.go` + +## Dependencies + +None. + +## Risks + +- Process lifecycle fields currently have mixed locking discipline. New marker + reads and writes must use the existing passthrough lifecycle lock without + holding it across event publication or PTY I/O. + +## Parallelism + +`sequential` + +## Inputs + +- `docs/specs/tasks/requirements/passthrough-initial-prompt-turn-boundary.md` +- `docs/specs/tasks/system-design/passthrough-initial-prompt-turn-boundary.md` +- `docs/decisions/2026-09-01-passthrough-initial-prompt-turn-boundary.md` +- `docs/specs/cli/requirements/cli-mode-parity.md` +- GitHub issue #3247 and the deterministic failing reproduction recorded in + Kandev task `28542e6c-c81b-4ba2-be6b-f46ee03b337d`. + +## Results + +- Added an in-memory marker that binds pending initial prompt delivery to one + passthrough process. +- Fresh launches and fresh-start fallbacks install the marker before later + startup work can publish a completion signal. +- Completion callbacks remain inactive until prompt injection finishes or + stops. Prompt-flag and resumed processes keep their existing behavior. +- Cleanup and PTY writes compare process identity. Work from an old process + cannot clear or write to its replacement. +- `go test -tags fts5 ./internal/agent/runtime/lifecycle -run 'Test(HandlePassthroughTurnComplete|AutoInject|Passthrough.*Fallback)' -count=1` + passed 16 tests. +- `go test -race -tags fts5 ./internal/agent/runtime/lifecycle -run 'Test(HandlePassthroughTurnComplete|AutoInject|Passthrough.*Fallback)' -count=1` + passed the same 16 tests. +- `go test -tags fts5 ./internal/agent/runtime/lifecycle -count=1` passed + 2,100 tests. diff --git a/docs/specs/tasks/README.md b/docs/specs/tasks/README.md index 0778df29e0..66f2974b7e 100644 --- a/docs/specs/tasks/README.md +++ b/docs/specs/tasks/README.md @@ -61,6 +61,7 @@ signals, and task-scoped scheduling contracts. - [Parent-Child Message Interrupt](requirements/parent-child-message-interrupt.md) - [Parent-Child Task Stop](requirements/parent-child-task-stop.md) - [Passthrough Queued Prompt Dispatch](requirements/passthrough-queued-prompt-dispatch.md) +- [Passthrough Initial Prompt Turn Boundary](requirements/passthrough-initial-prompt-turn-boundary.md) - [Prevent Agent Auto-Start On Open](requirements/prevent-agent-autostart-on-open.md) - [Prompt attachments](requirements/prompt-attachments.md) - [Quick Chat Sessions, Persistence, and Expiration](requirements/quick-chat-expiration.md) @@ -123,6 +124,7 @@ signals, and task-scoped scheduling contracts. - [MCP Tool Name Stability](system-design/mcp-tool-name-stability.md) - [Remote Contribution Tasks](system-design/remote-contribution-tasks.md) - [Passthrough Queued Prompt Dispatch](system-design/passthrough-queued-prompt-dispatch.md) +- [Passthrough Initial Prompt Turn Boundary](system-design/passthrough-initial-prompt-turn-boundary.md) - [Task Archive Confirmation](system-design/archive-confirmation.md) - [Task plan write lifecycle](system-design/plan-write-lifecycle.md) - [Task Runtime Cleanup](system-design/runtime-cleanup.md) diff --git a/docs/specs/tasks/requirements/passthrough-initial-prompt-turn-boundary.md b/docs/specs/tasks/requirements/passthrough-initial-prompt-turn-boundary.md new file mode 100644 index 0000000000..cb7cc1cd88 --- /dev/null +++ b/docs/specs/tasks/requirements/passthrough-initial-prompt-turn-boundary.md @@ -0,0 +1,62 @@ +--- +status: active +system: tasks +created: 2026-09-01 +owners: + - cfl +--- + +# Passthrough Initial Prompt Turn Boundary Requirements + +## Overview + +The task system owns session lifecycle state, turn completion, workflow +triggers, and downstream completion effects. A fresh CLI passthrough process +can become idle before Kandev injects its initial task prompt. That startup +readiness signal must not be interpreted as a completed agent turn. + +CLI passthrough owns the PTY transport and readiness detector. The task system +owns whether a detector signal is eligible to complete a turn. + +## Terminology + +- **Pending initial prompt:** A non-empty task prompt that Kandev must submit + through a fresh passthrough process's PTY because the agent does not receive + it at process launch. +- **Completion signal:** A passthrough prompt-pattern, status, or idle signal + that can normally mark an agent turn ready for follow-up work. + +## Requirements + +### REQ-TASKS-PASSTHROUGH-INITIAL-TURN-001: Initial prompt owns the first turn boundary + +**Intent:** A passthrough task shall remain active until its agent has received +and processed the initial prompt. + +#### Acceptance criteria + +- **AC-TASKS-PASSTHROUGH-INITIAL-TURN-001.1:** When a fresh passthrough process + has a pending initial prompt, completion signals received before Kandev + finishes submitting that prompt shall not complete a turn. These signals + shall not change session or task state, evaluate `on_turn_complete`, or invoke + other turn-completion consumers. +- **AC-TASKS-PASSTHROUGH-INITIAL-TURN-001.2:** After Kandev finishes submitting + the initial prompt, the next eligible completion signal shall complete the + initial turn through the ordinary passthrough completion path. +- **AC-TASKS-PASSTHROUGH-INITIAL-TURN-001.3:** A passthrough process that + receives its prompt at process launch or has no initial prompt shall retain + ordinary completion behavior. A resumed conversation without reinjection + shall use the same behavior. +- **AC-TASKS-PASSTHROUGH-INITIAL-TURN-001.4:** A fresh-start fallback that must + reinject the initial prompt shall use the same pending boundary as an + original fresh launch. +- **AC-TASKS-PASSTHROUGH-INITIAL-TURN-001.5:** When initial prompt submission + aborts, that process's pending boundary shall end. When its process is + replaced, the old boundary shall not suppress the replacement process or + later user work. + +## Out of scope + +- Replacing the passthrough idle heuristic for turns that have already started. +- Changing prompt formatting, submit sequences, or per-agent idle timeouts. +- Changing completion semantics for headless prompt-flag agents. diff --git a/docs/specs/tasks/system-design/passthrough-initial-prompt-turn-boundary.md b/docs/specs/tasks/system-design/passthrough-initial-prompt-turn-boundary.md new file mode 100644 index 0000000000..98f96071c3 --- /dev/null +++ b/docs/specs/tasks/system-design/passthrough-initial-prompt-turn-boundary.md @@ -0,0 +1,106 @@ +--- +status: current +system: tasks +requirements: + - REQ-TASKS-PASSTHROUGH-INITIAL-TURN-001 +--- + +# Passthrough Initial Prompt Turn Boundary System Design + +## Purpose and boundaries + +The task system owns whether a raw passthrough completion signal represents a +completed turn. The agentctl interactive runner continues to own PTY output, +prompt-pattern detection, status detection, and idle detection. The lifecycle +manager owns the delivery context needed to decide whether those raw signals +are eligible. + +The boundary applies only to fresh passthrough processes whose non-empty task +prompt must be injected through stdin. It does not change prompt-flag launches, +promptless tools, or resumed conversations. + +## Requirement mapping + +| Requirement | Design sections | +| --- | --- | +| `REQ-TASKS-PASSTHROUGH-INITIAL-TURN-001` | [Runtime contract](#runtime-contract), [Control flow](#control-flow), [Failure and recovery](#failure-and-recovery) | + +## Components and responsibilities + +- `process.InteractiveRunner` reports raw prompt-pattern, status, and idle + completion signals and exposes the first-idle wait used by prompt injection. +- `lifecycle.AgentExecution` owns an in-memory, process-scoped initial-prompt + marker protected with the passthrough process lifecycle state. +- `lifecycle.Manager.startPassthroughSession` and the fresh-start resume + fallback bind that marker to the newly active PTY process when stdin prompt + delivery is required. +- `lifecycle.Manager.autoInjectInitialPromptWith` waits for readiness, submits + all planned stdin chunks, and clears the matching marker on every exit path. +- `lifecycle.Manager.handlePassthroughTurnComplete` validates process identity + and suppresses completion while the active process owns the marker. Eligible + signals continue through `MarkReady` and the existing orchestrator path. + +## Runtime contract + +The pending marker contains the active passthrough process identity. It is +runtime-only state and is not persisted. A boolean without process identity is +insufficient. A delayed callback or cleanup from an old process must not affect +the replacement process's boundary. + +The marker exists only when all of these conditions hold: + +- the launch creates a fresh conversation, +- the agent has no `PromptFlag` delivery path, and +- the execution contains a non-empty task description for stdin injection. + +The marker does not emit `agent.boot_ready`, `agent.ready`, or +`agent.running`. The execution is already running while Kandev waits for the +startup idle and injects the prompt. + +## Control flow + +1. Lifecycle determines whether the fresh process needs stdin prompt delivery. +2. After the interactive runner returns the new process identity, lifecycle + installs that identity as the active process. Lifecycle installs the pending + initial-prompt owner under the same lock. +3. A first-idle, prompt-pattern, or status callback can wake prompt injection. + The completion handler observes the matching pending marker and returns + without calling `MarkReady`. +4. Prompt injection marks the execution running through the existing + idempotent path and writes every planned prompt and submit chunk. +5. Prompt injection clears the marker for that process after the final write. +6. Agent output resets the existing turn detector. The next eligible + completion callback finds no pending marker and follows the ordinary + `MarkReady` path, including workflow and queue consumers. + +Prompt-flag launches, promptless processes, and ordinary resumes never install +the marker, so their first completion signal remains eligible. + +## Failure and recovery + +- Prompt injection clears the matching marker when readiness times out, + shutdown interrupts delivery, or a PTY write fails. This avoids suppressing + later manual work indefinitely. +- Marker cleanup compares process identity. Cleanup from an old process cannot + clear a boundary installed for its replacement. +- A delayed completion callback from a replaced process continues to fail the + existing active-process identity check. +- A fresh-start resume fallback installs a new marker for its new process. A + successful resume does not install one because it does not reinject history. +- Mid-turn idle false positives remain a separate detector-accuracy concern. + +## Observability + +Lifecycle emits a debug diagnostic when it ignores a completion signal for a +process with a pending initial prompt. Existing timeout and write-failure +warnings continue to expose prompt injection failures. No persisted state or +new metric is required. + +Focused lifecycle tests cover startup suppression, ordinary completion after +submission, prompt-flag and promptless behavior, abort cleanup, fresh fallback, +and process replacement. + +## Related decisions + +- [Keep passthrough initial prompt state in lifecycle](../../../decisions/2026-09-01-passthrough-initial-prompt-turn-boundary.md) +- [Defer passthrough running publication until guard release](../../../decisions/2026-08-31-passthrough-running-publication.md)