Skip to content

fix(api): use sys.executable for plugin action subprocess calls#277

Merged
ChuckBuilds merged 2 commits intoChuckBuilds:mainfrom
5ymb01:fix/plugin-action-sys-executable
Mar 21, 2026
Merged

fix(api): use sys.executable for plugin action subprocess calls#277
ChuckBuilds merged 2 commits intoChuckBuilds:mainfrom
5ymb01:fix/plugin-action-sys-executable

Conversation

@5ymb01
Copy link
Copy Markdown
Contributor

@5ymb01 5ymb01 commented Mar 2, 2026

Description

execute_plugin_action hardcodes 'python3' in subprocess calls, which breaks in virtualenvs or when the system Python binary has a different name. Also, import sys inside a conditional block shadowed the module-level import.

Type of Change

  • Bug fix (non-breaking change)
  • New feature (non-breaking change)
  • Breaking change
  • Documentation update
  • Performance improvement
  • Refactoring

Changes

  • Replace all 6 ['python3', ...] subprocess calls with [sys.executable, ...]:
    • OAuth redirect wrapper (line ~5098)
    • Parameterized action wrapper (line ~5150)
    • Stdin-param wrapper (line ~5211)
    • No-param wrapper (line ~5417)
    • Auth script execution (line ~5524)
    • Original non-OAuth branch (already fixed in first commit)
  • Remove redundant import sys from inside the oauth_flow conditional block

Why this matters

  • Portability: sys.executable always points to the interpreter running the current process, regardless of system naming or virtualenv configuration
  • Safety: Removing the shadowed import eliminates a latent UnboundLocalError risk

Testing

  • Tested on Raspberry Pi hardware
  • Verified display rendering works correctly
  • Checked API integration functionality
  • Tested error handling scenarios

Test Plan

  • Execute a plugin action with parameters (e.g., of-the-day file upload) — verify it completes successfully
  • Execute a plugin action without parameters — verify script runs correctly
  • Execute an OAuth plugin action — verify redirect flow works
  • Verify sys.executable resolves to the correct Python interpreter` resolves to the correct Python interpreter

Checklist

  • Code follows project style guidelines
  • Self-review completed
  • Comments added for complex logic
  • No hardcoded values or API keys
  • Error handling implemented
  • Logging added where appropriate

Author: @5ymb01
AI Assistant: Claude Opus 4.6 (Anthropic)

🤖 Generated with Claude Code

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 2, 2026

📝 Walkthrough

Walkthrough

Removed a redundant local sys import in the OAuth branch and changed script execution to use sys.executable instead of a hardcoded python3, aligning the invoked interpreter with the running environment.

Changes

Cohort / File(s) Summary
Script Execution Portability
web_interface/blueprints/api_v3.py
Removed redundant local sys import inside OAuth flow; replaced hardcoded python3 with sys.executable for invoking scripts.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically summarizes the main change: replacing hardcoded 'python3' with sys.executable for plugin action subprocess calls.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
web_interface/blueprints/api_v3.py (1)

5150-5150: ⚠️ Potential issue | 🟠 Major

Remaining hardcoded python3 keeps the same bug for parameterized plugin actions.

Line 5150 and Line 5211 still hardcode python3, so actions with params/OAuth step 2 can fail in environments where only the current interpreter path is valid.

♻️ Proposed fix
-                        result = subprocess.run(
-                            ['python3', wrapper_path],
+                        result = subprocess.run(
+                            [sys.executable, wrapper_path],
                             capture_output=True,
                             text=True,
                             timeout=120,
                             env=env
                         )
...
-                        result = subprocess.run(
-                            ['python3', wrapper_path],
+                        result = subprocess.run(
+                            [sys.executable, wrapper_path],
                             capture_output=True,
                             text=True,
                             timeout=120,
                             env=env
                         )

As per coding guidelines, "Handle different deployment contexts with environment awareness in code".

Also applies to: 5211-5211

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

In `@web_interface/blueprints/api_v3.py` at line 5150, The code currently
hardcodes 'python3' in the command invocation that runs wrapper_path which
breaks in environments where the running interpreter is different; update the
command lists that include ['python3', wrapper_path] to use the running
interpreter (sys.executable) instead, add "import sys" if not present, and
change both occurrences around the wrapper_path use (the two hardcoded lines
noted) so they consistently use sys.executable to spawn the wrapper with the
current Python binary.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@web_interface/blueprints/api_v3.py`:
- Line 5150: The code currently hardcodes 'python3' in the command invocation
that runs wrapper_path which breaks in environments where the running
interpreter is different; update the command lists that include ['python3',
wrapper_path] to use the running interpreter (sys.executable) instead, add
"import sys" if not present, and change both occurrences around the wrapper_path
use (the two hardcoded lines noted) so they consistently use sys.executable to
spawn the wrapper with the current Python binary.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fe5c1d0 and b2aa11e.

