diff --git a/crates/tty7-core/src/core/github/accounts.rs b/crates/tty7-core/src/core/github/accounts.rs new file mode 100644 index 00000000..cddf10c8 --- /dev/null +++ b/crates/tty7-core/src/core/github/accounts.rs @@ -0,0 +1,297 @@ +//! Reading a repository with whichever signed-in account can see it. +//! +//! `gh` keeps several github.com accounts but lends out one at a time — the +//! active one. A private repository owned by a work organisation reads as a +//! 404 to a personal account, and flipping `gh auth switch` back and forth +//! for the panel's sake flips it for every repository at once. +//! +//! [`FallbackTransport`] asks the active account first. When the answer is +//! one another account could do better on — a 404 (GitHub's "not found, or +//! not yours to see"), a 401, a 403 such as SSO enforcement — it tries the +//! others in turn, and remembers, per repository owner, the account that got +//! through, so the next request for that owner goes to it directly. +//! +//! The other accounts are only listed when first needed: listing them means +//! `gh auth status`, which checks each one against GitHub. + +use std::collections::HashMap; +use std::sync::{Arc, Mutex, OnceLock}; + +use super::api::{ApiError, Reply, Transport}; + +/// One account's way in, and its login for the log. +pub struct Account { + pub login: String, + pub transport: Arc, +} + +/// Lists the accounts past the first. Blocking; called at most once. +pub type LoadOthers = Box Vec + Send>; + +pub struct FallbackTransport { + first: Arc, + load: Mutex>, + others: OnceLock>, + /// Owner (lowercased) → the account that last read it: 0 is `first`, + /// `i` is `others[i - 1]`. + by_owner: Mutex>, +} + +impl FallbackTransport { + pub fn new(first: Arc, load_others: LoadOthers) -> FallbackTransport { + FallbackTransport { + first, + load: Mutex::new(Some(load_others)), + others: OnceLock::new(), + by_owner: Mutex::new(HashMap::new()), + } + } + + fn others(&self) -> &[Account] { + self.others.get_or_init(|| { + let load = self.load.lock().unwrap_or_else(|e| e.into_inner()).take(); + load.map(|f| f()).unwrap_or_default() + }) + } + + fn account(&self, i: usize) -> Option<&dyn Transport> { + match i { + 0 => Some(&*self.first), + i => self.others().get(i - 1).map(|a| &*a.transport), + } + } + + fn call( + &self, + path: &str, + send: impl Fn(&dyn Transport) -> Result, + ) -> Result { + let Some(owner) = owner_of(path) else { + return send(&*self.first); + }; + let remembered = self + .by_owner + .lock() + .unwrap_or_else(|e| e.into_inner()) + .get(&owner) + .copied(); + // A remembered index always exists — the list never shrinks — but + // falling back to the active account costs nothing. + let (start, account) = remembered + .and_then(|i| Some((i, self.account(i)?))) + .unwrap_or((0, &*self.first)); + let err = match send(account) { + Err(e) if another_account_might_help(&e) => e, + reply => return reply, + }; + for i in (0..=self.others().len()).filter(|&i| i != start) { + let Some(account) = self.account(i) else { + continue; + }; + if let Ok(reply) = send(account) { + let login = match i { + 0 => "the active gh account", + i => &self.others()[i - 1].login, + }; + log::info!("github: reading {owner}'s repositories as {login}"); + self.by_owner + .lock() + .unwrap_or_else(|e| e.into_inner()) + .insert(owner, i); + return Ok(reply); + } + } + // Nobody got through: say what the account that was asked first + // heard. + Err(err) + } +} + +impl Transport for FallbackTransport { + fn get(&self, path: &str) -> Result { + self.call(path, |t| t.get(path)) + } + + fn get_full(&self, path: &str) -> Result { + self.call(path, |t| t.get_full(path)) + } + + fn authenticated(&self) -> bool { + self.first.authenticated() + } +} + +/// Errors a different account's token could turn into an answer. A spent +/// rate limit, a network failure or a bad body would come out the same. +fn another_account_might_help(e: &ApiError) -> bool { + matches!( + e, + ApiError::NotFound | ApiError::Unauthorized | ApiError::Forbidden(_) + ) +} + +/// The owner in `/repos/{owner}/…`, lowercased — GitHub logins are +/// case-insensitive. +fn owner_of(path: &str) -> Option { + let owner = path.strip_prefix("/repos/")?.split(['/', '?']).next()?; + (!owner.is_empty()).then(|| owner.to_ascii_lowercase()) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::core::github::api::tests::Fixture; + use std::sync::atomic::{AtomicUsize, Ordering}; + + const PATH: &str = "/repos/Work/secret/issues?page=1"; + const OTHER_REPO: &str = "/repos/work/other/issues?page=1"; + const MINE: &str = "/repos/me/public/issues?page=1"; + + fn fixture(paths: &[&str]) -> Arc { + let mut f = Fixture::new(); + for p in paths { + f.on(p, "[]", false); + } + Arc::new(f) + } + + struct Setup { + t: FallbackTransport, + first: Arc, + work: Arc, + loads: Arc, + } + + /// `first` sees only `MINE`; the second account, "work", sees the work + /// organisation's repositories. + fn setup() -> Setup { + let first = fixture(&[MINE]); + let work = fixture(&[PATH, OTHER_REPO]); + let loads = Arc::new(AtomicUsize::new(0)); + let (w, l) = (work.clone(), loads.clone()); + let t = FallbackTransport::new( + first.clone(), + Box::new(move || { + l.fetch_add(1, Ordering::SeqCst); + vec![ + Account { + login: "nobody".into(), + transport: fixture(&[]), + }, + Account { + login: "work".into(), + transport: w, + }, + ] + }), + ); + Setup { + t, + first, + work, + loads, + } + } + + fn asked(f: &Fixture) -> usize { + f.asked.lock().unwrap().len() + } + + #[test] + fn what_the_active_account_can_read_never_lists_the_others() { + let s = setup(); + assert!(s.t.get(MINE).is_ok()); + assert_eq!(s.loads.load(Ordering::SeqCst), 0); + assert_eq!(asked(&s.work), 0); + } + + #[test] + fn a_404_is_retried_with_the_other_accounts_until_one_reads_it() { + let s = setup(); + assert!(s.t.get(PATH).is_ok()); + assert_eq!(asked(&s.first), 1); + assert_eq!(asked(&s.work), 1); + } + + #[test] + fn the_account_that_got_through_is_asked_first_for_that_owner_next_time() { + let s = setup(); + s.t.get(PATH).unwrap(); + // Another repository of the same owner, spelled in another case: + // straight to "work", the active account is not bothered again. + s.t.get_full(OTHER_REPO).unwrap(); + assert_eq!(asked(&s.first), 1); + assert_eq!(asked(&s.work), 2); + // Other owners still start from the active account. + s.t.get(MINE).unwrap(); + assert_eq!(asked(&s.first), 2); + assert_eq!(s.loads.load(Ordering::SeqCst), 1, "listed once"); + } + + #[test] + fn a_remembered_account_that_cannot_read_a_repository_falls_back_again() { + const SHARED: &str = "/repos/work/shared/issues"; + let first = fixture(&[SHARED]); + let work = fixture(&[PATH]); + let w = work.clone(); + let t = FallbackTransport::new( + first.clone(), + Box::new(move || { + vec![Account { + login: "work".into(), + transport: w, + }] + }), + ); + t.get(PATH).unwrap(); + // Remembered: "work". This one only the active account sees. + t.get(SHARED).unwrap(); + assert_eq!(asked(&work), 2); + assert_eq!(asked(&first), 2); + // And now the active account is the one remembered. + t.get(SHARED).unwrap(); + assert_eq!(asked(&work), 2); + } + + #[test] + fn when_nobody_can_read_it_the_first_error_is_reported() { + let s = setup(); + assert_eq!(s.t.get("/repos/ghost/x").err(), Some(ApiError::NotFound)); + // Everyone was asked, once. + assert_eq!(asked(&s.first), 1); + assert_eq!(asked(&s.work), 1); + } + + #[test] + fn a_rate_limit_is_not_retried_as_someone_else() { + let mut first = Fixture::new(); + first + .replies + .insert(PATH.into(), Err(ApiError::RateLimited { reset: Some(9) })); + let loads = Arc::new(AtomicUsize::new(0)); + let l = loads.clone(); + let t = FallbackTransport::new( + Arc::new(first), + Box::new(move || { + l.fetch_add(1, Ordering::SeqCst); + Vec::new() + }), + ); + assert_eq!( + t.get(PATH).err(), + Some(ApiError::RateLimited { reset: Some(9) }) + ); + assert_eq!(loads.load(Ordering::SeqCst), 0); + } + + #[test] + fn owners_come_from_repo_paths_only() { + assert_eq!( + owner_of("/repos/L0ng-AI/tty7/pulls"), + Some("l0ng-ai".into()) + ); + assert_eq!(owner_of("/repos/x?y"), Some("x".into())); + assert_eq!(owner_of("/user"), None); + assert_eq!(owner_of("/repos//x"), None); + } +} diff --git a/crates/tty7-core/src/core/github/mod.rs b/crates/tty7-core/src/core/github/mod.rs index 9062eeb6..fea56f6a 100644 --- a/crates/tty7-core/src/core/github/mod.rs +++ b/crates/tty7-core/src/core/github/mod.rs @@ -6,6 +6,7 @@ //! behind the `github` feature, which only the GUI turns on — the headless //! server never talks to GitHub. +pub mod accounts; pub mod api; #[cfg(feature = "github")] pub mod http; diff --git a/crates/tty7-core/src/core/github/token.rs b/crates/tty7-core/src/core/github/token.rs index 963be814..d8057346 100644 --- a/crates/tty7-core/src/core/github/token.rs +++ b/crates/tty7-core/src/core/github/token.rs @@ -7,6 +7,11 @@ //! a CI-style setup behaves the same here as in the shell; //! 2. `gh auth token --hostname github.com`. //! +//! When the answer came from `gh`, the CLI's *other* github.com accounts are +//! on offer too ([`other_gh_accounts`]): a repository the active account +//! cannot see is retried with them (`super::accounts`). A token from the +//! environment is taken as a deliberate choice and gets no fallback. +//! //! No token is not an error. Public repositories read fine without one, at //! GitHub's unauthenticated rate (60 requests an hour). //! @@ -67,8 +72,8 @@ pub const GH_FALLBACKS: [&str; 3] = [ pub struct TokenEnv<'a> { pub var: &'a dyn Fn(&str) -> Option, pub is_file: &'a dyn Fn(&Path) -> bool, - /// Run ` auth token …` and hand back its stdout on success. - pub gh_token: &'a dyn Fn(&Path) -> Option, + /// Run ` ` and hand back its stdout on success. + pub gh: &'a dyn Fn(&Path, &[&str]) -> Option, } /// Resolve a token, or `None` to go unauthenticated. @@ -78,17 +83,87 @@ pub fn resolve_token(env: &TokenEnv<'_>) -> Option<(Token, TokenSource)> { return Some((token, TokenSource::Env(var))); } } - // Only the first `gh` found is asked. A second install answering for a - // different account than the one on PATH would be a surprise, not a - // fallback. - let gh = gh_candidates((env.var)("PATH").as_deref()) + let gh = find_gh(env)?; + gh_token(env, &gh, None).map(|t| (t, TokenSource::GhCli)) +} + +/// The `gh` to ask. Only the first one found: a second install answering for +/// a different account than the one on PATH would be a surprise, not a +/// fallback. +pub fn find_gh(env: &TokenEnv<'_>) -> Option { + gh_candidates((env.var)("PATH").as_deref()) .into_iter() - .find(|p| (env.is_file)(p))?; - let out = (env.gh_token)(&gh)?; + .find(|p| (env.is_file)(p)) +} + +/// `gh auth token` for github.com — the active account's, or `user`'s. +fn gh_token(env: &TokenEnv<'_>, gh: &Path, user: Option<&str>) -> Option { + let mut args = vec!["auth", "token", "--hostname", "github.com"]; + if let Some(user) = user { + args.extend(["--user", user]); + } + let out = (env.gh)(gh, &args)?; // `gh auth token` prints the token and a newline; anything multi-line is // not a token. - let line = out.lines().next()?; - Token::new(line).map(|t| (t, TokenSource::GhCli)) + Token::new(out.lines().next()?) +} + +/// Every github.com account `gh` is signed in to besides the active one, by +/// login, in the order `gh` lists them. Accounts whose sign-in `gh` itself +/// reports as broken are left out. +/// +/// Slow — `gh auth status` checks each account against GitHub — so it is +/// only asked once the active account has come up short. +pub fn other_gh_accounts(env: &TokenEnv<'_>) -> Vec<(String, Token)> { + let Some(gh) = find_gh(env) else { + return Vec::new(); + }; + let Some(status) = (env.gh)( + &gh, + &[ + "auth", + "status", + "--hostname", + "github.com", + "--json", + "hosts", + ], + ) else { + return Vec::new(); + }; + inactive_logins(&status) + .into_iter() + .filter_map(|login| gh_token(env, &gh, Some(&login)).map(|t| (login, t))) + .collect() +} + +/// The signed-in, inactive github.com logins in `gh auth status --json hosts`. +fn inactive_logins(status: &str) -> Vec { + #[derive(serde::Deserialize)] + struct Status { + #[serde(default)] + hosts: std::collections::HashMap>, + } + #[derive(serde::Deserialize)] + struct Account { + #[serde(default)] + state: String, + #[serde(default)] + active: bool, + #[serde(default)] + login: String, + } + let Ok(status) = serde_json::from_str::(status) else { + return Vec::new(); + }; + status + .hosts + .get("github.com") + .into_iter() + .flatten() + .filter(|a| !a.active && a.state == "success" && !a.login.is_empty()) + .map(|a| a.login.clone()) + .collect() } /// Every place `gh` might be, `PATH` first, without duplicates. @@ -116,25 +191,44 @@ pub fn gh_candidates(path_var: Option<&str>) -> Vec { /// wait on that forever. const GH_TIMEOUT: Duration = Duration::from_secs(5); +/// `gh auth status` asks GitHub about every account, so it gets a network +/// round trip's worth more. +const GH_STATUS_TIMEOUT: Duration = Duration::from_secs(20); + +fn system_env<'a>() -> TokenEnv<'a> { + TokenEnv { + var: &|name| std::env::var(name).ok(), + is_file: &|p| p.is_file(), + gh: &run_gh, + } +} + /// [`resolve_token`] against the real process environment. Blocking: call it /// off the UI thread. pub fn resolve_token_from_system() -> Option<(Token, TokenSource)> { - resolve_token(&TokenEnv { - var: &|name| std::env::var(name).ok(), - is_file: &|p| p.is_file(), - gh_token: &run_gh_auth_token, - }) + resolve_token(&system_env()) } -fn run_gh_auth_token(gh: &Path) -> Option { +/// [`other_gh_accounts`] against the real `gh`. Blocking: call it off the UI +/// thread. +pub fn other_gh_accounts_from_system() -> Vec<(String, Token)> { + other_gh_accounts(&system_env()) +} + +fn run_gh(gh: &Path, args: &[&str]) -> Option { use crate::core::proc::{hide_console, output_within}; let mut cmd = std::process::Command::new(gh); - cmd.args(["auth", "token", "--hostname", "github.com"]) + cmd.args(args) .env("GH_PROMPT_DISABLED", "1") .env("GH_NO_UPDATE_NOTIFIER", "1"); + let timeout = if args.get(1) == Some(&"status") { + GH_STATUS_TIMEOUT + } else { + GH_TIMEOUT + }; // A GUI app spawning a console program on Windows flashes a console // window unless told not to. - let out = output_within(hide_console(&mut cmd), GH_TIMEOUT).ok()?; + let out = output_within(hide_console(&mut cmd), timeout).ok()?; if !out.status.success() { return None; } @@ -151,7 +245,10 @@ mod tests { vars: HashMap<&'static str, &'static str>, files: Vec, gh_out: Option<&'static str>, + status_out: Option<&'static str>, + per_user: HashMap<&'static str, &'static str>, asked: RefCell>, + args: RefCell>, } impl Fake { @@ -160,20 +257,32 @@ mod tests { vars: HashMap::new(), files: Vec::new(), gh_out: None, + status_out: None, + per_user: HashMap::new(), asked: RefCell::new(Vec::new()), + args: RefCell::new(Vec::new()), } } - fn resolve(&self) -> Option<(Token, TokenSource)> { - resolve_token(&TokenEnv { + fn with_env(&self, f: impl FnOnce(&TokenEnv<'_>) -> T) -> T { + f(&TokenEnv { var: &|name| self.vars.get(name).map(|v| v.to_string()), is_file: &|p| self.files.iter().any(|f| f == p), - gh_token: &|p| { + gh: &|p, args| { self.asked.borrow_mut().push(p.to_path_buf()); - self.gh_out.map(str::to_string) + self.args.borrow_mut().push(args.join(" ")); + match args { + ["auth", "status", ..] => self.status_out.map(str::to_string), + [.., "--user", user] => self.per_user.get(user).map(|t| t.to_string()), + _ => self.gh_out.map(str::to_string), + } }, }) } + + fn resolve(&self) -> Option<(Token, TokenSource)> { + self.with_env(resolve_token) + } } #[test] @@ -274,6 +383,62 @@ mod tests { assert_eq!(unique.len(), c.len()); } + const STATUS: &str = r#"{"hosts":{"github.com":[ + {"state":"success","active":true,"host":"github.com","login":"me"}, + {"state":"success","active":false,"host":"github.com","login":"work"}, + {"state":"error","active":false,"host":"github.com","login":"expired"}, + {"state":"success","active":false,"host":"github.com","login":"side"} + ],"ghe.example.com":[ + {"state":"success","active":false,"host":"ghe.example.com","login":"elsewhere"} + ]}}"#; + + #[cfg(unix)] + #[test] + fn other_accounts_are_the_inactive_signed_in_github_com_ones() { + let mut f = Fake::new(); + f.files.push(PathBuf::from("/usr/local/bin/gh")); + f.status_out = Some(STATUS); + f.per_user.insert("work", "gho_work\n"); + f.per_user.insert("side", "gho_side\n"); + let got: Vec<(String, String)> = f + .with_env(other_gh_accounts) + .into_iter() + .map(|(login, t)| (login, t.expose().to_string())) + .collect(); + assert_eq!( + got, + vec![ + ("work".to_string(), "gho_work".to_string()), + ("side".to_string(), "gho_side".to_string()), + ] + ); + assert!( + f.args + .borrow() + .contains(&"auth token --hostname github.com --user work".to_string()) + ); + } + + #[cfg(unix)] + #[test] + fn an_account_gh_has_no_token_for_is_skipped() { + let mut f = Fake::new(); + f.files.push(PathBuf::from("/usr/local/bin/gh")); + f.status_out = Some(STATUS); + f.per_user.insert("side", "gho_side\n"); + let got = f.with_env(other_gh_accounts); + assert_eq!(got.len(), 1); + assert_eq!(got[0].0, "side"); + } + + #[test] + fn no_gh_or_an_unreadable_status_is_no_other_accounts() { + let f = Fake::new(); + assert!(f.with_env(other_gh_accounts).is_empty()); + assert!(inactive_logins("gh: unknown flag --json").is_empty()); + assert!(inactive_logins(r#"{"hosts":{}}"#).is_empty()); + } + #[test] fn a_token_never_prints_itself() { let token = Token::new("ghp_secretsecret").unwrap(); diff --git a/src/ui/github/mod.rs b/src/ui/github/mod.rs index 42eaa831..8ce8cc00 100644 --- a/src/ui/github/mod.rs +++ b/src/ui/github/mod.rs @@ -10,7 +10,9 @@ //! through the `Host` — the tree can be on another machine. //! - **Which token.** Resolved once, on a worker, from the environment or the //! GitHub CLI (`tty7_core::core::github::token`). Refresh resolves it again, -//! so a `gh auth login` in a pane takes effect without a restart. +//! so a `gh auth login` in a pane takes effect without a restart. A +//! repository the active `gh` account cannot see is retried with the +//! CLI's other accounts (`tty7_core::core::github::accounts`). //! - **The requests.** Made from *this* machine, never the remote host, on a //! dedicated thread each — a slow link must never hold a UI frame. //! @@ -141,18 +143,37 @@ where } /// The real connector: resolve a token (env, then `gh`), build the client. +/// A token from `gh` comes with the CLI's other accounts behind it, listed +/// only if the active one comes up short. fn system_connector(proxy: Option) -> Connector { + use tty7_core::core::github::accounts::{Account, FallbackTransport}; + use tty7_core::core::github::http::HttpTransport; + use tty7_core::core::github::token::{self, TokenSource}; Arc::new(move || { - let token = - tty7_core::core::github::token::resolve_token_from_system().map(|(t, source)| { - log::info!("github: signed in via {source:?}"); - t - }); + let resolved = token::resolve_token_from_system(); + if let Some((_, source)) = &resolved { + log::info!("github: signed in via {source:?}"); + } + let from_gh = matches!(resolved, Some((_, TokenSource::GhCli))); + let first: Arc = Arc::new(HttpTransport::new( + resolved.map(|(t, _)| t), + proxy.as_deref(), + )); + if !from_gh { + return Connection { transport: first }; + } + let proxy = proxy.clone(); + let load = Box::new(move || { + token::other_gh_accounts_from_system() + .into_iter() + .map(|(login, t)| Account { + login, + transport: Arc::new(HttpTransport::new(Some(t), proxy.as_deref())), + }) + .collect() + }); Connection { - transport: Arc::new(tty7_core::core::github::http::HttpTransport::new( - token, - proxy.as_deref(), - )), + transport: Arc::new(FallbackTransport::new(first, load)), } }) } diff --git a/src/ui/i18n/en.rs b/src/ui/i18n/en.rs index f37eda45..0539995a 100644 --- a/src/ui/i18n/en.rs +++ b/src/ui/i18n/en.rs @@ -2078,7 +2078,7 @@ pub fn translate_en(key: L10nKey) -> &'static str { "GitHub did not find this repository. If it is private, sign in first." } L10nKey::GitHubNotFoundSignedIn => { - "GitHub did not find this repository, or this account cannot see it." + "GitHub did not find this repository, or none of your gh accounts can see it." } L10nKey::GitHubUnauthorized => "GitHub rejected the saved sign-in.", L10nKey::GitHubRateLimited => "GitHub's rate limit is used up.", diff --git a/src/ui/i18n/ja.rs b/src/ui/i18n/ja.rs index be8e5db9..f477f18a 100644 --- a/src/ui/i18n/ja.rs +++ b/src/ui/i18n/ja.rs @@ -2135,7 +2135,7 @@ pub fn translate_ja(key: L10nKey) -> Option<&'static str> { "GitHub でこのリポジトリが見つかりません。非公開の場合は先にサインインしてください。" } L10nKey::GitHubNotFoundSignedIn => { - "GitHub でこのリポジトリが見つからないか、このアカウントでは閲覧できません。" + "GitHub でこのリポジトリが見つからないか、gh でサインイン中のどのアカウントでも閲覧できません。" } L10nKey::GitHubUnauthorized => "GitHub が保存済みのサインイン情報を拒否しました。", L10nKey::GitHubRateLimited => "GitHub のレート制限に達しました。", diff --git a/src/ui/i18n/zh.rs b/src/ui/i18n/zh.rs index 34571868..afc23def 100644 --- a/src/ui/i18n/zh.rs +++ b/src/ui/i18n/zh.rs @@ -1936,7 +1936,7 @@ pub fn translate_zh(key: L10nKey) -> Option<&'static str> { L10nKey::GitHubNoPulls => "没有匹配的拉取请求。", L10nKey::GitHubSignInHint => "在终端中运行 `gh auth login` 登录,然后刷新。", L10nKey::GitHubNotFoundSignedOut => "GitHub 找不到此仓库。如果它是私有仓库,请先登录。", - L10nKey::GitHubNotFoundSignedIn => "GitHub 找不到此仓库,或当前账户无权查看。", + L10nKey::GitHubNotFoundSignedIn => "GitHub 找不到此仓库,或 gh 登录的账户都无权查看。", L10nKey::GitHubUnauthorized => "GitHub 拒绝了已保存的登录凭据。", L10nKey::GitHubRateLimited => "GitHub 的请求额度已用完。", L10nKey::GitHubRateLimitResetIn => "{n} 分钟后重置。",