fix(provider): bound a stalled provider stream body with a configurable idle deadline

[kojiwakayama]

What was broken

Once response headers arrived, a provider response body had no deadline at all.
requestStream disposed its header deadline and handed the body to
streamWithCleanup, whose pull() awaited reader.read() with nothing bounding
it (src/provider/runtime-loader/provider-http.ts:987 before this change). A
model stream that went silent mid-response blocked its caller until the caller
cancelled.

Scope, corrected. An earlier revision of this description claimed
veryfront dev chat and hosted runs were unbounded. That was wrong, and it
overruled the issue triage comment that had it right. On origin/main:

• src/agent/runtime/chat-stream-handler.ts:89-90 declares
  STREAM_START_IDLE_MS = 60_000 and STREAM_OUTPUT_IDLE_MS = 15_000 and
  applies them at :1214/:1223 with no VF_STREAM_LIFECYCLE_MODE
  condition. src/agent/runtime/index.ts:3841 passes no override, so every
  streaming chat turn has always been bounded at 15-60s.
• child-fork-execution-runner.ts:77 bounds hosted child forks at 45s.
• hosted chat runs additionally carry createChatStreamWatchdog
  (DEFAULT_CHAT_STREAM_IDLE_TIMEOUT_MS, and a 300s tool-running window).
• veryfront eval gained --record-timeout in feat(eval): show live progress and add --record-timeout #4508.

The genuinely unbounded path is the non-streaming drain: agent.generate goes
through buildGenerateResultFromStream in runtime-bridge.ts (drain at :968)
with no timer at all, and veryfront-cloud/provider.ts:51 sets
_generateViaStream for every gateway model. That caller, and library
embedders on the same path, are what this change exists for. The safety margin
is therefore wider than the earlier description claimed, not narrower: a
stricter consumer bound fires first everywhere a consumer exists.

What this changes

• src/provider/runtime-loader/provider-http.ts - streamWithCleanup arms a
  deadline around each pending body read and disarms it as soon as bytes land, so
  it bounds provider silence rather than response length, and a consumer that
  stops pulling is never timed out for its own backpressure. On expiry it takes
  the same path as a failed read - abort the request, error the stream, cancel the
  upstream reader - so the connection is released rather than left open, and the
  caller sees a retryable ProviderRequestError naming the deadline that fired.
• Configurable, which the issue asks for: the bound resolves from
  VERYFRONT_PROVIDER_STREAM_IDLE_TIMEOUT_MS, read from the host environment on
  every stream request, falling back to a 120s default. 0 disables it. An
  explicit idleTimeoutMs argument still wins for custom provider extensions. It
  is read via getHostEnvExcludingEnvFile, not getEnv or plain getHostEnv, so
  a loaded project's .env cannot widen or switch off a safety bound the host
  operator set; a malformed value warns and uses the default instead of failing
  every provider request.
• docs/guides/configuration.md - a "Provider stream idle deadline"
  section under "Environment variables", stating the default, the 0-disables
  rule and the host-vs-.env rule. The generated api-reference row states none
  of those, so without this the knob had no durable user-facing documentation.

Regression tests, and confirmation they fail without the fix

• src/runtime/runtime-bridge.test.ts - "bounds a stalled provider body reached
  through generate": drives the real requestStream over a fetch whose body
  delivers one chunk and then goes quiet, through the buffering drain loop that
  generate runs. Nothing configures the deadline, so the shipped 120s default is
  what fires. FakeTime keeps it instant.
• src/provider/runtime-loader/provider-http.test.ts - "bounds a stalled body at
  the default deadline with no configuration" (same, at the requestStream
  level, and asserts the request is not aborted at 119_999ms), plus the
  existing explicit-idleTimeoutMs cases for re-arming, disabling, aborting the
  request, and rejecting invalid values before a request is issued.
• src/provider/runtime-loader/provider-http.test.ts - "bounds a stalled body at
  the host environment override", new in the latest revision: sets
  VERYFRONT_PROVIDER_STREAM_IDLE_TIMEOUT_MS=5000 in the real host environment
  and drives requestStream with no explicit idleTimeoutMs, asserting the
  request is not aborted at 4_999ms and that the error names a 5000ms deadline.
  This closes a real coverage gap: every other environment test called
  resolveProviderStreamIdleTimeoutMs directly with an injected reader, so
  replacing the resolver call inside requestStream with a plain
  default-or-normalize expression left all of them green while the knob stopped
  working for every caller this description names. Verified: with that
  substitution applied, the previously existing suite still reports
  ok | 1 passed, and with the new test present the run stops on it with
  error: Promise resolution is still pending but the event loop has already resolved.

Earlier, verified by neutering armIdleDeadline() in the working tree and
re-running: both files go from ok | 1 passed to EXIT 1, stopping on the new
test with the same pending-promise hang - i.e. exactly the hang this fixes.
Restored before committing.

Gates run

