chore: unit tests for CSM-016

.claude/projects/-Users-gb-Projects-bDS2/memory/MEMORY.md

@@ -0,0 +1 @@
+- [Fix all test failures](feedback_fix_all_failures.md) — Never dismiss failures as pre-existing; fix everything

.claude/projects/-Users-gb-Projects-bDS2/memory/feedback_fix_all_failures.md

@@ -0,0 +1,11 @@
+---
+name: Fix all test failures
+description: Never dismiss test failures as pre-existing — if tests fail after changes, fix them
+type: feedback
+---
+
+All test failures after changes must be fixed, even if they appear unrelated. The test suite was clean before, so any failure is the responsibility of the current change.
+
+**Why:** The user confirmed the suite was green before. Dismissing failures as "pre-existing" is wrong and wastes time.
+
+**How to apply:** After making changes, if any test fails, investigate and fix it before reporting the task as done. Never stash/skip/ignore failures.

CODESMELL.md

@@ -270,15 +270,15 @@
 
 ---
 
-### CSM-016 — String Concatenation for Paths
-- **Files:**
-  - `lib/bds/rendering/metadata.ex:43` — `"/#{slug}/"`
-  - `lib/bds/rendering/metadata.ex:112` — `prefix <> "/"`
-  - `lib/bds/publishing.ex:284` — `String.trim_trailing(path, "/") <> "/"`
-  - `lib/bds/rendering/file_system.ex:29` — `normalized_path <> ".liquid"`
-  - `lib/bds/rendering/links_and_languages.ex` — path construction with `<>`
-- **Fix:** Use `Path.join/1-2` and `Path.extname` / `Path.rootname`. For `"/#{slug}/"`, use `Path.join(["/", slug])` or `"/" <> slug <> "/"` → `URI.encode(slug)` is already used elsewhere.
-- **Test:** Test paths with trailing slashes, empty segments, and special characters.
+### ~~CSM-016 — String Concatenation for Paths~~ ✅ FIXED
+- **Fixed:** 2026-05-09
+- **What was done:**
+  - **`lib/bds/rendering/file_system.ex`** — Extracted `ensure_liquid_ext/1` using `Path.extname/1` to check before appending `.liquid`, preventing double-extension bugs (e.g. `"header.liquid.liquid"`).
+  - **`lib/bds/rendering/metadata.ex`** — `menu_item_href` for `:page` kind now applies `URI.encode/1` to the slug (matching the existing `:category_archive` pattern). `href_for_language/1` now uses `String.trim_trailing(prefix, "/")` before appending `/` to prevent double trailing slashes.
+  - **`lib/bds/rendering/metadata.ex`** — Added `menu_items_from_raw/1` public function for testability.
+  - **`lib/bds/rendering/links_and_languages.ex`** — `post_path/2` for `nil` language now uses `Path.join(["/", year, month, day, slug]) <> "/"` instead of building with `index.html` then stripping it. Language-prefix clause uses `String.trim_trailing/2` to prevent double slashes. `canonical_media_path_by_source_path/1` uses `Path.join("/", media.file_path)` instead of `"/" <> file_path`.
+  - **`lib/bds/publishing.ex`** — `ensure_trailing_slash/1` made public for testability (implementation already correct).
+  - Added 17 tests in `test/bds/csm016_path_concatenation_test.exs`: FileSystem extension handling (bare name, double extension, nested paths), `href_for_language` (empty, with/without trailing slash), menu item href encoding (special chars, plain slugs, category slugs), post_path construction (leading/trailing slashes, no double slashes, language prefix), `language_prefix` (same/nil/different language), `ensure_trailing_slash` (without/with trailing slash, empty string).
 
 ---
 

config/config.exs

@@ -6,6 +6,7 @@ config :bds,
 config :bds, BDS.Repo,
   database: Path.expand("../priv/data/bds_dev.db", __DIR__),
   pool_size: 5,
+  journal_mode: :wal,
   busy_timeout: 15_000,
   log: false,
   stacktrace: true,

config/test.exs

