fix(services): route under the public /services prefix without a stripping proxy - #6702
fix(services): route under the public /services prefix without a stripping proxy#6702mmabrouk wants to merge 1 commit into
Conversation
…pping proxy Traefik and the AWS ALB strip /services before a request reaches the services container, so its routes live at root. A managed ingress such as GKE cannot rewrite paths and forwards /services/... verbatim, which answered 404 on every route, including the agent invoke endpoint the playground uses. Add an ASGI middleware that strips the prefix inbound, in a loop and without redirects, the same way the api's ApiPrefixStripMiddleware does. The prefix comes from AGENTA_SERVICES_PATH_PREFIX, default /services; an empty value turns the strip off. Registered last so it runs before auth and routing. Claude-Session: https://claude.ai/code/session_01GZqpvjwLHgVnMrpKH8hzq8
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 SummarySummary by CodeRabbit
WalkthroughThe change adds configurable ChangesServices prefix handling
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: 🔵 Low · up to The new middleware enables services routes under a forwarded prefix, but encoded request paths may be altered after normalization and default configuration tests can be environment-dependent. These are bounded issues that should be addressed before relying on encoded-path behavior and test coverage. Sequence Diagram(s)sequenceDiagram
participant Ingress
participant ServicesPrefixStripMiddleware
participant FastAPI
Ingress->>ServicesPrefixStripMiddleware: Forward request with /services prefix
ServicesPrefixStripMiddleware->>ServicesPrefixStripMiddleware: Strip configured prefix
ServicesPrefixStripMiddleware->>FastAPI: Pass normalized request scope
FastAPI-->>Ingress: Return routed response
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Advanced
Run ID: 0b94556f-dda7-4b29-bdf4-6ea613286bef
📒 Files selected for processing (4)
services/entrypoints/main.pyservices/entrypoints/prefix.pyservices/oss/tests/pytest/unit/entrypoints/__init__.pyservices/oss/tests/pytest/unit/entrypoints/test_prefix_strip.py
Included review availability: Your plan provides up to 8 included reviews per hour; 1 remains after this review.
| scope = dict(scope) | ||
| scope["path"] = path | ||
| if isinstance(scope.get("raw_path"), (bytes, bytearray)): | ||
| scope["raw_path"] = path.encode("utf-8") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🤖 get_repo_knowledge executed:
get_repo_knowledge Agenta-AI/agenta /tmp/coderabbit-repo-knowledge/agenta-ai-agenta-4b53879a/conventions /tmp/coderabbit-repo-knowledge/agenta-ai-agenta-4b53879a/learnings
Length of output: 12008
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- target file ---'
sed -n '1,140p' services/entrypoints/prefix.py
printf '%s\n' '--- raw_path references ---'
rg -n -C 3 'raw_path|scope\["path"\]|scope\.get\(["'\'']path|scope\.get\(["'\'']raw_path' services
printf '%s\n' '--- relevant project files ---'
git ls-files | rg '(^|/)(prefix|middleware|entrypoint|router|routing|asgi|websocket)' | head -80Repository: Agenta-AI/agenta
Length of output: 6401
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- analogous API middleware ---'
sed -n '1,140p' api/oss/src/middlewares/prefix.py
printf '%s\n' '--- services prefix tests ---'
sed -n '1,240p' services/oss/tests/pytest/unit/entrypoints/test_prefix_strip.py
printf '%s\n' '--- API prefix tests ---'
sed -n '1,220p' api/oss/tests/pytest/unit/middlewares/test_prefix.py
printf '%s\n' '--- services raw_path consumers ---'
rg -n -C 4 'raw_path' services api/oss/src | head -160Repository: Agenta-AI/agenta
Length of output: 10445
🤖 get_repo_knowledge executed:
get_repo_knowledge Agenta-AI/agenta /tmp/coderabbit-repo-knowledge/agenta-ai-agenta-4b53879a/conventions /tmp/coderabbit-repo-knowledge/agenta-ai-agenta-4b53879a/learnings
Length of output: 14662
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- services entrypoint wiring ---'
sed -n '1,220p' services/entrypoints/main.py
printf '%s\n' '--- services middleware and raw_path references ---'
rg -n -C 5 'ServicesPrefixStripMiddleware|raw_path|scope\[["'\'']path["'\'']\]|scope\.get\(' services --glob '*.py'
printf '%s\n' '--- relevant diff ---'
git diff -- services/entrypoints/prefix.py services/oss/tests/pytest/unit/entrypoints/test_prefix_strip.pyRepository: Agenta-AI/agenta
Length of output: 12634
🌐 Web query:
official ASGI HTTP scope raw_path original encoded bytes specification
💡 Result:
According to the official ASGI specification, the raw_path key in the HTTP connection scope is defined as a byte string [1][2][3]. Its technical specification includes the following details: - Type: It must be a byte string [1][3]. - Content: It represents the original HTTP path component, excluding any query string [1][2][3]. It remains unmodified from the bytes as they were received by the web server [1][3]. - Status: It is an optional field [1][3]. If an underlying web server implementation is unable to provide the raw path, this field may be missing, in which case it defaults to None [1][3]. Unlike the path field, which is a Unicode string that has been processed to decode percent-encoded sequences and UTF-8 byte sequences, raw_path is intended to preserve the original byte-level representation of the path as received on the wire [1][2]. This allows applications to recover information that might otherwise be lost due to automatic unquoting by the server [4]. Note that it does not include the query string [1][3].
Citations:
- 1: https://asgi.readthedocs.io/en/stable/specs/www.html
- 2: https://github.com/django/asgiref/blob/master/specs/www.rst
- 3: https://asgi.readthedocs.io/en/latest/specs/www.html
- 4: GitHub issue 87 in django/asgiref (link omitted to avoid creating a cross-reference)
Preserve the request encoding in raw_path.
When _strip removes the prefix, line 42 rebuilds raw_path from decoded path. For /services/caf%C3%A9, downstream receives b"/caf\xc3\xa9" instead of b"/caf%C3%A9". Update raw_path from its existing bytes, remove the bytes for the consumed prefix, and preserve the remaining encoded bytes.
| from entrypoints.prefix import ServicesPrefixStripMiddleware | ||
|
|
||
|
|
||
| def _app(prefix=None) -> TestClient: |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Isolate default-prefix tests from AGENTA_SERVICES_PATH_PREFIX.
_app() passes None, so the middleware reads the process environment. If the test process sets AGENTA_SERVICES_PATH_PREFIX to another value, the default-prefix tests fail or validate the wrong prefix.
Clear this variable with monkeypatch for the default tests. Add a separate test for environment-based configuration.
Railway Preview Environment
|
1 participant
Why
The services app is published under
/serviceson the shared host. Traefik and the AWS ALB strip that prefix before the request reaches the container, so the routes live at root. A managed ingress such as GKE cannot rewrite paths and forwards/services/...verbatim. On the GKE staging stage every services route answered 404, including/services/agent/v0/invoke, the endpoint the playground and the release gate drive.The api already solves this with
ApiPrefixStripMiddleware. The services app had nothing equivalent.What changed
services/entrypoints/prefix.py: an ASGI middleware that strips the prefix inbound, in a loop so a double prefix still routes, and never issues a redirect. The prefix comes fromAGENTA_SERVICES_PATH_PREFIX, default/services; an empty value turns the strip off.services/entrypoints/main.py, so it runs before auth and routing.Behavior behind Traefik is unchanged: a request that arrives already stripped has no prefix to remove.
How to verify
Live: with this image the GKE staging stage answers
/services/agent/v0/invokeand the release gate's chat journey runs.https://claude.ai/code/session_01GZqpvjwLHgVnMrpKH8hzq8