-
Notifications
You must be signed in to change notification settings - Fork 7
Local Runner Options for /erk:plan-submit #6061
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Local Runner Options for /erk:plan-submit #6061
Conversation
Implements a tmux-based local runner as an alternative to GitHub Actions
for plan implementations, enabling faster iteration and live debugging.
Key Components:
- Add --local flag to `erk plan submit` command
- Implement local_runner.py orchestration with credential management
- Add CLI commands: `erk local-runner status/logs/stop`
- Create credential config template at .erk/local-runner-config.toml.example
- Update plan-submit documentation with local execution guide
Architecture:
- Uses git worktrees via `erk prepare` for isolation
- Launches tmux sessions for monitoring (erk-impl-{issue})
- Loads credentials from ~/.erk/local-runner-config.toml
- Runs `erk impl` directly in isolated worktree with venv
Benefits:
- No GitHub Actions queue delay
- Live debugging via tmux attach
- Full control over execution environment
- Simple setup with minimal dependencies
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
| Raises: | ||
| ValueError: If config file is missing or incomplete | ||
| """ | ||
| config_path = Path.home() / ".erk" / "local-runner-config.toml" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tripwire: Path.home() used directly - Use gateway abstractions instead. For ~/.erk/ paths use ErkInstallation gateway. Direct Path.home() access bypasses testability (fakes) and creates parallel test flakiness. See Exec Script Testing Patterns.
| @local_runner.command() | ||
| def status() -> None: | ||
| """List active local implementation sessions.""" | ||
| result = subprocess.run( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dignified Python: Missing check=True on subprocess.run - Subprocess calls must use check=True to ensure errors are not silently ignored. Without it, tmux list-sessions failures (e.g., tmux not installed) will be missed. Add check=True and handle CalledProcessError explicitly.
| """Attach to tmux session for a running implementation.""" | ||
| session_name = f"erk-impl-{issue_number}" | ||
|
|
||
| # Check if session exists |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dignified Python: Missing check=True on subprocess.run - Subprocess calls must use check=True to ensure errors are not silently ignored. The code checks returncode manually, but this creates silent failure paths. Use check=True and catch CalledProcessError instead.
| @local_runner.command() | ||
| def status() -> None: | ||
| """List active local implementation sessions.""" | ||
| result = subprocess.run( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tripwire: subprocess.run without wrapper - While this uses check=False (LBYL pattern), consider using run_with_error_reporting() or explicit CalledProcessError handling for consistency with erk patterns. See Subprocess Wrappers.
| """Stop a running local implementation.""" | ||
| session_name = f"erk-impl-{issue_number}" | ||
|
|
||
| result = subprocess.run( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dignified Python: Missing check=True on subprocess.run - Subprocess calls must use check=True to ensure errors are not silently ignored. The code checks returncode manually, but this creates silent failure paths. Use check=True and catch CalledProcessError instead.
|
|
||
| @local_runner.command() | ||
| def status() -> None: | ||
| """List active local implementation sessions.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dignified Code Simplifier Review: Bare subprocess.run detected - According to the Subprocess Wrappers tripwire, bare subprocess.run should use wrapper functions. Use run_subprocess_with_context() (gateway) or run_with_error_reporting() (CLI). Exception: Graceful degradation pattern with explicit CalledProcessError handling is acceptable for optional operations. This appears to be a graceful degradation case (checking tmux sessions), so explicit CalledProcessError handling would make the intent clearer.
| session_name = f"erk-impl-{issue_number}" | ||
|
|
||
| # Check if session exists | ||
| result = subprocess.run( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dignified Code Simplifier Review: Bare subprocess.run detected - According to the Subprocess Wrappers tripwire, bare subprocess.run should use wrapper functions. Use run_subprocess_with_context() (gateway) or run_with_error_reporting() (CLI). Exception: Graceful degradation pattern with explicit CalledProcessError handling is acceptable for optional operations. This appears to be a graceful degradation case (checking tmux session exists), so explicit CalledProcessError handling would make the intent clearer.
| user_output(click.style("Error: ", fg="red") + msg) | ||
| return | ||
|
|
||
| # Attach to session (this replaces the current process) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dignified Code Simplifier Review: Bare subprocess.run detected - According to the Subprocess Wrappers tripwire, bare subprocess.run should use wrapper functions. Use run_subprocess_with_context() (gateway) or run_with_error_reporting() (CLI). This case is particularly important as it replaces the current process - error handling should be explicit.
| """Stop a running local implementation.""" | ||
| session_name = f"erk-impl-{issue_number}" | ||
|
|
||
| result = subprocess.run( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dignified Code Simplifier Review: Bare subprocess.run detected - According to the Subprocess Wrappers tripwire, bare subprocess.run should use wrapper functions. Use run_subprocess_with_context() (gateway) or run_with_error_reporting() (CLI). Exception: Graceful degradation pattern with explicit CalledProcessError handling is acceptable for optional operations. This appears to be a graceful degradation case (stopping tmux session), so explicit CalledProcessError handling would make the intent clearer.
|
|
||
| # Create worktree via erk prepare | ||
| user_output(f"Creating worktree for issue #{issue_number}...") | ||
| try: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dignified Code Simplifier Review: Bare subprocess.run with check=True detected - According to the Subprocess Wrappers tripwire, bare subprocess.run with check=True should use wrapper functions: run_subprocess_with_context() (gateway) or run_with_error_reporting() (CLI). The current try/except CalledProcessError pattern is acceptable but using the wrapper would provide better error reporting consistency.
| """ | ||
|
|
||
| # Launch tmux session | ||
| user_output(f"Launching tmux session: {session_name}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dignified Code Simplifier Review: Bare subprocess.run with check=True detected - According to the Subprocess Wrappers tripwire, bare subprocess.run with check=True should use wrapper functions: run_subprocess_with_context() (gateway) or run_with_error_reporting() (CLI). The current try/except CalledProcessError pattern is acceptable but using the wrapper would provide better error reporting consistency.
❌ Dignified Python ReviewLast updated: 2026-01-25 18:26:18 PT Found 3 violations across 1 file. Inline comments posted for each. Patterns Checked✅ LBYL over EAFP - None found Violations Summary
Files Reviewed
Activity Log
|
❌ Tripwires ReviewLast updated: 2026-01-25 18:26:21 PT Found 2 violations across 2 files. Inline comments posted for each. Tripwires Triggered
Patterns Checked❌ Path.home() direct usage - Found in src/erk/cli/local_runner.py:30 Violations Summary
Files Reviewed
Activity Log
|
🤖 Automated fix by CI autofix workflow Failed checks: prettier Fixed: - .claude/commands/erk/plan-submit.md: markdown formatting
❌ Dignified Code Simplifier ReviewLast updated: 2026-01-25 18:26:35 PT Found 6 violations across 2 files. Inline comments posted for each. Patterns Checked❌ Bare subprocess.run - Found in src/erk/cli/commands/local_runner_cmd.py (4 instances) and src/erk/cli/local_runner.py (2 instances) Violations Summary
Files Reviewed
Activity Log
|
Author: @eric
Plan: #6050
Implements a tmux-based local runner as an alternative to GitHub Actions for plan implementations, enabling faster iteration and live debugging.
Summary
--localflag toerk plan submitcommand for local execution~/.erk/local-runner-config.tomlerk local-runner status/logs/stopfor monitoring.erk/local-runner-config.toml.exampleArchitecture
erk preparefor isolationerk-impl-{issue})erk impldirectly in isolated worktree with venvUsage
Benefits
Test plan
erk plan submit --localflag is recognized~/.erk/local-runner-config.tomlerk prepareCloses #6050
erk pr checkout 6061
🤖 Generated with Claude Code