Skip to content

feat(desktop): import agents as v2 terminal presets with live link#4101

Merged
Kitenite merged 4 commits intomainfrom
agent-presets-import
May 5, 2026
Merged

feat(desktop): import agents as v2 terminal presets with live link#4101
Kitenite merged 4 commits intomainfrom
agent-presets-import

Conversation

@Kitenite
Copy link
Copy Markdown
Collaborator

@Kitenite Kitenite commented May 5, 2026

Summary

  • Adds an Import agent dropdown to v2 terminal preset settings; presets carry an optional agentId that live-links to the agent definition. Editing the linked command happens in /settings/agents and propagates to every preset that points at it. Snapshot in commands is kept as a fallback when the linked agent is missing or disabled.
  • Migration shim seeds all builtin terminal agents (AGENT_TYPES) into the v2 collection on first run per org, on top of v1's verbatim copy. v1 schema, router, defaults, and launcher are untouched.
  • /settings/agents route now accepts ?agent=<presetId>. The preset editor's "Open" button deep-links so users land on the right agent. v2 selects the matching host config on first mount; v1 ignores the param.

Implementation notes

  • v2 launcher (useV2PresetExecution) resolves agentId to the live command at launch time via the vanilla electronTrpcClient (the React-hook form would route to the workspace HTTP server inside WorkspaceTrpcProvider and 404 silently).
  • Single-subscriber preserved: state.addTab(...) with initialCommand on pane data; TerminalPane is the only place that calls terminal.createSession. Same code path as a regular new-tab open, just with a command attached.
  • Shared UI (PresetEditorDialog, PresetRow, QuickAddPresets) accepts the new shape via optional props/new pill API. v1 callers don't pass agents and no v1 row has agentId, so the linked branch stays dormant in v1.
  • apps/desktop/plans/20260505-agent-preset-import-learnings.md captures the design decisions and the tRPC routing trap for future reference.

Test plan

  • Fresh org: open Terminal settings → all builtin agents appear as preset rows with their icons.
  • Open the Import agent dropdown → already-imported agents are disabled with a check; click an unimported one → row appears.
  • Click an imported preset → editor shows the linked banner + read-only command + Open button. Click Open → lands on /settings/agents with that agent selected.
  • Edit the agent's command in /settings/agents → return to the preset bar in a v2 workspace → click the pill → terminal launches with the new command.
  • Disable the linked agent → preset still launches using the snapshot.
  • Delete an imported preset → its pill becomes available again in the Import-agent dropdown.
  • v1 mode: terminal preset settings unchanged (Quick add still renders, no agent link surfaces).

Summary by cubic

Add an “Import agent” dropdown to v2 Terminal Presets that creates live‑linked presets to host‑service agent configs; edits in Agents settings update every linked preset, with a snapshot fallback if the agent is missing or disabled. Also added deep-linking to Agents settings (with retry until configs load) and live command resolution at launch for reliable execution.

  • New Features

    • Import agent dropdown lists host-configured terminal agents; already‑imported items are disabled with a check.
    • Presets store optional agentId (host preset id); the editor shows a linked banner, a read-only live command, and an “Open” link to /settings/agents?agent=<id> that pre-selects the agent in v2.
    • Launch resolves agentId via useV2AgentConfigs and passes the command as initialCommand; falls back to the snapshot when missing/disabled.
    • Agents settings deep-link now waits for configs and selects the match once available.
    • UI: preset icons (by name or agentId), wider Terminal settings, no table max-height; Quick Add is a dropdown deduped by agentId.
  • Migration

    • Added agentId to the v2 schema; v1 schema and launcher remain unchanged.
    • Migration upgrades v1 defaults whose names match a builtin (e.g. “claude”) into linked rows with proper labels, then seeds any remaining built-ins; runs once per org.
    • Settings search marks Terminal Quick Add as “shared” so it’s visible in v2.

Written for commit edf3d4c. Summary will update on new commits.

