Skip to content

feat(fetch): header-exact backoff, workdir watch, PR-list throttle #9

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jcardonne merged 1 commit into from
Jul 12, 2026
Merged

feat(fetch): header-exact backoff, workdir watch, PR-list throttle #9

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 85 additions & 20 deletions src-tauri/src/git/avatars.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use std::collections::{HashMap, HashSet};
use std::path::{Path, PathBuf};
use std::sync::LazyLock;
use std::time::{SystemTime, UNIX_EPOCH};
use tauri::{AppHandle, Manager};
use tauri::{AppHandle, Emitter, Manager};

/// The avatar disc in the graph is small; 64px covers retina.
const AVATAR_SIZE: u32 = 64;
Expand Down Expand Up @@ -116,14 +116,26 @@ pub fn resolve(

// Only write the cache when the fetch completed: on a transient error keep
// the cached positives and retry next time (don't poison with negatives).
if let Ok(found) = fetched {
if let Ok((found, backoff)) = fetched {
if let Some(secs) = backoff {
// Header-exact: tell the UI precisely how long to pause background fetch.
let _ = app.emit("rate-limited", secs);
}
let mut st = CACHE.lock();
for email in &missing {
let url = found.get(email).cloned().unwrap_or_default();
if !url.is_empty() {
out.insert(email.clone(), url.clone());
match found.get(email).filter(|u| !u.is_empty()) {
Some(url) => {
out.insert(email.clone(), url.clone());
st.map.insert(email.clone(), CacheEntry { url: url.clone(), fetched_at: now });
}
// Negative-cache a real miss only when the scan completed; a
// rate-limit cut it short, so don't mark un-scanned authors as
// "no avatar" (which would show Gravatar for the whole NEG TTL).
None if backoff.is_none() => {
st.map.insert(email.clone(), CacheEntry { url: String::new(), fetched_at: now });
}
None => {}
}
st.map.insert(email.clone(), CacheEntry { url, fetched_at: now });
}
save_cache(&path, &st.map);
}
Expand Down Expand Up @@ -196,11 +208,37 @@ pub(crate) fn encode_path(path: &str) -> String {
.join("/")
}

/// Seconds to pause background network after a rate-limit response. An explicit
/// `Retry-After` (seconds) wins; else the `*-RateLimit-Reset` epoch minus now;
/// clamped to 30s..1h; else a 15-min fallback that matches the frontend default.
fn backoff_secs(retry_after: Option<&str>, reset: Option<&str>, now: u64) -> u64 {
if let Some(ra) = retry_after.and_then(|v| v.trim().parse::<u64>().ok()) {
return ra.clamp(30, 3600);
}
if let Some(reset) = reset.and_then(|v| v.trim().parse::<u64>().ok()) {
if reset > now {
return (reset - now).clamp(30, 3600);
}
}
900
}

/// Pull the backoff window out of a rate-limited response's headers.
fn resp_backoff(resp: &ureq::Response) -> u64 {
backoff_secs(
resp.header("retry-after"),
resp.header("x-ratelimit-reset").or_else(|| resp.header("ratelimit-reset")),
now_secs(),
)
}

/// Returns the resolved avatars plus, when the API rate-limited us mid-scan, the
/// seconds to back off (so `resolve` can tell the UI exactly how long to pause).
fn fetch_github(
target: &RemoteTarget,
head: Option<&str>,
wanted: &HashSet<String>,
) -> AppResult<HashMap<String, String>> {
) -> AppResult<(HashMap<String, String>, Option<u64>)> {
let token = provider_token("github", &target.host);
let path = encode_path(&target.path);
let mut out = HashMap::new();
Expand All @@ -219,13 +257,15 @@ fn fetch_github(
// so resolve() caches nothing and retries next time - deliberately NOT falling
// through to cache a partial result, which would negative-cache feature-branch
// authors the aborted head scan never got to.
let mut backoff = None;
if let Some(h) = head {
scan_commits(&path, Some(&encode_path(h)), token.as_deref(), wanted, &mut out, &mut remaining)?;
backoff = scan_commits(&path, Some(&encode_path(h)), token.as_deref(), wanted, &mut out, &mut remaining)?;
}
if remaining > 0 {
scan_commits(&path, None, token.as_deref(), wanted, &mut out, &mut remaining)?;
// A rate-limited head scan means the default scan would hit the same wall.
if remaining > 0 && backoff.is_none() {
backoff = scan_commits(&path, None, token.as_deref(), wanted, &mut out, &mut remaining)?;
}
Ok(out)
Ok((out, backoff))
}

/// Page through a repo's commits (optionally pinned to `sha` = a branch/ref),
Expand All @@ -239,7 +279,7 @@ fn scan_commits(
wanted: &HashSet<String>,
out: &mut HashMap<String, String>,
remaining: &mut usize,
) -> AppResult<()> {
) -> AppResult<Option<u64>> {
for page in 1..=MAX_PAGES {
if *remaining == 0 {
break;
Expand All @@ -258,7 +298,12 @@ fn scan_commits(
}
let commits: Vec<GhCommit> = match req.call() {
Ok(r) => r.into_json()?,
Err(ureq::Error::Status(_, _)) => break,
// 403/429 = rate-limited: surface the reset so the UI backs off exactly.
// Any other Status (e.g. 404 unpushed ref) just ends the scan with what
// we have.
Err(ureq::Error::Status(code, resp)) => {
return Ok((code == 403 || code == 429).then(|| resp_backoff(&resp)));
}
Err(ureq::Error::Transport(_)) => return Err(AppError::Msg("github request failed".into())),
};
let n = commits.len();
Expand All @@ -270,7 +315,7 @@ fn scan_commits(
break; // last page
}
}
Ok(())
Ok(None)
}

fn sized_github(url: &str) -> String {
Expand Down Expand Up @@ -327,12 +372,12 @@ fn fetch_gitlab(
target: &RemoteTarget,
head: Option<&str>,
wanted: &HashSet<String>,
) -> AppResult<HashMap<String, String>> {
) -> AppResult<(HashMap<String, String>, Option<u64>)> {
// Private repos (the common case) require auth; with no token there's nothing
// to resolve, so bail to Gravatar instead of erroring.
let token = match provider_token("gitlab", &target.host) {
Some(t) => t,
None => return Ok(HashMap::new()),
None => return Ok((HashMap::new(), None)),
};
let refname = head.unwrap_or("HEAD");
let endpoint = format!("https://{}/api/graphql", target.host);
Expand All @@ -352,7 +397,12 @@ fn fetch_gitlab(
.send_json(body)
{
Ok(r) => r.into_json()?,
Err(ureq::Error::Status(_, _)) => break,
Err(ureq::Error::Status(code, resp)) => {
if code == 403 || code == 429 {
return Ok((out, Some(resp_backoff(&resp))));
}
break; // other Status (bad ref / no access): keep the partial result
}
Err(ureq::Error::Transport(_)) => return Err(AppError::Msg("gitlab request failed".into())),
};
let commits = match resp
Expand All @@ -376,7 +426,7 @@ fn fetch_gitlab(
None => break,
}
}
Ok(out)
Ok((out, None))
}

/// GitLab `avatarUrl` is instance-relative (`/uploads/.../avatar.png`); make it
Expand Down Expand Up @@ -476,6 +526,21 @@ mod tests {
RemoteTarget { host: host.into(), provider, path: path.into() }
}

#[test]
fn backoff_prefers_retry_after_then_reset_then_default() {
// Retry-After (seconds) wins, clamped to 30s..1h.
assert_eq!(backoff_secs(Some("90"), Some("999999"), 1000), 90);
assert_eq!(backoff_secs(Some("5"), None, 1000), 30); // clamp min
assert_eq!(backoff_secs(Some("99999"), None, 1000), 3600); // clamp max
// Else X-RateLimit-Reset (epoch) minus now.
assert_eq!(backoff_secs(None, Some("1300"), 1000), 300);
// Reset already in the past -> fall through to the default.
assert_eq!(backoff_secs(None, Some("900"), 1000), 900);
// No usable header, or an unparseable one -> 15-min default.
assert_eq!(backoff_secs(None, None, 1000), 900);
assert_eq!(backoff_secs(Some("soon"), None, 1000), 900);
}

#[test]
fn sizes_github_url() {
assert_eq!(
Expand Down Expand Up @@ -529,7 +594,7 @@ mod tests {
.collect();
assert!(!wanted.is_empty(), "no linked GitHub author emails discovered");
// Emails were discovered from the default branch (no sha), so scan that.
let out = fetch_github(&t, None, &wanted).unwrap();
let (out, _backoff) = fetch_github(&t, None, &wanted).unwrap();
assert!(!out.is_empty(), "expected resolved GitHub avatars for {} emails: {out:?}", wanted.len());
}

Expand Down Expand Up @@ -563,7 +628,7 @@ mod tests {
})
.unwrap_or_default();
assert!(!wanted.is_empty(), "no linked GitLab author emails discovered");
let out = fetch_gitlab(&t, Some("main"), &wanted).unwrap();
let (out, _backoff) = fetch_gitlab(&t, Some("main"), &wanted).unwrap();
assert!(!out.is_empty(), "expected resolved GitLab avatars for {} emails: {out:?}", wanted.len());
}
}
125 changes: 100 additions & 25 deletions src-tauri/src/watch.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,21 @@
//! Filesystem watching for open repos. Watching the `.git` dir lets the UI react
//! the instant git state changes underneath it - a commit/checkout/stash from the
//! terminal, a rebase by another tool, a branch deleted by a script - without the
//! user hitting refresh. It is purely local: no network, so it never touches a
//! remote or a rate limit. Remote changes still need an explicit/auto fetch.
//! Filesystem watching for open repos. Watching the `.git` dir AND the working
//! tree lets the UI react the instant things change underneath it - a
//! commit/checkout/stash/rebase from the terminal, a branch deleted by a script,
//! or a file edited in an external editor - without the user hitting refresh. It
//! is purely local: no network, so it never touches a remote or a rate limit.
//! Remote changes still need an explicit/auto fetch.
//!
//! We watch the git dir, NOT the working tree: refs/HEAD/index there are the
//! high-signal, low-noise surface (a whole `node_modules` under the workdir would
//! be the opposite). Working-tree edits still refresh on window focus.
//! The working-tree half is gitignore-filtered (`is_relevant_change`): a raw file
//! save refreshes the WIP list, but a build dir churning (node_modules, target,
//! dist) is ignored so it can't spam refreshes.
//!
//! ponytail: workdir file-watch skipped - focus-refresh already covers external
//! editor saves; add a (gitignore-filtered) workdir watch only if instant WIP
//! updates become worth the event noise.
//! ponytail: recursive-watch a giant workdir is cheap on macOS (one FSEvents
//! stream) but registers a watch per dir on Linux inotify - if a Linux build ships
//! and hits huge `node_modules` trees, cap depth or prune ignored dirs before the
//! watch, and mind `fs.inotify.max_user_watches`.

use std::collections::HashMap;
use std::path::PathBuf;
use std::path::{Path, PathBuf};
use std::time::Duration;

use notify_debouncer_mini::{
Expand All @@ -39,24 +41,53 @@ pub struct Watchers(Mutex<HashMap<String, Debouncer<RecommendedWatcher>>>);
/// should collapse to one refresh: wait this long after the last write.
const DEBOUNCE: Duration = Duration::from_millis(400);

/// Resolve symlinks so a watched dir and the paths notify reports for it compare
/// equal - on macOS a temp/workdir under `/var/...` surfaces as `/private/var/...`.
fn canon(p: &Path) -> PathBuf {
std::fs::canonicalize(p).unwrap_or_else(|_| p.to_path_buf())
}

/// The git dir(s) whose changes matter for `repo`. A linked worktree keeps
/// HEAD/index in its own git dir (`path()`) but shares refs/packed-refs in the
/// common dir, so both are watched; they're equal for a normal repo (one entry).
fn git_dirs(repo: &str) -> AppResult<Vec<PathBuf>> {
let git = git2::Repository::open(repo)?;
let mut dirs = vec![git.path().to_path_buf()];
let common = git.commondir().to_path_buf();
if common != git.path() {
let mut dirs = vec![canon(git.path())];
let common = canon(git.commondir());
if !dirs.contains(&common) {
dirs.push(common);
}
Ok(dirs)
}

/// Start watching `repo`'s git dir(s), calling `on_change` after each debounced
/// change batch. Idempotent: a second call for an already-watched path is a no-op
/// (so re-activating a tab doesn't stack watchers). The tauri command wraps this
/// with an event emit; the split keeps the file-watch mechanism testable without
/// an AppHandle.
/// Is this changed path worth a UI refresh? A git-dir write (refs/HEAD/index/new
/// objects) always is. A working-tree write matters only if git doesn't ignore it
/// - otherwise a build dir (node_modules, target, dist) churning would spam
/// refreshes. Anything outside both trees is noise.
fn is_relevant_change(
path: &Path,
git_dirs: &[PathBuf],
workdir: Option<&Path>,
repo: Option<&git2::Repository>,
) -> bool {
if git_dirs.iter().any(|g| path.starts_with(g)) {
return true; // git-state change (also covers the in-tree .git)
}
match workdir {
Some(wd) if path.starts_with(wd) => {
let rel = path.strip_prefix(wd).unwrap_or(path);
// No repo handle (open failed) -> can't check ignore, treat as relevant.
repo.map_or(true, |r| !r.is_path_ignored(rel).unwrap_or(false))
}
_ => false,
}
}

/// Start watching `repo`'s git dir(s) AND its working tree, calling `on_change`
/// after each debounced batch that carries a relevant change. Idempotent: a second
/// call for an already-watched path is a no-op (so re-activating a tab doesn't
/// stack watchers). The tauri command wraps this with an event emit; the split
/// keeps the file-watch mechanism testable without an AppHandle.
fn watch_with(
state: &Watchers,
repo: &str,
Expand All @@ -66,22 +97,42 @@ fn watch_with(
if map.contains_key(repo) {
return Ok(());
}
let dirs = git_dirs(repo)?;
let git_dirs = git_dirs(repo)?;
let workdir = git2::Repository::open(repo)?.workdir().map(canon);

let repo_path = repo.to_string();
let git_dirs_f = git_dirs.clone();
let workdir_f = workdir.clone();
let mut debouncer = new_debouncer(DEBOUNCE, move |res: DebounceEventResult| {
// A watch error (e.g. the dir vanished) just means no event this round;
// the next real change re-fires. Only a successful batch pings the UI.
if res.is_ok() {
let Ok(events) = res else { return }; // watch error: next real change re-fires
// Open the repo per batch (cheap) so .gitignore is honoured live; refresh
// only if some path in the batch is a git-state or non-ignored file change.
let r = git2::Repository::open(&repo_path).ok();
let hit = events
.iter()
.any(|ev| is_relevant_change(&ev.path, &git_dirs_f, workdir_f.as_deref(), r.as_ref()));
if hit {
on_change();
}
})
.map_err(|e| AppError::Msg(format!("watch init: {e}")))?;

for dir in &dirs {
// Watch the git dir(s) - covers a worktree/submodule gitdir that lives OUTSIDE
// the workdir - plus the workdir for file edits. In a normal repo the .git dir
// sits inside the workdir (watched twice); the debouncer coalesces the dup
// events and is_relevant_change classifies them once.
for dir in &git_dirs {
debouncer
.watcher()
.watch(dir, RecursiveMode::Recursive)
.map_err(|e| AppError::Msg(format!("watch {}: {e}", dir.display())))?;
}
if let Some(wd) = &workdir {
debouncer
.watcher()
.watch(wd, RecursiveMode::Recursive)
.map_err(|e| AppError::Msg(format!("watch {}: {e}", wd.display())))?;
}
map.insert(repo.to_string(), debouncer);
Ok(())
}
Expand Down Expand Up @@ -145,4 +196,28 @@ mod tests {
let _ = std::fs::remove_dir_all(&dir);
assert!(hits.load(Ordering::SeqCst) > 0, "a .git write should fire on_change");
}

/// The workdir/gitignore classifier: git-state and non-ignored file changes are
/// relevant; ignored build dirs and out-of-tree paths are not.
#[test]
fn classifies_git_state_workdir_and_ignored_paths() {
let dir = std::env::temp_dir().join(format!("gitchef-relevant-{}", std::process::id()));
let _ = std::fs::remove_dir_all(&dir);
let repo = git2::Repository::init(&dir).unwrap();
std::fs::write(dir.join(".gitignore"), "node_modules/\n").unwrap();
let wd = canon(repo.workdir().unwrap());
let gdirs = vec![canon(repo.path())];
let r = Some(&repo);

// git-state write (under .git) is always relevant.
assert!(is_relevant_change(&gdirs[0].join("refs/heads/x"), &gdirs, Some(&wd), r));
// an ordinary source edit refreshes the WIP list.
assert!(is_relevant_change(&wd.join("src/main.rs"), &gdirs, Some(&wd), r));
// a gitignored build dir is noise.
assert!(!is_relevant_change(&wd.join("node_modules/pkg/index.js"), &gdirs, Some(&wd), r));
// a path outside the tree is noise.
assert!(!is_relevant_change(Path::new("/tmp/somewhere-else/x"), &gdirs, Some(&wd), r));

let _ = std::fs::remove_dir_all(&dir);
}
}
Loading
Loading