Skip to content

fix: stream recipe-runner progress in Copilot launcher#3026

Merged
rysweet merged 14 commits intomainfrom
feat/issue-3024-runner-streaming
Mar 10, 2026
Merged

fix: stream recipe-runner progress in Copilot launcher#3026
rysweet merged 14 commits intomainfrom
feat/issue-3024-runner-streaming

Conversation

@rysweet
Copy link
Owner

@rysweet rysweet commented Mar 10, 2026

Summary

  • add explicit progress support to run_recipe_by_name() and the Rust recipe-runner wrapper
  • stream recipe-runner stderr live in progress mode while preserving stdout for JSON parsing
  • update the dev-orchestrator launch path to opt into progress=True and cover it with regression tests

Linked Issue

Step 13: Local Testing Results

Test Environment: /home/azureuser/src/amplihack12/worktrees/issue-3024-runner-streaming using repo-managed uv Python environment and local tmux session

Tests Executed:

  1. Simple: uv run python -m pytest tests/recipes/test_rust_runner.py tests/skills/test_dev_orchestrator_issue_3002.py tests/skills/test_dev_orchestrator_issue_3010.py tests/skills/test_dev_orchestrator_issue_3011.py tests/skills/test_dev_orchestrator_issue_3024.py -q -> passed ✅
  2. Complex: run_recipe_by_name('smart-orchestrator', dry_run=True, progress=True) from the checked-out source tree -> progress markers streamed during execution ✅

Regressions: existing #3002, #3010, and #3011 skill regression coverage still passed ✅ None detected
Issues Found: initial wrapper implementation buffered progress output; fixed by using a progress-specific Popen path that streams stderr while preserving stdout for JSON parsing

Step 19: Outside-In Testing Results

Test Environment: local tmux-backed CLI simulation with PYTHONPATH=src python3 from the feature worktree
Interface Type: CLI / orchestrator launch path

User Flows Tested:

  1. Flow 1: direct Python API dry-run with progress enabled -> success ✅

    • Commands/Actions: run_recipe_by_name('smart-orchestrator', dry_run=True, progress=True)
    • Expected: step/progress markers visible during execution while JSON handling remains intact
    • Actual: progress output streamed live to stderr during the run
    • Evidence: local command output showed markers such as ▶ classify-and-decompose
  2. Flow 2: tmux-style launch path used by the skill documentation -> success ✅

    • Commands/Actions: launch the documented Python snippet in tmux and inspect the resulting log output
    • Expected: tmux log shows activity during execution instead of staying blank until process exit
    • Actual: log file contained progress lines and step markers during execution
    • Evidence: /tmp/rr3024-tmux.log captured the streamed progress output

Edge Cases Tested: progress mode still preserved machine-readable stdout for the wrapper path ✅
Integration Points Verified: Python launcher -> Rust recipe-runner invocation path ✅
Observability Check: stderr progress output visible in realistic tmux/log usage ✅
Issues Found: none after the streaming/return-code fixes

Notes

  • #3010 and #3011 were already fixed on origin/main; this PR is the remaining launcher/wrapper side of #3024
  • paired runner-side changes are in rysweet/amplihack-recipe-runner#19

Copilot bot and others added 2 commits March 10, 2026 16:46
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions
Copy link
Contributor

🤖 Auto-fixed version bump

The version in pyproject.toml has been automatically bumped to the next patch version.

If you need a minor or major version bump instead, please update pyproject.toml manually and push the change.

@rysweet
Copy link
Owner Author

rysweet commented Mar 10, 2026

Philosophy / Compliance Review — PR #3026

Verdict: Clean ✓

All six amplihack principles pass:

Principle Status Evidence
Ruthless simplicity Single progress bool threads cleanly through 3 layers; no new abstractions
Zero-BS implementation progress=False preserves existing path; True does exactly one thing (relay stderr live)
No silent fallbacks TimeoutExpired → kill → re-raise; JSON parse errors raise RuntimeError with context
No swallowed exceptions No bare except, no pass blocks; all errors propagate with diagnostics
Understandable functions _stream_process_output is ~25 lines; thread drainers are trivial closures
Explicit over magic Opt-in via explicit kwarg; --progress flag passed explicitly to Rust binary

Minor nit (non-blocking): hasattr(process, "kill") guard in _stream_process_output is always True for Popen — could simplify to bare process.kill().

~80% of the test diff is formatting normalization (line-length compliance), not functional changes. New TestProgressStreaming class provides good coverage of the streaming path.

🤖 Generated with Claude Code

