Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions apps/harness/lib/harness/model_discovery.ex
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,10 @@ defmodule Harness.ModelDiscovery do
end
end

defp do_fetch(provider) do
{:error, "No model discovery for provider: #{provider}"}
end

defp try_http_model_discovery do
# Find any active OpenCode session and query models through it
sessions = Harness.SessionManager.list_sessions()
Expand All @@ -127,10 +131,6 @@ defmodule Harness.ModelDiscovery do
end
end

defp do_fetch(provider) do
{:error, "No model discovery for provider: #{provider}"}
end

defp fetch_from_cli(binary, args, parser) do
try do
case System.cmd(binary, args, stderr_to_stdout: true) do
Expand Down
9 changes: 9 additions & 0 deletions apps/harness/lib/harness/providers/claude_session.ex
Original file line number Diff line number Diff line change
Expand Up @@ -606,6 +606,15 @@ defmodule Harness.Providers.ClaudeSession do
"cwd" => get_in(msg, ["cwd"])
})

# Claude CLI includes mcp_servers in system/init just like Codex.
# Normalize to the same event method so the mapping pipeline is uniform.
for %{"name" => name, "status" => status} <- Map.get(msg, "mcp_servers", []) do
emit_event(state, :notification, "mcpServer/startupStatus/updated", %{
"name" => name,
"status" => status
})
end
Comment on lines +611 to +616

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

cat -n apps/harness/lib/harness/providers/claude_session.ex | sed -n '605,620p'

Repository: Ranvier-Technologies/t3code-OTP

Length of output: 665


🏁 Script executed:

cd apps/harness && head -20 lib/harness/providers/claude_session.ex

Repository: Ranvier-Technologies/t3code-OTP

Length of output: 971


🏁 Script executed:

cat -n apps/harness/lib/harness/providers/claude_session.ex | sed -n '550,650p'

Repository: Ranvier-Technologies/t3code-OTP

Length of output: 3842


🏁 Script executed:

cat > /tmp/test_map_get.exs << 'EOF'
# Test Map.get behavior with nil values
msg1 = %{"mcp_servers" => nil}
msg2 = %{}

result1 = Map.get(msg1, "mcp_servers", [])
result2 = Map.get(msg2, "mcp_servers", [])

IO.inspect(result1, label: "Map.get with nil value")
IO.inspect(result2, label: "Map.get with missing key")

# Test if for loop crashes with nil
IO.puts("\nTesting for loop with nil:")
try do
  result = for item <- nil, do: item
  IO.inspect(result)
rescue
  error -> IO.inspect(error, label: "Exception")
end
EOF
elixir /tmp/test_map_get.exs

Repository: Ranvier-Technologies/t3code-OTP

Length of output: 122


🏁 Script executed:

cat -n apps/harness/lib/harness/providers/claude_session.ex | sed -n '540,600p'

Repository: Ranvier-Technologies/t3code-OTP

Length of output: 2186


🏁 Script executed:

rg -n "handle_sdk_message\|jason\|JSON" apps/harness/lib/harness/providers/claude_session.ex | head -20

Repository: Ranvier-Technologies/t3code-OTP

Length of output: 57


Guard mcp_servers before iterating to avoid GenServer crashes.

Line 611 iterates directly over Map.get(msg, "mcp_servers", []), which crashes if the key exists with a nil value. Since this is external JSON from the Claude CLI, it can contain "mcp_servers": null. Use a case guard to safely check the type before iteration.

