feat(integrations): Add support for federated auth integrations, starting with BigQuery

[tkislan]

@coderabbitai ignore

Summary by CodeRabbit

Release Notes

• New Features
  • Added Google OAuth authentication support for BigQuery integrations with token status tracking in the integration panel.
  • Integration panel now displays authentication status and provides dedicated authenticate/re-authenticate buttons for BigQuery.
  • Kernels automatically restart when integration authentication tokens are refreshed or updated.
  • Unused authentication tokens are automatically cleaned up when integrations are removed or credentials change.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR implements federated authentication for BigQuery integrations in the VS Code Deepnote extension. It introduces OAuth-based token management with refresh logic, a loopback browser flow for acquiring credentials, SQL code generation that embeds fresh access tokens, and extension services that manage token lifecycle. The webview now displays token status and supports re-authentication. Cell execution conditionally runs a silent prelude to set connection context before executing federated SQL blocks. Kernel restarts are triggered when tokens change, and stale tokens are pruned when integrations are removed.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0%. Comparing base (8efe0ed) to head (be532fe).
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@     Coverage Diff     @@
##   main   #399   +/-   ##
===========================
===========================

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[coderabbitai]

Actionable comments posted: 9

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/kernels/execution/cellExecution.federatedAuth.unit.test.ts`:
- Around line 217-225: Replace the multiple individual property assertions on
the first call with a single assert.deepStrictEqual comparing the full request
payload: build an expected object for the prelude (e.g., { code: prelude,
silent: true, store_history: false, allow_stdin: false, stop_on_error: true })
and call assert.deepStrictEqual(preludeContent, expectedPrelude); keep the
existing boolean check for preludeDispose if desired or include it in a second
deepStrictEqual against the full args tuple; do the same refactor for the second
call (calls[1] / its requestArgs/requestContent) referenced in the comment so
each payload comparison uses a single assert.deepStrictEqual rather than many
assert.strictEqual calls.

In `@src/kernels/execution/cellExecution.ts`:
- Around line 486-500: The current await of
kernelConnection.requestExecute(...).done ignores execute_reply content.status
so errors inside the federated prelude don't throw; change the call to capture
the reply (e.g., const reply = await
kernelConnection.requestExecute({...}).done), then inspect reply.content?.status
and if it equals "error" call logger.error with context including
federated.prelude and the reply content, and return
this.completedWithErrors(reply) (or an Error created from the reply) to stop
main execution; keep existing try/catch for transport exceptions around the same
kernelConnection.requestExecute call and use federated.prelude,
kernelConnection.requestExecute, this.completedWithErrors, and logger.error to
locate where to implement the check.

In
`@src/notebooks/deepnote/integrations/federatedAuth/federatedAuthInvariants.unit.test.ts`:
- Around line 10-13: The current conditional type distributes over the union
'accessToken' | 'expiresAt' causing a false negative; update the invariant to
test the union as a whole by using a non-distributive or aggregate check (for
example, use Extract<keyof FederatedAuthTokenEntry, Forbidden> extends never ?
true : never or wrap Forbidden in a tuple like [Forbidden] extends [keyof
FederatedAuthTokenEntry] ?) so that the test fails if any forbidden key is
present; change the AssertEntryOmitsForbidden definition that references
FederatedAuthTokenEntry and Forbidden accordingly and keep the rest of the
assertion (_entryShapeCheck and void usage) intact.

In
`@src/notebooks/deepnote/integrations/federatedAuth/federatedAuthKernelRestartBridge.node.unit.test.ts`:
- Line 83: Replace the repeated magic-number waits by extracting a named
constant and helper: add a top-of-module constant (e.g., const TEST_WAIT_MS =
10) and a small async helper function (e.g., async function waitMs(ms =
TEST_WAIT_MS) { return new Promise(resolve => setTimeout(resolve, ms)); }) and
then replace every occurrence of await new Promise((resolve) =>
setTimeout(resolve, 10)); in the tests with await waitMs(); so fixtures and
tests (references in federatedAuthKernelRestartBridge.node.unit.test.ts) use the
shared constant and helper to avoid duplication.

In
`@src/notebooks/deepnote/integrations/federatedAuth/federatedAuthOrphanedTokenCleaner.node.ts`:
- Around line 31-33: In FederatedAuthOrphanedTokenCleaner, the activate() method
is currently a no-op so any orphaned tokens present at startup are never
cleaned; update activate() to invoke the existing orphaned-token cleanup routine
used for integration-change events (call the exact method name in this class —
e.g., cleanOrphanedTokens(), runOrphanedTokenCleanup(), or similar) so cleanup
runs once on activation, await or handle the returned Promise if the cleanup is
async, and catch/log any errors so activation doesn't crash.

In `@src/notebooks/deepnote/integrations/integrationWebview.ts`:
- Around line 665-666: When deleting an integration, token cleanup
(tokenStorage.delete(integrationId)) must not be allowed to abort the overall
delete/reset flow if it fails; wrap the tokenStorage.delete(integrationId) calls
(the ones at/around where integrationStorage.delete(...) is called in
integrationWebview.ts) in a try/catch, log the error (or call process/project
logger) inside the catch, and do not rethrow so the in-memory and project state
transitions (the integrationStorage.delete and subsequent state updates) always
complete; apply the same pattern to both occurrences (the calls at the two
locations noted).
- Around line 50-52: The event callback passed to tokenStorage.onDidChangeTokens
calls this.updateWebview() without handling rejections, which can produce
unhandled promise rejections; change the callback so it consumes errors from
updateWebview (for example replace void this.updateWebview() with
this.updateWebview().catch(err => { /* handle or log the error */ }) or wrap the
call in an async IIFE with try/catch) so any rejection from updateWebview is
caught; reference tokenStorage.onDidChangeTokens and the updateWebview method
when making the change.

In
`@src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts`:
- Around line 110-116: The Promise.all call loading integration configs will
abort all work if any getIntegrationConfig rejects; change to per-item error
isolation by using Promise.allSettled over projectIntegrations.map(...) (or wrap
each await in try/catch inside the map) to collect only fulfilled results into
allConfigs, filter out rejected/undefined entries, and log the individual errors
(including integration.id) so one failing getIntegrationConfig call in
integrationStorage.getIntegrationConfig does not prevent returning other env
vars (including DuckDB).

In `@src/webviews/webview-side/integrations/BigQueryForm.tsx`:
- Around line 44-55: buildInitialConfig currently returns a blind clone of
existingConfig which can carry an unsupported authMethod into the form; change
it to clone the object (structuredClone(existingConfig)) then validate
existingConfig.authMethod against the supported set (e.g., 'service-account' and
'google-oauth') and if it's not supported, normalize it to a safe default (for
example 'service-account') and clear or reset any auth-specific fields (e.g.,
serviceAccountKey, oauthToken, oauthClientId/secrets) so the form doesn't start
in an invalid state; use the function buildInitialConfig and
createEmptyBigQueryConfig as anchors for where to perform this normalization.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 36bbea8e-d36e-4a6d-b50b-3de8abb8e3e5

📥 Commits

Reviewing files that changed from the base of the PR and between 8efe0ed and 4da300a.

⛔ Files ignored due to path filters (1)

• package-lock.json is excluded by !**/package-lock.json

📒 Files selected for processing (41)

• package.json
• package.nls.json
• src/kernels/execution/cellExecution.federatedAuth.unit.test.ts
• src/kernels/execution/cellExecution.ts
• src/kernels/kernelExecution.ts
• src/kernels/kernelProvider.node.ts
• src/messageTypes.ts
• src/notebooks/deepnote/integrations/federatedAuth/federatedAuthCommandHandler.node.ts
• src/notebooks/deepnote/integrations/federatedAuth/federatedAuthCommandHandler.node.unit.test.ts
• src/notebooks/deepnote/integrations/federatedAuth/federatedAuthCommandHandler.web.ts
• src/notebooks/deepnote/integrations/federatedAuth/federatedAuthCommandHandler.web.unit.test.ts
• src/notebooks/deepnote/integrations/federatedAuth/federatedAuthInvariants.unit.test.ts
• src/notebooks/deepnote/integrations/federatedAuth/federatedAuthKernelRestartBridge.node.ts
• src/notebooks/deepnote/integrations/federatedAuth/federatedAuthKernelRestartBridge.node.unit.test.ts
• src/notebooks/deepnote/integrations/federatedAuth/federatedAuthOrphanedTokenCleaner.node.ts
• src/notebooks/deepnote/integrations/federatedAuth/federatedAuthOrphanedTokenCleaner.node.unit.test.ts
• src/notebooks/deepnote/integrations/federatedAuth/federatedAuthSqlBlockCodeGenerator.node.ts
• src/notebooks/deepnote/integrations/federatedAuth/federatedAuthSqlBlockCodeGenerator.node.unit.test.ts
• src/notebooks/deepnote/integrations/federatedAuth/federatedAuthTokenStorage.node.ts
• src/notebooks/deepnote/integrations/federatedAuth/federatedAuthTokenStorage.node.unit.test.ts
• src/notebooks/deepnote/integrations/federatedAuth/googleOAuthProvider.node.ts
• src/notebooks/deepnote/integrations/federatedAuth/googleOAuthProvider.node.unit.test.ts
• src/notebooks/deepnote/integrations/federatedAuth/oauthLoopbackFlow.node.ts
• src/notebooks/deepnote/integrations/federatedAuth/oauthLoopbackFlow.node.unit.test.ts
• src/notebooks/deepnote/integrations/federatedAuth/vendoredBlocksHelpers.ts
• src/notebooks/deepnote/integrations/federatedAuth/vendoredBlocksHelpers.unit.test.ts
• src/notebooks/deepnote/integrations/integrationWebview.ts
• src/notebooks/deepnote/integrations/integrationWebview.unit.test.ts
• src/notebooks/deepnote/integrations/types.ts
• src/notebooks/serviceRegistry.node.ts
• src/notebooks/serviceRegistry.web.ts
• src/platform/common/constants.ts
• src/platform/common/utils/localize.ts
• src/platform/notebooks/deepnote/integrationTypes.ts
• src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts
• src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts
• src/webviews/webview-side/integrations/BigQueryForm.tsx
• src/webviews/webview-side/integrations/IntegrationItem.tsx
• src/webviews/webview-side/integrations/IntegrationList.tsx
• src/webviews/webview-side/integrations/IntegrationPanel.tsx
• src/webviews/webview-side/integrations/types.ts

[coderabbitai]

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Prefer deepStrictEqual for request payload object comparisons.

These assertions are object-shape checks and should use one assert.deepStrictEqual(...) per payload to match the unit-test guideline and reduce drift.

Suggested refactor

-        assert.strictEqual(preludeContent.code, prelude);
-        assert.strictEqual(preludeContent.silent, true);
-        assert.strictEqual(preludeContent.store_history, false);
-        assert.strictEqual(preludeContent.allow_stdin, false);
-        assert.strictEqual(preludeContent.stop_on_error, true);
+        assert.deepStrictEqual(preludeContent, {
+            code: prelude,
+            silent: true,
+            store_history: false,
+            allow_stdin: false,
+            stop_on_error: true
+        });

...
-        assert.strictEqual(mainContent.code, cellCode);
-        assert.strictEqual(mainContent.silent, false);
-        assert.strictEqual(mainContent.store_history, true);
+        assert.deepStrictEqual(mainContent, {
+            code: cellCode,
+            silent: false,
+            stop_on_error: false,
+            allow_stdin: true,
+            store_history: true
+        });

As per coding guidelines: "Unit tests use Mocha/Chai framework with `.unit.test.ts` extension Use `assert.deepStrictEqual()` for object comparisons instead of checking individual properties".

Also applies to: 227-233

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/kernels/execution/cellExecution.federatedAuth.unit.test.ts` around lines
217 - 225, Replace the multiple individual property assertions on the first call
with a single assert.deepStrictEqual comparing the full request payload: build
an expected object for the prelude (e.g., { code: prelude, silent: true,
store_history: false, allow_stdin: false, stop_on_error: true }) and call
assert.deepStrictEqual(preludeContent, expectedPrelude); keep the existing
boolean check for preludeDispose if desired or include it in a second
deepStrictEqual against the full args tuple; do the same refactor for the second
call (calls[1] / its requestArgs/requestContent) referenced in the comment so
each payload comparison uses a single assert.deepStrictEqual rather than many
assert.strictEqual calls.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