@@ -4,6 +4,7 @@ config :bds, BDS.Repo,
   database: Path.expand("../priv/data/bds_test.db", __DIR__),
   pool: Ecto.Adapters.SQL.Sandbox,
   pool_size: 5,
+  journal_mode: :wal,
   busy_timeout: 15_000
 
 config :logger, level: :warning

lib/bds/publishing.ex

@@ -283,7 +283,7 @@ defmodule BDS.Publishing do
   defp rsync_excludes(%{kind: :media}), do: ["--exclude=*.meta"]
   defp rsync_excludes(_target), do: []
 
-  defp ensure_trailing_slash(path), do: String.trim_trailing(path, "/") <> "/"
+  def ensure_trailing_slash(path), do: String.trim_trailing(path, "/") <> "/"
 
   defp remote_dir_spec(credentials, remote_dir) do
     remote_base(credentials) <> ":" <> ensure_trailing_slash(remote_dir)

lib/bds/rendering/file_system.ex

@@ -25,18 +25,24 @@ defmodule BDS.Rendering.FileSystem do
         raise Liquex.Error, message: "Illegal template path '#{template_path}'"
 
       true ->
+        filename = ensure_liquid_ext(normalized_path)
+
         root_paths
-        |> Enum.map(&Path.expand(Path.join(&1, normalized_path <> ".liquid")))
+        |> Enum.map(&Path.expand(Path.join(&1, filename)))
         |> Enum.find(&File.regular?/1)
         |> case do
           nil ->
-            Path.expand(Path.join(List.first(root_paths) || ".", normalized_path <> ".liquid"))
+            Path.expand(Path.join(List.first(root_paths) || ".", filename))
 
           path ->
             path
         end
     end
   end
+
+  defp ensure_liquid_ext(path) do
+    if Path.extname(path) == ".liquid", do: path, else: path <> ".liquid"
+  end
 end
 
 defimpl Liquex.FileSystem, for: BDS.Rendering.FileSystem do

lib/bds/rendering/links_and_languages.ex

@@ -50,13 +50,13 @@ defmodule BDS.Rendering.LinksAndLanguages do
         ])
         |> String.downcase()
 
-      Map.put(acc, source_key, "/" <> media.file_path)
+      Map.put(acc, source_key, Path.join("/", media.file_path))
     end)
   end
 
   def post_path(post, language_prefix)
       when is_binary(language_prefix) and language_prefix != "" do
-    language_prefix <> post_path(post, nil)
+    String.trim_trailing(language_prefix, "/") <> post_path(post, nil)
   end
 
   def post_path(post, ""), do: post_path(post, nil)
@@ -65,13 +65,12 @@ defmodule BDS.Rendering.LinksAndLanguages do
     datetime = Persistence.from_unix_ms!(post.created_at)
 
     Path.join([
+      "/",
       Integer.to_string(datetime.year),
       String.pad_leading(Integer.to_string(datetime.month), 2, "0"),
       String.pad_leading(Integer.to_string(datetime.day), 2, "0"),
-      post.slug,
-      "index.html"
-    ])
-    |> then(&("/" <> String.trim_trailing(&1, "index.html")))
+      post.slug
+    ]) <> "/"
   end
 
   def post_path(post, language, main_language) do

lib/bds/rendering/metadata.ex

@@ -24,6 +24,10 @@ defmodule BDS.Rendering.Metadata do
     Enum.map(items, &to_template_menu_item/1)
   end
 
+  def menu_items_from_raw(items) when is_list(items) do
+    Enum.map(items, &to_template_menu_item/1)
+  end
+
   defp to_template_menu_item(item) do
     kind = Map.get(item, :kind)
     children = Enum.map(Map.get(item, :children, []), &to_template_menu_item/1)
@@ -40,7 +44,7 @@ defmodule BDS.Rendering.Metadata do
   defp menu_item_href(%{kind: :home}), do: "/"
 
   defp menu_item_href(%{kind: :page, slug: slug}) when is_binary(slug) and slug != "",
