mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-08-28 05:27:41 +00:00
refactor: simplify token plumbing and parallelize read_many_files
- Replace mint_github_token's hand-rolled Pat/Installation/App match with GitHubCredentials::resolve_bearer_token, removing a near-duplicate of the same logic already in run_metadata::mint_token. - Parallelize read_many_files via futures::future::join_all so the tool actually reads concurrently — previously serial despite the name. - Replace .expect() on the post-refresh GitHubTokenSource cache with a proper anyhow error so a refresh edge case can't panic. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
f1c247bc0f
commit
5a9f568e46
3 changed files with 35 additions and 42 deletions
|
|
@ -6,6 +6,7 @@ use fabro_llm::client::Client;
|
|||
use fabro_llm::types::{Message, Request, ToolDefinition};
|
||||
use fabro_model::ModelHandle;
|
||||
use fabro_static::EnvVars;
|
||||
use futures::future::join_all;
|
||||
|
||||
use crate::config::SessionOptions;
|
||||
use crate::sandbox::GrepOptions;
|
||||
|
|
@ -393,16 +394,25 @@ pub(crate) fn make_read_many_files_tool() -> RegisteredTool {
|
|||
},
|
||||
executor: Arc::new(|args, ctx| {
|
||||
Box::pin(async move {
|
||||
let paths = args["paths"]
|
||||
let paths: Vec<&str> = args["paths"]
|
||||
.as_array()
|
||||
.ok_or_else(|| "paths must be an array".to_string())?;
|
||||
.ok_or_else(|| "paths must be an array".to_string())?
|
||||
.iter()
|
||||
.map(|p| {
|
||||
p.as_str()
|
||||
.ok_or_else(|| "each path must be a string".to_string())
|
||||
})
|
||||
.collect::<Result<_, _>>()?;
|
||||
|
||||
let reads = paths.iter().map(|path| {
|
||||
let env = Arc::clone(&ctx.env);
|
||||
async move { (*path, env.read_file(path, None, None).await) }
|
||||
});
|
||||
let results = join_all(reads).await;
|
||||
|
||||
let mut output = String::new();
|
||||
for path_val in paths {
|
||||
let path = path_val
|
||||
.as_str()
|
||||
.ok_or_else(|| "each path must be a string".to_string())?;
|
||||
match ctx.env.read_file(path, None, None).await {
|
||||
for (path, result) in results {
|
||||
match result {
|
||||
Ok(content) => {
|
||||
ctx.env.mark_agent_read(path);
|
||||
let _ = write!(output, "=== {path} ===\n{content}\n\n");
|
||||
|
|
|
|||
|
|
@ -1234,33 +1234,19 @@ async fn mint_github_token(
|
|||
origin_url: &str,
|
||||
permissions: &HashMap<String, String>,
|
||||
) -> Result<String> {
|
||||
match creds {
|
||||
fabro_github::GitHubCredentials::Pat(token) => return Ok(token.clone()),
|
||||
fabro_github::GitHubCredentials::Installation(token) => {
|
||||
return token.valid_token().map(str::to_owned);
|
||||
}
|
||||
fabro_github::GitHubCredentials::App(_) => {}
|
||||
}
|
||||
|
||||
let https_url = fabro_github::ssh_url_to_https(origin_url);
|
||||
let (owner, repo) = fabro_github::parse_github_owner_repo(&https_url)?;
|
||||
let fabro_github::GitHubCredentials::App(creds) = creds else {
|
||||
unreachable!("token credentials return early");
|
||||
};
|
||||
let jwt = fabro_github::sign_app_jwt(&creds.app_id, &creds.private_key_pem)?;
|
||||
let client = fabro_http::http_client()?;
|
||||
let perms_json = serde_json::to_value(permissions)?;
|
||||
let install_url = creds.installation_url(&owner);
|
||||
fabro_github::create_installation_access_token_with_permissions_and_install_url(
|
||||
&client,
|
||||
&jwt,
|
||||
&owner,
|
||||
&repo,
|
||||
&fabro_github::github_api_base_url(),
|
||||
perms_json,
|
||||
install_url.as_deref(),
|
||||
)
|
||||
.await
|
||||
creds
|
||||
.resolve_bearer_token(
|
||||
&client,
|
||||
&owner,
|
||||
&repo,
|
||||
&fabro_github::github_api_base_url(),
|
||||
perms_json,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
fn preflight_response(
|
||||
|
|
|
|||
|
|
@ -111,23 +111,21 @@ impl GitHubTokenSource {
|
|||
SourceState::StaticIat(token) => token.valid_token().map(str::to_owned),
|
||||
SourceState::Mintable { minter, cache } => {
|
||||
let mut cache = cache.lock().await;
|
||||
let should_refresh = cache
|
||||
let cached_is_fresh = cache
|
||||
.as_ref()
|
||||
.is_none_or(|token| token.near_expiry(REFRESH_THRESHOLD));
|
||||
.is_some_and(|token| !token.near_expiry(REFRESH_THRESHOLD));
|
||||
|
||||
if should_refresh {
|
||||
if !cached_is_fresh {
|
||||
match minter.mint().await {
|
||||
Ok(token) => {
|
||||
*cache = Some(token);
|
||||
}
|
||||
Ok(token) => *cache = Some(token),
|
||||
Err(err) => {
|
||||
if let Some(token) = cache.as_ref() {
|
||||
if token.valid_token().is_ok() {
|
||||
if let Ok(value) = token.valid_token() {
|
||||
warn!(
|
||||
error = %err,
|
||||
"GitHub installation token refresh failed; using cached token"
|
||||
);
|
||||
return token.valid_token().map(str::to_owned);
|
||||
return Ok(value.to_owned());
|
||||
}
|
||||
}
|
||||
return Err(err)
|
||||
|
|
@ -136,11 +134,10 @@ impl GitHubTokenSource {
|
|||
}
|
||||
}
|
||||
|
||||
cache
|
||||
let token = cache
|
||||
.as_ref()
|
||||
.expect("mintable token source should have a token after refresh")
|
||||
.valid_token()
|
||||
.map(str::to_owned)
|
||||
.ok_or_else(|| anyhow::anyhow!("mintable token source has no cached token"))?;
|
||||
token.valid_token().map(str::to_owned)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue