fix(pty): make the foreground-handoff grace a config field
The local PTY readiness poll held its inferred_idle fallback for exactly one pollIntervalMs after a prompt marker, so a bash foreground handoff that lands on the silence boundary only wins the exact stdin_read attribution when the kernel publishes it inside that single poll. On a slow or loaded host it does not, and the attribution flips. handoffGraceMs replaces the hardcoded one-poll window as a validated, deployment-owned config field defaulting to 500ms, rejected at load when it cannot contain one readiness poll. Real-shell tests that interrupt a send now assert the session is usable again rather than which readiness tier observed the handoff, because no fixed grace removes the race.
This commit is contained in:
@@ -6,7 +6,7 @@ function config(overrides: Partial<Config> = {}): Config {
|
||||
return {
|
||||
backendType: 'shell', shellPath: '/bin/bash', shellArgs: [], rows: 40, cols: 160,
|
||||
scrollbackLines: 100, scrollbackMaxBytes: 1024, maxReadBytes: 512,
|
||||
pollIntervalMs: 10, exactProbeAfterMs: 20, idleSilenceMs: 100, timeoutMs: 1000,
|
||||
pollIntervalMs: 10, exactProbeAfterMs: 20, idleSilenceMs: 100, handoffGraceMs: 50, timeoutMs: 1000,
|
||||
disposeGraceMs: 100,
|
||||
...overrides,
|
||||
}
|
||||
@@ -24,4 +24,9 @@ describe('pty-local config', () => {
|
||||
expect(() => { validateConfig(config({ rows: 1.5 })) }).toThrow('rows')
|
||||
expect(() => { validateConfig(config({ maxReadBytes: 2048 })) }).toThrow('must not exceed')
|
||||
})
|
||||
|
||||
it('rejects a handoff grace shorter than one readiness poll', () => {
|
||||
expect(() => { validateConfig(config({ handoffGraceMs: 9, pollIntervalMs: 10 })) }).toThrow('handoffGraceMs must be at least pollIntervalMs')
|
||||
expect(() => { validateConfig(config({ handoffGraceMs: 10, pollIntervalMs: 10 })) }).not.toThrow()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -33,7 +33,7 @@ function config(): ResolvedConfig {
|
||||
return {
|
||||
backendType: 'shell', shellPath: '/bin/bash', shellArgs: [], rows: 24, cols: 80,
|
||||
scrollbackLines: 10, scrollbackMaxBytes: 100, maxReadBytes: 50,
|
||||
pollIntervalMs: 10, exactProbeAfterMs: 20, idleSilenceMs: 50, timeoutMs: 100,
|
||||
pollIntervalMs: 10, exactProbeAfterMs: 20, idleSilenceMs: 50, handoffGraceMs: 10, timeoutMs: 100,
|
||||
disposeGraceMs: 10,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -52,6 +52,7 @@ async function harness(mode: 'danger-full-access' | 'workspace-write') {
|
||||
pollIntervalMs: 10,
|
||||
exactProbeAfterMs: 20,
|
||||
idleSilenceMs: 250,
|
||||
handoffGraceMs: 250,
|
||||
timeoutMs: 2000,
|
||||
disposeGraceMs: 500,
|
||||
scrollbackLines: 100,
|
||||
@@ -73,6 +74,15 @@ async function waitForOutput(operation: PtySendOperation, expected: string): Pro
|
||||
expect(output).toContain(expected)
|
||||
}
|
||||
|
||||
// A send the test interrupts settles when bash returns to its prompt, so the
|
||||
// kernel may publish the foreground handoff on either side of the silence
|
||||
// bound. `handoffGraceMs` widens the window that wins the exact attribution but
|
||||
// cannot remove the race on a loaded host, so these settles assert that the
|
||||
// session became usable again, not which readiness tier observed it.
|
||||
function expectReadyForNextSend(waitReason: string): void {
|
||||
expect(['stdin_read', 'inferred_idle']).toContain(waitReason)
|
||||
}
|
||||
|
||||
describe('pty-local real shell', () => {
|
||||
it('persists cwd and environment across sends, scrubs secrets, and closes', async () => {
|
||||
const previous = process.env.DSH_TEST_SECRET
|
||||
@@ -116,7 +126,7 @@ describe('pty-local real shell', () => {
|
||||
const foreground = ctx.pty.startSend(agent, created.sessionId, { text: 'sleep 60', submit: true })
|
||||
await new Promise(resolve => setTimeout(resolve, 50))
|
||||
expect((await ctx.pty.signal(agent, created.sessionId, 'SIGINT')).delivered).toBe(true)
|
||||
expect((await foreground.done).waitReason).toBe('stdin_read')
|
||||
expectReadyForNextSend((await foreground.done).waitReason)
|
||||
|
||||
const background = ctx.pty.startSend(agent, created.sessionId, {
|
||||
text: 'sh -c \'trap "" TERM; sleep 60\' & echo CHILD=$!',
|
||||
@@ -143,13 +153,13 @@ describe('pty-local real shell', () => {
|
||||
await waitForOutput(foreground, 'RAW_READY')
|
||||
controller.abort()
|
||||
const result = await foreground.done
|
||||
expect(result.waitReason).toBe('stdin_read')
|
||||
expectReadyForNextSend(result.waitReason)
|
||||
const after = await ctx.pty.startSend(agent, created.sessionId, {
|
||||
text: 'echo AFTER_SIGINT',
|
||||
submit: true,
|
||||
}).done
|
||||
expect(after.viewport).toContain('AFTER_SIGINT')
|
||||
expect(after.waitReason).toBe('stdin_read')
|
||||
expectReadyForNextSend(after.waitReason)
|
||||
await ctx.pty.kill(agent, created.sessionId)
|
||||
}, 10_000)
|
||||
})
|
||||
|
||||
@@ -88,7 +88,7 @@ function config(overrides: Partial<ResolvedConfig> = {}): ResolvedConfig {
|
||||
return {
|
||||
backendType: 'shell', shellPath: '/bin/bash', shellArgs: [], rows: 24, cols: 80,
|
||||
scrollbackLines: 10, scrollbackMaxBytes: 128, maxReadBytes: 64,
|
||||
pollIntervalMs: 10, exactProbeAfterMs: 20, idleSilenceMs: 50, timeoutMs: 100,
|
||||
pollIntervalMs: 10, exactProbeAfterMs: 20, idleSilenceMs: 50, handoffGraceMs: 10, timeoutMs: 100,
|
||||
disposeGraceMs: 20,
|
||||
...overrides,
|
||||
}
|
||||
@@ -307,6 +307,27 @@ describe('LocalPtySession readiness and output', () => {
|
||||
expect((await operation.done).waitReason).toBe('stdin_read')
|
||||
})
|
||||
|
||||
it('holds the idle fallback for the configured handoff grace, not one poll', async () => {
|
||||
vi.useFakeTimers()
|
||||
const terminal = new FakeTerminal()
|
||||
const inspector = new FakeInspector()
|
||||
const session = new LocalPtySession(terminal.asPty(), inspector, config({ handoffGraceMs: 40 }))
|
||||
await initialize(session, terminal)
|
||||
|
||||
const operation = session.startSend({ text: 'run', submit: true })
|
||||
let settled = false
|
||||
void operation.done.then(() => { settled = true })
|
||||
inspector.pgid = 789
|
||||
terminal.emitData('\x1b]133;D;0\x07dsh> ')
|
||||
// One poll past the silence bound would already have settled inferred_idle.
|
||||
await vi.advanceTimersByTimeAsync(70)
|
||||
expect(settled).toBe(false)
|
||||
|
||||
inspector.pgid = 456
|
||||
await vi.advanceTimersByTimeAsync(10)
|
||||
expect((await operation.done).waitReason).toBe('stdin_read')
|
||||
})
|
||||
|
||||
it('falls back to inferred idle when a foreground child emits an inherited prompt marker', async () => {
|
||||
vi.useFakeTimers()
|
||||
const terminal = new FakeTerminal()
|
||||
|
||||
Reference in New Issue
Block a user