Skip to content

fix(client): preserve server URL path prefixes - #47442

Open
Rocklis wants to merge 3 commits into
anomalyco:v2from
Rocklis:codex/server-paths
Open

Rocklis wants to merge 3 commits into
anomalyco:v2from
Rocklis:codex/server-paths

Conversation

@Rocklis

@Rocklis Rocklis commented Sep 5, 2026

Copy link
Copy Markdown

Issue for this PR

Closes #46498

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

Connecting to http://host/proxy currently sends requests to /api/... instead of /proxy/api/.... The health check fails before the client can connect.

Resolve API paths relative to the configured server path, with or without a trailing slash. The fix includes the Promise client generator, terminal WebSocket URLs, and the direct requests in api and import, so those keep working after the health check succeeds.

How did you verify your code works?

  • Local HTTP/SSE regression tests cover root URLs, /proxy, trailing slashes, nested paths, query parameters, and authentication headers. The four prefixed cases still fail on the upstream-only generator.
  • CLI subprocess tests cover raw API requests, OpenAPI operation lookup, and session import. Terminal tests check both ticket and WebSocket URLs.
  • After merging v2 at 33f48f36c, with Bun 1.4.2: client suite 166 passed, code generator suite 94 passed, focused CLI tests 12 passed. This includes the upstream SSE activity callback test.
  • Regenerated the client with bun run generate; a second run produces no changes. Typechecks pass in all three affected packages and all 33 repository pre-push tasks.

Screenshots / recordings

Not applicable.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Thanks for your contribution!

This PR doesn't have a linked issue. All PRs must reference an existing issue.

Please:

  1. Open an issue describing the bug/feature (if one doesn't exist)
  2. Add Fixes #<number> or Closes #<number> to this PR description

See CONTRIBUTING.md for details.

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

The following comment was made by an LLM, it may be inaccurate:

Based on my search, I found two PRs that are potentially related to this fix:

  1. feat(server): runtime base path support for reverse proxy deployments (feat(server): runtime base path support for reverse proxy deployments #28326)

    • Related because it also addresses reverse proxy deployments and base path handling, though this appears to be an older server-side feature implementation.
  2. feat: base path support (feat: base path support #7625)

    • Related because it deals with the same base path prefix concept, though this is likely an older implementation.

These PRs address similar concerns around URL path prefixes and proxy deployments, but PR #47442 appears to be focused specifically on the client-side fix to preserve those prefixes correctly when connecting through a proxy path like /proxy.

The other health check and connection-related PRs found are not directly related to the URL path prefix preservation issue.

@Rocklis

Rocklis commented Sep 5, 2026

Copy link
Copy Markdown
Author

@jlongster I've put the fix for #46498 here. The local HTTP/SSE and CLI regressions pass, including the direct api/import requests and terminal connection URLs.

The issue check looks like a false positive: the description already has Closes #46498, but the workflow running from dev only checks closingIssuesReferences, which is empty for this v2 PR. The body-reference fallback is present in the v2 workflow but not in dev.

Could you link the issue and clear needs:issue when reviewing? The fork test workflows also need approval.

Keep the upstream SSE activity callback alongside prefix-aware URL construction, then regenerate the Promise client.
@Rocklis

Rocklis commented Sep 6, 2026

Copy link
Copy Markdown
Author

Merged the latest v2 in 52074f3 to resolve the generator conflict from #47571. Both the SSE activity callback and the path-prefix fix are preserved, and the client has been regenerated.

Rechecked with Bun 1.4.2: 166 client tests, 94 generator tests, 12 CLI tests, and all 33 pre-push typechecks pass. The diff against v2 is still limited to the original fix.

@Rocklis

Rocklis commented Sep 9, 2026

Copy link
Copy Markdown
Author

Synced v2 in 916949b to pick up the session import move from #48055. The import regression now uses the new command and still checks the proxy prefix. The client has been regenerated, with the upstream SSE activity handling preserved.

On that commit, 165 client tests, 96 generator tests, and 12 CLI tests pass (273 total). The three package typechecks and all 35 pre-push typechecks pass too. The diff against the synced v2 base is still limited to the original 10 files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant