Context
PR #1512 adds MetadataTokenSource (keyless ADC) alongside the existing GoogleChatTokenCache (SA-key exchange) in crates/openab-gateway/src/adapters/googlechat.rs. The two get_token implementations now maintain near-identical double-checked-locking cache machinery (read-check, write-check, refresh-or-serve-stale-on-error, refresh_threshold clamping), differing only in what refresh() calls.
Review round 5 finding F29 (consolidated review on #1512): the copies had already diverged in robustness once (the request-timeout fix landed in one path's history but not the other's), and the PR's own follow-ups forecast keyless auth for other Google-API adapters - which would create a third copy.
Requested work
- Extract the shared cache/refresh skeleton into one helper (e.g. a
TokenSource trait or a generic cached-token struct parameterized by the refresh future), and re-base both GoogleChatTokenCache and MetadataTokenSource on it.
- While touching the config surface, decide whether
use_adc: bool should become an auth_mode enum before the shape ossifies (one reviewer lane recommended the enum; another judged the bool adequately minimal - decide once, here).
- Carry over the existing regression tests (serve-stale, TTL clamp, blank-token rejection, timeout) so both paths keep identical robustness guarantees.
Non-goals
- No behavior change to auth precedence (SA key > ADC > static token).
- No new auth modes.
Related non-blocking follow-ups recorded in the #1512 review history that could ride along: case-insensitive bool parsing for GOOGLE_CHAT_USE_ADC (x3 call sites), the untested env path, and an IAM-propagation retry note.
Context
PR #1512 adds
MetadataTokenSource(keyless ADC) alongside the existingGoogleChatTokenCache(SA-key exchange) incrates/openab-gateway/src/adapters/googlechat.rs. The twoget_tokenimplementations now maintain near-identical double-checked-locking cache machinery (read-check, write-check, refresh-or-serve-stale-on-error,refresh_thresholdclamping), differing only in whatrefresh()calls.Review round 5 finding F29 (consolidated review on #1512): the copies had already diverged in robustness once (the request-timeout fix landed in one path's history but not the other's), and the PR's own follow-ups forecast keyless auth for other Google-API adapters - which would create a third copy.
Requested work
TokenSourcetrait or a generic cached-token struct parameterized by the refresh future), and re-base bothGoogleChatTokenCacheandMetadataTokenSourceon it.use_adc: boolshould become anauth_modeenum before the shape ossifies (one reviewer lane recommended the enum; another judged the bool adequately minimal - decide once, here).Non-goals
Related non-blocking follow-ups recorded in the #1512 review history that could ride along: case-insensitive bool parsing for
GOOGLE_CHAT_USE_ADC(x3 call sites), the untested env path, and an IAM-propagation retry note.