diff --git a/src/renderer/components/thread/ThreadAuthRequiredDock.tsx b/src/renderer/components/thread/ThreadAuthRequiredDock.tsx index a146f99a7..28e2f949c 100644 --- a/src/renderer/components/thread/ThreadAuthRequiredDock.tsx +++ b/src/renderer/components/thread/ThreadAuthRequiredDock.tsx @@ -3,6 +3,7 @@ import { toast } from "@heroui/react"; import { KeyRound, LogIn, RefreshCw, Settings } from "lucide-react"; import { Trans, useLingui } from "@lingui/react/macro"; import type { AgentStatus, Project } from "@/shared/contracts"; +import { friendlyError } from "@/shared/messages"; import { isRemoteSession, readBridge } from "@/renderer/bridge"; import { runAgentLoginCommand } from "@/renderer/actions/agentLoginActions"; import { openSettings } from "@/renderer/actions/panelActions"; @@ -104,7 +105,9 @@ export function ThreadAuthRequiredDock(props: { agentStatus: AgentStatus; projec toast.success(t`${agentStatus.label} authenticated.`); } catch (error) { toast.danger( - error instanceof Error ? error.message : t`Unable to authenticate ${agentStatus.label}.`, + error instanceof Error + ? friendlyError(error) + : t`Unable to authenticate ${agentStatus.label}.`, ); } finally { setPendingAction(undefined); diff --git a/src/renderer/i18n/sharedMessages.test.ts b/src/renderer/i18n/sharedMessages.test.ts index b906a49b0..00ddef1f7 100644 --- a/src/renderer/i18n/sharedMessages.test.ts +++ b/src/renderer/i18n/sharedMessages.test.ts @@ -54,6 +54,19 @@ describe("shared message i18n integration", () => { expect(summary.length).toBeGreaterThan(0); }); + it("translates wrapped ACP authentication verification errors", async () => { + await dynamicActivate("es"); + const summary = friendlyError( + new Error( + "Error invoking remote method 'poracode:authenticate-acp-agent': Error: My ACP reported authentication success, but Poracode could not verify it. Configure My ACP directly, then try again.", + ), + ); + + expect(summary).toBe( + "My ACP informó que la autenticación se realizó correctamente, pero Poracode no pudo verificarla. Configura My ACP directamente y vuelve a intentarlo.", + ); + }); + it("translates main-process SSH manifest errors and preserves their path", async () => { await dynamicActivate("es"); const path = "C:\\Poracode\\server.ssh-runtime-manifest.json"; diff --git a/src/renderer/i18n/sharedMessages.ts b/src/renderer/i18n/sharedMessages.ts index cf2dc4749..9c0d7b093 100644 --- a/src/renderer/i18n/sharedMessages.ts +++ b/src/renderer/i18n/sharedMessages.ts @@ -175,6 +175,10 @@ const SHARED_MESSAGE_DESCRIPTORS: Record = { "supervisor.exited": msg({ message: "Background process exited unexpectedly" }), "supervisor.notRunning": msg({ message: "Background process is not running" }), "supervisor.proposedPlan": msg({ message: "Proposed plan" }), + "acp.authenticationUnverified": msg({ + message: + "{agent} reported authentication success, but Poracode could not verify it. Configure {agent} directly, then try again.", + }), "kimi.credentialsLocked": msg({ message: "Kimi Code could not update its credentials because another process is using the credential file. Close other Poracode or Kimi Code processes, then retry.", diff --git a/src/renderer/locales/de/messages.po b/src/renderer/locales/de/messages.po index 3cc5f03ac..21833d0b1 100644 --- a/src/renderer/locales/de/messages.po +++ b/src/renderer/locales/de/messages.po @@ -388,6 +388,10 @@ msgstr "{0}/{1} ausgewählt" msgid "{0}mo ago" msgstr "vor {0} Mon." +#: src/renderer/i18n/sharedMessages.ts +msgid "{agent} reported authentication success, but Poracode could not verify it. Configure {agent} directly, then try again." +msgstr "{agent} hat eine erfolgreiche Authentifizierung gemeldet, Poracode konnte sie jedoch nicht überprüfen. Konfiguriere {agent} direkt und versuche es dann erneut." + #: src/renderer/views/SettingsOverlay/parts/AcpRegistrySettings.tsx msgid "{agentLabel} install targets" msgstr "{agentLabel} Installationsziele" diff --git a/src/renderer/locales/en/messages.po b/src/renderer/locales/en/messages.po index 478c1f3e6..70e545437 100644 --- a/src/renderer/locales/en/messages.po +++ b/src/renderer/locales/en/messages.po @@ -388,6 +388,10 @@ msgstr "{0}/{1} selected" msgid "{0}mo ago" msgstr "{0}mo ago" +#: src/renderer/i18n/sharedMessages.ts +msgid "{agent} reported authentication success, but Poracode could not verify it. Configure {agent} directly, then try again." +msgstr "{agent} reported authentication success, but Poracode could not verify it. Configure {agent} directly, then try again." + #: src/renderer/views/SettingsOverlay/parts/AcpRegistrySettings.tsx msgid "{agentLabel} install targets" msgstr "{agentLabel} install targets" diff --git a/src/renderer/locales/es/messages.po b/src/renderer/locales/es/messages.po index bd71acf24..281209e33 100644 --- a/src/renderer/locales/es/messages.po +++ b/src/renderer/locales/es/messages.po @@ -388,6 +388,10 @@ msgstr "{0}/{1} seleccionados" msgid "{0}mo ago" msgstr "hace {0} m" +#: src/renderer/i18n/sharedMessages.ts +msgid "{agent} reported authentication success, but Poracode could not verify it. Configure {agent} directly, then try again." +msgstr "{agent} informó que la autenticación se realizó correctamente, pero Poracode no pudo verificarla. Configura {agent} directamente y vuelve a intentarlo." + #: src/renderer/views/SettingsOverlay/parts/AcpRegistrySettings.tsx msgid "{agentLabel} install targets" msgstr "{agentLabel} objetivos de instalación" diff --git a/src/renderer/locales/fr/messages.po b/src/renderer/locales/fr/messages.po index c719694dd..fb9468cc7 100644 --- a/src/renderer/locales/fr/messages.po +++ b/src/renderer/locales/fr/messages.po @@ -388,6 +388,10 @@ msgstr "{0}/{1} sélectionnés" msgid "{0}mo ago" msgstr "il y a {0} mois" +#: src/renderer/i18n/sharedMessages.ts +msgid "{agent} reported authentication success, but Poracode could not verify it. Configure {agent} directly, then try again." +msgstr "{agent} a signalé que l’authentification avait réussi, mais Poracode n’a pas pu la vérifier. Configurez directement {agent}, puis réessayez." + #: src/renderer/views/SettingsOverlay/parts/AcpRegistrySettings.tsx msgid "{agentLabel} install targets" msgstr "{agentLabel} cibles d'installation" diff --git a/src/renderer/locales/ja/messages.po b/src/renderer/locales/ja/messages.po index 3487a8e19..87d65f514 100644 --- a/src/renderer/locales/ja/messages.po +++ b/src/renderer/locales/ja/messages.po @@ -388,6 +388,10 @@ msgstr "{0}/{1} 件を選択済み" msgid "{0}mo ago" msgstr "{0}か月前" +#: src/renderer/i18n/sharedMessages.ts +msgid "{agent} reported authentication success, but Poracode could not verify it. Configure {agent} directly, then try again." +msgstr "{agent} は認証に成功したと報告しましたが、Poracode では確認できませんでした。{agent} を直接設定してから、もう一度お試しください。" + #: src/renderer/views/SettingsOverlay/parts/AcpRegistrySettings.tsx msgid "{agentLabel} install targets" msgstr "{agentLabel}インストールターゲット" diff --git a/src/renderer/locales/ko/messages.po b/src/renderer/locales/ko/messages.po index e1d89196c..2b99e489e 100644 --- a/src/renderer/locales/ko/messages.po +++ b/src/renderer/locales/ko/messages.po @@ -388,6 +388,10 @@ msgstr "{0}/{1}개 선택됨" msgid "{0}mo ago" msgstr "{0}개월 전" +#: src/renderer/i18n/sharedMessages.ts +msgid "{agent} reported authentication success, but Poracode could not verify it. Configure {agent} directly, then try again." +msgstr "{agent}이(가) 인증에 성공했다고 보고했지만 Poracode에서 확인할 수 없습니다. {agent}을(를) 직접 구성한 후 다시 시도하세요." + #: src/renderer/views/SettingsOverlay/parts/AcpRegistrySettings.tsx msgid "{agentLabel} install targets" msgstr "{agentLabel} 설치 대상" diff --git a/src/renderer/locales/pl/messages.po b/src/renderer/locales/pl/messages.po index 17f11bf36..ef47f0104 100644 --- a/src/renderer/locales/pl/messages.po +++ b/src/renderer/locales/pl/messages.po @@ -388,6 +388,10 @@ msgstr "Wybrano {0}/{1}" msgid "{0}mo ago" msgstr "{0} mies. temu" +#: src/renderer/i18n/sharedMessages.ts +msgid "{agent} reported authentication success, but Poracode could not verify it. Configure {agent} directly, then try again." +msgstr "{agent} zgłosił pomyślne uwierzytelnienie, ale Poracode nie mógł go zweryfikować. Skonfiguruj bezpośrednio {agent}, a następnie spróbuj ponownie." + #: src/renderer/views/SettingsOverlay/parts/AcpRegistrySettings.tsx msgid "{agentLabel} install targets" msgstr "{agentLabel} cele instalacji" diff --git a/src/renderer/locales/pt-BR/messages.po b/src/renderer/locales/pt-BR/messages.po index 79f645e3f..360c08e08 100644 --- a/src/renderer/locales/pt-BR/messages.po +++ b/src/renderer/locales/pt-BR/messages.po @@ -388,6 +388,10 @@ msgstr "{0}/{1} selecionados" msgid "{0}mo ago" msgstr "há {0} meses" +#: src/renderer/i18n/sharedMessages.ts +msgid "{agent} reported authentication success, but Poracode could not verify it. Configure {agent} directly, then try again." +msgstr "{agent} informou que a autenticação foi bem-sucedida, mas o Poracode não conseguiu verificá-la. Configure {agent} diretamente e tente novamente." + #: src/renderer/views/SettingsOverlay/parts/AcpRegistrySettings.tsx msgid "{agentLabel} install targets" msgstr "{agentLabel} destinos de instalação" diff --git a/src/renderer/locales/ru/messages.po b/src/renderer/locales/ru/messages.po index 652c3ae99..2a1d9c32b 100644 --- a/src/renderer/locales/ru/messages.po +++ b/src/renderer/locales/ru/messages.po @@ -388,6 +388,10 @@ msgstr "Выбрано: {0}/{1}" msgid "{0}mo ago" msgstr "{0} мес. назад" +#: src/renderer/i18n/sharedMessages.ts +msgid "{agent} reported authentication success, but Poracode could not verify it. Configure {agent} directly, then try again." +msgstr "{agent} сообщил об успешной аутентификации, но Poracode не удалось её проверить. Настройте {agent} напрямую и повторите попытку." + #: src/renderer/views/SettingsOverlay/parts/AcpRegistrySettings.tsx msgid "{agentLabel} install targets" msgstr "{agentLabel} целей установки" diff --git a/src/renderer/locales/tr/messages.po b/src/renderer/locales/tr/messages.po index 81e5afbb1..a9f03b885 100644 --- a/src/renderer/locales/tr/messages.po +++ b/src/renderer/locales/tr/messages.po @@ -388,6 +388,10 @@ msgstr "{0}/{1} seçildi" msgid "{0}mo ago" msgstr "{0} ay önce" +#: src/renderer/i18n/sharedMessages.ts +msgid "{agent} reported authentication success, but Poracode could not verify it. Configure {agent} directly, then try again." +msgstr "{agent}, kimlik doğrulamanın başarılı olduğunu bildirdi ancak Poracode bunu doğrulayamadı. {agent} aracısını doğrudan yapılandırıp yeniden deneyin." + #: src/renderer/views/SettingsOverlay/parts/AcpRegistrySettings.tsx msgid "{agentLabel} install targets" msgstr "{agentLabel} yükleme hedefi" diff --git a/src/renderer/locales/uk/messages.po b/src/renderer/locales/uk/messages.po index 474b8bc8f..5e06231b0 100644 --- a/src/renderer/locales/uk/messages.po +++ b/src/renderer/locales/uk/messages.po @@ -388,6 +388,10 @@ msgstr "Вибрано: {0}/{1}" msgid "{0}mo ago" msgstr "{0} міс. тому" +#: src/renderer/i18n/sharedMessages.ts +msgid "{agent} reported authentication success, but Poracode could not verify it. Configure {agent} directly, then try again." +msgstr "{agent} повідомив про успішну автентифікацію, але Poracode не вдалося її перевірити. Налаштуйте {agent} безпосередньо та повторіть спробу." + #: src/renderer/views/SettingsOverlay/parts/AcpRegistrySettings.tsx msgid "{agentLabel} install targets" msgstr "{agentLabel} цілей встановлення" diff --git a/src/renderer/locales/vi/messages.po b/src/renderer/locales/vi/messages.po index db926bd48..192cf6d54 100644 --- a/src/renderer/locales/vi/messages.po +++ b/src/renderer/locales/vi/messages.po @@ -388,6 +388,10 @@ msgstr "Đã chọn {0}/{1}" msgid "{0}mo ago" msgstr "{0} tháng trước" +#: src/renderer/i18n/sharedMessages.ts +msgid "{agent} reported authentication success, but Poracode could not verify it. Configure {agent} directly, then try again." +msgstr "{agent} báo cáo xác thực thành công nhưng Poracode không thể xác minh. Hãy cấu hình trực tiếp {agent} rồi thử lại." + #: src/renderer/views/SettingsOverlay/parts/AcpRegistrySettings.tsx msgid "{agentLabel} install targets" msgstr "{agentLabel} mục tiêu cài đặt" diff --git a/src/renderer/locales/zh-CN/messages.po b/src/renderer/locales/zh-CN/messages.po index deb3ca083..e8bf98081 100644 --- a/src/renderer/locales/zh-CN/messages.po +++ b/src/renderer/locales/zh-CN/messages.po @@ -388,6 +388,10 @@ msgstr "已选择 {0}/{1} 个" msgid "{0}mo ago" msgstr "{0}个月前" +#: src/renderer/i18n/sharedMessages.ts +msgid "{agent} reported authentication success, but Poracode could not verify it. Configure {agent} directly, then try again." +msgstr "{agent} 报告身份验证成功,但 Poracode 无法验证。请直接配置 {agent},然后重试。" + #: src/renderer/views/SettingsOverlay/parts/AcpRegistrySettings.tsx msgid "{agentLabel} install targets" msgstr "{agentLabel}安装目标" diff --git a/src/renderer/state/agentStatusesStore.test.ts b/src/renderer/state/agentStatusesStore.test.ts index 9a5827615..0af99fa43 100644 --- a/src/renderer/state/agentStatusesStore.test.ts +++ b/src/renderer/state/agentStatusesStore.test.ts @@ -45,22 +45,24 @@ function reset() { beforeEach(reset); describe("persisted agent status cache", () => { - it("invalidates v7 statuses cached before Codex context-window capabilities", async () => { + it("invalidates v8 statuses cached before successful ACP sessions established auth", async () => { const options = useAgentStatusesStore.persist.getOptions(); - expect(options.version).toBe(8); - const staleCodex = makeStatus({ - kind: "codex", - label: "Codex", + expect(options.version).toBe(10); + const staleAcp = makeStatus({ + kind: "acp-generic:example", + label: "Example ACP", + authState: "missing", + authMethods: [{ id: "login", name: "Login" }], capabilities: { ...makeStatus().capabilities }, }); const migrated = await options.migrate!( { - agentStatuses: [staleCodex], + agentStatuses: [staleAcp], wslAgentStatuses: [], windowsLoaded: true, wslLoaded: true, }, - 7, + 8, ); expect(migrated).toMatchObject({ agentStatuses: [], @@ -72,7 +74,7 @@ describe("persisted agent status cache", () => { it("invalidates v6 statuses produced without the Grok login-shell environment", async () => { const options = useAgentStatusesStore.persist.getOptions(); - expect(options.version).toBe(8); + expect(options.version).toBe(10); expect(options.migrate).toBeTypeOf("function"); const grok = makeStatus({ @@ -156,6 +158,16 @@ describe("setAgentStatuses", () => { ); }); + it("replaces the array when ACP session readiness changes", () => { + const cached = makeStatus({ authState: "unknown" }); + const fresh = makeStatus({ authState: "unknown", acpSessionEstablished: true }); + useAgentStatusesStore.setState({ agentStatuses: [cached], windowsLoaded: true }); + + useAgentStatusesStore.getState().setAgentStatuses([fresh]); + + expect(useAgentStatusesStore.getState().agentStatuses).toEqual([fresh]); + }); + it("replaces the array when only a presentation capability catalog changes", () => { const cached = makeStatus({ capabilities: { diff --git a/src/renderer/state/agentStatusesStore.ts b/src/renderer/state/agentStatusesStore.ts index 21291919a..c132b8edb 100644 --- a/src/renderer/state/agentStatusesStore.ts +++ b/src/renderer/state/agentStatusesStore.ts @@ -101,6 +101,7 @@ function statusesEqual(a: AgentStatus[], b: AgentStatus[]): boolean { x.icon === b[i]!.icon && x.version === b[i]!.version && x.authState === b[i]!.authState && + x.acpSessionEstablished === b[i]!.acpSessionEstablished && areAgentPresentationRuntimeFieldsEqual(x, b[i]!) && x.loginCommand === b[i]!.loginCommand && x.envKind === b[i]!.envKind && @@ -254,11 +255,11 @@ export const useAgentStatusesStore = create()( }), { name: "poracode-agent-statuses-v1", - version: 8, - // v8 drops statuses cached before Codex advertised selectable context - // windows. This mirrors the supervisor STATUS_CACHE_VERSION=11 bump, which - // only invalidates the supervisor's on-disk cache, not this localStorage - // copy. + version: 10, + // v10 adds ACP session readiness separately from authentication and + // normalized ACP approval-policy labels. This mirrors the supervisor + // STATUS_CACHE_VERSION=13 bump, which only invalidates the supervisor's + // on-disk cache, not this localStorage copy. migrate: (persisted) => { const prev = (persisted ?? {}) as Partial; return { diff --git a/src/renderer/state/slices/agentStatusesSlice.ts b/src/renderer/state/slices/agentStatusesSlice.ts index e8c920097..0adb0bb6d 100644 --- a/src/renderer/state/slices/agentStatusesSlice.ts +++ b/src/renderer/state/slices/agentStatusesSlice.ts @@ -21,6 +21,7 @@ function statusesEqual(a: AgentStatus[], b: AgentStatus[]): boolean { x.icon === b[i]!.icon && x.version === b[i]!.version && x.authState === b[i]!.authState && + x.acpSessionEstablished === b[i]!.acpSessionEstablished && areAgentPresentationRuntimeFieldsEqual(x, b[i]!) && x.loginCommand === b[i]!.loginCommand && areAgentProviderMetadataEqual(x.providerMetadata, b[i]!.providerMetadata), diff --git a/src/renderer/views/SettingsOverlay/parts/AcpRegistrySettings.test.tsx b/src/renderer/views/SettingsOverlay/parts/AcpRegistrySettings.test.tsx index b52e27f8b..c285b3cfe 100644 --- a/src/renderer/views/SettingsOverlay/parts/AcpRegistrySettings.test.tsx +++ b/src/renderer/views/SettingsOverlay/parts/AcpRegistrySettings.test.tsx @@ -1000,6 +1000,34 @@ describe("AcpRegistrySettings", () => { await waitFor(() => expect(bridge.refreshAgentStatuses).toHaveBeenCalled()); }); + it("does not infer login is required from auth methods after ACP session setup", async () => { + settingsState.acpRegistryInstalledAgents = { + "glm-acp-agent": { + id: "glm-acp-agent", + name: "GLM Agent", + version: "1.1.3", + installedAt: new Date(0).toISOString(), + adapterKind: "acp-generic:glm-acp-agent", + installKind: "generic", + }, + }; + statusesState.agentStatuses = [ + makeStatus("acp-generic:glm-acp-agent", { + label: "GLM Agent", + authState: "unknown", + acpSessionEstablished: true, + authMethods: [{ id: "sso", name: "SSO" }], + }), + ]; + + render(); + + await screen.findByRole("heading", { name: "Agent Registry" }); + const glmCard = screen.getByText("GLM through ACP").closest(".rounded-lg"); + expect(glmCard).toBeTruthy(); + expect(within(glmCard as HTMLElement).queryByRole("button", { name: "Login" })).toBeNull(); + }); + it("runs Factory agent-owned auth from its native card", async () => { statusesState.agentStatuses = [ makeStatus("factory", { diff --git a/src/renderer/views/SettingsOverlay/parts/AcpRegistrySettings.tsx b/src/renderer/views/SettingsOverlay/parts/AcpRegistrySettings.tsx index b60a1701a..136717733 100644 --- a/src/renderer/views/SettingsOverlay/parts/AcpRegistrySettings.tsx +++ b/src/renderer/views/SettingsOverlay/parts/AcpRegistrySettings.tsx @@ -21,6 +21,7 @@ import { type Project, type RefreshAgentScope, } from "@/shared/contracts"; +import { friendlyError } from "@/shared/messages"; import { msg } from "@lingui/core/macro"; import { isWindows, readBridge } from "@/renderer/bridge"; import { i18n } from "@/renderer/i18n/i18n"; @@ -382,7 +383,7 @@ export function AcpRegistrySettings(props: { onOpenAgentSettings?: (kind: string }), ) .catch((err: unknown) => { - setError(err instanceof Error ? err.message : String(err)); + setError(err instanceof Error ? friendlyError(err) : String(err)); }) .finally(() => setPendingAuthAgentId(undefined)); }; diff --git a/src/renderer/views/SettingsOverlay/parts/SingleAgentSettings/SingleAgentSettings.test.tsx b/src/renderer/views/SettingsOverlay/parts/SingleAgentSettings/SingleAgentSettings.test.tsx index a3ee584f1..b1a6f42bd 100644 --- a/src/renderer/views/SettingsOverlay/parts/SingleAgentSettings/SingleAgentSettings.test.tsx +++ b/src/renderer/views/SettingsOverlay/parts/SingleAgentSettings/SingleAgentSettings.test.tsx @@ -1167,6 +1167,52 @@ describe("SingleAgentSettings", () => { expect(within(row).getByRole("button", { name: "Login" })).toBeInTheDocument(); }); + it("does not request login when ACP session setup succeeded without proving auth", () => { + statusesState.agentStatuses = [ + makeStatus("acp-generic:ready-agent", { + label: "Ready Agent", + authState: "unknown", + acpSessionEstablished: true, + authMethods: [{ id: "login", name: "Login" }], + }), + ]; + + render(); + + const row = envRow("Default"); + expect(within(row).queryByRole("button", { name: "Login" })).not.toBeInTheDocument(); + expect(screen.queryByText("Login required")).not.toBeInTheDocument(); + }); + + it("keeps login required for an unready WSL env when native ACP setup succeeded", () => { + statusesState.agentStatuses = [ + makeStatus("acp-generic:ready-agent", { + label: "Ready Agent", + authState: "unknown", + acpSessionEstablished: true, + authMethods: [{ id: "login", name: "Login" }], + envKind: "windows", + }), + ]; + statusesState.wslAgentStatuses = [ + makeStatus("acp-generic:ready-agent", { + label: "Ready Agent", + authState: "unknown", + authMethods: [{ id: "login", name: "Login" }], + envKind: "wsl", + envDistro: "Ubuntu", + }), + ]; + + render(); + + expect(screen.getAllByText("Login required").length).toBeGreaterThan(0); + expect(within(envRow("Windows")).queryByRole("button", { name: "Login" })).toBeNull(); + expect( + within(envRow("WSL (Ubuntu)")).getByRole("button", { name: "Login WSL (Ubuntu)" }), + ).toBeVisible(); + }); + it("offers logout (not re-login) for an authenticated ACP agent env", async () => { statusesState.agentStatuses = [ makeStatus("acp-generic:sso-agent", { diff --git a/src/renderer/views/SettingsOverlay/parts/SingleAgentSettings/SingleAgentSettings.tsx b/src/renderer/views/SettingsOverlay/parts/SingleAgentSettings/SingleAgentSettings.tsx index 58516ae23..1e3c6362d 100644 --- a/src/renderer/views/SettingsOverlay/parts/SingleAgentSettings/SingleAgentSettings.tsx +++ b/src/renderer/views/SettingsOverlay/parts/SingleAgentSettings/SingleAgentSettings.tsx @@ -14,6 +14,7 @@ import { extractAcpGenericInstanceId, extractClaudeProfileInstanceId, } from "@/shared/contracts"; +import { friendlyError } from "@/shared/messages"; import { runAgentInstallCommand, runAgentLoginCommand } from "@/renderer/actions/agentLoginActions"; import { useAppStore } from "@/renderer/state/appStore"; import { useAgentStatusesStore } from "@/renderer/state/agentStatusesStore"; @@ -53,8 +54,10 @@ import { findEnvVarAuthMethod, findTerminalLoginStatus, formatStatusList, + hasInteractiveAuthMethods, resolveLivePlanLabel, statusEnvKey, + statusNeedsInteractiveLogin, supportsAcpLogoutStatus, } from "./parts/authHelpers"; @@ -346,7 +349,7 @@ export function SingleAgentSettings(props: { .then(() => toast.success(t`${agent.label} authenticated.`)) .catch((error: unknown) => toast.danger( - error instanceof Error ? error.message : t`Unable to authenticate ${agent.label}.`, + error instanceof Error ? friendlyError(error) : t`Unable to authenticate ${agent.label}.`, ), ) .finally(() => { @@ -446,19 +449,14 @@ export function SingleAgentSettings(props: { logoutStatuses.length > 0 || hasAdvertisedAuthMethods; const includeAuthFallbackMetadata = !hasAuthSettings; - const authMissing = - missingAuthStatuses.length > 0 || - (hasAdvertisedAuthMethods && - !installedStatuses.some((status) => status.authState === "authenticated")); + const authMissing = installedStatuses.some(statusNeedsInteractiveLogin); const missingAuthLabel = formatStatusList(missingAuthStatuses); const showEnvVarOnly = envVarAuthMethod !== undefined && !authMissing; // Interactive auth (browser/CLI sign-in) is per-env — Windows and each WSL // distro hold their own sessions. We split the auth panel into one row per // env so each shows its own state independently. Env-var credentials stay // shared (single block above the per-env rows). - const hasInteractiveAuth = installedStatuses.some((status) => - status.authMethods?.some((method) => isAgentAuthMethod(method) || method.type === "terminal"), - ); + const hasInteractiveAuth = installedStatuses.some(hasInteractiveAuthMethods); // When env-var credentials already satisfy every env, the user is signed in // via the shared key — per-env Logout rows are misleading because there is // no per-env session to revoke. Show just the env-var block in that case. diff --git a/src/renderer/views/SettingsOverlay/parts/SingleAgentSettings/parts/AgentEnvironmentRow.tsx b/src/renderer/views/SettingsOverlay/parts/SingleAgentSettings/parts/AgentEnvironmentRow.tsx index f209c63f7..543adc53b 100644 --- a/src/renderer/views/SettingsOverlay/parts/SingleAgentSettings/parts/AgentEnvironmentRow.tsx +++ b/src/renderer/views/SettingsOverlay/parts/SingleAgentSettings/parts/AgentEnvironmentRow.tsx @@ -91,7 +91,8 @@ export function AgentEnvironmentRow(props: { const hasAnyMethod = authMethods.length > 0; const isAuthenticated = status.authState === "authenticated"; const isMissing = - status.authState === "missing" || (status.authState === "unknown" && hasAnyMethod); + status.authState === "missing" || + (status.authState === "unknown" && hasAnyMethod && status.acpSessionEstablished !== true); const env = envLabelForStatus(status); const canLogout = isAuthenticated && props.canLogout; const canReLogin = isAuthenticated && !canLogout && hasAnyMethod; diff --git a/src/renderer/views/SettingsOverlay/parts/SingleAgentSettings/parts/authHelpers.ts b/src/renderer/views/SettingsOverlay/parts/SingleAgentSettings/parts/authHelpers.ts index 057943ced..36d37e469 100644 --- a/src/renderer/views/SettingsOverlay/parts/SingleAgentSettings/parts/authHelpers.ts +++ b/src/renderer/views/SettingsOverlay/parts/SingleAgentSettings/parts/authHelpers.ts @@ -14,6 +14,34 @@ import { isTerminalAuthMethod, } from "@/renderer/utils/acpRegistryAuth"; +/** + * Whether the status advertises an interactive (browser/CLI) sign-in, i.e. the + * methods a per-env auth row can offer a Login button for. Env-var credentials + * are excluded — they are edited in the shared block above those rows. + */ +export function hasInteractiveAuthMethods(status: AgentStatus): boolean { + return ( + status.authMethods?.some( + (method) => isAgentAuthMethod(method) || isTerminalAuthMethod(method), + ) ?? false + ); +} + +/** + * Whether an env still needs a sign-in. `unknown` only counts when the agent + * advertises an interactive method and its ACP session setup did not succeed — + * a working session means the agent is usable, so prompting for login there + * would be a false alarm. + */ +export function statusNeedsInteractiveLogin(status: AgentStatus): boolean { + if (status.authState === "missing") return true; + return ( + status.authState === "unknown" && + status.acpSessionEstablished !== true && + hasInteractiveAuthMethods(status) + ); +} + /** * Live plan label to show instead of the one carried by `providerMetadata`. * diff --git a/src/shared/contracts/agent.test.ts b/src/shared/contracts/agent.test.ts index e43007b7b..cefa407cb 100644 --- a/src/shared/contracts/agent.test.ts +++ b/src/shared/contracts/agent.test.ts @@ -1,7 +1,30 @@ import { describe, expect, it } from "vitest"; -import { agentStatusSchema, areAgentPresentationRuntimeFieldsEqual } from "./agent"; +import { + agentStatusesResponseSchema, + agentStatusSchema, + areAgentPresentationRuntimeFieldsEqual, +} from "./agent"; describe("agentStatusSchema runtime variants", () => { + it("preserves optional ACP session readiness and accepts its absence", () => { + const status = { + kind: "acp-generic:example", + label: "Example ACP", + installed: true, + authState: "unknown", + capabilities: {}, + }; + + expect(agentStatusSchema.parse(status).acpSessionEstablished).toBeUndefined(); + expect( + agentStatusesResponseSchema.parse({ + windows: [{ ...status, acpSessionEstablished: true }], + wsl: [], + fromCache: false, + }).windows[0]?.acpSessionEstablished, + ).toBe(true); + }); + it("parses named runtime variants with full effective capability defaults and routing", () => { const parsed = agentStatusSchema.parse({ kind: "cursor", diff --git a/src/shared/contracts/agent.ts b/src/shared/contracts/agent.ts index 5f90e1b32..9851d76e3 100644 --- a/src/shared/contracts/agent.ts +++ b/src/shared/contracts/agent.ts @@ -408,6 +408,8 @@ export const agentStatusSchema = z.object({ version: z.string().optional(), update: agentUpdateInfoSchema.optional(), authState: authStateSchema, + /** ACP session setup succeeded, which proves readiness but not authentication. */ + acpSessionEstablished: z.boolean().optional(), /** * Authentication can differ between presentation runtimes even when they * share one provider tile. For example, Cursor's terminal/ACP surfaces use diff --git a/src/shared/messages.ts b/src/shared/messages.ts index 880c75daa..be153daa8 100644 --- a/src/shared/messages.ts +++ b/src/shared/messages.ts @@ -119,6 +119,10 @@ const messages = { "supervisor.notRunning": "Background process is not running", "supervisor.proposedPlan": "Proposed plan", + // ── ACP ─────────────────────────────────────────────────── + "acp.authenticationUnverified": + "{agent} reported authentication success, but Poracode could not verify it. Configure {agent} directly, then try again.", + // ── Kimi Code ───────────────────────────────────────────── "kimi.credentialsLocked": "Kimi Code could not update its credentials because another process is using the credential file. Close other Poracode or Kimi Code processes, then retry.", @@ -223,12 +227,19 @@ export function errorDetail(err: unknown): string { */ const pullDirtyWorktreePattern = /(?:\bgit\s+pull\b[\s\S]*(?:local changes|unstaged changes|would be overwritten)|cannot pull\b[\s\S]*(?:changes|stash)|local changes[\s\S]*(?:before|during)[\s\S]*(?:merge|pull)|please commit or stash[\s\S]*(?:merge|pull))/i; +const acpAuthenticationUnverifiedPattern = + /^(.+) reported authentication success, but Poracode could not verify it\. Configure \1 directly, then try again\.$/; const errorPatterns: Array<{ test: RegExp; key: MessageKey; params?: (raw: string) => Record; }> = [ + { + test: acpAuthenticationUnverifiedPattern, + key: "acp.authenticationUnverified", + params: (raw) => ({ agent: raw.slice(0, raw.indexOf(" reported authentication success")) }), + }, { test: pullDirtyWorktreePattern, key: "git.pull.localChanges", diff --git a/src/supervisor/agents/acp-generic/index.test.ts b/src/supervisor/agents/acp-generic/index.test.ts index 4a60e3aa2..56e50edb5 100644 --- a/src/supervisor/agents/acp-generic/index.test.ts +++ b/src/supervisor/agents/acp-generic/index.test.ts @@ -353,11 +353,20 @@ describe("createAcpGenericAdapter", () => { expect(status.authMethods).toEqual([{ id: "login", name: "Login" }]); }); - it("reports advertised agent auth as missing even when a session probe succeeds", async () => { - // `sessionEstablished` is not a reliable proxy for "signed in" — some - // agents (e.g. Cline) accept newSession unauthenticated. Until we have a - // positive identity signal, fall through to "missing" so the UI prompts - // for login instead of falsely claiming Signed in. + it("keeps auth unknown when newSession succeeds and auth methods remain advertised", async () => { + vi.mocked(probeAcpCapabilities).mockResolvedValue({ + authState: "authenticated", + sessionEstablished: true, + authMethods: [{ id: "browser-login", name: "Browser login" }], + }); + const adapter = createAcpGenericAdapter(baseInstance); + const status = await adapter.detectInstall(); + expect(status.authState).toBe("unknown"); + expect(status.acpSessionEstablished).toBe(true); + expect(status.authMethods).toEqual([{ id: "browser-login", name: "Browser login" }]); + }); + + it("reports advertised agent auth as missing when the probe has no explicit auth state", async () => { vi.mocked(probeAcpCapabilities).mockResolvedValue({ sessionEstablished: true, authMethods: [{ id: "browser-login", name: "Browser login" }], diff --git a/src/supervisor/agents/acp-generic/index.ts b/src/supervisor/agents/acp-generic/index.ts index 53737d424..7d2097539 100644 --- a/src/supervisor/agents/acp-generic/index.ts +++ b/src/supervisor/agents/acp-generic/index.ts @@ -106,6 +106,7 @@ export function createAcpGenericAdapter(instance: AgentInstanceConfig): AgentAda ...(instance.icon ? { icon: instance.icon } : {}), ...(instance.version ? { version: instance.version } : {}), authState, + ...(probeResult?.sessionEstablished ? { acpSessionEstablished: true } : {}), ...(loginCommand ? { loginCommand } : {}), ...(providerMetadata ? { providerMetadata } : {}), ...(probeResult?.authMethods ? { authMethods: probeResult.authMethods } : {}), @@ -382,6 +383,12 @@ function resolveGenericAuthState( if (isInteractiveAuthAcknowledged(instance, ctx)) { return "authenticated"; } + // ACP v1 exposes supported auth methods, not current auth state. A successful + // session/new proves the agent is ready for a prompt, but agents may defer + // credential validation until that prompt, so keep the state unknown. + if (probeResult?.authState === "authenticated") { + return "unknown"; + } if ( probeResult?.authMethods?.some( (method) => isAcpTerminalAuthMethod(method) || isAcpAgentAuthMethod(method), diff --git a/src/supervisor/agents/acp/probe.test.ts b/src/supervisor/agents/acp/probe.test.ts index 6dfd1da1c..55c851a06 100644 --- a/src/supervisor/agents/acp/probe.test.ts +++ b/src/supervisor/agents/acp/probe.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from "vitest"; import { + humanizeAcpModeName, humanizeModelId, mapAcpConfigModels, mapAcpModels, @@ -190,6 +191,22 @@ describe("mapAcpModes", () => { ]); }); + it("normalizes raw snake_case mode names into readable labels", () => { + const result = mapAcpModes([ + { id: "auto", name: "auto" }, + { id: "approve", name: "approve" }, + { id: "smart_approve", name: "smart_approve" }, + { id: "chat", name: "chat" }, + ]); + expect(result.modes).toEqual(["agent"]); + expect(result.approvalPolicies).toEqual([ + { id: "auto", label: "Auto" }, + { id: "approve", label: "Approve" }, + { id: "smart_approve", label: "Smart approve" }, + { id: "chat", label: "Chat" }, + ]); + }); + it("maps unknown mode IDs as agent approval policies", () => { const result = mapAcpModes([ { id: "default", name: "Default" }, @@ -378,3 +395,19 @@ describe("normalizeAcpModeId", () => { ).toBe("autopilot"); }); }); + +describe("humanizeAcpModeName", () => { + it("replaces underscores with spaces and capitalizes the first letter", () => { + expect(humanizeAcpModeName("smart_approve")).toBe("Smart approve"); + expect(humanizeAcpModeName("auto")).toBe("Auto"); + }); + + it("leaves prose labels untouched", () => { + expect(humanizeAcpModeName("Accept Edits")).toBe("Accept Edits"); + expect(humanizeAcpModeName("YOLO")).toBe("YOLO"); + }); + + it("collapses whitespace introduced by separators", () => { + expect(humanizeAcpModeName(" read_only mode ")).toBe("Read only mode"); + }); +}); diff --git a/src/supervisor/agents/acp/probe.ts b/src/supervisor/agents/acp/probe.ts index 904f5e108..319ed42a9 100644 --- a/src/supervisor/agents/acp/probe.ts +++ b/src/supervisor/agents/acp/probe.ts @@ -58,11 +58,13 @@ export interface AcpProbeResult { authLogoutSupported?: boolean; sessionEstablished?: boolean; /** - * Auth state derived directly from the ACP handshake — `"authenticated"` + * Operational auth signal derived from the ACP handshake — `"authenticated"` * when `newSession` succeeded, `"missing"` when the agent returned the - * `auth_required` JSON-RPC error (code -32000). Left undefined when the - * probe couldn't decide (spawn / transport / non-auth errors), so callers - * fall back to their own heuristics. + * `auth_required` JSON-RPC error (code -32000). ACP does not guarantee that + * session setup validates credentials, so callers with advertised auth + * methods must not treat the success value as proof of authentication. Left + * undefined when the probe couldn't decide (spawn / transport / non-auth + * errors), so callers fall back to their own heuristics. */ authState?: AuthState; models?: Array<{ id: string; label: string; description?: string; tooltipDescription?: string }>; @@ -130,6 +132,7 @@ const INITIAL_SLASH_COMMANDS_TIMEOUT_MS = 2_000; * ACP mode ID → Poracode mode + optional approval policy ID. * * Labels come from the ACP `SessionMode.name` field, not hardcoded here. + * They are normalized for display by `humanizeAcpModeName`. */ const MODE_MAP: Record = { default: { mode: "agent", approvalPolicyId: "default" }, @@ -148,9 +151,21 @@ export function normalizeAcpModeId(modeId: string): string { return (base ?? modeId).trim(); } +/** + * Normalize an ACP-provided mode label for display. Some agents (goose) return + * the raw mode id as `SessionMode.name` (`smart_approve`), so swap underscores + * for spaces and capitalize the first letter. Labels that already read as prose + * pass through unchanged. + */ +export function humanizeAcpModeName(name: string): string { + const spaced = name.replace(/_/g, " ").replace(/\s+/g, " ").trim(); + if (!spaced) return name.trim(); + return spaced[0]!.toUpperCase() + spaced.slice(1); +} + /** * Map ACP `SessionMode[]` to Poracode modes and approval policies. - * Labels are taken from ACP's `SessionMode.name`. + * Labels are taken from ACP's `SessionMode.name`, normalized for display. */ export function mapAcpModes(availableModes: SessionMode[]): { modes: ThreadMode[]; @@ -164,12 +179,15 @@ export function mapAcpModes(availableModes: SessionMode[]): { const mapped = MODE_MAP[normalizedModeId]; if (!mapped) { modes.add("agent"); - approvalPolicies.push({ id: normalizedModeId, label: acpMode.name }); + approvalPolicies.push({ id: normalizedModeId, label: humanizeAcpModeName(acpMode.name) }); continue; } modes.add(mapped.mode); if (mapped.approvalPolicyId) { - approvalPolicies.push({ id: mapped.approvalPolicyId, label: acpMode.name }); + approvalPolicies.push({ + id: mapped.approvalPolicyId, + label: humanizeAcpModeName(acpMode.name), + }); } } diff --git a/src/supervisor/agents/acpRegistry.test.ts b/src/supervisor/agents/acpRegistry.test.ts index 4cc2700cf..099d43fa3 100644 --- a/src/supervisor/agents/acpRegistry.test.ts +++ b/src/supervisor/agents/acpRegistry.test.ts @@ -157,6 +157,71 @@ describe("ACP registry installs", () => { } }); + it.runIf(process.platform === "win32")( + "passes Windows binary archive paths to PowerShell through the child environment", + async () => { + const dir = mkdtempSync(join(tmpdir(), "poracode acp registry-")); + const settingsPath = join(dir, "settings.json"); + const registry: AcpRegistryListResult = { + version: "1.0.0", + agents: [ + { + id: "binary-agent", + name: "Binary Agent", + version: "1.0.0", + description: "Binary agent via ACP", + distribution: { + binary: { + "windows-x86_64": { + archive: "https://example.com/agent.zip", + cmd: "agent.exe", + }, + }, + }, + }, + ], + }; + vi.stubGlobal( + "fetch", + vi.fn(async () => new Response(new Uint8Array([1]), { status: 200 })), + ); + try { + await installAcpRegistryAgent({ + agentId: "binary-agent", + baseDir: dir, + settingsPath, + iconsDir: join(dir, "acp-icons"), + registry, + }); + + expect(execFileMock).toHaveBeenCalledOnce(); + const [command, args, options] = execFileMock.mock.calls[0] ?? []; + expect(command).toBe("powershell.exe"); + expect(args).toEqual([ + "-NoLogo", + "-NoProfile", + "-Command", + "Expand-Archive -LiteralPath $env:PORACODE_ACP_ARCHIVE_PATH -DestinationPath $env:PORACODE_ACP_INSTALL_DIR -Force", + ]); + expect(options).toMatchObject({ + windowsHide: true, + env: { + PORACODE_ACP_ARCHIVE_PATH: join( + dir, + "acp-registry", + "binary-agent", + "1.0.0", + "agent.zip", + ), + PORACODE_ACP_INSTALL_DIR: join(dir, "acp-registry", "binary-agent", "1.0.0", "bin"), + }, + }); + } finally { + vi.unstubAllGlobals(); + } + }, + ); + it("backfills registry icons into existing generic installs and caches them locally", async () => { const dir = mkdtempSync(join(tmpdir(), "poracode-acp-registry-")); const settingsPath = join(dir, "settings.json"); diff --git a/src/supervisor/agents/acpRegistry.ts b/src/supervisor/agents/acpRegistry.ts index 78f6482a5..a77a6390c 100644 --- a/src/supervisor/agents/acpRegistry.ts +++ b/src/supervisor/agents/acpRegistry.ts @@ -1,6 +1,6 @@ import { execFile } from "node:child_process"; import { homedir } from "node:os"; -import { copyFileSync, chmodSync, existsSync, mkdirSync, rmSync, readFileSync } from "node:fs"; +import { copyFileSync, chmodSync, existsSync, mkdirSync, readFileSync } from "node:fs"; import { writeFileAtomic } from "@/shared/atomicFile"; import { basename, dirname, join } from "node:path"; import { promisify } from "node:util"; @@ -29,12 +29,16 @@ import { clearNpxExecutionCache, isNpxCacheCorruptionError, } from "./acpRegistryNpx"; +import { + ACP_REGISTRY_INSTALL_DIR, + acpRegistryAgentInstallDir, + removeAcpRegistryInstallDir, +} from "./acpRegistryInstallDir"; import { buildAgentCommand, type AgentEnvContext } from "./base"; const execFileAsync = promisify(execFile); const ACP_REGISTRY_URL = "https://cdn.agentclientprotocol.com/registry/v1/latest/registry.json"; -const ACP_REGISTRY_INSTALL_DIR = "acp-registry"; export async function fetchAcpRegistry(): Promise { const response = await fetch(ACP_REGISTRY_URL); @@ -265,11 +269,16 @@ async function extractArchive(archivePath: string, installDir: string): Promise< "-NoLogo", "-NoProfile", "-Command", - "Expand-Archive -LiteralPath $args[0] -DestinationPath $args[1] -Force", - archivePath, - installDir, + "Expand-Archive -LiteralPath $env:PORACODE_ACP_ARCHIVE_PATH -DestinationPath $env:PORACODE_ACP_INSTALL_DIR -Force", ], - { windowsHide: true }, + { + windowsHide: true, + env: { + ...process.env, + PORACODE_ACP_ARCHIVE_PATH: archivePath, + PORACODE_ACP_INSTALL_DIR: installDir, + }, + }, ); } else { await execFileAsync("unzip", ["-q", "-o", archivePath, "-d", installDir], { @@ -306,7 +315,7 @@ async function binaryInstance( const rootDir = join(baseDir, ACP_REGISTRY_INSTALL_DIR, agent.id, agent.version); const installDir = join(rootDir, "bin"); - rmSync(installDir, { recursive: true, force: true }); + await removeAcpRegistryInstallDir(installDir); mkdirSync(installDir, { recursive: true }); const archiveName = archiveFileName(target.archive); @@ -555,11 +564,11 @@ export async function autoUpdateAcpRegistryAgents(input: { return { updated, failed }; } -export function removeAcpRegistryAgent(input: { +export async function removeAcpRegistryAgent(input: { agentId: string; baseDir: string; settingsPath: string; -}): InstalledAcpRegistryAgent[] { +}): Promise { const settings = readAcpRegistrySettings(input.settingsPath); const agentKind = acpGenericKind(input.agentId); @@ -607,8 +616,7 @@ export function removeAcpRegistryAgent(input: { writeAcpRegistrySettings(input.settingsPath, settings); - const installDir = join(input.baseDir, ACP_REGISTRY_INSTALL_DIR, input.agentId); - rmSync(installDir, { recursive: true, force: true }); + await removeAcpRegistryInstallDir(acpRegistryAgentInstallDir(input.baseDir, input.agentId)); return Object.values(settings.acpRegistryInstalledAgents); } diff --git a/src/supervisor/agents/acpRegistryInstallDir.test.ts b/src/supervisor/agents/acpRegistryInstallDir.test.ts new file mode 100644 index 000000000..ff5a0bb9e --- /dev/null +++ b/src/supervisor/agents/acpRegistryInstallDir.test.ts @@ -0,0 +1,143 @@ +import { join } from "node:path"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const fsMocks = vi.hoisted(() => ({ + rmSync: vi.fn<(...args: unknown[]) => void>(), + renameSync: vi.fn<(...args: unknown[]) => void>(), + readdirSync: vi.fn<(...args: unknown[]) => string[]>(), +})); + +const rmMock = vi.hoisted(() => vi.fn<(...args: unknown[]) => Promise>()); + +vi.mock("node:fs", async () => { + const actual = await vi.importActual("node:fs"); + return { ...actual, ...fsMocks }; +}); + +vi.mock("node:fs/promises", async () => { + const actual = await vi.importActual("node:fs/promises"); + return { ...actual, rm: rmMock }; +}); + +vi.mock("node:timers/promises", () => ({ + setTimeout: vi.fn<(ms?: number) => Promise>(async () => undefined), +})); + +import { + acpRegistryAgentInstallDir, + pruneAcpRegistryPendingDeletes, + removeAcpRegistryInstallDir, +} from "./acpRegistryInstallDir"; + +const baseDir = join("/data", "poracode"); +const installDir = acpRegistryAgentInstallDir(baseDir, "goose"); + +beforeEach(() => { + for (const mock of Object.values(fsMocks)) mock.mockReset(); + rmMock.mockReset().mockResolvedValue(undefined); + fsMocks.readdirSync.mockReturnValue([]); +}); + +function epermOnce(): Error { + return Object.assign(new Error("EPERM, Permission denied"), { code: "EPERM" }); +} + +describe("removeAcpRegistryInstallDir", () => { + it("retries a lock that clears while the killed agent finishes exiting", async () => { + fsMocks.rmSync.mockImplementationOnce(() => { + throw epermOnce(); + }); + + await removeAcpRegistryInstallDir(installDir); + + expect(fsMocks.rmSync).toHaveBeenCalledTimes(2); + expect(fsMocks.rmSync).toHaveBeenLastCalledWith(installDir, { recursive: true, force: true }); + expect(fsMocks.renameSync).not.toHaveBeenCalled(); + }); + + it("parks a permanently locked directory instead of failing the removal", async () => { + fsMocks.rmSync.mockImplementation((target) => { + if (target === installDir) throw epermOnce(); + }); + + await expect(removeAcpRegistryInstallDir(installDir)).resolves.toBeUndefined(); + + expect(fsMocks.renameSync).toHaveBeenCalledTimes(1); + const [from, to] = fsMocks.renameSync.mock.calls[0] as [string, string]; + expect(from).toBe(installDir); + expect(to.startsWith(join(baseDir, "acp-registry", ".pending-delete-goose-"))).toBe(true); + // The parked copy is swept immediately when its handle is already gone. + expect(fsMocks.rmSync).toHaveBeenLastCalledWith(to, { recursive: true, force: true }); + }); + + it("stays silent when even the rename is refused", async () => { + fsMocks.rmSync.mockImplementation(() => { + throw epermOnce(); + }); + fsMocks.renameSync.mockImplementation(() => { + throw epermOnce(); + }); + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + + await expect(removeAcpRegistryInstallDir(installDir)).resolves.toBeUndefined(); + + expect(warn).toHaveBeenCalled(); + warn.mockRestore(); + }); +}); + +describe("pruneAcpRegistryPendingDeletes", () => { + const root = join(baseDir, "acp-registry"); + + /** Mimic `readdirSync` over a real install tree keyed by absolute path. */ + function mockTree(tree: Record) { + fsMocks.readdirSync.mockImplementation((dir) => { + const entries = tree[dir as string]; + if (!entries) throw Object.assign(new Error("ENOTDIR"), { code: "ENOTDIR" }); + return entries; + }); + } + + it("deletes parked leftovers and leaves installed agents alone", async () => { + mockTree({ + [root]: [".pending-delete-goose-123-0", "goose", "gemini"], + [join(root, "goose")]: ["1.0.0"], + [join(root, "goose", "1.0.0")]: ["bin"], + [join(root, "gemini")]: [], + }); + + await pruneAcpRegistryPendingDeletes(baseDir); + + expect(rmMock).toHaveBeenCalledExactlyOnceWith(join(root, ".pending-delete-goose-123-0"), { + recursive: true, + force: true, + }); + }); + + it("sweeps a parked bin dir left by a locked binary reinstall", async () => { + // `binaryInstance` wipes `///bin`, so its parked + // copy lands two levels below the root — a root-only scan would leak it. + mockTree({ + [root]: ["goose"], + [join(root, "goose")]: ["1.0.0"], + [join(root, "goose", "1.0.0")]: [".pending-delete-bin-123-0", "bin"], + [join(root, "goose", "1.0.0", "bin")]: ["goose.exe"], + }); + + await pruneAcpRegistryPendingDeletes(baseDir); + + expect(rmMock).toHaveBeenCalledExactlyOnceWith( + join(root, "goose", "1.0.0", ".pending-delete-bin-123-0"), + { recursive: true, force: true }, + ); + }); + + it("no-ops when the registry install root does not exist", async () => { + fsMocks.readdirSync.mockImplementation(() => { + throw Object.assign(new Error("ENOENT"), { code: "ENOENT" }); + }); + + await expect(pruneAcpRegistryPendingDeletes(baseDir)).resolves.toBeUndefined(); + expect(rmMock).not.toHaveBeenCalled(); + }); +}); diff --git a/src/supervisor/agents/acpRegistryInstallDir.ts b/src/supervisor/agents/acpRegistryInstallDir.ts new file mode 100644 index 000000000..dbf923597 --- /dev/null +++ b/src/supervisor/agents/acpRegistryInstallDir.ts @@ -0,0 +1,105 @@ +import { readdirSync, renameSync, rmSync } from "node:fs"; +import { rm } from "node:fs/promises"; +import { basename, dirname, join } from "node:path"; +import { setTimeout as sleep } from "node:timers/promises"; + +export const ACP_REGISTRY_INSTALL_DIR = "acp-registry"; + +/** Marker prefix for install dirs that couldn't be unlinked yet. */ +const PENDING_DELETE_PREFIX = ".pending-delete-"; + +/** + * Windows releases a handle on a just-exited process's executable a little + * after the process is gone (and AV scanners can hold it longer), so a delete + * that immediately follows the kill fails with EPERM. Retry across ~1s before + * falling back to the rename path. + */ +const REMOVE_RETRY_DELAYS_MS = [0, 100, 250, 600]; + +/** Keeps parked directory names unique within a single supervisor process. */ +let pendingDeleteSeq = 0; + +export function acpRegistryInstallRoot(baseDir: string): string { + return join(baseDir, ACP_REGISTRY_INSTALL_DIR); +} + +export function acpRegistryAgentInstallDir(baseDir: string, agentId: string): string { + return join(acpRegistryInstallRoot(baseDir), agentId); +} + +/** + * Remove an ACP registry install directory, tolerating a locked binary. Retries + * a few times, then renames the directory out of the way — Windows allows + * renaming a directory that still holds a running executable, so the install + * path is freed immediately either way and the leftover is swept by + * {@link pruneAcpRegistryPendingDeletes} on the next launch. + * + * Never throws: the caller has already dropped the agent from settings, so a + * stubborn handle must not surface as a failed removal. + */ +export async function removeAcpRegistryInstallDir(installDir: string): Promise { + let lastError: unknown; + for (const delay of REMOVE_RETRY_DELAYS_MS) { + if (delay > 0) await sleep(delay); + try { + rmSync(installDir, { recursive: true, force: true }); + return; + } catch (error) { + lastError = error; + } + } + + const pendingDir = join( + dirname(installDir), + `${PENDING_DELETE_PREFIX}${basename(installDir)}-${process.pid}-${pendingDeleteSeq++}`, + ); + try { + renameSync(installDir, pendingDir); + } catch (error) { + console.warn(`[supervisor] could not remove ACP install dir ${installDir}:`, lastError, error); + return; + } + try { + rmSync(pendingDir, { recursive: true, force: true }); + } catch { + // Still locked — swept on the next launch. + } +} + +/** + * Parked dirs sit next to the directory they replaced. Agent removals park at + * the registry root, but a binary (re)install parks its `bin` dir two levels + * down, under `///`, so the sweep must descend that far. + */ +const PENDING_DELETE_SCAN_DEPTH = 2; + +/** + * Delete leftovers parked by {@link removeAcpRegistryInstallDir} in an earlier + * session, where the lock is guaranteed to be gone. Async so a multi-hundred-MB + * sweep never stalls the supervisor at launch. Best-effort. + */ +export async function pruneAcpRegistryPendingDeletes(baseDir: string): Promise { + await sweepPendingDeletes(acpRegistryInstallRoot(baseDir), PENDING_DELETE_SCAN_DEPTH); +} + +async function sweepPendingDeletes(dir: string, depth: number): Promise { + let entries: string[]; + try { + entries = readdirSync(dir); + } catch { + // Missing root, or a file rather than a directory — nothing to sweep here. + return; + } + for (const entry of entries) { + const path = join(dir, entry); + if (entry.startsWith(PENDING_DELETE_PREFIX)) { + try { + await rm(path, { recursive: true, force: true }); + } catch { + // Best-effort — retried on the next launch. + } + continue; + } + if (depth > 0) await sweepPendingDeletes(path, depth - 1); + } +} diff --git a/src/supervisor/crossagentMcp/toolRegistry.test.ts b/src/supervisor/crossagentMcp/toolRegistry.test.ts index 53c37bd33..5a54c9ace 100644 --- a/src/supervisor/crossagentMcp/toolRegistry.test.ts +++ b/src/supervisor/crossagentMcp/toolRegistry.test.ts @@ -589,6 +589,13 @@ describe("subagent tool registration", () => { ); }); + it("tells namespacing hosts to resolve bare tool names against the crossagents server", () => { + expect(CROSSAGENT_MCP_INSTRUCTIONS_BASE).toContain("crossagents__list_agents"); + expect(CROSSAGENT_MCP_INSTRUCTIONS_BASE).toContain( + "never the same bare name under another server", + ); + }); + it("requires an explicit user ask in the thread before delegating", () => { expect(CROSSAGENT_MCP_INSTRUCTIONS_BASE).toContain("in this thread"); expect(CROSSAGENT_MCP_INSTRUCTIONS_BASE).toContain("rest of the thread"); diff --git a/src/supervisor/crossagentMcp/toolRegistry.ts b/src/supervisor/crossagentMcp/toolRegistry.ts index b198637fd..eff8feef2 100644 --- a/src/supervisor/crossagentMcp/toolRegistry.ts +++ b/src/supervisor/crossagentMcp/toolRegistry.ts @@ -55,6 +55,7 @@ export function classifyModelTier(modelId: string, modelLabel: string): ModelTie /** Base routing guidance always included in the MCP `initialize` instructions. */ export const CROSSAGENT_MCP_INSTRUCTIONS_BASE = [ "Use the Crossagents MCP server to delegate lightweight, ephemeral work to the other AI agents connected to this Poracode session.", + "Every tool named below belongs to this server. Hosts that namespace MCP tools expose them under this server's name (for example `crossagents__list_agents` or `mcp__crossagents__list_agents`), so resolve each bare name against your own tool list and call the crossagents entry — never the same bare name under another server such as `poracode`.", "Delegate only once the user has explicitly asked you to involve another agent in this thread, for example via an @Crossagents mention or a direct request to delegate or get a second opinion. That ask authorizes delegation for the rest of the thread, so later turns may spawn as the work requires; until then, never spawn subagents on your own initiative.", "Call list_agents when provider selection matters; call get_agent only when you need one provider's detailed models, reasoning options, Fast availability, or permissions preset.", "Classify every task with 1-5 concise lowercase tags and pass the same tags to list_agents and spawn_agent. Prefer this vocabulary when applicable: frontend, ui, design, backend, mobile, simulator, implementation, bugfix, review, testing, research, refactor, docs, devops, data. Crossagents learns tag-to-selection affinity from user-explicit selection choices without an extra model call.", diff --git a/src/supervisor/runtime/agentAuthentication.test.ts b/src/supervisor/runtime/agentAuthentication.test.ts index 40f5239e1..a4ed086e5 100644 --- a/src/supervisor/runtime/agentAuthentication.test.ts +++ b/src/supervisor/runtime/agentAuthentication.test.ts @@ -164,7 +164,9 @@ describe("authenticateAcpAgent", () => { agentKind: "acp-generic:my-acp", methodId: "browser-login", }), - ).rejects.toThrow("ACP authentication was not completed."); + ).rejects.toThrow( + "My ACP reported authentication success, but Poracode could not verify it. Configure My ACP directly, then try again.", + ); const settings = JSON.parse(readFileSync(settingsPath, "utf8")) as { agentInstances: Record; diff --git a/src/supervisor/runtime/agentRegistryService.test.ts b/src/supervisor/runtime/agentRegistryService.test.ts index 1a753e9f3..666cf1ec6 100644 --- a/src/supervisor/runtime/agentRegistryService.test.ts +++ b/src/supervisor/runtime/agentRegistryService.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it, vi } from "vitest"; import type { + AgentKind, AgentStatus, GetLatestAgentVersionResult, NpmPackageVersionQuery, @@ -23,6 +24,7 @@ const acpRegistryMocks = vi.hoisted(() => ({ vi.fn(), installAcpRegistryAgent: vi.fn(), readAcpRegistrySettings: vi.fn(), + removeAcpRegistryAgent: vi.fn(), })); const getLatestVersionForAdapterMock = vi.hoisted(() => @@ -58,6 +60,7 @@ vi.mock("../agents/acpRegistry", async (importOriginal) => { cacheLocalAcpRegistryIcons: acpRegistryMocks.cacheLocalAcpRegistryIcons, installAcpRegistryAgent: acpRegistryMocks.installAcpRegistryAgent, readAcpRegistrySettings: acpRegistryMocks.readAcpRegistrySettings, + removeAcpRegistryAgent: acpRegistryMocks.removeAcpRegistryAgent, }; }); @@ -123,6 +126,7 @@ describe("AgentRegistryService.updateAgentBinary", () => { } as unknown as SupervisorSharedSettingsCache, getAgentStatusService: () => agentStatusService, getActiveWslProjectDistros: () => [], + closeThreadsForAgentKind: vi.fn<(agentKind: AgentKind) => Promise>(async () => {}), }); runUpdateCommandWithFallbackMock.mockResolvedValue({ ok: false, @@ -219,6 +223,7 @@ describe("AgentRegistryService.updateAgentBinary", () => { } as unknown as SupervisorSharedSettingsCache, getAgentStatusService: () => agentStatusService, getActiveWslProjectDistros: () => ["Ubuntu"], + closeThreadsForAgentKind: vi.fn<(agentKind: AgentKind) => Promise>(async () => {}), }); runUpdateCommandWithFallbackMock.mockResolvedValue({ ok: true, @@ -289,6 +294,7 @@ describe("AgentRegistryService.updateAgentBinary", () => { } as unknown as SupervisorSharedSettingsCache, getAgentStatusService: () => agentStatusService, getActiveWslProjectDistros: () => [], + closeThreadsForAgentKind: vi.fn<(agentKind: AgentKind) => Promise>(async () => {}), }); detectProbeLocationMock.mockReturnValueOnce({ kind: "windows", @@ -347,6 +353,7 @@ describe("AgentRegistryService.getLatestAgentVersion", () => { } as unknown as SupervisorSharedSettingsCache, getAgentStatusService: () => ({}) as unknown as AgentStatusService, getActiveWslProjectDistros: () => [], + closeThreadsForAgentKind: vi.fn<(agentKind: AgentKind) => Promise>(async () => {}), }); } @@ -401,6 +408,7 @@ describe("AgentRegistryService project-scoped ACP refreshes", () => { } satisfies ReturnType; function createService(activeWslDistros: string[]) { + const closeThreadsForAgentKind = vi.fn<(agentKind: AgentKind) => Promise>(async () => {}); const refreshAgentStatuses = vi .fn() .mockResolvedValue({ windows: [], wsl: [], fromCache: false }); @@ -421,8 +429,9 @@ describe("AgentRegistryService project-scoped ACP refreshes", () => { } as unknown as SupervisorSharedSettingsCache, getAgentStatusService: () => agentStatusService, getActiveWslProjectDistros: () => activeWslDistros, + closeThreadsForAgentKind, }); - return { listWslDistros, refreshAgentStatuses, service }; + return { closeThreadsForAgentKind, listWslDistros, refreshAgentStatuses, service }; } it("does not enumerate WSL during launch icon propagation without a WSL project", async () => { @@ -439,6 +448,24 @@ describe("AgentRegistryService project-scoped ACP refreshes", () => { expect(listWslDistros).not.toHaveBeenCalled(); }); + it("stops the agent's live threads before deleting its install", async () => { + const order: string[] = []; + acpRegistryMocks.readAcpRegistrySettings.mockReset().mockReturnValue(settings); + acpRegistryMocks.removeAcpRegistryAgent.mockReset().mockImplementation(async () => { + order.push("remove"); + return []; + }); + const { closeThreadsForAgentKind, service } = createService([]); + closeThreadsForAgentKind.mockImplementation(async () => { + order.push("close"); + }); + + await service.removeAcpRegistryAgent({ agentId: "demo" }); + + expect(closeThreadsForAgentKind).toHaveBeenCalledExactlyOnceWith("acp-generic:demo"); + expect(order).toEqual(["close", "remove"]); + }); + it("does not enumerate WSL after an ACP install without a WSL project", async () => { acpRegistryMocks.installAcpRegistryAgent.mockReset().mockResolvedValue([]); acpRegistryMocks.readAcpRegistrySettings.mockReset().mockReturnValue(settings); diff --git a/src/supervisor/runtime/agentRegistryService.ts b/src/supervisor/runtime/agentRegistryService.ts index 3b6480b65..426a561fc 100644 --- a/src/supervisor/runtime/agentRegistryService.ts +++ b/src/supervisor/runtime/agentRegistryService.ts @@ -19,6 +19,7 @@ import type { RemoveAcpRegistryAgentPayload, } from "@/shared/contracts"; import { acpGenericKind, extractAcpGenericInstanceId } from "@/shared/contracts"; +import { msg } from "@/shared/messages"; import { verifyAcpGenericAuthentication } from "../agents/acp-generic"; import { dispatchAcpAuthenticate, @@ -39,6 +40,7 @@ import { setAcpRegistryAgentAuth as setAcpRegistryAgentAuthInRegistry, updateAcpRegistryAgent as updateAcpRegistryAgentFromRegistry, } from "../agents/acpRegistry"; +import { pruneAcpRegistryPendingDeletes } from "../agents/acpRegistryInstallDir"; import { detectProbeLocation, readDetectedVersion, @@ -62,6 +64,8 @@ export interface AgentRegistryServiceDeps { sharedSettingsCache: SupervisorSharedSettingsCache; getAgentStatusService: () => AgentStatusService; getActiveWslProjectDistros: () => string[]; + /** Stop every live thread hosting `agentKind`. */ + closeThreadsForAgentKind: (agentKind: AgentKind) => Promise; } /** @@ -197,6 +201,14 @@ export class AgentRegistryService { return this.agentStatusService.refreshAgentStatuses(payload); } + /** + * Delete install directories a previous session had to park because the agent + * binary was still locked (see `removeAcpRegistryInstallDir`). + */ + async pruneAcpRegistryLeftoversOnLaunch(): Promise { + await pruneAcpRegistryPendingDeletes(this.deps.baseDir); + } + async listAcpRegistry(): Promise { const registry = await fetchAcpRegistry(); let changed = await backfillAcpRegistryAgentIcons({ @@ -355,7 +367,11 @@ export class AgentRegistryService { async removeAcpRegistryAgent( payload: RemoveAcpRegistryAgentPayload, ): Promise { - const installed = removeAcpRegistryAgentFromRegistry({ + // A thread still hosting the agent keeps its process alive, and on Windows + // the running binary locks its own install directory — the delete would + // fail with EPERM after the agent was already dropped from settings. + await this.deps.closeThreadsForAgentKind(acpGenericKind(payload.agentId)); + const installed = await removeAcpRegistryAgentFromRegistry({ agentId: payload.agentId, baseDir: this.deps.baseDir, settingsPath: this.deps.settingsPath, @@ -406,14 +422,14 @@ export class AgentRegistryService { this.deps.sharedSettingsCache.invalidate(); this.refreshAgentRegistryAdapters(); void this.refreshAffectedAgentStatus(payload.agentKind); - throw new Error("ACP authentication was not completed."); + throw new Error(msg("acp.authenticationUnverified", { agent: adapter.label })); } setAcpGenericAgentAuthAcknowledged(this.deps.settingsPath, instanceId, ctx, true); } else { const status = await adapter.detectInstall(ctx); if (status.authState === "missing") { void this.refreshAffectedAgentStatus(payload.agentKind); - throw new Error("ACP authentication was not completed."); + throw new Error(msg("acp.authenticationUnverified", { agent: adapter.label })); } } this.deps.sharedSettingsCache.invalidate(); diff --git a/src/supervisor/runtime/agentStatusCache.test.ts b/src/supervisor/runtime/agentStatusCache.test.ts index 0eabedd2c..5db7432db 100644 --- a/src/supervisor/runtime/agentStatusCache.test.ts +++ b/src/supervisor/runtime/agentStatusCache.test.ts @@ -40,7 +40,7 @@ afterEach(() => { }); describe("agent status cache", () => { - it("invalidates v10 caches produced before Codex context-window capabilities", () => { + it("invalidates v11 caches produced before successful ACP sessions established auth", () => { const dataDir = makeTempDir(); process.env.PORACODE_DATA_DIR = dataDir; @@ -49,14 +49,15 @@ describe("agent status cache", () => { writeFileSync( statusCachePath, JSON.stringify({ - version: 10, + version: 11, windows: [ { - kind: "codex", - label: "Codex", + kind: "acp-generic:example", + label: "Example ACP", installed: true, - authState: "authenticated", - capabilities: { models: [{ id: "gpt-5.6-sol", label: "GPT-5.6 Sol" }] }, + authState: "missing", + authMethods: [{ id: "login", name: "Login" }], + capabilities: { models: [] }, }, ], }), diff --git a/src/supervisor/runtime/agentStatusService.ts b/src/supervisor/runtime/agentStatusService.ts index 3def1dc43..a8f8dde6d 100644 --- a/src/supervisor/runtime/agentStatusService.ts +++ b/src/supervisor/runtime/agentStatusService.ts @@ -50,9 +50,13 @@ const execFileAsync = promisify(execFile); * invalidates v9 results whose macOS Grok probe could not find Node because * the login-shell environment was not forwarded to the ACP child process. * v11 adds Codex context-window sizes (272k/400k/1m plus a user-editable list) - * so cached statuses without those capability fields are not reused. + * so cached statuses without those capability fields are not reused. v12 + * records successful ACP session setup separately from authentication so + * advertised auth methods do not create a false Login requirement. v13 + * normalizes ACP mode labels for display, so statuses cached with raw ids + * (`smart_approve`) as approval-policy labels must be re-probed. */ -export const STATUS_CACHE_VERSION = 11; +export const STATUS_CACHE_VERSION = 13; const WSL_AGENT_DETECTION_TIMEOUT_MS = 60_000; const WSL_LXSS_REGISTRY_KEY = "HKCU\\Software\\Microsoft\\Windows\\CurrentVersion\\Lxss"; diff --git a/src/supervisor/supervisorRuntime.ts b/src/supervisor/supervisorRuntime.ts index c9f8b5905..24d37170f 100644 --- a/src/supervisor/supervisorRuntime.ts +++ b/src/supervisor/supervisorRuntime.ts @@ -209,6 +209,7 @@ export class SupervisorRuntime { sharedSettingsCache: this.sharedSettingsCache, getAgentStatusService: () => this.agentStatusService, getActiveWslProjectDistros: () => this._projectWatcher?.getWslDistros() ?? [], + closeThreadsForAgentKind: (agentKind) => this.closeThreadsForAgentKind(agentKind), }); this.agentRegistryService.refreshAgentRegistryAdapters(); mkdirSync(paths.cacheDir, { recursive: true }); @@ -544,6 +545,30 @@ export class SupervisorRuntime { // through a network round-trip on every start. No-op (no network) once all // icons are local. Fire-and-forget — never blocks the window from opening. void this.agentRegistryService.cacheLocalAcpIconsOnLaunch(); + void this.agentRegistryService.pruneAcpRegistryLeftoversOnLaunch(); + } + + /** + * Stop every live thread running `agentKind`. Deleting an ACP registry agent + * goes through here first: the thread's process is the agent, so leaving it + * running would keep an uninstalled agent alive (and on Windows keep a lock on + * the install directory being removed). Per-thread failures are logged, never + * fatal — one stuck session must not block the removal. + */ + private async closeThreadsForAgentKind(agentKind: AgentKind): Promise { + const threadIds = [...this.sessions.values()] + .filter((session) => session.agentKind === agentKind) + .map((session) => session.threadId); + await Promise.all( + threadIds.map((threadId) => + this.threadSessionManager.closeThread({ threadId }).catch((error) => { + console.warn( + `[supervisor] failed to close thread ${threadId} while removing ${agentKind}:`, + error, + ); + }), + ), + ); } getAvailableWindowsShells() {