diff --git a/CHANGELOG.md b/CHANGELOG.md index 0da74d233..7e4490845 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,15 @@ upgrade, is in record already matched the document. Apply stays additive and prunes nothing. Rebinding a teammate retires the computer its old environment and vault named, and is refused while a turn is running on it. +- Conversations carry free-form `labels`, a map of at most 32 key/value + strings. Set them on creation or with `PATCH /api/conversations/:id/labels`, + which merges. A running agent stamps its own conversation with the + `_fountain/labels` ACP extension notification, and a sandbox callback token + can label only the conversation it was minted for, on every door that writes + labels. `GET /api/conversations` + and `GET /api/team/:agent_id/conversations` take a repeatable `label=key:value` + filter, combined with AND. `conversation.*` webhook payloads carry `labels`, + and the console's conversation lists render them as chips. ### Fixed diff --git a/apps/fountain/lib/fountain/conversations.ex b/apps/fountain/lib/fountain/conversations.ex index 24e599fca..6b32c8116 100644 --- a/apps/fountain/lib/fountain/conversations.ex +++ b/apps/fountain/lib/fountain/conversations.ex @@ -11,7 +11,7 @@ defmodule Fountain.Conversations do require Logger alias Fountain.Audit - alias Fountain.Conversations.{Blocks, Conversation, LogEvent, Sandbox, Turn, TurnImage} + alias Fountain.Conversations.{Blocks, Conversation, Labels, LogEvent, Sandbox, Turn, TurnImage} alias Fountain.Repo # ── on the _unsafe_ prefix ──────────────────────────────────────────────── @@ -696,6 +696,10 @@ defmodule Fountain.Conversations do for `limit: n` — which the console's dashboard uses to ask for the five it shows instead of every row a busy account has. + `labels: %{"env" => "prod"}` (#1637) keeps the conversations carrying every + one of those pairs — jsonb containment, so a row with more labels than the + filter names still matches, and the GIN index on the column serves it. + Populates the `last_active_at` virtual field using `kind: "output"` log events only — stage events (reconnects, lifecycle) are excluded so reconnects don't produce false unread indicators. @@ -729,6 +733,9 @@ defmodule Fountain.Conversations do {:channel_id, id}, q when is_binary(id) and id != "" -> where(q, [conv: c], c.channel_id == ^id) + {:labels, labels}, q when is_map(labels) and map_size(labels) > 0 -> + where(q, [conv: c], fragment("? @> ?", c.labels, type(^labels, :map))) + {:status, [_ | _] = statuses}, q -> where(q, [conv: c], c.status in ^statuses) @@ -751,16 +758,25 @@ defmodule Fountain.Conversations do the surface reading a channel — the team page — wants the last transcript even when nothing is running. """ - def list_channel_conversations(user_id, channel_id) + def list_channel_conversations(user_id, channel_id, opts \\ []) when is_binary(user_id) and is_binary(channel_id) do from(c in annotated_query(user_id), where: c.channel_id == ^channel_id, order_by: [desc: c.inserted_at, desc: c.id] ) + |> filter_by_labels(Keyword.get(opts, :labels)) |> Repo.all() |> Repo.preload([:agent, :sandbox]) end + # The same containment filter `list_conversations/2` applies (#1637), for + # the channel-bound list behind the team route. + defp filter_by_labels(query, labels) when is_map(labels) and map_size(labels) > 0 do + where(query, [conv: c], fragment("? @> ?", c.labels, type(^labels, :map))) + end + + defp filter_by_labels(query, _labels), do: query + @doc """ Scoped fetch that also populates the read-model annotations — `turn_count` and `last_active_at` — which `get_conversation/2` leaves at @@ -889,6 +905,134 @@ defmodule Fountain.Conversations do end end + @doc """ + Merge `labels` into `conversation_id`'s. **The door every request-shaped + caller uses** (#1637). + + Merge, not replace: a key that is not named is left alone and a key whose + value is `nil` is removed, so a run can stamp one outcome without reading + the rest first. `Conversations.Labels` owns the limits, and a write that + breaks one comes back as a changeset naming the offending key. + + **A sandbox may label its own conversation only.** Pass + `sandbox_key_id: key.id` whenever the caller authenticated with a + `sprite`-scoped token: the conversation must be the one that token was + minted for (`callback_api_key_id`), or the write is refused with + `:sprite_may_not_label_another_conversation`. Without that check a worker + holding an account-scoped callback key could relabel every other run on the + account, which is exactly the loop ADR 0045 describes. That is why the + check lives here and not in each controller: `PATCH .../labels`, the team + message and a `channel_id` resume all write labels, and the rule has to + hold on the door rather than on whichever of them remembered. + + `labels` that is not a map at all is a validation failure, not a silent + no-op, so every door refuses `{"labels": "env=prod"}` the same way. + + Tenant-scoped: an id belonging to another account reads as `:not_found`. + """ + @spec set_conversation_labels(binary(), binary(), term(), keyword()) :: + {:ok, Conversation.t()} | {:error, term()} + def set_conversation_labels(conversation_id, user_id, labels, opts \\ []) + when is_binary(conversation_id) and is_binary(user_id) do + case get_conversation(conversation_id, user_id) do + nil -> + {:error, :not_found} + + %Conversation{} = conv -> + if sandbox_owns?(conv, Keyword.get(opts, :sandbox_key_id)) do + # Ownership: `conv` came from the tenant-scoped fetch above. + _unsafe_merge_labels(conv, labels, opts) + else + {:error, :sprite_may_not_label_another_conversation} + end + end + end + + # No sandbox key on the request is the owner's own credential, which may + # label any conversation it can already fetch. + defp sandbox_owns?(_conv, nil), do: true + defp sandbox_owns?(%Conversation{callback_api_key_id: id}, key_id), do: id == key_id + + @doc """ + Merge `labels` into a conversation row, with no tenant scoping and no + credential rule. + + Unscoped, hence the prefix. The legitimate callers are + `set_conversation_labels/4`, which scopes and applies the sandbox rule + before delegating here, and `Labels._unsafe_stamp/2`, which runs inside the + conversation's own server and holds the row that server was started for. A + request path that calls this directly has skipped the rule that stops one + sandbox relabelling another, so do not add one. + + A merge that changes nothing writes nothing and records nothing — a + deterministic run re-stamping the same outcome on every tick is the normal + case. Audited as `conversation.labels_set` with the keys written and the + keys removed, never the values (ADR 0013). + """ + @spec _unsafe_merge_labels(Conversation.t(), term(), keyword()) :: + {:ok, Conversation.t()} | {:error, Ecto.Changeset.t()} + def _unsafe_merge_labels(conv, labels, opts \\ []) + + def _unsafe_merge_labels(%Conversation{} = conv, labels, opts) when is_map(labels) do + current = conv.labels || %{} + merged = Labels.merge(current, labels) + + cond do + merged == current -> {:ok, conv} + true -> write_labels(conv, current, labels, merged, opts) + end + end + + # Anything that is not a map is a validation failure with the same shape a + # broken limit produces, so a caller reads one answer whichever door it + # came through. + def _unsafe_merge_labels(%Conversation{} = conv, labels, _opts) do + {:error, label_refusal(conv, Labels.check(labels))} + end + + defp write_labels(conv, current, labels, merged, opts) do + case Labels.check_merge(current, labels) do + :ok -> + {written, removed} = Labels.changed_keys(current, labels) + + conv + |> Conversation.changeset(%{labels: merged}) + |> Repo.update() + |> tap(fn + {:ok, updated} -> record_labels_set(updated, written, removed, merged, opts) + _ -> :ok + end) + + refusal -> + {:error, label_refusal(conv, refusal)} + end + end + + defp record_labels_set(conv, written, removed, merged, opts) do + Audit.record(%{ + user_id: conv.user_id, + action: "conversation.labels_set", + resource_type: "conversation", + resource_id: conv.id, + actor: Keyword.get(opts, :actor, "self"), + request_ip: Keyword.get(opts, :request_ip), + metadata: %{ + "keys" => written, + "removed_keys" => removed, + "label_count" => map_size(merged) + } + }) + end + + # `Labels.check_merge/2` words the refusal from the write the caller made; + # this is what turns that sentence into the `errors.labels` a 422 renders, + # the same key the changeset validator would have used. + defp label_refusal(%Conversation{} = conv, {:error, message}) do + conv + |> Ecto.Changeset.change() + |> Ecto.Changeset.add_error(:labels, message) + end + def update_conversation(%Conversation{} = conv, attrs) do conv |> Conversation.changeset(attrs) @@ -1780,8 +1924,10 @@ defmodule Fountain.Conversations do independent of `inserted_at`'s one-second precision. Two concurrent first calls for one channel can both create; the next call - resumes whichever is newer. Nothing is audited on the resume path — nothing - changed. + resumes whichever is newer. Nothing is audited on the resume path unless + `attrs["labels"]` actually changes something: it is the same conversation, + so labels merge into the row it hands back (#1637) and that write records + `conversation.labels_set` like any other. """ def start_or_resume_conversation(attrs, opts \\ []) @@ -1801,6 +1947,7 @@ defmodule Fountain.Conversations do do: {:ok, fresh, :created} else with :ok <- check_sandbox_api_resume(conv, attrs["sandbox_api_access"]), + {:ok, conv} <- resume_labels(conv, attrs["labels"], opts), do: {:ok, conv, :resumed} end @@ -1814,6 +1961,20 @@ defmodule Fountain.Conversations do with {:ok, conv} <- start_conversation(attrs, opts), do: {:ok, conv, :created} end + # A resume lands on the conversation the binding already has, so labels on + # the request are merged into it rather than dropped (#1637). A caller that + # sends none changes nothing, and the resume stays the silent path it was. + # + # Through `set_conversation_labels/4` rather than the writer beneath it: + # this runs on `POST /api/conversations`, which a sandbox's own token may + # call, and a resume names an *existing* conversation. Writing here + # directly would let a sprite minted for one conversation relabel any other + # of the tenant's by resuming its channel. + defp resume_labels(%Conversation{} = conv, nil, _opts), do: {:ok, conv} + + defp resume_labels(%Conversation{} = conv, labels, opts), + do: set_conversation_labels(conv.id, conv.user_id, labels, opts) + # `true` or `"true"` — the ACP adapter sends a JSON boolean, a hand-built # request may send a string. Anything else is not a request. defp fresh_requested?(%{"fresh" => fresh}), do: fresh in [true, "true"] @@ -1905,6 +2066,7 @@ defmodule Fountain.Conversations do - `source` — optional; one of "ui", "api", "agent" (default "api") - `parent_conversation_id` — optional; UUID of the conversation that spawned this one - `title` — optional display title (the team page names a teammate with it) + - `labels` — optional `key => value` strings (#1637); see `Conversations.Labels` """ def start_conversation(attrs, opts \\ []) @@ -1974,7 +2136,8 @@ defmodule Fountain.Conversations do title: attrs["title"], sandbox_api_access: api_access, permission_policy: perm_policy, - caller_tools: attrs["caller_tools"] || [] + caller_tools: attrs["caller_tools"] || [], + labels: attrs["labels"] || %{} }) do # Recorded here rather than in either branch below: both of them return # {:ok, conv}. The row exists and the sandbox reservation is spent even @@ -2496,7 +2659,8 @@ defmodule Fountain.Conversations do permission_policy: perm_policy, # The bridge's tools (#1202) ride on both create paths: this # one is what a home sandbox's second conversation takes. - caller_tools: attrs["caller_tools"] || [] + caller_tools: attrs["caller_tools"] || [], + labels: attrs["labels"] || %{} }) do Audit.record(%{ user_id: user_id, diff --git a/apps/fountain/lib/fountain/conversations/conversation.ex b/apps/fountain/lib/fountain/conversations/conversation.ex index 8a295edac..ab0a059d8 100644 --- a/apps/fountain/lib/fountain/conversations/conversation.ex +++ b/apps/fountain/lib/fountain/conversations/conversation.ex @@ -56,6 +56,12 @@ defmodule Fountain.Conversations.Conversation do field :permission_policy, :map # The caller-defined tools of the bridge (#1202, `Fountain.CallerTools`). field :caller_tools, {:array, :map}, default: [] + # Free-form `key => value` strings (#1637). Set at launch, merged by the + # labels route and by the agent's own `_fountain/labels` ACP notification, + # and filtered on with jsonb containment. `Conversations.Labels` owns the + # limits and the merge; writes here go through `Labels.changeset/1` below, + # which is why every door enforces the same rule. + field :labels, :map, default: %{} # Populated by list_conversations_by_activity/1 — not persisted. field :turn_count, :integer, virtual: true, default: 0 @@ -120,7 +126,8 @@ defmodule Fountain.Conversations.Conversation do :environment_id, :channel_id, :permission_policy, - :caller_tools + :caller_tools, + :labels ]) |> validate_required([:runtime, :status, :sandbox_id, :user_id]) |> validate_length(:channel_id, max: 255) @@ -129,6 +136,7 @@ defmodule Fountain.Conversations.Conversation do |> validate_inclusion(:source, @sources) |> validate_inclusion(:sandbox_api_access, @sandbox_api_access_modes) |> validate_sandbox_api_access_immutable() + |> Fountain.Conversations.Labels.changeset() |> foreign_key_constraint(:sandbox_id) |> foreign_key_constraint(:agent_id) |> foreign_key_constraint(:agent_version_id) diff --git a/apps/fountain/lib/fountain/conversations/labels.ex b/apps/fountain/lib/fountain/conversations/labels.ex new file mode 100644 index 000000000..47b8ecd4b --- /dev/null +++ b/apps/fountain/lib/fountain/conversations/labels.ex @@ -0,0 +1,356 @@ +defmodule Fountain.Conversations.Labels do + @moduledoc """ + The rule for a conversation's labels (#1637), in one place. + + A label is a free-form `key => value` pair of strings on the conversation + row. A person titles and searches their own threads; a program running as a + teammate wants to slice its runs by facts it knew when the turn ended — + `env=prod`, `drift=true`, `gated=apply`. Those facts are not text worth + searching, so they are not in the full-text index and never will be. + + Every door that writes labels goes through `changeset/1` here, so the + limits cannot drift between the create request, the labels route, the team + message and the ACP extension notification: + + * at most 32 entries; + * a key is a non-empty string of at most 64 bytes; + * a value is a string of at most 256 bytes; + * neither may contain a NUL byte, which Postgres refuses inside `jsonb`. + + A refusal names the offending key, because a caller sending thirty-two of + them cannot otherwise tell which one Fountain disliked. On a merge the key + named is one the caller actually sent — see `check_merge/2`. + + ## Merge, and how a key is removed + + Writes merge (`merge/2`): a key that is not mentioned is left alone. A key + whose value is `null` is removed. That is what lets a run add one label + without reading the others first, and what makes a channel resume keep the + labels the binding already carried. + + ## The list filter + + `GET /api/conversations?label=env:prod&label=drift:true` is repeatable and + AND-combined. Each value splits on its **first** colon only, so + `label=path:a:b` filters `path` for `a:b`. The query is jsonb containment, + which the GIN index on the column serves. + """ + + import Ecto.Changeset, only: [get_change: 2, add_error: 3] + + require Logger + + @max_entries 32 + @max_key_bytes 64 + @max_value_bytes 256 + + @doc "At most this many labels on one conversation." + @spec max_entries() :: pos_integer() + def max_entries, do: @max_entries + + @doc "A label key is at most this many bytes." + @spec max_key_bytes() :: pos_integer() + def max_key_bytes, do: @max_key_bytes + + @doc "A label value is at most this many bytes." + @spec max_value_bytes() :: pos_integer() + def max_value_bytes, do: @max_value_bytes + + @doc """ + Validate the `:labels` change on a conversation changeset. + + Called from `Fountain.Conversations.Conversation.changeset/2`, which is the + only writer of the column, so every door inherits it. It sees the *merged* + map, which is the right thing to enforce and the wrong thing to word a + count refusal from; `Fountain.Conversations._unsafe_merge_labels/3` runs + `check_merge/2` first for that reason. + """ + @spec changeset(Ecto.Changeset.t()) :: Ecto.Changeset.t() + def changeset(changeset) do + case get_change(changeset, :labels) do + nil -> changeset + labels -> apply_check(changeset, check(labels)) + end + end + + defp apply_check(changeset, :ok), do: changeset + defp apply_check(changeset, {:error, message}), do: add_error(changeset, :labels, message) + + @doc """ + Whether `labels` is a legal label map. `:ok`, or `{:error, message}` naming + the offending key. + """ + @spec check(term()) :: :ok | {:error, String.t()} + def check(labels) when is_map(labels) do + with :ok <- check_entries(labels), do: check_count(labels) + end + + def check(_labels), do: {:error, "must be an object of string keys and string values"} + + @doc """ + Check what merging `incoming` into `current` would produce, wording the + refusal from the write the caller actually made. + + `check/1` alone would blame an arbitrary key for the count: merging one new + label into a conversation that already holds 32 puts a *pre-existing* key + over the boundary in sorted order, and telling somebody their write of + `run` failed because of `env` — a label they never touched — sends them to + fix the wrong thing. So the count is reported against the first key this + write adds. + + Entry-level problems need no such care: `current` is already on the row and + therefore already legal, so any entry `check/1` rejects came from + `incoming`. + """ + @spec check_merge(map(), term()) :: :ok | {:error, String.t()} + def check_merge(current, incoming) when is_map(current) and is_map(incoming) do + merged = merge(current, incoming) + + with :ok <- check_entries(merged) do + check_merged_count(current, incoming, merged) + end + end + + def check_merge(_current, incoming), do: check(incoming) + + defp check_merged_count(current, incoming, merged) do + if map_size(merged) > @max_entries do + {:error, + "at most #{@max_entries} labels; #{describe(blamed_key(current, incoming, merged))} does not fit"} + else + :ok + end + end + + # The first key of this write that does not fit. + # + # The labels already on the row are kept — the caller did not ask to change + # them — so the room left is the ceiling minus what survives the merge, and + # what spills past it is one of the keys this write added. Adding `run` to a + # conversation that already holds 32 therefore names `run`, and sending 33 + # at once names the 33rd rather than the first. + # + # The fallback covers a write that adds nothing and is over the limit + # anyway, which only a row already past the ceiling can produce. + defp blamed_key(current, incoming, merged) do + retained = Enum.count(Map.keys(current), &(not removed?(incoming, &1))) + + added = + incoming + |> Enum.reject(fn {key, value} -> is_nil(value) or Map.has_key?(current, key) end) + |> Enum.map(&elem(&1, 0)) + |> Enum.sort_by(&describe/1) + |> Enum.drop(max(@max_entries - retained, 0)) + + case added do + [key | _] -> key + [] -> merged |> Map.keys() |> Enum.sort_by(&describe/1) |> Enum.at(@max_entries) + end + end + + defp removed?(incoming, key), do: Map.has_key?(incoming, key) and is_nil(Map.get(incoming, key)) + + defp check_count(labels) do + if map_size(labels) > @max_entries do + # Sorted, so the key named is the same one on every run rather than + # whichever the map happened to iterate to last. + over = labels |> Map.keys() |> Enum.sort_by(&describe/1) |> Enum.at(@max_entries) + + {:error, "at most #{@max_entries} labels; #{describe(over)} does not fit"} + else + :ok + end + end + + defp check_entries(labels) do + labels + |> Enum.sort_by(fn {key, _value} -> describe(key) end) + |> Enum.reduce_while(:ok, fn {key, value}, _acc -> + case check_entry(key, value) do + :ok -> {:cont, :ok} + error -> {:halt, error} + end + end) + end + + defp check_entry(key, _value) when not is_binary(key), + do: {:error, "label key #{describe(key)} must be a string"} + + defp check_entry("", _value), do: {:error, "a label key must not be empty"} + + defp check_entry(key, _value) when byte_size(key) > @max_key_bytes, + do: {:error, "label key #{describe(key)} is longer than #{@max_key_bytes} bytes"} + + defp check_entry(key, value) when not is_binary(value), + do: {:error, "label #{describe(key)} must have a string value"} + + defp check_entry(key, value) when byte_size(value) > @max_value_bytes, + do: {:error, "label #{describe(key)} has a value longer than #{@max_value_bytes} bytes"} + + # Postgres refuses a NUL byte inside a jsonb string, so without this clause + # the write reaches `Repo.update` and comes back as a raised + # `Postgrex.Error` — a 500 on the HTTP doors, and on the ACP path a raise + # that would travel up through the turn machine and take the conversation's + # server down with the turn it was running. + defp check_entry(key, value) do + cond do + nul?(key) -> {:error, "label key #{describe(key)} must not contain a NUL byte"} + nul?(value) -> {:error, "label #{describe(key)} must not have a NUL byte in its value"} + true -> :ok + end + end + + defp nul?(binary) when is_binary(binary), do: String.contains?(binary, <<0>>) + + # The key, quoted, for a message a caller reads. An over-long key is cut + # rather than echoed whole: the message identifies which entry to fix, and a + # 4KB key in an error body helps nobody. Cut by bytes, because bytes are + # what the limit is measured in — and then backed off to a whole codepoint, + # since a message sliced through the middle of one is not valid UTF-8 and + # `Jason.encode!` would raise on it instead of rendering the 422. + defp describe(key) when is_binary(key) do + shown = + if byte_size(key) > @max_key_bytes, + do: cut(key, @max_key_bytes) <> "...", + else: key + + ~s("#{shown}") + end + + defp describe(key), do: inspect(key) + + defp cut(_binary, bytes) when bytes <= 0, do: "" + + defp cut(binary, bytes) do + candidate = binary_part(binary, 0, bytes) + if String.valid?(candidate), do: candidate, else: cut(binary, bytes - 1) + end + + @doc """ + Merge `incoming` into `current`. A key with a `nil` value is removed; a key + that is absent is left alone. + + Nothing is validated here — the merged map goes through `changeset/1` on + its way to the row, so a write that would break a limit is refused with the + key named rather than half-applied. + """ + @spec merge(map() | nil, map()) :: map() + def merge(current, incoming) when is_map(incoming) do + Enum.reduce(incoming, current || %{}, fn + {key, nil}, acc -> Map.delete(acc, key) + {key, value}, acc -> Map.put(acc, key, value) + end) + end + + @doc """ + Which keys `merge/2` would change, as `{written, removed}` — both sorted, + both keys only. + + The audit trail records these and never the values (ADR 0013). + """ + @spec changed_keys(map() | nil, map()) :: {[String.t()], [String.t()]} + def changed_keys(current, incoming) when is_map(incoming) do + current = current || %{} + + {removed, written} = + incoming + |> Enum.filter(fn {key, value} -> Map.get(current, key, :absent) != value end) + |> Enum.split_with(fn {_key, value} -> is_nil(value) end) + + {written |> Enum.map(&to_string(elem(&1, 0))) |> Enum.sort(), + removed + |> Enum.map(&to_string(elem(&1, 0))) + |> Enum.filter(&Map.has_key?(current, &1)) + |> Enum.sort()} + end + + @doc """ + Merge the labels an agent stamped on its own run over the ACP extension + notification (#1637). + + Unscoped, hence the prefix: it takes a bare conversation id and writes to + that row without a `user_id` and without a credential check. The only + caller is `Fountain.Conversations.TurnMachine`, running inside the + conversation's own `ConversationServer`, holding the id that server was + started with — so it cannot name another tenant's conversation, or another + conversation of the same tenant. A request-shaped caller wants + `Fountain.Conversations.set_conversation_labels/4`, which scopes by + `user_id` and applies the sandbox rule. + + Recorded as `sprite` — code in the sandbox acting on the tenant's behalf + (ADR 0013). + + **Nothing a label contains can take the turn down.** A stamp the limits + refuse is logged and dropped, and so is one that raises on its way to the + database. The run is mid-turn and doing real work, and losing it because a + value was 300 bytes long, or held a byte `jsonb` will not store, would be + the worse outcome by a distance. + """ + @spec _unsafe_stamp(String.t() | nil, map()) :: :ok + def _unsafe_stamp(conversation_id, labels) + + def _unsafe_stamp(conversation_id, labels) + when is_binary(conversation_id) and is_map(labels) do + # ownership: `conversation_id` is the id the calling `ConversationServer` + # was started with, held by its own turn machine. It cannot name another + # conversation, so both unscoped calls below are on this server's own row. + with %{} = conv <- Fountain.Conversations._unsafe_get_conversation(conversation_id), + {:error, changeset} <- + Fountain.Conversations._unsafe_merge_labels(conv, labels, actor: "sprite") do + Logger.warning( + "conv #{conversation_id}: refused _fountain/labels update: #{inspect(changeset.errors)}" + ) + end + + :ok + rescue + error -> + # The belt to `check_entry/2`'s braces. Every shape we know of is + # refused above with a message; this is here so that a shape we do not + # know of costs the stamp and not the turn. + Logger.warning( + "conv #{conversation_id}: _fountain/labels update raised: #{Exception.message(error)}" + ) + + :ok + end + + def _unsafe_stamp(_conversation_id, _labels), do: :ok + + @doc """ + Every `label` value in a raw query string, in the order they were sent. + + Read from the query string rather than from the parsed params because Plug + collapses a repeated key to its last value, and the filter is repeatable by + design. `label[]=` is accepted as well, for a client whose HTTP layer only + builds arrays that way. + """ + @spec from_query_string(String.t() | nil) :: [String.t()] + def from_query_string(nil), do: [] + + def from_query_string(query) when is_binary(query) do + for {key, value} <- URI.query_decoder(query), key in ["label", "label[]"], do: value + end + + @doc """ + Turn repeated `key:value` filter values into the map the list query + contains against. Splits on the first colon only, so a value may contain + colons of its own. + + `{:error, :invalid_label_filter}` for a value with no colon or an empty + key: a caller who typed `?label=prod` meant something, and matching + everything would be the wrong guess. + """ + @spec parse_filter([String.t()] | nil) :: {:ok, map()} | {:error, :invalid_label_filter} + def parse_filter(nil), do: {:ok, %{}} + + def parse_filter(values) when is_list(values) do + Enum.reduce_while(values, {:ok, %{}}, fn value, {:ok, acc} -> + case value |> to_string() |> String.trim() |> String.split(":", parts: 2) do + ["" | _] -> {:halt, {:error, :invalid_label_filter}} + [key, filter_value] -> {:cont, {:ok, Map.put(acc, key, filter_value)}} + [_no_colon] -> {:halt, {:error, :invalid_label_filter}} + end + end) + end +end diff --git a/apps/fountain/lib/fountain/conversations/turn_machine.ex b/apps/fountain/lib/fountain/conversations/turn_machine.ex index 0a07b465e..0d3dad61a 100644 --- a/apps/fountain/lib/fountain/conversations/turn_machine.ex +++ b/apps/fountain/lib/fountain/conversations/turn_machine.ex @@ -44,7 +44,7 @@ defmodule Fountain.Conversations.TurnMachine do require OpenTelemetry.Tracer alias Fountain.{Agents, Conversations} - alias Fountain.Conversations.Conversation + alias Fountain.Conversations.{Conversation, Labels} @typedoc "What the peer reports about a turn, with the command ref already matched." @type payload :: tuple() @@ -169,13 +169,34 @@ defmodule Fountain.Conversations.TurnMachine do # the full quiet window: `running` status, deferred idle park, billed turn # time. Metadata is nothing the agent did, so it opens no turn and re-arms # no quiet timer; with a turn in flight it still lands on the transcript. + # + # `_fountain/labels` is the third exception (#1637). ACP reserves a leading + # underscore for extensions, and `session/update` is the only notification + # the peer forwards, so that is where a deterministic run stamps its own + # outcome. It is a control message and not something the agent said, so it + # opens no turn, re-arms no quiet timer and never reaches the transcript. def handle(%__MODULE__{} = turn, {:lines, stream, data}, ctx) do + labels = if stream == "acp", do: label_update(data) + cond do stream == "acp" and MapSet.member?(turn.replay_dedup, data) -> # A replayed line we already hold (ACP reattach). Each persisted line # suppresses at most one arrival, so a legitimate later repeat survives. {%{turn | replay_dedup: MapSet.delete(turn.replay_dedup, data)}, []} + is_map(labels) -> + # Written here rather than handed back as an effect: nothing about it + # is the server's — no state, no process, no timer — and this machine + # already writes what a report means. + # + # Ownership: `turn.conversation_id` is the id this machine's own + # `ConversationServer` was started with, so the unscoped write below + # cannot name another conversation, of this tenant or any other. + # Nothing a stamp contains can fail the turn; `_unsafe_stamp/2` logs + # and drops instead. + Labels._unsafe_stamp(turn.conversation_id, labels) + {turn, []} + stream == "acp" and Managoat.ACP.Protocol.session_metadata?(data) -> if is_nil(turn.row) do # No turn to attach it to, and not worth opening one: dropped. @@ -489,6 +510,41 @@ defmodule Fountain.Conversations.TurnMachine do {turn, finish ++ [{:drop_connection, "failed"}]} end + # The extension update a run stamps its own outcome with (#1637): + # + # {"jsonrpc":"2.0","method":"session/update","params":{ + # "sessionId":"...", + # "update":{"sessionUpdate":"_fountain/labels", + # "labels":{"drift":"true","env":"prod"}}}} + # + # ACP reserves a leading `_` for extensions, and `Managoat.ACP.Peer` + # forwards `session/update` and drops every other notification method, so + # this rides there rather than on a method of its own. + # + # A cheap substring test before the decode: this runs on every protocol line + # of every turn, and all but a handful of them are agent output. + @label_kind "_fountain/labels" + + defp label_update(data) do + if String.contains?(data, @label_kind), do: decode_label_update(data) + end + + defp decode_label_update(data) do + case Managoat.ACP.Protocol.classify_line(data) do + {:notification, "session/update", %{"update" => %{"sessionUpdate" => @label_kind} = update}} -> + # An `update` carrying no `labels` object is read as an empty merge, + # which writes nothing. Raising on it would cost the run its turn for + # a stamp, which is the wrong trade. + case Map.get(update, "labels") do + labels when is_map(labels) -> labels + _ -> %{} + end + + _ -> + nil + end + end + # ── ending a turn ───────────────────────────────────────────────────────── @doc """ diff --git a/apps/fountain/lib/fountain/team.ex b/apps/fountain/lib/fountain/team.ex index cc1ab4435..b13af56fe 100644 --- a/apps/fountain/lib/fountain/team.ex +++ b/apps/fountain/lib/fountain/team.ex @@ -373,10 +373,15 @@ defmodule Fountain.Team do %{conversation: conv} -> if live?(conv) do - case ConversationServer.send_prompt(conv.id, text, images, opts) do - :ok -> {:ok, Conversations.get_conversation(conv.id, user_id) || conv} - {:error, :gone} -> start_fresh(user_id, agent_id, conv, text, images, opts) - {:error, _} = err -> err + # Labels before the prompt (#1637): merged first, so a label the + # limits refuse means nothing happened at all rather than "the + # message went and the labels did not". + with {:ok, conv} <- label(conv, opts) do + case ConversationServer.send_prompt(conv.id, text, images, opts) do + :ok -> {:ok, Conversations.get_conversation(conv.id, user_id) || conv} + {:error, :gone} -> start_fresh(user_id, agent_id, conv, text, images, opts) + {:error, _} = err -> err + end end else start_fresh(user_id, agent_id, conv, text, images, opts) @@ -384,6 +389,22 @@ defmodule Fountain.Team do end end + # `opts[:labels]` goes onto the conversation the message lands on (#1637). + # On the fresh path they ride in the create attrs instead, so a conversation + # that never existed is not labelled twice. + # + # Through `Conversations.set_conversation_labels/4`, not the writer beneath + # it: this route accepts a sandbox's own `sprite` token, and the teammate's + # conversation is somebody else's conversation as far as that token is + # concerned. The door is where the rule lives, so the refusal is the same + # one `PATCH .../labels` gives. + defp label(%Conversation{} = conv, opts) do + case Keyword.get(opts, :labels) do + nil -> {:ok, conv} + labels -> Conversations.set_conversation_labels(conv.id, conv.user_id, labels, opts) + end + end + # A new conversation under the team binding, seeded with the message. Not # `start_or_resume`: we are here precisely because the bound conversation # cannot be resumed, and `find_channel_conversation` would agree — but @@ -401,7 +422,8 @@ defmodule Fountain.Team do "images" => images, "title" => prev.title, "environment_id" => prev.environment_id, - "vault_id" => prev.vault_id + "vault_id" => prev.vault_id, + "labels" => Keyword.get(opts, :labels) || %{} }, opts ) @@ -746,10 +768,10 @@ defmodule Fountain.Team do first (`live?/1`, else the newest), then the retired ones newest first — a previous computer's thread, still bound to the channel until the teammate is removed. `[]` when the agent is not on the team. """ - def list_teammate_conversations(user_id, agent_id) + def list_teammate_conversations(user_id, agent_id, opts \\ []) when is_binary(user_id) and is_binary(agent_id) do case user_id - |> Conversations.list_channel_conversations(@channel) + |> Conversations.list_channel_conversations(@channel, opts) |> Enum.filter(&(&1.agent_id == agent_id)) do [] -> [] diff --git a/apps/fountain/lib/fountain/webhooks.ex b/apps/fountain/lib/fountain/webhooks.ex index dca685406..1134840b0 100644 --- a/apps/fountain/lib/fountain/webhooks.ex +++ b/apps/fountain/lib/fountain/webhooks.ex @@ -327,7 +327,8 @@ defmodule Fountain.Webhooks do user_id: c.user_id, agent_id: c.agent_id, parent_conversation_id: c.parent_conversation_id, - status: c.status + status: c.status, + labels: c.labels }, e } @@ -340,7 +341,8 @@ defmodule Fountain.Webhooks do end @doc """ - The event envelope. Ids, a stage, a status, a duration. Nothing else, ever. + The event envelope. Ids, a stage, a status, a duration, the labels. + Nothing else, ever. `status` is the conversation's status as read at dispatch time, which can be stale by a hair against the transition that triggered it. Documented as @@ -357,6 +359,10 @@ defmodule Fountain.Webhooks do "agent_id" => conv.agent_id, "parent_conversation_id" => conv.parent_conversation_id, "status" => conv.status, + # The conversation's labels (#1637). Ids and facts a program put + # there itself, never content, which is what keeps them inside the + # "ids, a stage, a status, a duration" rule above. + "labels" => conv.labels || %{}, "stage" => event.stage, "state" => event.state, "turn_id" => event.turn_id, diff --git a/apps/fountain/lib/fountain_web/components/core_components.ex b/apps/fountain/lib/fountain_web/components/core_components.ex index a7b9bcf40..1194c2bba 100644 --- a/apps/fountain/lib/fountain_web/components/core_components.ex +++ b/apps/fountain/lib/fountain_web/components/core_components.ex @@ -218,6 +218,47 @@ defmodule FountainWeb.CoreComponents do attr :status, :string, required: true def status_badge(assigns), do: badge(assigns) + # ──────────────────────────────────────────────────────────────────────────── + # label_chips/1 (#1637) + # + # A conversation's labels, as small `key=value` chips. Sorted by key, so a + # row reads the same on every render. Renders nothing at all when there are + # none — an empty row of chips is noise on a list where most conversations + # carry no labels. + # + # `href` makes each chip a link to the same list filtered by that one label. + # Give it a function of `{key, value}`; leave it nil for a read-only list. + # ──────────────────────────────────────────────────────────────────────────── + + attr :labels, :map, default: %{} + attr :href, :any, default: nil + + def label_chips(assigns) do + assigns = assign(assigns, :sorted, Enum.sort_by(assigns.labels || %{}, &elem(&1, 0))) + + ~H""" + + <.link + :for={{key, value} <- @sorted} + :if={@href} + patch={@href.({key, value})} + class="rounded bg-[var(--color-bg-2)] px-1.5 py-0.5 font-mono text-[11px] text-[var(--color-text-secondary)] hover:text-[var(--color-text)]" + data-label-chip={key} + > + {key}={value} + + + {key}={value} + + + """ + end + # ──────────────────────────────────────────────────────────────────────────── # modal/1 # Accessible: FocusTrap JS hook traps Tab, phx-window-keydown closes on diff --git a/apps/fountain/lib/fountain_web/controllers/conversation_controller.ex b/apps/fountain/lib/fountain_web/controllers/conversation_controller.ex index 524163a58..aba090abb 100644 --- a/apps/fountain/lib/fountain_web/controllers/conversation_controller.ex +++ b/apps/fountain/lib/fountain_web/controllers/conversation_controller.ex @@ -9,10 +9,17 @@ defmodule FountainWeb.ConversationController do alias Fountain.Conversations alias Fountain.Conversations.{ConversationServer, LogEvent} alias FountainWeb.Audited + alias FountainWeb.LabelFilter + alias FountainWeb.SandboxKey alias FountainWeb.Schemas action_fallback FountainWeb.FallbackController + # Before the cast, and only where the parameter exists: `label` is a + # repeated key, and the cast reads query parameters out of Plug, which + # keeps only the last of them. See the plug's moduledoc. + plug FountainWeb.Plugs.RepeatedQueryParam, "label" when action in [:index] + plug OpenApiSpex.Plug.CastAndValidate, replace_params: false, render_error: FountainWeb.Plugs.CastRenderError @@ -57,6 +64,20 @@ defmodule FountainWeb.ConversationController do required: false, description: "Comma-separated statuses to keep (`idle,terminated`); 400 on a value outside the vocabulary." + ], + label: [ + in: :query, + schema: %OpenApiSpex.Schema{type: :array, items: %OpenApiSpex.Schema{type: :string}}, + style: :form, + explode: true, + required: false, + description: + "Only conversations carrying these `key:value` labels (#1637). Repeat the " <> + "parameter to combine them with AND: `?label=env:prod&label=drift:true` keeps " <> + "the conversations that carry both. `label[]=` is accepted as well. Each value " <> + "splits on its first colon only, so `label=path:a:b` matches the label `path` " <> + "with the value `a:b`. 400 `invalid_label_filter` on a value with no colon or " <> + "an empty key." ] ], responses: [ @@ -70,7 +91,8 @@ defmodule FountainWeb.ConversationController do user = conn.assigns.current_user roots_only = parse_bool_param(params["roots_only"], false) - with {:ok, statuses} <- parse_statuses(params["status"]) do + with {:ok, statuses} <- parse_statuses(params["status"]), + {:ok, labels} <- LabelFilter.from(conn) do render(conn, :index, conversations: Conversations.list_conversations(user.id, @@ -78,7 +100,8 @@ defmodule FountainWeb.ConversationController do agent_id: params["agent_id"], channel_id: params["channel_id"], sandbox_id: params["sandbox_id"], - status: statuses + status: statuses, + labels: labels ) ) end @@ -123,6 +146,45 @@ defmodule FountainWeb.ConversationController do end end + operation(:labels, + summary: "Set a conversation's labels", + description: + "Merges `labels` into the conversation's own (#1637). A key the body does not name " <> + "is left alone, and a key whose value is `null` is removed, so a run can stamp one " <> + "outcome without reading the rest first.\n\n" <> + "At most 32 labels survive the merge; a key is at most 64 bytes and a value at most " <> + "256 bytes. A 422 names the offending key.\n\n" <> + "The account's own key may label any of its conversations. A sandbox callback token " <> + "may label **only the conversation it was minted for**; another id is refused with " <> + "403 `sprite_may_not_label_another_conversation`. An agent inside a turn does not " <> + "need this route at all: it sends the `_fountain/labels` ACP extension update " <> + "instead.", + parameters: [conversation_id: [in: :path, type: :string, required: true]], + request_body: {"Labels", "application/json", Schemas.ConversationLabelsRequest}, + responses: [ + ok: {"Conversation", "application/json", Schemas.ConversationResponse}, + not_found: {"Not found", "application/json", Schemas.Error}, + forbidden: + {"A sandbox token labelling another conversation", "application/json", Schemas.Error}, + unprocessable_entity: + {"Invalid labels", "application/json", Schemas.UnprocessableEntityError} + ] + ) + + def labels(conn, %{"conversation_id" => id} = params) do + user = conn.assigns.current_user + opts = SandboxKey.opts(conn) ++ Audited.attribution(conn) + + with {:ok, conv} <- + Conversations.set_conversation_labels(id, user.id, params["labels"], opts) do + # Re-read annotated, so this renders the same conversation object every + # other conversation route does rather than one with a null turn_count. + render(conn, :show, + conversation: Conversations.get_conversation_with_activity(conv.id, user.id) + ) + end + end + operation(:tree, summary: "Get the conversation's spawn tree", description: @@ -392,6 +454,9 @@ defmodule FountainWeb.ConversationController do "With `channel_id`, resumes the latest live conversation already bound to that " <> "channel for the same agent and vault (200, `meta.resumed: true`) instead of " <> "opening a new one (201). " <> + "`labels` (#1637) are stamped on the new conversation; with `channel_id`, a resume " <> + "merges them into the conversation it hands back, and a sandbox callback token " <> + "resuming a conversation it was not minted for is refused with 403. " <> "Pass `X-Fountain-Parent-Conversation-Id` header to record which conversation spawned this one. " <> "Legacy `X-AoD-Parent-Conversation-Id` is still accepted for sprites provisioned before the rename.", request_body: {"Conversation attrs", "application/json", Schemas.ConversationCreateRequest}, @@ -402,6 +467,9 @@ defmodule FountainWeb.ConversationController do created: {"Conversation", "application/json", Schemas.ConversationResponse}, ok: {"Conversation (resumed by channel_id)", "application/json", Schemas.ConversationResponse}, + forbidden: + {"A sandbox token labelling the conversation a resume landed on", "application/json", + Schemas.Error}, not_found: {"Agent not found", "application/json", Schemas.Error}, unprocessable_entity: {"Validation error", "application/json", Schemas.UnprocessableEntityError}, @@ -439,9 +507,14 @@ defmodule FountainWeb.ConversationController do |> Map.put("parent_conversation_id", parent_id) |> Map.put("user_id", user.id) + # `SandboxKey.opts/1` rides along because a `channel_id` resume lands on an + # *existing* conversation and merges this request's labels into it (#1637); + # without it a sandbox token could relabel any conversation of the tenant + # by resuming its channel. + opts = SandboxKey.opts(conn) ++ Audited.attribution(conn) + with :ok <- Billing.check_spend(user), - {:ok, conv, outcome} <- - Conversations.start_or_resume_conversation(params, Audited.attribution(conn)) do + {:ok, conv, outcome} <- Conversations.start_or_resume_conversation(params, opts) do # 201 when a conversation was opened; 200 when `channel_id` resumed an # existing one (#774). Same body either way, so a client that ignores # the status still gets the id it needs. diff --git a/apps/fountain/lib/fountain_web/controllers/conversation_json.ex b/apps/fountain/lib/fountain_web/controllers/conversation_json.ex index 75d765d6d..c77a98769 100644 --- a/apps/fountain/lib/fountain_web/controllers/conversation_json.ex +++ b/apps/fountain/lib/fountain_web/controllers/conversation_json.ex @@ -64,6 +64,9 @@ defmodule FountainWeb.ConversationJSON do source: c.source, parent_conversation_id: c.parent_conversation_id, channel_id: c.channel_id, + # Free-form key/value strings (#1637): what a program stamped on its own + # run, and what `?label=env:prod` filters the list by. + labels: c.labels || %{}, turn_count: c.turn_count, last_active_at: c.last_active_at, last_read_at: c.last_read_at, diff --git a/apps/fountain/lib/fountain_web/controllers/fallback_controller.ex b/apps/fountain/lib/fountain_web/controllers/fallback_controller.ex index 11df9fd3c..903d7a7ee 100644 --- a/apps/fountain/lib/fountain_web/controllers/fallback_controller.ex +++ b/apps/fountain/lib/fountain_web/controllers/fallback_controller.ex @@ -106,6 +106,24 @@ defmodule FountainWeb.FallbackController do }) end + # A sandbox's per-conversation token naming a conversation it was not minted + # for (#1637). 403 rather than 404: the holder knows the conversation exists + # — it belongs to the account the token authenticates as — and what is being + # refused is the credential, not the id. The same shape and the same reason + # as `sprite_may_not_answer` on the permission route. + # + # Here rather than in a controller because three doors write labels: the + # labels route, a team message, and a `channel_id` resume on conversation + # create. + def call(conn, {:error, :sprite_may_not_label_another_conversation}) do + conn + |> put_status(:forbidden) + |> json(%{ + error: "sprite_may_not_label_another_conversation", + message: "a sandbox callback token may label only the conversation it was minted for" + }) + end + # An unknown or cross-tenant parent conversation. 404 rather than 403 so the # caller cannot use the response to probe which conversation ids exist. def call(conn, {:error, :parent_not_found}) do diff --git a/apps/fountain/lib/fountain_web/controllers/team_controller.ex b/apps/fountain/lib/fountain_web/controllers/team_controller.ex index ee324fe94..3a1cade74 100644 --- a/apps/fountain/lib/fountain_web/controllers/team_controller.ex +++ b/apps/fountain/lib/fountain_web/controllers/team_controller.ex @@ -19,6 +19,9 @@ defmodule FountainWeb.TeamController do action_fallback FountainWeb.FallbackController + # Before the cast: `label` is a repeated key. See the plug's moduledoc. + plug FountainWeb.Plugs.RepeatedQueryParam, "label" when action in [:conversations] + plug OpenApiSpex.Plug.CastAndValidate, replace_params: false, render_error: FountainWeb.Plugs.CastRenderError @@ -120,9 +123,23 @@ defmodule FountainWeb.TeamController do "thread, read-only — behind it. Each is a full conversation object; read a " <> "retired thread with `GET /api/conversations/:id/events`. 404 when the agent is " <> "not on the team.", - parameters: [agent_id: [in: :path, type: :string, required: true]], + parameters: [ + agent_id: [in: :path, type: :string, required: true], + label: [ + in: :query, + schema: %OpenApiSpex.Schema{type: :array, items: %OpenApiSpex.Schema{type: :string}}, + style: :form, + explode: true, + required: false, + description: + "Only conversations carrying these `key:value` labels (#1637). Repeatable and " <> + "AND-combined, exactly as on `GET /api/conversations`. 400 " <> + "`invalid_label_filter` on a value with no colon or an empty key." + ] + ], responses: [ ok: {"Conversations", "application/json", Schemas.TeammateConversationListResponse}, + bad_request: {"Invalid label filter", "application/json", Schemas.Error}, not_found: {"Not on the team", "application/json", Schemas.Error} ] ) @@ -130,15 +147,12 @@ defmodule FountainWeb.TeamController do def conversations(conn, %{"agent_id" => agent_id}) do user = conn.assigns.current_user - case Team.get_teammate(user.id, agent_id) do - nil -> - {:error, :not_found} - - %{conversation: current} -> - render(conn, :conversations, - conversations: Team.list_teammate_conversations(user.id, agent_id), - current_id: current.id - ) + with {:ok, labels} <- FountainWeb.LabelFilter.from(conn), + %{conversation: current} <- Team.get_teammate(user.id, agent_id) || {:error, :not_found} do + render(conn, :conversations, + conversations: Team.list_teammate_conversations(user.id, agent_id, labels: labels), + current_id: current.id + ) end end @@ -370,12 +384,17 @@ defmodule FountainWeb.TeamController do "seeded with this message, so the response names the conversation the message " <> "went to. 400 `conversation_busy` while the previous turn is still running " <> "(the same shape as `POST /api/conversations/:id/prompts`), 503 while the " <> - "computer is still starting.", + "computer is still starting.\n\n" <> + "`labels` merges onto the conversation the message lands on (#1637), before the " <> + "turn is queued, so a label the limits refuse leaves the message unsent.", parameters: [agent_id: [in: :path, type: :string, required: true]], request_body: {"Message", "application/json", Schemas.TeamMessageRequest}, responses: [ accepted: {"Queued", "application/json", Schemas.TeamMessageResponse}, not_found: {"Not on the team", "application/json", Schemas.Error}, + unprocessable_entity: {"Invalid labels", "application/json", Schemas.ChangesetError}, + forbidden: + {"A sandbox token labelling another conversation", "application/json", Schemas.Error}, bad_request: {"A turn is still running", "application/json", Schemas.Error} ] ) @@ -383,8 +402,12 @@ defmodule FountainWeb.TeamController do def message(conn, %{"agent_id" => agent_id, "prompt" => prompt} = params) do user = conn.assigns.current_user + # `SandboxKey.opts/1`: this route writes labels onto the teammate's + # conversation (#1637), which a sandbox's own token does not own. with {:ok, images} <- FountainWeb.PromptImages.decode(params["images"]), - opts = [source: "api"] ++ Audited.attribution(conn), + opts = + [source: "api", labels: params["labels"]] ++ + FountainWeb.SandboxKey.opts(conn) ++ Audited.attribution(conn), {:ok, conv} <- Team.send_message(user.id, agent_id, prompt, images, opts) do conn |> put_status(:accepted) diff --git a/apps/fountain/lib/fountain_web/label_filter.ex b/apps/fountain/lib/fountain_web/label_filter.ex new file mode 100644 index 000000000..69d28ae5e --- /dev/null +++ b/apps/fountain/lib/fountain_web/label_filter.ex @@ -0,0 +1,59 @@ +defmodule FountainWeb.LabelFilter do + @moduledoc """ + The `?label=key:value` filter on a conversation list (#1637), for both + routes that take it. + + `GET /api/conversations` and `GET /api/team/:agent_id/conversations` accept + the same repeatable, AND-combined parameter, and a second copy of the + parsing in the team controller is exactly how the two would drift. + + Read from `conn.query_string` and not from `conn.params`, because Plug + collapses a repeated key to its last value and this filter is repeatable by + design. `Fountain.Conversations.Labels` owns the vocabulary; this is the + Plug half plus the error the fallback controller renders as a 400. + """ + + alias Fountain.Conversations.Labels + + @doc """ + The label filter on a request: `{:ok, %{"env" => "prod"}}`, or + `{:error, "invalid_label_filter"}` for a value with no colon or an empty + key, which `FountainWeb.FallbackController` renders as a 400. + + Reads the list `FountainWeb.Plugs.RepeatedQueryParam` left on the request. + `List.wrap/1` rather than a match, so a route that has not been given that + plug degrades to the single value Plug kept instead of raising. + """ + @spec from(Plug.Conn.t()) :: {:ok, map()} | {:error, String.t()} + def from(%Plug.Conn{} = conn) do + conn.params + |> Map.get("label") + |> List.wrap() + |> parse() + end + + @doc """ + The same filter out of a LiveView's `uri`, for the console's conversation + list. + + A LiveView gets no plug pipeline and its `params` are collapsed the same + way Plug collapses them, so the repeated key has to be read back off the + URI. One parser either way, so the console and the API cannot disagree + about what `?label=env:prod` means. + """ + @spec from_uri(String.t()) :: {:ok, map()} | {:error, String.t()} + def from_uri(uri) when is_binary(uri) do + uri + |> URI.parse() + |> Map.get(:query) + |> Labels.from_query_string() + |> parse() + end + + defp parse(values) do + case Labels.parse_filter(values) do + {:ok, labels} -> {:ok, labels} + {:error, :invalid_label_filter} -> {:error, "invalid_label_filter"} + end + end +end diff --git a/apps/fountain/lib/fountain_web/live/admin_live/user_detail.ex b/apps/fountain/lib/fountain_web/live/admin_live/user_detail.ex index aa3c1eaa8..a844fffbd 100644 --- a/apps/fountain/lib/fountain_web/live/admin_live/user_detail.ex +++ b/apps/fountain/lib/fountain_web/live/admin_live/user_detail.ex @@ -211,6 +211,7 @@ defmodule FountainWeb.AdminLive.UserDetail do {String.slice(c.id, 0, 8)} {c.title} + <.label_chips labels={c.labels} />
+ No conversation carries every one of those labels. +
+| <.conv_link conversation={c} app={@conversations_app} /> + <.label_chips + labels={c.labels} + href={fn {key, value} -> dashboard_path(Map.put(@label_filter, key, value)) end} + /> | <.badge status={c.status} /> |
diff --git a/apps/fountain/lib/fountain_web/plugs/repeated_query_param.ex b/apps/fountain/lib/fountain_web/plugs/repeated_query_param.ex
new file mode 100644
index 000000000..27380ac2e
--- /dev/null
+++ b/apps/fountain/lib/fountain_web/plugs/repeated_query_param.ex
@@ -0,0 +1,68 @@
+defmodule FountainWeb.Plugs.RepeatedQueryParam do
+ @moduledoc """
+ Collect a repeated query key into a list, before the OpenAPI cast sees it.
+
+ `?label=env:prod&label=drift:true` is one parameter sent twice, which is
+ what OpenAPI calls `style: form, explode: true` and what an array parameter
+ means on the wire. Two things get in the way:
+
+ * `Plug.Conn.Query` collapses a repeated key to its **last** value, so
+ `conn.params["label"]` is `"drift:true"` and the first filter is gone;
+ * `OpenApiSpex.CastParameters` reads query parameters straight out of
+ `Plug.Conn.fetch_query_params/1` and implements only the `explode:
+ false` (comma-joined) form itself, so a parameter *declared* as an
+ array would be handed that string and refuse it as "not an array" —
+ turning a perfectly ordinary `?label=env:prod` into a 422.
+
+ So the parameter cannot be declared honestly as an array until something
+ reshapes it first, and that is this plug. It reads the raw query string,
+ collects every occurrence of the named key, and writes the list back onto
+ both `query_params` and `params` so the cast and the action see the same
+ value. `name[]=` is collected too, for a client whose HTTP layer only knows
+ how to build arrays that way.
+
+ Declare it **before** `OpenApiSpex.Plug.CastAndValidate` in the controller,
+ and scope it to the actions that take the parameter:
+
+ plug FountainWeb.Plugs.RepeatedQueryParam, "label" when action in [:index]
+
+ A request that sends the key once still arrives as a one-element list, so
+ the action has one shape to read rather than two.
+ """
+
+ @behaviour Plug
+
+ @impl Plug
+ def init(name) when is_binary(name), do: name
+
+ @impl Plug
+ def call(%Plug.Conn{} = conn, name) do
+ conn = Plug.Conn.fetch_query_params(conn)
+
+ case collect(conn.query_string, name) do
+ [] -> conn
+ values -> put(conn, name, values)
+ end
+ end
+
+ defp collect(query, name) when is_binary(query) do
+ bracketed = name <> "[]"
+
+ for {key, value} <- URI.query_decoder(query), key in [name, bracketed], do: value
+ end
+
+ defp put(conn, name, values) do
+ %{
+ conn
+ | query_params: Map.put(conn.query_params, name, values),
+ params: put_param(conn.params, name, values)
+ }
+ end
+
+ # `params` is `%Plug.Conn.Unfetched{}` until the body parsers have run. Every
+ # route this plug is on has run them, but reshaping an unfetched struct into
+ # a map would quietly swallow the body, so leave it alone and let
+ # `query_params` carry the value.
+ defp put_param(%Plug.Conn.Unfetched{} = params, _name, _values), do: params
+ defp put_param(params, name, values) when is_map(params), do: Map.put(params, name, values)
+end
diff --git a/apps/fountain/lib/fountain_web/router.ex b/apps/fountain/lib/fountain_web/router.ex
index 059a89a63..9daae9d64 100644
--- a/apps/fountain/lib/fountain_web/router.ex
+++ b/apps/fountain/lib/fountain_web/router.ex
@@ -579,6 +579,13 @@ defmodule FountainWeb.Router do
# The JSON read-model for the log feed; /stream below is the tail (#519).
get "/events", ConversationController, :events, as: :events
post "/read", ConversationController, :read, as: :read
+ # Labels (#1637). Not behind :require_full_scope on purpose: the point
+ # of the route is that a sandbox stamps its own conversation.
+ # `Conversations.set_conversation_labels/4` is what refuses a sandbox
+ # token naming a different one, and it is the door the team message and
+ # a channel resume write through as well — the two other places a
+ # request can change a conversation's labels.
+ patch "/labels", ConversationController, :labels, as: :labels
# Answer a permission request the agent is blocked on (#940). Nested so
# the conversation is tenant-scoped before the request id is looked at.
post "/requests/:request_id", ConversationController, :answer_request, as: :answer_request
diff --git a/apps/fountain/lib/fountain_web/sandbox_key.ex b/apps/fountain/lib/fountain_web/sandbox_key.ex
new file mode 100644
index 000000000..a284d3e00
--- /dev/null
+++ b/apps/fountain/lib/fountain_web/sandbox_key.ex
@@ -0,0 +1,46 @@
+defmodule FountainWeb.SandboxKey do
+ @moduledoc """
+ Which sandbox credential a request was made with, for the contexts whose
+ rules depend on it.
+
+ A conversation hands its sandbox a `sprite`-scoped API key. That key
+ authenticates as the *account*, so every tenant-scoped check passes for
+ every conversation the account owns — which is exactly the gap ADR 0045
+ describes. A context that must tell "this sandbox's own conversation" from
+ "another conversation of the same tenant" needs the key's id, and
+ `FountainWeb.Audited.attribution/2` deliberately carries only the actor.
+
+ One derivation, in one place, so a new door cannot get it subtly wrong:
+ `opts/1` returns the keyword pair to append beside `attribution/1`, and is
+ `[]` for anything that is not a sandbox token (a session, the owner's own
+ full-scope key, a background caller), which every rule reads as "no sandbox
+ restriction applies".
+ """
+
+ alias Fountain.Accounts.ApiKey
+
+ @doc """
+ The api key id when the request carried a sandbox's per-conversation token,
+ and `nil` for anything else.
+ """
+ @spec id(Plug.Conn.t()) :: binary() | nil
+ def id(%Plug.Conn{} = conn) do
+ case conn.assigns[:current_api_key] do
+ %ApiKey{id: id, scopes: scopes} -> if "sprite" in scopes, do: id
+ _ -> nil
+ end
+ end
+
+ @doc """
+ `[sandbox_key_id: id]` for a sandbox token, `[]` otherwise. Append it to
+ `FountainWeb.Audited.attribution/1` on any door that writes a conversation
+ a sandbox might not own.
+ """
+ @spec opts(Plug.Conn.t()) :: keyword()
+ def opts(%Plug.Conn{} = conn) do
+ case id(conn) do
+ nil -> []
+ key_id -> [sandbox_key_id: key_id]
+ end
+ end
+end
diff --git a/apps/fountain/lib/fountain_web/schemas.ex b/apps/fountain/lib/fountain_web/schemas.ex
index 2d9a65cc4..cbaf56c80 100644
--- a/apps/fountain/lib/fountain_web/schemas.ex
+++ b/apps/fountain/lib/fountain_web/schemas.ex
@@ -368,6 +368,15 @@ defmodule FountainWeb.Schemas do
description:
"The external channel key this conversation is bound to, if it was created with one."
},
+ labels: %Schema{
+ type: :object,
+ additionalProperties: %Schema{type: :string},
+ description:
+ "Free-form key/value strings on the conversation. A program stamps its own " <>
+ "run with them (`env=prod`, `drift=true`) and `GET /api/conversations?label=env:prod` " <>
+ "filters on them. At most 32 entries; a key is at most 64 bytes and a value " <>
+ "at most 256 bytes. Always an object, empty when nothing set one."
+ },
turn_count: %Schema{type: :integer},
first_prompt: %Schema{
type: :string,
@@ -560,12 +569,46 @@ defmodule FountainWeb.Schemas do
"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."
+ },
+ labels: %Schema{
+ type: :object,
+ nullable: true,
+ additionalProperties: %Schema{type: :string},
+ 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."
}
},
required: [:agent_id]
})
end
+ defmodule ConversationLabelsRequest do
+ @moduledoc false
+ require OpenApiSpex
+
+ OpenApiSpex.schema(%{
+ title: "ConversationLabelsRequest",
+ description:
+ "Labels to merge into a conversation. A key not named is left alone; a key whose " <>
+ "value is null is removed.",
+ type: :object,
+ properties: %{
+ labels: %Schema{
+ type: :object,
+ additionalProperties: %Schema{type: :string, nullable: true},
+ description:
+ "The pairs to merge. null removes a key. At most 32 entries survive the merge; " <>
+ "a key is at most 64 bytes and a value at most 256 bytes. A 422 names the " <>
+ "offending key under `errors.labels`."
+ }
+ },
+ required: [:labels]
+ })
+ end
+
defmodule PromptRequest do
@moduledoc false
require OpenApiSpex
@@ -2422,7 +2465,16 @@ defmodule FountainWeb.Schemas do
type: :object,
properties: %{
prompt: %Schema{type: :string},
- images: %Schema{type: :array, items: ImageInput, nullable: true}
+ images: %Schema{type: :array, items: ImageInput, nullable: true},
+ labels: %Schema{
+ type: :object,
+ nullable: true,
+ additionalProperties: %Schema{type: :string, nullable: true},
+ description:
+ "Labels to merge into the conversation this message lands on, whether that is " <>
+ "the teammate's current one or the fresh one a retired thread is replaced by. " <>
+ "Same limits as everywhere else; null removes a key."
+ }
},
required: [:prompt]
})
diff --git a/apps/fountain/priv/repo/migrations/20260906130000_add_labels_to_conversations.exs b/apps/fountain/priv/repo/migrations/20260906130000_add_labels_to_conversations.exs
new file mode 100644
index 000000000..89dec1386
--- /dev/null
+++ b/apps/fountain/priv/repo/migrations/20260906130000_add_labels_to_conversations.exs
@@ -0,0 +1,23 @@
+defmodule Fountain.Repo.Migrations.AddLabelsToConversations do
+ use Ecto.Migration
+
+ # #1637: free-form `key => value` strings a program stamps on its own run —
+ # `env=prod`, `drift=true` — so the list can slice by them. At most 32
+ # entries; the rule lives in `Fountain.Conversations.Labels`.
+ #
+ # The list filter is jsonb containment (`labels @> '{"env":"prod"}'`), which
+ # is what the GIN index serves. `jsonb_path_ops` rather than the default
+ # operator class: containment is the only operator the filter ever uses, and
+ # that class indexes whole paths instead of every key and value separately,
+ # so it is smaller and faster for exactly this query.
+ def change do
+ alter table(:conversations) do
+ add :labels, :map, null: false, default: %{}
+ end
+
+ create index(:conversations, ["labels jsonb_path_ops"],
+ using: :gin,
+ name: :conversations_labels_gin_index
+ )
+ end
+end
diff --git a/apps/fountain/test/fountain/audit_guardrail_test.exs b/apps/fountain/test/fountain/audit_guardrail_test.exs
index a9ac16200..c5acaea36 100644
--- a/apps/fountain/test/fountain/audit_guardrail_test.exs
+++ b/apps/fountain/test/fountain/audit_guardrail_test.exs
@@ -57,6 +57,7 @@ defmodule Fountain.AuditGuardrailTest do
{"inference credential clear", &__MODULE__.do_cred_clear/1, "inference_credential.delete"},
{"conversation delete", &__MODULE__.do_conv_delete/1, "conversation.deleted"},
{"conversation caller tools", &__MODULE__.do_caller_tools/1, "conversation.caller_tools_set"},
+ {"conversation labels", &__MODULE__.do_labels/1, "conversation.labels_set"},
{"sandbox reset", &__MODULE__.do_sandbox_reset/1, "sandbox.reset"},
{"role change", &__MODULE__.do_role_change/1, "account.role_changed"},
{"sandbox limit change", &__MODULE__.do_limit_change/1, "account.sandbox_limit_changed"},
@@ -399,6 +400,12 @@ defmodule Fountain.AuditGuardrailTest do
])
end
+ def do_labels(user) do
+ conv = insert_conversation(user_id: user.id, agent: insert_agent(user_id: user.id))
+
+ {:ok, _} = Conversations._unsafe_merge_labels(conv, %{"env" => "prod"})
+ end
+
def do_sandbox_reset(user) do
agent = insert_agent(user_id: user.id)
diff --git a/apps/fountain/test/fountain/conversations/conversation_server_acp_test.exs b/apps/fountain/test/fountain/conversations/conversation_server_acp_test.exs
index 8ff99621f..8e065a30d 100644
--- a/apps/fountain/test/fountain/conversations/conversation_server_acp_test.exs
+++ b/apps/fountain/test/fountain/conversations/conversation_server_acp_test.exs
@@ -1504,4 +1504,160 @@ defmodule Fountain.Conversations.ConversationServerACPTest do
GenServer.call(pid, {:park_caller_tool, "lookup_order", %{}, self()})
end
end
+
+ describe "labels over the ACP extension (#1637)" do
+ setup do
+ user = insert_verified_user()
+ conv = insert_conversation(agent: acp_agent(user), user_id: user.id)
+ {pid, ref} = start_acp_turn(conv)
+ {:ok, user: user, conv: conv, pid: pid, ref: ref}
+ end
+
+ defp labels_of(conv_id), do: Conversations._unsafe_get_conversation!(conv_id).labels
+
+ test "the agent stamps its own conversation mid-turn", %{conv: conv, pid: pid, ref: ref} do
+ drive_to_prompt(pid, ref)
+
+ notify(pid, ref, %{
+ "sessionUpdate" => "_fountain/labels",
+ "labels" => %{"drift" => "true", "env" => "prod"}
+ })
+
+ assert labels_of(conv.id) == %{"drift" => "true", "env" => "prod"}
+ end
+
+ test "a second stamp merges rather than replaces", %{conv: conv, pid: pid, ref: ref} do
+ drive_to_prompt(pid, ref)
+
+ notify(pid, ref, %{"sessionUpdate" => "_fountain/labels", "labels" => %{"env" => "prod"}})
+ notify(pid, ref, %{"sessionUpdate" => "_fountain/labels", "labels" => %{"run" => "17"}})
+
+ assert labels_of(conv.id) == %{"env" => "prod", "run" => "17"}
+ end
+
+ test "a null value removes a key", %{conv: conv, pid: pid, ref: ref} do
+ drive_to_prompt(pid, ref)
+
+ notify(pid, ref, %{
+ "sessionUpdate" => "_fountain/labels",
+ "labels" => %{"env" => "prod", "run" => "17"}
+ })
+
+ notify(pid, ref, %{"sessionUpdate" => "_fountain/labels", "labels" => %{"env" => nil}})
+
+ assert labels_of(conv.id) == %{"run" => "17"}
+ end
+
+ test "the stamp never reaches the transcript", %{conv: conv, pid: pid, ref: ref} do
+ drive_to_prompt(pid, ref)
+
+ notify(pid, ref, %{"sessionUpdate" => "_fountain/labels", "labels" => %{"env" => "prod"}})
+
+ events = Conversations._unsafe_list_log_events(conv.id)
+ refute Enum.any?(events, &(&1.data =~ "_fountain/labels"))
+ end
+
+ test "a stamp the limits refuse is dropped and the turn survives", %{
+ conv: conv,
+ pid: pid,
+ ref: ref
+ } do
+ prompt_id = drive_to_prompt(pid, ref)
+
+ notify(pid, ref, %{
+ "sessionUpdate" => "_fountain/labels",
+ "labels" => %{"note" => String.duplicate("v", 300)}
+ })
+
+ assert labels_of(conv.id) == %{}
+ assert Process.alive?(pid)
+
+ # And the turn still ends normally.
+ reply(pid, ref, prompt_id, %{"stopReason" => "end_turn"})
+ assert Conversations._unsafe_get_conversation!(conv.id).status == "idle"
+ end
+
+ # Postgres refuses a NUL inside a jsonb string. Before `check_entry/2`
+ # rejected it, the write raised a Postgrex.Error inside the turn machine,
+ # which travelled up through `drive_turn/2` and killed the server and the
+ # turn it was running — a label costing a run.
+ test "a NUL byte in a stamp costs the stamp and not the turn", %{
+ conv: conv,
+ pid: pid,
+ ref: ref
+ } do
+ prompt_id = drive_to_prompt(pid, ref)
+
+ notify(pid, ref, %{
+ "sessionUpdate" => "_fountain/labels",
+ "labels" => %{"note" => "before\u0000after"}
+ })
+
+ assert labels_of(conv.id) == %{}
+ assert Process.alive?(pid)
+
+ # The turn still ends, and a later legal stamp still lands.
+ notify(pid, ref, %{"sessionUpdate" => "_fountain/labels", "labels" => %{"env" => "prod"}})
+ assert labels_of(conv.id) == %{"env" => "prod"}
+
+ reply(pid, ref, prompt_id, %{"stopReason" => "end_turn"})
+ assert Conversations._unsafe_get_conversation!(conv.id).status == "idle"
+ end
+
+ # The extension is recognised by a decode, not by a substring: the cheap
+ # `String.contains?` in front of it is an optimisation, and agent prose
+ # that happens to mention the kind is still prose.
+ test "agent output that mentions the kind is still transcript", %{
+ conv: conv,
+ pid: pid,
+ ref: ref
+ } do
+ drive_to_prompt(pid, ref)
+
+ notify(pid, ref, %{
+ "sessionUpdate" => "agent_message_chunk",
+ "content" => %{
+ "type" => "text",
+ "text" => ~s|stamp it with {"sessionUpdate":"_fountain/labels"}|
+ }
+ })
+
+ events = Conversations._unsafe_list_log_events(conv.id)
+ assert Enum.any?(events, &(&1.stream == "acp" and &1.data =~ "_fountain/labels"))
+
+ # And it labelled nothing.
+ assert labels_of(conv.id) == %{}
+ end
+
+ test "a stamp out of turn opens no autonomous turn", %{conv: conv, pid: pid, ref: ref} do
+ prompt_id = drive_to_prompt(pid, ref)
+ reply(pid, ref, prompt_id, %{"stopReason" => "end_turn"})
+
+ before = length(Conversations._unsafe_list_turns(conv.id))
+
+ notify(pid, ref, %{"sessionUpdate" => "_fountain/labels", "labels" => %{"env" => "prod"}})
+
+ assert labels_of(conv.id) == %{"env" => "prod"}
+ assert length(Conversations._unsafe_list_turns(conv.id)) == before
+ end
+
+ test "the write is recorded as the sprite, with keys and no values", %{
+ user: user,
+ pid: pid,
+ ref: ref
+ } do
+ drive_to_prompt(pid, ref)
+
+ notify(pid, ref, %{"sessionUpdate" => "_fountain/labels", "labels" => %{"drift" => "true"}})
+
+ assert [event] =
+ user.id
+ |> Fountain.Audit.list_recent_for_user(50)
+ |> Enum.filter(&(&1.action == "conversation.labels_set"))
+
+ assert event.actor == "sprite"
+ assert event.metadata["keys"] == ["drift"]
+ refute inspect(event.metadata) =~ "true"
+ end
+ end
end
diff --git a/apps/fountain/test/fountain/conversations/labels_test.exs b/apps/fountain/test/fountain/conversations/labels_test.exs
new file mode 100644
index 000000000..85ad2870a
--- /dev/null
+++ b/apps/fountain/test/fountain/conversations/labels_test.exs
@@ -0,0 +1,508 @@
+defmodule Fountain.Conversations.LabelsTest do
+ @moduledoc """
+ Labels on a conversation (#1637): the rule, the merge, the filter.
+
+ The doors are covered where they live — the API in
+ `FountainWeb.ConversationLabelsTest`, the ACP extension in
+ `Fountain.Conversations.ConversationServerACPTest` — so what is here is the
+ behaviour every one of them inherits.
+ """
+
+ use Fountain.DataCase, async: true
+
+ alias Fountain.Audit
+ alias Fountain.Conversations
+ alias Fountain.Conversations.Labels
+
+ describe "the limits" do
+ test "a legal map passes" do
+ assert :ok = Labels.check(%{"env" => "prod", "drift" => "true"})
+ assert :ok = Labels.check(%{})
+ end
+
+ test "more than 32 entries names the key over the limit" do
+ labels = for n <- 1..33, into: %{}, do: {String.pad_leading("#{n}", 3, "0"), "x"}
+
+ assert {:error, message} = Labels.check(labels)
+ assert message =~ "at most 32 labels"
+ assert message =~ ~s("033")
+ end
+
+ test "an over-long key names it, cut rather than echoed whole" do
+ key = String.duplicate("k", 200)
+
+ assert {:error, message} = Labels.check(%{key => "v"})
+ assert message =~ "longer than 64 bytes"
+ assert message =~ String.duplicate("k", 64) <> "..."
+ refute message =~ String.duplicate("k", 65)
+ end
+
+ test "an over-long value names its key" do
+ assert {:error, message} = Labels.check(%{"note" => String.duplicate("v", 257)})
+ assert message =~ ~s("note")
+ assert message =~ "longer than 256 bytes"
+ end
+
+ test "a non-string value names its key" do
+ assert {:error, message} = Labels.check(%{"count" => 3})
+ assert message =~ ~s("count")
+ assert message =~ "string value"
+ end
+
+ test "an empty key is refused" do
+ assert {:error, message} = Labels.check(%{"" => "v"})
+ assert message =~ "must not be empty"
+ end
+
+ test "something that is not a map at all is refused" do
+ assert {:error, _} = Labels.check(["env:prod"])
+ end
+
+ # Postgres refuses a NUL inside a jsonb string, so an unchecked one is a
+ # raised Postgrex.Error rather than a refusal — a 500 on the HTTP doors
+ # and a dead ConversationServer on the ACP one.
+ test "a NUL byte in a value names its key" do
+ assert {:error, message} = Labels.check(%{"note" => "a\u0000b"})
+ assert message =~ ~s("note")
+ assert message =~ "NUL byte"
+ end
+
+ test "a NUL byte in a key is refused" do
+ assert {:error, message} = Labels.check(%{"a\u0000b" => "v"})
+ assert message =~ "NUL byte"
+ end
+
+ test "the boundary values are legal" do
+ assert :ok =
+ Labels.check(%{String.duplicate("k", 64) => String.duplicate("v", 256)})
+
+ assert :ok = Labels.check(for(n <- 1..32, into: %{}, do: {"k#{n}", "v"}))
+ end
+
+ test "an over-long key is cut to a whole codepoint, so the message stays encodable" do
+ # Cutting at 64 *bytes* lands mid-character here: "é" is two bytes, so
+ # byte 64 is the tail of one. A message sliced there is not valid UTF-8
+ # and Jason.encode! would raise on it instead of rendering the 422.
+ key = String.duplicate("é", 40)
+
+ assert {:error, message} = Labels.check(%{key => "v"})
+ assert String.valid?(message)
+ assert {:ok, _} = Jason.encode(%{errors: %{labels: [message]}})
+ end
+
+ test "the same offending key is named on every run, whatever the map order" do
+ labels = for n <- 1..40, into: %{}, do: {String.pad_leading("#{n}", 3, "0"), "x"}
+
+ assert {:error, first} = Labels.check(labels)
+ assert {:error, second} = Labels.check(Map.new(Enum.shuffle(labels)))
+ assert first == second
+ end
+ end
+
+ describe "merge" do
+ test "adds and overwrites, leaves the rest alone" do
+ assert %{"a" => "2", "b" => "1"} = Labels.merge(%{"a" => "1", "b" => "1"}, %{"a" => "2"})
+ end
+
+ test "a null value removes the key" do
+ assert %{"b" => "1"} = Labels.merge(%{"a" => "1", "b" => "1"}, %{"a" => nil})
+ end
+
+ test "removing a key that is not there is not an error" do
+ assert %{"b" => "1"} = Labels.merge(%{"b" => "1"}, %{"a" => nil})
+ end
+
+ test "changed_keys reports written and removed, sorted" do
+ current = %{"a" => "1", "b" => "1", "z" => "1"}
+ incoming = %{"a" => "1", "b" => "2", "c" => "3", "z" => nil, "gone" => nil}
+
+ # "a" is unchanged so it is not written; "gone" was never there, so
+ # removing it removed nothing and the trail does not claim otherwise.
+ assert {["b", "c"], ["z"]} = Labels.changed_keys(current, incoming)
+ end
+ end
+
+ describe "the filter vocabulary" do
+ test "splits on the first colon only" do
+ assert {:ok, %{"path" => "a:b"}} = Labels.parse_filter(["path:a:b"])
+ end
+
+ test "combines repeated values" do
+ assert {:ok, %{"env" => "prod", "drift" => "true"}} =
+ Labels.parse_filter(["env:prod", "drift:true"])
+ end
+
+ test "an empty value is a legal filter" do
+ assert {:ok, %{"env" => ""}} = Labels.parse_filter(["env:"])
+ end
+
+ test "a value with no colon is refused rather than guessed at" do
+ assert {:error, :invalid_label_filter} = Labels.parse_filter(["prod"])
+ end
+
+ test "an empty key is refused" do
+ assert {:error, :invalid_label_filter} = Labels.parse_filter([":prod"])
+ end
+
+ test "reads every repetition out of a raw query string" do
+ assert ["env:prod", "drift:true"] =
+ Labels.from_query_string("roots_only=true&label=env:prod&label=drift:true")
+ end
+
+ test "accepts the bracketed array form too" do
+ assert ["env:prod"] = Labels.from_query_string("label%5B%5D=env%3Aprod")
+ end
+ end
+
+ describe "on the row" do
+ setup do
+ user = insert_active_user()
+ {:ok, user: user}
+ end
+
+ test "a conversation created with labels reads them back", %{user: user} do
+ conv = insert_conversation(user_id: user.id, labels: %{"env" => "prod"})
+
+ assert %{"env" => "prod"} = Conversations.get_conversation(conv.id, user.id).labels
+ end
+
+ test "a conversation created without labels has an empty map", %{user: user} do
+ conv = insert_conversation(user_id: user.id)
+
+ assert %{} == Conversations.get_conversation(conv.id, user.id).labels
+ end
+
+ test "the changeset refuses a write over the limits, naming the key", %{user: user} do
+ conv = insert_conversation(user_id: user.id)
+
+ assert {:error, changeset} =
+ Conversations._unsafe_merge_labels(conv, %{"note" => String.duplicate("v", 300)})
+
+ assert %{labels: [message]} = errors_on(changeset)
+ assert message =~ ~s("note")
+ end
+
+ test "the count ceiling counts the merged result, not the request", %{user: user} do
+ thirty_two = for n <- 1..32, into: %{}, do: {"k#{n}", "v"}
+ conv = insert_conversation(user_id: user.id, labels: thirty_two)
+
+ assert {:error, changeset} =
+ Conversations._unsafe_merge_labels(conv, %{"one-too-many" => "v"})
+
+ assert %{labels: [message]} = errors_on(changeset)
+ assert message =~ "at most 32 labels"
+
+ # And it names the key the caller sent, not whichever of the 32 already
+ # on the row happens to sort into the boundary position.
+ assert message =~ ~s("one-too-many")
+ refute message =~ ~s("k1")
+ end
+
+ test "a write of many at once names the one that does not fit", %{user: user} do
+ conv = insert_conversation(user_id: user.id)
+ labels = for n <- 1..33, into: %{}, do: {String.pad_leading("#{n}", 3, "0"), "x"}
+
+ assert {:error, changeset} = Conversations._unsafe_merge_labels(conv, labels)
+ assert %{labels: [message]} = errors_on(changeset)
+ assert message =~ ~s("033")
+ end
+
+ test "a merge that only removes keys never trips the ceiling", %{user: user} do
+ thirty_two = for n <- 1..32, into: %{}, do: {"k#{n}", "v"}
+ conv = insert_conversation(user_id: user.id, labels: thirty_two)
+
+ assert {:ok, updated} =
+ Conversations._unsafe_merge_labels(conv, %{"k1" => nil, "new" => "v"})
+
+ assert map_size(updated.labels) == 32
+ assert updated.labels["new"] == "v"
+ refute Map.has_key?(updated.labels, "k1")
+ end
+ end
+
+ describe "_unsafe_merge_labels/3" do
+ setup do
+ user = insert_active_user()
+ conv = insert_conversation(user_id: user.id, labels: %{"env" => "staging"})
+ {:ok, user: user, conv: conv}
+ end
+
+ test "merges rather than replaces", %{conv: conv} do
+ assert {:ok, updated} = Conversations._unsafe_merge_labels(conv, %{"drift" => "true"})
+ assert updated.labels == %{"env" => "staging", "drift" => "true"}
+ end
+
+ test "a null value removes one key", %{conv: conv} do
+ assert {:ok, updated} = Conversations._unsafe_merge_labels(conv, %{"env" => nil})
+ assert updated.labels == %{}
+ end
+
+ test "records the keys that changed and never the values", %{user: user, conv: conv} do
+ assert {:ok, _} =
+ Conversations._unsafe_merge_labels(conv, %{"drift" => "true", "env" => nil})
+
+ assert [event] =
+ user.id
+ |> Audit.list_recent_for_user(50)
+ |> Enum.filter(&(&1.action == "conversation.labels_set"))
+
+ assert event.metadata["keys"] == ["drift"]
+ assert event.metadata["removed_keys"] == ["env"]
+ assert event.metadata["label_count"] == 1
+ refute event.metadata |> inspect() =~ "true"
+ end
+
+ test "a merge that changes nothing writes nothing and records nothing", %{
+ user: user,
+ conv: conv
+ } do
+ assert {:ok, same} = Conversations._unsafe_merge_labels(conv, %{"env" => "staging"})
+ assert same.updated_at == conv.updated_at
+
+ assert [] =
+ user.id
+ |> Audit.list_recent_for_user(50)
+ |> Enum.filter(&(&1.action == "conversation.labels_set"))
+ end
+ end
+
+ describe "set_conversation_labels/4" do
+ setup do
+ user = insert_active_user()
+ {key, _raw} = insert_sprite_api_key(user)
+
+ mine =
+ insert_conversation(user_id: user.id, callback_api_key_id: key.id, labels: %{"a" => "1"})
+
+ theirs = insert_conversation(user_id: user.id)
+
+ {:ok, user: user, key: key, mine: mine, theirs: theirs}
+ end
+
+ test "the owner's own key may label any of their conversations", %{
+ user: user,
+ theirs: theirs
+ } do
+ assert {:ok, updated} =
+ Conversations.set_conversation_labels(theirs.id, user.id, %{"env" => "prod"})
+
+ assert updated.labels == %{"env" => "prod"}
+ end
+
+ test "a sandbox token may label the conversation it was minted for", %{
+ user: user,
+ key: key,
+ mine: mine
+ } do
+ assert {:ok, updated} =
+ Conversations.set_conversation_labels(mine.id, user.id, %{"env" => "prod"},
+ sandbox_key_id: key.id,
+ actor: "sprite"
+ )
+
+ assert updated.labels == %{"a" => "1", "env" => "prod"}
+ end
+
+ test "a sandbox token may not label another conversation", %{
+ user: user,
+ key: key,
+ theirs: theirs
+ } do
+ assert {:error, :sprite_may_not_label_another_conversation} =
+ Conversations.set_conversation_labels(theirs.id, user.id, %{"env" => "prod"},
+ sandbox_key_id: key.id
+ )
+
+ assert Conversations.get_conversation(theirs.id, user.id).labels == %{}
+ end
+
+ test "labels that are not a map at all are a validation failure, not a no-op", %{
+ user: user,
+ theirs: theirs
+ } do
+ assert {:error, changeset} =
+ Conversations.set_conversation_labels(theirs.id, user.id, "env=prod")
+
+ assert %{labels: [message]} = errors_on(changeset)
+ assert message =~ "object of string keys"
+ assert Conversations.get_conversation(theirs.id, user.id).labels == %{}
+ end
+
+ test "another tenant's conversation reads as not found", %{user: user} do
+ other = insert_conversation(user_id: insert_active_user().id)
+
+ assert {:error, :not_found} =
+ Conversations.set_conversation_labels(other.id, user.id, %{"env" => "prod"})
+ end
+ end
+
+ describe "labels and the rest of the row" do
+ setup do
+ {:ok, user: insert_active_user()}
+ end
+
+ test "an unrelated update leaves them alone", %{user: user} do
+ conv = insert_conversation(user_id: user.id, labels: %{"env" => "prod"})
+
+ assert {:ok, updated} = Conversations.update_conversation(conv, %{status: "idle"})
+ assert updated.status == "idle"
+ assert updated.labels == %{"env" => "prod"}
+ assert Conversations.get_conversation(conv.id, user.id).labels == %{"env" => "prod"}
+ end
+
+ test "a title change leaves them alone", %{user: user} do
+ conv = insert_conversation(user_id: user.id, labels: %{"env" => "prod"})
+
+ assert {:ok, updated} = Conversations.update_conversation(conv, %{title: "renamed"})
+ assert updated.labels == %{"env" => "prod"}
+ end
+ end
+
+ describe "a channel resume" do
+ setup do
+ user = insert_active_user()
+ agent = insert_agent(user_id: user.id)
+
+ bound =
+ insert_conversation(
+ user_id: user.id,
+ agent: agent,
+ status: "idle",
+ channel_id: "chat:1",
+ labels: %{"env" => "prod"},
+ sandbox: insert_sandbox(user_id: user.id, status: "ready")
+ )
+
+ {:ok, user: user, agent: agent, bound: bound}
+ end
+
+ defp resume(context, extra) do
+ Conversations.start_or_resume_conversation(
+ Map.merge(
+ %{
+ "agent_id" => context.agent.id,
+ "user_id" => context.user.id,
+ "channel_id" => "chat:1"
+ },
+ extra
+ )
+ )
+ end
+
+ test "merges the request's labels into the conversation it hands back", context do
+ assert {:ok, conv, :resumed} = resume(context, %{"labels" => %{"run" => "17"}})
+ assert conv.id == context.bound.id
+ assert conv.labels == %{"env" => "prod", "run" => "17"}
+ end
+
+ test "a resume with no labels changes nothing", context do
+ assert {:ok, conv, :resumed} = resume(context, %{})
+ assert conv.labels == %{"env" => "prod"}
+ end
+
+ test "a null value removes a key on resume", context do
+ assert {:ok, conv, :resumed} = resume(context, %{"labels" => %{"env" => nil}})
+ assert conv.labels == %{}
+ end
+
+ test "a label the limits refuse fails the resume rather than being dropped", context do
+ assert {:error, changeset} =
+ resume(context, %{"labels" => %{"note" => String.duplicate("v", 300)}})
+
+ assert %{labels: [message]} = errors_on(changeset)
+ assert message =~ ~s("note")
+
+ assert Conversations.get_conversation(context.bound.id, context.user.id).labels ==
+ %{"env" => "prod"}
+ end
+
+ test "a sandbox token may not relabel a conversation it was not minted for", context do
+ {key, _raw} = insert_sprite_api_key(context.user)
+
+ assert {:error, :sprite_may_not_label_another_conversation} =
+ Conversations.start_or_resume_conversation(
+ %{
+ "agent_id" => context.agent.id,
+ "user_id" => context.user.id,
+ "channel_id" => "chat:1",
+ "labels" => %{"run" => "17"}
+ },
+ sandbox_key_id: key.id
+ )
+
+ assert Conversations.get_conversation(context.bound.id, context.user.id).labels ==
+ %{"env" => "prod"}
+ end
+ end
+
+ describe "the list filter" do
+ setup do
+ user = insert_active_user()
+
+ prod_drift =
+ insert_conversation(user_id: user.id, labels: %{"env" => "prod", "drift" => "true"})
+
+ prod_clean =
+ insert_conversation(user_id: user.id, labels: %{"env" => "prod", "drift" => "false"})
+
+ staging = insert_conversation(user_id: user.id, labels: %{"env" => "staging"})
+ unlabelled = insert_conversation(user_id: user.id)
+
+ {:ok,
+ user: user,
+ prod_drift: prod_drift,
+ prod_clean: prod_clean,
+ staging: staging,
+ unlabelled: unlabelled}
+ end
+
+ defp ids(convs), do: convs |> Enum.map(& &1.id) |> MapSet.new()
+
+ test "one pair keeps every conversation carrying it", context do
+ found = Conversations.list_conversations(context.user.id, labels: %{"env" => "prod"})
+
+ assert ids(found) == MapSet.new([context.prod_drift.id, context.prod_clean.id])
+ end
+
+ test "two pairs are combined with AND", context do
+ found =
+ Conversations.list_conversations(context.user.id,
+ labels: %{"env" => "prod", "drift" => "true"}
+ )
+
+ assert ids(found) == MapSet.new([context.prod_drift.id])
+ end
+
+ test "a pair nothing carries matches nothing", context do
+ assert [] = Conversations.list_conversations(context.user.id, labels: %{"env" => "qa"})
+ end
+
+ test "no filter leaves the list alone", context do
+ assert MapSet.size(ids(Conversations.list_conversations(context.user.id))) == 4
+ assert MapSet.size(ids(Conversations.list_conversations(context.user.id, labels: %{}))) == 4
+ end
+
+ test "the filter is tenant-scoped like every other", context do
+ stranger = insert_active_user()
+ insert_conversation(user_id: stranger.id, labels: %{"env" => "prod"})
+
+ found = Conversations.list_conversations(context.user.id, labels: %{"env" => "prod"})
+ assert ids(found) == MapSet.new([context.prod_drift.id, context.prod_clean.id])
+ end
+
+ test "combines with the other filters", context do
+ agent = insert_agent(user_id: context.user.id)
+
+ mine =
+ insert_conversation(user_id: context.user.id, agent: agent, labels: %{"env" => "prod"})
+
+ found =
+ Conversations.list_conversations(context.user.id,
+ agent_id: agent.id,
+ labels: %{"env" => "prod"}
+ )
+
+ assert ids(found) == MapSet.new([mine.id])
+ end
+ end
+end
diff --git a/apps/fountain/test/fountain/webhooks_test.exs b/apps/fountain/test/fountain/webhooks_test.exs
index 8829aba36..8d4c3fc1e 100644
--- a/apps/fountain/test/fountain/webhooks_test.exs
+++ b/apps/fountain/test/fountain/webhooks_test.exs
@@ -202,6 +202,7 @@ defmodule Fountain.WebhooksTest do
"agent_id",
"conversation_id",
"duration_ms",
+ "labels",
"parent_conversation_id",
"stage",
"state",
@@ -210,6 +211,25 @@ defmodule Fountain.WebhooksTest do
]
end
+ test "the payload carries the conversation's labels (#1637)", %{user: user, conv: conv} do
+ {endpoint, _} = endpoint_for(user, %{"event_types" => ["*"]})
+ {:ok, _} = Conversations._unsafe_merge_labels(conv, %{"env" => "prod", "drift" => "true"})
+
+ Conversations.publish_stage(conv.id, "turn", "done", %{turn_id: nil})
+
+ assert [job] = jobs_for(endpoint)
+ assert job.args["payload"]["data"]["labels"] == %{"env" => "prod", "drift" => "true"}
+ end
+
+ test "an unlabelled conversation still carries an object", %{user: user, conv: conv} do
+ {endpoint, _} = endpoint_for(user, %{"event_types" => ["*"]})
+
+ Conversations.publish_stage(conv.id, "turn", "done", %{turn_id: nil})
+
+ assert [job] = jobs_for(endpoint)
+ assert job.args["payload"]["data"]["labels"] == %{}
+ end
+
test "output events never dispatch", %{user: user, conv: conv} do
{endpoint, _} = endpoint_for(user, %{"event_types" => ["*"]})
diff --git a/apps/fountain/test/fountain_web/controllers/conversation_labels_test.exs b/apps/fountain/test/fountain_web/controllers/conversation_labels_test.exs
new file mode 100644
index 000000000..fb218d52c
--- /dev/null
+++ b/apps/fountain/test/fountain_web/controllers/conversation_labels_test.exs
@@ -0,0 +1,410 @@
+defmodule FountainWeb.ConversationLabelsTest do
+ @moduledoc """
+ Labels over the wire (#1637): create, read back, the repeatable AND filter,
+ the merge route and who is allowed to call it.
+ """
+
+ use FountainWeb.ConnCase, async: true
+ use Mimic
+
+ alias Fountain.Conversations
+
+ setup do
+ user = insert_active_user()
+ {_key, raw_key} = insert_api_key(user)
+ {:ok, user: user, raw_key: raw_key}
+ end
+
+ describe "POST /api/conversations with labels" do
+ # Nothing stubbed but the supervisor: the point is to exercise the real
+ # `start_conversation/2` attrs, which is where `labels` actually reaches
+ # the row. A stub of `start_or_resume_conversation/2` would test the view
+ # and leave the create path uncovered.
+ defp create_conversation(conn, raw_key, body) do
+ Mimic.stub(Horde.DynamicSupervisor, :start_child, fn _s, _spec ->
+ {:ok, spawn(fn -> :ok end)}
+ end)
+
+ conn |> authed_with_key(raw_key) |> post_json("/api/conversations", body)
+ end
+
+ test "creates with them and returns them", %{conn: conn, user: user, raw_key: raw_key} do
+ agent = insert_agent(user_id: user.id)
+
+ conn =
+ create_conversation(conn, raw_key, %{
+ "agent_id" => agent.id,
+ "labels" => %{"env" => "prod", "drift" => "true"}
+ })
+
+ assert %{"data" => data} = json_response(conn, 201)
+ assert data["labels"] == %{"env" => "prod", "drift" => "true"}
+
+ # And on the row, not only in the response the create rendered.
+ assert Conversations._unsafe_get_conversation!(data["id"]).labels ==
+ %{"env" => "prod", "drift" => "true"}
+ end
+
+ test "creating with none leaves an empty map on the row", %{
+ conn: conn,
+ user: user,
+ raw_key: raw_key
+ } do
+ agent = insert_agent(user_id: user.id)
+
+ conn = create_conversation(conn, raw_key, %{"agent_id" => agent.id})
+
+ assert %{"data" => %{"id" => id, "labels" => %{}}} = json_response(conn, 201)
+ assert Conversations._unsafe_get_conversation!(id).labels == %{}
+ end
+
+ test "a label the limits refuse is a 422 and creates nothing", %{
+ conn: conn,
+ user: user,
+ raw_key: raw_key
+ } do
+ agent = insert_agent(user_id: user.id)
+ before = length(Conversations.list_conversations(user.id))
+
+ conn =
+ create_conversation(conn, raw_key, %{
+ "agent_id" => agent.id,
+ "labels" => %{"note" => String.duplicate("v", 300)}
+ })
+
+ assert [message] = json_response(conn, 422)["errors"]["labels"]
+ assert message =~ ~s("note")
+ assert length(Conversations.list_conversations(user.id)) == before
+ end
+
+ test "a conversation with no labels serves an empty object, never null", %{
+ conn: conn,
+ user: user,
+ raw_key: raw_key
+ } do
+ conv = insert_conversation(user_id: user.id)
+
+ conn = conn |> authed_with_key(raw_key) |> get("/api/conversations/#{conv.id}")
+
+ assert %{"data" => %{"labels" => %{}}} = json_response(conn, 200)
+ end
+ end
+
+ describe "GET /api/conversations?label=" do
+ setup %{user: user} do
+ prod_drift =
+ insert_conversation(user_id: user.id, labels: %{"env" => "prod", "drift" => "true"})
+
+ prod_clean = insert_conversation(user_id: user.id, labels: %{"env" => "prod"})
+ staging = insert_conversation(user_id: user.id, labels: %{"env" => "staging"})
+
+ {:ok, prod_drift: prod_drift, prod_clean: prod_clean, staging: staging}
+ end
+
+ defp listed(conn),
+ do: conn |> json_response(200) |> Map.fetch!("data") |> Enum.map(& &1["id"])
+
+ test "one pair keeps the conversations carrying it", context do
+ ids =
+ context.conn
+ |> authed_with_key(context.raw_key)
+ |> get("/api/conversations?label=env:prod")
+ |> listed()
+
+ assert Enum.sort(ids) == Enum.sort([context.prod_drift.id, context.prod_clean.id])
+ end
+
+ test "a repeated label is combined with AND", context do
+ ids =
+ context.conn
+ |> authed_with_key(context.raw_key)
+ |> get("/api/conversations?label=env:prod&label=drift:true")
+ |> listed()
+
+ assert ids == [context.prod_drift.id]
+ end
+
+ test "combines with the other filters", context do
+ ids =
+ context.conn
+ |> authed_with_key(context.raw_key)
+ |> get("/api/conversations?status=pending&label=env:staging")
+ |> listed()
+
+ assert ids == [context.staging.id]
+ end
+
+ test "a value splits on its first colon only", %{conn: conn, user: user, raw_key: raw_key} do
+ conv = insert_conversation(user_id: user.id, labels: %{"path" => "apps/fountain:lib"})
+
+ ids =
+ conn
+ |> authed_with_key(raw_key)
+ |> get("/api/conversations?label=path:apps/fountain:lib")
+ |> listed()
+
+ assert ids == [conv.id]
+ end
+
+ test "a value with no colon is a 400 rather than a silent match-all", context do
+ conn =
+ context.conn
+ |> authed_with_key(context.raw_key)
+ |> get("/api/conversations?label=prod")
+
+ assert %{"error" => "invalid_label_filter"} = json_response(conn, 400)
+ end
+ end
+
+ describe "PATCH /api/conversations/:id/labels" do
+ setup %{user: user} do
+ conv = insert_conversation(user_id: user.id, labels: %{"env" => "staging"})
+ {:ok, conv: conv}
+ end
+
+ test "merges into what is already there", context do
+ conn =
+ context.conn
+ |> authed_with_key(context.raw_key)
+ |> patch_json("/api/conversations/#{context.conv.id}/labels", %{
+ "labels" => %{"drift" => "true"}
+ })
+
+ assert %{"data" => data} = json_response(conn, 200)
+ assert data["labels"] == %{"env" => "staging", "drift" => "true"}
+ end
+
+ test "a null value removes one key", context do
+ conn =
+ context.conn
+ |> authed_with_key(context.raw_key)
+ |> patch_json("/api/conversations/#{context.conv.id}/labels", %{
+ "labels" => %{"env" => nil, "run" => "17"}
+ })
+
+ assert %{"data" => %{"labels" => %{"run" => "17"}}} = json_response(conn, 200)
+ end
+
+ test "another tenant's conversation is a 404", context do
+ other = insert_conversation(user_id: insert_active_user().id)
+
+ conn =
+ context.conn
+ |> authed_with_key(context.raw_key)
+ |> patch_json("/api/conversations/#{other.id}/labels", %{"labels" => %{"env" => "prod"}})
+
+ assert json_response(conn, 404)
+ end
+
+ test "a body without a labels object is a 422", context do
+ conn =
+ context.conn
+ |> authed_with_key(context.raw_key)
+ |> patch_json("/api/conversations/#{context.conv.id}/labels", %{"labels" => "env=prod"})
+
+ assert %{"error" => _} = json_response(conn, 422)
+ end
+ end
+
+ describe "the limits over the wire" do
+ setup %{user: user} do
+ {:ok, conv: insert_conversation(user_id: user.id)}
+ end
+
+ defp label_error(context, labels) do
+ context.conn
+ |> authed_with_key(context.raw_key)
+ |> patch_json("/api/conversations/#{context.conv.id}/labels", %{"labels" => labels})
+ |> json_response(422)
+ |> get_in(["errors", "labels"])
+ |> List.first()
+ end
+
+ test "too many entries names the key over the limit", context do
+ labels = for n <- 1..33, into: %{}, do: {String.pad_leading("#{n}", 3, "0"), "x"}
+
+ message = label_error(context, labels)
+ assert message =~ "at most 32 labels"
+ assert message =~ ~s("033")
+ end
+
+ test "an over-long key names it", context do
+ message = label_error(context, %{String.duplicate("k", 65) => "v"})
+
+ assert message =~ "longer than 64 bytes"
+ assert message =~ String.duplicate("k", 64)
+ end
+
+ test "an over-long value names its key", context do
+ message = label_error(context, %{"note" => String.duplicate("v", 257)})
+
+ assert message =~ ~s("note")
+ assert message =~ "longer than 256 bytes"
+ end
+
+ # Postgres will not store a NUL inside a jsonb string. Unrejected, the
+ # write reaches Repo.update and comes back as a raised Postgrex.Error —
+ # a 500 on a request the caller should have been told was invalid.
+ test "a NUL byte in a value is a 422, not a 500", context do
+ message = label_error(context, %{"note" => "before\u0000after"})
+
+ assert message =~ ~s("note")
+ assert message =~ "NUL byte"
+ end
+
+ test "a NUL byte in a key is a 422, not a 500", context do
+ message = label_error(context, %{"ke\u0000y" => "v"})
+
+ assert message =~ "NUL byte"
+ end
+ end
+
+ describe "a channel resume merges labels (#1637)" do
+ setup %{user: user} do
+ agent = insert_agent(user_id: user.id)
+
+ bound =
+ insert_conversation(
+ user_id: user.id,
+ agent: agent,
+ status: "idle",
+ channel_id: "chat:42",
+ labels: %{"env" => "prod"},
+ sandbox: insert_sandbox(user_id: user.id, status: "ready")
+ )
+
+ {:ok, agent: agent, bound: bound}
+ end
+
+ test "into the conversation it hands back", context do
+ conn =
+ context.conn
+ |> authed_with_key(context.raw_key)
+ |> post_json("/api/conversations", %{
+ "agent_id" => context.agent.id,
+ "channel_id" => "chat:42",
+ "labels" => %{"run" => "17"}
+ })
+
+ # 200, not 201: the binding resumed rather than opening a conversation.
+ assert %{"data" => data, "meta" => %{"resumed" => true}} = json_response(conn, 200)
+ assert data["id"] == context.bound.id
+ assert data["labels"] == %{"env" => "prod", "run" => "17"}
+ end
+
+ test "a resume with no labels leaves the ones already there", context do
+ conn =
+ context.conn
+ |> authed_with_key(context.raw_key)
+ |> post_json("/api/conversations", %{
+ "agent_id" => context.agent.id,
+ "channel_id" => "chat:42"
+ })
+
+ assert %{"data" => %{"labels" => %{"env" => "prod"}}} = json_response(conn, 200)
+ end
+
+ test "a label the limits refuse is a 422", context do
+ conn =
+ context.conn
+ |> authed_with_key(context.raw_key)
+ |> post_json("/api/conversations", %{
+ "agent_id" => context.agent.id,
+ "channel_id" => "chat:42",
+ "labels" => %{"note" => String.duplicate("v", 300)}
+ })
+
+ assert [message] = json_response(conn, 422)["errors"]["labels"]
+ assert message =~ ~s("note")
+ end
+
+ test "a sandbox token may not relabel another conversation by resuming its channel",
+ context do
+ {_key, sprite_raw} = insert_sprite_api_key(context.user)
+
+ conn =
+ context.conn
+ |> authed_with_key(sprite_raw)
+ |> post_json("/api/conversations", %{
+ "agent_id" => context.agent.id,
+ "channel_id" => "chat:42",
+ "labels" => %{"run" => "17"}
+ })
+
+ assert %{"error" => "sprite_may_not_label_another_conversation"} = json_response(conn, 403)
+
+ assert Conversations._unsafe_get_conversation!(context.bound.id).labels == %{
+ "env" => "prod"
+ }
+ end
+
+ test "a sandbox token may relabel the conversation it was minted for", context do
+ {key, sprite_raw} = insert_sprite_api_key(context.user)
+
+ {:ok, _} =
+ Conversations.update_conversation(context.bound, %{callback_api_key_id: key.id})
+
+ conn =
+ context.conn
+ |> authed_with_key(sprite_raw)
+ |> post_json("/api/conversations", %{
+ "agent_id" => context.agent.id,
+ "channel_id" => "chat:42",
+ "labels" => %{"run" => "17"}
+ })
+
+ assert %{"data" => %{"labels" => labels}} = json_response(conn, 200)
+ assert labels == %{"env" => "prod", "run" => "17"}
+ end
+ end
+
+ describe "a sandbox callback token" do
+ setup %{user: user} do
+ {key, raw} = insert_sprite_api_key(user)
+ mine = insert_conversation(user_id: user.id, callback_api_key_id: key.id)
+ theirs = insert_conversation(user_id: user.id)
+
+ {:ok, sprite_key: raw, mine: mine, theirs: theirs}
+ end
+
+ test "labels the conversation it was minted for", context do
+ conn =
+ context.conn
+ |> authed_with_key(context.sprite_key)
+ |> patch_json("/api/conversations/#{context.mine.id}/labels", %{
+ "labels" => %{"drift" => "true"}
+ })
+
+ assert %{"data" => %{"labels" => %{"drift" => "true"}}} = json_response(conn, 200)
+ end
+
+ test "is refused on another conversation of the same account", context do
+ conn =
+ context.conn
+ |> authed_with_key(context.sprite_key)
+ |> patch_json("/api/conversations/#{context.theirs.id}/labels", %{
+ "labels" => %{"drift" => "true"}
+ })
+
+ assert %{"error" => "sprite_may_not_label_another_conversation"} = json_response(conn, 403)
+ assert Conversations._unsafe_get_conversation!(context.theirs.id).labels == %{}
+ end
+
+ test "records the write as the sprite, with keys and no values", context do
+ context.conn
+ |> authed_with_key(context.sprite_key)
+ |> patch_json("/api/conversations/#{context.mine.id}/labels", %{
+ "labels" => %{"drift" => "true"}
+ })
+
+ assert [event] =
+ context.user.id
+ |> Fountain.Audit.list_recent_for_user(50)
+ |> Enum.filter(&(&1.action == "conversation.labels_set"))
+
+ assert event.actor == "sprite"
+ assert event.metadata["keys"] == ["drift"]
+ refute inspect(event.metadata) =~ "true"
+ end
+ end
+end
diff --git a/apps/fountain/test/fountain_web/controllers/team_controller_test.exs b/apps/fountain/test/fountain_web/controllers/team_controller_test.exs
index 9e5a27337..7d667612a 100644
--- a/apps/fountain/test/fountain_web/controllers/team_controller_test.exs
+++ b/apps/fountain/test/fountain_web/controllers/team_controller_test.exs
@@ -380,6 +380,37 @@ defmodule FountainWeb.TeamControllerTest do
|> get("/api/team/#{loner.id}/conversations")
|> json_response(404)
end
+
+ test "the label filter is repeatable and AND-combined (#1637)", %{
+ conn: conn,
+ user: user,
+ raw_key: key
+ } do
+ ada = insert_agent(user_id: user.id, name: "Ada")
+ drifted = insert_teammate_conv(user, ada, labels: %{"env" => "prod", "drift" => "true"})
+ insert_teammate_conv(user, ada, labels: %{"env" => "prod"})
+ insert_teammate_conv(user, ada, labels: %{"env" => "staging"})
+
+ body =
+ conn
+ |> authed_with_key(key)
+ |> get("/api/team/#{ada.id}/conversations?label=env:prod&label=drift:true")
+ |> json_response(200)
+
+ assert Enum.map(body["data"], & &1["id"]) == [drifted.id]
+ assert hd(body["data"])["labels"] == %{"env" => "prod", "drift" => "true"}
+ end
+
+ test "a label filter with no colon is a 400", %{conn: conn, user: user, raw_key: key} do
+ ada = insert_agent(user_id: user.id, name: "Ada")
+ insert_teammate_conv(user, ada)
+
+ assert %{"error" => "invalid_label_filter"} =
+ conn
+ |> authed_with_key(key)
+ |> get("/api/team/#{ada.id}/conversations?label=prod")
+ |> json_response(400)
+ end
end
describe "POST /api/team/:agent_id/conversations" do
@@ -500,6 +531,155 @@ defmodule FountainWeb.TeamControllerTest do
assert_received {:sent, ^conv_id, "hello", [], "api"}
end
+ test "labels on the message land on the conversation it goes to (#1637)", %{
+ conn: conn,
+ user: user,
+ raw_key: key
+ } do
+ ada = insert_agent(user_id: user.id, name: "Ada")
+ conv = insert_teammate_conv(user, ada, labels: %{"env" => "prod"})
+ stub(ConversationServer, :send_prompt, fn _id, _text, _images, _opts -> :ok end)
+
+ assert %{"conversation_id" => _} =
+ conn
+ |> authed_with_key(key)
+ |> post_json("/api/team/#{ada.id}/messages", %{
+ prompt: "hello",
+ labels: %{"run" => "17"}
+ })
+ |> json_response(202)
+
+ assert Fountain.Conversations._unsafe_get_conversation!(conv.id).labels ==
+ %{"env" => "prod", "run" => "17"}
+ end
+
+ test "labels ride onto the fresh conversation when the thread is past resuming (#1637)", %{
+ conn: conn,
+ user: user,
+ raw_key: key
+ } do
+ ada = insert_agent(user_id: user.id, name: "Ada")
+ dead = insert_teammate_conv(user, ada, status: "terminated")
+ inert_start_child()
+
+ body =
+ conn
+ |> authed_with_key(key)
+ |> post_json("/api/team/#{ada.id}/messages", %{
+ prompt: "are you there?",
+ labels: %{"env" => "prod"}
+ })
+ |> json_response(202)
+
+ refute body["conversation_id"] == dead.id
+
+ assert Fountain.Conversations._unsafe_get_conversation!(body["conversation_id"]).labels ==
+ %{"env" => "prod"}
+ end
+
+ test "a sandbox token may not label the teammate's conversation (#1637)", %{
+ conn: conn,
+ user: user
+ } do
+ ada = insert_agent(user_id: user.id, name: "Ada")
+ conv = insert_teammate_conv(user, ada, labels: %{"env" => "prod"})
+ {_sprite, sprite_raw} = insert_sprite_api_key(user)
+ test_pid = self()
+
+ stub(ConversationServer, :send_prompt, fn _id, _text, _images, _opts ->
+ send(test_pid, :sent)
+ :ok
+ end)
+
+ assert %{"error" => "sprite_may_not_label_another_conversation"} =
+ conn
+ |> authed_with_key(sprite_raw)
+ |> post_json("/api/team/#{ada.id}/messages", %{
+ prompt: "hello",
+ labels: %{"run" => "17"}
+ })
+ |> json_response(403)
+
+ assert Fountain.Conversations._unsafe_get_conversation!(conv.id).labels == %{
+ "env" => "prod"
+ }
+
+ refute_received :sent
+ end
+
+ test "a sandbox token may label the conversation it was minted for (#1637)", %{
+ conn: conn,
+ user: user
+ } do
+ ada = insert_agent(user_id: user.id, name: "Ada")
+ conv = insert_teammate_conv(user, ada, labels: %{"env" => "prod"})
+ {sprite, sprite_raw} = insert_sprite_api_key(user)
+
+ {:ok, _} =
+ Fountain.Conversations.update_conversation(conv, %{callback_api_key_id: sprite.id})
+
+ stub(ConversationServer, :send_prompt, fn _id, _text, _images, _opts -> :ok end)
+
+ assert %{"status" => "queued"} =
+ conn
+ |> authed_with_key(sprite_raw)
+ |> post_json("/api/team/#{ada.id}/messages", %{
+ prompt: "hello",
+ labels: %{"run" => "17"}
+ })
+ |> json_response(202)
+
+ assert Fountain.Conversations._unsafe_get_conversation!(conv.id).labels ==
+ %{"env" => "prod", "run" => "17"}
+ end
+
+ test "a message with no labels leaves the ones already there (#1637)", %{
+ conn: conn,
+ user: user,
+ raw_key: key
+ } do
+ ada = insert_agent(user_id: user.id, name: "Ada")
+ conv = insert_teammate_conv(user, ada, labels: %{"env" => "prod"})
+ stub(ConversationServer, :send_prompt, fn _id, _text, _images, _opts -> :ok end)
+
+ conn
+ |> authed_with_key(key)
+ |> post_json("/api/team/#{ada.id}/messages", %{prompt: "hello"})
+ |> json_response(202)
+
+ assert Fountain.Conversations._unsafe_get_conversation!(conv.id).labels == %{
+ "env" => "prod"
+ }
+ end
+
+ test "a label the limits refuse leaves the message unsent, naming the key", %{
+ conn: conn,
+ user: user,
+ raw_key: key
+ } do
+ ada = insert_agent(user_id: user.id, name: "Ada")
+ insert_teammate_conv(user, ada)
+ test_pid = self()
+
+ stub(ConversationServer, :send_prompt, fn _id, _text, _images, _opts ->
+ send(test_pid, :sent)
+ :ok
+ end)
+
+ body =
+ conn
+ |> authed_with_key(key)
+ |> post_json("/api/team/#{ada.id}/messages", %{
+ prompt: "hello",
+ labels: %{"note" => String.duplicate("v", 300)}
+ })
+ |> json_response(422)
+
+ assert [message] = body["errors"]["labels"]
+ assert message =~ ~s("note")
+ refute_received :sent
+ end
+
test "400 conversation_busy while the teammate is busy, 404 when not on the team", %{
conn: conn,
user: user,
diff --git a/apps/fountain/test/fountain_web/live/dashboard_live_test.exs b/apps/fountain/test/fountain_web/live/dashboard_live_test.exs
index 58e2c4140..0a74e1126 100644
--- a/apps/fountain/test/fountain_web/live/dashboard_live_test.exs
+++ b/apps/fountain/test/fountain_web/live/dashboard_live_test.exs
@@ -300,4 +300,77 @@ defmodule FountainWeb.DashboardLiveTest do
# And it does not ask for a conversation it has nowhere to start.
refute html =~ "Start one"
end
+
+ describe "conversation labels (#1637)" do
+ test "renders them as chips beside each conversation", %{conn: conn, user: user} do
+ insert_conversation(user_id: user.id, labels: %{"env" => "prod", "drift" => "true"})
+
+ {:ok, _lv, html} = live(conn, ~p"/dashboard")
+
+ assert html =~ "env=prod"
+ assert html =~ "drift=true"
+ end
+
+ test "a conversation with no labels renders no chips", %{conn: conn, user: user} do
+ insert_conversation(user_id: user.id)
+
+ {:ok, lv, _html} = live(conn, ~p"/dashboard")
+
+ refute has_element?(lv, "[data-label-chip]")
+ end
+
+ test "the URL filter narrows the list, repeatable and AND-combined", %{
+ conn: conn,
+ user: user
+ } do
+ drifted =
+ insert_conversation(user_id: user.id, labels: %{"env" => "prod", "drift" => "true"})
+
+ clean = insert_conversation(user_id: user.id, labels: %{"env" => "prod"})
+ staging = insert_conversation(user_id: user.id, labels: %{"env" => "staging"})
+
+ {:ok, _lv, one} = live(conn, "/dashboard?label=env:prod")
+ assert one =~ drifted.id
+ assert one =~ clean.id
+ refute one =~ staging.id
+
+ {:ok, _lv, both} = live(conn, "/dashboard?label=env:prod&label=drift:true")
+ assert both =~ drifted.id
+ refute both =~ clean.id
+ end
+
+ test "a filter nothing matches says so rather than showing everything", %{
+ conn: conn,
+ user: user
+ } do
+ insert_conversation(user_id: user.id, labels: %{"env" => "prod"})
+
+ {:ok, _lv, html} = live(conn, "/dashboard?label=env:qa")
+
+ assert html =~ "No conversation carries every one of those labels"
+ end
+
+ test "a chip links to the same list filtered by it", %{conn: conn, user: user} do
+ insert_conversation(user_id: user.id, labels: %{"env" => "prod"})
+
+ {:ok, lv, _html} = live(conn, ~p"/dashboard")
+
+ assert lv
+ |> element("[data-label-chip='env']")
+ |> render_click() =~ "clear"
+
+ assert_patched(lv, "/dashboard?label=env%3Aprod")
+ end
+
+ test "an unparseable filter value filters nothing rather than erroring", %{
+ conn: conn,
+ user: user
+ } do
+ conv = insert_conversation(user_id: user.id, labels: %{"env" => "prod"})
+
+ {:ok, _lv, html} = live(conn, "/dashboard?label=prod")
+
+ assert html =~ conv.id
+ end
+ end
end
diff --git a/docs/api.md b/docs/api.md
index bf88e960e..beed92cd4 100644
--- a/docs/api.md
+++ b/docs/api.md
@@ -229,6 +229,93 @@ event cursor so a reconnect can resume after the last event processed.
Request structured blocks to render runtime output; clients should not
parse each runtime's native dialect.
+### Labels
+
+A label is a `key=value` pair of strings on a conversation. A program stamps
+its own runs with the facts it knew when the turn ended. Examples are
+`env=prod`, `drift=true` and `gated=apply`. Labels are not searched. Use them
+to slice a list.
+
+Set them at creation, and read them back on every conversation object.
+
+```bash
+curl --fail-with-body \
+ -H "Authorization: Bearer $FOUNTAIN_API_KEY" \
+ -H "Content-Type: application/json" \
+ -d '{"agent_id":"YOUR_AGENT_ID","labels":{"env":"prod"}}' \
+ "$FOUNTAIN_URL/api/conversations"
+```
+
+Filter a list with a repeatable `label` parameter. Fountain combines the
+values with AND. The example keeps the conversations that carry both pairs.
+
+```bash
+curl --fail-with-body \
+ -H "Authorization: Bearer $FOUNTAIN_API_KEY" \
+ "$FOUNTAIN_URL/api/conversations?label=env:prod&label=drift:true"
+```
+
+Each value splits on its first colon. The key is the part before it, and the
+value is all of the rest. `label=path:apps/fountain:lib` therefore filters the
+key `path` for the value `apps/fountain:lib`. A value with no colon, or with
+an empty key, returns 400 `invalid_label_filter`. The same parameter works on
+`GET /api/team/{agent_id}/conversations`.
+
+The parameter is an array in the OpenAPI document, with `style: form` and
+`explode: true`. A client that builds arrays as `label[]=env:prod` is
+accepted too.
+
+`PATCH /api/conversations/{id}/labels` merges labels into a conversation. A
+key the body does not name stays as it is. A key with a `null` value is
+removed.
+
+```bash
+curl --fail-with-body -X PATCH \
+ -H "Authorization: Bearer $FOUNTAIN_API_KEY" \
+ -H "Content-Type: application/json" \
+ -d '{"labels":{"drift":"true","env":null}}' \
+ "$FOUNTAIN_URL/api/conversations/$CONVERSATION_ID/labels"
+```
+
+A conversation holds at most 32 labels. A key is at most 64 bytes and a value
+is at most 256 bytes. Neither can contain a NUL byte. A write that breaks one
+of these limits returns 422 and names the offending key under
+`errors.labels`. The count applies to the merged result, so a merge can fail
+against labels that are already there. The key named is one you sent, and
+never one that was already on the conversation.
+
+`POST /api/team/{agent_id}/messages` also takes `labels`. Fountain merges them
+into the conversation that receives the message, before it queues the turn.
+`POST /api/conversations` with a `channel_id` that resumes an existing
+conversation merges them into that conversation.
+
+The account's own API key can label any of its conversations. A sandbox
+callback token can label only the conversation it was minted for. Another
+conversation returns 403 `sprite_may_not_label_another_conversation`. This
+applies to all three doors that write labels, which are the labels route, a
+team message, and a `channel_id` resume.
+
+`conversation.*` webhook payloads carry `labels` under `data`. See
+[Webhooks](reference/webhooks.md).
+
+### An agent that labels its own run
+
+An agent inside a turn does not need the route above. It sends an ACP
+extension notification on the session it already holds. ACP reserves a leading
+underscore for extensions.
+
+```json
+{"jsonrpc":"2.0","method":"session/update","params":{
+ "sessionId":"sess_1",
+ "update":{"sessionUpdate":"_fountain/labels",
+ "labels":{"drift":"true","env":"prod"}}}}
+```
+
+Fountain merges the map with the same rules as the route. A `null` value
+removes a key. The notification never reaches the transcript, and it opens no
+turn of its own. A stamp that breaks a limit is logged and dropped, and the
+turn continues. Nothing in a stamp can end a run.
+
### Workers without Fountain API access
Set `sandbox_api_access` to `none` when the host must retain Fountain API
diff --git a/docs/concepts/conversation.md b/docs/concepts/conversation.md
index a3e3e2c36..d2b87404e 100644
--- a/docs/concepts/conversation.md
+++ b/docs/concepts/conversation.md
@@ -118,6 +118,26 @@ Use a schedule when the run must happen without you. Read the
- [Architecture](../architecture.md), for what runs where.
- [The guided tour](../tour.md), which runs one from start to finish.
+## Labels
+
+A conversation carries free-form `key=value` strings. They record what a run
+found, and not what it said. `env=prod` and `drift=true` are the shape of
+them.
+
+Set them at launch, merge them later with
+`PATCH /api/conversations/:id/labels`, or let the agent stamp its own run over
+the ACP extension notification. Filter a list with a repeatable
+`?label=env:prod` parameter, which Fountain combines with AND.
+
+A conversation holds at most 32 of them. A key is at most 64 bytes and a value
+is at most 256 bytes. Read the
+[Labels section](../api.md#labels) of the API reference for the wire format,
+the merge rules and the size limits.
+
+Labels are not part of full-text search. Search covers titles, prompts and
+replies, which is what a person scans. A label is a fact a program already
+knew.
+
## Discover conversations on a sandbox
Use `GET /api/conversations?sandbox_id= |