Uh oh!
There was an error while loading. Please reload this page.
feat(packs/openclaw): silent anonymous telemetron sidecar - #31
Conversation
Phase 3.1 of the telemetron enrollment plan. Installs telemetron v0.3.1 and lets it anonymously enroll against the Loki telemetry backend as the last step of the openclaw pack install. Design: - Entirely silent on stdout/stderr. All output — success, failure, skip reason — goes to $INSTALL_LOG only. Telemetron failure never blocks, warns, or prints anything in the user-visible install flow. - 30s hard outer timeout via `timeout 30 bash -o pipefail -c '...'`. pipefail is critical: without it, a curl failure in `curl ... | bash` would be masked as success because the RHS bash exits 0 on empty stdin. - Self-contained $install_log path with a default so `set -u` in the pack script cannot trip on an unexported INSTALL_LOG. - No sudo probe: lowkey runs as root. - No curl guard: curl is already a hard prereq of the lowkey installer. - Guards in order: --skip-telemetron → non-Linux → opt-out → systemd. Scope-back from Phase 3: - No correlation with lowkey's $_TELEM_INSTALL_ID. Telemetron generates its own UUID via the anonymous /v1/enroll path. Athena join by install_id deferred to Phase 3.2. - No profile tagging. Deferred to Phase 3.2. - No source=lowkey-installer override. Deferred to Phase 3.2 (blocked on telemetron v0.3.2 adding TELEMETRON_ENROLL_SOURCE). Reviewed by Codex three times (v1 REJECT, v2 REJECT, v3 APPROVE after pipefail fix). Smoke-tested locally against four scenarios: - --skip-telemetron=true: silent, log shows skip reason - LOWKEY_TELEMETRY=0: silent, log shows skip reason - Sabotaged URL (127.0.0.1:1): silent, log shows rc=7 from curl - Real URL, non-root: silent, log shows telemetron installer failed cleanly at the sudo step — outer pack install still returns 0. New opt-out lever: - --skip-telemetron pack flag (alongside existing LOWKEY_TELEMETRY=0, DO_NOT_TRACK=1, ~/.lowkey/telemetry-off). Plan: docs/telemetron-lowkey-phase3.1-plan.md in the loki-telemetry repo.
…ron contract 37 assertions covering: - manifest.yaml: structure, required keys, name match, skip-telemetron param - install.sh: shebang, syntax, common.sh source, done marker, --help flags, --skip-telemetron arg parsing - telemetron sidecar static contract: timeout + pipefail wrapper, defaulted INSTALL_LOG, opt-out honors, outer || true safety net - telemetron sidecar runtime behavior (isolated, no network): - PACK_ARG_SKIP_TELEMETRON=true: zero user-visible output, log shows skip - LOWKEY_TELEMETRY=0: zero user-visible output, log shows opt-out reason - Block does not trip outer set -euo pipefail All tests network-free and runnable without systemd, sudo, OpenClaw, or Bedrock — safe for GitHub Actions CI.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:b4137bbaed
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if [[ "${LOWKEY_TELEMETRY:-1}" = "0" ]] \ | ||
| || [[ "${DO_NOT_TRACK:-0}" = "1" ]] \ | ||
| || [[ -f "${HOME}/.lowkey/telemetry-off" ]]; then |
There was a problem hiding this comment.
The new opt-out guard relies on LOWKEY_TELEMETRY / DO_NOT_TRACK, but in the normal install flow these variables are not visible to packs/openclaw/install.sh: deploy/bootstrap.sh runs packs via sudo -u ec2-user --preserve-env=PACK_CONFIG,AWS_DEFAULT_REGION,XDG_RUNTIME_DIR,DBUS_SESSION_BUS_ADDRESS ..., which drops both telemetry vars. As a result, users who set LOWKEY_TELEMETRY=0 (or DO_NOT_TRACK=1) still run the telemetron sidecar unless they also create ~/.lowkey/telemetry-off, so the documented env-based opt-out is ineffective in production installs.
Useful? React with 👍 / 👎.
| PACK_ARG_LITELLM_KEY="$(pack_config_get litellm_key "")" | ||
| PACK_ARG_LITELLM_MODEL="$(pack_config_get litellm_model "claude-opus-4-6")" | ||
| PACK_ARG_PROVIDER_KEY="$(pack_config_get provider_key "")" | ||
| PACK_ARG_SKIP_TELEMETRON="$(pack_config_get skip_telemetron "false")" |
There was a problem hiding this comment.
This reads skip_telemetron, but the manifest declares the parameter as skip-telemetron. Since pack_config_get performs a direct key lookup (no hyphen/underscore normalization), configs that follow the manifest key will not disable telemetron, making the new manifest parameter effectively ignored unless callers know to use a different internal key name.
Useful? React with 👍 / 👎.
P1: deploy/bootstrap.sh runs packs under 'sudo -u ec2-user --preserve-env=...' which dropped LOWKEY_TELEMETRY and DO_NOT_TRACK, making the documented env opt-outs ineffective in production installs. Add both to --preserve-env in both pack invocations (dependency + primary). P2: openclaw pack read 'skip_telemetron' via pack_config_get, but manifest.yaml declares the parameter as 'skip-telemetron'. Since pack_config_get does a direct key lookup with no hyphen/underscore normalization, configs using the manifest key were silently ignored. Switch the read to 'skip-telemetron' to match. Addresses Codex P1+P2 on PR #31.
royosherove
commented
May 4, 2026
Addressed Codex review 4218735506 in 8e93737: P1 \u2014 env opt-outs not honored under sudo: AGREE, FIXED. P2 \u2014 manifest key mismatch: AGREE, FIXED. |
…ests The previous test suite had a cluster of assertions that grep'd install.sh for the presence of specific text — 'timeout 30 bash -o pipefail -c', 'INSTALL_LOG:-/tmp/loki-install.log', 'LOWKEY_TELEMETRY', etc. Those are tautologies: they would pass even if the pipefail fix regressed, because 'pipefail' would still appear somewhere in the file. Replaced with a PATH-hijack behavior harness that exercises the block's observable contract end-to-end with stubbed binaries (curl, systemctl, uname, timeout). Strict PATH — only the stub dir — so a real coreutil leak is a test failure. Scenarios now covered (all silent, all log-only, all non-blocking): - --skip-telemetron=true → log: skip: --skip-telemetron - LOWKEY_TELEMETRY=0 → log: lowkey telemetry opt-out - DO_NOT_TRACK=1 → log: lowkey telemetry opt-out - ~/.lowkey/telemetry-off file → log: lowkey telemetry opt-out - Darwin (non-Linux) → log: skip: non-Linux - No systemctl on PATH → log: skip: systemctl not found - Curl fails (rc=7) → log: install failed (rc=N), N > 0 THE pipefail regression test: a stub curl exits 7. If pipefail breaks, log would claim success and the test fails loud. - Hanging curl + clamped timeout → block returns in <10s, log records rc=124 or 143. Proves the outer timeout actually runs. Each scenario asserts: - zero user-visible stdout (only the AFTER_OK sentinel) - zero user-visible stderr (even when the stub explicitly writes to it) - outer 'set -euo pipefail' not tripped (AFTER_OK printed after source) - log contains the expected outcome tag 48/48 pass locally. Network-free, ~3s total runtime (timeout scenario clamps the 30s cap to 3s via a shim — we don't need to wait 30s to prove 'timeout' works, only that the block calls it with a bounded arg and honors its exit code). Follow-up to Codex senior-review note #3 / #4.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:06f9652a79
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| PACK_ARG_LITELLM_KEY="$(pack_config_get litellm_key "")" | ||
| PACK_ARG_LITELLM_MODEL="$(pack_config_get litellm_model "claude-opus-4-6")" | ||
| PACK_ARG_PROVIDER_KEY="$(pack_config_get provider_key "")" | ||
| PACK_ARG_SKIP_TELEMETRON="$(pack_config_get "skip-telemetron" "false")" |
There was a problem hiding this comment.
PACK_ARG_SKIP_TELEMETRON is read from pack_config_get("skip-telemetron", "false"), but the bootstrap dispatcher never writes that key into /tmp/loki-pack-config.json (see deploy/bootstrap.sh pack-config jq block) and it also invokes pack installers without passing CLI flags through. In the normal deploy/bootstrap.sh flow this means the new manifest param is effectively dead and always resolves to false, so operators cannot disable telemetron via pack config as intended.
Useful? React with 👍 / 👎.
Bootstrap now accepts '--skip-telemetron' and writes the value into PACK_CONFIG JSON under the 'skip-telemetron' key, matching the openclaw pack manifest and the installer's pack_config_get read path. Without this the manifest param was dead and always resolved to false. Note: top-level install.sh -> CFN -> bootstrap.sh plumbing for the flag is a separate change; this at least makes the pack config path live for direct bootstrap invocations and future CLI wiring. Addresses Codex P2 on PR #31.
royosherove
commented
May 4, 2026
Addressed Codex review 4218795510 in b21e590: P2 \u2014 skip-telemetron not wired through bootstrap: PARTIAL FIX. Correct catch \u2014 the previous commit fixed the key mismatch in the pack read, but b21e590 adds:
That makes the full chain Out of scope for this PR (follow-up): plumbing the flag through top-level |
…off critical path Addresses the senior-review findings on PR #31: 1. Extract the silent/bounded/pipefail bootstrap engine to packs/common.sh as `run_optional_sidecar NAME URL TIMEOUT_SECS LOG_FILE [ENV...]`. The openclaw pack no longer owns telemetry product policy — if a second pack wants a metrics sidecar tomorrow, it calls the same helper. 2. Split the pack-side code into two named functions: - `should_run_telemetron()` — pure policy. Returns 'yes' or 'skip: <reason>'. No side effects, no logging. Composable, testable, debuggable. - `_telemetron_sidecar()` — thin adapter that turns the decision into either a log entry (skip) or a call to run_optional_sidecar (yes). 3. Move write_done_marker + success banner BEFORE the sidecar. The user should not wait on best-effort work to see their install succeeded. A hung GitHub fetch adding 30 silent seconds to a user's install was the closest thing to a block in the senior review. 4. Replace the `bash -c ' ... "'"$url"'" ... '` quote-splicing with positional args: `bash -c '... "$1" ...' _ "$url" "$ct" "$mt"`. One-pass readable shell. 5. Rewrite tests against the new seams — should_run_telemetron() as a pure function, run_optional_sidecar() as an engine. No more sed-extracting comment-delimited blocks. The engine tests use strict `env -i PATH=stubdir bash --noprofile --norc -c` to defeat the PATH prepending that /etc/profile.d and ~/.bashrc were doing on AL2023. Metrics: - install.sh: 327 → 297 lines (-30) - test.sh: 48 → 39 assertions, all behavior-based, 3 layers covered - common.sh: +61 lines for the reusable engine All 39 tests pass locally, 3s wall-clock.
Codex reproduced a concrete leak: when $INSTALL_LOG points at a path with a nonexistent parent (e.g. /no/such/dir/log.txt), the exec >>"log" redirection fails *before* stdout/stderr are captured, so the shell diagnostic + all subsequent output leaks straight to the caller's terminal — contradicting the 'zero bytes on stdout/stderr' invariant. Fix in packs/common.sh:run_optional_sidecar(): Replace: exec >>"$log" 2>&1 || exec >/dev/null 2>&1 With a pre-flight writability test in a subshell so no diagnostic leaks before the exec commits: if ( >> "$log" ) 2>/dev/null; then exec >>"$log" 2>&1 else exec >/dev/null 2>&1 fi Fix in packs/openclaw/install.sh:_telemetron_sidecar(): Resolve the log path once at the top with the same writability probe before any printf calls, so the skip-path logger also stays silent when the log is unwritable. Added test I1b: 'unwritable log (engine)' — sc_run with log path under nonexistent directory, asserts zero stdout, zero stderr, AFTER_OK sentinel (set -euo pipefail not tripped). Passes only with the fix; would have caught the regression. 42/42 tests pass locally.
Uh oh!
There was an error while loading. Please reload this page.
What
Installs telemetron v0.3.1 as the last step of the
openclawpack and lets it anonymously enroll against the Loki telemetry backend. Zero-configuration metrics for every openclaw install.Phase 3.1 of the telemetron enrollment plan — deliberately scoped back from the rejected Phase 3 to eliminate correlation complexity.
Design
Deliberately silent. All output — success, failure, skip reason — goes to
$INSTALL_LOGonly. Telemetron failure never blocks, warns, or prints anything in the user-visible install flow.Hard 30s outer timeout via
timeout 30 bash -o pipefail -c '...'.pipefailis critical: without it,curl ... | bashreturns the RHS bash's exit code (0 on empty stdin) and a curl failure would be silently reported as success. Codex caught this in review v3.Root assumption. Lowkey runs as root. No
sudo -nprobing, no curl guard (curl is already a hard prereq).Opt-out levers (any of these skips):
--skip-telemetronpack flagLOWKEY_TELEMETRY=0envDO_NOT_TRACK=1env~/.lowkey/telemetry-offfileScoped out (deferred to Phase 3.2)
_TELEM_INSTALL_IDinto telemetron for Athena joinsource=lowkey-installeroverride (needs telemetron v0.3.2)Review history
bash -o pipefail -c, verified with the same reproTests
New
packs/openclaw/test.sh— 37 assertions, network-free, CI-safe:Smoke-tested locally against 4 scenarios (all silent):
--skip-telemetron=trueLOWKEY_TELEMETRY=0Files changed
packs/openclaw/install.sh— newinstall_telemetronblock between gateway health check andwrite_done_marker; new--skip-telemetronflagpacks/openclaw/manifest.yaml— newskip-telemetronparampacks/openclaw/test.sh— new test file (275 lines, 37 assertions)Rollback
Revert the commit. Already-installed hosts keep running telemetron just fine; it's self-contained.