refactor: agent provider simplification (#1286)

* docs: add spec for agent provider

* refactor(agent): remove BaseAgentProvider layer and simplify provider hierarchy

Author: zerob13

docs/specs/agent-provider-simplification/plan.md

@@ -0,0 +1,68 @@
+# Plan: Agent Provider Simplification (ACP-only)
+
+## Summary
+
+Replace the “agent provider” abstraction and detection logic with a single explicit rule: **ACP is the only agent provider and is identified by `providerId === 'acp'`.**
+
+## Current Call Flow (relevant parts)
+
+- Main:
+  - `ProviderInstanceManager.createProviderInstance()` already special-cases `provider.id === 'acp'`.
+  - `ProviderInstanceManager.isAgentProvider()` uses `instanceof BaseAgentProvider` and (if instance not created) a constructor prototype check (`isAgentConstructor`).
+  - `LLMProviderPresenter.isAgentProvider()` exposes this to the renderer via `ILlmProviderPresenter`.
+- Renderer:
+  - `src/renderer/src/stores/modelStore.ts` calls `llmproviderPresenter.isAgentProvider(providerId)` over IPC to choose between:
+    - `agentModelStore.refreshAgentModels(providerId)` (ACP path)
+    - `refreshStandardModels + refreshCustomModels` (standard path)
+  - Other renderer logic already treats ACP as special via `provider.id === 'acp'`.
+
+## Proposed Changes
+
+### 1) Remove agent-provider classification API
+
+- Remove `isAgentProvider(providerId: string)` from:
+  - `src/shared/types/presenters/llmprovider.presenter.d.ts`
+  - `src/shared/types/presenters/legacy.presenters.d.ts`
+  - `src/main/presenter/llmProviderPresenter/index.ts`
+  - `src/main/presenter/llmProviderPresenter/managers/providerInstanceManager.ts`
+
+Rationale: It is only used by the renderer for ACP gating, and ACP can be identified locally by ID.
+
+### 2) Replace renderer gating with an explicit ACP check
+
+- In `src/renderer/src/stores/modelStore.ts`:
+  - Remove the async IPC call `llmP.isAgentProvider(providerId)`.
+  - Replace with a local predicate: `providerId === 'acp'`.
+  - Keep the existing ACP refresh path using `agentModelStore.refreshAgentModels('acp')` (no behavioral change).
+
+### 3) Remove `BaseAgentProvider` (optional but preferred)
+
+Because `BaseAgentProvider` is only used by `AcpProvider`, delete the base class and:
+
+- Make `AcpProvider` extend `BaseLLMProvider` directly.
+- Move `cleanup()` logic into `AcpProvider` (or delegate to `AcpSessionManager` / `AcpProcessManager`).
+- Ensure `cleanup()` is safe to call multiple times and during shutdown.
+
+Notes:
+- `acpCleanupHook` currently awaits `cleanup()` even though `BaseAgentProvider.cleanup()` is `void`. Consider standardizing ACP cleanup to `Promise<void>` to match usage.
+
+## Compatibility / Migration
+
+- No user data migration.
+- Provider ID `acp` remains unchanged and is treated as a stable internal contract.
+- Any internal IPC typing generation must be updated to reflect removal of `isAgentProvider`.
+
+## Test Strategy
+
+Add minimal tests focusing on the only behavioral dependency (renderer model refresh selection):
+
+- Renderer unit test for `modelStore.refreshProviderModels()`:
+  - When `providerId === 'acp'`, it uses `agentModelStore.refreshAgentModels`.
+  - When `providerId !== 'acp'`, it uses standard refresh path.
+
+Main-process unit tests are optional; the change is mostly removal and ACP-id checks.
+
+## Rollout
+
+Single PR is acceptable if changes stay localized (types + modelStore + ACP provider base class cleanup).
+

docs/specs/agent-provider-simplification/spec.md

@@ -0,0 +1,43 @@
+# Agent Provider Simplification (ACP-only)
+
+## Background
+
+DeepChat currently distinguishes between:
+
+- **LLM providers**: network-backed providers that implement `BaseLLMProvider` (OpenAI/Anthropic/etc).
+- **Agent providers**: providers that manage local agent sessions/processes (currently only `acp` via `AcpProvider`).
+
+The codebase implements this distinction via a dedicated base class (`BaseAgentProvider`) and a runtime/type-detection API (`isAgentProvider`), which is then consumed from the renderer via IPC.
+
+## Problem
+
+- `BaseAgentProvider` is only used by `AcpProvider`, so the abstraction adds indirection without real reuse.
+- Provider type detection is over-engineered (`isAgentConstructor` + prototype checks) and duplicates existing ACP-specific branching.
+- The renderer calls `llmproviderPresenter.isAgentProvider(providerId)` over IPC, but the only “agent provider” is `providerId === 'acp'`. This creates unnecessary main↔renderer coupling and call complexity.
+
+## Goals
+
+- Treat **ACP as the only agent provider** and identify it **only by `providerId === 'acp'`**.
+- Remove the generic “agent provider type detection” path and the renderer IPC dependency for this decision.
+- Keep user-visible behavior unchanged:
+  - ACP agents still appear as selectable models when ACP is enabled.
+  - Non-ACP providers keep the standard model/custom-model refresh behavior.
+  - Shutdown and provider disable still clean up ACP resources.
+
+## Non-goals
+
+- Supporting multiple agent providers beyond ACP.
+- Redesigning ACP model derivation (agents-as-models) or session/workspace semantics.
+- Changing persisted provider IDs or stored settings schemas.
+
+## Acceptance Criteria
+
+- Renderer no longer calls `llmproviderPresenter.isAgentProvider(...)`; ACP decision is local (`providerId === 'acp'`).
+- Main process no longer needs `isAgentConstructor` / prototype-based provider classification.
+- No remaining runtime dependency on `BaseAgentProvider` for correctness (ACP cleanup remains correct).
+- `pnpm run typecheck`, `pnpm test`, `pnpm run lint` pass.
+
+## Open Questions
+
+- None.
+

docs/specs/agent-provider-simplification/tasks.md

@@ -0,0 +1,26 @@
+# Tasks: Agent Provider Simplification (ACP-only)
+
+1. Update renderer to stop using IPC for agent-provider detection
+   - Remove `llmproviderPresenter.isAgentProvider` usage from `src/renderer/src/stores/modelStore.ts`.
+   - Gate ACP behavior by `providerId === 'acp'`.
+
+2. Remove `isAgentProvider` from the presenter contract
+   - Remove from `src/shared/types/presenters/llmprovider.presenter.d.ts`.
+   - Remove from `src/shared/types/presenters/legacy.presenters.d.ts`.
+   - Remove implementation from `src/main/presenter/llmProviderPresenter/index.ts`.
+
+3. Remove main-side agent-provider classification implementation
+   - Delete `ProviderInstanceManager.isAgentProvider()` and `isAgentConstructor()` in `src/main/presenter/llmProviderPresenter/managers/providerInstanceManager.ts`.
+   - Ensure no other code path depends on `BaseAgentProvider` type checks.
+
+4. Remove `BaseAgentProvider` abstraction (preferred)
+   - Delete `src/main/presenter/llmProviderPresenter/baseAgentProvider.ts`.
+   - Update `src/main/presenter/llmProviderPresenter/providers/acpProvider.ts` to extend `BaseLLMProvider` directly.
+   - Keep/adjust ACP cleanup semantics (safe shutdown, provider disable, app quit).
+
+5. Add/adjust tests
+   - Add a Vitest suite under `test/renderer/**` validating model refresh selection for ACP vs non-ACP.
+
+6. Quality gates
+   - Run `pnpm run format`, `pnpm run lint`, `pnpm run typecheck`, and `pnpm test`.
+

src/main/presenter/llmProviderPresenter/baseAgentProvider.ts

@@ -118,10 +118,6 @@ export class LLMProviderPresenter implements ILlmProviderPresenter {
     return this.providerInstanceManager.getProviderById(id)
   }
 
-  isAgentProvider(providerId: string): boolean {
-    return this.providerInstanceManager.isAgentProvider(providerId)
-  }
-
   async setCurrentProvider(providerId: string): Promise<void> {
     // 如果有正在生成的流，先停止它们
     await this.stopAllStreams()

src/main/presenter/llmProviderPresenter/index.ts

@@ -1,7 +1,6 @@
 import { ProviderBatchUpdate, ProviderChange } from '@shared/provider-operations'
 import { IConfigPresenter, LLM_PROVIDER } from '@shared/presenter'
 import { BaseLLMProvider } from '../baseProvider'
-import { BaseAgentProvider } from '../baseAgentProvider'
 import { OpenAIProvider } from '../providers/openAIProvider'
 import { DeepseekProvider } from '../providers/deepseekProvider'
 import { SiliconcloudProvider } from '../providers/siliconcloudProvider'
@@ -134,11 +133,6 @@ export class ProviderInstanceManager {
     ])
   }
 
