From 4d25b4b784bf3720a6a376237e6aaeb8e09ecced Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Tue, 17 Mar 2026 20:35:27 +0100
Subject: [PATCH] Address remaining PR review comments

- Fix getAllInstances to handle empty instance array without throwing
- Persist updated token expiry timestamps to disk after refresh
- Mark internal httpJson helpers as @internal instead of @public

Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
Made-with: Cursor
---
 packages/cli-module-auth/src/lib/storage.ts |  3 +++
 packages/cli-node/src/auth/CliAuth.test.ts  |  4 ++++
 packages/cli-node/src/auth/CliAuth.ts       | 13 ++++++++-----
 packages/cli-node/src/auth/httpJson.ts      |  4 ++--
 packages/cli-node/src/auth/storage.ts       | 19 +++++++++++++++++++
 5 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/packages/cli-module-auth/src/lib/storage.ts b/packages/cli-module-auth/src/lib/storage.ts
index 07e48271a8..c5d275149c 100644
--- a/packages/cli-module-auth/src/lib/storage.ts
+++ b/packages/cli-module-auth/src/lib/storage.ts
@@ -96,6 +96,9 @@ export async function getAllInstances(): Promise<{
   selected: StoredInstance | undefined;
 }> {
   const { instances } = await readAll();
+  if (instances.length === 0) {
+    return { instances: [], selected: undefined };
+  }
   const selected = instances.find(i => i.selected) ?? instances[0];
   return {
     instances: instances.map(i => ({
diff --git a/packages/cli-node/src/auth/CliAuth.test.ts b/packages/cli-node/src/auth/CliAuth.test.ts
index 65aa340d33..4b7cb61f5a 100644
--- a/packages/cli-node/src/auth/CliAuth.test.ts
+++ b/packages/cli-node/src/auth/CliAuth.test.ts
@@ -145,6 +145,10 @@ describe('CliAuth', () => {
         'refreshToken',
         'new-refresh-token',
       );
+      expect(mockStorage.updateInstance).toHaveBeenCalledWith('production', {
+        issuedAt: expect.any(Number),
+        accessTokenExpiresAt: expect.any(Number),
+      });
     });
 
     it('throws when refresh token is missing and access token has expired', async () => {
diff --git a/packages/cli-node/src/auth/CliAuth.ts b/packages/cli-node/src/auth/CliAuth.ts
index d65fb9bbfa..6f1fae221e 100644
--- a/packages/cli-node/src/auth/CliAuth.ts
+++ b/packages/cli-node/src/auth/CliAuth.ts
@@ -19,6 +19,7 @@ import {
   getSelectedInstance,
   getInstanceMetadata,
   updateInstanceMetadata,
+  updateInstance,
   accessTokenNeedsRefresh,
 } from './storage';
 import { getSecretStore, type SecretStore } from './secretStore';
@@ -151,10 +152,12 @@ export class CliAuth {
     if (token.refresh_token) {
       await this.#secretStore.set(service, 'refreshToken', token.refresh_token);
     }
-    this.#instance = {
-      ...this.#instance,
-      issuedAt: Date.now(),
-      accessTokenExpiresAt: Date.now() + token.expires_in * 1000,
-    };
+    const issuedAt = Date.now();
+    const accessTokenExpiresAt = Date.now() + token.expires_in * 1000;
+    this.#instance = { ...this.#instance, issuedAt, accessTokenExpiresAt };
+    await updateInstance(this.#instance.name, {
+      issuedAt,
+      accessTokenExpiresAt,
+    });
   }
 }
diff --git a/packages/cli-node/src/auth/httpJson.ts b/packages/cli-node/src/auth/httpJson.ts
index 4861a07722..832178befb 100644
--- a/packages/cli-node/src/auth/httpJson.ts
+++ b/packages/cli-node/src/auth/httpJson.ts
@@ -16,7 +16,7 @@
 
 import { ResponseError } from '@backstage/errors';
 
-/** @public */
+/** @internal */
 export type HttpInit = {
   headers?: Record<string, string>;
   method?: string;
@@ -24,7 +24,7 @@ export type HttpInit = {
   signal?: AbortSignal;
 };
 
-/** @public */
+/** @internal */
 export async function httpJson<T>(url: string, init?: HttpInit): Promise<T> {
   const res = await fetch(url, {
     ...init,
diff --git a/packages/cli-node/src/auth/storage.ts b/packages/cli-node/src/auth/storage.ts
index 13a866d058..c58bd94505 100644
--- a/packages/cli-node/src/auth/storage.ts
+++ b/packages/cli-node/src/auth/storage.ts
@@ -124,6 +124,9 @@ export async function getAllInstances(): Promise<{
   selected: StoredInstance | undefined;
 }> {
   const { instances } = await readAll();
+  if (instances.length === 0) {
+    return { instances: [], selected: undefined };
+  }
   const selected = instances.find(i => i.selected) ?? instances[0];
   return {
     instances: instances.map(i => ({
@@ -189,6 +192,22 @@ export async function updateInstanceMetadata(
   });
 }
 
+/** @internal */
+export async function updateInstance(
+  instanceName: string,
+  updates: Partial<Pick<StoredInstance, 'issuedAt' | 'accessTokenExpiresAt'>>,
+): Promise<void> {
+  return withMetadataLock(async () => {
+    const data = await readAll();
+    const idx = data.instances.findIndex(i => i.name === instanceName);
+    if (idx === -1) {
+      throw new NotFoundError(`Instance '${instanceName}' not found`);
+    }
+    data.instances[idx] = { ...data.instances[idx], ...updates };
+    await writeAll(data);
+  });
+}
+
 /** @internal */
 export function accessTokenNeedsRefresh(instance: StoredInstance): boolean {
   // 2 minutes before expiration