external: stream hook input too, closing the last EOF-wait path (#1398) · Entire

External: Stream Hook Input Too, Closing the Last EOF-Wait Path (#1398)

6d40e25→main·Karthik Rameshkumar·4d ago·3 files·+63 added/-12 removed

Trail review correctly flagged that external.ParseHookEvent still used io.ReadAll(io.LimitReader(stdin, …)) for the piped case, so an external/plugin agent that keeps the stdin pipe open after delivering its payload would still hang forever — the exact #1398 bug the rest of the PR fixes. The StdinLooksInteractive guard only covered the TTY/no-payload case.

The external "parse-hook" contract receives the host's hook payload, which is JSON (verified by the agent's own tests/fixtures), and forwards its raw bytes to the subprocess. So it can stream a single JSON value like every other agent. Add ReadHookInputRawLimited (ReadHookInputRaw with a byte ceiling) so external keeps its 10 MB bound without waiting for EOF, and forward the decoded raw bytes verbatim. No behavior change to the subprocess payload.

Adds regression tests: the bounded reader returns before stdin EOF, and rejects an over-limit payload instead of reading unbounded.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com

Sessions

01KXDWPCKTKDTWM9D7YAEP2XFF View transcript

Changes

3

// (e.g. key-name fallbacks, or forwarding the bytes to a subprocess) use this
// directly, while the common case uses ReadAndParseHookInput.
func ReadHookInputRaw(stdin io.Reader) (json.RawMessage, error) {
    return ReadHookInputRawLimited(stdin, -1)
}

// ReadHookInputRawLimited is ReadHookInputRaw with a ceiling of limit bytes on
// the JSON value (limit < 0 means unlimited). It is used at the external/plugin
// boundary to bound an untrusted payload — without reintroducing the EOF-wait
// hang, since the streaming decoder still returns on the first complete value.
func ReadHookInputRawLimited(stdin io.Reader, limit int64) (json.RawMessage, error) {
    // If stdin is an interactive terminal there is no payload coming at all: the
    // command was run by hand, or the agent left the console attached instead of
    // wiring up a pipe. Decoding would block waiting for input that never comes,
    // ... remaining implementation ...
}

Test Cases

// TestReadHookInputRawLimited_ReturnsBeforeEOF tests that the size-bounded raw reader must also return on the first complete JSON value without waiting for stdin close (issue #1398).
func TestReadHookInputRawLimited_ReturnsBeforeEOF(t *testing.T) {
    t.Parallel()
    pr, pw := io.Pipe()
    go func() {
        if _, err := pw.Write([]byte(`{"session_file":"/t.jsonl"}`)); err != nil {
            _ = pw.CloseWithError(err)
        }
    }
    done := make(chan error, 1)
    go func() {
        _, err := ReadHookInputRawLimited(pr, 10*1024*1024)
        done <- err
    }
    select {
    case err := <-done:
        if err != nil {
            t.Fatalf("unexpected error: %v", err)
        }
    case <-time.After(3 * time.Second):
        t.Fatal("ReadHookInputRawLimited blocked waiting for EOF — regression of #1398")
    }
}

// TestReadHookInputRawLimited_RejectsOversized proves the byte ceiling turns an over-limit payload into an error rather than an unbounded read.
func TestReadHookInputRawLimited_RejectsOversized(t *testing.T) {
    t.Parallel()

big := `{"k":"` + strings.Repeat("x", 512) + `"}`
    _, err := ReadHookInputRawLimited(strings.NewReader(big), 64)
    if err == nil {
        t.Fatal("expected error for payload exceeding the limit, got nil")
    }
}

// TestReadAndParseHookInput_EmptyInputEOF ... remaining implementation ...

func (e *Agent) ParseHookEvent(ctx context.Context, hookName string, stdin io.Reader) (*agent.Event, error) {
    const maxParseHookBytes = 10 * 1024 * 1024 // 10 MB
    // Bail out promptly on an interactive terminal — no payload is coming, and
    // io.ReadAll would otherwise block forever (issue #1398). We keep io.ReadAll
    // for the piped case because the external "parse-hook" contract forwards the
    // raw stdin bytes verbatim (which may be empty or non-JSON), so unlike the
    // built-in agents we can't stream-decode a single JSON value here.
    // ... remaining implementation ...
}