-    do: "/#{slug}/"
+    do: "/#{URI.encode(slug)}/"
 
   defp menu_item_href(%{kind: :category_archive, slug: slug}) when is_binary(slug) and slug != "",
     do: "/category/#{URI.encode(slug)}/"
@@ -109,7 +113,7 @@ defmodule BDS.Rendering.Metadata do
   def default_pico_stylesheet_href(theme), do: PreviewAssets.stylesheet_href(theme)
 
   def href_for_language(""), do: "/"
-  def href_for_language(prefix), do: prefix <> "/"
+  def href_for_language(prefix), do: String.trim_trailing(prefix, "/") <> "/"
 
   def calendar_initial_year(%{created_at: created_at}) when is_integer(created_at),
     do: Persistence.from_unix_ms!(created_at).year

test/bds/csm007_reload_shell_test.exs

@@ -8,6 +8,7 @@ defmodule BDS.CSM007ReloadShellTest do
 
   setup do
     :ok = Ecto.Adapters.SQL.Sandbox.checkout(BDS.Repo)
+    Ecto.Adapters.SQL.Sandbox.mode(BDS.Repo, {:shared, self()})
 
     temp_dir =
       Path.join(System.tmp_dir!(), "bds-csm007-#{System.unique_integer([:positive])}")

test/bds/csm008_render_path_test.exs

@@ -8,6 +8,7 @@ defmodule BDS.CSM008RenderPathTest do
 
   setup do
     :ok = Ecto.Adapters.SQL.Sandbox.checkout(BDS.Repo)
+    Ecto.Adapters.SQL.Sandbox.mode(BDS.Repo, {:shared, self()})
 
     temp_dir =
       Path.join(System.tmp_dir!(), "bds-csm008-#{System.unique_integer([:positive])}")

test/bds/csm011_url_state_test.exs

@@ -8,6 +8,7 @@ defmodule BDS.CSM011UrlStateTest do
 
   setup do
     :ok = Ecto.Adapters.SQL.Sandbox.checkout(BDS.Repo)
+    Ecto.Adapters.SQL.Sandbox.mode(BDS.Repo, {:shared, self()})
 
     prev = System.get_env("BDS_DESKTOP_AUTOMATION")
     System.put_env("BDS_DESKTOP_AUTOMATION", "1")

test/bds/csm012_file_picker_async_test.exs

@@ -8,6 +8,7 @@ defmodule BDS.CSM012FilePickerAsyncTest do
 
   setup do
     :ok = Ecto.Adapters.SQL.Sandbox.checkout(BDS.Repo)
+    Ecto.Adapters.SQL.Sandbox.mode(BDS.Repo, {:shared, self()})
 
     prev = System.get_env("BDS_DESKTOP_AUTOMATION")
     System.put_env("BDS_DESKTOP_AUTOMATION", "1")

test/bds/csm016_path_concatenation_test.exs