In @jupyterlab/servicesv7.x, doeskernel.requestExecute(...).donereject onexecute_replywithstatus: "error", or resolve with an error-status reply?

💡 Result:

In @jupyterlab/services v7.x, the.done promise of a kernel future (returned by kernel.requestExecute) resolves with the execute_reply message, even when the kernel returns an execute_reply with status: "error" [1][2][3]. The.done promise is designed to resolve once the kernel has processed the request and sent the final idle status message [1][2]. It does not automatically reject if the code execution results in a kernel-side error [1][2]. If your application needs to handle errors during execution, you must inspect the content of the resolved execute_reply message to check if its status field is "error" [4]. Key points regarding this behavior: - The.done promise fulfills when the shell reply is received, validated, and the idle iopub status is received [1][3]. - The Jupyter messaging protocol distinguishes between protocol-level errors (like message format issues) and execution-level errors, where an execute_reply with status: "error" indicates the code failed to execute in the kernel [4]. - Because.done resolves regardless of whether the execution was successful or resulted in an error, relying on it to catch execution failures will result in "success" states for failed code execution [1][2]. Always check the status of the reply object [4].

Citations:

• 1: https://jupyterlab.readthedocs.io/en/stable/api/classes/services.KernelConnection.html
• 2: https://jupyterlab.readthedocs.io/en/4.0.x/api/interfaces/services.Kernel.IKernelConnection-1.html
• 3: https://github.com/jupyterlab/jupyterlab/blob/master/packages/services/src/kernel/kernel.ts
• 4: https://jupyter-client.readthedocs.io/en/latest/messaging.html

---

Handle federated prelude execution errors via execute_reply.content.status
.done resolves even when the kernel returns an execute_reply with content.status: "error", so the current try/catch won’t stop main execution on a failed federated prelude.

Suggested fix

-                await kernelConnection.requestExecute(
+                const preludeReply = await kernelConnection.requestExecute(
                     {
                         code: federated.prelude,
                         silent: true,
                         store_history: false,
                         allow_stdin: false,
                         stop_on_error: true
                     },
                     /* dispose: */ true,
                     /* metadata: */ undefined
                 ).done;
+                if (preludeReply.content.status === 'error') {
+                    return this.completedWithErrors(new KernelError(preludeReply.content), undefined, false);
+                }
             } catch (ex) {
                 logger.error(`Federated pre-execute failed for cell Index ${this.cell.index}`, ex);
                 return this.completedWithErrors(ex);
             }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/kernels/execution/cellExecution.ts` around lines 486 - 500, The current
await of kernelConnection.requestExecute(...).done ignores execute_reply
content.status so errors inside the federated prelude don't throw; change the
call to capture the reply (e.g., const reply = await
kernelConnection.requestExecute({...}).done), then inspect reply.content?.status
and if it equals "error" call logger.error with context including
federated.prelude and the reply content, and return
this.completedWithErrors(reply) (or an Error created from the reply) to stop
main execution; keep existing try/catch for transport exceptions around the same
kernelConnection.requestExecute call and use federated.prelude,
kernelConnection.requestExecute, this.completedWithErrors, and logger.error to
locate where to implement the check.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="src/notebooks/deepnote/integrations/federatedAuth/federatedAuthInvariants.unit.test.ts"

echo "== File exists? =="
ls -l "$FILE"

echo
echo "== Lines 1-80 =="
nl -ba "$FILE" | sed -n '1,120p'

Repository: deepnote/vscode-deepnote

Length of output: 303

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="src/notebooks/deepnote/integrations/federatedAuth/federatedAuthInvariants.unit.test.ts"

echo "== Lines 1-80 (with line numbers) =="
cat -n "$FILE" | sed -n '1,120p'

echo
echo "== Search for FederatedAuthTokenEntry and forbidden invariant =="
rg -n "FederatedAuthTokenEntry|Forbidden|AssertEntryOmitsForbidden|_entryShapeCheck" "$FILE"

Repository: deepnote/vscode-deepnote

Length of output: 1956

---

Fix forbidden-key type invariant in federatedAuthInvariants.unit.test.ts (lines 10-13).

Forbidden extends keyof FederatedAuthTokenEntry ? ... distributes over the 'accessToken' | 'expiresAt' union, so the check only fails when both forbidden keys are present—if only one is added, it still passes.

Proposed fix

 type Forbidden = 'accessToken' | 'expiresAt';
-type AssertEntryOmitsForbidden = Forbidden extends keyof FederatedAuthTokenEntry ? never : true;
+type PresentForbidden = Extract<keyof FederatedAuthTokenEntry, Forbidden>;
+type AssertEntryOmitsForbidden = PresentForbidden extends never ? true : never;
 const _entryShapeCheck: AssertEntryOmitsForbidden = true;

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	type Forbidden = 'accessToken' | 'expiresAt';
	type AssertEntryOmitsForbidden = Forbidden extends keyof FederatedAuthTokenEntry ? never : true;
	const _entryShapeCheck: AssertEntryOmitsForbidden = true;
	void _entryShapeCheck;
	type Forbidden = 'accessToken' | 'expiresAt';
	type PresentForbidden = Extract<keyof FederatedAuthTokenEntry, Forbidden>;
	type AssertEntryOmitsForbidden = PresentForbidden extends never ? true : never;
	const _entryShapeCheck: AssertEntryOmitsForbidden = true;
	void _entryShapeCheck;

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/notebooks/deepnote/integrations/federatedAuth/federatedAuthInvariants.unit.test.ts`
around lines 10 - 13, The current conditional type distributes over the union
'accessToken' | 'expiresAt' causing a false negative; update the invariant to
test the union as a whole by using a non-distributive or aggregate check (for
example, use Extract<keyof FederatedAuthTokenEntry, Forbidden> extends never ?
true : never or wrap Forbidden in a tuple like [Forbidden] extends [keyof
FederatedAuthTokenEntry] ?) so that the test fails if any forbidden key is
present; change the AssertEntryOmitsForbidden definition that references
FederatedAuthTokenEntry and Forbidden accordingly and keep the rest of the
assertion (_entryShapeCheck and void usage) intact.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Replace repeated timed waits with a shared helper and named constant.

