refactor(desktop): unify EditAgentDialog styling with PersonaDialog#1540
refactor(desktop): unify EditAgentDialog styling with PersonaDialog#1540klopez4212 wants to merge 20 commits into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f3a6bd0e4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Addressed the review comment about the avatar preview: Avatar not persisted (P2) — Fixed in 61d85ce. The avatar preview is now |
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
LGTM — approve, with one product question and two minor notes below as inline comments.
What I checked (review-level, CI already covers build/lint/tests — all green):
- State machinery preserved: the open-effect vs catalog-arrival race handling, the
runtimeTouchedguard, the biome-ignore'd effect deps that protect in-progress edits from the 5s background poll, inherit-harness pin logic, and all submit validation (parallelismValid/timeoutValid/acpCommandValid/respondToValid) are behaviorally identical to before the restyle. 👍 - Latent bug fixed: the old custom-provider
<Input>routed each keystroke throughhandleProviderDropdownChange, running the API-key-env-var clearing logic per character. The new code callssetProviderdirectly, matching PersonaDialog. Nice. - Avatar preview correctly read-only with a comment explaining why (
UpdateManagedAgentInputhas noavatarUrl) — honest scoping. - No test breakage: nothing in
desktop/testsreferences the old markup, and the newedit-agent-dialog/edit-agent-dialog-submittest ids are additive. - Merges clean with current main.
One heads-up, not a change request: Create and Edit dialogs now diverge visually (Create still uses CreateAgentRuntimeFields / the default respond-to variant) — assuming Create gets the same treatment in a follow-up.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aded896048
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
klopez4212
force-pushed
the
kennylopez-edit-agent-panel
branch
from
9d8c4dc to
08e0a03
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08e0a038ef
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Restyle the built-in agent edit dialog to match the simplified persona edit panel: - Use ChooserDialogContent shell with the same grid layout (avatar preview left, fields right) - Replace raw <select> elements with PersonaDropdownField component - Apply PERSONA_FIELD_SHELL_CLASS / PERSONA_FIELD_CONTROL_CLASS to all inputs for consistent rounded-xl bordered styling - Keep 'Who can talk to this agent' (respond-to) at top level below agent name - Move advanced fields (runtime args, MCP command, MCP toolsets, turn timeout, parallelism, relay URL, ACP command, system prompt, env vars) into a collapsible 'Advanced' section with animated expand/collapse - Extract EditAgentAdvancedFields into its own file to stay under the 1000-line file size limit The edit dialog for built-in agents now looks visually identical to the persona edit panel that non-built-in agents use.
Add variant='persona' prop to CreateAgentRespondToField. When set, the component renders a PersonaDropdownField instead of a raw <select>, matching the styling of all other fields in the unified edit agent dialog. The label also uses text-foreground for consistency. Other usages (CreateAgentDialog, AddChannelBotDialog, MembersSidebar) are unaffected — they continue using the default variant.
- Rename 'Owner only (default)' → 'Only me (default)' in the persona dropdown options - When variant='persona', the AllowlistPicker renders without the bordered/indented container — just a flat search row underneath the dropdown, matching the channel member invite pattern - Search placeholder uses 'Search people' in persona mode - Remove the redundant header/count badge in persona mode (the dropdown already indicates the mode)
…ariant In the persona-styled respond-to field: - Hide the descriptive paragraph below the dropdown - Hide the 'Paste pubkeys' collapsible section (only the search remains for adding allowlist entries)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c08e1a2bba
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Address code review findings:
1. Restore all stripped comments in handleSubmit — harness-pin
resolution rationale, tri-state provider-capability explanation
('clear only when we KNOW the runtime is provider-locked; OMIT when
unknown so a transient state never becomes a destructive write'),
acpCommandValid/respondToValid rationale, and handler-level docs.
3. Fix stale placeholder on respond-to PersonaDropdownField: 'Owner
only (default)' → 'Only me (default)' to match the option label.
UpdateManagedAgentInput has no avatarUrl field, so avatar edits from this dialog cannot be persisted. Disable the preview to prevent the misleading UX of uploading/clearing an avatar that reverts on save. The state scaffolding is kept so wiring persistence later is trivial.
ChooserDialogContent destructures description as _description and never renders it. Remove the prop rather than passing dead weight.
When the user pastes a valid 64-char hex pubkey into the search input and no user-search results match, show an 'Add pubkey directly' option. This restores the ability to add pubkeys that aren't discoverable via search (no profile on the relay) without needing the full paste-pubkeys panel that was removed from the persona variant.
…alog Selecting Custom command for an inheriting persona-linked agent left inheritHarness=true because that branch had no catalog runtime, so Save silently followed the inherit path and discarded the custom command. Clear inheritance for any explicit runtime selection so the command input becomes editable and the pin path persists the choice. Signed-off-by: npub13fn4ahfnvaa2qwylvegdgeajqs0mph6v4qsw4jcqnw4mjh3hzh2quuucm5 <8a675edd33677aa0389f6650d467b2041fb0df4ca820eacb009babb95e3715d4@sprout-oss.stage.blox.sqprod.co>
The prior fix cleared inheritance for every runtime selection, but a catalog entry with command:null (adapter missing / not installed) can't supply a runnable command. Pinning it left agentCommand unchanged on Save while provider/model logic treated the new runtime as effective, so an inherited Claude agent could persist a Databricks provider while still running Claude. Guard the inherit-clear on custom command or a concrete catalog command. Signed-off-by: npub13fn4ahfnvaa2qwylvegdgeajqs0mph6v4qsw4jcqnw4mjh3hzh2quuucm5 <8a675edd33677aa0389f6650d467b2041fb0df4ca820eacb009babb95e3715d4@sprout-oss.stage.blox.sqprod.co>
klopez4212
force-pushed
the
kennylopez-edit-agent-panel
branch
from
c08e1a2 to
7f7eb07
Compare
Selecting Custom command cleared inheritHarness, but the Agent command input lives inside the collapsed Advanced section. A user could Save without expanding Advanced, leaving agentCommand equal to the original effective command (so agentCommandUpdate stayed undefined) and the custom selection silently no-oped. Auto-expand Advanced when Custom command is chosen so the command input is visible, and block Save when a pinned custom runtime has an empty command (the backend would spawn a runtime with no command otherwise). Extract the Save-button field validity into computeEditAgentFormValidity in personaRuntimeModel.ts to keep EditAgentDialog.tsx under the 1000-line ceiling and to make the gate unit-testable. Signed-off-by: npub13fn4ahfnvaa2qwylvegdgeajqs0mph6v4qsw4jcqnw4mjh3hzh2quuucm5 <8a675edd33677aa0389f6650d467b2041fb0df4ca820eacb009babb95e3715d4@sprout-oss.stage.blox.sqprod.co>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2705e57244
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The 07-edit-dialog-extracted-fields smoke shot waited on #agent-provider to confirm the Edit dialog was open, but that id belongs to the Create dialog's shared provider field. The refactored Edit dialog renders LLM provider selection via PersonaDropdownField, whose trigger button carries #edit-agent-llm-provider, so the readiness wait timed out. Point openEditDialog at #edit-agent-llm-provider. All 7 agent-readiness shots pass locally. Signed-off-by: npub13fn4ahfnvaa2qwylvegdgeajqs0mph6v4qsw4jcqnw4mjh3hzh2quuucm5 <8a675edd33677aa0389f6650d467b2041fb0df4ca820eacb009babb95e3715d4@sprout-oss.stage.blox.sqprod.co>
For a persona-linked agent that was inheriting, selecting Custom command and saving the visible prefilled command wrote no override: handleSubmit omitted agentCommand because the command still equaled the resolved effective value (agent.agentCommand), so reopening showed it still inheriting — the pin silently no-oped. Force the pin on an inherit->pin transition (agentCommandOverride == null at open) by sending the current command even when it matches the original. An empty command can't reach this branch: Save is blocked for an empty pinned custom command and catalog runtimes always set a concrete command. Extract the sentinel/pin/no-op contract into resolveAgentCommandUpdate in personaRuntimeModel.ts with unit coverage (inherit->pin, no-op when already pinned, clear sentinel) and to keep EditAgentDialog.tsx under the 1000-line ceiling. Signed-off-by: npub13fn4ahfnvaa2qwylvegdgeajqs0mph6v4qsw4jcqnw4mjh3hzh2quuucm5 <8a675edd33677aa0389f6650d467b2041fb0df4ca820eacb009babb95e3715d4@sprout-oss.stage.blox.sqprod.co>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 923098fed0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex P2: when the Edit Agent dialog opens with Advanced collapsed (the default), a provider change that makes requiredEnvKeys non-empty only passed those keys into an unmounted EnvVarsEditor, so the required ANTHROPIC_API_KEY / DATABRICKS_HOST row never rendered while Save stayed enabled. A user could switch providers and persist a missing credential, leaving the agent to crash-loop on next start. Two coordinated fixes: - Block Save: fold hasMissingRequiredEnvKey into computeEditAgentFormValidity via a new requiredEnvKeyMissing input, so canSubmit is false while a runtime/provider-required key is unset. The test file had long described this gate (hasRequiredEnvKeyMissing) but it was never wired into the component. - Surface the row: auto-expand Advanced on the missing->present-requirement transition so the amber required-credential row is visible (mirrors the existing Custom-command auto-expand). Fires once per transition, so the user can still collapse Advanced after filling the key. Extract the cohesive required-credential derivation (providerForRequiredKeys, fileSatisfiedEnvKeys, requiredEnvKeys, the missing check, and the auto-expand effect) into useRequiredCredentialState, which also keeps EditAgentDialog.tsx under the 1000-line ceiling (955). Add the pure hasMissingRequiredEnvKey helper to personaRuntimeModel.ts with unit coverage, and switch the existing block-save tests to exercise it. Validation: desktop unit tests 1932/1932, biome clean, tsc clean, file-size guard passing. Signed-off-by: npub13fn4ahfnvaa2qwylvegdgeajqs0mph6v4qsw4jcqnw4mjh3hzh2quuucm5 <8a675edd33677aa0389f6650d467b2041fb0df4ca820eacb009babb95e3715d4@sprout-oss.stage.blox.sqprod.co>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 25d7efd75a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
When a pinned persona-linked agent toggles "Inherit runtime from persona", prospectiveRuntimeId derived the target runtime from agent.agentCommand (the current override/pin) rather than the persona's own runtime. A Claude-pinned agent inheriting a buzz-agent/Anthropic persona therefore showed no required ANTHROPIC_API_KEY and enabled a Save that would persist the agent inheriting buzz-agent with its provider/credentials cleared or missing. Resolve the inherit-branch runtime from linkedPersona.runtime first, falling back to the agent.agentCommand dual-match only when there is no linked persona/runtime. Add effectiveProvider (persona's provider when inheriting) and effectiveEnvVars (persona-layered env when inheriting) and feed those into useRequiredCredentialState so the credential gate checks the runtime that will actually run. handleSubmit already derives its provider-capability classification from the shared prospectiveRuntimeId, so the block-save gate and submit path stay in agreement. Signed-off-by: npub13fn4ahfnvaa2qwylvegdgeajqs0mph6v4qsw4jcqnw4mjh3hzh2quuucm5 <8a675edd33677aa0389f6650d467b2041fb0df4ca820eacb009babb95e3715d4@sprout-oss.stage.blox.sqprod.co>
The Tauri update path resolved agent_command_override with divergent_agent_command_override, which returns None whenever the picked command maps to the same runtime as the linked persona. So choosing "Custom command" and saving e.g. goose/claude-agent-acp verbatim (matching the persona's own runtime) was persisted as inherit, and future persona runtime edits kept propagating against the user's intent. Thread the explicit harness_override intent bit through the update request (mirroring create): a non-inheriting dialog selection sets harnessOverride=true, and the new update_time_agent_command_override preserves that pick VERBATIM as a pin — including a byte-identical same-runtime command, since at update time an equal command only reaches the force branch because the user picked Custom. The empty inherit sentinel still clears the pin, and harness_override=false / persona-less agents defer to divergent_agent_command_override unchanged. Unit coverage for update_time_agent_command_override across the override, same-runtime, alias, sentinel, non-override, and persona-less cases. Signed-off-by: npub13fn4ahfnvaa2qwylvegdgeajqs0mph6v4qsw4jcqnw4mjh3hzh2quuucm5 <8a675edd33677aa0389f6650d467b2041fb0df4ca820eacb009babb95e3715d4@sprout-oss.stage.blox.sqprod.co>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fa708c828f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 27a9f10f77
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: npub13fn4ahfnvaa2qwylvegdgeajqs0mph6v4qsw4jcqnw4mjh3hzh2quuucm5 <8a675edd33677aa0389f6650d467b2041fb0df4ca820eacb009babb95e3715d4@sprout-oss.stage.blox.sqprod.co>
klopez4212
force-pushed
the
kennylopez-edit-agent-panel
branch
from
27a9f10 to
f3d0178
Compare
Signed-off-by: npub13fn4ahfnvaa2qwylvegdgeajqs0mph6v4qsw4jcqnw4mjh3hzh2quuucm5 <8a675edd33677aa0389f6650d467b2041fb0df4ca820eacb009babb95e3715d4@sprout-oss.stage.blox.sqprod.co>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c4a5c735e5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: npub13fn4ahfnvaa2qwylvegdgeajqs0mph6v4qsw4jcqnw4mjh3hzh2quuucm5 <8a675edd33677aa0389f6650d467b2041fb0df4ca820eacb009babb95e3715d4@sprout-oss.stage.blox.sqprod.co>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 09d94a6f01
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…nherit-transition Two codex P2 fixes for the Edit Agent dialog inherit-transition path: - Snapshot persona model on inherit transition: resolveInheritedRuntimeSubmission now returns a model and, on the inherit-transition-from-a-harness-pin branch, fills an empty local model from linkedPersona.model so a provider-backed buzz-agent/goose runtime is never saved model-less (readiness requires one). A deliberate local model still wins; steady-state stays authoritative. - Use known runtime capability before catalog loads: new pure resolveRuntimeProviderCapability classifies provider capability statically by id (buzz-agent/goose -> capable, claude/codex -> locked, else unknown), so a not-yet-loaded catalog no longer forces a known runtime to 'unknown' and omit the provider while clearing the override / writing env. Both feed the shared inheritedSubmission memo + prospectiveRuntimeId, keeping gate <-> record <-> spawn aligned. Adds unit tests for the model substitution branches and the capability tri-state. Signed-off-by: npub13fn4ahfnvaa2qwylvegdgeajqs0mph6v4qsw4jcqnw4mjh3hzh2quuucm5 <8a675edd33677aa0389f6650d467b2041fb0df4ca820eacb009babb95e3715d4@sprout-oss.stage.blox.sqprod.co>
2 participants
Summary
Unifies the Edit Agent dialog (for built-in/persona-less agents) to match the visual styling of the persona edit panel. Moves advanced runtime fields into a collapsible section.
Default view
Advanced expanded
Changes
ChooserDialogContent,PersonaDropdownField, and persona field styling classes (PERSONA_FIELD_SHELL_CLASS/PERSONA_FIELD_CONTROL_CLASS)variant="persona"prop: usesPersonaDropdownFieldwith "Only me (default)" / "Anyone" / "Allowlist" options; hides help text and paste-pubkeys section in persona modeField organization
Testing
tsc --noEmit✅check:px-text✅check:file-sizes✅