Copilot bot and others added 2 commits March 10, 2026 17:03
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@rysweet
Copy link
Owner Author

rysweet commented Mar 10, 2026

Step 16b: Reviewer check

Review outcome: one substantive issue was found during review and has been addressed in f9b9a94c.

  • Issue found: _stream_process_output() could return after join(timeout=1) with drain threads still alive, which risked partial stdout/stderr capture and incomplete JSON.
  • Fix applied: the wrapper now waits for process exit (or kills + waits on timeout) and then joins both drain threads fully before returning.
  • Validation: uv run python -m pytest tests/recipes/test_rust_runner.py tests/skills/test_dev_orchestrator_issue_3024.py -q and pre-commit run --files .claude/skills/dev-orchestrator/SKILL.md src/amplihack/recipes/rust_runner.py tests/skills/test_dev_orchestrator_issue_3024.py both passed.

@github-actions
Copy link
Contributor

🤖 Auto-fixed version bump

The version in pyproject.toml has been automatically bumped to the next patch version.

If you need a minor or major version bump instead, please update pyproject.toml manually and push the change.

@rysweet
Copy link
Owner Author

rysweet commented Mar 10, 2026

Step 16c: Security review

Security outcome: one medium concern was found in the operator guidance and has been addressed in f9b9a94c.

  • Issue found: the documented tmux launch path wrote streamed runner output to a predictable /tmp/recipe-runner-output.log, which widened exposure of nested output/log data.
  • Fix applied: the skill now uses mktemp /tmp/recipe-runner-output.XXXXXX.log and chmod 600 "$LOG_FILE" before writing logs.
  • Additional check: the Python wrapper continues to invoke the Rust runner with argv lists, so no shell-injection issue was introduced by progress=True.

@rysweet
Copy link
Owner Author

rysweet commented Mar 10, 2026

Step 16d: Philosophy compliance

Philosophy outcome: one zero-BS / no-silent-fallback issue was found and fixed in f9b9a94c.

  • Issue found: timed thread joins created an implicit "return what we have" path, which is a silent partial-data fallback for JSON/stdout capture.
  • Fix applied: the wrapper now blocks until the reader threads finish after process exit, removing the implicit fallback.
  • Remaining design looks aligned: progress stays explicit and opt-in, non-progress execution preserves the old quiet path, and the implementation remains small and direct.

@rysweet
Copy link
Owner Author

rysweet commented Mar 10, 2026

Step 17: Review feedback addressed

Addressed the review findings with follow-up commit f9b9a94c:

  • removed the partial-output risk in _stream_process_output()
  • hardened the tmux log guidance to use a private temp file
  • extended the skill regression test to cover the private log path guidance

Post-fix validation passed:

  • uv run python -m pytest tests/recipes/test_rust_runner.py tests/skills/test_dev_orchestrator_issue_3024.py -q
  • pre-commit run --files .claude/skills/dev-orchestrator/SKILL.md src/amplihack/recipes/rust_runner.py tests/skills/test_dev_orchestrator_issue_3024.py

@rysweet
Copy link
Owner Author

rysweet commented Mar 10, 2026

Step 21: Ready for review\nAll required workflow artifacts are now present on this PR: Step 13 and Step 19 evidence are in the description, Step 16 review/security/philosophy comments are posted, and the follow-up review fixes are pushed and validated. Marking this PR ready for external review.

@rysweet rysweet marked this pull request as ready for review March 10, 2026 17:04
Copilot bot added 2 commits March 10, 2026 17:07
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions
Copy link
Contributor

Repo Guardian - Passed

All 7 changed files are durable project artifacts. No ephemeral content detected.

File Assessment
.claude/skills/dev-orchestrator/SKILL.md ✅ Permanent skill documentation
pyproject.toml ✅ Project configuration (version bump)
src/amplihack/recipes/__init__.py ✅ Production source code
src/amplihack/recipes/rust_runner.py ✅ Production source code
tests/recipes/test_rust_runner.py ✅ Permanent test suite
tests/skills/test_dev_orchestrator_issue_3024.py ✅ Regression test (issue-numbered tests are a durable convention)
uv.lock ✅ Dependency lock file

Generated by Repo Guardian for issue #3026 ·

Copilot bot and others added 2 commits March 10, 2026 17:40
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions
Copy link
Contributor

🤖 Auto-fixed version bump

The version in pyproject.toml has been automatically bumped to the next patch version.

If you need a minor or major version bump instead, please update pyproject.toml manually and push the change.

