From a5ac74db91dafa35300ce5cc1fa827ee5fc24b45 Mon Sep 17 00:00:00 2001
From: Chili Palmer <hugoms@me.com>
Date: Wed, 27 May 2026 19:19:50 +0200
Subject: [PATCH] fix(style): add missing @impl true to all handle_call clauses
 in Publishing GenServer (CSM-036)

---
 CODESMELL.md                       | 10 ++++---
 lib/bds/publishing.ex              |  4 +++
 test/bds/csm036_impl_true_test.exs | 45 ++++++++++++++++++++++++++++++
 3 files changed, 55 insertions(+), 4 deletions(-)
 create mode 100644 test/bds/csm036_impl_true_test.exs

diff --git a/CODESMELL.md b/CODESMELL.md
index 44ad4a3..fc86035 100644
--- a/CODESMELL.md
+++ b/CODESMELL.md
@@ -537,10 +537,12 @@
 
 ---
 
-### CSM-036 — Missing `@impl true` on GenServer Callbacks
-- **File:** `lib/bds/publishing.ex:46,61,71,75`
-- **What:** Only `init/1` (Z. 36) and the first `handle_call` (Z. 41) have `@impl true`. The remaining `handle_call` clauses at Z. 46, 61, 71, 75 lack it.
-- **Fix:** Add `@impl true` before every `handle_call`, `handle_cast`, `handle_info`, and `terminate`.
+### ~~CSM-036 — Missing `@impl true` on GenServer Callbacks~~ ✅ FIXED
+- **Fixed:** 2026-05-27
+- **What was done:**
+  - Added `@impl true` before all four `handle_call` clauses that were missing it in `lib/bds/publishing.ex`: `{:update_job, ...}`, `{:should_upload_scp_file, ...}`, `{:mark_uploaded_scp_file, ...}`, and `{:upload_site, ...}`.
+  - No `handle_cast`, `handle_info`, or `terminate` callbacks exist in this module; only `handle_call` needed fixing.
+  - Added 2 tests in `test/bds/csm036_impl_true_test.exs`: source-level assertion that every `handle_call` clause is preceded by `@impl true`, and a guard test for any future `handle_cast`/`handle_info`/`terminate` callbacks.
 
 ---
 
diff --git a/lib/bds/publishing.ex b/lib/bds/publishing.ex
index bc6dde7..53e61f8 100644
--- a/lib/bds/publishing.ex
+++ b/lib/bds/publishing.ex
@@ -46,6 +46,7 @@ defmodule BDS.Publishing do
     {:reply, Repo.get(PublishJob, job_id), state}
   end
 
+  @impl true
   def handle_call({:update_job, job_id, attrs}, _from, state) do
     with %PublishJob{} = job <- Repo.get(PublishJob, job_id) do
       attrs = Map.put(attrs, :updated_at, Persistence.now_ms())
@@ -55,6 +56,7 @@ defmodule BDS.Publishing do
     {:reply, :ok, state}
   end
 
+  @impl true
   def handle_call({:should_upload_scp_file, upload_key, local_mtime}, _from, state) do
     should_upload? =
       case state.scp_uploads[upload_key] do
@@ -65,10 +67,12 @@ defmodule BDS.Publishing do
     {:reply, should_upload?, state}
   end
 
+  @impl true
   def handle_call({:mark_uploaded_scp_file, upload_key, local_mtime}, _from, state) do
     {:reply, :ok, put_in(state, [:scp_uploads, upload_key], local_mtime)}
   end
 
+  @impl true
   def handle_call({:upload_site, project_id, credentials, targets, opts}, _from, state) do
     job_id = "publish-" <> Integer.to_string(System.unique_integer([:positive, :monotonic]))
     uploader = build_uploader(Keyword.put_new(opts, :project_id, project_id))
diff --git a/test/bds/csm036_impl_true_test.exs b/test/bds/csm036_impl_true_test.exs
new file mode 100644
index 0000000..cc86ba0
--- /dev/null
+++ b/test/bds/csm036_impl_true_test.exs
@@ -0,0 +1,45 @@
+defmodule BDS.CSM036ImplTrueTest do
+  use ExUnit.Case, async: true
+
+  @publishing_source File.read!("lib/bds/publishing.ex")
+
+  describe "CSM-036: @impl true on GenServer callbacks" do
+    test "every handle_call clause has @impl true" do
+      lines = String.split(@publishing_source, "\n")
+
+      handle_call_lines =
+        lines
+        |> Enum.with_index(1)
+        |> Enum.filter(fn {line, _idx} ->
+          String.contains?(line, "def handle_call(")
+        end)
+
+      assert length(handle_call_lines) >= 5, "expected at least 5 handle_call clauses"
+
+      for {_line, idx} <- handle_call_lines do
+        preceding = Enum.at(lines, idx - 2)
+
+        assert String.contains?(preceding, "@impl true"),
+               "handle_call at line #{idx} missing @impl true (preceding line: #{inspect(preceding)})"
+      end
+    end
+
+    test "no handle_cast, handle_info, or terminate without @impl true" do
+      lines = String.split(@publishing_source, "\n")
+
+      callback_lines =
+        lines
+        |> Enum.with_index(1)
+        |> Enum.filter(fn {line, _idx} ->
+          Regex.match?(~r/^\s+def (handle_cast|handle_info|terminate)\(/, line)
+        end)
+
+      for {_line, idx} <- callback_lines do
+        preceding = Enum.at(lines, idx - 2)
+
+        assert String.contains?(preceding, "@impl true"),
+               "callback at line #{idx} missing @impl true"
+      end
+    end
+  end
+end