fix: use ensureProjectContext for google_search project ID resolution#465
fix: use ensureProjectContext for google_search project ID resolution#465moha-abdi wants to merge 1 commit intoNoeFabris:mainfrom
Conversation
WalkthroughReplaces direct parsing of refresh token metadata with a call to Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryFixes 403 SUBSCRIPTION_REQUIRED errors in
Confidence Score: 5/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[google_search tool called] --> B{Auth valid?}
B -->|No| C[Return auth error]
B -->|Yes| D[ensureProjectContext]
D --> E{managedProjectId in token?}
E -->|Yes| F[Use managedProjectId]
E -->|No| G[loadManagedProject API call]
G --> H{Project found?}
H -->|Yes| I[Persist & use managedProjectId]
H -->|No| J[Auto-provision via onboardUser]
J --> K{Provisioning success?}
K -->|Yes| I
K -->|No| L{projectId available?}
L -->|Yes| M[Use projectId]
L -->|No| N[Use ANTIGRAVITY_DEFAULT_PROJECT_ID]
F --> O[Check access token validity]
I --> O
M --> O
N --> O
O --> P{Token expired?}
P -->|Yes| Q[refreshAccessToken]
P -->|No| R[Use existing token]
Q --> R
R --> S[executeSearch with valid projectId]
Last reviewed commit: bb3db28 |
src/plugin.ts
Outdated
| if (!accessToken || accessTokenExpired(auth)) { | ||
| try { | ||
| const refreshed = await refreshAccessToken(auth, client, providerId); | ||
| accessToken = refreshed?.access; | ||
| const refreshed = await refreshAccessToken(auth, client, providerId) |
There was a problem hiding this comment.
should check accessTokenExpired(projectContext.auth) and pass projectContext.auth to refreshAccessToken since ensureProjectContext may return updated auth with a new refresh token
| if (!accessToken || accessTokenExpired(auth)) { | |
| try { | |
| const refreshed = await refreshAccessToken(auth, client, providerId); | |
| accessToken = refreshed?.access; | |
| const refreshed = await refreshAccessToken(auth, client, providerId) | |
| if (!accessToken || accessTokenExpired(projectContext.auth)) { | |
| try { | |
| const refreshed = await refreshAccessToken(projectContext.auth, client, providerId) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/plugin.ts
Line: 1361-1363
Comment:
should check `accessTokenExpired(projectContext.auth)` and pass `projectContext.auth` to `refreshAccessToken` since `ensureProjectContext` may return updated auth with a new refresh token
```suggestion
if (!accessToken || accessTokenExpired(projectContext.auth)) {
try {
const refreshed = await refreshAccessToken(projectContext.auth, client, providerId)
```
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/plugin.ts (1)
1359-1359: Redundant|| auth.accessfallback (optional cleanup)
ensureProjectContexteither returns{ auth, … }unchanged (when there is no access token, in which case both sides are identical) or returns an updatedauthwith a freshaccessvalue. In neither path doesprojectContext.auth.accessdiffer meaningfully fromauth.accessin a way the fallback would catch. The expression can be simplified toprojectContext.auth.accesswithout any behavioral change — aligns with the fix above whereprojectContext.authbecomes the single source of truth.♻️ Proposed simplification
- let accessToken = projectContext.auth.access || auth.access + let accessToken = projectContext.auth.access🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/plugin.ts` at line 1359, The expression using a redundant fallback should be simplified: replace the use of `let accessToken = projectContext.auth.access || auth.access` with a single-source access from `projectContext.auth.access` because `ensureProjectContext` guarantees `projectContext.auth` is the authoritative auth payload; update the `accessToken` assignment to read from `projectContext.auth.access` and remove the `|| auth.access` fallback so `projectContext` is the single source of truth (refer to the `accessToken` variable, `projectContext`, `auth`, and `ensureProjectContext` usages in this file).
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
src/plugin.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/plugin.ts (4)
src/plugin/types.ts (1)
ProjectContextResult(108-111)src/plugin/project.ts (1)
ensureProjectContext(225-320)src/plugin/auth.ts (1)
accessTokenExpired(33-38)src/plugin/token.ts (1)
refreshAccessToken(85-169)
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/plugin.ts`:
- Around line 1361-1368: The expiry check and refresh call are using the
original auth instead of the possibly-updated projectContext.auth returned by
ensureProjectContext; update the google_search path to use projectContext.auth
(not the original auth) when calling accessTokenExpired and refreshAccessToken
so the code checks expires and uses the current refresh token from
projectContext.auth; ensure you seed accessToken from projectContext.auth.access
(already done) and pass projectContext.auth and providerId to refreshAccessToken
(and update accessToken from the refreshed result) to avoid false-positive
refreshes and stale refresh-token usage.
---
Nitpick comments:
In `@src/plugin.ts`:
- Line 1359: The expression using a redundant fallback should be simplified:
replace the use of `let accessToken = projectContext.auth.access || auth.access`
with a single-source access from `projectContext.auth.access` because
`ensureProjectContext` guarantees `projectContext.auth` is the authoritative
auth payload; update the `accessToken` assignment to read from
`projectContext.auth.access` and remove the `|| auth.access` fallback so
`projectContext` is the single source of truth (refer to the `accessToken`
variable, `projectContext`, `auth`, and `ensureProjectContext` usages in this
file).
539afff to
bb3db28
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/plugin.ts (1)
1351-1371: Missing semicolons throughout the changed block.The rest of this file consistently uses semicolons. The new/modified lines (1351, 1353, 1355, 1358, 1359, 1363, 1364, 1366, 1371) all omit them.
🧹 Add semicolons for consistency
- let projectContext: ProjectContextResult + let projectContext: ProjectContextResult; try { - projectContext = await ensureProjectContext(auth) + projectContext = await ensureProjectContext(auth); } catch (error) { - return `Error: Failed to resolve project context: ${error instanceof Error ? error.message : String(error)}` + return `Error: Failed to resolve project context: ${error instanceof Error ? error.message : String(error)}`; } - const projectId = projectContext.effectiveProjectId - let accessToken = projectContext.auth.access || auth.access + const projectId = projectContext.effectiveProjectId; + let accessToken = projectContext.auth.access || auth.access; if (!accessToken || accessTokenExpired(projectContext.auth)) { try { - const refreshed = await refreshAccessToken(projectContext.auth, client, providerId) - accessToken = refreshed?.access + const refreshed = await refreshAccessToken(projectContext.auth, client, providerId); + accessToken = refreshed?.access; } catch (error) { - return `Error: Failed to refresh access token: ${error instanceof Error ? error.message : String(error)}` + return `Error: Failed to refresh access token: ${error instanceof Error ? error.message : String(error)}`; } } if (!accessToken) { - return "Error: No valid access token available. Please run `opencode auth login` to re-authenticate." + return "Error: No valid access token available. Please run `opencode auth login` to re-authenticate."; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/plugin.ts` around lines 1351 - 1371, The changed block omits semicolons on several statements (variable declarations and return lines) causing inconsistency with the file style; add trailing semicolons to the statements around projectContext/ensureProjectContext, the return in the catch, the projectId assignment, the accessToken assignment, the if condition branch that calls refreshAccessToken and its catch return, and the final return message so lines referencing ProjectContextResult, ensureProjectContext, projectId, accessToken, accessTokenExpired, and refreshAccessToken all end with semicolons matching the rest of the file.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
src/plugin.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Greptile Review
🔇 Additional comments (1)
src/plugin.ts (1)
1351-1384: Core fix looks correct —ensureProjectContextproperly replaces theparseRefreshParts+"unknown"fallback.The project ID resolution now matches the model request path (compare lines 1784–1809), and the past review feedback about using
projectContext.authfor bothaccessTokenExpiredandrefreshAccessTokenhas been addressed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/plugin.ts`:
- Around line 1351-1371: The changed block omits semicolons on several
statements (variable declarations and return lines) causing inconsistency with
the file style; add trailing semicolons to the statements around
projectContext/ensureProjectContext, the return in the catch, the projectId
assignment, the accessToken assignment, the if condition branch that calls
refreshAccessToken and its catch return, and the final return message so lines
referencing ProjectContextResult, ensureProjectContext, projectId, accessToken,
accessTokenExpired, and refreshAccessToken all end with semicolons matching the
rest of the file.
…ss review issues - PR NoeFabris#476: guard isOAuthAuth against undefined/null (auth.ts) - PR NoeFabris#465: use ensureProjectContext for google_search project‑ID resolution - PR NoeFabris#460: throttle excessive file writes from account state updates - Fix abort‑during‑cooldown error‑escape (wrap all calls) - Remove stale THINKING_RECOVERY_NEEDED sentinel branch - Change abort status from HTTP 499 to standard 408 (Request Timeout) - All tests pass, type‑check clean
…ss review issues - PR NoeFabris#476: guard isOAuthAuth against undefined/null (auth.ts) - PR NoeFabris#465: use ensureProjectContext for google_search project‑ID resolution - PR NoeFabris#460: throttle excessive file writes from account state updates - Fix abort‑during‑cooldown error‑escape (wrap all calls) - Remove stale THINKING_RECOVERY_NEEDED sentinel branch - Change abort status from HTTP 499 to standard 408 (Request Timeout) - All tests pass, type‑check clean
Problem
The
google_searchtool usesparseRefreshParts()with an|| "unknown"fallback to resolve the project ID. This causes 403 SUBSCRIPTION_REQUIRED errors from cloudaicompanion.googleapis.com because"unknown"is not a valid project ID.Fix
Replace
parseRefreshPartswithensureProjectContext()— the same function already used by model requests (line ~1782). It handles managed project lookup, auto-provisioning via onboarding, and falls back toANTIGRAVITY_DEFAULT_PROJECT_ID. Also uses the refreshed auth fromensureProjectContextfor the access token.Testing
npm run typecheck)npm run build)npm test)Fixes #455.