Backport #106 to v1.x: bound dynamic-provider cache TTL + invalidation API (#105)#107
Backport #106 to v1.x: bound dynamic-provider cache TTL + invalidation API (#105)#107heskew wants to merge 1 commit into
Conversation
) The dynamic-provider cache (for onResolveProvider-resolved providers) was never evicted under the default config: cacheDynamicProviders defaulted to true (Infinity), and nothing — backing-store writes, config reloads, login/logout — cleared it. Disabling, deleting, or rotating credentials on a dynamically-resolved provider therefore had no effect until process restart, and a revoked SSO config kept authenticating through the session-validation middleware. Two changes: - Default cacheDynamicProviders to a bounded 300s TTL instead of forever, so every worker thread re-resolves within the window. `true` still opts into forever. This matches the cache's documented "bounded window" intent. - Add a public DynamicProviderCache.delete() and export invalidateDynamicProvider()/clearDynamicProviderCache() so a consumer can evict immediately when its backing config changes. The cache is per-worker-thread, so explicit invalidation reaches only the calling thread; the bounded TTL is the cross-thread convergence mechanism (documented). Adds delete() unit tests and corrects the default-TTL test (was asserting "forever"; now asserts the bounded default expires past the window). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request introduces module-level cache invalidation functions (invalidateDynamicProvider and clearDynamicProviderCache) and changes the default dynamic provider cache TTL from infinite to a bounded 300 seconds to prevent serving stale data. The review feedback suggests replacing the single global activeDynamicProviderCache variable with a Set of active caches. This improvement ensures that invalidation and clearing are correctly propagated across multiple active plugin instances running in the same thread, which is particularly useful in multi-tenant environments or during hot-reloads.
| // Active dynamic-provider cache for the loaded plugin instance, so consumers | ||
| // can invalidate entries when their backing config changes. Per-worker-thread: | ||
| // this references the cache in the thread that ran handleApplication. | ||
| let activeDynamicProviderCache: DynamicProviderCache | null = null; | ||
|
|
||
| /** | ||
| * Invalidate a single dynamically-resolved provider in the in-memory cache. | ||
| * Call this from application code after the backing config for `providerConfigId` | ||
| * changes (disabled, deleted, or credentials rotated) so the next request | ||
| * re-resolves it via the `onResolveProvider` hook instead of serving stale data. | ||
| * | ||
| * NOTE: the cache is per-worker-thread, so this evicts only in the thread that | ||
| * runs the call. Other threads converge within the configured TTL | ||
| * (`cacheDynamicProviders`, default {@link DEFAULT_DYNAMIC_PROVIDER_CACHE_TTL_SECONDS}s). | ||
| * For immediate cluster-wide effect, pair invalidation with a short TTL. | ||
| * | ||
| * @returns true if an entry was present and removed in this thread. | ||
| */ | ||
| export function invalidateDynamicProvider(providerConfigId: string): boolean { | ||
| return activeDynamicProviderCache?.delete(providerConfigId) ?? false; | ||
| } | ||
|
|
||
| /** | ||
| * Clear every dynamically-resolved provider from the in-memory cache (this | ||
| * worker thread only — see {@link invalidateDynamicProvider} for cross-thread notes). | ||
| */ | ||
| export function clearDynamicProviderCache(): void { | ||
| activeDynamicProviderCache?.clear(); | ||
| } |
There was a problem hiding this comment.
Using a single global variable activeDynamicProviderCache can lead to issues if multiple instances of the plugin are loaded in the same thread (e.g., in multi-tenant environments, multiple applications, or during hot-reloads where a new scope is initialized before the old one is fully closed). In such cases, the global variable only tracks the most recently initialized instance, and closing any instance could prematurely clear the reference or leave other active caches unreachable for invalidation.
Using a Set of active caches resolves this cleanly, ensuring that invalidation and clearing are correctly propagated to all active caches in the thread.
const activeDynamicProviderCaches = new Set<DynamicProviderCache>();
/**
* Invalidate a single dynamically-resolved provider in the in-memory cache.
* Call this from application code after the backing config for `providerConfigId`
* changes (disabled, deleted, or credentials rotated) so the next request
* re-resolves it via the `onResolveProvider` hook instead of serving stale data.
*
* NOTE: the cache is per-worker-thread, so this evicts only in the thread that
* runs the call. Other threads converge within the configured TTL
* (`cacheDynamicProviders`, default {@link DEFAULT_DYNAMIC_PROVIDER_CACHE_TTL_SECONDS}s).
* For immediate cluster-wide effect, pair invalidation with a short TTL.
*
* @returns true if an entry was present and removed in this thread.
*/
export function invalidateDynamicProvider(providerConfigId: string): boolean {
let anyDeleted = false;
for (const cache of activeDynamicProviderCaches) {
if (cache.delete(providerConfigId)) {
anyDeleted = true;
}
}
return anyDeleted;
}
/**
* Clear every dynamically-resolved provider from the in-memory cache (this
* worker thread only — see {@link invalidateDynamicProvider} for cross-thread notes).
*/
export function clearDynamicProviderCache(): void {
for (const cache of activeDynamicProviderCaches) {
cache.clear();
}
}| // Expose this thread's cache for consumer-driven invalidation | ||
| activeDynamicProviderCache = dynamicProviderCache; |
There was a problem hiding this comment.
Add the initialized cache to the set of active caches to support multiple active plugin instances in the same thread.
| // Expose this thread's cache for consumer-driven invalidation | |
| activeDynamicProviderCache = dynamicProviderCache; | |
| // Expose this thread's cache for consumer-driven invalidation | |
| activeDynamicProviderCaches.add(dynamicProviderCache); |
| if (activeDynamicProviderCache === dynamicProviderCache) { | ||
| activeDynamicProviderCache = null; | ||
| } |
|
1 blocker found. 1. Module-level cache reference loses track of active apps on closeFile: src/index.ts:318 In const activeCaches = new Set<DynamicProviderCache>();
export function invalidateDynamicProvider(providerConfigId: string): boolean {
let deleted = false;
for (const cache of activeCaches) {
if (cache.delete(providerConfigId)) deleted = true;
}
return deleted;
}
export function clearDynamicProviderCache(): void {
for (const cache of activeCaches) {
cache.clear();
}
}
// Inside handleApplication:
activeCaches.add(dynamicProviderCache);
// Inside scope.on('close'):
activeCaches.delete(dynamicProviderCache);This ensures that invalidation requests reach all active plugin instances in the worker and that closing one app doesn't break invalidation for others. (Note: the existing |
|
Reviewed; no blockers found. |
Backport of #106 to the
v1.xline (the 1.x release central-manager currently runs). Addresses #105.What
Cherry-pick of the #106 fix:
cacheDynamicProvidersto a bounded 300s TTL instead of forever, so disabled/deleted/credential-rotated dynamic providers stop being served without a process restart.DynamicProviderCache.delete()+ exportedinvalidateDynamicProvider()/clearDynamicProviderCache()for immediate consumer-driven eviction (per-worker-thread; TTL is the cross-thread convergence mechanism).Backport notes
The only conflict was the
index.tsimport block —mainbundles aregisterWellKnownHandlers(MCP) import that does not exist onv1.x; dropped it. Verified:dynamicProviderCache.tsdiff is byte-identical to themainchange.delete(),invalidateDynamicProvider/clearDynamicProviderCache, scope-close ownership guard); no MCP references.Same as #106: default goes from cache-forever → 300s.
cacheDynamicProviders: truestill opts into forever.Tests
delete()tests and the newindex.tswiring test (invalidate/clear/close-guard), both green on v1.x.Review
The code is the cross-model-reviewed change from #106 (thorough: codex + gemini + Harper adjudication — no blocker; the module-level singleton flag was adjudicated NOISE under Harper's per-thread model). The backport delta is solely the dropped MCP import, verified equivalent above.
Follow-up
central-manager's
updateOrganizationSettingsshould callinvalidateDynamicProvider(configId)after writing/disabling/rotating an org OAuth config for immediate effect. Separate CM change.🤖 Generated with Claude Code