Suggested fix
-    for %{"name" => name, "status" => status} <- Map.get(msg, "mcp_servers", []) do
-      emit_event(state, :notification, "mcpServer/startupStatus/updated", %{
-        "name" => name,
-        "status" => status
-      })
-    end
+    mcp_servers =
+      case Map.get(msg, "mcp_servers") do
+        servers when is_list(servers) -> servers
+        _ -> []
+      end
+
+    Enum.each(mcp_servers, fn
+      %{"name" => name, "status" => status} when is_binary(name) and is_binary(status) ->
+        emit_event(state, :notification, "mcpServer/startupStatus/updated", %{
+          "name" => name,
+          "status" => status
+        })
+
+      _ ->
+        :ok
+    end)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
for %{"name" => name, "status" => status} <- Map.get(msg, "mcp_servers", []) do
emit_event(state, :notification, "mcpServer/startupStatus/updated", %{
"name" => name,
"status" => status
})
end
mcp_servers =
case Map.get(msg, "mcp_servers") do
servers when is_list(servers) -> servers
_ -> []
end
Enum.each(mcp_servers, fn
%{"name" => name, "status" => status} when is_binary(name) and is_binary(status) ->
emit_event(state, :notification, "mcpServer/startupStatus/updated", %{
"name" => name,
"status" => status
})
_ ->
:ok
end)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/harness/lib/harness/providers/claude_session.ex` around lines 611 - 616,
The code iterates over Map.get(msg, "mcp_servers", []) without guarding for a
nil value, which can crash when the external JSON contains "mcp_servers": null;
change the logic around the for-comprehension to first fetch Map.get(msg,
"mcp_servers") and pattern-match or use a case to only run the for when the
value is a list (e.g., case Map.get(msg, "mcp_servers") do servers when
is_list(servers) -> for %{"name" => name, "status" => status} <- servers do
emit_event(state, :notification, "mcpServer/startupStatus/updated", %{"name" =>
name, "status" => status}) end; _ -> :ok end), so only lists are iterated and
nil or other types are ignored; reference Map.get(msg, "mcp_servers"), the for
comprehension, and emit_event to locate the change.


state
end

Expand Down
111 changes: 77 additions & 34 deletions apps/harness/lib/harness/providers/opencode_session.ex
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,11 @@ defmodule Harness.Providers.OpenCodeSession do
case fetch_server_messages(state) do
{:ok, msgs} when msgs != [] ->
turns = server_messages_to_turns(msgs)
Logger.info("Hydrated #{length(turns)} turns from server for thread #{state.thread_id}")

Logger.info(
"Hydrated #{length(turns)} turns from server for thread #{state.thread_id}"
)

%{state | messages: turns}

_ ->
Expand Down Expand Up @@ -539,18 +543,12 @@ defmodule Harness.Providers.OpenCodeSession do

@impl true
def handle_call(:mcp_status, _from, state) do
result =
case http_get("#{state.base_url}/mcp") do
{:ok, data} -> {:ok, data}
{:error, reason} -> {:error, reason}
end

{:reply, result, state}
{:reply, http_get("#{state.base_url}/mcp"), state}
end

@impl true
def handle_call({:mcp_add, name, config}, _from, state) do
body = Map.merge(%{"name" => name}, config)
body = %{"name" => name, "config" => config}

result =
case http_post("#{state.base_url}/mcp", body) do
Expand All @@ -563,8 +561,10 @@ defmodule Harness.Providers.OpenCodeSession do

@impl true
def handle_call({:mcp_connect, name}, _from, state) do
encoded_name = encode_path_segment(name)

result =
case http_post("#{state.base_url}/mcp/#{name}/connect", %{}) do
case http_post("#{state.base_url}/mcp/#{encoded_name}/connect", %{}) do
{:ok, _} -> :ok
{:error, reason} -> {:error, reason}
end
Expand All @@ -574,8 +574,10 @@ defmodule Harness.Providers.OpenCodeSession do

@impl true
def handle_call({:mcp_disconnect, name}, _from, state) do
encoded_name = encode_path_segment(name)

result =
case http_post("#{state.base_url}/mcp/#{name}/disconnect", %{}) do
case http_post("#{state.base_url}/mcp/#{encoded_name}/disconnect", %{}) do
{:ok, _} -> :ok
{:error, reason} -> {:error, reason}
end
Expand Down Expand Up @@ -1340,9 +1342,14 @@ defmodule Harness.Providers.OpenCodeSession do

defp http_delete(url) do
case Req.request(method: :delete, url: url, receive_timeout: 10_000) do
{:ok, %{status: status}} when status in 200..204 -> {:ok, %{}}
{:ok, %{status: status, body: body}} -> {:error, "HTTP #{status}: #{inspect(body) |> String.slice(0, 200)}"}
{:error, reason} -> {:error, inspect(reason)}
{:ok, %{status: status}} when status in 200..204 ->
{:ok, %{}}

{:ok, %{status: status, body: body}} ->
{:error, "HTTP #{status}: #{inspect(body) |> String.slice(0, 200)}"}

{:error, reason} ->
{:error, inspect(reason)}
end
end

Expand Down Expand Up @@ -1444,32 +1451,33 @@ defmodule Harness.Providers.OpenCodeSession do
end

# Fetch providers from the OpenCode server's GET /provider endpoint.
# Returns {:ok, provider_map} or {:error, reason}.
# Returns {:ok, provider_list} or {:error, reason}.
defp fetch_providers(state) do
case http_get("#{state.base_url}/provider") do
{:ok, %{"all" => providers}} when is_map(providers) ->
{:ok, %{"all" => providers}} when is_list(providers) ->
{:ok, providers}

{:ok, providers} when is_map(providers) ->
{:ok, providers} when is_list(providers) ->
{:ok, providers}
Comment on lines 1455 to 1461

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🌐 Web query:

In Req (Elixir), when a JSON response body is a top-level array, is response.body decoded into an Elixir list by default?

💡 Result:

Yes, in Req (Elixir), when a JSON response body is a top-level array and the Content-Type header indicates application/json, response.body is decoded into an Elixir list by default. Req's decode_body response step automatically detects the format from the content-type header and uses Jason.decode!/2 to parse JSON bodies. Jason decodes JSON arrays to Elixir lists [1,2,3] -> [1,2,3]. This is enabled by default (decode_body: true).

Citations:


🏁 Script executed:

cd apps/harness && sed -n '1330,1341p' lib/harness/providers/opencode_session.ex

Repository: Ranvier-Technologies/t3code-OTP

Length of output: 495


The new list-response branch is unreachable with the current http_get/1 implementation.

Line 1331-1334 checks only for maps; when Req decodes a JSON array from /provider, it returns an Elixir list. Since is_map(body) is false for lists, the code calls Jason.decode(to_string(body)). The to_string/1 call converts the list to its string representation (e.g., "[%{...}]"), which is not valid JSON. The subsequent Jason.decode/1 call will raise an error, preventing fetch_providers/1 from ever receiving the decoded list. This breaks provider discovery on any endpoint returning a JSON array.

Fix: Update the condition to if is_map(body) or is_list(body), do: {:ok, body}, else: Jason.decode(to_string(body)) to pass through already-decoded lists.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/harness/lib/harness/providers/opencode_session.ex` around lines 1455 -
1461, fetch_providers/1 never receives decoded JSON arrays because http_get/1
only treats maps as already-decoded; when Req returns a list body it falls
through to Jason.decode(to_string(body)) which fails. Modify http_get/1 so it
treats lists like maps: change the branch that currently checks is_map(body) to
check is_map(body) or is_list(body) (i.e., if is_map(body) or is_list(body), do:
{:ok, body}, else: Jason.decode(to_string(body))) so that http_get/1 returns
{:ok, body} for already-decoded lists and fetch_providers/1 can handle the
list-response branch.


{:ok, _} ->
{:ok, %{}}
{:ok, []}

{:error, reason} ->
{:error, reason}
end
end

# Extract a flat list of %{"slug" => "provider/model", "name" => "..."} from the
# provider map returned by GET /provider.
defp extract_models_from_providers(providers) when is_map(providers) do
Enum.flat_map(providers, fn {provider_key, provider_data} ->
# provider list returned by GET /provider.
defp extract_models_from_providers(providers) when is_list(providers) do
Enum.flat_map(providers, fn provider_data ->
provider_id = Map.get(provider_data, "id")
models = Map.get(provider_data, "models", %{})

if is_map(models) do
if is_binary(provider_id) and is_map(models) do
Enum.map(models, fn {model_key, model_data} ->
slug = "#{provider_key}/#{model_key}"
slug = "#{provider_id}/#{model_key}"
name = Map.get(model_data, "name", model_key)
%{"slug" => slug, "name" => name}
end)
Expand All @@ -1496,7 +1504,10 @@ defmodule Harness.Providers.OpenCodeSession do
{:ok, []}

{:error, reason} ->
Logger.warning("Failed to fetch server messages for session #{state.opencode_session_id}: #{inspect(reason)}")
Logger.warning(
"Failed to fetch server messages for session #{state.opencode_session_id}: #{inspect(reason)}"
)

{:ok, []}
end
else
Expand All @@ -1508,11 +1519,11 @@ defmodule Harness.Providers.OpenCodeSession do
# used by read_thread. Filters for assistant-role messages only.
defp server_messages_to_turns(messages) when is_list(messages) do
messages
|> Enum.filter(fn msg -> Map.get(msg, "role") == "assistant" end)
|> Enum.filter(fn msg -> get_in(msg, ["info", "role"]) == "assistant" end)
|> Enum.map(fn msg ->
%{
turn_id: Map.get(msg, "id", generate_id()),
started_at: Map.get(msg, "createdAt", now_iso()),
turn_id: get_in(msg, ["info", "id"]) || generate_id(),
started_at: server_message_started_at(msg),
items:
(Map.get(msg, "parts", []) || [])
|> Enum.flat_map(fn part ->
Expand All @@ -1522,11 +1533,13 @@ defmodule Harness.Providers.OpenCodeSession do
if text != "", do: [%{"type" => "text", "text" => text}], else: []

"tool" ->
[%{
"type" => "tool",
"tool" => Map.get(part, "tool", "unknown"),
"state" => Map.get(part, "state", %{})
}]
[
%{
"type" => "tool",
"tool" => Map.get(part, "tool", "unknown"),
"state" => Map.get(part, "state", %{})
}
]

_ ->
[]
Expand All @@ -1542,10 +1555,14 @@ defmodule Harness.Providers.OpenCodeSession do
if state.opencode_session_id do
case http_delete("#{state.base_url}/session/#{state.opencode_session_id}") do
{:ok, _} ->
Logger.info("Deleted OpenCode session #{state.opencode_session_id} for thread #{state.thread_id}")
Logger.info(
"Deleted OpenCode session #{state.opencode_session_id} for thread #{state.thread_id}"
)

{:error, reason} ->
Logger.warning("Failed to delete OpenCode session #{state.opencode_session_id}: #{inspect(reason)}")
Logger.warning(
"Failed to delete OpenCode session #{state.opencode_session_id}: #{inspect(reason)}"
)
end
end
end
Expand Down Expand Up @@ -1591,6 +1608,32 @@ defmodule Harness.Providers.OpenCodeSession do
defp turn_id_from_state(%{turn_state: %{turn_id: turn_id}}), do: turn_id
defp turn_id_from_state(_), do: nil

defp server_message_started_at(message) do
message
|> get_in(["info", "time", "created"])
|> unix_timestamp_to_iso()
end

defp unix_timestamp_to_iso(timestamp) when is_integer(timestamp) do
{value, unit} =
cond do
timestamp >= 100_000_000_000_000 -> {timestamp, :microsecond}
timestamp >= 100_000_000_000 -> {timestamp, :millisecond}
true -> {timestamp, :second}
end

case DateTime.from_unix(value, unit) do
{:ok, datetime} -> DateTime.to_iso8601(datetime)
_ -> now_iso()
end
end

defp unix_timestamp_to_iso(_), do: now_iso()

defp encode_path_segment(value) when is_binary(value) do
URI.encode(value, &URI.char_unreserved?/1)
end

defp persist_binding(state) do
# Only persist durable identifiers — port is ephemeral and stale after restart
cursor_json =
Expand Down
20 changes: 8 additions & 12 deletions apps/harness/lib/harness/session_manager.ex
Original file line number Diff line number Diff line change
Expand Up @@ -255,19 +255,15 @@ defmodule Harness.SessionManager do
Falls back to {:error, reason} if no session is running or the call fails.
"""
def list_models_from_session(thread_id) do
case Registry.lookup(Harness.SessionRegistry, thread_id) do
[{pid, "opencode"}] ->
try do
GenServer.call(pid, :list_models, 15_000)
catch
:exit, reason -> {:error, "GenServer call failed: #{inspect(reason)}"}
end

[{_pid, other}] ->
with_opencode_session(thread_id, fn pid ->
GenServer.call(pid, :list_models, 15_000)
end)
|> case do
{:error, {:provider_mismatch, other}} ->
{:error, "list_models_from_session only supports opencode, got: #{other}"}

[] ->
{:error, "Session not found: #{thread_id}"}
other ->
other
end
end

Expand Down Expand Up @@ -324,7 +320,7 @@ defmodule Harness.SessionManager do
end

[{_pid, other}] ->
{:error, "MCP management only supports opencode, got: #{other}"}
{:error, {:provider_mismatch, other}}

[] ->
{:error, "Session not found: #{thread_id}"}
Expand Down
30 changes: 30 additions & 0 deletions apps/harness/lib/harness_web/harness_channel.ex
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,11 @@ defmodule HarnessWeb.HarnessChannel do
end
end

@impl true
def handle_in("mcp.status", _params, socket) do
{:reply, {:error, %{message: "Missing required param: threadId"}}, socket}
end
Comment on lines +216 to +219

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

These fallback clauses still let JSON null through.

The earlier heads on Line 206, Line 222, Line 245, and Line 262 already match %{"threadId" => nil} / %{"name" => nil} / %{"config" => nil}, so these new handlers only fire when the key is absent, not when the client sends null. That still forwards malformed MCP commands into SessionManager instead of returning the intended validation error. Guard the success clauses with when not is_nil(...) or switch them to the same with-style validation used in the session handlers.

Also applies to: 232-276

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/harness/lib/harness_web/harness_channel.ex` around lines 216 - 219, The
fallback handle_in clauses currently match absent keys but still allow JSON
nulls through (e.g., the handler def handle_in("mcp.status", _params, socket)
lets %{"threadId" => nil} slip); change the success clause signatures to
explicitly bind and guard non-nil values (for example: def
handle_in("mcp.status", %{"threadId" => threadId} = params, socket) when not
is_nil(threadId) do ...) or convert those handlers to the same with-style
validation used elsewhere so that %{"threadId" => nil} / %{"name" => nil} /
%{"config" => nil} are rejected and routed to the existing nil-match error
clauses instead; apply the same pattern to the other MCP handlers (the prior
success clauses around session creation/update/status) so null values are
rejected, not forwarded to SessionManager.


@impl true
def handle_in("mcp.add", %{"threadId" => thread_id, "name" => name, "config" => config}, socket) do
case SessionManager.mcp_add(thread_id, name, config) do
Expand All @@ -224,6 +229,18 @@ defmodule HarnessWeb.HarnessChannel do
end
end

@impl true
def handle_in("mcp.add", params, socket) do
missing =
cond do
is_nil(Map.get(params, "threadId")) -> "threadId"
is_nil(Map.get(params, "name")) -> "name"
true -> "config"
end

{:reply, {:error, %{message: "Missing required param: #{missing}"}}, socket}
end

@impl true
def handle_in("mcp.connect", %{"threadId" => thread_id, "name" => name}, socket) do
case SessionManager.mcp_connect(thread_id, name) do
Expand All @@ -235,6 +252,12 @@ defmodule HarnessWeb.HarnessChannel do
end
end

@impl true
def handle_in("mcp.connect", params, socket) do
missing = if is_nil(Map.get(params, "threadId")), do: "threadId", else: "name"
{:reply, {:error, %{message: "Missing required param: #{missing}"}}, socket}
end

@impl true
def handle_in("mcp.disconnect", %{"threadId" => thread_id, "name" => name}, socket) do
case SessionManager.mcp_disconnect(thread_id, name) do
Expand All @@ -246,6 +269,12 @@ defmodule HarnessWeb.HarnessChannel do
end
end

@impl true
def handle_in("mcp.disconnect", params, socket) do
missing = if is_nil(Map.get(params, "threadId")), do: "threadId", else: "name"
{:reply, {:error, %{message: "Missing required param: #{missing}"}}, socket}
end

# --- Snapshot ---

@impl true
Expand Down Expand Up @@ -303,5 +332,6 @@ defmodule HarnessWeb.HarnessChannel do
end

defp format_error(reason) when is_binary(reason), do: reason
defp format_error({:provider_mismatch, other}), do: "Only supports opencode, got: #{other}"
defp format_error(reason), do: inspect(reason)
end
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,10 @@ function createProviderServiceHarness(
mcpConfig: "basic" as const,
}),
rollbackConversation,
mcpStatus: () => Effect.succeed({}),
mcpAdd: () => Effect.succeed({}),
mcpConnect: () => Effect.void,
mcpDisconnect: () => Effect.void,
streamEvents: Stream.fromPubSub(runtimeEventPubSub),
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,10 @@ describe("ProviderCommandReactor", () => {
mcpConfig: "basic",
}),
rollbackConversation: () => unsupported(),
mcpStatus: () => Effect.succeed({}),
mcpAdd: () => Effect.succeed({}),
mcpConnect: () => Effect.void,
mcpDisconnect: () => Effect.void,
streamEvents: Stream.fromPubSub(runtimeEventPubSub),
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,10 @@ function createProviderServiceHarness() {
mcpConfig: "basic" as const,
}),
rollbackConversation: () => unsupported(),
mcpStatus: () => Effect.succeed({}),
mcpAdd: () => Effect.succeed({}),
mcpConnect: () => Effect.void,
mcpDisconnect: () => Effect.void,
streamEvents: Stream.fromPubSub(runtimeEventPubSub),
};

Expand Down
Loading
Loading