From f8ebe9a62172bf6534abc4a728c53ec8cb4b677d Mon Sep 17 00:00:00 2001 From: "jinli.yl" Date: Thu, 5 Nov 2026 16:01:53 +0800 Subject: [PATCH] fix(dsh): finish environment lifecycle and readiness safeguards --- integrations/dsh/docs/environment.md | 14 +- integrations/dsh/docs/environment_ZH.md | 14 +- integrations/dsh/scripts/test-package.mjs | 2 +- integrations/dsh/src/client/status-page.tsx | 6 +- .../dsh/src/environment/client/dependency.tsx | 3 +- .../dsh/src/environment/client/locales.ts | 7 +- .../dsh/src/environment/client/service.tsx | 29 ++- .../dsh/src/environment/client/tab.tsx | 11 +- .../dsh/src/environment/dependency.ts | 38 +-- .../dsh/src/environment/host/install.ts | 14 +- .../dsh/src/environment/host/manager.ts | 222 ++++++------------ .../dsh/src/environment/host/runner.ts | 17 ++ .../dsh/src/environment/host/service.ts | 58 +++-- integrations/dsh/src/environment/types.ts | 1 + integrations/dsh/tests/env-manager.test.mjs | 166 +++++++++++++ integrations/dsh/tests/env-rules.test.mjs | 14 +- integrations/dsh/tests/env-service.test.mjs | 186 ++++++++++++++- integrations/dsh/tests/env-tab.test.mjs | 96 ++++++++ 18 files changed, 647 insertions(+), 251 deletions(-) diff --git a/integrations/dsh/docs/environment.md b/integrations/dsh/docs/environment.md index fbbdb05d..8fa41886 100644 --- a/integrations/dsh/docs/environment.md +++ b/integrations/dsh/docs/environment.md @@ -57,7 +57,8 @@ The check asks the selected interpreter three questions and reports them as metr - `reme-ai` metadata version (`importlib.metadata.version("reme-ai")`), and whether it satisfies the minimum this integration targets. -- `[core]` extras: whether `import agentscope` works. Without it, `auto_memory` and `auto_dream` are unavailable. +- `[core]` extras: whether `import agentscope` works. Supported ReMe releases import AgentScope while loading the + CLI, so it is required to start the service. Install or reinstall `reme-ai[core]` when it is missing. - `reme` CLI: the console script beside the interpreter, or the Python wrapper fallback used at start time. **Install reme-ai[core]** runs `python -m pip install reme-ai[core]`, optionally with a package index (PyPI, @@ -186,14 +187,18 @@ processes it manages when the Host exits. The plugin owns a small service adapte user with a process to kill by hand. - **Stop** tree-kills the recorded process (process group on POSIX, `taskkill /T /F` on Windows), waits for it to exit, escalates (POSIX: `SIGTERM` then `SIGKILL`; Windows: force `taskkill` again), and clears the record only - after the pid is gone. A service started elsewhere stays untouched and is only observed. + after the pid and the POSIX process group are gone. A child that outlives its parent still receives the force-kill. + If the group cannot be confirmed empty, its record is retained; a later unverifiable group is never signalled again. + A service started elsewhere stays untouched and is only observed. - A recorded launch that is still alive but never became healthy is shown as **Last launch not ready** with both **Stop** and **Start**: Stop clears the record, while Start makes the Host stop a provably owned previous launch or discard a proven-foreign one. It is not shown as "Starting" unless a start from this tab is actually in flight. - **Start** is serialized: two requests cannot spawn two services, and a second request observes the launch already in flight. Before replacing a recorded launch that never became healthy, Start only kills it when - ownership is proven; an unproven record is kept and reported, never silently dropped. + ownership is proven; an unproven record is kept and reported, never silently dropped. The selected interpreter's + dependencies are checked before stopping the previous service. - **Auto-start** (`autoStart`, off by default) pulls the service up when DSH starts, if it is not already running. + A refused launch is reported in the DSH Host log. Python probes and pip installs are bounded Host operations; the service is the only process that must outlive DSH. @@ -221,7 +226,8 @@ with other projects. **Saving `.env` is refused.** `LLM_API_KEY` and `EMBEDDING_API_KEY` must both be non-empty in the saved file; use `EMPTY` for the embedding key when vector search is off. Emptying a secret field never reaches the file, so the stored key is kept; a required key that is not stored at all still fails the save, and the file is left -unchanged. +unchanged. Only a missing file is treated as a first save; permission or I/O read errors abort the save without +replacing the active file or its backup. **The service reports “Port occupied”.** Something else listens on the endpoint. Change the port in the service URL (Memory settings) or stop the other program. diff --git a/integrations/dsh/docs/environment_ZH.md b/integrations/dsh/docs/environment_ZH.md index 49394855..2e075b02 100644 --- a/integrations/dsh/docs/environment_ZH.md +++ b/integrations/dsh/docs/environment_ZH.md @@ -49,7 +49,8 @@ ReMe 需要 **Python 3.11 或更新版本**。标签页在 Host 上探测下列 检查会向所选解释器询问三件事,并以指标形式展示: - `reme-ai` 元数据版本(`importlib.metadata.version("reme-ai")`),以及是否达到本集成要求的最低版本。 -- `[core]` 附加依赖:`import agentscope` 是否可用。缺失时 `auto_memory` 与 `auto_dream` 不可用。 +- `[core]` 附加依赖:`import agentscope` 是否可用。受支持的 ReMe 版本在加载 CLI 时就会导入 AgentScope,因此缺失时 + 无法启动服务;请安装或重装 `reme-ai[core]`。 - `reme` CLI:解释器旁的命令行入口,或启动时回退使用的 Python 包装。 **安装 reme-ai[core]** 执行 `python -m pip install reme-ai[core]`,可附加软件源(PyPI、清华、阿里云)。**升级**追加 @@ -146,13 +147,14 @@ ReMe 在服务启动时读取 workspace 目录下的 `.env`;标签页从不把 一样被管理:给出**停止**按钮,**启动**在它尚未健康时替换它。它不会再被当成标签页无从处理的外部进程,否则用户 只能自己去杀进程。 - **停止**会树杀记录的进程(POSIX 上按进程组、Windows 上为 `taskkill /T /F`),等待其退出,必要时升级(POSIX: - `SIGTERM` 后 `SIGKILL`;Windows:再次强制 `taskkill`),并**在 pid 真正消失之后**才清除记录。别处启动的服务不受影响, - 只会被观察。 + `SIGTERM` 后 `SIGKILL`;Windows:再次强制 `taskkill`),并**在 pid 及 POSIX 进程组真正消失之后**才清除记录。 + 父进程先退出时,仍会强制结束存活的子进程。无法确认进程组已清空就保留记录;后续无法验证身份的进程组不会再次被发信号。 + 别处启动的服务不受影响,只会被观察。 - 记录中的进程仍存活但从未就绪时,显示为**上次启动未就绪**,并同时给出**停止**与**启动**:停止负责清记录,启动则让 Host 停掉可证明归属的旧启动、或丢弃已证明属于外部的记录。只有本标签页发起的启动仍在进行时才会显示“启动中”。 - **启动**是串行化的:两个请求不会拉起两个服务,后到的请求会观察已在进行的启动。替换“一直没有就绪”的旧记录时,只有 - 归属被证明才会杀掉它;证明不了就保留记录并报错,不会静默丢弃。 -- **自动启动**(`autoStart`,默认关闭)会在 DSH 启动时把尚未运行的服务拉起。 + 归属被证明才会杀掉它;证明不了就保留记录并报错,不会静默丢弃。停止旧服务前会先检查所选解释器的启动依赖。 +- **自动启动**(`autoStart`,默认关闭)会在 DSH 启动时把尚未运行的服务拉起;被拒绝的启动会记入 DSH Host 日志。 Python 探测与 pip 安装都是有界的 Host 操作;服务是唯一需要比 DSH 活得更久的进程。 @@ -172,6 +174,8 @@ Python 探测与 pip 安装都是有界的 Host 操作;服务是唯一需要 **所选解释器不在列表里。** 标签页会用“不在探测结果中”标注它,但仍然使用它。重新探测,或改选探测到的解释器。 +**保存 `.env` 时读取失败。** 只有文件不存在才按首次保存处理;权限或 I/O 错误会中止保存,不会替换原文件或备份。 + **安装失败。** 先看日志尾部:缺少 wheel、代理配置过期或网络受限通常都能看出来,可以换一个软件源。conda 环境里 `pip` 也可 用,但独立环境更能避免与其它项目的依赖冲突。 diff --git a/integrations/dsh/scripts/test-package.mjs b/integrations/dsh/scripts/test-package.mjs index 6775cdb4..e42b5807 100644 --- a/integrations/dsh/scripts/test-package.mjs +++ b/integrations/dsh/scripts/test-package.mjs @@ -195,7 +195,7 @@ try { .map((file) => path.join("tests", file)); const tested = await run( process.execPath, - ["--test", ...testFiles], + ["--test", "--test-reporter=tap", ...testFiles], installed, `tests DSH ${release.dsh}`, ); diff --git a/integrations/dsh/src/client/status-page.tsx b/integrations/dsh/src/client/status-page.tsx index 8abf8e69..f7a4c1e2 100644 --- a/integrations/dsh/src/client/status-page.tsx +++ b/integrations/dsh/src/client/status-page.tsx @@ -1224,7 +1224,8 @@ function componentGroupLabel(group: string, t: StatusTranslator): string { file_store: "fileStore", keyword_index: "keywordIndex", }; - return labels[group] === undefined ? readableName(group) : t(labels[group]); + const key = Object.hasOwn(labels, group) ? labels[group] : undefined; + return key === undefined ? readableName(group) : t(key); } function componentFieldLabel(field: string, t: StatusTranslator): string { @@ -1242,7 +1243,8 @@ function componentFieldLabel(field: string, t: StatusTranslator): string { vocab_size: "vocabulary", memory: "memoryUsage", }; - return labels[field] === undefined ? readableName(field) : t(labels[field]); + const key = Object.hasOwn(labels, field) ? labels[field] : undefined; + return key === undefined ? readableName(field) : t(key); } function readableName(value: string): string { diff --git a/integrations/dsh/src/environment/client/dependency.tsx b/integrations/dsh/src/environment/client/dependency.tsx index 92668728..00b03fbb 100644 --- a/integrations/dsh/src/environment/client/dependency.tsx +++ b/integrations/dsh/src/environment/client/dependency.tsx @@ -1,6 +1,7 @@ import { useCallback, useEffect, useRef, useState } from "react"; import type { StatusRpc } from "../../client/status-page.js"; import { safeError } from "../../endpoint.js"; +import { canHostService } from "../dependency.js"; import { interpreterKey } from "../interpreter-key.js"; import type { InstallStatus, RemeCheck } from "../types.js"; import { fill, type EnvTranslator } from "./locales.js"; @@ -368,7 +369,7 @@ export function DependencySection({ ? undefined : !checkData.installed ? { tone: "progress" as const, text: t("depMissing") } - : checkData.versionOk && checkData.hasCore + : canHostService(checkData) ? { tone: "ok" as const, text: t("depReady") } : checkData.versionOk ? { tone: "progress" as const, text: t("coreMissingBadge") } diff --git a/integrations/dsh/src/environment/client/locales.ts b/integrations/dsh/src/environment/client/locales.ts index ed797fe7..4bd643f1 100644 --- a/integrations/dsh/src/environment/client/locales.ts +++ b/integrations/dsh/src/environment/client/locales.ts @@ -41,7 +41,7 @@ export const envEn = { notInstalled: "Not installed", upgradeRecommended: "upgrade recommended", coreOk: "Available", - coreMissing: "Missing (auto_memory / auto_dream unavailable)", + coreMissing: "Missing (required to start the service)", coreMissingBadge: "core missing", cliFallback: "python wrapper fallback", installButton: "Install reme-ai[core]", @@ -130,6 +130,8 @@ export const envEn = { "reme-ai is not installed in the selected interpreter.", startErrorVersionTooOld: "The installed reme-ai is too old; upgrade it first.", + startErrorCoreMissing: + "AgentScope is missing; install reme-ai[core] before starting the service.", startErrorFailed: "The service could not be started; check the log.", stopNoRecord: "No service was started by this plugin.", stopForeign: "The recorded service does not belong to this endpoint.", @@ -246,7 +248,7 @@ export const envZh: typeof envEn = { notInstalled: "未安装", upgradeRecommended: "建议升级", coreOk: "可用", - coreMissing: "缺失(auto_memory / auto_dream 不可用)", + coreMissing: "缺失(启动服务所必需)", coreMissingBadge: "缺少 core", cliFallback: "回退到 python 包装", installButton: "安装 reme-ai[core]", @@ -325,6 +327,7 @@ export const envZh: typeof envEn = { startErrorNoPython: "请先选择一个 Python 解释器。", startErrorNotInstalled: "所选解释器尚未安装 reme-ai。", startErrorVersionTooOld: "已安装的 reme-ai 版本过低,请先升级。", + startErrorCoreMissing: "缺少 AgentScope,请先安装 reme-ai[core] 再启动服务。", startErrorFailed: "服务未能启动,请查看日志。", stopNoRecord: "本插件没有启动过该服务。", stopForeign: "记录中的服务不属于当前服务地址。", diff --git a/integrations/dsh/src/environment/client/service.tsx b/integrations/dsh/src/environment/client/service.tsx index 9b19c2c1..f314a878 100644 --- a/integrations/dsh/src/environment/client/service.tsx +++ b/integrations/dsh/src/environment/client/service.tsx @@ -39,6 +39,7 @@ const START_ERROR_KEYS: Record = { "no-python": "startErrorNoPython", "not-installed": "startErrorNotInstalled", "version-too-old": "startErrorVersionTooOld", + "core-missing": "startErrorCoreMissing", "start-failed": "startErrorFailed", }; @@ -47,7 +48,10 @@ function startErrorText(t: EnvTranslator, result: ServiceStartResult): string { // can tell the user what to look for instead of a generic sentence. if (result.code === "unverified-record" && result.pid !== undefined) return fill(t("startErrorUnverifiedRecordPid"), { pid: result.pid }); - const key = result.code ? START_ERROR_KEYS[result.code] : undefined; + const key = + result.code && Object.hasOwn(START_ERROR_KEYS, result.code) + ? START_ERROR_KEYS[result.code] + : undefined; return key ? t(key) : result.error || t("serviceError"); } @@ -63,7 +67,10 @@ function stopReasonText( t: EnvTranslator, result: { code?: string; reason?: string }, ): string { - const key = result.code ? STOP_REASON_KEYS[result.code] : undefined; + const key = + result.code && Object.hasOwn(STOP_REASON_KEYS, result.code) + ? STOP_REASON_KEYS[result.code] + : undefined; return key ? t(key) : result.reason || t("serviceError"); } @@ -84,15 +91,15 @@ const RECORD_ONLY_CODES = new Set([ * The Host names the record it refused on: that record may belong to another * endpoint than the one on screen, and forgetting the endpoint on screen * would be refused as foreign. A refusal that names no record (a conflict, a - * missing interpreter) has no record-only way out. An older Host names no - * record either, so the endpoint on screen stays the fallback. + * missing interpreter) has no record-only way out. An older Host that names + * no record cannot safely offer Forget for the endpoint on screen. */ -function recoveryFor( - result: { code?: string; record?: ServiceRecordRef }, - endpoint: string, -): ServiceRecordRef | undefined { +function recoveryFor(result: { + code?: string; + record?: ServiceRecordRef; +}): ServiceRecordRef | undefined { if (!result.code || !RECORD_ONLY_CODES.has(result.code)) return undefined; - return result.record ?? { endpoint }; + return result.record; } export interface ServiceViewState { @@ -313,7 +320,7 @@ export function useService( // the record-only escape hatch, aimed at the record the Host named. // A refusal that names no record leaves an existing offer alone: the // record it describes is still there. - const recovery = recoveryFor(started, endpoint); + const recovery = recoveryFor(started); if (recovery) setForgetTarget(recovery); applyState({ phase: "error", error: startErrorText(t, started) }); return; @@ -415,7 +422,7 @@ export function useService( // A record the Host refuses to stop (unverifiable, or a process that // survived the force-kill) cannot be cleared by either button: offer the // record-only way out instead of leaving two actions that both fail. - const recovery = recoveryFor(result, target); + const recovery = recoveryFor(result); if (recovery) setForgetTarget(recovery); // A stopped service has no record left; a refusal that names none (a // foreign record, a missing one) leaves an existing offer alone. diff --git a/integrations/dsh/src/environment/client/tab.tsx b/integrations/dsh/src/environment/client/tab.tsx index 453e8ca0..6fef1658 100644 --- a/integrations/dsh/src/environment/client/tab.tsx +++ b/integrations/dsh/src/environment/client/tab.tsx @@ -5,7 +5,7 @@ import type { StatusRpc } from "../../client/status-page.js"; import { safeError } from "../../endpoint.js"; import { serviceEndpointKey } from "../endpoint-key.js"; import { isLocalEndpoint } from "../endpoint-rules.js"; -import { hasCoreFeatures } from "../dependency.js"; +import { canHostService } from "../dependency.js"; import type { HealthErrorCode, RemeCheck } from "../types.js"; import type { ReMeSettings } from "../../types.js"; import { DependencySection, useDependency } from "./dependency.js"; @@ -60,7 +60,10 @@ function healthErrorText( connection: ConnectionState, ): string { if (connection.phase !== "fail") return ""; - const key = connection.code ? HEALTH_ERROR_KEYS[connection.code] : undefined; + const key = + connection.code && Object.hasOwn(HEALTH_ERROR_KEYS, connection.code) + ? HEALTH_ERROR_KEYS[connection.code] + : undefined; return key ? t(key) : connection.error ?? ""; } @@ -95,7 +98,7 @@ function remeSummaryBadge( text: `${t("heroReme")} · ${t("notInstalled")}`, }; return { - tone: check.versionOk && check.hasCore ? "ok" : "progress", + tone: canHostService(check) ? "ok" : "progress", text: `${t("heroReme")} ${check.version}`, }; } @@ -215,7 +218,7 @@ export function ReMeEnvironmentTab({ const ready = current && connection.phase === "ok" && - (!local || hasCoreFeatures(dependency.checkData)); + (!local || canHostService(dependency.checkData)); const scannedAt = python.detection.data?.scannedAt ? Date.parse(python.detection.data.scannedAt) : Number.NaN; diff --git a/integrations/dsh/src/environment/dependency.ts b/integrations/dsh/src/environment/dependency.ts index 6487f4c4..83894e41 100644 --- a/integrations/dsh/src/environment/dependency.ts +++ b/integrations/dsh/src/environment/dependency.ts @@ -1,36 +1,14 @@ -/** - * What one interpreter can and cannot run. - * - * The dependency card, the hero and the service card all have to answer the - * same question about the selected interpreter, and the Host answers it again - * when a start is requested. The answers live here so the four cannot drift - * apart: a button that is offered for an interpreter the Host will refuse - * only produces an error, and a button that is hidden for one the Host would - * accept takes a capability away. - * - * This module stays browser-safe: the client imports it too. - */ +/** Shared, browser-safe readiness rule for the Host and setup cards. */ import type { RemeCheck } from "./types.js"; /** - * Whether the interpreter can host the ReMe service. - * - * This is the Host's own start gate: `reme-ai` must be installed and satisfy - * the minimum version, because the Host refuses a start otherwise with - * `not-installed` or `version-too-old`. + * Supported ReMe releases import AgentScope while loading the CLI, so a bare + * installation cannot start the service. Install `reme-ai[core]` first. */ export function canHostService(check: RemeCheck | undefined): boolean { - return check?.installed === true && check.versionOk === true; -} - -/** - * Whether the interpreter offers the whole feature set. - * - * The `[core]` extras provide automatic memory and automatic consolidation, so - * an interpreter without them runs the service but not those two jobs. The - * hero reports this state; a start is still offered, because the Host accepts - * one and search still works. - */ -export function hasCoreFeatures(check: RemeCheck | undefined): boolean { - return canHostService(check) && check?.hasCore === true; + return ( + check?.installed === true && + check.versionOk === true && + check.hasCore === true + ); } diff --git a/integrations/dsh/src/environment/host/install.ts b/integrations/dsh/src/environment/host/install.ts index b6c437b9..5a66e9f8 100644 --- a/integrations/dsh/src/environment/host/install.ts +++ b/integrations/dsh/src/environment/host/install.ts @@ -6,6 +6,7 @@ import type { InstallPhase, InstallRequest, InstallStatus } from "../types.js"; import { interpreterKey } from "../interpreter-key.js"; import { INSTALL_LOG_KEEP, openAppendLog, pruneInstallLogs } from "./log.js"; import { isAbsolutePath } from "./paths.js"; +import { processGroupAlive } from "./runner.js"; export interface InstallChild { pid: number | undefined; @@ -172,18 +173,7 @@ export function createInstallSpawner( } }, groupAlive() { - // Only POSIX puts the install in its own process group, and only - // there can the leader exit while the group outlives it. - if (process.platform === "win32" || child.pid === undefined) - return false; - try { - process.kill(-child.pid, 0); - return true; - } catch (error) { - // ESRCH means the group is empty; anything else cannot be proven - // empty, so the job waits instead of settling early. - return (error as NodeJS.ErrnoException).code !== "ESRCH"; - } + return processGroupAlive(child.pid); }, }; }, diff --git a/integrations/dsh/src/environment/host/manager.ts b/integrations/dsh/src/environment/host/manager.ts index de3a2462..34a37d4c 100644 --- a/integrations/dsh/src/environment/host/manager.ts +++ b/integrations/dsh/src/environment/host/manager.ts @@ -165,8 +165,8 @@ export class ReMeEnvManager { let existing = ""; try { existing = readFileSync(path, "utf8"); - } catch { - // First write creates the file. + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ENOENT") throw error; } const stored = parseEnvFile(existing); // The update rules are applied here as well as in the tab: a client that @@ -345,12 +345,14 @@ export class ReMeEnvManager { pythonPath = detection.suggested; } if (!pythonPath) return; - await this.serviceStart( + const result = await this.serviceStart( options.endpoint, pythonPath, options.workspaceDir, options.authToken ?? "", ); + if (!result.started && !result.alreadyRunning) + throw new Error(result.error || "Could not auto-start the ReMe service"); } /** @@ -567,41 +569,31 @@ export class ReMeEnvManager { startedAt: now, ...(current.version ? { version: current.version } : {}), }; - if (current.phase === "conflict") { - // The port is answered by something that is not a healthy service. The - // record may name that process: one of ours (still booting or rejecting - // our token) is stopped and replaced; one whose identity cannot be read - // is named, so the tab can forget it; anything else is left alone. - const { record, ownership } = await this.recordedService(); - if ( - record && - ownership === "owned" && - isLocalEndpoint(endpointValue) && - this.sameOrigin(record.endpoint, endpointValue) - ) { - const previous = await stopService(this.serviceIO, { - statePath: this.serviceStatePath, - endpoint: record.endpoint, - ownership: "owned", - }); - if (!previous.stopped) - return this.refused( - logPath, - now, - "previous-not-stopped", - `A previous launch could not be stopped: ${ - previous.reason ?? "unknown reason" - }`, - { record: previous.record }, - ); - } else if ( + const { record, ownership, alive } = await this.recordedService(); + const replace = + record !== undefined && + ownership === "owned" && + isLocalEndpoint(endpointValue) && + this.sameOrigin(record.endpoint, endpointValue); + // A port conflict is replaceable only when this record proves that our + // own service holds it. An unverified related record retains Forget. + if ( + current.phase === "conflict" && + !replace && + !( record && ownership === "unknown" && this.relatedRecord(record.endpoint, endpointValue) - ) { - // The process may be ours, but its identity cannot be read: it must - // never be signalled. The record has to be named anyway, because it is - // the only way out of a port the tab cannot clear. + ) + ) + return this.refused( + logPath, + now, + "conflict", + "Another process is already listening on this endpoint", + ); + if (record && alive) { + if (ownership === "unknown") return this.refused( logPath, now, @@ -609,117 +601,13 @@ export class ReMeEnvManager { `The recorded service for ${record.endpoint} (pid ${record.pid}) could not be verified; leaving it untouched`, { pid: record.pid, record: serviceRecordRef(record) }, ); - } else { + if (ownership === "owned" && !replace) return this.refused( logPath, now, - "conflict", - "Another process is already listening on this endpoint", + "other-endpoint-running", + `A service for ${record.endpoint} is still running; stop it from that endpoint first`, ); - } - } - // A record for a *different* endpoint that is still alive would be - // overwritten by the new launch, leaving that endpoint unable to stop it. - // A provably owned or an unverifiable process blocks the new start; only a - // provably foreign record is discarded, just like the same-endpoint case - // below. - const other = await this.recordedService(); - const recorded = other.record; - if ( - recorded && - other.alive && - serviceEndpointKey(recorded.endpoint) !== - serviceEndpointKey(endpointValue) - ) { - if (other.ownership === "owned") { - // The launch configures only host and port, so two endpoints on the - // same origin name the same local service even when their paths - // differ. A record left by a version that still launched a prefixed - // endpoint (for example `/reme`) is that service, and the endpoint on - // screen cannot reach it to stop it: replace it here instead of - // refusing with an instruction the tab cannot carry out. - // - // Only a launchable request may replace it: a prefixed request is - // refused by `buildStartCommand` below, so stopping a healthy - // same-origin service first would leave nothing running (autoStart - // would do that on every DSH start). - if ( - isLocalEndpoint(endpointValue) && - this.sameOrigin(recorded.endpoint, endpointValue) - ) { - const previous = await stopService(this.serviceIO, { - statePath: this.serviceStatePath, - endpoint: recorded.endpoint, - ownership: "owned", - }); - if (!previous.stopped) - return this.refused( - logPath, - now, - "previous-not-stopped", - `A previous launch could not be stopped: ${ - previous.reason ?? "unknown reason" - }`, - { record: previous.record }, - ); - } else - return this.refused( - logPath, - now, - "other-endpoint-running", - `A service for ${recorded.endpoint} is still running; stop it from that endpoint first`, - ); - } - // An unreadable image/command line is not proof the process is gone: - // dropping the record would orphan a live service this plugin can no - // longer stop. Keep it and report why the start is blocked. The record is - // named, so the client can offer to forget exactly this one. - if (other.ownership === "unknown") - return this.refused( - logPath, - now, - "unverified-record", - `The recorded service for ${recorded.endpoint} (pid ${recorded.pid}) could not be verified; leaving it untouched`, - { pid: recorded.pid, record: serviceRecordRef(recorded) }, - ); - removeServiceState(this.serviceStatePath); - } - // A previous launch that never became healthy may still be alive. Stop it - // when ownership is proven; an unprovable record is kept and reported, - // because dropping it would orphan a live process the tab cannot stop. - if (current.pid !== undefined) { - if (other.ownership === "owned") { - const previous = await stopService(this.serviceIO, { - statePath: this.serviceStatePath, - endpoint: endpointValue, - ownership: "owned", - }); - if (!previous.stopped) - return this.refused( - logPath, - now, - "previous-not-stopped", - `A previous launch could not be stopped: ${ - previous.reason ?? "unknown reason" - }`, - { record: previous.record }, - ); - } else if (other.ownership === "unknown") { - // PowerShell/ps failed to describe a live process. Discarding the - // record here would orphan it: the tab could never stop or replace it. - // The message and the record name the same pid: it is the one the - // confirmed Forget is pinned to. - const pid = other.record?.pid ?? current.pid; - return this.refused( - logPath, - now, - "unverified-record", - `The recorded service for ${endpointValue} (pid ${pid}) could not be verified; leaving it untouched`, - { pid, record: serviceRecordRef(other.record) }, - ); - } else { - removeServiceState(this.serviceStatePath); - } } const path = pythonPath.trim(); if (!path) @@ -732,17 +620,45 @@ export class ReMeEnvManager { const check = await this.checkReme(path); // The same rule the tab uses to offer a start decides it here, so a // button is never offered for an interpreter this refuses. - if (!canHostService(check)) - return this.refused( - logPath, - now, - check.installed ? "version-too-old" : "not-installed", - check.installed - ? `reme-ai ${ - check.version || "(unknown version)" - } is older than ${MINIMUM_REME}; upgrade it first` - : "reme-ai is not installed in the selected interpreter", - ); + if (!canHostService(check)) { + const code = !check.installed + ? "not-installed" + : !check.versionOk + ? "version-too-old" + : "core-missing"; + const errors = { + "not-installed": "reme-ai is not installed in the selected interpreter", + "version-too-old": `reme-ai ${ + check.version || "(unknown version)" + } is older than ${MINIMUM_REME}; upgrade it first`, + "core-missing": + "AgentScope is required to start ReMe; install reme-ai[core] in the selected interpreter", + }; + return this.refused(logPath, now, code, errors[code]); + } + // All replacement cases share this path. Validate the new interpreter + // before stopping anything, then re-verify ownership at the stop boundary. + if (replace && record) { + const previous = await stopService(this.serviceIO, { + statePath: this.serviceStatePath, + endpoint: record.endpoint, + expectPid: record.pid, + }); + const remaining = readServiceState(this.serviceStatePath); + if (!previous.stopped || remaining) + return this.refused( + logPath, + now, + "previous-not-stopped", + `A previous launch could not be stopped: ${ + previous.reason ?? "the service record changed while stopping" + }`, + { record: previous.record ?? serviceRecordRef(remaining) }, + ); + } else if (record) { + // Only dead or provably foreign records reach here. + removeServiceState(this.serviceStatePath); + } // Set before the spawn so a second request arriving while the service is // booting observes the first launch instead of spawning another one. It is // cleared again when the spawn fails, so a retry is possible immediately. diff --git a/integrations/dsh/src/environment/host/runner.ts b/integrations/dsh/src/environment/host/runner.ts index 5e568adf..5a76283b 100644 --- a/integrations/dsh/src/environment/host/runner.ts +++ b/integrations/dsh/src/environment/host/runner.ts @@ -1,5 +1,22 @@ import { spawn } from "node:child_process"; +/** Only ESRCH proves a detached POSIX process group is empty. */ +export function processGroupAlive(pid: number | undefined): boolean { + if ( + process.platform === "win32" || + pid === undefined || + !Number.isInteger(pid) || + pid <= 1 + ) + return false; + try { + process.kill(-pid, 0); + return true; + } catch (error) { + return (error as NodeJS.ErrnoException).code !== "ESRCH"; + } +} + export interface RunResult { code: number | null; stdout: string; diff --git a/integrations/dsh/src/environment/host/service.ts b/integrations/dsh/src/environment/host/service.ts index 511a6bd5..8feaa6cf 100644 --- a/integrations/dsh/src/environment/host/service.ts +++ b/integrations/dsh/src/environment/host/service.ts @@ -26,6 +26,7 @@ import type { } from "../types.js"; import { openAppendLog } from "./log.js"; import { isWindowsPath } from "./paths.js"; +import { processGroupAlive } from "./runner.js"; export { serviceEndpointKey, serviceOriginKey } from "../endpoint-key.js"; export { parseLocalEndpoint } from "../endpoint-rules.js"; @@ -73,6 +74,8 @@ export interface ServiceIO { */ killTree(pid: number, force?: boolean): void; probePid(pid: number): void; + /** Whether the detached POSIX process group still exists after its leader exits. */ + groupAlive?(pid: number): boolean; /** OS image name of a pid, or "" when it cannot be read. */ processImage(pid: number): Promise; /** Full command line of a pid, or "" when it cannot be read. */ @@ -208,9 +211,9 @@ function alive(io: ServiceIO, pid?: number): boolean { } } -/** Whether a pid is alive, rejecting pids this plugin never records. */ +/** Whether the recorded pid or its detached process group still exists. */ export function processAlive(io: ServiceIO, pid?: number): boolean { - return alive(io, pid); + return isValidPid(pid) && (alive(io, pid) || io.groupAlive?.(pid) === true); } /** @@ -305,7 +308,10 @@ export async function serviceOwnership( if (!persisted || !isValidPid(persisted.pid)) return "foreign"; if (serviceEndpointKey(persisted.endpoint) !== serviceEndpointKey(endpoint)) return "foreign"; - if (!alive(io, persisted.pid)) return "foreign"; + // A failed stop may leave children after the verified leader exited. Keep + // the record, but do not claim ownership we can no longer verify. + if (!alive(io, persisted.pid)) + return processAlive(io, persisted.pid) ? "unknown" : "foreign"; let image = ""; try { image = await io.processImage(persisted.pid); @@ -358,7 +364,7 @@ export async function serviceStatus( persisted !== undefined && serviceEndpointKey(persisted.endpoint) === serviceEndpointKey(options.endpoint); - const recordAlive = alive(io, persisted?.pid); + const recordAlive = processAlive(io, persisted?.pid); // A record whose process is gone must not be reported, or a later start // would keep seeing a stale pid. Remove it while nobody can be relying on it. if (persisted && !recordAlive) clearServiceState(options.statePath); @@ -562,7 +568,7 @@ function delay(ms: number): Promise { }); } -/** Poll until the pid disappears; true when it exited within `timeoutMs`. */ +/** Wait for both the leader and its detached process group to disappear. */ async function waitForExit( io: ServiceIO, pid: number, @@ -571,7 +577,7 @@ async function waitForExit( ): Promise { const deadline = Date.now() + timeoutMs; for (;;) { - if (!alive(io, pid)) return true; + if (!processAlive(io, pid)) return true; const remaining = deadline - Date.now(); if (remaining <= 0) return false; await delay(Math.max(1, Math.min(pollMs, remaining))); @@ -582,7 +588,7 @@ async function waitForExit( * Stop the service this plugin started for the endpoint, and only that one: a * service on another port, or a foreign process, is left running. * - * The record is removed only after the process is confirmed gone. Otherwise a + * The record is removed only after the process group is confirmed gone. Otherwise a * SIGTERM the service ignored would delete the pid, leaving a running process * that the tab can never stop again. */ @@ -591,12 +597,12 @@ export async function stopService( options: { statePath: string; endpoint: string; + /** Pin a replacement to the process inspected before dependency checks. */ + expectPid?: number; /** Grace period before escalating to SIGKILL (tests shorten it). */ timeoutMs?: number; forceTimeoutMs?: number; pollMs?: number; - /** Precomputed ownership from the caller; avoids a second process probe. */ - ownership?: ServiceOwnership; }, ): Promise { const persisted = readServiceState(options.statePath); @@ -608,9 +614,14 @@ export async function stopService( code: "no-record", }; } - const ownership = - options.ownership ?? - (await serviceOwnership(io, persisted, options.endpoint)); + if (options.expectPid !== undefined && persisted.pid !== options.expectPid) + return { + stopped: false, + reason: "The recorded service changed since it was inspected", + code: "record-changed", + record: serviceRecordRef(persisted), + }; + const ownership = await serviceOwnership(io, persisted, options.endpoint); if (ownership === "unknown") return { stopped: false, @@ -626,6 +637,24 @@ export async function stopService( code: "foreign", }; const pid = persisted.pid; + const isCurrentRecord = (): boolean => { + const current = readServiceState(options.statePath); + return ( + current?.pid === pid && + current.endpoint === persisted.endpoint && + current.startedAt === persisted.startedAt + ); + }; + // Identity reads yield to the event loop. Never signal a record that was + // replaced while those reads were in flight, or erase a newer launch later. + if (!isCurrentRecord()) + return { + stopped: false, + pid, + reason: "The recorded service changed since it was inspected", + code: "record-changed", + record: serviceRecordRef(readServiceState(options.statePath)), + }; const pollMs = options.pollMs ?? STOP_POLL_MS; try { io.killTree(pid); @@ -635,7 +664,7 @@ export async function stopService( if ( await waitForExit(io, pid, options.timeoutMs ?? STOP_TIMEOUT_MS, pollMs) ) { - clearServiceState(options.statePath); + if (isCurrentRecord()) clearServiceState(options.statePath); return { stopped: true, pid }; } // The service ignored (or slowly handled) SIGTERM: escalate, then only drop @@ -653,7 +682,7 @@ export async function stopService( pollMs, ) ) { - clearServiceState(options.statePath); + if (isCurrentRecord()) clearServiceState(options.statePath); return { stopped: true, pid }; } return { @@ -863,6 +892,7 @@ export function createServiceIO(): ServiceIO { throw new RangeError("Refusing to probe an invalid pid"); process.kill(pid, 0); }, + groupAlive: processGroupAlive, processImage(pid) { return new Promise((resolve) => { if (!isValidPid(pid)) { diff --git a/integrations/dsh/src/environment/types.ts b/integrations/dsh/src/environment/types.ts index a4e1e299..8ad1ddc5 100644 --- a/integrations/dsh/src/environment/types.ts +++ b/integrations/dsh/src/environment/types.ts @@ -164,6 +164,7 @@ export type ServiceStartErrorCode = | "no-python" | "not-installed" | "version-too-old" + | "core-missing" | "start-failed"; /** Stable reason for a refused/unsuccessful stop, so the client can localize it. */ diff --git a/integrations/dsh/tests/env-manager.test.mjs b/integrations/dsh/tests/env-manager.test.mjs index 823979fe..711eeffe 100644 --- a/integrations/dsh/tests/env-manager.test.mjs +++ b/integrations/dsh/tests/env-manager.test.mjs @@ -1,5 +1,7 @@ import assert from "node:assert/strict"; import test from "node:test"; +import fs from "node:fs"; +import { syncBuiltinESMExports } from "node:module"; import { mkdirSync, mkdtempSync, @@ -139,6 +141,130 @@ test("a failed spawn can be retried immediately", async (t) => { assert.equal(calls.spawn.length, 2); }); +test("auto-start reports a refused launch to the Host logger boundary", async (t) => { + const { manager, directory, calls } = managerIn(t); + manager.checkReme = async () => ({ + installed: true, + version: "0.4.2.1", + versionOk: true, + hasCore: false, + cliPath: "", + error: "", + }); + await assert.rejects( + manager.autoStart({ + endpoint: "http://127.0.0.1:2334", + pythonPath: "/env/bin/python", + workspaceDir: directory, + }), + /AgentScope/, + ); + assert.equal(calls.spawn.length, 0); +}); + +test("missing core dependencies leave the previous service untouched", async (t) => { + const { manager, directory, calls, io } = managerIn(t); + const statePath = join(directory, "reme-memory", "service.json"); + writeServiceState(statePath, { + pid: 6000, + endpoint: "http://127.0.0.1:2334", + workspaceDir: directory, + pythonPath: "/old/bin/python", + command: "reme start", + startedAt: "old", + }); + io.processCommandLine = async () => + `reme start workspace_dir=${directory} service.port=2334`; + io.killTree = (pid) => calls.killTree.push(pid); + manager.checkReme = async () => ({ + installed: true, + version: "0.4.2.1", + versionOk: true, + hasCore: false, + cliPath: "", + error: "", + }); + const result = await manager.serviceStart( + "http://127.0.0.1:2334", + "/bare/bin/python", + directory, + ); + assert.equal(result.started, false); + assert.equal(result.code, "core-missing"); + assert.match(result.error, /AgentScope/); + assert.deepEqual(calls.killTree, []); + assert.deepEqual(calls.spawn, []); + assert.equal(readServiceState(statePath)?.pid, 6000); +}); + +test("a replacement never stops a launch recorded during the dependency check", async (t) => { + const { manager, directory, calls, io } = managerIn(t); + const statePath = join(directory, "reme-memory", "service.json"); + const original = { + pid: 6000, + endpoint: "http://127.0.0.1:2334", + workspaceDir: directory, + pythonPath: "/old/bin/python", + command: "reme start", + startedAt: "old", + }; + writeServiceState(statePath, original); + io.processCommandLine = async () => + `reme start workspace_dir=${directory} service.port=2334`; + io.killTree = (pid) => calls.killTree.push(pid); + const check = manager.checkReme; + manager.checkReme = async (path) => { + writeServiceState(statePath, { ...original, pid: 7000, startedAt: "new" }); + return check(path); + }; + const result = await manager.serviceStart( + original.endpoint, + "/new/bin/python", + directory, + ); + assert.equal(result.code, "previous-not-stopped"); + assert.deepEqual(result.record, { endpoint: original.endpoint, pid: 7000 }); + assert.deepEqual(calls.killTree, []); + assert.deepEqual(calls.spawn, []); + assert.equal(readServiceState(statePath)?.pid, 7000); +}); + +test("a replacement never overwrites a launch recorded while the old service stops", async (t) => { + const { manager, directory, calls, io } = managerIn(t); + const statePath = join(directory, "reme-memory", "service.json"); + const original = { + pid: 6000, + endpoint: "http://127.0.0.1:2334", + workspaceDir: directory, + pythonPath: "/old/bin/python", + command: "reme start", + startedAt: "old", + }; + writeServiceState(statePath, original); + io.processCommandLine = async () => + `reme start workspace_dir=${directory} service.port=2334`; + let running = true; + io.probePid = (pid) => { + if (pid === 6000 && !running) + throw Object.assign(new Error("gone"), { code: "ESRCH" }); + }; + io.killTree = (pid) => { + calls.killTree.push(pid); + running = false; + writeServiceState(statePath, { ...original, pid: 7000, startedAt: "new" }); + }; + const result = await manager.serviceStart( + original.endpoint, + "/new/bin/python", + directory, + ); + assert.equal(result.code, "previous-not-stopped"); + assert.deepEqual(result.record, { endpoint: original.endpoint, pid: 7000 }); + assert.deepEqual(calls.killTree, [6000]); + assert.deepEqual(calls.spawn, []); + assert.equal(readServiceState(statePath)?.pid, 7000); +}); + test("refuses to spawn while another process owns the port", async (t) => { const { manager, calls, directory } = managerIn(t); manager.serviceStatus = async (endpoint) => ({ @@ -863,3 +989,43 @@ test("writes .env atomically with a backup and owner-only mode", (t) => { if (process.platform !== "win32") assert.equal(statSync(envPath).mode & 0o777, 0o600); }); + +for (const code of ["EIO", "EACCES"]) { + test(`a ${code} .env read failure preserves the file and backup`, (t) => { + const { manager, directory } = managerIn(t); + const envPath = join(directory, ".env"); + const original = "# keep\r\nLLM_API_KEY=old\r\nUNRELATED=keep\r\n"; + writeFileSync(envPath, original); + writeFileSync(`${envPath}.bak`, "previous backup"); + const read = fs.readFileSync; + let injected = false; + const stub = t.mock.method(fs, "readFileSync", (path, ...args) => { + if (path === envPath && !injected) { + injected = true; + throw Object.assign(new Error(code), { code }); + } + return read(path, ...args); + }); + syncBuiltinESMExports(); + try { + assert.throws(() => manager.writeEnv(directory, { LLM_API_KEY: "new" }), { + code, + }); + } finally { + stub.mock.restore(); + syncBuiltinESMExports(); + } + assert.equal(injected, true); + assert.equal(readFileSync(envPath, "utf8"), original); + assert.equal(readFileSync(`${envPath}.bak`, "utf8"), "previous backup"); + }); +} + +test("a missing .env can still be created on the first save", (t) => { + const { manager, directory } = managerIn(t); + manager.writeEnv(directory, { LLM_API_KEY: "new" }); + assert.match( + readFileSync(join(directory, ".env"), "utf8"), + /^LLM_API_KEY=new$/m, + ); +}); diff --git a/integrations/dsh/tests/env-rules.test.mjs b/integrations/dsh/tests/env-rules.test.mjs index 820c070f..5425cc08 100644 --- a/integrations/dsh/tests/env-rules.test.mjs +++ b/integrations/dsh/tests/env-rules.test.mjs @@ -4,10 +4,7 @@ import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { - canHostService, - hasCoreFeatures, -} from "../dist/environment/dependency.js"; +import { canHostService } from "../dist/environment/dependency.js"; import { isLocalEndpoint, parseLocalEndpoint, @@ -45,8 +42,7 @@ const INSTALLED = { }; test("the interpreter readiness has one definition", () => { - // The Host refuses a start unless reme-ai is installed and new enough, so - // the tab must offer exactly the same starts. + // Supported CLI releases need AgentScope even before running any jobs. assert.equal(canHostService(INSTALLED), true); assert.equal(canHostService(undefined), false); assert.equal( @@ -55,11 +51,7 @@ test("the interpreter readiness has one definition", () => { ); assert.equal(canHostService({ ...INSTALLED, versionOk: false }), false); - // The [core] extras are a stronger state: without them the service still - // starts, but automatic memory and consolidation do not. - assert.equal(hasCoreFeatures(INSTALLED), true); - assert.equal(hasCoreFeatures({ ...INSTALLED, hasCore: false }), false); - assert.equal(hasCoreFeatures({ ...INSTALLED, versionOk: false }), false); + assert.equal(canHostService({ ...INSTALLED, hasCore: false }), false); }); test("only a loopback HTTP endpoint is a manageable local service", () => { diff --git a/integrations/dsh/tests/env-service.test.mjs b/integrations/dsh/tests/env-service.test.mjs index 66bb4ed3..9c4964d4 100644 --- a/integrations/dsh/tests/env-service.test.mjs +++ b/integrations/dsh/tests/env-service.test.mjs @@ -1,7 +1,7 @@ import assert from "node:assert/strict"; import test from "node:test"; import { createServer } from "node:http"; -import { mkdtempSync, rmSync } from "node:fs"; +import { mkdtempSync, readFileSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -541,6 +541,101 @@ test("refuses to stop a service recorded for another endpoint", async (t) => { assert.ok(readServiceState(statePath), "the foreign record is left in place"); }); +for (const exits of [true, false]) { + test(`a parent-first exit ${ + exits ? "waits for" : "retains" + } its process group`, async (t) => { + const { directory, statePath } = tempState(t, "reme-stop-group-"); + record(statePath, { pid: 999, workspaceDir: directory }); + let parent = true; + let group = true; + const { io, calls } = fakeIO({ + platform: "linux", + processCommandLine: async () => commandFor(statePath), + probePid() { + if (!parent) throw Object.assign(new Error("gone"), { code: "ESRCH" }); + }, + groupAlive: () => group, + killTree(pid, force = false) { + calls.killTree.push(pid); + parent = false; + if (force) { + calls.force.push(pid); + group = !exits; + } + }, + }); + const result = await stopService(io, { + statePath, + endpoint: ENDPOINT, + timeoutMs: 10, + forceTimeoutMs: 10, + pollMs: 2, + }); + assert.deepEqual(calls.force, [999], "the surviving child gets SIGKILL"); + assert.equal(result.stopped, exits); + if (exits) { + assert.equal(readServiceState(statePath), undefined); + } else { + assert.equal(result.code, "not-exited"); + const status = await serviceStatus(io, { + statePath, + endpoint: ENDPOINT, + logPath: join(directory, "service.log"), + }); + assert.equal(readServiceState(statePath)?.pid, 999); + assert.equal(status.pid, 999); + assert.equal( + await serviceOwnership(io, readServiceState(statePath), ENDPOINT), + "unknown", + ); + // A later stop no longer has a leader whose identity it can verify. + const later = await stopService(io, { statePath, endpoint: ENDPOINT }); + assert.equal(later.code, "unverified"); + assert.deepEqual( + calls.force, + [999], + "an unverified group is not signalled again", + ); + } + }); +} + +test("a stop never signals a record replaced during the ownership probe", async (t) => { + const { statePath } = tempState(t, "reme-stop-race-"); + record(statePath); + const command = commandFor(statePath); + const { io, calls } = fakeIO({ + processCommandLine: async () => { + record(statePath, { pid: 888, startedAt: "new" }); + return command; + }, + }); + const result = await stopService(io, { statePath, endpoint: ENDPOINT }); + assert.equal(result.code, "record-changed"); + assert.deepEqual(calls.killTree, []); + assert.equal(readServiceState(statePath)?.pid, 888); +}); + +test("finishing an old stop does not erase a newer service record", async (t) => { + const { statePath } = tempState(t, "reme-stop-new-record-"); + record(statePath); + let running = true; + const { io } = fakeIO({ + processCommandLine: async () => commandFor(statePath), + probePid() { + if (!running) throw Object.assign(new Error("gone"), { code: "ESRCH" }); + }, + killTree() { + running = false; + record(statePath, { pid: 888, startedAt: "new" }); + }, + }); + const result = await stopService(io, { statePath, endpoint: ENDPOINT }); + assert.equal(result.stopped, true); + assert.equal(readServiceState(statePath)?.pid, 888); +}); + test("refuses to stop a process whose identity cannot be verified", async (t) => { const { statePath } = tempState(t, "reme-unknown-"); record(statePath); @@ -710,3 +805,92 @@ test("real IO spawns, identifies and stops a detached process", async (t) => { } assert.equal(running, false, "the detached process must actually exit"); }); + +test( + "real POSIX stop kills a listening child after its parent exits", + { + skip: process.platform === "win32", + timeout: 15_000, + }, + async (t) => { + const { directory, statePath } = tempState(t, "reme-real-group-"); + const io = createServiceIO(); + const logPath = join(directory, "service.log"); + const childCode = ` + process.on('SIGTERM', () => {}); + const server = require('node:net').createServer(socket => socket.end()); + server.listen(0, '127.0.0.1', () => { + console.log(JSON.stringify({ port: server.address().port, pid: process.pid })); + }); + `; + const parentCode = ` + process.on('SIGTERM', () => process.exit(0)); + const child = require('node:child_process').spawn(process.execPath, + ['-e', ${JSON.stringify( + childCode, + )}], { stdio: ['ignore', 'pipe', 'ignore'] }); + child.stdout.pipe(process.stdout); + `; + const { pid } = await io.spawnService({ + exe: process.execPath, + args: [ + "-e", + parentCode, + `workspace_dir=${directory}`, + "service.port=2334", + ], + cwd: directory, + logPath, + env: process.env, + }); + const kill = io.killTree; + t.after(() => kill(pid, true)); + let ready; + const deadline = Date.now() + 5_000; + while (!ready && Date.now() < deadline) { + try { + ready = JSON.parse(readFileSync(logPath, "utf8").trim()); + } catch { + /* not listening yet */ + } + if (!ready) await new Promise((resolve) => setTimeout(resolve, 20)); + } + assert.ok(ready, "the child listener must be ready before signalling"); + assert.equal(await io.portOpen("127.0.0.1", ready.port), true); + record(statePath, { pid, workspaceDir: directory }); + // The fixture uses Node instead of Python; spawn, command line, signals, + // group liveness and socket checks still use the real service adapter. + io.processImage = async () => "python"; + let escalated = false; + io.killTree = (target, force) => { + if (force) { + escalated = true; + assert.throws(() => io.probePid(pid), { code: "ESRCH" }); + assert.doesNotThrow(() => io.probePid(ready.pid)); + } + kill(target, force); + }; + const result = await stopService(io, { + statePath, + endpoint: ENDPOINT, + timeoutMs: 200, + forceTimeoutMs: 2_000, + pollMs: 20, + }); + assert.equal( + escalated, + true, + "parent exit must not bypass child termination", + ); + assert.equal(await io.portOpen("127.0.0.1", ready.port), false); + // Some container init processes leave orphaned zombies unreaped. A group + // still visible to signal 0 conservatively retains its record. + if (result.stopped) { + assert.equal(io.groupAlive(pid), false); + assert.equal(readServiceState(statePath), undefined); + } else { + assert.equal(result.code, "not-exited"); + assert.equal(readServiceState(statePath)?.pid, pid); + } + }, +); diff --git a/integrations/dsh/tests/env-tab.test.mjs b/integrations/dsh/tests/env-tab.test.mjs index ec3b74f7..530bcd78 100644 --- a/integrations/dsh/tests/env-tab.test.mjs +++ b/integrations/dsh/tests/env-tab.test.mjs @@ -230,6 +230,99 @@ test("renders the status hero and the four setup steps", async (t) => { assert.match(copy, /C:\\Users\\test\\\.reme\\\.env/); }); +test("an interpreter without AgentScope cannot offer Start or report ready", async (t) => { + const { renderer, restore } = await mount( + {}, + { + responses: { + "remeEnv/checkReme": { + ...RESPONSES["remeEnv/checkReme"], + hasCore: false, + }, + "remeEnv/serviceStatus": { + ...RESPONSES["remeEnv/serviceStatus"], + phase: "stopped", + pid: undefined, + }, + }, + }, + ); + t.after(restore); + assert.equal(buttonByText(renderer, envZh.startService).props.disabled, true); + const copy = texts(renderer.toJSON()).join(" "); + assert.ok(copy.includes(envZh.coreMissing)); + assert.equal(copy.includes(envZh.heroReady), false); +}); + +for (const [code, error, expected] of [ + ["core-missing", "AgentScope missing", envZh.startErrorCoreMissing], + ["constructor", "Unknown Host refusal", "Unknown Host refusal"], +]) { + test(`localizes or safely falls back for the ${code} start refusal`, async (t) => { + const { renderer, restore } = await mount( + {}, + { + responses: { + "remeEnv/serviceStatus": { + ...RESPONSES["remeEnv/serviceStatus"], + phase: "stopped", + pid: undefined, + }, + "remeEnv/serviceStart": { + started: false, + alreadyRunning: false, + command: "", + logPath: "", + startedAt: new Date().toISOString(), + code, + error, + }, + }, + }, + ); + t.after(restore); + await act(async () => + buttonByText(renderer, envZh.startService).props.onClick(), + ); + assert.ok(texts(renderer.toJSON()).join(" ").includes(expected)); + }); +} + +test("an older Host refusal without a record never guesses a Forget target", async (t) => { + const { renderer, calls, restore } = await mount( + {}, + { + responses: { + "remeEnv/serviceStatus": { + ...RESPONSES["remeEnv/serviceStatus"], + phase: "stopped", + pid: undefined, + }, + "remeEnv/serviceStart": { + started: false, + alreadyRunning: false, + command: "", + logPath: "", + startedAt: new Date().toISOString(), + code: "unverified-record", + }, + }, + }, + ); + t.after(restore); + await act(async () => + buttonByText(renderer, envZh.startService).props.onClick(), + ); + assert.equal( + texts(renderer.toJSON()).join(" ").includes(envZh.forgetRecord), + false, + ); + assert.equal( + calls.rpc.some((call) => call.method === "remeEnv/serviceForget"), + false, + ); +}); + test("keeps the interpreter list collapsed until asked, then paginates", async (t) => { const { renderer, restore } = await mount(); t.after(restore); @@ -474,6 +567,7 @@ test("shows why Stop left the service alone", async (t) => { stopped: false, reason: "The recorded process identity could not be verified", code: "unverified", + record: { endpoint: "http://127.0.0.1:2333", pid: 98820 }, }, }, }, @@ -514,6 +608,7 @@ test("a process that survived the force-kill still offers forgetting the record" stopped: false, reason: "The service did not exit after the force-kill", code: "not-exited", + record: { endpoint: "http://127.0.0.1:2333", pid: 98820 }, }, }, }, @@ -1335,6 +1430,7 @@ test("an unverified record reports the pid it refused to touch", async (t) => { alreadyRunning: false, error: "The recorded service identity could not be verified", code: "unverified-record", + record: { endpoint: "http://127.0.0.1:2333", pid: 6000 }, pid: 6000, command: "", logPath: stopped.logPath,