Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 68 additions & 0 deletions docs/specs/agent-provider-simplification/plan.md
Original file line number Diff line number Diff line change
@@ -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).

43 changes: 43 additions & 0 deletions docs/specs/agent-provider-simplification/spec.md
Original file line number Diff line number Diff line change
@@ -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.

26 changes: 26 additions & 0 deletions docs/specs/agent-provider-simplification/tasks.md
Original file line number Diff line number Diff line change
@@ -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`.

47 changes: 0 additions & 47 deletions src/main/presenter/llmProviderPresenter/baseAgentProvider.ts

This file was deleted.

4 changes: 0 additions & 4 deletions src/main/presenter/llmProviderPresenter/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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

Expand Down
25 changes: 2 additions & 23 deletions src/main/presenter/llmProviderPresenter/providers/acpProvider.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down
10 changes: 1 addition & 9 deletions src/renderer/src/stores/modelStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[] },
Expand Down Expand Up @@ -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)
Expand Down
1 change: 0 additions & 1 deletion src/shared/types/presenters/legacy.presenters.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
1 change: 0 additions & 1 deletion src/shared/types/presenters/llmprovider.presenter.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[]>
Expand Down
Loading