Conversation
Reviewer's GuideThis PR refactors and exposes the worker task implementation to streamline GitHub comment posting, updates the global cooldown from 15 to 16 minutes across code and documentation, and adds end-to-end behavioural tests for run_worker using wiremock and Cucumber. Sequence diagram for the updated worker task GitHub posting processsequenceDiagram
participant Worker
participant Queue
participant GitHubAPI
loop For each job in queue
Worker->>Queue: rx.recv().await (get job)
Queue-->>Worker: CommentRequest
Worker->>GitHubAPI: create_comment(owner, repo, pr_number, body)
alt Success
GitHubAPI-->>Worker: 201 Created
Worker->>Queue: guard.commit() (remove job)
else Failure
GitHubAPI-->>Worker: Error/Timeout
Worker--x Queue: (job remains in queue)
end
Worker->>Worker: sleep(cooldown_period_seconds)
end
Class diagram for updated run_worker and config cooldownclassDiagram
class Config {
github_token: String
socket_path: PathBuf
queue_path: PathBuf
cooldown_period_seconds: u64
}
class run_worker {
+run_worker(config: Arc<Config>, rx: Receiver, octocrab: Arc<Octocrab>)
}
Config <.. run_worker : uses
class Octocrab
run_worker <.. Octocrab : uses
class CommentRequest {
owner: String
repo: String
pr_number: u64
body: String
}
run_worker <.. CommentRequest : processes
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary by CodeRabbit
WalkthroughIncrease the default cooldown period for posting GitHub comments from 15 to 16 minutes across code and documentation. Refactor the worker's cooldown logic to always apply after processing, regardless of success or failure. Expand worker testing with new async unit and Cucumber integration tests, and update development dependencies for mocking and API interaction. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test Suite
participant Worker as run_worker
participant MockGH as Mock GitHub Server
participant Queue as Message Queue
Test->>Queue: Enqueue comment request
Test->>MockGH: Configure response (success or error)
Test->>Worker: Start worker
Worker->>Queue: Dequeue request
Worker->>MockGH: POST /repos/:owner/:repo/issues/:num/comments
alt Success (201)
MockGH-->>Worker: 201 Created
Worker->>Queue: Commit (remove job)
else Error (500)
MockGH-->>Worker: 500 Error
Worker->>Queue: Leave job for retry
end
Worker->>Worker: Sleep for cooldown period
Test->>Worker: Abort worker (after short delay)
Test->>Queue: Assert queue state
Test->>MockGH: Assert request count
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~18 minutes Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (9)
🧰 Additional context used📓 Path-based instructions (2)**/*.rs⚙️ CodeRabbit Configuration File
Files:
**/*.md⚙️ CodeRabbit Configuration File
Files:
🧠 Learnings (1)docs/comenq-design.md (3)Learnt from: CR Learnt from: CR Learnt from: CR 🪛 LanguageTooldocs/roadmap.md[uncategorized] ~74-~74: Possible missing article found. (AI_HYDRA_LEO_MISSING_A) 🔇 Additional comments (20)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Hey @leynos - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `crates/comenqd/src/daemon.rs:135` </location>
<code_context>
-
- match tokio::time::timeout(Duration::from_secs(10), post).await {
- Ok(Ok(_)) => {
+ match issues
+ .create_comment(request.pr_number, &request.body)
+ .await
+ {
+ Ok(_) => {
guard.commit()?;
- tokio::time::sleep(Duration::from_secs(config.cooldown_period_seconds)).await;
</code_context>
<issue_to_address>
The timeout around the GitHub API call has been removed, potentially increasing risk of worker hangs.
Without the timeout, the worker may block indefinitely if the GitHub API is slow or unresponsive. Please confirm if this is intentional or consider alternative safeguards against long-running requests.
</issue_to_address>
### Comment 2
<location> `crates/comenqd/src/daemon.rs:169` </location>
<code_context>
+ use wiremock::matchers::{method, path};
+ use wiremock::{Mock, MockServer, ResponseTemplate};
#[tokio::test]
async fn ensure_queue_dir_creates_directory() {
</code_context>
<issue_to_address>
Consider extracting the repeated test setup code into a shared async helper function to reduce duplication and simplify each test.
Here’s one way to drastically cut down on the noise in each test by extracting all of the common setup into a single async helper. You can keep all the functionality exactly the same but now each test is just a couple lines:
```rust
// inside your `mod tests { … }`
async fn setup_run_worker(
status: u16
) -> (MockServer, Arc<Config>, Receiver, Arc<Octocrab>) {
let dir = tempdir().unwrap();
let cfg = Arc::new(Config {
github_token: "t".into(),
socket_path: dir.path().join("sock"),
queue_path: dir.path().join("q"),
cooldown_period_seconds: 0,
});
let (sender, rx) = channel(&cfg.queue_path).unwrap();
let req = CommentRequest { owner: "o".into(), repo: "r".into(), pr_number: 1, body: "b".into() };
sender.send(serde_json::to_vec(&req).unwrap()).await.unwrap();
let server = MockServer::start().await;
Mock::given(method("POST"))
.and(path("/repos/o/r/issues/1/comments"))
.respond_with(ResponseTemplate::new(status))
.mount(&server).await;
let octo = Arc::new(
Octocrab::builder()
.personal_token("t".to_string())
.base_uri(server.uri()).unwrap()
.build().unwrap()
);
(server, cfg, rx, octo)
}
#[tokio::test]
async fn run_worker_commits_on_success() {
let (server, cfg, rx, octo) = setup_run_worker(201).await;
let h = tokio::spawn(run_worker(cfg.clone(), rx, octo));
sleep(Duration::from_millis(50)).await;
h.abort();
assert_eq!(server.received_requests().await.unwrap().len(), 1);
assert_eq!(std::fs::read_dir(&cfg.queue_path).unwrap().count(), 0);
}
#[tokio::test]
async fn run_worker_requeues_on_error() {
let (server, cfg, rx, octo) = setup_run_worker(500).await;
let h = tokio::spawn(run_worker(cfg.clone(), rx, octo));
sleep(Duration::from_millis(50)).await;
h.abort();
assert_eq!(server.received_requests().await.unwrap().len(), 1);
assert!(std::fs::read_dir(&cfg.queue_path).unwrap().count() > 0);
}
```
This:
- centralizes tempdir/Config/channel/request/MockServer/Octocrab setup
- makes each test just “call helper, spawn, sleep, assert”
- keeps exactly the same behavior, but removes ~30 lines of duplication per test.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Summary
Testing
make lintmake testhttps://chatgpt.com/codex/tasks/task_e_6886b799da148322bca3e3f4deb909a0
Summary by Sourcery
Expose and finalise the run_worker async task for dequeuing jobs and posting GitHub comments, simplify its logic, bump the default cooldown period to 16 minutes, update related documentation and roadmap entries, and introduce behavioural tests for the worker.
New Features:
Enhancements:
Build:
Documentation:
Tests: