From d869fadf42556d08f569b2ac73ebcef203b351c0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pedro=20Pi=C3=B1era=20Buend=C3=ADa?= <pedro@pepicrft.me>
Date: Wed, 23 Sep 2026 23:34:29 +0200
Subject: [PATCH 01/17] Add Pulso.Storage.S3 adapter and swap MinIO for RustFS

Step 2 of the storage plan: append/query now round-trip logs through an
S3-compatible endpoint via the existing Rustler object_store NIF. Objects
are one NDJSON per append batch, keyed by tenant-scoped prefix so isolation
holds by construction. Memory adapter stays wired as the test-only default.

docker-compose.yml swaps MinIO for RustFS since MinIO's community edition
is no longer maintained and RustFS is a drop-in S3-compatible replacement.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 config/runtime.exs               |  31 ++++++
 docker-compose.yml               |  67 ++++++++-----
 lib/pulso/application.ex         |  30 ++++--
 lib/pulso/storage.ex             |   8 +-
 lib/pulso/storage/memory.ex      |   8 +-
 lib/pulso/storage/s3.ex          | 160 +++++++++++++++++++++++++++++++
 test/pulso/object_store_test.exs |  18 ++--
 test/pulso/storage/s3_test.exs   | 128 +++++++++++++++++++++++++
 8 files changed, 401 insertions(+), 49 deletions(-)
 create mode 100644 lib/pulso/storage/s3.ex
 create mode 100644 test/pulso/storage/s3_test.exs

diff --git a/config/runtime.exs b/config/runtime.exs
index 5da5ce0..36a5d6b 100644
--- a/config/runtime.exs
+++ b/config/runtime.exs
@@ -22,6 +22,37 @@ end
 
 config :pulso, PulsoWeb.Endpoint, http: [port: String.to_integer(System.get_env("PORT", "4000"))]
 
+# Log storage adapter. Tests keep the in-memory adapter (see config/test.exs);
+# dev and prod use the S3 adapter against any S3-compatible endpoint. RustFS
+# runs locally via docker-compose.yml — the dev defaults below match its
+# out-of-the-box credentials. Prod requires the env vars to be set explicitly.
+case config_env() do
+  :dev ->
+    config :pulso, Pulso.Storage, adapter: Pulso.Storage.S3
+
+    config :pulso, Pulso.Storage.S3,
+      bucket: System.get_env("PULSO_S3_BUCKET", "pulso"),
+      endpoint: System.get_env("PULSO_S3_ENDPOINT", "http://localhost:9000"),
+      region: System.get_env("PULSO_S3_REGION", "us-east-1"),
+      access_key_id: System.get_env("PULSO_S3_ACCESS_KEY_ID", "rustfsadmin"),
+      secret_access_key: System.get_env("PULSO_S3_SECRET_ACCESS_KEY", "rustfsadmin"),
+      allow_http: System.get_env("PULSO_S3_ALLOW_HTTP", "true") in ["1", "true", "yes"]
+
+  :prod ->
+    config :pulso, Pulso.Storage, adapter: Pulso.Storage.S3
+
+    config :pulso, Pulso.Storage.S3,
+      bucket: System.fetch_env!("PULSO_S3_BUCKET"),
+      endpoint: System.get_env("PULSO_S3_ENDPOINT"),
+      region: System.fetch_env!("PULSO_S3_REGION"),
+      access_key_id: System.fetch_env!("PULSO_S3_ACCESS_KEY_ID"),
+      secret_access_key: System.fetch_env!("PULSO_S3_SECRET_ACCESS_KEY"),
+      allow_http: System.get_env("PULSO_S3_ALLOW_HTTP", "false") in ["1", "true", "yes"]
+
+  :test ->
+    :noop
+end
+
 if config_env() == :prod do
   # The secret key base is used to sign/encrypt cookies and other secrets.
   # A default value is used in config/dev.exs and config/test.exs but you
diff --git a/docker-compose.yml b/docker-compose.yml
index 650dd88..cc96fb4 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -1,39 +1,54 @@
 # Local development stack for Pulso.
 #
-# `docker compose up -d` gives you MinIO on http://localhost:9000 with a
-# preseeded bucket named `pulso`. The console is on http://localhost:9001
-# (user/password: minioadmin/minioadmin).
+# `docker compose up -d` gives you a RustFS (S3-compatible) server on
+# http://localhost:9000 with a preseeded bucket named `pulso`. The console is
+# on http://localhost:9001 (user/password: rustfsadmin/rustfsadmin).
+#
+# RustFS is used instead of MinIO because MinIO's community edition is no
+# longer actively maintained. Pulso only talks to the S3 API, so any
+# S3-compatible backend works; RustFS is a drop-in that stays maintained.
 
 services:
-  minio:
-    image: quay.io/minio/minio:latest
-    command: server /data --console-address ":9001"
+  rustfs:
+    image: rustfs/rustfs:latest
     environment:
-      MINIO_ROOT_USER: minioadmin
-      MINIO_ROOT_PASSWORD: minioadmin
+      RUSTFS_ADDRESS: 0.0.0.0:9000
+      RUSTFS_CONSOLE_ADDRESS: 0.0.0.0:9001
+      RUSTFS_CONSOLE_ENABLE: "true"
+      RUSTFS_ACCESS_KEY: rustfsadmin
+      RUSTFS_SECRET_KEY: rustfsadmin
     ports:
       - "9000:9000"
       - "9001:9001"
     volumes:
-      - minio-data:/data
-    healthcheck:
-      test: ["CMD", "curl", "-f", "http://localhost:9000/minio/health/live"]
-      interval: 5s
-      timeout: 3s
-      retries: 10
+      - rustfs-data:/data
 
-  minio-init:
-    image: quay.io/minio/mc:latest
+  rustfs-init:
+    image: amazon/aws-cli:2
     depends_on:
-      minio:
-        condition: service_healthy
-    entrypoint: >
-      /bin/sh -c "
-        mc alias set local http://minio:9000 minioadmin minioadmin;
-        mc mb --ignore-existing local/pulso;
-        mc anonymous set none local/pulso;
-        echo 'pulso bucket ready';
-      "
+      - rustfs
+    environment:
+      AWS_ACCESS_KEY_ID: rustfsadmin
+      AWS_SECRET_ACCESS_KEY: rustfsadmin
+      AWS_DEFAULT_REGION: us-east-1
+    entrypoint:
+      - /bin/sh
+      - -c
+      - |
+        for i in $$(seq 1 60); do
+          if aws --endpoint-url http://rustfs:9000 s3 mb s3://pulso 2>/dev/null; then
+            echo "pulso bucket created"
+            exit 0
+          fi
+          if aws --endpoint-url http://rustfs:9000 s3api head-bucket --bucket pulso 2>/dev/null; then
+            echo "pulso bucket already exists"
+            exit 0
+          fi
+          echo "waiting for rustfs..."
+          sleep 2
+        done
+        echo "gave up waiting for rustfs" >&2
+        exit 1
 
 volumes:
-  minio-data:
+  rustfs-data:
diff --git a/lib/pulso/application.ex b/lib/pulso/application.ex
index 3ac731a..2c94cb0 100644
--- a/lib/pulso/application.ex
+++ b/lib/pulso/application.ex
@@ -9,13 +9,12 @@ defmodule Pulso.Application do
 
   @impl true
   def start(_type, _args) do
-    children = [
-      PulsoWeb.Telemetry,
-      {DNSCluster, query: Application.get_env(:pulso, :dns_cluster_query) || :ignore},
-      {Phoenix.PubSub, name: Pulso.PubSub},
-      Memory,
-      PulsoWeb.Endpoint
-    ]
+    children =
+      [
+        PulsoWeb.Telemetry,
+        {DNSCluster, query: Application.get_env(:pulso, :dns_cluster_query) || :ignore},
+        {Phoenix.PubSub, name: Pulso.PubSub}
+      ] ++ storage_children() ++ [PulsoWeb.Endpoint]
 
     # See https://elixir.hexdocs.pm/Supervisor.html
     # for other strategies and supported options
@@ -30,4 +29,21 @@ defmodule Pulso.Application do
     PulsoWeb.Endpoint.config_change(changed, removed)
     :ok
   end
+
+  # `Memory` only runs when it is the configured adapter — in `mix test` no
+  # adapter is set, so `Pulso.Storage.adapter/0` falls back to it. In dev and
+  # prod the S3 adapter is configured and Memory would just be dead weight.
+  defp storage_children do
+    adapter =
+      case Application.get_env(:pulso, Pulso.Storage) do
+        nil -> nil
+        env -> Keyword.get(env, :adapter)
+      end
+
+    case adapter do
+      nil -> [Memory]
+      Pulso.Storage.Memory -> [Memory]
+      _ -> []
+    end
+  end
 end
diff --git a/lib/pulso/storage.ex b/lib/pulso/storage.ex
index 765a420..af6134e 100644
--- a/lib/pulso/storage.ex
+++ b/lib/pulso/storage.ex
@@ -3,9 +3,11 @@ defmodule Pulso.Storage do
   Behaviour for log storage backends and the runtime dispatcher.
 
   The active adapter is read from application configuration at call time so it
-  can be swapped in tests without recompiling. In step 1 the default adapter is
-  `Pulso.Storage.Memory`; step 2 will introduce an S3-backed adapter and demote
-  Memory to a test-only backend.
+  can be swapped in tests without recompiling. In dev and prod the default
+  adapter is `Pulso.Storage.S3` (step 2). Tests fall back to
+  `Pulso.Storage.Memory` because no adapter is configured. Step 3 will replace
+  the flat NDJSON layout of the S3 adapter with columnar segments and a
+  manifest that supports conditional writes.
   """
 
   alias Pulso.Record.Log
diff --git a/lib/pulso/storage/memory.ex b/lib/pulso/storage/memory.ex
index 927b3d8..66e9a55 100644
--- a/lib/pulso/storage/memory.ex
+++ b/lib/pulso/storage/memory.ex
@@ -2,10 +2,10 @@ defmodule Pulso.Storage.Memory do
   @moduledoc """
   In-memory log storage backed by a public ETS table.
 
-  Step 1 default adapter. Meant to prove the ingest → storage → query spine
-  end to end without dragging in Rust, Parquet, or S3. Step 2 will replace it
-  with a real S3-backed adapter; this module will move to `test/support/` and
-  keep serving the test suite.
+  Test-only adapter as of step 2. Meant to prove the ingest → storage → query
+  spine end to end without dragging in Rust, Parquet, or S3. Dev and prod use
+  `Pulso.Storage.S3`; this module stays wired as the default in `mix test`
+  because `config/test.exs` sets no adapter override.
 
   Records for each tenant are kept in a private list-per-tenant, appended to
   as batches arrive and scanned linearly on query. That is deliberately naive:
diff --git a/lib/pulso/storage/s3.ex b/lib/pulso/storage/s3.ex
new file mode 100644
index 0000000..7a619be
--- /dev/null
+++ b/lib/pulso/storage/s3.ex
@@ -0,0 +1,160 @@
+defmodule Pulso.Storage.S3 do
+  @moduledoc """
+  S3-backed log storage. Step 2 adapter.
+
+  Each `append/2` writes one NDJSON object under a tenant-scoped prefix
+  (`tenants/<tenant>/logs/<sort_key>.ndjson`). Tenant isolation is enforced by
+  key construction: tenant names are validated against a conservative charset
+  so a batch cannot land outside its own prefix. `query/2` lists the tenant's
+  prefix, downloads every object, decodes NDJSON, filters and sorts in Elixir.
+
+  This is deliberately naive. Step 3 replaces the flat NDJSON layout with
+  columnar segments and a manifest that supports conditional writes; the
+  point of step 2 is only to prove logs round-trip through real object
+  storage against the `Pulso.ObjectStore` NIF.
+  """
+
+  @behaviour Pulso.Storage
+
+  alias Pulso.ObjectStore
+  alias Pulso.Record.Log
+
+  @tenant_regex ~r/\A[A-Za-z0-9_.\-]{1,128}\z/
+  # 20 decimal digits fits a u64 nanosecond timestamp (max ~1.84e19). Zero-padding
+  # this way makes the S3 list order roughly chronological, which lets query
+  # short-circuit once the sort/limit is satisfied in a later step.
+  @sort_key_width 20
+
+  @impl Pulso.Storage
+  def append(_tenant, []), do: :ok
+
+  def append(tenant, records) when is_binary(tenant) and is_list(records) do
+    with :ok <- validate_tenant(tenant),
+         normalized <- normalize(records),
+         payload when is_binary(payload) <- encode(normalized),
+         key <- object_key(tenant, batch_sort_ns(normalized)) do
+      ObjectStore.put(config!(), key, payload)
+    end
+  end
+
+  @impl Pulso.Storage
+  def query(tenant, opts) when is_binary(tenant) and is_list(opts) do
+    with :ok <- validate_tenant(tenant),
+         config <- config!(),
+         {:ok, keys} <- ObjectStore.list(config, prefix(tenant)),
+         {:ok, records} <- fetch_records(config, keys) do
+      filtered =
+        records
+        |> filter_by_time(Keyword.get(opts, :start_ts), Keyword.get(opts, :end_ts))
+        |> filter_by_service(Keyword.get(opts, :service))
+        |> Enum.sort_by(& &1.timestamp_ns, :desc)
+        |> take_limit(Keyword.get(opts, :limit))
+
+      {:ok, filtered}
+    end
+  end
+
+  # -- helpers -----------------------------------------------------------------
+
+  defp validate_tenant(tenant) do
+    if Regex.match?(@tenant_regex, tenant) do
+      :ok
+    else
+      {:error, {:invalid_tenant, tenant}}
+    end
+  end
+
+  defp normalize(records) do
+    now = System.system_time(:nanosecond)
+
+    for %Log{} = record <- records do
+      %{record | observed_timestamp_ns: record.observed_timestamp_ns || now}
+    end
+  end
+
+  defp batch_sort_ns(records) do
+    # Pick the smallest observed_timestamp_ns so the first key in a listing is
+    # the earliest batch. Every record is normalized, so this is never nil.
+    records
+    |> Enum.map(& &1.observed_timestamp_ns)
+    |> Enum.min()
+  end
+
+  defp encode(records) do
+    records
+    |> Enum.map(&(Jason.encode!(Map.from_struct(&1)) <> "\n"))
+    |> IO.iodata_to_binary()
+  end
+
+  defp fetch_records(config, keys) do
+    Enum.reduce_while(keys, {:ok, []}, fn key, {:ok, acc} ->
+      case ObjectStore.get(config, key) do
+        {:ok, blob} -> {:cont, {:ok, acc ++ decode(blob)}}
+        {:error, _} = err -> {:halt, err}
+      end
+    end)
+  end
+
+  defp decode(blob) do
+    blob
+    |> String.split("\n", trim: true)
+    |> Enum.map(&decode_line/1)
+  end
+
+  defp decode_line(line) do
+    map = Jason.decode!(line)
+
+    %Log{
+      timestamp_ns: Map.fetch!(map, "timestamp_ns"),
+      observed_timestamp_ns: map["observed_timestamp_ns"],
+      severity_number: map["severity_number"],
+      severity_text: map["severity_text"],
+      service: map["service"],
+      body: map["body"],
+      trace_id: map["trace_id"],
+      span_id: map["span_id"],
+      attributes: map["attributes"] || %{},
+      resource: map["resource"] || %{}
+    }
+  end
+
+  defp prefix(tenant), do: "tenants/#{tenant}/logs/"
+
+  defp object_key(tenant, sort_ns) do
+    "#{prefix(tenant)}#{zero_pad(sort_ns)}-#{rand_suffix()}.ndjson"
+  end
+
+  defp zero_pad(ns) when is_integer(ns) and ns >= 0 do
+    ns
+    |> Integer.to_string()
+    |> String.pad_leading(@sort_key_width, "0")
+  end
+
+  defp rand_suffix do
+    :crypto.strong_rand_bytes(8) |> Base.encode16(case: :lower)
+  end
+
+  defp filter_by_time(records, nil, nil), do: records
+
+  defp filter_by_time(records, start_ts, end_ts) do
+    Enum.filter(records, fn %Log{timestamp_ns: ts} ->
+      (start_ts == nil or ts >= start_ts) and (end_ts == nil or ts <= end_ts)
+    end)
+  end
+
+  defp filter_by_service(records, nil), do: records
+  defp filter_by_service(records, service), do: Enum.filter(records, &(&1.service == service))
+
+  defp take_limit(records, nil), do: records
+  defp take_limit(records, limit) when is_integer(limit) and limit > 0, do: Enum.take(records, limit)
+
+  defp config! do
+    case Application.get_env(:pulso, __MODULE__) do
+      nil ->
+        raise "Pulso.Storage.S3 is not configured. Set `config :pulso, Pulso.Storage.S3, bucket: ..., endpoint: ..., region: ..., access_key_id: ..., secret_access_key: ..., allow_http: ...`"
+
+      config ->
+        Map.new(config)
+    end
+  end
+end
diff --git a/test/pulso/object_store_test.exs b/test/pulso/object_store_test.exs
index 77076a6..231f99e 100644
--- a/test/pulso/object_store_test.exs
+++ b/test/pulso/object_store_test.exs
@@ -1,8 +1,8 @@
 defmodule Pulso.ObjectStoreTest do
-  # This module talks to a live MinIO endpoint via the Rust NIF.
-  # It only runs when the caller opts in with PULSO_INTEGRATION=1 (see
-  # test/test_helper.exs), so plain `mix test` on a machine without
-  # docker-compose still passes.
+  # This module talks to a live S3-compatible endpoint (RustFS by default via
+  # docker-compose.yml) through the Rust NIF. It only runs when the caller
+  # opts in with PULSO_INTEGRATION=1 (see test/test_helper.exs), so plain
+  # `mix test` on a machine without docker-compose still passes.
 
   use ExUnit.Case, async: false
 
@@ -12,11 +12,11 @@ defmodule Pulso.ObjectStoreTest do
 
   setup do
     config = %{
-      bucket: System.get_env("PULSO_MINIO_BUCKET", "pulso"),
-      endpoint: System.get_env("PULSO_MINIO_ENDPOINT", "http://localhost:9000"),
-      region: System.get_env("PULSO_MINIO_REGION", "us-east-1"),
-      access_key_id: System.get_env("PULSO_MINIO_ACCESS_KEY_ID", "minioadmin"),
-      secret_access_key: System.get_env("PULSO_MINIO_SECRET_ACCESS_KEY", "minioadmin"),
+      bucket: System.get_env("PULSO_S3_BUCKET", "pulso"),
+      endpoint: System.get_env("PULSO_S3_ENDPOINT", "http://localhost:9000"),
+      region: System.get_env("PULSO_S3_REGION", "us-east-1"),
+      access_key_id: System.get_env("PULSO_S3_ACCESS_KEY_ID", "rustfsadmin"),
+      secret_access_key: System.get_env("PULSO_S3_SECRET_ACCESS_KEY", "rustfsadmin"),
       allow_http: true
     }
 
diff --git a/test/pulso/storage/s3_test.exs b/test/pulso/storage/s3_test.exs
new file mode 100644
index 0000000..0406fc4
--- /dev/null
+++ b/test/pulso/storage/s3_test.exs
@@ -0,0 +1,128 @@
+defmodule Pulso.Storage.S3Test do
+  # Round-trips logs through the real S3-compatible endpoint (RustFS via
+  # docker-compose). Only runs with PULSO_INTEGRATION=1; plain `mix test`
+  # skips it. See test/test_helper.exs.
+
+  use ExUnit.Case, async: false
+
+  alias Pulso.ObjectStore
+  alias Pulso.Record.Log
+  alias Pulso.Storage.S3
+
+  @moduletag :integration
+
+  setup do
+    config = %{
+      bucket: System.get_env("PULSO_S3_BUCKET", "pulso"),
+      endpoint: System.get_env("PULSO_S3_ENDPOINT", "http://localhost:9000"),
+      region: System.get_env("PULSO_S3_REGION", "us-east-1"),
+      access_key_id: System.get_env("PULSO_S3_ACCESS_KEY_ID", "rustfsadmin"),
+      secret_access_key: System.get_env("PULSO_S3_SECRET_ACCESS_KEY", "rustfsadmin"),
+      allow_http: true
+    }
+
+    Application.put_env(:pulso, Pulso.Storage.S3, config)
+
+    tenant = "test-#{System.unique_integer([:positive])}"
+
+    on_exit(fn ->
+      # The adapter writes objects under `tenants/<tenant>/logs/`; clean up so
+      # a re-run starts empty.
+      case ObjectStore.list(config, "tenants/#{tenant}/logs/") do
+        {:ok, keys} -> Enum.each(keys, &ObjectStore.delete(config, &1))
+        _ -> :ok
+      end
+    end)
+
+    {:ok, config: config, tenant: tenant}
+  end
+
+  defp record(ts, opts \\ []) do
+    %Log{
+      timestamp_ns: ts,
+      severity_text: Keyword.get(opts, :severity_text),
+      service: Keyword.get(opts, :service),
+      body: Keyword.get(opts, :body),
+      attributes: Keyword.get(opts, :attributes, %{}),
+      resource: Keyword.get(opts, :resource, %{})
+    }
+  end
+
+  test "append then query round-trips log records", %{tenant: tenant} do
+    assert :ok =
+             S3.append(tenant, [
+               record(10, service: "api", body: "hello"),
+               record(20, service: "web", body: "world")
+             ])
+
+    assert {:ok, records} = S3.query(tenant, [])
+    assert Enum.map(records, & &1.timestamp_ns) == [20, 10]
+    assert Enum.map(records, & &1.service) == ["web", "api"]
+    assert Enum.map(records, & &1.body) == ["world", "hello"]
+  end
+
+  test "records for one tenant are invisible to another", %{tenant: tenant, config: config} do
+    other = "test-other-#{System.unique_integer([:positive])}"
+
+    on_exit(fn ->
+      case ObjectStore.list(config, "tenants/#{other}/logs/") do
+        {:ok, keys} -> Enum.each(keys, &ObjectStore.delete(config, &1))
+        _ -> :ok
+      end
+    end)
+
+    assert :ok = S3.append(tenant, [record(1)])
+    assert :ok = S3.append(other, [record(2)])
+
+    assert {:ok, [%Log{timestamp_ns: 1}]} = S3.query(tenant, [])
+    assert {:ok, [%Log{timestamp_ns: 2}]} = S3.query(other, [])
+  end
+
+  test "filters by time range and service", %{tenant: tenant} do
+    assert :ok =
+             S3.append(tenant, [
+               record(10, service: "api"),
+               record(20, service: "web"),
+               record(30, service: "api"),
+               record(40, service: "api")
+             ])
+
+    assert {:ok, records} = S3.query(tenant, start_ts: 15, end_ts: 35, service: "api")
+    assert Enum.map(records, & &1.timestamp_ns) == [30]
+  end
+
+  test "applies limit", %{tenant: tenant} do
+    assert :ok = S3.append(tenant, [record(1), record(2), record(3), record(4)])
+    assert {:ok, records} = S3.query(tenant, limit: 2)
+    assert length(records) == 2
+    assert Enum.map(records, & &1.timestamp_ns) == [4, 3]
+  end
+
+  test "populates observed_timestamp_ns when the record does not carry one", %{tenant: tenant} do
+    before_append = System.system_time(:nanosecond)
+    assert :ok = S3.append(tenant, [record(1)])
+    after_append = System.system_time(:nanosecond)
+
+    assert {:ok, [%Log{observed_timestamp_ns: observed}]} = S3.query(tenant, [])
+    assert observed >= before_append and observed <= after_append
+  end
+
+  test "preserves NDJSON-hostile bodies through the round trip", %{tenant: tenant} do
+    tricky = "line1\nline2\t\"quoted\"\r\nline3"
+    assert :ok = S3.append(tenant, [record(1, body: tricky)])
+    assert {:ok, [%Log{body: ^tricky}]} = S3.query(tenant, [])
+  end
+
+  test "append with an empty batch is a no-op", %{tenant: tenant, config: config} do
+    assert :ok = S3.append(tenant, [])
+    assert {:ok, keys} = ObjectStore.list(config, "tenants/#{tenant}/logs/")
+    assert keys == []
+  end
+
+  test "rejects tenant names that could escape the prefix" do
+    for bad <- ["../evil", "foo/bar", "foo bar", "", String.duplicate("a", 200)] do
+      assert {:error, {:invalid_tenant, ^bad}} = S3.append(bad, [record(1)])
+      assert {:error, {:invalid_tenant, ^bad}} = S3.query(bad, [])
+    end
+  end
+end

From 8ea1c7a3398114973bd9e8916ae190e80a019af7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pedro=20Pi=C3=B1era=20Buend=C3=ADa?= <pedro@pepicrft.me>
Date: Wed, 23 Sep 2026 23:39:46 +0200
Subject: [PATCH 02/17] Address Codex adversarial review of Pulso.Storage.S3

Fixes the concrete correctness/config/deploy issues Codex flagged; documents
the concerns that are deliberately deferred to step 3 (idempotency,
unbounded query work, cross-tenant auth).

- Validate tenant name even on an empty batch, so an adversarial name is
  rejected on the first attempt rather than only once a record survives OTLP
  decoding.
- Return {:error, {:encode_failed, _}} on Jason encode failure (e.g. a body
  with non-UTF-8 bytes) instead of raising and killing the ingest process.
- Rewrite fetch_records to prepend batches and flatten once, avoiding
  O(n^2) list concatenation on tenants with many objects.
- Reject empty PULSO_S3_* env values in prod (fetch_env! only guarded
  against missing keys, not empty strings).
- docker-compose: add the mandatory RUSTFS_VOLUMES env var (RustFS refused
  to start without it) and bind the published ports to 127.0.0.1 so the
  well-known dev credentials cannot be reached from another host on the LAN.
- Add non-integration unit tests for tenant validation (including empty
  batch) and the encode-failure path, so `mix test` protects against
  regressions without needing RustFS running.
- Rewrite the module docstring to name the known limits of the step-2
  layout (not idempotent, unbounded query, no auth here, no columnar
  layout) so a future reader knows what step 3 has to fix.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 config/runtime.exs                  | 31 ++++++++++++----
 docker-compose.yml                  |  9 ++++-
 lib/pulso/application.ex            |  2 +-
 lib/pulso/storage/s3.ex             | 57 ++++++++++++++++++++++-------
 test/pulso/storage/s3_test.exs      |  2 +-
 test/pulso/storage/s3_unit_test.exs | 46 +++++++++++++++++++++++
 6 files changed, 121 insertions(+), 26 deletions(-)
 create mode 100644 test/pulso/storage/s3_unit_test.exs

diff --git a/config/runtime.exs b/config/runtime.exs
index 36a5d6b..aca7253 100644
--- a/config/runtime.exs
+++ b/config/runtime.exs
@@ -16,6 +16,8 @@ import Config
 #
 # Alternatively, you can use `mix phx.gen.release` to generate a `bin/server`
 # script that automatically sets the env var above.
+alias Pulso.Storage.S3
+
 if System.get_env("PHX_SERVER") do
   config :pulso, PulsoWeb.Endpoint, server: true
 end
