From 590203a2933ac25f9f2c2faf80226b5106f895fa Mon Sep 17 00:00:00 2001
From: Bryan Van Deusen <bryan.vandeusen@fabledsword.com>
Date: Sat, 22 Aug 2026 15:07:23 -0400
Subject: [PATCH] refactor(tests+frontend): one http_sink helper for the hook
 tests; apiErrorMessage replaces ten hand-rolled error-body parses; `type X,`
 import specifiers are not definitions (#2904, milestone 299 step 6)

tests/helpers.http_sink replaces three module-local _Sink handlers (the
write-path tests and the after-write test). ProjectView + SettingsView
parsed `(e as {body?:{error?}}).body?.error || fallback` by hand ten times
beside the apiErrorMessage canon (#2853) - all ten now call it. The
extractor (server + the hook awk mirror) no longer reads `import { type Foo }`
as a definition of Foo - that was the last "identical body" sym family.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---
 frontend/src/views/ProjectView.vue  |  5 +-
 frontend/src/views/SettingsView.vue | 27 +++------
 plugin/hooks/scribe_defs.sh         |  7 +++
 src/scribe/services/coverage.py     |  6 ++
 tests/helpers.py                    | 37 ++++++++++++
 tests/test_after_write_hook.py      | 94 +++++++++++------------------
 tests/test_pattern_coverage.py      | 15 +++++
 tests/test_write_path_trigger.py    | 79 ++++++++----------------
 8 files changed, 137 insertions(+), 133 deletions(-)

diff --git a/frontend/src/views/ProjectView.vue b/frontend/src/views/ProjectView.vue
index 61b3a12..ed5cc02 100644
--- a/frontend/src/views/ProjectView.vue
+++ b/frontend/src/views/ProjectView.vue
@@ -1,7 +1,7 @@
 <script setup lang="ts">
 import { ref, computed, onMounted, watch } from "vue";
 import { useRoute, useRouter } from "vue-router";
-import { apiGet, apiPatch, apiDelete, apiPost, apiPut } from "@/api/client";
+import { apiGet, apiPatch, apiDelete, apiPost, apiPut, apiErrorMessage } from "@/api/client";
 import { useAuthStore } from "@/stores/auth";
 import { useToastStore } from "@/stores/toast";
 import { useTasksStore } from "@/stores/tasks";
@@ -543,8 +543,7 @@ async function saveForgePin() {
     if (project.value) project.value.forge_connection_id = forgePin.value;
     await loadCoverage();
   } catch (e) {
-    const body = (e as { body?: { error?: string } }).body;
-    toast.show(body?.error || "Failed to change the project's forge", "error");
+    toast.show(apiErrorMessage(e, "Failed to change the project's forge"), "error");
     forgePin.value = project.value?.forge_connection_id ?? null;
   } finally {
     savingForgePin.value = false;
diff --git a/frontend/src/views/SettingsView.vue b/frontend/src/views/SettingsView.vue
index 27540a7..6df3462 100644
--- a/frontend/src/views/SettingsView.vue
+++ b/frontend/src/views/SettingsView.vue
@@ -780,8 +780,7 @@ async function saveConnection() {
     connFormOpen.value = false;
     await loadForgeConnections();
   } catch (e) {
-    const body = (e as { body?: { error?: string } }).body;
-    toastStore.show(body?.error || "Failed to save forge connection", "error");
+    toastStore.show(apiErrorMessage(e, "Failed to save forge connection"), "error");
   } finally {
     savingConn.value = false;
   }
@@ -793,8 +792,7 @@ async function removeConnection(id: number) {
     connTestResult.value = null;
     await loadForgeConnections();
   } catch (e) {
-    const body = (e as { body?: { error?: string } }).body;
-    toastStore.show(body?.error || "Failed to delete forge connection", "error");
+    toastStore.show(apiErrorMessage(e, "Failed to delete forge connection"), "error");
   }
 }
 
@@ -810,10 +808,9 @@ async function testConnection(id: number) {
       message: `Connected — ${res.version}, authenticated as ${res.username}`,
     };
   } catch (e) {
-    const body = (e as { body?: { error?: string } }).body;
     connTestResult.value = {
       id, ok: false,
-      message: body?.error || "Connection test failed",
+      message: apiErrorMessage(e, "Connection test failed"),
     };
   } finally {
     testingConnId.value = 0;
@@ -830,8 +827,7 @@ async function saveForgeWebhook() {
     forgeWebhookSaved.value = true;
     setTimeout(() => (forgeWebhookSaved.value = false), 2000);
   } catch (e) {
-    const body = (e as { body?: { error?: string } }).body;
-    toastStore.show(body?.error || "Failed to save webhook secret", "error");
+    toastStore.show(apiErrorMessage(e, "Failed to save webhook secret"), "error");
   } finally {
     savingForgeWebhook.value = false;
   }
@@ -864,8 +860,7 @@ async function saveMarketplaceUrl() {
     marketplaceUrlSaved.value = true;
     setTimeout(() => (marketplaceUrlSaved.value = false), 2000);
   } catch (e) {
-    const body = (e as { body?: { error?: string } }).body;
-    toastStore.show(body?.error || "Failed to save marketplace URL", "error");
+    toastStore.show(apiErrorMessage(e, "Failed to save marketplace URL"), "error");
   } finally {
     savingMarketplaceUrl.value = false;
   }
@@ -882,8 +877,7 @@ async function saveDbMaintenance() {
     dbMaintSaved.value = true;
     setTimeout(() => (dbMaintSaved.value = false), 2000);
   } catch (e) {
-    const body = (e as { body?: { error?: string } }).body;
-    toastStore.show(body?.error || "Failed to save maintenance settings", "error");
+    toastStore.show(apiErrorMessage(e, "Failed to save maintenance settings"), "error");
   } finally {
     savingDbMaint.value = false;
   }
@@ -912,8 +906,7 @@ async function runDbMaintenanceNow() {
     );
     await loadDbHealth(); // reflect the dead-tuple drop
   } catch (e) {
-    const body = (e as { body?: { error?: string } }).body;
-    toastStore.show(body?.error || "Maintenance run failed", "error");
+    toastStore.show(apiErrorMessage(e, "Maintenance run failed"), "error");
   } finally {
     runningDbMaint.value = false;
   }
@@ -1144,8 +1137,7 @@ async function sendInvite() {
     inviteEmail.value = "";
     await fetchInvitations();
   } catch (e: unknown) {
-    const body = (e as { body?: { error?: string } })?.body;
-    toastStore.show(body?.error || "Failed to send invitation", "error");
+    toastStore.show(apiErrorMessage(e, "Failed to send invitation"), "error");
   } finally {
     sendingInvite.value = false;
   }
@@ -1196,8 +1188,7 @@ async function deleteUser(userId: number) {
     users.value = users.value.filter((u) => u.id !== userId);
     toastStore.show("User deleted");
   } catch (e: unknown) {
-    const body = (e as { body?: { error?: string } })?.body;
-    toastStore.show(body?.error || "Failed to delete user", "error");
+    toastStore.show(apiErrorMessage(e, "Failed to delete user"), "error");
   } finally {
     deleting.value = null;
   }
diff --git a/plugin/hooks/scribe_defs.sh b/plugin/hooks/scribe_defs.sh
index c9713f2..61991bc 100644
--- a/plugin/hooks/scribe_defs.sh
+++ b/plugin/hooks/scribe_defs.sh
@@ -58,6 +58,13 @@ scribe_defs() {
       if (match(line, /^(function|def|class|func|fun|fn|sub|struct|trait|interface|enum|object|protocol|type)[[:space:]]+[A-Za-z_$]/)) {
         t = line; sub(/^[a-z]+[[:space:]]+/, "", t)
         sub(/[^A-Za-z0-9_$].*$/, "", t)
+        # `type` defines only when something follows the name (= or {); an
+        # import specifier `type Foo,` is the same two words and defines
+        # nothing (mirror of coverage.py, #2904).
+        if (line ~ /^type[[:space:]]/) {
+          rest = line; sub(/^type[[:space:]]+[A-Za-z_$][A-Za-z0-9_$]*/, "", rest)
+          if (rest !~ /[={]/) next
+        }
         if (t != "" && t !~ /^__.*__$/) print "sym\t" t; next
       }
       # Arrow/expression assignment: const name = (…) / let name = async (
diff --git a/src/scribe/services/coverage.py b/src/scribe/services/coverage.py
index eabda24..0695d29 100644
--- a/src/scribe/services/coverage.py
+++ b/src/scribe/services/coverage.py
@@ -123,6 +123,12 @@ def _definition_on(raw: str) -> tuple[str, str] | None:
         name = m.group(1)
         if name.startswith("__") and name.endswith("__"):
             return None
+        # `type` announces a definition only when something is declared after
+        # the name (`type Foo = …`, `type Foo struct {`); an import specifier
+        # (`import { type Foo, bar }`) is the same two words and defines
+        # nothing — it showed up as a two-file "identical body" family (#2904).
+        if line.startswith("type") and not re.search(r"[={]", line[m.end():]):
+            return None
         return ("sym", name)
     if m := _ARROW_RE.match(line):
         return ("sym", m.group(1))
diff --git a/tests/helpers.py b/tests/helpers.py
index 862c781..757e542 100644
--- a/tests/helpers.py
+++ b/tests/helpers.py
@@ -6,6 +6,7 @@ them; a module imports what it needs with ``from tests.helpers import ...``.
 """
 from __future__ import annotations
 
+from contextlib import contextmanager
 from datetime import datetime, timezone
 from types import SimpleNamespace
 from unittest.mock import AsyncMock, MagicMock
@@ -182,3 +183,39 @@ def design_token_stub(name, value_by_mode, group_name=None, purpose=None,
         name=name, value_by_mode=value_by_mode, group_name=group_name,
         purpose=purpose, order_index=order_index, supersedes=supersedes or [],
     )
+
+
+@contextmanager
+def http_sink(reply: bytes = b'{"context":"","note_ids":[]}'):
+    """A throwaway local HTTP listener for hook end-to-end tests: yields
+    ``(port, seen)`` where ``seen`` collects every GET's parsed query string
+    (one dict per request, in order). Lets the shell be tested end to end —
+    the extraction, the encoding, the URL — without a Scribe instance.
+
+    Three test modules each carried their own ``_Sink`` handler before #2904
+    consolidated them here; pass ``reply`` for the body the hook should see.
+    """
+    import http.server
+    import threading
+    import urllib.parse
+
+    seen: list[dict] = []
+
+    class _Sink(http.server.BaseHTTPRequestHandler):
+        def do_GET(self):
+            seen.append(urllib.parse.parse_qs(urllib.parse.urlparse(self.path).query))
+            self.send_response(200)
+            self.send_header("Content-Type", "application/json")
+            self.end_headers()
+            self.wfile.write(reply)
+
+        def log_message(self, *a):
+            pass
+
+    server = http.server.HTTPServer(("127.0.0.1", 0), _Sink)
+    threading.Thread(target=server.serve_forever, daemon=True).start()
+    try:
+        yield server.server_port, seen
+    finally:
+        server.shutdown()
+        server.server_close()
diff --git a/tests/test_after_write_hook.py b/tests/test_after_write_hook.py
index f536a42..5a8ec7e 100644
--- a/tests/test_after_write_hook.py
+++ b/tests/test_after_write_hook.py
@@ -6,17 +6,16 @@ the pre-write hook's end-to-end tests. Skips where the hook's tools are
 missing; asserts on content where they are present."""
 from __future__ import annotations
 
-import http.server
 import json
 import os
 import shutil
 import subprocess
-import threading
-import urllib.parse
 from pathlib import Path
 
 import pytest
 
+from tests.helpers import http_sink
+
 PLUGIN = Path(__file__).resolve().parents[1] / "plugin"
 HOOK = PLUGIN / "hooks" / "scribe_after_write.sh"
 
@@ -53,66 +52,43 @@ def _run(repo, env, session="s-after-1", tool="Bash"):
     return out.stdout
 
 
-class _Sink(http.server.BaseHTTPRequestHandler):
-    seen: list[dict] = []
-    reply = b'{"context":"> family named","note_ids":[],"sync_note_ids":[],"derive_keys":["dup:483a"]}'
-
-    def do_GET(self):
-        type(self).seen.append(urllib.parse.parse_qs(urllib.parse.urlparse(self.path).query))
-        self.send_response(200)
-        self.send_header("Content-Type", "application/json")
-        self.end_headers()
-        self.wfile.write(type(self).reply)
-
-    def log_message(self, *a):
-        pass
+SINK_REPLY = b'{"context":"> family named","note_ids":[],"sync_note_ids":[],"derive_keys":["dup:483a"]}'
 
 
-@pytest.fixture
-def sink():
-    _Sink.seen = []
-    server = http.server.HTTPServer(("127.0.0.1", 0), _Sink)
-    threading.Thread(target=server.serve_forever, daemon=True).start()
-    try:
-        yield server
-    finally:
-        server.shutdown()
+def test_after_write_names_what_bash_just_wrote_then_stays_quiet_until_the_next_change(tmp_path):
+    with http_sink(SINK_REPLY) as (port, seen):
+        env = _env(tmp_path, url=f"http://127.0.0.1:{port}")
+        repo = _repo(tmp_path, env)
+        # "A Bash call" wrote an untracked stylesheet and appended to a tracked file.
+        (repo / "a.css").write_text(".log-empty {\n  color: red;\n}\n")
+        (repo / "b.py").write_text("def one():\n    return 1\n\ndef slug(t):\n    return t\n")
+        out = _run(repo, env)
+        by_path = {q["path"][0]: q for q in seen}
+        assert set(by_path) == {"a.css", "b.py"}           # repo-relative, like the pre hook
+        assert by_path["a.css"]["shapes"] == ["css:log-empty"]
+        assert by_path["b.py"]["shapes"] == ["sym:slug"]
+        # Added lines only for the tracked file — the existing def is not "just written".
+        assert "def slug" in by_path["b.py"]["code"][0] and "def one" not in by_path["b.py"]["code"][0]
+        ctx = json.loads(out)["hookSpecificOutput"]
+        assert ctx["hookEventName"] == "PostToolUse"
+        assert "> family named" in ctx["additionalContext"]
+        # The local by-name arm rides along: `slug` already lives in c.py.
+        assert "`slug` is already defined in 1 other file(s): c.py" in ctx["additionalContext"]
+        # Derive keys landed on the SHARED channel the pre-write hook reads.
+        state = tmp_path / "scribe-priorart" / "s-after-1.derive.ids"
+        assert "dup:483a" in state.read_text().split()
 
+        # Nothing changed → one git status, no request, no output.
+        seen.clear()
+        assert _run(repo, env) == ""
+        assert seen == []
 
-def test_after_write_names_what_bash_just_wrote_then_stays_quiet_until_the_next_change(tmp_path, sink):
-    env = _env(tmp_path, url=f"http://127.0.0.1:{sink.server_port}")
-    repo = _repo(tmp_path, env)
-    # "A Bash call" wrote an untracked stylesheet and appended to a tracked file.
-    (repo / "a.css").write_text(".log-empty {\n  color: red;\n}\n")
-    (repo / "b.py").write_text("def one():\n    return 1\n\ndef slug(t):\n    return t\n")
-    out = _run(repo, env)
-    by_path = {q["path"][0]: q for q in _Sink.seen}
-    assert set(by_path) == {"a.css", "b.py"}           # repo-relative, like the pre hook
-    assert by_path["a.css"]["shapes"] == ["css:log-empty"]
-    assert by_path["b.py"]["shapes"] == ["sym:slug"]
-    # Added lines only for the tracked file — the existing def is not "just written".
-    assert "def slug" in by_path["b.py"]["code"][0] and "def one" not in by_path["b.py"]["code"][0]
-    ctx = json.loads(out)["hookSpecificOutput"]
-    assert ctx["hookEventName"] == "PostToolUse"
-    assert "> family named" in ctx["additionalContext"]
-    # The local by-name arm rides along: `slug` already lives in c.py.
-    assert "`slug` is already defined in 1 other file(s): c.py" in ctx["additionalContext"]
-    # Derive keys landed on the SHARED channel the pre-write hook reads.
-    state = tmp_path / "scribe-priorart" / "s-after-1.derive.ids"
-    assert "dup:483a" in state.read_text().split()
-
-    # Nothing changed → one git status, no request, no output.
-    _Sink.seen = []
-    assert _run(repo, env) == ""
-    assert _Sink.seen == []
-
-    # Another change → only that file, and the dedup channel goes back up.
-    (repo / "a.css").write_text(".log-empty {\n  color: red;\n}\n.other {\n  margin: 0;\n}\n")
-    _run(repo, env)
-    assert [q["path"][0] for q in _Sink.seen] == ["a.css"]
-    assert _Sink.seen[0]["exclude_derive"] == ["dup:483a"]
-    assert set(_Sink.seen[0]["shapes"][0].split(",")) == {"css:log-empty", "css:other"}
-
+        # Another change → only that file, and the dedup channel goes back up.
+        (repo / "a.css").write_text(".log-empty {\n  color: red;\n}\n.other {\n  margin: 0;\n}\n")
+        _run(repo, env)
+        assert [q["path"][0] for q in seen] == ["a.css"]
+        assert seen[0]["exclude_derive"] == ["dup:483a"]
+        assert set(seen[0]["shapes"][0].split(",")) == {"css:log-empty", "css:other"}
 
 def test_after_write_is_silent_where_it_has_nothing_to_say(tmp_path):
     env = _env(tmp_path)
diff --git a/tests/test_pattern_coverage.py b/tests/test_pattern_coverage.py
index 24bb0d5..54fa989 100644
--- a/tests/test_pattern_coverage.py
+++ b/tests/test_pattern_coverage.py
@@ -149,6 +149,21 @@ def test_largest_gaps_ranks_by_unclassified_and_drops_clean_dirs():
     assert gaps == [{"dir": "src", "unclassified": 2, "total": 3}]
 
 
+def test_type_import_specifiers_are_not_definitions():
+    """#2904: `import { type Foo, bar }` is the same two words as `type Foo =`
+    and defines nothing; only a `type` line with a declaration after the
+    name counts (TS alias, Go/Rust type)."""
+    from scribe.services.coverage import extract_shapes
+    src = (
+        'import { type DesignSystem, fetchDesignSystems } from "@/api/designSystems";\n'
+        'import { type Project } from "./x";\n'
+        "type Baz = { a: number };\n"
+        "type Wide<T> = T | null;\n"
+        "type Point struct {\n\tX int\n}\n"
+    )
+    assert extract_shapes(src) == [("sym", "Baz"), ("sym", "Wide"), ("sym", "Point")]
+
+
 def test_coverage_line_is_evidence_carrying_and_labeled_estimate():
     line = coverage_line({
         "total": 4573, "accounted": 3100, "unclassified": 1473,
diff --git a/tests/test_write_path_trigger.py b/tests/test_write_path_trigger.py
index c71fb15..dbbd923 100644
--- a/tests/test_write_path_trigger.py
+++ b/tests/test_write_path_trigger.py
@@ -13,7 +13,7 @@ from pathlib import Path
 from unittest.mock import AsyncMock, MagicMock, patch
 
 import pytest
-from tests.helpers import fake_note
+from tests.helpers import fake_note, http_sink
 
 PLUGIN = Path(__file__).resolve().parents[1] / "plugin"
 HOOK = PLUGIN / "hooks" / "scribe_prior_art.sh"
@@ -1149,6 +1149,23 @@ def test_route_stamps_only_for_a_caller_allowed_to_write():
     assert "&shapes=" in hook
 
 
+def test_hook_does_not_name_a_type_import_specifier_as_a_shape(tmp_path):
+    """#2904, the awk mirror of the server rule: `type Foo,` inside an import
+    list is not a definition; `type Baz = …` on its own line is."""
+    env = _hook_runtime_env()
+    repo = tmp_path / "repo"
+    repo.mkdir()
+    subprocess.run(["git", "init", "-q"], cwd=repo, check=True, env=env)
+    seen = _run_hook_against_sink(tmp_path, {
+        "session_id": "s-type", "cwd": str(repo), "tool_name": "Write",
+        "tool_input": {"file_path": str(repo / "x.ts"),
+                       "content": 'import { type Foo, bar } from "./y";\n'
+                                  "type Baz = { a: number };\n"
+                                  "export function use(): Baz {\n  return { a: 1 };\n}\n"},
+    })
+    assert seen["shapes"] == ["sym:Baz,sym:use"]
+
+
 def test_hook_names_the_shapes_being_written():
     """The feed's two inputs: every definition in the payload, or — for an Edit
     that changes a body, not a signature — the definition enclosing the edit,
@@ -1165,39 +1182,15 @@ def test_hook_names_the_shapes_being_written():
 
 def _run_hook_against_sink(tmp_path, payload):
     """Run the hook with SCRIBE_URL pointed at a throwaway local listener and
-    return the query the hook sent. Lets the shell be tested end to end —
-    the extraction, the encoding, the URL — without a Scribe instance."""
-    import http.server
-    import threading
-    import urllib.parse
-
-    seen: dict = {}
-
-    class _Sink(http.server.BaseHTTPRequestHandler):
-        def do_GET(self):
-            seen.update(urllib.parse.parse_qs(urllib.parse.urlparse(self.path).query))
-            self.send_response(200)
-            self.send_header("Content-Type", "application/json")
-            self.end_headers()
-            self.wfile.write(b'{"context":"","note_ids":[]}')
-
-        def log_message(self, *a):
-            pass
-
-    server = http.server.HTTPServer(("127.0.0.1", 0), _Sink)
-    thread = threading.Thread(target=server.serve_forever, daemon=True)
-    thread.start()
-    try:
-        env = dict(_hook_runtime_env(), SCRIBE_URL=f"http://127.0.0.1:{server.server_port}")
+    return the query the hook sent (tests.helpers.http_sink)."""
+    with http_sink() as (port, seen):
+        env = dict(_hook_runtime_env(), SCRIBE_URL=f"http://127.0.0.1:{port}")
         out = subprocess.run(
             ["bash", str(HOOK)], input=json.dumps(payload),
             capture_output=True, text=True, env=env,
         )
         assert out.returncode == 0, out.stderr
-    finally:
-        server.shutdown()
-        server.server_close()
-    return seen
+    return seen[0] if seen else {}
 
 
 def test_hook_sends_every_definition_in_a_write(tmp_path):
@@ -1321,28 +1314,10 @@ def test_the_hook_keeps_a_derive_channel_and_sends_it_back(tmp_path):
     """#2900: derive keys the server returns land in the session's own
     `.derive.ids` file and go back as `exclude_derive` on the next write —
     a family is named once per session, not at every edit."""
-    import http.server
-    import threading
-    import urllib.parse
-
-    seen: list[dict] = []
-
-    class _Sink(http.server.BaseHTTPRequestHandler):
-        def do_GET(self):
-            seen.append(urllib.parse.parse_qs(urllib.parse.urlparse(self.path).query))
-            self.send_response(200)
-            self.send_header("Content-Type", "application/json")
-            self.end_headers()
-            self.wfile.write(b'{"context":"> family","note_ids":[],"sync_note_ids":[],'
-                             b'"derive_keys":["dup:483a","canon:2855"]}')
-
-        def log_message(self, *a):
-            pass
-
-    server = http.server.HTTPServer(("127.0.0.1", 0), _Sink)
-    threading.Thread(target=server.serve_forever, daemon=True).start()
-    try:
-        env = dict(_hook_runtime_env(), SCRIBE_URL=f"http://127.0.0.1:{server.server_port}",
+    reply = (b'{"context":"> family","note_ids":[],"sync_note_ids":[],'
+             b'"derive_keys":["dup:483a","canon:2855"]}')
+    with http_sink(reply) as (port, seen):
+        env = dict(_hook_runtime_env(), SCRIBE_URL=f"http://127.0.0.1:{port}",
                    TMPDIR=str(tmp_path))
         payload = {"session_id": "s-derive-1", "cwd": str(tmp_path), "tool_name": "Write",
                    "tool_input": {"file_path": str(tmp_path / "a.css"),
@@ -1351,8 +1326,6 @@ def test_the_hook_keeps_a_derive_channel_and_sends_it_back(tmp_path):
             out = subprocess.run(["bash", str(HOOK)], input=json.dumps(payload),
                                  capture_output=True, text=True, env=env)
             assert out.returncode == 0, out.stderr
-    finally:
-        server.shutdown()
     assert "exclude_derive" not in seen[0]
     assert seen[1]["exclude_derive"] == ["dup:483a,canon:2855"]
     state = tmp_path / "scribe-priorart" / "s-derive-1.derive.ids"