Run Ralph in place inside FeatureListPanel; harden run lifecycle

[shuveb]

Delete the standalone ralph_panel.py (~755 lines) and merge Ralph orchestration into the unified FeatureListPanel, which now doubles as the in-place run view. Add RalphConfig.module_ids so the TUI can scope a run to a single module without affecting the console subcommand.

Robustness fixes from the PR review:

• Wrap the _run_orchestrator and _run_single_retry worker bodies so a mid-run network/API failure surfaces the auth modal or a notification and returns to browsing instead of crashing the whole TUI and orphaning the agent subprocess.
• Treat an empty module_ids tuple as "no restriction" (restricts_modules property); warn via the display when the filter matches nothing.
• Narrow the resolve_phases fallback to typer.Exit so genuine API errors propagate to the user instead of a silent orphan-only list.
• Restore a targeted single-module feature fetch for module-scope browsing instead of building the whole project list and discarding it.
• kill_agent only reports success when terminate() did not raise; add debug breadcrumbs to best-effort teardown swallows; marshal the status-count update onto the main thread.
• Drop the redundant _ralph_panel attribute (use _current_feature_panel).
• Fix CLAUDE.md / README.md rot referencing the deleted panel.

Tests cover the module_ids construction seam, retry success/failure, return-to-browsing teardown, and orchestrator empty/non-matching module scoping. Full unit suite: 766 passed.

[greptile-apps]

Greptile Summary

This PR merges the standalone ralph_panel.py (~755 lines) into FeatureListPanel, which now serves as both the normal browse view and an in-place Ralph orchestration view (BROWSING → RUNNING → PAUSED). It also adds RalphConfig.module_ids so the TUI can scope a run to a single module, hardens the run lifecycle with auth-error surfacing and teardown breadcrumbs, and narrows the resolve_phases fallback to typer.Exit.

• FeatureListPanel gains the full Ralph orchestration lifecycle (three-stage state machine, timers, retry, pause, kill) in place of the deleted panel, with scope=\"module\" vs scope=\"global\" deciding whether a targeted single-module API call or the full project feature list is fetched.
• RalphConfig.module_ids (tuple or None) + restricts_modules property let the orchestrator filter features post-fetch; empty tuple is treated as no-restriction, with a user-visible warning when the filter matches nothing.
• AuthRequiredModal's "Cancel" button was relabelled "Quit" but the underlying handler still only dismisses the modal without quitting the app, leaving users stranded in an unauthenticated TUI.

Confidence Score: 3/5

Safe to merge after fixing the auth modal Quit label — it dismisses without quitting, leaving users in a broken unauthenticated TUI state.

The auth modal rename from Cancel to Quit introduces a concrete wrong behavior: clicking Quit dismisses the modal and returns to a TUI that has no valid session, rather than exiting the application. Every subsequent action re-triggers the same modal, effectively trapping the user. Additionally, _return_to_browsing is not idempotent — a concurrent worker teardown can call it twice, spawning two parallel feature-reload workers on paths users are likely to hit.

src/mfbt/tui/screens/auth_modal.py for the Quit label and src/mfbt/tui/screens/feature_list.py for the _return_to_browsing idempotency and private orchestrator access in _run_single_retry.

Important Files Changed

Filename	Overview
src/mfbt/tui/screens/feature_list.py	Core of the PR: replaces the old per-module feature list and standalone ralph_panel.py with a unified panel. Adds Ralph orchestration lifecycle, timer animation, retry flow, and in-place run view. Two logic concerns: potential double-invocation of _return_to_browsing on concurrent teardown, and _run_single_retry coupling to private orchestrator internals.
src/mfbt/tui/screens/auth_modal.py	Renames Cancel to Quit but the handler still only dismisses the modal; misleading label leaves users stranded in an unauthenticated TUI.
src/mfbt/tui/app.py	Removes _ralph_active/_ralph_panel in favour of _current_feature_panel() lookup and _ralph_return flag. Adds multi-modal guard in show_auth_required_modal. Logic looks sound.
src/mfbt/commands/ralph/orchestrator.py	Adds module_ids filtering after build_global_feature_list; empty tuple treated as no-restriction via restricts_modules. Warns on no-match. Well-tested.
src/mfbt/commands/ralph/types.py	Adds module_ids field and restricts_modules property to RalphConfig. Empty-tuple sentinel is well-documented and tested.
src/mfbt/commands/ralph/progress.py	Extracts get_module_feature_list from the inline loop in get_orphan_module_features. Delegates cleanly.
src/mfbt/commands/ralph/tui_display.py	session_summary correctly transitions stage to PAUSED via call_from_thread. feature_complete sets _running_feature_key and _feature_start directly from the worker thread; safe in CPython but technically a data race.
tests/unit/tui/test_ralph_panel.py	Rewrites panel tests for the merged lifecycle. Covers module_ids construction seam, retry success/failure, return-to-browsing teardown, and orchestrator scoping.
tests/unit/ralph/test_orchestrator.py	Adds TestOrchestratorModuleScope with four tests covering module filter, None/empty-tuple no-restriction, and non-matching warning path.
src/mfbt/tui/screens/ralph_panel.py	Deleted (~755 lines). All functionality merged into FeatureListPanel.

Comments Outside Diff (3)

1. src/mfbt/tui/screens/auth_modal.py, line 37-43 (link)

   "Quit" button does not quit the app

   The button was renamed from "Cancel" to "Quit" with variant="error", but the on_button_pressed handler still calls self.dismiss(False), and show_auth_required_modal's _on_result callback does nothing when authenticate is False. A user who clicks "Quit" expecting the app to exit is instead returned to a TUI that has no valid auth session — every subsequent action triggers the same modal again. The label should either revert to "Cancel" (or "Dismiss"), or the callback in app.py's show_auth_required_modal must call self.action_quit() when the result is False.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: src/mfbt/tui/screens/auth_modal.py
   Line: 37-43
   
   Comment:
   **"Quit" button does not quit the app**
   
   The button was renamed from "Cancel" to "Quit" with `variant="error"`, but the `on_button_pressed` handler still calls `self.dismiss(False)`, and `show_auth_required_modal`'s `_on_result` callback does nothing when `authenticate is False`. A user who clicks "Quit" expecting the app to exit is instead returned to a TUI that has no valid auth session — every subsequent action triggers the same modal again. The label should either revert to "Cancel" (or "Dismiss"), or the callback in `app.py`'s `show_auth_required_modal` must call `self.action_quit()` when the result is `False`.
   
   How can I resolve this? If you propose a fix, please make it concise.

   

2. src/mfbt/tui/screens/feature_list.py, line 1148-1167 (link)

   Direct access to private orchestrator internals

   _run_single_retry reaches into self._orchestrator._stop_event.clear(), self._orchestrator._implement_feature(feature, 0, 0), and self._orchestrator._results.append(result) — all private members of RalphOrchestrator. If those internals are ever renamed or the signature of _implement_feature changes, the retry path will break with no compile-time or type-checker indication. The orchestrator should expose a public retry_feature(feature) method that encapsulates the stop-event reset, the single-feature run, and the results append.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: src/mfbt/tui/screens/feature_list.py
   Line: 1148-1167
   
   Comment:
   **Direct access to private orchestrator internals**
   
   `_run_single_retry` reaches into `self._orchestrator._stop_event.clear()`, `self._orchestrator._implement_feature(feature, 0, 0)`, and `self._orchestrator._results.append(result)` — all private members of `RalphOrchestrator`. If those internals are ever renamed or the signature of `_implement_feature` changes, the retry path will break with no compile-time or type-checker indication. The orchestrator should expose a public `retry_feature(feature)` method that encapsulates the stop-event reset, the single-feature run, and the results append.
   
   How can I resolve this? If you propose a fix, please make it concise.

   

3. src/mfbt/tui/screens/feature_list.py, line 977-996 (link)

   Double _return_to_browsing invocation on concurrent teardown

   _return_to_browsing can be called from two code paths at the same time: (1) the main thread via go_back() when the user presses ESC while PAUSED, and (2) the orchestrator worker thread via call_from_thread(self._return_to_browsing) if the worker experiences an exception while winding down. Both paths call run_worker(self._load_features, thread=True) unconditionally, so a concurrent teardown starts two feature-reload workers that can produce duplicate or interleaved table population. Adding a stage guard (e.g., if self._stage != RalphStage.BROWSING: return) at the top of _return_to_browsing would make it idempotent.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: src/mfbt/tui/screens/feature_list.py
   Line: 977-996
   
   Comment:
   **Double `_return_to_browsing` invocation on concurrent teardown**
   
   `_return_to_browsing` can be called from two code paths at the same time: (1) the main thread via `go_back()` when the user presses ESC while PAUSED, and (2) the orchestrator worker thread via `call_from_thread(self._return_to_browsing)` if the worker experiences an exception while winding down. Both paths call `run_worker(self._load_features, thread=True)` unconditionally, so a concurrent teardown starts two feature-reload workers that can produce duplicate or interleaved table population. Adding a stage guard (e.g., `if self._stage != RalphStage.BROWSING: return`) at the top of `_return_to_browsing` would make it idempotent.
   
   How can I resolve this? If you propose a fix, please make it concise.

   

Prompt To Fix All With AI

Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
src/mfbt/tui/screens/auth_modal.py:37-43
**"Quit" button does not quit the app**

The button was renamed from "Cancel" to "Quit" with `variant="error"`, but the `on_button_pressed` handler still calls `self.dismiss(False)`, and `show_auth_required_modal`'s `_on_result` callback does nothing when `authenticate is False`. A user who clicks "Quit" expecting the app to exit is instead returned to a TUI that has no valid auth session — every subsequent action triggers the same modal again. The label should either revert to "Cancel" (or "Dismiss"), or the callback in `app.py`'s `show_auth_required_modal` must call `self.action_quit()` when the result is `False`.

### Issue 2 of 3
src/mfbt/tui/screens/feature_list.py:1148-1167
**Direct access to private orchestrator internals**

`_run_single_retry` reaches into `self._orchestrator._stop_event.clear()`, `self._orchestrator._implement_feature(feature, 0, 0)`, and `self._orchestrator._results.append(result)` — all private members of `RalphOrchestrator`. If those internals are ever renamed or the signature of `_implement_feature` changes, the retry path will break with no compile-time or type-checker indication. The orchestrator should expose a public `retry_feature(feature)` method that encapsulates the stop-event reset, the single-feature run, and the results append.

### Issue 3 of 3
src/mfbt/tui/screens/feature_list.py:977-996
**Double `_return_to_browsing` invocation on concurrent teardown**

`_return_to_browsing` can be called from two code paths at the same time: (1) the main thread via `go_back()` when the user presses ESC while PAUSED, and (2) the orchestrator worker thread via `call_from_thread(self._return_to_browsing)` if the worker experiences an exception while winding down. Both paths call `run_worker(self._load_features, thread=True)` unconditionally, so a concurrent teardown starts two feature-reload workers that can produce duplicate or interleaved table population. Adding a stage guard (e.g., `if self._stage != RalphStage.BROWSING: return`) at the top of `_return_to_browsing` would make it idempotent.

_{Reviews (1): Last reviewed commit: "Run Ralph in place inside FeatureListPan..." | Re-trigger Greptile}