Skip to content

feat(tools): add node-runner TUI for Nitro + Nethermind orchestration#769

Open
AnkushinDaniil wants to merge 3 commits intomainfrom
daniil/tools/node-runner
Open

feat(tools): add node-runner TUI for Nitro + Nethermind orchestration#769
AnkushinDaniil wants to merge 3 commits intomainfrom
daniil/tools/node-runner

Conversation

@AnkushinDaniil
Copy link
Copy Markdown
Collaborator

@AnkushinDaniil AnkushinDaniil commented Mar 22, 2026

Summary

  • Adds tools/node-runner/, a Python TUI tool (Textual) for running Arbitrum Nitro and Nethermind nodes in multiple modes (comparison, standalone, init-only)
  • Manages process lifecycle, health checks, crash reports, and live log streaming
  • Adds NITRO_PATH / NETHERMIND_PATH env vars to .env.example for node-runner configuration

Python TUI tool (Textual) for running Arbitrum Nitro and Nethermind
nodes in multiple modes (comparison, standalone, init-only). Manages
process lifecycle, health checks, crash reports, and log streaming.

Includes:
- Process orchestration for Nitro EL/CL and Nethermind
- Textual-based TUI with live log panels and status widgets
- CLI with mode selection and configuration via .env
- Unit tests for config, models, and orchestrator
Copilot AI review requested due to automatic review settings March 22, 2026 13:41
@AnkushinDaniil AnkushinDaniil marked this pull request as draft March 22, 2026 13:42
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new tools/node-runner Python (Textual) TUI utility to orchestrate Arbitrum Nitro + Nethermind in multiple modes, including init sequencing, health checks, log streaming, and shutdown reporting, plus supporting configuration/tests.

Changes:

  • Introduces the node-runner package (CLI entrypoint, config/models/exceptions, orchestrator, process wrappers, crash/error reporting).
  • Adds a Textual TUI (split/combined log panes, keybindings, clipboard copy, live status updates).
  • Adds pytest coverage for config/models/orchestrator behavior and updates .env.example with NITRO_PATH / NETHERMIND_PATH.

Reviewed changes

Copilot reviewed 24 out of 25 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
tools/node-runner/tests/test_orchestrator.py Tests for process graph construction, init detection, shutdown flagging, and verification flag behavior.
tools/node-runner/tests/test_models.py Tests for log/model semantics (ring buffers, counts, enums).
tools/node-runner/tests/test_config.py Tests for configuration defaults, frozen models, derived paths, and flags.
tools/node-runner/tests/init.py Marks tests as a package (empty).
tools/node-runner/pyproject.toml Declares the node-runner project, deps, scripts, and tool configs (pytest/mypy/ruff).
tools/node-runner/node_runner/tui/widgets.py Implements per-process and combined log panes plus status header rendering.
tools/node-runner/node_runner/tui/styles.tcss Defines TUI layout and styling (status bar, panes, headers, log view).
tools/node-runner/node_runner/tui/app.py Main Textual app: starts orchestrator, streams logs, monitors status, handles actions (stop/filter/copy).
tools/node-runner/node_runner/tui/init.py TUI package marker.
tools/node-runner/node_runner/processes/nitro_el.py Nitro EL process wrapper and command builder.
tools/node-runner/node_runner/processes/nitro_cl.py Nitro CL process wrapper with mode-specific EL connection flags.
tools/node-runner/node_runner/processes/nethermind.py Nethermind process wrapper and command builder (incl. VerifyBlockHash flags).
tools/node-runner/node_runner/processes/init_el.py One-shot Nitro init-el process with timeout/error context.
tools/node-runner/node_runner/processes/base.py Base async subprocess lifecycle, log parsing, health checks, and shutdown escalation.
tools/node-runner/node_runner/processes/init.py Processes package marker.
tools/node-runner/node_runner/orchestrator.py Startup sequencing (incl. optional init), health gating, stop ordering, and monitoring helpers.
tools/node-runner/node_runner/models.py Dataclasses/enums for process state and log entries (ring buffers, counts).
tools/node-runner/node_runner/exceptions.py Custom exception hierarchy for config/startup/health/init/shutdown errors.
tools/node-runner/node_runner/crash_report.py Writes shutdown/crash reports and error-only log outputs.
tools/node-runner/node_runner/config.py Defines modes/networks/ports/constants and frozen RunnerConfig model.
tools/node-runner/node_runner/cli.py Argparse CLI, env var resolution, validation, and RunnerConfig construction.
tools/node-runner/node_runner/main.py Module entrypoint to parse args and run the TUI.
tools/node-runner/node_runner/init.py Package marker with version.
tools/node-runner/.gitignore Ignores local tool data directories.
.env.example Documents NITRO_PATH / NETHERMIND_PATH for node-runner configuration.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

name = "node-runner"
version = "0.1.0"
description = "TUI tool for running Arbitrum Nitro + Nethermind nodes in multiple modes"
readme = "README.md"
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

readme = "README.md" points to a file that doesn’t exist in tools/node-runner/ (only the repo root has a README). Building a wheel/sdist for this tool will fail. Add a tools/node-runner/README.md (recommended) or update the readme field to reference an existing file.

Suggested change
readme = "README.md"
readme = "../README.md"

Copilot uses AI. Check for mistakes.
Comment on lines +116 to +138
def write_error_logs(
processes: Sequence[BaseProcess],
error_dir: Path,
) -> None:
"""Write persistent error-only log files (appended during runtime).

These are small files containing only ERROR/WARN lines, suitable
for long-running sessions where full logs would be too large.

Args:
processes: List of managed processes
error_dir: Directory for error log files
"""
error_dir.mkdir(parents=True, exist_ok=True)

