dreampiler/kandev
1
0
Fork 0
Code Issues 43 Pull Requests Actions 6 Packages Projects Releases Wiki Activity

fix(orchestrator): deliver initial prompt to passthrough start_agent (#1306)

Browse Source
This commit is contained in:
Nicolas Lenepveu
2026-06-09 10:36:13 +02:00
committed by GitHub
parent 525e932df7
commit 63434f29cf
4 changed files with 225 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/backend/internal/orchestrator/session_launch.go
+40 -15
View File
@@ -27,20 +27,29 @@ const (
// LaunchSessionRequest is the unified request for session.launch.
type LaunchSessionRequest struct {
TaskID string `json:"task_id"`
Intent SessionIntent `json:"intent,omitempty"`
SessionID string `json:"session_id,omitempty"`
AgentProfileID string `json:"agent_profile_id,omitempty"`
ExecutorID string `json:"executor_id,omitempty"`
ExecutorProfileID string `json:"executor_profile_id,omitempty"`
Prompt string `json:"prompt,omitempty"`
PlanMode bool `json:"plan_mode,omitempty"`
WorkflowStepID string `json:"workflow_step_id,omitempty"`
Priority string `json:"priority,omitempty"`
LaunchWorkspace bool `json:"launch_workspace,omitempty"`
SkipMessageRecord bool `json:"skip_message_record,omitempty"`
AutoStart bool `json:"auto_start,omitempty"`
Attachments []v1.MessageAttachment `json:"attachments,omitempty"`
TaskID string `json:"task_id"`
Intent SessionIntent `json:"intent,omitempty"`
SessionID string `json:"session_id,omitempty"`
AgentProfileID string `json:"agent_profile_id,omitempty"`
ExecutorID string `json:"executor_id,omitempty"`
ExecutorProfileID string `json:"executor_profile_id,omitempty"`
Prompt string `json:"prompt,omitempty"`
PlanMode bool `json:"plan_mode,omitempty"`
WorkflowStepID string `json:"workflow_step_id,omitempty"`
Priority string `json:"priority,omitempty"`
LaunchWorkspace bool `json:"launch_workspace,omitempty"`
SkipMessageRecord bool `json:"skip_message_record,omitempty"`
AutoStart bool `json:"auto_start,omitempty"`
// DeferredStart marks a prepare whose caller will follow up with an explicit
// IntentStartCreated that carries the prompt (the two-phase create flow:
// cheap sync prepare + async start). It suppresses the passthrough
// launchPrepare→launchStart upgrade so the eager launch doesn't spawn a
// promptless PTY and pre-empt the prompt-bearing start. It is an internal
// server-side coordination flag set by the deferred-start handlers, so it is
// kept off the wire protocol (`json:"-"`) — a client must not be able to
// suppress the upgrade and strand a passthrough session without a PTY.
DeferredStart bool `json:"-"`
Attachments []v1.MessageAttachment `json:"attachments,omitempty"`
}
// LaunchSessionResponse is the unified response for session.launch.
@@ -105,8 +114,14 @@ func (s *Service) LaunchSession(ctx context.Context, req *LaunchSessionRequest)
// AutoStart=true means we arrived here from launchStart's blocked-auto-start
// downgrade path; skipping the upgrade in that case avoids a launchStart ↔
// launchPrepare bounce.
//
// DeferredStart=true means a prompt-bearing IntentStartCreated will follow this
// prepare (the two-phase create flow); skipping the upgrade there leaves the
// session CREATED so that follow-up start launches the passthrough agent WITH
// the prompt — eagerly launching here would spawn a promptless PTY and the
// later start would be rejected against the now-running session.
func (s *Service) launchPrepare(ctx context.Context, req *LaunchSessionRequest) (*LaunchSessionResponse, error) {
if !req.AutoStart && s.isPassthroughProfile(ctx, req.AgentProfileID) {
if s.shouldUpgradePassthroughPrepare(ctx, req) {
return s.launchStart(ctx, req)
}
sessionID, err := s.PrepareTaskSession(
@@ -124,6 +139,16 @@ func (s *Service) launchPrepare(ctx context.Context, req *LaunchSessionRequest)
}, nil
}
// shouldUpgradePassthroughPrepare reports whether a prepare request for a
// passthrough profile should be eagerly upgraded to a full launch so a PTY
// exists for the terminal to attach to. It is the single decision point for the
// upgrade documented on launchPrepare: only genuine prepare-only callers (no
// imminent prompt-bearing start) get the eager launch. See launchPrepare for
// why AutoStart and DeferredStart each suppress it.
func (s *Service) shouldUpgradePassthroughPrepare(ctx context.Context, req *LaunchSessionRequest) bool {
return !req.AutoStart && !req.DeferredStart && s.isPassthroughProfile(ctx, req.AgentProfileID)
}
func (s *Service) isPassthroughProfile(ctx context.Context, profileID string) bool {
if profileID == "" || s.agentManager == nil {
return false
apps/backend/internal/orchestrator/session_launch_test.go
+57
View File
@@ -295,6 +295,63 @@ func TestIsPassthroughProfile(t *testing.T) {
})
}
// TestShouldUpgradePassthroughPrepare pins the launchPrepare upgrade decision.
//
// Regression guard for the prompt-delivery break introduced by the passthrough
// upgrade (PR #744): the two-phase create flow does a cheap prompt-less prepare
// followed by an async IntentStartCreated that carries the prompt. If a
// passthrough prepare is eagerly upgraded to a full launch there, the PTY spawns
// with an empty prompt and the prompt-bearing start is rejected against the
// now-running session — so the agent sits at an empty prompt. DeferredStart must
// suppress the upgrade so that follow-up start launches the passthrough WITH the
// prompt, exactly as ACP already does.
func TestShouldUpgradePassthroughPrepare(t *testing.T) {
repo := setupTestRepo(t)
passthrough := &mockAgentManager{isPassthrough: true}
regular := &mockAgentManager{isPassthrough: false}
tests := []struct {
name string
mgr *mockAgentManager
req LaunchSessionRequest
want bool
}{
{
name: "prepare-only passthrough upgrades (quick-chat terminal needs a PTY)",
mgr: passthrough,
req: LaunchSessionRequest{AgentProfileID: "profile1"},
want: true,
},
{
name: "deferred-start passthrough does NOT upgrade (prompt-bearing start follows)",
mgr: passthrough,
req: LaunchSessionRequest{AgentProfileID: "profile1", DeferredStart: true},
want: false,
},
{
name: "auto-start passthrough does NOT upgrade (avoids launchStart bounce)",
mgr: passthrough,
req: LaunchSessionRequest{AgentProfileID: "profile1", AutoStart: true},
want: false,
},
{
name: "non-passthrough never upgrades",
mgr: regular,
req: LaunchSessionRequest{AgentProfileID: "profile1"},
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
svc := createTestServiceWithAgent(repo, newMockStepGetter(), newMockTaskRepo(), tt.mgr)
if got := svc.shouldUpgradePassthroughPrepare(context.Background(), &tt.req); got != tt.want {
t.Errorf("shouldUpgradePassthroughPrepare()=%v, want %v", got, tt.want)
}
})
}
}
// TestLaunchPrepare_PassthroughDoesNotRecurse guards against an infinite
// launchStart ↔ launchPrepare bounce when a passthrough profile is combined
// with AutoStart=true and a step that blocks auto-start.
apps/backend/internal/task/handlers/task_http_handlers.go
+14
View File
@@ -777,6 +777,10 @@ func (h *TaskHandlers) handlePostCreateTaskSession(
return
}
if body.PrepareSession && !body.StartAgent {
// Prepare-only: no follow-up start is coming, so DeferredStart is
// intentionally omitted — a passthrough profile should be eagerly
// upgraded to a full launch here so the terminal has a PTY to attach to.
// (Contrast startAgentForNewTask below, which sets DeferredStart=true.)
resp, err := h.orchestrator.LaunchSession(c.Request.Context(), &orchestrator.LaunchSessionRequest{
TaskID: taskID,
Intent: orchestrator.IntentPrepare,
@@ -816,6 +820,10 @@ func (h *TaskHandlers) startAgentForNewTask(
ExecutorID: body.ExecutorID,
ExecutorProfileID: body.ExecutorProfileID,
WorkflowStepID: resolvedStepID,
// The async IntentStartCreated below carries the prompt. Mark this as a
// deferred start so a passthrough profile is not eagerly launched here
// with an empty prompt (which would pre-empt that prompt-bearing start).
DeferredStart: true,
})
if err != nil {
h.logger.Error("failed to prepare session for task", zap.Error(err), zap.String("task_id", taskID))
@@ -1314,6 +1322,12 @@ func (h *TaskHandlers) httpStartConfigChat(c *gin.Context) {
Intent: orchestrator.IntentPrepare,
AgentProfileID: agentProfileID,
ExecutorID: executorID,
// When a prompt is present, launchConfigChatAgent below follows with a
// prompt-bearing IntentStartCreated. Defer the start so a passthrough
// profile isn't eagerly launched here with an empty prompt. With no
// prompt there is no follow-up, so keep the eager upgrade that gives the
// terminal a PTY to attach to.
DeferredStart: body.Prompt != "",
})
if err != nil {
h.deleteTaskOnError(task.ID, "config chat", err)
apps/backend/internal/task/handlers/task_http_handlers_test.go
+114
View File
@@ -5,6 +5,7 @@ import (
"errors"
"net/http"
"net/http/httptest"
"strconv"
"strings"
"sync"
"testing"
@@ -15,10 +16,123 @@ import (
"github.com/stretchr/testify/require"
"github.com/kandev/kandev/internal/common/logger"
"github.com/kandev/kandev/internal/orchestrator"
"github.com/kandev/kandev/internal/task/models"
"github.com/kandev/kandev/internal/task/service"
)
// captureOrchestrator records every LaunchSession request so tests can assert
// on the fields the handler set. prepErr, when non-nil, is returned from
// LaunchSession to short-circuit the two-phase create flow before its async
// start goroutine spawns (keeping assertions race-free).
type captureOrchestrator struct {
mu sync.Mutex
requests []*orchestrator.LaunchSessionRequest
prepErr error
}
func (m *captureOrchestrator) LaunchSession(_ context.Context, req *orchestrator.LaunchSessionRequest) (*orchestrator.LaunchSessionResponse, error) {
m.mu.Lock()
m.requests = append(m.requests, req)
m.mu.Unlock()
if m.prepErr != nil {
return nil, m.prepErr
}
return &orchestrator.LaunchSessionResponse{SessionID: "sess-1"}, nil
}
func (m *captureOrchestrator) EnsureSession(_ context.Context, _ string, _ ...orchestrator.EnsureSessionOptions) (*orchestrator.EnsureSessionResponse, error) {
return nil, nil
}
// TestStartAgentForNewTask_SetsDeferredStart pins the call-site half of the
// passthrough start_agent prompt-delivery fix: the synchronous prepare must
// carry DeferredStart=true so launchPrepare does not eagerly upgrade a
// passthrough profile into a promptless PTY launch and pre-empt the
// prompt-bearing IntentStartCreated that follows. Returning an error from the
// prepare call keeps the async start goroutine from spawning, so the assertion
// reads orch.requests without racing it.
func TestStartAgentForNewTask_SetsDeferredStart(t *testing.T) {
orch := &captureOrchestrator{prepErr: errors.New("prepare failed")}
h := &TaskHandlers{orchestrator: orch, logger: newTestLogger(t)}
resp := &createTaskResponse{}
body := httpCreateTaskRequest{StartAgent: true, AgentProfileID: "profile-1"}
h.startAgentForNewTask(context.Background(), resp, "task-1", "do the thing", body, "step-1")
orch.mu.Lock()
defer orch.mu.Unlock()
require.Len(t, orch.requests, 1, "prepare error must short-circuit before the async start goroutine")
prep := orch.requests[0]
assert.Equal(t, orchestrator.IntentPrepare, prep.Intent)
assert.True(t, prep.DeferredStart,
"sync prepare must defer the start so the passthrough PTY is launched with the prompt by the follow-up IntentStartCreated")
}
// configChatRepo returns a non-nil workspace so resolveConfigChatDefaults does
// not nil-deref; everything else inherits mockRepository's no-op stubs.
type configChatRepo struct {
mockRepository
}
func (r *configChatRepo) GetWorkspace(_ context.Context, id string) (*models.Workspace, error) {
return &models.Workspace{ID: id}, nil
}
// TestHttpStartConfigChat_SetsDeferredStart pins the second call site of the
// passthrough prompt-delivery fix. Unlike start_agent (always deferred), config
// chat defers the start only when a prompt is present — with no prompt there is
// no follow-up start, so the passthrough upgrade must stay on to give the
// terminal a PTY. Both branches are load-bearing, so both are pinned. The prepare
// LaunchSession is invoked synchronously before the async launchConfigChatAgent
// goroutine spawns, so requests[0] is the prepare and is read under the mock's
// mutex — race-free without asserting the (timing-dependent) total count.
func TestHttpStartConfigChat_SetsDeferredStart(t *testing.T) {
gin.SetMode(gin.TestMode)
log := newTestLogger(t)
cases := []struct {
name string
prompt string
wantDeferredStart bool
}{
{name: "prompt present defers the start", prompt: "configure my workflow", wantDeferredStart: true},
{name: "no prompt keeps the eager PTY upgrade", prompt: "", wantDeferredStart: false},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
repo := &configChatRepo{}
svc := service.NewService(service.Repos{
Workspaces: repo, Tasks: repo, TaskRepos: repo,
Workflows: repo, Messages: repo, Turns: repo,
Sessions: repo, GitSnapshots: repo, RepoEntities: repo,
Executors: repo, Environments: repo, TaskEnvironments: repo,
Reviews: repo,
}, nil, log, service.RepositoryDiscoveryConfig{})
orch := &captureOrchestrator{}
h := &TaskHandlers{service: svc, orchestrator: orch, logger: log}
rec := httptest.NewRecorder()
c, _ := gin.CreateTestContext(rec)
body := `{"agent_profile_id":"cfg-profile","prompt":` + strconv.Quote(tc.prompt) + `}`
c.Request = httptest.NewRequest(http.MethodPost, "/workspaces/ws-1/config-chat", strings.NewReader(body))
c.Request.Header.Set("Content-Type", "application/json")
c.Params = gin.Params{{Key: "id", Value: "ws-1"}}
h.httpStartConfigChat(c)
orch.mu.Lock()
defer orch.mu.Unlock()
require.GreaterOrEqual(t, len(orch.requests), 1, "the synchronous prepare must have been issued")
prep := orch.requests[0]
assert.Equal(t, orchestrator.IntentPrepare, prep.Intent, "first call is the synchronous prepare")
assert.Equal(t, tc.wantDeferredStart, prep.DeferredStart,
"config-chat prepare must defer the start iff a prompt will be delivered by the follow-up IntentStartCreated")
})
}
}
type captureCreateTaskRepo struct {
mockRepository
captured *models.Task