Skip to content

Commit 3520e90

Browse files
committed
feat(pty): surface notify-on-exit guidance
1 parent cf716c7 commit 3520e90

File tree

8 files changed

+133
-30
lines changed

8 files changed

+133
-30
lines changed

src/plugin/pty/session-lifecycle.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,7 @@ export class SessionLifecycleManager {
150150
args: session.args,
151151
workdir: session.workdir,
152152
status: session.status,
153+
notifyOnExit: session.notifyOnExit,
153154
exitCode: session.exitCode,
154155
exitSignal: session.exitSignal,
155156
pid: session.pid,

src/plugin/pty/tools/read.ts

Lines changed: 58 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,16 @@ import { formatLine } from '../formatters.ts'
66
import type { PTYSessionInfo } from '../types.ts'
77
import DESCRIPTION from './read.txt'
88

9+
const NOTIFY_ON_EXIT_REMINDER = [
10+
`<system_reminder>`,
11+
`This session was started with \`notifyOnExit=true\`.`,
12+
`Completion signal is the future \`<pty_exited>\` message, not repeated \`pty_read\` calls.`,
13+
`If you only need to know whether the command finished, stop polling and wait for \`<pty_exited>\`.`,
14+
`Do not use sleep plus \`pty_read\` loops to check completion.`,
15+
`Use \`pty_read\` only when you need live output now, the user explicitly asks for logs, or the exit notification reports a non-zero status and you need to investigate.`,
16+
`</system_reminder>`,
17+
].join('\n')
18+
919
interface ReadArgs {
1020
id: string
1121
offset?: number
@@ -36,6 +46,14 @@ function formatPtyOutput(
3646
return output.join('\n')
3747
}
3848

49+
function appendNotifyOnExitReminder(output: string, session: PTYSessionInfo): string {
50+
if (!session.notifyOnExit || session.status !== 'running') {
51+
return output
52+
}
53+
54+
return `${output}\n\n${NOTIFY_ON_EXIT_REMINDER}`
55+
}
56+
3957
/**
4058
* Validates and creates a RegExp from pattern string
4159
*/
@@ -73,12 +91,15 @@ function handlePatternRead(
7391
}
7492

7593
if (result.matches.length === 0) {
76-
return [
77-
`<pty_output id="${id}" status="${session.status}" pattern="${pattern}">`,
78-
`No lines matched the pattern '${pattern}'.`,
79-
`Total lines in buffer: ${result.totalLines}`,
80-
`</pty_output>`,
81-
].join('\n')
94+
return appendNotifyOnExitReminder(
95+
[
96+
`<pty_output id="${id}" status="${session.status}" pattern="${pattern}">`,
97+
`No lines matched the pattern '${pattern}'.`,
98+
`Total lines in buffer: ${result.totalLines}`,
99+
`</pty_output>`,
100+
].join('\n'),
101+
session
102+
)
82103
}
83104

84105
const formattedLines = result.matches.map((match) =>
@@ -88,14 +109,17 @@ function handlePatternRead(
88109
const paginationMessage = `(${result.matches.length} of ${result.totalMatches} matches shown. Use offset=${offset + result.matches.length} to see more.)`
89110
const endMessage = `(${result.totalMatches} match${result.totalMatches === 1 ? '' : 'es'} from ${result.totalLines} total lines)`
90111

91-
return formatPtyOutput(
92-
id,
93-
session.status,
94-
pattern,
95-
formattedLines,
96-
result.hasMore,
97-
paginationMessage,
98-
endMessage
112+
return appendNotifyOnExitReminder(
113+
formatPtyOutput(
114+
id,
115+
session.status,
116+
pattern,
117+
formattedLines,
118+
result.hasMore,
119+
paginationMessage,
120+
endMessage
121+
),
122+
session
99123
)
100124
}
101125

@@ -114,12 +138,15 @@ function handlePlainRead(
114138
}
115139

116140
if (result.lines.length === 0) {
117-
return [
118-
`<pty_output id="${args.id}" status="${session.status}">`,
119-
`(No output available - buffer is empty)`,
120-
`Total lines: ${result.totalLines}`,
121-
`</pty_output>`,
122-
].join('\n')
141+
return appendNotifyOnExitReminder(
142+
[
143+
`<pty_output id="${args.id}" status="${session.status}">`,
144+
`(No output available - buffer is empty)`,
145+
`Total lines: ${result.totalLines}`,
146+
`</pty_output>`,
147+
].join('\n'),
148+
session
149+
)
123150
}
124151

125152
const formattedLines = result.lines.map((line, index) =>
@@ -129,14 +156,17 @@ function handlePlainRead(
129156
const paginationMessage = `(Buffer has more lines. Use offset=${result.offset + result.lines.length} to read beyond line ${result.offset + result.lines.length})`
130157
const endMessage = `(End of buffer - total ${result.totalLines} lines)`
131158

132-
return formatPtyOutput(
133-
args.id,
134-
session.status,
135-
undefined,
136-
formattedLines,
137-
result.hasMore,
138-
paginationMessage,
139-
endMessage
159+
return appendNotifyOnExitReminder(
160+
formatPtyOutput(
161+
args.id,
162+
session.status,
163+
undefined,
164+
formattedLines,
165+
result.hasMore,
166+
paginationMessage,
167+
endMessage
168+
),
169+
session
140170
)
141171
}
142172

src/plugin/pty/tools/read.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ Tips:
2828
- To tail recent output, calculate offset as (totalLines - N) where N is how many recent lines you want
2929
- Lines longer than 2000 characters are truncated
3030
- Empty output may mean the process hasn't produced output yet
31+
- If the session was started with `notifyOnExit=true`, do not use repeated `pty_read` calls only to detect completion; wait for the future `<pty_exited>` message instead
3132

3233
Examples:
3334
- Read first 100 lines: offset=0, limit=100

src/plugin/pty/tools/spawn.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,15 @@ import { manager } from '../manager.ts'
33
import { checkCommandPermission, checkWorkdirPermission } from '../permissions.ts'
44
import DESCRIPTION from './spawn.txt'
55

6+
const NOTIFY_ON_EXIT_INSTRUCTIONS = [
7+
`<system_reminder>`,
8+
`Completion signal for this session is the future \`<pty_exited>\` message.`,
9+
`If you only need to know whether the command finished, do not call \`pty_read\`; wait for \`<pty_exited>\`.`,
10+
`Never use sleep plus \`pty_read\` loops to check completion for this session.`,
11+
`Call \`pty_read\` before exit only if you need live output now, the user explicitly asks for logs, or the exit notification reports a non-zero status and you need to investigate.`,
12+
`</system_reminder>`,
13+
].join('\n')
14+
615
export const ptySpawn = tool({
716
description: DESCRIPTION,
817
args: {
@@ -52,7 +61,9 @@ export const ptySpawn = tool({
5261
`Workdir: ${info.workdir}`,
5362
`PID: ${info.pid}`,
5463
`Status: ${info.status}`,
64+
`NotifyOnExit: ${info.notifyOnExit}`,
5565
`</pty_spawned>`,
66+
...(info.notifyOnExit ? ['', NOTIFY_ON_EXIT_INSTRUCTIONS] : []),
5667
].join('\n')
5768

5869
return output

src/plugin/pty/tools/spawn.txt

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,11 @@ When `notifyOnExit` is true, you will receive a message when the process exits c
3434
- Last line of output (truncated to 250 chars)
3535

3636
This is useful for long-running processes where you want to be notified when they complete
37-
instead of polling with `pty_read`. If the process fails (non-zero exit code), the notification
38-
will suggest using `pty_read` with the `pattern` parameter to search for errors.
37+
instead of polling with `pty_read`.
38+
- Completion signal is the future `<pty_exited>` message
39+
- If you only need to know whether the command finished, do not call `pty_read`; wait for `<pty_exited>`
40+
- Never use sleep plus `pty_read` loops to check completion
41+
- Use `pty_read` before exit only if you need live output now, the user explicitly asks for logs, or the exit notification reports a non-zero status and you need to investigate
3942

4043
Examples:
4144
- Start a dev server: command="npm", args=["run", "dev"], title="Dev Server"

src/plugin/pty/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ export interface PTYSessionInfo {
3131
args: string[]
3232
workdir: string
3333
status: PTYStatus
34+
notifyOnExit: boolean
3435
exitCode?: number
3536
exitSignal?: number | string
3637
pid: number

test/pty-tools.test.ts

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ describe('PTY Tools', () => {
1919
workdir: opts.workdir || '/tmp',
2020
pid: 12345,
2121
status: 'running',
22+
notifyOnExit: opts.notifyOnExit ?? false,
2223
createdAt: new Date().toISOString(),
2324
lineCount: 0,
2425
}))
@@ -58,7 +59,9 @@ describe('PTY Tools', () => {
5859
expect(result).toContain('<pty_spawned>')
5960
expect(result).toContain('ID: test-session-id')
6061
expect(result).toContain('Command: echo hello')
62+
expect(result).toContain('NotifyOnExit: false')
6163
expect(result).toContain('</pty_spawned>')
64+
expect(result).not.toContain('<system_reminder>')
6265
})
6366

6467
it('should spawn with all optional args', async () => {
@@ -101,6 +104,14 @@ describe('PTY Tools', () => {
101104
expect(result).toContain('Command: node script.js')
102105
expect(result).toContain('PID: 12345')
103106
expect(result).toContain('Status: running')
107+
expect(result).toContain('NotifyOnExit: true')
108+
expect(result).toContain('<system_reminder>')
109+
expect(result).toContain('Completion signal for this session is the future `<pty_exited>` message.')
110+
expect(result).toContain(
111+
'If you only need to know whether the command finished, do not call `pty_read`; wait for `<pty_exited>`.'
112+
)
113+
expect(result).toContain('Never use sleep plus `pty_read` loops to check completion for this session.')
114+
expect(result).toContain('</system_reminder>')
104115
})
105116
})
106117

@@ -114,6 +125,7 @@ describe('PTY Tools', () => {
114125
args: ['hello'],
115126
workdir: '/tmp',
116127
status: 'running',
128+
notifyOnExit: false,
117129
pid: 12345,
118130
createdAt: new Date().toISOString(),
119131
lineCount: 2,
@@ -157,6 +169,46 @@ describe('PTY Tools', () => {
157169
expect(result).toContain('</pty_output>')
158170
})
159171

172+
it('should include notifyOnExit reminder for running sessions', async () => {
173+
spyOn(manager, 'get').mockReturnValue({
174+
id: 'test-session-id',
175+
title: 'Test Session',
176+
description: 'A session for testing',
177+
command: 'echo',
178+
args: ['hello'],
179+
workdir: '/tmp',
180+
status: 'running',
181+
notifyOnExit: true,
182+
pid: 12345,
183+
createdAt: new Date().toISOString(),
184+
lineCount: 2,
185+
})
186+
187+
const args = { id: 'test-session-id' }
188+
const ctx = {
189+
sessionID: 'parent',
190+
messageID: 'msg',
191+
agent: 'agent',
192+
abort: new AbortController().signal,
193+
metadata: () => {},
194+
ask: async () => {},
195+
directory: '/tmp',
196+
worktree: '/tmp',
197+
}
198+
199+
const result = await ptyRead.execute(args, ctx)
200+
201+
expect(result).toContain('<system_reminder>')
202+
expect(result).toContain('This session was started with `notifyOnExit=true`.')
203+
expect(result).toContain(
204+
'Completion signal is the future `<pty_exited>` message, not repeated `pty_read` calls.'
205+
)
206+
expect(result).toContain(
207+
'If you only need to know whether the command finished, stop polling and wait for `<pty_exited>`.'
208+
)
209+
expect(result).toContain('</system_reminder>')
210+
})
211+
160212
it('should read with pattern', async () => {
161213
const args = { id: 'test-session-id', pattern: 'line' }
162214
const ctx = {
@@ -224,6 +276,7 @@ describe('PTY Tools', () => {
224276
command: 'echo',
225277
args: ['hello'],
226278
status: 'running' as const,
279+
notifyOnExit: false,
227280
pid: 12345,
228281
lineCount: 10,
229282
workdir: '/tmp',

test/types.test.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ describe('Web Types', () => {
2626
title: 'Test Session',
2727
command: 'echo',
2828
status: 'running',
29+
notifyOnExit: false,
2930
pid: 1234,
3031
lineCount: 5,
3132
createdAt: new Date().toISOString(),
@@ -61,6 +62,7 @@ describe('Web Types', () => {
6162
title: 'Test Echo Session',
6263
command: 'echo',
6364
status: 'exited',
65+
notifyOnExit: true,
6466
exitCode: 0,
6567
pid: 1234,
6668
lineCount: 2,
@@ -85,6 +87,7 @@ describe('Web Types', () => {
8587
title: 'Running Session',
8688
command: 'sleep',
8789
status: 'running',
90+
notifyOnExit: false,
8891
pid: 5678,
8992
lineCount: 0,
9093
createdAt: new Date('2026-01-21T10:00:00.000Z').toISOString(),

0 commit comments

Comments
 (0)