Repository navigation
feat: account-level Chat setting (user_settings), out of PostHog - #9162
Conversation
Account-level switch for opening new agent sessions as chat, so desktop, mobile and server-side launches read one value. Constant-default ADD COLUMN: metadata-only on Postgres 11+.
Per review, account settings stay off auth.users, which every authenticated request reads. user_settings follows channel_notification_prefs: user_id primary key, typed columns with defaults, and no row means the defaults, so Chat stays off for every account without a backfill. Replaces the users.experimental_chat column from the previous commit, which never shipped.
…m clients Automations no longer read the acp-chat PostHog flag: they ask for a chat whenever the agent supports ACP, and the host still answers with a terminal when it can't chat with that agent. cloudWorkspace.create takes an optional `surface`. Desktop and mobile send their Chat experimental setting; callers that omit it (released builds, CLI, MCP, SDK) keep the old flag lookup, so the input is additive.
The Chat switch is shown to everyone and is the only gate; the acp-chat PostHog flag is no longer read. A stored null (never touched) now means off. ACP agents open as chat when it is on; every other agent, and every agent when it is off, opens a terminal. - Show the presets bar on chat tabs again; agent presets already pick chat or terminal from the setting and ACP support. - Hide "Fork into chat" while Chat is off. - Cloud workspace creation sends the setting as `surface`.
Device-local switch in Settings → Experimental replaces the acp-chat PostHog flag in every mobile chat gate. Cloud workspace creation (composer and voice tool) sends it as `surface`.
The Chat experimental switch now writes users.experimental_chat through user.updateExperimentalChat and every surface reads it from the session, so desktop, mobile and the server agree. It only decides how new agent sessions open; chats that already exist (from automations, another device, or a released build) are always adopted and listed. - Drop the device-local stores; desktop's acp-chat key moves to DEAD_KEYS. - Drop the unreleased cloudWorkspace.create `surface` input: the server reads the account setting for every caller. - Desktop: chat sessions are adopted and automation run links focus chats regardless of the setting; the switch moves into AcpChatSetting. - Mobile: chats are listed, managed and stoppable regardless of the setting; only new sessions follow it.
Automations open as chat only when the owner has Chat on and the agent supports ACP; otherwise a terminal. Everyone starts with Chat off, so released builds, which hide chats from accounts outside the acp-chat flag, keep seeing every run.
Per review, the setting moves off auth.users into user_settings, so the session and auth stay untouched. - New userSettings router (get/update), mirroring channelNotificationPrefs: no row means the defaults, update upserts only the given fields. - Server reads it through loadUserSettings for automations and cloud workspace starts. - Desktop and mobile read it with a userSettings.get query; the switch writes the returned settings back into that query. - Desktop: the "Track file changes per turn" switch moves into its own component, alongside AcpChatSetting.
|
Caution Review failedThe pull request is closed. Finishing Touches
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 |
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
[Medium impact] The Chat preference changes new-session launches across client and server workflows. Wait to merge until new sessions resolve the account setting before choosing chat or terminal. Findings
Fix with agent prompt### Issue 1
apps/desktop/src/renderer/hooks/useAcpChatEnabled/useAcpChatEnabled.ts:4-5
`useAcpChatEnabled` treats an unanswered request as Chat being off. On a fresh launch, someone with Chat enabled can start a supported agent before `userSettings.get` returns and get a terminal instead. Desktop previously waited for the setting to resolve. Keep a loading state and resolve the setting before choosing the session type; apply the same fix to mobile.
### Issue 2
apps/desktop/src/renderer/hooks/useAcpChatEnabled/useAcpChatEnabled.ts:4-5
The desktop settings query has no account in its key, and signing out does not clear its cache. If another user signs in before that cache expires, the hook returns the previous user's setting while it refetches. That can show the wrong switch value and choose the wrong session type. Key the query by the signed-in account or remove it when the account changes.
### Issue 3
apps/desktop/src/renderer/routes/_authenticated/settings/experimental/components/ExperimentalSettings/components/AcpChatSetting/AcpChatSetting.tsx:41-46
The switch stays usable while its save is pending. Turning Chat on and then off can send overlapping writes; if the first write finishes last, Chat ends up on despite the user's last choice. Mobile has the same race. Disable the switch while saving, or serialize saves so the last choice wins.
### Issue 4
apps/desktop/src/renderer/routes/_authenticated/settings/experimental/components/ExperimentalSettings/components/ChatTurnSnapshotsSetting/ChatTurnSnapshotsSetting.tsx:18
`ChatTurnSnapshotsSetting` now disappears when Chat is off, but settings search still counts its match. Searching for `snapshot` can point to Experimental with no matching control. The search also hides the Chat switch needed to reveal it. Keep the control visible but disabled, or make search account for this dependency.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.SummaryMoves Chat from PostHog and desktop-local storage to an account setting in
Acknowledged scope:
Diagram%%{init: {'theme': 'neutral'}}%%
flowchart TD
Settings[Desktop or mobile settings] --> Update[userSettings.update]
Update --> DB[(user_settings)]
DB --> Get[userSettings.get]
Get --> Clients[Desktop and mobile launchers]
DB --> Server[Cloud starts and automations]
Clients --> Gate{Chat enabled and agent supports ACP?}
Server --> Gate
Gate -->|Yes| Chat[New chat]
Gate -->|No| Terminal[New terminal]
Existing[Existing chats] --> Visible[Always listed]
Reviews (1) · Last reviewed commit: "feat: read the Chat setting from user_se..." · Reviewed by Greptile |
|
Review at
Desktop: the Chat switch shows with no flag and is off by default. A failed update reverts the switch and shows a toast. I could not check the "on" state: this workspace's DB branch has no Full report and screenshots: https://app.superset.sh/page/pr-9162-review-account-level-chat-setting-euo24t |
- Launch paths wait for the setting (useResolveAcpChatEnabled) instead of reading "off" while userSettings.get loads: desktop launcher, new agent session, new workspace; mobile new terminal workspace and new session. - Desktop sign-out resets userSettings.get, so the next account never sees the previous one's value. Mobile already clears its query cache. - The Chat switch is disabled while a save is in flight, so quick toggles cannot land out of order (desktop and mobile). - "Track file changes per turn" stays visible, disabled while Chat is off, so settings search never points at a hidden control. - Comments name user_settings.experimental_chat and the Chat setting.
|
Review follow-up (fd92c2c), for the Greptile threads (now resolved) and @superset-home's review:
Checks: desktop |
Stacked on #9155 (
user_settingstable); merge that first and let it deploy. Supersedes #9154.Summary
user_settings.experimental_chatand off by default. Turn it on once and it applies on every device and on the server.How It Works
userSettings.get/userSettings.updatechannelNotificationPrefs. No row means the defaults;updateupserts only the fields it was given and returns the settingslib/user-settings.tsloadUserSettings/acpChatEnabled(userId). Used by automations (relay and cloud) andcloudWorkspace.createuseAcpChatEnabled()is acloudTrpc.userSettings.getquery.AcpChatSettingwrites the result back withsetData. The old localacp-chatstore is deleted and its key moved toDEAD_KEYSuseAcpChatEnabled()is a React QueryuserSettings.get(refetched on foreground through the existing focus manager); the Settings switch writes the result into the cacheShow vs open
useAutoAdoptChatSessions,useConsumeAutomationRunLink,seedPendingCloudChat; mobile home list, "Manage sessions", chat tabs and stop button, ended chats.Auth and the session are untouched.
Compatibility
userSettingsrouter). Nothing released loses an input or output.acp-chatlose chat (and their automations switch to terminals) until they turn the setting on. An automation that continues a previous chat starts a fresh terminal while its owner has Chat off (hostSession.test.ts:156).Manual QA
Earlier iteration (#9154, device-local setting), desktop dev app over CDP, before (
main) vs after:Testing
tsc:packages/trpc,packages/auth,apps/desktopclean.apps/mobile: same 188 errors asmain, none in changed filesbun test: trpc 817 (newuser-settings.test.ts), desktop full suite (useCreateAgentSessionfixture now mocksuseAcpChatEnabled), mobile 248, auth 90bun run check:i18nclean (2 new strings translated in all 16 locales);biome checkcleanuser_settingsand having automations follow the owner's settingFollow-ups
acp-chatPostHog flag once no live build reads itSummary by cubic
Moves the Chat experimental setting out of PostHog and per-device storage into an account-level
user_settingsrow, so desktop, mobile, and server-side automations all agree on one value that's off by default.userSettingstRPC router (get/update) that reads and upsertsexperimentalChat; no row means off.acp-chatPostHog flag and local store withuserSettings.getqueries and persist viauserSettings.update.useResolveAcpChatEnabled) instead of reading "off" while it loads; desktop sign-out resets the query so the next account never sees the previous one's value.acp-chatPostHog flag remains only for released builds and can be removed later.acp-chatstore is deleted and its key moves toDEAD_KEYS, so anyone with the setting enabled in a previous build will see terminals until they flip the new account-level toggle.Migration
Existing
acp-chatfeature-flag users lose chat access until they enable the new setting in Settings → Experimental → Chat; the old local key is dead.Written for commit fd92c2c. Summary will update on new commits.