From d98a4ec1cad4690fc024db218689d2231e4eef67 Mon Sep 17 00:00:00 2001 From: Bradenream <51544548+Bradenream@users.noreply.github.com> Date: Fri, 2 Oct 2026 18:35:44 +0000 Subject: [PATCH] fix: stop waiting on the silent stdin an agent's shell hands vf (#38) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Claude Code runs any Bash command that contains a heredoc with stdin connected to a Unix socket. It never writes to that socket and never closes it; commands without a heredoc get `/dev/null` instead. vf took the socket for a piped body. Every body command in such a call waited the full 10s stdin deadline and then failed, even with every field given as a flag. An agent then had to retry with ` 0 { + silence := time.NewTimer(silenceLimit) + defer silence.Stop() + select { + case <-first.arrived: + case <-silence.C: + return nil, nil + } + } + select { case res := <-done: return res.data, res.err @@ -76,3 +144,17 @@ func readStdinBounded(r io.Reader) ([]byte, error) { ) } } + +// firstReadSignal closes arrived once the first read from r returns, whether +// with data, EOF or an error. +type firstReadSignal struct { + r io.Reader + once sync.Once + arrived chan struct{} +} + +func (f *firstReadSignal) Read(p []byte) (int, error) { + n, err := f.r.Read(p) + f.once.Do(func() { close(f.arrived) }) + return n, err +} diff --git a/internal/flagutil/stdin_test.go b/internal/flagutil/stdin_test.go new file mode 100644 index 00000000..33f43486 --- /dev/null +++ b/internal/flagutil/stdin_test.go @@ -0,0 +1,97 @@ +package flagutil + +import ( + "io" + "os" + "strings" + "testing" + "time" +) + +func TestSilenceMeansNoBodyOnlyForASocketInAgentMode(t *testing.T) { + cases := []struct { + name string + isAgentMode bool + stdin os.FileMode + want bool + }{ + {"socket in agent mode", true, os.ModeSocket, true}, + {"socket outside agent mode", false, os.ModeSocket, false}, + // A pipe is what a shell pipeline gives vf; a slow producer must be read. + {"pipe in agent mode", true, os.ModeNamedPipe, false}, + {"file in agent mode", true, 0, false}, + } + for _, tc := range cases { + if got := silenceMeansNoBody(tc.isAgentMode, tc.stdin); got != tc.want { + t.Errorf("%s: silenceMeansNoBody = %v, want %v", tc.name, got, tc.want) + } + } +} + +// silentReader returns a reader that delivers nothing and never closes until +// the test ends. +func silentReader(t *testing.T) io.Reader { + t.Helper() + r, w := io.Pipe() + t.Cleanup(func() { w.Close() }) + return r +} + +// writeAfter returns a reader that delivers body after delay, then closes. +func writeAfter(delay time.Duration, body string) io.Reader { + r, w := io.Pipe() + go func() { + time.Sleep(delay) + io.WriteString(w, body) + w.Close() + }() + return r +} + +func TestReadStdinBoundedTreatsSilenceAsNoBody(t *testing.T) { + started := time.Now() + data, err := readStdinBounded(silentReader(t), 20*time.Millisecond) + if err != nil || data != nil { + t.Fatalf("got (%q, %v), want no body and no error", data, err) + } + if waited := time.Since(started); waited > time.Second { + t.Errorf("waited %s on a silent reader, want about the silence limit", waited) + } +} + +func TestReadStdinBoundedReadsABodyThatStartsInTime(t *testing.T) { + data, err := readStdinBounded(writeAfter(10*time.Millisecond, `{"prompt":"x"}`), time.Second) + if err != nil || string(data) != `{"prompt":"x"}` { + t.Fatalf("got (%q, %v), want the body", data, err) + } +} + +// The limit is on silence, not on the whole read: once anything arrives, the +// rest may take as long as StdinReadTimeout allows. +func TestReadStdinBoundedKeepsReadingAfterTheFirstBytes(t *testing.T) { + r, w := io.Pipe() + go func() { + io.WriteString(w, `{"prompt":`) + time.Sleep(100 * time.Millisecond) + io.WriteString(w, `"x"}`) + w.Close() + }() + data, err := readStdinBounded(r, 20*time.Millisecond) + if err != nil || string(data) != `{"prompt":"x"}` { + t.Fatalf("got (%q, %v), want the whole body", data, err) + } +} + +func TestReadStdinBoundedReturnsAtOnceOnEOF(t *testing.T) { + data, err := readStdinBounded(strings.NewReader(""), time.Second) + if err != nil || len(data) != 0 { + t.Fatalf("got (%q, %v), want an empty body", data, err) + } +} + +func TestReadStdinBoundedWithoutALimitWaitsForASlowBody(t *testing.T) { + data, err := readStdinBounded(writeAfter(300*time.Millisecond, `{"prompt":"slow"}`), 0) + if err != nil || string(data) != `{"prompt":"slow"}` { + t.Fatalf("got (%q, %v), want the slow body", data, err) + } +} diff --git a/internal/output/agentmodecheck.go b/internal/output/agentmodecheck.go new file mode 100644 index 00000000..1b6eea96 --- /dev/null +++ b/internal/output/agentmodecheck.go @@ -0,0 +1,15 @@ +// This file is not generated by Speakeasy. It lets flagutil ask whether agent +// mode is on: in agent mode a silent socket on stdin is no request body (see +// internal/flagutil/stdin.go), and flagutil cannot import this package to ask, +// since this package imports flagutil. +// +// flagutil calls IsAgentMode itself instead of keeping a copy of the answer, so +// it follows however agent mode is finally resolved, flags included. + +package output + +import "github.com/voiceflow/cli/internal/flagutil" + +func init() { + flagutil.SetAgentModeCheck(IsAgentMode) +} diff --git a/test/stdin-handling.test.ts b/test/stdin-handling.test.ts index a7a25fd3..6bad1498 100644 --- a/test/stdin-handling.test.ts +++ b/test/stdin-handling.test.ts @@ -33,19 +33,24 @@ const ARGS = [ '--token', 'vfp_not_a_real_token', ].join(' '); -/** Runs a real shell pipeline: | vf agent update --dry-run */ -function pipeline(producer: string, options: { timeout?: number } = {}) { - return execa({ reject: false, timeout: options.timeout ?? 20_000, stdin: 'ignore' })( - 'sh', ['-c', `${producer} | ${VF} ${ARGS}`], +/** Runs a real shell pipeline: | vf agent update --dry-run [flags] */ +function pipeline(producer: string, options: { timeout?: number; flags?: string; env?: Record } = {}) { + return execa({ reject: false, timeout: options.timeout ?? 20_000, stdin: 'ignore', env: options.env })( + 'sh', ['-c', `${producer} | ${VF} ${ARGS} ${options.flags ?? ''}`], ); } -/** The dry-run output contains the request body; pull the prompt back out. */ -function sentPrompt(output: string): string | null { - const m = output.match(/"prompt":\s*"([^"]*)"/); +/** The dry-run output contains the request body; pull a string field back out. */ +function sentField(output: string, field: string): string | null { + const m = output.match(new RegExp(`"${field}":\\s*"([^"]*)"`)); return m ? m[1] : null; } +const sentPrompt = (output: string) => sentField(output, 'prompt'); + +/** Puts vf in agent mode the way Claude Code's shell does. */ +const AGENT = { CLAUDECODE: '1' }; + describe('stdin from cold producers', () => { // Each of these is a producer that does real work before writing. They are // the shapes that a readiness check drops, so they are the regression guard. @@ -97,6 +102,57 @@ describe('stdin that never delivers', () => { }); }); +describe("stdin in an agent's shell", () => { + // Claude Code runs any Bash command that contains a heredoc with stdin + // connected to a Unix socket that it never writes to and never closes. Every + // body command in such a call used to wait the full 10s and then fail, flags + // or not. execa's stdin: 'pipe' hands vf the same thing: Node spawns children + // over a socketpair, and nothing is written unless the test writes it. + const agentShell = (args: string[]) => + execa({ reject: false, timeout: 20_000, stdin: 'pipe', env: AGENT })(VF, args); + + // Well under the 10s the silent socket used to cost, with room for a slow CI + // machine to start the binary. + const NO_WAIT_MS = 5_000; + + it('takes the body from the flags without waiting on a silent socket', async () => { + const started = Date.now(); + const result = await agentShell([...ARGS.split(' '), '--prompt', 'from-flag']); + + expect(Date.now() - started, 'vf waited on a socket that never writes').toBeLessThan(NO_WAIT_MS); + expect(result.exitCode, result.stderr).toBe(0); + expect(sentPrompt(result.stderr + result.stdout)).toBe('from-flag'); + }); + + it('does not wait either when no body flag is given', async () => { + const started = Date.now(); + const result = await agentShell(ARGS.split(' ')); + + expect(Date.now() - started, 'vf waited on a socket that never writes').toBeLessThan(NO_WAIT_MS); + expect(result.exitCode, result.stderr).toBe(0); + }); + + it('still reads a body the socket delivers as vf starts', async () => { + const result = await execa({ reject: false, timeout: 20_000, input: '{"prompt":"from-socket"}', env: AGENT })( + VF, ARGS.split(' '), + ); + expect(sentPrompt(result.stderr + result.stdout)).toBe('from-socket'); + }); + + // Only a socket is cut short. A shell pipe is what `producer | vf` gives vf, + // in agent mode too, so a producer slower than the socket's wait still has + // its body read, and merged with the flags as the README documents. + it('still reads a slow shell producer, merged with the flags', async () => { + const result = await pipeline(`sh -c 'sleep 1; echo "{\\"prompt\\":\\"from-slow-pipe\\"}"'`, { + flags: '--instructions from-flag', + env: AGENT, + }); + expect(result.timedOut).toBe(false); + expect(sentPrompt(result.stderr + result.stdout), 'slow body dropped in agent mode').toBe('from-slow-pipe'); + expect(sentField(result.stderr + result.stdout, 'instructions')).toBe('from-flag'); + }); +}); + describe('backward compatibility', () => { // execa's `input:` is how the rest of the suite pipes stdin. It must keep // working, but it is deliberately NOT counted as coverage for the cases above.