From 86175f65eb88b1d0ea9918dca7f1cf130ec6bc15 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Mon, 16 Feb 2026 20:11:17 +0000 Subject: [PATCH 1/2] docs: fix README discrepancies and cleanup outdated docs - Delete `GITLAB_API_CONSOLIDATION_ANALYSIS.md` as its recommendations are implemented. - Add `LICENSE` file (MIT) and update `Cargo.toml`. - Update `README.md` to clarify command usage order. - Restore `CLAUDE.md` as symlink to `AGENTS.md`. Co-authored-by: myaple <10523487+myaple@users.noreply.github.com> --- Cargo.toml | 1 + GITLAB_API_CONSOLIDATION_ANALYSIS.md | 288 --------------------------- LICENSE | 21 ++ README.md | 1 + 4 files changed, 23 insertions(+), 288 deletions(-) delete mode 100644 GITLAB_API_CONSOLIDATION_ANALYSIS.md create mode 100644 LICENSE diff --git a/Cargo.toml b/Cargo.toml index 0a1f339..c2722cc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,6 +3,7 @@ name = "gitbot" version = "0.1.0" edition = "2021" description = "A GitLab bot that responds to mentions using AI" +license = "MIT" [dependencies] tokio = { version = "1", features = ["macros", "rt-multi-thread"] } diff --git a/GITLAB_API_CONSOLIDATION_ANALYSIS.md b/GITLAB_API_CONSOLIDATION_ANALYSIS.md deleted file mode 100644 index 3f4d64a..0000000 --- a/GITLAB_API_CONSOLIDATION_ANALYSIS.md +++ /dev/null @@ -1,288 +0,0 @@ -# GitLab API Consolidation Analysis - -## Executive Summary - -This analysis identifies inconsistencies in how we handle GitLab API operations and provides specific recommendations for consolidating our GitLab API surface area. By consolidating these operations, we can reduce code duplication, improve maintainability, and simplify our API client. - -## Key Findings - -### 1. **CRITICAL: Duplicate Search Functionality** - -**Current State:** -- `search_files_by_name()` (src/gitlab.rs:565) - delegates to `search_files_by_content` -- `search_files_by_content()` (src/gitlab.rs:575) - searches with `scope=blobs` -- `search_code()` (src/gitlab.rs:678) - searches with `scope=blobs` - -**Problem:** -Both `search_files_by_content()` and `search_code()` hit the exact same GitLab API endpoint: -``` -GET /api/v4/projects/{project_id}/search?scope=blobs&search={query}&ref={ref} -``` - -The only difference is that `search_code()` allows URL encoding in the path while `search_files_by_content()` uses query params, but they're functionally identical. - -**Usage:** -- `search_files_by_name`: Not used directly in codebase -- `search_files_by_content`: Not used directly in codebase -- `search_code`: Used in tools.rs:510 for the search_code tool - -**Recommendation:** -- **CONSOLIDATE** into a single `search_repository()` method with flexible parameters -- Remove `search_files_by_name()` and `search_files_by_content()` -- Keep `search_code()` but refactor to use the consolidated method - ---- - -### 2. **Issues Fetching - Multiple Overlapping Methods** - -**Current State:** -- `get_issues()` (src/gitlab.rs:221) - fetches issues with `updated_after` filter (all states) -- `get_opened_issues()` (src/gitlab.rs:249) - fetches issues with `updated_after` + `state=opened` filter -- `get_issues_with_label()` (src/gitlab.rs:710) - fetches issues with `labels` + `state=opened` filter - -**Problem:** -All three methods hit the same endpoint: -``` -GET /api/v4/projects/{project_id}/issues -``` - -Each method hardcodes different query parameters, but they could be unified with a flexible parameter approach. - -**Usage:** -- `get_issues`: polling.rs:153 (fetching recent issues) -- `get_opened_issues`: polling.rs:227 (stale issue checking) -- `get_issues_with_label`: triage.rs:157 (label learning) - -**Recommendation:** -- **CONSOLIDATE** into a single `get_issues()` method with an optional `IssueQueryOptions` struct: - ```rust - pub struct IssueQueryOptions { - pub updated_after: Option, - pub state: Option, // "opened", "closed", etc. - pub labels: Option>, - pub per_page: Option, - pub order_by: Option, - pub sort: Option, - } - ``` -- Keep convenience methods as thin wrappers if needed for backward compatibility - -**Impact:** Medium - affects 3 call sites (polling.rs, triage.rs) - ---- - -### 3. **Notes Fetching - Unnecessary Wrapper Pattern** - -**Current State (Issues):** -- `get_issue_notes()` (src/gitlab.rs:306) - wrapper calling `get_issue_notes_with_options` with `Some(timestamp)` -- `get_issue_notes_with_options()` (src/gitlab.rs:318) - main implementation -- `get_all_issue_notes()` (src/gitlab.rs:392) - wrapper calling `get_issue_notes_with_options` with `None` - -**Current State (MRs):** -- `get_merge_request_notes()` (src/gitlab.rs:348) - wrapper calling `get_merge_request_notes_with_options` with `Some(timestamp)` -- `get_merge_request_notes_with_options()` (src/gitlab.rs:360) - main implementation -- `get_all_merge_request_notes()` (src/gitlab.rs:403) - wrapper calling `get_merge_request_notes_with_options` with `None` - -**Problem:** -Six methods to fetch notes when we really only need two (one for issues, one for MRs). The "options" pattern is overly complex for a single optional parameter. - -**Usage:** -- `get_issue_notes`: polling.rs:330, polling.rs:561, handlers.rs:651 -- `get_all_issue_notes`: handlers.rs:603, tools.rs:884 -- `get_merge_request_notes`: polling.rs:407 - -**Recommendation:** -- **CONSOLIDATE** by making `since_timestamp` an `Option` parameter -- Remove the `_with_options` and `get_all_*` variants -- Update method signatures: - ```rust - pub async fn get_issue_notes( - &self, - project_id: i64, - issue_iid: i64, - since_timestamp: Option, - ) -> Result, GitlabError> - - pub async fn get_merge_request_notes( - &self, - project_id: i64, - mr_iid: i64, - since_timestamp: Option, - ) -> Result, GitlabError> - ``` - -**Impact:** Medium - affects 6 call sites (polling.rs, handlers.rs, tools.rs) - ---- - -### 4. **Label Operations - Multiple Methods for Same Endpoint** - -**Current State:** -- `add_issue_label()` (src/gitlab.rs:634) - adds single label via `PUT` with `add_labels` -- `remove_issue_label()` (src/gitlab.rs:647) - removes single label via `PUT` with `remove_labels` -- `set_issue_labels()` (src/gitlab.rs:730) - replaces all labels via `PUT` with `labels` -- `add_issue_labels()` (src/gitlab.rs:745) - adds multiple labels via `PUT` with `add_labels` - -**Problem:** -All four methods hit the same endpoint: -``` -PUT /api/v4/projects/{project_id}/issues/{issue_iid} -``` - -They only differ in the JSON body parameter (`add_labels`, `remove_labels`, or `labels`). - -**Usage:** -- `add_issue_label`: polling.rs:617 (stale label management) -- `remove_issue_label`: polling.rs:634 (stale label management), handlers.rs:1525 (slash command) -- `add_issue_labels`: triage.rs:463 (triage labels) -- `set_issue_labels`: Not used directly in codebase - -**Recommendation:** -- **CONSOLIDATE** into a single `update_issue_labels()` method: - ```rust - pub enum LabelOperation { - Add(Vec), // add_labels - Remove(Vec), // remove_labels - Set(Vec), // labels (replaces all) - } - - pub async fn update_issue_labels( - &self, - project_id: i64, - issue_iid: i64, - operation: LabelOperation, - ) -> Result - ``` -- Optionally keep convenience methods as thin wrappers for common operations - -**Impact:** Low - affects 4 call sites, but changes are straightforward - ---- - -### 5. **Comment Posting - Nearly Identical Implementations** - -**Current State:** -- `post_comment_to_issue()` (src/gitlab.rs:187) - POST to `/api/v4/projects/{project_id}/issues/{issue_iid}/notes` -- `post_comment_to_merge_request()` (src/gitlab.rs:200) - POST to `/api/v4/projects/{project_id}/merge_requests/{mr_iid}/notes` - -**Problem:** -These methods are 95% identical - same body structure, same error handling, only the URL path differs (issues vs merge_requests). - -**Usage:** -- `post_comment_to_issue`: handlers.rs:1074 -- `post_comment_to_merge_request`: handlers.rs:1103 - -**Recommendation:** -Two options: - -**Option A: Generic Internal Method** -```rust -enum NoteableType { - Issue, - MergeRequest, -} - -async fn post_comment_internal( - &self, - project_id: i64, - noteable_iid: i64, - noteable_type: NoteableType, - comment_body: &str, -) -> Result -``` - -**Option B: Keep As-Is (Recommended)** -The duplication is minimal and the API is clearer with two explicit methods. This is acceptable as-is. - -**Impact:** Very Low - the current approach is reasonable - ---- - -## Summary of Recommendations - -| Priority | Issue | Methods Affected | Lines of Code Saved | Complexity Reduction | -|----------|-------|------------------|---------------------|---------------------| -| **HIGH** | Duplicate Search | 3 methods | ~80 lines | Eliminates duplicate endpoint | -| **MEDIUM** | Issues Fetching | 3 methods | ~60 lines | Single flexible method | -| **MEDIUM** | Notes Fetching | 6 methods | ~100 lines | Simplifies API | -| **LOW** | Label Operations | 4 methods | ~50 lines | Cleaner abstraction | -| **VERY LOW** | Comment Posting | 2 methods | ~20 lines | Keep as-is | - -## Total Impact - -- **Potential reduction**: ~290 lines of code in gitlab.rs -- **Methods reduced**: From 33 to ~20 methods (~40% reduction) -- **API surface area**: Significantly simplified -- **Call sites to update**: ~15 locations across the codebase - -## Implementation Priority - -1. **Phase 1** (High Priority): Consolidate search functionality - - Remove duplicate search methods - - Update tools.rs search_code tool - -2. **Phase 2** (Medium Priority): Consolidate issue fetching - - Create flexible IssueQueryOptions - - Update polling.rs and triage.rs call sites - -3. **Phase 3** (Medium Priority): Simplify notes fetching - - Make since_timestamp optional - - Update all notes fetching call sites - -4. **Phase 4** (Low Priority): Consolidate label operations - - Create LabelOperation enum - - Update label management call sites - -## Additional Observations - -### Repository Tree Caching -The `get_repository_tree()` method (src/gitlab.rs:414) implements its own pagination and caching logic. This is good, but the cache TTL is hardcoded. Consider making it configurable. - -### Timestamp Handling Duplication -Multiple methods repeat the same timestamp conversion logic: -```rust -let dt = DateTime::from_timestamp(timestamp as i64, 0).unwrap_or_else(|| { - Utc.timestamp_opt(0, 0) - .single() - .expect("Fallback timestamp failed for 0") -}); -let formatted_timestamp_string = dt.to_rfc3339(); -``` - -This appears in: -- `get_issues()` (line 227-232) -- `get_opened_issues()` (line 255-260) -- `get_merge_requests()` (line 284-289) -- `get_issue_notes_with_options()` (line 330-336) -- `get_merge_request_notes_with_options()` (line 372-378) - -**Recommendation**: Extract to a helper method: -```rust -fn format_timestamp(timestamp: u64) -> String { - DateTime::from_timestamp(timestamp as i64, 0) - .unwrap_or_else(|| Utc.timestamp_opt(0, 0).single().expect("Fallback timestamp failed")) - .to_rfc3339() -} -``` - -## Testing Considerations - -Any consolidation work should maintain or improve test coverage. Current test coverage is good: -- `test_get_issues()` (src/tests/gitlab_tests.rs:239) -- `test_get_opened_issues()` (src/tests/gitlab_tests.rs:1054) -- `test_get_issue_notes()` (src/tests/gitlab_tests.rs:333) -- `test_get_all_issue_notes()` (src/tests/gitlab_tests.rs:825) -- Multiple label operation tests - -Tests should be updated to cover the consolidated methods with various parameter combinations. - -## Conclusion - -The GitLab API client has grown organically with specific methods for specific use cases. While this approach works, consolidating these methods into more flexible, parameterized versions will: - -1. Reduce code duplication -2. Make the API easier to understand and maintain -3. Reduce the total surface area of GitLab API we need to cover -4. Make future extensions easier (e.g., adding new query parameters) - -The most critical issue is the duplicate search functionality, which should be addressed first. diff --git a/LICENSE b/LICENSE new file mode 100644 index 0000000..76548fa --- /dev/null +++ b/LICENSE @@ -0,0 +1,21 @@ +MIT License + +Copyright (c) 2024 GitBot Contributors + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/README.md b/README.md index 1ab0966..51a71e6 100644 --- a/README.md +++ b/README.md @@ -171,6 +171,7 @@ GitBot supports extensive configuration through command line arguments or enviro 1. **Set up the bot**: Configure GitBot with your GitLab and LLM API credentials 2. **Add to repositories**: Ensure the bot user has access to the repositories you want to monitor 3. **Mention the bot**: Use `@your-bot-username` in issue or MR comments to trigger responses + - **Note**: Commands must follow the mention (e.g., `@gitbot /plan`, not `/plan @gitbot`) 4. **Get AI assistance**: The bot will analyze the context and provide intelligent responses ### Example Interactions From e15a9b97ded2c9be54a9039334e2be205edeacba Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Mon, 16 Feb 2026 20:24:23 +0000 Subject: [PATCH 2/2] docs: fix README discrepancies and cleanup outdated docs - Delete `GITLAB_API_CONSOLIDATION_ANALYSIS.md` as its recommendations are implemented. - Add `LICENSE` file (MIT) and update `Cargo.toml`. - Update `README.md` to clarify command usage order. - Restore `CLAUDE.md` as symlink to `AGENTS.md`. Co-authored-by: myaple <10523487+myaple@users.noreply.github.com>