Address connector authentication review feedback

Keep terminal sign-in status retry-safe, preserve operational Azure Identity errors, retry transient legacy-cache cleanup, clarify secure persistence requirements, and fix the blocking spelling check.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

extensions/connector-namespaces/README.md

@@ -36,6 +36,10 @@ and select **Install in GitHub Copilot app**.
 - Permission to view the namespace and create its connections and hosted MCP
   server configurations.
 - A browser for Microsoft Entra sign-in and connector consent.
+- An operating-system secure credential store. Windows and macOS provide one by
+  default. Linux and WSL require a Secret Service-compatible keyring, such as
+  GNOME Keyring, with `libsecret` available. Unencrypted token storage is
+  intentionally disabled.
 
 Connector Namespace is currently an Azure preview service and availability can
 vary by region.
@@ -60,9 +64,13 @@ playground. Use **Change namespace** to switch subscriptions or namespaces.
 Azure sign-in and connector sign-in are separate:
 
 - **Azure sign-in** lets the canvas discover and manage Connector Namespace
-  resources. The account cache is stored in the operating system's encrypted
-  credential store; raw access and refresh tokens are not written to extension
-  files.
+  resources. Access and refresh tokens are stored in the operating system's
+  encrypted credential store. To select that encrypted cache entry after an app
+  restart, the extension separately saves a non-secret authentication record
+  containing the authority, client ID, account ID, tenant ID, and username under
+  `~/.copilot/extensions/connector-namespaces/artifacts/azure-auth-record.json`,
+  with user-only permissions where supported. Raw tokens are never written to
+  extension files.
 - **Connector sign-in** grants an individual MCP server access to its backing
   service. The resulting connection is managed by Connector Namespace.
 

extensions/connector-namespaces/auth.mjs

@@ -129,7 +129,13 @@ export class InteractiveAuthBroker {
 
     ensureLegacyCredentialsRemoved() {
         if (!this.cleanupInFlight) {
-            this.cleanupInFlight = Promise.resolve().then(() => this.cleanupLegacyCredentials());
+            const cleanup = Promise.resolve()
+                .then(() => this.cleanupLegacyCredentials())
+                .catch((error) => {
+                    if (this.cleanupInFlight === cleanup) this.cleanupInFlight = null;
+                    throw error;
+                });
+            this.cleanupInFlight = cleanup;
         }
         return this.cleanupInFlight;
     }
@@ -198,7 +204,6 @@ export class InteractiveAuthBroker {
         if (!session) return { ok: false, status: "unknown" };
         if (session.status === "pending") return { ok: true, status: "pending", mode: "interactive" };
 
-        this.sessions.delete(sessionId);
         if (session.status === "done") return { ok: true, status: "done" };
         if (session.status === "cancelled") return { ok: true, status: "cancelled" };
         return { ok: false, status: "error", error: session.error || "Azure sign-in failed." };
@@ -224,10 +229,13 @@ export class InteractiveAuthBroker {
                 credential = this.createInteractiveCredential(authenticationRecord);
                 this.credential = credential;
             } catch (error) {
-                throw new ConnectorAuthenticationRequiredError(
-                    "Azure sign-in is unavailable. Check the secure token cache installation and try again.",
-                    { cause: error },
-                );
+                if (isAuthenticationRequiredError(error)) {
+                    throw new ConnectorAuthenticationRequiredError(
+                        "Sign in to Azure to continue.",
+                        { cause: error },
+                    );
+                }
+                throw error;
             }
         }
         const request = credential.getToken(this.scope)
@@ -239,6 +247,7 @@ export class InteractiveAuthBroker {
                 return accessToken.token;
             })
             .catch((error) => {
+                if (!isAuthenticationRequiredError(error)) throw error;
                 if (this.credential === credential) {
                     this.credential = null;
                     this.accessToken = null;

extensions/connector-namespaces/auth.test.mjs

@@ -96,6 +96,7 @@ test("interactive sign-in reports pending then done and caches the ARM token", a
     });
     assert.equal(authenticateOptions.abortSignal.aborted, false);
     assert.deepEqual(broker.getSignInStatus(started.sessionId), { ok: true, status: "done" });
+    assert.deepEqual(broker.getSignInStatus(started.sessionId), { ok: true, status: "done" });
     assert.equal(await broker.getToken(), "token");
 });
 
