Repository navigation
Conversation
guidovizoso
left a comment
There was a problem hiding this comment.
Thanks for this. The Settings profile endpoint, the profileNameSet flag, the dev-actor change that stops overwriting the name, and keeping the greeting in the UI rather than in message history all look good.
The main UX points are inline:
- Name prompt: move it into the onboarding flow instead of a global modal, which currently stacks on top of
/onboarding. - Recipient picker: your own Bots should come before "New Bot", which is currently auto-highlighted.
- Bot creation: "Connect a custom agent" should be a selectable card like "Run here", with the footer button switching between Create Bot and Continue.
Also: the PR has merge conflicts with main, and there's some import-reordering noise in server/src/index.ts and app.ts that makes the diff harder to review.
| <CopilotProvider> | ||
| <AppHotkeys /> | ||
| <Outlet /> | ||
| <FirstRunNamePrompt /> |
There was a problem hiding this comment.
This is mounted in the /_authed layout, which also wraps /onboarding. So a brand-new user gets a modal that can't be dismissed, sitting on top of the "Welcome to OpenBot" onboarding slides. It also gates every authed page, admin included.
Suggestion: make the name a step in onboarding.tsx's STEPS (right after WelcomeStep), reusing ProfileNameForm firstRun, and drop this global prompt. The Settings Profile form stays as is. For people who already finished onboarding and have needsName, the generic greeting from firstConversationGreeting is a fine fallback, or show a lighter one-time prompt outside /onboarding.
| multiple | ||
| items={profiles ?? []} | ||
| isItemEqualToValue={(item: AgentProfile, value: AgentProfile) => | ||
| items={[NEW_BOT, ...(profiles ?? [])]} |
There was a problem hiding this comment.
With defaultOpen and autoHighlight, "New Bot" is the highlighted item when you land on a new conversation, so pressing Enter takes you to /bots?new=true instead of picking your first agent. Your own Bots should come first.
Two smaller issues:
- In multi-select, picking New Bot after choosing some Bots throws that selection away.
- After creating the Bot, you aren't brought back to the conversation with it selected.
Suggestion: move it to the end, or better, make it a footer action outside the list so it can never be auto-highlighted. Ideally open NewBotSheet in place and select the new Bot as recipient once it's created.
| * profile links and reloads preserve the pending recipient without creating an empty channel. | ||
| */ | ||
| /** What `GET /api/agents/:id` answers for a Bot this person cannot see. */ | ||
| const NEW_BOT = { |
There was a problem hiding this comment.
Nit: this was inserted between the AGENT_NOT_FOUND JSDoc and its constant, so "What GET /api/agents/:id answers for a Bot this person cannot see" now describes NEW_BOT.
| aria-pressed={custom} | ||
| onClick={() => { | ||
| if (busy) return; | ||
| setCustom(true); |
There was a problem hiding this comment.
The two cards share OPTION_CLASSES and aria-pressed, so they look like a pair of choices, but they behave differently: "Run here" toggles, while this one validates the name and jumps to the framework step immediately.
Suggestion: make this card just setCustom(true) and let the footer button carry the action. It already switches between "Create Bot" and "Continue", and advance() already branches (custom ? go("runtime") : create(...)). The name check belongs in advance. canContinue already requires named.
| const [visibility, setVisibility] = useState<Visibility>("private"); | ||
| /** Run it here unless the person picks otherwise: the answer that needs nothing set up. */ | ||
| const [runtime, setRuntime] = useState<Runtime | null>({ kind: "builtin" }); | ||
| const [custom, setCustom] = useState(false); |
There was a problem hiding this comment.
When builtInAvailable is false, "Run here" is disabled but nothing is selected, so Create stays disabled until the user happens to click the other card. Consider useState(!builtInAvailable). Since capabilities loads asynchronously, deriving it would work too: const effectiveCustom = custom || !builtInAvailable.
| */ | ||
| const STEPS = ["runtime", "endpoint", "about"] as const; | ||
| /** Details come first; custom agents then choose a framework and verify their endpoint. */ | ||
| const STEPS = ["about", "runtime", "endpoint"] as const; |
There was a problem hiding this comment.
This PR replaces several longer "why" comments here and on NewBotSheet, AboutStep and RuntimeStep with one-liners. Some of that reasoning still applies after the restructure, e.g. why the heading takes focus on arrival, and why there's no separate Test button because the action that moves on also tests the endpoint. Worth keeping the parts that are still true, in line with the rest of the codebase.
| const loadName = deps.loadNameForActor; | ||
| if (loadName) { | ||
| for (const agent of Object.values(agents)) { | ||
| agent.use(new NameContextMiddleware(() => loadName(actor.id))); |
There was a problem hiding this comment.
This attaches the name middleware to every resolved agent, including custom (BYO) agents. So the user's display name is now sent as context to third-party endpoints, which the PR description doesn't mention. Either limit it to built-in agents or call it out explicitly.
| } from "@ag-ui/client"; | ||
| import { defer, from, map, type Observable, switchMap } from "rxjs"; | ||
|
|
||
| const LEGACY_PROFILE_CONTEXT = "OpenBot account owner profile"; |
There was a problem hiding this comment.
LEGACY_PROFILE_CONTEXT and the PROFILE_GUIDANCE_ID message filtering (on the input and on MESSAGES_SNAPSHOT) handle entries nothing creates any more. It looks left over from an earlier iteration of this PR, so it can probably be removed.
| coordinationForActor(actorId, initiator, depth), | ||
| ), | ||
| loadInstructionsForActor, | ||
| loadNameForActor: async (actorId) => { |
There was a problem hiding this comment.
Minor: this builds a new ProfileStore and does a DB read on every agent run (and /api/me gains a query too). Build the store once, alongside the createProfileStore(database) passed to createApp.
Summary
This PR simplifies OpenBot's first-run and Bot creation flows, adds an editable display name, and makes explanatory copy more consistent across the app. Built-in Bots can be created from one screen, while custom agents follow a shorter connection flow that preserves entered values when navigating back.
Bot creation
Conversation entry and app chrome
First-run profile and Settings
Restrained name personalization
Copy consistency
Scope and review focus
This PR contains the UX work on top of current main. The separate local setup fixes remain in #810 and #811.
Review the Bot creation flow in both hosting modes, including validation and Back navigation; the first-run name prompt and subsequent Settings edits; and the scope and handling of agent profile context. The pre-PR review fixed stale Settings state after the welcome dialog and ensured the name-use guidance reaches built-in agents.
CI follow-up
x-api-keyheader or the Bearer header used by Anthropic SDK 2.3. Endpoint, model, and successful-run assertions remain intact. All five AG2 tests pass locally with the CI dependency versions.Validation
Model wording itself has not been evaluated in a live model conversation; sparing name use is guidance, not a guaranteed model behavior.