-  private static isAgentConstructor(ctor?: ProviderConstructor): boolean {
-    if (!ctor) return false
-    return BaseAgentProvider.prototype.isPrototypeOf(ctor.prototype)
-  }
-
   init(): void {
     const providers = this.options.configPresenter.getProviders()
     for (const provider of providers) {
@@ -250,24 +244,6 @@ export class ProviderInstanceManager {
     return instance
   }
 
-  isAgentProvider(providerId: string): boolean {
-    const instance = this.providerInstances.get(providerId)
-    if (instance) {
-      return instance instanceof BaseAgentProvider
-    }
-
-    const provider = this.providers.get(providerId)
-    if (!provider) {
-      return false
-    }
-
-    const ProviderClass =
-      ProviderInstanceManager.PROVIDER_ID_MAP.get(provider.id) ??
-      ProviderInstanceManager.PROVIDER_TYPE_MAP.get(provider.apiType)
-
-    return ProviderInstanceManager.isAgentConstructor(ProviderClass)
-  }
-
   private handleProviderAdd(change: ProviderChange): void {
     if (!change.provider) return
 

src/main/presenter/llmProviderPresenter/managers/providerInstanceManager.ts

@@ -1,6 +1,5 @@
 import type * as schema from '@agentclientprotocol/sdk/dist/schema.js'
-import { SUMMARY_TITLES_PROMPT } from '../baseProvider'
-import { BaseAgentProvider } from '../baseAgentProvider'
+import { BaseLLMProvider, SUMMARY_TITLES_PROMPT } from '../baseProvider'
 import type {
   ChatMessage,
   LLMResponse,
@@ -58,12 +57,7 @@ type PendingPermissionState = {
   reject: (error: Error) => void
 }
 
-export class AcpProvider extends BaseAgentProvider<
-  AcpSessionManager,
-  AcpProcessManager,
-  schema.RequestPermissionRequest,
-  schema.RequestPermissionResponse
-> {
+export class AcpProvider extends BaseLLMProvider {
   private readonly processManager: AcpProcessManager
   private readonly sessionManager: AcpSessionManager
   private readonly sessionPersistence: AcpSessionPersistence
@@ -102,21 +96,6 @@ export class AcpProvider extends BaseAgentProvider<
     void this.initWhenEnabled()
   }
 
-  protected getSessionManager(): AcpSessionManager {
-    return this.sessionManager
-  }
-
-  protected getProcessManager(): AcpProcessManager {
-    return this.processManager
-  }
-
-  protected async requestPermission(
-    params: schema.RequestPermissionRequest
-  ): Promise<schema.RequestPermissionResponse> {
-    void params
-    return { outcome: { outcome: 'cancelled' } }
-  }
-
   protected async fetchProviderModels(): Promise<MODEL_META[]> {
     try {
       const acpEnabled = await this.configPresenter.getAcpEnabled()

src/main/presenter/llmProviderPresenter/providers/acpProvider.ts

@@ -38,14 +38,6 @@ export const useModelStore = defineStore('model', () => {
   const customModelQueries = new Map<string, ModelQueryHandle<MODEL_META[]>>()
   const enabledModelQueries = new Map<string, ModelQueryHandle<RENDERER_MODEL_META[]>>()
   const queryCache = useQueryCache()
-  const isAgentProvider = async (providerId: string): Promise<boolean> => {
-    try {
-      return Boolean(await llmP.isAgentProvider(providerId))
-    } catch (error) {
-      console.warn(`[ModelStore] Failed to determine provider type: ${providerId}`, error)
-      return false
-    }
-  }
 
   const matchesProviderModelsEntry = (
     entry: { key: readonly unknown[] },
@@ -436,7 +428,7 @@ export const useModelStore = defineStore('model', () => {
   }
 
   const refreshProviderModels = async (providerId: string) => {
-    if (await isAgentProvider(providerId)) {
+    if (providerId === 'acp') {
       try {
         const { rendererModels, modelMetas } = await agentModelStore.refreshAgentModels(providerId)
         updateProviderModelsCache(providerId, modelMetas)

src/renderer/src/stores/modelStore.ts

@@ -946,7 +946,6 @@ export interface ILlmProviderPresenter {
   setProviders(provider: LLM_PROVIDER[]): void
   getProviders(): LLM_PROVIDER[]
   getProviderById(id: string): LLM_PROVIDER
-  isAgentProvider(providerId: string): boolean
   getExistingProviderInstance?(providerId: string): unknown
   getModelList(providerId: string): Promise<MODEL_META[]>
   updateModelStatus(providerId: string, modelId: string, enabled: boolean): Promise<void>

src/shared/types/presenters/legacy.presenters.d.ts

@@ -175,7 +175,6 @@ export interface ILlmProviderPresenter {
   setProviders(provider: LLM_PROVIDER[]): void
   getProviders(): LLM_PROVIDER[]
   getProviderById(id: string): LLM_PROVIDER
-  isAgentProvider(providerId: string): boolean
   getProviderInstance(providerId: string): unknown
   getExistingProviderInstance(providerId: string): unknown
   getModelList(providerId: string): Promise<MODEL_META[]>