Summary by CodeRabbit

  • New Features

    • Link terminal presets to agents so preset commands can auto-sync from live agents (with snapshot fallback).
    • Import agent presets via a redesigned dropdown quick-add menu; quick-add now available in both v1 and v2 settings.
    • Built-in agent presets are seeded during migration/first-run.
  • Improvements

    • Preset editor shows linked-agent info and renders live command read-only when linked.
    • Preset rows now display icons.
    • Agents settings accept an initial preset selector for preselection.

Kitenite added 2 commits May 5, 2026 11:41
Adds an Import-agent dropdown to v2 terminal preset settings, seeds all
builtin terminal agents on first migration, and live-resolves the
linked agent's command at launch (snapshot fallback when missing or
disabled). v1 data layer is untouched; shared UI components accept the
new shape without affecting v1 behavior.
The /settings/agents route now accepts ?agent=<presetId>. v2 selects the
matching host config on first mount; v1 ignores the param. The preset
editor dialog's "Open" link passes the linked agentId so users land on
the right agent.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 5, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1da80da1-607e-4577-9be7-67ec59ff0f73

📥 Commits

Reviewing files that changed from the base of the PR and between f94e990 and edf3d4c.

📒 Files selected for processing (1)
  • apps/desktop/src/renderer/routes/_authenticated/settings/agents/components/V2AgentsSettings/V2AgentsSettings.tsx

📝 Walkthrough

Walkthrough

Adds an optional agentId to v2 terminal presets, seeds/migrates builtin agents, provides an agent-backed quick-add/import UI, shows a linked-agent read-only editor branch, and resolves live agent commands at execution with snapshot fallback.

Changes

Agent Preset Import Feature

Layer / File(s) Summary
Data Shape
apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.ts
Adds optional agentId?: string to v2TerminalPresetSchema / V2TerminalPresetRow.
Migration / Seeding
apps/desktop/src/renderer/routes/_authenticated/hooks/useMigrateV1PresetsToV2/useMigrateV1PresetsToV2.ts
Maps v1 presets to v2, links rows to builtin AGENT_TYPES when names match, and seeds missing builtin agent presets with agentId and sequential tabOrder.
Agent Fetching / Command Resolution
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2PresetExecution/useV2PresetExecution.ts, useV2AgentConfigs, useLocalHostService
Loads host agent configs, builds agentCommandsById, adds resolvePresetCommands(preset) to substitute a live agent command when preset.agentId matches, and uses resolved commands everywhere executePreset consumes commands.
V2 Presets Import Wiring
apps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/components/V2PresetsSection/V2PresetsSection.tsx
Fetches agents via host service, dedupes by agent.presetId, builds quickAddPills, marks already-added by agentId, and persists agentId when inserting presets via extended insertV2Preset.
Quick-Add UI & Types
.../QuickAddPresets/QuickAddPresets.tsx, .../QuickAddPresets/index.ts
Rewrites quick-add to a dropdown importer; introduces QuickAddAgentPill (agentId,label,description,commands) and updated QuickAddPresets props (isPillAdded/onAddPill/isAddDisabled).
Presets Section Integration
apps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/components/PresetsSection/PresetsSection.tsx
Switches to pills-based quick-add, computes quickAddPills, and creates presets from pill fields via handleAddPill.
Editor: Linked-Agent Branch
.../PresetEditorDialog/PresetEditorDialog.tsx
Adds optional agents?: HostAgentConfigDto[] prop; when preset.agentId matches an enabled agent, shows linked-agent title/banner and read-only command derived from live agent (fallback to snapshot); otherwise retains editable fields.
Agents Settings / Routing
apps/desktop/src/renderer/routes/_authenticated/settings/agents/page.tsx, .../AgentsSettings/AgentsSettings.tsx, .../V2AgentsSettings/V2AgentsSettings.tsx
Adds agent search param and AgentsSettingsSearch type; passes initialAgentPresetId into AgentsSettingsV2AgentsSettings, which consumes it once to auto-select a matching agent.
UI: Row/Table/Layout
.../PresetRow/PresetRow.tsx, .../PresetsTable/PresetsTable.tsx, .../TerminalSettings/TerminalSettings.tsx
Adds leading icon in preset rows resolving by name then agentId, removes hard max-height from PresetsTable, and expands container max-width to max-w-6xl.
Plan / Docs
apps/desktop/plans/20260505-agent-preset-import-learnings.md
New v2-only implementation plan documenting schema, UI/launcher changes, tRPC/workarounds, single-subscriber execution constraint, and rewrite order.

