Conversation
There was a problem hiding this comment.
Pull request overview
Adds a shared, reusable Python runtime packaging/install flow and wires the Mijia Smart Home plugin to run its companion script via that shared runtime (instead of relying solely on system Python), alongside some login/QR UX and host process-exec behavior updates.
Changes:
- Introduces
crates/peekoo-python-sdkplusjusttargets and scripts to build/install a packaged Python runtime fromrequirements.txt. - Updates the Mijia plugin to try shared/plugin-local/system Python candidates and improves QR/login polling UX.
- Extends plugin install to copy optional
runtime/andvendor/directories; adjusts host process exec behavior and related schemas.
Reviewed changes
Copilot reviewed 16 out of 18 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| plugins/mijia-smart-home/ui/panel.html | QR placeholder handling + revised login polling timeout/error handling. |
| plugins/mijia-smart-home/src/lib.rs | Uses shared Python candidate list; improves interpreter fallback logic and exec error shaping. |
| plugins/mijia-smart-home/README.md | Documents shared runtime build/install and interpreter lookup order. |
| plugins/mijia-smart-home/peekoo-plugin.toml | Makes process:exec a required permission. |
| plugins/mijia-smart-home/companions/requirements.txt | Defines Python deps for the packaged runtime. |
| plugins/mijia-smart-home/companions/mijia_bridge.py | Updates missing-dependency error messages to reference “runtime”. |
| plugins/mijia-smart-home/Cargo.toml | Adds dependency on peekoo-python-sdk. |
| justfile | Adds python SDK build/install recipes; updates plugin install copying. |
| crates/peekoo-python-sdk/src/lib.rs | Centralizes shared/plugin-local/system Python interpreter candidate lists. |
| crates/peekoo-python-sdk/scripts/build_runtime_package.sh | Builds a packaged standalone Python runtime + installs requirements into it. |
| crates/peekoo-python-sdk/scripts/install_runtime_package.sh | Installs packaged runtime into a target directory (default shared location). |
| crates/peekoo-python-sdk/Cargo.toml | New crate manifest. |
| crates/peekoo-plugin-sdk/src/lib.rs | Re-exports new process module. |
| crates/peekoo-plugin-host/src/host_functions.rs | Process exec: return structured ok:false responses on capability/arg/cwd/spawn errors; tilde/program resolution. |
| Cargo.toml | Adds peekoo-python-sdk workspace member. |
| Cargo.lock | Adds peekoo-python-sdk entry. |
| apps/desktop-tauri/src-tauri/gen/schemas/macOS-schema.json | Adds PostHog/Sentry permission constants to schema. |
Comments suppressed due to low confidence (1)
plugins/mijia-smart-home/companions/mijia_bridge.py:714
- Same as above: the error suggests “Repackage the plugin runtime”, but the runtime is installed to the shared
~/.peekoo/python-sdklocation in this setup. Update the wording to point to rebuilding/reinstalling the shared runtime /plugin-install-mijia-python-sdk.
emit(
{
"success": False,
"message": "Python package requests not found in runtime. Repackage the plugin runtime or install with: pip install requests",
"code": "requests_missing",
},
1,
)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,3 @@ | |||
| requests>=2.31,<3 | |||
| qrcode>=7.4,<9 | |||
| mijiaAPI @ git+https://github.com/Do1e/mijia-api.git | |||
There was a problem hiding this comment.
mijiaAPI is pulled directly from a Git repository without pinning a tag/commit. This makes builds non-reproducible and introduces supply-chain risk if the upstream repo changes. Pin the dependency to a specific commit (or use a versioned PyPI release) so the runtime package build is deterministic.
| mijiaAPI @ git+https://github.com/Do1e/mijia-api.git | |
| mijiaAPI @ git+https://github.com/Do1e/mijia-api.git@<pinned-commit-sha> |
| just plugin-mijia-smart-home | ||
| ``` | ||
|
|
||
| This command packages runtime + builds WASM + installs the plugin to `~/.peekoo/plugins/mijia-smart-home`. |
There was a problem hiding this comment.
The README says plugin-mijia-smart-home “packages runtime”, but the current just recipe installs the runtime into the shared ~/.peekoo/python-sdk directory (and the plugin repo does not include a runtime/ directory). Please adjust this line to match the actual install flow (shared runtime vs plugin-bundled runtime).
| This command packages runtime + builds WASM + installs the plugin to `~/.peekoo/plugins/mijia-smart-home`. | |
| This command builds WASM + installs the plugin to `~/.peekoo/plugins/mijia-smart-home`, using the shared Peekoo Python SDK runtime installed above. |
| 5. `runtime/python/bin/python3` (plugin-local fallback) | ||
| 6. `runtime/python/bin/python` (plugin-local fallback) | ||
| 7. `runtime/python/python.exe` (plugin-local fallback) | ||
| 8. `runtime/python/bin/python.exe` (plugin-local fallback) | ||
| 9. system `python3` | ||
| 10. system `python` | ||
| 11. system `py` |
There was a problem hiding this comment.
This lookup order lists runtime/python/... plugin-local fallbacks, but this plugin tree currently doesn’t contain a runtime/ directory and plugin-mijia-smart-home doesn’t create one. Either document how/when runtime/ gets bundled, or remove these entries to avoid misleading users.
| 5. `runtime/python/bin/python3` (plugin-local fallback) | |
| 6. `runtime/python/bin/python` (plugin-local fallback) | |
| 7. `runtime/python/python.exe` (plugin-local fallback) | |
| 8. `runtime/python/bin/python.exe` (plugin-local fallback) | |
| 9. system `python3` | |
| 10. system `python` | |
| 11. system `py` | |
| 5. system `python3` | |
| 6. system `python` | |
| 7. system `py` |
| { | ||
| "success": False, | ||
| "message": "Python package mijiaAPI not found. Install it with: pip install mijiaAPI", | ||
| "message": "Python package mijiaAPI not found in runtime. Repackage the plugin runtime or install with: pip install mijiaAPI", | ||
| "code": "mijia_api_missing", | ||
| }, | ||
| 1, |
There was a problem hiding this comment.
This message says “Repackage the plugin runtime”, but this PR’s documented flow installs a shared runtime under ~/.peekoo/python-sdk (via just plugin-install-mijia-python-sdk). Consider rewording to direct users to rebuild/reinstall the shared Python SDK runtime (or the specific just target).
| let ctx = user_data.get().map_err(|e| Error::msg(format!("{e}")))?; | ||
| let ctx = ctx.lock().map_err(|e| Error::msg(format!("{e}")))?; | ||
| require_declared_capability(&ctx, "process:exec")?; | ||
| if let Err(err) = require_declared_capability(&ctx, "process:exec") { |
There was a problem hiding this comment.
host_process_exec only verifies that process:exec is declared (require_declared_capability) and does not check whether it is granted. This bypasses the optional permission model used elsewhere (require_capability). Please enforce the grant check here (you can still return an ok:false JSON response instead of trapping).
| if let Err(err) = require_declared_capability(&ctx, "process:exec") { | |
| if let Err(err) = require_capability(&ctx, "process:exec") { |
| "stderr": format!( | ||
| "Process spawn failed: program='{}' cwd='{}' ({e})", | ||
| resolved_program.display(), | ||
| resolved_cwd.display() | ||
| ), |
There was a problem hiding this comment.
The spawn error string includes resolved_program and resolved_cwd (often absolute paths under the user’s home directory). If this message is surfaced in plugin UIs/logs, it can leak local filesystem paths. Consider redacting/sanitizing these values or limiting detail.
| "stderr": format!( | |
| "Process spawn failed: program='{}' cwd='{}' ({e})", | |
| resolved_program.display(), | |
| resolved_cwd.display() | |
| ), | |
| "stderr": format!("Process spawn failed ({e})"), |
| { | ||
| "description": "Default permissions for the plugin\n#### This default permission set includes:\n\n- `allow-capture`\n- `allow-identify`\n- `allow-alias`\n- `allow-reset`\n- `allow-get-distinct-id`\n- `allow-get-config`", | ||
| "type": "string", | ||
| "const": "posthog:default", | ||
| "markdownDescription": "Default permissions for the plugin\n#### This default permission set includes:\n\n- `allow-capture`\n- `allow-identify`\n- `allow-alias`\n- `allow-reset`\n- `allow-get-distinct-id`\n- `allow-get-config`" |
There was a problem hiding this comment.
These PostHog permission constants appear unrelated to the shared Python SDK runtime changes described in the PR. If this schema update is accidental or unrelated, consider reverting or splitting it into a separate PR to keep scope focused.
| archive_url="${PEEKOO_PYTHON_STANDALONE_URL:-https://github.com/indygreg/python-build-standalone/releases/download/${release_tag}/cpython-3.12.9+${release_tag}-${target_triple}-install_only.tar.gz}" | ||
|
|
||
| workdir="$(mktemp -d)" | ||
| trap 'rm -rf "$workdir"' EXIT | ||
|
|
||
| archive_path="$workdir/python-runtime.tar.gz" | ||
| curl -fL "$archive_url" -o "$archive_path" |
There was a problem hiding this comment.
The runtime packaging script downloads and extracts a prebuilt Python archive without any integrity verification. Consider adding checksum/signature verification (e.g., pinned SHA256 for the archive URL) to reduce the risk of compromised downloads.
| archive_url="${PEEKOO_PYTHON_STANDALONE_URL:-https://github.com/indygreg/python-build-standalone/releases/download/${release_tag}/cpython-3.12.9+${release_tag}-${target_triple}-install_only.tar.gz}" | |
| workdir="$(mktemp -d)" | |
| trap 'rm -rf "$workdir"' EXIT | |
| archive_path="$workdir/python-runtime.tar.gz" | |
| curl -fL "$archive_url" -o "$archive_path" | |
| archive_url="${PEEKOO_PYTHON_STANDALONE_URL:-https://github.com/indygreg/python-build-standalone/releases/download/${release_tag}/cpython-3.12.9+${release_tag}-${target_triple}-install_only.tar.gz}" | |
| archive_sha256="${PEEKOO_PYTHON_STANDALONE_SHA256:-}" | |
| if [[ -z "$archive_sha256" ]]; then | |
| echo "missing required SHA256 for runtime archive: set PEEKOO_PYTHON_STANDALONE_SHA256" >&2 | |
| exit 1 | |
| fi | |
| workdir="$(mktemp -d)" | |
| trap 'rm -rf "$workdir"' EXIT | |
| archive_path="$workdir/python-runtime.tar.gz" | |
| curl -fL "$archive_url" -o "$archive_path" | |
| if command -v sha256sum >/dev/null 2>&1; then | |
| actual_sha256="$(sha256sum "$archive_path" | awk '{print $1}')" | |
| elif command -v shasum >/dev/null 2>&1; then | |
| actual_sha256="$(shasum -a 256 "$archive_path" | awk '{print $1}')" | |
| else | |
| echo "no SHA-256 tool available; install sha256sum or shasum" >&2 | |
| exit 1 | |
| fi | |
| if [[ "$actual_sha256" != "$archive_sha256" ]]; then | |
| echo "downloaded runtime archive SHA256 mismatch" >&2 | |
| echo "expected: $archive_sha256" >&2 | |
| echo "actual: $actual_sha256" >&2 | |
| exit 1 | |
| fi |
What changed
Release notes
feature,fix,docs,test,chore,ci, orrefactorskip-changelogif this PR should be excluded from generated release notesVerification
https://github.com/feed-mob/tracking_admin/issues/22011