for proc in processes:
if not proc.state.error_log:
continue

path = error_dir / f"{proc.name}.errors.log"
try:
with path.open("w", encoding="utf-8") as f:
for entry in proc.state.error_log:
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The write_error_logs docstring says the files are “appended during runtime”, but the implementation opens each file with mode "w" (overwrites). Either update the docstring to match the current shutdown-time overwrite behavior, or change the implementation to append if that’s the intended operational model.

Copilot uses AI. Check for mistakes.
Comment on lines +141 to +142
deadline = asyncio.get_event_loop().time() + timeout
while asyncio.get_event_loop().time() < deadline:
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wait_for_healthy() uses asyncio.get_event_loop().time() from inside a coroutine. On Python 3.12+ this API is discouraged/deprecated in favor of asyncio.get_running_loop(). Using the running loop avoids deprecation warnings and is more robust under different event loop policies.

Suggested change
deadline = asyncio.get_event_loop().time() + timeout
while asyncio.get_event_loop().time() < deadline:
loop = asyncio.get_running_loop()
deadline = loop.time() + timeout
while loop.time() < deadline:

Copilot uses AI. Check for mistakes.
Comment on lines +102 to +121
try:
os.killpg(self._proc.pid, signal.SIGTERM)
except ProcessLookupError:
self.state.status = ProcessStatus.STOPPED
self._cancel_log_task()
return True

try:
await asyncio.wait_for(self._proc.wait(), timeout=grace_s)
self.state.status = ProcessStatus.STOPPED
self.state.exit_code = self._proc.returncode
self._cancel_log_task()
return True
except TimeoutError:
pass

# Escalate to SIGKILL
try:
os.killpg(self._proc.pid, signal.SIGKILL)
await asyncio.wait_for(self._proc.wait(), timeout=5.0)
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Process shutdown relies on POSIX-only primitives (os.killpg, signal.SIGTERM/SIGKILL). On Windows this will raise or behave differently, which can make the TUI unable to stop subprocesses. If Windows support is not intended, it would help to fail fast with a clear error (or document the platform requirement); otherwise consider using a cross-platform termination strategy (e.g., proc.terminate()/proc.kill() with creationflags on Windows).

Copilot uses AI. Check for mistakes.
Comment on lines +149 to +166
while True:
for proc in self._orchestrator.processes:
buf = proc.state.log_buffer
pointer = self._log_pointers.get(proc.name, 0)
buf_list = list(buf)
new_entries = buf_list[pointer:]

if new_entries:
pane = self._panes.get(proc.name)
if pane is None:
pane = self._panes.get(_PANE_FALLBACK.get(proc.name, ""))
for entry in new_entries:
if pane is not None:
pane.write_entry(entry)
if self._combined_pane is not None:
self._combined_pane.write_entry(entry)

self._log_pointers[proc.name] = len(buf_list)
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The log streaming pointer is based on len(list(proc.state.log_buffer)). Since log_buffer is a fixed-size ring buffer (maxlen), len(...) stops increasing once full, so new_entries = buf_list[pointer:] becomes permanently empty and the TUI stops showing new logs after the buffer fills. Consider tracking an ever-increasing sequence number per log entry (or a total-lines-written counter in ProcessState) and stream based on that, rather than list length / index into the current snapshot.

Copilot uses AI. Check for mistakes.
if pane is not None:
pane.write_entry(entry)
if self._combined_pane is not None:
self._combined_pane.write_entry(entry)
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

action_toggle_filter() clears the visible logs and then replays the entire in-memory buffers, but it doesn’t update/reset _log_pointers. The background _stream_logs() loop will therefore re-emit already-replayed entries (duplicates) on the next poll. After replaying, update each process’s pointer to the current buffer position (or reset the stream state consistently with the new filtering behavior).

Suggested change
self._combined_pane.write_entry(entry)
self._combined_pane.write_entry(entry)
# After replaying the buffer, advance the log pointer so the
# background streaming loop does not re-emit these entries.
if hasattr(self, "_log_pointers"):
self._log_pointers[proc.name] = len(proc.state.log_buffer)

Copilot uses AI. Check for mistakes.
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 22, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.86%. Comparing base (f19796a) to head (a914db1).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #769      +/-   ##
==========================================
- Coverage   77.24%   76.86%   -0.39%     
==========================================
  Files         185      217      +32     
  Lines       11889    13222    +1333     
  Branches     1615     1886     +271     
==========================================
+ Hits         9184    10163     +979     
- Misses       2113     2382     +269     
- Partials      592      677      +85     

see 50 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@AnkushinDaniil AnkushinDaniil marked this pull request as ready for review March 22, 2026 20:21
Copy link
Copy Markdown
Collaborator

@wurdum wurdum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tools need a comprehensive readme and demo on Thursday 😉

- Add comprehensive README.md for node-runner
- Fix crash_report.py docstring (said "appended" but uses overwrite)
- Use asyncio.get_running_loop() instead of deprecated get_event_loop()
- Fix log streaming bug: ring buffer pointer stopped working once full
  by tracking a monotonic write_count instead of buffer length
- Fix toggle_filter not resetting log pointers after replay
Load Nitro's .env file after resolving nitro_path so that
SEPOLIA_L1_RPC, MAINNET_L1_BEACON, etc. are picked up automatically.
Priority: --l1-rpc flag > Nitro .env > hardcoded default.
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.

4 participants