Sequence Diagram

sequenceDiagram
    actor User
    participant AgentsSettings as AgentsSettings
    participant V2AgentsSettings as V2AgentsSettings
    participant V2PresetsSection as V2PresetsSection
    participant PresetEditorDialog as PresetEditorDialog
    participant useV2PresetExecution as useV2PresetExecution
    participant Terminal as Terminal

    User->>AgentsSettings: Open /settings/agents?agent=presetId
    AgentsSettings->>V2AgentsSettings: initialAgentPresetId
    V2AgentsSettings->>V2AgentsSettings: Auto-select matching agent
    User->>V2PresetsSection: Open Presets
    V2PresetsSection->>V2PresetsSection: Fetch agents via host service
    V2PresetsSection->>V2PresetsSection: Build quickAddPills (dedupe by presetId)
    User->>V2PresetsSection: Select pill from Import dropdown
    V2PresetsSection->>V2PresetsSection: insertV2Preset(name,commands,agentId)
    User->>PresetEditorDialog: Open preset editor
    PresetEditorDialog->>PresetEditorDialog: Match preset.agentId -> agents
    alt linked agent present
        PresetEditorDialog->>PresetEditorDialog: Show linked-agent banner and read-only command
    else
        PresetEditorDialog->>PresetEditorDialog: Show editable fields
    end
    User->>useV2PresetExecution: Execute preset
    useV2PresetExecution->>useV2PresetExecution: Fetch live agent configs
    useV2PresetExecution->>useV2PresetExecution: resolvePresetCommands(preset)
    alt live agent command available
        useV2PresetExecution->>Terminal: Launch using live command
    else
        useV2PresetExecution->>Terminal: Launch using preset.commands snapshot
    end
Loading

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly Related PRs

  • superset-sh/superset#3274: Modifies V2 terminal presets surface (V2PresetsSection, schema, quick-add/editor) with direct overlap.

Poem

🐰 I hopped through presets, found agents to bind,
Linked commands that now refresh from their kind.
A dropdown to import, an editor that knows,
Live when it can, or snapshot it shows —
Quick, small changes that make terminal flows glow.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 15.38% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: adding agent import functionality as v2 terminal presets with live linking capability.
Description check ✅ Passed The description comprehensively covers all required sections: summary of changes, detailed implementation notes, test plan, and additional context about the design decisions.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch agent-presets-import

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 5, 2026

Greptile Summary

This PR wires builtin/custom agents into the v2 terminal preset system: presets can now carry an optional agentId that live-links to the agent definition, with the stored commands array kept as a snapshot fallback. A migration shim seeds all AGENT_TYPES into the v2 collection on first run, and /settings/agents gains a ?agent= search param so the preset editor's "Open" button can deep-link to the right agent config.

  • Schema + migration: agentId?: string added to v2TerminalPresetSchema; migration copies v1 rows verbatim and appends agent-linked rows for any builtin not already present by name.
  • Launcher: useV2PresetExecution fetches agents via the vanilla electronTrpcClient (correctly bypassing WorkspaceTrpcProvider) and resolves the live command at launch time, falling back to the snapshot when the agent is missing or disabled.
  • Settings UI: V2PresetsSection gains an "Import agent" dropdown; PresetEditorDialog shows a read-only linked-agent banner with an Open button; PresetRow gains an icon column; QuickAddPresets is redesigned from inline pills to a DropdownMenu.

Confidence Score: 3/5

The core agent-link mechanics and the tRPC routing workaround are sound, but the interaction between the name-based migration dedup and the agentId-only Import dropdown check will silently allow duplicate preset rows for existing users with default v1 preset names.

The migration copies v1 presets by name and skips adding agent-linked rows for any name that already exists (e.g., 'Claude', 'Codex'), leaving those rows without an agentId. The Import dropdown then checks only existingAgentIds—so those unlinked rows don't count as already-added—and allows a second, linked 'Claude' row to be created alongside the old one. This affects any user whose v1 preset names happen to match agent labels, which is the common case for users who kept the defaults.

