Skip to content

Make onboarding honest, Gmail disconnect real, and the app readable at large text - #75

Merged
jerelvelarde merged 1 commit into
mainfrom
jerel/openduet-fix-app-ui
Oct 26, 2026
Merged

jerelvelarde merged 1 commit into
mainfrom
jerel/openduet-fix-app-ui

Conversation

@jerelvelarde

Copy link
Copy Markdown
Collaborator

The problem

The release review ran the app end to end at phone width with a real model and found these, all reproduced on current main (71dc24c):

  • M1. Onboarding's "It runs on your machine" step said conversations, files and Bots are stored on this computer. They are not. The server cannot start without CopilotKit Intelligence, which keeps threads and memory (by default in CopilotKit's cloud), and runtime telemetry is on by default.
  • M3. Connections › Gmail and Google Calendar › Connected › Disconnect only said "You can't disconnect here yet", and the token stayed. The copy also said "OpenBot".
  • M4. In the README's dev setup, every screen showed CopilotKit's inspector button and a promotional popover over the header, the New channel sheet's Create and a page's "…".
  • M6. At 200% text: page view lines overlapped (leading-[27px] fixed while the text grew), the page title broke mid-word into a column, the Tasks search button was pushed off-screen, and the + covered the first row.
  • Minors. Escape on New channel threw away the typed name and purpose. "OpenBot" in text people read. The "Pick a Bot" step showed an approval card with the roster cut off, and the brief demo was dated "Friday, October 23". A model failure showed raw SDK text ("…Is the computer able to access the url?") and was lost on reload. A hanging model said "Thinking it through" for over two minutes. The checklist read "Bot computer: Not checked, set AGENT_COMPUTER_URL…" on an install that had set it, and called the official https://api.anthropic.com a custom endpoint.

The approach

M1. Accurate copy. The step is now "Where your data lives": Bots and settings stay in this install's database; conversations and memory are in CopilotKit Intelligence, in CopilotKit's cloud unless you host it yourself; what you ask goes to your AI model; and a footnote says CopilotKit receives usage events and DO_NOT_TRACK=1 turns them off. Each sentence is checked against code (.env.example, docs/telemetry.md, server/src/telemetry.ts, the schema). Files and connected accounts are left out on purpose, because a file's content also reaches the thread and the model, and a brokered app's account is held by the broker. The step only shows for a single-user install, so naming the variable is fine.

