Add pi coding agent tool#362
Merged
Merged
Conversation
|
Warning Review limit reached
More reviews will be available in 42 minutes and 50 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (17)
📝 WalkthroughWalkthroughThis PR introduces a complete coding-agent feature enabling background code execution tasks. The LLM enqueues jobs via ChangesCoding Agent Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
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 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
PR Check ReportSummary
Test Results
Code Quality
Test Artifacts
LinksThis report is auto-generated by GitHub Actions |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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 `@TelegramSearchBot.Common/Env.cs`:
- Around line 220-227: The current deduplication uses
StringComparer.OrdinalIgnoreCase in ResolveCodingAgentDeniedPathPrefixes which
collapses distinct paths on case-sensitive OSes; change the Distinct comparer to
an OS-aware comparer that matches CodingAgentPathPolicy.GetPathComparison() so
deduplication follows the same case-sensitivity rules used later (e.g.,
construct or select a StringComparer based on
CodingAgentPathPolicy.GetPathComparison() and use that in Distinct), ensuring
ResolveCodingAgentDeniedPathPrefixes and
CodingAgentPathPolicy.GetPathComparison() treat path casing consistently.
In `@TelegramSearchBot.LLM/Service/Tools/CodingAgentJobService.cs`:
- Around line 30-55: The current concurrency gate uses separate calls
(SetLengthAsync and SetAddAsync) so two callers can race; replace the separate
check/add/queue sequence with a single Redis-side atomic operation (use a Lua
script via IDatabase.ScriptEvaluate or a MULTI/EXEC transaction) that performs:
check the size of LlmAgentRedisKeys.CodingAgentActiveJobSet (SCARD), if below
Env.CodingAgentMaxConcurrentJobs then SADD the request.JobId into
CodingAgentActiveJobSet, HSET the stateKey (same fields written by
HashSetAsync), set the key TTL (EXPIRE StateTtl), and LPUSH (or RPUSH consistent
with ListLeftPushAsync) the payload into LlmAgentRedisKeys.CodingAgentJobQueue,
returning success/failure; update code around SetLengthAsync, HashSetAsync,
KeyExpireAsync, SetAddAsync, and ListLeftPushAsync to call this script and
handle the failure path (throw InvalidOperationException when script returns
"limit reached").
In `@TelegramSearchBot.LLM/Service/Tools/CodingAgentToolService.cs`:
- Around line 10-11: The CodingAgentToolService class is currently registered
with
[Injectable(Microsoft.Extensions.DependencyInjection.ServiceLifetime.Transient)]
but must follow the repository convention for IService implementations and be
registered as a singleton; update the attribute on the CodingAgentToolService
class (the Injectable attribute applied to class CodingAgentToolService) to use
ServiceLifetime.Singleton so the DI scanner treats this IService implementation
as a singleton.
In `@TelegramSearchBot.RmuxSidecar/src/main.rs`:
- Around line 133-139: The BRPOP call currently uses .await? which will
propagate any Redis error out of main; instead wrap the BRPOP/query_async call
in error handling: if query_async on queue_conn returns an Err, log the error
(including context like "BRPOP failed"), drop/close the existing queue_conn,
attempt to recreate it by calling
client.get_multiplexed_async_connection().await in a small retry/backoff loop,
and then continue the outer loop to resume consuming jobs; keep the successful
path returning Some((String,String)) unchanged so only transient Redis errors
trigger reconnect and not process exit (refer to queue_conn,
client.get_multiplexed_async_connection, BRPOP and JOB_QUEUE).
- Around line 165-310: process_job can return early after setting the job
Running and leave state/report/cleanup incomplete; wrap the main work in a
fallible inner block (e.g. an async closure or helper run_core) that returns the
final_status, error_message, outputs, rmux_session_names and report payload,
then in a guaranteed outer finally/cleanup section (after the inner block
completes or errors) perform the terminal update_state call, push the final
CodingAgentJobReport to REPORT_QUEUE, call cleanup_job, and append the final log
line; ensure you reference and use the existing functions/symbols update_state,
cleanup_job, CodingAgentJobReport, REPORT_QUEUE, append_line and the local
variables started_at/log_path/request.job_id so the finalization always runs
even if the inner block returns Err early.
In `@TelegramSearchBot/Service/AI/LLM/CodingAgentReportConsumer.cs`:
- Around line 55-58: The consumer's loop in CodingAgentReportConsumer is letting
a malformed JSON payload throw and fault the BackgroundService; wrap the
JsonConvert.DeserializeObject<CodingAgentJobReport>(payload) call (and the
second DeserializeObject usage around lines 70-75) in a try/catch that catches
JsonException/Exception, log the payload and error via the existing logger, and
continue the loop instead of allowing the exception to escape; ensure you only
rethrow OperationCanceledException/RedisException as before and keep the rest of
the loop resilient to bad payloads.
- Around line 212-218: The report currently exposes local filesystem details by
appending report.WorkingDirectory and report.LogPath directly in
CodingAgentReportConsumer; remove or redact these fields before sending to
Telegram: stop calling sb.AppendLine for report.WorkingDirectory and
report.LogPath (or replace them with a job-scoped identifier such as a short
hash or the directory basename) and keep non-sensitive fields like
report.RmuxSessionNames unchanged (e.g., use string.Join on
report.RmuxSessionNames as already done). Ensure any redaction logic is applied
where Workspace (report.WorkingDirectory) and Log (report.LogPath) are formatted
so no absolute paths are emitted.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8f97d202-e7d5-4861-830d-14216870d72e
⛔ Files ignored due to path filters (1)
TelegramSearchBot.RmuxSidecar/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
.gitignoreDocs/README_CodingAgentTool.mdDocs/README_MCP.mdTelegramSearchBot.Common/Env.csTelegramSearchBot.Common/Model/AI/LlmAgentContracts.csTelegramSearchBot.LLM/Service/Tools/CodingAgentJobService.csTelegramSearchBot.LLM/Service/Tools/CodingAgentPathPolicy.csTelegramSearchBot.LLM/Service/Tools/CodingAgentToolService.csTelegramSearchBot.RmuxSidecar/Cargo.tomlTelegramSearchBot.RmuxSidecar/README.mdTelegramSearchBot.RmuxSidecar/src/main.rsTelegramSearchBot.Test/Service/AI/LLM/CodingAgentToolServiceTests.csTelegramSearchBot.Test/Service/AI/LLM/InMemoryRedisTestHarness.csTelegramSearchBot/Extension/ServiceCollectionExtension.csTelegramSearchBot/Service/AI/LLM/CodingAgentReportConsumer.csTelegramSearchBot/Service/AI/LLM/LLMTaskQueueService.csTelegramSearchBot/Service/AI/LLM/RmuxSidecarLauncherService.cs
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
TelegramSearchBot.CodingAgentSidecar/src/main.rs (1)
624-635:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winWrite each log line in a single append.
append_line()does two separate writes, whilerun_agent()and the stderr task can append to the same file concurrently. That can interleave bytes across lines and make job logs unreadable.Proposed fix
async fn append_line(path: &Path, line: &str) -> Result<()> { if let Some(parent) = path.parent() { tokio::fs::create_dir_all(parent).await?; } let mut file = tokio::fs::OpenOptions::new() .create(true) .append(true) .open(path) .await?; - file.write_all(line.as_bytes()).await?; - file.write_all(b"\n").await?; + let mut buffer = Vec::with_capacity(line.len() + 1); + buffer.extend_from_slice(line.as_bytes()); + buffer.push(b'\n'); + file.write_all(&buffer).await?; Ok(()) }🤖 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 `@TelegramSearchBot.CodingAgentSidecar/src/main.rs` around lines 624 - 635, append_line() currently issues two separate writes which can interleave when run_agent() and the stderr appender write concurrently; change append_line() to perform a single atomic write by composing the line with its trailing newline (e.g., format!("{}\n", line) or by building a single byte buffer) and calling write_all once so each log line is appended in one syscall.
🤖 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.
Outside diff comments:
In `@TelegramSearchBot.CodingAgentSidecar/src/main.rs`:
- Around line 624-635: append_line() currently issues two separate writes which
can interleave when run_agent() and the stderr appender write concurrently;
change append_line() to perform a single atomic write by composing the line with
its trailing newline (e.g., format!("{}\n", line) or by building a single byte
buffer) and calling write_all once so each log line is appended in one syscall.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 305e5c34-e6c2-4d73-be99-47a7b770e16c
⛔ Files ignored due to path filters (1)
TelegramSearchBot.CodingAgentSidecar/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
.gitignoreDocs/README_CodingAgentTool.mdDocs/README_MCP.mdTelegramSearchBot.CodingAgentSidecar/Cargo.tomlTelegramSearchBot.CodingAgentSidecar/README.mdTelegramSearchBot.CodingAgentSidecar/src/main.rsTelegramSearchBot.Common/Env.csTelegramSearchBot.Common/Model/AI/LlmAgentContracts.csTelegramSearchBot.LLM/Service/Tools/CodingAgentToolService.csTelegramSearchBot/Extension/ServiceCollectionExtension.csTelegramSearchBot/Service/AI/LLM/CodingAgentReportConsumer.csTelegramSearchBot/Service/AI/LLM/CodingAgentSidecarLauncherService.cs
💤 Files with no reviewable changes (2)
- TelegramSearchBot.Common/Model/AI/LlmAgentContracts.cs
- TelegramSearchBot/Service/AI/LLM/CodingAgentReportConsumer.cs
✅ Files skipped from review due to trivial changes (3)
- TelegramSearchBot.CodingAgentSidecar/README.md
- Docs/README_CodingAgentTool.md
- Docs/README_MCP.md
🚧 Files skipped from review as they are similar to previous changes (3)
- TelegramSearchBot/Extension/ServiceCollectionExtension.cs
- TelegramSearchBot.LLM/Service/Tools/CodingAgentToolService.cs
- TelegramSearchBot.Common/Env.cs
ModerRAS
force-pushed
the
codex/rmux-pi-coding-agent
branch
from
9898ced to
ec32871
Compare
ModerRAS
force-pushed
the
codex/rmux-pi-coding-agent
branch
from
ec32871 to
af2fac8
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
1 participant
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.
Summary
Validation
Summary by CodeRabbit
New Features
Documentation
Tests