diff --git a/cli/internal/cli/error_envelope.go b/cli/internal/cli/error_envelope.go index ebfc75ace..7709ac5be 100644 --- a/cli/internal/cli/error_envelope.go +++ b/cli/internal/cli/error_envelope.go @@ -15,6 +15,8 @@ const ( errorCodeUnknownCommand = "UNKNOWN_COMMAND" errorCodeProjectNotFound = "PROJECT_NOT_FOUND" errorCodeUnityNotReachable = "UNITY_NOT_REACHABLE" + errorCodeUnityStartupTimeout = "UNITY_STARTUP_TIMEOUT" + errorCodeUnityProcessExitTimeout = "UNITY_PROCESS_EXIT_TIMEOUT" errorCodeUnityDisconnectedAfterDispatch = "UNITY_DISCONNECTED_AFTER_DISPATCH" errorCodeUnityDisconnectedAfterAccept = "UNITY_DISCONNECTED_AFTER_ACCEPT" errorCodeUnityResponseTimeoutAfterAccept = "UNITY_RESPONSE_TIMEOUT_AFTER_ACCEPT" @@ -145,6 +147,16 @@ func classifyError(err error, context errorContext) cliError { return argumentErr.toCLIError(context) } + var startupTimeoutErr launchStartupTimeoutError + if errors.As(err, &startupTimeoutErr) { + return unityStartupTimeoutCLIError(startupTimeoutErr, context) + } + + var processExitTimeoutErr launchProcessExitTimeoutError + if errors.As(err, &processExitTimeoutErr) { + return unityProcessExitTimeoutCLIError(processExitTimeoutErr, context) + } + var notRespondingErr unityServerNotRespondingError if errors.As(err, ¬RespondingErr) { return cliError{ diff --git a/cli/internal/cli/error_envelope_test.go b/cli/internal/cli/error_envelope_test.go index e40b23865..25ed98185 100644 --- a/cli/internal/cli/error_envelope_test.go +++ b/cli/internal/cli/error_envelope_test.go @@ -140,6 +140,55 @@ func TestClassifyUnityServerNotRespondingError(t *testing.T) { } } +func TestClassifyLaunchStartupTimeoutError(t *testing.T) { + // Verifies launch startup timeouts do not look like generic reachability or package failures. + cliErr := classifyError( + launchStartupTimeoutError{ + projectRoot: "/tmp/MyProject", + cause: errors.New("timed out waiting for Unity tool readiness"), + }, + errorContext{projectRoot: "/tmp/MyProject", command: launchCommandName}, + ) + + if cliErr.ErrorCode != errorCodeUnityStartupTimeout { + t.Fatalf("error code mismatch: %#v", cliErr) + } + if cliErr.Message != "Unity is running, but the Editor did not finish startup before the launch timeout." { + t.Fatalf("message mismatch: %#v", cliErr) + } + for _, action := range cliErr.NextActions { + lowerAction := strings.ToLower(action) + if strings.Contains(lowerAction, "package") || strings.Contains(lowerAction, "uloop launch") { + t.Fatalf("next action should avoid package guesses and launch retry guidance: %#v", cliErr.NextActions) + } + } + if cliErr.Details["timeoutSeconds"] != 600 { + t.Fatalf("timeout details mismatch: %#v", cliErr.Details) + } +} + +func TestClassifyLaunchProcessExitTimeoutError(t *testing.T) { + // Verifies restart and quit process-exit timeouts are structured as retryable launch failures. + cliErr := classifyError( + launchProcessExitTimeoutError{ + projectRoot: "/tmp/MyProject", + pid: 123, + timeout: launchProcessExitTimeout, + }, + errorContext{projectRoot: "/tmp/MyProject", command: launchCommandName}, + ) + + if cliErr.ErrorCode != errorCodeUnityProcessExitTimeout { + t.Fatalf("error code mismatch: %#v", cliErr) + } + if !cliErr.Retryable || !cliErr.SafeToRetry { + t.Fatalf("process exit timeout should be retryable: %#v", cliErr) + } + if cliErr.Details["pid"] != 123 { + t.Fatalf("pid details mismatch: %#v", cliErr.Details) + } +} + func TestWriteToolFailureWhenServerStopsBeforeAcceptingDispatchedRequestIsNotSafeToRetry(t *testing.T) { // Verifies pre-accept server silence does not advertise a dispatched state-changing command as safe to retry. var stderr bytes.Buffer diff --git a/cli/internal/cli/launch.go b/cli/internal/cli/launch.go index 3690e277c..95fe69ef7 100644 --- a/cli/internal/cli/launch.go +++ b/cli/internal/cli/launch.go @@ -17,13 +17,16 @@ import ( ) const ( - launchCommandName = "launch" - launchLockfilePoll = 100 * time.Millisecond - launchLockfileTimeout = 5 * time.Second - projectVersionFilePath = "ProjectSettings/ProjectVersion.txt" - recoveryDirectoryPath = "Assets/_Recovery" - launchTempDirectoryName = "Temp" - unityLockfileName = "UnityLockfile" + launchCommandName = "launch" + launchLockfilePoll = 100 * time.Millisecond + launchLockfileTimeout = 5 * time.Second + launchProcessExitPoll = 100 * time.Millisecond + launchProcessExitTimeout = 20 * time.Second + launchReadinessTimeout = 10 * time.Minute + projectVersionFilePath = "ProjectSettings/ProjectVersion.txt" + recoveryDirectoryPath = "Assets/_Recovery" + launchTempDirectoryName = "Temp" + unityLockfileName = "UnityLockfile" ) var ( @@ -31,8 +34,9 @@ var ( focusUnityProcessForLaunch = focusUnityProcess killUnityProcessForLaunch = killUnityProcess resolveUnityExecutablePathForLaunch = resolveUnityExecutablePath - waitForUnityLockfileForLaunch = waitForUnityLockfile - waitForToolReadinessForLaunch = waitForToolReadiness + waitForUnityProcessExitForLaunch = waitForUnityProcessExit + waitForUnityStartupMarkerForLaunch = waitForUnityStartupMarkerOrTimeout + waitForToolReadinessForLaunch = waitForToolReadinessWithTimeout probeProjectIpcForLaunchFallback = probeToolReadinessSequence ) @@ -214,7 +218,7 @@ func runLaunch(ctx context.Context, options launchOptions, startPath string, std spinner := newLaunchSpinner(stdout, stderr) defer spinner.Stop() writeLaunchReadinessWait(stdout, spinner) - if err := waitForToolReadinessForLaunch(ctx, projectRoot); err != nil { + if err := waitForLaunchReadiness(ctx, projectRoot); err != nil { writeClassifiedError(stderr, err, errorContext{projectRoot: projectRoot, command: launchCommandName}) return 1 } @@ -225,6 +229,10 @@ func runLaunch(ctx context.Context, options launchOptions, startPath string, std writeClassifiedError(stderr, err, errorContext{projectRoot: projectRoot, command: launchCommandName}) return 1 } + if err := waitForUnityProcessExitForLaunch(ctx, projectRoot, runningProcess.pid, launchProcessExitPoll, launchProcessExitTimeout); err != nil { + writeClassifiedError(stderr, err, errorContext{projectRoot: projectRoot, command: launchCommandName}) + return 1 + } if options.quit { return writeLaunchQuitResponse(stdout, stderr, projectRoot, &runningProcess.pid, launchStoppedMessage) } @@ -276,12 +284,12 @@ func runLaunch(ctx context.Context, options launchOptions, startPath string, std writeClassifiedError(stderr, err, errorContext{projectRoot: projectRoot, command: launchCommandName}) return 1 } - if err := waitForUnityLockfileForLaunch(ctx, unityLockfilePath(projectRoot), launchLockfilePoll, launchLockfileTimeout); err != nil { + if err := waitForUnityStartupMarkerForLaunch(ctx, unityLockfilePath(projectRoot), launchLockfilePoll, launchLockfileTimeout); err != nil { writeClassifiedError(stderr, err, errorContext{projectRoot: projectRoot, command: launchCommandName}) return 1 } writeLaunchReadinessWait(stdout, spinner) - if err := waitForToolReadinessForLaunch(ctx, projectRoot); err != nil { + if err := waitForLaunchReadiness(ctx, projectRoot); err != nil { writeClassifiedError(stderr, err, errorContext{projectRoot: projectRoot, command: launchCommandName}) return 1 } @@ -312,7 +320,44 @@ func cleanStaleUnityTemp(projectRoot string) (bool, error) { return true, os.RemoveAll(filepath.Join(projectRoot, launchTempDirectoryName)) } -func waitForUnityLockfile(ctx context.Context, lockfilePath string, pollInterval time.Duration, timeout time.Duration) error { +func waitForUnityProcessExit(ctx context.Context, projectRoot string, pid int, pollInterval time.Duration, timeout time.Duration) error { + timeoutContext, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + timeoutError := func() error { + if ctx.Err() != nil { + return ctx.Err() + } + return launchProcessExitTimeoutError{ + projectRoot: projectRoot, + pid: pid, + timeout: timeout, + } + } + + ticker := time.NewTicker(pollInterval) + defer ticker.Stop() + + for { + runningProcess, err := findRunningUnityProcessForLaunch(timeoutContext, projectRoot) + if err != nil { + if timeoutContext.Err() != nil { + return timeoutError() + } + return err + } + if runningProcess == nil || runningProcess.pid != pid { + return nil + } + + select { + case <-timeoutContext.Done(): + return timeoutError() + case <-ticker.C: + } + } +} + +func waitForUnityStartupMarkerOrTimeout(ctx context.Context, lockfilePath string, pollInterval time.Duration, timeout time.Duration) error { timeoutContext, cancel := context.WithTimeout(ctx, timeout) defer cancel() diff --git a/cli/internal/cli/launch_process_exit_timeout_error.go b/cli/internal/cli/launch_process_exit_timeout_error.go new file mode 100644 index 000000000..7ffa9a005 --- /dev/null +++ b/cli/internal/cli/launch_process_exit_timeout_error.go @@ -0,0 +1,37 @@ +package cli + +import ( + "fmt" + "time" +) + +type launchProcessExitTimeoutError struct { + projectRoot string + pid int + timeout time.Duration +} + +func (err launchProcessExitTimeoutError) Error() string { + return fmt.Sprintf("Unity process %d did not exit within %s", err.pid, err.timeout) +} + +func unityProcessExitTimeoutCLIError(err launchProcessExitTimeoutError, context errorContext) cliError { + projectRoot := firstNonEmpty(context.projectRoot, err.projectRoot) + return cliError{ + ErrorCode: errorCodeUnityProcessExitTimeout, + Phase: errorPhaseExecution, + Message: fmt.Sprintf("Unity process %d did not exit before the launch timeout.", err.pid), + Retryable: true, + SafeToRetry: true, + ProjectRoot: projectRoot, + Command: context.command, + NextActions: []string{ + "Wait for Unity to finish exiting, then retry the launch command.", + "If Unity remains visible or keeps project files locked, close the Unity process from the OS and retry.", + }, + Details: map[string]any{ + "pid": err.pid, + "timeoutSeconds": int(err.timeout.Seconds()), + }, + } +} diff --git a/cli/internal/cli/launch_startup_timeout_error.go b/cli/internal/cli/launch_startup_timeout_error.go new file mode 100644 index 000000000..6c479f3e3 --- /dev/null +++ b/cli/internal/cli/launch_startup_timeout_error.go @@ -0,0 +1,66 @@ +package cli + +import ( + "context" + "errors" + "fmt" +) + +type launchStartupTimeoutError struct { + projectRoot string + cause error +} + +func (err launchStartupTimeoutError) Error() string { + if err.cause == nil { + return "Unity startup did not finish before the launch timeout" + } + return fmt.Sprintf("Unity startup did not finish before the launch timeout: %s", err.cause.Error()) +} + +func (err launchStartupTimeoutError) Unwrap() error { + return err.cause +} + +func waitForLaunchReadiness(ctx context.Context, projectRoot string) error { + err := waitForToolReadinessForLaunch(ctx, projectRoot, launchReadinessTimeout) + if err == nil { + return nil + } + if ctx.Err() != nil || isReadinessCLIUpdateRequiredError(err) { + return err + } + var notRespondingErr unityServerNotRespondingError + if !errors.As(err, ¬RespondingErr) { + return err + } + return launchStartupTimeoutError{ + projectRoot: projectRoot, + cause: err, + } +} + +func unityStartupTimeoutCLIError(err launchStartupTimeoutError, context errorContext) cliError { + projectRoot := firstNonEmpty(context.projectRoot, err.projectRoot) + details := map[string]any{ + "timeoutSeconds": int(launchReadinessTimeout.Seconds()), + } + if err.cause != nil { + details["cause"] = err.cause.Error() + } + return cliError{ + ErrorCode: errorCodeUnityStartupTimeout, + Phase: errorPhaseConnection, + Message: "Unity is running, but the Editor did not finish startup before the launch timeout.", + Retryable: true, + SafeToRetry: true, + ProjectRoot: projectRoot, + Command: context.command, + NextActions: []string{ + "Wait for Unity to finish importing assets, compiling scripts, or reloading the domain.", + "After the Editor becomes responsive, continue with the uloop command you wanted to run.", + "If Unity appears stuck, focus the Editor and check the Console or Editor log.", + }, + Details: details, + } +} diff --git a/cli/internal/cli/launch_test.go b/cli/internal/cli/launch_test.go index ea175c395..1593903ee 100644 --- a/cli/internal/cli/launch_test.go +++ b/cli/internal/cli/launch_test.go @@ -11,6 +11,8 @@ import ( "strings" "testing" "time" + + "github.com/hatayama/unity-cli-loop/cli/internal/unityipc" ) func TestParseLaunchOptionsSupportsCoreFlags(t *testing.T) { @@ -118,7 +120,7 @@ func TestRunLaunchWritesReadyResponseAfterToolReadiness(t *testing.T) { // Verifies launch reports an explicit ready payload after Unity accepts tool requests. originalFinder := findRunningUnityProcessForLaunch originalResolver := resolveUnityExecutablePathForLaunch - originalLockfileWait := waitForUnityLockfileForLaunch + originalStartupMarkerWait := waitForUnityStartupMarkerForLaunch originalReadinessWait := waitForToolReadinessForLaunch findRunningUnityProcessForLaunch = func(context.Context, string) (*unityProcess, error) { return nil, nil @@ -126,16 +128,16 @@ func TestRunLaunchWritesReadyResponseAfterToolReadiness(t *testing.T) { resolveUnityExecutablePathForLaunch = func(string) (string, error) { return "/usr/bin/true", nil } - waitForUnityLockfileForLaunch = func(context.Context, string, time.Duration, time.Duration) error { + waitForUnityStartupMarkerForLaunch = func(context.Context, string, time.Duration, time.Duration) error { return nil } - waitForToolReadinessForLaunch = func(context.Context, string) error { + waitForToolReadinessForLaunch = func(context.Context, string, time.Duration) error { return nil } t.Cleanup(func() { findRunningUnityProcessForLaunch = originalFinder resolveUnityExecutablePathForLaunch = originalResolver - waitForUnityLockfileForLaunch = originalLockfileWait + waitForUnityStartupMarkerForLaunch = originalStartupMarkerWait waitForToolReadinessForLaunch = originalReadinessWait }) @@ -169,6 +171,121 @@ func TestRunLaunchWritesReadyResponseAfterToolReadiness(t *testing.T) { } } +func TestWaitForLaunchReadinessUsesLaunchTimeout(t *testing.T) { + // Verifies launch gets a longer startup window without changing shared readiness defaults. + originalReadinessWait := waitForToolReadinessForLaunch + var capturedTimeout time.Duration + waitForToolReadinessForLaunch = func(ctx context.Context, projectRoot string, timeout time.Duration) error { + capturedTimeout = timeout + return nil + } + t.Cleanup(func() { + waitForToolReadinessForLaunch = originalReadinessWait + }) + + if err := waitForLaunchReadiness(context.Background(), t.TempDir()); err != nil { + t.Fatalf("waitForLaunchReadiness failed: %v", err) + } + if capturedTimeout != launchReadinessTimeout { + t.Fatalf("launch readiness timeout mismatch: %s", capturedTimeout) + } +} + +func TestWaitForLaunchReadinessWrapsStartupTimeout(t *testing.T) { + // Verifies launch timeout errors receive the launch-specific startup classification. + originalReadinessWait := waitForToolReadinessForLaunch + waitForToolReadinessForLaunch = func(ctx context.Context, projectRoot string, timeout time.Duration) error { + return unityServerNotRespondingError{ + projectRoot: projectRoot, + endpoint: "/tmp/uloop/UnityCliLoop-sample.sock", + cause: errors.New("timed out waiting for Unity tool readiness"), + } + } + t.Cleanup(func() { + waitForToolReadinessForLaunch = originalReadinessWait + }) + + err := waitForLaunchReadiness(context.Background(), t.TempDir()) + + var startupErr launchStartupTimeoutError + if !errors.As(err, &startupErr) { + t.Fatalf("expected launch startup timeout error, got %v", err) + } +} + +func TestWaitForLaunchReadinessWrapsInternalProbeDeadline(t *testing.T) { + // Verifies probe deadlines are classified as launch startup timeouts while the parent context is active. + originalReadinessWait := waitForToolReadinessForLaunch + waitForToolReadinessForLaunch = func(ctx context.Context, projectRoot string, timeout time.Duration) error { + return unityServerNotRespondingError{ + projectRoot: projectRoot, + endpoint: "/tmp/uloop/UnityCliLoop-sample.sock", + cause: fmt.Errorf("probe deadline: %w", context.DeadlineExceeded), + } + } + t.Cleanup(func() { + waitForToolReadinessForLaunch = originalReadinessWait + }) + + err := waitForLaunchReadiness(context.Background(), t.TempDir()) + + var startupErr launchStartupTimeoutError + if !errors.As(err, &startupErr) { + t.Fatalf("expected launch startup timeout error, got %v", err) + } + if !errors.Is(startupErr.Unwrap(), context.DeadlineExceeded) { + t.Fatalf("startup timeout should preserve probe deadline cause: %v", startupErr.Unwrap()) + } +} + +func TestWaitForLaunchReadinessPreservesNoProcessReachability(t *testing.T) { + // Verifies a launch whose Editor exited before readiness does not report that Unity is running. + originalReadinessWait := waitForToolReadinessForLaunch + waitForToolReadinessForLaunch = func(ctx context.Context, projectRoot string, timeout time.Duration) error { + return fmt.Errorf("timed out waiting for Unity tool readiness: %w", &unityipc.ConnectionAttemptError{ + ProjectRoot: projectRoot, + Endpoint: "/tmp/uloop/UnityCliLoop-sample.sock", + Cause: errors.New("connect failed"), + }) + } + t.Cleanup(func() { + waitForToolReadinessForLaunch = originalReadinessWait + }) + + err := waitForLaunchReadiness(context.Background(), t.TempDir()) + + var startupErr launchStartupTimeoutError + if errors.As(err, &startupErr) { + t.Fatalf("exited launch should not be classified as startup timeout: %v", err) + } + cliErr := classifyError(err, errorContext{command: launchCommandName}) + if cliErr.ErrorCode != errorCodeUnityNotReachable { + t.Fatalf("error code mismatch: %#v", cliErr) + } + if strings.Contains(cliErr.Message, "Unity is running") { + t.Fatalf("message should not claim Unity is running: %#v", cliErr) + } +} + +func TestWaitForLaunchReadinessPreservesParentCancellation(t *testing.T) { + // Verifies caller cancellation is not converted into a launch startup timeout. + originalReadinessWait := waitForToolReadinessForLaunch + waitForToolReadinessForLaunch = func(ctx context.Context, projectRoot string, timeout time.Duration) error { + return ctx.Err() + } + t.Cleanup(func() { + waitForToolReadinessForLaunch = originalReadinessWait + }) + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + err := waitForLaunchReadiness(ctx, t.TempDir()) + + if !errors.Is(err, context.Canceled) { + t.Fatalf("expected parent cancellation, got %v", err) + } +} + func TestRunLaunchWritesStructuredResponseForExistingUnityProcess(t *testing.T) { // Verifies launch reports machine-readable readiness when Unity was already running. originalFinder := findRunningUnityProcessForLaunch @@ -181,7 +298,7 @@ func TestRunLaunchWritesStructuredResponseForExistingUnityProcess(t *testing.T) focusUnityProcessForLaunch = func(context.Context, int) error { return nil } - waitForToolReadinessForLaunch = func(context.Context, string) error { + waitForToolReadinessForLaunch = func(context.Context, string, time.Duration) error { readinessChecked = true return nil } @@ -229,9 +346,11 @@ func TestRunLaunchRestartWritesProcessTransitionResponse(t *testing.T) { originalFinder := findRunningUnityProcessForLaunch originalKiller := killUnityProcessForLaunch originalResolver := resolveUnityExecutablePathForLaunch - originalLockfileWait := waitForUnityLockfileForLaunch + originalExitWait := waitForUnityProcessExitForLaunch + originalStartupMarkerWait := waitForUnityStartupMarkerForLaunch originalReadinessWait := waitForToolReadinessForLaunch killedPid := 0 + waitedPid := 0 findRunningUnityProcessForLaunch = func(context.Context, string) (*unityProcess, error) { return &unityProcess{pid: 222}, nil } @@ -239,20 +358,25 @@ func TestRunLaunchRestartWritesProcessTransitionResponse(t *testing.T) { killedPid = pid return nil } + waitForUnityProcessExitForLaunch = func(ctx context.Context, projectRoot string, pid int, pollInterval time.Duration, timeout time.Duration) error { + waitedPid = pid + return nil + } resolveUnityExecutablePathForLaunch = func(string) (string, error) { return "/usr/bin/true", nil } - waitForUnityLockfileForLaunch = func(context.Context, string, time.Duration, time.Duration) error { + waitForUnityStartupMarkerForLaunch = func(context.Context, string, time.Duration, time.Duration) error { return nil } - waitForToolReadinessForLaunch = func(context.Context, string) error { + waitForToolReadinessForLaunch = func(context.Context, string, time.Duration) error { return nil } t.Cleanup(func() { findRunningUnityProcessForLaunch = originalFinder killUnityProcessForLaunch = originalKiller resolveUnityExecutablePathForLaunch = originalResolver - waitForUnityLockfileForLaunch = originalLockfileWait + waitForUnityProcessExitForLaunch = originalExitWait + waitForUnityStartupMarkerForLaunch = originalStartupMarkerWait waitForToolReadinessForLaunch = originalReadinessWait }) @@ -274,6 +398,9 @@ func TestRunLaunchRestartWritesProcessTransitionResponse(t *testing.T) { if killedPid != 222 { t.Fatalf("restart killed pid mismatch: %d", killedPid) } + if waitedPid != 222 { + t.Fatalf("restart waited pid mismatch: %d", waitedPid) + } response := decodeLaunchResponseFromOutput(t, stdout.String()) if !response.Success || !response.Ready || !response.ServerReady || !response.ProjectIpcReady { t.Fatalf("ready flags mismatch: %#v", response) @@ -289,6 +416,105 @@ func TestRunLaunchRestartWritesProcessTransitionResponse(t *testing.T) { } } +func TestRunLaunchQuitWaitsForKilledUnityProcess(t *testing.T) { + // Verifies quit does not report success before the killed Unity process disappears. + originalFinder := findRunningUnityProcessForLaunch + originalKiller := killUnityProcessForLaunch + originalExitWait := waitForUnityProcessExitForLaunch + waitedPid := 0 + findRunningUnityProcessForLaunch = func(context.Context, string) (*unityProcess, error) { + return &unityProcess{pid: 333}, nil + } + killUnityProcessForLaunch = func(pid int) error { + return nil + } + waitForUnityProcessExitForLaunch = func(ctx context.Context, projectRoot string, pid int, pollInterval time.Duration, timeout time.Duration) error { + waitedPid = pid + return nil + } + t.Cleanup(func() { + findRunningUnityProcessForLaunch = originalFinder + killUnityProcessForLaunch = originalKiller + waitForUnityProcessExitForLaunch = originalExitWait + }) + + projectRoot := createLaunchTestProject(t) + var stdout bytes.Buffer + var stderr bytes.Buffer + + code := runLaunch( + context.Background(), + launchOptions{projectPath: projectRoot, quit: true}, + projectRoot, + &stdout, + &stderr, + ) + + if code != 0 { + t.Fatalf("exit code mismatch: %d stderr=%s", code, stderr.String()) + } + if waitedPid != 333 { + t.Fatalf("quit waited pid mismatch: %d", waitedPid) + } + response := decodeLaunchResponseFromOutput(t, stdout.String()) + if !response.Success || !response.Quit { + t.Fatalf("quit response mismatch: %#v", response) + } + if response.PreviousProcessId == nil || *response.PreviousProcessId != 333 { + t.Fatalf("previous process id mismatch: %#v", response.PreviousProcessId) + } +} + +func TestRunLaunchRestartReportsProcessExitWaitFailure(t *testing.T) { + // Verifies restart stops before Temp cleanup when the killed Unity process still holds files. + originalFinder := findRunningUnityProcessForLaunch + originalKiller := killUnityProcessForLaunch + originalExitWait := waitForUnityProcessExitForLaunch + originalResolver := resolveUnityExecutablePathForLaunch + resolverCalled := false + findRunningUnityProcessForLaunch = func(context.Context, string) (*unityProcess, error) { + return &unityProcess{pid: 444}, nil + } + killUnityProcessForLaunch = func(pid int) error { + return nil + } + waitForUnityProcessExitForLaunch = func(ctx context.Context, projectRoot string, pid int, pollInterval time.Duration, timeout time.Duration) error { + return errors.New("still exiting") + } + resolveUnityExecutablePathForLaunch = func(string) (string, error) { + resolverCalled = true + return "/usr/bin/true", nil + } + t.Cleanup(func() { + findRunningUnityProcessForLaunch = originalFinder + killUnityProcessForLaunch = originalKiller + waitForUnityProcessExitForLaunch = originalExitWait + resolveUnityExecutablePathForLaunch = originalResolver + }) + + projectRoot := createLaunchTestProject(t) + var stdout bytes.Buffer + var stderr bytes.Buffer + + code := runLaunch( + context.Background(), + launchOptions{projectPath: projectRoot, restart: true}, + projectRoot, + &stdout, + &stderr, + ) + + if code != 1 { + t.Fatalf("expected failure, got %d stdout=%s", code, stdout.String()) + } + if resolverCalled { + t.Fatal("restart should not launch a new Unity process before the old one exits") + } + if !strings.Contains(stderr.String(), "still exiting") { + t.Fatalf("stderr should include wait failure: %s", stderr.String()) + } +} + // Verifies launch logs when it focuses an already-running Unity process. func TestRunLaunchWritesExistingFocusSuccessVibeLog(t *testing.T) { enableCliVibeLog(t) @@ -302,7 +528,7 @@ func TestRunLaunchWritesExistingFocusSuccessVibeLog(t *testing.T) { focusUnityProcessForLaunch = func(context.Context, int) error { return nil } - waitForToolReadinessForLaunch = func(context.Context, string) error { + waitForToolReadinessForLaunch = func(context.Context, string, time.Duration) error { return nil } t.Cleanup(func() { @@ -352,7 +578,7 @@ func TestRunLaunchWritesExistingFocusFailureVibeLog(t *testing.T) { focusUnityProcessForLaunch = func(context.Context, int) error { return fmt.Errorf("activation denied") } - waitForToolReadinessForLaunch = func(context.Context, string) error { + waitForToolReadinessForLaunch = func(context.Context, string, time.Duration) error { return nil } t.Cleanup(func() { @@ -432,13 +658,14 @@ func TestCleanStaleUnityTempDeletesTempWhenLockfileExists(t *testing.T) { } } -func TestWaitForUnityLockfileReturnsAfterLockfileAppears(t *testing.T) { +func TestWaitForUnityStartupMarkerReturnsAfterLockfileAppears(t *testing.T) { + // Verifies the startup marker wait returns as soon as Unity creates the lockfile. projectRoot := createLaunchTestProject(t) lockfilePath := unityLockfilePath(projectRoot) errChan := make(chan error, 1) go func() { - errChan <- waitForUnityLockfile(context.Background(), lockfilePath, time.Millisecond, time.Second) + errChan <- waitForUnityStartupMarkerOrTimeout(context.Background(), lockfilePath, time.Millisecond, time.Second) }() if err := os.MkdirAll(filepath.Dir(lockfilePath), 0o755); err != nil { @@ -451,10 +678,45 @@ func TestWaitForUnityLockfileReturnsAfterLockfileAppears(t *testing.T) { select { case err := <-errChan: if err != nil { - t.Fatalf("waitForUnityLockfile failed: %v", err) + t.Fatalf("waitForUnityStartupMarkerOrTimeout failed: %v", err) } case <-time.After(time.Second): - t.Fatal("timed out waiting for waitForUnityLockfile") + t.Fatal("timed out waiting for waitForUnityStartupMarkerOrTimeout") + } +} + +func TestWaitForUnityStartupMarkerReturnsNilWhenLockfileDoesNotAppear(t *testing.T) { + // Verifies the startup marker is only a short hint before the real readiness wait. + lockfilePath := filepath.Join(t.TempDir(), launchTempDirectoryName, unityLockfileName) + + err := waitForUnityStartupMarkerOrTimeout(context.Background(), lockfilePath, time.Millisecond, time.Millisecond) + if err != nil { + t.Fatalf("missing startup marker should not fail launch: %v", err) + } +} + +func TestWaitForUnityProcessExitBoundsProcessScan(t *testing.T) { + // Verifies exit waiting applies the exit timeout to each running-process scan. + originalFinder := findRunningUnityProcessForLaunch + findRunningUnityProcessForLaunch = func(ctx context.Context, projectRoot string) (*unityProcess, error) { + if _, ok := ctx.Deadline(); !ok { + return nil, errors.New("missing process scan deadline") + } + <-ctx.Done() + return nil, ctx.Err() + } + t.Cleanup(func() { + findRunningUnityProcessForLaunch = originalFinder + }) + + err := waitForUnityProcessExit(context.Background(), t.TempDir(), 123, time.Hour, 10*time.Millisecond) + + var timeoutErr launchProcessExitTimeoutError + if !errors.As(err, &timeoutErr) { + t.Fatalf("process exit timeout mismatch: %v", err) + } + if timeoutErr.pid != 123 { + t.Fatalf("process exit timeout pid mismatch: %#v", timeoutErr) } } diff --git a/cli/internal/cli/tool_readiness.go b/cli/internal/cli/tool_readiness.go index 491174fff..712213901 100644 --- a/cli/internal/cli/tool_readiness.go +++ b/cli/internal/cli/tool_readiness.go @@ -3,6 +3,7 @@ package cli import ( "context" "encoding/json" + "errors" "fmt" "time" @@ -19,21 +20,31 @@ const ( const executeDynamicCodeReadinessProbe = `return "Unity CLI Loop dynamic code prewarm";` -var findRunningUnityProcessForReadiness = findRunningUnityProcess +var ( + findRunningUnityProcessForReadiness = findRunningUnityProcess + probeToolReadinessSequenceForReadiness = probeToolReadinessSequence +) func waitForToolReadiness(ctx context.Context, projectRoot string) error { + return waitForToolReadinessWithTimeout(ctx, projectRoot, toolReadinessTimeout) +} + +func waitForToolReadinessWithTimeout(ctx context.Context, projectRoot string, timeout time.Duration) error { // Why: launch and compile can both recreate Unity's project IPC server; a real // tool request proves the user-visible command will not be the cold transport probe. - timeoutContext, cancel := context.WithTimeout(ctx, toolReadinessTimeout) + timeoutContext, cancel := context.WithTimeout(ctx, timeout) defer cancel() var lastErr error ticker := time.NewTicker(toolReadinessPoll) defer ticker.Stop() for { - if err := probeToolReadinessSequence(timeoutContext, projectRoot); err == nil { + if err := probeToolReadinessSequenceForReadiness(timeoutContext, projectRoot); err == nil { return nil } else { + if isReadinessCLIUpdateRequiredError(err) { + return err + } lastErr = err } @@ -63,6 +74,23 @@ func toolReadinessDoneError(ctx context.Context, projectRoot string, cause error return fmt.Errorf("timed out waiting for Unity tool readiness") } +func isReadinessCLIUpdateRequiredError(err error) bool { + var rpcErr *unityipc.RPCError + if !errors.As(err, &rpcErr) || len(rpcErr.Data) == 0 { + return false + } + + var data any + if json.Unmarshal(rpcErr.Data, &data) != nil { + return false + } + typedData, ok := data.(map[string]any) + if !ok { + return false + } + return rpcDataType(typedData) == "cli_update_required" +} + func probeToolReadinessSequence(ctx context.Context, projectRoot string) error { // The tool catalog is read from disk; it can change between poll ticks (Unity writes // it during server startup) but not within one probe sequence, so read it once here diff --git a/cli/internal/cli/tool_readiness_test.go b/cli/internal/cli/tool_readiness_test.go index 8d04e3822..85ba74f03 100644 --- a/cli/internal/cli/tool_readiness_test.go +++ b/cli/internal/cli/tool_readiness_test.go @@ -2,10 +2,59 @@ package cli import ( "context" + "encoding/json" "errors" "testing" + "time" + + "github.com/hatayama/unity-cli-loop/cli/internal/unityipc" ) +// Verifies shared readiness waits keep the shorter non-launch timeout. +func TestWaitForToolReadinessUsesDefaultTimeout(t *testing.T) { + originalProbe := probeToolReadinessSequenceForReadiness + probeToolReadinessSequenceForReadiness = func(ctx context.Context, projectRoot string) error { + deadline, ok := ctx.Deadline() + if !ok { + t.Fatal("readiness probe context should have a deadline") + } + remaining := time.Until(deadline) + if remaining < toolReadinessTimeout-time.Second || remaining > toolReadinessTimeout { + t.Fatalf("readiness timeout mismatch: %s", remaining) + } + return nil + } + t.Cleanup(func() { + probeToolReadinessSequenceForReadiness = originalProbe + }) + + if err := waitForToolReadiness(context.Background(), t.TempDir()); err != nil { + t.Fatalf("waitForToolReadiness failed: %v", err) + } +} + +// Verifies protocol mismatch responses surface immediately instead of waiting for readiness timeout. +func TestWaitForToolReadinessReturnsCliUpdateRequiredImmediately(t *testing.T) { + originalProbe := probeToolReadinessSequenceForReadiness + expectedErr := &unityipc.RPCError{ + Code: -32603, + Message: "The installed uloop CLI uses an IPC protocol that does not match this Unity package.", + Data: json.RawMessage(`{"type":"cli_update_required"}`), + } + probeToolReadinessSequenceForReadiness = func(context.Context, string) error { + return expectedErr + } + t.Cleanup(func() { + probeToolReadinessSequenceForReadiness = originalProbe + }) + + err := waitForToolReadinessWithTimeout(context.Background(), t.TempDir(), time.Hour) + + if !errors.Is(err, expectedErr) { + t.Fatalf("expected cli update error, got %v", err) + } +} + // Verifies that parent cancellation is preserved instead of being reported as a timeout. func TestToolReadinessDoneErrorPropagatesParentCancellation(t *testing.T) { ctx, cancel := context.WithCancel(context.Background())