Gate	Result
deno task test:file src/provider/runtime-loader/provider-http.test.ts	ok, 109 steps, 0 failed
deno task test:file src/runtime/runtime-bridge.test.ts	ok, 49 steps, 0 failed
sweep of src/provider/**/*.test.ts + anthropic/openai/google provider tests	all pass
deno fmt --check docs/guides/configuration.md	clean (the file was already fmt-clean, so the added section is the whole diff)
deno check --no-lock on the changed .ts files	clean
deno task lint	clean
deno fmt --check on the changed files	clean
deno task lint:test-typecheck	pass
deno task lint:test-semantic-dispositions	pass
deno task lint:testing-front-door	pass
deno task lint:client-bundle	pass
deno task lint:anti-slop, lint:style, lint:module-boundaries, lint:barrel-jsdoc, lint:check-awaits, lint:cwd-relative-test-reads, lint:skipped-tests, lint:dependency-boundaries	pass
deno task docs + docs:api-reference:check	pass; two generated lines added, for the new exported constant and the env key

lint:imports (86 findings), lint:ban-deep-imports (3) and lint:platform
(119) fail, but fail identically on origin/main with the same counts and no
entries in the files this PR touches - pre-existing, not gating in the lint
chain.

lint:test-semantic-dispositions initially failed on an earlier revision of
this branch because the configurability tests mutated the host environment with
setEnv/deleteEnv. Rather than grow
scripts/test/test-semantic-audit-migration.ts, the resolver now takes an
injectable readEnv lookup that defaults to the real one. The two tests that
still mutate the host environment - the .env-provenance case and the new
requestStream override case - are the two where the seam under test is the
default reader, so injecting past it would assert on the mock; the audit passes
with both present.

Review findings addressed

1. The agent.generate regression test was vacuous. Correct, and confirmed:
   it hand-built a ProviderRequestError, errored a fake model stream with it,
   and asserted generateText rejected with that same object - zero lines of the
   change. Replaced with the end-to-end test described above, which goes through
   the real requestStream, and which covers the default path rather than an
   explicit short timeout.
2. The design note above the default was false - twice over, and the second
   revision of it was false in a new way. The original note claimed a margin
   below the chat watchdog. The replacement fixed that but asserted the 60s/15s
   figures "belong to lifecycle/policy.ts, which only applies under
   VF_STREAM_LIFECYCLE_MODE=shadow|active" - which quietly omitted that
   chat-stream-handler.ts declares its own ungated copy of those same two
   numbers and applies them to every streaming chat turn. The note now states
   the real layering (see "What was broken"), names agent.generate as the
   unwatched caller, and drops the claim that the 120s equality with
   DEFAULT_CHAT_STREAM_IDLE_TIMEOUT_MS is deliberate.
3. "Configurable" was not met. Correct: grep -rn idleTimeoutMs extensions/
   confirms no shipped provider forwards it. Fixed with the environment knob
   above, and the CHANGELOG migration advice rewritten to point at it. The new
   requestStream override test is what makes that claim checkable.
4. The guard test pinned the wrong thing. Dropped the
   DEFAULT_PROVIDER_STREAM_IDLE_TIMEOUT_MS === DEFAULT_CHAT_STREAM_IDLE_TIMEOUT_MS
   assertion. The equality is two independent choices landing on the same round
   number, not a designed relation, and the constant it pinned governs hosted
   chat runs through createChatStreamWatchdog rather than the caller the note
   names. The > 45s and < 300s relations do carry design content and stay
   pinned. This is a partial walk-back of the previous revision's "pin all three"
   decision; the purpose of that decision - the note must not be able to drift
   into stating something untrue - is preserved by removing the numeric claim
   from the note rather than by pinning it.
5. The keepalive rationale was scoped to evidence that does not cover every
   provider. The ping-frame and 15s-gateway-keepalive evidence covers
   Anthropic and the Veryfront Cloud gateway. ext-llm-openai (:1119, :1213)
   and ext-llm-google (:618) call requestStream with no idleTimeoutMs and
   decode no heartbeat. The conclusion still holds - streamWithCleanup disarms
   on raw bytes before provider-sse.ts parses anything, so any keepalive, SSE
   comment line or progress event re-arms the deadline whether or not the
   extension has a case for that event - but the note and the CHANGELOG now say
   that for a directly-configured OpenAI or Google model the argument rests on
   the transport emitting something rather than on a documented interval, and
   point at the environment knob.

Review findings declined

• That the 120s deadline regresses the 300s tool_running window for
  provider-executed tools. The two deadlines do not measure the same thing: the
  chat watchdog counts semantic chunks, this one counts bytes on the wire,
  and it disarms before any SSE parsing. A provider running a server-side tool
  (web_search, web_fetch, code_execution, the MCP connector) holds the HTTP
  response open but is not silent on the wire. Making the provider deadline
  phase-aware, or raising it above 300s, would also put it outside the 60-120s
  the issue asks for. See finding 5 above for the scope limit now recorded.
• Per-chunk setTimeout/clearTimeout churn (minor, perf). Measured at
  ~1.15us per arm/disarm pair on this runtime (200k pairs in 230ms), so a
  5,000-chunk SSE response spends under 6ms on timers across a response lasting
  tens of seconds. A polling interval over a lastChunkAt timestamp would trade
  that for deadline precision and a timer outliving the read it guards. Declined
  as a rewrite, but the measurement is now recorded in the comment so it is not
  re-litigated.
