fix(dispatch): failed fleet dispatch reports failure, not success

lib/fleet_dispatcher.ex

@@ -443,8 +443,14 @@ defmodule Hypatia.FleetDispatcher do
     end
   end
 
-  # Build the submitProofObligation mutation using the input-object syntax
-  # that ProofObligationInput expects. The prover field is omitted when
+  # Build the submitProofObligation mutation (input-object syntax).
+  #
+  # CONTRACT GAP (2026-10-07): echidnabot's MutationRoot (src/api/graphql.rs)
+  # has no submitProofObligation and no ProofObligationInput; it only offers
+  # triggerCheck(repoId, commitSha, provers) on a registered repo. A live
+  # dispatch of this mutation therefore returns GraphQL `errors`, which
+  # execute_graphql/2 now reports as {:error, _}. Which side changes is an
+  # open owner decision. The prover field is omitted when
   # prover_hint is nil OR the hint names a prover echidnabot's GraphQL
   # schema doesn't know about (VeriSimDB tracks more provers than echidnabot
   # currently exposes). In either case echidnabot uses its own default.
@@ -756,6 +762,14 @@ defmodule Hypatia.FleetDispatcher do
   # Helpers
   # ====================================================================
 
+  # Dispatch a GraphQL document to a bot: append it to the dispatch manifest,
+  # then POST it live when a URL is configured.
+  #
+  # Returns `{:ok, :dispatched}` only when the live call succeeded (2xx and,
+  # on the per-bot path, no GraphQL `errors`), `{:ok, :file_dispatched}` only
+  # when no URL is configured and the manifest write succeeded, and
+  # `{:error, reason}` otherwise. A configured URL that fails is an error even
+  # though the manifest line was written: the caller asked for live dispatch.
   defp execute_graphql(query, bot_name) do
     # Dual dispatch: file-based (immediate) + HTTP (when fleet API available)
     dispatch_record = %{
@@ -766,21 +780,7 @@ defmodule Hypatia.FleetDispatcher do
     }
 
     # 1. Always write to dispatch manifest (dispatch-runner.sh reads this)
-    manifest_path =
-      Path.join([
-        Application.get_env(:hypatia, :verisimdb_data_path, "data/verisim"),
-        "dispatch",
-        "pending.jsonl"
-      ])
-
-    case Jason.encode(dispatch_record) do
-      {:ok, json} ->
-        File.mkdir_p!(Path.dirname(manifest_path))
-        File.write(manifest_path, json <> "\n", [:append, :utf8])
-
-      {:error, reason} ->
-        Logger.error("Failed to write dispatch manifest: #{inspect(reason)}")
-    end
+    manifest_result = write_manifest(dispatch_record)
 
     # 2. Attempt HTTP dispatch (graceful degradation if unavailable).
     #
@@ -795,28 +795,79 @@ defmodule Hypatia.FleetDispatcher do
     # deployments that front multiple bots behind one dispatcher.
     {target_url, description, body} = resolve_dispatch_url(bot_name, query)
 
-    if target_url do
-      try do
-        case http_post(target_url, body) do
-          {:ok, _response} ->
+    graphql_envelope? = description != :none and String.starts_with?(description, "via per-bot")
+
+    cond do
+      target_url ->
+        case live_post(target_url, body, graphql_envelope?) do
+          :ok ->
             Logger.info("Live dispatch to #{bot_name} succeeded (#{description})")
             {:ok, :dispatched}
 
           {:error, reason} ->
-            Logger.warning(
-              "Live dispatch to #{bot_name} failed (#{inspect(reason)}), file dispatch used"
-            )
-
-            {:ok, :file_dispatched}
+            Logger.error("Live dispatch to #{bot_name} failed (#{inspect(reason)})")
+            {:error, {:live_dispatch_failed, bot_name, reason}}
         end
