Add URL refresh strategy for storage sync pre-signed URLs

extensions/cli/docs/storage-sync.md

@@ -40,11 +40,20 @@ This document captures the responsibilities for both the CLI and backend compone
   - Resolve `storageId` into the desired S3 prefix (e.g., `sessions/<org>/<storageId>/`).
   - Generate two short-lived pre-signed `PUT` URLs: one for `session.json`, one for `diff.txt`.
   - Return both URLs and their keys in the response payload described above.
-- **Expiration**: The initial implementation can hand out URLs with a generous TTL (e.g., 60 minutes). Later we will add CLI-side refresh logic before expiry.
+- **Expiration**: URLs are issued with a 60-minute TTL. The CLI automatically refreshes them before expiry (see URL Refresh Strategy below).
+
+## URL Refresh Strategy
+
+Pre-signed URLs are automatically refreshed using a dual-strategy approach:
+
+1. **Proactive Refresh**: URLs are refreshed at the 50-minute mark (10 minutes before expiry) to prevent disruption
+2. **Reactive Refresh**: If a 403 Forbidden error is detected (indicating expired URLs), an immediate refresh is triggered
+3. **Error Handling**: Upload errors are logged but non-fatal; the service continues running with automatic recovery
+
+This ensures continuous operation during devbox suspension, network interruptions, and clock drift.
 
 ## Open Questions & Future Enhancements
 
-- **URL refresh**: When the presigned URLs expire we currently have no refresh cycle. We'd need either a renewal endpoint or to re-call the existing endpoint on failure.
 - **Upload cadence**: The 30-second interval is hard-coded for now. Consider making it configurable in both CLI and backend policies.
 - **Error telemetry**: Decide if repeated upload failures should trip analytics or circuit breakers.
 - **Diff source**: `diff.txt` currently mirrors the `/diff` endpoint response. Confirm backend expectations for format and size limits.

extensions/cli/src/services/StorageSyncService.test.ts

@@ -276,4 +276,87 @@ describe("StorageSyncService", () => {
 
     service.stop();
   });
+
+  it("refreshes URLs on 403 error", async () => {
+    const syncSessionHistory = vi.fn();
+    const getCompleteStateSnapshot = vi.fn().mockReturnValue({ test: "data" });
+
+    // Mock initial presign success
+    fetchMock.mockResolvedValueOnce({
+      ok: true,
+      json: async () => ({
+        session: { putUrl: "https://upload/session-old", key: "session.json" },
+        diff: { putUrl: "https://upload/diff-old", key: "diff.txt" },
+      }),
+    });
+
+    // Mock 403 upload failure
+    fetchMock.mockResolvedValueOnce({
+      ok: false,
+      status: 403,
+      statusText: "Forbidden",
+      text: async () => "Request has expired",
+    });
+
+    // Mock refresh presign success
+    fetchMock.mockResolvedValueOnce({
+      ok: true,
+      json: async () => ({
+        session: { putUrl: "https://upload/session-new", key: "session.json" },
+        diff: { putUrl: "https://upload/diff-new", key: "diff.txt" },
+      }),
+    });
+
+    gitDiffMock.mockResolvedValue({ diff: "test", repoFound: true });
+
+    const result = await service.startFromOptions({
+      storageOption: "session-123",
+      accessToken: "token",
+      syncSessionHistory,
+      getCompleteStateSnapshot,
+      isActive: () => true,
+    });
+
+    expect(result).toBe(true);
+
+    // Wait for async refresh
+    await new Promise((resolve) => setTimeout(resolve, 100));
+
+    expect(fetchMock).toHaveBeenCalledTimes(3); // presign + failed upload + refresh
+    expect(service.getState().isEnabled).toBe(true);
+
+    service.stop();
+  });
+
+  it("prevents concurrent refresh requests", async () => {
+    const syncSessionHistory = vi.fn();
+    const getCompleteStateSnapshot = vi.fn().mockReturnValue({});
+
+    fetchMock.mockResolvedValue({
+      ok: true,
+      json: async () => ({
+        session: { putUrl: "https://upload/session", key: "session.json" },
+        diff: { putUrl: "https://upload/diff", key: "diff.txt" },
+      }),
+    });
+
+    await service.start({
+      storageId: "session-123",
+      accessToken: "token",
+      syncSessionHistory,
+      getCompleteStateSnapshot,
+    });
+
+    const refreshMethod = (service as any).refreshStorageTargets.bind(service);
+    const results = await Promise.all([
+      refreshMethod(),
+      refreshMethod(),
+      refreshMethod(),
+    ]);
+
+    const successCount = results.filter((r) => r === true).length;
+    expect(successCount).toBe(1); // Only first succeeds
+
+    service.stop();
+  });
 });

