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
21 changes: 21 additions & 0 deletions docs/specs/acp-agent-uninstall/plan.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# ACP Agent Uninstall Plan

## Main Process

- Add `configPresenter.uninstallAcpRegistryAgent(agentId)` as the orchestration entrypoint.
- Keep sqlite persistence in `AgentRepository`.
- Add uninstall cleanup to `AcpLaunchSpecService` for registry install artifacts with path-boundary checks.
- After uninstall, mark the agent disabled and set install state back to `not_installed`.
- Reuse `handleAcpAgentsMutated([agentId])` so ACP processes are released and renderer state refreshes.

## Renderer

- Add uninstall actions in `AcpSettings.vue` for both installed cards and registry overlay rows.
- Confirm uninstall with a lightweight AlertDialog-based confirmation flow (`AlertDialog` component/modal) so the renderer uses the built-in AlertDialog UI for lightweight confirmation.
- Refresh ACP settings data after uninstall completes.

## Tests

- Cover binary uninstall cleanup and safe path handling in `AcpLaunchSpecService`.
- Cover repository/state reset for registry agents.
- Cover renderer uninstall CTA wiring in `AcpSettings.vue`.
25 changes: 25 additions & 0 deletions docs/specs/acp-agent-uninstall/spec.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# ACP Agent Uninstall

## Summary

Add uninstall support for registry-backed ACP agents. The current ACP settings flow can install and enable registry agents, but it cannot uninstall them after they are added.

## User Stories

- As a user, I can uninstall an installed ACP registry agent from settings.
- As a user, uninstall removes local install artifacts and hides the agent from enabled ACP model choices.
- As a user, old sessions referencing that agent are preserved and can recover by reinstalling later.

## Acceptance Criteria

- ACP settings shows an uninstall action for installed registry agents.
- Uninstall deletes local binary install directories when the agent uses a binary distribution.
- Uninstall resets registry install state to `not_installed` and disables the agent.
- Uninstalled registry agents no longer appear in the enabled ACP model list.
- Existing session records keep their `agentId`; uninstall does not delete session history.

## Non-Goals

- No hard-delete of the registry agent row.
- No schema migration.
- No forced rewrite of historical session, remote binding, or subagent references.
18 changes: 18 additions & 0 deletions src/main/presenter/agentRepository/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,24 @@ export class AgentRepository {
return true
}

clearRegistryAcpAgentInstallation(agentId: string, installState: AcpAgentInstallState): boolean {
const row = this.sqlitePresenter.agentsTable.get(agentId)
if (!row || row.agent_type !== 'acp' || row.source !== 'registry') {
return false
}

const state = parseJson<StoredAgentState>(row.state_json) ?? {}
this.sqlitePresenter.agentsTable.update(agentId, {
enabled: false,
stateJson: stringifyJson({
...state,
installState
} satisfies StoredAgentState)
})

return true
}

toAcpAgentConfig(
agentId: string,
preview?: Pick<AcpAgentConfig, 'command' | 'args'>
Expand Down
66 changes: 62 additions & 4 deletions src/main/presenter/configPresenter/acpLaunchSpecService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,15 @@ const ensureDir = (dir: string): void => {
}
}

const isPathWithinRoot = (candidatePath: string, rootPath: string, allowEqual = false): boolean => {
const relative = path.relative(rootPath, candidatePath)
if (!relative) {
return allowEqual
}

return !relative.startsWith('..') && !path.isAbsolute(relative)
}

const listFilesRecursive = (root: string): string[] => {
if (!fs.existsSync(root)) {
return []
Expand Down Expand Up @@ -218,6 +227,28 @@ export class AcpLaunchSpecService {
}
}

async uninstallRegistryAgent(
agent: AcpRegistryAgent,
installState: AcpAgentInstallState | null
): Promise<void> {
const selection = this.selectRegistryDistribution(agent)
if (selection?.type !== 'binary') {
return
}

const candidateDirs = new Set<string>()
if (typeof installState?.installDir === 'string' && installState.installDir.trim()) {
candidateDirs.add(path.resolve(installState.installDir))
}
candidateDirs.add(this.getBinaryInstallDir(agent))

for (const installDir of candidateDirs) {
this.removeInstallDir(installDir)
}

this.pruneEmptyAgentInstallRoot(agent.id)
}

async resolveRegistryLaunchSpec(
agent: AcpRegistryAgent,
installState: AcpAgentInstallState | null
Expand Down Expand Up @@ -289,10 +320,7 @@ export class AcpLaunchSpecService {
const resolvedInstallDir = path.resolve(installDir)
const resolvedInstallRoot = path.resolve(this.installRoot)

if (
resolvedInstallDir !== resolvedInstallRoot &&
!resolvedInstallDir.startsWith(`${resolvedInstallRoot}${path.sep}`)
) {
if (!isPathWithinRoot(resolvedInstallDir, resolvedInstallRoot)) {
throw new Error(`Unsafe ACP install directory for ${agent.id}@${agent.version}`)
}

Expand All @@ -310,6 +338,36 @@ export class AcpLaunchSpecService {
return listFilesRecursive(installDir).find((file) => path.basename(file) === basename) ?? null
}

private removeInstallDir(installDir: string): void {
const resolvedInstallDir = path.resolve(installDir)
const resolvedInstallRoot = path.resolve(this.installRoot)

if (!isPathWithinRoot(resolvedInstallDir, resolvedInstallRoot)) {
throw new Error(`Unsafe ACP install directory for uninstall: ${installDir}`)
}

if (!fs.existsSync(resolvedInstallDir)) {
return
}

fs.rmSync(resolvedInstallDir, { recursive: true, force: true })
}

private pruneEmptyAgentInstallRoot(agentId: string): void {
const safeAgentId = sanitizeInstallSegment(agentId, 'agent id')
const agentRoot = path.resolve(path.join(this.installRoot, safeAgentId))
const resolvedInstallRoot = path.resolve(this.installRoot)

if (!isPathWithinRoot(agentRoot, resolvedInstallRoot) || !fs.existsSync(agentRoot)) {
return
}

const remainingEntries = fs.readdirSync(agentRoot)
if (remainingEntries.length === 0) {
fs.rmSync(agentRoot, { recursive: true, force: true })
}
}

private getPlatformKey(): string | null {
const platformMap: Record<string, string> = {
darwin: 'darwin',
Expand Down
29 changes: 29 additions & 0 deletions src/main/presenter/configPresenter/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1635,6 +1635,35 @@ export class ConfigPresenter implements IConfigPresenter {
}
}

async uninstallAcpRegistryAgent(agentId: string): Promise<void> {
const resolvedId = resolveAcpAgentAlias(agentId)
const registryAgent = this.getRegistryAgentOrThrow(resolvedId)
const currentState = this.getAgentRepositoryOrThrow().getAgentInstallState(registryAgent.id)

await this.acpLaunchSpecService.uninstallRegistryAgent(registryAgent, currentState)

const uninstalledState: AcpAgentInstallState = {
status: 'not_installed',
version: registryAgent.version,
distributionType:
this.acpLaunchSpecService.selectRegistryDistribution(registryAgent)?.type ?? undefined,
lastCheckedAt: Date.now(),
installedAt: null,
installDir: null,
error: null
}

const updated = this.getAgentRepositoryOrThrow().clearRegistryAcpAgentInstallation(
registryAgent.id,
uninstalledState
)
if (!updated) {
throw new Error(`ACP registry agent not found: ${registryAgent.id}`)
}

this.handleAcpAgentsMutated([registryAgent.id])
}

async getAcpAgentInstallStatus(agentId: string): Promise<AcpAgentInstallState | null> {
return this.agentRepository?.getAgentInstallState(resolveAcpAgentAlias(agentId)) ?? null
}
Expand Down
Loading
Loading