fix: improve session failure recovery UX (#1866)
This commit is contained in:
@@ -6,6 +6,8 @@ import (
|
||||
"fmt"
|
||||
"os"
|
||||
"os/exec"
|
||||
"regexp"
|
||||
"sort"
|
||||
"strings"
|
||||
"sync"
|
||||
"time"
|
||||
@@ -106,7 +108,7 @@ func (p *LocalPreparer) Prepare(ctx context.Context, req *EnvPrepareRequest, onP
|
||||
step = beginStep("Checkout branch")
|
||||
step.Command = fmt.Sprintf("git fetch origin %s && git checkout %s", effectiveBranch, effectiveBranch)
|
||||
reportProgress(onProgress, step, stepIdx, totalSteps)
|
||||
output, err := checkoutBranch(ctx, workspacePath, effectiveBranch)
|
||||
output, err := checkoutBranch(ctx, workspacePath, effectiveBranch, gitCredentialValues(req.Env))
|
||||
if err != nil {
|
||||
errMsg := fmt.Sprintf("failed to checkout branch %q: %s", effectiveBranch, output)
|
||||
completeStepError(&step, errMsg)
|
||||
@@ -171,21 +173,62 @@ func readCurrentBranchForLocal(workDir string) string {
|
||||
// Best-effort fetch first so newly-created remote branches are visible, then
|
||||
// the checkout. If the local branch doesn't exist but the remote tracking
|
||||
// branch does (from the fetch), git creates a local branch tracking it.
|
||||
func checkoutBranch(ctx context.Context, workDir, branch string) (string, error) {
|
||||
func checkoutBranch(ctx context.Context, workDir, branch string, sensitiveValues []string) (string, error) {
|
||||
fetchCmd := exec.CommandContext(ctx, "git", "fetch", "origin", branch)
|
||||
fetchCmd.Dir = workDir
|
||||
_ = subproc.RunGit(ctx, fetchCmd) // fetch failure is non-fatal, fall back to local
|
||||
fetchOut, fetchErr := subproc.RunGitCombinedOutput(ctx, fetchCmd)
|
||||
|
||||
cmd := exec.CommandContext(ctx, "git", "checkout", branch)
|
||||
cmd.Dir = workDir
|
||||
out, err := subproc.RunGitCombinedOutput(ctx, cmd)
|
||||
outStr := strings.TrimSpace(string(out))
|
||||
outStr := redactCheckoutOutput(strings.TrimSpace(string(out)), sensitiveValues)
|
||||
if err != nil {
|
||||
if fetchErr != nil {
|
||||
fetchDetail := redactCheckoutOutput(strings.TrimSpace(string(fetchOut)), sensitiveValues)
|
||||
outStr = fmt.Sprintf("fetch branch failed: %v: %s\ncheckout branch failed: %s", fetchErr, fetchDetail, outStr)
|
||||
}
|
||||
return outStr, worktree.ClassifyGitError(outStr, err)
|
||||
}
|
||||
return outStr, nil
|
||||
}
|
||||
|
||||
var credentialURLPattern = regexp.MustCompile(`(?i)(https?://)[^\s/@]+@`)
|
||||
|
||||
// redactCheckoutOutput removes credentials from git diagnostics before they
|
||||
// are persisted or shown to a user. Git commonly echoes the configured remote
|
||||
// URL on fetch failures, and local executor remotes may contain URL userinfo.
|
||||
func redactCheckoutOutput(output string, sensitiveValues []string) string {
|
||||
redacted := credentialURLPattern.ReplaceAllString(output, "${1}[REDACTED]@")
|
||||
for _, value := range sensitiveValues {
|
||||
if value != "" {
|
||||
redacted = strings.ReplaceAll(redacted, value, "[REDACTED]")
|
||||
}
|
||||
}
|
||||
return redacted
|
||||
}
|
||||
|
||||
// gitCredentialValues returns request environment values that may authenticate
|
||||
// git operations. Longest values go first so overlapping values are fully
|
||||
// replaced rather than leaving a suffix exposed.
|
||||
func gitCredentialValues(env map[string]string) []string {
|
||||
values := make([]string, 0)
|
||||
for key, value := range env {
|
||||
if value != "" && isSensitiveGitEnvKey(key) {
|
||||
values = append(values, value)
|
||||
}
|
||||
}
|
||||
sort.Slice(values, func(i, j int) bool { return len(values[i]) > len(values[j]) })
|
||||
return values
|
||||
}
|
||||
|
||||
func isSensitiveGitEnvKey(key string) bool {
|
||||
key = strings.ToUpper(key)
|
||||
return strings.Contains(key, "TOKEN") ||
|
||||
strings.Contains(key, "SECRET") ||
|
||||
strings.Contains(key, "PASSWORD") ||
|
||||
strings.HasSuffix(key, "_KEY")
|
||||
}
|
||||
|
||||
// setupScriptStreamInterval is the minimum gap between streaming-output callbacks
|
||||
// while a setup script runs. Chatty scripts (e.g. npm install with progress bars)
|
||||
// can emit hundreds of writes per second; throttling keeps WS event volume sane
|
||||
|
||||
@@ -327,6 +327,49 @@ func TestLocalPreparer_CheckoutFailureSurfaces(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestLocalPreparer_PreservesFetchFailureWhenCheckoutAlsoFails(t *testing.T) {
|
||||
const token = "github-token-that-must-not-leak"
|
||||
fakeBin := t.TempDir()
|
||||
fakeGit := filepath.Join(fakeBin, "git")
|
||||
script := `#!/bin/sh
|
||||
case "$1" in
|
||||
fetch)
|
||||
echo "fatal: unable to access 'https://oauth2:github-token-that-must-not-leak@github.com/kdlbs/kandev.git/': Could not resolve host: github.com" >&2
|
||||
exit 128
|
||||
;;
|
||||
checkout)
|
||||
echo "error: pathspec '$2' did not match any file(s) known to git; token=github-token-that-must-not-leak" >&2
|
||||
exit 1
|
||||
;;
|
||||
esac
|
||||
`
|
||||
if err := os.WriteFile(fakeGit, []byte(script), 0o755); err != nil {
|
||||
t.Fatalf("write fake git: %v", err)
|
||||
}
|
||||
t.Setenv("PATH", fakeBin+string(os.PathListSeparator)+os.Getenv("PATH"))
|
||||
|
||||
preparer := NewLocalPreparer(newTestLocalLogger())
|
||||
result, err := preparer.Prepare(context.Background(), &EnvPrepareRequest{
|
||||
TaskID: "task-1",
|
||||
RepositoryPath: t.TempDir(),
|
||||
CheckoutBranch: "feature/missing",
|
||||
Env: map[string]string{"GITHUB_TOKEN": token},
|
||||
}, nil)
|
||||
if err == nil {
|
||||
t.Fatal("expected Prepare() to fail")
|
||||
}
|
||||
for _, want := range []string{"fetch branch failed", "Could not resolve host: github.com", "pathspec 'feature/missing' did not match", "[REDACTED]"} {
|
||||
if !strings.Contains(result.ErrorMessage, want) || !strings.Contains(err.Error(), want) {
|
||||
t.Fatalf("expected result and error to contain %q\nresult: %s\nerror: %v", want, result.ErrorMessage, err)
|
||||
}
|
||||
}
|
||||
for _, secret := range []string{"oauth2", token} {
|
||||
if strings.Contains(result.ErrorMessage, secret) || strings.Contains(err.Error(), secret) {
|
||||
t.Fatalf("checkout failure leaked %q\nresult: %s\nerror: %v", secret, result.ErrorMessage, err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestLocalPreparer_RunsSetupScriptWithoutCheckout(t *testing.T) {
|
||||
isolateGitEnv(t)
|
||||
log := newTestLocalLogger()
|
||||
|
||||
@@ -22,8 +22,9 @@ type blockingAgentProfileResolver struct {
|
||||
}
|
||||
|
||||
type cancellableAgentProfileResolver struct {
|
||||
entered chan struct{}
|
||||
release chan struct{}
|
||||
entered chan struct{}
|
||||
cancelled chan struct{}
|
||||
release chan struct{}
|
||||
}
|
||||
|
||||
func (r *cancellableAgentProfileResolver) ResolveProfile(
|
||||
@@ -33,6 +34,8 @@ func (r *cancellableAgentProfileResolver) ResolveProfile(
|
||||
close(r.entered)
|
||||
select {
|
||||
case <-ctx.Done():
|
||||
close(r.cancelled)
|
||||
<-r.release
|
||||
return nil, ctx.Err()
|
||||
case <-r.release:
|
||||
return &AgentProfileInfo{AgentID: "mock-agent"}, nil
|
||||
@@ -140,7 +143,7 @@ func TestTerminalInvalidationCancelsAndDrainsAdmittedStart(t *testing.T) {
|
||||
coordinator := activity.NewCoordinator(activity.Options{})
|
||||
mgr.SetActivityCoordinator(coordinator)
|
||||
resolver := &cancellableAgentProfileResolver{
|
||||
entered: make(chan struct{}), release: make(chan struct{}),
|
||||
entered: make(chan struct{}), cancelled: make(chan struct{}), release: make(chan struct{}),
|
||||
}
|
||||
mgr.profileResolver = resolver
|
||||
if err := mgr.executionStore.Add(&AgentExecution{
|
||||
@@ -153,6 +156,7 @@ func TestTerminalInvalidationCancelsAndDrainsAdmittedStart(t *testing.T) {
|
||||
go func() { result <- mgr.StartAgentProcess(context.Background(), "exec-invalidated") }()
|
||||
<-resolver.entered
|
||||
mgr.releaseActivity(executionActivityKey("exec-invalidated"))
|
||||
<-resolver.cancelled
|
||||
maintenance, _, maintenanceErr := coordinator.TryAcquireMaintenance(context.Background(), 0)
|
||||
if maintenance != nil {
|
||||
maintenance.Release()
|
||||
@@ -162,6 +166,7 @@ func TestTerminalInvalidationCancelsAndDrainsAdmittedStart(t *testing.T) {
|
||||
<-result
|
||||
t.Fatalf("maintenance error = %v, want ErrBusy until invalidated start drains", maintenanceErr)
|
||||
}
|
||||
close(resolver.release)
|
||||
if err := <-result; !errors.Is(err, errExecutionActivityInvalidated) {
|
||||
t.Fatalf("StartAgentProcess error = %v, want claim invalidation", err)
|
||||
}
|
||||
|
||||
@@ -477,6 +477,25 @@ func (s *Service) shouldDropCompletedExecutionStreamEvent(payload *lifecycle.Age
|
||||
// Returns the session row after a successful write (refreshed from DB when possible); callers
|
||||
// that need authoritative UpdatedAt should use the return value, not the preloaded input.
|
||||
func (s *Service) updateTaskSessionState(ctx context.Context, taskID, sessionID string, nextState models.TaskSessionState, errorMessage string, allowWakeFromWaiting bool, preloadedSession ...*models.TaskSession) *models.TaskSession {
|
||||
updated, _ := s.updateTaskSessionStateWithHook(
|
||||
ctx, taskID, sessionID, nextState, errorMessage, allowWakeFromWaiting, nil, preloadedSession...,
|
||||
)
|
||||
return updated
|
||||
}
|
||||
|
||||
// updateTaskSessionStateWithHook is updateTaskSessionState with an optional
|
||||
// callback that runs only after the state CAS succeeds and before the state
|
||||
// change is published. It lets a caller attach state-specific UI metadata
|
||||
// without creating it when a concurrent terminal transition won the race.
|
||||
func (s *Service) updateTaskSessionStateWithHook(
|
||||
ctx context.Context,
|
||||
taskID, sessionID string,
|
||||
nextState models.TaskSessionState,
|
||||
errorMessage string,
|
||||
allowWakeFromWaiting bool,
|
||||
onChanged func(),
|
||||
preloadedSession ...*models.TaskSession,
|
||||
) (*models.TaskSession, bool) {
|
||||
var session *models.TaskSession
|
||||
if len(preloadedSession) > 0 && preloadedSession[0] != nil {
|
||||
session = preloadedSession[0]
|
||||
@@ -484,25 +503,28 @@ func (s *Service) updateTaskSessionState(ctx context.Context, taskID, sessionID
|
||||
var err error
|
||||
session, err = s.repo.GetTaskSession(ctx, sessionID)
|
||||
if err != nil {
|
||||
return nil
|
||||
return nil, false
|
||||
}
|
||||
}
|
||||
if session.State == models.TaskSessionStateWaitingForInput && nextState == models.TaskSessionStateRunning && !allowWakeFromWaiting {
|
||||
return session
|
||||
return session, false
|
||||
}
|
||||
oldState := session.State
|
||||
switch session.State {
|
||||
case models.TaskSessionStateCompleted, models.TaskSessionStateFailed, models.TaskSessionStateCancelled:
|
||||
return session
|
||||
return session, false
|
||||
}
|
||||
if session.State == nextState {
|
||||
return session
|
||||
return session, false
|
||||
}
|
||||
session, authoritativeUpdatedAt, changed := s.persistTaskSessionState(
|
||||
ctx, sessionID, session, nextState, errorMessage,
|
||||
)
|
||||
if !changed {
|
||||
return session
|
||||
return session, false
|
||||
}
|
||||
if onChanged != nil {
|
||||
onChanged()
|
||||
}
|
||||
if authoritativeUpdatedAt == nil {
|
||||
s.logger.Warn("skipping session state_changed publish; could not read authoritative updated_at",
|
||||
@@ -520,7 +542,7 @@ func (s *Service) updateTaskSessionState(ctx context.Context, taskID, sessionID
|
||||
|
||||
// Auto-promote another session to primary when the current primary enters a terminal state
|
||||
s.maybePromotePrimary(ctx, taskID, sessionID, nextState)
|
||||
return session
|
||||
return session, true
|
||||
}
|
||||
|
||||
func (s *Service) persistTaskSessionState(
|
||||
@@ -590,6 +612,7 @@ func (s *Service) transitionTaskSessionState(
|
||||
taskID, sessionID string,
|
||||
nextState models.TaskSessionState,
|
||||
errorMessage string,
|
||||
onChanged func(),
|
||||
) (bool, models.TaskSessionState, error) {
|
||||
session, err := s.repo.GetTaskSession(ctx, sessionID)
|
||||
if err != nil {
|
||||
@@ -612,6 +635,9 @@ func (s *Service) transitionTaskSessionState(
|
||||
if !changed {
|
||||
return false, refreshed.State, nil
|
||||
}
|
||||
if onChanged != nil {
|
||||
onChanged()
|
||||
}
|
||||
s.publishTaskSessionStateChanged(
|
||||
ctx,
|
||||
taskID,
|
||||
|
||||
@@ -266,6 +266,7 @@ func TestTransitionTaskSessionStateReportsAcceptedWrite(t *testing.T) {
|
||||
"s1",
|
||||
models.TaskSessionStateCancelled,
|
||||
"coordinator stop",
|
||||
nil,
|
||||
)
|
||||
|
||||
require.NoError(t, err)
|
||||
@@ -291,6 +292,7 @@ func TestTransitionTaskSessionStateReportsPersistenceFailure(t *testing.T) {
|
||||
"s1",
|
||||
models.TaskSessionStateCancelled,
|
||||
"coordinator stop",
|
||||
nil,
|
||||
)
|
||||
|
||||
require.ErrorIs(t, err, writeFailure)
|
||||
|
||||
@@ -551,11 +551,16 @@ type TaskRuntimeStateReconcileFunc func(ctx context.Context, taskID, sessionID s
|
||||
// publish events (e.g. WebSocket notifications) alongside the DB update.
|
||||
type SessionStateChangeFunc func(ctx context.Context, taskID, sessionID string, state models.TaskSessionState, errorMessage string) error
|
||||
|
||||
// SessionStateTransitionFunc performs a guarded session-state transition and
|
||||
// reports whether the requested state was accepted plus the final observed
|
||||
// state. Coordinator stop uses this stricter callback so terminal races and
|
||||
// persistence failures cannot be mistaken for accepted cancellation.
|
||||
type SessionStateTransitionFunc func(ctx context.Context, taskID, sessionID string, state models.TaskSessionState, errorMessage string) (changed bool, finalState models.TaskSessionState, err error)
|
||||
// SessionStateTransitionFunc performs a guarded session-state transition,
|
||||
// invokes onChanged after persistence but before publication, and reports the
|
||||
// final observed state.
|
||||
type SessionStateTransitionFunc func(
|
||||
ctx context.Context,
|
||||
taskID, sessionID string,
|
||||
state models.TaskSessionState,
|
||||
errorMessage string,
|
||||
onChanged func(),
|
||||
) (changed bool, finalState models.TaskSessionState, err error)
|
||||
|
||||
// SessionStartingFunc is called when the executor has prepared/resumed an
|
||||
// execution and needs to mark the session STARTING while preserving other
|
||||
|
||||
@@ -391,9 +391,19 @@ func (e *Executor) transitionSessionState(
|
||||
taskID, sessionID string,
|
||||
state models.TaskSessionState,
|
||||
errorMessage string,
|
||||
) (bool, models.TaskSessionState, error) {
|
||||
return e.transitionSessionStateWithHook(ctx, taskID, sessionID, state, errorMessage, nil)
|
||||
}
|
||||
|
||||
func (e *Executor) transitionSessionStateWithHook(
|
||||
ctx context.Context,
|
||||
taskID, sessionID string,
|
||||
state models.TaskSessionState,
|
||||
errorMessage string,
|
||||
onChanged func(),
|
||||
) (bool, models.TaskSessionState, error) {
|
||||
if e.onSessionStateTransition != nil {
|
||||
return e.onSessionStateTransition(ctx, taskID, sessionID, state, errorMessage)
|
||||
return e.onSessionStateTransition(ctx, taskID, sessionID, state, errorMessage, onChanged)
|
||||
}
|
||||
|
||||
current, err := e.repo.GetTaskSession(ctx, sessionID)
|
||||
@@ -419,6 +429,9 @@ func (e *Executor) transitionSessionState(
|
||||
if refreshed.State != state {
|
||||
return false, refreshed.State, nil
|
||||
}
|
||||
if onChanged != nil {
|
||||
onChanged()
|
||||
}
|
||||
return true, refreshed.State, nil
|
||||
}
|
||||
|
||||
@@ -976,20 +989,26 @@ func (e *Executor) handleLaunchFailure(ctx context.Context, taskID, sessionID, r
|
||||
e.logger.Error("failed to launch agent",
|
||||
zap.String("task_id", taskID),
|
||||
zap.Error(launchErr))
|
||||
// Call onLaunchFailed before state updates so it can set the suppressToast
|
||||
// flag that updateSessionState will propagate to the frontend.
|
||||
var onChanged func()
|
||||
if e.onLaunchFailed != nil {
|
||||
e.onLaunchFailed(failCtx, taskID, sessionID, repositoryID, launchErr)
|
||||
onChanged = func() {
|
||||
e.onLaunchFailed(failCtx, taskID, sessionID, repositoryID, launchErr)
|
||||
}
|
||||
}
|
||||
if updateErr := e.updateSessionState(failCtx, taskID, sessionID, models.TaskSessionStateFailed, launchErr.Error()); updateErr != nil {
|
||||
changed, _, updateErr := e.transitionSessionStateWithHook(
|
||||
failCtx, taskID, sessionID, models.TaskSessionStateFailed, launchErr.Error(), onChanged,
|
||||
)
|
||||
if updateErr != nil {
|
||||
e.logger.Warn("failed to mark session as failed after launch error",
|
||||
zap.String("session_id", sessionID),
|
||||
zap.Error(updateErr))
|
||||
}
|
||||
if updateErr := e.updateTaskState(failCtx, taskID, v1.TaskStateFailed); updateErr != nil {
|
||||
e.logger.Warn("failed to mark task as failed after launch error",
|
||||
zap.String("task_id", taskID),
|
||||
zap.Error(updateErr))
|
||||
if changed {
|
||||
if updateErr := e.updateTaskState(failCtx, taskID, v1.TaskStateFailed); updateErr != nil {
|
||||
e.logger.Warn("failed to mark task as failed after launch error",
|
||||
zap.String("task_id", taskID),
|
||||
zap.Error(updateErr))
|
||||
}
|
||||
}
|
||||
return launchErr
|
||||
}
|
||||
|
||||
@@ -1510,6 +1510,7 @@ func TestHandleAgentProcessStartFailure_CancellationDuringCallbackStopsUnclaimed
|
||||
string,
|
||||
models.TaskSessionState,
|
||||
string,
|
||||
func(),
|
||||
) (bool, models.TaskSessionState, error) {
|
||||
transitionCalls.Add(1)
|
||||
return false, models.TaskSessionStateCancelled, nil
|
||||
|
||||
@@ -1543,9 +1543,24 @@ func isMissingBranchError(err error) bool {
|
||||
return false
|
||||
}
|
||||
msg := strings.ToLower(err.Error())
|
||||
return strings.Contains(msg, "couldn't find remote ref") ||
|
||||
(strings.Contains(msg, "not found locally or on remote") && strings.Contains(msg, "branch")) ||
|
||||
(strings.Contains(msg, "pathspec") && strings.Contains(msg, "did not match"))
|
||||
if strings.Contains(msg, "couldn't find remote ref") {
|
||||
return true
|
||||
}
|
||||
if strings.Contains(msg, "pathspec") && strings.Contains(msg, "did not match") &&
|
||||
!strings.Contains(msg, "fetch branch failed") {
|
||||
return true
|
||||
}
|
||||
|
||||
const missingBranchMarker = "not found locally or on remote"
|
||||
markerIndex := strings.Index(msg, missingBranchMarker)
|
||||
if markerIndex < 0 || !strings.Contains(msg[:markerIndex], "branch") {
|
||||
return false
|
||||
}
|
||||
|
||||
// The worktree layer appends the underlying fetch failure after this marker.
|
||||
// Only the marker by itself is evidence that the remote branch is missing.
|
||||
detail := strings.TrimSpace(msg[markerIndex+len(missingBranchMarker):])
|
||||
return detail == ""
|
||||
}
|
||||
|
||||
var (
|
||||
@@ -1573,6 +1588,25 @@ func extractMissingBranchName(err error) string {
|
||||
return ""
|
||||
}
|
||||
|
||||
const missingPRBranchRecoveryClaimKey = "missing_pr_branch_recovery_claimed"
|
||||
|
||||
type failedSessionMetadataClaimer interface {
|
||||
SetSessionMetadataKeyIfAbsentIfState(
|
||||
ctx context.Context,
|
||||
sessionID, key string,
|
||||
value interface{},
|
||||
expectedState models.TaskSessionState,
|
||||
) (bool, error)
|
||||
}
|
||||
|
||||
type failedSessionMetadataClaimReleaser interface {
|
||||
RemoveSessionMetadataKeyIfState(
|
||||
ctx context.Context,
|
||||
sessionID, key string,
|
||||
expectedState models.TaskSessionState,
|
||||
) (bool, error)
|
||||
}
|
||||
|
||||
func (s *Service) matchingTaskPRState(ctx context.Context, taskID, repositoryID, branch string) string {
|
||||
repositoryID = strings.TrimSpace(repositoryID)
|
||||
if s.githubService == nil || repositoryID == "" || branch == "" {
|
||||
@@ -1635,16 +1669,56 @@ func (s *Service) hasOnlyTaskRepository(ctx context.Context, taskID, repositoryI
|
||||
strings.TrimSpace(repositories[0].RepositoryID) == repositoryID
|
||||
}
|
||||
|
||||
// handleSessionLaunchFailed creates branch guidance only after the caller has
|
||||
// persisted FAILED. The claim is stored on the session because prepare, start,
|
||||
// and resume may race.
|
||||
func (s *Service) handleSessionLaunchFailed(ctx context.Context, taskID, sessionID, repositoryID string, launchErr error) {
|
||||
if s.messageCreator == nil || !isMissingBranchError(launchErr) {
|
||||
return
|
||||
}
|
||||
|
||||
recoveryCtx := context.WithoutCancel(ctx)
|
||||
branch := extractMissingBranchName(launchErr)
|
||||
prState := s.matchingTaskPRState(ctx, taskID, repositoryID, branch)
|
||||
prState := s.matchingTaskPRState(recoveryCtx, taskID, repositoryID, branch)
|
||||
if prState == githubPRStateOpen {
|
||||
return
|
||||
}
|
||||
claimer, ok := s.repo.(failedSessionMetadataClaimer)
|
||||
if !ok {
|
||||
s.logger.Warn("session repository cannot claim missing-branch recovery",
|
||||
zap.String("task_id", taskID), zap.String("session_id", sessionID))
|
||||
return
|
||||
}
|
||||
claimed, err := claimer.SetSessionMetadataKeyIfAbsentIfState(
|
||||
recoveryCtx, sessionID, missingPRBranchRecoveryClaimKey, true, models.TaskSessionStateFailed,
|
||||
)
|
||||
if err != nil {
|
||||
s.logger.Warn("failed to claim missing-branch recovery",
|
||||
zap.String("task_id", taskID), zap.String("session_id", sessionID), zap.Error(err))
|
||||
return
|
||||
}
|
||||
if !claimed {
|
||||
return
|
||||
}
|
||||
if err := s.createMissingPRBranchRecoveryMessage(recoveryCtx, taskID, sessionID, branch, prState); err != nil {
|
||||
releaser, ok := s.repo.(failedSessionMetadataClaimReleaser)
|
||||
if !ok {
|
||||
s.logger.Warn("session repository cannot release missing-branch recovery claim after message failure",
|
||||
zap.String("task_id", taskID), zap.String("session_id", sessionID))
|
||||
return
|
||||
}
|
||||
if _, releaseErr := releaser.RemoveSessionMetadataKeyIfState(
|
||||
recoveryCtx, sessionID, missingPRBranchRecoveryClaimKey, models.TaskSessionStateFailed,
|
||||
); releaseErr != nil {
|
||||
s.logger.Warn("failed to release missing-branch recovery claim after message failure",
|
||||
zap.String("task_id", taskID), zap.String("session_id", sessionID), zap.Error(releaseErr))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func (s *Service) createMissingPRBranchRecoveryMessage(
|
||||
ctx context.Context,
|
||||
taskID, sessionID, branch, prState string,
|
||||
) error {
|
||||
content := "Kandev couldn't fetch the requested branch from the configured repository. Verify the task's repository branch or PR link, then retry."
|
||||
if branch != "" {
|
||||
content = "Kandev couldn't fetch branch \"" + branch + "\" from the configured repository. Verify the task's repository branch or PR link, then retry."
|
||||
@@ -1681,7 +1755,6 @@ func (s *Service) handleSessionLaunchFailed(ctx context.Context, taskID, session
|
||||
},
|
||||
}
|
||||
}
|
||||
s.suppressToast.Store(sessionID, true)
|
||||
msgCtx, cancel := context.WithTimeout(ctx, 5*time.Second)
|
||||
defer cancel()
|
||||
if err := s.messageCreator.CreateSessionMessage(
|
||||
@@ -1698,7 +1771,10 @@ func (s *Service) handleSessionLaunchFailed(ctx context.Context, taskID, session
|
||||
zap.String("task_id", taskID),
|
||||
zap.String("session_id", sessionID),
|
||||
zap.Error(err))
|
||||
return err
|
||||
}
|
||||
s.suppressToast.Store(sessionID, true)
|
||||
return nil
|
||||
}
|
||||
|
||||
// IsRunning returns true if the service is running
|
||||
|
||||
@@ -102,6 +102,14 @@ func TestHandleSessionLaunchFailed_UnavailablePRStateCreatesNeutralFetchGuidance
|
||||
|
||||
err := errors.New("environment preparation failed: branch \"feature/foo\" not found locally or on remote: fatal: couldn't find remote ref feature/foo")
|
||||
svc.handleSessionLaunchFailed(ctx, "task1", "session1", "repo-a", err)
|
||||
if len(mc.sessionMessages) != 0 {
|
||||
t.Fatalf("launch callback created guidance before the FAILED transition: %d messages", len(mc.sessionMessages))
|
||||
}
|
||||
if err := repo.UpdateTaskSessionState(ctx, "session1", models.TaskSessionStateFailed, err.Error()); err != nil {
|
||||
t.Fatalf("mark session failed: %v", err)
|
||||
}
|
||||
svc.handleSessionLaunchFailed(ctx, "task1", "session1", "repo-a", err)
|
||||
svc.handleSessionLaunchFailed(ctx, "task1", "session1", "repo-a", err)
|
||||
|
||||
if len(mc.sessionMessages) != 1 {
|
||||
t.Fatalf("expected 1 session message, got %d", len(mc.sessionMessages))
|
||||
@@ -134,7 +142,7 @@ func TestHandleSessionLaunchFailed_UnavailablePRStateCreatesNeutralFetchGuidance
|
||||
func TestHandleSessionLaunchFailed_OpenMatchingPRDoesNotCreateMissingBranchGuidance(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
repo := setupTestRepo(t)
|
||||
seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateStarting)
|
||||
seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateFailed)
|
||||
|
||||
mc := &mockMessageCreator{}
|
||||
svc := createTestService(repo, newMockStepGetter(), newMockTaskRepo())
|
||||
@@ -157,7 +165,7 @@ func TestHandleSessionLaunchFailed_OpenMatchingPRDoesNotCreateMissingBranchGuida
|
||||
func TestHandleSessionLaunchFailed_SecondaryOpenPRDoesNotUsePrimaryTerminalPR(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
repo := setupTestRepo(t)
|
||||
seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateStarting)
|
||||
seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateFailed)
|
||||
|
||||
mc := &mockMessageCreator{}
|
||||
svc := createTestService(repo, newMockStepGetter(), newMockTaskRepo())
|
||||
@@ -177,7 +185,7 @@ func TestHandleSessionLaunchFailed_SecondaryOpenPRDoesNotUsePrimaryTerminalPR(t
|
||||
|
||||
func TestHandleSessionLaunchFailed_BoundsBlockingPRStateLookup(t *testing.T) {
|
||||
repo := setupTestRepo(t)
|
||||
seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateStarting)
|
||||
seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateFailed)
|
||||
|
||||
mc := &mockMessageCreator{}
|
||||
svc := createTestService(repo, newMockStepGetter(), newMockTaskRepo())
|
||||
@@ -213,7 +221,7 @@ func TestHandleSessionLaunchFailed_BoundsBlockingPRStateLookup(t *testing.T) {
|
||||
func TestHandleSessionLaunchFailed_ConflictingSameBranchPRStatesCreateNeutralGuidance(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
repo := setupTestRepo(t)
|
||||
seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateStarting)
|
||||
seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateFailed)
|
||||
|
||||
mc := &mockMessageCreator{}
|
||||
svc := createTestService(repo, newMockStepGetter(), newMockTaskRepo())
|
||||
@@ -244,7 +252,7 @@ func TestHandleSessionLaunchFailed_ConflictingSameBranchPRStatesCreateNeutralGui
|
||||
func TestHandleSessionLaunchFailed_MultipleTerminalMatchingPRsCreateNeutralGuidance(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
repo := setupTestRepo(t)
|
||||
seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateStarting)
|
||||
seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateFailed)
|
||||
|
||||
mc := &mockMessageCreator{}
|
||||
svc := createTestService(repo, newMockStepGetter(), newMockTaskRepo())
|
||||
@@ -277,7 +285,7 @@ func TestHandleSessionLaunchFailed_ClosedOrMergedMatchingPRCreatesMissingBranchG
|
||||
t.Run(prState, func(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
repo := setupTestRepo(t)
|
||||
seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateStarting)
|
||||
seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateFailed)
|
||||
|
||||
mc := &mockMessageCreator{}
|
||||
svc := createTestService(repo, newMockStepGetter(), newMockTaskRepo())
|
||||
@@ -310,7 +318,7 @@ func TestHandleSessionLaunchFailed_ClosedOrMergedMatchingPRCreatesMissingBranchG
|
||||
func TestHandleSessionLaunchFailed_UnrelatedOpenPRDoesNotSuppressNeutralGuidance(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
repo := setupTestRepo(t)
|
||||
seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateStarting)
|
||||
seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateFailed)
|
||||
|
||||
mc := &mockMessageCreator{}
|
||||
svc := createTestService(repo, newMockStepGetter(), newMockTaskRepo())
|
||||
@@ -337,7 +345,7 @@ func TestHandleSessionLaunchFailed_UnrelatedOpenPRDoesNotSuppressNeutralGuidance
|
||||
func TestHandleSessionLaunchFailed_ClosedPRInAnotherRepositoryCreatesNeutralGuidance(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
repo := setupTestRepo(t)
|
||||
seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateStarting)
|
||||
seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateFailed)
|
||||
|
||||
mc := &mockMessageCreator{}
|
||||
svc := createTestService(repo, newMockStepGetter(), newMockTaskRepo())
|
||||
@@ -367,7 +375,7 @@ func TestHandleSessionLaunchFailed_ClosedPRInAnotherRepositoryCreatesNeutralGuid
|
||||
func TestHandleSessionLaunchFailed_SingleRepositoryLegacyPRCreatesMissingBranchGuidance(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
repo := setupTestRepo(t)
|
||||
seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateStarting)
|
||||
seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateFailed)
|
||||
if err := repo.CreateRepository(ctx, &models.Repository{
|
||||
ID: "repo-a", WorkspaceID: "ws1", Name: "repo-a", SourceType: "local",
|
||||
}); err != nil {
|
||||
@@ -404,7 +412,7 @@ func TestHandleSessionLaunchFailed_SingleRepositoryLegacyPRCreatesMissingBranchG
|
||||
func TestHandleSessionLaunchFailed_LegacyPRForDifferentRepositoryCreatesNeutralGuidance(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
repo := setupTestRepo(t)
|
||||
seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateStarting)
|
||||
seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateFailed)
|
||||
if err := repo.CreateRepository(ctx, &models.Repository{
|
||||
ID: "repo-a", WorkspaceID: "ws1", Name: "repo-a", SourceType: "local",
|
||||
}); err != nil {
|
||||
@@ -441,7 +449,7 @@ func TestHandleSessionLaunchFailed_LegacyPRForDifferentRepositoryCreatesNeutralG
|
||||
func TestHandleSessionLaunchFailed_MultipleRepositoriesLegacyPRCreatesNeutralGuidance(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
repo := setupTestRepo(t)
|
||||
seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateStarting)
|
||||
seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateFailed)
|
||||
for position, repositoryID := range []string{"repo-a", "repo-b"} {
|
||||
if err := repo.CreateRepository(ctx, &models.Repository{
|
||||
ID: repositoryID, WorkspaceID: "ws1", Name: repositoryID, SourceType: "local",
|
||||
@@ -480,7 +488,7 @@ func TestHandleSessionLaunchFailed_MultipleRepositoriesLegacyPRCreatesNeutralGui
|
||||
func TestHandleSessionLaunchFailed_EmptyRepositoryIDCreatesNeutralGuidance(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
repo := setupTestRepo(t)
|
||||
seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateStarting)
|
||||
seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateFailed)
|
||||
|
||||
mc := &mockMessageCreator{}
|
||||
svc := createTestService(repo, newMockStepGetter(), newMockTaskRepo())
|
||||
@@ -507,10 +515,46 @@ func TestHandleSessionLaunchFailed_EmptyRepositoryIDCreatesNeutralGuidance(t *te
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleSessionLaunchFailed_RetriesAfterGuidanceMessageWriteFailure(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
repo := setupTestRepo(t)
|
||||
seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateFailed)
|
||||
|
||||
writeErr := errors.New("message store unavailable")
|
||||
mc := &mockMessageCreator{sessionMessageErr: writeErr}
|
||||
svc := createTestService(repo, newMockStepGetter(), newMockTaskRepo())
|
||||
svc.messageCreator = mc
|
||||
err := errors.New("environment preparation failed: fatal: couldn't find remote ref feature/foo")
|
||||
|
||||
svc.handleSessionLaunchFailed(ctx, "task1", "session1", "", err)
|
||||
if len(mc.sessionMessages) != 0 {
|
||||
t.Fatalf("failed message write persisted %d messages", len(mc.sessionMessages))
|
||||
}
|
||||
if _, suppressed := svc.suppressToast.Load("session1"); suppressed {
|
||||
t.Fatal("failed message write suppressed the fallback toast")
|
||||
}
|
||||
failed, getErr := repo.GetTaskSession(ctx, "session1")
|
||||
if getErr != nil {
|
||||
t.Fatalf("get failed session: %v", getErr)
|
||||
}
|
||||
if _, claimed := failed.Metadata[missingPRBranchRecoveryClaimKey]; claimed {
|
||||
t.Fatal("failed message write left the recovery claim persisted")
|
||||
}
|
||||
|
||||
mc.sessionMessageErr = nil
|
||||
svc.handleSessionLaunchFailed(ctx, "task1", "session1", "", err)
|
||||
if len(mc.sessionMessages) != 1 {
|
||||
t.Fatalf("retry created %d messages, want 1", len(mc.sessionMessages))
|
||||
}
|
||||
if _, suppressed := svc.suppressToast.Load("session1"); !suppressed {
|
||||
t.Fatal("successful recovery message did not suppress the fallback toast")
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleSessionLaunchFailed_IgnoresUnrelatedErrors(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
repo := setupTestRepo(t)
|
||||
seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateStarting)
|
||||
seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateFailed)
|
||||
|
||||
mc := &mockMessageCreator{}
|
||||
svc := createTestService(repo, newMockStepGetter(), newMockTaskRepo())
|
||||
@@ -522,3 +566,45 @@ func TestHandleSessionLaunchFailed_IgnoresUnrelatedErrors(t *testing.T) {
|
||||
t.Fatalf("expected no session messages for unrelated error, got %d", len(mc.sessionMessages))
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleSessionLaunchFailed_IgnoresMissingBranchWrapperAroundTransportError(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
repo := setupTestRepo(t)
|
||||
seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateFailed)
|
||||
|
||||
mc := &mockMessageCreator{}
|
||||
svc := createTestService(repo, newMockStepGetter(), newMockTaskRepo())
|
||||
svc.messageCreator = mc
|
||||
|
||||
err := errors.New("environment preparation failed: branch \"codex/enhance-prompt-result-delivery\" not found locally or on remote: fatal: unable to access 'https://github.com/kdlbs/kandev.git/': Could not resolve host: github.com")
|
||||
if isMissingBranchError(err) {
|
||||
t.Fatal("expected transport failure not to be classified as a missing PR branch")
|
||||
}
|
||||
|
||||
svc.handleSessionLaunchFailed(ctx, "task1", "session1", "", err)
|
||||
|
||||
if len(mc.sessionMessages) != 0 {
|
||||
t.Fatalf("expected no guidance message for transport failure, got %d", len(mc.sessionMessages))
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleSessionLaunchFailed_IgnoresPathspecAfterFetchFailure(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
repo := setupTestRepo(t)
|
||||
seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateFailed)
|
||||
|
||||
mc := &mockMessageCreator{}
|
||||
svc := createTestService(repo, newMockStepGetter(), newMockTaskRepo())
|
||||
svc.messageCreator = mc
|
||||
|
||||
err := errors.New("environment preparation failed: checkout branch: git command failed: fetch branch failed: exit status 128: fatal: unable to access 'https://github.com/kdlbs/kandev.git/': Could not resolve host: github.com\ncheckout branch failed: error: pathspec 'codex/enhance-prompt-result-delivery' did not match any file(s) known to git")
|
||||
if isMissingBranchError(err) {
|
||||
t.Fatal("expected pathspec after fetch failure not to be classified as a missing PR branch")
|
||||
}
|
||||
|
||||
svc.handleSessionLaunchFailed(ctx, "task1", "session1", "", err)
|
||||
|
||||
if len(mc.sessionMessages) != 0 {
|
||||
t.Fatalf("expected no guidance message after fetch failure, got %d", len(mc.sessionMessages))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -257,6 +257,10 @@ func (s *Service) PrepareTaskSession(ctx context.Context, taskID string, agentPr
|
||||
bgCtx := context.Background()
|
||||
prepExec, launchErr := s.executor.LaunchPreparedSession(bgCtx, task, sessionID, executor.LaunchOptions{AgentProfileID: agentProfileID, ExecutorID: executorID, WorkflowStepID: workflowStepID})
|
||||
if launchErr != nil {
|
||||
// LaunchAgent failures persist FAILED in the executor. Earlier
|
||||
// workspace failures return here and are recorded through the same
|
||||
// session-level recovery claim.
|
||||
s.handleEarlyMissingPRBranchLaunchFailure(bgCtx, taskID, sessionID, launchErr)
|
||||
s.logger.Warn("failed to launch workspace for prepared session (file browsing may be unavailable)",
|
||||
zap.String("task_id", taskID),
|
||||
zap.String("session_id", sessionID),
|
||||
@@ -476,6 +480,9 @@ func (s *Service) StartCreatedSession(
|
||||
}
|
||||
execution, err := s.executor.LaunchPreparedSession(ctx, task, sessionID, executor.LaunchOptions{AgentProfileID: effectiveProfileID, ExecutorID: executorID, Prompt: effectivePrompt, StartAgent: true, McpMode: mcpMode, Attachments: attachments})
|
||||
if err != nil {
|
||||
// The executor persists LaunchAgent failures. Cover earlier prepared-session
|
||||
// failures here; the session-level claim makes either completion order safe.
|
||||
s.handleEarlyMissingPRBranchLaunchFailure(ctx, taskID, sessionID, err)
|
||||
return nil, err
|
||||
}
|
||||
|
||||
@@ -492,6 +499,56 @@ func (s *Service) StartCreatedSession(
|
||||
return execution, nil
|
||||
}
|
||||
|
||||
// handleEarlyMissingPRBranchLaunchFailure covers failures that occur before
|
||||
// Executor reaches AgentManager.LaunchAgent. Those failures do not run the
|
||||
// executor's launch-failure bookkeeping, so this fallback owns the terminal
|
||||
// transition and lets the shared persisted claim create guidance once.
|
||||
func (s *Service) handleEarlyMissingPRBranchLaunchFailure(
|
||||
ctx context.Context,
|
||||
taskID, sessionID string,
|
||||
launchErr error,
|
||||
) {
|
||||
if !isMissingBranchError(launchErr) {
|
||||
return
|
||||
}
|
||||
s.recordSessionLaunchFailure(ctx, taskID, sessionID, launchErr)
|
||||
}
|
||||
|
||||
// recordSessionLaunchFailure transitions a still-active session to FAILED,
|
||||
// attaches missing-branch guidance only after that CAS succeeds, and updates
|
||||
// the task only while the same failed session still owns it.
|
||||
func (s *Service) recordSessionLaunchFailure(ctx context.Context, taskID, sessionID string, launchErr error, preloadedSession ...*models.TaskSession) {
|
||||
_, changed := s.updateTaskSessionStateWithHook(
|
||||
ctx, taskID, sessionID, models.TaskSessionStateFailed, launchErr.Error(), false,
|
||||
func() { s.handleSessionLaunchFailed(ctx, taskID, sessionID, "", launchErr) }, preloadedSession...,
|
||||
)
|
||||
if !changed {
|
||||
// A resume may begin from an already FAILED session. Its state CAS is
|
||||
// intentionally a no-op, but the persisted claim still lets this newly
|
||||
// classified failure publish guidance exactly once. The claim itself is
|
||||
// state-guarded, so a concurrently cancelled/completed session cannot
|
||||
// receive stale recovery UI.
|
||||
if len(preloadedSession) > 0 && preloadedSession[0] != nil && preloadedSession[0].State == models.TaskSessionStateFailed {
|
||||
s.handleSessionLaunchFailed(ctx, taskID, sessionID, "", launchErr)
|
||||
}
|
||||
return
|
||||
}
|
||||
updated, err := s.taskRepo.UpdateTaskStateIfSessionState(
|
||||
ctx, taskID, sessionID, models.TaskSessionStateFailed, v1.TaskStateFailed,
|
||||
)
|
||||
if err != nil {
|
||||
s.logger.Warn("failed to update task state to FAILED after early launch error",
|
||||
zap.String("task_id", taskID),
|
||||
zap.String("session_id", sessionID),
|
||||
zap.Error(err))
|
||||
return
|
||||
}
|
||||
if !updated {
|
||||
return
|
||||
}
|
||||
s.processParentChildrenCompletedForTaskState(ctx, taskID, v1.TaskStateFailed)
|
||||
}
|
||||
|
||||
func (s *Service) scheduleTaskForSession(ctx context.Context, taskID, sessionID string) error {
|
||||
s.taskRuntimeStateMu.Lock()
|
||||
defer s.taskRuntimeStateMu.Unlock()
|
||||
@@ -1205,15 +1262,10 @@ func (s *Service) ResumeTaskSession(ctx context.Context, taskID, sessionID strin
|
||||
// away), the SessionStateFailed and TaskStateFailed updates would
|
||||
// themselves fail with "context canceled" and leave the task stuck
|
||||
// looking "running" forever.
|
||||
s.updateTaskSessionState(resumeCtx, taskID, sessionID, models.TaskSessionStateFailed, err.Error(), false, session)
|
||||
if stateErr := s.taskRepo.UpdateTaskState(resumeCtx, taskID, v1.TaskStateFailed); stateErr != nil {
|
||||
s.logger.Warn("failed to update task state to FAILED after resume error",
|
||||
zap.String("task_id", taskID),
|
||||
zap.String("session_id", sessionID),
|
||||
zap.Error(stateErr))
|
||||
} else {
|
||||
s.processParentChildrenCompletedForTaskState(resumeCtx, taskID, v1.TaskStateFailed)
|
||||
}
|
||||
// ResumeSession launches the workspace directly rather than through
|
||||
// LaunchPreparedSession. Record this failure with the same state CAS,
|
||||
// persisted recovery claim, and archive-safe task CAS as early launch.
|
||||
s.recordSessionLaunchFailure(resumeCtx, taskID, sessionID, err, session)
|
||||
return nil, err
|
||||
}
|
||||
}
|
||||
|
||||
@@ -15,6 +15,7 @@ import (
|
||||
"github.com/kandev/kandev/internal/agent/runtime/lifecycle"
|
||||
"github.com/kandev/kandev/internal/agentctl/types/streams"
|
||||
"github.com/kandev/kandev/internal/events"
|
||||
"github.com/kandev/kandev/internal/events/bus"
|
||||
"github.com/kandev/kandev/internal/orchestrator/dto"
|
||||
"github.com/kandev/kandev/internal/orchestrator/executor"
|
||||
"github.com/kandev/kandev/internal/orchestrator/messagequeue"
|
||||
@@ -67,6 +68,21 @@ func seedTaskAndSession(t *testing.T, repo *sqliterepo.Repository, taskID, sessi
|
||||
}
|
||||
}
|
||||
|
||||
// taskEnvironmentFailureRepo injects an error after session creation but before
|
||||
// Executor reaches AgentManager.LaunchAgent. This models workspace-preparation
|
||||
// failures, which do not trigger the executor's launch-failure callback.
|
||||
type taskEnvironmentFailureRepo struct {
|
||||
*sqliterepo.Repository
|
||||
err error
|
||||
called chan struct{}
|
||||
once sync.Once
|
||||
}
|
||||
|
||||
func (r *taskEnvironmentFailureRepo) GetTaskEnvironmentByTaskID(_ context.Context, _ string) (*models.TaskEnvironment, error) {
|
||||
r.once.Do(func() { close(r.called) })
|
||||
return nil, r.err
|
||||
}
|
||||
|
||||
func TestCreateStartSession_KanbanRunnerCreatesDistinctSession(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
repo := setupTestRepo(t)
|
||||
@@ -2251,6 +2267,421 @@ func TestStartCreatedSession_NotInCreatedState(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestStartCreatedSession_MissingRemoteRefCreatesNeutralRecoveryMessage(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
repo := setupTestRepo(t)
|
||||
seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateCreated)
|
||||
|
||||
launchErr := errors.New("environment preparation failed: fatal: couldn't find remote ref feature/deleted-pr")
|
||||
agentMgr := &mockAgentManager{
|
||||
launchAgentFunc: func(_ context.Context, _ *executor.LaunchAgentRequest) (*executor.LaunchAgentResponse, error) {
|
||||
return nil, launchErr
|
||||
},
|
||||
}
|
||||
taskRepo := newMockTaskRepo()
|
||||
taskRepo.tasks["task1"] = &v1.Task{
|
||||
ID: "task1",
|
||||
Title: "Test Task",
|
||||
Description: "start the task",
|
||||
State: v1.TaskStateInProgress,
|
||||
}
|
||||
svc := createTestServiceWithScheduler(repo, newMockStepGetter(), taskRepo, agentMgr)
|
||||
messages := &mockMessageCreator{}
|
||||
svc.messageCreator = messages
|
||||
svc.executor.SetOnLaunchFailed(svc.handleSessionLaunchFailed)
|
||||
svc.executor.SetOnSessionStateChange(func(callbackCtx context.Context, callbackTaskID, callbackSessionID string, state models.TaskSessionState, errorMessage string) error {
|
||||
svc.updateTaskSessionState(callbackCtx, callbackTaskID, callbackSessionID, state, errorMessage, true)
|
||||
return nil
|
||||
})
|
||||
|
||||
_, err := svc.StartCreatedSession(ctx, "task1", "session1", "profile1", "start the task", true, false, true, nil, nil)
|
||||
if !errors.Is(err, launchErr) {
|
||||
t.Fatalf("StartCreatedSession error = %v, want %v", err, launchErr)
|
||||
}
|
||||
|
||||
if len(messages.sessionMessages) != 1 {
|
||||
t.Fatalf("expected one recovery message, got %d", len(messages.sessionMessages))
|
||||
}
|
||||
message := messages.sessionMessages[0]
|
||||
if message.metadata["failure_kind"] != "branch_fetch_failed" {
|
||||
t.Fatalf("failure_kind = %#v, want branch_fetch_failed", message.metadata["failure_kind"])
|
||||
}
|
||||
if _, ok := message.metadata["actions"]; ok {
|
||||
t.Fatalf("expected neutral guidance without archive/delete actions, got %#v", message.metadata["actions"])
|
||||
}
|
||||
}
|
||||
|
||||
func TestStartCreatedSession_MissingRemoteRefDoesNotDuplicateExecutorRecoveryMessage(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
repo := setupTestRepo(t)
|
||||
seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateCreated)
|
||||
|
||||
launchErr := errors.New("environment preparation failed: fatal: couldn't find remote ref feature/deleted-pr")
|
||||
agentMgr := &mockAgentManager{
|
||||
launchAgentFunc: func(_ context.Context, _ *executor.LaunchAgentRequest) (*executor.LaunchAgentResponse, error) {
|
||||
return nil, launchErr
|
||||
},
|
||||
}
|
||||
taskRepo := newMockTaskRepo()
|
||||
taskRepo.tasks["task1"] = &v1.Task{
|
||||
ID: "task1",
|
||||
Title: "Test Task",
|
||||
Description: "start the task",
|
||||
State: v1.TaskStateInProgress,
|
||||
}
|
||||
svc := createTestServiceWithScheduler(repo, newMockStepGetter(), taskRepo, agentMgr)
|
||||
messages := &mockMessageCreator{}
|
||||
svc.messageCreator = messages
|
||||
svc.eventBus = bus.NewMemoryEventBus(testLogger())
|
||||
svc.executor.SetOnLaunchFailed(svc.handleSessionLaunchFailed)
|
||||
// Production routes terminal failures through the strict transition callback
|
||||
// so recovery guidance can set suppressToast before state_changed publishes it.
|
||||
svc.executor.SetOnSessionStateTransition(svc.transitionTaskSessionState)
|
||||
|
||||
_, err := svc.StartCreatedSession(ctx, "task1", "session1", "profile1", "start the task", true, false, true, nil, nil)
|
||||
if !errors.Is(err, launchErr) {
|
||||
t.Fatalf("StartCreatedSession error = %v, want %v", err, launchErr)
|
||||
}
|
||||
if len(messages.sessionMessages) != 1 {
|
||||
t.Fatalf("expected exactly one recovery message, got %d", len(messages.sessionMessages))
|
||||
}
|
||||
if _, suppressed := svc.suppressToast.Load("session1"); suppressed {
|
||||
t.Fatal("state-change publishing did not consume the toast-suppression marker")
|
||||
}
|
||||
}
|
||||
|
||||
func TestPrepareTaskSession_WorkspaceLaunchFailureRecovery(t *testing.T) {
|
||||
const (
|
||||
taskID = "task1"
|
||||
)
|
||||
|
||||
newTask := func() *v1.Task {
|
||||
return &v1.Task{
|
||||
ID: taskID,
|
||||
WorkspaceID: "ws1",
|
||||
WorkflowID: "wf1",
|
||||
Title: "Test Task",
|
||||
Description: "prepare the workspace",
|
||||
State: v1.TaskStateInProgress,
|
||||
}
|
||||
}
|
||||
|
||||
t.Run("early missing remote ref creates one neutral recovery message", func(t *testing.T) {
|
||||
baseRepo := setupTestRepo(t)
|
||||
seedTaskAndSession(t, baseRepo, taskID, "existing-session", models.TaskSessionStateCreated)
|
||||
failureRepo := &taskEnvironmentFailureRepo{
|
||||
Repository: baseRepo,
|
||||
err: errors.New("environment preparation failed: fatal: couldn't find remote ref feature/deleted-pr"),
|
||||
called: make(chan struct{}),
|
||||
}
|
||||
taskRepo := newMockTaskRepo()
|
||||
taskRepo.tasks[taskID] = newTask()
|
||||
agentMgr := &mockAgentManager{}
|
||||
exec := executor.NewExecutor(agentMgr, failureRepo, testLogger(), executor.ExecutorConfig{})
|
||||
svc := &Service{
|
||||
logger: testLogger(),
|
||||
repo: failureRepo,
|
||||
workflowStepGetter: newMockStepGetter(),
|
||||
taskRepo: taskRepo,
|
||||
agentManager: agentMgr,
|
||||
executor: exec,
|
||||
messageQueue: messagequeue.NewServiceMemory(testLogger()),
|
||||
scheduler: scheduler.NewScheduler(queue.NewTaskQueue(1), exec, taskRepo, testLogger(), scheduler.SchedulerConfig{}),
|
||||
}
|
||||
messages := &mockMessageCreator{sessionMessageDone: make(chan struct{})}
|
||||
svc.messageCreator = messages
|
||||
svc.eventBus = bus.NewMemoryEventBus(testLogger())
|
||||
|
||||
sessionID, err := svc.PrepareTaskSession(context.Background(), taskID, "profile1", "", "", "", true)
|
||||
if err != nil {
|
||||
t.Fatalf("PrepareTaskSession: %v", err)
|
||||
}
|
||||
|
||||
select {
|
||||
case <-messages.sessionMessageDone:
|
||||
case <-time.After(time.Second):
|
||||
t.Fatal("timed out waiting for missing-branch recovery message")
|
||||
}
|
||||
if len(messages.sessionMessages) != 1 {
|
||||
t.Fatalf("expected exactly one recovery message, got %d", len(messages.sessionMessages))
|
||||
}
|
||||
message := messages.sessionMessages[0]
|
||||
if message.sessionID != sessionID {
|
||||
t.Fatalf("recovery message session = %q, want %q", message.sessionID, sessionID)
|
||||
}
|
||||
if message.metadata["failure_kind"] != "branch_fetch_failed" {
|
||||
t.Fatalf("failure_kind = %#v, want branch_fetch_failed", message.metadata["failure_kind"])
|
||||
}
|
||||
if _, ok := message.metadata["actions"]; ok {
|
||||
t.Fatalf("expected neutral guidance without archive/delete actions, got %#v", message.metadata["actions"])
|
||||
}
|
||||
require.Eventually(t, func() bool {
|
||||
failedSession, getErr := baseRepo.GetTaskSession(context.Background(), sessionID)
|
||||
return getErr == nil && failedSession.State == models.TaskSessionStateFailed
|
||||
}, time.Second, 10*time.Millisecond, "expected early launch failure to mark the session FAILED")
|
||||
require.Eventually(t, func() bool {
|
||||
taskRepo.mu.Lock()
|
||||
defer taskRepo.mu.Unlock()
|
||||
return taskRepo.updatedStates[taskID] == v1.TaskStateFailed
|
||||
}, time.Second, 10*time.Millisecond, "expected early launch failure to mark the task FAILED")
|
||||
if _, suppressed := svc.suppressToast.Load(sessionID); suppressed {
|
||||
t.Fatal("missing-branch recovery left a stale toast-suppression marker")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("transport failure does not create missing-branch recovery", func(t *testing.T) {
|
||||
baseRepo := setupTestRepo(t)
|
||||
seedTaskAndSession(t, baseRepo, taskID, "existing-session", models.TaskSessionStateCreated)
|
||||
failureRepo := &taskEnvironmentFailureRepo{
|
||||
Repository: baseRepo,
|
||||
err: errors.New("environment preparation failed: branch \"feature/deleted-pr\" not found locally or on remote: fatal: unable to access 'https://github.com/kdlbs/kandev.git/': Could not resolve host: github.com"),
|
||||
called: make(chan struct{}),
|
||||
}
|
||||
taskRepo := newMockTaskRepo()
|
||||
taskRepo.tasks[taskID] = newTask()
|
||||
agentMgr := &mockAgentManager{}
|
||||
exec := executor.NewExecutor(agentMgr, failureRepo, testLogger(), executor.ExecutorConfig{})
|
||||
svc := &Service{
|
||||
logger: testLogger(),
|
||||
repo: failureRepo,
|
||||
workflowStepGetter: newMockStepGetter(),
|
||||
taskRepo: taskRepo,
|
||||
agentManager: agentMgr,
|
||||
executor: exec,
|
||||
messageQueue: messagequeue.NewServiceMemory(testLogger()),
|
||||
scheduler: scheduler.NewScheduler(queue.NewTaskQueue(1), exec, taskRepo, testLogger(), scheduler.SchedulerConfig{}),
|
||||
}
|
||||
messages := &mockMessageCreator{sessionMessageDone: make(chan struct{})}
|
||||
svc.messageCreator = messages
|
||||
|
||||
if _, err := svc.PrepareTaskSession(context.Background(), taskID, "profile1", "", "", "", true); err != nil {
|
||||
t.Fatalf("PrepareTaskSession: %v", err)
|
||||
}
|
||||
select {
|
||||
case <-failureRepo.called:
|
||||
case <-time.After(time.Second):
|
||||
t.Fatal("timed out waiting for workspace launch")
|
||||
}
|
||||
select {
|
||||
case <-messages.sessionMessageDone:
|
||||
t.Fatal("transport failure created missing-branch recovery message")
|
||||
case <-time.After(100 * time.Millisecond):
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("executor callback recovery is not duplicated", func(t *testing.T) {
|
||||
repo := setupTestRepo(t)
|
||||
seedTaskAndSession(t, repo, taskID, "existing-session", models.TaskSessionStateCreated)
|
||||
taskRepo := newMockTaskRepo()
|
||||
taskRepo.tasks[taskID] = newTask()
|
||||
launchErr := errors.New("environment preparation failed: fatal: couldn't find remote ref feature/deleted-pr")
|
||||
agentMgr := &mockAgentManager{
|
||||
launchAgentFunc: func(context.Context, *executor.LaunchAgentRequest) (*executor.LaunchAgentResponse, error) {
|
||||
return nil, launchErr
|
||||
},
|
||||
}
|
||||
svc := createTestServiceWithScheduler(repo, newMockStepGetter(), taskRepo, agentMgr)
|
||||
svc.executor.SetOnLaunchFailed(svc.handleSessionLaunchFailed)
|
||||
svc.eventBus = bus.NewMemoryEventBus(testLogger())
|
||||
svc.executor.SetOnSessionStateChange(func(callbackCtx context.Context, callbackTaskID, callbackSessionID string, state models.TaskSessionState, errorMessage string) error {
|
||||
svc.updateTaskSessionState(callbackCtx, callbackTaskID, callbackSessionID, state, errorMessage, true)
|
||||
return nil
|
||||
})
|
||||
messages := &mockMessageCreator{sessionMessageDone: make(chan struct{})}
|
||||
svc.messageCreator = messages
|
||||
|
||||
sessionID, err := svc.PrepareTaskSession(context.Background(), taskID, "profile1", "", "", "", true)
|
||||
if err != nil {
|
||||
t.Fatalf("PrepareTaskSession: %v", err)
|
||||
}
|
||||
select {
|
||||
case <-messages.sessionMessageDone:
|
||||
case <-time.After(time.Second):
|
||||
t.Fatal("timed out waiting for executor recovery message")
|
||||
}
|
||||
require.Eventually(t, func() bool {
|
||||
session, getErr := repo.GetTaskSession(context.Background(), sessionID)
|
||||
return getErr == nil && session.State == models.TaskSessionStateFailed
|
||||
}, time.Second, 10*time.Millisecond, "expected failed state after workspace launch error")
|
||||
if len(messages.sessionMessages) != 1 {
|
||||
t.Fatalf("expected exactly one recovery message, got %d", len(messages.sessionMessages))
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func TestHandleEarlyMissingPRBranchLaunchFailure_SkipsTaskFailureWhenSessionTransitionLoses(t *testing.T) {
|
||||
repo := setupTestRepo(t)
|
||||
seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateCancelled)
|
||||
taskRepo := newMockTaskRepo()
|
||||
taskRepo.tasks["task1"] = &v1.Task{ID: "task1", State: v1.TaskStateInProgress}
|
||||
svc := createTestService(repo, newMockStepGetter(), taskRepo)
|
||||
messages := &mockMessageCreator{}
|
||||
svc.messageCreator = messages
|
||||
svc.eventBus = bus.NewMemoryEventBus(testLogger())
|
||||
|
||||
svc.handleEarlyMissingPRBranchLaunchFailure(
|
||||
context.Background(), "task1", "session1",
|
||||
errors.New("fatal: couldn't find remote ref feature/deleted-pr"),
|
||||
)
|
||||
|
||||
if len(messages.sessionMessages) != 0 {
|
||||
t.Fatalf("terminal-race loser created %d recovery messages", len(messages.sessionMessages))
|
||||
}
|
||||
taskRepo.mu.Lock()
|
||||
defer taskRepo.mu.Unlock()
|
||||
if taskRepo.stateWrites["task1"] != 0 {
|
||||
t.Fatalf("terminal-race loser wrote task FAILED %d times", taskRepo.stateWrites["task1"])
|
||||
}
|
||||
if got := taskRepo.tasks["task1"].State; got != v1.TaskStateInProgress {
|
||||
t.Fatalf("task state = %s, want IN_PROGRESS", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleEarlyMissingPRBranchLaunchFailure_ArchiveSafeTaskWrite(t *testing.T) {
|
||||
repo := setupTestRepo(t)
|
||||
seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateCreated)
|
||||
taskRepo := newMockTaskRepo()
|
||||
taskRepo.tasks["task1"] = &v1.Task{ID: "task1", State: v1.TaskStateInProgress}
|
||||
// Model ArchiveTask winning after the session CAS but before the task-state
|
||||
// CAS: UpdateTaskStateIfSessionState must decline the write.
|
||||
taskRepo.updateIfSessionState = func(context.Context, string, string, models.TaskSessionState, v1.TaskState) (bool, error) {
|
||||
return false, nil
|
||||
}
|
||||
svc := createTestService(repo, newMockStepGetter(), taskRepo)
|
||||
messages := &mockMessageCreator{}
|
||||
svc.messageCreator = messages
|
||||
svc.eventBus = bus.NewMemoryEventBus(testLogger())
|
||||
|
||||
svc.handleEarlyMissingPRBranchLaunchFailure(
|
||||
context.Background(), "task1", "session1",
|
||||
errors.New("fatal: couldn't find remote ref feature/deleted-pr"),
|
||||
)
|
||||
|
||||
failedSession, err := repo.GetTaskSession(context.Background(), "session1")
|
||||
if err != nil {
|
||||
t.Fatalf("GetTaskSession: %v", err)
|
||||
}
|
||||
if failedSession.State != models.TaskSessionStateFailed {
|
||||
t.Fatalf("session state = %s, want FAILED", failedSession.State)
|
||||
}
|
||||
taskRepo.mu.Lock()
|
||||
defer taskRepo.mu.Unlock()
|
||||
if taskRepo.stateWrites["task1"] != 0 || taskRepo.unconditionalWrites["task1"] != 0 {
|
||||
t.Fatalf("archive-safe task CAS was bypassed: state writes=%d unconditional=%d", taskRepo.stateWrites["task1"], taskRepo.unconditionalWrites["task1"])
|
||||
}
|
||||
if got := taskRepo.tasks["task1"].State; got != v1.TaskStateInProgress {
|
||||
t.Fatalf("archived task state = %s, want IN_PROGRESS", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleEarlyMissingPRBranchLaunchFailure_ConcurrentLaunchesCreateOneMessage(t *testing.T) {
|
||||
repo := setupTestRepo(t)
|
||||
seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateCreated)
|
||||
taskRepo := newMockTaskRepo()
|
||||
taskRepo.tasks["task1"] = &v1.Task{ID: "task1", State: v1.TaskStateInProgress}
|
||||
svc := createTestService(repo, newMockStepGetter(), taskRepo)
|
||||
messages := &mockMessageCreator{}
|
||||
svc.messageCreator = messages
|
||||
svc.eventBus = bus.NewMemoryEventBus(testLogger())
|
||||
|
||||
launchErr := errors.New("fatal: couldn't find remote ref feature/deleted-pr")
|
||||
start := make(chan struct{})
|
||||
var ready sync.WaitGroup
|
||||
var done sync.WaitGroup
|
||||
ready.Add(2)
|
||||
done.Add(2)
|
||||
for range 2 {
|
||||
go func() {
|
||||
defer done.Done()
|
||||
ready.Done()
|
||||
<-start
|
||||
svc.handleEarlyMissingPRBranchLaunchFailure(context.Background(), "task1", "session1", launchErr)
|
||||
}()
|
||||
}
|
||||
ready.Wait()
|
||||
close(start)
|
||||
done.Wait()
|
||||
|
||||
messages.mu.Lock()
|
||||
messageCount := len(messages.sessionMessages)
|
||||
messages.mu.Unlock()
|
||||
if messageCount != 1 {
|
||||
t.Fatalf("recovery message count = %d, want 1", messageCount)
|
||||
}
|
||||
taskRepo.mu.Lock()
|
||||
defer taskRepo.mu.Unlock()
|
||||
if taskRepo.stateWrites["task1"] != 1 {
|
||||
t.Fatalf("task FAILED writes = %d, want 1", taskRepo.stateWrites["task1"])
|
||||
}
|
||||
}
|
||||
|
||||
func TestPrepareAndStartCreatedSession_MissingRemoteRefClaimsRecoveryOnce(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
repo := setupTestRepo(t)
|
||||
seedTaskAndSession(t, repo, "task1", "existing-session", models.TaskSessionStateCreated)
|
||||
taskRepo := newMockTaskRepo()
|
||||
taskRepo.tasks["task1"] = &v1.Task{
|
||||
ID: "task1", WorkspaceID: "ws1", WorkflowID: "wf1", Title: "Test Task", Description: "start task", State: v1.TaskStateInProgress,
|
||||
}
|
||||
launchEntered := make(chan struct{})
|
||||
var launchEnteredOnce sync.Once
|
||||
releaseLaunch := make(chan struct{})
|
||||
launchErr := errors.New("environment preparation failed: fatal: couldn't find remote ref feature/deleted-pr")
|
||||
agentMgr := &mockAgentManager{
|
||||
launchAgentFunc: func(context.Context, *executor.LaunchAgentRequest) (*executor.LaunchAgentResponse, error) {
|
||||
launchEnteredOnce.Do(func() { close(launchEntered) })
|
||||
<-releaseLaunch
|
||||
return nil, launchErr
|
||||
},
|
||||
}
|
||||
svc := createTestServiceWithScheduler(repo, newMockStepGetter(), taskRepo, agentMgr)
|
||||
svc.eventBus = bus.NewMemoryEventBus(testLogger())
|
||||
svc.executor.SetOnSessionStateTransition(svc.transitionTaskSessionState)
|
||||
svc.executor.SetOnLaunchFailed(svc.handleSessionLaunchFailed)
|
||||
messages := &mockMessageCreator{sessionMessageDone: make(chan struct{})}
|
||||
svc.messageCreator = messages
|
||||
|
||||
sessionID, err := svc.PrepareTaskSession(ctx, "task1", "profile1", "", "", "", true)
|
||||
if err != nil {
|
||||
t.Fatalf("PrepareTaskSession: %v", err)
|
||||
}
|
||||
select {
|
||||
case <-launchEntered:
|
||||
case <-time.After(time.Second):
|
||||
t.Fatal("prepared launch did not reach agent launch")
|
||||
}
|
||||
|
||||
startDone := make(chan error, 1)
|
||||
go func() {
|
||||
_, startErr := svc.StartCreatedSession(ctx, "task1", sessionID, "profile1", "start task", true, false, false, nil, nil)
|
||||
startDone <- startErr
|
||||
}()
|
||||
close(releaseLaunch)
|
||||
select {
|
||||
case <-startDone:
|
||||
case <-time.After(time.Second):
|
||||
t.Fatal("StartCreatedSession did not settle after prepared launch failed")
|
||||
}
|
||||
select {
|
||||
case <-messages.sessionMessageDone:
|
||||
case <-time.After(time.Second):
|
||||
t.Fatal("timed out waiting for missing-branch recovery message")
|
||||
}
|
||||
|
||||
failed, err := repo.GetTaskSession(ctx, sessionID)
|
||||
if err != nil {
|
||||
t.Fatalf("GetTaskSession: %v", err)
|
||||
}
|
||||
if failed.State != models.TaskSessionStateFailed {
|
||||
t.Fatalf("session state = %s, want FAILED", failed.State)
|
||||
}
|
||||
messages.mu.Lock()
|
||||
defer messages.mu.Unlock()
|
||||
if len(messages.sessionMessages) != 1 {
|
||||
t.Fatalf("recovery message count = %d, want 1", len(messages.sessionMessages))
|
||||
}
|
||||
}
|
||||
|
||||
func TestStartCreatedSession_WorkflowOverridePromotesPreparedWhenTaskHasNoPrimary(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
repo := setupTestRepo(t)
|
||||
@@ -2600,8 +3031,12 @@ func TestIssue1884_StepProfileSignalGateStaysInTaskMode(t *testing.T) {
|
||||
// mockMessageCreator implements MessageCreator for testing.
|
||||
// Only CreateUserMessage is tracked; all other methods are no-op stubs.
|
||||
type mockMessageCreator struct {
|
||||
mu sync.Mutex
|
||||
userMessages []mockUserMessage
|
||||
sessionMessages []mockSessionMessage
|
||||
sessionMessageDone chan struct{}
|
||||
sessionMessageOnce sync.Once
|
||||
sessionMessageErr error
|
||||
agentMessages []mockAgentMessage
|
||||
agentMessageWrites int
|
||||
agentStreamWrites int
|
||||
@@ -2651,6 +3086,11 @@ func (m *mockMessageCreator) UpdateToolCallMessage(context.Context, string, stri
|
||||
}
|
||||
|
||||
func (m *mockMessageCreator) CreateSessionMessage(_ context.Context, taskID, content, sessionID, messageType, turnID string, metadata map[string]interface{}, requestsInput bool) error {
|
||||
m.mu.Lock()
|
||||
defer m.mu.Unlock()
|
||||
if m.sessionMessageErr != nil {
|
||||
return m.sessionMessageErr
|
||||
}
|
||||
m.sessionMessages = append(m.sessionMessages, mockSessionMessage{
|
||||
taskID: taskID,
|
||||
content: content,
|
||||
@@ -2660,6 +3100,9 @@ func (m *mockMessageCreator) CreateSessionMessage(_ context.Context, taskID, con
|
||||
metadata: metadata,
|
||||
requestsInput: requestsInput,
|
||||
})
|
||||
if m.sessionMessageDone != nil {
|
||||
m.sessionMessageOnce.Do(func() { close(m.sessionMessageDone) })
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -2697,7 +3140,7 @@ func (m *mockMessageCreator) InvalidateModelCache(string) {}
|
||||
func TestBackfillInitialUserMessageIfMissing_RecordsWhenSessionEmpty(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
repo := setupTestRepo(t)
|
||||
seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateFailed)
|
||||
seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateWaitingForInput)
|
||||
|
||||
mc := &mockMessageCreator{}
|
||||
svc := createTestService(repo, newMockStepGetter(), newMockTaskRepo())
|
||||
@@ -3378,6 +3821,58 @@ func TestResumeTaskSession_ArchivedDuringLaunch(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestResumeTaskSession_FailureTaskWriteIsConditionalOnFailedSession(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
repo := setupTestRepo(t)
|
||||
taskRepo := newMockTaskRepo()
|
||||
taskRepo.tasks["task1"] = &v1.Task{ID: "task1", State: v1.TaskStateInProgress}
|
||||
// Model ArchiveTask winning after the session FAILED CAS but before the
|
||||
// task write. Resume must not use its legacy unconditional task update.
|
||||
taskRepo.updateIfSessionState = func(context.Context, string, string, models.TaskSessionState, v1.TaskState) (bool, error) {
|
||||
return false, nil
|
||||
}
|
||||
launchErr := errors.New("resume workspace failed")
|
||||
agentMgr := &mockAgentManager{
|
||||
launchAgentFunc: func(context.Context, *executor.LaunchAgentRequest) (*executor.LaunchAgentResponse, error) {
|
||||
return nil, launchErr
|
||||
},
|
||||
}
|
||||
svc := createTestServiceWithAgent(repo, newMockStepGetter(), taskRepo, agentMgr)
|
||||
svc.executor = executor.NewExecutor(agentMgr, repo, testLogger(), executor.ExecutorConfig{})
|
||||
seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateWaitingForInput)
|
||||
session, err := repo.GetTaskSession(ctx, "session1")
|
||||
if err != nil {
|
||||
t.Fatalf("GetTaskSession: %v", err)
|
||||
}
|
||||
session.AgentProfileID = "profile-1"
|
||||
if err := repo.UpdateTaskSession(ctx, session); err != nil {
|
||||
t.Fatalf("UpdateTaskSession: %v", err)
|
||||
}
|
||||
now := time.Now().UTC()
|
||||
if err := repo.UpsertExecutorRunning(ctx, &models.ExecutorRunning{
|
||||
ID: "resume-archive-race", SessionID: "session1", TaskID: "task1", CreatedAt: now, UpdatedAt: now,
|
||||
}); err != nil {
|
||||
t.Fatalf("UpsertExecutorRunning: %v", err)
|
||||
}
|
||||
|
||||
_, err = svc.ResumeTaskSession(ctx, "task1", "session1")
|
||||
if !errors.Is(err, launchErr) {
|
||||
t.Fatalf("ResumeTaskSession error = %v, want %v", err, launchErr)
|
||||
}
|
||||
failed, err := repo.GetTaskSession(ctx, "session1")
|
||||
if err != nil {
|
||||
t.Fatalf("GetTaskSession after failure: %v", err)
|
||||
}
|
||||
if failed.State != models.TaskSessionStateFailed {
|
||||
t.Fatalf("session state = %s, want FAILED", failed.State)
|
||||
}
|
||||
taskRepo.mu.Lock()
|
||||
defer taskRepo.mu.Unlock()
|
||||
if taskRepo.stateWrites["task1"] != 0 || taskRepo.unconditionalWrites["task1"] != 0 {
|
||||
t.Fatalf("resume bypassed conditional task write: conditional=%d unconditional=%d", taskRepo.stateWrites["task1"], taskRepo.unconditionalWrites["task1"])
|
||||
}
|
||||
}
|
||||
|
||||
// TestResumeTaskSession_FailedKeepsResumeToken verifies that resuming a FAILED
|
||||
// session preserves the ACP resume token so the relaunched agent restores the
|
||||
// prior conversation via ACP session/load (for native-resume agents).
|
||||
@@ -3552,6 +4047,93 @@ func TestResumeTaskSession_FailedStateWriteSurvivesCancelledCallerCtx(t *testing
|
||||
}
|
||||
}
|
||||
|
||||
func TestResumeTaskSession_AlreadyFailedMissingRemoteRefCreatesNeutralRecoveryMessage(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
repo := setupTestRepo(t)
|
||||
taskRepo := newMockTaskRepo()
|
||||
launchErr := errors.New("environment preparation failed: fatal: couldn't find remote ref feature/foo")
|
||||
agentMgr := &mockAgentManager{
|
||||
launchAgentFunc: func(_ context.Context, _ *executor.LaunchAgentRequest) (*executor.LaunchAgentResponse, error) {
|
||||
return nil, launchErr
|
||||
},
|
||||
}
|
||||
svc := createTestServiceWithAgent(repo, newMockStepGetter(), taskRepo, agentMgr)
|
||||
svc.executor = executor.NewExecutor(agentMgr, repo, testLogger(), executor.ExecutorConfig{})
|
||||
svc.messageCreator = &mockMessageCreator{}
|
||||
|
||||
seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateFailed)
|
||||
session, err := repo.GetTaskSession(ctx, "session1")
|
||||
if err != nil {
|
||||
t.Fatalf("get seeded session: %v", err)
|
||||
}
|
||||
session.AgentProfileID = "profile-1"
|
||||
if err := repo.UpdateTaskSession(ctx, session); err != nil {
|
||||
t.Fatalf("update seeded session: %v", err)
|
||||
}
|
||||
now := time.Now().UTC()
|
||||
if err := repo.UpsertExecutorRunning(ctx, &models.ExecutorRunning{
|
||||
ID: "resume-missing-ref", SessionID: "session1", TaskID: "task1", CreatedAt: now, UpdatedAt: now,
|
||||
}); err != nil {
|
||||
t.Fatalf("seed executor record: %v", err)
|
||||
}
|
||||
|
||||
_, err = svc.ResumeTaskSession(ctx, "task1", "session1")
|
||||
if !errors.Is(err, launchErr) {
|
||||
t.Fatalf("ResumeTaskSession error = %v, want %v", err, launchErr)
|
||||
}
|
||||
|
||||
messages := svc.messageCreator.(*mockMessageCreator).sessionMessages
|
||||
if len(messages) != 1 {
|
||||
t.Fatalf("expected one recovery message, got %d", len(messages))
|
||||
}
|
||||
message := messages[0]
|
||||
if message.metadata["failure_kind"] != "branch_fetch_failed" {
|
||||
t.Fatalf("failure_kind = %#v, want branch_fetch_failed", message.metadata["failure_kind"])
|
||||
}
|
||||
if _, ok := message.metadata["actions"]; ok {
|
||||
t.Fatalf("expected no destructive actions without a repository-scoped PR match, got %#v", message.metadata["actions"])
|
||||
}
|
||||
taskRepo.mu.Lock()
|
||||
defer taskRepo.mu.Unlock()
|
||||
if taskRepo.stateWrites["task1"] != 0 {
|
||||
t.Fatalf("already failed resume rewrote task state %d times", taskRepo.stateWrites["task1"])
|
||||
}
|
||||
}
|
||||
|
||||
func TestResumeTaskSession_UnrelatedLaunchFailureDoesNotCreateRecoveryMessage(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
repo := setupTestRepo(t)
|
||||
taskRepo := newMockTaskRepo()
|
||||
launchErr := errors.New("failed to launch container")
|
||||
agentMgr := &mockAgentManager{
|
||||
launchAgentFunc: func(_ context.Context, _ *executor.LaunchAgentRequest) (*executor.LaunchAgentResponse, error) {
|
||||
return nil, launchErr
|
||||
},
|
||||
}
|
||||
svc := createTestServiceWithAgent(repo, newMockStepGetter(), taskRepo, agentMgr)
|
||||
svc.executor = executor.NewExecutor(agentMgr, repo, testLogger(), executor.ExecutorConfig{})
|
||||
messageCreator := &mockMessageCreator{}
|
||||
svc.messageCreator = messageCreator
|
||||
|
||||
seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateFailed)
|
||||
session, err := repo.GetTaskSession(ctx, "session1")
|
||||
if err != nil {
|
||||
t.Fatalf("get seeded session: %v", err)
|
||||
}
|
||||
session.AgentProfileID = "profile-1"
|
||||
if err := repo.UpdateTaskSession(ctx, session); err != nil {
|
||||
t.Fatalf("update seeded session: %v", err)
|
||||
}
|
||||
|
||||
_, err = svc.ResumeTaskSession(ctx, "task1", "session1")
|
||||
if !errors.Is(err, launchErr) {
|
||||
t.Fatalf("ResumeTaskSession error = %v, want %v", err, launchErr)
|
||||
}
|
||||
if len(messageCreator.sessionMessages) != 0 {
|
||||
t.Fatalf("expected no recovery message, got %#v", messageCreator.sessionMessages)
|
||||
}
|
||||
}
|
||||
|
||||
// --- CompleteTask ---
|
||||
|
||||
func TestCompleteTask_UpdatesTaskState(t *testing.T) {
|
||||
|
||||
@@ -952,6 +952,56 @@ func (r *Repository) SetSessionMetadataKeyIfAbsent(
|
||||
return rows > 0, nil
|
||||
}
|
||||
|
||||
// SetSessionMetadataKeyIfAbsentIfState atomically claims a metadata key only
|
||||
// while the session remains in expectedState. It is used when a terminal
|
||||
// transition owns a one-time side effect that must not be emitted by a stale
|
||||
// launch callback.
|
||||
func (r *Repository) SetSessionMetadataKeyIfAbsentIfState(
|
||||
ctx context.Context,
|
||||
sessionID, key string,
|
||||
value interface{},
|
||||
expectedState models.TaskSessionState,
|
||||
) (bool, error) {
|
||||
valueJSON, err := json.Marshal(value)
|
||||
if err != nil {
|
||||
return false, fmt.Errorf("failed to serialize metadata value: %w", err)
|
||||
}
|
||||
now := time.Now().UTC()
|
||||
driver := r.db.DriverName()
|
||||
path := key
|
||||
if !dialect.IsPostgres(driver) {
|
||||
path = "$." + key
|
||||
}
|
||||
result, err := r.db.ExecContext(ctx, r.db.Rebind(setSessionMetadataKeyIfAbsentIfStateQuery(driver)), path, string(valueJSON), now, sessionID, path, expectedState)
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
rows, _ := result.RowsAffected()
|
||||
return rows > 0, nil
|
||||
}
|
||||
|
||||
// RemoveSessionMetadataKeyIfState removes a claimed metadata key only while
|
||||
// the session remains in expectedState. It releases one-time side-effect
|
||||
// claims when their downstream write fails, allowing a later retry.
|
||||
func (r *Repository) RemoveSessionMetadataKeyIfState(
|
||||
ctx context.Context,
|
||||
sessionID, key string,
|
||||
expectedState models.TaskSessionState,
|
||||
) (bool, error) {
|
||||
now := time.Now().UTC()
|
||||
driver := r.db.DriverName()
|
||||
path := key
|
||||
if !dialect.IsPostgres(driver) {
|
||||
path = "$." + key
|
||||
}
|
||||
result, err := r.db.ExecContext(ctx, r.db.Rebind(removeSessionMetadataKeyIfStateQuery(driver)), path, now, sessionID, path, expectedState)
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
rows, _ := result.RowsAffected()
|
||||
return rows > 0, nil
|
||||
}
|
||||
|
||||
func setSessionMetadataKeyIfAbsentQuery(driver string) string {
|
||||
if dialect.IsPostgres(driver) {
|
||||
return `
|
||||
@@ -979,6 +1029,37 @@ func setSessionMetadataKeyIfAbsentQuery(driver string) string {
|
||||
`
|
||||
}
|
||||
|
||||
func setSessionMetadataKeyIfAbsentIfStateQuery(driver string) string {
|
||||
return setSessionMetadataKeyIfAbsentQuery(driver) + " AND state = ?"
|
||||
}
|
||||
|
||||
func removeSessionMetadataKeyIfStateQuery(driver string) string {
|
||||
if dialect.IsPostgres(driver) {
|
||||
return `
|
||||
UPDATE task_sessions
|
||||
SET metadata = (
|
||||
CASE WHEN metadata IS NULL OR metadata = 'null' OR metadata = '' THEN '{}'::jsonb ELSE metadata::jsonb END
|
||||
#- ARRAY[?]::text[]
|
||||
)::text,
|
||||
updated_at = ?
|
||||
WHERE id = ?
|
||||
AND jsonb_extract_path(
|
||||
CASE WHEN metadata IS NULL OR metadata = 'null' OR metadata = '' THEN '{}'::jsonb ELSE metadata::jsonb END,
|
||||
?
|
||||
) IS NOT NULL
|
||||
AND state = ?
|
||||
`
|
||||
}
|
||||
return `
|
||||
UPDATE task_sessions
|
||||
SET metadata = json_remove(CASE WHEN metadata IS NULL OR metadata = 'null' OR metadata = '' THEN '{}' ELSE metadata END, ?),
|
||||
updated_at = ?
|
||||
WHERE id = ?
|
||||
AND json_type(CASE WHEN metadata IS NULL OR metadata = 'null' OR metadata = '' THEN '{}' ELSE metadata END, ?) IS NOT NULL
|
||||
AND state = ?
|
||||
`
|
||||
}
|
||||
|
||||
// SetSessionACPSessionID mirrors the agent's ACP session id into the session's
|
||||
// "acp" metadata map as a single atomic UPDATE. json_patch merges the sub-key,
|
||||
// so keys session_info already wrote (title, ...) survive without a
|
||||
|
||||
@@ -223,6 +223,78 @@ func TestSetSessionMetadataKeyIfAbsentSQLiteIsWriteOnce(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestSetSessionMetadataKeyIfAbsentIfStateSQLiteRequiresFailedSession(t *testing.T) {
|
||||
repo := newRepoForSessionTests(t)
|
||||
seedForMsgTest(t, repo, "task-recovery", "session-recovery", "turn-recovery")
|
||||
ctx := context.Background()
|
||||
|
||||
stored, err := repo.SetSessionMetadataKeyIfAbsentIfState(
|
||||
ctx, "session-recovery", "missing_pr_branch_recovery_claimed", true, models.TaskSessionStateFailed,
|
||||
)
|
||||
if err != nil {
|
||||
t.Fatalf("claim before failed state: %v", err)
|
||||
}
|
||||
if stored {
|
||||
t.Fatal("claim should reject a session that is not FAILED")
|
||||
}
|
||||
if err := repo.UpdateTaskSessionState(ctx, "session-recovery", models.TaskSessionStateFailed, "missing branch"); err != nil {
|
||||
t.Fatalf("mark failed: %v", err)
|
||||
}
|
||||
stored, err = repo.SetSessionMetadataKeyIfAbsentIfState(
|
||||
ctx, "session-recovery", "missing_pr_branch_recovery_claimed", true, models.TaskSessionStateFailed,
|
||||
)
|
||||
if err != nil {
|
||||
t.Fatalf("claim after failed state: %v", err)
|
||||
}
|
||||
if !stored {
|
||||
t.Fatal("claim should store after FAILED state")
|
||||
}
|
||||
stored, err = repo.SetSessionMetadataKeyIfAbsentIfState(
|
||||
ctx, "session-recovery", "missing_pr_branch_recovery_claimed", true, models.TaskSessionStateFailed,
|
||||
)
|
||||
if err != nil {
|
||||
t.Fatalf("repeat claim: %v", err)
|
||||
}
|
||||
if stored {
|
||||
t.Fatal("repeat claim should not overwrite")
|
||||
}
|
||||
}
|
||||
|
||||
func TestRemoveSessionMetadataKeyIfStateSQLiteRequiresFailedSession(t *testing.T) {
|
||||
repo := newRepoForSessionTests(t)
|
||||
seedForMsgTest(t, repo, "task-recovery", "session-recovery", "turn-recovery")
|
||||
ctx := context.Background()
|
||||
const key = "missing_pr_branch_recovery_claimed"
|
||||
|
||||
if err := repo.SetSessionMetadataKey(ctx, "session-recovery", key, true); err != nil {
|
||||
t.Fatalf("seed claim: %v", err)
|
||||
}
|
||||
removed, err := repo.RemoveSessionMetadataKeyIfState(ctx, "session-recovery", key, models.TaskSessionStateFailed)
|
||||
if err != nil {
|
||||
t.Fatalf("remove before failed state: %v", err)
|
||||
}
|
||||
if removed {
|
||||
t.Fatal("remove should reject a session that is not FAILED")
|
||||
}
|
||||
if err := repo.UpdateTaskSessionState(ctx, "session-recovery", models.TaskSessionStateFailed, "missing branch"); err != nil {
|
||||
t.Fatalf("mark failed: %v", err)
|
||||
}
|
||||
removed, err = repo.RemoveSessionMetadataKeyIfState(ctx, "session-recovery", key, models.TaskSessionStateFailed)
|
||||
if err != nil {
|
||||
t.Fatalf("remove after failed state: %v", err)
|
||||
}
|
||||
if !removed {
|
||||
t.Fatal("remove should delete the failed-session claim")
|
||||
}
|
||||
session, err := repo.GetTaskSession(ctx, "session-recovery")
|
||||
if err != nil {
|
||||
t.Fatalf("get session after remove: %v", err)
|
||||
}
|
||||
if _, exists := session.Metadata[key]; exists {
|
||||
t.Fatal("removed claim remains in session metadata")
|
||||
}
|
||||
}
|
||||
|
||||
// TestSetSessionMetadataKeyIfAbsentQueryUsesPostgresJSONB verifies the
|
||||
// Postgres dialect's write-once query uses jsonb_set/jsonb_extract_path
|
||||
// rather than SQLite's json_set/json_type/json functions.
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { cleanup, render, screen } from "@testing-library/react";
|
||||
import { cleanup, fireEvent, render, screen } from "@testing-library/react";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import type { Message } from "@/lib/types/http";
|
||||
import type { PrepareStepInfo } from "@/lib/state/slices/session-runtime/types";
|
||||
@@ -7,6 +7,8 @@ let mockSteps: PrepareStepInfo[] = [];
|
||||
let mockPrepareStatus: "preparing" | "completed" | "failed" = "preparing";
|
||||
let mockSessionState: string = "STARTING";
|
||||
let mockMessages: Message[] = [];
|
||||
let mockPrepareError: string | undefined;
|
||||
const CREATE_WORKTREE = "Create worktree";
|
||||
|
||||
vi.mock("@/components/state-provider", () => ({
|
||||
useAppStore: (selector: (state: Record<string, unknown>) => unknown) =>
|
||||
@@ -17,6 +19,7 @@ vi.mock("@/components/state-provider", () => ({
|
||||
sessionId: "session-1",
|
||||
status: mockPrepareStatus,
|
||||
steps: mockSteps,
|
||||
errorMessage: mockPrepareError,
|
||||
},
|
||||
},
|
||||
},
|
||||
@@ -45,6 +48,7 @@ describe("PrepareProgress", () => {
|
||||
afterEach(() => {
|
||||
cleanup();
|
||||
mockMessages = [];
|
||||
mockPrepareError = undefined;
|
||||
});
|
||||
|
||||
it("hides skipped steps that have no useful details", () => {
|
||||
@@ -127,6 +131,32 @@ describe("PrepareProgress", () => {
|
||||
expect(screen.getByText("Environment prepared with warnings")).toBeTruthy();
|
||||
expect(screen.queryByText("Environment prepared on a fresh sandbox")).toBeNull();
|
||||
});
|
||||
|
||||
it("keeps failed preparation diagnostics collapsed until requested", () => {
|
||||
mockPrepareStatus = "failed";
|
||||
mockSessionState = "FAILED";
|
||||
mockPrepareError =
|
||||
"branch feature/very-long-name not found locally or on remote: fatal: could not resolve host";
|
||||
mockSteps = [
|
||||
{
|
||||
name: CREATE_WORKTREE,
|
||||
status: "failed",
|
||||
error: "fatal: could not resolve host github.com",
|
||||
output: "git fetch origin feature/very-long-name",
|
||||
},
|
||||
];
|
||||
|
||||
render(<PrepareProgress sessionId="session-1" />);
|
||||
|
||||
expect(screen.getByText("Environment setup failed")).toBeTruthy();
|
||||
expect(screen.queryByText(/branch feature\/very-long-name not found/)).toBeNull();
|
||||
expect(screen.queryByText(CREATE_WORKTREE)).toBeNull();
|
||||
|
||||
fireEvent.click(screen.getByRole("button", { name: "Show preparation details" }));
|
||||
|
||||
expect(screen.getByText(/branch feature\/very-long-name not found/)).toBeTruthy();
|
||||
expect(screen.getByText(CREATE_WORKTREE)).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
||||
function makeSetupScriptMessage(overrides: Partial<Message["metadata"] & object> = {}): Message {
|
||||
@@ -162,7 +192,7 @@ describe("PrepareProgress per-repo setup script", () => {
|
||||
mockSessionState = "STARTING";
|
||||
mockSteps = [
|
||||
{ name: "Validate repository", status: "completed" },
|
||||
{ name: "Create worktree", status: "completed" },
|
||||
{ name: CREATE_WORKTREE, status: "completed" },
|
||||
];
|
||||
mockMessages = [makeSetupScriptMessage()];
|
||||
|
||||
@@ -176,11 +206,12 @@ describe("PrepareProgress per-repo setup script", () => {
|
||||
// `failed` keeps the panel auto-expanded.
|
||||
mockPrepareStatus = "failed";
|
||||
mockSessionState = "FAILED";
|
||||
mockSteps = [{ name: "Create worktree", status: "completed" }];
|
||||
mockSteps = [{ name: CREATE_WORKTREE, status: "completed" }];
|
||||
mockMessages = [makeSetupScriptMessage({ exit_code: 2 })];
|
||||
|
||||
render(<PrepareProgress sessionId="session-1" />);
|
||||
|
||||
fireEvent.click(screen.getByRole("button", { name: "Show preparation details" }));
|
||||
expect(screen.getByText("Script exited with code 2")).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -78,7 +78,7 @@ function StepDetails({ step, blockCommand }: { step: PrepareStepInfo; blockComma
|
||||
</pre>
|
||||
)}
|
||||
{step.output && (
|
||||
<pre className="text-muted-foreground/60 max-h-48 overflow-auto whitespace-pre text-xs">
|
||||
<pre className="text-muted-foreground/60 max-h-48 max-w-full overflow-y-auto whitespace-pre-wrap break-words text-xs">
|
||||
{stripAnsi(step.output)}
|
||||
</pre>
|
||||
)}
|
||||
@@ -101,7 +101,7 @@ function StepWarning({ warning, warningDetail }: { warning: string; warningDetai
|
||||
Details
|
||||
</button>
|
||||
{detailExpanded && (
|
||||
<pre className="text-amber-500/60 mt-0.5 overflow-x-auto whitespace-pre text-xs">
|
||||
<pre className="text-amber-500/60 mt-0.5 max-w-full whitespace-pre-wrap break-words text-xs">
|
||||
{warningDetail}
|
||||
</pre>
|
||||
)}
|
||||
@@ -116,7 +116,7 @@ function StepMessages({ step }: { step: PrepareStepInfo }) {
|
||||
<>
|
||||
{step.warning && <StepWarning warning={step.warning} warningDetail={step.warningDetail} />}
|
||||
{step.error && (
|
||||
<pre className="text-destructive mt-0.5 overflow-x-auto whitespace-pre text-xs">
|
||||
<pre className="text-destructive mt-0.5 max-w-full whitespace-pre-wrap break-words text-xs">
|
||||
{step.error}
|
||||
</pre>
|
||||
)}
|
||||
@@ -140,13 +140,15 @@ function StepRow({ step }: { step: PrepareStepInfo }) {
|
||||
|
||||
return (
|
||||
<div className="text-xs">
|
||||
<div className="flex items-center gap-2">
|
||||
<div className="flex min-w-0 flex-wrap items-center gap-2">
|
||||
<div className="flex-shrink-0">
|
||||
<StepIcon status={step.status} hasWarning={Boolean(step.warning)} />
|
||||
</div>
|
||||
<span className={nameClass}>{step.name || "Preparing..."}</span>
|
||||
{inlineCommand && (
|
||||
<code className="text-muted-foreground/50 font-mono text-[10px]">{inlineCommand}</code>
|
||||
<code className="text-muted-foreground/50 min-w-0 break-all font-mono text-[10px]">
|
||||
{inlineCommand}
|
||||
</code>
|
||||
)}
|
||||
{duration && <span className="text-muted-foreground/40 text-[10px]">{duration}</span>}
|
||||
{hasExpandable && (
|
||||
@@ -279,11 +281,9 @@ function HeaderIcon({ status }: { status: EffectiveStatus }) {
|
||||
|
||||
function getHeaderLabel(status: EffectiveStatus, prepareState: SessionPrepareState): string {
|
||||
if (status === "preparing") return "Preparing environment...";
|
||||
if (status === "failed") return prepareState.errorMessage || "Environment preparation failed";
|
||||
if (status === "failed") return "Environment setup failed";
|
||||
if (status === "completed_with_error") {
|
||||
const failed = prepareState.steps.find((s) => s.status === "failed");
|
||||
// `||` (not `??`) so an empty-string error also falls through to the name fallback.
|
||||
return failed?.error || `${failed?.name ?? "Environment prepared"} — step failed`;
|
||||
return "Environment setup finished with errors";
|
||||
}
|
||||
if (status === "completed_with_warnings") {
|
||||
const warningSteps = prepareState.steps.filter((s) => s.warning);
|
||||
@@ -452,13 +452,21 @@ function logPrepareSnapshotChange(
|
||||
});
|
||||
}
|
||||
|
||||
function hasPrepareDetails(
|
||||
visibleStepCount: number,
|
||||
hasSessionInfo: boolean,
|
||||
hasFailureDetails: boolean,
|
||||
): boolean {
|
||||
return visibleStepCount > 0 || hasSessionInfo || hasFailureDetails;
|
||||
}
|
||||
|
||||
export function PrepareProgress({ sessionId }: PrepareProgressProps) {
|
||||
const { status, prepareState } = usePrepareStatus(sessionId);
|
||||
const session = useAppStore((state) => state.taskSessions.items[sessionId]);
|
||||
const setupScriptSteps = useSetupScriptSteps(sessionId);
|
||||
// Only the clean-success status auto-collapses. Anything with running/error/
|
||||
// warning context stays open by default so the user sees what's happening.
|
||||
const autoExpand = status !== "completed";
|
||||
// Keep active progress and warnings visible. Failure diagnostics remain
|
||||
// secondary until the user explicitly asks to inspect them.
|
||||
const autoExpand = status === "preparing" || status === "completed_with_warnings";
|
||||
const [expanded, setExpanded] = useExpandedFlag(sessionId, autoExpand);
|
||||
|
||||
// Track status/expand transitions over the panel's lifetime — the
|
||||
@@ -496,20 +504,55 @@ export function PrepareProgress({ sessionId }: PrepareProgressProps) {
|
||||
const headerLabel = getHeaderLabel(status, prepareState);
|
||||
const isErrorStatus = status === "failed" || status === "completed_with_error";
|
||||
const headerClass = cn("text-xs", isErrorStatus ? "text-destructive" : "text-muted-foreground");
|
||||
const hasFailureDetails = isErrorStatus && Boolean(prepareState.errorMessage);
|
||||
const hasExpandableContent = hasPrepareDetails(
|
||||
visibleSteps.length,
|
||||
hasSessionInfo,
|
||||
hasFailureDetails,
|
||||
);
|
||||
|
||||
return (
|
||||
<div data-testid="prepare-progress-panel" data-status={status} data-expanded={expanded}>
|
||||
<div
|
||||
data-testid="prepare-progress-panel"
|
||||
data-status={status}
|
||||
data-expanded={expanded}
|
||||
className="min-w-0 max-w-full"
|
||||
>
|
||||
<ExpandableRow
|
||||
icon={<HeaderIcon status={status} />}
|
||||
header={
|
||||
<span data-testid="prepare-progress-toggle" className={headerClass}>
|
||||
{headerLabel}
|
||||
</span>
|
||||
<div className="flex min-w-0 flex-wrap items-center justify-between gap-2">
|
||||
<span
|
||||
data-testid="prepare-progress-toggle"
|
||||
className={cn(headerClass, "min-w-0 break-words font-medium")}
|
||||
>
|
||||
{headerLabel}
|
||||
</span>
|
||||
{hasExpandableContent && (
|
||||
<button
|
||||
type="button"
|
||||
aria-expanded={expanded}
|
||||
aria-label={expanded ? "Hide preparation details" : "Show preparation details"}
|
||||
className="min-h-11 cursor-pointer text-xs text-muted-foreground underline-offset-4 hover:underline sm:min-h-0"
|
||||
onClick={(event) => {
|
||||
event.stopPropagation();
|
||||
setExpanded(!expanded);
|
||||
}}
|
||||
>
|
||||
{expanded ? "Hide details" : "Show details"}
|
||||
</button>
|
||||
)}
|
||||
</div>
|
||||
}
|
||||
hasExpandableContent={visibleSteps.length > 0 || hasSessionInfo}
|
||||
hasExpandableContent={hasExpandableContent}
|
||||
isExpanded={expanded}
|
||||
onToggle={() => setExpanded(!expanded)}
|
||||
>
|
||||
{hasFailureDetails && (
|
||||
<pre className="mb-2 max-h-48 max-w-full overflow-y-auto whitespace-pre-wrap break-words rounded bg-muted/40 p-2 text-xs text-muted-foreground">
|
||||
{prepareState.errorMessage}
|
||||
</pre>
|
||||
)}
|
||||
<div className="space-y-1">
|
||||
{visibleSteps.map((step, i) => (
|
||||
<StepRow key={i} step={step} />
|
||||
|
||||
@@ -0,0 +1,113 @@
|
||||
import { cleanup, render, screen } from "@testing-library/react";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { sessionId as toSessionId, taskId as toTaskId, type Message } from "@/lib/types/http";
|
||||
|
||||
vi.mock("@/components/task/chat/messages/agent-status", () => ({
|
||||
AgentStatus: () => <div data-testid="agent-status">Agent failed</div>,
|
||||
}));
|
||||
|
||||
vi.mock("@/components/task/chat/message-renderer", () => ({
|
||||
MessageRenderer: ({ comment }: { comment: Message }) => (
|
||||
<div data-testid="action-message">{comment.content}</div>
|
||||
),
|
||||
}));
|
||||
|
||||
import { MessageListFooter } from "./message-list-footer";
|
||||
|
||||
afterEach(cleanup);
|
||||
|
||||
const AGENT_STATUS_TEST_ID = "agent-status";
|
||||
|
||||
const actionableFailure = {
|
||||
id: "failure-1",
|
||||
session_id: toSessionId("session-1"),
|
||||
task_id: toTaskId("task-1"),
|
||||
author_type: "agent",
|
||||
type: "status",
|
||||
created_at: "2026-07-22T00:00:00Z",
|
||||
content: "Branch recovery",
|
||||
metadata: {
|
||||
variant: "warning",
|
||||
failure_kind: "missing_pr_branch",
|
||||
actions: [{ type: "archive_task", label: "Archive task" }],
|
||||
},
|
||||
} satisfies Message;
|
||||
|
||||
const laterFailure = {
|
||||
...actionableFailure,
|
||||
id: "failure-2",
|
||||
content: "Agent encountered an authentication error",
|
||||
metadata: {
|
||||
variant: "error",
|
||||
recovery_actions: true,
|
||||
actions: [{ type: "ws_request", label: "Resume session" }],
|
||||
},
|
||||
} satisfies Message;
|
||||
|
||||
describe("MessageListFooter", () => {
|
||||
it("lets an actionable footer failure own the failure presentation", () => {
|
||||
render(
|
||||
<MessageListFooter
|
||||
sessionState="FAILED"
|
||||
sessionId="session-1"
|
||||
messages={[]}
|
||||
footerActionMessages={[actionableFailure]}
|
||||
/>,
|
||||
);
|
||||
|
||||
expect(screen.getByTestId("action-message")).toBeTruthy();
|
||||
expect(screen.queryByTestId(AGENT_STATUS_TEST_ID)).toBeNull();
|
||||
});
|
||||
|
||||
it("keeps the generic status for a failed session without an actionable footer", () => {
|
||||
render(<MessageListFooter sessionState="FAILED" sessionId="session-1" messages={[]} />);
|
||||
|
||||
expect(screen.getByTestId(AGENT_STATUS_TEST_ID)).toBeTruthy();
|
||||
});
|
||||
|
||||
it("retains the running status when an action message is hidden during startup", () => {
|
||||
render(
|
||||
<MessageListFooter
|
||||
sessionState="STARTING"
|
||||
sessionId="session-1"
|
||||
messages={[]}
|
||||
footerActionMessages={[actionableFailure]}
|
||||
/>,
|
||||
);
|
||||
|
||||
expect(screen.getByTestId(AGENT_STATUS_TEST_ID)).toBeTruthy();
|
||||
});
|
||||
|
||||
it("switches ownership when missing-branch recovery arrives after failure", () => {
|
||||
const { rerender } = render(
|
||||
<MessageListFooter sessionState="FAILED" sessionId="session-1" messages={[]} />,
|
||||
);
|
||||
expect(screen.getByTestId(AGENT_STATUS_TEST_ID)).toBeTruthy();
|
||||
|
||||
rerender(
|
||||
<MessageListFooter
|
||||
sessionState="FAILED"
|
||||
sessionId="session-1"
|
||||
messages={[]}
|
||||
footerActionMessages={[actionableFailure]}
|
||||
/>,
|
||||
);
|
||||
|
||||
expect(screen.queryByTestId(AGENT_STATUS_TEST_ID)).toBeNull();
|
||||
expect(screen.getByTestId("action-message")).toBeTruthy();
|
||||
});
|
||||
|
||||
it("does not restore stale missing-branch recovery after a later failure", () => {
|
||||
render(
|
||||
<MessageListFooter
|
||||
sessionState="FAILED"
|
||||
sessionId="session-1"
|
||||
messages={[actionableFailure, laterFailure]}
|
||||
footerActionMessages={[actionableFailure]}
|
||||
/>,
|
||||
);
|
||||
|
||||
expect(screen.getByTestId(AGENT_STATUS_TEST_ID)).toBeTruthy();
|
||||
expect(screen.queryByTestId("action-message")).toBeNull();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,60 @@
|
||||
"use client";
|
||||
|
||||
import type { Message, TaskSessionState } from "@/lib/types/http";
|
||||
import { AgentStatus } from "@/components/task/chat/messages/agent-status";
|
||||
import { MessageRenderer } from "@/components/task/chat/message-renderer";
|
||||
|
||||
type MessageListFooterProps = {
|
||||
sessionState?: TaskSessionState;
|
||||
sessionId: string | null;
|
||||
messages: Message[];
|
||||
footerActionMessages?: Message[];
|
||||
};
|
||||
|
||||
function isMissingBranchFailure(message: Message): boolean {
|
||||
const metadata = message.metadata as Record<string, unknown> | undefined;
|
||||
const actions = metadata?.actions;
|
||||
if (!Array.isArray(actions) || actions.length === 0) return false;
|
||||
return metadata?.failure_kind === "missing_pr_branch";
|
||||
}
|
||||
|
||||
function isActionableFailure(message: Message): boolean {
|
||||
const metadata = message.metadata as Record<string, unknown> | undefined;
|
||||
return Array.isArray(metadata?.actions) && metadata.actions.length > 0;
|
||||
}
|
||||
|
||||
function findCurrentActionableFailure(
|
||||
messages: Message[],
|
||||
footerActionMessages: Message[],
|
||||
): Message | undefined {
|
||||
for (let index = messages.length - 1; index >= 0; index--) {
|
||||
if (isActionableFailure(messages[index])) return messages[index];
|
||||
}
|
||||
return footerActionMessages.at(-1);
|
||||
}
|
||||
|
||||
export function MessageListFooter({
|
||||
sessionState,
|
||||
sessionId,
|
||||
messages,
|
||||
footerActionMessages = [],
|
||||
}: MessageListFooterProps) {
|
||||
const currentActionableFailure = findCurrentActionableFailure(messages, footerActionMessages);
|
||||
const recoveryOwnsFailure =
|
||||
sessionState === "FAILED" &&
|
||||
currentActionableFailure !== undefined &&
|
||||
isMissingBranchFailure(currentActionableFailure);
|
||||
const visibleFooterActionMessages = footerActionMessages.filter(
|
||||
(message) => !isMissingBranchFailure(message) || message.id === currentActionableFailure?.id,
|
||||
);
|
||||
return (
|
||||
<>
|
||||
{!recoveryOwnsFailure && (
|
||||
<AgentStatus sessionState={sessionState} sessionId={sessionId} messages={messages} />
|
||||
)}
|
||||
{visibleFooterActionMessages.map((message) => (
|
||||
<MessageRenderer key={message.id} comment={message} isTaskDescription={false} />
|
||||
))}
|
||||
</>
|
||||
);
|
||||
}
|
||||
@@ -4,9 +4,8 @@ import { useCallback, useEffect, useLayoutEffect, useMemo, useRef, memo } from "
|
||||
import { SessionPanelContent } from "@kandev/ui/pannel-session";
|
||||
import { useDockviewStore } from "@/lib/state/dockview-store";
|
||||
import type { Message } from "@/lib/types/http";
|
||||
import { AgentStatus } from "@/components/task/chat/messages/agent-status";
|
||||
import { MessageRenderer } from "@/components/task/chat/message-renderer";
|
||||
import { useLazyLoadMessages } from "@/hooks/use-lazy-load-messages";
|
||||
import { MessageListFooter } from "./message-list-footer";
|
||||
import {
|
||||
type MessageListProps,
|
||||
MessageListStatus,
|
||||
@@ -256,10 +255,12 @@ export const NativeMessageList = memo(function NativeMessageList({
|
||||
);
|
||||
})}
|
||||
|
||||
<AgentStatus sessionState={sessionState} sessionId={sessionId} messages={messages} />
|
||||
{(footerActionMessages ?? []).map((msg: Message) => (
|
||||
<MessageRenderer key={msg.id} comment={msg} isTaskDescription={false} />
|
||||
))}
|
||||
<MessageListFooter
|
||||
sessionState={sessionState}
|
||||
sessionId={sessionId}
|
||||
messages={messages}
|
||||
footerActionMessages={footerActionMessages}
|
||||
/>
|
||||
|
||||
{/* Bottom anchor — browser keeps scroll pinned here when new content appends */}
|
||||
<div style={{ overflowAnchor: "auto", height: 1 }} />
|
||||
|
||||
@@ -6,9 +6,8 @@ import { Virtuoso, type VirtuosoHandle } from "react-virtuoso";
|
||||
import { SessionPanelContent } from "@kandev/ui/pannel-session";
|
||||
import type { RenderItem } from "@/hooks/use-processed-messages";
|
||||
import type { Message, TaskSessionState } from "@/lib/types/http";
|
||||
import { AgentStatus } from "@/components/task/chat/messages/agent-status";
|
||||
import { MessageRenderer } from "@/components/task/chat/message-renderer";
|
||||
import { useLazyLoadMessages } from "@/hooks/use-lazy-load-messages";
|
||||
import { MessageListFooter } from "./message-list-footer";
|
||||
import {
|
||||
type MessageListProps,
|
||||
MessageListStatus,
|
||||
@@ -431,12 +430,12 @@ function useVirtuosoHeaderFooter(args: HeaderFooterArgs) {
|
||||
|
||||
const Footer = useCallback(
|
||||
() => (
|
||||
<>
|
||||
<AgentStatus sessionState={sessionState} sessionId={sessionId} messages={messages} />
|
||||
{footerActions.map((msg) => (
|
||||
<MessageRenderer key={msg.id} comment={msg} isTaskDescription={false} />
|
||||
))}
|
||||
</>
|
||||
<MessageListFooter
|
||||
sessionState={sessionState}
|
||||
sessionId={sessionId}
|
||||
messages={messages}
|
||||
footerActionMessages={footerActions}
|
||||
/>
|
||||
),
|
||||
[sessionId, sessionState, messages, footerActions],
|
||||
);
|
||||
@@ -503,10 +502,12 @@ export const VirtuosoMessageList = memo(function VirtuosoMessageList(props: Mess
|
||||
messagesCount={messages.length}
|
||||
onLoadMore={loadMore}
|
||||
/>
|
||||
<AgentStatus sessionState={sessionState} sessionId={sessionId} messages={messages} />
|
||||
{footerActions.map((msg) => (
|
||||
<MessageRenderer key={msg.id} comment={msg} isTaskDescription={false} />
|
||||
))}
|
||||
<MessageListFooter
|
||||
sessionState={sessionState}
|
||||
sessionId={sessionId}
|
||||
messages={messages}
|
||||
footerActionMessages={footerActions}
|
||||
/>
|
||||
</SessionPanelContent>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -23,6 +23,7 @@ afterEach(() => {
|
||||
});
|
||||
|
||||
const CANCEL_TEST_ID = "recovery-cancel-retry-button";
|
||||
const TECHNICAL_DETAILS = "Technical details";
|
||||
|
||||
function retryMessage(overrides: Partial<Message> = {}): Message {
|
||||
return {
|
||||
@@ -60,9 +61,15 @@ function retryMessage(overrides: Partial<Message> = {}): Message {
|
||||
|
||||
/** ActionMessage reads session state from the store (keyed by comment.session_id),
|
||||
* so seed it via the provider instead of passing a prop. */
|
||||
function renderAction(comment: Message, sessionState?: TaskSessionState) {
|
||||
function renderAction(comment: Message, sessionState?: TaskSessionState, sessionError?: string) {
|
||||
const initialState: Partial<AppState> = sessionState
|
||||
? { taskSessions: { items: { "sess-1": { state: sessionState } as TaskSession } } }
|
||||
? {
|
||||
taskSessions: {
|
||||
items: {
|
||||
"sess-1": { state: sessionState, error_message: sessionError } as TaskSession,
|
||||
},
|
||||
},
|
||||
}
|
||||
: {};
|
||||
return render(<ActionMessage comment={comment} />, {
|
||||
wrapper: ({ children }) => (
|
||||
@@ -95,6 +102,11 @@ describe("ActionMessage — transient retry (warning variant)", () => {
|
||||
expect(container.firstChild).toBeNull();
|
||||
});
|
||||
|
||||
it("hides while the session is STARTING so the startup status remains visible", () => {
|
||||
const { container } = renderAction(retryMessage(), "STARTING");
|
||||
expect(container.firstChild).toBeNull();
|
||||
});
|
||||
|
||||
it("renders the red variant for a non-warning recovery banner", () => {
|
||||
const errorMsg = retryMessage({
|
||||
content: "Agent encountered an error",
|
||||
@@ -112,3 +124,77 @@ describe("ActionMessage — transient retry (warning variant)", () => {
|
||||
expect(text.className).not.toContain("text-amber-600");
|
||||
});
|
||||
});
|
||||
|
||||
describe("ActionMessage — missing PR branch", () => {
|
||||
it("renders a plain-language recovery panel with collapsed technical details", () => {
|
||||
renderAction(
|
||||
retryMessage({
|
||||
content:
|
||||
'The remote PR branch "codex/enhance-prompt-result-delivery" no longer exists (likely merged and deleted).',
|
||||
metadata: {
|
||||
variant: "warning",
|
||||
failure_kind: "missing_pr_branch",
|
||||
missing_branch: "codex/enhance-prompt-result-delivery",
|
||||
error_output: "fatal: unable to access github.com: Could not resolve host",
|
||||
actions: [
|
||||
{
|
||||
type: "archive_task",
|
||||
label: "Archive task",
|
||||
icon: "archive",
|
||||
test_id: "missing-branch-archive-button",
|
||||
},
|
||||
{
|
||||
type: "delete_task",
|
||||
label: "Delete task",
|
||||
icon: "trash",
|
||||
variant: "destructive",
|
||||
test_id: "missing-branch-delete-button",
|
||||
},
|
||||
],
|
||||
},
|
||||
} as Partial<Message>),
|
||||
"FAILED",
|
||||
);
|
||||
|
||||
expect(screen.getByTestId("missing-branch-recovery")).toBeTruthy();
|
||||
expect(screen.getByText("Branch is no longer available")).toBeTruthy();
|
||||
expect(screen.getByText("codex/enhance-prompt-result-delivery")).toBeTruthy();
|
||||
const technicalDetails = screen.getByText(TECHNICAL_DETAILS).closest("details");
|
||||
expect(technicalDetails?.open).toBe(false);
|
||||
expect(screen.getByTestId("missing-branch-archive-button").className).toContain("min-h-11");
|
||||
expect(screen.getByTestId("missing-branch-delete-button").className).toContain("min-h-11");
|
||||
|
||||
fireEvent.click(screen.getByText(TECHNICAL_DETAILS));
|
||||
expect(technicalDetails?.open).toBe(true);
|
||||
expect(screen.getByText(/Could not resolve host/)).toBeTruthy();
|
||||
});
|
||||
|
||||
it("uses the current session error as collapsed technical details", () => {
|
||||
renderAction(
|
||||
retryMessage({
|
||||
content: 'The remote PR branch "feature/missing" no longer exists.',
|
||||
metadata: {
|
||||
variant: "warning",
|
||||
failure_kind: "missing_pr_branch",
|
||||
missing_branch: "feature/missing",
|
||||
actions: [
|
||||
{
|
||||
type: "archive_task",
|
||||
label: "Archive task",
|
||||
test_id: "missing-branch-archive-button",
|
||||
},
|
||||
],
|
||||
},
|
||||
} as Partial<Message>),
|
||||
"FAILED",
|
||||
"environment preparation failed: fatal: could not resolve host github.com",
|
||||
);
|
||||
|
||||
const details = screen.getByText(TECHNICAL_DETAILS).closest("details");
|
||||
expect(details?.open).toBe(false);
|
||||
expect(screen.getByText(/could not resolve host github.com/)).toBeTruthy();
|
||||
|
||||
fireEvent.click(screen.getByText(TECHNICAL_DETAILS));
|
||||
expect(details?.open).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -10,6 +10,7 @@ import {
|
||||
IconSparkles,
|
||||
IconGitCommit,
|
||||
IconX,
|
||||
IconChevronDown,
|
||||
} from "@tabler/icons-react";
|
||||
import { cn } from "@/lib/utils";
|
||||
import { Button } from "@kandev/ui/button";
|
||||
@@ -41,6 +42,8 @@ type ActionMeta = {
|
||||
is_auth_error?: boolean;
|
||||
auth_methods?: RecoveryAuthMethod[];
|
||||
error_output?: string;
|
||||
failure_kind?: string;
|
||||
missing_branch?: string;
|
||||
};
|
||||
|
||||
function isSessionActive(state?: TaskSessionState) {
|
||||
@@ -56,6 +59,11 @@ export const ActionMessage = memo(function ActionMessage({ comment }: { comment:
|
||||
? (state.taskSessions.items[comment.session_id]?.state ?? undefined)
|
||||
: undefined,
|
||||
);
|
||||
const sessionError = useAppStore((state) =>
|
||||
comment.session_id
|
||||
? (state.taskSessions.items[comment.session_id]?.error_message as string | undefined)
|
||||
: undefined,
|
||||
);
|
||||
const metadata = comment.metadata as ActionMeta | undefined;
|
||||
const isWarning = metadata?.variant === "warning";
|
||||
const message = comment.content || "An error occurred";
|
||||
@@ -63,6 +71,17 @@ export const ActionMessage = memo(function ActionMessage({ comment }: { comment:
|
||||
// Hide once session is active again (recovery succeeded)
|
||||
if (isSessionActive(sessionState)) return null;
|
||||
|
||||
if (metadata?.failure_kind === "missing_pr_branch") {
|
||||
return (
|
||||
<MissingBranchRecovery
|
||||
metadata={metadata}
|
||||
taskId={comment.task_id}
|
||||
fallbackMessage={message}
|
||||
technicalDetails={sessionError}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
const iconClass = isWarning ? "text-amber-500" : "text-red-500";
|
||||
const textClass = isWarning
|
||||
? "text-amber-600 dark:text-amber-400"
|
||||
@@ -75,7 +94,7 @@ export const ActionMessage = memo(function ActionMessage({ comment }: { comment:
|
||||
<IconAlertTriangle className={cn("h-4 w-4", iconClass)} />
|
||||
</div>
|
||||
<div className="flex-1 min-w-0 pt-0.5">
|
||||
<div className={cn("text-xs font-mono", textClass)}>{message}</div>
|
||||
<div className={cn("text-xs break-words", textClass)}>{message}</div>
|
||||
<ActionMessageDetails metadata={metadata} />
|
||||
{metadata?.actions && metadata.actions.length > 0 && (
|
||||
<ActionButtons actions={metadata.actions} taskId={comment.task_id} />
|
||||
@@ -86,7 +105,73 @@ export const ActionMessage = memo(function ActionMessage({ comment }: { comment:
|
||||
);
|
||||
});
|
||||
|
||||
function ActionMessageDetails({ metadata }: { metadata: ActionMeta | undefined }) {
|
||||
function MissingBranchRecovery({
|
||||
metadata,
|
||||
taskId,
|
||||
fallbackMessage,
|
||||
technicalDetails,
|
||||
}: {
|
||||
metadata: ActionMeta;
|
||||
taskId?: string;
|
||||
fallbackMessage: string;
|
||||
technicalDetails?: string;
|
||||
}) {
|
||||
const branch = metadata.missing_branch?.trim();
|
||||
return (
|
||||
<section
|
||||
data-testid="missing-branch-recovery"
|
||||
role="alert"
|
||||
className="w-full min-w-0 rounded-md border border-amber-500/25 bg-amber-500/[0.06] p-3 sm:p-4"
|
||||
>
|
||||
<div className="flex min-w-0 items-start gap-3">
|
||||
<IconAlertTriangle
|
||||
className="mt-0.5 h-4 w-4 flex-shrink-0 text-amber-500"
|
||||
aria-hidden="true"
|
||||
/>
|
||||
<div className="min-w-0 flex-1">
|
||||
<h3 className="text-sm font-medium text-foreground">Branch is no longer available</h3>
|
||||
<p className="mt-1 text-xs leading-relaxed text-muted-foreground">
|
||||
{branch ? (
|
||||
<>
|
||||
This task points to <code className="break-all text-foreground">{branch}</code>, but
|
||||
that branch could not be found on the remote repository. It may have been merged or
|
||||
deleted.
|
||||
</>
|
||||
) : (
|
||||
fallbackMessage
|
||||
)}
|
||||
</p>
|
||||
<ActionMessageDetails metadata={metadata} technicalDetails={technicalDetails} />
|
||||
{metadata.actions && metadata.actions.length > 0 && (
|
||||
<ActionButtons actions={metadata.actions} taskId={taskId} />
|
||||
)}
|
||||
</div>
|
||||
</div>
|
||||
</section>
|
||||
);
|
||||
}
|
||||
|
||||
function TechnicalDetails({ children }: { children: string }) {
|
||||
return (
|
||||
<details className="mt-2 min-w-0 text-xs text-muted-foreground">
|
||||
<summary className="flex min-h-11 cursor-pointer list-none items-center gap-1.5 sm:min-h-8">
|
||||
<IconChevronDown className="h-3.5 w-3.5" />
|
||||
Technical details
|
||||
</summary>
|
||||
<pre className="max-h-[300px] max-w-full overflow-y-auto whitespace-pre-wrap break-words rounded bg-muted/50 p-2 font-mono text-[11px]">
|
||||
{children}
|
||||
</pre>
|
||||
</details>
|
||||
);
|
||||
}
|
||||
|
||||
function ActionMessageDetails({
|
||||
metadata,
|
||||
technicalDetails,
|
||||
}: {
|
||||
metadata: ActionMeta | undefined;
|
||||
technicalDetails?: string;
|
||||
}) {
|
||||
const [hostShellOpen, setHostShellOpen] = useState(false);
|
||||
const [hostShellCommand, setHostShellCommand] = useState<string | undefined>(undefined);
|
||||
|
||||
@@ -106,13 +191,10 @@ function ActionMessageDetails({ metadata }: { metadata: ActionMeta | undefined }
|
||||
}, []);
|
||||
|
||||
if (!metadata) return null;
|
||||
const errorOutput = metadata.error_output || technicalDetails;
|
||||
return (
|
||||
<>
|
||||
{metadata.error_output && (
|
||||
<pre className="mt-1.5 text-[11px] font-mono text-muted-foreground bg-muted/50 rounded p-2 overflow-auto max-h-[300px] whitespace-pre-wrap break-words">
|
||||
{metadata.error_output}
|
||||
</pre>
|
||||
)}
|
||||
{errorOutput && <TechnicalDetails>{errorOutput}</TechnicalDetails>}
|
||||
{metadata.is_auth_error && metadata.auth_methods && metadata.auth_methods.length > 0 && (
|
||||
<AuthMethodsPanel
|
||||
methods={metadata.auth_methods}
|
||||
@@ -133,7 +215,7 @@ function ActionMessageDetails({ metadata }: { metadata: ActionMeta | undefined }
|
||||
|
||||
function ActionButtons({ actions, taskId }: { actions: MessageAction[]; taskId?: string }) {
|
||||
return (
|
||||
<div className="mt-2 flex items-center gap-2">
|
||||
<div className="mt-2 flex flex-col gap-2 sm:flex-row sm:flex-wrap sm:items-center">
|
||||
{actions.map((action, i) => (
|
||||
<ActionButton key={action.test_id ?? i} action={action} messageTaskId={taskId} />
|
||||
))}
|
||||
@@ -206,7 +288,7 @@ function ActionButton({
|
||||
variant="outline"
|
||||
size="sm"
|
||||
className={cn(
|
||||
"h-7 text-xs cursor-pointer gap-1.5",
|
||||
"h-auto min-h-11 w-full gap-1.5 text-xs cursor-pointer sm:min-h-8 sm:w-auto",
|
||||
isDestructive && "text-destructive hover:text-destructive",
|
||||
)}
|
||||
disabled={disabled}
|
||||
|
||||
@@ -140,9 +140,8 @@ function AgentErrorStatus({
|
||||
? (state.taskSessions.items[sessionId]?.error_message as string | undefined)
|
||||
: undefined,
|
||||
);
|
||||
const [userCollapsed, setUserCollapsed] = useState(false);
|
||||
const expanded = !!errorMessage && !userCollapsed;
|
||||
const toggle = useCallback(() => setUserCollapsed((v) => !v), []);
|
||||
const [expanded, setExpanded] = useState(false);
|
||||
const toggle = useCallback(() => setExpanded((value) => !value), []);
|
||||
|
||||
const displayLabel = resolveAgentErrorLabel(errorMessage, config.label);
|
||||
const hasDetails = !!errorMessage;
|
||||
@@ -155,12 +154,16 @@ function AgentErrorStatus({
|
||||
>
|
||||
<button
|
||||
type="button"
|
||||
className={`flex w-full items-center gap-2 px-3 py-2 ${hasDetails ? "cursor-pointer" : ""}`}
|
||||
className={`flex min-h-11 w-full items-center gap-2 px-3 py-2 text-left sm:min-h-0 ${hasDetails ? "cursor-pointer" : ""}`}
|
||||
onClick={hasDetails ? toggle : undefined}
|
||||
disabled={!hasDetails}
|
||||
aria-expanded={hasDetails ? expanded : undefined}
|
||||
aria-label={
|
||||
hasDetails ? `${expanded ? "Hide" : "Show"} error details: ${displayLabel}` : displayLabel
|
||||
}
|
||||
>
|
||||
<IconAlertCircle className="h-3.5 w-3.5 flex-shrink-0" aria-hidden="true" />
|
||||
<span className="font-medium">{displayLabel}</span>
|
||||
<span className="min-w-0 break-words font-medium">{displayLabel}</span>
|
||||
{hasDetails && (
|
||||
<IconChevronDown
|
||||
className={`ml-auto h-3.5 w-3.5 transition-transform ${expanded ? "rotate-180" : ""}`}
|
||||
@@ -168,7 +171,7 @@ function AgentErrorStatus({
|
||||
)}
|
||||
</button>
|
||||
{expanded && errorMessage && (
|
||||
<pre className="px-3 pb-2 whitespace-pre-wrap break-words text-[11px] text-destructive/80 max-h-40 overflow-y-auto">
|
||||
<pre className="max-h-40 max-w-full overflow-y-auto whitespace-pre-wrap break-words px-3 pb-2 text-[11px] text-destructive/80">
|
||||
{errorMessage}
|
||||
</pre>
|
||||
)}
|
||||
|
||||
@@ -0,0 +1,27 @@
|
||||
import { cleanup, render, screen } from "@testing-library/react";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { renderSessionOrLoadState } from "./file-browser-load-state";
|
||||
|
||||
afterEach(cleanup);
|
||||
|
||||
describe("renderSessionOrLoadState", () => {
|
||||
it("uses the compact workspace failure state for failed sessions", () => {
|
||||
const result = renderSessionOrLoadState({
|
||||
isSessionFailed: true,
|
||||
sessionError: "raw environment preparation failure",
|
||||
loadState: "idle",
|
||||
isLoadingTree: false,
|
||||
tree: null,
|
||||
loadError: null,
|
||||
onRetry: () => {},
|
||||
});
|
||||
|
||||
render(<>{result}</>);
|
||||
|
||||
expect(screen.getByTestId("workspace-unavailable")).toBeTruthy();
|
||||
expect(screen.getByText("Workspace unavailable")).toBeTruthy();
|
||||
expect(screen.getByText("Technical details")).toBeTruthy();
|
||||
expect(screen.getByText("raw environment preparation failure")).toBeTruthy();
|
||||
expect(screen.queryByText("Session failed")).toBeNull();
|
||||
});
|
||||
});
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
import { PanelLoadingState } from "@/components/panel-loading-state";
|
||||
import type { FileTreeNode } from "@/lib/types/backend";
|
||||
import { WorkspaceUnavailable } from "./workspace-unavailable";
|
||||
|
||||
type RenderSessionOrLoadStateInput = {
|
||||
isSessionFailed: boolean;
|
||||
@@ -23,12 +24,7 @@ export function renderSessionOrLoadState({
|
||||
onRetry,
|
||||
}: RenderSessionOrLoadStateInput) {
|
||||
if (isSessionFailed) {
|
||||
return (
|
||||
<div className="p-4 text-sm text-destructive/80 space-y-2">
|
||||
<div>Session failed</div>
|
||||
{sessionError && <div className="text-xs text-muted-foreground">{sessionError}</div>}
|
||||
</div>
|
||||
);
|
||||
return <WorkspaceUnavailable error={sessionError} />;
|
||||
}
|
||||
if ((loadState === "loading" || isLoadingTree) && !tree) {
|
||||
return <PanelLoadingState label="Loading files..." />;
|
||||
|
||||
@@ -20,6 +20,7 @@ import { TerminalSearchBar } from "./terminal-search-bar";
|
||||
import { usePanelSearch } from "@/hooks/use-panel-search";
|
||||
import { suppressIOSKeyboardAssists } from "@/lib/terminal/suppress-ios-keyboard-assists";
|
||||
import { sendShellInput } from "@/lib/terminal/send-shell-input";
|
||||
import { WorkspaceUnavailable } from "./workspace-unavailable";
|
||||
|
||||
type ShellTerminalProps = {
|
||||
sessionId?: string;
|
||||
@@ -498,12 +499,7 @@ export function ShellTerminal({
|
||||
);
|
||||
}
|
||||
if (isSessionFailed) {
|
||||
return (
|
||||
<div className="h-full p-4 w-full bg-transparent flex flex-col gap-2">
|
||||
<div className="text-sm text-destructive/80">Session failed</div>
|
||||
{errorMessage && <div className="text-xs text-muted-foreground">{errorMessage}</div>}
|
||||
</div>
|
||||
);
|
||||
return <WorkspaceUnavailable error={errorMessage} />;
|
||||
}
|
||||
return (
|
||||
<div
|
||||
|
||||
@@ -0,0 +1,25 @@
|
||||
import { cleanup, fireEvent, render, screen } from "@testing-library/react";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { WorkspaceUnavailable } from "./workspace-unavailable";
|
||||
|
||||
afterEach(cleanup);
|
||||
|
||||
describe("WorkspaceUnavailable", () => {
|
||||
it("keeps the raw session error behind a collapsed disclosure", () => {
|
||||
render(
|
||||
<WorkspaceUnavailable error="fatal: unable to access github.com: Could not resolve host" />,
|
||||
);
|
||||
|
||||
expect(screen.getByText("Workspace unavailable")).toBeTruthy();
|
||||
expect(screen.queryByText("Session failed")).toBeNull();
|
||||
const details = screen.getByText("Technical details").closest("details");
|
||||
expect(details?.open).toBe(false);
|
||||
|
||||
fireEvent.click(screen.getByText("Technical details"));
|
||||
expect(details?.open).toBe(true);
|
||||
const error = screen.getByText(/Could not resolve host/);
|
||||
expect(error).toBeTruthy();
|
||||
expect(error.className).toContain("max-h-48");
|
||||
expect(error.className).toContain("overflow-y-auto");
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,42 @@
|
||||
"use client";
|
||||
|
||||
import { IconAlertCircle, IconChevronDown } from "@tabler/icons-react";
|
||||
|
||||
type WorkspaceUnavailableProps = {
|
||||
error?: string | null;
|
||||
};
|
||||
|
||||
export function WorkspaceUnavailable({ error }: WorkspaceUnavailableProps) {
|
||||
return (
|
||||
<div
|
||||
data-testid="workspace-unavailable"
|
||||
role="status"
|
||||
aria-label="Workspace unavailable"
|
||||
className="h-full w-full min-w-0 p-4"
|
||||
>
|
||||
<div className="flex min-w-0 items-start gap-2">
|
||||
<IconAlertCircle
|
||||
className="mt-0.5 h-4 w-4 flex-shrink-0 text-muted-foreground"
|
||||
aria-hidden="true"
|
||||
/>
|
||||
<div className="min-w-0 flex-1">
|
||||
<div className="text-sm font-medium text-foreground">Workspace unavailable</div>
|
||||
<p className="mt-1 text-xs leading-relaxed text-muted-foreground">
|
||||
This session did not finish setting up its workspace.
|
||||
</p>
|
||||
{error && (
|
||||
<details className="mt-2 min-w-0 text-xs text-muted-foreground">
|
||||
<summary className="flex min-h-11 cursor-pointer list-none items-center gap-1.5 sm:min-h-8">
|
||||
<IconChevronDown className="h-3.5 w-3.5" aria-hidden="true" />
|
||||
Technical details
|
||||
</summary>
|
||||
<pre className="max-h-48 max-w-full overflow-y-auto overscroll-contain whitespace-pre-wrap break-words rounded bg-muted/50 p-2 font-mono text-[11px]">
|
||||
{error}
|
||||
</pre>
|
||||
</details>
|
||||
)}
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
@@ -1,5 +1,5 @@
|
||||
import { type Page } from "@playwright/test";
|
||||
import { execSync } from "node:child_process";
|
||||
import { execFileSync, execSync } from "node:child_process";
|
||||
import fs from "node:fs";
|
||||
import path from "node:path";
|
||||
import { backendFixture, type BackendContext } from "./backend";
|
||||
@@ -14,6 +14,10 @@ export type SeedData = {
|
||||
startStepId: string;
|
||||
steps: WorkflowStep[];
|
||||
repositoryId: string;
|
||||
/** Local repository path used by the standard E2E fixture. */
|
||||
repositoryPath: string;
|
||||
/** Offline bare origin for tests that must exercise remote-ref failures. */
|
||||
repositoryRemoteURL: string;
|
||||
agentProfileId: string;
|
||||
/** Executor profile ID for the worktree executor — use to create tasks with git worktree isolation. */
|
||||
worktreeExecutorProfileId: string;
|
||||
@@ -91,9 +95,14 @@ export const test = backendFixture.extend<
|
||||
|
||||
// Create a minimal git repository inside backend.tmpDir (the backend's HOME).
|
||||
// This ensures discoveryRoots() allows the path for branch listing.
|
||||
// It also has an offline bare origin with main, so tests can distinguish
|
||||
// a genuinely missing remote ref from an unavailable Git host.
|
||||
const remoteDir = path.join(backend.tmpDir, "repos", "e2e-remote.git");
|
||||
const repoDir = path.join(backend.tmpDir, "repos", "e2e-repo");
|
||||
fs.mkdirSync(path.dirname(remoteDir), { recursive: true });
|
||||
fs.mkdirSync(repoDir, { recursive: true });
|
||||
const gitEnv = makeGitEnv(backend.tmpDir);
|
||||
execSync(`git init --bare -b main "${remoteDir}"`, { env: gitEnv });
|
||||
execSync("git init -b main", { cwd: repoDir, env: gitEnv });
|
||||
fs.writeFileSync(
|
||||
path.join(repoDir, "walkthrough_base.txt"),
|
||||
@@ -101,6 +110,8 @@ export const test = backendFixture.extend<
|
||||
);
|
||||
execSync("git add walkthrough_base.txt", { cwd: repoDir, env: gitEnv });
|
||||
execSync('git commit -m "init"', { cwd: repoDir, env: gitEnv });
|
||||
execSync(`git remote add origin "file://${remoteDir}"`, { cwd: repoDir, env: gitEnv });
|
||||
execSync("git push origin main", { cwd: repoDir, env: gitEnv });
|
||||
const repo = await apiClient.createRepository(workspace.id, repoDir);
|
||||
|
||||
// Agent registry seeding (runInitialAgentSetup → discovery) is
|
||||
@@ -144,6 +155,8 @@ export const test = backendFixture.extend<
|
||||
startStepId: startStep.id,
|
||||
steps: sorted,
|
||||
repositoryId: repo.id,
|
||||
repositoryPath: repoDir,
|
||||
repositoryRemoteURL: `file://${remoteDir}`,
|
||||
agentProfileId,
|
||||
worktreeExecutorProfileId,
|
||||
});
|
||||
@@ -240,6 +253,19 @@ export const test = backendFixture.extend<
|
||||
],
|
||||
});
|
||||
|
||||
/**
|
||||
* Restores the fixture's offline origin after a GitLab E2E cleanup removes it.
|
||||
* This lets focused tests distinguish a deleted remote ref from a transport error.
|
||||
*/
|
||||
export function restoreSeedRepositoryOrigin(seedData: SeedData) {
|
||||
const baseArgs = ["-C", seedData.repositoryPath, "remote"];
|
||||
try {
|
||||
execFileSync("git", [...baseArgs, "set-url", "origin", seedData.repositoryRemoteURL]);
|
||||
} catch {
|
||||
execFileSync("git", [...baseArgs, "add", "origin", seedData.repositoryRemoteURL]);
|
||||
}
|
||||
}
|
||||
|
||||
// Reset the active workspace pointer before every test so that specs which
|
||||
// do not use the testPage fixture (e.g. API-only routing tests) start from
|
||||
// a known workspace_id instead of whatever a previous test's completeOnboarding
|
||||
|
||||
@@ -114,6 +114,7 @@ type CreateTaskOpts = {
|
||||
workflow_id?: string;
|
||||
workflow_step_id?: string;
|
||||
agent_profile_id?: string;
|
||||
executor_profile_id?: string;
|
||||
repository_ids?: string[];
|
||||
repositories?: TaskRepositoryInput[];
|
||||
plan_mode?: boolean;
|
||||
@@ -128,6 +129,7 @@ type TaskRepositoryInput = {
|
||||
repository_id?: string;
|
||||
base_branch?: string;
|
||||
checkout_branch?: string;
|
||||
pr_number?: number;
|
||||
remote_url?: string;
|
||||
provider?: string;
|
||||
provider_repo_id?: string;
|
||||
@@ -156,6 +158,8 @@ function buildCreateTaskBody(
|
||||
};
|
||||
setIf(body, "workflow_id", options.workflow_id);
|
||||
setIf(body, "workflow_step_id", options.workflow_step_id);
|
||||
setIf(body, "agent_profile_id", options.agent_profile_id);
|
||||
setIf(body, "executor_profile_id", options.executor_profile_id);
|
||||
setIf(body, "metadata", buildTaskMetadata(options));
|
||||
setIf(
|
||||
body,
|
||||
@@ -344,9 +348,11 @@ export class ApiClient {
|
||||
workflow_step_id?: string;
|
||||
/** Stored in task.Metadata so auto_start_agent can pick it up on on_enter. */
|
||||
agent_profile_id?: string;
|
||||
/** Executor profile used when the task session is prepared. */
|
||||
executor_profile_id?: string;
|
||||
/** Repository IDs to associate with the task (required for agent execution). */
|
||||
repository_ids?: string[];
|
||||
/** Full repository entries with optional checkout_branch / base_branch. */
|
||||
/** Full repository entries with optional checkout_branch / base_branch / pr_number. */
|
||||
repositories?: TaskRepositoryInput[];
|
||||
/** When true, task is placed at position 0 regardless of is_start_step. */
|
||||
plan_mode?: boolean;
|
||||
@@ -496,7 +502,7 @@ export class ApiClient {
|
||||
workflow_id?: string;
|
||||
workflow_step_id?: string;
|
||||
repository_ids?: string[];
|
||||
/** Full repository entries with optional checkout_branch / base_branch. */
|
||||
/** Full repository entries with optional checkout_branch / base_branch / pr_number. */
|
||||
repositories?: TaskRepositoryInput[];
|
||||
executor_id?: string;
|
||||
executor_profile_id?: string;
|
||||
|
||||
@@ -256,12 +256,17 @@ test.describe("Git commit pre-hooks", () => {
|
||||
await dialogCommitBtn.click();
|
||||
|
||||
// The error message should appear in the chat
|
||||
await expect(session.gitOperationErrorMessage()).toBeVisible({ timeout: 30_000 });
|
||||
const errorMessage = session.gitOperationErrorMessage();
|
||||
await expect(errorMessage).toBeVisible({ timeout: 30_000 });
|
||||
await expect(errorMessage.getByText("Git commit failed", { exact: true })).toBeVisible();
|
||||
await expect(session.gitFixButton()).toBeVisible();
|
||||
|
||||
// Verify the error output contains the pre-commit hook message
|
||||
await expect(
|
||||
session.gitOperationErrorMessage().getByText("lint check failed: missing semicolons"),
|
||||
).toBeVisible({ timeout: 15_000 });
|
||||
// The concise summary and Fix action remain available while command
|
||||
// output stays collapsed until the user asks to inspect it.
|
||||
const hookOutput = errorMessage.getByText("lint check failed: missing semicolons");
|
||||
await expect(hookOutput).toBeHidden();
|
||||
await errorMessage.locator("summary", { hasText: "Technical details" }).click();
|
||||
await expect(hookOutput).toBeVisible({ timeout: 15_000 });
|
||||
|
||||
// Click the Fix button
|
||||
await session.gitFixButton().click();
|
||||
|
||||
@@ -0,0 +1,113 @@
|
||||
// Filename starts with "mobile-" so this runs on the mobile-chrome project.
|
||||
import { restoreSeedRepositoryOrigin, test, expect } from "../../fixtures/test-base";
|
||||
import { assertNoDocumentHorizontalOverflow } from "../../helpers/layout-assertions";
|
||||
import { SessionPage } from "../../pages/session-page";
|
||||
|
||||
test.describe("mobile PR watcher missing branch", () => {
|
||||
test("keeps the recovery summary and task actions reachable", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
}, testInfo) => {
|
||||
test.setTimeout(90_000);
|
||||
|
||||
restoreSeedRepositoryOrigin(seedData);
|
||||
|
||||
await apiClient.mockGitHubReset();
|
||||
await apiClient.mockGitHubSetUser("test-user");
|
||||
|
||||
const prBranch = "feature/already-merged-and-deleted-mobile";
|
||||
await apiClient.mockGitHubAddPRs([
|
||||
{
|
||||
number: 1000,
|
||||
title: "Already merged mobile feature",
|
||||
state: "closed",
|
||||
head_branch: prBranch,
|
||||
base_branch: "main",
|
||||
author_login: "test-user",
|
||||
repo_owner: "testorg",
|
||||
repo_name: "testrepo",
|
||||
},
|
||||
]);
|
||||
|
||||
const task = await apiClient.createTaskWithAgent(
|
||||
seedData.workspaceId,
|
||||
"PR #1000: Already merged mobile feature",
|
||||
seedData.agentProfileId,
|
||||
{
|
||||
workflow_id: seedData.workflowId,
|
||||
workflow_step_id: seedData.startStepId,
|
||||
executor_profile_id: seedData.worktreeExecutorProfileId,
|
||||
repositories: [
|
||||
{
|
||||
repository_id: seedData.repositoryId,
|
||||
base_branch: "main",
|
||||
checkout_branch: prBranch,
|
||||
pr_number: 1000,
|
||||
},
|
||||
],
|
||||
metadata: {
|
||||
pr_number: 1000,
|
||||
pr_branch: prBranch,
|
||||
pr_repo: "testorg/testrepo",
|
||||
pr_author: "test-user",
|
||||
},
|
||||
},
|
||||
);
|
||||
|
||||
await apiClient.mockGitHubAssociateTaskPR({
|
||||
task_id: task.id,
|
||||
owner: "testorg",
|
||||
repo: "testrepo",
|
||||
pr_number: 1000,
|
||||
pr_url: "https://github.com/testorg/testrepo/pull/1000",
|
||||
pr_title: "Already merged mobile feature",
|
||||
head_branch: prBranch,
|
||||
base_branch: "main",
|
||||
author_login: "test-user",
|
||||
state: "closed",
|
||||
});
|
||||
|
||||
await testPage.goto(`/t/${task.id}`);
|
||||
const session = new SessionPage(testPage);
|
||||
const chat = session.activeChat();
|
||||
const recovery = chat.getByTestId("missing-branch-recovery");
|
||||
|
||||
await expect(recovery).toHaveCount(1, { timeout: 30_000 });
|
||||
await expect(
|
||||
recovery.getByRole("heading", { name: "Branch is no longer available" }),
|
||||
).toBeVisible();
|
||||
await expect(recovery).toContainText(prBranch);
|
||||
await expect(
|
||||
chat.getByRole("status", { name: /Session failed|Environment setup failed/i }),
|
||||
).toHaveCount(0);
|
||||
|
||||
const archiveButton = recovery.getByTestId("missing-branch-archive-button");
|
||||
const deleteButton = recovery.getByTestId("missing-branch-delete-button");
|
||||
for (const button of [archiveButton, deleteButton]) {
|
||||
await expect(button).toBeVisible();
|
||||
await expect(button).toBeInViewport();
|
||||
const box = await button.boundingBox();
|
||||
expect(box).not.toBeNull();
|
||||
expect(box!.height).toBeGreaterThanOrEqual(44);
|
||||
}
|
||||
|
||||
const technicalDetails = recovery.locator("details");
|
||||
const technicalOutput = technicalDetails.locator("pre");
|
||||
await expect(technicalDetails).not.toHaveAttribute("open");
|
||||
await expect(technicalOutput).not.toBeVisible();
|
||||
const disclosure = recovery.getByText("Technical details", { exact: true });
|
||||
await disclosure.tap();
|
||||
await expect(technicalOutput).toBeVisible();
|
||||
await expect(technicalOutput).toContainText("couldn't find remote ref pull/1000/head");
|
||||
const disclosureBox = await disclosure.boundingBox();
|
||||
expect(disclosureBox).not.toBeNull();
|
||||
expect(disclosureBox!.height).toBeGreaterThanOrEqual(44);
|
||||
|
||||
await assertNoDocumentHorizontalOverflow(testPage, "missing branch recovery");
|
||||
await testPage.screenshot({
|
||||
path: testInfo.outputPath("missing-pr-branch-mobile.png"),
|
||||
fullPage: true,
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -1,4 +1,4 @@
|
||||
import { test, expect } from "../../fixtures/test-base";
|
||||
import { restoreSeedRepositoryOrigin, test, expect } from "../../fixtures/test-base";
|
||||
import { SessionPage } from "../../pages/session-page";
|
||||
|
||||
test.describe("PR watcher missing branch", () => {
|
||||
@@ -15,13 +15,15 @@ test.describe("PR watcher missing branch", () => {
|
||||
* - Environment preparation fails because the branch doesn't exist
|
||||
* - Assert the guidance message appears in chat
|
||||
*/
|
||||
test("shows guidance message when PR branch is deleted", async ({
|
||||
test("shows one focused recovery panel when PR branch is deleted", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
}) => {
|
||||
}, testInfo) => {
|
||||
test.setTimeout(90_000);
|
||||
|
||||
restoreSeedRepositoryOrigin(seedData);
|
||||
|
||||
// --- Setup mock GitHub ---
|
||||
await apiClient.mockGitHubReset();
|
||||
await apiClient.mockGitHubSetUser("test-user");
|
||||
@@ -44,21 +46,23 @@ test.describe("PR watcher missing branch", () => {
|
||||
|
||||
// Create a task as the PR watcher would: with a checkout_branch that
|
||||
// no longer exists on remote (PR was merged, branch deleted).
|
||||
const task = await apiClient.createTask(
|
||||
const task = await apiClient.createTaskWithAgent(
|
||||
seedData.workspaceId,
|
||||
"PR #999: Already merged feature",
|
||||
seedData.agentProfileId,
|
||||
{
|
||||
workflow_id: seedData.workflowId,
|
||||
workflow_step_id: seedData.startStepId,
|
||||
executor_profile_id: seedData.worktreeExecutorProfileId,
|
||||
repositories: [
|
||||
{
|
||||
repository_id: seedData.repositoryId,
|
||||
base_branch: "main",
|
||||
checkout_branch: prBranch,
|
||||
pr_number: 999,
|
||||
},
|
||||
],
|
||||
metadata: {
|
||||
agent_profile_id: seedData.agentProfileId,
|
||||
pr_number: 999,
|
||||
pr_branch: prBranch,
|
||||
pr_repo: "testorg/testrepo",
|
||||
@@ -87,20 +91,44 @@ test.describe("PR watcher missing branch", () => {
|
||||
|
||||
const session = new SessionPage(testPage);
|
||||
|
||||
// --- Assert the guidance message appears in chat ---
|
||||
// The MissingBranchMessage component renders with archive/delete action buttons.
|
||||
await expect(session.chat.getByText("no longer exists", { exact: false })).toBeVisible({
|
||||
timeout: 30_000,
|
||||
});
|
||||
// --- Assert the primary recovery message appears once in chat ---
|
||||
const chat = session.activeChat();
|
||||
const recovery = chat.getByTestId("missing-branch-recovery");
|
||||
await expect(recovery).toHaveCount(1, { timeout: 30_000 });
|
||||
await expect(
|
||||
recovery.getByRole("heading", { name: "Branch is no longer available" }),
|
||||
).toBeVisible();
|
||||
await expect(recovery).toContainText(prBranch);
|
||||
await expect(recovery).toContainText("merged or deleted");
|
||||
|
||||
// The actionable recovery panel replaces the generic failed-agent banner.
|
||||
await expect(
|
||||
chat.getByRole("status", { name: /Session failed|Environment setup failed/i }),
|
||||
).toHaveCount(0);
|
||||
|
||||
// Diagnostics stay secondary until the user explicitly expands them.
|
||||
const technicalDetails = recovery.locator("details");
|
||||
const technicalOutput = technicalDetails.locator("pre");
|
||||
await expect(technicalDetails).not.toHaveAttribute("open");
|
||||
await expect(technicalOutput).not.toBeVisible();
|
||||
await recovery.getByText("Technical details", { exact: true }).click();
|
||||
await expect(technicalDetails).toHaveAttribute("open", "");
|
||||
await expect(technicalOutput).toBeVisible();
|
||||
await expect(technicalOutput).toContainText("couldn't find remote ref pull/999/head");
|
||||
|
||||
// Verify the action buttons are present
|
||||
await expect(session.chat.getByTestId("missing-branch-archive-button")).toBeVisible({
|
||||
await expect(recovery.getByTestId("missing-branch-archive-button")).toBeVisible({
|
||||
timeout: 5_000,
|
||||
});
|
||||
await expect(session.chat.getByTestId("missing-branch-delete-button")).toBeVisible({
|
||||
await expect(recovery.getByTestId("missing-branch-delete-button")).toBeVisible({
|
||||
timeout: 5_000,
|
||||
});
|
||||
|
||||
await testPage.screenshot({
|
||||
path: testInfo.outputPath("missing-pr-branch-desktop.png"),
|
||||
fullPage: true,
|
||||
});
|
||||
|
||||
// Verify the session state via API — should be FAILED
|
||||
const { sessions } = await apiClient.listTaskSessions(task.id);
|
||||
const failedSession = sessions.find((s) => s.state === "FAILED");
|
||||
|
||||
@@ -104,7 +104,7 @@ test.describe("Setup script progress UX", () => {
|
||||
}
|
||||
});
|
||||
|
||||
test("stays expanded on setup script failure with output visible", async ({
|
||||
test("keeps setup script failure details collapsed until requested", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
@@ -150,21 +150,24 @@ test.describe("Setup script progress UX", () => {
|
||||
|
||||
// Setup script failures are non-fatal (agent still starts), so the panel
|
||||
// transitions preparing → completed_with_error rather than the fatal
|
||||
// `failed` state — and stays expanded so the error is readable.
|
||||
// `failed` state. The concise error state is visible immediately while
|
||||
// command output remains collapsed until the user requests it.
|
||||
await expect(panel).toBeVisible({ timeout: 15_000 });
|
||||
await expect(panel).toHaveAttribute("data-status", "completed_with_error", {
|
||||
timeout: 30_000,
|
||||
});
|
||||
await expect(panel).toHaveAttribute("data-expanded", "true");
|
||||
|
||||
// Both stdout and stderr from the failing script must remain visible so
|
||||
// the user can diagnose — this is the core fix of this change.
|
||||
await expect(panel).toContainText("[setup] starting install");
|
||||
await expect(panel).toContainText("ENOENT: package-lock.json missing");
|
||||
|
||||
// User can click the card header to collapse it.
|
||||
await panel.getByTestId("prepare-progress-toggle").click();
|
||||
await expect(panel).toHaveAttribute("data-expanded", "false");
|
||||
|
||||
const output = panel.locator("pre").filter({ hasText: /^\[setup\] starting install/ });
|
||||
await expect(panel.getByRole("button", { name: "Show preparation details" })).toBeVisible();
|
||||
await expect(output).toBeHidden();
|
||||
|
||||
// Both streams remain available after the user opens the disclosure.
|
||||
await panel.getByRole("button", { name: "Show preparation details" }).click();
|
||||
await expect(panel).toHaveAttribute("data-expanded", "true");
|
||||
await expect(output).toBeVisible();
|
||||
await expect(output).toContainText("[setup] starting install");
|
||||
await expect(output).toContainText("ENOENT: package-lock.json missing");
|
||||
} finally {
|
||||
await apiClient.deleteExecutorProfile(profile.id).catch(() => {});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user