[robertn702]

What this fixes

#84: GET /v1/models returns 404 when a query string is present. Codex adds ?client_version=<version> when it refreshes its model catalog, so every refresh through the proxy failed. /health, /ready, /v1/responses, and /v1/messages used the same exact match on request.url and failed the same way.

What changed

• packages/proxy/src/server.ts
  • New requestPath(request): returns request.url up to the first ?.
  • createAppServer: the draining exemption (/health, /ready) and the proxied in-flight check now compare against requestPath.
  • handle: every route match and the /v1/messages provider choice now compare against requestPath. Method checks and the {"error":"not_found","message":"unknown route"} 404 are unchanged.
• packages/proxy/test/server.test.ts: new query strings suite.
  • /health, /ready, and /v1/models with ?client_version=1 return the same status and body as without it, and POST /v1/responses?... and POST /v1/messages?... reach the upstream.
  • An unknown path with a query string still returns the same 404 body.
• .changeset/proxy-route-query-string.md: patch bump for @reasoning-router/proxy.

Decision: split on ? instead of new URL(...).pathname

new URL(request.url, base).pathname would also normalize absolute-form request targets, so POST http://attacker.example:<port>/v1/responses would start matching a route. Today it returns 404, and an existing Host-validation test checks that. Splitting on ? fixes the bug and leaves that behavior alone. The query string is still not forwarded upstream (the same as before for the bare path).

Verification

• npm run check: typecheck and lint pass (1 Biome warning, also on main). Vitest: 36 files, 478 tests passed.
• The new routing test fails with the old server.ts and passes with the fix.
• Live check: built the proxy and ran node packages/proxy/dist/index.js on port 4392 against a stub upstream on 4391, with a bearer dummy key.

Before (old server.ts):

$ curl -s -o /dev/null -w '%{http_code}\n' http://127.0.0.1:4392/v1/models
200
$ curl -s -w '\n%{http_code}\n' 'http://127.0.0.1:4392/v1/models?client_version=1'
{"error":"not_found","message":"unknown route"}
404

After:

$ curl -s -o /dev/null -w '%{http_code}\n' http://127.0.0.1:4392/v1/models
200
$ curl -s -w '\n%{http_code}\n' 'http://127.0.0.1:4392/v1/models?client_version=1'
{"object":"list","data":[],"path":"/v1/models"}
200

Fixes #84

Summary by CodeRabbit

• Bug Fixes
  • Requests to recognized health, readiness, model, responses, and messages routes now work when the URL includes a query string.
  • Responses and messages requests continue to reach the upstream service at the expected route.
  • Unknown routes with query strings still return a 404 response.

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: b9b4e661-b0b1-4018-b7e7-f36f1e7cc8bf

Commits

Reviewing files that changed from the base of the PR and between db653e0 and 8f7139f.

Files selected for processing: 3

• .changeset/proxy-route-query-string.md
• packages/proxy/src/server.ts
• packages/proxy/test/server.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Walkthrough

The proxy now matches known routes using the request path without its query string. Tests cover health, readiness, models, responses, messages, and unknown routes with query strings.

Changes: Proxy route matching

Layer / File(s)	Summary
Query-free route matching packages/proxy/src/server.ts, packages/proxy/test/server.test.ts, .changeset/proxy-route-query-string.md	The proxy uses the query-free path for draining exemptions, route eligibility, and handling health, readiness, models, responses, and configured messages routes. Tests check route responses, query-free upstream paths, and the existing 404 response for unknown paths. A patch changeset records the change.

Priority: Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: Minimal · up to 8f713

Known routes can match requests with query strings, and no actionable merge-blocking risk is identified.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files. (1 skipped: 1 …	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: routing requests by pathname so query strings match known proxy routes.
Linked Issues check		Issue #84 requires known routes to work when the request includes a query string. packages/proxy/src/server.ts defines requestPath by removing the query portion and uses it for health, readiness, …
Out of Scope Changes check		The reviewed changes stay within issue #84. The implementation changes route-path handling, preserves absolute-form target behavior, and keeps query strings out of upstream requests. The added tests v…

Full details: Linked Issues check

Explanation

Issue #84 requires known routes to work when the request includes a query string. packages/proxy/src/server.ts defines requestPath by removing the query portion and uses it for health, readiness, models, responses, messages, draining, and in-flight routing checks. The new query strings tests cover the required routes and preserve the unknown-route 404 response. The tests also verify that upstream receives /models, /responses, and /messages without the query string.

Full details: Out of Scope Changes check

Explanation

The reviewed changes stay within issue #84. The implementation changes route-path handling, preserves absolute-form target behavior, and keeps query strings out of upstream requests. The added tests verify the required behavior. The proxy patch changeset supports the implementation release.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches: 💡 1

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[robertn702]

Superseded by #86, which implements the same path-based route matching and is now merged into main (2849392 + 77621f8 coverage, merge commit 866a2ca). This branch came from the first worker attempt that was stopped mid-run, so it carries the same fix without the extra route-level coverage. Closing rather than merging to keep one implementation of the change.