Skip to content

feat!: remove Python recipe runner — Rust-only execution#2962

Merged
rysweet merged 3 commits intomainfrom
feat/remove-python-recipe-runner
Mar 9, 2026
Merged

feat!: remove Python recipe runner — Rust-only execution#2962
rysweet merged 3 commits intomainfrom
feat/remove-python-recipe-runner

Conversation

@rysweet
Copy link
Owner

@rysweet rysweet commented Mar 9, 2026

Summary

  • Remove the Python recipe runner (RecipeRunner, RecipeContext, adapters/) — -12,241 lines
  • All recipe execution now goes through recipe-runner-rs (Rust binary)
  • run_recipe_by_name() accepts adapter kwarg for backward compat but ignores it
  • Version bumped to 0.6.0 (breaking change)

What was removed

Component Lines Purpose
runner.py 472 Python execution engine
context.py 260 Template rendering + condition eval
adapters/ (6 files) 691 SDK adapters (CLI, Claude SDK, nested session)
Tests (24 files) 10,818 Tests for the above

What was updated

  • recipes/__init__.py — Rust-only run_recipe_by_name(), removed Python imports
  • recipe_cli/recipe_command.pyhandle_run uses run_recipe_via_rust()
  • multitask/orchestrator.py — Launcher no longer imports CLISubprocessAdapter
  • tests/workflows/test_regression.py — Updated import checks
  • pyproject.toml — Version 0.5.121 → 0.6.0

Migration

# Before (Python runner)
from amplihack.recipes import run_recipe_by_name
from amplihack.recipes.adapters.cli_subprocess import CLISubprocessAdapter
adapter = CLISubprocessAdapter()
result = run_recipe_by_name("my-recipe", adapter=adapter)

# After (Rust runner) 
from amplihack.recipes import run_recipe_by_name
result = run_recipe_by_name("my-recipe")

Prerequisites

recipe-runner-rs must be installed: cargo install --git https://github.com/rysweet/amplihack-recipe-runner

Test plan

  • python -c "from amplihack.recipes import run_recipe_by_name" works
  • from amplihack.recipes import RecipeRunner raises ImportError
  • from amplihack.recipes.adapters import CLISubprocessAdapter raises ModuleNotFoundError
  • CI passes (may need test adjustments for remaining references)
  • Verify recipe-runner-rs is installed in CI environment

🤖 Generated with Claude Code

BREAKING: The Python recipe runner (RecipeRunner, RecipeContext, adapters/)
has been removed. All recipe execution now goes through recipe-runner-rs.

- Remove runner.py, context.py, and adapters/ directory
- Update __init__.py to export only Rust runner functions
- run_recipe_by_name() now always delegates to run_recipe_via_rust()
- Accept but ignore `adapter` kwarg for backward compatibility
- Update recipe_command.py CLI to use Rust runner
- Update multitask orchestrator launcher to remove adapter import
- Remove 12,241 lines of Python runner code and tests
- Bump version to 0.6.0

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Contributor

github-actions bot commented Mar 9, 2026

Repo Guardian - Passed

All 37 changed files have been reviewed. This PR contains legitimate code changes as part of the Python→Rust recipe runner migration:

  • 30 removed files: Python recipe runner implementation and tests (runner.py, context.py, adapters/, test files)
  • 6 modified files: API wrappers, version bump, and test updates to route through Rust runner
  • 1 modified file: pyproject.toml version 0.5.121 → 0.6.0

✅ No ephemeral content detected (no meeting notes, investigation notes, temporary scripts, or point-in-time documents)

AI generated by Repo Guardian

- Remove test_explicit_python_engine (engine no longer exists)
- Remove test_auto_detect_uses_python_when_no_rust (no fallback)
- Remove test_invalid_engine_raises_valueerror (env var not checked)
- Add test_always_uses_rust, test_adapter_kwarg_accepted_but_ignored
- Add test_python_runner_no_longer_importable, test_adapters_no_longer_importable
- Add test_context_no_longer_importable

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions github-actions bot mentioned this pull request Mar 9, 2026
24 tests across 7 scenarios verifying from the user's perspective:

1. Real recipe dry-run execution (default-workflow, smart-orchestrator,
   investigation-workflow) — actually invokes the Rust binary
2. Recipe discovery and parsing (list, find, parse YAML, parse real file)
3. Python runner completely gone (modules, classes, source files)
4. Backward compatibility (adapter kwarg accepted but ignored)
5. Error behavior (clear error when Rust binary missing, unknown recipe)
6. Rust runner result structure (recipe_name, step_results, success)
7. CLI module clean (no RecipeRunner/CLISubprocessAdapter references)

