fix(tmux-context): contain executor rejection as a warning, correct suppression claim
Review round on #758. bash.run() only promises to resolve for nonzero exits, timeouts, and aborts, and bash.resolve() can reject on policy grounds, so either could escape the serial agent/step listener and abort the model turn — contradicting the plugin's documented failed-query no-op contract. Contain both and log a warning instead; the location is optional context. The Agent Note claimed an unchanged location suppresses the query. It does not: only the interval floor is checked before the query, while change suppression compares state the query returned. Corrected in both languages and re-recorded the i18n pairs.
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/context/tmux-context/README.md
|
||||
README.md: 5ea36948d6d83135c5aa97650c0d77e942adbbaa
|
||||
README.zh.md: 914d8d7c99c37de2c64541bcf4968996d819077d
|
||||
README.md: a166a46d20f472cb5d8f045e2456ce3e6de7a2f2
|
||||
README.zh.md: 0575d549e352239e7d954870eaf40beea1169cc6
|
||||
|
||||
@@ -27,7 +27,7 @@ pane_tty=$(tmux display-message -t "$TMUX_PANE" -p '#{pane_tty}') || exit 1
|
||||
exec tmux display-message -t "$TMUX_PANE" -p '<format>'
|
||||
```
|
||||
|
||||
`$TMUX_PANE` alone is insufficient: a terminal launched from a tmux shell (a VS Code integrated terminal, a desktop launcher) **inherits** `$TMUX` and `$TMUX_PANE` from that ancestor, so the variables are present even though the process does not live in that pane. The command therefore also compares the pane's `#{pane_tty}` against this process's own controlling terminal (`ps -o tty=` for its pid): a genuine pane owns this process's tty, while an inherited environment names some other pane's tty. Running through `ctx.bash` applies the deployment's sandbox and policy; the plugin owns no subprocess code. When `ctx.bash` is absent, the process is not in a real tmux pane (`$TMUX_PANE` unset, or the tty does not match ⇒ nonzero exit), or the reading is malformed, the attempt is a no-op, never an error.
|
||||
`$TMUX_PANE` alone is insufficient: a terminal launched from a tmux shell (a VS Code integrated terminal, a desktop launcher) **inherits** `$TMUX` and `$TMUX_PANE` from that ancestor, so the variables are present even though the process does not live in that pane. The command therefore also compares the pane's `#{pane_tty}` against this process's own controlling terminal (`ps -o tty=` for its pid): a genuine pane owns this process's tty, while an inherited environment names some other pane's tty. Running through `ctx.bash` applies the deployment's sandbox and policy; the plugin owns no subprocess code. When `ctx.bash` is absent, the process is not in a real tmux pane (`$TMUX_PANE` unset, or the tty does not match ⇒ nonzero exit), or the reading is malformed, the attempt is a no-op, never an error. The location is optional, so an executor rejection — a policy refusal from `resolve()` or an infrastructure failure from `run()` — is contained and logged as a warning rather than failing the turn.
|
||||
|
||||
State is pulled on every eligible turn — a moved, renamed, or re-laid-out pane is picked up without any tmux hook or background process. The plugin re-injects only when the rendered tmux state differs from its last injection, so an unchanged location adds nothing.
|
||||
|
||||
|
||||
@@ -27,7 +27,7 @@ pane_tty=$(tmux display-message -t "$TMUX_PANE" -p '#{pane_tty}') || exit 1
|
||||
exec tmux display-message -t "$TMUX_PANE" -p '<format>'
|
||||
```
|
||||
|
||||
仅凭 `$TMUX_PANE` 并不足够:从 tmux shell 启动的终端(VS Code 集成终端、桌面启动器)会从该祖先进程**继承** `$TMUX` 与 `$TMUX_PANE`,因此即使进程并不位于那个 pane 中,这些变量依然存在。为此该命令还会把 pane 的 `#{pane_tty}` 与本进程自己的控制终端(对其 pid 执行 `ps -o tty=`)作比较:真正的 pane 拥有本进程的 tty,而继承而来的环境指向的是另一个 pane 的 tty。通过 `ctx.bash` 运行会应用部署方的沙箱与策略;插件不拥有任何子进程代码。当 `ctx.bash` 缺失、进程不在真实的 tmux pane 内(`$TMUX_PANE` 未设置,或 tty 不匹配 ⇒ 非零退出)或读取结果格式非法时,本次尝试为空操作,绝不报错。
|
||||
仅凭 `$TMUX_PANE` 并不足够:从 tmux shell 启动的终端(VS Code 集成终端、桌面启动器)会从该祖先进程**继承** `$TMUX` 与 `$TMUX_PANE`,因此即使进程并不位于那个 pane 中,这些变量依然存在。为此该命令还会把 pane 的 `#{pane_tty}` 与本进程自己的控制终端(对其 pid 执行 `ps -o tty=`)作比较:真正的 pane 拥有本进程的 tty,而继承而来的环境指向的是另一个 pane 的 tty。通过 `ctx.bash` 运行会应用部署方的沙箱与策略;插件不拥有任何子进程代码。当 `ctx.bash` 缺失、进程不在真实的 tmux pane 内(`$TMUX_PANE` 未设置,或 tty 不匹配 ⇒ 非零退出)或读取结果格式非法时,本次尝试为空操作,绝不报错。由于位置信息是可选的,执行器的拒绝——`resolve()` 的策略拒绝或 `run()` 的基础设施故障——会被兜住并记录为警告,而不会使该轮失败。
|
||||
|
||||
状态在每个符合条件的轮次拉取——pane 被移动、改名或重新布局都会被感知,无需任何 tmux hook 或后台进程。插件仅在渲染出的 tmux 状态与上次注入不同时才重新注入,因此位置不变时不会新增任何内容。
|
||||
|
||||
|
||||
@@ -12,15 +12,16 @@
|
||||
* only when the rendered tmux state changes since the last injection (a moved,
|
||||
* renamed, or re-laid-out pane), with an optional `refreshIntervalMs` floor
|
||||
* between injections. Absent tmux environment, an inherited-only environment,
|
||||
* absent `ctx.bash`, or a failed query is a no-op, never an error.
|
||||
* absent `ctx.bash`, or a failed query is a no-op, never an error: an executor
|
||||
* rejection is contained and logged as a warning so the turn continues.
|
||||
*
|
||||
* @module @deepseek-ai/dsh-tmux-context
|
||||
*/
|
||||
|
||||
import type { Context } from 'cordis'
|
||||
import type { Context, LoggerService } from 'cordis'
|
||||
import z from 'schemastery'
|
||||
import type { Agent } from '@deepseek-ai/dsh-agent'
|
||||
import type { BashExecutor } from '@deepseek-ai/dsh-bash'
|
||||
import type { BashExecutor, BashRunResult } from '@deepseek-ai/dsh-bash'
|
||||
import { createUserMessage } from '@deepseek-ai/dsh-llm'
|
||||
|
||||
/** Cordis plugin name used by loader diagnostics. */
|
||||
@@ -92,13 +93,20 @@ const FIELD_SEP = '\\t'
|
||||
* on a match, so an inherited environment reads as "not in tmux" and injects
|
||||
* nothing.
|
||||
*
|
||||
* The location is optional context, so an executor rejection is a failed query,
|
||||
* not a turn failure: `resolve()` may reject the command on policy grounds and
|
||||
* `run()` only promises to resolve for nonzero exits, timeouts, and aborts, so
|
||||
* both are contained and reported as a warning.
|
||||
*
|
||||
* @param bash - the executor seam used to run the read-only tmux/ps commands.
|
||||
* @param logger - receives a warning when the executor rejects the query.
|
||||
* @param processId - this agent process's pid, whose controlling tty must match the pane.
|
||||
* @param signal - abort signal forwarded to the executor.
|
||||
* @returns the parsed location, or `undefined` when not in a real pane or on any failure.
|
||||
*/
|
||||
async function queryTmuxLocation(
|
||||
bash: BashExecutor,
|
||||
logger: LoggerService,
|
||||
processId: number,
|
||||
signal: AbortSignal,
|
||||
): Promise<TmuxLocation | undefined> {
|
||||
@@ -111,8 +119,14 @@ async function queryTmuxLocation(
|
||||
'[ "$pane_tty" = "/dev/$self_tty" ] || exit 1',
|
||||
`exec tmux display-message -t "$TMUX_PANE" -p '${format}'`,
|
||||
].join('\n')
|
||||
const spec = bash.resolve({ command, signal })
|
||||
const result = await bash.run(spec)
|
||||
let result: BashRunResult
|
||||
try {
|
||||
result = await bash.run(bash.resolve({ command, signal }))
|
||||
} catch (error: unknown) {
|
||||
const message = error instanceof Error ? error.message : String(error)
|
||||
logger.warn(`tmux location query failed: ${message}; injecting no location this turn`)
|
||||
return undefined
|
||||
}
|
||||
if (result.exitCode !== 0) return undefined
|
||||
const line = result.stdout.text.split('\n', 1)[0] as string
|
||||
const parts = line.split(FIELD_SEP)
|
||||
@@ -215,7 +229,7 @@ export function apply(ctx: Context, config: Config): void {
|
||||
const now = Date.now()
|
||||
if (now >= previous.time && now - previous.time < refreshIntervalMs) return
|
||||
}
|
||||
const location = await queryTmuxLocation(bash, process.pid, signal)
|
||||
const location = await queryTmuxLocation(bash, ctx.logger, process.pid, signal)
|
||||
if (location === undefined) return
|
||||
const state = renderState(location)
|
||||
if (previous !== undefined && previous.state === state) return
|
||||
|
||||
@@ -51,8 +51,10 @@ class FakeBash extends BashExecutor {
|
||||
commands: string[] = []
|
||||
result: BashRunResult = runResult(`${tmuxLine()}\n`)
|
||||
runError?: Error
|
||||
resolveError?: Error
|
||||
|
||||
override resolve(request: BashExecRequest): BashExecSpec {
|
||||
if (this.resolveError) throw this.resolveError
|
||||
return {
|
||||
command: request.command,
|
||||
workdir: request.workdir ?? '/work',
|
||||
@@ -325,6 +327,46 @@ describe('tmux-context no-op paths', () => {
|
||||
expect(contextTexts(session)).toHaveLength(0)
|
||||
})
|
||||
|
||||
it('warns and injects nothing when the executor rejects the run', async () => {
|
||||
const { ctx, bash } = await mount({}, true)
|
||||
bash.runError = new Error('bash executor unavailable')
|
||||
const warn = vi.spyOn(ctx.logger, 'warn')
|
||||
const session = new Session(SessionId('run-rejected'))
|
||||
openMessageTurn(session, 1)
|
||||
|
||||
await fire(ctx, sessionAgent(session), 1, 1)
|
||||
|
||||
expect(contextTexts(session)).toHaveLength(0)
|
||||
expect(warn).toHaveBeenCalledWith(expect.stringContaining('bash executor unavailable'))
|
||||
})
|
||||
|
||||
it('warns and injects nothing when the executor rejects the command at resolve', async () => {
|
||||
const { ctx, bash } = await mount({}, true)
|
||||
bash.resolveError = new Error('command denied by policy')
|
||||
const warn = vi.spyOn(ctx.logger, 'warn')
|
||||
const session = new Session(SessionId('resolve-rejected'))
|
||||
openMessageTurn(session, 1)
|
||||
|
||||
await fire(ctx, sessionAgent(session), 1, 1)
|
||||
|
||||
expect(contextTexts(session)).toHaveLength(0)
|
||||
expect(warn).toHaveBeenCalledWith(expect.stringContaining('command denied by policy'))
|
||||
})
|
||||
|
||||
it('reports a non-Error rejection in the warning', async () => {
|
||||
const { ctx, bash } = await mount({}, true)
|
||||
// Non-Error throw: the executor seam is typed, but a bad impl can reject with anything.
|
||||
bash.runError = 'spawn refused' as unknown as Error
|
||||
const warn = vi.spyOn(ctx.logger, 'warn')
|
||||
const session = new Session(SessionId('non-error-rejection'))
|
||||
openMessageTurn(session, 1)
|
||||
|
||||
await fire(ctx, sessionAgent(session), 1, 1)
|
||||
|
||||
expect(contextTexts(session)).toHaveLength(0)
|
||||
expect(warn).toHaveBeenCalledWith(expect.stringContaining('spawn refused'))
|
||||
})
|
||||
|
||||
it('skips an already-aborted step and runs before ordinary agent/step listeners', async () => {
|
||||
const { ctx } = await mount({}, true)
|
||||
const session = new Session(SessionId('ordering'))
|
||||
|
||||
Reference in New Issue
Block a user