fix: installer portability, description limits, and eval honesty

.github/workflows/validate.yml

@@ -0,0 +1,70 @@
+name: validate
+
+on:
+  push:
+    branches: [main]
+  pull_request:
+
+jobs:
+  skills:
+    name: Skill frontmatter
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions/setup-python@v5
+        with:
+          python-version: "3.11"
+      - name: Validate SKILL.md frontmatter
+        run: python3 scripts/validate_skills.py
+
+  installer:
+    name: install.sh
+    strategy:
+      fail-fast: false
+      matrix:
+        # macOS covers BSD userland and bash 3.2, where GNU-only builtins break.
+        os: [ubuntu-latest, macos-latest]
+    runs-on: ${{ matrix.os }}
+    steps:
+      - uses: actions/checkout@v4
+      - name: Syntax check
+        run: bash -n install.sh
+      - name: shellcheck
+        if: runner.os == 'Linux'
+        run: |
+          sudo apt-get update && sudo apt-get install -y shellcheck
+          shellcheck install.sh
+      - name: Smoke test with the system shell
+        run: |
+          set -euo pipefail
+          /bin/bash install.sh --list
+          /bin/bash install.sh --target "${RUNNER_TEMP}/all" --skills all
+          /bin/bash install.sh --target "${RUNNER_TEMP}/bundle"
+          /bin/bash install.sh --target "${RUNNER_TEMP}/sel" --skills ros2 robotics-testing
+          test -f "${RUNNER_TEMP}/sel/ros2/SKILL.md"
+          expected=$(/bin/bash install.sh --list | wc -l)
+          actual=$(ls "${RUNNER_TEMP}/all" | wc -l)
+          test "${expected}" -eq "${actual}"
+      - name: Reject unknown and traversing skill names
+        run: |
+          set -uo pipefail
+          # A traversing name reaches `rm -rf "${target}/${skill}"`, so it must be
+          # rejected by name before any filesystem operation runs.
+          for bad in does-not-exist ../evals ../../skills /etc "skills;rm"; do
+            if /bin/bash install.sh --target "${RUNNER_TEMP}/bad" --skills "${bad}"; then
+              echo "install.sh accepted the rejected skill name: ${bad}" >&2
+              exit 1
+            fi
+          done
+          test ! -e "${RUNNER_TEMP}/bad/evals"
+
+  evals:
+    name: Eval packages
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions/setup-python@v5
+        with:
+          python-version: "3.11"
+      - name: Byte-compile eval sources
+        run: python3 -m compileall -q evals

CONTRIBUTING.md

@@ -11,6 +11,7 @@ real failure modes from robot software.
 - Add a production checklist to a skill.
 - Add a small, realistic code example.
 - Improve the eval prompts or add another before/after eval.
+- Make an eval build-verified: get a generated package to `colcon build` and record how.
 - Start one of the roadmap skills from the README.
 
 ## Skill Structure
@@ -37,10 +38,28 @@ Then include:
 - Testing and debugging notes.
 - Production checklist.
 
+The `name` **must** match the skill's directory name exactly, and `description` must stay
+under 1024 characters — agents will not load a skill that violates either constraint.
+
+## Validating Your Change
+
+```bash
+python3 scripts/validate_skills.py   # frontmatter, name/directory match, README links
+bash -n install.sh                   # installer syntax
+```
+
+Both run in CI on every pull request, along with an `install.sh` smoke test on Linux and
+macOS. The validator needs no dependencies beyond Python 3.
+
+It also warns when a `SKILL.md` exceeds 1,000 lines. That is a budget, not an error: past
+that size a skill costs a lot of context every time it triggers, and the detail is better
+placed in a `references/` file the agent loads only when it needs it.
+
 ## Review Checklist
 
 Before opening a PR:
 
+- `python3 scripts/validate_skills.py` passes.
 - The skill has specific trigger language in `description`.
 - Examples use realistic ROS names, QoS settings, parameters, and launch patterns.
 - Advice distinguishes simulation, development, and production when it matters.

README.md

@@ -35,6 +35,11 @@ once with the robotics skills loaded as context.
 | Structure | Single node module | Separate writer module and cleaner responsibilities |
 | Config surface | 4 parameters | 13 parameters |
 
+This is a single-task, single-run comparison, self-assessed, and neither package was
+built against a real ROS2 distribution — treat it as an illustration of the design
+decisions the skills push toward, not as a benchmark. Caveats and the known build
+defects in both artifacts are documented in the report.
+
 Full report: [evals/EVAL_REPORT.md](evals/EVAL_REPORT.md)
 
 ## Quick Start

