[hivecommons-hive]

Closes hivecommons/crux#137

Why

/v1/health always answered 200, even when tmux (which hosts every agent) is missing from PATH, so probes saw a healthy server that fails every crux run. Crux's Go code is a sync of src/crux, so the fix goes here rather than in hivecommons/crux (its Sync guard rejects Go changes).

What

• api.Health gains an additive tmux field (path from tmux.LookPath); existing fields unchanged.
• handleHealth returns 503 (full body still sent) when tmux is missing.
• changelog.d fragment.

Tests

• serve/health_test.go: TestHealthTmuxMissingIs503 (PATH without tmux → 503, empty field), TestHealthTmuxPresentIs200 (skips if tmux not installed).
• TestServeWithoutHostOrBackground now puts a fake tmux on PATH so it still expects 200.

Not run locally; CI is the verdict. The runbooks/agent-not-working.md symptom-table row is a follow-up in hivecommons/crux.

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

[hivecommons-hive]

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

hive/awaiting-ci: the 18:23Z run failed because the required context scope and runtime reusable-workflow lanes were abandoned (several sibling workflows on the same SHA — file-size ratchet, FOSSA, Docs Link Check — also finished with zero jobs), so build-and-test and coverage refused to report without a diff verdict. v5 is green at 4126d8b; merged v5 into the branch to re-run CI on a fresh commit. 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-3 (folded here because this head already carries its hive review):

