The auto-notify added in the prior commit awaited _notify_nodes_mcp_reload
inline in create/update/delete, coupling each admin write's latency — and
success — to cluster reachability: on a large cluster with slow/unreachable
nodes the write could hang up to ceil(nodes/fan_out_limit)*30s behind the
fan-out, and a post-commit fan-out error would 500 a write that already landed.
Schedule the fan-out as a BackgroundTask that runs AFTER the 200 instead — the
"trigger, not drain" contract already used by _cascade_cancel_to_children — so
the write's response is never blocked on, nor failed by, the fan-out. The
pre-existing registry-install path is converted the same way for consistency.
There is no periodic node->DB reconcile, so a node that misses the reload serves
a stale MCP catalog until the next POST /reload. The background _run therefore
logs any unreached node (or a systemic fan-out fault) at WARNING — visible at
the default INFO level — rather than swallowing it; the per-node status view
also surfaces the divergence. A non-2xx reply from a node's reload/action
endpoint now counts as a failure (raise_for_status) rather than a reached node,
so neither the WARNING nor the operator /reload results miss a 5xx node.
Revert the getattr None-guard on _notify_nodes_mcp_reload: it turned the
operator-triggered POST /reload into a silent success ({} with 200) when the
fan-out infra was absent — a fail-loudly violation — and diverged from the
unguarded sibling _notify_nodes_mcp_action. The helper is drain-style again,
awaited only by /reload (which must surface fan-out failures); writes go through
the best-effort scheduler.
Tests: assert the reload is NOT scheduled on a delete/update 404 or a create
secret-store 503; that an unreached-node, raising, or non-2xx fan-out is logged
at WARNING / recorded as an error; and that operator POST /reload fails loudly
(500) without fan-out infra.
oauth_user/oauth_obo servers hold no cluster-level session — they connect per-user on demand — so the admin status pill rendered 'connecting'/'idle', which reads as broken, when zero warm users is the normal resting state. Render 'per-user' for pool-backed servers instead.
admin_create/update/delete_mcp_server wrote to the DB but never told nodes to reconcile — only the registry-install path and the explicit /reload did — so a programmatic create/edit/delete was inert on nodes until a manual reload (and the mid-session re-prime self-heal never fired). Call _notify_nodes_mcp_reload after each write, mirroring registry-install; also make that helper best-effort (skip when the cluster fan-out infra is absent) so a write can't 500 on it.
Microsoft Entra's discovery document advertises userinfo_endpoint on graph.microsoft.com — a host distinct from the login.microsoftonline.com issuer — so discover_oidc's cross-host guard rejected it and disabled OIDC unless the operator set trusted_endpoint_hosts. Add login.microsoftonline.com to the built-in KNOWN_TRUSTED_OAUTH_ENDPOINT_HOSTS allow-list (mirroring the Google entry) so Azure AD OIDC works with no extra configuration. Surfaced by the live obo integration test.
- detect an in-place oauth_user<->oauth_obo flip by diffing the pool servers' (name -> auth_type) view instead of names only, so a migrated server re-primes active sessions (a name-only diff saw the same name on both sides and missed it);
- guard prime_user_pools per-user so one scheduling failure can't propagate out of reconcile_sync (500 the reload) or skip the remaining users;
- log what was SCHEDULED (prime is fire-and-forget and no-ops for credential-less users / a down loop), not 're-primed', and take an int changed-count instead of a set whose name falsely implied per-server scoping.
Startup key-enforcement counts ALL user-scoped auth types (oauth_user and oauth_obo, per is_user_scoped_auth), and the entra mint leg always carries scope=<audience>/.default (per-server oauth_scopes is ignored on that leg). The docstrings named only oauth_user / left the scope behavior ambiguous. Comment-only; no behavior change.
prime_user_pools runs once at ChatSession start, so an oauth_user/oauth_obo server registered while a session is already open never reached it — and for oauth_obo (no consent flow) priming is the ONLY path tools take into the catalog, so a mid-session registration stayed invisible until the session restarted. reconcile_sync now diffs the pool-server name set and re-primes every active session's user when a new server appears; idempotent (skips already-warm pools) and a no-op for users without a captured credential.
Round-12 review follow-up (no correctness findings). obo_grant_profile is
a static config field that OIDC runtime rediscovery never changes, so
recomputing profile/mint after maybe_rediscover_oidc was dead work that
implied the grant profile could change across a heal (it cannot). Re-read
only the discovery-derived state (enabled / token_endpoint).
The remaining review findings are accepted by design: the credential-
rotation CAS's sub-millisecond read->write window (self-heals on next
login; a full fix needs SELECT FOR UPDATE or a version column) and the
per-server delete loop on identity deletion (the per-server try/except
buys partial-failure resilience a single bulk delete would not).
Round-11 review follow-up — no correctness findings; efficiency/DRY cleanups.
- Session-start priming already confirms the captured credential exists
once for all of a user's obo servers, but each per-server
get_obo_access_token_classified re-read it pre-lock (N+1 reads). The
priming path now passes credential_present=True so the per-server
existence read is skipped; other callers keep their own read.
- _clear_pending_consent_best_effort (the sweep clear path) now routes
through _mark_pending_consent_cleared instead of inlining the
prune-then-stamp step, matching the helper's documented contract so the
two DB-confirmed clear sites can't drift.
The per-dispatch pending-consent clear's DELETE volume and the removed
interactive.js no-consent-URL fallback are left as-is: the former is the
deliberate, TTL-bounded cost of cross-node badge self-heal, and the
latter is unreachable for oauth_user (which always carries a consent_url)
and intended for oauth_obo (which has no per-server consent flow).
Round-10 review follow-up.
- A server scoped under obo_grant_profile=rfc8693 that survives a switch
to the entra profile mints <audience>/.default (the entra leg cannot
honor per-server oauth_scopes), but the cache row recorded the
configured narrow scope — so _is_fresh_obo_cache_row kept serving the
broad .default bearer believing it was narrow, and a scope change that
can't apply under entra looked like it had. The freshness gate and the
cache row now record the EFFECTIVE scope the leg actually mints ('' for
entra, the configured scope for rfc8693); the raw scope is still passed
to the mint so the entra leg's "oauth_scopes ignored" warning still
surfaces the misconfigured leftover.
Cleanup: the R9-5 single-per-mint client made every token-POST caller pass
a non-None client, so the transient-client fallback in _hardened_token_post
was dead and two doc/comment blocks described the opposite of the real
behavior. Removed the dead branch, tightened the http_client typing across
the mint chain, and corrected the docs.
Round-9 review follow-up.
- Runtime OIDC rediscovery was triggered only from the obo mint path,
which needs an already-signed-in user — so a single-node install (or
one where every node booted during a transient IdP outage) kept OIDC
LOGIN dark until an operator restart. The authorize and callback
handlers now trigger maybe_rediscover_oidc before their enabled gate,
so login self-heals too.
- A transient storage error on the obo mint-cache write (delete+create)
raised out of get_obo_access_token_classified, discarding a valid
just-minted token and breaking the classified-result contract. The
cache write is now best-effort — the working bearer is returned and the
next dispatch re-mints. Likewise the runtime rediscovery's discover_oidc
call is wrapped in except Exception (like the boot path) so an
unexpected discovery error can't escape the mint's contract.
- Login-time credential capture could race an in-flight mint on a
strict-rotation IdP: the mint's rotation write-back would clobber the
fresh login refresh token with a stale rotated one. The rotation
write-back is now a value compare-and-swap against the token the mint
read, so a credential a concurrent login just refreshed is not
overwritten.
Cleanup: the rfc8693 mint now opens one transient httpx client for the
whole mint so the token-exchange leg reuses the refresh leg's connection
instead of a second TLS handshake.
Round-8 review follow-up — two correctness follow-ons from the round-7
rediscovery/console-gate fixes, plus two cleanups.
- The console obo write gate ran the OIDC-deployment checks on EVERY
update, so once OIDC was operator-disabled any edit of an existing
oauth_obo server — including the natural remedy of setting
enabled=false — was rejected 400, leaving DELETE as the only way out.
The deployment-level checks (encryption key, OIDC enabled/configured,
capture opt-in, valid grant profile) now run only when a write is a NEW
obo enablement (create or flip INTO obo); a same-type edit keeps only
the per-server validity checks (audience required, entra-scope reject),
so an operator can always disable or edit an existing obo server.
- Probing rediscovery with enabled forced True carried the retryable boot
flag into discover_oidc, whose config-error branches returned enabled=
False without clearing it, so a config-invalid IdP (an endpoint failing
SSRF/same-origin validation) re-probed every 60s forever. The config-
error branches now latch discovery_retryable=False (terminal), and
maybe_rediscover installs that terminal config so the node stops
probing; the transient fetch/degraded branches keep retrying.
Cleanups: fold the obo missing-expires_in fallback into
_expires_at_from_response via a default_ttl_seconds param (one owner of
the stored-expiry format), and drop the redundant audience-change
inequality already guaranteed by the no-op normalization (matching the
sibling scopes_changing).
Round-7 review follow-up.
- The runtime OIDC re-discovery feature was dead code: discover_oidc
PRESERVES the input config's `enabled` flag on success (only
load_oidc_config ever sets it True), and maybe_rediscover_oidc always
probed from the disabled boot config, so a successful rediscovery still
returned enabled=False and the config swap was unreachable — the whole
boot-outage auto-heal never worked. It now probes with enabled forced on
so the flag is a reliable success signal. The unit test that "covered"
this was mocking discover_oidc to return enabled=True, masking the bug;
it now drives the real discover_oidc through a mocked HTTP discovery GET.
- The console never runs runtime rediscovery, so a transient discovery
failure at console boot made every oauth_obo server un-editable and
un-disable-able. The write gate now accepts a discovery_retryable config
(OIDC configured, discovery transiently down) and rejects only a
genuinely absent OIDC.
- The first rediscovery probe was suppressed for ~60s after host boot
because the "last probe" timestamp defaulted to 0.0; it now uses a None
sentinel for "never probed".
- Two behavior-preservation fixes for the pre-existing oauth_user path:
the shared hardened token-POST no longer escalates oauth_user oversized
error bodies (that status-based classification is opt-in for the obo
legs only), and the token_revoked audit fires unconditionally for
oauth_user again (a refresh failure means a real grant died) while
staying delete-gated for obo to avoid revocation rows for tokens that
never existed.
Cleanups: drop a throwaway set allocation in the pool-emptiness check,
compute the create handler's cleaned OAuth text once, remove a dead
no-op pop with a false comment, and simplify the cleared-map prune to two
non-overlapping passes.
Round-6 review follow-up — no CONFIRMED correctness bugs; one plausible
edge case and four DRY/drift cleanups.
- The shared hardened token-POST raised its 64KB body-size guard with the
default TRANSIENT class before the non-200 was classified, so a permanent
dead-grant whose error body exceeded the cap looped "please retry"
forever and never escalated. An over-sized client-error response is now
classified AMBIGUOUS by status (without reading the over-sized body), so
it still escalates to the honest re-login/admin remedy after the streak.
- The admin update handler re-derived the is_flip predicate inline in the
three token-purge guards (and computed target_auth / auth_type_now as two
names for the same effective auth type). Both now reuse the single
is_flip / target_auth derivations, so the purge guards and the column
scrub can't desync on what counts as a flip.
- The oauth_obo server-name scan was hand-rolled in two places (the
connections-list filter and the identity-delete cache purge) with
divergent null handling. Extracted obo_server_names(storage) so a change
to how sign-in-passthrough is recognised can't leave one path silently
missing servers.
- Inlined the two single-use _*_detail wrappers into direct
_pool_error_detail calls, keeping named wrappers only for the
multi-caller situations.
Round-5 review follow-up — three CONFIRMED (one security) plus two
correctness issues, all traceable to earlier fixes in this branch.
SECURITY: the round-2 redesign gated the "scrub OAuth columns this
auth_type doesn't use" on is_flip, replacing the old unconditional
scrub. A same-type static/none/obo edit could then inject an
oauth_authorization_server_url that survived a later flip to oauth_user
(which uses that column) and redirected every consenting user's OAuth
traffic to an attacker AS. The scrub is now applied on EVERY write, and a
flip into oauth_user recomputes the oauth_user-only columns from the
request so a stale value can't carry in — the persisted OAuth columns
are once again a pure function of the target auth_type.
- The oauth_obo write gate now also requires capture_user_credential to
be enabled: without it, login persists no credential and every dispatch
returns "missing" with a remedy that can never succeed — the permanent
misconfig the gate exists to reject.
- A permanent obo mint failure arms the cooldown (its shared credential
survives the per-server revoke), but the in-cooldown short-circuit
reported it as a retryable transient for the whole window, flapping
against the honest re-login/admin affordance. The backoff state now
records whether the arming failure was permanent, and the short-circuit
surfaces the matching classification.
- The ambiguous-escalation revoke cleared the cooldown without re-arming;
for obo (surviving credential) that let the next dispatch immediately
re-mint against the still-failing IdP. It now re-arms the same terminal
backstop the permanent branch has.
- The force-refresh reuse gate keyed on the cache row's 1-second `created`
time, which couldn't tell a concurrent peer's fresh mint from the
caller's own just-rejected token minted in the same second — so a retry
could re-serve the rejected bearer. It now decides by token identity
(the under-lock row differs from the pre-lock one), preserving the
single-flight reuse while never re-serving a rejected token.
Also: guard _pool_error_detail's str.format so placeholder-free copy
can't raise inside the error renderer, and note why the connections-list
classifies obo rows by authoritative auth_type on that cold path.
Round-4 review follow-up — no correctness findings; these are the four
cleanups it surfaced.
- The obo mint path decrypted the captured IdP refresh token twice per
mint: once pre-lock only to test presence, then again under the lock.
The pre-lock presence check now uses the raw existence read (no
decrypt), mirroring the priming path; the single authoritative decrypt
happens under the lock. Removes N throwaway decrypts per user at
session-start priming across N obo servers.
- The "arm the per-(user,server) cooldown" idiom was written inline at
four failure sites. Extracted _arm_cooldown (returns the backoff state
so the streak-mutating callers reuse it), so a change to how backoff
works is one edit.
- The oauth_user|obo pool-membership union was rebuilt inline at three
iteration sites. Added a _pool_server_names property, the set-level
counterpart to _is_pool_server, so a future third pool-backed auth type
is registered in one place.
- The four per-situation remediation-copy helpers each repeated the
oauth_user-vs-obo branch. Consolidated the copy into one
(auth_model, situation) table behind _pool_error_detail — the single
place the auth-model decision is made — so a dispatch site can't pair a
situation with the wrong auth model's copy (the wrong-remediation bug
class this review caught repeatedly). The named helpers remain as thin,
tested wrappers.
Round-3 review follow-up.
- The startup guard that refuses to boot without a token-encryption key
when capture_user_credential is enabled was gated on oidc_config.enabled.
Enabled reflects whether OIDC *discovery* succeeded, which is transient:
a node that boots while the IdP is unreachable comes up enabled=False,
so the guard was silently skipped exactly when it was needed, and runtime
rediscovery would later re-enable OIDC with the first login persisting a
refresh token and no key. Gate on the operator's capture opt-in alone
(a static config value), independent of discovery state.
- An obo mint response omitting the RFC 8693-optional expires_in cached
expires_at=NULL, which the freshness gate reads as never-expiring — fine
for opaque oauth_user tokens, wrong for a short-lived minted token, which
would then be served indefinitely and defeat audience/scope narrowing
that relies on TTL turnover. Fall back to a bounded default expiry.
- The empty-token fallback in the shared pool-lookup error mapping now uses
the auth-model-aware consent detail like its sibling missing branch, so an
obo row never shows per-server-consent copy with a null consent URL.
- Documented the _build_consent_url invariant at the chat error-card render
gate: oauth_user rows always carry a consent URL, so gating the Connect
button on its presence never hides a needed button for them; the button's
absence for sign-in passthrough is intended (the detail text is the
affordance).
Round-2 review follow-up. The headline is a redesign of the OAuth
column carry-over so scopes/audience can no longer leak or vanish across
an auth-type flip:
- oauth_audience and oauth_scopes keep their meaning only WITHIN an auth
type (a resource indicator vs. an IdP app id; AS-consent scopes vs. an
rfc8693 exchange scope). On any oauth_user<->oauth_obo flip they are
now recomputed from the request (present -> value, absent -> NULL) and
never carried from the old row. A shared _oauth_columns_to_clear policy
drives both the create and update handlers. No-op normalization of a
re-sent equal value applies only to same-type edits.
- The console form clears both semantic fields when the auth type
changes and always submits the visible values; the previous
"omit unchanged scopes" logic collided with the backend's flip
handling and could silently drop or carry scopes.
Write-time validation now rejects oauth_obo rows that can never mint —
OIDC disabled/unconfigured, or an invalid obo_grant_profile — instead of
letting them surface per-dispatch as a retryable transient that never
heals.
Honest failure affordances for sign-in passthrough (no per-server
consent flow exists):
- the token_revoked audit fires only when a row was actually deleted, so
a permanent mint rejection against a surviving credential no longer
appends a bogus revocation on every post-cooldown dispatch/prime;
- the 403 insufficient-scope detail and the chat error card's action
button are now auth-model-aware — obo errors point at the
administrator rather than a dead-end re-consent, and the Connect button
renders only when a real consent URL is present;
- the read-side freshness gate now enforces scopes as well as audience,
so an rfc8693 scope narrowing takes effect on the next dispatch even if
the best-effort admin cache purge failed.
Cleanups: the five decrypt-failure result constructions collapse into
_decrypt_failure_result; the cleared-pairs TTL bookkeeping into
_mark_pending_consent_cleared; drop the dead USER_SCOPED_AUTH_TYPES
re-export from mcp_oauth; correct the now-bidirectional oidc<->mcp_oauth
lazy-import note. Docs updated for the flip semantics and the OIDC
prerequisite.
Mint engine: guard the credential-rotation persist so a storage blip
cannot escape the classified-result contract mid-mint (and cannot brick
the user's other obo servers on strict-rotation IdPs); stop borrowing
the login flow's httpx client across event loops — mints use a transient
per-request client (obo_http_client remains as a test seam); retry OIDC
discovery at runtime (cooldown-gated, single-flight) so a node that
booted during an IdP outage can mint again without a restart; key the
under-lock force-refresh reuse gate on created, which delete+create
makes the mint time (obo rows never set last_refreshed, so the copied
oauth_user gate never fired and serialized waiters each re-redeemed).
Cross-node consent badges: the cleared-pairs set becomes a TTL map with
bounded growth, so a badge written by another node after this node's
last clear self-heals within one TTL window instead of surviving until
a restart.
Admin lifecycle: purge the mint cache when oauth_scopes changes on an
obo row (an rfc8693 privilege reduction now applies immediately, like
audience changes); normalize no-op scope/audience re-sends out of
updates — the admin form re-submits pre-filled fields on every save,
which both re-triggered purges and made entra-profile rows with legacy
scopes un-editable; make flip-into-obo scope handling grant-profile
aware (entra clears the carry-over, rfc8693 honors the request); clear
obo-era audience/scopes when flipping back to oauth_user (the IdP-side
app identifier is not a resource indicator); mirror the same column
policy in the create handler.
Revocation honesty: hide obo mint-cache rows from the user connections
list and refuse the per-server disconnect with 409 — deleting the row
returned 204, audited token_revoked, and then session-start priming
silently re-minted from the surviving captured credential.
Console form: keep the audience-from-URL autofill off for sign-in
passthrough (the audience there is an IdP application identifier, and
the prefilled URL passed every validation layer then failed every
mint); clear the autofill artifact when switching modes; omit unchanged
scopes from submissions.
Dispatchers: route tool/resource/prompt through one shared lookup-error
mapping and an auth-model-aware 401-exhausted detail (obo users are no
longer pointed at a consent flow that does not exist). The consent-url
audit count drops 13 → 7: the three per-dispatcher mapping copies
collapsed into _pool_lookup_error.
Priming: skip all obo servers for users with no captured credential via
one existence SELECT (previously three reads per server per session).
Also: USER_SCOPED_AUTH_TYPES now lives in storage._protocol so the
backend SQL predicates share the application layer's set; docs describe
the actual purge-on-transition behavior (the orphan-and-reactivate
claims were wrong); the entra e2e setup script no longer aborts
silently under set -e with suppressed stderr.
Frontend↔backend interaction bugs the backend-only rounds couldn't see:
- flip oauth_user->oauth_obo: the admin form re-submits the pre-filled
oauth_user scopes, so the flip-clear (gated on 'oauth_scopes' not in
body) was skipped -> rfc8693 mints broke permanently. Clear now
compares to the existing value, robust to the re-send.
- entra edit-lockout: update validated the MERGED scopes, so a
pre-existing scoped obo row under the entra profile became un-editable
(every PUT 400'd). Reject only when the request actually SETS scopes.
- flush-cache button never rendered: consented_users_count is now
populated for oauth_obo rows too, not just oauth_user.
Mint engine + priming:
- audience guard: a cached token minted for a since-narrowed audience is
no longer served (extracted _is_fresh_obo_cache_row, used pre/post-lock,
checks refresh-less + audience-match + fresh). _persist_obo_cache_row
now delete+creates so the row's audience column tracks the mint (a
plain update kept the stale audience -> re-mint loop).
- obo session priming passes revoke_ambiguous_escalation=False (new param
threaded through get_obo_...), so an IdP wobble during a bulk prime
can't escalate-revoke obo cache rows cluster-wide.
Cross-node + lifecycle:
- pending-consent success-clear now clears once-per-failure-cycle via a
_pending_consent_cleared set (was gated on 'we wrote it' -> never fired
cross-node/after-restart -> stale badge). Still no per-call SQL.
- identity-unlink cache purge: per-server try/except so one failure
doesn't leave other servers' bearers un-purged.
- entra ignored-scopes: warn once per audience (was per-mint flood ->
downgraded to debug -> no signal on a profile switch).
- entra_setup.sh writes single-quoted .env values (secret may contain $).
+6 regression tests. 1892 mcp/oidc/console tests green; mypy clean.
Refs #551.
Manual (non-CI) harnesses that exercise the real oauth_obo mint path
against a live IdP, kept for future validation of the feature:
- entra_e2e.py: real Entra tenant, one interactive sign-in, drives
get_obo_access_token_classified -> _obo_mint_entra (E1-E7)
- keycloak_e2e.py + .sh: ephemeral Keycloak, fully headless, drives the
rfc8693 leg (refresh grant -> token exchange)
- entra_spike.py: raw-OAuth wire probe (pre-implementation reference)
- entra_setup.sh: creates the Entra spike app registrations
- .env.example template; real creds stay in a gitignored .env
Both legs pass E1-E7 (mint + aud, cache hit, single-credential->multi-
audience, rotation write-back, force_refresh, unconsented->credential
survives, flush->re-mint). Not wired into CI.
Refs #551.
Adds the oauth_obo section to docs/mcp-oauth.md:
- when to use it vs oauth_user (mode table row)
- deployment config ([oidc] capture_user_credential + obo_grant_profile,
encryption-key requirement)
- per-IdP setup: Entra (delegated permissions + admin consent, plus the
verified admin-consent-propagation AADSTS65001 gotcha) and Keycloak
RFC 8693 (standard token exchange + audience client scopes)
- revocation & custody model: identity-unlink cuts a user off (credential
+ cache purge); flush-cache is an honest re-mint, not a revoke; per-server
revocation is IdP-governed
- auth-type-transition + troubleshooting table rows for obo
- interim #682 note (Entra pre-authorized-clients removes the second
consent for plain oauth_user, tenant-config only)
Refs #551.
Operators can now select sign-in passthrough (oauth_obo) in the console,
not just via the API:
- new 'Sign-in passthrough' auth-type radio with plain-language copy
('uses your org login - no separate connect')
- the shared OAuth fields block hides the oauth_user-only inputs
(AS URL / registration / client id / secret) for obo and shows just
the audience (marked required) plus scopes (hinted rfc8693-only), with
an explanatory note
- client-side audience-required validation (inline error, not a 400)
- edit-populate + reset handle the new radio
- server list: obo servers get an honest 'flush cache (N)' action
(drops minted tokens -> re-mint) instead of connect/bulk-revoke, with
a confirm dialog that states it does NOT cut off access (that is
IdP-governed / identity-unlink)
Refs #551.
Addresses the high follow-up review of the first fix round:
Revocation lifecycle (the review's dominant theme):
- identity-unlink now purges the user's minted obo cache rows in addition
to revoking the credential, and the response/audit report the actual
effect (credential + N cache rows) instead of a blanket revoked=true;
warmed-session residual (bounded by token TTL) documented
- bulk-revoke on obo is now an honest cache-FLUSH: distinct audit event
(obo_cache_flushed) + response effect=cache_flush_remints, since the
shared credential survives and the next dispatch re-mints (oauth_user
keeps its durable revoke semantics)
- changing oauth_audience on a pool-backed row now purges cached tokens
(audience is the token binding), like URL/name/auth_type changes
- flipping oauth_user->oauth_obo now clears the stale AS-consent scopes
(else rfc8693 sends them -> invalid_scope loop); write path rejects
oauth_scopes under the entra profile (it mints <audience>/.default)
- a cache row bearing a refresh token is never served as an obo token
(guards the cross-node purge-vs-refresh race)
Self-inflicted regression:
- _clear_pending_consent_sync is now gated on an in-memory
_pending_consent_written hint, so the common successful-dispatch path
issues ZERO SQL (was an unconditional per-dispatch DELETE)
Observability + cleanups:
- restore the obo_mint_rejected log carrying the IdP error text (the
shared-helper unification dropped it); event names passed as whole
literals so alerting can grep them
- persist_rotation typed Callable[[str], Awaitable[None]] (was Any)
- _prime_one branches on _obo_server_names (no pre-lookup SQL for
oauth_user)
- removed now-dead any_oauth_user_mcp_servers (3 impls + tests)
+13 regression tests. Full mcp/oidc/console suite 1888 green; mypy clean.
Refs #551.
- credential revocation (440): admin OIDC identity-unlink now deletes the
captured IdP credential too (via delete_oidc_credential, previously
zero callers), so a deprovisioned user stops minting — audited with
obo_credential_revoked
- pending-consent badge gate (3539): new any_user_scoped_mcp_servers
(oauth_user OR oauth_obo) replaces the oauth_user-only gate, so an
obo-only install no longer short-circuits the badge to {pending: 0}
- pending-consent clear (5966): dispatch SUCCESS now clears the pending
row (auth-blind _clear_pending_consent_sync) — the only clear path that
covers obo, whose rows the token sweep (skips obo) and consent callback
(obo never runs) would otherwise never clear
- test:50: strengthened the created-preservation assertion to plant a
distinctly-past created via SQL so a reset is actually detectable
+4 tests (obo/user-scoped gate). NOTE: finding 1992 (orphan cache row on
concurrent delete-during-mint) accepted as bounded residual — the orphan
is a short-lived access-token cache row with NO refresh token, useless
without the deleted credential and self-expiring; a full fix needs FKs or
a delete-spanning lock. Tracked for follow-up.
Refs #551.
The review's most severe finding: nothing warmed oauth_obo pools, so
their tools never entered any per-user catalog and the documented 'mint
on first dispatch' was unreachable (the model can't dispatch a tool it
can't see) — the whole feature was dead in chat.
prime_user_pools now iterates both pool-backed registries. _prime_one
fetches server_row first, then routes oauth_obo through
get_obo_access_token_classified (mints from the captured credential;
missing credential → skipped, the re-login rail handles it) and
oauth_user through its own path unchanged. _rebuild_user_tool_map is
already auth-type-blind, so a warmed obo entry surfaces its tools.
+2 regression tests (obo routed through mint + warmed; skipped cleanly
when the user has no credential).
Refs #551.
- write-time validation (_enforce_oauth_obo_requirements): reject an
oauth_obo row with no oauth_audience (400) or no encryption key (503,
else it SystemExits the cluster at next boot) — at the save choke
point, not per-dispatch (findings 10137/10163)
- update handler no longer nulls oauth_audience/oauth_scopes for
oauth_obo (it needs them); clears only the oauth_user-only columns
(10344)
- auth_type-transition purge now covers every pool-backed transition,
including oauth_user->oauth_obo (was skipped: old per-server-AS refresh
tokens leaked into the mint cache + left a live grant at the old AS
unrevoked) and oauth_obo->static/none (10326)
- URL-change purge + https enforcement + client-secret clear now apply to
oauth_obo, not just oauth_user (10339)
- bulk-revoke accepts oauth_obo — the documented remediation for the
stale rows a flip leaves behind (10705)
+6 console tests (obo audience/key required, happy path, flip-purge, obo
bulk-revoke).
Refs #551.
Addresses the review's B/D/F classes + single-sourcing:
- B (credential corruption): the rfc8693 refresh-leg rotation is now
persisted the instant it is obtained, BEFORE the exchange leg, via a
persist_rotation callback under the held credential lock. A rotated RT
survives an exchange-leg failure (no more cascade lockout), and the
exchange response's own audience-scoped RT is never written to the
shared credential.
- D (wrong-audience bearer): the entra leg ALWAYS pins scope=<audience>/
.default (scope is Entra's only audience carrier); per-server
oauth_scopes no longer replaces it (that dropped the audience and
leaked a Graph-audience token to the MCP server). oauth_scopes stays a
rfc8693-only knob.
- F (state-machine divergence): extracted _handle_refresh_failure, called
by BOTH oauth_user and oauth_obo — oauth_user behaviour byte-identical
(1304 tests green). Fixes: obo cooldown now gated on needs-mint so a
force_refresh 401-retry falls through (2063); credential decrypt errors
classified not raised (2099); permanent-rejection arms the cooldown as
a terminal backstop so it stops re-minting + re-auditing every dispatch
(2156); malformed-200 resets the ambiguous streak (2196); misconfig
arms the cooldown to dampen the log/SQL flood (2089); server_row
threaded from the dispatch caller to drop a hot-path SQL round-trip (2069).
- messaging (5993): obo refresh_failed now points at re-login/admin, not a
nonexistent per-server consent flow.
- single-source (580/9732/217/1830): USER_SCOPED_AUTH_TYPES +
is_user_scoped_auth live in mcp_crypto (leaf), re-exported; OBO_GRANT_
PROFILES derives from _OBO_MINT_LEGS and drives oidc validation (was
dead-exported).
+5 obo regression tests (rotation-survives-exchange-fail, exchange-RT-
ignored, terminal cooldown, cooldown fall-through, decrypt classified).
Refs #551.
Gate sweep of the pool-backed class: oauth_obo joins oauth_user at
every pool-keying site, judged individually -
- _obo_server_names sibling registry (reconcile + boot); priming,
keep-alive sweep, and consent-flow sites deliberately keep iterating
_oauth_user_server_names only (obo has no per-server consent; its
keep-alive lands with the credential lifecycle work)
- pool routing/status/static-health/tool-resolve gates use the shared
is_user_scoped_auth predicate; status reports the real auth_type
- dispatch: _pool_token_lookup routes oauth_obo to the mint engine;
'missing' detail becomes a re-login message (no per-server Connect
URL is advertised - _build_consent_url already returns None)
- _db_servers_to_config skips obo rows from static auto-connect (would
handshake-fail with empty headers and trip the breaker)
- web_search backend refusal covers both per-user auth types
- console: oauth_obo in _MCP_AUTH_TYPES, https enforcement extended;
startup key requirement counts obo rows (encrypted mint cache)
Refs #551.
get_obo_access_token_classified: sibling of the oauth_user classified
lookup sharing its result vocabulary, cache table, locks, and backoff,
but 'refresh' = mint from the user's captured credential via the
deployment grant leg ([oidc] obo_grant_profile):
- entra: one refresh-token redemption, scope=<audience>/.default
- rfc8693: refresh grant -> standard token exchange (audience=)
Both wire shapes are spike-verified (docs/design/obo-spike). Key
semantics: a missing cache row mints (no consent prerequisite); a
PERMANENT rejection drops only the per-server cache row - the shared
credential is never auto-deleted, so one mis-granted server cannot
lock a user out of the rest; rotation write-back persists the newest
credential BEFORE the cache write; mints single-flight cluster-wide on
a per-(user, issuer) advisory lock.
is_user_scoped_auth/USER_SCOPED_AUTH_TYPES define the pool-keyed auth
class once for the upcoming client-side gate sweep.
Refs #551.
[oidc] capture_user_credential (default off; env
TURNSTONE_OIDC_CAPTURE_USER_CREDENTIAL) persists the user's IdP refresh
token - encrypted with the MCP token envelope - as the single
credential oauth_obo servers will redeem on demand.
- enabling the knob appends offline_access to the login scopes
(idempotent when the operator already lists it)
- capture runs after user provisioning and is best-effort: a capture
failure logs loudly but never blocks login; the mint path surfaces a
missing credential on the reconnect rail
- startup hard-fails (SystemExit) when capture is enabled without a
[security] token encryption key, same as the oauth_user enforcement
Refs #551.
One captured IdP refresh token per (user, issuer), Fernet-encrypted with
the same envelope as mcp_user_tokens - the credential that
auth_type='oauth_obo' servers will redeem on demand for per-server
access tokens instead of holding per-(user, server) refresh tokens.
- migration 067 + mirrored create_all schema (parity-tested)
- storage protocol + both backends: upsert (replace-on-conflict),
get, rotation write-back, delete, delete_user cascade
- MCPTokenStore encrypt/decrypt wrappers
Refs #551.
Deciding the installer up front — get.docker.com for the IDs it recognizes,
Docker's repo directly for unrecognized derivatives — avoids treating a
transient get.docker.com failure (network, apt lock, EOL sleep) on a supported
distro as an "unsupported distro" and silently routing it into the repo path.
Recognized IDs now surface the real failure via die instead of masking it;
unrecognized derivatives (Nobara, Mint, …) skip the doomed call and its
"Unsupported distribution" output entirely rather than running it to fail.
Addresses review feedback on #829.
run.sh delegates Docker installation to get.docker.com, which detects the
distro from $ID alone and aborts with "Unsupported distribution '<id>'" on
any derivative it doesn't hardcode — Nobara (the reported case), Linux Mint,
Pop!_OS, AlmaLinux, Oracle Linux, and so on. run.sh's own detection already
resolves these via ID_LIKE/fallback, so the family is known; only the
delegated install fails.
When get.docker.com exits non-zero, fall back to adding Docker's official CE
repo for the upstream the family maps to and installing the same packages
(including the compose plugin the rest of run.sh depends on). Upstream is
chosen from PLATFORM_ID for the dnf family — Fedora is platform:fNN, Enterprise
Linux platform:elN, which ID_LIKE cannot distinguish (Nobara's is
"rhel centos fedora" yet it is pure Fedora) — and from UBUNTU_CODENAME for the
apt family, which is present only on Ubuntu lineage and is the exact codename
Docker's repo expects (Mint's VERSION_CODENAME is not).
Fixes#822.
The installer's "Finish setup" told users to run `turnstone-admin create-user`,
which creates a user with no role. Web login derives scopes solely from assigned
roles (empty perms -> read only), so that account logs in read-only and every
admin action fails with "Forbidden: token lacks 'approve' scope". Creating any
user also flips setup_required to false, so the browser first-run wizard -- the
only path that assigns the builtin-admin role -- never appears.
- run.sh: point "Finish setup" at the web setup wizard; use create-admin as the
headless fallback instead of create-user
- admin.py: add `create-admin` -- creates a user + assigns builtin-admin, or
promotes an existing role-less user (idempotent); guards on the seeded admin
role and enforces the wizard's 8-char password floor for fresh accounts
- tests: cover fresh-grant (approve reaches the derived login scope), the
promote/recovery path, idempotency, and both validation exits
Fixes#824
The intent judge and output-guard judge were the only create_completion
callers that never passed model-definition capabilities, so operator-declared
capabilities (effort passthrough, tool support, temperature, verbosity) were
silently ignored on judge calls. Every in-ChatSession lane threads them via
_resolve_capabilities; the judges live outside the session and never reached
it.
Add a shared _resolve_model_capabilities() helper mirroring
ChatSession._resolve_capabilities, and have both judges resolve
self._capabilities — from the judge alias's model definition, or the injected
session capabilities on the session-model fallback — and pass capabilities=
into create_completion. Replace each judge's context_window int arg with
session_capabilities: the fallback window now derives from the resolved caps
(identical to what the session passed before), while the alias path keeps
reading ModelConfig.context_window, a separate field the capability merge must
not touch.
Refresh the stale docs/judge.md note claiming sub-agents are exempt from intent
validation — task agents have been judge-gated since #773.
Refs #823
If sub-turns ever persist: Turn-IR verbatim, re-mint at load (run_seq is
session-scoped), rebuild the wire map from the native lane's structural
1:1 pairing with the mirror; turns without native client tool blocks
need no entries. The map itself is never persisted — it is derivable,
and a second durable source of truth would have to be kept in lockstep
with the turns. Also documents why the mint must never be string-split
(not injective: parent and original may contain the delimiter).
The fidelity swap now requires the raw lane to be a faithful counterpart
of the mirror — same length, every id present — before replacing
tool_calls; a partially-corrupted lane (filtered non-dict elements)
would otherwise swap a shorter list over the mirror and orphan a
mirrored call whose tool result remains in history. The _run_agent
call-site comment now matches the builder's reasoning_text-only
blank-id rule.
The blank-id gate's strip-then-filter semantics left two residual
hazards (surviving Responses reasoning items whose pairing contract
needs their original sibling items; an asymmetric Messages-shaped lane
surviving when no client block was actually stripped). The rule is now
total and simpler: on a blank-id turn only the loose-text
reasoning_text synth block survives — it carries no id and is
shape-invalid on the Messages translator by design, and real-world
blank-id servers are Chat-Completions locals whose reasoning IS that
loose text. This also removes the builder's per-call provider import.
The Google fidelity swap now skips raw rows carrying a blank id
(historical captures that predate the gate would otherwise resurrect
the blank id on every replay — the sanitized mirror stays), guards
against non-dict lane elements, and legalizes via the new shared
lowering.legalize_tool_call_entry — the ONE per-entry legalizer the
sanitize pass also uses, so the two seats cannot drift on semantics or
the wire.tool_args_legalized breadcrumb.
The blank-provider-id gate lived only at the _run_agent call site while
the main-loop stream accumulator has the identical back-fill-then-carry
seam — and it over-dropped, discarding the reasoning lane for exactly
the servers that emit blank ids. The gate now lives in
_finalize_provider_blocks as a had_blank_ids parameter both harnesses
thread: client tool blocks (which keep the blank id the mirror back-fill
never reached) are stripped, and when any were present the remaining
Messages-shaped blocks go with them (a surviving native lane REPLACES
the rebuilt content on the Anthropic translator, so a lane missing its
tool_use would orphan every mirrored call) — while shape-invalid
reasoning residuals (reasoning_text, Responses reasoning items) are
kept. This also closes the pre-existing main-loop case: a Gemini
openai-compat turn with a blank tool id no longer persists a raw
fidelity dict whose blank id the swap would resurrect on every replay.
The Google fidelity-swap legalization now reuses the canonical
lowering.legalized_arguments (made public) instead of a hand-rolled
narrower copy: dict-shaped arguments are serialized rather than
collapsed to {}, the standard wire.tool_args_legalized breadcrumb is
logged, and a degenerate non-dict function entry passes through
untouched instead of raising.
- Skip the native lane on a turn whose provider left a tool-call id
blank: the uuid back-fill reaches only the tool_calls mirror, so a
carried native tool_use block would replay the blank id and desync
from the restored tool_result (Anthropic orphans the result; Google
re-fills a fresh uuid). The rebuild path keeps every representation
on the back-filled id — the pre-native behaviour, for exactly the
degenerate case.
- Extract _reasoning_text as the ONE Chat-Completions reasoning
extractor shared by the streaming and non-streaming paths: first
non-empty STRING of reasoning/reasoning_content wins, so a server
putting a structured object in reasoning can neither shadow valid
text in reasoning_content nor leak a non-str into the session's
reasoning accumulator.
- Legalize arguments when GoogleProvider's fidelity swap replaces the
sanitized tool_calls mirror with the raw provider dicts — the swap
could resurrect a malformed arguments string the upstream sanitize
pass had fixed (pre-existing on the main loop; ids and
thought_signature untouched).
- Drop the redundant emptiness guard on the agent seam's
reasoning_parts (the shared finalize helper already guards) and
document the wire_id_map lifetime invariant for future
resumable/background agents.
A task agent's replayed turns now carry the native reasoning lane the
model produced (Anthropic thinking blocks + signatures, OpenAI Responses
reasoning items, Gemini thought_signature blocks, vLLM/llama.cpp parsed
reasoning text) instead of being rebuilt from content + tool_calls with
the reasoning dropped — restoring reasoning continuity across the
agent's own multi-turn tool loop on every provider lane.
The prerequisite is the id half: replace legalize_tool_call_ids with
restore_provider_tool_ids, a lowering pass that maps the session-minted
sub-tool ids back to the provider's own ids on the transient wire copy
(from the per-run mint map, never by string-splitting). The native
tool_use block is replayed verbatim — its id and signature untouched —
and the top-level mirror and tool_result agree with it on every request.
The minted id stays the sole internal key (registry, DOM, recall,
cancel ledger), #820 unchanged.
Chat-Completions lane: non-streaming create_completion now surfaces
reasoning/reasoning_content as CompletionResult.reasoning (the twin of
the streaming reasoning_delta extraction), and the agent seam runs the
Phase 5 vLLM reasoning-field replay against the agent's own provider
and alias. The native lane is finalized by a shared helper
(_finalize_provider_blocks) so the main loop and the sub-harness cannot
drift; replay honors the per-model replay_reasoning_to_model flag on
every lane, and llama.cpp stays capture-only, matching the main loop.
- wire_safe_tool_call_id: SHA-256 not SHA-1 for the deterministic token —
matches the codebase convention for fingerprints (attachments, auth,
session) and drops the SHA-1 scanner flag. Non-crypto use, ids unchanged
in shape (tid_ + 32 hex); no test pins the literal value.
- interactive.js: the two sub-agent child-id example comments now show the
real minted shape (<parent>::r{run}s{step}::<id>), not a stale <seq> form.
Sub-agent tool ids were namespaced {parent}::{provider_id} — unique
across concurrent agents but not across turns within one agent. A local
provider reissuing "call_0" every response minted the same id twice, so
the live card's DOM row lookup collapsed distinct calls onto one row
while FIFO recall kept them apart: two views of one trajectory disagreed
on identical input (the bug-3 id-consistency defect). When the provider
also reuses the PARENT call id, sequential runs repeated the collision
one level up.
Mint {parent}::r{run}s{step}::{provider_id} at the single rewrite point:
a session-monotonic run tag (lock-allocated; runs start concurrently on
the 4-wide task pool) plus a per-run step tag make each id unique within
the session, and every consumer — nesting registry, error flags, DOM
data-call-id, recall projection, cancel ledger — keys on that one id.
The FIFO pairing helper stays as honest pairing for un-minted input
(unparented runs, direct construction), with its rationale rewritten.
The agent wire seam (_run_agent's _api_call) also runs the same two
validity passes the main loop already ran — sanitize_tool_call_arguments
(a documented vLLM deepseek_v4 renders malformed args and 400s; agents
hit the same backends) and legalize_tool_call_ids (projects the long,
::-containing ids to plain tokens, call/result pairing preserved). The
id projection is DEFENSIVE hardening, not a fix for an observed break:
the ids replay fine on the lenient anthropic-compatible deployment (the
prior ::-containing format ran reliably), it just keeps an agent's
self-built history valid on a hypothetically stricter backend. Applied
at the agent seam only — main-loop assistant turns carry a provider-
native block lane whose id must stay byte-identical to the mirrored
tool_calls, so the projection cannot run there without desyncing them.
Follow-ups: parent-level card aliasing under a reused parent id; the same
id hygiene for the main conversation loop / native lane.
A dirty flag set by touching a verbosity/reasoning-mode select survives a
model/provider/surface change, so the merge-side delete could destroy a key
hand-typed into the Advanced JSON for the renamed row. Honor the dirty
override only while the identity still matches the row that made it dirty.
Also document the captured-value fallback contract at both sites (the
baseline is deliberately not consulted: it arrives async or never on the
compat lane, capture has already lifted the value out of the row JSON, and
emission is gated server-side on the merged supports_* flag) and pin the
fallback plus the scoped dirty-delete in test_app_js.
- capability-gated "Response controls" on the Models create/edit
shelf: Output verbosity (low/medium/high) and Reasoning mode
(Standard/Pro), shown only for Responses-surface models; the empty
selection means provider default and omits the capability key
- values lift out of the capabilities JSON into the selects on edit
and merge back on save with identity tracking, so changing the
provider/model/surface resets them instead of carrying a value
across models; the Advanced JSON textarea wins unless the select
was touched last
- known GPT-5.6 models inherit support from the static table without
persisting redundant support flags; OpenAI-compatible models pinned
to the Responses surface opt in via the supports_verbosity /
supports_pro_mode tiles
- invalidate in-flight capability lookups on any identity field
change and on modal open so a stale response cannot clobber a fresh
shelf; API-surface changes now run the full field-change path
- model list rows surface verbosity= / mode= override chips
- every 5.6 tier accepts effort "max" and reasoning.mode
"standard"/"pro" (GA docs: pro is a request mode on any GPT-5.6
model) -- drop the Sol-only gating
- GPT-5.6 deprecates prompt_cache_retention; send
prompt_cache_options={"ttl": "30m"} (its only supported lifetime)
and keep the 24h retention policy for pre-5.6 models
- never inject commercial cache params into local lanes: dropped from
the Chat Completions lane (which serves only openai-compatible and
google) and gated off the compat-pinned Responses lane -- a gpt-5*
served-model name is not an OpenAI account
- account cache writes: usage *_tokens_details.cache_write_tokens
flows into cache_creation_tokens (5.6 bills writes at 1.25x the
uncached input rate)
- drop non-string verbosity/reasoning_mode overrides with a warning
instead of raising on unhashable capability-JSON values
- keep ModelCapabilities' public positional prefix stable by appending
the verbosity/pro fields at the tail; pin it with a constructor test
- openai floor 2.44 -> 2.45, the first release with the typed
prompt_cache_options kwarg
renovate[bot]