Skip to content

Commit 4a2e02a

Browse files
authored
[Repo Assist] fix(rust-guard): remove write-only owner-type cache and simplify get_issue_author_association (#4339)
🤖 *This is an automated pull request from Repo Assist.* Closes #4329 ## Root Cause Two dead-code / duplication issues in `guards/github-guard/rust-guard/src/labels/backend.rs`: **Issue 1 — Write-only `repo_owner_type_cache`** `set_cached_owner_is_org` (and the `repo_owner_type_cache` it writes to) existed in anticipation of a `get_cached_owner_is_org` reader that was never wired up. Every `is_repo_private_with_callback` call acquired a Mutex lock and inserted into a `HashMap` that grew without bound and was never consulted. Classic incomplete-feature footgun. **Issue 2 — `get_issue_author_association_with_callback` is a strict subset of `get_issue_author_info_with_callback`** Both functions made the same `issue_read` backend call, parsed the same args, allocated the same buffer size, and extracted `author_association` via the identical two-field fallback. The only difference was that the `info` variant also extracted `author_login`. This 35-line duplication meant any future change to the `issue_read` call shape had to be applied in two places. ## Fix 1. **Remove** `repo_owner_type_cache`, `set_cached_owner_is_org`, and the call site inside `is_repo_private_with_callback`. The debug log line is retained; only the cache write is dropped. 2. **Replace** the 35-line body of `get_issue_author_association_with_callback` with a one-liner that delegates to `get_issue_author_info_with_callback`: ```rust get_issue_author_info_with_callback(callback, owner, repo, issue_number) .and_then(|info| info.author_association) ``` ## Trade-offs - Purely mechanical changes; zero semantic change. - Removes 41 lines net. The `get_issue_author_association` public API is unchanged. ## Test Status - ✅ `cargo build` — compiles without warnings - ✅ `cargo test` — **322 / 322 tests pass** - ⚠️ Go build/test skipped — `proxy.golang.org` blocked by network firewall (pre-existing infrastructure limitation) > Generated by [Repo Assist](https://github.com/github/gh-aw-mcpg/actions/runs/24778701637/agentic_workflow) · ● 3M · [◷](https://github.com/search?q=repo%3Agithub%2Fgh-aw-mcpg+%22gh-aw-workflow-id%3A+repo-assist%22&type=pullrequests) > > To install this [agentic workflow](https://github.com/githubnext/agentics/blob/851905c06e905bf362a9f6cc54f912e3df747d55/workflows/repo-assist.md), run > ``` > gh aw add githubnext/agentics@851905c > ``` <!-- gh-aw-agentic-workflow: Repo Assist, engine: copilot, model: auto, id: 24778701637, workflow_id: repo-assist, run: https://github.com/github/gh-aw-mcpg/actions/runs/24778701637 --> <!-- gh-aw-workflow-id: repo-assist -->
2 parents 8f964e0 + 98c24b9 commit 4a2e02a

1 file changed

Lines changed: 3 additions & 41 deletions

File tree

  • guards/github-guard/rust-guard/src/labels

guards/github-guard/rust-guard/src/labels/backend.rs

Lines changed: 3 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,6 @@ fn repo_visibility_cache() -> &'static Mutex<HashMap<String, bool>> {
1919
CACHE.get_or_init(|| Mutex::new(HashMap::new()))
2020
}
2121

22-
fn repo_owner_type_cache() -> &'static Mutex<HashMap<String, bool>> {
23-
static CACHE: OnceLock<Mutex<HashMap<String, bool>>> = OnceLock::new();
24-
CACHE.get_or_init(|| Mutex::new(HashMap::new()))
25-
}
26-
2722
/// Cache for collaborator permission lookups keyed by "owner/repo:username" (all lowercase).
2823
/// This is a process-wide static cache that persists across requests. Because the WASM guard
2924
/// is instantiated per-request in the gateway, entries accumulate over the process lifetime.
@@ -60,12 +55,6 @@ fn set_cached_repo_visibility(repo_id: &str, is_private: bool) {
6055
}
6156
}
6257

63-
fn set_cached_owner_is_org(repo_id: &str, is_org: bool) {
64-
if let Ok(mut cache) = repo_owner_type_cache().lock() {
65-
cache.insert(repo_id.to_string(), is_org);
66-
}
67-
}
68-
6958
#[derive(Debug, Clone)]
7059
pub struct PullRequestFacts {
7160
pub author_association: Option<String>,
@@ -218,9 +207,8 @@ pub fn is_repo_private_with_callback(
218207
}
219208

220209
let result = extract_repo_private_flag(&response, &repo_id);
221-
// Piggyback owner type extraction from the same search response
210+
// Piggyback owner type extraction from the same search response for debug logging
222211
if let Some(is_org) = extract_owner_is_org(&response, &repo_id) {
223-
set_cached_owner_is_org(&repo_id, is_org);
224212
crate::log_debug(&format!(
225213
"Repo owner type for {}: {}",
226214
repo_id,
@@ -347,34 +335,8 @@ pub fn get_issue_author_association_with_callback(
347335
repo: &str,
348336
issue_number: &str,
349337
) -> Option<String> {
350-
if owner.is_empty() || repo.is_empty() || issue_number.is_empty() {
351-
return None;
352-
}
353-
354-
let args = serde_json::json!({
355-
"owner": owner,
356-
"repo": repo,
357-
"issue_number": issue_number,
358-
"method": "get",
359-
});
360-
361-
let args_str = args.to_string();
362-
let mut result_buffer = vec![0u8; SMALL_BUFFER_SIZE];
363-
364-
let len = match callback("issue_read", &args_str, &mut result_buffer) {
365-
Ok(len) if len > 0 => len,
366-
_ => return None,
367-
};
368-
369-
let response_str = std::str::from_utf8(&result_buffer[..len]).ok()?;
370-
let response = serde_json::from_str::<Value>(response_str).ok()?;
371-
let issue = super::extract_mcp_response(&response);
372-
373-
issue
374-
.get("author_association")
375-
.or_else(|| issue.get("authorAssociation"))
376-
.and_then(|v| v.as_str())
377-
.map(String::from)
338+
get_issue_author_info_with_callback(callback, owner, repo, issue_number)
339+
.and_then(|info| info.author_association)
378340
}
379341

380342
pub fn get_pull_request_facts(

0 commit comments

Comments
 (0)