Skip to content

feat: add ability to pin agents which will be visible everytime user …#1353

Open
AniketR10 wants to merge 2 commits intogeneralaction:mainfrom
AniketR10:feat/pin-agents
Open

feat: add ability to pin agents which will be visible everytime user …#1353
AniketR10 wants to merge 2 commits intogeneralaction:mainfrom
AniketR10:feat/pin-agents

Conversation

@AniketR10
Copy link

@AniketR10 AniketR10 commented Mar 7, 2026

…creates a new Task

Summary

Briefly describe what this PR does and why

Fixes

Fixes #1273

Snapshot

image

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Chore (refactoring code, technical debt, workflow improvements)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (does not change functionality, e.g. code style improvements, linting)
  • This change requires a documentation update

Mandatory Tasks

  • I have self-reviewed the code
  • A decent size PR without self-review might be rejected

Checklist

  • I have read the contributing guide
  • My code follows the style guidelines of this project (pnpm run format)
  • I have commented my code, particularly in hard-to-understand areas
  • I have checked if my PR needs changes to the documentation
  • I have checked if my changes generate no new warnings (pnpm run lint)
  • I have added tests that prove my fix is effective or that my feature works
  • I haven't checked if new and existing unit tests pass locally with my changes

@vercel
Copy link

vercel bot commented Mar 7, 2026

@AniketR10 is attempting to deploy a commit to the General Action Team on Vercel.

A member of the Team first needs to authorize it.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 7, 2026

Greptile Summary

This PR adds the ability to pin agents in the MultiAgentDropdown, persisting the preference via rpc.appSettings. Pinned agents are sorted to the top of the list and a filled Pin icon distinguishes them from unpinned agents. The feature integrates cleanly with the existing settings infrastructure (AppSettings, normalizeSettings, DEFAULT_SETTINGS).

Key issues found:

  • Optimistic update without rollback: setPinnedAgents(next) is called before rpc.appSettings.update. If the RPC call fails the UI diverges from persisted state with no recovery path, causing pins to silently disappear on the next app launch.
  • Misleading comment: The pinnedAgents state comment says "using localStorage" but the implementation uses the Electron app settings via rpc.
  • Missing deduplication: The normalizeSettings block for pinnedAgents omits the new Set(...) deduplication that the adjacent hiddenOpenInApps block applies, allowing duplicate IDs to survive in persisted settings.

Confidence Score: 3/5

  • Safe to review further but the optimistic-update bug should be fixed before merging to avoid silent pin-loss on settings save failures.
  • The settings integration is solid and the UI logic is clear, but the missing rollback on rpc.appSettings.update failure is a real user-facing bug. The other two issues (misleading comment and missing deduplication) are lower severity but add up to a PR that needs minor fixes before merge.
  • Pay close attention to src/renderer/components/MultiAgentDropdown.tsx, specifically the togglePin function's error-handling path.

Important Files Changed

Filename Overview
src/renderer/components/MultiAgentDropdown.tsx Adds pin/unpin UI to the agent dropdown with optimistic state updates, but the state is not rolled back when the settings save fails, and a comment incorrectly references localStorage instead of app settings.
src/main/settings.ts Adds pinnedAgents field to AppSettings interface, default settings, and normalization — correctly typed and guarded, but missing deduplication that the adjacent hiddenOpenInApps block applies.

Sequence Diagram

sequenceDiagram
    participant U as User
    participant D as MultiAgentDropdown
    participant RPC as rpc.appSettings
    participant S as settings.ts (main)

    U->>D: Opens dropdown (component mounts)
    D->>RPC: get()
    RPC->>S: getAppSettings()
    S-->>RPC: AppSettings { pinnedAgents: [...] }
    RPC-->>D: settings
    D-->>D: setPinnedAgents(settings.pinnedAgents)

    U->>D: Clicks Pin button for an agent
    D-->>D: setPinnedAgents(next) [optimistic]
    D->>RPC: update({ pinnedAgents: next })
    RPC->>S: updateAppSettings(partial)
    S-->>S: normalizeSettings() → persists to disk
    alt Save succeeds
        RPC-->>D: updated settings
    else Save fails
        RPC-->>D: throws error
        D-->>D: console.error (no state rollback ⚠️)
    end
Loading

Last reviewed commit: 467935b

Comment on lines +50 to +56
setPinnedAgents(next);

try {
await rpc.appSettings.update({ pinnedAgents: next });
} catch (error) {
console.error('Failed to save pinned agents', error);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

No rollback on settings save failure

setPinnedAgents(next) is called optimistically before the async rpc.appSettings.update call. If the update fails, the UI reflects the pin as toggled but the setting is never persisted — on the next app launch (or the next time this component mounts) the pin will silently disappear, leaving the user confused. The state should be rolled back when the call fails.

Suggested change
setPinnedAgents(next);
try {
await rpc.appSettings.update({ pinnedAgents: next });
} catch (error) {
console.error('Failed to save pinned agents', error);
}
setPinnedAgents(next);
try {
await rpc.appSettings.update({ pinnedAgents: next });
} catch (error) {
console.error('Failed to save pinned agents', error);
setPinnedAgents(pinnedAgents); // rollback optimistic update
}

Comment on lines +28 to +29
// Setup state for pinned agents (using localStorage to remember preferences)
const [pinnedAgents, setPinnedAgents] = useState<string[]>([]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Misleading comment references localStorage

The comment says "using localStorage to remember preferences" but the implementation reads from and writes to the Electron app settings via rpc.appSettings, not localStorage. This could mislead future contributors.

Suggested change
// Setup state for pinned agents (using localStorage to remember preferences)
const [pinnedAgents, setPinnedAgents] = useState<string[]>([]);
// Setup state for pinned agents (persisted in app settings)
const [pinnedAgents, setPinnedAgents] = useState<string[]>([]);

Comment on lines +535 to +541
// Pinned Agents
const rawPinned = (input as any)?.pinnedAgents;
if (Array.isArray(rawPinned)) {
out.pinnedAgents = rawPinned.filter((item) => typeof item === 'string');
} else {
out.pinnedAgents = [];
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing deduplication for pinnedAgents

The hiddenOpenInApps normalization immediately above uses [...new Set(validated)] to deduplicate entries. pinnedAgents skips this step, so a corrupt or manually edited settings file could contain duplicate agent IDs. Duplicates won't break rendering but will cause an agent to appear pinned even after being "unpinned" (since filter only removes it once per call).

Suggested change
// Pinned Agents
const rawPinned = (input as any)?.pinnedAgents;
if (Array.isArray(rawPinned)) {
out.pinnedAgents = rawPinned.filter((item) => typeof item === 'string');
} else {
out.pinnedAgents = [];
}
// Pinned Agents
const rawPinned = (input as any)?.pinnedAgents;
if (Array.isArray(rawPinned)) {
out.pinnedAgents = [...new Set(rawPinned.filter((item) => typeof item === 'string'))];
} else {
out.pinnedAgents = [];
}

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.

[feat]: Allow users to pin agents in selection list

1 participant