-
Notifications
You must be signed in to change notification settings - Fork 1
fix: make regions cache profile-aware by including API URL #544
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
fix: make regions cache profile-aware by including API URL #544
Conversation
The regions cache at ~/.config/agentuity/regions.json was not profile-aware, causing the wrong region to be used after switching profiles. This fix: 1. Adds apiUrl field to RegionsCacheData interface 2. Updates getCachedRegions to validate cached API URL matches current 3. Updates saveRegionsCache to include API URL in cache data 4. Updates fetchRegionsWithCache to pass API URL through 5. Updates all resolveRegion calls to pass the API URL When the cached API URL doesn't match the current profile's API URL, the cache is invalidated and regions are re-fetched from the correct API. Fixes #543 Co-Authored-By: Rick Blalock <rickblalock@mac.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
📝 WalkthroughWalkthroughThe changes introduce API URL awareness to region caching and resolution. The Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
📦 Canary Packages Publishedversion: PackagesInstallAdd to your Or install directly: CLI Executables
Run Canary CLI |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/cli/src/cli.ts (1)
1237-1245: Contract is correct; however,getAPIBaseURL(baseCtx.config)is recomputed 8 times across call sites with identical input.
Confirmed:getAPIBaseURLcorrectly acceptsConfig | null, andbaseCtx.configis the same stable value at each call site, so the returnedapiUrlstring will be identical. This matches the contract expected byresolveRegionand theAPIClientinstance.However, since
baseCtx.configdoes not change between calls, the same string is computed redundantly. Consider caching the result inbaseCtx(e.g.,baseCtx.apiUrl) to compute it once.
🧹 Nitpick comments (3)
packages/cli/src/cli.ts (3)
703-731: Harden cache parsing + normalizeapiUrlbefore comparison.
Right now, a corrupted/hand-edited cache can parse but still returndata.regionswith a bad shape (andtimestampcan yieldNaN), and URL comparisons can spuriously mismatch on trailing slashes.Proposed hardening (validation + normalization)
async function getCachedRegions(apiUrl: string, logger: Logger): Promise<RegionList | null> { try { + const normalizeApiUrl = (u: string) => u.replace(/\/+$/, ''); + const currentApiUrl = normalizeApiUrl(apiUrl); + const cachePath = join(getDefaultConfigDir(), REGIONS_CACHE_FILE); const file = Bun.file(cachePath); if (!(await file.exists())) { return null; } const data: RegionsCacheData = await file.json(); + + // Basic shape validation (handle corrupted/partial caches safely) + if ( + !data || + typeof data.timestamp !== 'number' || + !Array.isArray(data.regions) + ) { + logger.trace('regions cache has invalid shape; ignoring'); + return null; + } + // Check if cache is for the same API URL (profile-aware) - if (data.apiUrl && data.apiUrl !== apiUrl) { + const cachedApiUrl = data.apiUrl ? normalizeApiUrl(data.apiUrl) : undefined; + if (cachedApiUrl && cachedApiUrl !== currentApiUrl) { logger.trace( 'regions cache is for different API URL (cached: %s, current: %s)', - data.apiUrl, - apiUrl + cachedApiUrl, + currentApiUrl ); return null; } const age = Date.now() - data.timestamp; if (age > REGIONS_CACHE_MAX_AGE_MS) { logger.trace('regions cache expired (age: %dms)', age); return null; } logger.trace('using cached regions (age: %dms)', age); return data.regions; } catch (error) { logger.trace('failed to read regions cache: %s', error); return null; } }
733-752: Consider atomic writes for the cache file (avoid partial JSON on interruption).
Not a huge deal, but if the process is killed mid-write you can end up with unreadable JSON until TTL expires / next refresh.
754-773: EnsureapiUrlused for caching matches the actualapiClientbase URL.
Right nowfetchRegionsWithCache(apiUrl, apiClient, ...)will calllistRegions(apiClient)(source of truth) but key the cache off a separately-computedapiUrl; if those ever diverge (overrides/env/trailing slash), you’ll get confusing cache reuse/invalidations.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/cli/src/cli.ts
🧰 Additional context used
📓 Path-based instructions (2)
packages/cli/src/**/*.ts
📄 CodeRabbit inference engine (packages/cli/AGENTS.md)
Use
Bun.file(f).exists()instead ofexistsSync(f)for file existence checks
Files:
packages/cli/src/cli.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use Prettier formatter with tabs (width 3), single quotes, and semicolons for TypeScript files
Use TypeScript strict mode with ESNext target and bundler moduleResolution
UseStructuredErrorfrom@agentuity/corefor error handling
Files:
packages/cli/src/cli.ts
🧠 Learnings (1)
📚 Learning: 2025-12-21T00:31:41.858Z
Learnt from: jhaynie
Repo: agentuity/sdk PR: 274
File: packages/cli/src/cmd/build/vite/server-bundler.ts:12-41
Timestamp: 2025-12-21T00:31:41.858Z
Learning: In Bun runtime, BuildMessage and ResolveMessage are global types and are not exported from the bun module. Do not import { BuildMessage } from 'bun' or similar; these types are available globally and should be used without import. This applies to all TypeScript files that target the Bun runtime within the repository.
Applied to files:
packages/cli/src/cli.ts
🧬 Code graph analysis (1)
packages/cli/src/cli.ts (3)
packages/server/src/api/region/list.ts (2)
RegionList(14-14)listRegions(22-32)packages/cli/src/config.ts (1)
getDefaultConfigDir(27-29)packages/cli/src/api.ts (1)
getAPIBaseURL(86-89)
🔇 Additional comments (1)
packages/cli/src/cli.ts (1)
685-701: Good direction: profile-aware cache key is now explicit and backward-compatible.
AddingResolveRegionOptions.apiUrl+RegionsCacheData.apiUrl?cleanly scopes the cache without breaking existing cache files.
fix: make regions cache profile-aware by including API URL
Summary
Fixes #543 - The regions cache at
~/.config/agentuity/regions.jsonwas not profile-aware, causing the wrong region to be used after switching profiles. When a user switched from a local profile to production, the cache still contained regions from the local API endpoint, resulting in projects being created withregion: "local"instead of the production region.Changes:
apiUrlfield toRegionsCacheDatainterface to track which API the cache was fetched fromgetCachedRegions()to validate the cached API URL matches the current profile's API URLsaveRegionsCache()to include the API URL when savingresolveRegion()to passapiUrl: getAPIBaseURL(baseCtx.config)The
apiUrlfield is optional for backward compatibility - existing cache files without this field will continue to work.Review & Testing Checklist for Human
~/.config/agentuity/regions.json, create another project - verify the new project has the correct production region (not "local")resolveRegion()calls inregisterSubcommand()andregisterCommands()now includeapiUrl: getAPIBaseURL(baseCtx.config)apiUrlfield still works (should be used if within TTL)Recommended test plan:
agentuity.jsonhas the correct production regionNotes
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.