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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -258,9 +258,15 @@ func sendWithTransientConnectionRetryWithDeps(
)
}

// A caller that cancelled the command gets that cancellation back, as everywhere else that
// waits on Unity. Why here: the probe below inherits the cancellation and fails, and its
// failure would otherwise be reported as an unreachable Unity — telling the user to launch
// an editor they never asked about, and recording a probe warning for their own Ctrl-C.
if ctx.Err() != nil {
return outcome, ctx.Err()
}
runningProcess, processErr := deps.findRunningUnityProcess(retryContext, connection.ProjectRoot)
if finished, finalOutcome, finalErr := finishUndispatchedRetryProbe(
ctx,
retryContext,
connection,
sendAttempt{
Expand Down
44 changes: 28 additions & 16 deletions cli/project-runner/internal/projectrunner/connection_retry_flow.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ 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"
"github.com/hatayama/unity-cli-loop/common/vibelog"
)

func newConnectionRetryClient(
Expand Down Expand Up @@ -93,8 +94,13 @@ type sendAttempt struct {
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
// failure goes to the CLI vibe log instead of replacing that error.
func finishUndispatchedRetryProbe(
ctx context.Context,
retryContext context.Context,
connection unityipc.Connection,
currentAttempt sendAttempt,
Expand All @@ -103,31 +109,37 @@ func finishUndispatchedRetryProbe(
lastAttempt sendAttempt,
) (bool, unityipc.UnitySendOutcome, error) {
if processErr != nil {
if retryContext.Err() == nil {
return true, currentAttempt.outcome, processErr
}
if ctx.Err() != nil {
return true, currentAttempt.outcome, ctx.Err()
}
// A busy response seen during the window is the truer diagnosis than a
// final dial cut short by the expiring retry context.
if isUnityServerBusyRPCError(lastAttempt.err) {
return true, lastAttempt.outcome, lastAttempt.err
}
return true, currentAttempt.outcome, newUnityServerNotRespondingError(connection, currentAttempt.err)
logUnityProcessProbeFailure(connection, currentAttempt.err, processErr)
}
if runningProcess != nil {
return false, currentAttempt.outcome, nil
}
// Same masking as the probe-error path: 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.
// 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) {
return true, lastAttempt.outcome, lastAttempt.err
}
return true, currentAttempt.outcome, currentAttempt.err
}

// Records a process probe that could not answer whether Unity is running. Why log it at all: the
// probe failure no longer shows up in the returned error, and it is the only clue that the
// diagnosis was decided without a process reading — a sysctl refusal on macOS, or a PowerShell
// launch failure or process-list timeout on Windows.
func logUnityProcessProbeFailure(connection unityipc.Connection, dialErr error, probeErr 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.",
Context: map[string]any{
"endpoint": connection.Endpoint.Address,
"dial_cause": clicore.ErrorMessage(dialErr),
"cause": clicore.ErrorMessage(probeErr),
},
CorrelationID: vibelog.NewCLIVibeCorrelationID(),
})
}

func finishUnityAliveRetryWait(
ctx context.Context,
retryContext context.Context,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,19 @@
package projectrunner

import (
"context"
"errors"
"os"
"path/filepath"
"strings"
"testing"

clierrors "github.com/hatayama/unity-cli-loop/common/errors"

"github.com/hatayama/unity-cli-loop/common/clicore"
"github.com/hatayama/unity-cli-loop/common/unityipc"
"github.com/hatayama/unity-cli-loop/common/unityprocess"
"github.com/hatayama/unity-cli-loop/common/vibelog"
)

// Verifies only execute-dynamic-code gets main-thread stall tolerance: other commands'
Expand All @@ -19,3 +29,147 @@ func TestCommandNeedsSelfInducedStallToleranceOnlyForExecuteDynamicCode(t *testi
t.Fatal("expected run-tests to not need self-induced stall tolerance")
}
}

func refusedDialAttempt() sendAttempt {
return sendAttempt{
outcome: unityipc.UnitySendOutcome{},
err: &unityipc.ConnectionAttemptError{
ProjectRoot: "/projects/sample",
Endpoint: "/tmp/uloop/sample.sock",
Cause: errors.New("dial unix /tmp/uloop/sample.sock: connect: connection refused"),
},
}
}

func expiredRetryContext() context.Context {
expired, cancel := context.WithCancel(context.Background())
cancel()
return expired
}

// Verifies a failed process probe never upgrades the diagnosis to "Unity is running": the probe
// observed nothing, so the dial error must be reported exactly as it is on the no-process path.
func TestFinishUndispatchedRetryProbeDoesNotClaimUnityIsRunningWhenTheProbeFailed(t *testing.T) {
currentAttempt := refusedDialAttempt()

finished, _, err := finishUndispatchedRetryProbe(
expiredRetryContext(),
unityipc.Connection{ProjectRoot: t.TempDir()},
currentAttempt,
errors.New("sysctl kern.proc.all: operation not permitted"),
nil,
sendAttempt{},
)

if !finished {
t.Fatal("expected the retry loop to finish after a failed probe with an expired window")
}
var notResponding clierrors.UnityServerNotRespondingError
if errors.As(err, &notResponding) {
t.Fatalf("a failed probe must not report Unity as running: %v", err)
}
if err != currentAttempt.err {
t.Fatalf("expected the dial error verbatim, got: %v", err)
}
}

// Verifies the same fallback applies while the retry window is still alive: the probe failure
// alone would hide the dial error, which is the fact the caller acts on.
func TestFinishUndispatchedRetryProbeReportsTheDialErrorWhileTheWindowIsAlive(t *testing.T) {
currentAttempt := refusedDialAttempt()
probeErr := errors.New("listing Unity processes timed out")

finished, _, err := finishUndispatchedRetryProbe(
context.Background(),
unityipc.Connection{ProjectRoot: t.TempDir()},
currentAttempt,
probeErr,
nil,
sendAttempt{},
)

if !finished {
t.Fatal("expected the retry loop to finish after a failed probe")
}
if err != currentAttempt.err {
t.Fatalf("expected the dial error verbatim, got: %v", err)
}
}

// Verifies a busy response seen earlier in the window still wins over the final dial error when
// the probe failed, because a server that answered moments ago is the truer diagnosis.
func TestFinishUndispatchedRetryProbeKeepsABusyResponseWhenTheProbeFailed(t *testing.T) {
busyAttempt := sendAttempt{
err: &unityipc.RPCError{
Code: -32603,
Message: "Unity is busy running 'compile'.",
Data: []byte(`{"type":"server_busy"}`),
},
}

finished, _, err := finishUndispatchedRetryProbe(
expiredRetryContext(),
unityipc.Connection{ProjectRoot: t.TempDir()},
refusedDialAttempt(),
errors.New("sysctl kern.proc.all: operation not permitted"),
nil,
busyAttempt,
)

if !finished {
t.Fatal("expected the retry loop to finish after a failed probe with an expired window")
}
if err != busyAttempt.err {
t.Fatalf("expected the busy response to be preserved, got: %v", err)
}
}

// Verifies a probe that found a running process still lets the retry loop continue.
func TestFinishUndispatchedRetryProbeContinuesWhenUnityIsRunning(t *testing.T) {
finished, _, err := finishUndispatchedRetryProbe(
context.Background(),
unityipc.Connection{ProjectRoot: t.TempDir()},
refusedDialAttempt(),
nil,
&unityprocess.UnityProcess{Pid: 4321},
sendAttempt{},
)

if finished {
t.Fatalf("expected the retry loop to continue while Unity is running, got: %v", err)
}
if err != nil {
t.Fatalf("expected no error while continuing, got: %v", err)
}
}

// 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) {
projectRoot := t.TempDir()
t.Setenv(vibelog.CLIVibeLogEnvName, "1")

_, _, _ = finishUndispatchedRetryProbe(
expiredRetryContext(),
unityipc.Connection{ProjectRoot: projectRoot},
refusedDialAttempt(),
errors.New("sysctl kern.proc.all: operation not permitted"),
nil,
sendAttempt{},
)

entries, globErr := filepath.Glob(filepath.Join(projectRoot, vibelog.CLIVibeLogDirectory, "*.json"))
if globErr != nil {
t.Fatalf("reading the vibe log directory failed: %v", globErr)
}
if len(entries) == 0 {
t.Fatal("expected the failed process probe to be written to the CLI vibe log")
}
contents, readErr := os.ReadFile(entries[0])
if readErr != nil {
t.Fatalf("reading the vibe log failed: %v", readErr)
}
if !strings.Contains(string(contents), "operation not permitted") {
t.Fatalf("the probe failure was not recorded: %s", contents)
}
}
79 changes: 74 additions & 5 deletions cli/project-runner/internal/projectrunner/connection_retry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,8 +203,10 @@ func TestSendWithTransientConnectionRetryWritesFocusFailureVibeLog(t *testing.T)
}
}