evals/EVAL_REPORT.md

@@ -13,7 +13,7 @@ demonstration episodes during teleoperation. (See [PROMPT.md](PROMPT.md) for ful
 | `with-skills/` | All 5 skills as raw context, no extra instructions | [PROMPT.md](PROMPT.md) + [AGENT_PROMPT.md](AGENT_PROMPT.md) |
 
 Both runs received the **same task prompt**. The only difference is whether the 5 skill
-files (ros2-development, robot-perception, robotics-design-patterns,
+files (ros2, robot-perception, robotics-design-patterns,
 robotics-software-principles, robotics-testing) were loaded as context. The agent was
 given no explicit design instructions — it had to independently decide which patterns
 from the skills to apply.
@@ -582,13 +582,13 @@ patterns to use. It independently identified and applied every relevant pattern:
 
 | Pattern Applied | Source Skill |
 |----------------|-------------|
-| Lifecycle node (on_configure/activate/deactivate/cleanup/shutdown) | ros2-development |
-| QoS profiles (BEST_EFFORT for sensors, RELIABLE for status) | ros2-development |
-| ParameterDescriptor with FloatingPointRange/IntegerRange | ros2-development |
-| Runtime parameter change callback | ros2-development |
-| ament_cmake + rosidl for custom msgs | ros2-development |
-| Launch arguments + auto-lifecycle orchestration | ros2-development |
-| MultiThreadedExecutor with callback groups | ros2-development |
+| Lifecycle node (on_configure/activate/deactivate/cleanup/shutdown) | ros2 |
+| QoS profiles (BEST_EFFORT for sensors, RELIABLE for status) | ros2 |
+| ParameterDescriptor with FloatingPointRange/IntegerRange | ros2 |
+| Runtime parameter change callback | ros2 |
+| ament_cmake + rosidl for custom msgs | ros2 |
+| Launch arguments + auto-lifecycle orchestration | ros2 |
+| MultiThreadedExecutor with callback groups | ros2 |
 | Threaded bounded sensor buffers (SensorBuffer class) | robot-perception |
 | Software timestamp synchronization with configurable threshold | robot-perception |
 | Sensor health watchdog with timeout detection | robot-perception |
@@ -608,15 +608,59 @@ patterns to use. It independently identified and applied every relevant pattern:
 
 ---
 
+## Known Defects in the Generated Artifacts
+
+Both packages are **verbatim agent output**, kept unmodified so the comparison stays
+honest. Neither has been built against a real ROS2 distribution, and a review found
+build defects in both. They are recorded here rather than patched, because editing the
+artifacts would destroy their value as evidence.
+
+| Defect | Run | Effect |
+|--------|-----|--------|
+| `CMakeLists.txt` generates interfaces but never calls `ament_python_install_package` or installs the node | without-skills | Nothing is installed; `ros2 run demo_recorder demo_recorder_node` and the launch file both fail |
+| `install(PROGRAMS demo_recorder/demo_recorder_node.py DESTINATION lib/${PROJECT_NAME})` has no `RENAME`, so the executable is named `demo_recorder_node.py` | with-skills | The launch file requests `executable="demo_recorder_node"` and fails to resolve it |
+| `setup.py` ships alongside `<build_type>ament_cmake</build_type>` | both | Dead file — never invoked by the ament_cmake build; the install rules in `CMakeLists.txt` are what count |
+
+The with-skills defect is a one-line fix (`RENAME demo_recorder_node`); the
+without-skills package needs install rules written from scratch. That difference is
+itself a data point, but neither package should be copied as-is.
+
+**What this means for the metrics:** the table above measures the *design decisions* in
+the generated code — lifecycle transitions, QoS selection, buffering, test coverage. It
+does not measure whether the code runs, because neither run was build-verified.
+
+---
+
+## Methodology Caveats
+
+Read the numbers with these limits in mind:
+
+- **n=1.** One prompt, one task, one model, one run per arm. No repeated sampling, so
+  run-to-run variance is unmeasured and the deltas below are not significance-tested.
+- **Self-assessed.** The comparison was written by the same system that produced the
+  artifacts. No independent robotics reviewer graded either package.
+- **Line counts are a proxy, not a result.** "6.3x total lines" measures volume, and
+  volume is the axis a language model inflates for free. 2,107 lines for an episode
+  recorder is arguably over-engineered for some deployments. The defensible claims are
+  the categorical ones: lifecycle transitions present vs. absent, sensor QoS chosen
+  deliberately vs. left at default, 601 lines of tests vs. zero.
+- **Not build-verified.** See the defects above.
+
+Adding a build-verified, multi-run eval across several task prompts is the highest-value
+improvement to this directory. See [CONTRIBUTING.md](../CONTRIBUTING.md).
+
+---
+
 ## Conclusion
 
-The **without-skills** version is a functional but naive implementation — it would work in a
-demo but would fail in production due to QoS mismatches, no sensor health monitoring, no
-graceful degradation, and no tests.
+The **without-skills** version is a naive implementation. Beyond its missing install
+rules, it would fail in production due to QoS mismatches, no sensor health monitoring,
+no graceful degradation, and no tests.
 
 The **with-skills** version applies production robotics patterns throughout. The agent was
 not told which patterns to use — it read the skills, recognized which ones were relevant
-to the task, and applied them independently. The result is code that a senior robotics
-engineer would recognize as production-ready.
+to the task, and applied them independently. The result is a design a senior robotics
+engineer would recognize as production-shaped — though, as noted above, it still needs a
+build fix before it runs.
 
 **The skills didn't make the agent more capable — they made it more experienced.**

evals/with-skills/demo_recorder/README.md

@@ -1,5 +1,11 @@
 # demo_recorder
 
+> **Eval artifact — not build-verified.** This is verbatim agent output kept unmodified
+> as evidence for [EVAL_REPORT.md](../../EVAL_REPORT.md). It has a known build defect:
+> `install(PROGRAMS ...)` in `CMakeLists.txt` lacks `RENAME demo_recorder_node`, so the
+> installed executable is `demo_recorder_node.py` and the launch file cannot resolve it.
+> Read it for the patterns, not as a package to copy.
+
 A production-grade ROS2 package for recording robot demonstration episodes
 during teleoperation.  Designed for a 7-DOF Franka Emika Panda arm with a
 wrist-mounted Intel RealSense camera.

evals/without-skills/demo_recorder/README.md

@@ -1,5 +1,10 @@
 # Demo Recorder
 
+> **Eval artifact — not build-verified.** This is verbatim agent output kept unmodified
+> as evidence for [EVAL_REPORT.md](../../EVAL_REPORT.md). Its `CMakeLists.txt` never
+> installs the Python package or the node, so the build instructions below do not
+> produce a runnable package. Read it as a baseline, not as a package to copy.
+
 A ROS2 Python package for recording robot demonstration episodes during teleoperation.
 
 ## Overview

install.sh

@@ -24,8 +24,13 @@ target=""
 declare -a requested_skills=()
 declare -a default_skills=(ros2 robotics-software-principles robotics-testing robot-bringup)
 
+# Globbing rather than `find -printf`: BSD find (macOS) has no -printf.
 list_skills() {
-  find "${skills_dir}" -mindepth 1 -maxdepth 1 -type d -printf '%f\n' | sort
+  local path
+  for path in "${skills_dir}"/*/; do
+    [[ -d "${path}" ]] || continue
+    basename "${path}"
+  done | sort
 }
 
 while [[ $# -gt 0 ]]; do
@@ -67,8 +72,12 @@ if [[ ${#requested_skills[@]} -eq 0 ]]; then
   requested_skills=("${default_skills[@]}")
 fi
 
+# Read loop rather than `mapfile`: macOS ships bash 3.2, which has no mapfile.
 if [[ ${#requested_skills[@]} -eq 1 && "${requested_skills[0]}" == "all" ]]; then
-  mapfile -t requested_skills < <(list_skills)
+  requested_skills=()
+  while IFS= read -r skill; do
+    requested_skills+=("${skill}")
+  done < <(list_skills)
 fi
 
 mkdir -p "${target}"
@@ -87,7 +96,9 @@ for skill in "${requested_skills[@]}"; do
     exit 1
   fi
 
-  rm -rf "${target}/${skill}"
+  # ${target:?} so an empty target can never make this `rm -rf /${skill}`.
+  # ${skill} is already constrained to [A-Za-z0-9_-] above.
+  rm -rf "${target:?}/${skill}"
   cp -R "${src}" "${target}/${skill}"
   echo "Installed ${skill} -> ${target}/${skill}"
 done