mirror of
https://github.com/l0ng-ai/openidea.git
synced 2026-11-06 08:07:49 +00:00
fix(agents): keep registry installs running after Settings closes (#1460)
Closing Settings while an ACP Registry agent was installing cancelled the install, and Install was offered again while a previous attempt could still hold the agent's folder. Installs now outlive the page as a background task of the project windows (stoppable from the status bar); the backend refuses a second install of the same agent and marks it installing in the list, which a reopened page follows until it ends. Fixes #1430
This commit is contained in:
@@ -76,6 +76,8 @@ pub struct RegistryAgent {
|
||||
pub installed: Option<String>,
|
||||
/// Whether [`Self::version`] is newer than the installed one.
|
||||
pub update: bool,
|
||||
/// An install of it is running (it goes on after the Settings page that started it closes).
|
||||
pub installing: bool,
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Type)]
|
||||
@@ -332,6 +334,7 @@ fn visible(
|
||||
distribution,
|
||||
no_checksum,
|
||||
installed,
|
||||
installing: false,
|
||||
})
|
||||
})
|
||||
.collect();
|
||||
|
||||
@@ -8,8 +8,11 @@ use ide_agent::{
|
||||
AgentChange, AgentCommand, AgentEntry, AgentEvent, AgentProbe, AgentPrompt, AgentProvider, AgentQueued, AgentTask,
|
||||
NewAgentTask,
|
||||
};
|
||||
use ide_core::{IdeError, IdeResult, IpcPath, RequestId};
|
||||
use ide_core::error::ErrorKind;
|
||||
use ide_core::{IdeError, IdeEvent, IdeResult, IpcPath, RequestId, SharedSink, TaskId};
|
||||
use ide_tools::InstallEvent;
|
||||
use std::collections::HashSet;
|
||||
use std::sync::{LazyLock, Mutex};
|
||||
|
||||
/// Streams the project's agent events to this window; returns every task.
|
||||
pub fn subscribe(host: &Host, window: &str, on_event: Emit<AgentEvent>) -> IdeResult<Vec<AgentTask>> {
|
||||
@@ -58,7 +61,39 @@ pub async fn registry_list(host: &Host, _window: &str, request: RequestId) -> Id
|
||||
let result =
|
||||
tokio::task::spawn_blocking(move || ide_agent::registry::list(&paths, &cancel)).await.map_err(join_err);
|
||||
host.end_request(request);
|
||||
result?
|
||||
let mut registry = result??;
|
||||
let installing = installing();
|
||||
for agent in &mut registry.agents {
|
||||
agent.installing = installing.contains(&agent.id);
|
||||
}
|
||||
Ok(registry)
|
||||
}
|
||||
|
||||
/// Registry installs in progress, by agent id. An install outlives the Settings page
|
||||
/// that started it, so a second one of the same agent is refused rather than racing it
|
||||
/// for the same folder.
|
||||
static INSTALLING: LazyLock<Mutex<HashSet<String>>> = LazyLock::new(Mutex::default);
|
||||
|
||||
fn installing() -> std::sync::MutexGuard<'static, HashSet<String>> {
|
||||
INSTALLING.lock().unwrap_or_else(|e| e.into_inner())
|
||||
}
|
||||
|
||||
/// Marks agent `id` as installing until dropped.
|
||||
struct Installing(String);
|
||||
|
||||
impl Installing {
|
||||
fn begin(id: &str) -> IdeResult<Self> {
|
||||
if !installing().insert(id.to_owned()) {
|
||||
return Err(IdeError::new(ErrorKind::AlreadyExists, format!("{id} is already being installed")));
|
||||
}
|
||||
Ok(Self(id.to_owned()))
|
||||
}
|
||||
}
|
||||
|
||||
impl Drop for Installing {
|
||||
fn drop(&mut self) {
|
||||
installing().remove(&self.0);
|
||||
}
|
||||
}
|
||||
|
||||
/// Installs the registry's current version of agent `id` (also to update it), streaming
|
||||
@@ -72,11 +107,43 @@ pub async fn registry_install(
|
||||
on_event: Channel<InstallEvent>,
|
||||
) -> IdeResult<()> {
|
||||
let paths = registry_paths(host);
|
||||
let installing = Installing::begin(&id)?;
|
||||
let cancel = host.begin_request(request);
|
||||
// Also a background task of every project window, so it stays visible and can be
|
||||
// stopped there once the Settings page is closed.
|
||||
let task = TaskId::next();
|
||||
let title = format!("Installing {id}");
|
||||
let sinks: Vec<SharedSink> = host.projects().iter().map(|p| p.sink.clone()).collect();
|
||||
let report = move |message: Option<String>, fraction: Option<f32>, done: bool| {
|
||||
for sink in &sinks {
|
||||
sink.emit(IdeEvent::Task {
|
||||
id: task,
|
||||
title: title.clone(),
|
||||
message: message.clone(),
|
||||
fraction,
|
||||
done,
|
||||
cancellable: !done,
|
||||
});
|
||||
}
|
||||
};
|
||||
let result = tokio::task::spawn_blocking(move || {
|
||||
ide_agent::registry::install(&paths, &id, &cancel, &|e| {
|
||||
let _installing = installing;
|
||||
let _cancel = ide_core::task::on_cancel(task, {
|
||||
let cancel = cancel.clone();
|
||||
move || cancel.cancel()
|
||||
});
|
||||
report(None, None, false);
|
||||
let result = ide_agent::registry::install(&paths, &id, &cancel, &|e| {
|
||||
match &e {
|
||||
InstallEvent::Line { text } => report(Some(text.clone()), None, false),
|
||||
InstallEvent::Progress { fraction, message } => {
|
||||
report(Some(message.clone()), fraction.map(|f| f as f32), false)
|
||||
}
|
||||
}
|
||||
let _ = on_event.send(e);
|
||||
})
|
||||
});
|
||||
report(None, Some(1.0), true);
|
||||
result
|
||||
})
|
||||
.await
|
||||
.map_err(join_err);
|
||||
@@ -263,6 +330,20 @@ fn validate_project_root(root: &std::path::Path, requested: Option<&std::path::P
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn an_agent_installs_once_at_a_time() {
|
||||
let first = Installing::begin("test-agent").unwrap();
|
||||
assert!(installing().contains("test-agent"));
|
||||
let again = Installing::begin("test-agent").err().unwrap();
|
||||
assert_eq!(again.kind, ErrorKind::AlreadyExists);
|
||||
let other = Installing::begin("test-other-agent").unwrap();
|
||||
drop(first);
|
||||
assert!(!installing().contains("test-agent"));
|
||||
drop(Installing::begin("test-agent").unwrap());
|
||||
drop(other);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn agent_roots_are_exact_window_or_workspace_members() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
|
||||
@@ -6016,6 +6016,8 @@ export type RegistryAgent = {
|
||||
installed: string | null,
|
||||
/** Whether [`Self::version`] is newer than the installed one. */
|
||||
update: boolean,
|
||||
/** An install of it is running (it goes on after the Settings page that started it closes). */
|
||||
installing: boolean,
|
||||
};
|
||||
|
||||
export type RegistryDistribution =
|
||||
|
||||
@@ -1,9 +1,10 @@
|
||||
/**
|
||||
* Settings › Tools › Agents: the launch command of built-in agents (Claude Code…), and
|
||||
* agents of the ACP Registry this machine can run, with Install / Update / Remove.
|
||||
* Installs run in the background with progress; leaving the page cancels one.
|
||||
* Installs run in the background with progress, and keep running after the page closes
|
||||
* (as a background task of the project windows); the page then shows them as installing.
|
||||
*/
|
||||
import { useEffect, useMemo, useRef, useState } from "react";
|
||||
import { useEffect, useMemo, useState } from "react";
|
||||
import type { AgentRegistry, RegistryAgent, RequestId } from "@/bindings";
|
||||
import { isCancelled, nextRequestId } from "@/platform/ipc";
|
||||
import { tx } from "@/platform/i18n";
|
||||
@@ -28,13 +29,26 @@ import styles from "./AgentRegistry.module.css";
|
||||
|
||||
const errorText = (e: unknown) => (e instanceof Error ? e.message : String(e));
|
||||
|
||||
/** How often the page checks on installs it did not start. */
|
||||
const BACKGROUND_POLL_MS = 1500;
|
||||
|
||||
interface Job {
|
||||
id: string;
|
||||
request: RequestId;
|
||||
fraction: number | null;
|
||||
message: string;
|
||||
}
|
||||
|
||||
interface PageJob extends Job {
|
||||
request: RequestId;
|
||||
}
|
||||
|
||||
/** An install started before the page opened; its progress shows in the background tasks. */
|
||||
const backgroundJob = (agent: RegistryAgent): Job => ({
|
||||
id: agent.id,
|
||||
fraction: null,
|
||||
message: tx`Installing ${agent.name} in the background…`,
|
||||
});
|
||||
|
||||
export function AgentRegistryPage({
|
||||
onChanged,
|
||||
backend = tauriRegistry,
|
||||
@@ -72,7 +86,7 @@ function RegistryList({ backend, onChanged }: { backend: RegistryBackend; onChan
|
||||
const [registry, setRegistry] = useState<AgentRegistry | null>(null);
|
||||
const [loadError, setLoadError] = useState<string | null>(null);
|
||||
const [query, setQuery] = useState("");
|
||||
const [job, setJob] = useState<Job | null>(null);
|
||||
const [job, setJob] = useState<PageJob | null>(null);
|
||||
const [removing, setRemoving] = useState<string | null>(null);
|
||||
const [errors, setErrors] = useState<Record<string, string>>({});
|
||||
|
||||
@@ -89,11 +103,39 @@ function RegistryList({ backend, onChanged }: { backend: RegistryBackend; onChan
|
||||
};
|
||||
}, [backend]);
|
||||
|
||||
// Installs started before the page opened: follow them until they end.
|
||||
const background = registry?.agents.some((a) => a.installing) ?? false;
|
||||
useEffect(() => {
|
||||
if (!background) return;
|
||||
let live = true;
|
||||
let request: RequestId | null = null;
|
||||
const timer = setInterval(() => {
|
||||
if (request != null) return;
|
||||
const r = (request = nextRequestId());
|
||||
backend
|
||||
.list(r)
|
||||
.then((next) => {
|
||||
if (!live) return;
|
||||
setRegistry(next);
|
||||
if (!next.agents.some((a) => a.installing)) onChanged?.();
|
||||
})
|
||||
.catch(() => {})
|
||||
.finally(() => {
|
||||
if (request === r) request = null;
|
||||
});
|
||||
}, BACKGROUND_POLL_MS);
|
||||
return () => {
|
||||
live = false;
|
||||
clearInterval(timer);
|
||||
if (request != null) backend.cancel(request);
|
||||
};
|
||||
}, [backend, background, onChanged]);
|
||||
|
||||
const agents = useMemo(
|
||||
() => sortAgents((registry?.agents ?? []).filter((a) => matchesQuery(a, query))),
|
||||
[registry, query],
|
||||
);
|
||||
const busy = job != null || removing != null;
|
||||
const busy = job != null || removing != null || background;
|
||||
|
||||
const setError = (id: string, error: string | null) =>
|
||||
setErrors((all) => {
|
||||
@@ -101,9 +143,6 @@ function RegistryList({ backend, onChanged }: { backend: RegistryBackend; onChan
|
||||
return error ? { ...rest, [id]: error } : rest;
|
||||
});
|
||||
|
||||
// The install in progress, for leaving the page.
|
||||
const running = useRef<RequestId | null>(null);
|
||||
|
||||
const install = async (agent: RegistryAgent) => {
|
||||
if (
|
||||
agent.noChecksum &&
|
||||
@@ -116,7 +155,6 @@ function RegistryList({ backend, onChanged }: { backend: RegistryBackend; onChan
|
||||
)
|
||||
return;
|
||||
const request = nextRequestId();
|
||||
running.current = request;
|
||||
setError(agent.id, null);
|
||||
setJob({ id: agent.id, request, fraction: null, message: tx`Installing ${agent.name} ${agent.version}…` });
|
||||
backend
|
||||
@@ -134,10 +172,7 @@ function RegistryList({ backend, onChanged }: { backend: RegistryBackend; onChan
|
||||
onChanged?.();
|
||||
})
|
||||
.catch((e: unknown) => !isCancelled(e) && setError(agent.id, errorText(e)))
|
||||
.finally(() => {
|
||||
if (running.current === request) running.current = null;
|
||||
setJob(null);
|
||||
});
|
||||
.finally(() => setJob(null));
|
||||
};
|
||||
|
||||
const remove = (agent: RegistryAgent) => {
|
||||
@@ -153,14 +188,6 @@ function RegistryList({ backend, onChanged }: { backend: RegistryBackend; onChan
|
||||
.finally(() => setRemoving(null));
|
||||
};
|
||||
|
||||
// Leaving the page stops an install.
|
||||
useEffect(
|
||||
() => () => {
|
||||
if (running.current != null) backend.cancel(running.current);
|
||||
},
|
||||
[backend],
|
||||
);
|
||||
|
||||
return (
|
||||
<div className={styles.body}>
|
||||
<SearchField
|
||||
@@ -202,13 +229,13 @@ function RegistryList({ backend, onChanged }: { backend: RegistryBackend; onChan
|
||||
<AgentRow
|
||||
key={agent.id}
|
||||
agent={agent}
|
||||
job={job?.id === agent.id ? job : null}
|
||||
job={job?.id === agent.id ? job : agent.installing ? backgroundJob(agent) : null}
|
||||
removing={removing === agent.id}
|
||||
disabled={busy}
|
||||
error={errors[agent.id] ?? null}
|
||||
onInstall={() => void install(agent)}
|
||||
onRemove={() => remove(agent)}
|
||||
onCancel={() => job && backend.cancel(job.request)}
|
||||
onCancel={job?.id === agent.id ? () => backend.cancel(job.request) : undefined}
|
||||
/>
|
||||
))}
|
||||
</ul>
|
||||
@@ -235,7 +262,8 @@ function AgentRow({
|
||||
error: string | null;
|
||||
onInstall: () => void;
|
||||
onRemove: () => void;
|
||||
onCancel: () => void;
|
||||
/** Absent for an install this page did not start (stop it from the background tasks). */
|
||||
onCancel?: () => void;
|
||||
}) {
|
||||
const actions = rowActions(agent);
|
||||
const titleId = `agent-${agent.id}`;
|
||||
@@ -274,9 +302,11 @@ function AgentRow({
|
||||
</div>
|
||||
<div className={styles.actions}>
|
||||
{job ? (
|
||||
<Button size="small" onClick={onCancel}>
|
||||
Cancel
|
||||
</Button>
|
||||
onCancel && (
|
||||
<Button size="small" onClick={onCancel}>
|
||||
Cancel
|
||||
</Button>
|
||||
)
|
||||
) : (
|
||||
<>
|
||||
{actions.install && (
|
||||
|
||||
@@ -42,6 +42,7 @@ const agent = (partial: Partial<RegistryAgent> & Pick<RegistryAgent, "id" | "nam
|
||||
noChecksum: false,
|
||||
installed: null,
|
||||
update: false,
|
||||
installing: false,
|
||||
...partial,
|
||||
});
|
||||
|
||||
@@ -67,14 +68,16 @@ const AGENTS: RegistryAgent[] = [AMP, CLINE, FAST];
|
||||
class FakeBackend implements RegistryBackend {
|
||||
registry: AgentRegistry = { agents: AGENTS, warning: null };
|
||||
installs: string[] = [];
|
||||
installRequests: RequestId[] = [];
|
||||
removed: string[] = [];
|
||||
cancelled: RequestId[] = [];
|
||||
finish: () => void = () => {};
|
||||
fail: (e: unknown) => void = () => {};
|
||||
emit: (e: InstallEvent) => void = () => {};
|
||||
list = vi.fn(async () => this.registry);
|
||||
install(id: string, _request: RequestId, onEvent: (e: InstallEvent) => void) {
|
||||
install(id: string, request: RequestId, onEvent: (e: InstallEvent) => void) {
|
||||
this.installs.push(id);
|
||||
this.installRequests.push(request);
|
||||
this.emit = onEvent;
|
||||
return new Promise<void>((resolve, reject) => {
|
||||
this.finish = resolve;
|
||||
@@ -267,7 +270,7 @@ describe("Agents settings page", () => {
|
||||
expect(screen.queryByRole("dialog")).toBeNull();
|
||||
});
|
||||
|
||||
it("shows why an install failed, and cancels one when the page closes", async () => {
|
||||
it("shows why an install failed, cancels one on Cancel, and leaves it running when the page closes", async () => {
|
||||
const backend = new FakeBackend();
|
||||
const { unmount } = renderRegistry(<AgentRegistryPage backend={backend} />);
|
||||
await waitFor(() => expect(row("Cline")).toBeInTheDocument());
|
||||
@@ -278,10 +281,34 @@ describe("Agents settings page", () => {
|
||||
fireEvent.click(within(row("Amp")).getByRole("button", { name: "Install" }));
|
||||
fireEvent.click(within(row("Amp")).getByRole("button", { name: "Cancel" }));
|
||||
expect(backend.cancelled).toHaveLength(1);
|
||||
// Leaving the page stops an install still running.
|
||||
const install = backend.cancelled[0];
|
||||
const cancelled = backend.cancelled[0];
|
||||
await act(async () => backend.fail(new IpcError({ kind: "cancelled", message: "cancelled" })));
|
||||
|
||||
// An install keeps going after the page closes (it is a background task then).
|
||||
fireEvent.click(within(row("Amp")).getByRole("button", { name: "Install" }));
|
||||
const running = backend.installRequests.at(-1);
|
||||
unmount();
|
||||
expect(backend.cancelled.filter((r) => r === install)).toHaveLength(2);
|
||||
expect(backend.cancelled).not.toContain(running);
|
||||
expect(backend.cancelled.filter((r) => r === cancelled)).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("follows an install started before the page opened until it ends", async () => {
|
||||
const backend = new FakeBackend();
|
||||
const onChanged = vi.fn();
|
||||
backend.registry = { agents: [{ ...AMP, installing: true }, CLINE], warning: null };
|
||||
renderRegistry(<AgentRegistryPage onChanged={onChanged} backend={backend} />);
|
||||
await waitFor(() => expect(row("Amp")).toBeInTheDocument());
|
||||
expect(within(row("Amp")).getByText("Installing Amp in the background…")).toBeInTheDocument();
|
||||
// It is stopped from the background tasks, and nothing else starts meanwhile.
|
||||
for (const name of ["Cancel", "Install", "Remove"]) {
|
||||
expect(within(row("Amp")).queryByRole("button", { name })).toBeNull();
|
||||
}
|
||||
expect(within(row("Cline")).getByRole("button", { name: "Update" })).toBeDisabled();
|
||||
|
||||
backend.registry = { agents: [{ ...AMP, installed: AMP.version }, CLINE], warning: null };
|
||||
await waitFor(() => expect(within(row("Amp")).getByText("Installed")).toBeInTheDocument(), { timeout: 4000 });
|
||||
expect(onChanged).toHaveBeenCalledTimes(1);
|
||||
expect(within(row("Cline")).getByRole("button", { name: "Update" })).toBeEnabled();
|
||||
});
|
||||
|
||||
it("removes an installed agent", async () => {
|
||||
|
||||
@@ -4911,6 +4911,7 @@
|
||||
"The registry has no agents for this system": "Registry 中没有适用于此系统的 Agent",
|
||||
"{version} available": "{version} 可用",
|
||||
"Installing {name} {version}…": "正在安装 {name} {version}…",
|
||||
"Installing {name} in the background…": "正在后台安装 {name}…",
|
||||
"Website": "网站",
|
||||
"Removing…": "正在移除…",
|
||||
"Registry agents": "Registry Agent",
|
||||
|
||||
Reference in New Issue
Block a user