V2PresetsSection.tsx (isPillAdded dedup), useMigrateV1PresetsToV2.ts (name-based skip), and V2AgentsSettings.tsx (deep-link ref consumed before match confirmed)

Important Files Changed

Filename Overview
apps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/components/V2PresetsSection/V2PresetsSection.tsx Core v2 surface for the feature — adds agent query, pill deduplication by agentId, and Import agent dropdown. The agentId-only dedup in isPillAdded lets v1-migrated presets (which lack agentId) show as importable, allowing duplicate rows to be created.
apps/desktop/src/renderer/routes/_authenticated/settings/agents/components/V2AgentsSettings/V2AgentsSettings.tsx Adds deep-link support via initialAgentPresetId prop and a one-shot ref guard. The ref is consumed before confirming a match, so a missing/stale presetId silently drops the deep-link on the first effect run.
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2PresetExecution/useV2PresetExecution.ts Adds vanilla-client agent fetch (correctly bypasses WorkspaceTrpcProvider), builds agentCommandsById map, and resolves live commands at launch with snapshot fallback. The 30-second staleTime creates a window where stale commands can be executed after an agent edit.
apps/desktop/src/renderer/routes/_authenticated/hooks/useMigrateV1PresetsToV2/useMigrateV1PresetsToV2.ts Migration shim extended to seed all AGENT_TYPES into v2 with agentId links. Name-based deduplication correctly avoids collisions with v1-copied rows, but leaves those v1 rows without agentId, which interacts poorly with the Import dropdown's agentId-only dedup check.
apps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/components/PresetsSection/components/PresetEditorDialog/PresetEditorDialog.tsx Adds linked-agent branch: read-only command display with snapshot fallback, and an Open button deep-linking to /settings/agents. v1 callers are unaffected. Logic is clean and handles the missing-agent case gracefully.
apps/desktop/src/renderer/routes/_authenticated/settings/agents/page.tsx Adds validateSearch for the ?agent= param and threads it through to AgentsSettings. Straightforward and correctly typed.
apps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/components/PresetsSection/components/QuickAddPresets/QuickAddPresets.tsx Redesigned from pill buttons to a DropdownMenu with icon, label, and description per item. The alreadyAdded guard is redundant alongside the disabled prop but harmless.
apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.ts Adds optional agentId field to v2TerminalPresetSchema. Minimal, well-commented change with no breaking impact on existing rows.
apps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/components/PresetRow/PresetRow.tsx Adds icon column with name-first then agentId fallback lookup. Uses a type-cast (PresetWithAgent) to access agentId without changing the shared TerminalPreset type; safe since v1 rows simply return undefined for agentId.
apps/desktop/src/renderer/routes/_authenticated/settings/utils/settings-search/settings-search.ts Flips TERMINAL_QUICK_ADD variant from 'v1' to 'shared' so the Import agent dropdown is visible in v2 settings search results.

Sequence Diagram

sequenceDiagram
    participant User
    participant V2PresetsSection
    participant ImportDropdown as QuickAddPresets (Import agent)
    participant Collection as v2TerminalPresets (localStorage)
    participant PresetEditorDialog
    participant AgentsPage as /settings/agents (?agent=presetId)
    participant V2PresetExecution as useV2PresetExecution
    participant ElectronClient as electronTrpcClient (bypasses WorkspaceTrpcProvider)

    Note over Collection: Migration seeds all AGENT_TYPES with agentId links on first run

    User->>V2PresetsSection: Open Terminal settings
    V2PresetsSection->>ElectronClient: getAgentPresets (electronTrpc hook)
    ElectronClient-->>V2PresetsSection: agents[]

    User->>ImportDropdown: Click Import agent
    ImportDropdown->>V2PresetsSection: isPillAdded(pill) check existingAgentIds
    User->>ImportDropdown: Select an agent
    ImportDropdown->>Collection: insertV2Preset with agentId, name, commands

    User->>V2PresetsSection: Click preset row to Edit
    V2PresetsSection->>PresetEditorDialog: open(preset, agents)
    PresetEditorDialog->>PresetEditorDialog: linkedAgent = agents.find by preset.agentId
    PresetEditorDialog-->>User: Show read-only command + Open button

    User->>AgentsPage: Click Open, navigate with ?agent=presetId
    AgentsPage->>AgentsPage: consumedInitialPresetIdRef guard, setSelectedAgentId(match.id)

    User->>V2PresetExecution: Click preset pill in workspace
    V2PresetExecution->>ElectronClient: useQuery getAgentPresets staleTime 30s
    ElectronClient-->>V2PresetExecution: agents[]
    V2PresetExecution->>V2PresetExecution: resolvePresetCommands live command or snapshot fallback
    V2PresetExecution->>V2PresetExecution: state.addTab makeTerminalPane with initialCommand