📒 Files selected for processing (1)
  • web_interface/blueprints/api_v3.py

The execute_plugin_action endpoint hardcoded 'python3' when spawning
plugin scripts via subprocess. This can fail if the system Python is
named differently or if a virtualenv is active, since 'python3' may
not point to the correct interpreter.

Changes:
- Replace 'python3' with sys.executable in the non-OAuth script
  execution branch (uses the same interpreter running the web service)
- Remove redundant 'import sys' inside the oauth_flow conditional
  block (sys is already imported at module level; the local import
  shadows the top-level binding for the entire function scope, which
  would cause UnboundLocalError if sys were referenced in the else
  branch on Python 3.12+)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@5ymb01 5ymb01 force-pushed the fix/plugin-action-sys-executable branch from b2aa11e to 118f1c5 Compare March 7, 2026 08:54
Fix 4 additional subprocess calls that still used 'python3' instead of
sys.executable: parameterized action wrapper (line 5150), stdin-param
wrapper (line 5211), no-param wrapper (line 5417), and OAuth auth
script (line 5524). Ensures plugin actions work in virtualenvs and
non-standard Python installations.

Addresses CodeRabbit findings on PR ChuckBuilds#277.

Co-Authored-By: 5ymb01 <5ymb01@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@5ymb01
Copy link
Copy Markdown
Contributor Author

5ymb01 commented Mar 20, 2026

Test Results

Tested on Raspberry Pi (ledpi), Python 3.13.5.

Verification

  • sys.executable resolves correctly/usr/bin/python3 on Pi
  • sys.executable used in plugin actions — confirmed at 5 call sites in api_v3.py (lines 5132, 5150, 5193, 5211, 5306)
  • Plugin action endpoint validates input — returns proper error for missing plugin_id/action_id
  • Plugin action endpoint finds plugin directory — correctly locates plugin and reads manifest
  • Action not found handled gracefully — returns "Action refresh not found in plugin manifest" (no crash, no traceback)

Unit Tests

290 passed, 4 pre-existing failures (stale config mock — fixed in merged PR #281)

Note: OAuth redirect flow not tested (no OAuth plugin currently configured on test Pi). The code path uses the same sys.executable subprocess pattern.

@ChuckBuilds ChuckBuilds merged commit c8737d1 into ChuckBuilds:main Mar 21, 2026
1 check passed
5ymb01 added a commit to 5ymb01/LEDMatrix that referenced this pull request Mar 23, 2026
Reconcile squash-merged PRs (ChuckBuilds#277, ChuckBuilds#290, ChuckBuilds#291) from upstream.
Resolve conflict in vegas_mode/coordinator.py: keep both the
interrupt check callback (upstream) and inline cycle restart (Dev).

Co-Authored-By: 5ymb01 <noreply@github.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
5ymb01 added a commit to 5ymb01/LEDMatrix that referenced this pull request Mar 23, 2026
Reconcile squash-merged PRs (ChuckBuilds#277, ChuckBuilds#290, ChuckBuilds#291) from upstream.
Resolve conflict in vegas_mode/coordinator.py: keep both the
interrupt check callback (upstream) and inline cycle restart (wip).

Co-Authored-By: 5ymb01 <noreply@github.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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.

2 participants