fix(pty,subprocess,lsp): address review round on cancellation, lookup, and contracts
- pty-local: a pre-write inspection rejection no longer releases a canceled send while its foreground SIGINT is in flight; the interrupt path retains the slot and its post-signal tail resumes polling. Regression pins the failure-shaped cancellation and a close-during-write release. - pty-local: SEND_ACTIVE names which provider operation is draining; README states the never-settling-provider outcome (slot retained, close recovers). - subprocess-local: resolveExecutable rejects relative paths containing separators instead of expanding them per PATH entry with a misleading not-found error; seam JSDoc pins the rule for every provider. - subprocess-local: LocalTerminalHandle documents why the seam's in-flight-join promise holds without operation tracking. - lsp-local: the oversized-source diagnostic reports the observed byte lower bound; README documents that processId: null trades away server-side orphan cleanup after a hard-killed harness. - pty/subprocess seams: cross-reference the twin five-member signal unions. - pty-local: TODO markers for send-state consolidation and initialize-race relocation.
This commit is contained in:
@@ -2,5 +2,5 @@
|
||||
# side as of the last confirmed-consistent state. Both languages carry equal authority;
|
||||
# after editing either side, bring the other along and re-record with:
|
||||
# pnpm run verify-translation-pairing --write packages/subprocess/subprocess-local/README.md
|
||||
README.md: 85103c2634bd35b188acd71c7f037e3678a2542e
|
||||
README.zh.md: 505eac4f2650e6723e042cdb1122f9ef504ad7df
|
||||
README.md: b901a69d6cfd45a084711ba0d32e555c481fd626
|
||||
README.zh.md: 5723dc99ff955c5f4b07ea96e7d2134100d9fd6a
|
||||
|
||||
@@ -10,7 +10,7 @@ Local implementation of the [`@deepseek-ai/dsh-subprocess`](../subprocess/README
|
||||
- **Per-stream dispositions** — `'pipe'` hands the raw stream to the caller untouched (protocol framing stays consumer-owned); `'inherit'` passes the parent descriptor through; collect mode keeps the in-memory TAIL beyond its cap (errors and results cluster at the end — pi/OpenCode rationale) while the FULL stream is appended to a private temp file when a spill cap is configured — omitting `spill` keeps only the tail, the diagnostic shape. A stream larger than the spill cap discards its now-incomplete spill and returns only the marked truncated tail; spill fds are sealed at settlement, and a failed final close withholds the path rather than advertising an incomplete file. Spill files are `0600` with random names under a lazily-created `0700` per-process directory.
|
||||
- **Credential scrub + explicit merge** — `process.env` minus credential-shaped vars (`*KEY*`/`*SECRET*`/`*TOKEN*`) and all ambient `DSH_*` names; the spec's explicit `env` merges after that scrub with no namespace validation, so a deliberately supplied credential or current `DSH_*` fact wins while stale nested-harness identity cannot leak in ambiently. Supplied stdin is written and closed; otherwise fd 0 is `/dev/null`. See the [stdin/env Agent Note](../../../.agents/notes/implemented/architecture/2026-06-30-bash-stdin-env-trusted-plugin-surface.md) and [managed environment Agent Note](../../../.agents/notes/implemented/feature/2026-07-10-agent-session-identity-and-log-location.md).
|
||||
- **Offset-based reads** — collect-mode readers return deltas in whole-stream byte coordinates; the service never holds a cursor, so consumer-owned cursors (the bash background read path) and full-stream re-reads coexist, before and after settlement.
|
||||
- **Executable lookup** — `resolveExecutable` checks absolute files or searches the scrubbed effective PATH with platform-aware executable extensions; relative PATH entries resolve from the host process cwd.
|
||||
- **Executable lookup** — `resolveExecutable` checks absolute files or searches the scrubbed effective PATH with platform-aware executable extensions; relative paths containing separators are rejected at the seam, and relative PATH entries resolve from the host process cwd.
|
||||
- **Terminal-process ownership** — `spawnTerminal` allocates `node-pty`, bridges UTF-8 terminal text, inspects and signals the current foreground process group, and exposes one awaited termination operation that sweeps descendants before and after terminating the top-level shell. Each foreground inspection retains exact identities from the rooted tree; Linux also enumerates the POSIX session after its leader exits. A previously observed macOS descendant and any same-session Linux member therefore remain fenced after reparenting, while pid/start identity prevents cleanup from following PID reuse. The higher PTY backend owns prompt readiness, buffers, and model-facing operations.
|
||||
- **Terminate-and-join disposal** — the service retains live handles only so its own disposal can escalate every running tree and await its exit; settled and spawn-failed handles leave the live set on settlement.
|
||||
|
||||
|
||||
@@ -10,7 +10,7 @@
|
||||
- **按流划分的处置方式**:`'pipe'` 把原始流原样交给调用方(协议分帧仍归消费方所有);`'inherit'` 直通父进程的描述符;收集模式(collect)在输出超过上限后于内存中保留尾部(错误与结果通常聚集在末尾,沿用 pi/OpenCode 的理由),并在配置了 spill 上限时把完整流追加到一个私有临时文件;省略 `spill` 则只保留用于诊断的尾部。某条流大于 spill 上限时,会丢弃已不完整的 spill,仅返回带截断标记的尾部;spill 文件描述符在结算时封存,最终关闭失败时则不公布路径,以免声称存在不完整的文件。spill 文件权限为 `0600`、名称随机,位于按需创建、权限为 `0700` 的每进程目录之下。
|
||||
- **凭据清除 + 显式合并**:以 `process.env` 为基础,移除形似凭据的变量(`*KEY*`/`*PASSWORD*`/`*SECRET*`/`*TOKEN*`)和所有环境中已有的 `DSH_*` 名称;spec 的显式 `env` 在该清除之后合并且不做命名空间校验,因此有意提供的凭据或当前 `DSH_*` 事实会胜出,而陈旧的嵌套 harness 身份无法从环境中隐式漏入。提供的 stdin 会被写入后关闭;否则 fd 0 指向 `/dev/null`。参见 [stdin/env Agent Note(agent 决策记录)](../../../.agents/notes/implemented/architecture/2026-06-30-bash-stdin-env-trusted-plugin-surface.md)与[受管环境 Agent Note](../../../.agents/notes/implemented/feature/2026-07-10-agent-session-identity-and-log-location.md)。
|
||||
- **基于偏移量的读取**:收集模式的读取器按完整流的字节坐标返回增量;服务自身从不持有游标,因此消费方自有的游标(bash 的后台读取路径)与完整流重读可以共存,结算前后皆然。
|
||||
- **可执行文件查找**:`resolveExecutable` 检查绝对文件,或根据平台可执行文件扩展名在清理后的有效 PATH 中搜索;相对 PATH 条目从宿主进程 cwd 解析。
|
||||
- **可执行文件查找**:`resolveExecutable` 检查绝对文件,或根据平台可执行文件扩展名在清理后的有效 PATH 中搜索;含分隔符的相对路径在接缝处被拒绝,相对 PATH 条目从宿主进程 cwd 解析。
|
||||
- **终端进程所有权**:`spawnTerminal` 分配 `node-pty`,桥接 UTF-8 终端文本,检查当前前台进程组并向其发送信号,还会公开一项须等待的终止操作,在终止顶层 shell 前后清理后代进程。每次前台检查都会保留根进程树中的精确身份;Linux 还会在 POSIX 会话 leader 退出后枚举该会话。因此,之前观察到的 macOS 后代以及同会话 Linux 成员在重新设定父进程后仍受围栏保护,pid/start 身份则防止清理跟随 PID 复用。上层 PTY 后端负责提示符就绪、缓冲区与面向模型的操作。
|
||||
- **先终止再等待退出的 dispose(资源释放)**:服务保留存活句柄,只为让自身的 dispose 能对每个仍在运行的进程树执行升级并等待其退出;已结算与 spawn 失败的句柄在结算时即离开存活集合。
|
||||
|
||||
|
||||
@@ -77,6 +77,11 @@ export class LocalSubprocessService extends SubprocessService {
|
||||
signal?.throwIfAborted()
|
||||
const environment = childEnv(env)
|
||||
const absolute = isAbsolute(command)
|
||||
if (!absolute && (command.includes('/') || (process.platform === 'win32' && command.includes('\\')))) {
|
||||
throw new Error(
|
||||
`subprocess-local: command ${JSON.stringify(command)} is a relative path; use an absolute path or a bare PATH name`,
|
||||
)
|
||||
}
|
||||
const candidates = absolute ? [command] : this.executableCandidates(command, environment)
|
||||
for (const candidate of candidates) {
|
||||
signal?.throwIfAborted()
|
||||
|
||||
@@ -24,7 +24,14 @@ function signalName(number: number | undefined): NodeJS.Signals | null {
|
||||
return null
|
||||
}
|
||||
|
||||
/** A local terminal whose process-session ownership stays below the PTY backend. */
|
||||
/**
|
||||
* A local terminal whose process-session ownership stays below the PTY backend.
|
||||
* The seam's terminate() promise — no write, inspection, or signal in flight
|
||||
* after settlement — holds here without operation tracking only because every
|
||||
* handle call completes synchronously under the hood (node-pty write, ps-based
|
||||
* inspection). A first genuinely asynchronous step in any handle call must add
|
||||
* the tracking a remote provider needs.
|
||||
*/
|
||||
export class LocalTerminalHandle implements SubprocessTerminalHandle {
|
||||
readonly pid: number
|
||||
readonly output = new PassThrough()
|
||||
|
||||
@@ -32,6 +32,10 @@ describe('LocalSubprocessService', () => {
|
||||
PATH: relative(process.cwd(), dirname(process.execPath)) || '.',
|
||||
})).toBe(process.execPath)
|
||||
await expect(ctx.subprocess.resolveExecutable('')).rejects.toThrow('must be non-empty')
|
||||
await expect(ctx.subprocess.resolveExecutable('./bin/tsserver'))
|
||||
.rejects.toThrow('is a relative path')
|
||||
await expect(ctx.subprocess.resolveExecutable('node_modules/.bin/server'))
|
||||
.rejects.toThrow('is a relative path')
|
||||
await expect(ctx.subprocess.resolveExecutable('dsh-command-that-does-not-exist', { PATH: '' }))
|
||||
.rejects.toThrow('was not found on PATH')
|
||||
await expect(ctx.subprocess.resolveExecutable('/dsh-absolute-command-that-does-not-exist'))
|
||||
@@ -61,6 +65,8 @@ describe('LocalSubprocessService', () => {
|
||||
.toEqual(['/explicit/tool.EXE'])
|
||||
expect(candidates('tool.exe', {})).toEqual([resolve(process.cwd(), 'tool.exe')])
|
||||
expect(candidates('tool', { PATH: '/bin' })).toHaveLength(4)
|
||||
await expect(ctx.subprocess.resolveExecutable(String.raw`bin\server.exe`))
|
||||
.rejects.toThrow('is a relative path')
|
||||
} finally {
|
||||
platform.mockRestore()
|
||||
await fiber.dispose()
|
||||
|
||||
@@ -107,7 +107,9 @@ export abstract class SubprocessService extends Service {
|
||||
/**
|
||||
* Resolve one configured executable in this provider's execution world.
|
||||
* Absolute paths are verified; bare names use the provider's scrubbed PATH
|
||||
* plus explicit environment overrides.
|
||||
* plus explicit environment overrides. Relative paths containing separators
|
||||
* are rejected: no current consumer defines which directory they would
|
||||
* resolve against, so providers fail loud instead of guessing.
|
||||
* @param command - absolute executable path or bare PATH name.
|
||||
* @param env - explicit environment entries used for lookup.
|
||||
* @param signal - aborts remote or local lookup.
|
||||
|
||||
@@ -193,7 +193,11 @@ export interface SubprocessHandle {
|
||||
waitForExit(signal?: AbortSignal): Promise<boolean>
|
||||
}
|
||||
|
||||
/** Signals supported by the terminal-process primitive. */
|
||||
/**
|
||||
* Signals supported by the terminal-process primitive. Kept member-identical
|
||||
* to `PtySignal` in `@deepseek-ai/dsh-pty` without a cross-seam dependency;
|
||||
* change both together.
|
||||
*/
|
||||
export type SubprocessTerminalSignal = 'SIGINT' | 'SIGTERM' | 'SIGKILL' | 'SIGTSTP' | 'SIGHUP'
|
||||
|
||||
/** A fully specified terminal-process spawn. */
|
||||
|
||||
Reference in New Issue
Block a user