fix(cli): keep a path prefix in the login URLs - #6793
Conversation
`sim login` built both of its URLs with `new URL('/path', endpoint)`. A
leading-slash path is absolute, so it resolves against the endpoint's origin
and drops any path the endpoint carries: a deployment served at
`https://host/sim` sent the browser to `https://host/cli/auth` and polled
`https://host/api/cli/auth/poll`, neither of which exists there.
Every other command builds its URL by concatenation and was unaffected, so
the endpoint looked correct and login alone failed.
Both now go through the client's `buildUrl`, which is exported for it rather
than duplicated — one URL builder for the whole CLI is the point, since two
of them is how the halves drifted apart. Its TSDoc now names the trap.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryLow Risk Overview Approval and poll URLs now go through the shared Tests assert prefixed approval and poll URLs and workspace omission. Reviewed by Cursor Bugbot for commit 3885a15. Configure here. |
Greptile SummaryThe PR routes both device-login URLs through the shared URL builder so self-hosted endpoint path prefixes are preserved.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| packages/sim-cli/src/auth/device-flow.test.ts | Adds prefixed-endpoint and optional-workspace coverage; the fetch spy is correctly restored by the existing teardown. |
| packages/sim-cli/src/auth/device-flow.ts | Builds approval and polling URLs through the shared prefix-preserving URL helper. |
| packages/sim-cli/src/http/client.ts | Exports and documents the existing shared URL builder for reuse by device login. |
Reviews (2): Last reviewed commit: "test(cli): restore fetch with spyOn so t..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
vi.stubGlobal is not undone by restoreAllMocks, so the completed-auth response would have leaked into whatever ran next. The rest of this file already spies on globalThis.fetch, which the existing teardown restores.
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 3885a15. Configure here.
1 participant
Summary
sim loginbuilt both of its URLs withnew URL('/path', endpoint). A leading-slash path is absolute, so it resolves against the endpoint's origin and drops any path the endpoint carries. A self-hosted deployment served athttps://host/sim— a proxy fronting several apps — sent the browser tohttps://host/cli/authand polledhttps://host/api/cli/auth/poll, neither of which exists there.buildUrl, exported for this rather than duplicated. One URL builder for the whole CLI is the point — two of them is how the halves drifted apart in the first place. Its TSDoc now names the trap so the next caller does not reintroduce it.Pre-existing; found while fixing the adjacent redirect handling in #6788/#6791. Deployments at a domain root, including sim.ai, were never affected.
Type of Change
Testing
Tested manually.
sim login --no-browseragainst a normal endpoint prints the same approval URL as before. Added a test asserting both the approval URL and the poll URL keep a/simprefix, proven red against the previous implementation (expected 'https://host.test/cli/auth?…' to match /^https:\/\/host\.test\/sim\/cli\/auth…/). Also covered that an absent workspace is omitted rather than sent blank, which the rewrite had to preserve.Suite is 345 passed / 1 skipped.
lint,type-check,build, and all 29 audits incheck:auditspass.Checklist