From b1d20fb38cd609070c14134e94704c16d2862045 Mon Sep 17 00:00:00 2001 From: "detail-app[bot]" <180357370+detail-app[bot]@users.noreply.github.com> Date: Sun, 6 Sep 2026 02:26:31 +0000 Subject: [PATCH 1/3] fix(git-auth): fall back to still-valid token on near-expiry refresh failure --- CHANGELOG.md | 3 + crates/pcb-diode-api/src/auth.rs | 5 +- crates/pcb-diode-api/src/git_auth.rs | 52 +++++++++-- crates/pcbc/tests/auth_git.rs | 135 ++++++++++++++++++++++++++- 4 files changed, 186 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cdd24b2c9..e0cef6f8f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,9 @@ and this project adheres to Semantic Versioning (https://semver.org/spec/v2.0.0. - Support opaque service-account client IDs and proper OAuth Basic encoding. - Allow off-page schematic placement and avoid existing text and graphics. +- Fall back to the still-valid access token in `pb auth git` when a transient + OAuth refresh failure lands in the final minutes of a token's life, instead + of aborting Git operations with a misleading re-login prompt. ## [0.4.49] - 2026-09-04 diff --git a/crates/pcb-diode-api/src/auth.rs b/crates/pcb-diode-api/src/auth.rs index 9f0e43d16..6044b623c 100644 --- a/crates/pcb-diode-api/src/auth.rs +++ b/crates/pcb-diode-api/src/auth.rs @@ -17,7 +17,8 @@ use crate::WorkspaceContext; mod service_account; -const NOT_AUTHENTICATED_MESSAGE: &str = "Not authenticated. Run `pcb auth login` to authenticate."; +pub(crate) const NOT_AUTHENTICATED_MESSAGE: &str = + "Not authenticated. Run `pcb auth login` to authenticate."; const DIODE_API_AUTH_NONE: &str = "none"; #[derive(Debug, Clone, Serialize, Deserialize)] @@ -118,7 +119,7 @@ fn load_auth(ctx: &WorkspaceContext) -> Result> { Ok(Some(auth)) } -fn load_tokens_with_context(ctx: &WorkspaceContext) -> Result> { +pub(crate) fn load_tokens_with_context(ctx: &WorkspaceContext) -> Result> { match load_auth(ctx)? { Some(StoredAuth::User(tokens)) => Ok(Some(tokens)), Some(StoredAuth::ServiceAccount(_)) => { diff --git a/crates/pcb-diode-api/src/git_auth.rs b/crates/pcb-diode-api/src/git_auth.rs index d1ef355ca..b8a0cd197 100644 --- a/crates/pcb-diode-api/src/git_auth.rs +++ b/crates/pcb-diode-api/src/git_auth.rs @@ -173,13 +173,53 @@ fn exchange_credential( .build() .context("Failed to create Git credential HTTP client")?; - let request = client - .post(url) - .json(&GitCredentialExchangeRequest { host, path }); + let build_request = || { + client + .post(&url) + .json(&GitCredentialExchangeRequest { host, path }) + }; - let response = crate::auth::apply_api_auth_with_context(ctx, request)? - .send() - .context("Failed to exchange Diode authentication for a Git credential")?; + // The exchange is a single POST bounded by the 30s client timeout above, but + // `apply_api_auth_with_context` proactively refreshes any token with less + // than 300s of life remaining (so longer-running callers keep a >=300s + // margin) and returns `NOT_AUTHENTICATED_MESSAGE` if that refresh fails -- + // even when the on-disk access token is still server-valid for this short + // request. Capture the stored token before the auth call (a concurrent + // write cannot then interpose) so that, only when the shared layer's + // refresh fails, we can retry the exchange with the still-valid bearer. + // The 30s predicate is load-bearing: it matches the exchange client's + // timeout, so the fallback never hands out a token the request could + // outlive. The shared auth layer is left unchanged for everyone else. + let fallback_tokens = crate::auth::load_tokens_with_context(ctx).ok().flatten(); + + let response = match crate::auth::apply_api_auth_with_context(ctx, build_request()) { + Ok(request) => request + .send() + .context("Failed to exchange Diode authentication for a Git credential")?, + Err(error) => { + if error.to_string() != crate::auth::NOT_AUTHENTICATED_MESSAGE { + return Err(error); + } + let Some(tokens) = fallback_tokens else { + return Err(error); + }; + // Re-evaluate the remaining lifetime *after* the refresh attempt so + // the predicate accounts for any time the refresh consumed (e.g. a + // 30s read timeout); a token that entered the 300s skew with little + // margin may now have less than the exchange needs. + let now = SystemTime::now() + .duration_since(UNIX_EPOCH) + .context("System clock is before the Unix epoch")? + .as_secs() as i64; + if tokens.expires_at - now <= 30 { + return Err(error); + } + build_request() + .bearer_auth(tokens.access_token) + .send() + .context("Failed to exchange Diode authentication for a Git credential")? + } + }; if !response.status().is_success() { bail!("Git credential exchange failed: {}", response.status()); diff --git a/crates/pcbc/tests/auth_git.rs b/crates/pcbc/tests/auth_git.rs index 47830017d..4b359af44 100644 --- a/crates/pcbc/tests/auth_git.rs +++ b/crates/pcbc/tests/auth_git.rs @@ -646,7 +646,7 @@ fn legacy_helper_defaults_to_the_commercial_diodehub_host() { #[test] fn store_and_erase_are_silent_without_exchanging_credentials() { - let context = TestContext::new("http://127.0.0.1:1".to_string()); + let context = TestContext::new("http://127.0.0.1".to_string()); for operation in ["store", "erase"] { let output = run_with_input( @@ -662,3 +662,136 @@ fn store_and_erase_are_silent_without_exchanging_credentials() { assert!(output.stderr.is_empty()); } } + +fn write_device_flow_token( + context: &TestContext, + api_url: &str, + access_token: &str, + remaining_seconds: i64, +) { + let now = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_secs() as i64; + let slug = auth_scope_slug(api_url); + let auth_file = context.config_dir.join("auth").join(format!("{slug}.toml")); + fs::write( + &auth_file, + format!( + "access_token = \"{access_token}\"\n\ + refresh_token = \"refresh-token\"\n\ + expires_at = {}\n\ + token_endpoint = \"{api_url}/oauth/token\"\n\ + client_id = \"test-client-id\"\n", + now + remaining_seconds, + ), + ) + .expect("write device-flow auth tokens"); +} + +fn git_get_helper(context: &TestContext) -> Command { + let mut command = context.pcbc(); + command.args(["auth", "git", "--host", GIT_API_HOST, "get"]); + command +} + +#[test] +fn near_expiry_token_with_refresh_failure_uses_the_still_valid_token() { + // The shared auth layer refreshes any token with < 300s of remaining life + // and returns "Not authenticated" if that refresh fails, even though the + // on-disk access token is still server-valid for the 30s-timeout exchange. + // `exchange_credential` must fall back to that still-valid bearer instead + // of aborting the helper with `quit=true`. + let server = MockServer::start(); + let api_url = server.base_url(); + let refresh = server.mock(|when, then| { + when.method(POST).path("/oauth/token"); + then.status(503); + }); + let exchange = mock_exchange(&server, GIT_API_HOST, 200, Some("Bearer still-valid-token")); + let context = TestContext::new(api_url.clone()); + write_device_flow_token(&context, &api_url, "still-valid-token", 200); + + let fill = run_with_input(git_get_helper(&context), &credential_request()); + assert_success(&fill); + assert!(fill.stderr.is_empty()); + + let stdout = String::from_utf8_lossy(&fill.stdout); + assert!(!stdout.contains("quit=true")); + assert!(stdout.contains("authtype=Bearer")); + assert!(stdout.contains(&format!("credential={REPOSITORY_TOKEN}"))); + assert!(stdout.contains(&format!( + "password_expiry_utc={REPOSITORY_TOKEN_EXPIRES_AT}" + ))); + + refresh.assert_calls(1); + exchange.assert_calls(1); +} + +#[test] +fn almost_expired_token_with_refresh_failure_does_not_fall_back() { + // When the still-valid token has less than the 30s exchange client + // timeout of remaining life, the fallback must decline so the request + // cannot outlive the token. The original "Not authenticated" error then + // propagates with `quit=true` exactly as before the fix. + let server = MockServer::start(); + let api_url = server.base_url(); + let refresh = server.mock(|when, then| { + when.method(POST).path("/oauth/token"); + then.status(503); + }); + let exchange = mock_exchange( + &server, + GIT_API_HOST, + 200, + Some("Bearer almost-expired-token"), + ); + let context = TestContext::new(api_url.clone()); + write_device_flow_token(&context, &api_url, "almost-expired-token", 10); + + let fill = run_with_input(git_get_helper(&context), &credential_request()); + + // `pb auth git get` always exits 0; on failure it emits `quit=true` to + // stdout and the error to stderr (git translates `quit=true` into a + // non-zero exit when invoked via `git credential fill`). + let stdout = String::from_utf8_lossy(&fill.stdout); + let stderr = String::from_utf8_lossy(&fill.stderr); + assert!(stdout.contains("quit=true")); + assert!(stderr.contains("Not authenticated")); + assert!(!stderr.contains("Git credential exchange failed")); + assert!(!stdout.contains("credential=")); + + refresh.assert_calls(1); + exchange.assert_calls(0); +} + +#[test] +fn near_expiry_token_with_refresh_success_uses_refreshed_token() { + // When the proactive refresh succeeds, the refreshed bearer must be used + // for the exchange and the still-valid-token fallback must not fire with + // the stale on-disk bearer. + let server = MockServer::start(); + let api_url = server.base_url(); + let refresh = server.mock(|when, then| { + when.method(POST).path("/oauth/token"); + then.status(200).json_body(json!({ + "access_token": "refreshed-token", + "refresh_token": "new-refresh-token", + "expires_in": 3600, + })); + }); + let exchange = mock_exchange(&server, GIT_API_HOST, 200, Some("Bearer refreshed-token")); + let context = TestContext::new(api_url.clone()); + write_device_flow_token(&context, &api_url, "still-valid-token", 200); + + let fill = run_with_input(git_get_helper(&context), &credential_request()); + assert_success(&fill); + assert!(fill.stderr.is_empty()); + + let stdout = String::from_utf8_lossy(&fill.stdout); + assert!(!stdout.contains("quit=true")); + assert!(stdout.contains(&format!("credential={REPOSITORY_TOKEN}"))); + + refresh.assert_calls(1); + exchange.assert_calls(1); +} From 7328f15c24d37b86fd9598d2c554da4a689f376f Mon Sep 17 00:00:00 2001 From: Akhil Velagapudi Date: Mon, 7 Sep 2026 04:27:04 +0000 Subject: [PATCH 2/3] Simplify Git token refresh fallback at the auth source Amp-Thread-ID: https://ampcode.com/threads/T-01a07a15-eb6d-7309-a05b-f00c2009a4c6 --- CHANGELOG.md | 4 +- crates/pcb-diode-api/src/auth.rs | 44 +++++-- crates/pcb-diode-api/src/git_auth.rs | 58 ++------- crates/pcbc/tests/auth_git.rs | 186 ++++++++------------------- 4 files changed, 98 insertions(+), 194 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e0cef6f8f..72c2e33d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ and this project adheres to Semantic Versioning (https://semver.org/spec/v2.0.0. - Keep inward decimation within the boundary deviation limit for spikes beyond chord endpoints. - Honor explicit `in_bom` overrides and inherit omitted values in KiCad `extends` symbols. - Import metric-only `0402Metric`/`0603Metric` passives as imperial `01005`/`0201` packages, ignoring library namespaces in package detection. +- Keep Git authentication working when token refresh fails but the access token remains valid for the credential exchange. ## [0.4.51] - 2026-09-05 @@ -57,9 +58,6 @@ and this project adheres to Semantic Versioning (https://semver.org/spec/v2.0.0. - Support opaque service-account client IDs and proper OAuth Basic encoding. - Allow off-page schematic placement and avoid existing text and graphics. -- Fall back to the still-valid access token in `pb auth git` when a transient - OAuth refresh failure lands in the final minutes of a token's life, instead - of aborting Git operations with a misleading re-login prompt. ## [0.4.49] - 2026-09-04 diff --git a/crates/pcb-diode-api/src/auth.rs b/crates/pcb-diode-api/src/auth.rs index 6044b623c..ef42789c1 100644 --- a/crates/pcb-diode-api/src/auth.rs +++ b/crates/pcb-diode-api/src/auth.rs @@ -17,8 +17,7 @@ use crate::WorkspaceContext; mod service_account; -pub(crate) const NOT_AUTHENTICATED_MESSAGE: &str = - "Not authenticated. Run `pcb auth login` to authenticate."; +const NOT_AUTHENTICATED_MESSAGE: &str = "Not authenticated. Run `pcb auth login` to authenticate."; const DIODE_API_AUTH_NONE: &str = "none"; #[derive(Debug, Clone, Serialize, Deserialize)] @@ -119,7 +118,7 @@ fn load_auth(ctx: &WorkspaceContext) -> Result> { Ok(Some(auth)) } -pub(crate) fn load_tokens_with_context(ctx: &WorkspaceContext) -> Result> { +fn load_tokens_with_context(ctx: &WorkspaceContext) -> Result> { match load_auth(ctx)? { Some(StoredAuth::User(tokens)) => Ok(Some(tokens)), Some(StoredAuth::ServiceAccount(_)) => { @@ -304,11 +303,12 @@ pub fn refresh_tokens() -> Result { } pub fn get_valid_token_with_context(ctx: &WorkspaceContext) -> Result { - get_valid_token_with_sources(ctx, refresh_tokens_with_context) + get_valid_token_with_sources(ctx, None, refresh_tokens_with_context) } fn get_valid_token_with_sources( ctx: &WorkspaceContext, + refresh_fallback_lifetime: Option, refresh_tokens: impl Fn(&WorkspaceContext) -> Result, ) -> Result { let not_authenticated = || anyhow::anyhow!(NOT_AUTHENTICATED_MESSAGE); @@ -331,7 +331,16 @@ fn get_valid_token_with_sources( match refresh_tokens(ctx) { Ok(new_tokens) => Ok(new_tokens.access_token), - Err(_) => Err(not_authenticated()), + Err(_) => { + // Check after refreshing: a slow failure may consume the remaining lifetime. + if let Some(lifetime) = refresh_fallback_lifetime { + let now = SystemTime::now().duration_since(UNIX_EPOCH)?.as_secs() as i64; + if tokens.expires_at - now > lifetime.as_secs() as i64 { + return Ok(tokens.access_token); + } + } + Err(not_authenticated()) + } } } @@ -353,6 +362,17 @@ pub fn get_api_token_with_context(ctx: &WorkspaceContext) -> Result Result> { + if api_auth_disabled() { + Ok(None) + } else { + get_valid_token_with_sources(ctx, Some(lifetime), refresh_tokens_with_context).map(Some) + } +} + pub fn get_api_token() -> Result> { let ctx = WorkspaceContext::from_cwd().unwrap_or_default(); get_api_token_with_context(&ctx) @@ -817,7 +837,7 @@ mod tests { let (_tempdir, _guard, ctx) = isolated_context(); let refresh_calls = Cell::new(0); - let err = get_valid_token_with_sources(&ctx, |_| { + let err = get_valid_token_with_sources(&ctx, None, |_| { refresh_calls.set(refresh_calls.get() + 1); anyhow::bail!("refresh should not be called") }) @@ -851,13 +871,13 @@ mod tests { #[test] #[serial] - fn expired_auth_file_refresh_failure_returns_not_authenticated() { + fn near_expiry_auth_file_refresh_failure_without_fallback_returns_not_authenticated() { let (_tempdir, _guard, ctx) = isolated_context(); save_tokens( &ctx, - "expired-token", + "still-valid-token", "refresh-token", - unix_now() - 3600, + unix_now() + 200, Some("user@example.com"), None, None, @@ -865,7 +885,7 @@ mod tests { .unwrap(); let refresh_calls = Cell::new(0); - let err = get_valid_token_with_sources(&ctx, |_| { + let err = get_valid_token_with_sources(&ctx, None, |_| { refresh_calls.set(refresh_calls.get() + 1); anyhow::bail!("refresh failed") }) @@ -891,7 +911,7 @@ mod tests { .unwrap(); let refresh_calls = Cell::new(0); - let token = get_valid_token_with_sources(&ctx, |_| { + let token = get_valid_token_with_sources(&ctx, None, |_| { refresh_calls.set(refresh_calls.get() + 1); Ok(AuthTokens { access_token: "refreshed-token".to_string(), @@ -924,7 +944,7 @@ mod tests { .unwrap(); let refresh_calls = Cell::new(0); - let token = get_valid_token_with_sources(&ctx, |_| { + let token = get_valid_token_with_sources(&ctx, None, |_| { refresh_calls.set(refresh_calls.get() + 1); anyhow::bail!("refresh should not be called") }) diff --git a/crates/pcb-diode-api/src/git_auth.rs b/crates/pcb-diode-api/src/git_auth.rs index b8a0cd197..f6f8f1abd 100644 --- a/crates/pcb-diode-api/src/git_auth.rs +++ b/crates/pcb-diode-api/src/git_auth.rs @@ -167,59 +167,21 @@ fn exchange_credential( path: &str, ) -> Result { let url = format!("{}/api/git/credentials", ctx.api_base_url()); + let timeout = Duration::from_secs(30); let client = Client::builder() .user_agent(format!("diode-pcb/{}", env!("CARGO_PKG_VERSION"))) - .timeout(Duration::from_secs(30)) + .timeout(timeout) .build() .context("Failed to create Git credential HTTP client")?; - let build_request = || { - client - .post(&url) - .json(&GitCredentialExchangeRequest { host, path }) - }; - - // The exchange is a single POST bounded by the 30s client timeout above, but - // `apply_api_auth_with_context` proactively refreshes any token with less - // than 300s of life remaining (so longer-running callers keep a >=300s - // margin) and returns `NOT_AUTHENTICATED_MESSAGE` if that refresh fails -- - // even when the on-disk access token is still server-valid for this short - // request. Capture the stored token before the auth call (a concurrent - // write cannot then interpose) so that, only when the shared layer's - // refresh fails, we can retry the exchange with the still-valid bearer. - // The 30s predicate is load-bearing: it matches the exchange client's - // timeout, so the fallback never hands out a token the request could - // outlive. The shared auth layer is left unchanged for everyone else. - let fallback_tokens = crate::auth::load_tokens_with_context(ctx).ok().flatten(); - - let response = match crate::auth::apply_api_auth_with_context(ctx, build_request()) { - Ok(request) => request - .send() - .context("Failed to exchange Diode authentication for a Git credential")?, - Err(error) => { - if error.to_string() != crate::auth::NOT_AUTHENTICATED_MESSAGE { - return Err(error); - } - let Some(tokens) = fallback_tokens else { - return Err(error); - }; - // Re-evaluate the remaining lifetime *after* the refresh attempt so - // the predicate accounts for any time the refresh consumed (e.g. a - // 30s read timeout); a token that entered the 300s skew with little - // margin may now have less than the exchange needs. - let now = SystemTime::now() - .duration_since(UNIX_EPOCH) - .context("System clock is before the Unix epoch")? - .as_secs() as i64; - if tokens.expires_at - now <= 30 { - return Err(error); - } - build_request() - .bearer_auth(tokens.access_token) - .send() - .context("Failed to exchange Diode authentication for a Git credential")? - } - }; + // Only this short exchange may reuse a token after refresh failure. + let token = crate::auth::get_api_token_with_refresh_fallback(ctx, timeout)?; + let request = client + .post(url) + .json(&GitCredentialExchangeRequest { host, path }); + let response = crate::auth::apply_bearer_auth(request, token.as_deref()) + .send() + .context("Failed to exchange Diode authentication for a Git credential")?; if !response.status().is_success() { bail!("Git credential exchange failed: {}", response.status()); diff --git a/crates/pcbc/tests/auth_git.rs b/crates/pcbc/tests/auth_git.rs index 4b359af44..05221ef16 100644 --- a/crates/pcbc/tests/auth_git.rs +++ b/crates/pcbc/tests/auth_git.rs @@ -646,7 +646,7 @@ fn legacy_helper_defaults_to_the_commercial_diodehub_host() { #[test] fn store_and_erase_are_silent_without_exchanging_credentials() { - let context = TestContext::new("http://127.0.0.1".to_string()); + let context = TestContext::new("http://127.0.0.1:1".to_string()); for operation in ["store", "erase"] { let output = run_with_input( @@ -663,135 +663,59 @@ fn store_and_erase_are_silent_without_exchanging_credentials() { } } -fn write_device_flow_token( - context: &TestContext, - api_url: &str, - access_token: &str, - remaining_seconds: i64, -) { - let now = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_secs() as i64; - let slug = auth_scope_slug(api_url); - let auth_file = context.config_dir.join("auth").join(format!("{slug}.toml")); - fs::write( - &auth_file, - format!( - "access_token = \"{access_token}\"\n\ - refresh_token = \"refresh-token\"\n\ - expires_at = {}\n\ - token_endpoint = \"{api_url}/oauth/token\"\n\ - client_id = \"test-client-id\"\n", - now + remaining_seconds, - ), - ) - .expect("write device-flow auth tokens"); -} - -fn git_get_helper(context: &TestContext) -> Command { - let mut command = context.pcbc(); - command.args(["auth", "git", "--host", GIT_API_HOST, "get"]); - command -} - -#[test] -fn near_expiry_token_with_refresh_failure_uses_the_still_valid_token() { - // The shared auth layer refreshes any token with < 300s of remaining life - // and returns "Not authenticated" if that refresh fails, even though the - // on-disk access token is still server-valid for the 30s-timeout exchange. - // `exchange_credential` must fall back to that still-valid bearer instead - // of aborting the helper with `quit=true`. - let server = MockServer::start(); - let api_url = server.base_url(); - let refresh = server.mock(|when, then| { - when.method(POST).path("/oauth/token"); - then.status(503); - }); - let exchange = mock_exchange(&server, GIT_API_HOST, 200, Some("Bearer still-valid-token")); - let context = TestContext::new(api_url.clone()); - write_device_flow_token(&context, &api_url, "still-valid-token", 200); - - let fill = run_with_input(git_get_helper(&context), &credential_request()); - assert_success(&fill); - assert!(fill.stderr.is_empty()); - - let stdout = String::from_utf8_lossy(&fill.stdout); - assert!(!stdout.contains("quit=true")); - assert!(stdout.contains("authtype=Bearer")); - assert!(stdout.contains(&format!("credential={REPOSITORY_TOKEN}"))); - assert!(stdout.contains(&format!( - "password_expiry_utc={REPOSITORY_TOKEN_EXPIRES_AT}" - ))); - - refresh.assert_calls(1); - exchange.assert_calls(1); -} - -#[test] -fn almost_expired_token_with_refresh_failure_does_not_fall_back() { - // When the still-valid token has less than the 30s exchange client - // timeout of remaining life, the fallback must decline so the request - // cannot outlive the token. The original "Not authenticated" error then - // propagates with `quit=true` exactly as before the fix. - let server = MockServer::start(); - let api_url = server.base_url(); - let refresh = server.mock(|when, then| { - when.method(POST).path("/oauth/token"); - then.status(503); - }); - let exchange = mock_exchange( - &server, - GIT_API_HOST, - 200, - Some("Bearer almost-expired-token"), - ); - let context = TestContext::new(api_url.clone()); - write_device_flow_token(&context, &api_url, "almost-expired-token", 10); - - let fill = run_with_input(git_get_helper(&context), &credential_request()); - - // `pb auth git get` always exits 0; on failure it emits `quit=true` to - // stdout and the error to stderr (git translates `quit=true` into a - // non-zero exit when invoked via `git credential fill`). - let stdout = String::from_utf8_lossy(&fill.stdout); - let stderr = String::from_utf8_lossy(&fill.stderr); - assert!(stdout.contains("quit=true")); - assert!(stderr.contains("Not authenticated")); - assert!(!stderr.contains("Git credential exchange failed")); - assert!(!stdout.contains("credential=")); - - refresh.assert_calls(1); - exchange.assert_calls(0); -} - #[test] -fn near_expiry_token_with_refresh_success_uses_refreshed_token() { - // When the proactive refresh succeeds, the refreshed bearer must be used - // for the exchange and the still-valid-token fallback must not fire with - // the stale on-disk bearer. - let server = MockServer::start(); - let api_url = server.base_url(); - let refresh = server.mock(|when, then| { - when.method(POST).path("/oauth/token"); - then.status(200).json_body(json!({ - "access_token": "refreshed-token", - "refresh_token": "new-refresh-token", - "expires_in": 3600, - })); - }); - let exchange = mock_exchange(&server, GIT_API_HOST, 200, Some("Bearer refreshed-token")); - let context = TestContext::new(api_url.clone()); - write_device_flow_token(&context, &api_url, "still-valid-token", 200); - - let fill = run_with_input(git_get_helper(&context), &credential_request()); - assert_success(&fill); - assert!(fill.stderr.is_empty()); - - let stdout = String::from_utf8_lossy(&fill.stdout); - assert!(!stdout.contains("quit=true")); - assert!(stdout.contains(&format!("credential={REPOSITORY_TOKEN}"))); - - refresh.assert_calls(1); - exchange.assert_calls(1); +fn near_expiry_token_refresh() { + for (remaining_seconds, refresh_status, bearer) in [ + (200, 503, Some("Bearer still-valid-token")), + (30, 503, None), + (200, 200, Some("Bearer refreshed-token")), + ] { + let server = MockServer::start(); + let api_url = server.base_url(); + let context = TestContext::new(api_url.clone()); + let now = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_secs(); + fs::write( + context + .config_dir + .join("auth") + .join(format!("{}.toml", auth_scope_slug(&api_url))), + format!( + "access_token = \"still-valid-token\"\n\ + refresh_token = \"refresh-token\"\n\ + expires_at = {}\n\ + token_endpoint = \"{api_url}/oauth/token\"\n\ + client_id = \"test-client-id\"\n", + now + remaining_seconds, + ), + ) + .unwrap(); + let refresh = server.mock(|when, then| { + when.method(POST).path("/oauth/token"); + then.status(refresh_status).json_body(json!({ + "access_token": "refreshed-token", + "refresh_token": "new-refresh-token", + "expires_in": 3600, + })); + }); + let exchange = mock_exchange(&server, GIT_API_HOST, 200, bearer); + let mut command = context.pcbc(); + command.args(["auth", "git", "--host", GIT_API_HOST, "get"]); + let fill = run_with_input(command, &credential_request()); + assert_success(&fill); + let stdout = String::from_utf8_lossy(&fill.stdout); + if bearer.is_some() { + assert!(fill.stderr.is_empty(), "{fill:?}"); + assert!(!stdout.contains("quit=true")); + assert!(stdout.contains(&format!("credential={REPOSITORY_TOKEN}"))); + } else { + assert!(stdout.contains("quit=true")); + assert!(String::from_utf8_lossy(&fill.stderr).contains("Not authenticated")); + assert!(!stdout.contains("credential=")); + } + refresh.assert_calls(1); + exchange.assert_calls(usize::from(bearer.is_some())); + } } From 1e86c223f567e1c91e4c31b61bd7052475726f32 Mon Sep 17 00:00:00 2001 From: Akhil Velagapudi Date: Mon, 7 Sep 2026 14:25:15 +0000 Subject: [PATCH 3/3] Remove Git auth fallback release note Amp-Thread-ID: https://ampcode.com/threads/T-01a07a15-eb6d-7309-a05b-f00c2009a4c6 --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 72c2e33d7..cdd24b2c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,7 +28,6 @@ and this project adheres to Semantic Versioning (https://semver.org/spec/v2.0.0. - Keep inward decimation within the boundary deviation limit for spikes beyond chord endpoints. - Honor explicit `in_bom` overrides and inherit omitted values in KiCad `extends` symbols. - Import metric-only `0402Metric`/`0603Metric` passives as imperial `01005`/`0201` packages, ignoring library namespaces in package detection. -- Keep Git authentication working when token refresh fails but the access token remains valid for the credential exchange. ## [0.4.51] - 2026-09-05