diff --git a/CHANGELOG.md b/CHANGELOG.md index a614fadba..15f9ac818 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,16 @@ upgrade, is in admin sandbox list. That retires the row and releases the slot; the machine at the provider is then the operator's to check. +- `POST /api/conversations` refuses an opening prompt it cannot use, before it + reserves a sandbox or creates the conversation. Whitespace-only text and a + non-string prompt return `422 invalid_prompt`. Images sent with no opening + text return `422 invalid_prompt` as well; previously such a request was + accepted and provisioned a machine that received pixels and no instruction. + An image whose `media_type` is unsupported, whose decoded bytes are empty, or + which exceeds the 10MB ceiling returns `422 invalid_images`. The OpenAI- + compatible endpoint is unchanged: it still synthesizes a caption for an + image-only message, because clients of that dialect cannot always send one. + ### Added - **An `acp` runtime launches a named command, so a deterministic program can diff --git a/apps/fountain/lib/fountain/conversations.ex b/apps/fountain/lib/fountain/conversations.ex index 5596e66b1..05c126e99 100644 --- a/apps/fountain/lib/fountain/conversations.ex +++ b/apps/fountain/lib/fountain/conversations.ex @@ -2506,6 +2506,7 @@ defmodule Fountain.Conversations do def start_conversation(%{"agent_id" => agent_id, "user_id" => user_id} = attrs, opts) when is_binary(user_id) do with :ok <- require_provider_commit_boundary(), + :ok <- Fountain.Conversations.PromptInput.validate_initial(attrs), %Agents.Agent{} = agent <- Agents.get_agent(agent_id, user_id) || {:error, :not_found}, :ok <- check_execution_limits(user_id, attrs["execution_limits"]), {:ok, runtime_module} <- Fountain.RuntimeDispatch.for_agent(agent), @@ -3258,6 +3259,7 @@ defmodule Fountain.Conversations do ) when is_binary(user_id) do with :ok <- require_provider_commit_boundary(), + :ok <- Fountain.Conversations.PromptInput.validate_initial(attrs), %Agents.Agent{} = agent <- Agents.get_agent(agent_id, user_id) || {:error, :not_found}, :ok <- check_execution_limits(user_id, attrs["execution_limits"]), {:ok, _runtime_module} <- Fountain.RuntimeDispatch.for_agent(agent), diff --git a/apps/fountain/lib/fountain/conversations/prompt_input.ex b/apps/fountain/lib/fountain/conversations/prompt_input.ex new file mode 100644 index 000000000..2a543fb9b --- /dev/null +++ b/apps/fountain/lib/fountain/conversations/prompt_input.ex @@ -0,0 +1,81 @@ +defmodule Fountain.Conversations.PromptInput do + @moduledoc """ + Validate opening input before a launch reserves a sandbox or creates a conversation. + + `validate_initial/1` runs as the first step of `start_conversation/2` on both + the create and attach paths, ahead of the agent fetch and every reservation, + so malformed input costs no machine. + + The rule is that images need words. A launch with neither is fine — that is + how a conversation opens without a first turn — but images with a blank or + absent prompt are refused, because the runtime is handed pixels and no + instruction. The OpenAI-compatible controller decides the other way for its + own dialect and synthesizes a caption + (`FountainWeb.OpenAIController.non_empty/2`); that is a shim for clients that + cannot send one, not the native contract. + + The media-type and size checks repeat what `FountainWeb.PromptImages.decode/1` + already did. That is deliberate: `decode/1` belongs to the two HTTP + transports, and a context caller (`Fountain.Team`, a schedule, a future + worker) reaches `start_conversation/2` without passing through it. The web + layer's message is the friendlier one and still wins for HTTP callers, + because it runs first. + + `attrs["images"]` is the **decoded** shape — `[%{media_type: binary, data: binary}]` + with atom keys and raw bytes, which is what `PromptImages.decode/1` returns and + what every caller already passes. This is not a preference. It is the shape the + rest of the pipeline pattern-matches on, so accepting anything else here would + only move the failure later and make it worse: + + Conversations._unsafe_insert_turn_images/2 fn {%{media_type: mt, data: data}, idx} -> ... + Output.write_image_temp_files/3 fn {%{media_type: mt, data: data}, idx} -> ... + + `TurnMachine.store_images/2` is on every runtime's path + (`ConversationServer.run_turn/6`, before the ACP branch) and handles an + `{:error, changeset}` by logging and continuing — but a key it cannot match + raises `FunctionClauseError`, which is not an error tuple and which no `rescue` + on that path catches. A validator that said `:ok` to a shape those three cannot + read would trade a free refusal for a crashed turn on a sandbox the tenant had + already paid to provision, which is the opposite of the point of validating + here at all. + + So an image that is not the decoded shape is `{:error, :invalid_images}`, and + a caller that has bytes of its own runs them through + `FountainWeb.PromptImages.decode/1` first. + """ + + alias Fountain.Images + + @doc """ + `:ok`, `{:error, :invalid_prompt}` or `{:error, :invalid_images}` for the + opening `prompt` and `images` of a launch. Both keys are optional; `attrs` + is the string-keyed map `start_conversation/2` takes. + """ + @spec validate_initial(map()) :: :ok | {:error, :invalid_prompt | :invalid_images} + def validate_initial(attrs) do + case {attrs["prompt"], attrs["images"] || []} do + {prompt, []} when prompt in [nil, ""] -> :ok + {prompt, images} -> validate_payload(prompt, images) + end + end + + defp validate_payload(prompt, images) when is_binary(prompt) and is_list(images) do + cond do + String.trim(prompt) == "" -> {:error, :invalid_prompt} + Enum.any?(images, &(not valid_image?(&1))) -> {:error, :invalid_images} + true -> :ok + end + end + + defp validate_payload(_, _), do: {:error, :invalid_prompt} + + # A pattern match, not `Map.get/2` or Access: this is the decoded shape or it + # is nothing, and a struct or a string-keyed map falls to the clause below + # rather than raising. + defp valid_image?(%{media_type: media_type, data: data}) when is_binary(data), + do: + Images.valid_media_type?(media_type) and byte_size(data) > 0 and + byte_size(data) <= Images.max_prompt_image_bytes() + + defp valid_image?(_), do: false +end diff --git a/apps/fountain/lib/fountain_web/controllers/fallback_controller.ex b/apps/fountain/lib/fountain_web/controllers/fallback_controller.ex index 5dc800807..eea3e4f7c 100644 --- a/apps/fountain/lib/fountain_web/controllers/fallback_controller.ex +++ b/apps/fountain/lib/fountain_web/controllers/fallback_controller.ex @@ -42,6 +42,34 @@ defmodule FountainWeb.FallbackController do }) end + # Opening input a launch cannot use (Fountain.Conversations.PromptInput). + # Refused before any sandbox is reserved, so nothing was spent. Named here + # rather than left to the terminal safety net below: these are ordinary + # client mistakes, and the net logs a warning and answers without a message. + def call(conn, {:error, :invalid_prompt}) do + conn + |> put_status(:unprocessable_entity) + |> json(%{ + error: "invalid_prompt", + message: + "prompt must be a string with words in it; images require one. " <> + "Omit both to open a conversation with no first turn." + }) + end + + def call(conn, {:error, :invalid_images}) do + conn + |> put_status(:unprocessable_entity) + |> json(%{ + error: "invalid_images", + message: + "each image needs a supported media_type (" <> + Enum.join(Fountain.Images.valid_media_types(), ", ") <> + ") and between 1 byte and " <> + "#{div(Fountain.Images.max_prompt_image_bytes(), 1024 * 1024)}MB of data" + }) + end + # start_conversation rejects an unknown / cross-tenant vault by returning # {:error, :vault_not_found}. Surface as 404 so callers can't tell the # difference between "no such vault" and "vault belongs to someone else". diff --git a/apps/fountain/lib/fountain_web/schemas.ex b/apps/fountain/lib/fountain_web/schemas.ex index f9e104f19..f2ff1f7d5 100644 --- a/apps/fountain/lib/fountain_web/schemas.ex +++ b/apps/fountain/lib/fountain_web/schemas.ex @@ -639,7 +639,13 @@ defmodule FountainWeb.Schemas do "permission_policy_widens rather than silently clamped, and one the runtime " <> "never consults is refused with 422 permission_policy_unenforceable." }, - prompt: %Schema{type: :string, description: "Optional first turn prompt."}, + prompt: %Schema{ + type: :string, + description: + "Optional first turn prompt. A launch may open with no prompt at all, but a " <> + "prompt that is present must carry words: blank and whitespace-only text is " <> + "refused with 422 invalid_prompt, before the launch reserves a sandbox." + }, title: %Schema{ type: :string, nullable: true, @@ -649,7 +655,12 @@ defmodule FountainWeb.Schemas do images: %Schema{ type: :array, items: ImageInput, - description: "Optional images to attach to the initial prompt.", + description: + "Optional images to attach to the initial prompt. They require that prompt: " <> + "images with no opening text are refused with 422 invalid_prompt, and an " <> + "image whose media_type is unsupported or whose decoded bytes are empty or " <> + "over the 10MB ceiling is refused with 422 invalid_images. Both refusals " <> + "happen before a sandbox is reserved.", nullable: true }, sprite_name: %Schema{ diff --git a/apps/fountain/test/fountain/conversations/opening_input_test.exs b/apps/fountain/test/fountain/conversations/opening_input_test.exs new file mode 100644 index 000000000..cf4cf75e3 --- /dev/null +++ b/apps/fountain/test/fountain/conversations/opening_input_test.exs @@ -0,0 +1,180 @@ +defmodule Fountain.Conversations.OpeningInputTest do + use Fountain.DataCase, async: true + use Mimic + + alias Fountain.Conversations + alias Fountain.Conversations.{Conversation, ConversationServer, PromptInput, Sandbox} + + setup do + user = insert_active_user() + env = insert_env(user_id: user.id) + agent = insert_agent(user_id: user.id, runtime: "claude", environment_id: env.id) + + sandbox = + insert_sandbox( + user_id: user.id, + status: "ready", + agent_id: agent.id, + environment_id: env.id + ) + + {:ok, user: user, agent: agent, sandbox: sandbox} + end + + for path <- [:create, :attach] do + @tag path: path + test "#{path} refuses invalid text before allocating rows or starting work", ctx do + reject(Horde.DynamicSupervisor, :start_child, 2) + reject(ConversationServer, :send_prompt, 4) + counts = row_counts() + + for prompt <- [" ", "\n\t", 123, ["hello"]] do + assert {:error, :invalid_prompt} = start(ctx, %{"prompt" => prompt}) + assert row_counts() == counts + end + end + + @tag path: path + test "#{path} refuses images without opening text", ctx do + reject(Horde.DynamicSupervisor, :start_child, 2) + reject(ConversationServer, :send_prompt, 4) + counts = row_counts() + image = %{media_type: "image/png", data: <<1>>} + + for prompt <- [nil, ""] do + assert {:error, :invalid_prompt} = start(ctx, %{"prompt" => prompt, "images" => [image]}) + assert row_counts() == counts + end + end + + @tag path: path + test "#{path} rejects malformed, empty, unsupported and oversized image bytes", ctx do + reject(Horde.DynamicSupervisor, :start_child, 2) + reject(ConversationServer, :send_prompt, 4) + counts = row_counts() + large = :binary.copy(<<0>>, Fountain.Images.max_prompt_image_bytes() + 1) + + for image <- [ + %{}, + %{media_type: "image/png", data: ""}, + %{media_type: "text/html", data: "html"}, + %{media_type: "image/png", data: large} + ] do + assert {:error, :invalid_images} = + start(ctx, %{"prompt" => "Review", "images" => [image]}) + + assert row_counts() == counts + end + end + + @tag path: path + test "#{path} still accepts a launch without an opening prompt", ctx do + stub(Horde.DynamicSupervisor, :start_child, fn _, _ -> {:ok, self()} end) + assert {:ok, _} = start(ctx, %{}) + end + + @tag path: path + test "#{path} passes valid opening text and image bytes to delivery", ctx do + image = %{media_type: "image/png", data: <<0, 1, 2>>} + + case ctx.path do + :create -> + expect(Horde.DynamicSupervisor, :start_child, fn _, _ -> {:ok, self()} end) + + :attach -> + expect(ConversationServer, :send_prompt, fn _, "Review", [^image], _ -> :ok end) + end + + assert {:ok, _} = start(ctx, %{"prompt" => "Review", "images" => [image]}) + + if ctx.path == :create, + do: assert_received({:"$gen_cast", {:initial_prompt, "Review", [^image]}}) + end + end + + # `attrs["images"]` is the decoded shape. Each refusal is paired with the + # accept, so the pair only passes when the map is actually read rather than + # rejected wholesale. + test "only the decoded shape is accepted" do + good = %{media_type: "image/png", data: <<0, 1, 2>>} + + for bad <- [ + %{good | media_type: "text/html"}, + %{good | data: ""}, + Map.delete(good, :data), + Map.delete(good, :media_type), + %{good | data: :binary.copy(<<0>>, Fountain.Images.max_prompt_image_bytes() + 1)}, + # Not the decoded shape: `PromptImages.decode/1` returns atom keys, + # and the three consumers below pattern-match them. + %{"media_type" => "image/png", "data" => <<0, 1, 2>>}, + %URI{} + ] do + assert {:error, :invalid_images} = + PromptInput.validate_initial(%{"prompt" => "Review", "images" => [bad]}) + + assert :ok = PromptInput.validate_initial(%{"prompt" => "Review", "images" => [good]}) + end + end + + # The gap the mocked delivery test leaves: `validate_initial/1` saying `:ok` + # is only worth anything if the shape it accepts survives the consumers that + # run after a sandbox has been paid for. `store_images/2` sits in + # `run_turn/6` before the ACP branch and turns an `{:error, changeset}` into + # a log line, but a key it cannot match raises out of the server instead. + test "the accepted shape survives every consumer that runs after provisioning", ctx do + image = %{media_type: "image/png", data: <<0, 1, 2>>} + assert :ok = PromptInput.validate_initial(%{"prompt" => "Review", "images" => [image]}) + + conv = insert_conversation(user_id: ctx.user.id, agent: ctx.agent) + + # A turn each: `store_images/2` swallows a duplicate-position changeset + # error by design, so sharing one would hide whether it read the map. + [direct, stored] = + for n <- 1..2 do + {:ok, turn} = + Conversations._unsafe_create_turn(%{ + conversation_id: conv.id, + turn_number: n, + status: "running", + prompt: "Review" + }) + + turn + end + + assert {:ok, 1} = Conversations._unsafe_insert_turn_images(direct.id, [image]) + assert :ok = Fountain.Conversations.TurnMachine.store_images(stored, [image]) + + assert %{images: [%{media_type: "image/png", data: <<0, 1, 2>>}]} = + Repo.preload(stored, :images) + + expect(Managoat.Sandbox.Sprites, :write_file, fn _h, _path, data, _opts -> + assert data == image.data + :ok + end) + + assert [{path, "image/png"}] = + Fountain.Conversations.Output.write_image_temp_files( + %Managoat.Sandbox.Handle{provider: :sprites, name: "s"}, + direct.id, + [image] + ) + + assert path =~ ".png" + end + + @tag path: :attach + test "the decoded shape reaches delivery unchanged", ctx do + image = %{media_type: "image/png", data: <<0, 1, 2>>} + expect(ConversationServer, :send_prompt, fn _, "Review", [^image], _ -> :ok end) + assert {:ok, _} = start(ctx, %{"prompt" => "Review", "images" => [image]}) + end + + defp start(ctx, extra) do + attrs = %{"user_id" => ctx.user.id, "agent_id" => ctx.agent.id} + attrs = if ctx.path == :attach, do: Map.put(attrs, "sandbox_id", ctx.sandbox.id), else: attrs + Conversations.start_conversation(Map.merge(attrs, extra)) + end + + defp row_counts, do: {Repo.aggregate(Conversation, :count), Repo.aggregate(Sandbox, :count)} +end diff --git a/apps/fountain/test/fountain_web/controllers/conversation_controller_test.exs b/apps/fountain/test/fountain_web/controllers/conversation_controller_test.exs index f505d06e5..5d1145b91 100644 --- a/apps/fountain/test/fountain_web/controllers/conversation_controller_test.exs +++ b/apps/fountain/test/fountain_web/controllers/conversation_controller_test.exs @@ -1427,12 +1427,12 @@ defmodule FountainWeb.ConversationControllerTest do end describe "POST /api/conversations with images" do - test "returns 201 with conversation when images array is provided (decode_images non-empty branch)", + test "delivers opening text and image bytes when returning 201", %{conn: conn, user: user, raw_key: raw_key} do agent = insert_agent(user_id: user.id) stub(Horde.DynamicSupervisor, :start_child, fn _supervisor, _child_spec -> - {:ok, spawn(fn -> :ok end)} + {:ok, self()} end) image_data = Base.encode64("fake-image-bytes") @@ -1442,10 +1442,34 @@ defmodule FountainWeb.ConversationControllerTest do |> authed_with_key(raw_key) |> post_json("/api/conversations", %{ "agent_id" => agent.id, + "prompt" => "Review", "images" => [%{"media_type" => "image/png", "data" => image_data}] }) assert json_response(conn, 201) + assert_received {:"$gen_cast", {:initial_prompt, "Review", [image]}} + assert image == %{media_type: "image/png", data: "fake-image-bytes"} + end + + test "images without opening text are refused before reserving a sandbox", %{ + conn: conn, + user: user, + raw_key: raw_key + } do + agent = insert_agent(user_id: user.id) + reject(Horde.DynamicSupervisor, :start_child, 2) + + conn = + conn + |> authed_with_key(raw_key) + |> post_json("/api/conversations", %{ + "agent_id" => agent.id, + "images" => [%{"media_type" => "image/png", "data" => Base.encode64("bytes")}] + }) + + assert json_response(conn, 422)["error"] == "invalid_prompt" + assert Fountain.Repo.aggregate(Fountain.Conversations.Conversation, :count) == 0 + assert Fountain.Repo.aggregate(Fountain.Conversations.Sandbox, :count) == 0 end test "returns 201 with conversation when no images provided (decode_images [] branch)", %{ diff --git a/apps/fountain/test/fountain_web/controllers/fallback_controller_test.exs b/apps/fountain/test/fountain_web/controllers/fallback_controller_test.exs index 21200811b..a2f36c7f7 100644 --- a/apps/fountain/test/fountain_web/controllers/fallback_controller_test.exs +++ b/apps/fountain/test/fountain_web/controllers/fallback_controller_test.exs @@ -10,6 +10,18 @@ defmodule FountainWeb.FallbackControllerTest do assert %{"error" => "sandbox_reset_pending"} = json_response(conn, 409) end + test "unusable opening input names itself rather than falling to the safety net", %{conn: conn} do + for {reason, error} <- [invalid_prompt: "invalid_prompt", invalid_images: "invalid_images"] do + body = + conn + |> FountainWeb.FallbackController.call({:error, reason}) + |> json_response(422) + + assert body["error"] == error + assert is_binary(body["message"]) and body["message"] != "" + end + end + describe "{:error, %Ecto.Changeset{}} → 422" do test "POST /api/agents with missing required fields returns 422 with errors body", %{ conn: conn diff --git a/apps/fountain/test/fountain_web/controllers/turn_image_ingest_test.exs b/apps/fountain/test/fountain_web/controllers/turn_image_ingest_test.exs index 530e73c75..2a6050eaa 100644 --- a/apps/fountain/test/fountain_web/controllers/turn_image_ingest_test.exs +++ b/apps/fountain/test/fountain_web/controllers/turn_image_ingest_test.exs @@ -42,7 +42,11 @@ defmodule FountainWeb.TurnImageIngestTest do defp post_create(conn, raw_key, agent, images) do conn |> authed_with_key(raw_key) - |> post_json("/api/conversations", %{"agent_id" => agent.id, "images" => images}) + |> post_json("/api/conversations", %{ + "agent_id" => agent.id, + "prompt" => "Describe these images", + "images" => images + }) end describe "media type" do @@ -65,7 +69,7 @@ defmodule FountainWeb.TurnImageIngestTest do test "every allowed type is accepted", %{raw_key: raw_key, agent: agent} do stub(Horde.DynamicSupervisor, :start_child, fn _s, _spec -> - {:ok, spawn(fn -> :ok end)} + {:ok, self()} end) for type <- Conversations.TurnImage.valid_media_types() do @@ -74,6 +78,9 @@ defmodule FountainWeb.TurnImageIngestTest do |> post_create(raw_key, agent, [%{"media_type" => type, "data" => png()}]) assert conn.status in [200, 201], "#{type} was rejected with #{conn.status}" + assert_received {:"$gen_cast", {:initial_prompt, "Describe these images", [image]}} + assert image.media_type == type + assert image.data == Base.decode64!(png()) end end end diff --git a/docs/api.md b/docs/api.md index 82d302ef3..2a04d977c 100644 --- a/docs/api.md +++ b/docs/api.md @@ -244,6 +244,15 @@ curl --fail-with-body \ "$FOUNTAIN_URL/api/conversations" ``` +A first prompt is optional. A launch with no prompt creates the conversation +and waits for a later turn. A prompt that is present must contain words. +Fountain refuses blank or whitespace-only text with `422 invalid_prompt`. + +Images need that first prompt. Fountain refuses images that arrive with no +text, and the error is `422 invalid_prompt`. An unsupported `media_type`, +empty bytes, or more than 10MB give `422 invalid_images`. Both checks run +before the launch reserves a sandbox, so bad input costs no machine. + This starts real work and can consume credits and provider usage. [Conversation states](reference/conversation-states.md) explains lifecycle transitions. The Conversations operations in the diff --git a/sdk/typescript/src/generated/openapi.ts b/sdk/typescript/src/generated/openapi.ts index 40a6df7a6..59d76e66c 100644 --- a/sdk/typescript/src/generated/openapi.ts +++ b/sdk/typescript/src/generated/openapi.ts @@ -3616,7 +3616,7 @@ export interface components { environment_id?: string | null; /** @description With channel_id: skip the resume and open a new conversation (201), which then becomes the channel's binding. Sent by a chat harness relaying its owner's rotate command. Ignored without channel_id. */ fresh?: boolean | null; - /** @description Optional images to attach to the initial prompt. */ + /** @description Optional images to attach to the initial prompt. They require that prompt: images with no opening text are refused with 422 invalid_prompt, and an image whose media_type is unsupported or whose decoded bytes are empty or over the 10MB ceiling is refused with 422 invalid_images. Both refusals happen before a sandbox is reserved. */ images?: components["schemas"]["ImageInput"][] | null; /** @description Key/value strings to stamp on the conversation. At most 32 entries; a key is at most 64 bytes and a value at most 256 bytes, and a 422 names the offending key under `errors.labels`. With channel_id, a resume merges these into the conversation it hands back rather than dropping them. */ labels?: { @@ -3629,7 +3629,7 @@ export interface components { } & { [key: string]: ("auto_allow" | "ask" | "auto_deny") | number; }) | null; - /** @description Optional first turn prompt. */ + /** @description Optional first turn prompt. A launch may open with no prompt at all, but a prompt that is present must carry words: blank and whitespace-only text is refused with 422 invalid_prompt, before the launch reserves a sandbox. */ prompt?: string; /** * @description none omits the sandbox Fountain credential on provision and every wake. Requires a fresh ephemeral sandbox; unavailable on attach or policy-changing channel resume.