@@ -150,7 +151,9 @@ test("cancelling sign-in aborts the credential request", async () => {
             });
         },
         async getToken() {
-            throw new Error("getToken should not run after cancellation");
+            const error = new Error("No cached account found.");
+            error.name = "AuthenticationRequiredError";
+            throw error;
         },
     };
     const broker = new InteractiveAuthBroker({
@@ -170,6 +173,7 @@ test("cancelling sign-in aborts the credential request", async () => {
 
     assert.equal(abortSignal.aborted, true);
     assert.deepEqual(broker.getSignInStatus(started.sessionId), { ok: true, status: "cancelled" });
+    assert.deepEqual(broker.getSignInStatus(started.sessionId), { ok: true, status: "cancelled" });
     await assert.rejects(broker.getToken(), ConnectorAuthenticationRequiredError);
 });
 
@@ -195,4 +199,70 @@ test("sign-in failures are surfaced through the status endpoint contract", async
         status: "error",
         error: "browser launch failed",
     });
+    assert.deepEqual(broker.getSignInStatus(started.sessionId), {
+        ok: false,
+        status: "error",
+        error: "browser launch failed",
+    });
+});
+
+test("legacy credential cleanup retries after a transient failure", async () => {
+    let cleanupCalls = 0;
+    const broker = new InteractiveAuthBroker({
+        cleanupLegacyCredentials: async () => {
+            cleanupCalls++;
+            if (cleanupCalls === 1) throw new Error("legacy cache is locked");
+        },
+        createCredential: () => ({
+            async getToken() {
+                return accessToken();
+            },
+        }),
+        loadAuthRecord: async () => authenticationRecord(),
+    });
+
+    await assert.rejects(broker.getToken(), /legacy cache is locked/);
+    assert.equal(await broker.getToken(), "token");
+    assert.equal(cleanupCalls, 2);
+});
+
+test("token acquisition preserves operational errors and retries the credential", async () => {
+    const outage = new Error("Azure Identity network request timed out");
+    let createCredentialCalls = 0;
+    let tokenCalls = 0;
+    const broker = new InteractiveAuthBroker({
+        createCredential: () => {
+            createCredentialCalls++;
+            return {
+                async getToken() {
+                    tokenCalls++;
+                    if (tokenCalls === 1) throw outage;
+                    return accessToken();
+                },
+            };
+        },
+        loadAuthRecord: async () => authenticationRecord(),
+    });
+
+    await assert.rejects(broker.getToken(), (error) => error === outage);
+    assert.equal(await broker.getToken(), "token");
+    assert.equal(createCredentialCalls, 1);
+    assert.equal(tokenCalls, 2);
+});
+
+test("incomplete tokens remain operational errors instead of prompting sign-in", async () => {
+    const broker = new InteractiveAuthBroker({
+        createCredential: () => ({
+            async getToken() {
+                return { token: "incomplete" };
+            },
+        }),
+        loadAuthRecord: async () => authenticationRecord(),
+    });
+
+    await assert.rejects(
+        broker.getToken(),
+        (error) => !(error instanceof ConnectorAuthenticationRequiredError)
+            && error.message === "Azure identity returned an incomplete ARM access token.",
+    );
 });

extensions/connector-namespaces/renderer.mjs

@@ -468,7 +468,7 @@ async function loadSubscriptions(force) {
     } catch (error) {
         setupContent.hidden = false;
         signinPanel.hidden = true;
-        gatewayList.innerHTML = '<div class="empty" style="color:var(--danger);">Couldn\\'t load subscriptions: ' + escH(error.message) + '<br><button id="retry-subscriptions" class="change-btn" type="button" style="margin-top:.6rem;">Retry</button></div>';
+        gatewayList.innerHTML = '<div class="empty" style="color:var(--danger);">Unable to load subscriptions: ' + escH(error.message) + '<br><button id="retry-subscriptions" class="change-btn" type="button" style="margin-top:.6rem;">Retry</button></div>';
         document.getElementById("retry-subscriptions").addEventListener("click", () => loadSubscriptions(true));
         return false;
     }
@@ -515,7 +515,7 @@ async function startSignin() {
         signinPanel.dataset.state = "error";
         signinPanel.setAttribute("aria-busy", "false");
         signinButton.disabled = false;
-        signinMessage.textContent = "Couldn\\'t start sign-in: " + error.message;
+        signinMessage.textContent = "Unable to start sign-in: " + error.message;
     }
 }
 

extensions/connector-namespaces/test/README.md

@@ -9,7 +9,7 @@ connect → initialize → tools/list → a safe tools/call
 
 It imports the `connector-namespaces` extension's real pipeline (`install.mjs`,
 `catalog.mjs`, `armClient.mjs`) and connects through the same native Streamable
-HTTP endpoint that the extension writes to the Copilot CLI config. The probe
+HTTP endpoint that the extension writes to the GitHub Copilot MCP config. The probe
 uses the configured `X-API-Key`, follows `Mcp-Session-Id`, and accepts standard
 JSON or SSE JSON-RPC responses.
 
@@ -27,6 +27,11 @@ MCP server issue locally.
    (`{ subscriptionId, resourceGroup, gatewayName }`). Pick a gateway once in
    the connector-namespaces canvas, or write that file by hand.
 3. **Node 20+** (developed on Node 24).
+4. **Extension dependencies installed.** From the repository root, run:
+
+   ```bash
+   npm install --prefix extensions/connector-namespaces
+   ```
 
 ## Run it
 
@@ -70,7 +75,7 @@ connector's own consent. The model:
    loopback page is just a redirect target and nothing is listening on it.
 3. **Re-run the harness.** It sees the pending record, confirms the gateway
    connection is now `Connected`, finishes the install (mints the API key,
-   writes the CLI entry), and probes it headless. From then on the connector is
+   writes the Copilot MCP entry), and probes it headless. From then on the connector is
    reused without repeating its consent.
 
 So the server taxonomy is: