Two same-named external-linkage test types can silently collide (ODR violation, no compiler/linker diagnostic)

[Yaraslaut]

What happened

While rebasing PR #79 (issue-55-testkit), CI was failing nearly the entire Linux matrix on test_conflict_resolution.cpp with a queue never drains symptom — reproducing 100% deterministically on Linux/clang-ubsan and real CI, but never on Windows.

Root cause: tests/test_remote_step_interleaving.cpp (added by the same PR) declared its own file-scope struct OrderModel { ... } — a bare stub with no onBackendChanged() — with the exact same name as tests/test_conflict_resolution.cpp's own, pre-existing, unrelated OrderModel (which has onBackendChanged(), notifyCount, offline-queue draining logic).

Both types have external linkage (declared at file scope, not inside an anonymous or named namespace). Two external-linkage types with the same name and different definitions is a One Definition Rule (ODR) violation — undefined behavior that neither the compiler nor the linker diagnoses. Whichever definition the linker keeps for a given translation unit is compiler/link-order dependent.

Confirmed via targeted tracing: in the affected builds, LocalBackend::_changeAware was empty after registering a model built from test_conflict_resolution.cpp's own modelFactory — i.e. the linker had resolved BackendChangedNotifiable<OrderModel> using test_remote_step_interleaving.cpp's bare stub definition instead of the real one, so notifyBackendChanged() never actually invoked onBackendChanged(), and the offline queue never drained.

Fix applied (this instance)

Renamed test_remote_step_interleaving.cpp's OrderModel/OrderAction to StepILOrderModel/StepILOrderAction (matching that file's own StepIL_* wire-typeId convention). Verified 10/10 clean under WSL/clang-ubsan (both the whole [conflict] tag and CI's own per-test-case invocation pattern), full suite green.

Scope check

A full sweep of every .cpp file under tests/ (109 files, 273 genuine file-scope struct/class declarations after excluding template specializations) found no other current instances of this collision — all remaining file-scope type names in the test suite are unique. Types declared inside a named namespace (e.g. namespace issue21::models { struct Report { ... }; }, already used and tested for BRIDGE_REGISTER_MODEL/BRIDGE_REGISTER_ACTION per issue #21) are safe from this class of bug — their linker symbols are namespace-qualified and cannot collide with a same-named type in a different namespace. Types inside an anonymous namespace are also safe (internal linkage, no cross-TU visibility).

Ask

This bug class is real, silent, and will recur as the test suite grows (git blame/history shows the test files are typically written independently, each choosing convenient short names like OrderModel, Widget, etc., with no central registry of what's already taken). Worth considering one or more of:

1. A CI lint step that scans tests/**/*.cpp for file-scope (non-anonymous-namespace, non-named-namespace) struct/class declarations and fails if the same simple name appears in more than one file.
2. A project convention: new test files should wrap their local model/action types in an anonymous namespace by default, only lifting them to file scope when a macro (BRIDGE_REGISTER_MODEL, etc.) requires external linkage — and in that case, prefer a short, file/feature-specific prefix (as this fix did with StepIL*) over a generic name like OrderModel/Widget/Model.
3. Documenting this gotcha in docs/spec/testing_strategy.md or a CLAUDE.md-adjacent contributor note, since it is exactly the kind of defect that is invisible in code review and only reproduces on specific compiler/OS/optimization combinations.

🤖 Generated with Claude Code

[Yaraslaut]

Why UBSan didn't catch this (added to docs/spec/testing_strategy.md in #85):

UBSan instruments individual operations within a translation unit at compile time (signed overflow, invalid enum loads, misaligned access, bad vptr casts) and its checks fire while that instrumented code runs. Both colliding TUs here were independently well-formed — test_conflict_resolution.cpp's real OrderModel and test_remote_step_interleaving.cpp's bare stub each compile and run correctly in isolation. The defect exists only in what the linker does across the two .o files (silently keeping one TU's definition of BackendChangedNotifiable<OrderModel> for both), which happens before any sanitizer runtime is even loaded. This is a structural blind spot of the entire compile-time-instrumentation family (ASan/UBSan/TSan/MSan alike) — none of them are designed to catch cross-TU ODR violations.

The linker's own duplicate-symbol diagnostics don't fire either: the colliding symbol is a template instantiation (implicitly inline), and the linker treats multiple same-mangled inline/template definitions across TUs as expected COMDAT folding — the normal case when two TUs instantiate a template identically. It can't distinguish that from two genuinely different definitions sharing a mangled name.

This is exactly why #85's fix is a source-level static check (scripts/check_test_type_names.sh) rather than a build/sanitizer flag — it's the only layer that sees both TUs' declarations before they're compiled down into symbols nothing downstream can reason about.

🤖 Generated with Claude Code