fix(ai_chat): forward conversation_timeout on the start path - #84
Closed
grandcamel wants to merge 2 commits into
Closed
fix(ai_chat): forward conversation_timeout on the start path#84grandcamel wants to merge 2 commits into
grandcamel wants to merge 2 commits into
Conversation
prepare() puts conversation_timeout into the start params, but the HTTP dispatch rebuilt the create_conversation call with only id and config_url, silently dropping it. A gateway configured with conversation_timeout=900 told the browser 900 (via effective_timeout) while the service kept its own 3600 default — the page schedules its idle warning around a number the service never enforces. The chat path was unaffected (raw_post streams params verbatim), which kept the drift invisible for conversations opened by a first message. The regression test drives the real HTTP dispatch through the ASGI harness — prepare() was already correct, so a prepare()-level assertion would have passed with the bug in place. Verified red without the fix, green with it; it also pins the auto-create chat path so the two paths cannot drift apart again. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MgA3KeCEPMKMJVvroZY1wV
The config puts tests in scope on purpose ("a new untyped test fails the
gate"), so an unannotated test function reds TYPECHECK on main.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ChatGateway.prepare()putsconversation_timeoutinto thestartparams, but the HTTP dispatch inrouter()rebuilt thecreate_conversationcall with onlyidandconfig_url— silently dropping the timeout. A gateway configured withconversation_timeout=900would tell the browser"timeout": 900(viaeffective_timeout) while the service kept its own 3600-second default: the page schedules its "your next message starts a new conversation" warning around a number the service never enforces.The
chatpath was unaffected (it streamsparamsthrough verbatim viaraw_post), which made the drift invisible to any conversation opened by a first message rather than bystart.Fix
Forward
params.get("conversation_timeout")as the client'stimeout=kwarg on thecreate_conversationdispatch.AIChatClient.create_conversationalready accepted it and maps it onto the wire asconversation_timeout;Noneis skipped, so gateways without a configured timeout emit exactly what they did before.Test
test_start_forwards_the_configured_timeout_upstreamdrives the real HTTP dispatch (ASGI harness + recording stub service) rather thanprepare()—prepare()was already correct, so a unit test at that level would have passed with the bug in place. Verified red without the fix (KeyError: 'conversation_timeout'on the stub's received params), green with it. The test also pins the auto-createchatpath so the two paths can't drift apart again.Note: local FMT gate currently fails on pre-existing formatting in
gateway.py/function_result.py— fixed in the companion CI-gates PR; merge that one first.🤖 Generated with Claude Code
https://claude.ai/code/session_01MgA3KeCEPMKMJVvroZY1wV