extensions/cli/src/services/StorageSyncService.ts

@@ -45,6 +45,9 @@ export class StorageSyncService {
   private missingRepoLogged = false;
   private targets: StorageTargets | null = null;
   private options: StorageSyncStartOptions | null = null;
+  private targetsExpiresAt: number | null = null;
+  private refreshTimer: NodeJS.Timeout | null = null;
+  private isRefreshing = false;
 
   async initialize(): Promise<StorageSyncServiceState> {
     this.stop();
@@ -112,6 +115,7 @@ export class StorageSyncService {
     }
 
     this.targets = targets;
+    this.targetsExpiresAt = Date.now() + 60 * 60 * 1000; // Track 60-minute expiration
     this.options = options;
     this.stopped = false;
     this.uploadInFlight = false;
@@ -130,10 +134,17 @@ export class StorageSyncService {
       ),
     );
 
+    // Schedule proactive URL refresh at 50-minute mark
+    this.scheduleProactiveRefresh();
+
     await this.performUpload();
 
     this.intervalHandle = setInterval(() => {
-      void this.performUpload();
+      this.performUpload().catch((error) => {
+        logger.warn(
+          `Storage sync upload failed in interval: ${formatError(error)}`,
+        );
+      });
     }, intervalMs);
 
     return true;
@@ -178,6 +189,7 @@ export class StorageSyncService {
   stop(): void {
     this.stopped = true;
     this.targets = null;
+    this.targetsExpiresAt = null;
     this.options = null;
     this.uploadInFlight = false;
 
@@ -186,6 +198,12 @@ export class StorageSyncService {
       this.intervalHandle = null;
     }
 
+    // Clear refresh timer
+    if (this.refreshTimer) {
+      clearTimeout(this.refreshTimer);
+      this.refreshTimer = null;
+    }
+
     if (this.state.isEnabled) {
       this.setState({
         isEnabled: false,
@@ -245,6 +263,59 @@ export class StorageSyncService {
     }
   }
 
+  private async refreshStorageTargets(): Promise<boolean> {
+    if (this.isRefreshing || !this.options) {
+      return false;
+    }
+
+    this.isRefreshing = true;
+    logger.info("Refreshing storage pre-signed URLs...");
+
+    try {
+      const newTargets = await this.requestStorageTargets(
+        this.options.storageId,
+        this.options.accessToken,
+      );
+
+      if (!newTargets) {
+        logger.warn(
+          "Failed to refresh storage URLs - continuing with existing URLs",
+        );
+        return false;
+      }
+
+      this.targets = newTargets;
+      this.targetsExpiresAt = Date.now() + 60 * 60 * 1000; // 60 minutes from now
+      logger.info("Storage URLs refreshed successfully");
+
+      // Schedule next proactive refresh at 50-minute mark
+      this.scheduleProactiveRefresh();
+
+      return true;
+    } catch (error) {
+      logger.warn(`Storage URL refresh error: ${formatError(error)}`);
+      return false;
+    } finally {
+      this.isRefreshing = false;
+    }
+  }
+
+  private scheduleProactiveRefresh(): void {
+    // Clear any existing refresh timer
+    if (this.refreshTimer) {
+      clearTimeout(this.refreshTimer);
+      this.refreshTimer = null;
+    }
+
+    // Schedule refresh at 50-minute mark (10 minutes before expiry)
+    const refreshIn = 50 * 60 * 1000; // 50 minutes
+    this.refreshTimer = setTimeout(() => {
+      this.refreshStorageTargets().catch((error) => {
+        logger.warn(`Proactive URL refresh failed: ${formatError(error)}`);
+      });
+    }, refreshIn);
+  }
+
   private async uploadToPresignedUrl(
     url: string,
     body: string,
@@ -271,6 +342,16 @@ export class StorageSyncService {
     if (!response.ok) {
       const statusText = `${response.status} ${response.statusText}`.trim();
       const responseBody = await response.text();
+
+      // Detect expired URL (403 Forbidden) and trigger refresh
+      if (response.status === 403) {
+        logger.warn("Storage URL expired (403), triggering refresh...");
+        // Trigger async refresh without blocking the current upload error
+        this.refreshStorageTargets().catch((error) => {
+          logger.debug(`Background URL refresh failed: ${formatError(error)}`);
+        });
+      }
+
       throw new Error(`Storage upload failed (${statusText}): ${responseBody}`);
     }
   }