Repository navigation
fix(cli): continue explicit --server after legacy health 404 - #424
Merged
Merged
Conversation
pc_db4667ac9ad5 An installed server that still answers /api/health or /api/status returns 404 for /api/info. Explicit --server now warns and continues instead of exiting.
…cy-health-probe-5e6a
shuv1337
marked this pull request as ready for review
October 5, 2026 02:43
This was referenced Oct 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
The captain approved resuming the issue backlog (2026-10-04): "triage the ~22 Cursor draft PRs first, then small batches", and later answered yes that shuvbro's triage review counts as the first review for these Cursor PRs. The read-only triage report is at ~/repos/shuvbro/data/triage-cursor-shuvcode/report.md; read the section for each PR you handle. Verdicts: #424 (fix(cli): continue explicit --server after legacy health 404, issue #399) is land: only a modern-server UnexpectedStatus 404 on /api/info falls back to probing /api/health then /api/status and continues with a warning; 401/503 still fail; 10 local tests pass. #428 (fix(cli): accept --standalone before subcommands, issue #416) is fix-then-land with an open review finding (#428 (comment)): globally accepted --server/--standalone are silently ignored by handlers that bypass ServerConnection.resolve - service stop/restart/start/set/unset, pair, acp, serve - so e.g. shuvcode --server http://other:4096 service stop stops the LOCAL service. Required: keep true global parsing for supported client commands but reject explicitly requested unsupported connection flags BEFORE any side effect on those paths (or implement real endpoint support where valid), update help with the support boundary, and add regressions for rejection, no local effects, and real endpoint/private selection across flag placements. Both PRs change packages/cli/src/services/server-connection.ts. The triage's land verdict preserves explicit endpoint selection without managed-service replacement, warns on modern version skew, legacy version skew, versionless health, or all-404, and falls back only on modern UnexpectedStatus 404. For the global flag repair, every local lifecycle/config/pair/serve/ACP case listed must reject unsupported explicitly requested connection flags before effects; ignoring flags on unrelated non-server commands is allowed. Existing tests only prove placements reach mutual exclusion; actual requested remote endpoint and standalone selection must be exercised.
What Changed
ServerConnection.resolvewith an explicit--servernow treats anUnexpectedStatus404 from/api/infoas a sign of an older server build. It probes/api/healthand then/api/status, and continues with that endpoint instead of failing or replacing it with a managed service. Every other/api/infofailure, such as 401 or 503, still fails as before./api/infoversion mismatch still prints the existing skew warning and skips the legacy probe.server-connectiontests for: falling back to/api/health, falling back to/api/status, all paths returning 404, modern version skew with no legacy probe, legacy health with no version, and the unchanged 401 and 503 failures.Risk Assessment
✅ Low: The change is small. Only an UnexpectedStatus 404 from /api/info enters the legacy fallback, while 401 and 503 still fail. The explicit endpoint is kept without managed-service replacement, and behavior-level tests cover each warning path. This matches the triage 'land' verdict for PR #424.
Testing
I installed dependencies and ran the real shuvcode CLI (
models --server) against a local stub server in seven modes, using isolated XDG dirs. The 404 fallback paths (legacy /api/health version, /api/status version, versionless health, all-404) each print the right warning, send the follow-up /api/model request to the explicit endpoint, and list the model with exit 0. A modern server with a different version warns and does not probe the legacy routes. 401 and 503 still fail with the existing errors. An A/B run with the base resolver shows the old behaviour: legacy-404 and all-404 exited 1. The focused server-connection test file also passes. The evidence is the CLI transcripts and the stub/driver scripts in the evidence directory. There is no UI surface, so no screenshots.shuvcode models --server <legacy-url>where /api/info is 404 and /api/health reports a version: the CLI warns about the legacy version and completes against that serverEvidence: CLI transcript: shuvcode models --server against 7 stub modes (HEAD)
Evidence: Baseline CLI transcript (base commit resolver) showing the legacy 404 failure
Evidence: Stub server used for the remote endpoint
Evidence: Driver script
Evidence: Legacy health fallback excerpt
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
⏭️ **Rebase** - skipped
Step was skipped.
packages/cli/src/services/server-connection.ts:106- legacyHealth treats every non-OK legacy response the same way, including 401/403, network errors, and invalid JSON. Example: a legacy server returns 404 for /api/info and 401 for /api/health because the password is wrong. The command still continues, and the warning says the server "has no legacy /api/health or /api/status response". On the modern path a 401 fails immediately with a 'requires a password' message, so a credential problem here is mislabeled and only shows up later. Triage accepted continuing when every probe returns 404. Failing on a legacy 401, or naming the status in the warning, is optional polish.✅ **Test** - passed
✅ No issues found.
shuvcode models --server <legacy-url>where /api/info is 404 and /api/health reports a version: the CLI warns about the legacy version and completes against that serverbun installat the repo root (needed to run the CLI)drive.sh: for each stub mode (legacy-health, legacy-status, versionless, all404, modern-skew, unauthorized, unavailable), ranbun run src/index.ts models --server <stub-url>in packages/cli with isolated XDG/HOME dirs and recorded exit code, stdout, stderr and the requests the stub receivedA/B baseline: temporarily put backgit show 97f9afcadf:packages/cli/src/services/server-connection.ts, ran the legacy-health and all404 modes again, then restored HEAD (git status clean)bun test test/server-connection.test.tsin packages/cli (9 pass)✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.