feat(workers-dev): introducing dev tui for easier dev experience - #311
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (8)
📝 WalkthroughWalkthroughIntroduces a complete Changesworkers-dev crate
Sequence Diagram(s)sequenceDiagram
participant User
participant main as main / CLI
participant Config
participant Orchestrator
participant WorkerGraph
participant CargoProcess as cargo run (worker)
participant EngineIII as iii engine
User->>main: workers-dev up (or subcommand)
main->>Config: Config::load (YAML + flags + env)
Config-->>main: Config { worker_specs, engine_url, ... }
main->>Orchestrator: Orchestrator::new(config)
Orchestrator->>WorkerGraph: WorkerGraph::load(repo_root, workers)
WorkerGraph-->>Orchestrator: adjacency list + topo order
main->>Orchestrator: start_harness_stack() then start_all_managed()
loop each worker in topo order
Orchestrator->>CargoProcess: spawn cargo run --manifest-path ...
Orchestrator-->>Orchestrator: read_stream (stdout/stderr → RingBuffer + broadcast)
Orchestrator->>EngineIII: fetch_engine_workers (poll until connected)
EngineIII-->>Orchestrator: worker status: connected
end
main->>main: tui::run(orchestrator) OR commands::run_*(orchestrator)
loop TUI poll_interval_ms
main->>Orchestrator: worker_views()
Orchestrator->>EngineIII: fetch_engine_workers
EngineIII-->>Orchestrator: statuses
Orchestrator-->>main: Vec<WorkerView>
User->>main: keypress (s/S/r/l/q)
main->>Orchestrator: start_workers / stop_workers / restart_worker
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
skill-check — worker0 verified, 25 skipped (no docs/).
Four for four. Nicely done. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (1)
workers-dev/PLAN.md (1)
9-12: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAdd language identifiers to fenced code blocks to satisfy markdown linting.
The unnamed fenced blocks flagged at Line 9, Line 58, Line 98, Line 174, and Line 198 should use a language tag (for example,
text,bash, orrust) to avoid MD040 warnings.Also applies to: 58-76, 98-103, 174-183, 198-223
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workers-dev/PLAN.md` around lines 9 - 12, The PLAN.md file contains fenced code blocks without language identifiers at lines 9, 58, 98, 174, and 198, which violates markdown linting rules (MD040). To fix this, add an appropriate language identifier to each unnamed fenced code block by specifying a language tag after the opening triple backticks (such as text, bash, rust, etc. depending on the content). For example, change triple backticks followed by a newline to triple backticks followed by the language name, then the content. Apply this fix to all five flagged locations to satisfy the markdown linter.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@workers-dev/PLAN.md`:
- Around line 19-27: The plan document inconsistently uses both `harness-dev`
and `workers-dev` as binary names, creating ambiguity with the actual crate and
README examples. Replace all occurrences of `harness-dev` with `workers-dev`
throughout the entire document, paying special attention to the sections
starting at line 19, lines 59-79, and lines 157-176 where this inconsistency is
most prevalent, to establish a single consistent binary name throughout the
plan.
In `@workers-dev/src/commands/mod.rs`:
- Around line 76-83: The error handler for RecvError::Closed in the worker log
subscription loop only exits when the worker status is "stopped", but does not
account for other terminal states like "crashed" or absent workers, which causes
infinite re-subscribe loops. Modify the condition in the any() iterator check to
also break on "crashed" status or when the worker is not found in the proc list,
ensuring that any terminal state of the worker causes the loop to exit instead
of retrying indefinitely.
In `@workers-dev/src/config.rs`:
- Around line 175-188: The parse_engine_url function fails when parsing
WebSocket URLs with paths or trailing slashes because split_once(':') captures
port_str as "49134/" instead of just "49134". After stripping the protocol in
the stripped variable, split on '/' first to isolate just the host:port portion
before then splitting on ':' to extract the port number, ensuring port_str only
contains the numeric port value when calling .parse() to convert it to u16.
- Around line 97-107: The precedence for the `release` field (line 97) and
`stop_on_exit` field (line 107) is reversed—they currently prioritize the file
configuration over CLI flags. Swap the precedence order so that CLI flag values
take priority over the file configuration values. For both fields, the CLI value
should be checked first, and only if it's not explicitly set should the file
configuration value be used as a fallback.
In `@workers-dev/src/graph.rs`:
- Around line 67-113: The topo_start_order method needs to validate and
deduplicate the subset parameter before building the topological graph.
Currently, the HashMap-based approach implicitly deduplicates entries, but the
final length comparison against subset.len() can produce false "dependency
cycle" errors when duplicates exist in the input, and unknown worker names are
silently accepted as valid nodes. Add validation at the start of the method to
ensure all names in subset correspond to valid workers in the graph, then
deduplicate the subset and use the deduplicated collection for the remaining
algorithm and the final length check to ensure correctness.
In `@workers-dev/src/logs.rs`:
- Around line 85-99: The code unconditionally adds a space separator after the
timestamp (the Span::raw(" ") on line 92), which causes a visual overflow when
the timestamp already consumes the full max_width due to truncation. Modify the
logic to only add the separator space when the timestamp does not consume the
full width. Check if the character count of the timestamp (calculated as
ts.chars().count().min(max_width)) is less than max_width before pushing the
Span::raw(" ") to the spans vector.
In `@workers-dev/src/main.rs`:
- Around line 128-130: The orchestrator.stop_workers call within the
stop_on_exit conditional block is currently using let _ to silently ignore any
errors that occur during shutdown. Instead of discarding the result with let _,
handle the error returned from orchestrator.stop_workers by either logging it or
printing it to the user so that shutdown failures are visible rather than
silently dropped. This ensures users are aware if workers fail to stop properly.
In `@workers-dev/src/orchestrator.rs`:
- Around line 155-159: The wait_for_exit task spawned in the tokio::spawn call
is bound only to the worker name (name_wait), not to a specific instance,
causing old waiters from previous restarts to attach to and monitor newer child
processes. This creates accumulating concurrent watchers for the same worker. To
fix this, add an instance-specific identifier (such as a generation ID, restart
counter, or unique instance handle) that gets passed to the wait_for_exit
function alongside name_wait and runtimes_wait. This ensures each waiter is tied
to its specific process instance and prevents old waiters from attaching to new
processes after a restart. Apply the same fix to the similar code at lines
366-409.
In `@workers-dev/src/status.rs`:
- Around line 54-76: The current implementation calls child.wait() before
reading from stdout and stderr, which creates a deadlock risk if the child
process output exceeds pipe buffer capacity—the process will block trying to
write while the parent waits for it to exit. Replace the manual spawn, wait, and
read pattern with Command::output() which safely handles reading pipes while the
process runs, or add a timeout to the wait operation to prevent indefinite hangs
in the polling loop that expects responses every ~500ms.
In `@workers-dev/src/tui/mod.rs`:
- Around line 183-190: The KeyCode::Char('l') handler blocks the entire
dashboard event loop by awaiting crate::commands::run_logs inline, preventing
input and refresh handling during log tailing. Move the log execution to run
asynchronously without blocking the event loop (consider spawning it as a
background task or using a non-blocking approach), and update or remove the
misleading "Ctrl+C to return" prompt text since the TUI is currently
unresponsive during log execution. This will allow the dashboard to remain
responsive while logs run.
- Around line 45-127: The run function has early return paths from error
propagation (? operators) within the while loop that bypass the terminal
restoration code at the end. Restructure the run function to guarantee that
disable_raw_mode() and the LeaveAlternateScreen execute are always called, even
when errors occur. Consider wrapping the main event loop logic in a separate
function or using a guard pattern that automatically restores terminal state
when exiting scope, ensuring cleanup happens regardless of which ? operator
returns an error.
- Around line 497-502: The overlay block unconditionally applies the
overlay_bg_style() regardless of whether colors are disabled. Modify the
.style(overlay_bg_style()) call to conditionally apply the overlay background
style only when colors are enabled. Check the appropriate color setting or flag
that respects the --color never option, and pass either overlay_bg_style() or a
neutral/default style based on that condition to the .style() method call.
---
Nitpick comments:
In `@workers-dev/PLAN.md`:
- Around line 9-12: The PLAN.md file contains fenced code blocks without
language identifiers at lines 9, 58, 98, 174, and 198, which violates markdown
linting rules (MD040). To fix this, add an appropriate language identifier to
each unnamed fenced code block by specifying a language tag after the opening
triple backticks (such as text, bash, rust, etc. depending on the content). For
example, change triple backticks followed by a newline to triple backticks
followed by the language name, then the content. Apply this fix to all five
flagged locations to satisfy the markdown linter.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5a51494e-bad7-4453-b55c-e0355ca01b3a
⛔ Files ignored due to path filters (1)
workers-dev/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
workers-dev/Cargo.tomlworkers-dev/PLAN.mdworkers-dev/README.mdworkers-dev/src/color.rsworkers-dev/src/commands/mod.rsworkers-dev/src/config.rsworkers-dev/src/discover.rsworkers-dev/src/graph.rsworkers-dev/src/logs.rsworkers-dev/src/main.rsworkers-dev/src/orchestrator.rsworkers-dev/src/runtime.rsworkers-dev/src/status.rsworkers-dev/src/tui/mod.rsworkers-dev/src/tui/theme.rs
Comment on lines
+183
to
+190
| KeyCode::Char('l') => { | ||
| if let Some(name) = worker_name { | ||
| disable_raw_mode()?; | ||
| execute!(io::stdout(), LeaveAlternateScreen)?; | ||
| println!("Following logs for {name} (Ctrl+C to return)…\n"); | ||
| let _ = crate::commands::run_logs(orchestrator.clone(), name, true, 20).await; | ||
| enable_raw_mode()?; | ||
| execute!(io::stdout(), EnterAlternateScreen)?; |
There was a problem hiding this comment.
l blocks the dashboard event loop until logs end.
Line 188 awaits follow mode inline, so the TUI stops handling input/refresh while log tailing runs. The prompt text (“Ctrl+C to return”) is also misleading in this flow.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workers-dev/src/tui/mod.rs` around lines 183 - 190, The KeyCode::Char('l')
handler blocks the entire dashboard event loop by awaiting
crate::commands::run_logs inline, preventing input and refresh handling during
log tailing. Move the log execution to run asynchronously without blocking the
event loop (consider spawning it as a background task or using a non-blocking
approach), and update or remove the misleading "Ctrl+C to return" prompt text
since the TUI is currently unresponsive during log execution. This will allow
the dashboard to remain responsive while logs run.
ytallo
approved these changes
Jun 22, 2026
This was referenced Jul 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Introducing a Dev TUI to be used to improve quality of life while developing workers on this repository
Summary by CodeRabbit
Release Notes
workers-dev, a new local development tool for managing worker processes.status(view worker status),logs(stream/follow logs),start/stop/restart(manage workers).