@@ -0,0 +1,136 @@
+defmodule BDS.CSM016PathConcatenationTest do
+  use ExUnit.Case, async: false
+
+  alias BDS.Rendering.FileSystem, as: TemplateFileSystem
+  alias BDS.Rendering.LinksAndLanguages
+  alias BDS.Rendering.Metadata
+
+  setup do
+    :ok = Ecto.Adapters.SQL.Sandbox.checkout(BDS.Repo)
+
+    temp_dir = Path.join(System.tmp_dir!(), "bds-csm016-#{System.unique_integer([:positive])}")
+    File.mkdir_p!(temp_dir)
+    on_exit(fn -> File.rm_rf(temp_dir) end)
+
+    %{temp_dir: temp_dir}
+  end
+
+  describe "FileSystem.full_path/2 extension handling" do
+    test "adds .liquid extension to bare template name", %{temp_dir: temp_dir} do
+      File.write!(Path.join(temp_dir, "header.liquid"), "")
+      fs = TemplateFileSystem.new(temp_dir)
+      assert String.ends_with?(TemplateFileSystem.full_path(fs, "header"), ".liquid")
+    end
+
+    test "does not double .liquid extension", %{temp_dir: temp_dir} do
+      File.write!(Path.join(temp_dir, "header.liquid"), "")
+      fs = TemplateFileSystem.new(temp_dir)
+      path = TemplateFileSystem.full_path(fs, "header.liquid")
+      refute String.ends_with?(path, ".liquid.liquid")
+      assert String.ends_with?(path, ".liquid")
+    end
+
+    test "handles nested template paths", %{temp_dir: temp_dir} do
+      nested_dir = Path.join(temp_dir, "partials")
+      File.mkdir_p!(nested_dir)
+      File.write!(Path.join(nested_dir, "footer.liquid"), "")
+      fs = TemplateFileSystem.new(temp_dir)
+      path = TemplateFileSystem.full_path(fs, "partials/footer")
+      assert String.ends_with?(path, "partials/footer.liquid")
+    end
+  end
+
+  describe "Metadata.href_for_language/1" do
+    test "empty prefix returns root" do
+      assert Metadata.href_for_language("") == "/"
+    end
+
+    test "prefix without trailing slash gets one" do
+      assert Metadata.href_for_language("/de") == "/de/"
+    end
+
+    test "prefix with trailing slash does not get double slash" do
+      assert Metadata.href_for_language("/de/") == "/de/"
+    end
+  end
+
+  describe "Metadata menu_item_href via to_template_menu_item" do
+    test "page slug with special characters is encoded" do
+      items = Metadata.menu_items_from_raw([%{kind: :page, slug: "über uns", children: []}])
+      [item] = items
+      assert item.href == "/%C3%BCber%20uns/"
+    end
+
+    test "page slug without special chars is unchanged" do
+      items = Metadata.menu_items_from_raw([%{kind: :page, slug: "about", children: []}])
+      [item] = items
+      assert item.href == "/about/"
+    end
+
+    test "category archive slug is encoded" do
+      items =
+        Metadata.menu_items_from_raw([
+          %{kind: :category_archive, slug: "café", children: []}
+        ])
+
+      [item] = items
+      assert item.href == "/category/caf%C3%A9/"
+    end
+  end
+
+  describe "LinksAndLanguages.post_path/2 construction" do
+    test "produces correct path with leading slash and trailing slash" do
+      post = %{
+        slug: "hello-world",
+        created_at: 1_746_792_000_000
+      }
+
+      path = LinksAndLanguages.post_path(post, nil)
+      assert String.starts_with?(path, "/")
+      assert String.ends_with?(path, "/")
+      refute String.contains?(path, "//")
+    end
+
+    test "language prefix path has no double slashes" do
+      post = %{
+        slug: "hello-world",
+        created_at: 1_746_792_000_000
+      }
+
+      path = LinksAndLanguages.post_path(post, "/de")
+      assert String.starts_with?(path, "/de/")
+      refute String.contains?(path, "//")
+    end
+  end
+
+  describe "LinksAndLanguages.language_prefix/2" do
+    test "same language returns empty string" do
+      assert LinksAndLanguages.language_prefix("en", "en") == ""
+    end
+
+    test "nil language returns empty string" do
+      assert LinksAndLanguages.language_prefix(nil, "en") == ""
+    end
+
+    test "different language returns prefix with leading slash" do
+      prefix = LinksAndLanguages.language_prefix("de", "en")
+      assert prefix == "/de"
+      assert String.starts_with?(prefix, "/")
+      refute String.ends_with?(prefix, "/")
+    end
+  end
+
+  describe "publishing ensure_trailing_slash" do
+    test "path without trailing slash gets one" do
+      assert BDS.Publishing.ensure_trailing_slash("/var/www") == "/var/www/"
+    end
+
+    test "path with trailing slash keeps exactly one" do
+      assert BDS.Publishing.ensure_trailing_slash("/var/www/") == "/var/www/"
+    end
+
+    test "empty string gets a slash" do
+      assert BDS.Publishing.ensure_trailing_slash("") == "/"
+    end
+  end
+end