fix: post-v0.5.0 external audit — Lots 0–5 (DDS correctness, memory bounds, tests, docs)

CHANGELOG.md

@@ -7,6 +7,139 @@ and this project adheres to [Semantic Versioning 2.0.0](https://semver.org/spec/
 
 ## [Unreleased]
 
+### Changed
+
+- **DDS pure logic extracted for testability (Lot 0, external audit
+  2026-07-08).** The QoS-profile normalizers (`_cyclone_qos_to_profile` /
+  `_fast_qos_to_profile`) and the discovery-sample field extractors
+  (`_extract_guid` / `_extract_vendor_id` / `_extract_hostname` /
+  `_extract_topic_name` / `_is_removal`) were moved out of
+  `adapters/dds_cyclone/adapter.py` and `adapters/dds_fast/adapter.py` —
+  which import their vendor binding at module top level and were therefore
+  never exercised by the test suite — into the binding-free
+  `adapters/common/qos_normalize.py` and `adapters/common/dds_introspection.py`.
+  The adapters import them back under their original private names, so every
+  call site is unchanged (verified by `ruff check` static analysis, since the
+  adapters are not importable without their SDKs). `fast_qos_to_profile`
+  takes the binding's int→str enum maps as parameters so it stays
+  import-free. This is the highest-value item from the audit: the QoS
+  normalization feeds `detect_qos_mismatches` (the flagship DDS diagnostic)
+  and was previously untestable and untested.
+
+### Added
+
+- `tests/test_dds_qos_normalization.py` and `tests/test_dds_introspection.py`
+  drive the extracted logic with synthetic duck-typed objects (no
+  `cyclonedds` / `fastdds` needed), including regression guards for the
+  "renamed policy key silently yields no QoS profile → no mismatch ever
+  reported" failure mode. The extracted modules are now ~91–92% covered.
+- `pytest-cov` and `rosbags` added to the `[dev]` extra, plus `[tool.coverage]`
+  config with a `fail_under = 85` floor (the two binding-only adapter shells
+  are `omit`ted as structurally unreachable without their SDKs). Adding
+  `rosbags` un-skips the real `.db3` bag-analysis I/O test.
+
+### Fixed
+
+- `tests/test_bag_service.py` bag-generation helper updated for the current
+  `rosbags` API (`Writer(..., version=Writer.VERSION_LATEST)`; typestore keyed
+  by `std_msgs/msg/String`, not `std_msgs__msg__String`). The test was
+  previously auto-skipped and had gone silently stale — un-skipping it in CI
+  surfaced the drift.
+- **`topic_metrics` frequency was wrong (Lot 2, audit C5).** It divided the
+  sample count by `(now − oldest_sample)` — folding in idle time since the
+  last peek — and counted N intervals instead of N−1. Now measured as
+  `(N−1) / (newest − oldest)` over the samples' own arrival span; samples
+  surfaced by a single opportunistic peek share one timestamp (span 0) and
+  correctly yield `frequency_hz_observed = null` instead of a fabricated rate.
+- **`topic_metrics` sequence-gap count exploded on multi-writer topics,
+  publisher restarts, and counter wrap (Lot 2, audit C6).** Gaps are now
+  counted per writer (new best-effort `MetricsSample.writer_guid`), so
+  independent writers' counter offsets are not read as phantom gaps, and a
+  single jump wider than 10 000 is treated as a reset/wrap discontinuity
+  rather than that many losses.
+- **QoS Deadline false negative (Lot 2, audit C3/P1-3).**
+  `detect_qos_mismatches` now flags a reader that requests a finite Deadline
+  against a writer that offers none — an absent deadline is the infinite
+  (loosest) period and cannot satisfy a finite request. The previous rule
+  required both sides non-null and silently missed this incompatibility.
+- **`LifecycleBuffer` participant map was unbounded (Lot 3, audit P1-4 / M1 /
+  P1).** Only the event ring was capped; the participant dict grew one entry
+  per GUID ever seen (a churny bus mints a fresh RTPS GUID on each node
+  restart), and `list_participants` returned every tombstone forever. Now
+  capped at `MAX_PARTICIPANTS = 4096`, evicting `"left"` tombstones first then
+  the oldest-inserted entry — the docstring's "Bounded" claim is now true.
+- **`MetricsBuffer` topic map was unbounded (Lot 3, audit P2-5).** Per-topic
+  rings were capped but the number of topic keys was not; now capped at
+  `MAX_TOPICS = 4096`, oldest-inserted topic evicted on overflow.
+- **OpenDDS stub `is_available()` always returns False (Lot 4, audit S1).** A
+  stub that advertised availability (when a `pyopendds` module happened to be
+  importable) could be auto-selected by the factory, after which every tool
+  call raised. Now consistent with the Dust stub.
+- **`iter_field_names` mis-decoded a string `__slots__` (Lot 4, audit C2).**
+  `__slots__ = "value"` was exploded into `['v','a','l','u','e']`; a bare
+  string slot is now treated as a single field name.
+- **`decode_field_value` recursion is depth-capped at 32 (Lot 4, audit M6).**
+  A pathologically deep decoded object graph collapses to `repr()` instead of
+  risking `RecursionError`.
+- **`_encode_raw_bytes` slices before hex-encoding (Lot 4, audit M5).** A large
+  raw payload no longer allocates its full 2×-size hex string only to truncate
+  it to the 4096-char preview.
+
+### Added (Lot 4 — test hardening)
+
+- End-to-end test that a failing tool call surfaces as an MCP error
+  (`ToolError`) rather than being masked as a success — pins the thin-handler
+  contract (CLAUDE.md §8, audit test-gap #4).
+- Test pinning that every canonical vendor tag is a valid
+  `ParticipantInfo`/`ParticipantEvent.vendor` Literal (guards against
+  vendor-map ↔ schema drift, audit P2-3). Scenario allowlist `_KNOWN_TOOLS`
+  now includes the 11th tool `peek_bag_samples`.
+
+### Removed (Lot 4)
+
+- Dead `annotate_full` / `annotate_partial` imports and the `_ = (...)`
+  unused-suppressor from `services/bag_service.py`.
+
+### Documentation (Lot 1 — reconcile the strategic source of truth)
+
+- **`docs/product-plan.md` realigned on the shipped 11-tool surface (audit
+  M6/P1-6).** §1 and §4 said "five typed tools today" / DDS "roadmapped"
+  while six DDS/observability tools had shipped across v0.2.0–v0.4.0. §11's
+  risk register carried a self-imposed governance gate — "any 9th tool needs
+  an explicit re-scope discussion documented in this register before code
+  lands" — that was crossed during v0.4.0 without the discussion being
+  recorded. Added a retroactive re-scope decision closing that gap: the three
+  ceiling-breaking tools are accepted, the new ceiling is 11 tools, a 12th
+  needs a documented re-scope.
+- **User-topic `raw` decode honesty (audit C1).** README and the
+  `peek_dds_samples` tool description no longer imply the `raw` fallback
+  preserves the payload in `_raw_bytes_hex` — on the current user-topic raw
+  path that field is empty (a `raw` status means "present but not decoded").
+  Capturing the on-wire CDR bytes is stated as roadmapped rather than done.
+
+### Changed (Lot 5 — DDS adapter deduplication)
+
+- **QoS-mismatch endpoint pairing deduplicated (audit D1/M7/P2).** The ~40
+  identical lines in each adapter's `detect_qos_mismatches` (group endpoints by
+  topic, pair reader × writer, build `MismatchReport`) moved to the binding-free
+  `common/qos_endpoints.detect_mismatches_across_endpoints`, unit-tested without
+  a binding. Each writer's QoS profile is now parsed once per topic instead of
+  once per reader (fixes the O(readers × writers) re-parse). Both adapters
+  delegate to it.
+- **Shared `validate_domain_id` (`common/dds_helpers`).** The identical 0..232
+  bound check in all four DDS adapter constructors (Cyclone, Fast, OpenDDS,
+  Dust) is now defined once.
+- **Cyclone discovery/sample reads switched from `take_iter` to `read_iter`
+  (audit A1/P1-5).** Destructive `take` drained the builtin discovery cache,
+  risking spurious lost / re-discovered participant flapping across polls;
+  `read` is non-destructive — the correct choice for read-only observability.
+  ⚠️ **Requires real-bus validation on `scripts/integration/` before release**:
+  the read-vs-take semantics cannot be exercised without `cyclonedds` installed
+  (the adapter is not importable in the unit environment; these edits are
+  validated only by `ruff` static analysis + `py_compile`).
+
+Baseline: 399 → 485 passed, 24 → 23 skipped, ruff clean, coverage 89.12%.
+
 ## [0.5.0] - 2026-05-21
 
 ### Sprint v0.5.0 — Polish + validation (pre-marketing-publication)

README.md

@@ -185,7 +185,7 @@ Six DDS / observability tools (in addition to the five ROS2 tools above) :
 
 **Composite adapter (v0.4.0 Phase 1+).** When `TOPICFORGE_MODE=live` is paired with a DDS backend (`cyclone`, `fast`, …), TopicForge instantiates **both** a ROS2 CLI adapter and the chosen DDS adapter and routes per-tool category — the 5 ROS2 tools hit the CLI, the DDS / observability tools hit the DDS backend. ROS2-only or DDS-only setups still work — the missing half is skipped and the present half serves what it can. The mock backend continues to expose all 11 tools against deterministic fixtures for local development.
 
-**`peek_dds_samples` payload shape (v0.4.0 Phase 1.5).** Full-fidelity on the 4 builtin DCPS topics (`DCPSParticipant`, `DCPSSubscription`, `DCPSPublication`). Arbitrary user topics return best-effort decoded samples with a `_decode_status` annotation : `"full"` (every IDL field decoded — currently a v0.4.0+ Cyclone XTypes path), `"partial"` (some fields decoded, others opaque), or `"raw"` (binding could not resolve the dynamic XTypes — bytes preserved as hex in `_raw_bytes_hex`). The diagnostic key `_decode_note` carries a short explanation when the status is non-`full`. The wire shape is identical across Cyclone and Fast backends.
+**`peek_dds_samples` payload shape (v0.4.0 Phase 1.5).** Full-fidelity on the 4 builtin DCPS topics (`DCPSParticipant`, `DCPSSubscription`, `DCPSPublication`). Arbitrary user topics return best-effort decoded samples with a `_decode_status` annotation : `"full"` (every IDL field decoded — currently a v0.4.0+ Cyclone XTypes path), `"partial"` (some fields decoded, others opaque), or `"raw"` (binding could not resolve the dynamic XTypes). The diagnostic key `_decode_note` carries a short explanation when the status is non-`full`. The wire shape is identical across Cyclone and Fast backends. **Caveat (v0.5.x):** on the current user-topic *raw* path `_raw_bytes_hex` is **empty** — a `"raw"` status means "topic present on the bus but not decoded", not "here are the serialized bytes to re-decode". Capturing the on-wire CDR bytes into the fallback is roadmapped ; until then use the Cyclone full/partial XTypes path for actual user-topic payloads. The 4 builtin DCPS topics are unaffected (always structured).
 
 **`RTI Connext`** is v0.4.0+ Pro tier (BYO license — see `docs/pro.md`).
 

docs/product-plan.md

@@ -8,7 +8,7 @@
 
 TopicForge is **the safety-first read-only MCP for ROS2 robotics**. Where general-purpose ROS-MCP servers let an LLM publish topics, call services, and command robots — useful for demos, untenable for production fleets, defense systems, or anything safety-certified — TopicForge is read-only by **architecture**, not by configuration. There is no write path to misconfigure, no permission system to audit, no liability conversation to have. The MCP client can see the robot stack; it cannot touch it.
 
-Concretely, the server exposes five typed tools today — `health_check`, `list_topics`, `get_topic_info`, `sample_messages`, `analyze_bag` — backed by either a deterministic mock adapter (no ROS2 required) or a `ros2` CLI wrapper (full live introspection). Outputs are frozen Pydantic schemas, stable across runtime modes. Telemetry is opt-in, six fields, zero user payload.
+Concretely, the server exposes **eleven typed read-only tools today** (v0.5.0): the five ROS2-graph tools (`health_check`, `list_topics`, `get_topic_info`, `sample_messages`, `analyze_bag`) plus the six DDS / observability tools shipped across v0.2.0–v0.4.0 (`list_participants`, `detect_qos_mismatches`, `peek_dds_samples`, `participant_events`, `topic_metrics`, `peek_bag_samples`). They are backed by a deterministic mock adapter (no ROS2/DDS required), a `ros2` CLI wrapper, or an OSS DDS participant (Eclipse CycloneDDS / eProsima Fast DDS). Outputs are frozen Pydantic schemas, stable across runtime modes. Telemetry is opt-in, six fields, zero user payload.
 
 The ROS-MCP category is no longer empty (see §11 Risk register for the competitive landscape as of 2026-05-13). What TopicForge defends, and the rest of the pack will inherit, is the read-only-by-architecture stance and the production-quality engineering envelope around it — frozen schemas, mock-first development, telemetry contract pinned by tests, Windows-first cross-platform, no shell injection, deterministic outputs.
 
@@ -40,13 +40,13 @@ Three concentric circles, ranked by strategic priority rather than acquisition c
 
 The strategic bet is **pack breadth via two focused products plus a modular surface inside TopicForge**. Two MCPs, not three to five — solo maintenance cost was the binding constraint and the 2026-05-14 audit collapsed the earlier 3-to-5-MCP plan accordingly.
 
-**MCP 01 — TopicForge umbrella.** Covers ROS2 introspection today (shipped v0.1.2) and DDS observability as the next module (roadmapped, see §8). The `RosAdapter` protocol generalizes into a `MiddlewareAdapter` protocol that supports CycloneDDS in the OSS core and RTI Connext under the existing `topicforge_pro` license-gated package. One install (`pip install topicforge`, optional extras for DDS), one CLI, one license key — the umbrella keeps the developer ergonomics tight while extending coverage to the DDS-native audience the ROS-MCP competitive set does not reach. Module spec at `docs/projet-file/mcp-02-spec.md`.
+**MCP 01 — TopicForge umbrella.** Covers ROS2 introspection (shipped v0.1.2) and DDS observability, which **shipped as a module across v0.2.0–v0.4.0** (see §8) — no longer roadmapped. The `RosAdapter` protocol was generalized into a `MiddlewareAdapter` protocol that supports Eclipse CycloneDDS and eProsima Fast DDS in the OSS core and RTI Connext under the existing `topicforge_pro` license-gated package. One install (`pip install topicforge`, optional extras for DDS), one CLI, one license key — the umbrella keeps the developer ergonomics tight while extending coverage to the DDS-native audience the ROS-MCP competitive set does not reach. Module spec at `docs/projet-file/mcp-02-spec.md`.
 
 **MCP 02 — DatasetForge.** Vision Dataset Inspector. Read images + annotations (COCO at MVP; YOLO / HF Datasets on roadmap) and answer structured questions about class balance, split coherence, annotation quality. Targets the ML/CV audience overlapping with TopicForge but distinct enough in domain (training data vs runtime graph) to warrant a separate product, separate repo, separate PyPI name. Full spec at `docs/projet-file/mcp-03-spec.md` (the file is still named `mcp-03-spec.md` for historical continuity ; the slot is MCP 02 of the 2-MCP pack).
 
 **Motif of the pivot.** Earlier drafts of this plan sequenced a 3-to-5-MCP pack with a separate DDS observability MCP as MCP 02. The 2026-05-14 audit collapsed that into a 2-product strategy: TopicForge as an umbrella covering both middlewares, DatasetForge as the second standalone product. The binding constraints were (a) solo-maintenance cost of running two repos in parallel and (b) the fact that ROS2 and DDS are the same problem shape — a typed pub/sub graph that needs structured introspection — and the `RosAdapter` protocol already generalizes to a `MiddlewareAdapter` superset with zero rework. Two products instead of three reduces the surface area without losing coverage.
 
-The umbrella commits TopicForge to a slightly broader scope (cap: 5 ROS2 tools today + at most 3 DDS-side tools when the module ships, ceiling enforced by §11). The pack inherits the layer separation, mock-first development, opt-in telemetry, and read-only-by-architecture commitments from TopicForge. Pack-shared infrastructure extraction (telemetry, license, settings resolver into a `pack-template/` repo) becomes a non-decision at 2 products: fork-and-tweak from TopicForge to DatasetForge is acceptable ; revisit only if a third product is ever planned.
+The umbrella commits TopicForge to a broader scope than first drafted: the DDS module shipped **six** DDS / observability tools across v0.2.0–v0.4.0 (not the three originally scoped), taking the surface to **11 tools total**. The original 8-tool ceiling was formally revised — see the re-scope decision in §11. The pack inherits the layer separation, mock-first development, opt-in telemetry, and read-only-by-architecture commitments from TopicForge. Pack-shared infrastructure extraction (telemetry, license, settings resolver into a `pack-template/` repo) becomes a non-decision at 2 products: fork-and-tweak from TopicForge to DatasetForge is acceptable ; revisit only if a third product is ever planned.
 
 ---
 
@@ -171,7 +171,7 @@ The risks worth tracking explicitly. Updated 2026-05-13 with the competitive lan
 - **Cross-platform regressions on Windows.** TopicForge's primary developer environment is Windows. The Makefile uses POSIX shell syntax; users on plain PowerShell need the documented escape hatches. Mitigation: tested directly in CI on `ubuntu-latest` only today; Windows coverage is documented in `docs/TESTING.md` and exercised manually before each release.
 - **Telemetry trust.** Even opt-in telemetry can damage trust if the payload contract drifts. Mitigation: `tests/test_telemetry.py::test_payload_contains_only_whitelisted_keys` pins the six allowed keys. Any change requires a CHANGELOG entry and a README Telemetry section update in the same PR.
 - **Time / focus dilution.** A solo maintainer trying to drive two products (TopicForge umbrella + DatasetForge), a Pro tier inside each, marketing, and the DDS module on top of TopicForge is the realistic risk. The 2026-05-14 pivot from a 3-to-5-MCP pack to a 2-product strategy reduced the surface but did not eliminate the risk. Mitigation: explicit phase gates (do not start Phase 2 until Phase 1 is shipped, do not act on the DDS module marketing until Phase 2 has shipped) — though §8 schedules `MiddlewareAdapter` protocol prep during Phase 1.
-- **Scope creep within the TopicForge umbrella.** Combining ROS2 + DDS introspection in one product risks bloating the tool surface beyond what a focused MCP should expose. Mitigation: tool surface stays capped at the 5 ROS2 tools today ; the DDS module adds at most 3 new tools (`list_participants`, `detect_qos_mismatches`, `peek_dds_samples`) when it ships. Any 9th tool needs an explicit re-scope discussion documented in this register before code lands.
+- **Scope creep within the TopicForge umbrella.** Combining ROS2 + DDS introspection in one product risks bloating the tool surface beyond what a focused MCP should expose. **Re-scope decision (2026-07-08, ratified retroactively).** The register's original ceiling — 5 ROS2 tools + at most 3 DDS tools, any 9th tool gated on a re-scope discussion documented *here* before code lands — was crossed during v0.4.0 **without that discussion being recorded in this register**, a governance gap surfaced by the 2026-07-08 external audit. The three tools that broke it are deliberate and were acknowledged in the CHANGELOG and `docs/projet-file/mcp-02-spec.md §2` at ship time: `participant_events` (9th, v0.4.0 Phase 1), `topic_metrics` (10th, Phase 2), `peek_bag_samples` (11th, Phase 3). They are accepted; the revised ceiling is **11 tools**. A 12th tool now needs an explicit re-scope discussion documented in this register before code lands. Mitigation going forward: the `verify-change` skill's doc-drift step and the `docs-curator` sweep keep this register, `README.md`, and `CLAUDE.md` in sync so a ceiling break cannot ship undocumented again.
 
 ---