@@ -28,9 +30,9 @@ config :pulso, PulsoWeb.Endpoint, http: [port: String.to_integer(System.get_env(
 # out-of-the-box credentials. Prod requires the env vars to be set explicitly.
 case config_env() do
   :dev ->
-    config :pulso, Pulso.Storage, adapter: Pulso.Storage.S3
+    config :pulso, Pulso.Storage, adapter: S3
 
-    config :pulso, Pulso.Storage.S3,
+    config :pulso, S3,
       bucket: System.get_env("PULSO_S3_BUCKET", "pulso"),
       endpoint: System.get_env("PULSO_S3_ENDPOINT", "http://localhost:9000"),
       region: System.get_env("PULSO_S3_REGION", "us-east-1"),
@@ -39,14 +41,27 @@ case config_env() do
       allow_http: System.get_env("PULSO_S3_ALLOW_HTTP", "true") in ["1", "true", "yes"]
 
   :prod ->
-    config :pulso, Pulso.Storage, adapter: Pulso.Storage.S3
+    require_env = fn name ->
+      case System.get_env(name) do
+        value when is_binary(value) and value != "" ->
+          value
+
+        _ ->
+          raise """
+          environment variable #{name} is missing or empty.
+          Pulso.Storage.S3 requires bucket/region/credentials in prod.
+          """
+      end
+    end
+
+    config :pulso, Pulso.Storage, adapter: S3
 
-    config :pulso, Pulso.Storage.S3,
-      bucket: System.fetch_env!("PULSO_S3_BUCKET"),
+    config :pulso, S3,
+      bucket: require_env.("PULSO_S3_BUCKET"),
       endpoint: System.get_env("PULSO_S3_ENDPOINT"),
-      region: System.fetch_env!("PULSO_S3_REGION"),
-      access_key_id: System.fetch_env!("PULSO_S3_ACCESS_KEY_ID"),
-      secret_access_key: System.fetch_env!("PULSO_S3_SECRET_ACCESS_KEY"),
+      region: require_env.("PULSO_S3_REGION"),
+      access_key_id: require_env.("PULSO_S3_ACCESS_KEY_ID"),
+      secret_access_key: require_env.("PULSO_S3_SECRET_ACCESS_KEY"),
       allow_http: System.get_env("PULSO_S3_ALLOW_HTTP", "false") in ["1", "true", "yes"]
 
   :test ->
diff --git a/docker-compose.yml b/docker-compose.yml
index cc96fb4..546950a 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -12,14 +12,19 @@ services:
   rustfs:
     image: rustfs/rustfs:latest
     environment:
+      # RUSTFS_VOLUMES is required — without it RustFS exits at startup.
+      # The `{0..3}` template creates four storage volumes under /data.
+      RUSTFS_VOLUMES: /data/rustfs{0..3}
       RUSTFS_ADDRESS: 0.0.0.0:9000
       RUSTFS_CONSOLE_ADDRESS: 0.0.0.0:9001
       RUSTFS_CONSOLE_ENABLE: "true"
       RUSTFS_ACCESS_KEY: rustfsadmin
       RUSTFS_SECRET_KEY: rustfsadmin
+    # Bind to loopback only so the well-known dev credentials cannot be reached
+    # from another host on the LAN.
     ports:
-      - "9000:9000"
-      - "9001:9001"
+      - "127.0.0.1:9000:9000"
+      - "127.0.0.1:9001:9001"
     volumes:
       - rustfs-data:/data
 
diff --git a/lib/pulso/application.ex b/lib/pulso/application.ex
index 2c94cb0..a671c7e 100644
--- a/lib/pulso/application.ex
+++ b/lib/pulso/application.ex
@@ -42,7 +42,7 @@ defmodule Pulso.Application do
 
     case adapter do
       nil -> [Memory]
-      Pulso.Storage.Memory -> [Memory]
+      Memory -> [Memory]
       _ -> []
     end
   end
diff --git a/lib/pulso/storage/s3.ex b/lib/pulso/storage/s3.ex
index 7a619be..141d80c 100644
--- a/lib/pulso/storage/s3.ex
+++ b/lib/pulso/storage/s3.ex
@@ -8,10 +8,18 @@ defmodule Pulso.Storage.S3 do
   so a batch cannot land outside its own prefix. `query/2` lists the tenant's
   prefix, downloads every object, decodes NDJSON, filters and sorts in Elixir.
 
-  This is deliberately naive. Step 3 replaces the flat NDJSON layout with
-  columnar segments and a manifest that supports conditional writes; the
-  point of step 2 is only to prove logs round-trip through real object
-  storage against the `Pulso.ObjectStore` NIF.
+  Known limits of this adapter, deferred to step 3 (segments + manifest CAS):
+
+    * **Not idempotent.** An `append` that succeeds but whose response is lost
+      will be duplicated by a retry — each call generates a fresh random
+      object key. Step 3 introduces a manifest with conditional writes.
+    * **Unbounded query work.** Every query lists and downloads every object
+      under the tenant prefix before applying `limit`. Fine for a small tenant
+      or a smoke test; not fine at scale.
+    * **No cross-tenant authentication.** Tenant validation here only prevents
+      key-escape; it does not verify that the caller is *allowed* to read or
+      write the tenant they named. Auth belongs at the ingest boundary.
+    * **No columnar layout.** Records go on the wire as NDJSON, not Parquet.
   """
 
   @behaviour Pulso.Storage
@@ -26,13 +34,17 @@ defmodule Pulso.Storage.S3 do
   @sort_key_width 20
 
   @impl Pulso.Storage
-  def append(_tenant, []), do: :ok
+  def append(tenant, []) when is_binary(tenant) do
+    # Validate even on empty so an adversarial tenant name is rejected on the
+    # first attempt, not only once a real record survives OTLP decoding.
+    validate_tenant(tenant)
+  end
 
   def append(tenant, records) when is_binary(tenant) and is_list(records) do
     with :ok <- validate_tenant(tenant),
-         normalized <- normalize(records),
-         payload when is_binary(payload) <- encode(normalized),
-         key <- object_key(tenant, batch_sort_ns(normalized)) do
+         normalized = normalize(records),
+         {:ok, payload} <- encode(normalized) do
+      key = object_key(tenant, batch_sort_ns(normalized))
       ObjectStore.put(config!(), key, payload)
     end
   end
@@ -40,7 +52,7 @@ defmodule Pulso.Storage.S3 do
   @impl Pulso.Storage
   def query(tenant, opts) when is_binary(tenant) and is_list(opts) do
     with :ok <- validate_tenant(tenant),
-         config <- config!(),
+         config = config!(),
          {:ok, keys} <- ObjectStore.list(config, prefix(tenant)),
          {:ok, records} <- fetch_records(config, keys) do
       filtered =
@@ -81,18 +93,35 @@ defmodule Pulso.Storage.S3 do
   end
 
   defp encode(records) do
-    records
-    |> Enum.map(&(Jason.encode!(Map.from_struct(&1)) <> "\n"))
-    |> IO.iodata_to_binary()
+    encoded =
+      Enum.reduce_while(records, {:ok, []}, fn record, {:ok, acc} ->
+        case Jason.encode(Map.from_struct(record)) do
+          {:ok, line} -> {:cont, {:ok, [[line, "\n"] | acc]}}
+          {:error, reason} -> {:halt, {:error, {:encode_failed, reason}}}
+        end
+      end)
+
+    with {:ok, lines} <- encoded do
+      {:ok, lines |> Enum.reverse() |> IO.iodata_to_binary()}
+    end
   end
 
   defp fetch_records(config, keys) do
-    Enum.reduce_while(keys, {:ok, []}, fn key, {:ok, acc} ->
+    # Accumulate batches as a list of lists then flatten once, so a large
+    # tenant does not pay O(n^2) list concatenation. Any get error halts the
+    # query — silently skipping would hide backend outages. Distinguishing a
+    # since-deleted key from a real failure requires typed errors from the
+    # NIF (step 3, once compaction can delete out from under a reader).
+    Enum.reduce_while(keys, {:ok, []}, fn key, {:ok, batches} ->
       case ObjectStore.get(config, key) do
-        {:ok, blob} -> {:cont, {:ok, acc ++ decode(blob)}}
+        {:ok, blob} -> {:cont, {:ok, [decode(blob) | batches]}}
         {:error, _} = err -> {:halt, err}
       end
     end)
+    |> case do
+      {:ok, batches} -> {:ok, batches |> Enum.reverse() |> List.flatten()}
+      {:error, _} = err -> err
+    end
   end
 
   defp decode(blob) do
diff --git a/test/pulso/storage/s3_test.exs b/test/pulso/storage/s3_test.exs
index 0406fc4..3a2c39f 100644
--- a/test/pulso/storage/s3_test.exs
+++ b/test/pulso/storage/s3_test.exs
@@ -21,7 +21,7 @@ defmodule Pulso.Storage.S3Test do
       allow_http: true
     }
 
-    Application.put_env(:pulso, Pulso.Storage.S3, config)
+    Application.put_env(:pulso, S3, config)
 
     tenant = "test-#{System.unique_integer([:positive])}"
 
diff --git a/test/pulso/storage/s3_unit_test.exs b/test/pulso/storage/s3_unit_test.exs
new file mode 100644
index 0000000..7054382
--- /dev/null
+++ b/test/pulso/storage/s3_unit_test.exs
@@ -0,0 +1,46 @@
+defmodule Pulso.Storage.S3UnitTest do
+  # Pure-Elixir tests of Pulso.Storage.S3's pre-network guards. Anything that
+  # actually talks to an S3 endpoint lives in s3_test.exs behind the
+  # `:integration` tag.
+
+  use ExUnit.Case, async: true
+
+  alias Pulso.Record.Log
+  alias Pulso.Storage.S3
+
+  describe "tenant validation" do
+    test "rejects adversarial tenant names before touching the object store" do
+      for bad <- ["", "../evil", "foo/bar", "foo bar", String.duplicate("a", 200)] do
+        assert {:error, {:invalid_tenant, ^bad}} = S3.append(bad, [%Log{timestamp_ns: 1}])
+        assert {:error, {:invalid_tenant, ^bad}} = S3.query(bad, [])
+      end
+    end
+
+    test "rejects adversarial tenant names even for empty batches" do
+      # Prior version bailed early on `[]` and returned :ok, letting an
+      # attacker probe the auth surface with a no-op payload.
+      assert {:error, {:invalid_tenant, "../evil"}} = S3.append("../evil", [])
+      assert {:error, {:invalid_tenant, ""}} = S3.append("", [])
+    end
+
+    test "accepts common tenant name shapes" do
+      # These do not touch the object store because the batch is empty.
+      for good <- ["default", "customer-42", "team.alpha", "svc_web"] do
+        assert :ok = S3.append(good, [])
+      end
+    end
+  end
+
+  describe "encode failures" do
+    test "returns an error tuple instead of raising on non-UTF-8 body bytes" do
+      # A Jason encoder that meets non-UTF-8 bytes in a string field must not
+      # crash the ingest process; it must surface an error the OTLP controller
+      # can map to a 4xx/5xx response.
+      record = %Log{timestamp_ns: 1, body: <<0xFF, 0xFE>>}
+
+      # `append` on a well-formed tenant with a non-encodable body should
+      # bail before any network I/O.
+      assert {:error, {:encode_failed, _}} = S3.append("default", [record])
+    end
+  end
+end

From 5a628e3a07379d3774954aeedd760a22e4736a48 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pedro=20Pi=C3=B1era=20Buend=C3=ADa?= <pedro@pepicrft.me>
Date: Thu, 24 Sep 2026 06:29:28 +0200
Subject: [PATCH 03/17] Address remaining Codex findings; scope dev ports per
 worktree
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Follow-up on the earlier Codex adversarial review. Every finding is either
fixed here or documented as fundamentally step-3 shaped in the module doc.

Correctness / idempotency
- Object keys are now content-addressed
  (tenants/<tenant>/logs/<sort_ns>-<sha256_prefix>.ndjson). A retry with an
  identical batch lands on the same key, so a lost-response retry does not
  duplicate records. Truncated SHA-256 (64 bits) also drops the collision
  risk of the old random 8-byte suffix.
- Attribute and resource map keys are coerced to strings on append via
  `Pulso.Storage.S3.sanitize_map/1`, so a caller that hands us atom or int
  keys (a future path where OTLP is not the only producer) does not silently
  collide on the JSON round trip.
- Query order is now delegated to `Pulso.Storage.SortOrder` and shared by
  Memory and S3. Ties break on observed_timestamp_ns, then trace_id,
  span_id, body — deterministic across adapters so a client that switches
  backends never sees the limit response reshuffle.
- The Rust NIF distinguishes NotFound from other errors (returns the atom
  `:not_found`). `Pulso.Storage.S3.query/2` treats it as "raced with a
  delete" and skips; other errors halt so an outage is never hidden.

Auth boundary
- New `Pulso.Auth` behavior with two impls:
    * `Pulso.Auth.Open` (default; accepts everything, matches prior behavior
      for dev/test)
    * `Pulso.Auth.SharedSecret` (bearer token per tenant, hashed with
      sha256, compared in constant time; required in prod)
- `PulsoWeb.OTLPController` verifies the caller before appending, returning
  401 for auth failures, 400 for invalid tenant names, 500 otherwise.
- Prod runtime.exs requires `PULSO_TENANT_TOKENS` (JSON) and rejects empty
  values for every PULSO_S3_* env var — same treatment as the S3 config.

Per-worktree port scoping
- Ports adapted from tuist/tuist. `mise/utilities/dev_instance_env.sh`
  runs on every `mise` invocation, persists a suffix into
  `.git/worktrees/<name>/pulso-dev-instance` (falling back to
  `.pulso-dev-instance` in the checkout root), and exports:
    * PULSO_DEV_INSTANCE
    * PORT                       (4000 + suffix; Phoenix)
    * PULSO_RUSTFS_API_PORT      (9095 + suffix)
    * PULSO_RUSTFS_CONSOLE_PORT  (9098 + suffix)
    * PULSO_S3_ENDPOINT          (http://localhost:$PULSO_RUSTFS_API_PORT)
  Two worktrees now run their own Phoenix and RustFS side by side without
  the second one binding on top of the first.
- docker-compose.yml interpolates the RustFS host ports from these vars
  (with the previous defaults as fallback).
- runtime.exs dev reads PULSO_S3_ENDPOINT — mise sets it, and the fallback
  9195 matches the compose default port.

What's still deferred, deliberately
- Unbounded query work when `limit` is set. Segment min/max metadata (step
  3) is required to safely early-exit; short-circuiting on key order alone
  would break sort semantics because a recently-written batch can carry
  old-timestamp records. The module docstring names this explicitly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 .gitignore                                    |   6 +
 config/runtime.exs                            |  34 +++-
 docker-compose.yml                            |  16 +-
 lib/pulso/auth.ex                             |  41 +++++
 lib/pulso/auth/open.ex                        |  14 ++
 lib/pulso/auth/shared_secret.ex               |  67 ++++++++
 lib/pulso/storage/memory.ex                   |   3 +-
 lib/pulso/storage/s3.ex                       | 103 ++++++++----
 lib/pulso/storage/sort_order.ex               |  33 ++++
 lib/pulso_web/controllers/otlp_controller.ex  |  32 +++-
 mise.toml                                     |   7 +
 mise/utilities/dev_instance_env.sh            | 147 ++++++++++++++++++
 native/pulso_object_store/src/lib.rs          |  17 +-
 test/pulso/auth_test.exs                      |  80 ++++++++++
 test/pulso/storage/memory_test.exs            |  13 ++
 test/pulso/storage/s3_test.exs                |  45 ++++++
 test/pulso/storage/s3_unit_test.exs           |  70 +++++++--
 test/pulso/storage/sort_order_test.exs        |  55 +++++++
 .../controllers/otlp_controller_test.exs      |  60 +++++++
 19 files changed, 786 insertions(+), 57 deletions(-)
 create mode 100644 lib/pulso/auth.ex
 create mode 100644 lib/pulso/auth/open.ex
 create mode 100644 lib/pulso/auth/shared_secret.ex
 create mode 100644 lib/pulso/storage/sort_order.ex
 create mode 100644 mise/utilities/dev_instance_env.sh
 create mode 100644 test/pulso/auth_test.exs
 create mode 100644 test/pulso/storage/sort_order_test.exs

diff --git a/.gitignore b/.gitignore
index 86caa1f..66795bf 100644
--- a/.gitignore
+++ b/.gitignore
@@ -29,3 +29,9 @@ pulso-*.tar
 # shared object into priv/native/. Both are build artifacts.
 native/*/target/
 /priv/native/
+
+# Per-worktree dev-instance suffix written by mise/utilities/dev_instance_env.sh.
+# Persisted inside .git/worktrees/*/pulso-dev-instance by design; the
+# top-level fallback is tracked here as a safety net for setups where the
+# git-scoped path is unwritable.
+/.pulso-dev-instance
diff --git a/config/runtime.exs b/config/runtime.exs
index aca7253..1f6e312 100644
--- a/config/runtime.exs
+++ b/config/runtime.exs
@@ -16,6 +16,7 @@ import Config
 #
 # Alternatively, you can use `mix phx.gen.release` to generate a `bin/server`
 # script that automatically sets the env var above.
+alias Pulso.Auth.SharedSecret
 alias Pulso.Storage.S3
 
 if System.get_env("PHX_SERVER") do
@@ -34,7 +35,10 @@ case config_env() do
 
     config :pulso, S3,
       bucket: System.get_env("PULSO_S3_BUCKET", "pulso"),
-      endpoint: System.get_env("PULSO_S3_ENDPOINT", "http://localhost:9000"),
+      # mise/utilities/dev_instance_env.sh sets PULSO_S3_ENDPOINT per worktree.
+      # The fallback matches the docker-compose default host port when mise
+      # is not in the loop.
+      endpoint: System.get_env("PULSO_S3_ENDPOINT", "http://localhost:9195"),
       region: System.get_env("PULSO_S3_REGION", "us-east-1"),
       access_key_id: System.get_env("PULSO_S3_ACCESS_KEY_ID", "rustfsadmin"),
       secret_access_key: System.get_env("PULSO_S3_SECRET_ACCESS_KEY", "rustfsadmin"),
@@ -54,6 +58,34 @@ case config_env() do
       end
     end
 
+    # Tenant tokens. Expected shape: a JSON object mapping tenant name to
+    # "sha256$<hex-of-sha256-of-token>". Deployments compute the hash offline
+    # and store only the digest in env, never the plaintext token.
+    tokens =
+      case System.get_env("PULSO_TENANT_TOKENS") do
+        blob when is_binary(blob) and blob != "" ->
+          case Jason.decode(blob) do
+            {:ok, map} when is_map(map) ->
+              map
+
+            {:ok, _} ->
+              raise "PULSO_TENANT_TOKENS must decode to a JSON object"
+
+            {:error, reason} ->
+              raise "PULSO_TENANT_TOKENS is not valid JSON: #{inspect(reason)}"
+          end
+
+        _ ->
+          raise """
+          environment variable PULSO_TENANT_TOKENS is missing or empty.
+          Pulso.Auth.SharedSecret requires at least one tenant token in prod.
+          """
+      end
+
+    config :pulso, Pulso.Auth,
+      module: SharedSecret,
+      tokens: tokens
+
     config :pulso, Pulso.Storage, adapter: S3
 
     config :pulso, S3,
diff --git a/docker-compose.yml b/docker-compose.yml
index 546950a..0c1b99d 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -1,8 +1,11 @@
 # Local development stack for Pulso.
 #
-# `docker compose up -d` gives you a RustFS (S3-compatible) server on
-# http://localhost:9000 with a preseeded bucket named `pulso`. The console is
-# on http://localhost:9001 (user/password: rustfsadmin/rustfsadmin).
+# `docker compose up -d` gives you a RustFS (S3-compatible) server with a
+# preseeded bucket named `pulso`. The published ports are derived from
+# PULSO_RUSTFS_API_PORT / PULSO_RUSTFS_CONSOLE_PORT, which
+# mise/utilities/dev_instance_env.sh scopes per git worktree so parallel
+# worktrees do not collide. Defaults (9195 / 9198) apply when the vars are
+# not set. Credentials for the console: rustfsadmin / rustfsadmin.
 #
 # RustFS is used instead of MinIO because MinIO's community edition is no
 # longer actively maintained. Pulso only talks to the S3 API, so any
@@ -21,10 +24,11 @@ services:
       RUSTFS_ACCESS_KEY: rustfsadmin
       RUSTFS_SECRET_KEY: rustfsadmin
     # Bind to loopback only so the well-known dev credentials cannot be reached
-    # from another host on the LAN.
+    # from another host on the LAN. Host ports come from the per-worktree env
+    # to keep multiple checkouts from fighting over 9000/9001.
     ports:
-      - "127.0.0.1:9000:9000"
-      - "127.0.0.1:9001:9001"
+      - "127.0.0.1:${PULSO_RUSTFS_API_PORT:-9195}:9000"
+      - "127.0.0.1:${PULSO_RUSTFS_CONSOLE_PORT:-9198}:9001"
     volumes:
       - rustfs-data:/data
 
diff --git a/lib/pulso/auth.ex b/lib/pulso/auth.ex
new file mode 100644
index 0000000..43c3bd8
--- /dev/null
+++ b/lib/pulso/auth.ex
@@ -0,0 +1,41 @@
+defmodule Pulso.Auth do
+  @moduledoc """
+  Boundary for tenant authorization checks.
+
+  `verify/2` is called from ingest and read paths before Pulso attributes a
+  request to a tenant. Implementations decide whether the caller is allowed
+  to speak for the tenant they named; they do **not** decide which tenant a
+  request belongs to (that is a routing concern, e.g. `X-Scope-OrgID`).
+
+  Two implementations ship in-tree:
+
+    * `Pulso.Auth.Open` — accepts everything. Default for dev and test.
+    * `Pulso.Auth.SharedSecret` — requires an `Authorization: Bearer <token>`
+      header that matches a per-tenant token from application env. Default
+      for prod.
+
+  The active implementation is read from `Application.get_env(:pulso,
+  Pulso.Auth)[:module]` at call time so tests can swap it without
+  recompiling.
+  """
+
+  alias Plug.Conn
+  alias Pulso.Auth.Open
+
+  @type reason :: :missing_token | :invalid_token | :unknown_tenant | term()
+
+  @callback verify(Conn.t(), tenant :: String.t()) :: :ok | {:error, reason()}
+
+  @spec verify(Conn.t(), String.t()) :: :ok | {:error, reason()}
+  def verify(conn, tenant) when is_binary(tenant) do
+    module().verify(conn, tenant)
+  end
+
+  @spec module() :: module()
+  def module do
+    case Application.get_env(:pulso, __MODULE__) do
+      nil -> Open
+      env -> Keyword.get(env, :module, Open)
+    end
+  end
+end
diff --git a/lib/pulso/auth/open.ex b/lib/pulso/auth/open.ex
new file mode 100644
index 0000000..d19c02e
--- /dev/null
+++ b/lib/pulso/auth/open.ex
@@ -0,0 +1,14 @@
+defmodule Pulso.Auth.Open do
+  @moduledoc """
+  No-auth implementation of `Pulso.Auth`. Accepts every request.
+
+  Default for dev and test where the loopback-bound ingest port makes a
+  real credential check net negative. Never suitable for a prod deployment
+  reachable from the network.
+  """
+
+  @behaviour Pulso.Auth
+
+  @impl Pulso.Auth
+  def verify(_conn, _tenant), do: :ok
+end
diff --git a/lib/pulso/auth/shared_secret.ex b/lib/pulso/auth/shared_secret.ex
new file mode 100644
index 0000000..fdf0a3a
--- /dev/null
+++ b/lib/pulso/auth/shared_secret.ex
@@ -0,0 +1,67 @@
+defmodule Pulso.Auth.SharedSecret do
+  @moduledoc """
+  Per-tenant Bearer-token authentication. A minimal but real auth surface
+  suitable for prod until Pulso grows a real accounts service.
+
+  Tokens are configured as:
+
+      config :pulso, Pulso.Auth,
+        module: Pulso.Auth.SharedSecret,
+        tokens: %{"acme" => "sha256$<hex>", "beta" => "sha256$<hex>"}
+
+  Values are of the form `"<algo>$<hex>"`. Only `sha256` is accepted today.
+  Comparison is constant-time (`Plug.Crypto.secure_compare/2`) so token
+  presence cannot be inferred from response timing.
+
+  A tenant with no configured token is rejected as `:unknown_tenant`. Empty
+  tokens are rejected as `:invalid_token` so an accidentally-blank env var
+  cannot turn into "auth off for this tenant".
+  """
+
+  @behaviour Pulso.Auth
+
+  alias Plug.Conn
+
+  @impl Pulso.Auth
+  def verify(conn, tenant) when is_binary(tenant) do
+    with {:ok, presented} <- extract_token(conn),
+         {:ok, stored_hash} <- fetch_stored_hash(tenant) do
+      compare_hash(presented, stored_hash)
+    end
+  end
+
+  defp extract_token(conn) do
+    case Conn.get_req_header(conn, "authorization") do
+      ["Bearer " <> token] when byte_size(token) > 0 -> {:ok, token}
+      ["bearer " <> token] when byte_size(token) > 0 -> {:ok, token}
+      _ -> {:error, :missing_token}
+    end
+  end
+
+  defp fetch_stored_hash(tenant) do
+    tokens =
+      case Application.get_env(:pulso, Pulso.Auth) do
+        nil -> %{}
+        env -> Keyword.get(env, :tokens, %{})
+      end
+
+    case Map.fetch(tokens, tenant) do
+      {:ok, "sha256$" <> hex} when byte_size(hex) == 64 -> {:ok, hex}
+      {:ok, _malformed} -> {:error, :invalid_token}
+      :error -> {:error, :unknown_tenant}
+    end
+  end
+
+  defp compare_hash(presented, stored_hex) do
+    computed_hex =
+      :sha256
+      |> :crypto.hash(presented)
+      |> Base.encode16(case: :lower)
+
+    if Plug.Crypto.secure_compare(computed_hex, stored_hex) do
+      :ok
+    else
+      {:error, :invalid_token}
+    end
+  end
+end
diff --git a/lib/pulso/storage/memory.ex b/lib/pulso/storage/memory.ex
index 66e9a55..cad7277 100644
--- a/lib/pulso/storage/memory.ex
+++ b/lib/pulso/storage/memory.ex
@@ -18,6 +18,7 @@ defmodule Pulso.Storage.Memory do
   use GenServer
 
   alias Pulso.Record.Log
+  alias Pulso.Storage.SortOrder
 
   @table __MODULE__
 
@@ -57,7 +58,7 @@ defmodule Pulso.Storage.Memory do
       records
       |> filter_by_time(Keyword.get(opts, :start_ts), Keyword.get(opts, :end_ts))
       |> filter_by_service(Keyword.get(opts, :service))
-      |> Enum.sort_by(& &1.timestamp_ns, :desc)
+      |> SortOrder.sort()
       |> take_limit(Keyword.get(opts, :limit))
 
     {:ok, filtered}
diff --git a/lib/pulso/storage/s3.ex b/lib/pulso/storage/s3.ex
index 141d80c..7064f80 100644
--- a/lib/pulso/storage/s3.ex
+++ b/lib/pulso/storage/s3.ex
@@ -3,22 +3,29 @@ defmodule Pulso.Storage.S3 do
   S3-backed log storage. Step 2 adapter.
 
   Each `append/2` writes one NDJSON object under a tenant-scoped prefix
-  (`tenants/<tenant>/logs/<sort_key>.ndjson`). Tenant isolation is enforced by
-  key construction: tenant names are validated against a conservative charset
-  so a batch cannot land outside its own prefix. `query/2` lists the tenant's
-  prefix, downloads every object, decodes NDJSON, filters and sorts in Elixir.
-
-  Known limits of this adapter, deferred to step 3 (segments + manifest CAS):
-
-    * **Not idempotent.** An `append` that succeeds but whose response is lost
-      will be duplicated by a retry — each call generates a fresh random
-      object key. Step 3 introduces a manifest with conditional writes.
-    * **Unbounded query work.** Every query lists and downloads every object
-      under the tenant prefix before applying `limit`. Fine for a small tenant
-      or a smoke test; not fine at scale.
-    * **No cross-tenant authentication.** Tenant validation here only prevents
-      key-escape; it does not verify that the caller is *allowed* to read or
-      write the tenant they named. Auth belongs at the ingest boundary.
+  (`tenants/<tenant>/logs/<sort_ns>-<content_hash>.ndjson`). Tenant isolation
+  is enforced by key construction: tenant names are validated against a
+  conservative charset so a batch cannot land outside its own prefix. The
+  key's suffix is a truncated SHA-256 of the encoded payload, which makes
+  identical retries land on the same object — a client that PUTs the same
+  batch twice under a lost-response retry does not create a duplicate.
+
+  `query/2` lists the tenant prefix, downloads every object, decodes NDJSON,
+  filters, and sorts. Order matches `Pulso.Storage.Memory`: `timestamp_ns`
+  descending, then `observed_timestamp_ns` descending, then `trace_id`, then
+  `body`, so equal-timestamp ties resolve identically across adapters.
+  A `NotFound` for a key that was listed but disappeared before the fetch
+  (concurrent retention, compaction, another process deleting) is skipped
+  rather than aborting the query.
+
+  Known limits, deferred to step 3 (segments + manifest):
+
+    * **Unbounded query work when `limit` is set.** Without per-batch time
+      metadata this adapter cannot safely skip objects: a batch written
+      recently may contain an old-timestamp record, so scanning every
+      object is required to preserve the sort semantics. Step 3's segment
+      manifest will carry min/max `timestamp_ns` per segment and let the
+      query short-circuit.
     * **No columnar layout.** Records go on the wire as NDJSON, not Parquet.
   """
 
@@ -26,12 +33,15 @@ defmodule Pulso.Storage.S3 do
 
   alias Pulso.ObjectStore
   alias Pulso.Record.Log
+  alias Pulso.Storage.SortOrder
 
   @tenant_regex ~r/\A[A-Za-z0-9_.\-]{1,128}\z/
   # 20 decimal digits fits a u64 nanosecond timestamp (max ~1.84e19). Zero-padding
-  # this way makes the S3 list order roughly chronological, which lets query
-  # short-circuit once the sort/limit is satisfied in a later step.
+  # keeps S3's UTF-8 list order chronological by write time (`sort_ns`).
   @sort_key_width 20
+  # 16 hex chars = 64 bits from SHA-256. Collision probability is negligible
+  # for the volumes any single tenant will produce in a step-2 adapter.
+  @content_hash_width 16
 
   @impl Pulso.Storage
   def append(tenant, []) when is_binary(tenant) do
@@ -44,7 +54,7 @@ defmodule Pulso.Storage.S3 do
     with :ok <- validate_tenant(tenant),
          normalized = normalize(records),
          {:ok, payload} <- encode(normalized) do
-      key = object_key(tenant, batch_sort_ns(normalized))
+      key = object_key(tenant, batch_sort_ns(normalized), payload)
       ObjectStore.put(config!(), key, payload)
     end
   end
@@ -59,7 +69,7 @@ defmodule Pulso.Storage.S3 do
         records
         |> filter_by_time(Keyword.get(opts, :start_ts), Keyword.get(opts, :end_ts))
         |> filter_by_service(Keyword.get(opts, :service))
-        |> Enum.sort_by(& &1.timestamp_ns, :desc)
+        |> SortOrder.sort()
         |> take_limit(Keyword.get(opts, :limit))
 
       {:ok, filtered}
@@ -80,13 +90,39 @@ defmodule Pulso.Storage.S3 do
     now = System.system_time(:nanosecond)
 
     for %Log{} = record <- records do
-      %{record | observed_timestamp_ns: record.observed_timestamp_ns || now}
+      %{
+        record
+        | observed_timestamp_ns: record.observed_timestamp_ns || now,
+          attributes: sanitize_map(record.attributes || %{}),
+          resource: sanitize_map(record.resource || %{})
+      }
     end
   end
 
+  # Force every attribute/resource map key to be a string and drop nils. OTLP
+  # decoding already produces string keys, but a caller building `%Log{}`
+  # directly (or a future backend surface) could hand us atoms or integers.
+  # Encoding those with Jason coerces them to strings, so two logical keys
+  # can silently collapse on the round trip. Doing the coercion here makes
+  # the write path deterministic and the decode path lossless.
+  @doc false
+  @spec sanitize_map(map()) :: map()
+  def sanitize_map(map) when is_map(map) do
+    Map.new(map, fn {k, v} -> {stringify_key(k), sanitize_value(v)} end)
+  end
+
+  defp stringify_key(k) when is_binary(k), do: k
+  defp stringify_key(k) when is_atom(k), do: Atom.to_string(k)
+  defp stringify_key(k) when is_integer(k), do: Integer.to_string(k)
+  defp stringify_key(k), do: inspect(k)
+
+  defp sanitize_value(v) when is_map(v), do: sanitize_map(v)
+  defp sanitize_value(v) when is_list(v), do: Enum.map(v, &sanitize_value/1)
+  defp sanitize_value(v), do: v
+
   defp batch_sort_ns(records) do
-    # Pick the smallest observed_timestamp_ns so the first key in a listing is
-    # the earliest batch. Every record is normalized, so this is never nil.
+    # Pick the smallest observed_timestamp_ns so identical retries hash into
+    # the same object key. Every record is normalized, so this is never nil.
     records
     |> Enum.map(& &1.observed_timestamp_ns)
     |> Enum.min()
@@ -108,13 +144,13 @@ defmodule Pulso.Storage.S3 do
 
   defp fetch_records(config, keys) do
     # Accumulate batches as a list of lists then flatten once, so a large
-    # tenant does not pay O(n^2) list concatenation. Any get error halts the
-    # query — silently skipping would hide backend outages. Distinguishing a
-    # since-deleted key from a real failure requires typed errors from the
-    # NIF (step 3, once compaction can delete out from under a reader).
+    # tenant does not pay O(n^2) list concatenation. A `:not_found` for a
+    # key that vanished after `list` is treated as "raced with a delete" and
+    # skipped; anything else halts the query so an outage is not hidden.
     Enum.reduce_while(keys, {:ok, []}, fn key, {:ok, batches} ->
       case ObjectStore.get(config, key) do
         {:ok, blob} -> {:cont, {:ok, [decode(blob) | batches]}}
+        {:error, :not_found} -> {:cont, {:ok, batches}}
         {:error, _} = err -> {:halt, err}
       end
     end)
@@ -149,8 +185,10 @@ defmodule Pulso.Storage.S3 do
 
   defp prefix(tenant), do: "tenants/#{tenant}/logs/"
 
-  defp object_key(tenant, sort_ns) do
-    "#{prefix(tenant)}#{zero_pad(sort_ns)}-#{rand_suffix()}.ndjson"
+  @doc false
+  @spec object_key(String.t(), non_neg_integer(), binary()) :: String.t()
+  def object_key(tenant, sort_ns, payload) when is_binary(tenant) and is_binary(payload) do
+    "#{prefix(tenant)}#{zero_pad(sort_ns)}-#{content_hash(payload)}.ndjson"
   end
 
   defp zero_pad(ns) when is_integer(ns) and ns >= 0 do
@@ -159,8 +197,11 @@ defmodule Pulso.Storage.S3 do
     |> String.pad_leading(@sort_key_width, "0")
   end
 
-  defp rand_suffix do
-    :crypto.strong_rand_bytes(8) |> Base.encode16(case: :lower)
+  defp content_hash(payload) do
+    :sha256
+    |> :crypto.hash(payload)
+    |> Base.encode16(case: :lower)
+    |> binary_part(0, @content_hash_width)
   end
 
   defp filter_by_time(records, nil, nil), do: records
diff --git a/lib/pulso/storage/sort_order.ex b/lib/pulso/storage/sort_order.ex
new file mode 100644
index 0000000..adf31ad
--- /dev/null
+++ b/lib/pulso/storage/sort_order.ex
@@ -0,0 +1,33 @@
+defmodule Pulso.Storage.SortOrder do
+  @moduledoc """
+  Canonical sort order for a query result. Every storage adapter must apply
+  this so a client that switches adapters — or fans a query out to more than
+  one — cannot observe the tiebreaker changing.
+
+  Primary key: `timestamp_ns` descending (newest first).
+  Tiebreakers, in order: `observed_timestamp_ns` desc, `trace_id`, `span_id`,
+  `body`. `nil` sorts last within its position.
+  """
+
+  alias Pulso.Record.Log
+
+  @spec sort([Log.t()]) :: [Log.t()]
+  def sort(records) do
+    Enum.sort_by(records, &sort_key/1, &compare_desc/2)
+  end
+
+  defp sort_key(%Log{} = r) do
+    {
+      r.timestamp_ns || 0,
+      r.observed_timestamp_ns || 0,
+      # Strings compare lexicographically. Descending sort is what the caller
+      # asked for, so we invert the two lower-priority string tiebreakers by
+      # negating the comparison in `compare_desc/2`.
+      r.trace_id || "",
+      r.span_id || "",
+      r.body || ""
+    }
+  end
+
+  defp compare_desc(a, b), do: a >= b
+end
diff --git a/lib/pulso_web/controllers/otlp_controller.ex b/lib/pulso_web/controllers/otlp_controller.ex
index 23144b9..ea76607 100644
--- a/lib/pulso_web/controllers/otlp_controller.ex
+++ b/lib/pulso_web/controllers/otlp_controller.ex
@@ -1,6 +1,7 @@
 defmodule PulsoWeb.OTLPController do
   use PulsoWeb, :controller
 
+  alias Pulso.Auth
   alias Pulso.OTLP.Logs
   alias Pulso.Storage
 
@@ -10,16 +11,35 @@ defmodule PulsoWeb.OTLPController do
   OTLP/HTTP JSON logs receiver at `POST /v1/logs`.
 
   Tenant is taken from the `X-Scope-OrgID` header (Loki/Cortex convention) and
-  defaults to `"default"` when absent. The success response is the empty
-  `ExportLogsServiceResponse` object per the OTLP spec.
+  defaults to `"default"` when absent. The caller is then verified against
+  the configured `Pulso.Auth` module: `Pulso.Auth.Open` in dev/test accepts
+  everything; `Pulso.Auth.SharedSecret` in prod requires a bearer token.
+
+  On success, the response body is the empty `ExportLogsServiceResponse`
+  object per the OTLP spec.
   """
   def logs(conn, params) do
     tenant = tenant_from(conn)
-    records = Logs.decode(params)
 
-    case Storage.append(tenant, records) do
-      :ok -> json(conn, %{})
-      {:error, reason} -> conn |> put_status(:internal_server_error) |> json(%{error: inspect(reason)})
+    with :ok <- Auth.verify(conn, tenant),
+         records = Logs.decode(params),
+         :ok <- Storage.append(tenant, records) do
+      json(conn, %{})
+    else
+      {:error, reason} when reason in [:missing_token, :invalid_token, :unknown_tenant] ->
+        conn
+        |> put_status(:unauthorized)
+        |> json(%{error: to_string(reason)})
+
+      {:error, {:invalid_tenant, _} = reason} ->
+        conn
+        |> put_status(:bad_request)
+        |> json(%{error: inspect(reason)})
+
+      {:error, reason} ->
+        conn
+        |> put_status(:internal_server_error)
+        |> json(%{error: inspect(reason)})
     end
   end
 
diff --git a/mise.toml b/mise.toml
index 4fe72bc..664d28c 100644
--- a/mise.toml
+++ b/mise.toml
@@ -1,3 +1,10 @@
 [tools]
 erlang = "29.1"
 elixir = "1.20.4-otp-29"
+
+[env]
+# Sourced by mise on every invocation inside the project (and each worktree).
+# Exports PULSO_DEV_INSTANCE, PORT, PULSO_RUSTFS_API_PORT,
+# PULSO_RUSTFS_CONSOLE_PORT, and PULSO_S3_ENDPOINT so multiple worktrees do
+# not fight over the same TCP ports.
+_.source = "{{config_root}}/mise/utilities/dev_instance_env.sh"
diff --git a/mise/utilities/dev_instance_env.sh b/mise/utilities/dev_instance_env.sh
new file mode 100644
index 0000000..72c9000
--- /dev/null
+++ b/mise/utilities/dev_instance_env.sh
@@ -0,0 +1,147 @@
+# Per-worktree port and instance suffix scoping. Sourced by mise so every
+# `mise` invocation inside this project (or a linked worktree) exports a
+# stable instance suffix and a set of ports derived from it.
+#
+# Adapted from tuist/tuist. The persisted suffix lives inside git's per-
+# worktree state directory, so two worktrees pointing at the same repo get
+# distinct suffixes and never collide on Phoenix, RustFS, or console ports.
+
+if [[ -n "${BASH_SOURCE[0]:-}" ]]; then
+  SCRIPT_PATH="${BASH_SOURCE[0]}"
+elif [[ -n "${ZSH_VERSION:-}" ]]; then
+  SCRIPT_PATH="${(%):-%x}"
+else
+  SCRIPT_PATH="${0}"
+fi
+
+SCRIPT_DIR="$(cd "$(dirname "${SCRIPT_PATH}")" && pwd)"
+PROJECT_ROOT="$(cd "${SCRIPT_DIR}/../.." && pwd)"
+ROOT_INSTANCE_FILE="${PROJECT_ROOT}/.pulso-dev-instance"
+
+resolve_git_path() {
+  local target_name="$1"
+  local fallback_path="$2"
+  local git_path=""
+
+  if command -v git >/dev/null 2>&1 && git -C "${PROJECT_ROOT}" rev-parse --is-inside-work-tree >/dev/null 2>&1; then
+    git_path="$(
+      git -C "${PROJECT_ROOT}" rev-parse --path-format=absolute --git-path "${target_name}" 2>/dev/null ||
+        git -C "${PROJECT_ROOT}" rev-parse --git-path "${target_name}" 2>/dev/null ||
+        true
+    )"
+
+    if [[ -n "${git_path}" && "${git_path}" != /* ]]; then
+      git_path="${PROJECT_ROOT}/${git_path#./}"
+    fi
+  fi
+
+  if [[ -n "${git_path}" ]]; then
+    printf '%s' "${git_path}"
+  else
+    printf '%s' "${fallback_path}"
+  fi
+}
+
+INSTANCE_FILE="$(resolve_git_path "pulso-dev-instance" "${ROOT_INSTANCE_FILE}")"
+
+validate_suffix() {
+  local suffix="$1"
+  [[ "$suffix" =~ ^[0-9]+$ ]] || return 1
+  (( suffix >= 1 && suffix <= 999 ))
+}
+
+persist_suffix() {
+  local suffix="$1"
+  local target="$2"
+
+  mkdir -p "$(dirname "${target}")" 2>/dev/null || return 1
+  printf '%s' "${suffix}" | tee "${target}" >/dev/null 2>&1
+}
+
+collect_used_suffixes() {
+  # Suffixes already claimed by the main checkout and every linked worktree,
+  # so a freshly generated one can dodge collisions.
+  local common_dir="" f
+  if command -v git >/dev/null 2>&1 && git -C "${PROJECT_ROOT}" rev-parse --is-inside-work-tree >/dev/null 2>&1; then
+    common_dir="$(git -C "${PROJECT_ROOT}" rev-parse --path-format=absolute --git-common-dir 2>/dev/null || true)"
+  fi
+  [[ -n "${common_dir}" && -d "${common_dir}" ]] || return 0
+
+  for f in "${common_dir}/pulso-dev-instance" "${common_dir}"/worktrees/*/pulso-dev-instance; do
+    [[ -s "${f}" ]] || continue
+    [[ "${f}" -ef "${INSTANCE_FILE}" ]] 2>/dev/null && continue
+    tr -d '[:space:]' < "${f}"
+    printf '\n'
+  done
+}
+
+generate_suffix() {
+  # Pick a suffix in [100, 999] not used by any other instance. Seed awk's RNG
+  # with the PID so worktrees bootstrapped within the same second diverge
+  # instead of sharing awk's default time(0) seed.
+  local used
+  used="$(collect_used_suffixes | tr '\n' ' ')"
+  awk -v used="${used}" -v seed="$$" '
+    BEGIN {
+      srand(seed)
+      n = split(used, list, " ")
+      for (i = 1; i <= n; i++) taken[list[i]] = 1
+      for (attempt = 0; attempt < 100000; attempt++) {
+        candidate = int(100 + rand() * 900)
+        if (!(candidate in taken)) { print candidate; exit 0 }
+      }
+      exit 1
+    }
+  '
+}
+
+ensure_suffix() {
+  local suffix=""
+
+  # This instance's own persisted suffix wins over everything else. Nested
+  # worktrees would otherwise inherit the parent's PULSO_DEV_INSTANCE.
+  if [[ -s "${INSTANCE_FILE}" ]]; then
+    suffix="$(tr -d '[:space:]' < "${INSTANCE_FILE}")"
+  elif [[ -n "${PULSO_DEV_INSTANCE:-}" ]] &&
+    { [[ "${PULSO_DEV_INSTANCE_ROOT:-}" == "${PROJECT_ROOT}" ]] || [[ -z "${PULSO_DEV_INSTANCE_ROOT:-}" ]]; }; then
+    suffix="${PULSO_DEV_INSTANCE}"
+  elif [[ -s "${ROOT_INSTANCE_FILE}" ]]; then
+    suffix="$(tr -d '[:space:]' < "${ROOT_INSTANCE_FILE}")"
+  else
+    suffix="$(generate_suffix)"
+  fi
+
+  validate_suffix "${suffix}" || {
+    echo "Invalid dev instance suffix '${suffix}'. Expected an integer between 1 and 999." >&2
+    return 1
+  }
+
+  if ! persist_suffix "${suffix}" "${INSTANCE_FILE}"; then
+    if [[ "${INSTANCE_FILE}" != "${ROOT_INSTANCE_FILE}" ]] &&
+      persist_suffix "${suffix}" "${ROOT_INSTANCE_FILE}"; then
+      INSTANCE_FILE="${ROOT_INSTANCE_FILE}"
+    else
+      echo "Failed to persist dev instance suffix '${suffix}'." >&2
+      return 1
+    fi
+  fi
+
+  printf '%s' "${suffix}"
+}
+
+suffix="$(ensure_suffix)"
+
+export PULSO_DEV_INSTANCE="${suffix}"
+export PULSO_DEV_INSTANCE_ROOT="${PROJECT_ROOT}"
+
+# Phoenix endpoint port. Base 4000 + suffix keeps the range inside 4100..4999,
+# well clear of the default Phoenix dev port (4000) and the ExUnit port (4002).
+export PORT="$((4000 + suffix))"
+
+# RustFS via docker-compose. Base 9095/9098 mirrors the tuist convention so a
+# host running both projects side by side does not collide. Range: 9195..10097.
+export PULSO_RUSTFS_API_PORT="$((9095 + suffix))"
+export PULSO_RUSTFS_CONSOLE_PORT="$((9098 + suffix))"
+
+# What the Elixir app reads. runtime.exs reads PULSO_S3_ENDPOINT verbatim.
+export PULSO_S3_ENDPOINT="http://localhost:${PULSO_RUSTFS_API_PORT}"
diff --git a/native/pulso_object_store/src/lib.rs b/native/pulso_object_store/src/lib.rs
index 70d74af..a95cf87 100644
--- a/native/pulso_object_store/src/lib.rs
+++ b/native/pulso_object_store/src/lib.rs
@@ -11,7 +11,7 @@ use bytes::Bytes;
 use futures::TryStreamExt;
 use object_store::aws::AmazonS3Builder;
 use object_store::path::Path;
-use object_store::{ObjectStore, PutPayload};
+use object_store::{Error as ObjectStoreError, ObjectStore, PutPayload};
 use once_cell::sync::Lazy;
 use rustler::{Atom, Binary, Env, Error, NifResult, OwnedBinary};
 use std::sync::Arc;
@@ -46,6 +46,17 @@ fn nif_error<E: std::fmt::Display>(err: E) -> Error {
     Error::Term(Box::new(err.to_string()))
 }
 
+// A NotFound response from the object store surfaces as the atom
+// `:not_found` on the Elixir side. Everything else stays as a string
+// message so the caller keeps the underlying context (permission denied,
+// throttling, timeout, etc.).
+fn map_object_store_error(err: ObjectStoreError) -> Error {
+    match err {
+        ObjectStoreError::NotFound { .. } => Error::Term(Box::new(atoms::not_found())),
+        other => Error::Term(Box::new(other.to_string())),
+    }
+}
+
 fn build_store(config: &StoreConfig) -> Result<Arc<dyn ObjectStore>, Error> {
     let mut builder = AmazonS3Builder::new()
         .with_bucket_name(&config.bucket)
@@ -87,7 +98,7 @@ fn get<'a>(env: Env<'a>, config: StoreConfig, key: String) -> NifResult<(Atom, B
             let obj = store.get(&path).await?;
             obj.bytes().await
         })
-        .map_err(nif_error)?;
+        .map_err(map_object_store_error)?;
 
     let mut owned = OwnedBinary::new(bytes.len())
         .ok_or_else(|| Error::Term(Box::new("failed to allocate binary")))?;
@@ -103,7 +114,7 @@ fn delete(config: StoreConfig, key: String) -> NifResult<Atom> {
 
     RUNTIME
         .block_on(async { store.delete(&path).await })
-        .map_err(nif_error)?;
+        .map_err(map_object_store_error)?;
 
     Ok(atoms::ok())
 }
diff --git a/test/pulso/auth_test.exs b/test/pulso/auth_test.exs
new file mode 100644
index 0000000..3681ecb
--- /dev/null
+++ b/test/pulso/auth_test.exs
@@ -0,0 +1,80 @@
+defmodule Pulso.AuthTest do
+  use ExUnit.Case, async: false
+
+  alias Pulso.Auth
+  alias Pulso.Auth.Open
+  alias Pulso.Auth.SharedSecret
+
+  setup do
+    on_exit(fn -> Application.delete_env(:pulso, Auth) end)
+    :ok
+  end
+
+  defp conn(headers \\ []) do
+    Enum.reduce(headers, %Plug.Conn{}, fn {k, v}, c ->
+      Plug.Conn.put_req_header(c, k, v)
+    end)
+  end
+
+  describe "Pulso.Auth.module/0" do
+    test "falls back to Pulso.Auth.Open when nothing is configured" do
+      Application.delete_env(:pulso, Auth)
+      assert Auth.module() == Open
+    end
+
+    test "returns the configured module" do
+      Application.put_env(:pulso, Auth, module: SharedSecret)
+      assert Auth.module() == SharedSecret
+    end
+  end
+
+  describe "Pulso.Auth.Open" do
+    test "accepts any tenant on any conn" do
+      assert Open.verify(conn(), "acme") == :ok
+      assert Open.verify(conn(), "") == :ok
+    end
+  end
+
+  describe "Pulso.Auth.SharedSecret" do
+    setup do
+      token = "the-secret"
+      hex = Base.encode16(:crypto.hash(:sha256, token), case: :lower)
+      Application.put_env(:pulso, Auth, tokens: %{"acme" => "sha256$#{hex}"})
+      {:ok, token: token}
+    end
+
+    test "accepts the correct bearer token", %{token: token} do
+      assert SharedSecret.verify(conn([{"authorization", "Bearer #{token}"}]), "acme") == :ok
+    end
+
+    test "accepts lowercase 'bearer' too", %{token: token} do
+      assert SharedSecret.verify(conn([{"authorization", "bearer #{token}"}]), "acme") == :ok
+    end
+
+    test "rejects a wrong token with :invalid_token" do
+      assert SharedSecret.verify(conn([{"authorization", "Bearer wrong"}]), "acme") ==
+               {:error, :invalid_token}
+    end
+
+    test "rejects a missing header with :missing_token" do
+      assert SharedSecret.verify(conn(), "acme") == {:error, :missing_token}
+    end
+
+    test "rejects an empty bearer with :missing_token" do
+      assert SharedSecret.verify(conn([{"authorization", "Bearer "}]), "acme") ==
+               {:error, :missing_token}
+    end
+
+    test "rejects a tenant with no configured token as :unknown_tenant", %{token: token} do
+      assert SharedSecret.verify(conn([{"authorization", "Bearer #{token}"}]), "other") ==
+               {:error, :unknown_tenant}
+    end
+
+    test "rejects a malformed stored value as :invalid_token" do
+      Application.put_env(:pulso, Auth, tokens: %{"acme" => "plaintext-not-hashed"})
+
+      assert SharedSecret.verify(conn([{"authorization", "Bearer whatever"}]), "acme") ==
+               {:error, :invalid_token}
+    end
+  end
+end
diff --git a/test/pulso/storage/memory_test.exs b/test/pulso/storage/memory_test.exs
index b756c15..d31d6b4 100644
--- a/test/pulso/storage/memory_test.exs
+++ b/test/pulso/storage/memory_test.exs
@@ -61,4 +61,17 @@ defmodule Pulso.Storage.MemoryTest do
     assert {:ok, [%Log{observed_timestamp_ns: observed}]} = Storage.query("t")
     assert observed >= before_append and observed <= after_append
   end
+
+  test "equal timestamps are broken by observed_timestamp_ns then trace_id" do
+    # Guard against a limit response depending on adapter-internal insertion
+    # order. Two adapters must sort ties the same way; both delegate to
+    # Pulso.Storage.SortOrder.
+    a = %Log{timestamp_ns: 10, observed_timestamp_ns: 100, trace_id: "aaa"}
+    b = %Log{timestamp_ns: 10, observed_timestamp_ns: 300, trace_id: "aaa"}
+    c = %Log{timestamp_ns: 10, observed_timestamp_ns: 200, trace_id: "bbb"}
+
+    :ok = Storage.append("t", [a, b, c])
+    assert {:ok, sorted} = Storage.query("t")
+    assert Enum.map(sorted, & &1.observed_timestamp_ns) == [300, 200, 100]
+  end
 end
diff --git a/test/pulso/storage/s3_test.exs b/test/pulso/storage/s3_test.exs
index 3a2c39f..250817c 100644
--- a/test/pulso/storage/s3_test.exs
+++ b/test/pulso/storage/s3_test.exs
@@ -125,4 +125,49 @@ defmodule Pulso.Storage.S3Test do
       assert {:error, {:invalid_tenant, ^bad}} = S3.query(bad, [])
     end
   end
+
+  test "identical batches are idempotent — retrying does not duplicate", %{
+    tenant: tenant,
+    config: config
+  } do
+    # Content-addressed object keys mean the same batch written twice lands
+    # on the same key. This is the retry story for step 2.
+    payload = [record(1, body: "same", service: "api")]
+
+    assert :ok = S3.append(tenant, payload)
+    assert :ok = S3.append(tenant, payload)
+
+    assert {:ok, keys} = ObjectStore.list(config, "tenants/#{tenant}/logs/")
+    assert length(keys) == 1
+
+    assert {:ok, records} = S3.query(tenant, [])
+    assert length(records) == 1
+  end
+
+  test "a key deleted after listing does not fail the query", %{
+    tenant: tenant,
+    config: config
+  } do
+    assert :ok = S3.append(tenant, [record(1), record(2)])
+    assert :ok = S3.append(tenant, [record(3)])
+
+    # Delete one of the objects between our own list and get, mimicking a
+    # compaction / retention job racing with a query.
+    assert {:ok, [first | _]} = ObjectStore.list(config, "tenants/#{tenant}/logs/")
+    assert :ok = ObjectStore.delete(config, first)
+
+    # Query should still return the surviving records, not error.
+    assert {:ok, remaining} = S3.query(tenant, [])
+    assert length(remaining) >= 1
+  end
+
+  test "equal timestamps sort deterministically across adapters", %{tenant: tenant} do
+    a = %Log{timestamp_ns: 10, observed_timestamp_ns: 100, trace_id: "aaa"}
+    b = %Log{timestamp_ns: 10, observed_timestamp_ns: 300, trace_id: "aaa"}
+    c = %Log{timestamp_ns: 10, observed_timestamp_ns: 200, trace_id: "bbb"}
+
+    assert :ok = S3.append(tenant, [a, b, c])
+    assert {:ok, sorted} = S3.query(tenant, [])
+    assert Enum.map(sorted, & &1.observed_timestamp_ns) == [300, 200, 100]
+  end
 end
diff --git a/test/pulso/storage/s3_unit_test.exs b/test/pulso/storage/s3_unit_test.exs
index 7054382..f297fca 100644
--- a/test/pulso/storage/s3_unit_test.exs
+++ b/test/pulso/storage/s3_unit_test.exs
@@ -1,7 +1,7 @@
 defmodule Pulso.Storage.S3UnitTest do
-  # Pure-Elixir tests of Pulso.Storage.S3's pre-network guards. Anything that
-  # actually talks to an S3 endpoint lives in s3_test.exs behind the
-  # `:integration` tag.
+  # Pure-Elixir tests of Pulso.Storage.S3's pre-network guards and key
+  # construction. Anything that actually talks to an S3 endpoint lives in
+  # s3_test.exs behind the `:integration` tag.
 
   use ExUnit.Case, async: true
 
@@ -24,7 +24,6 @@ defmodule Pulso.Storage.S3UnitTest do
     end
 
     test "accepts common tenant name shapes" do
-      # These do not touch the object store because the batch is empty.
       for good <- ["default", "customer-42", "team.alpha", "svc_web"] do
         assert :ok = S3.append(good, [])
       end
@@ -33,14 +32,67 @@ defmodule Pulso.Storage.S3UnitTest do
 
   describe "encode failures" do
     test "returns an error tuple instead of raising on non-UTF-8 body bytes" do
-      # A Jason encoder that meets non-UTF-8 bytes in a string field must not
-      # crash the ingest process; it must surface an error the OTLP controller
-      # can map to a 4xx/5xx response.
       record = %Log{timestamp_ns: 1, body: <<0xFF, 0xFE>>}
 
-      # `append` on a well-formed tenant with a non-encodable body should
-      # bail before any network I/O.
       assert {:error, {:encode_failed, _}} = S3.append("default", [record])
     end
   end
+
+  describe "content-addressed object keys" do
+    test "the same payload maps to the same key so retries do not duplicate" do
+      # Two callers PUT the same batch; if the response of the first is lost
+      # and the second retries, the deterministic key means both PUTs land on
+      # the same object and the second is a no-op overwrite.
+      k1 = S3.object_key("acme", 100, "same-batch")
+      k2 = S3.object_key("acme", 100, "same-batch")
+      assert k1 == k2
+    end
+
+    test "different payloads map to different keys" do
+      k1 = S3.object_key("acme", 100, "batch-a")
+      k2 = S3.object_key("acme", 100, "batch-b")
+      refute k1 == k2
+    end
+
+    test "different tenants never share a prefix" do
+      # Substring is enough — S3 list uses prefix, so any escape would show
+      # up as a shared prefix here.
+      k1 = S3.object_key("alpha", 100, "shared")
+      k2 = S3.object_key("beta", 100, "shared")
+      refute String.starts_with?(k1, "tenants/beta/")
+      refute String.starts_with?(k2, "tenants/alpha/")
+    end
+
+    test "keys sort chronologically by sort_ns within a tenant" do
+      k_early = S3.object_key("acme", 100, "x")
+      k_late = S3.object_key("acme", 200, "x")
+      # S3 list returns keys in UTF-8 byte order; zero-padding puts earlier
+      # timestamps first.
+      assert k_early < k_late
+    end
+  end
+
+  describe "attribute sanitization" do
+    test "coerces non-string map keys to strings recursively" do
+      sanitized =
+        S3.sanitize_map(%{
+          :status => 200,
+          "nested" => %{404 => "missing", :ref => "abc"},
+          "list" => [%{true => 1}]
+        })
+
+      assert Map.has_key?(sanitized, "status")
+      assert sanitized["nested"] == %{"404" => "missing", "ref" => "abc"}
+      assert sanitized["list"] == [%{"true" => 1}]
+    end
+
+    test "collapses keys that stringify to the same value" do
+      # This is a hazard the sanitizer surfaces rather than hides: if two
+      # keys collapse, the map loses a pair. Documenting it as expected here
+      # means a future callsite that relies on distinct integer/string keys
+      # will have to model that at its own layer.
+      sanitized = S3.sanitize_map(%{1 => "int", "1" => "string"})
+      assert map_size(sanitized) == 1
+    end
+  end
 end
diff --git a/test/pulso/storage/sort_order_test.exs b/test/pulso/storage/sort_order_test.exs
new file mode 100644
index 0000000..764b343
--- /dev/null
+++ b/test/pulso/storage/sort_order_test.exs
@@ -0,0 +1,55 @@
+defmodule Pulso.Storage.SortOrderTest do
+  # SortOrder is the shared tie-breaker for every storage adapter. A client
+  # that swaps adapters must never see the order change when timestamps are
+  # equal, so this coverage runs against the pure sort — no adapter needed.
+
+  use ExUnit.Case, async: true
+
+  alias Pulso.Record.Log
+  alias Pulso.Storage.SortOrder
+
+  defp log(ts, opts \\ []) do
+    %Log{
+      timestamp_ns: ts,
+      observed_timestamp_ns: Keyword.get(opts, :observed),
+      trace_id: Keyword.get(opts, :trace_id),
+      span_id: Keyword.get(opts, :span_id),
+      body: Keyword.get(opts, :body)
+    }
+  end
+
+  test "primary sort is timestamp_ns descending" do
+    result = SortOrder.sort([log(10), log(30), log(20)])
+    assert Enum.map(result, & &1.timestamp_ns) == [30, 20, 10]
+  end
+
+  test "equal timestamp_ns falls through to observed_timestamp_ns descending" do
+    result =
+      SortOrder.sort([
+        log(10, observed: 100),
+        log(10, observed: 300),
+        log(10, observed: 200)
+      ])
+
+    assert Enum.map(result, & &1.observed_timestamp_ns) == [300, 200, 100]
+  end
+
+  test "trace_id and body break further ties deterministically" do
+    a = log(10, observed: 5, trace_id: "aaa", body: "one")
+    b = log(10, observed: 5, trace_id: "aaa", body: "two")
+    c = log(10, observed: 5, trace_id: "bbb", body: "one")
+
+    # The precise order matters less than the fact that repeated sorts of
+    # the same input produce the same output.
+    assert SortOrder.sort([a, b, c]) == SortOrder.sort([c, b, a])
+    assert SortOrder.sort([a, b, c]) == SortOrder.sort([b, a, c])
+  end
+
+  test "nil fields sort last within their position" do
+    with_trace = log(10, observed: 5, trace_id: "aaa")
+    without_trace = log(10, observed: 5, trace_id: nil)
+
+    result = SortOrder.sort([without_trace, with_trace])
+    assert Enum.map(result, & &1.trace_id) == ["aaa", nil]
+  end
+end
diff --git a/test/pulso_web/controllers/otlp_controller_test.exs b/test/pulso_web/controllers/otlp_controller_test.exs
index da6d7c3..815471e 100644
--- a/test/pulso_web/controllers/otlp_controller_test.exs
+++ b/test/pulso_web/controllers/otlp_controller_test.exs
@@ -1,6 +1,7 @@
 defmodule PulsoWeb.OTLPControllerTest do
   use PulsoWeb.ConnCase, async: false
 
+  alias Pulso.Auth.SharedSecret
   alias Pulso.Record.Log
   alias Pulso.Storage
   alias Pulso.Storage.Memory
@@ -66,4 +67,63 @@ defmodule PulsoWeb.OTLPControllerTest do
     assert json_response(conn, 200) == %{}
     assert {:ok, []} = Storage.query("default")
   end
+
+  describe "with Pulso.Auth.SharedSecret enabled" do
+    setup do
+      hex = Base.encode16(:crypto.hash(:sha256, "the-token"), case: :lower)
+
+      Application.put_env(:pulso, Pulso.Auth,
+        module: SharedSecret,
+        tokens: %{"acme" => "sha256$#{hex}"}
+      )
+
+      on_exit(fn -> Application.delete_env(:pulso, Pulso.Auth) end)
+      :ok
+    end
+
+    test "accepts the correct token", %{conn: conn} do
+      conn =
+        conn
+        |> put_req_header("content-type", "application/json")
+        |> put_req_header("x-scope-orgid", "acme")
+        |> put_req_header("authorization", "Bearer the-token")
+        |> post(~p"/v1/logs", payload())
+
+      assert json_response(conn, 200) == %{}
+      assert {:ok, [%Log{service: "api", body: "hello"}]} = Storage.query("acme")
+    end
+
+    test "rejects a request with a bad token", %{conn: conn} do
+      conn =
+        conn
+        |> put_req_header("content-type", "application/json")
+        |> put_req_header("x-scope-orgid", "acme")
+        |> put_req_header("authorization", "Bearer wrong")
+        |> post(~p"/v1/logs", payload())
+
+      assert json_response(conn, 401) == %{"error" => "invalid_token"}
+      assert {:ok, []} = Storage.query("acme")
+    end
+
+    test "rejects a tenant with no configured token", %{conn: conn} do
+      conn =
+        conn
+        |> put_req_header("content-type", "application/json")
+        |> put_req_header("x-scope-orgid", "unknown")
+        |> put_req_header("authorization", "Bearer the-token")
+        |> post(~p"/v1/logs", payload())
+
+      assert json_response(conn, 401) == %{"error" => "unknown_tenant"}
+    end
+
+    test "rejects a request with no authorization header", %{conn: conn} do
+      conn =
+        conn
+        |> put_req_header("content-type", "application/json")
+        |> put_req_header("x-scope-orgid", "acme")
+        |> post(~p"/v1/logs", payload())
+
+      assert json_response(conn, 401) == %{"error" => "missing_token"}
+    end
+  end
 end

From 9fcb019c49d94d01495d005767b89af39f011d9a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pedro=20Pi=C3=B1era=20Buend=C3=ADa?= <pedro@pepicrft.me>
Date: Thu, 24 Sep 2026 06:39:51 +0200
Subject: [PATCH 04/17] Address second-round Codex findings
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Second adversarial pass on top of 5a628e3. Highest-severity findings:

CRITICAL — MCP read path bypassed the auth boundary
- Pulso.MCP.dispatch/2 now takes a context; PulsoWeb.MCPController seeds it
  with {conn: conn} so tools that reference a tenant can call
  Pulso.Auth.verify/2. Pulso.MCP.Tools.call/3 does this for query_logs and
  returns {:error, {:unauthorized, reason}} on failure, which the MCP layer
  surfaces as an errored tool result. Ping, tools/list, and initialize still
  work without a conn (no tenant to authorize).

HIGH — Auth fell open when config was missing
- config/config.exs now sets an explicit default (module: Pulso.Auth.Open)
  for dev/test.
- config/prod.exs sets a sentinel (:must_configure_at_runtime).
- runtime.exs :prod overrides with Pulso.Auth.SharedSecret and its tokens.
- Pulso.Auth.module/0 raises loudly on the sentinel or a missing config —
  a release that skips runtime.exs will not silently accept traffic.

HIGH — Content-hash keys collapsed distinct-but-identical batches
- append/3 now takes opts. When idempotency_key is present, the object
  key is deterministic (SHA-256 of tenant || key, mirroring Stripe /
  RFC 9457 idempotency semantics). When it is absent, the key includes a
  fresh random suffix so two producers with identical bytes never collide.
- PulsoWeb.OTLPController reads `Idempotency-Key` and threads it through.
- The Storage behaviour callback is now append/3; Memory is a no-op on the
  opt.

MEDIUM — Invalid tenant returned 401 under shared-secret auth
- Tenant name is validated in the OTLP controller BEFORE Auth.verify, so a
  `bad/name` tenant returns 400 regardless of the auth outcome.

MEDIUM — sanitize_map silently collapsed colliding keys
- Now returns {:error, {:attribute_key_collision, [key]}} on collision.
  OTLP-produced attributes are unaffected (string keys throughout); a
  hand-built Log with `%{1 => a, "1" => b}` gets a diagnostic instead of a
  dropped value.

MEDIUM — Port suffixes 3 apart could collide
- Ranges widened from 9095/9098 + suffix (overlap for suffixes 3 apart) to
  9000 + suffix / 10000 + suffix (non-overlapping across the whole
  100..999 range). ClickHouse on 9000 stays clear of the API range's
  9100 floor.

LOW — Sort ties conflated nil and empty string
- Pulso.Storage.SortOrder wraps every string tiebreaker in
  `{presence_flag, value}` so nil sorts after every real string, including
  "".

New test coverage: MCP query_logs auth (3 cases), OTLP invalid-tenant 400,
OTLP Idempotency-Key passthrough, S3 idempotency (with vs without key),
sanitize_map collision → error, nil-vs-"" sort distinction, auth
misconfiguration crashes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 config/config.exs                             |   7 +
 config/prod.exs                               |   6 +
 config/runtime.exs                            |   2 +-
 docker-compose.yml                            |   4 +-
 lib/pulso/auth.ex                             |  25 ++-
 lib/pulso/mcp.ex                              |  29 +--
 lib/pulso/mcp/tools.ex                        |  30 +++-
 lib/pulso/storage.ex                          |  15 +-
 lib/pulso/storage/memory.ex                   |   4 +-
 lib/pulso/storage/s3.ex                       | 166 ++++++++++++++----
 lib/pulso/storage/sort_order.ex               |  18 +-
 lib/pulso_web/controllers/mcp_controller.ex   |   6 +-
 lib/pulso_web/controllers/otlp_controller.ex  |  40 ++++-
 mise/utilities/dev_instance_env.sh            |  15 +-
 test/pulso/auth_test.exs                      |  19 +-
 test/pulso/mcp/tools_test.exs                 |  46 +++++
 test/pulso/storage/s3_test.exs                |  29 ++-
 test/pulso/storage/s3_unit_test.exs           |  79 +++++----
 test/pulso/storage/sort_order_test.exs        |  11 ++
 .../controllers/otlp_controller_test.exs      |  33 +++-
 20 files changed, 463 insertions(+), 121 deletions(-)

diff --git a/config/config.exs b/config/config.exs
index 29688eb..2c33632 100644
--- a/config/config.exs
+++ b/config/config.exs
@@ -7,6 +7,8 @@
 # General application configuration
 import Config
 
+alias Pulso.Auth.Open
+
 # Configure Elixir's Logger
 config :logger, :default_formatter,
   format: "$time $metadata[$level] $message\n",
@@ -15,6 +17,11 @@ config :logger, :default_formatter,
 # Use Jason for JSON parsing in Phoenix
 config :phoenix, :json_library, Jason
 
+# Explicit auth default. Environment-specific configs override; prod requires
+# a runtime override to `Pulso.Auth.SharedSecret` via runtime.exs — an unset
+# release still raises rather than falling back to open access.
+config :pulso, Pulso.Auth, module: Open
+
 # Configure the endpoint
 config :pulso, PulsoWeb.Endpoint,
   url: [host: "localhost"],
diff --git a/config/prod.exs b/config/prod.exs
index 30393dd..385c668 100644
--- a/config/prod.exs
+++ b/config/prod.exs
@@ -3,6 +3,12 @@ import Config
 # Do not print debug messages in production
 config :logger, level: :info
 
+# A sentinel that runtime.exs is expected to overwrite. If a release starts
+# without runtime.exs having populated the real auth module, `Pulso.Auth`
+# raises rather than serving requests with the Open (accept-everything)
+# fallback that ships in config.exs.
+config :pulso, Pulso.Auth, module: :must_configure_at_runtime
+
 config :pulso, PulsoWeb.Endpoint,
   force_ssl: [
     rewrite_on: [:x_forwarded_proto],
diff --git a/config/runtime.exs b/config/runtime.exs
index 1f6e312..8ac3d75 100644
--- a/config/runtime.exs
+++ b/config/runtime.exs
@@ -38,7 +38,7 @@ case config_env() do
       # mise/utilities/dev_instance_env.sh sets PULSO_S3_ENDPOINT per worktree.
       # The fallback matches the docker-compose default host port when mise
       # is not in the loop.
-      endpoint: System.get_env("PULSO_S3_ENDPOINT", "http://localhost:9195"),
+      endpoint: System.get_env("PULSO_S3_ENDPOINT", "http://localhost:9100"),
       region: System.get_env("PULSO_S3_REGION", "us-east-1"),
       access_key_id: System.get_env("PULSO_S3_ACCESS_KEY_ID", "rustfsadmin"),
       secret_access_key: System.get_env("PULSO_S3_SECRET_ACCESS_KEY", "rustfsadmin"),
diff --git a/docker-compose.yml b/docker-compose.yml
index 0c1b99d..49605c4 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -27,8 +27,8 @@ services:
     # from another host on the LAN. Host ports come from the per-worktree env
     # to keep multiple checkouts from fighting over 9000/9001.
     ports:
-      - "127.0.0.1:${PULSO_RUSTFS_API_PORT:-9195}:9000"
-      - "127.0.0.1:${PULSO_RUSTFS_CONSOLE_PORT:-9198}:9001"
+      - "127.0.0.1:${PULSO_RUSTFS_API_PORT:-9100}:9000"
+      - "127.0.0.1:${PULSO_RUSTFS_CONSOLE_PORT:-10100}:9001"
     volumes:
       - rustfs-data:/data
 
diff --git a/lib/pulso/auth.ex b/lib/pulso/auth.ex
index 43c3bd8..c568de4 100644
--- a/lib/pulso/auth.ex
+++ b/lib/pulso/auth.ex
@@ -20,7 +20,6 @@ defmodule Pulso.Auth do
   """
 
   alias Plug.Conn
-  alias Pulso.Auth.Open
 
   @type reason :: :missing_token | :invalid_token | :unknown_tenant | term()
 
@@ -34,8 +33,28 @@ defmodule Pulso.Auth do
   @spec module() :: module()
   def module do
     case Application.get_env(:pulso, __MODULE__) do
-      nil -> Open
-      env -> Keyword.get(env, :module, Open)
+      nil ->
+        raise """
+        Pulso.Auth is not configured. This should never happen — config/config.exs
+        sets `Pulso.Auth.Open` as the compile-time default. Refusing to accept
+        traffic rather than fail open.
+        """
+
+      env ->
+        case Keyword.get(env, :module) do
+          nil ->
+            raise "Pulso.Auth :module key is not set. See config/config.exs."
+
+          :must_configure_at_runtime ->
+            raise """
+            Pulso.Auth is still set to the prod sentinel `:must_configure_at_runtime`.
+            runtime.exs must set `config :pulso, Pulso.Auth, module: Pulso.Auth.SharedSecret, tokens: %{...}`
+            before the app accepts traffic.
+            """
+
+          mod when is_atom(mod) ->
+            mod
+        end
     end
   end
 end
diff --git a/lib/pulso/mcp.ex b/lib/pulso/mcp.ex
index e24890f..83b97e5 100644
--- a/lib/pulso/mcp.ex
+++ b/lib/pulso/mcp.ex
@@ -13,27 +13,36 @@ defmodule Pulso.MCP do
   @protocol_version "2025-06-18"
   @server_info %{"name" => "pulso", "version" => "0.1.0"}
 
+  @type context :: %{optional(:conn) => Plug.Conn.t()}
+
   @doc """
   Dispatch a single JSON-RPC message.
 
+  `context` carries per-request state that individual tools need to run
+  authorization or other checks. Today only `:conn` is populated (from
+  `PulsoWeb.MCPController`); the shape is intentionally open for future
+  fields (tenant hint, feature flags).
+
   Returns `{:reply, response}` for requests and `:noreply` for notifications
   (messages without an `id`).
   """
-  @spec dispatch(map()) :: {:reply, map()} | :noreply
-  def dispatch(%{"method" => method} = msg) do
+  @spec dispatch(map(), context()) :: {:reply, map()} | :noreply
+  def dispatch(msg, context \\ %{})
+
+  def dispatch(%{"method" => method} = msg, context) when is_map(context) do
     id = Map.get(msg, "id")
     params = Map.get(msg, "params", %{})
 
-    case {id, handle(method, params)} do
+    case {id, handle(method, params, context)} do
       {nil, _} -> :noreply
       {id, {:ok, result}} -> {:reply, ok(id, result)}
       {id, {:error, code, message}} -> {:reply, error(id, code, message)}
     end
   end
 
-  def dispatch(_), do: {:reply, error(nil, -32_600, "Invalid Request")}
+  def dispatch(_, _), do: {:reply, error(nil, -32_600, "Invalid Request")}
 
-  defp handle("initialize", _params) do
+  defp handle("initialize", _params, _context) do
     {:ok,
      %{
        "protocolVersion" => @protocol_version,
@@ -42,14 +51,14 @@ defmodule Pulso.MCP do
      }}
   end
 
-  defp handle("tools/list", _params) do
+  defp handle("tools/list", _params, _context) do
     {:ok, %{"tools" => Tools.list()}}
   end
 
-  defp handle("tools/call", %{"name" => name} = params) do
+  defp handle("tools/call", %{"name" => name} = params, context) do
     arguments = Map.get(params, "arguments", %{})
 
-    case Tools.call(name, arguments) do
+    case Tools.call(name, arguments, context) do
       {:ok, content} ->
         {:ok, %{"content" => content, "isError" => false}}
 
@@ -62,8 +71,8 @@ defmodule Pulso.MCP do
     end
   end
 
-  defp handle("ping", _params), do: {:ok, %{}}
-  defp handle(_unknown, _params), do: {:error, -32_601, "Method not found"}
+  defp handle("ping", _params, _context), do: {:ok, %{}}
+  defp handle(_unknown, _params, _context), do: {:error, -32_601, "Method not found"}
 
   defp ok(id, result), do: %{"jsonrpc" => "2.0", "id" => id, "result" => result}
 
diff --git a/lib/pulso/mcp/tools.ex b/lib/pulso/mcp/tools.ex
index ef0b659..2cfd649 100644
--- a/lib/pulso/mcp/tools.ex
+++ b/lib/pulso/mcp/tools.ex
@@ -7,6 +7,7 @@ defmodule Pulso.MCP.Tools do
   `docs/architecture.md`.
   """
 
+  alias Pulso.Auth
   alias Pulso.Record.Log
   alias Pulso.Storage
 
@@ -43,8 +44,10 @@ defmodule Pulso.MCP.Tools do
   @spec list() :: [map()]
   def list, do: @tools
 
-  @spec call(String.t(), map()) :: {:ok, [map()]} | {:error, term()}
-  def call("query_logs", %{"tenant" => tenant} = args) when is_binary(tenant) do
+  @spec call(String.t(), map(), Pulso.MCP.context()) :: {:ok, [map()]} | {:error, term()}
+  def call(name, args, context \\ %{})
+
+  def call("query_logs", %{"tenant" => tenant} = args, context) when is_binary(tenant) do
     opts =
       []
       |> put_opt(:start_ts, args["start_ts_ns"])
@@ -52,13 +55,30 @@ defmodule Pulso.MCP.Tools do
       |> put_opt(:limit, args["limit"])
       |> put_opt(:service, args["service"])
 
-    with {:ok, records} <- Storage.query(tenant, opts) do
+    with :ok <- verify(context, tenant),
+         {:ok, records} <- Storage.query(tenant, opts) do
       {:ok, [%{"type" => "text", "text" => Jason.encode!(Enum.map(records, &encode_record/1))}]}
     end
   end
 
-  def call("query_logs", _args), do: {:error, {:invalid_arguments, "tenant is required"}}
-  def call(name, _args), do: {:error, {:unknown_tool, name}}
+  def call("query_logs", _args, _context), do: {:error, {:invalid_arguments, "tenant is required"}}
+  def call(name, _args, _context), do: {:error, {:unknown_tool, name}}
+
+  # Every tool that names a tenant runs it through `Pulso.Auth.verify/2`. The
+  # MCP dispatch is the same JSON-RPC transport for read and write; without
+  # this hop the ingest boundary's auth check would be bypassable via the
+  # read path.
+  defp verify(%{conn: conn}, tenant) when not is_nil(conn) do
+    case Auth.verify(conn, tenant) do
+      :ok -> :ok
+      {:error, reason} -> {:error, {:unauthorized, reason}}
+    end
+  end
+
+  # No conn (e.g. an in-process caller writing a test): keep the default-open
+  # behavior so unit tests do not need to build a fake connection. Callers
+  # that reach the network path always have a conn.
+  defp verify(_context, _tenant), do: :ok
 
   defp put_opt(opts, _key, nil), do: opts
   defp put_opt(opts, key, value), do: Keyword.put(opts, key, value)
diff --git a/lib/pulso/storage.ex b/lib/pulso/storage.ex
index af6134e..8f8a5f8 100644
--- a/lib/pulso/storage.ex
+++ b/lib/pulso/storage.ex
@@ -14,6 +14,15 @@ defmodule Pulso.Storage do
   alias Pulso.Storage.Memory
 
   @type tenant :: String.t()
+  @type append_opts :: [
+          # Opt-in idempotency: two `append` calls with the same tenant and
+          # the same idempotency_key resolve to the same underlying object
+          # so a retry does not duplicate. Callers who want distinct writes
+          # for identical payloads (e.g. two producers with genuinely
+          # different events that happen to serialize the same) simply
+          # omit the key.
+          {:idempotency_key, String.t()}
+        ]
   @type query_opts :: [
           {:start_ts, non_neg_integer()}
           | {:end_ts, non_neg_integer()}
@@ -21,11 +30,11 @@ defmodule Pulso.Storage do
           | {:service, String.t()}
         ]
 
-  @callback append(tenant, [Log.t()]) :: :ok | {:error, term()}
+  @callback append(tenant, [Log.t()], append_opts) :: :ok | {:error, term()}
   @callback query(tenant, query_opts) :: {:ok, [Log.t()]} | {:error, term()}
 
-  @spec append(tenant, [Log.t()]) :: :ok | {:error, term()}
-  def append(tenant, records), do: adapter().append(tenant, records)
+  @spec append(tenant, [Log.t()], append_opts) :: :ok | {:error, term()}
+  def append(tenant, records, opts \\ []), do: adapter().append(tenant, records, opts)
 
   @spec query(tenant, query_opts) :: {:ok, [Log.t()]} | {:error, term()}
   def query(tenant, opts \\ []), do: adapter().query(tenant, opts)
diff --git a/lib/pulso/storage/memory.ex b/lib/pulso/storage/memory.ex
index cad7277..d389f70 100644
--- a/lib/pulso/storage/memory.ex
+++ b/lib/pulso/storage/memory.ex
@@ -28,7 +28,9 @@ defmodule Pulso.Storage.Memory do
   end
 
   @impl Pulso.Storage
-  def append(tenant, records) when is_binary(tenant) and is_list(records) do
+  def append(tenant, records, _opts \\ []) when is_binary(tenant) and is_list(records) do
+    # Memory ignores :idempotency_key — it's a test adapter, and repeat
+    # tests reset state between cases anyway.
     now = System.system_time(:nanosecond)
 
     normalized =
diff --git a/lib/pulso/storage/s3.ex b/lib/pulso/storage/s3.ex
index 7064f80..7e57234 100644
--- a/lib/pulso/storage/s3.ex
+++ b/lib/pulso/storage/s3.ex
@@ -2,21 +2,30 @@ defmodule Pulso.Storage.S3 do
   @moduledoc """
   S3-backed log storage. Step 2 adapter.
 
-  Each `append/2` writes one NDJSON object under a tenant-scoped prefix
-  (`tenants/<tenant>/logs/<sort_ns>-<content_hash>.ndjson`). Tenant isolation
-  is enforced by key construction: tenant names are validated against a
-  conservative charset so a batch cannot land outside its own prefix. The
-  key's suffix is a truncated SHA-256 of the encoded payload, which makes
-  identical retries land on the same object — a client that PUTs the same
-  batch twice under a lost-response retry does not create a duplicate.
+  Each `append/3` writes one NDJSON object under a tenant-scoped prefix
+  (`tenants/<tenant>/logs/<sort_ns>-<suffix>.ndjson`). Tenant isolation is
+  enforced by key construction: tenant names are validated against a
+  conservative charset so a batch cannot land outside its own prefix.
+
+  The key suffix depends on whether the caller supplied an
+  `idempotency_key`:
+
+    * **With `idempotency_key`** — the suffix is a deterministic hash of
+      `tenant || idempotency_key`. Two `append` calls with the same key
+      resolve to the same object, so a lost-response retry does not
+      duplicate. Two producers that pick the same idempotency key are
+      explicitly claiming "these are the same write" — semantics that
+      match Stripe's `Idempotency-Key` and RFC 9457.
+    * **Without `idempotency_key`** — the suffix mixes a truncated content
+      hash with random bytes. Distinct calls always produce distinct
+      objects (no accidental collapse of two identical-content batches).
+      Retries in this mode duplicate — callers who need dedup must opt in.
 
   `query/2` lists the tenant prefix, downloads every object, decodes NDJSON,
-  filters, and sorts. Order matches `Pulso.Storage.Memory`: `timestamp_ns`
-  descending, then `observed_timestamp_ns` descending, then `trace_id`, then
-  `body`, so equal-timestamp ties resolve identically across adapters.
-  A `NotFound` for a key that was listed but disappeared before the fetch
-  (concurrent retention, compaction, another process deleting) is skipped
-  rather than aborting the query.
+  filters, and sorts. Order is shared with `Pulso.Storage.Memory` via
+  `Pulso.Storage.SortOrder`. A `NotFound` for a key that was listed but
+  disappeared before the fetch (concurrent retention, compaction, another
+  process deleting) is skipped rather than aborting the query.
 
   Known limits, deferred to step 3 (segments + manifest):
 
@@ -44,17 +53,19 @@ defmodule Pulso.Storage.S3 do
   @content_hash_width 16
 
   @impl Pulso.Storage
-  def append(tenant, []) when is_binary(tenant) do
+  def append(tenant, records, opts \\ [])
+
+  def append(tenant, [], _opts) when is_binary(tenant) do
     # Validate even on empty so an adversarial tenant name is rejected on the
     # first attempt, not only once a real record survives OTLP decoding.
     validate_tenant(tenant)
   end
 
-  def append(tenant, records) when is_binary(tenant) and is_list(records) do
+  def append(tenant, records, opts) when is_binary(tenant) and is_list(records) do
     with :ok <- validate_tenant(tenant),
-         normalized = normalize(records),
+         {:ok, normalized} <- normalize(records),
          {:ok, payload} <- encode(normalized) do
-      key = object_key(tenant, batch_sort_ns(normalized), payload)
+      key = object_key(tenant, batch_sort_ns(normalized), payload, Keyword.get(opts, :idempotency_key))
       ObjectStore.put(config!(), key, payload)
     end
   end
@@ -89,26 +100,83 @@ defmodule Pulso.Storage.S3 do
   defp normalize(records) do
     now = System.system_time(:nanosecond)
 
-    for %Log{} = record <- records do
-      %{
-        record
-        | observed_timestamp_ns: record.observed_timestamp_ns || now,
-          attributes: sanitize_map(record.attributes || %{}),
-          resource: sanitize_map(record.resource || %{})
-      }
+    Enum.reduce_while(records, {:ok, []}, fn
+      %Log{} = record, {:ok, acc} ->
+        with {:ok, attrs} <- sanitize_map(record.attributes || %{}),
+             {:ok, resource} <- sanitize_map(record.resource || %{}) do
+          normalized = %{
+            record
+            | observed_timestamp_ns: record.observed_timestamp_ns || now,
+              attributes: attrs,
+              resource: resource
+          }
+
+          {:cont, {:ok, [normalized | acc]}}
+        else
+          err -> {:halt, err}
+        end
+    end)
+    |> case do
+      {:ok, records} -> {:ok, Enum.reverse(records)}
+      err -> err
     end
   end
 
-  # Force every attribute/resource map key to be a string and drop nils. OTLP
+  # Coerce every attribute/resource map key to a string, recursively. OTLP
   # decoding already produces string keys, but a caller building `%Log{}`
   # directly (or a future backend surface) could hand us atoms or integers.
-  # Encoding those with Jason coerces them to strings, so two logical keys
-  # can silently collapse on the round trip. Doing the coercion here makes
-  # the write path deterministic and the decode path lossless.
+  # If two logical keys coerce to the same string (`%{1 => a, "1" => b}`) we
+  # refuse — silently dropping either value would surprise a reader looking
+  # at either the original struct or the JSON-encoded record.
   @doc false
-  @spec sanitize_map(map()) :: map()
+  @spec sanitize_map(map()) :: {:ok, map()} | {:error, {:attribute_key_collision, [String.t()]}}
   def sanitize_map(map) when is_map(map) do
-    Map.new(map, fn {k, v} -> {stringify_key(k), sanitize_value(v)} end)
+    Enum.reduce_while(map, {:ok, %{}}, fn {k, v}, {:ok, acc} ->
+      string_key = stringify_key(k)
+
+      cond do
+        Map.has_key?(acc, string_key) ->
+          {:halt, {:error, {:attribute_key_collision, [string_key]}}}
+
+        is_map(v) ->
+          case sanitize_map(v) do
+            {:ok, sanitized} -> {:cont, {:ok, Map.put(acc, string_key, sanitized)}}
+            err -> {:halt, err}
+          end
+
+        is_list(v) ->
+          case sanitize_list(v) do
+            {:ok, sanitized} -> {:cont, {:ok, Map.put(acc, string_key, sanitized)}}
+            err -> {:halt, err}
+          end
+
+        true ->
+          {:cont, {:ok, Map.put(acc, string_key, v)}}
+      end
+    end)
+  end
+
+  defp sanitize_list(list) do
+    Enum.reduce_while(list, {:ok, []}, fn
+      v, {:ok, acc} when is_map(v) ->
+        case sanitize_map(v) do
+          {:ok, sanitized} -> {:cont, {:ok, [sanitized | acc]}}
+          err -> {:halt, err}
+        end
+
+      v, {:ok, acc} when is_list(v) ->
+        case sanitize_list(v) do
+          {:ok, sanitized} -> {:cont, {:ok, [sanitized | acc]}}
+          err -> {:halt, err}
+        end
+
+      v, {:ok, acc} ->
+        {:cont, {:ok, [v | acc]}}
+    end)
+    |> case do
+      {:ok, sanitized} -> {:ok, Enum.reverse(sanitized)}
+      err -> err
+    end
   end
 
   defp stringify_key(k) when is_binary(k), do: k
@@ -116,10 +184,6 @@ defmodule Pulso.Storage.S3 do
   defp stringify_key(k) when is_integer(k), do: Integer.to_string(k)
   defp stringify_key(k), do: inspect(k)
 
-  defp sanitize_value(v) when is_map(v), do: sanitize_map(v)
-  defp sanitize_value(v) when is_list(v), do: Enum.map(v, &sanitize_value/1)
-  defp sanitize_value(v), do: v
-
   defp batch_sort_ns(records) do
     # Pick the smallest observed_timestamp_ns so identical retries hash into
     # the same object key. Every record is normalized, so this is never nil.
@@ -186,9 +250,26 @@ defmodule Pulso.Storage.S3 do
   defp prefix(tenant), do: "tenants/#{tenant}/logs/"
 
   @doc false
-  @spec object_key(String.t(), non_neg_integer(), binary()) :: String.t()
-  def object_key(tenant, sort_ns, payload) when is_binary(tenant) and is_binary(payload) do
-    "#{prefix(tenant)}#{zero_pad(sort_ns)}-#{content_hash(payload)}.ndjson"
+  @spec object_key(String.t(), non_neg_integer(), binary(), String.t() | nil) :: String.t()
+  def object_key(tenant, sort_ns, payload, idempotency_key) when is_binary(tenant) and is_binary(payload) do
+    suffix =
+      case idempotency_key do
+        # Opt-in idempotency: a retry of the same batch under the same key
+        # deterministically lands on the same object, so the second PUT is a
+        # no-op overwrite. Distinct producers that pick the same key are
+        # explicitly claiming "these two calls are the same write".
+        <<key::binary>> when byte_size(key) > 0 ->
+          "idem-" <> stable_hash(tenant <> "\0" <> key)
+
+        # No idempotency key: the caller is fine with a retry producing a
+        # duplicate. A random suffix guarantees distinct writes even when
+        # payload and timestamp collide. `content_hash` is still mixed in
+        # so a corrupted binary at least sorts predictably.
+        _ ->
+          "rand-" <> content_hash(payload) <> "-" <> rand_hex()
+      end
+
+    "#{prefix(tenant)}#{zero_pad(sort_ns)}-#{suffix}.ndjson"
   end
 
   defp zero_pad(ns) when is_integer(ns) and ns >= 0 do
@@ -204,6 +285,17 @@ defmodule Pulso.Storage.S3 do
     |> binary_part(0, @content_hash_width)
   end
 
+  defp stable_hash(bin) do
+    :sha256
+    |> :crypto.hash(bin)
+    |> Base.encode16(case: :lower)
+    |> binary_part(0, @content_hash_width)
+  end
+
+  defp rand_hex do
+    :crypto.strong_rand_bytes(8) |> Base.encode16(case: :lower)
+  end
+
   defp filter_by_time(records, nil, nil), do: records
 
   defp filter_by_time(records, start_ts, end_ts) do
diff --git a/lib/pulso/storage/sort_order.ex b/lib/pulso/storage/sort_order.ex
index adf31ad..bccb410 100644
--- a/lib/pulso/storage/sort_order.ex
+++ b/lib/pulso/storage/sort_order.ex
@@ -16,18 +16,24 @@ defmodule Pulso.Storage.SortOrder do
     Enum.sort_by(records, &sort_key/1, &compare_desc/2)
   end
 
+  # Each string-shaped tiebreaker becomes `{presence_flag, value}` so a nil
+  # sorts *after* every real string in descending order, and — crucially —
+  # never collapses with an empty string. `1` for present, `0` for nil:
+  # descending order places `1 > 0` first, so real strings win the tie and
+  # a nil-vs-"" comparison sees a real difference in the first element of
+  # the pair.
   defp sort_key(%Log{} = r) do
     {
       r.timestamp_ns || 0,
       r.observed_timestamp_ns || 0,
-      # Strings compare lexicographically. Descending sort is what the caller
-      # asked for, so we invert the two lower-priority string tiebreakers by
-      # negating the comparison in `compare_desc/2`.
-      r.trace_id || "",
-      r.span_id || "",
-      r.body || ""
+      presence_pair(r.trace_id),
+      presence_pair(r.span_id),
+      presence_pair(r.body)
     }
   end
 
+  defp presence_pair(nil), do: {0, ""}
+  defp presence_pair(value) when is_binary(value), do: {1, value}
+
   defp compare_desc(a, b), do: a >= b
 end
diff --git a/lib/pulso_web/controllers/mcp_controller.ex b/lib/pulso_web/controllers/mcp_controller.ex
index ce475fb..63434fc 100644
--- a/lib/pulso_web/controllers/mcp_controller.ex
+++ b/lib/pulso_web/controllers/mcp_controller.ex
@@ -2,13 +2,15 @@ defmodule PulsoWeb.MCPController do
   use PulsoWeb, :controller
 
   def rpc(conn, params) when is_map(params) do
-    respond(conn, Pulso.MCP.dispatch(params))
+    respond(conn, Pulso.MCP.dispatch(params, %{conn: conn}))
   end
 
   def rpc(conn, params) when is_list(params) do
+    context = %{conn: conn}
+
     responses =
       params
-      |> Enum.map(&Pulso.MCP.dispatch/1)
+      |> Enum.map(&Pulso.MCP.dispatch(&1, context))
       |> Enum.flat_map(fn
         {:reply, response} -> [response]
         :noreply -> []
diff --git a/lib/pulso_web/controllers/otlp_controller.ex b/lib/pulso_web/controllers/otlp_controller.ex
index ea76607..025b518 100644
--- a/lib/pulso_web/controllers/otlp_controller.ex
+++ b/lib/pulso_web/controllers/otlp_controller.ex
@@ -18,23 +18,42 @@ defmodule PulsoWeb.OTLPController do
   On success, the response body is the empty `ExportLogsServiceResponse`
   object per the OTLP spec.
   """
+  # A conservative tenant charset; mirrors `Pulso.Storage.S3`. Validating
+  # here (before auth) is what makes a `bad/name` tenant come back as 400
+  # rather than 401 or 500 — the storage layer would still reject it, but
+  # by then the request has already spent an auth check on a value we know
+  # is invalid.
+  @tenant_regex ~r/\A[A-Za-z0-9_.\-]{1,128}\z/
+
   def logs(conn, params) do
     tenant = tenant_from(conn)
+    opts = append_opts(conn)
 
-    with :ok <- Auth.verify(conn, tenant),
+    with :ok <- validate_tenant(tenant),
+         :ok <- Auth.verify(conn, tenant),
          records = Logs.decode(params),
-         :ok <- Storage.append(tenant, records) do
+         :ok <- Storage.append(tenant, records, opts) do
       json(conn, %{})
     else
+      {:error, {:invalid_tenant, _}} ->
+        conn
+        |> put_status(:bad_request)
+        |> json(%{error: "invalid_tenant"})
+
       {:error, reason} when reason in [:missing_token, :invalid_token, :unknown_tenant] ->
         conn
         |> put_status(:unauthorized)
         |> json(%{error: to_string(reason)})
 
-      {:error, {:invalid_tenant, _} = reason} ->
+      {:error, {:encode_failed, _}} ->
         conn
         |> put_status(:bad_request)
-        |> json(%{error: inspect(reason)})
+        |> json(%{error: "encode_failed"})
+
+      {:error, {:attribute_key_collision, _}} ->
+        conn
+        |> put_status(:bad_request)
+        |> json(%{error: "attribute_key_collision"})
 
       {:error, reason} ->
         conn
@@ -43,10 +62,23 @@ defmodule PulsoWeb.OTLPController do
     end
   end
 
+  defp validate_tenant(tenant) do
+    if Regex.match?(@tenant_regex, tenant),
+      do: :ok,
+      else: {:error, {:invalid_tenant, tenant}}
+  end
+
   defp tenant_from(conn) do
     case Plug.Conn.get_req_header(conn, "x-scope-orgid") do
       [tenant | _] when is_binary(tenant) and tenant != "" -> tenant
       _ -> @default_tenant
     end
   end
+
+  defp append_opts(conn) do
+    case Plug.Conn.get_req_header(conn, "idempotency-key") do
+      [key | _] when is_binary(key) and byte_size(key) > 0 -> [idempotency_key: key]
+      _ -> []
+    end
+  end
 end
diff --git a/mise/utilities/dev_instance_env.sh b/mise/utilities/dev_instance_env.sh
index 72c9000..21f7285 100644
--- a/mise/utilities/dev_instance_env.sh
+++ b/mise/utilities/dev_instance_env.sh
@@ -134,14 +134,17 @@ suffix="$(ensure_suffix)"
 export PULSO_DEV_INSTANCE="${suffix}"
 export PULSO_DEV_INSTANCE_ROOT="${PROJECT_ROOT}"
 
-# Phoenix endpoint port. Base 4000 + suffix keeps the range inside 4100..4999,
-# well clear of the default Phoenix dev port (4000) and the ExUnit port (4002).
+# Phoenix endpoint. 4100..4999 — clear of the default Phoenix dev port (4000)
+# and the ExUnit port (4002).
 export PORT="$((4000 + suffix))"
 
-# RustFS via docker-compose. Base 9095/9098 mirrors the tuist convention so a
-# host running both projects side by side does not collide. Range: 9195..10097.
-export PULSO_RUSTFS_API_PORT="$((9095 + suffix))"
-export PULSO_RUSTFS_CONSOLE_PORT="$((9098 + suffix))"
+# RustFS via docker-compose. The API and console ports use base ports
+# 1000 apart so two suffixes N and N+3 (tuist's convention had this bug)
+# cannot accidentally claim the same TCP port. Ranges: 9100..9999 for the
+# S3 API, 10100..10999 for the console. 9000 itself is left free so a
+# ClickHouse install on 9000 keeps working.
+export PULSO_RUSTFS_API_PORT="$((9000 + suffix))"
+export PULSO_RUSTFS_CONSOLE_PORT="$((10000 + suffix))"
 
 # What the Elixir app reads. runtime.exs reads PULSO_S3_ENDPOINT verbatim.
 export PULSO_S3_ENDPOINT="http://localhost:${PULSO_RUSTFS_API_PORT}"
diff --git a/test/pulso/auth_test.exs b/test/pulso/auth_test.exs
index 3681ecb..33aa470 100644
--- a/test/pulso/auth_test.exs
+++ b/test/pulso/auth_test.exs
@@ -6,7 +6,10 @@ defmodule Pulso.AuthTest do
   alias Pulso.Auth.SharedSecret
 
   setup do
-    on_exit(fn -> Application.delete_env(:pulso, Auth) end)
+    # Restore the compile-time default (module: Pulso.Auth.Open, set in
+    # config/config.exs) after each test so we do not poison subsequent
+    # tests that rely on the default.
+    on_exit(fn -> Application.put_env(:pulso, Auth, module: Open) end)
     :ok
   end
 
@@ -17,9 +20,19 @@ defmodule Pulso.AuthTest do
   end
 
   describe "Pulso.Auth.module/0" do
-    test "falls back to Pulso.Auth.Open when nothing is configured" do
+    test "raises when nothing is configured — refuses to silently fail open" do
       Application.delete_env(:pulso, Auth)
-      assert Auth.module() == Open
+      # config.exs sets an explicit default. Reaching this branch means
+      # someone deleted it; the correct response is a loud crash, not a
+      # quiet accept-everything.
+      assert_raise RuntimeError, ~r/Pulso.Auth is not configured/, fn ->
+        Auth.module()
+      end
+    end
+
+    test "raises when the prod sentinel is still in place" do
+      Application.put_env(:pulso, Auth, module: :must_configure_at_runtime)
+      assert_raise RuntimeError, ~r/must_configure_at_runtime/, fn -> Auth.module() end
     end
 
     test "returns the configured module" do
diff --git a/test/pulso/mcp/tools_test.exs b/test/pulso/mcp/tools_test.exs
index 930debc..c718b6e 100644
--- a/test/pulso/mcp/tools_test.exs
+++ b/test/pulso/mcp/tools_test.exs
@@ -1,6 +1,8 @@
 defmodule Pulso.MCP.ToolsTest do
   use ExUnit.Case, async: false
 
+  alias Pulso.Auth.Open
+  alias Pulso.Auth.SharedSecret
   alias Pulso.MCP.Tools
   alias Pulso.Record.Log
   alias Pulso.Storage
@@ -45,4 +47,48 @@ defmodule Pulso.MCP.ToolsTest do
   test "query_logs errors when tenant is missing" do
     assert {:error, {:invalid_arguments, _}} = Tools.call("query_logs", %{})
   end
+
+  describe "auth on the read path" do
+    setup do
+      hex = Base.encode16(:crypto.hash(:sha256, "the-token"), case: :lower)
+
+      Application.put_env(:pulso, Pulso.Auth,
+        module: SharedSecret,
+        tokens: %{"acme" => "sha256$#{hex}"}
+      )
+
+      on_exit(fn ->
+        Application.put_env(:pulso, Pulso.Auth, module: Open)
+      end)
+
+      :ok
+    end
+
+    test "rejects a query_logs call with no bearer token" do
+      Storage.append("acme", [%Log{timestamp_ns: 1}])
+
+      assert {:error, {:unauthorized, :missing_token}} =
+               Tools.call("query_logs", %{"tenant" => "acme"}, %{conn: %Plug.Conn{}})
+    end
+
+    test "rejects a query_logs call with a bad token" do
+      Storage.append("acme", [%Log{timestamp_ns: 1}])
+
+      conn = %Plug.Conn{} |> Plug.Conn.put_req_header("authorization", "Bearer wrong")
+
+      assert {:error, {:unauthorized, :invalid_token}} =
+               Tools.call("query_logs", %{"tenant" => "acme"}, %{conn: conn})
+    end
+
+    test "accepts a query_logs call with the correct token" do
+      Storage.append("acme", [%Log{timestamp_ns: 1, body: "ok"}])
+
+      conn = %Plug.Conn{} |> Plug.Conn.put_req_header("authorization", "Bearer the-token")
+
+      assert {:ok, [%{"text" => text}]} =
+               Tools.call("query_logs", %{"tenant" => "acme"}, %{conn: conn})
+
+      assert [%{"body" => "ok"}] = Jason.decode!(text)
+    end
+  end
 end
diff --git a/test/pulso/storage/s3_test.exs b/test/pulso/storage/s3_test.exs
index 250817c..e99caff 100644
--- a/test/pulso/storage/s3_test.exs
+++ b/test/pulso/storage/s3_test.exs
@@ -126,16 +126,18 @@ defmodule Pulso.Storage.S3Test do
     end
   end
 
-  test "identical batches are idempotent — retrying does not duplicate", %{
+  test "same idempotency_key deduplicates identical retries", %{
     tenant: tenant,
     config: config
   } do
-    # Content-addressed object keys mean the same batch written twice lands
-    # on the same key. This is the retry story for step 2.
-    payload = [record(1, body: "same", service: "api")]
+    # Opt-in idempotency: a caller that wants retry safety passes the same
+    # idempotency_key on the retry. The object key becomes deterministic,
+    # so the second PUT overwrites the first with identical content and no
+    # duplicate appears at query time.
+    batch = [record(1, body: "same", service: "api")]
 
-    assert :ok = S3.append(tenant, payload)
-    assert :ok = S3.append(tenant, payload)
+    assert :ok = S3.append(tenant, batch, idempotency_key: "req-1")
+    assert :ok = S3.append(tenant, batch, idempotency_key: "req-1")
 
     assert {:ok, keys} = ObjectStore.list(config, "tenants/#{tenant}/logs/")
     assert length(keys) == 1
@@ -144,6 +146,21 @@ defmodule Pulso.Storage.S3Test do
     assert length(records) == 1
   end
 
+  test "without an idempotency_key identical batches remain distinct writes", %{
+    tenant: tenant,
+    config: config
+  } do
+    # If a caller doesn't opt in, two producers with byte-identical payloads
+    # must not silently collapse — that would drop data.
+    batch = [record(1, body: "same", service: "api")]
+
+    assert :ok = S3.append(tenant, batch)
+    assert :ok = S3.append(tenant, batch)
+
+    assert {:ok, keys} = ObjectStore.list(config, "tenants/#{tenant}/logs/")
+    assert length(keys) == 2
+  end
+
   test "a key deleted after listing does not fail the query", %{
     tenant: tenant,
     config: config
diff --git a/test/pulso/storage/s3_unit_test.exs b/test/pulso/storage/s3_unit_test.exs
index f297fca..8cea36b 100644
--- a/test/pulso/storage/s3_unit_test.exs
+++ b/test/pulso/storage/s3_unit_test.exs
@@ -38,61 +38,78 @@ defmodule Pulso.Storage.S3UnitTest do
     end
   end
 
-  describe "content-addressed object keys" do
-    test "the same payload maps to the same key so retries do not duplicate" do
-      # Two callers PUT the same batch; if the response of the first is lost
-      # and the second retries, the deterministic key means both PUTs land on
-      # the same object and the second is a no-op overwrite.
-      k1 = S3.object_key("acme", 100, "same-batch")
-      k2 = S3.object_key("acme", 100, "same-batch")
+  describe "object key construction" do
+    test "with an idempotency key, the same call maps to the same key" do
+      k1 = S3.object_key("acme", 100, "payload", "req-1")
+      k2 = S3.object_key("acme", 100, "payload", "req-1")
       assert k1 == k2
     end
 
-    test "different payloads map to different keys" do
-      k1 = S3.object_key("acme", 100, "batch-a")
-      k2 = S3.object_key("acme", 100, "batch-b")
+    test "with an idempotency key, different keys map to different objects" do
+      k1 = S3.object_key("acme", 100, "payload", "req-1")
+      k2 = S3.object_key("acme", 100, "payload", "req-2")
       refute k1 == k2
     end
 
+    test "without an idempotency key, identical payloads still map to distinct objects" do
+      # This is the "two legitimate producers happen to have identical bytes"
+      # case Codex flagged in review 2: content-addressing alone would
+      # silently drop one. With no idempotency key we always take a random
+      # suffix so distinct calls always land on distinct objects.
+      k1 = S3.object_key("acme", 100, "payload", nil)
+      k2 = S3.object_key("acme", 100, "payload", nil)
+      refute k1 == k2
+    end
+
+    test "an empty-string idempotency key is treated as absent" do
+      # Prevents a client that sends `Idempotency-Key: ` from accidentally
+      # collapsing every batch onto one key.
+      k1 = S3.object_key("acme", 100, "payload", "")
+      k2 = S3.object_key("acme", 100, "payload", "")
+      refute k1 == k2
+    end
+
+    test "the idempotency key is scoped by tenant" do
+      # A shared idempotency key across tenants must not collide.
+      k1 = S3.object_key("alpha", 100, "p", "req-1")
+      k2 = S3.object_key("beta", 100, "p", "req-1")
+      refute String.replace(k1, "alpha", "beta") == k2
+    end
+
     test "different tenants never share a prefix" do
-      # Substring is enough — S3 list uses prefix, so any escape would show
-      # up as a shared prefix here.
-      k1 = S3.object_key("alpha", 100, "shared")
-      k2 = S3.object_key("beta", 100, "shared")
+      k1 = S3.object_key("alpha", 100, "p", "req-1")
+      k2 = S3.object_key("beta", 100, "p", "req-1")
       refute String.starts_with?(k1, "tenants/beta/")
       refute String.starts_with?(k2, "tenants/alpha/")
     end
 
     test "keys sort chronologically by sort_ns within a tenant" do
-      k_early = S3.object_key("acme", 100, "x")
-      k_late = S3.object_key("acme", 200, "x")
-      # S3 list returns keys in UTF-8 byte order; zero-padding puts earlier
-      # timestamps first.
+      k_early = S3.object_key("acme", 100, "p", "req-1")
+      k_late = S3.object_key("acme", 200, "p", "req-1")
       assert k_early < k_late
     end
   end
 
   describe "attribute sanitization" do
     test "coerces non-string map keys to strings recursively" do
-      sanitized =
-        S3.sanitize_map(%{
-          :status => 200,
-          "nested" => %{404 => "missing", :ref => "abc"},
-          "list" => [%{true => 1}]
-        })
+      assert {:ok, sanitized} =
+               S3.sanitize_map(%{
+                 :status => 200,
+                 "nested" => %{404 => "missing", :ref => "abc"},
+                 "list" => [%{true => 1}]
+               })
 
       assert Map.has_key?(sanitized, "status")
       assert sanitized["nested"] == %{"404" => "missing", "ref" => "abc"}
       assert sanitized["list"] == [%{"true" => 1}]
     end
 
-    test "collapses keys that stringify to the same value" do
-      # This is a hazard the sanitizer surfaces rather than hides: if two
-      # keys collapse, the map loses a pair. Documenting it as expected here
-      # means a future callsite that relies on distinct integer/string keys
-      # will have to model that at its own layer.
-      sanitized = S3.sanitize_map(%{1 => "int", "1" => "string"})
-      assert map_size(sanitized) == 1
+    test "rejects rather than collapses keys that stringify to the same value" do
+      # A previous version silently dropped one value on collision. Codex
+      # flagged the risk (map size decreases without a diagnostic). Now we
+      # return an explicit error so the caller can surface it.
+      assert {:error, {:attribute_key_collision, _}} =
+               S3.sanitize_map(%{1 => "int", "1" => "string"})
     end
   end
 end
diff --git a/test/pulso/storage/sort_order_test.exs b/test/pulso/storage/sort_order_test.exs
index 764b343..80f9c6b 100644
--- a/test/pulso/storage/sort_order_test.exs
+++ b/test/pulso/storage/sort_order_test.exs
@@ -52,4 +52,15 @@ defmodule Pulso.Storage.SortOrderTest do
     result = SortOrder.sort([without_trace, with_trace])
     assert Enum.map(result, & &1.trace_id) == ["aaa", nil]
   end
+
+  test "nil and empty string in the same position are distinguishable" do
+    # Prior version collapsed `nil` and `""` into the same sort key, so two
+    # otherwise-identical records could reorder unpredictably. Now `nil`
+    # sorts after every real string, including `""`.
+    with_empty = log(10, observed: 5, trace_id: "", body: "z")
+    without = log(10, observed: 5, trace_id: nil, body: "z")
+
+    result = SortOrder.sort([without, with_empty])
+    assert Enum.map(result, & &1.trace_id) == ["", nil]
+  end
 end
diff --git a/test/pulso_web/controllers/otlp_controller_test.exs b/test/pulso_web/controllers/otlp_controller_test.exs
index 815471e..accf1a1 100644
--- a/test/pulso_web/controllers/otlp_controller_test.exs
+++ b/test/pulso_web/controllers/otlp_controller_test.exs
@@ -1,6 +1,7 @@
 defmodule PulsoWeb.OTLPControllerTest do
   use PulsoWeb.ConnCase, async: false
 
+  alias Pulso.Auth.Open
   alias Pulso.Auth.SharedSecret
   alias Pulso.Record.Log
   alias Pulso.Storage
@@ -68,6 +69,33 @@ defmodule PulsoWeb.OTLPControllerTest do
     assert {:ok, []} = Storage.query("default")
   end
 
+  test "POST /v1/logs returns 400 for a tenant name that would escape the prefix",
+       %{conn: conn} do
+    conn =
+      conn
+      |> put_req_header("content-type", "application/json")
+      |> put_req_header("x-scope-orgid", "bad/name")
+      |> post(~p"/v1/logs", payload())
+
+    assert json_response(conn, 400) == %{"error" => "invalid_tenant"}
+  end
+
+  test "POST /v1/logs passes the Idempotency-Key header through", %{conn: conn} do
+    # The Idempotency-Key header should end up in Storage.append opts. Two
+    # POSTs with the same key + same payload are the same write to the store,
+    # so downstream queries see one record even if two arrived over the wire.
+    # (The Memory adapter ignores :idempotency_key, so we only assert the
+    # controller accepts the header; S3 adapter coverage of the actual
+    # dedup lives in s3_test.exs.)
+    conn =
+      conn
+      |> put_req_header("content-type", "application/json")
+      |> put_req_header("idempotency-key", "req-1")
+      |> post(~p"/v1/logs", payload())
+
+    assert json_response(conn, 200) == %{}
+  end
+
   describe "with Pulso.Auth.SharedSecret enabled" do
     setup do
       hex = Base.encode16(:crypto.hash(:sha256, "the-token"), case: :lower)
@@ -77,7 +105,10 @@ defmodule PulsoWeb.OTLPControllerTest do
         tokens: %{"acme" => "sha256$#{hex}"}
       )
 
-      on_exit(fn -> Application.delete_env(:pulso, Pulso.Auth) end)
+      on_exit(fn ->
+        Application.put_env(:pulso, Pulso.Auth, module: Open)
+      end)
+
       :ok
     end
 

From 7d08100c59d232d8857a054ea8cb2b8caa64ea7f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pedro=20Pi=C3=B1era=20Buend=C3=ADa?= <pedro@pepicrft.me>
Date: Thu, 24 Sep 2026 06:44:22 +0200
Subject: [PATCH 05/17] Address third-round Codex findings
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

HIGH — MCP.dispatch/1 with no context could bypass auth
- The "no conn = allow" fallback in Pulso.MCP.Tools was a bypass. Now a
  missing conn falls through to an empty %Plug.Conn{}, which fails
  Pulso.Auth.SharedSecret.verify/2 as :missing_token. Under
  Pulso.Auth.Open (dev/test default) the empty conn still passes, so
  existing tests still work.

HIGH — Retries with no observed timestamp still duplicated
- Pulso.Storage.S3.batch_sort_ns/1 now keys on `timestamp_ns` (caller-
  provided, stable per request) instead of `observed_timestamp_ns` (set
  fresh-now on every call). Two retries under the same Idempotency-Key
  now produce the same sort_ns and land on the same object.

HIGH — SortOrder.presence_pair crashed on non-string log bodies
- Pulso.Storage.SortOrder.presence_pair/1 now coerces non-string, non-nil
  bodies via inspect/1. OTLP AnyValue can produce int/bool/list bodies;
  the sort layer must not crash on those. Coverage in sort_order_test.

MEDIUM — Reused idempotency key + different content silently overwrote
- Pulso.Storage.S3.object_key/4 now hashes tenant || key || content_hash
  when idempotency_key is present. Same content + same key still
  collapses (idempotent). Different content + same key now diverges,
  which surfaces the client bug rather than losing the earlier write.

LOW — Port range collided with Node.js debugger (9229) and others
- Shifted RustFS host ports away from the crowded 9xxx band:
    * API:      11000 + suffix (was 9000 + suffix)
    * Console:  12000 + suffix (was 10000 + suffix)
  Avoids Node debugger (9229), ClickHouse (9000), Prometheus (9090),
  gRPC dev (50051), etc.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 config/runtime.exs                     |  2 +-
 docker-compose.yml                     |  4 ++--
 lib/pulso/mcp/tools.ex                 | 22 ++++++++++++----------
 lib/pulso/storage/s3.ex                | 16 ++++++++++++----
 lib/pulso/storage/sort_order.ex        |  7 +++++++
 mise/utilities/dev_instance_env.sh     | 15 ++++++++-------
 test/pulso/mcp/tools_test.exs          | 12 ++++++++++++
 test/pulso/storage/s3_unit_test.exs    | 10 ++++++++++
 test/pulso/storage/sort_order_test.exs | 13 +++++++++++++
 9 files changed, 77 insertions(+), 24 deletions(-)

diff --git a/config/runtime.exs b/config/runtime.exs
index 8ac3d75..3c6a79a 100644
--- a/config/runtime.exs
+++ b/config/runtime.exs
@@ -38,7 +38,7 @@ case config_env() do
       # mise/utilities/dev_instance_env.sh sets PULSO_S3_ENDPOINT per worktree.
       # The fallback matches the docker-compose default host port when mise
       # is not in the loop.
-      endpoint: System.get_env("PULSO_S3_ENDPOINT", "http://localhost:9100"),
+      endpoint: System.get_env("PULSO_S3_ENDPOINT", "http://localhost:11100"),
       region: System.get_env("PULSO_S3_REGION", "us-east-1"),
       access_key_id: System.get_env("PULSO_S3_ACCESS_KEY_ID", "rustfsadmin"),
       secret_access_key: System.get_env("PULSO_S3_SECRET_ACCESS_KEY", "rustfsadmin"),
diff --git a/docker-compose.yml b/docker-compose.yml
index 49605c4..1bdf511 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -27,8 +27,8 @@ services:
     # from another host on the LAN. Host ports come from the per-worktree env
     # to keep multiple checkouts from fighting over 9000/9001.
     ports:
-      - "127.0.0.1:${PULSO_RUSTFS_API_PORT:-9100}:9000"
-      - "127.0.0.1:${PULSO_RUSTFS_CONSOLE_PORT:-10100}:9001"
+      - "127.0.0.1:${PULSO_RUSTFS_API_PORT:-11100}:9000"
+      - "127.0.0.1:${PULSO_RUSTFS_CONSOLE_PORT:-12100}:9001"
     volumes:
       - rustfs-data:/data
 
diff --git a/lib/pulso/mcp/tools.ex b/lib/pulso/mcp/tools.ex
index 2cfd649..7141611 100644
--- a/lib/pulso/mcp/tools.ex
+++ b/lib/pulso/mcp/tools.ex
@@ -64,22 +64,24 @@ defmodule Pulso.MCP.Tools do
   def call("query_logs", _args, _context), do: {:error, {:invalid_arguments, "tenant is required"}}
   def call(name, _args, _context), do: {:error, {:unknown_tool, name}}
 
-  # Every tool that names a tenant runs it through `Pulso.Auth.verify/2`. The
-  # MCP dispatch is the same JSON-RPC transport for read and write; without
-  # this hop the ingest boundary's auth check would be bypassable via the
-  # read path.
-  defp verify(%{conn: conn}, tenant) when not is_nil(conn) do
+  # Every tool that names a tenant runs it through `Pulso.Auth.verify/2`.
+  # MCP is the same JSON-RPC transport for read and write; without this hop
+  # the ingest boundary's auth check would be bypassable via the read path.
+  #
+  # A missing conn falls through to a fresh `%Plug.Conn{}`. When the active
+  # auth module is `Pulso.Auth.Open` (dev/test default) that still returns
+  # :ok. When it is `Pulso.Auth.SharedSecret` (prod) it fails, closed —
+  # there is no in-process caller that legitimately reaches this path
+  # without a conn under real auth.
+  defp verify(context, tenant) do
+    conn = Map.get(context, :conn) || %Plug.Conn{}
+
     case Auth.verify(conn, tenant) do
       :ok -> :ok
       {:error, reason} -> {:error, {:unauthorized, reason}}
     end
   end
 
-  # No conn (e.g. an in-process caller writing a test): keep the default-open
-  # behavior so unit tests do not need to build a fake connection. Callers
-  # that reach the network path always have a conn.
-  defp verify(_context, _tenant), do: :ok
-
   defp put_opt(opts, _key, nil), do: opts
   defp put_opt(opts, key, value), do: Keyword.put(opts, key, value)
 
diff --git a/lib/pulso/storage/s3.ex b/lib/pulso/storage/s3.ex
index 7e57234..b9279d1 100644
--- a/lib/pulso/storage/s3.ex
+++ b/lib/pulso/storage/s3.ex
@@ -185,10 +185,13 @@ defmodule Pulso.Storage.S3 do
   defp stringify_key(k), do: inspect(k)
 
   defp batch_sort_ns(records) do
-    # Pick the smallest observed_timestamp_ns so identical retries hash into
-    # the same object key. Every record is normalized, so this is never nil.
+    # Pick the smallest `timestamp_ns` — the caller-provided event time.
+    # `observed_timestamp_ns` is fresh-now on each retry, so keying on it
+    # would make identical retries land on different objects even under an
+    # idempotency key. `timestamp_ns` is required by OTLP and stable per
+    # request, so retries with the same batch produce the same sort_ns.
     records
-    |> Enum.map(& &1.observed_timestamp_ns)
+    |> Enum.map(& &1.timestamp_ns)
     |> Enum.min()
   end
 
@@ -259,7 +262,12 @@ defmodule Pulso.Storage.S3 do
         # no-op overwrite. Distinct producers that pick the same key are
         # explicitly claiming "these two calls are the same write".
         <<key::binary>> when byte_size(key) > 0 ->
-          "idem-" <> stable_hash(tenant <> "\0" <> key)
+          # The hash covers tenant, key, AND content hash so a caller who
+          # accidentally reuses a key with different content does not
+          # silently overwrite the prior write — the two calls produce
+          # different objects. Same content + same key still collapses,
+          # which is the point of idempotency.
+          "idem-" <> stable_hash(tenant <> "\0" <> key <> "\0" <> content_hash(payload))
 
         # No idempotency key: the caller is fine with a retry producing a
         # duplicate. A random suffix guarantees distinct writes even when
diff --git a/lib/pulso/storage/sort_order.ex b/lib/pulso/storage/sort_order.ex
index bccb410..9c130bc 100644
--- a/lib/pulso/storage/sort_order.ex
+++ b/lib/pulso/storage/sort_order.ex
@@ -32,8 +32,15 @@ defmodule Pulso.Storage.SortOrder do
     }
   end
 
+  # `body` is `String.t() | nil` by the internal Log spec, but the OTLP
+  # decoder can produce integers, booleans, or lists when an incoming
+  # record uses those OTLP `AnyValue` shapes. Rather than crash the sort,
+  # coerce any non-string, non-nil term to a string so it still tiebreaks
+  # deterministically. `inspect/1` gives a stable, bounded representation
+  # for every Elixir term.
   defp presence_pair(nil), do: {0, ""}
   defp presence_pair(value) when is_binary(value), do: {1, value}
+  defp presence_pair(value), do: {1, inspect(value)}
 
   defp compare_desc(a, b), do: a >= b
 end
diff --git a/mise/utilities/dev_instance_env.sh b/mise/utilities/dev_instance_env.sh
index 21f7285..6b96355 100644
--- a/mise/utilities/dev_instance_env.sh
+++ b/mise/utilities/dev_instance_env.sh
@@ -138,13 +138,14 @@ export PULSO_DEV_INSTANCE_ROOT="${PROJECT_ROOT}"
 # and the ExUnit port (4002).
 export PORT="$((4000 + suffix))"
 
-# RustFS via docker-compose. The API and console ports use base ports
-# 1000 apart so two suffixes N and N+3 (tuist's convention had this bug)
-# cannot accidentally claim the same TCP port. Ranges: 9100..9999 for the
-# S3 API, 10100..10999 for the console. 9000 itself is left free so a
-# ClickHouse install on 9000 keeps working.
-export PULSO_RUSTFS_API_PORT="$((9000 + suffix))"
-export PULSO_RUSTFS_CONSOLE_PORT="$((10000 + suffix))"
+# RustFS via docker-compose. Ranges are 1000 apart so two suffixes N and
+# N+3 cannot accidentally claim the same TCP port. Bases picked to avoid
+# common developer defaults on macOS: ClickHouse (9000), Node.js debugger
+# (9229), Prometheus (9090), Grafana (3000), gRPC dev (50051).
+#     API:      11100..11999
+#     Console:  12100..12999
+export PULSO_RUSTFS_API_PORT="$((11000 + suffix))"
+export PULSO_RUSTFS_CONSOLE_PORT="$((12000 + suffix))"
 
 # What the Elixir app reads. runtime.exs reads PULSO_S3_ENDPOINT verbatim.
 export PULSO_S3_ENDPOINT="http://localhost:${PULSO_RUSTFS_API_PORT}"
diff --git a/test/pulso/mcp/tools_test.exs b/test/pulso/mcp/tools_test.exs
index c718b6e..7d3bfd1 100644
--- a/test/pulso/mcp/tools_test.exs
+++ b/test/pulso/mcp/tools_test.exs
@@ -71,6 +71,18 @@ defmodule Pulso.MCP.ToolsTest do
                Tools.call("query_logs", %{"tenant" => "acme"}, %{conn: %Plug.Conn{}})
     end
 
+    test "fails closed when no conn is passed at all" do
+      # Previous version had a "no conn = allow" fallback for in-process
+      # callers. That was a bypass: any code path that forgot the context
+      # would silently read another tenant's data under shared-secret
+      # auth. Now the fallback constructs an empty %Plug.Conn{}, which
+      # SharedSecret.verify sees as :missing_token.
+      Storage.append("acme", [%Log{timestamp_ns: 1}])
+
+      assert {:error, {:unauthorized, :missing_token}} =
+               Tools.call("query_logs", %{"tenant" => "acme"}, %{})
+    end
+
     test "rejects a query_logs call with a bad token" do
       Storage.append("acme", [%Log{timestamp_ns: 1}])
 
diff --git a/test/pulso/storage/s3_unit_test.exs b/test/pulso/storage/s3_unit_test.exs
index 8cea36b..28ccae6 100644
--- a/test/pulso/storage/s3_unit_test.exs
+++ b/test/pulso/storage/s3_unit_test.exs
@@ -69,6 +69,16 @@ defmodule Pulso.Storage.S3UnitTest do
       refute k1 == k2
     end
 
+    test "the same idempotency key with different content produces different objects" do
+      # A caller who reuses an idempotency key with a different payload is
+      # almost certainly buggy. We must not silently overwrite the earlier
+      # write with the newer one; distinct content should land on distinct
+      # keys.
+      k1 = S3.object_key("acme", 100, "payload-a", "req-1")
+      k2 = S3.object_key("acme", 100, "payload-b", "req-1")
+      refute k1 == k2
+    end
+
     test "the idempotency key is scoped by tenant" do
       # A shared idempotency key across tenants must not collide.
       k1 = S3.object_key("alpha", 100, "p", "req-1")
diff --git a/test/pulso/storage/sort_order_test.exs b/test/pulso/storage/sort_order_test.exs
index 80f9c6b..de23623 100644
--- a/test/pulso/storage/sort_order_test.exs
+++ b/test/pulso/storage/sort_order_test.exs
@@ -53,6 +53,19 @@ defmodule Pulso.Storage.SortOrderTest do
     assert Enum.map(result, & &1.trace_id) == ["aaa", nil]
   end
 
+  test "a non-string body sorts without crashing" do
+    # OTLP `AnyValue` can produce a body that is not a String — an
+    # integer, a boolean, a list. The sort layer must not crash on those;
+    # sorting is for stability, not for user-facing order.
+    int_body = %Log{timestamp_ns: 10, observed_timestamp_ns: 5, body: 42}
+    list_body = %Log{timestamp_ns: 10, observed_timestamp_ns: 5, body: [1, 2, 3]}
+    bool_body = %Log{timestamp_ns: 10, observed_timestamp_ns: 5, body: true}
+    string_body = %Log{timestamp_ns: 10, observed_timestamp_ns: 5, body: "z"}
+    nil_body = %Log{timestamp_ns: 10, observed_timestamp_ns: 5, body: nil}
+
+    assert [_, _, _, _, _] = SortOrder.sort([int_body, list_body, bool_body, string_body, nil_body])
+  end
+
   test "nil and empty string in the same position are distinguishable" do
     # Prior version collapsed `nil` and `""` into the same sort key, so two
     # otherwise-identical records could reorder unpredictably. Now `nil`

From 81f20b636067efa4db5d04e1ba8cc49d54c64218 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pedro=20Pi=C3=B1era=20Buend=C3=ADa?= <pedro@pepicrft.me>
Date: Thu, 24 Sep 2026 06:48:21 +0200
Subject: [PATCH 06/17] Address fourth-round Codex findings
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

HIGH — Retries with a missing observed timestamp still duplicated
- Previous fix put the payload content hash into the idempotent object
  key, but `normalize/1` fills `observed_timestamp_ns` with `now` before
  encoding, so the payload varied between retries and the key drifted.
- Introduce `Pulso.Storage.S3.caller_content_hash/1`: hashes a canonical
  view of the pre-normalization records (`timestamp_ns`, `severity_*`,
  `service`, `body`, `trace_id`, `span_id`, `attributes`, `resource` —
  every field the caller controls, none the normalizer will set).
- `object_key/4` now takes this fingerprint directly. The stored NDJSON
  payload still carries `observed_timestamp_ns` as ingest metadata; only
  the key derivation is stabilized.

MEDIUM — Key format is not a public API
- Add a "Key format stability" section to the S3 module docstring: any
  future change to hashing, delimiters, or sort-key width invalidates
  cross-version idempotency and needs a migration plan.

LOW — Integration tests pointed at the pre-shift port
- Update the PULSO_S3_ENDPOINT fallback in
  `test/pulso/object_store_test.exs` and `test/pulso/storage/s3_test.exs`
  from 9000 to 11100 to match the new docker-compose default.

New unit tests: identical records produce identical fingerprints;
observed_timestamp_ns does not influence the fingerprint; changing any
caller-controlled field does; a non-UTF-8 body surfaces as an encode
error rather than a crash.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 lib/pulso/storage/s3.ex             | 102 +++++++++++++++++++++-------
 test/pulso/object_store_test.exs    |   2 +-
 test/pulso/storage/s3_test.exs      |   2 +-
 test/pulso/storage/s3_unit_test.exs |  47 +++++++++++++
 4 files changed, 125 insertions(+), 28 deletions(-)

diff --git a/lib/pulso/storage/s3.ex b/lib/pulso/storage/s3.ex
index b9279d1..ca64983 100644
--- a/lib/pulso/storage/s3.ex
+++ b/lib/pulso/storage/s3.ex
@@ -36,6 +36,16 @@ defmodule Pulso.Storage.S3 do
       manifest will carry min/max `timestamp_ns` per segment and let the
       query short-circuit.
     * **No columnar layout.** Records go on the wire as NDJSON, not Parquet.
+
+  ## Key format stability
+
+  The exact object-key format is deliberately not a public API. Changing
+  the hashing scheme, the delimiter, or the sort-key width invalidates
+  cross-version idempotency: a retry landing on a newer server would not
+  find its earlier write and would create a duplicate. Any change to the
+  format needs a migration plan (either a rolling upgrade that reads both
+  formats during a window, or an explicit break with a version bump on the
+  storage schema).
   """
 
   @behaviour Pulso.Storage
@@ -62,10 +72,21 @@ defmodule Pulso.Storage.S3 do
   end
 
   def append(tenant, records, opts) when is_binary(tenant) and is_list(records) do
+    idempotency_key = Keyword.get(opts, :idempotency_key)
+
     with :ok <- validate_tenant(tenant),
+         # The idempotency hash comes from the caller-provided records
+         # BEFORE `normalize/1` fills in `observed_timestamp_ns` with the
+         # current wall clock. Otherwise a legitimate retry (same records,
+         # no observed_ts set by the caller) would produce a different
+         # payload byte sequence per call, and the deterministic idempotent
+         # suffix would change. `caller_content_hash/1` computes on the
+         # pre-normalization records; the STORED payload still carries
+         # `observed_timestamp_ns` as ingest metadata.
+         {:ok, caller_hash} <- caller_content_hash(records),
          {:ok, normalized} <- normalize(records),
          {:ok, payload} <- encode(normalized) do
-      key = object_key(tenant, batch_sort_ns(normalized), payload, Keyword.get(opts, :idempotency_key))
+      key = object_key(tenant, batch_sort_ns(normalized), caller_hash, idempotency_key)
       ObjectStore.put(config!(), key, payload)
     end
   end
@@ -252,47 +273,76 @@ defmodule Pulso.Storage.S3 do
 
   defp prefix(tenant), do: "tenants/#{tenant}/logs/"
 
+  # `caller_hash` is a 16-hex fingerprint of the pre-normalization records
+  # from `caller_content_hash/1`. The pre-normalization form matters:
+  # `normalize/1` fills in a fresh wall-clock `observed_timestamp_ns` on
+  # every call, so hashing after normalization would make identical retries
+  # produce different keys even under an idempotency key.
   @doc false
-  @spec object_key(String.t(), non_neg_integer(), binary(), String.t() | nil) :: String.t()
-  def object_key(tenant, sort_ns, payload, idempotency_key) when is_binary(tenant) and is_binary(payload) do
+  @spec object_key(String.t(), non_neg_integer(), String.t(), String.t() | nil) :: String.t()
+  def object_key(tenant, sort_ns, caller_hash, idempotency_key) when is_binary(tenant) and is_binary(caller_hash) do
     suffix =
       case idempotency_key do
-        # Opt-in idempotency: a retry of the same batch under the same key
-        # deterministically lands on the same object, so the second PUT is a
-        # no-op overwrite. Distinct producers that pick the same key are
-        # explicitly claiming "these two calls are the same write".
         <<key::binary>> when byte_size(key) > 0 ->
-          # The hash covers tenant, key, AND content hash so a caller who
-          # accidentally reuses a key with different content does not
-          # silently overwrite the prior write — the two calls produce
-          # different objects. Same content + same key still collapses,
-          # which is the point of idempotency.
-          "idem-" <> stable_hash(tenant <> "\0" <> key <> "\0" <> content_hash(payload))
-
-        # No idempotency key: the caller is fine with a retry producing a
-        # duplicate. A random suffix guarantees distinct writes even when
-        # payload and timestamp collide. `content_hash` is still mixed in
-        # so a corrupted binary at least sorts predictably.
+          # Mixes tenant, key, and caller_hash. A client that accidentally
+          # reuses an idempotency key with different content produces a
+          # different object (no silent overwrite). Same content + same key
+          # collapses onto one object, which is the point of idempotency.
+          "idem-" <> stable_hash(tenant <> "\0" <> key <> "\0" <> caller_hash)
+
         _ ->
-          "rand-" <> content_hash(payload) <> "-" <> rand_hex()
+          # No idempotency key: the caller accepts duplicates on retry. A
+          # random suffix guarantees distinct writes even when payload and
+          # timestamp collide.
+          "rand-" <> caller_hash <> "-" <> rand_hex()
       end
 
     "#{prefix(tenant)}#{zero_pad(sort_ns)}-#{suffix}.ndjson"
   end
 
+  # Fingerprint of the raw caller records, deliberately excluding fields the
+  # normalizer will fill in (`observed_timestamp_ns`). Two calls that pass
+  # the same records produce the same fingerprint; changing anything the
+  # caller controls (body, service, attributes, timestamp_ns) changes it.
+  @doc false
+  @spec caller_content_hash([Log.t()]) :: {:ok, String.t()} | {:error, term()}
+  def caller_content_hash(records) when is_list(records) do
+    payload_terms =
+      Enum.map(records, fn %Log{} = r ->
+        %{
+          "timestamp_ns" => r.timestamp_ns,
+          "severity_number" => r.severity_number,
+          "severity_text" => r.severity_text,
+          "service" => r.service,
+          "body" => r.body,
+          "trace_id" => r.trace_id,
+          "span_id" => r.span_id,
+          "attributes" => r.attributes,
+          "resource" => r.resource
+        }
+      end)
+
+    case Jason.encode(payload_terms) do
+      {:ok, bin} ->
+        digest =
+          :sha256
+          |> :crypto.hash(bin)
+          |> Base.encode16(case: :lower)
+          |> binary_part(0, @content_hash_width)
+
+        {:ok, digest}
+
+      {:error, reason} ->
+        {:error, {:encode_failed, reason}}
+    end
+  end
+
   defp zero_pad(ns) when is_integer(ns) and ns >= 0 do
     ns
     |> Integer.to_string()
     |> String.pad_leading(@sort_key_width, "0")
   end
 
-  defp content_hash(payload) do
-    :sha256
-    |> :crypto.hash(payload)
-    |> Base.encode16(case: :lower)
-    |> binary_part(0, @content_hash_width)
-  end
-
   defp stable_hash(bin) do
     :sha256
     |> :crypto.hash(bin)
diff --git a/test/pulso/object_store_test.exs b/test/pulso/object_store_test.exs
index 231f99e..d0fa72a 100644
--- a/test/pulso/object_store_test.exs
+++ b/test/pulso/object_store_test.exs
@@ -13,7 +13,7 @@ defmodule Pulso.ObjectStoreTest do
   setup do
     config = %{
       bucket: System.get_env("PULSO_S3_BUCKET", "pulso"),
-      endpoint: System.get_env("PULSO_S3_ENDPOINT", "http://localhost:9000"),
+      endpoint: System.get_env("PULSO_S3_ENDPOINT", "http://localhost:11100"),
       region: System.get_env("PULSO_S3_REGION", "us-east-1"),
       access_key_id: System.get_env("PULSO_S3_ACCESS_KEY_ID", "rustfsadmin"),
       secret_access_key: System.get_env("PULSO_S3_SECRET_ACCESS_KEY", "rustfsadmin"),
diff --git a/test/pulso/storage/s3_test.exs b/test/pulso/storage/s3_test.exs
index e99caff..0bbf6ab 100644
--- a/test/pulso/storage/s3_test.exs
+++ b/test/pulso/storage/s3_test.exs
@@ -14,7 +14,7 @@ defmodule Pulso.Storage.S3Test do
   setup do
     config = %{
       bucket: System.get_env("PULSO_S3_BUCKET", "pulso"),
-      endpoint: System.get_env("PULSO_S3_ENDPOINT", "http://localhost:9000"),
+      endpoint: System.get_env("PULSO_S3_ENDPOINT", "http://localhost:11100"),
       region: System.get_env("PULSO_S3_REGION", "us-east-1"),
       access_key_id: System.get_env("PULSO_S3_ACCESS_KEY_ID", "rustfsadmin"),
       secret_access_key: System.get_env("PULSO_S3_SECRET_ACCESS_KEY", "rustfsadmin"),
diff --git a/test/pulso/storage/s3_unit_test.exs b/test/pulso/storage/s3_unit_test.exs
index 28ccae6..774258d 100644
--- a/test/pulso/storage/s3_unit_test.exs
+++ b/test/pulso/storage/s3_unit_test.exs
@@ -100,6 +100,53 @@ defmodule Pulso.Storage.S3UnitTest do
     end
   end
 
+  describe "caller_content_hash" do
+    test "identical caller-provided records produce the same fingerprint" do
+      records = [
+        %Log{timestamp_ns: 1, service: "api", body: "hello"},
+        %Log{timestamp_ns: 2, service: "web", body: "world"}
+      ]
+
+      assert {:ok, h1} = S3.caller_content_hash(records)
+      assert {:ok, h2} = S3.caller_content_hash(records)
+      assert h1 == h2
+    end
+
+    test "observed_timestamp_ns does not influence the fingerprint" do
+      # This is the behavior we actually need. Two retries whose only
+      # difference is `observed_timestamp_ns` (filled in by `normalize` on
+      # each call, so distinct across retries) must still produce the same
+      # fingerprint so the idempotency-key path lands on the same object.
+      without_obs = [%Log{timestamp_ns: 1, body: "same"}]
+      with_obs = [%Log{timestamp_ns: 1, observed_timestamp_ns: 999, body: "same"}]
+
+      assert {:ok, h1} = S3.caller_content_hash(without_obs)
+      assert {:ok, h2} = S3.caller_content_hash(with_obs)
+      assert h1 == h2
+    end
+
+    test "every caller-controlled field influences the fingerprint" do
+      base = [%Log{timestamp_ns: 1, service: "api", body: "same"}]
+      diff_body = [%Log{timestamp_ns: 1, service: "api", body: "different"}]
+      diff_service = [%Log{timestamp_ns: 1, service: "web", body: "same"}]
+      diff_ts = [%Log{timestamp_ns: 2, service: "api", body: "same"}]
+
+      {:ok, h_base} = S3.caller_content_hash(base)
+      {:ok, h_body} = S3.caller_content_hash(diff_body)
+      {:ok, h_service} = S3.caller_content_hash(diff_service)
+      {:ok, h_ts} = S3.caller_content_hash(diff_ts)
+
+      refute h_base == h_body
+      refute h_base == h_service
+      refute h_base == h_ts
+    end
+
+    test "returns encode error for a non-UTF-8 body" do
+      assert {:error, {:encode_failed, _}} =
+               S3.caller_content_hash([%Log{timestamp_ns: 1, body: <<255>>}])
+    end
+  end
+
   describe "attribute sanitization" do
     test "coerces non-string map keys to strings recursively" do
       assert {:ok, sanitized} =

From 0e70c15e92a11160912afa7bc0b59eeabe91b864 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pedro=20Pi=C3=B1era=20Buend=C3=ADa?= <pedro@pepicrft.me>
Date: Thu, 24 Sep 2026 06:51:27 +0200
Subject: [PATCH 07/17] Address fifth-round Codex findings
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

HIGH — Caller-supplied observed_timestamp_ns was silently ignored
- The prior canonicalization excluded observed_timestamp_ns entirely so
  retries whose observed_ts was filled in by the normalizer would
  dedupe. That over-corrected: a caller who explicitly sets an observed
  timestamp is declaring it as part of the record. Excluding it meant
  two writes with different observed_ts but the same idempotency key
  overwrote each other.
- caller_content_hash/1 now includes observed_timestamp_ns as the caller
  provided it (nil when they didn't set it). Retries with nil-on-both
  still dedupe. A distinct observed_ts produces a distinct object.

MEDIUM — Hash was not canonical across Elixir/Jason versions
- caller_content_hash/1 no longer hashes Jason-encoded bytes. Jason
  serializes map keys in Map.to_list/1 order, which can shift when a
  small map promotes to a hash map or a runtime upgrade changes map
  layout. Instead, hash `:erlang.term_to_binary(term, [:deterministic])`
  bytes — the BEAM guarantees stable key ordering with that flag from
  OTP 24.1 onward. Fingerprint is now identical across runtimes, GC
  cycles, and library versions.
- New test proves attributes inserted in a different order produce the
  same fingerprint.

Cleanup: caller_content_hash/1 no longer returns {:error, _}. The
:erlang encoder handles any Elixir term including invalid UTF-8. Encode
errors on the stored payload still surface via `encode/1`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 lib/pulso/storage/s3.ex             | 82 ++++++++++++++++-------------
 test/pulso/storage/s3_unit_test.exs | 63 +++++++++++++++++-----
 2 files changed, 96 insertions(+), 49 deletions(-)

diff --git a/lib/pulso/storage/s3.ex b/lib/pulso/storage/s3.ex
index ca64983..a22191e 100644
--- a/lib/pulso/storage/s3.ex
+++ b/lib/pulso/storage/s3.ex
@@ -75,15 +75,11 @@ defmodule Pulso.Storage.S3 do
     idempotency_key = Keyword.get(opts, :idempotency_key)
 
     with :ok <- validate_tenant(tenant),
-         # The idempotency hash comes from the caller-provided records
-         # BEFORE `normalize/1` fills in `observed_timestamp_ns` with the
-         # current wall clock. Otherwise a legitimate retry (same records,
-         # no observed_ts set by the caller) would produce a different
-         # payload byte sequence per call, and the deterministic idempotent
-         # suffix would change. `caller_content_hash/1` computes on the
-         # pre-normalization records; the STORED payload still carries
-         # `observed_timestamp_ns` as ingest metadata.
-         {:ok, caller_hash} <- caller_content_hash(records),
+         # The idempotency hash is derived from the caller-provided records
+         # BEFORE `normalize/1` fills in a wall-clock `observed_timestamp_ns`.
+         # A legitimate retry has identical records; the normalizer would
+         # otherwise inject a fresh `now` and defeat the fingerprint.
+         {:ok, caller_hash} = caller_content_hash(records),
          {:ok, normalized} <- normalize(records),
          {:ok, payload} <- encode(normalized) do
       key = object_key(tenant, batch_sort_ns(normalized), caller_hash, idempotency_key)
@@ -300,43 +296,57 @@ defmodule Pulso.Storage.S3 do
     "#{prefix(tenant)}#{zero_pad(sort_ns)}-#{suffix}.ndjson"
   end
 
-  # Fingerprint of the raw caller records, deliberately excluding fields the
-  # normalizer will fill in (`observed_timestamp_ns`). Two calls that pass
-  # the same records produce the same fingerprint; changing anything the
-  # caller controls (body, service, attributes, timestamp_ns) changes it.
+  # Fingerprint of the raw caller records. Two properties hold:
+  #
+  # 1. Every field the caller controls (including `observed_timestamp_ns`
+  #    when they set it) is included — a caller who legitimately changes
+  #    that field on a "retry" is signalling a distinct write, and gets a
+  #    distinct object.
+  # 2. The encoding is canonical across runtime, GC, and Jason versions.
+  #    Two identical records always hash to the same byte sequence, in
+  #    this process and in any future release. That is what makes cross-
+  #    version idempotency safe.
+  #
+  # `:erlang.term_to_binary/2` with `:deterministic` gives us the canonical
+  # form for free: map keys are sorted, atoms and integers are encoded
+  # canonically, and the format is stable across OTP versions from OTP 24.1
+  # onward. That is much stronger than `Jason.encode/1`, which serializes
+  # map keys in `Map.to_list/1` order — a non-canonical order that can
+  # change with Elixir's map representation (small map -> hash map, GC).
   @doc false
-  @spec caller_content_hash([Log.t()]) :: {:ok, String.t()} | {:error, term()}
+  @spec caller_content_hash([Log.t()]) :: {:ok, String.t()}
   def caller_content_hash(records) when is_list(records) do
-    payload_terms =
+    canonical =
       Enum.map(records, fn %Log{} = r ->
         %{
-          "timestamp_ns" => r.timestamp_ns,
-          "severity_number" => r.severity_number,
-          "severity_text" => r.severity_text,
-          "service" => r.service,
-          "body" => r.body,
-          "trace_id" => r.trace_id,
-          "span_id" => r.span_id,
-          "attributes" => r.attributes,
-          "resource" => r.resource
+          timestamp_ns: r.timestamp_ns,
+          observed_timestamp_ns: r.observed_timestamp_ns,
+          severity_number: r.severity_number,
+          severity_text: r.severity_text,
+          service: r.service,
+          body: normalize_hash_value(r.body),
+          trace_id: r.trace_id,
+          span_id: r.span_id,
+          attributes: r.attributes,
+          resource: r.resource
         }
       end)
 
-    case Jason.encode(payload_terms) do
-      {:ok, bin} ->
-        digest =
-          :sha256
-          |> :crypto.hash(bin)
-          |> Base.encode16(case: :lower)
-          |> binary_part(0, @content_hash_width)
+    digest =
+      :sha256
+      |> :crypto.hash(:erlang.term_to_binary(canonical, [:deterministic]))
+      |> Base.encode16(case: :lower)
+      |> binary_part(0, @content_hash_width)
 
-        {:ok, digest}
-
-      {:error, reason} ->
-        {:error, {:encode_failed, reason}}
-    end
+    {:ok, digest}
   end
 
+  # `body` can arrive as a non-string term via OTLP AnyValue (int, bool,
+  # list). `:erlang.term_to_binary` handles all of these fine; nothing to
+  # normalize. This hook exists so a future callsite that wants a stable
+  # representation can add one without changing the fingerprint contract.
+  defp normalize_hash_value(v), do: v
+
   defp zero_pad(ns) when is_integer(ns) and ns >= 0 do
     ns
     |> Integer.to_string()
diff --git a/test/pulso/storage/s3_unit_test.exs b/test/pulso/storage/s3_unit_test.exs
index 774258d..1320ebd 100644
--- a/test/pulso/storage/s3_unit_test.exs
+++ b/test/pulso/storage/s3_unit_test.exs
@@ -112,19 +112,36 @@ defmodule Pulso.Storage.S3UnitTest do
       assert h1 == h2
     end
 
-    test "observed_timestamp_ns does not influence the fingerprint" do
-      # This is the behavior we actually need. Two retries whose only
-      # difference is `observed_timestamp_ns` (filled in by `normalize` on
-      # each call, so distinct across retries) must still produce the same
-      # fingerprint so the idempotency-key path lands on the same object.
-      without_obs = [%Log{timestamp_ns: 1, body: "same"}]
-      with_obs = [%Log{timestamp_ns: 1, observed_timestamp_ns: 999, body: "same"}]
-
-      assert {:ok, h1} = S3.caller_content_hash(without_obs)
-      assert {:ok, h2} = S3.caller_content_hash(with_obs)
+    test "two calls with observed_timestamp_ns left nil produce the same fingerprint" do
+      # This is the retry-safety case: OTLP callers commonly omit
+      # `observedTimeUnixNano`. Two retries both arrive with nil, both hash
+      # to the same value, and the idempotency-key path deduplicates. The
+      # normalizer fills in a wall clock later, but that happens AFTER the
+      # fingerprint is computed.
+      r = [%Log{timestamp_ns: 1, body: "same"}]
+
+      assert {:ok, h1} = S3.caller_content_hash(r)
+      assert {:ok, h2} = S3.caller_content_hash(r)
       assert h1 == h2
     end
 
+    test "a caller-set observed_timestamp_ns IS part of the fingerprint" do
+      # If the caller explicitly declares an observed timestamp, that is
+      # part of the record they authored. A "retry" that changes it is a
+      # distinct write; silently overwriting the earlier value would lose
+      # data.
+      no_obs = [%Log{timestamp_ns: 1, body: "same"}]
+      with_obs_a = [%Log{timestamp_ns: 1, observed_timestamp_ns: 100, body: "same"}]
+      with_obs_b = [%Log{timestamp_ns: 1, observed_timestamp_ns: 200, body: "same"}]
+
+      {:ok, h_none} = S3.caller_content_hash(no_obs)
+      {:ok, h_a} = S3.caller_content_hash(with_obs_a)
+      {:ok, h_b} = S3.caller_content_hash(with_obs_b)
+
+      refute h_none == h_a
+      refute h_a == h_b
+    end
+
     test "every caller-controlled field influences the fingerprint" do
       base = [%Log{timestamp_ns: 1, service: "api", body: "same"}]
       diff_body = [%Log{timestamp_ns: 1, service: "api", body: "different"}]
@@ -141,9 +158,29 @@ defmodule Pulso.Storage.S3UnitTest do
       refute h_base == h_ts
     end
 
-    test "returns encode error for a non-UTF-8 body" do
-      assert {:error, {:encode_failed, _}} =
-               S3.caller_content_hash([%Log{timestamp_ns: 1, body: <<255>>}])
+    test "a map with keys inserted in different orders produces the same fingerprint" do
+      # `:erlang.term_to_binary(_, [:deterministic])` sorts map keys before
+      # encoding. Without that, two logically-equal records could hash
+      # differently just because the caller inserted attributes in a
+      # different order — which would defeat idempotent retries across
+      # heterogeneous producers.
+      order_a = %{"a" => 1, "b" => 2, "c" => 3}
+      order_b = order_a |> Map.delete("a") |> Map.put("a", 1)
+
+      r_a = [%Log{timestamp_ns: 1, attributes: order_a}]
+      r_b = [%Log{timestamp_ns: 1, attributes: order_b}]
+
+      {:ok, h_a} = S3.caller_content_hash(r_a)
+      {:ok, h_b} = S3.caller_content_hash(r_b)
+      assert h_a == h_b
+    end
+
+    test "a non-UTF-8 body still fingerprints without crashing" do
+      # `:erlang.term_to_binary` handles any Elixir term, including binaries
+      # that are not valid UTF-8. The stored payload's `encode/1` still
+      # surfaces `:encode_failed` at write time — we just don't need to
+      # trip that path here.
+      assert {:ok, _} = S3.caller_content_hash([%Log{timestamp_ns: 1, body: <<255>>}])
     end
   end
 

From 0f1c30eba7c0ef3b101871233067792a088bc3e3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pedro=20Pi=C3=B1era=20Buend=C3=ADa?= <pedro@pepicrft.me>
Date: Thu, 24 Sep 2026 06:54:29 +0200
Subject: [PATCH 08/17] Address sixth-round Codex findings
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Both flagged the same theme — key format is not stable across releases —
so the fix is one architectural change that closes both.

HIGH — Cross-commit key format changes would orphan retries after deploy
- The whole branch has been iterating the key format (fingerprint algo,
  suffix layout, schema). If Pulso were already running in prod, each
  iteration would have orphaned prior objects. Since this is step 2 with
  no prior deployment, no migration is needed today, but the risk needs
  to be structurally closed.

MEDIUM — `:erlang.term_to_binary(_, [:deterministic])` is stable within
  an OTP release, not across major OTP upgrades. A future OTP major
  bump could change the fingerprint bytes and thus the object keys.

Fix: bake a schema version into every object key.
- New path: `tenants/<tenant>/v1/logs/<sort_ns>-<suffix>.ndjson`
- @schema_version constant in Pulso.Storage.S3
- Any future change to the fingerprint algorithm, sort-key width, or
  delimiter bumps to v2/. Old objects live at v1/, new at v2/. A reader
  can be taught to look at both during a migration window, and a
  compaction job re-keys at its own pace.
- Module docstring's "Key format stability" section rewritten to reflect
  the versioning contract and to correct the earlier overstated
  cross-version stability claim on term_to_binary.
- Tests: integration cleanup paths updated; unit test asserts the v1/
  segment is present so a future removal is caught immediately.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 lib/pulso/storage/s3.ex             | 38 +++++++++++++++++++----------
 test/pulso/storage/s3_test.exs      | 14 +++++------
 test/pulso/storage/s3_unit_test.exs |  8 ++++++
 3 files changed, 40 insertions(+), 20 deletions(-)

diff --git a/lib/pulso/storage/s3.ex b/lib/pulso/storage/s3.ex
index a22191e..4f2b2f4 100644
--- a/lib/pulso/storage/s3.ex
+++ b/lib/pulso/storage/s3.ex
@@ -39,13 +39,16 @@ defmodule Pulso.Storage.S3 do
 
   ## Key format stability
 
-  The exact object-key format is deliberately not a public API. Changing
-  the hashing scheme, the delimiter, or the sort-key width invalidates
-  cross-version idempotency: a retry landing on a newer server would not
-  find its earlier write and would create a duplicate. Any change to the
-  format needs a migration plan (either a rolling upgrade that reads both
-  formats during a window, or an explicit break with a version bump on the
-  storage schema).
+  Every object lives under a versioned prefix (`tenants/<tenant>/v1/…`).
+  Within one schema version, the exact suffix format is deliberately not
+  a public API. It is derived from `:erlang.term_to_binary(_,
+  [:deterministic])`, which is stable within an OTP release but is not
+  guaranteed to survive a major OTP upgrade (per the erts release notes,
+  the algorithm can change intentionally). Any change to the fingerprint,
+  the delimiter, or the sort-key width bumps the schema version — new
+  writes go to `v2/`, old objects stay at `v1/`, and a compaction job
+  migrates at its own pace. The reader can be taught to look at both
+  during the migration window.
   """
 
   @behaviour Pulso.Storage
@@ -267,7 +270,14 @@ defmodule Pulso.Storage.S3 do
     }
   end
 
-  defp prefix(tenant), do: "tenants/#{tenant}/logs/"
+  # Schema version segment. Baked into every object key so a future change
+  # to the key format (a new fingerprint algorithm, a different sort key
+  # width) can coexist with v1 objects rather than orphan them. Bump the
+  # version, keep readers that recognize both, and let a compaction job
+  # migrate the old prefix at leisure.
+  @schema_version "v1"
+
+  defp prefix(tenant), do: "tenants/#{tenant}/#{@schema_version}/logs/"
 
   # `caller_hash` is a 16-hex fingerprint of the pre-normalization records
   # from `caller_content_hash/1`. The pre-normalization form matters:
@@ -308,11 +318,13 @@ defmodule Pulso.Storage.S3 do
   #    version idempotency safe.
   #
   # `:erlang.term_to_binary/2` with `:deterministic` gives us the canonical
-  # form for free: map keys are sorted, atoms and integers are encoded
-  # canonically, and the format is stable across OTP versions from OTP 24.1
-  # onward. That is much stronger than `Jason.encode/1`, which serializes
-  # map keys in `Map.to_list/1` order — a non-canonical order that can
-  # change with Elixir's map representation (small map -> hash map, GC).
+  # form for free within an OTP release: map keys are sorted, atoms and
+  # integers are encoded canonically, and the same term always produces
+  # the same bytes. That is stronger than `Jason.encode/1` (map keys emit
+  # in `Map.to_list/1` order, which is not canonical and can shift when a
+  # small map promotes to a hash map). It is NOT guaranteed across major
+  # OTP upgrades — see the module docstring's "Key format stability"
+  # section for how a version bump migrates old objects when that happens.
   @doc false
   @spec caller_content_hash([Log.t()]) :: {:ok, String.t()}
   def caller_content_hash(records) when is_list(records) do
diff --git a/test/pulso/storage/s3_test.exs b/test/pulso/storage/s3_test.exs
index 0bbf6ab..f385bc5 100644
--- a/test/pulso/storage/s3_test.exs
+++ b/test/pulso/storage/s3_test.exs
@@ -26,9 +26,9 @@ defmodule Pulso.Storage.S3Test do
     tenant = "test-#{System.unique_integer([:positive])}"
 
     on_exit(fn ->
-      # The adapter writes objects under `tenants/<tenant>/logs/`; clean up so
+      # The adapter writes objects under `tenants/<tenant>/v1/logs/`; clean up so
       # a re-run starts empty.
-      case ObjectStore.list(config, "tenants/#{tenant}/logs/") do
+      case ObjectStore.list(config, "tenants/#{tenant}/v1/logs/") do
         {:ok, keys} -> Enum.each(keys, &ObjectStore.delete(config, &1))
         _ -> :ok
       end
@@ -65,7 +65,7 @@ defmodule Pulso.Storage.S3Test do
     other = "test-other-#{System.unique_integer([:positive])}"
 
     on_exit(fn ->
-      case ObjectStore.list(config, "tenants/#{other}/logs/") do
+      case ObjectStore.list(config, "tenants/#{other}/v1/logs/") do
         {:ok, keys} -> Enum.each(keys, &ObjectStore.delete(config, &1))
         _ -> :ok
       end
@@ -115,7 +115,7 @@ defmodule Pulso.Storage.S3Test do
 
   test "append with an empty batch is a no-op", %{tenant: tenant, config: config} do
     assert :ok = S3.append(tenant, [])
-    assert {:ok, keys} = ObjectStore.list(config, "tenants/#{tenant}/logs/")
+    assert {:ok, keys} = ObjectStore.list(config, "tenants/#{tenant}/v1/logs/")
     assert keys == []
   end
 
@@ -139,7 +139,7 @@ defmodule Pulso.Storage.S3Test do
     assert :ok = S3.append(tenant, batch, idempotency_key: "req-1")
     assert :ok = S3.append(tenant, batch, idempotency_key: "req-1")
 
-    assert {:ok, keys} = ObjectStore.list(config, "tenants/#{tenant}/logs/")
+    assert {:ok, keys} = ObjectStore.list(config, "tenants/#{tenant}/v1/logs/")
     assert length(keys) == 1
 
     assert {:ok, records} = S3.query(tenant, [])
@@ -157,7 +157,7 @@ defmodule Pulso.Storage.S3Test do
     assert :ok = S3.append(tenant, batch)
     assert :ok = S3.append(tenant, batch)
 
-    assert {:ok, keys} = ObjectStore.list(config, "tenants/#{tenant}/logs/")
+    assert {:ok, keys} = ObjectStore.list(config, "tenants/#{tenant}/v1/logs/")
     assert length(keys) == 2
   end
 
@@ -170,7 +170,7 @@ defmodule Pulso.Storage.S3Test do
 
     # Delete one of the objects between our own list and get, mimicking a
     # compaction / retention job racing with a query.
-    assert {:ok, [first | _]} = ObjectStore.list(config, "tenants/#{tenant}/logs/")
+    assert {:ok, [first | _]} = ObjectStore.list(config, "tenants/#{tenant}/v1/logs/")
     assert :ok = ObjectStore.delete(config, first)
 
     # Query should still return the surviving records, not error.
diff --git a/test/pulso/storage/s3_unit_test.exs b/test/pulso/storage/s3_unit_test.exs
index 1320ebd..80f8cfe 100644
--- a/test/pulso/storage/s3_unit_test.exs
+++ b/test/pulso/storage/s3_unit_test.exs
@@ -93,6 +93,14 @@ defmodule Pulso.Storage.S3UnitTest do
       refute String.starts_with?(k2, "tenants/alpha/")
     end
 
+    test "the key path carries a schema version segment" do
+      # v1 is the current write format. A future format change bumps to
+      # v2/ so old objects can be migrated at their own pace rather than
+      # orphaned. Guarding this at the key level catches an accidental
+      # removal of the versioning.
+      assert String.starts_with?(S3.object_key("acme", 100, "p", "req-1"), "tenants/acme/v1/logs/")
+    end
+
     test "keys sort chronologically by sort_ns within a tenant" do
       k_early = S3.object_key("acme", 100, "p", "req-1")
       k_late = S3.object_key("acme", 200, "p", "req-1")

From 7bae27a5a828a9f9df5bb6b82d1b461075f68f65 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pedro=20Pi=C3=B1era=20Buend=C3=ADa?= <pedro@pepicrft.me>
Date: Thu, 24 Sep 2026 06:57:40 +0200
Subject: [PATCH 09/17] Address seventh-round Codex finding
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

HIGH — OTLP ingest silently dropped records without timeUnixNano
- Pulso.OTLP.Logs.decode/1 previously returned only the surviving records,
  and PulsoWeb.OTLPController answered 200 with an empty body. A sender
  whose batch contained a record with a missing/malformed timeUnixNano
  received success and never retried the dropped record — silent data
  loss.
- decode/1 now returns {records, rejected_count}. The controller wraps a
  non-zero rejected count in an OTLP-native `partialSuccess` object per
  the ExportLogsServiceResponse schema. The sender can then decide
  whether to retry the batch (with client-side fixups on the rejected
  records) or accept the loss.
- Empty-batch and fully-successful responses still send `{}`.

Test coverage: logs_test.exs updated to the new tuple shape and asserts
the reject counter; otlp_controller_test.exs adds a case for the
partialSuccess response.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 lib/pulso/otlp/logs.ex                        | 75 ++++++++++++-------
 lib/pulso_web/controllers/otlp_controller.ex  | 20 ++++-
 test/pulso/otlp/logs_test.exs                 | 49 ++++++------
 .../controllers/otlp_controller_test.exs      | 30 ++++++++
 4 files changed, 125 insertions(+), 49 deletions(-)

diff --git a/lib/pulso/otlp/logs.ex b/lib/pulso/otlp/logs.ex
index b324c27..e800f8e 100644
--- a/lib/pulso/otlp/logs.ex
+++ b/lib/pulso/otlp/logs.ex
@@ -13,52 +13,77 @@ defmodule Pulso.OTLP.Logs do
   alias Pulso.Record.Log
 
   @doc """
-  Decode a parsed JSON payload. Returns the flat list of log records; malformed
-  entries are skipped rather than aborting the whole batch.
+  Decode a parsed JSON payload.
+
+  Returns `{records, rejected}`:
+
+    * `records` — the flat list of `Pulso.Record.Log` that survived decoding.
+    * `rejected` — the count of `LogRecord` entries that could not be
+      decoded (missing or malformed `timeUnixNano`, wrong shape, etc.).
+      The OTLP receiver surfaces this to the sender via
+      `ExportLogsPartialSuccess.rejected_log_records` per the OTLP spec so
+      the sender knows some records did not make it into storage.
   """
-  @spec decode(map()) :: [Log.t()]
+  @spec decode(map()) :: {[Log.t()], non_neg_integer()}
   def decode(%{"resourceLogs" => resource_logs}) when is_list(resource_logs) do
-    Enum.flat_map(resource_logs, &decode_resource_logs/1)
+    resource_logs
+    |> Enum.reduce({[], 0}, fn rl, {records, rejected} ->
+      {rl_records, rl_rejected} = decode_resource_logs(rl)
+      {[rl_records | records], rejected + rl_rejected}
+    end)
+    |> then(fn {records, rejected} -> {records |> Enum.reverse() |> List.flatten(), rejected} end)
   end
 
-  def decode(_), do: []
+  # A top-level shape that is not an ExportLogsServiceRequest is not a valid
+  # OTLP body at all — treat every record as rejected (well, zero counted,
+  # since we can't count what we couldn't parse) and return empty.
+  def decode(_), do: {[], 0}
 
   defp decode_resource_logs(%{"scopeLogs" => scope_logs} = resource_logs) when is_list(scope_logs) do
     resource_attrs = attributes(resource_logs["resource"])
     service = resource_attrs["service.name"]
 
-    Enum.flat_map(scope_logs, fn scope_logs ->
-      records = scope_logs["logRecords"] || []
-      Enum.flat_map(records, &decode_log_record(&1, resource_attrs, service))
+    Enum.reduce(scope_logs, {[], 0}, fn sl, {records, rejected} ->
+      raw = sl["logRecords"] || []
+
+      {sl_records, sl_rejected} =
+        Enum.reduce(raw, {[], 0}, fn record, {rs, rj} ->
+          case decode_log_record(record, resource_attrs, service) do
+            {:ok, r} -> {[r | rs], rj}
+            :error -> {rs, rj + 1}
+          end
+        end)
+
+      {[Enum.reverse(sl_records) | records], rejected + sl_rejected}
     end)
+    |> then(fn {records, rejected} -> {records |> Enum.reverse() |> List.flatten(), rejected} end)
   end
 
-  defp decode_resource_logs(_), do: []
+  defp decode_resource_logs(_), do: {[], 0}
 
   defp decode_log_record(%{} = record, resource_attrs, service) do
     case timestamp(record["timeUnixNano"]) do
       {:ok, ts} ->
-        [
-          %Log{
-            timestamp_ns: ts,
-            observed_timestamp_ns: nano(record["observedTimeUnixNano"]),
-            severity_number: record["severityNumber"],
-            severity_text: record["severityText"],
-            service: service,
-            body: any_value(record["body"]),
-            trace_id: nil_if_empty(record["traceId"]),
-            span_id: nil_if_empty(record["spanId"]),
-            attributes: attributes(record),
-            resource: resource_attrs
-          }
-        ]
+        {:ok,
+         %Log{
+           timestamp_ns: ts,
+           observed_timestamp_ns: nano(record["observedTimeUnixNano"]),
+           severity_number: record["severityNumber"],
+           severity_text: record["severityText"],
+           service: service,
+           body: any_value(record["body"]),
+           trace_id: nil_if_empty(record["traceId"]),
+           span_id: nil_if_empty(record["spanId"]),
+           attributes: attributes(record),
+           resource: resource_attrs
+         }}
 
       _ ->
-        []
+        :error
     end
   end
 
-  defp decode_log_record(_, _, _), do: []
+  defp decode_log_record(_, _, _), do: :error
 
   defp timestamp(nil), do: :error
 
diff --git a/lib/pulso_web/controllers/otlp_controller.ex b/lib/pulso_web/controllers/otlp_controller.ex
index 025b518..be771fd 100644
--- a/lib/pulso_web/controllers/otlp_controller.ex
+++ b/lib/pulso_web/controllers/otlp_controller.ex
@@ -31,9 +31,13 @@ defmodule PulsoWeb.OTLPController do
 
     with :ok <- validate_tenant(tenant),
          :ok <- Auth.verify(conn, tenant),
-         records = Logs.decode(params),
+         {records, rejected} = Logs.decode(params),
          :ok <- Storage.append(tenant, records, opts) do
-      json(conn, %{})
+      # OTLP requires the receiver to signal partial success via a
+      # top-level `partialSuccess` block instead of a plain success. That
+      # lets the sender know some records did not make it into storage
+      # without turning the whole batch into a retry.
+      json(conn, partial_success_body(rejected, length(records)))
     else
       {:error, {:invalid_tenant, _}} ->
         conn
@@ -81,4 +85,16 @@ defmodule PulsoWeb.OTLPController do
       _ -> []
     end
   end
+
+  defp partial_success_body(0, _accepted), do: %{}
+
+  defp partial_success_body(rejected, accepted) do
+    %{
+      "partialSuccess" => %{
+        "rejectedLogRecords" => rejected,
+        "errorMessage" =>
+          "#{rejected} log record(s) rejected; #{accepted} accepted. Cause: missing or malformed timeUnixNano."
+      }
+    }
+  end
 end
diff --git a/test/pulso/otlp/logs_test.exs b/test/pulso/otlp/logs_test.exs
index b17ce46..81100f5 100644
--- a/test/pulso/otlp/logs_test.exs
+++ b/test/pulso/otlp/logs_test.exs
@@ -4,9 +4,9 @@ defmodule Pulso.OTLP.LogsTest do
   alias Pulso.OTLP.Logs
   alias Pulso.Record.Log
 
-  test "returns [] for a payload without resourceLogs" do
-    assert Logs.decode(%{}) == []
-    assert Logs.decode(%{"resourceLogs" => "not-a-list"}) == []
+  test "returns {[], 0} for a payload without resourceLogs" do
+    assert Logs.decode(%{}) == {[], 0}
+    assert Logs.decode(%{"resourceLogs" => "not-a-list"}) == {[], 0}
   end
 
   test "decodes a full record with resource, attributes, and body" do
@@ -42,34 +42,39 @@ defmodule Pulso.OTLP.LogsTest do
       ]
     }
 
-    assert [
-             %Log{
-               timestamp_ns: 1_700_000_000_000_000_000,
-               observed_timestamp_ns: 1_700_000_000_000_000_001,
-               severity_number: 9,
-               severity_text: "INFO",
-               service: "api",
-               body: "hello",
-               trace_id: "abc",
-               span_id: "def",
-               attributes: %{"user.id" => "u1"},
-               resource: %{"service.name" => "api", "deploy.env" => "prod"}
-             }
-           ] = Logs.decode(payload)
+    assert {[
+              %Log{
+                timestamp_ns: 1_700_000_000_000_000_000,
+                observed_timestamp_ns: 1_700_000_000_000_000_001,
+                severity_number: 9,
+                severity_text: "INFO",
+                service: "api",
+                body: "hello",
+                trace_id: "abc",
+                span_id: "def",
+                attributes: %{"user.id" => "u1"},
+                resource: %{"service.name" => "api", "deploy.env" => "prod"}
+              }
+            ], 0} = Logs.decode(payload)
   end
 
-  test "skips records without a timestamp" do
+  test "counts records rejected for a missing timestamp" do
     payload = %{
       "resourceLogs" => [
         %{
           "scopeLogs" => [
-            %{"logRecords" => [%{"body" => %{"stringValue" => "no ts"}}]}
+            %{
+              "logRecords" => [
+                %{"body" => %{"stringValue" => "no ts"}},
+                %{"timeUnixNano" => "1", "body" => %{"stringValue" => "ok"}}
+              ]
+            }
           ]
         }
       ]
     }
 
-    assert Logs.decode(payload) == []
+    assert {[%Log{body: "ok"}], 1} = Logs.decode(payload)
   end
 
   test "decodes AnyValue variants in attributes" do
@@ -111,7 +116,7 @@ defmodule Pulso.OTLP.LogsTest do
       ]
     }
 
-    assert [%Log{attributes: attrs}] = Logs.decode(payload)
+    assert {[%Log{attributes: attrs}], 0} = Logs.decode(payload)
     assert attrs["s"] == "x"
     assert attrs["i"] == 42
     assert attrs["b"] == true
@@ -141,7 +146,7 @@ defmodule Pulso.OTLP.LogsTest do
       ]
     }
 
-    records = Logs.decode(payload)
+    assert {records, 0} = Logs.decode(payload)
     assert length(records) == 4
     assert Enum.map(records, & &1.service) == ["a", "a", "a", "b"]
     assert Enum.map(records, & &1.timestamp_ns) == [1, 2, 3, 4]
diff --git a/test/pulso_web/controllers/otlp_controller_test.exs b/test/pulso_web/controllers/otlp_controller_test.exs
index accf1a1..515000d 100644
--- a/test/pulso_web/controllers/otlp_controller_test.exs
+++ b/test/pulso_web/controllers/otlp_controller_test.exs
@@ -80,6 +80,36 @@ defmodule PulsoWeb.OTLPControllerTest do
     assert json_response(conn, 400) == %{"error" => "invalid_tenant"}
   end
 
+  test "POST /v1/logs surfaces rejected records via partialSuccess per the OTLP spec",
+       %{conn: conn} do
+    # One valid record + one missing timeUnixNano. The receiver must
+    # signal the drop rather than acknowledging silently — otherwise the
+    # sender never retries the record that was never stored.
+    payload = %{
+      "resourceLogs" => [
+        %{
+          "scopeLogs" => [
+            %{
+              "logRecords" => [
+                %{"timeUnixNano" => "1700000000000000000", "body" => %{"stringValue" => "ok"}},
+                %{"body" => %{"stringValue" => "missing ts"}}
+              ]
+            }
+          ]
+        }
+      ]
+    }
+
+    conn =
+      conn
+      |> put_req_header("content-type", "application/json")
+      |> post(~p"/v1/logs", payload)
+
+    body = json_response(conn, 200)
+    assert %{"partialSuccess" => %{"rejectedLogRecords" => 1}} = body
+    assert body["partialSuccess"]["errorMessage"] =~ "timeUnixNano"
+  end
+
   test "POST /v1/logs passes the Idempotency-Key header through", %{conn: conn} do
     # The Idempotency-Key header should end up in Storage.append opts. Two
     # POSTs with the same key + same payload are the same write to the store,

From 29897b88645746c7ca2a1533f9400bad233261d8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pedro=20Pi=C3=B1era=20Buend=C3=ADa?= <pedro@pepicrft.me>
Date: Thu, 24 Sep 2026 06:59:50 +0200
Subject: [PATCH 10/17] Address eighth-round Codex finding
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

HIGH — Logs with only observedTimeUnixNano were dropped and marked rejected
- Per the OTLP data model, `time_unix_nano` MAY be absent on a
  LogRecord. The receiver should fall back to `observed_time_unix_nano`
  and, if that is also absent, to the current wall clock. Rejecting
  those valid records via `partialSuccess` was silent data loss —
  clients are asked NOT to retry a partial-success batch.
- Pulso.OTLP.Logs.decode_log_record/3 now:
    * uses `time_unix_nano` when present,
    * falls back to `observed_time_unix_nano`,
    * falls back to `System.system_time(:nanosecond)`.
- Only genuinely malformed entries (a `logRecords` element that is not
  a JSON object) count as rejected.

Tests: two new decode cases (observed-only, both absent), one for the
non-map reject path; controller test updated to trigger reject with a
malformed entry instead of a missing timestamp.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 lib/pulso/otlp/logs.ex                        | 55 +++++++++----------
 lib/pulso_web/controllers/otlp_controller.ex  |  3 +-
 test/pulso/otlp/logs_test.exs                 | 49 ++++++++++++++++-
 .../controllers/otlp_controller_test.exs      |  5 +-
 4 files changed, 77 insertions(+), 35 deletions(-)

diff --git a/lib/pulso/otlp/logs.ex b/lib/pulso/otlp/logs.ex
index e800f8e..8c79eac 100644
--- a/lib/pulso/otlp/logs.ex
+++ b/lib/pulso/otlp/logs.ex
@@ -61,39 +61,38 @@ defmodule Pulso.OTLP.Logs do
 
   defp decode_resource_logs(_), do: {[], 0}
 
+  # Per the OTLP logs data model, `time_unix_nano` MAY be absent when the
+  # emitter cannot determine an event time. In that case the receiver
+  # should fall back to `observed_time_unix_nano`, and if that is also
+  # absent, to the current wall clock. Rejecting for missing time_unix_nano
+  # would silently drop valid records — a partial-success response makes it
+  # worse because the sender is asked NOT to retry.
   defp decode_log_record(%{} = record, resource_attrs, service) do
-    case timestamp(record["timeUnixNano"]) do
-      {:ok, ts} ->
-        {:ok,
-         %Log{
-           timestamp_ns: ts,
-           observed_timestamp_ns: nano(record["observedTimeUnixNano"]),
-           severity_number: record["severityNumber"],
-           severity_text: record["severityText"],
-           service: service,
-           body: any_value(record["body"]),
-           trace_id: nil_if_empty(record["traceId"]),
-           span_id: nil_if_empty(record["spanId"]),
-           attributes: attributes(record),
-           resource: resource_attrs
-         }}
-
-      _ ->
-        :error
-    end
+    observed = nano(record["observedTimeUnixNano"])
+
+    timestamp_ns =
+      case nano(record["timeUnixNano"]) do
+        nil -> observed || System.system_time(:nanosecond)
+        ns -> ns
+      end
+
+    {:ok,
+     %Log{
+       timestamp_ns: timestamp_ns,
+       observed_timestamp_ns: observed,
+       severity_number: record["severityNumber"],
+       severity_text: record["severityText"],
+       service: service,
+       body: any_value(record["body"]),
+       trace_id: nil_if_empty(record["traceId"]),
+       span_id: nil_if_empty(record["spanId"]),
+       attributes: attributes(record),
+       resource: resource_attrs
+     }}
   end
 
   defp decode_log_record(_, _, _), do: :error
 
-  defp timestamp(nil), do: :error
-
-  defp timestamp(value) do
-    case nano(value) do
-      nil -> :error
-      ns -> {:ok, ns}
-    end
-  end
-
   defp nano(nil), do: nil
   defp nano(value) when is_integer(value), do: value
 
diff --git a/lib/pulso_web/controllers/otlp_controller.ex b/lib/pulso_web/controllers/otlp_controller.ex
index be771fd..51f1720 100644
--- a/lib/pulso_web/controllers/otlp_controller.ex
+++ b/lib/pulso_web/controllers/otlp_controller.ex
@@ -92,8 +92,7 @@ defmodule PulsoWeb.OTLPController do
     %{
       "partialSuccess" => %{
         "rejectedLogRecords" => rejected,
-        "errorMessage" =>
-          "#{rejected} log record(s) rejected; #{accepted} accepted. Cause: missing or malformed timeUnixNano."
+        "errorMessage" => "#{rejected} log record(s) rejected; #{accepted} accepted. Cause: malformed logRecord entry."
       }
     }
   end
diff --git a/test/pulso/otlp/logs_test.exs b/test/pulso/otlp/logs_test.exs
index 81100f5..e0eea25 100644
--- a/test/pulso/otlp/logs_test.exs
+++ b/test/pulso/otlp/logs_test.exs
@@ -58,14 +58,59 @@ defmodule Pulso.OTLP.LogsTest do
             ], 0} = Logs.decode(payload)
   end
 
-  test "counts records rejected for a missing timestamp" do
+  test "falls back to observedTimeUnixNano when timeUnixNano is absent" do
+    # Per the OTLP data model, `time_unix_nano` MAY be absent. The
+    # receiver should use `observed_time_unix_nano` in that case.
+    # Rejecting valid records would be silent data loss.
     payload = %{
       "resourceLogs" => [
         %{
           "scopeLogs" => [
             %{
               "logRecords" => [
-                %{"body" => %{"stringValue" => "no ts"}},
+                %{
+                  "observedTimeUnixNano" => "42",
+                  "body" => %{"stringValue" => "only observed"}
+                }
+              ]
+            }
+          ]
+        }
+      ]
+    }
+
+    assert {[%Log{timestamp_ns: 42, observed_timestamp_ns: 42, body: "only observed"}], 0} =
+             Logs.decode(payload)
+  end
+
+  test "falls back to the current wall clock when both timestamps are absent" do
+    before_call = System.system_time(:nanosecond)
+
+    payload = %{
+      "resourceLogs" => [
+        %{
+          "scopeLogs" => [
+            %{"logRecords" => [%{"body" => %{"stringValue" => "no ts at all"}}]}
+          ]
+        }
+      ]
+    }
+
+    assert {[%Log{timestamp_ns: ts, observed_timestamp_ns: nil}], 0} = Logs.decode(payload)
+    after_call = System.system_time(:nanosecond)
+    assert ts >= before_call and ts <= after_call
+  end
+
+  test "counts a non-map logRecords entry as rejected" do
+    # A logRecord entry that is not a map is genuinely malformed — the
+    # decoder cannot invent fields, so it counts toward rejected.
+    payload = %{
+      "resourceLogs" => [
+        %{
+          "scopeLogs" => [
+            %{
+              "logRecords" => [
+                "not-a-map",
                 %{"timeUnixNano" => "1", "body" => %{"stringValue" => "ok"}}
               ]
             }
diff --git a/test/pulso_web/controllers/otlp_controller_test.exs b/test/pulso_web/controllers/otlp_controller_test.exs
index 515000d..6012cbc 100644
--- a/test/pulso_web/controllers/otlp_controller_test.exs
+++ b/test/pulso_web/controllers/otlp_controller_test.exs
@@ -82,7 +82,7 @@ defmodule PulsoWeb.OTLPControllerTest do
 
   test "POST /v1/logs surfaces rejected records via partialSuccess per the OTLP spec",
        %{conn: conn} do
-    # One valid record + one missing timeUnixNano. The receiver must
+    # One valid record + one malformed (non-map) entry. The receiver must
     # signal the drop rather than acknowledging silently — otherwise the
     # sender never retries the record that was never stored.
     payload = %{
@@ -92,7 +92,7 @@ defmodule PulsoWeb.OTLPControllerTest do
             %{
               "logRecords" => [
                 %{"timeUnixNano" => "1700000000000000000", "body" => %{"stringValue" => "ok"}},
-                %{"body" => %{"stringValue" => "missing ts"}}
+                "not-a-map"
               ]
             }
           ]
@@ -107,7 +107,6 @@ defmodule PulsoWeb.OTLPControllerTest do
 
     body = json_response(conn, 200)
     assert %{"partialSuccess" => %{"rejectedLogRecords" => 1}} = body
-    assert body["partialSuccess"]["errorMessage"] =~ "timeUnixNano"
   end
 
   test "POST /v1/logs passes the Idempotency-Key header through", %{conn: conn} do

From e4045f488c212feb75dd59bebe271a152e0b2914 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pedro=20Pi=C3=B1era=20Buend=C3=ADa?= <pedro@pepicrft.me>
Date: Thu, 24 Sep 2026 07:04:07 +0200
Subject: [PATCH 11/17] Address ninth-round Codex findings
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

HIGH — Zero timestamps sent every record to the Unix epoch
- Per the OTLP spec, `*_unix_nano: 0` means "unknown", identical to the
  field being absent. The prior decoder passed the 0 through, so a
  record with only observedTimeUnixNano set was stored at ts=0 and
  vanished from time-bounded queries.
- Pulso.OTLP.Logs.nano_or_nil/1 now folds 0 (integer or "0" string) to
  nil.

HIGH — Wall-clock fallback at decode broke idempotency
- The previous fix stuffed `now` into `timestamp_ns` whenever both OTLP
  timestamps were missing. But that value flowed into
  caller_content_hash AND batch_sort_ns, so two retries under the same
  Idempotency-Key produced different fingerprints AND different sort_ns
  prefixes — the second PUT landed at a distinct key and duplicated.
- Split the responsibilities. The decoder preserves the caller's intent
  (nil for absent). Pulso.Record.Log's `timestamp_ns` is now nilable.
- Storage adapters (Memory and S3) backfill on ingest: prefer the
  observed timestamp, then the wall clock. This runs AFTER
  caller_content_hash and caller_sort_ns compute their fingerprints on
  the pre-normalization records, so retries stay deterministic.
- Rename Pulso.Storage.S3.batch_sort_ns/1 to caller_sort_ns/1 and
  compute it from pre-normalization records with a 0 fallback for nil
  timestamps.

New test coverage:
- OTLP.Logs: decoder preserves nil ts (both absent, observed-only,
  time_unix_nano=0 folds to nil).
- Memory adapter: backfill from observed, backfill from wall clock.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 lib/pulso/otlp/logs.ex             | 38 +++++++++++------------
 lib/pulso/record/log.ex            |  9 ++++--
 lib/pulso/storage/memory.ex        |  8 ++++-
 lib/pulso/storage/s3.ex            | 37 +++++++++++++++--------
 test/pulso/otlp/logs_test.exs      | 48 ++++++++++++++++++++++--------
 test/pulso/storage/memory_test.exs | 16 ++++++++++
 6 files changed, 108 insertions(+), 48 deletions(-)

diff --git a/lib/pulso/otlp/logs.ex b/lib/pulso/otlp/logs.ex
index 8c79eac..2f3ebb6 100644
--- a/lib/pulso/otlp/logs.ex
+++ b/lib/pulso/otlp/logs.ex
@@ -61,25 +61,18 @@ defmodule Pulso.OTLP.Logs do
 
   defp decode_resource_logs(_), do: {[], 0}
 
-  # Per the OTLP logs data model, `time_unix_nano` MAY be absent when the
-  # emitter cannot determine an event time. In that case the receiver
-  # should fall back to `observed_time_unix_nano`, and if that is also
-  # absent, to the current wall clock. Rejecting for missing time_unix_nano
-  # would silently drop valid records — a partial-success response makes it
-  # worse because the sender is asked NOT to retry.
+  # Per the OTLP logs data model, both `time_unix_nano` and
+  # `observed_time_unix_nano` MAY be absent, and a value of 0 explicitly
+  # means "unknown". The decoder preserves the caller's intent verbatim
+  # (nil for absent or 0). The storage adapter fills a stored timestamp
+  # later so that (a) the caller_content_hash is stable across retries
+  # even when both timestamps are absent, and (b) records with only an
+  # observed time are not lost or misplaced at the Unix epoch.
   defp decode_log_record(%{} = record, resource_attrs, service) do
-    observed = nano(record["observedTimeUnixNano"])
-
-    timestamp_ns =
-      case nano(record["timeUnixNano"]) do
-        nil -> observed || System.system_time(:nanosecond)
-        ns -> ns
-      end
-
     {:ok,
      %Log{
-       timestamp_ns: timestamp_ns,
-       observed_timestamp_ns: observed,
+       timestamp_ns: nano_or_nil(record["timeUnixNano"]),
+       observed_timestamp_ns: nano_or_nil(record["observedTimeUnixNano"]),
        severity_number: record["severityNumber"],
        severity_text: record["severityText"],
        service: service,
@@ -93,17 +86,22 @@ defmodule Pulso.OTLP.Logs do
 
   defp decode_log_record(_, _, _), do: :error
 
-  defp nano(nil), do: nil
-  defp nano(value) when is_integer(value), do: value
+  # OTLP: a `*_unix_nano` value of 0 signals "unknown", identical in
+  # meaning to the field being absent. Fold both into nil so downstream
+  # code has one shape to reason about.
+  defp nano_or_nil(nil), do: nil
+  defp nano_or_nil(0), do: nil
+  defp nano_or_nil(value) when is_integer(value), do: value
 
-  defp nano(value) when is_binary(value) do
+  defp nano_or_nil(value) when is_binary(value) do
     case Integer.parse(value) do
+      {0, ""} -> nil
       {int, ""} -> int
       _ -> nil
     end
   end
 
-  defp nano(_), do: nil
+  defp nano_or_nil(_), do: nil
 
   defp attributes(%{"attributes" => kvs}) when is_list(kvs) do
     Map.new(kvs, fn
diff --git a/lib/pulso/record/log.ex b/lib/pulso/record/log.ex
index b764343..a24aefa 100644
--- a/lib/pulso/record/log.ex
+++ b/lib/pulso/record/log.ex
@@ -7,7 +7,12 @@ defmodule Pulso.Record.Log do
   everything else lives in `attributes`.
   """
 
-  @enforce_keys [:timestamp_ns]
+  # `timestamp_ns` may be nil at decode time — the OTLP spec allows both
+  # `time_unix_nano` and `observed_time_unix_nano` to be absent, and it
+  # explicitly treats a value of 0 as "unknown". The storage adapter fills
+  # in a stored timestamp from the observed value or the wall clock; the
+  # nil is what lets a retry keep an idempotent fingerprint (the caller
+  # sent the same batch with no timestamps, so both retries hash the same).
   defstruct [
     :timestamp_ns,
     :observed_timestamp_ns,
@@ -22,7 +27,7 @@ defmodule Pulso.Record.Log do
   ]
 
   @type t :: %__MODULE__{
-          timestamp_ns: non_neg_integer(),
+          timestamp_ns: non_neg_integer() | nil,
           observed_timestamp_ns: non_neg_integer() | nil,
           severity_number: non_neg_integer() | nil,
           severity_text: String.t() | nil,
diff --git a/lib/pulso/storage/memory.ex b/lib/pulso/storage/memory.ex
index d389f70..4055470 100644
--- a/lib/pulso/storage/memory.ex
+++ b/lib/pulso/storage/memory.ex
@@ -35,7 +35,13 @@ defmodule Pulso.Storage.Memory do
 
     normalized =
       for %Log{} = record <- records do
-        %{record | observed_timestamp_ns: record.observed_timestamp_ns || now}
+        observed = record.observed_timestamp_ns || now
+
+        %{
+          record
+          | timestamp_ns: record.timestamp_ns || observed,
+            observed_timestamp_ns: observed
+        }
       end
 
     existing =
diff --git a/lib/pulso/storage/s3.ex b/lib/pulso/storage/s3.ex
index 4f2b2f4..9839636 100644
--- a/lib/pulso/storage/s3.ex
+++ b/lib/pulso/storage/s3.ex
@@ -78,14 +78,16 @@ defmodule Pulso.Storage.S3 do
     idempotency_key = Keyword.get(opts, :idempotency_key)
 
     with :ok <- validate_tenant(tenant),
-         # The idempotency hash is derived from the caller-provided records
-         # BEFORE `normalize/1` fills in a wall-clock `observed_timestamp_ns`.
-         # A legitimate retry has identical records; the normalizer would
-         # otherwise inject a fresh `now` and defeat the fingerprint.
+         # Both the fingerprint AND the sort-key prefix are derived from the
+         # caller-provided records BEFORE `normalize/1` fills any wall-clock
+         # timestamps. A legitimate retry then produces the same object key
+         # in full — the sort_ns prefix and the idempotency suffix are both
+         # stable, so the second PUT overwrites the first as intended.
          {:ok, caller_hash} = caller_content_hash(records),
+         sort_ns = caller_sort_ns(records),
          {:ok, normalized} <- normalize(records),
          {:ok, payload} <- encode(normalized) do
-      key = object_key(tenant, batch_sort_ns(normalized), caller_hash, idempotency_key)
+      key = object_key(tenant, sort_ns, caller_hash, idempotency_key)
       ObjectStore.put(config!(), key, payload)
     end
   end
@@ -124,9 +126,17 @@ defmodule Pulso.Storage.S3 do
       %Log{} = record, {:ok, acc} ->
         with {:ok, attrs} <- sanitize_map(record.attributes || %{}),
              {:ok, resource} <- sanitize_map(record.resource || %{}) do
+          observed_ts = record.observed_timestamp_ns || now
+
+          # OTLP allows both timestamps to be absent (or explicitly zero,
+          # which the decoder folds to nil). We backfill: prefer the
+          # observed timestamp, then wall clock. This runs AFTER the
+          # caller_content_hash, so retries with identical raw records
+          # still hash to the same fingerprint.
           normalized = %{
             record
-            | observed_timestamp_ns: record.observed_timestamp_ns || now,
+            | timestamp_ns: record.timestamp_ns || observed_ts,
+              observed_timestamp_ns: observed_ts,
               attributes: attrs,
               resource: resource
           }
@@ -204,14 +214,15 @@ defmodule Pulso.Storage.S3 do
   defp stringify_key(k) when is_integer(k), do: Integer.to_string(k)
   defp stringify_key(k), do: inspect(k)
 
-  defp batch_sort_ns(records) do
-    # Pick the smallest `timestamp_ns` — the caller-provided event time.
-    # `observed_timestamp_ns` is fresh-now on each retry, so keying on it
-    # would make identical retries land on different objects even under an
-    # idempotency key. `timestamp_ns` is required by OTLP and stable per
-    # request, so retries with the same batch produce the same sort_ns.
+  # Pick the smallest caller-supplied `timestamp_ns` for the object key
+  # prefix. Runs on records BEFORE normalization so a retry with identical
+  # caller input produces the same sort_ns. A record with no timestamp
+  # contributes 0, which parks the object at the head of the tenant
+  # listing — good enough for the fallback case and, importantly,
+  # deterministic across retries.
+  defp caller_sort_ns(records) do
     records
-    |> Enum.map(& &1.timestamp_ns)
+    |> Enum.map(fn %Log{timestamp_ns: ts} -> ts || 0 end)
     |> Enum.min()
   end
 
diff --git a/test/pulso/otlp/logs_test.exs b/test/pulso/otlp/logs_test.exs
index e0eea25..fe7f626 100644
--- a/test/pulso/otlp/logs_test.exs
+++ b/test/pulso/otlp/logs_test.exs
@@ -58,10 +58,26 @@ defmodule Pulso.OTLP.LogsTest do
             ], 0} = Logs.decode(payload)
   end
 
-  test "falls back to observedTimeUnixNano when timeUnixNano is absent" do
-    # Per the OTLP data model, `time_unix_nano` MAY be absent. The
-    # receiver should use `observed_time_unix_nano` in that case.
-    # Rejecting valid records would be silent data loss.
+  test "preserves absent timestamps as nil so storage can backfill deterministically" do
+    # OTLP allows both timestamps to be absent, and a zero value means
+    # "unknown". The decoder does NOT invent a wall-clock value here —
+    # that would defeat the caller_content_hash for a retry. The storage
+    # adapter fills in a stored timestamp later.
+    payload = %{
+      "resourceLogs" => [
+        %{
+          "scopeLogs" => [
+            %{"logRecords" => [%{"body" => %{"stringValue" => "no ts"}}]}
+          ]
+        }
+      ]
+    }
+
+    assert {[%Log{timestamp_ns: nil, observed_timestamp_ns: nil, body: "no ts"}], 0} =
+             Logs.decode(payload)
+  end
+
+  test "preserves an observed-only timestamp verbatim" do
     payload = %{
       "resourceLogs" => [
         %{
@@ -79,26 +95,34 @@ defmodule Pulso.OTLP.LogsTest do
       ]
     }
 
-    assert {[%Log{timestamp_ns: 42, observed_timestamp_ns: 42, body: "only observed"}], 0} =
+    assert {[%Log{timestamp_ns: nil, observed_timestamp_ns: 42, body: "only observed"}], 0} =
              Logs.decode(payload)
   end
 
-  test "falls back to the current wall clock when both timestamps are absent" do
-    before_call = System.system_time(:nanosecond)
-
+  test "folds a zero timestamp to nil per the OTLP spec" do
+    # `time_unix_nano: 0` means "unknown", identical in meaning to the
+    # field being absent. If the decoder left it as 0, the record would
+    # be stored at the Unix epoch and vanish from time-bounded queries.
     payload = %{
       "resourceLogs" => [
         %{
           "scopeLogs" => [
-            %{"logRecords" => [%{"body" => %{"stringValue" => "no ts at all"}}]}
+            %{
+              "logRecords" => [
+                %{
+                  "timeUnixNano" => "0",
+                  "observedTimeUnixNano" => "1700000000000000000",
+                  "body" => %{"stringValue" => "zero ts"}
+                }
+              ]
+            }
           ]
         }
       ]
     }
 
-    assert {[%Log{timestamp_ns: ts, observed_timestamp_ns: nil}], 0} = Logs.decode(payload)
-    after_call = System.system_time(:nanosecond)
-    assert ts >= before_call and ts <= after_call
+    assert {[%Log{timestamp_ns: nil, observed_timestamp_ns: 1_700_000_000_000_000_000}], 0} =
+             Logs.decode(payload)
   end
 
   test "counts a non-map logRecords entry as rejected" do
diff --git a/test/pulso/storage/memory_test.exs b/test/pulso/storage/memory_test.exs
index d31d6b4..da92dba 100644
--- a/test/pulso/storage/memory_test.exs
+++ b/test/pulso/storage/memory_test.exs
@@ -62,6 +62,22 @@ defmodule Pulso.Storage.MemoryTest do
     assert observed >= before_append and observed <= after_append
   end
 
+  test "backfills a nil timestamp_ns from the observed timestamp" do
+    # OTLP allows `time_unix_nano` to be absent. Storage picks up the
+    # observed timestamp when the caller supplied one.
+    :ok = Storage.append("t", [%Log{timestamp_ns: nil, observed_timestamp_ns: 42}])
+    assert {:ok, [%Log{timestamp_ns: 42, observed_timestamp_ns: 42}]} = Storage.query("t")
+  end
+
+  test "backfills a nil timestamp_ns from the wall clock when observed is absent too" do
+    before_append = System.system_time(:nanosecond)
+    :ok = Storage.append("t", [%Log{timestamp_ns: nil, observed_timestamp_ns: nil}])
+    after_append = System.system_time(:nanosecond)
+
+    assert {:ok, [%Log{timestamp_ns: ts}]} = Storage.query("t")
+    assert ts >= before_append and ts <= after_append
+  end
+
   test "equal timestamps are broken by observed_timestamp_ns then trace_id" do
     # Guard against a limit response depending on adapter-internal insertion
     # order. Two adapters must sort ties the same way; both delegate to

From 3e94cb34a1a19ff2355e7de35a8b9633d6c96604 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pedro=20Pi=C3=B1era=20Buend=C3=ADa?= <pedro@pepicrft.me>
Date: Thu, 24 Sep 2026 07:06:58 +0200
Subject: [PATCH 12/17] Address tenth-round Codex finding + nil-ts filter trap
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

HIGH — Retries silently mutated the stored timestamp
- `normalize/1` in both storage adapters was injecting `now` for a nil
  timestamp on every append. A retry under the same idempotency_key
  therefore landed on the same object key but overwrote the first
  record with a later ts. An already-acknowledged log could disappear
  from its original time range and re-emerge in a later one.
- Storage no longer backfills timestamps at all. Records arrive
  verbatim; retries are truly idempotent because the payload is
  identical byte-for-byte. If a caller needs a wall-clock timestamp,
  they set it themselves at ingest. Step 3's conditional PUT will
  allow first-write-wins backfill safely.

Bonus catch while reviewing the query path:
- `filter_by_time` compared `ts >= start_ts` — Elixir's term ordering
  puts atoms above numbers, so `nil >= 5` was true, and nil-ts records
  leaked through every start_ts filter. Added an `is_integer(ts)`
  guard in both adapters.

Tests: Memory & S3 tests updated to verify records stored verbatim;
new test explicitly guards the nil-ts / time-filter behavior.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 lib/pulso/storage/memory.ex        | 28 ++++++++++----------
 lib/pulso/storage/s3.ex            | 32 ++++++++++-------------
 test/pulso/storage/memory_test.exs | 41 +++++++++++++++---------------
 test/pulso/storage/s3_test.exs     | 18 ++++++++-----
 4 files changed, 60 insertions(+), 59 deletions(-)

diff --git a/lib/pulso/storage/memory.ex b/lib/pulso/storage/memory.ex
index 4055470..8837cbe 100644
--- a/lib/pulso/storage/memory.ex
+++ b/lib/pulso/storage/memory.ex
@@ -29,20 +29,12 @@ defmodule Pulso.Storage.Memory do
 
   @impl Pulso.Storage
   def append(tenant, records, _opts \\ []) when is_binary(tenant) and is_list(records) do
-    # Memory ignores :idempotency_key — it's a test adapter, and repeat
-    # tests reset state between cases anyway.
-    now = System.system_time(:nanosecond)
-
-    normalized =
-      for %Log{} = record <- records do
-        observed = record.observed_timestamp_ns || now
-
-        %{
-          record
-          | timestamp_ns: record.timestamp_ns || observed,
-            observed_timestamp_ns: observed
-        }
-      end
+    # Memory ignores :idempotency_key — it's a test adapter. Storage does
+    # NOT backfill timestamps: injecting `now` would defeat the retry
+    # story that the S3 adapter relies on for idempotency (see
+    # `Pulso.Storage.S3` docstring). Callers that need a wall-clock
+    # timestamp set it themselves at ingest.
+    normalized = records
 
     existing =
       case :ets.lookup(@table, tenant) do
@@ -92,7 +84,13 @@ defmodule Pulso.Storage.Memory do
 
   defp filter_by_time(records, start_ts, end_ts) do
     Enum.filter(records, fn %Log{timestamp_ns: ts} ->
-      (start_ts == nil or ts >= start_ts) and (end_ts == nil or ts <= end_ts)
+      # A nil timestamp does not fit inside a time-bounded range. Elixir's
+      # term ordering puts atoms greater than numbers, so `nil >= 5` is
+      # true without an explicit guard — leaving nil-ts records leaking
+      # through every time filter.
+      is_integer(ts) and
+        (start_ts == nil or ts >= start_ts) and
+        (end_ts == nil or ts <= end_ts)
     end)
   end
 
diff --git a/lib/pulso/storage/s3.ex b/lib/pulso/storage/s3.ex
index 9839636..02dc52b 100644
--- a/lib/pulso/storage/s3.ex
+++ b/lib/pulso/storage/s3.ex
@@ -120,27 +120,17 @@ defmodule Pulso.Storage.S3 do
   end
 
   defp normalize(records) do
-    now = System.system_time(:nanosecond)
-
     Enum.reduce_while(records, {:ok, []}, fn
       %Log{} = record, {:ok, acc} ->
         with {:ok, attrs} <- sanitize_map(record.attributes || %{}),
              {:ok, resource} <- sanitize_map(record.resource || %{}) do
-          observed_ts = record.observed_timestamp_ns || now
-
-          # OTLP allows both timestamps to be absent (or explicitly zero,
-          # which the decoder folds to nil). We backfill: prefer the
-          # observed timestamp, then wall clock. This runs AFTER the
-          # caller_content_hash, so retries with identical raw records
-          # still hash to the same fingerprint.
-          normalized = %{
-            record
-            | timestamp_ns: record.timestamp_ns || observed_ts,
-              observed_timestamp_ns: observed_ts,
-              attributes: attrs,
-              resource: resource
-          }
-
+          # Deliberately no wall-clock backfill here. Injecting `now` for a
+          # nil timestamp would make retries under the same idempotency key
+          # overwrite the first stored record with a later timestamp, so an
+          # already-acknowledged log would disappear from its original time
+          # range and re-emerge in a later one. Step 3's conditional PUT
+          # (write-only-if-absent) will allow first-write-wins backfill.
+          normalized = %{record | attributes: attrs, resource: resource}
           {:cont, {:ok, [normalized | acc]}}
         else
           err -> {:halt, err}
@@ -391,7 +381,13 @@ defmodule Pulso.Storage.S3 do
 
   defp filter_by_time(records, start_ts, end_ts) do
     Enum.filter(records, fn %Log{timestamp_ns: ts} ->
-      (start_ts == nil or ts >= start_ts) and (end_ts == nil or ts <= end_ts)
+      # A record with a nil timestamp has no place inside a time-bounded
+      # range. Elixir's term ordering puts atoms greater than numbers, so
+      # `nil >= 5` is true — without the `is_integer` guard, nil-ts
+      # records would leak through every time filter.
+      is_integer(ts) and
+        (start_ts == nil or ts >= start_ts) and
+        (end_ts == nil or ts <= end_ts)
     end)
   end
 
diff --git a/test/pulso/storage/memory_test.exs b/test/pulso/storage/memory_test.exs
index da92dba..6c12aec 100644
--- a/test/pulso/storage/memory_test.exs
+++ b/test/pulso/storage/memory_test.exs
@@ -53,29 +53,30 @@ defmodule Pulso.Storage.MemoryTest do
     assert Enum.map(records, & &1.timestamp_ns) == [4, 3]
   end
 
-  test "populates observed_timestamp_ns when the record does not carry one" do
-    before_append = System.system_time(:nanosecond)
-    :ok = Storage.append("t", [record(1)])
-    after_append = System.system_time(:nanosecond)
-
-    assert {:ok, [%Log{observed_timestamp_ns: observed}]} = Storage.query("t")
-    assert observed >= before_append and observed <= after_append
-  end
-
-  test "backfills a nil timestamp_ns from the observed timestamp" do
-    # OTLP allows `time_unix_nano` to be absent. Storage picks up the
-    # observed timestamp when the caller supplied one.
-    :ok = Storage.append("t", [%Log{timestamp_ns: nil, observed_timestamp_ns: 42}])
-    assert {:ok, [%Log{timestamp_ns: 42, observed_timestamp_ns: 42}]} = Storage.query("t")
+  test "nil-timestamp records do not leak through time-bounded queries" do
+    # Guards a subtle Elixir term-ordering trap: nil >= 5 returns true
+    # because atoms sort above integers. Without an explicit is_integer
+    # guard in filter_by_time, nil-ts records would slip past every
+    # start_ts filter.
+    :ok = Storage.append("t", [%Log{timestamp_ns: nil}, record(100)])
+
+    assert {:ok, [%Log{timestamp_ns: 100}]} = Storage.query("t", start_ts: 50)
+    assert {:ok, [%Log{timestamp_ns: 100}]} = Storage.query("t", end_ts: 200)
+
+    # But an unbounded query still returns them.
+    assert {:ok, records} = Storage.query("t")
+    assert Enum.any?(records, &is_nil(&1.timestamp_ns))
   end
 
-  test "backfills a nil timestamp_ns from the wall clock when observed is absent too" do
-    before_append = System.system_time(:nanosecond)
+  test "stores records verbatim without wall-clock backfill" do
+    # Prior versions injected `now` when a timestamp was nil. That was
+    # dropped because injecting a fresh timestamp per call breaks the
+    # retry-idempotency story in the S3 adapter — the second PUT under
+    # the same idempotency key would overwrite the first with a later
+    # timestamp. Memory is the test-only adapter and mirrors the same
+    # contract for consistency.
     :ok = Storage.append("t", [%Log{timestamp_ns: nil, observed_timestamp_ns: nil}])
-    after_append = System.system_time(:nanosecond)
-
-    assert {:ok, [%Log{timestamp_ns: ts}]} = Storage.query("t")
-    assert ts >= before_append and ts <= after_append
+    assert {:ok, [%Log{timestamp_ns: nil, observed_timestamp_ns: nil}]} = Storage.query("t")
   end
 
   test "equal timestamps are broken by observed_timestamp_ns then trace_id" do
diff --git a/test/pulso/storage/s3_test.exs b/test/pulso/storage/s3_test.exs
index f385bc5..f8f8688 100644
--- a/test/pulso/storage/s3_test.exs
+++ b/test/pulso/storage/s3_test.exs
@@ -98,13 +98,19 @@ defmodule Pulso.Storage.S3Test do
     assert Enum.map(records, & &1.timestamp_ns) == [4, 3]
   end
 
-  test "populates observed_timestamp_ns when the record does not carry one", %{tenant: tenant} do
-    before_append = System.system_time(:nanosecond)
-    assert :ok = S3.append(tenant, [record(1)])
-    after_append = System.system_time(:nanosecond)
+  test "stores records verbatim without injecting a wall-clock timestamp", %{tenant: tenant} do
+    # Wall-clock backfill would defeat retry idempotency (the second call
+    # under the same idempotency key would overwrite the first with a
+    # later observed_ts). Records that arrive without timestamps are
+    # stored as they came in; queries with time bounds naturally skip
+    # them, unbounded queries return them.
+    assert :ok =
+             S3.append(tenant, [
+               %Log{timestamp_ns: nil, observed_timestamp_ns: nil, body: "no ts"}
+             ])
 
-    assert {:ok, [%Log{observed_timestamp_ns: observed}]} = S3.query(tenant, [])
-    assert observed >= before_append and observed <= after_append
+    assert {:ok, [%Log{timestamp_ns: nil, observed_timestamp_ns: nil, body: "no ts"}]} =
+             S3.query(tenant, [])
   end
 
   test "preserves NDJSON-hostile bodies through the round trip", %{tenant: tenant} do

From 7f6a908574d93f113fd66ee844fa8bd049c76604 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pedro=20Pi=C3=B1era=20Buend=C3=ADa?= <pedro@pepicrft.me>
Date: Thu, 24 Sep 2026 07:15:05 +0200
Subject: [PATCH 13/17] style: satisfy Credo nesting and length/1 checks

Credo failed CI on two "nested too deep" refactor findings and one
"prefer comparing against an empty list" warning.

- Pulso.Storage.S3.sanitize_map: extract per-entry logic into
  `insert_sanitized/2` + `put_sanitized/3` + `sanitize_value/1` so the
  cond block collapses to a single-level dispatch.
- Pulso.OTLP.Logs.decode_resource_logs: extract the inner scope-logs
  and per-record reducers into named helpers.
- test/pulso/storage/s3_test.exs: replace `length(remaining) >= 1`
  with the cheaper `remaining != []`.

Ran `mix credo` (no issues) and `mix test` (73 passed) locally.
---
 lib/pulso/otlp/logs.ex         | 33 ++++++++++-------
 lib/pulso/storage/s3.ex        | 68 +++++++++++++++-------------------
 test/pulso/storage/s3_test.exs |  2 +-
 3 files changed, 51 insertions(+), 52 deletions(-)

diff --git a/lib/pulso/otlp/logs.ex b/lib/pulso/otlp/logs.ex
index 2f3ebb6..b5e2b90 100644
--- a/lib/pulso/otlp/logs.ex
+++ b/lib/pulso/otlp/logs.ex
@@ -43,24 +43,31 @@ defmodule Pulso.OTLP.Logs do
     resource_attrs = attributes(resource_logs["resource"])
     service = resource_attrs["service.name"]
 
-    Enum.reduce(scope_logs, {[], 0}, fn sl, {records, rejected} ->
-      raw = sl["logRecords"] || []
-
-      {sl_records, sl_rejected} =
-        Enum.reduce(raw, {[], 0}, fn record, {rs, rj} ->
-          case decode_log_record(record, resource_attrs, service) do
-            {:ok, r} -> {[r | rs], rj}
-            :error -> {rs, rj + 1}
-          end
-        end)
-
-      {[Enum.reverse(sl_records) | records], rejected + sl_rejected}
-    end)
+    scope_logs
+    |> Enum.reduce({[], 0}, fn sl, acc -> decode_scope_logs(sl, resource_attrs, service, acc) end)
     |> then(fn {records, rejected} -> {records |> Enum.reverse() |> List.flatten(), rejected} end)
   end
 
   defp decode_resource_logs(_), do: {[], 0}
 
+  defp decode_scope_logs(scope_logs, resource_attrs, service, {records, rejected}) do
+    raw = scope_logs["logRecords"] || []
+
+    {sl_records, sl_rejected} =
+      Enum.reduce(raw, {[], 0}, fn record, acc ->
+        decode_and_collect(record, resource_attrs, service, acc)
+      end)
+
+    {[Enum.reverse(sl_records) | records], rejected + sl_rejected}
+  end
+
+  defp decode_and_collect(record, resource_attrs, service, {rs, rj}) do
+    case decode_log_record(record, resource_attrs, service) do
+      {:ok, r} -> {[r | rs], rj}
+      :error -> {rs, rj + 1}
+    end
+  end
+
   # Per the OTLP logs data model, both `time_unix_nano` and
   # `observed_time_unix_nano` MAY be absent, and a value of 0 explicitly
   # means "unknown". The decoder preserves the caller's intent verbatim
diff --git a/lib/pulso/storage/s3.ex b/lib/pulso/storage/s3.ex
index 02dc52b..7eb6abc 100644
--- a/lib/pulso/storage/s3.ex
+++ b/lib/pulso/storage/s3.ex
@@ -151,54 +151,46 @@ defmodule Pulso.Storage.S3 do
   @doc false
   @spec sanitize_map(map()) :: {:ok, map()} | {:error, {:attribute_key_collision, [String.t()]}}
   def sanitize_map(map) when is_map(map) do
-    Enum.reduce_while(map, {:ok, %{}}, fn {k, v}, {:ok, acc} ->
-      string_key = stringify_key(k)
-
-      cond do
-        Map.has_key?(acc, string_key) ->
-          {:halt, {:error, {:attribute_key_collision, [string_key]}}}
-
-        is_map(v) ->
-          case sanitize_map(v) do
-            {:ok, sanitized} -> {:cont, {:ok, Map.put(acc, string_key, sanitized)}}
-            err -> {:halt, err}
-          end
-
-        is_list(v) ->
-          case sanitize_list(v) do
-            {:ok, sanitized} -> {:cont, {:ok, Map.put(acc, string_key, sanitized)}}
-            err -> {:halt, err}
-          end
-
-        true ->
-          {:cont, {:ok, Map.put(acc, string_key, v)}}
-      end
-    end)
+    Enum.reduce_while(map, {:ok, %{}}, &insert_sanitized/2)
   end
 
-  defp sanitize_list(list) do
-    Enum.reduce_while(list, {:ok, []}, fn
-      v, {:ok, acc} when is_map(v) ->
-        case sanitize_map(v) do
-          {:ok, sanitized} -> {:cont, {:ok, [sanitized | acc]}}
-          err -> {:halt, err}
-        end
+  defp insert_sanitized({k, v}, {:ok, acc}) do
+    string_key = stringify_key(k)
 
-      v, {:ok, acc} when is_list(v) ->
-        case sanitize_list(v) do
-          {:ok, sanitized} -> {:cont, {:ok, [sanitized | acc]}}
-          err -> {:halt, err}
-        end
+    if Map.has_key?(acc, string_key) do
+      {:halt, {:error, {:attribute_key_collision, [string_key]}}}
+    else
+      put_sanitized(acc, string_key, v)
+    end
+  end
 
-      v, {:ok, acc} ->
-        {:cont, {:ok, [v | acc]}}
-    end)
+  defp put_sanitized(acc, key, value) do
+    case sanitize_value(value) do
+      {:ok, sanitized} -> {:cont, {:ok, Map.put(acc, key, sanitized)}}
+      err -> {:halt, err}
+    end
+  end
+
+  defp sanitize_value(v) when is_map(v), do: sanitize_map(v)
+  defp sanitize_value(v) when is_list(v), do: sanitize_list(v)
+  defp sanitize_value(v), do: {:ok, v}
+
+  defp sanitize_list(list) do
+    list
+    |> Enum.reduce_while({:ok, []}, &prepend_sanitized/2)
     |> case do
       {:ok, sanitized} -> {:ok, Enum.reverse(sanitized)}
       err -> err
     end
   end
 
+  defp prepend_sanitized(value, {:ok, acc}) do
+    case sanitize_value(value) do
+      {:ok, sanitized} -> {:cont, {:ok, [sanitized | acc]}}
+      err -> {:halt, err}
+    end
+  end
+
   defp stringify_key(k) when is_binary(k), do: k
   defp stringify_key(k) when is_atom(k), do: Atom.to_string(k)
   defp stringify_key(k) when is_integer(k), do: Integer.to_string(k)
diff --git a/test/pulso/storage/s3_test.exs b/test/pulso/storage/s3_test.exs
index f8f8688..2d8f414 100644
--- a/test/pulso/storage/s3_test.exs
+++ b/test/pulso/storage/s3_test.exs
@@ -181,7 +181,7 @@ defmodule Pulso.Storage.S3Test do
 
     # Query should still return the surviving records, not error.
     assert {:ok, remaining} = S3.query(tenant, [])
-    assert length(remaining) >= 1
+    assert remaining != []
   end
 
   test "equal timestamps sort deterministically across adapters", %{tenant: tenant} do

From 25c665efaf729209cd80db9e30f1d8fe08d2edca Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pedro=20Pi=C3=B1era=20Buend=C3=ADa?= <pedro@pepicrft.me>
Date: Thu, 24 Sep 2026 07:23:25 +0200
Subject: [PATCH 14/17] refactor: switch Jason to Elixir's built-in JSON

Elixir 1.18+ ships a `JSON` module with encode!/1, encode_to_iodata!/1,
decode/1, and decode!/1. It covers everything Pulso needs from a JSON
codec, so drop the direct Jason dep and route Phoenix's `:json_library`
to `JSON`. Jason may still show up as a transitive optional dep of
Phoenix or other libraries; that is fine as long as no code in this
repo references it directly.

Callsite changes:

- lib/pulso/mcp/tools.ex, test/pulso/mcp/tools_test.exs,
  lib/pulso/storage/s3.ex: swap `Jason.encode!/1`, `Jason.decode!/1` for
  the JSON equivalents.
- lib/pulso/storage/s3.ex `encode/1`: Elixir's JSON only exposes bang
  encoders, so wrap the call in a try/rescue to keep the
  `{:ok, iodata} | {:error, {:encode_failed, reason}}` contract.
- config/runtime.exs: `Jason.decode/1` -> `JSON.decode/1` (same
  `{:ok, term} | {:error, reason}` shape).
- config/config.exs: `config :phoenix, :json_library, JSON`.
- mix.exs: drop the direct `:jason` dep.
---
 config/config.exs             |  5 +++--
 config/runtime.exs            |  2 +-
 lib/pulso/mcp/tools.ex        |  2 +-
 lib/pulso/storage/s3.ex       | 12 +++++++-----
 mix.exs                       |  1 -
 test/pulso/mcp/tools_test.exs |  6 +++---
 6 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/config/config.exs b/config/config.exs
index 2c33632..99b6a8d 100644
--- a/config/config.exs
+++ b/config/config.exs
@@ -14,8 +14,9 @@ config :logger, :default_formatter,
   format: "$time $metadata[$level] $message\n",
   metadata: [:request_id]
 
-# Use Jason for JSON parsing in Phoenix
-config :phoenix, :json_library, Jason
+# Use Elixir's built-in JSON module for Phoenix (avoids the Jason dependency;
+# see AGENTS.md conventions).
+config :phoenix, :json_library, JSON
 
 # Explicit auth default. Environment-specific configs override; prod requires
 # a runtime override to `Pulso.Auth.SharedSecret` via runtime.exs — an unset
diff --git a/config/runtime.exs b/config/runtime.exs
index 3c6a79a..b6589fb 100644
--- a/config/runtime.exs
+++ b/config/runtime.exs
@@ -64,7 +64,7 @@ case config_env() do
     tokens =
       case System.get_env("PULSO_TENANT_TOKENS") do
         blob when is_binary(blob) and blob != "" ->
-          case Jason.decode(blob) do
+          case JSON.decode(blob) do
             {:ok, map} when is_map(map) ->
               map
 
diff --git a/lib/pulso/mcp/tools.ex b/lib/pulso/mcp/tools.ex
index 7141611..5c7c620 100644
--- a/lib/pulso/mcp/tools.ex
+++ b/lib/pulso/mcp/tools.ex
@@ -57,7 +57,7 @@ defmodule Pulso.MCP.Tools do
 
     with :ok <- verify(context, tenant),
          {:ok, records} <- Storage.query(tenant, opts) do
-      {:ok, [%{"type" => "text", "text" => Jason.encode!(Enum.map(records, &encode_record/1))}]}
+      {:ok, [%{"type" => "text", "text" => JSON.encode!(Enum.map(records, &encode_record/1))}]}
     end
   end
 
diff --git a/lib/pulso/storage/s3.ex b/lib/pulso/storage/s3.ex
index 7eb6abc..5a3a0da 100644
--- a/lib/pulso/storage/s3.ex
+++ b/lib/pulso/storage/s3.ex
@@ -211,9 +211,11 @@ defmodule Pulso.Storage.S3 do
   defp encode(records) do
     encoded =
       Enum.reduce_while(records, {:ok, []}, fn record, {:ok, acc} ->
-        case Jason.encode(Map.from_struct(record)) do
-          {:ok, line} -> {:cont, {:ok, [[line, "\n"] | acc]}}
-          {:error, reason} -> {:halt, {:error, {:encode_failed, reason}}}
+        try do
+          line = JSON.encode!(Map.from_struct(record))
+          {:cont, {:ok, [[line, "\n"] | acc]}}
+        rescue
+          e -> {:halt, {:error, {:encode_failed, e}}}
         end
       end)
 
@@ -247,7 +249,7 @@ defmodule Pulso.Storage.S3 do
   end
 
   defp decode_line(line) do
-    map = Jason.decode!(line)
+    map = JSON.decode!(line)
 
     %Log{
       timestamp_ns: Map.fetch!(map, "timestamp_ns"),
@@ -313,7 +315,7 @@ defmodule Pulso.Storage.S3 do
   # `:erlang.term_to_binary/2` with `:deterministic` gives us the canonical
   # form for free within an OTP release: map keys are sorted, atoms and
   # integers are encoded canonically, and the same term always produces
-  # the same bytes. That is stronger than `Jason.encode/1` (map keys emit
+  # the same bytes. That is stronger than a JSON encoder (map keys emit
   # in `Map.to_list/1` order, which is not canonical and can shift when a
   # small map promotes to a hash map). It is NOT guaranteed across major
   # OTP upgrades — see the module docstring's "Key format stability"
diff --git a/mix.exs b/mix.exs
index f540b9d..3230c4e 100644
--- a/mix.exs
+++ b/mix.exs
@@ -42,7 +42,6 @@ defmodule Pulso.MixProject do
       {:phoenix, "~> 1.8.14"},
       {:telemetry_metrics, "~> 1.0"},
       {:telemetry_poller, "~> 1.0"},
-      {:jason, "~> 1.2"},
       {:dns_cluster, "~> 0.2.0"},
       {:bandit, "~> 1.5"},
       {:req, "~> 0.5"},
diff --git a/test/pulso/mcp/tools_test.exs b/test/pulso/mcp/tools_test.exs
index 7d3bfd1..948ba00 100644
--- a/test/pulso/mcp/tools_test.exs
+++ b/test/pulso/mcp/tools_test.exs
@@ -27,7 +27,7 @@ defmodule Pulso.MCP.ToolsTest do
       ])
 
     assert {:ok, [%{"type" => "text", "text" => text}]} = Tools.call("query_logs", %{"tenant" => "acme"})
-    assert [%{"body" => "two"}, %{"body" => "one"}] = Jason.decode!(text)
+    assert [%{"body" => "two"}, %{"body" => "one"}] = JSON.decode!(text)
   end
 
   test "query_logs applies service and limit filters" do
@@ -41,7 +41,7 @@ defmodule Pulso.MCP.ToolsTest do
     assert {:ok, [%{"text" => text}]} =
              Tools.call("query_logs", %{"tenant" => "acme", "service" => "api", "limit" => 1})
 
-    assert [%{"body" => "c", "service" => "api"}] = Jason.decode!(text)
+    assert [%{"body" => "c", "service" => "api"}] = JSON.decode!(text)
   end
 
   test "query_logs errors when tenant is missing" do
@@ -100,7 +100,7 @@ defmodule Pulso.MCP.ToolsTest do
       assert {:ok, [%{"text" => text}]} =
                Tools.call("query_logs", %{"tenant" => "acme"}, %{conn: conn})
 
-      assert [%{"body" => "ok"}] = Jason.decode!(text)
+      assert [%{"body" => "ok"}] = JSON.decode!(text)
     end
   end
 end

From 88c325eadf410da30e109a4befac424c9406e1fc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pedro=20Pi=C3=B1era=20Buend=C3=ADa?= <pedro@pepicrft.me>
Date: Thu, 24 Sep 2026 07:23:35 +0200
Subject: [PATCH 15/17] ci: run Integration against RustFS via docker compose

The Integration job started failing after the branch swapped MinIO for
RustFS and renamed PULSO_MINIO_* env vars to PULSO_S3_*: the workflow
was still starting MinIO on port 9000 and setting PULSO_MINIO_*
variables that no test reads, so every integration test tried to
connect to http://localhost:11100 (the docker-compose default host
port) and got an "error sending request" failure.

- Replace the ad-hoc `docker run minio` step with
  `docker compose up -d rustfs`, which reuses the checked-in
  docker-compose.yml and therefore stays in sync with local dev.
- Set PULSO_RUSTFS_API_PORT=11100 / PULSO_RUSTFS_CONSOLE_PORT=12100 so
  the compose port interpolation picks them up without depending on the
  mise-derived per-worktree suffix.
- Set PULSO_S3_ENDPOINT/BUCKET/REGION/ACCESS_KEY_ID/SECRET_ACCESS_KEY to
  the values the tests actually read, using RustFS's default admin
  credentials.
- Health-check loop probes the RustFS S3 endpoint on the mapped port
  (accepting 200/403/404 as "listener up").
- Bucket seed uses the AWS CLI with the same env vars, pointing at
  http://localhost:${PULSO_RUSTFS_API_PORT}.
---
 .github/workflows/ci.yml | 40 +++++++++++++++++++++-------------------
 1 file changed, 21 insertions(+), 19 deletions(-)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 02c7b86..92d7dbf 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -74,11 +74,15 @@ jobs:
     env:
       MIX_ENV: test
       PULSO_INTEGRATION: "1"
-      PULSO_MINIO_ENDPOINT: "http://localhost:9000"
-      PULSO_MINIO_BUCKET: "pulso"
-      PULSO_MINIO_REGION: "us-east-1"
-      PULSO_MINIO_ACCESS_KEY_ID: "minioadmin"
-      PULSO_MINIO_SECRET_ACCESS_KEY: "minioadmin"
+      # RustFS in docker-compose binds to these host ports by default (see
+      # mise/utilities/dev_instance_env.sh for the local per-worktree scheme).
+      PULSO_RUSTFS_API_PORT: "11100"
+      PULSO_RUSTFS_CONSOLE_PORT: "12100"
+      PULSO_S3_ENDPOINT: "http://localhost:11100"
+      PULSO_S3_BUCKET: "pulso"
+      PULSO_S3_REGION: "us-east-1"
+      PULSO_S3_ACCESS_KEY_ID: "rustfsadmin"
+      PULSO_S3_SECRET_ACCESS_KEY: "rustfsadmin"
     steps:
       - uses: actions/checkout@v4
 
@@ -105,29 +109,27 @@ jobs:
           restore-keys: |
             ${{ runner.os }}-mix-deps-
 
-      - name: Start MinIO
+      - name: Start RustFS via docker compose
         run: |
-          docker run -d --name minio -p 9000:9000 \
-            -e MINIO_ROOT_USER=minioadmin \
-            -e MINIO_ROOT_PASSWORD=minioadmin \
-            quay.io/minio/minio:latest server /data
-          for _ in $(seq 1 30); do
-            if curl -sf http://localhost:9000/minio/health/live > /dev/null; then
-              echo "MinIO is up"
+          docker compose up -d rustfs
+          for _ in $(seq 1 60); do
+            if curl -sf "http://localhost:${PULSO_RUSTFS_API_PORT}/" > /dev/null 2>&1 \
+              || curl -sf -o /dev/null -w '%{http_code}' "http://localhost:${PULSO_RUSTFS_API_PORT}/" | grep -qE '^(200|403|404)$'; then
+              echo "RustFS is up"
               exit 0
             fi
             sleep 1
           done
-          echo "MinIO did not start in time"
-          docker logs minio
+          echo "RustFS did not start in time"
+          docker compose logs rustfs
           exit 1
 
       - name: Create bucket
         env:
-          AWS_ACCESS_KEY_ID: minioadmin
-          AWS_SECRET_ACCESS_KEY: minioadmin
-          AWS_DEFAULT_REGION: us-east-1
-        run: aws --endpoint-url http://localhost:9000 s3 mb s3://pulso
+          AWS_ACCESS_KEY_ID: ${{ env.PULSO_S3_ACCESS_KEY_ID }}
+          AWS_SECRET_ACCESS_KEY: ${{ env.PULSO_S3_SECRET_ACCESS_KEY }}
+          AWS_DEFAULT_REGION: ${{ env.PULSO_S3_REGION }}
+        run: aws --endpoint-url "http://localhost:${PULSO_RUSTFS_API_PORT}" s3 mb "s3://${PULSO_S3_BUCKET}"
 
       - name: Install dependencies
         run: mix deps.get

From bc9b19c0d78cc92749bfb735b156ce793404c45b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pedro=20Pi=C3=B1era=20Buend=C3=ADa?= <pedro@pepicrft.me>
Date: Thu, 24 Sep 2026 07:23:42 +0200
Subject: [PATCH 16/17] chore: ban Jason via Credo ForbiddenModule and document
 the convention

Jason still shows up as a transitive optional dep of Phoenix and a few
dev tools, so `mix deps.get` cannot fully remove it. Enforce "no direct
Jason usage in our code" at CI time instead.

- .credo.exs: enable Credo.Check.Warning.ForbiddenModule with
  `Jason` and a message pointing the reader at AGENTS.md. Verified
  locally with a throwaway `lib/jason_probe.ex` that referenced
  `Jason.encode!/1` -- Credo flagged it as expected.
- AGENTS.md: replace the "JSON: Jason" convention line with a
  concrete "use Elixir's built-in JSON, direct Jason.* fails CI" note.
---
 .credo.exs | 10 ++++++++++
 AGENTS.md  |  2 +-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/.credo.exs b/.credo.exs
index 3d043d9..678b812 100644
--- a/.credo.exs
+++ b/.credo.exs
@@ -142,6 +142,16 @@
           {Credo.Check.Warning.BoolOperationOnSameValues, []},
           {Credo.Check.Warning.Dbg, []},
           {Credo.Check.Warning.ExpensiveEmptyEnumCheck, []},
+          # Pulso uses Elixir's built-in `JSON` module (Elixir 1.18+); the
+          # `Jason` dependency is only present transitively for optional deps
+          # of other libraries. Any direct `Jason.*` reference in our code
+          # should fail CI. See AGENTS.md > Conventions.
+          {Credo.Check.Warning.ForbiddenModule,
+           [
+             modules: [
+               {Jason, "Use Elixir's built-in `JSON` module instead of Jason (see AGENTS.md)."}
+             ]
+           ]},
           {Credo.Check.Warning.IExPry, []},
           {Credo.Check.Warning.IoInspect, []},
           {Credo.Check.Warning.MissedMetadataKeyInLoggerConfig, []},
diff --git a/AGENTS.md b/AGENTS.md
index 9c0d9c3..bdf0031 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -80,7 +80,7 @@ Storage backend URLs are read from `config :pulso, Pulso.Loki, base_url: ...` an
 ## Conventions
 
 - **HTTP client**: use `Req`. Never `HTTPoison`, `Tesla`, `:httpc`, or `Finch` directly.
-- **JSON**: `Jason` (Phoenix's configured library).
+- **JSON**: use Elixir's built-in `JSON` module (Elixir 1.18+), never `Jason`. Phoenix's `:json_library` is set to `JSON` in `config/config.exs`, and a Credo rule (`Credo.Check.Warning.ForbiddenModule`) fails CI on any direct `Jason.*` reference. Jason may still appear as a transitive dep of `phoenix` or a dev dep, but no code in `lib/`, `config/`, or `test/` may call it.
 - **New backends** go under `Pulso.<Backend>` (e.g. `Pulso.Mimir`, `Pulso.Tempo`), with the same read-only-first shape as `Pulso.Loki`. Every read function must accept a `:base_url` override in opts.
 - **MCP tools** live in `Pulso.MCP.Tools`. Each tool has an `inputSchema`, and its `call/2` clause returns `{:ok, [content_block]}` or `{:error, reason}`. Content blocks follow the MCP shape: `%{"type" => "text", "text" => "..."}`.
 - **Alerting** (when added): each rule is its own supervised process, cluster-wide singleton via Horde. Rules that require exactly-once firing route through `ra`.

From 4c28cea8eabcc366d04379b9168c9e96f0f8abab Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pedro=20Pi=C3=B1era=20Buend=C3=ADa?= <pedro@pepicrft.me>
Date: Thu, 24 Sep 2026 07:28:15 +0200
Subject: [PATCH 17/17] fix(ci): run RustFS on a single volume so CI runners
 can start it

RustFS refuses to start when the volumes in `RUSTFS_VOLUMES` resolve to
the same underlying st_dev (safety check against erasure coding across
one physical disk). Local laptops and GitHub Actions runners both hit
this: every `/data/rustfsN` subdirectory sits on the same block device,
so the 4-volume layout that RustFS ships in its own compose example
fails immediately.

Switch to single-volume `/data/rustfs0`. That matches how Pulso uses
RustFS in dev and CI (single-node, single disk), avoids the erasure
path, and does not require the RUSTFS_UNSAFE_BYPASS_DISK_CHECK escape
hatch.
---
 docker-compose.yml | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/docker-compose.yml b/docker-compose.yml
index 1bdf511..608fc0f 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -15,9 +15,12 @@ services:
   rustfs:
     image: rustfs/rustfs:latest
     environment:
-      # RUSTFS_VOLUMES is required — without it RustFS exits at startup.
-      # The `{0..3}` template creates four storage volumes under /data.
-      RUSTFS_VOLUMES: /data/rustfs{0..3}
+      # RUSTFS_VOLUMES is required — without it RustFS exits at startup. We
+      # run single-volume in dev and CI; the 4-volume erasure layout that
+      # RustFS ships in its own compose expects four distinct physical disks
+      # and refuses to start when they resolve to the same st_dev (any
+      # laptop or CI runner).
+      RUSTFS_VOLUMES: /data/rustfs0
       RUSTFS_ADDRESS: 0.0.0.0:9000
       RUSTFS_CONSOLE_ADDRESS: 0.0.0.0:9001
       RUSTFS_CONSOLE_ENABLE: "true"