From 7474c7c1a226d966445cd3110a03ce03cbe52269 Mon Sep 17 00:00:00 2001 From: hatayama Date: Tue, 27 Oct 2026 14:30:25 +0900 Subject: [PATCH 01/12] Move the server-gone predicate into clierrors The compile wait's check for "Unity's server went away" is about to be needed outside the project runner: the readiness wait in clicore will use it to decide when to look for the Editor process. Moving it next to the other transport predicates keeps one definition for both modules. --- cli/common/errors/transport_errors.go | 15 ++++++++ cli/common/errors/transport_errors_test.go | 36 +++++++++++++++++++ cli/dispatcher/shared-inputs-stamp.json | 2 +- .../projectrunner/compile_fresh_recovery.go | 17 +-------- .../compile_fresh_recovery_test.go | 2 +- cli/project-runner/shared-inputs-stamp.json | 2 +- 6 files changed, 55 insertions(+), 19 deletions(-) diff --git a/cli/common/errors/transport_errors.go b/cli/common/errors/transport_errors.go index a0abd491f9..732e6edede 100644 --- a/cli/common/errors/transport_errors.go +++ b/cli/common/errors/transport_errors.go @@ -60,6 +60,21 @@ func IsPermanentConnectError(err error) bool { return errors.Is(connectionErr, os.ErrPermission) } +// IsUnityServerGoneError reports whether a failed request shows that Unity's server went away: the +// connection dropped mid-request, or nobody was listening. +func IsUnityServerGoneError(err error) bool { + if IsTransportDisconnectError(err) { + return true + } + var connectionErr *unityipc.ConnectionAttemptError + if !errors.As(err, &connectionErr) { + return false + } + // Why not a connect timeout or a denied connect: a Windows named pipe times out while all of its + // instances are busy, and a sandbox can deny the connect, both while the server is alive. + return !IsFinalResponseTimeoutError(err) && !IsPermanentConnectError(err) +} + // Reports whether the error is a connection deadline expiry. The Timeout() probe // runs through the unwrap chain because go-winio's named pipe deadline error is not // os.ErrDeadlineExceeded and os.IsTimeout does not unwrap fmt.Errorf("%w") wrapping. diff --git a/cli/common/errors/transport_errors_test.go b/cli/common/errors/transport_errors_test.go index bc6c8627a5..0fac82acd5 100644 --- a/cli/common/errors/transport_errors_test.go +++ b/cli/common/errors/transport_errors_test.go @@ -184,3 +184,39 @@ func TestIsFinalResponseTimeoutErrorUnwrapsTimeoutCauses(t *testing.T) { t.Fatal("wrapped Timeout()-reporting error was not classified as timeout") } } + +// Verifies a dropped connection and a dial nobody answered count as the server being gone, while a +// dial timeout and a denied dial do not, because the server can be alive behind both. +func TestIsUnityServerGoneErrorSeparatesAGoneServerFromALiveOne(t *testing.T) { + cases := []struct { + name string + err error + want bool + }{ + {name: "dropped connection", err: fmt.Errorf("read: %w", io.EOF), want: true}, + { + name: "nobody listening", + err: &unityipc.ConnectionAttemptError{Cause: syscall.ECONNREFUSED}, + want: true, + }, + { + name: "dial timeout", + err: &unityipc.ConnectionAttemptError{Cause: os.ErrDeadlineExceeded}, + want: false, + }, + { + name: "denied dial", + err: &unityipc.ConnectionAttemptError{Cause: syscall.EPERM}, + want: false, + }, + {name: "unrelated error", err: fmt.Errorf("decode failed"), want: false}, + } + + for _, testCase := range cases { + t.Run(testCase.name, func(t *testing.T) { + if got := IsUnityServerGoneError(testCase.err); got != testCase.want { + t.Fatalf("IsUnityServerGoneError(%v) = %v, want %v", testCase.err, got, testCase.want) + } + }) + } +} diff --git a/cli/dispatcher/shared-inputs-stamp.json b/cli/dispatcher/shared-inputs-stamp.json index 40ae1d7507..8f5f51c779 100644 --- a/cli/dispatcher/shared-inputs-stamp.json +++ b/cli/dispatcher/shared-inputs-stamp.json @@ -1,4 +1,4 @@ { "schemaVersion": 1, - "sharedInputsHash": "b453636d0586a882f075ef2d0e77f2da79354454" + "sharedInputsHash": "55f3c34ad0b7085874015766a1782d91433445e2" } diff --git a/cli/project-runner/internal/projectrunner/compile_fresh_recovery.go b/cli/project-runner/internal/projectrunner/compile_fresh_recovery.go index 359e48de3b..8a1226fda2 100644 --- a/cli/project-runner/internal/projectrunner/compile_fresh_recovery.go +++ b/cli/project-runner/internal/projectrunner/compile_fresh_recovery.go @@ -101,7 +101,7 @@ type compileRequestMissingTracker struct { // slot the first request still holds. func (tracker *compileRequestMissingTracker) observe(status compileStatusResponse, err error) bool { if err != nil { - if isServerGoneError(err) { + if clierrors.IsUnityServerGoneError(err) { tracker.serverRestartSeen = true } tracker.missingStreak = 0 @@ -118,21 +118,6 @@ func (tracker *compileRequestMissingTracker) observe(status compileStatusRespons return tracker.serverRestartSeen && tracker.missingStreak >= compileAttachMissingResultStreak } -// isServerGoneError reports whether a failed status query shows that Unity's server went away: the -// connection dropped mid-query, or nobody was listening. -func isServerGoneError(err error) bool { - if clierrors.IsTransportDisconnectError(err) { - return true - } - var connectionErr *unityipc.ConnectionAttemptError - if !errors.As(err, &connectionErr) { - return false - } - // Why not a connect timeout or a denied connect: a Windows named pipe times out while all of its - // instances are busy, and a sandbox can deny the connect, both while the server is alive. - return !clierrors.IsFinalResponseTimeoutError(err) && !clierrors.IsPermanentConnectError(err) -} - // canResendCompile reports whether at least compileResendMinimumBudget is left before resendBefore. // A zero resendBefore never allows a resend. func canResendCompile(resendBefore time.Time) bool { diff --git a/cli/project-runner/internal/projectrunner/compile_fresh_recovery_test.go b/cli/project-runner/internal/projectrunner/compile_fresh_recovery_test.go index 4a9129b410..1b596ea6e6 100644 --- a/cli/project-runner/internal/projectrunner/compile_fresh_recovery_test.go +++ b/cli/project-runner/internal/projectrunner/compile_fresh_recovery_test.go @@ -641,7 +641,7 @@ func TestStatusQueryAgainstNoListenerCountsAsTheServerBeingGone(t *testing.T) { if errors.Is(err, os.ErrPermission) { t.Skipf("the environment denied the connection itself, so it cannot show a missing listener: %v", err) } - if !isServerGoneError(err) { + if !clierrors.IsUnityServerGoneError(err) { t.Fatalf("a status query with no listener must count as the server being gone: %v", err) } } diff --git a/cli/project-runner/shared-inputs-stamp.json b/cli/project-runner/shared-inputs-stamp.json index 44a5818d18..639babeae7 100644 --- a/cli/project-runner/shared-inputs-stamp.json +++ b/cli/project-runner/shared-inputs-stamp.json @@ -1,4 +1,4 @@ { "schemaVersion": 1, - "sharedInputsHash": "7b1a205dce89310e512997eef8defeb7ecc4e533" + "sharedInputsHash": "8b1946904c0c5107e5a7e2c96a4e896a46b0e959" } From fc5cb6d846b143790a67e2625fc4f097c710444f Mon Sep 17 00:00:00 2001 From: hatayama Date: Tue, 27 Oct 2026 14:32:44 +0900 Subject: [PATCH 02/12] Report UNITY_NOT_RUNNING when no Unity Editor is running Agents kept missing that Unity had crashed or closed. A command against a closed Editor answered UNITY_NOT_REACHABLE with Retryable true and "if Unity is starting, compiling, or reloading scripts, wait and retry", even though the retry loop had just looked up the process list and found no Editor. The agent was told to wait for an Editor that was not there. - When that lookup answers without an Editor, the command now fails with UNITY_NOT_RUNNING, Retryable false, and `uloop launch` as the step. A lookup that failed still leaves the dial error as it was. - A connection that drops after the request reached Unity is checked the same way, so an Editor that exits mid-command is no longer reported as a domain reload to wait out. - When Temp/UnityLockfile is left behind, the message says the Editor did not shut down normally and may have crashed, and names the Editor log to read before launching, since the next launch renames it. `uloop status` NotRunning uses the same wording. The new error neither unwraps to its cause nor repeats it in Error(), so the callers that wait out a disconnect do not mistake it for one. --- cli/common/clicore/unity_exit.go | 34 ++++ cli/common/clicore/unity_exit_test.go | 45 +++++ cli/common/errors/error_envelope.go | 1 + cli/common/errors/unity_not_running.go | 99 +++++++++++ cli/common/errors/unity_not_running_test.go | 167 ++++++++++++++++++ cli/common/unityprocess/editor_files.go | 54 ++++++ cli/common/unityprocess/editor_files_test.go | 46 +++++ cli/dispatcher/internal/dispatcher/launch.go | 6 +- cli/dispatcher/shared-inputs-stamp.json | 2 +- .../projectrunner/connection_retry.go | 5 +- .../projectrunner/connection_retry_flow.go | 36 +++- .../connection_retry_flow_test.go | 98 ++++++++++ .../projectrunner/connection_retry_test.go | 5 +- .../control_play_mode_wait_test.go | 1 + .../hot_reload_editor_ready_retry_test.go | 1 + .../internal/projectrunner/status_command.go | 13 +- .../projectrunner/status_command_test.go | 35 ++++ .../projectrunner/unity_process_lookup.go | 11 ++ .../unity_process_lookup_helpers_test.go | 37 ++++ cli/project-runner/shared-inputs-stamp.json | 2 +- 20 files changed, 683 insertions(+), 15 deletions(-) create mode 100644 cli/common/clicore/unity_exit.go create mode 100644 cli/common/clicore/unity_exit_test.go create mode 100644 cli/common/errors/unity_not_running.go create mode 100644 cli/common/errors/unity_not_running_test.go create mode 100644 cli/common/unityprocess/editor_files.go create mode 100644 cli/common/unityprocess/editor_files_test.go create mode 100644 cli/project-runner/internal/projectrunner/unity_process_lookup.go create mode 100644 cli/project-runner/internal/projectrunner/unity_process_lookup_helpers_test.go diff --git a/cli/common/clicore/unity_exit.go b/cli/common/clicore/unity_exit.go new file mode 100644 index 0000000000..343f2715c1 --- /dev/null +++ b/cli/common/clicore/unity_exit.go @@ -0,0 +1,34 @@ +package clicore + +import ( + "os" + + clierrors "github.com/hatayama/unity-cli-loop/common/errors" + "github.com/hatayama/unity-cli-loop/common/unityprocess" +) + +// NewUnityNotRunningError describes a project whose Unity Editor a successful process lookup did not +// find, noting whether the Editor left its lockfile behind. exitedDuringCommand says the command had +// reached Unity before it went away. +func NewUnityNotRunningError( + projectRoot string, + endpoint string, + cause error, + exitedDuringCommand bool, +) clierrors.UnityNotRunningError { + return clierrors.UnityNotRunningError{ + ProjectRoot: projectRoot, + Endpoint: endpoint, + Cause: cause, + LockfileLeftBehind: lockfileLeftBehind(projectRoot), + EditorLogPath: unityprocess.DefaultEditorLogPath(), + ExitedDuringCommand: exitedDuringCommand, + } +} + +// Why only a lockfile that is certainly there counts: a stat that fails for another reason does not show +// how Unity shut down, and the abnormal-exit guidance must not rest on it. +func lockfileLeftBehind(projectRoot string) bool { + _, err := os.Stat(unityprocess.LockfilePath(projectRoot)) + return err == nil +} diff --git a/cli/common/clicore/unity_exit_test.go b/cli/common/clicore/unity_exit_test.go new file mode 100644 index 0000000000..21a62d3f51 --- /dev/null +++ b/cli/common/clicore/unity_exit_test.go @@ -0,0 +1,45 @@ +package clicore + +import ( + "io" + "os" + "path/filepath" + "testing" + + "github.com/hatayama/unity-cli-loop/common/unityprocess" +) + +// Verifies the not-running error records a lockfile left behind, which only a shutdown that did not +// finish leaves, along with the Editor log location. +func TestNewUnityNotRunningErrorRecordsALockfileLeftBehind(t *testing.T) { + projectRoot := t.TempDir() + lockfilePath := unityprocess.LockfilePath(projectRoot) + if err := os.MkdirAll(filepath.Dir(lockfilePath), 0o755); err != nil { + t.Fatalf("creating Temp failed: %v", err) + } + if err := os.WriteFile(lockfilePath, []byte{}, 0o644); err != nil { + t.Fatalf("writing the lockfile failed: %v", err) + } + + err := NewUnityNotRunningError(projectRoot, "/tmp/uloop/sample.sock", io.EOF, true) + + if !err.LockfileLeftBehind { + t.Fatal("expected the lockfile left behind to be recorded") + } + if err.EditorLogPath != unityprocess.DefaultEditorLogPath() { + t.Fatalf("EditorLogPath = %q, want the default Editor log path", err.EditorLogPath) + } + if err.ProjectRoot != projectRoot || err.Endpoint != "/tmp/uloop/sample.sock" || err.Cause != io.EOF || + !err.ExitedDuringCommand { + t.Fatalf("the arguments were not carried over: %#v", err) + } +} + +// Verifies a project without a lockfile is reported as a normal shutdown. +func TestNewUnityNotRunningErrorWithoutALockfile(t *testing.T) { + err := NewUnityNotRunningError(t.TempDir(), "", nil, false) + + if err.LockfileLeftBehind { + t.Fatal("a project without a lockfile must not be reported as an abnormal exit") + } +} diff --git a/cli/common/errors/error_envelope.go b/cli/common/errors/error_envelope.go index c4460d6bbf..44ddd80e75 100644 --- a/cli/common/errors/error_envelope.go +++ b/cli/common/errors/error_envelope.go @@ -14,6 +14,7 @@ const ( ErrorCodeUnknownCommand = "UNKNOWN_COMMAND" errorCodeProjectNotFound = "PROJECT_NOT_FOUND" ErrorCodeUnityNotReachable = "UNITY_NOT_REACHABLE" + ErrorCodeUnityNotRunning = "UNITY_NOT_RUNNING" ErrorCodeUnityStartupTimeout = "UNITY_STARTUP_TIMEOUT" ErrorCodeUnityProcessExitTimeout = "UNITY_PROCESS_EXIT_TIMEOUT" errorCodeUnityDisconnectedAfterDispatch = "UNITY_DISCONNECTED_AFTER_DISPATCH" diff --git a/cli/common/errors/unity_not_running.go b/cli/common/errors/unity_not_running.go new file mode 100644 index 0000000000..112c9223d0 --- /dev/null +++ b/cli/common/errors/unity_not_running.go @@ -0,0 +1,99 @@ +package clierrors + +// UnityNotRunningError reports that a process lookup that succeeded found no Unity Editor for the +// project, after the server could not be reached. Only a lookup that answered may produce it: a lookup +// that failed observed nothing and must leave the connection error as it is. +// +// Why it neither unwraps to Cause nor repeats it in Error(): the callers that wait out a domain reload +// recognise a dropped connection through errors.Is and through message text such as "EOF". Matching +// either would send them into a wait for a server that cannot come back, which is the failure this error +// exists to end. The cause is kept for Details only. +type UnityNotRunningError struct { + ProjectRoot string + Endpoint string + Cause error + // LockfileLeftBehind is true when Temp/UnityLockfile, which Unity removes when it shuts down normally, + // is still there. + LockfileLeftBehind bool + // EditorLogPath is Unity's default Editor log location; empty when it could not be resolved. + EditorLogPath string + // ExitedDuringCommand is true when this command had reached Unity before it went away, so the command + // may have partly run. + ExitedDuringCommand bool +} + +func (err UnityNotRunningError) Error() string { + if err.ExitedDuringCommand { + return "Unity exited while the command was running" + } + return "no Unity Editor is running for this project" +} + +func (err UnityNotRunningError) ToCLIError(context ErrorContext) CLIError { + phase := ErrorPhaseConnection + safeToRetry := true + if err.ExitedDuringCommand { + phase = ErrorPhaseResponseWaiting + safeToRetry = isSafeRetryCommand(context.Command) + } + return CLIError{ + ErrorCode: ErrorCodeUnityNotRunning, + Phase: phase, + Message: err.message(), + Retryable: false, + SafeToRetry: safeToRetry, + ProjectRoot: firstNonEmpty(context.ProjectRoot, err.ProjectRoot), + Command: context.Command, + NextActions: err.nextActions(safeToRetry), + Details: err.details(), + } +} + +// Why "may have crashed" and not "crashed": a lockfile left behind proves only that the normal shutdown +// did not finish, which a forced quit or an interrupted cleanup also leaves. +func (err UnityNotRunningError) message() string { + message := "No Unity Editor is running for this project." + if err.ExitedDuringCommand { + message = "Unity exited while this command was running; no Unity Editor is running for this project now." + } + if err.LockfileLeftBehind { + message += " It left Temp/UnityLockfile behind, so it did not shut down normally and may have crashed." + } + return message +} + +func (err UnityNotRunningError) nextActions(safeToRetry bool) []string { + actions := []string{} + if err.LockfileLeftBehind { + actions = append(actions, err.EditorLogNextAction()) + } + if !safeToRetry { + actions = append(actions, "The command may have partly run before Unity exited; check the project and scene state before running it again.") + } + return append(actions, "Run `uloop launch` to start Unity, then run the command again.") +} + +// EditorLogNextAction is the step that sends the caller to the Editor log before launching Unity again. +// Why before the launch step: Unity renames Editor.log to Editor-prev.log when it starts, and keeps only +// that one previous log, so a second launch loses the record of why the Editor went away. +func (err UnityNotRunningError) EditorLogNextAction() string { + if err.EditorLogPath == "" { + return "Before launching, read the end of Unity's Editor log for why it exited; the next launch renames it to Editor-prev.log." + } + return "Before launching, read the end of the Editor log for why it exited: " + err.EditorLogPath + + " (the next launch renames it to Editor-prev.log)." +} + +func (err UnityNotRunningError) details() map[string]any { + details := map[string]any{} + if err.Endpoint != "" { + details["Endpoint"] = err.Endpoint + } + if err.Cause != nil { + details["Cause"] = err.Cause.Error() + } + if err.LockfileLeftBehind && err.EditorLogPath != "" { + details["EditorLog"] = err.EditorLogPath + } + return details +} diff --git a/cli/common/errors/unity_not_running_test.go b/cli/common/errors/unity_not_running_test.go new file mode 100644 index 0000000000..9d80229e66 --- /dev/null +++ b/cli/common/errors/unity_not_running_test.go @@ -0,0 +1,167 @@ +package clierrors + +import ( + "bytes" + "encoding/json" + "errors" + "io" + "strings" + "testing" + + "github.com/hatayama/unity-cli-loop/common/unityipc" +) + +const unityNotRunningTestEditorLog = "/home/user/Library/Logs/Unity/Editor.log" + +// Verifies a command that never reached Unity reports a closed Editor as a condition retrying cannot +// clear, with launching Unity as the first step and no crash guidance. +func TestUnityNotRunningErrorBeforeTheCommandReachedUnity(t *testing.T) { + err := UnityNotRunningError{ + ProjectRoot: "/projects/sample", + Endpoint: "/tmp/uloop/sample.sock", + Cause: errors.New("dial unix /tmp/uloop/sample.sock: connect: no such file or directory"), + } + + cliErr := ClassifyError(err, ErrorContext{Command: "get-logs"}) + + if cliErr.ErrorCode != ErrorCodeUnityNotRunning || cliErr.Phase != ErrorPhaseConnection { + t.Fatalf("unexpected classification: %#v", cliErr) + } + if cliErr.Retryable { + t.Fatal("retrying cannot start Unity, so the error must not be Retryable") + } + if !cliErr.SafeToRetry { + t.Fatal("nothing reached Unity, so running the command again must be SafeToRetry") + } + if cliErr.Message != "No Unity Editor is running for this project." { + t.Fatalf("unexpected message: %q", cliErr.Message) + } + if len(cliErr.NextActions) != 1 || !strings.Contains(cliErr.NextActions[0], "`uloop launch`") { + t.Fatalf("expected launch as the only next action, got %#v", cliErr.NextActions) + } + if cliErr.ProjectRoot != "/projects/sample" { + t.Fatalf("expected the error's project root, got %q", cliErr.ProjectRoot) + } + if cliErr.Details["Cause"] != err.Cause.Error() || cliErr.Details["Endpoint"] != err.Endpoint { + t.Fatalf("expected the cause and endpoint in Details, got %#v", cliErr.Details) + } + if _, ok := cliErr.Details["EditorLog"]; ok { + t.Fatalf("a clean exit must not point at the Editor log, got %#v", cliErr.Details) + } +} + +// Verifies a lockfile left behind is reported as an abnormal exit without asserting a crash, and that +// the Editor log is named before launch because the next launch moves it to Editor-prev.log. +func TestUnityNotRunningErrorPointsAtTheEditorLogWhenTheLockfileWasLeftBehind(t *testing.T) { + err := UnityNotRunningError{ + ProjectRoot: "/projects/sample", + LockfileLeftBehind: true, + EditorLogPath: unityNotRunningTestEditorLog, + } + + cliErr := err.ToCLIError(ErrorContext{Command: "get-logs"}) + + if !strings.Contains(cliErr.Message, "did not shut down normally") || + !strings.Contains(cliErr.Message, "may have crashed") { + t.Fatalf("expected an abnormal-exit message, got %q", cliErr.Message) + } + if len(cliErr.NextActions) != 2 { + t.Fatalf("expected the log step and the launch step, got %#v", cliErr.NextActions) + } + if !strings.Contains(cliErr.NextActions[0], unityNotRunningTestEditorLog) || + !strings.Contains(cliErr.NextActions[0], "Editor-prev.log") { + t.Fatalf("expected the Editor log step first, got %q", cliErr.NextActions[0]) + } + if !strings.Contains(cliErr.NextActions[1], "`uloop launch`") { + t.Fatalf("expected launch after the log step, got %q", cliErr.NextActions[1]) + } + if cliErr.Details["EditorLog"] != unityNotRunningTestEditorLog { + t.Fatalf("expected the Editor log path in Details, got %#v", cliErr.Details) + } +} + +// Verifies the Editor log step still appears, without a path, when the default log location could not +// be resolved. +func TestUnityNotRunningErrorNamesTheEditorLogWithoutAPath(t *testing.T) { + err := UnityNotRunningError{LockfileLeftBehind: true} + + cliErr := err.ToCLIError(ErrorContext{Command: "get-logs"}) + + if len(cliErr.NextActions) != 2 || !strings.Contains(cliErr.NextActions[0], "Editor log") { + t.Fatalf("expected an Editor log step, got %#v", cliErr.NextActions) + } + if _, ok := cliErr.Details["EditorLog"]; ok { + t.Fatalf("an unresolved log path must not appear in Details, got %#v", cliErr.Details) + } +} + +// Verifies an Editor that went away while a mutating command was running warns that the command may +// have partly run, and marks the command unsafe to repeat. +func TestUnityNotRunningErrorAfterUnityExitedDuringAMutatingCommand(t *testing.T) { + err := UnityNotRunningError{ExitedDuringCommand: true} + + cliErr := err.ToCLIError(ErrorContext{Command: "compile"}) + + if cliErr.Phase != ErrorPhaseResponseWaiting { + t.Fatalf("expected the response-waiting phase, got %q", cliErr.Phase) + } + if cliErr.SafeToRetry { + t.Fatal("compile may have partly run, so it must not be SafeToRetry") + } + if !strings.HasPrefix(cliErr.Message, "Unity exited while this command was running") { + t.Fatalf("expected the exit-during-command message, got %q", cliErr.Message) + } + if len(cliErr.NextActions) != 2 || !strings.Contains(cliErr.NextActions[0], "partly run") { + t.Fatalf("expected the partial-run warning before launch, got %#v", cliErr.NextActions) + } +} + +// Verifies a read-only command that was running when Unity went away stays safe to repeat and gets no +// partial-run warning. +func TestUnityNotRunningErrorAfterUnityExitedDuringAReadOnlyCommand(t *testing.T) { + err := UnityNotRunningError{ExitedDuringCommand: true} + + cliErr := err.ToCLIError(ErrorContext{Command: "get-logs"}) + + if !cliErr.SafeToRetry { + t.Fatal("get-logs cannot double-apply, so it must stay SafeToRetry") + } + if len(cliErr.NextActions) != 1 { + t.Fatalf("expected only the launch step, got %#v", cliErr.NextActions) + } +} + +// Verifies the error never reads as a dropped connection, even when its cause is one: callers that see a +// disconnect start waiting for a domain reload that cannot come. +func TestUnityNotRunningErrorDoesNotLookLikeADisconnect(t *testing.T) { + err := UnityNotRunningError{ExitedDuringCommand: true, Cause: io.EOF} + + if IsTransportDisconnectError(err) { + t.Fatalf("UnityNotRunningError must not be classified as a disconnect: %v", err) + } + if IsUnityServerGoneError(err) { + t.Fatalf("UnityNotRunningError must not be classified as a server that may come back: %v", err) + } +} + +// Verifies the tool failure writer prints the not-running envelope for a dispatched request instead of +// the disconnect envelope that tells the caller to wait for a reload. +func TestWriteToolFailureReportsUnityNotRunningForADispatchedRequest(t *testing.T) { + var output bytes.Buffer + err := UnityNotRunningError{ExitedDuringCommand: true, Cause: io.EOF} + + WriteToolFailure( + &output, + err, + unityipc.UnitySendOutcome{RequestDispatched: true, RequestAccepted: true}, + ErrorContext{Command: "run-tests"}, + ) + + var envelope CLIErrorEnvelope + if decodeErr := json.Unmarshal(output.Bytes(), &envelope); decodeErr != nil { + t.Fatalf("decoding the envelope failed: %v\n%s", decodeErr, output.String()) + } + if envelope.Error.ErrorCode != ErrorCodeUnityNotRunning { + t.Fatalf("expected %s, got %#v", ErrorCodeUnityNotRunning, envelope.Error) + } +} diff --git a/cli/common/unityprocess/editor_files.go b/cli/common/unityprocess/editor_files.go new file mode 100644 index 0000000000..7c58f6a833 --- /dev/null +++ b/cli/common/unityprocess/editor_files.go @@ -0,0 +1,54 @@ +package unityprocess + +import ( + "os" + "path/filepath" + "runtime" +) + +const ( + // TempDirectoryName is the project directory Unity keeps its per-session files in. + TempDirectoryName = "Temp" + // LockfileName is the file Unity holds while it has the project open and removes on a normal + // shutdown. + LockfileName = "UnityLockfile" +) + +// LockfilePath returns where Unity keeps the lockfile for the project at projectRoot. +func LockfilePath(projectRoot string) string { + return filepath.Join(projectRoot, TempDirectoryName, LockfileName) +} + +// DefaultEditorLogPath returns where Unity writes its Editor log when it was not started with -logFile, +// or an empty string when the platform's base directory cannot be resolved. +func DefaultEditorLogPath() string { + homeDir, err := os.UserHomeDir() + if err != nil { + homeDir = "" + } + return editorLogPathFor(runtime.GOOS, homeDir, os.Getenv("LOCALAPPDATA")) +} + +// editorLogPathFor takes the platform and its base directories as parameters so every platform's +// layout can be checked on any host. +func editorLogPathFor(goos string, homeDir string, localAppData string) string { + switch goos { + case "darwin": + return joinUnderBase(homeDir, "Library", "Logs", "Unity", "Editor.log") + case "windows": + return joinUnderBase(localAppData, "Unity", "Editor", "Editor.log") + case "linux": + return joinUnderBase(homeDir, ".config", "unity3d", "Editor.log") + default: + return "" + } +} + +// Why refuse an empty base: filepath.Join would turn it into a path relative to the working directory, +// which points at a file that does not exist. +func joinUnderBase(base string, elements ...string) string { + if base == "" { + return "" + } + return filepath.Join(append([]string{base}, elements...)...) +} diff --git a/cli/common/unityprocess/editor_files_test.go b/cli/common/unityprocess/editor_files_test.go new file mode 100644 index 0000000000..8da266dc64 --- /dev/null +++ b/cli/common/unityprocess/editor_files_test.go @@ -0,0 +1,46 @@ +package unityprocess + +import ( + "path/filepath" + "testing" +) + +// Verifies the lockfile is looked up where Unity keeps it, under the project's Temp directory. +func TestLockfilePathIsUnderTheProjectTempDirectory(t *testing.T) { + projectRoot := filepath.Join("projects", "sample") + + if got, want := LockfilePath(projectRoot), filepath.Join(projectRoot, "Temp", "UnityLockfile"); got != want { + t.Fatalf("LockfilePath() = %q, want %q", got, want) + } +} + +// Verifies the default Editor log location for each platform Unity documents, and that a missing base +// directory yields no path instead of one rooted in the working directory. +func TestEditorLogPathForEachPlatform(t *testing.T) { + const homeDir = "/home/" + const localAppData = `C:\Users\\AppData\Local` + cases := []struct { + name string + goos string + homeDir string + localAppData string + want string + }{ + {name: "macOS", goos: "darwin", homeDir: homeDir, want: filepath.Join(homeDir, "Library", "Logs", "Unity", "Editor.log")}, + {name: "Windows", goos: "windows", localAppData: localAppData, want: filepath.Join(localAppData, "Unity", "Editor", "Editor.log")}, + {name: "Linux", goos: "linux", homeDir: homeDir, want: filepath.Join(homeDir, ".config", "unity3d", "Editor.log")}, + {name: "macOS without a home directory", goos: "darwin", want: ""}, + {name: "Windows without LOCALAPPDATA", goos: "windows", homeDir: homeDir, want: ""}, + {name: "Linux without a home directory", goos: "linux", want: ""}, + {name: "other platform", goos: "plan9", homeDir: homeDir, want: ""}, + } + + for _, testCase := range cases { + t.Run(testCase.name, func(t *testing.T) { + got := editorLogPathFor(testCase.goos, testCase.homeDir, testCase.localAppData) + if got != testCase.want { + t.Fatalf("editorLogPathFor() = %q, want %q", got, testCase.want) + } + }) + } +} diff --git a/cli/dispatcher/internal/dispatcher/launch.go b/cli/dispatcher/internal/dispatcher/launch.go index 36c0bbf683..56a313a4e8 100644 --- a/cli/dispatcher/internal/dispatcher/launch.go +++ b/cli/dispatcher/internal/dispatcher/launch.go @@ -34,8 +34,8 @@ const ( launchFreshFocusRetryDelay = 2 * time.Second projectVersionFilePath = "ProjectSettings/ProjectVersion.txt" recoveryDirectoryPath = "Assets/_Recovery" - launchTempDirectoryName = "Temp" - unityLockfileName = "UnityLockfile" + launchTempDirectoryName = unityprocess.TempDirectoryName + unityLockfileName = unityprocess.LockfileName ) var editorVersionPattern = regexp.MustCompile(`(?m)^m_EditorVersion:\s*(.+)$`) @@ -405,7 +405,7 @@ func waitForUnityStartupMarkerOrTimeout(ctx context.Context, lockfilePath string } func unityLockfilePath(projectRoot string) string { - return filepath.Join(projectRoot, launchTempDirectoryName, unityLockfileName) + return unityprocess.LockfilePath(projectRoot) } func resolveLaunchProjectRoot(startPath string, options launchOptions) (string, error) { diff --git a/cli/dispatcher/shared-inputs-stamp.json b/cli/dispatcher/shared-inputs-stamp.json index 8f5f51c779..2b79f7d52a 100644 --- a/cli/dispatcher/shared-inputs-stamp.json +++ b/cli/dispatcher/shared-inputs-stamp.json @@ -1,4 +1,4 @@ { "schemaVersion": 1, - "sharedInputsHash": "55f3c34ad0b7085874015766a1782d91433445e2" + "sharedInputsHash": "bdd7c1afa781aafaa99f83d118fe6bd616ea7ce0" } diff --git a/cli/project-runner/internal/projectrunner/connection_retry.go b/cli/project-runner/internal/projectrunner/connection_retry.go index 3a523ee896..1bf0c7a5ee 100644 --- a/cli/project-runner/internal/projectrunner/connection_retry.go +++ b/cli/project-runner/internal/projectrunner/connection_retry.go @@ -33,7 +33,7 @@ type connectionRetryDeps struct { func defaultConnectionRetryDeps() connectionRetryDeps { return connectionRetryDeps{ - findRunningUnityProcess: unityprocess.FindRunningUnityProcess, + findRunningUnityProcess: unityProcessLookup, focusUnityProcess: unityprocess.FocusUnityProcessWithRestore, retryTimeout: serverConnectionRetryDefaultTimeout, retryPoll: serverConnectionRetryDefaultPoll, @@ -277,7 +277,7 @@ func sendWithTransientConnectionRetryWithDeps( continue } if !shouldRetryUndispatchedConnection(err, outcome) { - return finishNonRetryableConnectionAttempt( + finalOutcome, finalErr := finishNonRetryableConnectionAttempt( ctx, connection, sendAttempt{ @@ -291,6 +291,7 @@ func sendWithTransientConnectionRetryWithDeps( responseTimeout, focusController, ) + return finalOutcome, reportUnityExitDuringRequest(ctx, connection, finalOutcome, finalErr, deps) } // A caller that cancelled the command gets that cancellation back, as everywhere else that diff --git a/cli/project-runner/internal/projectrunner/connection_retry_flow.go b/cli/project-runner/internal/projectrunner/connection_retry_flow.go index ce248030fa..7b2373975a 100644 --- a/cli/project-runner/internal/projectrunner/connection_retry_flow.go +++ b/cli/project-runner/internal/projectrunner/connection_retry_flow.go @@ -101,16 +101,36 @@ func finishNonRetryableConnectionAttempt( return currentAttempt.outcome, currentAttempt.err } +// reportUnityExitDuringRequest turns a connection that dropped after the request reached Unity into +// UnityNotRunningError when a process lookup finds no Editor, and returns err unchanged otherwise. +// Why one lookup and no wait: an Editor still listed while it tears down keeps the disconnect, and the +// caller's next command then fails before dispatch, where the same lookup reports the exit. +func reportUnityExitDuringRequest( + ctx context.Context, + connection unityipc.Connection, + outcome unityipc.UnitySendOutcome, + err error, + deps connectionRetryDeps, +) error { + if ctx.Err() != nil || !outcome.RequestDispatched || !clierrors.IsTransportDisconnectError(err) { + return err + } + runningProcess, processErr := deps.findRunningUnityProcess(ctx, connection.ProjectRoot) + if processErr != nil || runningProcess != nil { + return err + } + return clicore.NewUnityNotRunningError(connection.ProjectRoot, connection.Endpoint.Address, err, true) +} + type sendAttempt struct { outcome unityipc.UnitySendOutcome err error } // finishUndispatchedRetryProbe decides whether the retry loop keeps waiting after a dial that -// never reached Unity. The process probe's only job here is to promote the diagnosis from "not -// reachable" to "running but not responding", so a probe that failed cannot promote anything: it -// observed no process, and claiming one would be an assertion this code has no evidence for. -// Both probe outcomes therefore report the dial error the caller can act on, and the probe +// never reached Unity. A probe that found the process keeps the loop waiting for its server, and a +// probe that answered without one reports that no Editor is running. A probe that failed observed +// nothing, so it can claim neither: it reports the dial error the caller can act on, and the probe // failure goes to the CLI vibe log instead of replacing that error. func finishUndispatchedRetryProbe( retryContext context.Context, @@ -126,6 +146,14 @@ func finishUndispatchedRetryProbe( if runningProcess != nil { return false, currentAttempt.outcome, nil } + if processErr == nil { + return true, currentAttempt.outcome, clicore.NewUnityNotRunningError( + connection.ProjectRoot, + connection.Endpoint.Address, + currentAttempt.err, + false, + ) + } // A busy response seen during the window proves a server answered moments ago, so it is a // truer diagnosis than a final dial cut short by the expiring retry context. if retryContext.Err() != nil && isUnityServerBusyRPCError(lastAttempt.err) { diff --git a/cli/project-runner/internal/projectrunner/connection_retry_flow_test.go b/cli/project-runner/internal/projectrunner/connection_retry_flow_test.go index cfb2b7d3e7..fb1cbc70f2 100644 --- a/cli/project-runner/internal/projectrunner/connection_retry_flow_test.go +++ b/cli/project-runner/internal/projectrunner/connection_retry_flow_test.go @@ -146,6 +146,47 @@ func TestFinishUndispatchedRetryProbeContinuesWhenUnityIsRunning(t *testing.T) { } } +// Verifies a probe that answered without a process ends the retry loop with UnityNotRunningError +// instead of the dial error, whose guidance tells the caller to wait for an Editor that is not there. +// Why a busy answer earlier in the window does not win: the server that answered it is gone now. +func TestFinishUndispatchedRetryProbeReportsUnityNotRunningWhenNoProcessIsFound(t *testing.T) { + busyAttempt := sendAttempt{ + err: &unityipc.RPCError{ + Code: -32603, + Message: "Unity is busy running 'compile'.", + Data: []byte(`{"type":"server_busy"}`), + }, + } + connection := unityipc.Connection{ + ProjectRoot: t.TempDir(), + Endpoint: unityipc.Endpoint{Address: "/tmp/uloop/sample.sock"}, + } + currentAttempt := refusedDialAttempt() + + finished, _, err := finishUndispatchedRetryProbe( + expiredRetryContext(), + connection, + currentAttempt, + nil, + nil, + busyAttempt, + ) + + if !finished { + t.Fatal("expected the retry loop to finish when no Unity process is running") + } + var notRunning clierrors.UnityNotRunningError + if !errors.As(err, ¬Running) { + t.Fatalf("expected UnityNotRunningError, got %v", err) + } + if notRunning.ExitedDuringCommand { + t.Fatal("an undispatched request never reached Unity, so it cannot have exited during it") + } + if notRunning.Cause != currentAttempt.err || notRunning.Endpoint != connection.Endpoint.Address { + t.Fatalf("expected the dial error and endpoint to be kept, got %#v", notRunning) + } +} + // Verifies the swallowed probe failure is still recorded: dropping the diagnosis from the error // must not drop it from the diagnostics too. func TestFinishUndispatchedRetryProbeRecordsTheProbeFailureInTheVibeLog(t *testing.T) { @@ -510,3 +551,60 @@ func TestFinishUnityAliveRetryWaitWhenRetryContextEnds(t *testing.T) { t.Fatalf("finished=%v err=%v, want UnityServerNotRespondingError", finished, err) } } + +// Verifies a connection that dropped after the request reached Unity becomes UnityNotRunningError only +// when a lookup that answered finds no Editor, and stays the disconnect in every other case, so a domain +// reload still reaches the callers that wait it out. +func TestReportUnityExitDuringRequest(t *testing.T) { + dispatched := unityipc.UnitySendOutcome{RequestDispatched: true} + accepted := unityipc.UnitySendOutcome{RequestDispatched: true, RequestAccepted: true} + failedLookup := func(context.Context, string) (*unityprocess.UnityProcess, error) { + return nil, errors.New("sysctl kern.proc.all: operation not permitted") + } + cases := []struct { + name string + ctx context.Context + outcome unityipc.UnitySendOutcome + err error + lookup func(context.Context, string) (*unityprocess.UnityProcess, error) + wantExitErr bool + }{ + {name: "dropped before the accept, no Editor", outcome: dispatched, err: io.EOF, lookup: noUnityProcessLookup, wantExitErr: true}, + {name: "dropped after the accept, no Editor", outcome: accepted, err: io.ErrUnexpectedEOF, lookup: noUnityProcessLookup, wantExitErr: true}, + {name: "dropped, Editor running", outcome: accepted, err: io.EOF, lookup: runningUnityProcessLookup}, + {name: "dropped, lookup failed", outcome: accepted, err: io.EOF, lookup: failedLookup}, + {name: "dropped before dispatch", outcome: unityipc.UnitySendOutcome{}, err: io.EOF, lookup: noUnityProcessLookup}, + {name: "caller cancelled", ctx: cancelledContext(), outcome: accepted, err: io.EOF, lookup: noUnityProcessLookup}, + {name: "not a disconnect", outcome: accepted, err: os.ErrDeadlineExceeded, lookup: noUnityProcessLookup}, + } + for _, testCase := range cases { + t.Run(testCase.name, func(t *testing.T) { + ctx := testCase.ctx + if ctx == nil { + ctx = context.Background() + } + deps := defaultConnectionRetryDeps() + deps.findRunningUnityProcess = testCase.lookup + connection := unityipc.Connection{ + ProjectRoot: t.TempDir(), + Endpoint: unityipc.Endpoint{Address: "/tmp/uloop/sample.sock"}, + } + + err := reportUnityExitDuringRequest(ctx, connection, testCase.outcome, testCase.err, deps) + + var notRunning clierrors.UnityNotRunningError + if !testCase.wantExitErr { + if err != testCase.err { + t.Fatalf("err = %v, want the original error %v", err, testCase.err) + } + return + } + if !errors.As(err, ¬Running) { + t.Fatalf("err = %v, want UnityNotRunningError", err) + } + if !notRunning.ExitedDuringCommand || notRunning.Cause != testCase.err { + t.Fatalf("expected an exit during the command caused by the disconnect, got %#v", notRunning) + } + }) + } +} diff --git a/cli/project-runner/internal/projectrunner/connection_retry_test.go b/cli/project-runner/internal/projectrunner/connection_retry_test.go index f6d20556e7..23397e7d9c 100644 --- a/cli/project-runner/internal/projectrunner/connection_retry_test.go +++ b/cli/project-runner/internal/projectrunner/connection_retry_test.go @@ -1685,9 +1685,8 @@ func TestSendWithTransientConnectionRetrySurfacesADroppedConnectionAfterBusy(t * deps := defaultConnectionRetryDeps() deps.retryTimeout = 150 * time.Millisecond deps.retryPoll = 5 * time.Millisecond - deps.findRunningUnityProcess = func(context.Context, string) (*clicore.UnityProcess, error) { - return nil, nil - } + // The Editor is still running: the connection dropped for a domain reload, not an exit. + deps.findRunningUnityProcess = runningUnityProcessLookup connection := busyFirstServerConnection(t, func(conn net.Conn) { accepted := `{"jsonrpc":"2.0","result":{"accepted":true},"uloop":{"phase":"accepted"},"id":1}` _ = unityipc.Write(conn, []byte(accepted)) diff --git a/cli/project-runner/internal/projectrunner/control_play_mode_wait_test.go b/cli/project-runner/internal/projectrunner/control_play_mode_wait_test.go index 85700b080d..dc5c31c012 100644 --- a/cli/project-runner/internal/projectrunner/control_play_mode_wait_test.go +++ b/cli/project-runner/internal/projectrunner/control_play_mode_wait_test.go @@ -887,6 +887,7 @@ func serveDroppedConnections(listener net.Listener, onSecondAccept func()) { // Verifies a disconnect during the action request moves on to the state wait, and the caller's // context ending during that wait is reported as a failure. func TestRunControlPlayModeWithStateWaitReportsContextEndDuringWait(t *testing.T) { + useRunningUnityProcess(t) originalPoll := controlPlayModeStatePoll controlPlayModeStatePoll = time.Millisecond t.Cleanup(func() { controlPlayModeStatePoll = originalPoll }) diff --git a/cli/project-runner/internal/projectrunner/hot_reload_editor_ready_retry_test.go b/cli/project-runner/internal/projectrunner/hot_reload_editor_ready_retry_test.go index a461ee5c88..19b653bee6 100644 --- a/cli/project-runner/internal/projectrunner/hot_reload_editor_ready_retry_test.go +++ b/cli/project-runner/internal/projectrunner/hot_reload_editor_ready_retry_test.go @@ -175,6 +175,7 @@ func runScriptedHotReload( compileResult compileExecutionResult, ) editorReadyRetryRun { t.Helper() + useRunningUnityProcess(t) listener := newLoopbackIpcListener(t) requests := make(chan scriptedIPCRequest, len(steps)+1) serverErr := make(chan error, len(steps)+1) diff --git a/cli/project-runner/internal/projectrunner/status_command.go b/cli/project-runner/internal/projectrunner/status_command.go index b819011be4..1016ba7db3 100644 --- a/cli/project-runner/internal/projectrunner/status_command.go +++ b/cli/project-runner/internal/projectrunner/status_command.go @@ -260,13 +260,24 @@ func reportFromUnreachableServer( "(uloop cannot run in Safe Mode).") report.UnityProcessRunning = &running default: - report = newStatusReport(statusStateNotRunning, "No Unity Editor is running for this project.") + report = notRunningStatusReport(projectRoot, probeErr) report.UnityProcessRunning = &running } report.ConnectionError = probeErr.Error() return report } +// notRunningStatusReport words NotRunning as uloop's not-running error does, so status and a failed +// command agree on whether the Editor shut down normally. +func notRunningStatusReport(projectRoot string, probeErr error) statusReport { + notRunning := clicore.NewUnityNotRunningError(projectRoot, "", probeErr, false) + report := newStatusReport(statusStateNotRunning, notRunning.ToCLIError(clierrors.ErrorContext{}).Message) + if notRunning.LockfileLeftBehind { + report.NextActions = append([]string{notRunning.EditorLogNextAction()}, report.NextActions...) + } + return report +} + func reportFromEditorResponse(response editorStatusResponse) statusReport { state := classifyEditorState(response) report := newStatusReport(state, editorStateMessage(state, response)) diff --git a/cli/project-runner/internal/projectrunner/status_command_test.go b/cli/project-runner/internal/projectrunner/status_command_test.go index fe3fbeb013..28b50cfe73 100644 --- a/cli/project-runner/internal/projectrunner/status_command_test.go +++ b/cli/project-runner/internal/projectrunner/status_command_test.go @@ -9,6 +9,7 @@ import ( "fmt" "net" "os" + "path/filepath" "reflect" "sort" "strings" @@ -16,6 +17,7 @@ import ( "time" "github.com/hatayama/unity-cli-loop/common/unityipc" + "github.com/hatayama/unity-cli-loop/common/unityprocess" ) const statusTestAcceptedResponse = `{"jsonrpc":"2.0","id":1,"uloop":{"phase":"accepted"}}` @@ -436,6 +438,39 @@ func TestRunStatusCommandReportsNotRunning(t *testing.T) { assertStatusConnectionError(t, report, "") } +// Verifies NotRunning says the Editor did not shut down normally when it left its lockfile behind, and +// points at the Editor log before the launch step, because the next launch renames that log. +func TestRunStatusCommandReportsAnAbnormalExitWhenTheLockfileWasLeftBehind(t *testing.T) { + projectRoot := writeFakeUnityProject(t) + lockfilePath := unityprocess.LockfilePath(projectRoot) + if err := os.MkdirAll(filepath.Dir(lockfilePath), 0o755); err != nil { + t.Fatalf("creating Temp failed: %v", err) + } + if err := os.WriteFile(lockfilePath, []byte{}, 0o644); err != nil { + t.Fatalf("writing the lockfile failed: %v", err) + } + deps := statusTestDeps(t) + deps.isUnityProcessRunning = fakeUnityProcessLookup(t, projectRoot, false, nil) + + run := runStatusForTest(context.Background(), unreachableConnection(projectRoot), nil, deps) + + report := assertStatusReport(t, run, "NotRunning") + message, _ := report["Message"].(string) + if !strings.HasPrefix(message, "No Unity Editor is running for this project.") || !strings.Contains(message, "may have crashed") { + t.Fatalf("Message = %q, want the abnormal-exit message", message) + } + nextActions, _ := report["NextActions"].([]any) + if len(nextActions) != 2 { + t.Fatalf("NextActions = %#v, want the Editor log step and the launch step", report["NextActions"]) + } + if first, _ := nextActions[0].(string); !strings.Contains(first, "Editor log") { + t.Fatalf("NextActions[0] = %q, want the Editor log step", first) + } + if second, _ := nextActions[1].(string); second != "Start it with uloop launch." { + t.Fatalf("NextActions[1] = %q, want the launch step", second) + } +} + // Verifies a refused connection whose process lookup fails is Unreachable, naming the lookup // error and leaving out the process flag it could not determine. func TestRunStatusCommandReportsUnreachableWhenProcessLookupFails(t *testing.T) { diff --git a/cli/project-runner/internal/projectrunner/unity_process_lookup.go b/cli/project-runner/internal/projectrunner/unity_process_lookup.go new file mode 100644 index 0000000000..61bf608396 --- /dev/null +++ b/cli/project-runner/internal/projectrunner/unity_process_lookup.go @@ -0,0 +1,11 @@ +package projectrunner + +import ( + "github.com/hatayama/unity-cli-loop/common/unityprocess" +) + +// unityProcessLookup is the lookup every default dependency set uses to find this project's Editor. +// Why a package variable: whole-command tests drive a scripted server through the default +// dependencies, and they must be able to say that an Editor process stands behind it. The tests of +// this package are not parallel. +var unityProcessLookup = unityprocess.FindRunningUnityProcess diff --git a/cli/project-runner/internal/projectrunner/unity_process_lookup_helpers_test.go b/cli/project-runner/internal/projectrunner/unity_process_lookup_helpers_test.go new file mode 100644 index 0000000000..046b36cff3 --- /dev/null +++ b/cli/project-runner/internal/projectrunner/unity_process_lookup_helpers_test.go @@ -0,0 +1,37 @@ +package projectrunner + +import ( + "context" + "os" + "testing" + + "github.com/hatayama/unity-cli-loop/common/unityprocess" +) + +// useUnityProcessLookup replaces the default Editor process lookup for one test. +func useUnityProcessLookup( + t *testing.T, + lookup func(context.Context, string) (*unityprocess.UnityProcess, error), +) { + t.Helper() + original := unityProcessLookup + unityProcessLookup = lookup + t.Cleanup(func() { + unityProcessLookup = original + }) +} + +// useRunningUnityProcess makes the default lookup find an Editor, as a scripted server stands in for one. +// Why this test process's own pid: a focus attempt on it can never raise another application's window. +func useRunningUnityProcess(t *testing.T) { + t.Helper() + useUnityProcessLookup(t, runningUnityProcessLookup) +} + +func runningUnityProcessLookup(context.Context, string) (*unityprocess.UnityProcess, error) { + return &unityprocess.UnityProcess{Pid: os.Getpid()}, nil +} + +func noUnityProcessLookup(context.Context, string) (*unityprocess.UnityProcess, error) { + return nil, nil +} diff --git a/cli/project-runner/shared-inputs-stamp.json b/cli/project-runner/shared-inputs-stamp.json index 639babeae7..d57fec769e 100644 --- a/cli/project-runner/shared-inputs-stamp.json +++ b/cli/project-runner/shared-inputs-stamp.json @@ -1,4 +1,4 @@ { "schemaVersion": 1, - "sharedInputsHash": "8b1946904c0c5107e5a7e2c96a4e896a46b0e959" + "sharedInputsHash": "9aebf20610dd0b92632ffe7d94412a40b91da890" } From 9004dfeca30eedf917a05d4aed72b9ff21adc817 Mon Sep 17 00:00:00 2001 From: hatayama Date: Tue, 27 Oct 2026 14:33:28 +0900 Subject: [PATCH 03/12] Stop waiting on Unity once its Editor process has exited When Unity crashed during a compile, launch, PlayMode change, hot reload, or pause point wait, the CLI kept polling a server that could not come back until its own timeout: ten minutes for compile, launch, and hot reload. The agent sat through that wait and then got a timeout that did not say Unity was gone. Each of those waits now feeds its poll errors to a UnityExitWatch, which looks up the Editor process while the server stays gone and ends the wait with UNITY_NOT_RUNNING once two lookups at least two seconds apart both find no Editor. Why two spaced lookups: an Editor that is tearing down can still be listed for a moment, and one empty listing is thin evidence for ending a ten-minute wait. A failed lookup, a successful poll, or an error a live server also produces restarts the confirmation, and lookups are throttled to one per interval so a 50ms pause point poll does not list processes on every poll. --- cli/common/clicore/tool_readiness.go | 8 + cli/common/clicore/tool_readiness_test.go | 26 +++ cli/common/clicore/unity_exit.go | 69 ++++++++ cli/common/clicore/unity_exit_test.go | 166 ++++++++++++++++++ cli/dispatcher/shared-inputs-stamp.json | 2 +- .../internal/projectrunner/compile_attach.go | 4 + .../internal/projectrunner/compile_wait.go | 4 + .../projectrunner/compile_wait_deps.go | 24 ++- .../projectrunner/compile_wait_test.go | 2 + .../compile_wait_unity_exit_test.go | 115 ++++++++++++ .../projectrunner/control_play_mode_wait.go | 6 +- .../control_play_mode_wait_test.go | 5 +- .../projectrunner/hot_reload_busy_wait.go | 10 +- .../hot_reload_editor_ready_retry.go | 32 ++-- .../projectrunner/pause_point_wait_poll.go | 13 +- .../projectrunner/unity_exit_waits_test.go | 106 +++++++++++ .../projectrunner/unity_process_lookup.go | 8 + cli/project-runner/shared-inputs-stamp.json | 2 +- 18 files changed, 573 insertions(+), 29 deletions(-) create mode 100644 cli/project-runner/internal/projectrunner/compile_wait_unity_exit_test.go create mode 100644 cli/project-runner/internal/projectrunner/unity_exit_waits_test.go diff --git a/cli/common/clicore/tool_readiness.go b/cli/common/clicore/tool_readiness.go index f31a5dcc6b..28a6ceca01 100644 --- a/cli/common/clicore/tool_readiness.go +++ b/cli/common/clicore/tool_readiness.go @@ -28,6 +28,8 @@ const executeDynamicCodeReadinessProbe = `return "Unity CLI Loop dynamic code pr type toolReadinessDeps struct { findRunningUnityProcess func(context.Context, string) (*unityprocess.UnityProcess, error) probeToolReadinessSequence func(context.Context, string) error + // now is the clock the Unity exit watch spaces its lookups with; nil uses time.Now. + now func() time.Time } func WaitForToolReadiness(ctx context.Context, projectRoot string) error { @@ -52,6 +54,7 @@ func waitForToolReadinessWithDeps(ctx context.Context, projectRoot string, timeo defer cancel() var lastErr error + exitWatch := NewUnityExitWatch(projectRoot, resolveProjectEndpointAddress(projectRoot), deps.findRunningUnityProcess, deps.now) ticker := time.NewTicker(ToolReadinessPoll) defer ticker.Stop() for { @@ -67,6 +70,11 @@ func waitForToolReadinessWithDeps(ctx context.Context, projectRoot string, timeo if clierrors.IsPermanentConnectError(err) { return err } + // Why abort: an Editor that exited cannot become ready, and launch would otherwise wait + // its whole ten minutes for it. + if exitErr := exitWatch.Observe(timeoutContext, err); exitErr != nil { + return exitErr + } lastErr = err } diff --git a/cli/common/clicore/tool_readiness_test.go b/cli/common/clicore/tool_readiness_test.go index ee4f1a10fe..be503cbb85 100644 --- a/cli/common/clicore/tool_readiness_test.go +++ b/cli/common/clicore/tool_readiness_test.go @@ -264,3 +264,29 @@ func assertConnectionAttemptToProject(t *testing.T, err error, projectRoot strin t.Fatalf("connection endpoint = %q, want %q", attemptErr.Endpoint, expected) } } + +// Verifies a readiness wait whose Editor exited ends as soon as the exit is confirmed, instead of +// polling a server that cannot come back until the wait times out. +func TestWaitForToolReadinessStopsOnceUnityHasExited(t *testing.T) { + clock := time.Unix(1_700_000_000, 0) + deps := toolReadinessDeps{ + probeToolReadinessSequence: func(context.Context, string) error { + return &unityipc.ConnectionAttemptError{Cause: syscall.ECONNREFUSED} + }, + findRunningUnityProcess: func(context.Context, string) (*UnityProcess, error) { + return nil, nil + }, + // Each reading moves a full confirm interval, so the second poll can already confirm. + now: func() time.Time { + clock = clock.Add(unityExitConfirmInterval) + return clock + }, + } + + err := waitForToolReadinessWithDeps(context.Background(), t.TempDir(), time.Hour, deps) + + var notRunning clierrors.UnityNotRunningError + if !errors.As(err, ¬Running) { + t.Fatalf("expected UnityNotRunningError, got %v", err) + } +} diff --git a/cli/common/clicore/unity_exit.go b/cli/common/clicore/unity_exit.go index 343f2715c1..09a1708017 100644 --- a/cli/common/clicore/unity_exit.go +++ b/cli/common/clicore/unity_exit.go @@ -1,12 +1,22 @@ package clicore import ( + "context" "os" + "time" clierrors "github.com/hatayama/unity-cli-loop/common/errors" "github.com/hatayama/unity-cli-loop/common/unityprocess" ) +const ( + // Why spaced lookups: an Editor that is tearing down can still be listed for a moment after its + // server closed, and one empty listing is thin evidence for ending a ten-minute wait. Two lookups this + // far apart that both find nothing are not a transient. + unityExitConfirmInterval = 2 * time.Second + unityExitConfirmations = 2 +) + // NewUnityNotRunningError describes a project whose Unity Editor a successful process lookup did not // find, noting whether the Editor left its lockfile behind. exitedDuringCommand says the command had // reached Unity before it went away. @@ -32,3 +42,62 @@ func lockfileLeftBehind(projectRoot string) bool { _, err := os.Stat(unityprocess.LockfilePath(projectRoot)) return err == nil } + +// UnityExitWatch tells a wait loop that keeps losing its connection to Unity when the Editor process +// for the project has exited, so the loop can stop instead of polling until its own timeout. +type UnityExitWatch struct { + projectRoot string + endpoint string + findProcess func(context.Context, string) (*unityprocess.UnityProcess, error) + now func() time.Time + lastLookupAt time.Time + goneLookups int +} + +// NewUnityExitWatch creates a watch for the project at projectRoot. findProcess is the process lookup and +// now the clock; a nil now uses time.Now. +func NewUnityExitWatch( + projectRoot string, + endpoint string, + findProcess func(context.Context, string) (*unityprocess.UnityProcess, error), + now func() time.Time, +) *UnityExitWatch { + if now == nil { + now = time.Now + } + return &UnityExitWatch{ + projectRoot: projectRoot, + endpoint: endpoint, + findProcess: findProcess, + now: now, + } +} + +// Observe takes the error of one poll, nil for a poll that succeeded. It returns a +// clierrors.UnityNotRunningError once polls that found the server gone were followed by two lookups, at +// least unityExitConfirmInterval apart, that both found no Editor process; otherwise it returns nil. +func (watch *UnityExitWatch) Observe(ctx context.Context, pollErr error) error { + // Any other outcome, a success included, shows a server that may be alive, so the lookups that + // found nothing so far no longer belong to one exit. + if !clierrors.IsUnityServerGoneError(pollErr) { + watch.goneLookups = 0 + return nil + } + now := watch.now() + if !watch.lastLookupAt.IsZero() && now.Sub(watch.lastLookupAt) < unityExitConfirmInterval { + return nil + } + watch.lastLookupAt = now + + process, err := watch.findProcess(ctx, watch.projectRoot) + // A lookup that failed observed nothing, so it cannot count toward an exit. + if err != nil || process != nil { + watch.goneLookups = 0 + return nil + } + watch.goneLookups++ + if watch.goneLookups < unityExitConfirmations { + return nil + } + return NewUnityNotRunningError(watch.projectRoot, watch.endpoint, pollErr, true) +} diff --git a/cli/common/clicore/unity_exit_test.go b/cli/common/clicore/unity_exit_test.go index 21a62d3f51..08ec73d40e 100644 --- a/cli/common/clicore/unity_exit_test.go +++ b/cli/common/clicore/unity_exit_test.go @@ -1,14 +1,180 @@ package clicore import ( + "context" + "errors" "io" "os" "path/filepath" + "syscall" "testing" + "time" + clierrors "github.com/hatayama/unity-cli-loop/common/errors" + "github.com/hatayama/unity-cli-loop/common/unityipc" "github.com/hatayama/unity-cli-loop/common/unityprocess" ) +// unityExitWatchFixture drives a UnityExitWatch with a hand-moved clock and a scripted process lookup. +type unityExitWatchFixture struct { + now time.Time + running bool + lookErr error + lookups int + watch *UnityExitWatch +} + +func newUnityExitWatchFixture(t *testing.T) *unityExitWatchFixture { + t.Helper() + fixture := &unityExitWatchFixture{now: time.Unix(1_700_000_000, 0), running: true} + fixture.watch = NewUnityExitWatch( + t.TempDir(), + "/tmp/uloop/sample.sock", + func(context.Context, string) (*unityprocess.UnityProcess, error) { + fixture.lookups++ + if fixture.lookErr != nil { + return nil, fixture.lookErr + } + if fixture.running { + return &unityprocess.UnityProcess{Pid: 4321}, nil + } + return nil, nil + }, + func() time.Time { return fixture.now }, + ) + return fixture +} + +func (fixture *unityExitWatchFixture) observe(pollErr error) error { + return fixture.watch.Observe(context.Background(), pollErr) +} + +func (fixture *unityExitWatchFixture) advance(duration time.Duration) { + fixture.now = fixture.now.Add(duration) +} + +func serverGonePollError() error { + return &unityipc.ConnectionAttemptError{Cause: syscall.ECONNREFUSED} +} + +// Verifies the watch reports Unity as gone only after two lookups, spaced at least the confirm +// interval apart, both find no process, and that the report says Unity went away during the command. +func TestUnityExitWatchReportsAfterTwoSpacedLookupsFindNoProcess(t *testing.T) { + fixture := newUnityExitWatchFixture(t) + fixture.running = false + + if err := fixture.observe(serverGonePollError()); err != nil { + t.Fatalf("one lookup must not be enough, got %v", err) + } + fixture.advance(unityExitConfirmInterval - time.Millisecond) + if err := fixture.observe(serverGonePollError()); err != nil { + t.Fatalf("a poll inside the confirm interval must not confirm, got %v", err) + } + fixture.advance(time.Millisecond) + err := fixture.observe(serverGonePollError()) + + var notRunning clierrors.UnityNotRunningError + if !errors.As(err, ¬Running) { + t.Fatalf("expected UnityNotRunningError after the confirming lookup, got %v", err) + } + if !notRunning.ExitedDuringCommand { + t.Fatal("a wait loop's report must say Unity went away during the command") + } + if fixture.lookups != 2 { + t.Fatalf("lookups = %d, want 2 (the poll inside the interval must not look up)", fixture.lookups) + } +} + +// Verifies a running Editor is looked up at most once per confirm interval, so a wait that polls every +// few milliseconds through a long domain reload does not list processes on every poll. +func TestUnityExitWatchThrottlesLookupsWhileUnityRuns(t *testing.T) { + fixture := newUnityExitWatchFixture(t) + + for poll := 0; poll < 10; poll++ { + if err := fixture.observe(serverGonePollError()); err != nil { + t.Fatalf("a running Editor must never be reported as gone, got %v", err) + } + fixture.advance(unityExitConfirmInterval / 4) + } + + if fixture.lookups != 3 { + t.Fatalf("lookups = %d, want 3 over 2.5 confirm intervals", fixture.lookups) + } +} + +// Verifies each kind of evidence that the Editor may still be there restarts the confirmation: a process +// found, a lookup that failed, a poll that succeeded, and a poll error that a live server also produces. +func TestUnityExitWatchRestartsTheConfirmationOnEvidenceOfALiveEditor(t *testing.T) { + cases := []struct { + name string + interrupt func(fixture *unityExitWatchFixture) error + wantLookup bool + }{ + { + name: "process found", + interrupt: func(fixture *unityExitWatchFixture) error { + fixture.running = true + defer func() { fixture.running = false }() + return fixture.observe(serverGonePollError()) + }, + }, + { + name: "lookup failed", + interrupt: func(fixture *unityExitWatchFixture) error { + fixture.lookErr = errors.New("sysctl kern.proc.all: operation not permitted") + defer func() { fixture.lookErr = nil }() + return fixture.observe(serverGonePollError()) + }, + }, + { + name: "poll succeeded", + interrupt: func(fixture *unityExitWatchFixture) error { + return fixture.observe(nil) + }, + }, + { + name: "dial timed out", + interrupt: func(fixture *unityExitWatchFixture) error { + return fixture.observe(&unityipc.ConnectionAttemptError{Cause: os.ErrDeadlineExceeded}) + }, + }, + } + + for _, testCase := range cases { + t.Run(testCase.name, func(t *testing.T) { + fixture := newUnityExitWatchFixture(t) + fixture.running = false + if err := fixture.observe(serverGonePollError()); err != nil { + t.Fatalf("one lookup must not be enough, got %v", err) + } + fixture.advance(unityExitConfirmInterval) + if err := testCase.interrupt(fixture); err != nil { + t.Fatalf("the interruption must not report Unity as gone, got %v", err) + } + fixture.advance(unityExitConfirmInterval) + if err := fixture.observe(serverGonePollError()); err != nil { + t.Fatalf("the lookup after the interruption must start a new confirmation, got %v", err) + } + fixture.advance(unityExitConfirmInterval) + if err := fixture.observe(serverGonePollError()); err == nil { + t.Fatal("two spaced lookups after the interruption must report Unity as gone") + } + }) + } +} + +// Verifies a dropped connection counts as the server going away, the same as a dial nobody answered. +func TestUnityExitWatchTreatsADroppedConnectionAsTheServerGoingAway(t *testing.T) { + fixture := newUnityExitWatchFixture(t) + fixture.running = false + + _ = fixture.observe(io.EOF) + fixture.advance(unityExitConfirmInterval) + if err := fixture.observe(io.EOF); err == nil { + t.Fatal("two spaced lookups after dropped connections must report Unity as gone") + } +} + // Verifies the not-running error records a lockfile left behind, which only a shutdown that did not // finish leaves, along with the Editor log location. func TestNewUnityNotRunningErrorRecordsALockfileLeftBehind(t *testing.T) { diff --git a/cli/dispatcher/shared-inputs-stamp.json b/cli/dispatcher/shared-inputs-stamp.json index 2b79f7d52a..d58506c1de 100644 --- a/cli/dispatcher/shared-inputs-stamp.json +++ b/cli/dispatcher/shared-inputs-stamp.json @@ -1,4 +1,4 @@ { "schemaVersion": 1, - "sharedInputsHash": "bdd7c1afa781aafaa99f83d118fe6bd616ea7ce0" + "sharedInputsHash": "bb1095cefcb0afeb776b946a08071b49ebc46477" } diff --git a/cli/project-runner/internal/projectrunner/compile_attach.go b/cli/project-runner/internal/projectrunner/compile_attach.go index 078aacd1ea..2cf100bc45 100644 --- a/cli/project-runner/internal/projectrunner/compile_attach.go +++ b/cli/project-runner/internal/projectrunner/compile_attach.go @@ -243,6 +243,7 @@ func waitForAttachedCompileCompletion( logCompileStatusPollStart(options, startedAt, deadline) interim := newCompileWaitInterimState(compileWaitNow(deps)) + exitWatch := newCompileUnityExitWatch(options.connection, deps) ticker := time.NewTicker(options.pollInterval) defer ticker.Stop() @@ -277,6 +278,9 @@ func waitForAttachedCompileCompletion( } } logCompileStatusPollObservedIfChanged(options, startedAt, attempts, status, err, &lastObservationKey) + if exitErr := exitWatch.Observe(ctx, err); exitErr != nil { + return nil, attachWaitFailed, lastObservedCompileStatus(lastStatus, observedStatus), exitErr + } observeCompileWaitInterim(&interim, deps, status, err) select { diff --git a/cli/project-runner/internal/projectrunner/compile_wait.go b/cli/project-runner/internal/projectrunner/compile_wait.go index 3c2a43fc5a..59e3f6c879 100644 --- a/cli/project-runner/internal/projectrunner/compile_wait.go +++ b/cli/project-runner/internal/projectrunner/compile_wait.go @@ -195,6 +195,7 @@ func waitForCompileCompletionWithDeps( logCompileStatusPollStart(options, startedAt, deadline) interim := newCompileWaitInterimState(compileWaitNow(deps)) missing := compileRequestMissingTracker{serverRestartSeen: options.serverRestartSeen} + exitWatch := newCompileUnityExitWatch(options.connection, deps) ticker := time.NewTicker(options.pollInterval) defer ticker.Stop() @@ -224,6 +225,9 @@ func waitForCompileCompletionWithDeps( return nil, false, lastObservedCompileStatus(lastStatus, observedStatus), errCompileRequestMissing } logCompileStatusPollObservedIfChanged(options, startedAt, attempts, status, err, &lastObservationKey) + if exitErr := exitWatch.Observe(ctx, err); exitErr != nil { + return nil, false, lastObservedCompileStatus(lastStatus, observedStatus), exitErr + } observeCompileWaitInterim(&interim, deps, status, err) select { diff --git a/cli/project-runner/internal/projectrunner/compile_wait_deps.go b/cli/project-runner/internal/projectrunner/compile_wait_deps.go index 959b6807a3..9e5d8c6cd0 100644 --- a/cli/project-runner/internal/projectrunner/compile_wait_deps.go +++ b/cli/project-runner/internal/projectrunner/compile_wait_deps.go @@ -4,7 +4,9 @@ import ( "context" "time" + "github.com/hatayama/unity-cli-loop/common/clicore" "github.com/hatayama/unity-cli-loop/common/unityipc" + "github.com/hatayama/unity-cli-loop/common/unityprocess" ) type compileSendFunc func( @@ -28,6 +30,19 @@ type compileWaitDeps struct { // Zero keeps compileWaitPollInterval for a fresh compile's status wait. Tests shorten it so // they do not wait 1s between status queries. freshWaitPollInterval time.Duration + // findRunningUnityProcess is the Editor process lookup a status wait uses to notice that Unity + // exited. + findRunningUnityProcess func(context.Context, string) (*unityprocess.UnityProcess, error) +} + +// newCompileUnityExitWatch creates the exit watch for one compile status wait, on the wait's clock. +func newCompileUnityExitWatch(connection unityipc.Connection, deps compileWaitDeps) *clicore.UnityExitWatch { + return clicore.NewUnityExitWatch( + connection.ProjectRoot, + connection.Endpoint.Address, + deps.findRunningUnityProcess, + func() time.Time { return compileWaitNow(deps) }, + ) } func freshWaitPollIntervalFor(deps compileWaitDeps) time.Duration { @@ -46,9 +61,10 @@ func compileSendOrDefault(deps compileWaitDeps) compileSendFunc { func defaultCompileWaitDeps() compileWaitDeps { return compileWaitDeps{ - queryCompileStatus: queryCompileStatusFromUnity, - attachProbeTimeout: compileAttachProbeTimeout, - attachProbeInterval: compileAttachProbeInterval, - attachWaitPollInterval: compileWaitPollInterval, + queryCompileStatus: queryCompileStatusFromUnity, + attachProbeTimeout: compileAttachProbeTimeout, + attachProbeInterval: compileAttachProbeInterval, + attachWaitPollInterval: compileWaitPollInterval, + findRunningUnityProcess: unityProcessLookup, } } diff --git a/cli/project-runner/internal/projectrunner/compile_wait_test.go b/cli/project-runner/internal/projectrunner/compile_wait_test.go index bf44801812..5c90ceccf0 100644 --- a/cli/project-runner/internal/projectrunner/compile_wait_test.go +++ b/cli/project-runner/internal/projectrunner/compile_wait_test.go @@ -1055,6 +1055,8 @@ func compileWaitTestDeps( attachProbeTimeout: 40 * time.Millisecond, attachProbeInterval: 5 * time.Millisecond, attachWaitPollInterval: 5 * time.Millisecond, + // The scripted status answers stand in for a running Editor, whose server a reload takes down. + findRunningUnityProcess: runningUnityProcessLookup, } } diff --git a/cli/project-runner/internal/projectrunner/compile_wait_unity_exit_test.go b/cli/project-runner/internal/projectrunner/compile_wait_unity_exit_test.go new file mode 100644 index 0000000000..d059eaf92d --- /dev/null +++ b/cli/project-runner/internal/projectrunner/compile_wait_unity_exit_test.go @@ -0,0 +1,115 @@ +package projectrunner + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "syscall" + "testing" + "time" + + clierrors "github.com/hatayama/unity-cli-loop/common/errors" + "github.com/hatayama/unity-cli-loop/common/unityipc" +) + +// exitedEditorCompileDeps answers every status query as a server nobody listens on, finds no Editor +// process, and moves the wait's clock a second on every reading so lookups are spaced without sleeping. +func exitedEditorCompileDeps() compileWaitDeps { + clock := time.Unix(1_700_000_000, 0) + deps := compileWaitTestDeps(func(context.Context, unityipc.Connection, string) (compileStatusResponse, error) { + return compileStatusResponse{}, &unityipc.ConnectionAttemptError{Cause: syscall.ECONNREFUSED} + }) + deps.findRunningUnityProcess = noUnityProcessLookup + deps.now = func() time.Time { + clock = clock.Add(time.Second) + return clock + } + return deps +} + +func assertUnityExitedDuringCommand(t *testing.T, err error) { + t.Helper() + var notRunning clierrors.UnityNotRunningError + if !errors.As(err, ¬Running) { + t.Fatalf("err = %v, want UnityNotRunningError", err) + } + if !notRunning.ExitedDuringCommand { + t.Fatalf("expected the exit to be reported as during the command, got %#v", notRunning) + } +} + +// Verifies a fresh compile's status wait ends with UnityNotRunningError once the Editor is confirmed +// gone, instead of polling a server that cannot come back for the whole compile timeout. +func TestWaitForCompileCompletionStopsOnceUnityHasExited(t *testing.T) { + _, completed, _, err := waitForCompileCompletionWithDeps(context.Background(), compileCompletionOptions{ + connection: compileWaitTestConnection(t), + requestID: "compile_unity_exited", + // Long enough that only the exit watch, never the timeout, can end the wait first. + timeout: 5 * time.Second, + pollInterval: time.Millisecond, + }, exitedEditorCompileDeps()) + + if completed { + t.Fatal("a compile whose Editor exited cannot complete") + } + assertUnityExitedDuringCommand(t, err) +} + +// Verifies the attach wait stops the same way when the Editor running the earlier compile is gone. +func TestWaitForAttachedCompileCompletionStopsOnceUnityHasExited(t *testing.T) { + _, outcome, _, err := waitForAttachedCompileCompletion(context.Background(), compileCompletionOptions{ + connection: compileWaitTestConnection(t), + requestID: "compile_attach_unity_exited", + // Long enough that only the exit watch, never the timeout, can end the wait first. + timeout: 5 * time.Second, + pollInterval: time.Millisecond, + }, exitedEditorCompileDeps()) + + if outcome != attachWaitFailed { + t.Fatalf("outcome = %v, want attachWaitFailed", outcome) + } + assertUnityExitedDuringCommand(t, err) +} + +// Verifies uloop compile prints the not-running envelope when the Editor exits during the domain reload +// wait, so the caller is told to launch Unity rather than to wait. +func TestRunCompileReportsUnityNotRunningWhenTheEditorExitsDuringTheWait(t *testing.T) { + deps := exitedEditorCompileDeps() + deps.freshWaitPollInterval = time.Millisecond + deps.sendCompile = func( + context.Context, + unityipc.Connection, + string, + map[string]any, + unityipc.ProgressFunc, + time.Duration, + ) (unityipc.UnitySendOutcome, error) { + return unityipc.UnitySendOutcome{RequestDispatched: true, RequestAccepted: true}, errors.New("read: EOF") + } + var stdout, stderr bytes.Buffer + + code := runCompileWithDomainReloadWaitWithDeps( + context.Background(), + compileWaitTestConnection(t), + map[string]any{}, + &stdout, + &stderr, + deps, + ) + + if code != 1 || stdout.Len() != 0 { + t.Fatalf("code = %d, stdout = %q, want 1 and nothing", code, stdout.String()) + } + jsonStart := bytes.IndexByte(stderr.Bytes(), '{') + if jsonStart < 0 { + t.Fatalf("stderr has no JSON envelope: %s", stderr.String()) + } + var envelope clierrors.CLIErrorEnvelope + if err := json.Unmarshal(stderr.Bytes()[jsonStart:], &envelope); err != nil { + t.Fatalf("decoding stderr failed: %v\n%s", err, stderr.String()) + } + if envelope.Error.ErrorCode != clierrors.ErrorCodeUnityNotRunning { + t.Fatalf("ErrorCode = %q, want %q\n%s", envelope.Error.ErrorCode, clierrors.ErrorCodeUnityNotRunning, stderr.String()) + } +} diff --git a/cli/project-runner/internal/projectrunner/control_play_mode_wait.go b/cli/project-runner/internal/projectrunner/control_play_mode_wait.go index 31f15da437..84a9cade36 100644 --- a/cli/project-runner/internal/projectrunner/control_play_mode_wait.go +++ b/cli/project-runner/internal/projectrunner/control_play_mode_wait.go @@ -120,7 +120,7 @@ func runControlPlayModeWithStateWait( } spinner.Update("Waiting for play mode state...") - response, completed, waitErr := waitForControlPlayModeState(ctx, connection, action, timeout) + response, completed, waitErr := waitForControlPlayModeState(ctx, connection, action, timeout, newUnityExitWatch(connection)) spinner.Stop() if waitErr != nil { writeDebugTiming(stderr, controlPlayModeCommandName, time.Since(startedAt), outcome) @@ -168,6 +168,7 @@ func waitForControlPlayModeState( connection unityipc.Connection, action string, timeout time.Duration, + exitWatch *clicore.UnityExitWatch, ) (controlPlayModeResponse, bool, error) { waitContext, cancel := context.WithTimeout(ctx, timeout) defer cancel() @@ -190,6 +191,9 @@ func waitForControlPlayModeState( } } else { lastErr = err + if exitErr := exitWatch.Observe(waitContext, err); exitErr != nil { + return lastResponse, false, exitErr + } } select { diff --git a/cli/project-runner/internal/projectrunner/control_play_mode_wait_test.go b/cli/project-runner/internal/projectrunner/control_play_mode_wait_test.go index dc5c31c012..6a6eda3d3c 100644 --- a/cli/project-runner/internal/projectrunner/control_play_mode_wait_test.go +++ b/cli/project-runner/internal/projectrunner/control_play_mode_wait_test.go @@ -13,6 +13,7 @@ import ( "testing" "time" + "github.com/hatayama/unity-cli-loop/common/clicore" clierrors "github.com/hatayama/unity-cli-loop/common/errors" "github.com/hatayama/unity-cli-loop/common/unityipc" @@ -925,11 +926,13 @@ func TestWaitForControlPlayModeStateTimesOutWithLastError(t *testing.T) { controlPlayModeStatePoll = time.Millisecond t.Cleanup(func() { controlPlayModeStatePoll = originalPoll }) + connection := unreachableConnection(t.TempDir()) _, completed, err := waitForControlPlayModeState( context.Background(), - unreachableConnection(t.TempDir()), + connection, "Play", 20*time.Millisecond, + clicore.NewUnityExitWatch(connection.ProjectRoot, "", runningUnityProcessLookup, nil), ) if completed || err == nil { diff --git a/cli/project-runner/internal/projectrunner/hot_reload_busy_wait.go b/cli/project-runner/internal/projectrunner/hot_reload_busy_wait.go index 5a41e201cf..92d221e6c6 100644 --- a/cli/project-runner/internal/projectrunner/hot_reload_busy_wait.go +++ b/cli/project-runner/internal/projectrunner/hot_reload_busy_wait.go @@ -67,7 +67,7 @@ func sendHotReloadWaitingForBusyEditor( logHotReloadBusyWaitDecided(connection, first.correlationID, err) // A plain line for the same reason as the settle wait's: the spinner's Stop erases its own line. _, _ = fmt.Fprintf(stderr, hotReloadBusyWaitingLineFormat+"\n", runningToolName) - wait, waitErr := waitForBusyEditor(ctx, connection, params, stderr, deps, hotReloadEditorReadyWaitDefaults) + wait, waitErr := waitForBusyEditor(ctx, connection, params, stderr, deps, hotReloadEditorReadyWaitDefaults, newUnityExitWatch(connection)) if waitErr != nil { writeHotReloadClassifiedError(stderr, connection, waitErr) logHotReloadBusyWaitComplete(connection, first.correlationID, wait, nil) @@ -128,6 +128,7 @@ func waitForBusyEditor( stderr io.Writer, deps connectionRetryDeps, options hotReloadEditorReadyWaitOptions, + exitWatch *clicore.UnityExitWatch, ) (hotReloadBusyWait, error) { startedAt := time.Now() deadline := startedAt.Add(options.budget) @@ -135,7 +136,12 @@ func waitForBusyEditor( lastResend := startedAt wait := hotReloadBusyWait{} for { - status, answered := probeHotReloadEditorStatus(ctx, connection, options.probeTimeout) + status, probeErr := probeHotReloadEditorStatus(ctx, connection, options.probeTimeout) + answered := probeErr == nil + if exitErr := exitWatch.Observe(ctx, probeErr); exitErr != nil { + wait.waited = time.Since(startedAt) + return wait, exitErr + } if answered && classifyEditorState(status) == statusStateReady { wait.waited, wait.ready = time.Since(startedAt), true return wait, nil diff --git a/cli/project-runner/internal/projectrunner/hot_reload_editor_ready_retry.go b/cli/project-runner/internal/projectrunner/hot_reload_editor_ready_retry.go index cf96c6844b..900ccd8ece 100644 --- a/cli/project-runner/internal/projectrunner/hot_reload_editor_ready_retry.go +++ b/cli/project-runner/internal/projectrunner/hot_reload_editor_ready_retry.go @@ -86,7 +86,7 @@ func retryHotReloadAfterEditorReady( // A plain line rather than the tool spinner: the spinner is off on a non-TTY stderr, and on a // TTY its Stop erases its own line, so the reader would not see why the command paused. _, _ = fmt.Fprintln(stderr, hotReloadEditorReadyWaitingLine) - waited, ready, err := waitForEditorReady(ctx, connection, hotReloadEditorReadyWaitDefaults) + waited, ready, err := waitForEditorReady(ctx, connection, hotReloadEditorReadyWaitDefaults, newUnityExitWatch(connection)) if err != nil { clicore.WriteJSON(stdout, first.result) writeHotReloadClassifiedError(stderr, connection, err) @@ -200,20 +200,25 @@ func hotReloadRetryParams(params map[string]any, selectedFiles []string) map[str } // waitForEditorReady polls the Editor status until the Editor answers and is ready, the budget -// runs out, or ctx is cancelled. -// Why every probe error counts as "not yet": the domain reload that follows Unity's compile takes -// the server down and brings it back. +// runs out, the Editor exits, or ctx is cancelled. +// Why every other probe error counts as "not yet": the domain reload that follows Unity's compile +// takes the server down and brings it back. func waitForEditorReady( ctx context.Context, connection unityipc.Connection, options hotReloadEditorReadyWaitOptions, + exitWatch *clicore.UnityExitWatch, ) (time.Duration, bool, error) { startedAt := time.Now() deadline := startedAt.Add(options.budget) for { - if probeEditorReady(ctx, connection, options.probeTimeout) { + status, probeErr := probeHotReloadEditorStatus(ctx, connection, options.probeTimeout) + if probeErr == nil && classifyEditorState(status) == statusStateReady { return time.Since(startedAt), true, nil } + if exitErr := exitWatch.Observe(ctx, probeErr); exitErr != nil { + return time.Since(startedAt), false, exitErr + } if !time.Now().Before(deadline) { return time.Since(startedAt), false, nil } @@ -225,18 +230,13 @@ func waitForEditorReady( } } -func probeEditorReady(ctx context.Context, connection unityipc.Connection, probeTimeout time.Duration) bool { - status, answered := probeHotReloadEditorStatus(ctx, connection, probeTimeout) - return answered && classifyEditorState(status) == statusStateReady -} - -// probeHotReloadEditorStatus asks the Editor for its status once; answered is false when it did not answer -// or the answer was not readable. +// probeHotReloadEditorStatus asks the Editor for its status once. The error is non-nil when the Editor +// did not answer or the answer was not readable. func probeHotReloadEditorStatus( ctx context.Context, connection unityipc.Connection, probeTimeout time.Duration, -) (editorStatusResponse, bool) { +) (editorStatusResponse, error) { probeCtx, cancel := context.WithTimeout(ctx, probeTimeout) defer cancel() raw, err := unityipc.NewClient(connection, clicontract.ProjectRunnerVersion()).Send( @@ -244,13 +244,13 @@ func probeHotReloadEditorStatus( editorStatusBridgeCommandName, map[string]any{}) if err != nil { - return editorStatusResponse{}, false + return editorStatusResponse{}, err } var response editorStatusResponse if err := json.Unmarshal(raw, &response); err != nil { - return editorStatusResponse{}, false + return editorStatusResponse{}, err } - return response, true + return response, nil } // composeHotReloadEditorReadyNote appends sentence to the note an earlier wait in this command left diff --git a/cli/project-runner/internal/projectrunner/pause_point_wait_poll.go b/cli/project-runner/internal/projectrunner/pause_point_wait_poll.go index f5db9ca269..884c5c1e19 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_wait_poll.go +++ b/cli/project-runner/internal/projectrunner/pause_point_wait_poll.go @@ -5,6 +5,7 @@ import ( "fmt" "time" + "github.com/hatayama/unity-cli-loop/common/clicore" clierrors "github.com/hatayama/unity-cli-loop/common/errors" "github.com/hatayama/unity-cli-loop/common/unityipc" ) @@ -265,6 +266,7 @@ type pausePointWaitLoopState struct { baselineSequence int hasBaseline bool baselineDecided bool + exitWatch *clicore.UnityExitWatch } func waitForPausePointStatus( @@ -284,13 +286,14 @@ func waitForPausePointStatus( baselineSequence: baselineSequence, hasBaseline: hasBaseline, baselineDecided: baselineDecided, + exitWatch: newUnityExitWatch(connection), } triggerDone := triggerHandle.doneChannel() ticker := time.NewTicker(pausePointStatusPoll) defer ticker.Stop() for { response, err := queryPausePointStatus(waitContext, connection, options.id) - waitState, done, pollErr := applyPausePointWaitPoll(&state, response, err, options.markerJustEnabled) + waitState, done, pollErr := applyPausePointWaitPoll(waitContext, &state, response, err, options.markerJustEnabled) if done { return state.lastResponse, waitState, state.triggerResult, state.hasBaseline, pollErr } @@ -315,14 +318,18 @@ func waitForPausePointStatus( } // applyPausePointWaitPoll records one status query. Why abort on a permanent -// connect error: every poll dials again, so a refused connect would otherwise -// burn the whole --timeout before the refusal is reported. +// connect error or an exited Editor: every poll dials again, so either would +// otherwise burn the whole --timeout before it is reported. func applyPausePointWaitPoll( + ctx context.Context, state *pausePointWaitLoopState, response pausePointStatusResponse, err error, markerJustEnabled bool, ) (pausePointWaitState, bool, error) { + if exitErr := state.exitWatch.Observe(ctx, err); exitErr != nil { + return "", true, exitErr + } if err != nil { if clierrors.IsPermanentConnectError(err) { return "", true, err diff --git a/cli/project-runner/internal/projectrunner/unity_exit_waits_test.go b/cli/project-runner/internal/projectrunner/unity_exit_waits_test.go new file mode 100644 index 0000000000..8b2e10dbc8 --- /dev/null +++ b/cli/project-runner/internal/projectrunner/unity_exit_waits_test.go @@ -0,0 +1,106 @@ +package projectrunner + +import ( + "bytes" + "context" + "testing" + "time" + + "github.com/hatayama/unity-cli-loop/common/clicore" +) + +// exitedEditorWatch finds no Editor process and moves its clock a second on every reading, so a wait +// against a server nobody listens on confirms the exit without sleeping. +func exitedEditorWatch(projectRoot string) *clicore.UnityExitWatch { + clock := time.Unix(1_700_000_000, 0) + return clicore.NewUnityExitWatch(projectRoot, "", noUnityProcessLookup, func() time.Time { + clock = clock.Add(time.Second) + return clock + }) +} + +// Verifies the play mode state wait ends with UnityNotRunningError once the Editor is confirmed gone, +// instead of polling until its timeout. +func TestWaitForControlPlayModeStateStopsOnceUnityHasExited(t *testing.T) { + originalPoll := controlPlayModeStatePoll + controlPlayModeStatePoll = time.Millisecond + t.Cleanup(func() { controlPlayModeStatePoll = originalPoll }) + connection := unreachableConnection(t.TempDir()) + + _, completed, err := waitForControlPlayModeState( + context.Background(), + connection, + "Play", + // Long enough that only the exit watch, never the timeout, can end the wait first. + 5*time.Second, + exitedEditorWatch(connection.ProjectRoot), + ) + + if completed { + t.Fatal("a play mode wait whose Editor exited cannot complete") + } + assertUnityExitedDuringCommand(t, err) +} + +// Verifies hot reload's wait for the Editor to settle ends with UnityNotRunningError once the Editor is +// confirmed gone. +func TestWaitForEditorReadyStopsOnceUnityHasExited(t *testing.T) { + connection := unreachableConnection(t.TempDir()) + options := fastEditorReadyWaitOptions() + options.budget = 5 * time.Second + + _, ready, err := waitForEditorReady(context.Background(), connection, options, exitedEditorWatch(connection.ProjectRoot)) + + if ready { + t.Fatal("an Editor that exited cannot become ready") + } + assertUnityExitedDuringCommand(t, err) +} + +// Verifies hot reload's wait for a busy Editor ends with UnityNotRunningError once the Editor is +// confirmed gone. +func TestWaitForBusyEditorStopsOnceUnityHasExited(t *testing.T) { + connection := unreachableConnection(t.TempDir()) + options := fastEditorReadyWaitOptions() + options.budget = 5 * time.Second + var stderr bytes.Buffer + + wait, err := waitForBusyEditor( + context.Background(), + connection, + map[string]any{}, + &stderr, + hotReloadBusyWaitSendDeps(), + options, + exitedEditorWatch(connection.ProjectRoot), + ) + + if wait.ready || wait.answered { + t.Fatalf("an Editor that exited cannot become ready or answer, got %#v", wait) + } + assertUnityExitedDuringCommand(t, err) +} + +// Verifies a pause point wait ends with UnityNotRunningError once polls against a gone server confirm +// that the Editor exited, instead of polling until --timeout. +func TestApplyPausePointWaitPollStopsOnceUnityHasExited(t *testing.T) { + connection := unreachableConnection(t.TempDir()) + state := pausePointWaitLoopState{exitWatch: exitedEditorWatch(connection.ProjectRoot)} + _, pollErr := queryPausePointStatusFromUnity(context.Background(), connection, "marker") + if pollErr == nil { + t.Fatal("a status query against an endpoint nobody listens on must fail") + } + + if _, done, err := applyPausePointWaitPoll(context.Background(), &state, pausePointStatusResponse{}, pollErr, false); done || err != nil { + t.Fatalf("one lookup must not end the wait, got done=%v err=%v", done, err) + } + // The watch's clock moves a second per poll, so a few more polls reach the confirming lookup. + for poll := 0; poll < 5; poll++ { + _, done, err := applyPausePointWaitPoll(context.Background(), &state, pausePointStatusResponse{}, pollErr, false) + if done { + assertUnityExitedDuringCommand(t, err) + return + } + } + t.Fatal("the confirmed exit must end the wait") +} diff --git a/cli/project-runner/internal/projectrunner/unity_process_lookup.go b/cli/project-runner/internal/projectrunner/unity_process_lookup.go index 61bf608396..b4c3ccc5b6 100644 --- a/cli/project-runner/internal/projectrunner/unity_process_lookup.go +++ b/cli/project-runner/internal/projectrunner/unity_process_lookup.go @@ -1,6 +1,8 @@ package projectrunner import ( + "github.com/hatayama/unity-cli-loop/common/clicore" + "github.com/hatayama/unity-cli-loop/common/unityipc" "github.com/hatayama/unity-cli-loop/common/unityprocess" ) @@ -9,3 +11,9 @@ import ( // dependencies, and they must be able to say that an Editor process stands behind it. The tests of // this package are not parallel. var unityProcessLookup = unityprocess.FindRunningUnityProcess + +// newUnityExitWatch creates the exit watch for one wait on the connection, using the default Editor +// process lookup and the real clock. +func newUnityExitWatch(connection unityipc.Connection) *clicore.UnityExitWatch { + return clicore.NewUnityExitWatch(connection.ProjectRoot, connection.Endpoint.Address, unityProcessLookup, nil) +} diff --git a/cli/project-runner/shared-inputs-stamp.json b/cli/project-runner/shared-inputs-stamp.json index d57fec769e..786e8d8954 100644 --- a/cli/project-runner/shared-inputs-stamp.json +++ b/cli/project-runner/shared-inputs-stamp.json @@ -1,4 +1,4 @@ { "schemaVersion": 1, - "sharedInputsHash": "9aebf20610dd0b92632ffe7d94412a40b91da890" + "sharedInputsHash": "46cd6ede805dddfb98dee3d2f179012325dac2f1" } From 1945194b627a3ebb6a9922962093027bad952f09 Mon Sep 17 00:00:00 2001 From: hatayama Date: Tue, 27 Oct 2026 14:48:01 +0900 Subject: [PATCH 04/12] Word an Editor exit during launch as a startup that did not finish The readiness wait behind `uloop launch` now ends with UNITY_NOT_RUNNING when the Editor exits, but the envelope was worded for a command that reached a running Editor: SafeToRetry false, a warning that the command may have partly run, and "run `uloop launch`, then run the command again", which for launch itself is circular. For launch the error now says Unity exited before it finished starting, stays SafeToRetry like every other launch failure, since another launch starts a new Editor and cannot double-apply anything, and tells the caller to run launch again after the Editor log step. --- cli/common/errors/error_envelope.go | 1 + cli/common/errors/unity_not_running.go | 19 +++++++++----- cli/common/errors/unity_not_running_test.go | 25 +++++++++++++++++++ .../internal/dispatcher/launch_test.go | 23 +++++++++++++++++ cli/dispatcher/shared-inputs-stamp.json | 2 +- cli/project-runner/shared-inputs-stamp.json | 2 +- 6 files changed, 64 insertions(+), 8 deletions(-) diff --git a/cli/common/errors/error_envelope.go b/cli/common/errors/error_envelope.go index 44ddd80e75..25904bf206 100644 --- a/cli/common/errors/error_envelope.go +++ b/cli/common/errors/error_envelope.go @@ -58,6 +58,7 @@ const ( ErrorPhaseExecution = "execution" commandRunTests = "run-tests" + commandLaunch = "launch" // Why this wording: --skip-compile only skips the CLI-side compile, so agents read a // disconnect here as an IPC fault instead of the domain reload that discarded their patches. diff --git a/cli/common/errors/unity_not_running.go b/cli/common/errors/unity_not_running.go index 112c9223d0..b7e7196b80 100644 --- a/cli/common/errors/unity_not_running.go +++ b/cli/common/errors/unity_not_running.go @@ -30,30 +30,34 @@ func (err UnityNotRunningError) Error() string { } func (err UnityNotRunningError) ToCLIError(context ErrorContext) CLIError { + launching := err.ExitedDuringCommand && context.Command == commandLaunch phase := ErrorPhaseConnection safeToRetry := true - if err.ExitedDuringCommand { + // Why launch stays safe: running it again starts a new Editor and cannot double-apply anything. + if err.ExitedDuringCommand && !launching { phase = ErrorPhaseResponseWaiting safeToRetry = isSafeRetryCommand(context.Command) } return CLIError{ ErrorCode: ErrorCodeUnityNotRunning, Phase: phase, - Message: err.message(), + Message: err.message(launching), Retryable: false, SafeToRetry: safeToRetry, ProjectRoot: firstNonEmpty(context.ProjectRoot, err.ProjectRoot), Command: context.Command, - NextActions: err.nextActions(safeToRetry), + NextActions: err.nextActions(launching, safeToRetry), Details: err.details(), } } // Why "may have crashed" and not "crashed": a lockfile left behind proves only that the normal shutdown // did not finish, which a forced quit or an interrupted cleanup also leaves. -func (err UnityNotRunningError) message() string { +func (err UnityNotRunningError) message(launching bool) string { message := "No Unity Editor is running for this project." - if err.ExitedDuringCommand { + if launching { + message = "Unity exited before it finished starting; no Unity Editor is running for this project now." + } else if err.ExitedDuringCommand { message = "Unity exited while this command was running; no Unity Editor is running for this project now." } if err.LockfileLeftBehind { @@ -62,11 +66,14 @@ func (err UnityNotRunningError) message() string { return message } -func (err UnityNotRunningError) nextActions(safeToRetry bool) []string { +func (err UnityNotRunningError) nextActions(launching bool, safeToRetry bool) []string { actions := []string{} if err.LockfileLeftBehind { actions = append(actions, err.EditorLogNextAction()) } + if launching { + return append(actions, "Run `uloop launch` again.") + } if !safeToRetry { actions = append(actions, "The command may have partly run before Unity exited; check the project and scene state before running it again.") } diff --git a/cli/common/errors/unity_not_running_test.go b/cli/common/errors/unity_not_running_test.go index 9d80229e66..3f76177273 100644 --- a/cli/common/errors/unity_not_running_test.go +++ b/cli/common/errors/unity_not_running_test.go @@ -165,3 +165,28 @@ func TestWriteToolFailureReportsUnityNotRunningForADispatchedRequest(t *testing. t.Fatalf("expected %s, got %#v", ErrorCodeUnityNotRunning, envelope.Error) } } + +// Verifies an Editor that exited while launch waited for it is reported as a startup that did not +// finish: launch stays safe to run again, and no step tells the caller to run launch and then launch. +func TestUnityNotRunningErrorAfterUnityExitedDuringLaunch(t *testing.T) { + err := UnityNotRunningError{ExitedDuringCommand: true, LockfileLeftBehind: true, EditorLogPath: unityNotRunningTestEditorLog} + + cliErr := err.ToCLIError(ErrorContext{Command: "launch"}) + + if !cliErr.SafeToRetry { + t.Fatal("running launch again cannot double-apply anything, so it must stay SafeToRetry") + } + if !strings.HasPrefix(cliErr.Message, "Unity exited before it finished starting") || + !strings.Contains(cliErr.Message, "may have crashed") { + t.Fatalf("expected the startup-exit message with the abnormal-exit note, got %q", cliErr.Message) + } + if len(cliErr.NextActions) != 2 { + t.Fatalf("expected the Editor log step and the launch step, got %#v", cliErr.NextActions) + } + if !strings.Contains(cliErr.NextActions[0], unityNotRunningTestEditorLog) { + t.Fatalf("expected the Editor log step first, got %q", cliErr.NextActions[0]) + } + if cliErr.NextActions[1] != "Run `uloop launch` again." { + t.Fatalf("expected launch to be run again, got %q", cliErr.NextActions[1]) + } +} diff --git a/cli/dispatcher/internal/dispatcher/launch_test.go b/cli/dispatcher/internal/dispatcher/launch_test.go index 6e424f22a5..e1b80655ee 100644 --- a/cli/dispatcher/internal/dispatcher/launch_test.go +++ b/cli/dispatcher/internal/dispatcher/launch_test.go @@ -696,6 +696,29 @@ func TestWaitForLaunchReadinessPreservesNoProcessReachability(t *testing.T) { } } +func TestWaitForLaunchReadinessReportsAnEditorThatExitedDuringStartup(t *testing.T) { + // Verifies an Editor that exited while launch waited for it is reported as not running, with launch + // still safe to run again, rather than as a startup timeout that says Unity is running. + deps := defaultLaunchDeps() + deps.waitForToolReadiness = func(ctx context.Context, projectRoot string, timeout time.Duration) error { + return clierrors.UnityNotRunningError{ProjectRoot: projectRoot, ExitedDuringCommand: true} + } + + err := waitForLaunchReadinessWithDeps(context.Background(), t.TempDir(), deps) + + var startupErr launchStartupTimeoutError + if errors.As(err, &startupErr) { + t.Fatalf("an exited Editor must not be classified as a startup timeout: %v", err) + } + cliErr := clierrors.ClassifyError(err, clierrors.ErrorContext{Command: clicore.LaunchCommandName}) + if cliErr.ErrorCode != clierrors.ErrorCodeUnityNotRunning || !cliErr.SafeToRetry { + t.Fatalf("expected a SafeToRetry %s, got %#v", clierrors.ErrorCodeUnityNotRunning, cliErr) + } + if !strings.HasPrefix(cliErr.Message, "Unity exited before it finished starting") { + t.Fatalf("message should say the Editor exited during startup: %#v", cliErr) + } +} + func TestWaitForLaunchReadinessPreservesParentCancellation(t *testing.T) { // Verifies caller cancellation is not converted into a launch startup timeout. deps := defaultLaunchDeps() diff --git a/cli/dispatcher/shared-inputs-stamp.json b/cli/dispatcher/shared-inputs-stamp.json index d58506c1de..20b224f4f2 100644 --- a/cli/dispatcher/shared-inputs-stamp.json +++ b/cli/dispatcher/shared-inputs-stamp.json @@ -1,4 +1,4 @@ { "schemaVersion": 1, - "sharedInputsHash": "bb1095cefcb0afeb776b946a08071b49ebc46477" + "sharedInputsHash": "8a2afeea01c5b6e7f6acaa4dd2dc7f40b5ba662d" } diff --git a/cli/project-runner/shared-inputs-stamp.json b/cli/project-runner/shared-inputs-stamp.json index 786e8d8954..0cca73a66c 100644 --- a/cli/project-runner/shared-inputs-stamp.json +++ b/cli/project-runner/shared-inputs-stamp.json @@ -1,4 +1,4 @@ { "schemaVersion": 1, - "sharedInputsHash": "46cd6ede805dddfb98dee3d2f179012325dac2f1" + "sharedInputsHash": "ad2ffdcf2ef901f43c9ccd606e4a4b2a574cd5c3" } From 9999c81b46ba19df4ef614dd0475e790d38712af Mon Sep 17 00:00:00 2001 From: hatayama Date: Tue, 27 Oct 2026 14:48:19 +0900 Subject: [PATCH 05/12] Close compile waits that end on an Editor exit A compile status wait that stopped because the Editor exited returned without a terminal vibe log entry, leaving a status poll start with no matching end: the exact signature docs/vibe-logs.md tells readers to look for behind a hang, in the one case where nothing hung. The wait now logs cli_compile_status_poll_unity_exited before returning. The attach path also kept its pending record after such an exit. That record points at a compile result held in the Editor's memory, which went away with the Editor, so the next compile after a relaunch would spend its attach probes looking for a result that cannot come back. It is now removed when the wait ended with UNITY_NOT_RUNNING; other failures keep it as before. --- .../internal/projectrunner/compile_attach.go | 16 ++++++- .../internal/projectrunner/compile_wait.go | 1 + .../projectrunner/compile_wait_log.go | 24 ++++++++++ .../compile_wait_unity_exit_test.go | 48 +++++++++++++++++++ 4 files changed, 88 insertions(+), 1 deletion(-) diff --git a/cli/project-runner/internal/projectrunner/compile_attach.go b/cli/project-runner/internal/projectrunner/compile_attach.go index 2cf100bc45..db74f2aef1 100644 --- a/cli/project-runner/internal/projectrunner/compile_attach.go +++ b/cli/project-runner/internal/projectrunner/compile_attach.go @@ -3,6 +3,7 @@ package projectrunner import ( "context" "encoding/json" + "errors" "fmt" "io" "time" @@ -183,7 +184,7 @@ func attachWaitForPendingCompile( }, deps) if waitErr != nil { spinner.Stop() - logCompileAttachResult(connection, record.RequestID, "error", false) + logCompileAttachResult(connection, record.RequestID, "error", clearCompilePendingRecordAfterEditorExit(connection.ProjectRoot, waitErr)) clierrors.WriteClassifiedError(stderr, waitErr, clierrors.ErrorContext{ ProjectRoot: connection.ProjectRoot, Command: clicore.CompileCommandName, @@ -227,6 +228,18 @@ func attachWaitForPendingCompile( } } +// clearCompilePendingRecordAfterEditorExit removes the pending record when the wait ended because the +// Editor exited, and reports whether it did. Why: the record points at a result held in the Editor's +// memory, which went away with it, so the next compile would only spend its attach probes finding nothing. +func clearCompilePendingRecordAfterEditorExit(projectRoot string, waitErr error) bool { + var notRunning clierrors.UnityNotRunningError + if !errors.As(waitErr, ¬Running) { + return false + } + clearCompilePendingRecord(projectRoot) + return true +} + func waitForAttachedCompileCompletion( ctx context.Context, options compileCompletionOptions, @@ -279,6 +292,7 @@ func waitForAttachedCompileCompletion( } logCompileStatusPollObservedIfChanged(options, startedAt, attempts, status, err, &lastObservationKey) if exitErr := exitWatch.Observe(ctx, err); exitErr != nil { + logCompileWaitUnityExited(options, startedAt, attempts, lastStatus, lastErr) return nil, attachWaitFailed, lastObservedCompileStatus(lastStatus, observedStatus), exitErr } observeCompileWaitInterim(&interim, deps, status, err) diff --git a/cli/project-runner/internal/projectrunner/compile_wait.go b/cli/project-runner/internal/projectrunner/compile_wait.go index 59e3f6c879..f8a8fb7cd4 100644 --- a/cli/project-runner/internal/projectrunner/compile_wait.go +++ b/cli/project-runner/internal/projectrunner/compile_wait.go @@ -226,6 +226,7 @@ func waitForCompileCompletionWithDeps( } logCompileStatusPollObservedIfChanged(options, startedAt, attempts, status, err, &lastObservationKey) if exitErr := exitWatch.Observe(ctx, err); exitErr != nil { + logCompileWaitUnityExited(options, startedAt, attempts, lastStatus, lastErr) return nil, false, lastObservedCompileStatus(lastStatus, observedStatus), exitErr } observeCompileWaitInterim(&interim, deps, status, err) diff --git a/cli/project-runner/internal/projectrunner/compile_wait_log.go b/cli/project-runner/internal/projectrunner/compile_wait_log.go index 2d3db34f10..86abe66eed 100644 --- a/cli/project-runner/internal/projectrunner/compile_wait_log.go +++ b/cli/project-runner/internal/projectrunner/compile_wait_log.go @@ -259,6 +259,30 @@ func logCompileWaitCancelled( }) } +// logCompileWaitUnityExited closes the poll's start entry when the wait ended because the Editor exited, +// so the log never shows a status poll that started and never finished. +func logCompileWaitUnityExited( + options compileCompletionOptions, + startedAt time.Time, + attempts int, + lastStatus compileStatusResponse, + lastErr error, +) { + writeCompileVibeLog(options.connection.ProjectRoot, func() vibelog.CLIVibeLogEntry { + return vibelog.CLIVibeLogEntry{ + Level: "WARNING", + Operation: "cli_compile_status_poll_unity_exited", + Message: "Stopped polling Unity compile status because the Unity Editor process exited.", + Context: compileWaitLogContext(options, startedAt, map[string]any{ + "poll_attempts": attempts, + "last_status": compileStatusLogContext(lastStatus), + "last_transport_error": clicore.ErrorMessage(lastErr), + }), + CorrelationID: options.requestID, + } + }) +} + func compileWaitLogContext( options compileCompletionOptions, startedAt time.Time, diff --git a/cli/project-runner/internal/projectrunner/compile_wait_unity_exit_test.go b/cli/project-runner/internal/projectrunner/compile_wait_unity_exit_test.go index d059eaf92d..97ec240b25 100644 --- a/cli/project-runner/internal/projectrunner/compile_wait_unity_exit_test.go +++ b/cli/project-runner/internal/projectrunner/compile_wait_unity_exit_test.go @@ -56,6 +56,25 @@ func TestWaitForCompileCompletionStopsOnceUnityHasExited(t *testing.T) { assertUnityExitedDuringCommand(t, err) } +// Verifies a compile status wait that stops on an exited Editor closes its start entry in the CLI vibe +// log, so the log does not show a poll that started and never ended. +func TestWaitForCompileCompletionLogsTheEditorExit(t *testing.T) { + enableCliVibeLog(t) + connection := compileWaitTestConnection(t) + + _, _, _, _ = waitForCompileCompletionWithDeps(context.Background(), compileCompletionOptions{ + connection: connection, + requestID: "compile_unity_exited_log", + timeout: 5 * time.Second, + pollInterval: time.Millisecond, + }, exitedEditorCompileDeps()) + + logContent := readOnlyCliVibeLog(t, connection.ProjectRoot) + if entries := cliVibeEntriesForOperation(t, logContent, "cli_compile_status_poll_unity_exited"); len(entries) != 1 { + t.Fatalf("unity-exited entries = %d, want 1\n%s", len(entries), logContent) + } +} + // Verifies the attach wait stops the same way when the Editor running the earlier compile is gone. func TestWaitForAttachedCompileCompletionStopsOnceUnityHasExited(t *testing.T) { _, outcome, _, err := waitForAttachedCompileCompletion(context.Background(), compileCompletionOptions{ @@ -113,3 +132,32 @@ func TestRunCompileReportsUnityNotRunningWhenTheEditorExitsDuringTheWait(t *test t.Fatalf("ErrorCode = %q, want %q\n%s", envelope.Error.ErrorCode, clierrors.ErrorCodeUnityNotRunning, stderr.String()) } } + +// Verifies an attach wait that ended because the Editor exited drops the pending record, whose result +// lived in the exited Editor, while any other failure keeps it for the next compile to reattach to. +func TestClearCompilePendingRecordAfterEditorExit(t *testing.T) { + cases := []struct { + name string + waitErr error + wantCleared bool + }{ + {name: "Editor exited", waitErr: clierrors.UnityNotRunningError{ExitedDuringCommand: true}, wantCleared: true}, + {name: "other failure", waitErr: errors.New("status query failed"), wantCleared: false}, + } + for _, testCase := range cases { + t.Run(testCase.name, func(t *testing.T) { + projectRoot := t.TempDir() + record := compilePendingRecord{RequestID: "compile_pending", TimedOutAtUtc: time.Now().UTC()} + if err := writeCompilePendingRecord(projectRoot, record); err != nil { + t.Fatalf("writing the pending record failed: %v", err) + } + + cleared := clearCompilePendingRecordAfterEditorExit(projectRoot, testCase.waitErr) + + _, kept := readCompilePendingRecord(projectRoot) + if cleared != testCase.wantCleared || kept == testCase.wantCleared { + t.Fatalf("cleared = %v, record kept = %v, want cleared = %v", cleared, kept, testCase.wantCleared) + } + }) + } +} From fcb9cf715dd724ec162100bcadaeb79a403155c2 Mon Sep 17 00:00:00 2001 From: hatayama Date: Tue, 27 Oct 2026 14:48:20 +0900 Subject: [PATCH 06/12] Log a failed Editor lookup after a dispatched disconnect When the request reached Unity and the connection dropped, a process lookup that failed left the disconnect in place without any trace, while the same failure before dispatch writes cli_unity_process_probe_failed. Both now log it, so a disconnect that was not upgraded to UNITY_NOT_RUNNING can be told apart from one where an Editor was found. The log message is reworded to cover both paths. The test helper that makes the default lookup find an Editor also notes that the default focus is still real, so a test ending in an undispatched dial failure should stub it. --- .../projectrunner/connection_retry_flow.go | 8 ++++-- .../connection_retry_flow_test.go | 27 +++++++++++++++++++ .../unity_process_lookup_helpers_test.go | 2 ++ 3 files changed, 35 insertions(+), 2 deletions(-) diff --git a/cli/project-runner/internal/projectrunner/connection_retry_flow.go b/cli/project-runner/internal/projectrunner/connection_retry_flow.go index 7b2373975a..ac1cbc2161 100644 --- a/cli/project-runner/internal/projectrunner/connection_retry_flow.go +++ b/cli/project-runner/internal/projectrunner/connection_retry_flow.go @@ -116,7 +116,11 @@ func reportUnityExitDuringRequest( return err } runningProcess, processErr := deps.findRunningUnityProcess(ctx, connection.ProjectRoot) - if processErr != nil || runningProcess != nil { + if processErr != nil { + logUnityProcessProbeFailure(connection, err, processErr) + return err + } + if runningProcess != nil { return err } return clicore.NewUnityNotRunningError(connection.ProjectRoot, connection.Endpoint.Address, err, true) @@ -170,7 +174,7 @@ func logUnityProcessProbeFailure(connection unityipc.Connection, dialErr error, _ = vibelog.WriteCLIVibeLog(connection.ProjectRoot, vibelog.CLIVibeLogEntry{ Level: "WARNING", Operation: "cli_unity_process_probe_failed", - Message: "Could not determine whether Unity is running while recovering an unreachable request.", + Message: "Could not determine whether Unity is running after a request failed to reach it or lost its connection.", Context: map[string]any{ "endpoint": connection.Endpoint.Address, "dial_cause": clicore.ErrorMessage(dialErr), diff --git a/cli/project-runner/internal/projectrunner/connection_retry_flow_test.go b/cli/project-runner/internal/projectrunner/connection_retry_flow_test.go index fb1cbc70f2..e3d780f158 100644 --- a/cli/project-runner/internal/projectrunner/connection_retry_flow_test.go +++ b/cli/project-runner/internal/projectrunner/connection_retry_flow_test.go @@ -608,3 +608,30 @@ func TestReportUnityExitDuringRequest(t *testing.T) { }) } } + +// Verifies a process lookup that failed after a dispatched disconnect leaves a trace in the CLI vibe log, +// as the same failure before dispatch does, since the command output keeps only the disconnect. +func TestReportUnityExitDuringRequestLogsAFailedLookup(t *testing.T) { + enableCliVibeLog(t) + deps := defaultConnectionRetryDeps() + deps.findRunningUnityProcess = func(context.Context, string) (*unityprocess.UnityProcess, error) { + return nil, errors.New("sysctl kern.proc.all: operation not permitted") + } + connection := unityipc.Connection{ + ProjectRoot: t.TempDir(), + Endpoint: unityipc.Endpoint{Address: "/tmp/uloop/sample.sock"}, + } + + _ = reportUnityExitDuringRequest( + context.Background(), + connection, + unityipc.UnitySendOutcome{RequestDispatched: true, RequestAccepted: true}, + io.EOF, + deps, + ) + + logContent := readOnlyCliVibeLog(t, connection.ProjectRoot) + if entries := cliVibeEntriesForOperation(t, logContent, "cli_unity_process_probe_failed"); len(entries) != 1 { + t.Fatalf("probe failure entries = %d, want 1\n%s", len(entries), logContent) + } +} diff --git a/cli/project-runner/internal/projectrunner/unity_process_lookup_helpers_test.go b/cli/project-runner/internal/projectrunner/unity_process_lookup_helpers_test.go index 046b36cff3..56f60bd287 100644 --- a/cli/project-runner/internal/projectrunner/unity_process_lookup_helpers_test.go +++ b/cli/project-runner/internal/projectrunner/unity_process_lookup_helpers_test.go @@ -23,6 +23,8 @@ func useUnityProcessLookup( // useRunningUnityProcess makes the default lookup find an Editor, as a scripted server stands in for one. // Why this test process's own pid: a focus attempt on it can never raise another application's window. +// The default focus is still the real one, so a test whose command ends in a dial that never reaches +// the server should also stub focusUnityProcess, or it waits out a real focus attempt. func useRunningUnityProcess(t *testing.T) { t.Helper() useUnityProcessLookup(t, runningUnityProcessLookup) From 43dd4eec4c9b887fcd243ca462e3b017c14e29bf Mon Sep 17 00:00:00 2001 From: hatayama Date: Tue, 27 Oct 2026 15:02:01 +0900 Subject: [PATCH 07/12] Read -createProject as the project an Editor has open The Editor process lookup ties a Unity process to a project only through -projectPath on its command line. The Editor that creates a project opens it in the same session, and that session's command line may name the project only through -createProject. Such an Editor was invisible to the lookup, which now ends compile and other waits with UNITY_NOT_RUNNING, and a new user is likely to install uloop in exactly that session. --- cli/common/unityprocess/process.go | 25 ++++++++++++--------- cli/common/unityprocess/process_test.go | 17 ++++++++++++++ cli/dispatcher/shared-inputs-stamp.json | 2 +- cli/project-runner/shared-inputs-stamp.json | 2 +- 4 files changed, 33 insertions(+), 13 deletions(-) diff --git a/cli/common/unityprocess/process.go b/cli/common/unityprocess/process.go index 6f8f734e6c..fdffafaf3a 100644 --- a/cli/common/unityprocess/process.go +++ b/cli/common/unityprocess/process.go @@ -22,8 +22,10 @@ const windowsPowerShellCommand = "powershell" var ( macUnityExecutablePattern = regexp.MustCompile(`(?i)Unity\.app/Contents/MacOS/Unity`) windowsUnityExecutablePattern = regexp.MustCompile(`(?i)Unity\.exe`) - projectPathFlagPattern = regexp.MustCompile(`(?i)-projectpath(?:=|\s+)(.+)$`) - nextUnityFlagPattern = regexp.MustCompile(`\s-[A-Za-z][A-Za-z0-9-]*(?:=|\s|$)`) + // Why -createProject too: the Editor that creates a project opens it in the same session, and that + // session's command line may name the project only through -createProject. + projectPathFlagPattern = regexp.MustCompile(`(?i)-(?:projectpath|createproject)(?:=|\s+)(.+)$`) + nextUnityFlagPattern = regexp.MustCompile(`\s-[A-Za-z][A-Za-z0-9-]*(?:=|\s|$)`) ) type UnityProcess struct { @@ -237,20 +239,21 @@ func hasNonEditorLinuxArg(args []string) bool { return false } -// linuxProjectPathFromArgs returns the value of Unity's -projectPath flag, accepting +// linuxProjectPathFromArgs returns the value of Unity's -projectPath or -createProject flag, accepting // both the "-projectPath " and "-projectPath=" forms case-insensitively. func linuxProjectPathFromArgs(args []string) string { - const projectPathFlag = "-projectpath" for index, arg := range args { lowerArg := strings.ToLower(arg) - if lowerArg == projectPathFlag { - if index+1 >= len(args) { - return "" + for _, flag := range []string{"-projectpath", "-createproject"} { + if lowerArg == flag { + if index+1 >= len(args) { + return "" + } + return args[index+1] + } + if strings.HasPrefix(lowerArg, flag+"=") { + return arg[len(flag+"="):] } - return args[index+1] - } - if strings.HasPrefix(lowerArg, projectPathFlag+"=") { - return arg[len(projectPathFlag+"="):] } } return "" diff --git a/cli/common/unityprocess/process_test.go b/cli/common/unityprocess/process_test.go index ac8a3c8c4e..5fc1ee921f 100644 --- a/cli/common/unityprocess/process_test.go +++ b/cli/common/unityprocess/process_test.go @@ -180,6 +180,21 @@ func TestExtractProjectPathSupportsEqualsAndSpaces(t *testing.T) { } } +// Verifies an Editor started to create a project is read as having that project open, since the Editor +// that creates a project opens it in the same session without a -projectPath flag. +func TestExtractProjectPathReadsCreateProject(t *testing.T) { + cases := map[string]string{ + `Unity -createProject "/Users//New Project" -cloneFromTemplate /tmp/template.tgz -useHub`: "/Users//New Project", + `Unity -createproject=/Users//Plain -useHub`: "/Users//Plain", + } + + for command, expected := range cases { + if actual := extractProjectPath(command); actual != expected { + t.Fatalf("project path mismatch for %q: %q", command, actual) + } + } +} + // Verifies comparable project paths preserve case on case-sensitive-capable platforms. func TestNormalizeComparablePathPreservesCaseOutsideWindows(t *testing.T) { if runtime.GOOS == "windows" { @@ -229,6 +244,8 @@ func TestMatchLinuxUnityProcessExtractsProjectPath(t *testing.T) { {"equals form with mixed case", []string{"/opt/Unity/Editor/Unity", "-projectPath=/work/p"}, "/work/p"}, {"project path containing batchmode", []string{"/opt/Unity/Editor/Unity", "-projectpath", "/work/nightly-batchmode"}, "/work/nightly-batchmode"}, {"project path containing assetimportworker", []string{"/opt/Unity/Editor/Unity", "-projectpath", "/work/AssetImportWorker-lab"}, "/work/AssetImportWorker-lab"}, + {"create project", []string{"/opt/Unity/Editor/Unity", "-createProject", "/work/New Project", "-cloneFromTemplate", "/tmp/template.tgz"}, "/work/New Project"}, + {"create project equals form", []string{"/opt/Unity/Editor/Unity", "-createproject=/work/new"}, "/work/new"}, {"unity hub hardlink name with lowercase projectpath", []string{"/home/user/Unity/Hub/Editor/2022.3.62f3/Editor/unityhub-unity-editor-2022.3.62f3", "-projectpath", "/home/user/project", "-acceptSoftwareTermsForThisRunOnly", "-useHub", "-hubIPC", "-cloudEnvironment", "production", "-licensingIpc", "LicenseClient-user", "-hubSessionId", "session-placeholder", "-accessToken", "token-placeholder"}, "/home/user/project"}, } for _, testCase := range cases { diff --git a/cli/dispatcher/shared-inputs-stamp.json b/cli/dispatcher/shared-inputs-stamp.json index 20b224f4f2..7e55d00d20 100644 --- a/cli/dispatcher/shared-inputs-stamp.json +++ b/cli/dispatcher/shared-inputs-stamp.json @@ -1,4 +1,4 @@ { "schemaVersion": 1, - "sharedInputsHash": "8a2afeea01c5b6e7f6acaa4dd2dc7f40b5ba662d" + "sharedInputsHash": "67cfcd0899eee491a401d6c5e35a981ea3ea0e41" } diff --git a/cli/project-runner/shared-inputs-stamp.json b/cli/project-runner/shared-inputs-stamp.json index 0cca73a66c..8906edae08 100644 --- a/cli/project-runner/shared-inputs-stamp.json +++ b/cli/project-runner/shared-inputs-stamp.json @@ -1,4 +1,4 @@ { "schemaVersion": 1, - "sharedInputsHash": "ad2ffdcf2ef901f43c9ccd606e4a4b2a574cd5c3" + "sharedInputsHash": "628fba4cdbdf4dce0ffa03fc6f89137779c8a500" } From 113ccc554127f36328651c42b5f6b8037b1af9da Mon Sep 17 00:00:00 2001 From: hatayama Date: Tue, 27 Oct 2026 15:06:55 +0900 Subject: [PATCH 08/12] Do not report Unity as not running while an unidentified Editor runs The process lookup dropped every Editor whose command line names no project, so "nothing found" also covered an Editor that may be this project's. That was harmless while it only chose whether to focus, but UNITY_NOT_RUNNING, the exit watch that ends compile and other waits, and the crash wording in `uloop status` now conclude from it that Unity is gone. Such an Editor would have its compile wait cut short by an ordinary domain reload. The lookup now keeps those Editors as unidentified. A new FindRunningUnityProcessDefinitively returns ErrUnidentifiedUnityEditor instead of nil while one runs, and every caller that concludes Unity is not running uses it, so they treat it as a lookup that could not tell: commands keep UNITY_NOT_REACHABLE, waits keep waiting, and status reports Unreachable with the reason. Launch and focus keep the plain lookup. --- cli/common/clicore/focus.go | 4 -- cli/common/clicore/tool_readiness.go | 2 +- cli/common/unityprocess/process.go | 64 +++++++++++++------ .../unityprocess/process_branches_test.go | 53 +++++++++++++-- cli/common/unityprocess/process_test.go | 32 +++++++++- cli/dispatcher/shared-inputs-stamp.json | 2 +- .../internal/projectrunner/status_command.go | 7 +- .../projectrunner/status_command_test.go | 14 ++++ .../projectrunner/unity_process_lookup.go | 5 +- cli/project-runner/shared-inputs-stamp.json | 2 +- 10 files changed, 147 insertions(+), 38 deletions(-) diff --git a/cli/common/clicore/focus.go b/cli/common/clicore/focus.go index 4285ed4c30..10fbb9c58e 100644 --- a/cli/common/clicore/focus.go +++ b/cli/common/clicore/focus.go @@ -28,10 +28,6 @@ func RunFocusWindow(ctx context.Context, projectRoot string, stdout io.Writer, s return runFocusWindow(ctx, projectRoot, stdout, stderr, defaultFocusWindowDeps()) } -func FindRunningUnityProcess(ctx context.Context, projectRoot string) (*UnityProcess, error) { - return unityprocess.FindRunningUnityProcess(ctx, projectRoot) -} - func FocusUnityProcess(ctx context.Context, pid int) error { return unityprocess.FocusUnityProcess(ctx, pid) } diff --git a/cli/common/clicore/tool_readiness.go b/cli/common/clicore/tool_readiness.go index 28a6ceca01..a35a0926a6 100644 --- a/cli/common/clicore/tool_readiness.go +++ b/cli/common/clicore/tool_readiness.go @@ -42,7 +42,7 @@ func WaitForToolReadinessWithTimeout(ctx context.Context, projectRoot string, ti func defaultToolReadinessDeps() toolReadinessDeps { return toolReadinessDeps{ - findRunningUnityProcess: unityprocess.FindRunningUnityProcess, + findRunningUnityProcess: unityprocess.FindRunningUnityProcessDefinitively, probeToolReadinessSequence: ProbeToolReadinessSequence, } } diff --git a/cli/common/unityprocess/process.go b/cli/common/unityprocess/process.go index fdffafaf3a..716563c2bd 100644 --- a/cli/common/unityprocess/process.go +++ b/cli/common/unityprocess/process.go @@ -29,34 +29,69 @@ var ( ) type UnityProcess struct { - Pid int + Pid int + // projectPath is empty for an Editor whose command line does not name its project. projectPath string } +// ErrUnidentifiedUnityEditor reports that no Editor is listed for the project while an Editor whose +// command line does not name its project is running, so that Editor may be the project's. +var ErrUnidentifiedUnityEditor = errors.New( + "a Unity Editor whose command line does not name its project is running, so it may be this project's Editor") + type RestoreFocusFunc func(context.Context) error +// FindRunningUnityProcess returns the running Editor listed for projectRoot, or nil when none is. func FindRunningUnityProcess(ctx context.Context, projectRoot string) (*UnityProcess, error) { + process, _, err := findRunningUnityProcess(ctx, projectRoot) + return process, err +} + +// FindRunningUnityProcessDefinitively is FindRunningUnityProcess for callers that conclude from a nil +// result that no Editor is running for the project: it returns nil only when no running Editor can be +// the project's, and ErrUnidentifiedUnityEditor when an Editor whose project is unknown may be. +func FindRunningUnityProcessDefinitively(ctx context.Context, projectRoot string) (*UnityProcess, error) { + process, unidentifiedRunning, err := findRunningUnityProcess(ctx, projectRoot) + if err == nil && process == nil && unidentifiedRunning { + return nil, ErrUnidentifiedUnityEditor + } + return process, err +} + +func findRunningUnityProcess(ctx context.Context, projectRoot string) (*UnityProcess, bool, error) { processes, err := listUnityProcesses(ctx) if err != nil { - return nil, err + return nil, false, err } + return selectProjectProcess(processes, projectRoot) +} +// selectProjectProcess picks projectRoot's Editor from the listed Editors, and reports whether an Editor +// whose project is unknown was listed. +func selectProjectProcess(processes []UnityProcess, projectRoot string) (*UnityProcess, bool, error) { normalizedTarget, err := normalizeComparablePath(projectRoot) if err != nil { - return nil, err + return nil, false, err } + unidentifiedRunning := false for _, candidate := range processes { + // Why not compare an empty path: filepath.Abs resolves it to the working directory, so it + // would match a project the command runs from. + if candidate.projectPath == "" { + unidentifiedRunning = true + continue + } normalizedCandidate, err := normalizeComparablePath(candidate.projectPath) if err != nil { continue } if normalizedCandidate == normalizedTarget { process := candidate - return &process, nil + return &process, unidentifiedRunning, nil } } - return nil, nil + return nil, unidentifiedRunning, nil } func listUnityProcesses(ctx context.Context) ([]UnityProcess, error) { @@ -137,11 +172,7 @@ func matchMacUnityProcess(pid int, command string) (UnityProcess, bool) { if !isUnityEditorCommand(command, macUnityExecutablePattern) { return UnityProcess{}, false } - projectPath := extractProjectPath(command) - if projectPath == "" { - return UnityProcess{}, false - } - return UnityProcess{Pid: pid, projectPath: projectPath}, true + return UnityProcess{Pid: pid, projectPath: extractProjectPath(command)}, true } // parseMacProcArgs2 decodes the kern.procargs2 sysctl buffer layout: a leading int32 @@ -208,11 +239,7 @@ func matchLinuxUnityProcess(pid int, args []string) (UnityProcess, bool) { if hasNonEditorLinuxArg(args) { return UnityProcess{}, false } - projectPath := linuxProjectPathFromArgs(args) - if projectPath == "" { - return UnityProcess{}, false - } - return UnityProcess{Pid: pid, projectPath: projectPath}, true + return UnityProcess{Pid: pid, projectPath: linuxProjectPathFromArgs(args)}, true } // isLinuxUnityEditorExecutable reports whether argv[0]'s basename is a Unity Editor @@ -286,12 +313,7 @@ func parseWindowsUnityProcesses(output string) []UnityProcess { if !isUnityEditorCommand(command, windowsUnityExecutablePattern) { continue } - projectPath := extractProjectPath(command) - if projectPath == "" { - continue - } - - processes = append(processes, UnityProcess{Pid: pid, projectPath: projectPath}) + processes = append(processes, UnityProcess{Pid: pid, projectPath: extractProjectPath(command)}) } return processes } diff --git a/cli/common/unityprocess/process_branches_test.go b/cli/common/unityprocess/process_branches_test.go index 7184191c01..f04362402b 100644 --- a/cli/common/unityprocess/process_branches_test.go +++ b/cli/common/unityprocess/process_branches_test.go @@ -5,6 +5,7 @@ import ( "encoding/binary" "errors" "fmt" + "os" "os/exec" "path/filepath" "runtime" @@ -12,14 +13,57 @@ import ( "testing" ) -// Verifies a Unity editor command without any -projectPath flag is not reported as a project's Editor. -func TestMatchMacUnityProcessRejectsCommandWithoutProjectPath(t *testing.T) { +// Verifies a Unity editor command without a project flag is kept as an Editor whose project is unknown, +// so the lookup can tell it apart from no Editor at all. +func TestMatchMacUnityProcessKeepsAnEditorWithoutProjectPathUnidentified(t *testing.T) { process, matched := matchMacUnityProcess( 123, `/Applications/Unity/Hub/Editor/6000.0.0f1/Unity.app/Contents/MacOS/Unity -useHub -hubIPC`) - if matched { - t.Fatalf("expected no match without a project path, got %#v", process) + if !matched || process.Pid != 123 || process.projectPath != "" { + t.Fatalf("expected an Editor with no project path, got matched = %v, %#v", matched, process) + } +} + +// Verifies the project's Editor is picked by its project path, and that an Editor whose project is +// unknown is reported beside the result and never matched, not even for a project at the working +// directory, which an empty path would resolve to. +func TestSelectProjectProcess(t *testing.T) { + workingDirectory, err := os.Getwd() + if err != nil { + t.Fatalf("reading the working directory failed: %v", err) + } + projectRoot := t.TempDir() + unidentified := UnityProcess{Pid: 1} + project := UnityProcess{Pid: 2, projectPath: projectRoot} + other := UnityProcess{Pid: 3, projectPath: filepath.Join(projectRoot, "Other")} + cases := []struct { + name string + processes []UnityProcess + projectRoot string + wantPid int + wantUnidentified bool + }{ + {name: "project Editor beside an unidentified one", processes: []UnityProcess{unidentified, project}, projectRoot: projectRoot, wantPid: 2, wantUnidentified: true}, + {name: "only another project's Editor", processes: []UnityProcess{other}, projectRoot: projectRoot}, + {name: "only an unidentified Editor", processes: []UnityProcess{other, unidentified}, projectRoot: projectRoot, wantUnidentified: true}, + {name: "unidentified Editor and a project at the working directory", processes: []UnityProcess{unidentified}, projectRoot: workingDirectory, wantUnidentified: true}, + } + for _, testCase := range cases { + t.Run(testCase.name, func(t *testing.T) { + process, unidentifiedRunning, err := selectProjectProcess(testCase.processes, testCase.projectRoot) + if err != nil { + t.Fatalf("selectProjectProcess failed: %v", err) + } + gotPid := 0 + if process != nil { + gotPid = process.Pid + } + if gotPid != testCase.wantPid || unidentifiedRunning != testCase.wantUnidentified { + t.Fatalf("pid = %d, unidentified = %v, want pid = %d, unidentified = %v", + gotPid, unidentifiedRunning, testCase.wantPid, testCase.wantUnidentified) + } + }) } } @@ -54,7 +98,6 @@ func TestParseWindowsUnityProcessesSkipsMalformedLines(t *testing.T) { }{ {"no delimiter", validEditorCommand}, {"pid is not a number", "notapid|" + validEditorCommand}, - {"editor command without project path", "456|" + encode(`C:\Editor\Unity.exe -useHub`)}, } for _, testCase := range cases { t.Run(testCase.name, func(t *testing.T) { diff --git a/cli/common/unityprocess/process_test.go b/cli/common/unityprocess/process_test.go index 5fc1ee921f..3f122fcd5d 100644 --- a/cli/common/unityprocess/process_test.go +++ b/cli/common/unityprocess/process_test.go @@ -116,6 +116,17 @@ func TestParseWindowsUnityProcessesExtractsProjectPath(t *testing.T) { } } +// Verifies a Windows Editor whose command line does not name its project is kept with no project path. +func TestParseWindowsUnityProcessesKeepsAnEditorWithoutProjectPathUnidentified(t *testing.T) { + output := "123|" + base64.StdEncoding.EncodeToString([]byte(`C:\Editor\Unity.exe -useHub -hubIPC`)) + "\r\n" + + processes := parseWindowsUnityProcesses(output) + + if len(processes) != 1 || processes[0].Pid != 123 || processes[0].projectPath != "" { + t.Fatalf("expected one Editor with no project path, got %#v", processes) + } +} + // Verifies non-ASCII project paths survive the PowerShell boundary because command lines travel as UTF-8 Base64. func TestParseWindowsUnityProcessesPreservesNonASCIIProjectPath(t *testing.T) { projectPath := `C:\Users\\test[1] 検証用\proj` @@ -261,6 +272,25 @@ func TestMatchLinuxUnityProcessExtractsProjectPath(t *testing.T) { } } +// Verifies a Linux Editor whose command line does not name its project is kept with no project path. +func TestMatchLinuxUnityProcessKeepsAnEditorWithoutProjectPathUnidentified(t *testing.T) { + cases := []struct { + name string + args []string + }{ + {"no project path flag", []string{"/opt/Unity/Editor/Unity"}}, + {"project path flag without value", []string{"/opt/Unity/Editor/Unity", "-projectpath"}}, + } + for _, testCase := range cases { + t.Run(testCase.name, func(t *testing.T) { + process, matched := matchLinuxUnityProcess(42, testCase.args) + if !matched || process.Pid != 42 || process.projectPath != "" { + t.Fatalf("expected an Editor with no project path, got matched = %v, %#v", matched, process) + } + }) + } +} + // Verifies Linux Unity process matching rejects each non-Editor shape independently. func TestMatchLinuxUnityProcessRejectsNonEditorProcesses(t *testing.T) { cases := []struct { @@ -272,8 +302,6 @@ func TestMatchLinuxUnityProcessRejectsNonEditorProcesses(t *testing.T) { {"executable name is not Unity", []string{"/opt/Unity/Editor/Data/Tools/UnityShaderCompiler", "-projectpath", "/work/p"}}, {"asset import worker with relative hub hardlink name", []string{"unityhub-unity-editor-2022.3.62f3", "-batchMode", "-name", "AssetImportWorker0", "-projectPath", "/home/user/project"}}, {"unrelated executable with unityhub prefix in directory only", []string{"/home/user/Unity/Hub/Editor/2022.3.62f3/Editor/Data/Tools/UnityShaderCompiler", "-projectpath", "/home/user/project"}}, - {"no project path flag", []string{"/opt/Unity/Editor/Unity"}}, - {"project path flag without value", []string{"/opt/Unity/Editor/Unity", "-projectpath"}}, {"empty argv", []string{}}, } for _, testCase := range cases { diff --git a/cli/dispatcher/shared-inputs-stamp.json b/cli/dispatcher/shared-inputs-stamp.json index 7e55d00d20..307346c698 100644 --- a/cli/dispatcher/shared-inputs-stamp.json +++ b/cli/dispatcher/shared-inputs-stamp.json @@ -1,4 +1,4 @@ { "schemaVersion": 1, - "sharedInputsHash": "67cfcd0899eee491a401d6c5e35a981ea3ea0e41" + "sharedInputsHash": "27e41300f8d098b84e3a080ff62d01ce80808804" } diff --git a/cli/project-runner/internal/projectrunner/status_command.go b/cli/project-runner/internal/projectrunner/status_command.go index 1016ba7db3..7ef950b254 100644 --- a/cli/project-runner/internal/projectrunner/status_command.go +++ b/cli/project-runner/internal/projectrunner/status_command.go @@ -14,6 +14,7 @@ import ( "github.com/hatayama/unity-cli-loop/common/clicontract" "github.com/hatayama/unity-cli-loop/common/clicore" "github.com/hatayama/unity-cli-loop/common/unityipc" + "github.com/hatayama/unity-cli-loop/common/unityprocess" ) const editorStatusBridgeCommandName = "get-editor-status" @@ -49,7 +50,7 @@ func defaultStatusCommandDeps() statusCommandDeps { probeTimeout: 5 * time.Second, reprobeDelay: 1 * time.Second, isUnityProcessRunning: func(ctx context.Context, projectRoot string) (bool, error) { - process, err := clicore.FindRunningUnityProcess(ctx, projectRoot) + process, err := unityprocess.FindRunningUnityProcessDefinitively(ctx, projectRoot) return process != nil, err }, } @@ -249,6 +250,10 @@ func reportFromUnreachableServer( var report statusReport running, lookupErr := deps.isUnityProcessRunning(ctx, projectRoot) switch { + case errors.Is(lookupErr, unityprocess.ErrUnidentifiedUnityEditor): + report = newStatusReport(statusStateUnreachable, + "The uloop server is not accepting connections, and a running Unity Editor's command line "+ + "does not name its project, so uloop cannot tell whether Unity is running for this project.") case lookupErr != nil: report = newStatusReport(statusStateUnreachable, fmt.Sprintf( "The uloop server is not accepting connections, and the Unity process list could not be read: %s.", diff --git a/cli/project-runner/internal/projectrunner/status_command_test.go b/cli/project-runner/internal/projectrunner/status_command_test.go index 28b50cfe73..0fbadcf065 100644 --- a/cli/project-runner/internal/projectrunner/status_command_test.go +++ b/cli/project-runner/internal/projectrunner/status_command_test.go @@ -471,6 +471,20 @@ func TestRunStatusCommandReportsAnAbnormalExitWhenTheLockfileWasLeftBehind(t *te } } +// Verifies a refused connection is Unreachable rather than NotRunning when an Editor whose project the +// lookup cannot read is running, since that Editor may be this project's and alive. +func TestRunStatusCommandReportsUnreachableWhileAnUnidentifiedEditorRuns(t *testing.T) { + projectRoot := writeFakeUnityProject(t) + deps := statusTestDeps(t) + deps.isUnityProcessRunning = fakeUnityProcessLookup(t, projectRoot, false, unityprocess.ErrUnidentifiedUnityEditor) + + run := runStatusForTest(context.Background(), unreachableConnection(projectRoot), nil, deps) + + report := assertStatusReport(t, run, "Unreachable") + assertStatusMessage(t, report, "The uloop server is not accepting connections, and a running Unity Editor's command line "+ + "does not name its project, so uloop cannot tell whether Unity is running for this project.") +} + // Verifies a refused connection whose process lookup fails is Unreachable, naming the lookup // error and leaving out the process flag it could not determine. func TestRunStatusCommandReportsUnreachableWhenProcessLookupFails(t *testing.T) { diff --git a/cli/project-runner/internal/projectrunner/unity_process_lookup.go b/cli/project-runner/internal/projectrunner/unity_process_lookup.go index b4c3ccc5b6..fc3a45ccec 100644 --- a/cli/project-runner/internal/projectrunner/unity_process_lookup.go +++ b/cli/project-runner/internal/projectrunner/unity_process_lookup.go @@ -9,8 +9,9 @@ import ( // unityProcessLookup is the lookup every default dependency set uses to find this project's Editor. // Why a package variable: whole-command tests drive a scripted server through the default // dependencies, and they must be able to say that an Editor process stands behind it. The tests of -// this package are not parallel. -var unityProcessLookup = unityprocess.FindRunningUnityProcess +// this package are not parallel. Why the definitive lookup: its callers conclude from a nil result that +// Unity is not running. +var unityProcessLookup = unityprocess.FindRunningUnityProcessDefinitively // newUnityExitWatch creates the exit watch for one wait on the connection, using the default Editor // process lookup and the real clock. diff --git a/cli/project-runner/shared-inputs-stamp.json b/cli/project-runner/shared-inputs-stamp.json index 8906edae08..1850ef3ffd 100644 --- a/cli/project-runner/shared-inputs-stamp.json +++ b/cli/project-runner/shared-inputs-stamp.json @@ -1,4 +1,4 @@ { "schemaVersion": 1, - "sharedInputsHash": "628fba4cdbdf4dce0ffa03fc6f89137779c8a500" + "sharedInputsHash": "cb5acf770b02bb1ebcc3338d9af609f88b1a704f" } From 563721058890e299b28167e309f687fcd4ad4b80 Mon Sep 17 00:00:00 2001 From: hatayama Date: Tue, 27 Oct 2026 15:20:06 +0900 Subject: [PATCH 09/12] Match an Editor whose project path names the same directory A command's project root has its symlinks evaluated, while an Editor's command line keeps the path it was started with; `uloop launch` itself passes the unevaluated path. The lookup compared the two as strings, so an Editor opened through a symlinked directory was never found for its own project. That used to cost only a NotRunning from `uloop status`, but the lookup now decides UNITY_NOT_RUNNING and ends compile and other waits, so every domain reload in such a project would have been reported as an exit. A candidate whose path differs as a string now still matches when both paths stat to the same directory, which also covers a case-insensitive file system accepting either spelling. --- cli/common/unityprocess/process.go | 18 +++++++++++++++++- .../unityprocess/process_branches_test.go | 16 ++++++++++++++++ cli/dispatcher/shared-inputs-stamp.json | 2 +- cli/project-runner/shared-inputs-stamp.json | 2 +- 4 files changed, 35 insertions(+), 3 deletions(-) diff --git a/cli/common/unityprocess/process.go b/cli/common/unityprocess/process.go index 716563c2bd..d6ee3b4e71 100644 --- a/cli/common/unityprocess/process.go +++ b/cli/common/unityprocess/process.go @@ -7,6 +7,7 @@ import ( "encoding/binary" "errors" "fmt" + "os" "os/exec" "path" "path/filepath" @@ -86,7 +87,7 @@ func selectProjectProcess(processes []UnityProcess, projectRoot string) (*UnityP if err != nil { continue } - if normalizedCandidate == normalizedTarget { + if normalizedCandidate == normalizedTarget || isSameDirectory(candidate.projectPath, projectRoot) { process := candidate return &process, unidentifiedRunning, nil } @@ -94,6 +95,21 @@ func selectProjectProcess(processes []UnityProcess, projectRoot string) (*UnityP return nil, unidentifiedRunning, nil } +// isSameDirectory reports whether both paths name one existing directory. Why on top of the string +// comparison: a command's project root has its symlinks evaluated, while an Editor's command line keeps +// the path it was started with, and a case-insensitive file system accepts either spelling. +func isSameDirectory(first string, second string) bool { + firstInfo, err := os.Stat(first) + if err != nil { + return false + } + secondInfo, err := os.Stat(second) + if err != nil { + return false + } + return os.SameFile(firstInfo, secondInfo) +} + func listUnityProcesses(ctx context.Context) ([]UnityProcess, error) { switch runtime.GOOS { case "darwin": diff --git a/cli/common/unityprocess/process_branches_test.go b/cli/common/unityprocess/process_branches_test.go index f04362402b..f32a2c2a70 100644 --- a/cli/common/unityprocess/process_branches_test.go +++ b/cli/common/unityprocess/process_branches_test.go @@ -67,6 +67,22 @@ func TestSelectProjectProcess(t *testing.T) { } } +// Verifies an Editor started with a path through a symlink is matched to the project root a command +// resolves, which has its symlinks evaluated, so the lookup does not report that Editor as missing. +func TestSelectProjectProcessMatchesAPathThroughASymlink(t *testing.T) { + projectRoot := t.TempDir() + linkedRoot := filepath.Join(t.TempDir(), "linked") + if err := os.Symlink(projectRoot, linkedRoot); err != nil { + t.Skipf("creating a symlink is not permitted here: %v", err) + } + + process, _, err := selectProjectProcess([]UnityProcess{{Pid: 7, projectPath: linkedRoot}}, projectRoot) + + if err != nil || process == nil || process.Pid != 7 { + t.Fatalf("expected the Editor started through the symlink, got %#v, %v", process, err) + } +} + // Verifies a procargs2 buffer whose last argv entry lacks its NUL terminator keeps only the complete entries. func TestParseMacProcArgs2StopsAtUnterminatedArgument(t *testing.T) { buf := make([]byte, 4) diff --git a/cli/dispatcher/shared-inputs-stamp.json b/cli/dispatcher/shared-inputs-stamp.json index 307346c698..f4074c3755 100644 --- a/cli/dispatcher/shared-inputs-stamp.json +++ b/cli/dispatcher/shared-inputs-stamp.json @@ -1,4 +1,4 @@ { "schemaVersion": 1, - "sharedInputsHash": "27e41300f8d098b84e3a080ff62d01ce80808804" + "sharedInputsHash": "21bb76e9ca81f1867406700f22ef2c3b25691e13" } diff --git a/cli/project-runner/shared-inputs-stamp.json b/cli/project-runner/shared-inputs-stamp.json index 1850ef3ffd..68ca5c57c0 100644 --- a/cli/project-runner/shared-inputs-stamp.json +++ b/cli/project-runner/shared-inputs-stamp.json @@ -1,4 +1,4 @@ { "schemaVersion": 1, - "sharedInputsHash": "cb5acf770b02bb1ebcc3338d9af609f88b1a704f" + "sharedInputsHash": "bf3dad5b3d8b33396c2c3c1ca76bfa7294268584" } From 6b67b07d2124d69bcb8184a52703ea71ee6c0235 Mon Sep 17 00:00:00 2001 From: hatayama Date: Tue, 27 Oct 2026 15:25:28 +0900 Subject: [PATCH 10/12] Point at the Editor log only when it is the exited Editor's The abnormal-exit step named Unity's default Editor.log, but every Editor start renames Editor.log to Editor-prev.log and replaces the previous one. With several Editors open, Editor.log usually belongs to whichever started last, so the step sent the caller to another project's log for the reason this Editor exited. The log now counts as this Editor's only when the project path Unity writes near its top ("Successfully changed project path to: ...") names the project, checking Editor.log and then Editor-prev.log. When neither is, no log step is given. The project path comparison is shared with the process lookup, so a path through a symlink matches here too. --- cli/common/clicore/unity_exit.go | 14 +++- cli/common/clicore/unity_exit_test.go | 27 ++++++- cli/common/errors/unity_not_running.go | 25 +++---- cli/common/errors/unity_not_running_test.go | 16 ++-- cli/common/unityprocess/editor_files.go | 65 +++++++++++++++++ cli/common/unityprocess/editor_files_test.go | 73 +++++++++++++++++++ cli/common/unityprocess/process.go | 23 ++++-- cli/dispatcher/shared-inputs-stamp.json | 2 +- .../internal/projectrunner/status_command.go | 4 +- .../projectrunner/status_command_test.go | 14 +++- cli/project-runner/shared-inputs-stamp.json | 2 +- 11 files changed, 224 insertions(+), 41 deletions(-) diff --git a/cli/common/clicore/unity_exit.go b/cli/common/clicore/unity_exit.go index 09a1708017..6813b35c6c 100644 --- a/cli/common/clicore/unity_exit.go +++ b/cli/common/clicore/unity_exit.go @@ -18,20 +18,26 @@ const ( ) // NewUnityNotRunningError describes a project whose Unity Editor a successful process lookup did not -// find, noting whether the Editor left its lockfile behind. exitedDuringCommand says the command had -// reached Unity before it went away. +// find, noting whether the Editor left its lockfile behind and, if so, where its log is. exitedDuringCommand +// says the command had reached Unity before it went away. func NewUnityNotRunningError( projectRoot string, endpoint string, cause error, exitedDuringCommand bool, ) clierrors.UnityNotRunningError { + leftBehind := lockfileLeftBehind(projectRoot) + editorLogPath := "" + // Why only after an abnormal exit: the log is pointed at only then, and finding it reads two logs. + if leftBehind { + editorLogPath = unityprocess.EditorLogPathFor(projectRoot) + } return clierrors.UnityNotRunningError{ ProjectRoot: projectRoot, Endpoint: endpoint, Cause: cause, - LockfileLeftBehind: lockfileLeftBehind(projectRoot), - EditorLogPath: unityprocess.DefaultEditorLogPath(), + LockfileLeftBehind: leftBehind, + EditorLogPath: editorLogPath, ExitedDuringCommand: exitedDuringCommand, } } diff --git a/cli/common/clicore/unity_exit_test.go b/cli/common/clicore/unity_exit_test.go index 08ec73d40e..296aa9a60c 100644 --- a/cli/common/clicore/unity_exit_test.go +++ b/cli/common/clicore/unity_exit_test.go @@ -175,8 +175,27 @@ func TestUnityExitWatchTreatsADroppedConnectionAsTheServerGoingAway(t *testing.T } } +// writeProjectEditorLogForTest points Unity's default log location at a temporary home and writes a log +// there that records projectRoot as its project, returning the log's path. +func writeProjectEditorLogForTest(t *testing.T, projectRoot string) string { + t.Helper() + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + t.Setenv("LOCALAPPDATA", home) + editorLogPath := unityprocess.DefaultEditorLogPath() + if err := os.MkdirAll(filepath.Dir(editorLogPath), 0o755); err != nil { + t.Fatalf("creating the log directory failed: %v", err) + } + content := "COMMAND LINE ARGUMENTS:\nSuccessfully changed project path to: " + projectRoot + "\n" + if err := os.WriteFile(editorLogPath, []byte(content), 0o644); err != nil { + t.Fatalf("writing the Editor log failed: %v", err) + } + return editorLogPath +} + // Verifies the not-running error records a lockfile left behind, which only a shutdown that did not -// finish leaves, along with the Editor log location. +// finish leaves, along with the log that Editor wrote. func TestNewUnityNotRunningErrorRecordsALockfileLeftBehind(t *testing.T) { projectRoot := t.TempDir() lockfilePath := unityprocess.LockfilePath(projectRoot) @@ -187,13 +206,15 @@ func TestNewUnityNotRunningErrorRecordsALockfileLeftBehind(t *testing.T) { t.Fatalf("writing the lockfile failed: %v", err) } + editorLogPath := writeProjectEditorLogForTest(t, projectRoot) + err := NewUnityNotRunningError(projectRoot, "/tmp/uloop/sample.sock", io.EOF, true) if !err.LockfileLeftBehind { t.Fatal("expected the lockfile left behind to be recorded") } - if err.EditorLogPath != unityprocess.DefaultEditorLogPath() { - t.Fatalf("EditorLogPath = %q, want the default Editor log path", err.EditorLogPath) + if err.EditorLogPath != editorLogPath { + t.Fatalf("EditorLogPath = %q, want the project's Editor log %q", err.EditorLogPath, editorLogPath) } if err.ProjectRoot != projectRoot || err.Endpoint != "/tmp/uloop/sample.sock" || err.Cause != io.EOF || !err.ExitedDuringCommand { diff --git a/cli/common/errors/unity_not_running.go b/cli/common/errors/unity_not_running.go index b7e7196b80..c26c5db559 100644 --- a/cli/common/errors/unity_not_running.go +++ b/cli/common/errors/unity_not_running.go @@ -15,7 +15,8 @@ type UnityNotRunningError struct { // LockfileLeftBehind is true when Temp/UnityLockfile, which Unity removes when it shuts down normally, // is still there. LockfileLeftBehind bool - // EditorLogPath is Unity's default Editor log location; empty when it could not be resolved. + // EditorLogPath is the log this Editor wrote at Unity's default location; empty when no log there is + // this Editor's. EditorLogPath string // ExitedDuringCommand is true when this command had reached Unity before it went away, so the command // may have partly run. @@ -67,10 +68,7 @@ func (err UnityNotRunningError) message(launching bool) string { } func (err UnityNotRunningError) nextActions(launching bool, safeToRetry bool) []string { - actions := []string{} - if err.LockfileLeftBehind { - actions = append(actions, err.EditorLogNextAction()) - } + actions := err.EditorLogNextActions() if launching { return append(actions, "Run `uloop launch` again.") } @@ -80,15 +78,16 @@ func (err UnityNotRunningError) nextActions(launching bool, safeToRetry bool) [] return append(actions, "Run `uloop launch` to start Unity, then run the command again.") } -// EditorLogNextAction is the step that sends the caller to the Editor log before launching Unity again. -// Why before the launch step: Unity renames Editor.log to Editor-prev.log when it starts, and keeps only -// that one previous log, so a second launch loses the record of why the Editor went away. -func (err UnityNotRunningError) EditorLogNextAction() string { - if err.EditorLogPath == "" { - return "Before launching, read the end of Unity's Editor log for why it exited; the next launch renames it to Editor-prev.log." +// EditorLogNextActions returns the step that sends the caller to this Editor's log before launching Unity +// again, or no step when the Editor shut down normally or no log of it was found. +// Why before the launch step: every Editor start renames Editor.log to Editor-prev.log and replaces the +// previous Editor-prev.log, so launching first can lose the record of why the Editor went away. +func (err UnityNotRunningError) EditorLogNextActions() []string { + if !err.LockfileLeftBehind || err.EditorLogPath == "" { + return []string{} } - return "Before launching, read the end of the Editor log for why it exited: " + err.EditorLogPath + - " (the next launch renames it to Editor-prev.log)." + return []string{"Before launching, read the end of this Editor's log for why it exited: " + err.EditorLogPath + + " (the next Editor start moves or replaces it)."} } func (err UnityNotRunningError) details() map[string]any { diff --git a/cli/common/errors/unity_not_running_test.go b/cli/common/errors/unity_not_running_test.go index 3f76177273..9cb7526e57 100644 --- a/cli/common/errors/unity_not_running_test.go +++ b/cli/common/errors/unity_not_running_test.go @@ -51,7 +51,7 @@ func TestUnityNotRunningErrorBeforeTheCommandReachedUnity(t *testing.T) { } // Verifies a lockfile left behind is reported as an abnormal exit without asserting a crash, and that -// the Editor log is named before launch because the next launch moves it to Editor-prev.log. +// the Editor's own log is named before launch because the next Editor start moves or replaces it. func TestUnityNotRunningErrorPointsAtTheEditorLogWhenTheLockfileWasLeftBehind(t *testing.T) { err := UnityNotRunningError{ ProjectRoot: "/projects/sample", @@ -69,7 +69,7 @@ func TestUnityNotRunningErrorPointsAtTheEditorLogWhenTheLockfileWasLeftBehind(t t.Fatalf("expected the log step and the launch step, got %#v", cliErr.NextActions) } if !strings.Contains(cliErr.NextActions[0], unityNotRunningTestEditorLog) || - !strings.Contains(cliErr.NextActions[0], "Editor-prev.log") { + !strings.Contains(cliErr.NextActions[0], "next Editor start") { t.Fatalf("expected the Editor log step first, got %q", cliErr.NextActions[0]) } if !strings.Contains(cliErr.NextActions[1], "`uloop launch`") { @@ -80,18 +80,18 @@ func TestUnityNotRunningErrorPointsAtTheEditorLogWhenTheLockfileWasLeftBehind(t } } -// Verifies the Editor log step still appears, without a path, when the default log location could not -// be resolved. -func TestUnityNotRunningErrorNamesTheEditorLogWithoutAPath(t *testing.T) { +// Verifies no Editor log step appears when no log at the default location is this Editor's, since +// pointing at another Editor's log would send the caller to the wrong reason for the exit. +func TestUnityNotRunningErrorOmitsTheEditorLogStepWithoutTheEditorsLog(t *testing.T) { err := UnityNotRunningError{LockfileLeftBehind: true} cliErr := err.ToCLIError(ErrorContext{Command: "get-logs"}) - if len(cliErr.NextActions) != 2 || !strings.Contains(cliErr.NextActions[0], "Editor log") { - t.Fatalf("expected an Editor log step, got %#v", cliErr.NextActions) + if len(cliErr.NextActions) != 1 || !strings.Contains(cliErr.NextActions[0], "`uloop launch`") { + t.Fatalf("expected the launch step only, got %#v", cliErr.NextActions) } if _, ok := cliErr.Details["EditorLog"]; ok { - t.Fatalf("an unresolved log path must not appear in Details, got %#v", cliErr.Details) + t.Fatalf("no log path may appear in Details, got %#v", cliErr.Details) } } diff --git a/cli/common/unityprocess/editor_files.go b/cli/common/unityprocess/editor_files.go index 7c58f6a833..9f8851db36 100644 --- a/cli/common/unityprocess/editor_files.go +++ b/cli/common/unityprocess/editor_files.go @@ -1,9 +1,12 @@ package unityprocess import ( + "errors" + "io" "os" "path/filepath" "runtime" + "strings" ) const ( @@ -29,6 +32,68 @@ func DefaultEditorLogPath() string { return editorLogPathFor(runtime.GOOS, homeDir, os.Getenv("LOCALAPPDATA")) } +const ( + previousEditorLogName = "Editor-prev.log" + // editorLogProjectPathMarker starts the line an Editor writes near the top of its log once it has + // opened a project. + editorLogProjectPathMarker = "Successfully changed project path to: " + // editorLogHeadBytes bounds how much of a log is read to find its project. The marker line follows the + // command line arguments within the first few kilobytes, and a log can grow to many megabytes. + editorLogHeadBytes = 64 * 1024 +) + +// EditorLogPathFor returns the log the Editor of the project at projectRoot wrote at the default location, +// or an empty string when no log there is that Editor's. +// Why either log can miss: every Editor start renames Editor.log to Editor-prev.log, so the log moves once +// any other Editor starts and is gone after a second start, and -logFile writes it elsewhere. +func EditorLogPathFor(projectRoot string) string { + return projectEditorLogPath(DefaultEditorLogPath(), projectRoot) +} + +func projectEditorLogPath(editorLogPath string, projectRoot string) string { + if editorLogPath == "" { + return "" + } + previousLogPath := filepath.Join(filepath.Dir(editorLogPath), previousEditorLogName) + for _, candidate := range []string{editorLogPath, previousLogPath} { + if editorLogBelongsTo(candidate, projectRoot) { + return candidate + } + } + return "" +} + +func editorLogBelongsTo(logPath string, projectRoot string) bool { + file, err := os.Open(logPath) + if err != nil { + return false + } + defer func() { _ = file.Close() }() + + head := make([]byte, editorLogHeadBytes) + readBytes, err := io.ReadFull(file, head) + if err != nil && !errors.Is(err, io.ErrUnexpectedEOF) && !errors.Is(err, io.EOF) { + return false + } + loggedProjectPath := loggedEditorProjectPath(string(head[:readBytes])) + return loggedProjectPath != "" && isProjectPath(loggedProjectPath, projectRoot) +} + +// loggedEditorProjectPath returns the project path a log head records, or an empty string when it records +// none. +func loggedEditorProjectPath(head string) string { + markerIndex := strings.Index(head, editorLogProjectPathMarker) + if markerIndex < 0 { + return "" + } + line := head[markerIndex+len(editorLogProjectPathMarker):] + if lineEnd := strings.IndexByte(line, '\n'); lineEnd >= 0 { + line = line[:lineEnd] + } + // Why TrimSpace: a log written on Windows ends the line with "\r\n". + return strings.TrimSpace(line) +} + // editorLogPathFor takes the platform and its base directories as parameters so every platform's // layout can be checked on any host. func editorLogPathFor(goos string, homeDir string, localAppData string) string { diff --git a/cli/common/unityprocess/editor_files_test.go b/cli/common/unityprocess/editor_files_test.go index 8da266dc64..b8a53685dd 100644 --- a/cli/common/unityprocess/editor_files_test.go +++ b/cli/common/unityprocess/editor_files_test.go @@ -1,7 +1,9 @@ package unityprocess import ( + "os" "path/filepath" + "strings" "testing" ) @@ -44,3 +46,74 @@ func TestEditorLogPathForEachPlatform(t *testing.T) { }) } } + +// writeEditorLogForTest writes an Editor log whose head records projectPath as Unity does at startup, +// with the given line ending. +func writeEditorLogForTest(t *testing.T, logPath string, projectPath string, lineEnding string) { + t.Helper() + lines := []string{ + "COMMAND LINE ARGUMENTS:", + "Unity", + "-projectPath", + projectPath, + "Successfully changed project path to: " + projectPath, + "Loading GUID <-> Path mappings...", + } + if err := os.WriteFile(logPath, []byte(strings.Join(lines, lineEnding)+lineEnding), 0o644); err != nil { + t.Fatalf("writing %s failed: %v", logPath, err) + } +} + +// Verifies the Editor log that names the project is picked whether it is still Editor.log or a later +// Editor start already renamed it to Editor-prev.log, and that logs of other projects, which every later +// start leaves at both names, are never picked. +func TestProjectEditorLogPath(t *testing.T) { + projectRoot := t.TempDir() + otherProject := t.TempDir() + cases := []struct { + name string + currentLog string + previousLog string + lineEnding string + wantPrevious bool + wantNone bool + }{ + {name: "Editor.log is the project's", currentLog: projectRoot, previousLog: otherProject, lineEnding: "\n"}, + {name: "renamed to Editor-prev.log", currentLog: otherProject, previousLog: projectRoot, lineEnding: "\n", wantPrevious: true}, + {name: "Windows line endings", currentLog: projectRoot, lineEnding: "\r\n"}, + {name: "both are other projects'", currentLog: otherProject, previousLog: otherProject, lineEnding: "\n", wantNone: true}, + {name: "no log at all", lineEnding: "\n", wantNone: true}, + } + for _, testCase := range cases { + t.Run(testCase.name, func(t *testing.T) { + logDirectory := t.TempDir() + currentLogPath := filepath.Join(logDirectory, "Editor.log") + previousLogPath := filepath.Join(logDirectory, "Editor-prev.log") + if testCase.currentLog != "" { + writeEditorLogForTest(t, currentLogPath, testCase.currentLog, testCase.lineEnding) + } + if testCase.previousLog != "" { + writeEditorLogForTest(t, previousLogPath, testCase.previousLog, testCase.lineEnding) + } + want := currentLogPath + if testCase.wantPrevious { + want = previousLogPath + } + if testCase.wantNone { + want = "" + } + + if got := projectEditorLogPath(currentLogPath, projectRoot); got != want { + t.Fatalf("projectEditorLogPath() = %q, want %q", got, want) + } + }) + } +} + +// Verifies no log is picked when the default log location could not be resolved, instead of looking in +// the working directory. +func TestProjectEditorLogPathWithoutADefaultLocation(t *testing.T) { + if got := projectEditorLogPath("", t.TempDir()); got != "" { + t.Fatalf("projectEditorLogPath() = %q, want no log", got) + } +} diff --git a/cli/common/unityprocess/process.go b/cli/common/unityprocess/process.go index d6ee3b4e71..25a29fd5b9 100644 --- a/cli/common/unityprocess/process.go +++ b/cli/common/unityprocess/process.go @@ -70,8 +70,7 @@ func findRunningUnityProcess(ctx context.Context, projectRoot string) (*UnityPro // selectProjectProcess picks projectRoot's Editor from the listed Editors, and reports whether an Editor // whose project is unknown was listed. func selectProjectProcess(processes []UnityProcess, projectRoot string) (*UnityProcess, bool, error) { - normalizedTarget, err := normalizeComparablePath(projectRoot) - if err != nil { + if _, err := normalizeComparablePath(projectRoot); err != nil { return nil, false, err } @@ -83,11 +82,7 @@ func selectProjectProcess(processes []UnityProcess, projectRoot string) (*UnityP unidentifiedRunning = true continue } - normalizedCandidate, err := normalizeComparablePath(candidate.projectPath) - if err != nil { - continue - } - if normalizedCandidate == normalizedTarget || isSameDirectory(candidate.projectPath, projectRoot) { + if isProjectPath(candidate.projectPath, projectRoot) { process := candidate return &process, unidentifiedRunning, nil } @@ -95,6 +90,20 @@ func selectProjectProcess(processes []UnityProcess, projectRoot string) (*UnityP return nil, unidentifiedRunning, nil } +// isProjectPath reports whether path, as an Editor's command line or log records it, names the project at +// projectRoot. +func isProjectPath(path string, projectRoot string) bool { + normalizedPath, err := normalizeComparablePath(path) + if err != nil { + return false + } + normalizedRoot, err := normalizeComparablePath(projectRoot) + if err != nil { + return false + } + return normalizedPath == normalizedRoot || isSameDirectory(path, projectRoot) +} + // isSameDirectory reports whether both paths name one existing directory. Why on top of the string // comparison: a command's project root has its symlinks evaluated, while an Editor's command line keeps // the path it was started with, and a case-insensitive file system accepts either spelling. diff --git a/cli/dispatcher/shared-inputs-stamp.json b/cli/dispatcher/shared-inputs-stamp.json index f4074c3755..caa3d4eb8a 100644 --- a/cli/dispatcher/shared-inputs-stamp.json +++ b/cli/dispatcher/shared-inputs-stamp.json @@ -1,4 +1,4 @@ { "schemaVersion": 1, - "sharedInputsHash": "21bb76e9ca81f1867406700f22ef2c3b25691e13" + "sharedInputsHash": "d0bf9515ba91cfa45e18a6ae0219b5a1c644f0f8" } diff --git a/cli/project-runner/internal/projectrunner/status_command.go b/cli/project-runner/internal/projectrunner/status_command.go index 7ef950b254..a7d5de0700 100644 --- a/cli/project-runner/internal/projectrunner/status_command.go +++ b/cli/project-runner/internal/projectrunner/status_command.go @@ -277,9 +277,7 @@ func reportFromUnreachableServer( func notRunningStatusReport(projectRoot string, probeErr error) statusReport { notRunning := clicore.NewUnityNotRunningError(projectRoot, "", probeErr, false) report := newStatusReport(statusStateNotRunning, notRunning.ToCLIError(clierrors.ErrorContext{}).Message) - if notRunning.LockfileLeftBehind { - report.NextActions = append([]string{notRunning.EditorLogNextAction()}, report.NextActions...) - } + report.NextActions = append(notRunning.EditorLogNextActions(), report.NextActions...) return report } diff --git a/cli/project-runner/internal/projectrunner/status_command_test.go b/cli/project-runner/internal/projectrunner/status_command_test.go index 0fbadcf065..9f414ea47a 100644 --- a/cli/project-runner/internal/projectrunner/status_command_test.go +++ b/cli/project-runner/internal/projectrunner/status_command_test.go @@ -449,6 +449,18 @@ func TestRunStatusCommandReportsAnAbnormalExitWhenTheLockfileWasLeftBehind(t *te if err := os.WriteFile(lockfilePath, []byte{}, 0o644); err != nil { t.Fatalf("writing the lockfile failed: %v", err) } + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + t.Setenv("LOCALAPPDATA", home) + editorLogPath := unityprocess.DefaultEditorLogPath() + if err := os.MkdirAll(filepath.Dir(editorLogPath), 0o755); err != nil { + t.Fatalf("creating the log directory failed: %v", err) + } + logContent := "COMMAND LINE ARGUMENTS:\nSuccessfully changed project path to: " + projectRoot + "\n" + if err := os.WriteFile(editorLogPath, []byte(logContent), 0o644); err != nil { + t.Fatalf("writing the Editor log failed: %v", err) + } deps := statusTestDeps(t) deps.isUnityProcessRunning = fakeUnityProcessLookup(t, projectRoot, false, nil) @@ -463,7 +475,7 @@ func TestRunStatusCommandReportsAnAbnormalExitWhenTheLockfileWasLeftBehind(t *te if len(nextActions) != 2 { t.Fatalf("NextActions = %#v, want the Editor log step and the launch step", report["NextActions"]) } - if first, _ := nextActions[0].(string); !strings.Contains(first, "Editor log") { + if first, _ := nextActions[0].(string); !strings.Contains(first, editorLogPath) { t.Fatalf("NextActions[0] = %q, want the Editor log step", first) } if second, _ := nextActions[1].(string); second != "Start it with uloop launch." { diff --git a/cli/project-runner/shared-inputs-stamp.json b/cli/project-runner/shared-inputs-stamp.json index 68ca5c57c0..89dbc1ca66 100644 --- a/cli/project-runner/shared-inputs-stamp.json +++ b/cli/project-runner/shared-inputs-stamp.json @@ -1,4 +1,4 @@ { "schemaVersion": 1, - "sharedInputsHash": "bf3dad5b3d8b33396c2c3c1ca76bfa7294268584" + "sharedInputsHash": "0ffa3644ee31ba39f749c07a1f6e57e459828b08" } From 4960cdb1b14b23a06b95a3eae45ad4239a824a4a Mon Sep 17 00:00:00 2001 From: hatayama Date: Tue, 27 Oct 2026 15:34:31 +0900 Subject: [PATCH 11/12] Restart the play mode wait's exit count after an answer The play mode state wait fed only failed polls to its exit watch, so a status answer between two lost connections did not restart the watch's count of lookups that found no Editor. Every other wait observes each poll, an answer included, and ends only after two lookups in a row find nothing; this one could end after two lookups in total. --- .../projectrunner/control_play_mode_wait.go | 8 +- .../unity_exit_scripted_server_unix_test.go | 74 +++++++++++++++++++ 2 files changed, 79 insertions(+), 3 deletions(-) create mode 100644 cli/project-runner/internal/projectrunner/unity_exit_scripted_server_unix_test.go diff --git a/cli/project-runner/internal/projectrunner/control_play_mode_wait.go b/cli/project-runner/internal/projectrunner/control_play_mode_wait.go index 84a9cade36..8ba9cd2dca 100644 --- a/cli/project-runner/internal/projectrunner/control_play_mode_wait.go +++ b/cli/project-runner/internal/projectrunner/control_play_mode_wait.go @@ -180,6 +180,11 @@ func waitForControlPlayModeState( defer ticker.Stop() for { response, err := requestControlPlayModeStatus(waitContext, connection, action) + // Why every poll, an answer included: an answer restarts the watch's count of lookups that + // found no Editor. + if exitErr := exitWatch.Observe(waitContext, err); exitErr != nil { + return lastResponse, false, exitErr + } if err == nil { lastResponse = response hasResponse = true @@ -191,9 +196,6 @@ func waitForControlPlayModeState( } } else { lastErr = err - if exitErr := exitWatch.Observe(waitContext, err); exitErr != nil { - return lastResponse, false, exitErr - } } select { diff --git a/cli/project-runner/internal/projectrunner/unity_exit_scripted_server_unix_test.go b/cli/project-runner/internal/projectrunner/unity_exit_scripted_server_unix_test.go new file mode 100644 index 0000000000..65ec2dcf2e --- /dev/null +++ b/cli/project-runner/internal/projectrunner/unity_exit_scripted_server_unix_test.go @@ -0,0 +1,74 @@ +//go:build !windows + +package projectrunner + +import ( + "bufio" + "context" + "errors" + "testing" + "time" + + "github.com/hatayama/unity-cli-loop/common/clicore" + clierrors "github.com/hatayama/unity-cli-loop/common/errors" + "github.com/hatayama/unity-cli-loop/common/unityipc" + "github.com/hatayama/unity-cli-loop/common/unityprocess" +) + +// scriptedDropServerConnection serves one connection per entry of results, in order: an empty entry reads +// the request and drops the connection, as a server that went away does, and any other entry answers with +// that result. Every connection after the script is dropped too. +func scriptedDropServerConnection(t *testing.T, results []string) unityipc.Connection { + t.Helper() + listener := newLoopbackIpcListener(t) + go func() { + for index := 0; ; index++ { + conn, err := listener.Accept() + if err != nil { + return + } + if _, err := unityipc.Read(bufio.NewReader(conn)); err == nil && index < len(results) && results[index] != "" { + _ = unityipc.Write(conn, []byte(`{"jsonrpc":"2.0","result":`+results[index]+`,"id":1}`)) + } + _ = conn.Close() + } + }() + return unityipc.Connection{ + ProjectRoot: t.TempDir(), + Endpoint: unityipc.Endpoint{Network: "tcp", Address: listener.Addr().String()}, + } +} + +// Verifies a status answer between two lost connections restarts the play mode wait's exit count, so an +// Editor that answered in between is not taken as gone after only two lookups in total. +func TestWaitForControlPlayModeStateRestartsTheExitCountAfterAnAnswer(t *testing.T) { + originalPoll := controlPlayModeStatePoll + controlPlayModeStatePoll = time.Millisecond + t.Cleanup(func() { controlPlayModeStatePoll = originalPoll }) + connection := scriptedDropServerConnection(t, []string{"", `{"IsPlaying":false}`}) + lookups := 0 + clock := time.Unix(1_700_000_000, 0) + exitWatch := clicore.NewUnityExitWatch( + connection.ProjectRoot, + "", + func(context.Context, string) (*unityprocess.UnityProcess, error) { + lookups++ + return nil, nil + }, + func() time.Time { + // Two seconds per reading keeps every lookup past the watch's spacing. + clock = clock.Add(2 * time.Second) + return clock + }, + ) + + _, _, err := waitForControlPlayModeState(context.Background(), connection, "Play", 5*time.Second, exitWatch) + + var notRunning clierrors.UnityNotRunningError + if !errors.As(err, ¬Running) { + t.Fatalf("err = %v, want UnityNotRunningError once the connection keeps dropping", err) + } + if lookups != 3 { + t.Fatalf("lookups = %d, want 3: one before the answer, then two that confirm the exit", lookups) + } +} From cb82a539d798bf0a1080398508bc08b298d95a88 Mon Sep 17 00:00:00 2001 From: hatayama Date: Tue, 27 Oct 2026 15:34:42 +0900 Subject: [PATCH 12/12] Cover the not-running wiring and the definitive lookup with tests Three parts of the UNITY_NOT_RUNNING path could be undone with every test still passing: the send path's check for an Editor that exited after dispatch, the mapping of an Editor of unknown project to ErrUnidentifiedUnityEditor, and the defaults that pick the definitive lookup. The last one carries the guarantee that an unidentified Editor falls back to the behavior from before. - A scripted server now drops a dispatched request through the full send path and expects UnityNotRunningError. - The mapping moves into selectProjectProcessDefinitively and is tested for each answer. - `uloop status` now goes through the same package lookup as the other defaults, and tests pin that lookup and the readiness wait's default to the definitive one. --- cli/common/clicore/tool_readiness_test.go | 11 ++++++ cli/common/unityprocess/process.go | 24 +++++++------ .../unityprocess/process_branches_test.go | 33 +++++++++++++++++ cli/dispatcher/shared-inputs-stamp.json | 2 +- .../internal/projectrunner/status_command.go | 2 +- .../unity_exit_scripted_server_unix_test.go | 35 +++++++++++++++++++ .../unity_process_lookup_helpers_test.go | 20 +++++++++++ cli/project-runner/shared-inputs-stamp.json | 2 +- 8 files changed, 116 insertions(+), 13 deletions(-) diff --git a/cli/common/clicore/tool_readiness_test.go b/cli/common/clicore/tool_readiness_test.go index be503cbb85..9724034bf7 100644 --- a/cli/common/clicore/tool_readiness_test.go +++ b/cli/common/clicore/tool_readiness_test.go @@ -7,6 +7,7 @@ import ( "net" "os" "path/filepath" + "reflect" "strings" "syscall" "testing" @@ -290,3 +291,13 @@ func TestWaitForToolReadinessStopsOnceUnityHasExited(t *testing.T) { t.Fatalf("expected UnityNotRunningError, got %v", err) } } + +// Verifies the readiness wait's default lookup is the one that reports an Editor of unknown project +// instead of "none", since its exit watch concludes from "none" that Unity is not running. +func TestDefaultToolReadinessLookupDoesNotTakeAnUnidentifiedEditorAsNone(t *testing.T) { + lookup := defaultToolReadinessDeps().findRunningUnityProcess + + if reflect.ValueOf(lookup).Pointer() != reflect.ValueOf(unityprocess.FindRunningUnityProcessDefinitively).Pointer() { + t.Fatal("the default lookup must be unityprocess.FindRunningUnityProcessDefinitively") + } +} diff --git a/cli/common/unityprocess/process.go b/cli/common/unityprocess/process.go index 25a29fd5b9..45a509598d 100644 --- a/cli/common/unityprocess/process.go +++ b/cli/common/unityprocess/process.go @@ -44,7 +44,11 @@ type RestoreFocusFunc func(context.Context) error // FindRunningUnityProcess returns the running Editor listed for projectRoot, or nil when none is. func FindRunningUnityProcess(ctx context.Context, projectRoot string) (*UnityProcess, error) { - process, _, err := findRunningUnityProcess(ctx, projectRoot) + processes, err := listUnityProcesses(ctx) + if err != nil { + return nil, err + } + process, _, err := selectProjectProcess(processes, projectRoot) return process, err } @@ -52,19 +56,19 @@ func FindRunningUnityProcess(ctx context.Context, projectRoot string) (*UnityPro // result that no Editor is running for the project: it returns nil only when no running Editor can be // the project's, and ErrUnidentifiedUnityEditor when an Editor whose project is unknown may be. func FindRunningUnityProcessDefinitively(ctx context.Context, projectRoot string) (*UnityProcess, error) { - process, unidentifiedRunning, err := findRunningUnityProcess(ctx, projectRoot) - if err == nil && process == nil && unidentifiedRunning { - return nil, ErrUnidentifiedUnityEditor + processes, err := listUnityProcesses(ctx) + if err != nil { + return nil, err } - return process, err + return selectProjectProcessDefinitively(processes, projectRoot) } -func findRunningUnityProcess(ctx context.Context, projectRoot string) (*UnityProcess, bool, error) { - processes, err := listUnityProcesses(ctx) - if err != nil { - return nil, false, err +func selectProjectProcessDefinitively(processes []UnityProcess, projectRoot string) (*UnityProcess, error) { + process, unidentifiedRunning, err := selectProjectProcess(processes, projectRoot) + if err == nil && process == nil && unidentifiedRunning { + return nil, ErrUnidentifiedUnityEditor } - return selectProjectProcess(processes, projectRoot) + return process, err } // selectProjectProcess picks projectRoot's Editor from the listed Editors, and reports whether an Editor diff --git a/cli/common/unityprocess/process_branches_test.go b/cli/common/unityprocess/process_branches_test.go index f32a2c2a70..687fc97fb7 100644 --- a/cli/common/unityprocess/process_branches_test.go +++ b/cli/common/unityprocess/process_branches_test.go @@ -67,6 +67,39 @@ func TestSelectProjectProcess(t *testing.T) { } } +// Verifies the lookup for callers that conclude Unity is not running answers "none" only when no listed +// Editor can be the project's: an Editor of unknown project turns "none" into ErrUnidentifiedUnityEditor, +// while the project's own Editor is still returned beside one. +func TestSelectProjectProcessDefinitively(t *testing.T) { + projectRoot := t.TempDir() + unidentified := UnityProcess{Pid: 1} + project := UnityProcess{Pid: 2, projectPath: projectRoot} + other := UnityProcess{Pid: 3, projectPath: filepath.Join(projectRoot, "Other")} + cases := []struct { + name string + processes []UnityProcess + wantPid int + wantErr error + }{ + {name: "unidentified Editor only", processes: []UnityProcess{other, unidentified}, wantErr: ErrUnidentifiedUnityEditor}, + {name: "project Editor beside an unidentified one", processes: []UnityProcess{unidentified, project}, wantPid: 2}, + {name: "no Editor can be the project's", processes: []UnityProcess{other}}, + } + for _, testCase := range cases { + t.Run(testCase.name, func(t *testing.T) { + process, err := selectProjectProcessDefinitively(testCase.processes, projectRoot) + + gotPid := 0 + if process != nil { + gotPid = process.Pid + } + if gotPid != testCase.wantPid || !errors.Is(err, testCase.wantErr) || (testCase.wantErr == nil && err != nil) { + t.Fatalf("pid = %d, err = %v, want pid = %d, err = %v", gotPid, err, testCase.wantPid, testCase.wantErr) + } + }) + } +} + // Verifies an Editor started with a path through a symlink is matched to the project root a command // resolves, which has its symlinks evaluated, so the lookup does not report that Editor as missing. func TestSelectProjectProcessMatchesAPathThroughASymlink(t *testing.T) { diff --git a/cli/dispatcher/shared-inputs-stamp.json b/cli/dispatcher/shared-inputs-stamp.json index caa3d4eb8a..3ae049d319 100644 --- a/cli/dispatcher/shared-inputs-stamp.json +++ b/cli/dispatcher/shared-inputs-stamp.json @@ -1,4 +1,4 @@ { "schemaVersion": 1, - "sharedInputsHash": "d0bf9515ba91cfa45e18a6ae0219b5a1c644f0f8" + "sharedInputsHash": "201f7fc23d2f35969582d5f290edaf13a1556929" } diff --git a/cli/project-runner/internal/projectrunner/status_command.go b/cli/project-runner/internal/projectrunner/status_command.go index a7d5de0700..014b548713 100644 --- a/cli/project-runner/internal/projectrunner/status_command.go +++ b/cli/project-runner/internal/projectrunner/status_command.go @@ -50,7 +50,7 @@ func defaultStatusCommandDeps() statusCommandDeps { probeTimeout: 5 * time.Second, reprobeDelay: 1 * time.Second, isUnityProcessRunning: func(ctx context.Context, projectRoot string) (bool, error) { - process, err := unityprocess.FindRunningUnityProcessDefinitively(ctx, projectRoot) + process, err := unityProcessLookup(ctx, projectRoot) return process != nil, err }, } diff --git a/cli/project-runner/internal/projectrunner/unity_exit_scripted_server_unix_test.go b/cli/project-runner/internal/projectrunner/unity_exit_scripted_server_unix_test.go index 65ec2dcf2e..1ef4fe178a 100644 --- a/cli/project-runner/internal/projectrunner/unity_exit_scripted_server_unix_test.go +++ b/cli/project-runner/internal/projectrunner/unity_exit_scripted_server_unix_test.go @@ -72,3 +72,38 @@ func TestWaitForControlPlayModeStateRestartsTheExitCountAfterAnAnswer(t *testing t.Fatalf("lookups = %d, want 3: one before the answer, then two that confirm the exit", lookups) } } + +// Verifies a connection that drops after the request reached Unity comes back from the send as +// UnityNotRunningError when no Editor process is left, rather than as a disconnect to wait out. +func TestSendWithTransientConnectionRetryReportsAnEditorThatExitedAfterDispatch(t *testing.T) { + deps := defaultConnectionRetryDeps() + deps.findRunningUnityProcess = noUnityProcessLookup + listener := newLoopbackIpcListener(t) + go func() { + conn, err := listener.Accept() + if err != nil { + return + } + defer func() { _ = conn.Close() }() + if _, err := unityipc.Read(bufio.NewReader(conn)); err != nil { + return + } + accepted := `{"jsonrpc":"2.0","result":{"accepted":true},"uloop":{"phase":"accepted"},"id":1}` + _ = unityipc.Write(conn, []byte(accepted)) + }() + connection := unityipc.Connection{ + ProjectRoot: t.TempDir(), + Endpoint: unityipc.Endpoint{Network: "tcp", Address: listener.Addr().String()}, + } + + outcome, err := sendWithTransientConnectionRetryWithDeps( + context.Background(), connection, "compile", map[string]any{}, nil, 0, deps) + + var notRunning clierrors.UnityNotRunningError + if !errors.As(err, ¬Running) || !notRunning.ExitedDuringCommand { + t.Fatalf("err = %v, want UnityNotRunningError for an exit during the command", err) + } + if !outcome.RequestDispatched { + t.Fatalf("outcome = %+v, want a dispatched request", outcome) + } +} diff --git a/cli/project-runner/internal/projectrunner/unity_process_lookup_helpers_test.go b/cli/project-runner/internal/projectrunner/unity_process_lookup_helpers_test.go index 56f60bd287..92f1f19ba4 100644 --- a/cli/project-runner/internal/projectrunner/unity_process_lookup_helpers_test.go +++ b/cli/project-runner/internal/projectrunner/unity_process_lookup_helpers_test.go @@ -2,7 +2,9 @@ package projectrunner import ( "context" + "errors" "os" + "reflect" "testing" "github.com/hatayama/unity-cli-loop/common/unityprocess" @@ -37,3 +39,21 @@ func runningUnityProcessLookup(context.Context, string) (*unityprocess.UnityProc func noUnityProcessLookup(context.Context, string) (*unityprocess.UnityProcess, error) { return nil, nil } + +// Verifies the lookup behind every default dependency set, status included, is the one that reports an +// Editor of unknown project instead of "none", since those callers conclude from "none" that Unity is +// not running. +func TestDefaultUnityProcessLookupDoesNotTakeAnUnidentifiedEditorAsNone(t *testing.T) { + if reflect.ValueOf(unityProcessLookup).Pointer() != reflect.ValueOf(unityprocess.FindRunningUnityProcessDefinitively).Pointer() { + t.Fatal("the default lookup must be unityprocess.FindRunningUnityProcessDefinitively") + } + useUnityProcessLookup(t, func(context.Context, string) (*unityprocess.UnityProcess, error) { + return nil, unityprocess.ErrUnidentifiedUnityEditor + }) + + _, err := defaultStatusCommandDeps().isUnityProcessRunning(context.Background(), t.TempDir()) + + if !errors.Is(err, unityprocess.ErrUnidentifiedUnityEditor) { + t.Fatalf("status lookup err = %v, want the default lookup's answer", err) + } +} diff --git a/cli/project-runner/shared-inputs-stamp.json b/cli/project-runner/shared-inputs-stamp.json index 89dbc1ca66..466b358c17 100644 --- a/cli/project-runner/shared-inputs-stamp.json +++ b/cli/project-runner/shared-inputs-stamp.json @@ -1,4 +1,4 @@ { "schemaVersion": 1, - "sharedInputsHash": "0ffa3644ee31ba39f749c07a1f6e57e459828b08" + "sharedInputsHash": "30c21c3eae97d957604131bdd7f60b201fd1b64a" }