fix: Docker entrypoint arg handling + configurable model directory#402
Conversation
Fixes ruvnet#384: docker run with --source/--tick-ms flags now works correctly. Fixes ruvnet#399: model files in mounted volumes are now discoverable via MODELS_DIR env var. Root cause (issue ruvnet#384): The Dockerfile used ENTRYPOINT ["/bin/sh", "-c"] with a shell-form CMD. When users passed flags like `--source wifi --tick-ms 500` as docker run arguments, Docker replaced CMD entirely, resulting in `/bin/sh -c "--source wifi --tick-ms 500"` which executes `--source` as a shell command → `--source: not found`. Root cause (issue ruvnet#399): Model directory was hardcoded to the relative path `data/models`. When Docker users mounted models to `/app/models/`, the scan looked in the wrong place. Changes: 1. docker/docker-entrypoint.sh (new): - Proper entrypoint script that handles both env-var-based defaults and user-passed CLI flags - No arguments → starts server with CSI_SOURCE env var as --source - Flag arguments (start with -) → prepends /app/sensing-server + defaults, appends user flags (clap last-wins allows overrides) - Non-flag first arg → exec passthrough (e.g., /bin/sh for debugging) - Sets --bind-addr 0.0.0.0 (was 127.0.0.1 which blocks container access) 2. docker/Dockerfile.rust: - Switch from ENTRYPOINT ["/bin/sh", "-c"] to exec-form entrypoint - Add MODELS_DIR env var (default: data/models) - COPY the entrypoint script into the image 3. docker/docker-compose.yml: - Remove shell-form command (entrypoint handles defaults) - Add MODELS_DIR env var 4. model_manager.rs + main.rs: - Replace hardcoded `data/models` path with `effective_models_dir()` / `models_dir()` that reads MODELS_DIR env var at runtime - Docker users can now: docker run -v /host/models:/app/models -e MODELS_DIR=/app/models 5. tests/test_docker_entrypoint.sh (new, 17 tests): - Default CSI_SOURCE substitution (6 assertions) - Custom CSI_SOURCE propagation - User-passed flag arguments (--source, --tick-ms, --model) - Unset CSI_SOURCE defaults to auto - Explicit command passthrough - MODELS_DIR env var propagation
|
Thanks @voidborne-d — really nice, thorough fix. Reproduced the bug on
Your |
|
@ruvnet — recommend merging this PR. Verified locally against
One awareness item (not blocking): the entrypoint now binds Optional follow-ups:
This PR is a superset of #407 — see my comment there for why #407 alone is insufficient. |
Upstream moved forward with v0.6.2-esp32 (ADR-081 adaptive CSI mesh kernel, Timer Svc stack fix) and the Docker entrypoint merge of PR ruvnet#402. Conflicts resolved: - `firmware/esp32-csi-node/sdkconfig.defaults`: both sides appended a new config block. Kept both — `CONFIG_ESP_WIFI_EXTRA_IRAM_OPT=y` (ours, defense-in-depth for RuView#396 SPI cache race) AND `CONFIG_FREERTOS_TIMER_TASK_STACK_DEPTH=8192` (upstream's ADR-081 Timer Svc stack bump). They target different crash modes. - Applied the same `CONFIG_ESP_WIFI_EXTRA_IRAM_OPT=y` line to `sdkconfig.defaults.4mb` and `sdkconfig.defaults.template` for consistency — the SPI cache race is not flash-size specific, and the 4MB / template variants run the same CSI collector code with the same MGMT-only callback path. - `firmware/esp32-csi-node/version.txt`: both sides bumped to 0.6.2. Upstream already released v0.6.2-esp32, so our in-flight work bumps to 0.6.3. - `CHANGELOG.md`: auto-merge placed our [Unreleased] ruvnet#397 entries (and the older ruvnet#391 / ruvnet#390 entries) inside the newly-cut [v0.6.2-esp32] section. Moved them back to [Unreleased] — they describe work that has not been released yet. Auto-merged cleanly: `csi_collector.c`, `csi_collector.h`, `main.c`, `docker/*`, `README.md`, `docs/user-guide.md`. Verified the PR's defensive-copy code (`s_node_id_early_set`, `s_filter_mac`, `CSI_MIN_PROCESS_INTERVAL_US`, `s_early_drop`, the 50 Hz rate gate, MGMT-only filter, and `csi_collector_set_node_id()` API) is still present, and that the dropped probe-injection symbols stay absent (grep confirms 0 / 27 hits). Validation in this devcontainer: - ADR-081 host tests built and ran from `firmware/esp32-csi-node/tests/host/`: `test_adaptive_controller` 18/18 pass, `test_rv_feature_state` 15/15 pass, `test_rv_mesh` 27/27 pass — 60/60 total. These exercise the merged-in pure-C logic that this PR has no changes against, so they're a regression check that the merge didn't corrupt the upstream modules. - `edge_processing.c` still has `const float sample_rate = 10.0f;`. - Brace balance and dangling-ref checks on `csi_collector.c` pass. ESP-IDF firmware build, flash, and miniterm soak still deferred to @ruvnet's COM7 per the original review comment. Co-Authored-By: claude-flow <ruv@ruv.net>
Summary
Fixes #384 and #399 by replacing the broken Docker entrypoint pattern and making the model scan directory configurable.
Problem
Issue #384:
--source: not foundwhen passing flags todocker runThe Dockerfile used
ENTRYPOINT ["/bin/sh", "-c"]with a shell-form CMD. When users passed flags:Docker replaced CMD entirely, producing
/bin/sh -c "--source wifi --tick-ms 500"which tries to execute--sourceas a shell command →--source: not found.The
--separator also fails:docker run ... -- --source wifiproduces/bin/sh -c "-- --source wifi"→--: not found.Issue #399:
--model/--load-rvfparameters ignored in DockerThe model discovery API (
GET /api/v1/models,scan_model_files()) hardcodes the pathdata/models. When Docker users mount models to/app/models/:The
--modelCLI flag loads the specific file correctly, butGET /api/v1/modelsscans/app/data/models/and finds nothing →Discovered 0 model files.Solution
1.
docker/docker-entrypoint.sh(new)A proper entrypoint script that handles three usage patterns:
When the first argument starts with
-(or no args), the script prepends the server binary with sensible defaults. User-passed flags are appended and override defaults via clap's last-wins behavior.Also sets
--bind-addr 0.0.0.0(the binary defaults to127.0.0.1which blocks access from outside the container).2.
MODELS_DIRenvironment variableThe model scan path is now configurable:
The
effective_models_dir()helper readsMODELS_DIRat runtime with a default ofdata/models, preserving backward compatibility.3. Updated
Dockerfile.rustENTRYPOINTpointing to the entrypoint scriptENV MODELS_DIR=data/modelsCOPYthe entrypoint script into the image4. Updated
docker-compose.ymlcommandMODELS_DIRenvironment variableFiles changed
docker/docker-entrypoint.shdocker/Dockerfile.rustdocker/docker-compose.ymlsrc/model_manager.rsmodels_dir()reads MODELS_DIR env varsrc/main.rseffective_models_dir()replaces hardcoded pathstests/test_docker_entrypoint.shTests