• retryable: true on a post-partial-output timeout (minor). Kept, but the
  stated reason was wrong and is corrected. The earlier note said "nothing
  retries automatically". extensions/ext-llm-anthropic/src/anthropic-provider.ts
  does: :99 (isReplayableAnthropicStreamFailure) and the catch at :751 are
  a mid-turn replay loop keyed on exactly this flag. It cannot duplicate output
  because yieldedThisAttempt short-circuits that catch before the predicate is
  consulted, so a timeout that fired after the first chunk rethrows. The call-site
  comment now names that guard as the reason the flag is safe, rather than
  claiming no retry loop exists.

Known gap, filed separately rather than fixed here

Commit a534666 found that getHostEnv returns values loadEnv copied out of a
project .env, and fixed only its own new read. src/security/http/outbound-fetch.ts:246
(VERYFRONT_HOST_ALLOW_INTERNAL_EGRESS) and :295
(HOST_ALLOWED_INTERNAL_PROVIDER_ORIGINS) still read through plain getHostEnv
while :400 in the same file already uses getHostEnvExcludingEnvFile. Those
two keys gate whether run-scoped inference credentials may leave over a
non-HTTPS, non-loopback origin. Different file, different subsystem, untouched by
this diff - so it is filed on its own rather than held here, as
veryfront/veryfront-issue-inbox#1623, with the reachability argument
(src/utils/env-loader.ts copies every .env key into the process
environment, and the if (existing && !override) continue guard does not
protect a gate that is off by default, because off-by-default means unset).

Deliberately not addressed here

Two logging-policy gaps in resolveProviderStreamIdleTimeoutMs, left open
because the sensible fixes trade against each other and against text already
in this CHANGELOG. They need a maintainer's call, not a patch:

• A malformed value re-warns on every stream request. There is no
  dedupe, so one typo in a deployment's environment emits a warning per model
  call for the life of the process.
• A well-formed value that loadEnv copied out of a project .env is
  discarded silently - no log at any level. A veryfront dev user who
  widens the bound in .env gets the 120s default with no signal that their
  setting was ignored.

Caching the resolved value at module scope would fix both at once, but it
contradicts the "read from the host environment on every stream request"
contract stated above and in the CHANGELOG, and the new requestStream
override test depends on the per-call read. See the review comment on this PR
for the options.

Refs veryfront/veryfront-issue-inbox#1465

[coderabbitai]

Warning

Review limit reached

Next included review available in 42 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 31685de5-18fd-4305-8c45-c8a18b545458

📥 Commits

Reviewing files that changed from the base of the PR and between dfd95e9 and 7c4d264.

📒 Files selected for processing (8)

• CHANGELOG.md
• docs/api-reference/veryfront/provider.md
• docs/guides/configuration.md
• src/provider/runtime-loader.ts
• src/provider/runtime-loader/provider-http.test.ts
• src/provider/runtime-loader/provider-http.ts
• src/provider/shared/index.ts
• src/runtime/runtime-bridge.test.ts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

📦 Client bundle boundary

Entrypoint	Modules	Source size	Server leaks
src/index.client.ts	290	2322 KiB	✅ 0

A server module in a client graph aborts hydration in the browser. New leaks fail CI; known leaks are tracked in scripts/lint/client-bundle-baseline.json to burn down.

[gitar-bot]

Gitar is working

_Gitar

[codecov]

Codecov Report

❌ Patch coverage is 98.46154% with 1 line in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/provider/runtime-loader/provider-http.ts	98.30%	0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

[kojiwakayama]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1287706c60

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[kojiwakayama]

Independent review: 🟡 minor fixes first

Risk if merged as-is: medium | Reviewer confidence: high

Look at this first

Read src/agent/runtime/chat-stream-handler.ts:89-90 and :1214/:1223 on origin/main before reading anything in the diff. They declare STREAM_START_IDLE_MS=60_000 and STREAM_OUTPUT_IDLE_MS=15_000 and apply them with no VF_STREAM_LIFECYCLE_MODE gate, which means veryfront dev chat was never unbounded. The PR's 'What was broken' section, its CHANGELOG entry, the design comment at provider-http.ts:44-75 and the new guard test all rest on the opposite claim - and the PR reached that claim by overruling the issue triage comment that had cited those exact lines correctly. The code is fine and the safety margin is actually wider than the PR says (a stricter 15s consumer bound fires first, not a 120s tie), but this one premise is now written into shipped release notes and pinned by an assertion against a hosted-only constant in a module its own JSDoc marks as a legacy shim. Everything else in this review is cheaper to fix than to re-derive.

Blocking

