fix: resolve e2e cloud host from region, not a hardcoded default - #7
Conversation
The e2e cloud suite still encoded an invalid host and a key-only flow that now fails fast, so running it without XAGENT_BASE_URL routed runtime traffic to a dead host. - Resolve the cloud host from XAGENT_REGION (au/sg) or XAGENT_BASE_URL; there is no default host, since the service is per-region and a key only authenticates against the region that minted it. - The bad-key probe now takes a resolved host via the cloud_base_url fixture, so an empty host no longer raises ValueError in the constructor before the unauthorized path is exercised. - Drop the documentation describing a key-only workflow and a hosted default that no longer exist.
There was a problem hiding this comment.
Code Review
This pull request updates the end-to-end tests to support per-region cloud hosts by introducing a cloud_base_url fixture and helper functions in conftest.py that resolve the host using XAGENT_BASE_URL or XAGENT_REGION. The smoke tests are refactored to consume this new fixture. The feedback suggests normalizing the XAGENT_REGION environment variable to lowercase before passing it to the Region enum to avoid potential case-sensitivity issues.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
XAGENT_REGION=SG should resolve like sg; the Region enum matches on its lowercase value, so strip and lowercase the env var first.
rogercloud
left a comment
There was a problem hiding this comment.
Design verdict: sound ✅
This is the follow-up to the PR #6 review, and it resolves all three findings cleanly. The fix mirrors the library's own host-resolution contract inside the e2e test surface: a single _cloud_base_url() resolver (explicit XAGENT_BASE_URL wins → else XAGENT_REGION via the Region enum → else None/skip), exposed through a shared cloud_base_url fixture. No hardcoded host, no guessing — consistent with the per-region model the library now enforces.
Resolution of prior findings
- #1 [Major] — Fixed.
_runtime_base_url()and its hardcodedhttps://cloud.xagent.runare gone;test_workspace_full_flownow drives the runtimeAgentClientoff the resolvedcloud_base_url. - #2 [Minor] — Fixed.
test_bad_workspace_key_unauthorizednow takescloud_base_url, so it reaches theInvalidAPIKeyassertion instead of raisingValueErrorfrom the constructor whenXAGENT_BASE_URLwas unset. - #3 [Minor] — Fixed. Stale "defaults to the hosted endpoint" docstrings in
python/tests/e2e/conftest.pyandpython/tests/e2e/test_cloud_smoke.pyare corrected to theXAGENT_REGION/XAGENT_BASE_URLstory.
The earlier case-sensitivity suggestion is also handled — Region(region.strip().lower()) normalizes " SG ", AU, etc.
Verification
ruff checkandmypy --strictboth clean.- e2e suite collects and skips cleanly with no env set (0 errors).
- Resolution paths exercised:
none→None," SG "→sg host,AU→au host, explicitbase_urlwins, invalid region → clearValueError.
Minor observation (non-blocking)
An invalid XAGENT_REGION raises ValueError during fixture setup rather than pytest.skip. This is defensible fail-fast behavior — a typo'd region should be loud rather than silently skipped — so no change needed.
LGTM.
The e2e cloud suite still encoded an invalid host and a key-only flow that now fails fast: running it without
XAGENT_BASE_URLrouted runtime traffic to a dead host, and the bad-key probe raisedValueErrorin the constructor before it could reach the unauthorized path.Changes
XAGENT_REGION(au/sg) orXAGENT_BASE_URL. There is no default host: the service is per-region and a key only authenticates against the region that minted it.cloud_base_urlfixture, so an empty host can no longer raiseValueErrorbefore the unauthorized path runs.Verification
ruff+mypy --strictclean,pytest215 passed, the e2e suite collects cleanly (skipped by default; not run in CI).