1610 Commits

This Branch

This Branch

All Branches

Author	SHA1	Message	Date
Patrick Buckley	91c46051d9	feat(providers): drop o-series and pre-5.4 GPT-5 capability rows ... The OpenAI commercial capability table floor is now gpt-5.4: o1, o1-mini, o3, o3-mini, o3-pro, o4-mini, gpt-5, gpt-5-mini, gpt-5-nano, gpt-5-pro, gpt-5.1, gpt-5.1-codex-max, gpt-5.2, gpt-5.2-pro, and gpt-5.3 are effectively unused in the field. The gpt-5-search-api row (different product surface) and the audio/STT/TTS rows stay. A legacy id now resolves to OPENAI_DEFAULT (temperature sent, no declared effort vocabulary, 200K window) — which those models may reject; the remediation is the model definition's capabilities JSON or a current model, release-noted under Unreleased → Removed. This also retires the transport-collapse review's thrice-reported "stream-rejecting o1-era models are stranded" finding by removing its subject: no row in the table describes a non-streaming model anymore. Tests migrate to 5.4-era equivalents that pin the same behaviors: always-reasoning temperature suppression and off-list effort snap (gpt-5.4-pro for gpt-5-pro/o3), explicit-none forwarding (gpt-5.4 for gpt-5.1), empty-effort-vocabulary knob drop (gpt-5-search-api for o1-mini), and the longest-prefix shadow hazard (gpt-5.4-pro vs gpt-5.4 for codex-max vs gpt-5.1).	2026-07-13 22:39:19 -07:00
Patrick Buckley	3a28dc2f16	fix(providers): review round 5 — same-id fragment merge, post-finish blip tolerance, chat finish shim ... Correctness: - ToolCallSlotter's reannounce split is gated to ID-LESS deltas: id equality proves the same call, so compat servers that repeat the id+name header on every argument fragment merge back into one call with valid JSON (round 4's ungated heuristic split them into duplicate half-JSON calls — execution-confirmed by the review). The residual id-less repeat-name-per-fragment shape is documented as inherently ambiguous; ids are the only disambiguator. - drain_stream keeps a completed result when the transport blips AFTER the finish reason (trailing usage chunk / citation footer window): the generation is in hand, so forfeit the trailing metadata instead of discarding a fully-delivered verdict or re-paying a compaction. - The chat iterator shims finish_reason="stop" when a stream ends CLEANLY after delivering content or tool calls — the deleted non-streaming `or "stop"` default for lax finish-reason-less servers, now safe to restore because abrupt deaths surface as httpx.TransportError (round 4) rather than clean exhaustion. This supersedes the round-3 keep-the-gate ruling: the httpx catch changed the calculus, and the Anthropic/Responses lanes already got their marker-based shims. Empty/reasoning-only streams still fail the complete-or-error gate. Two streaming tests gained the shim chunk. Dispositions held: o1-era stream-rejecting models (third re-report) stay a release-note remediation per the earlier ruling. Cleanup: the two Responses terminal branches collapse into one path (status derived from the event type when the payload is missing — also fixes the end-of-stream debug log reporting finish_reason=None for completed lax streams); the annotations walk is one shared helper (the two copies had already diverged on None-content guarding); _raise_responses_failure is annotated NoReturn; scripts/livepass.py drops the phantom supports_streaming key; test_model_registry's capture helpers ride scripted_chat_client; _openai_stream_chunk points at its fake_chat_stream shape-twin for future consolidation.	2026-07-13 22:39:19 -07:00
Patrick Buckley	cf7cfe8932	fix(providers): review round 4 — wire-error retryability, tap/mirror slot parity, terminal completeness ... Correctness: - drain_stream chains raw httpx.TransportError from stream iteration into retryable IncompleteStreamError (original type+message preserved via __cause__): streaming moved the body read out of the SDK's APIConnectionError-wrapped request, so mid-body connection drops and read timeouts — retried transparently on 1.7 — were escaping every single-shot retry loop as instantly-fatal raw httpx names. - The index remap is extracted as ToolCallSlotter and GoogleProvider's raw tap slots THROUGH IT over the same delta sequence as the base iterator: round 3's mirror-side de-fusion had left the tap keying by wire index, so a degenerate stream produced 2 mirror calls vs 1 fused raw dict — _prepare_messages' length gate then silently dropped the thought_signature lane (400 on signature-strict Gemini models). - The slotter also splits ID-LESS degenerate parallel calls: a delta announcing a name for a slot that already accumulated arguments is a second whole call, not a fragment (fragmented single calls pinned unaffected). - A payload-less Responses terminal event keeps the provider_blocks already collected from output_item.done events (they came from the stream, not the missing payload); only usage is genuinely lost. - The truncation-rebuild path walks the terminal output's message annotations, so truncated web-search turns keep their Sources footer (the in-flight item never received output_item.done). Cleanup: one _raise_responses_failure ladder serves both in-band failure shapes (error events + response.failed); IncompleteStreamError joins the public providers export (docstrings tell callers to catch it); the de-fusion tests ride the file's existing _openai_stream_chunk helpers instead of a third hand-rolled SSE fake; the dead if-response guard in the terminal branch is gone. Deferred with note: classifying IncompleteStreamError once at the retry-predicate consultation site instead of per-provider strings is #832 territory (the predicate lives in ChatSession); the six-lane parametrized test guards the listing until then.	2026-07-13 22:39:19 -07:00
Patrick Buckley	56b7674dfa	fix(providers): review round 3 — in-band error events, terminal-marker tolerance, adapter-owned de-fusion ... Correctness: - Responses _iter_stream handles the SDK's in-band `error` SSE event (ResponseErrorEvent is YIELDED, not raised, and no response.failed need follow): the real API code/message now surfaces — code-gated for retryability like response.failed — instead of the stream exhausting finish-less and hiding the cause behind a retried IncompleteStreamError. - Anthropic message_stop supplies a missing stop_reason: it is a genuine terminal marker, so a compat /v1/messages shim whose message_delta omits stop_reason completes (blocks intact) rather than failing a generation that arrived — tolerance the retired non-streaming default provided, restored without weakening the died-mid-response gate. - A Responses terminal event without its response payload still emits the finish reason its type implies (lax compat servers), losing only usage/blocks rather than the whole result. Dispositions held (documented, not re-coded): the complete-or-error gate stays for finish-less Chat Completions streams — indistinguishable in-band from a died generation, and silent partial-storage is the worse failure; CHANGELOG now names the shape and each provider's accepted terminal markers. supports_streaming deletion and the stream_options wire delta were ruled earlier and keep their release-note remediations. Cleanup: index-degenerate de-fusion MOVED from drain_stream into the chat adapter's iterator (mirroring the Anthropic iterator's index assignment) so the interactive loop is fixed too and the drain returns to a plain mirror of the main-loop accumulator; a parametrized test locks "IncompleteStreamError is retryable" across all six provider lanes instead of trusting per-adapter memory; scripted_anthropic_client joins scripted_chat_client (shared _ScriptedClient class, no function attrs) and the two remaining hand-rolled anthropic closures convert.	2026-07-13 22:39:19 -07:00
Patrick Buckley	3ffa8b9057	fix(providers): review round 2 — complete-or-error drain, code-gated retries, truncation-safe blocks ... Correctness (3 confirmed + 2 plausible, all fixed): - drain_stream now raises typed, retryable IncompleteStreamError when a stream exhausts without any finish reason — every adapter emits one on a healthy stream, so its absence means the generation died mid-response behind a cleanly-closing proxy. This restores the retired transport's complete-or-error contract (a half-generated compaction summary was previously returned as finish=stop and stored, silently replacing real history) and DELETES round 1's suffix-info fold: with no finish-less success path there is nothing to classify, so a trailing status ping can never be stored as content either. - Index-degenerate parallel tool calls get distinct slots: a delta whose id differs from its slot's opens a new call (id-less fragments still follow their index's current call), so historical compat servers that emit every parallel call at index 0 no longer fuse distinct calls into concatenated garbage arguments. Result order stays index-sorted (stable) like the retired array parse. - response.failed retryability is code-gated: only transient codes (server_error, rate_limit_exceeded) raise the retryable typed error; deterministic rejections (invalid prompt, image fetch, policy) raise plain RuntimeError and stop retry loops on attempt zero instead of running the full backoff ladder against a doomed request. - Terminal Responses events rebuild provider_blocks from response.output when present: the item being generated at max_output_tokens truncation never receives output_item.done, and storing a reasoning item without its required following item made the next turn's replay a 400. - merge_usage's base case uses dataclasses.replace so a future UsageInfo field can't be silently zeroed on drained lanes. Cleanup: run_abortable_with_deadline bundles the three-point abort wiring (ref + cancel_ref + on_abandon) so it cannot be half-wired — both judges converted; scripted_chat_client hoists the 14 chat-lane fake_create closures (call scripts + .calls recording replace per-test counter cells); fake_chat_stream gains reasoning=, collapsing the reasoning-capture suite's hand-rolled chunk shape; FakeAnthropicBlock hoists the duplicated _Block test class; the class and judge PlantUML diagrams drop the retired create_completion flow. Also converts test_model_registry's agent-model fakes, which returned legacy response objects that iterated as EMPTY streams — they only passed through the old drain's silent finish=stop default, exactly the hazard the new gate exists to catch.	2026-07-13 22:39:19 -07:00
Patrick Buckley	08580f25f9	fix(providers): review round 1 — streaming parity gaps the collapse exposed ... Correctness (4 confirmed + 1 plausible fixed, 2 accepted+documented): - Anthropic _iter_anthropic_stream handles citations_delta: text-block citations now ride the raw block into provider_blocks, as replay requires (the retired non-streaming lane preserved them via model_dump; the streaming lane dropped them — a pre-existing main-loop gap the collapse would have extended to single-shot lanes). - Anthropic text blocks separate with "\n" at each subsequent block start, restoring the retired lane's "\n".join rendering on drained lanes AND un-fusing streamed web-search responses in the chat loop. - response.failed raises typed ResponsesStreamFailedError, listed in the provider's retryable_error_names — retry loops treat an in-band failure like the wire errors it stands in for instead of hard-stopping on a bare RuntimeError (judges keep their heuristic fallback after retries). - drain_stream folds a finish-less stream's terminal citations footer (suffix rule: pre-finish info invalidated by any later payload), so lax compat servers that never send finish_reason keep their Sources. - usage max-merge extracted as merge_usage() in _protocol.py — the one definition drain uses now and the session's inline consumer adopts on #832. Accepted + release-noted instead of coded around: strict pre-2024 compat servers that 400 on stream_options (such a server already cannot serve the chat loop; CHANGELOG caveat extended), and repeated-index parallel tool-call merging on legacy compat servers (identical to the main loop's accumulator semantics; a shared guard belongs in the #832 unification). Cleanup: run_with_deadline grows on_abandon (best-effort, cannot mask the deadline error) and both judges drop the copy-pasted abort choreography; StreamAbortRef documents the _CancelRef adoption plan; test_model_turn's fake replays through the shared as_stream adapter; docs/architecture.md drops the retired Protocol row. Tests: refusal handler pinned (was advertised, untested); typed-failed retryability; citations capture; text-block separator (plus the mixed text+search expectation updated for the separator chunk); finish-less citation fold; on_abandon firing matrix; StreamAbortRef arrival race.	2026-07-13 22:39:19 -07:00
Patrick Buckley	1e7ad7bcb6	feat(providers): one transport — drain create_streaming, retire create_completion (#831) ... Every single-shot lane (model_turn: judges, titles, compaction, web-fetch extraction, perception, eval, optimizer) now samples through the provider's streaming entry and accumulates via a shared drain_stream(), deleting create_completion from the Protocol and all three adapters (xai/google inherit). Request shaping can no longer drift between the two consumption styles, and callers keep the exact CompletionResult contract. The drain mirrors the main loop's proven chunk semantics: per-field max-merge for usage (Anthropic splits prompt/completion across message_start/message_delta), tool-call assembly by delta index, provider_blocks from the terminal emission, trailing citation info folded back into content (byte-matching the old format_citations append), mid-stream status pings dropped. Also in this change: - model_turn grows cancel_ref; both judges wire their run_with_deadline abandon paths to a new StreamAbortRef (deadline.py) that closes the SDK stream — a timed-out judge call now aborts its HTTP read instead of pinning a daemon thread until the next upstream chunk. The append hook covers the arrival race, mirroring ChatSession._CancelRef. - Responses streaming gains the response.incomplete terminal handler (truncated runs were mislabeled finish=stop and lost final usage AND collected provider_blocks) and a refusal handler ([Refused: …] content, matching the retired non-streaming rendering). Both also fix the main chat loop, which shared the gaps. - supports_streaming capability flag deleted (zero readers) along with its admin capability tile; o1-era models that reject streaming need a model alias pointing at a current model (release-noted). - Helpers that existed only for the deleted transport go with it: Responses._parse_response, chat/google._extract_tool_calls. Known behavioral deltas (release-noted): OpenAI-compatible servers that ignore stream_options.include_usage stop producing usage rows on these lanes; multiple Anthropic text blocks concatenate without the old "\n" joint (matching the main loop); model_turn lanes no longer risk client read-timeouts on long generations — the reason the Anthropic adapter already drained a stream internally. Tests: new test_drain_stream.py pins the accumulator rules; shared fakes (as_stream, fake_chat_stream, fake_anthropic_stream) migrate 11 suites to the streaming transport, with the task-agent and adapter suites now exercising the real _iter_stream + drain path end to end.	2026-07-13 22:39:19 -07:00
Patrick Buckley	a66e9d456d	fix(mcp): gate the marker release on non-acquisition; structure the paired protocols ... Close the round-8 review findings: - The refresh runner's finally-discard releases the coalesce marker ONLY when the lock was never acquired (cancelled while parked). After the at-acquire discard, a marker present at exit belongs to the successor spawned during the in-flight list call — discarding it unconditionally let the handler mint one extra runner per debounce window while the lock was congested, reopening the unbounded runner FIFO the marker exists to bound. - The observe-before-lookup preamble lives once in _pool_lookup_checked (snapshot taken synchronously before the lookup await, render paired with the convergence drop) instead of verbatim in all three dispatchers — the ordering contract is now structural rather than comment discipline. - drop_session is paired with its debounce-stamp pop in _drop_session_and_stamp, shared by the eviction, teardown, and owner-death paths; the shutdown sweep clears the pool notification stamp dict and the coalesce marker set alongside the other pool state. - _mcp_tools_change_seq is initialized unconditionally for every session kind, so the attribute's existence no longer encodes whether an MCP client was wired at construction.	2026-07-13 21:13:42 -07:00
Patrick Buckley	e9ecf91c07	fix(mcp): observe before the lookup; coalesce queued refreshes; pop stamps on every teardown ... Close the round-7 review findings: - The dead-grant observation is now snapshotted BEFORE the classified lookup's first await, by the callers (the three dispatchers via _pool_lookup_failure, _prime_one, and the obo credential gate), and _schedule_dead_grant_drop requires it as a parameter: snapshotting after the lookup returned could capture a session the consent-completion prime connected mid-lookup — its awaits can park on executor hops — and the drop then evicted the just-restored catalog it exists to spare, with no remaining re-prime path. - Spawned list_changed refreshes coalesce on a per-(key, kind) marker: set at spawn, cleared the moment the runner acquires open_lock (before its list call, so a change the in-flight list missed spawns exactly one successor). Admission was one per 5s debounce window while each runner can hold the lock up to the 30s refresh timeout, so a notifying-but-slow server accreted lock waiters without bound — FIFO dispatch waits past the 120s budget, idle eviction starved by the contested lock, and background tasks growing for as long as the server kept notifying. The runner also returns quietly for an evicted session instead of failing through the log. The residual duty-cycle case (a wedged-but-notifying server defers idle eviction of its own entry until the first dispatch, recovery, or silence) is documented at the runner. - Every teardown path now pops the notification debounce stamp: _teardown_pool_entry and _on_pool_owner_death left it in place, so the keep-stamp design's documented reconnect backstop did not exist on the idle-collapse and connect-failure paths — a change announced in a failed window could be debounced against a pre-collapse stamp after reconnect and never land. The idle-close path's own pop is now owned by _teardown_pool_entry. - Cleanups: the notification table maps type to kind label only, with the kind-to-refresher map bound at dispatch time (mypy-checked attribute references, instance overrides keep working) instead of getattr on a name string; _schedule_dead_grant_drop skips when there is provably nothing to converge (no entry, or a session-less catalog-less stub), sparing a tracked no-op task per unconsented server per prime at scale; the fire-and-forget prime idiom's three hand-synced copies collapse into try_prime_user_pools (session construction, acting-user change, OIDC capture); the stale lock-contract docstrings on the resources/prompts refreshers now state the held-lock requirement; has_live_session_listener is the sole listener-liveness predicate (the private alias is gone); the construction-scoped tools-seq read is a constructor local instead of a persistent ChatSession attribute.	2026-07-13 21:13:42 -07:00
Patrick Buckley	7186e1e709	fix(mcp): observe sessions at dead-grant discovery; serialize spawned refreshes ... Close the round-6 review findings, all in the round-5 surface: - Dead-grant drops snapshot the entry's session when the failed lookup is observed and skip only when the session CHANGED since: a warm transport that predates the revocation is evicted with the catalog (failed lookups short-circuit dispatch before any 401 could evict it, so nothing else converges a warm entry until the idle TTL), while a session a re-consent prime created after the observation still parks the drop. The obo credential gate inherits the same semantics for warm obo entries. - The spawned notification refresh serializes on open_lock with a same-entry recheck: unserialized it raced the connect wiring block (older discovery snapshot republished over the refresh's newer catalog, permanently hiding the change behind the consumed debounce stamp) and sibling same-key refreshes (the slower list call publishing the older catalog last). - The refresh failure path keeps the debounce stamp instead of popping it: pop-on-failure re-armed the handler on every notification, so a fast-failing server spawned refresh tasks unthrottled at its notification rate. Changes announced in a failed window converge on the next list_changed or reconnect (teardown pops the stamp). - The refresh runner catches BaseExceptionGroup alongside Exception: a wedged anyio transport surfaces session-op failures as groups, which escaped to the background-task failure log whose exc_info serializes the chained httpx request carrying the user's bearer. - Cleanups: the three list_changed handler branches collapse into one table-driven path; _reprime_active_users reuses _live_listener_uids; the obo gate's synthesized kind="missing" verdict is contract-pinned to get_obo_access_token_classified's missing-credential return.	2026-07-13 21:13:42 -07:00
Patrick Buckley	2ce6638761	fix(mcp): spawn list_changed refreshes off the receive loop; close round-5 findings ... The headline finding is pre-existing and structural, surfaced by this branch's timeout: the SDK awaits notification handlers INLINE in its receive loop, so a handler that awaits a request on the same session can never receive its response — push-driven catalog refreshes have never completed against a healthy server, and with the new timeout they also stalled every in-flight call on the session for its duration. Refreshes are now spawned as tracked background tasks, and a FAILED refresh returns the debounce stamp so the server's next list_changed retries instead of being dropped inside the window. Also from the round: - The obo credential-presence gate skipped exactly the per-server lookup whose kind='missing' would have dropped retained catalogs, so unlinked users' ghosts survived every new-session prime. The gate now schedules the same dead-grant drop for catalog-bearing obo entries before skipping the servers. - Dead-grant drops re-validate under open_lock via skip_if_connected: a drop parked behind a re-consent prime's connect must not clear the freshly restored catalog (a live session proves a connect succeeded after the failed lookup that scheduled the drop). The explicit revocation path still clears warm entries unconditionally. - The constructor's convergence re-check moved to the end of tool setup, where every _on_mcp_tools_changed dependency exists — the while-loop re-read could still be clobbered by the tool-search construction reading mixed state, and a mid-construction callback crash (pre-existing, swallowed by the fan-out) loses its update. - _drop_catalog_locked's docstring told the truth about its wait bound (a same-key dispatch holds open_lock across its entire SDK call). - The OIDC capture-site liveness gate call moved inside its try — nothing on that best-effort path may fail a login. - One _pool_lookup_failure helper pairs render+drop for all three dispatchers; fake pool-tool seeds deduped to one module helper; the sleep-based test syncs replaced with a deterministic _background_tasks drain.	2026-07-13 21:13:42 -07:00
Patrick Buckley	1c4761971c	fix(mcp): strip the revocation-generation protocol; keep the stable core ... Round 4 confirmed six correctness bugs, all inside round 3's catalog_gen machinery (a ChatSession.__init__ crash from the mirror- race re-run, an orphaned-lock race created by the ensure-before-lookup reorder, no generation memory across entry re-creation, gen reset on re-ensure, a raw internal error surfacing to the session layer). Four rounds of evidence: hardening this event-driven subsystem with new concurrency machinery breeds interaction bugs about as fast as it closes cosmetic races. Decision: remove the protocol, keep the core. Stripped: PoolEntryState.catalog_gen, the expected_gen threading through dispatch/prime/connect, the dispatcher ensure-before-lookup reorders, _PoolGrantRevokedError, and the refresh gen-guards (the entry-identity check stays — it protects against entry replacement with no protocol). The publisher-suspended-across-a-drop races those closed are now ACCEPTED RESIDUALS, documented at _evict_session_drop_catalog: the ghost self-heals at next use via the dead-grant drop (dispatch AND priming), and a reconnected stale bearer dies at access-token expiry — the same bound every warm session already rides at revocation time. Kept from round 3 (stable, orthogonal): staged discovery publication, prime-side dead-grant drops, the obo re-login prime, the single _pool_lookup_verdict classification, drop_session() pairing, and the tracked revocation drop task. Fixed from round 4's orthogonal findings: - ChatSession construction converges its tool lists with a bounded re-read loop instead of calling _on_mcp_tools_changed, which dereferences tool-search state initialized later in construction. - _refresh_pool_server_tools gets the asyncio.timeout its resource and prompt siblings already had — a wedged server no longer hangs the notification-handler task. - The OIDC capture-site prime is gated on the user having a live session listener (new public has_live_session_listener): routine SSO re-logins with nothing open no longer fan out mints and connects. - _pool_lookup_verdict returns a Literal so a typo'd verdict comparison fails mypy instead of silently never matching. - The triplicated double-401 comment blocks shrink to two-liners pointing at the single rationale in _evict_session's docstring.	2026-07-13 21:13:42 -07:00
Patrick Buckley	1e84f62619	fix(mcp): round-3 review fixes — revocation generation for catalog publishers ... Round 3 identified the class behind the remaining bugs: catalog PUBLISHERS never re-validate revocation state, so anything that read a token or suspended before a drop could republish (resurrect) a revoked catalog that retention then keeps forever. One primitive closes the class: - PoolEntryState.catalog_gen, bumped by _evict_session_drop_catalog. The three list_changed refreshes snapshot it before their awaits and discard results if it moved; dispatch and priming snapshot it before their token reads, and _connect_one_pool refuses to connect (raising _PoolGrantRevokedError, a non-breaker failure) when the generation moved past the caller's snapshot — the bearer in hand predates a disconnect. - _connect_one_pool stages all three discovery results locally and publishes them together in the final wiring block: a mid-discovery failure now leaves the retained catalog exactly as it was instead of a torn half-update diverging from the per-user maps. - Priming converges dead grants too: _prime_one schedules the same catalog drop the dispatchers use, so a NEW session's prime clears ghosts left by a disconnect made on another node. - obo re-login is the obo restore moment: a successful credential capture at the OIDC callback now schedules prime_user_pools, so a previously dropped obo catalog returns to LIVE sessions (obo has no consent flow to heal through). - ChatSession construction re-runs its tool rebuild when the change marker advanced during its authoritative read — the mirror race where a fresher listener update was clobbered by the constructor's staler snapshot. - evict_user_session's drop task is now tracked (_spawn_background) so shutdown cancels it instead of abandoning a parked task. Dedup/altitude from the round: _schedule_dead_grant_drop is the single drop block (was three byte-identical copies); _pool_lookup_verdict is the single lookup classification — rendering and _lookup_grant_dead both derive from it, with literal code strings kept so the consent-url sibling audit still sees the sites (expected count 7 -> 5 after the collapse); PoolEntryState.drop_session() pairs session/bound_token clearing structurally (owner-death was missing the bearer clear).	2026-07-13 21:13:42 -07:00
Patrick Buckley	49a30c1547	fix(mcp): round-2 review fixes for the catalog-retention branch ... Five confirmed correctness findings, all in the round-1 fix code: - The dead-grant catalog drop at the token-lookup error sites is now SCHEDULED instead of awaited: the drop waits on open_lock, which a same-key dispatch holds across its entire SDK call, so awaiting let a token-side error stall past the sync timeout and charge the breaker it is documented to bypass. - The double-401 drop is removed entirely: a second 401 after a SUCCESSFUL forced refresh proves the grant is alive at the AS — it is the resource server rejecting a fresh bearer (JWKS lag, audience misconfig, clock skew), and dropping the catalog made RS recovery unhealable for live sessions. A genuinely revoked grant converges via the token-lookup drop (its row is gone by then). - The drop decision has one source of truth (_lookup_grant_dead), gated on the token store + storage actually being wired: the obo lookup returns kind='missing' for boot-window infrastructure absences too, which must not clear catalogs. The empty-token fallback now classifies with its consent_required siblings. - The LRU pass re-checks the LIVE warm count per iteration again — the one-shot over-count never saw concurrent warm-set changes (revocation evictions, owner deaths, connects) and closed healthy transports below the cap. - _on_pool_owner_death clears bound_token: the third session-drop site the bearer-clearing sweep missed, and the one that cools an entry indefinitely. Also from the round: reconcile stores both pool-name registries as adjacent assignments and _retain_cooled documents the residual single-bytecode flip-tear window (restored by the same reconcile's re-prime); catalog-less drops skip the zero-delta rebuild+notify fan-out; session construction does one authoritative post-registration read instead of read-twice; evict_user_session schedules the locked drop directly.	2026-07-13 21:13:42 -07:00
Patrick Buckley	4d89efa7b8	fix(mcp): registry-liveness for cooled entries, dead-grant convergence, revoke interlock ... Fix round for the review of the #836 catalog-retention change (14 findings: 9 correctness, 5 cleanup): - Cooled retention now requires the server to still exist in the pool registries (_retain_cooled — ONE policy shared by the TTL skip and the close path): an admin delete/disable/rename/auth-flip drops the ghost catalog within one eviction tick. Pre-#836 the idle TTL bounded such ghosts to ~10 minutes; retention made them immortal, including a disabled server that stayed dispatchable and duplicate tool names after a flip to static. - A dispatch that learns the grant is durably GONE (token row missing or refresh permanently rejected — the mcp_consent_required class) drops that (user, server) catalog, so a disconnect made on another node converges here at first touch instead of re-offering revoked tools behind a consent card. Re-consent restores the tools through the existing consent-completion single-server prime. - Revocation drops serialize against an in-flight connect via the entry's open_lock (_drop_catalog_locked): an unserialized drop was republished (resurrected) by the connect's completing discovery, with nothing left to ever clear it. - The LRU pass counts closes incrementally and the TTL pass checks a once-per-tick listener snapshot instead of scanning the listener registry per entry under its lock. - bound_token (a plaintext bearer) is cleared whenever the session is dropped — it is dead on a session-less entry, and cooling otherwise retained it for the life of the user's sessions. - Per-user status falls back to the cooled catalog for its counts and reports the idle pool separately (user_pools_idle): cooled is the steady state now, and the warm-only view said '0 tools' for a catalog the same user's chat was actively offered. - Session construction re-reads the merged tool lists after listener registration, closing the read-then-register window that missed a concurrent drop's only notification. - Dedup: one retention policy, one warm predicate, one rebuild+notify sequence (was three copies), and the drop-catalog path now layers on _evict_session instead of copying its prologue. Known limits, deliberately deferred: shared-workstream participants who are not the acting user still lose their catalogs at TTL (not a regression — the next send re-primes), and the pre-existing orphaned-lock race on full-drop is unchanged.	2026-07-13 21:13:42 -07:00
Patrick Buckley	cb94ea349f	fix(mcp): retain per-user catalogs when pool sessions close under live sessions ... Idle-TTL eviction tore down a per-user pool entry, rebuilt the user's tool/resource/prompt catalogs (now empty), and notified listeners — so every live ChatSession for that user silently lost the server's tools after 10 idle minutes, with no way back: prime_user_pools only runs at session construction, acting-user change, and reconcile, and the emptied catalog closes the session-side is_mcp_tool gate, so even a history-motivated call can't reach the lazy-reconnect dispatch path. The dispatch-failure paths (_evict_session on 401/403/transport) cleared catalogs the same way, so a transport blip during a tool call caused the same permanent loss with no TTL involved — and made the breaker's half-open recovery and the consent/step-up cards unreachable. Both now follow _on_pool_owner_death's evict-session-keep-entry shape: - _evict_session drops only the session. The catalog stays; the next dispatch connect-or-reuses and re-runs discovery, so drift self-corrects and the refresh notification fans out then. - TTL eviction COOLS entries of users with a live session (a registered user-scoped tool listener): transport closed, entry and catalog retained, no fan-out. Users without one keep the full drop, so departed users' entries don't outlive their sessions. - The LRU cap now bounds WARM entries — the connection resources it exists to limit. Over the cap, live-listener users' entries are cooled rather than dropped; cooled catalog-only entries are bounded by live users x pool servers and reaped one tick after the user's last listener goes away. - Explicit disconnect keeps its semantics: evict_user_session routes to the new _evict_session_drop_catalog (clear + rebuild + notify) — the user asked for the tools to leave. Clearing the catalog also marks the entry droppable, so it can't linger cooled. Never-discovered stubs (no catalog) are always dropped, already-cooled entries are skipped by later ticks, and a cooled entry keeps its open_lock object for in-flight dispatchers. Applies to oauth_user and oauth_obo alike: the pool and its eviction are auth-type-agnostic, and for obo priming is the only path tools enter a catalog at all. Fixes #836	2026-07-13 21:13:42 -07:00
Patrick Buckley	3742e9660a	docs(changelog): #827 turn-interface unification + sampling-knob assignment scheme with upgrade notes	2026-07-13 08:48:27 -07:00
Patrick Buckley	e8a17921bf	fix(model-turn): round-3 review — complete the scheme rollout to the main loop, coordinator role, admin save path, and CLI switch ... - The main streaming loop now applies the in-code model-definition rung (caps.default_reasoning_effort) exactly like model_turn does, so the same alias samples identically between chat and every auxiliary lane (resolve_lane's stated contract). This also unblocks operator temperature on gpt-5.x aliases whose declared default is "none" — the main loop previously sent neither knob while aux lanes sent both. - coordinator.reasoning_effort default "medium" -> "" (the missed unset sentinel): coordinators inherit like every other lane; the role rung fires only when the operator stored a value. - admin webux: _onSettingChange no longer hides the save button for a blanked nullable number input, so the blank-means-inherit save path is actually reachable from the field it decorates. - /model switch on STORE-LESS sessions (the CLI) keeps the user's explicit --temperature//reason knobs when the target alias declares no override — the current knobs are the only authority there (mirrors the max_tokens fallback). Store-backed sessions still re-resolve. - ModelLane docstring no longer documents the removed caller-default effort rung; CLI status line shows any resolved effort ("medium" is no longer a hidden code default); dead `u = usage` alias dropped; three test docstrings re-pointed from the deleted ChatSession._maybe_synth_reasoning_block to model_turn.synth_reasoning_block.	2026-07-13 08:48:27 -07:00
Patrick Buckley	257a8c12ec	fix(model-turn): no caller-default effort rung — local vocabularies make any code effort token unsafe ... Follow-up ruling on the round-2 batch: default_reasoning_effort is removed entirely. On local lanes effort_passthrough forwards the value VERBATIM with the template as the sole authority on validity, and we explicitly do not define effort vocabularies (or floors) for local models — so a code-chosen "low" is an unvetted token, and on manual-thinking boxes it flips enable_thinking on for lanes the operator never configured, diverging from the main loop's unset. The effort scheme is now exactly the temperature scheme: explicit relay > alias > stored config > model definition > omit. Utility/guard consequences handled the honest way instead: - title gen: _TITLE_MAX_TOKENS 2048 -> 8192 (the budget must fit a full thinking pass at the MODEL'S OWN default now that code never bounds it) and the prompt enforces a hard 3-word maximum so the visible answer is trivially cheap regardless of what thinking spent. - output guard: keeps its 512 cap; an unbounded thinking model that overruns it parses to a labelled llm_error verdict (heuristic tier stands) and the documented remediation is an effort value on the guard's model alias.	2026-07-13 08:48:27 -07:00
Patrick Buckley	b6391d1f90	fix(model-turn): one sampling-knob assignment scheme — alias > config > model definition > omit ... Round-2 review fixes. The round-1 de-pinning collided with ConfigStore.get's default-on-miss semantics: the registry defaults (temperature 1.0, effort "medium") were manufactured onto every store-backed lane's wire, making the documented "unset -> omit" terminal unreachable. Unset is now representable end to end, and one scheme governs every lane: per-model alias value > operator-stored global setting > in-code model definition (effort only: caps declaration) > field omitted, inference engine's default rules. - settings_registry: model.temperature default None, model.reasoning_effort default "" — the registered defaults ARE the unset sentinels, so the admin UI and the wire agree. Admin webux renders nullable floats blank ("(inherit model default)") and maps blank-save to reset; the "" effort choice reads "(inherit)". - model_turn: resolve_temperature_setting/resolve_effort_setting are the ONE pair of operator-rung resolvers, shared by resolve_lane, both session factories, and the /model switch (the 4th-copy mirror is gone; the switch no longer leaks the previous model's override on store-less sessions). The caps rung moved out of the lane into model_turn's effective computation, below a new request-shaped default_reasoning_effort parameter (utility + output guard pass "low": budget coherence with their small token caps, not sampling policy — any operator or model-definition value beats it). The hidden "medium" terminal is gone. - providers: Protocol + all adapters take reasoning_effort: str | None = None (the Protocol-signature "medium" was the same manufactured pin one layer down); ModelCapabilities.default_reasoning_effort defaults "" — commercial rows all declare theirs explicitly, so only local lanes and Anthropic change, both to match their real serving defaults (Anthropic manual-thinking models no longer get implicit thinking-on-medium). reasoning_template_kwargs distinguishes unset (inject nothing; template default rules) from the explicit "none" off-switch. apply_temperature skips temperature unless reasoning is EXPLICITLY off on none-declaring models (unset leaves the server default in charge, possibly reasoning-on). - session: ctor takes temperature: float | None / reasoning_effort: str | None = None; _save_config/resume round-trip unset as "" (the str(None) era guarded); _run_agent relays session temperature AND effort on the same-alias fall-through only (a task alias's configured knobs stay reachable in both directions). - optimizer: the five meta lanes are decoupled from --temperature/ --reasoning-effort (test-model knobs, per their documented meaning); registry-less meta lanes omit both fields. - cli: --temperature/--reasoning-effort default unset and fall through the model config instead of pinning 0.5/"medium" for every CLI session. - cleanup from the review's below-cap findings: dead resolve_server_type deleted (tests re-pointed at _server_type_of), stale ChatSession comments in _openai_responses fixed, _store_get_or_none extracted, eval system-turn conversion hoisted out of the per-turn loop, dead _provider_extra_params patch removed, test_perception uses the shared mock_completion_result, effort_ladder uses apply_capability_overrides instead of a SimpleNamespace fake config. Wire goldens regenerated: the only drift is the manufactured "medium" effort vanishing from unset-effort requests (Responses reasoning.effort, Chat/Google reasoning_effort, Anthropic output_config.effort) — pure removals, no additions. Ladder tests now fake ConfigStore with the REAL get() semantics (registry default on miss) so a forgiving fake can't mask this class of bug again.	2026-07-13 08:48:27 -07:00
Patrick Buckley	09fd17f2da	fix(model-turn): reasoning effort rides the ladder too; round-1 review fixes ... Patrick's rulings applied from the round-1 high review: - reasoning_effort loses every code pin, same as temperature: ModelLane resolves the ladder (ModelConfig.reasoning_effort → global model.reasoning_effort setting → the lane capabilities' default_reasoning_effort), model_turn takes str | None, and the pins in both judges, all five optimizer lanes, _utility_completion's signature default, and model_turn's own "medium" default are gone. Explicit relays of user/operator knobs (session effort on the agent seam and web-fetch, harness knobs in eval) stay relays. Effort's terminal is the caps default, not wire omission — it gates thinking modes, so unset ≠ the explicit "none" value. - model.temperature setting default 0.5 → 1.0 (safer for modern models; several providers no longer accept temperature at all — those drop it via capabilities regardless). No judge-specific knob: a judge alias with a per-model override is the remediation path. - The agent seam keeps the alias ladder (configured → inherited global → none), per ruling; the ModelLane docstring no longer documents the removed session-relay convention. - Optimizer lanes get real operator knobs: the existing --temperature / --reasoning-effort CLI flags now relay into all five internal LLM steps (previously they reached only the eval sessions, leaving the deleted pins with no replacement mechanism). Round-1 cleanups: create_streaming widened to float | None (the Protocol's two entry points agree; all callers pass explicitly); model_turn's provider invocation is a direct keyword call again (strict mypy re-checks it); perception threads the caller's already-resolved capabilities (one config generation across gate and wire); redundant extra_params pre-resolution dropped at utility/agent/eval; the synth source-tag joins the one-fetch-per-call cfg chain; effort_ladder delegates its capability merge to resolve_capabilities; stale _maybe_synth_reasoning_block pointers fixed in the providers package. Wire goldens regenerated: the only drift is the hidden "temperature": 0.5 pin vanishing from unset-temperature requests.	2026-07-13 08:48:27 -07:00
Patrick Buckley	3eb789dfff	fix(model-turn): temperature truly inherits — None never reaches the wire ... The second xhigh review caught the fix-round design error one layer down: omitting the temperature kwarg did not yield the server default — every adapter's create_completion signature defaulted it to 0.5 and apply_temperature wrote it to the wire, so the deleted lane pins had silently become a hidden universal 0.5 pin. The house rule is now implemented end to end: - Protocol + adapters take temperature: float | None = None, and None is OMITTED from the wire (apply_temperature None-gate; Anthropic's builder keeps its API-required thinking=1.0 forcing but never writes an unresolved value; Responses/xAI builders widened). - resolve_lane climbs the documented ladder: ModelConfig.temperature → ConfigStore global model.temperature (new config_store param, threaded from ChatSession into both judges and perception) → None. - perception.describe/describe_cached take alias/registry/config_store so operator settings on the perception alias actually reach the wire (previously structurally unreachable — no remediation path for a degraded memoized description). - The agent seam stops relaying the SESSION model's temperature: the task/agent alias's own ladder governs, per the inherit-from-the-model contract. Generation-coherence and audit fixes from the same review: - ChatSession._resolve_capabilities fetches its config UNCAUGHT again — a registry failure on the session's own alias raises loudly instead of silently caching degraded static-table caps for the session lifetime (the never-crash fetch is a judge-constructor property). - Judge constructors pass cfg=model_cfg (zero independent get_config fetches; pinned by test); the per-evaluation lane's constructor- frozen capabilities are documented as deliberate (window-coupled, refreshed on judge swap). - OutputGuardJudge splits _lane_alias from _judge_model_alias so the audit label keeps its pre-#827 fallback semantics ("" → raw model id) while lane resolution inherits the session alias. - model_turn fetches the alias config ONCE per call and threads it into both live flags (cfg sentinel standardized across the resolvers: ... = fetch for me, None = fetched-and-missed — also removes resolve_lane's latent double-fetch on a miss). - cap_tool_calls shared by the eval and optimizer loops; hand-built ModelLane sites converted to resolve_lane; hand-rolled test result namespaces consolidated onto mock_completion_result; stale synth-test module docstring re-pointed.	2026-07-13 08:48:27 -07:00
Patrick Buckley	7e07f2ea93	feat(core): phase 2 — every single-shot lane speaks Turn IR (#827) ... create_completion now has exactly one caller: model_turn. The π-side lanes migrate off hand-built OpenAI dicts: - _utility_completion (title gen, compaction, web-fetch extraction) takes list[Turn] and runs the session's primary lane through model_turn; its three call sites build Turn.system/Turn.user. - perception.describe builds a by-reference trajectory (AttachmentRef + the prebuilt parts via resolve_attachments, reintroduced on model_turn with its first caller and pinned by tests) — Turn IR never carries inline media bytes, matching the main loop's wire path. Its temperature=0.2 pin is gone (house rule). - eval HeadlessSession's loop lowers system prompts through the turns_from_dicts bridge and appends result.turn; the parallel-call cap now also drops the native lane on a capped turn (a capped mirror with a full native lane would replay orphan tool blocks). - optimizer: all five sites (diversifier, observer, analyst loop, tool optimizer, prompt optimizer) build Turn IR through per-function lanes; every temperature pin (0.8/0.3/0.3/0.3/0.6) removed per house rule — sampling behavior belongs in the model's configuration. Test mocks move to the shared full-shape helper where the model_turn re-ingest now runs; perception/attachment tests assert the by-reference placeholder + resolver contract instead of inline parts.	2026-07-13 08:48:27 -07:00
Patrick Buckley	b0937683ae	fix(model-turn): apply the #827 phase-1 review round ... Behavior fixes, per review + house rules: - Judges no longer pin temperature=0.0 — the lane inherits the model's configured temperature (ModelConfig.temperature via resolve_lane), and model_turn omits the kwarg entirely when nothing resolves. House rule: code never pins a temperature; modern models often misbehave below 1.0, so the model's configuration is the source of truth. This also dissolves the extra_body-overrides-judge-pins collision: operator pins reaching the judge lane is the doctrine working. - Session-fallback judges inherit the session's registry alias (session_model_alias threaded from ChatSession), so the registry- resolved extra_params / replay flag / vLLM attach apply on the default judge.model-unset configuration instead of only on explicit aliases. - Blank-id native lanes are repaired, not dropped: model_turn backfills the manufactured mirror ids into blank-id native client tool blocks pairwise (the #825 1:1 ordering invariant), so thought_signature survives Google's blank-id compat responses and thinking blocks keep their continuity on blank-id locals. Only blank ids are ever written — a provider-assigned id (possibly signature-covered) is never touched — and any pairing mismatch falls back to the #825-converged total drop. - model_turn(mint=...) without wire_id_map now raises: minted ids are unrestorable without the recovery map, and the two parameters were independently optional by accident. - Lane resolution reads ONE defensively-fetched ModelConfig (_get_config_or_none): a registry hot-reload mid-resolution can't mix config generations, and an alias that raced away degrades each facet to its miss behavior instead of aborting a judge constructor into the silent session-model downgrade. Extraction hygiene, per review: - Dead session wrappers deleted (_resolve_server_type, _maybe_synth_reasoning_block, _get_server_compat) and their tests re-pointed at the module functions; the stranded reasoning-types comment and two stale doc pointers cleaned up. - Speculative extra_headers / resolve_attachments pass-throughs dropped from model_turn until a caller lands (phase 2/4 reintroduces them with their lane). - _server_type_of(cfg) is the one reader of server_compat.server_type; the vLLM-attach gate and resolve_server_type both use it, retiring the change-both-readers discipline comment. - dataclasses import hoisted; module docstring restated as the durable contract (grep callers for coverage) instead of a rotting snapshot. - mock_completion_result shared in tests/_session_helpers.py — one definition of "every field the re-ingest reads".	2026-07-13 08:48:27 -07:00
Patrick Buckley	54dd4ed50a	feat(judge): both judges speak Turn IR through model_turn (#827) ... The intent judge's evidence loop and the output-guard's single shot now build list[Turn] and call model_turn — the hand-built OpenAI-dict message construction is gone, and with it the judges' private interlingua. The assistant turns they append carry the provider-native lane, so the loop keeps reasoning continuity across its own turns. That is what unblocks Gemini: thought_signature rides provider_blocks and is reconstructed by the Google adapter's fidelity swap, so the provider_name == "google" tool-skip is deleted — the Gemini judge runs the same evidence-tool loop as every other provider instead of degrading to a single-shot, tool-blind verdict. judge.py's _resolve_model_capabilities mirror (#826) is deleted; both judges resolve capabilities through the shared lane resolver, and each evaluation builds a ModelLane (fresh client, constructor caps, registry-resolved extra_params + live flags). The shared resolver inherits the mirror's defensive non-dict capabilities check — without it a malformed registry row would silently downgrade a judge to the session model instead of just skipping the overrides. Judge calls now resolve extra_params and replay_reasoning_to_model from the registry like every other lane (previously: never sent, and the protocol's back-compat default respectively). Test mocks grow the CompletionResult fields the model_turn re-ingest reads (provider_blocks, reasoning); alias-registry mocks wire get_config, which the unified resolver uses.	2026-07-13 08:48:27 -07:00
Patrick Buckley	ab35eb4215	refactor(core): extract model_turn, the shared plant-call primitive (#827) ... Lower-and-sample is now one surface: core/model_turn.py owns the Turn-IR lowering seam (dicts_from_turns -> sanitize_tool_call_arguments -> restore_provider_tool_ids -> Phase 5 vLLM attach), the provider call, and the re-ingest to an assistant Turn carrying the native lane. ModelLane binds a resolved lane (provider, client, model, capabilities, extra_params) and carries the registry so live operator toggles (replay-reasoning, vLLM attach) keep re-resolving per call. The task-agent seam is the first client: _run_agent builds a ModelLane and calls model_turn with a mint closure; the inline mint/back-fill/ finalize block collapses to appending result.turn. Session capability/ extra-params/replay/finalize helpers become delegates to the module functions, so lane resolution has exactly one logic path. model_turn is policy-free by contract: retry, deadlines, tool execution, and usage recording stay with each caller. Two agent-path tests move their replay-flag pin to the module seam (one had gone vacuous against the session wrapper); _record_aux_usage now takes UsageInfo rather than a CompletionResult.	2026-07-13 08:48:27 -07:00
renovate[bot]	9b01d8e569	chore(deps): update dependency typescript to v7	2026-07-13 04:57:36 -07:00
renovate[bot]	4878f16475	chore(deps): update github actions	2026-07-13 04:57:21 -07:00
Patrick Buckley	b2b8b6f65e	fix(mcp): schedule node reload after admin write instead of blocking on it ... 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.	2026-07-12 19:03:35 -07:00
Patrick Buckley	d6ccc5ed17	fix(console): show 'per-user' for idle pool MCP servers, not 'connecting' ... 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.	2026-07-12 19:03:35 -07:00
Patrick Buckley	b3cd91f1a0	fix(mcp): auto-notify nodes on admin create/update/delete ... 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.	2026-07-12 19:03:35 -07:00
Patrick Buckley	9391509e85	fix(oidc): trust Entra's graph.microsoft.com userinfo out of the box ... 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.	2026-07-12 19:03:35 -07:00
Patrick Buckley	49f2266e20	fix(mcp): address review of the re-prime self-heal ... - 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.	2026-07-12 19:03:35 -07:00
Patrick Buckley	d1de602b78	docs(mcp): align token-encryption + mint docstrings with oauth_obo ... 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.	2026-07-12 19:03:35 -07:00
Patrick Buckley	86253a3b07	fix(mcp): re-prime active sessions when a pool server appears mid-reconcile ... 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.	2026-07-12 19:03:35 -07:00
Patrick Buckley	530aaa6632	refactor(mcp): drop dead grant-profile recompute after runtime rediscovery ... 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).	2026-07-12 19:03:35 -07:00
Patrick Buckley	8a1efaf55e	perf(mcp): skip redundant priming credential read; dedup sweep clear bookkeeping ... 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).	2026-07-12 19:03:35 -07:00
Patrick Buckley	08d765a74a	fix(mcp): record effective obo scope so entra .default isn't cached as narrow ... 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.	2026-07-12 19:03:35 -07:00
Patrick Buckley	903cf5f72d	fix(oidc/mcp): login-path self-heal, guard mint persist, CAS credential rotation ... 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.	2026-07-12 19:03:35 -07:00
Patrick Buckley	ec079f0df3	fix(oidc/console): unblock obo edits when OIDC off; latch config-invalid rediscovery ... 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).	2026-07-12 19:03:35 -07:00
Patrick Buckley	af56170be6	fix(oidc/mcp): make runtime OIDC rediscovery actually work; preserve oauth_user paths ... 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.	2026-07-12 19:03:35 -07:00
Patrick Buckley	50d0ac9833	fix(mcp): classify oversized token error by status; dedup transition/obo-scan ... 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.	2026-07-12 19:03:35 -07:00
Patrick Buckley	09aa50b7a1	fix(mcp): close obo auth-column leak, capture gate, and cooldown classification ... 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.	2026-07-12 19:03:35 -07:00
Patrick Buckley	c53bd464d0	refactor(mcp): dedup obo credential decrypt, cooldown arming, pool set, error copy ... 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.	2026-07-12 19:03:35 -07:00
Patrick Buckley	6d80051925	fix(mcp): decouple capture key guard from OIDC discovery; bound obo token TTL ... 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).	2026-07-12 19:03:35 -07:00
Patrick Buckley	d2e69ca527	fix(mcp): coherent obo auth-type carry-over + honest error affordances ... 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.	2026-07-12 19:03:35 -07:00
Patrick Buckley	32c76499fa	fix(mcp): harden obo mint path and admin lifecycle after review ... 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.	2026-07-12 19:03:35 -07:00
Patrick Buckley	44e9d46e40	fix(mcp): address pre-push review — obo scope/audience/priming defects ... 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.	2026-07-12 19:03:35 -07:00
Patrick Buckley	3e88c54751	test(mcp): check in oauth_obo e2e harnesses under scripts/obo-e2e ... 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.	2026-07-12 19:03:35 -07:00
Patrick Buckley	891c8b1785	docs(mcp): operator guide for oauth_obo single-credential sign-in passthrough (slice 5) ... 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.	2026-07-12 19:03:35 -07:00