fix: Stop reporting Unity as running when the process check itself failed - #2020
Conversation
finishUndispatchedRetryProbe returned "Unity is running for this project, but the Unity CLI Loop server is not responding" (Retryable and SafeToRetry both true) whenever the process probe returned an error, without ever looking at the process it failed to find. The probe's only role is to promote the diagnosis from "not reachable" to "running but not responding", so a failed probe cannot promote anything: it observed nothing. Both probe-failure branches now fall through to the same handling as a probe that found no process and report the dial error verbatim, and the swallowed probe error goes to the CLI vibe log instead of replacing it. The branch that returned the probe error alone while the retry window was still alive is aligned with the same fallback: it was honest but discarded the dial error, which is the fact the caller acts on. Reachable in ordinary operation: a sysctl failure on macOS, and a PowerShell launch failure or a process-list timeout on Windows.
|
Warning Review limit reached
Next review available in: 22 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Removing the probe-error branch also removed its ctx.Err() check, which is the convention everywhere else that waits on Unity (finishUnityAliveRetry, the tool readiness wait, the pause-point poll). Without it, a Ctrl-C or SIGTERM during the dial-retry loop cancels the process probe, and the probe failure is reported as an unreachable Unity: "If Unity is closed, run `uloop launch`" for a command the user interrupted themselves, plus a probe-failure warning in the vibe log. The check goes in the caller, before the probe runs, so a cancelled command does not scan the process table at all. Also give the endpoint directory the 0700 mode endpoint validation requires, so the retry tests fail on the dial itself rather than on validation.
errors.Is held whether or not the cancellation guard was present: a dial cut short by the same cancellation wraps context.Canceled too. The test only caught the regression through its vibe-log assertion, leaving the user-visible difference (a cancellation versus an unreachable Unity with launch guidance) unchecked.
fe05dbd
into
feature/cli-discoverability-integration
Summary
User Impact
Before: whenever the process check failed — it could not read the process list at all — the command still answered
with
Retryable: trueandSafeToRetry: true, i.e. "wait and retry". That claim was made without ever looking at a process, and the reason the check failed was dropped entirely, so nothing in the output hinted that the diagnosis was a guess.After: the connection error is reported verbatim, exactly as it already is when the check runs and finds no Unity process, with the guidance that belongs to it. The check failure itself is written to the CLI vibe log (
ULOOP_DEBUG), so it stays diagnosable without being presented to the user as a conclusion.This is a potential path whose symptoms could match the "Unity is running but not responding" report seen on 2026-07-26; that report was not reproduced, and this change is not claimed to be its root cause. The branch asserts something it has no evidence for regardless of whether that incident ever took it.
Reachable in ordinary operation: a
sysctlfailure when listing processes on macOS, and a PowerShell launch failure or a process-list timeout on Windows.Changes
finishUndispatchedRetryProbe: a process-check error no longer selects its own branch. The check exists only to promote the diagnosis from "not reachable" to "running but not responding"; a check that observed nothing cannot promote anything, so both outcomes now return the same dial error. No new error type and no new envelope field.ctx.Err()branch goes away with it, matching the existing no-process path.cli_unity_process_probe_failed, WARNING) including the dial cause, instead of being swallowed.ctxparameter is dropped from the signature and the call site.uloop launch" back — plus a check-failure warning in the vibe log — for an interruption the user caused. Running it before the check also means a cancelled command never scans the process table.Verification
a failed probe must not report Unity as running: Unity is running but the Unity CLI Loop server is not responding: ...), green after the change.connection_retry_flow_test.go: probe failure with an expired window returns the dial error; probe failure with a live window does the same; an earlierserver_busyresponse is still preserved; a successful probe still continues retrying; the probe failure is written to the vibe log.connection_retry_test.go: the existing end-to-end test that asserted the old behavior is inverted — a probe timeout must now yield the connection attempt error and must not yield the server-not-responding error. Its endpoint directory is now given the 0700 mode endpoint validation requires, so the failure comes from the dial rather than from validation.context.Canceledand writes nocli_unity_process_probe_failedentry. Removing the guard makes it fail on both counts.newUnityServerNotRespondingErrorreturn makes both the unit test and the end-to-end test fail with the old message; restored afterwards.sh scripts/check-go-cli.shpasses (format, vet, lint0 issues., all module tests, binary rebuild).cli/commonsources changed, soscripts/stamp-release-inputs.shdoes not apply.