Intent alignment (read at head 1351417, against hivecommons/crux#137)

The diff implements the issue's suggested fix #1 as written: additive Tmux field (src/crux/api/api.go:110), 503 with full body when tmux.LookPath fails (src/crux/serve/serve.go:392-398), PATH-less test. runtime-tests is green, so CI runners have tmux. Two gaps between what the body claims and what the tree does:

1. Supervisor probe now kills crux serve with an opaque error, losing the reason the 503 exists to convey. api.Client.WaitReady (src/crux/api/client.go:370-372) treats any non-401 error — now including the new 503 — as "not yet", and on deadline returns bare ctx.Err(). runtime/supervise.go:223-236 then logs crux serve never became ready and returns not ready within <ReadyTimeout>: context deadline exceeded, then the server is killed. The tmux: "" body that handleHealth deliberately still sends (serve.go:397) is read into StatusError.Body (client.go:346-348) and discarded. The issue's stated concern was exactly this — "the 503 changes probe behaviour for existing deployments" — and the PR body/changelog say only that "probes now see the failure", not that the supervisor will now fail startup. Concrete fix: have WaitReady remember the last Health error and return it wrapped alongside ctx.Err() on deadline (e.g. fmt.Errorf("%w: last health: %v", ctx.Err(), last)), so the supervisor log names the 503/tmux; and state the supervisor consequence in the changelog fragment.

2. Stale "no tmux involved" assertions now false. src/crux/scripts/smoke.sh:5 ("Needs no agents, tmux or network") and .github/workflows/v2-ci.yml:211 ("No agents or tmux involved") describe a smoke that does curl -fsS …/v1/health (smoke.sh:68), which now fails on any runner lacking tmux with the uninformative "GET /v1/health failed". Update both comments (or have smoke print the body on failure). Related: only TestServeWithoutHostOrBackground was given fakeTmuxDir; serve_test.go:474,490, principal_test.go:77, cli/serve_test.go:109 still expect 200 and rely on the host having tmux.

Advisory, not blocking: the issue deferred "gate on 503 vs. report only" to maintainers; the hive took the gate option (issue comment). Both Dockerfiles install tmux, so production images are unaffected; the behaviour change lands on bare hosts only.

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

The tmux field is a good addition; the 503 gate as implemented causes two concrete regressions I verified in the v5 tree, and crux#137 itself left "gate on 503 or only report the field" as a question for the maintainers. I'd suggest landing the field now and keeping the response 200, or fixing both callers below before the 503 lands.

1. The supervisor restart-loops a tmux-less server instead of starting it (high).
src/crux/runtime/supervise.go:223 waits on probe.WaitReady, and WaitReady (src/crux/api/client.go:362-377) only returns nil when Health returns no error; do turns any non-2xx into an error via checkStatus (client.go:279). With this PR, a host without tmux never produces a 200, so after ReadyTimeout the supervisor logs crux serve never became ready; killing (supervise.go:231), kills the child and restarts it. Before this change such a host served /v1/health, ps, etc. and failed per-agent on crux run with a tmux error; after it, the server is killed before an operator can read the body the PR goes to the trouble of still sending. The runbook row crux#138 adds ("Health returns 503 and tmux is empty") is unreachable under the supervisor — the operator sees the "uptime_seconds keeps resetting" row instead.

2. crux's smoke job goes red on sync (medium).
src/crux/scripts/smoke.sh:68 fetches health with curl -fsS, so a 503 is "GET /v1/health failed". In hivecommons/crux ci.yml:249-269, the smoke crux serve (ubuntu-latest, macos-latest) job builds and runs that script with no tmux install step (the ./agent job installs tmux explicitly at ci.yml:91; smoke does not). hive's own CI has no smoke job on this PR, so this surfaces only after the sync.

Not a defect, just a note: TestHealthTmuxPresentIs200 skips when tmux is absent, so on the hive CI runners the 200 path is covered only if tmux happens to be installed.

Body/diff match otherwise: additive field, 503 with full body, changelog fragment present. I have not changed or blocked anything.

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

The gate itself is implemented as described, and the tests are sound (famServer.expect decodes the body regardless of status, so TestHealthTmuxMissingIs503 genuinely checks the 503 body). CI red here is infra, not this diff: build-and-test failed with go=success contracts=success runtime=abandoned.

One consequence I think needs addressing before this lands:

The supervisor turns the 503 into a kill/restart loop with an error that never names tmux (medium). runtime/supervise.go:223 waits on probe.WaitReady, and api/client.go:369 only returns when Health() returns a nil error. client.do (api/client.go:279) runs checkStatus before decoding, so a 503 is a *StatusError, never a Health — the "full body still sent" is unreadable to every Go caller. WaitReady therefore spins until ReadyTimeout (30s), the supervisor logs crux serve never became ready; killing (supervise.go:230), kills the child and restarts it on backoff, forever. The error text is StatusError.Error() (api/client.go:119-124): api.Health has no error field, so se.Body.Error is empty and the operator sees only crux server returned HTTP 503. Before this PR a tmux-less host ran crux and failed each crux run with a tmux error; after it, crux never comes up and the log doesn't say why. If the gate stays, the 503 should carry something StatusError can surface — e.g. an Error string \json:"error,omitempty"`onapi.Healthset totmux not found on PATH— and ideallyWaitReady` should stop early on 503 rather than burning the whole ready timeout.

Two smaller notes:

• crux#137 explicitly left "gate on 503 vs only report the field" for maintainers to confirm because of exactly this probe-behaviour change. The PR picks the gate without saying why; worth a sentence in the body so the choice is on record.
• The body uses Closes hivecommons/crux#137 but defers the issue's item 2 (runbook symptom row in hivecommons/crux) to an untracked follow-up. Either Refs with what is left, or file the follow-up so it does not evaporate when Add no-build HARD GATE to all 5 agent CLAUDE.md files #137 auto-closes.

Evidence bundle `#17382f727b08` sha256:af0651ffd80072f7863a62ec8581cd6294ea12928d1766e9c82f3ff40657ebff — download from the dashboard or `hivectl review evidence #17382`.

[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]

Style review (head 1351417) — approve, one advisory.

• fakeTmuxDir is applied only in TestServeWithoutHostOrBackground (src/crux/serve/families_test.go:887). Other tests that assert 200 on /v1/health — serve_test.go:474, serve_test.go:490, principal_test.go:77, cli/serve_test.go:109 — now depend on a real tmux on the host. CI is fine (v2-tests.yml:974 installs tmux explicitly), but locally without tmux those unrelated auth/version tests fail with status 503 want 200 and no hint. Suggest applying the same fake PATH there, or noting the host-tmux dependency in fakeTmuxDir's doc comment.
• Adjacent, non-blocking: src/docs/crux-serve.md:238 and src/docs/design/crux.md:751 enumerate the health fields and don't mention tmux/503.

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

[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 style — Health field lists in docs not extended with tmux (src/docs/crux-serve.md:238): src/docs/crux-serve.md:238 and src/docs/design/crux.md:751 enumerate the /v1/health fields (hive_callback, started_at, uptime_seconds). The new tmux field and the 503 semantics are only in the chan…

[hivecommons-hive]

Needs a decision, not a merge:

• the reviewer swarm returned requires_human @1351417
• the review found: sensitive-paths question from the correctness review
• reviewer question: @clubanderson should decide whether /v1/health gates on 503 when tmux is missing (as this PR does, which makes hive's crux supervisor kill and restart-loop a tmux-less server) or only reports the new tmux field, which is the open question [operations] /v1/health returns 200 without checking tmux, which every agent needs crux#137 explicitly left to maintainers.
• the automerge lane will not merge this unattended

Head: verdict requires_human @1351417, 69/69 checks green, DCO yes, soak 0s of 10m0s.

What to do: approve this head (1351417) to merge as-is, or leave a changes requested review (or reply /hive decision <what to change>) and the hive will push the fix.

How to release it:

• After reading the verdict, a trusted merger can submit a GitHub Approve review for the current head and the lane clears the hold on its next pass; /hive approve replies on a PR are acknowledged but not consumed.

_{Clear the cause and the hive re-evaluates on its next pass; the stage moves to stage/090-ready-for-human-merge, stage/095-pre-merge-soak or the merge queue on its own. Approval does not bypass CI, hold labels or other merge gates.}

[hivecommons-hive]

/lgtm

Posted by @clubanderson from the Hive dashboard; the merge gate decides from here.

[kubestellar-prow]

@hivecommons-hive[bot]: you cannot LGTM your own PR.

Details

In response to this:

/lgtm

Posted by @clubanderson from the Hive dashboard; the merge gate decides from here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[hivecommons-hive]

/approve

Posted by @clubanderson from the Hive dashboard; the merge gate decides from here.

[kubestellar-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: hivecommons-hive[bot]
Once this PR has been reviewed and has the lgtm label, please assign clubanderson 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

[hivecommons-hive]

Decision needed: @clubanderson should decide whether /v1/health may return 503 when tmux is missing (which makes runtime.Supervisor kill and restart crux serve on every tmux-less host) or should only report the new tmux field and stay 200, as crux#137 left open.

Conclusion: the diff is correct for what it does, but it decides a question crux#137 explicitly left for maintainers, and the 503 has a concrete consequence beyond "probes see red". Routing for a decision rather than judging it myself.

What I verified (read at 7c08fcc):

• src/crux/serve/serve.go:392-398 — handleHealth now calls tmux.LookPath() (src/crux/agent/tmux/tmux.go:23-25, a plain exec.LookPath) and answers 503 with the full body when it errors. api.Health.Tmux is additive (src/crux/api/api.go:108-110). Fine.
• src/crux/api/client.go:342-349 — checkStatus turns any non-2xx into *StatusError, so Client.Health (:139-143) now returns an error on a tmux-less host and the caller never sees the body that says why.
• src/crux/api/client.go:362-379 — WaitReady only returns nil when Health returns nil. It will therefore spin until ctx expires.
• src/crux/runtime/supervise.go:223-235 — the supervisor's readiness probe is exactly that WaitReady. On timeout it logs crux serve never became ready; killing, kills the child and (per the restart loop in the same file) starts it again. So on a host without tmux this PR converts "server up, agents fail to launch" into "server killed and restarted on a loop", with the tmux diagnosis discarded by checkStatus.

That may well be the desired behaviour (fail loud, let the orchestrator surface it), but crux#137 says in so many words: "the 503 changes probe behaviour for existing deployments and the maintainers should confirm whether to gate on 503 or only report the field." The PR body does not mention that this was left open, nor the supervisor interaction above. If the gate is wanted, WaitReady probably needs to treat a 503-with-body as "reachable, degraded" rather than "not ready", or the supervisor will mask the real cause.

Also noted:

• src/crux/serve/families_test.go:887 — TestServeWithoutHostOrBackground now needs a fake tmux on PATH to keep expecting 200, which is a small tell that the CI runners don't have tmux; TestHealthTmuxPresentIs200 skips there, so the 200 path is only exercised through the fake.
• build-and-test and coverage are red on this head but neither job exposed a log I could read, so I can't say whether that is the diff or infrastructure. Open question, not a finding.
• The runbook row for runbooks/agent-not-working.md is deferred to hivecommons/crux, as the body says; nothing else in src/docs documents /v1/health status codes.

I have not approved, requested changes, or closed anything.

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

[hivecommons-hive]

CI triage (head 7c08fcc): build-and-test and coverage failed because every test shard was skipped/abandoned (hub=skipped, sliced=skipped, agent=skipped, rest=skipped; runtime lane abandoned) — the self-hosted runner pool is red, see #17418. build-go and contract-tests passed on this head. No code cause; deferring until the pool is healthy and CI re-runs.

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

[hivecommons-hive]

Looks correct for HTTP probes; one verified gap for Go callers — the 503 body's tmux field is unreachable through api.Client, so the diagnostic this PR adds never reaches the hive supervisor's log.

Read at 5afc5e6: src/crux/serve/serve.go:392-398, src/crux/api/client.go:257-289, 342-350, 362-380, src/crux/runtime/supervise.go:218-236.

Finding (medium, correctness): Client.do → checkStatus (client.go:342-350) treats any non-2xx as *StatusError and unmarshals the body into api.Error. The health body on 503 is api.Health, which has no error/code keys, so StatusError.Error() (client.go:119-124) falls through to "crux server returned HTTP 503". The tmux: "" field the PR body says is "still sent" is discarded before any Go caller sees it.

Consequence on a host without tmux, with the hive-side supervisor: supervise.go:223 polls WaitReady, which only stops early on 401 (client.go:369-372), so it loops for the full DefaultReadyTimeout (30s), then logs crux serve never became ready; killing ... err=crux server returned HTTP 503 (supervise.go:230), kills the server and restarts it. Louder than today's silent-200, which is the issue's goal, but the operator still has to guess that tmux is the cause — and this is the only route in serve/ that writes a non-api.Error body with a non-2xx status (writeError at serve.go:736 is the pattern everywhere else).

Two small options, either is fine:

• add Error string \json:"error,omitempty"`/Codetoapi.Healthand setError: "tmux not found on PATH"on the 503 path — the existingcheckStatus` then surfaces it with no client change; or
• keep the body and have WaitReady return immediately on a 503 (StatusError.Status == http.StatusServiceUnavailable) so the supervisor fails fast instead of polling 30s.

Everything else checked: tmux.LookPath (agent/tmux/tmux.go:23) is a plain exec.LookPath, so t.Setenv("PATH", …) in the tests drives it correctly; the TestServeWithoutHostOrBackground PATH fix is needed since that test asserts 200; Health is additive, no existing field moved. No /v1/health body docs exist under src/crux to update; the changelog fragment covers the user-facing change.

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

[hivecommons-hive]

Looks correct to me at 400a2ed; the one open finding from the previous round is resolved, and the 503 decision was taken by @clubanderson (/approve from the dashboard, 17:35Z).

What I verified in the tree:

• src/crux/api/api.go:112-114 — api.Health now carries Error/Code with the same wire keys as api.Error, omitempty, so a 200 body is unchanged apart from the additive tmux field.
• src/crux/serve/serve.go:392-401 — handleHealth sets Error: "tmux not found on PATH: …" and Code: api.CodeUnavailable on the 503 path only. checkStatus (src/crux/api/client.go:342-350) unmarshals that body into StatusError.Body, and StatusError.Error() (client.go:119-124) now returns the tmux message instead of "crux server returned HTTP 503". The supervisor's WaitReady loop (src/crux/runtime/supervise.go:223-231) therefore logs the cause when it gives up — exactly the gap flagged at 5afc5e6.
• src/crux/api/client_test.go:518-547 pins both halves: the error string names tmux, and WaitReady still polls a 503 to deadline. src/crux/serve/health_test.go:25-35 asserts the server side.

Remaining advisory, not blocking: TestHealthTmuxPresentIs200 and the pre-existing 200-asserting tests (serve/serve_test.go:490,627, serve/principal_test.go:77) depend on a real tmux on the host; CI's runner image bakes it in (#7206), so this only bites local runs. Docs follow-up (src/docs/crux-serve.md health field list) is already tracked in the review follow-ups comment.

I have not approved or merged anything.

— 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