Loading

Comments Outside Diff (2)

  1. apps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/components/V2PresetsSection/V2PresetsSection.tsx, line 1184-1186 (link)

    P1 Import dropdown shows stale "available" state for v1-migrated presets

    The migration in useMigrateV1PresetsToV2 deduplicates new agent-linked rows by name, so v1 default presets named "Claude", "Codex", etc. are copied without an agentId. But isPillAdded only checks existingAgentIds (filtered from agentId fields), so those unlinked presets don't register as already-added. The result: a user with default v1 preset names will see every builtin agent as importable in the dropdown, and importing one creates a duplicate row alongside the old unlinked preset.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: apps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/components/V2PresetsSection/V2PresetsSection.tsx
    Line: 1184-1186
    
    Comment:
    **Import dropdown shows stale "available" state for v1-migrated presets**
    
    The migration in `useMigrateV1PresetsToV2` deduplicates new agent-linked rows by *name*, so v1 default presets named "Claude", "Codex", etc. are copied without an `agentId`. But `isPillAdded` only checks `existingAgentIds` (filtered from `agentId` fields), so those unlinked presets don't register as already-added. The result: a user with default v1 preset names will see every builtin agent as importable in the dropdown, and importing one creates a duplicate row alongside the old unlinked preset.
    
    How can I resolve this? If you propose a fix, please make it concise.
  2. apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2PresetExecution/useV2PresetExecution.ts, line 253-257 (link)

    P2 30-second stale window can execute the old command

    staleTime: 30_000 means that if a user edits an agent's command in /settings/agents and then runs a linked preset within 30 seconds, agentCommandsById still holds the pre-edit command. The live-link semantics advertised in the PR description ("edits propagate to every preset that points at it") are only guaranteed after the cache expires. Consider a shorter staleTime or invalidating this query when the settings page is visited/exited.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2PresetExecution/useV2PresetExecution.ts
    Line: 253-257
    
    Comment:
    **30-second stale window can execute the old command**
    
    `staleTime: 30_000` means that if a user edits an agent's command in `/settings/agents` and then runs a linked preset within 30 seconds, `agentCommandsById` still holds the pre-edit command. The live-link semantics advertised in the PR description ("edits propagate to every preset that points at it") are only guaranteed after the cache expires. Consider a shorter `staleTime` or invalidating this query when the settings page is visited/exited.
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
apps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/components/V2PresetsSection/V2PresetsSection.tsx:1184-1186
**Import dropdown shows stale "available" state for v1-migrated presets**

The migration in `useMigrateV1PresetsToV2` deduplicates new agent-linked rows by *name*, so v1 default presets named "Claude", "Codex", etc. are copied without an `agentId`. But `isPillAdded` only checks `existingAgentIds` (filtered from `agentId` fields), so those unlinked presets don't register as already-added. The result: a user with default v1 preset names will see every builtin agent as importable in the dropdown, and importing one creates a duplicate row alongside the old unlinked preset.

### Issue 2 of 3
apps/desktop/src/renderer/routes/_authenticated/settings/agents/components/V2AgentsSettings/V2AgentsSettings.tsx:135-142
**Deep-link ref consumed before confirming a match**

`consumedInitialPresetIdRef.current = true` is set unconditionally before checking whether a matching config exists. If `configs` is non-empty but doesn't yet contain the target `presetId` (e.g., the linked agent was deleted, or the preset's `agentId` no longer matches any `config.presetId`), the ref is permanently consumed on the first effect run. Every subsequent `configs` update—including one that would have matched—skips the deep-link silently, and the user lands on the first agent instead of the intended one.

