Skip to content

fix(a2a): scope A2A_PUBLIC_URL per multiplex profile - #110131

Closed
EloquentBrush0x wants to merge 1 commit into
NousResearch:mainfrom
EloquentBrush0x:fix/a2a-scope-public-url
Closed

EloquentBrush0x wants to merge 1 commit into
NousResearch:mainfrom
EloquentBrush0x:fix/a2a-scope-public-url

Conversation

@EloquentBrush0x

Copy link
Copy Markdown

Summary

  • A2A_PORT and A2A_ADVERTISED_TOOLSETS are already captured at construction time (inside _profile_runtime_scope) via _get_scoped_secret(), but A2A_PUBLIC_URL was still read with a bare os.getenv() inside A2ARequestHandler._request_public_url() — which runs on ThreadingHTTPServer's per-connection OS thread, not the constructing thread.
  • Raw threading.Thread never inherits contextvars, so simply swapping the reader to _get_scoped_secret() at that call site would not have helped: the request thread has no scope installed, so get_scoped_secret()'s internal UnscopedSecretError handling would just fall back to os.environ anyway — the value has to be captured once at construction time (which does run in profile scope) and threaded through as instance state, same shape as the existing A2A_PORT fix.
  • A secondary multiplex profile without its own A2A_PUBLIC_URL now falls back to the X-Forwarded-Host/Host-derived URL (or the bind host), instead of silently advertising the default profile's public URL in its Agent Card / discovery response.

Scope note: I initially also fixed the analogous A2A_REPLY_TIMEOUT gap in _await_reply/_rpc_tasks_subscribe, but a fresh competitor check turned up an active, more comprehensive open PR (#91686, "make reply deadline config-native") that already captures self.reply_timeout at construction time and fixes the exact same two call sites, plus adds config.yaml-native configuration. I dropped that half from this PR to avoid duplicating #91686 — this PR is scoped to A2A_PUBLIC_URL only.

Test plan

  • Extended TestMultiplexConstructionScope in tests/plugins/test_a2a_plugin.py (the existing scoped-construction test class for this exact adapter) with _public_url assertions in both directions: a secondary profile's own A2A_PUBLIC_URL is honored, a secondary profile without one falls back to "" (not the default profile's URL), and the unscoped default-profile path still gets its own env value.
  • Mutation-verified: reverting the fix makes both extended tests fail with AttributeError: 'A2AAdapter' object has no attribute '_public_url'.
  • tests/plugins/test_a2a_plugin.py (full suite, -m "" to include normally-deselected slices): 115 passed, no regressions.

🤖 Generated with Claude Code

A2A_PORT and A2A_ADVERTISED_TOOLSETS are already captured at
construction time (inside _profile_runtime_scope) via
_get_scoped_secret(), but A2A_PUBLIC_URL was still read with a bare
os.getenv() inside A2ARequestHandler._request_public_url() - which
runs on ThreadingHTTPServer's per-connection OS thread, not the
constructing thread.

Raw threading.Thread never inherits contextvars, so even swapping the
reader to _get_scoped_secret() at that call site would not help: the
request thread has no scope, secret_scope falls back to os.environ
either way. The value must be captured once at construction time
(which does run in profile scope) and threaded through as instance
state instead - same fix shape as A2A_PORT above.

A secondary multiplex profile without its own A2A_PUBLIC_URL now
falls back to the X-Forwarded-Host/Host-derived URL (or the bind
host) instead of silently advertising the default profile's public
URL in its Agent Card / discovery response.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/plugins Plugin system and bundled plugins area/profiles Multi-profile isolation, HERMES_HOME scoping sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Sep 13, 2026
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks @EloquentBrush0x — landed on main via #110293 (merge b602ba296e81). Your commit is on main as 8c58e4f97651 with authorship preserved, taken as-is — you were right that a call-site swap alone would not work because the per-connection handler thread has no contextvar scope, so capturing _public_url at construction is the correct shape. Closing in favour of the merged salvage.

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

Labels

area/profiles Multi-profile isolation, HERMES_HOME scoping comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants