Greptile Summary

This PR adds a configurable idle timeout for LLM streaming responses, aborting with a descriptive error message if no token is received within the threshold (default 60 s). The feature is well-scoped and fits cleanly into the existing attempt.ts retry loop via a thin wrapper around streamFn.

Key implementation decisions look correct:

• The per-next() timer pattern (clearTimer() → createTimeoutPromise() → Promise.race) correctly resets the idle clock on each received chunk, not on the overall request duration.
• clearTimer() is invoked on all three exit paths of next() (done, non-done, and catch), preventing lingering timers.
• controller.abort(error) is called inside the timeout callback so upstream abort handling preserves the user-friendly error message (LLM idle timeout (60s): …) via the extended makeAbortError in attempt.ts.
• Zod schema uses .nonnegative() and the JSON schema uses minimum: 0, correctly allowing 0 to opt out of the feature.

One minor style nit: streamSimple is imported as a value on line 2 of llm-idle-timeout.ts but is used only in the type position ReturnType<typeof streamSimple> — it should be import type.

Confidence Score: 5/5

Safe to merge — all previous P1 concerns (empty abort body, timer leak on non-done path, schema rejecting 0) are resolved in this version.

The only remaining finding is a P2 style issue (value import used only for its type). All core logic — timer lifecycle, abort propagation, config validation — is correct and covered by 13 tests.

No files require special attention.

This is a comment left during a code review.
Path: src/agents/pi-embedded-runner/run/llm-idle-timeout.ts
Line: 2

Comment:
**Value import used only for its type**

`streamSimple` is imported as a value but used exclusively as a type in `ReturnType<typeof streamSimple>` on line 52. This should be `import type` to avoid a potential runtime side-effect load of `@mariozechner/pi-ai`. Projects using `verbatimModuleSyntax: true` would also require this.

Alternatively, you can drop the dependency on `streamSimple` entirely by deriving the type from `StreamFn`'s own return type (e.g. `Awaited<ReturnType<StreamFn>>`), which keeps the module self-contained.

```suggestion
import type { streamSimple } from "@mariozechner/pi-ai";
```

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (3): Last reviewed commit: "fix: stop rewriting abort reason names i..." | Re-trigger Greptile}

Thanks for the thorough review! Both issues have been fixed in the latest commit:

1. Schema: Changed .positive() → .nonnegative() in zod-schema.agent-defaults.ts. The generated schema (schema.base.generated.ts) was also regenerated and now has minimum: 0 instead of exclusiveMinimum: 0.

2. Abort propagation: The signal parameter has been replaced with controller: AbortController. On timeout, we now call controller.abort(error) to properly cancel the underlying request.

@greptile-apps Could you re-review the latest commit 8e4335cbb5? The fixes are on the same branch.

Addressed all review comments:

• Changed .positive() to .nonnegative() to allow idleTimeoutSeconds: 0 (disabled)
• Added controller.abort(error) to cancel network requests on timeout
• Modified makeAbortError() to preserve original error message ("LLM idle timeout..." instead of "aborted")
• Added clearTimer() before returning non-done chunks

@greptile-apps @chatgpt-codex-connector

@greptileai please rescore

Patched the idle-timeout path.

• streamWithIdleTimeout() now reports idle stalls through the existing runner abort path instead of aborting the raw controller directly.
• Idle timeouts now set normal timeout state, stop the active session, and reuse the standard cleanup path.
• Kept the change small: one callback seam in the wrapper, one handoff back into abortRun(true, error).

Verified with:

• pnpm test -- src/agents/pi-embedded-runner/run/llm-idle-timeout.test.ts
• pnpm tsgo

Landed on main.

• Landed source commit: ef0af15
• Merge commit: 6f09a68

Thanks @liuy.