Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions apps/backend/internal/orchestrator/session_ensure.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,23 @@ func acquireEnsureLock(taskID string) func() {
return mu.Unlock
}

// ensureLockHeldKey marks a context whose caller already owns the per-task
// ensureLocks mutex. launchStart consults it to skip a recursive acquire when
// EnsureSession re-enters via LaunchSession — sync.Mutex isn't reentrant, so
// without this the second Lock would deadlock the request.
type ensureLockHeldCtxKey struct{}

var ensureLockHeldKey = ensureLockHeldCtxKey{}

func withEnsureLockHeld(ctx context.Context) context.Context {
return context.WithValue(ctx, ensureLockHeldKey, true)
}

func ensureLockHeld(ctx context.Context) bool {
v, _ := ctx.Value(ensureLockHeldKey).(bool)
return v
}

// EnsureSession is the server-authoritative idempotent entry point for opening
// a task: it returns the existing primary (or newest) session if any, otherwise
// resolves the agent profile from the task's full context and creates a session
Expand All @@ -61,6 +78,7 @@ func (s *Service) EnsureSession(ctx context.Context, taskID string, opts ...Ensu
}
release := acquireEnsureLock(taskID)
defer release()
ctx = withEnsureLockHeld(ctx)

var o EnsureSessionOptions
if len(opts) > 0 {
Expand Down
84 changes: 84 additions & 0 deletions apps/backend/internal/orchestrator/session_launch.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,36 @@ func (s *Service) launchStart(ctx context.Context, req *LaunchSessionRequest) (*
return s.launchPrepare(ctx, req)
}

// Opening a task whose step lacks auto_start_agent calls session.ensure
// which creates a CREATED session via prepare. A subsequent explicit start
// (e.g. the edit-dialog "Start Agent" button) would otherwise create a
// duplicate session row and surface two tabs on a fresh task. Promote the
// prepared session to a start_created launch instead. ensureLocks is shared
// with EnsureSession so the two paths cannot race and both decide to create.
// EnsureSession holds the same mutex while re-entering through LaunchSession,
// so skip the acquire when its ctx marker is set — sync.Mutex isn't
// reentrant, and a recursive Lock would deadlock the request.
//
// Reuse is also skipped when the caller has explicitly chosen an executor
// or priority: start_created does not honor those, so adopting a prepared
// session would silently drop the caller's selection. Same logic applies
// when the caller's agent profile is set and disagrees with the prepared
// session's profile — that signals an intentionally different launch.
if s.canReusePreparedSession(req) {
if !ensureLockHeld(ctx) {
release := acquireEnsureLock(req.TaskID)
defer release()
}
if reused := s.findReusableCreatedSession(ctx, req.TaskID); reused != nil &&
profilesCompatible(req.AgentProfileID, reused.AgentProfileID) {
req.SessionID = reused.ID
if req.AgentProfileID == "" {
req.AgentProfileID = reused.AgentProfileID
}
return s.launchStartCreated(ctx, req)
}
}

execution, err := s.StartTask(
ctx, req.TaskID, req.AgentProfileID, req.ExecutorID,
req.ExecutorProfileID, req.Priority, req.Prompt,
Expand All @@ -181,6 +211,60 @@ func (s *Service) launchStart(ctx context.Context, req *LaunchSessionRequest) (*
return executionToLaunchResponse(req.TaskID, execution), nil
}

// canReusePreparedSession reports whether launchStart should consider adopting
// an existing CREATED session. Reuse is unsafe when the caller has explicitly
// chosen an executor or priority — start_created carries neither field, so
// reusing would silently drop the caller's selection.
func (s *Service) canReusePreparedSession(req *LaunchSessionRequest) bool {
return req.SessionID == "" &&
req.ExecutorID == "" &&
req.ExecutorProfileID == "" &&
req.Priority == ""
}

// profilesCompatible reports whether two agent profile IDs are compatible for
// reuse: identical, or at least one is empty (start_created can resolve the
// missing side from the other). Different non-empty profiles are intentionally
// distinct launches and must NOT share a session row.
func profilesCompatible(caller, session string) bool {
if caller == "" || session == "" {
return true
}
return caller == session
}

// findReusableCreatedSession returns a CREATED session on the task that an
// explicit start should adopt instead of creating a new row. Primary wins; the
// most-recently-updated CREATED session is the fallback. Non-CREATED states
// (RUNNING, WAITING_FOR_INPUT, COMPLETED, FAILED, CANCELLED) are skipped — they
// reflect either an active agent or an explicit terminal state and must not be
// implicitly revived from a fresh start request.
func (s *Service) findReusableCreatedSession(ctx context.Context, taskID string) *models.TaskSession {
sessions, err := s.repo.ListTaskSessions(ctx, taskID)
if err != nil {
// Returning nil here means launchStart still falls through to
// StartTask — same behavior as before this PR landed when the lookup
// failed. The warn just keeps a transient DB failure visible in the
// logs rather than letting it race silently into a fresh session.
s.logger.Warn("findReusableCreatedSession: list sessions failed; skipping reuse",
Comment thread
jcfs marked this conversation as resolved.
Comment thread
jcfs marked this conversation as resolved.
zap.String("task_id", taskID), zap.Error(err))
return nil
}
Comment thread
jcfs marked this conversation as resolved.
var best *models.TaskSession
for _, sess := range sessions {
if sess.State != models.TaskSessionStateCreated {
continue
}
if sess.IsPrimary {
return sess
}
if best == nil || sess.UpdatedAt.After(best.UpdatedAt) {
best = sess
}
}
return best
}

// shouldBlockAutoStart checks whether the task's workflow step allows auto-starting
// the agent. Returns true when the step exists but does not have auto_start_agent
// in its on_enter events. Tasks without a workflow step are never blocked.
Expand Down
Loading
Loading