Repository navigation
fix: preserve base URL path prefix in direct requests - #60559
fabiovincenzi wants to merge 5 commits into
Conversation
![opencodejr[bot]](https://avatars.githubusercontent.com/in/5497873?s=60&v=4){kind=small}
There was a problem hiding this comment.
This looks good to me. I ran the #60542 reproduction from source: on the base, opencode api (both the raw path and the /openapi.json lookup) and opencode session import sent their requests to the host root. With this change they all go to /proxy/..., and query strings survive. The new tests pass. With plain tsc, the changed CLI and client files have no type errors. The packages' typecheck scripts fail the same way on the base in this environment, and I didn't type-check the app package.
Public API: unchanged. ServerConnection.requestUrl is internal to the CLI.
One small inline note on the new CLI test. Nothing else blocking.
| import { OPENCODE_VERSION } from "../src/version" | ||
| import { isolatedEnv } from "./fixture/environment" | ||
|
|
||
| test.each(["", "/proxy", "/proxy/"])( |
There was a problem hiding this comment.
This test starts the CLI as a subprocess six times (~1.6s each), and server-connection.test.ts already covers the joining cases for requestUrl. Could you cut it down to a single prefixed case (e.g. /proxy) that runs both the raw and the operation form? The "" case doesn't add anything here.
There was a problem hiding this comment.
The api prefix test now runs once against /proxy and still covers both forms. That addresses my earlier point. It passes with the change and fails on the base. The source is unchanged since my last review, and I have nothing new to raise. Public API: unchanged.
There was a problem hiding this comment.
The new commit merges dev. The conflict in the attachment upload is resolved correctly: it applies the prefix-preserving join to dev's new sdk.http.url. The rest of the change is unchanged, and the CLI and client tests still pass after the merge. I have nothing new to raise. Public API: unchanged.
|
This unblocks #61624, which we'd use to run OpenCode inside marimohub behind its authenticated proxy under a per-session path prefix. A review would be much appreciated. |
Issue for this PR
Closes #60542
Type of change
What does this PR do?
#50428 fixed the generated client, so
--server http://host/prefixnow passes the connection check. A few places still build request URLs by hand withnew URL("/api/...", serverUrl). The leading slash makes the path absolute, so the prefix is dropped.With a prefixed
--serverthis breaksopencode api(the request and the/openapi.jsonlookup) andopencode session import.The fix uses the same approach as #50428: add a trailing slash to the base and join the path without its leading slash. The three CLI call sites share a small
ServerConnection.requestUrlhelper.The service health probe (
client/src/service-probe.ts) and the attachment upload (app/src/composer/attachments/destination.ts) use the same pattern. They can't receive a prefixed URL today, but I fixed them too so they don't break once they can.How did you verify your code works?
packages/cli/test/api-prefix.test.tsrunsopencode apiagainst a server mounted under a prefix. Without the fix, the two prefixed cases fail.packages/client/test/service-probe.test.tscovers the probe, andserver-connection.test.tscoversrequestUrl.opencode servebehind a reverse proxy on/proxy:opencode api --server http://127.0.0.1:4999/proxy GET /api/sessiongets a 404 from the proxy ondev, and the session list with this change.Visual
N/A
Screenshots / recordings
N/A, not a UI change.
Checklist