M3. A real disconnect. DELETE /api/plugins/servers/:id/connection now handles user-oauth apps (Gmail and Calendar, Drive, Notion). Before, it answered "That app doesn't connect this way." A new store.disconnectHeld does this:

  1. Locks the person's connection row and its vault row.
  2. Asks the vendor to withdraw the refresh token at the catalogue's own revokeUrl (RFC 7009). That address was already declared "where a disconnect is sent" but never used.
  3. Revokes the vault credential and deletes the connection row in the same transaction.
  4. Writes mcp.account_disconnected (reason self, plus the vendor's answer as one word).

If the vendor can't be reached (timeout, 5xx, 429), the transaction rolls back and nothing changes; pressing again retries. If the vendor answers no, access still ends here, and the page tells the person to remove the app at Google too. The page's menu item is now a working "Disconnect", with a status line afterwards. The copy uses the product name from app config.

M4. enableInspector={false} on CopilotKitProvider (react-core 1.77's documented prop). VITE_COPILOTKIT_INSPECTOR=true turns it back on for developers.

M6.

  • Page body and bullet line heights are ratios, and the bullet dot is placed in em.
  • The page title wraps only between words, ends in an ellipsis after two lines, and gets its own row under Back and "…" when the header is narrower than 20rem. That is a container query, so it only happens at large text or below 320px.
  • The Tasks header wraps, and sort and search drop under the tabs.
  • The + and the sort and search buttons are fixed px (56 and 44). The spacer at the foot of the list is sized from the +.
  • Section headings wrap too.

Minors.

  • New channel sheet. Escape and a tap outside close only the innermost thing. The picker folds first, then "Discard this channel?" with Keep editing focused. A sheet with nothing typed still closes at once.
  • "OpenBot" copy. Every user-visible "OpenBot" in the app now reads the product name from app config (34 strings in 18 files), as does the server's "listening on" log line.
  • Onboarding demo. The roster is the "Pick a Bot" step's only picture, and the brief demo shows today's date.
  • Model failures. These are mapped to plain sentences: can't reach the model, busy, key refused, took too long. A bare connection error still names the Bot. The packaged /bot chat uses the same mapping. The sentence is kept in sessionStorage with the id of the last message, so a reload shows it again only while nothing newer has arrived.
  • Slow model. The Chat pill moves from "Thinking it through" to "Still working on it" after 20 quiet seconds, then to "Taking longer than usual" after 60. Any progress resets it. Server stall behaviour is unchanged.
  • Setup checklist. When the computer service answers and no Bot computer is running yet, the check reports idle. The row reads Ready, "each Bot's computer starts the first time it's needed", and names no variables. When the service doesn't answer, the row names the address and token. The official OpenAI and Anthropic base URLs (with or without /v1) no longer count as custom endpoints.

What is not covered

  • Docs. openbot/docs/plugins/google-workspace.md and google-drive.md still say Disconnect is "Not built yet". The docs belong to another agent's lane, so I left them alone.
  • Offboarding. Removing a person (retireConnectionsFor) still revokes only our vault copy and doesn't ask the vendor. That is a separate change.
  • Server copy. Refusal strings that reach the browser still say "OpenBot" (for example "OpenBot has no app called …"). The same goes for app/serve.ts's production log line. Fixing those means passing the product name through server modules, which belongs in its own change.
  • Real Google. The revoke request uses RFC 7009 with client authentication in the body, the same way the token exchange does. It is tested against the fake Google and with stubs, but not against real Google. If Google ever refuses that shape, the disconnect still ends access here and tells the person to finish at Google, so nobody gets stuck.
  • Metric tiles. On a page, metric tiles keep their fixed 28px figures. They don't overlap at 200%, but they don't grow either.

Verification

  • bun test app/tests: 2421 pass, 0 fail (265 files). Adds copilot-inspector, held-account-disconnect, patient-step and run-error-memory, plus new cases in channel-sheet, chat-run-lifecycle, onboarding-route, run-lifecycle and plugin-mutation-refresh.
  • bun test server (full suite against a throwaway Postgres 17 on :55473): 6334 pass, 14 fail of 6367. 13 of the 14 also fail on pristine origin/main on this machine: app-access-routes.integration (5, Team Bot timeouts), test-network-guard (5), learning-mastra (1) and learning-langgraph (2), which need the real network or providers. The 14th is assertion-shape's whole-tree scan going past bun's 5s timeout under full-suite load, and it passes on its own (10/10). Three guards this change did trip are fixed in the commit. sole-definition and assertion-shape now pass: the disconnect audit writes a literal boolean, and an un-awaited rejects the allow-list was carrying is now awaited and its entry removed.
  • Focused server files: plugin-user-credential.integration 22/22 (7 new store cases and 2 route cases for disconnect, including an unreachable vendor, a refusal, already-revoked, other people's rows and the audit row), oauth-revoke 5/5, google-workspace-fake 18/18 (a real revoke against the fake Google), plugin-routes 86/86, openduet-setup 45/45, model-availability-check 16/16.
  • bun run typecheck: clean. bun run typecheck:tests: clean against the baseline. I typed the test vault stub fully, which fixes one baselined error, and removed that line from the baseline.
  • biome lint --error-on-warnings: clean. biome format: clean.
  • Visual check at 390px, light, through the app's own Vite config (a temporary preview page with a mocked API, deleted before commit). Screenshots are in /private/tmp/claude-501/-Users-jerel-cpk-OpenDuet/8d570fdc-9cf4-4d73-a2c3-5a0db6c25efb/scratchpad/shots/:
    • before-tasks-200pct.jpg → after-tasks-200pct.jpg, after-tasks-100pct.jpg
    • before-page-200pct.jpg → after-page-200pct.jpg, after-page-100pct.jpg
    • before-onboarding-roster.jpg → after-onboarding-roster.jpg
    • after-onboarding-local-step.jpg
    • before-gmail-disconnect-menu.jpg → after-gmail-disconnected.jpg
    • after-channel-sheet-escape.jpg
    • inspector-with-opt-in-as-before.jpg (the overlay as everybody saw it) → inspector-off-by-default-after.jpg

M1: onboarding's local step said conversations stay on this computer. They
live in CopilotKit Intelligence (its cloud unless self-hosted), every message
goes to the model, and runtime telemetry is on by default. The step now says
each of those, and how to turn telemetry off.

M3: Disconnect on Gmail and Google Calendar now works. DELETE
/servers/:id/connection takes user-oauth apps down a new store.disconnectHeld:
it asks the vendor to revoke the refresh token at the catalogue's revokeUrl
(RFC 7009), then revokes the vault row and deletes the connection in one
transaction, and audits it. If the vendor can't be reached, nothing changes.
If the vendor refuses, access still ends here and the page says to finish at
Google.

M4: CopilotKit's inspector and its popover are off unless
VITE_COPILOTKIT_INSPECTOR=true.

M6: page body line heights are ratios. The page title wraps between words and
gets its own row at large text. The Tasks header wraps. The + and the icon
buttons stay a fixed size, with a spacer that clears the +.

Also: Escape on New channel closes only the innermost thing and asks before
dropping typed fields. User-visible "OpenBot" becomes the product name. The
roster step loses the approval demo and the brief demo shows today's date.
Model and network failures are plain sentences that survive a reload while
still current. The Chat pill says "Still working on it" after 20 quiet seconds.
The checklist reads an idle Bot computer as Ready, and the official API base
URLs are no longer "custom".
@jerelvelarde

Copy link
Copy Markdown
Collaborator Author

Security review

I reviewed the whole diff, with most attention on the disconnect change (M3). No findings that block this PR. Details below.

Disconnect (DELETE /api/plugins/servers/:id/connection → store.disconnectHeld)

  • Whose account. Only the caller's own. The route passes context.var.actor.id as both userId and by, and reads no user id from the path, query or body. The store matches the row on (serverId, userId). Tests check that another person's credential stays live and their calls still go out with their own token (ends only the asker's own account, and the route-level ends the signed-in person's own account, and only theirs). The anonymous actor ("") matches nothing.
  • Auth. Same as before: requireUser, no admin gate. A person ends their own grant, just as they start it. For a Shared app the account belongs to the deployment and has no per-person row, so a member's DELETE does nothing (disconnected: false). The page still offers no disconnect there.
  • Which apps take this path. A server row whose URL names no broker app and whose catalogue entry is user-oauth. Brokered apps keep their existing path. The revoke URL comes from the reviewed catalogue entry, or the fake-Google override in non-production only. It never comes from the request or a stored row. So this cannot be turned into an SSRF or a token sent somewhere else.
  • What leaves the server. A form POST to the vendor's revoke endpoint with the person's refresh token and the deployment's client id and secret. That is the same material, sent to the same vendor, as the token exchange the server already makes on every call. It uses redirect: "manual", so a 3xx can't forward the secrets, and has a 10 s timeout. Nothing from the vendor's response is passed on: the outcome is one of four words (revoked, already_gone, refused, unreachable).
  • Order and failure. The vendor is asked first, under SELECT … FOR UPDATE locks on the connection row and the vault row. Then the vault row is revoked and the connection row deleted, in the same transaction. If the vendor is unreachable, everything rolls back, so access is either untouched or ended, never ended here while the grant stays live with no way to reach it. A vendor refusal still ends our copy, and the page tells the person to remove the app at Google too. In-flight tool calls are serialised behind the same row lock, and a call queued behind the disconnect finds the row gone and is refused (connectionTokenFor already handles this). No access token is cached anywhere, so there is nothing left to purge.
  • Audit. mcp.account_disconnected with reason: "self", vendorRevocationRequested (true only when the vendor confirmed, or said the token was already dead) and vendorAnswer as one word. A test checks that the refresh token is not in the audit payload. The note on the event in audit.ts is updated.
  • Errors. A 502 says "Couldn't reach Google, so nothing was disconnected. Try again in a moment." Any other failure is thrown as before. No vendor text, token or client id reaches the response.

Other changes

  • Inspector off by default (M4). This removes a developer overlay. The opt-in is a build-time VITE_ variable, so nothing from a request can switch it on.
  • Failure sentence kept for a reload. It goes in sessionStorage, per tab, and it is the already-mapped plain sentence, never the raw error. It is shown again only while the conversation's last message id still matches.
  • Model base-URL check. It is stricter, not looser: only https, the exact official host, the default port, no credentials, query or fragment, and a path of nothing or /v1 count as "not custom". Anything else is still skipped as before, so a key is never sent to a gateway for the model lookup.
  • Setup checklist. No new data. idle comes from the existing authenticated list() call, and the member view still gets no variable names.
  • Onboarding copy. Text only. It names DO_NOT_TRACK, which is fine here because the step is shown only to a single-user install, whose one reader runs the server.

Residual risk (not blocking)

The RFC 7009 request includes client credentials in the body. That matches the token exchange and is what Notion needs, but it has not been tried against production Google. If Google refuses it, the outcome is refused: local access still ends and the person is told to finish at Google. Offboarding (retireConnectionsFor) still doesn't revoke at the vendor. That is unchanged and listed under "What is not covered".

@jerelvelarde
jerelvelarde merged commit cd4b622 into main Oct 26, 2026
19 checks passed
jerelvelarde added a commit that referenced this pull request Oct 26, 2026
#75 made Disconnect real for user-held accounts: it asks Google to revoke the
refresh token, then revokes the stored credential and removes the connection.
The Gmail and Calendar and Google Drive pages still said it was not built and
sent people to Google's settings instead.
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