fix(test): keep the wait needles out of the shells the skip cases spawn
main went red on the merge commit (run 34579312780) with a real assertion: `tests/agent-pty.spec.ts:413` expected `skipWait` to report 1 registered wait and got 0. The cause is the test's own preparation, not the registry: an interactive shell ECHOES what is written to it, so a terminal opened with a command that contains the wait needle already has that needle in its transcript by the time `waitFor` runs — `waitFor`'s documented fast path then resolves the wait immediately and `skipWait` finds nothing to skip. POSIX /bin/sh does not echo a command written to a non-tty, so this had been latent for as long as the suite existed; PowerShell does echo it, which is why it only ever failed on Windows, and why it took a Windows run to expose it rather than a code change. The three affected cases (skip / waiting-snapshot / wait-notification) now open a BARE shell and wait for the shell to be up rather than for a printed marker, via a new `waitForShellReady` helper that watches for output of any kind. That removes both halves of the hazard: no needle in the command, and no dependency on which words a given shell prints at startup. Nothing about what the cases assert changed — they are about the registry's wait bookkeeping, which needs a live terminal and no particular output. Verified: tests/agent-pty.spec.ts 33 passed (and the three cases now finish in milliseconds instead of waiting out a transcript poll), full suite 127 files / 1343 passed / 9 skipped.
This commit is contained in:
+38
-7
@@ -22,17 +22,37 @@ function testShell(): string {
|
||||
return process.platform === 'win32' ? 'powershell.exe' : '/bin/sh'
|
||||
}
|
||||
|
||||
/**
|
||||
* Wait for a freshly spawned shell to have produced ANY output.
|
||||
*
|
||||
* The alternative — waiting for a string the prepared command prints — ties
|
||||
* the test to shell wording, and waiting for a string the command CONTAINS is
|
||||
* worse: an interactive shell echoes what is written to it (PowerShell does,
|
||||
* POSIX /bin/sh does not), so the needle would already be in the transcript
|
||||
* before the point the test means to exercise. The suites that only need "the
|
||||
* shell is up" therefore wait for output of any kind.
|
||||
* @param registry - the registry under test.
|
||||
* @param uuid - the terminal to watch.
|
||||
* @returns nothing; the caller asserts straight after.
|
||||
*/
|
||||
async function waitForShellReady(registry: AgentPtyRegistry, uuid: string): Promise<void> {
|
||||
await waitForTranscript(registry, uuid, '', 0, true)
|
||||
}
|
||||
|
||||
/** Wait for a terminal's transcript to contain a substring (or timeout). */
|
||||
async function waitForTranscript(
|
||||
registry: AgentPtyRegistry,
|
||||
uuid: string,
|
||||
needle: string,
|
||||
timeoutMs = 15_000,
|
||||
anyOutput = false,
|
||||
): Promise<string> {
|
||||
const deadline = Date.now() + timeoutMs
|
||||
while (Date.now() < deadline) {
|
||||
const handle = registry.get(uuid)
|
||||
if (handle !== undefined && handle.transcript.includes(needle)) return handle.transcript
|
||||
if (handle !== undefined) {
|
||||
if (anyOutput ? handle.transcript.length > 0 : handle.transcript.includes(needle)) return handle.transcript
|
||||
}
|
||||
await new Promise(resolve => setTimeout(resolve, 50))
|
||||
}
|
||||
const handle = registry.get(uuid)
|
||||
@@ -405,8 +425,15 @@ describe('AgentPtyRegistry', { timeout: 30_000 }, () => {
|
||||
it('waitFor returns skipped when the user skips from the sidebar', async () => {
|
||||
const registry = new AgentPtyRegistry(testShell())
|
||||
try {
|
||||
const uuid = registry.create('s1', 'skip-test', 'echo skip-ready', process.cwd(), 80, 24)
|
||||
await waitForTranscript(registry, uuid, 'skip-ready')
|
||||
// A BARE shell, and the wait needle appears nowhere in its preparation:
|
||||
// an interactive shell echoes what is written to it, so a command that
|
||||
// spelled the needle would already have put it in the transcript before
|
||||
// this point — waitFor's fast path would resolve the wait and there
|
||||
// would be nothing left to skip. POSIX /bin/sh does not echo a command
|
||||
// written to a non-tty and hid this for a long time; PowerShell does,
|
||||
// which is why it only ever failed on Windows.
|
||||
const uuid = registry.create('s1', 'skip-test', '', process.cwd(), 80, 24)
|
||||
await waitForShellReady(registry, uuid)
|
||||
// waitFor registers its record synchronously (before the first poll
|
||||
// await), so the skip can fire immediately after the call.
|
||||
const waitPromise = registry.waitFor(uuid, 'NEVER_APPEARS_XYZ', 30_000)
|
||||
@@ -424,8 +451,10 @@ describe('AgentPtyRegistry', { timeout: 30_000 }, () => {
|
||||
it('snapshot exposes waiting while a wait is active and clears after it ends', async () => {
|
||||
const registry = new AgentPtyRegistry(testShell())
|
||||
try {
|
||||
const uuid = registry.create('s1', 'wait-snap', 'echo snap-ready', process.cwd(), 80, 24)
|
||||
await waitForTranscript(registry, uuid, 'snap-ready')
|
||||
// Same echo hazard as the skip case above: a bare shell puts no needle
|
||||
// in the transcript on its own.
|
||||
const uuid = registry.create('s1', 'wait-snap', '', process.cwd(), 80, 24)
|
||||
await waitForShellReady(registry, uuid)
|
||||
const waitPromise = registry.waitFor(uuid, 'LATER_MARK_9', 30_000)
|
||||
// Registration happens synchronously before waitFor's first await.
|
||||
expect(registry.list('s1')[0]?.waiting?.needle).toBe('LATER_MARK_9')
|
||||
@@ -467,8 +496,10 @@ describe('AgentPtyRegistry', { timeout: 30_000 }, () => {
|
||||
it('fires change listeners when a wait starts and ends', async () => {
|
||||
const registry = new AgentPtyRegistry(testShell())
|
||||
try {
|
||||
const uuid = registry.create('s1', 'watched-wait', 'echo notify-ready', process.cwd(), 80, 24)
|
||||
await waitForTranscript(registry, uuid, 'notify-ready')
|
||||
// Same echo hazard as the skip case above: a bare shell puts no needle
|
||||
// in the transcript on its own.
|
||||
const uuid = registry.create('s1', 'watched-wait', '', process.cwd(), 80, 24)
|
||||
await waitForShellReady(registry, uuid)
|
||||
let changes = 0
|
||||
const unsubscribe = registry.subscribe(() => { changes += 1 })
|
||||
const waitPromise = registry.waitFor(uuid, 'NEVER_NOTIFY_1', 30_000)
|
||||
|
||||
Reference in New Issue
Block a user