[Sim Theme C] Turtlesim visualizer (VNC)

[jmservera]

Implements Theme C of the Simulation & Visualization PRD (FR-C1..C4). Lane 3 (Turtlesim backend) — service/route infra parallelizable immediately; menu wiring depends on Themes A/B. Adds a turtlesim service (Xvfb+x11vnc+websockify) and its noVNC proxy route, ROS-native. Plan: docs/plans/sim-theme-c-turtlesim.md. Draft.

[jmservera]

🤖 Automated PR review

Turtlesim visualizer — single-container Xvfb + turtlesim_node + openbox + x11vnc + websockify, streamed over noVNC through a lazy-DNS proxy route. Base: #22.

Findings

Severity	Location	Finding
🟡 Medium	services/turtlesim/entrypoint.sh	A trap '… kill -TERM "${TURTLE_PID}"' TERM INT is set immediately before exec websockify …. exec replaces the shell process and discards the trap, so the intended graceful turtlesim shutdown is dead code (websockify becomes PID 1 and receives the signal directly; turtlesim is left to container teardown). Either drop the misleading trap, or run websockify in the background and wait on it so the trap survives. Compare with #27/#26, which handle this correctly via wait -n.
🟢 Low	services/turtlesim/entrypoint.sh	Xvfb/x11vnc/openbox are never reaped; fine for a container, but worth a note.

✅ nginx lazy-resolver route (resolver 127.0.0.11 + variable proxy_pass) is excellent and well-documented. DDS client profile matches the ros/simulator services. x11vnc -listen localhost + no host-published ports keeps it proxy-only. RMW_IMPLEMENTATION spelling verified correct across the repo.

Recommendation: Approve after removing/repairing the dead trap.

[jmservera]

🤖 Automated PR review

Turtlesim visualizer — single-container Xvfb + turtlesim_node + openbox + x11vnc + websockify, streamed over noVNC through a lazy-DNS proxy route. Base: #22.

Findings

Severity	Location	Finding
🟡 Medium	services/turtlesim/entrypoint.sh	A trap '… kill -TERM "${TURTLE_PID}"' TERM INT is set immediately before exec websockify …. exec replaces the shell process and discards the trap, so the intended graceful turtlesim shutdown is dead code (websockify becomes PID 1 and receives the signal directly; turtlesim is left to container teardown). Either drop the misleading trap, or run websockify in the background and wait on it so the trap survives. Compare with #27/#26, which handle this correctly via wait -n.
🟢 Low	services/turtlesim/entrypoint.sh	Xvfb/x11vnc/openbox are never reaped; fine for a container, but worth a note.

✅ nginx lazy-resolver route (resolver 127.0.0.11 + variable proxy_pass) is excellent and well-documented. DDS client profile matches the ros/simulator services. x11vnc -listen localhost + no host-published ports keeps it proxy-only. RMW_IMPLEMENTATION spelling verified correct across the repo.

Recommendation: Approve after removing/repairing the dead trap.

[jmservera]

🤖 Automated line-anchored review — see the summary comment for the full findings table.

[Copilot]

Pull request overview

Adds a ROS-native Turtlesim “lightweight simulator” that renders a GUI in-container (Xvfb + Openbox + x11vnc + websockify/noVNC) and exposes it through the existing reverse proxy under a dedicated route, alongside related planning and ROS/Gazebo baseline documentation touch-ups.

Changes:

• Add a new turtlesim container image and entrypoint that runs the full VNC/noVNC pipeline plus a Fast DDS discovery-server client profile.
• Extend the nginx reverse proxy to serve /sim/turtlesim/novnc/ with lazy DNS resolution so the proxy can start even when turtlesim is not running.
• Define the turtlesim service in compose.yaml (on-demand via the simulators profile) and enable it in the simulator registry.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
services/turtlesim/openbox-rc.xml	Minimal Openbox rules to maximize/undecorate the turtlesim window for noVNC display.
services/turtlesim/entrypoint.sh	Entrypoint orchestration for Xvfb/Openbox/turtlesim/x11vnc/websockify.
services/turtlesim/Dockerfile	New image packaging turtlesim + VNC/noVNC toolchain and DDS profile.
services/turtlesim/config/dds_discovery.xml	Fast DDS discovery-server client profile for ROS-native graph participation.
services/proxy/nginx.conf	Add /sim/turtlesim/novnc/ reverse-proxy route with Docker DNS resolver.
services/control/simulators.js	Enable the turtlesim simulator entry in the registry.
docs/specs/01-Init-research.md	Update research notes around images/pairings (Ionic/Harmonic references).
docs/plans/sim-theme-c-turtlesim.md	Add Theme C plan stub for the turtlesim visualizer lane.
docs/decisions/ADR-001-ros-distro.md	Update distro decision narrative (Kilted baseline) and fallback guidance text.
compose.yaml	Add the turtlesim service under the simulators profile with healthcheck and networks.