These repeated setTimeout(..., 10) waits are a magic number and duplicate logic across tests.

Proposed refactor

 suite('FederatedAuthKernelRestartBridge', () => {
+    const BRIDGE_ASYNC_SETTLE_MS = 10;
+    const waitForBridgeAsyncSettle = () =>
+        new Promise((resolve) => setTimeout(resolve, BRIDGE_ASYNC_SETTLE_MS));
+
     let bridge: FederatedAuthKernelRestartBridge;
@@
-        await new Promise((resolve) => setTimeout(resolve, 10));
+        await waitForBridgeAsyncSettle();
@@
-        await new Promise((resolve) => setTimeout(resolve, 10));
+        await waitForBridgeAsyncSettle();
@@
-        await new Promise((resolve) => setTimeout(resolve, 10));
+        await waitForBridgeAsyncSettle();
@@
-        await new Promise((resolve) => setTimeout(resolve, 10));
+        await waitForBridgeAsyncSettle();
@@
-        await new Promise((resolve) => setTimeout(resolve, 10));
+        await waitForBridgeAsyncSettle();
@@
-        await new Promise((resolve) => setTimeout(resolve, 10));
+        await waitForBridgeAsyncSettle();
@@
-        await new Promise((resolve) => setTimeout(resolve, 10));
+        await waitForBridgeAsyncSettle();
@@
-        await new Promise((resolve) => setTimeout(resolve, 10));
+        await waitForBridgeAsyncSettle();

As per coding guidelines, “Extract magic numbers (retry counts, delays, timeouts) as named constants near the top of the module” and “Extract duplicate logic into helper methods to prevent drift following DRY principle.”

Also applies to: 107-107, 131-131, 149-149, 166-166, 189-189, 206-206, 232-232

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/notebooks/deepnote/integrations/federatedAuth/federatedAuthKernelRestartBridge.node.unit.test.ts`
at line 83, Replace the repeated magic-number waits by extracting a named
constant and helper: add a top-of-module constant (e.g., const TEST_WAIT_MS =
10) and a small async helper function (e.g., async function waitMs(ms =
TEST_WAIT_MS) { return new Promise(resolve => setTimeout(resolve, ms)); }) and
then replace every occurrence of await new Promise((resolve) =>
setTimeout(resolve, 10)); in the tests with await waitMs(); so fixtures and
tests (references in federatedAuthKernelRestartBridge.node.unit.test.ts) use the
shared constant and helper to avoid duplication.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Run orphaned-token cleanup once on activation.

Right now cleanup runs only after future integration-change events. Existing orphaned tokens at startup are left behind indefinitely if no changes occur.

Proposed fix

 public activate(): void {
-    // Service is activated via constructor.
+    void this.cleanupOrphanedTokens().catch((err) =>
+        logger.error('FederatedAuthOrphanedTokenCleaner: Initial orphaned token cleanup failed', err)
+    );
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/notebooks/deepnote/integrations/federatedAuth/federatedAuthOrphanedTokenCleaner.node.ts`
around lines 31 - 33, In FederatedAuthOrphanedTokenCleaner, the activate()
method is currently a no-op so any orphaned tokens present at startup are never
cleaned; update activate() to invoke the existing orphaned-token cleanup routine
used for integration-change events (call the exact method name in this class —
e.g., cleanOrphanedTokens(), runOrphanedTokenCleanup(), or similar) so cleanup
runs once on activation, await or handle the returned Promise if the cleanup is
async, and catch/log any errors so activation doesn't crash.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Handle token-change refresh failures to avoid unhandled promise rejections.

At Line 50, void this.updateWebview() can reject and bubble as an unhandled rejection from the event callback.

Proposed fix

 this.tokenStorageDisposables.push(
     this.tokenStorage.onDidChangeTokens(() => {
-        void this.updateWebview();
+        void this.updateWebview().catch((err) => {
+            logger.error('IntegrationWebviewProvider: failed to refresh webview after token change', err);
+        });
     })
 );

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	this.tokenStorage.onDidChangeTokens(() => {
	void this.updateWebview();
	})
	this.tokenStorage.onDidChangeTokens(() => {
	void this.updateWebview().catch((err) => {
	logger.error('IntegrationWebviewProvider: failed to refresh webview after token change', err);
	});
	})

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/notebooks/deepnote/integrations/integrationWebview.ts` around lines 50 -
52, The event callback passed to tokenStorage.onDidChangeTokens calls
this.updateWebview() without handling rejections, which can produce unhandled
promise rejections; change the callback so it consumes errors from updateWebview
(for example replace void this.updateWebview() with
this.updateWebview().catch(err => { /* handle or log the error */ }) or wrap the
call in an async IIFE with try/catch) so any rejection from updateWebview is
caught; reference tokenStorage.onDidChangeTokens and the updateWebview method
when making the change.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Don’t let token cleanup failure break reset/delete state transitions.

At Line 665 and Line 701, token deletion is a secondary cleanup step, but if it throws after integrationStorage.delete() succeeds, the method aborts and leaves in-memory/project state stale.

Proposed fix

+    private async deleteFederatedTokenBestEffort(integrationId: string): Promise<void> {
+        if (!this.tokenStorage) {
+            return;
+        }
+        try {
+            await this.tokenStorage.delete(integrationId);
+        } catch (error) {
+            logger.warn(`IntegrationWebviewProvider: failed to delete federated token for ${integrationId}`, error);
+        }
+    }

     private async resetConfiguration(integrationId: string): Promise<void> {
         try {
             await this.integrationStorage.delete(integrationId);
-            await this.tokenStorage?.delete(integrationId);
+            await this.deleteFederatedTokenBestEffort(integrationId);
             // ...
         } catch (error) {
             // ...
         }
     }

     private async deleteConfiguration(integrationId: string): Promise<void> {
         try {
             await this.integrationStorage.delete(integrationId);
-            await this.tokenStorage?.delete(integrationId);
+            await this.deleteFederatedTokenBestEffort(integrationId);
             // ...
         } catch (error) {
             // ...
         }
     }

Also applies to: 701-702

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/notebooks/deepnote/integrations/integrationWebview.ts` around lines 665 -
666, When deleting an integration, token cleanup
(tokenStorage.delete(integrationId)) must not be allowed to abort the overall
delete/reset flow if it fails; wrap the tokenStorage.delete(integrationId) calls
(the ones at/around where integrationStorage.delete(...) is called in
integrationWebview.ts) in a try/catch, log the error (or call process/project
logger) inside the catch, and do not rethrow so the in-memory and project state
transitions (the integrationStorage.delete and subsequent state updates) always
complete; apply the same pattern to both occurrences (the calls at the two
locations noted).

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Harden integration-config loading so one failure doesn’t abort all env vars.

If one getIntegrationConfig() rejects, Promise.all rejects and you return no vars at all (including valid integrations + DuckDB). Use per-item error isolation (allSettled or try/catch in map).

Suggested fix

-        const allConfigs: Array<DatabaseIntegrationConfig> = (
-            await Promise.all(
-                projectIntegrations.map((integration) => {
-                    return this.integrationStorage.getIntegrationConfig(integration.id);
-                })
-            )
-        ).filter((config) => config != null);
+        const configResults = await Promise.allSettled(
+            projectIntegrations.map((integration) => this.integrationStorage.getIntegrationConfig(integration.id))
+        );
+        const allConfigs: Array<DatabaseIntegrationConfig> = configResults.flatMap((result, index) => {
+            if (result.status === 'fulfilled') {
+                return result.value ? [result.value] : [];
+            }
+            logger.error(
+                `SqlIntegrationEnvironmentVariablesProvider: Failed to load integration config ${projectIntegrations[index].id}`,
+                result.reason
+            );
+            return [];
+        });

As per coding guidelines: "Use per-iteration error handling in loops - wrap each iteration in try/catch so one failure doesn't stop the rest".

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts`
around lines 110 - 116, The Promise.all call loading integration configs will
abort all work if any getIntegrationConfig rejects; change to per-item error
isolation by using Promise.allSettled over projectIntegrations.map(...) (or wrap
each await in try/catch inside the map) to collect only fulfilled results into
allConfigs, filter out rejected/undefined entries, and log the individual errors
(including integration.id) so one failing getIntegrationConfig call in
integrationStorage.getIntegrationConfig does not prevent returning other env
vars (including DuckDB).

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Normalize unsupported persisted auth methods in initial state.

At Line 54, cloning existingConfig blindly can trap the form in an invalid state if backend data contains an unexpected authMethod.

Proposed fix

 function buildInitialConfig(
     existingConfig: BigQueryConfig | null,
     integrationId: string,
     defaultName?: string
 ): BigQueryConfig {
     if (!existingConfig) {
         return createEmptyBigQueryConfig({ id: integrationId, name: defaultName });
     }
-    // Preserve existing config when its auth method is supported. Both
-    // service-account and google-oauth are editable in this milestone.
-    return structuredClone(existingConfig);
+    if (!isBigQueryAuthMethod(existingConfig.metadata.authMethod)) {
+        return createEmptyBigQueryConfig({
+            id: integrationId,
+            name: existingConfig.name || defaultName,
+            authMethod: BigQueryAuthMethods.ServiceAccount
+        });
+    }
+    return structuredClone(existingConfig);
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/webviews/webview-side/integrations/BigQueryForm.tsx` around lines 44 -
55, buildInitialConfig currently returns a blind clone of existingConfig which
can carry an unsupported authMethod into the form; change it to clone the object
(structuredClone(existingConfig)) then validate existingConfig.authMethod
against the supported set (e.g., 'service-account' and 'google-oauth') and if
it's not supported, normalize it to a safe default (for example
'service-account') and clear or reset any auth-specific fields (e.g.,
serviceAccountKey, oauthToken, oauthClientId/secrets) so the form doesn't start
in an invalid state; use the function buildInitialConfig and
createEmptyBigQueryConfig as anchors for where to perform this normalization.