// Verifies process probe timeouts keep the structured server-not-responding error.
func TestSendWithTransientConnectionRetryClassifiesProcessProbeTimeout(t *testing.T) {
// Verifies a process probe that timed out reports the dial error instead of the
// server-not-responding error: a probe that never read the process table cannot be the evidence
// for claiming Unity is running.
func TestSendWithTransientConnectionRetryReportsTheDialErrorWhenTheProcessProbeTimesOut(t *testing.T) {
deps := defaultConnectionRetryDeps()
deps.findRunningUnityProcess = func(ctx context.Context, projectRoot string) (*clicore.UnityProcess, error) {
<-ctx.Done()
Expand All @@ -216,7 +218,7 @@ func TestSendWithTransientConnectionRetryClassifiesProcessProbeTimeout(t *testin
connection := unityipc.Connection{
Endpoint: unityipc.Endpoint{
Network: "unix",
Address: t.TempDir() + "/missing.sock",
Address: endpointDirectoryWithRequiredMode(t) + "/missing.sock",
},
ProjectRoot: t.TempDir(),
}
Expand All @@ -231,8 +233,75 @@ func TestSendWithTransientConnectionRetryClassifiesProcessProbeTimeout(t *testin
deps)

var notRespondingErr clierrors.UnityServerNotRespondingError
if !errors.As(err, &notRespondingErr) {
t.Fatalf("expected unityServerNotRespondingError, got %v", err)
if errors.As(err, &notRespondingErr) {
t.Fatalf("a failed process probe must not report Unity as running: %v", err)
}
var connectionErr *unityipc.ConnectionAttemptError
if !errors.As(err, &connectionErr) {
t.Fatalf("expected the connection attempt error, got %v", err)
}
}

// Endpoint validation rejects any directory that is not 0700, which a plain t.TempDir() is not.
// Tests that need the dial itself to fail must get past that check first.
func endpointDirectoryWithRequiredMode(t *testing.T) string {
t.Helper()
directory := t.TempDir()
if err := os.Chmod(directory, 0o700); err != nil {
t.Fatalf("failed to set the endpoint directory mode: %v", err)
}
return directory
}

// Verifies a cancelled command reports the cancellation rather than an unreachable Unity: the
// process probe inherits the cancellation and fails, and that failure must not be turned into
// "Unity may be closed, run uloop launch" guidance for a user who pressed Ctrl-C.
func TestSendWithTransientConnectionRetryPreservesParentCancellation(t *testing.T) {
projectRoot := t.TempDir()
t.Setenv(vibelog.CLIVibeLogEnvName, "1")

deps := defaultConnectionRetryDeps()
deps.findRunningUnityProcess = func(ctx context.Context, projectRoot string) (*clicore.UnityProcess, error) {
return nil, ctx.Err()
}
deps.retryPoll = time.Nanosecond

cancelledContext, cancel := context.WithCancel(context.Background())
cancel()

_, err := sendWithTransientConnectionRetryWithDeps(
cancelledContext,
unityipc.Connection{
Endpoint: unityipc.Endpoint{
Network: "unix",
Address: endpointDirectoryWithRequiredMode(t) + "/missing.sock",
},
ProjectRoot: projectRoot,
},
"get-logs",
map[string]any{},
nil,
0,
deps)

// Identity, not errors.Is: a dial cut short by the cancellation wraps context.Canceled too, so
// errors.Is holds with or without the guard. The contract is that the cancellation itself comes
// back, because anything wrapping it is classified as an unreachable Unity.
if err != context.Canceled {
t.Fatalf("expected the cancellation to be preserved, got %v", err)
}
logFiles, globErr := filepath.Glob(filepath.Join(projectRoot, vibelog.CLIVibeLogDirectory, "*.json"))
if globErr != nil {
t.Fatalf("reading the vibe log directory failed: %v", globErr)
}
for _, logFile := range logFiles {
contents, readErr := os.ReadFile(logFile)
if readErr != nil {
t.Fatalf("reading the vibe log failed: %v", readErr)
}
if strings.Contains(string(contents), "cli_unity_process_probe_failed") {
t.Fatalf("a cancelled command must not record a process probe failure: %s", contents)
}
}
}

Expand Down