1. CHANGELOG.md and the design comment at src/provider/runtime-loader/provider-http.ts:44-75 both assert that veryfront dev chat and hosted runs had no bound, and that the 60s/15s figures 'belong to lifecycle/policy.ts, which only applies under VF_STREAM_LIFECYCLE_MODE=shadow|active'. Both are false on origin/main: src/agent/runtime/chat-stream-handler.ts:89-90 declares STREAM_START_IDLE_MS=60_000 / STREAM_OUTPUT_IDLE_MS=15_000 and applies them at :1214/:1223 with no lifecycle-mode condition, so chat has always been bounded at 15-60s. DEFAULT_CHAT_STREAM_IDLE_TIMEOUT_MS=120_000 is reached only via createChatStreamWatchdog, built only in src/agent/hosted/. The CHANGELOG ships this to users as release-note text and must be corrected to name agent.generate (the genuinely unbounded path) instead.
2. The new guard test 'pins the default against the consumer watchdog windows' (provider-http.test.ts) hard-asserts DEFAULT_PROVIDER_STREAM_IDLE_TIMEOUT_MS === DEFAULT_CHAT_STREAM_IDLE_TIMEOUT_MS against watchdog-compat-adapter.ts, a module its own JSDoc marks as the legacy-compat shim for the lifecycle rollout. It locks the wrong rationale in place (the constant it pins does not govern the caller the comment names) and turns a doc note into a cross-module tripwire on code scheduled for deletion. Drop the equality assertion; the > 45s and < 300s relations are defensible and can stay.
3. The environment knob - the PR's entire answer to the issue's 'configurable' acceptance item - is never exercised through requestStream. Every env test calls resolveProviderStreamIdleTimeoutMs() directly with an injected readEnv; every requestStream test passes an explicit idleTimeoutMs (20/150/0) or nothing. Replacing const idleTimeoutMs = resolveProviderStreamIdleTimeoutMs(options.idleTimeoutMs) in requestStream with a plain default-or-normalize expression kills VERYFRONT_PROVIDER_STREAM_IDLE_TIMEOUT_MS for every caller the PR body names, and all eight new provider-http tests plus the runtime-bridge test still pass. Add one test that sets the host env and drives requestStream (the .env-provenance test already mutates the environment, so the hermeticity objection has a precedent).
4. Scope the keepalive rationale honestly in the CHANGELOG and the design comment. The claim that a provider-executed tool run 'is not silent' is evidenced only for Anthropic and the Veryfront Cloud gateway; ext-llm-openai (:1119, :1213) and ext-llm-google (:618) call requestStream with no idleTimeoutMs and no keepalive contract of any kind. The conclusion is probably still right (the deadline counts raw bytes, so any SSE traffic re-arms it - see disagreementBetweenLenses), but the stated evidence does not cover a directly-configured OpenAI/Google model, which is the caller most likely to be cut.

Non-blocking

• FILE SEPARATELY, higher value than anything in this diff: commit a534666 discovered that getHostEnv returns values loadEnv copied out of a project .env, and fixed only its own new line. src/security/http/outbound-fetch.ts:246 (VERYFRONT_HOST_ALLOW_INTERNAL_EGRESS) and :295 (HOST_ALLOWED_INTERNAL_PROVIDER_ORIGINS) still read through plain getHostEnv, while :400 in the same file already uses getHostEnvExcludingEnvFile - so the asymmetry is an oversight. Those two keys gate whether run-scoped inference credentials may leave over a non-HTTPS, non-loopback origin (veryfront-cloud/shared.ts:156-166), and env-loader.ts:63-72 copies every .env key into the process environment with no allowlist. loadEnv skips keys the host already set, which is exactly the state of a default-off gate, so the path is reachable. Pre-existing and outside this diff, so it does not block PR 4531.
• The comment at provider-http.ts:~1470 says 'nothing retries automatically' on retryable: true. False: extensions/ext-llm-anthropic/src/anthropic-provider.ts:99/:755 is an automatic mid-turn replay loop keyed on ProviderError.retryable. I checked the guard and output duplication is genuinely impossible - yieldedThisAttempt short-circuits the catch before the predicate is even consulted - so this is a comment defect, not the live hazard one reviewer described. Fix the sentence to point at yieldedThisAttempt rather than at an absent retry loop.
• resolveProviderStreamIdleTimeoutMs silently discards a well-formed VERYFRONT_PROVIDER_STREAM_IDLE_TIMEOUT_MS that loadEnv marked as coming from a project .env - no log line at all, while a malformed value warns. A veryfront dev user who widens the bound in .env gets the 120s default with no signal. One debug/warn line when hasEnvFileValueSource is true closes it.
• No entry in docs/guides/configuration.md ('## Environment variables', line 265), which already documents this exact shape of knob for VERYFRONT_FILE_CACHE_L1_* with default, clamp and 0-semantics. The only durable artifact is a generated api-reference row stating none of the default, the 0-disables rule, or the host-vs-.env rule.
• resolveProviderStreamIdleTimeoutMs runs once per requestStream call and its logger.warn has no dedupe, so one typo in a deployment's environment emits a warning per model call for the life of the process. Caching the resolved value at module scope would fix both this and the runtime-Deno.env.set path that bypasses the .env provenance check.
• The .env-provenance test's finally calls clearEnvFileValueSources() (plural), wiping process-wide provenance rather than the one key it marked. clearEnvFileValueSource(key) is exported from the same module and is already the repo precedent (outbound-fetch.test.ts:778). Deno shares one process across test files and several suites mark provenance for VERYFRONT_API_TOKEN/URL, so this is a latent ordering hazard for exactly the credential-provenance checks that matter most.

Where the reviewers disagreed

Four real conflicts; I adjudicated each against origin/main rather than averaging.

1. MERGE VERDICT (2-1). Correctness and security said merge; contract/test-quality said no. I side with the majority on the code and with the dissenter on the gaps: there is no correctness defect and all three issue acceptance items are met, so the objections are text and coverage, not behaviour - hence minor-fixes-first rather than needs-work.

