[fix] Connect-flow feedback: actionable errors, correct auth mode, working decline - #5909
Merged
Merged
Conversation
POST /api/tools/connections/ returned a bare, unactionable 500 when Composio has no managed OAuth auth config for a toolkit (e.g. telegram) — the adapter raised AdapterError (core/gateway/connections/exceptions), a different class from the one @handle_adapter_exceptions catches (core/tools/exceptions), so it fell through to the generic @intercept_exceptions handler. create_connection now catches the gateway AdapterError directly and returns a 422 with a message telling the user this toolkit needs custom OAuth credentials in this environment; any other AdapterError from create_connection also gets the upstream detail instead of a generic 500. Before-state reproduced live on the 8180 dev stack (POST .../tools/connections/ for telegram -> 500, traceback matching adapter.py:188/router.py:773 exactly).
…OAuth integration.auth_schemes came back None for toolkits like telegram that have no Composio-managed auth config at all (composio_managed_auth_schemes is empty), even though they ARE connectable via use_custom_auth. Every caller that picks a connect mode from this field — the settings ConnectModal's resolveAvailableModes, and the agent connect widget's mode resolver added in this PR — defaults to "oauth" when it sees no schemes, which Composio then 404s on for these toolkits. Fall back to auth_config_details (the same field the connections adapter already reads to pick use_custom_auth's authScheme) when the managed list is empty, mapping any non-oauth/non-no_auth mode to api_key — mirrors the connections adapter's own "oauth" not in mode.lower() heuristic exactly. Verified live on the 8180 dev stack: GET .../tools/catalog/providers/composio/ integrations/telegram currently returns no auth_schemes at all (before-state). POST .../tools/connections/ with data.auth_scheme=api_key for telegram returns 200 with a redirect_url identical in shape to the OAuth path — Composio's own hosted page collects the credential, so no new widget UI is needed for this mode, only correct mode selection (see the next commit).
…al auth mode
Two bugs in the request_connection widget, both from a failed create POST
(e.g. bug 1's telegram 500/422):
1. Silent failure. useConnectFlow settled a create failure through the same
path as decline/cancel/timeout (finish() -> ConnectOutput, not
{errorText}), so ConnectToolWidget's render order — which checks
meta.settled/outcome before phase==="error" — always fell through to a
generic "Connection not completed" chip with no indication anything had
gone wrong. From the user's seat, Retry looked completely dead. The reason
is now carried through settle()'s output and rendered verbatim unless it's
one of the three expected non-error terminal states (declined/cancelled/
timeout).
2. Wrong auth mode. mode was read straight from the agent's input.mode hint,
defaulting to "oauth" whenever the agent didn't say "api_key" — but the
agent has no way to know Composio's supported schemes either. useConnectFlow
now cross-checks the hint against the toolkit's real auth_schemes (fetched
via useToolIntegrationDetail, mirroring the settings ConnectModal's
resolveAvailableModes) and overrides it when the toolkit doesn't actually
support it.
Error messages prefer the backend's own 4xx detail (extractConnectErrorMessage)
over Fern's default Error.message, which bundles a multi-line status/body dump
not fit to show a user.
Verified: 7 new vitest unit tests for the two pure helpers (resolveConnectMode,
extractConnectErrorMessage); tsc --noEmit clean; pnpm turbo run lint clean.
… decline Clicking "Not now" on a parked connect request fired ZERO network requests, the session_interactions row stayed pending forever, and the agent turn never resumed. Reproduced live on the 8180 dev stack: no new /services/agent/v0/invoke request after the click, and the transcript visibly reverted to an earlier turn. Root cause: useSessionRecordsWatch's SSE relay can tick and call refreshFromRecords while the run is idle (busy=false — the resume hasn't been dispatched yet) but a local addToolOutput settle (decline, or any client-tool settle) is still waiting for sendAutomaticallyWhen to actually dispatch it. liveGateInteractionRef is written synchronously the moment the settle fires and only cleared once dispatch lands, so it is non-null for exactly that gap. A relay tick landing in that window adopted a server transcript that predates the settle, silently discarding it before the resume could ever be sent. useSessionHydration now takes pendingResumeRef (wired to the existing liveGateInteractionRef in useAgentChatSession — no new signal invented) and skips the records-changed relay's adoption while it is set, via the new shouldSkipRecordsRefresh guard alongside the existing busy check. The poll and one-time SWR-on-open paths don't need the guard: they don't fire mid- conversation from a local settle the way the relay does. Verified: 4 new unit tests for shouldSkipRecordsRefresh; the live repro above pinned the before-state (this exact worktree's fix could not be re-verified live, since the 8180 stack hot-reloads the main checkout, not this branch — see the AGENTS.md verification note); full AgentChatSlice vitest suite (162 tests) and tsc --noEmit stay clean.
dosubot
Bot
added
size:L
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR adds Composio auth-scheme fallback, HTTP 422 handling for connection adapter failures, clearer client connection errors, and safeguards that prevent session refreshes from overwriting active client-tool interactions. ChangesConnection flow
Session hydration safeguards
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ConnectToolWidget
participant useConnectFlow
participant ToolsConnectionsAPI
participant ComposioAdapter
ConnectToolWidget->>useConnectFlow: request connection
useConnectFlow->>ToolsConnectionsAPI: create connection with resolved auth mode
ToolsConnectionsAPI->>ComposioAdapter: create connection
ComposioAdapter-->>ToolsConnectionsAPI: result or adapter error detail
ToolsConnectionsAPI-->>useConnectFlow: connection result or HTTP 422 detail
useConnectFlow-->>ConnectToolWidget: terminal reason or failure message
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 13b9d158-7c91-4cd5-914b-d8194d78e91d
📒 Files selected for processing (10)
api/oss/src/apis/fastapi/tools/router.pyapi/oss/src/core/gateway/catalog/providers/composio/adapter.pyapi/oss/tests/pytest/unit/tools/test_catalog_auth_schemes_fallback.pyapi/oss/tests/pytest/unit/tools/test_create_connection_errors.pyweb/oss/src/components/AgentChatSlice/components/clientTools/ConnectToolWidget.tsxweb/oss/src/components/AgentChatSlice/components/clientTools/useConnectFlow.test.tsweb/oss/src/components/AgentChatSlice/components/clientTools/useConnectFlow.tsweb/oss/src/components/AgentChatSlice/hooks/useAgentChatSession.tsweb/oss/src/components/AgentChatSlice/hooks/useSessionHydration.test.tsweb/oss/src/components/AgentChatSlice/hooks/useSessionHydration.ts
Contributor
Railway Preview Environment
Updated at 2026-08-10T16:51:18.407Z |
This was referenced Aug 10, 2026
Open
…nown CodeRabbit finding on #5909: during the initial catalog request, `integrationDetail` is undefined and `resolveConnectMode` retains the raw agent hint — a user who clicks Connect in that window still sends "oauth" for a toolkit like telegram that only supports api_key, 404ing exactly like before bug 1's fix. `resolveConnectMode`'s "keep the hint" fallback is correct for the RENDER (it must return *something*), but a click landing in that same window must not be allowed to act on it. New `isConnectModeResolving` (pure, unit-tested): true only while there's a real integration key AND its catalog lookup is genuinely in flight AND a bounded 8s wait hasn't elapsed. Both `hasIntegrationKey: false` (a malformed call — a disabled TanStack Query reports `isLoading: true` forever, which must not permanently disable Connect) and `timedOut: true` (a dead network must not either) force it false — the gate cannot latch shut. Connect/Retry are disabled in the dock and the inline widget for this window, and `runConnect` itself refuses to fire as defense-in-depth against a click that raced the disabled state. 4 new tests pin the four cases (blocks / unblocks-on-resolve / unblocks-for-no-key / unblocks-on-timeout); the existing "keeps the hint while loading" test on `resolveConnectMode` is re-labeled to make clear it's describing the RENDER fallback, not a guarantee a click can act on.
…olves CodeRabbit finding on #5909: `shouldSkipRecordsRefresh` was only checked at the TOP of `refreshFromRecords`, before `loadSessionMessages` (a real network round trip) even starts. If a client-tool settle (e.g. a connect decline) arrives WHILE that fetch is in flight, the entry check already passed and never re-runs — so the `.then` still adopts the (now stale) transcript it fetched before the settle happened, clobbering it exactly the way the previous commit in this family (the `pendingResumeRef` guard itself) was meant to prevent. `refreshFromRecords` now re-checks `shouldSkipRecordsRefresh` a second time, immediately before `adoptServerTranscriptRef.current(...)` inside the `.then`, using the same live refs — guarded at both the entry point and the adoption point, closing the gap between "checked" and "the fetch that was in flight when a settle landed."
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Mahmoud hit a dead end trying to connect a Telegram agent to Composio (live sessions
e8c3b72a-0fb0-4895-a77d-3f073672da8aand9d4e0324-344c-42f0-ab72-a7afe0246b72on the 8180 dev stack,session_interactionsrows stuckpending). Four bugs compounded into that dead end; a fifth (found mid-fix, on the coordinator's steer) is the actual root cause that made the flow unusable for telegram in the first place. This PR fixes four of the five. The fifth, the dock'sstoppedgate silently hiding a re-parked interaction, is being isolated by a separate instrumented capture and will land as its own small follow-up (see "Known remaining issue" below).Changes
1. Backend 500 -> actionable 422. Composio has no managed OAuth config for some toolkits (telegram is one; it only supports
use_custom_auth).POST /api/tools/connections/raisedAdapterErrorfrom the gatewayconnectionsdomain, a different class from the one@handle_adapter_exceptionscatches (core/tools/exceptions), so it fell through to the generic@intercept_exceptionshandler and returned a bare 500.create_connectionnow catches the gatewayAdapterErrordirectly and returns 422 with a message telling the user the toolkit needs custom OAuth credentials in this environment.2. The real root cause: wrong auth mode, not a Composio outage. While reproducing bug 1 live, the coordinator caught that the telegram POST always sent
auth_scheme: "oauth", and traced it touseConnectFlow'smodeblindly trusting the agent'sinput.modehint (which defaults to oauth). Telegram's real scheme is a bot token (api_key-style), and the adapter already supportsuse_custom_authfor it (adapter.py:162-175) - the flow just never asked for it. Live proof this works:POST /tools/connections/for telegram withauth_scheme: "api_key"on a minted 8180 account returns 200 with aredirect_url(https://connect.composio.dev/link/...), identical in shape to the OAuth path - Composio's own hosted page collects the bot token, so no new widget UI is needed.I first checked whether the settings page's
ConnectModalalready picks the right mode (it looked like it should) - it doesn't. ItsresolveAvailableModesreadsintegration.auth_schemes, which the backend built only from Composio'scomposio_managed_auth_schemesfield. For telegram that field is empty (confirmed live:GET .../tools/catalog/providers/composio/integrations/telegramreturns noauth_schemesat all), soresolveAvailableModesfalls through its ownlength === 0 -> oauthdefault. Same latent bug, just unhit because most toolkits do have managed oauth. Fixed at the shared source:_parse_integration_detailnow falls back toauth_config_details(the same field the connections adapter reads) when the managed list is empty, so both the settings modal and the agent widget see the toolkit's real scheme.useConnectFlownow cross-checks the agent'smodehint against the toolkit's realauth_schemes(fetched viauseToolIntegrationDetail) and overrides it when the toolkit doesn't actually support the hinted mode.3. Silent failure on the widget. When the create POST failed,
useConnectFlowsettled it through the exact same path as decline/cancel/timeout (finish()-> aConnectOutput, not{errorText}).ConnectToolWidget's render order checksmeta.settled/outcomebeforephase === "error", so a real failure always fell through to a generic "Connection not completed" chip - Retry looked completely dead, no error anywhere. The failure reason is now carried throughsettle()'s output and rendered verbatim unless it's one of the three expected non-error terminal states (declined/cancelled/timeout). Error messages prefer the backend's own 4xxdetailover Fern's defaultError.message, which bundles a multi-line status/body dump not fit to show a user.4. "Not now" fired zero network requests. Reproduced live: clicking "Not now" on a parked connect request produced no new
/services/agent/v0/invokecall, and the transcript visibly reverted to an earlier turn. Root cause:useSessionRecordsWatch's SSE relay can tick and adopt a fresh server transcript while the run is idle (busy=false- the resume hasn't been dispatched yet) but a localaddToolOutputsettle is still waiting forsendAutomaticallyWhento actually fire it.liveGateInteractionRefis written synchronously the instant the settle happens and only cleared once dispatch lands, so it's non-null for exactly that gap. A relay tick landing in that window adopted a transcript that predated the settle, silently discarding it before the resume could ever be sent.useSessionHydrationnow takes that same ref (pendingResumeRef, no new signal invented) and skips the relay's adoption while it's set.Known remaining issue (not in this PR)
AgentConversation.tsx:351gates the connect dock onbusy || stopped ? null : ....stoppedonly clears on a new submit/regenerate, but the server-side run can continue past a client Stop and park a NEW interaction, which the latchedstoppedthen hides forever. A separate instrumented capture is isolating this (two competing theories for why the widget never rendered in one observed case); it'll land as its own small follow-up once the evidence picks the winner.Tests / notes
ruff format+ruff checkclean. New unit tests:api/oss/tests/pytest/unit/tools/test_create_connection_errors.py(3 tests, router-level, exercises the actualToolsRouter.create_connectionhandler with a faked service raising the real exception),api/oss/tests/pytest/unit/tools/test_catalog_auth_schemes_fallback.py(4 tests, theauth_config_detailsfallback and a regression guard that managed-auth toolkits are untouched). All pass.tsc --noEmitclean,pnpm turbo run lint --filter=@agenta/ossclean (no new warnings). New unit tests:useConnectFlow.test.ts(7 tests,resolveConnectMode+extractConnectErrorMessage),useSessionHydration.test.ts(4 tests, the newshouldSkipRecordsRefreshguard). FullAgentChatSlicevitest suite (162 tests, 21 files) passes with no regressions.POST /tools/connections/for telegram -> 500, traceback matchingadapter.py:188/router.py:773exactly.GET .../catalog/providers/composio/integrations/telegram-> noauth_schemes;POST /tools/connections/withauth_scheme: "api_key"-> 200 +redirect_url(proves the fix's target shape works today, api key just isn't being requested)./services/agent/v0/invokerequests and the transcript reverting to an earlier turn (matches thesession_interactionsrows for Mahmoud's two sessions, stillpendingwith noupdated_at).maincheckout, not this branch. Verified by unit test + code trace instead, per the repo's verification convention for this situation.What to QA
api_key-mode custom auth and open a redirect popup, not 404./services/agent/v0/invokerequest fires and the agent's next turn acknowledges the decline.