[jmservera]

@claude[agent] resolve the two remaining PR comments about the entrypoint.sh sigkill issue.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

services/turtlesim/entrypoint.sh:64

• The TERM/INT trap is ineffective because exec websockify ... replaces the bash PID 1 process, so the trap never runs and turtlesim_node (and the other background processes) may be left running until Docker sends SIGKILL. Keep bash as PID 1 and explicitly manage the websockify child so signals trigger cleanup.

# Forward termination to the turtlesim node for a clean shutdown.
trap 'kill -TERM "${TURTLE_PID}" 2>/dev/null || true' TERM INT

# Serve noVNC static assets and bridge WebSocket -> VNC on :6080 (foreground).
exec websockify \

docs/decisions/ADR-001-ros-distro.md:18

• The ADR’s Context section now frames the candidates as Kilted/Jazzy/Humble, but immediately after it references the ROS 2 “Lyrical package path” and later says to treat Jazzy/Ionic as the fallback while “the Lyrical package path remains blocked…”. That historical note is useful, but it’s confusing without explicitly stating that Lyrical was a previously-considered candidate and that the Jetty/Noble constraint is only relevant in that context.

Kilted offers the longest runway but its ecosystem package coverage
(`rosbridge-suite`, `ros-gz`) was unverified at research time.

During development we confirmed that the Jetty image uses Ubuntu Noble, which blocks the ROS 2 Lyrical package path on that image. Gazebo Ionic remains compatible with both Jazzy and Kilted, so it was retained while the repository baseline stayed on Kilted.

[Claude]

@claude[agent] resolve the two remaining PR comments about the entrypoint.sh sigkill issue.

Fixed in 6782f78. Replaced exec websockify with running websockify in the background and wait -n, so the shell (and its trap) stays alive to forward SIGTERM to turtlesim_node on container stop.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

docs/specs/01-Init-research.md:1686

• This footnote sentence is internally ambiguous: it says the source “records … Kilted in November 2026” and also “Kilted ending in approximately May 2031” without clarifying what the November 2026 date represents (release? EOL? something else). Please rewrite to disambiguate the meaning of each date (or remove the ambiguous one) so the citation remains self-consistent.

[^ros-eol]: ROS 2 lifecycle schedule: <https://endoflife.date/ros-2>, last updated 2026-06-01 and accessed 2026-07-17. It records Iron EOL in November 2024, Humble in May 2027, Jazzy in May 2029, Kilted in November 2026, and Kilted ending in approximately May 2031. REP-2000 had no Kilted section at the research date.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

services/turtlesim/Dockerfile:22

• The Dockerfile comment says procps/pgrep are used for a health check, but this container’s health check is curl-based (see compose.yaml) and there’s no pgrep usage in the turtlesim image. This makes the comment misleading and pulls an extra package unnecessarily.

# turtlesim + the VNC toolchain (Xvfb framebuffer, openbox window manager,
# x11vnc export, novnc/websockify browser bridge). x11-utils provides xdpyinfo
# for the framebuffer readiness probe; curl is used by the container health check.
RUN apt-get update \

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

services/turtlesim/entrypoint.sh:71

• x11vnc is started in the background but its PID is never tracked. If x11vnc crashes/exits, the entrypoint will keep running (websockify can still serve static assets), so the container may stay "healthy" while the VNC backend is dead and users just see connection failures. Track x11vnc’s PID and include it in the TERM/INT trap and wait set so the container restarts cleanly when the VNC server dies.

trap 'kill -TERM "${TURTLE_PID}" "${WEBSOCKIFY_PID}" 2>/dev/null || true' TERM INT

wait -n "${TURTLE_PID}" "${WEBSOCKIFY_PID}"

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

services/turtlesim/entrypoint.sh:54

• x11vnc is started in the background but its PID is never tracked. If x11vnc exits/crashes, the container can keep running (websockify + turtlesim still up) while the noVNC WebSocket backend is broken, which makes failures harder to detect/recover. Capture the x11vnc PID and include it in the TERM/INT trap and the wait -n set so the container stops (and can restart) when x11vnc dies.

x11vnc \
  -display "${DISPLAY_NUM}" \
  -nopw \
  -listen localhost \
  -xkb \