Skip to content

feat(example): GitHub OAuth for the assistant + resume-stream stability fixes#1374

Merged
threepointone merged 5 commits into
mainfrom
feat/assistant-github-auth
Apr 24, 2026
Merged

feat(example): GitHub OAuth for the assistant + resume-stream stability fixes#1374
threepointone merged 5 commits into
mainfrom
feat/assistant-github-auth

Conversation

@threepointone
Copy link
Copy Markdown
Contributor

@threepointone threepointone commented Apr 23, 2026

Summary

This PR started as "add GitHub OAuth to the assistant example" and grew to include two library fixes that fell out of actually deploying it and exercising mid-stream refresh, plus a follow-on MCP-routing fix caught in review. The branch is structured as focused commits so reviewers can look at each concern independently; the two fix(...) library commits are self-contained and can be split into their own PRs if preferred.

feat(example): add GitHub OAuth to the assistant (8223667)

  • Lifts the GitHub OAuth helpers from examples/auth-agent into examples/assistant, so the assistant is deployable and user-scoped without any browser-chosen DO names.
  • Worker handles /auth/* and forwards /chat* to the authenticated user's MyAssistant instance via getAgentByName(env.MyAssistant, user.login).
  • Client gates the existing Think UI behind a GitHub sign-in view and adds a sign-out button to the header. The only functional client change is useAgent({ agent: "MyAssistant", basePath: "chat" }).
  • README and .env.example walk through the OAuth setup and deployment steps.

fix(example): close auth bypass + polish assistant UX (18e9202)

  • Security fix. The previous config routed /agents/* through the Worker and fell back to routeAgentRequest, which let anyone reach /agents/my-assistant/<login> unauthenticated and talk directly to that user's Durable Object. The Worker routes are now narrowed to /auth/* and /chat*, and the routeAgentRequest fallback is gone. The only path into MyAssistant is now the GitHub-authenticated /chat* route.
  • Unifies the two "loading" UIs (auth-check spinner + useAgentChat Suspense fallback) so the shell stays consistent across phases.
  • Opts MyAssistant into sendIdentityOnConnect: true, safe now that the ai-chat fix below prevents the resulting agent.name transition from orphaning the Chat instance.

fix(think): do not broadcast CHAT_MESSAGES while a stream is in flight (ff19bde)

Think was unconditionally sending cf_agent_chat_messages on every WebSocket connect. On a mid-stream refresh that broadcast arrived in the same connect sequence as cf_agent_stream_resuming and overwrote the in-progress assistant message the client was about to rebuild from the resumed stream — the reply stayed hidden until the turn completed. Now onConnect suppresses that broadcast when _resumableStream.hasActiveStream() is true; resume is the authoritative source of state during an in-flight turn. Matches what AIChatAgent already did. Regression tests cover all three states (no active stream, active stream, post-completion).

fix(ai-chat): keep Chat instance stable across agent.name identity transitions (8788939)

Even after the Think fix above, mid-stream refresh still didn't resume. useAgentChat was recreating the AI SDK Chat instance whenever agent.name mutated in place — which is exactly what useAgent({ basePath }) + sendIdentityOnConnect: true does (placeholder "default" → server-assigned login). The recreated Chat orphaned the in-flight resumeStream(), and since the resume effect is keyed on the ref, it didn't re-fire. stableChatIdRef now distinguishes an in-place name mutation from a genuine chat switch by checking the agent object's reference identity; the one-time URL-arrival upgrade (fallback → resolved key) still runs. Two regression tests cover the in-place mutation and the agent-object swap.

fix(example): route MCP OAuth callbacks through /chat* (7294a92)

Caught in review on this PR. addMcpServer(name, url) with no explicit callbackPath falls back to ${host}/agents/my-assistant/${this.name}/callback. Previously that worked because /agents/* was routed to the Worker with a routeAgentRequest fallback. After narrowing run_worker_first to /auth/* and /chat* (the security fix above), that default callback URL hits the SPA asset handler, serves index.html with a 200, and the DO never sees the OAuth code — MCP servers hang in AUTHENTICATING. sendIdentityOnConnect: true incidentally disables the framework's enforcement that would normally require callbackPath, so it slipped through silently. Passing callbackPath: "chat/mcp-callback" routes the redirect through the same authenticated /chat* dispatcher, and Agent._onRequest picks up the callback via mcp.isCallbackRequest().

Changesets

  • .changeset/think-onconnect-no-clobber-resume.md (@cloudflare/think, patch)
  • .changeset/ai-chat-stable-chat-id.md (@cloudflare/ai-chat, patch)

Test plan

  • npm run ci
  • New unit tests: packages/think/src/tests/onconnect-broadcast.test.ts, two new cases in packages/ai-chat/src/react-tests/use-agent-chat.test.tsx
  • Manual end-to-end: local GitHub OAuth App, npm start, sign in, start a long-running "tell me a story" stream, refresh mid-stream — the stream now resumes and the in-progress assistant message is preserved.
  • Manual end-to-end: add an OAuth-requiring MCP server via the UI and confirm the callback completes and the server reaches READY.

Follow-ups

The multi-session Chats/useChats() prototype is still a follow-up PR, per wip/think-multi-session-assistant-plan.md.

Made with Cursor

Copy the GitHub auth layer from examples/auth-agent into examples/assistant so
each user gets their own MyAssistant Durable Object scoped to their GitHub
login. The Worker handles /auth/* and forwards /chat* to the user-scoped agent
via getAgentByName. The client gates the existing Think UI behind a GitHub
sign-in and adds a sign-out button to the header.

Made-with: Cursor
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Apr 23, 2026

🦋 Changeset detected

Latest commit: 7294a92

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@cloudflare/ai-chat Patch
@cloudflare/think Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Apr 23, 2026

Open in StackBlitz

agents

npm i https://pkg.pr.new/agents@1374

@cloudflare/ai-chat

npm i https://pkg.pr.new/@cloudflare/ai-chat@1374

@cloudflare/codemode

npm i https://pkg.pr.new/@cloudflare/codemode@1374

hono-agents

npm i https://pkg.pr.new/hono-agents@1374

@cloudflare/shell

npm i https://pkg.pr.new/@cloudflare/shell@1374

@cloudflare/think

npm i https://pkg.pr.new/@cloudflare/think@1374

@cloudflare/voice

npm i https://pkg.pr.new/@cloudflare/voice@1374

@cloudflare/worker-bundler

npm i https://pkg.pr.new/@cloudflare/worker-bundler@1374

commit: 18e9202

@threepointone threepointone marked this pull request as ready for review April 23, 2026 18:10
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 potential issues.

View 5 additional findings in Devin Review.

Open in Devin Review

Comment thread examples/assistant/src/server.ts Outdated
Comment thread examples/assistant/env.d.ts
Previously, Think unconditionally sent `cf_agent_chat_messages` from
`onConnect` on every WebSocket connection. When a client refreshed
mid-stream, that broadcast arrived in the same connect sequence as
`cf_agent_stream_resuming` and overwrote the in-progress assistant
message the client was about to rebuild from the resumed stream. The
assistant reply would stay hidden until the server finished the turn
and re-broadcast the persisted history.

Only broadcast `CHAT_MESSAGES` on connect when there is no active
resumable stream. During an active stream the resume flow is the
authoritative source of state — STREAM_RESUMING triggers chunk replay,
and the final state broadcast happens when the turn completes. This
matches the behavior that AIChatAgent already had.

Promote the internal `_resumableStream` field to `protected` so
framework subclasses and focused tests can coordinate around the
resume lifecycle.

Regression test in `onconnect-broadcast.test.ts` covers all three
states: no active stream (broadcasts), active stream (suppresses
broadcast + sends STREAM_RESUMING), and post-completion (back to
broadcasting).

Made-with: Cursor
…ansitions

`useAgentChat` was recreating the AI SDK Chat instance and orphaning
any in-flight `resumeStream` whenever `agent.name` transitioned in
place. This broke stream resumption on page refresh for any consumer
using `useAgent({ basePath })` together with
`static options = { sendIdentityOnConnect: true }`: the client starts
with a placeholder name ("default"), then useAgent mutates
`agent.name` to the server-assigned value when the identity frame
arrives. The resulting change to `stableChatIdRef` flipped the `id`
prop passed to `useChat`, and AI SDK's `shouldRecreateChat` check then
replaced the Chat instance.

The useEffect that fires `chatRef.current.resumeStream()` is keyed on
the ref object, not the Chat instance, so it does not re-fire on
recreation. The orphaned Chat kept feeding replayed chunks into its
own state while React subscribed to the new Chat's state — so the
user saw an empty assistant reply after a mid-stream refresh until
the server's final `CF_AGENT_CHAT_MESSAGES` broadcast landed.

Distinguish an in-place `agent.name` mutation from a genuine
"consumer switched chats" event by checking the agent object's
reference identity:

- same `agent` reference, `.name` mutated → not a chat switch; keep
  the Chat instance stable.
- new `agent` reference → chat switch; recompute the stable chat id
  so the AI SDK recreates the Chat against the new conversation.

The one-time URL-arrival upgrade (fallback → resolved key when the
WebSocket handshake completes) still runs.

Two regression tests in `react-tests/use-agent-chat.test.tsx`:
in-place name mutation (Chat stable) and agent-object swap (Chat
recreates). The existing `should refetch initial messages when the
agent name changes` test also continues to pass — it already used
distinct agent object references.

Consumers who want to switch chats without remounting should pass a
different `agent` object (e.g. a new `useAgent({...})` call with a
different `name`). For a completely fresh Chat, the conventional
React pattern — `key={chatId}` on the parent or swapping the subtree
— continues to work.

Made-with: Cursor
Tightens the assistant example after real-world testing:

- wrangler.jsonc: drop `/agents/*` from `run_worker_first`. The
  previous config routed all `/agents/*` requests to the Worker and
  fell back to `routeAgentRequest`, which let any client reach
  `/agents/my-assistant/<login>` unauthenticated and talk directly to
  that user's Durable Object. Narrow the Worker routes to only the
  OAuth endpoints and `/chat*`, and drop the `routeAgentRequest`
  fallback in `src/server.ts`. The only way to reach MyAssistant is
  now the GitHub-authenticated `/chat*` path, which resolves the DO
  instance via `getAgentByName(env.MyAssistant, user.login)`.

- client.tsx: unify the two "loading" UIs. `useAgentChat` Suspense-
  fires during the initial `/get-messages` HTTP fetch, which
  previously flashed an ugly default "Loading..." string between the
  auth-shell spinner and the ready chat. Wrap just the authenticated
  `<Chat>` in a `Suspense` fallback that reuses `LoadingView` with a
  contextual message so the shell stays consistent across the auth-
  check and chat-ready phases.

- server.ts: opt MyAssistant into `sendIdentityOnConnect: true` so
  the client knows which DO it ended up talking to (this is the
  canonical companion to `useAgent({ basePath })`). This is safe now
  that the ai-chat stableChatIdRef fix in the same PR prevents the
  resulting `agent.name` transition from orphaning the Chat instance.

- wip/think-multi-session-assistant-plan.md: clear out an outdated
  "known wart" paragraph that was written during debugging against
  an incorrect hypothesis. Replace with a short historical note now
  that the actual library bugs are fixed.

Made-with: Cursor
@threepointone threepointone changed the title feat(example): add GitHub OAuth to the assistant feat(example): GitHub OAuth for the assistant + resume-stream stability fixes Apr 24, 2026
`addMcpServer(name, url)` with no explicit `callbackPath` falls back
to `${host}/agents/my-assistant/${this.name}/callback`. On this
example, `sendIdentityOnConnect: true` disables the framework's
normal enforcement that would have required `callbackPath`, so this
default slipped through silently.

With the previous commit narrowing `run_worker_first` to `/auth/*`
and `/chat*` (and dropping the `routeAgentRequest` fallback), those
`/agents/...` callback URLs are no longer routed to the Worker —
the SPA asset handler serves `index.html` with a 200, the DO never
sees the OAuth `code`, and the server hangs in `AUTHENTICATING`.

Pass `callbackPath: "chat/mcp-callback"` so the OAuth redirect
follows the same authenticated path as the rest of the app: Worker
authenticates the GitHub cookie, forwards to the user's MyAssistant
DO, and `Agent._onRequest` dispatches the callback via
`mcp.isCallbackRequest()` (which matches on the stored origin +
pathname).

Made-with: Cursor
@threepointone threepointone merged commit a6e22c3 into main Apr 24, 2026
1 check failed
@threepointone threepointone deleted the feat/assistant-github-auth branch April 24, 2026 13:51
@github-actions github-actions Bot mentioned this pull request Apr 24, 2026
threepointone added a commit that referenced this pull request Apr 24, 2026
- Mark "PR 2" as shipped in two parts: 2a (auth + stability, #1374)
  already landed, 2b (parent/child refactor + useChats prototype) is
  the next action.
- Add a short history of what landed in #1374 (auth, security fix,
  Think onConnect fix, ai-chat stableChatIdRef fix, MCP callback
  routing).
- Queue #1378 as a follow-up from PR 2a.
- Refresh "Current status" and "Likely next action" accordingly.

Made-with: Cursor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant