Skip to content

Update cli-tooling to use the new telemetry approach in iii monorepo (PR# 1390)#12

Merged
anthonyiscoding merged 2 commits intomainfrom
chore/amplitude-telemetry-update
Apr 2, 2026
Merged

Update cli-tooling to use the new telemetry approach in iii monorepo (PR# 1390)#12
anthonyiscoding merged 2 commits intomainfrom
chore/amplitude-telemetry-update

Conversation

@anthonyiscoding
Copy link
Copy Markdown
Contributor

@anthonyiscoding anthonyiscoding commented Apr 1, 2026

Summary by CodeRabbit

  • New Features

    • Added worker module with WebSocket protocol support for enhanced inter-service communication.
  • Improvements

    • Strengthened telemetry system with improved validation and error handling when telemetry configuration is unavailable.
  • Chores

    • Updated project dependencies to optimize development and testing workflows.

Related to/requires: iii-hq/iii#1390

…f self-managed toml

Replace the scaffolder's own telemetry.toml identity and machine-id
hashing with a read of the engine-managed ~/.iii/telemetry.yaml.
Drop user_id from events (device_id only), send a telemetry_failed
fallback when the yaml is missing, and swap toml/sha2 deps for
serde_yaml/tempfile.
WorkerModule is mandatory but omitted from the template config,
causing it to be auto-injected with config: None. This bypasses
serde defaults and uses Rust's Default (empty host, port 0),
which panics on TcpListener::bind.
@anthonyiscoding anthonyiscoding requested a review from ytallo April 1, 2026 22:32
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 1, 2026

📝 Walkthrough

Walkthrough

The PR migrates telemetry identity storage and event handling from TOML (telemetry.toml) to YAML (telemetry.yaml) format. Dependencies are adjusted by removing toml and sha2, adding tempfile, and refactoring telemetry event payload construction, Amplitude HTTP posting, and failure handling. A WorkerModule configuration is added to the default III config.

Changes

Cohort / File(s) Summary
Workspace Dependencies
Cargo.toml
Removed toml and sha2 workspace dependencies; added tempfile.
Telemetry Core
crates/scaffolder-core/Cargo.toml
Removed toml and sha2 from dependencies; added tempfile to dev-dependencies.
Telemetry Module Refactor
crates/scaffolder-core/src/telemetry.rs
Migrated from TOML-based to YAML-based telemetry storage (schema version 2); removed machine-id hashing; refactored event payload construction to use device_id from YAML; centralized Amplitude HTTP posting logic with optional user_id field; added failure event emission when telemetry.yaml is missing; rewrote tests for new YAML format and failure scenarios.
Default Configuration
templates/iii/default-iii-config.yaml
Added WorkerModule entry with port 49134 for WebSocket protocol.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • ytallo

Poem

🐰 From TOML to YAML we now leap,
With telemetry secrets no longer deep,
Schema version two, device_id bright,
Failure events when things aren't right! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 58.82% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: updating cli-tooling to use a new telemetry approach in the iii monorepo, which aligns with the primary modifications across Cargo.toml, telemetry.rs, and config files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/amplitude-telemetry-update

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@anthonyiscoding anthonyiscoding changed the title Update cli-tooling to use the new telemetry approach in iii monorepo Update cli-tooling to use the new telemetry approach in iii monorepo (PR# 1390) Apr 1, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@crates/scaffolder-core/src/telemetry.rs`:
- Around line 368-380: The test mutates HOME globally which is unsafe for
parallel tests; annotate the test containing the tempfile HOME manipulation and
the send_amplitude_to call with #[serial_test::serial] (and add the serial_test
crate if missing) so it runs sequentially, and remove the unsafe global HOME
mutation by scoping it to that single serial test (leave telemetry_yaml_path()
unchanged). This ensures the test that calls send_amplitude_to and depends on
telemetry_yaml_path() won't race with other tokio::test cases.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8b55dba3-3f7c-4a54-90fe-3b7f58c87091

📥 Commits

Reviewing files that changed from the base of the PR and between db271de and c6ced37.

⛔ Files ignored due to path filters (3)
  • Cargo.lock is excluded by !**/*.lock
  • templates/iii/quickstart.zip is excluded by !**/*.zip
  • templates/motia/quickstart.zip is excluded by !**/*.zip
📒 Files selected for processing (4)
  • Cargo.toml
  • crates/scaffolder-core/Cargo.toml
  • crates/scaffolder-core/src/telemetry.rs
  • templates/iii/default-iii-config.yaml

@anthonyiscoding anthonyiscoding merged commit 9157898 into main Apr 2, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant