Skip to content

fix(server): configurable /chat+ACP turn timeout; no 504-while-billing (#359) - #376

Open
gnanirahulnutakki wants to merge 1 commit into
devfrom
fix/359-http-turn-timeout
Open

fix(server): configurable /chat+ACP turn timeout; no 504-while-billing (#359)#376
gnanirahulnutakki wants to merge 1 commit into
devfrom
fix/359-http-turn-timeout

Conversation

@gnanirahulnutakki

Copy link
Copy Markdown
Member

Root cause

The synchronous POST /chat and ACP HTTP handlers wrapped state.submit_chat in a hardcoded 30s tokio::time::timeout (HTTP_TURN_TIMEOUT in routes.rs).

submit_chat enqueues the turn onto a detached worker thread (which owns the !Send fused runtime) over a bounded mpsc, then awaits a oneshot reply. When the 30s wait elapsed:

  1. the handler returned 504 and dropped the submit_chat future (dropping the oneshot receiver), but
  2. the worker's handle_http had already called runtime.submit().await and ran the turn to completion regardless — committing a signed receipt and billing cost — then failed the reply.send(...) silently.

Result: the client saw a 504, but the turn succeeded, minted a receipt, and billed. The streaming path (handle_http_stream) already guards against this with tokio::select! on events.closed(); the synchronous path had no equivalent.

Fix (tightest scope)

  • Configurable ceilingARDUR_HTTP_TURN_TIMEOUT_SECS (default 30, rejects 0/unparseable at boot), surfaced as Config.http_turn_timeoutAppState::http_turn_timeout(). Both handlers read it instead of the const.
  • Billing aligned with what the client was toldhandle_http now races runtime.submit against reply.closed() (biased toward the completion arm), mirroring the streaming path's events.closed() guard. When the client-facing timeout fires (or the caller hangs up), the submit future is dropped before it commits the receipt/journal/cost side effects (all at .await points inside submit_inner), so a 504 is never paired with a silently-billed turn. A turn that finishes exactly at the deadline still reports its already-committed outcome (biased select).

Verification

  • New regression test crates/server/tests/chat_turn_timeout.rs:
    • a turn that outruns a tiny timeout returns 504 and mints zero receipts — fails without the fix (observed count == 1, i.e. billed under a 504), passes with it;
    • a turn within budget returns 200 and mints exactly one receipt (guards against over-cancellation).
  • cargo test -p ardur-server ✅ · cargo test -p ardur-e2e-tests
  • cargo clippy --all-targets --all-features -- -D warnings ✅ · cargo fmt --check

RUN.md documents the new knob.

Fixes #359

#359)

The synchronous POST /chat and ACP HTTP handlers wrapped submit_chat in a
hardcoded 30s tokio::time::timeout. submit_chat enqueues the turn onto a
detached worker thread (which owns the !Send fused runtime) and awaits a
oneshot. When the 30s wait elapsed, the handler returned 504 and dropped
the oneshot receiver — but the worker's handle_http ran runtime.submit()
to completion regardless, committing a receipt and billing cost for a reply
the client never received. A misleading failure with real billing impact.

Fix (tightest scope):
- Make the ceiling configurable: ARDUR_HTTP_TURN_TIMEOUT_SECS (default 30,
  rejects 0/unparseable at boot), surfaced as Config.http_turn_timeout and
  AppState::http_turn_timeout(); both handlers now read it instead of a const.
- Align billing with what the client was told: handle_http now races
  runtime.submit against reply.closed() (biased toward completion), mirroring
  the streaming path's events.closed() guard. When the client-facing timeout
  fires (or the caller hangs up), the submit future is dropped before it
  commits the receipt/journal/cost side effects (all at .await points inside
  submit_inner), so a 504 never pairs with a silently-billed turn.

Verification:
- New regression test crates/server/tests/chat_turn_timeout.rs: a turn that
  outruns a tiny timeout returns 504 and mints zero receipts (fails pre-fix
  with count==1); a turn within budget returns 200 and mints one.
- cargo test -p ardur-server, cargo test -p ardur-e2e-tests, clippy
  --all-targets --all-features -D warnings, and fmt --check all green.

Fixes #359

Checkpoint: architect/sessions/http-turn-timeout-359-2026-07-23/journal.md
Signed-off-by: GR <gnanirn@gmail.com>
@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: f387ebcf-a126-4207-be39-37e391467906

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/359-http-turn-timeout

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant