Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 Walkthrough🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
cli/src/main.rs (2)
121-121: Unnecessary clone.
runis not used after this line, so you can moverun.iddirectly instead of cloning.♻️ Proposed fix
- let job_id = run.id.clone(); + let job_id = run.id;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/src/main.rs` at line 121, Replace the unnecessary clone by moving the id out of run: change the assignment of job_id so it takes ownership of run.id (use let job_id = run.id;) instead of cloning, and ensure no further uses of run remain after this line so the move is valid.
152-156: Consider printing stderr content to stderr.Job stderr output is printed to stdout, which mixes output streams. Using
eprintln!would preserve the distinction and allow users to separate stdout from stderr when piping or redirecting CLI output.♻️ Proposed fix
if let Some(ref err) = st.stderr { if !err.is_empty() { - println!("{err}"); + eprintln!("{err}"); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/src/main.rs` around lines 152 - 156, The snippet printing job stderr currently uses println!("{err}") which writes to stdout; change it to write to stderr by using eprintln! so stderr remains separate — locate the block that checks st.stderr (the if let Some(ref err) = st.stderr { if !err.is_empty() { println!("{err}"); } }) and replace the println call with eprintln!("{err}") to preserve proper output streams.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cli/src/main.rs`:
- Around line 127-138: The JSON deserialization chain on the status response
(status_resp.json().await.unwrap_or(...)) is misformatted and failing cargo fmt;
run rustfmt by executing `cd cli && cargo fmt` (or `cargo fmt -- --check` to
verify) to auto-fix the formatting, or manually reformat the chain so the call
to status_resp.json().await.unwrap_or(...) is properly aligned/indented; look
for the symbols status_resp, ErrorBody, client.get and status_url to locate the
block and ensure the return Err(...) line remains unchanged after formatting.
- Around line 123-159: The polling loop in main (the section building status_url
and awaiting status_resp) has no timeout/retry limit; modify it to stop after a
configurable duration or maximum attempts (e.g., add a max_retries or
timeout_secs CLI/config option) and return an error if exceeded. Implement this
by wrapping the polling in a tokio::time::timeout or by counting iterations in
the loop (use a counter like attempts and compare to max_retries), aborting with
a clear error message instead of blocking; update the code that uses
client.get(&status_url).send().await and st: StatusResponse parsing to respect
the timeout/retry logic so any pending json().await calls are not left hanging.
---
Nitpick comments:
In `@cli/src/main.rs`:
- Line 121: Replace the unnecessary clone by moving the id out of run: change
the assignment of job_id so it takes ownership of run.id (use let job_id =
run.id;) instead of cloning, and ensure no further uses of run remain after this
line so the move is valid.
- Around line 152-156: The snippet printing job stderr currently uses
println!("{err}") which writes to stdout; change it to write to stderr by using
eprintln! so stderr remains separate — locate the block that checks st.stderr
(the if let Some(ref err) = st.stderr { if !err.is_empty() { println!("{err}");
} }) and replace the println call with eprintln!("{err}") to preserve proper
output streams.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a88bc6a4-1332-4236-a03c-966e1bdb41be
📒 Files selected for processing (2)
backend/.gitignorecli/src/main.rs
Comment on lines
+123
to
+159
| loop { | ||
| tokio::time::sleep(Duration::from_secs(1)).await; | ||
|
|
||
| let status_url = format!("{backend}/status/{job_id}"); | ||
| let status_resp = client.get(&status_url).send().await?; | ||
|
|
||
| if !status_resp.status().is_success() { | ||
| let status = status_resp.status(); | ||
| let err: ErrorBody = status_resp | ||
| .json() | ||
| .await | ||
| .unwrap_or(ErrorBody { | ||
| error: format!("HTTP {status}"), | ||
| }); | ||
| return Err(format!("Backend error (HTTP {status}): {}", err.error).into()); | ||
| } | ||
|
|
||
| let st: StatusResponse = status_resp.json().await?; | ||
|
|
||
| if st.status == "done" || st.status == "error" { | ||
| println!("Status: {}", st.status); | ||
| if let Some(code) = st.exit_code { | ||
| println!("Exit code: {code}"); | ||
| } | ||
| if let Some(ref out) = st.stdout { | ||
| if !out.is_empty() { | ||
| println!("{out}"); | ||
| } | ||
| } | ||
| if let Some(ref err) = st.stderr { | ||
| if !err.is_empty() { | ||
| println!("{err}"); | ||
| } | ||
| } | ||
| return Ok(()); | ||
| } | ||
| } |
There was a problem hiding this comment.
Add a timeout or maximum retry limit to prevent indefinite blocking.
The polling loop has no upper bound. If the backend becomes unresponsive, hangs, or a job gets stuck in a non-terminal state, the CLI will block indefinitely. Consider adding a configurable timeout or maximum number of retries.
🛡️ Proposed fix to add a timeout
+ const MAX_POLL_DURATION: Duration = Duration::from_secs(300); // 5 minutes
+ let poll_start = std::time::Instant::now();
+
loop {
tokio::time::sleep(Duration::from_secs(1)).await;
+ if poll_start.elapsed() > MAX_POLL_DURATION {
+ return Err("Timeout waiting for job to complete".into());
+ }
+
let status_url = format!("{backend}/status/{job_id}");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| loop { | |
| tokio::time::sleep(Duration::from_secs(1)).await; | |
| let status_url = format!("{backend}/status/{job_id}"); | |
| let status_resp = client.get(&status_url).send().await?; | |
| if !status_resp.status().is_success() { | |
| let status = status_resp.status(); | |
| let err: ErrorBody = status_resp | |
| .json() | |
| .await | |
| .unwrap_or(ErrorBody { | |
| error: format!("HTTP {status}"), | |
| }); | |
| return Err(format!("Backend error (HTTP {status}): {}", err.error).into()); | |
| } | |
| let st: StatusResponse = status_resp.json().await?; | |
| if st.status == "done" || st.status == "error" { | |
| println!("Status: {}", st.status); | |
| if let Some(code) = st.exit_code { | |
| println!("Exit code: {code}"); | |
| } | |
| if let Some(ref out) = st.stdout { | |
| if !out.is_empty() { | |
| println!("{out}"); | |
| } | |
| } | |
| if let Some(ref err) = st.stderr { | |
| if !err.is_empty() { | |
| println!("{err}"); | |
| } | |
| } | |
| return Ok(()); | |
| } | |
| } | |
| const MAX_POLL_DURATION: Duration = Duration::from_secs(300); // 5 minutes | |
| let poll_start = std::time::Instant::now(); | |
| loop { | |
| tokio::time::sleep(Duration::from_secs(1)).await; | |
| if poll_start.elapsed() > MAX_POLL_DURATION { | |
| return Err("Timeout waiting for job to complete".into()); | |
| } | |
| let status_url = format!("{backend}/status/{job_id}"); | |
| let status_resp = client.get(&status_url).send().await?; | |
| if !status_resp.status().is_success() { | |
| let status = status_resp.status(); | |
| let err: ErrorBody = status_resp | |
| .json() | |
| .await | |
| .unwrap_or(ErrorBody { | |
| error: format!("HTTP {status}"), | |
| }); | |
| return Err(format!("Backend error (HTTP {status}): {}", err.error).into()); | |
| } | |
| let st: StatusResponse = status_resp.json().await?; | |
| if st.status == "done" || st.status == "error" { | |
| println!("Status: {}", st.status); | |
| if let Some(code) = st.exit_code { | |
| println!("Exit code: {code}"); | |
| } | |
| if let Some(ref out) = st.stdout { | |
| if !out.is_empty() { | |
| println!("{out}"); | |
| } | |
| } | |
| if let Some(ref err) = st.stderr { | |
| if !err.is_empty() { | |
| println!("{err}"); | |
| } | |
| } | |
| return Ok(()); | |
| } | |
| } |
🧰 Tools
🪛 GitHub Actions: Check source code
[error] 128-133: cargo fmt --check failed: code formatting issue detected in cli/src/main.rs (lines 128-133). Ensure correct formatting of the JSON deserialization and awaiting expressions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cli/src/main.rs` around lines 123 - 159, The polling loop in main (the
section building status_url and awaiting status_resp) has no timeout/retry
limit; modify it to stop after a configurable duration or maximum attempts
(e.g., add a max_retries or timeout_secs CLI/config option) and return an error
if exceeded. Implement this by wrapping the polling in a tokio::time::timeout or
by counting iterations in the loop (use a counter like attempts and compare to
max_retries), aborting with a clear error message instead of blocking; update
the code that uses client.get(&status_url).send().await and st: StatusResponse
parsing to respect the timeout/retry logic so any pending json().await calls are
not left hanging.
Proxyfil
approved these changes
Mar 16, 2026
lsoulet
force-pushed
the
feat/cli-backend-link
branch
from
adc5aac to
578eaaf
Compare
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.
To test it, build and run backend following README. then run these commands :
Run command with Python :
cargo run -p cli -- go --language python --file agent/examples/hello.pyRun command with NodeJS :
cargo run -p cli -- go --language node --file agent/examples/hello.jsFor status command, type a run command, check the backend logs and copy the job ID.
Then, type this command :
cargo run -p cli -- status <JOB_ID>Summary by CodeRabbit
New Features
Bug Fixes
Chores
Style