Repository navigation
fix: forward generation timeout to model HTTP requests - #462
Sun-sunshine06 merged 1 commit into
Conversation
generationTimeoutSec only armed the run-level AbortController. The HTTP requests themselves went through pi-ai without timeoutMs, so the OpenAI / Anthropic SDK clients applied their 600s default and cut long local-model turns (LM Studio, Ollama, vLLM) at 10 minutes regardless of the setting. Thread the configured timeout through: - providers: complete() accepts timeoutMs and passes it to completeSimple - core: GenerateInput.requestTimeoutMs wraps the agent stream function so every streamSimple call carries timeoutMs - desktop: the generate run derives it from generationTimeoutSec for the agent and the visual-parity judge; a disabled timeout (0) maps to the largest delay Node timers accept Refs OpenCoworkAI#199
There was a problem hiding this comment.
Findings
No Blocker, Major, or Minor findings. The change is additive (timeoutMs on GenerateOptions, requestTimeoutMs on GenerateInput), keeps every model call inside @mariozechner/pi-ai, adds no dependencies, and includes a changeset (.changeset/forward-generation-timeout-to-requests.md:1). The per-request timeout math mirrors the existing run timer (apps/desktop/src/main/generation-ipc.ts:110) and is covered by tests.
Questions
apps/desktop/src/main/ipc/generate.ts:710now readsreadPreferences()unconditionally at handler entry, before the run's timeout is armed.armGenerationTimeoutdeliberately rethrows prefs failures asPREFERENCES_READ_FAIL/PREFERENCES_INVALID_TIMEOUT(apps/desktop/src/main/generation-ipc.ts,armGenerationTimeout). Confirm the early read sits inside that same error-mapping path, or deriverequestTimeoutMsfrom the valuearmGenerationTimeoutalready reads, so a corrupt/invalid prefs file still yields the documented error code instead of a raw rejection.packages/core/src/agent.ts:30importsstreamSimpledirectly instead of routing the agent stream throughpackages/providers. AGENTS.md prefers extendingpackages/providerswhen pi-ai lacks a capability. Was a providers-levelstreamWithTimeouthelper considered, and is the directstreamSimplewrapper in core intentional?
Summary
- Review mode: initial
- Direction is sound. The provider SDKs default to a 10-minute per-request timeout that ignored
generationTimeoutSec; forwarding the setting through pi-ai'stimeoutMsis the right, non-invasive fix, and mapping a disabled (0) or oversized value to the int32 timer max preserves the intent of #411 without adding a new setting. Since the run-levelAbortControllerand the per-request timeout use the same value and the run timer starts no later than the first request, run-level abort still ends a run as the PR describes. - Residual risk: desktop always computes a concrete
requestTimeoutMs, so the agent always receives a customstreamFn; theundefinedfallback back to pi-agent-core's default stream is only reachable from non-desktop callers.packages/core/src/agent.test.tsmocks pi-agent-core and only checks argument passthrough — the only end-to-end evidence thattimeoutMsreaches the SDK ispackages/providers/src/request-timeout.test.ts. - The
Closes #199/Refs #411claims could not be validated against the issue bodies in this run (issue text not present in the provided public context).
Testing
- Coverage is solid: real stalled-server integration test (
packages/providers/src/request-timeout.test.ts), providers passthrough (packages/providers/src/index.test.ts), agentstreamFnpresent/absent (packages/core/src/agent.test.ts), and desktop mapping (apps/desktop/src/main/generation-ipc.test.ts). - Gap: no test asserts the
apps/desktop/src/main/ipc/generate.tswiring actually passesrequestTimeoutMsto bothgenerateViaAgentand the visual-parity judgecomplete()opts. A small unit test (or an explicit comment if the preflight harness makes that impractical) would close this.
Open-CoDesign Bot
|
Thanks for the review. I traced the preference read against Preference errors. The new
Wiring test. |
Summary
generationTimeoutSec(Settings → Advanced → Generation timeout) only arms the run-levelAbortController. The model HTTP requests go through pi-ai withouttimeoutMs, so the OpenAI and Anthropic SDK clients use their own 600 000 ms default. Any single request longer than 10 minutes gets cut by the client. This is common with large local models on LM Studio, Ollama, or vLLM, and it happens even when the user has set 3600 s or 7200 s.@mariozechner/pi-ai0.72.1 (already our pinned version) exposesStreamOptions.timeoutMsand maps it to the SDKtimeoutinopenai-completions,openai-responses,azure-openai-responses, andanthropic. This PR passes the configured value through:GenerateOptions.timeoutMsis forwarded tocompleteSimple.GenerateInput.requestTimeoutMs. pi-agent-core'sAgentdoes not forwardtimeoutMsfrom its own options, so when a timeout is set, the agent gets astreamFnthat callsstreamSimplewith{ ...options, timeoutMs }. It applies to every turn and retry agent. With no timeout set, the agent keeps pi-agent-core's default stream.requestTimeoutMsfromgenerationTimeoutSec(generationRequestTimeoutMs) and passes it to the agent and the visual-parity judgecomplete()call. The request timeout is equal to the run timeout, so the run-level abort (with its clear "Generation aborted after Ns" message) is still what ends a run.Relation to #411 (unlimited timeout):
armGenerationTimeoutalready treats0as "disabled".generationRequestTimeoutMs(0)maps that to the largest delay Node timers accept (2 147 483 647 ms ≈ 24.8 days) instead of falling back to the SDK's 10 minutes. Huge values are clamped the same way, because larger delays overflow and fire immediately. That means #411 only needs the UI/preferences side once this lands. This PR does not change the Settings options.Out of scope (follow-up): short auxiliary calls (run-preference router, title, memory/brief summaries) still use the SDK default. They have small output budgets, and the router call doesn't take a
signalyet either.Type of change
Linked issue
Closes #199
Refs #411
Checklist
pnpm lint && pnpm typecheck && pnpm testpasses locallypnpm changeset) if user-visibleTests
packages/providers/src/request-timeout.test.ts: real pi-ai and OpenAI SDK against a local stalled OpenAI-compatible server.complete(..., { timeoutMs: 200 })rejects with "Request timed out" in about 2 s. Without the fix it hangs on the SDK's 10-minute default (the test times out at 20 s).packages/providers/src/index.test.ts:timeoutMsreachescompleteSimple.packages/core/src/agent.test.ts: withrequestTimeoutMs, the agent'sstreamFncallsstreamSimplewithtimeoutMs. Without it, no customstreamFnis installed.apps/desktop/src/main/generation-ipc.test.ts:generationRequestTimeoutMs(1200 s → 1 200 000 ms, 7200 s → 7 200 000 ms, 0/huge → int32 max).PRINCIPLES §5b
timeoutMs(for example Google and Bedrock) ignore it in pi-ai.timeoutMsoption (no patching of SDK internals) and nothing is persisted.