From b9296574ae576ecd67f75b780ac3fdfa121a84e8 Mon Sep 17 00:00:00 2001 From: Szymon Kurcab Date: Fri, 18 Sep 2026 20:38:17 +0200 Subject: [PATCH] feat: let callers set headers and a per-call model There was no way to add a header. Not one. OpenRouter reads HTTP-Referer and X-Title for app attribution, tracing needs a correlation id, and a gateway in front of either may want a tenant header; all of it required replacing the transport or subclassing the provider to override #headers. The JS SDK takes headers per call and per client, and this took neither. Nor was there a way to use a second model. `model:` is fixed when the client is built, so asking jev-preview one question meant a second client with a second connection and a second copy of the config. Client.new takes headers:, ask takes model: and headers:. They merge provider, then client, then call, matched without regard to case so an override of "content-type" replaces "Content-Type" instead of travelling beside it. A per-call model resolves through the provider's aliases like any other and leaves client.model alone. Header names and values are checked for newlines. Net::HTTP rejects those with an ArgumentError raised from inside the request, which tells you nothing about which header did it; this says so at the call site. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 11 +++ README.md | 22 +++++ lib/ruby_decision_model/client.rb | 50 +++++++++- test/request_options_test.rb | 146 ++++++++++++++++++++++++++++++ 4 files changed, 224 insertions(+), 5 deletions(-) create mode 100644 test/request_options_test.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 846df76..50baf17 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,16 @@ # Changelog +## Unreleased + +- `Client.new` takes `headers:`, and `ask` takes `model:` and `headers:` for a + single call. There was no way to send an extra header at all, which ruled + out OpenRouter's `HTTP-Referer`/`X-Title` attribution and any tracing header, + and no way to use a second model without building a second client. Headers + merge provider, then client, then call, matched case-insensitively. +- Header names and values are validated. A newline in either used to surface as + an `ArgumentError` from inside `Net::HTTP` on the first request; it now + raises `ConfigurationError` naming the header. + ## 0.1.0 - 2026-09-18 Provider-neutral release. One `Client`, two providers behind it. diff --git a/README.md b/README.md index 79638d6..ee91010 100644 --- a/README.md +++ b/README.md @@ -78,6 +78,7 @@ RubyDecisionModel::Client.new( base_url: nil, # overrides the provider base URL timeout: 5, # open and read timeout in seconds retry: { max_retries: 2 }, # RetryPolicy or a Hash of overrides + headers: {}, # merged over the provider's headers transport: nil # see Transport ) @@ -87,6 +88,27 @@ client.model # => "jev-latest" (resolved after aliasing) Both providers send `User-Agent: ruby_decision_model/`. +### Per-request options + +`model:` and `headers:` can be set once on the client or overridden per call: + +```ruby +client = RubyDecisionModel::Client.new( + headers: { "HTTP-Referer" => "https://example.com", "X-Title" => "My App" } +) + +client.ask(state: state, questions: questions, + model: "jev-preview", + headers: { "X-Trace-Id" => trace_id }) +``` + +Headers merge provider, then client, then call, matched without regard to case, +so an override replaces a provider header rather than being sent beside it. A +per-call `model:` goes through the provider's aliases like any other and leaves +`client.model` alone. A header name or value carrying a newline raises +`ConfigurationError` naming the header, rather than an `ArgumentError` from +inside `Net::HTTP` on the first request. + ### Model aliases Each provider resolves a few friendly names to its own canonical model name. diff --git a/lib/ruby_decision_model/client.rb b/lib/ruby_decision_model/client.rb index 3457726..f0f09f9 100644 --- a/lib/ruby_decision_model/client.rb +++ b/lib/ruby_decision_model/client.rb @@ -17,7 +17,7 @@ class Client RETRYABLE_STATUSES = RetryPolicy::DEFAULT_STATUSES RETRYABLE_EXCEPTIONS = (RetryPolicy::TIMEOUT_EXCEPTIONS + RetryPolicy::CONNECTION_EXCEPTIONS).freeze - attr_reader :provider, :model, :retry_policy, :timeout + attr_reader :provider, :model, :retry_policy, :timeout, :headers # provider: :open_router, :typesafe, or a Providers::Base instance. When # nil, api_key: alone selects OpenRouter; otherwise the @@ -32,7 +32,7 @@ class Client # random: callable returning a Float in 0...1, used for backoff jitter. # clock: callable returning monotonic seconds, used for total_timeout. def initialize(provider: nil, api_key: nil, model: nil, base_url: nil, timeout: 5, - transport: nil, sleeper: ->(seconds) { sleep(seconds) }, retry: {}, + transport: nil, sleeper: ->(seconds) { sleep(seconds) }, retry: {}, headers: {}, random: -> { rand }, clock: -> { Process.clock_gettime(Process::CLOCK_MONOTONIC) }) @provider = resolve_provider(provider, api_key: api_key, base_url: base_url) unless @provider.api_key? @@ -41,6 +41,7 @@ def initialize(provider: nil, api_key: nil, model: nil, base_url: nil, timeout: end @model = @provider.resolve_model(model) + @headers = validate_headers(headers) @timeout = timeout @transport = transport || default_transport @sleeper = sleeper @@ -53,18 +54,57 @@ def base_url @provider.base_url end - def ask(state:, questions:) + # model: overrides the client's model for this call, aliases and all. + # headers: merged over the client's headers for this call. + def ask(state:, questions:, model: nil, headers: {}) raise RequestError, "questions must not be empty" if questions.nil? || questions.empty? - body = @provider.request_body(model: @model, state: state, questions: questions) + body = @provider.request_body( + model: model.nil? ? @model : @provider.resolve_model(model), state: state, questions: questions + ) status, response_body, response_headers = perform_with_retry( - url: @provider.url, headers: @provider.headers, body: body + url: @provider.url, headers: request_headers(headers), body: body ) handle_response(status, response_body, response_headers, questions) end private + # The provider's headers, then the client's, then this call's. Later wins, + # matched without regard to case so "content-type" replaces "Content-Type" + # rather than being sent alongside it. + def request_headers(per_call) + merged = @provider.headers.dup + [@headers, validate_headers(per_call)].each do |overrides| + overrides.each do |name, value| + merged.delete_if { |existing, _| existing.to_s.casecmp?(name.to_s) } + merged[name.to_s] = value + end + end + merged + end + + # A CR or LF in a header is rejected by Net::HTTP with an ArgumentError + # from somewhere deep in the request, long after the value was set. Say + # so here, where the caller can see which header it was. + def validate_headers(headers) + return {} if headers.nil? + + unless headers.is_a?(Hash) + raise ConfigurationError, "headers must be a Hash of name => value, got #{headers.class}" + end + + headers.each do |name, value| + if name.to_s.strip.empty? || name.to_s.match?(/[\r\n:]/) + raise ConfigurationError, "header name #{name.inspect} is blank or contains a separator" + end + + raise ConfigurationError, "header #{name} contains a newline" if value.to_s.match?(/[\r\n]/) + end + + headers + end + def resolve_provider(provider, api_key:, base_url:) case provider when Providers::Base diff --git a/test/request_options_test.rb b/test/request_options_test.rb new file mode 100644 index 0000000..f67e67a --- /dev/null +++ b/test/request_options_test.rb @@ -0,0 +1,146 @@ +# frozen_string_literal: true + +require "test_helper" + +# Headers and model, set once on the client or per call. +class RequestOptionsTest < Minitest::Test + def questions + { "urgent" => RubyDecisionModel::Questions.noul("Is this urgent?") } + end + + def success_body + JSON.generate("answers" => { "urgent" => { "type" => "noul", "noul" => 0.5 } }, "usage" => {}) + end + + def transport + @transport ||= FakeTransport.new([[200, success_body]]) + end + + def build_client(**options) + RubyDecisionModel::Client.new( + **{ api_key: "test-key", transport: transport, sleeper: no_sleep }.merge(options) + ) + end + + def sent + transport.calls.first + end + + def sent_headers + sent[:headers] + end + + def sent_model + JSON.parse(sent[:body])["model"] + end + + # --- headers --- + + def test_the_provider_headers_are_sent_by_default + build_client.ask(state: {}, questions: questions) + + assert_equal "Bearer test-key", sent_headers["Authorization"] + assert_equal "application/json", sent_headers["Content-Type"] + assert_match(%r{\Aruby_decision_model/}, sent_headers["User-Agent"]) + end + + def test_client_headers_are_added + # OpenRouter reads these for app attribution. + build_client(headers: { "HTTP-Referer" => "https://example.com", "X-Title" => "My App" }) + .ask(state: {}, questions: questions) + + assert_equal "https://example.com", sent_headers["HTTP-Referer"] + assert_equal "My App", sent_headers["X-Title"] + assert_equal "Bearer test-key", sent_headers["Authorization"] + end + + def test_per_call_headers_are_added + build_client.ask(state: {}, questions: questions, headers: { "X-Trace-Id" => "abc" }) + + assert_equal "abc", sent_headers["X-Trace-Id"] + end + + def test_per_call_headers_win_over_client_headers + build_client(headers: { "X-Tenant" => "default" }) + .ask(state: {}, questions: questions, headers: { "X-Tenant" => "acme" }) + + assert_equal "acme", sent_headers["X-Tenant"] + end + + def test_an_override_replaces_rather_than_duplicates_a_provider_header + build_client(headers: { "user-agent" => "my-app/1.0" }).ask(state: {}, questions: questions) + + agents = sent_headers.select { |name, _| name.to_s.casecmp?("user-agent") } + assert_equal 1, agents.size + assert_equal "my-app/1.0", agents.values.first + end + + def test_the_client_headers_hash_is_readable + assert_equal({ "X-Title" => "t" }, build_client(headers: { "X-Title" => "t" }).headers) + assert_empty build_client.headers + end + + def test_headers_do_not_leak_between_calls + transport = FakeTransport.new([[200, success_body], [200, success_body]]) + client = RubyDecisionModel::Client.new(api_key: "k", transport: transport, sleeper: no_sleep) + + client.ask(state: {}, questions: questions, headers: { "X-Once" => "1" }) + client.ask(state: {}, questions: questions) + + assert_equal "1", transport.calls[0][:headers]["X-Once"] + refute transport.calls[1][:headers].key?("X-Once") + end + + # --- header values Net::HTTP would choke on --- + + def test_a_header_value_with_a_newline_is_refused + error = assert_raises(RubyDecisionModel::ConfigurationError) do + build_client(headers: { "X-Thing" => "a\r\nX-Injected: yes" }) + end + assert_match(/contains a newline/, error.message) + end + + def test_a_blank_or_malformed_header_name_is_refused + assert_raises(RubyDecisionModel::ConfigurationError) { build_client(headers: { "" => "v" }) } + assert_raises(RubyDecisionModel::ConfigurationError) { build_client(headers: { "X: Y" => "v" }) } + end + + def test_headers_must_be_a_hash + assert_raises(RubyDecisionModel::ConfigurationError) { build_client(headers: [%w[a b]]) } + end + + def test_a_bad_per_call_header_is_refused_before_the_request + client = build_client + assert_raises(RubyDecisionModel::ConfigurationError) do + client.ask(state: {}, questions: questions, headers: { "X" => "a\nb" }) + end + assert_empty transport.calls + end + + # --- model --- + + def test_the_client_model_is_sent_by_default + build_client.ask(state: {}, questions: questions) + + assert_equal "typesafe/jev-1.13", sent_model + end + + def test_a_per_call_model_overrides_it + build_client.ask(state: {}, questions: questions, model: "some/other-model") + + assert_equal "some/other-model", sent_model + end + + def test_a_per_call_model_goes_through_the_providers_aliases + build_client(provider: :typesafe).ask(state: {}, questions: questions, model: "jev") + + assert_equal "jev-latest", sent_model + end + + def test_a_per_call_model_does_not_change_the_client + client = build_client + client.ask(state: {}, questions: questions, model: "other") + + assert_equal "typesafe/jev-1.13", client.model + end +end