2. THE OPENAI/GOOGLE KEEPALIVE MAJOR - the test-quality lens is wrong on the mechanism, right to be uneasy. Its argument is that only ext-llm-anthropic decodes a ping event, so the 120s default will cut long server-side tool runs on OpenAI/Google. That conflates decoding with receiving: streamWithCleanup wraps the fetch body as ReadableStream and disarms on any bytes, before provider-sse.ts ever parses an event. Heartbeats, SSE comment lines and progress events all re-arm the deadline whether or not the extension has a case for them. OpenAI's Responses API emits in_progress/searching/interpreting events during tool calls, so >120s of true wire silence on a healthy stream is unlikely. I demoted this from blocking-major to a blocking-but-cheap wording fix: the conclusion probably holds, the stated evidence does not cover the two direct-configured providers, and the PR's own comment makes the same decode-vs-receive conflation.

3. THE OUTBOUND-FETCH EGRESS GAP - the security lens is factually right and right not to block. I confirmed outbound-fetch.ts:246/:295 read two HOST_* egress keys through plain getHostEnv while :400 in the same file uses getHostEnvExcludingEnvFile, and env-loader.ts:63-72 copies every project .env key into the process environment unfiltered. It is a genuine reachable path to the same class of defect a534666 fixed here, and it is the single most valuable thing any of the three reviewers found - but it is a different file in a different subsystem, untouched by this diff. The rule that a security PR leaving a reachable path open must block does not reach a stream-deadline PR that neither introduced nor claimed to close that path. File it as its own issue; do not hold 4531 for it.

4. THE FALSE-RATIONALE FINDING - only the correctness lens caught it, and it under-rated its own catch. It labelled the chat-idle-window misattribution 'minor, does not change runtime behaviour', which is true of the source comment. But the same falsehood is in CHANGELOG.md, where it becomes shipped release-note text, and it is now pinned by an equality assertion against DEFAULT_CHAT_STREAM_IDLE_TIMEOUT_MS. I raised that half to blocking. The security and test-quality lenses both missed it entirely, and the test-quality lens independently flagged the same assertion as a brittle tripwire on a legacy shim without noticing it also encodes a false premise - the two findings are the same defect seen from opposite ends, and I merged them.

5. One reviewer's retryable-replay finding overstated the live risk. The Anthropic replay loop is real and is keyed on exactly this flag, but yieldedThisAttempt short-circuits the catch before the predicate runs, so no half-emitted turn can be duplicated even with the knob set below the 40s header budget. Comment defect, not a hazard; demoted to non-blocking.

6. testFailsFirstVerified: only the correctness lens actually checked it (neutered armIdleDeadline at a534666, both suites went to EXIT=1 on the new tests with a pending-promise hang); the other two returned could-not-check. I accept the one that did the work - the tests assert error class, message, retryable, status, fetch-signal abort, upstream reader cancellation with the same object, and nothing firing at 119_999ms. This is not coverage padding.

---

_{Provenance: independent automated review (Claude Code) via @kojiwakayama's token. Tooling output, not a human approval. Three reviewers examined this diff through separate lenses (correctness, security/authz, contract & test quality); a synthesis pass adjudicated their disagreements against origin/main and dropped findings that did not survive. Verify before acting.}

[kojiwakayama]

Pushed 913fb55 addressing the four blocking findings from the independent audit. All four were evidence defects rather than behaviour defects, so the runtime change in this PR is byte-identical to the previous head; what moved is what the PR, the CHANGELOG, the design note and the guard test claim.

1. The scope claim was false, and it was about to ship as release notes. Fixed.

Verified on origin/main before changing anything:

• src/agent/runtime/chat-stream-handler.ts:89-90 declares STREAM_START_IDLE_MS = 60_000 and STREAM_OUTPUT_IDLE_MS = 15_000, applied at :1214/:1223. The guards are shouldStopForIdleStart / shouldStopForIdleOutput, which are composed only from hasActiveLocalToolInput, shouldStopForCommittedLocalToolCallNow and hasStreamOutput(state) -- there is no VF_STREAM_LIFECYCLE_MODE condition anywhere on that path, and src/agent/runtime/index.ts:3841 passes no override.
• So veryfront dev chat has always been bounded at 15-60s. streaming/lifecycle/policy.ts does contain the identical 60_000/15_000 pair and is mode-gated; the previous revision's note used that fact to explain away numbers that also exist, ungated, one module over.
• DEFAULT_CHAT_STREAM_IDLE_TIMEOUT_MS (120s) is reached only through createChatStreamWatchdog, whose only construction sites are src/agent/hosted/chat-execution-runtime.ts:244 and cloud-prepared-chat-execution-runtime.ts:99. Hosted runs were bounded too.

The genuinely unwatched caller is the non-streaming drain: buildGenerateResultFromStream in runtime-bridge.ts:951 is a bare for await over the stream with no timer, and wrapVeryfrontCloudModel (src/provider/veryfront-cloud/provider.ts:52) sets _generateViaStream: true unconditionally for every gateway model. The issue triage comment had this right and this branch overruled it; it now says what triage said.

Corrected in the CHANGELOG entry, in the design note at provider-http.ts, and in the PR body. The practical effect is that the safety margin is wider than this PR previously claimed, not narrower: wherever a consumer exists, a stricter bound fires first.

2. Dropped the equality assertion in the guard test.

Removed assertEquals(DEFAULT_PROVIDER_STREAM_IDLE_TIMEOUT_MS, DEFAULT_CHAT_STREAM_IDLE_TIMEOUT_MS). The equality is two independent choices landing on the same round number, not a designed relation, and the constant it pinned governs hosted chat runs through createChatStreamWatchdog -- a different caller from the one the note names -- out of the lifecycle rollout's compatibility adapter.

Flagging this explicitly because it partially walks back commit 493eb27, which deliberately chose to "pin all three relations". I read that commit's reasoning first: the purpose was to stop the note drifting into an untrue claim. That purpose is better served by removing the numeric claim from the note, which is what this commit does. The > 45s and < 300s relations carry real design content and stay pinned.

3. Added the missing end-to-end coverage for the environment knob.

This was the sharpest finding. The knob is this PR's entire answer to the issue's "configurable" acceptance item, and nothing exercised it through requestStream: all eight environment tests called resolveProviderStreamIdleTimeoutMs directly with an injected readEnv, and every requestStream test passed an explicit idleTimeoutMs or none.

Confirmed the gap by mutation. Replacing

const idleTimeoutMs = resolveProviderStreamIdleTimeoutMs(options.idleTimeoutMs);

with a plain options.idleTimeoutMs === undefined ? DEFAULT_... : normalizeTimerDurationMs(...) -- which kills VERYFRONT_PROVIDER_STREAM_IDLE_TIMEOUT_MS for every caller the CHANGELOG names -- left the suite at ok | 1 passed (102 steps) | 0 failed.

New test "bounds a stalled body at the host environment override" sets the variable to 5000 in the real host environment, drives requestStream with no explicit deadline over a stalled body, and asserts the request is not aborted at 4_999ms and that the error names a 5000ms deadline. Under the same mutation the run now stops on it with error: Promise resolution is still pending but the event loop has already resolved. It mutates the host environment rather than injecting a reader because the seam under test is the default argument requestStream relies on; lint:test-semantic-dispositions passes with it (2312 considered, 734 disposed, unchanged).

4. Scoped the keepalive rationale to its evidence.

The ping-frame and 15s gateway keepalive evidence covers Anthropic and Veryfront Cloud. ext-llm-openai (:1119, :1213) and ext-llm-google (:618) call requestStream with no idleTimeoutMs and decode no heartbeat, so a directly-configured OpenAI or Google model was not covered by the stated argument.

I did not change the conclusion, because the mechanism holds independently: streamWithCleanup disarms on the raw Uint8Array a read yields, before provider-sse.ts parses an event, so any keepalive, SSE comment line or progress event re-arms the deadline whether or not the extension has a case for it. Decoding and receiving are different things and the previous note conflated them. The note and the CHANGELOG now say the argument for those two transports rests on the wire not going silent rather than on a documented interval, and point at the environment knob.

Also corrected (was non-blocking, but it was a false claim)

The call-site note said retryable: true was safe because "nothing retries automatically". It does: extensions/ext-llm-anthropic/src/anthropic-provider.ts:99 (isReplayableAnthropicStreamFailure) plus the catch at :751 is a mid-turn replay loop keyed on exactly this flag. It still cannot duplicate output, because yieldedThisAttempt short-circuits that catch before the predicate is consulted -- so the flag is safe, but for a different reason than the note gave. The note now names that guard.

Declined / deferred

• The outbound-fetch.ts egress gap (:246 VERYFRONT_HOST_ALLOW_INTERNAL_EGRESS and :295 HOST_ALLOWED_INTERNAL_PROVIDER_ORIGINS still read through plain getHostEnv, while :400 in the same file already uses getHostEnvExcludingEnvFile). Confirmed, and it is the same class of defect a534666 fixed here. Different file, different subsystem, untouched by this diff, and this PR neither introduced nor claimed to close it -- so it is recorded under "Known gap" in the PR body for its own issue rather than held here.
• I did not add the docs/guides/configuration.md entry or the warn-on-ignored-.env-value log line. Both are reasonable and both were raised as non-blocking; say the word and I will add them, but they widen a diff whose remaining findings were all about claims rather than code.

Gates

deno task test:file on provider-http.test.ts (ok, 109 steps) and runtime-bridge.test.ts (ok, 49 steps); deno check --no-lock on both changed .ts files; deno task lint; deno fmt --check on the three changed files; lint:test-semantic-dispositions, lint:testing-front-door, lint:anti-slop; docs:api-reference:check reports the reference is current (no exported symbol changed -- the edits are comments and one test import).

One thing I did not touch

GitHub reports this branch as CONFLICTING against main: a CHANGELOG collision with a newer ## Unreleased entry. I checked and it is pre-existing -- git merge-tree origin/main a534666 (the previous head, before this push) conflicts on exactly the same file -- so it is main advancing, not this commit. I have left it rather than adding a merge commit to a head someone may be about to review; say which you prefer and I will rebase or merge.

[kojiwakayama]

CI on 913fb55: green.

Full CI/CD matrix passed -- run 35568822297, completed success, every job success or skipped: ci (lint), ci (format), ci (typecheck), ci (test-layout), all four coverage shards, tests (integration), the deno/node/bun runtime-critical-flow jobs, the binary and RSC browser e2e jobs, coverage gate, quality gate (artifact), quality gate (merge), codecov upload, SonarQube Cloud scan and SonarQube Cloud quality gate. CodeQL Analyze (javascript-typescript) and Analyze (ruby) both pass on this head too.

One process note worth someone's attention: the pull_request-triggered workflows did not dispatch automatically for this push. gh api .../actions/runs?head_sha=913fb552fd returns only the pull_request_target ones (CLA Check, Automated review gate) plus the dynamic CodeQL run -- no CI/CD, no Framework performance, no Client bundle report, although all three fired normally for the previous head a534666 on 2026-09-20. I ran CI/CD via workflow_dispatch on fix/issue-1465 to get the result above, so this head is genuinely covered, but the missing auto-dispatch is not something this diff explains and it would silently leave a future push unverified.

Automated review remains pending with "PR#4531 draft waits for review" -- that is the draft gate, not a failure.

[kojiwakayama]

State of this PR for a reviewer

An independent audit rated this minor-fixes-first with four blocking items.
All four are now closed in the diff, and I re-verified each against
origin/main rather than trusting the earlier self-reports. One thing the
audit did not cover also turned up and is fixed: the branch had drifted into a
merge conflict.

Closed since the audit

1. The false scope claim. The PR body, the CHANGELOG entry and the design
   comment all asserted that veryfront dev chat was unbounded and that the
   60s/15s figures were mode-gated behind VF_STREAM_LIFECYCLE_MODE. That was
   false. Verified on origin/main: src/agent/runtime/chat-stream-handler.ts:89-90
   declares STREAM_START_IDLE_MS = 60_000 / STREAM_OUTPUT_IDLE_MS = 15_000
   and applies them at :1214/:1223 with no lifecycle-mode condition. All
   three now name agent.generate (the genuinely unwatched drain) instead.
   This mattered most in the CHANGELOG, where it was shipping to users as
   release-note text.
2. The guard test pinned the wrong thing. The
   DEFAULT_PROVIDER_STREAM_IDLE_TIMEOUT_MS === DEFAULT_CHAT_STREAM_IDLE_TIMEOUT_MS
   assertion is gone. It pinned a hosted-only constant, in a module its own
   JSDoc marks as the legacy-compat shim for the lifecycle rollout, against a
   rationale about a different caller. The > 45s and < 300s relations do
   carry design content and stay.
3. The env knob was never exercised end to end. VERYFRONT_PROVIDER_STREAM_IDLE_TIMEOUT_MS
   is this PR's entire answer to the issue's "configurable" item, and every
   test either called the resolver directly with an injected reader or passed
   an explicit idleTimeoutMs. Replacing the resolver call inside
   requestStream with a plain default left all of them green while the knob
   silently stopped working. "bounds a stalled body at the host environment
   override" now drives requestStream with nothing but the environment set.
4. The keepalive rationale over-claimed its evidence. The ping-frame and
   15s-gateway evidence only ever covered Anthropic and Veryfront Cloud;
   ext-llm-openai and ext-llm-google call requestStream with no
   idleTimeoutMs and decode no heartbeat. The comment and the CHANGELOG now
   say that for a directly-configured OpenAI or Google model the argument rests
   on the transport emitting something, not on a documented interval.

Also fixed in this round

• Merge conflict resolved. mergeable was CONFLICTING; main had added
  its own ## Unreleased section. Rebased onto dfd95e9dd8 and kept both
  entries. mergeable is now MERGEABLE. The diff is byte-identical across
  the rebase (799 insertions, 3 deletions before and after).
• The "filed separately" claim was unbacked. The body said the
  outbound-fetch.ts egress gap was "filed on its own" — no such issue
  existed. It is now really filed, as
  veryfront/veryfront-issue-inbox#1623, with the reachability argument written
  out. This is the item the audit called the most valuable thing any reviewer
  found, and it is correctly not held against this PR: different file,
  different subsystem, untouched by this diff.
• The knob is now documented. Added a "Provider stream idle deadline"
  section to docs/guides/configuration.md with the default, the 0-disables
  rule and the host-vs-.env rule. The generated api-reference row states none
  of those.
• Three stale claims in the body corrected: the api-reference adds two
  generated lines, not one; lint:platform is 119 pre-existing findings, not
  117 (main moved); added the configuration.md fmt gate.

Deliberately deferred

The audit's remaining non-blocking items are the two logging-policy gaps, now
recorded in the PR body under "Deliberately not addressed here". They are
not fixed, and I did not want to pick between them unilaterally — see the
decision below.

The retryable-comment defect and the missing docs entry, also non-blocking,
are fixed.

What I need from you

Decision 1 — the resolver's logging policy. Pick one; I can implement
whichever you choose.

• (A) Leave it. A malformed value re-warns on every stream request (no
  dedupe, so one typo emits a warning per model call for the process
  lifetime), and a well-formed value that came from a project .env is
  dropped with no log at any level. Zero new code, zero new state.
• (B) Cache the resolved value at module scope. Fixes both at once — one
  read, one warning, one .env-ignored notice. But it contradicts the "read
  from the host environment on every stream request" contract now stated in
  both the JSDoc and the CHANGELOG, it breaks the new requestStream
  override test (which depends on the per-call read), and it makes a runtime
  Deno.env.set inert.
• (C) Keep per-call reads, add a dedupe guard plus a one-time debug line
  when the .env-sourced value was skipped. Preserves the documented
  contract. Costs a second env read per call, module-level mutable state on a
  hot path, and new branches that need test coverage.

My read is (C) if you want it closed in this PR and (A) if you would rather
not grow the diff — but (B) is the one that reads as obviously correct until
you notice it contradicts shipped text, so I did not want it chosen by
default.

Decision 2 — ratify the corrected scope. This PR reached its original
premise by overruling the issue triage comment that had cited
chat-stream-handler.ts:89-90 correctly. That is now reversed. Someone who
owns this area should confirm the current framing — that agent.generate and
library embedders on the same path are the real motivation, and that a 120s
provider-silence bound underneath a stricter 15-60s consumer bound is the
intended layering — because it is the premise the release note now ships.

Decision 3 — draft status and merge. Leaving this as a draft and not
marking it ready for review. That is your call, not mine.

Verification

Run locally on the pushed head, against the rebased tree:

Gate	Result
test:file provider-http.test.ts	ok, 111 steps, 0 failed
test:file runtime-bridge.test.ts	ok, 49 steps, 0 failed
sweep src/provider/	ok, 30 passed / 348 steps
anthropic / openai / google extension tests	7 / 10 / 4 passed, 0 failed
deno check --no-lock on changed files	clean
deno fmt --check on all 8 changed files	clean
deno task lint	clean (5450 + 32 + 5 files)
docs, docs:api-reference:check	pass
lint:test-typecheck, lint:test-semantic-dispositions, lint:testing-front-door, lint:anti-slop, lint:style, lint:module-boundaries, lint:barrel-jsdoc	pass
lint:imports / lint:ban-deep-imports / lint:platform	86 / 3 / 119 — identical counts on origin/main, verified in a clean worktree; none in this PR's files

No test was weakened and no assertion was relaxed to pass anything. The one
assertion removed (finding 2) was removed because it encoded a false premise,
and the removal is argued in the test's own comment.

CI on 7c4d264, honestly stated: at the time of writing, 22 checks green
and none failed, including ci (lint), ci (format), ci (typecheck),
ci (test-layout), coverage shard 3/4, native executor coverage,
tests (Windows localhost routing), tests (rsc browser e2e),
tests (sentry runtime packages), tests (integration)'s peers,
tests (runtime critical flow: node), tests (node sandbox, minimum Node),
tests-proxy-binary, profile, npm smoke Node version contract and
npm compatibility artifact. Still running under runner contention when I
stopped waiting: coverage shards 1/2/4, tests (integration),
tests (binary e2e), tests (bun), the two node shard jobs, the remaining
runtime critical flow jobs and CodeQL Analyze (javascript-typescript).

I am therefore not claiming a fully green CI run — please confirm the
matrix before acting on this. The same matrix passed end to end on the
previous head (913fb55), and the only source changes since then are comment
text, one CHANGELOG section kept through a rebase, and a new
docs/guides/configuration.md section, so I expect it to pass — but expecting
is not verifying, and this PR has already been reported green once on evidence
that did not support it.

Sonar last reported Quality Gate passed (97.7% coverage on new code, 0
security hotspots, 2 new issues) on the head of 20 Sep. It re-runs off the
coverage shards, which had not finished, so there is no Sonar result for
7c4d264 yet. Automated review stays PENDING by design while this is a
draft.

[sonarqubecloud]

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
97.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[kwakayama]

Review score: 93/100\n\nReviewed head: \n\nThe provider stream body now has a bounded per-read idle deadline with explicit per-request and host-level configuration, aborts the upstream request on timeout, and preserves retryability/error context. The implementation correctly excludes project .env values from the host safety override and avoids logging malformed configured values. The latest source comments and tests also correct the earlier scope claim about existing chat watchdogs.\n\nValidation:\n- Focused provider HTTP and runtime bridge tests pass: 2 suites, 160 steps\n- CI, coverage, Sonar, CodeQL, lint, format, and typecheck pass\n- The two prior findings are fixed in source, but their review threads still need resolution\n- The PR is still draft, so the Automated review gate is waiting on readiness\n\nScore is 93/100. Resolve the addressed threads and promote to ready for review.

[kwakayama]

Review score correction: 93/100

Reviewed head: 7c4d2643bce4bbe690e8b908d3c2cc7b46f0ee3f

The provider stream body has a bounded per-read idle deadline with explicit per-request and host-level configuration, aborts the upstream request on timeout, and preserves retryability and error context. The implementation excludes project .env values from the host safety override and avoids logging malformed configured values.

Validation:

• Focused provider HTTP and runtime bridge tests pass: 2 suites, 160 steps
• CI, coverage, Sonar, CodeQL, lint, format, and typecheck pass
• The two prior findings are fixed in source, but their review threads still need resolution
• PR is still draft, so Automated review is waiting on readiness

The PR scores 93/100. Resolve the addressed threads and promote to ready for review.

[chatgpt-codex-connector]

You have reached your Codex usage limits for security reviews. Please try again later.

[greptile-apps]

Your trial has ended. Reactivate Greptile to resume code reviews.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	🔄 Running since 2026-09-22T06:36:39.945363Z	7c4d264	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[github-actions]

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Chef's kiss.

Reviewed commit: 7c4d2643bc

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".