```suggestion
		if (initialAgentPresetId && !consumedInitialPresetIdRef.current) {
			const match = configs.find((c) => c.presetId === initialAgentPresetId);
			if (match) {
				consumedInitialPresetIdRef.current = true;
				setSelectedAgentId(match.id);
				return;
			}
		}
```

### Issue 3 of 3
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2PresetExecution/useV2PresetExecution.ts:253-257
**30-second stale window can execute the old command**

`staleTime: 30_000` means that if a user edits an agent's command in `/settings/agents` and then runs a linked preset within 30 seconds, `agentCommandsById` still holds the pre-edit command. The live-link semantics advertised in the PR description ("edits propagate to every preset that points at it") are only guaranteed after the cache expires. Consider a shorter `staleTime` or invalidating this query when the settings page is visited/exited.

Reviews (1): Last reviewed commit: "feat(desktop): deep-link agents settings..." | Re-trigger Greptile

Comment on lines +135 to +142
if (initialAgentPresetId && !consumedInitialPresetIdRef.current) {
consumedInitialPresetIdRef.current = true;
const match = configs.find((c) => c.presetId === initialAgentPresetId);
if (match) {
setSelectedAgentId(match.id);
return;
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Deep-link ref consumed before confirming a match

consumedInitialPresetIdRef.current = true is set unconditionally before checking whether a matching config exists. If configs is non-empty but doesn't yet contain the target presetId (e.g., the linked agent was deleted, or the preset's agentId no longer matches any config.presetId), the ref is permanently consumed on the first effect run. Every subsequent configs update—including one that would have matched—skips the deep-link silently, and the user lands on the first agent instead of the intended one.

Suggested change
if (initialAgentPresetId && !consumedInitialPresetIdRef.current) {
consumedInitialPresetIdRef.current = true;
const match = configs.find((c) => c.presetId === initialAgentPresetId);
if (match) {
setSelectedAgentId(match.id);
return;
}
}
if (initialAgentPresetId && !consumedInitialPresetIdRef.current) {
const match = configs.find((c) => c.presetId === initialAgentPresetId);
if (match) {
consumedInitialPresetIdRef.current = true;
setSelectedAgentId(match.id);
return;
}
}
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/settings/agents/components/V2AgentsSettings/V2AgentsSettings.tsx
Line: 135-142

Comment:
**Deep-link ref consumed before confirming a match**

`consumedInitialPresetIdRef.current = true` is set unconditionally before checking whether a matching config exists. If `configs` is non-empty but doesn't yet contain the target `presetId` (e.g., the linked agent was deleted, or the preset's `agentId` no longer matches any `config.presetId`), the ref is permanently consumed on the first effect run. Every subsequent `configs` update—including one that would have matched—skips the deep-link silently, and the user lands on the first agent instead of the intended one.