@github-actions
Copy link
Contributor

Repo Guardian - Passed

All 13 changed files are durable project artifacts. No ephemeral content detected.

File Type Assessment
.claude/bin/.gitkeep Placeholder ✅ Standard gitkeep file
.claude/skills/dev-orchestrator/SKILL.md Documentation ✅ Permanent skill documentation
build_hooks.py Toolchain ✅ Project build toolchain
pyproject.toml Configuration ✅ Project configuration (version bump)
src/amplihack/__init__.py Source code ✅ Production source code
src/amplihack/recipes/__init__.py Source code ✅ Production source code
src/amplihack/recipes/rust_runner.py Source code ✅ Production source code
src/amplihack/settings.py Source code ✅ Production source code
tests/recipes/test_rust_runner.py Tests ✅ Permanent test suite
tests/skills/test_dev_orchestrator_issue_3024.py Tests ✅ Regression test (issue-referenced tests are a durable convention)
tests/test_rust_hook_engine.py Tests ✅ Permanent test suite
tests/test_wheel_packaging.py Tests ✅ Permanent test suite
uv.lock Lock file ✅ Dependency lock file

Generated by Repo Guardian for issue #3026 ·

Copilot bot added 4 commits March 10, 2026 18:46
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions
Copy link
Contributor

Repo Guardian - Passed

All 15 changed files are durable project artifacts. No ephemeral content detected.

File Type Assessment
.claude/bin/.gitkeep Placeholder ✅ Standard gitkeep file
.claude/skills/dev-orchestrator/SKILL.md Documentation ✅ Permanent skill documentation
README.md Documentation ✅ Project README
build_hooks.py Toolchain ✅ Project build toolchain
pyproject.toml Configuration ✅ Project configuration (version bump)
src/amplihack/__init__.py Source code ✅ Production source code
src/amplihack/recipes/__init__.py Source code ✅ Production source code
src/amplihack/recipes/rust_runner.py Source code ✅ Production source code
src/amplihack/rust_trial.py Source code ✅ Pre-existing module (import modernization only)
src/amplihack/settings.py Source code ✅ Production source code
tests/recipes/test_rust_runner.py Tests ✅ Permanent test suite
tests/skills/test_dev_orchestrator_issue_3024.py Tests ✅ Regression test (issue-referenced tests are a durable convention)
tests/test_rust_hook_engine.py Tests ✅ Permanent test suite
tests/test_wheel_packaging.py Tests ✅ Permanent test suite
uv.lock Lock file ✅ Dependency lock file

Generated by Repo Guardian for issue #3026 ·

Copilot bot and others added 2 commits March 10, 2026 18:52
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions
Copy link
Contributor

🤖 Auto-fixed version bump

The version in pyproject.toml has been automatically bumped to the next patch version.

If you need a minor or major version bump instead, please update pyproject.toml manually and push the change.

@rysweet rysweet merged commit 4e2f96e into main Mar 10, 2026
1 check passed
@rysweet rysweet deleted the feat/issue-3024-runner-streaming branch March 10, 2026 18:55
@github-actions
Copy link
Contributor

Repo Guardian - Passed

All 16 changed files are durable project artifacts. No ephemeral content detected.

File Type Assessment
.claude/bin/.gitkeep Placeholder ✅ Standard gitkeep file
.claude/skills/dev-orchestrator/SKILL.md Documentation ✅ Permanent skill documentation
README.md Documentation ✅ Project README
build_hooks.py Toolchain ✅ Project build toolchain
pyproject.toml Configuration ✅ Project configuration (version bump)
src/amplihack/__init__.py Source code ✅ Production source code
src/amplihack/recipes/__init__.py Source code ✅ Production source code
src/amplihack/recipes/rust_runner.py Source code ✅ Production source code
src/amplihack/rust_trial.py Source code ✅ Production source code
src/amplihack/settings.py Source code ✅ Production source code
tests/recipes/test_rust_runner.py Tests ✅ Permanent test suite
tests/skills/test_dev_orchestrator_issue_3024.py Tests ✅ Regression test (issue-referenced tests are a durable convention)
tests/test_rust_hook_engine.py Tests ✅ Permanent test suite
tests/test_rust_trial.py Tests ✅ Permanent test suite for rust_trial.py module
tests/test_wheel_packaging.py Tests ✅ Permanent test suite
uv.lock Lock file ✅ Dependency lock file

Generated by Repo Guardian for issue #3026 ·

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.

recipe-runner does not stream nested agent progress or sub-recipe failure details

1 participant