From a5fd68fbd7e0a762babd6daea8032095438f62c1 Mon Sep 17 00:00:00 2001 From: hatayama Date: Mon, 27 Jul 2026 10:10:39 +0900 Subject: [PATCH 1/4] Abort dial retries on a connect the OS refused permanently A connect refused with EPERM or EACCES cannot become reachable while the caller waits, but both dial-retry paths treated every connection attempt error as transient. The project runner spent its full 60-second unity-alive window on it and then reported the window's own deadline as `i/o timeout`, so the syscall error the first attempt already had never reached the caller; the readiness wait behaved the same way over its own timeout. The reported envelope then advised waiting and retrying a condition that never clears, which is what sent a 2026-07-26 investigation after a healthy Editor. Both loops now stop at the first refused connect and report that error, and its envelope states the syscall text verbatim with Retryable and SafeToRetry false plus guidance pointing at sandboxing and socket permissions. --- cli/common/clicore/tool_readiness.go | 6 + cli/common/clicore/tool_readiness_test.go | 34 ++++++ .../errors/error_envelope_classification.go | 27 +++++ cli/common/errors/error_envelope_test.go | 32 +++++ cli/common/errors/transport_errors.go | 9 ++ cli/common/errors/transport_errors_test.go | 37 ++++++ .../projectrunner/connection_retry.go | 8 +- .../projectrunner/connection_retry_test.go | 109 ++++++++++++++++++ 8 files changed, 261 insertions(+), 1 deletion(-) diff --git a/cli/common/clicore/tool_readiness.go b/cli/common/clicore/tool_readiness.go index 9686e06973..2dfdbc7ca6 100644 --- a/cli/common/clicore/tool_readiness.go +++ b/cli/common/clicore/tool_readiness.go @@ -61,6 +61,12 @@ func waitForToolReadinessWithDeps(ctx context.Context, projectRoot string, timeo if IsReadinessCLIUpdateRequiredError(err) { return err } + // Why abort: polling cannot outlast a connect the kernel refused permanently, and + // waiting out the timeout replaces that syscall error with server-not-responding + // guidance the caller cannot act on. + if clierrors.IsPermanentConnectError(err) { + return err + } lastErr = err } diff --git a/cli/common/clicore/tool_readiness_test.go b/cli/common/clicore/tool_readiness_test.go index bad93a6557..0421a02c1c 100644 --- a/cli/common/clicore/tool_readiness_test.go +++ b/cli/common/clicore/tool_readiness_test.go @@ -4,6 +4,9 @@ import ( "context" "encoding/json" "errors" + "net" + "os" + "syscall" "testing" "time" @@ -53,6 +56,37 @@ func TestWaitForToolReadinessReturnsCliUpdateRequiredImmediately(t *testing.T) { } } +// Verifies a connect() the operating system refused permanently ends the readiness wait at the +// first probe and reports that error, instead of polling out the whole timeout and replacing it +// with server-not-responding guidance the caller cannot act on. +func TestWaitForToolReadinessReturnsPermanentlyRefusedConnectImmediately(t *testing.T) { + expectedErr := &unityipc.ConnectionAttemptError{ + Endpoint: "/tmp/uloop-501/UnityCliLoop-sample.sock", + Cause: &net.OpError{ + Op: "dial", + Net: "unix", + Addr: &net.UnixAddr{Name: "/tmp/uloop-501/UnityCliLoop-sample.sock", Net: "unix"}, + Err: os.NewSyscallError("connect", syscall.EPERM), + }, + } + probeCount := 0 + deps := toolReadinessDeps{ + probeToolReadinessSequence: func(context.Context, string) error { + probeCount++ + return expectedErr + }, + } + + err := waitForToolReadinessWithDeps(context.Background(), t.TempDir(), time.Hour, deps) + + if !errors.Is(err, expectedErr) { + t.Fatalf("expected the refused connect error, got %v", err) + } + if probeCount != 1 { + t.Fatalf("expected the wait to stop after the first probe, got %d probes", probeCount) + } +} + // Verifies that parent cancellation is preserved instead of being reported as a timeout. func TestToolReadinessDoneErrorPropagatesParentCancellation(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) diff --git a/cli/common/errors/error_envelope_classification.go b/cli/common/errors/error_envelope_classification.go index 9942c37841..c3ecd2f107 100644 --- a/cli/common/errors/error_envelope_classification.go +++ b/cli/common/errors/error_envelope_classification.go @@ -89,6 +89,9 @@ func unityServerNotRespondingCLIError(err UnityServerNotRespondingError, context } func connectionAttemptCLIError(err *unityipc.ConnectionAttemptError, context ErrorContext) CLIError { + if IsPermanentConnectError(err) { + return refusedConnectionAttemptCLIError(err, context) + } return CLIError{ ErrorCode: ErrorCodeUnityNotReachable, Phase: ErrorPhaseConnection, @@ -109,6 +112,30 @@ func connectionAttemptCLIError(err *unityipc.ConnectionAttemptError, context Err } } +// Reports a connect the operating system refused permanently. Waiting changes nothing here, so +// the envelope states the syscall error as it came back and points at what actually blocks the +// socket instead of repeating the reachability guidance. +func refusedConnectionAttemptCLIError(err *unityipc.ConnectionAttemptError, context ErrorContext) CLIError { + return CLIError{ + ErrorCode: ErrorCodeUnityNotReachable, + Phase: ErrorPhaseConnection, + Message: "The operating system refused the connection to the Unity CLI Loop server for this project.", + Retryable: false, + SafeToRetry: false, + ProjectRoot: firstNonEmpty(context.ProjectRoot, err.ProjectRoot), + Command: context.Command, + NextActions: []string{ + "Retrying will not help: the connection was refused before it reached Unity, and the Editor never saw it.", + "If this command ran inside a sandbox (for example an AI agent's sandboxed shell), the sandbox denied the connection; run it with sandboxing disabled for this command.", + "Otherwise check the ownership and permissions of the endpoint path in Details.", + }, + Details: map[string]any{ + "Endpoint": err.Endpoint, + "Cause": connectionAttemptCause(err), + }, + } +} + func classifyRPCError(rpcErr *unityipc.RPCError, context ErrorContext) CLIError { details, decodedData := rpcErrorDetails(rpcErr) switch RPCDataType(decodedData) { diff --git a/cli/common/errors/error_envelope_test.go b/cli/common/errors/error_envelope_test.go index a788e4eae7..f46f75172f 100644 --- a/cli/common/errors/error_envelope_test.go +++ b/cli/common/errors/error_envelope_test.go @@ -4,7 +4,10 @@ import ( "bytes" "encoding/json" "errors" + "net" + "os" "strings" + "syscall" "testing" "github.com/hatayama/unity-cli-loop/common/unityipc" @@ -65,6 +68,35 @@ func TestClassifyConnectionAttemptError(t *testing.T) { } } +// Verifies a connect() the operating system refused outright is reported as a permanent +// failure carrying the syscall text verbatim: the retry guidance sent the agent into a +// 60-second wait for a condition (sandbox policy, socket permissions) that never clears. +func TestClassifyConnectionAttemptErrorForRefusedConnect(t *testing.T) { + err := &unityipc.ConnectionAttemptError{ + ProjectRoot: "/tmp/MyProject", + Endpoint: "/tmp/uloop-501/UnityCliLoop-sample.sock", + Cause: &net.OpError{ + Op: "dial", + Net: "unix", + Addr: &net.UnixAddr{Name: "/tmp/uloop-501/UnityCliLoop-sample.sock", Net: "unix"}, + Err: os.NewSyscallError("connect", syscall.EPERM), + }, + } + + cliErr := ClassifyError(err, ErrorContext{Command: "compile"}) + if cliErr.Retryable || cliErr.SafeToRetry { + t.Fatalf("a permanently refused connect must not be advertised as retryable: %#v", cliErr) + } + if cliErr.Details["Cause"] != err.Cause.Error() { + t.Fatalf("the syscall error must be reported verbatim: %#v", cliErr.Details) + } + for _, action := range cliErr.NextActions { + if strings.Contains(strings.ToLower(action), "wait and retry") { + t.Fatalf("next actions must not advise waiting: %#v", cliErr.NextActions) + } + } +} + func TestClassifyConnectionAttemptAllowsNilCause(t *testing.T) { // Verifies connection classification handles a missing low-level cause. err := &unityipc.ConnectionAttemptError{ diff --git a/cli/common/errors/transport_errors.go b/cli/common/errors/transport_errors.go index ac9955a3cc..db11c0e8f2 100644 --- a/cli/common/errors/transport_errors.go +++ b/cli/common/errors/transport_errors.go @@ -42,6 +42,15 @@ func IsTransportDisconnectError(err error) bool { strings.Contains(message, "use of closed network connection") } +// Reports whether a dial failed for a reason that cannot clear while the caller waits: the +// kernel refused the socket outright (a sandbox policy that denies Unix socket connects, +// permissions on the socket path) instead of reporting that nobody is listening yet. Retrying +// such an error wastes the whole dial-retry window and then reports the window's own deadline +// expiry, so the syscall error the first attempt already had never reaches the caller. +func IsPermanentConnectError(err error) bool { + return errors.Is(err, syscall.EPERM) || errors.Is(err, syscall.EACCES) +} + // 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 00025b082c..89c6ee685f 100644 --- a/cli/common/errors/transport_errors_test.go +++ b/cli/common/errors/transport_errors_test.go @@ -94,6 +94,43 @@ func TestIsTransportDisconnectErrorMatchesWrappedNoResponseError(t *testing.T) { } } +// Verifies a connect() refused by the operating system is classified as permanent, so the +// dial-retry loops abort instead of spending their window on an error that cannot clear. +func TestIsPermanentConnectErrorMatchesRefusedSyscalls(t *testing.T) { + refusedSyscalls := []syscall.Errno{syscall.EPERM, syscall.EACCES} + + for _, errno := range refusedSyscalls { + dialError := &net.OpError{ + Op: "dial", + Net: "unix", + Addr: &net.UnixAddr{Name: "/tmp/uloop-501/UnityCliLoop-sample.sock", Net: "unix"}, + Err: os.NewSyscallError("connect", errno), + } + wrapped := &unityipc.ConnectionAttemptError{Cause: dialError} + if !IsPermanentConnectError(wrapped) { + t.Fatalf("refused connect was not classified as permanent: %v", wrapped) + } + } +} + +// Verifies the errors a retry is meant to absorb — the socket not existing yet, nobody +// listening yet, a deadline expiry — stay retryable. +func TestIsPermanentConnectErrorRejectsTransientFailures(t *testing.T) { + transientErrors := []error{ + nil, + &unityipc.ConnectionAttemptError{Cause: os.NewSyscallError("connect", syscall.ENOENT)}, + &unityipc.ConnectionAttemptError{Cause: os.NewSyscallError("connect", syscall.ECONNREFUSED)}, + &unityipc.ConnectionAttemptError{Cause: timeoutOnlyError{}}, + fmt.Errorf("dial unix /tmp/uloop-501/UnityCliLoop-sample.sock: i/o timeout"), + } + + for _, err := range transientErrors { + if IsPermanentConnectError(err) { + t.Fatalf("transient error was classified as permanent: %v", err) + } + } +} + // Verifies that final-response timeout classification matches typed deadline errors // and Timeout()-reporting errors instead of relying on the "i/o timeout" message. func TestIsFinalResponseTimeoutErrorMatchesTypedCauses(t *testing.T) { diff --git a/cli/project-runner/internal/projectrunner/connection_retry.go b/cli/project-runner/internal/projectrunner/connection_retry.go index 6c163230b9..9c0a593598 100644 --- a/cli/project-runner/internal/projectrunner/connection_retry.go +++ b/cli/project-runner/internal/projectrunner/connection_retry.go @@ -358,7 +358,13 @@ func shouldRetryUndispatchedConnection(err error, outcome unityipc.UnitySendOutc } var connectionErr *unityipc.ConnectionAttemptError - return errors.As(err, &connectionErr) + if !errors.As(err, &connectionErr) { + return false + } + // The retry window exists for a server that is not listening yet. A connect the kernel + // refused permanently never becomes reachable inside it, and retrying it replaces the + // syscall error with the window's own deadline expiry. + return !clierrors.IsPermanentConnectError(connectionErr) } func logConnectionRetryFocusAttempt( diff --git a/cli/project-runner/internal/projectrunner/connection_retry_test.go b/cli/project-runner/internal/projectrunner/connection_retry_test.go index 779d6423dd..c6d90ab28d 100644 --- a/cli/project-runner/internal/projectrunner/connection_retry_test.go +++ b/cli/project-runner/internal/projectrunner/connection_retry_test.go @@ -10,6 +10,7 @@ import ( "path/filepath" "runtime" "strings" + "syscall" "testing" "time" @@ -390,6 +391,114 @@ func TestConnectionRetryFocusControllerRetriesAfterProcessDiscoveryMiss(t *testi } } +// Verifies a connect() the operating system refused permanently is not retried. Retrying it +// burned the whole 60-second window and then reported the window's own deadline error, hiding +// the real syscall error the first attempt already had. +func TestShouldNotRetryPermanentlyRefusedConnection(t *testing.T) { + err := &unityipc.ConnectionAttemptError{ + Endpoint: "/tmp/uloop-501/UnityCliLoop-sample.sock", + Cause: &net.OpError{ + Op: "dial", + Net: "unix", + Addr: &net.UnixAddr{Name: "/tmp/uloop-501/UnityCliLoop-sample.sock", Net: "unix"}, + Err: os.NewSyscallError("connect", syscall.EPERM), + }, + } + + if shouldRetryUndispatchedConnection(err, unityipc.UnitySendOutcome{}) { + t.Fatal("a permanently refused connect must not enter the retry loop") + } +} + +// Verifies the whole send path fails at the first attempt when the operating system refuses the +// connect, and reports that syscall error itself. Retrying it consumed the extended +// unity-alive window and then reported the window's own deadline as `i/o timeout`. +func TestSendWithTransientConnectionRetryAbortsOnRefusedConnect(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("unix socket file permissions do not apply to named pipes") + } + if os.Geteuid() == 0 { + t.Skip("root bypasses socket file permissions") + } + + // Why a real listening socket with mode 000: the refusal has to come from the kernel's + // connect, which is the only thing that produces the errno this path classifies. The + // directory comes from MkdirTemp rather than t.TempDir because a socket path built from this + // test's name exceeds the sockaddr_un limit. + endpointDirectory, err := os.MkdirTemp("", "uloop-refused") + if err != nil { + t.Fatalf("failed to create the endpoint directory: %v", err) + } + t.Cleanup(func() { + _ = os.RemoveAll(endpointDirectory) + }) + socketPath := filepath.Join(endpointDirectory, "refused.sock") + listener, listenErr := net.Listen("unix", socketPath) + if listenErr != nil { + t.Fatalf("failed to listen on the endpoint: %v", listenErr) + } + defer func() { + _ = listener.Close() + }() + if err := os.Chmod(socketPath, 0o000); err != nil { + t.Fatalf("failed to remove socket permissions: %v", err) + } + + deps := defaultConnectionRetryDeps() + deps.findRunningUnityProcess = func(context.Context, string) (*clicore.UnityProcess, error) { + return &clicore.UnityProcess{Pid: 123}, nil + } + connection := unityipc.Connection{ + Endpoint: unityipc.Endpoint{Network: "unix", Address: socketPath}, + ProjectRoot: t.TempDir(), + } + + startedAt := time.Now() + _, sendErr := sendWithTransientConnectionRetryWithDeps( + context.Background(), + connection, + "get-logs", + map[string]any{}, + nil, + 0, + deps) + elapsed := time.Since(startedAt) + + var connectionErr *unityipc.ConnectionAttemptError + if !errors.As(sendErr, &connectionErr) { + t.Fatalf("expected the connection attempt error, got %v", sendErr) + } + var notRespondingErr clierrors.UnityServerNotRespondingError + if errors.As(sendErr, ¬RespondingErr) { + t.Fatalf("a refused connect must not be reported as a non-responding server: %v", sendErr) + } + if !clierrors.IsPermanentConnectError(sendErr) { + t.Fatalf("the syscall error was replaced on the way out: %v", sendErr) + } + if elapsed >= deps.retryPoll { + t.Fatalf("expected the send to abort before the first retry wait, took %s", elapsed) + } +} + +// Verifies the dial failures the retry window exists for — the socket not created yet, nobody +// listening yet — keep being retried. +func TestShouldRetryTransientlyFailedConnection(t *testing.T) { + transientCauses := []error{ + os.NewSyscallError("connect", syscall.ENOENT), + os.NewSyscallError("connect", syscall.ECONNREFUSED), + } + + for _, cause := range transientCauses { + err := &unityipc.ConnectionAttemptError{ + Endpoint: "/tmp/uloop-501/UnityCliLoop-sample.sock", + Cause: cause, + } + if !shouldRetryUndispatchedConnection(err, unityipc.UnitySendOutcome{}) { + t.Fatalf("a transient dial failure must stay retryable: %v", cause) + } + } +} + // Verifies accepted RPCs can outlive the pre-dispatch connection retry timeout. func TestSendWithTransientConnectionRetryDoesNotCancelAcceptedRequestAtRetryTimeout(t *testing.T) { if runtime.GOOS == "windows" { From 28efe2369fe41965167eec0272a9515b44ca94c5 Mon Sep 17 00:00:00 2001 From: hatayama Date: Mon, 27 Jul 2026 10:10:48 +0900 Subject: [PATCH 2/4] Update the sandbox document for the new refused-connect report and transport scope The Symptom section described the old misdiagnosis as current behavior, which would now read as a bug against the fixed CLI. It also implied the block was a Unix socket property, and that reading produced the wrong inference that a TCP transport would get through: with the default allowedHosts policy the sandbox stops localhost TCP too, and a plain `uloop` command succeeds only because excludedCommands removes it from the sandbox. The stamp files move because this change touches non-test cli/common sources, which CI requires to reach every affected release. --- cli/dispatcher/shared-inputs-stamp.json | 2 +- cli/project-runner/shared-inputs-stamp.json | 2 +- docs/claude-code-sandbox.md | 17 +++++++++++++---- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/cli/dispatcher/shared-inputs-stamp.json b/cli/dispatcher/shared-inputs-stamp.json index d7905df2ab..10901d236d 100644 --- a/cli/dispatcher/shared-inputs-stamp.json +++ b/cli/dispatcher/shared-inputs-stamp.json @@ -1,4 +1,4 @@ { "schemaVersion": 1, - "sharedInputsHash": "1f44f61e2cada92cf5f4ecba945c1ec2400b0dc3" + "sharedInputsHash": "667b8efddc774a83ea10201938098209858ab6d1" } diff --git a/cli/project-runner/shared-inputs-stamp.json b/cli/project-runner/shared-inputs-stamp.json index 5ad8b3a9b0..7174cb942a 100644 --- a/cli/project-runner/shared-inputs-stamp.json +++ b/cli/project-runner/shared-inputs-stamp.json @@ -1,4 +1,4 @@ { "schemaVersion": 1, - "sharedInputsHash": "679552d1e9b44928dc92f0b49ff7476f7f49d60d" + "sharedInputsHash": "249e6a48ac6be5b9cd66a1fc9598c05054b46d57" } diff --git a/docs/claude-code-sandbox.md b/docs/claude-code-sandbox.md index 7caada7536..885eb64884 100644 --- a/docs/claude-code-sandbox.md +++ b/docs/claude-code-sandbox.md @@ -12,10 +12,13 @@ all along" — this document exists so nobody walks that path again. sees the connection attempt (server-side logs are VibeLogger-based and exist only when the `ULOOP_DEBUG` scripting define is set — do not read missing log lines as evidence either way). - Commands that never touch the Editor (`uloop --version`, `uloop --help`) work normally. -- The reported error is misleading: the CLI retries for 60 seconds and then reports - `dial unix ...: i/o timeout`, while the underlying per-attempt error is - `connect: operation not permitted` (EPERM). Fixing that misdiagnosis is tracked as its own - work item (report the permanent errno verbatim and stop advising retries). +- The reported error names the refusal: the command fails on the first attempt with + `ErrorCode: UNITY_NOT_REACHABLE`, `Retryable: false`, `SafeToRetry: false`, and + `Details.Cause` carrying the syscall error verbatim + (`dial unix ...: connect: operation not permitted`). Its next actions point at sandboxing and + socket permissions, not at waiting. An older CLI instead retried for 60 seconds and then + reported `dial unix ...: i/o timeout` with retry guidance — that misdiagnosis is what cost the + 2026-07-26 investigation, so an `i/o timeout` here means the CLI predates the fix. ## Cause @@ -27,6 +30,12 @@ digits of the SHA-256 of the canonical project root) through that policy — `co access to the socket's directory does not help; this was verified empirically: a directory the sandbox allowed file writes into still refused a socket `bind()`. +The block is not specific to the transport: with the default `allowedHosts` policy the sandbox +also stops V2's localhost TCP connection (verified 2026-07-27), so a plain `uloop ...` command +succeeding proves only that `excludedCommands` took it out of the sandbox — not that Unix sockets +are the problem and TCP would get through. What is specific to a Unix socket is only that it has +no hostname to put on `allowedHosts`, so that escape hatch does not exist for it. + This is specific to the sandboxed shell. The same command in a normal terminal, or in a session without sandboxing, is unaffected. Windows uses a named pipe instead of a Unix socket; whether the sandbox blocks named-pipe connects the same way has not been verified — treat an From 8fff41cec4ddc8483b132110cfaaa5d32cdb1fe3 Mon Sep 17 00:00:00 2001 From: hatayama Date: Mon, 27 Jul 2026 10:41:50 +0900 Subject: [PATCH 3/4] Cover Windows access denial and the pause-point wait in the permanent-connect abort Matching EPERM and EACCES left Windows out: a named pipe reports access denial as ERROR_ACCESS_DENIED, which maps to os.ErrPermission but to neither POSIX errno, so the refusal stayed retryable there and the 60-second window still replaced it with its own deadline error. The classification now tests os.ErrPermission, scoped to a connection attempt so file permission failures reaching the same callers are not mistaken for a refused dial. The pause-point wait dials on every poll as well, so a refused connect spent the whole --timeout before reporting itself; it now aborts at the first poll. The readiness test's own timeout is shorter than one poll interval, so a regression fails on its assertions instead of hanging until the package deadline. --- cli/common/clicore/tool_readiness_test.go | 12 +++- cli/common/errors/transport_errors.go | 11 +++- cli/common/errors/transport_errors_test.go | 31 +++++++++++ cli/dispatcher/shared-inputs-stamp.json | 2 +- .../projectrunner/pause_point_wait_poll.go | 7 +++ .../pause_point_wait_poll_test.go | 55 +++++++++++++++++++ cli/project-runner/shared-inputs-stamp.json | 2 +- 7 files changed, 114 insertions(+), 6 deletions(-) diff --git a/cli/common/clicore/tool_readiness_test.go b/cli/common/clicore/tool_readiness_test.go index 0421a02c1c..f078ad5c1a 100644 --- a/cli/common/clicore/tool_readiness_test.go +++ b/cli/common/clicore/tool_readiness_test.go @@ -75,12 +75,18 @@ func TestWaitForToolReadinessReturnsPermanentlyRefusedConnectImmediately(t *test probeCount++ return expectedErr }, + findRunningUnityProcess: func(context.Context, string) (*unityprocess.UnityProcess, error) { + return nil, nil + }, } - err := waitForToolReadinessWithDeps(context.Background(), t.TempDir(), time.Hour, deps) + // Why a timeout shorter than the poll interval: without the abort the wait falls through to + // its own timeout, and this test then fails on the assertions below rather than hanging until + // the package test deadline. + err := waitForToolReadinessWithDeps(context.Background(), t.TempDir(), ToolReadinessPoll/10, deps) - if !errors.Is(err, expectedErr) { - t.Fatalf("expected the refused connect error, got %v", err) + if err != error(expectedErr) { + t.Fatalf("expected the refused connect error itself, got %v", err) } if probeCount != 1 { t.Fatalf("expected the wait to stop after the first probe, got %d probes", probeCount) diff --git a/cli/common/errors/transport_errors.go b/cli/common/errors/transport_errors.go index db11c0e8f2..a0abd491f9 100644 --- a/cli/common/errors/transport_errors.go +++ b/cli/common/errors/transport_errors.go @@ -47,8 +47,17 @@ func IsTransportDisconnectError(err error) bool { // permissions on the socket path) instead of reporting that nobody is listening yet. Retrying // such an error wastes the whole dial-retry window and then reports the window's own deadline // expiry, so the syscall error the first attempt already had never reaches the caller. +// Why os.ErrPermission rather than the POSIX errnos: a Windows named pipe reports access denial +// as ERROR_ACCESS_DENIED, which maps to os.ErrPermission but matches neither EPERM nor EACCES, so +// an errno test would leave Windows retrying a refusal that never clears. Why only inside a +// connection attempt: os.ErrPermission also covers file permission failures that reach the same +// callers (project resolution, endpoint inspection), and those are not dial outcomes. func IsPermanentConnectError(err error) bool { - return errors.Is(err, syscall.EPERM) || errors.Is(err, syscall.EACCES) + var connectionErr *unityipc.ConnectionAttemptError + if !errors.As(err, &connectionErr) { + return false + } + return errors.Is(connectionErr, os.ErrPermission) } // Reports whether the error is a connection deadline expiry. The Timeout() probe diff --git a/cli/common/errors/transport_errors_test.go b/cli/common/errors/transport_errors_test.go index 89c6ee685f..bc6c8627a5 100644 --- a/cli/common/errors/transport_errors_test.go +++ b/cli/common/errors/transport_errors_test.go @@ -113,6 +113,37 @@ func TestIsPermanentConnectErrorMatchesRefusedSyscalls(t *testing.T) { } } +// Verifies a named pipe access denial is classified as permanent too. go-winio reports it as a +// path error whose cause maps to os.ErrPermission and to neither POSIX errno, so matching errnos +// alone would leave Windows retrying a refusal that never clears. +func TestIsPermanentConnectErrorMatchesNamedPipeAccessDenial(t *testing.T) { + deniedPipe := &unityipc.ConnectionAttemptError{ + Cause: &os.PathError{ + Op: "open", + Path: `\\.\pipe\UnityCliLoop-sample`, + Err: os.ErrPermission, + }, + } + + if !IsPermanentConnectError(deniedPipe) { + t.Fatalf("denied named pipe was not classified as permanent: %v", deniedPipe) + } +} + +// Verifies a permission failure that is not a dial outcome stays out of this classification: the +// same callers also surface project and endpoint file errors, and those must not abort a wait. +func TestIsPermanentConnectErrorIgnoresPermissionErrorsOutsideDialing(t *testing.T) { + fileError := &os.PathError{ + Op: "open", + Path: "/tmp/MyProject/ProjectSettings/ProjectVersion.txt", + Err: syscall.EACCES, + } + + if IsPermanentConnectError(fileError) { + t.Fatalf("a file permission error was classified as a refused connect: %v", fileError) + } +} + // Verifies the errors a retry is meant to absorb — the socket not existing yet, nobody // listening yet, a deadline expiry — stay retryable. func TestIsPermanentConnectErrorRejectsTransientFailures(t *testing.T) { diff --git a/cli/dispatcher/shared-inputs-stamp.json b/cli/dispatcher/shared-inputs-stamp.json index 10901d236d..1ea01b792d 100644 --- a/cli/dispatcher/shared-inputs-stamp.json +++ b/cli/dispatcher/shared-inputs-stamp.json @@ -1,4 +1,4 @@ { "schemaVersion": 1, - "sharedInputsHash": "667b8efddc774a83ea10201938098209858ab6d1" + "sharedInputsHash": "b9bc288d3a76099a1fe4cee8843e510bf5bbe632" } 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 c5a835ce35..e9da76523e 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" + clierrors "github.com/hatayama/unity-cli-loop/common/errors" "github.com/hatayama/unity-cli-loop/common/unityipc" ) @@ -162,6 +163,12 @@ func waitForPausePointStatus( return response, state, triggerResult, nil } } else { + // Why abort: every poll dials again, so a connect the operating system refused + // permanently keeps failing for the whole --timeout and the refusal is reported only + // after that wait is spent. + if clierrors.IsPermanentConnectError(err) { + return lastResponse, "", triggerResult, err + } lastErr = err } diff --git a/cli/project-runner/internal/projectrunner/pause_point_wait_poll_test.go b/cli/project-runner/internal/projectrunner/pause_point_wait_poll_test.go index 63b88d220a..6882040ca0 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_wait_poll_test.go +++ b/cli/project-runner/internal/projectrunner/pause_point_wait_poll_test.go @@ -3,8 +3,13 @@ package projectrunner import ( "bytes" "context" + "errors" + "fmt" "io" + "net" + "os" "strings" + "syscall" "testing" "time" @@ -39,6 +44,56 @@ func pausePointArmedStatusResponse(id string) pausePointStatusResponse { } } +// Verifies a connect the operating system refused permanently aborts the wait at the first poll. +// Every poll dials again, so without the abort the refusal is reported only after the whole +// --timeout has been spent on an error that cannot clear. +func TestWaitForPausePointAbortsWhenTheConnectIsRefused(t *testing.T) { + originalQuery := queryPausePointStatus + originalPoll := pausePointStatusPoll + pausePointStatusPoll = time.Millisecond + defer func() { + queryPausePointStatus = originalQuery + pausePointStatusPoll = originalPoll + }() + + refusedConnect := &unityipc.ConnectionAttemptError{ + Endpoint: "/tmp/uloop-501/UnityCliLoop-sample.sock", + Cause: &net.OpError{ + Op: "dial", + Net: "unix", + Addr: &net.UnixAddr{Name: "/tmp/uloop-501/UnityCliLoop-sample.sock", Net: "unix"}, + Err: os.NewSyscallError("connect", syscall.EPERM), + }, + } + queryCount := 0 + queryPausePointStatus = func( + ctx context.Context, + connection unityipc.Connection, + id string, + ) (pausePointStatusResponse, error) { + queryCount++ + return pausePointStatusResponse{}, fmt.Errorf("pause point status query failed: %w", refusedConnect) + } + + startedAt := time.Now() + _, _, _, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + id: "jump", + timeoutSeconds: 60, + timeout: 10 * time.Second, + }) + elapsed := time.Since(startedAt) + + if !errors.Is(err, refusedConnect) { + t.Fatalf("expected the refused connect error, got %v", err) + } + if queryCount != 1 { + t.Fatalf("expected the wait to stop after the first poll, got %d polls", queryCount) + } + if elapsed >= 5*time.Second { + t.Fatalf("expected an early abort, waited %v of a 10s timeout", elapsed) + } +} + // Verifies a --trigger rejected by its own argument parsing aborts the wait immediately, instead of // waiting out the marker's --timeout-seconds, and reports the rejection on TriggerResult. func TestWaitForPausePointAbortsWhenTriggerRejectsArguments(t *testing.T) { diff --git a/cli/project-runner/shared-inputs-stamp.json b/cli/project-runner/shared-inputs-stamp.json index 7174cb942a..1c3005f88e 100644 --- a/cli/project-runner/shared-inputs-stamp.json +++ b/cli/project-runner/shared-inputs-stamp.json @@ -1,4 +1,4 @@ { "schemaVersion": 1, - "sharedInputsHash": "249e6a48ac6be5b9cd66a1fc9598c05054b46d57" + "sharedInputsHash": "f6f1ad4c29722943c9a66a64cfc68d706ff8b141" } From b8becdb8a303c824135a0b9d1be354b5ee638738 Mon Sep 17 00:00:00 2001 From: hatayama Date: Mon, 27 Jul 2026 10:48:20 +0900 Subject: [PATCH 4/4] Pin the Windows access-denied mapping with a Windows-only test The cross-platform test substitutes os.ErrPermission for the status go-winio actually returns, so it proves the classifier's branch but not the assumption underneath it: that Go maps ERROR_ACCESS_DENIED to that target. This test uses the status code itself, so the assumption fails loudly on the Windows CI job instead of leaving Windows silently retrying a refusal that never clears. --- .../errors/transport_errors_windows_test.go | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 cli/common/errors/transport_errors_windows_test.go diff --git a/cli/common/errors/transport_errors_windows_test.go b/cli/common/errors/transport_errors_windows_test.go new file mode 100644 index 0000000000..a7628f0124 --- /dev/null +++ b/cli/common/errors/transport_errors_windows_test.go @@ -0,0 +1,33 @@ +//go:build windows + +package clierrors + +import ( + "os" + "syscall" + "testing" + + "github.com/hatayama/unity-cli-loop/common/unityipc" +) + +// The status Windows returns when opening the project's named pipe is denied, and the one +// go-winio puts in the path error it hands back. +const windowsErrorAccessDenied = syscall.Errno(5) + +// Verifies the classification holds against the real Windows status code. The cross-platform test +// substitutes os.ErrPermission for it, which assumes the mapping Go's syscall package performs; +// this test fails if that assumption ever stops holding and Windows silently starts retrying a +// refusal that never clears. +func TestIsPermanentConnectErrorMatchesWindowsAccessDenied(t *testing.T) { + deniedPipe := &unityipc.ConnectionAttemptError{ + Cause: &os.PathError{ + Op: "open", + Path: `\\.\pipe\UnityCliLoop-sample`, + Err: windowsErrorAccessDenied, + }, + } + + if !IsPermanentConnectError(deniedPipe) { + t.Fatalf("denied named pipe was not classified as permanent: %v", deniedPipe) + } +}