```suggestion
		if (initialAgentPresetId && !consumedInitialPresetIdRef.current) {
			const match = configs.find((c) => c.presetId === initialAgentPresetId);
			if (match) {
				consumedInitialPresetIdRef.current = true;
				setSelectedAgentId(match.id);
				return;
			}
		}
```

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/desktop/plans/20260505-agent-preset-import-learnings.md`:
- Around line 41-42: Update the stale doc text that claims "/settings/agents"
doesn't accept an agent search param: change the copy to reflect that the PR now
supports deep-linking via the query param "?agent=<presetId>" and remove or
reword any sentence stating it "doesn't (yet)" accept a search param; apply the
same update to the other occurrence of this claim in the document (the paragraph
referencing "/settings/agents" and the lines mentioning acceptance of an agent
search param).

In
`@apps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/components/PresetsSection/components/PresetEditorDialog/PresetEditorDialog.tsx`:
- Around line 337-345: The displayed read-only commands in PresetEditorDialog
are unselectable because body disables user-select; update the rendering of
liveCommands inside the div that maps over liveCommands (the element containing
key={cmd}) to opt into selection and pointer cursor by adding the classes
select-text and cursor-text and replace/trade `truncate` with a wrapping class
that allows full visible text (e.g., use `break-all` or remove `truncate`) so
the command text is fully selectable and copyable while preserving styling of
the parent container.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: fb2dcacf-b389-4ffa-bdb5-f437ae69f338

📥 Commits

Reviewing files that changed from the base of the PR and between e119d15 and 925f58f.

📒 Files selected for processing (16)
  • apps/desktop/plans/20260505-agent-preset-import-learnings.md
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2PresetExecution/useV2PresetExecution.ts
  • apps/desktop/src/renderer/routes/_authenticated/hooks/useMigrateV1PresetsToV2/useMigrateV1PresetsToV2.ts
  • apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.ts
  • apps/desktop/src/renderer/routes/_authenticated/settings/agents/components/AgentsSettings/AgentsSettings.tsx
  • apps/desktop/src/renderer/routes/_authenticated/settings/agents/components/V2AgentsSettings/V2AgentsSettings.tsx
  • apps/desktop/src/renderer/routes/_authenticated/settings/agents/page.tsx
  • apps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/TerminalSettings.tsx
  • apps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/components/PresetRow/PresetRow.tsx
  • apps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/components/PresetsSection/PresetsSection.tsx
  • apps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/components/PresetsSection/components/PresetEditorDialog/PresetEditorDialog.tsx
  • apps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/components/PresetsSection/components/PresetsTable/PresetsTable.tsx
  • apps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/components/PresetsSection/components/QuickAddPresets/QuickAddPresets.tsx
  • apps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/components/PresetsSection/components/QuickAddPresets/index.ts
  • apps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/components/V2PresetsSection/V2PresetsSection.tsx
  • apps/desktop/src/renderer/routes/_authenticated/settings/utils/settings-search/settings-search.ts

Comment on lines +41 to +42
exists; a deep-link to the agent settings page requires the agents page to
support a search param, which it doesn't (yet).
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Doc is stale about agent deep-link support.

Line 41–42 and Line 220–221 state /settings/agents doesn’t accept an agent search param yet, but this PR now adds ?agent=<presetId>. Please update these lines to avoid misleading future contributors.

Suggested doc update
- support a search param, which it doesn't (yet).
+ support a search param, which is now implemented via `?agent=<presetId>`.

- Deep-linking the "Open" button to a specific agent on `/settings/agents`
- (route doesn't accept an agent search param yet).
+ Additional deep-link UX enhancements beyond `?agent=<presetId>` (if any).

Also applies to: 220-221

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/desktop/plans/20260505-agent-preset-import-learnings.md` around lines 41
- 42, Update the stale doc text that claims "/settings/agents" doesn't accept an
agent search param: change the copy to reflect that the PR now supports
deep-linking via the query param "?agent=<presetId>" and remove or reword any
sentence stating it "doesn't (yet)" accept a search param; apply the same update
to the other occurrence of this claim in the document (the paragraph referencing
"/settings/agents" and the lines mentioning acceptance of an agent search
param).

Previously the v2 launcher and editor read the main-process agent catalog
(electronTrpc.settings.getAgentPresets), but the v2 /settings/agents page
edits a separate host-service table. Edits there never propagated.

Switch v2-only paths (V2PresetsSection, useV2PresetExecution,
PresetEditorDialog when given agents) to read from useV2AgentConfigs,
which the agents settings page already invalidates on every mutation.
agentId on a preset is now interpreted as HostAgentConfigDto.presetId.

Also fix duplicate rows on first migration: when a v1 row's name matches
a builtin agent id ("claude", "codex"), upgrade it in place to the
linked label instead of seeding a parallel agent-linked row.

Long commands in the linked-preset dialog now wrap (break-all) instead
of forcing nowrap and overflowing the modal.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
apps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/components/PresetsSection/components/PresetEditorDialog/PresetEditorDialog.tsx (1)

329-342: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

select-text cursor-text still missing on read-only command block

The outer container and inner <div> elements still lack select-text cursor-text, so user-select: none on body prevents the user from copying the displayed command. break-all was added (good), but the selectability fix is still missing.

