fix(remote): restore authenticated GUI health (carry of #3168) - #3179
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
✅ Deterministic PR hygiene checks passed. |
⏳ DRAFT
What to do
Automatic draft conversion failed (token cannot change draft status). Please convert this pull request to a draft manually. The required |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ed0db2ceb2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| try { | ||
| const [hRes, pRes] = await Promise.all([ | ||
| fetch(`${apiBase}/healthz`, { signal }), | ||
| fetch(`${apiBase}/api/system/health`, { signal }), |
There was a problem hiding this comment.
When a connected client is upgraded before its hub, the local GUI uses this new endpoint while the still-supported protocol-1 hub returns 404 because /api/system/health does not exist there. requireJson then rejects and the catch marks the entire overview unavailable, discarding the successful /api/providers response and showing “Cannot connect” even though the hub is reachable. The remote protocol remains at version 1 and explicitly permits additive version skew, so either negotiate/fallback for older hubs or raise the minimum hub protocol before making this route mandatory.
Useful? React with 👍 / 👎.
2 participants
Summary
Maintainer rebase carry of #3168 by @Ingwannu, cherry-picked onto current
devwith author credit preserved.Restores authenticated GUI health through the management plane: a GUI attached to a connected runtime could not reach a health endpoint because the existing one lives on the data plane. This is the T3 follow-up recorded in #3158.
Why a carry branch. #3168 was
DIRTY— it conflicted withdev. A maintainer cannot push a rebase to a contributor branch, so the commit is re-applied here.The conflicts, and how they were resolved. Both were in documentation that #3173 had touched in the same week, and both sides had independently documented the same facts:
reference/cli/lifecycle.md— both added the three/readyzprotocol fields. Kept the fuller wording, which additionally says wheremanagementUrlcomes from in each runtime role and that a readiness request with no HTTP(S) origin is rejected rather than guessed.reference/configuration/server.md— both documentedremoteGui.allowInsecureHttpas retired. Kept the table form, which covers all four remote-hub keys with their defaults and failure modes rather than that one key alone.No code conflicted. The resolution keeps both authors' conclusions; they agreed on the substance.
Verification
Exact head
ed0db2ceb:bun test ./tests/server-management-auth.test.ts ./tests/cli-capabilities.test.ts— 55 pass, 0 fail, 421 expect() calls. Those are the suites covering the management-auth surface and the capability/route parity ratchet this diff extends.git show --statconfirms the carry: 25 files, +112/-19, authorIngwannu.Full-suite, GUI, and typecheck coverage is left to CI on this exact head.
Checklist
dev