fix(install.sh): enforce 60s smoke-test timeout; don't misreport Ctrl+C as failure#3181
Conversation
… Ctrl+C as failure Resolves dimensionalOS#3017. - Wrap the post-install smoke test in `timeout 60` (with a portable sleep+kill watchdog fallback for systems without coreutils `timeout`, e.g. stock macOS) so the advertised '60s' cap is actually honored. - Add a local INT trap around the wait so Ctrl+C stops only the smoke test and reports 'stopped by user' instead of aborting the installer and printing 'installation failed'.
Greptile SummaryThis PR adds a 60-second smoke-test limit and localized interruption handling.
Confidence Score: 2/5The PR should not merge until direct installer execution is restored and smoke-test paths are passed safely to the child shell. The checked-in installer is no longer executable, and valid project directories containing an apostrophe break or alter the generated smoke-test command. Files Needing Attention: scripts/install.sh Important Files Changed
Sequence DiagramsequenceDiagram
participant Installer
participant Wrapper as timeout / watchdog
participant Smoke as DimOS smoke test
Installer->>Wrapper: launch with 60s limit
Wrapper->>Smoke: start unitree-go2
alt Ctrl+C
Installer->>Wrapper: SIGINT
else 60 seconds elapsed
Wrapper->>Smoke: SIGTERM
end
Wrapper-->>Installer: exit status
Installer->>Installer: classify result and restore INT trap
|
| timeout 60 bash -c "cd '$dir'; source '$venv'; $cmd" & | ||
| else | ||
| bash -c "cd '$dir'; source '$venv'; $cmd" & |
There was a problem hiding this comment.
Paths alter generated shell syntax
When a supported project directory supplied through DIMOS_PROJECT_DIR, --project-dir, or the interactive prompt contains a single quote, embedding it in bash -c "cd '$dir'; source '$venv'; $cmd" terminates the quoting and treats part of the path as shell syntax, causing the smoke test to fail or execute unintended commands.
Knowledge Base Used: Build, CI, and Packaging
❌ 12 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
Fixes #3017.
Bug 1 — the 60s timeout never existed. The smoke-test prompt advertises 'starts unitree-go2 for 60s' and the result handler even checks for exit code 124 (what
timeoutreturns), but the command was launched directly with$cmd &and never wrapped intimeout. So it ran indefinitely.Bug 2 — Ctrl+C reported 'installation failed'. A bare
wait $pidlet the global installer INT/EXIT traps treat a smoke-test interruption as a full install failure.Fix
timeout 60(preferred). When coreutilstimeoutis unavailable (e.g. stock macOS), fall back to a portablesleep 60; killwatchdog, so the 60s cap is real on every platform.waitthat forwards Ctrl+C to the child and reports 'stopped by user', then restores the original handler — the installer no longer aborts and prints 'installation failed'.Validated with
bash -n.