Skip to content

feat: add broker-mode ServiceX client factory (--broker-url) - #3

Merged
kratsg merged 4 commits into
mainfrom
feat/broker-mode-client-factory
Sep 9, 2026
Merged

kratsg merged 4 commits into
mainfrom
feat/broker-mode-client-factory

Conversation

@kratsg

@kratsg kratsg commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds --broker-url and a second HTTP-mode auth factory, BrokerServiceXClientFactory, alongside the existing bridge-mode BearerTokenClientFactory.

  • --broker-url CLI flag (cli.py): when set, --transport http runs in broker mode — clients authenticate with an AF Broker Identity Token (forwarded by af-mcp-broker's aggregator), not a ServiceX personal refresh token.
  • _BrokerServiceXAdapter (auth/factory.py): a ServiceXAdapter subclass that overrides only _get_token() to redeem the access token from the broker instead of exchanging a refresh token via ServiceX's own /token/refresh. _get_authorization()'s existing JWT-expiry-aware caching is untouched, so this re-redeems exactly as lazily as the stock path re-refreshes.
  • BrokerServiceXClientFactory: mirrors BearerTokenClientFactory's per-session caching exactly; no redeem call happens in get_client() itself — it's deferred to the adapter's own lazy _get_token().
  • ServiceXRedeemer protocol: the redeem call is injected via structural typing rather than a hard import of af_credentials.proxy.ProxyClient, so auth/factory.py's tests stay independent of af-credentials' release status. The one concrete ProxyClient(broker_url, kind="servicex") construction lives in server.py.
  • classify_error (tools/_helpers.py): two new branches distinguishing "no ServiceX credential linked at the broker" (ProxyNotAvailableError) from "broker/servicex-token-service infra failure" (ProxyRedeemError) — these propagate automatically through every existing tool call's except Exception as exc: return classify_error(exc), no per-tool changes needed.
  • _make_broker_http_mcp (server.py): broker mode registers no OAuth 2.1 authorization server of its own and no /bridge routes — the broker's aggregator already authenticated the caller. resource_url-derived transport security (DNS-rebinding protection) still applies uniformly via serve_http.

Depends on / coordinate with

Known follow-up

pixi.lock could not be regenerated in this environment: this workspace's exclude-newer = "7d" policy excludes af-credentials 0.3.0 (uploaded within the last 7 days as of this PR). pyproject.toml/pixi.toml correctly declare the dependency; pixi.lock will need pixi lock/pixi install re-run once that window passes (or exclude-newer is bumped).

Test plan

  • New unit tests for _BrokerServiceXAdapter, build_broker_servicex_client, BrokerServiceXClientFactory, the two new classify_error branches, --broker-url CLI wiring, and _make_broker_http_mcp/serve_http broker-mode dispatch.
  • Full suite (297 tests, excluding the integration/ live tests) green.
  • mypy, ruff check, pylint (10.00/10), pydocstyle all clean.
  • Verified in a scratch venv with af-credentials==0.3.0 actually installed (real ProxyClient/ProxyNotAvailableError/ProxyRedeemError imports, not mocks) since pixi install is blocked by the lockfile issue above.

🤖 Generated with Claude Code

Adds BrokerServiceXClientFactory and _BrokerServiceXAdapter, mirroring
BearerTokenClientFactory's shape but authenticating clients via an AF
Broker Identity Token (forwarded by af-mcp-broker's aggregator instead
of a ServiceX personal refresh token. The actual ServiceX access token
is redeemed lazily from the broker's POST /v1/credentials/servicex/redeem
via a ServiceXRedeemer protocol, satisfied by af-credentials' ProxyClient
once its kind=servicex support ships (maniaclab/af-credentials#9).

Closes #2

Assisted-by: Claude Sonnet 5 <noreply@anthropic.com>
EOF
)
af-credentials' published conda-forge build depends on python>=3.11
(verified via `pixi search`), which conflicted with this project's
py310 pixi environment once af-credentials became a hard runtime
dependency for broker mode (server.py imports it unconditionally at
module level, so the whole package -- not just broker mode -- was
unimportable under 3.10).

Raise the floor to python>=3.11 everywhere (pyproject.toml classifiers,
mypy/pylint target versions, pixi.toml's py310 feature/environment, CI
matrix) rather than special-casing the import, since af-credentials is
now a first-class dependency for this project's HTTP transport, not an
optional extra.

Also fixes two lint issues surfaced by re-locking with a synced dev
environment: the mypy pre-commit hook's additional_dependencies was
missing af-credentials entirely (mypy was silently treating the whole
module as untyped and flagging the deliberate type: ignore as unused),
and ruff's UP017 rule now fires under the 3.11 floor (datetime.UTC
alias).

Assisted-by: Claude Sonnet 5 <noreply@anthropic.com>
@kratsg

kratsg commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator Author

Update: dropped python 3.10 support project-wide (pyproject.toml, pixi.toml's py310 feature/environment, CI matrix) — af-credentials' published conda-forge build depends on python>=3.11, and since server.py imports it unconditionally at module level, the whole package (not just broker mode) was unimportable under 3.10 once af-credentials became a hard dependency here.

af-credentials v0.3.1 (with the kind="servicex"/access_token() support from af-credentials#9) is merged and tagged upstream but not yet built on conda-forge (conda-forge/af-credentials-feedstock#11 is open for the version bump) — pixi.lock currently resolves against the still-published 0.3.0, which is why the structural ServiceXRedeemer protocol + the # type: ignore[arg-type] at the one concrete ProxyClient(...) construction site in server.py are both still load-bearing (mypy now genuinely enforces that ignore, since af-credentials is properly resolvable — added it to the mypy pre-commit hook's additional_dependencies, which had been silently missing). Once conda-forge picks up 0.3.1, re-run pixi install and both the protocol cast and the ignore comment can likely be removed as a small follow-up.

All environments (py311/py312/py313) green: 297 tests each, mypy/ruff/pylint(10.00/10)/pre-commit all clean.

af-credentials v0.3.1 (kind="servicex"/access_token() support,
maniaclab/af-credentials#9) is now built on conda-forge, so pixi.lock
picks it up directly. Tighten the version constraint to >=0.3.1 in
pyproject.toml and pixi.toml (0.3.0 predates the feature entirely --
the previous >=0.3.0 pin would have let a fresh install resolve a
version missing it).

Drop the cast()/type: ignore workaround at server.py's ProxyClient
construction site now that af-credentials is genuinely resolvable by
mypy: it surfaced a real structural mismatch, not just the stale
"unreleased interface" placeholder -- af-credentials' ServiceXAccessToken
is a frozen dataclass, but the ServiceXRedeemer protocol's
_RedeemedAccessToken declared access_token as a plain (read-write)
attribute, which PEP 544 protocols require write access for by default.
Redeclared it as a read-only @Property, which is all this code actually
needs and what frozen dataclasses can satisfy.

Assisted-by: Claude Sonnet 5 <noreply@anthropic.com>
@kratsg

kratsg commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator Author

Update: af-credentials 0.3.1 is now built on conda-forge, so pixi.lock was regenerated against the real release (pixi update af-credentials), the version constraint tightened to >=0.3.1 (0.3.0 predates the kind="servicex" feature entirely), and the cast()/# type: ignore workaround at server.py's ProxyClient construction site is gone.

That workaround turned out to be catching a real bug, not just a placeholder for an unreleased interface: af-credentials's ServiceXAccessToken is a frozen dataclass, but ServiceXRedeemer's _RedeemedAccessToken protocol declared access_token as a plain attribute, which PEP 544 protocols require write access for by default — a frozen dataclass can never satisfy that. Redeclared it as a read-only @property (all this code ever does is read it), which mypy now verifies cleanly with the real installed package.

All environments (py311/py312/py313) still green: 297 tests each, mypy/ruff/pylint(10.00/10)/pre-commit all clean against the real, released interface (no more structural-typing workaround needed).

@kratsg
kratsg merged commit 6f00355 into main Sep 9, 2026
11 checks passed
@kratsg
kratsg deleted the feat/broker-mode-client-factory branch September 18, 2026 11:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant