Skip to content
Open
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
17 changes: 13 additions & 4 deletions src-tauri/src/acp/binary_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,15 +214,24 @@ pub(crate) fn binary_dir(agent_id: &str, version: &str) -> Result<PathBuf, AcpEr
.join(registry::current_platform()))
}

pub fn clear_agent_cache(agent_type: AgentType) -> Result<(), AcpError> {
/// Remove the managed binary tree for `agent_type`.
///
/// `Ok(true)` means codeg actually removed something it owned; `Ok(false)`
/// means there was nothing of codeg's to remove. That distinction is the whole
/// point of the return value: an agent whose binary came from somewhere codeg
/// does not manage (PATH, `~/.local/bin`, a package manager) leaves no managed
/// tree, so clearing the cache removes NOTHING and the agent stays installed
/// and launchable. Reporting that as a successful uninstall is what issue #631
/// is about, so the caller needs to be able to tell the two apart.
pub fn clear_agent_cache(agent_type: AgentType) -> Result<bool, AcpError> {
let agent_id = agent_cache_key(agent_type);
let dir = cache_dir()?.join(&agent_id);
if !dir.exists() {
return Ok(());
return Ok(false);
}

if std::fs::remove_dir_all(&dir).is_ok() {
return Ok(());
return Ok(true);
}

// Windows: a running `<cmd>.exe` (ours or anti-virus scanning it) keeps the
Expand All @@ -242,7 +251,7 @@ pub fn clear_agent_cache(agent_type: AgentType) -> Result<(), AcpError> {
.map_err(|e| AcpError::DownloadFailed(format!("failed to clear cache: {e}")))?;

let _ = std::fs::remove_dir_all(&aside);
Ok(())
Ok(true)
}

/// Best-effort cleanup of trash directories left behind by
Expand Down
111 changes: 103 additions & 8 deletions src-tauri/src/commands/acp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10842,6 +10842,42 @@ pub async fn acp_clear_binary_cache(agent_type: AgentType) -> Result<(), AcpErro
Ok(())
}

/// What an uninstall actually achieved, so the completion line can say it
/// instead of always claiming success.
///
/// `removed_managed` is whether codeg deleted a binary tree it owns.
/// `remaining_version` is what a re-probe found AFTER that removal: it is
/// `Some` exactly when an installation codeg does not manage survived and is
/// still what a connection would launch. The two are independent: an agent
/// installed only outside codeg reports `false` / `Some(..)`, which is the
/// case that used to be reported as "uninstalled successfully" while nothing
/// had been removed and the agent still worked (#631).
#[derive(Debug, Clone, PartialEq, Eq)]
pub(crate) struct AgentUninstallOutcome {
pub removed_managed: bool,
pub remaining_version: Option<String>,
}

/// The install-stream completion line for an uninstall. Pure so the four
/// outcomes can be pinned by a test; every branch has to survive a re-probe
/// that can still find the agent.
fn uninstall_completion_message(agent_name: &str, outcome: &AgentUninstallOutcome) -> String {
match (outcome.removed_managed, outcome.remaining_version.as_deref()) {
(true, None) => format!("{agent_name} uninstalled successfully"),
(true, Some(version)) => format!(
"Removed the copy of {agent_name} codeg manages. Version {version} is still \
installed outside codeg and is what a connection will launch; remove it with \
whatever installed it."
),
(false, Some(version)) => format!(
"Nothing to uninstall: codeg never installed {agent_name}. Version {version} \
comes from outside codeg and is what a connection will launch; remove it with \
whatever installed it."
),
(false, None) => format!("Nothing to uninstall: codeg has no copy of {agent_name}"),
}
}

#[allow(clippy::too_many_arguments)]
pub(crate) async fn acp_update_agent_preferences_core(
agent_type: AgentType,
Expand Down Expand Up @@ -12436,34 +12472,44 @@ pub(crate) async fn acp_uninstall_agent_core(
format!("Uninstalling {}...", meta.name),
);

let result: Result<(), AcpError> = async {
match meta.distribution {
let result: Result<AgentUninstallOutcome, AcpError> = async {
let removed_managed = match meta.distribution {
registry::AgentDistribution::Binary { .. } => {
binary_cache::clear_agent_cache(agent_type)?;
binary_cache::clear_agent_cache(agent_type)?
}
registry::AgentDistribution::Npx { package, .. } => {
uninstall_npm_global_package(package).await?;
true
}
registry::AgentDistribution::Uvx { .. } => {
binary_cache::clear_uvx_agent_prepared(agent_type)?;
true
}
}
};

agent_setting_service::set_installed_version(&db.conn, agent_type, None)
.await
.map_err(|e| AcpError::protocol(e.to_string()))?;
emit_acp_agents_updated(emitter, "agent_uninstalled", Some(agent_type));
Ok(())
// Re-probe AFTER the removal, exactly like the status / list paths do.
// A version still answering here is an install codeg does not manage
// and did not touch, and it is what the next connection will launch,
// so the completion line has to say so rather than report a clean
// uninstall the user never got (#631).
Ok(AgentUninstallOutcome {
removed_managed,
remaining_version: detect_local_version(agent_type).await,
})
}
.await;

match &result {
Ok(()) => {
Ok(outcome) => {
emit_agent_install_event(
emitter,
&task_id,
AgentInstallEventKind::Completed,
format!("{} uninstalled successfully", meta.name),
uninstall_completion_message(meta.name, outcome),
);
}
Err(e) => {
Expand All @@ -12475,7 +12521,7 @@ pub(crate) async fn acp_uninstall_agent_core(
);
}
}
result
result.map(|_| ())
}

#[cfg(feature = "tauri-runtime")]
Expand Down Expand Up @@ -16174,6 +16220,55 @@ wire_api = "chat"
assert!(build_npm_install_spec("cline@3.0.9", Some("latest")).is_err());
}

/// #631: an uninstall used to report "uninstalled successfully" whatever
/// happened, including for an agent codeg had never installed and did not
/// touch: the reporter's OpenCode came from `~/.local/bin` and stayed
/// there, fully launchable, behind a success toast. Only the one outcome
/// that really is a clean uninstall may claim it; the other three have to
/// name the version that survived so the user knows what still runs.
#[test]
fn uninstall_completion_message_reports_what_actually_happened() {
let clean = AgentUninstallOutcome {
removed_managed: true,
remaining_version: None,
};
assert_eq!(
uninstall_completion_message("OpenCode", &clean),
"OpenCode uninstalled successfully"
);

for outcome in [
AgentUninstallOutcome {
removed_managed: true,
remaining_version: Some("1.18.25".into()),
},
AgentUninstallOutcome {
removed_managed: false,
remaining_version: Some("1.18.25".into()),
},
] {
let message = uninstall_completion_message("OpenCode", &outcome);
assert!(
message.contains("1.18.25"),
"a surviving install must be named: {message}"
);
assert!(
!message.contains("uninstalled successfully"),
"an agent that is still installed must not read as uninstalled: {message}"
);
}

let nothing = AgentUninstallOutcome {
removed_managed: false,
remaining_version: None,
};
let message = uninstall_completion_message("OpenCode", &nothing);
assert!(
!message.contains("uninstalled successfully"),
"removing nothing must not read as an uninstall: {message}"
);
}

// The pinned default is byte-identical to what `build_npm_install_spec`
// produced before the channel existed, with no fallback attempt.
#[test]
Expand Down
19 changes: 19 additions & 0 deletions src/components/settings/acp-agent-settings.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {
setClaudeEnvFlagInConfigText,
setHostToolsAgentMode,
showsCodexReadOnlyAcpWarning,
survivingInstallVersion,
} from "./acp-agent-settings"
import { parse as parseTomlDocument } from "smol-toml"
import type {
Expand Down Expand Up @@ -148,6 +149,24 @@ function codexSandboxDraft(
}
}

// #631: uninstall removes only the copy codeg manages. A version still
// answering a re-probe afterwards belongs to an install codeg does not own and
// is what the next connection launches, so the row must keep showing it and the
// toast must not claim the local version was removed.
describe("survivingInstallVersion", () => {
it("keeps a version that outlived the uninstall", () => {
expect(survivingInstallVersion("1.18.25")).toBe("1.18.25")
expect(survivingInstallVersion(" 1.18.25 ")).toBe("1.18.25")
})

it("reports nothing left for an empty or absent probe", () => {
expect(survivingInstallVersion(null)).toBeNull()
expect(survivingInstallVersion(undefined)).toBeNull()
expect(survivingInstallVersion("")).toBeNull()
expect(survivingInstallVersion(" ")).toBeNull()
})
})

describe("buildCodexSandboxConfig — Codex sandbox/approval save patch", () => {
// The core contract. The panel also sends the raw config.toml text and the
// backend applies this patch last, so a field the user did not move must not
Expand Down
29 changes: 27 additions & 2 deletions src/components/settings/acp-agent-settings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3870,6 +3870,23 @@ export function buildAcpAdapterCheck(
}
}

/**
* The version that survived an uninstall, read from a re-probe run AFTER it.
*
* Uninstall only ever removes the copy codeg manages, so a version still
* answering here belongs to an install codeg does not own (PATH,
* `~/.local/bin`, a package manager) and is exactly what the next connection
* will launch. Reporting that as "local version removed" while forcing the
* agent row to "not installed" is #631. A blank probe result means nothing
* answered rather than a version, so it collapses to `null` instead of putting
* an empty version string on the row.
*/
export function survivingInstallVersion(
probed: string | null | undefined
): string | null {
return probed?.trim() || null
}

// `uvReady` reports whether the uv runtime (uvx) is installed — only meaningful
// for uvx agents (custom Python-package agents; built-in Hermes moved to the
// npm bridge). Derived from the uv preflight check by the caller. uvx agents
Expand Down Expand Up @@ -5127,16 +5144,24 @@ export function AcpAgentSettings() {
await installStream.start(taskId)
try {
await acpUninstallAgent(agent.agent_type, taskId)
// Re-probe instead of assuming the agent is gone: uninstall only ever
// removes the copy codeg manages, and an install from somewhere else
// survives it untouched. See `survivingInstallVersion`.
const surviving = survivingInstallVersion(
await acpDetectAgentLocalVersion(agent.agent_type).catch(() => null)
)
setAgents((prev) =>
prev.map((item) =>
item.agent_type === agent.agent_type
? { ...item, installed_version: null }
? { ...item, installed_version: surviving }
: item
)
)
await runPreflight(agent.agent_type)
toast.success(t("toasts.uninstallCompleted", { name: agent.name }), {
description: t("toasts.localVersionRemoved"),
description: surviving
? t("toasts.unmanagedInstallRemains", { version: surviving })
: t("toasts.localVersionRemoved"),
})
} catch (err) {
const message = toErrorMessage(err)
Expand Down
1 change: 1 addition & 0 deletions src/i18n/messages/ar.json
Original file line number Diff line number Diff line change
Expand Up @@ -1364,6 +1364,7 @@
"uninstallCompleted": "اكتملت إزالة تثبيت {name}",
"uninstallFailed": "فشلت إزالة تثبيت {name}",
"localVersionRemoved": "تمت إزالة الإصدار المحلي",
"unmanagedInstallRemains": "الإصدار {version} لا يزال مثبتًا خارج codeg وسيستمر استخدامه",
"saveAgentOrderFailed": "فشل حفظ ترتيب Agent",
"saveAgentSwitchFailed": "فشل حفظ مفتاح Agent",
"saveEnvFailed": "فشل حفظ متغيرات البيئة",
Expand Down
1 change: 1 addition & 0 deletions src/i18n/messages/de.json
Original file line number Diff line number Diff line change
Expand Up @@ -1364,6 +1364,7 @@
"uninstallCompleted": "Deinstallation von {name} abgeschlossen",
"uninstallFailed": "Deinstallation von {name} fehlgeschlagen",
"localVersionRemoved": "Lokale Version entfernt",
"unmanagedInstallRemains": "Version {version} ist weiterhin außerhalb von codeg installiert und wird weiterhin verwendet",
"saveAgentOrderFailed": "Speichern der Agent-Reihenfolge fehlgeschlagen",
"saveAgentSwitchFailed": "Speichern des Agent-Schalters fehlgeschlagen",
"saveEnvFailed": "Speichern der Umgebungsvariablen fehlgeschlagen",
Expand Down
1 change: 1 addition & 0 deletions src/i18n/messages/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -1364,6 +1364,7 @@
"uninstallCompleted": "{name} uninstall completed",
"uninstallFailed": "{name} uninstall failed",
"localVersionRemoved": "Local version removed",
"unmanagedInstallRemains": "Version {version} is still installed outside codeg and will still be used",
"saveAgentOrderFailed": "Failed to save Agent order",
"saveAgentSwitchFailed": "Failed to save Agent switch",
"saveEnvFailed": "Failed to save environment variables",
Expand Down
1 change: 1 addition & 0 deletions src/i18n/messages/es.json
Original file line number Diff line number Diff line change
Expand Up @@ -1364,6 +1364,7 @@
"uninstallCompleted": "Desinstalación de {name} completada",
"uninstallFailed": "Desinstalación de {name} fallida",
"localVersionRemoved": "Versión local eliminada",
"unmanagedInstallRemains": "La versión {version} sigue instalada fuera de codeg y se seguirá usando",
"saveAgentOrderFailed": "No se pudo guardar el orden de Agent",
"saveAgentSwitchFailed": "No se pudo guardar el switch de Agent",
"saveEnvFailed": "No se pudieron guardar las variables de entorno",
Expand Down
1 change: 1 addition & 0 deletions src/i18n/messages/fr.json
Original file line number Diff line number Diff line change
Expand Up @@ -1364,6 +1364,7 @@
"uninstallCompleted": "Désinstallation de {name} terminée",
"uninstallFailed": "Échec de la désinstallation de {name}",
"localVersionRemoved": "Version locale supprimée",
"unmanagedInstallRemains": "La version {version} est toujours installée en dehors de codeg et continuera d’être utilisée",
"saveAgentOrderFailed": "Échec de l’enregistrement de l’ordre des agents",
"saveAgentSwitchFailed": "Échec de l’enregistrement du switch agent",
"saveEnvFailed": "Échec de l’enregistrement des variables d’environnement",
Expand Down
1 change: 1 addition & 0 deletions src/i18n/messages/ja.json
Original file line number Diff line number Diff line change
Expand Up @@ -1364,6 +1364,7 @@
"uninstallCompleted": "{name} のアンインストールが完了しました",
"uninstallFailed": "{name} のアンインストールに失敗しました",
"localVersionRemoved": "ローカルバージョンを削除しました",
"unmanagedInstallRemains": "バージョン {version} は codeg の外に残っており、接続時にはこれが使われます",
"saveAgentOrderFailed": "Agent の並び順の保存に失敗しました",
"saveAgentSwitchFailed": "Agent の有効スイッチ保存に失敗しました",
"saveEnvFailed": "環境変数の保存に失敗しました",
Expand Down
1 change: 1 addition & 0 deletions src/i18n/messages/ko.json
Original file line number Diff line number Diff line change
Expand Up @@ -1364,6 +1364,7 @@
"uninstallCompleted": "{name} 제거 완료",
"uninstallFailed": "{name} 제거 실패",
"localVersionRemoved": "로컬 버전이 제거되었습니다",
"unmanagedInstallRemains": "버전 {version}이(가) codeg 외부에 그대로 설치되어 있어 연결 시 계속 사용됩니다",
"saveAgentOrderFailed": "Agent 순서 저장 실패",
"saveAgentSwitchFailed": "Agent 스위치 저장 실패",
"saveEnvFailed": "환경 변수 저장 실패",
Expand Down
1 change: 1 addition & 0 deletions src/i18n/messages/pt.json
Original file line number Diff line number Diff line change
Expand Up @@ -1364,6 +1364,7 @@
"uninstallCompleted": "Desinstalação de {name} concluída",
"uninstallFailed": "Desinstalação de {name} falhou",
"localVersionRemoved": "Versão local removida",
"unmanagedInstallRemains": "A versão {version} continua instalada fora do codeg e continuará sendo usada",
"saveAgentOrderFailed": "Falha ao salvar a ordem dos Agents",
"saveAgentSwitchFailed": "Falha ao salvar o switch dos Agents",
"saveEnvFailed": "Falha ao salvar variáveis de ambiente",
Expand Down
1 change: 1 addition & 0 deletions src/i18n/messages/zh-CN.json
Original file line number Diff line number Diff line change
Expand Up @@ -1364,6 +1364,7 @@
"uninstallCompleted": "{name}卸载完成",
"uninstallFailed": "{name}卸载失败",
"localVersionRemoved": "本地版本已移除",
"unmanagedInstallRemains": "版本 {version} 仍安装在 codeg 之外,连接时仍会使用它",
"saveAgentOrderFailed": "保存 Agent 排序失败",
"saveAgentSwitchFailed": "保存 Agent 开关失败",
"saveEnvFailed": "保存环境变量失败",
Expand Down
1 change: 1 addition & 0 deletions src/i18n/messages/zh-TW.json
Original file line number Diff line number Diff line change
Expand Up @@ -1364,6 +1364,7 @@
"uninstallCompleted": "{name}卸載完成",
"uninstallFailed": "{name}卸載失敗",
"localVersionRemoved": "本地版本已移除",
"unmanagedInstallRemains": "版本 {version} 仍安裝在 codeg 之外,連線時仍會使用它",
"saveAgentOrderFailed": "儲存 Agent 排序失敗",
"saveAgentSwitchFailed": "儲存 Agent 開關失敗",
"saveEnvFailed": "儲存環境變數失敗",
Expand Down
Loading