-      rescue
-        e ->
-          Logger.warning("HTTP dispatch error: #{inspect(e)}, file dispatch used")
-          {:ok, :file_dispatched}
-      end
+
+      manifest_result == :ok ->
+        Logger.info("Dispatched to #{bot_name} via manifest (no URL configured)")
+        {:ok, :file_dispatched}
+
+      true ->
+        {:error, manifest_result}
+    end
+  end
+
+  # Append one dispatch record to the JSONL manifest that dispatch-runner.sh
+  # reads. Returns `:ok` or `{:manifest_write_failed, reason}`; never raises.
+  defp write_manifest(dispatch_record) do
+    manifest_path =
+      Path.join([
+        Application.get_env(:hypatia, :verisimdb_data_path, "data/verisim"),
+        "dispatch",
+        "pending.jsonl"
+      ])
+
+    with {:ok, json} <- Jason.encode(dispatch_record),
+         :ok <- File.mkdir_p(Path.dirname(manifest_path)),
+         :ok <- File.write(manifest_path, json <> "\n", [:append, :utf8]) do
+      :ok
     else
-      Logger.info("Dispatched to #{bot_name} via manifest (no URL configured)")
-      {:ok, :file_dispatched}
+      {:error, reason} ->
+        Logger.error("Failed to write dispatch manifest #{manifest_path}: #{inspect(reason)}")
+        {:manifest_write_failed, reason}
+    end
+  end
+
+  # POST a dispatch body and judge the response. On the per-bot GraphQL path
+  # a 2xx whose JSON body carries a non-empty `errors` list is a failure, as
+  # GraphQL-over-HTTP reports resolver errors with status 200. Exceptions and
+  # exits (e.g. :inets not started) become `{:error, _}` instead of crashing.
+  defp live_post(url, body, graphql_envelope?) do
+    case http_post(url, body) do
+      {:ok, _status, response_body} when graphql_envelope? ->
+        graphql_errors(response_body)
+
+      {:ok, _status, _response_body} ->
+        :ok
+
+      {:error, reason} ->
+        {:error, reason}
+    end
+  rescue
+    e -> {:error, {:exception, Exception.message(e)}}
+  catch
+    :exit, reason -> {:error, {:exit, reason}}
+  end
+
+  # Classify a GraphQL-over-HTTP response body: `:ok` when it decodes and has
+  # no non-empty `errors`, `{:error, _}` when it reports errors or is not JSON.
+  defp graphql_errors(response_body) do
+    case Jason.decode(response_body) do
+      {:ok, %{"errors" => [_ | _] = errors}} -> {:error, {:graphql_errors, errors}}
+      {:ok, %{}} -> :ok
+      {:ok, other} -> {:error, {:unexpected_graphql_body, other}}
+      {:error, _} -> {:error, {:non_json_graphql_body, String.slice(response_body, 0, 200)}}
     end
   end
 
@@ -846,6 +897,8 @@ defmodule Hypatia.FleetDispatcher do
     end
   end
 
+  # POST a JSON body. Returns `{:ok, status, body}` for a 2xx response,
+  # `{:error, {:http_status, status}}` for any other status, or the :httpc error.
   defp http_post(url, body) do
     # Attempt HTTP POST -- works if :httpc is available (OTP built-in)
     case :httpc.request(
@@ -855,8 +908,8 @@ defmodule Hypatia.FleetDispatcher do
            [{:timeout, 10_000}],
            []
          ) do
-      {:ok, {{_, status, _}, _, _response_body}} when status in 200..299 ->
-        {:ok, status}
+      {:ok, {{_, status, _}, _, response_body}} when status in 200..299 ->
+        {:ok, status, IO.iodata_to_binary(response_body)}
 
       {:ok, {{_, status, _}, _, _}} ->
         {:error, {:http_status, status}}

test/fleet_dispatcher_honesty_test.exs

@@ -0,0 +1,98 @@
+# SPDX-License-Identifier: MPL-2.0
+
+defmodule Hypatia.FleetDispatcherHonestyTest do
+  # Mutates process-global env (HYPATIA_RHODIBOT_URL, :verisimdb_data_path),
+  # so it cannot run concurrently with other tests.
+  use ExUnit.Case, async: false
+
+  alias Hypatia.FleetDispatcher
+
+  @finding %{
+    type: :fix_suggestion,
+    repo: "test-repo",
+    file: "scripts/deploy.sh",
+    issue: "Unquoted variable",
+    suggestion: "Add double quotes"
+  }
+
+  setup do
+    original_path = Application.get_env(:hypatia, :verisimdb_data_path)
+
+    on_exit(fn ->
+      System.delete_env("HYPATIA_RHODIBOT_URL")
+      Application.put_env(:hypatia, :verisimdb_data_path, original_path)
+    end)
+
+    :ok
+  end
+
+  # Serve exactly one HTTP request with the given status and body on an
+  # ephemeral localhost port; returns the base URL.
+  defp one_shot_server(status, body) do
+    {:ok, listen} = :gen_tcp.listen(0, [:binary, packet: :raw, active: false, reuseaddr: true])
+    {:ok, port} = :inet.port(listen)
+
+    spawn_link(fn ->
+      {:ok, sock} = :gen_tcp.accept(listen)
+      {:ok, _request} = :gen_tcp.recv(sock, 0, 5_000)
+
+      :gen_tcp.send(
+        sock,
+        "HTTP/1.1 #{status} X\r\ncontent-type: application/json\r\n" <>
+          "content-length: #{byte_size(body)}\r\nconnection: close\r\n\r\n" <> body
+      )
+
+      :gen_tcp.close(sock)
+      :gen_tcp.close(listen)
+    end)
+
+    "http://127.0.0.1:#{port}"
+  end
+
+  test "a 2xx GraphQL response carrying errors is a failure, not a dispatch" do
+    url = one_shot_server(200, ~s({"errors":[{"message":"Unknown field \\"suggestFix\\""}]}))
+    System.put_env("HYPATIA_RHODIBOT_URL", url)
+
+    assert {:error, {:live_dispatch_failed, "rhodibot", {:graphql_errors, [_]}}} =
+             FleetDispatcher.dispatch_finding(@finding)
+  end
+
+  test "a 2xx GraphQL response with data is a live dispatch" do
+    url = one_shot_server(200, ~s({"data":{"suggestFix":{"success":true,"prNumber":1}}}))
+    System.put_env("HYPATIA_RHODIBOT_URL", url)
+
+    assert {:ok, :dispatched} = FleetDispatcher.dispatch_finding(@finding)
+  end
+
+  test "a non-2xx status from a configured URL is an error" do
+    url = one_shot_server(500, "{}")
+    System.put_env("HYPATIA_RHODIBOT_URL", url)
+
+    assert {:error, {:live_dispatch_failed, "rhodibot", {:http_status, 500}}} =
+             FleetDispatcher.dispatch_finding(@finding)
+  end
+
+  test "an unreachable configured URL is an error, not a file dispatch" do
+    {:ok, listen} = :gen_tcp.listen(0, [])
+    {:ok, port} = :inet.port(listen)
+    :gen_tcp.close(listen)
+    System.put_env("HYPATIA_RHODIBOT_URL", "http://127.0.0.1:#{port}")
+
+    assert {:error, {:live_dispatch_failed, "rhodibot", _}} =
+             FleetDispatcher.dispatch_finding(@finding)
+  end
+
+  test "an unwritable manifest with no URL configured is an error" do
+    blocker =
+      Path.join(
+        System.tmp_dir!(),
+        "hypatia-manifest-blocker-#{System.unique_integer([:positive])}"
+      )
+
+    File.write!(blocker, "a file, so mkdir beneath it fails")
+    on_exit(fn -> File.rm(blocker) end)
+    Application.put_env(:hypatia, :verisimdb_data_path, blocker)
+
+    assert {:error, {:manifest_write_failed, _}} = FleetDispatcher.dispatch_finding(@finding)
+  end
+end