Skip to content

Commit e7e0ae4

Browse files
authored
Merge pull request #413 from code-yeongyu/fix/monitor-wait-guidance
fix(coding-agent): route waits to the monitor tool — kill the tmux contradiction, teach the bash surface, dedup guidance
2 parents e2581c5 + 0d11c37 commit e7e0ae4

9 files changed

Lines changed: 138 additions & 28 deletions

File tree

packages/coding-agent/src/core/extensions/builtin/bash-timeout/timeout.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,5 +42,5 @@ export function applyBashTimeout<TInput extends BashToolInputLike>(
4242

4343
export function buildBashTimeoutPrompt(defaults: BashTimeoutDefaults): string {
4444
const minutes = (seconds: number): string => (seconds % 60 === 0 ? `${seconds / 60} min` : `${seconds}s`);
45-
return `\n## Bash Tool Timeout Policy\n\nThe \`bash\` tool enforces timeouts even when you omit the \`timeout\` parameter:\n\n- Default timeout: ${defaults.defaultSeconds}s (${minutes(defaults.defaultSeconds)}). Applied automatically when you do not set \`timeout\`.\n- Recommended maximum timeout: ${defaults.maxSeconds}s (${minutes(defaults.maxSeconds)}). Explicit \`timeout\` values are preserved because different hosts may use different timeout units.\n- For long-running commands (builds, installs, test suites), set an explicit \`timeout\` that fits the workload. Do not assume commands run forever.\n- For commands that legitimately need to run beyond the recommended maximum, run them in the background via tmux or a similar mechanism instead of relying on bash timeout.\n`;
45+
return `\n## Bash Tool Timeout Policy\n\nThe \`bash\` tool enforces timeouts even when you omit the \`timeout\` parameter:\n\n- Default timeout: ${defaults.defaultSeconds}s (${minutes(defaults.defaultSeconds)}). Applied automatically when you do not set \`timeout\`.\n- Recommended maximum timeout: ${defaults.maxSeconds}s (${minutes(defaults.maxSeconds)}). Explicit \`timeout\` values are preserved because different hosts may use different timeout units.\n- For long-running commands (builds, installs, test suites), set an explicit \`timeout\` that fits the workload. Do not assume commands run forever.\n- For commands that legitimately need to run beyond the recommended maximum, start them with \`run_in_background: true\` and watch the decisive output with \`monitor\` instead of raising the timeout.\n`;
4646
}

packages/coding-agent/src/core/extensions/builtin/changes.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,17 @@
11
# Builtin extensions changes
22

3+
## bash-timeout: beyond-max routing to run_in_background + monitor (2026-07-28)
4+
5+
- `bash-timeout/timeout.ts` `buildBashTimeoutPrompt()`: the beyond-max bullet no longer teaches
6+
"run them in the background via tmux or a similar mechanism" — it now routes to
7+
`run_in_background: true` with the decisive output watched via `monitor`. The old advice
8+
directly contradicted TERMINAL_PROMPT_SECTION ("do NOT use tmux"), which is appended to the
9+
same system prompt immediately after this section (builtin #11#12), and contradictions
10+
destabilize instruction following more than missing detail.
11+
- `test/suite/bash-timeout-extension.test.ts`: the "references tmux as the escape hatch" pin is
12+
replaced by the new contract (run_in_background + monitor present, tmux absent).
13+
- Expected merge conflict zones: LOW — fork-owned `timeout.ts` prompt string and its test.
14+
315
## Remove the /sessions session-observer HUD (2026-07-26)
416

517
- Deleted the `session-observer/` builtin (11 files: `index`, `loader`, `overlay`, `overlay-format`, `scanner`, `text`, `transcript`, `transcript-entries`, `transcript-format`, `types`) and its three vitest suites (`session-observer-picker`, `session-observer-overlay`, `session-observer-scanner`).

packages/coding-agent/src/core/extensions/builtin/terminal/changes.md

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,39 @@
33
The persistent-terminal tool suite (`bash` swapped to PTY-backed + `bash_output`,
44
`kill_bash`, `bash_input`, `bash_resize`). Backed by `@earendil-works/pi-pty`.
55

6+
## Wait-discipline routing: bash surface redirect + guidance dedup (2026-07-28)
7+
8+
### What changed
9+
10+
- `tools/bash.ts`: the PTY `bash` tool description now carries the wait redirect — waiting on
11+
observable state (a build finishing, a server coming up, a log line) is never a sleep/poll
12+
loop; subscribe with the `monitor` tool instead. The bash surface is where a model actually
13+
types `sleep 30`, and cross-tool routing in the misused tool's description follows the same
14+
pattern as the grep→rg snippet rule (`test/bash-prompt-snippet.test.ts`). The upstream core
15+
bash (`src/core/tools/bash.ts`) is deliberately untouched: its toolset has no monitor, and
16+
guidance must never name a tool the toolset lacks.
17+
- `tools/monitor.ts`: promptGuidelines collapsed to the single when-to-use decision rule. The
18+
command-shaping sentence duplicated the TERMINAL_PROMPT_SECTION bullet near-verbatim; each
19+
aspect is now stated once (decision rule → Tool Guidelines; mechanics → terminal section;
20+
redirect → bash schema; long-run routing → bash-timeout policy).
21+
- `prompt.ts`: the monitor bullet dropped its embedded when-to-use sentence (kept as the monitor
22+
tool's guideline) and keeps the subscribe framing plus shaping/filtering/rearm mechanics.
23+
- `test/prompt-surface-stale-wait-idioms.test.ts`: the consistency gate now enumerates every
24+
registered terminal tool surface (description + promptSnippet + promptGuidelines of bash,
25+
bash_output, monitor, bash_input, bash_resize, kill_bash) plus the bash-timeout prompt
26+
section; new gates assert the bash description routes waits to monitor, and that no
27+
agent-facing terminal surface teaches tmux as the backgrounding mechanism — a tmux mention is
28+
allowed only when the negation targets tmux itself ("do NOT use tmux"), so "use tmux; never
29+
X" cannot slip through.
30+
- `test/suite/terminal-monitor-notify.test.ts`: the watcher-discipline case now asserts the
31+
routing rule at its owning surface (the monitor tool's promptGuidelines) instead of
32+
TERMINAL_PROMPT_SECTION, and the noise-control match is wrap-tolerant.
33+
34+
### Expected merge conflict zones on next upstream sync
35+
36+
- LOW: `tools/bash.ts` description string, `tools/monitor.ts` promptGuidelines, `prompt.ts`
37+
monitor bullet, gate-test surface list (all fork-owned).
38+
639
## Monitor flat schema + subscribe-not-poll prompt (2026-07-27)
740

841
### What changed

packages/coding-agent/src/core/extensions/builtin/terminal/prompt.ts

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,11 @@ manual \`&\` backgrounding — use the built-in session tools:
1414
\`view: "screen"\`. Completion arrives as a notification carrying the exit code and output
1515
tail — peeking is for steering, never for waiting.
1616
- \`monitor({ description, command, filter?, timeout_ms?, persistent? })\` subscribes you to a
17-
command: its stdout lines arrive as injected events while you keep working. Any wait on
18-
observable state (CI checks, builds, log patterns, deploys) is a monitor, never a foreground
19-
sleep/poll loop. Shape the command by notifications needed: exit-on-condition for one
20-
completion event; emit-per-occurrence (\`tail -f | grep --line-buffered\`, a polling loop
21-
inside the command) for a stream. Filter noise at the command source, stop with \`kill_bash\`,
22-
and use \`monitor({ action: "rearm", bash_id })\` only after a wake-budget pause.
17+
command: its stdout lines arrive as injected events while you keep working. Shape the command
18+
by notifications needed: exit-on-condition for one completion event; emit-per-occurrence
19+
(\`tail -f | grep --line-buffered\`, a polling loop inside the command) for a stream. Filter
20+
noise at the command source, stop with \`kill_bash\`, and use
21+
\`monitor({ action: "rearm", bash_id })\` only after a wake-budget pause.
2322
- \`bash_input({ bash_id, input, keys, submit })\` sends stdin or named keys (e.g.
2423
\`["ctrl+c"]\`, \`["enter"]\`) to steer a REPL or interrupt a process.
2524
- \`bash_resize({ bash_id, cols, rows })\` resizes the PTY so full-screen programs reflow.

packages/coding-agent/src/core/extensions/builtin/terminal/tools/bash.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,7 @@ export function createPtyBashTool(ctx: TerminalToolContext) {
285285
name: TERMINAL_BASH_TOOL,
286286
label: "bash",
287287
description:
288-
"Execute a shell command in a persistent PTY-backed session. Set run_in_background:true for long-lived or interactive sessions; steer them with bash_input, snapshot with bash_output, tear down with kill_bash. Foreground timeout is a kill deadline in seconds.",
288+
"Execute a shell command in a persistent PTY-backed session. Set run_in_background:true for long-lived or interactive sessions; steer them with bash_input, snapshot with bash_output, tear down with kill_bash. To wait on observable state (a build finishing, a server coming up, a log line), never run sleep or poll loops — subscribe with the monitor tool instead. Foreground timeout is a kill deadline in seconds.",
289289
promptSnippet: "Run shell commands; run_in_background:true for long-lived/interactive PTY sessions",
290290
promptGuidelines: ["Inspect PI_* environment variables for current model and session details."],
291291
parameters: ptyBashSchema,

packages/coding-agent/src/core/extensions/builtin/terminal/tools/monitor.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,6 @@ export function createMonitorTool(ctx: TerminalToolContext) {
115115
promptSnippet: "Subscribe to a command's stdout lines as injected events instead of polling",
116116
promptGuidelines: [
117117
"Waiting on observable state (CI checks, builds, log patterns, deploys) means a monitor, never a foreground sleep/poll loop.",
118-
"Shape the command by notifications needed: exit-on-condition for one completion event; emit-per-occurrence (`tail -f | grep --line-buffered`, a polling loop inside the command) for a stream. Filter noise at the source.",
119118
],
120119
parameters: monitorSchema,
121120
renderCall: renderMonitorCall,

packages/coding-agent/test/prompt-surface-stale-wait-idioms.test.ts

Lines changed: 66 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,21 @@ import { readFileSync } from "node:fs";
22
import { join } from "node:path";
33
import { fileURLToPath } from "node:url";
44
import { describe, expect, it } from "vitest";
5+
import {
6+
buildBashTimeoutPrompt,
7+
resolveBashTimeoutDefaults,
8+
} from "../src/core/extensions/builtin/bash-timeout/timeout.ts";
59
import { TERMINAL_PROMPT_SECTION } from "../src/core/extensions/builtin/terminal/prompt.ts";
10+
import { createPtyBashTool } from "../src/core/extensions/builtin/terminal/tools/bash.ts";
11+
import { createBashInputTool } from "../src/core/extensions/builtin/terminal/tools/bash-input.ts";
612
import {
713
BASH_OUTPUT_WAIT_REMOVED_GUIDANCE,
814
createBashOutputTool,
915
} from "../src/core/extensions/builtin/terminal/tools/bash-output.ts";
16+
import { createBashResizeTool } from "../src/core/extensions/builtin/terminal/tools/bash-resize.ts";
1017
import type { TerminalToolContext } from "../src/core/extensions/builtin/terminal/tools/context.ts";
18+
import { createKillBashTool } from "../src/core/extensions/builtin/terminal/tools/kill-bash.ts";
19+
import { createMonitorTool } from "../src/core/extensions/builtin/terminal/tools/monitor.ts";
1120

1221
/**
1322
* Consistency gate: no shipped senpi prompt surface may teach the removed
@@ -23,24 +32,60 @@ const CODING_AGENT_ROOT = join(REPO_ROOT, "packages", "coding-agent");
2332
/** Lines mentioning wait_for are allowed only when they are removal guidance. */
2433
const GHOST_GUIDANCE_MARKER = /removed|no longer/i;
2534

35+
/** Lines mentioning tmux are allowed only when the negation targets tmux itself. */
36+
const TMUX_NEGATION = /(do not|don't|never)[^.!?]*\btmux\b/i;
37+
2638
function surfaceLines(name: string, text: string): Array<{ name: string; line: number; text: string }> {
2739
return text.split("\n").map((line, index) => ({ name, line: index + 1, text: line }));
2840
}
2941

30-
function loadSurfaces(): Array<{ name: string; line: number; text: string }> {
31-
const stubCtx = {
42+
function stubTerminalCtx(): TerminalToolContext {
43+
return {
3244
manager: { get: () => undefined },
3345
cwd: process.cwd(),
3446
defaultCols: 120,
3547
defaultRows: 40,
3648
getEnv: () => process.env,
3749
} as unknown as TerminalToolContext;
38-
const bashOutput = createBashOutputTool(stubCtx);
50+
}
51+
52+
type PromptSurfaceTool = {
53+
name: string;
54+
description: string;
55+
promptSnippet?: string;
56+
promptGuidelines?: readonly string[];
57+
};
3958

59+
/** description + promptSnippet + promptGuidelines of every registered terminal tool. */
60+
function terminalToolSurfaces(): Array<{ name: string; line: number; text: string }> {
61+
const stubCtx = stubTerminalCtx();
62+
const tools: PromptSurfaceTool[] = [
63+
createPtyBashTool(stubCtx),
64+
createBashOutputTool(stubCtx),
65+
createMonitorTool(stubCtx),
66+
createBashInputTool(stubCtx),
67+
createBashResizeTool(stubCtx),
68+
createKillBashTool(stubCtx),
69+
];
70+
return tools.flatMap((tool) => [
71+
...surfaceLines(`${tool.name} description`, tool.description),
72+
...surfaceLines(`${tool.name} promptSnippet`, tool.promptSnippet ?? ""),
73+
...surfaceLines(`${tool.name} promptGuidelines`, (tool.promptGuidelines ?? []).join("\n")),
74+
]);
75+
}
76+
77+
/** The prompt surfaces the model actually sees (system-prompt sections + tool schemas). */
78+
function agentFacingSurfaces(): Array<{ name: string; line: number; text: string }> {
4079
return [
4180
...surfaceLines("terminal/prompt.ts TERMINAL_PROMPT_SECTION", TERMINAL_PROMPT_SECTION),
42-
...surfaceLines("bash_output tool description", bashOutput.description),
43-
...surfaceLines("bash_output promptSnippet", bashOutput.promptSnippet ?? ""),
81+
...surfaceLines("bash-timeout prompt section", buildBashTimeoutPrompt(resolveBashTimeoutDefaults({}))),
82+
...terminalToolSurfaces(),
83+
];
84+
}
85+
86+
function loadSurfaces(): Array<{ name: string; line: number; text: string }> {
87+
return [
88+
...agentFacingSurfaces(),
4489
...surfaceLines(
4590
"packages/coding-agent/docs/terminal-tools.md",
4691
readFileSync(join(CODING_AGENT_ROOT, "docs", "terminal-tools.md"), "utf8"),
@@ -69,15 +114,23 @@ describe("stale wait-idiom consistency gate", () => {
69114
expect(TERMINAL_PROMPT_SECTION).not.toContain("wait_for");
70115
});
71116

117+
it("the bash tool surface routes waits to the monitor tool", () => {
118+
const bash = createPtyBashTool(stubTerminalCtx());
119+
expect(bash.description).toContain("monitor");
120+
});
121+
122+
it("no agent-facing terminal surface teaches tmux as the backgrounding mechanism", () => {
123+
const violations = agentFacingSurfaces().filter(
124+
({ text }) => /tmux/i.test(text) && !TMUX_NEGATION.test(text),
125+
);
126+
expect(
127+
violations.map(({ name, line, text }) => `${name}:${line}: ${text.trim()}`),
128+
"tmux taught as a backgrounding/wait mechanism outside negative guidance",
129+
).toEqual([]);
130+
});
131+
72132
it("the bash_output tool surface no longer advertises blocking", () => {
73-
const stubCtx = {
74-
manager: { get: () => undefined },
75-
cwd: process.cwd(),
76-
defaultCols: 120,
77-
defaultRows: 40,
78-
getEnv: () => process.env,
79-
} as unknown as TerminalToolContext;
80-
const bashOutput = createBashOutputTool(stubCtx);
133+
const bashOutput = createBashOutputTool(stubTerminalCtx());
81134
expect(bashOutput.description.toLowerCase()).not.toContain("block until");
82135
expect(bashOutput.description).not.toContain("wait_for");
83136
expect(bashOutput.promptSnippet ?? "").not.toContain("wait_for");

packages/coding-agent/test/suite/bash-timeout-extension.test.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,9 +131,11 @@ describe("buildBashTimeoutPrompt", () => {
131131
expect(prompt).toMatch(/explicit `timeout`/i);
132132
});
133133

134-
it("references tmux as the escape hatch for very long workloads", () => {
134+
it("routes beyond-max workloads to background sessions and monitor, not tmux", () => {
135135
const prompt = buildBashTimeoutPrompt({ defaultSeconds: 120, maxSeconds: 600 });
136136

137-
expect(prompt).toContain("tmux");
137+
expect(prompt).toContain("run_in_background");
138+
expect(prompt).toContain("monitor");
139+
expect(prompt).not.toContain("tmux");
138140
});
139141
});

packages/coding-agent/test/suite/terminal-monitor-notify.test.ts

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import {
55
} from "../../src/core/extensions/builtin/terminal/monitor-notify.ts";
66
import type { MonitorEvent } from "../../src/core/extensions/builtin/terminal/monitor-registry.ts";
77
import { TERMINAL_PROMPT_SECTION } from "../../src/core/extensions/builtin/terminal/prompt.ts";
8+
import type { TerminalToolContext } from "../../src/core/extensions/builtin/terminal/tools/context.ts";
9+
import { createMonitorTool } from "../../src/core/extensions/builtin/terminal/tools/monitor.ts";
810

911
type SentMessage = { readonly content: string; readonly options: { deliverAs?: "steer" | "followUp" } | undefined };
1012

@@ -180,11 +182,21 @@ describe("terminal monitor event delivery", () => {
180182
}
181183
});
182184

183-
it("teaches watcher discipline in the terminal prompt", () => {
185+
it("teaches watcher discipline at the owning terminal surfaces", () => {
184186
expect(TERMINAL_PROMPT_SECTION).toContain("monitor");
185187
// Discipline is the routing rule (waits are monitors, not poll loops) plus noise control,
186-
// not any particular wording — assert the guidance, never a pinned sentence.
187-
expect(TERMINAL_PROMPT_SECTION).toMatch(/never a foreground\s+sleep\/poll loop/);
188-
expect(TERMINAL_PROMPT_SECTION).toMatch(/Filter noise at the command source/);
188+
// not any particular wording — assert each rule at the surface that owns it, never a
189+
// pinned sentence: the routing rule ships as the monitor tool's guideline, the noise
190+
// rule stays in the terminal section.
191+
const stubCtx = {
192+
manager: { get: () => undefined },
193+
cwd: process.cwd(),
194+
defaultCols: 120,
195+
defaultRows: 40,
196+
getEnv: () => process.env,
197+
} as unknown as TerminalToolContext;
198+
const guidelines = (createMonitorTool(stubCtx).promptGuidelines ?? []).join("\n");
199+
expect(guidelines).toMatch(/never a foreground\s+sleep\/poll loop/);
200+
expect(TERMINAL_PROMPT_SECTION).toMatch(/Filter\s+noise at the command source/);
189201
});
190202
});

0 commit comments

Comments
 (0)