Also includes gadugi-agentic-test YAML scenario for CI integration.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Contributor

github-actions bot commented Mar 9, 2026

🔴 PR Triage: BLOCKED — Do Not Merge

Triage Date: 2026-03-09T02:32:47Z
Risk Level: 🔴 EXTREME
Priority: 🔴 CRITICAL
Status: ❌ BLOCKED


Summary

This PR removes the entire Python recipe runner (-12,241 LOC) and creates a hard dependency on the external Rust binary recipe-runner-rs. This is a breaking change that will break all recipe execution in environments without the Rust toolchain.


Critical Blockers (Must Fix Before Merge)

1. ❌ Rust Binary Not in CI

The PR assumes recipe-runner-rs is available but doesn't ensure it's installed in CI. The test plan shows unchecked item: "Verify recipe-runner-rs is installed in CI environment".

Required action: Add step to CI workflow to install Rust toolchain + cargo binary

2. ❌ Test Plan Incomplete

Test plan shows unchecked items:

  • CI passes (may need test adjustments for remaining references)
  • Verify recipe-runner-rs is installed in CI environment

Required action: Verify all tests pass with Rust runner before merge

3. ❌ No Fallback Strategy

The PR description states: "Prerequisites: recipe-runner-rs must be installed". There's no graceful degradation or fallback.

Required action: Either:

  • Document that this is a breaking change requiring Rust toolchain
  • Add fallback to Python runner with deprecation warning
  • Add startup check that fails fast with clear error if binary missing

4. ⚠️ Massive Test Coverage Deletion

Removes 24 test files covering 10,818 LOC. Are these tests ported to Rust? If not, we're losing coverage.

Required action: Document test migration strategy


Breaking Change Impact

This change affects every user of the recipe system:

  • Migration required: Users must install Rust toolchain + cargo
  • CI/CD impact: All CI environments need Rust binary
  • Deployment impact: Production environments need Rust
  • Developer impact: All contributors need Rust installed

This is a major breaking change that needs:

  1. Communication plan (announcement, migration guide)
  2. Version policy (is 0.6.0 appropriate for this scale of breaking change?)
  3. Timeline for deprecation (or is this immediate?)

Recommendations

Do NOT merge until:

  1. ✅ Rust binary installed in CI (verify in GitHub Actions workflow)
  2. ✅ All tests pass with Rust runner
  3. ✅ Test migration documented (where did 10K LOC of tests go?)
  4. ✅ Fallback strategy defined (or explicit acceptance of breakage)
  5. ✅ Migration guide validated by attempting migration on clean environment
  6. ✅ Breaking change communicated to users

Estimated effort to unblock: 2-4 hours


Alternative Approach

Consider phased migration instead:

  1. Phase 1: Add Rust runner as optional (keep Python as fallback)
  2. Phase 2: Mark Python runner as deprecated
  3. Phase 3: Remove Python runner (this PR) after adoption period

This reduces risk and gives users time to migrate.


Triage tracking: See #2964 for full report

AI generated by PR Triage Agent

@rysweet rysweet merged commit f8009bd into main Mar 9, 2026
23 checks passed
@rysweet rysweet deleted the feat/remove-python-recipe-runner branch March 9, 2026 02:43
rysweet pushed a commit that referenced this pull request Mar 10, 2026
The adapters namespace package still exists (as an empty directory) but
CLISubprocessAdapter was removed by PR #2962. The import raises ImportError
(name not found in module), not ModuleNotFoundError (module itself missing).

Also removed stale __pycache__ from the empty adapters directory.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
rysweet added a commit that referenced this pull request Mar 10, 2026
…rs (#2998)

* fix: correct install message and add version check for recipe-runner-rs

- install.py: Change misleading 'Python runner will be used' to
  'recipe execution will fail without it' since Python runner was removed
- rust_runner.py: Add get_runner_version(), check_runner_version(),
  MIN_RUNNER_VERSION constant for binary compatibility checking
- Version check runs on every recipe execution (best-effort, warns on old binary)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* [skip ci] chore: Auto-bump patch version

* fix: correct test_adapters_no_longer_importable exception type

The adapters namespace package still exists (as an empty directory) but
CLISubprocessAdapter was removed by PR #2962. The import raises ImportError
(name not found in module), not ModuleNotFoundError (module itself missing).

Also removed stale __pycache__ from the empty adapters directory.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* [skip ci] chore: Auto-bump patch version

---------

Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant