Respect explicit first-token timeout disable

resolveLlmFirstTokenTimeoutMs intentionally returns 0 when firstTokenTimeoutSeconds: 0 is configured, but this wrapper immediately maps 0 back to idleTimeoutMs, so the first-token timer still fires. In practice, configs like idleTimeoutSeconds: 30 + firstTokenTimeoutSeconds: 0 will still abort after 30s before the first token, which contradicts the new config contract and schema text that says 0 disables first-token timeout.

Useful? React with 👍 / 👎.

Fixed in 5d1e90d. firstTokenTimeoutMs === 0 now truly disables the first-token timer (waits indefinitely for the first chunk; the idle timer still kicks in once chunks start arriving). The resolver returns number | undefined: undefined means inherit-idle, 0 means disabled — preserving the contract the schema text now describes.

Misleading "disable" wording for firstTokenTimeoutSeconds: 0

Setting firstTokenTimeoutSeconds: 0 does not disable the first-token timeout — it falls back to using idleTimeoutSeconds for that phase (because streamWithIdleTimeout treats firstTokenTimeoutMs === 0 the same as "unset"). An operator wanting an unlimited first-token wait (e.g. for extreme cold starts) while keeping idle protection would set this to 0 expecting no cap, but still receive an idleTimeoutSeconds-bounded first-token window.

The description should clarify the actual semantics, e.g.:

This is a comment left during a code review.
Path: src/config/types.agent-defaults.ts
Line: 333

Comment:
**Misleading "disable" wording for `firstTokenTimeoutSeconds: 0`**

Setting `firstTokenTimeoutSeconds: 0` does not disable the first-token timeout — it falls back to using `idleTimeoutSeconds` for that phase (because `streamWithIdleTimeout` treats `firstTokenTimeoutMs === 0` the same as "unset"). An operator wanting an unlimited first-token wait (e.g. for extreme cold starts) while keeping idle protection would set this to 0 expecting no cap, but still receive an `idleTimeoutSeconds`-bounded first-token window.

The description should clarify the actual semantics, e.g.:

```suggestion
   * Set to 0 to remove the dedicated first-token window (the first-token phase
   * will then use `idleTimeoutSeconds`, same as when this field is unset).
   * To wait indefinitely for the first token, set a very large value instead.
```

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

Fixed in 5d1e90d. firstTokenTimeoutSeconds: 0 now genuinely disables the first-token timer (waits indefinitely for the first chunk). Updated docstring to describe the new contract: leave unset to inherit idleTimeoutSeconds, set to a positive value for a dedicated window, set to 0 to wait indefinitely for the first chunk.

Re-flagged on the rebased commit, but the underlying semantics were already corrected: firstTokenTimeoutSeconds: 0 now genuinely disables the first-token timer (waits indefinitely for the first chunk; idle timer still applies after the first chunk arrives). Tightened the docstring in 668f3dc to spell that out explicitly so it can't be re-misread.

options parameter shadows the outer options: StreamIdleTimeoutOptions

The inner function returned by streamWithIdleTimeout reuses options as the name of the StreamFn call-site parameter, shadowing the outer options: StreamIdleTimeoutOptions. There is no runtime bug (the outer options is fully destructured at the top of the function), but the shadowing is new — introduced because the outer positional arguments were collapsed into an options object in this PR. If someone adds code inside the inner closure expecting options to refer to timeout config, they'll silently pick up the stream-call options instead.

and update the one use inside the body:

const maybeStream = baseFn(model, context, streamCallOptions);

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

Comment:
**`options` parameter shadows the outer `options: StreamIdleTimeoutOptions`**

The inner function returned by `streamWithIdleTimeout` reuses `options` as the name of the `StreamFn` call-site parameter, shadowing the outer `options: StreamIdleTimeoutOptions`. There is no runtime bug (the outer `options` is fully destructured at the top of the function), but the shadowing is new — introduced because the outer positional arguments were collapsed into an `options` object in this PR. If someone adds code inside the inner closure expecting `options` to refer to timeout config, they'll silently pick up the stream-call options instead.

```suggestion
  return (model, context, streamCallOptions) => {
```

and update the one use inside the body:

```ts
const maybeStream = baseFn(model, context, streamCallOptions);
```

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

Fixed in 5d1e90d. Renamed the inner closure parameter to streamCallOptions and updated the body — no more shadowing.

Preserve timeout phase when surfacing timeout errors

This callback collapses both timeout phases into a single idle signal: streamWithIdleTimeout now invokes it for first-token and mid-stream idle timeouts, but attempt.ts only records idleTimedOut, so downstream error handling cannot tell which phase actually failed. In practice, a run that times out on firstTokenTimeoutSeconds (for example idle=30s, first-token=10s) is still reported as an idle-timeout case and points users at agents.defaults.llm.idleTimeoutSeconds, which does not fix the configured first-token limit; propagate the timeout phase (or split callbacks/flags) so retries and user guidance target the correct setting.

Useful? React with 👍 / 👎.

Addressed in 668f3dc. Added llmTimeoutPhase: "first-token" | "idle" | null to EmbeddedRunAttemptResult. The idleTimeoutTrigger callback now records the phase by inspecting the error message (streamWithIdleTimeout already labels phase in the message text). The user-facing error in run.ts now points at agents.defaults.llm.firstTokenTimeoutSeconds for first-token timeouts and agents.defaults.llm.idleTimeoutSeconds for mid-stream idle timeouts. Retry/failover logic still keys off the idleTimedOut boolean so both phases stay eligible for the same-model retry — both warrant a retry. Added a parallel test in run.timeout-triggered-compaction.test.ts.

Keep timeout leak detection aligned with first-token errors

This new first-token timeout text does not match the timeout leak pattern used by QA character eval (extensions/qa-lab/src/character-eval.ts only matches model did not produce a response before the LLM idle timeout). In scenarios where a run times out before the first token, the failure text can now pass through detectTranscriptFailure unclassified, so eval runs may be marked clean even though a timeout error leaked into the transcript. Please update the timeout detection/matching path to treat first-token and idle timeout messages consistently.

Useful? React with 👍 / 👎.