From fddeb8d710c490ac49c1d1a33a7539781a6d0116 Mon Sep 17 00:00:00 2001 From: LiangshouX Date: Wed, 28 Oct 2026 23:13:49 +0800 Subject: [PATCH] fix(dsh): harden environment management from review - wire the package mirror into installs - wait for service exit, escalate to SIGKILL, keep the record on failure - fail closed on ownership and clear dead service records - clear the start lock on failure and reject overlapping starts - bound probes, make config writes atomic, scope auth tokens - fix docs and add regression tests --- integrations/dsh/README.md | 36 +- integrations/dsh/README_ZH.md | 36 +- integrations/dsh/docs/environment.md | 22 +- integrations/dsh/docs/environment_ZH.md | 19 +- integrations/dsh/src/client/index.tsx | 138 ++++---- integrations/dsh/src/client/settings-specs.ts | 18 +- integrations/dsh/src/client/status-page.tsx | 3 + .../dsh/src/environment/client/dependency.tsx | 50 ++- .../dsh/src/environment/client/locales.ts | 14 +- .../dsh/src/environment/client/python.tsx | 13 +- .../dsh/src/environment/client/rpc.ts | 154 +++++++-- .../dsh/src/environment/client/service.tsx | 76 ++++- .../dsh/src/environment/client/tab.tsx | 82 ++++- .../dsh/src/environment/client/workspace.tsx | 31 +- .../dsh/src/environment/host/detect.ts | 28 +- .../dsh/src/environment/host/install.ts | 74 +++- .../dsh/src/environment/host/manager.ts | 127 +++++-- .../dsh/src/environment/host/paths.ts | 13 + integrations/dsh/src/environment/host/reme.ts | 22 +- .../dsh/src/environment/host/runner.ts | 47 ++- .../dsh/src/environment/host/service.ts | 315 +++++++++++++++--- .../dsh/src/environment/host/workspace.ts | 40 ++- integrations/dsh/src/environment/types.ts | 2 + integrations/dsh/src/reme/client.ts | 12 +- integrations/dsh/tests/client.test.mjs | 26 ++ integrations/dsh/tests/env-detect.test.mjs | 43 +++ integrations/dsh/tests/env-manager.test.mjs | 83 ++++- integrations/dsh/tests/env-reme.test.mjs | 12 + integrations/dsh/tests/env-runner.test.mjs | 30 ++ integrations/dsh/tests/env-service.test.mjs | 172 +++++++++- integrations/dsh/tests/env-tab.test.mjs | 37 +- 31 files changed, 1458 insertions(+), 317 deletions(-) create mode 100644 integrations/dsh/tests/env-runner.test.mjs diff --git a/integrations/dsh/README.md b/integrations/dsh/README.md index 4fe00b44..0fe1be7b 100644 --- a/integrations/dsh/README.md +++ b/integrations/dsh/README.md @@ -110,24 +110,24 @@ If an earlier upgrade already renamed `settings.yaml` to `settings.yaml.imported ![ReMe Memory plugin configuration](./figures/reme-memory-settings.png) -| UI meaning | Configuration key | Default | Description | -| ---------------------- | ---------------------- | ----------------------- | --------------------------------------------------------------------------------------------------- | -| Service URL | `endpoint` | `http://127.0.0.1:2333` | Absolute HTTP(S) URL without credentials, query, or fragment. | -| Auth token (optional) | `authToken` | empty | Bearer token for a self-hosted gateway; stored in clear text in the profile, trusted networks only. | -| Guidance language | `language` | `en` | `en` or `zh`; controls guidance injected into new sessions. | -| Default search results | `searchLimit` | `5` | Default `reme_search` result limit, from 1 to 50. | -| Search timeout | `requestTimeoutMs` | `10000` | Search timeout in milliseconds, from 1,000 to 120,000. | -| Automatic memory | `autoMemoryEnabled` | `true` | Capture completed user/assistant turns for `auto_memory`. | -| Exclude subagents | `rootAgentsOnly` | `true` | Inject guidance and capture conversations only for root agents. | -| Submission interval | `autoMemoryInterval` | `5` | Submit after this many completed turns, from 1 to 1,000. | -| Memory consolidation | `autoDreamEnabled` | `true` | Run `auto_dream` on the daily schedule. | -| Consolidation schedule | `dreamCron` | `0 23 * * *` | Five-field cron expression interpreted in `timezone`. | -| Consolidation guidance | `dreamHint` | empty | Optional guidance passed to `auto_dream`. | -| Workspace timezone | `timezone` | `Asia/Shanghai` | IANA timezone used for batching and scheduling. | -| Background timeout | `backgroundTimeoutMs` | `3600000` | Timeout for `auto_memory` and `auto_dream`. | -| Shutdown flush timeout | `shutdownTimeoutMs` | `5000` | Budget for draining background work during shutdown. | -| Retry delay | `autoMemoryRetryMs` | `1000` | Initial retry delay; doubles after each failure. | -| Maximum retry delay | `autoMemoryRetryMaxMs` | `60000` | Upper bound for retry delay. | +| UI meaning | Configuration key | Default | Description | +| ---------------------- | ---------------------- | ----------------------- | -------------------------------------------------------------------------------------------------------------------------------------- | +| Service URL | `endpoint` | `http://127.0.0.1:2333` | Absolute HTTP(S) URL without credentials, query, or fragment. | +| Auth token (optional) | `authToken` | empty | Bearer token for a self-hosted gateway; set in the profile only (not shown in this form), stored in clear text, trusted networks only. | +| Guidance language | `language` | `en` | `en` or `zh`; controls guidance injected into new sessions. | +| Default search results | `searchLimit` | `5` | Default `reme_search` result limit, from 1 to 50. | +| Search timeout | `requestTimeoutMs` | `10000` | Search timeout in milliseconds, from 1,000 to 120,000. | +| Automatic memory | `autoMemoryEnabled` | `true` | Capture completed user/assistant turns for `auto_memory`. | +| Exclude subagents | `rootAgentsOnly` | `true` | Inject guidance and capture conversations only for root agents. | +| Submission interval | `autoMemoryInterval` | `5` | Submit after this many completed turns, from 1 to 1,000. | +| Memory consolidation | `autoDreamEnabled` | `true` | Run `auto_dream` on the daily schedule. | +| Consolidation schedule | `dreamCron` | `0 23 * * *` | Five-field cron expression interpreted in `timezone`. | +| Consolidation guidance | `dreamHint` | empty | Optional guidance passed to `auto_dream`. | +| Workspace timezone | `timezone` | `Asia/Shanghai` | IANA timezone used for batching and scheduling. | +| Background timeout | `backgroundTimeoutMs` | `3600000` | Timeout for `auto_memory` and `auto_dream`. | +| Shutdown flush timeout | `shutdownTimeoutMs` | `5000` | Budget for draining background work during shutdown. | +| Retry delay | `autoMemoryRetryMs` | `1000` | Initial retry delay; doubles after each failure. | +| Maximum retry delay | `autoMemoryRetryMaxMs` | `60000` | Upper bound for retry delay. | Deployment configuration also supports `REME_URL`, or `REME_HOST` together with `REME_PORT`. The timer-only test option `dreamIntervalMs` is intentionally excluded from user settings. diff --git a/integrations/dsh/README_ZH.md b/integrations/dsh/README_ZH.md index f9fcc5ca..67cf147b 100644 --- a/integrations/dsh/README_ZH.md +++ b/integrations/dsh/README_ZH.md @@ -115,24 +115,24 @@ dsh web --no-open --port 3090 截图中的测试配置使用 `http://127.0.0.1:3457`、English 指引、默认搜索数量 5、搜索超时 10 秒，并启用了自动记忆和“Exclude subagents”。完整字段如下： -| 界面含义 | 配置键 | 默认值 | 说明 | -| -------------------- | ---------------------- | ----------------------- | -------------------------------------------------------------------------- | -| 服务地址 | `endpoint` | `http://127.0.0.1:2333` | DSH 访问的绝对 HTTP(S) 地址，不支持 URL 内嵌凭据、查询参数或片段。 | -| 服务令牌（可选） | `authToken` | 空 | 自部署网关的 Bearer 令牌；明文保存在 profile 中，仅建议内网/受信环境使用。 | -| 记忆指引语言 | `language` | `en` | `en` 或 `zh`；决定新会话中注入的指引语言。 | -| 默认搜索数量 | `searchLimit` | `5` | `reme_search` 未传 `limit` 时的默认结果上限，范围 1–50。 | -| 搜索超时（毫秒） | `requestTimeoutMs` | `10000` | `search` 请求的超时，范围 1,000–120,000。 | -| 自动记录已完成的对话 | `autoMemoryEnabled` | `true` | 是否捕获完成的用户/助手回合并提交给 `auto_memory`。 | -| 排除子 Agent | `rootAgentsOnly` | `true` | 开启时，只为根 Agent 注入指引并捕获对话。 | -| 自动记忆提交间隔 | `autoMemoryInterval` | `5` | 每完成多少轮对话提交一次，范围 1–1,000。退出时插件会在时间预算内尽力排空。 | -| 自动整理记忆 | `autoDreamEnabled` | `true` | 是否按照每日计划调用 `auto_dream`。 | -| 整理计划 | `dreamCron` | `0 23 * * *` | 五段 cron 表达式，按 `timezone` 解释；默认每天 23:00。 | -| 整理指引 | `dreamHint` | 空 | 可选的整理提示，原样交给 `auto_dream`。 | -| 工作区时区 | `timezone` | `Asia/Shanghai` | 有效的 IANA 时区，用于每日批次和 cron 计划。 | -| 后台任务超时（毫秒） | `backgroundTimeoutMs` | `3600000` | `auto_memory` 和 `auto_dream` 的最大等待时间。 | -| 退出等待（毫秒） | `shutdownTimeoutMs` | `5000` | DSH 退出时等待后台任务排空的时间预算。 | -| 失败重试间隔（毫秒） | `autoMemoryRetryMs` | `1000` | 首次失败后的等待时间，连续失败时翻倍。 | -| 最大重试间隔（毫秒） | `autoMemoryRetryMaxMs` | `60000` | 重试等待时间的上限。 | +| 界面含义 | 配置键 | 默认值 | 说明 | +| -------------------- | ---------------------- | ----------------------- | -------------------------------------------------------------------------------------------------- | +| 服务地址 | `endpoint` | `http://127.0.0.1:2333` | DSH 访问的绝对 HTTP(S) 地址，不支持 URL 内嵌凭据、查询参数或片段。 | +| 服务令牌（可选） | `authToken` | 空 | 自部署网关的 Bearer 令牌；仅通过 profile 配置（表单中不显示），明文保存，仅建议内网/受信环境使用。 | +| 记忆指引语言 | `language` | `en` | `en` 或 `zh`；决定新会话中注入的指引语言。 | +| 默认搜索数量 | `searchLimit` | `5` | `reme_search` 未传 `limit` 时的默认结果上限，范围 1–50。 | +| 搜索超时（毫秒） | `requestTimeoutMs` | `10000` | `search` 请求的超时，范围 1,000–120,000。 | +| 自动记录已完成的对话 | `autoMemoryEnabled` | `true` | 是否捕获完成的用户/助手回合并提交给 `auto_memory`。 | +| 排除子 Agent | `rootAgentsOnly` | `true` | 开启时，只为根 Agent 注入指引并捕获对话。 | +| 自动记忆提交间隔 | `autoMemoryInterval` | `5` | 每完成多少轮对话提交一次，范围 1–1,000。退出时插件会在时间预算内尽力排空。 | +| 自动整理记忆 | `autoDreamEnabled` | `true` | 是否按照每日计划调用 `auto_dream`。 | +| 整理计划 | `dreamCron` | `0 23 * * *` | 五段 cron 表达式，按 `timezone` 解释；默认每天 23:00。 | +| 整理指引 | `dreamHint` | 空 | 可选的整理提示，原样交给 `auto_dream`。 | +| 工作区时区 | `timezone` | `Asia/Shanghai` | 有效的 IANA 时区，用于每日批次和 cron 计划。 | +| 后台任务超时（毫秒） | `backgroundTimeoutMs` | `3600000` | `auto_memory` 和 `auto_dream` 的最大等待时间。 | +| 退出等待（毫秒） | `shutdownTimeoutMs` | `5000` | DSH 退出时等待后台任务排空的时间预算。 | +| 失败重试间隔（毫秒） | `autoMemoryRetryMs` | `1000` | 首次失败后的等待时间，连续失败时翻倍。 | +| 最大重试间隔（毫秒） | `autoMemoryRetryMaxMs` | `60000` | 重试等待时间的上限。 | 部署层还支持 `REME_URL`，或组合使用 `REME_HOST` 与 `REME_PORT`。仅供定时器测试的 `dreamIntervalMs` 不出现在用户设置中。 diff --git a/integrations/dsh/docs/environment.md b/integrations/dsh/docs/environment.md index 56839e0a..08ad8eab 100644 --- a/integrations/dsh/docs/environment.md +++ b/integrations/dsh/docs/environment.md @@ -28,10 +28,11 @@ time and the two refresh actions. ## Python discovery ReMe needs **Python 3.11 or newer**. The tab probes these channels on the Host and keeps only interpreters that -answered `--version`: +answered its verification snippet (a `-c` probe reporting `sys.executable`, the version and the base prefix): - Windows: PEP 514 registry entries (`HKLM`/`HKCU`), the `py` launcher list, `PATH`, and `conda env list`. -- macOS and Linux: `PATH`, `/usr/bin`, Homebrew and `pyenv` locations, and `conda env list`. +- macOS and Linux: `PATH` (`which -a`), the well-known `/usr/bin`, `/usr/local`, Homebrew (`/opt/homebrew`), + `~/.pyenv/shims` and `~/.local/bin` locations, and `conda env list`. Microsoft Store placeholder links under `WindowsApps` are skipped, because they are shell aliases rather than interpreters. When conda is present, named environments are listed both under `conda env list` and as @@ -91,6 +92,8 @@ DSH settings. component unconditionally, so `EMBEDDING_API_KEY` must be non-empty even without vector search (`EMPTY` is the documented placeholder). A duplicated key is rewritten in every position, and the file keeps its own line ending. +- The write is atomic and owner-only (0600 on POSIX); the previous file is kept as `.env.bak`, so a failed save + cannot lose unrelated keys. - **Test LLM connection** runs a one-token `chat/completions` request from inside the workspace with the selected interpreter, and adds a hint for `401`/`403` (key), `404` (model or base URL) and network errors. - Drafts are bound to the workspace they were typed in: switching the workspace drops them, so a key typed for @@ -115,10 +118,13 @@ processes it manages when the Host exits. The plugin owns a small service adapte **after** the OS acknowledged the spawn, so a missing interpreter leaves no state behind. - A service is reported as managed — and offered a **Stop** button — only when the record matches the endpoint on screen, the pid is still alive, and its image is still a Python or `reme` executable. A record for port 2333 - never claims a service on port 2444, and a recycled pid is not mistaken for this service. -- **Stop** tree-kills the recorded process (on Windows `taskkill /T /F`) and clears the record. A service started - elsewhere stays untouched and is only observed. -- **Start** is serialized: two requests cannot spawn two services, and the second observes the first launch. + never claims a service on port 2444, and a recycled pid is not mistaken for this service. When the image cannot + be read, ownership fails closed: the process is shown as foreign rather than offered a Stop button. +- **Stop** tree-kills the recorded process (process group on POSIX, `taskkill /T /F` on Windows), waits for it to + exit, escalates to `SIGKILL` if it ignores the graceful signal, and clears the record only after the pid is + gone. A service started elsewhere stays untouched and is only observed. +- **Start** is serialized: two requests cannot spawn two services, and a request that arrives while a launch is + still in flight is told so instead of silently pretending to start. - **Auto-start** (`autoStart`, off by default) pulls the service up when DSH starts, if it is not already running. Python probes and pip installs are bounded Host operations; the service is the only intentionally detached @@ -131,7 +137,7 @@ process. | DSH | `0.1.7-rc.2`, `0.2.0-rc.2`, `0.2.1-alpha.2`, `0.2.1-alpha.4`, `0.2.1-rc.2`, `0.2.2-alpha.2` | | Node.js | `^22.19.0` or `>=24.0.0`; current DSH releases need Node 24.9 or newer, and the compatibility matrix is validated with Node 24.16 | | Windows | Tested on Windows 11 with Miniconda: registry/PATH/conda discovery, `tasklist`/`taskkill` for identity and stop, `PYTHONUTF8`/`PYTHONIOENCODING` for pip output | -| macOS / Linux | Discovery, detached start and `SIGTERM` stop are covered by unit tests; documented but less exercised on real machines than Windows | +| macOS / Linux | Discovery, detached spawn, identity lookup and stop are covered by unit tests that run a real child process; still less exercised on real machines than Windows | ## Troubleshooting @@ -152,7 +158,7 @@ the file is left unchanged. **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. -**The service does not become healthy.** The Start action polls for 90 seconds and then reports a timeout. Use +**The service does not become healthy.** The Start action polls for 120 seconds and then reports a timeout. Use **View log** (`$DSH_HOME`-independent: the workspace `service.log`) for the startup trace — a missing `reme-ai`, an invalid `.env`, or a port conflict all appear there. diff --git a/integrations/dsh/docs/environment_ZH.md b/integrations/dsh/docs/environment_ZH.md index 31838266..2e9a2945 100644 --- a/integrations/dsh/docs/environment_ZH.md +++ b/integrations/dsh/docs/environment_ZH.md @@ -24,10 +24,12 @@ ## Python 探测 -ReMe 需要 **Python 3.11 或更新版本**。标签页在 Host 上探测下列来源，只保留能回应 `--version` 的解释器： +ReMe 需要 **Python 3.11 或更新版本**。标签页在 Host 上探测下列来源，只保留能通过验证片段（上报 `sys.executable`、 +版本号与 base prefix 的 `-c` 探测）的解释器： - Windows：PEP 514 注册表项（`HKLM`/`HKCU`）、`py` 启动器列表、`PATH`、`conda env list`。 -- macOS / Linux：`PATH`、`/usr/bin`、Homebrew 与 `pyenv` 常见路径、`conda env list`。 +- macOS / Linux：`PATH`（`which -a`）、常见的 `/usr/bin`、`/usr/local`、Homebrew（`/opt/homebrew`）、 + `~/.pyenv/shims` 与 `~/.local/bin` 路径，以及 `conda env list`。 `WindowsApps` 下的 Microsoft Store 占位符会被跳过，因为它们只是别名而不是解释器。装有 conda 时，命名环境会同时出现在 `conda env list` 与环境列表中，conda 根目录单独上报。 @@ -75,6 +77,7 @@ ReMe 在服务启动时读取 workspace 目录下的 `.env`；标签页从不把 - 清空非密钥字段并保存即表示**删除**该值。只有编辑过的字段会被写入，文件其余部分的注释、未知键与顺序保持不变。 - 保存时会校验合并后的整份文件，而不只是本次输入：ReMe 0.4.0.x 会无条件实例化 embedding 组件，因此即使不使用向量检索 `EMBEDDING_API_KEY` 也必须非空（`EMPTY` 是文档化的占位值）。重复出现的键会在每一处被改写，文件原有的换行风格也会保留。 +- 写入是原子且仅属主可读写的（POSIX 上为 0600）；旧文件会保留为 `.env.bak`，因此一次失败的保存不会丢掉其它键。 - **测试 LLM 连接**在 workspace 内、用所选解释器发送一次只需 1 个 token 的 `chat/completions` 请求，并对 `401`/`403` （密钥）、`404`（模型名或 Base URL）与网络错误给出提示。 - 草稿与输入它的 workspace 绑定：切换 workspace 会丢弃草稿，因此为某个 workspace 输入的密钥不会被保存到另一个。 @@ -95,9 +98,11 @@ ReMe 在服务启动时读取 workspace 目录下的 `.env`；标签页从不把 - 启动在操作系统确认 spawn 之后才记录到 `$DSH_HOME/reme-memory/service.json`（pid、地址、workspace、命令行、启动时间）， 所以解释器缺失时不会留下残留状态。 - 只有记录与当前地址一致、pid 仍存活、且进程映像仍是 Python 或 `reme` 时，服务才被判定为“由本插件托管”并给出**停止**按钮。 - 2333 端口上的记录绝不会声称管理 2444 端口上的服务，被复用的 pid 也不会被误认。 -- **停止**会树杀记录的进程（Windows 上为 `taskkill /T /F`）并清除记录。别处启动的服务不受影响，只会被观察。 -- **启动**是串行化的：两个请求不会拉起两个服务，第二个会看到第一个的启动结果。 + 2333 端口上的记录绝不会声称管理 2444 端口上的服务，被复用的 pid 也不会被误认。读不到进程映像时按失败关闭处理： + 显示为外部进程，不给出停止按钮。 +- **停止**会树杀记录的进程（POSIX 上按进程组、Windows 上为 `taskkill /T /F`），等待其退出，忽略优雅信号时升级为 + `SIGKILL`，并**在 pid 真正消失之后**才清除记录。别处启动的服务不受影响，只会被观察。 +- **启动**是串行化的：两个请求不会拉起两个服务；启动仍在进行时到达的请求会被告知，而不是假装正在启动。 - **自动启动**（`autoStart`，默认关闭）会在 DSH 启动时把尚未运行的服务拉起。 Python 探测与 pip 安装都是有界的 Host 操作；服务是唯一被刻意分离出去的进程。 @@ -109,7 +114,7 @@ Python 探测与 pip 安装都是有界的 Host 操作；服务是唯一被刻 | DSH | `0.1.7-rc.2`、`0.2.0-rc.2`、`0.2.1-alpha.2`、`0.2.1-alpha.4`、`0.2.1-rc.2`、`0.2.2-alpha.2` | | Node.js | `^22.19.0` 或 `>=24.0.0`；当前 DSH 需要 Node 24.9 以上，兼容矩阵以 Node 24.16 验证 | | Windows | 在 Windows 11 + Miniconda 上实测：注册表/PATH/conda 探测、`tasklist`/`taskkill` 身份校验与停止、`PYTHONUTF8`/`PYTHONIOENCODING` 保证 pip 输出不乱码 | -| macOS/Linux | 探测、分离启动与 `SIGTERM` 停止均有单元测试覆盖；真机验证程度低于 Windows | +| macOS/Linux | 探测、分离启动、身份查询与停止均有运行真实子进程的单元测试覆盖；真机验证程度仍低于 Windows | ## 常见问题 @@ -126,7 +131,7 @@ Python 探测与 pip 安装都是有界的 Host 操作；服务是唯一被刻 **服务显示“端口被占用”。** 该端口被其它程序占用。请在服务地址（记忆设置）中换一个端口，或停止占用它的程序。 -**服务迟迟未就绪。** 启动会轮询 90 秒后报超时。用**查看日志**（workspace 下的 `service.log`）看启动过程——缺少 `reme-ai`、 +**服务迟迟未就绪。** 启动会轮询 120 秒后报超时。用**查看日志**（workspace 下的 `service.log`）看启动过程——缺少 `reme-ai`、 `.env` 不合法、端口冲突都会体现在那里。 **服务显示为外部启动。** 它不是由本插件启动的，因此只被观察而不被管理。若希望在标签页里管理，先在它自己的终端里停止，再从 diff --git a/integrations/dsh/src/client/index.tsx b/integrations/dsh/src/client/index.tsx index 5a1e83b9..5c74ae5d 100644 --- a/integrations/dsh/src/client/index.tsx +++ b/integrations/dsh/src/client/index.tsx @@ -357,6 +357,8 @@ function ReMeSettingsCard({ - {tab === "environment" ? ( - rpc && envT ? ( - - ) : ( +
+ {tab === "environment" ? ( + rpc && envT ? ( + + ) : ( +

{t("loading")}

+ ) + ) : value === undefined ? (

{t("loading")}

- ) - ) : value === undefined ? ( -

{t("loading")}

- ) : ( - <> - {shell.dirty ? ( -

{t("unsaved")}

- ) : null} -
- -
- {!shell.writable ? ( -

- {t("readOnly")} -

- ) : null} - {blocked ? ( -

- {t("invalid")} -

- ) : null} -
- - - {shell.failed ? ( - {t("saveFailed")} - ) : null} - {saved && !shell.dirty ? ( - {t("saved")} - ) : null} - - - -
- - )} + + + {!shell.writable ? ( +

+ {t("readOnly")} +

+ ) : null} + {blocked ? ( +

+ {t("invalid")} +

+ ) : null} +
+ + + {shell.failed ? ( + + {t("saveFailed")} + + ) : null} + {saved && !shell.dirty ? ( + {t("saved")} + ) : null} + + + +
+ + )} +
); diff --git a/integrations/dsh/src/client/settings-specs.ts b/integrations/dsh/src/client/settings-specs.ts index c66a3672..3de32349 100644 --- a/integrations/dsh/src/client/settings-specs.ts +++ b/integrations/dsh/src/client/settings-specs.ts @@ -1,6 +1,10 @@ import type { SettingsFieldSpec } from "@deepseek-ai/dsh-client-ui-primitives"; import { validEndpoint } from "../endpoint.js"; +import { validTimezone } from "../scheduling.js"; + +// Re-exported for callers that imported it from this module before it moved. +export { validTimezone }; /** * How each memory-settings field converts between its stored value and the text @@ -84,16 +88,6 @@ export function timeFromCron(cron: string): string { ).padStart(2, "0")}`; } -/** A timezone the browser's `Intl` accepts, which is what DSH validates with. */ -export function validTimezone(value: string): boolean { - try { - new Intl.DateTimeFormat("en", { timeZone: value.trim() }).format(0); - return value.trim().length > 0; - } catch { - return false; - } -} - export const SETTINGS_SPECS: SettingsFieldSpec[] = [ { // The endpoint loses its trailing slashes on the way in, the way the @@ -133,6 +127,8 @@ export const SETTINGS_SPECS: SettingsFieldSpec[] = [ field: "timezone", format: (value) => (typeof value === "string" ? value : ""), parse: (text) => - validTimezone(text) ? { kind: "set", value: text.trim() } : undefined, + validTimezone(text.trim()) + ? { kind: "set", value: text.trim() } + : undefined, }, ]; diff --git a/integrations/dsh/src/client/status-page.tsx b/integrations/dsh/src/client/status-page.tsx index d60e379e..062efea2 100644 --- a/integrations/dsh/src/client/status-page.tsx +++ b/integrations/dsh/src/client/status-page.tsx @@ -1186,6 +1186,9 @@ function diagnosticClient(settings: ReMeSettings): ReMeClient { endpoint: settings.endpoint, requestTimeoutMs: settings.requestTimeoutMs, backgroundTimeoutMs: settings.backgroundTimeoutMs, + // The memory runtime authenticates, so diagnostics must too; otherwise a + // gateway-protected service shows 401s that look like a broken workspace. + authToken: settings.authToken, }); } diff --git a/integrations/dsh/src/environment/client/dependency.tsx b/integrations/dsh/src/environment/client/dependency.tsx index 25c15295..394c621c 100644 --- a/integrations/dsh/src/environment/client/dependency.tsx +++ b/integrations/dsh/src/environment/client/dependency.tsx @@ -31,7 +31,7 @@ export interface DependencyState { /** Wall clock tick while an installation runs, for the elapsed label. */ now: number; recheck(): void; - start(upgrade: boolean): Promise; + start(upgrade: boolean, indexUrl?: string): Promise; cancel(): Promise; dismiss(): void; } @@ -42,9 +42,14 @@ export interface DependencyState { */ export function useDependency( rpc: StatusRpc, - options: { pythonPath: string; onInstalled(): void }, + options: { + pythonPath: string; + /** False for a remote endpoint, whose local steps are hidden. */ + enabled?: boolean; + onInstalled(): void; + }, ): DependencyState { - const { pythonPath, onInstalled } = options; + const { pythonPath, enabled = true, onInstalled } = options; const [check, setCheck] = useState({ phase: "idle", forPath: "", @@ -56,7 +61,7 @@ export function useDependency( installed.current = onInstalled; useEffect(() => { - if (!pythonPath) return; + if (!enabled || !pythonPath) return; const controller = new AbortController(); void (async () => { try { @@ -66,6 +71,7 @@ export function useDependency( { pythonPath }, controller.signal, ); + if (controller.signal.aborted) return; setCheck({ phase: "ready", data: parseRemeCheck(value), @@ -82,18 +88,28 @@ export function useDependency( } })(); return () => controller.abort(); - }, [rpc, pythonPath, tick]); + }, [rpc, pythonPath, tick, enabled]); + + // Progress belongs to an interpreter: switching interpreters must not keep + // showing (or cancelling) a job that was started for the previous one. + const installFor = useRef(pythonPath); + useEffect(() => { + if (installFor.current === pythonPath) return; + installFor.current = pythonPath; + setInstall({ phase: "idle" }); + }, [pythonPath]); // Restoring a running job is per interpreter: a job started for another // interpreter belongs to that interpreter's section, not this one. useEffect(() => { - if (!pythonPath) return; + if (!enabled || !pythonPath) return; const controller = new AbortController(); void (async () => { try { const status = parseInstallStatus( await envCall(rpc, "installStatus", {}, controller.signal), ); + if (controller.signal.aborted) return; if (status?.phase !== "running" || status.pythonPath !== pythonPath) return; setInstall({ phase: "running", jobId: status.id, job: status }); @@ -102,7 +118,7 @@ export function useDependency( } })(); return () => controller.abort(); - }, [rpc, pythonPath]); + }, [rpc, pythonPath, enabled]); const jobId = install.jobId; useEffect(() => { @@ -161,12 +177,16 @@ export function useDependency( }, []); const start = useCallback( - async (upgrade: boolean) => { + async (upgrade: boolean, indexUrl?: string) => { if (!pythonPath || install.phase === "running") return; setInstall({ phase: "idle" }); try { const status = parseInstallStatus( - await envCall(rpc, "install", { pythonPath, upgrade }), + await envCall(rpc, "install", { + pythonPath, + upgrade, + ...(indexUrl ? { indexUrl } : {}), + }), ); setInstall( status && status.phase === "running" @@ -235,6 +255,8 @@ export function DependencySection({ ? { tone: "progress" as const, text: t("depMissing") } : checkData.versionOk && checkData.hasCore ? { tone: "ok" as const, text: t("depReady") } + : checkData.versionOk + ? { tone: "progress" as const, text: t("coreMissing") } : { tone: "progress" as const, text: t("depUpgrade") }; return ( @@ -279,6 +301,9 @@ export function DependencySection({ {checkError ? `: ${checkError}` : ""}

) : null} + {checkData?.error ? ( +

{checkData.error}

+ ) : null} {checkData ? (
@@ -361,7 +386,12 @@ export function DependencySection({ className="reme-settings-save" onClick={() => { setConfirming(false); - void dependency.start(!checkData.versionOk); + // The mirror choice reaches pip as `--index-url`; without + // it the selector would be decorative. + void dependency.start( + !checkData.versionOk, + mirror || undefined, + ); }} > {t("confirmInstall")} diff --git a/integrations/dsh/src/environment/client/locales.ts b/integrations/dsh/src/environment/client/locales.ts index 1c777f10..f394759e 100644 --- a/integrations/dsh/src/environment/client/locales.ts +++ b/integrations/dsh/src/environment/client/locales.ts @@ -1,6 +1,5 @@ /** Copy for the Environment tab, registered under `reme.environment`. */ export const envEn = { - connectionTitle: "Service connection", endpointLabel: "Service URL", checking: "Checking…", connected: "Connected", @@ -19,7 +18,6 @@ export const envEn = { "Install Miniconda or any Python 3.11+, then refresh. Existing conda environments are detected automatically.", miniconda: "Miniconda download", recommended: "recommended", - okBadge: "OK", tooOld: "Needs 3.11+", originConda: "Conda ({name})", originPythonOrg: "python.org installer", @@ -103,7 +101,7 @@ export const envEn = { serviceRunning: "Running", startService: "Start service", stopService: "Stop service", - startTimeout: "The service did not become healthy within 90 seconds", + startTimeout: "The service did not become healthy within 120 seconds", stopConfirm: "Stop the ReMe service? Automatic memory in flight may need a retry.", foreignNote: @@ -115,6 +113,8 @@ export const envEn = { autoStartLabel: "Start the service automatically with DSH", managedNote: "Managed by this plugin · PID {pid}", serviceError: "Service operation failed", + settingsWriteFailed: + "The setting was not saved. The form may have changed in another window; retry.", configTitle: "ReMe parameters", controlTitle: "Service control", heroEyebrow: "ReMe local runtime", @@ -131,8 +131,6 @@ export const envEn = { scannedAt: "Detected at {time}", currentInterpreter: "Current interpreter", notInDetection: "not detected", - autoSelected: - "The recommended interpreter is used until you pick another one.", savingSelection: "Saving…", changeInterpreter: "Change interpreter", collapseList: "Collapse list", @@ -153,7 +151,6 @@ export const envEn = { }; export const envZh: typeof envEn = { - connectionTitle: "服务连接", endpointLabel: "服务地址", checking: "正在检查…", connected: "已连接", @@ -171,7 +168,6 @@ export const envZh: typeof envEn = { "可安装 Miniconda 或任意 Python 3.11+ 后点击“重新探测”；已有的 conda 环境会被自动识别。", miniconda: "下载 Miniconda", recommended: "推荐", - okBadge: "可用", tooOld: "需要 3.11+", originConda: "Conda（{name}）", originPythonOrg: "python.org 安装器", @@ -252,7 +248,7 @@ export const envZh: typeof envEn = { serviceRunning: "运行中", startService: "启动服务", stopService: "停止服务", - startTimeout: "服务在 90 秒内未就绪", + startTimeout: "服务在 120 秒内未就绪", stopConfirm: "停止 ReMe 服务？进行中的自动记忆可能需要重试。", foreignNote: "该服务由 DSH 之外的方式启动，这里只显示状态、不由本插件管理。", conflictNote: "端口被其他程序占用；请在『记忆设置』中修改服务地址的端口。", @@ -261,6 +257,7 @@ export const envZh: typeof envEn = { autoStartLabel: "随 DSH 启动时自动拉起服务", managedNote: "由本插件托管 · PID {pid}", serviceError: "服务操作失败", + settingsWriteFailed: "设置未保存；表单可能已在其他窗口更新，请重试。", configTitle: "ReMe 参数", controlTitle: "服务控制", heroEyebrow: "ReMe 本地运行时", @@ -277,7 +274,6 @@ export const envZh: typeof envEn = { scannedAt: "探测于 {time}", currentInterpreter: "当前解释器", notInDetection: "不在探测结果中", - autoSelected: "当前使用推荐解释器；选择其他解释器后会写入配置。", savingSelection: "正在保存…", changeInterpreter: "更换解释器", collapseList: "收起列表", diff --git a/integrations/dsh/src/environment/client/python.tsx b/integrations/dsh/src/environment/client/python.tsx index 540c4319..afbb4fac 100644 --- a/integrations/dsh/src/environment/client/python.tsx +++ b/integrations/dsh/src/environment/client/python.tsx @@ -40,10 +40,12 @@ export function usePython( options: { pythonPath: string; writable: boolean; + /** False for a remote endpoint, whose local steps are hidden. */ + enabled?: boolean; onSelect(exe: string): Promise; }, ): PythonSelection { - const { pythonPath, writable, onSelect } = options; + const { pythonPath, writable, enabled = true, onSelect } = options; const [detection, setDetection] = useState({ phase: "loading", }); @@ -51,6 +53,7 @@ export function usePython( const [saving, setSaving] = useState(""); useEffect(() => { + if (!enabled) return; const controller = new AbortController(); void (async () => { try { @@ -60,6 +63,7 @@ export function usePython( { refresh: tick > 0 }, controller.signal, ); + if (controller.signal.aborted) return; setDetection({ phase: "ready", data: parseDetection(value) }); } catch (error) { if (!controller.signal.aborted) @@ -67,7 +71,7 @@ export function usePython( } })(); return () => controller.abort(); - }, [rpc, tick]); + }, [rpc, tick, enabled]); const detected = detection.phase === "ready" ? detection.data : undefined; const suggested = detected?.suggested ?? ""; @@ -215,11 +219,6 @@ export function PythonSection({
) : null} - {!selectedPath && suggested ? ( -

- {t("autoSelected")} -

- ) : null} {others.length > 0 ? (
diff --git a/integrations/dsh/src/environment/client/rpc.ts b/integrations/dsh/src/environment/client/rpc.ts index c28821e2..a175137b 100644 --- a/integrations/dsh/src/environment/client/rpc.ts +++ b/integrations/dsh/src/environment/client/rpc.ts @@ -48,66 +48,168 @@ export async function envCall( } function record(value: unknown, what: string): Record { - if (typeof value !== "object" || value === null) + if (typeof value !== "object" || value === null || Array.isArray(value)) throw new Error(`Invalid ${what} response`); return value as Record; } -/** Guard a detection payload: the interpreter list is what the tab needs. */ +function fail(what: string): never { + throw new Error(`Invalid ${what} response`); +} + +function stringField( + body: Record, + key: string, + what: string, +): string { + const value = body[key]; + if (typeof value !== "string") fail(what); + return value; +} + +function booleanField( + body: Record, + key: string, + what: string, +): boolean { + const value = body[key]; + if (typeof value !== "boolean") fail(what); + return value; +} + +function arrayField( + body: Record, + key: string, + what: string, +): unknown[] { + const value = body[key]; + if (!Array.isArray(value)) fail(what); + return value; +} + +/** + * Guard a detection payload. The renderers dereference `errors`, `origin` and + * the per-interpreter fields, so a Host contract drift must surface as an + * error in the section, not as a blank page. + */ export function parseDetection(value: unknown): PythonDetection { - const body = record(value, "environment detection"); - if (!Array.isArray(body.envs)) - throw new Error("Invalid environment detection response"); + const what = "environment detection"; + const body = record(value, what); + stringField(body, "platform", what); + stringField(body, "scannedAt", what); + stringField(body, "suggested", what); + stringField(body, "condaRoot", what); + arrayField(body, "errors", what); + for (const item of arrayField(body, "envs", what)) { + const env = record(item, what); + stringField(env, "exe", what); + stringField(env, "version", what); + booleanField(env, "ok", what); + record(env.origin, what); + } return value as PythonDetection; } export function parseRemeCheck(value: unknown): RemeCheck { - return record(value, "ReMe check") as unknown as RemeCheck; + const what = "ReMe check"; + const body = record(value, what); + booleanField(body, "installed", what); + booleanField(body, "versionOk", what); + booleanField(body, "hasCore", what); + stringField(body, "version", what); + stringField(body, "cliPath", what); + return value as unknown as RemeCheck; } /** `null` is the valid "no job yet" answer, so it is not an error. */ export function parseInstallStatus(value: unknown): InstallStatus | null { - if (typeof value !== "object" || value === null) return null; - const body = value as Record; - return typeof body.id === "string" && typeof body.phase === "string" - ? (value as unknown as InstallStatus) - : null; + if (value === null) return null; + const what = "install status"; + const body = record(value, what); + stringField(body, "id", what); + stringField(body, "pythonPath", what); + stringField(body, "startedAt", what); + stringField(body, "logPath", what); + const phase = stringField(body, "phase", what); + if (!["running", "succeeded", "failed", "cancelled"].includes(phase)) + fail(what); + return value as unknown as InstallStatus; } export function parseEnvSummary(value: unknown): EnvSummary { - const body = record(value, ".env summary"); - if (!Array.isArray(body.entries)) - throw new Error("Invalid .env summary response"); + const what = ".env summary"; + const body = record(value, what); + stringField(body, "workspaceDir", what); + stringField(body, "envPath", what); + booleanField(body, "exists", what); + for (const item of arrayField(body, "entries", what)) { + const entry = record(item, what); + stringField(entry, "key", what); + booleanField(entry, "secret", what); + booleanField(entry, "set", what); + stringField(entry, "masked", what); + } return value as unknown as EnvSummary; } export function parseLlmResult(value: unknown): LlmTestResult { - return record(value, "LLM test") as unknown as LlmTestResult; + const what = "LLM test"; + const body = record(value, what); + booleanField(body, "ok", what); + if (typeof body.status !== "number" || typeof body.latencyMs !== "number") + fail(what); + return value as unknown as LlmTestResult; } export function parseHealth(value: unknown): ConnectionHealth { - const body = record(value, "health"); - if (typeof body.ok !== "boolean") throw new Error("Invalid health response"); + const what = "health"; + const body = record(value, what); + booleanField(body, "ok", what); + stringField(body, "version", what); + stringField(body, "error", what); + stringField(body, "endpoint", what); return value as unknown as ConnectionHealth; } export function parseServiceState(value: unknown): ServiceState { - const body = record(value, "service status"); - if (typeof body.phase !== "string") - throw new Error("Invalid service status response"); + const what = "service status"; + const body = record(value, what); + const phase = stringField(body, "phase", what); + if (!["running", "stopped", "conflict"].includes(phase)) fail(what); + booleanField(body, "managed", what); + stringField(body, "endpoint", what); + stringField(body, "logPath", what); return value as unknown as ServiceState; } export function parseServiceStart(value: unknown): ServiceStartResult { - const body = record(value, "service start"); - if (typeof body.started !== "boolean") - throw new Error("Invalid service start response"); + const what = "service start"; + const body = record(value, what); + booleanField(body, "started", what); + booleanField(body, "alreadyRunning", what); + stringField(body, "command", what); + stringField(body, "logPath", what); + stringField(body, "startedAt", what); return value as unknown as ServiceStartResult; } +export interface ServiceStopView { + stopped: boolean; + pid?: number; + reason?: string; +} + +export function parseServiceStop(value: unknown): ServiceStopView { + const what = "service stop"; + const body = record(value, what); + booleanField(body, "stopped", what); + return value as ServiceStopView; +} + export function parseServiceLog(value: unknown): ServiceLog { - const body = record(value, "service log"); - if (typeof body.lines !== "string") - throw new Error("Invalid service log response"); + const what = "service log"; + const body = record(value, what); + stringField(body, "lines", what); + stringField(body, "logPath", what); return value as unknown as ServiceLog; } diff --git a/integrations/dsh/src/environment/client/service.tsx b/integrations/dsh/src/environment/client/service.tsx index 2e4c4c9c..a15d88e7 100644 --- a/integrations/dsh/src/environment/client/service.tsx +++ b/integrations/dsh/src/environment/client/service.tsx @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useState } from "react"; +import { useCallback, useEffect, useRef, useState } from "react"; import type { StatusRpc } from "../../client/status-page.js"; import { safeError } from "../../endpoint.js"; import type { ServiceState } from "../types.js"; @@ -11,10 +11,15 @@ import { parseServiceLog, parseServiceStart, parseServiceState, + parseServiceStop, } from "./rpc.js"; -/** How long a detached start may take to answer `/health_check`. */ -const START_DEADLINE_MS = 90_000; +/** + * How long a detached start may take to answer `/health_check`. Kept at the + * Host's start grace period, so the client never gives up while the Host still + * considers the launch in flight. + */ +const START_DEADLINE_MS = 120_000; export interface ServiceViewState { phase: "checking" | "running" | "starting" | "stopped" | "conflict" | "error"; @@ -31,6 +36,8 @@ export interface ServiceLogView { export interface ServiceControl { state: ServiceViewState; stopping: boolean; + /** Why the last Stop request left the service alone, if it did. */ + stopError: string; start(): Promise; stop(): Promise; refresh(): void; @@ -58,12 +65,22 @@ export function useService( const { t, endpoint, local, pythonPath, workspaceDir, authToken } = options; const [state, setState] = useState({ phase: "checking" }); const [stopping, setStopping] = useState(false); + const [stopError, setStopError] = useState(""); const [tick, setTick] = useState(0); const [log, setLog] = useState({ open: false, lines: "", logPath: "", }); + const startController = useRef(undefined); + + // A new endpoint has no known phase yet; keeping the previous endpoint's + // "running + old pid/version" on screen would mislabel the new one. + useEffect(() => { + setState({ phase: "checking" }); + }, [endpoint]); + + useEffect(() => () => startController.current?.abort(), []); useEffect(() => { if (!local || !endpoint) return; @@ -100,17 +117,22 @@ export function useService( const start = useCallback(async () => { if (!pythonPath || state.phase === "starting") return; + startController.current?.abort(); + const controller = new AbortController(); + startController.current = controller; setState({ phase: "starting" }); + setStopError(""); setLog({ open: false, lines: "", logPath: "" }); try { const started = parseServiceStart( - await envCall(rpc, "serviceStart", { - endpoint, - pythonPath, - workspaceDir, - authToken, - }), + await envCall( + rpc, + "serviceStart", + { endpoint, pythonPath, workspaceDir, authToken }, + controller.signal, + ), ); + if (controller.signal.aborted) return; if (started.error) { setState({ phase: "error", error: started.error }); return; @@ -118,13 +140,19 @@ export function useService( const deadline = Date.now() + START_DEADLINE_MS; for (;;) { await new Promise((resolve) => setTimeout(resolve, 1000)); + if (controller.signal.aborted) return; if (Date.now() > deadline) { setState({ phase: "error", error: t("startTimeout") }); return; } try { const status = parseServiceState( - await envCall(rpc, "serviceStatus", { endpoint, authToken }), + await envCall( + rpc, + "serviceStatus", + { endpoint, authToken }, + controller.signal, + ), ); if (status.phase === "running") { setState({ phase: "running", data: status }); @@ -135,6 +163,7 @@ export function useService( } } } catch (error) { + if (controller.signal.aborted) return; setState({ phase: "error", error: safeError(error) }); } }, [authToken, endpoint, pythonPath, rpc, state.phase, t, workspaceDir]); @@ -143,14 +172,17 @@ export function useService( if (stopping) return; setStopping(true); try { - await envCall(rpc, "serviceStop", { endpoint }); + const result = parseServiceStop( + await envCall(rpc, "serviceStop", { endpoint }), + ); + setStopError(result.stopped ? "" : result.reason || t("serviceError")); refresh(); } catch (error) { setState({ phase: "error", error: safeError(error) }); } finally { setStopping(false); } - }, [endpoint, refresh, rpc, stopping]); + }, [endpoint, refresh, rpc, stopping, t]); const viewLog = useCallback(async () => { try { @@ -168,7 +200,17 @@ export function useService( [], ); - return { state, stopping, start, stop, refresh, log, viewLog, dismissLog }; + return { + state, + stopping, + stopError, + start, + stop, + refresh, + log, + viewLog, + dismissLog, + }; } export interface ConnectionState { @@ -245,7 +287,7 @@ export function ServiceSection({ writable: boolean; onAutoStart(checked: boolean): void; }): JSX.Element { - const { state, stopping, log } = control; + const { state, stopping, stopError, log } = control; const [confirming, setConfirming] = useState(false); return ( @@ -311,6 +353,11 @@ export function ServiceSection({ {`${t("serviceError")}: ${state.error}`}

) : null} + {stopError ? ( +

+ {stopError} +

+ ) : null}
{state.phase === "running" ? ( confirming ? ( @@ -368,6 +415,7 @@ export function ServiceSection({ { if (!writable) return; onAutoStart(event.target.checked); diff --git a/integrations/dsh/src/environment/client/tab.tsx b/integrations/dsh/src/environment/client/tab.tsx index 90257cec..b9d570e4 100644 --- a/integrations/dsh/src/environment/client/tab.tsx +++ b/integrations/dsh/src/environment/client/tab.tsx @@ -2,6 +2,7 @@ import { useCallback, useState, useSyncExternalStore } from "react"; import type { ConfigForm } from "@deepseek-ai/dsh-client-ui-settings/client"; import type { StatusRpc } from "../../client/status-page.js"; +import { safeError } from "../../endpoint.js"; import type { RemeCheck } from "../types.js"; import type { ReMeSettings } from "../../types.js"; import { DependencySection, useDependency } from "./dependency.js"; @@ -25,16 +26,38 @@ export interface ReMeEnvironmentTabProps { t: EnvTranslator; } -/** Only a loopback endpoint can be managed as a local service. */ +/** Only a loopback HTTP endpoint can be managed as a local service. */ function isLocalEndpoint(endpoint: string): boolean { try { const url = new URL(endpoint); - return ["127.0.0.1", "localhost", "[::1]", "::1"].includes(url.hostname); + return ( + url.protocol === "http:" && + ["127.0.0.1", "localhost", "[::1]", "::1"].includes(url.hostname) + ); } catch { return false; } } +/** + * Compare endpoints the way the Host does, so a default port or a trailing + * slash cannot make the Host's answer look like it belongs to an older URL. + */ +function endpointKey(endpoint: string): string { + try { + const url = new URL(endpoint); + const port = url.port || (url.protocol === "https:" ? "443" : "80"); + return `${ + url.protocol + }//${url.hostname.toLowerCase()}:${port}${url.pathname.replace( + /\/+$/, + "", + )}`; + } catch { + return endpoint.trim(); + } +} + function connectionBadge( t: EnvTranslator, connection: ConnectionState, @@ -109,24 +132,41 @@ export function ReMeEnvironmentTab({ const workspaceDir = value?.remeWorkspaceDir ?? ""; const authToken = value?.authToken ?? ""; const [heroTick, setHeroTick] = useState(0); + // Settings writes are invisible to the sections otherwise: a rejected + // revision or a transport error would silently roll the control back. + const [settingsError, setSettingsError] = useState(""); const writeSetting = useCallback( - (field: string, next: unknown): Promise => { - if (!snapshot.writable) return Promise.resolve(false); - return scope - .mutate([{ op: "set", path: [field], value: next }], snapshot.revision) - .catch(() => undefined); + async (field: string, next: unknown): Promise => { + if (!snapshot.writable) return false; + try { + const applied = await scope.mutate( + [{ op: "set", path: [field], value: next }], + snapshot.revision, + ); + if (applied === false) { + setSettingsError(t("settingsWriteFailed")); + return false; + } + setSettingsError(""); + return true; + } catch (error) { + setSettingsError(safeError(error)); + return false; + } }, - [scope, snapshot.revision, snapshot.writable], + [scope, snapshot.revision, snapshot.writable, t], ); const python = usePython(rpc, { pythonPath: value?.pythonPath ?? "", writable: snapshot.writable, + enabled: local, onSelect: (exe) => writeSetting("pythonPath", exe), }); const dependency = useDependency(rpc, { pythonPath: python.selectedPath, + enabled: local, onInstalled: () => { python.refresh(); }, @@ -134,6 +174,7 @@ export function ReMeEnvironmentTab({ const workspace = useWorkspace(rpc, { workspaceDir, writable: snapshot.writable, + enabled: local, onWorkspaceDir: (dir) => writeSetting("remeWorkspaceDir", dir), }); const service = useService(rpc, { @@ -154,18 +195,30 @@ export function ReMeEnvironmentTab({ tick: heroTick, authToken, }); + // The Host names the endpoint its answer belongs to; comparing that with the + // settings endpoint is what makes a stale answer fail the `current` test. + const serviceEndpoint = service.state.data?.endpoint ?? endpoint; const connection: ConnectionState = local ? service.state.phase === "running" - ? { phase: "ok", endpoint, version: service.state.data?.version ?? "" } + ? { + phase: "ok", + endpoint: serviceEndpoint, + version: service.state.data?.version ?? "", + } : service.state.phase === "checking" || service.state.phase === "starting" - ? { phase: "loading", endpoint } - : { phase: "fail", endpoint, error: service.state.error } + ? { phase: "loading", endpoint: serviceEndpoint } + : { + phase: "fail", + endpoint: serviceEndpoint, + error: service.state.error, + } : remote; if (snapshot.status === "unavailable") return null; const current = - connection.phase !== "loading" && connection.endpoint === endpoint; + connection.phase !== "loading" && + endpointKey(connection.endpoint) === endpointKey(endpoint); const ready = current && connection.phase === "ok" && @@ -241,6 +294,11 @@ export function ReMeEnvironmentTab({ {connection.error}

) : null} + {settingsError ? ( +

+ {settingsError} +

+ ) : null} {!local ? (

{t("remoteNotice")} diff --git a/integrations/dsh/src/environment/client/workspace.tsx b/integrations/dsh/src/environment/client/workspace.tsx index 9e423ee1..751d76d5 100644 --- a/integrations/dsh/src/environment/client/workspace.tsx +++ b/integrations/dsh/src/environment/client/workspace.tsx @@ -144,10 +144,12 @@ export function useWorkspace( options: { workspaceDir: string; writable: boolean; + /** False for a remote endpoint, whose local steps are hidden. */ + enabled?: boolean; onWorkspaceDir(dir: string): Promise; }, ): WorkspaceState { - const { workspaceDir, writable, onWorkspaceDir } = options; + const { workspaceDir, writable, enabled = true, onWorkspaceDir } = options; const [env, setEnv] = useState({ phase: "loading" }); const [draft, setDraft] = useState({ dir: "", values: {} }); const [saving, setSaving] = useState(false); @@ -158,12 +160,18 @@ export function useWorkspace( // Read by a save that resolves after a workspace switch, so the answer for // the previous workspace is dropped instead of shown against the new one. const latestDir = useRef(workspaceDir); + const llmController = useRef(undefined); useEffect(() => { latestDir.current = workspaceDir; + // A probe for the previous workspace must not land on the new one. + llmController.current?.abort(); }, [workspaceDir]); + useEffect(() => () => llmController.current?.abort(), []); + useEffect(() => { + if (!enabled) return; const controller = new AbortController(); void (async () => { try { @@ -173,6 +181,7 @@ export function useWorkspace( { workspaceDir }, controller.signal, ); + if (controller.signal.aborted) return; setEnv({ phase: "ready", data: parseEnvSummary(value), @@ -188,7 +197,7 @@ export function useWorkspace( } })(); return () => controller.abort(); - }, [rpc, workspaceDir, tick]); + }, [rpc, workspaceDir, tick, enabled]); // A workspace switch drops drafts typed for the previous workspace, so a key // typed for one workspace can never be written into another. The read is @@ -262,13 +271,24 @@ export function useWorkspace( const testLlm = useCallback( async (pythonPath: string) => { if (!pythonPath || llmTest.phase === "running") return; + const dir = workspaceDir; + llmController.current?.abort(); + const controller = new AbortController(); + llmController.current = controller; setLlmTest({ phase: "running" }); try { const data = parseLlmResult( - await envCall(rpc, "testLlm", { workspaceDir, pythonPath }), + await envCall( + rpc, + "testLlm", + { workspaceDir: dir, pythonPath }, + controller.signal, + ), ); + if (controller.signal.aborted || latestDir.current !== dir) return; setLlmTest({ phase: data.ok ? "ok" : "fail", result: data }); } catch (error) { + if (controller.signal.aborted || latestDir.current !== dir) return; setLlmTest({ phase: "fail", result: { @@ -287,7 +307,10 @@ export function useWorkspace( if (!writable) return; const next = workspaceInput ?? workspaceDir; setWorkspaceInput(undefined); - await onWorkspaceDir(next.trim()).catch(() => undefined); + const saved = await onWorkspaceDir(next.trim()).catch(() => false); + // A rejected save is reported by the settings writer; do not reload the + // old directory as if the new one had been accepted. + if (saved === false) return; setTick((current) => current + 1); }, [onWorkspaceDir, workspaceDir, workspaceInput, writable]); diff --git a/integrations/dsh/src/environment/host/detect.ts b/integrations/dsh/src/environment/host/detect.ts index dadd66bc..a2a7ed3a 100644 --- a/integrations/dsh/src/environment/host/detect.ts +++ b/integrations/dsh/src/environment/host/detect.ts @@ -225,6 +225,24 @@ export function makeCondaRunner( }; } +/** + * Unix interpreters that a `which -a python3` can miss when the shell has a + * trimmed PATH: the system install, Homebrew on both Apple Silicon and Intel, + * and pyenv/local shims. Windows has `where` and the registry for this. + */ +function commonPythonPaths(io: DetectIO, windows: boolean): string[] { + if (windows) return []; + const path = pathOf(io.homedir); + return [ + "/usr/bin/python3", + "/usr/bin/python", + "/usr/local/bin/python3", + "/opt/homebrew/bin/python3", + path.join(io.homedir, ".pyenv", "shims", "python3"), + path.join(io.homedir, ".local", "bin", "python3"), + ]; +} + function commonCondaPaths(io: DetectIO, windows: boolean): string[] { const programData = io.env["ProgramData"] ?? ""; const path = windows ? win32 : pathOf(io.homedir); @@ -340,7 +358,11 @@ function classifyOrigin( }; if ([...candidate.sources].some((source) => source.startsWith("registry"))) return { kind: "python.org" }; - if (candidate.sources.has("PATH") || candidate.sources.has("which")) + if ( + candidate.sources.has("PATH") || + candidate.sources.has("which") || + candidate.sources.has("well-known") + ) return { kind: "system" }; return { kind: "unknown" }; } @@ -472,6 +494,10 @@ export async function detectPythonEnvs(io: DetectIO): Promise { if (isWindowsAppsAlias(exe)) continue; addCandidate(candidates, exe, windows ? "PATH" : "which", windows); } + // Well-known Unix install locations, independent of the Host PATH. + for (const exe of commonPythonPaths(io, windows)) + if (io.exists(exe)) + addCandidate(candidates, exe, "well-known", windows); })(), // conda: PATH first, then well-known install roots. (async () => { diff --git a/integrations/dsh/src/environment/host/install.ts b/integrations/dsh/src/environment/host/install.ts index f274c90a..662b0508 100644 --- a/integrations/dsh/src/environment/host/install.ts +++ b/integrations/dsh/src/environment/host/install.ts @@ -1,14 +1,15 @@ import { spawn } from "node:child_process"; -import { closeSync, mkdirSync, openSync, readFileSync } from "node:fs"; +import { closeSync, fstatSync, mkdirSync, openSync, readSync } from "node:fs"; import { join } from "node:path"; import type { InstallPhase, InstallRequest, InstallStatus } from "../types.js"; -import { pathOf } from "./paths.js"; +import { isAbsolutePath } from "./paths.js"; export interface InstallChild { pid: number | undefined; onExit(callback: (code: number | null) => void): void; - kill(): void; + /** `force` escalates to SIGKILL on POSIX; Windows always force-kills. */ + kill(force?: boolean): void; } export interface InstallSpawner { @@ -33,18 +34,32 @@ export function pipArgs(request: InstallRequest): string[] { if (indexUrl) { if (!/^https?:\/\/\S+$/i.test(indexUrl)) throw new TypeError("Package index must be an http(s) URL"); + // Credentials embedded in the URL would be echoed in the job command, the + // install log and the browser; pip's own config is the safe place for them. + const parsed = new URL(indexUrl); + if (parsed.username || parsed.password) + throw new TypeError( + "Package index must not embed credentials; configure them in pip.conf", + ); args.push("--index-url", indexUrl); } args.push("reme-ai[core]"); return args; } -/** Last `maxLines` non-empty lines of a log file, normalized to "\n". */ +/** + * Last `maxLines` non-empty lines of a log file, normalized to "\n". Only the + * tail is read: this runs on every 2 s poll and the log can grow for minutes. + */ export function tailLog(path: string, maxLines = 30, maxBytes = 8192): string { + let fd: number | undefined; try { - const buffer = readFileSync(path); + fd = openSync(path, "r"); + const size = fstatSync(fd).size; + const length = Math.min(size, maxBytes); + const buffer = Buffer.alloc(length); + readSync(fd, buffer, 0, length, Math.max(0, size - length)); const text = buffer - .subarray(Math.max(0, buffer.length - maxBytes)) .toString("utf8") .replaceAll("\r\n", "\n") .replaceAll("\r", "\n"); @@ -55,6 +70,14 @@ export function tailLog(path: string, maxLines = 30, maxBytes = 8192): string { .join("\n"); } catch { return ""; + } finally { + if (fd !== undefined) { + try { + closeSync(fd); + } catch { + // Already closed. + } + } } } @@ -70,6 +93,9 @@ export function createInstallSpawner( child = spawn(exe, args, { shell: false, windowsHide: true, + // POSIX: lead a process group so a cancel can stop pip's build + // backends too, not only pip itself. + detached: process.platform !== "win32", env: { ...base, PYTHONIOENCODING: "utf-8", @@ -94,17 +120,30 @@ export function createInstallSpawner( child.on("close", finish); child.on("error", () => finish(null)); }, - kill() { + kill(force = false) { if (child.pid !== undefined && process.platform === "win32") { // Tree-kill: pip's build backends are child processes. - spawn("taskkill", ["/PID", String(child.pid), "/T", "/F"], { - windowsHide: true, - stdio: "ignore", - }); + const killer = spawn( + "taskkill", + ["/PID", String(child.pid), "/T", "/F"], + { windowsHide: true, stdio: "ignore" }, + ); + // Without this listener an async spawn failure would exit the Host. + killer.on("error", () => undefined); + killer.unref(); return; } + const signal = force ? "SIGKILL" : "SIGTERM"; + // The install process leads its own group on POSIX, so pip's build + // backends receive the signal as well. try { - child.kill("SIGTERM"); + if (child.pid !== undefined) process.kill(-child.pid, signal); + return; + } catch { + // Not a group leader (e.g. spawn failed); signal the process only. + } + try { + child.kill(signal); } catch { // Already gone. } @@ -148,7 +187,7 @@ export class InstallJobs { const pythonPath = (request.pythonPath ?? "").trim(); // The interpreter may be written for another platform than the host, so its // own flavor decides whether the path is absolute. - if (!pythonPath || !pathOf(pythonPath).isAbsolute(pythonPath)) + if (!pythonPath || !isAbsolutePath(pythonPath)) throw new TypeError( "Install requires an absolute Python interpreter path", ); @@ -199,8 +238,13 @@ export class InstallJobs { if (!job || job.phase !== "running" || job.cancelling) return false; job.cancelling = true; job.child.kill(); - // A process that ignores the kill must not lock the interpreter forever. - job.cancelTimer = setTimeout(() => this.finish(job, null), 15_000); + // A process that ignores the graceful kill must not keep the interpreter + // locked, so escalate before the last-resort fallback settles the job. + job.cancelTimer = setTimeout(() => { + job.child.kill(true); + job.cancelTimer = setTimeout(() => this.finish(job, null), 15_000); + job.cancelTimer.unref(); + }, 5_000); job.cancelTimer.unref(); return true; } diff --git a/integrations/dsh/src/environment/host/manager.ts b/integrations/dsh/src/environment/host/manager.ts index 3cc00823..b7b71400 100644 --- a/integrations/dsh/src/environment/host/manager.ts +++ b/integrations/dsh/src/environment/host/manager.ts @@ -2,9 +2,10 @@ import { existsSync, readFileSync } from "node:fs"; import { homedir } from "node:os"; import { join } from "node:path"; +import { safeError } from "../../endpoint.js"; import { detectPythonEnvs } from "./detect.js"; import { createInstallSpawner, InstallJobs, tailLog } from "./install.js"; -import { checkReme, remeVersionAtLeast } from "./reme.js"; +import { checkReme, MINIMUM_REME, remeVersionAtLeast } from "./reme.js"; import { createNodeRunner } from "./runner.js"; import { createServiceIO, @@ -189,8 +190,16 @@ export class ReMeEnvManager { "service.log", ), }); - if (status.phase === "running" && this.starting?.endpoint === endpointValue) - this.starting = undefined; + if (this.starting?.endpoint === endpointValue) { + // Running is the happy end; stopped without a live pid means the launch + // died before it ever listened, so a retry must not be blocked by the + // grace period. A stopped status *with* a pid is a service still booting. + if ( + status.phase === "running" || + (status.phase === "stopped" && status.pid === undefined) + ) + this.starting = undefined; + } return status; } @@ -268,7 +277,15 @@ export class ReMeEnvManager { } if (url.protocol !== "http:" && url.protocol !== "https:") throw new TypeError("ReMe endpoint must be an http(s) URL"); - return value; + if (url.username || url.password) + throw new TypeError("ReMe endpoint must not contain credentials"); + if (url.search || url.hash) + throw new TypeError( + "ReMe endpoint must not contain a query string or fragment", + ); + // The client appends job names to the endpoint, so it never keeps a + // trailing slash; normalising here is what makes records comparable. + return `${url.protocol}//${url.host}${url.pathname.replace(/\/+$/, "")}`; } private async startLocked( @@ -281,17 +298,30 @@ export class ReMeEnvManager { const dir = resolveWorkspaceDir(workspaceDir, homedir()); const logPath = join(dir, "service.log"); const now = new Date().toISOString(); - if ( - this.starting?.endpoint === endpointValue && - Date.now() - this.starting.at < START_GRACE_MS - ) + const starting = this.starting; + if (starting && Date.now() - starting.at < START_GRACE_MS) { + if (starting.endpoint === endpointValue) + return { + started: false, + alreadyRunning: true, + command: "", + logPath, + startedAt: now, + error: "The service is already starting for this endpoint", + }; + // The service record is per install, not per endpoint; spawning a second + // endpoint's service now would overwrite the first one's record and + // orphan it. return { started: false, - alreadyRunning: true, + alreadyRunning: false, command: "", logPath, startedAt: now, + error: `A service start for ${starting.endpoint} is still in progress`, }; + } + this.starting = undefined; const current = await this.serviceStatus(endpointValue, authToken); if (current.phase === "running") return { @@ -302,6 +332,35 @@ export class ReMeEnvManager { startedAt: now, ...(current.version ? { version: current.version } : {}), }; + if (current.phase === "conflict") + return { + started: false, + alreadyRunning: false, + command: "", + logPath, + startedAt: now, + error: "Another process is already listening on this endpoint", + }; + // A previous launch that never became healthy may still be alive. Stop it + // before spawning a replacement, so it cannot be orphaned when the state + // record is overwritten. + if (current.pid !== undefined) { + const previous = await stopService(this.serviceIO, { + statePath: this.serviceStatePath, + endpoint: endpointValue, + }); + if (!previous.stopped) + return { + started: false, + alreadyRunning: false, + command: "", + logPath, + startedAt: now, + error: `A previous launch could not be stopped: ${ + previous.reason ?? "unknown reason" + }`, + }; + } const path = pythonPath.trim(); if (!path) return { @@ -322,17 +381,45 @@ export class ReMeEnvManager { startedAt: now, error: "reme-ai is not installed in the selected interpreter", }; + if (!check.versionOk) + return { + started: false, + alreadyRunning: false, + command: "", + logPath, + startedAt: now, + error: `reme-ai ${ + check.version || "(unknown version)" + } is older than ${MINIMUM_REME}; upgrade it first`, + }; + // 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. this.starting = { endpoint: endpointValue, at: Date.now() }; - return startService(this.serviceIO, { - endpoint: endpointValue, - workspaceDir: dir, - pythonPath: path, - cliPath: check.cliPath, - // 0.4.0.x has no jobs..enabled; only newer releases accept it. - disableDreamCron: remeVersionAtLeast(check.version, "0.4.1"), - statePath: this.serviceStatePath, - logPath, - env: this.env, - }); + try { + const result = await startService(this.serviceIO, { + endpoint: endpointValue, + workspaceDir: dir, + pythonPath: path, + cliPath: check.cliPath, + // 0.4.0.x has no jobs..enabled; only newer releases accept it. + disableDreamCron: remeVersionAtLeast(check.version, "0.4.1"), + statePath: this.serviceStatePath, + logPath, + env: this.env, + }); + if (!result.started) this.starting = undefined; + return result; + } catch (error) { + this.starting = undefined; + return { + started: false, + alreadyRunning: false, + command: "", + logPath, + startedAt: now, + error: `Could not start the ReMe service: ${safeError(error)}`, + }; + } } } diff --git a/integrations/dsh/src/environment/host/paths.ts b/integrations/dsh/src/environment/host/paths.ts index 3f61756b..942f1d35 100644 --- a/integrations/dsh/src/environment/host/paths.ts +++ b/integrations/dsh/src/environment/host/paths.ts @@ -20,3 +20,16 @@ export function isWindowsPath(value: string): boolean { export function pathOf(value: string): PlatformPath { return isWindowsPath(value) ? win32 : posix; } + +/** + * Whether a path is absolute in its own platform's semantics. + * + * `win32.isAbsolute("\\python.exe")` is true, but such a path is relative to + * the current drive, which is not what the interpreter and install checks must + * accept. Windows therefore needs a drive letter or a UNC prefix. + */ +export function isAbsolutePath(value: string): boolean { + if (isWindowsPath(value)) + return /^[A-Za-z]:[\\/]/.test(value) || value.startsWith("\\\\"); + return value.startsWith("/"); +} diff --git a/integrations/dsh/src/environment/host/reme.ts b/integrations/dsh/src/environment/host/reme.ts index 5e6ea244..df30cd13 100644 --- a/integrations/dsh/src/environment/host/reme.ts +++ b/integrations/dsh/src/environment/host/reme.ts @@ -1,7 +1,7 @@ import { win32 } from "node:path"; import type { RemeCheck, RemeProbe } from "../types.js"; -import { pathOf } from "./paths.js"; +import { isAbsolutePath, pathOf } from "./paths.js"; import type { RunCommand } from "./runner.js"; /** Minimum reme-ai version this DSH integration is tested against. */ @@ -54,15 +54,25 @@ export async function probeReme( timeoutMs: 10_000, }); if (result.error || result.code !== 0) - return { installed: false, version: "", versionOk: false }; + return { + installed: false, + version: "", + versionOk: false, + ...(result.error ? { error: result.error } : {}), + }; const version = parseRemeVersion(result.stdout); return { installed: version !== "", version, versionOk: remeVersionAtLeast(version), }; - } catch { - return { installed: false, version: "", versionOk: false }; + } catch (error) { + return { + installed: false, + version: "", + versionOk: false, + error: String(error), + }; } } @@ -95,7 +105,7 @@ export async function checkReme( pythonPath: string, ): Promise { const path = pythonPath.trim(); - if (!path || !pathOf(path).isAbsolute(path)) { + if (!path || !isAbsolutePath(path)) { return { installed: false, version: "", @@ -112,7 +122,7 @@ export async function checkReme( versionOk: false, hasCore: false, cliPath: "", - error: "", + error: probe.error ?? "", }; } let hasCore = false; diff --git a/integrations/dsh/src/environment/host/runner.ts b/integrations/dsh/src/environment/host/runner.ts index 57346a8a..5e568adf 100644 --- a/integrations/dsh/src/environment/host/runner.ts +++ b/integrations/dsh/src/environment/host/runner.ts @@ -53,10 +53,42 @@ export function createNodeRunner( } let stdout = ""; let stderr = ""; + let settled = false; + let timedOut = false; + const timers: ReturnType[] = []; + const settle = (result: RunResult): void => { + if (settled) return; + settled = true; + for (const timer of timers) clearTimeout(timer); + resolve(result); + }; + const timeoutMs = options.timeoutMs ?? 10_000; const timer = setTimeout(() => { + // The caller must see an explicit error: an exit caused by our own + // timeout is not "the interpreter has no reme-ai". + timedOut = true; child.kill(); - }, options.timeoutMs ?? 10_000); + // A child that ignores SIGTERM must not make the probe hang forever: + // force it and settle, rather than waiting for a close that may never + // arrive. + const grace = setTimeout(() => { + try { + child.kill("SIGKILL"); + } catch { + // Already exited. + } + settle({ + code: null, + stdout, + stderr, + error: `Command timed out after ${timeoutMs} ms`, + }); + }, 1_000); + grace.unref(); + timers.push(grace); + }, timeoutMs); timer.unref(); + timers.push(timer); child.stdout?.on("data", (chunk) => { stdout += String(chunk); }); @@ -64,12 +96,17 @@ export function createNodeRunner( stderr += String(chunk); }); child.on("error", (error) => { - clearTimeout(timer); - resolve({ code: null, stdout, stderr, error: String(error) }); + settle({ code: null, stdout, stderr, error: String(error) }); }); child.on("close", (code) => { - clearTimeout(timer); - resolve({ code, stdout, stderr }); + settle({ + code, + stdout, + stderr, + ...(timedOut + ? { error: `Command timed out after ${timeoutMs} ms` } + : {}), + }); }); }); } diff --git a/integrations/dsh/src/environment/host/service.ts b/integrations/dsh/src/environment/host/service.ts index 0bf73937..f7bd370d 100644 --- a/integrations/dsh/src/environment/host/service.ts +++ b/integrations/dsh/src/environment/host/service.ts @@ -5,11 +5,12 @@ import { mkdirSync, openSync, readFileSync, + renameSync, rmSync, writeFileSync, } from "node:fs"; import { createConnection } from "node:net"; -import { dirname } from "node:path"; +import { basename, dirname } from "node:path"; import { safeError } from "../../endpoint.js"; import type { ServiceStartResult, ServiceState } from "../types.js"; @@ -56,7 +57,11 @@ export interface ServiceIO { logPath: string; env: NodeJS.ProcessEnv; }): Promise<{ pid: number | undefined }>; - killTree(pid: number): void; + /** + * Ask the process, and its process group on POSIX, to stop. `force` + * escalates to `SIGKILL` (always `/F` on Windows). + */ + killTree(pid: number, force?: boolean): void; probePid(pid: number): void; /** OS image name of a pid, or "" when it cannot be read. */ processImage(pid: number): Promise; @@ -64,7 +69,30 @@ export interface ServiceIO { platform: string; } -/** Only loopback endpoints may be managed as a local service. */ +/** A pid we are willing to signal: > 1, so kill(0)/kill(-1) is impossible. */ +export function isValidPid(pid: unknown): pid is number { + return typeof pid === "number" && Number.isInteger(pid) && pid > 1; +} + +/** + * Canonical identity of an endpoint for service-record matching. Loopback + * aliases and default ports compare equal, so saving `localhost:2333` does not + * make the service this plugin started on `127.0.0.1:2333` look foreign. + */ +export function serviceEndpointKey(endpoint: string): string { + try { + const url = new URL(endpoint.trim()); + const hostname = url.hostname.toLowerCase(); + const host = hostname === "localhost" ? "127.0.0.1" : hostname; + const port = url.port || (url.protocol === "https:" ? "443" : "80"); + const path = url.pathname.replace(/\/+$/, ""); + return `${url.protocol}//${host}:${port}${path}`; + } catch { + return endpoint.trim(); + } +} + +/** Only loopback HTTP endpoints may be managed as a local service. */ export function parseLocalEndpoint(endpoint: string): { host: string; port: number; @@ -75,13 +103,17 @@ export function parseLocalEndpoint(endpoint: string): { } catch { throw new TypeError("Invalid ReMe endpoint"); } - if (url.protocol !== "http:" && url.protocol !== "https:") - throw new TypeError("ReMe endpoint must be an http(s) URL"); + // The service this plugin launches is plain HTTP, so an https loopback URL + // could never become healthy and must not be offered as manageable. + if (url.protocol !== "http:") + throw new TypeError( + "Local service management requires an http:// endpoint", + ); if (!["127.0.0.1", "localhost", "[::1]", "::1"].includes(url.hostname)) throw new TypeError( "Local service management requires a loopback endpoint", ); - const port = Number(url.port || (url.protocol === "https:" ? "443" : "80")); + const port = Number(url.port || "80"); if (!Number.isInteger(port) || port < 1 || port > 65535) throw new TypeError("Invalid ReMe endpoint port"); return { host: "127.0.0.1", port }; @@ -129,21 +161,57 @@ export function buildStartCommand(options: { }; } +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +/** Read a record defensively: a malformed file is "no record", not a crash. */ export function readServiceState(path: string): PersistedService | undefined { + let value: unknown; try { - return JSON.parse(readFileSync(path, "utf8")) as PersistedService; + value = JSON.parse(readFileSync(path, "utf8")); } catch { return undefined; } + if (!isRecord(value)) return undefined; + const endpoint = typeof value.endpoint === "string" ? value.endpoint : ""; + if (!endpoint.trim()) return undefined; + return { + ...(isValidPid(value.pid) ? { pid: value.pid } : {}), + endpoint, + workspaceDir: + typeof value.workspaceDir === "string" ? value.workspaceDir : "", + pythonPath: typeof value.pythonPath === "string" ? value.pythonPath : "", + command: typeof value.command === "string" ? value.command : "", + startedAt: typeof value.startedAt === "string" ? value.startedAt : "", + }; } export function writeServiceState(path: string, state: PersistedService): void { mkdirSync(dirname(path), { recursive: true }); - writeFileSync(path, JSON.stringify(state, null, 2), "utf8"); + const temporary = `${path}.${process.pid}.tmp`; + writeFileSync(temporary, JSON.stringify(state, null, 2), { + encoding: "utf8", + mode: 0o600, + }); + try { + renameSync(temporary, path); + } catch (error) { + rmSync(temporary, { force: true }); + throw error; + } +} + +function clearServiceState(path: string): void { + try { + rmSync(path, { force: true }); + } catch { + // A stale state file is retried by the next status read. + } } function alive(io: ServiceIO, pid?: number): boolean { - if (pid === undefined) return false; + if (!isValidPid(pid)) return false; try { io.probePid(pid); return true; @@ -152,23 +220,37 @@ function alive(io: ServiceIO, pid?: number): boolean { } } +/** + * Whether an OS image name is a process that could be running the ReMe + * service: an exact Python-family name or `reme`, never a prefix match (which + * would accept e.g. `pycharm64.exe`). + */ +export function isServiceImage(image: string): boolean { + const name = basename(image.trim().replaceAll("\\", "/")).toLowerCase(); + return /^(python|pythonw|reme)([0-9.]*)?(\.exe)?$/.test(name); +} + /** * Whether the recorded process is the service this plugin started for the * endpoint on screen. The saved endpoint must match the requested one, the pid * must still be alive, and its image must still be a Python (or `reme`) * executable, which is what a recycled pid would rarely be. + * + * This guards a destructive operation, so it fails *closed*: an unreadable + * image is not proof of ownership. */ export async function ownsService( io: ServiceIO, persisted: PersistedService | undefined, endpoint: string, ): Promise { - if (!persisted || persisted.pid === undefined) return false; - if (persisted.endpoint.trim() !== endpoint.trim()) return false; + if (!persisted || !isValidPid(persisted.pid)) return false; + if (serviceEndpointKey(persisted.endpoint) !== serviceEndpointKey(endpoint)) + return false; if (!alive(io, persisted.pid)) return false; - const image = (await io.processImage(persisted.pid)).toLowerCase(); - if (image === "") return true; - return /^(reme|python|py)/.test(image); + const image = await io.processImage(persisted.pid); + if (!image.trim()) return false; + return isServiceImage(image); } /** Running / conflict / stopped, with ownership of the running process. */ @@ -186,8 +268,19 @@ export async function serviceStatus( options.authToken ?? "", ); const persisted = readServiceState(options.statePath); + const recordForEndpoint = + persisted !== undefined && + serviceEndpointKey(persisted.endpoint) === + serviceEndpointKey(options.endpoint); + const recordAlive = alive(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); if (health.ok) { - const managed = await ownsService(io, persisted, options.endpoint); + const managed = + recordForEndpoint && + recordAlive && + (await ownsService(io, persisted, options.endpoint)); return { phase: "running", managed, @@ -200,8 +293,14 @@ export async function serviceStatus( logPath: options.logPath, }; } - const { host, port } = parseLocalEndpoint(options.endpoint); - if (await io.portOpen(host, port)) + let portOpen = false; + try { + const { host, port } = parseLocalEndpoint(options.endpoint); + portOpen = await io.portOpen(host, port); + } catch { + // A remote or https endpoint is health-checked only; it has no local port. + } + if (portOpen) return { phase: "conflict", managed: false, @@ -212,7 +311,11 @@ export async function serviceStatus( phase: "stopped", managed: false, endpoint: options.endpoint, - ...(persisted?.pid === undefined ? {} : { pid: persisted.pid }), + // A live record for this endpoint means a launch that never became + // healthy; the manager stops it before spawning a replacement. + ...(recordForEndpoint && recordAlive && persisted?.pid !== undefined + ? { pid: persisted.pid } + : {}), logPath: options.logPath, }; } @@ -235,16 +338,28 @@ export async function startService( env: NodeJS.ProcessEnv; }, ): Promise { - mkdirSync(options.workspaceDir, { recursive: true }); - const command = buildStartCommand({ - cliPath: options.cliPath, - pythonPath: options.pythonPath, - workspaceDir: options.workspaceDir, - endpoint: options.endpoint, - disableDreamCron: options.disableDreamCron, - exists: io.exists, - }); const startedAt = new Date().toISOString(); + let command: { exe: string; args: string[]; display: string }; + try { + mkdirSync(options.workspaceDir, { recursive: true }); + command = buildStartCommand({ + cliPath: options.cliPath, + pythonPath: options.pythonPath, + workspaceDir: options.workspaceDir, + endpoint: options.endpoint, + disableDreamCron: options.disableDreamCron, + exists: io.exists, + }); + } catch (error) { + return { + started: false, + alreadyRunning: false, + command: "", + logPath: options.logPath, + startedAt, + error: `Could not start the ReMe service: ${safeError(error)}`, + }; + } let child: { pid: number | undefined }; try { child = await io.spawnService({ @@ -269,14 +384,32 @@ export async function startService( error: `Could not start the ReMe service: ${safeError(error)}`, }; } - writeServiceState(options.statePath, { - pid: child.pid, - endpoint: options.endpoint, - workspaceDir: options.workspaceDir, - pythonPath: options.pythonPath, - command: command.display, - startedAt, - }); + try { + writeServiceState(options.statePath, { + pid: child.pid, + endpoint: options.endpoint, + workspaceDir: options.workspaceDir, + pythonPath: options.pythonPath, + command: command.display, + startedAt, + }); + } catch (error) { + // The process exists but could not be recorded; leaving it running would + // make it unmanageable, so stop it before reporting the failure. + try { + if (child.pid !== undefined) io.killTree(child.pid, true); + } catch { + // Best effort: the error below already explains the launch failed. + } + return { + started: false, + alreadyRunning: false, + command: command.display, + logPath: options.logPath, + startedAt, + error: `Could not record the ReMe service: ${safeError(error)}`, + }; + } return { started: true, alreadyRunning: false, @@ -287,34 +420,97 @@ export async function startService( }; } +const STOP_TIMEOUT_MS = 5_000; +const STOP_FORCE_TIMEOUT_MS = 3_000; +const STOP_POLL_MS = 100; + +function delay(ms: number): Promise { + return new Promise((resolve) => { + setTimeout(resolve, ms); + }); +} + +/** Poll until the pid disappears; true when it exited within `timeoutMs`. */ +async function waitForExit( + io: ServiceIO, + pid: number, + timeoutMs: number, + pollMs: number, +): Promise { + const deadline = Date.now() + timeoutMs; + for (;;) { + if (!alive(io, pid)) return true; + const remaining = deadline - Date.now(); + if (remaining <= 0) return false; + await delay(Math.max(1, Math.min(pollMs, remaining))); + } +} + /** * 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 + * SIGTERM the service ignored would delete the pid, leaving a running process + * that the tab can never stop again. */ export async function stopService( io: ServiceIO, - options: { statePath: string; endpoint: string }, + options: { + statePath: string; + endpoint: string; + /** Grace period before escalating to SIGKILL (tests shorten it). */ + timeoutMs?: number; + forceTimeoutMs?: number; + pollMs?: number; + }, ): Promise { const persisted = readServiceState(options.statePath); - if (!persisted || persisted.pid === undefined) + if (!persisted || !isValidPid(persisted.pid)) { + if (persisted) clearServiceState(options.statePath); return { stopped: false, reason: "No service started by this plugin" }; + } if (!(await ownsService(io, persisted, options.endpoint))) return { stopped: false, reason: "The recorded service does not belong to this endpoint", }; const pid = persisted.pid; + const pollMs = options.pollMs ?? STOP_POLL_MS; try { io.killTree(pid); } catch { // Already exited. } - try { - rmSync(options.statePath, { force: true }); - } catch { - // A stale state file is harmless; the next start overwrites it. + if ( + await waitForExit(io, pid, options.timeoutMs ?? STOP_TIMEOUT_MS, pollMs) + ) { + clearServiceState(options.statePath); + return { stopped: true, pid }; } - return { stopped: true, pid }; + // The service ignored (or slowly handled) SIGTERM: escalate, then only drop + // the record once the process is really gone. + try { + io.killTree(pid, true); + } catch { + // Already exited. + } + if ( + await waitForExit( + io, + pid, + options.forceTimeoutMs ?? STOP_FORCE_TIMEOUT_MS, + pollMs, + ) + ) { + clearServiceState(options.statePath); + return { stopped: true, pid }; + } + return { + stopped: false, + pid, + reason: "The service did not exit after SIGKILL", + }; } /** Node-backed IO: fetch health, probe the port, spawn detached, tree-kill. */ @@ -402,30 +598,50 @@ export function createServiceIO(): ServiceIO { child.unref(); return { pid: child.pid }; }, - killTree(pid) { + killTree(pid, force = false) { + if (!isValidPid(pid)) return; if (process.platform === "win32") { - spawn("taskkill", ["/PID", String(pid), "/T", "/F"], { + const child = spawn("taskkill", ["/PID", String(pid), "/T", "/F"], { windowsHide: true, stdio: "ignore", }); + // A spawn failure is an async event; without a listener it would exit + // the Host. + child.on("error", () => undefined); + child.unref(); return; } + // The service was spawned detached, so it leads its own process group; + // signalling the group stops pip's children too. + const signal = force ? "SIGKILL" : "SIGTERM"; try { - process.kill(pid, "SIGTERM"); + process.kill(-pid, signal); + return; + } catch { + // Not a group leader (e.g. a record from an older launch). + } + try { + process.kill(pid, signal); } catch { // Already exited. } }, probePid(pid) { + if (!isValidPid(pid)) + throw new RangeError("Refusing to probe an invalid pid"); process.kill(pid, 0); }, processImage(pid) { return new Promise((resolve) => { + if (!isValidPid(pid)) { + resolve(""); + return; + } if (process.platform === "win32") { execFile( "tasklist", ["/FI", `PID eq ${pid}`, "/FO", "CSV", "/NH"], - { windowsHide: true }, + { windowsHide: true, timeout: 3000 }, (error, stdout) => { const match = /^"([^"]+)"/.exec(String(stdout ?? "").trim()); resolve(error ? "" : match?.[1] ?? ""); @@ -433,8 +649,11 @@ export function createServiceIO(): ServiceIO { ); return; } - execFile("ps", ["-p", String(pid), "-o", "comm="], (error, stdout) => - resolve(error ? "" : String(stdout ?? "").trim()), + execFile( + "ps", + ["-p", String(pid), "-o", "comm="], + { timeout: 3000 }, + (error, stdout) => resolve(error ? "" : String(stdout ?? "").trim()), ); }); }, diff --git a/integrations/dsh/src/environment/host/workspace.ts b/integrations/dsh/src/environment/host/workspace.ts index ff8c5d31..d01a534f 100644 --- a/integrations/dsh/src/environment/host/workspace.ts +++ b/integrations/dsh/src/environment/host/workspace.ts @@ -1,4 +1,13 @@ -import { mkdirSync, readFileSync, writeFileSync } from "node:fs"; +import { + chmodSync, + copyFileSync, + existsSync, + mkdirSync, + readFileSync, + renameSync, + rmSync, + writeFileSync, +} from "node:fs"; import { dirname, isAbsolute, join, resolve } from "node:path"; import { safeError } from "../../endpoint.js"; @@ -6,13 +15,13 @@ import type { EnvSummary, LlmTestResult } from "../types.js"; import type { RunCommand } from "./runner.js"; /** Keys the Environment tab manages; split for the two form groups. */ -export const LLM_KEYS = [ +const LLM_KEYS = [ "LLM_BACKEND", "LLM_MODEL_NAME", "LLM_API_KEY", "LLM_BASE_URL", ] as const; -export const EMBEDDING_KEYS = [ +const EMBEDDING_KEYS = [ "EMBEDDING_BACKEND", "EMBEDDING_MODEL_NAME", "EMBEDDING_API_KEY", @@ -138,10 +147,33 @@ export function validateEnvUpdate(values: Record): void { /** * Write a fully merged `.env`. The caller merges and validates first, so the * file on disk is exactly what was validated. + * + * The write is atomic (same-directory temp file plus rename) and keeps the + * previous file as `.env.bak`: a crash or a full disk must not take unrelated + * keys such as `LLM_API_KEY` with it. New files are owner-only (0600) because + * they hold credentials. */ export function writeEnvText(path: string, text: string): void { mkdirSync(dirname(path), { recursive: true }); - writeFileSync(path, text, "utf8"); + const temporary = `${path}.${process.pid}.tmp`; + writeFileSync(temporary, text, { encoding: "utf8", mode: 0o600 }); + try { + if (existsSync(path)) { + const backup = `${path}.bak`; + copyFileSync(path, backup); + // The backup holds the same credentials, so it must not be more readable + // than the file it preserves. + try { + chmodSync(backup, 0o600); + } catch { + // Windows has no POSIX mode bits; the copy is still user-owned. + } + } + renameSync(temporary, path); + } catch (error) { + rmSync(temporary, { force: true }); + throw error; + } } /** diff --git a/integrations/dsh/src/environment/types.ts b/integrations/dsh/src/environment/types.ts index ebc09168..82fceaf1 100644 --- a/integrations/dsh/src/environment/types.ts +++ b/integrations/dsh/src/environment/types.ts @@ -47,6 +47,8 @@ export interface RemeProbe { version: string; /** True when the version satisfies the minimum this integration targets. */ versionOk: boolean; + /** Non-empty when the probe itself failed (spawn error, timeout). */ + error?: string; } export interface RemeCheck extends RemeProbe { diff --git a/integrations/dsh/src/reme/client.ts b/integrations/dsh/src/reme/client.ts index 50d7f65b..e00b0998 100644 --- a/integrations/dsh/src/reme/client.ts +++ b/integrations/dsh/src/reme/client.ts @@ -52,9 +52,15 @@ export class ReMeClient { sessionId: string, options: AutoMemoryOptions = {}, ): Promise { - const config = { - ...this.configSource(), - ...(options.endpoint ? { endpoint: options.endpoint } : {}), + const base = this.configSource(); + const endpoint = options.endpoint || base.endpoint; + // A recovered batch keeps the endpoint it was queued for. The token is + // scoped to the *configured* endpoint, so it must not ride along when the + // batch goes somewhere else. + const config: ReMeClientConfig = { + ...base, + endpoint, + ...(endpoint === base.endpoint ? {} : { authToken: "" }), }; return this.request( "auto_memory", diff --git a/integrations/dsh/tests/client.test.mjs b/integrations/dsh/tests/client.test.mjs index 21ed3d3c..5c0d8a81 100644 --- a/integrations/dsh/tests/client.test.mjs +++ b/integrations/dsh/tests/client.test.mjs @@ -100,6 +100,32 @@ test("uses the persisted destination and request ID when sending a recovered bat } }); +test("scopes the bearer token to the endpoint it was configured for", async () => { + const originalFetch = globalThis.fetch; + const requests = []; + globalThis.fetch = async (url, init) => { + requests.push({ url, authorization: init.headers.Authorization }); + return Response.json({ success: true }); + }; + try { + const client = new ReMeClient({ + endpoint: "http://new.test", + authToken: "current-token", + requestTimeoutMs: 1000, + backgroundTimeoutMs: 1000, + }); + // The current endpoint keeps the token. + await client.search("fact"); + assert.equal(requests.at(-1).authorization, "Bearer current-token"); + // A recovered batch for another endpoint must not receive it. + await client.autoMemory([], "session", { endpoint: "http://old.test" }); + assert.equal(requests.at(-1).url, "http://old.test/auto_memory"); + assert.equal(requests.at(-1).authorization, undefined); + } finally { + globalThis.fetch = originalFetch; + } +}); + test("calls ReMe jobs with their native request and response envelopes", async () => { const calls = []; const originalFetch = globalThis.fetch; diff --git a/integrations/dsh/tests/env-detect.test.mjs b/integrations/dsh/tests/env-detect.test.mjs index 118ea565..f1749cf9 100644 --- a/integrations/dsh/tests/env-detect.test.mjs +++ b/integrations/dsh/tests/env-detect.test.mjs @@ -607,6 +607,49 @@ test("detects Unix interpreters and merges paths that resolve together", async ( assert.equal(named.version, "3.11.9"); }); +test("probes well-known Unix locations beyond the Host PATH", async () => { + const homebrew = "/opt/homebrew/bin/python3"; + const pyenv = "/Users/u/.pyenv/shims/python3"; + const rules = [ + { + match: (exe) => exe === "which", + result: { code: 1, stdout: "", stderr: "" }, + }, + verifyRule(homebrew, "3.12.4", "/opt/homebrew"), + // pyenv shims report the real interpreter they dispatch to. + { + match: (actual, args) => + actual === pyenv && + args[0] === VERIFY_ARG0 && + String(args[1]).includes("import sys"), + result: { + code: 0, + stdout: + '{"exe":"/Users/u/.pyenv/versions/3.12.4/bin/python3","ver":"3.12.4","base":"/Users/u/.pyenv/versions/3.12.4"}\n', + stderr: "", + }, + }, + remeRule(homebrew, "0.4.1.11"), + remeRule(pyenv, null), + remeRule("/Users/u/.pyenv/versions/3.12.4/bin/python3", null), + ]; + const detection = await detectPythonEnvs({ + run: fakeRun(rules), + platform: "darwin", + env: {}, + homedir: "/Users/u", + exists: (path) => path === homebrew || path === pyenv, + }); + assert.deepEqual( + detection.envs.map((env) => env.exe).sort(), + [homebrew, "/Users/u/.pyenv/versions/3.12.4/bin/python3"].sort(), + ); + assert.ok( + detection.envs.every((env) => env.origin.kind === "system"), + "well-known paths are classified as system installations", + ); +}); + test("collects conda failures without rejecting the whole detection", async () => { const { io, miniconda } = windowsFixture(); io.run = async (exe, args) => { diff --git a/integrations/dsh/tests/env-manager.test.mjs b/integrations/dsh/tests/env-manager.test.mjs index ae7cd21d..85603a99 100644 --- a/integrations/dsh/tests/env-manager.test.mjs +++ b/integrations/dsh/tests/env-manager.test.mjs @@ -5,10 +5,11 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { ReMeEnvManager } from "../dist/environment/host/manager.js"; +import { writeServiceState } from "../dist/environment/host/service.js"; /** A service adapter that records launches instead of starting processes. */ function fakeServiceIO() { - const calls = { spawn: [] }; + const calls = { killTree: [], spawn: [] }; return { calls, io: { @@ -49,7 +50,7 @@ function managerIn(t) { endpoint: endpoint || "http://127.0.0.1:2333", logPath: join(directory, "service.log"), }); - return { manager, calls, directory }; + return { manager, calls, directory, io }; } test("serializes concurrent starts into one service launch", async (t) => { @@ -69,6 +70,84 @@ test("serializes concurrent starts into one service launch", async (t) => { assert.equal(second.alreadyRunning, true); }); +test("a failed spawn can be retried immediately", async (t) => { + const { manager, calls, directory, io } = managerIn(t); + io.spawnService = async (options) => { + calls.spawn.push(options); + if (calls.spawn.length === 1) throw new Error("spawn ENOENT"); + return { pid: 7000 }; + }; + const options = [ + "http://127.0.0.1:2334", + "D:\\env\\python.exe", + join(directory, "workspace"), + ]; + const first = await manager.serviceStart(...options); + assert.equal(first.started, false); + assert.match(first.error, /ENOENT/); + + // The start marker was cleared with the failure; a retry must really spawn + // instead of being reported as "already starting". + const second = await manager.serviceStart(...options); + assert.equal(second.started, true); + assert.equal(calls.spawn.length, 2); +}); + +test("refuses to spawn while another process owns the port", async (t) => { + const { manager, calls, directory } = managerIn(t); + manager.serviceStatus = async (endpoint) => ({ + phase: "conflict", + managed: false, + endpoint, + logPath: join(directory, "service.log"), + }); + const result = await manager.serviceStart( + "http://127.0.0.1:2334", + "D:\\env\\python.exe", + join(directory, "workspace"), + ); + assert.equal(result.started, false); + assert.match(result.error, /already listening/); + assert.equal(calls.spawn.length, 0); +}); + +test("stops a previous launch that never became healthy before replacing it", async (t) => { + const { manager, calls, directory, io } = managerIn(t); + manager.serviceStatus = async (endpoint) => ({ + phase: "stopped", + managed: false, + endpoint, + // A live record for this endpoint means a launch that is still booting. + pid: 6000, + logPath: join(directory, "service.log"), + }); + writeServiceState(join(directory, "reme-memory", "service.json"), { + pid: 6000, + endpoint: "http://127.0.0.1:2334", + workspaceDir: directory, + pythonPath: "D:\\env\\python.exe", + command: "reme start", + startedAt: "2026-10-27T00:00:00.000Z", + }); + let running = true; + io.probePid = () => { + if (!running) throw new Error("ESRCH"); + }; + io.killTree = (pid) => { + calls.killTree.push(pid); + running = false; + }; + + const result = await manager.serviceStart( + "http://127.0.0.1:2334", + "D:\\env\\python.exe", + join(directory, "workspace"), + ); + assert.equal(result.started, true); + assert.deepEqual(calls.killTree, [6000]); + assert.equal(calls.spawn.length, 1); +}); + test("probes health from the Host and rejects a non-http endpoint", async (t) => { const { manager } = managerIn(t); const health = await manager.health("http://127.0.0.1:2334"); diff --git a/integrations/dsh/tests/env-reme.test.mjs b/integrations/dsh/tests/env-reme.test.mjs index 3d70a672..ff8b1d0b 100644 --- a/integrations/dsh/tests/env-reme.test.mjs +++ b/integrations/dsh/tests/env-reme.test.mjs @@ -148,6 +148,15 @@ test("composes pip arguments for install, upgrade and mirrors", () => { assert.throws(() => pipArgs({ pythonPath: "C:\\env\\python.exe", indexUrl: "ftp://x" }), ); + // Credentials would be echoed in the job command, the log and the browser. + assert.throws( + () => + pipArgs({ + pythonPath: "C:\\env\\python.exe", + indexUrl: "https://user:token@mirror.example/simple", + }), + /must not embed credentials/, + ); }); test("tails a log with CRLF and progress carriage returns", (t) => { @@ -281,5 +290,8 @@ test("reuses the running job for the same interpreter and validates paths", (t) const second = jobs.start({ pythonPath: "C:\\env\\python.exe" }); assert.equal(first.id, second.id); assert.throws(() => jobs.start({ pythonPath: "relative\\python.exe" })); + // Root-relative Windows paths pass win32.isAbsolute but are not absolute in + // the sense the install needs (they resolve against the current drive). + assert.throws(() => jobs.start({ pythonPath: "\\python.exe" })); assert.equal(jobs.status("unknown-job"), null); }); diff --git a/integrations/dsh/tests/env-runner.test.mjs b/integrations/dsh/tests/env-runner.test.mjs new file mode 100644 index 00000000..4e359b7b --- /dev/null +++ b/integrations/dsh/tests/env-runner.test.mjs @@ -0,0 +1,30 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { createNodeRunner } from "../dist/environment/host/runner.js"; + +test("a timeout settles with an explicit error instead of hanging", async () => { + // The child ignores SIGTERM, so the grace path must force it and settle. + const run = createNodeRunner({ PATH: process.env.PATH }); + const started = Date.now(); + const result = await run( + process.execPath, + ["-e", "process.on('SIGTERM', () => {}); setInterval(() => {}, 1000)"], + { timeoutMs: 200 }, + ); + assert.match(result.error, /timed out after 200 ms/); + assert.ok( + Date.now() - started < 5_000, + "the probe must not wait for a process that never closes", + ); +}); + +test("a child that exits on SIGTERM is still reported as a timeout", async () => { + const run = createNodeRunner({ PATH: process.env.PATH }); + const result = await run( + process.execPath, + ["-e", "setInterval(() => {}, 1000)"], + { timeoutMs: 200 }, + ); + assert.match(result.error ?? "", /timed out after 200 ms/); +}); diff --git a/integrations/dsh/tests/env-service.test.mjs b/integrations/dsh/tests/env-service.test.mjs index c848e3fb..c7c0504b 100644 --- a/integrations/dsh/tests/env-service.test.mjs +++ b/integrations/dsh/tests/env-service.test.mjs @@ -1,11 +1,13 @@ import assert from "node:assert/strict"; import test from "node:test"; +import { createServer } from "node:http"; import { mkdtempSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { buildStartCommand, + createServiceIO, ownsService, parseLocalEndpoint, readServiceState, @@ -18,7 +20,7 @@ import { const ENDPOINT = "http://127.0.0.1:2334"; function fakeIO(overrides = {}) { - const calls = { killTree: [], spawn: [] }; + const calls = { killTree: [], force: [], spawn: [] }; const io = { fetchHealth: async () => ({ ok: false, version: "", error: "" }), portOpen: async () => false, @@ -26,8 +28,9 @@ function fakeIO(overrides = {}) { calls.spawn.push(options); return { pid: 4321 }; }, - killTree(pid) { + killTree(pid, force = false) { calls.killTree.push(pid); + if (force) calls.force.push(pid); }, probePid() {}, processImage: async () => "python.exe", @@ -56,7 +59,7 @@ function record(statePath, overrides = {}) { }); } -test("accepts only loopback endpoints and validates the port", () => { +test("accepts only loopback HTTP endpoints and validates the port", () => { assert.deepEqual(parseLocalEndpoint("http://127.0.0.1:2334"), { host: "127.0.0.1", port: 2334, @@ -68,6 +71,12 @@ test("accepts only loopback endpoints and validates the port", () => { assert.throws(() => parseLocalEndpoint("http://10.0.0.5:2333")); assert.throws(() => parseLocalEndpoint("not-a-url")); assert.throws(() => parseLocalEndpoint("http://127.0.0.1:0")); + // The plugin starts a plain HTTP service, so https on loopback can never be + // healthy; it must not be offered as manageable. + assert.throws( + () => parseLocalEndpoint("https://127.0.0.1:2333"), + /http:\/\//, + ); }); test("builds reme start with the workspace cwd contract", () => { @@ -162,6 +171,21 @@ test("a service on another endpoint is not reported as managed", async (t) => { assert.equal(status.pid, undefined); }); +test("a loopback alias still identifies the service this plugin started", async (t) => { + const { directory, statePath } = tempState(t, "reme-alias-"); + record(statePath, { endpoint: "http://localhost:2334/" }); + const { io } = fakeIO({ + fetchHealth: async () => ({ ok: true, version: "0.4.0.9", error: "" }), + }); + const status = await serviceStatus(io, { + endpoint: ENDPOINT, + statePath, + logPath: join(directory, "service.log"), + }); + assert.equal(status.managed, true); + assert.equal(status.pid, 777); +}); + test("a recycled pid that is no longer the service is not managed", async (t) => { const { statePath } = tempState(t, "reme-pid-"); record(statePath); @@ -182,6 +206,35 @@ test("a recycled pid that is no longer the service is not managed", async (t) => await ownsService(gone, readServiceState(statePath), ENDPOINT), false, ); + // Ownership guards a destructive action: an image that cannot be read is + // not evidence that the pid is ours, and a prefix match would accept e.g. + // pycharm64.exe. + for (const image of ["", " ", "pycharm64.exe", "python-config"]) { + const { io: unreadable } = fakeIO({ processImage: async () => image }); + assert.equal( + await ownsService(unreadable, readServiceState(statePath), ENDPOINT), + false, + `image ${JSON.stringify(image)} must not be owned`, + ); + } +}); + +test("a dead record is cleared instead of reporting a stale pid", async (t) => { + const { directory, statePath } = tempState(t, "reme-dead-"); + record(statePath); + const { io } = fakeIO({ + probePid() { + throw new Error("ESRCH"); + }, + }); + const status = await serviceStatus(io, { + endpoint: ENDPOINT, + statePath, + logPath: join(directory, "service.log"), + }); + assert.equal(status.phase, "stopped"); + assert.equal(status.pid, undefined); + assert.equal(readServiceState(statePath), undefined); }); test("starts detached with the GBK-safe environment and records state", async (t) => { @@ -230,11 +283,24 @@ test("records nothing when the spawn fails", async (t) => { assert.equal(readServiceState(statePath), undefined); }); -test("stops the managed pid and clears the state file", async (t) => { +test("stops the managed pid and clears the state file after it exits", async (t) => { const { directory, statePath } = tempState(t, "reme-stop-"); record(statePath, { pid: 999, workspaceDir: directory }); const { io, calls } = fakeIO(); - const stopped = await stopService(io, { statePath, endpoint: ENDPOINT }); + let running = true; + io.probePid = () => { + if (!running) throw new Error("ESRCH"); + }; + io.killTree = (pid) => { + calls.killTree.push(pid); + running = false; + }; + const stopped = await stopService(io, { + statePath, + endpoint: ENDPOINT, + timeoutMs: 100, + pollMs: 5, + }); assert.deepEqual(stopped, { stopped: true, pid: 999 }); assert.deepEqual(calls.killTree, [999]); assert.equal(readServiceState(statePath), undefined); @@ -244,6 +310,25 @@ test("stops the managed pid and clears the state file", async (t) => { }); }); +test("escalates to SIGKILL and keeps the record when the service never exits", async (t) => { + const { directory, statePath } = tempState(t, "reme-stubborn-"); + record(statePath, { pid: 999, workspaceDir: directory }); + const { io, calls } = fakeIO(); + const stopped = await stopService(io, { + statePath, + endpoint: ENDPOINT, + timeoutMs: 10, + forceTimeoutMs: 10, + pollMs: 2, + }); + assert.equal(stopped.stopped, false); + assert.match(stopped.reason, /did not exit/); + assert.deepEqual(calls.killTree, [999, 999]); + assert.deepEqual(calls.force, [999]); + // The pid survives in the record, so Stop can be tried again later. + assert.equal(readServiceState(statePath)?.pid, 999); +}); + test("refuses to stop a service recorded for another endpoint", async (t) => { const { statePath } = tempState(t, "reme-refuse-"); record(statePath, { endpoint: "http://127.0.0.1:2333" }); @@ -254,3 +339,80 @@ test("refuses to stop a service recorded for another endpoint", async (t) => { assert.deepEqual(calls.killTree, []); assert.ok(readServiceState(statePath), "the foreign record is left in place"); }); + +/* ------------------------------------------------------------------ */ +/* Real IO: the pieces the fake adapter above cannot vouch for. */ +/* ------------------------------------------------------------------ */ + +async function listening(handler, t) { + const server = createServer(handler); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + t.after(() => server.close()); + const address = server.address(); + return `http://127.0.0.1:${address.port}`; +} + +test("real IO accepts only a ReMe health report", async (t) => { + const reme = await listening((request, response) => { + response.setHeader("Content-Type", "application/json"); + if (request.url === "/health_check") + response.end( + JSON.stringify({ + success: true, + metadata: { health: { version: "0.4.2.1", healthy: true } }, + }), + ); + else response.end("{}"); + }, t); + const generic = await listening((_request, response) => { + response.setHeader("Content-Type", "application/json"); + response.end("{}"); + }, t); + + const io = createServiceIO(); + const good = await io.fetchHealth(reme, ""); + assert.equal(good.ok, true); + assert.equal(good.version, "0.4.2.1"); + + const empty = await io.fetchHealth(generic, ""); + assert.equal(empty.ok, false); + assert.match(empty.error, /ReMe health report/); + + // The Authorization header is actually sent when a token is configured. + let authorization = ""; + const guarded = await listening((request, response) => { + authorization = String(request.headers.authorization ?? ""); + response.end("{}"); + }, t); + await io.fetchHealth(guarded, "secret-token"); + assert.equal(authorization, "Bearer secret-token"); +}); + +test("real IO spawns, identifies and stops a detached process", async (t) => { + const directory = mkdtempSync(join(tmpdir(), "reme-real-service-")); + t.after(() => rmSync(directory, { recursive: true, force: true })); + const io = createServiceIO(); + const { pid } = await io.spawnService({ + exe: process.execPath, + args: ["-e", "setInterval(() => {}, 1000)"], + cwd: directory, + logPath: join(directory, "service.log"), + env: process.env, + }); + assert.ok(pid !== undefined && pid > 1); + assert.doesNotThrow(() => io.probePid(pid)); + assert.doesNotMatch(await io.processImage(pid), /^$/); + + io.killTree(pid); + const deadline = Date.now() + 5_000; + let running = true; + while (running && Date.now() < deadline) { + try { + io.probePid(pid); + await new Promise((resolve) => setTimeout(resolve, 50)); + } catch { + running = false; + } + } + assert.equal(running, false, "the detached process must actually exit"); +}); diff --git a/integrations/dsh/tests/env-tab.test.mjs b/integrations/dsh/tests/env-tab.test.mjs index c75d3560..43e0eeff 100644 --- a/integrations/dsh/tests/env-tab.test.mjs +++ b/integrations/dsh/tests/env-tab.test.mjs @@ -52,13 +52,21 @@ const RESPONSES = { }, "remeEnv/installStatus": null, "remeEnv/readEnv": { + workspaceDir: "C:\\Users\\test\\.reme", exists: true, envPath: "C:\\Users\\test\\.reme\\.env", entries: [ - { key: "LLM_BACKEND", value: "dashscope", secret: false, set: true }, + { + key: "LLM_BACKEND", + value: "dashscope", + masked: "dashscope", + secret: false, + set: true, + }, { key: "LLM_MODEL_NAME", value: "qwen3.7-plus", + masked: "qwen3.7-plus", secret: false, set: true, }, @@ -70,6 +78,7 @@ const RESPONSES = { managed: false, pid: 98820, endpoint: "http://127.0.0.1:2333", + logPath: "C:\\Users\\test\\.reme\\service.log", }, }; @@ -250,6 +259,32 @@ test("marks the suggested interpreter and explains why", async (t) => { ]); }); +test("sends the chosen package mirror with the install request", async (t) => { + const { renderer, calls, restore } = await mount(); + t.after(restore); + + const select = renderer.root.findAll((node) => node.type === "select")[0]; + await act(async () => { + select.props.onChange({ + target: { value: "https://pypi.tuna.tsinghua.edu.cn/simple" }, + }); + }); + await act(async () => { + buttonByText(renderer, envZh.reinstall).props.onClick(); + }); + await act(async () => { + buttonByText(renderer, envZh.confirmInstall).props.onClick(); + }); + + const install = calls.rpc.filter((call) => call.method === "remeEnv/install"); + assert.ok(install.length > 0, "the install request must be sent"); + assert.deepEqual(install.at(-1).input, { + pythonPath: SUGGESTED, + upgrade: false, + indexUrl: "https://pypi.tuna.tsinghua.edu.cn/simple", + }); +}); + test("drops .env drafts when the workspace changes", async (t) => { const { renderer, calls, setValue, restore } = await mount(); t.after(restore);