🛠 Proposed fix
-<div className="min-w-0 rounded-md border border-border bg-muted/30 px-3 py-2 font-mono text-xs">
+<div className="min-w-0 rounded-md border border-border bg-muted/30 px-3 py-2 font-mono text-xs select-text cursor-text">

As per coding guidelines: "Error text must be selectable by users with explicit select-text cursor-text classes; renderer sets user-select: none on body."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@apps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/components/PresetsSection/components/PresetEditorDialog/PresetEditorDialog.tsx`
around lines 329 - 342, The read-only command block in PresetEditorDialog (the
outer div with className "min-w-0 rounded-md border border-border bg-muted/30
px-3 py-2 font-mono text-xs" and the inner command-containing divs rendered in
liveCommands.map) must explicitly include the select-text and cursor-text
classes so text becomes selectable despite body-level user-select: none; update
the outer container and the inner command div (and the fallback "—" div) to
append " select-text cursor-text" to their className strings.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/components/PresetsSection/components/PresetEditorDialog/PresetEditorDialog.tsx`:
- Around line 329-342: The read-only command block in PresetEditorDialog (the
outer div with className "min-w-0 rounded-md border border-border bg-muted/30
px-3 py-2 font-mono text-xs" and the inner command-containing divs rendered in
liveCommands.map) must explicitly include the select-text and cursor-text
classes so text becomes selectable despite body-level user-select: none; update
the outer container and the inner command div (and the fallback "—" div) to
append " select-text cursor-text" to their className strings.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a33b1a97-2a45-4c4d-b797-10aa1e37c41d

📥 Commits

Reviewing files that changed from the base of the PR and between 925f58f and f94e990.

📒 Files selected for processing (4)
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2PresetExecution/useV2PresetExecution.ts
  • apps/desktop/src/renderer/routes/_authenticated/hooks/useMigrateV1PresetsToV2/useMigrateV1PresetsToV2.ts
  • apps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/components/PresetsSection/components/PresetEditorDialog/PresetEditorDialog.tsx
  • apps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/components/V2PresetsSection/V2PresetsSection.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/desktop/src/renderer/routes/_authenticated/hooks/useMigrateV1PresetsToV2/useMigrateV1PresetsToV2.ts

@Kitenite Kitenite merged commit 73025b4 into main May 5, 2026
10 checks passed
@Kitenite Kitenite deleted the agent-presets-import branch May 5, 2026 20:49
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2026

🧹 Preview Cleanup Complete

The following preview resources have been cleaned up:

  • ✅ Neon database branch

Thank you for your contribution! 🎉

saddlepaddle pushed a commit that referenced this pull request May 6, 2026
…4101)

* feat(desktop): import agents as v2 terminal presets with live link

Adds an Import-agent dropdown to v2 terminal preset settings, seeds all
builtin terminal agents on first migration, and live-resolves the
linked agent's command at launch (snapshot fallback when missing or
disabled). v1 data layer is untouched; shared UI components accept the
new shape without affecting v1 behavior.

* feat(desktop): deep-link agents settings to a specific agent

The /settings/agents route now accepts ?agent=<presetId>. v2 selects the
matching host config on first mount; v1 ignores the param. The preset
editor dialog's "Open" link passes the linked agentId so users land on
the right agent.

* fix(desktop): live-link v2 presets to host-service agent configs

Previously the v2 launcher and editor read the main-process agent catalog
(electronTrpc.settings.getAgentPresets), but the v2 /settings/agents page
edits a separate host-service table. Edits there never propagated.

Switch v2-only paths (V2PresetsSection, useV2PresetExecution,
PresetEditorDialog when given agents) to read from useV2AgentConfigs,
which the agents settings page already invalidates on every mutation.
agentId on a preset is now interpreted as HostAgentConfigDto.presetId.

Also fix duplicate rows on first migration: when a v1 row's name matches
a builtin agent id ("claude", "codex"), upgrade it in place to the
linked label instead of seeding a parallel agent-linked row.

Long commands in the linked-preset dialog now wrap (break-all) instead
of forcing nowrap and overflowing the modal.

* fix(desktop): retry agent deep-link until match found
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