From 63b4895d18bec5d3156711592d815e00d4c557a2 Mon Sep 17 00:00:00 2001
From: Juniper Alanna <201364921+juniper-shopify@users.noreply.github.com>
Date: Fri, 19 Jun 2026 14:26:01 -0400
Subject: [PATCH] Fix valid_provider! raising ArgumentError instead of
 InvalidConfigError
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

## Problem

Both `Chat::Config#valid_provider!` and `Agent::Config#valid_provider!`
raise `ArgumentError` when given an invalid provider name, despite their
doc comments explicitly promising `InvalidConfigError`:

```ruby
# Note: this method will return the name of a valid provider
# or raise an `InvalidConfigError`.
def valid_provider!
  # ...
  raise ArgumentError, "'...' is not a valid provider..."
end
```

This means any caller writing `rescue InvalidConfigError` — following
the documented contract — will not catch invalid provider errors. The
exception escapes as an unhandled `ArgumentError` instead of flowing
through the `ConfigError` hierarchy like every other config validation.

## Fix

Changed both methods to raise `InvalidConfigError` (which they inherit
from `Cog::Config` via the class hierarchy). This is consistent with:

- `Chat::Config#valid_api_key!` (line 124), which already raises
  `InvalidConfigError` for missing API keys
- `Map::Config#valid_parallel!` (line 89)
- `Config#working_directory` (lines 297–298)
- All `Config#validate!` overrides across the cog tree

Updated the corresponding tests in both `chat/config_test.rb` and
`agent/config_test.rb` to assert `Cog::Config::InvalidConfigError`
instead of `ArgumentError`.

## Files changed

- `lib/roast/cogs/chat/config.rb` — line 70
- `lib/roast/cogs/agent/config.rb` — line 54
- `test/roast/cogs/chat/config_test.rb` — line 53
- `test/roast/cogs/agent/config_test.rb` — line 34
---
 lib/roast/cogs/agent/config.rb       | 2 +-
 lib/roast/cogs/chat/config.rb        | 2 +-
 test/roast/cogs/agent/config_test.rb | 2 +-
 test/roast/cogs/chat/config_test.rb  | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/roast/cogs/agent/config.rb b/lib/roast/cogs/agent/config.rb
index 8160e0e6..51aa8112 100644
--- a/lib/roast/cogs/agent/config.rb
+++ b/lib/roast/cogs/agent/config.rb
@@ -51,7 +51,7 @@ def use_default_provider!
         def valid_provider!
           provider = @values[:provider] || VALID_PROVIDERS.first
           unless VALID_PROVIDERS.include?(provider)
-            raise ArgumentError, "'#{provider}' is not a valid provider. Available providers include: #{VALID_PROVIDERS.join(", ")}"
+            raise InvalidConfigError, "'#{provider}' is not a valid provider. Available providers include: #{VALID_PROVIDERS.join(", ")}"
           end
 
           provider
diff --git a/lib/roast/cogs/chat/config.rb b/lib/roast/cogs/chat/config.rb
index 93acc83a..62cc7caf 100644
--- a/lib/roast/cogs/chat/config.rb
+++ b/lib/roast/cogs/chat/config.rb
@@ -67,7 +67,7 @@ def use_default_provider!
         def valid_provider!
           provider = @values[:provider] || PROVIDERS.keys.first
           unless PROVIDERS.include?(provider)
-            raise ArgumentError, "'#{provider}' is not a valid provider. Available providers include: #{PROVIDERS.keys.join(", ")}"
+            raise InvalidConfigError, "'#{provider}' is not a valid provider. Available providers include: #{PROVIDERS.keys.join(", ")}"
           end
 
           provider
diff --git a/test/roast/cogs/agent/config_test.rb b/test/roast/cogs/agent/config_test.rb
index c0e31814..80c27762 100644
--- a/test/roast/cogs/agent/config_test.rb
+++ b/test/roast/cogs/agent/config_test.rb
@@ -31,7 +31,7 @@ def setup
       test "valid_provider! raises on invalid provider" do
         @config.provider(:invalid_provider)
 
-        error = assert_raises(ArgumentError) do
+        error = assert_raises(Cog::Config::InvalidConfigError) do
           @config.valid_provider!
         end
 
diff --git a/test/roast/cogs/chat/config_test.rb b/test/roast/cogs/chat/config_test.rb
index 2445a5ac..68137d04 100644
--- a/test/roast/cogs/chat/config_test.rb
+++ b/test/roast/cogs/chat/config_test.rb
@@ -50,7 +50,7 @@ def setup
       test "valid_provider! raises on invalid provider" do
         @config.provider(:invalid_provider)
 
-        error = assert_raises(ArgumentError) do
+        error = assert_raises(Cog::Config::InvalidConfigError) do
           @config.valid_provider!
         end