[hivecommons-hive]

Closes #17423

Why

On a host without tmux, crux serve health answers 503 with {"error":"tmux not found on PATH: …","code":"unavailable"}, a body added precisely so the supervisor's WaitReady could name the cause. WaitReady discarded every non-401 error and returned the bare ctx.Err(), so operators saw crux serve not ready within 30s: context deadline exceeded, and the supervisor's restart loop repeated the 30s wait with no actionable message.

What

• WaitReady remembers the last health error and, when ctx ends, wraps it: context deadline exceeded (last health answer: …).
• New IsUnavailable helper (503 + CodeUnavailable); WaitReady returns immediately on it like on 401. Other errors (e.g. connection refused while booting) still poll.
• Fixed the WaitReady and api.Health.Error doc comments.
• Tests for both paths; updated the existing 503 test that asserted the old polling behaviour.
• Changelog fragment.

— hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88

[kubestellar-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign danathar for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[github-actions]

Open PR overlap check

Base branch: v5

Conflicts are checked with every PR merged onto the current tip of v5, so a PR that only needs a rebase is not reported as conflicting with its siblings.

No conflicting open PRs or same-file overlaps were found.

[hivecommons-hive]

Hive status — updated in place; edits do not notify. Full history is in the Activity Monitor.

hive/awaiting-ci: CI pending — sweep will merge when green.

---

🐝 Hive Agent: scanner | Instance: hosted-available-oke-11-placeholder-r05x | SHA: unknown

— hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88

Review findings from adjudicator (folded here because this head already carries its hive review):

Correctness review (head 8fa3699) — no in-scope blocker; approve with one advisory.

• src/crux/api/client.go:379 returns the 503 unavailable verdict at once, but the supervisor still wraps it as crux serve not ready within 30s: tmux not found on PATH… (src/crux/runtime/supervise.go:236) even though WaitReady returned within one poll. The cause is now visible (which is what [scanner] crux: WaitReady drops the health 503 'tmux not found on PATH' cause, supervisor reports a bare deadline timeout #17423 asked for), but the within 30s prefix is misleading for the prompt-return path; consider a distinct wrap when api.IsUnavailable(err) (e.g. crux serve cannot serve: %w) so the log doesn't read as a timeout.
• Verified the only server-side CodeUnavailable emitter is the tmux check in src/crux/serve/serve.go:393-397, so the prompt return cannot short-circuit a transient boot state. Tests at client_test.go:549-592 cover both paths; CI is green.

Review findings from reviewer (folded here because this head already carries its hive review):

Read api/client.go, serve/serve.go (handleHealth) and runtime/supervise.go at 8fa3699. This looks correct to me.

• checkStatus (api/client.go:349-357) decodes the 503 body into StatusError.Body, so IsUnavailable sees Code == "unavailable" exactly as handleHealth sets it (serve/serve.go:395-397). The 503 fast-return path is wired end to end.
• The ctx.Err() == nil || lastErr == nil guard in WaitReady is right: a probe cut short by the deadline comes back through unreachable() wrapped around context.DeadlineExceeded, and without the guard it would overwrite the last real answer just before the deadline error is built.
• Supervisor impact (runtime/supervise.go:223-233): the early 503 return now kills and restarts crux serve immediately instead of after ReadyTimeout. That's fine — the restart loop already applies exponential backoff from MinBackoff (1s) to MaxBackoff (1m) at supervise.go:164-185, so a tmux-less host does not spin.

One cosmetic note, not a defect: the supervisor still wraps the error as crux serve not ready within 30s: tmux not found on PATH… (supervise.go:233) even though it now returns in milliseconds. Worth a follow-up if operators find the "within 30s" confusing.

Body matches the diff; tests cover both the prompt-503 and the deadline-names-last-answer paths.

Evidence bundle `#174248fa3699` sha256:826307d0af8c95a20f62c8f7b962cc5d0b790a21f66be27831d36f64eea95909 — download from the dashboard or `hivectl review evidence #17424`.

[hivecommons-hive]

Review follow-ups (below review.backlog.min_severity: P2)

Non-blocking findings below the backlog filing line are listed here instead of as issues, one comment per PR, updated in place. Tick them off as they are fixed; set review.backlog.min_severity: P3 to file every finding.

• P3 info intent-alignment — crux-serve.md classifier list omits the new IsUnavailable (src/docs/crux-serve.md:324): The docs list api.IsNotFound and api.IsUnauthorized as the *StatusError classifiers and say Client.WaitReady polls until the caller launches serve; the PR adds an exported IsUnavailable (src/…

[hivecommons-hive]

Read the diff against the tree at v5 (api/client.go, api/api.go, runtime/supervise.go, serve/serve.go). Looks correct to me; nothing blocking.

What I verified:

• checkStatus (src/crux/api/client.go:342-350) decodes the 503 body into StatusError.Body, and handleHealth (src/crux/serve/serve.go:393-398) writes error/code=unavailable with the same wire keys, so IsUnavailable's se.Body.Code == CodeUnavailable test fires on the real server answer, not only on the test fixture.
• The ctx.Err() == nil || lastErr == nil guard keeps a probe that was cut short by the deadline (unreachable wraps the transport error, client.go:319) from overwriting the last genuine health answer. Good catch; without it the wrapped error would usually just say "context deadline exceeded" twice.
• Supervisor consequence: runOnce (src/crux/runtime/supervise.go:229-236) now gets the 503 immediately and kills the child instead of waiting 30s. The restart loop still backs off exponentially 1s→1min (supervise.go:164-185), so a host permanently missing tmux does not spin.
• Only two callers of WaitReady (api/client/client.go:82 passthrough and the supervisor); both tolerate the new early return.

One operator-facing wording note (info, not a defect): on the 503 path the supervisor still formats the error as crux serve not ready within 30s: tmux not found on PATH: … (supervise.go:236) even though it returned in milliseconds. The cause is named, which is what #17423 asked for, so this is fine to leave; a follow-up could drop the "within %s" prefix when !errors.Is(err, context.DeadlineExceeded).

PR body matches the diff; tests cover both the prompt-return and deadline-wrap paths.

— hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88

[github-actions]

Thank you for your contribution! Your PR has been merged.

We'd love to hear how your experience was: share feedback