Skip to content

feat: send pid for the engine#1229

Merged
guibeira merged 8 commits intomainfrom
feat/send-pid-for-the-engine
Mar 10, 2026
Merged

feat: send pid for the engine#1229
guibeira merged 8 commits intomainfrom
feat/send-pid-for-the-engine

Conversation

@guibeira
Copy link
Contributor

@guibeira guibeira commented Mar 4, 2026

Previous prs:
iii-hq/sdk#40
#180

Summary by CodeRabbit

  • New Features

    • Workers can now include an optional process ID (PID) when registering, exposing PID in listings for better identification and tracking across SDKs.
  • Logging / Observability

    • Registration and unregistration now emit runtime logs that include worker ID, IP address, and PID when available.
  • Tests

    • Added tests validating PID presence, absence, storage, and listing behaviors.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 4, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ececa68a-eb00-454c-99a2-6fcdbb5052a2

📥 Commits

Reviewing files that changed from the base of the PR and between 9af933b and daeeefb.

📒 Files selected for processing (2)
  • sdk/packages/node/iii/src/iii.ts
  • sdk/packages/python/iii/src/iii/iii.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • sdk/packages/node/iii/src/iii.ts
  • sdk/packages/python/iii/src/iii/iii.py

📝 Walkthrough

Walkthrough

Adds optional process ID (pid) tracking across worker SDKs and the engine: pid is accepted on registration, stored on Worker, exposed in WorkerInfo listings, and logged during register/unregister flows. SDKs (Node, Python, Rust) now include pid in worker metadata.

Changes

Cohort / File(s) Summary
Engine — worker module
engine/src/modules/worker/mod.rs
Added pid: Option<u32> to WorkerInfo and RegisterWorkerInput; propagate input.pid into registry update and include pid when listing worker infos.
Engine — worker registry
engine/src/workers/mod.rs
Added pub pid: Option<u32> to Worker; update_worker_metadata(...) gains pid: Option<u32> param and stores it; constructors initialize pid: None; added tracing logs on register/unregister.
Engine tests
engine/tests/invocation_extensibility.rs
Added test worker_pid_is_stored_and_listed to assert registered worker pid is stored and returned by list. Updated/added unit tests covering pid presence/defaults.
SDKs — Node / Python / Rust
sdk/packages/node/iii/src/iii.ts, sdk/packages/python/iii/src/iii/iii.py, sdk/packages/rust/iii/src/iii.rs
Include current process id in worker metadata payloads: Node uses process.pid, Python uses os.getpid(), Rust adds optional pid to WorkerMetadata and Default.
Python tooling
frameworks/motia/motia-py/playground/pyproject.toml
Added local path source for the iii Python SDK under tool.uv.sources.

Sequence Diagram(s)

mermaid
sequenceDiagram
participant SDK as SDK (Node/Python/Rust)
participant EngineMod as Engine Module (modules/worker)
participant Registry as WorkerRegistry
participant ListAPI as Listing API
SDK->>EngineMod: registerWorkerMetadata(payload with pid)
EngineMod->>Registry: update_worker_metadata(worker_id, ..., pid)
Registry-->>EngineMod: ack/store worker (pid persisted)
EngineMod->>ListAPI: list_worker_infos()
ListAPI-->>SDK: returns WorkerInfo (includes pid)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

Suggested reviewers

  • sergiofilhowz

Poem

🐰 I carried a pid on a tiny note,

From Node, Python, Rust it wrote —
Through engine lanes and registry nests,
Now every worker shows where it rests,
Hopping logs and tests, we cheer this mote! 🥕✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 54.17% 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 'feat: send pid for the engine' directly and clearly describes the main change: adding process ID (pid) functionality to the engine system.

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

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/send-pid-for-the-engine

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.

Copy link
Contributor

@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 `@frameworks/motia/motia-py/packages/motia/pyproject.toml`:
- Around line 51-53: Replace the two incorrect GitHub Actions working-directory
entries that are currently set to "motia/motia-py/packages/motia" with the
correct project root "frameworks/motia/motia-py/packages/motia" so the relative
dependency path in pyproject.toml (the iii-sdk path) resolves during release;
update both occurrences of the YAML key working-directory in release.yml
accordingly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 90a99f92-938f-486c-b608-825e85713394

📥 Commits

Reviewing files that changed from the base of the PR and between c730b31 and 4c5ffa9.

⛔ Files ignored due to path filters (2)
  • frameworks/motia/motia-py/packages/motia/uv.lock is excluded by !**/*.lock
  • frameworks/motia/motia-py/playground/uv.lock is excluded by !**/*.lock
📒 Files selected for processing (8)
  • engine/src/modules/worker/mod.rs
  • engine/src/workers/mod.rs
  • engine/tests/invocation_extensibility.rs
  • frameworks/motia/motia-py/packages/motia/pyproject.toml
  • frameworks/motia/motia-py/playground/pyproject.toml
  • sdk/packages/node/iii/src/iii.ts
  • sdk/packages/python/iii/src/iii/iii.py
  • sdk/packages/rust/iii/src/iii.rs

Comment on lines +51 to +53
[tool.uv.sources]
iii-sdk = { path = "../../../../../sdk/packages/python/iii" }

Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Directory existence check:"
for d in frameworks/motia/motia-py/packages/motia motia/motia-py/packages/motia; do
  if [ -d "$d" ]; then
    echo "exists: $d"
  else
    echo "missing: $d"
  fi
done

echo
echo "Release workflow working-directory lines:"
rg -n 'working-directory:\s*.*motia.*/packages/motia' .github/workflows/release.yml

echo
echo "CI workflow working-directory lines:"
rg -n 'working-directory:\s*frameworks/motia/motia-py/packages/motia' .github/workflows/ci.yml

Repository: MotiaDev/motia

Length of output: 724


Update release.yml working-directory to match actual project structure.

The pyproject.toml Lines 51-53 introduces a relative path dependency (../../../../../sdk/packages/python/iii) that resolves correctly from frameworks/motia/motia-py/packages/motia, the actual project root in this repository. However, .github/workflows/release.yml lines 129 and 387 use working-directory: motia/motia-py/packages/motia, which does not exist and will break dependency resolution during release. Update both occurrences to frameworks/motia/motia-py/packages/motia to match the CI workflow configuration.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frameworks/motia/motia-py/packages/motia/pyproject.toml` around lines 51 -
53, Replace the two incorrect GitHub Actions working-directory entries that are
currently set to "motia/motia-py/packages/motia" with the correct project root
"frameworks/motia/motia-py/packages/motia" so the relative dependency path in
pyproject.toml (the iii-sdk path) resolves during release; update both
occurrences of the YAML key working-directory in release.yml accordingly.

@guibeira guibeira changed the title Feat/send pid for the engine feat: send pid for the engine Mar 4, 2026
Copy link
Contributor

@ytallo ytallo left a comment

Choose a reason for hiding this comment

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

The monorepo uv path source is fine for local development, but the published motia package dependency should not be relaxed to an unbounded iii-sdk requirement. Please keep a compatible version floor or range so downstream consumers do not resolve older or future incompatible SDK releases.

…-engine

# Conflicts:
#	engine/src/modules/worker/mod.rs
#	engine/src/workers/mod.rs
@vercel
Copy link
Contributor

vercel bot commented Mar 9, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
iii-docs Ready Ready Preview, Comment Mar 10, 2026 0:38am
motia-docs Ready Ready Preview, Comment Mar 10, 2026 0:38am

Request Review

Copy link
Contributor

@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.

🧹 Nitpick comments (1)
engine/src/workers/mod.rs (1)

111-134: Consider refactoring to reduce parameter count.

The #[allow(clippy::too_many_arguments)] suppression indicates this function has accumulated too many parameters (8 total). While acceptable for now, consider refactoring to use a struct parameter (e.g., WorkerMetadataUpdate) in a future iteration to improve maintainability and make the API clearer.

The conditional update pattern (if pid.is_some()) is consistent with how name and os are handled.

♻️ Optional: Example struct-based approach
pub struct WorkerMetadataUpdate {
    pub runtime: String,
    pub version: Option<String>,
    pub name: Option<String>,
    pub os: Option<String>,
    pub telemetry: Option<WorkerTelemetryMeta>,
    pub pid: Option<u32>,
}

pub fn update_worker_metadata(&self, worker_id: &Uuid, update: WorkerMetadataUpdate) {
    // ...
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@engine/src/workers/mod.rs` around lines 111 - 134, The function
update_worker_metadata currently accepts many parameters (runtime, version,
name, os, telemetry, pid) and uses #[allow(clippy::too_many_arguments)];
refactor by introducing a single struct (e.g., WorkerMetadataUpdate) that
contains the fields runtime: String, version: Option<String>, name:
Option<String>, os: Option<String>, telemetry: Option<WorkerTelemetryMeta>, pid:
Option<u32> and change the signature to update_worker_metadata(&self, worker_id:
&Uuid, update: WorkerMetadataUpdate); inside the method, replace direct
parameter usage with update.runtime, update.version, etc., preserving the
current conditional update behavior (e.g., only set worker.name if
update.name.is_some()) so callers build and pass the struct instead of many
arguments.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@engine/src/workers/mod.rs`:
- Around line 111-134: The function update_worker_metadata currently accepts
many parameters (runtime, version, name, os, telemetry, pid) and uses
#[allow(clippy::too_many_arguments)]; refactor by introducing a single struct
(e.g., WorkerMetadataUpdate) that contains the fields runtime: String, version:
Option<String>, name: Option<String>, os: Option<String>, telemetry:
Option<WorkerTelemetryMeta>, pid: Option<u32> and change the signature to
update_worker_metadata(&self, worker_id: &Uuid, update: WorkerMetadataUpdate);
inside the method, replace direct parameter usage with update.runtime,
update.version, etc., preserving the current conditional update behavior (e.g.,
only set worker.name if update.name.is_some()) so callers build and pass the
struct instead of many arguments.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ab65f1ae-f759-4b7c-be38-dd08e641e0fb

📥 Commits

Reviewing files that changed from the base of the PR and between 36b82c6 and 9af933b.

📒 Files selected for processing (3)
  • engine/src/modules/worker/mod.rs
  • engine/src/workers/mod.rs
  • sdk/packages/node/iii/src/iii.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • sdk/packages/node/iii/src/iii.ts

@guibeira guibeira merged commit 9e9d250 into main Mar 10, 2026
20 checks passed
@guibeira guibeira deleted the feat/send-pid-for-the-engine branch March 10, 2026 00:48
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.

3 participants