[MAJOR] §9: TransparentBridge not implemented despite CLAUDE.md documentation

[mvillmow]

Finding

Severity: MAJOR
Section: 9
Evidence: CLAUDE.md (Architecture section), include/ and src/ (no TransparentBridge class), include/core/message_bus.hpp:201-208 (setNatsPublisher manual hook)
Principle: MODULARITY

CLAUDE.md describes a 'Transparent Bridge' that 'automatically promotes messages destined for off-host agents from the local MessageBus onto the appropriate NATS subject'. No TransparentBridge class exists in include/ or src/. The setNatsPublisher hook exists but requires the caller to manually wire up the bridge — the automatic promotion behavior documented in CLAUDE.md is unimplemented.

---

Part of #504

[mvillmow]

Implementation Plan

I now have all the context needed. Here is the implementation plan:

---

Implementation Plan: Issue #512 — TransparentBridge

1. Objective

Implement a TransparentBridge class in keystone_transport that encapsulates the wiring between MessageBus and NatsConnection. The bridge must automatically:

• Promote messages that fail local routing to the correct NATS subject, using KIM serialization.
• Subscribe to inbound NATS subjects and inject deserialized KeystoneMessage objects back into the local MessageBus.
• Remove the manual lambda wiring currently scattered in src/daemon/main.cpp:80-95.

The goal is to make the bridge behavior declared by CLAUDE.md real and self-contained — callers simply construct TransparentBridge and call attach().

---

2. Approach

Key decisions:

• New class, new files — TransparentBridge goes in include/transport/transparent_bridge.hpp and src/transport/transparent_bridge.cpp, parallel to NatsConnection. This keeps transport concerns together and avoids polluting core/.
• Ownership model — TransparentBridge takes non-owning references (raw pointers or std::reference_wrapper) to MessageBus and NatsConnection. Both objects are owned by the daemon or test harness, which controls lifetime. This avoids circular shared-ownership.
• Subject derivation — Map msg.receiver_id → NATS subject using a simple deterministic rule (e.g., hi.agents.<receiver_id>) consistent with the NATS subject schema in CLAUDE.md. Keep a SubjectMapper helper (free function or small struct) so the mapping can be unit-tested independently.
• Inbound path — TransparentBridge creates its own NATSListener-style pull loop (or reuses NATSListener) on hi.agents.>, deserializes each natsMsg* payload via MessageSerializer::deserialize(), and calls MessageBus::routeMessage().
• Outbound path — Bridge calls MessageBus::setNatsPublisher() with a lambda that serializes the KeystoneMessage and calls natsConnection_Publish() — exactly what main.cpp does today, but encapsulated.
• routeMessage fallthrough — MessageBus::routeMessage() already returns false when the local agent is unknown and calls nats_publisher_ before returning false. The bridge hooks into that existing fallthrough; no change to MessageBus internal routing logic is needed.
• Daemon simplification — After the bridge is created, the manual setNatsPublisher lambda in main.cpp is deleted and replaced by bridge.attach().

---

3. Files to Create

File	Purpose
include/transport/transparent_bridge.hpp	Class declaration for TransparentBridge and BridgeConfig
src/transport/transparent_bridge.cpp	Implementation: attach(), outbound publisher lambda, inbound pull loop
tests/unit/test_transparent_bridge.cpp	Unit tests using mock NATS publisher and mock MessageBus (no live NATS needed)
tests/integration/test_bridge_integration.cpp	Integration test exercising the full bridge against a real NATS server (guarded by KEYSTONE_INTEGRATION_TESTS=1)

---

4. Files to Modify

include/transport/transparent_bridge.hpp (new)

Define:

namespace keystone::transport {

struct BridgeConfig {
  std::string inbound_subject{"hi.agents.>"};  // subject to subscribe on
  std::string durable_name{"keystone-bridge"};
  int max_ack_pending{1};
};

class TransparentBridge {
 public:
  TransparentBridge(core::MessageBus& bus, NatsConnection& conn, BridgeConfig cfg = {});
  ~TransparentBridge();

  // Wire outbound (MessageBus -> NATS) and start inbound pull loop.
  // Calls bus.setNatsPublisher(...) and starts the listener thread.
  // safe to call only once; call stop() before destroying.
  natsStatus attach(jsCtx* js);

  // Stop the inbound pull loop. Idempotent.
  void stop();

 private:
  void inbound_loop() noexcept;

  core::MessageBus& bus_;
  NatsConnection& conn_;
  BridgeConfig cfg_;
  natsSubscription* sub_{nullptr};
  std::atomic<bool> stopped_{false};
  std::thread inbound_thread_;
};

// Derive NATS subject from a receiver agent ID.
// e.g. "worker-1" -> "hi.agents.worker-1"
std::string deriveNatsSubject(std::string_view receiver_id);

}  // namespace keystone::transport

src/transport/transparent_bridge.cpp (new)

• attach():
  1. Calls bus_.setNatsPublisher(lambda) where the lambda serializes with MessageSerializer::serialize() → natsConnection_Publish().
  2. Subscribes to cfg_.inbound_subject using jsCtx* (matching NATSListener::start() pattern).
  3. Starts inbound_thread_ running inbound_loop().
• inbound_loop():
  • Mirrors NATSListener::pull_loop(): calls natsSubscription_Fetch(sub_, 1, timeout_ms), deserializes with MessageSerializer::deserialize(), calls bus_.routeMessage(), acks/naks.
• stop(): sets stopped_, joins inbound_thread_, calls natsSubscription_Unsubscribe().
• deriveNatsSubject(): "hi.agents." + std::string(receiver_id).

src/daemon/main.cpp (lines 78–95)

• Delete the manual message_bus.setNatsPublisher(...) lambda block.
• Replace with:

  keystone::transport::TransparentBridge bridge(message_bus, nats_conn);
  // ... (after nats_conn.connect() succeeds and jsCtx* is obtained)
  bridge.attach(js);

• Add bridge.stop() in the shutdown sequence before nats_conn.disconnect().

CMakeLists.txt (line ~559)

• Add src/transport/transparent_bridge.cpp to keystone_transport:

  add_library(keystone_transport
    src/transport/nats_connection.cpp
    src/network/nats_listener.cpp
    src/transport/transparent_bridge.cpp)   # <- add

• Add new test executables (see step 9 below).

tests/unit/test_transparent_bridge.cpp (new)

• Use a fake/mock MessageBus subclass that captures setNatsPublisher() and records calls to routeMessage().
• Use a fake NATS publisher lambda (no live NATS) to test outbound subject derivation and serialization round-trip.
• Tests:
  • AttachRegistersNatsPublisher — after attach() with a null js, the outbound publisher is set.
  • OutboundSubjectDerivedFromReceiverId — publisher lambda called with correct subject for given receiver_id.
  • OutboundPayloadRoundTrips — serialized payload deserializes back to equivalent KeystoneMessage.
  • InboundMessageRoutedToLocalBus — inject a serialized message into the inbound path; verify routeMessage() called with correct fields.
  • StopIsIdempotent — stop() can be called multiple times without crash.
  • DeriveNatsSubjectFormatsCorrectly — unit test the free function directly.

tests/integration/test_bridge_integration.cpp (new)

• Guarded by integrationTestsEnabled() (env KEYSTONE_INTEGRATION_TESTS=1).
• Tests:
  • BridgeForwardsOffHostMessage — register no local agent, publish a KeystoneMessage via routeMessage(), verify it appears on NATS subject.
  • BridgeInjectsInboundMessage — publish a serialized KeystoneMessage to NATS hi.agents.<id>, verify the local agent registered under <id> receives it.

---

5. Implementation Order

1. Write include/transport/transparent_bridge.hpp — header only, no NATS dependency in the declaration (forward-declare jsCtx).
2. Write src/transport/transparent_bridge.cpp — implement attach(), inbound_loop(), stop(), deriveNatsSubject().
3. Update CMakeLists.txt — add transparent_bridge.cpp to keystone_transport; add bridge_unit_tests and bridge_integration_tests executables.
4. Write tests/unit/test_transparent_bridge.cpp — unit tests (no NATS server needed).
5. Update src/daemon/main.cpp — remove the manual lambda, add TransparentBridge bridge(...) and wire attach() / stop() into the daemon lifecycle.
6. Write tests/integration/test_bridge_integration.cpp — integration tests requiring a live NATS server.
7. Build and run unit tests — cmake --preset debug && cmake --build --preset debug && ctest --preset debug -R bridge_unit.
8. Run sanitizer builds — cmake --preset asan && cmake --build --preset asan && ctest --preset asan.
9. Run integration tests (optional, requires NATS) — KEYSTONE_INTEGRATION_TESTS=1 ctest --preset debug -R bridge_integration.
10. Verify daemon still starts — smoke-test src/daemon/main.cpp binary against a local NATS server.

---

6. Verification

Build correctness

cmake --preset debug && cmake --build --preset debug

Expect zero warnings (CI enforces -Werror).

Unit tests (no NATS required)

ctest --preset debug -R bridge_unit_tests --output-on-failure

All six unit tests must pass.

Sanitizer builds

cmake --preset asan && cmake --build --preset asan && ctest --preset asan
cmake --preset tsan && cmake --build --preset tsan && ctest --preset tsan

No ASan or TSan errors.

Format and lint gates

just format-check
just lint

Integration tests (live NATS)

# Start NATS server first
docker-compose -f docker-compose.test.yml up -d
KEYSTONE_INTEGRATION_TESTS=1 ctest --preset debug -R bridge_integration --output-on-failure

Manual daemon smoke-test

./build/debug/bin/keystone-daemon
# Verify log: "keystone-daemon: TransparentBridge attached subject=hi.agents.>"
# Verify old log line about manual publisher is gone

Regression: existing tests must still pass

ctest --preset debug --output-on-failure

nats_integration_tests, transport_unit_tests, registry_integration_tests must all continue to pass.

---

7. Skills Used During Planning

Directly Invoked

• Explore subagent — broad codebase survey of include/, src/, tests/, and CMakeLists.txt to map existing NATS integration points, MessageBus API, serialization interface, and test patterns.

Team Knowledge Base Skills Referenced (from Prior Learnings)

• natsc-fetchcontent-cpp20-integration — nats.c C++20 JetStream pub/sub integration patterns. Used to confirm the correct natsSubscription_Fetch / natsConnection_Publish call sites and the jsCtx* lifetime model that inbound_loop() should follow.
• architecture-crosshost-nats-compose-deployment — Cross-host NATS over Tailscale context. Used to validate that hi.agents.> is the appropriate inbound subject and that BridgeConfig.inbound_subject should default to that pattern.
• crosshost-per-component-launcher-pattern — Per-component service wiring via NATS_URL. Used to understand how the daemon wires NatsConnection and how TransparentBridge should receive a pre-connected NatsConnection& rather than owning its own connection.
• nats-py-connection-resilience-patterns — NATS reconnect/retry behavior reference. Used to confirm that TransparentBridge should not own reconnect logic — it delegates to the NatsConnection's existing callback chain — and that stop() must be idempotent to handle reconnect races cleanly.

---

Generated by Claude Code Planner