Repository navigation
Conversation
|
Sorry @appflowy, your pull request is larger than the review limit of 150,000 diff characters |
5 of 6 tasks
Add the AppFlowy Web side of connecting Claude, ChatGPT, Cursor, VS Code, Claude Code, Codex and other AI apps through the AppFlowy MCP server. - /oauth/mcp/authorize: consent page with flow-bound sign-in continuation, workspace picker, verified-publisher and local-app notices, permission groups, approve-and-allow for owners and approval requests for members. - /oauth/mcp/setup: public setup page with the server URL and per-app instructions. - Settings -> AI apps: the server URL with Copy (discovered from server-info `mcp_url`, or APPFLOWY_MCP_BASE_URL), a setup prompt, setup cards per app, the member's own connections per device, and for owners workspace approval modes, requests, approved apps and member connections. - When an administrator turns AI apps off, Settings and the consent page say so; members can still see and disconnect their connections. - Anti-framing headers for the consent routes, i18n, doc/MCP.md, and a screenshot gallery under playwright/bdd/evidence/mcp-connect-flow. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`space-y-1.5` was the only use of that class, so it added a new featureless sibling selector and failed the CSS shape budget test. Reuse the existing `space-y-2` spacing instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…atus Settings → AI apps loaded the caller's own connections and their cross-workspace summary only after the member status returned, one round trip later than needed. They now load alongside it. A failed status still shows neither and its Retry reloads them, and the owner lists still wait for the status. Also adds playwright/e2e/mcp/connect-and-create-pages.spec.ts. It copies the MCP server URL from Settings, connects an MCP client with only that URL through the consent page, creates a document and a grid with create-pages, and finds both in Web. doc/MCP.md documents how to run it against a local stack. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The tab sat next to the existing "AI Settings" tab. Its label and heading,
its loading and error text, and every string that sends people to it now
say MCP ("Settings → MCP" on the consent and setup pages). Copy that
describes AI apps themselves keeps that wording, and the ?setting=AI_APPS
deep link is unchanged.
Screenshots 01, 03, 04, 06, 07 and 09 are recaptured to show the new name;
02, 05 and 08 do not show it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The tab rename left the operator comments in deploy.env and the compose
example pointing at "Settings > AI apps", and doc/MCP.md at the "AI apps
menu item". deploy.env also said /oauth/mcp/* "stay hidden" without an
MCP URL, while those pages render a not-configured view.
- deploy.env and the compose example: name Settings → MCP, say Settings
hides the tab and /oauth/mcp/* show not configured, and use the
same-domain example origin the bundled deployments serve.
- doc/MCP.md: the MCP menu item and tab; AppFlowy Cloud's deploy.env
(not this repo's) sets APPFLOWY_MCP_BASE_URL=${APPFLOWY_BASE_URL};
approve takes a client_id or an app:<app_id> approval_key, as the
Settings section says; Desktop UI is no longer "not in v1"
(AppFlowy-Premium #1555), only mobile is.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…strict The two MCP Playwright specs each carried their own loopback callback server, PKCE and token exchange, and the copies had drifted: the connect-and-create copy never cleared its timeout, kept its listen error listener and ignored close errors. oauth-consent also read the well-known documents leniently, so a 404 or 500 left it running without `resource`, skipping the iss-equals-issuer check (RFC 9207), and only checking the narrowed scope when the token answer had one. - playwright/support/mcp-oauth-helpers.ts holds the callback server (the oauth-consent version, with waitForCallback(timeoutMs)), pkcePair, a strict getJson and exchangeAuthorizationCode; both specs use it and keep their own endpoint source. - oauth-consent requires both well-known documents and string `resource` and `issuer`, always sends `resource`, and asserts `iss` and the read-only scope unconditionally. - A Jest test pins the harness (callback query, 404, timeout, close errors, S256, strict reads, token form), since CI runs no MCP spec. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…st IDs
oauth-consent found the client's row by name but then clicked the
panel-wide mcp-my-disconnect-0, and found the confirmation through the
English "Disconnect {name}?" title, `.last()` and the "Disconnect"
button label. A copy edit broke it, and a second connection sorted
first would have disconnected the wrong app.
It now clicks the disconnect button inside the matched row and confirms
through the mcp-disconnect-dialog and mcp-confirm-disconnect test IDs
the panel already exposes.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The hosts add X-Frame-Options and frame-ancestors 'none' by a case-sensitive, undecoded /oauth/ prefix, but React Router matches routes case-insensitively and after percent-decoding each segment. /OAuth/mcp/... (every host) and /%6Fauth/mcp/... (the Bun SSR server) therefore rendered the consent page without the headers, leaving the in-page frame check as the only defence. The three header copies (deploy/routes.ts, nginx.conf, vercel.json) were also only tested in the Bun router. Make the two MCP routes caseSensitive, so case variants render NotFound on every host. On Bun, match frame-deny paths after decoding each segment the way React Router does, and serve them as the app shell with the headers. Move FRAME_DENY_HEADERS next to FRAME_DENY_PATHS and add a parity test that reads vercel.json and the nginx /oauth/ block (headers, and the rewrite that keeps them on the SPA shell). The keep-in-step comments now name all three copies. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…debar checks connect-and-create-pages checked that the turned-off notice was absent as soon as the panel container mounted, while its body was still the loading spinner, so the check always passed. With AI apps off the run then failed 30 s later on the endpoint field with a generic timeout. The "both in the sidebar" check matched the document's name in its own title or breadcrumb, and the initialize and tools/list replies were read without a guard, so a failed call surfaced as a TypeError. - Wait for mcp-panel-loading to go before asserting mcp-panel-disabled is absent, and say on the hidden-tab failure that Web must know the MCP origin. - Find the created pages by their sidebar rows (page-<view_id>). - Assert the tools/list status and that both replies carry a result before reading them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…erun The evidence README said the screenshots came from end-to-end Playwright runs, and the PR's test plan reports a live 16/16 run, but the script behind both is not in the repository. The committed specs cover only owner consent and disconnect, Cancel, and copy URL → connect → create pages, so the off state, member approval and turning AI apps off and on cannot be rerun, and none of the screenshots can be regenerated. The README now says the run came from an uncommitted local script, names the scenarios the committed specs rerun and the ones they do not, and notes that a future script should switch AI apps through the admin policy endpoint and restore the saved value. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
isLoopbackHost accepted any *.localhost, all of 127.0.0.0/8 and a trailing dot, while the server's is_loopback_host (which sets a client's loopback_only and decides which http redirects DCR accepts) accepts only localhost, 127.0.0.1, ::1 and [::1], and the two test suites pinned opposite answers. For an https DCR redirect to dev.localhost or 127.0.0.2, the consent page said the app runs on this computer and skipped the "I recognize <host>" tick, while Settings showed the same client as a remote host. Use the server's exact list. https://dev.localhost and https://127.0.0.2 redirects now ask for the host acknowledgment, matching Settings; real loopback clients are unaffected, since the server refuses any other http loopback host. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
"Trust app_id and app_name only when verified_app is set" was restated by hand at about seven sites in two forms: a truthy verified_app with a raw app_id (mcpApprovalKey, mcpApprovalCovers, the access-request scope line) and verified_app === true with a trimmed app_id (setupApps, mcpRemainingAppApproval). McpClientIdentity also rebuilt the naming chain instead of sharing getMcpClientDisplayName's, and McpClientAvatar's prop type left out the two fields its initial depends on. Add trustedCatalogAppId and getMcpClientOwnName next to getMcpClientDisplayName (which now builds on them) and use them for the approval key, approval coverage, the remaining app approval, gallery grouping and McpClientIdentity's name. The access-request scope line now shows exactly when Approve sends an app key. A table-driven test runs one client of each kind through every helper; it fails on the old untrimmed approval key. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
AppFlowy Desktop ports setup.ts, setupApps.ts and setupPrompt.ts byte for byte and checks itself against a golden snapshot of Web output, but the Web side never recorded that contract: no fixture or generator, headers that do not mention the port, and a prompt version said to trace "the Web build that wrote it" although Desktop writes the same v1 text. Rewording a prompt sentence or changing a builder passed setupPrompt.test.ts's toContain checks, leaving Desktop silently stale. Add setupGolden.test.ts, which builds every snippet, install link and both prompts for Desktop's four server URLs in the exact shape Desktop's web_setup_golden.json has (plus promptVersion) and compares them with a checked-in __golden__/setup-texts.json; UPDATE_GOLDEN=1 regenerates it for Desktop to copy. The generated cases equal Desktop's current golden. Name the Desktop port in the three module headers and say the prompt version identifies the text both clients write. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
buildVsCodeCliCommand wraps its JSON in POSIX single quotes, which cmd passes through and PowerShell forwards with the inner double quotes stripped, so --add-mcp receives broken JSON on Windows. The setup guide showed it under a bare "Or run:" and the setup prompt told agents to run it on any platform. Label the guide's CLI line "Or run on macOS or Linux:" (Windows users have the Install in VS Code link above it), and have the setup prompt give the command for bash or zsh and VS Code's own .vscode/mcp.json entry for PowerShell or cmd. The prompt's instructions changed, so its version is now 2 and the Desktop golden is regenerated; Desktop must port the new line and version. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The snippets and the prompt take the server name from MCP_SERVER_NAME,
which AI apps list under that exact, case-sensitive name, but six setup
guide strings spelled it as bare prose ("choose appflowy", "find appflowy",
"next to appflowy"). A translator could capitalize or translate it and
send users looking for an entry that does not exist; the other four
strings already quote it in backticks, which RichText renders as code.
Quote the six occurrences, and add a test that fails when any string under
mcp.setup names MCP_SERVER_NAME outside backticks.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The global runtime-config mock, which jest.config.cjs maps for every suite, set APPFLOWY_MCP_BASE_URL. No MCP suite used that default: each mocks runtime-config itself or passes the endpoint. It only reached unrelated Settings suites, which then saw MCP as configured through the override, rendered the MCP tab and, for owner fixtures, started the pending-request load. A new MCP test that forgot its own mock would also have exercised the override instead of server-info discovery. Drop the default so an unset override means server-info, as in a build without it, and pin that with a test of the mock. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
doc/MCP.md said connect-and-create-pages "needs no APPFLOWY_MCP_BASE_URL of its own", and its single-origin command on localhost:18180 neither set nor cleared it. But playwright.config.ts fills unset variables from .env, which DEVELOPMENT_GUIDE copies from dev.env with APPFLOWY_MCP_BASE_URL=http://localhost:8002; the sign-in helper then injects that into the Web under test, and a Vite-served Web also bakes it in. Settings showed http://localhost:8002/mcp, so the spec connected to a port the bundled stack does not publish, or to another MCP service. Set APPFLOWY_MCP_BASE_URL=http://localhost:18180 in the command (the runtime injection wins over the .env and build values) and say why the stray value matters. Also make the sign-in helper's comment name the build-time fallback, as installRuntimeTestConfig's already does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The runtime-config key list was kept by hand in two Playwright injectors and a Storybook copy of the Window type, and APPFLOWY_MCP_BASE_URL had to be added to each. enableTestMode (conditional spread) and installRuntimeTestConfig (ternary) built the same object from process.env in different shapes. The Storybook decorators re-declared Window.__APP_CONFIG__, which must stay type-identical to src/utils/runtime-config.ts, but pnpm type-check excludes .storybook, so drift there never failed CI. Add runtimeConfigFromEnv to playwright/support/test-config.ts: the three service URLs plus the optional MCP override, or the first service URL left unset. Both injectors use it and keep their own gate (skip silently, or throw naming the key). Drop the Storybook re-declaration; the decorators already import runtime-config, whose global declaration applies. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The refresh-token leak test called updateMcpWorkspaceSettings(WS, false),
the boolean signature the function had before approval modes, so it sent
{approval_mode: false}, a body the server rejects. clientTrust.test typed
its fixtures with a nullable verified_app that mcpApprovalKey does not
accept. Neither showed up: pnpm type-check excludes test files and
ts-jest compiles them with isolatedModules, which reports no type errors.
Pass 'approved_only', and type the fixtures' verified_app as the boolean
the MCP service sends. A tsc run over the MCP test files now reports
nothing.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The ineligible branch of Settings -> MCP rendered Your connections only when the list had rows. useMyMcpConnections returns an empty list until a load succeeds, so a failed first load (a brief 503 or a throttle) rendered nothing: no error and no Retry, only the ineligible notice, and no way to reach a connection the account must still be able to revoke. Show the section from the first failed load on, so its error and Retry appear and the section stays, loading, while the Retry runs. Accounts whose list loads empty still see no section. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The server lets a workspace owner whom the MCP email allowlist excludes list and withdraw access (lists, disconnects, revokes, dismissals and tightening skip the email gate; only approving and loosening check it). Settings -> MCP shows such an owner only the ineligible notice and their own connections, the same v1 scope as an owner while AI apps are off, but unlike that case the decision was not written down, and the menu badge kept counting requests the panel never showed. The badge loader's comment and doc/MCP.md also implied owner lists refuse these owners with email_not_allowed. Clear the badge when the member status says the account is ineligible, as already done while AI apps are off. Record the excluded owner next to the off state under "Not in v1", and say that only approving and loosening check the email gate. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The McpService facade re-exported getMcpBaseUrl, isMcpApprovalMode, isMcpErrorReason and isMcpConfigured, which no app code imports, and resetMcpConfigurationForTests, a test hook that let app code drop the confirmed configuration mid-session. isMcpConfigured had no caller at all, yet doc/MCP.md offered it as a "known now" check that is false while server-info loads. Delete isMcpConfigured (its assertions now check getMcpConfiguration or the override), drop the five re-exports, and have the tests import the reset from the HTTP module that owns the state, like the repo's other *ForTests hooks. The reset also forgets which invalid URLs were warned about, so warning-count tests do not depend on test order. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
When GET .../settings failed, Workspace access rendered two role="alert" boxes with two Retry buttons for that one request: one in place of the approval policy and one in Approved apps. Screen readers announced the failure twice. SectionError and PanelFailure also took a `loading` prop for a Retry spinner that could never show: useMcpResource clears the failure in the same batch that starts a reload, and records a failure in the same batch that ends one, so no render has failed && loading. Every caller passed a value that was false whenever the box was on screen. Keep the alert in the policy slot and leave Approved apps out while the settings load has failed; Retry brings both back. Remove the dead `loading` prop from both components and their callers, and say in their doc comments what shows while a Retry runs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Your connections is the control Settings -> MCP keeps usable while AI apps are off, yet no test made listMyMcpConnections or disconnectMyMcpConnection fail. A refactor that rendered a failed list as "No apps connected", or closed the disconnect confirmation on failure, would have told users their apps were gone while they still held tokens, with every test green. Add McpPanel tests for a failed first load (with AI apps on and off), a failed reload after a disconnect, and a failed disconnect: the list shows its error and Retry instead of the empty state, Retry recovers it, and a failed disconnect toasts and keeps its confirmation and the row. They fail without McpMyConnections' `failed` branch or its `if (success)` guard. The settings-section failure is covered by the single-alert test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…resh isMcpServiceUnavailable counted every request without a response as the MCP service being unavailable, including session_refresh_failed, which the client raises when GoTrue cannot refresh the AppFlowy session before the MCP service is ever asked. A brief GoTrue outage therefore made Settings -> MCP say the server has no MCP service configured. It now counts only network_error, timeout and not_configured; a failed refresh shows the panel's error view with the temporarily-unavailable text. Also delete what nothing in the app used: isRetryableMcpError, whose rule already disagreed with the consent page's own retry set, and McpApiError.retryAfter with its Retry-After parsing, which no Retry control read. doc/MCP.md no longer documents them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The ServerInfo.mcp_url comment said the value comes from the server's APPFLOWY_MCP_BASE_URL and is absent only without an MCP service or on older servers. Cloud also falls back to its APPFLOWY_BASE_URL, and advertises the origin only while the MCP policy has AI apps turned on, so the field is also absent while they are off or before the policy loads: the case that hides Settings -> MCP in a Web without the override. Say so, matching Cloud's ServerInfo DTO. The mcp-api module header and doc/MCP.md also claimed the service answers 403 for every refusal; it refuses with 400, 403, 404, 409, 429 and 503, each with a reason. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
normalizeMcpBaseUrl checked only that APPFLOWY_MCP_BASE_URL (and server-info's mcp_url) was an http(s) URL, although doc/MCP.md and Cloud's parse_mcp_public_origin define it as a bare origin. An operator who set the URL users paste, https://host/mcp, got no warning: Settings called https://host/mcp/api/mcp/..., got a 404 and showed "unavailable", and /oauth/mcp/setup told users to paste https://host/mcp/mcp. Apply Cloud's rule: reject a path, query, fragment or credentials, warn once with Cloud's guidance and without echoing the value, which may hold credentials. The ignored override then falls back to server-info. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The Web's APPFLOWY_MCP_BASE_URL wins over the mcp_url Cloud advertises, without comparing them. Deployments set it wherever the MCP service runs, because Cloud leaves mcp_url out while AI apps are off. So after an operator moved the MCP service and updated only Cloud and MCP, Settings and /oauth/mcp/setup kept showing the old origin and consent calls went there, with nothing reporting the mismatch. Keep the override (it is what lets Settings show the turned-off state), but when server-info is loaded and advertises a different origin, log a console warning naming both, once per pair. A missing mcp_url is no mismatch. doc/MCP.md no longer says the URL is configured once on the server, and the compose example says to keep both values equal. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… app
The consent view's Cancel and the error views' Cancel called
window.location.assign with the deny redirect and returned 'leave', so
run() never cleared busy. That assumed the navigation unloads the page,
but a private-use redirect (raycast://, which a client ID metadata
document may list) opens the app and keeps the tab: the Cancel button
spun forever and every other control stayed disabled, with nothing to
explain or retry if the user dismissed the "Open app?" prompt. Allow
already rendered its done view first and navigated from an effect; the
deny paths had no matching terminal view.
A successful deny now sets a 'cancelled' outcome. The outcome effect
navigates for it as for 'done' (once, behind navigatedRef), and the flow
renders McpConsentCancelledView: "Request cancelled. You can close this
tab and return to {name}." With no task left returning 'leave', run()
always clears busy. The page test that pinned the disabled buttons after
Cancel now expects the cancelled view; new tests cover a private-use deny
from the consent view and from an error view (one navigation, nothing
left aria-busy, no buttons).
en.json (settings lane) gains only mcp.consent.cancelledTitle and
cancelledDescription.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
cancelFromError set a hook-wide cancelRefused flag for every failure that was not an abort, including a network error, a timeout, a 429 or a 5xx, and nothing reset it. After one transient failure the button vanished without a message from that error view and from every later one (an Allow refused with invalid_scope after a successful retry, say), so the user lost the only way to tell the AI app to stop waiting. The consent view's Cancel calls the same endpoint but already told transient failures from refusals, so the two paths had drifted. classifyDenyFailure now decides for both paths: a retryable reason is transient, anything else a refusal of the request itself. The error-view path keeps one CancelFailure state: a refusal holds for the rest of the flow and hides Cancel as before; a transient failure keeps the button and shows the reason's message in a note (mcp-consent-cancel-error) on McpConsentErrorView, cleared by the next action, Try again or going back. doc/MCP.md (settings lane) says so in the Cancel paragraph, which also mentions the Request cancelled view from the previous commit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Every rejection on the consent page was mapped to a ConsentErrorReason and dropped. The MCP client uses fetch, not the shared axios layer, so it skipped that layer's logging too, and responseError discarded a reason this Web does not know. A 400 with a new reason, a Cloud API outage and a TypeError in the flow all read as "Something went wrong" with nothing in the console, although McpApiError.description is documented as being for logs. The MCP client now logs each failed request once with Log.warn: method, path without its query, status, error code, the server's raw reason (an unknown one included) and description. That covers error answers, unreadable success answers, an unsafe redirect and requests that got no answer, for the consent page and Settings alike; caller aborts are not logged, and neither is the query or any body, so no client_id, state, code challenge, redirect URL or token. useMcpConsent logs a failure that is not an McpApiError (a Cloud call, a bug on the page) with Log.error so it keeps its stack. The MCP client tests quiet Log.warn file-wide and assert it in their own block. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…build's
getConfigValue used the injected runtime value only when it was
non-empty, else import.meta.env, which Vite fills from the build
checkout's .env. Both Docker entrypoints inject an unset
APPFLOWY_MCP_BASE_URL as '' and log that the origin comes from
server-info, but dev.env (copied to .env for pnpm dev) sets it to
http://localhost:8002 and .dockerignore does not exclude .env. An image
built from such a checkout and deployed without the variable sent the
consent page, the setup page and Settings to localhost:8002 while its
log claimed server-info discovery. The three required keys never hit
this because the entrypoints refuse to start without them.
The precedence now lives in resolveConfigValue (runtime-config-value.ts,
free of import.meta so Jest can test it): for APPFLOWY_MCP_BASE_URL an
injected value wins even when empty and reads as unset; a key the
runtime config leaves out (pnpm dev, Playwright, Vercel) still falls back
to the build, and so does an unsubstituted ${KEY} template or an empty
required key, as before.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2ed3e9f made the fields the MCP service always sends required (enabled, approved_apps, approval_key, verified_app, identity_verified, family_id, last_used_at), but only mcp-api.test.ts got matching fixtures. Jest only transpiles test files and pnpm type-check excludes them, so 51 type errors in eight MCP suites went unseen, and some fixtures modelled shapes no server sends: requests without approval_key, settings with the dropped allow_unapproved_clients flag, an error reason 'storage_unavailable' outside McpErrorReason, and summaries without the catalog flags. The fixtures now carry every field the server serializes, with a member status whose approval_mode follows requires_approval as the server derives it; the cases that pinned the absent-field fallbacks are gone. tsconfig.mcp-tests.json covers the MCP test files this PR added or changed, pnpm type-check:mcp-tests runs it, and Web-CI runs that after the lint check, so a DTO change that leaves a fixture behind fails CI. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2ed3e9f made approval_key, approved_apps, enabled, verified_app and identity_verified required, but the panel still read them as possibly missing: format.ts fell back from approval_key to the client ID in three places, McpPanel guarded status.approved_apps, mcpApprovalKey preferred an approval_key when one was passed, and consentStatusBlocker, mcpManagesApprovals and mcpModeAdmitsWithoutApproval compared the flags with === true/false so an absent value read as a default. Those fallbacks could only fire for shapes no server sends, and would hide a server change instead of failing a test. Approvals and access requests now have their approval_key read as sent (the request rows, dismiss and the optimistic filters included), and mcpApprovalKey only derives the key of what carries none, such as a connection. The flags are read as the booleans they are. Also finishes WEB8: useMcpGovernance.approve takes a `target` (a client ID or an app:<app_id> key) and clears the request filed under that key or registered under that client ID, instead of a client_id comparison that never matched an app key. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Since 2ed3e9f an approval_mode this Web does not know (a newer server's) reads as undefined instead of being coerced to a known one. Workspace access treated that like settings still loading: the owner saw a spinner where the policy choices go, forever, with no message, and chooseMode ignored clicks. Loaded settings without a known policy now show a note that this workspace uses a policy this version of AppFlowy does not know and that updating AppFlowy shows or changes it. The policy choices stay hidden: changing an unknown policy would need to know how strict it is. The approved apps, requests and member connections below still load and work. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
McpPanel.test's beforeEach called jest.clearAllMocks(), which clears calls but keeps answers queued with mockResolvedValueOnce, mockRejectedValueOnce and mockReturnValueOnce. A test that failed before using its queued answers handed them to the next test, so one failure showed up as several unrelated ones (reverting WEB31 failed three member tests as well). beforeEach now calls jest.resetAllMocks(), which drops queued answers too, and then installs every default answer as before. Checked with a temporary test that queued two rejections and failed at once: with clearAllMocks the next test failed as well, with resetAllMocks only the temporary one did. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
cd06071 made the consent route caseSensitive, so /OAuth/mcp/authorize renders NotFound. The sign-in continuation still matched its path through normalizeRoutePathname, which lower-cases for the case-insensitive /login and /auth/callback routes. A login link whose redirectTo was a case variant started an MCP consent continuation (the 10-minute window, the expired view) for a destination that then showed NotFound. normalizeRoutePathname takes a caseSensitive option, and the consent continuation uses it: segments are still decoded and a trailing slash dropped as React Router does, but case is kept, so the continuation matches exactly the paths that render the consent page. Tests cover two case variants (not consent) and /%6Fauth/... and a trailing slash (consent, as React Router renders them). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The /oauth/ anti-framing headers have three copies: Bun's deploy/app-paths.ts, the nginx-only image's location block and vercel.json. deploy/frame-deny.test.ts checks that their values agree, but nothing fetched a page from a built image, so an nginx edit such as a try_files fallback (which jumps to `location /` and drops the headers) or a location without the trailing slash would pass CI. docker-runtime-test now requests /oauth/mcp/authorize from both images and requires a 200, X-Frame-Options: DENY, Content-Security-Policy: frame-ancestors 'none' and the app shell with __APP_CONFIG__, and requires /app (and, in the nginx-only image, /oauthors/x) to carry neither header. Run against nginx with docker/nginx.conf it passes; with the X-Frame-Options line removed, a try_files fallback, or `location ^~ /oauth` it fails. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
McpConsentDetails passed McpClientIdentity a made-up client summary to get the publisher row it wanted: client_name and app_name null so the host leads, loopback_only false so no local badge, and verified_app rewritten to verified_app && identity_verified so a catalog app on the user's computer skipped the native-app badge. The row depended on how McpClientIdentity orders its branches, and would change quietly if that order did. McpClientIdentity takes variant='publisher': the host is the title, a proven identity is the only verified app, and the native-app and local badges stay off (where the app runs belongs to the redirect row). McpConsentDetails now passes the real client. The consent page renders as before; a McpClientIdentity test covers the variant for a native catalog app (CIMD and DCR) and a proven web app. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
mcpErrorText worked out the reason of an error without one on its own: network_error and timeout read as temporarily unavailable, everything else as generic. The consent page's toConsentErrorReason also maps not_configured to its own message and a 401 without a reason to invalid_session, so the same failure read "Something went wrong" in Settings and something specific on the consent page, and a new fallback had two places to change. mcpErrorText now takes the reason from toConsentErrorReason and keeps only its Settings wording for not_workspace_owner, unknown_client, invalid_request and the consent-only reasons. A table test runs network errors, timeouts, not_configured, a failed session refresh, a 401 and a 404 without a reason, an unreadable answer, a throttle and mcp_disabled through both and requires the same message. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
send() aborts a request after REQUEST_TIMEOUT_MS (30 s) and reports
{status: 0, error: 'timeout'}, which Settings shows as "unavailable" with
Retry and the consent page as temporarily unavailable. No test drove it:
a timer cleared too early, or a lost timedOut flag, would leave the
consent page waiting until the browser's own limit, or report a timeout
as a network error, with every test green.
A fake-timer test sends a member-status request whose fetch only settles
when aborted, checks it is still pending at 29.999 s, and requires the
timeout error, read as the service being unavailable, at 30 s. It fails
(hangs past Jest's limit) when the timer no longer aborts the request.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
useMcpConsent drops a member status that answers after the workspace changed (the abort check in the status effect), runs one decision at a time (busyRef in run) and ignores a workspace switch while a decision is in flight. Every consent test answered at once and clicked once per render, so removing any of these left the suite green: workspace A's late status could leave B's view loading forever, and a double click could send two authorization codes or a Cancel after Allow. Two page tests answer by hand and ignore the abort. One switches from the owner to the member workspace, answers the member status (Request approval), then the owner's late (Allow), and requires the member view to stay. The other clicks Allow twice, Cancel and another workspace before a re-render, and requires one code request for the owner workspace, no deny and no switch. Removing each guard fails one of them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
summarizeConnectedApps counted each app's connections into McpConnectedApp.connectionCount, which only tests read (the Desktop port has no such field), and scopeLevel accepted a list of scopes although every caller passes the granted scope string; McpScopeBadge already takes only a string. Its comment also said Settings' "N apps across M workspaces" matches "Connected apps (N)", but the overview covers every workspace and the guide only the current one. summarizeConnectedApps keeps the first connection of each app and nothing else, scopeLevel takes a string, and the comment says the two counts group apps the same way over different workspaces. The tests drop their count and list cases; grouping, order and the scope levels are tested as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Both Dockerfiles copy the checkout and run the Vite build, which inlines every APPFLOWY_* variable from .env files. .dockerignore left out only .env.local, .env.development and .env.test, so a checkout with dev.env copied to .env (as the development guide says) baked its values, such as APPFLOWY_MCP_BASE_URL=http://localhost:8002, into the image. a0e2354 stopped an empty runtime value from falling back to the baked one for that key; this closes the gap for every key at the source. .dockerignore now excludes .env and .env.*. No committed file matches and the Dockerfiles need neither. Checked with a throwaway build of a context holding .env, .env.local and .env.production: none reached the image, deploy.env and src/ did. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
67a0223 added a console warning when the Web's APPFLOWY_MCP_BASE_URL and the mcp_url Cloud advertises differ, and told the compose example to keep both equal. deploy.env, the other place operators read about the override, still only said to set it wherever the MCP service runs. It now says the same as the compose example: keep it equal to AppFlowy Cloud's APPFLOWY_MCP_BASE_URL, because it wins over server-info, and the browser console warns when they differ. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
doc/MCP.md's Testing section is where maintainers look for how the MCP work is checked, but it named neither of the two CI checks added since: pnpm type-check:mcp-tests (tsconfig.mcp-tests.json), which catches fixtures that drift from the wire types, and the docker-runtime-test step that fetches /oauth/mcp/authorize from both images and checks its anti-framing headers. It now names both. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… does
"Connected apps (N)" sorted the apps without a gallery card with
localeCompare and no locale, so the order followed the browser's
language (a Swedish browser puts Ö after Z) and ICU's tie rules
("raycast" before "Raycast", "Straße" before "Strata"). Desktop
(fd175a28e2) dropped its imitation of that collation and now compares
names folded with normalizeDashboardText, then as written, both by
UTF-16 code unit: the rule dashboard labels already sort by on both
clients (compareChartLabels). The two clients could list the same apps
in different orders.
The Web now uses the same rule. Gallery rank and unnamed-last are
unchanged. The tests take Desktop's vectors: accents and case ignored
with an as-written tie-break, ß kept a letter of its own, and a
Cyrillic list; the first two fail with localeCompare. The setup golden
holds no ordering and is unaffected.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The pending-request badge store dropped a menu load's answer only when the panel had published since the load started. Two loads in flight were not ordered: the one that answered first published, which made the other look stale, so an older load could win over a newer one. The panel also cleared the badge by its own rule (AI apps off, or the email gate excluding the caller), not the mcpManagesApprovals rule the menu load and the owner section use, so a caller the MCP service now lists as a member (after an owner action answered not_workspace_owner) kept the old count next to a panel without requests. As Desktop does since f010e3fc91, the store now owns the one load routine and a write sequence per workspace: every publish and every load started advances it, and a load writes only while it is still the latest write. The panel clears the badge whenever its status is known and the caller does not manage AI apps there. Tests: a newer load wins when the older one answers first or last; the menu shows no badge when the MCP service lists the caller as a member; the panel drops the badge when an owner action finds the caller is no longer an owner. The first and last fail without the fix. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
useMcpResource.update kept the failure of an earlier load next to an action's fresh data. Most actions reload right after, which clears it; a policy change without apps kept approved does not reload the settings. So when the settings reload an approval started failed while a policy change was in flight, the PUT's answer stored the new policy and the section still showed "couldn't load" with Retry, as if the change had failed. An action's answer is newer than any load, so update() now clears the failure too, as Desktop's McpLoad.updated does since 1393bd2f9c. A useMcpRequest test fails a reload and then updates the data; it fails without the fix. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
React Router decodes each path segment before it matches, so /%6Fauth/mcp/authorize rendered the consent page. Bun and nginx send the anti-framing headers for that spelling, but vercel.json matches source '/oauth/(.*)' and nobody checked whether Vercel applies it to an encoded path, leaving the in-page frame check as the only defence there. The sign-in continuation also treated the encoded spelling as a consent request. isMcpConsentPathname (consentUrl.ts) is now the one rule: the undecoded pathname is exactly /oauth/mcp/authorize, with the trailing slashes the router ignores. The consent page reads the router's location (undecoded, from window.location under BrowserRouter) and renders NotFound for any other spelling before it loads anything; sign_in.ts matches its consent continuation with the same rule, so normalizeRoutePathname loses its caseSensitive option. The MCP service always redirects to the canonical path, so real flows are unaffected. Bun and nginx keep their headers on encoded paths (the setup page still renders there); the app-paths comment no longer says the consent page does. Tests: three encoded spellings render NotFound without loading or starting sign-in, and the trailing slash still shows the request; sign-in no longer treats /%6Fauth/... or /oauth/mcp/%61uthorize as consent. The NotFound and sign-in tests fail without the fix. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
fee3ed9 gave "Connected apps (N)" the dashboard label order, but did it by copying codeUnitCompare and the body of compareChartLabels into setupApps.ts rather than importing group-sort.ts, whose imports (chart settings, group keys, dayjs, i18n) the setup module should not pull in. Two copies of one parity rule can drift apart without either test noticing. normalize-text.ts, which already owns the fold, now exports the order too: compareCodeUnits and compareDashboardText (folded, then as written, both by UTF-16 code unit). group-sort.ts keeps compareChartLabels, the name the parity fixture and Desktop use, as an alias of it and uses compareCodeUnits for its cached and tie-break comparisons; setupApps.ts calls compareDashboardText. Behaviour is unchanged; the chart parity tests and the setupApps vectors still pass, and a new test runs the labelCompare fixture against compareDashboardText where it lives. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The nine screenshots of the AI apps (MCP) connect flow came from a local script that was never committed (review finding WEB42). capture.spec.ts now writes them through its own capture.config.ts, so the gallery can be reproduced. No other Playwright config reaches the evidence directory, so neither CI nor the e2e suites run it. The script signs up a fresh owner and member, connects two loopback MCP clients through the OAuth flow (mcp-oauth-helpers), creates a document and a grid through create-pages, and changes the workspace approval policy as the owner in Settings -> MCP. It switches AI apps through PUT /api/admin/mcp/policy with expected_revision. A hosted build answers 403 mcp_not_entitled; there it writes the same af_system_config row through docker exec psql. The starting value is restored at the end, also on failure. Captured on 2026-11-03 with headless Chromium at 1360x900 against the local single-origin stack (Cloud and MCP from AppFlowy-Cloud-Premium #1521 at 97d942b1a, Web from this branch). 01, 02 and 06 are byte-identical to the previous capture. 03 now shows the whole consent card with Approve and allow, written by a second run with MCP_CAPTURE_FRAMES=03. README.md records the provenance and the rerun command. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Rerun capture.spec.ts once against the rebased branch (b7a6772, on main 7558479) and the same local stack (Cloud and MCP at 97d942b1a). Since #706, Settings no longer lists AI Settings (01, 04, 07, 09) and the page header has no Present button (05). 02, 06 and 08 came out byte-identical and are unchanged. The README now names this run and notes that create-pages can place the AI app's pages in General or Shared. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
appflowy
force-pushed
the
feat/mcp-connect-flow
branch
from
November 3, 2026 10:49
8ae5496 to
0bcfc28
Compare
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Bring the UI this branch adds (Settings -> MCP, the consent page and its state views, the setup guide and the public setup page) in line with the AppFlowy Design-System Figma file. Only these screens change; the shared components in src/components/ui and src/components/_shared, the tokens and MUI are untouched, and no text, behaviour, test id or aria attribute moves. - Notices follow Message Box: borderless Fill/*-light boxes, radius 12, 16 px padding, a 20 px coloured status icon 12 px from Text/primary text, and Filled S actions in the type's thick fill (Retry is red on errors). info_colored.svg adds the missing filled info icon, drawn like error.svg. - Badges follow Status tag: 22 px pills in Body_S with the status *-on-fill text pairs; scope, verified and local-app badges lose borders. - Radios and checkboxes are drawn to the Figma 14 px box and circle in a 20 px target, with hover, focus-visible, disabled and checked states. Approval-policy and workspace cards follow Toggle Button (radius 8, Border/theme-thick when chosen, Border/primary-hover on hover). - The consent page and its state views use the Web landing layout: a 400 px column without a card, 64 px avatars and illustrations, H3 bold titles, the primary action above Cancel, and the account line below. - Avatars use the Badge colour-20 pair for initials and the Workspace radii; setup monograms use Badge light-2/thick-3 pairs. - Captions are Caption_S (12/18), icons 20 px, row actions M 32, and the request row orders Dismiss before Approve as dialog footers do. - The setup cards need at least 14rem, so the 680 px setup page shows two columns and descriptions keep to two lines; Settings keeps three. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Rerun capture.spec.ts once against the restyled branch and the same local single-origin stack. All nine frames change; README.md records the run, the server build it used and 03's new height (1360x1259). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Settings → MCP now has two tabs: "Connect AI apps" (setup guide) and "Manage AI apps" (your connections plus workspace access). Owners see a pending-request count on the Manage tab. - The Connect tab ends with a link to Settings → Connections for Google Drive and Google Calendar. - The setup gallery keeps Claude, ChatGPT, Claude Code, Codex, VS Code and Other app. Cursor, Gemini CLI and Windsurf cards, the "Connected apps" strip, the setup-prompt row and "Having trouble?" are removed. - Connection rows put the name and badges on one line, host and device on the next, then the connected and last-used times. - The OAuth consent page scrolls when it is taller than the window. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
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.
Summary
The AppFlowy Web side of connecting AI apps (Claude, ChatGPT, Cursor, VS Code, Claude Code, Codex and others) to AppFlowy through the AppFlowy MCP server. The server side is in AppFlowy-IO/AppFlowy-Cloud-Premium#1521.
/oauth/mcp/authorize):/oauth/mcp/setup): public, signed out. It shows the server URL and instructions for each AI app.?setting=AI_APPSstill opens it):mcp_urlor set withAPPFLOWY_MCP_BASE_URL./oauth.docker/nginx.confandvercel.json.APPFLOWY_MCP_BASE_URLin the runtime config, with a CI injection check.doc/MCP.md.Screenshots
Captured by the committed
playwright/bdd/evidence/mcp-connect-flow/capture.spec.tsin one run on 2026-11-04, after the design-system restyle below. It ran against Cloud and MCP built from the server PR on 2026-11-03 (right afterd069a1111), served behind its bundlednginx.confon a single origin, and Web from this branch at42f4e9bd. The script signs up fresh users, connects real MCP clients through OAuth and restores the AI apps setting it changes. It is not part of CI. The README beside it explains how to rerun it.Changes since the first review
These came from the appflowy-code-smell review of every branch of this feature.
/oauth/mcp/authorizepath renders the consent page, matching case exactly; any other spelling renders Not Found.docker/nginx.confandvercel.json, and the Docker image CI step requests the route.is_loopback_host.appflowyis shown as code.pnpm type-check:mcp-tests)..dockerignoreexcludes.env.Design-system restyle
88893ee3restyles only the screens this PR adds (Settings → MCP, the consent page and its state views, the setup guide and the public setup page) to the AppFlowy Design-System Figma file. The shared components insrc/components/uiandsrc/components/_shared, the tokens and MUI are unchanged, and so are texts, behaviour, test ids and aria attributes. It builds on main's Figma token sync (#713), merged into this branch as55df61fe.Fill/*-light, radius 12, a 20 px coloured status icon, and Filled S actions in the type's colour (Retry is red).info_colored.svgadds the filled info icon the repo lacked.*-on-filltext colours.Left for a design-system sync of the shared components (found during the restyle, not changed here):
Button: radius one step above Figma (8/12 vs 6/8), Normal-weight labels (Figma: Enhanced), no focus-visible ring, and no tertiary colour on a disabled link.ConfirmModal: always confirms with Filled Red, so the non-destructive "Require approval" in 07 is red. Its dialog usesSurface/primaryand radius 12, where Figma usesSurface/layer-03and 16.TooltipandInput: radius 12 and 8, where Figma uses 8 and 6.Avatar: no 48 or 64 px size.Progress: a 20 px spinner, where Figma's indicator is 36 px.Related PRs
Test plan
mainat 4b78f4c (the Figma token sync, chore: sync color tokens with the Figma design system #713) as55df61fe, with no conflicts.42f4e9bd):pnpm type-check,pnpm type-check:mcp-tests, Prettier and ESLint on the 21 changed files. Jest over the MCP, consent, setup and settings suites: 33 suites, 650 tests passed. Every class the restyle adds exists in the Tailwind config: a build over the changed files emits all 171.capture.spec.tsrecaptured all nine frames. A dark-mode frame of the setup page was checked by hand; it is not in the gallery.mainat 7558479. The conflicts were with fix: remove AI settings tab and hide Present button #706: AI Settings and Present stay hidden, and the MCP tab and its?setting=AI_APPSdeep link stay.pnpm type-check,pnpm type-check:mcp-testsand ESLint on the changed filescapture.spec.tsagainst a local single-origin stack: the off state, owner consent, URL with Copy, AI-created pages showing in Web, a member's request, requiring approval again, and turning AI apps off with the connection kept🤖 Generated with Claude Code